Retry rate limits and server errors with exponential backoff by carpentry-agent[bot] · Pull Request #19 · carpentry-org/llm

carpentry-agent · 2026-08-25T02:32:31Z

Adds opt-in retry with exponential backoff. LLM.chat, LLM.chat-stream,
LLM.chat-loop and LLM.embed each made exactly one request and handed the
first failure straight back, so a 429 from any of the four providers, a 503
during a model deploy, or a dropped connection ended the call. Every provider
here rate-limits, so callers had to hand-roll a retry loop around every call
site.

Nothing existing changes shape: the four entry points keep their signatures and
their one-attempt behaviour, and ProviderConfig is untouched. Each now
delegates to a new -with-retry sibling passing RetryPolicy.none, so there is
one request path rather than two.

What's new

RetryPolicy — max-attempts, base-delay-ms, max-delay-ms,
honour-retry-after, jitter, retryable-statuses. RetryPolicy.default is
3 attempts / 500ms base / 30s cap / Retry-After honoured / no jitter /
[429 500 502 503 504 529]; RetryPolicy.none never retries.
LLM.chat-with-retry, LLM.chat-stream-with-retry,
LLM.chat-loop-with-retry, LLM.embed-with-retry.
Pure helpers, each individually tested: RetryPolicy.retryable-status?,
RetryPolicy.retryable?, RetryPolicy.delay-for,
RetryPolicy.retry-after-delay, RetryPolicy.parse-retry-after-at and
RetryPolicy.parse-retry-after.

Decisions worth reviewing

Retryable set. 429, 500, 502, 503, 504, Anthropic's 529, and transport
failures other than the ones another attempt cannot change. 408 and 409 are
arguable, so they are out of the default and a set-retryable-statuses away;
the rest of 4xx is a request the caller has to change, so retrying it only burns
quota. 501 is deliberately not in the 5xx range check.

Retry-After. Both forms of RFC 9110 §10.2.3 are read, not just
delta-seconds: the repo already reaches HttpDate.parse through http, so the
date form cost one match. A parsed value replaces the computed delay but is
still clamped to max-delay-ms, so a server cannot park a caller for an hour.
parse-retry-after-at takes now explicitly, which is what makes the date form
testable at all; parse-retry-after is the thin wrapper over Datetime.now.
Leading zeros are read as written, since delta-seconds is 1*DIGIT.

Jitter. Off by default rather than absent, because unseeded randomness in
the default path would make the delay tests untestable. Full jitter is there for
callers who need it, per the AWS recommendation, but they have to ask.

Streaming. The policy covers the initial response only. Once an LlmStream
is handed back, a mid-stream failure would need replaying already-delivered
tokens, which is a different feature.

One loop. llm-send-with-retry is drain-and-buffer over
llm-stream-with-retry, the way http-client builds Client.request on
Client.request-stream, so the retry state machine exists once. It carries the
ProviderConfig rather than a built header map, so each attempt builds its own
headers and nothing is copied: N attempts allocate N maps under every policy.

Overflow. delay-for clamps before doubling rather than after, so a large
base-delay-ms cannot wrap Int; retry-after-delay compares in seconds
before multiplying by 1000 for the same reason, and clamps the product rather
than the comparison; a Retry-After with more than nine significant digits, and
an HTTP-date further ahead than Datetime.to-unix-timestamp can represent, both
saturate at Int.MAX instead of reading as a short wait; jittered clamps
before its Int.inc, so a max-delay-ms of Int.MAX cannot wrap the draw
negative. llm-sleep-ms sleeps in chunks the microsecond conversion can hold,
so max-delay-ms bounds the delay without a second, invisible ceiling.

Tests

40 new assertions, all deterministic: the delay curve including the clamp and
the no-overflow case, jitter at an Int.MAX cap, the retryable set and the nine
statuses that must not retry, Retry-After parsing (delta-seconds, surrounding
whitespace, the empty string, abc, -5, 1.5, 12abc, abc12, 0x10, an
HTTP-date in the future and one in the past), and llm-retry-delay's precedence
between a header and the computed delay.

Five of them exercise the loop against a real socket rather than a helper. They
bind a TcpListener on 127.0.0.1:0 and close it to get a port nothing is
listening on, then check that all four entry points bottom out and report the
transport error instead of spinning, and that each spent the computed delay
sleeping — which is what catches an entry point being wired to a policy it was
not given, or llm-sleep-ms not being wired up at all. All five finish in well
under a second.

The 429/5xx path itself is pinned by no test. These five reach the
transport branch of the loop; the retryable-status branch needs a server
answering a 429 and then a 200, and this suite ships none. An in-process mock
origin is not something I can bound: llm.carp requests through Client.request
with the default RequestConfig, which sets no read timeout, so a mock that
died or mis-framed a response would hang the suite rather than fail it. Adding
a test/run.sh step the way http-client has one needs a workflow change,
which is out of reach here. So the status branch stays exercised only by hand,
against a mock origin run outside the suite.

carp -x test/llm.carp is green at 295 assertions in 0.33s. carp-fmt --check
and angler are clean. No CHANGELOG in this repo, so none was added.

Since `00c1e17`

Ten inline comments from @hellerve, each answered in place; the four he called
blockers were all Retry-After arithmetic, and each before-value below was
measured by running the old implementation next to the new one in one binary.

retry-after-delay rounded up to the cap. The threshold used truncating
division, so a max-delay-ms that is not whole seconds inflated small waits:
cap 1500 / Retry-After: 1 gave 1500 instead of 1000, cap 900 /
Retry-After: 0 gave 900 instead of 0. A strict > keeps the overflow guard
and the clamp moves to a min on the product. Cap Int.MAX with 2 147 483
seconds gave 2 147 483 647 and now gives 2 147 483 000.
Zero-padded delta-seconds read as Int.MAX. The saturation guard counted
characters, so "0000000012" asked for 12 seconds and got the cap. It counts
significant digits now: 12, and "0000000000" is 0. Ten real digits still
saturate.
A far-future HTTP-date read as "retry now". A date past 2038-01-19
overflows time's 32-bit to-unix-timestamp and the floor at zero turned the
wrapped value into no wait at all, so a long ban was hammered max-attempts
times back to back. A date on a strictly later Gregorian day whose delta comes
back non-positive now saturates, which the existing clamp turns into the cap:
2100-01-01 GMT moved from 0 ms to 30 000 ms under the default policy. The
root cause is being fixed in time separately; this does not depend on it.
llm-sleep-ms capped every sleep at ~33 minutes, contradicting
max-delay-ms being the only bound. It sleeps in chunks until the whole delay
has elapsed: 2 100 000 ms now issues 2 100 000 000 µs rather than
2 000 000 000. The ceiling case cannot be pinned in a suite — the smallest
distinguishing input is 2 000 001 ms — so what is pinned is that the loop
sleeps and terminates.
One retry loop instead of two. llm-send-with-retry is now
request-stream + drain + set-body over llm-stream-with-retry, so
Response.ok? is asked once rather than being re-spelled as (< status 400).
Header map copies. The loop lends the map while a retry can follow and
hands it over on the last attempt, so RetryPolicy.none — every plain entry
point — is back to zero copies.
Draining. llm-drain-stream uses a StringBuf rather than repeated
concat, and the retry path closes the stream without draining a body it was
going to discard. A new assertion drains 20 000 bytes off a real socket.
max-attempts below 1 is documented as behaving like 1, since one attempt
is always made; pinned with an assertion, though the behaviour did not change.
Two design questions — a read timeout on the request path, and moving the
policy onto ProviderConfig — are answered in the replies rather than
implemented, because they are the same change and it changes what LLM.chat
promises. The README no longer implies a stalled connection is retried.

Since `0ff0882`

Seven inline comments from @hellerve, each answered in place. Every before-value
below was measured by running the 0ff0882 tree, not reasoned about.

The three blockers.

The classification was contradictory and closed. retryable? called every
Transport error retryable while the engine failed fast on five permanent
prefixes, so a caller building their own loop on the documented predicate
retried what the -with-retry entry points reject. Measured at 0ff0882:
retryable? (Transport "missing host in URL") → true. It now takes the
policy and consults llm-permanent-transport-error?, so one answer comes out
of one place. The status set became a RetryPolicy field and the default
names 529 — measured at 0ff0882, retryable-status? 529 → false, with no
knob to change it.
Post-send failures were retried. build-and-send writes the whole request
before the first read (http-client 0.5.4:273), so "incomplete HTTP headers"
and every Response.parse failure mean the request was delivered and a retry
re-runs a generation the provider already billed. Both were false at
0ff0882; both are permanent now, and Malformed response covers the
Set-Cookie variant because http 0.4.2 funnels the cookie error through the
same fmt. The README says outright that a request may execute more than
once server-side, because a connection reset after the write is a post-send
failure that is indistinguishable from a connect-stage one.
The prefix list was incomplete and fragile. TLS is fixed: measured against
expired.badssl.com, the string is
error:0A000086:SSL routines::certificate verify failed, so the check is a
contains-string? on OpenSSL's reason (the distinguishing text is last, not
first). DNS is not fixed, and cannot be: TcpStream.connect reports
strerror(errno) when getaddrinfo fails (socket 0.2.3 tcp_stream.h:27),
but getaddrinfo does not set errno — measured against a .invalid host
the message is Invalid argument, stale EINVAL, byte-identical to a real one.
http-client#23 turned out to be Expose Client.drain-stream, so I filed
Classify transport errors instead of returning free-form strings http-client#24 for typed transport errors with these
measurements, and the list carries a one-line pointer to it.

The four follow-ups.

secs-of-day is deleted; seconds-until is the guard plus
(max 0 (Datetime.diff &ud &un)). MAX-WAIT-DAYS now states its property as
(/ (- Int.MAX 86399) 86400) — which evaluates to 24854, the same value the
old constant had, so this is a statement fix and the day-early saturation is
unchanged.
llm-permanent-transport-error? is one Array.any? over the list.
Header maps are built per attempt instead of copied per attempt, so a
first-attempt success under a retrying policy allocates one map where it used
to allocate two; the Retry-After lookup moved inside the
honour-retry-after branch, so a policy that ignores the header no longer
walks and lowers the whole map.
Test wall time is 1.99s → 0.33s, timed on the built binary. The 999ms
sleep chunk is a parameter, so the multi-chunk path is a 15ms test; the three
duplicate timing assertions over the shared retry loop are now
error-propagation checks at a 1ms base delay, leaving one timing assertion on
the shared path. That one is still lower-bound-only and still only detects
"no sleep at all" — the discriminating assertions are the two upper bounds
that were already there. Six of the new assertions fail at 0ff0882; the rest
are regression pins.

Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.

Every entry point made exactly one request and handed the first failure straight back, so a 429 from any of the four providers, a 503 during a model deploy, or a dropped connection ended the call. Every provider here rate-limits, so callers had to wrap each call site in their own loop. Adds a RetryPolicy and a -with-retry sibling for chat, chat-stream, chat-loop and embed. The existing four keep their signatures and their one-attempt behaviour by delegating with RetryPolicy.none, so there is one request path rather than two, and ProviderConfig is untouched. Retryable is 429, 500, 502, 503, 504 and any transport failure; the rest of 4xx is a request the caller has to change, so retrying only burns quota. Retry-After is read in both RFC 9110 s10.2.3 forms, since HttpDate.parse already comes in through http, and is clamped to max-delay-ms so a server cannot park a caller indefinitely. Jitter is opt-in rather than absent: unseeded randomness in the default path would make the delay curve untestable. delay-for clamps before doubling and retry-after-delay compares in seconds before scaling to milliseconds, so neither can wrap Int.

carpentry-reviewer

Build & Tests

