test(semantic): close the decision gate, 69.4% -> 100% (and correct #490) - #494
Merged
Merged
Conversation
) PR #490 reported "19 mutants, 94.7%" for policy.py + guard.py. That was measured over 19 of the module pair's 62 mutation sites, because of an indexing defect in the mutation harness, so it described roughly a third of the module rather than the module. The true figure was 69.4% with 19 survivors. This adds 30 tests and takes it to 62/62 with no survivors. The survivors included the most safety-critical mutants found anywhere in this work: is_id = column in ctx.id_columns or (info is not None and info.identifier_like) flipped to `and` narrows identifier protection so a column detected as identifier-like, but not explicitly listed, silently loses it. The gate's own review-threshold comparison was movable. `format_alignment and _payload_preserving` flipped to `or` would let a non-payload-preserving repair through. In guard.py the row-count check was movable, and the NA comparison could be flipped so the byte-identity guard misses a violation. It also corrects a false claim this file shipped in #490. The docstring asserted that flipping `if left_na or right_na` to `and` was an equivalent mutant, "verified across 289 input pairs ... zero behavioural differences. It cannot be killed." It can. The pool held only ordinary scalars and no cell whose __eq__ returns True against anything; for such a cell the fall-through comparison returns True, so the mutant reports a missing value and a present one as equal and the final identifier-protection layer misses the violation. A 1296-pair differential finds 18 such disagreements. The mutant is now killed and the docstring says what went wrong. That note also cited mutant ids, which are positional and did not survive the harness fix: its `bool#5` is today's `bool#2`. The corrected docstring quotes the source line instead. Tests only; no src change.
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
PR #490 reported "19 mutants, 94.7%" for
semantic/policy.py+guard.py. That figure was measured over 19 of the module pair's 62 mutation sites — an indexing defect in the mutation harness meant most(kind, index)pairs matched no node and were silently skipped. It described roughly a third of the module, not the module.True figure: 69.4%, with 19 survivors. This adds 30 tests and closes it.
not applied: 0on every run. There are now no equivalent mutants in the gate pair.Why the survivors mattered
This is the module pair the whole safety story rests on:
policy.decide()is the gate that decides whether a repair is applied, suggested or skipped, andguard.pyis the last of the five identifier-protection layers.policy.py— identifier protection narrowed.Flipped to
and, a column that is detected as identifier-like but not explicitly listed inid_columnssilently loses its protection. This is the brief's named "ID-protection removed" class, sitting in the gate itself.policy.py— the gate's own review boundary.proposal.confidence < ctx.review_threshold→<=was movable. #491 closed the equivalent boundary inscoring.py; the gate's copy was never covered.policy.py—issue_type == 'format_alignment' and _payload_preserving(proposal)→orwould let a non-payload-preserving repair through the format-alignment path.guard.py— the byte-identity comparison. The row-count checklen(after) < len(before)was movable, and the NA comparison could be flipped so the guard misses a violation (see below).This corrects a false claim shipped in #490
tests/test_policy_guard_mutants.pyasserted in a docstring:It can be killed, and now is. The 289-pair pool held only ordinary scalars and contained no cell whose
__eq__returns True against anything —unittest.mock.ANY, and wildcard/matcher objects generally. For such a cell the fall-throughbool(left == right)returns True, not False:So the mutant reports a missing value and a present one as equal, and the final identifier-protection layer passes a column whose NA cell was replaced by a real value. A 1296-pair differential finds 18 such disagreements, in both argument orders.
The claim was not merely imprecise: it asserted that an unkillable mutant sat on the last safety layer, when in fact an uncovered path let that layer miss a violation.
The note also cited mutant IDs, which are positional and did not survive the harness fix — its
bool#5is today'sbool#2, and today'sbool#5is a different site entirely. The corrected docstring quotes the source line instead of an ID.The methodological lesson is in the docstring: an equivalence claim is only as strong as the input pool it was checked over, and a pool of ordinary scalars cannot rule out exotic
__eq__. Prefer a proof that the branch is unreachable over a differential that merely found no counterexample.Scope
Tests only — no
src/change. No behaviour changes, so no changelog entry and no compatibility impact. The one non-new-file edit is the corrected docstring.Verification
test_policy_guard_mutants.py+test_guard_protected.py— 63 passedruff check .clean (the repo's CI lint;ruff formatis not enforced ontests/andtest_policy_guard_mutants.pyis not format-clean onmaineither, so its formatting is left alone)One latent defect found (reported, not fixed here)
decide()gates auto mode on the pre-override risk while_decision()rewrites the recorded risk afterwards:So a proposal with
issue_type="unsafe_ambiguous"andrisk="low"is auto-applied and recorded as high risk, contradicting the module docstring's "never auto-apply high risk".Not reachable from the pipeline today — verified: the only in-tree producer (
apply.py:131) callsmake_proposalwithoutrisk_override,scoring.risk_forfloors that issue type to"high"unconditionally, and every otherrisk_overrideinsrc/is"high"or conditional-high. It becomes reachable the moment any expert emitsunsafe_ambiguouswith an explicit non-high override, and nothing prevents that. Minimal fix is to have the gate test the same value it records; that touchessrc/and wants its own regression test.