Repository navigation
compass(tests): hold four refusals the artifact store states and nothing reached - #306
Conversation
…ing reached Eight tests in tests/compass/test_artifact_invalidation.py, no production change: - a recorded fingerprint cell with no matrix column is refused on read rather than silently dropped - publish refuses a gates value that is not a GateState by name, and has no default for gates - a load where both the gate and a fingerprint cell moved is filed in the ledger as GATE_STATE, with a device-only control - GateState refuses a dead gate recorded beside a live twin of its name - differences refuses two rows with identical axes - GateState.check refuses a non-GateState at load - Conditions.of refuses an axis the matrix has no column for Closes #225 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
| def test_a_publish_that_omits_the_gates_is_a_type_error(tmp_path): | ||
| """No default: an entry that states no gates cannot be checked on load.""" | ||
| store = ArtifactStore(tmp_path) | ||
| with pytest.raises(TypeError): |
There was a problem hiding this comment.
Non-blocking. Principles 6 and 8: name the TypeError you mean.
The TypeError test is the right pin. It is stronger than a check on the signature default, and I ruled on that in the review comment. But pytest.raises(TypeError) with no match= accepts any TypeError raised inside publish. So a mutant that adds a default and then fails later with a different TypeError (for example from json.dumps or a members check) would still pass this test.
The text Python gives here, measured at 9f053e342 on node 18, is:
ArtifactStore.publish() missing 1 required keyword-only argument: 'gates'
Suggested change: with pytest.raises(TypeError, match="argument: 'gates'"):. It is the same number of lines. Only the missing-argument error contains that substring. The sibling at L725 (conditions) has the same gap, so if you change this one, change that one too, for the same reason.
| conditions=BASE, | ||
| members={member_name("rows", rank, "json"): b"{}" for rank in TP2.ranks()}, | ||
| ) | ||
| assert not list(tmp_path.iterdir()) |
There was a problem hiding this comment.
ponytail: delete: This line can never fail when the raises above is satisfied by the missing argument.
L826: delete: assert not list(tmp_path.iterdir()). If binding the arguments fails, the body of publish never ran, so nothing was written. Nothing replaces it once the raises matches 'gates'.
It could only fail if the TypeError came from inside the body. That is the loose-raises case from the comment on L818, and the match= closes it more precisely. Net -1 line. Non-blocking.
|
Review record: PR #306 at head This review is agent-authored. I read the eight design principles in Verdict: APPROVE 1. Mutations, reproduced on node 18 (
|
| # | mutation (lines before->after) | control 93be90841 |
head 9f053e342: failing node id and assertion |
|---|---|---|---|
| 1 | fingerprints.py cell unknown = [] (324->324) |
118 passed | 1 failed / 125 passed: test_a_recorded_cell_the_matrix_has_no_column_for_is_refused, DID NOT RAISE ArtifactRefusal |
| 2 | store.py _fingerprints gates guard -> if False: (493->493) |
118 passed | 1 failed / 125 passed: test_a_publish_handed_something_that_is_not_a_gate_state_is_refused, AttributeError: 'str' object has no attribute 'as_json' |
| 2b | gates: GateState = GateState.of(), (493->493) |
118 passed | 1 failed / 125 passed: test_a_publish_that_omits_the_gates_is_a_type_error, DID NOT RAISE TypeError |
| 3 | swap entry.gate_state.check(gates) / verify(...) in load (493->493) |
118 passed | 1 failed / 125 passed: test_a_miss_where_the_gate_and_the_device_both_moved_is_filed_as_the_gate, Rule.INVALIDATED != Rule.GATE_STATE at index 0 |
| 4a | gates.py duplicate-name guard -> if False: (171->171) |
118 passed | 1 failed / 125 passed: test_a_dead_gate_cannot_hide_behind_a_live_twin_of_its_name, DID NOT RAISE |
| 4b | fingerprints.py recorded.row is not current.row -> if False: (324->324) |
118 passed | 1 failed / 125 passed: test_two_rows_over_the_same_axes_are_still_not_compared, DID NOT RAISE |
| 4c | gates.py check isinstance guard -> if False: (171->171) |
118 passed | 1 failed / 125 passed: test_a_load_handed_something_that_is_not_a_gate_state_is_refused, AttributeError: 'str' object has no attribute 'by_name' |
| 4d | fingerprints.py Conditions.of unknown = [] (324->324) |
118 passed | 1 failed / 125 passed: test_conditions_that_state_an_axis_the_matrix_has_no_column_for_are_refused, KeyError: 'compiler' |
| own-1 | Fingerprint.from_json: cells = {k: v for k, v in document["cells"].items() if k in {a.field for a in Axis}}. The drop happens upstream of the check, so the guard stays but never sees the cell (324->324) |
118 passed | 1 failed / 125 passed: test_a_recorded_cell_the_matrix_has_no_column_for_is_refused, DID NOT RAISE |
| own-2b | gates: GateState = None, plus gates = gates or GateState.of(); prints = .... A None default with a fallback, not a literal default (493->493, 2 lines) |
118 passed | 1 failed / 125 passed: test_a_publish_that_omits_the_gates_is_a_type_error, DID NOT RAISE TypeError |
| own-4a | GateState.from_json: return cls(tuple({g.name: g for g in map(Gate.from_json, document)}.values())). The rows are collapsed by name on read, so the duplicate-name guard is bypassed rather than deleted (171->171) |
118 passed | 1 failed / 125 passed: test_a_dead_gate_cannot_hide_behind_a_live_twin_of_its_name, DID NOT RAISE |
| null | # line-drift control at line 2 of store.py, fingerprints.py and gates.py, and of the test file |
118 passed x4 | 126 passed x4 |
Restored: control 118, head 126. The developer's battery3.out figures reproduce exactly. Each red names the test written for that property, and no two mutations share a failing test except where one property was attacked twice. So none of the reds is a drift guard firing.
2. Principle 8: is each test pinning the property, or an incidental detail?
Mutation 3: precedence is a contract, not an accident.
ArtifactStore.load's docstring (store.pyL332-336) states it: "The gate state is checked first and the fingerprint second, because a measurement taken under other gates is not the thing the fingerprint describes."answerrecords whichever refusalloadraises asMiss.rule, so the order decides what the ledger says declined.- The test pins that observable value,
Miss.rule, not line order, which is exactly the named result in compass(artifacts): four stated properties the artifact store holds by nothing #225. - Its device-only control (
[Rule.INVALIDATED]) proves the fingerprint move refuses on its own. Without it, the order would be unobservable.
One limit, for the record rather than as a gap: under OnMismatch.WARN both orders give GATE_STATE, because verify only warns and the gate still refuses. So the order is observable only under REFUSE, and the test correctly uses the default.
2b: the TypeError is the right pin. A signature-default check would be weaker.
- The TypeError pins behaviour, which is what the
publishdocstring promises ("conditionsandgateshave no defaults on purpose"). inspect.signature(...).parameters["gates"].default is Parameter.emptypins only the declaration. It is blind to afunctools.wrapsdecorator that injects a default, becauseinspect.signaturefollows__wrapped__and reports the unwrapped signature.- The test also matches its sibling for
conditionsat L722. - My own-2b (a
Nonedefault with anorfallback, the principle-6 fallback in its usual form) reddens it.
The one weakness is the missing match=. See the inline comment on L818. It is non-blocking.
The other six pin the stated property:
- Each asserts the rule and a substring that only the intended refusal emits. For example, test 1's
records \compiler`, which the invalidation matrix has no column foris distinct from the row-level refusal'srecords a fingerprint for`. - Test 4b's
axes_of(...) == axes_of(...)precondition shuts out the axes check as a stand-in. - Test 4a's ordering, dead gate first and live twin last, is what makes the collapsed-by-name mutant load clean. So the red is the defect the docstring describes: a wrong answer, not merely a different refusal.
Watch item, no action. Tests 2 and 4c share the needle 'off' is not a gate state, which both _fingerprints and GateState.check emit. Today each path reaches only one of the two guards, and mutations 2 and 4c prove it. If publish ever starts calling GateState.check, test 2 could be satisfied by the neighbour. The remedies differ ("shaped this entry" vs "in force"), and that would be the sharper needle.
3. Size: ponytail-review over the diff
test_artifact_invalidation.py L826: delete: assert not list(tmp_path.iterdir()). It cannot fail when the TypeError is the missing-argument one, because the body never ran. The match= on L818 replaces it.
I also considered two shrinks and rejected both:
- A shared
rewrite_entryfixture. The read/chmod/write sequence now appears 3 times: L666-671 (existing) and the new L801-806 and L869-875. A helper costs about 6 lines and saves about 1 per site, so the net is about 0. - Parametrizing tests 2 and 4c. They reach different guards. Merging them would hide which refusal each node id holds, and the brief asks for each to go red by name.
Folding 2b into test 2, like the conditions sibling does, would break the same one-red-per-mutation attribution.
net: -1 lines possible.
4. No design-doc references
I grepped the whole test file at the head, not only the added lines, for D<n>, P<n>.<n>, T<n>, W<n>.<n>, principle, §, backticked doc numbers and Gate <n>: no hits. The file opens 07_calibration_toolchain.md by path (MATRIX_DOC), but that predates this PR and is a dependency, not a citation. Measured in xiaobizh_n18_cpu, in the full staged trees: ruff check RUFF_RC=0 and black --check BLACK_RC=0 on the file, at both the head and the control.
5. Gate: the tree that will land
The tip moved twice during this review. #303 was on the tip when I started, and #304 and #302 landed while the first gate was running. So I gated both merged trees, one run at a time. Each gate:
- used the tree's own
scripts/compass/gate_cpu.shwith stamps, and itscommit:line matched; - was bounded by
timeout -k 10 1800and not piped; - had
atom.__file__resolve under its staged root.
| tip | git merge-tree --write-tree <tip> 9f053e342 |
stamp (git commit-tree, parents tip + head) |
atom.__file__ |
result |
|---|---|---|---|---|
93be90841 (#303) |
251016807c8a… (rc 0) |
272598404 |
/tmp/r306/merged/ATOM/atom/__init__.py |
5217 passed, 155 skipped, 3 xfailed, GATE_CPU_RC=0 PASSED |
f84b4d778 (#304, #302), current |
0f0488d617d036a70c9acf33fc71d79d3c203edb (rc 0) |
b2813078c |
/tmp/r306/merged2/ATOM/atom/__init__.py |
5222 passed, 155 skipped, 3 xfailed, GATE_CPU_RC=0 PASSED |
How the counts decompose:
- The developer's control at
b3078336dwas 5206. - compass(tests): pin the geometry refusal for every field it reads #303 turned one test into 4 parametrized cases (+3), giving 5209.
- This PR adds +8, giving 5217.
- compass(tools): pin the loose-refusal detector's BROAD and SHARP printouts, argv guard and recursion #304 adds +3 new tests and splits 1 test into 2 cases (+1), and compass(tests): hold the profiler-reply assertion to the roots REPLY_SURFACE names #302 adds +1: +5 in all, giving 5222.
The skipped (155) and xfailed (3) counts are unchanged from the developer's measurement. All eight new node ids are PASSED in both logs. Neither timing class nor test_gc_utils failed.
#304 and #302 touch only test_detect_loose_refusals.py and test_runner_rpc_surface.py. Neither reads tests/compass/ of the live tree: the detector test states it runs on fixtures only, and the RPC-surface scan walks atom/. The gate for landing tree 0f0488d61 stands. Recompute merge-tree if the tip moves again.
6. Out of scope, for a follow-up
atom/compass/artifacts/fingerprints.py L270-271 (the differences axes-mismatch refusal) renders "... over X and and the matrix now rows it over ...": one f-string ends in and and the next begins with and . No test under tests/compass/ reads that message's text (a grep for fingerprinted over finds nothing). It is in #225's file set but not its four properties, so it does not block. The rules say a finding not fixed in the PR that found it gets an issue, so the developer should either fix it here, with a pin on the text, or file it.
What the next task in this area should watch: the shared 'off' is not a gate state needle (section 2), and the pytest.raises(TypeError)-without-match= shape at L725, which predates this PR.
Closes #225
Dev record. Eight CPU-only tests in
tests/compass/test_artifact_invalidation.pythat hold the four properties #225 names. Production lines: 0. Test lines: +132 / -0 (claimed estimate ~130). There are no blocking issues.Head
9f053e342on tipb3078336d. This is a rebase of the claim-time commitd3c70a045(on070b2bb06): same patch-id04ad6eb6f, andgit range-diffreports=.Already held vs newly pinned
Already held at the tip: none. At
b3078336d, every one of the eight defects below leaves the two artifact test files at 118 passed. Newly pinned: all eight.Named result, re-measured on node 18 (
xiaobizh_n18_cpu)Each defect is a one-line, line-count-preserving mutation of
atom/compass/artifacts/. I ran each one againsttest_artifact_invalidation.pyandtest_artifact_store.py, at the tip (control) and at the head. Every head red is exactly one test, the new one, named here:fingerprints.pyunknown = sorted(set(cells) - ...)->unknown = [](324->324)test_a_recorded_cell_the_matrix_has_no_column_for_is_refused:DID NOT RAISE ArtifactRefusalpublish(gates="off")is a namedGATE_STATErefusalstore.py_fingerprints:if not isinstance(gates, GateState):->if False:(493->493)test_a_publish_handed_something_that_is_not_a_gate_state_is_refused:AttributeError: 'str' object has no attribute 'as_json'publishhas no default forgatesgates: GateState,->gates: GateState = GateState.of(),(493->493)test_a_publish_that_omits_the_gates_is_a_type_error:DID NOT RAISE TypeErrorentry.gate_state.check(gates)andverify(...)inload(493->493)test_a_miss_where_the_gate_and_the_device_both_moved_is_filed_as_the_gate:Miss.ruleisRule.INVALIDATED!=Rule.GATE_STATEGateStateduplicate name (a dead gate behind a live twin)gates.pyif len(set(named)) != len(named):->if False:(171->171)test_a_dead_gate_cannot_hide_behind_a_live_twin_of_its_name:DID NOT RAISE ArtifactRefusaldifferencesrefuses a row mismatchfingerprints.pyif recorded.row is not current.row:->if False:(324->324)test_two_rows_over_the_same_axes_are_still_not_compared:DID NOT RAISE ArtifactRefusalGateState.checkrefuses a non-GateStategates.pyif not isinstance(in_force, GateState):->if False:(171->171)test_a_load_handed_something_that_is_not_a_gate_state_is_refused:AttributeError: 'str' object has no attribute 'by_name'Conditions.ofrefuses an unknown axisfingerprints.pyunknown = sorted(set(values) - set(wanted))->unknown = [](324->324)test_conditions_that_state_an_axis_the_matrix_has_no_column_for_are_refused:KeyError: 'compiler'All node ids are under
tests/compass/test_artifact_invalidation.py::. The clean runs give 118 passed at the tip and 126 at the head. After each file was restored, the tip was back at 118 and the head at 126.For item 3, the named result is the
Miss.rulevalue. With both a moved gate and a moved device, the head files the miss asGATE_STATE. The reversed code files it asINVALIDATED. The same test carries a device-only control, which is filed asINVALIDATEDboth at the head and under the mutation. That control shows the fingerprint move refuses on its own, so the order is observable.Rows 2 and 4c fail the way #225 predicted: the refusal degrades to an
AttributeErrorat a distance.Null (line-drift) controls. One comment line inserted at line 2 of each mutated module (
store.py493->494,fingerprints.py324->325,gates.py171->172). Tip: 118 passed, three times. Head: 126 passed, three times. So no new test is a positional guard.ATOM suite (
scripts/compass/gate_cpu.sh, the tree's own copy)I staged each tree with
git archiveand the.compass-commit/.compass-changedstamps, then copied it intoxiaobizh_n18_cpuat/tmp/i225v3/{control,branch}/ATOM. The tarball md5 matched on both ends.atom.__file__resolved under each staged root. I ran one gate at a time, each bounded bytimeout -k 10 1800, with no pipe.atom.__file__b3078336d/tmp/i225v3/control/ATOM/atom/__init__.pyGATE_CPU_RC=0 PASSED9f053e342/tmp/i225v3/branch/ATOM/atom/__init__.pyGATE_CPU_RC=0 PASSEDNode-id delta: 0 ids only in the control. 8 only in the head, which are exactly the eight tests above, all PASSED. No timing-class flake showed on either side.
Merged tree.
git merge-tree --write-tree b3078336d 9f053e342exits 0 and gives79de8d395b4144bb957e249f4137ff3e7d78d55e, which equals9f053e342^{tree}.Decisions the brief did not cover
REGION_TERMSagainstMEMORY_READINGS. The test first asserts that theiraxes_ofare equal, so the axes check cannot stand in for the row check.store.load, not by callingGateState.checkdirectly. The refusal is held at the boundary a caller uses.Surprises
d3c70a045had been pushed before the rebase, and no PR existed. I replaced it with--force-with-leasepinned to that sha. It carries the same patch-id.Left undone
Nothing in the file set. Review is pending.
🤖 Generated with Claude Code