Skip to content

Reviews with only discretionary findings render no menu and trigger no fix cycle #1293

Description

@williamthorsen

Problem

Two tier lists assert that some findings are not actionable, contradicting the Actionability gate that every finding has already cleared.

_partials/next-steps-after-review.md:151 renders the Findings sub-block only for F, W, or T. The sibling sub-blocks gate independently, so a review whose findings are all Recommendations and Suggestions trips none of the three conditions and renders no next-steps block at all: no implement-directly option, no author-handoff routing, no recommendation gradient.

_data/artifact-conventions.md:831 maps a review holding only S to criticality none, glossed "No actionable findings". orchestrate/modules/review-cycle.md:302 and :480 read that as report-only with no fix attempt, so an orchestrated run silently skips the fix cycle for a suggestion-only review.

Both contradict the Actionability gate, which requires every emitted F/W/T/R/S finding to "hand the author a concrete decision they can act on in the change under review". Every authored finding is actionable by construction. Discretionary means the user chooses; suppressing the menu and skipping the fix cycle convert that into dropped.

Context

The scoring model already treats these tiers as actionable. Per review-branch/SKILL.md:277, every authored finding lowers the score, "weighted by severity from F (heaviest) down to S (lightest). No tier is score-neutral."

The presentation gate dates to #331 (2026-03-17) and has never been edited. #838 turned it into a contradiction: before it, the review template said Recommendations and Suggestions "don't count against score", and the PR that deleted those carve-outs also added the self-challenge step that leaves clean reviews carrying only well-justified R/S.

#1180 is absorbed here, and should be closed as subsumed. It diagnoses the Findings sub-block's first option as a conditionless default that structurally outranks every conditioned option. Opening the gate to discretionary-only reviews makes that failure mode fire more often, since a discretionary finding is frequently the trivial determinate case where implementing directly is correct. Both tickets rewrite the same fifteen lines.

Proposed solution

State the presentation invariant rather than extending the tier list: severity orders how findings rank and what blocks merge, and never decides whether they are shown or whether the user can act on them. Deriving the trigger from the Actionability gate rather than enumerating tiers is what keeps a later scheme change from silently re-breaking it.

Correct the criticality mapping so none means no authored findings:

Highest finding present Criticality Meaning
None, or only -L none No authored findings
S, R, and/or T (no W/F) low Deferrable items available

Legacy-only stays none, keeping unattended fix cycles out of pre-existing code.

Floor the approval threshold at medium wherever it is set: the engine default, orchestrate-review's default, and orchestrate-dev's high-effort preset. Promoting S into low otherwise lets a review holding only deferrable findings gate approval and exit needs_manual_review, contradicting the finding scheme's Merge-blocking column, which reads "Never" for T, R, and S alike. The contradiction predates this change for T and R; promoting S extends it to the tier defined as optional, and one value in three places resolves all three tiers. Budget-threshold stays at low and keeps descending with effort, so those tiers still draw opportunistic fix cycles.

Condition every option in the Findings recommendation rules, and separate two things those rules conflate: whether a change is optional, and who decides. An S reading "rename x to descriptiveName" is discretionary but fully determinate; the only decision is yes or no, and the person reading the menu is the one making it.

Bound "Implement directly" on determinacy and record why. Implementing forfeits the second look, which is tolerable only when the finding's Recommendation states the exact change, so the fix's diff is the finding restated and a reviewer would be re-reading text the review already contains. Absent that rationale, the option creeps into findings that need judgment.

Add a follow-up-ticket disposition to both variants, routed per scope-and-deferral.md, which is already cited as governing a deferred finding while no option leads there.

A legacy-only review renders the sub-block with the full option pool, not a reduced one. Trimming the author hand-offs would leave implementing-in-place as the only route and remove the adversarial second look, which pre-existing code needs at least as much as authored code: no ticket criterion constrains a legacy fix and no design discussion stands behind it.

No authorship detection: agents commit under the user's git identity, so the review's resolved $author cannot distinguish the user from an agent session.

Acceptance criteria

Must have

  • A review whose findings are all R and/or S renders the Findings sub-block, with numbered options and strength markers.
  • A legacy-only review renders the sub-block with the full option pool; a review with no findings of any kind renders no next-steps block.
  • The trigger is stated as a principle derived from the Actionability gate rather than as a list of tiers.
  • Both entry-point variants, review-branch and review-pr, are covered.
  • Every option in the Findings recommendation rules carries a firing condition; none is labelled a default without one.
  • "Implement directly" is conditioned on determinacy, and the spec records that forfeiting the second look is what the condition bounds.
  • The marker-strengths tiebreaker applies only where the conditions leave two options in balance, and never overrides a condition that fired.
  • Both variants offer a follow-up-ticket disposition, routed per scope-and-deferral.md.
  • Criticality none means no authored findings, and S maps to low alongside R and T.
  • Wording glossing none as "no actionable findings" is corrected wherever it appears.
  • Worked examples render a discretionary-only review and an all-trivial review, the latter carrying the strongest marker on "Implement directly".
  • No default and no effort preset gates approval on T, R, or S, and the finding scheme's per-finding criticality column agrees with the overall rollup. low stays selectable for a project that deliberately wants those tiers to gate.

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions