fix(local_verify): say so when the overlay carried the FIX into the base - #125
Merged
Merged
Conversation
The overlay is meant to add the candidate tests to the base tree and nothing else. When the fix
itself lives IN TEST FILES, the default `--test-path` scope — "every changed test file" — is every
changed file, so the base tree after the overlay is identical to the worktree in every file that
differs. RED and GREEN then run the same code and neither verdict means anything.
WHAT IT REPORTED INSTEAD, captured from the fixture with the guard disabled:
verdict: FAIL_HOLLOW
reason: candidate tests still pass against the base implementation; 2 of 2 candidate nodes do
not discriminate: tests/test_guard.py::TestGuard::test_guard,
tests/test_regression.py::TestRegression::test_regression
A confident statement about the TESTS, and a false one — the tests were fine and the RUN was
misconfigured. Worse, the per-node pass corroborates it by naming both nodes as tautologies, so the
diagnosis reads well-evidenced and its implied fix (rewrite the tests) is the opposite of the real
one (scope `--test-path` to the new module). Measured on Counter_Risk #964, where getting the scope
right was load-bearing and had to be known in advance.
`overlay_covers_every_change()` is the EXACT condition — every path that differs from the base was
copied into it — not a heuristic about which files look like tests. Conservative by construction: it
fires only when NOTHING is left uncovered, so the correct usage (overlay the new test, leave the fix
behind in the base) can never trip it, and a deletion cannot be copied so a removing worktree is
never flagged.
ONE PREDICATE, TWO CAUSES. `invalid_demonstration()` folds this together with the node-id case #118
added, so a consumer has a single question to ask. And it reaches the RESULT DICT, not only the
transcript: the JSON consumer is the one being misled, and a finding that only the renderer can see
is the delivery defect `how_to_use` already paid for once. It rides `reason`, which `record_verdict`
writes to `outcomes.notes`, so the evidence stream sees it too — with the computed verdict preserved
inside the sentence.
REPORTED, NEVER GATED. `verdict`, `ok` and the exit code are computed ABOVE this and are untouched,
matching `--transcript`'s rule that a reporting feature which could move a gate would make the
artifact and the gate two answers to one question. A selftest pins the verdict unchanged.
`verify()` now reads `_changed_paths` ONCE and filters it for the default overlay, because the whole
changed list is what answers the coverage question; two git calls could disagree about what changed.
BREAK -> REVERT (four; two of them fire on the previously-silent path, which is the direction that
matters most for a new warning)
* `overlay_covers_every_change` returns False -> `AssertionError: overlay_covers_every_change(
degenerate)` with the full FAIL_HOLLOW result quoted above.
* the finding stops riding `reason` -> `AssertionError: candidate tests still pass against the base
implementation; 2 of 2 candidate nodes do not discriminate...`
* `changed_paths` no longer carried -> the guard goes blind and fires on the SOUND run:
`AssertionError: "THIS IS NOT A VALID DEMONSTRATION" not in sound_text`
* `all(...)` -> `any(...)` -> the same false positive, from the other side.
Reverted; selftest green. The fixture asserts BOTH directions on one repo: the default scope is
flagged, and the same fix with `--test-path` scoped to the new module alone is silent — a warning
that fires on every run is a warning nobody reads.
verify.py: 458 passed, 85 selftests, 5 of 5 gates green. Collection unchanged (458), no floor move.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 49 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 72 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
stranske
temporarily deployed
to
agent-standard
August 25, 2026 04:21 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
Workflow source neededPR #125 needs either a linked GitHub issue or one valid non-issue Workflow Source before PR metadata automation can manage it safely. Please do one of:
Once a valid source is present, this warning will not be reposted. |
Automated Status SummaryHead SHA: aabfee6
Coverage Overview
Coverage Trend
Top Coverage Hotspots (lowest coverage)
Low Coverage Files (<50.0%)
Updated automatically; will refresh on subsequent CI/Docker completions. Keepalive checklistScopeNo scope information available Tasks
Acceptance criteria
|
Contributor
|
Workflow state fingerprint for Agents Gate Followups. Do not edit. |
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
1 participant
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.
The defect
The overlay is meant to add the candidate tests to the base tree and nothing else. When the fix itself lives in test files, the default
--test-pathscope — "every changed test file" — is every changed file, so the base tree after the overlay is identical to the worktree in every file that differs. RED and GREEN then run the same code, and neither verdict means anything.What it reported instead, captured from the new fixture with the guard disabled:
A confident statement about the tests, and a false one — the tests were fine, the run was misconfigured. Worse, the per-node pass corroborates it by naming both nodes as tautologies, so the diagnosis reads well-evidenced and its implied fix (rewrite the tests) is the opposite of the real one (scope
--test-pathto the new module). Measured on Counter_Risk #964, where getting that scope right was load-bearing and had to be known in advance.What changed
overlay_covers_every_change()— the exact condition (every path that differs from the base was copied into it), not a heuristic about which files look like tests. Conservative by construction: it fires only when nothing is left uncovered, so the correct usage — overlay the new test, leave the fix behind in the base — can never trip it, and a deletion cannot be copied so a removing worktree is never flagged.invalid_demonstration()folds this together with the node-id case fix(capabilities): say what the concept IS, and ship the break proof in the shape an issue body takes #118 added, so a consumer has a single question to ask.how_to_usealready paid for once. It ridesreason(whichrecord_verdictwrites tooutcomes.notes), with the computed verdict preserved inside the sentence.verify()reads_changed_pathsonce and filters it for the default overlay — the whole changed list is what answers the coverage question, and two git calls could disagree about what changed.Reported, never gated.
verdict,okand the exit code are computed above this and untouched, matching--transcript's rule that a reporting feature which could move a gate would make the artifact and the gate two answers to one question. A selftest pins the verdict unchanged.Break → revert (four; two fire on the previously-silent path, which is the direction that matters most for a new warning)
overlay_covers_every_changereturnsFalseAssertionError: overlay_covers_every_change(degenerate)— with theFAIL_HOLLOWresult quoted abovereasonAssertionError: candidate tests still pass against the base implementation; 2 of 2 candidate nodes do not discriminate...changed_pathsno longer carried (guard goes blind)AssertionError: "THIS IS NOT A VALID DEMONSTRATION" not in sound_textall(...)→any(...)Reverted; selftest green. The fixture asserts both directions on one repo: the default scope is flagged, and the same fix with
--test-pathscoped to the new module alone is silent — a warning that fires on every run is a warning nobody reads.Verification
python3 src/verify.py— 458 passed, 85 selftests, 5 of 5 gates green. Collection unchanged at 458, no.verify-floor.jsonmove.🤖 Generated with Claude Code