modelerrors: make overflow errors more specific#2818
Conversation
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The overflow classification logic is well-structured and correct. The two-tier classifier (structured signals → prose fallback), the OverflowKindOf unwrapping semantics, FormatError switch, and classifyErrorCode routing all look sound. Backward compatibility is preserved: IsContextOverflowError still matches the same error set, and unclassifiable wraps default to OverflowKindTokens. Test coverage is thorough across 26 classifier cases, wrap semantics, and all three FormatError paths.
Split the single ContextOverflowError bucket into three OverflowKinds so
the runtime and clients can react differently to each:
* tokens — conversation exceeds the model's context window.
Compaction can usually help.
* wire — request body exceeds the provider's wire-level limit
(e.g. HTTP 413). The latest turn alone is over the cap;
compaction-as-retry cannot help.
* media — image, PDF, or similar attachment too large.
Classification runs two tiers, in order:
1. Structured signals (high confidence): HTTP 413, body.error.type =
"request_too_large", body.error.code = "context_length_exceeded".
2. Provider-prose substring patterns as fallback, covering the
observed wording across Anthropic, OpenAI, Bedrock, Gemini,
Mistral, Groq, Vertex, OpenRouter, Ollama, Kimi, MiniMax, z.ai,
and others. Best-effort; provider wording is not contractual.
FormatError now returns three distinct, actionable, provider-agnostic
messages instead of one. The runtime emits new ErrorCodes
"request_too_large" and "media_too_large" alongside the existing
"context_exceeded" so external consumers can render the right hint.
Behaviour is preserved: IsContextOverflowError matches the same set of
errors as before, retry classification is unchanged, and auto-compaction
still fires on overflow. NewContextOverflowError now auto-fills Kind
from the underlying error so existing wrap sites get the correct shape
for free.
5448e5f to
a6c7044
Compare
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The classifier's two-tier approach (structured signals first, substring patterns as fallback) is sound. Tier 1 structured checks correctly map HTTP 413 → OverflowKindWire and context_length_exceeded → OverflowKindTokens. The OverflowKindOf helper correctly handles legacy zero-Kind wraps by falling back to classification on the underlying error. NewContextOverflowError auto-fills Kind correctly via classifyOverflow. The classifyErrorCode switch in loop_steps.go correctly routes all three kinds to distinct ErrorCode constants.
Intentional design noted: The PR description explicitly states "Auto-compaction still fires on overflow (this PR labels; it does not yet change control flow)" — the handleStreamError check is intentionally unchanged; control-flow differentiation for Wire/Media is deferred to a follow-up.
No bugs found in the changed code.
Motivation
Context-overflow errors from LLM providers come in three distinct shapes, but
modelerrorscollapses them into one. The conflation has user-visible consequences:/compactcannot help, because the offending turn still has to be sent on every subsequent call. Sessions get stuck failing in the same way until the user starts over.Without distinguishing them at the classifier, every downstream consumer (runtime retry logic, error-code routing, UI hints) has to guess.
Change
Introduce
OverflowKindwith three values, classified by a small two-tier function:tokenswiremediaClassifier:
body.error.type == "request_too_large",body.error.code == "context_length_exceeded".FormatErrornow returns three distinct, provider-agnostic messages (no vendor names — the wire cap is a deployment detail of the provider, not something the user can act on by name). Two newErrorCodeconstants (request_too_large,media_too_large) reach external consumers via the existingErrorEvent.Codechannel so they can render the right hint per kind.What is preserved
IsContextOverflowErrormatches the same set of errors as before.ClassifyModelErroris unchanged (overflow stays non-retryable).NewContextOverflowErrornow auto-fillsKindfrom the underlying error, so existing wrap sites inpkg/runtime/fallback.goget the correct kind without code changes.