Skip to content

compass(tests): pin the geometry refusal for every field it reads - #303

Merged
jgong5 merged 1 commit into
feature/atomcompass_newfrom
compass/issue-301-geometry-fields
Sep 23, 2026
Merged

jgong5 merged 1 commit into
feature/atomcompass_newfrom
compass/issue-301-geometry-fields

Conversation

@jgong5

@jgong5 jgong5 commented Sep 23, 2026

Copy link
Copy Markdown
Owner

Closes #301

What changed

test_a_config_with_no_hidden_size_refuses_naming_the_field becomes test_a_config_missing_a_geometry_field_refuses_naming_that_field, parametrized over the four fields _geometry reads in ModelTerms.declared_for_m1 (hidden_size, intermediate_size, head_dim, max_position_embeddings). Each case deletes its field and asserts MemoryRefusal with .what naming that field. The remedy assertion is unchanged.

File set: tests/compass/test_memory_readings.py only. Production lines: 0. Test lines: +6 / -3 (net +3), inside the +2-5 estimate.

Named result (node 18, xiaobizh_n18_cpu)

Mutations are line-count-preserving swaps of atom/compass/memory/readings.py (366 lines each), run on the tip (b3078336d) and the head (b736fd8ba), tests/compass/test_memory_readings.py:

mutant edit tip head
null docstring wording on _geometry only 47 passed 50 passed
M2 line 76: `{name}` -> `hidden_size` in the message 47 passed 3 failed, 47 passed
M4 line 137: int(_geometry(config, "intermediate_size")) -> int(getattr(text, "intermediate_size", 0)) 47 passed 1 failed, 49 passed

Failing node ids at the head:

  • M2: tests/compass/test_memory_readings.py::test_a_config_missing_a_geometry_field_refuses_naming_that_field[intermediate_size], [head_dim], [max_position_embeddings], each AssertionError on .what (states no hidden_size`` against the deleted field). [hidden_size] stays green, as it must.
  • M4: ...::test_a_config_missing_a_geometry_field_refuses_naming_that_field[intermediate_size], Failed: DID NOT RAISE MemoryRefusal.

Original readings.py restored after each run and md5-checked (438f6578b550 on both trees).

Gates

Gate 1: ATOM suite, unmodified, CPU tier (scripts/compass/gate_cpu.sh from each tree, git archive snapshots with .compass-commit / .compass-changed stamps, atom.__file__ asserted under the staged root):

tree atom.file result GATE_CPU_RC
control b3078336d /tmp/i301gates/ctl/ATOM/atom/__init__.py 5206 passed, 155 skipped, 3 xfailed 0
head b736fd8ba /tmp/i301gates/head/ATOM/atom/__init__.py 5209 passed, 155 skipped, 3 xfailed 0

Delta +3 passed, 0 failed. Node-id delta (collect-only on the changed file, 47 -> 50): removed test_a_config_with_no_hidden_size_refuses_naming_the_field; added test_a_config_missing_a_geometry_field_refuses_naming_that_field[hidden_size|intermediate_size|head_dim|max_position_embeddings]. No flaky-class failures on either side.

git merge-tree --write-tree b3078336d b736fd8ba = 17e76b25c472beca4c0c65e47646fe56944614bd = b736fd8ba^{tree}.

Gate 2: the change is itself a CPU-only test in tests/compass/; it exercises _geometry and ModelTerms.declared_for_m1, which this PR does not touch.

Gate 3: the named result above.

Gate 4: reviewer to be dispatched by the coordinator.

Dev record

  • No surprises. The fixture's text_config carries all four fields, so delattr removes a real attribute in every case.
  • The pending declared_for_m1 rename noted in the brief has no open PR; nothing to coordinate.
  • ruff format and ruff check clean on the changed file.

🤖 Generated with Claude Code

The refusal test deleted `hidden_size` only, while the memory model reads
four geometry fields off the config. A message that hardcoded the field
name, or a field read with a silent default, stayed green. The test now
runs once per field, deletes that field, and asserts the refusal names it.

Closes #301

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@jgong5

jgong5 commented Sep 23, 2026

Copy link
Copy Markdown
Owner Author

Review: APPROVE at head b736fd8bafbf05df42f871096f5990d83669417e

Verdict: APPROVE, covering b736fd8bafbf05df42f871096f5990d83669417e (tree 17e76b25c472beca4c0c65e47646fe56944614bd). There are no blocking findings and no inline comments.

I read the eight design principles and AI_DEV_RULES.md first. A previous reviewer stopped on an API error before posting, so this is the first review on this thread.

1. Field completeness (principle 6)

At the tip b3078336d, atom/compass/memory/readings.py calls _geometry exactly four times, at lines 136-139: hidden_size, intermediate_size, head_dim and max_position_embeddings. The parametrize list is exactly that set.

