feat(triage): add effort-based gating before auto-promoting to coder#3048
feat(triage): add effort-based gating before auto-promoting to coder#3048rh-hemartin wants to merge 1 commit into
Conversation
|
🤖 Finished Review · ✅ Success · Started 7:57 AM UTC · Completed 8:07 AM UTC |
PR Summary by Qodotriage: estimate effort and gate ready-to-code auto-promotion
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
Site previewPreview: https://d02ed479-site.fullsend-ai.workers.dev Commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Code Review by Qodo
1.
|
| CATEGORY=$(jq -r '.triage_summary.category // "unknown"' "${RESULT_FILE}") | ||
| echo "Category: ${CATEGORY}" | ||
| EFFORT=$(jq -r '.triage_summary.effort // 1.0' "${RESULT_FILE}") | ||
| echo "Category: ${CATEGORY}, Effort: ${EFFORT}" | ||
| case "${CATEGORY}" in | ||
| bug|documentation|performance) | ||
| echo "Deferring ready-to-code label (${CATEGORY}) until after label_actions..." | ||
| DEFERRED_LABEL="ready-to-code" | ||
| # Effort threshold: >= 2.0 is substantial work, route to human review. | ||
| if awk -v effort="${EFFORT}" 'BEGIN { exit (effort >= 2.0 ? 0 : 1) }'; then | ||
| echo "High effort (${EFFORT}) — applying triaged label for human review..." | ||
| add_label "triaged" | ||
| else | ||
| echo "Low effort (${EFFORT}) — deferring ready-to-code label until after label_actions..." | ||
| DEFERRED_LABEL="ready-to-code" | ||
| fi |
There was a problem hiding this comment.
2. Ready-to-code not cleared 🐞 Bug ≡ Correctness
In post-triage.sh (sufficient handler), the high-effort path adds triaged but never removes an existing ready-to-code label, leaving contradictory control labels and undermining the intended human-review gate. The dispatch workflow routes to the code stage when the triggering label is ready-to-code, so leaving that label present keeps the issue in a state that implies code dispatch eligibility.
Agent Prompt
## Issue description
High-effort bug/docs/perf issues are supposed to be routed to human review by applying the `triaged` label. The new effort-gating branch adds `triaged` but does not clear `ready-to-code` if it is already present, leaving contradictory control labels and weakening the meaning of the gate.
## Issue Context
The dispatcher routes to the code stage based on `ready-to-code` label events, and `ready-to-code` is the canonical “eligible for coding” control label. If it remains present while `triaged` is applied, the issue state is inconsistent.
## Fix Focus Areas
- internal/scaffold/fullsend-repo/scripts/post-triage.sh[259-292]
- internal/scaffold/fullsend-repo/scripts/post-triage-test.sh[177-207]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
ReviewVerdict: Comment — the implementation is correct and secure, but test coverage for a key fallback path is missing and documentation has not been updated to reflect the new behavior. SummaryThis PR adds effort-based gating to the triage pipeline so that high-effort bug/documentation/performance issues (effort ≥ 2.0) route to human review instead of auto-promoting to
Findings1. Missing backward-compatibility test for absent
|
Add effort estimation to triage agent (0.25-3 scale matching RICE). Bug/docs/performance issues with effort >= 2.0 get triaged instead of ready-to-code, routing high-complexity work through human review before dispatch. Fixes #2207 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Hector Martinez <hemartin@redhat.com>
d8c0a84 to
9cabba6
Compare
|
🤖 Finished Review · ✅ Success · Started 8:15 AM UTC · Completed 8:25 AM UTC |
ralphbean
left a comment
There was a problem hiding this comment.
This is an automated comment. We are moving agent content from internal/scaffold/fullsend-repo/ to https://github.com/fullsend-ai/agents -- changes should be made to agent definitions there going forwards.
waynesun09
left a comment
There was a problem hiding this comment.
Review Squad — 5 agents (Claude x3, Gemini, Codex) | 3 medium findings after dedup and verification
Overall: solid, well-scoped PR. The effort-based gating logic is correct and the conservative fallback (missing effort → human review) is a good design choice. The three inline findings are actionable — a missing test path, a defense-in-depth guard, and a label mismatch between schema and agent prompt.
|
|
||
| run_test "sufficient-performance-high-effort-gets-triaged" \ | ||
| '{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Optimize database queries across layers","severity":"high","category":"performance","problem":"Slow queries","root_cause_hypothesis":"Missing indexes and N+1 queries","reproduction_steps":["step 1"],"environment":"Production","impact":"All users","recommended_fix":"Add indexes and refactor query patterns","proposed_test_case":"test_query_performance","effort":3.0},"comment":"## Triage Summary\n\nLarge performance optimization."}' \ | ||
| "gh api repos/test-org/test-repo/issues/42/labels -f labels[]=triaged --silent" |
There was a problem hiding this comment.
MEDIUM — Missing test for null-effort fallback path
The script has an explicit branch for missing effort ("null") at post-triage.sh:278 that routes to triaged, but every single test fixture includes an explicit "effort" value. The null-effort codepath is completely untested. Since effort is optional in the schema (not in the required array), this path is reachable when the agent omits it.
Suggestion: Add a test case:
run_test "sufficient-bug-missing-effort-gets-triaged" \
'{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Fix crash","severity":"high","category":"bug","problem":"Crash","root_cause_hypothesis":"Buffer overflow","reproduction_steps":["step 1"],"environment":"Linux","impact":"All users","recommended_fix":"Fix buffer","proposed_test_case":"test_crash"},"comment":"## Triage Summary\n\nReady."}' \
"gh api repos/test-org/test-repo/issues/42/labels -f labels[]=triaged --silent"Flagged by 5/5 review agents (full consensus).
| echo "Missing effort estimate — applying triaged label for human review..." | ||
| remove_label "ready-to-code" | ||
| add_label "triaged" | ||
| elif awk -v effort="${EFFORT}" 'BEGIN { exit (effort >= 2.0 ? 0 : 1) }'; then |
There was a problem hiding this comment.
MEDIUM — Non-numeric effort value silently auto-promotes to ready-to-code
If effort passes the null check but is a non-numeric string (e.g., "high"), awk coerces it to 0, and 0 >= 2.0 is false, routing to ready-to-code. This is the wrong direction for a safety gate — invalid values should fail closed (triaged/human review), not open (auto-promote to coder).
The schema's "type": "number" constraint mitigates this, but defense-in-depth is warranted since this gate controls auto-dispatch.
Suggestion: Add a regex guard before the awk comparison:
elif ! [[ "${EFFORT}" =~ ^[0-9]+(\.[0-9]+)?$ ]]; then
echo "::warning::Non-numeric effort value '${EFFORT}' — treating as high effort for human review"
remove_label "ready-to-code"
add_label "triaged"
elif awk -v effort="${EFFORT}" 'BEGIN { exit (effort >= 2.0 ? 0 : 1) }'; thenFlagged by 4/5 review agents.
| "type": "number", | ||
| "minimum": 0.25, | ||
| "maximum": 3, | ||
| "description": "Estimated implementation effort (0.25 = trivial, 1 = moderate, 3 = substantial)" |
There was a problem hiding this comment.
MEDIUM — Schema description mismatches triage.md effort scale labels
The description says "0.25 = trivial, 1 = moderate, 3 = substantial" but triage.md defines 2.0 as "Substantial" and 3.0 as "Large". The word "substantial" maps to different scores in each file. An LLM reading the schema alone could calibrate effort differently than one reading the agent prompt.
Suggestion:
"description": "Estimated implementation effort (0.25 = trivial, 1 = moderate, 2 = substantial, 3 = large)"Flagged by 3/5 review agents.
|
Migrated to fullsend-ai/agents#36 |
|
🤖 Finished Retro · ✅ Success · Started 6:37 AM UTC · Completed 6:44 AM UTC |
Retro: PR #3048The retro agent was unable to analyze PR #3048 because GitHub API access from within the sandbox failed. Root cause: The retro agent depends entirely on runtime GitHub API access from inside the sandbox. Unlike the review agent (which has What the agent could still analyze: The local codebase, harness configs, agent definitions, skills, policies, and scripts. This analysis yielded one actionable proposal focused on making the retro agent resilient to GitHub API failures. Note: Could not verify whether existing issues already cover this proposal because GitHub API was unavailable. The post-script should deduplicate if a matching issue already exists. Proposals filed
|
Summary
Adds effort estimation to the triage agent and gates auto-promotion to
ready-to-codebased on complexity. High-effort bug/documentation/performance issues now route through human review before dispatching to the code agent.Problem
Currently
post-triage.shauto-appliesready-to-codeto all bug/documentation/performance issues regardless of complexity. A trivial typo fix and a multi-file architectural change both auto-promote equally, leading to inadequate first attempts on complex issues and costly review-fix iteration cycles.Fixes #2207
Changes
effortfield totriage_summary(0.25–3 scale, matching RICE)post-triage.shsufficient handler:triagedlabel (routes to human review)ready-to-codelabel (auto-promote to coder)Test Plan
post-triage-test.shready-to-codetriagedtriagedtriaged)Example
Low effort (0.5) — auto-promotes:
{ "category": "bug", "effort": 0.5, ... }→
ready-to-codelabelHigh effort (2.5) — human review:
{ "category": "bug", "effort": 2.5, ... }→
triagedlabel