fix(litellm): use context limit from /model/info for custom models - #9303
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17424a49bb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // from the LiteLLM /model/info endpoint so custom/local models get the | ||
| // correct limit instead of silently defaulting to 128k. | ||
| if provider.model.context_limit.is_none() { | ||
| if let Ok(models) = provider.fetch_models().await { |
There was a problem hiding this comment.
This new eager call to fetch_models().await runs during provider construction whenever context_limit is unset, so environments where /model/info is slow, disabled, or permission-blocked will stall initialization until the full request timeout elapses (default is 600 seconds via LITELLM_TIMEOUT). Because the error is ignored after awaiting, startup can hang for minutes and then silently continue, which is a user-visible regression for custom LiteLLM deployments where chat completions work but model metadata does not.
Useful? React with 👍 / 👎.
3e05628 to
727f257
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 727f257742
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if config.context_limit.is_none() { | ||
| if let Some(models) = self.cached_model_info.get() { | ||
| if let Some(info) = models.iter().find(|m| m.name == config.model_name) { |
There was a problem hiding this comment.
get_model_config() now only reads cached_model_info and never initiates a fetch when context_limit is None, so on a fresh provider instance the first request still uses the 128k default until some other path warms the cache. In the normal chat flow, provider.get_model_config() is consumed before supports_cache_control() performs any fetch, so custom LiteLLM models with smaller windows can still hit context-overflow errors on initial turns instead of compacting to the real limit.
Useful? React with 👍 / 👎.
| if let Some(info) = models.iter().find(|m| m.name == config.model_name) { | ||
| config.context_limit = Some(info.context_limit); | ||
| } |
There was a problem hiding this comment.
This assignment accepts info.context_limit verbatim, including 0, which can happen when upstream metadata is unset/sentinel-valued; once stored, downstream context_limit() calls treat it as authoritative and can break compaction math or force pathological behavior. The provider should ignore non-positive limits here (consistent with other model-limit normalization paths) and keep the fallback/default instead.
Useful? React with 👍 / 👎.
When a model isn't in the canonical database (e.g. local/custom models behind a LiteLLM proxy), context_limit was left as None and silently defaulted to 128k. This caused context overflow on smaller models and broke context compaction. Cache the /model/info response in a OnceCell, populated lazily on first use (supports_cache_control during stream, or fetch_supported_models). get_model_config() reads the cache synchronously — no blocking at init, no extra HTTP calls, and the context limit is available before any compaction decision since the first stream() call populates the cache. Zero-value context limits from upstream metadata are ignored. Also fixes the pre-existing issue where supports_cache_control and fetch_supported_models each made separate HTTP calls to /model/info on every invocation. Fixes #8835 Signed-off-by: Douwe Osinga <douwe@squareup.com>
727f257 to
346b7f6
Compare
|
Addressing Codex review feedback: P1 (blocking init): Already addressed in the previous push — replaced the eager P1 (get_model_config not triggering fetch): By design — P2 (zero context limits): Fixed — now skips non-positive context limits from upstream metadata. |
…aif-goose#9303) Signed-off-by: Douwe Osinga <douwe@squareup.com> Co-authored-by: Douwe Osinga <douwe@squareup.com>
3 participants
When a model isn't in the canonical database (e.g. local/custom models behind a LiteLLM proxy),
context_limitwas left asNoneand silently defaulted to 128k. This caused context overflow on smaller models and broke context compaction.Cache the
/model/inforesponse in aOnceCell, populated lazily on first use (supports_cache_controlduringstream(), orfetch_supported_models).get_model_config()reads the cache synchronously — no blocking at init, no extra HTTP calls, and the context limit is available before any compaction decision since the firststream()call populates the cache.Also fixes the pre-existing issue where
supports_cache_controlandfetch_supported_modelseach made separate HTTP calls to/model/infoon every invocation.Fixes #8835