fix(pr-agent): classify timeouts per ATTEMPT, not on total job time - #39
fix(pr-agent): classify timeouts per ATTEMPT, not on total job time#39yakimoto wants to merge 1 commit into
Conversation
Re-syncs this repo to wave-foundation-public#72, which landed after the inline lane was adopted here. THE DEFECT. The adopted template stamped AGENT_START once, before attempt 1, then compared TOTAL job time — attempt 1 + the 45s backoff + attempt 2 — against STEP_BUDGET_S=360, a budget its own comment calls PER-ATTEMPT. Two healthy-but-slow attempts (~180s each, ~405s together) therefore reported "pr-agent TIMED OUT ... A hang, NOT a rate limit." sending the next reader to debug a hang that never happened; the else-branch lied the other way, asserting the run was "well inside the budget" from the same misused total. Found by qodo review on wave-monitor#48 and confirmed against the file before acting. THE FIX. Stamp each attempt separately and classify on the LONGEST attempt, with if: always() end stamps so an attempt killed BY its step timeout still records one — exactly the case the classifier exists to catch. Total wall time is still reported as context but no longer decides the verdict. NOT URGENT, NOT IGNORABLE. The defect is in a MESSAGE, not in behaviour: the lane still retries, still renders NEUTRAL, still never blocks a PR. But that verdict step exists precisely because "a confidently wrong cause is worse than no cause", so shipping a classifier that can misname a hang defeats its purpose. Job id pr_agent and every on: trigger unchanged — the job id is the check-run context and branch protection matches on it. Refs wave-av/wave-pen#417, wave-av/wave-pen#388
🤖 CodeAnt AI — Review Status
|
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_34975281-3570-4902-9076-c2b6559de2d0) |
|
Warning Review limit reachedNext included review available in 28 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 91 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Comment |
codeant-ai
Bot
added
the
size:M
ApprovabilityVerdict: Would Approve Macroscope's review found this PR approvable — This is a narrowly scoped CI diagnostic fix that measures each pr-agent attempt separately and leaves retry behavior, advisory status, and production workflows unchanged. The sole modified workflow is owned by the author, with no schema, deployment, security, billing, or Macroscope configuration impact. Not approved because:
Review your spending limits in Billing settings. You can add or adjust custom eligibility rules. Learn more. |
|
Note Automatic reviews are paused because your team has used its included automatic processing for this billing period (headroom scales with your seat count). You can still comment "Gitar review" to run one anytime, and automatic reviews resume on their own by September 1. Add seats for more headroom. Code Review ✅ ApprovedRefactors PR-agent timeout classification to evaluate per-attempt duration rather than total job time, preventing false timeout reports on slow retries. No issues found. OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
PR Summary by QodoFix pr-agent timeout classification to use per-attempt duration
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Code Review by Qodo
1. Missing changelog for verdict text
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| echo "::warning::pr-agent TIMED OUT — the longest attempt ran ${LONGEST}s against a ${STEP_BUDGET_S}s per-attempt budget (attempt 1 ${A1}s, attempt 2 ${A2}s), so it was killed by its step timeout rather than returning an error. A hang, NOT a rate limit. Rendering NEUTRAL: an advisory reviewer must not block the PR (#3128)." | ||
| exit 0 | ||
| fi | ||
| echo "::warning::pr-agent failed after 2 attempts (45s backoff, ${ELAPSED}s total — well inside the ${STEP_BUDGET_S}s budget, so it returned an error rather than hanging) — most commonly an upstream 429/rate-limit from the LLM router. Rendering NEUTRAL: an advisory reviewer must not block the PR (#3128)." | ||
| echo "::warning::pr-agent failed after 2 attempts (attempt 1 ${A1}s, attempt 2 ${A2}s, ${ELAPSED}s wall including the 45s backoff — NEITHER attempt reached the ${STEP_BUDGET_S}s per-attempt budget, so it returned an error rather than hanging) — most commonly an upstream 429/rate-limit from the LLM router. Rendering NEUTRAL: an advisory reviewer must not block the PR (#3128)." |
There was a problem hiding this comment.
1. Missing changelog for verdict text 📘 Rule violation § Compliance
This PR changes user-visible GitHub Actions warning text for the pr-agent verdict, but CHANGELOG.md has no Unreleased entry documenting the change. This violates the requirement to document user-facing changes in the Unreleased section.
Agent Prompt
## Issue description
The PR changes user-visible workflow output (the `::warning::pr-agent ...` verdict messages) but there is no corresponding entry under `## [Unreleased]` in the root `CHANGELOG.md`.
## Issue Context
Compliance requires documenting user-facing changes in the `Unreleased` section.
## Fix Focus Areas
- CHANGELOG.md[7-7]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
The pull request modifies the warning messages emitted by the pr-agent GitHub Action, which constitutes a user-facing change. According to the project's compliance requirements, such changes must be documented in the |
Qodo Fixer🍒 Ready to be cherry-picked — ✅ Merged (0) · ☑ Fixed (1) 🔗 Fix PR: #40 This fix PR was closed automatically. Its branch is preserved so you can cherry pick the changes into the original PR. Prompt for coding agent Process — 1 fixed
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Reviewer's GuideUpdates the pr-agent workflow’s diagnostic classifier to measure the longest individual attempt rather than total job duration, preventing retries and backoff from causing false timeout/hang messages while preserving existing retry and non-blocking behavior. Sequence diagram for per-attempt pr-agent timeout classificationsequenceDiagram
participant Workflow
participant Agent as PR-Agent
participant Classifier
Workflow->>Workflow: stamp attempt 1 start
Workflow->>Agent: run attempt 1
Workflow->>Workflow: stamp attempt 1 end
alt attempt 1 failed
Workflow->>Workflow: sleep 45s
Workflow->>Workflow: stamp attempt 2 start
Workflow->>Agent: run attempt 2
Workflow->>Workflow: stamp attempt 2 end
end
Workflow->>Classifier: calculate A1, A2, and LONGEST
alt LONGEST >= STEP_BUDGET_S - 15
Classifier-->>Workflow: report TIMED OUT and render NEUTRAL
else longest attempt below budget
Classifier-->>Workflow: report failed after attempts and render NEUTRAL
end
Flow diagram for longest-attempt verdict selectionflowchart TD
A[PR-Agent attempt durations recorded] --> B[Calculate A1 and A2]
B --> C[Select LONGEST attempt]
C --> D{LONGEST >= STEP_BUDGET_S - 15?}
D -->|Yes| E[TIMED OUT: likely hang]
D -->|No| F[Failed after attempts: likely rate limit]
E --> G[Render NEUTRAL; do not block PR]
F --> G
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
1 participant
Re-syncs this repo to wave-foundation-public#72, which landed after the inline pr-agent lane was adopted here. Tracked as wave-pen#417.
The defect
The adopted template stamped
AGENT_STARTonce, before attempt 1, then compared total job time — attempt 1 + the 45s backoff + attempt 2 — againstSTEP_BUDGET_S=360, a budget its own comment calls per-attempt.Two healthy-but-slow attempts (~180s each, ~405s together) therefore reported:
…sending the next reader to debug a hang that never happened. The else-branch lied the other way, asserting the run was "well inside the budget" from the same misused total.
Found by qodo review on wave-monitor#48 and confirmed against the file before acting.
The fix
Stamp each attempt separately and classify on the longest attempt, with
if: always()end stamps so an attempt killed by its step timeout still records one — exactly the case the classifier exists to catch. Total wall time is still reported as context but no longer decides the verdict.failed after 2 attempts✅TIMED OUT✅Not urgent, not ignorable
The defect is in a message, not behaviour — the lane still retries, still renders NEUTRAL, still never blocks a PR. But that verdict step exists precisely because "a confidently wrong cause is worse than no cause", so a classifier that can misname a hang defeats its own purpose.
Job id
pr_agentand everyon:trigger unchanged — the job id is the check-run context and branch protection matches on it.Refs wave-av/wave-pen#417, wave-av/wave-pen#388
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Note
Low Risk
CI diagnostic messaging only; retry, check-run identity, and non-blocking advisory behavior are unchanged.
Overview
Fixes the pr-agent verdict so hang vs rate-limit is decided from per-attempt duration, not wall-clock job time.
The workflow used to stamp one
AGENT_STARTand compare total time (attempt 1 + 45s backoff + attempt 2) against the 360s per-attempt budget. Two slow-but-healthy ~180s attempts (~405s together) were therefore labeled a hang. Each attempt now gets start/end stamps (if: always()so a timeout-killed step still records an end), and the classifier uses the longest attempt plus 15s slack. Wall time is still logged but no longer drives the verdict. Retry and NEUTRAL/non-blocking behavior are unchanged.Reviewed by Cursor Bugbot for commit fbf5313. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by Sourcery
Classify pr-agent failures using per-attempt timeout durations so slow retries are not incorrectly reported as hangs while preserving advisory, retry, and non-blocking behavior.
Bug Fixes:
Enhancements: