A 429 Loop Is Not Resilience: Reviewing Agent PRs That Retry Model Calls

An agent pull request that wraps model calls in retries is a capacity change, not a reliability patch. Trust the error classification and the stop condition. Revert unbounded loops, retries on non-idempotent writes, and any comment that treats a shared server as spare capacity. Prove the budget with a local fault injector before the branch is allowed to touch a hosted endpoint.

Read the loop before the summary

Agent summaries promise resilience. The diff says otherwise when a new function calls the client inside a loop. Start with the loop bound, the status set, and whether the call writes state. Those three facts decide the review.

The initial call counts as attempt 1. A helper that retries three times after a failure sends four calls. Spell that in the test name so a later edit cannot widen the budget silently.

Look for four smells before reading the rest of the PR body.

• A loop with no ceiling, or a ceiling copied from a comment rather than a named constant with a test.
• A bare exception handler, or a retry on every status below 500, including 401, 403, and 422.
• Sleep on the request thread, with no jitter and no respect for Retry-After.
• The same wrapper around reads and writes, with no idempotency key.

Label every hunk

Use one label per hunk. Do not average the patch into a single pass.

Diff signal	Label	Why
Typed timeout, no retry added	Trust	Bounds wait time without multiplying calls
Retry only on connection reset, cap 2, read-only	Prove	The cap is necessary, not sufficient
Unbounded loop or a high cap with no test	Revert	Unjustified amplification
Retry on 400, 401, 403, 404, 409, 422	Revert	Deterministic client failures will not heal
Retry on 429 with jitter and a hard stop	Prove	Needs a clock-injection test
Retry around mail, deploy, or billing POST	Revert	Replay is a second effect
Idempotency key plus a replay test	Prove	The key is untrusted without the test
Hardcoded quota or a free-tier capacity claim	Revert	Unverified capacity is not an input
Test that only mocks the final 200	Revert the test	It hides the policy

A green unit test that stubs the second attempt as success does not move a row from Prove to Trust. The label column is the merge gate.

429 Too Many Requests is specified by RFC 6585. Retry-After belongs to HTTP semantics in RFC 9110. Treat other 4xx codes as non-retryable unless the spec for that code says the request can be repeated safely.

A classifier you can run locally

The module below is a review aid, not a client SDK and not a benchmark. It has no network calls. It targets Python 3.10 or newer.

The cap of 3 is a merge heuristic for interactive calls, not a vendor limit. Nothing here was executed for this article.

from dataclasses import dataclass

NON_RETRYABLE = frozenset({400, 401, 403, 404, 409, 422})
TRANSIENT = frozenset({408, 425, 429, 500, 502, 503, 504})

@dataclass(frozen=True)
class Attempt:
    status: int | None
    attempt: int
    max_attempts: int
    idempotent: bool
    writes_side_effect: bool
    honors_retry_after: bool

def label(a: Attempt) -> str:
    if a.max_attempts < 1 or a.max_attempts > 3:
        return 'revert'
    if a.writes_side_effect and not a.idempotent:
        return 'revert'
    if a.status in NON_RETRYABLE:
        return 'revert'
    if a.status == 429 and not a.honors_retry_after:
        return 'prove'
    if a.status in TRANSIENT or a.status is None:
        if a.attempt >= a.max_attempts:
            return 'trust_stop'
        return 'prove'
    if a.status is not None and 200 <= a.status < 300:
        return 'trust'
    return 'prove'

if __name__ == '__main__':
    samples = [
        Attempt(429, 1, 3, True, False, False),
        Attempt(401, 1, 2, True, False, True),
        Attempt(503, 2, 2, True, False, True),
        Attempt(503, 1, 8, False, True, True),
    ]
    for s in samples:
        print(s.status, label(s))

Expected lines are 429 prove, 401 revert, 503 trust_stop, and 503 revert. If those four labels are not obvious from the diff under review, the helper is not ready.

Pair the classifier with five local cases. Keep every case on a stub. None of them should open a socket.

