compass(tests): check the reply-scan reader at the constant it runs at - #337
Conversation
#335) The reader test checked the scan at two surfaces, () and (ENGINE,). A widening that adds only the Compass root's parent is the identity at both, yet at the real constant it scans all of atom/compass again. Add REPLY_SURFACE as a third surface, so that widening fails [surface2]. Rewrite the reader docstring so each clause names the runs its mutation fails at three surfaces, and qualify the E4 pin's docstring: it holds a root read when the scan is handed none, and a root gated on roots or on a non-empty needle passes the file. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Review cycle 1: APPROVE at
|
| row | tip feb1b27a2 |
head 720cd47a1 |
developer's claim |
|---|---|---|---|
| null | 68 passed | 69 passed | same |
| m3 | 2 F: R0, R1 | 3 F: R0, R1, R2 | same |
| m4 | 1 F: R0 | 1 F: R0 | same |
| m5 | 1 F: S | 1 F: S | same |
| E1 | 1 F: R1 | 2 F: R1, R2 | same |
| E2 | 1 F: R1 | 2 F: R1, R2 | same |
| E2b | 1 F: R1 | 2 F: R1, R2 | same |
| E4 | 1 F: I | 1 F: I | same |
| E4 + pin off | 68 passed | 69 passed | same |
| E6 | 68 passed | 1 F: R2 | same |
| E6b | 68 passed | 1 F: R2 | same |
| W | 2 F: R0, R1 | 2 F: R0, R1 | same |
| BYP | 2 F: R0, R1 | 3 F: R0, R1, R2 | same |
| E4or | 1 F: I | 1 F: I | same |
| E4g / E4gb / E4nb | 68 passed each | 69 passed each | same |
- The named result holds. E6 and E6b fail by name at the head, on R2, with
assert [(…engine, …compass/runner, …compass)] == [(…engine, …compass/runner)], and pass at the tip. m3, m4, m5, E1, E2, E2b and E4 are red at the head on every node they were red on at the tip. The null control is green on both sides. - Revert witness (AI_DEV_RULES: "A reviewer credits a test with holding a defect only after reinstating it … line count preserved, nothing else changed"). At the head I put back only the tip's parametrize line,
[(), (ENGINE,)]. REV alone gives 68 passed. REV+E6, REV+E6b and REV+E6r each give 68 passed. So the third surface is exactly what turns E6 and E6b red.
2. Every rewritten docstring clause measures true (principle 8)
Principle 8: "Every claim carries its measurement." Each row below was measured at the head.
| clause | row | head result | true? |
|---|---|---|---|
| L800 "A root added beside the constant fails all three runs" | m3 | R0, R1, R2 | yes |
| L801 "the constant's two roots written out at the call site fail the first two" | W | R0, R1 only | yes |
| L802 "a fallback for an empty constant fails the empty one" | m4 | R0 only | yes |
| L802–803 "a root that appears only when the constant is set … fails the last two" | E1b (REPLY_SURFACE + ((REPO / "atom",) if REPLY_SURFACE else ()), the clause's literal form; E1 is a replacement, not an addition) |
R1, R2 | yes |
| L803–804 "one derived from every root -- each root's parent, say -- fails the last two" | E2, E2b | R1, R2 | yes |
L804–805 "one derived from the Compass root alone -- its parent, all of atom/compass -- fails the last" |
E6, E6b, and E6r (below) | R2 only | yes |
L806 "A scan that bypasses _mentions reaches the spy not at all" |
BYP | R0 fails inside the reply assertion (set() == {'atom/model_engine/model_runner.py'}); R1 and R2 fail on seen == […] |
yes |
| L806–808 "What passes is any expression that is the identity at these three surfaces" | E7 (below) | 69 passed | yes, and E7 sits exactly on this boundary |
| L783–785 "Nor may the scan read a root of its own when handed none" | E4, E4or; E4 + pin off | I; I; 69 passed (so the _mentions((), "") pin is what catches it) |
yes |
| L785–786 "A root added only when roots are handed in, or only for a non-empty needle, passes every test in this file" | E4g, E4gb, E4nb | 69 passed each | yes |
Non-blocking note, PR body only. The developer's clause-to-row table has no row for the "appears only when the constant is set" clause. E1 is the and form, which replaces the constant rather than adding to it. I measured the literal form, E1b, and it agrees. Nothing in the tree needs to change.
3. Extra widenings
| row | edit | tip | head | verdict |
|---|---|---|---|---|
| E6r | call site: tuple(r.parent if r.name == "runner" else r for r in REPLY_SURFACE), which replaces the Compass root with all of atom/compass |
68 passed | 1 F: R2 | This one-edit undo of #326 slipped past the tip. The head catches it. |
| E7 | call site: REPLY_SURFACE + tuple(r.parent for r in REPLY_SURFACE[2:]) |
68 passed | 69 passed | Non-blocking. It is the identity at every surface the constant can take today, so the real scan is unchanged. It widens only a future third root, and adding that root already means editing S (set(REPLY_SURFACE) == {ENGINE, PACKAGE}). The docstring's "any expression that is the identity at these three surfaces" states this boundary truthfully. |
| E8 | inside _mentions: (*roots, *(r.parent for r in roots if r.name == "runner")) |
1 F: I | 1 F: I | caught |
| E9 | inside _mentions: (*roots, *(r.parent / "spec" for r in roots if r.name == "runner")) |
1 F: I | 1 F: I | caught |
The open class is still the gated fixed root (E4g, E4gb and E4nb). The docstring now names it, and #326's reviewer advised against pinning it. Next task in this area should watch E7-shaped widenings if the constant ever grows a third root.
4. ponytail-review
The diff is one parametrize entry plus two docstring rewrites. The per-surface clauses are the only record of why each of the three surfaces exists. Cutting them would invite a successor to drop a surface, and dropping one is exactly the E6 hole this PR closes.
Lean already. Ship.
5. Gate: the merged tree, once
- Tip read again:
feb1b27a2089b84977f579e3554afeb9c307d457(compass(spec): a saved document merged again does not claim the transfer was asked #333 landed afterb63f1a711). The developer's3b28ccadb= head-tree identity is therefore stale. It does reproduce againstb63f1a711. - Merged tree:
git merge-tree --write-tree feb1b27a2 720cd47a1=f6ae5f5fe680da116a29de0df9270eef33ebaf65. It is not the head tree, so it was gated. - Stamp:
git commit-tree f6ae5f5fe -p feb1b27a2 -p 720cd47a1=4bdef577b451202724facee7216c236565149529..compass-changedlists onlytests/compass/test_runner_rpc_surface.py. - Staging:
git archiveinto a tarball, piped into the container at/tmp/pr337r1/. The md5287f7c49…matched on both ends, and the shared mount was not touched. - Run: the tree's own
scripts/compass/gate_cpu.sh, once, unpiped, undertimeout -k 10 2400.
| tree | commit: |
atom: |
passed | skipped | xfailed | junit cases | GATE_CPU_RC |
|---|---|---|---|---|---|---|---|
merged 4bdef577b (tree f6ae5f5fe) |
4bdef577b (stamp) |
/tmp/pr337r1/gate/ATOM/atom/__init__.py |
5240 | 155 | 3 | 5398 (0 failures, 0 errors) | 0 PASSED |
- Control. Tip tree
170da7165is the tree compass(spec): a saved document merged again does not claim the transfer was asked #333's reviewer gated at 5239 / 155 / 3, with 5397 cases. The +1 is[surface2], which is present in my junit, as is compass(spec): a saved document merged again does not claim the transfer was asked #333'stest_a_saved_document_merged_again_does_not_claim_to_have_asked_the_transfer. - Timing classes: none fired (0 failures, and the skip count equals the control's).
- Lint:
ruff checkandblack --checkon the file at the head are clean.
6. Coordination with #329
#329 (head d2b3cf8f3) changes one line of this file, :221, the Runner stub's SimpleNamespace(disagg_is_decode=False, speculative_config=None). git merge-tree --write-tree d2b3cf8f3 720cd47a1 is clean (rc 0, tree 110a198ca). There is no conflict in either landing order.
Scope and rules
- Brief: N1, N2 and N3 are delivered as specified. The file set is the one file.
- Effort: +15/−11 test lines against an estimate of 5–10, under 2x. There are 0 production lines.
- Refs: no design-doc references in the file at the head.
Closes #335
This PR was written by an agent. I read the eight design principles in
atom/compass/design/README.mdandatom/compass/AI_DEV_RULES.mdat the tipb63f1a711before starting.What changes
The PR changes one file,
tests/compass/test_runner_rpc_surface.py. It adds 15 test lines and removes 11 (net +4), with 0 production lines. It carries the three non-blocking findings from #326's cycle-2 review.test_the_reply_assertion_scans_the_roots_the_constant_namesnow runs at three surfaces,[(), (ENGINE,), REPLY_SURFACE]. The third is the value the profiler-reply assertion actually runs at.Named result: mutation battery on node 18
Setup.
agent_scratch/compass_dev/pr326-review2/mutate.py, unchanged apart from its header line.720cd47a1: md585521f28…, 1213 lines,atom.__file__=/tmp/i335gates/head/ATOM/atom/__init__.py.b63f1a711: md598e18b22…, 1209 lines,atom.__file__=/tmp/i335gates/tip/ATOM/atom/__init__.py.split("\n")counts, so each is one more thanwc -l.Node-id key. The prefix is
tests/compass/test_runner_rpc_surface.py::.test_the_reply_assertion_scans_the_roots_the_constant_names[surface0]/[surface1]/[surface2]test_the_scan_looks_at_both_sides_of_the_replytest_the_scan_ignores_a_compass_package_the_reply_never_reachesb63f1a711720cd47a1# nullon the consumer lineREPLY_SURFACE + (REPO / "atom" / "compass",)REPLY_SURFACE or (REPO / "atom",)atom/compass/clockREPLY_SURFACE and (REPO / "atom",)REPLY_SURFACE + tuple(r.parent for r in REPLY_SURFACE)tuple(r.parent for r in REPLY_SURFACE)_mentions:(*roots, REPO / … / "clock")_mentions((), "")replaced bypassREPLY_SURFACE + tuple(r.parent for r in REPLY_SURFACE if "compass" in r.parts)REPLY_SURFACE + tuple(r.parent for r in REPLY_SURFACE[1:])_mentions((), "")pin is still what catches E4.The rows behind the new docstring sentences were run in the same battery, at the same two commits:
(ENGINE, REPO / "atom" / "compass" / "runner")seen == [])_mentions:roots or (clock,)_mentions:(*roots, REPO/…/clock) if roots else rootsbase_mentions:(*roots, base/…/clock) if needle else rootsOther attacks, for the next task in this area:
(ENGINE, PACKAGE) if REPLY_SURFACE else ()tuple(dict.fromkeys(REPLY_SURFACE + (ENGINE,)))tuple(dict.fromkeys(REPLY_SURFACE + (PACKAGE,)))set(REPLY_SURFACE) | {REPO/"atom"/"compass"}set(REPLY_SURFACE)set() != ())Gate 1: the suite, as a delta against a control
b63f1a711469007dfba903bfba90c50236bf1dd2. It was read again before staging, and compass(tests): refuse any reply-scan root set but REPLY_SURFACE, and read relative engine imports #326 is included.git merge-tree --write-tree b63f1a711 720cd47a1=3b28ccadb6ab92896e8641841acdaf48ba237559. That equals720cd47a1^{tree}, because the branch is on the tip.git commit-tree 3b28ccadb -p b63f1a711 -p 720cd47a1=b44a73a39..compass-changedlists onlytests/compass/test_runner_rpc_surface.py.git archiveinto a tarball, piped todocker exec -i … catat/tmp/i335gates/. The md58a91fddd…matched on both ends, and the shared mount was not touched.scripts/compass/gate_cpu.sh, once, one at a time, unpiped, undertimeout -k 10 2400, with--junitxml.atom:GATE_CPU_RCb63f1a711/tmp/i335gates/tip/ATOM/atom/__init__.pyb44a73a39/tmp/i335gates/gate/ATOM/atom/__init__.pytests/compass/test_runner_rpc_surface.py::test_the_reply_assertion_scans_the_roots_the_constant_names[surface2].TestTheRegionIsNotCopiedPerChunk(4 nodes),TestNoSizeAtWhichACallStopsBeingOne::…[minimax]andtest_freezing_twice_is_additive_and_harmlessall passed.ruff checkandblack --checkon the file are clean at the head.Dev record
🤖 Generated with Claude Code