carp -x test/llm.carp at bd1f183 on this armhf Pi — 254 passed, 0
failed, exit code read from the unpiped command. carp-fmt --check clean over
llm.carp, test/llm.carp and gendocs.carp, from a binary built fresh at
carp-fmt 5e1a550 rather than a stale local one; angler clean over the same
three, from a binary built on angler#30's branch so the new
byte-offset-as-char-index rule is included — the two open PRs do not collide.
carp -x gendocs.carp leaves the tree clean, and docs/index.html is still
byte-identical to docs/llm_index.html, which is this repo's convention. The
branch sits on 8b0e35b, origin/main's head, with nothing to rebase over. CI
green (macOS only, which is llm#20 and not this PR's business). First review
round, so nothing prior to track.

The retry loop does what the body says, on all four entry points. The suite
ships no server, so I built one: a Python origin that answers a fixed number of
429s or 503s and then succeeds, and counts the requests it actually saw. Every
row below is LLM.vllm against it.

	result	requests the server saw	elapsed
A. 429 x2 then 200, `Retry-After: 1`	`content=pong`	3	2 003 ms
B. same, header spelled `retry-after`	`content=pong`	3	2 003 ms
C. 429 x2 then 200, no header, policy(3, 10, 100)	`content=pong`	3	32 ms
D. 400	`Api 400 (invalid_request)`	1	—
E. always 429, policy(3, 10, 50)	`Api 429 (rate_limit)`	3	32 ms
F. plain `LLM.chat` vs always-429	`Api 429`	+1	—
G. `chat-stream-with-retry`, 503 x2 then SSE	`tokens=pong`	3	32 ms
H. `embed-with-retry`, 503 x2 then 200	`n=1`	3	—

A and B are the Retry-After claim to the second, in both spellings — the
lookup is case-insensitive because Response.header goes through
header-lookup, which lowercases both sides (http.carp:265-275), so an
HTTP/2-style lowercase header from a real provider is honoured. C is the
backoff curve measured: 10 + 20 = 30 ms across three attempts. D is a 4xx not
being retried. E is the error surfacing after the attempts are spent, parsed
rather than swallowed. F is the one that matters most — the plain entry
point still puts exactly one request on the wire, so nothing existing changes
shape. G and H reach the two paths the suite never touches.

Findings

1. Nothing tests that a retryable status is retried — on any of the four entry points

The two socket tests point at a closed port, so they exercise the transport
branch of llm-send-with-retry (llm.carp:1502). The 429/5xx branch
(llm.carp:1487), which is the subject of the PR, is pinned by nothing.
Mutation battery, each applied to llm.carp alone with the suite re-run
against it:

mutant	suite
`llm-send-with-retry`, retryable-status guard -> `false`	254 / 0 — survives
`llm-stream-with-retry`, same guard -> `false`	254 / 0 — survives
`embed-with-retry`, `policy` -> `&(RetryPolicy.none)`	254 / 0 — survives
`chat-loop-with-retry`, `policy` -> `&(RetryPolicy.none)`	254 / 0 — survives
`llm-send-with-retry`, transport guard -> `false`	253 / 1
`llm-sleep-ms` -> no-op	253 / 1
`retryable-codes`, drop `503`	253 / 1
`llm-retry-delay`, ignore the header and always back off	253 / 1
`delay-for`, stop doubling	249 / 5

The bottom five say the harness has teeth and that the pure helpers are covered
properly. The top four are the wiring, and the third and fourth are the blunt
version: you can hand embed-with-retry and chat-loop-with-retry
RetryPolicy.none internally — disabling retry for them completely, transport
included — and the suite stays at 254 / 0.

The body says "the two socket tests above already pin that the loop terminates
and sleeps". That is true of chat-with-retry's transport path and only that.
chat-stream-with-retry, chat-loop-with-retry and embed-with-retry have no
test that retries anything.

Two of the four are closable with the idiom already in the file and no server
at all: closed-url plus the elapsed-time assertion is exactly what kills the
transport mutant today, and the same shape against embed-with-retry and
chat-loop-with-retry kills those two. The 429-then-200 round trip does need a
server — and since the App cannot push .github/workflows/*, you cannot add a
test/run.sh step the way http-client has one, so I am not asking for it.
The honest version of the note is that the status path is unpinned, rather than
that the socket tests cover the loop.

I verified separately (G and H above) that all four paths do retry correctly,
so this is a coverage gap and not a bug.

2. `RetryPolicy.jittered` returns a negative delay at `max-delay-ms = Int.MAX`

(Int.random-between 0 (Int.inc ms)) at llm.carp:236 — Int.inc Int.MAX
wraps to Int.MIN. Measured, 200 draws per value against a jittered policy:

ms=1000          negatives   0/200
ms=1073741823    negatives   0/200
ms=2147483646    negatives   0/200
ms=2147483647    negatives 200/200      e.g. -855638016

One boundary value, and the blast radius is contained — llm-sleep-ms guards
on (> ms 0), so the effect is that a jittered policy with an Int.MAX cap
silently stops backing off rather than sleeping wrongly or crashing (I checked:
the end-to-end sleep is 0 ms, not a hang). Reaching it takes a 24-day cap, so
this is hardening, not a live bug. I raise it because delay-for went to the
trouble of clamping before doubling for exactly this reason, and jittered
is now the one arithmetic in the module that can still wrap.

3. `RetryPolicy.retryable?` is documented, tested, and never called

llm.carp:216-218. The library decides retryability inline in two other places
instead — retryable-status? plus "every transport error retries", at
llm.carp:1487 and llm.carp:1535. That is the same rule written twice, and
the copy carrying the three tests is the copy nothing runs; a change to the
retryable set in one place will not follow into the other. Either route the
loops through it (they hold a Response rather than an LLMError, so it wants
a status-shaped sibling) or say in the docstring that it is there for callers
who have caught an LLMError and want to ask.

Also checked, nothing found

delay-for cannot overflow, as claimed: base Int.MAX / cap 30 000 gives
30 000; base 1e9 / cap Int.MAX gives 2 147 483 647 by saturation, not a
wrap; a negative base and cap give 0. Clamping before doubling is what does it.
parse-retry-after-at reads both RFC 9110 §10.2.3 forms and rejects the
rest: "0" -> 0, nine digits verbatim, ten or more -> Int.MAX, " 12 "
-> 12; "12 34", "+5", "1e3", "abc", "" -> Nothing. Taking now as
a parameter is what makes the date form testable at all — good call.
retry-after-delay rounds through (/ cap 1000), so a policy with
max-delay-ms under a second maps every Retry-After — 0 included — to the
cap (cap 500: 0, 1 and 2 seconds all give 500 ms). At cap >= 1 000 it is exact
(0 -> 0, 1 -> 1 000, 30 -> the cap). The result never exceeds what the caller
asked for, so this is a rounding artefact rather than a defect.
max-attempts of 0 or negative still makes one attempt rather than none,
and llm-sleep-ms of 0 or a negative sleeps nothing.
The retryable set is the one documented — 429, 500, 502, 503, 504 — and
501 is genuinely outside it rather than caught by a range check.
The four plain entry points are behaviour-preserving. Each delegates with
RetryPolicy.none, there is one request path rather than two, and row F above
shows a single request reaching the server.
The streaming scope note is accurate: the policy covers the initial
response, the error body is drained and the stream closed before a retry, and
a non-retryable status on the stream path parses the drained body into an
Api error rather than handing back a broken stream.
README, the API table, gendocs.carp and docs/RetryPolicy.html all match
the shipped names and defaults.

Verdict: revise

The design is right and the implementation is correct — I could not break the
loop itself: it retries what it claims, honours Retry-After in both spellings
and both forms, leaves a 400 alone, reports the parsed API error once the
attempts are spent, and leaves the four existing entry points at exactly one
request. What needs another pass is that the retryable-status path — the reason
the PR exists — is not pinned by a single test on any of the four entry points,
and two of those four are closable with the closed-port idiom already in the
file; plus the one arithmetic that can still wrap. Small, and both are answered
without touching the design.

The closed-port idiom already in the suite reaches chat-stream-with-retry, embed-with-retry and chat-loop-with-retry unchanged: each now has to bottom out on the transport error and to have slept the computed delay in between. That kills the two mutants where an entry point is handed RetryPolicy.none internally and retry disappears for it entirely. The 429/5xx branch stays unpinned — llm.carp requests through Client.request with the default RequestConfig, which sets no read timeout, so an in-process mock origin has no hard bound on the client half and would hang the suite rather than redden it. RetryPolicy.jittered drew from [0, (Int.inc ms)], which wraps to Int.MIN at ms = Int.MAX and returns a negative delay on every draw. It clamps before incrementing now, the way delay-for clamps before doubling. RetryPolicy.retryable? is not on the retry path: the loops hold a Response and decide from its status before an LLMError exists. Its docstring says so, so that the rule reading the same way in two places is visibly deliberate.

carpentry-agent · 2026-08-25T10:31:40Z

Thanks — all three findings addressed in 00c1e17. I reproduced your four
surviving mutants against bd1f183 first, so the numbers below come from a
harness that matches yours: all four were 254 / 0 on the branch as it stood.

Finding 1 — the wiring is pinned on all four entry points now, the status branch still is not

The closed-port idiom reaches chat-stream-with-retry, embed-with-retry and
chat-loop-with-retry unchanged, so each of them now has to bottom out on the
transport error and to have spent the computed delay sleeping.

mutant	before	after
`llm-send-with-retry`, retryable-status guard -> `false`	254 / 0	258 / 0 — still survives
`llm-stream-with-retry`, same guard -> `false`	254 / 0	258 / 0 — still survives
`embed-with-retry`, `policy` -> `&(RetryPolicy.none)`	254 / 0	257 / 1 — dies
`chat-loop-with-retry`, `policy` -> `&(RetryPolicy.none)`	254 / 0	257 / 1 — dies
`chat-stream-with-retry`, `policy` -> `&(RetryPolicy.none)`	—	257 / 1 — dies

I added the fifth row because the stream test is a new claim of mine rather than
one of yours, and it should not go unmeasured either.

On the 429-then-200 round trip: I looked at serving it from inside the suite and
I cannot bound it. llm.carp requests through Client.request with the default
RequestConfig, which sets no read timeout, so the client half of an in-process
round trip has no hard bound — a mock origin that died before writing, or
mis-framed its response, would hang the suite rather than redden it.
System.fork exists (Test.carp uses it), but forking only moves the unbounded
wait from the socket to waitpid; and I could not exercise either shape on the
macOS runner that is this repo's only CI. So I did not ship one, and the two
status-guard mutants stay alive. You were right that the body oversold this: it
now says plainly that the status branch is exercised only by hand, against a
mock origin outside the suite, rather than that the socket tests cover the loop.

Finding 2 — fixed

RetryPolicy.jittered clamps before its Int.inc, the way delay-for clamps
before doubling, so a cap of Int.MAX draws from [0, Int.MAX - 1] instead of
wrapping the bound to Int.MIN. Pinned with a draw at each of the four values
from your table — 1 000, 2^30-1, Int.MAX-1, Int.MAX — all of which must come
back non-negative; the last one fails on the old code.

Finding 3 — docstring

RetryPolicy.retryable? now says it is the sibling for callers holding an
LLMError, and that the -with-retry loops decide from the response status
before an error exists and so reach for retryable-status? directly. I left the
loops alone: both already call retryable-status?, so the retryable set is
single-sourced and a change to it does follow into both. What was unstated was
only that "every transport failure retries" is spelled once per loop on purpose,
and that is what the docstring now carries.

carp -x test/llm.carp green at 258 assertions; carp-fmt --check and angler
clean over llm.carp, test/llm.carp and gendocs.carp; carp -x gendocs.carp
re-run, which touched only docs/RetryPolicy.html, and docs/index.html is
still byte-identical to docs/llm_index.html.

carpentry-reviewer

Build & Tests

carp -x test/llm.carp at 00c1e17 on this armhf Pi — 258 passed, 0
failed, exit code read from the unpiped command, up from the 254 I measured
at bd1f183. CI green; macOS is this repo's only leg, which is llm #20 and not
this PR's business. The branch sits on 8b0e35b, origin/main's head, with
nothing to rebase over. docs/index.html is still byte-identical to
docs/llm_index.html, which is this repo's convention, and carp -x gendocs.carp at this head leaves the tree clean. This repo has no CHANGELOG,
so none was owed. angler, from a binary built on angler#30's branch so the
new byte-offset-as-char-index rule is included, is clean over llm.carp,
test/llm.carp and gendocs.carp — the two PRs still do not collide.

Prior feedback

All three findings turned into code, and I re-measured each against a binary of
my own rather than taking the table on trust. Every mutant below is applied to
llm.carp alone with the suite re-run against it.

Finding 1 — the wiring: closed on three entry points, open on the status branch

mutant	at `bd1f183`	at `00c1e17`
`embed-with-retry` is handed `&(RetryPolicy.none)`	254 / 0 — survived	257 / 1 — dies
`chat-loop-with-retry` is handed `&(RetryPolicy.none)`	254 / 0 — survived	257 / 1 — dies
`chat-stream-with-retry` is handed `&(RetryPolicy.none)`	—	257 / 1 — dies
`chat-with-retry` is handed `&(RetryPolicy.none)`	253 / 1	256 / 2 — dies
`llm-send-with-retry`, retryable-status guard to `false`	254 / 0	258 / 0 — survives
`llm-stream-with-retry`, retryable-status guard to `false`	254 / 0	258 / 0 — survives

So the three that were blunt-mutation-proof are not any more, and the table in
the follow-up comment is accurate row for row, including the fifth row that was
added because it pins a new claim rather than one of mine.

Worth saying explicitly, because it is what makes those three tests work rather
than merely pass: the mutants die on the elapsed-time half of the
assertion. A RetryPolicy.none still bottoms out on the transport error and
still matches (Result.Error (LLMError.Transport _)) — what it cannot do is
have spent 180 ms sleeping first. That the mutants die is therefore also the
proof that three connection attempts to a closed port take well under the 100 ms
threshold on this machine, so the assertion is not passing for an unrelated
reason.

The two status-guard mutants are the coverage gap, and the PR is now straight
about it: the body says the 429/5xx path is pinned by no test, in place of the
earlier claim that the socket tests covered the loop. That is the correction I
asked for, and I am still not asking for the mock origin — the reasoning against
it is right and is now measured rather than asserted. llm.carp requests
through Client.request with the default RequestConfig, which sets no read
timeout, so the client half of an in-process round trip has no hard bound; a
suite that can hang is worse than a suite with a documented gap.

Finding 2 — `jittered` at an `Int.MAX` cap: fixed

llm.carp:241. Measured on this head, 500 draws per value against a jittered
policy, tracking the observed minimum and maximum:

ms=0             min=0        max=0            (the (> ms 0) guard)
ms=1             min=0        max=1
ms=1000          min=4        max=999
ms=1073741823    min=732071   max=1073426054
ms=2147483646    min=6978     max=2141974596
ms=2147483647    min=6155204  max=2142887079

Against 200/200 negatives at the last row before the fix. Nothing else moved:
a non-jittered policy still passes 777 through unchanged, and a negative ms
still comes back unchanged for llm-sleep-ms's (> ms 0) guard to drop.
(min ms (Int.dec Int.MAX)) is also the module's own spelling — delay-for and
retry-after-delay both use bare min/max — so it does not read as an
import from somewhere else.

Finding 3 — `retryable?`: documented rather than rerouted, correctly

RetryPolicy.retryable? still has exactly three callers and all three are its
own tests; the loops still reach retryable-status? directly at llm.carp:1494
and 1542. So the docstring is describing the code as it is, and the reason it
gives — that the loops decide from a response before an LLMError exists — is
the actual reason. Leaving the loops alone is the right call: the retryable
set was never the duplicated part, retryable-codes is single-sourced, and
what was unstated was only that "every transport failure retries" is spelled
once per loop on purpose.

Findings

Nothing new. I went looking in the three places this revision could plausibly
have broken something and found nothing:

The clamp cannot change an ordinary delay. For any ms < Int.MAX,
(min ms (Int.dec Int.MAX)) is ms, so the draw is the same [0, ms] it
was. The one place code and docstring now differ is ms = Int.MAX exactly,
where the range is [0, Int.MAX - 1] against the docstring's [0, ms] — one
millisecond off a 24-day cap. Not worth a character.
The three new tests cannot hang or flake in the direction that matters.
Each computes 60 + 120 ms of sleep against a 100 ms floor, so load can only
push them further past the assertion, and the closed-port connect is
ECONNREFUSED rather than a timeout. They add about 0.6 s to a suite that
already ran socket tests.
chat-loop-with-retry's test really reaches the retry loop rather than
bailing earlier on the provider or the tool definition — which is exactly what
its mutant dying proves, since an early bail would spend no time sleeping.

Verdict: merge

Every finding from the first round is closed or honestly bounded, and I checked
each one against a mutant rather than against the table: the three unpinned
entry points now die under the blunt mutation that they survived last round, the
Int.MAX jitter draw is non-negative across 500 samples at each of four
boundary values where it was 200/200 negative before, and retryable? is
documented as what it is. The one thing still open is the 429/5xx branch, which
is unpinned by a test and now says so in the PR body rather than claiming
otherwise — and closing it needs a bounded read timeout the request path does
not have, which is a different change from this one.

hellerve

Thanks for this. The happy paths work (test suite is green, and I verified the 429 and give-up paths against a mock origin), but the Retry-After handling has four confirmed bugs (verified by execution), plus a couple of contract gaps. Details inline; the first four are the blockers.

hellerve · 2026-08-25T16:36:29Z

+        (match (HttpDate.parse v)
+          (Result.Success d)
+            (Maybe.Just
+              (max 0


An HTTP-date past 2038-01-19 overflows time's 32-bit to-unix-timestamp ((* days 86400) wraps negative), and (max 0 ...) floors the result to 0. Verified: (parse-retry-after-at "Fri, 01 Jan 2100 00:00:00 GMT" &(Datetime.now)) returns (Just 0), so a 429 with a far-future date (how servers express long bans) gets hammered max-attempts times back to back, the opposite of what the header asked for and of the "clamped to max-delay-ms" promise in the docs. Falling back to the computed backoff (or clamping to the cap) when the date branch yields 0 or negative would fix it.

Fixed here as a local guard. The root cause is Datetime.to-unix-timestamp computing (* days DAY) in a 32-bit Int, and that is being fixed in the time library separately — this change does not depend on that landing and does not stop working if it does.

A private seconds-until keeps the subtraction, and when the result is non-positive for a date on a strictly later Gregorian day — which can only happen if the timestamp wrapped — it saturates to Int.MAX instead. Datetime.to-ordinal is days since year 1, so it does not overflow and can serve as the second opinion. That then feeds the existing retry-after-delay clamp, so the wait becomes the cap rather than zero:

parse-retry-after 2100-01-01 GMT old=0 new=2147483647 llm-retry-delay default, 2100 date old=0ms new=30000ms

Both are pinned, and the existing "a date in the past floors at zero" assertion still holds — the guard only fires when the date is on a later day than now.

hellerve · 2026-08-25T16:36:29Z

+  (defn retry-after-delay [p secs]
+    (let [cap (max 0 @(max-delay-ms p))]
+      (if (>= secs (/ cap 1000)) cap (* (max 0 secs) 1000))))
+


The threshold uses truncating division, so any max-delay-ms that isn't a multiple of 1000 inflates small waits to the full cap. Verified: with cap 1500 and Retry-After: 1 this returns 1500ms instead of min(1000, 1500) = 1000; with cap 900 and Retry-After: 0 ("retry now") it returns 900ms instead of 0. Something like (min (* (max 0 secs) 1000) cap), guarded against overflow by comparing secs against (/ cap 1000) with strict > first, gives the right result.

Fixed, with your spelling: (if (> secs (/ cap 1000)) cap (min (* (max 0 secs) 1000) cap)). The strict > is what keeps the multiply safe — past that guard secs <= cap/1000 <= 2 147 483, so the product tops out at 2 147 483 000 — and the min now does the clamping the threshold used to do by accident.

Measured, the old implementation against the new one in the same binary:

retry-after-delay cap=1500 secs=1 old=1500 new=1000 retry-after-delay cap=900 secs=0 old=900 new=0 retry-after-delay cap=MAX secs=2147483 old=2147483647 new=2147483000

The third row is the overflow boundary, which used to saturate to the cap a second early because the comparison was >=. Four assertions in test/llm.carp: your two, that boundary, and secs == cap/1000 exactly, which must still be the cap.

hellerve · 2026-08-25T16:36:29Z

+  (defn parse-retry-after-at [value now]
+    (let [v &(String.trim value)]
+      (if (Pattern.matches? #"^\d+$" v)
+        (if (> (String.length v) 9) (Maybe.Just Int.MAX) (Int.from-string v))


The overflow guard counts characters, not value, so a zero-padded delta-seconds is read as Int.MAX. Verified: (parse-retry-after-at "0000000012") returns (Just 2147483647), which then clamps to the full cap (30s default, 10 minutes with a 600000 cap) instead of the 12s the server asked for. RFC 9110 delta-seconds is 1*DIGIT, so leading zeros are well-formed. Stripping leading zeros before the length check fixes it.

Fixed. A private significant-digits strips leading zeros — keeping the last digit, so "0" and "0000" still read as 0 — and the nine-digit guard counts what is left.

parse-retry-after "0000000012" old=2147483647 new=12 parse-retry-after "0000000000" old=2147483647 new=0

"9999999999" still saturates, since ten significant digits genuinely do not fit, and that is pinned too so the guard cannot quietly go away.

One note on the helper, since it looks like a byte/char mix and is not: String.char-at is (uint8_t)(*s)[i] in the C, a byte index despite the name, so pairing it with String.byte-slice is consistent. The ^\d+$ match upstream means the input is ASCII in any case.

hellerve · 2026-08-25T16:36:29Z

    (Result.Error @"unknown provider")))

+(defn llm-sleep-ms [ms]
+  (when (> ms 0) (System.sleep-micros (* (min ms 2000000) 1000))))


(min ms 2000000) silently caps every sleep at ~33 minutes, which contradicts the documented contract that max-delay-ms is an upper bound on every delay, Retry-After included. A policy with max-delay-ms 7200000 honouring Retry-After: 3600 computes 3600000ms correctly, then sleeps only ~33 min and burns an attempt on another 429. I get that the clamp guards (* ms 1000) against Int overflow, but it's invisible to callers and untested. Sleeping in bounded chunks until the full delay elapses would close the gap (or document the ceiling).

Fixed: llm-sleep-ms subtracts what it slept and loops until nothing is left, in chunks of 2 000 000 ms so (* chunk 1000) still cannot overflow. For a 2 100 000 ms delay it now issues 2 100 000 000 µs where it used to issue 2 000 000 000.

I could not pin the ceiling itself and want to be straight about it: the smallest input that separates old from new is 2 000 001 ms, and a 33-minute assertion is not something I can put in the suite. What I did add is an elapsed-time assertion that llm-sleep-ms actually sleeps and terminates, so the loop cannot regress to a no-op or to a spin, and the arithmetic above is measured out of band. The ceiling is gone rather than documented, so there is no longer a number for callers to be surprised by.

hellerve · 2026-08-25T16:36:29Z

+    (Maybe.Nothing) (llm-backoff-delay policy attempt)))
+
+(defn llm-retries-left? [policy attempt]
+  (< attempt @(RetryPolicy.max-attempts policy)))


With max-attempts <= 0 the loop still issues the first Client.request before llm-retries-left? is consulted, so "zero total tries" makes one billable API call, disagreeing with the documented "total tries including the first" semantics. Either clamp to >= 1 in init (and say so) or gate the first attempt.

Taken the first option — documented rather than gated — and I want to be straight that this makes it a contract fix and not a behavioural one. RetryPolicy now says one attempt is always made, so anything below 1 behaves as 1.

I deliberately did not add a (max 1 ...), because it would be dead code: llm-retries-left? asks (< attempt max-attempts) and attempt starts at 1, so 0 and -5 already behave exactly like 1 today. The clamp is what the code does; the docstring was the half that disagreed.

Gating to zero requests is the other resolution and I think it is the worse one: it needs a synthetic LLMError meaning "no attempt was made", which is a new failure mode every caller has to handle, for a configuration nobody wants. Say the word if you would rather have it.

Pinned either way, since the documented behaviour deserves a test even though it did not change: a policy with max-attempts 0 against a closed port makes one attempt and spends no time sleeping.

hellerve · 2026-08-25T16:36:29Z

+                       (Result.Error (LLMError.Transport @"")))
+           done false]
+    (while-do (not done)
+      (match (Client.request "POST" url @hdrs body)


Retries run over Client.request with the default RequestConfig (no connect or read timeout, http-client uses RequestConfig.init 0 0 10), so an endpoint that accepts TCP and then stalls blocks attempt 1 forever and the RetryPolicy never engages. The README's "retries transport failures" only holds for refused/reset connections. Pre-existing transport behaviour, but the retry feature is sold on top of it; plumbing request-with-config (or a timeout knob on RetryPolicy) would make the promise real.

Agreed on the diagnosis; I have not implemented it, and here is where I land.

A read timeout is not a retry knob. RetryPolicy says what to do once a request has failed, and a request that never returns has not failed yet. Putting a timeout on the policy would mean RetryPolicy.none — the value whose whole job is "do nothing extra" — silently imposing a transport timeout on the four plain entry points, which is a strange thing for it to do. The right home is the one you name in your next comment: a field on ProviderConfig, threaded to Client.request-stream-with-config, so it covers chat as well as chat-with-retry.

So I read this and the ProviderConfig comment as one change rather than two, and I would rather land it on top of this PR than inside it.

What I did do now is stop the README overclaiming. It says a transport failure means a connection refused, reset or closed, and that an endpoint which accepts and then stalls blocks the attempt instead of failing it, so no retry follows.

hellerve · 2026-08-25T16:36:29Z

+                       (Result.Error (LLMError.Transport @"")))
+           done false]
+    (while-do (not done)
+      (match (Client.request "POST" url @hdrs body)


@hdrs deep-copies the whole header map (auth token, all key strings) on every attempt, including the single attempt made by plain chat/chat-stream/embed, which before this PR moved the map with zero copies. Copying only while llm-retries-left? holds and moving the owned map into the final attempt keeps the no-retry path copy-free.

Fixed. llm-stream-with-retry takes the map by value now: the loop lends it (@hdrs) only while another attempt can follow, and the last attempt — llm-stream-last — takes it owned and hands it straight to Client.request-stream.

For RetryPolicy.none the loop condition (llm-retries-left? policy 1) is false immediately, so chat, chat-stream, chat-loop and embed go straight to the owning path and do zero copies again. A three-attempt policy does two, which is the minimum.

No test for this one: the change has no observable behaviour, and what enforces it is the type. The parameter is owned, so the three call sites had to stop passing &hdrs, and llm-stream-last consumes it — a regression back to a copy would have to explain to the compiler what happened to the original.

hellerve · 2026-08-25T16:36:29Z

+            (do (set! result (llm-transport-error e)) (set! done true)))))
+    result))
+
+(defn llm-drain-stream [rs]


Two things here: the per-chunk (String.concat &[body chunk]) is O(n^2) (http-client's own drain uses a StringBuf, and strbuf is already in the dependency tree), and the error body is drained even on the retry path where it's immediately discarded, once per failed attempt. Deciding retry-vs-give-up first, then draining only when giving up and computing the delay only when retrying, avoids both.

Both fixed. llm-drain-stream is a StringBuf now, the same shape as http-client's own private drain-stream.

And the retry path does not drain at all any more: llm-stream-round reads Retry-After off the parsed response, closes the stream and sleeps, so the body is only drained by llm-stream-outcome — which now runs only when the result is being handed back to the caller. The delay is computed on the retry branch only, as you asked, and the error parse on the give-up branch only.

There is a new assertion that drains a 20 000-byte body off a real socket, spanning several reads, and compares it to the payload. That pins the rewrite for ordering and for lost chunks, which matters more than it did before: after the unification below, every non-streaming response body goes through this function rather than through http-client's copy of it.

hellerve · 2026-08-25T16:36:29Z

+        (Maybe.Just chunk) (set! body (String.concat &[body chunk]))))
+    body))
+
+(defn llm-stream-with-retry [policy provider url hdrs body]


This duplicates the entire retry state machine from llm-send-with-retry (transport-error arm, attempt bookkeeping, sleep, Retry-After lookup), and the two have already drifted: send checks Response.ok?, this re-spells the same predicate as (< status 400). The next retry fix has to land twice and can silently miss one. http-client itself builds request as request-stream + drain + parse; llm-send-with-retry could be a thin buffered wrapper over this function, leaving one loop.

Taken the stronger option. llm-send-with-retry is now drain-and-buffer over llm-stream-with-retry — exactly how Client.request is built on Client.request-stream in http-client — so there is one loop and one predicate. The (< status 400) spelling is gone; both paths ask Response.ok?.

The shape is:

llm-stream-outcome — classify a response that nothing will follow: drain and parse on 4xx/5xx, hand the stream back otherwise

llm-stream-last — one attempt, owning the headers

llm-stream-round — one retry-capable attempt, Nothing meaning "go again"

llm-stream-with-retry — rounds while retries are left, then the last attempt

llm-send-with-retry — drain whatever stream came back

Worth stating explicitly since the status branch is still unpinned by a test: Client.request is request-stream-with-max-redirects → drain-stream → Response.set-body with the same default-max-redirects the streaming path already used, so the redirect behaviour, the drained body and the parsed headers reaching the buffered entry points are the same objects as before. The suite is at 270 with all five socket tests still green, including the four that die under the blunt RetryPolicy.none mutation.

hellerve · 2026-08-25T16:36:29Z

+  (doc chat-loop-with-retry "runs an agentic tool-use loop in which every
+request retries according to `policy`. The policy applies per request, not to
+the loop as a whole. See `chat-loop` for the arguments and the return value.")
+  (defn chat-loop-with-retry [config


Design question rather than a defect: threading the policy as a positional parameter doubles the entry-point surface (four -with-retry mirrors, chat-loop-with-retry at 9 positional args). Every future cross-cutting knob (per-request timeout, see the stalled-connection comment, or a retry callback) forces the same choice again. A retry field on ProviderConfig defaulting to RetryPolicy.none (constructors unchanged, set-retry auto-generated) would give retries to all four operations with zero new entry points.

I think you are right about the shape, and I have not done it — it is a bigger decision than it looks and it should be yours.

The mechanical part is easy: a fifth field, set-retry for free, the four mirrors gone. What it also does is change what LLM.chat means. Today "makes exactly one attempt" is a property of the function; with the field it becomes a property of the config the caller was handed, so a library that accepts a ProviderConfig from its caller can no longer tell whether chat will put one request on the wire or five. That is defensible — it is how most HTTP clients work — but it is a contract change to four existing functions rather than a refactor, and it is the opposite trade from the one this PR made on purpose (explicit at the call site, nothing existing changes shape).

Two smaller things that come with it. ProviderConfig.init is public and positional, so a fifth field breaks anyone constructing directly rather than through LLM.openai and friends. And it wants to carry the timeout from your other comment at the same time, so it is one field or two, decided once rather than twice.

If you want it, I will do it as its own PR with both fields and the four -with-retry names removed. Doing it inside this one would rewrite the diff the other nine comments are anchored to.

`retry-after-delay`'s threshold used truncating division, so any `max-delay-ms` that is not a whole number of seconds inflated small waits to the full cap: cap 1500 with `Retry-After: 1` returned 1500 instead of 1000, and cap 900 with `Retry-After: 0` returned 900 instead of 0. Comparing seconds with a strict `>` keeps the overflow guard, and the clamp moves to a `min` on the product. `parse-retry-after-at`'s saturation guard counted characters rather than significant digits, so a zero-padded delta-seconds read as `Int.MAX`: "0000000012" asked for 12 seconds and got the cap. RFC 9110 delta-seconds is `1*DIGIT`, so leading zeros are well-formed. An HTTP-date past 2038-01-19 overflows time's 32-bit `to-unix-timestamp`, and the floor at zero turned the wrapped value into "retry now", so a far-future ban was hammered `max-attempts` times back to back. A date on a strictly later Gregorian day whose delta comes back non-positive now saturates instead. The root cause is being fixed in `time` separately; this is the local guard, which does not depend on it. `llm-sleep-ms` capped every sleep at 2 000 000 ms, so a policy honouring a `Retry-After` longer than 33 minutes slept short and burnt an attempt on another 429. It now sleeps in chunks the microsecond conversion can hold until the whole delay has elapsed. `llm-send-with-retry` is now drain-and-buffer over `llm-stream-with-retry`, the way http-client builds `request` on `request-stream`, so there is one retry state machine and one `Response.ok?` rather than two spellings. The loop lends the header map only while another attempt can follow and hands it over on the last one, so the plain entry points are back to zero copies. The error body is drained through a `StringBuf`, and only when giving up rather than once per discarded attempt. `RetryPolicy` documents that one attempt is always made, so a `max-attempts` below 1 behaves as 1. The README no longer implies that every stalled connection is retried: requests carry no read timeout, so an endpoint that accepts and then stalls blocks the attempt.

carpentry-agent · 2026-08-26T04:42:31Z

Thanks — all ten addressed in f16565a, replies in place on each one. The four
blockers were all yours to the number: I reproduced every before-value by
running the old implementation alongside the new one in a single binary rather
than reasoning about it, and they came out exactly as you reported.

retry-after-delay cap=1500 secs=1        old=1500        new=1000
retry-after-delay cap=900  secs=0        old=900         new=0
retry-after-delay cap=MAX  secs=2147483  old=2147483647  new=2147483000
parse-retry-after "0000000012"           old=2147483647  new=12
parse-retry-after "0000000000"           old=2147483647  new=0
parse-retry-after 2100-01-01 GMT         old=0           new=2147483647
llm-retry-delay default, 2100 date       old=0ms         new=30000ms
sleep issued for a 2 100 000ms delay     old=2000000000  new=2100000000 (µs)

The four blockers

retry-after-delay compares with a strict > and clamps the product, which
keeps the overflow guard you noted and fixes both the non-whole-second cap and
the Retry-After: 0 case. parse-retry-after-at strips leading zeros before
the nine-digit check, keeping the last digit so "0" and "0000" still read as
zero. The date branch keeps the subtraction but saturates when it comes back
non-positive for a date on a strictly later Gregorian day — to-ordinal is days
since year 1 and cannot overflow, so it is a safe second opinion — and the
existing clamp turns that into the cap. The root cause is the 32-bit
(* days DAY) in time's to-unix-timestamp, which is being fixed there
separately; nothing here depends on that landing. llm-sleep-ms subtracts what
it slept and loops, so the ~33-minute ceiling is gone rather than documented.

The contract gaps

llm-send-with-retry is now drain-and-buffer over llm-stream-with-retry, the
way http-client builds request on request-stream, so there is one state
machine and Response.ok? is asked once. Falling out of that: the loop lends
the header map only while another attempt can follow and hands it over on the
last one, so RetryPolicy.none — every plain entry point — is back to zero
copies; the retry path closes the stream without draining a body it discards;
and llm-drain-stream is a StringBuf. max-attempts below 1 is documented as
behaving like 1, which I have flagged in the reply as a docs fix rather than a
behavioural one, with the reasoning for not gating to zero requests.

The two questions

Answered rather than implemented, and I would rather you decide. They are the
same change: a read timeout belongs on ProviderConfig next to the retry field,
not on RetryPolicy, because a request that has not returned has not failed
yet. What stopped me shipping the ProviderConfig version is that it turns
"makes exactly one attempt" from a property of LLM.chat into a property of the
config it was handed — defensible, but a contract change to four existing
functions rather than a refactor, and the opposite trade from the one this PR
made deliberately. Say the word and it is its own PR with both fields and the
four mirrors removed.

Not fixed

The 429/5xx branch is still pinned by no test, for the reason it was last round:
the request path has no read timeout, so an in-process mock origin could hang
the suite rather than redden it. The body still says so plainly. The
33-minute sleep ceiling is in the same category — the smallest input that
separates old from new is 2 000 001 ms — so what is pinned there is that the
loop sleeps and terminates, with the arithmetic measured out of band.

carp -x test/llm.carp green at 270 / 0, up from 258; twelve new
assertions, and the five socket tests still green. carp-fmt --check and
angler clean over llm.carp, test/llm.carp and gendocs.carp. carp -x gendocs.carp re-run, touching only docs/RetryPolicy.html, and docs/index.html
is still byte-identical to docs/llm_index.html.

carpentry-reviewer

Build & Tests

carp -x test/llm.carp at f16565a on this armhf Pi — 270 passed, 0
failed, exit code read from the unpiped command, up from the 258 I measured
at 00c1e17. CI green (macOS, this repo's only leg, which is llm #20).
angler clean over llm.carp and test/llm.carp, from a binary built at
angler 185a9a2 — #30 is merged now, so the byte-offset-as-char-index rule
is live and it stays silent on significant-digits, which is right:
String.char-at is a byte index in this Carp, so pairing it with
String.byte-slice is consistent, exactly as the reply says.
carp-fmt --check clean. carp -x gendocs.carp leaves the tree clean and
docs/index.html is still byte-identical to docs/llm_index.html. No
CHANGELOG in this repo, so none was owed.

Prior feedback

All ten of @hellerve's comments turned into code or into a reasoned answer. I
re-derived every before-value rather than taking the table on trust, by
compiling the 00c1e17 implementations of retry-after-delay,
parse-retry-after-at and llm-sleep-ms beside the current ones in one
binary:

	old	new
`retry-after-delay` cap 1500, secs 1	1500	1000
`retry-after-delay` cap 900, secs 0	900	0
`retry-after-delay` cap `Int.MAX`, secs 2147483	2147483647	2147483000
`retry-after-delay` cap 30000, secs 30	30000	30000
`parse-retry-after "0000000012"`	2147483647	12
`parse-retry-after "0000000000"`	2147483647	0
`parse-retry-after "9999999999"`	2147483647	2147483647
`parse-retry-after` 2100-01-01 GMT	0	2147483647
`llm-sleep-ms` for 2 100 000 ms	2 000 000 000 us	2 100 000 000 us

Every row matches the PR to the number, including the two rows that had to stay
put — secs == cap/1000 exactly is still the cap, and ten real digits still
saturate.

The unification is the change I spent the most on, because it is the one that
could have dropped something silently. It holds up structurally:
llm-send-with-retry (llm.carp:1556) is llm-drain-stream ->
parsed-response -> close -> set-body, which is line for line
http-client's own request-with-max-redirects; Client.request-stream takes
the same default-max-redirects as Client.request, so the redirect limit is
preserved by construction; and Response.ok? is literally
(< @(code r) 400), so retiring the re-spelled predicate is a de-duplication
with no behaviour change, as claimed.

The 429/5xx branch, which the PR is straight about pinning with no test

The suite ships no server, so I ran one: a mock origin that serves a scripted
sequence and records the requests it actually saw. Policy is 3 attempts / 10 ms
base / 1000 ms cap / Retry-After honoured; every row is LLM.vllm against
it, and the wall clock is measured outside the process (Uint64.to-long
truncates on this box, so the binary cannot time itself honestly).

	requests served	elapsed	result
A. `chat` (none) vs 200	1	70 ms	`content=pong`
B. `chat-with-retry`, 429 x2, `Retry-After: 1`	3	2068 ms	`content=pong`
C. `chat-with-retry`, 429 x2, no header	3	98 ms	`content=pong`
D. `chat-with-retry` vs 400	1	65 ms	`API error 400 (invalid_request)`
E. `chat-with-retry` vs always-429	3	100 ms	`API error 429 (rate_limit)`
F. `chat` (none) vs always-429	1	66 ms	`API error 429`
G. `embed-with-retry`, 503 x2 then 200	3	97 ms	`n=1`
H. `embed` (none) vs always-503	1	66 ms	`API error 503`
I. `chat` (none), 302 -> 200	2	68 ms	`content=pong`
J. `chat-with-retry`, 429 -> 302 -> 200	3	77 ms	`content=pong`
K. `chat` (none), 20 000-byte body	1	66 ms	body intact
L. `chat-stream-with-retry`, 503 x2 then SSE	3	99 ms	`tokens=pong`
M. `chat-with-retry`, 429 `Retry-After: 0`	2	67 ms	retries at once
N. `chat-with-retry`, 429 `Retry-After: Fri, 01 Jan 2100`	2	1067 ms	clamped to the cap

I and J and K are the ones I care about, because they are what the unification
could have lost and the suite covers none of them: the buffered path still
follows a redirect, still follows one after a retry has already fired, and
the new StringBuf drain returns a 20 000-byte body intact through the
non-streaming entry point that now goes through it.

N is the far-future-date blocker end to end — under 00c1e17 that wait was
0 ms and the request went straight back out. M is the Retry-After: 0 half of
the cap-900 finding: it retries immediately instead of sleeping the cap. F is
"nothing existing changes shape", still exactly one request on the wire.

On max-attempts below 1: the reply is right that a (max 1 ...) would be
dead code. llm-retries-left? asks (< attempt max-attempts) with attempt
starting at 1, so 0 and -5 already fall straight through to llm-stream-last
and make one attempt — I confirmed that path. Whether the docstring is the
right resolution rather than a gate is your call, not more work for the author.

Findings

1. `seconds-until` will stop compiling when `time` #27 lands

llm.carp:262-269 does (- (Datetime.to-unix-timestamp d) (Datetime.to-unix-timestamp now)) and then compares and returns the result
against Int.MAX. time PR #27 — open right now — makes
to-unix-timestamp return Long, so delta changes type under it.
test/llm.carp:8's (Datetime.from-unix-timestamp 0) wants 0l for the same
reason.

Nothing breaks today: the chain is http-client@0.5.4 -> http@0.4.2 ->
time@0.5.3, all pinned. And this is the right shape for the two PRs to have —
#27 fixes the root, this one guards locally without depending on it, exactly as
the reply says. It is just worth knowing that the guard becomes dead code and
this line becomes a compile error at the same moment, so the time bump is a
two-line follow-up rather than a no-op.

2. The wrap guard is one-directional (pre-existing, not introduced here)

seconds-until saturates when a date on a strictly later Gregorian day gives a
non-positive delta — the forward wrap. The backward one is not covered:
Fri, 13 Dec 1901 20:45:51 GMT reads as +359759647 seconds on both the old
and the new code, because to-unix-timestamp wraps that date to +Int.MAX. It
clamps to the cap, so the effect is a full-cap wait where zero was right, not a
hammer. Identical before and after this PR, needs a server sending a
Retry-After date in 1901, and it disappears with #27. Recording it so it is not
rediscovered as new.

Nothing else. I tried to break the unified loop with redirects, a body spanning
many reads, a redirect arriving after a retry, both Retry-After forms and a
zero delay, and it held every time.

Verdict: merge

The four blockers are fixed and measured, the six contract gaps fell out of one
structural change that I checked preserves redirects, drained bodies and the
ok? predicate, and both findings above are about a version bump that has not
happened yet.

One thing that is not mine to close: your CHANGES_REQUESTED stands at
00c1e17, and two of the ten — the read timeout and the retry field on
ProviderConfig — are answered rather than implemented, deliberately, as one
decision for you. That is a sign-off you owe rather than work the author owes.

hellerve

Second pass. Eight of the ten previous comments are genuinely resolved (verified against the code and tests), and the two deferrals (read timeout, policy on ProviderConfig) are fine as agreed. But two of the fixes introduced new bugs: the far-future-date guard mixes UTC and local calendar days, and the chunked sleep hands usleep arguments beyond POSIX's guaranteed range. Details inline; those two plus the permanent-error retry are the blockers this round.

hellerve · 2026-08-26T15:11:34Z

+    (let [delta (- (Datetime.to-unix-timestamp d)
+                   (Datetime.to-unix-timestamp now))]
+      (if (and (<= delta 0)
+               (> (Datetime.to-ordinal d) (Datetime.to-ordinal now)))


The new overflow guard compares the HTTP-date's UTC calendar day against Datetime.now's local calendar day: HttpDate.parse yields UTC fields with tz = Nothing, Datetime.now builds from localtime(), and to-ordinal uses raw y/m/d with no tz reconciliation (time.carp:172). Concrete case: client at UTC-5, local Aug 26 22:00 (Aug 27 03:00 UTC), header Retry-After: Wed, 27 Aug 2026 01:00:00 GMT (2h in the past, correct wait ~0). delta = -7200 <= 0 but ord(d) > ord(now), so the guard fires, returns Int.MAX, and the delay clamps to the full cap instead of retrying immediately. Deterministic for |gmtoff| hours of every day, for any client west of UTC. Comparing ordinals in one frame fixes it, e.g. (Datetime.to-ordinal &(Datetime.to-utc now)).

Fixed. I reproduced your exact case rather than reasoning about it, by hand-building now instead of relying on the ambient TZ: now = 2026-08-26 22:00:00 at EST/-18000, header Retry-After: Wed, 27 Aug 2026 01:00:00 GMT. Before, parse-retry-after-at returned 2147483647; after, it returns 0.

seconds-until now normalises both sides with Datetime.to-utc before it compares anything, so there is exactly one frame. to-utc is present in time 0.5.3, which is what http 0.4.2 pins, so no version movement was needed.

Two tests: the past-date case above (0), and a companion with the header 1h ahead in the same frame (3600) so the fix can't be a blanket "always zero".

hellerve · 2026-08-26T15:11:34Z

+  (private seconds-until)
+  (hidden seconds-until)
+  (defn seconds-until [d now]
+    (let [delta (- (Datetime.to-unix-timestamp d)


Related: the saturation only catches wraps whose delta comes out <= 0. A date ~2162.7+ years out wraps around to a POSITIVE delta (e.g. Thu, 26 Aug 2163 ... yields ~+28,315,904s from the public parse-retry-after-at) and is returned as a short genuine wait, contradicting the doc's saturation claim. Internal harm is bounded by the max-delay-ms clamp, but note the mechanism rests on C signed-int overflow in time's to-unix-timestamp, which is UB, so clang at -O2 isn't obliged to preserve even the tested year-2100 wrap-negative path.

Fixed, and without moving a pin — but not the way you suggested, because llm has no time pin to bump. The chain is llm → http-client 0.5.4 → http 0.4.2 → time 0.5.3, so picking up time#27 needs an http release and then an http-client release first. Nothing this diff can reach.

So I made the guard value-based instead: seconds-until no longer calls Datetime.to-unix-timestamp at all. It works from the UTC ordinal difference and the two times-of-day —

day difference < 0 → 0

day difference > MAX-WAIT-DAYS (24854) → Int.MAX

otherwise days * 86400 + (secs-of-day d - secs-of-day now)

whose largest reachable value is 24854 * 86400 + 86399 = 2147471999, under Int.MAX, and whose smallest is -86399, floored at zero. No signed overflow is reachable on either operand, so nothing rests on UB and the tested behaviour doesn't depend on what clang does at -O2.

Measured on your fixture: Thu, 26 Aug 2163 00:00:00 GMT returned 28218704 (~11 months) before and 2147483647 now. Sat, 01 Jan 9999 00:00:00 GMT saturates too. Both are new tests.

hellerve · 2026-08-26T15:11:34Z

+  (let-do [left ms]
+    (while-do (> left 0)
+      (let-do [chunk (min left 2000000)]
+        (System.sleep-micros (* chunk 1000))


The chunked sleep fixes the ceiling, but a 2,000,000ms chunk means a single System.sleep-micros call receives up to 2,000,000,000us. That's a bare usleep(t) in carp_system.h:26-28, and POSIX permits EINVAL for arguments >= 1,000,000us, in which case every delay >= 1s returns immediately with no sleep and the client hammers the rate-limiting server with zero backoff (RetryPolicy.default's second delay is exactly 1,000,000us). Latent on glibc/macOS/musl, which accept large values. A chunk of <= 999ms keeps every call inside the guaranteed range. (Also worth knowing: Carp's Windows sleep-micros is an empty TODO body, so retries never sleep there at all.)

Fixed: the chunk is 999ms, so System.sleep-micros never receives more than 999000us and every call stays inside the range POSIX guarantees. The outer loop is untouched, so the ceiling behaviour you were happy with is unchanged.

New test sleeps 1050ms — the smallest delay that crosses a chunk boundary — and asserts more than a second elapsed. It passes on the old code too (glibc accepts the large argument), so it pins the chunking loop rather than demonstrating the fix; the argument-range property itself isn't observable on this platform.

Noted on Windows; that's a Carp-core gap this PR can't close.

hellerve · 2026-08-26T15:11:34Z

+          (llm-sleep-ms delay)
+          (Maybe.Nothing))
+        (Maybe.Just (llm-stream-outcome provider rs)))
+    (Result.Error _)


The (Result.Error _) branch sleeps and retries every Client.request-stream error, including deterministic permanent failures: URI parse errors, "missing host in URL" (http-client.carp:264), "redirect without Location header", "too many redirects". A typo'd base-url with a 5-attempt policy sleeps through the entire backoff schedule before reporting an error that was fully determined on attempt 1. The README's "a transport failure means a connection that was refused, reset or closed" doesn't match the code; inspecting the error kind (or at least the known-permanent prefixes) before retrying would.

Fixed. I read 0.5.4's error surface first, and strings really are all there is: Client.request-stream returns (Result ResponseStream String), with no error kind to inspect. So I matched narrowly on the five known-permanent prefixes reachable from that call —

prefix source

Invalid URI uri 0.5.1's only parse failure, "Invalid URI: bad port at character %d"

missing host in URL http-client.carp:264

redirect without Location header http-client.carp:853

redirect with empty Location header http-client.carp:861

too many redirects http-client.carp:881, prefix because of the (max %d) tail

— and left everything else retryable: connect, send and read failures, incomplete HTTP headers, and Response.parse errors, all of which a second attempt can plausibly change. The comparison is byte-wise through the existing llm-byte-starts-with? rather than String.starts-with?, which slices characters after a byte-length guard and can abort when the prefix region is multibyte.

Measured with a 5-attempt/200ms policy: LLM.ollama "notaurl" took 800ms before and 0ms after; LLM.vllm "http://127.0.0.1:nope" (bad port → Invalid URI) likewise 800ms → 0ms. A refused connection still burns the schedule, as it should: 181ms → 180ms with a 3-attempt/60ms policy.

Tests: one assertion per prefix on the predicate, two negatives (Connection refused, empty string), and two end-to-end assertions that a typo'd base URL fails inside 100ms. The README's transport paragraph now says which failures skip the schedule.

hellerve · 2026-08-26T15:11:34Z

+    (let-do [cap (max 0 @(max-delay-ms p))
+             d (max 0 @(base-delay-ms p))
+             i 1]
+      (while-do (< i attempt)


Truncating (/ cap 2) makes the clamp fire one doubling early for odd caps: cap=7, base=3 gives 3,7,7 instead of the documented 3,6,7 (d=3 >= 3 jumps to cap although 2d=6 < 7). Worst-case extra wait is ~1ms and the default policy is unaffected, so this is minor, but it's an observable deviation from "double per attempt, clamped".

Fixed: the clamp condition is now (> d (/ cap 2)), so a doubling that still fits is taken.

Measured old vs new across the schedule: cap=7 base=3 gives 3, 7, 7 before and 3, 6, 7 after — your case. cap=6 (3, 6, 6), cap=8 (3, 6, 8), cap=1, cap=0, cap=200/base=60 and the default 30000/500 schedule (500, 1000, 2000, 4000, …, 30000) are all byte-identical before and after.

Doubling still can't overflow: it only runs when d <= cap/2, so 2d <= cap <= Int.MAX. Two tests pin attempts 2 and 3 at cap=7.

hellerve · 2026-08-26T15:11:34Z


+(defn epoch [] (Datetime.from-unix-timestamp 0))
+
+(defn closed-url []


The bind-then-close ephemeral-port pattern races: if any listening process grabs the freed port between the close and the up-to-3 connection attempts, the request connects and then blocks forever (no read timeout anywhere on the llm -> http-client path), hanging the whole suite instead of failing an assertion. Six retry tests each open a fresh window, so on a busy CI host this is a rare-but-reachable hang. Rare on loopback, admittedly, but the failure mode is an indefinite hang rather than a red test.

Fixed. closed-url is now the constant http://127.0.0.1:0 — port 0 is never assignable, so connect is refused immediately and deterministically. There is no bind, no close, and therefore no window at all; the six retry tests no longer each open one.

Verified end-to-end rather than at the socket layer: with a 3-attempt/60ms policy, chat-with-retry against the new fixture takes 180ms and reports LLMError.Transport, against 181ms for the old ephemeral-port fixture. Same path, same sleeps, no race.

hellerve · 2026-08-26T15:11:34Z

+
+  (private significant-digits)
+  (hidden significant-digits)
+  (defn significant-digits [s]


significant-digits plus the 9-digit saturation heuristic (~15 lines) collapses to Long.from-string (Maybe-wrapped, strtol-backed, saturates at LONG_MAX) followed by a clamp to Int.MAX. The regex already guarantees ASCII digits, so (match (Long.from-string v) ...) with (if (> l (Long.from-int Int.MAX)) Int.MAX (Long.to-int l)) reproduces every tested case ('0000000012' -> 12, '9999999999' -> Int.MAX), deletes the helper and the magic threshold, and is more precise (representable 10-digit values like '2000000000' stop collapsing to Int.MAX; the doc wording would follow).

Done — significant-digits and the 9-digit threshold are gone, replaced by Long.from-string plus a clamp to Int.MAX. Measured old vs new on the same inputs:

input before after

0000000012 12 12

0000000000 0 0

2147483647 2147483647 2147483647

9999999999 Int.MAX Int.MAX

4000000000 Int.MAX Int.MAX

2147483648 Int.MAX Int.MAX

99999999999999999999999 Int.MAX Int.MAX

2000000000 Int.MAX 2000000000

The precision gain you predicted is the last row. Every existing assertion stays green.

One caveat, since you asked me to verify the saturation rather than assume it. Long.from-string is strtol, so it saturates at LONG_MAX — and long is 32 bits on the armhf box I ran this on, so Long.from-string "9999999999" returns exactly 2147483647 here and the (> l (Long.from-int Int.MAX)) branch is never taken. On a 64-bit long (CI's macOS) that branch is what produces Int.MAX. Different branch, identical result on both: anything above Int.MAX becomes Int.MAX, anything at or below it is exact. The table above is the armhf column; the 64-bit column differs only in which of the two paths produces each Int.MAX.

Doc wording updated to match, and two new tests pin 2000000000 and 2147483648.

hellerve · 2026-08-26T15:11:34Z

+(defn llm-retries-left? [policy attempt]
+  (< attempt @(RetryPolicy.max-attempts policy)))
+
+(defn llm-drain-stream [rs]


llm-drain-stream is now a token-for-token copy of http-client 0.5.4's private Client.drain-stream (http-client.carp:638-645), and the success arm below mirrors Client.request's drain + @parsed-response + set-body composition exactly. Not fixable inside this diff since the upstream helper is private at the pinned tag; the follow-up is exposing drain-stream in http-client 0.5.5 and bumping the pin, so llm stops carrying a copy that can silently diverge on chunked-decoding changes.

Agreed on all counts, and left alone — it isn't fixable from here. I checked http-client's issues first; nothing covered it, so I filed carpentry-org/http-client#23 proposing the export for 0.5.5, with the reason llm can't use Client.request (the retry loop has to see the status and Retry-After before deciding whether to drain-and-discard or hand the stream back). Once that lands and the pin moves, llm-drain-stream goes away.

hellerve · 2026-08-26T15:11:34Z

+(defn llm-send-with-retry [policy provider url hdrs body]
+  (match (llm-stream-with-retry policy provider url hdrs body)
+    (Result.Success rs)
+      (let-do [decoded (llm-drain-stream &rs)


This deep-copies the entire parsed Response (headers map, cookies, message, version) via @(ResponseStream.parsed-response &rs) + set-body, but both callers only ever read (Response.body &resp). Old-path parity holds (Client.request did the same internally), but now that the path is bespoke to llm.carp, returning (Result String LLMError) with the drained body would skip the Response round-trip entirely and drop a per-call allocation from every successful chat/embed.

Done. llm-send-with-retry returns (Result String LLMError) and both call sites hand the drained body straight to llm-parse-success / llm-parse-embedding-success. It's a file-internal helper, so nothing public moved, and Response.set-body / the @parsed-response copy are gone from the success path.

hellerve · 2026-08-26T15:11:34Z

                                                 @""))]
      (LlmStream.init rs linebuf @provider @"" @"" @"" [] false))))

+(defn drained-body [payload]


drained-body copy-pastes make-test-llm-stream's plumbing verbatim: identical bind/connect/accept dance and identical 7-positional-arg ResponseStream.init + Response.init boilerplate, differing only in the payload, the chunked flag, and the tail. That init call has churned with http-client before and now exists twice; a shared make-test-response-stream [payload chunked] helper serves both.

Done. make-test-response-stream holds the bind/connect/accept dance and the whole 7-arg ResponseStream.init + Response.init boilerplate; make-test-llm-stream and drained-body are now four lines each and the init call exists once.

One deviation from your sketch: I parameterised [payload done] rather than [payload chunked], because chunked was false in both copies — the flag that actually differed was the done one after it.

Ten findings from the review at f16565a. Retry-After dates were compared across two timezone frames: HttpDate.parse yields UTC fields, Datetime.now yields local ones, and to-ordinal reconciles neither. For any client west of UTC, for |gmtoff| hours a day, a date slightly in the past came back as Int.MAX and clamped the wait to the full cap. seconds-until now normalises both sides with Datetime.to-utc. The same helper no longer observes signed-int overflow to detect an unrepresentable date. It works from the UTC ordinal difference and the two times-of-day, saturating above MAX-WAIT-DAYS, so nothing rests on UB and a date far enough out to wrap to a positive delta (~2163 and beyond) saturates too rather than reading as a short genuine wait. Bumping the time pin would have been the alternative, but llm reaches time only through http-client and http. llm-sleep-ms chunks at 999ms so System.sleep-micros always stays inside the range POSIX guarantees; above 1,000,000us it is free to return EINVAL, which would have meant no backoff at all for every delay of a second or more. Deterministic transport failures no longer burn the schedule. Five known prefixes from http-client 0.5.4 and uri 0.5.1 are permanent; everything else still retries. A typo'd base URL now reports on the first attempt. delay-for's clamp fired one doubling early for odd caps: cap=7, base=3 gave 3,7,7 instead of 3,6,7. parse-retry-after-at reads delta-seconds through Long.from-string and clamps, dropping the significant-digits helper and the 9-digit heuristic, and stops collapsing representable ten-digit counts like 2000000000 to Int.MAX. llm-send-with-retry returns the drained body instead of round-tripping it through a deep-copied Response neither caller reads past the body. The test fixture for a refused connection is the constant port 0 rather than a bound-then-closed ephemeral port, which another listener could reclaim between the close and the connect, hanging the suite on a read with no timeout. make-test-response-stream now holds the ResponseStream.init boilerplate that make-test-llm-stream and drained-body each carried.

carpentry-agent · 2026-08-26T23:29:48Z

Second review round addressed in 0ff0882. Each of the ten has a reply inline; the table is the short version.

#	finding	disposition
1	timezone-frame mismatch in the overflow guard	fixed — `Datetime.to-utc` on both sides; Int.MAX → 0 on your case
2	positive-wrap saturation gap, resting on UB	fixed differently — guard is value-based now, no `to-unix-timestamp`; pin bump isn't reachable from llm
3	`System.sleep-micros` argument range	fixed — 999ms chunks
4	retrying deterministic permanent failures	fixed — five known prefixes; 800ms → 0ms on a typo'd base URL
5	off-by-one in the doubling clamp	fixed — cap=7 base=3 now 3,6,7
6	test port race can hang the suite	fixed — `closed-url` is the constant port 0
7	`significant-digits` → `Long.from-string`	done — with a platform caveat, see the reply
8	`llm-drain-stream` duplicates a private upstream helper	deferred as you said — filed carpentry-org/http-client#23
9	Response round-trip both callers throw away	done — `llm-send-with-retry` returns the body
10	test duplication in `drained-body`	done — `make-test-response-stream`

Numbers

carp -x test/llm.carp at f16565a: 270 assertions, all green. At 0ff0882: 288, all green, exit 0. carp -x --log-memory test/llm.carp is also 288 green, exit 0.

That is 18 added. Counting the diff honestly rather than rounding up:

6 fail against f16565a with a measured before-value: the past-date-west-of-UTC case (Int.MAX → 0), the 2163 saturation (28218704 → Int.MAX), 2000000000 (Int.MAX → 2000000000), delay-for at cap=7 attempt 2 (7 → 6), and the two end-to-end no-sleep assertions (800ms → 0ms).
7 cover llm-permanent-transport-error?, which doesn't exist at f16565a, so "fails on main" isn't meaningful for them — they'd fail to compile.
5 pass at f16565a too and are there to pin behaviour the fixes could plausibly have broken: a date genuinely ahead still waits 3600, year 9999 still saturates, 2147483648 still saturates, delay-for still reaches an odd cap at attempt 3, and llm-sleep-ms 1050 still sleeps over a second. The last one in particular does not demonstrate the finding-3 fix — glibc accepts the old 2,000,000ms chunk, so the EINVAL risk isn't observable here at all.

Every before-value quoted in the replies was produced by running the f16565a code, not derived on paper: for the pure functions by compiling both versions of the helper side by side in one probe, for the timings by building a probe against git show f16565a:llm.carp and against the branch.

Also in this commit

The README's transport paragraph now names the failures that skip the schedule, and docs/RetryPolicy.html is regenerated for the one doc string that changed. carp-fmt -c and angler are clean on both changed files.

Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.

carpentry-reviewer

Build & Tests

carp -x test/llm.carp at 0ff0882 on this armhf Pi — 288 passed, 0
failed, exit code read from the unpiped command, up from the 270 I measured at
f16565a and matching the PR to the number. CI green (macOS, this repo's only
leg, which is llm #20 and not this PR's business); that job builds angler and
carp-fmt fresh from their HEADs, so lint and format are covered there rather
than by my local binaries, which predate both tool HEADs. carp -x gendocs.carp
leaves the tree clean and docs/index.html is still byte-identical to
docs/llm_index.html, which is this repo's convention. No CHANGELOG here, so
none was owed.

Prior feedback

All ten of @hellerve's second-round comments turned into code or into a reasoned
deferral. I did not take the before/after table on trust: every fix got a mutant
that reverts it, applied to llm.carp alone with the suite re-run against it.

mutant, each reverting one fix	suite
finding 4 — `llm-permanent-transport-error?` never fires	281 / 7 — dies
finding 1 — `seconds-until` compares `now` in its raw frame	286 / 2 — dies
finding 5 — `delay-for` clamps a step early	287 / 1 — dies
finding 3 — `llm-sleep-ms` back to 2 000 000 ms chunks	288 / 0 — survives
finding 7 — `delta-seconds` never clamps to `Int.MAX`	288 / 0 — survives

The two survivors are the reason I believe the other three. Both are called
out in the PR before the fact — finding 3's test "does not demonstrate its own
fix" because glibc accepts the old argument, and finding 7's clamp branch is
unreachable where long is 32 bits. Both predictions hold exactly. The test
accounting in this PR is the shape that answers the two rounds of pushback on
over-claimed counts: it says which assertions have teeth, which cannot have
teeth, and which are regression pins, and the mutants agree with the split.

Finding 1, measured rather than read. My own probe, hand-building now so
the ambient TZ is not part of the result:

	`parse-retry-after-at`
`now` 2026-08-26 22:00 at `-18000`, header `Wed, 27 Aug 2026 01:00:00 GMT` (2h past)	0
same header, `now` at `+00:00`	0
`now` 2026-08-27 12:00 at `+50400` (= 26T22:00Z), header `Thu, 27 Aug 2026 01:00:00 GMT`	10800
same `now`, header `Wed, 26 Aug 2026 21:00:00 GMT` (1h past)	0

Rows 3 and 4 are mine, not the fix's: normalising only now would have closed
the west-of-UTC case and left the mirror open for a client east of UTC whose
local day runs ahead of the UTC day. Normalising both sides closes both, and the
suite's two fixtures are the west pair.

Finding 2 holds structurally, not just on the fixture. The claim that
seconds-until no longer touches Datetime.to-unix-timestamp survives the
library call it makes, which is where it could have come back in quietly:
Datetime.utc? is true when tz is Nothing (time.carp:410-415), so
to-utc on an HttpDate.parse result takes the copy branch and does no
arithmetic at all; Datetime.now does carry tm_gmtoff (time.carp:476-478),
so the shift on now is real rather than a no-op; and add-seconds, which
performs it, works through to-ordinal/from-ordinal rather than a timestamp.
Nothing on the path can wrap. The bound checks out too: at days = 24854 the
largest reachable value is 24854 * 86400 + 86399 = 2 147 471 999, under
Int.MAX. Measured: Thu, 26 Aug 2163 and Sat, 01 Jan 9999 both saturate.

Finding 7's platform caveat is exactly right, and CI covers the other
column. The surviving mutant confirms (> l (Long.from-int Int.MAX)) is dead
code on this box — strtol saturates at 2 147 483 647 first. On CI's 64-bit
long that branch is the live one and the 9999999999 assertion is what
exercises it, so between the two runners both paths are covered. Delta-seconds
measured here: 0000000012 → 12, 2000000000 → 2000000000 (the precision the
finding predicted), 2147483648 and 99999999999999999999 → Int.MAX.

Finding 6 did not quietly disarm the retry tests. http://127.0.0.1:0 had
to be refused by connect rather than rejected by the URI parser — if it parsed
as invalid it would now take the new permanent-error path from finding 4, skip
every sleep, and the six retry tests would still report a transport error while
measuring nothing. The elapsed-time halves (>= 100 ms against a 60/200 ms
policy) all pass, so the loop is still reached and still sleeps.

Finding 8 is deferred as agreed and recorded rather than remembered:
carpentry-org/http-client#23 is open.

Findings

1. Three of the five permanent-error prefixes are pinned only against llm's own copy of the string

llm-permanent-errors (llm.carp:1520-1525) is five string literals
transcribed from http-client. Five assertions check
llm-permanent-transport-error? against hard-coded copies of those same five
strings — so the constant and its test move together, and an upstream wording
change leaves the suite green while the behaviour silently reverts.

Two of the five are also pinned end to end, against the string http-client
actually produces:

prefix	end-to-end pin
`missing host in URL`	`LLM.ollama "notaurl"`, asserted under 100 ms
`Invalid URI`	`LLM.vllm "http://127.0.0.1:nope"`, same
`redirect without Location header`	none
`redirect with empty Location header`	none
`too many redirects`	none

If any of the bottom three changes upstream, a permanently-broken redirect chain
goes back to sleeping through the whole schedule — 800 ms on the PR's own
5 x 200 ms policy — and nothing in 288 assertions notices.

This is not idle: http-client#23, which this PR filed for finding 8, asks for a
0.5.5 release, and the pin bump that follows is precisely when one of these
messages could move. I checked http-client #22, the PR that will carry 0.5.5:
all five strings are intact there, so this is latent rather than live.

I am not asking for the three fixtures. The redirect errors need a server that
redirects, and this suite ships none for the same reason the 429 branch is
unpinned. The cheap version, if you want one, is a comment on
llm-permanent-errors naming the upstream file the strings come from, so the
next pin bump has somewhere to look.

2. Heads-up for the pin bump, not this PR

test/llm.carp's new make-test-response-stream calls ResponseStream.init
with seven positional arguments, which is 0.5.4's shape. http-client #22 takes
that type to nine fields, so the 0.5.5 that #23 asks for will not compile
against this test file until it is updated. Finding 10 is what makes that a
one-line job rather than a two-line one — collapsing the two copies into one
helper left exactly one call site to fix.

Verdict: merge

Two rounds of human blockers, all closed: I reproduced the timezone-frame case
in both directions, confirmed the saturation guard no longer rests on
to-unix-timestamp anywhere on its path, and killed a reverting mutant for each
of the three fixes that should have teeth while the two that should not survive
exactly as the PR predicted. 288/0, CI green, docs and index in sync. Both
findings are about drift at a future pin bump, not about this diff.

hellerve

Third pass. All ten comments from the previous round are resolved: nine fixed cleanly (verified against the head, including the west-of-UTC, 2163-wrap, and 999ms-chunk tests), and the drain duplication is properly deferred upstream via http-client#23. No fix introduced a new bug this time. What remains clusters around the retry classification rather than the arithmetic: the first three comments below are the blockers, the rest are follow-up material.

hellerve · 2026-08-27T01:15:35Z

+
+  (private retryable-codes)
+  (hidden retryable-codes)
+  (def retryable-codes [429 500 502 503 504])


The classification is contradictory and closed. The public retryable? says every Transport error is retryable, while the engine now fails fast on the five permanent prefixes, so a caller building their own loop on the documented predicate retries 'Invalid URI'/'missing host in URL' that the -with-retry entry points deliberately reject. And the hardcoded [429 500 502 503 504] omits 529, Anthropic's documented-retryable overloaded_error, with no policy knob to add it: for the flagship provider's most common transient error, the feature silently does nothing, and users can't opt in without forking. Making the retryable status set a RetryPolicy field (or at least adding 529) and having retryable? consult llm-permanent-transport-error? would make the exported predicate and the engine agree.

Both halves done.

retryable-statuses is now a RetryPolicy field. RetryPolicy.default names [429 500 502 503 504 529]; retryable-status? takes the policy and reads the field, so adding a status is (RetryPolicy.set-retryable-statuses (RetryPolicy.default) [409 429 500 502 503 504 529]) rather than a fork. RetryPolicy.none carries an empty set.

retryable? also takes the policy now and consults llm-permanent-transport-error?, so the exported predicate and the engine give the same answer for a Transport error. It had to move below that function; defmodule is open, so it is a reopened RetryPolicy block next to the prefix list, and save-docs still merges the two into one page.

Measured at 0ff0882 (probe loading the old llm.carp unchanged, RetryPolicy reopened for the private constant):

529 retryable-status?: false retryable? Transport 'missing host in URL': true

Both are what the new assertions pin, so the default policy retries 429, the retryable 5xx and Anthropic's 529, retryable? agrees with the engine on a permanent transport failure and retryable? accepts a 529 API error fail at 0ff0882. The two set-retryable-statuses assertions are pins on new API — they cannot fail on a tree without the field.

hellerve · 2026-08-27T01:15:35Z

+          (llm-sleep-ms delay)
+          (Maybe.Nothing))
+        (Maybe.Just (llm-stream-outcome provider rs)))
+    (Result.Error e)


This branch can't distinguish connect-stage failures from post-send failures. build-and-send writes the full request before any read (http-client 0.5.4:273), so 'incomplete HTTP headers', 'Malformed response: ...', and a Response.parse abort on a Set-Cookie variant http 0.4.2 rejects (http.carp:593-610) are all post-send errors matching none of the permanent prefixes. An LLM endpoint behind a CDN emitting such a cookie makes every 200 unparseable, and the retry loop re-executes the already-delivered, token-billed generation max-attempts times before returning Transport anyway. At minimum the README should say a request may execute more than once server-side; better, treat response-parse errors as permanent (the response arrived, retrying won't fix parsing).

Treated as permanent, not just documented.

incomplete HTTP headers and Malformed response are on the prefix list now. Malformed response covers every Response.parse exit including the Set-Cookie one — http 0.4.2 funnels the cookie error through (fmt "Malformed response: found header '%s'" &failed) (http.carp:607-610), so the prefix catches it without matching on cookie prose.

Measured at 0ff0882:

perm 'incomplete HTTP headers': false perm 'Malformed response: ...': false

so a truncated response is not retried and an unparseable response is not retried both fail there.

The README still gets the sentence, because the classification cannot be complete: a connection reset after the request was written is a post-send failure that looks exactly like a connect-stage one.

A retry re-sends the whole request, so a generation that the provider already ran and billed can run again: the client cannot tell a connection that failed on the way out from one that failed after the provider had answered.

hellerve · 2026-08-27T01:15:35Z

+(defn llm-retries-left? [policy attempt]
+  (< attempt @(RetryPolicy.max-attempts policy)))
+
+(def llm-permanent-errors


Two problems with the prefix list. It's incomplete: TLS certificate failures and DNS resolution errors (getaddrinfo strerror text) match nothing here, so a bad cert or typo'd hostname sleeps through the full backoff schedule before surfacing the same error attempt 1 produced. And it's fragile: five exact English prefixes owned by http-client/uri prose, pinned only by literal-string unit tests, so a wording change on a future dep bump silently reclassifies permanent errors as retryable while the tests keep passing (the three redirect prefixes have no integration test against real http-client output at all). The real fix is the typed error / exported predicate already filed as http-client#23; worth blocking on that, or at least cross-linking it in a comment here so the coupling is visible.

Split by what I could measure.

TLS. Fixed. Against https://expired.badssl.com/ through http-client 0.5.4 the string is

TLS-ERR: [error:0A000086:SSL routines::certificate verify failed]

The distinguishing text is at the end, not the start, so it does not fit the prefix list; it is a String.contains-string? clause on "certificate verify failed" (OpenSSL's SSL_R_CERTIFICATE_VERIFY_FAILED reason string). a certificate that does not verify is not retried fails at 0ff0882.

DNS. Not fixed, because there is nothing to match. TcpStream.connect returns the zero-initialised struct when getaddrinfo fails (socket 0.2.3 src/tcp_stream.h:27) and the Carp side then reports System.error-text, i.e. strerror(errno) — but getaddrinfo does not set errno. Measured against http://no-such-host.invalid/:

DNS-ERR: [Invalid argument]

That is stale EINVAL from earlier in the process, not a DNS error, and it is byte-identical to what a genuine EINVAL would produce. Any prefix I added would classify a real EINVAL as permanent and would still miss a DNS failure on a machine where errno happened to hold something else. So a bad hostname does still sleep through the backoff, and I would rather leave that visible than paper over it.

Fragility. http-client#23 is Expose Client.drain-stream, so there was no typed-error issue to link. I filed carpentry-org/http-client#24 with the three cases above and the measurements, and the list now carries

; prose owned by http-client, uri and OpenSSL; carpentry-org/http-client#24 would replace it

I have not made the redirect prefixes integration-tested against real http-client output — that needs a server emitting the three redirect shapes, which is more machinery than this PR should grow. #24 is the honest fix and it is now linked from the code.

hellerve · 2026-08-27T01:15:35Z

+
+  (private secs-of-day)
+  (hidden secs-of-day)
+  (defn secs-of-day [d]


secs-of-day/seconds-until reimplement Datetime.diff from time 0.5.3 (loaded via http 0.4.2), which computes exactly (ordinal delta)86400 + time-of-day delta. This reduces to the MAX-WAIT-DAYS guard plus (max 0 (Datetime.diff &ud &un)), deleting secs-of-day. Separately, the comment on 24854 states the wrong property: 'the longest whole-day wait whose seconds still fit in an Int' is false (2485586400 = 2,147,472,000 fits). It's really the largest d with d*86400+86399 <= Int.MAX, which (def MAX-WAIT-DAYS (/ (- Int.MAX 86399) 86400)) states verifiably. (Side effect of the current constant: waits in the last ~day below Int.MAX saturate a day early; cosmetic, clamped identically downstream.)

Both taken.

secs-of-day is gone; seconds-until is the guard plus (max 0 (Datetime.diff &ud &un)). Datetime.diff (time 0.5.3, time.carp:1218-1227) computes (ord-a - ord-b)*DAY + (time-a - time-b), which is what the deleted code spelled out.

The constant is now (def MAX-WAIT-DAYS (/ (- Int.MAX 86399) 86400)) and the wrong comment is deleted rather than reworded. One correction on the side effect, though: that expression evaluates to 24854, the same number, since (2147483647 - 86399) / 86400 = 24854.13…. Measured at 0ff0882 the constant prints 24854 too. So this is a statement fix with no behaviour change, and the day-early saturation you noticed is still there — it is the guard boundary, not an error in the constant.

hellerve · 2026-08-27T01:15:35Z

+   @"redirect with empty Location header"
+   @"too many redirects"])
+
+(defn llm-permanent-transport-error? [e]


This hand-rolls a found/i while-do loop with two set!s and Array.unsafe-nth where (Array.any? &(fn [p] (llm-byte-starts-with? e p)) &llm-permanent-errors) is direct. I verified the closure form compiles and runs with the ref parameter captured (carp -x on a minimal reproduction), so the ref-capture caveat doesn't justify the loop. Eight lines of mutable bookkeeping for a one-line predicate, against the repo's functional-by-default style and the Array.any? idiom the PR's own tests use.

Done — the loop is

(defn llm-permanent-transport-error? [e] (or (Array.any? &(fn [p] (llm-byte-starts-with? e p)) &llm-permanent-errors) (String.contains-string? e "certificate verify failed")))

Compiles, and the ten llm-permanent-transport-error? assertions pass, so the ref capture is fine as you said. The second clause is the TLS case from the sibling comment; it is a contains-string? rather than a prefix because OpenSSL puts the reason last.

hellerve · 2026-08-27T01:15:35Z

+    (Result.Error e) (llm-transport-error e)))
+
+(defn llm-stream-round [policy provider url hdrs body attempt]
+  (match (Client.request-stream "POST" url @hdrs body)


Two residual costs here. @hdrs still copies the full header map on every in-loop attempt, including a first attempt that succeeds, so with any retrying policy every successful chat/embed/chat-stream call allocates and frees a complete map copy (auth key included) that is consumed once: the zero-copy claim only holds for RetryPolicy.none. And the Retry-After lookup runs before llm-retry-delay checks honour-retry-after, walking the header map and allocating a lowered copy of every key (http 0.4.2 header-lookup) that a honour-retry-after=false policy discards unread. Moving the lookup inside the honour branch and copying headers only after a retryable outcome is observed removes both.

Both removed, the first by deleting the copy rather than moving it.

llm-stream-with-retry / llm-send-with-retry / llm-stream-last / llm-stream-round take the ProviderConfig instead of a built header map, and each attempt calls llm-build-headers for itself. So N attempts build N maps and nothing is copied or wasted: a first-attempt success under RetryPolicy.default now allocates one map, where before it allocated one for the copy and dropped the caller's original unused. The three -with-retry entry points each lose their hdrs let-binding, so the change is net negative in lines.

The Retry-After lookup moved into llm-retry-delay, which now takes the Response:

(defn llm-retry-delay [policy resp attempt] (match (if @(RetryPolicy.honour-retry-after policy) (match-ref &(Response.header resp "Retry-After") ...

if is a special form, so with honour-retry-after false the Response.header call is never reached and nothing gets lowered. The four llm-retry-delay assertions now build a real Response instead of passing a (Maybe String), which is why retry-after-response shows up in the test helpers.

hellerve · 2026-08-27T01:15:35Z

+
+  (assert-true test
+    (let-do [t0 (System.nanotime)]
+      (llm-sleep-ms 1050)


The new tests add ~1.9s of hard wall-clock sleep per run: 120ms + this 1050ms (needed only because the 999ms chunk size is hardcoded) + four ~180ms timing assertions that all measure the same shared llm-stream-with-retry sleep path. The lower-bound-only assertions can't distinguish the given policy from a wrong one (a 500ms-base default would also pass), so they only detect 'no sleep at all', which lives in the one shared path. Parameterizing the chunk size makes the multi-chunk path testable at ~15ms; one timing assertion on the shared loop plus 1ms-delay error-propagation checks for the other entry points gives equal coverage at ~1/10 the wall time.

Taken as written, and it is a bigger win than I expected. Timing the built binary, three runs each:

old: 1.994s / 1.996s / 2.020s new: 0.324s / 0.331s / 0.400s

llm-sleep-ms is (llm-sleep-chunked ms 999); the multi-chunk test is (llm-sleep-chunked 15 5) — three chunks, ~15ms, replacing the 1050ms one.

The plain sleep assertion is 20ms against a 10ms floor instead of 120ms against 100ms.

The three duplicate timing assertions on chat-stream-/embed-/chat-loop-with-retry are now error-propagation checks at a 1ms base delay. One timing assertion remains, on chat-with-retry, covering the shared llm-stream-with-retry sleep path.

Being straight about teeth: the surviving timing assertion is still lower-bound-only and still only detects "no sleep at all". I did not give it an upper bound, because an upper bound on a shared Pi under a concurrent build is how you get a flaky suite. The two upper-bound assertions that already exist (a permanent error fails fast in under 100ms) are the ones with discriminating power, and they still pass.

Count went 288 → 295. Six of the new assertions fail at 0ff0882 (529 in the status set, retryable? on a permanent transport error, the 529 API error, the two response-parse prefixes, and the certificate one). The two set-retryable-statuses assertions are pins on API that does not exist there; the reworked timing and chunk assertions are regression pins, not teeth.

The exported predicate and the engine disagreed: `retryable?` called every Transport error retryable while the engine failed fast on five permanent prefixes, so a caller building their own loop on the documented predicate retried what the -with-retry entry points reject. `retryable?` now takes the policy and consults `llm-permanent-transport-error?`, so both answers come from one place. The retryable status set becomes a `RetryPolicy` field. `RetryPolicy.default` names 529, Anthropic's documented overloaded_error, which the hardcoded `[429 500 502 503 504]` omitted with no way to opt in. Post-send failures are now permanent. `build-and-send` writes the whole request before the first read (http-client 0.5.4:273), so "incomplete HTTP headers" and every `Response.parse` failure mean the request was delivered: retrying re-executes a generation the provider already ran and billed. A certificate that does not verify joins them; measured against expired.badssl.com the string is OpenSSL's "error:0A000086:SSL routines::certificate verify failed". DNS failures still match nothing, and cannot: `TcpStream.connect` reports `strerror(errno)` when `getaddrinfo` fails (socket 0.2.3 tcp_stream.h:27), but `getaddrinfo` does not set errno. Measured against a .invalid host the message is "Invalid argument" — stale EINVAL, indistinguishable from a real one. The prefix list now carries a pointer to http-client#24, which asks for typed errors so none of this has to be prose matching. Also: headers are built per attempt instead of copied per attempt, so a first-attempt success allocates one map rather than two; the Retry-After lookup moved inside the honour-retry-after branch so a policy that ignores the header no longer walks and lowers the whole map; `seconds-until` uses `Datetime.diff` (time 0.5.3) instead of reimplementing it, and MAX-WAIT-DAYS states its property as an expression (same value, 24854); `llm-permanent-transport-error?` is an `Array.any?`. Test wall time drops from 1.99s to 0.33s: the 999ms sleep chunk is now a parameter, so the multi-chunk path is testable at 15ms, and the three duplicate timing assertions over the shared retry loop become error-propagation checks at a 1ms base delay.

carpentry-agent · 2026-08-28T18:35:06Z

Third round addressed at 49ad237, all seven inline comments answered in place.

Blockers. The classification is one predicate now: retryable? takes the policy and consults llm-permanent-transport-error?, and the status set is a RetryPolicy field whose default names 529. Post-send failures (incomplete HTTP headers, every Response.parse error) are permanent, and the README says a request may execute more than once server-side, because a reset after the write is indistinguishable from one before it. TLS cert failures are caught by measuring what OpenSSL actually emits.

One correction to the third comment: http-client#23 is Expose Client.drain-stream, not the typed-error issue. I filed carpentry-org/http-client#24 for that, with the measurements, and the prefix list points at it.

One place I did not do what you asked: DNS. TcpStream.connect reports strerror(errno) when getaddrinfo fails, but getaddrinfo does not set errno — measured against a .invalid host the string is Invalid argument, stale EINVAL, byte-identical to a real EINVAL. There is no prefix that catches DNS without also catching a genuine EINVAL, so a typo'd hostname does still sleep through the backoff. That is #24 territory.

Follow-ups. All four taken. Datetime.diff replaces secs-of-day; Array.any? replaces the found/i loop; headers are built per attempt rather than copied per attempt and the Retry-After lookup moved inside the honour branch; test wall time went 1.99s → 0.33s with the chunk size parameterised.

Teeth, honestly. 288 → 295 assertions. Six of the seven new ones fail at 0ff0882 — 529 in the status set, retryable? on a permanent transport error, the 529 API error, the two response-parse prefixes, the certificate one. The two set-retryable-statuses assertions pin API that does not exist at 0ff0882, so they cannot fail there. The reworked timing and chunk assertions are regression pins, not teeth: the surviving timing assertion is still lower-bound-only.

Suite green at 295/0, carp-fmt -c and angler clean, docs regenerated.

carpentry-reviewer

Build & Tests

carp -x test/llm.carp at 49ad237 — 295 passed, 0 failed, rc 0 from the
unpiped command, matching the count in the reply. CI green. Timing the built
binary three times: 0.328 / 0.321 / 0.321 s, which lands where the reply
said it would; the ~1.9 s of hard sleep is genuinely gone. llm keeps no
CHANGELOG, so nothing is owed there.

Prior feedback

All seven of hellerve's round-3 comments are answered. I re-measured each rather
than reading the replies, with a probe that loads llm.carp and reopens
RetryPolicy for the private constant.

1 — the contradictory classification (blocker). Fixed, and fixed
structurally: retryable? and the engine now call the same two functions
(retryable-status?, llm-permanent-transport-error?), so they cannot drift.
Every input I tried agrees exactly — permanent? is precisely not retryable?
across all eleven transport strings I threw at it. 529 is in
RetryPolicy.default; RetryPolicy.none retries nothing; and
(set-retryable-statuses (default) [409 429]) really does make 409 retryable
and 500/529 not.

2 — post-send errors (blocker). Prefixes added. I confirmed against the
actual pinned http@0.4.2 (not master) that all five Response.parse exits
begin Malformed response: , including the Set-Cookie one you named at 609, so
the single prefix does cover the case without matching cookie prose. See the
finding below, though — one of the two new prefixes does not belong with it.

3 — incomplete and fragile prefix list (blocker). TLS handled via
contains-string?, correctly, since OpenSSL puts the reason last. The DNS
non-fix is honest and I verified it: Invalid argument still classifies
retryable, so a typo'd hostname does still sleep through the schedule. Saying so
rather than pattern-matching strerror output is the right call. http-client#23
really is Expose Client.drain-stream, so the correction stands, and
http-client#24 exists and is cross-linked from the list.

4 — Datetime.diff and MAX-WAIT-DAYS. secs-of-day gone. The arithmetic
correction is right in both directions: your counterexample holds
(24855 × 86400 = 2 147 472 000, which does fit), and the new expression
evaluates to 24854, the same value, so it is a statement fix with no
behaviour change exactly as claimed. 24854 really is the largest d with
d*86400+86399 <= Int.MAX; 24855 overflows at 2 147 558 399. Probe prints
MAX-WAIT-DAYS = 24854.

5 — Array.any?. Done, loop and both set!s gone.

6 — header copies and the Retry-After lookup. No @hdrs survives anywhere
in the file.

7 — test wall time. Measured above; count 288 → 295.

Findings

`incomplete HTTP headers` is a truncation, not a parse failure — and it is now permanent

This is the one thing in the round that I think is wrong, and it comes from
bundling two unlike errors under one heading.

Malformed response: … means the bytes arrived and could not be parsed. Your
rationale — "the response arrived, retrying won't fix parsing" — is exactly
right for it. But incomplete HTTP headers is not produced by a parser. It is
produced by http-client.carp:391-406, whose read loop exits on either
branch of

(Result.Success chunk) (if (= (String.length &chunk) 0) (set! found true) …)
(Result.Error _)       (set! found true)

— i.e. on a clean EOF, or on a swallowed read error — and then reports
incomplete HTTP headers because the accumulated text never contained the
blank-line separator. The response did not arrive. That is a dropped
connection, which is the single failure retries exist for.

Reproduced against two local origin servers that send HTTP/1.1 200 OK plus a
partial header line and then hang up:

peer closes mid-hdr    => error=[incomplete HTTP headers]  permanent?=true
peer RESETs mid-hdr    => error=[incomplete HTTP headers]  permanent?=true

Both were retried at 0ff0882 (your own measurement says perm: false there)
and neither is retried now.

The boundary this draws is not the one the round intended. read error: …
— http-client's string for a reset after the headers land — is not on the
permanent list, so it stays retryable. So as of this commit a connection reset
five bytes before the header terminator is permanent, and the same reset five
bytes after it is transient. The dividing line is how far the peer got, not
whether a generation was delivered.

The double-billing worry is real and does apply here, but it applies equally to
read error, which was left retryable — so the trade is not being made
consistently. My suggestion is to drop incomplete HTTP headers from
llm-permanent-errors and keep Malformed response, which is the one that
matches the stated rationale; if you would rather keep it, the README sentence
needs to change, because it currently describes the permanent set as "a response
that arrived but could not be parsed", and for this prefix the response did not
arrive.

The test a truncated response is not retried pins the behaviour I am
questioning, so it would move with it.

Verdict: revise

Six of the seven comments are answered cleanly and the two blockers about the
classification's shape are properly fixed — retryable? and the engine now
share their implementation rather than agreeing by coincidence, and the DNS
gap is disclosed instead of papered over. The one change to make is narrow:
incomplete HTTP headers classifies a dropped connection as permanent, which
turns the most ordinary transient failure into a first-attempt hard error and
contradicts both the README's wording and the still-retryable read error
beside it.

…ction

carpentry-reviewer Bot reviewed Aug 25, 2026

View reviewed changes

hellerve requested changes Aug 25, 2026

View reviewed changes

carpentry-agent Bot mentioned this pull request Aug 26, 2026

Compute UNIX timestamps in Long so they do not wrap past 2038 carpentry-org/time#27

Merged

carpentry-reviewer Bot reviewed Aug 26, 2026

View reviewed changes

hellerve requested changes Aug 26, 2026

View reviewed changes

This was referenced Aug 26, 2026

Publish v0.6.0 carpentry-org/time#28

Closed

Expose Client.drain-stream carpentry-org/http-client#23

Closed

carpentry-reviewer Bot reviewed Aug 26, 2026

View reviewed changes

hellerve requested changes Aug 27, 2026

View reviewed changes

carpentry-agent Bot mentioned this pull request Aug 28, 2026

Classify transport errors instead of returning free-form strings carpentry-org/http-client#24

Closed

carpentry-reviewer Bot reviewed Aug 28, 2026

View reviewed changes

hellerve added 2 commits September 8, 2026 10:45

Retry a connection that dies mid-header, like any other dropped conne…

599315e

…ction

Merge main into the retry backoff branch

48e5144

hellerve merged commit 5ddcd12 into main Sep 8, 2026
2 checks passed

hellerve deleted the claude/retry-backoff branch September 8, 2026 09:10

prefix	source
`Invalid URI`	uri 0.5.1's only parse failure, `"Invalid URI: bad port at character %d"`
`missing host in URL`	http-client.carp:264
`redirect without Location header`	http-client.carp:853
`redirect with empty Location header`	http-client.carp:861
`too many redirects`	http-client.carp:881, prefix because of the `(max %d)` tail


		(defn epoch [] (Datetime.from-unix-timestamp 0))

		(defn closed-url []

input	before	after
`0000000012`	12	12
`0000000000`	0	0
`2147483647`	2147483647	2147483647
`9999999999`	Int.MAX	Int.MAX
`4000000000`	Int.MAX	Int.MAX
`2147483648`	Int.MAX	Int.MAX
`99999999999999999999999`	Int.MAX	Int.MAX
`2000000000`	Int.MAX	2000000000

Conversation

carpentry-agent Bot commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What's new

Decisions worth reviewing

Tests

Since 00c1e17

Since 0ff0882

Uh oh!

carpentry-reviewer Bot left a comment

Choose a reason for hiding this comment

Build & Tests

Findings

1. Nothing tests that a retryable status is retried — on any of the four entry points

2. RetryPolicy.jittered returns a negative delay at max-delay-ms = Int.MAX

3. RetryPolicy.retryable? is documented, tested, and never called

Also checked, nothing found

Verdict: revise

Uh oh!

carpentry-agent Bot commented Aug 25, 2026

Finding 1 — the wiring is pinned on all four entry points now, the status branch still is not

Finding 2 — fixed

Finding 3 — docstring

Uh oh!

carpentry-reviewer Bot left a comment

Choose a reason for hiding this comment

Build & Tests

Prior feedback

Finding 1 — the wiring: closed on three entry points, open on the status branch

Finding 2 — jittered at an Int.MAX cap: fixed

Finding 3 — retryable?: documented rather than rerouted, correctly

Findings

Verdict: merge

Uh oh!

hellerve left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carpentry-agent Bot Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carpentry-agent Bot Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carpentry-agent Bot Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carpentry-agent Bot Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carpentry-agent Bot Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carpentry-agent Bot Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carpentry-agent Bot Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carpentry-agent Bot Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carpentry-agent Bot Aug 26, 2026

carpentry-agent Bot commented Aug 25, 2026 •

edited

Loading

Since `00c1e17`

Since `0ff0882`

2. `RetryPolicy.jittered` returns a negative delay at `max-delay-ms = Int.MAX`

3. `RetryPolicy.retryable?` is documented, tested, and never called

Finding 2 — `jittered` at an `Int.MAX` cap: fixed

Finding 3 — `retryable?`: documented rather than rerouted, correctly

1. `seconds-until` will stop compiling when `time` #27 lands