1. Inject 401 and assert a single attempt.
2. Inject 429 with a two-second Retry-After and assert one bounded delay, not a tight loop.
3. Inject 503 until the cap and assert stop, counting the original call.
4. Mark a POST as a side effect without an idempotency key and assert refusal.
5. Advance a fake clock. Do not call time.sleep in CI.

class FakeClock:
    def __init__(self) -> None:
        self.delays: list[float] = []

    def sleep(self, seconds: float) -> None:
        if seconds < 0 or seconds > 30:
            raise AssertionError(seconds)
        self.delays.append(seconds)

That clock is an unexecuted proposal. Wire it into the helper under review. A production sleeper that ignores the injected function fails case 5 by construction.

Idempotency key, not a comment

A comment that says a call is safe to retry is not a key. Trust a stable key derived from the job id and the step name. Revert a key built with a fresh random value inside the loop. A new value per attempt defeats deduplication on the server.

def idempotency_key(job_id: str, step: str) -> str:
    # Unexecuted proposal: same key on every attempt of this step.
    return 'job-%s-%s' % (job_id, step)

Cancellation between attempts

A loop that checks an abort flag only before the first call will keep going after the worker is cancelled. Trust a check between attempts. Revert a helper that cannot observe shutdown. On a shared server, a cancelled job that continues is wasted capacity and a stale-write risk.

Commands for the pass

rg is convenient. git grep -n is the fallback when rg is absent. Replace the base ref with the PR base.

git diff --unified=5 origin/main -- '*.py' '*.ts' '*.go' | rg -n 'retry|backoff|max_retries|Retry-After|while True|tenacity'
rg -n 'time.sleep' tests

The second search is a smell detector, not a proof. A clean result still needs the five cases. A hit on time.sleep in tests means the clock is real, so the job can stall under a 429.

Suggested comment to paste and edit, not a record of a specific review. Trim it to the hunk you are looking at.

This wrapper multiplies calls. Cap reads at 2 attempts after you count the original call, refuse retry when the call writes state, and honor Retry-After on 429. Inject failures with a fake clock. A job that loops against a hosted server to see what happens should not be accepted.

Where a free model path changes the gate

Some teams let an agent draft this patch on a hosted assistant, then point CI at a free server instead of a stub. That choice does not relax the table.

Disclosure: This article was prepared as part of MonkeyCode's product outreach.

MonkeyCode's free model access and free server option are the only availability claims used here. This article does not state a token quota, a model list, hardware, uptime, or a duration. Those figures were not verified, and they are the wrong kind of constant to paste into a retry helper. If a diff comment cites a specific free allowance as the reason retries are safe, revert the justification and ask for a primary doc link in the PR.

Keep the hosted path in one place. Use free model access, if it is already available, to draft the classifier and the five tests. Run the tests on a local stub.

After they pass, one canary call can confirm auth and URL shape. It cannot tell you whether backoff terminates.

Do not parallelize that canary. Do not loop it to find a ceiling. A shared free server has neighbors.

It is not a load-test target. A 429 from it is a stop sign for this review, not a defect to hammer through.

Limitations

Status codes are a partial taxonomy. Timeouts, reset streams, and truncated JSON need separate branches. This classifier does not cover them.

Retry-After may arrive as a delta or as an HTTP date. The sample does not parse either form.

The cap of 3 is a review heuristic for interactive calls, not a capacity study. Batch jobs need a ceiling taken from your own queue metrics. This protocol will not tell you whether a model answer is correct. It only decides whether a retry policy is safe to land.

Skip the approach when the call sends mail, charges a card, or mutates production without an idempotency key already under test. Skip it if the only oracle available is the hosted server. Also skip it when the change needs a formal error budget. A table is a gate, not an SLO.

If MonkeyCode is already how the draft gets written, keep the free server outside the retry loop and spend one capped canary only after the local labels match. Read the live terms before anyone writes a number into the patch.