Update Sage to current OpenAI, Claude, Gemini, Z.AI, and Kimi defaults - #2
Merged
Conversation
Sage still shipped GPT-4, Gemini 1.5, and older Claude defaults across runtime config, provider catalogs, and model discovery. This change moves the built-in defaults to current official OpenAI, Anthropic, Google, Z.AI, and Moonshot/Kimi families and wires Z.AI and Moonshot in as first-class OpenAI-compatible providers so the newer models are actually selectable and configurable. Constraint: Keep the existing provider architecture and reuse the OpenAI-compatible transport for Z.AI and Moonshot Rejected: Expose Z.AI and Kimi only through custom_* providers | users would lose registry, onboarding, and model-select support Confidence: medium Scope-risk: moderate Reversibility: clean Directive: When bumping model families again, update runtime defaults, embedded catalogs, context heuristics, and slash-command model discovery together Tested: cargo test -q -p sage-core -p sage-cli Not-tested: Live requests against vendor APIs
The latest-model refresh changed several match arms and long string lists in provider/config code. This follow-up applies the repository rustfmt style so CI can evaluate the actual behavioral diff instead of failing early on formatting noise. Constraint: CI format gate runs before most validation jobs Rejected: Leave formatting to reviewers or CI reruns | blocks signal from downstream checks Confidence: high Scope-risk: narrow Reversibility: clean Directive: After large provider-catalog edits, run rustfmt before pushing so CI reaches behavioral checks Tested: cargo fmt Not-tested: Full test suite rerun after formatting
![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: 893e07bcad
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+131
to
+133
| ("anthropic", m) | ||
| if m.contains("claude-opus-4") || m.contains("claude-sonnet-4") => | ||
| { |
There was a problem hiding this comment.
The new claude-opus-4/claude-sonnet-4 branch is placed after the existing m.contains("claude-opus") branch, so claude-opus-4-7 never reaches the 1M-token path and is treated as a 200K model instead. With the commit’s new default Anthropic model (claude-opus-4-7), this causes premature context truncation in normal runs; the broader claude-opus rule should be narrowed or moved below the v4-specific rule.
Useful? React with 👍 / 👎.
Comment on lines
+59
to
63
| if model.contains("claude-opus-4") || model.contains("claude-sonnet-4") { | ||
| (1_000_000, 32_000) | ||
| } else if model.contains("3.5") || model.contains("3-5") { | ||
| (200_000, 13_000) // Claude 3.5: 200K context, 13K reserved (like Claude Code) | ||
| } else { |
There was a problem hiding this comment.
This Anthropic branch only special-cases Opus/Sonnet v4 and otherwise falls back to much smaller defaults, so the newly added claude-haiku-4-5 model gets 100K context behavior despite being configured elsewhere as a 200K-context model. That mismatch triggers compaction/truncation earlier than necessary for Haiku 4.5 sessions and can degrade response quality under longer chats.
Useful? React with 👍 / 👎.
The model-update PR surfaced two unrelated repository-wide gates: strict Clippy warnings and a RustSec finding in Cargo.lock. This commit resolves the mechanical lint failures that Clippy reported and updates rustls-webpki in the lockfile so the security audit can move past the current known vulnerability. Constraint: CI runs Clippy and cargo-audit repo-wide, not only on touched modules Rejected: Ignore repo-wide warnings and security findings as unrelated | the PR would remain non-mergeable Confidence: medium Scope-risk: moderate Reversibility: clean Directive: Keep CI gates green before treating provider-model updates as review-ready Tested: cargo fmt Not-tested: Full clippy/audit rerun pending CI
The provider-model update itself was viable, but CI surfaced a mix of repo-wide style debt and one compile regression from the follow-up lint cleanup. This commit finishes the mechanical Clippy cleanups, fixes the background task sort key compile error, and keeps the lockfile on the audited rustls-webpki release. Constraint: Keep the PR mergeable under the repository's existing CI gates Rejected: Scope the PR to model changes only | the branch would stay blocked on existing CI failures Confidence: medium Scope-risk: moderate Reversibility: clean Directive: Treat repo-wide CI failures as part of delivery when a PR cannot advance without them Tested: cargo fmt; cargo check --all-targets --all-features (in progress locally at commit time) Not-tested: Final CI rerun after this follow-up
Automated review caught two real heuristic mismatches in the latest-model update: Opus 4 was still being shadowed by the older claude-opus branch, and Haiku 4.5 was falling back to overly aggressive auto-compact limits. This commit reorders the Anthropic matching rules and gives Haiku 4.5 the same 200K path used elsewhere in the repo. Constraint: Keep context management aligned with the newly selected Anthropic defaults Rejected: Leave heuristics inconsistent and rely on provider-side limits | would still trigger premature truncation/compaction in long sessions Confidence: high Scope-risk: narrow Reversibility: clean Directive: When adding a new model family, verify both ContextConfig and AutoCompactConfig match the intended context window Tested: cargo test -q -p sage-core context::config -- --nocapture; cargo test -q -p sage-core context::auto_compact -- --nocapture Not-tested: End-to-end long-context Anthropic session
majiayu000
added a commit
that referenced
this pull request
May 8, 2026
…oviders Closes #23. Three registries previously hand-maintained the same canonical truth (provider id, base URL, default model): * config/embedded_providers.rs (registry + onboarding UI) * config/provider_defaults.rs (Config::default ModelParameters) * config/provider/defaults.rs (ProviderDefaults::* / for_provider) That triple bookkeeping had drifted. Concrete bugs visible at the moment of this commit: * OpenAI base URL: provider_defaults said `https://api.openai.com`, registry said `/v1`. The non-`/v1` form 404s on `/chat/completions`. * OpenRouter default model: provider_defaults said `anthropic/claude-3.5-sonnet`, registry had been bumped to `anthropic/claude-sonnet-4` by #2 and not propagated. * `zhipu` and `kimi` were full duplicate HashMap entries of `glm` / `moonshot` — the U-24 alias-violation called out in the issue. This commit makes `embedded_providers()` the single source of truth for `id` / `base_url` / `default_model` and derives the other two files from it. Per-provider tuning that's NOT part of the registry contract (temperature / top_p / top_k / parallel_tool_calls / max_retries / api_version / timeouts / rate_limit) stays in the respective files because the registry doesn't model those. * `provider_defaults.rs::create_default_providers` now iterates `embedded_providers()` and applies `provider_tuning(id, model, base_url)` to fill behavioural fields. The `zhipu` / `kimi` duplicate entries are gone. Doubao keeps a hand-maintained entry because it's not (yet) in the registry. * `provider/defaults.rs::ProviderDefaults::*` now calls `with_registry_url(id)` instead of hardcoding base URLs. Aliases `zhipu` / `kimi` are still accepted by `for_provider` and route to the canonical provider's URL — existing user configs keep working. `embedded_providers::embedded_providers()` is widened from `pub(super)` to `pub(crate)` so siblings under `config/` can call it. Drift-guard regression tests: * defaults_cover_every_embedded_provider — the default ModelParameters map covers every registry entry. * defaults_do_not_drift_on_base_url_or_model — for every registry entry with a non-empty URL, the default's base_url matches; if the registry has a default model, the default's `model` matches. * zhipu_and_kimi_aliases_are_not_shipped_as_default_entries — U-24 guard. * doubao_default_is_shipped_even_without_registry_entry — doubao fallback is intact. * registry_base_url_matches_embedded_providers — same drift guard on the ProviderConfig side. * openai_uses_v1_path — pin the specific historical bug. * aliases_route_to_canonical_provider — `for_provider("zhipu")` and `for_provider("kimi")` resolve to the same URL as their canonical sibling. Public API: unchanged. Existing user configs that mention `zhipu` or `kimi` keep validating (validation/provider.rs still accepts them) and keep routing (LlmProvider::from_str still maps them to Glm / Moonshot). What changes is that `Config::default()` no longer ships duplicate HashMap entries for those aliases. Signed-off-by: majiayu000 <1835304752@qq.com>
7 tasks
majiayu000
added a commit
that referenced
this pull request
May 8, 2026
…oviders (#39) Closes #23. Three registries previously hand-maintained the same canonical truth (provider id, base URL, default model): * config/embedded_providers.rs (registry + onboarding UI) * config/provider_defaults.rs (Config::default ModelParameters) * config/provider/defaults.rs (ProviderDefaults::* / for_provider) That triple bookkeeping had drifted. Concrete bugs visible at the moment of this commit: * OpenAI base URL: provider_defaults said `https://api.openai.com`, registry said `/v1`. The non-`/v1` form 404s on `/chat/completions`. * OpenRouter default model: provider_defaults said `anthropic/claude-3.5-sonnet`, registry had been bumped to `anthropic/claude-sonnet-4` by #2 and not propagated. * `zhipu` and `kimi` were full duplicate HashMap entries of `glm` / `moonshot` — the U-24 alias-violation called out in the issue. This commit makes `embedded_providers()` the single source of truth for `id` / `base_url` / `default_model` and derives the other two files from it. Per-provider tuning that's NOT part of the registry contract (temperature / top_p / top_k / parallel_tool_calls / max_retries / api_version / timeouts / rate_limit) stays in the respective files because the registry doesn't model those. * `provider_defaults.rs::create_default_providers` now iterates `embedded_providers()` and applies `provider_tuning(id, model, base_url)` to fill behavioural fields. The `zhipu` / `kimi` duplicate entries are gone. Doubao keeps a hand-maintained entry because it's not (yet) in the registry. * `provider/defaults.rs::ProviderDefaults::*` now calls `with_registry_url(id)` instead of hardcoding base URLs. Aliases `zhipu` / `kimi` are still accepted by `for_provider` and route to the canonical provider's URL — existing user configs keep working. `embedded_providers::embedded_providers()` is widened from `pub(super)` to `pub(crate)` so siblings under `config/` can call it. Drift-guard regression tests: * defaults_cover_every_embedded_provider — the default ModelParameters map covers every registry entry. * defaults_do_not_drift_on_base_url_or_model — for every registry entry with a non-empty URL, the default's base_url matches; if the registry has a default model, the default's `model` matches. * zhipu_and_kimi_aliases_are_not_shipped_as_default_entries — U-24 guard. * doubao_default_is_shipped_even_without_registry_entry — doubao fallback is intact. * registry_base_url_matches_embedded_providers — same drift guard on the ProviderConfig side. * openai_uses_v1_path — pin the specific historical bug. * aliases_route_to_canonical_provider — `for_provider("zhipu")` and `for_provider("kimi")` resolve to the same URL as their canonical sibling. Public API: unchanged. Existing user configs that mention `zhipu` or `kimi` keep validating (validation/provider.rs still accepts them) and keep routing (LlmProvider::from_str still maps them to Glm / Moonshot). What changes is that `Config::default()` no longer ships duplicate HashMap entries for those aliases. Signed-off-by: majiayu000 <1835304752@qq.com>
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.
Summary
Verification
Notes
/modelselection instead of being left as custom-only endpoints