refactor(code-review): excise inert Premise category layer; SCHEMA_VERSION 2 (v3.0.0) - #168
Merged
Merged
Conversation
…RSION 2 The premise reviewer was removed earlier, so nothing emits `category: "Premise"` — the entire category-processing layer was dead code. This removes it and bumps the finding/result schema to v2. Removed: - `Premise` from the CATEGORIES schema enum. - The two verdict rules that gated on Premise findings (priority-0 -> CHANGES_REQUESTED; cumulative-Premise -> NEEDS_ATTENTION) plus `_count_gateable_premise_medium`, the `premise_cumulative_medium` threshold, and its `justification_rate_alert` companion. - Premise-scoped `stats` telemetry (`by_subcategory`, `justification`, `premise_cumulative_medium_count`) and the helpers that built them. - The verifier "always verify Premise" eligibility branch and the Premise extra-strictness blocks in verifier_prompt.txt. - The four `premise_*.md` fix templates + their /fix dispatch rows, and the reserved `golden_premise_*` fixtures. - Premise category-layer refs across shared/impact prompts, present-local and github-review rendering, SCHEMA.md, README.md, and start.md. Kept (general, not premise-specific): - The "PREMISE:" reasoning step every reviewer uses. - The author-justification machinery: `justified[]`, JUSTIFIED-VALID/ INVALID, the verifier J1/J2 audit, and the justification-audit learning stream (its `premise-justifications.jsonl` filename is kept stable for the self-learning consumer). SCHEMA_VERSION 1 -> 2 (folded into the prompt/cache hash, so BHA + verification caches invalidate once on rollout). verdict-thresholds.json now exposes only `impact_cumulative`; `stats` keeps `impact_cumulative_count`. Tests: 1209 pass, ruff + pyright clean. Deleted the dead premise test classes, cleaned stale fixtures to valid vocabulary, and regenerated the golden + run_plan snapshots (schema_version 1->2). Version 3.0.0 (MAJOR: breaking change to the review_result.json contract). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…excise-premise-category # Conflicts: # CHANGELOG.md # plugins/code-review/.claude-plugin/plugin.json
…ion effort can't starve reviewers Reasoning effort, unlike model, has no per-Task override — a spawned subagent inherits the session effort unless its own frontmatter pins one. The two worker shells (code-review-worker, code-review-worker-graph) carried no effort, so running /code-review from a lowered-effort session would silently drop every spawned reviewer's reasoning depth to that lower level, degrading review quality even though per-spawn model routing keeps their model identity intact. Pin effort: high on both worker defs (valid on both Opus and Sonnet, the two route-assigned reviewer models) so reviewer reasoning depth is fixed regardless of session effort. This is the effort-axis analogue of the per-spawn model insulation and makes the "run /code-review from a cheap Sonnet session" cost path safe to combine with a lowered session effort. Document the effort axis in start.md and README alongside the existing session-model cost guidance. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…gory excision Follow-up cleanup for the v3.0.0 Premise excision, addressing stale artifacts left behind: - Remove the four now-unreachable Premise reasoning-certificate kinds (necessity, cohesion, workaround, complexity) from REASONING_CERTIFICATE_KINDS; no producer emits them and the constant was never referenced. - Retarget the orphaned PLN-773 stats test from the removed stats.justification / stats.by_subcategory premise sub-blocks to the live stats.verification sub-block (justified_valid_count / justified_invalid_count / by_reviewer). - Disambiguate the cumulative Impact verdict gate's cross-references: start.md and README.md now name it (FEA-1401 / PLN-726 OQ#6) instead of a bare "Rule 6" ordinal that collided with SCHEMA.md's renumbered sequential precedence list (Impact is item 4 there; Rule 6 = APPROVED). Add a numbering note to SCHEMA.md recording that _compute_canonical_verdict's plan-derived rule labels (2.5, 3.5, "Rule 6") do not map 1:1 to the table's sequential numbers. The code and test labels are left intact -- internally consistent and OQ#6-tagged. - Fix the SCHEMA.md deferred-fixture count to match _DEFERRED_FIXTURES (3 deferred; golden_injection_quarantine was promoted to a shipped end-to-end fixture by PLN-720). No behavior change. 1209 tests pass; ruff + pyright clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… v1 round-trip fixture Two more residual-Premise-excision follow-ups: - github-review.md's Verifier Stats block was missing the Impact gateable count line that present-local/SKILL.md already shows. The Premise cumulative-gate display removed from both presenters was replaced only in local mode, leaving GitHub-mode operators with no line for stats.impact_cumulative_count (the envelope's sole operator-tunable verdict-gate count). Mirror the local-mode line before the Partition mode line. - Clarify the golden_schema_v1_round_trip fixture's config description. The SCHEMA_VERSION 1->2 bump restamps the envelope to 2 while each finding's own stamp is preserved, so the fixture round-trips a schema_version: 1 finding through a schema_version: 2 envelope. The finding is genuinely v1 (same as every other golden fixture's findings), so the name/"v1" are accurate -- not renamed; the description now states the v1-finding / v2-envelope split explicitly so it reads as the backward-compat probe it is. No behavior change. 1209 tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Why
PR #166 removed the premise reviewer, so nothing emits
category: "Premise"anymore — the entire category-processing layer (verdict gates, telemetry, schema enum, verifier policy, fix templates) became dead code. This is the deferred fast-follow that excises it.Scope boundary (the key call)
The justification subsystem turned out to be general, not premise-only — the verifier's J1/J2 audit,
JUSTIFIED-VALID/JUSTIFIED-INVALID, and thejustified[]bucket apply to any author-justified finding; the premise reviewer just had extra strictness layered on top. So I kept the general machinery and removed only the premise-specific pieces.PremisefromCATEGORIESenum_count_gateable_premise_mediumjustified[]bucket +JUSTIFIED-VALID/INVALIDverdictspremise_cumulative_medium+justification_rate_alertthresholdsstats.by_subcategory,stats.justification,premise_cumulative_medium_countpremise-justifications.jsonlfilename kept stable for the self-learning consumer)stats.verification.justified_valid_count/justified_invalid_countpremise_*.mdfix templates +/fixrows; reservedgolden_premise_*fixturesSchema bump (per your call)
SCHEMA_VERSION1 → 2 for theFinding+ResultEnvelopecontract — the category vocabulary and telemetry keys genuinely changed. Because the schema version is folded into the prompt/cache hash, BHA + verification caches invalidate once on rollout (already busted by PR #166's prompt change, so effectively free).verdict-thresholds.jsonnow exposes onlyimpact_cumulative;statskeepsimpact_cumulative_countin place of the removed premise count.This is the breaking change that makes it a MAJOR bump (
2.37.0 → 3.0.0): a consumer ofreview_result.jsonpinned to schema v1 / expecting thePremisecategory or the removedstatskeys would need to update.Validation
TestLoadVerdictThresholdsto the survivingimpact_cumulativekey, cleaned stale fixtures to valid vocabulary (nocategory="Premise"or removed config remains), and regenerated the golden fixtures + run_plan snapshots (schema_version1→2 only — diffs verified scoped).Note
/coststill listspremiseas a reviewer role: intentional — the cost tool reports it for historical transcripts (pre-PR #166 reviews that did run premise). New reviews won't produce it.🤖 Generated with Claude Code