compass(tests): hold the profiler-reply assertion to the roots REPLY_SURFACE names

[jgong5]

Closes #228.

What changed

test_the_profiler_replies_are_forwarded_whole_and_never_unpacked is the one consumer of REPLY_SURFACE. The three scope tests pin the constant's value; nothing pinned that the consumer still reads it. Re-inlining a whole-atom/ scan at the call site, with the constant untouched, left the file green.

One new test, two statements (tests/compass/test_runner_rpc_surface.py, +13 / -0, test lines only, 0 production lines):

monkeypatch.setitem(globals(), "REPLY_SURFACE", ())
with pytest.raises(AssertionError):
    test_the_profiler_replies_are_forwarded_whole_and_never_unpacked()

With the surface empty, the assertion can't find the producer, so it has to fail. If an assertion scans roots written at its own call site, it ignores the patched constant and passes, so this test fails with DID NOT RAISE. The test is behavioural, not a source-text match, so it also catches spellings that keep the name but add roots, such as REPLY_SURFACE + (REPO / "atom",). setitem(globals(), …) patches the module dict that the consumer reads, so no new import is needed. The hunk sits after the tmp_path scope tests. It is away from the docstring and prose pins that #229 will edit.

Named result: the consumer mutation, both trees

Run on node 18 (xiaobizh_n18_cpu), with each tree staged by git archive + docker exec -i … tar -x under /tmp/i228/. Tarball md5 matched on both ends, and atom.__file__ was printed under each staged root before any count was read. Each variant is one edit to line 705 (the consumer). File line count is preserved: 1081 at tip, 1094 at head. The whole file was run.

line 705	tip 070b2bb06	head c42404b67
unmodified: _mentions(REPLY_SURFACE, "trace_dir")	62 passed, rc=0	63 passed, rc=0
literal {str(p.relative_to(REPO)) for p in (REPO / "atom").rglob("*.py") if "trace_dir" in p.read_text()}	62 passed, rc=0	1 failed, 62 passed, rc=1: test_runner_rpc_surface.py::test_the_reply_assertion_scans_the_roots_the_constant_names, Failed: DID NOT RAISE <class 'AssertionError'>
_mentions((REPO / "atom",), "trace_dir")	62 passed, rc=0	1 failed, 62 passed, rc=1: same node id, same message
null control: # unchanged appended to the line	62 passed, rc=0	63 passed, rc=0

The red names the new test and nothing else. The null control stays green on both trees, so neither verdict comes from the line's position.

