fix(validation): stop capping max_tokens below what models actually serve#27
Merged
Conversation
…erve `validate_max_tokens` rejected anything over 100000 client-side. That number was not a model limit and not the gateway's — it was a hardcoded sanity check with no comment, and it silently became the binding constraint on every SDK caller. Verified against the live gateway 2026-07-21 by bypassing the guard: 19 models advertise a ceiling above 100000 and all 19 accepted it. zai/glm-5.2 262144 accepted claude-opus-4.8 / sonnet-5 / fable-5 / opus-4.7 / sonnet-4.6 128000 accepted gpt-5.6-sol / -terra / -luna, gpt-5.5, gpt-5.4, gpt-5.4-pro, gpt-5.4-mini, gpt-5.2, gpt-5.2-pro, gpt-5.3-codex 128000 accepted glm-5 / glm-5.1 / glm-5-turbo 128000 accepted Zero rejections. A caller asking glm-5.2 for the 262144 it advertises got a ValueError instead of a request. The message made it worse: "max_tokens too large (maximum: 100000)" reads like a provider response. It was read as exactly that during an investigation and recorded as an upstream ceiling in a downstream token table, on the strength of 19 identical "rejections" that never left the process. Now MAX_TOKENS_SANITY_LIMIT = 1_000_000, documented as a typo guard, with a message that says the number is the SDK's and the gateway owns the real limit. Keeping a bound is still right — a byte count or stray 1e9 should fail locally rather than become a payment quote — it just must never be reachable by a real request. Tests: rejection case moved to 2_000_000; added a regression asserting 128000 and 262144 pass. Suite 372 pass.
CI runs black --check; the new test file was not formatted.
VickyXAI
added a commit
that referenced
this pull request
Jul 21, 2026
Follow-up to #27. Two gaps in its tests: - The real-ceiling assertions (128000, 262144) sat inside test_accept_valid_values alongside 1/100/1000, so a failure there would not say which kind of value broke. Split into their own test with the probe evidence attached. - Nothing asserted the message content. The whole reason #27 exists is that the old text, 'max_tokens too large (maximum: 100000)', read like a provider response and was recorded as an upstream model ceiling in a downstream token table. If the message regresses to that shape the bug returns, and no test would have caught it. Now pinned: the message must name the SDK's own limit and disclaim being a model limit. Also references MAX_TOKENS_SANITY_LIMIT instead of hardcoding 2_000_000, so the test follows the constant if it moves. Co-authored-by: 1bcMax <[email protected]>
This was referenced Jul 21, 2026
VickyXAI
added a commit
that referenced
this pull request
Jul 21, 2026
…y max_tokens clamping (1.8.2) (#32) * fix(payments): disclose gateway max_tokens clamping, stop re-paying after settlement The comment justifying MAX_TOKENS_SANITY_LIMIT claimed the gateway "rejects with that model's own number". It does not. Probed against the live 402 leg 2026-07-21 (unpaid quote leg only): opus-4.8 sent 262144 and 1000000 both quote the 128000 price, and gpt-5.2 sent 1e12 returns a quote rather than a 400. The gateway silently clamps to the model ceiling and charges for the clamped value, so there is no server-side rejection to fall back on. The only disclosure is the 402's resource.description, which was read solely to feed resource_description into the signature and then discarded — the one string that would tell a caller "you asked for 500000, you are paying for 128000" was thrown away at the moment it was in hand. _warn_if_clamped now surfaces it on every body-bearing payment path before the caller pays. Separately: _should_fallback returned True for any timeout, including one raised after the PAYMENT-SIGNATURE had gone out. The fallback chain then signed a fresh payment per model, so smart_chat PREMIUM COMPLEX could settle six times and return nothing — the "CHARGED BUT REQUEST FAILED" outcome the CHANGELOG already documents. Errors raised past the settlement boundary are now tagged and refused for fallback. Tagging via attribute rather than a new exception type keeps callers catching httpx.TimeoutException working. Also drops the Raises: docstring promising "PaymentError: If budget is set and would be exceeded" — no budget parameter exists anywhere in the SDK. * style: normalize line endings repo-wide via .gitattributes The repository had no .gitattributes and mixed endings: 17 tracked files were CRLF while the rest were LF. PR #27 converted 3 of them as a side effect of an unrelated fix, which inflated that diff from 51 real lines to 1045 and buried the behavioural change under formatting noise — `git diff` was ~95% churn and `--ignore-cr-at-eol` was needed to review it at all. Converting only 3 left the other 14 to regenerate the same problem on the next contributor's machine. `* text=auto` plus `git add --renormalize .` converts the remaining 14 in one formatting-only commit, so blame stays readable and endings stop depending on who checked out the repo. Verified content-free: `git diff --ignore-cr-at-eol` against the staged tree is empty. * release: 1.8.2 — correct the version the package reports about itself 1.8.1 shipped to PyPI with VERSION and __init__.py still reading 1.8.0, so the installed package reports __version__ == '1.8.0' while PyPI says 1.8.1. pyproject.toml was the only one of three locations I bumped. tests/unit/test_version_consistency.py exists precisely to catch this and did catch it — on CI, after the release had already been cut. I bumped, committed, tagged and released without re-running the suite in between, so a known-failing test never had a chance to stop the publish. Fixing the artifact needs a new version; main alone doesn't repair a wheel already on PyPI. * fix(payments): close the paid-5xx hole in the settlement guard, harden the clamp parse The settlement guard shipped incomplete. _mark_settled was applied only to httpx.TimeoutException and NetworkError, but the dominant post-settlement failure is a paid 5xx, which surfaces as APIError(503) — precisely a status _should_fallback treats as retriable. So the six-settlements-for-zero-tokens path the previous commit claimed to close was still fully open, and the CHANGELOG entry asserting otherwise was wrong. Reproduced before the fix: a 402-then-503 chain across three models sent six signatures. Every exception escaping the paid leg is now tagged. Over-tagging is the safe direction: those handlers begin at an already-read 402 response and create_payment_payload is local EIP-712 signing with no network I/O, so the only exceptions taggable without a settlement are ones _should_fallback already refuses. _warn_if_clamped also had two ways to break the request it was diagnosing. It runs on resource.description, a server-controlled string, immediately before signing: a non-string value raised TypeError, and `(\d[\d,]*)` backtracked super-linearly on a digit run (measured 2.78s at 16k digits). Now a bounded pattern over a 512-char slice, silent when the description is ambiguous rather than reporting a per-unit rate as the ceiling, and wrapped so nothing escapes. Tests: 20 new, covering the tag classification, one-settlement-per-call for both timeout and paid-5xx, that unpaid failures still walk the chain, and the parse. Mutation-verified — reverting the guard fails 5, narrowing it back to timeouts fails 1, restoring the old regex fails 1. Counts distinct signatures rather than signed requests, since the paid leg replays one signature on 502/503. * fix(payments): extend the settlement guard to Solana, gate publish on tests The guard was Base-only. _should_fallback_solana never consulted the settled tag and no Solana paid leg set it, so the double-payment path stayed fully open on one of the two chains while the CHANGELOG read as though both were covered. SPL USDC leaves the wallet on signing exactly like USDC on Base does. Both Solana paid stream phases now tag anything that escapes, and _should_fallback_solana refuses tagged exceptions ahead of its existing checks, so the issue #6 permanent-reason guard is untouched. Tests mirror the Base ones and assert both chains agree on what the tag means; mutation-verified by removing the guard (4 fail). Guarded with importorskip so the 3.9 CI job, which installs without the solana extra, stays green (see #19/#20). publish.yml built and published on release with no test step. That is how 1.8.1 reached PyPI with VERSION and __init__.py still at 1.8.0: the guard test caught it on the push-triggered run, after the release was cut, and PyPI does not allow overwriting a published file. The build job now installs the dev+solana extras and runs the suite before it builds. Promotes the changelog's Unreleased section to 1.8.2 so the four version declarations agree, and says plainly that 1.8.2 exists to supersede a wheel that misreports its own version. * fix(payments): narrow the settled tag to payment outcomes, close abandoned paid streams Review of #32 found three defects in the previous two commits. `except Exception` was too wide. It tagged a paid-leg 402 — which means the facilitator REJECTED the payment and the funds did not move — as blockrun_payment_settled, the one case where the name is exactly backwards. It also relabeled SDK bugs (AttributeError, KeyError, a PEP-479 RuntimeError) as payment outcomes, and `from None` erased the __context__ that explained them: x402.parse_payment_required deliberately raises an opaque "invalid format" whose only diagnostic value is its cause. Narrowed to `(httpx.HTTPError, APIError)`, which is provably exactly the fallback-eligible set — TimeoutException and NetworkError are both HTTPError subclasses, APIError is caught directly, and PaymentError is deliberately not an APIError subclass — so nothing that could trigger a second settlement escapes untagged, while rejections and bugs propagate as themselves. Re-raises bare instead of `from None`, preserving traceback and context. `async for chunk in self._astream_paid_phase(...)` did not close the inner generator when the outer was closed, so an abandoned paid stream stranded the `async with self._client.stream(...)` and its connection until GC finalization. Extracting the paid phase introduced it; the sync path never had it because `yield from` propagates close(). The same defect was already present at the `chat_completion_stream` fallback boundary, so a caller that breaks mid-stream stranded two generators. Both now aclose explicitly. The ReDoS timings in the code comment were wrong — 8k/16k were cited as 1.07s/11.88s, never measured by the author. Re-measured on CPython 3.13: 4k 0.13s, 8k 0.49s, 16k 1.95s. Corrected in the comment, the CHANGELOG and the test docstring so all three agree. Tests: 9 new, and the first drafts of two were rewritten after mutation testing showed they passed for the wrong reason — they asserted isolated shapes rather than driving the client, and the async one could not distinguish a deterministic close from CPython's asyncgen finalizer running at loop teardown. All 7 mutations now fail: removing either aclose, widening or narrowing the handlers, removing either chain's guard, restoring the old regex. * fix(release): make the Solana guard match its CHANGELOG claim, harden the publish gate The CHANGELOG said the settlement guard covers "both Base and Solana", but on Solana only the two streaming legs were tagged. The three non-stream paid legs were unwrapped. Not exploitable today, because _should_fallback_solana is only consulted from the streaming fallback loops and Solana non-stream chat exposes no fallback_models — but the claim was wrong, and the hole would reopen silently the day someone adds a fallback chain there. publish.yml claimed to gate on "the same suite" as CI while running only pytest on 3.11. Now runs black and ruff too, and the comment says plainly what it does and does not cover: the 3.9 and 3.12 legs still only run on push, so version-incompatible syntax is caught there, not here. Also asserts the release tag matches VERSION before anything is built. Cutting v1.8.3 from a 1.8.2 tree passed all 407 tests; PyPI's duplicate-version check was the only thing standing between that and a wrong publish, and it only fires after the release is cut. Same family as the 1.8.1 incident, closed at the same place. --------- Co-authored-by: 1bcMax <[email protected]>
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.
The defect
validate_max_tokensrejected anything over 100,000 client-side. That number is not a model limit and not the gateway's — it was a hardcoded sanity check with no comment, and it quietly became the binding constraint on every SDK caller.A caller asking
zai/glm-5.2for the 262,144 it advertises got aValueErrorinstead of a request.Verification
Probed the live gateway on 2026-07-21 with the guard bypassed in-process. Every model advertising a ceiling above 100,000:
zai/glm-5.2claude-opus-4.8,claude-sonnet-5,claude-fable-5,claude-opus-4.7,claude-sonnet-4.6gpt-5.6-sol/terra/luna,gpt-5.5,gpt-5.4,gpt-5.4-pro,gpt-5.4-mini,gpt-5.2,gpt-5.2-pro,gpt-5.3-codexglm-5,glm-5.1,glm-5-turbo19 models, 19 accepted, zero rejections. The catalog was right the whole time.
The message made it worse
"max_tokens too large (maximum: 100000)"reads like a provider response. During an investigation it was taken for exactly that and recorded as an upstream model ceiling in a downstream token table — on the strength of 19 identical "rejections" that never left the process. The uniform number across three different providers should have been the tell.The fix
MAX_TOKENS_SANITY_LIMIT = 1_000_000, documented as a typo guard, with a message that says the number is the SDK's and the gateway owns the real limit.Keeping a bound is still right — a byte count, a timestamp, or a stray
1e9should fail locally rather than become a payment quote. It just must never be reachable by a real request.Tests
Rejection case moved from 200,000 (a plausible real value) to 2,000,000. Added a regression asserting 128,000 and 262,144 pass, with the probe evidence in the docstring.
372 pass.
Companion PR in the TypeScript SDK, same defect, same hardcoded number.