Skip to content

compass(tests): the first-refusal test tells document order from sorted order - #497

Merged
jgong5 merged 1 commit into
feature/atomcompass_newfrom
compass/issue-494-walk-order
Sep 29, 2026
Merged

jgong5 merged 1 commit into
feature/atomcompass_newfrom
compass/issue-494-walk-order

Conversation

@jgong5

@jgong5 jgong5 commented Sep 29, 2026

Copy link
Copy Markdown
Owner

Closes #494

What changed

tests/compass/test_spec_schema.py::test_the_reader_raises_the_first_refusal_the_walk_meets put two unknown keys, first_knob and last_knob, in the device block. They sort in the same order as they appear, so the test could not tell a walk in document order from a walk in sorted order.

The keys are now zeta_knob (before the block's fields) and alpha_knob (after them). The test asserts `device.zeta_knob`. Both sorted order and "raise the last refusal" now name alpha_knob. The test's structure and its other checks are unchanged.

-    # reader that raised any later refusal names the other key.
+    # reader that raised any later refusal names the other key. The first key
+    # sorts last, so a walk in sorted order names the other key as well.
     edited = document()
-    edited["device"] = {"first_knob": 1, **edited["device"], "last_knob": 2}
+    edited["device"] = {"zeta_knob": 1, **edited["device"], "alpha_knob": 2}
     with pytest.raises(SpecRefusal) as refusal:
         MachineSpec.from_mapping(edited)
-    assert "`device.first_knob`" in refusal.value.what
+    assert "`device.zeta_knob`" in refusal.value.what

Dev record

Step 0: the premise was re-measured at the tip c59351778 before any edit. At the tip, spec/machine.py::_walk does not iterate the node itself. It raises the first refusal that spec/machine.py::_survey yields, and _survey is the function that iterates node.items(). So the sorted-order mutant goes on _survey's loop header. It changes one line and keeps the line count:

for key, value in node.items(): -> for key, value in sorted(node.items(), key=lambda kv: str(kv[0])):

Tree Mutant tests/compass/test_spec_schema.py
tip c59351778 none 143 passed
tip c59351778 sorted walk in _survey 143 passed: the premise holds, and no other test in the file notices
tip c59351778 last refusal in _walk 1 failed, 142 passed
head none 143 passed
head sorted walk in _survey 1 failed, 142 passed
head last refusal in _walk 1 failed, 142 passed

The "last refusal" mutant is for refusal in _survey(node, prefix, found): -> for refusal in list(_survey(node, prefix, found))[::-1]:.

At the head, both mutants fail the same node, tests/compass/test_spec_schema.py::test_the_reader_raises_the_first_refusal_the_walk_meets, with the same assertion:

E       assert '`device.zeta_knob`' in '`device.alpha_knob` is not a field of this schema'

machine.py was restored with git checkout after each run, and the worktree diff was checked afterwards.

Named result (from the brief): at the PR head, the sorted(node) mutant of the walk fails this test. It sits on _survey, for the reason given above.

Surprise: the brief puts the sorted iteration in _walk. At this tip, _walk is a two-line wrapper, and the document-order iteration lives in _survey. _survey is shared with spec/validate.py, which collects every refusal. spec/merge.py also calls _walk. This test covers the reader path only.

Left undone: nothing in the brief.

Gates

  • Gate 1: scripts/compass/gate_cpu.sh in xiaobizh_n18_cpu on node 18. Each tree was staged with git archive and md5-checked on both ends. The branch and the control (the tip this branch forked from) have the same counts, as expected for a test-only rename:
    • branch 856205b9d: 5209 passed, 155 skipped, 3 xfailed; GATE_CPU_RC=0 PASSED, rc 0
    • control c59351778: 5209 passed, 155 skipped, 3 xfailed; GATE_CPU_RC=0 PASSED, rc 0
  • Gate 2: the edited test exercises spec/machine.py::_survey/_walk, which this PR does not touch.
  • Gate 3: the named result is above.
  • black --check and ruff check pass on the edited file.

Principle 8: each claim above names the run that measured it.

🤖 Generated with Claude Code

…ed order (#494)

test_the_reader_raises_the_first_refusal_the_walk_meets put two unknown
keys, first_knob and last_knob, in the device block. Their sorted order
matched their document order, so a spec/machine.py::_survey that walked the
block in sorted key order still raised first_knob and stayed green.

The keys are now zeta_knob (before the block's fields) and alpha_knob (after
them), and the test asserts zeta_knob. A walk in sorted order raises
alpha_knob, and so does a reader that raises the last refusal.

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

jgong5 commented Sep 29, 2026

Copy link
Copy Markdown
Owner Author

This review is agent-authored.

Review cycle 1

Verdict: APPROVE

Head covered: 856205b9d187d7b2b77ca881d80a51d132cbfb90, base feature/atomcompass_new at c59351778, which was still the branch tip after a fresh fetch.

Mutations (serial, each one changes one line and keeps the line count, restored with git checkout after each, git status clean afterwards)

I confirmed that import atom resolves to the review worktree. Counts are for tests/compass/test_spec_schema.py + tests/compass/test_spec_verbs.py (298 tests), except where marked.

Tree Mutant in atom/compass/spec/machine.py Result
head none 298 passed
head null comment on _survey's loop header (control) 298 passed
head _survey walks sorted(node.items(), key=lambda kv: str(kv[0])) 1 failed, 297 passed
head _survey walks reversed(list(node.items())) 1 failed, 297 passed
head _walk raises the last refusal (list(_survey(...))[::-1]) 1 failed, 297 passed
head _walk iterates the refusals sorted by message 1 failed, 297 passed
head _survey walks in reverse-sorted key order 298 passed (see note 1)
base test file sorted walk in _survey 143 passed (the #494 premise, reproduced)
base test file last refusal in _walk 1 failed, 142 passed

Every red at the head is tests/compass/test_spec_schema.py::test_the_reader_raises_the_first_refusal_the_walk_meets, and the sorted mutant fails on assert 'device.zeta_knob' in 'device.alpha_knob is not a field of this schema'. So the pin goes red on the defect it names. Both base-file rows match the PR's dev record.

I threw one mutant away: _walk raising max(_survey(...), key=...) gave 144 failed. That is a ValueError on every document that has no refusal, so it went red for the wrong reason and is not a result.

Where the mutant belongs: the developer is right. At this tip, _walk is a two-line wrapper that raises the first thing _survey yields. The key iteration is _survey's for key, value in node.items():. _survey is also imported by spec/validate.py, which calls list(_survey(document, "", found)), and spec/merge.py calls _walk. So a sorted walk in _survey would also reorder what the check path reports. The test pins the reader path only, which is what the PR body says.

Findings (none blocking)

  1. Reverse-sorted walk survives (non-blocking). It left 298 passed on the two spec files. It also left 777 passed across the ten tests/compass files that touch the spec module (test_kv_budget*.py, test_ir_data_model.py, test_memory_*.py, test_artifact_invalidation.py, test_spec_*.py, test_kv_simulated_connector.py, test_runner_rpc_surface.py). I did not run the full CPU gate. With two keys, the document-first key has to sort either first or last, so one of the two sort directions always matches document order. A third unknown key would close the gap: put mid_knob first and alpha_knob/zeta_knob after the fields, and assert mid_knob. Reverse sorting is not a realistic recurrence, and compass(tests): the first-refusal test cannot tell document order from sorted order #494 named sorted(node), so this is the author's call and not a hold.
  2. Assertions: the test's only assertion is kept, with the key renamed. pytest.raises(SpecRefusal) and the fixture's shape (one unknown key before the block's fields, one after) are unchanged.
  3. docs(compass): prose is not a test subject; no line numbers or item counts; the kind of claim decides a doc-code disagreement #489 / AI_DEV_RULES: there are no prose assertions. A grep of the added lines for design-doc references, principle or gate mentions, file.py:NNN citations and item counts found nothing. The one added comment line says what the fixture does. Only tests/compass/test_spec_schema.py changes (+4/-3). The counts in the PR body are measured records with their commits, which the rules allow.
  4. PR body claims checked: _survey iterates and _walk wraps it; _survey is shared with validate.py; merge.py calls _walk; the base and head mutant counts; the assertion text. All of them hold.

Merge-tree

git merge-tree --write-tree fork/feature/atomcompass_new 856205b9d gives 4d87f44afbf6343408d347a2aab59fe042493505. That is identical to the head's tree (856205b9d^{tree} = 4d87f44af...), with no conflicts.

ponytail-review

Lean already. Ship.

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