feat(code-review): scope backstop and repeat runs to the unreviewed delta - #2199
Merged
Merged
Conversation
…elta Implements #2167 (part of #2164): a new scripts/verdict_scope.py resolver consults the per-lens verdict ledger (#2166) before dispatching a review lens, skipping only an exact (lens, file, current-content-hash) match whose most-recent ledger row is `pass`. Wired into /code-review step 4's per-agent dispatch (with loud, never-silent reporting via a new `ledgerSkipped` output-format field) and into /build's sub-steps 4/6 checkpoints (which now also emit the dispatch-prompt scope marker so their own dispatches get recorded), so Step 6's backstop scopes to the unreviewed delta for free. Backstop review (correctness/security/structure/test/test-smell/ spec-compliance/doc/arch-review) surfaced and fixed: - _load_json_arg crashed (ENAMETOOLONG) on a realistic multi-lens, multi-file --lens-files payload; now tries an inline JSON parse first. - The ledger's own file_path/hash canonicalization was writer-only; added a shared hooks/lib/review_verdicts.canonical_path used by both the writer and the reader, closing a silent-miss (never a false skip) gap. - The verdict recorder could record `pass` for an in-scope file on a non-clean (fail/warn) lens result when no issue happened to name that file; it now marks every in-scope file `findings` in that case. - The ledger is untrusted, unauthenticated local state; load_verdicts now refuses a ledger that is tracked by git, closing a self-certification path where a forged, committed row could suppress security-review. - Fixed a field-naming mismatch between output-format.md's ledgerSkipped example and the ledger's real snake_case schema. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KGHpm7gtqNb8NM2Smf76u7
Merged
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
2 participants
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
scripts/verdict_scope.pyresolver consults the per-lens verdict ledger (feat(hooks): record per-lens review verdicts bound to a content hash #2166) and skips dispatching lensLagainst fileFonly on an exact(L, F, current-content-hash)match whose most-recent row ispass— fail-closed everywhere else./code-reviewstep 4 (per-agent dispatch, with a newledgerSkipped--jsonfield and "report loudly, never silently" prose) and into/build's sub-steps 4/6 checkpoints (which now also emit the dispatch-prompt scope marker so their own dispatches get recorded) — Step 6's backstop scopes to the unreviewed delta for free, with no Step-6-specific code._load_json_argcrashed (ENAMETOOLONG) on a realistic multi-lens, multi-file--lens-filespayload — verified by reproduction before fixing.hooks/lib/review_verdicts.canonical_pathused by both the writer andverdict_scope.py's reader.passfor an in-scope file on a non-clean (fail/warn) lens result when no issue happened to name that specific file; it now marks every in-scope filefindingsin that case.load_verdictsnow refuses a ledger tracked by git, closing a path where a forged, committed row could suppresssecurity-reviewon a malicious file.output-format.md'sledgerSkippedexample and the ledger's real snake_case schema.Test Plan
python3 -m pytest plugins/dev-team/tests tests/repo tests/agents tests/commands tests/docs tests/knowledge tests/stack_aware tests/skills tests/scripts tests/hooks -q— 11239 passed, 0 failed, 48 skipped.scripts/check_md_references.pyandplugins/dev-team/hooks/lib/build_knowledge_index.pyclean.ruff checkclean on all new/changed Python files.--lens-filescrash reproduced against unpatched code, then confirmed fixed.emit_review_verdict-written ledger (test_cli_warm_run_after_real_pass_verdicts_skips_unchanged_files_only).resolve_dispatch's gate can actually fail (per this repo's own "a gate that cannot fail is worse than no gate" rule).pre-pushlocal CI gate (full suite + hook/script unit tests + static checks) green before push.Closes #2167
Part of #2164
🤖 Generated with Claude Code
https://claude.ai/code/session_01KGHpm7gtqNb8NM2Smf76u7
Generated by Claude Code