Conversation
…ge fields - Change Message.Content from string to any to support multimodal content (arrays of text/image parts) instead of just strings - Add Message.Custom map to preserve non-standard fields like tool_calls, function_call, tool_call_id, refusal, name, etc. - Add custom UnmarshalJSON/MarshalJSON for Message to capture and re-emit unknown fields during serialization (needed for prompt caching) - Fix streaming extractors in bedrock and googleai to handle Content as any - Enhance Responses API extractor with: - Annotations support (url_citation for web search results) - Reasoning output type with summary field - Function call support (name, arguments) - Array response format (streaming output) - Store full output in Custom for access to web_search_call, etc. - Add comprehensive tests for: - Content as string, array, null, number, boolean - Non-standard message properties - Round-trip marshaling preserving custom fields - Web search with annotations - Reasoning outputs with reasoning_tokens - Multiple text segments - Error handling Fixes error: "json: cannot unmarshal array into Go struct field Message.Alias.messages.content of type string"
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 5 minutes and 54 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughMessage model now accepts arbitrary JSON in Changes
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![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
🧹 Nitpick comments (3)
metadata.go (1)
87-89: Don't letCustomoverrideroleorcontent.Because
Customis merged after the typed fields, a caller can serialize a differentrole/contentthanMessageactually holds. Skipping reserved keys here keeps the JSON output consistent with the struct state.Suggested guard
for k, v := range m.Custom { + if k == "role" || k == "content" { + continue + } result[k] = v }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@metadata.go` around lines 87 - 89, When merging m.Custom into result (the loop over m.Custom), skip any reserved keys "role" and "content" so they cannot override the typed fields; update the loop that iterates m.Custom (referencing m.Custom and result) to continue/ignore entries where k == "role" or k == "content", leaving the existing typed values intact.providers/openai_compatible/parser_test.go (2)
607-623: Tighten this test to assert text payloads, not just count.Right now it only checks array length. Adding assertions for both
"hello"and"world"would make this test materially stronger.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@providers/openai_compatible/parser_test.go` around lines 607 - 623, The test TestParser_ContentAsArrayOfText currently only asserts that meta.Messages[0].Content is an array of length 2; update it to also assert the actual text payloads by casting each element of meta.Messages[0].Content to the expected map/struct shape and verifying the "text" fields equal "hello" and "world" respectively (i.e., after obtaining content := meta.Messages[0].Content.([]interface{}), check content[0]["text"] == "hello" and content[1]["text"] == "world" or the equivalent field access used by Parser.Parse).
593-603: Avoid panic-prone type assertions in test checks.Several direct casts can panic and hide the specific assertion failure. Prefer checked assertions to keep failures actionable.
Proposed test-hardening pattern
- textPart := content[0].(map[string]interface{}) + textPart, ok := content[0].(map[string]interface{}) + if !ok { + t.Fatalf("expected content[0] to be object, got %T", content[0]) + } - imagePart := content[1].(map[string]interface{}) + imagePart, ok := content[1].(map[string]interface{}) + if !ok { + t.Fatalf("expected content[1] to be object, got %T", content[1]) + }Also applies to: 649-652, 673-676, 711-714
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@providers/openai_compatible/responses_extractor.go`:
- Around line 20-29: The fallback that checks if the payload is an array
inspects body[0], which fails when the JSON array is preceded by whitespace;
modify the logic in the responses extractor around the json.Unmarshal error
handling (variables: body, responsesResp, outputItems) to first find the first
non-whitespace byte (scan bytes until a non-space/newline/tab/carriage-return)
and test that byte == '[' before attempting the alternate unmarshal into
[]ResponsesOutputItem and setting responsesResp.Output/Status; keep existing
error returns otherwise.
---
Nitpick comments:
In `@metadata.go`:
- Around line 87-89: When merging m.Custom into result (the loop over m.Custom),
skip any reserved keys "role" and "content" so they cannot override the typed
fields; update the loop that iterates m.Custom (referencing m.Custom and result)
to continue/ignore entries where k == "role" or k == "content", leaving the
existing typed values intact.
In `@providers/openai_compatible/parser_test.go`:
- Around line 607-623: The test TestParser_ContentAsArrayOfText currently only
asserts that meta.Messages[0].Content is an array of length 2; update it to also
assert the actual text payloads by casting each element of
meta.Messages[0].Content to the expected map/struct shape and verifying the
"text" fields equal "hello" and "world" respectively (i.e., after obtaining
content := meta.Messages[0].Content.([]interface{}), check content[0]["text"] ==
"hello" and content[1]["text"] == "world" or the equivalent field access used by
Parser.Parse).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8c89ce81-2a2c-4ba6-b3c4-af3100188e83
📒 Files selected for processing (9)
metadata.goproviders/anthropic/parser_test.goproviders/bedrock/parser_test.goproviders/bedrock/streaming_extractor.goproviders/googleai/parser_test.goproviders/googleai/streaming_extractor.goproviders/openai_compatible/parser_test.goproviders/openai_compatible/responses_extractor.goproviders/openai_compatible/responses_test.go
📜 Review details
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (11)
providers/googleai/streaming_extractor.go (1)
106-110: Nice type guard for streaming text accumulation.This avoids invalid string concatenation now that
Message.Contentcan hold non-string shapes.providers/anthropic/parser_test.go (1)
205-219: Good multimodal regression test.This locks in the current behavior for mixed text/image content without assuming
Message.Contentis always a string.providers/bedrock/parser_test.go (1)
195-206: Useful multi-block coverage.This protects the Bedrock text-concatenation path for messages with multiple content blocks.
providers/bedrock/streaming_extractor.go (1)
255-259: Nice compatibility fix for streamed deltas.The type check keeps Bedrock streaming accumulation safe after
Message.Contentstopped being string-only.providers/googleai/parser_test.go (1)
189-213: Good coverage for multipart and inline-data requests.These cases exercise both text concatenation and the non-text-part path.
providers/openai_compatible/responses_test.go (1)
291-1227: Great edge-case coverage for the Responses API.This suite materially lowers risk around array outputs, annotations, reasoning items, tool calls, usage details, and mixed multimodal inputs.
metadata.go (1)
49-59: Unknown request fields lose numeric precision, but this doesn't affect prompt-cache hashing.Unmarshalling into
map[string]anydoes convert JSON numbers tofloat64, so{"x": 123}becomes{"x": 123.0}when round-tripped. However, this is a minor issue for truly unknown fields. Prompt-cache hashing usesrawBody(the original request bytes), notm.Custom, so field precision here has no impact on cache key generation. If a provider includes structured metadata in requests, it should be modeled as a typed field inBodyMetadata.Customrather than relying on lossy map conversion. For truly dynamic fields you cannot model, consider preserving critical numeric values as strings if exact round-trip is required.> Likely an incorrect or invalid review comment.providers/openai_compatible/parser_test.go (4)
5-5: Good update: assertions now match polymorphicContenttype.Switching assertion logs to
%vis the right move now thatMessage.Contentcan be non-string values.Also applies to: 33-34, 136-137, 150-151, 236-237, 265-269, 294-295
562-624: Strong content-type coverage for parser behavior.These cases give solid protection for
Contentasany(string, array, null, number, boolean), which is the core regression surface of this PR.Also applies to: 789-829
625-716: Great validation of non-standard message field preservation.The new tests meaningfully protect
Message.Custombehavior for tool/function/refusal and arbitrary provider-specific keys.Also applies to: 831-843
754-787: Round-trip test is high-value for serialization safety.Nice addition—this catches regressions where custom keys or multimodal content shape could be dropped during marshal/unmarshal.
- Handle whitespace before JSON array in responses extractor - Skip reserved keys (role, content) when merging Custom in MarshalJSON - Add assertions for text content in TestParser_ContentAsArrayOfText - Add tests for whitespace array and Custom field override protection
There was a problem hiding this comment.
🧹 Nitpick comments (1)
providers/openai_compatible/responses_extractor.go (1)
95-103: Consider usingstrings.Joinfor cleaner text concatenation.The manual loop concatenation works but
strings.Joinwould be more idiomatic and slightly more efficient for larger arrays.♻️ Suggested simplification
- if len(texts) == 0 { - return "" - } - if len(texts) == 1 { - return texts[0] - } - // Join multiple text segments with newline - result := texts[0] - for _, t := range texts[1:] { - result += "\n" + t - } - return result + return strings.Join(texts, "\n")Note: This requires adding
"strings"to the imports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@providers/openai_compatible/responses_extractor.go` around lines 95 - 103, Replace the manual loop that concatenates the slice variable texts with strings.Join(texts, "\n") for clearer and more efficient concatenation; add "strings" to the imports and remove the special-case if len(texts) == 1 branch (strings.Join handles single-element slices correctly). Use the same surrounding function in responses_extractor.go that returns the concatenated result so callers are unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@providers/openai_compatible/responses_extractor.go`:
- Around line 95-103: Replace the manual loop that concatenates the slice
variable texts with strings.Join(texts, "\n") for clearer and more efficient
concatenation; add "strings" to the imports and remove the special-case if
len(texts) == 1 branch (strings.Join handles single-element slices correctly).
Use the same surrounding function in responses_extractor.go that returns the
concatenated result so callers are unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bf585c7f-a1f3-4042-8b01-b3e7b740a7a6
📒 Files selected for processing (4)
metadata.goproviders/openai_compatible/parser_test.goproviders/openai_compatible/responses_extractor.goproviders/openai_compatible/responses_test.go
✅ Files skipped from review due to trivial changes (1)
- providers/openai_compatible/responses_test.go
📜 Review details
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (10)
providers/openai_compatible/responses_extractor.go (3)
21-37: Whitespace handling before JSON array detection looks correct.The implementation now properly scans for the first non-whitespace character before checking if the body is a JSON array. This addresses the previous review feedback. The fallback parsing path correctly populates
responsesResp.Outputand sets a default status.
117-145: Type definitions are well-structured for Responses API output.The new fields (
Name,Arguments,Summary,Annotations,Logprobs) properly model the richer output structures from the Responses API. Using*intforIndexcorrectly handles nullable integers, andinterface{}forLogprobsaccommodates varying structures.
74-76: LGTM - storing full output in Custom.Preserving the complete
Outputarray inmeta.Custom["output"]allows callers to access provider-specific fields (e.g.,web_search_call, function call details) that aren't captured in the normalizedChoicesstructure.metadata.go (3)
28-35: LGTM - Message struct changes are well-designed.Changing
Contenttoanyproperly supports multimodal content arrays. TheCustommap withjson:"-"tag correctly excludes it from standard marshaling while the customMarshalJSONhandles its serialization.
37-62: LGTM - UnmarshalJSON correctly captures unknown fields.The implementation uses the standard alias pattern to avoid recursion and properly filters reserved fields (
role,content) when populatingCustom. Double unmarshaling is the idiomatic Go approach for this use case.
64-95: LGTM - MarshalJSON correctly protects reserved fields.The implementation properly prevents
Customentries from overridingroleorcontent(lines 88-90). While the triple marshal/unmarshal pattern has some overhead, it ensures correctness and handles edge cases well. The early return optimization for emptyCustom(lines 78-80) avoids unnecessary work in the common case.providers/openai_compatible/parser_test.go (4)
32-33: LGTM - Format string update is correct.Changing from
%qto%vis appropriate sinceContentis nowanytype. The%vverb handles all types correctly for error messages.
562-843: Comprehensive test coverage for Content type variations.The tests thoroughly cover all JSON value types that
Contentcan now hold: strings, arrays (multimodal), null, numbers, and booleans. This validates theanytype change works correctly across all expected inputs.
639-729: LGTM - Custom field preservation tests are thorough.These tests validate that provider-specific message fields (
tool_calls,tool_call_id,function_call,name,refusal) are correctly captured inMessage.Custom. This is essential for accurate API proxying and prompt-caching hash computation.
768-889: LGTM - Round-trip and reserved field protection tests.
TestParser_MessageRoundTripvalidates that custom properties survive parse → marshal → unmarshal cycles, which is critical for prompt-caching hash computation.TestParser_MessageCustomDoesNotOverrideReservedFieldsconfirms the security of the MarshalJSON implementation.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
providers/openai_compatible/responses_extractor.go (2)
45-49:⚠️ Potential issue | 🟠 Major
reasoning_tokensis parsed but never exposed.
ResponsesUsage.OutputTokensDetailswas added, butExtractonly copies prompt/completion/total counts. That silently drops reasoning-token accounting from the returned metadata, so the new reasoning support is incomplete.📦 Suggested fix
meta.Custom["status"] = responsesResp.Status meta.Custom["api_type"] = llmproxy.APITypeResponses + if responsesResp.Usage.OutputTokensDetails != nil && responsesResp.Usage.OutputTokensDetails.ReasoningTokens > 0 { + meta.Custom["output_tokens_details"] = map[string]any{ + "reasoning_tokens": responsesResp.Usage.OutputTokensDetails.ReasoningTokens, + } + } if responsesResp.Error != nil { meta.Custom["error"] = responsesResp.Error }Also applies to: 70-77, 152-154
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@providers/openai_compatible/responses_extractor.go` around lines 45 - 49, The Extract logic is dropping the new reasoning-token counts; update every place where a llmproxy.Usage literal is constructed (the blocks that currently set PromptTokens, CompletionTokens, TotalTokens) to also populate the new reasoning token field by reading responsesResp.Usage.OutputTokensDetails.Reasoning (or ReasoningTokens if that is the exact field name on OutputTokensDetails) into the corresponding llmproxy.Usage field (e.g., ReasoningTokens). Ensure you update all occurrences (the Usage: llmproxy.Usage{...} construction sites in responses_extractor.go) so reasoning token accounting is carried through the returned metadata.
82-88:⚠️ Potential issue | 🟠 MajorAdd
Refusalfield handling toResponsesOutputContentstruct and extraction logic.
extractResponsesContentonly appendsoutput_textcontent, but Responses API can include message content items with typerefusal. When a response contains only refusal content and no output_text, the message content becomes empty, losing the refusal message.Add a
Refusalfield toResponsesOutputContentand update the extraction logic to include it:Suggested changes
type ResponsesOutputContent struct { Type string `json:"type"` Text string `json:"text,omitempty"` + Refusal string `json:"refusal,omitempty"` Annotations []ResponsesOutputAnnotation `json:"annotations,omitempty"` Logprobs interface{} `json:"logprobs,omitempty"` } @@ if item.Type == "message" { for _, c := range item.Content { if c.Type == "output_text" && c.Text != "" { texts = append(texts, c.Text) + } else if c.Type == "refusal" && c.Refusal != "" { + texts = append(texts, c.Refusal) } } }Also applies to similar extraction logic at lines 126-131.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@providers/openai_compatible/responses_extractor.go` around lines 82 - 88, The ResponsesOutputContent struct and extraction logic currently ignore refusal items, causing messages with only refusal to be dropped; update the ResponsesOutputContent model to add a Refusal string field (e.g., Refusal string `json:"refusal,omitempty"`), then modify extractResponsesContent (and the analogous extraction block around lines 126-131) to check each content item for Type == "refusal" and if present append its text (or refusal text) into the texts slice (or incorporate it into the built message) alongside output_text so refusal-only messages are preserved; update any JSON tags/parsing as needed to map the refusal payload into the new field.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@providers/openai_compatible/responses_extractor.go`:
- Around line 75-76: The ResponsesOutputAnnotation struct and the code that
stores responsesResp.Output into meta.Custom["output"] are dropping
start_index/end_index span fields; update ResponsesOutputAnnotation to include
StartIndex and EndIndex fields with `json:"start_index"` and `json:"end_index"`
tags (matching the Responses API names) so unmarshaling preserves span metadata,
and change the assignment that writes to meta.Custom["output"] to preserve the
raw JSON (e.g., store the original raw output bytes or json.RawMessage instead
of the partially unmarshaled struct) so callers can access both the annotated
structs and the original output with character positions.
---
Outside diff comments:
In `@providers/openai_compatible/responses_extractor.go`:
- Around line 45-49: The Extract logic is dropping the new reasoning-token
counts; update every place where a llmproxy.Usage literal is constructed (the
blocks that currently set PromptTokens, CompletionTokens, TotalTokens) to also
populate the new reasoning token field by reading
responsesResp.Usage.OutputTokensDetails.Reasoning (or ReasoningTokens if that is
the exact field name on OutputTokensDetails) into the corresponding
llmproxy.Usage field (e.g., ReasoningTokens). Ensure you update all occurrences
(the Usage: llmproxy.Usage{...} construction sites in responses_extractor.go) so
reasoning token accounting is carried through the returned metadata.
- Around line 82-88: The ResponsesOutputContent struct and extraction logic
currently ignore refusal items, causing messages with only refusal to be
dropped; update the ResponsesOutputContent model to add a Refusal string field
(e.g., Refusal string `json:"refusal,omitempty"`), then modify
extractResponsesContent (and the analogous extraction block around lines
126-131) to check each content item for Type == "refusal" and if present append
its text (or refusal text) into the texts slice (or incorporate it into the
built message) alongside output_text so refusal-only messages are preserved;
update any JSON tags/parsing as needed to map the refusal payload into the new
field.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 191e8ab9-23ce-4e1e-a019-246d90848284
📒 Files selected for processing (1)
providers/openai_compatible/responses_extractor.go
📜 Review details
🔇 Additional comments (1)
providers/openai_compatible/responses_extractor.go (1)
21-38: Nice fix on the array fallback.Scanning to the first non-whitespace byte before checking for
[closes the whitespace-prefixed array case cleanly.
- Add StartIndex and EndIndex fields to ResponsesOutputAnnotation - Store raw output as json.RawMessage in output_raw Custom field - Add comprehensive tests for span fields and raw output preservation
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@providers/openai_compatible/responses_extractor.go`:
- Around line 75-81: The code currently only sets meta.Custom["output_raw"] when
the response is wrapped as { "output": ... }, losing raw payload for bare-array
responses; modify the block around responsesResp.Output handling in
responses_extractor.go (where responsesResp, meta.Custom and output_raw are
referenced) to also detect and preserve a top-level JSON array: if
json.Unmarshal into the wrapper fails or if the raw body begins with '[' (or can
be unmarshaled into []ResponsesOutputItem), set meta.Custom["output_raw"] to the
raw body (as json.RawMessage) so array-mode responses preserve extra fields.
Ensure this logic runs alongside the existing wrapper unmarshalling so both
wrapped and bare-array responses populate output_raw.
- Around line 139-145: The ResponsesOutputAnnotation struct's StartIndex and
EndIndex should be changed from int to *int so zero values (0) are preserved
during JSON marshalling; update the struct definition
(ResponsesOutputAnnotation) to use StartIndex *int and EndIndex *int and then
audit places that construct or mutate ResponsesOutputAnnotation (e.g., any code
that sets StartIndex/EndIndex) to allocate integers (take addresses or use
helper funcs) so callers provide non-nil pointers when they intend to include 0.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b17e86dd-c658-4b6e-8c8b-4069be6cd598
📒 Files selected for processing (2)
providers/openai_compatible/responses_extractor.goproviders/openai_compatible/responses_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- providers/openai_compatible/responses_test.go
- Change StartIndex/EndIndex to *int to preserve zero values in JSON - Set output_raw for bare-array responses (not just wrapped responses) - Add tests for zero span values, bare-array raw output, missing fields
1 participant
Summary
json: cannot unmarshal array into Go struct field Message.Alias.messages.content of type stringthat occurs when OpenAI returns multimodal content as an arrayMessage.Contentfromstringtoanyto support both string content and multimodal content arrays (text/image parts)Message.Custommap to preserve non-standard message fields liketool_calls,function_call,tool_call_id,refusal,name, etc.UnmarshalJSON/MarshalJSONforMessageto capture and re-emit unknown fields during serialization (required for prompt caching hash computation)ContentasanyTest plan
tool_calls,function_call,tool_call_id,refusal,name)reasoning_tokensSummary by CodeRabbit
New Features
Bug Fixes
Tests