From 7172e2513f9561146eebd1fd76e7def939ce0529 Mon Sep 17 00:00:00 2001 From: MumuTW <42820974+MumuTW@users.noreply.github.com> Date: Sat, 25 Jul 2026 17:51:06 +0800 Subject: [PATCH] refactor(sse): extract combo 400-classification predicates into a leaf module combo.ts sits at 3640 lines against a frozen file-size ratchet of 3642. The gate is shrink-only, so ANY change that adds a line to this file is rejected -- including a one-line bug fix plus its explanatory comment. Two in-flight #7847 fixes both hit this, and one shipped a violation because lint-staged's Prettier reflowed an unrelated over-long line (+4 lines) AFTER the gate had already been checked. Squeezing comments to fit is optimizing the wrong thing. This restores real headroom instead: the three 400-classification predicates are pure string classifiers over upstream error text, sharing the canonical pattern lists in accountFallback.ts, and have no business inside the routing loop. Moving them takes combo.ts to 3610 -- 30 lines of headroom. Behavior-preserving. The bodies were extracted from both revisions and compared after normalizing whitespace: 3 of 3 byte-identical. combo.ts imports them for the fallback guard and re-exports them, so the public surface is unchanged and tests/unit/combo-param-validation-fallback-4519.test.ts still imports them from combo.ts without modification (6/6 pass). Note for anyone repeating this: a bare re-export does not bind the name locally, so combo.ts's own use of the predicates needed a separate import. typecheck caught it. --- .../maintenance/combo-error-classifiers.md | 1 + open-sse/services/combo.ts | 58 +++++-------------- .../services/combo/errorClassification.ts | 55 ++++++++++++++++++ 3 files changed, 70 insertions(+), 44 deletions(-) create mode 100644 changelog.d/maintenance/combo-error-classifiers.md create mode 100644 open-sse/services/combo/errorClassification.ts diff --git a/changelog.d/maintenance/combo-error-classifiers.md b/changelog.d/maintenance/combo-error-classifiers.md new file mode 100644 index 000000000000..95c816673be0 --- /dev/null +++ b/changelog.d/maintenance/combo-error-classifiers.md @@ -0,0 +1 @@ +- chore(sse): extract the combo 400-classification predicates (`isContextOverflow400`, `isParamValidation400`, `isModelScoped400`) into `combo/errorClassification.ts` — behavior-preserving move that takes `combo.ts` from 3640 to 3610 lines, restoring headroom under the frozen file-size ratchet so ordinary fixes to that file are no longer blocked by the gate diff --git a/open-sse/services/combo.ts b/open-sse/services/combo.ts index 6009430177b2..46b07032b2f3 100644 --- a/open-sse/services/combo.ts +++ b/open-sse/services/combo.ts @@ -680,49 +680,15 @@ async function isPinnedModelDurablyUnhealthy(pinnedModel: string): Promise p.test(errorText)) - ); -} -/** @param {string} errorText */ -export function isParamValidation400(errorText) { - return ( - /\bmax_tokens\b.*(?:illegal|must|range|invalid)/i.test(errorText) || - /\bparameter is illegal\b/i.test(errorText) || - /\bis illegal.*range\b/i.test(errorText) - ); -} -/** - * #5249 / #2101: model-scoped 400s must NEVER stop the combo. - * Upstream often wraps "model X is not supported" in `invalid_request_error` / - * "Bad Request" envelopes. Those wrapper words match the body-specific stop - * substrings, so without this exemption the combo hard-stops on the first - * unavailable model instead of trying the next target. Keep the models in the - * combo — if one rejects, advance. - * @param {string} errorText - */ -export function isModelScoped400(errorText) { - const text = String(errorText || ""); - if (!text) return false; - if (MODEL_ACCESS_DENIED_PATTERNS.some((p) => p.test(text))) return true; - // Extra model-rejection shapes that providers emit outside the shared list - // (Responses API, Copilot, gateway wrappers). - return ( - /\bmodel\b[\s\S]{0,80}?\b(?:not\s+supported|unsupported|unknown|unavailable)\b/i.test(text) || - /\b(?:not\s+supported|unsupported|unknown)\b[\s\S]{0,80}?\bmodel\b/i.test(text) || - /\bunsupported_api_for_model\b/i.test(text) || - /\bdoes\s+not\s+support\s+(?:the\s+)?responses\s+api\b/i.test(text) - ); -} +// The 400 classifiers live in ./combo/errorClassification.ts. Imported for local use in the +// fallback guard below and re-exported for the historical public surface +// (combo-param-validation-fallback-4519 imports them from combo.ts). +import { + isContextOverflow400, + isParamValidation400, + isModelScoped400, +} from "./combo/errorClassification.ts"; +export { isContextOverflow400, isParamValidation400, isModelScoped400 }; /** @param {object} options */ export async function handleComboChat({ @@ -2924,7 +2890,11 @@ async function handleRoundRobinCombo({ // BEFORE availability is known; if every compat-kept target then turns out to be // runtime-unavailable, we must reconsider these before returning 503, instead of // permanently dropping a compat-rejected-but-healthy provider. - const compatRejectedTargets = computeCompatRejectedTargets(evalRankedTargets, filteredTargets, body); + const compatRejectedTargets = computeCompatRejectedTargets( + evalRankedTargets, + filteredTargets, + body + ); let modelCount = filteredTargets.length; if (modelCount === 0) { return comboModelNotFoundResponse("Round-robin combo has no executable targets"); diff --git a/open-sse/services/combo/errorClassification.ts b/open-sse/services/combo/errorClassification.ts new file mode 100644 index 000000000000..2cecb706ac07 --- /dev/null +++ b/open-sse/services/combo/errorClassification.ts @@ -0,0 +1,55 @@ +/** + * Upstream 400 classification for combo fallback (extracted from combo.ts). + * + * These three predicates decide whether a 400 should ADVANCE the combo to the next target or + * hard-stop it. They are pure classifiers over the upstream error text and share the canonical + * pattern lists in accountFallback.ts, so they do not belong inside the routing loop. + * + * combo.ts re-exports them, so the public surface is unchanged (combo-param-validation-fallback-4519 + * imports them from combo.ts). Behavior-preserving move — bodies are identical to the originals. + */ +import { CONTEXT_OVERFLOW_PATTERNS, MODEL_ACCESS_DENIED_PATTERNS } from "../accountFallback.ts"; + +/** @param {string} errorText */ +export function isContextOverflow400(errorText) { + return ( + /\bcontext.*(?:length_exceeded|too long|overflow|exceeded|window|limit)\b/i.test(errorText) || + /exceeds.*context/i.test(errorText) || + /your input exceeds/i.test(errorText) || + // Reuse accountFallback.ts's CONTEXT_OVERFLOW_PATTERNS (single source of truth) + // so wording like Kimi's "exceeded model token limit" — which never says the + // literal word "context" — is still recognized as an overflow/fallback-worthy + // 400 instead of halting the whole combo (issue #6637). + CONTEXT_OVERFLOW_PATTERNS.some((p) => p.test(errorText)) + ); +} +/** @param {string} errorText */ +export function isParamValidation400(errorText) { + return ( + /\bmax_tokens\b.*(?:illegal|must|range|invalid)/i.test(errorText) || + /\bparameter is illegal\b/i.test(errorText) || + /\bis illegal.*range\b/i.test(errorText) + ); +} +/** + * #5249 / #2101: model-scoped 400s must NEVER stop the combo. + * Upstream often wraps "model X is not supported" in `invalid_request_error` / + * "Bad Request" envelopes. Those wrapper words match the body-specific stop + * substrings, so without this exemption the combo hard-stops on the first + * unavailable model instead of trying the next target. Keep the models in the + * combo — if one rejects, advance. + * @param {string} errorText + */ +export function isModelScoped400(errorText) { + const text = String(errorText || ""); + if (!text) return false; + if (MODEL_ACCESS_DENIED_PATTERNS.some((p) => p.test(text))) return true; + // Extra model-rejection shapes that providers emit outside the shared list + // (Responses API, Copilot, gateway wrappers). + return ( + /\bmodel\b[\s\S]{0,80}?\b(?:not\s+supported|unsupported|unknown|unavailable)\b/i.test(text) || + /\b(?:not\s+supported|unsupported|unknown)\b[\s\S]{0,80}?\bmodel\b/i.test(text) || + /\bunsupported_api_for_model\b/i.test(text) || + /\bdoes\s+not\s+support\s+(?:the\s+)?responses\s+api\b/i.test(text) + ); +}