Groq configure fix
Summary
Fixed goose configure command for Groq (d) provider. Previously, if the groq_api_key was unset, the cli would not ask for it and error out at the end of the setup. That said, I still think there are some issues with how goose handles Groq's conversations, with all of them being stuck on "Context limit reached. Compacting to continue conversation..."
Type of Change
- [ ] Feature
- [x] Bug fix
- [ ] Refactor / Code quality
- [ ] Performance improvement
- [ ] Documentation
- [ ] Tests
- [ ] Security fix
- [ ] Build / Release
- [ ] Other (specify below)
AI Assistance
- [x] This PR was created or reviewed with AI assistance Made with goose :P
Testing
Manual testing + the following:
cargo test -p goose --lib providers::factory::tests
cargo test -p goose --lib providers::formats::openai::tests
cargo test -p goose --lib providers::factory::tests::test_groq_provider_config_keys (per commit)
Screenshots/Demos (for UX changes)
Previously:
Now
thanks @neiii - just some nits (other comments) to fix, but also - can you explain more why it needs to set specifically the first key (ie overwrite whatever was there in the config), that seems odd as it would effect config for other providers too?
you're right, this is a rather silly implementation, not sure how I overlooked this. I'm currently debugging another implementation, by the looks of it, Groq is not the only affected provider; any openAI compatible provider is affected, including deepseek, and mistral. I'll update in a bit
thanks @neiii - let's get groq work here completely. I agree with @michaelneale that blindly copying the key into the first parameter might have side effects though. a better but more involved solution would be to move the api_key out of the config_keys in ProviderMetaData and just have it there as an explicit field.
hey @DOsinga, I pushed a different implementation just as you were writing the message. Now, it gets the position of the OPENAI_API_KEY and then swaps it with the provider's API key. Looks good or would you like more change?
In terms of getting Groq fixed completely, quite frankly I'm not sure what's causing the issue, I've went back-and-forth with it for 2 hours yesterday, I'll tinker some more tonight. The requests go through (and it shows on Groq's dashboard), it's just that when Goose is processing them for some reason it sees it as a context overflow and tries to infinitely compress it, causing the requests to spam. I'll tinker some more tonight
ok, thanks for the info, very useful. replacing OPEN_AI_KEY is a good start and helps for this for sure, but custom providers based on Anthropic (not very common to be sure) will run into the same issue. we can leave your solution in place though
as for groq, the reason this happens can be either that we have the token count on the models they return wrong so we think we're immediately out of context tokens, or there is something in the reply like "too many" and it is a 500 http error. If you can share a diagnostics report that could be helpful
@DOsinga I checked again with Groq now on a paid plan, the (bigger context) models seem to work, I suppose it was a tier issue (?). Is there anything else blocking for this change to be implemented?
@DOsinga compressed the tests and re-added the comment. Anything else?