ci(release-benchmark): isolate client-stack memory from the browser subtree

[suchintan]

https://github.com/Skyvern-AI/rustwright-cloud/pull/189

[claude]

Summary

This is a tools-only sync from rustwright-cloud (release-benchmark diagnostics) — no changes to the Rust core, Python API, or Node bindings. The new client-vs-browser-subtree RSS split in measure_process_tree.py is well-built (bracketed reads with a pid/starttime re-check to avoid TOCTOU/PID-reuse issues, graceful unresolved_records accounting). However, there's a real pipeline bug that will silently break the new reporting.

🔴 Critical Issues (1)

• peak_client_rss_kb / peak_excluded_process_count are dropped by validate_binding_results.py, so the new columns/savings-delta will always render as missing. measure_process_tree.py now writes peak_client_rss_kb and peak_excluded_process_count into the raw measurement JSON (tools/measure_process_tree.py:270-293), but measurement_payload() in tools/validate_binding_results.py:80-120 only reads/validates wall_seconds, peak_tree_rss_kb, exit_code, samples, unresolved_samples, and unresolved_records_total — it never reads peak_client_rss_kb or peak_excluded_process_count, and its returned dict (used to build the --metrics-out file that aggregate_release_benchmark.py actually consumes) omits both keys entirely. Since aggregate_release_benchmark.py reads only from the metrics-out file (matched via lang+impl keys, tools/aggregate_release_benchmark.py:84-136), every real CI run will produce metrics JSON without these two fields. Consequences:
  • The new "Client-stack peak RSS MB" and "Browser processes" table columns will show — for every real run (tools/aggregate_release_benchmark.py:183-208).
  • Worse, this is a regression, not just an incomplete feature: the "Savings" row's memory-delta calculation was changed from peak_tree_rss_kb to peak_client_rss_kb (tools/aggregate_release_benchmark.py:225-226), which previously worked and will now always show — too.
  • Fix: add peak_client_rss_kb and peak_excluded_process_count to measurement_payload()'s validation and return dict (mirroring how unresolved_samples/unresolved_records_total were added).

🟡 Suggestions (2)

• /proc/pid/comm truncation could misclassify sandboxed browser subprocesses. In classify_process_from_proc (tools/measure_process_tree.py), when /proc/pid/exe can't be resolved (plausible for sandboxed Chrome renderer/GPU/zygote processes under namespacing), the fallback compares comm against excluded_names (basenames of the excluded executables). /proc/pid/comm is kernel-truncated to 15 bytes, so an excluded executable with a basename longer than 15 chars (e.g. chrome-headless-shell) would never match via the comm candidate, only via cmdline (which itself requires successful argv[0] extraction). Worth a comment or explicit truncated-prefix comparison so this doesn't silently misclassify a chrome subtree as "client" in the one scenario (sandboxed subprocesses) this fallback exists for.
• peak_client_rss_kb/peak_excluded_process_count don't get the same hard-failure treatment as peak_tree_rss_kb. These two new fields go through the generic normalize-loop (tools/aggregate_release_benchmark.py:121-136), which only checks type/non-negativity and silently pops on failure — reasonable, but it means a payload with a garbage value degrades straight to — rather than being distinguished from "field genuinely absent." Minor, since it does still append to problems either way.

📝 Minor / Style (2)

• Redundant exception types: classify_process_from_proc and process_tree_sample_from_proc catch (FileNotFoundError, PermissionError, OSError, ...) — the first two are already OSError subclasses, so listing them adds nothing (matches the pre-existing style in the file, so not a blocker).
• The updated caveats text ("Binding client-stack memory excludes... the workflow-resolved browser executable...") is a good, honest clarification of what's being measured — no overclaiming here, and it correctly notes older artifacts without the new fields aren't implied to have unresolved reads.