-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix(translator): extract system messages from input in codex response… #1121
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
base: main
Are you sure you want to change the base?
Conversation
…s API
When codexInstructionsEnabled is false (default), the hasOfficialInstructions
flag is set to true, causing the code to enter the 'passthrough' branch.
Previously, this branch forwarded the input array directly to OpenAI Responses
API without filtering out system messages.
The OpenAI Responses API (/v1/responses) does not accept messages with
role: 'system' in the input array, returning:
{"detail":"System messages are not allowed"}
This commit fixes the issue by:
1. Extracting content from any system messages in the input array
2. Removing system messages from the input before forwarding
3. Appending the extracted system content to the instructions field
This ensures compatibility with the OpenAI Responses API while preserving
the system instructions for the model (e.g., Cline tool definitions).
Closes: Related to codexInstructionsEnabled behavior when false
Summary of ChangesHello @ZqinKing, 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! This pull request addresses a compatibility issue with the OpenAI Responses API by modifying how system messages are processed. Previously, these messages were directly forwarded, leading to API rejections. The updated logic now intelligently extracts the content from system messages, removes them from the primary input, and appends their content to the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe 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 by creating a comment using either
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 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. 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
|
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.
Code Review
This pull request addresses a bug where system messages were incorrectly passed to the OpenAI Responses API, causing errors. The fix involves extracting system message content, removing these messages from the input, and appending their content to the instructions field. The implementation is logical and correctly solves the problem. I've included a couple of comments regarding potential improvements for maintainability and performance.
| if content := item.Get("content"); content.Exists() && content.IsArray() { | ||
| content.ForEach(func(_, contentItem gjson.Result) bool { | ||
| text := contentItem.Get("text").String() | ||
| if systemContentBuilder.Len() > 0 && text != "" { | ||
| systemContentBuilder.WriteByte('\n') | ||
| } | ||
| systemContentBuilder.WriteString(text) | ||
| return true | ||
| }) | ||
| } |
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.
The logic to extract content from a system message's content array is also present earlier in this file (lines 57-67). This duplication could lead to maintenance issues if one part is updated and the other is forgotten. To improve maintainability and adhere to the DRY (Don't Repeat Yourself) principle, consider refactoring this logic into a separate helper function that can be called from both places. This could be addressed in a follow-up PR.
| } | ||
| continue | ||
| } | ||
| newInput, _ = sjson.SetRaw(newInput, "-1", item.Raw) |
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.
Calling sjson.SetRaw inside a loop can be inefficient as it may re-parse and re-serialize the newInput JSON string on each iteration. For better performance, especially with a large number of messages, consider collecting the raw message strings for non-system messages into a slice and then using strings.Join to construct the final JSON array string once, after the loop. This avoids the overhead of repeated JSON processing.
Extracts the logic for parsing system message content into a helper function `extractSystemContent` to remove duplication. Refactors the input reconstruction loop to collect raw JSON items and join them, replacing repeated sjson set operations for better efficiency and cleaner code.
Explicitly delete the "metadata" field from the raw JSON payload in ConvertOpenAIResponsesRequestToCodex. This ensures that unsupported metadata fields are stripped before the request is forwarded, aligning behavior with other removed fields like temperature and service_tier.
When codexInstructionsEnabled is false (default), the hasOfficialInstructions flag is set to true, causing the code to enter the 'passthrough' branch. Previously, this branch forwarded the input array directly to OpenAI Responses API without filtering out system messages.
The OpenAI Responses API (/v1/responses) does not accept messages with role: 'system' in the input array, returning:
{"detail":"System messages are not allowed"}
This commit fixes the issue by:
This ensures compatibility with the OpenAI Responses API while preserving the system instructions for the model (e.g., Cline tool definitions).
Closes: Related to codexInstructionsEnabled behavior when false