openai integration content string handling - #3949
Conversation
|
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR fixes OpenAI Responses API content handling and coordinates transitive dependency upgrades across all modules. The functional changes normalize reasoning content representation and fix empty content marshaling to comply with API contracts. The dependency updates bring compression, encoding, authentication, observability, and Google platform libraries to newer versions across 15 modules. ChangesOpenAI Responses API Content Handling
Transitive Dependency Version Updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Confidence Score: 5/5Safe to merge; the normalization logic is correct, operates on value copies, and is well-tested. The reasoning-message normalization in responses.go is correctly scoped, uses value semantics to avoid mutating caller input, and is covered by dedicated tests including a marshal-output check. The MarshalJSON fallback change in schemas/responses.go is global but the affected state (both fields nil on a non-nil pointer) is unreachable through normal deserialization and suppressed by omitempty on the outbound path. core/schemas/responses.go — the MarshalJSON fallback is a global behavior change worth a second look to confirm no other code paths construct &ResponsesMessageContent{} with neither field set for non-reasoning message types. Important Files Changed
Reviews (7): Last reviewed commit: "openai integration content string handli..." | Re-trigger Greptile |
3a811a7 to
3497c10
Compare
c2485e4 to
7a77a2e
Compare
3497c10 to
d365c69
Compare
7a77a2e to
29216bc
Compare
d365c69 to
f282051
Compare
693dc82 to
96acc40
Compare
f282051 to
1b092da
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
transports/go.mod (1)
134-134:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winResolve leftover merge-conflict marker in go.mod (build blocker).
Line 134 includes
<<<<<<< HEADin a dependency comment, which makesgo.modinvalid and can breakgo mod tidy/build tooling. Remove the conflict artifact and keep a single clean dependency entry.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@transports/go.mod` at line 134, Remove the leftover merge-conflict marker and duplicate text from the go.mod dependency entry: locate the line containing "github.com/mattn/go-sqlite3 v1.14.32 // indirect; indirect<<<<<<< HEAD" and edit it to a single valid dependency comment (e.g., "github.com/mattn/go-sqlite3 v1.14.32 // indirect") so go.mod is syntactically valid and builds pass.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/providers/openai/responses_test.go`:
- Around line 236-287: After calling ToOpenAIResponsesRequest(bifrostReq) add
assertions that the original source message was not mutated: check
bifrostReq.Input[0].Content.ContentStr is still the original pointer/value and
that bifrostReq.Input[0].Content.ContentBlocks is nil or empty; do this in both
the "empty string content is dropped" and "non-empty string content becomes a
reasoning_text block" subtests to ensure the function works on a local copy and
does not populate ContentBlocks on the input message.
In `@core/providers/openai/responses.go`:
- Around line 154-165: The code currently only normalizes an empty ContentStr
but can still leave a zero-value schemas.ResponsesMessageContent (e.g.,
&schemas.ResponsesMessageContent{}), which will serialize to an empty string and
cause OpenAI rejections; update the block that handles message.Content in
core/providers/openai/responses.go to detect and drop zero-value content objects
as well—check if message.Content.ContentBlocks is nil or empty (and any other
fields are zero) and set message.Content = nil in that case, otherwise construct
the ResponsesMessageContent with a single ResponsesMessageContentBlock as you're
already doing (use schemas.ResponsesMessageContent, ContentBlocks, and
ResponsesOutputMessageContentTypeReasoning to locate the change).
---
Outside diff comments:
In `@transports/go.mod`:
- Line 134: Remove the leftover merge-conflict marker and duplicate text from
the go.mod dependency entry: locate the line containing
"github.com/mattn/go-sqlite3 v1.14.32 // indirect; indirect<<<<<<< HEAD" and
edit it to a single valid dependency comment (e.g., "github.com/mattn/go-sqlite3
v1.14.32 // indirect") so go.mod is syntactically valid and builds pass.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 01ac69c2-5e88-4c18-8b61-ddec791f212e
⛔ Files ignored due to path filters (13)
core/go.sumis excluded by!**/*.sumframework/go.sumis excluded by!**/*.sumplugins/compat/go.sumis excluded by!**/*.sumplugins/governance/go.sumis excluded by!**/*.sumplugins/jsonparser/go.sumis excluded by!**/*.sumplugins/logging/go.sumis excluded by!**/*.sumplugins/maxim/go.sumis excluded by!**/*.sumplugins/mocker/go.sumis excluded by!**/*.sumplugins/otel/go.sumis excluded by!**/*.sumplugins/prompts/go.sumis excluded by!**/*.sumplugins/semanticcache/go.sumis excluded by!**/*.sumplugins/telemetry/go.sumis excluded by!**/*.sumtransports/go.sumis excluded by!**/*.sum
📒 Files selected for processing (17)
core/go.modcore/providers/openai/responses.gocore/providers/openai/responses_test.gocore/schemas/responses.gocore/schemas/responses_test.goframework/go.modplugins/compat/go.modplugins/governance/go.modplugins/jsonparser/go.modplugins/logging/go.modplugins/maxim/go.modplugins/mocker/go.modplugins/otel/go.modplugins/prompts/go.modplugins/semanticcache/go.modplugins/telemetry/go.modtransports/go.mod
7a8fb4f to
a3b4a05
Compare
1b092da to
58e1a89
Compare
Merge activity
|
## Summary When replaying reasoning items through the OpenAI Responses API (e.g. Codex/GPT-5.5), the `content` field on a reasoning message can arrive as a string (notably an empty `""` after round-tripping). OpenAI types `reasoning.content` as an array of `reasoning_text` blocks and rejects a string value with `"expected an array ... got a string"`. This fix normalizes string content on outbound reasoning messages: empty strings are dropped entirely, and non-empty strings are promoted to a `reasoning_text` block. ## Changes - In `ToOpenAIResponsesRequest`, when a reasoning message has `Content.ContentStr` set, the string is either dropped (if empty) or converted to a `ResponsesMessageContentBlock` with type `reasoning_text` (if non-empty). The reassignment operates on the local value copy to avoid mutating the caller's input. - Tests cover both the empty-string drop case (including a marshal check to ensure `"content":""` never appears in the serialized output) and the non-empty string promotion case. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./core/providers/openai/... -run TestToOpenAIResponsesRequest_ReasoningStringContent -v ``` Expected: both subtests (`empty string content is dropped` and `non-empty string content becomes a reasoning_text block`) pass, and the marshalled output does not contain `"content":""` on any reasoning item. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations No auth, secrets, or PII implications. The fix only affects how reasoning message content is serialized before being sent to the OpenAI API. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **Bug Fixes** * Fixed handling of reasoning message content when formatted as strings in OpenAI responses * Fixed JSON serialization of empty content fields to ensure API compliance * **Chores** * Updated Go module dependencies across core, framework, and plugin packages for performance and security improvements * **Tests** * Added test coverage for reasoning content normalization and empty content marshaling <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary When replaying reasoning items through the OpenAI Responses API (e.g. Codex/GPT-5.5), the `content` field on a reasoning message can arrive as a string (notably an empty `""` after round-tripping). OpenAI types `reasoning.content` as an array of `reasoning_text` blocks and rejects a string value with `"expected an array ... got a string"`. This fix normalizes string content on outbound reasoning messages: empty strings are dropped entirely, and non-empty strings are promoted to a `reasoning_text` block. ## Changes - In `ToOpenAIResponsesRequest`, when a reasoning message has `Content.ContentStr` set, the string is either dropped (if empty) or converted to a `ResponsesMessageContentBlock` with type `reasoning_text` (if non-empty). The reassignment operates on the local value copy to avoid mutating the caller's input. - Tests cover both the empty-string drop case (including a marshal check to ensure `"content":""` never appears in the serialized output) and the non-empty string promotion case. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./core/providers/openai/... -run TestToOpenAIResponsesRequest_ReasoningStringContent -v ``` Expected: both subtests (`empty string content is dropped` and `non-empty string content becomes a reasoning_text block`) pass, and the marshalled output does not contain `"content":""` on any reasoning item. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations No auth, secrets, or PII implications. The fix only affects how reasoning message content is serialized before being sent to the OpenAI API. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **Bug Fixes** * Fixed handling of reasoning message content when formatted as strings in OpenAI responses * Fixed JSON serialization of empty content fields to ensure API compliance * **Chores** * Updated Go module dependencies across core, framework, and plugin packages for performance and security improvements * **Tests** * Added test coverage for reasoning content normalization and empty content marshaling <!-- end of auto-generated comment: release notes by coderabbit.ai -->
When replaying reasoning items through the OpenAI Responses API (e.g. Codex/GPT-5.5), the `content` field on a reasoning message can arrive as a string (notably an empty `""` after round-tripping). OpenAI types `reasoning.content` as an array of `reasoning_text` blocks and rejects a string value with `"expected an array ... got a string"`. This fix normalizes string content on outbound reasoning messages: empty strings are dropped entirely, and non-empty strings are promoted to a `reasoning_text` block. - In `ToOpenAIResponsesRequest`, when a reasoning message has `Content.ContentStr` set, the string is either dropped (if empty) or converted to a `ResponsesMessageContentBlock` with type `reasoning_text` (if non-empty). The reassignment operates on the local value copy to avoid mutating the caller's input. - Tests cover both the empty-string drop case (including a marshal check to ensure `"content":""` never appears in the serialized output) and the non-empty string promotion case. - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI - [ ] Core (Go) - [ ] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ```sh go test ./core/providers/openai/... -run TestToOpenAIResponsesRequest_ReasoningStringContent -v ``` Expected: both subtests (`empty string content is dropped` and `non-empty string content becomes a reasoning_text block`) pass, and the marshalled output does not contain `"content":""` on any reasoning item. N/A - [ ] Yes - [x] No N/A No auth, secrets, or PII implications. The fix only affects how reasoning message content is serialized before being sent to the OpenAI API. - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable <!-- This is an auto-generated comment: release notes by coderabbit.ai --> * **Bug Fixes** * Fixed handling of reasoning message content when formatted as strings in OpenAI responses * Fixed JSON serialization of empty content fields to ensure API compliance * **Chores** * Updated Go module dependencies across core, framework, and plugin packages for performance and security improvements * **Tests** * Added test coverage for reasoning content normalization and empty content marshaling <!-- end of auto-generated comment: release notes by coderabbit.ai -->

Summary
When replaying reasoning items through the OpenAI Responses API (e.g. Codex/GPT-5.5), the
contentfield on a reasoning message can arrive as a string (notably an empty""after round-tripping). OpenAI typesreasoning.contentas an array ofreasoning_textblocks and rejects a string value with"expected an array ... got a string". This fix normalizes string content on outbound reasoning messages: empty strings are dropped entirely, and non-empty strings are promoted to areasoning_textblock.Changes
ToOpenAIResponsesRequest, when a reasoning message hasContent.ContentStrset, the string is either dropped (if empty) or converted to aResponsesMessageContentBlockwith typereasoning_text(if non-empty). The reassignment operates on the local value copy to avoid mutating the caller's input."content":""never appears in the serialized output) and the non-empty string promotion case.Type of change
Affected areas
How to test
go test ./core/providers/openai/... -run TestToOpenAIResponsesRequest_ReasoningStringContent -vExpected: both subtests (
empty string content is droppedandnon-empty string content becomes a reasoning_text block) pass, and the marshalled output does not contain"content":""on any reasoning item.Screenshots/Recordings
N/A
Breaking changes
Related issues
N/A
Security considerations
No auth, secrets, or PII implications. The fix only affects how reasoning message content is serialized before being sent to the OpenAI API.
Checklist
docs/contributing/README.mdand followed the guidelinesSummary by CodeRabbit
Release Notes
Bug Fixes
Chores
Tests