compass(spec): hold the transfer stanzas through their method, and the reimport helper's whole docstring - #365
Conversation
…e helper's whole docstring The two startswith needles in the stack-pin arms stopped at "(machine ", which is the same for every fragment, so a refusal that kept the source name but rendered the method, author and date of another fragment in the merge still passed. Both needles now run through the whole stanza, the way the first-hand control already does; with the mismatch arm's endswith they cover the whole refusal. reimported_validate says it does not register what it loads, but only the package's name was checked. It now also asserts its own name is not in sys.modules, so the whole sentence is held. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
| assert checked.refusals[0].what.startswith( | ||
| "'tier2' (machine 'mi355x-8gpu-2node', transferred-from:mi300x-8gpu, " | ||
| "by a person on 2026-09-18) carried " | ||
| ) | ||
| assert checked.refusals[0].what.endswith( | ||
| "carried constants over from 'mi300x-8gpu', measured against rocm " | ||
| "'7.0.2', into a spec pinned to rocm '7.2.4'" |
There was a problem hiding this comment.
N1 (non-blocking): the startswith and the endswith do not cover the whole refusal; the middle is free.
Principle 8: "Every claim carries its measurement. A number without a source is a defect." The PR body and the commit message both say that, with this endswith, the two checks "overlap and cover the whole refusal". They share the word carried , but nothing makes them share the same occurrence. Any text between the needle's carried and the tail's carried constants over from passes both.
Measured on node 18 (reviewer's harness plus one mutant, GAPm): validate.py:398 becomes f"{fragment.stanza()} carried {merged.fragments[-1].stanza()} carried constants over from ". The line count is kept at 452. The refusal then reads 'tier2' (machine …, transferred-from:mi300x-8gpu, by a person on 2026-09-18) carried 'links' (machine …, probed, by a person on 2026-09-18) carried constants over from 'mi300x-8gpu', measured against ….
- Tip
a9c7aed9d: 1309 passed, 6 skipped. - Head
c26a3a507: 1307 passed, 6 skipped: green. Both checks pass.
So this test does not hold the whole text; it holds the two ends.
Fix (also the ponytail shrink:, 8 lines → 5). The refusal is fully determined by this test's own fixtures, so a single equality holds all of it, the way the first_hand control at L613 already does:
assert checked.refusals[0].what == (
"'tier2' (machine 'mi355x-8gpu-2node', transferred-from:mi300x-8gpu, "
"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'"
)Measured on node 18. I applied this and N2's needle to a copy of the head:
- ruff
checkandformat --checkboth give rc 0; - the file goes from 2100 lines to 2096;
- both tests pass;
GAPmgoes red, as doCA2mLandCA2L.
Non-blocking. The brief's named mutant CA2mL is red at the head, and the gap is a new mutant outside #361.
The false sentence is in the PR body and in c26a3a507's message. Under the no-amend rule, the message stays. So if this is fixed here, the fix commit's message should say the earlier sentence was wrong, and the PR body should be corrected. If it is not fixed here, file the gap as an issue.
| 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 " | ||
| ) | ||
| assert "over from 'mi300x-8gpu', and its provenance names the" in refused.what | ||
| assert "fragments an earlier merge built it from" in refused.what |
There was a problem hiding this comment.
N2 (non-blocking): the same gap here, between the needle's over from and the in check's over from.
Principle 8, "Every claim carries its measurement", as in N1. This needle now ends carried constants over from , and the next line looks for over from 'mi300x-8gpu', and its provenance names the anywhere in the string. Nothing ties those two over froms to the same occurrence.
Measured on node 18 (mutant GAPs): validate.py:368 becomes f"{fragment.stanza()} carried constants over from {merged.fragments[-1].stanza()} over from ". The line count is kept at 452. The refusal reads 't.yaml' (…) carried constants over from 'links' (machine …, probed, …) over from 'mi300x-8gpu', and its provenance names the ….
- Tip
a9c7aed9d: 1309 passed, 6 skipped. - Head
c26a3a507: 1307 passed, 6 skipped: green.
This test does not claim to cover the whole refusal, so no sentence is false here, and the named mutant CA2L is red. It is recorded so the next reader does not count this test as holding the middle.
The cheapest close is one contiguous needle through to the end of the first clause. Both in lines (L609–610) are then subsumed, so the change is net −1 line:
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 'mi300x-8gpu', "
"and its provenance names the fragments an earlier merge built it from"
)The text is the refusal's own, as printed at the head. Measured with N1's change on a copy of the head:
- ruff is clean;
- both tests pass;
GAPsgoes red (1 failed,…merged_again_names_the_merge_that_dropped_its_pin), andCA2Lstays red.
Or leave it as it is and file the gap.
| ) | ||
| module = importlib.util.module_from_spec(loaded) | ||
| loaded.loader.exec_module(module) | ||
| assert loaded.name not in sys.modules |
There was a problem hiding this comment.
R1-3 is held, and it holds for the right reason. One optional shrink: follows.
Principle 8, "Every claim carries its measurement." "Does not register it" is the docstring's claim, and this line is its measurement.
What loaded.name is. It is f"atom.compass.spec.{name}": atom.compass.spec.validate_with_a_table_no_probe_is_named_for and atom.compass.spec.validate_with_every_width_filled. The name is synthetic, and nothing in ATOM registers it. That does not make the check vacuous. It is the spec.name the importlib recipe (module_from_spec, then sys.modules[spec.name] = module) would register under, so it is the key a regression in this helper would use.
Measured on node 18:
- REGown (
sys.modules[loaded.name] = moduleafterexec_module): 1309 passed at the tip. At the head it gives 2 failed (test_a_width_table_no_probe_is_named_for_is_refused_and_not_an_import_errorandtest_a_probe_given_the_width_that_has_none_empties_the_probe_tables), both atassert 'atom.compass.spec.validate_with_…' not in {…}, this line. - A probe that snapshots
sys.modulesaround onereimported_validate("probe_instance")call finds no new keys, at the tip and at the head. So the assertion is not passing because the helper registers under some other key.
What it does not see. A registration under any other key, for example the bare name, passes. This is mutant REGbare: 1307 passed at the head: green. I would not ask for that to be closed: nothing plausible registers under the bare name.
shrink: assert module not in sys.modules.values() covers every key, so it also subsumes the canonical-name check on L2019 (REG). The two lines become one. The probe confirms it reads False for an unregistered instance at both trees. This is optional.
|
Review cycle 1: PR #365 (issue #361), head Agent-authored review. I read the eight Design principles in Verdict: APPROVE at There are three non-blocking findings, all posted inline:
1. Mutation table (node 18,
|
| mutant | tip a9c7aed9d |
head c26a3a507 |
red ids at the head (tests/compass/test_spec_verbs.py::) |
|---|---|---|---|
| none (null control) | 1309 passed, 6 skipped | 1307 passed, 6 skipped | none |
| CA2L | 1309 passed | 1 failed | test_a_saved_transfer_merged_again_names_the_merge_that_dropped_its_pin: assert False (the startswith) |
| CA2mL | 1309 passed | 1 failed | test_a_transfer_keeps_the_source_stack_out_of_this_machines_pin: assert False (the startswith) |
| REGown | 1309 passed | 2 failed | test_a_width_table_no_probe_is_named_for_is_refused_and_not_an_import_error and test_a_probe_given_the_width_that_has_none_empties_the_probe_tables: assert 'atom.compass.spec.validate_with_…' not in {…} |
| CA2 | 1 failed | 1 failed | …merged_again_names_the_merge_that_dropped_its_pin, the same at both trees |
| CA2m | 1 failed | 1 failed | …source_stack_out_of_this_machines_pin, the same at both trees |
| REG | 2 failed | 2 failed | both reimport tests, at import_module(...) is not module, the same at both trees |
| PE | 8 failed | 8 failed | the same 8 ids at both trees |
GAPm (new) |
1309 passed | 1307 passed: green | none. See N1. |
GAPs (new) |
1309 passed | 1307 passed: green | none. See N2. |
REGbare (new) |
1309 passed | 1307 passed: green | none. See N3; not asked to close. |
Developer's claims checked against this table:
- The table in the PR body reproduces row for row. The only difference is the tip null of 1309 against 1307, which is explained by the tip having moved: the developer's tip column was
d10cb834f. - The "no refusal text changes" claim holds:
validate.pyis identical at both trees, at 452 lines.
2. Are the extended startswith needles stable?
Yes. Every value in the two needles comes from this module's own fixtures:
'mi355x-8gpu-2node'isMACHINE(L71), thefragment()default;transferred-from:mi300x-8gpuis passed explicitly by the test;a personand2026-09-18arefragment()'s own defaults (L167–168);- the layout is
Fragment.stanza()(merge.py:155), which is the rendering under test.
Incidental text. None. The only way to break the needles without touching this refusal is to change the fragment() defaults. That would already break the first_hand == at L613 and the provenance checks at L383–384, so the dependency is not new.
Whole-refusal coverage. The claim does not hold (N1). The needle ends carried and the endswith starts carried constants over from, but nothing ties them to the same occurrence, and GAPm is green. A single == closes it and is 3 lines shorter.
3. loaded.name not in sys.modules
It holds for the right reason (N3):
loaded.nameisatom.compass.spec.<name>. That is thespec.namethe importlib recipe registers under, so it is the key a regression would use, not a vacuous one. REGown is red.- A
sys.modulessnapshot around one helper call adds no keys at the tip or the head. - It does not see other keys (
REGbareis green). That is acceptable.
4. Every changed sentence (principle 8: "Every claim carries its measurement")
In the code: the diff changes no prose in code. The unchanged docstring's "does not register it" is now held for the helper's own name and the canonical name.
In the PR body and in c26a3a507's message:
- "the two now overlap and cover the whole refusal" is false, by measurement (N1).
- "In these fixtures only the method differs from
'tier2'" is true:fragment("links", LINKS)takes the same author and date defaults. - "ruff 0.16.7:
checkandformat --checkboth pass" is true: I re-ran both at the head on node 18, and each gave rc 0. - "the file carries no
#NNN,D<n>or 'principle N'" is true: a grep at the head finds none.
5. ponytail-review over the diff
AI_DEV_RULES gate 4: "The reviewer also runs the ponytail-review skill over the diff to catch over-engineering." Principle 3: "Prioritise simplicity. Add only what is necessary, and nothing more."
tests/compass/test_spec_verbs.py:L569-576: shrink: startswith + endswith pair that leaves the middle free. One `==` over the whole refusal, 5 lines.
tests/compass/test_spec_verbs.py:L605-610: shrink: startswith + two `in` checks over one contiguous clause. One startswith through "...built it from", 5 lines.
tests/compass/test_spec_verbs.py:L2018-2019: shrink: two per-name registration checks. `assert module not in sys.modules.values()`, 1 line, every key.
net: -5 lines possible.
Measurement: the first two were applied to a copy of the head. The file went from 2100 to 2096 lines, ruff was clean, and both tests passed. GAPm, GAPs, CA2L and CA2mL each go red against them. The third is unmeasured beyond the probe showing False for an unregistered instance, and it is optional.
6. Gate (node 18, xiaobizh_n18_cpu)
Setup:
- Tip: re-read at gate time as
a9c7aed9dad55aa0826a1e392e2145e1b6fb7ef8. The PR head was stillc26a3a507. - Merged tree:
git merge-tree --write-tree a9c7aed9d c26a3a507gives6de78ebbbe8a8c3d73fd2b20a75ddd662f057271with rc 0, the same as the developer's. It is not the head tree8a7c9b9d4, because the tip has moved since the branch pointd10cb834f. - Stamp:
git commit-tree 6de78ebbb -p a9c7aed9d -p c26a3a507gives5de14ecac9038744acf2980b680e4ec7edec3176. - Staging:
git archive, thendocker exec -i … tar -xinto/tmp/p365r1(not the shared mount). The tarball md57893ea97…matched on both ends..compass-commitand.compass-changedwere written from the samerev-parse;.compass-changedistests/compass/test_spec_verbs.py. - Script: the tree's own
scripts/compass/gate_cpu.sh(treebf85ab6e4, the same at tip and merged), run once, unpiped, undertimeout -k 10 2400.
| tree | printed commit: / atom: |
passed | skipped | xfailed | junit cases | GATE_CPU_RC |
|---|---|---|---|---|---|---|
merged 6de78ebbb |
5de14ecac (stamp) / /tmp/p365r1/stage/merged/ATOM/atom/__init__.py |
5265 | 155 | 3 | 5423 | 0 |
Against the developer's control: this equals their measured control at tip a9c7aed9d (5265 / 155 / 3, 5423 cases) and their merged run.
Timing classes: all 17 cases passed, so no re-run was needed. These are TestTheRegionIsNotCopiedPerChunk, TestNoSizeAtWhichACallStopsBeingOne and test_freezing_twice_is_additive_and_harmless.
Changed tests: the four touched tests all passed in the gate's junit.
Lands against tree 6de78ebbb while the tip is a9c7aed9d. If the tip moves, recompute the tree first.
For the next task in this area
- The string checks in the
PINNED_STACKtests are now the file's strongest pins, except for the two gaps above. - A needle pair that shares a word is not a covering pair unless the lengths are also pinned. Prefer
==when the fixtures determine the whole text.
|
Review cycle 1 addendum: PR #365, head Agent-authored. The verdict is unchanged: APPROVE at Gate at the second tipWhat moved:
Setup: the same recipe as above. The stamp was
Node-id delta between the two gated merged trees: 0 ids only in the first, 0 only in the second, and no changed outcomes. The timing classes passed 17 of 17 in both. Land against Under #367's amended landing ruleThe rule now says: "an APPROVE covering each head (the reviewer checks gates 1-3 before approving, and the approval is gate 4)". All three were checked before this approval:
"A label on an issue a PR delivers counts as on that PR": #361 and #365 both carry no labels, and #361's timeline has no label events. |
…s key (#374) The mismatch test checked its refusal with a startswith plus an endswith. #365 claimed the two together "cover the whole refusal". That was wrong: both needles contain "carried", and nothing tied them to the same occurrence, so another fragment's stanza rendered between them passed both. The refusal is fully determined by the test's own fixtures, so it is now one equality over the whole text. The saved-transfer test had the same gap between its startswith and an "over from" substring check. Its first clause is now one contiguous startswith, which subsumes both substring checks. The reimport helper checked that its instance was not registered under its own synthetic name, and that the canonical name did not resolve to it. One check that the instance is not any value in sys.modules covers both of those and every other key. Tests only; no refusal text changes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 participant
Closes #361
This is the follow-up to #357's review cycle 1 (#357 (comment)). It takes the three non-blocking findings R1-1, R1-2 and R1-3. The only file changed is
tests/compass/test_spec_verbs.py: +10 / −2. No refusal text changes.Dev record
What changed:
test_a_saved_transfer_merged_again_names_the_merge_that_dropped_its_pin): thestartswithneedle now runs through the whole stanza, meaning method, author and date, up tocarried constants over from. That is the shape of thefirst_handcontrol a few lines below.test_a_transfer_keeps_the_source_stack_out_of_this_machines_pin): the needle now runs through the stanza up tocarried. Theendswithbelow starts atcarried constants over from, so the two now overlap and cover the whole refusal.reimported_validate): this addsassert loaded.name not in sys.modules, andimport systo support it.Why I asserted rather than narrowed the docstring: the assertion holds more. The docstring's "does not register it" becomes true in full, where narrowing would drop the half that REGown shows is unheld (principle 6: a narrowed claim is a declared gap, while an assertion refuses the defect). It costs one line and one import. The existing
import_module(...) is not modulecheck still holds the canonical-name half, as REG shows below.Why the needles go past the method: CA2L and CA2mL render the method, author and date from
'links'. In these fixtures only the method differs from'tier2', so a needle that stopped at the method would catch both mutants. But it would leave author and date free for a future fixture where they differ. Running tocarriedcosts no extra line after ruff's reflow.What surprised me: the tip moved twice while I was gating. First it went from
d10cb834ftoc77d26a38(#362, docs only). Then it went toa9c7aed9d(#359, which adds 2 tests intest_capture_real_model.py). I re-gated against each tip with its own control; see below. Neither move touchesvalidate.pyor this file, so the mutation verdicts carry over byte for byte.Named result: mutation table (node 18,
xiaobizh_n18_cpu,tests/compass)Setup:
pr357-review1/tools/mutate.py, with the mutants byte-identical and only the docstring line changed.git archivetree.atom.__file__was under that copy in all 16 jobs.validate.py; 2092 for the test file at the tip and 2100 at the head.d10cb834fc26a3a507tests/compass/test_spec_verbs.py::)test_a_saved_transfer_merged_again_names_the_merge_that_dropped_its_pin:assert False(thestartswith). Head only.test_a_transfer_keeps_the_source_stack_out_of_this_machines_pin:assert False(thestartswith). Head only.test_a_width_table_no_probe_is_named_for_is_refused_and_not_an_import_errorandtest_a_probe_given_the_width_that_has_none_empties_the_probe_tables. Both fail onassert 'atom.compass.spec.validate_with_…' not in {…sys.modules…}. Head only.…merged_again_names_the_merge_that_dropped_its_pin. Same id at both trees.…source_stack_out_of_this_machines_pin. Same id at both trees.import_module(...) is not module. Same ids at both trees.The tip is the pre-fix code, so the tip column is the revert-your-own-fix check. Every other test passed in every job, and 6 were skipped each time.
Gate 1: ATOM's CPU tier, unmodified, as a delta against a measured control
How it ran:
xiaobizh_n18_cpu, with each tree's ownscripts/compass/gate_cpu.sh;timeout -k 10 2400, unpiped, one gate at a time;git archiveinto/tmp/i361viadocker exec -i … tar -x, with.compass-commitand.compass-changedwritten from the samerev-parse;git commit-treeover the merged tree, with parents tip and head.git merge-tree --write-tree <tip> c26a3a507commit:/atom:GATE_CPU_RCa9c7aed9d(current tip)a9c7aed9d (stamp)//tmp/i361/stage/tip3/ATOM/atom/__init__.py83661555c6de78ebbbe8a8c3d73fd2b20a75ddd662f057271, rc 083661555c (stamp)//tmp/i361/stage/merged3/ATOM/atom/__init__.pyc77d26a38c77d26a38 (stamp)/…/stage/tip2/ATOM/…5c433e7cf744e9f3c132009c3f73ab849b60d414885631445, rc 05c433e7cf (stamp)/…/stage/merged2/ATOM/…d10cb834f(tip at branch)d10cb834f (stamp)/…/stage/tip/ATOM/…eca17849f8a7c9b9d409bab836c2598c04e28b8b5d516a90a(the head tree), rc 0eca17849f (stamp)/…/stage/merged/ATOM/…Node-id delta (junit, at each tip: 5423 cases on each side at
a9c7aed9d, 5421 at the two earlier tips):Timing classes: all 17 cases passed on all six runs, so none needed a re-run. These are
TestTheRegionIsNotCopiedPerChunk,…[minimax]andtest_freezing_twice….Land against tree
6de78ebbb(tipa9c7aed9d).Other checks
checkandformat --checkboth pass on the file (rc 0).#NNN,D<n>or "principle N".Left undone
Nothing from #361.
🤖 Generated with Claude Code