-
Notifications
You must be signed in to change notification settings - Fork 617
fix groq and cerebras clients #672
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
|
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.
PR Summary
Added consistent response formatting and error handling for Groq and Cerebras LLM clients to ensure proper data shape and usage metrics.
- Added structured response format
{ data: result, usage: response.usage }
in both clients - Improved JSON parsing from tool calls and content with proper error handling
- Added caching support for both raw and formatted responses
- Enhanced retry mechanism (up to 5 attempts) for failed JSON parsing
- Added comprehensive logging for response handling and errors
2 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile
if (content) { | ||
try { | ||
// Try to extract JSON from the content | ||
const jsonMatch = content.match(/\{[\s\S]*\}/); |
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.
logic: Regex pattern /{[\s\S]*}/ may match nested JSON objects incorrectly, leading to invalid parsing
const jsonMatch = content.match(/\{[\s\S]*\}/); | ||
if (jsonMatch) { |
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.
logic: regex pattern /{[\s\S]*}/ could match nested JSON objects incorrectly or match the first {} it finds instead of the complete JSON object
why
what changed
test plan