Skip to content

compass(memory): drive two spellings of one phase at both shape checks - #298

Merged
jgong5 merged 1 commit into
feature/atomcompass_newfrom
compass/issue-296-phase-spelling
Sep 23, 2026
Merged

jgong5 merged 1 commit into
feature/atomcompass_newfrom
compass/issue-296-phase-spelling

Conversation

@jgong5

@jgong5 jgong5 commented Sep 23, 2026

Copy link
Copy Markdown
Owner

Closes #296

What changed

test_a_decode_side_refuses_a_prefill_side_at_the_same_token_count gains a second parameter axis, the phase pair: ("prefill", "decode"), as before, and ("decode", "Decode"), two spellings of one phase. Both run against both shape checks in compare(): the outright refusal (at_shape empty) and the per-term refusal (at_shape names activations). atom/compass/memory/compare.py is unchanged.

Lines: production 0; test +13 / -5, all in tests/compass/test_memory_compare.py. The estimate was about 8.

Decision: phases are case-sensitive by design

  • The docstring says so. Shape says: "Equality is exact, so two spellings of one phase refuse each other, which is the safe direction."
  • The code agrees. Shape is a frozen dataclass compared with == at both sites (predicted.shape != recorded.shape for the outright refusal, shapes_agree = predicted.shape == recorded.shape for the per-term one). __post_init__ checks only that the phase is non-blank. Nothing normalises it.
  • No production code builds a memory.Shape. Every phase value in the tree is a literal in tests/compass/test_memory_compare.py: "prefill" and "decode", all lower case. (atom/compass/audit/sync_scan.py has its own, unrelated Shape class.) The phase comes from callers, so this module cannot know which spellings they mean.
  • Two spellings are plausible. ATOM's own SequenceType members are PREFILL and DECODE. A caller that writes .name gets "DECODE", and one that writes a literal gets "decode".
  • Normalising would be a guess. Folding case would decide that two strings name one phase, which is the fallback the package refuses. The docstring already records that exact equality errs toward refusal.

So this PR takes the "yes" branch of the brief: it adds a test and leaves the docstring and code alone.

Named result

Each mutation is line-count-preserving (847 lines before and after), applied to a copy of each staged tree on node 18 (xiaobizh_n18_cpu). Only tests/compass/test_memory_compare.py was run.

mutation tip 6a83b56bc head ad9a1c5e0
outright site: (tokens, phase.lower()) on both sides of != 57 passed 1 failed, 58 passed: [two-spellings-outright], DID NOT RAISE MemoryRefusal
per-term site: (tokens, phase.lower()) on both sides of == 57 passed 1 failed, 58 passed: [two-spellings-per-term], assert set() == {'activations'}
null control: operands swapped at both sites 57 passed 59 passed

Failing node ids:

  • tests/compass/test_memory_compare.py::test_a_decode_side_refuses_a_prefill_side_at_the_same_token_count[two-spellings-outright]
  • tests/compass/test_memory_compare.py::test_a_decode_side_refuses_a_prefill_side_at_the_same_token_count[two-spellings-per-term]

Gates

Both runs were on node 18 (xiaobizh_n18_cpu) using the tree's own scripts/compass/gate_cpu.sh. Each tree was a git archive with .compass-commit and .compass-changed stamps, staged at /tmp/i296gates/{tip,head}/ATOM. atom.__file__ resolved under each staged root.

tip 6a83b56bc (control) head ad9a1c5e0
result 5166 passed, 155 skipped, 3 xfailed 5168 passed, 155 skipped, 3 xfailed
GATE_CPU_RC 0 0

The node-id delta from junit comes to 5324 on the tip and 5326 at the head. Every changed id is in this test:

  • removed: [outright], [per-term]
  • added: [prefill-decode-outright], [prefill-decode-per-term], [two-spellings-outright], [two-spellings-per-term]

No other test changed status.

The merged tree: git merge-tree --write-tree 6a83b56bc ad9a1c5e0 gives 81414803cbbd3c9944c3f95a76c405cdbba3c139, which equals ad9a1c5e0^{tree}.

ruff format --check and ruff check pass on the test file.

Left undone

Nothing. The two existing parameter ids were renamed, from [outright] and [per-term] to [prefill-decode-*]. Anything that selects them by id needs the new names. Nothing in the tree does.

🤖 Generated with Claude Code

Shape's docstring says phases compare exactly, so two spellings of one
phase refuse each other. The only test of a phase difference drove
decode against prefill, which differ in content, so a case-insensitive
phase comparison at either shape check in compare() left the suite
green.

Parametrize that test over a second phase pair, "decode" against
"Decode", at both the outright refusal and the per-term refusal.
compare.py is unchanged: phases stay case-sensitive.

