-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Feat: Let providers configure a fast model for summarization #4228
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
crates/goose/src/providers/openai.rs
Outdated
| let model = get_model(&json_response); | ||
| emit_debug_trace(model_config, &payload, &json_response, &usage); | ||
| Ok((message, ProviderUsage::new(model, usage))) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was previously existing, but now that we pass in the model, no need to fish it out of the json response
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got rid of it, but confused by this one since we already had the model name from before the request?
crates/goose/src/providers/openai.rs
Outdated
| ) -> Result<(Message, ProviderUsage), ProviderError> { | ||
| // Use the fast model (gpt-4o-mini) for fast completions | ||
| let mut fast_config = self.model.clone(); | ||
| fast_config.model_name = OPEN_AI_FAST_MODEL.to_string(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make this a method on config - config.for_fast_model() should return a copy of itself with the model_name swapped out for the smart model if that is available
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| config: CustomProviderConfig, | ||
| ) -> Result<Self> { | ||
| // Set the default fast model for Anthropic | ||
| model.fast_model = Some("claude-3-5-haiku-latest".to_string()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't want to do this - custom_config means it is a provider that speaks anthropic but isn't. they will not have this model
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack. Done + applied to openAI/google.
| pub fn from_env(model: ModelConfig) -> Result<Self> { | ||
| pub fn from_env(mut model: ModelConfig) -> Result<Self> { | ||
| // Set the default fast model for Anthropic | ||
| model.fast_model = Some("claude-3-5-haiku-latest".to_string()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make that into a constant on top - ideally we'd do this in a more elegant way, but let's do that later. don't make this mutable though, but have something like model = model.with_fast(CONSTANT) - possibly also check whether that is already set and don't overwrite it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done + applied to openAI/google
crates/goose/src/providers/base.rs
Outdated
| /// | ||
| /// # Errors | ||
| /// ProviderError | ||
| /// - It's important to raise ContextLengthExceeded correctly since agent handles it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what are these comments? they repeat the existing comments which are now out of date demonstrating why we shouldn't have them in the first place
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
crates/goose/src/providers/base.rs
Outdated
| /// - It's important to raise ContextLengthExceeded correctly since agent handles it | ||
| async fn complete_with_model( | ||
| &self, | ||
| model: &str, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be modelConfig, not model. if you pass a string but then you need model config, you're going to create one yourself
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| async fn complete( | ||
| async fn complete_with_model( | ||
| &self, | ||
| _model: &str, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now we are lying
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
* 'main' of github.com:block/goose: chore: upgrade rmcp to 0.6.0 (#4243) doc: uvx not npx (#4240) Add PKCE support for Tetrate Agent Router Service (#4165) Read AGENTS.md by default (#4232) docs: configure provider and model (#4235) docs: add figma tutorial (#4231) Add Nix flake for reproducible builds (#4213) Enhanced onboarding page visual design (#4156) feat: adds mtls to all providers (#2794) (#2799) Don't show a confirm dialog for quitting (#4225) Fix: Missing smart_approve in CLI /mode help text and error message (#4132)
* main: (108 commits) Remove unused game (#4226) fix issue where app redirects to home after initialization but user has already started a chat (#4260) Feat: Let providers configure a fast model for summarization (#4228) docs: update tool selection strategy (#4258) feat: upgrade `@mcp-ui/client` package and improve UI message handling (#4164) stop replacing chat window when changing working directory (#4200) Only fetch session tokens when chat state is idle to avoid resetting during streaming (#4104) bump timeouts for e2e tests (#4251) docs: custom context files improvements (#4096) chore: upgrade rmcp to 0.6.0 (#4243) doc: uvx not npx (#4240) Add PKCE support for Tetrate Agent Router Service (#4165) Read AGENTS.md by default (#4232) docs: configure provider and model (#4235) docs: add figma tutorial (#4231) Add Nix flake for reproducible builds (#4213) Enhanced onboarding page visual design (#4156) feat: adds mtls to all providers (#2794) (#2799) Don't show a confirm dialog for quitting (#4225) Fix: Missing smart_approve in CLI /mode help text and error message (#4132) ...
* main: docs: update View/Edit Recipe menu item name (#4267) Remove unused game (#4226) fix issue where app redirects to home after initialization but user has already started a chat (#4260) Feat: Let providers configure a fast model for summarization (#4228) docs: update tool selection strategy (#4258) feat: upgrade `@mcp-ui/client` package and improve UI message handling (#4164) stop replacing chat window when changing working directory (#4200) Only fetch session tokens when chat state is idle to avoid resetting during streaming (#4104) bump timeouts for e2e tests (#4251) docs: custom context files improvements (#4096) chore: upgrade rmcp to 0.6.0 (#4243) doc: uvx not npx (#4240) Add PKCE support for Tetrate Agent Router Service (#4165) Read AGENTS.md by default (#4232) docs: configure provider and model (#4235)
* main: (42 commits) feat: Add message queue system with interruption handling (#4179) Start extensions concurrently (#4234) Add X-Title and referer headers on exchange to tetrate (#4250) docs: update View/Edit Recipe menu item name (#4267) Remove unused game (#4226) fix issue where app redirects to home after initialization but user has already started a chat (#4260) Feat: Let providers configure a fast model for summarization (#4228) docs: update tool selection strategy (#4258) feat: upgrade `@mcp-ui/client` package and improve UI message handling (#4164) stop replacing chat window when changing working directory (#4200) Only fetch session tokens when chat state is idle to avoid resetting during streaming (#4104) bump timeouts for e2e tests (#4251) docs: custom context files improvements (#4096) chore: upgrade rmcp to 0.6.0 (#4243) doc: uvx not npx (#4240) Add PKCE support for Tetrate Agent Router Service (#4165) Read AGENTS.md by default (#4232) docs: configure provider and model (#4235) docs: add figma tutorial (#4231) Add Nix flake for reproducible builds (#4213) ...
No description provided.