compass(tests): pin the backend and runner module walks to every module, not to one (#232)

[jgong5]

Part of #232

What this does

test_the_package_was_found in test_backend_interface.py and test_runner_non_allocating.py asks whether each package walk found anything. Suppose the walk is narrowed to rglob("__init__.py"), sliced to [:1], or made non-recursive. It still finds something, so the whole tests/compass suite stays green while cases silently disappear. This PR pins each walk to finding everything. In both files:

• test_the_walk_returns_every_module_under_the_root runs the walk over a tree the test builds (__init__.py, a.py, sub/b.py) and asserts that exact set.
• test_every_module_the_walk_returns_is_a_case compares the parametrisation's values (pytestmark[0].args[1]) to the walk. A slice where the walk is handed to parametrize leaves the walk whole, so the first test cannot see it.
• Runner only: the engine-import exemption is keyed by path == PACKAGE / "model_runner.py", not by path.name. Case ids are now paths under the package. For today's flat package these are the same ids, and the gate's node-id delta below confirms it.

Scope: tests only, 0 production lines, 54 added / 2 removed across two files. The estimate in the phase-1 inventory was ~56; the halt line was 112.

Phase-1 inventory and proposal, posted before this PR: #232 (comment)

The pin, and why

The expected set, over a fixture tree, not over the real package.

• A count over the real package names nothing when it fails, and moves every time a module is added. That is the churn test(compass): state the non-empty case for two derived parametrisations in test_cpu_gate_exclude.py #210's review priced.
• An expected set over the real package names what is missing, but it churns the same way.
• The fixture's set never moves when the package grows. It also holds the two shapes the flat packages lack:
  • a module one directory down, which a non-recursive walk misses;
  • a module that is not __init__.py, which rglob("__init__.py") or a [:1] slice misses.
• It reuses the mechanism the landed moved-root control already uses (monkeypatch on PACKAGE), so there is no new one.

Convention, stated once. A walk over a package is held by four checks:

1. non-empty on the real root (landed);
2. empty on a moved root, with a sibling one level out (landed);
3. the exact module set of a fixture tree with a nested module and a non-__init__ module (new);
4. the parametrised values equal to the walk (new).

#266 proposes no convention for these walks: it records B7, B14 and B28 as unreachable and leaves them. So this does not compete with #266.

Named result, re-measured

Node 18, xiaobizh_n18_cpu. The tip cae9607f0 and the head 7f45c3be8 were each staged by git archive + docker exec -i into my own /tmp/i232gates/<side>/ATOM, and the md5 matched on both ends. atom.__file__ was asserted under each staged root. Each mutation is line-count-preserving and the harness asserts a unique anchor. The whole tests/compass suite ran per mutation under timeout -k 10 1800, and failures were read from junit by node id.

Null control: tip 1312 passed, head 1316 passed (+4, the four new tests), 6 skipped on both sides, rc=0.

All node ids below are tests/compass/….

1. Recursion is covered. Each plant is sub/leaky.py, containing import atom.model_engine.model_runner.

id	mutation	tip cae9607f0	head 7f45c3be8
b_plant	plant in backends/, walk untouched	1 failed: test_backend_interface.py::test_the_package_imports_nothing_from_the_engine[sub/leaky.py]	the same
b_plant_glob	the same plant, rglob → glob in _backend_modules	1312 passed, rc=0	1 failed: test_backend_interface.py::test_the_walk_returns_every_module_under_the_root
r_plant	plant in runner/, walk untouched	2 failed: test_runner_non_allocating.py::test_only_the_binding_module_reaches_the_engine[leaky.py], test_runner_rpc_surface.py::test_the_package_docstring_lists_every_module_beside_it	the same two, with the id [sub/leaky.py]
r_plant_glob	the same plant, rglob → glob in _runner_modules	1 failed, only test_runner_rpc_surface.py::test_the_package_docstring_lists_every_module_beside_it; the engine guard is blind	2 failed: test_runner_non_allocating.py::test_the_walk_returns_every_module_under_the_root, plus the same docstring test
r_plant_glob_doc	as r_plant_glob, with sub also listed as a bullet in runner/__init__.py's docstring	1312 passed, rc=0	1 failed: test_runner_non_allocating.py::test_the_walk_returns_every_module_under_the_root

At the tip, the runner's red under glob comes from the docstring test, which fails on any new subdirectory whatever it imports. r_plant_glob_doc shows it: once the subpackage is documented, as anyone adding one legitimately would do, the engine breach is fully green. At the head it is red by the walk's own name.

2. Narrowing is refused.

id	mutation	tip	head
b_init	rglob("*.py") → rglob("__init__.py")	1307 passed, rc=0, 5 cases gone	1 failed: test_backend_interface.py::test_the_walk_returns_every_module_under_the_root
b_fn1	sorted(...)[:1] inside _backend_modules	1307 passed, rc=0, 5 gone	1 failed: same
b_site1	_backend_modules()[:1], at the parametrize	1307 passed, rc=0, 5 gone	1 failed: test_backend_interface.py::test_every_module_the_walk_returns_is_a_case
b_glob	rglob → glob, no plant	1312 passed	1 failed: test_backend_interface.py::test_the_walk_returns_every_module_under_the_root
r_init	rglob("__init__.py")	1309 passed, rc=0, 3 gone	1 failed: test_runner_non_allocating.py::test_the_walk_returns_every_module_under_the_root
r_fn1	[:1] inside _runner_modules	1309 passed, rc=0, 3 gone	1 failed: same
r_site1	_runner_modules()[:1], at the parametrize	1309 passed, rc=0, 3 gone	1 failed: test_runner_non_allocating.py::test_every_module_the_walk_returns_is_a_case
r_glob	rglob → glob, no plant	1312 passed	1 failed: test_runner_non_allocating.py::test_the_walk_returns_every_module_under_the_root

3. The basename exemption (runner). The plant is runner/sub/model_runner.py, importing the engine, with the walk untouched.

id	tip	head
r_plant_mr	1 failed, only the docstring test; the engine guard exempts the file by name	2 failed: test_runner_non_allocating.py::test_only_the_binding_module_reaches_the_engine[sub/model_runner.py], plus the docstring test
r_plant_mr_doc (sub also listed in the docstring)	1313 passed, rc=0	1 failed: test_runner_non_allocating.py::test_only_the_binding_module_reaches_the_engine[sub/model_runner.py]

Every head failure is in the guard named for that row. None comes from a line-drift guard.

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

Both runs were on node 18 in xiaobizh_n18_cpu, one at a time:

• each tree was gated with its own scripts/compass/gate_cpu.sh;
• the .compass-commit and .compass-changed stamps were written from the same rev-parse as the archive;
• each run was bounded by timeout -k 10 2400, unpiped, and written to a log file;
• node ids came from --junitxml.

tree	gate header	atom.__file__	passed / skipped / xfailed	failed	GATE_CPU_RC
control, tip cae9607f0	commit: cae9607f0 (stamp), 29 files excluded + tests/plugin, gpu: not required	/tmp/i232gates/tip/ATOM/atom/__init__.py	5268 / 155 / 3, 186 s	0	0
head 7f45c3be8	commit: 7f45c3be8 (stamp), same exclusions, gpu: not required	/tmp/i232gates/head/ATOM/atom/__init__.py	5272 / 155 / 3, 193 s	0	0

The node-id delta runs from 5426 to 5430 ids.

• Added (4): tests/compass/test_backend_interface.py::test_the_walk_returns_every_module_under_the_root, …test_backend_interface.py::test_every_module_the_walk_returns_is_a_case, …test_runner_non_allocating.py::test_the_walk_returns_every_module_under_the_root and …test_runner_non_allocating.py::test_every_module_the_walk_returns_is_a_case.
• Removed: none. The runner's ids moved from p.name to the path under the package, and no id changed.

No test_stream_marker_properties.py timing class failed on either side, so no re-run was needed.

The tip moved during the gate, to f89b1492c (#376, #380, #383, #386, #388). Those commits touch atom/config.py, atom/compass/kv/handoff.py, three other tests and two documents, none of them in this diff. git merge-tree --write-tree f89b1492c 7f45c3be8 gives 6ed48c6e69e236d79052594ab45602e618f8ab7a with rc=0. I did not gate that merged tree; the tree check at landing covers it.

Lint over the two files: black 26.5.1 --target-version py312 --check, BLACK_RC=0; ruff 0.16.7 check, RUFF_RC=0.

Left for #223, and why this says Part of

• The three landed sites, test_spec_schema.py:412, test_ir_data_model.py:721 and test_clock_lp_identity.py:440, carry the same non-empty-only guard. They are out of scope pending compass(tests): the tests that inspect code structure fail open on other spellings #223's ruling and the held compass(tests): AST guards refuse the spellings they cannot read #266, and they are untouched here. Each can take the same two tests, plus a moved-root control where one is missing.
• The same shape also exists in test_kv_driverless_import.py:130. It is outside this task's file set, so it is noted and not edited.
• test_runner_step_semantics.py::_reply_attribute_reads walks atom/model_engine with glob (compass(tests): the tests that inspect code structure fail open on other spellings #223's B14). Its exact nine-attribute set already refuses narrowing, and the file is in compass(tests): AST guards refuse the spellings they cannot read #266's diff, so it is not touched.
• compass(tests): rpc_surface says the package docstring was added with the comment; #90 only amended it #384 (a docstring sentence in test_runner_rpc_surface.py) is not folded in, because this diff does not edit that file.

What surprised me

1. The runner's "recursion is covered" looked true at the tip, for the wrong reason. A nested plant under glob reddens test_the_package_docstring_lists_every_module_beside_it, because that test counts a subdirectory as a module. Documenting the subpackage turns it fully green. So the neighbour holds "every subpackage is documented", and not "no module under the runner reaches the engine".
2. The recursive walk made the basename exemption reachable. With rglob, path.name == "model_runner.py" exempts a model_runner.py at any depth. So the landed recursion widened what the exemption lets through, and nothing noticed.

No design-doc references in code, test names, comments or emitted data.

🤖 Generated with Claude Code

[jgong5]

Non-blocking. This docstring sentence is false as written. The pin itself holds. Principle 8: "Every claim carries its measurement."

The sentence says a walk "narrowed to a pattern or sliced to a prefix misses a.py or more". I measured two narrowings on the merged tree (tip f89b1492c + head 7f45c3be8), with -k test_the_walk_returns_every_module_under_the_root. Neither of them misses a.py:

mutation of _backend_modules	what the walk returns over the fixture	result
sorted(PACKAGE.rglob("*.py"))[:2]	{'__init__.py', 'a.py'}: only sub/b.py is missing	1 failed
sorted(PACKAGE.rglob("[!_]*.py"))	{'a.py', 'sub/b.py'}: only __init__.py is missing	1 failed

The whole tests/compass suite gives the same verdict on the second one:

• tip: 1318 passed, rc 0, one case gone;
• merged: 1 failed, test_backend_interface.py::test_the_walk_returns_every_module_under_the_root.

So the set equality catches both, and only the prose is wrong.

Suggested wording: "…and one narrowed to a pattern or sliced to a prefix misses at least one of the three." The identical sentence is in test_runner_non_allocating.py:475.

[jgong5]

Non-blocking. This is the same false sentence as test_backend_interface.py:124. Principle 8.

A [:2] slice drops only sub/b.py, and a [!_]*.py pattern drops only __init__.py. Neither misses a.py. The set equality refuses both, so only the prose needs the fix, for example "misses at least one of the three". The measurements are on the backend comment.

[jgong5]

This review is agent-authored. It is review cycle 1 of PR #395 (part of #232), at head 7f45c3be87866e57aff1648db8814d20278449ea.

Verdict: APPROVE 7f45c3be8. No finding blocks.

There are two non-blocking findings. Both are prose, and neither is in the code or the pins:

• N1 (inline, on test_backend_interface.py:124 and test_runner_non_allocating.py:475): a docstring sentence is false as written.
• N2 (below): a sentence in the commit message and the PR body is false as written.

If they are not fixed on this branch, they need an issue before landing, per the rule "A finding not fixed in the PR that found it gets an issue".

I read the eight design principles and AI_DEV_RULES.md at f89b1492c first. The brief is #232's body, its comments and the coordinator scoping, and the phase-1 inventory is comment 5806036107.

What I checked

Every changed line: 54 added and 2 removed, in two test files, with no production line.

• The design-reference grep over both whole files finds nothing.
• ruff 0.16.7 check gives RUFF_RC=0, and black 26.5.1 --target-version py312 --check gives BLACK_RC=0. Both were run on the merged tree.
• No other file references the changed test names or ids. I grepped the whole merged tree, scripts/compass included.

Is the built-tree pin run against the real walk, or against a copy? It runs the real walk. test_the_walk_returns_every_module_under_the_root calls _backend_modules() or _runner_modules() itself, with PACKAGE monkeypatched. This is the same function the parametrize site calls, and the same mechanism the landed moved-root control uses. Mutating the real walk's body reddens the pin by name: see P1 and P3 below.

Plants: whole tests/compass, one at a time

Method:

• Node 18, xiaobizh_n18_cpu.
• Each tree was staged by git archive and docker exec -i, with the md5 matched on both ends.
• The merged tree is git merge-tree --write-tree f89b1492c 7f45c3be8 = 6ed48c6e6, stamped as commit 28f83b271 with git commit-tree. The tip is f89b1492c.
• Each mutation is line-count-preserving, applied to a fresh copy with an anchor that must be unique, and atom.__file__ is asserted under that copy.
• Failures were read from junit by node id.

Null: tip 1319 passed, merged 1323 passed (+4, the four new tests), 6 skipped, rc 0.

id	mutation	tip f89b1492c	merged
P1	plant backends/sub/leaky.py (import atom.model_engine.model_runner), and rglob→glob in _backend_modules	1319 passed, rc 0: the breach is invisible	1 failed: test_backend_interface.py::test_the_walk_returns_every_module_under_the_root, {'__init__.py', 'a.py'} == {…, 'sub/b.py'}
P2	_runner_modules()[:1], at the parametrize	1316 passed, rc 0: 3 cases gone	1 failed: test_runner_non_allocating.py::test_every_module_the_walk_returns_is_a_case
P3 (mine)	rglob("[!_]*.py") in _backend_modules, which skips __init__	1318 passed, rc 0: 1 case gone	1 failed: test_backend_interface.py::test_the_walk_returns_every_module_under_the_root, {'a.py', 'sub/b.py'} == …
P4 (mine, re-inline the consumer)	sorted(PACKAGE.glob("*.py")), inlined at the runner's parametrize, plus the plant runner/sub/leaky.py, plus a sub bullet in runner/__init__.py's docstring	1319 passed, rc 0: the engine guard is blind	1 failed: test_runner_non_allocating.py::test_every_module_the_walk_returns_is_a_case
P4b	the P4 inline, with no plant	—	1323 passed, rc 0 (see reservation R2)
P5	plant runner/sub/model_runner.py importing the engine, plus the sub bullet	1320 passed, rc 0: exempt by basename	1 failed: test_only_the_binding_module_reaches_the_engine[sub/model_runner.py], {'atom.model_…model_runner'} == set()
P5r (revert the fix)	P5, with the base's key path.name == "model_runner.py" put back on the merged tree	—	1324 passed, rc 0: the breach is invisible again
P6 (mine)	walk bounded at depth one, [*PACKAGE.glob("*.py"), *PACKAGE.glob("*/*.py")], plus the plant backends/sub/deeper/leaky.py	—	1323 passed, rc 0 (see reservation R1)

The results:

• Every red is in the guard named for its row.
• None is a line-drift guard, since every mutation preserves line count.
• No pin is inert. P5r is the witness that the path key is the load-bearing line of the exemption change.

Ruling on the exemption change: in scope, and correct

In scope. #232 item 2 names the recursive walk as an unmeasured claim that fails open (principle 6: "Refuse rather than fall back."). A basename exemption under a recursive walk is the same failure one directory in: a model_runner.py at any depth was exempt, so P5 is green at the tip.

The developer disclosed this in the phase-1 inventory before editing. It is inside the brief's file set and inside the 56-line estimate. It touches tests only and changes no production behaviour. It only makes the guard refuse more.

Correct. The evidence:

• P5 and P5r show that it bites and which line bites.
• The ids stay unchanged for today's flat package. The gate junit carries [__init__.py], [model_runner.py], [overrides.py] and [step_output.py], as before.
• path == PACKAGE / "model_runner.py" compares two paths built from the same resolved PACKAGE, so the comparison is sound.

Every comment and docstring sentence, measured

• test_the_walk_returns_every_module_under_the_root, "Non-empty says the walk found something; this says it found everything": this holds over the fixture (P1, P3).
• The same docstring, "A walk that is not recursive misses sub/b.py": this holds (P1).
• The same docstring, "one narrowed to a pattern or sliced to a prefix misses a.py or more": this is false (N1, inline). [:2] misses only sub/b.py, and [!_]*.py misses only __init__.py. The pin catches both.
• test_every_module_the_walk_returns_is_a_case, "A slice or filter where the walk is handed to the parametrisation drops cases while the walk itself still returns every module": this holds (P2, P4).
• The unchanged file docstring and comment, "rglob, so a module added under the package is covered the day it lands": this now holds to one level of nesting. Deeper nesting is R1.

N2 (non-blocking, no line). The commit message says "A walk narrowed to rglob("__init__.py"), sliced to [:1], or made non-recursive still finds something, so each of those passed the suite with cases silently gone". The PR body's first paragraph says the same. The "non-recursive" part is false on today's flat packages, where no case disappears:

• the developer's own b_glob and r_glob rows at the tip give 1312 passed, equal to the null;
• P1 at the tip gives 1319 passed, equal to the null, and tests=1325, also equal to the null.

What a non-recursive walk hides is a breach one directory down. Principle 8. The squash message is written at landing (-F commit_message=@file), so the lander can correct it there without a new commit on the branch.

Accepted with reservation, and what the next task in this area should watch

• R1. The fixture holds exactly one level of nesting. P6, a walk bounded at depth one with a breach at sub/deeper/, is green. No realistic spelling produces that walk, and the named defect, rglob→glob, is held, so I ask for no change (principle 3: "Add only what is necessary, and nothing more").
• R2. test_every_module_the_walk_returns_is_a_case compares against today's real tree. A divergent consumer is therefore green while the package is flat (P4b), and red by name as soon as a nested module exists (P4). That is the reach it needs: any site divergence that drops a real case is refused.
• (mark,) = …pytestmark fails closed with a ValueError if a second mark is ever added.
• The same two tests are the convention offered for compass(tests): the tests that inspect code structure fail open on other spellings #223's three held sites and test_kv_driverless_import.py. R1 applies there too.

ponytail-review

• The only duplication is the same two tests in two files. A shared helper would be an abstraction with two call sites (yagni), so I did not tag it.
• The fixture's path tuple and its expected-set literal repeat three strings, but merging them saves 0 lines.

Lean already. Ship.

Gate 1: merged tree, gated once

• Tree: git merge-tree --write-tree f89b1492c 7f45c3be8 = 6ed48c6e69e236d79052594ab45602e618f8ab7a (rc 0). It is not the head's own tree, so it was gated as a combined tree.
• Stamp: git commit-tree gave 28f83b271.
• Script: the tree's own scripts/compass/gate_cpu.sh, with the .compass-commit and .compass-changed stamps.
• Run: atom.__file__ = /tmp/pr395r1/merged/ATOM/atom/__init__.py, under timeout -k 10 2400, unpiped, with no other gate_cpu.sh running.

tree	header	passed / skipped / xfailed	failed	GATE_CPU_RC
merged 6ed48c6e6	commit: 28f83b271 (stamp), 29 files excluded + tests/plugin, gpu: not required	5279 / 155 / 3, 194 s	0	0 PASSED

That is the expected tip count of 5275 plus the head's 4. No timing-flake class failed, so no re-run was needed.

The tip moved again during this review, to 37fba4df0, six commits later (#391, #392, #393, #396, #397, #399). They touch atom/compass/runner/overrides.py, which is walked by the runner guard, and tests/compass/test_runner_rpc_surface.py.

• git merge-tree --write-tree 37fba4df0 7f45c3be8 = d2d1fd4a891db5ff2167d39f834da78c23ec63eb, clean (rc 0).
• On that tree, test_backend_interface.py, test_runner_non_allocating.py and test_runner_rpc_surface.py give 114 passed, rc 0.
• I did not full-gate d2d1fd4a8. The landing tree check is the step that covers it.