-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(combos): hop on a definite zero-output context overflow #4744
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
834e86a
af708f4
bc648a2
861988e
74a23c4
246d703
e3b9913
99749e1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -401,11 +401,15 @@ export function comboFailureCooldownScope( | |||||||||||
| ): ComboFailureCooldownScope { | ||||||||||||
| const code = normalizedFailureCode(options?.code); | ||||||||||||
| // Request-shape refusals first: an oversized request must not cool a healthy target. | ||||||||||||
| // A native transport can surface a zero-output model overflow as a generic | ||||||||||||
| // upstream_server_error carrying precise context-window prose, so consult the bounded | ||||||||||||
| // message classifier too: that target is healthy, the turn was simply too large for it. | ||||||||||||
| if ( | ||||||||||||
| status === 413 | ||||||||||||
| || REQUEST_SHAPE_FAILURE_CODES.has(code) | ||||||||||||
| || isRequestLocalFreePromptCap(status, message, options?.code) | ||||||||||||
| || isProviderTargetContextOverflow(status, message, options?.code) | ||||||||||||
| || isDefiniteContextOverflow(status, message) | ||||||||||||
| || isRequestLocalTargetIncompatibility(status, message, options?.code) | ||||||||||||
| ) return "none"; | ||||||||||||
| if (isProviderScopedQuotaCap(status, message, options?.code)) return "provider"; | ||||||||||||
|
|
@@ -451,13 +455,85 @@ function isProviderTargetContextOverflow( | |||||||||||
| && /\bprompt\s+\d+\s*>\s*\d+\s+maximum context length\b/i.test(message); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| /** A status can carry a verdict about the REQUEST; 401/403/429 speak about the credential. */ | ||||||||||||
| const CONTEXT_VERDICT_STATUSES: ReadonlySet<number> = new Set([400, 413, 422]); | ||||||||||||
|
|
||||||||||||
| /** | ||||||||||||
| * Phrases a provider emits when the INPUT does not fit this model's context window. Matched | ||||||||||||
| * against the innermost provider message only, so an unrelated refusal that merely quotes one | ||||||||||||
| * of these tokens in a code field cannot authorize a replay. | ||||||||||||
| */ | ||||||||||||
| const DEFINITE_CONTEXT_OVERFLOW_PHRASES = [ | ||||||||||||
| "exceeds the context window", | ||||||||||||
| "exceed the context window", | ||||||||||||
| "context window exceeded", | ||||||||||||
| "context length exceeded", | ||||||||||||
| "maximum context length", | ||||||||||||
| "maximum context window", | ||||||||||||
| "too many tokens", | ||||||||||||
| ]; | ||||||||||||
|
|
||||||||||||
| /** Wrapper envelopes unwrapped before the leaf message is read. */ | ||||||||||||
| const MAX_CONTEXT_OVERFLOW_ENVELOPES = 4; | ||||||||||||
|
|
||||||||||||
| function isDefiniteContextOverflowMessage(text: string): boolean { | ||||||||||||
| const normalized = text.toLowerCase(); | ||||||||||||
| return normalized === "context_length_exceeded" | ||||||||||||
| || DEFINITE_CONTEXT_OVERFLOW_PHRASES.some(phrase => normalized.includes(phrase)); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| /** | ||||||||||||
| * Confirm a context overflow from the provider MESSAGE rather than from a code token that | ||||||||||||
| * merely appears somewhere in the envelope. An upstream controls both fields and can emit a | ||||||||||||
| * contradictory pair -- `context_length_exceeded` beside `Unsupported parameter: user` -- and | ||||||||||||
| * that is not evidence the turn is too large for this model. `classifyError` reads the whole | ||||||||||||
| * blob, which is exactly the looseness this must not inherit. | ||||||||||||
| * | ||||||||||||
| * A JSON-shaped body that fails to parse is truncated or corrupt, not prose: `classificationText` | ||||||||||||
| * is capped at 500 characters by `normalizeUpstreamErrorText` before it reaches this function, so | ||||||||||||
| * a long envelope arrives here as a JSON prefix. Reading that prefix as plain text would let an | ||||||||||||
| * arbitrary field that happens to sit in the first 500 bytes authorize a hop, so it fails closed. | ||||||||||||
| * | ||||||||||||
| * Only the exact proxy wrapper is unwrapped, within a fixed envelope budget and 16,384 characters. | ||||||||||||
| */ | ||||||||||||
| function isDefiniteContextOverflow(status: number, message: string): boolean { | ||||||||||||
| if (!CONTEXT_VERDICT_STATUSES.has(status) && status < 500) return false; | ||||||||||||
| if (message.length > 16_384) return false; | ||||||||||||
| let text = message.trim(); | ||||||||||||
| // One pass per unwrapped envelope, plus one for the leaf the last envelope yields. | ||||||||||||
| for (let unwrapped = 0; unwrapped <= MAX_CONTEXT_OVERFLOW_ENVELOPES; unwrapped += 1) { | ||||||||||||
| const providerPrefix = /^Provider error \d{3}:\s*/.exec(text); | ||||||||||||
| if (providerPrefix) text = text.slice(providerPrefix[0].length).trim(); | ||||||||||||
| if (!text.startsWith("{")) return isDefiniteContextOverflowMessage(text); | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Parse non-object JSON before text classification. At The existing tests cover non-object JSON without an overflow phrase, but not phrase-bearing arrays or string scalars. Parse these JSON-shaped values and reject every parsed value that is not an object. Proposed fix- if (!text.startsWith("{")) return isDefiniteContextOverflowMessage(text);
+ const startsJsonValue = text.startsWith("{")
+ || text.startsWith("[")
+ || text.startsWith('"');
+ if (!startsJsonValue) return isDefiniteContextOverflowMessage(text);
if (unwrapped === MAX_CONTEXT_OVERFLOW_ENVELOPES) return false;
let payload: unknown;
try { payload = JSON.parse(text); } catch { return false; }
if (!payload || typeof payload !== "object" || Array.isArray(payload)) return false;📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||
| if (unwrapped === MAX_CONTEXT_OVERFLOW_ENVELOPES) return false; | ||||||||||||
| let payload: unknown; | ||||||||||||
| try { payload = JSON.parse(text); } catch { return false; } | ||||||||||||
| if (!payload || typeof payload !== "object" || Array.isArray(payload)) return false; | ||||||||||||
| const record = payload as Record<string, unknown>; | ||||||||||||
| const response = record.response && typeof record.response === "object" && !Array.isArray(record.response) | ||||||||||||
| ? record.response as Record<string, unknown> | ||||||||||||
| : undefined; | ||||||||||||
| const source = [record.error, response?.error, response?.last_error, record.last_error, record] | ||||||||||||
| .find((candidate): candidate is Record<string, unknown> => | ||||||||||||
| !!candidate && typeof candidate === "object" && !Array.isArray(candidate) | ||||||||||||
| && typeof (candidate as Record<string, unknown>).message === "string"); | ||||||||||||
| if (!source) return false; | ||||||||||||
| text = (source.message as string).trim(); | ||||||||||||
| } | ||||||||||||
| return false; | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| export function comboFailureDecision( | ||||||||||||
| status: number, | ||||||||||||
| message: string, | ||||||||||||
| options?: { code?: string | null }, | ||||||||||||
| ): ComboFailureDecision { | ||||||||||||
| if (status === 499) return "stop"; | ||||||||||||
| if (message.toLowerCase().includes("origin_rejected")) return "stop"; | ||||||||||||
| // Structured form of the same hard refusal. The prose test above misses it when the origin | ||||||||||||
| // reports the code out of band, and every hop rule below -- including the context-overflow | ||||||||||||
| // one -- must stay subordinate to it. | ||||||||||||
| if (normalizedFailureCode(options?.code) === "origin_rejected") return "stop"; | ||||||||||||
| // The origin may already be executing this turn (the Codex WebSocket relay sent the create | ||||||||||||
| // frame and never saw a response event). Hopping would send the same request to a second | ||||||||||||
| // target while the first may still be generating; the honest status goes to the client. | ||||||||||||
|
|
@@ -476,6 +552,15 @@ export function comboFailureDecision( | |||||||||||
| // (for example 5059 + invalid_request_prompt_too_long). That is evidence that this | ||||||||||||
| // target is too small, not that every later combo target is incapable of serving it. | ||||||||||||
| if (isProviderTargetContextOverflow(status, message, options?.code)) return "hop"; | ||||||||||||
| // A definite context-window refusal is target-local inside a heterogeneous combo: this model | ||||||||||||
| // cannot hold the turn, but a later target may have a larger window. Two boundaries keep this | ||||||||||||
| // safe. It is reached only after cancellation, structured origin/cyber refusals and | ||||||||||||
| // non-replayable post-send codes have already stopped. And it only ever classifies a failure | ||||||||||||
| // the combo stream preflight already proved emitted no output: `comboStreamPayloadCommitsOutput` | ||||||||||||
| // commits the child on any text, tool call or unknown event, and only a zero-output terminal | ||||||||||||
| // becomes a failure response at all, so a turn whose text the client already saw is never | ||||||||||||
| // reclassified here. | ||||||||||||
| if (isDefiniteContextOverflow(status, message)) return "hop"; | ||||||||||||
| // A local input-admission refusal (#1524) says "this candidate cannot fit the request", | ||||||||||||
| // not "the request is impossible": the next candidate may have a larger context window. | ||||||||||||
| // | ||||||||||||
|
|
||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a provider returns a 400/5xx credential or billing error whose leaf message also mentions a phrase such as “maximum context window” (for example,
code: "invalid_api_key"with “key is invalid for the maximum context window tier”), this new predicate returns"none"beforePROVIDER_SCOPED_FAILURE_CODESis checked. That contradicts the existing provider-wide handling for these structured codes and causes every target sharing the bad credential to remain eligible and be retried on later requests. Provider-scoped status/codes should take precedence over the prose-only context classifier.Useful? React with 👍 / 👎.