fix(server): return effective context limit from /model-info - #10165
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: 0b477d7039
ℹ️ 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".
|
Noted. Updated the success path to override only when |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a381f03472
ℹ️ 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 let Some(limit) = model_config.context_limit { | ||
| info.context_limit = limit; |
There was a problem hiding this comment.
For Databricks endpoints whose name canonically parses differently from the resolved upstream model, fetch_model_info has already looked up the endpoint and computed info.context_limit from resolved_model, but this branch overwrites it with the locally normalized endpoint-name limit. For example, an endpoint named like kgoose-gpt-4o that actually points at a larger Claude/GPT model will be reported with the gpt-4o limit instead of the upstream model's limit. Fresh evidence: this version avoids the old default fallback case, but line 505 still populates model_config.context_limit from canonical/known metadata rather than only from an explicit user override.
Useful? React with 👍 / 👎.
73a1647 to
e101bd3
Compare
|
CI is green on batched commit e101bd3 (rust fix + EnvVarConfig schema sync). Ready for maintainer review. |
DOsinga
left a comment
DOsinga
left a comment
There was a problem hiding this comment.
LGTM. Clean, minimal fix that does exactly what #10163 asked for: routing /model-info through normalize_model_config and only overriding the fetched context_limit when an explicit override is resolved, so provider-resolved limits (e.g. Databricks) are preserved. Both codex comments are correctly addressed. Thanks!
Use ProviderEntry::normalize_model_config for the effective limit. On fetch_model_info success, override context_limit only when model_config.context_limit is explicitly set (GOOSE_CONTEXT_LIMIT, declarative provider, or canonical registry) so provider-fetched limits (e.g. Databricks upstream resolution) are preserved. Update EnvVarConfig OpenAPI descriptions to match generated schema. Fixes aaif-goose#10163
e101bd3 to
ee1cdb5
Compare
|
@DOsinga Thanks again for the review and approval. Rebased onto latest Whenever you have a moment, this should be ready to merge. Happy to follow up on anything else. |
…mestamp-timezone * origin/main: (34 commits) feat: askai bot hints for followups (aaif-goose#10291) feat(desktop): per-message usage stats UI (tokens, cost, TTFT, tok/s) (aaif-goose#10210) fix(server): return effective context limit from /model-info (aaif-goose#10165) fix(deps): gate rcgen aws_lc_rs feature behind rustls-tls (aaif-goose#10116) Disable thinking preservation for Groq (aaif-goose#10284) Bound code-mode execution with timeout and cancellation (aaif-goose#10214) chore(deps): bump docker/setup-buildx-action from 4.1.0 to 4.2.0 (aaif-goose#10232) chore(deps): bump docker/build-push-action from 7.2.0 to 7.3.0 (aaif-goose#10233) chore(deps): bump docker/metadata-action from 6.1.0 to 6.2.0 (aaif-goose#10235) chore(deps): bump mockall from 0.14.0 to 0.15.0 (aaif-goose#10240) chore(deps): bump uniffi from 0.31.1 to 0.32.0 (aaif-goose#10247) chore(deps): bump sigstore-verify from 0.9.0 to 0.10.0 (aaif-goose#10250) chore(deps): bump cmov from 0.5.3 to 0.5.4 (aaif-goose#10282) chore(deps): bump candle-transformers from 0.10.2 to 0.11.0 (aaif-goose#10242) chore(deps): bump candle-core from 0.10.2 to 0.11.0 (aaif-goose#10241) fix: recover malformed tool calls from GLM/Minimax models instead of rejecting them (aaif-goose#10230) chore(deps): bump jsonschema from 0.30.0 to 0.46.9 (aaif-goose#10244) move google provider into goose-providers (aaif-goose#10216) chore(deps): bump umya-spreadsheet from 2.3.3 to 3.0.0 (aaif-goose#10248) add provider bindings MVP to goose-sdk, and add python wheel publishing (aaif-goose#10208) ...
* main: Update Rust toolchain and raise recursion limit (#10303) Switch the local inference provider MLX backend to use the safemlx crate (#10304) feat: askai bot hints for followups (#10291) feat(desktop): per-message usage stats UI (tokens, cost, TTFT, tok/s) (#10210) fix(server): return effective context limit from /model-info (#10165) fix(deps): gate rcgen aws_lc_rs feature behind rustls-tls (#10116)
2 participants
Summary
/config/providers/{name}/model-infousingProviderEntry::normalize_model_config, the same path the session layer uses.fetch_model_infosuccess path, overridecontext_limitwith the normalized value soGOOSE_CONTEXT_LIMIT, declarative provider limits, and canonical registry precedence are honored.GOOSE_CONTEXT_LIMIToverriding both unknown-model defaults and declarative provider known-model limits.Test plan
cargo fmtcargo test -p goose test_goose_context_limitcargo test -p goose-servercargo clippy -p goose-server -p goose --all-targets -- -D warningsFixes #10163