The module reads two other config fields, and both are already pinned by their own tests:

  • dtype goes through _dtype and refuses. test_a_config_with_no_dtype_refuses_rather_than_assuming_one pins it.
  • partial_rotary_factor defaults to 1.0 on purpose (line 147), and the default is labelled in the term source. test_an_absent_partial_rotary_factor_says_so_in_the_table pins it.

getattr(config, "text_config", config) is a structural lookup, not a value default. Nothing is missing.

2. Named result, reproduced (principle 8)

Measured on node 18, xiaobizh_n18_cpu, file tests/compass/test_memory_readings.py.

  • Trees are git archive snapshots staged at /tmp/rv303b/{tip,head}/ATOM, and atom.__file__ resolves under each staged root.
  • Each mutant is a one-line swap of readings.py: 366 lines before and after, and diff shows one changed line.
  • The original file was restored after each run and md5-checked (438f6578b550).
mutant edit tip b3078336d head b736fd8ba
none 47 passed 50 passed
null line 71, docstring wording only 47 passed 50 passed
M2 line 76, `{name}` → `hidden_size` 47 passed 3 failed, 47 passed
M4 line 137, int(getattr(text, "intermediate_size", 0)) 47 passed 1 failed, 49 passed
M5 (reviewer) line 138, int(getattr(text, "head_dim", 128)) 47 passed 1 failed, 49 passed
M6 (reviewer) line 139, int(getattr(text, "max_position_embeddings", 40960)) 47 passed 1 failed, 49 passed
M7 (reviewer) line 74, if value is None and name == "hidden_size":, so the refusal fires only for the field the old test deleted 47 passed 3 failed, 47 passed

Failing node ids at the head, all tests/compass/test_memory_readings.py::test_a_config_missing_a_geometry_field_refuses_naming_that_field[...]:

  • M2: [intermediate_size], [head_dim], [max_position_embeddings]. Each is an AssertionError on .what, reading states no hidden_size`` against the deleted field. [hidden_size] stays green, as it must.
  • M4: [intermediate_size], with Failed: DID NOT RAISE <class 'atom.compass.memory.readings.MemoryRefusal'>.
  • M5: [head_dim], with DID NOT RAISE MemoryRefusal.
  • M6: [max_position_embeddings], with DID NOT RAISE MemoryRefusal.
  • M7: [intermediate_size], [head_dim], [max_position_embeddings], each TypeError: int() argument must be ... not 'NoneType'.

Every mutant is green on the tip, so the old single-field test saw none of them. On the head, each field's case fires on its own. The pin is not inert.

3. Tree that will land

  • git merge-tree --write-tree b3078336d b736fd8ba gives 17e76b25c472beca4c0c65e47646fe56944614bd. That equals b736fd8ba^{tree}, so the head is already based on the tip.
  • For the stamp I built a commit object with git commit-tree 17e76b25c -p b3078336d -p b736fd8ba, which gives 1ffa6fc39fb1c0124fe7dd86aa0ef655e885a7ca.
  • .compass-commit and .compass-changed were written from that commit. The changed set is tests/compass/test_memory_readings.py.

The gate was the tree's own scripts/compass/gate_cpu.sh, run once on node 18 from /tmp/rv303b/merged/ATOM:

atom:   /tmp/rv303b/merged/ATOM/atom/__init__.py
commit: 1ffa6fc39 (stamp)
gpu:    not required (.compass-changed stamp)
5209 passed, 155 skipped, 3 xfailed, 16 warnings in 189.07s
GATE_CPU_RC=0 PASSED

That is 5209 passed, identical to the dev record's head figure. It is +3 over the dev record's control of 5206 at b3078336d: one test was removed and four parametrized cases added. Nothing failed, including the known timing-flaky classes in tests/entrypoints/test_stream_marker_properties.py. ruff check and ruff format --check (0.16.7) are clean on the changed file.

4. No design-doc references

I scanned tests/compass/test_memory_readings.py in full at the head for D<n>, P<n>.<n>, T<n>, W<n>, "principle", "Gate N", doc numbers and design/. There were no hits.

5. ponytail-review (principle 3)

The diff is +6/−3 and replaces a single-field test with a four-value parametrize and an f-string. Nothing is dead or speculative, and no shorter form of the same check exists.

Lean already. Ship.

Notes (non-blocking)

  • Hand-kept field list (principle 3). A fifth _geometry call added later would stay uncovered until someone extends this list. Deriving the list from the source would be machinery for a four-item constant, so I accept the list as written. The next task that adds a geometry field should extend this parametrize list in the same PR.
  • Dev-record wording (principle 8). The record says "the fixture's text_config carries all four fields". The qwen fixture is PretrainedConfig.from_dict(raw["text_config"]), a flat config with no text_config attribute, so _geometry reads the config itself. The substance holds, as M4-M6 show: delattr removes the attribute _geometry actually reads.

No blocking issues.

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