Disable thinking for tool call labels - #11207
Conversation
…l-thinking # Conflicts: # crates/goose/src/tool_call_labels.rs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 14ec632666
ℹ️ 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 Ok((response, _)) = with_session_id( | ||
| Some(session_id.to_string()), | ||
| provider.complete(model_config, system_prompt, from_ref(message), &[]), | ||
| if let Ok((response, _)) = crate::model_config::complete_fast( |
There was a problem hiding this comment.
Disable local-model thinking for label completions
When the session uses local_inference with its default enable_thinking = true, routing the label through complete_fast still does not disable reasoning. complete_fast only sets thinking_effort=off, while LocalInferenceProvider::stream in crates/goose-local-inference/src/lib.rs:667-677 exclusively checks whether enable_thinking is explicitly false; the registry default remains true (local_model_registry.rs:90-91,140). Consequently these label calls still run the local model's thinking path, so this change does not achieve its latency/token objective for local models; the one-shot config also needs to override enable_thinking.
Useful? React with 👍 / 👎.
…l-thinking # Conflicts: # crates/goose/src/model_config.rs
* origin/main: fix: bound retry command diagnostics (#11365) Disable thinking for tool call labels (#11207) fix(review): contain REVIEW.md discovery (#11367) fix(acp): preserve tool result audience metadata (#11375) test(providers): isolate environment-proxy test in its own binary (#11262) fix(security): bind Foundry API keys to request origin (#11347) fix: canonicalize mangled tool names before permission inspection (follow-up to #10230) (#10285) feat(providers): add Lynkr as a declarative OpenAI-compatible provider (#11372) fix: sanitize Pi imported output (#10990) fix(otel): honor RUST_LOG directives for logs (#11360) feat(skills): add web-search and browser-use built-in skills (#11233) feat(dictation): add model-native audio transcription provider (#10589) feat(aws_bedrock): route OpenAI GPT-5.6 (sol/terra/luna) via Bedrock … (#10502) refactor(goose-local-inference): move mlx deps under macos (#11328) feat(providers): add PleumRouter declarative provider (#10479)
* origin/main: (50 commits) chore(deps): bump the ui-minor-and-patch group across 1 directory with 53 updates (#11386) fix(security): bound call graph traversal (#11193) fix: pin arrayref to known-good commit (#11389) feat(providers): add SayGM as declarative OpenAI-compatible provider (#11267) feat(providers): custom provider cost fields drive cost tracking (config-declared pricing fallback) (#11220) fix(deps): repair dangling syn reference in Cargo.lock (#11385) fix(flake): add cudaforge hash for git dependency (#10910) feat: auto-focus chat input when user starts typing (#11184) fix(security): fail closed on invalid Codex ACP mode (#11362) fix(mcp): keep stdio extensions alive across worker exits (#10364) feat(ui): collapse scheduled job sessions into accordion in chat history (#11265) fix: bound retry command diagnostics (#11365) Disable thinking for tool call labels (#11207) fix(review): contain REVIEW.md discovery (#11367) fix(acp): preserve tool result audience metadata (#11375) test(providers): isolate environment-proxy test in its own binary (#11262) fix(security): bind Foundry API keys to request origin (#11347) fix: canonicalize mangled tool names before permission inspection (follow-up to #10230) (#10285) feat(providers): add Lynkr as a declarative OpenAI-compatible provider (#11372) fix: sanitize Pi imported output (#10990) ... # Conflicts: # crates/goose/src/agents/state_machine/tests/hooks_lifecycle.rs
* origin/main: (59 commits) chore(deps): bump the ui-minor-and-patch group across 1 directory with 53 updates (#11386) fix(security): bound call graph traversal (#11193) fix: pin arrayref to known-good commit (#11389) feat(providers): add SayGM as declarative OpenAI-compatible provider (#11267) feat(providers): custom provider cost fields drive cost tracking (config-declared pricing fallback) (#11220) fix(deps): repair dangling syn reference in Cargo.lock (#11385) fix(flake): add cudaforge hash for git dependency (#10910) feat: auto-focus chat input when user starts typing (#11184) fix(security): fail closed on invalid Codex ACP mode (#11362) fix(mcp): keep stdio extensions alive across worker exits (#10364) feat(ui): collapse scheduled job sessions into accordion in chat history (#11265) fix: bound retry command diagnostics (#11365) Disable thinking for tool call labels (#11207) fix(review): contain REVIEW.md discovery (#11367) fix(acp): preserve tool result audience metadata (#11375) test(providers): isolate environment-proxy test in its own binary (#11262) fix(security): bind Foundry API keys to request origin (#11347) fix: canonicalize mangled tool names before permission inspection (follow-up to #10230) (#10285) feat(providers): add Lynkr as a declarative OpenAI-compatible provider (#11372) fix: sanitize Pi imported output (#10990) ...
Fixes #11154
What changed
complete_fastcomplete_fastcallerWhy
Tool-call labels used
get_fast_modelfollowed byprovider.completedirectly. That bypassed the thinking-off behavior incomplete_fast, so reasoning-capable models could spend tokens deliberating over short labels whose reasoning output was discarded.Verification
cargo fmtgit diff --checkcargo test -p goose tool_call_labels(18 passed)