compass(spec): a re-merged saved transfer names the merge that dropped its pin - #338
Conversation
…d its pin A document saved from a merge of transfers from one spec reads back as a transfer fragment with no stack pin, because a merge keeps a transfer's pin out of what it writes. Re-merged, it was refused PINNED_STACK with the text for a transfer whose author gave no stack. The verdict is right; the cause named was not. Where the transfer fragment states provenance.fragments, which only a merge writes, the refusal now says an earlier merge built it and kept the pin out, and points at merging the fragments it was built from. The verdict and every other refusal text are unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
| for component in PINNED | ||
| if f"{PINNED_BLOCK}.{component}" in fragment.values | ||
| } | ||
| if not declared and "provenance.fragments" in fragment.values: |
There was a problem hiding this comment.
Non-blocking (principle 8, "Every claim carries its measurement"). "provenance.fragments" in fragment.values is also true for a fragment that states provenance.fragments: []. The schema accepts that value, and such a fragment gets the new wording, "its provenance names the fragments an earlier merge built it from", even though its provenance names none.
Measured on node 18 (xiaobizh_n18_cpu, merged stamp tree af99b9eb6), probe P7: a first-hand transferred-from:mi300x-8gpu fragment with no pin and fragments=[], merged with tier0, tier1 and links, is refused PINNED_STACK with the new text. At the tip it gets the first-hand text.
merge never writes an empty list, because _provenance adds every fragment's own source. So an empty list is always hand-written, and the author omitted the pin. if not declared and fragment.values.get("provenance.fragments"): is the same length and sends that case back to the first-hand wording. The verdict does not change either way.
The non-empty hand-written case (probe P5) also gets the new text. I accept that as honest: the document claims its own merge history, no field can tell it apart from a real merge output, and the PR body says so.
| "fragments an earlier merge built it from; a merge keeps a " | ||
| "transfer's source stack pin out of the document it writes, so " | ||
| "this one cannot say which stack they were measured against", | ||
| "merge the fragments it was built from in its place, since a " |
There was a problem hiding this comment.
Non-blocking suggestion (principle 6, "A declined answer with a named reason is a result"). The remedy says to merge "the fragments it was built from" but does not name them. fragment.values["provenance.fragments"] already holds the names, so the refusal could list them.
This matters most for a document saved twice. Probe P9 (merge a pinned transfer, save it as t0.yaml, merge that alone, save it as t1.yaml) gives provenance.fragments == ('tier2', 't0.yaml'). Only tier2 carries a pin. If you follow the remedy literally with both names, t0.yaml is refused again with the same wording, and it takes a second round to converge (probe_extra, below). Naming the list would at least show the reader which names are candidates.
I checked the remedy's other sentences against the code, and they are true:
merge.py:285skipsdevice.software_pinned_to.*for any fragment withtransferred_from.fields.py:127-129are the only pin fields in the schema.- So the source fragment is the only place the pin survives.
Review cycle 1: PR #338 (issue #334). Verdict: APPROVE at head
|
| tree | validate.py lines |
spec suite | new node id |
|---|---|---|---|
head e56ecb103 |
448 | 294 passed, rc 0 | passed |
merged stamp (tree af99b9eb6) |
448 | 294 passed, rc 0 | passed |
mutant A: merged with the tip's validate.py (cmp-identical to it, the only file that differs) |
433 | 1 failed, 293 passed, rc 1 | failed |
mutant B: merged with the condition cut to if not declared:, line count kept |
448 | 2 failed, 292 passed, rc 1 | failed |
- Mutant A's assertion:
assert 'fragments an earlier merge built it from' in "'t.yaml' (...) carried constants over from 'mi300x-8gpu' without saying which stack they were measured against". - Mutant B shows the control is live. The new test's exact-string first-hand assertion fails, with
+ ..., and its provenance names the fragments an earlier merge built it from; .... So doestest_a_transfer_that_names_no_stack_at_all_is_refused. The pin holds the trigger as well as the text. - The first-hand control text is unchanged. Its refusal list has md5
257a1f7a…in the new test's secondvalidatecall and in the tip'stest_a_transfer_that_names_no_stack_at_all_is_refused.
2. Text diff, re-run with my own recorder
The recorder (agent_scratch/compass_dev/pr338-review1/tools/rec_refusals.py) records two things:
- every
validate()result: rule,what,remedy,stack_differences,not_asked,asked_in_partandstr(); - every
SpecRefusalconstructed anywhere, which also covers merge and read refusals.
After collection it sweeps sys.modules for any leftover binding to the unwrapped validate, and found 0. It ran over tests/compass/test_spec_*.py at tip 196fd3711 and at the merged stamp.
| tip | merged | |
|---|---|---|
| node ids | 293 passed | 294 passed |
validate calls / refusals in them |
94 / 119 | 96 / 121 |
SpecRefusals constructed |
315 | 317 |
- Outcome diff: one line,
> PASSED …names_the_merge_that_dropped_its_pin. validaterecords outside the new test:cmp-identical, md5d9de1575…on both sides. The full diff is the new test's two added records and nothing else.SpecRefusalrecords outside the new test: identical once pytest's own tmp counter (pytest-5258vspytest-5259) is normalised intest_a_host_that_publishes_no_topology_is_refused_rather_than_halved, which embedstmp_path. That is 315 records.- Result: only the new case's text changed. The developer's "94 byte-identical" claim holds.
3. Trigger condition (principle 6: "Refuse rather than fall back")
I ran probes (probe338.py) at the tip and at the merged tree. Each re-merge is with tier0, tier1 and links.
| probe | subject | tip text | head text | verdict |
|---|---|---|---|---|
| P1 | pinned transfer, merged alone, saved, re-merged | first-hand | new | PINNED_STACK, both sides |
| P3 | same, source pinned to rocm 7.0.2 | first-hand | new | PINNED_STACK |
| P4 | same, source never pinned | first-hand | new | PINNED_STACK |
| P9 | saved twice (provenance.fragments == ('tier2','t0.yaml')) |
first-hand | new | PINNED_STACK |
| P10s | saved merge of two same-source transfers pinned to different rocm | first-hand | new | PINNED_STACK |
| P5 | hand-written first-hand transfer, no pin, fragments=['tier2-by-hand'] |
first-hand | new | PINNED_STACK |
| P7 | hand-written transfer, no pin, fragments=[] |
first-hand | new (claim false) | PINNED_STACK |
| P6 | hand-written, fragments stated and pinned |
ok | ok | goes the declared way |
| P8 | saved mixed merge (transfer plus probed), re-merged | ok, transfers=[] |
same | never reaches _transfers |
| P11 | P1's re-merge validated as a document | ok, TRANSFERS in not_asked |
same | _transfers not called |
- Is "no pin, and
provenance.fragmentsstated" exactly the re-merged case? For every documentmergewrites, yes._provenancealways writesfragments, and nothing else inatom/does (git grep).- A saved merge is a transfer only when every input shared one
transferred-from:Xmethod. merge.py:285then drops every input's pin. So such a document is pinless, always.
- Can a hand-written fragment get the new text? Yes, P5 and P7.
- P5 is honest. The document itself claims a merge history, and no field can tell it apart from a real merge output. The PR body discloses it. Accepted.
- P7 is not. An empty list names no fragments, yet the text says it does.
mergenever writes[]. This is non-blocking, inline on L364.
- Does a
mixedre-merge reach this branch? No.Merge.transfersfilters on thetransferred-from:prefix, and amixeddocument is not in it (P8,transfers=[]). compass(spec): a saved document merged again does not claim the transfer was asked #333'snot_asked/asked_in_partwording covers that case, not this branch.
4. Truth of each sentence (principle 8: "Every claim carries its measurement")
- "A merge keeps a transfer's source stack pin out of the document it writes": true.
merge.py:285hasif path.startswith(PINNED_BLOCK) and fragment.transferred_from: continue.- P1's saved document has no
device.software_pinned_to.*path, although its source was pinned.
- "A transfer states its source's stack pin there and nowhere else": true.
- The schema's only pin fields are
device.software_pinned_to.{rocm,aiter,rccl}(fields.py:127-129). mergedrops them for transfers, so only the source fragment still carries one.- When the source never had one (P4), the sentence overstates, but following the remedy yields the first-hand refusal, which names the true cause.
- The schema's only pin fields are
- "cannot say which stack": true. It does not claim a pin existed, which is right for P4.
- Docstring sentence: true. A merge of one-spec transfers keeps
transferred-from:X(_provenance: one method means that method), and P1 reads back as a pinless transfer. - Does the remedy work? Yes, measured. Merging the source fragments in place of the saved document gives:
- P1 → P2:
ok=True. The refusal is gone. - P3 → P3r: refused
PINNED_STACKfor a different stated reason, "measured against rocm '7.0.2', into a spec pinned to rocm '7.2.4'". The saved document had been hiding a real cross-stack transfer. - P10s → P10r: the same, naming
tier2b. - P4 → P4r: the first-hand text, now naming the right cause.
- P9 (saved twice): with both listed names in place,
t0.yamlis refused again with the new text. Withtier2alone,ok=True. It converges in two rounds. Naming the list would help (inline on L372).
- P1 → P2:
5. ponytail-review over the diff
I read the raw file. Considered and not flagged:
- Folding the new wording into the existing
if not declared:block. It saves about 3 lines (oneyield SpecRefusal(,Rule.PINNED_STACK,,continue), but it rewrites the first-hand refusal whose bytes the brief pins. - The test's exact-string control. It is the brief's required first-hand check, and mutant B shows it fires.
- The 4-line docstring sentence. It carries the module's reasoning for the branch.
Lean already. Ship.
6. Gate (merged tree, node 18)
- Tip read again: after a fetch,
fork/feature/atomcompass_newis196fd3711e8a213763c02a3bff301f8895d048a3, unmoved. - Merged tree:
git merge-tree --write-tree 196fd3711 e56ecb103givesaf99b9eb61da57078f4e24ceeb4eea7324299001, with no conflict. This is the developer's hash.git diff --stat 196fd3711 <stamp>is the PR's two files, +37/−1. - Stamp:
git commit-treegaveb0c4a16abd611f5831d919a63781bef4b9b9b69d, with parents the tip and the head..compass-changedholds the two PR files. - Staging:
git archiveplusdocker exec -i … tar -xinto/tmp/r338. The tarball md598a547e0…matched on both ends. The shared mount was not touched. - Gate script: the tree's own
scripts/compass/gate_cpu.sh.scripts/compassiscae1d491bat both the tip and the merged tree. - Printed stamps: the gate printed
commit: b0c4a16ab (stamp)andatom: /tmp/r338/stage/merged/ATOM/atom/__init__.py. My wrapper had already asserted that. - Run: one run,
timeout -k 10 2400, unpiped, load 13.9 → 8.2.
| tree | passed | skipped | xfailed | junit cases | GATE_CPU_RC |
|---|---|---|---|---|---|
merged b0c4a16ab (tree af99b9eb6) |
5245 | 155 | 3 | 5403 | 0 |
- Against the developer: this matches their merged run (5245/155/3, 5403, rc 0). Their tip control is 5244, so the difference is +1, the new node id, which is
passedin my junit. - Timing classes: all 17 cases of
TestTheRegionIsNotCopiedPerChunk,TestNoSizeAtWhichACallStopsBeingOne(both[minimax]cases included) andtest_freezing_twice_is_additive_and_harmlesspassed, so none needed a re-run. - Ruff: 0.16.7 on node 18.
ruff checkandruff format --checkon both files give rc 0. - Design-doc references: a grep for
#NNN,D<n>, "principle N", "Gate N" andP0.xover both files at the merged tree finds none (rc 1). That meets the rule's check at the head over the whole file set. - Lines: +37 against a 10–20 estimate, under the 2x line.
Verdict: APPROVE at e56ecb1037085f8f56c5232ee58370b838f7e770. Both findings are non-blocking.
Closes #334
Agent-authored (developer). Principles read first:
atom/compass/design/README.md(all eight) andatom/compass/AI_DEV_RULES.mdat tipfeb1b27a2.What changed
A merge whose fragments all state
transferred-from:Xwrites a document with that same method and nodevice.software_pinned_to, becausemergeskips every pin path on a transfer fragment. Saved and merged again withprobedfragments, that document is a transfer fragment with no pin, and_transfersrefused itPINNED_STACKwith the text for a transfer whose author gave no stack. The verdict is right. The cause it named was not: in the new test the transfer was pinned, to this stack._transfersinatom/compass/spec/validate.pynow picks a second wording when a pinless transfer fragment statesprovenance.fragments. In this package onlymergewrites that field. The wording names the earlier merge as the reason no pin is present, and the remedy points at merging the fragments it was built from.rules.pyholds only the rule name, so nothing changed there. The verdict is unchanged: the new branch yields the sameRule.PINNED_STACKat the same place, and at the tip the new test's rule assertion passes and its text assertion fails.The refusal text, tip against head, for the re-merged saved transfer
Named result (node 18,
xiaobizh_n18_cpu)New node id:
tests/compass/test_spec_verbs.py::test_a_saved_transfer_merged_again_names_the_merge_that_dropped_its_pin."Spec suite" is
tests/compass/test_spec_schema.pyplustests/compass/test_spec_verbs.py.validate.pylinesce7cedc3c)validate.pyput back (cmp-identical to the tip's; the only file that differs from head)assert 'fragments an earlier merge built it from' in "'t.yaml' (...) carried constants over from 'mi300x-8gpu' without saying which stack they were measured against"The same test holds the control: a first-hand transfer with no pin keeps today's text, asserted as an exact string.
Every other refusal text is byte-identical
A pytest plugin wrapped
validateand wrote one JSON line per call, per node id:ok, each refusal's rule,whatandremedy,stack_differences,not_asked,asked_in_partandstr(). I ran it overtests/compass/test_spec_*.pyat the tip196fd3711and at the merged stamp.validatecalls recorded.> PASSEDfor the new node id.cmpidentical to the tip's.test_a_transfer_that_names_no_stack_at_all_is_refused. Its first call is the changed text above.Gate 1: ATOM's suite, unmodified, as a delta
The tip moved twice while I gated (
feb1b27a2tof9edfcf11for #327, then to196fd3711for #337). Neither touched this PR's files. I re-staged and re-gated at each tip; these are the numbers at the current one.git archiveplusdocker exec -i … tar -xinto/tmp/i334insidexiaobizh_n18_cpu. The tarball md5588d45e8bea8572f7d13f3e25b0f6896matched on both ends. The shared mount was not touched.git merge-tree --write-tree 196fd3711 e56ecb103givesaf99b9eb61da57078f4e24ceeb4eea7324299001, with no conflict. It is note56ecb103^{tree}(6344e06c9), because it also carries compass(gates): join or stop restating the counts scripts/compass/ repeats #327 and compass(tests): check the reply-scan reader at the constant it runs at #337.git commit-treegavece7cedc3cb3799622f5a1a1db2c2bdc49ea12191, with parents the tip and the head, and treeaf99b9eb6..compass-changedholds the two PR files.scripts/compass/gate_cpu.sh.scripts/compassiscae1d491bat both.atom.__file__resolves under its staged root before the gate started, and the gate printed the same path and the stamp.timeout -k 10 2400, unpiped.GATE_CPU_RC196fd3711(control)ce7cedc3ctest_a_saved_transfer_merged_again_names_the_merge_that_dropped_its_pin passed. No existing node id changed outcome.TestTheRegionIsNotCopiedPerChunk,TestNoSizeAtWhichACallStopsBeingOneandtests/test_gc_utils.py::test_freezing_twice_is_additive_and_harmlesspassed on both sides, so none needed a re-run.feb1b27a2(5239 to 5240) and atf9edfcf11(5243 to 5244).Lines
git diff --numstatagainst the tip:validate.py+16/−1. That is +12 code, the new branch in_transfers, and a docstring sentence (+4/−1) saying a saved merge of transfers reads back as a pinless transfer and is refused naming the merge.test_spec_verbs.py+21/−0.That makes 37 added lines against the brief's estimate of 10–20, which is under the 2x line.
Ruff:
ruff checkandruff format --checkon both files give rc 0 (ruff 0.16.7, node 18). The two files carry no#NNN,D<n>, "principle N" or "Gate N" references (grep -nErc 1).Decided, not in the brief
provenance.fragments, not the method. The brief asks for "where the fragment's provenance shows it was itself a merge"._provenanceinmerge.pyalways writesfragments, and nothing else in the package does. A first-hand fragment could state it by hand; its own provenance then claims a merge, and the text says only that the provenance names one.Left undone
Nothing in the brief. Whether a saved merge should carry its transfers' source pins, so it could be asked rather than refused, needs a new schema field (
fields.py). #333's cycle-1 review recorded this, and it is out of this file set.🤖 Generated with Claude Code