Repository navigation
compass(memory): drive a decode shape against a prefill one at both refusal sites - #283
Conversation
…efusal sites Shape agreement in compare() is dataclass equality over tokens and phase. Every Shape the comparator tests built was a prefill shape, so the phase half of that equality was never exercised: comparing tokens only left the file green. The new test builds a decode recording at the prediction's own token count, so only the phase separates the two sides. It is parametrised over the two places that read agreement: the per-term refusal, where a shaped term is named, and the outright MemoryRefusal, where neither side names one. Each parameter asserts which refusal fired and that its text names both phases. Closes #217 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
| ) | ||
| decode = Recorded( | ||
| run="a decode step at the prediction's token count", | ||
| shape=Shape(tokens=HISTORICAL_SHAPE.tokens, phase="decode"), |
There was a problem hiding this comment.
Non-blocking (principle 8). This line pins prefill differs from decode. It does not pin the other half of the claim in Shape's docstring: "Equality is exact, so two spellings of one phase refuse each other." #176's audit row states that claim as its property.
Measured on node 18, against the merged tree 66614e4bd and the tip 37824643b. I compared phase case-insensitively at both sites, (tokens, phase.lower()) at compare.py:587 and at :607. The mutation keeps the file at 847 lines.
- merged tree: 53 passed, which is what the unmutated file gives;
- tip: 51 passed, also unchanged.
So a comparator that treated "Prefill" and "prefill" as one phase stays green. The docstring calls that direction unsafe. "prefill" against "decode" still differs after casefolding, so this test cannot see the change, and no other test can either.
This does not block the PR. #217's named result is decode against prefill, and that is met: every mutation that removes the phase distinction goes red. Adding a third parameter would close it in about five lines: the same token count, with phase="Prefill" on the recorded side. That could be done here, or in whichever follow-up next touches Shape, with #176's tokens < 1 and blank-phase row.
Review, cycle 1: APPROVE at head
|
mutation to compare.py |
merged tree 66614e4bd |
tip 37824643b |
|---|---|---|
| none | 53 passed | 51 passed |
null control: :587 != → not … == |
53 passed | 51 passed |
tokens only at both sites (:587, :607) |
2 failed: [per-term], [outright] |
51 passed |
tokens only at :587 (outright) |
1 failed: [outright] |
51 passed |
tokens only at :607 (per-term) |
1 failed: [per-term] |
51 passed |
phase: str = field(compare=False) (+ field import) |
2 failed: both | 51 passed |
mine: Shape.__post_init__ overwrites every phase with "prefill" (phase erased from the type) |
2 failed: both | 51 passed |
mine: Shape.__str__ drops the phase |
4 failed: both new parameters, plus test_a_pair_taken_at_two_shapes_refuses_and_names_both and test_the_table_states_the_shape_of_each_side |
2 failed: the two existing tests only |
| mine: phase compared case-insensitively at both sites | 53 passed: survives | 51 passed |
The node ids are tests/compass/test_memory_compare.py::test_a_decode_side_refuses_a_prefill_side_at_the_same_token_count[per-term] and …[outright].
- The developer's table reproduces exactly on the tree that will land. Every phase-erasing mutant goes red at the head and stays green at the tip. That is the blind spot, now closed.
- The case-insensitive survivor is the inline finding. It is a second claim in
Shape's docstring, "two spellings of one phase refuse each other", and this test does not reach it. It does not affect compass(memory): a phase disagreement is stated to refuse and is never measured #217's named result. Principle 8.
2. Is the two-way parametrisation needed? Yes, measured.
Each partial mutant reddens only its own parameter:
- tokens-only at
:587fails[outright]alone, 1 failed / 52 passed; - tokens-only at
:607fails[per-term]alone, 1 failed / 52 passed.
A test that drove only one site would pass the other site's partial. The parametrisation is load-bearing, which is the same shape as F2 in #176's audit.
3. Property or incidental text? The property (principle 8)
Each parameter first pins which refusal fired, by its structure:
[outright]pins it by exception type:pytest.raises(MemoryRefusal).[per-term]pins it by the refused set,== {"activations"}, and by the compared set,== {"weights", UNATTRIBUTED}. That second assertion also catches a refusal that spreads beyond the shaped term.
The text needles come second:
taken at different shapes -- … --occurs only in the outright template, atcompare.py:588-590.the two disagree --occurs only in_refuse_shape, at:495-496.
The high-water refusals share neither string. Both needles embed str(Shape) for both sides. The __str__ mutant above confirms that the needles check that the refusal names both phases, not just that some refusal fired. The literal 4096 is coupled to HISTORICAL_SHAPE, but a drift there fails loudly rather than silently. That is acceptable.
4. The six open blind spots: the follow-up ruling
File the three "no default" rows as one follow-up issue now. Those rows are SummedCheck.band, Comparison.summed's band, and from_readings' at_shape. The coordinator files it; I have not.
They share:
- one mechanism: a default is added to a signature and no test notices;
- one claim: compass: hold each non-KV memory term to its own gate, never a sum (MEM-3) #176's decision that a default here is the substitution the incident is made of;
- one test shape: a parametrised
TypeError-on-omission, one parameter per signature, about 15–20 lines; - one file set:
compare.py+tests/compass/test_memory_compare.py.
Reinstating each default was measured as 50 → 50 in #176's audit. So each one is a stated refusal with no measurement, under principles 6 and 8. The one existing test, test_a_comparison_has_no_total_and_a_sum_cannot_be_built_without_one, asserts only the field set, which a default does not change.
The other three stay open as the developer argued, at low severity:
Predicted's unknown-at_shapeguard;Shape'stokens < 1and blank-phase guards;compare's gate range and_named's duplicate guard.
I would add the case-insensitive survivor above to the Shape row, so that whichever cut next touches Shape closes both.
5. No design-doc references
The 33 added lines contain no doc numbers, principle citations, gate labels or issue numbers. The test comment says what the test does. It complies with AI_DEV_RULES.
Gate: the merged tree, gated once with its own scripts/compass/gate_cpu.sh
- Stamps:
.compass-commit=66614e4bd…, the merged tree object, since it has no commit;.compass-changed=tests/compass/test_memory_compare.py.
- Run:
timeout -k 10 2400, not piped, with output redirected to a file.
tree: /tmp/pr283rev/merged/ATOM
atom: /tmp/pr283rev/merged/ATOM/atom/__init__.py
commit: 66614e4bd (stamp)
gate: 29 files excluded + tests/plugin
gpu: not required (.compass-changed stamp)
5157 passed, 155 skipped, 3 xfailed, 16 warnings in 173.22s (0:02:53)
pytest: rc=0
GATE_CPU_RC=0 PASSED
5157 = the tip's 5155 + 2, which are the two new parameters. Skipped and xfailed match the developer's control, 155 and 3. No flaky test fired: neither TestTheRegionIsNotCopiedPerChunk, nor TestNoSizeAtWhichACallStopsBeingOne, nor test_gc_utils. The tip's 5155 is the coordinator's figure; I did not re-measure it here.
Verdict
APPROVE, head f09135f4a24f1022a5660f01e3d672c262c57372. There are no blocking issues.
I did not change the PR state: it remains a draft, with no labels, and was not merged.
Closes #217
What this does
Shape's docstring says that a phase disagreement refuses. Before this PR, no test built a decode shape, so nothing measured that claim. This PR adds one test with two parameters and changes no production code.test_a_decode_side_refuses_a_prefill_side_at_the_same_token_countbuilds adecoderecording at the prediction's own token count, 4096. Only the phase separates the two sides. The test is parametrised over the two places incompare()that read shape agreement:[per-term]shapes_agree(per-term guard);at_shape = {"activations"}activationsis refused; itswhatcontainsthe two disagree -- predicted at 4096 tokens, prefill, recorded at 4096 tokens, decode;weightsand the unattributed term are still compared[outright]MemoryRefusalraised when neither side names a shaped termMemoryRefusal, and its text containstaken at different shapes -- predicted at 4096 tokens, prefill, recorded at 4096 tokens, decode --Each needle is a substring that only its own refusal template produces. The per-term template says
the two disagree --and the outright one saystaken at different shapes --. A mutant that fired the other refusal would therefore not satisfy the assertion.Named result: the refusal exists, fires, and is now measured
It is not a production defect. At the unmutated head both parameters pass, so the refusal fires. Dataclass equality over
(tokens, phase)is doing the work.Measured on node 18 (
xiaobizh_n18_cpu). The trees were staged bygit archiveinto a private path, andatom.__file__was asserted under each staged root before any count was read. Each run istests/compass/test_memory_compare.pyalone, with__pycache__cleared before it. Every mutation keepscompare.pyat 847 lines.atom/compass/memory/compare.pyf09135f4a75a265a3fpredicted.shape != recorded.shape→not predicted.shape == recorded.shape(equivalent)[per-term],[outright][outright]only[per-term]onlyShape.phaseexcluded from equality:phase: str = field(compare=False), withfieldadded to the existing importFailing node ids, all at head:
tests/compass/test_memory_compare.py::test_a_decode_side_refuses_a_prefill_side_at_the_same_token_count[per-term]: fails withassert set(refused) == {"activations"}tests/compass/test_memory_compare.py::test_a_decode_side_refuses_a_prefill_side_at_the_same_token_count[outright]: fails withDID NOT RAISE MemoryRefusalThe parametrisation is load-bearing. Each realistic partial reddens exactly one parameter, the one at the site it disabled. A test that drove only one site would have passed the other partial.
The #176 audit counted 50 at
714bd1c02. The tip's count here is 51, re-measured rather than cited; the difference is what has landed since.Gates
Gate 1:
scripts/compass/gate_cpu.shfrom each tree's own copy, with.compass-commitand.compass-changedstamps. The runs were sequential and unpiped, each undertimeout -k 10 1500.75a265a3f(control)f09135f4aGATE_CPU_RCcommit:stamp75a265a3f (stamp)f09135f4a (stamp)The node-id delta comes from
gate_cpu.sh --collect-only, which collects 5283 at the tip and 5285 at the head. The head adds 2 node ids and removes none. They are the two parameters above. No flaky test fired on either side.Lines: 0 production, +33 test, in
tests/compass/test_memory_compare.py.git merge-tree --write-tree 75a265a3f f09135f4a→d1af5ad492a0468b0bbfa08c363322f293694b3a, rc 0. That equals the head's own tree, because the head sits directly on the tip.Gate 2: the new test is CPU-only and uses the file's existing
no_device_readingsautouse fixture.Gate 3: the named result, above.
Gate 4: an independent reviewer, dispatched by the coordinator.
The staging on node 18 has been removed, and the shared mount was not touched.
Estimate against actual
The estimate was about 30 test lines and 0 production lines, and the actual is 33 test lines and 0 production lines. That is within the estimate.
The six other blind spots from the #176 audit: all left open
The coordinator bounded this task to one test plus what that edit's own shape needs, so none of the six is closed here.
SummedCheck.bandhas no defaultTypeError-per-signature test.Comparison.summed's band has no defaultfrom_readings'at_shapeis requiredPredicted's unknown-at_shapeguard (itsRecordedmirror is pinned)Shaperefusestokens < 1and a blank phaseShapeis touched again.comparerefuses a gate outside(0, 1);_namedrefuses a duplicate termMy suggestion is that the three "no default" rows become one follow-up task. That would be about one parametrised test, some 15–20 lines.
Nothing surprised
The refusal was there and fired, as the code implies. The gap was purely in measurement. There are no blocking issues.
🤖 Generated with Claude Code