Skip to content

fix(#45): tighten review verdict criteria for medium findings#49

Open
fullsend-ai-coder[bot] wants to merge 2 commits into
mainfrom
agent/45-fix-review-verdict-consistency
Open

fix(#45): tighten review verdict criteria for medium findings#49
fullsend-ai-coder[bot] wants to merge 2 commits into
mainfrom
agent/45-fix-review-verdict-consistency

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Replace the subjective "multiple medium-severity findings which could affect the intended outcome" criterion with an objective rule: two or more medium findings, at least one identifying a functional bug, require request-changes.

Add a self-consistency check to the pr-review synthesis step: if the summary text states findings should be addressed before merge, the verdict must be request-changes, not comment.

Update comment-only criteria to clarify it applies only to stylistic, advisory, or process-related medium findings.

Changes:

  • agents/review.md: Outcome section verdict criteria
  • skills/pr-review/SKILL.md: Step 6f verdict logic and
    self-consistency check

Note: pre-commit could not run (shellcheck-py failed to install due to sandbox network restrictions). Secret scan passed.


Closes #45

Post-script verification

  • Branch is not main/master (agent/45-fix-review-verdict-consistency)
  • Secret scan passed (gitleaks — 2540ede96537b32447321564bec60226efca120e..HEAD)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

Replace the subjective "multiple medium-severity findings which
could affect the intended outcome" criterion with an objective
rule: two or more medium findings, at least one identifying a
functional bug, require request-changes.

Add a self-consistency check to the pr-review synthesis step:
if the summary text states findings should be addressed before
merge, the verdict must be request-changes, not comment.

Update comment-only criteria to clarify it applies only to
stylistic, advisory, or process-related medium findings.

Changes:
- agents/review.md: Outcome section verdict criteria
- skills/pr-review/SKILL.md: Step 6f verdict logic and
  self-consistency check

Note: pre-commit could not run (shellcheck-py failed to
install due to sandbox network restrictions). Secret scan
passed.

Closes #45
@ben-alkov

Copy link
Copy Markdown
Member

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 9, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:54 PM UTC · Completed 9:09 PM UTC
Commit: e8381e3 · View workflow run →

@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

Medium

  • [logic-error] agents/review.md:238 — The new verdict criteria create a logical gap for exactly one medium finding that identifies a functional bug. The request-changes criterion requires "two or more" medium findings with at least one functional bug. The comment-only criterion explicitly excludes functional bugs ("not when any medium finding identifies a functional bug"). A single medium functional-bug finding satisfies neither condition, creating a contradiction. The same gap exists in the parallel criteria in skills/pr-review/SKILL.md.
    Remediation: Add explicit handling — either lower the request-changes threshold to include one functional-bug medium, broaden comment-only to permit it, or add a dedicated bullet for this case.

  • [protected-path] agents/review.md, skills/pr-review/SKILL.md — Both files are under protected paths (agents/, skills/). The PR links to issue Review agent verdict contradicted its own analysis on PR #32: chose comment despite saying findings should be addressed before merge #45 and explains the rationale. Human approval is always required for protected-path changes.

Low

  • [stale-reference] skills/code-review/SKILL.md:279 — The code-review skill retains the old verdict criterion ("Multiple medium findings which could affect the intended outcome") that this PR replaces in agents/review.md and skills/pr-review/SKILL.md. The skill's override clause (line 298) deferring to agents/review.md mitigates functional impact, but the textual inconsistency may cause confusion.

  • [pattern-inconsistency] agents/review.md:238 — The new multi-part conditional introduces semantic content qualification into verdict criteria that previously used only severity counts. This is intentional per issue Review agent verdict contradicted its own analysis on PR #32: chose comment despite saying findings should be addressed before merge #45 but diverges from the established pattern.

  • [pattern-inconsistency] skills/pr-review/SKILL.md:779 — Same pattern divergence in the parallel verdict criteria text.


Labels: PR fixes a review verdict bug and modifies protected agent/skill definitions

@fullsend-ai-review fullsend-ai-review Bot 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.

See the review comment for full details.

Comment thread agents/review.md Outdated
- `request-changes` — findings *requiring* resolution: one or more critical or
high findings; multiple medium-severity findings which could affect the
intended outcome of the PR
high findings; two or more medium-severity findings where at least one

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[medium] logic-error

The new verdict criteria create a logical gap for exactly one medium finding that identifies a functional bug. The request-changes criterion requires two or more medium findings with at least one functional bug. The comment-only criterion explicitly excludes functional bugs. A single medium functional-bug finding satisfies neither condition, creating a contradiction. The same gap exists in skills/pr-review/SKILL.md at the parallel criteria.

Suggested fix: Add explicit handling for one medium functional-bug finding: either lower the request-changes threshold to include it, broaden comment-only to permit it, or add a dedicated bullet for this case. Apply the same fix to skills/pr-review/SKILL.md section 6f.

Comment thread agents/review.md Outdated
Comment thread skills/pr-review/SKILL.md Outdated
@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment bug Something isn't working labels Jul 9, 2026
@ben-alkov

Copy link
Copy Markdown
Member

/fs-fix lower the request-changes threshold to include one functional-bug medium finding; update outdated language in "skills/code-review"; ignore all other findings

@fullsend-ai-coder

fullsend-ai-coder Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 9:58 PM UTC · Completed 10:05 PM UTC
Commit: e8381e3 · View workflow run →

…m finding

- agents/review.md: request-changes now triggers on one or more medium
  findings identifying a functional bug (was: two or more)
- skills/pr-review/SKILL.md: same threshold change in step 6f
- skills/code-review/SKILL.md: update outdated verdict language to match
  new criteria (was still using old 'multiple medium findings which
  could affect the intended outcome' wording)

Addresses review feedback on #49
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 1 (human-triggered)

Addressed all 2 human-directed findings. Lowered the request-changes threshold from two-or-more to one-or-more functional-bug medium findings across agents/review.md and skills/pr-review/SKILL.md, and updated outdated verdict language in skills/code-review/SKILL.md to match. All other review findings ignored per human instruction.

Fixed (3):

  1. lower request-changes threshold to one functional-bug medium finding (agents/review.md): Changed 'two or more medium-severity findings where at least one identifies a functional bug' to 'one or more medium-severity findings identifying a functional bug' so a single functional-bug medium finding triggers request-changes
  2. lower request-changes threshold to one functional-bug medium finding (skills/pr-review/SKILL.md): Changed 'Two or more medium findings, at least one of which identifies a functional bug' to 'One or more medium findings identifying a functional bug' and updated the comment-only bullet to 'One or more medium findings that are all stylistic/advisory/process-related'
  3. update outdated language in skills/code-review (skills/code-review/SKILL.md): Replaced outdated verdict criteria ('Multiple medium findings which could affect the intended outcome of the PR' and 'One medium finding (but no critical/high)') with new criteria matching the lowered threshold: one or more medium findings identifying a functional bug triggers request-changes; stylistic/advisory/process-related medium findings get comment-only

Tests: passed

Updated by fullsend fix agent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Review agent verdict contradicted its own analysis on PR #32: chose comment despite saying findings should be addressed before merge

1 participant