docs(review): skip the challenger when there are no findings

[guyoron1]

Heyaa : )

Noticed the challenger — a serial opus dispatch whose only inputs are the merged findings — runs even on a clean PR, where it adjudicates an empty list for real money and latency.

Two hunks in skills/pr-review/SKILL.md:

1. Step 6d: if the merged finding set from 6a–6c is empty, skip the challenger dispatch entirely and proceed to the verdict with the empty set. Applies regardless of why it's empty — a dimension failure already surfaces via the existing sub-agent-failure finding. Dispatch mechanics, output parsing, and the fallback-on-failure path are untouched.
2. The re-review rule updated to match: "always dispatch when findings exist".

Verdict logic unchanged — an empty set approved before and approves now. The skip is noted in the orchestrator's own reasoning trace, not the review body: step 7 forbids footers and the result schema is additionalProperties: false, so there's no honest place for it in the output — auditable in the transcript, invisible to the PR author.

One trade-off, disclosed rather than buried: this forfeits the challenger's secondary, explicitly not-owned allowance to flag something it happens to notice while reading the diff. Exercising that on every clean PR would mean re-reading the whole diff each time — exactly the cost this skip exists to remove.

Prompt-only change; the challenger exists only on the claude review path, so this is complete for the only path that has one to skip.

Refs #1144

[github-actions]

Functional tests did not run

Functional tests run automatically for org/repo members and collaborators on pull requests.

For other contributors, a maintainer must add the ok-to-test label after the latest push.

[qodo-code-review]

PR Summary by Qodo

Skip challenger dispatch when review findings are empty

✨ Enhancement 📝 Documentation 🕐 Less than 10 minutes

AI Description

• Skip challenger dispatch when merged review findings are empty, reducing serial model cost and
 latency.
• Preserve approval behavior while recording the skip only in orchestrator reasoning for
 auditability.
• Align re-review dispatch guidance with the findings-dependent challenger rule.

Diagram

graph TD
  A["Merged findings"] --> B{"Findings exist?"}
  B -- Yes --> C["Challenger pass"] --> D["Final verdict"]
  B -- No --> E["Record skip"] --> D

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Always dispatch the challenger

• ➕ Preserves the possibility of discovering incidental issues on otherwise clean reviews.
• ➕ Keeps challenger invocation behavior unconditional.
• ➖ Pays serial model cost and latency to adjudicate an empty list.
• ➖ Requires rereading the diff despite having no findings to challenge.

2. Sample clean reviews

• ➕ Retains occasional independent checks for false-negative monitoring.
• ➕ Reduces cost compared with unconditional dispatch.
• ➖ Adds sampling policy and observability complexity.
• ➖ Produces inconsistent latency and does not directly serve the challenger's adjudication role.

Recommendation: Use the PR's findings-dependent dispatch. It directly matches the challenger's primary responsibility, removes deterministic waste on clean reviews, preserves verdict semantics, and records the decision internally. Sampling clean reviews could be introduced separately if false-negative measurement becomes an explicit requirement.

Files changed (1) +22 / -1

Documentation (1) +22 / -1

SKILL.mdGate challenger dispatch on non-empty findings +22/-1

Gate challenger dispatch on non-empty findings

• Documents that the challenger is skipped when merged findings are empty and that the skip is recorded only in orchestrator reasoning. It also aligns re-review dispatch guidance while preserving challenger behavior, failure handling, and verdict logic for non-empty findings.

skills/pr-review/SKILL.md

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (2)

1. Empty set bypasses checks ✓ Resolved 🐞 Bug ≡ Correctness

Description

The new instruction says to proceed straight to the verdict with an empty set, bypassing step 6e
checks that run after the challenger and can independently add findings. A clean dimension review
could therefore approve a protected-path or scope-unauthorized PR without emitting the mandatory
finding.

Code

skills/pr-review/SKILL.md[R831-833]

+from steps 6a–6c is empty, skip the challenger dispatch entirely and
+proceed straight to the verdict with the empty set — the empty-findings
+approval path (step 7) is unchanged; this skip does not add anything to

Relevance

●●● Strong

The branch appears to bypass later mandatory checks that can independently produce findings.

PR-#49

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The added branch explicitly directs an empty merged set straight to the verdict, but step 6e
requires PR-specific checks after the challenger pass. Those checks include mandatory protected-path
findings, and step 6f determines the outcome only after merging findings produced by those checks.

skills/pr-review/SKILL.md[830-849]
skills/pr-review/SKILL.md[937-941]
skills/pr-review/SKILL.md[984-1010]
skills/pr-review/SKILL.md[1099-1115]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The empty-findings branch says to proceed directly to the verdict, which can skip mandatory orchestrator-only checks that occur after the challenger and may add findings.

## Issue Context
Only the challenger dispatch should be skipped. The workflow must continue through steps 6e, 6e-1, and 6f before reaching the verdict.

## Fix Focus Areas
- skills/pr-review/SKILL.md[830-849]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Challenger skip lacks tests 📜 Skill insight ▣ Testability

