From 9e68f6f3b1faae55318dcb38c996c9dd7fc1d97c Mon Sep 17 00:00:00 2001 From: 1bcMax <195689928+1bcMax@users.noreply.github.com> Date: Tue, 21 Jul 2026 09:40:47 -0500 Subject: [PATCH] fix(proxy): honor an explicit max_tokens instead of overwriting it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The proxy replaced whatever the caller sent with min(adaptive, modelCap), where adaptive is twice the previous reply on that model. A client asking for 500 tokens right after a long turn was forwarded several thousand instead. That is not just a bigger number. The gateway derives its price quote from the ceiling a request asks for (quotedOutputTokens = maxOutputTokens * OUTPUT_QUOTE_FACTOR), so the overwrite inflated the upfront hold on a request the caller had deliberately kept small. Settlement is on actual usage, so nobody was overcharged — but the amount held was not the amount asked for. The mechanism predates this session. What changed is reach: raising MODEL_MAX_OUTPUT in 3.35.2, 3.35.4 and 3.35.5 lifted modelCap for several models from 16384/32768 to 128000, so the overwrite could push roughly 4x further than before. Old mechanism, new blast radius — the same shape as the gateway ceiling raise that needed a headroom guard added after the fact. Now: an explicit, positive, finite ask is honored; only a missing or unusable one gets the adaptive default. Clamping to modelCap stays on both paths, since asking past a model's own ceiling is a request the provider rejects outright and failing it here is worse than quietly making it legal. Extracted the decision into resolveProxyMaxTokens() and exported it. It was buried in the request handler with no way to test it, which is why the overwrite went unnoticed. Four tests now cover the explicit path, the adaptive path, clamping on both, and junk input (0, -1, NaN, Infinity, '4096', {}, true) falling back rather than propagating. Local suite 640 pass. --- src/proxy/server.ts | 39 ++++++++++++++++++++++++++++++++------- test/local.mjs | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 7 deletions(-) diff --git a/src/proxy/server.ts b/src/proxy/server.ts index 537208f..0f1f56c 100644 --- a/src/proxy/server.ts +++ b/src/proxy/server.ts @@ -66,6 +66,37 @@ import { logger, setDebugMode } from '../logger.js'; import { isTestFixtureModel } from '../stats/test-fixture.js'; const DEFAULT_MAX_TOKENS = 4096; + +/** + * Decide the `max_tokens` the proxy forwards upstream. + * + * An explicit ask is honored; only a missing one gets the adaptive default. + * This overwrote unconditionally until 3.35.6 — whatever the caller sent was + * replaced by min(adaptive, modelCap), where `adaptive` grows to twice the + * previous reply. A client asking for 500 tokens after a long turn got several + * thousand instead, and because the gateway quotes on the ceiling it is + * requested, that inflated the hold on a request the caller had deliberately + * kept small. + * + * Clamping an explicit ask to `modelCap` stays: asking past the model's own + * ceiling is a request the provider rejects outright, and failing it here is + * worse than quietly making it legal. + * + * @param asked what the caller sent, if anything + * @param lastOutput tokens the previous reply on this model produced (0 = none) + * @param modelCap the model's own output ceiling + */ +export function resolveProxyMaxTokens( + asked: unknown, + lastOutput: number, + modelCap: number, +): number { + const adaptive = + lastOutput > 0 ? Math.max(lastOutput * 2, DEFAULT_MAX_TOKENS) : DEFAULT_MAX_TOKENS; + const explicit = + typeof asked === 'number' && Number.isFinite(asked) && asked > 0 ? asked : null; + return Math.min(explicit ?? adaptive, modelCap); +} // 180s budget for *time-to-headers* — reasoning-class models (zai/glm-*, // nemotron *-reasoning, deepseek-r*, gpt-5-codex, anthropic extended-thinking) // routinely take 60–120s to first token on cache-cold prompts or busy @@ -390,14 +421,8 @@ export function createProxy(options: ProxyOptions): http.Server { // could emit 65K+ (Kimi K3, GPT-5.6 Sol). Same table both paths now. const modelCap = getMaxOutputTokens(parsed.model || ''); - // Use max of (last output × 2, default 4096) capped by model limit - // This ensures short replies don't starve the next request const lastOut = lastOutputByModel.get(requestModel) ?? 0; - const adaptive = - lastOut > 0 - ? Math.max(lastOut * 2, DEFAULT_MAX_TOKENS) - : DEFAULT_MAX_TOKENS; - parsed.max_tokens = Math.min(adaptive, modelCap); + parsed.max_tokens = resolveProxyMaxTokens(original, lastOut, modelCap); if (original !== parsed.max_tokens && options.debug) { logger.debug(`[franklin] max_tokens: ${original || 'unset'} → ${parsed.max_tokens} (last output: ${lastOut || 'none'})`); diff --git a/test/local.mjs b/test/local.mjs index f0ebb33..4d97827 100644 --- a/test/local.mjs +++ b/test/local.mjs @@ -6728,6 +6728,45 @@ test('openai ceilings are measured, not read off the catalog', async () => { assert.equal(getMaxOutputTokens('openai/gpt-5-mini'), 65_536); }); +// ─── proxy max_tokens: honor an explicit ask ────────────────────────────── + +test('proxy: an explicit max_tokens is honored, not overwritten', async () => { + // Regression. The proxy used to overwrite unconditionally with + // min(adaptive, modelCap), where adaptive = 2x the previous reply. A caller + // asking for 500 after a long turn got 4096+, and since the gateway quotes + // on the ceiling it is asked for, that inflated the hold on a request the + // caller had deliberately kept small. + const { resolveProxyMaxTokens } = await import('../dist/proxy/server.js'); + assert.equal(resolveProxyMaxTokens(500, 20_000, 128_000), 500, + 'a small explicit ask must survive a large previous reply'); + assert.equal(resolveProxyMaxTokens(1, 60_000, 128_000), 1); +}); + +test('proxy: a missing max_tokens still gets the adaptive default', async () => { + // The adaptive path is why this code exists — a caller that omits the field + // should not be starved by a 4096 floor after a long reply. + const { resolveProxyMaxTokens } = await import('../dist/proxy/server.js'); + assert.equal(resolveProxyMaxTokens(undefined, 0, 128_000), 4096, 'no history -> floor'); + assert.equal(resolveProxyMaxTokens(undefined, 10_000, 128_000), 20_000, 'twice the last reply'); + assert.equal(resolveProxyMaxTokens(null, 1_000, 128_000), 4096, 'floor wins under 2x4096'); +}); + +test('proxy: the model ceiling still clamps both paths', async () => { + // Asking past the model's own ceiling is a request the provider rejects, so + // clamping is friendlier than forwarding a guaranteed 400. + const { resolveProxyMaxTokens } = await import('../dist/proxy/server.js'); + assert.equal(resolveProxyMaxTokens(999_999, 0, 128_000), 128_000, 'explicit ask clamped'); + assert.equal(resolveProxyMaxTokens(undefined, 500_000, 65_536), 65_536, 'adaptive clamped'); +}); + +test('proxy: junk in max_tokens falls back to adaptive rather than propagating', async () => { + const { resolveProxyMaxTokens } = await import('../dist/proxy/server.js'); + for (const junk of [0, -1, NaN, Infinity, '4096', {}, true]) { + assert.equal(resolveProxyMaxTokens(junk, 0, 128_000), 4096, + `${String(junk)} must not become the forwarded ceiling`); + } +}); + // ─── gateway catalog as a GAP-FILLER, never an override ─────────────────── // // The static tables are authoritative. Several entries are deliberate