Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors streaming functionality across OpenAI-compatible providers by extracting common code into a reusable utility function and adds streaming support to the xAI provider.
Key changes:
- Introduced
stream_openai_compatutility function to eliminate duplicate streaming code across providers - Added
for_streamingparameter tocreate_requestto declaratively configure streaming options - Implemented streaming support for xAI provider
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/goose/src/providers/utils.rs | Added stream_openai_compat utility function to handle OpenAI-compatible streaming responses |
| crates/goose/src/providers/formats/openai.rs | Added for_streaming parameter to create_request to automatically configure streaming options; simplified tools spec creation |
| crates/goose/src/providers/xai.rs | Added streaming support by implementing supports_streaming and stream methods |
| crates/goose/src/providers/openai.rs | Refactored to use stream_openai_compat utility, reducing code duplication |
| crates/goose/src/providers/ollama.rs | Refactored to use stream_openai_compat utility, reducing code duplication |
| crates/goose/src/providers/tetrate.rs | Refactored to use stream_openai_compat utility, reducing code duplication |
| crates/goose/src/providers/databricks.rs | Refactored to use stream_openai_compat utility, reducing code duplication |
| crates/goose/src/providers/azure.rs | Updated create_request call to include new for_streaming parameter |
| crates/goose/src/providers/githubcopilot.rs | Updated create_request call to include new for_streaming parameter |
| crates/goose/src/providers/litellm.rs | Updated create_request call to include new for_streaming parameter |
| crates/goose/src/providers/openrouter.rs | Updated create_request call to include new for_streaming parameter |
| crates/goose/src/providers/toolshim.rs | Updated create_request call to include new for_streaming parameter |
| if for_streaming { | ||
| payload["stream"] = json!(true); | ||
| payload["stream_options"] = json!({"include_usage": true}); | ||
| } |
There was a problem hiding this comment.
The new for_streaming parameter lacks test coverage. Consider adding a test that verifies create_request(..., true) correctly adds the stream: true and stream_options: {"include_usage": true} fields to the payload.
crates/goose/src/providers/openai.rs
Outdated
| messages, | ||
| tools, | ||
| &ImageFormat::OpenAi, | ||
| false, |
There was a problem hiding this comment.
I wish the rules didn't make this so vertical... ugh...
|
code looks good - main worry and maybe need some manual testing, making sure we are accounting for tokens correctly (I know we had past issues with that with anthropic, but probably long ago) |
|
@DOsinga still in progress? |
| } else { | ||
| vec![] | ||
| }; | ||
| let mut tools_spec = format_tools(tools)?; |
There was a problem hiding this comment.
The conditional check for empty tools was removed, causing format_tools to be called even when tools is empty. This creates unnecessary work and validation. Restore the empty check to skip processing when there are no tools.
| let mut tools_spec = format_tools(tools)?; | ||
|
|
||
| // Validate tool schemas | ||
| validate_tool_schemas(&mut tools_spec); |
There was a problem hiding this comment.
This validation runs even when tools_spec is empty, performing unnecessary work. Move this inside the if !tools_spec.is_empty() block at line 672.
* 'main' of github.com:block/goose: (22 commits) OpenRouter & Xai streaming (#5873) fix: resolve mcp-hermit cleanup path expansion issue (#5953) feat: add goose PR reviewer workflow (#6124) perf: Avoid repeated MCP queries during streaming responses (#6138) Fix YAML serialization for recipes with special characters (#5796) Add more posthog analytics (privacy aware) (#6122) docs: add Sugar MCP server to extensions registry (#6077) Fix tokenState loading on new sessions (#6129) bump bedrock dep versions (#6090) Don't persist ephemeral extensions when resuming sessions (#5974) chore(deps): bump mdast-util-to-hast from 13.2.0 to 13.2.1 in /ui/desktop (#5939) chore(deps): bump node-forge from 1.3.1 to 1.3.2 in /documentation (#5898) Add Scorecard supply-chain security workflow (#5810) Don't show subagent tool when we're a subagent (#6125) Fix keyboard shortcut conflict for Focus Goose Window (#5809) feat(goose-cli): add feature to disable update (#5886) workflow: enable docs-update-recipe-ref (#6132) fix: filter tools in Ollama streaming when chat mode is enabled (#6118) feat(mcp): platform extension for "code mode" MCP tool calling (#6030) workflow: auto-update recipe-reference on release (#5988) ... # Conflicts: # ui/desktop/src/App.tsx # ui/desktop/src/api/sdk.gen.ts # ui/desktop/src/components/ChatInput.tsx # ui/desktop/src/components/recipes/RecipesView.tsx
…s-predefined-models * 'main' of github.com:block/goose: (81 commits) fix: display shell output as static text instead of spinner (#6041) fix : Custom providers with empty API keys show as configured in desktop (#6105) Add .agents/skills and ~/.config/agent/skills to skills discovery paths (#6139) fix: use instructions for system prompt and prompt for user message in subagents (#6121) Fix compaction loop for small models or large input (#5803) feat: Centralize theme management with ThemeContext (#6137) OpenRouter & Xai streaming (#5873) fix: resolve mcp-hermit cleanup path expansion issue (#5953) feat: add goose PR reviewer workflow (#6124) perf: Avoid repeated MCP queries during streaming responses (#6138) Fix YAML serialization for recipes with special characters (#5796) Add more posthog analytics (privacy aware) (#6122) docs: add Sugar MCP server to extensions registry (#6077) Fix tokenState loading on new sessions (#6129) bump bedrock dep versions (#6090) Don't persist ephemeral extensions when resuming sessions (#5974) chore(deps): bump mdast-util-to-hast from 13.2.0 to 13.2.1 in /ui/desktop (#5939) chore(deps): bump node-forge from 1.3.1 to 1.3.2 in /documentation (#5898) Add Scorecard supply-chain security workflow (#5810) Don't show subagent tool when we're a subagent (#6125) ... # Conflicts: # crates/goose/src/providers/formats/databricks.rs
* main: fix: we don't need to warn about tool count when in code mode (#6149) deps: upgrade agent-client-protocol to 0.9.0 (#6109) fix(providers): fix for gemini-cli on windows to work around cmd's multiline prompt limitations #5911 (#5966) More slash commands (#5858) fix: MCP UI not rendering due to CallToolResult structure change (#6143) fix: display shell output as static text instead of spinner (#6041) fix : Custom providers with empty API keys show as configured in desktop (#6105) Add .agents/skills and ~/.config/agent/skills to skills discovery paths (#6139) fix: use instructions for system prompt and prompt for user message in subagents (#6121) Fix compaction loop for small models or large input (#5803) feat: Centralize theme management with ThemeContext (#6137) OpenRouter & Xai streaming (#5873) fix: resolve mcp-hermit cleanup path expansion issue (#5953) feat: add goose PR reviewer workflow (#6124) perf: Avoid repeated MCP queries during streaming responses (#6138) Fix YAML serialization for recipes with special characters (#5796) Add more posthog analytics (privacy aware) (#6122) docs: add Sugar MCP server to extensions registry (#6077)
* origin/main: (57 commits) docs: create/edit recipe button (#6145) fix(google): Fix 400 Bad Request error with Gemini 3 thought signatures (#6035) fix: we don't need to warn about tool count when in code mode (#6149) deps: upgrade agent-client-protocol to 0.9.0 (#6109) fix(providers): fix for gemini-cli on windows to work around cmd's multiline prompt limitations #5911 (#5966) More slash commands (#5858) fix: MCP UI not rendering due to CallToolResult structure change (#6143) fix: display shell output as static text instead of spinner (#6041) fix : Custom providers with empty API keys show as configured in desktop (#6105) Add .agents/skills and ~/.config/agent/skills to skills discovery paths (#6139) fix: use instructions for system prompt and prompt for user message in subagents (#6121) Fix compaction loop for small models or large input (#5803) feat: Centralize theme management with ThemeContext (#6137) OpenRouter & Xai streaming (#5873) fix: resolve mcp-hermit cleanup path expansion issue (#5953) feat: add goose PR reviewer workflow (#6124) perf: Avoid repeated MCP queries during streaming responses (#6138) Fix YAML serialization for recipes with special characters (#5796) Add more posthog analytics (privacy aware) (#6122) docs: add Sugar MCP server to extensions registry (#6077) ...
…icing * 'main' of github.com:block/goose: (35 commits) docs: skills (#6062) fix: add conditional configuration for GOOSE_BIN_DIR in PATH (#5940) Update dependencies to help in Fedora packaging (#5835) fix: make goose reviewer less bad (#6154) docs: create/edit recipe button (#6145) fix(google): Fix 400 Bad Request error with Gemini 3 thought signatures (#6035) fix: we don't need to warn about tool count when in code mode (#6149) deps: upgrade agent-client-protocol to 0.9.0 (#6109) fix(providers): fix for gemini-cli on windows to work around cmd's multiline prompt limitations #5911 (#5966) More slash commands (#5858) fix: MCP UI not rendering due to CallToolResult structure change (#6143) fix: display shell output as static text instead of spinner (#6041) fix : Custom providers with empty API keys show as configured in desktop (#6105) Add .agents/skills and ~/.config/agent/skills to skills discovery paths (#6139) fix: use instructions for system prompt and prompt for user message in subagents (#6121) Fix compaction loop for small models or large input (#5803) feat: Centralize theme management with ThemeContext (#6137) OpenRouter & Xai streaming (#5873) fix: resolve mcp-hermit cleanup path expansion issue (#5953) feat: add goose PR reviewer workflow (#6124) ...
Summary
Remove some duplication on how we do streaming and also add it to xai