Closes #296

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
def test_a_decode_side_refuses_a_prefill_side_at_the_same_token_count(at_shape):
@pytest.mark.parametrize(
"mine, theirs",
[("prefill", "decode"), ("decode", "Decode")],

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking (principle 8, evidence matches the claim). The dev record's evidence for two spellings is ATOM's SequenceType, whose .name is "DECODE" (all upper case, atom/model_engine/sequence.py, members are auto(), so .value is an int and .name is the only string). This pair uses "Decode", which nothing in the tree produces. Every case-insensitive mechanism I tried catches both, so this does not weaken the pin, but ("decode", "DECODE") would drive the exact pair the record names as plausible. Optional.

[("prefill", "decode"), ("decode", "Decode")],
ids=["prefill-decode", "two-spellings"],
)
def test_a_decode_side_refuses_a_prefill_side_at_the_same_token_count(

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking (principle 4). The test name now overstates two of its four cases: [two-spellings-*] has no prefill side, and the recorded side's run= label and the local name decode still say "decode step" when theirs is the variable. The added comment line explains it, so this reads fine; a rename (for example ..._refuses_another_phase_at_the_same_token_count) would rename the node ids again, and nothing in the tree selects them. Optional.

@pytest.mark.parametrize(
"mine, theirs",
[("prefill", "decode"), ("decode", "Decode")],
ids=["prefill-decode", "two-spellings"],

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ponytail-review — L646-650: shrink: drop ids=; pytest's generated ids for these tuples are prefill-decode and decode-Decode, and without it the decorator fits one 87-column line: @pytest.mark.parametrize("mine, theirs", [("prefill", "decode"), ("decode", "Decode")]). -4 lines. Non-blocking: it trades the word two-spellings for the literal spellings in the node id.

@jgong5

jgong5 commented Sep 23, 2026

Copy link
Copy Markdown
Owner Author

Review 1 — PR #298 (issue #296), head ad9a1c5e0e35aaf5afed62ac3ef20b6788984bb2

Agent-authored review. Read the eight design principles in atom/compass/design/README.md and atom/compass/AI_DEV_RULES.md first.

Verdict: APPROVE at ad9a1c5e0. No blocking findings. Three non-blocking inline comments: the spelling choice, the test name, and one ponytail shrink:.

1. Design ruling (principle 6): case-sensitive is the right direction

  • Refusing is the principle-6 direction. Folding case would decide on the caller's behalf that "decode" and "Decode" are one phase. That is a guess, and the package refuses guesses. With exact equality, a caller that mixes spellings gets a named refusal that quotes both spellings. The Shape docstring already states this.
  • The evidence is complete; checked at the head.
    • Nothing under atom/ or scripts/ builds a memory.Shape. git grep "Shape(" finds only atom/compass/audit/sync_scan.py, which defines its own unrelated class. runner/overrides.py imports EAGER_SOURCE, DeviceReadings, SizedKVPool from memory, but not Shape.
    • No Compass module emits a phase string.
    • SequenceType members are auto(), so a caller's only string form is .name, which is PREFILL or DECODE. Two spellings are therefore plausible from future callers. None arises today.
  • So there is no legitimate same-side producer that this would break, and the "yes" branch of the brief is correct.

2. Named result, reproduced (principle 8)

Setup:

  • Node 18, xiaobizh_n18_cpu.
  • Each tree was a git archive, md5 verified on both ends, and extracted fresh for each mutation.
  • atom.__file__ was asserted under each staged root.
  • Every mutation keeps compare.py at 847 lines.
  • Only tests/compass/test_memory_compare.py was run.
mutation (atom/compass/memory/compare.py) tip 6a83b56bc head/merged 81414803c
none 57 passed 59 passed
outright site: (tokens, phase.casefold()) on both sides of != 57 passed 1 failed, 58 passed: [two-spellings-outright], DID NOT RAISE MemoryRefusal
per-term site: (tokens, phase.casefold()) on both sides of == 57 passed 1 failed, 58 passed: [two-spellings-per-term], assert set() == {'activations'}
both sites casefolded 57 passed 2 failed, 57 passed: both ids above
normalise in Shape.__post_init__: object.__setattr__(self, "phase", self.phase.casefold()) folded into the blank check 57 passed 2 failed, 57 passed: both ids above
null control: operands swapped at both sites 57 passed 59 passed

Failing node ids:

  • tests/compass/test_memory_compare.py::test_a_decode_side_refuses_a_prefill_side_at_the_same_token_count[two-spellings-outright]
  • tests/compass/test_memory_compare.py::test_a_decode_side_refuses_a_prefill_side_at_the_same_token_count[two-spellings-per-term]

This matches the dev record. My independent mechanism, normalising at construction, is caught at both sites. The pin is live, not inert.

3. Renamed node ids

[outright] and [per-term] are now [prefill-decode-outright] and [prefill-decode-per-term]. I found no reference to the old ids, or to the test name:

No other open PR touches test_memory_compare.py or memory/.

4. ponytail-review

  • tests/compass/test_memory_compare.py L646-650: shrink: drop ids=. pytest already generates prefill-decode and decode-Decode, and the decorator then fits one 87-column line. Non-blocking.

net: -4 lines possible.

5. Design-doc references

None in the diff. The only hits in the file are the pre-existing guard fixtures near L1085-1099, which are strings the reference guard is driven with, not citations. compare.py is unchanged and has none.

Gate: the tree that will land

  • The tip was re-read after fetch: fork/feature/atomcompass_new = 6a83b56bc, unchanged.
  • git merge-tree --write-tree 6a83b56bc ad9a1c5e0 = 81414803cbbd3c9944c3f95a76c405cdbba3c139, which equals ad9a1c5e0^{tree}.
  • I gated it once on node 18 with the tree's own scripts/compass/gate_cpu.sh. Stamps were written: .compass-commit = ad9a1c5e0, and .compass-changed = the one test file.
  • The gate printed commit: ad9a1c5e0 (stamp), atom: /tmp/pr298rev/merged/ATOM/atom/__init__.py and gpu: not required.
  • Result: 5168 passed, 155 skipped, 3 xfailed, GATE_CPU_RC=0 PASSED. No flake re-runs were needed.
  • Against the developer's control of 5166 at 6a83b56bc, the delta is +2, exactly the two added parameter cases.

Staging was cleaned up after the run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant