compass(spec): hold the probe tables, the reimport helper and the transfer stanzas against their defects - #357
Conversation
…ir defects The two probe-table pins accepted an always-empty PROBE_TABLES. One expected `()`, which is what a total failure produces, and the other asked only that a path be absent. Both now compare against the retained-bytes table, so an empty derivation is red on each of them. `reimported_validate` now asserts what its docstring says: the canonical `atom.compass.spec.validate` is still the instance the suite imported, not the one the helper loaded. The saved-transfer and stack-mismatch refusal arms now hold which fragment they name, with a `startswith` on the fragment's quoted source. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
| rest = fragments()[:2] + [fragment("links", LINKS)] | ||
| (refused,) = validate(merge([saved] + rest)).refusals | ||
| assert refused.rule is Rule.PINNED_STACK | ||
| assert refused.what.startswith("'t.yaml' (machine ") |
There was a problem hiding this comment.
Non-blocking, R1-1: the needle stops just before the part of the stanza that differs between fragments.
Principle 6: "A declined answer with a named reason is a result. A guessed one is a defect." The rule: "A check counts only once someone has seen it fire."
't.yaml' (machine holds the source name. (machine …) is the same for every fragment, because _one_machine enforces that. After it, the stanza renders the method, author and date, and in these fixtures the method is the field that tells fragments apart.
Mutant CA2L. The saved arm keeps fragment.source, but the rest of its stanza comes from merged.fragments[-1] ('links'). It is one line, the line count is kept, and the file is atom/compass/spec/validate.py. Here is the text it produces, probed on node 18:
't.yaml' (machine 'mi355x-8gpu-2node', probed, by a person on 2026-09-18) carried constants over from 'mi300x-8gpu', and its provenance names ...
It says a probed fragment carried constants over. That contradicts itself: the real stanza reads transferred-from:mi300x-8gpu.
Result: tests/compass, head 064ac89bd, 1307 passed, 0 failed. The null control is also 1307 passed. At the tip it is 1307 passed as well.
A fix of the same shape as your first_hand control a few lines below:
assert refused.what.startswith(
"'t.yaml' (machine 'mi355x-8gpu-2node', transferred-from:mi300x-8gpu, "
"by a person on 2026-09-18) carried constants over from "
)The needle you have does what the brief asked, and it reddens the #346 reviewer's CA2 (0 F at the tip, 1 F at the head). So this finding is non-blocking. If it is not taken here, it needs a follow-up issue. The same gap is in the mismatch arm (R1-2).
| checked = validate(combination) | ||
| assert not checked.ok | ||
| assert checked.refusals[0].rule is Rule.PINNED_STACK | ||
| assert checked.refusals[0].what.startswith("'tier2' (machine ") |
There was a problem hiding this comment.
Non-blocking, R1-2: same as R1-1, in the stack-mismatch arm.
Principle 6, and the rule "A check counts only once someone has seen it fire."
Mutant CA2mL. The mismatch arm keeps fragment.source, and renders the rest of the stanza from merged.fragments[-1]. The text it produces, probed on node 18:
'tier2' (machine 'mi355x-8gpu-2node', probed, by a person on 2026-09-18) carried constants over from 'mi300x-8gpu', measured against rocm '7.0.2', into a spec pinned to rocm '7.2.4'
Result: head 064ac89bd, 1307 passed, 0 failed. The endswith below pins everything after the stanza, and this startswith pins everything up to (machine . That leaves exactly the method, author and date free.
The fix: extend the needle through the method, "'tier2' (machine 'mi355x-8gpu-2node', transferred-from:mi300x-8gpu, by a person on 2026-09-18) carried ". With that, startswith plus endswith together cover the whole string.
This is non-blocking for the same reason as R1-1: the #346 reviewer's CA2m is red at the head (1 F) and green at the tip.
| ) | ||
| module = importlib.util.module_from_spec(loaded) | ||
| loaded.loader.exec_module(module) | ||
| assert importlib.import_module("atom.compass.spec.validate") is not module |
There was a problem hiding this comment.
Non-blocking, R1-3: the new assertion holds half of the docstring's sentence.
Principle 8: "Every claim carries its measurement."
The docstring (L2002) says the helper "does not register it". The assertion holds a narrower claim: that the helper does not register the module under the canonical name.
Mutant REGown. It changes exec_module(module) to exec_module(module); __import__('sys').modules[loaded.name] = module, which registers the module under its own name. The line count is kept.
Result: head 064ac89bd, 1307 passed, 0 failed. For comparison, your REG mutant, which registers under the canonical name, gives 2 F by both callers' ids, and it is green at the tip. I reproduced that too.
The consequence clause is held. "the instance the rest of the suite is holding is the one it started with" is exactly what import_module(...) is not module checks.
It cannot pass for the wrong reason in this file. The canonical module is imported at collection, by L57–68. I probed that "atom.compass.spec.validate" in sys.modules is True at the call, so import_module returns the suite's instance and never imports a fresh one.
Two one-line fixes: narrow the sentence to "does not register it under the package's name", or add assert loaded.name not in sys.modules. The first costs no import.
|
Review cycle 1, PR #357 (issue #231): APPROVE at head This review is agent-authored. No blocking issues. There are three non-blocking findings, posted inline: R1-1 (L601), R1-2 (L568) and R1-3 (L2011/L2002). Every pin this PR claims fires. I reinstated each defect at the head and at the tip, and they redden by their own ids. That satisfies the rule "A reviewer credits a test with holding a defect only after reinstating it … An inert pin on a required finding blocks APPROVE." The three findings are narrower mutants that the required findings did not name. 1. Mutation table, reproduced on node 18Setup:
The PE red is the same set at both trees, apart from pins B and C. Six older ids catch PE at both trees:
These match the PR body's table in every cell. 2. Inertness checks(a) Pin B's
(b) The helper's
(c) The
3. Truth checks (principle 8)
PR-body claims:
4. ponytail-reviewL1992–1993 Lean already. Ship. 5. Gate 1: the merged tree at the current tipThe tip moved during the review, from
How it ran:
What the junit shows: 5421 cases, and nothing outside passed or skipped. The 17 timing-class cases ( The control count: 5263 equals the developer's measured tip control at Non-blocking findings (inline)
If these are not taken in this PR, they need one follow-up issue. Next action: land at |
This PR is agent-authored.
Closes #231. It also closes CA2/CA2m, which #346's cycle-2 review found and #231's last comment added to this issue.
Tests only. 0 production lines.
tests/compass/test_spec_verbs.pyis +12 / −3. No refusal text changes.What changed
872edea12)test_a_width_table_no_probe_is_named_for_is_refused_and_not_an_import_erroradded.path not in under_test.PROBE_TABLES, which an empty tuple satisfiesunder_test.PROBE_TABLES == (THE_HOLE,). The added table is not a probe table, and the one table that has a hole still is.test_a_probe_given_the_width_that_has_none_empties_the_probe_tablesunder_test.PROBE_TABLES == (), which is the value a total failure produces(PROBE_TABLES, under_test.PROBE_TABLES) == ((THE_HOLE,), ()). The table is emptied by the probe, and it was not empty before the probe.reimported_validate: "does not register it…"assert importlib.import_module("atom.compass.spec.validate") is not module, in the helper, so both callers hold itrefused.what.startswith("'t.yaml' (machine ")checked.refusals[0].what.startswith("'tier2' (machine ")THE_HOLEis"device.runtime_constants.allocator_retained_after_load_bytes": the one width table thatFILLED_BYleaves a probe hole in, at width 1.Re-measured: all four items still reproduced at the tip
I measured on node 18 (
xiaobizh_n18_cpu), over the whole oftests/compass, with 10 jobs.git archivetree, andatom.__file__was under that job's own copy in all 10.pr346-review2/tools/mutate.py.872edea12064ac89bdtests/compass/test_spec_verbs.py::)PROBE_TABLESforced emptyvalidate.py:PROBE_TABLES = tuple(→PROBE_TABLES = () and tuple(…refused_and_not_an_import_error:assert () == ('device.runt..._load_bytes',)pin C,
…empties_the_probe_tables:assert ((), ()) == (('device.run..._bytes',), ())exec_module(module)→exec_module(module); __import__('sys').modules['atom.compass.spec.validate'] = module…refused_and_not_an_import_errorand…empties_the_probe_tables, both on the helper's newis not modulelisted[0]('tier2')test_a_saved_transfer_merged_again_names_the_merge_that_dropped_its_pin, at the newstartswith'tier1'test_a_transfer_keeps_the_source_stack_out_of_this_machines_pin, at the newstartswithThe PE red is the same at both trees, apart from pin B and pin C. Six other ids catch PE at both trees:
test_each_condition_in_the_check_set_is_earned_by_a_spec[whether a probe fills …]test_a_missing_entry_no_probe_fills_is_two_refusals_and_not_onetest_the_check_does_not_agree_a_probe_exists_for_a_width_below_onetest_the_probe_question_is_asked_of_the_tables_it_says_it_readstest_the_probe_question_reads_only_the_tables_a_probe_can_fall_short_ontest_the_probe_question_says_it_could_not_be_asked_when_its_table_is_goneThe brief counted four of these at
80a508dff; the tree has grown since. As the brief said, the tree was covered, and the hole was in these two pins.Gate 1: ATOM's CPU tier, unmodified, against a measured control
git merge-tree --write-tree 872edea12 064ac89bdgivesf1ce8b3d47a69906b1c3ffcf1a51944dbe98821d, rc 0. That equals the head's tree, because the head's parent is the tip.git commit-treegives0c2a2efa8, with parents the tip and the head..compass-changedlists the one test file.scripts/compassis treebc0d1dc99on both sides.git archive, then shipped overdocker exec -iinto/tmp/i231and extracted withtar -x. The tar md5 matched on both ends;gate_cpu.sh, undertimeout -k 10 2400, unpiped;commit:atom:GATE_CPU_RC0c2a2efa8 (stamp)/tmp/i231/stage/merged/ATOM/atom/__init__.py872edea12 (stamp)/tmp/i231/stage/tip/ATOM/atom/__init__.pyTestTheRegionIsNotCopiedPerChunk,…[minimax],test_freezing_twice…): 17 cases on each run, all passed, so none needed a re-run.checkandformat --checkpass on the file, as does black 26.5.1--check. The file has no design-doc references and no#NNN.Dev record
== (THE_HOLE,)impliesadded.path not in …, because the added path is notTHE_HOLE.exec_module, so both callers go red by their own ids when it breaks. The sentence's consequence is that "the instance the rest of the suite is holding is the one it started with". The assertion checks exactly that, through the canonicalimport_module.🤖 Generated with Claude Code