fix(context): overflow classifier gets non-overflow guards + two behavioral detectors (#414) - #437
Merged
Merged
Conversation
…vioral detectors (#414) The #193 classifier was six case-sensitive substrings and a two-code list. Three gaps, all reported against real providers: 1. Nothing was checked BEFORE the overflow patterns. Bedrock formats throttling as "ThrottlingException: Too many tokens, please wait before trying again." — wording that collides head-on with the kind of generic "too many tokens" fallback a broad table needs. Classifying that as overflow discards real conversation to fix a problem compaction cannot fix, and shadows the Retry-After ladder that would have worked. A guard list now wins outright, scoped to throttling/quota: the family whose remedy is "wait", not "send less". Broader guards (a bare "overloaded", a generic "server_error") were deliberately rejected — a guard that swallows a REAL overflow wedges the session, which is worse. 2. Some providers accept an over-window input and never send an error. z.ai returns HTTP 200 with an empty completion; the only evidence is usage.input at or over the window. Classified from the usage now, and recovered through the existing pin-trim-retry path. 3. Others truncate the input to fit and report finish_reason=length with zero output — the model answered a conversation we did not send. Named distinctly (a notice, a trace note, telemetry, and last_api_error when recovery is unavailable) instead of reaching the user as an inexplicably short answer. Conservative by construction: output_tokens must be exactly 0, no content, and the input must fill >= 99% of the window, so an ordinary max-tokens completion cannot trip it. The pattern table also grew to the phrasings twelve more providers actually send, and matches case-insensitively — a Title-Cased rejection used to miss and wedge the session. The guards are what make that breadth safe. Classification moved to agent_overflow.zig; agent_request_policy.zig was at the 600-line cap and re-exports it, keeping its two original test names so the behavioral eval harness does not read the move as deleted coverage. Tests: table-driven fixtures for every guard, every added provider phrasing, and regression rows for all six original patterns proving no reclassification (37 rows), plus 18 completion fixtures covering both detectors and the shapes that must NOT trip them. scripts/test-pty-overflow.py gains three real-PTY scenarios (Bedrock throttle stays on the retry path; the silent 200 compacts and retries; truncate-then-length is reported). Suite 994 -> 1000; golden eval harness byte-identical to a pristine build of the base commit. Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
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
2 participants
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.
Implements #414. Classification moved to a new
agent_overflow.zig(policy was at 589/600), with the order guards → codes → needles, all case-insensitive (a Title-Cased rejection used to slip past and wedge the session).The three additions
ThrottlingException: Too many tokensand friends (rate limits, quota, TPM/RPM) classify as throttle and ride the existing retry/Retry-After path instead of triggering a pointless compaction. Deliberately scoped:overloaded/server_errorwere considered and rejected — a guard that swallows a real overflow wedges the session (the codex/gpt-5.x: a single-turn tool-output burst overflows the context window before auto-compaction fires (no pre-send local-estimate gate) #193 symptom), strictly worse than one wasted compaction. Documented in-source.finish_reason: length, zero output, input ≥99% of window → named distinctly instead of shipping as an inexplicably short answer. A normal max-tokens completion is blocked by three independent conditions; over-window-but-answered means the window figure is stale, and trimming real history over a bad constant is the costlier mistake.Also expanded the needle table with 13 provider-real phrasings (Bedrock, Gemini, Copilot, xAI, Groq, llama.cpp, Kimi, Mistral, Ollama + two generic fallbacks — which are exactly what makes the Bedrock guard load-bearing).
Experiments
scripts/test-pty-overflow.pyextended with 3 scenarios (throttle must-not-pin; silent-200 recovers; truncate-then-length named) — 5/5 pass. Break-and-revert: unwiring the detector reproduces the exact Overflow classifier: non-overflow guard patterns + behavioral detectors #414 symptom ([compaction failed: empty summary]wedge).Gates
Tests 994 → 1000 (+6 exactly); all 9 golden eval files PASS; fmt + line-guard clean (
agent_overflow.zig512, policy 543↓, main.zig held at 600 via one comment rewrap); changelog underv0.0.241 (unreleased).Pre-existing quirks confirmed on pristine base builds, not this change: the tier1
testscheck can't read a count off a cached build, and the PTY harness has an intermittent/help-echo race. Both being filed separately.Part of the prime-agent adoption batch.