Repository navigation
feat(coding): bound long-stream CPU and harden interrupted-stream recovery - #41
Merged
Merged
Conversation
Streaming thinking appended to one growing MessageDelta slice, so every delta copied the whole history: a 20k-delta reasoning response allocated 45.6 GB, kept a core and a half busy, copied the draft again on every snapshot, and grew worse the longer the thought ran. State.Draft becomes a StreamDraft value over immutable chunks. An append copies only a bounded tail, a batch fills canonical blocks, the summary (count, per-kind presence, UTF-8 byte totals) travels with the value, and State.Clone shares the draft instead of copying it. Usage stays caller-owned on ingress and on materialized egress, and the JSON field keeps the legacy record array with its null-versus-[] distinction. Measured on the fixed replay (20k reasoning deltas, 4-byte bodies, ~10 s, same PTY and frame cadence): CPU 17.2 s -> 3.5 s in the repeat shape and 19.9 s -> 2.9 s in the prose shape; allocations 45.6 GB -> 0.3 GB; GC 2,744 -> 60; peak RSS 214 MB -> 85 MB on the heaviest shape. The AC5 matrix keeps a 14% to 90% CPU reduction across fixed byte totals, large signatures, empty increments, mixed text/reasoning streams, tool-call records and history sizes, with every visible text and reasoning byte still compared against the independent source prefix frame by frame. A real 32-minute long-thinking task (466 KB reasoning, 654 KB signature) ran at 8.9% mean CPU with a 23.1% one-second peak. The copied partial tail reserves only the room the current call can fill, min(block, len+incoming), instead of a whole block: the next append copies that tail again, so the spare capacity was pure churn. That halves the append-cycle bytes with the allocation count unchanged. The block constant stays 32 after a B=16/32/64 sweep on complete cycles with a fixed batch width: the append path prefers a smaller block, the chunk directory prefers a larger one, and the whole frame cannot tell them apart within noise. Acceptance still open, recorded in the task under .trellis/tasks/10-09-long-thinking-cpu: a terminal-visible stage after rasterization, approval responsiveness under a long stream, and the renderer's repeat-shape byte inflation.
…dleware budget A stream that broke after streaming only reasoning was treated exactly like one that had already produced an answer: the loop retracted the thinking, re-issued the request on its own three-attempt budget, and made the model think the whole thing through again. The comparable harnesses treat that shape as the safest to replay — nothing a consumer has to retract — and give it the full pre-output budget instead. The loop now classifies what an attempt streamed before it failed. Answer text, tool-call events, and citations are retractable, because a re-issue would make a consumer retract what it already read; reasoning deltas are droppable, because dropping thinking costs the consumer no answer. `agent.WithStreamReplayAttempts` is the allowance for the droppable case, and Coding passes the middleware's own `model.RetryBudget`, so a reasoning-only interruption earns what a request that produced nothing would have earned. The two allowances are counted apart, and an agent that sets no replay allowance keeps one tier, which is what every existing frontend already gets. Unknown and future stream event types count as retractable, so a new event kind inherits the conservative path. Giving up with nothing retained now retracts the draft instead of leaving the thinking on screen beside the failure, which the RunInterrupted contract already required of a run with visible output.
…e retry window The streaming idle bound was a hardcoded ten minutes. The adapters already exposed `WithStreamIdleTimeout`, but nothing in Coding called it, so a provider that legitimately pauses longer than that — a long reasoning block the endpoint does not stream incrementally — was aborted as `ai.ErrStreamIdle` and replayed from scratch on every turn, with no way to raise the bound short of editing the source. The retry window was hardcoded to twenty minutes with a doc comment telling the reader to raise it alongside the bound, which configuration could not express either. `stream_idle_timeout_seconds` is now accepted per provider and per model, the model's value overriding the provider's, and an absent key at both layers selecting the transport default. The bound reaches every protocol the factory constructs, and `model.RetryWindowFor` derives the episode from it as `max(RetryWindowFloor, 2×idle)`, so both owners — the middleware's `WithMaxElapsed` and the loop's `WithStreamRecoveryWindow` — scale together, and an unset bound keeps the twenty minutes that ship today. The guard cannot be switched off: an explicit zero is refused at load rather than read as "not declared", because a parked stream is the one interruption nothing else can reach. The setting follows the model it is declared on, including into a subagent that runs a different model. `ai.DefaultStreamIdleTimeout` becomes the canonical default so the transport and the derivation share one number; `httpx` aliases it, and the internal package stays unreachable from Coding. `config show` reports the bound and the window it implies, which is otherwise a policy value a user cannot see. The three protocol branches now share one `adapterOptions` helper, so a new endpoint or transport knob cannot be added to two protocols and forgotten in the third.
The two move together: golangci-lint supports only Go versions at or below the one it was built with (golangci-lint#6643), and its release binaries are built with whichever Go was GA when they shipped. - v2.12.2 (built with go1.26.4) cannot parse the Go 1.27 standard library at all. - v2.13.0-v2.13.2 support Go 1.27 but depend on golang.org/x/tools v0.49.0, whose export-data reader stops at version 4. Go 1.27.2 introduced version 5 (go.dev/issue/81188), so they fail with "export data version 5 is greater than maximum supported version 4". - v2.14.0 depends on x/tools v0.50.0, which reads version 5. The CI action and the local tool are both pinned to v2.14.0 now. The `go` directive moves rather than a `toolchain` line being added, so the module adopts Go 1.27's language version. That also moves two GODEBUG defaults, which is the intended consequence: `tracebacklabels` (goroutine labels in tracebacks) and `x509sslcertoverrideplatform` — on Windows and Darwin, a set SSL_CERT_FILE or SSL_CERT_DIR now selects the roots loaded from disk. The latter is the one with reach: a machine whose SSL_CERT_FILE names an incomplete bundle would newly fail certificate verification, which pips reports as a non-retryable error. `GODEBUG=x509sslcertoverrideplatform=0` restores the previous behaviour. Nothing else moves. `go mod tidy -diff` is empty, so the module graph and go.sum are unchanged, and build, vet, the full test suite, `make deps-check`, `make provider-smoke`, `go mod verify` and `go tool govulncheck` all pass.
The gofumpt in golangci-lint 2.14.0 formats two constructs the previous one left alone: a multi-line call whose last argument ends in a closing brace now puts that brace and the call's parentheses on their own lines, and adjacent parameters of the same type are grouped (`left, right T`). Seventeen files carry one or both; two of them were also plain `gofmt` violations that the older linter did not report. Mechanical only. Build, vet and the full test suite pass unchanged, and `golangci-lint run ./...` drops the 19 gofumpt, gofmt and whitespace findings this removes without adding any. Archived Trellis task research under `.trellis/tasks/archive/` is left as it is: those files are historical records.
rsbin1178
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this fixes
Streaming thinking appended to one growing
MessageDeltaslice, so every delta copied thewhole history and every snapshot copied it again. On the fixed replay (20k reasoning deltas,
4-byte bodies, ~10 s, real PTY at 120×40) the old code spent 17.2 s of CPU in a 10.6 s
window (162% of one core), allocated 45.6 GB, ran 2,744 GC cycles and peaked at
214 MB RSS; the longer the thought ran, the worse each delta got.
The change
State.Draftbecomes aStreamDraftvalue over immutable chunks:min(block, len+incoming)records), never ahistorical prefix; a batch copies the partial tail once and then fills canonical blocks,
so chunk boundaries never depend on the caller's batch split;
the activity query reads it without scanning records;
State.Cloneshares the draft value and keeps copying the other mutable projections;Usagestays caller-owned on ingress and on materialized egress; the JSON field keeps thelegacy record array with its null-versus-
[]distinction.The copied partial tail reserves only the room the current call can fill instead of a whole
block (the next append copies that tail again, so the spare capacity was churn): that halves
the append-cycle bytes with the allocation count unchanged.
The block constant stays 32. A B=16/32/64 sweep on complete 64-append cycles with a fixed
8-record batch width showed the append path prefers a smaller block (−48.4% append bytes at
B=16), the chunk directory prefers a larger one (+100% directory bytes at B=16), and the
whole frame cannot tell the three apart (≤0.6%, inside noise).
Measured
Fixed replay, three serial repetitions per shape, medians:
The 50k-delta case is the one where the old build cannot hold the paced window: it finishes
in 25.57 s instead of 20.03 s while allocating 281 GB.
One real 32-minute long-thinking task on the built binary (466 KB reasoning, 654 KB
signature, 50 tool calls) ran at 8.9% mean CPU, a 23.1% one-second peak and 93.9 MB peak
RSS; its last PTY output landed 44 ms after the committed turn.
Verification
go test -count=1 ./internal/coding/...andgo test -race -p 2 -count=1 ./internal/coding/...pass.go build ./...,go vet ./internal/coding/..., changed-linesgolangci-lint(0 issues) pass.Full-repository lint is unchanged: 794 findings outside this change's lines.
deltas+2, and a frame-verifying passcompares every composed frame's text and reasoning projection against the independent
source prefix — including mixed streams, tool-call records, signatures that must not count
as visible text, and empty metadata increments.
cancel path (handled in 2.3–4.4 ms, input probes still answered 22/22 afterwards).
Still open
Recorded in the task, not claimed here: a terminal-visible stage after rasterization,
approval responsiveness under a long stream (needs a real tool call), and the renderer's
repeat-shape byte inflation (+27% PTY bytes with total write time of 12–18 ms per run).
Evidence, method and limitations live in the task workspace (the
pips-trellisrepo):the implementation review, the exact-tail capacity round, the live real-model capture and the
AC1/AC5/AC4 matrices, under
tasks/archive/2026-10/10-09-long-thinking-cpu/research/.The task is archived as
10-09-long-thinking-cpu(commit375377e, this PR).Also in this branch
had produced an answer: the loop retracted the thinking and re-issued the turn on its three-attempt
re-issue budget. Reasoning is now classified as output a consumer can drop, so that failure draws the
middleware's ten replays — what a request that produced nothing would have earned — while answer text
and tool calls keep the shorter budget. A give-up with nothing retained retracts the draft instead of
leaving the partial thinking on screen beside the error.
stream_idle_timeout_secondsis accepted per provider and per model, reachesevery protocol the factory constructs, and the retry episode is derived from it as
max(10m, 2×idle)for both owners: the middleware'sWithMaxElapsedand the loop'sWithStreamRecoveryWindow. An unset bound keeps the twenty minutes that ship today.config showreports the bound and the window it implies.
go.modmoves to Go 1.27.2 and the lint pin to golangci-lint 2.14.0; the two movetogether because golangci-lint supports only Go versions at or below the one it was built with, and
v2.13.x cannot read Go 1.27.2's export data (version 5). The released binaries therefore adopt Go
1.27's
x509sslcertoverrideplatformdefault, whichdocs/releases/v0.1.8.mdcalls out.docs/releases/v0.1.8.md, the body the release workflow publishes forv0.1.8.