Conversation
- Ignore errors when determining if the config file exists before creating it - Add support for returning detailed prompt token usage in OpenAI responses - Use MaxCompletionTokens instead of MaxTokens when specifying completion limits - Remove redundant handling for O-series OpenAI models in completion requests Signed-off-by: appleboy <[email protected]>
WalkthroughThe updates revise error handling in configuration file existence checks within the command logic and enhance OpenAI provider integration by updating token usage reporting and standardizing the use of the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @appleboy, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
I've focused on refining the interaction with the OpenAI API and improving configuration file handling. This pull request introduces more detailed token usage reporting, standardizes the use of MaxCompletionTokens for setting completion limits, and streamlines config file creation by ignoring non-critical errors during the existence check. The goal is to make the API integration more robust and provide better insights into token consumption.
Highlights
- Configuration File Handling: Improved robustness when creating the configuration file by ignoring errors during the initial check for file existence, allowing for smoother setup.
- OpenAI Token Usage Reporting: Enhanced OpenAI API responses to include detailed prompt token usage, providing more granular insights into API consumption and cost analysis.
- OpenAI Completion Limits: Standardized the use of
MaxCompletionTokensinstead ofMaxTokensfor specifying completion limits in OpenAI requests, aligning with current API best practices and ensuring correct token management. - OpenAI Model Handling Simplification: Removed redundant conditional logic specifically for O-series OpenAI models, simplifying the codebase and potentially improving maintainability and clarity.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces several improvements. It updates the OpenAI integration to use MaxCompletionTokens instead of MaxTokens, removes redundant logic for specific models, and adds support for detailed prompt token usage in responses. These changes align with recent API updates and enhance reporting capabilities.
Additionally, the logic for handling config file creation is modified to ignore errors when checking for the file's existence. While this simplifies the code, I've provided a suggestion in cmd/cmd.go to log these errors instead of swallowing them completely. This will improve the application's observability and make it easier to diagnose potential file system issues without altering the core logic.
| // Config file status could not be determined; handle or ignore as needed | ||
| // Optionally: log.Fatalf("failed to check if config file %s is a file: %v", cfgFile, err) | ||
| } | ||
| exists, _ := file.IsFile(cfgFile) |
There was a problem hiding this comment.
Ignoring errors by using the blank identifier _ can hide underlying problems, such as permission issues, that might prevent the application from working correctly. While the subsequent os.Create might also fail and report an error, it's a Go best practice to handle or at least log errors to provide more context for debugging when things go wrong.
exists, err := file.IsFile(cfgFile)
if err != nil {
log.Printf("warning: could not check for config file %s: %v", cfgFile, err)
}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
cmd/cmd.go (1)
74-81: Don’t ignore IsFile errors; ensure parent dir exists; close the created fileIgnoring the error from file.IsFile can hide permission/path issues. Also, os.Create returns a file handle that isn’t closed, leaking FDs. Finally, creating a user-specified cfgFile should ensure its parent directory exists.
Apply:
- viper.SetConfigFile(cfgFile) - exists, _ := file.IsFile(cfgFile) - if !exists { - // Config file not found; ignore error if desired - _, err := os.Create(cfgFile) - if err != nil { - log.Fatal(err) - } - } + viper.SetConfigFile(cfgFile) + exists, err := file.IsFile(cfgFile) + if err != nil && !os.IsNotExist(err) { + log.Fatalf("failed to stat config file %s: %v", cfgFile, err) + } + if !exists { + // Ensure parent directory exists for user-provided cfgFile + if err := os.MkdirAll(filepath.Dir(cfgFile), os.ModePerm); err != nil { + log.Fatalf("failed to create config directory for %s: %v", cfgFile, err) + } + f, err := os.Create(cfgFile) + if err != nil { + log.Fatal(err) + } + defer f.Close() + }And add the missing import:
import ( "context" "fmt" "log" "os" "path" + "path/filepath" "strings"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cmd/cmd.go(1 hunks)provider/openai/openai.go(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: windows-latest @ Go stable
- GitHub Check: Analyze (go)
- GitHub Check: windows-latest @ Go stable
🔇 Additional comments (2)
provider/openai/openai.go (2)
45-53: Include prompt token details in usage: verifiedForwarding PromptTokensDetails significantly improves observability of prompt-side costs.
Confirmed that core.Usage in core/openai.go defines:
- PromptTokensDetails *openai.PromptTokensDetails
matching the type from the OpenAI SDK, and that it’s correctly mapped in provider/openai/openai.go.LGTM!
73-80: Approve Usage details forwardingBoth
CompletionTokensDetailsandPromptTokensDetailsare defined oncore.Usage(core/openai.go:18–19), so forwarding them inprovider/openai/openai.gois safe and keeps reporting consistent.
| req := openai.ChatCompletionRequest{ | ||
| Model: c.model, | ||
| MaxTokens: c.maxTokens, | ||
| Temperature: c.temperature, | ||
| TopP: c.topP, | ||
| FrequencyPenalty: c.frequencyPenalty, | ||
| PresencePenalty: c.presencePenalty, | ||
| Model: c.model, | ||
| MaxCompletionTokens: c.maxTokens, | ||
| Temperature: c.temperature, | ||
| TopP: c.topP, | ||
| FrequencyPenalty: c.frequencyPenalty, | ||
| PresencePenalty: c.presencePenalty, | ||
| Messages: []openai.ChatCompletionMessage{ |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Switch to MaxCompletionTokens: verify library support and gate zero values
Using MaxCompletionTokens aligns with newer OpenAI APIs. Two follow-ups:
- Verify the go-openai version in use supports ChatCompletionRequest.MaxCompletionTokens and maps it correctly for both OpenAI and Azure.
- If c.maxTokens can be 0, avoid sending a zero value unless the JSON tag uses omitempty; otherwise conditionally set it only when > 0.
Check library support and JSON tags:
Additionally, confirm minimal supported version via web search:
Suggested change (apply in both CreateFunctionCall and CreateChatCompletion):
- req := openai.ChatCompletionRequest{
- Model: c.model,
- MaxCompletionTokens: c.maxTokens,
- Temperature: c.temperature,
- TopP: c.topP,
- FrequencyPenalty: c.frequencyPenalty,
- PresencePenalty: c.presencePenalty,
- ...
- }
+ req := openai.ChatCompletionRequest{
+ Model: c.model,
+ Temperature: c.temperature,
+ TopP: c.topP,
+ FrequencyPenalty: c.frequencyPenalty,
+ PresencePenalty: c.presencePenalty,
+ ...
+ }
+ if c.maxTokens > 0 {
+ req.MaxCompletionTokens = c.maxTokens
+ }Note: If the library lacks MaxCompletionTokens or its JSON tag omitempty is missing, this change may break requests or send an invalid zero; please verify first.
Also applies to: 143-149
🏁 Script executed:
#!/bin/bash
set -e
echo "go-openai version in go.mod/go.sum:"
rg -n "github.com/sashabaranov/go-openai" go.mod go.sum || true
echo "Occurrences of MaxCompletionTokens in repo (and vendored code if present):"
rg -n "MaxCompletionTokens" || true
echo "Definition of ChatCompletionRequest (if vendored):"
rg -n "type ChatCompletionRequest struct" -A 60 vendor 2>/dev/null || trueLength of output: 901
🌐 Web query:
What version of github.com/sashabaranov/go-openai introduced ChatCompletionRequest.MaxCompletionTokens and Usage.PromptTokensDetails?
💡 Result:
ChatCompletionRequest.MaxCompletionTokens and Usage.PromptTokensDetails were both introduced in version v1.39.0 of github.com/sashabaranov/go-openai.
Supporting details:
- The current API documentation and
common.goshowPromptTokensDetailsas part of theUsagestruct5, and recent test code (e.g.,chat_test.go) referencesMaxCompletionTokensinChatCompletionRequest[3]. - These fields are absent from the older v1.38.x documentation and changelogs1, but clearly present in the code, tests, and types corresponding with v1.39.0 and newer.
- Specifically, the releases page shows that v1.39.0 (released April 21, 2025) added features related to detailed tokens accounting, matching these new fields. The changelog for v1.39.0 directly mentions "Add MaxCompletionTokens to ChatCompletionRequest and support detailed token usage," covering both fields.
Summary Table:
| Feature | Introduced in Version |
|---|---|
| ChatCompletionRequest.MaxCompletionTokens | v1.39.0 |
| Usage.PromptTokensDetails | v1.39.0 |
Reference: See the go-openai releases page and common.go for current field definitions.
Citations:
- 1: https://github.com/sashabaranov/go-openai/releases
- 2: https://pkg.go.dev/github.com/sashabaranov/go-gpt3
- 3: https://github.com/sashabaranov/go-openai/blob/master/chat_test.go
- 4: https://github.com/openai/openai-go
- 5: https://github.com/sashabaranov/go-openai/blob/master/common.go
- 6: https://github.com/sashabaranov/go-openai/blob/master/api_integration_test.go
- 7: https://github.com/sashabaranov/go-openai
- 8: https://codetrips.com/2023/11/19/using-openai-gpt-to-build-a-coding-assistant-that-uses-openai-gpt-to-build-apps-including-itself/
- 9: https://community.openai.com/t/issue-with-openai-chatcompletion-create-in-latest-openai-python-library/926301
- 10: https://pkg.go.dev/github.com/sashabaranov/go-openai
Switch to MaxCompletionTokens: v1.40.5 supports it; guard zero values
We’re on v1.40.5 (introduced in v1.39.0), so ChatCompletionRequest.MaxCompletionTokens is available for both OpenAI and Azure backends. To avoid sending an explicit 0 when c.maxTokens isn’t set, only assign it when > 0. If the library’s JSON tag for MaxCompletionTokens isn’t omitempty, this guard is required; otherwise it’s optional.
• Locations to update:
- provider/openai/openai.go, lines 108–115
- provider/openai/openai.go, lines 143–149
Suggested refactor in both CreateChatCompletion and CreateFunctionCall:
@@ -108,8 +108,6 @@ func (c *Client) CreateChatCompletion(ctx context.Context, messages []openai.Ch
- req := openai.ChatCompletionRequest{
- Model: c.model,
- MaxCompletionTokens: c.maxTokens,
- Temperature: c.temperature,
- TopP: c.topP,
- FrequencyPenalty: c.frequencyPenalty,
- PresencePenalty: c.presencePenalty,
- Messages: messages,
- }
+ req := openai.ChatCompletionRequest{
+ Model: c.model,
+ Temperature: c.temperature,
+ TopP: c.topP,
+ FrequencyPenalty: c.frequencyPenalty,
+ PresencePenalty: c.presencePenalty,
+ Messages: messages,
+ }
+ if c.maxTokens > 0 {
+ req.MaxCompletionTokens = c.maxTokens
+ }Apply the same change at lines 143–149.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| req := openai.ChatCompletionRequest{ | |
| Model: c.model, | |
| MaxTokens: c.maxTokens, | |
| Temperature: c.temperature, | |
| TopP: c.topP, | |
| FrequencyPenalty: c.frequencyPenalty, | |
| PresencePenalty: c.presencePenalty, | |
| Model: c.model, | |
| MaxCompletionTokens: c.maxTokens, | |
| Temperature: c.temperature, | |
| TopP: c.topP, | |
| FrequencyPenalty: c.frequencyPenalty, | |
| PresencePenalty: c.presencePenalty, | |
| Messages: []openai.ChatCompletionMessage{ | |
| req := openai.ChatCompletionRequest{ | |
| Model: c.model, | |
| Temperature: c.temperature, | |
| TopP: c.topP, | |
| FrequencyPenalty: c.frequencyPenalty, | |
| PresencePenalty: c.presencePenalty, | |
| Messages: messages, | |
| } | |
| if c.maxTokens > 0 { | |
| req.MaxCompletionTokens = c.maxTokens | |
| } |
🤖 Prompt for AI Agents
In provider/openai/openai.go at lines 108 to 115 and 143 to 149, the
MaxCompletionTokens field in ChatCompletionRequest is always set, which can send
an explicit zero value if c.maxTokens is not set. To fix this, modify the code
to assign MaxCompletionTokens only when c.maxTokens is greater than zero,
preventing sending zero values unnecessarily. Apply this conditional assignment
in both the CreateChatCompletion and CreateFunctionCall functions at the
specified lines.
1 participant
Signed-off-by: appleboy [email protected]
Summary by CodeRabbit
New Features
Refactor
Bug Fixes