diff --git a/devlog/_plan/260918_lane_a_bug_train/040_combo_response_format.md b/devlog/_plan/260918_lane_a_bug_train/040_combo_response_format.md new file mode 100644 index 00000000000..008a3d6a09a --- /dev/null +++ b/devlog/_plan/260918_lane_a_bug_train/040_combo_response_format.md @@ -0,0 +1,59 @@ +# #4903 — a capability refusal classified as a request-shape refusal + +## Reproduction, re-derived at the tip + +A combo opens a new conversation. The shadow title call carries `response_format`, the first +target's Alibaba gateway answers HTTP 400, and the chain stops instead of trying the target +behind it. + +`comboFailureDecision` reaches +`["origin_rejected", "context_length_exceeded", "invalid_request_error"].includes(error.code)` +and returns `stop`. `isRequestLocalTargetIncompatibility` runs first and could return `hop`, +but it refuses at its own first guard: the gateway's `invalid_parameter_error` is not in the +generic code set, and none of its three accepted shapes — `Unsupported parameter: user`, an +`unsupported_value` on `reasoning.effort`, and a model-scoped image-input rejection — describes +a `response_format` refusal. + +There is a second blocker the issue body does not name, and it is why the reported text reads +`Provider error 400: data: {...}`. The gateway reports the refusal inside a single SSE frame. +`normalizeUpstreamErrorText` cannot parse `data: {...}` as JSON, so `classificationText` keeps +the raw frame and `upstreamCode` arrives `undefined`. Even with the code set widened, the +envelope check would still fail on the unparsed frame. + +## Why neither obvious option was taken + +Hopping on every 400 replays a genuinely malformed request against every remaining target. +Dropping `response_format` changes the output contract the caller asked for, silently, on a path +whose entire purpose is a structured result. + +So the verdict is narrowed to a capability claim: the message must name `response_format` AND +say it is unavailable or unsupported. "Invalid schema for response_format" names the field and +claims nothing about capability, and stays terminal. + +## The envelope + +- HTTP 400, intact provider JSON, `type: "invalid_request_error"`, three-envelope depth budget, + 16,384-character bound — the same discipline the existing predicate uses. +- Code set is the shared generic one plus `invalid_parameter_error`, held in its own set so the + `user` and image branches are not widened by a code they were never reasoned about. +- `param` may be absent or explicitly null; a param naming another field contradicts the message + and fails closed. +- One `data:` prefix is unwrapped, and only on a single-line body. That unwraps one frame rather + than parsing a stream, so a multi-event body is left alone and still fails closed. + +## What the next target gets + +The same request, `response_format` included. A target that can honour the contract honours it; +one that cannot is skipped in turn. Traversal stays finite because combo excludes each attempted +target and policy tries each candidate once. The verdict records no cooldown, since a capability +gap says the target is healthy and the request did not fit it. + +Cancellation, structured origin and cyber-policy refusals, and the non-replayable post-send codes +are all tested before this verdict and remain authoritative. + +## Relationship to #4817 + +`#4817` forwards a zero-output SSE bare error event to the next target only when +`comboFailureDecision` already says `hop`. This issue is the opposite half: the decision said +`stop`, so that path could never carry it. The two are complementary and neither closes the +other. diff --git a/src/combos/failover.ts b/src/combos/failover.ts index f5715da9baf..21de270e93c 100644 --- a/src/combos/failover.ts +++ b/src/combos/failover.ts @@ -394,6 +394,103 @@ function isRequestLocalTargetIncompatibility(status: number, message: string, co return false; } +/** + * Codes a gateway uses when it declines a request field it cannot serve. + * + * Wider than the set {@link isRequestLocalTargetIncompatibility} accepts, by exactly one member: + * Alibaba's gateway reports `invalid_parameter_error`. It is kept in its own set rather than + * added to the shared one, because that set also governs the `user` parameter and image-input + * branches and widening it there would admit shapes those branches were reasoned about without. + */ +const RESPONSE_FORMAT_REFUSAL_CODES = new Set([ + "", + "invalid_request_error", + "invalid_parameter_error", + "unsupported_parameter", + "unsupported_value", +]); + +/** + * Does this message say the target cannot PROVIDE `response_format`, rather than that the + * request's `response_format` was malformed? + * + * That distinction is the whole point of #4903 and it is why neither obvious option was taken. + * Hopping on every 400 would replay a genuinely malformed request against every remaining + * target. Dropping `response_format` would silently change the output contract the caller + * asked for, on a path whose entire purpose is a structured result. + * + * So both halves are required: the message must name the field, AND it must say the field is + * unavailable or unsupported. "Invalid schema for response_format" names the field and claims + * nothing about capability, so it stays terminal. + * + * `param` may be absent or explicitly null -- the reported gateway sends `param: null` -- but a + * param naming a DIFFERENT field contradicts the message and fails closed. + */ +function namesResponseFormatIncapability(message: string, param: unknown): boolean { + if (param !== undefined && param !== null && param !== "response_format") return false; + const text = message.toLowerCase(); + if (!text.includes("response_format")) return false; + return /(unavailable|not available|unsupported|not supported|does not support|doesn't support|cannot be used|is not enabled)/u + .test(text); +} + +/** + * A `response_format` capability gap is target-local: this model cannot produce the requested + * output shape, which says nothing about the next target in the combo. + * + * Reported against a shadow title-generation call, where a combo's first target rejects + * `response_format` and the chain stops instead of trying the target behind it (#4903). + * + * The next target receives the SAME request, `response_format` included, so a target that can + * honour the contract honours it and one that cannot is skipped in turn. Traversal stays finite + * because combo excludes each attempted target and policy tries each candidate once. + * + * The envelope is bounded exactly like {@link isRequestLocalTargetIncompatibility}: an intact + * provider JSON object, a depth budget, `type: "invalid_request_error"`, and a code from a + * closed set. Nothing is inferred from echoed prompt text and no field is removed. + */ +function isResponseFormatCapabilityRefusal( + status: number, + message: string, + code?: string | null, +): boolean { + if (status !== 400 || message.length > 16_384) return false; + if (!RESPONSE_FORMAT_REFUSAL_CODES.has(normalizedFailureCode(code))) return false; + let text = message.trim(); + for (let depth = 0; depth < 3; depth += 1) { + if (text.startsWith("Provider error 400: ")) { + text = text.slice("Provider error 400: ".length).trim(); + } + // A chat gateway can report the refusal inside a single SSE frame, and the combo consumer + // keeps the raw text when that frame stops the error object from being extracted -- which is + // why the reported classification text reads `data: {"error":...}` and why the structured + // code arrives undefined. Exactly one `data:` prefix is removed, and only when the body is + // one line: this unwraps a single frame rather than parsing a stream, so a multi-event body + // is left alone and still fails closed. + if (text.startsWith("data:") && !text.includes("\n")) { + text = text.slice("data:".length).trim(); + } + let payload: unknown; + try { payload = JSON.parse(text); } catch { return false; } + if (!payload || typeof payload !== "object" || Array.isArray(payload)) return false; + const error = (payload as Record).error; + if (!error || typeof error !== "object" || Array.isArray(error)) return false; + const e = error as Record; + if (e.code !== undefined && e.code !== null && typeof e.code !== "string") return false; + if (typeof e.message !== "string") return false; + // Our own wrapper, re-wrapped by a downstream hop. Peel it and look again, within budget. + if (e.message.startsWith("Provider error 400: ") && e.param === undefined) { + text = e.message; + continue; + } + if (e.type !== "invalid_request_error") return false; + const errorCode = normalizedFailureCode(typeof e.code === "string" ? e.code : undefined); + if (!RESPONSE_FORMAT_REFUSAL_CODES.has(errorCode)) return false; + return namesResponseFormatIncapability(e.message, e.param); + } + return false; +} + export function comboFailureCooldownScope( status: number, message: string, @@ -411,6 +508,9 @@ export function comboFailureCooldownScope( || isProviderTargetContextOverflow(status, message, options?.code) || isDefiniteContextOverflow(status, message) || isRequestLocalTargetIncompatibility(status, message, options?.code) + // A capability gap says the target is healthy and the request did not fit it, which is the + // same reason every other entry here refuses to cool a target. + || isResponseFormatCapabilityRefusal(status, message, options?.code) ) return "none"; if (isProviderScopedQuotaCap(status, message, options?.code)) return "provider"; // A rejected or unpaid credential is provider-wide evidence: every target that routes @@ -595,6 +695,11 @@ export function comboFailureDecision( // per-request cap, not provider-wide evidence), so keep its hop verdict explicit here. if (failureCode === "free_rate_limited") return "hop"; if (isRequestLocalTargetIncompatibility(status, message, options?.code)) return "hop"; + // Must precede the generic `invalid_request_error` stop below, which is where this refusal + // ended the chain: the gateway reports `type: "invalid_request_error"`, so the classifier + // reaches that list and returns terminal before anything can ask whether the next target + // could have served the request (#4903). + if (isResponseFormatCapabilityRefusal(status, message, options?.code)) return "hop"; if (["origin_rejected", "context_length_exceeded", "invalid_request_error"].includes(error.code ?? "")) { return "stop"; } diff --git a/structure/runtime.md b/structure/runtime.md index 94c8ad9fc82..634c44a074e 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -502,6 +502,8 @@ Translated audio/file admission follows the [final-adapter input contract](adapt `src/combos/failover.ts` treats three intact HTTP 400 invalid-request envelopes as request-local incompatibilities: exactly `Unsupported parameter: user`; `unsupported_value` naming `reasoning.effort` or `reasoning_effort` with an explicit unsupported-value message; and `param: input` with a bounded model-scoped `does not support image inputs` message. A null provider code is accepted only for that observed image envelope. Only the exact proxy wrapper is unwrapped, within three envelopes and 16,384 characters; conflicting codes, malformed/truncated envelopes and reflected JSON do not gain hop permission. +A `response_format` capability refusal is a fourth envelope, kept separate because it needs one code and one frame the three above do not admit. The refusal must name `response_format` AND state that it is unavailable or unsupported; a message that merely names the field, such as an invalid-schema complaint, stays terminal, because replaying a malformed request at every later target is the outcome this distinction exists to avoid. `param` may be absent or explicitly null and a param naming another field fails closed. Its code set is the shared generic one plus `invalid_parameter_error`, held separately so the `user` and image branches are not widened by it. It also unwraps a single `data:` SSE frame on a one-line body — the reported gateway answers on the stream, so the error object is never extracted and the structured code arrives undefined — while a multi-event body is left alone. The next target receives the same request with `response_format` intact: no field is dropped and the output contract the caller asked for is unchanged. Traversal stays finite because combo excludes each attempted target. This verdict records no cooldown, and cancellation, origin/cyber-policy rejection and the non-replayable post-send codes are all tested before it (#4903). + The combo may advance to its next eligible unattempted target before output commitment. It records no target/provider cooldown for these request-local mismatches and does not silently drop reasoning controls or raise `none` to a supported rung. Cancellation, origin/cyber-policy rejection, non-replayable post-send errors and the existing streaming commit boundary stay authoritative. Apart from the definite context overflow below, other invalid requests remain terminal. A definite context-window overflow is the fourth request-local verdict. A heterogeneous combo mixes windows, so "this turn does not fit THIS model" is not "this turn is impossible", and stopping at the first undersized target burned the ladder on turns a later target could hold. Evidence must come from the innermost provider message: `classifyError` remaps any occurrence of `context window`, `context length`, `maximum context` or `too many tokens` anywhere in the blob, and inheriting that looseness would let a `context_length_exceeded` token sitting in a `code` field beside `Unsupported parameter: user` authorize a replay. `src/combos/failover.ts` therefore unwraps only the exact proxy wrapper, within four envelopes and 16,384 characters, and reads the leaf message. A JSON-shaped body that does not parse fails closed, because `normalizeUpstreamErrorText` caps `classificationText` at 500 characters and a long envelope arrives here as a prefix. The verdict is admitted only for statuses that speak about the request — 400, 413, 422 and 5xx — so a 401/403 body that merely quotes context prose keeps its provider-wide cooldown instead of being rescored as request-shaped. Structured `origin_rejected`, cyber policy and the non-replayable post-send codes are all tested before it. diff --git a/tests/routing/router-combo-failover-classification.test.ts b/tests/routing/router-combo-failover-classification.test.ts index 0704dc30f22..d4755185812 100644 --- a/tests/routing/router-combo-failover-classification.test.ts +++ b/tests/routing/router-combo-failover-classification.test.ts @@ -356,3 +356,88 @@ describe("image rejection classifier bounds", () => { expect(comboFailureCooldownScope(400, message, { code: "invalid_request_error" })).toBe("none"); }); }); + +/** + * #4903. A combo opens a new conversation, the shadow title call carries `response_format`, and + * the first target's gateway refuses it. The chain stopped instead of trying the target behind + * it, because the gateway reports `type: "invalid_request_error"` and that reaches the generic + * terminal list before anything asks whether the next target could serve the request. + * + * Neither obvious alternative is taken here. Hopping on every 400 would replay a genuinely + * malformed request against every remaining target; dropping `response_format` would change the + * output contract the caller asked for. Only a refusal that names the field AND says it is + * unavailable is treated as a target-local capability gap. + */ +describe("response_format capability refusal", () => { + const unavailable = { + code: "invalid_parameter_error", + param: null, + message: "This response_format type is unavailable now", + type: "invalid_request_error", + }; + const reported = JSON.stringify({ error: unavailable, id: "chatcmpl-946b178a" }); + + test("hops without cooling, through every envelope the report shows", () => { + // The last two are the shape actually observed: the gateway reports the refusal in a single + // SSE frame, so the error object is never extracted and the structured code arrives + // undefined -- which is why the user sees `Provider error 400: data: {...}`. + for (const message of [ + reported, + `Provider error 400: ${reported}`, + `data: ${reported}`, + `Provider error 400: data: ${reported}`, + ]) { + expect(comboFailureDecision(400, message)).toBe("hop"); + expect(comboFailureCooldownScope(400, message)).toBe("none"); + } + // And when the gateway's own code does survive extraction. + expect(comboFailureDecision(400, reported, { code: "invalid_parameter_error" })).toBe("hop"); + expect(comboFailureCooldownScope(400, reported, { code: "invalid_parameter_error" })).toBe("none"); + }); + + test("a malformed response_format is a request defect and stays terminal", () => { + // Names the field, claims nothing about capability. Replaying this at every later target + // is exactly what the issue asked not to do. + for (const message of [ + "Invalid schema for response_format 'reply': 'type' is a required property.", + "response_format.type must be one of 'text', 'json_object', 'json_schema'.", + ]) { + expect(comboFailureDecision(400, JSON.stringify({ + error: { ...unavailable, message }, + }))).toBe("stop"); + } + }); + + test("an unavailability claim about some other field stays terminal", () => { + expect(comboFailureDecision(400, JSON.stringify({ + error: { ...unavailable, message: "This parameter type is unavailable now" }, + }))).toBe("stop"); + // A param naming a different field contradicts the message, so it fails closed. + expect(comboFailureDecision(400, JSON.stringify({ + error: { ...unavailable, param: "messages" }, + }))).toBe("stop"); + }); + + test("the envelope stays bounded and fails closed", () => { + // Reflected prose, a truncated body, a non-400 status, an unrecognized code, a wrong type, + // and a multi-event stream body are all refused. The single-frame unwrap is one frame. + expect(comboFailureDecision(400, `please fix ${reported}`)).toBe("stop"); + expect(comboFailureDecision(400, reported.slice(0, -1))).toBe("stop"); + expect(comboFailureDecision(422, reported)).toBe("stop"); + expect(comboFailureDecision(400, reported, { code: "unknown_terminal_code" })).toBe("stop"); + expect(comboFailureDecision(400, JSON.stringify({ + error: { ...unavailable, type: "server_error" }, + }))).toBe("stop"); + expect(comboFailureDecision(400, `data: ${reported}\ndata: [DONE]`)).toBe("stop"); + expect(comboFailureDecision(400, JSON.stringify({ + error: { ...unavailable, message: `This response_format type is unavailable now ${"x".repeat(16_384)}` }, + }))).toBe("stop"); + }); + + test("a hard refusal still outranks the capability verdict", () => { + for (const code of ["origin_rejected", "upstream_no_response", "cyber_policy"]) { + expect(comboFailureDecision(400, reported, { code })).toBe("stop"); + } + expect(comboFailureDecision(499, reported)).toBe("stop"); + }); +});