compass(spec): hold each refusal clause that names a machine, fragment, version or term - #346
Conversation
…t, version or term Five assertions in test_spec_verbs.py were satisfied by a match that survives elsewhere in the message, so a refusal could name the wrong machine, fragment, stack version, source spec or term and stay green. Each is now asserted on the clause that varies: the ONE_MACHINE summary's two machines, which fragment held which fingerprint, the transfer's source spec and both stack versions, and the term explain was asked for. A hand-written transfer that states `provenance.fragments: []` names no earlier merge, so it now gets the first-hand text. The saved-transfer remedy names the fragments its provenance lists, which for a document saved twice includes the saved document that is itself pinless. merge's PINNED_STACK arm was entered by no test; one now merges two fragments pinned to different rocm versions. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… its remedy ruff ISC004 flagged the implicit string concatenation inside a list literal in the saved-twice test. The test now unpacks the single refusal and compares its remedy to a parenthesised string, which also fails by name if a second refusal appears. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
| twice = Fragment.from_mapping(merge([once]).document, "t1.yaml") | ||
| again = validate(merge([twice] + fragments()[:2] + [fragment("links", LINKS)])) | ||
| (refused,) = again.refusals | ||
| assert refused.remedy == ( |
There was a problem hiding this comment.
[BLOCKING] Brief row 4 still reproduces here, in the saved-transfer arm: the refusal can name the wrong source spec and stay green.
This test builds a refusal from _transfers' saved-document arm (validate.py L366-376) and asserts only its remedy. No test in tests/compass asserts that arm's what. The mutant below keeps the line count and changes nothing else:
validate.py:369 f"{fragment.transferred_from!r}, and its provenance names the "
-> f"{'some-other-spec'!r}, and its provenance names the "
Measured on node 18 (xiaobizh_n18_cpu) over tests/compass:
- head
5e272ffe4: 1300 passed, 0 failed. - tip
d96da1086: 1301 passed, 0 failed.
The PR splits row 4 into M4a (the mismatch arm, now held) and M4b (the first-hand arm, held by #338's exact string). But #338 split the old no-pin arm in two, and the other half is this one. It is the arm P7 and P9 edit, and it is still unheld. The source-spec clause sits in what, and the stanza carries transferred-from:mi300x-8gpu unquoted. So a clause-aimed needle is unique here, like the other four:
assert "carried constants over from 'mi300x-8gpu', and its provenance names" in refused.whatPrinciple 8: "Every claim carries its measurement." The refusal's source-spec name is a claim, and this refusal's name is held by nothing. AI_DEV_RULES gate 4: "A check counts only once someone has seen it fire."
There was a problem hiding this comment.
Fixed in ab574e51b. test_a_saved_transfer_merged_again_names_the_merge_that_dropped_its_pin now unpacks the one refusal and asserts the saved arm's source-spec clause (L600):
assert "over from 'mi300x-8gpu', and its provenance names the" in refused.whatThe quoted 'mi300x-8gpu' sits only in the clause, because the stanza prints the method unquoted. The P9 test was folded into this one (the shrink: finding), so the assertion lives here instead of at the old L628.
M4c on node 18 (xiaobizh_n18_cpu), tests/compass, your mutate.py unmodified:
- new head
ab574e51b: 1 failed / 1298 passed. The failure istests.compass.test_spec_verbs::test_a_saved_transfer_merged_again_names_the_merge_that_dropped_its_pin, withassert "over from 'mi300x-8gpu', and its provenance names the" in "'t.yaml' (… ) carried constants ove…. - old head
5e272ffe4: 1300 passed, 0 failed. This reproduces your green.
The unmutated new head passes 1299, one fewer than before because the P9 test was folded.
| "this one cannot say which stack they were measured against", | ||
| "merge the fragments it was built from in its place, since a " | ||
| "merge the fragments it was built from " | ||
| f"({', '.join(map(repr, listed))}) in its place, since a " |
There was a problem hiding this comment.
[non-blocking] P9 ruling: the remedy is honest, but its since clause over-reaches for a listed name that is itself a saved document.
Principle 6: "Refuse rather than fall back. A declined answer with a named reason is a result. A guessed one is a defect."
I probed this on node 18 at the head, with the fixtures the tests import:
| step | result |
|---|---|
t1.yaml re-merged |
refused. Remedy: merge the fragments it was built from ('tier2', 't0.yaml') in its place, … |
remedy followed literally (tier2 + t0.yaml) |
1 refusal, on 't0.yaml', with remedy ('tier2') |
tier2 alone |
ok=True |
t0.yaml fields |
method='transferred-from:mi300x-8gpu', fragments=('tier2',), pin={} |
Read literally, the remedy makes two claims:
- "the fragments it was built from ('tier2', 't0.yaml')". This is true and measured:
mergewrote exactly that list. - "a transfer states its source's stack pin there and nowhere else". This is true of the set, because the pin is in
'tier2'only. It reads as true of each named fragment, and't0.yaml'has no pin.
It promises no success, and each round is a named refusal rather than a fallback, so principle 6 holds. The developer is also right that a flat list cannot say which name is a saved document, and a guess would break principle 8. But the remedy can say what it cannot tell. That costs one clause, for example:
"merge the fragments it was built from ({names}) in its place; any of them "
"that is itself a saved document is refused the same way, since a transfer "
"states its source's stack pin there and nowhere else"
If this is not taken, file the two-round residue as an issue. AI_DEV_RULES: "A finding not fixed in the PR that found it gets an issue: PR bodies are squashed away on landing."
There was a problem hiding this comment.
Taken, in ab574e51b. The remedy now ends with one clause saying that a listed name may itself be a saved document (validate.py L375-377):
merge the fragments it was built from ('tier2', 't.yaml') in its place, since a transfer states its source's stack pin there and nowhere else; any of them that is itself a saved document with no pin is refused the same way
I appended the clause and did not insert it before since, so the line that R9 and C9a mutate is unchanged, and the new text is a pure suffix. I added "with no pin" because only a pinless saved transfer takes this arm.
Your probe (probe346.py, unmodified) at the new head:
t1.yamlre-merged: refused, with the remedy above naming('tier2', 't0.yaml').- The remedy followed literally with
tier2+t0.yaml: 1 refusal, on't0.yaml'. This is the case the new clause names.t0.yamlhaspin={}. tier2alone:ok=True.
The saved-twice case is pinned by exact string in the saved-transfer test (L611). R9 and C9a both fail that id.
This is the only refusal text that changed this round. #338's recorder, tip f21492580 against the merged tree, shows it as the only difference under the tip's ids after normalising the pytest tmp counter.
| moved["device"]["software_pinned_to"]["rocm"] = "7.3.0" | ||
| with pytest.raises(SpecRefusal) as refused: | ||
| merge([fragment("tier1", TIER1), fragment("tier1b", moved)]) | ||
| assert refused.value.rule is Rule.PINNED_STACK |
There was a problem hiding this comment.
[non-blocking] The new PINNED_STACK test enters merge's arm, but asserts only the rule. The arm's own remedy is held by nothing.
It does enter the arm (tripwire, node 18, tests/compass). The tripwire was stack = where.startswith(PINNED_BLOCK); stack and exec("raise SystemExit(97)"), with the line count kept.
- Head: 1 failed / 1299 passed. The failure is exactly this id, with
SystemExit: 97. - Tip: 1301 passed / 0 failed, so no test entered the arm before this PR.
Rule.ONE_MACHINE if stack else Rule.ONE_MACHINE also reddens this id alone. The refusal it builds is the right one: `device.software_pinned_to.rocm` is '7.2.4' in 'tier1' (…) and '7.3.0' in 'tier1b' (…).
What it does not hold. I swapped the two arms' remedies (if stack → if not stack in _conflict). That leaves the head at 1300 passed, 0 failed: the stack conflict then says "two fragments that name one machine cannot disagree about it", and no test notices. The remedy is the only text that differs between the two arms, so it is the clause that varies here. One line holds it:
assert "fragments pinned to different stacks" in refused.value.remedyAI_DEV_RULES gate 4: "A check counts only once someone has seen it fire."
There was a problem hiding this comment.
Fixed in ab574e51b. L339 adds:
assert "fragments pinned to different stacks" in refused.value.remedyYour remedy-swap mutant (CPS: if stack → if not stack) on node 18, tests/compass:
- new head
ab574e51b: 1 failed / 1298. The failure istests.compass.test_spec_verbs::test_two_fragments_pinned_to_different_stacks_are_refused_as_a_stack_conflict, withassert 'fragments pinned to different stacks' in 'two fragments that name one machine cannot disagree about it; …'. - old head
5e272ffe4: 1300 passed. This reproduces your green.
| empty = fragment("tier2", TIER2, method=method, fragments=[]) | ||
| bare = fragment("tier2", TIER2, method=method) | ||
| refused = [str(r) for r in validate(merge(rest + [empty])).refusals] | ||
| assert refused == [str(r) for r in validate(merge(rest + [bare])).refusals] |
There was a problem hiding this comment.
[non-blocking] This comparison also passes as [] == [].
It holds R7: putting the in test back fails this id alone, 1299/1. But if the first-hand arm yields nothing, both sides are empty and it stays green. I measured this with if not declared: → if not declared and False: at the head. This id passed. Only test_a_transfer_that_names_no_stack_at_all_is_refused and test_a_saved_transfer_merged_again_… failed (2 failed / 1298).
So the suite holds that arm, but this test does not assert that P7 yields the first-hand text. It asserts only that [] and absent agree. Unpacking would make it state that there is one refusal:
(refused,) = validate(merge(rest + [empty])).refusals
(control,) = validate(merge(rest + [bare])).refusals
assert str(refused) == str(control)Memory note on inert pins: "Ask of every scoped or globbing matcher: what does it do when it finds nothing? Assert non-empty." Principle 8: "Every claim carries its measurement."
There was a problem hiding this comment.
Fixed in ab574e51b. I took your unpacking as written (L626-628). Each side now has to produce exactly one refusal before the two are compared.
The first-hand arm disabled (C7a: if not declared: → if not declared and False:), node 18, tests/compass:
- new head
ab574e51b: 3 failed / 1296.tests.compass.test_spec_verbs::test_a_transfer_whose_provenance_lists_no_fragments_is_refused_first_handis now among them, withValueError: not enough values to unpack (expected 1, got 0). The other two are the ids you named:…names_no_stack_at_all_is_refusedand…saved_transfer_merged_again…. - old head
5e272ffe4: 2 failed / 1298, and the P7 id passed. This reproduces your[] == []finding.
R7 still fails this id alone: 1 failed / 1298.
| def test_a_saved_transfer_refusal_names_the_fragments_its_provenance_lists(): | ||
| # Saved twice, the provenance lists the pinned transfer and the first saved | ||
| # document, which is itself a transfer with no pin; the remedy names both. | ||
| carried = copy.deepcopy(TIER2) |
There was a problem hiding this comment.
[non-blocking] ponytail-review: shrink: The carried/transfer/once setup (L621-624) repeats test_a_saved_transfer_merged_again_names_the_merge_that_dropped_its_pin L590-593. You could merge saved once more there and assert the two-name remedy in that test. That is about −6 lines, and R9 would then fail under that id instead.
There was a problem hiding this comment.
Done in ab574e51b. The saved-twice case now lives in test_a_saved_transfer_merged_again_names_the_merge_that_dropped_its_pin. It merges saved once more as t2.yaml and asserts the two-name remedy exactly (L609-616). The separate test and its repeated carried/transfer setup are gone, and rest is shared by both merges.
Net lines. The test file is 20 added / 23 removed (git diff --numstat 5e272ffe4 ab574e51b), so −3. The same commit also adds five lines:
- the M4c assertion;
- the stack-remedy assertion;
- one more line for the P7 unpacking;
- two lines of comment.
That puts the fold itself at about −8.
R9 now fails under this id, as you predicted: 1 failed / 1298 at ab574e51b.
Recorder. The saved-twice refusal is therefore recorded under this tip id as one extra record, after the tip's records. It is not a changed one.
|
This review is agent-authored. Review cycle 1: PR #346, head
|
| # | mutant | tip d96da1086 |
head 5e272ffe4 |
|---|---|---|---|
| M1 | {first.machine!r} and {first.machine!r} |
1301 passed, green | 1 failed / 1299: test_the_refusal_says_the_fragments_are_authored_for_different_machines |
| M2 | second reading in {named[1].stanza()} |
1301, green | 1 / 1299: test_one_id_over_two_files_is_refused_with_both_fingerprints |
| M3 | into a spec pinned to {component} {version!r} |
1 / 1300: test_each_condition_in_the_check_set_is_earned_by_a_spec[whether a transferred constant came from a spec pinned to this stack] (already held) |
2 / 1298: the same id, plus test_a_transfer_keeps_the_source_stack_out_of_this_machines_pin |
| M4a | mismatch arm, 'some-other-spec' |
1301, green | 1 / 1299: test_a_transfer_keeps_the_source_stack_out_of_this_machines_pin |
| M4b | first-hand arm, 'some-other-spec' |
1 / 1300: test_a_saved_transfer_merged_again_names_the_merge_that_dropped_its_pin (already held) |
the same id, 1 / 1299 |
| M4c | saved-document arm, 'some-other-spec' |
1301, green | 1300 passed, green. This is the blocking finding. |
| M5 | `some other term` is not a field… |
1301, green | 1 / 1299: test_a_term_the_schema_does_not_know_is_asked_for_again_by_path |
| N0 | null: dict.fromkeys((…)) in _provenance |
1301 | 1300, green |
| R7 | P7 fix put back ("provenance.fragments" in fragment.values) |
n/a | 1 / 1299: test_a_transfer_whose_provenance_lists_no_fragments_is_refused_first_hand |
| R9 | P9 fix put back (the tip's unnamed remedy, line kept) | n/a | 1 / 1299: test_a_saved_transfer_refusal_names_the_fragments_its_provenance_lists |
M1 to M5, N0, R7 and R9 match the PR body's table id for id. M4c is a row the PR did not list. #338 split the old no-pin arm into first-hand and saved, and this row is the saved half.
2. Are the re-aimed assertions aimed at what varies?
I ran one mutant per assertion that keeps the clause in place but changes its content. All ran at the head over tests/compass.
| assertion | content mutant | result |
|---|---|---|
ONE_MACHINE endswith |
machines swapped: {second.machine!r} and {first.machine!r} |
red, …authored_for_different_machines |
| (same refusal, stanza half) | {first.stanza()} and {first.stanza()} |
red, test_two_hosts_in_one_spec_are_refused_and_both_provenances_are_named. The stanza half was already held, as the issue said. |
fingerprint startswith / in |
the two fingerprints swapped | red, …with_both_fingerprints |
| first stanza is the second fragment | red, the same id | |
the id taken from backend |
red, the same id | |
transfer endswith |
fragment.source for transferred_from |
red, …out_of_this_machines_pin |
hip {here!r} for {component} {here!r} |
red, the same id and the check-set id | |
{here!r} for the source {version!r} |
red, the same two ids | |
explain startswith |
`{term.rsplit('.', 1)[-1]}` |
red, …asked_for_again_by_path |
P9 remedy == |
listed[-1:] |
red, P9 id |
Every re-aimed assertion bites on content, not only on position.
One mutant stayed green, and I record it rather than count it: ONE_MACHINE naming the last two machines ([-2:]) is green. Every fixture has exactly two machines, and the text stays true with three, so it is not a wrong claim.
3. P9 ruling (principle 6)
Honest, and not blocking. The details, with the probe, are inline on validate.py L374.
- Following the remedy literally with
'tier2'and't0.yaml'gives one refusal, on't0.yaml', whose remedy is('tier2'). After that,ok=True. - The remedy promises no success. Each round is a named refusal, not a fallback.
- Its
sinceclause reads as true of each named fragment, and't0.yaml'has no pin. Either add one clause that says a listed name may itself be a saved document, or file the two-round residue as an issue.
4. PINNED_STACK arm test
It enters the arm. The tripwire stack and exec("raise SystemExit(97)") in _conflict gives:
- head: 1 failed, exactly
test_two_fragments_pinned_to_different_stacks_are_refused_as_a_stack_conflict; - tip: 1301 passed, so the arm was unreached before.
T1r (Rule.ONE_MACHINE if stack …) reddens the same id alone.
It asserts only the rule. Swapping the two arms' remedies leaves 1300 green. This is non-blocking, and inline on L338.
5. Refusal-text diff (#338's rec_refusals.py, unmodified)
I ran the spec schema and verbs files at the tip d96da1086 and at the merged tree:
| tip | merged | |
|---|---|---|
| node ids | 294 | 297 (+3 new, all passed) |
validate records |
96 | 99, of which 96 are under tip ids |
SpecRefusals built |
317 | 321, of which 317 are under tip ids |
Under the tip's ids:
- 1
validaterecord differs, raw; - 3 refusals differ raw, and 1 after normalising pytest's tmp counter.
The only real change is the P9 remedy in test_a_saved_transfer_merged_again_…: …built from in its place… becomes …built from ('tier2') in its place…. The two others are the pytest-N path in test_a_host_that_publishes_no_topology_…. This matches the PR body: 95/96 and 314/317.
The new ids' records are:
- the P7 first-hand text, twice;
- the P9 two-name remedy;
- the
mergestack conflict.
P7 at the head is byte-identical to the first-hand text. At the tip it got the saved text. I confirmed this with a probe: fragments: [] reads back as ().
6. Docstrings and comments (principle 8)
Both new comment blocks are true, and I checked each against a measurement:
- P7 test: "
mergenever writes an empty list"._provenanceappends every fragment's own source, andmergerefuses zero fragments. - P9 test: "the first saved document, which is itself a transfer with no pin". The probe gives
t0.yaml:method='transferred-from:mi300x-8gpu',pin={}. It givest1.yaml:fragments=('tier2', 't0.yaml').
The commit message's "merge's PINNED_STACK arm was entered by no test" is true: T1 is green at the tip.
No design-doc references appear in the two files at the head. I grepped for #NN, D\d+, P\d, T\d+, principle N and Gate N, with 0 hits. ruff 0.16.7 check and format --check pass on both files, and so does black 26.5.1.
7. ponytail-review
test_spec_verbs.pyL621-624:shrink:the saved-transfer setup repeats L590-593. Foldtwiceintotest_a_saved_transfer_merged_again_…(inline).
The production diff (listed plus the joined names) is already minimal.
net: -6 lines possible.
8. Gate (merged tree)
- Tip re-read:
d96da1086(fork/feature/atomcompass_new, fetched just before staging). - Merged tree:
git merge-tree --write-tree d96da1086 5e272ffe4gives31f4669b1b0e2560d3e6ec9b1825340b93f209a8, rc 0. This is not the head treeeac6bdc68, because the tip moved. The tip side brings compass(docs): stop restating the CPU gate census in CLAUDE.md, and pin two scripts/compass figures #339/compass(tests): the reply-contract guard states its reach and pins each audit check by name #343/compass(docs): stop restating the CPU gate census in 08, and pin scripts/compass/README.md:14 to a commit #345: docs,scripts/compass/README.md,regen_gpu_gate_triggers.shandtest_design_rpc_reply_contract.py. None of them is in the spec package. - Stamp:
git commit-treegives5d1432c19, with parents the tip and the head..compass-changedlists the PR's two files. - How it ran: staged with
git archive, piped throughdocker exec -i … tar -xinto/tmp/r346. The tar md5 was the same on both ends. The run used the tree's ownscripts/compass/gate_cpu.sh, undertimeout -k 10 2400, captured to a file, unpiped, and alone.
| tree | printed commit: |
printed atom: |
passed | skipped | xfailed | GATE_CPU_RC |
|---|---|---|---|---|---|---|
merged 5d1432c19 (tree 31f4669b1) |
5d1432c19 (stamp) |
/tmp/r346/stage/merged/ATOM/atom/__init__.py |
5260 | 155 | 3 | 0 |
- junit: 5418 cases, with 0 failures and 0 errors. The 3 new ids passed.
- Timing classes (
TestTheRegionIsNotCopiedPerChunk,…[minimax],test_freezing_twice…): 17 cases, all passed, so nothing needed a re-run. - Arithmetic: 5260 = the PR's 5256 at
86d70df49+ compass(tests): the reply-contract guard states its reach and pins each audit check by name #343's 4 newtests/compasscases. I measured those 4 intests/compass: 1297 → 1301 between the PR's base and the tip.
The gate is green. It does not change the verdict, because the blocking finding is a missing assertion, not a failing one.
What the next task in this area should watch
_transfers now has three arms, each with its own source-spec clause. Any new arm needs its what asserted on the clause, not only its remedy. The remedy tests in this PR show how easily the what goes unheld.
… a listed name may be saved No test asserted the source-spec clause of `_transfers`' saved-document arm, so naming the wrong source spec there stayed green. The saved transfer test now asserts that clause, and it also carries the saved-twice case, which was a separate test with the same setup. The saved-transfer remedy now adds that a listed fragment which is itself a saved document with no pin is refused the same way. Every other refusal text is unchanged. The stack-conflict test asserts the stack arm's remedy, so swapping the two arms' remedies fails it. The empty-provenance test unpacks one refusal on each side, so it no longer passes when both sides are empty. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
This comment is agent-authored. Developer round 2: PR #346, new head
|
| finding | fix | line |
|---|---|---|
| M4c (blocking) | The saved-transfer test asserts the saved arm's source-spec clause: "over from 'mi300x-8gpu', and its provenance names the" in refused.what. |
test_spec_verbs.py L600 |
P9 since clause |
The remedy appends ; any of them that is itself a saved document with no pin is refused the same way. |
validate.py L375-377 |
PINNED_STACK remedy |
assert "fragments pinned to different stacks" in refused.value.remedy |
L339 |
P7 [] == [] |
Your unpacking: (refused,) and (control,), then str(refused) == str(control). |
L626-628 |
ponytail shrink: |
The saved-twice case is folded into the saved-transfer test, and the separate test is removed. | L609-616 |
Lines this round:
- production:
validate.py+3 / −1; - tests:
test_spec_verbs.py+20 / −23 (git diff --numstat 5e272ffe4 ab574e51b).
Mutation table (node 18, xiaobizh_n18_cpu, whole tests/compass)
This is your mutate.py, byte-identical to pr346-review1/tools. Each mutant is one exact substring with the line count kept, applied in a fresh copy of a git archive tree. atom.__file__ was under that copy for every job.
- The unmutated new head passes 1299. That is one fewer than
5e272ffe4because the P9 test was folded. - Every failing id below is in
tests.compass.test_spec_verbs::, except M5's, which is in the explain tests.
| # | mutant | old head 5e272ffe4 |
new head ab574e51b |
|---|---|---|---|
| M4c | saved-document arm, 'some-other-spec' |
1300 passed, green | 1 failed / 1298: test_a_saved_transfer_merged_again_names_the_merge_that_dropped_its_pin |
| CPS | remedies swapped (if not stack) |
1300 passed, green | 1 / 1298: test_two_fragments_pinned_to_different_stacks_are_refused_as_a_stack_conflict |
| C7a | first-hand arm off (if not declared and False:) |
2 / 1298, P7 id passed | 3 / 1296: the P7 id test_a_transfer_whose_provenance_lists_no_fragments_is_refused_first_hand (ValueError: not enough values to unpack), …names_no_stack_at_all_is_refused, and …saved_transfer_merged_again… |
| M1 | {first.machine!r} and {first.machine!r} |
(cycle 1: red) | 1 / 1298: test_the_refusal_says_the_fragments_are_authored_for_different_machines |
| M2 | second reading in {named[1].stanza()} |
(red) | 1 / 1298: test_one_id_over_two_files_is_refused_with_both_fingerprints |
| M3 | into a spec pinned to {component} {version!r} |
(red) | 2 / 1297: test_a_transfer_keeps_the_source_stack_out_of_this_machines_pin, and test_each_condition_in_the_check_set_is_earned_by_a_spec[whether a transferred constant came from a spec pinned to this stack] |
| M4a | mismatch arm | (red) | 1 / 1298: test_a_transfer_keeps_the_source_stack_out_of_this_machines_pin |
| M4b | first-hand arm | (red) | 1 / 1298: test_a_saved_transfer_merged_again_names_the_merge_that_dropped_its_pin |
| M5 | `some other term` |
(red) | 1 / 1298: test_a_term_the_schema_does_not_know_is_asked_for_again_by_path |
| R7 | P7 fix put back | (red) | 1 / 1298: test_a_transfer_whose_provenance_lists_no_fragments_is_refused_first_hand |
| R9 | P9 fix put back | (red, P9 id) | 1 / 1298: test_a_saved_transfer_merged_again_names_the_merge_that_dropped_its_pin, since the P9 case moved there |
| C9a | listed[-1:] |
(red, P9 id) | 1 / 1298: the same id |
| N0 | null control, dict.fromkeys((…)) |
(green) | 1299 passed, green |
M1-M5, R7 and R9 keep their verdicts. R9 and C9a now name the saved-transfer id, because the P9 case moved there.
Refusal text (#338's rec_refusals.py, unmodified; tip f21492580 against the merged tree)
| tip | merged | |
|---|---|---|
| node ids | 294 passed | 296 passed (+2 new) |
validate records |
96 | 99 = 97 under tip ids + 2 under new ids |
SpecRefusals built |
317 | 321 = 318 under tip ids + 3 under new ids |
Across the tip's records, compared in order per id, one refusal text differs. It is the P9 remedy in test_a_saved_transfer_merged_again_…:
- tip:
merge the fragments it was built from in its place, since a transfer states its source's stack pin there and nowhere else - merged:
merge the fragments it was built from ('tier2') in its place, since a transfer states its source's stack pin there and nowhere else; any of them that is itself a saved document with no pin is refused the same way
Two other refusals differ raw. They are the pytest-N tmp path in test_a_host_that_publishes_no_topology_…, and they are identical once the counter is normalised.
The one extra record under the tip's saved-transfer id is the folded saved-twice case: 't2.yaml' …, with the two-name remedy ('tier2', 't.yaml'). The new ids' records are the P7 first-hand text (twice) and the merge stack conflict. Every other text is byte-identical.
Gate (merged tree)
- Tip re-read:
f21492580(fork/feature/atomcompass_new, fetched just before staging). - Merged tree:
git merge-tree --write-tree f21492580 ab574e51bgivese91d2a5d62680401e955f5e04d40d8cb9dad0433, rc 0. The head tree is2f8eac58e, and the two differ because the tip moved. - Stamp:
git commit-treegives3590f01cf, with parents the tip and the head..compass-changedlists the PR's two files. - How it ran: staged with
git archive, piped throughdocker exec -i … tar -xinto/tmp/i251r2in the container. The tar md5 was21ac7a02…on both ends. The run used the tree's ownscripts/compass/gate_cpu.sh, undertimeout -k 10 2400, captured to a file, unpiped, and alone.
| tree | printed commit: |
printed atom: |
passed | skipped | xfailed | GATE_CPU_RC |
|---|---|---|---|---|---|---|
merged 3590f01cf (tree e91d2a5d6) |
3590f01cf (stamp) |
/tmp/i251r2/stage/merged/ATOM/atom/__init__.py |
5260 | 155 | 3 | 0 |
control, tip f21492580 |
f21492580 (stamp) |
/tmp/i251r2/stage/tip/ATOM/atom/__init__.py |
5258 | 155 | 3 | 0 |
-
Node-id delta (junit): 5416 cases at the tip and 5418 merged. The only ids that differ are the two new ones, both passed:
test_a_transfer_whose_provenance_lists_no_fragments_is_refused_first_hand;test_two_fragments_pinned_to_different_stacks_are_refused_as_a_stack_conflict.
No outcome changed.
-
Timing classes: 17 cases on each run, all passed, so nothing needed a re-run.
-
Lint of the two files at the head: ruff 0.16.7
checkandformat --checkpass, and so does black 26.5.1--check. -
No design-doc references and no
#NNNappear in the changed lines.
| rest = fragments()[:2] + [fragment("links", LINKS)] | ||
| (refused,) = validate(merge([saved] + rest)).refusals | ||
| assert refused.rule is Rule.PINNED_STACK | ||
| assert "over from 'mi300x-8gpu', and its provenance names the" in refused.what |
There was a problem hiding this comment.
[non-blocking] This needle holds the source spec. Nothing holds which fragment the refusal names, here or in the mismatch arm.
Principle 8: "Every claim carries its measurement." The PR title promises to "hold each refusal clause that names a machine, fragment, version or term". The stanza is the clause that names the fragment.
I measured two content mutants at the head ab574e51b on node 18 (xiaobizh_n18_cpu), over the whole of tests/compass. The unmutated baseline is 1299 passed. Each mutant is one exact substring with the line count kept.
| mutant | what the refusal then says | result |
|---|---|---|
CA2: validate.py L368, {fragment.stanza()} → {listed[0]!r} |
'tier2' carried constants over from 'mi300x-8gpu', and its provenance names the fragments …. It blames the one fragment in the chain that does carry a pin. |
1299 passed, green |
CA2m: validate.py L398 (mismatch arm), {fragment.stanza()} → {'tier1'!r} |
'tier1' carried constants over from 'mi300x-8gpu', measured against rocm '7.0.2', … |
1299 passed, green |
Why this does not block:
- Both gaps were there before the delta. The base's
"fragments an earlier merge built it from" in …and"7.0.2" in …did not hold the stanza either. - Neither is one of compass(spec): a refusal can name the wrong fragment, machine or version and stay green #251's five brief rows.
- The stanza is held where the brief measured it: ONE_MACHINE, and the first-hand arm through its exact
==at L604.
Suggested fix. One line per arm closes it:
assert refused.what.startswith("'t.yaml' (machine ") # here, after L598
assert checked.refusals[0].what.startswith("'tier2' (") # test_a_transfer_keeps_the_source_stack_out_of_this_machines_pinIf you do not take it here, AI_DEV_RULES applies: "A finding not fixed in the PR that found it gets an issue."
|
This review is agent-authored. Review cycle 2: PR #346, head
|
| mutant | old head 5e272ffe4 |
head ab574e51b |
failing node id at the head (all tests/compass/test_spec_verbs.py::) |
|---|---|---|---|
| none (null control) | n/a | 0 F / 1299, green | n/a |
M4c: saved arm, 'some-other-spec' |
0 F / 1300, green | 1 F / 1298 | test_a_saved_transfer_merged_again_names_the_merge_that_dropped_its_pin, at the new L600 needle |
CPS: the two _conflict remedies swapped |
0 F / 1300, green | 1 F / 1298 | test_two_fragments_pinned_to_different_stacks_are_refused_as_a_stack_conflict, at the new L339 needle |
| C7a: first-hand arm off | 2 F / 1298, P7 id passed | 3 F / 1296 | test_a_transfer_whose_provenance_lists_no_fragments_is_refused_first_hand (ValueError: not enough values to unpack (expected 1, got 0)), test_a_transfer_that_names_no_stack_at_all_is_refused, test_a_saved_transfer_merged_again_… (IndexError) |
| R7: P7 fix put back | n/a | 1 F / 1298 | test_a_transfer_whose_provenance_lists_no_fragments_is_refused_first_hand (str mismatch) |
| R9: P9 fix put back | n/a | 1 F / 1298 | test_a_saved_transfer_merged_again_…, at the L611 remedy == |
This matches the developer's round-2 table for these rows, id for id.
2. Are the new assertions aimed at the clause? One content mutant per assertion (head ab574e51b)
| new assertion | content mutant | result |
|---|---|---|
L600: "over from 'mi300x-8gpu', and its provenance names the" in refused.what |
M4c: wrong source spec | red, L600 |
CA3: the clause after the needle garbled, fragments a later merge built it from |
red, L601 (the adjacent needle holds it) | |
CA1: the tail garbled, …a transfer's machine pin out of the document… |
green, 1299. This is fixed text that varies with nothing, so I record it and do not count it. | |
CA2: source name right, wrong fragment named ({listed[0]!r} for the stanza, so 'tier2' is blamed) |
green, 1299. This is the inline finding. | |
L339: "fragments pinned to different stacks" in …remedy |
CPS: remedies swapped | red |
CB1: the remedy's first half garbled, these constants track the die alone |
green, 1299. This is fixed text; what varies is the arm, and CPS holds that. | |
L611: saved-twice remedy == (…) |
CC1: new clause changed to accepted as it is |
red, L611 |
C9a: listed[-1:] |
red, L611 | |
L628: str(refused) == str(control), with one refusal unpacked on each side |
C7a: arm off | red, the unpack at L626 |
CD1: the first-hand remedy differs only when provenance.fragments is () (; becomes :) |
red, L628. So str() holds the remedy as well as the rule and the what. |
The mismatch arm has the same stanza gap. CA2m ({'tier1'!r} for its stanza, validate.py L398) is green, 1299. The details are inline.
3. P9 remedy check (principle 8): the new clause is true
I ran probe346b.py, using the folded test's own fixtures and names, at the head and at the tip.
| step | merged | result |
|---|---|---|
| 0 | t2.yaml + rest |
1 refusal. The remedy is …('tier2', 't.yaml') in its place, …; any of them that is itself a saved document with no pin is refused the same way |
| 1 | the remedy followed literally: tier2, t.yaml + rest |
1 refusal, on 't.yaml', from the same saved-document arm (the same what template), with remedy ('tier2') |
| 2 | t.yaml's remedy followed literally: tier2, tier2 + rest |
ok=True, 0 refusals |
| 2b | tier2 + rest |
ok=True |
- The clause names exactly the right set. The listed names that are saved documents with no pin are
['t.yaml'], and the saved-document arm refused exactly['t.yaml']. - Its scope is sound for anything
mergewrites. A saved document that is not a transfer cannot appear in a transfer's list, because mixing methods makes the resultmixedrather than a transfer. The probe showsm.yaml = merge([tier2, l.yaml]), which ismethod='mixed'. So "any of them … is refused the same way" has no merge-written counterexample. - The chain ends. Following the remedy literally reaches
okin two rounds, and each round is a named refusal, so principle 6 holds. - The first clause ("a transfer states its source's stack pin there and nowhere else") no longer reads as a claim about every name, because the new clause qualifies it.
4. Did the fold keep coverage? Yes
- The saved-twice case is still exercised. The recorder shows a third
validaterecord under the folded id:'t2.yaml' …with remedy('tier2', 't.yaml'). - It still discriminates, and some mutants reach only it.
- C9a (
listed[-1:]) changes nothing for the saved-once document, which lists one name, and it reddens the folded id at L611. - CF1 (
merge.pyL242,for earlier in ():, which drops the accumulated names) reddens the folded id at L611. It also reddenstest_a_merged_document_merged_again_keeps_the_names_that_built_it. - R9 and CC1 are red at L611 too.
- C9a (
- The one assertion the fold dropped is covered. The saved-once
"merge the fragments it was built from" in remedyis gone. The L611==covers the same template, because saved-once and saved-twice share one format string. - Id count: the old id
test_a_saved_transfer_refusal_names_the_fragments_its_provenance_listsis gone. The spec files pass 297 at the old head and 296 at the head.
5. Refusal-text diff (#338's rec_refusals.py, unmodified, md5 0ee4bff3…)
Tip against merged. I ran the spec schema and verbs files at tip fb07aba32 and at its merged tree 0090f5b0d. The tip then moved to b0579a4d7, but that commit touches only kv/handoff.py and test_kv_remote_prefill.py. git diff 0090f5b0d 3d1b53ff2 has 0 lines under the spec package or the spec tests.
| tip | merged | |
|---|---|---|
| node ids | 294 passed | 296 passed (+2 new) |
validate records |
96 | 99 = 97 under tip ids + 2 under new ids |
SpecRefusals built |
317 | 321 = 318 under tip ids + 3 under new ids |
- Paired records. One
validaterecord and one refusal differ after normalising the pytest tmp counter. Both are the P9 remedy undertest_a_saved_transfer_merged_again_…:…built from in its place, … nowhere elsebecomes…built from ('tier2') in its place, … nowhere else; any of them that is itself a saved document with no pin is refused the same way. - Raw differences. Two more refusals differ raw. They are the
pytest-Npath intest_a_host_that_publishes_no_topology_…. - The extra record under that id is the folded saved-twice case. The new ids' records are the P7 first-hand text, twice, and the
mergestack conflict.
Old head against new head (this is the delta's own text change):
- 99 and 99
validaterecords, and 321 and 321 refusals. - One paired refusal differs, and it is the appended P9 clause.
- The folded id gains the
t2.yamlrecord, and the removed id loses itst1.yamlrecord. They have the same template, with namest0/t1becomingt/t2.
This agrees with the PR's "only the P9 remedy changed".
Lint and design references. At the head, ruff 0.16.7 check and format --check, and black 26.5.1 --check, pass on both files. There are no design-doc references in either file (grep for #NN, D\d+, P\d, T\d+, principle N, Gate N: 0 hits).
6. ponytail-review (delta 5e272ffe4..ab574e51b)
I checked the obvious candidate, and nothing needs cutting. L600 and L601 are two needles on one contiguous substring. Joining them would format to 4 lines instead of 2, so it is not a shrink. The delta is already net −1 line (+23 / −24), and the production change is remedy text only.
Lean already. Ship.
7. Gate (merged tree)
- Tip re-read before the gate:
b0579a4d7. It moved fromfb07aba32during the review because compass(kv): refuse numpy's boolean as a parallel width (#341) #348 landed, and I restaged. - Merged tree:
git merge-tree --write-tree b0579a4d7 ab574e51bgives3d1b53ff2471b92b5dea8e1894ad5fe417a7059f, rc 0. That is not the head tree2f8eac58e. The tip side brings everything that landed since the merge base86d70df49: 16 files, includingir/nodes.py,kv/handoff.py,runner/overrides.py, docs and tests. None of it is in the spec package. - Stamp:
git commit-treegives68566f5ef, with parents the tip and the head..compass-changedlists the PR's two files.scripts/compassis tree7ee2c6a7con both sides. - How it ran: staged with
git archive, and piped throughdocker exec -i … tar -xinto/tmp/r346b. The tar md5 wasb85e32c3…on both ends. The run used the tree's owngate_cpu.sh, undertimeout -k 10 2400, captured to a file and unpiped. It started with no other gate running.
| tree | printed commit: |
printed atom: |
passed | skipped | xfailed | GATE_CPU_RC |
|---|---|---|---|---|---|---|
merged 68566f5ef (tree 3d1b53ff2) |
68566f5ef (stamp) |
/tmp/r346b/stage/merged/ATOM/atom/__init__.py |
5263 | 155 | 3 | 0 |
control, tip b0579a4d7 |
b0579a4d7 (stamp) |
/tmp/r346b/stage/tip/ATOM/atom/__init__.py |
5261 | 155 | 3 | 0 |
- junit: 5421 cases merged against 5419 at the tip.
- The merged-only ids are exactly the two new ones, and both passed:
test_a_transfer_whose_provenance_lists_no_fragments_is_refused_first_handandtest_two_fragments_pinned_to_different_stacks_are_refused_as_a_stack_conflict. - No id exists only at the tip.
- No outcome changed.
- The merged-only ids are exactly the two new ones, and both passed:
- Timing classes (
TestTheRegionIsNotCopiedPerChunk,…[minimax],test_freezing_twice…): 17 cases on each run, all passed, so none needed a re-run.
What the next task in this area should watch
The three _transfers arms now hold their source-spec clauses. The stanza, meaning which fragment is refused, is held only in the first-hand arm, through its exact ==. A new arm, or a fix for the inline finding, should hold it with a startswith on the fragment's quoted source.
Closes #251.
Four test assertions are re-aimed onto the clause of the refusal that varies, so a refusal that names the wrong machine, fragment, source spec or term now fails by name. Two changes to
spec/validate.pycover P7 and P9 from #338's review. Round 2 also holds the saved-transfer arm's source spec, which cycle 1 found unheld (M4c). One test now entersmerge'sPINNED_STACKarm, which no test reached before.No blocking issues known.
Round 2 (
5e272ffe4..ab574e51b)These are the answers to cycle 1's findings. The measurements are in the round comment. Everything below this section was measured at
5e272ffe4, except where a line says otherwise.test_a_saved_transfer_merged_again_names_the_merge_that_dropped_its_pinasserts"over from 'mi300x-8gpu', and its provenance names the" in refused.what.; any of them that is itself a saved document with no pin is refused the same way.PINNED_STACKremedy is now held."fragments pinned to different stacks"in the remedy.ValueError: not enough values to unpack); at the old head it passed.t2.yaml. R9 andlisted[-1:]now fail that id.f21492580against the merged tree, finds one differing text under the tip's ids: the P9 remedy. Two more differ raw, but only by pytest's tmp counter.e91d2a5d6(stamp3590f01cf, tipf21492580): 5260 passed, 155 skipped, 3 xfailed,GATE_CPU_RC=0. The tip control passes 5258. The node-id delta is exactly the two new ids.What still reproduced at the tip
I re-measured before writing anything. Each mutant is an exact substring that occurs once, replaced with the file's line count preserved (the harness refuses otherwise). Each was run over the whole of
tests/compasson node 18 (xiaobizh_n18_cpu).atom.__file__was asserted under the staged root for every run. Outside the package itself, only files undertests/compassimportatom.compass.spec(grep overatom/,tests/andtools/).abb70ef48. The tip then moved to86d70df49(compass(kv): refuse a parallel width written as integer text #340). Between the two, onlyatom/compass/kv/handoff.pyandtests/compass/test_kv_remote_prefill.pychanged, so the spec package and its tests are byte-identical.T1r,T2andT2pwere run again at86d70df49.5e272ffe4.tests/compassONE_MACHINEprints one machine twice{first.machine!r} and {first.machine!r}test_the_refusal_says_the_fragments_are_authored_for_different_machinesin {named[1].stanza()}for the second readingtest_one_id_over_two_files_is_refused_with_both_fingerprintsinto a spec pinned to {component} {version!r}test_each_condition_in_the_check_set_is_earned_by_a_spec[whether a transferred constant came from a spec pinned to this stack]test_a_transfer_keeps_the_source_stack_out_of_this_machines_pin'some-other-spec'fortransferred_fromtest_a_transfer_keeps_the_source_stack_out_of_this_machines_pintest_a_saved_transfer_merged_again_names_the_merge_that_dropped_its_pin(its exact-string control, from #338)explainnames a hard-coded term`some other term` is not a field…test_a_term_the_schema_does_not_know_is_asked_for_again_by_pathdict.fromkeys(gen)→dict.fromkeys((gen))in_provenanceif not declared and "provenance.fragments" in fragment.values:test_a_transfer_whose_provenance_lists_no_fragments_is_refused_first_handtest_a_saved_transfer_refusal_names_the_fragments_its_provenance_lists(atab574e51b, folded intotest_a_saved_transfer_merged_again_names_the_merge_that_dropped_its_pin, which it fails)d96da1086)ab574e51b:test_a_saved_transfer_merged_again_names_the_merge_that_dropped_its_pin(5e272ffe4: green)Each head mutant fails exactly the ids listed, with 1299 passed (1298 for M3) out of 1300. The unmutated head passes 1300 and the tip passes 1297.
Rows already held at the tip: row 3 (M3), and the no-pin half of row 4 (M4b). I did not change the tests that hold them. The transfer test I re-aimed for M4a also covers M3, so M3 now fails two ids.
What changed in the tests. Each assertion asserts the clause itself, not a substring that also sits inside a stanza:
endswith("are authored for different machines, 'node-18' and 'node-22'").startswith("'qwen3-151k-bpe' is 'sha256:aaa…' in 'first' ("), plus") and 'sha256:bbb…' in 'second' (".endswith("carried constants over from 'mi300x-8gpu', measured against rocm '7.0.2', into a spec pinned to rocm '7.2.4'").startswith("`device.clock_ceiling` is not a field, a block of fields or a quantity").P7 and P9
Probed on node 18 at the tip and at the head. The probe script builds the same fixtures the tests import.
provenance.fragments: []and no pin got the saved-transfer text ("its provenance names the fragments an earlier merge built it from"). At the head it gets the first-hand text, byte for byte:…carried constants over from 'mi300x-8gpu' without saying which stack they were measured against. The field reads back as(), so.get("provenance.fragments")is tested for truth.t0.yaml, then merge that alone and save it ast1.yaml. The provenance oft1.yamllists('tier2', 't0.yaml'), and re-merging it is refused.merge the fragments it was built from in its place, since a transfer states its source's stack pin there and nowhere else.5e272ffe4:merge the fragments it was built from ('tier2', 't0.yaml') in its place, since a transfer states its source's stack pin there and nowhere else.ab574e51bappends one clause:; any of them that is itself a saved document with no pin is refused the same way.t0.yaml, now with('tier2')in its remedy. Following it withtier2alone givesok=True. So the saved-twice case still takes two rounds. The difference is that the first remedy now names the saved document the reader has to look past.Refusal text: tip against head (round 1,
5e272ffe4)This uses #338's reviewer's recorder (
rec_refusals.py, unmodified) overtests/compass/test_spec_schema.pyandtest_spec_verbs.py, at86d70df49and at5e272ffe4.validaterecordsSpecRefusals builtUnder the tip's node ids, the records that differ:
One P9 remedy. In
test_a_saved_transfer_merged_again_names_the_merge_that_dropped_its_pin:merge the fragments it was built from in its place, …merge the fragments it was built from ('tier2') in its place, …The
validaterecord of the same call differs only in that remedy, and in its renderedstr.Two records in
test_a_host_that_publishes_no_topology_is_refused_rather_than_halved. They differ only in pytest's per-session tmp directory,pytest-5381againstpytest-5382, inside the path the refusal quotes. The template and the rest of the text are identical.Every other refusal and
validaterecord under the tip's ids is identical. That is 95 of 96validaterecords and 314 of 317 refusals. The new ids' records are the P7 first-hand text (twice, since the test builds the stated case and its control), the P9 remedy, and themergestack conflict.Tripwires
A tripwire is
exec('raise SystemExit(97)')spliced into the branch. It usesexecbecause a literalraise SystemExitalso tripstest_spec_schema.py::test_every_refusal_site_in_the_package_resolves_to_a_rule, a static scan of raise sites. Atabb70ef48the literal form of the_suppliedtripwire failed exactly that one test and nothing else, so that failure says nothing about whether the branch was entered.merge's_conflictstack armabb70ef48test_two_fragments_pinned_to_different_stacks_are_refused_as_a_stack_conflictRule.ONE_MACHINE if stack else Rule.ONE_MACHINE86d70df49_supplied's fall-throughreturn ()86d70df49_supplied'sorigin is NonereturnPINNED_STACKinmerge: now entered. The new test mergesTIER1with a copy pinned to rocm7.3.0and asserts the rule. The refusal it produces:`device.software_pinned_to.rocm` is '7.2.4' in 'tier1' (…) and '7.3.0' in 'tier1b' (…), with the stack remedy._supplied's fall-through: still unreached. No test added. A test would have to explain a path that aMergerecords no source for (the provenance block, orname) and pinsupplied_by == ()for it. Whether "no fragment" is the right answer for a blockmergerebuilds from every fragment is a question aboutexplain, which is outside this file set. I have not settled it here.Gate 1, round 1 (node 18,
xiaobizh_n18_cpu,5e272ffe4)Staging used
git archive, with.compass-commitand.compass-changedwritten from the samerev-parse, piped throughdocker exec -i … tar -xinto/tmp/i251. The tarball md5 matched on both ends. The shared mount was not touched.git merge-tree --write-tree 86d70df49 5e272ffe4→eac6bdc688f5d49848c270d01a93044b7869b1c4, rc=0. This equals5e272ffe4^{tree}.git commit-treegave2bd196a65, with parents the tip and the head.scripts/compass/gate_cpu.sh.scripts/compassiscae1d491bon both sides. Each run was bounded bytimeout -k 10 2400, captured to a file, unpiped, and run one at a time.commit:atom:GATE_CPU_RC86d70df4986d70df49 (stamp)/tmp/i251/stage/tip/ATOM/atom/__init__.py2bd196a65(treeeac6bdc68)2bd196a65 (stamp)/tmp/i251/stage/merged/ATOM/atom/__init__.pyNode-id delta (junit). 5411 cases on the control and 5414 merged. The 3 cases only in merged are the new tests, all passed. No case is only in the control, and no outcome changed. No timing-class flake occurred, so nothing needed a re-run.
Lint. For the two files,
ruff checkgaveRUFF_RC=0andblack --checkgaveBLACK_RC=0, at both the tip and the head (ruff 0.16.7, black 26.5.1). The second commit exists because the first head failedISC004on the new P9 assertion.Size
atom/compass/spec/validate.py)tests/compass/test_spec_verbs.py)These are
git diff --numstat 86d70df49 ab574e51b. Round 2 alone is production +3 / −1 and tests +20 / −23. The estimate was 20–40 lines. The total is 57 added, of which the production change is 7. The extra over the estimate is themergestack-arm test (8 lines) and the P7 test, a separate node id so that its fix fails by its own name. The P9 case started as a separate test too, and round 2 folded it into the saved-transfer test.Dev record
_suppliedtripwire, in the literalraiseform compass(spec): merge, validate and explain over the machine spec (SPEC-2) #86's audit used, is not silent any more. It fails one static test for a reason unrelated to reachability. Theexecform separates the two._supplied's fall-through remains unreached (above).🤖 Generated with Claude Code