docs(problems): add graduated approval policy problem doc - #2012
docs(problems): add graduated approval policy problem doc#2012Benkapner wants to merge 2 commits into
Conversation
Proposes risk-scored approval routing to replace binary approve/reject verdicts, with scoring signals, routing rules, and multiple implementation approaches. References issues fullsend-ai#1143, fullsend-ai#1453, #1462. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Benjamin Kapner <bkapner@redhat.com>
ReviewFindingsMedium
Low
Previous runReviewFindingsMedium
Low
|
Correct autonomy-spectrum.md vs intent-representation.md distinction. Replace broken tool-call-risk-assessment.md link with PR fullsend-ai#2009 reference. Add README.md entry. Clarify Challenger as intra-agent verification, not inter-agent disagreement. Add mixed-path PR semantics for CODEOWNERS interaction. Qualify change-type scoring for additions of new attack surface. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Benjamin Kapner <bkapner@redhat.com>
|
Addressed all findings in b099173. [internal-consistency] (medium, line 97) Corrected the autonomy-spectrum.md reference. The autonomy spectrum defines binary per-repo autonomy (autonomous or not), while Tier 0-3 classification comes from intent-representation.md. Updated the text to reference both documents accurately and clarify that graduated approval operates orthogonally to both. [broken-reference] (medium, line 106) Replaced the broken markdown link to [missing-readme-link] (medium) Added README.md entry for graduated-approval-policy.md, positioned after the Code Review entry. [internal-consistency] (low, line 88) Corrected the Challenger characterization. The Challenger is an intra-agent verification step within the orchestrator's own process (step 6e), not disagreement between independent sub-agents. Updated the text to reflect this distinction. [edge-case] (low, line 61) Added a note clarifying mixed-path PR semantics: when a PR touches both CODEOWNERS-guarded and non-guarded paths, CODEOWNERS takes precedence for the entire PR, but the risk score still provides context to the human reviewer. [algorithm-logic] (low, line 38) Qualified the change-type scoring statement. Additions of new attack surface (API endpoints, dependencies, permission grants) can have equal or greater blast radius than modifications to existing code, and should be scored independently. |
Site previewPreview: https://75d40f56-site.fullsend-ai.workers.dev Commit: |
ralphbean
left a comment
There was a problem hiding this comment.
I like this idea — a rubric for review sub-agents to score against, with the coordinator making a routing decision based on that. The problem statement is sharp and the referenced issues make the case well.
But I think this is a solution proposal, not a problem doc. The first 25 lines frame the problem (binary verdicts are insufficient), and then the remaining 90 lines propose a specific architecture: numeric scoring, a 5-tier routing table, three implementation approaches. That's an ADR.
We've been making changes to the agents without eval coverage, and it's been bugging me. A change like this — restructuring how the review agent renders verdicts — is exactly the kind of thing we should have evals in place for before we move on it. #1682 and @ascerra's eval work are getting us there.
If you're up for it: write eval cases where the current review agent gets it wrong (approves something it shouldn't, blocks something it shouldn't). Show that graduated scoring produces better outcomes on those cases. TDD for agent architecture. Then propose the ADR with those cases as evidence, implement it, and let the evals tell us if it's actually better.
Closing this, but the idea has legs. Thanks for the writeup.
|
🤖 Finished Retro · ✅ Success · Started 6:39 PM UTC · Completed 6:44 PM UTC |
Retro: PR #2012 — graduated approval policy problem docWhat happenedPR #2012 added a problem doc proposing risk-scored approval routing. The review agent requested changes, finding legitimate issues: incorrect cross-references between autonomy-spectrum.md and intent-representation.md, a broken link to a non-existent file, and a mischaracterization of the Challenger role. The author addressed all findings in one iteration. However, a human reviewer (ralphbean) then closed the PR without merging for a completely different reason: the document was structured as a solution proposal / ADR, not a problem doc. The first ~25 lines framed the problem, but the remaining ~90 lines proposed a specific architecture with scoring tables and implementation approaches. The review agent caught low-level correctness issues but missed the highest-impact structural concern — the one that actually determined the PR's outcome. Proposals1 proposal filed. 1 candidate skipped due to overlap with existing issues (#2233, #1906 cover doc convention enforcement and structure-aware review broadly). Proposals filed
|
Reframes the graduated approval concept as a problem statement without prescribing a specific architecture. Identifies the binary verdict's failure modes with concrete examples, describes what graduated approval would need, and defers solution design to an ADR backed by eval evidence. Based on feedback from PR fullsend-ai#2012. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Benjamin Kapner <bkapner@redhat.com>
Review agents currently make a single binary decision: approve or request changes. There is no middle ground. This forces a trade-off between two failure modes.
When the agent leans toward approving, risky changes slip through. Submodule bumps get approved without the agent inspecting the actual changes (#1462). Medium-severity correctness findings get suppressed to avoid false-positive noise (#1453). Author uncertainty signals like "i think this is right but i'm not sure" get ignored in the approval decision (#1143).
When the agent leans toward blocking, legitimate work gets stuck. Humans spend time reviewing changes that are almost certainly fine, defeating the purpose of automation.
Human reviewers don't work this way. A human would say "LGTM, but get a second pair of eyes on the crypto changes" or "looks fine to me but the test coverage concerns me." That graduated response doesn't exist for agents today. It's yes or no.
This doc proposes replacing the binary verdict with risk-scored approval routing. The review agent (or a separate scoring layer) assigns a risk score based on multiple signals:
From the diff: which files changed (auth code is riskier than docs), change type (deletions score higher than additions), whether tests changed too (no tests = higher risk), and whether binary/opaque files are present.
From the review findings: number and severity of findings, confidence level, and whether sub-agents disagreed with each other (disagreement = uncertainty = higher risk).
From context: author history (first-time contributors score higher), branch target (default branch = higher risk), and whether related code was recently modified.
The score maps to a routing action:
Thresholds are configurable per repo and per org.
The doc explores three implementation approaches: scoring inside the review agent (simplest but model-dependent), a separate deterministic scoring layer in the harness (more robust against manipulation), and multi-reviewer consensus where disagreement triggers escalation (expensive but naturally handles uncertainty). The parallel sub-agent architecture from #1550 partially enables the third approach, since the Challenger role already contests findings, but disagreement between sub-agents does not currently influence the approval decision.
Addresses #1143, #1453, and fullsend-ai/agents#189.