feat(review-suite)!: remove the unproven verification-sufficiency pass - #106
Merged
Conversation
- Remove the verification-sufficiency pass and its mandatory `verification_sufficiency_evidence` field from `skills/review-correctness/SKILL.md` and the shared review-result contract; advance `schema_version` `1.3 -> 1.4`. - Update `review-suite/CONTRACT.md` and `review-suite/scripts/validate.py` to match: a `clean` verdict no longer requires or accepts this evidence, and a stale `1.3` result now fails with a clear migration-to-`1.4` error. - Migrate every canonical fixture, standalone expected result, and orchestration case to `schema_version` `1.4`, then refresh all six bundled `references/review-suite/` copies with `just sync-contracts`. - Retire the `verification-sufficiency-guard` fixture (its sole purpose was the removed field's clean-pairing behavior); adapt, not delete, the `standalone-verification-sufficiency-gap` eval case, since its underlying defect pattern (a claimed test that misses the actual triggering condition) is still a genuine correctness catch expected from ordinary review, per `review-suite/fixtures/missing-test/`. - Add `review-suite/evals/v2/VERIFICATION-SUFFICIENCY-REMOVAL.md`, the decision record citing `S1-ABLATION-MATRIX.md` and `discriminating-case-validation.md` directly, and describe the removal in `README.md` and `CHANGELOG.md`. - Two independent, honestly-sourced measurements found no demonstrated value for this pass: `stale-claim-release-guard` (#57's ablation matrix) and `audit-log-flush-keyword-probe` (#89's harder-case validation) both resolve identically at `mean_combined_recall 1.0` whether the pass is enabled or not. - It also carries a confirmed, twice-reproduced cost: run in isolation (traversal pass disabled), it regresses `session-continuation-summary` - a clean control - to 3/5 false positives. - The traversal (consumer/impact) pass and `consumer_impact_evidence` showed a real, reproducible gap in the same evidence (`artifact-promotion-environment-shortcut`, 1.0 vs 0.4) and are explicitly out of scope for this ticket; both are verified byte-identical to their #52/#53-shipped state. - Approved directly by the repository owner (#93), executing a removal decision already made rather than re-litigating it. Fixes #93
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.
Summary
Removes the verification-sufficiency pass and its mandatory
verification_sufficiency_evidencefield fromreview-correctnessand theshared review-result contract, advancing
schema_version1.3 → 1.4. Thetraversal (consumer/impact) pass and
consumer_impact_evidence— the othermechanism #52/#53 added — are unchanged, byte-for-byte.
skills/review-correctness/SKILL.md: removed the verification-sufficiencypass instruction. The underlying question (does a claimed test actually
exercise the triggering condition it addresses?) remains part of ordinary
correctness review, just without a separately mandated pass or required
evidence field. The traversal pass instruction is preserved exactly as
shipped.
review-suite/contracts/review-result.schema.json,review-suite/CONTRACT.md,review-suite/scripts/validate.py:verification_sufficiency_evidenceremoved entirely (full removal, not anoptional field — see the decision record for why);
schema_version1.3 → 1.4; a stale1.3result now fails with a clear migration error.across all six bundled review skills migrated to
1.4atomically; all sixbundled
references/review-suite/copies refreshed viajust sync-contractsand confirmed byte-identical.verification-sufficiency-guardfixture retired (its sole purpose was theremoved field's clean-pairing behavior).
standalone-verification- sufficiency-gapadapted, not deleted: its underlying defect pattern isa genuine correctness catch independent of the removed pass (see
review-suite/fixtures/missing-test/for the same pattern caught byordinary review), so its
changes_requiredverdict and blocking findingare preserved.
stale-claim-release-guardandaudit-log-flush-keyword-probecorpuscases untouched — still general correctness cases.
review-suite/evals/v2/VERIFICATION-SUFFICIENCY-REMOVAL.md,citing
S1-ABLATION-MATRIX.mdanddiscriminating-case-validation.mddirectly.
README.mdandCHANGELOG.mdupdated to describe the removal and itsevidentiary basis.
Why
Two independent, honestly-sourced measurements found no demonstrated value
for this pass:
stale-claim-release-guard(Run the preregistered review v2 ablation and integration closeout #57's ablation matrix): resolves identicallyat
mean_combined_recall 1.0whether the pass is enabled or disabled.audit-log-flush-keyword-probe(Source harder discriminating cases for #52/#53's traversal and verification-sufficiency passes #89's harder-case validation): alsoresolves identically at
mean_combined_recall 1.0in both configurations.On top of the absence of demonstrated value, the pass carries a confirmed,
twice-reproduced cost: run in isolation (traversal pass disabled), it
regresses
session-continuation-summary— a previously clean control — to3/5 false positives.
By contrast, the traversal pass does show a real, reproducible gap in
the same evidence (
artifact-promotion-environment-shortcut,mean_combined_recall1.0 → 0.4) and is out of scope for this removal.This ticket executes a removal decision already approved by the repository
owner on 2026-07-29; it does not re-litigate the evidence.
Non-goals honored
consumer_impact_evidence: untouched (verifiedbyte-identical against base, independently confirmed by a fresh
review-code-changecycle).stale-claim-release-guard/audit-log-flush-keyword-probecorpus cases:untouched as corpus content.
review-suite/evals/baseline/v1/,gate-manifest.json,DECISION-RECORD.md,FAILURE-TAXONOMY.md,audits/, and every existingreview-suite/evals/v2/measurement document (S1-ABLATION-MATRIX.md,FROZEN-V2-CONFIGURATION.md,CLOSEOUT-REPORT.md,discriminating-case-validation.md, etc.): untouched.carve-changesets: not invoked — this candidate fit an ordinary single PR.Test plan
just format— all checks passed, no reformatting neededjust lint—skills-ref validatepassed for all 8 skills; pluginpackaging validation passed
just test— 432 tests acrossreview-suite/scripts/tests(310) andall 6 skill-level
scripts/testssuites (122), all passingreview-code-changecycle at the final head — cleanaggregate verdict, zero findings across all three lenses
(solution-simplicity, correctness, code-simplicity)
Fixes #93