Description

The operational review skill now skips the challenger when findings are empty, but the PR includes
no test change covering that behavior. A test should prove both that empty findings suppress
dispatch and that non-empty findings still dispatch the challenger.

Code

skills/pr-review/SKILL.md[R830-833]

+**Skip when there is nothing to adjudicate.** If the merged finding set
+from steps 6a–6c is empty, skip the challenger dispatch entirely and
+proceed straight to the verdict with the empty set — the empty-findings
+approval path (step 7) is unchanged; this skip does not add anything to

Relevance

●● Moderate

No close test-coverage precedent found for behavioral changes in operational skill documentation.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The cited instructions introduce a new conditional dispatch path in the operational orchestrator
skill, while the PR diff changes only SKILL.md and contains no corresponding test update. This
violates the requirement that every behavioral change have test coverage.

skills/pr-review/SKILL.md[830-849]
Skill: code-implementation

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new empty-findings challenger skip is an untested behavioral change.

## Issue Context
Add repository-standard behavioral coverage demonstrating that an empty merged finding set skips challenger dispatch and proceeds to approval, while a non-empty set still invokes the challenger.

## Fix Focus Areas
- skills/pr-review/SKILL.md[830-849]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Fallback set drops failure finding ✓ Resolved 📜 Skill insight ≡ Correctness ⭐ New

Description

The new fallback definition says a challenger failure leaves the steps 6a–6c set unchanged,
contradicting step 6d’s requirement to add an info-level sub-agent-failure finding. As a result,
steps 6e–6f can proceed without preserving the required failure diagnostic in the final review set.

Code

skills/pr-review/SKILL.md[R861-863]

+Steps 6e–6f below refer to the *adjudicated set*: the challenger's
+`adjudicated_findings`, or the unchanged 6a–6c set when the challenger
+was skipped or fell back after a failure (step 4 below).

Relevance

●●● Strong

Recent history accepts documentation fixes closing logical gaps and preserving required failure
diagnostics.

PR-#1038
PR-#49

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1538375 requires technical algorithm documentation to be internally consistent. The
definition at skills/pr-review/SKILL.md:861–863 describes the fallback as the unchanged steps 6a–6c
set, while the failure procedure at lines 939–950 requires adding an info-level sub-agent-failure
finding for challenger timeout, error, or empty output; because step 6f evaluates the adjudicated
set, the documented flow provides no path for that diagnostic to enter the final set.

skills/pr-review/SKILL.md[861-863]
skills/pr-review/SKILL.md[939-950]
skills/pr-review/SKILL.md[1115-1118]
Skill: pr-review

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The adjudicated fallback-set definition excludes the info-level `sub-agent-failure` finding that step 6d requires recording when the challenger times out, errors, or returns empty output, allowing the diagnostic to disappear before final evaluation.

## Issue Context
Clarify that the fallback adjudicated set contains both the unchanged pre-challenger findings from steps 6a–6c and the recorded challenger `sub-agent-failure` finding, so steps 6e–6f preserve and evaluate the required diagnostic.

## Fix Focus Areas
- skills/pr-review/SKILL.md[861-864]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Challenger examples stay unconditional ✓ Resolved 📜 Skill insight ≡ Correctness

Description

The updated rule makes challenger dispatch conditional on the current merged finding set, but the
re-review examples still list the challenger unconditionally based on prior findings. This internal
contradiction can cause the challenger to be dispatched after a successful re-review produces no
findings.

Code

skills/pr-review/SKILL.md[347]

+4. **Challenger** — always dispatch when findings exist (step 6d).

Relevance

●●● Strong

Recent reviews accept fixes resolving contradictory or duplicated documentation guidance in
SKILL.md.

PR-#753
PR-#1038

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1538375 requires technical algorithm documentation to be internally consistent. The
changed policy at line 347 conditions challenger dispatch on current findings, while lines 365–366
continue to list the challenger in re-review dispatch examples without that condition.

skills/pr-review/SKILL.md[347-347]
skills/pr-review/SKILL.md[365-366]
Skill: pr-review

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The challenger dispatch rule is conditional on findings, while the re-review examples still present challenger dispatch as unconditional.

## Issue Context
Update the examples to clarify that the challenger appears only when steps 6a–6c produce findings during the current review, including re-reviews.

## Fix Focus Areas
- skills/pr-review/SKILL.md[347-347]
- skills/pr-review/SKILL.md[354-366]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

5. Behavior change lacks authorization ✗ Dismissed 📜 Skill insight § Compliance

Description

The PR makes a non-trivial orchestrator behavior change with more than 20 changed lines but does not
link an authorizing issue. The explanatory PR description does not satisfy the rule's explicit
linked-issue requirement.

Code

skills/pr-review/SKILL.md[R830-833]

+**Skip when there is nothing to adjudicate.** If the merged finding set
+from steps 6a–6c is empty, skip the challenger dispatch entirely and
+proceed straight to the verdict with the empty set — the empty-findings
+approval path (step 7) is unchanged; this skip does not add anything to

