feat(goose): honor GOOSE_FAST_MODEL env var in ModelConfig::with_fast - #9296
Conversation
DOsinga
left a comment
There was a problem hiding this comment.
Pushed a cleanup commit: dropped the verbose comment, simplified the trim/filter chain (avoids unnecessary allocation), removed the three trivial tests, and added GOOSE_FAST_MODEL to the environment variables documentation.
One thing left: your original commit is missing the DCO sign-off (Signed-off-by line). The DCO check will fail without it. You can fix this with:
git commit --amend -s
git push --force-with-lease
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d02655204e
ℹ️ 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".
| let name = std::env::var("GOOSE_FAST_MODEL") | ||
| .ok() | ||
| .filter(|v| !v.trim().is_empty()) | ||
| .unwrap_or_else(|| fast_model_name.to_string()); |
There was a problem hiding this comment.
Trim GOOSE_FAST_MODEL before constructing fast model config
The new override path checks v.trim().is_empty() but then passes the untrimmed String into ModelConfig::new, so values like " gpt-4o-mini " are treated as non-empty yet produce a model name with embedded whitespace. In that case auxiliary fast-model calls can fail with an invalid model identifier even though the intended model is correct; trimming the value before ModelConfig::new would make this override behave consistently with the emptiness check.
Useful? React with 👍 / 👎.
Each provider's `from_env()` calls `ModelConfig::with_fast(...)` with a hardcoded per-provider fast-model constant (e.g. `OPENROUTER_DEFAULT_FAST_MODEL = "google/gemini-2.5-flash"`, `OPEN_AI_DEFAULT_FAST_MODEL = "gpt-4o-mini"`). `use_fast_model()` is invoked by Goose internals for auxiliary calls — tool-selection, classification, session-title generation — independent of `GOOSE_MODEL`, so users have no way to redirect that traffic to a model of their choice. This adds a `GOOSE_FAST_MODEL` env var honored at the single call site inside `with_fast`. When set and non-empty it replaces the provider default; when unset/empty behavior is unchanged. Applies to all 8 providers that call `with_fast` (openrouter, openai, anthropic, google, gemini_oauth, ollama, databricks, kimicode) without per-provider edits, mirroring the existing `GOOSE_MODEL` env var pattern. Closes aaif-goose#9295 Signed-off-by: Vladislav Dobromyslov <vladik.dobrik@gmail.com>
Signed-off-by: Douwe Osinga <douwe@squareup.com>
d026552 to
86ceab2
Compare
The previous override path checked `!v.trim().is_empty()` in the filter but then passed the untrimmed `String` into `ModelConfig::new`. Values like `" gpt-4o-mini "` survived the emptiness check yet produced a model name with embedded whitespace, which downstream APIs reject as an invalid model identifier — a hard-to-debug failure mode when a user has a stray space in their env var. Move the trim into a `.map(...)` step so the emptiness check and the consumed value see the same trimmed string. Addresses chatgpt-codex-connector review on aaif-goose#9296. Signed-off-by: Vladislav Dobromyslov <vladik.dobrik@gmail.com>
…194) Goose hardcodes google/gemini-2.5-flash as the OpenRouter auxiliary "fast model" (tool-selection/classification calls), which bled spend independent of GOOSE_MODEL. aaif-goose/goose#9296 added a GOOSE_FAST_MODEL override (ModelConfig::with_fast). Wire it to deepseek/deepseek-v4-flash across codex-security-fix, codex-semver-fix, and dependency-review. Inert on the pinned Goose 1.34.0 (the override is not in any release yet — merged after v1.35.0 was cut); auto-activates once the setup-goose-action version is bumped to a release containing #9296.
…aaif-goose#9296) Signed-off-by: Vladislav Dobromyslov <vladik.dobrik@gmail.com> Signed-off-by: Douwe Osinga <douwe@squareup.com> Co-authored-by: Douwe Osinga <douwe@squareup.com>
Closes #9295.
Summary
Adds
GOOSE_FAST_MODELenv var, honored at the single call site insideModelConfig::with_fastincrates/goose/src/model.rs. When set and non-empty it replaces the provider-supplied fast-model default; when unset/empty (or whitespace-only) behavior is unchanged.Mirrors the existing
GOOSE_MODELenv-var pattern for the primary model.Motivation
Each provider's
from_env()callswith_fast(...)with a hardcoded constant (e.g.OPENROUTER_DEFAULT_FAST_MODEL = "google/gemini-2.5-flash",OPEN_AI_DEFAULT_FAST_MODEL = "gpt-4o-mini").use_fast_model()is invoked by Goose internals for auxiliary calls (tool-selection, classification, session-title generation) independent ofGOOSE_MODEL— so today users have no way to redirect that traffic.Concrete impact: cost-controlled deployments that intentionally pin a single model on OpenRouter still bleed spend on Gemini 2.5 Flash because the auxiliary path is unreachable from config. Privacy/compliance setups have the same gap. Closely related to the closed #4350 / #4318, which addressed canonical lookups but left the override gap in place.
Design
Single point of override at the existing
with_fastcall site means all 8 providers (openrouter,openai,anthropic,google,gemini_oauth,ollama,databricks,kimicode) pick it up via existing call sites without per-provider edits.The trim+filter handles both unset and accidentally-empty/whitespace values so a stray
GOOSE_FAST_MODEL=in.envdoesn't break the auxiliary path.Tests
Three unit tests in the existing
model::testsmodule using the sameenv_lockpattern that coversGOOSE_MAX_TOKENS/GOOSE_TEMPERATURE:test_with_fast_uses_default_when_env_unset— preserves existing behaviortest_with_fast_honors_env_override— env var wins over provider defaulttest_with_fast_ignores_empty_env— whitespace-only value is treated as unsetcargo fmt -p goose --checkandcargo clippy -p goose --lib --no-deps -- -D warningsboth clean.Backwards compatibility
No behavior change when
GOOSE_FAST_MODELis unset or empty. No new dependencies. No public API change beyond a new env var honored by an existing method.Out of scope (follow-ups, happy to address separately)
ModelConfigfor non-env-var users — chose env-var-only for this PR per CONTRIBUTING.md "start small" guidance.OPENROUTER_FAST_MODEL, etc.) — 8x the surface area for the same outcome when users typically run one provider per session.GOOSE_DISABLE_FAST_MODEL=1flag to skip the fast-model optimization entirely.Checklist
cargo buildcargo test-D warningscargo fmt --checkclean