fix(proxy): honor an explicit max_tokens instead of overwriting it#115
Merged
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found while reviewing #113. Flagged there, fixed here.
The bug
The proxy replaced whatever the caller sent with
min(adaptive, modelCap), whereadaptiveis 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; the amount held just wasn't the amount asked for.Old mechanism, new blast radius
The overwrite predates this session. What changed is reach: raising
MODEL_MAX_OUTPUTin 3.35.2 / 3.35.4 / 3.35.5 liftedmodelCapfor several models from 16,384–32,768 to 128,000, so the overwrite could push roughly 4× further than before.Same shape as the gateway ceiling raise in BlockRunAI/blockrun#266, which needed a context-headroom guard (#268) added after the fact: raising a ceiling disturbs code that quietly relied on it being low.
The fix
500(after a 20k reply)999999An explicit, positive, finite ask is honored; only a missing or unusable one gets the adaptive default. Clamping to
modelCapstays on both paths — 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.Why it went unnoticed
The decision was buried inline in the request handler with no way to test it. Extracted to
resolveProxyMaxTokens()and exported.Four tests: the explicit path, the adaptive path, clamping on both, and junk input (
0,-1,NaN,Infinity,'4096',{},true) falling back rather than propagating into a payment quote.Local suite 640 pass, 0 fail.