Skip to content

fix(code-review-gate): restate the effort rule by what effort selects - #248

Merged
ultimatile merged 2 commits into
mainfrom
fix/219-code-review-gate-effort
Oct 4, 2026
Merged

ultimatile merged 2 commits into
mainfrom
fix/219-code-review-gate-effort

Conversation

@ultimatile

@ultimatile ultimatile commented Oct 4, 2026 •

Copy link
Copy Markdown
Owner

Summary

code-review-gate runs 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-pipeline invokes 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:

  • The effort input is medium or high, and states what each reports: medium only the findings the review is most confident in; high covers more and may report findings it is less sure of.
  • The caller passes high when at least one behavior the change alters meets at least one of three conditions, and medium otherwise:
    1. it cannot be exercised in the caller's current execution environment with the inputs, scale, and dependencies of its intended use;
    2. it changes persistent external state that reverting the change does not restore;
    3. it computes a numerical or other non-prose data result whose value no test verifies.
  • The caller judges those conditions from its prompt, its memory, and its knowledge of the project.
  • A gate loop is a run of the gate together with each re-run that follows it, and each run is one iteration of its loop.
  • A run made to review a fix for an earlier run's findings is a re-run, in that earlier run's loop. Every other run starts a new loop.
  • Effort is held fixed across a gate loop.
  • A lane abandoned in one run stays abandoned for the rest of the gate loop.
  • The subagent lane tells its reviewer which findings to report by passing on what the effort input says the level reports, and at high also tells it to go through the gate scope, the branch's diff plus uncommitted and untracked changes, hunk by hunk.

.claude-plugin/marketplace.json carries version 2026.10.6.

Impact

review-pipeline is 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-branch reaches the gate through review-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.md returns 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.
  • Two existing test suites, run after the edit: bash skills/gh-body-audit/test-body-math-scan.sh prints All checks passed., and uv run pytest -q in skills/docreaper reports 97 passed.

Notes

  • What the levels report. Claude Code's documentation for /code-review says that at low and medium the review "reports only the findings it's most confident in", and that high through max "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 how low differs from medium.
  • Why these three conditions. In review-pipeline the 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. high is 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.
  • Why effort is held fixed. 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.
  • Why the loop ends where the re-runs end. The gate stores neither the effort a loop started with nor the lanes it has abandoned; both live in the session running the fix and re-run cycle, so that cycle is the extent over which the gate can keep them. A run that starts over, on the same branch or another, chooses its effort and builds its lane chain afresh.
  • The review-pipeline call 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.

…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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 Medium severity

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 medium or high.
  • 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.

Comment thread skills/code-review-gate/SKILL.md Outdated
…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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The instruction changes are internally consistent, align with the caller workflow, and include the required version bump.

Review effort: Balanced
Findings: None

Resolved since last review (1)

@ultimatile
ultimatile merged commit 09af9b4 into main Oct 4, 2026
1 check passed
@ultimatile
ultimatile deleted the fix/219-code-review-gate-effort branch October 4, 2026 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

code-review-gate: the effort rule does not match what effort selects

2 participants