Skip to content

test(real-cpu): portable peak RSS and a pinned, isolated chain process for #298 - #322

Draft
SamGu-NRX wants to merge 3 commits into
obv/burst14-bs-real-poses-scalefrom
gpt-burst/1022z-bs-298-rss
Draft

SamGu-NRX wants to merge 3 commits into
obv/burst14-bs-real-poses-scalefrom
gpt-burst/1022z-bs-298-rss

Conversation

@SamGu-NRX

@SamGu-NRX SamGu-NRX commented Oct 22, 2026 •

Copy link
Copy Markdown
Owner

Zephyrus task Z1. Fixes #298's peak-RSS reading on macOS and its open review threads on the CPU budget, memory isolation and scale assertions.

What was wrong

  • _vm_hwm_mb() (backend/tests/test_real_cpu_upstream.py:72 on [obvious-fleet] Backend: the first four real stages on CPU on a real app-format capture #298) read /proc/self/status with no fallback, so on macOS every test in the file failed with FileNotFoundError.
  • The module set OMP_NUM_THREADS, OPENBLAS_NUM_THREADS, MKL_NUM_THREADS and HSS_POSES_THREADS with setdefault at import, which changed them for every later test and kept any higher value already set. Nothing pinned the process to two CPUs unless the caller used taskset, so make test never measured the 2-CPU budget.
  • The peak RSS was the whole pytest process's peak, so an earlier test could fail the 8 GB budget.
  • The scale assertions accepted any non-none source, a verified status and a zero bound, and passed if the LiDAR attempt disappeared.

What changed

  • _peak_rss_mb() (backend/tests/test_real_cpu_upstream.py:79) reads resource.getrusage(RUSAGE_SELF).ru_maxrss, dividing by 1024² on macOS (bytes) and by 1024 elsewhere (KiB).
  • Each chain runs in its own process through the new backend/tests/realcpu_launcher.py, which pins to the first two allowed CPUs, gets the four thread variables set to 2 in its environment only, and forks once before importing the module under test. The fork is there because a subprocess child on Linux reports its parent's lifetime peak as its own ru_maxrss (CPython starts it with vfork, and execve folds the parent's high-water mark into the child's counter). On WSL2, kernel 6.18, a child whose VmHWM was 11 MB reported 626 MB after its parent had touched 600 MB. A fresh fork() from a bare interpreter starts a clean counter.
  • The child reports the CPUs and thread settings it actually ran with. _assert_budget (:267) checks the thread settings and the RSS budget, then asserts the 600 s wall budget only when the chain ran on one or two CPUs. On macOS, which has no sched_setaffinity, it skips with a reason after the correctness checks pass, instead of claiming the budget.
  • The LiDAR chain now asserts source arkit, status unverified, scale_unresolved / meter_closeup_missing, confidence near_only, a positive rel_bound_95, ARKit poses as the only used estimate, and the unused LiDAR estimate with lidar_too_few_pairs and 0 pairs (:331-341). The no-depth chain also asserts source arkit, a positive bound, no LiDAR estimate, and ARKit poses as the only used estimate (:366-374).

Tests

New tests in backend/tests/test_real_cpu_upstream.py:

  • test_peak_rss_mb_reads_bytes_on_darwin sets sys.platform to darwin and resource.getrusage to return 2×1024×1024, and expects 2.0 MB. test_peak_rss_mb_reads_kib_on_linux expects 2048.0 MB from the same number.
  • test_peak_rss_mb_matches_vmhwm_in_the_chain_process (Linux only) raises pytest's own peak by 256 MiB, then runs a probe through the launcher that touches 64 MiB and compares _peak_rss_mb() with VmHWM, within 1 MB.
  • test_importing_this_module_leaves_thread_env_alone imports the module in a clean interpreter and checks that none of the four thread variables got set.

Failing first. Commit a43cc0e adds the tests against #298's code, with _vm_hwm_mb renamed to _peak_rss_mb and its body unchanged:

cd backend && OMP_NUM_THREADS=1 uv run pytest -q tests/test_real_cpu_upstream.py -k "peak_rss or thread_env"
3 failed, 2 deselected
  darwin: assert 226.85 == 2.0   (the /proc reading, getrusage ignored)
  linux:  assert 228.46 == 2048.0
  env:    ['HSS_POSES_THREADS', 'MKL_NUM_THREADS', 'OMP_NUM_THREADS', 'OPENBLAS_NUM_THREADS'] set after import

The VmHWM test can't fail on #298 because #298 has no launcher. As a check of what it guards, I removed the launcher's fork locally: it failed with 479.9 == 263.8 ± 1, the pytest peak leaking into the child. I restored the fork before committing.

The stricter scale assertions pass on #298 too, because the stages already produce the right claim. They don't fail first; they catch a regression that would claim a scale the fixture can't support.

After the fix:

cd backend && OMP_NUM_THREADS=1 uv run pytest -q tests/test_real_cpu_upstream.py
6 passed in 38s   (both chains ran on CPUs [0, 1] with the thread variables at 2; LiDAR poses 18.7 s, peak 1825 MB)

Full suite, run once on this head (uv sync --locked && OMP_NUM_THREADS=1 uv run pytest -q; pytest-xdist isn't in the lock, so no -n 2):

5 failed, 1507 passed, 16 skipped, 21 errors in 2559s (42:39)
all 6 tests in tests/test_real_cpu_upstream.py passed

The machine was shared and busy, so I ran the suite once, on this head only, and didn't rerun #298. Every failure and error is outside the changed file. Compared with #298's reported run (make test: 5 failed, 1507 passed, 19 skipped, 14 errors):

  • Same as [obvious-fleet] Backend: the first four real stages on CPU on a real app-format capture #298: the four server/eval/tests/test_gates.py failures, and the 14 tests/test_capture_cold_restart.py errors (ImportError: cannot import name 'load_receipt').
  • server/packet/tests/test_round2.py::test_many_missing_files_stay_linear failed on time (63.8 s against a 60 s limit). It passes alone.
  • tests/test_capture_real_downstream.py had 7 setup errors, all a finalize drain TimeoutError (server/api/deps.py:112). [obvious-fleet] Backend: the first four real stages on CPU on a real app-format capture #298's import-time setdefault used to cap the BLAS and pose threads at 2 for every later test, and this PR removes that cap. So I ran the module alone without the cap, at a load average of 16 to 22 on 16 cores: 12 passed in 150 s. The timeouts came from contention in the full run, not from the removed cap.
  • The skip count differs because this run used uv sync --locked without --extra api, as the task rules ask.

uv run ruff check . passes. uv run ruff format --check . flags one file, server/orchestrator/tests/test_greptile_followup.py, which is unchanged from #298 and outside this diff.

Not verified

  • I couldn't run on macOS. The darwin branch is covered only by the monkeypatched test, and the launcher's os.fork() and the wall-budget skip have not been run there.
  • The wall budget is still measured on whichever two CPUs the host allows first. It doesn't model a slower worker.

🤖 Generated with Claude Code

Review fix (Oct 23)

The launcher now polls waitpid(..., WNOHANG) against a 900-second deadline, then sends SIGTERM, waits one second, sends SIGKILL if needed, reaps the child, and raises a timeout naming the stage. Normal child exit codes still pass through unchanged. The two subprocess.run calls now use timeouts of 900 seconds for real chains and 30 seconds for the import probe. These values are chosen well above the expected runtime; they were not measured.

The new launcher test runs a child that sleeps past a 50 ms limit, checks that the call returns within two seconds, and verifies that the child is gone. It also checks normal exit-status preservation.

Against 9237a5b8, the old launcher reproduced the hang under timeout 3s with exit 124. After the fix, OMP_NUM_THREADS=1 uv run pytest -q tests/test_real_cpu_launcher.py passes 2 tests, and OMP_NUM_THREADS=1 uv run pytest -q tests/test_real_cpu_upstream.py passes all 6 tests.

SamGu-NRX and others added 2 commits October 22, 2026 12:04
…d env alone

Fails on the parent: _peak_rss_mb (the old _vm_hwm_mb, renamed only) reads
/proc/self/status and ignores ru_maxrss, and importing the module sets the
four thread variables for every later test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…axrss

_peak_rss_mb reads resource.getrusage(RUSAGE_SELF).ru_maxrss, KiB on Linux
and bytes on macOS, instead of /proc/self/status, which macOS lacks.

Each chain now runs through tests/realcpu_launcher.py: pinned to two CPUs,
the four thread variables set to 2 in that process only, and forked from a
bare interpreter so its peak RSS is its own. A subprocess child inherits the
parent's lifetime peak in ru_maxrss on Linux (vfork + execve); measured on
WSL2 6.18 as 626 MB in a child whose VmHWM was 11 MB. The module no longer
sets thread variables at import. The wall budget is asserted only when the
chain actually ran on two CPUs or fewer, and skipped loudly on macOS.

The scale assertions now pin the shipped claim: source arkit, status
unverified, the refusal reason, ARKit poses as the only used estimate, a
positive bound, and on the LiDAR fixture the unused LiDAR estimate with
lidar_too_few_pairs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 22, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the size:L 100-499 effective changed lines. label Oct 22, 2026
@SamGu-NRX

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-23T11:08:01.158604Z
We couldn’t complete this review. Comment @codex review to try again.
9237a5b Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@SamGu-NRX

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Something went wrong. Try again later by commenting “@codex review”.

An unknown error occurred
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Co-Authored-By: Claude Code <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 effective changed lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant