Skip to content

review-branch: Tighten threshold for raising Suggestions to require a concrete improvement #674

Description

@williamthorsen

Problem

The review-branch (and shared review) skill produces Suggestion findings that propose stylistic variations rather than concrete improvements. Recent observed cases:

  • Proposed dropping an explicit Dirent annotation in favor of inferred types, with the reasoning "either form is defensible" and no codebase precedent pulling either way.
  • Proposed switching pnpm exec relay-compiler to a direct binary path, with the reasoning "slightly faster startup" and no measured impact — and contrary to the project's established pnpm exec convention.

A related class of drift also occurs at the Warning tier (e.g., a typo in a comment flagged as a Warning), where the existing Gate filters mechanical oversights but not cosmetic non-risks.

Each such finding consumes reviewer time, user time, response tokens, and adds permanent noise to the review artifact (which is part of the project's documentation trail). The cumulative tax across reviews is non-trivial.

Considerations

  • A valid code choice is not a defect. The bar for raising a finding — even a Suggestion — should be evidence that the suggested change is an improvement, not merely an alternative.
  • Anchoring criteria already exist. The canonical finding scheme defines Suggestions as "optional improvements" and Recommendations as "advisable but discretionary." The skill's behavior drifts from its own definition.
  • Codebase conventions are the cheap signal. Most of these noise findings would be filtered out by a single check: does the codebase have an established convention here that the change violates? If no, and there's no measured impact, the finding shouldn't be raised.
  • The principle is universal across change-proposing tiers. W/T/R/S all assert that something should change; F asserts a defect. The "improvement, not alternative" gate naturally applies to W/T/R/S, with the evidence bar tightening as severity decreases.
  • Self-correction in conversation is too late. The artifact is saved before the user sees it; editing it after the fact still leaves an edit trail and burns iterations. The threshold needs to apply at finding-generation time.
  • Skill-local reinforcement. Global prose rules land less reliably than point-of-use language. The categories most prone to drift (R, S, and W in cosmetic-risk cases) should carry the gate inline, not just inherit it from a top-level rule.

Proposed solution

Add a universal "authored-choice" gate to the canonical finding scheme in _data/artifact-conventions.md (where Suggestion and the existing Warning Gate are defined), plus per-category Gate lines at the point of use:

  1. A single universal gate at the top of the "Category criteria" section, scoped to W/T/R/S, stating that a proposed change must be a genuine improvement, not merely an alternative — and that the per-category criteria set the evidence bar.
  2. Augment the existing Warning Gate to also require a defensible risk of functional or maintainability harm (closing the typo-as-warning class).
  3. Add a Recommendation Gate requiring the change be plausibly better (improved clarity, reduced complexity, better-aligned pattern).
  4. Add a Suggestion Gate requiring alignment with a codebase convention the code violates, measurable improvement evidence, or a widely accepted external standard.

The principle propagates to review-criteria/SKILL.md and review-branch/SKILL.md via their existing references to the canonical scheme — no edits required there.

Acceptance criteria

  • _data/artifact-conventions.md carries a universal "authored-choice gate" introducing the "improvement, not alternative" principle scoped to W/T/R/S, at the top of the "Category criteria" section.
  • Warning's existing Gate is augmented to require defensible risk of functional or maintainability harm in addition to filtering mechanical oversights.
  • Recommendation carries a Gate line ruling out "another valid way to write it" justifications.
  • Suggestion carries a Gate line ruling out "another valid way to write it" justifications, naming the three acceptable evidence bars (codebase convention violation, measurable improvement, widely accepted external standard).
  • FIXME criteria are unmodified (defect claim, not improvement claim — exempt by definition).

Metadata

Metadata

Labels

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions