A Retry Is a Load Multiplier: Reviewing Agent PRs That Wrap Failures

Retries look like robustness. In an agent-generated pull request they are usually a policy change: more traffic, a longer tail, and a different definition of failure.

Treat a new retry loop the way you would treat a new queue. Count the extra calls. Name the status codes. Decide what happens when the first attempt already committed work.

Agents add retries because the prompt said “make it reliable.” Reliability without a budget is just amplification.

What the diff is actually changing

A typical agent patch wraps an HTTP or RPC call in for attempt in range(3) and sleeps on any exception. The production change is not the sleep. It is the multiplier on every timeout, every 500, and every caller that already fans out.

Three attempts on a handler that calls three dependencies is nine upstream requests in the worst case. Add a 2s backoff and a 10s client timeout, and you have rewritten the SLA without touching an SLO document.

Review that math before you review naming.

Signals that the retry is a product change

• New max_retries, attempts, or backoff constants with no owner comment.
• A catch of Exception, Error, or a transport timeout around a POST, PATCH, or DELETE.
• Sleeps that are linear (attempt * 0.5) with no jitter and no cap.
• Retries on 400, 401, 403, or 404 as well as 429 and 503.
• No idempotency key, no Retry-After handling, no metric for retry_count.

If two or more of those appear, do not merge on “looks defensive.” Merge only after the contract is explicit.

Trust, revert, prove

Use a three-bucket pass. Do not debate style until the buckets are filled.

Diff signal	Trust	Revert	Prove
Retry only on 429/503 with jitter and a hard cap	Keep the structure	Drop retries on 4xx and on POST without a key	Count upstream calls under injected 503s
Bare except Exception plus three attempts	Nothing	The catch-all and the loop	Re-introduce a typed retry allowlist
Timeout lowered and retries raised in the same PR	Neither change	One of the two; they fight	Show p99 vs attempt budget
Idempotency key added with the retry	The header plumbing	Key reused across users or requests	Duplicate POST does not double-apply
“Temporary” retry around a flaky test	Nothing	The retry in test helpers	Fix the race or mark the test

The table is the review. Comments about clean code come after the traffic story is settled.

Review fixture (labeled, unexecuted)

The following is a review fixture, not a production client and not a measured benchmark. It shows the question a reviewer must force the PR to answer: how many times does the upstream run?

# review_fixture_retry.py
from dataclasses import dataclass, field

RETRYABLE = {429, 503}


@dataclass
class FakeUpstream:
    status_sequence: list[int]
    calls: int = 0
    bodies: list[str] = field(default_factory=list)

    def handle(self, method: str, body: str) -> int:
        self.calls += 1
        self.bodies.append(body)
        idx = min(self.calls - 1, len(self.status_sequence) - 1)
        return self.status_sequence[idx]


def agent_style_call(up: FakeUpstream, method: str, body: str, attempts: int = 3) -> int:
    last = 599
    for _ in range(attempts):
        last = up.handle(method, body)
        if last < 500:  # agent often treats all 5xx + some 4xx as “transient”
            return last
    return last


def reviewed_call(
    up: FakeUpstream,
    method: str,
    body: str,
    attempts: int = 3,
    idempotency_key: str | None = None,
) -> int:
    if method in {"POST", "PATCH", "DELETE"} and not idempotency_key:
        return up.handle(method, body)  # no silent multiplier
    last = 599
    for _ in range(attempts):
        last = up.handle(method, body)
        if last not in RETRYABLE:
            return last
    return last

# test_retry_amplification.py
from review_fixture_retry import FakeUpstream, agent_style_call, reviewed_call


def test_agent_retry_multiplies_post_without_key():
    up = FakeUpstream(status_sequence=[503, 503, 200])
    status = agent_style_call(up, "POST", body='{"charge": 1}')
    assert status == 200
    assert up.calls == 3  # three charges if the first 503 already committed


def test_reviewed_post_does_not_retry_without_key():
    up = FakeUpstream(status_sequence=[503, 200])
    status = reviewed_call(up, "POST", body='{"charge": 1}')
    assert status == 503
    assert up.calls == 1


def test_reviewed_get_retries_only_allowlisted_codes():
    up = FakeUpstream(status_sequence=[503, 200])
    status = reviewed_call(up, "GET", body="")
    assert status == 200
    assert up.calls == 2


def test_reviewed_get_does_not_retry_404():
    up = FakeUpstream(status_sequence=[404, 200])
    status = reviewed_call(up, "GET", body="")
    assert status == 404
    assert up.calls == 1

Run the fixture locally:

python -m pytest -q test_retry_amplification.py

The interesting assertion is not status == 200. It is up.calls. Agent tests usually assert the happy path. Reviewer tests assert the multiplier.

A concrete review sequence

Work the PR in this order. Stop when a step fails; later nits are noise.

1. Name the unit of work. Is the call idempotent? If the handler creates a row, charges a card, or sends a webhook, a retry is a duplicate unless a key or a natural unique index exists.
2. Write the attempt budget as an inequality. attempts * per_try_timeout * fanout <= caller_timeout. If the PR cannot show this inequality, revert the retry values.
3. List retryable codes. Allow 429 and 503 by default. Require a comment for 408 or 425. Reject 400–404 and 401/403 unless the PR is talking to a known misbehaving dependency and the ticket says so.
4. Honor Retry-After. A fixed sleep that ignores the header will stampede when the dependency recovers.
5. Add jitter and a cap. Linear backoff without jitter synchronizes clients. Uncapped exponential backoff turns a 30s outage into a minutes-long retry storm.
6. Emit one metric and one log field. retry_count, retry_reason, idempotency_key present or absent. If you cannot graph retries, you cannot operate them.
7. Fail the first-attempt success that returns slowly. Timeout-then-retry is the double-apply path: the server may have finished after the client gave up.

Suggested review comment (copy, then edit)

Retry changes failure policy, not just resilience.

Please:
- Restrict retries to {429, 503} (and documented others).
- Do not retry POST/PATCH/DELETE without an idempotency key.
- Show attempts * timeout * fanout <= caller timeout.
- Add a test that asserts upstream call count, not only final status.
- Log retry_count and honor Retry-After.

I am -1 on merging the loop until the call-count test exists.

Short comments survive. Essays in GitHub do not.

Where a coding agent is useful — and where it is not

Reproducing the patch against a stub is the whole job. You do not need a large context window to count up.calls.

If you want an isolated checkout to replay the agent diff against the fixture above, MonkeyCode’s free model access and free server option can host that loop. Disclosure: This article was prepared as part of MonkeyCode's product outreach. Use them only as a sandbox for the tests; they do not decide whether a retry is safe.

Do not ask an agent to “add retries until CI is green.” That prompt produces the catch-all loop this review exists to reject. Ask it to generate the call-count test first. Then accept or revert the production loop against that test.

Limitations

This protocol assumes you can stub the dependency. It does not replace load tests for a retry storm at real QPS. It does not tell you the correct attempts for your SLO. Those numbers come from error budgets, not from a model.

Skip this approach when:

• The call is a non-idempotent write and you cannot add a key or a uniqueness constraint.
• The PR is already past incident response and you need a one-line freeze, not a redesign.
• The “retry” lives in a test helper to hide flakes. Delete it. Fix the race.
• You cannot observe retry count in production. Operating blind retries is worse than failing once.

A retry is a scheduler. Review it like one.