Repository navigation
Context-overflow detection needs overflow wording - #30
Open
solvyxtech wants to merge 1 commit into
Open
solvyxtech wants to merge 1 commit into
solvyxtech wants to merge 1 commit into
Conversation
solvyxtech
commented
Oct 7, 2026
solvyxtech
left a comment
Owner
Author
There was a problem hiding this comment.
Grok review: approve (posted as a comment because GitHub won't let this account approve its own PR; please treat it as an approval of 5de04bc).
Pairing rules are fine. This is claude/overflow-classify into claude/sync-local-work, not main. It adds no subscription backends, puts no competitor names in product copy (Grok and the OpenRouter provider are only mentioned in code comments and tests), and doesn't touch fine-tune data. I reviewed only the commit on top of #27 (5de04bc, src/engine.ts plus test/turn-loss.test.ts); the other six commits in the diff come from #24 to #27 and were reviewed there.
What I checked
tsc --noEmitis clean at5de04bc, andtest/turn-loss.test.tspasses (35/35).contextOverflowis still only consulted on a 400 (engine.ts:6236), so the looser alternatives liketoo many tokensandrequest too largecan't fire on a 429 tokens-per-minute rate-limit body.- The new
body: body.slice(0, 600)in the journal note goes throughredactDataand the hidden-checkmaskDeepfrom #27, so it can't leak a key or a withheld command. - I tried a few common overflow wordings against the new regex: OpenAI-style "maximum context length is N", llama.cpp "exceeds the available context size", Anthropic-style "prompt is too long", and "exceed context limit" all still match. Nothing that used to match a real overflow stops matching.
Should fix (not blocking this commit)
- The PR conflicts with its base (
mergeable_state: dirty, insrc/acp.tsandsrc/opencode.ts). That conflict is inherited from #27 against4d94f37(OpenCode Zen-only), the same one flagged on #29. Retargeting this PR's base toclaude/hide-checkswould make the diff show just this commit, and the rebase there fixes both. - No CI runs on this PR, because checks only trigger for PRs into main. The "npm run check green" in the description is local only, so it's worth confirming once the stack lands.
Nits
context (length|window|size)still matches with no number anywhere in the body, so a 400 that just names "context window" in some other sense would still trigger a shed. A cheap backstop for this whole class of bug: when the match finds no window and no count (over.window === 0 && over.sent === 0) and Maat's own estimate is far below any window it knows of, treat it as an ordinary 400 instead of shedding. The 2,476-token run in the description would have survived that even with the old regex.- Optional extra coverage, not a regression since the old regex missed these too: wordings without the word "context", like "input token count (N) exceeds the maximum number of tokens allowed (M)" or TGI's "
inputstokens +max_new_tokensmust be <= N". - The code comment says "a pinned OpenRouter provider" while the description says a GMI-pinned Qwen3-235B lane. Both are probably right, but one consistent phrase would help whoever greps for it later.
Looks good to land with the stack.
Owner
Author
solvyxtech
force-pushed
the
claude/overflow-classify
branch
from
October 7, 2026 21:58
5de04bc to
da5cc3f
Compare
…ry ended a 2.5k-token run as too large
solvyxtech
force-pushed
the
claude/overflow-classify
branch
from
October 7, 2026 22:24
da5cc3f to
c773f36
Compare
solvyxtech
changed the base branch from
claude/sync-local-work
to
claude/hide-checks
October 7, 2026 23:28
Owner
Author
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.
A GMI-pinned Qwen3-235B lane hit 429 (rate-limited upstream), retried after 30 s, and got a 400 whose body mentioned "context". contextOverflow matched the bare word, found no window and no count, shed a 2,476-token request to nothing and ended the run as an error (3 of 7 runs in results-q235). Now: overflow wording or numbers only (n_ctx, too many tokens, maximum context length, context length/window/size, exceeds…context, context…exceed/too long, prompt is too long, request too large); the 400 body is journalled with the overflow note. Tests added in test/turn-loss.test.ts. Builds on #27. npm run check green (e2e + self-check).