fix(responses): refuse input that cannot fit the model context window - #1696
fix(responses): refuse input that cannot fit the model context window#1696lidge-jun wants to merge 1 commit into
Conversation
#1412 reported ~127k of real context compounding to 1.3M-1.6M tokens and crashing the proxy. A turn that large is rejected upstream anyway, so the round trip buys nothing: it spends auth resolution, host-circuit budget, and bandwidth to arrive at a worse error than we can produce locally. The gate runs after final route normalization, so it measures what will actually be sent including replay expansion, and before auth and the circuit, so an oversized turn costs neither. Three things it deliberately does not do. It does not refuse compaction turns. Codex sends compaction_trigger BECAUSE context is full, and routed /v1/responses/compact re-enters the same handler with that trigger appended. Refusing them would tell the client to compact and then deny the compaction. It does not re-derive the context window. route.provider is already the routedProviderConfig output, which refuses registry merging when the transport does not match, so a user provider that merely shares a built-in name keeps its own limits. Native models fall back to nativeOpenAiContextWindow, which reads static maps only -- the canonical openai registry entry declares no context fields, so without that fallback the gate would be inert on the default route. Reading the Codex catalog was rejected: it costs existsSync + readFileSync + statSync even on a cache hit, and this is the request path. It does not treat the estimate as exact. estimateTokens is a char-ratio heuristic whose CJK sampling aliases: a payload of 62-char records each starting with one Hangul character samples as 100% CJK while being 1.6% CJK, inflating the estimate 1.6x (measured: 126,046 chars, true ratio 0.0161). Since 4.0/2.5 is exactly 1.6, that is the branch's maximum divergence, so the 2.5x tolerance sits above it and #1412's 10x still clears it fourfold. The regression test pins that payload as admissible. Repairing cjkRatio is left alone on purpose -- estimateTokens also feeds usage accounting and auto-compact, so it needs its own change and its own tests. Verified: bun test tests/input-admission.test.ts (19 pass), bun run typecheck, and tests/core-lab-boundary.test.ts still green for the new core.ts import.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Deterministic PR hygiene checks passed. |
|
Landed on dev as babf254 (cherry-picked from 0298177). Verified that dev previously had context-window accounting but no pre-dispatch admission check: route normalization at core.ts:1840 went straight to circuit/auth admission with no size decision in between. The new src/server/responses/input-admission.ts resolves the ceiling and fails open when no ceiling is known. Verification on the landed commit: bun run typecheck clean, 137 pass / 0 fail across input-admission, continuation-dedup and responses-state, 222 pass / 0 fail across the widened regression slice. Closing in favour of the direct landing. 다음 배포에 포함됩니다. |
1 participant
Summary
Layer 1 of the usage/memory stack. Refuses a turn whose estimated input cannot plausibly fit the model's context window, before any upstream I/O. Design doc:
devlog/_plan/260814_usage_memory_roadmap/010_m0_1_input_admission.md(layer 0, #1693).#1412 reported ~127k of real context compounding to 1.3M–1.6M tokens and crashing the proxy. A turn that large is rejected upstream anyway, so the round trip buys nothing — it spends auth resolution, host-circuit budget, and bandwidth to arrive at a worse error than we can produce locally.
The gate sits in
handleResponsesInnerafterapplyFinalRouteRequestNormalization(so it measures what will actually be sent, including replay expansion) and before thepreAuthHostKeycircuit block (so an oversized turn costs neither auth nor circuit budget).Three deliberate non-behaviors, each of which was a review blocker:
It does not refuse compaction turns. Codex sends
compaction_triggerbecause context is full (src/responses/parser.ts:355), and routed/v1/responses/compactre-enters the same handler with that trigger appended (src/server/responses/compact.ts:659). 413-ing those would tell the client to compact and then deny the compaction — a deadlock against the very limit the error reports.It does not re-derive the context window.
route.provideris already theroutedProviderConfigoutput, which refuses registry merging when the transport does not match (src/router.ts:251), so a user provider that merely shares a built-in name keeps its own limits. Native models fall back tonativeOpenAiContextWindow— static maps only. That fallback is load-bearing: the canonicalopenairegistry entry declares no context fields, so without it the gate would be inert on the default Codex route. Reading the Codex catalog was rejected because it costsexistsSync+readFileSync+statSynceven on a cache hit, and this is the request path.It does not treat the estimate as exact.
estimateTokensis a char-ratio heuristic whose CJK sampling aliases:cjkRatiosamples everystride-th character, so a payload of 62-char records each starting with one Hangul character samples as 100% CJK while being 1.6% CJK, firing the 2.5-chars/token clamp instead of 4.0.Since 4.0 / 2.5 = 1.6 is that branch's maximum divergence,
ADMISSION_TOLERANCEis 2.5 — above the demonstrated overshoot, while #1412's 10x still clears it fourfold. A regression test pins that exact payload as admissible. RepairingcjkRatiois deliberately left alone:estimateTokensalso feeds usage accounting and auto-compact (src/server/chat-completions.ts:140), so it needs its own change and its own tests.Verification
bun test tests/input-admission.test.ts— 19 pass, 0 failbun run typecheck— cleanbun test tests/core-lab-boundary.test.ts— still green; the newcore.tsimport pulls no Lab moduleNotable test cases: a custom provider named
openaidoes not inherit native limits; a routedprovider/modelid does not take the native fallback; a_compactionRequestturn is admitted at any size; ceiling resolution performs zerofscalls; the aliased CJK payload stays admitted.Stack
Depends on #1693 (layer 0, docs). Review this PR's diff only.
Checklist