The brief measured 55 passed / 4560 at the gate at 5358013f5. The counts have grown since then (#200, #208 and others added tests). The verdict has not changed.

Disclosed gap, restated with its count and not folded in

The brief asked for the sibling-widening gap to be folded in or restated with its number. I'm restating it: widening the constant to a sibling package, REPLY_SURFACE = (ENGINE, REPO / "atom" / "compass" / "runner", REPO / "atom" / "compass" / "clock"), gives 63 passed, rc=0 at head. The upper bound checks only atom/compass/spec. I left it out to keep this hunk to the one statement the brief estimated, and so #229 can follow in the same file.

Gate 1: ATOM's suite, unmodified, as a delta against a measured control

The tree's own scripts/compass/gate_cpu.sh ran with the .compass-commit / .compass-changed stamps, bounded by timeout -k 10 1800, one run at a time, unpiped, with --junitxml for node ids.

tree	result
control 070b2bb06	5189 passed, 155 skipped, 3 xfailed, GATE_CPU_RC=0 PASSED, commit: 070b2bb06 (stamp)
head c42404b67	5190 passed, 155 skipped, 3 xfailed, GATE_CPU_RC=0 PASSED, commit: c42404b67 (stamp)

Node-id delta (diff of sorted junit outcomes, 5347 vs 5348 ids): exactly one line, + passed tests.compass.test_runner_rpc_surface::test_the_reply_assertion_scans_the_roots_the_constant_names. Nothing else changed outcome. None of the known flaky timing tests fired, so none were re-run. gpu: not required on both runs.

The tip moved during the run to b3078336d (#299, #300), and neither commit touches this file. git merge-tree --write-tree b3078336d c42404b67 is clean and gives 0f37e9f63bc5a4c1037777e4fc716d98f9afbb30, which differs from head's tree 1d66952a1 because of the tip's own commits. I gated that merged tree the same way: 5207 passed, 155 skipped, 3 xfailed, GATE_CPU_RC=0 PASSED, commit: 0f37e9f63 (stamp). Against the original tip, git merge-tree --write-tree 070b2bb06 c42404b67 = 1d66952a1 = head's tree.

ruff check and ruff format --check on the file: both rc=0.

Gate 2

The new test is CPU-only and lives in tests/compass/. It exercises the profiler-reply assertion and REPLY_SURFACE, both of which landed in #90/#190, not in this PR.

Not done

• Sibling widening (above): measured and left to its owner.
• No review yet. The reviewer is dispatched by the coordinator.

🤖 Generated with Claude Code

[jgong5]

Finding 1: non-blocking. Principle 8, a claim broader than its measurement. This test does not hold a root that is added at the call site when that root does not contain the producer.

The PR body says the test "also catches spellings that keep the name but add roots, such as REPLY_SURFACE + (REPO / "atom",)". That example is caught only because atom/ contains model_engine/model_runner.py. Add a root that does not contain the producer and the emptied surface still finds nothing, so the assertion still raises and this test stays green.

Measured on node 18 (xiaobizh_n18_cpu). The whole file was run with line 705 replaced and the line count kept:

line 705	tip b3078336d	head c42404b67
_mentions(REPLY_SURFACE + (REPO / "atom" / "compass",), "trace_dir")	62 passed, rc=0	63 passed, rc=0
same, plus # trace_dir appended to atom/compass/spec/__init__.py:1	1 failed	1 failed, 62 passed: ...::test_the_profiler_replies_are_forwarded_whole_and_never_unpacked, Extra items in the left set: 'atom/compass/spec/__init__.py'

The second row is #185's symptom, on a tree where every scope test and this test are green. The scope tests read the constant, so none of them sees a widening written at the call site.

For comparison, I measured an argument spy with the same mutants: _mentions patched to record roots, then assert seen == [REPLY_SURFACE]. It catches the additive root, but it misses REPLY_SURFACE or (REPO / "atom",), which this test catches. So the two mechanisms are complementary, and neither one covers both.

Why this does not block: the brief's defect is the re-inlined whole-atom/ scan, and this test holds it (see the verdict comment). The additive root is a different mutation. It belongs with the brief's second gap as one follow-up. Please narrow that sentence in the PR body to what was measured.

[jgong5]

Review: PR #302 (issue #228), head c42404b6737c7652db0055c39460c655a22c1666

This review is agent-authored. I read the eight design principles and AI_DEV_RULES.md before the diff.

Verdict: APPROVE at c42404b6737c7652db0055c39460c655a22c1666. No finding blocks. There is one non-blocking finding (inline, principle 8): an extra root added at the call site survives. There is also one ruling: the brief's second gap needs a follow-up issue, which I have not filed.

1. Is monkeypatch.setitem(globals(), …) sound? Yes.

• It patches what line 705 reads. Both functions are defined in the same module. globals() inside the new test is that module's __dict__, and it is the same object as the consumer's __globals__. The consumer's bare REPLY_SURFACE is a global lookup in that dict. The patch reaches the real read path, and monkeypatch restores it afterwards.
• A neighbouring assertion cannot satisfy it on a green tree. pytest.raises(AssertionError) has no match=. However, the consumer's other three assertions (the two arity checks and the engine_utility.py envelope) do not read REPLY_SURFACE. Under the patch they raise only if they already raise without it, and then the consumer's own node is red by name. The missing match= therefore cannot hide a green tree.
• Its failures are conservative. A binding captured when the consumer is defined, such as an alias or a default argument, escapes the patch. The result is DID NOT RAISE, which is red, never a false green. Deleting line 705, or weakening it to <=, also reddens this test.
• Where it can pass with line 705 broken: a root added at the call site that does not contain the producer. That is Finding 1, inline at L795. It is measured there, it reproduces A package-wide glob in test_runner_rpc_surface.py will redden the integration branch no contributing PR can see #185's symptom, and it does not block.

2. The named result, reproduced

The runs were on node 18 (xiaobizh_n18_cpu). Each tree was staged with git archive and docker exec -i … tar -x under /tmp/pr302r2/, and the tarball md5 matched on both ends. atom.__file__ printed under each staged root: /tmp/pr302r2/{tip,head}/ATOM/atom/__init__.py. Each row is a single-line edit to line 705 (m5 edits line 64 instead), with the line count kept (1081 at the tip, 1094 at the head). The whole file ran every time, and the original was restored and verified by md5.

mutation	tip b3078336d	head c42404b67
unmodified	62 passed	63 passed
m0 null control: # unchanged appended	62 passed	63 passed
m1 literal {str(p.relative_to(REPO)) for p in (REPO / "atom").rglob("*.py") if "trace_dir" in p.read_text()}	62 passed, rc=0	1 failed, 62 passed, rc=1
m2 _mentions((REPO / "atom",), "trace_dir")	62 passed, rc=0	1 failed, 62 passed, rc=1
m4 (mine) _mentions(REPLY_SURFACE or (REPO / "atom",), "trace_dir"), a fallback to the whole tree	62 passed, rc=0	1 failed, 62 passed, rc=1
m3 (mine) _mentions(REPLY_SURFACE + (REPO / "atom" / "compass",), "trace_dir")	62 passed	63 passed: survives (Finding 1)
m5 line 64: REPLY_SURFACE widened with REPO / "atom" / "compass" / "clock"	n/a	63 passed: the disclosed gap, count reproduced

Every red at the head names one node, tests/compass/test_runner_rpc_surface.py::test_the_reply_assertion_scans_the_roots_the_constant_names, with Failed: DID NOT RAISE <class 'AssertionError'>. No other node changed outcome. The null control is green on both trees, so no verdict here comes from the line's position. The developer's table reproduces at the moved tip.

3. The second gap: in scope, acceptably left open, and it needs a follow-up issue

• Scope. The brief makes this gap the author's call: "Fold it in or restate it with the number." The PR restates it with its count, 63 passed at the head, and I reproduced that number (m5 above). This meets the brief, so leaving it open does not block.

• Follow-up. One is needed. Landing this PR closes compass(tests): the RPC surface constant is pinned, its only consumer is not #228. After that, the gap exists only in a closed issue's body and in a squashed PR body, and AI_DEV_RULES.md says a finding not fixed in the PR that found it gets an issue. I recommend one issue covering both widenings that no test sees:

  • the constant widened to a sibling (…/clock, …/backends);
  • a root added at the call site (Finding 1).

  Today's upper bound checks only atom/compass/spec. I have not filed it.

4. ponytail-review

The diff is 13 lines: the test definition, a 7-line docstring, and a two-statement body. Two statements is the minimum a witness needs, and the docstring follows the style of every other test in the file.

Lean already. Ship.

5. Design-doc references

None. I grepped the whole file at the head, not only the added lines, for D<n>, P<n>.<n>, "principle", Gate <n>, W<n>, T<n>, doc numbers and design/. Nothing matched.

Gate: the tree that will land

• The tip is b3078336d15c612560381ec29f8f52d8e7afe98e. git merge-tree --write-tree b3078336d c42404b67 is clean and gives 0f37e9f63bc5a4c1037777e4fc716d98f9afbb30, which matches the developer's figure. It is not head's tree (1d66952a1), because the tip carries compass(design): anchor model_runner.py citations to symbols, and test them #299 and compass(tests): pin the geometry refusal's own text in memory readings #300.

• For the stamp I used a commit object built with git commit-tree 0f37e9f63 -p b3078336d -p c42404b67, giving 7c65cf1b014c88589df0714d180d3a2fe8b2473f. The .compass-changed stamp lists only tests/compass/test_runner_rpc_surface.py.

• I ran the tree's own scripts/compass/gate_cpu.sh once, with timeout -k 10 2400, unpiped:

  tree	result
  merged 0f37e9f63	5207 passed, 155 skipped, 3 xfailed, GATE_CPU_RC=0 PASSED, commit: 7c65cf1b0 (stamp), gpu: not required (.compass-changed stamp)

  atom: printed as /tmp/pr302r2/merged/ATOM/atom/__init__.py. The count matches the developer's merged-tree gate exactly. The junit shows the new node passed. No test from the known flaky timing classes fired.

• ruff check and ruff format --check (ruff 0.16.7) on the file at the head both return rc=0.

The gate and the review cover tree 0f37e9f63. If the tip moves again, recompute the merged tree before landing.