fix(server): clamp /v1/models context_length to the allocated KV pool - #531
Open
oscar-investmatic wants to merge 1 commit into
Open
oscar-investmatic wants to merge 1 commit into
oscar-investmatic wants to merge 1 commit into
Conversation
/v1/models published `config.max_seq_len` -- the model's positional ceiling -- while the engine clamps its own `max_seq_len` to `min(config.max_seq_len, num_pages * page_size)` at startup and again in `_refresh_seq_state` after a rebuild, and the scheduler admits against that clamped value. The frontend process holds an unclamped ServerArgs copy, so the two disagreed about one quantity: on a 12 GiB card, 262144 advertised against a 178176-token pool. `ft launch` reads this number to size each agent's context window (opencode `limit.context`, codex `context_window`/`max_context_window`, CLAUDE_CODE_MAX_CONTEXT_TOKENS), so agents set their compaction threshold past what the server can hold and hit `context_length_exceeded` instead of compacting in time. Split the pool resolution out of `cache_geometry` into `kv_pool_geometry` so /v1/models and /v1/cache/status cannot drift apart, and clamp the advertised value with it. Falls back to the model ceiling while `num_pages` is still 0 (loading, or an engine build that sends no meta ack). The existing route test passed either way -- FakeState carries no pool geometry, so it fell through to the ceiling. It now asserts that fallback explicitly, alongside new cases for the clamp, a pool larger than the ceiling, and a rebuild superseding the load-time allocation. Fixes FlashML-org#448 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
JUNQINGV587
added a commit
to JUNQINGV587/FreeToken
that referenced
this pull request
Sep 23, 2026
…cated KV pool Both sides already clamped, but from different sources: this fork trusted the enforced max_seq_len the engine publishes in its readiness meta (falling back to the checkpoint ceiling while that is in flight), upstream derived the limit from the pool geometry (num_pages x page_size). Merged both intents: take the enforced value when present and clamp it by kv_pool_geometry() too, so a still-in-flight meta or an older engine can no longer advertise more context than the scheduler admits. Upstream's two new tests failed against the fork's version alone (262144 advertised vs 178176 allocated) and pass after the combination; the fork's own route tests unchanged. Verified: 951 passed (tests/server).
This branch has not been deployed
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
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.
Fixes #448.
The problem
The engine clamps its own
max_seq_lento the allocated KV pool, at startup and again after every rebuild:and the scheduler admits against that clamped value (
scheduler/scheduler.py:513). That is why #448 shows a correct rejection:171008is the clampedmax_seq_len— the real pool. The server knows its true limit.But
/v1/modelsis served from the frontend process, which holds an unclampedServerArgscopy, so it published the model's positional ceiling instead. The engine and this route disagreed about one quantity, and the route's answer is the one agents consume.Why it bites
launch.py:215readsmax_model_leninto_context_window()(:283), which feeds every agent integration:limit.contextlaunch.py:449context_window/max_context_windowlaunch.py:301CLAUDE_CODE_MAX_CONTEXT_TOKENSlaunch.py:422So agents size their compaction threshold past what the server can hold and run into
context_length_exceededinstead of compacting in time. That matches the report in #448 that it is "not just that agent and model".Worth noting
FALLBACK_CONTEXT_WINDOW = 128_000(launch.py:24), used when the server reports no context length, is commented "Guessing low only costs earlier compaction." The blind fallback was safer than the number read from the API.Measured on 0.1.3 (
cac247a86), RTX 4070 12GB, Linux:/v1/modelsbefore/v1/modelsafterThe change
kv_pool_geometry(state)— the pool resolution lifted out ofcache_geometry(last rebuild → running snapshot → load-time("meta", …)ack). Extracted rather than duplicated so/v1/modelsand/v1/cache/statuscannot report different capacities for the same server, and so this tracks rebuilds rather than freezing the load-time value._model_context_lengthclamps with it, falling back to the model ceiling whilenum_pagesis still 0 (loading, or an engine build that sends no meta ack). Import is function-local becauseapi_serverimportsopenai_api.On the previous behavior
The old docstring made this call deliberately — "The model ceiling, not
min(ceiling, KV budget): a rebuild moves the latter, and agents read this once at startup." The rebuild concern is real, but the engine already recomputes the clamp on rebuild (engine.py:859), so only an agent's startup copy goes stale — and a stale reachable number seems strictly better than one that was never reachable. Happy to drop this if you disagree; the complete fix probably also re-reads/v1/models(or rewrites the agent config) after a rebuild, which I have not attempted here.Tests
tests/server/test_openai_api.py:445assertedmax_model_len == 262144and passed either way —FakeStatecarries no pool geometry, so it fell through to the ceiling. It was green by accident. It now asserts that fallback explicitly, plus:Verified the new cases fail on
mainand pass with the change:tests/server/+tests/scheduler/test_scheduler_kv_usage.py(the othercache_geometryconsumer): 566 passed.Not covered: I verified against the test suite and stubbed states, not a restarted server end to end, and I have not run the full suite (no GPU-free path for the engine tests on my box).