Profile F# programs correctly: busy threads once per tick, arrays never located, honest truncation and error codes, fresh leak badges

.claude/skills/submit-pr/SKILL.md

@@ -1,7 +1,7 @@
 ---
 name: submit-pr
 description: Creates a pull request with a well-structured description after verifying CI passes. Use when the user asks to submit, create, or open a pull request.
-disable-model-invocation: true
+disable-model-invocation: false
 ---
 <!-- agent-pmo:74cf183 -->
 

crates/lsp-profiler-dotnet/src/cpu.rs

@@ -214,7 +214,8 @@ fn sampler_error(error: EngineError, pid: u32) -> SamplerError {
     }
 }
 
-fn attach_kind(kind: ErrorKind) -> Option<AttachErrorKind> {
+/// The attach failure an engine error kind classifies, if any.
+pub(crate) fn attach_kind(kind: ErrorKind) -> Option<AttachErrorKind> {
     match kind {
         ErrorKind::ProcessNotFound => Some(AttachErrorKind::ProcessNotFound),
         ErrorKind::UnsupportedRuntime => Some(AttachErrorKind::UnsupportedRuntime),

crates/lsp-profiler-dotnet/src/memory.rs

@@ -18,6 +18,7 @@ use lsp_profiler::wire::{
 use tokio::sync::Mutex;
 use tracing::{debug, warn};
 
+use crate::cpu::attach_kind;
 use crate::engine::{Engine, EngineError};
 use crate::protocol::{EngineMessage, ObjectRecord, RefEdgeRecord, RefNodeRecord, TypeRow};
 
@@ -348,7 +349,10 @@ fn object(item: ObjectRecord) -> ObjectSample {
 fn memory_error(error: EngineError) -> MemoryError {
     match error {
         EngineError::Missing(message) => MemoryError::ToolMissing(message),
-        EngineError::Failed { message, .. } => MemoryError::Backend(message),
+        EngineError::Failed { kind, message } => match attach_kind(kind) {
+            Some(kind) => MemoryError::Attach { kind, message },
+            None => MemoryError::Backend(message),
+        },
         other => {
             warn!(%other, "the .NET profiling engine failed");
             MemoryError::Backend(other.to_string())

crates/lsp-profiler-dotnet/tests/dotnet_e2e.rs

@@ -24,11 +24,13 @@ use std::time::Duration;
 use lsp_profiler::wire::methods;
 use lsp_profiler::ProfilerService;
 use serde_json::{json, Value};
-use support::{call, completed, engine, service, Fixture};
+use support::{assert_only_busy_threads_count, call, completed, engine, service, Fixture};
 
 const FIXTURE: &str = "HotLeakApp";
 const FIXTURE_TYPE: &str = "HotLeakApp.LeakedPayload";
 const HOT_FUNCTION: &str = "HotLeakApp.Hot.Spin";
+/// `Program.Main`, which only ever waits on stdin.
+const ENTRY_POINT: &str = "HotLeakApp.Program.Main(";
 
 #[tokio::test(flavor = "multi_thread")]
 async fn the_fixture_is_recognised_as_an_attachable_dotnet_process() {
@@ -104,6 +106,7 @@ async fn cpu_profiling_finds_the_hot_function_and_its_lines() {
             && (29..=32).contains(&l["line"].as_i64().unwrap_or(0))),
         "hot line inside Hot.Spin expected: {report}"
     );
+    assert_only_busy_threads_count(&report, ENTRY_POINT, HOT_FUNCTION);
     for key in ["speedscope_path", "flamegraph_path", "cpuprofile_path"] {
         let path = report["artifacts"][key]
             .as_str()
@@ -258,6 +261,58 @@ async fn a_live_process_no_runtime_recognises_is_unsupported_and_never_killed()
     service.shutdown().await;
 }
 
+/// `cpu.start` resolves the runtime before anything else ([PROF-CPU-START]
+/// step 1): naming a runtime that cannot profile the PID is
+/// `UNSUPPORTED_RUNTIME` even while the PID has a CPU session, and only a
+/// start that could attach is `ALREADY_PROFILING`.
+#[tokio::test(flavor = "multi_thread")]
+async fn a_named_runtime_is_resolved_before_the_pid_is_found_busy() {
+    let fixture = Fixture::start(FIXTURE);
+    let dir = tempfile::tempdir().expect("tempdir");
+    let service = service(dir.path().to_path_buf());
+    let started = call(&service, methods::CPU_START, json!({ "pid": fixture.pid })).await;
+    let named = json!({ "pid": fixture.pid, "runtime": "python" });
+    assert_eq!(
+        error_code(&service, methods::CPU_START, named).await,
+        -32002
+    );
+    let again = json!({ "pid": fixture.pid });
+    assert_eq!(
+        error_code(&service, methods::CPU_START, again).await,
+        -32004
+    );
+    let _ = call(
+        &service,
+        methods::CPU_STOP,
+        json!({ "session_id": started["session_id"] }),
+    )
+    .await;
+    service.shutdown().await;
+}
+
+/// A memory step on a program that has exited is `PROCESS_NOT_FOUND`, the
+/// engine's classified cause, never a bare `OTHER` ([PROF-ERRORS]).
+#[tokio::test(flavor = "multi_thread")]
+async fn a_memory_step_on_an_exited_program_is_process_not_found() {
+    let mut fixture = Fixture::start(FIXTURE);
+    let dir = tempfile::tempdir().expect("tempdir");
+    let service = service(dir.path().to_path_buf());
+    let started = call(
+        &service,
+        methods::MEMORY_START,
+        json!({ "pid": fixture.pid }),
+    )
+    .await;
+    let session = json!({ "memory_session_id": started["memory_session_id"] });
+    let _ = fixture.child.kill();
+    let _ = fixture.child.wait();
+    assert_eq!(
+        error_code(&service, methods::MEMORY_SNAPSHOT, session).await,
+        -32001
+    );
+    service.shutdown().await;
+}
+
 /// A target that exits mid-session keeps the samples taken while it lived: the
 /// recorded trace is still symbolized and reported on stop ([PROF-CPU-STOP]).
 #[tokio::test(flavor = "multi_thread")]

crates/lsp-profiler-dotnet/tests/dotnet_fsharp_e2e.rs

@@ -23,13 +23,15 @@ use std::time::Duration;
 
 use lsp_profiler::wire::methods;
 use serde_json::{json, Value};
-use support::{call, completed, service, Fixture};
+use support::{assert_only_busy_threads_count, call, completed, service, Fixture};
 
 const FIXTURE: &str = "HotLeakAppFs";
 const RECORD_TYPE: &str = "HotLeakAppFs.LeakedPayload";
 const UNION_CASE_TYPE: &str = "HotLeakAppFs.PayloadKind+Small";
 const GENERIC_RECORD_TYPE: &str = "HotLeakAppFs.Tagged`1[System.Int32]";
 const HOT_FUNCTION: &str = "HotLeakAppFs.Hot.spin";
+/// `Program.main`, which only ever waits on stdin.
+const ENTRY_POINT: &str = "HotLeakAppFs.Program.main(";
 /// `items.Add { Data = …; Label = …; Kind = kind }` in `LeakCache.grow`.
 const RECORD_CONSTRUCTED_AT: i64 = 23;
 /// `let kind = Small items.Count` in `LeakCache.grow`.
@@ -89,6 +91,7 @@ async fn cpu_profiling_finds_the_fsharp_hot_function_and_its_lines() {
             .any(|l| in_program_fs(l) && (26..=31).contains(&l["line"].as_i64().unwrap_or(0))),
         "hot line inside Hot.spin expected: {report}"
     );
+    assert_only_busy_threads_count(&report, ENTRY_POINT, HOT_FUNCTION);
     service.shutdown().await;
 }
 

crates/lsp-profiler-dotnet/tests/support/mod.rs

@@ -108,6 +108,29 @@ pub async fn call(service: &ProfilerService, method: &str, params: Value) -> Val
         .unwrap_or_else(|error| panic!("{method} failed: {error}"))
 }
 
+/// [PROF-AGGREGATION-LOGIC] and [PROF-DOTNET-CPU-SYMBOLS]: only a thread
+/// executing managed code counts. A fixture's main thread waits on stdin for
+/// the whole session, so its entry point is never hot, and the worker's hot
+/// function holds most of the share. Counting the idle thread caps that share
+/// at half, since the runtime samples both threads every tick.
+pub fn assert_only_busy_threads_count(report: &Value, entry_point: &str, hot_function: &str) {
+    let functions = report["hot_functions"].as_array().expect("hot_functions");
+    let named = |prefix: &str| {
+        functions
+            .iter()
+            .find(|f| f["name"].as_str().is_some_and(|n| n.starts_with(prefix)))
+    };
+    assert!(
+        named(entry_point).is_none(),
+        "{entry_point} only runs on the idle main thread: {report}"
+    );
+    let share = named(hot_function).map_or(0.0, |f| f["percentage"].as_f64().unwrap_or(0.0));
+    assert!(
+        share > 50.0,
+        "{hot_function} owns the busy thread, {share}%: {report}"
+    );
+}
+
 /// The result of a memory step the direct backend completed.
 pub fn completed(step: &Value) -> &Value {
     step.get("result")