Repository navigation
fix(code-review-gate): restate the effort rule by what effort selects - #248
Merged
Merged
Conversation
…loop Effort trades precision for recall, so diff size is not a reason to raise it; only the cost of a missed defect is. State the two levels in scope. The gate runs before a pull request exists, so the effort pin and lane abandonment are scoped to a gate loop, the runs of the gate over one branch, instead of to a pull request.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The branch-lifetime loop requires state that is not persisted across sessions or independent pipeline runs.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Updates code-review-gate so review effort reflects missed-defect cost and gate state is scoped to a defined loop.
Changes:
- Restricts effort to
mediumorhigh. - Replaces PR-based wording with gate-loop terminology.
- Bumps the plugin version to
2026.10.6.
| File | Description |
|---|---|
skills/code-review-gate/SKILL.md |
Revises effort and lane-abandonment rules. |
.claude-plugin/marketplace.json |
Bumps the plugin version. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ck cheaply The effort input now states what each level reports and names three conditions that select high: a behavior the change alters cannot be exercised in the caller's environment at its intended scale, changes external state a revert does not restore, or computes a data result no test verifies. The subagent lane takes the level's meaning from the effort input instead of carrying its own. A gate loop is a run together with the re-runs that review fixes for its findings; any other run starts a new loop.
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
code-review-gateruns a code review through a chain of fallback lanes, each a different way of getting the review run. It raised its review effort for a large diff, and scoped two of its rules to "a PR's iterations". Neither holds up. Effort trades precision for recall, so diff size is not what decides whether raising it pays. And the gate runs before a pull request exists:review-pipelineinvokes it in a phase that comes before the one that opens the pull request.The effort input now says what each level reports and names the conditions that select
high, and both rules are scoped to a run of the gate and the re-runs that review fixes for its findings.Closes #219
Changes
In
skills/code-review-gate/SKILL.md:mediumorhigh, and states what each reports:mediumonly the findings the review is most confident in;highcovers more and may report findings it is less sure of.highwhen at least one behavior the change alters meets at least one of three conditions, andmediumotherwise:highalso tells it to go through the gate scope, the branch's diff plus uncommitted and untracked changes, hunk by hunk..claude-plugin/marketplace.jsoncarries version2026.10.6.Impact
review-pipelineis the gate's one direct caller. It passes "an effort chosen on the gate's terms", so it picks up the new conditions with no edit.land-via-integration-branchreaches the gate throughreview-pipeline, once per pull request, each on its own branch.Verification
No test executes this text; the change is to instructions only.
rg -n '\bPR\b' skills/code-review-gate/SKILL.mdreturns no match. Before the change it matched three times: once in the effort input's definition, and twice in the paragraph that states when a lane is abandoned.bash skills/gh-body-audit/test-body-math-scan.shprintsAll checks passed., anduv run pytest -qinskills/docreaperreports 97 passed.Notes
/code-reviewsays that atlowandmediumthe review "reports only the findings it's most confident in", and thathighthroughmax"broaden coverage and may include findings the review is less sure about". The effort input restates that for the two levels the gate uses. That page gives no rule for choosing between them, and does not say howlowdiffers frommedium.review-pipelinethe gate is followed by two more reviewers, a Codex review and a Copilot review, that also read the change without running it, so a defect a reader can see can still be caught one review round later. What none of them catches is a defect only execution reveals.highis worth its extra uncertain findings where that first execution is not available to the caller, cannot be undone, or would not expose the defect at all.review-pipeline's fix loop re-runs the review fresh after each fix and compares one iteration's actionable topics against the previous iteration's, halting when a topic recurs. If effort moved between iterations, a topic could appear or vanish because coverage changed and not because the code did, and that comparison would stop meaning anything. The skill states the scope of the pin and leaves this reason out, since the scope is all an executor needs.review-pipelinecall site is unchanged. code-review-gate: the effort rule does not match what effort selects #219 proposed dropping that caller's own copy of the pin once the gate stated its scope. The sentence there now re-runs the gate "with the inputs step 2 passed", which also carries the gate's other two inputs: the root the change is measured from, and the working tree that holds the change. The gate does not state those two as fixed, so removing the sentence would leave a re-run's inputs unspecified.