Skip to content

feat(review): add confidence annotation to review verdicts (#5514) - #5515

Closed
Benkapner wants to merge 1 commit into
fullsend-ai:mainfrom
Benkapner:feat/review-confidence-annotation
Closed

feat(review): add confidence annotation to review verdicts (#5514)#5515
Benkapner wants to merge 1 commit into
fullsend-ai:mainfrom
Benkapner:feat/review-confidence-annotation

Conversation

@Benkapner

@Benkapner Benkapner commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a confidence level (high/medium/low) to the review agent's structured output. This is the minimal first step toward graduated approval, emitting the data without changing any routing or auto-merge behavior.

What changes

  • pr-review skill (step 6g): new step after verdict determination that derives confidence from sub-agent agreement, challenger removal rate, and verdict threshold proximity
  • review-result schema: adds an optional confidence field (high/medium/low)

Confidence derivation

Level When
high all sub-agents agreed, challenger removed 0-1 findings, verdict not near threshold
medium challenger removed >1 finding, OR sub-agents disagreed on severity, OR verdict is one finding from flipping
low severe sub-agent disagreement, OR challenger failed (fallback set used), OR ambiguous change scope

What does NOT change

  • Dispatch logic, label behavior, verdict thresholds, auto-merge rules
  • The field is optional in the schema; omitting it is valid

Related Issue

Closes #5514

Checklist

  • PR title follows Conventional Commits
  • Commits are signed off (DCO)

…ai#5514)

Add a confidence level (high/medium/low) to the review agent's output.
Derived from sub-agent agreement, challenger removal rate, and verdict
threshold proximity.

Changes:
- pr-review skill: add step 6g (confidence derivation) after verdict
- review-result schema: add optional confidence field

The confidence field is informational only. No changes to dispatch,
labels, or auto-merge behavior.

Signed-off-by: Benjamin Kapner <bkapner@redhat.com>
@Benkapner
Benkapner requested a review from a team as a code owner July 23, 2026 08:52
@github-actions

Copy link
Copy Markdown

E2E tests did not run

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

See E2E testing guide for details.

@github-actions

Copy link
Copy Markdown

Site preview

Preview: https://9e9638bd-site.fullsend-ai.workers.dev

Commit: 7cac2159eea79d49c1f1aed4eb1456cc03b1163d

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add optional confidence level to review agent verdict output

✨ Enhancement ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Add optional confidence (high/medium/low) to the review agent result schema.
• Document a new pr-review step to derive confidence after verdict selection.
• Keep verdict thresholds and routing behavior unchanged; confidence is informational only.
Diagram

graph TD
  A([Review Agent Run]) --> B["Verdict evaluation (step 6f)"] --> C["Confidence derivation (step 6g)"] --> D["Review result JSON"] --> E["Harness validation"] --> F([GitHub review action])
  E --> G["review-result.schema.json"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Numeric confidence score (0–1)
  • ➕ More granular than three buckets; can support future automation thresholds
  • ➕ Easier to trend/aggregate across runs
  • ➖ Harder to interpret consistently for humans
  • ➖ Requires calibration guidance and may create false precision
2. Expose contributing signals instead of a single label
  • ➕ More transparent: reviewers can see why confidence is low/medium/high
  • ➕ Keeps future scoring logic flexible without schema churn
  • ➖ More verbose payload; higher risk of schema/consumer coupling
  • ➖ Harder to standardize across agent versions
3. Compute confidence in a post-processing script (outside the skill)
  • ➕ Keeps prompt/skill simpler; logic becomes testable code
  • ➕ Can evolve heuristics without changing agent instructions
  • ➖ Requires additional implementation and integration work
  • ➖ May not have access to all prompt-level context (e.g., perceived ambiguity)

Recommendation: Proceed with the current enum-based, optional confidence field as a minimal, backwards-compatible first step. It adds human-usable signal without impacting routing/auto-merge, and it leaves room to later evolve into richer signals or a numeric score if consumers emerge.

Files changed (2) +31 / -0

Documentation (1) +26 / -0
SKILL.mdDocument step 6g: confidence derivation after verdict selection +26/-0

Document step 6g: confidence derivation after verdict selection

• Introduces a new documented step (6g) that derives confidence after the verdict is determined. Specifies heuristic criteria for high/medium/low confidence and instructs emitting it as an optional top-level 'confidence' field in the result JSON.

internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md

Other (1) +5 / -0
review-result.schema.jsonAllow optional confidence enum on review agent result +5/-0

Allow optional confidence enum on review agent result

• Adds a top-level optional 'confidence' string field to the review-result JSON schema. The field is constrained to 'high|medium|low' and is explicitly documented as informational.

internal/scaffold/fullsend-repo/schemas/review-result.schema.json

@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

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

Context used
✅ Compliance rules (platform): 61 rules

Grey Divider


Informational

1. Confidence schema untested 🐞 Bug ☼ Reliability
Description
No schema-validation tests currently assert that confidence is accepted for valid enum values or
rejected for invalid ones, so regressions to the new contract may go uncaught. Adding targeted
fixtures would harden the rollout of this new field.
Code

internal/scaffold/fullsend-repo/schemas/review-result.schema.json[R30-34]

+    "confidence": {
+      "type": "string",
+      "enum": ["high", "medium", "low"],
+      "description": "How confident the review agent is in its verdict. Derived from sub-agent agreement, challenger removals, and verdict threshold proximity."
    }
Relevance

⭐⭐⭐ High

Team has accepted adding targeted schema-validation fixtures for new/fragile schema constraints.

PR-#337

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The schema now defines confidence as an enum, but the existing review-result schema tests shown
only cover approve/reject payloads without confidence, and the label_actions test suite similarly
lacks any confidence fixtures.

internal/scaffold/fullsend-repo/schemas/review-result.schema.json[18-35]
internal/scaffold/fullsend-repo/scripts/validate-output-schema-test.sh[345-371]
internal/scaffold/fullsend-repo/schemas/review-result-label-actions-test.sh[40-101]

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

### Issue description
`confidence` was added to `review-result.schema.json`, but CI fixtures don’t currently validate acceptance/rejection behavior for this new enum field.

### Issue Context
Existing tests validate several review-result shapes but do not include any cases with `confidence`, so they don’t exercise the new property.

### Fix Focus Areas
- internal/scaffold/fullsend-repo/scripts/validate-output-schema-test.sh[345-371]
- internal/scaffold/fullsend-repo/schemas/review-result-label-actions-test.sh[40-100]
- internal/scaffold/fullsend-repo/schemas/review-result.schema.json[18-35]

### Suggested changes
- Add a passing fixture that includes `"confidence":"high"` (or medium/low) on an approve/reject payload.
- Add a failing fixture with an invalid value like `"confidence":"unknown"` to ensure enum enforcement is covered.

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


2. Confidence field prompt mismatch 🐞 Bug ⚙ Maintainability
Description
The PR adds confidence to the review-result schema and instructs the pr-review skill to emit it,
but the review agent definition (agents/review.md) still tells the agent to “Only include fields
listed below” and that list omits confidence, creating conflicting instructions that can cause the
agent to omit the new field. This undermines the PR’s goal of emitting confidence annotations (even
though omission remains schema-valid).
Code

internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md[R802-804]

+Include the confidence level in the review result JSON as a top-level
+`confidence` field. This field is optional in the schema; omitting it
+is acceptable if you cannot determine it.
Relevance

⭐ Low

Similar prompt/definition inconsistency fixes have been rejected as not worth changing upstream
docs/prompts.

PR-#717

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The pr-review skill explicitly instructs emitting a top-level confidence field, and the schema now
allows it, but the review agent definition still presents an exhaustive top-level field list that
omits confidence and warns the schema will reject unknown fields—creating conflicting guidance in
the exact execution path used by the harness.

internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md[780-804]
internal/scaffold/fullsend-repo/schemas/review-result.schema.json[1-35]
internal/scaffold/fullsend-repo/agents/review.md[239-259]
internal/scaffold/fullsend-repo/harness/review.yaml[1-20]

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-review skill now instructs emitting a top-level `confidence` field, but the review agent definition/prompt still lists allowed top-level fields and omits `confidence`, which can lead to the agent not emitting the field.

### Issue Context
- The harness runs `agents/review.md` with skills including `skills/pr-review`.
- `agents/review.md` explicitly warns to only include the fields it lists, but that list is now incomplete.

### Fix Focus Areas
- internal/scaffold/fullsend-repo/agents/review.md[239-340]
- internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md[780-804]
- internal/scaffold/fullsend-repo/schemas/review-result.schema.json[1-35]

### Suggested changes
- Add `confidence` to the top-level object table in `agents/review.md` (type string, enum high/medium/low, optional).
- Update the jq examples in `agents/review.md` to show how to include `confidence` when present.
- Consider softening the phrasing “Only include fields listed below” to clarify that the list is exhaustive and must be kept in sync with the schema (or reference the schema properties directly).

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


Grey Divider

Qodo Logo

Comment on lines +30 to 34
"confidence": {
"type": "string",
"enum": ["high", "medium", "low"],
"description": "How confident the review agent is in its verdict. Derived from sub-agent agreement, challenger removals, and verdict threshold proximity."
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Informational

2. Confidence schema untested 🐞 Bug ☼ Reliability

No schema-validation tests currently assert that confidence is accepted for valid enum values or
rejected for invalid ones, so regressions to the new contract may go uncaught. Adding targeted
fixtures would harden the rollout of this new field.
Agent Prompt
### Issue description
`confidence` was added to `review-result.schema.json`, but CI fixtures don’t currently validate acceptance/rejection behavior for this new enum field.

### Issue Context
Existing tests validate several review-result shapes but do not include any cases with `confidence`, so they don’t exercise the new property.

### Fix Focus Areas
- internal/scaffold/fullsend-repo/scripts/validate-output-schema-test.sh[345-371]
- internal/scaffold/fullsend-repo/schemas/review-result-label-actions-test.sh[40-100]
- internal/scaffold/fullsend-repo/schemas/review-result.schema.json[18-35]

### Suggested changes
- Add a passing fixture that includes `"confidence":"high"` (or medium/low) on an approve/reject payload.
- Add a failing fixture with an invalid value like `"confidence":"unknown"` to ensure enum enforcement is covered.

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the field is optional in the schema, so omitting it is valid and won't break existing tests. adding test fixtures for this is reasonable as a follow-up once the field is actually in use.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the pr-review skill is what drives agent behavior, not agents/review.md. the skill now has the confidence derivation instructions. agents/review.md is a high-level doc for humans, not an instruction set for the agent.

@rh-hemartin

Copy link
Copy Markdown
Member

Hello again! We have moved the agents to fullsend-ai/agents, you can gh issue transfer ABCDE fullsend-ai/agents, not sure if you can do it with the PR, but it should be trivial to port the changes there.

@Benkapner

Copy link
Copy Markdown
Contributor Author

ported to fullsend-ai/agents per feedback that agents have moved there. new PR: fullsend-ai/agents#396

closing this one.

@Benkapner Benkapner closed this Jul 23, 2026
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 23, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 12:01 PM UTC · Completed 12:10 PM UTC
Commit: 7cac215 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #5515 — confidence annotation for review verdicts

PR #5515 was a human-authored contribution by @Benkapner adding a confidence field (high/medium/low) to the review agent's structured output (2 files, +31 lines). It was opened against fullsend-ai/fullsend targeting scaffold files under internal/scaffold/fullsend-repo/, but the actual agent content has moved to fullsend-ai/agents. After ~1 hour, @rh-hemartin redirected the contributor, who ported the changes to fullsend-ai/agents#396 and closed this PR.

Timeline

  1. 08:50 — Issue #5514 filed referencing scaffold paths
  2. 08:52 — PR feat(review): add confidence annotation to review verdicts (#5514) #5515 opened from fork (Benkapner/fullsend)
  3. 08:52 — CI workflows triggered; fullsend dispatch skipped review agent (fork PR, is_event_actor_authorized returned false)
  4. 08:56–08:59 — Qodo-code-review ran: flagged untested schema field (reasonable but deferrable) and a prompt mismatch with agents/review.md (incorrect — SKILL.md drives behavior)
  5. 09:26@Benkapner responded explaining why both qodo findings are non-issues
  6. 09:57@rh-hemartin redirected to fullsend-ai/agents
  7. 11:59 — PR ported to agents#396 and closed; retro dispatched

What went well

  • Human redirect was timely (~1 hour after PR opened)
  • Contributor ported the PR quickly and cleanly
  • CI checks all passed; build/test infrastructure worked correctly

What could go better — all covered by existing issues

No new proposals warranted. All improvement opportunities are already tracked:

  • #3157 (AGENTS.md migration docs): PR feat(review): add confidence annotation to review verdicts (#5514) #5515 is the 5th+ instance of a contributor targeting deprecated scaffold files. Both the issue (feat(review): add confidence annotation to review verdicts #5514) and PR referenced paths under internal/scaffold/fullsend-repo/. AGENTS.md still has no mention of the migration to fullsend-ai/agents.
  • #3488 (CI check for deprecated paths on fork PRs): Because this was a fork PR, the fullsend review agent never ran — only qodo reviewed. A CI-level check would have caught the deprecated-path problem within minutes instead of requiring human intervention after ~1 hour. This PR matches the exact scenario Add CI-level check for deprecated scaffold paths, covering fork PRs that the review agent skips #3488 was filed for.
  • #5375 (Skip retro for zero-agent-involvement unmerged PRs): This retro itself is an example of the waste — a full retro agent session dispatched for a closed-without-merge fork PR where no fullsend agent ran. The only automated reviewer was qodo (third-party).
  • #3476 / #3659 (Review agent detecting deprecated/migrating paths): When the review agent does run (non-fork PRs), it should flag deprecated scaffold paths. These issues remain relevant for internal contributor PRs targeting the same files.

@github-actions
github-actions Bot deleted the feat/review-confidence-annotation branch August 23, 2026 03:55
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.

feat(review): add confidence annotation to review verdicts

2 participants