Repository navigation
feat(p10): upstream retry, token cost metering, and diagnostics port - #5
Merged
Merged
Conversation
…nostics port
llm: doWithRetry on all four send paths — retry only 429/500/502/503/504,
honour Retry-After (seconds or HTTP-date) capped by MaxDelay, backoff
selects on ctx.Done so timeout/cancel/shutdown take effect mid-wait, and
the request is rebuilt per attempt because http.Request.Body is not
re-readable. Streaming retries only before the first delta: a mid-stream
break is terminal, otherwise already-emitted text would be replayed.
OnRetry callback injects metrics from the composition root (the llm
package's allowed import set is empty in the AST gate).
metrics/store/worker: agent_llm_{retries,prompt_tokens,completion_tokens}
counters plus per-task prompt_tokens/completion_tokens columns written via
a separate TaskStore.RecordUsage — metering stays out of the terminal-state
transition, failed calls still bill, and a metering error never flips a
successful task to failed. Old databases gain the columns through an
idempotent migration.
config/worker: session_history_limit (default 100) replaces the hardcoded
window; pprof_addr gates a diagnostics listener on its own port and mux,
off by default, warning when bound to a non-loopback address.
pool.go and pool_test.go hit the 400/800 line budgets, so cancel handling
and the store double moved into sibling files instead of raising the cap.
…hors ADR-0021 carries the rejected-alternatives table (no retryablehttp, no infinite retry, no usage-in-trace-only, no token fields inside Complete, no pprof on the main mux, no retry after a stream break) so the next session does not re-litigate them. README: 14 config keys, agent_llm_* metrics, per-task token fields in the GET /tasks contract, and the load-probe evidence. ARCHITECTURE: P1-P10 scope, production capabilities, diagnostics in the observability subsection, ADR-0021 in the index. .agent-context: refreshed baseline (360 func Test / 545 RUN / 99 files / 20,851 lines), new retry and metering red lines. WORKMEMORY: PROJECT_OVERVIEW P10 entry, work.log event, notes file, INDEX row.
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
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.
Closes the production gaps that P9 left open, each traced to a physical code fact rather than prose.
What was actually broken
llm.Clientreturned*APIErrorfor any non-200. DeepSeek answers429/503withRetry-Afterat quota boundaries and during rolling deploys; the service wrotetask.error = "upstream error"— reporting "wait a second" as "service broken" and discarding an agent loop that had already burned 3-4 steps.llm.Usagewas consumed only by the eval harness. Runtime exposed no token metric and persisted nothing, so "how much did today cost" had no answer.worker/session.gopinned the history fetch at 100;grep -rn pprofhad zero hits, so goroutine/memory problems required a restart to reproduce.Changes
llm/retry.go—doWithRetryon all four send paths (Chat,Embed,ChatWithTools,ChatWithToolsStream):429/500/502/503/504;400/401/403/404/422never retried (auth and body errors repeat identically and multiply the bill)Retry-Afterparsed as delta-seconds or HTTP-date, capped byMaxDelay— upstream may ask for 60s, but holding a worker that long costs more than abandoning the retryselects onctx.Done(), so task timeout, user cancellation and shutdown take effect mid-waithttp.Request.Bodyis not re-readableToken cost, two channels —
agent_llm_retries_total,agent_llm_prompt_tokens_total,agent_llm_completion_tokens_totalcounters, plusprompt_tokens/completion_tokensonstore.Taskwritten through a separateTaskStore.RecordUsage. Metering stays out of the terminal-state transition, runs on the failure path too (failed calls still bill), and a metering error never flips a successful task to failed. Old databases gain the columns via an idempotent migration.Config —
session_history_limit(default 100) replaces the hardcoded window;pprof_addrgates a diagnostics listener on its own port and mux, off by default, warning when bound to a non-loopback address (the pprof index enumerates every profile, so the main mux's public/demo route is the wrong place for it).Architecture gates —
pool.gohit 401 lines andpool_test.gohit 810, so cancel handling and the store double moved into sibling files rather than raising the 400/800 budgets.Verification
gofmt -l .go build ./...go vet ./...go test -race ./...TestArchitectureTestSubmitLatencyPercentilesDeliberate non-goals
No
go-retryablehttp(60 lines of logic for a dependency that does not know about the stream-replay invariant), no unbounded retry, no usage-in-trace-only (the buffer evicts), no token fields insideComplete, no retry after a stream break, and no end-to-end p99 requiring a live API key. All recorded in ADR-0021's rejected-alternatives table.