Relevance

●● Moderate

No close authorization precedent found; applicability of the linked-issue rule is uncertain.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The cited block begins a 21-line addition implementing a structural dispatch change, while the
supplied PR metadata contains no linked issue. Rule 1538390 requires explicit issue authorization
for non-trivial changes of this size.

skills/pr-review/SKILL.md[830-850]
Skill: pr-review

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The non-trivial challenger-dispatch change has no linked issue authorizing the work.

## Issue Context
The diff contains more than 20 changed lines and changes the review orchestrator's behavior. Link an issue that explicitly authorizes skipping challenger dispatch for empty finding sets.

## Fix Focus Areas
- skills/pr-review/SKILL.md[830-833]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

6. Protected skill requires human review 📜 Skill insight § Compliance

Description

This PR modifies skills/pr-review/SKILL.md, which is a protected governance path that must not be
auto-approved. Human approval is required even though the PR description explains the change.

Code

skills/pr-review/SKILL.md[R830-831]

+**Skip when there is nothing to adjudicate.** If the merged finding set
+from steps 6a–6c is empty, skip the challenger dispatch entirely and

Relevance

● Weak

Recent protected-path governance guidance precedent was rejected by this team.

PR-#753

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The changed file is under the protected skills/ path, and the cited lines introduce new
orchestrator dispatch behavior. Rule 1538392 requires a protected-path finding and human approval
whenever such a file is modified.

skills/pr-review/SKILL.md[830-831]
Skill: pr-review

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR modifies a protected governance path and therefore cannot be auto-approved.

## Issue Context
`skills/` is explicitly protected by PR Compliance ID 1538392. Preserve the documented change, but route the PR through the required human governance review and record that approval.

## Fix Focus Areas
- skills/pr-review/SKILL.md[830-831]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

✅ Compliance rules (platform): 57 rules

✅ Skills: 4 invoked

  code-review

  code-implementation

  pr-review

  docs-review

Review mode: ⚖️ Balanced

Tip of the day

💡 Did you know, you can copy the agent prompt from any finding and feed it to your IDE agent

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit ff49262

Results up to commit c302b85 ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Challenger examples stay unconditional ✓ Resolved 📜 Skill insight ≡ Correctness

Description

The updated rule makes challenger dispatch conditional on the current merged finding set, but the
re-review examples still list the challenger unconditionally based on prior findings. This internal
contradiction can cause the challenger to be dispatched after a successful re-review produces no
findings.

Code

skills/pr-review/SKILL.md[347]

+4. **Challenger** — always dispatch when findings exist (step 6d).

Relevance

●●● Strong

Recent reviews accept fixes resolving contradictory or duplicated documentation guidance in
SKILL.md.

PR-#753
PR-#1038

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1538375 requires technical algorithm documentation to be internally consistent. The
changed policy at line 347 conditions challenger dispatch on current findings, while lines 365–366
continue to list the challenger in re-review dispatch examples without that condition.

skills/pr-review/SKILL.md[347-347]
skills/pr-review/SKILL.md[365-366]
Skill: pr-review

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The challenger dispatch rule is conditional on findings, while the re-review examples still present challenger dispatch as unconditional.

## Issue Context
Update the examples to clarify that the challenger appears only when steps 6a–6c produce findings during the current review, including re-reviews.

## Fix Focus Areas
- skills/pr-review/SKILL.md[347-347]
- skills/pr-review/SKILL.md[354-366]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-code-review]

Code review by qodo was updated up to the latest commit c302b85

[guyoron1]

/review

[qodo-code-review]

PR Reviewer Guide 🔍

Warning

/review is deprecated. Use /agentic_review instead (removal date not yet scheduled).

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ No major issues detected

[guyoron1]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 7cf0f6a

[rh-hemartin]

Hello! Could you attach transcripts so we can see how bad it is the challenger? So find multiple no findings reviews and scan their transcripts with the "analyze-transcript" skill on fullsend-ai/fullsend, also post the runs here.

[waynesun09]

Review findings (2 inline comments, both MEDIUM).

[guyoron1]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit dfcd074

[guyoron1]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit c76ba43

[guyoron1]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 28c2610

[guyoron1]

@rh-hemartin Can't produce that from here — I don't have a runner with org access to execute real reviews against fullsend-ai/fullsend, and there's no skill literally named analyze-transcript in this repo's skills/ (closest are finding-agent-runs and retro-analysis). I'm not going to fabricate transcripts to fill this in.

If you can point me at a handful of recent real "no findings" review runs (or grant this a runner), I'll pull and analyze the actual transcripts rather than guess at the challenger's false-positive rate.

[ralphbean]

Oh, he meant this skill: https://github.com/fullsend-ai/fullsend/tree/main/skills/analyze-transcript (from the fullsend-ai/fullsend repo).

You should be able to scrape the gh workflow runs history in this repo and the other one to find cases that support this change - even without org membership. All those logs are public.