Skip to content

feat(anthropic): preserve thinking signatures in compat APIs#178

Open
SantiagoDePolonia wants to merge 6 commits intomainfrom
feature/fingerprint-anthropic-flag
Open

feat(anthropic): preserve thinking signatures in compat APIs#178
SantiagoDePolonia wants to merge 6 commits intomainfrom
feature/fingerprint-anthropic-flag

Conversation

@SantiagoDePolonia
Copy link
Copy Markdown
Contributor

@SantiagoDePolonia SantiagoDePolonia commented Mar 26, 2026

Summary

  • add opt-in Anthropic thinking/signature preservation behind
  • expose and round-trip reasoning signatures through OpenAI-compatible chat and responses payloads
  • fix Responses adapter edge cases so reasoning items reject malformed content, preserve opaque fields, and do not drop multimodal assistant content

Testing

  • go test ./internal/core ./internal/providers ./internal/providers/anthropic

Summary by CodeRabbit

  • New Features

    • Improved reasoning support: reasoning items are preserved, can be emitted as dedicated reasoning output, and can merge into assistant messages while retaining per-reasoning signatures.
    • Responses content may include an optional signature attribute for reasoning parts.
    • Cross-provider compatibility to map reasoning/thinking fields between providers and convert reasoning content both directions.
  • Documentation

    • Added an experimental compatibility flag to docs and config to control preservation of Anthropic reasoning signatures.
  • Tests

    • Added extensive tests for parsing, signature propagation, streaming, conversion, merging, and validation.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 26, 2026

Warning

Rate limit exceeded

@SantiagoDePolonia has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 17 minutes and 3 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: f80952c7-99cb-4842-a4cb-d51755b4b40c

📥 Commits

Reviewing files that changed from the base of the PR and between 497b18c and 7647aef.

📒 Files selected for processing (5)
  • internal/providers/anthropic/anthropic_test.go
  • internal/providers/anthropic/reasoning_compat.go
  • internal/providers/anthropic/request_translation.go
  • internal/providers/responses_adapter.go
  • internal/providers/responses_adapter_test.go
📝 Walkthrough

Walkthrough

Adds OpenAI-style "reasoning" support and signature propagation: new UnknownJSONFields merge helper, Responses model/JSON variants for reasoning, Anthropic↔OpenAI reasoning translation, streaming/non-streaming conversion updates, and adapters to merge reasoning items into assistant messages.

Changes

Cohort / File(s) Summary
JSON Field Utilities
internal/core/json_fields.go, internal/core/json_fields_test.go
Add MergeUnknownJSONFields(...) to decode and merge multiple UnknownJSONFields (later values override earlier); simplify internal buffer init; add unit test for merge/override semantics.
Core Responses Types & JSON
internal/core/responses.go, internal/core/responses_json.go, internal/core/responses_json_test.go
Add "reasoning" variant handling to marshal/unmarshal logic and include signature on ResponsesContentItem; add tests covering reasoning parsing/serialization.
Anthropic Compatibility Layer
internal/providers/anthropic/reasoning_compat.go
New env-gated mapping between Anthropic thinking blocks and OpenAI-style reasoning fields (parse/validate/emit both directions) with invalid-request errors on malformed input.
Anthropic Provider & Streaming
internal/providers/anthropic/anthropic.go, internal/providers/anthropic/anthropic_test.go
Extend content blocks with thinking/signature; extract reasoningDetails in non-streaming conversion; refactor streaming to optionally emit reasoning_content, reasoning_signature, reasoning_index behind a feature flag; add extensive tests.
Anthropic Request Translation
internal/providers/anthropic/request_translation.go
buildAnthropicMessageContent extracts/prepends reasoning blocks from message ExtraFields and integrates reasoning with tool-call handling; conversions now use options-aware call enabling Anthropic reasoning compat.
Responses Adapter & Tests
internal/providers/responses_adapter.go, internal/providers/responses_adapter_test.go
Add ResponsesToChatOptions and ConvertResponsesRequestToChatWithOptions; support reasoning input items (validation, normalization) and merge reasoning-only assistant items into subsequent assistant messages when compat is enabled; add tests for merging and validation.
Env & Docs
.env.template, README.md, docs/advanced/configuration.mdx
Add OPENAI_COMPAT_BREAKING_ANTHROPIC_THINKING_SIGNATURES env template and documentation describing experimental compatibility flag and its effects.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Client
  participant ResponsesAdapter
  participant Core
  participant AnthropicProvider
  participant StreamOut

  Client->>ResponsesAdapter: Send Responses request (may include `reasoning` items)
  ResponsesAdapter->>Core: Parse & validate `reasoning` -> merge into Message.ExtraFields (`reasoning_details`)
  ResponsesAdapter->>AnthropicProvider: Send Chat request with Messages (may include reasoning_details)
  AnthropicProvider->>Core: Return streaming events (content_block_start/delta/stop, signature_delta)
  Core->>ResponsesAdapter: Convert events -> reasoning_text.delta / reasoning_signature / reasoning_index events
  ResponsesAdapter->>StreamOut: Emit SSE / response.output_item.added/done with reasoning payloads
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

enhancement

Poem

🐰 I nibble through RawMessage and thread,
Reasoning hops where signatures led.
I merge and trim with a twitchy nose,
From thinking burrows to response rows.
🥕📜 Happy hops — the JSON grows!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'feat(anthropic): preserve thinking signatures in compat APIs' accurately and clearly summarizes the main change: adding support for preserving Anthropic thinking signatures in OpenAI-compatible APIs through an opt-in feature flag.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/fingerprint-anthropic-flag

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/providers/anthropic/anthropic_test.go`:
- Around line 556-617: Update the test(s) to also assert the default-off
behavior by adding a disabled-path assertion: in
TestStreamChatCompletion_EmitsReasoningSignatureBehindFeatureFlag (and the other
similar test blocks referenced) add a run where the environment variable
openAICompatBreakingAnthropicThinkingSignaturesEnv is unset or set to "" before
calling provider.StreamChatCompletion, read the response body, and assert that
the response does NOT contain `"reasoning_signature":"..."` and does NOT contain
`"reasoning_index":...`; ensure you mirror the existing test flow (using
NewWithHTTPClient, SetBaseURL, io.ReadAll on body) but verify absence of those
substrings to lock in the opt-in-only behavior.

In `@internal/providers/anthropic/anthropic.go`:
- Around line 799-805: The current code uses the raw Anthropic block index
(event.Index) for reasoning_index/signatures which misaligns when empty
preambles create gaps; change to track a dense reasoning ordinal counter (e.g.,
a per-stream map like the Responses stream converter uses) that increments only
when event.Delta.Thinking contains non-empty reasoning content and emit that
dense index instead of event.Index when sc.emitReasoningSignatures is true;
update both the block around formatChatChunk(delta, nil, nil) and the analogous
site at lines ~808-812 to read from this reasoning-index map (keyed by
stream/response id) rather than event.Index so reasoning_signature and
reasoning_index remain compact and sequential.
- Around line 1695-1746: The code currently emits empty reasoning events for
placeholder thinking blocks; update reasoningTextDoneEvent (used when emitting
per-block done events) to return "" unless the referenced block has non-empty
trimmed text (i.e., skip emitting when strings.TrimSpace(block.Text.String()) ==
""), and update completeReasoningOutputIfNeeded (used to finalize the reasoning
item) to either (a) build the content slice by filtering out blocks whose
trimmed Text is empty and if the filtered content is empty, do not mark
state.Done or call WriteEvent (return ""), or (b) lazily create the reasoning
state only when the first non-empty thinking delta arrives; implement the
simpler fix (filter empty blocks on finalization and skip emitting per-block
done events for empty blocks) referencing the methods reasoningTextDoneEvent and
completeReasoningOutputIfNeeded and ensure signature emission logic remains
unchanged for non-empty blocks.

In `@internal/providers/anthropic/reasoning_compat.go`:
- Around line 126-158: The handler currently silently drops malformed/empty
reasoning payloads; update the logic so that when "reasoning_details" is present
but parseOpenAIReasoningDetails returns zero usable items, you return a
core.NewInvalidRequestError (not nil,nil), and likewise if either
"reasoning_content" or "reasoning_signature" is present but the other is missing
you return an invalid request error instead of nil,nil; specifically, after
fields.Lookup("reasoning_details") call check whether
parseOpenAIReasoningDetails yields an empty slice and return an error, and
change the early return when one of reasoningContentRaw/reasoningSignatureRaw is
empty to return core.NewInvalidRequestError describing the missing partner
field; also ensure strings.TrimSpace checks on reasoningContent and
reasoningSignature still return an error (not nil,nil) when normalized-empty,
and preserve use of parseOpenAIReasoningDetails,
openAIReasoningDetailsToAnthropicBlocks, and the anthropicContentBlock
construction.

In `@internal/providers/responses_adapter_test.go`:
- Around line 542-585: The test
TestConvertResponsesRequestToChat_RejectsNonReasoningTextParts only checks the
error string; update it to assert the returned error is a core.GatewayError and
that its ClientFacingCategory (or equivalent field/method on core.GatewayError)
equals "invalid_request_error" when calling ConvertResponsesRequestToChat;
locate the assertions after the ConvertResponsesRequestToChat call, perform a
type assertion to the core.GatewayError type (or call the accessor) and fail the
test if the error is not that typed GatewayError or its category is not
"invalid_request_error".

In `@internal/providers/responses_adapter.go`:
- Around line 173-179: The mergeResponsesReasoningExtraFields path currently
returns a successful merge even when buildResponsesReasoningExtraFields yields
no preserved reasoning_details, which allows a ContentNull assistant turn with
no payload; update mergeResponsesReasoningExtraFields to check the resulting
reasoningExtras (from buildResponsesReasoningExtraFields) and if it contains
zero preserved reasoning_details entries (i.e., is empty after filtering) return
an error (or a non-nil error) instead of merging; apply the same check to the
analogous merge logic in the nearby function(s) in the 181-233 region so callers
like convertResponsesInputItems never receive an empty reasoning payload to
synthesize an assistant message.
- Around line 280-284: mergeReasoningIntoAssistantMessage currently rebuilds
reasoning_details only from dst, which causes src.ExtraFields.reasoning_details
to be overwritten; to fix this, extract reasoning details from both dst and src
using reasoningDetailsFromUnknownFields, append/concatenate the slices
(preserving both payloads) into a single slice, then call
buildReasoningUnknownFields on that combined slice and pass the result to
core.MergeUnknownJSONFields when merging dst.ExtraFields and src.ExtraFields
(keeping the cloneResponsesMessage(dst) / merged assignment logic intact).
🪄 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: ASSERTIVE

Plan: Pro

Run ID: 0d802bfa-44d0-492d-bb9c-d98714a78462

📥 Commits

Reviewing files that changed from the base of the PR and between adbdc56 and 01288fa.

📒 Files selected for processing (11)
  • internal/core/json_fields.go
  • internal/core/json_fields_test.go
  • internal/core/responses.go
  • internal/core/responses_json.go
  • internal/core/responses_json_test.go
  • internal/providers/anthropic/anthropic.go
  • internal/providers/anthropic/anthropic_test.go
  • internal/providers/anthropic/reasoning_compat.go
  • internal/providers/anthropic/request_translation.go
  • internal/providers/responses_adapter.go
  • internal/providers/responses_adapter_test.go

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/providers/anthropic/reasoning_compat.go`:
- Around line 61-64: The code currently swallows json.Marshal errors when
creating reasoningContent from joinAnthropicReasoningText(details); update the
logic in the block around reasoningContent so that if json.Marshal returns an
error you either log the error (using the existing logger) or return the error
upstream instead of silently ignoring it; reference the variables and functions
reasoningContent, json.Marshal, joinAnthropicReasoningText, and the fields map
when implementing the change so the marshal failure is handled explicitly (log
or propagate) before attempting to set fields["reasoning_content"].
🪄 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: ASSERTIVE

Plan: Pro

Run ID: 7e97a2aa-aa9c-4ad2-8214-19207e09e4ab

📥 Commits

Reviewing files that changed from the base of the PR and between 01288fa and d7263a5.

📒 Files selected for processing (5)
  • internal/providers/anthropic/anthropic.go
  • internal/providers/anthropic/anthropic_test.go
  • internal/providers/anthropic/reasoning_compat.go
  • internal/providers/responses_adapter.go
  • internal/providers/responses_adapter_test.go

Comment on lines +61 to +64
reasoningContent, err := json.Marshal(joinAnthropicReasoningText(details))
if err == nil {
fields["reasoning_content"] = reasoningContent
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Marshal error silently swallowed.

If json.Marshal fails for reasoningContent, the error is discarded and the field is simply not added. While unlikely to fail for a simple string, this hides potential issues. Consider logging the error or returning early since a marshal failure for such simple data suggests a deeper problem.

🔧 Suggested improvement
 	reasoningContent, err := json.Marshal(joinAnthropicReasoningText(details))
-	if err == nil {
-		fields["reasoning_content"] = reasoningContent
+	if err != nil {
+		// String marshal should not fail; log if it does
+		return core.UnknownJSONFields{}
 	}
+	fields["reasoning_content"] = reasoningContent
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/providers/anthropic/reasoning_compat.go` around lines 61 - 64, The
code currently swallows json.Marshal errors when creating reasoningContent from
joinAnthropicReasoningText(details); update the logic in the block around
reasoningContent so that if json.Marshal returns an error you either log the
error (using the existing logger) or return the error upstream instead of
silently ignoring it; reference the variables and functions reasoningContent,
json.Marshal, joinAnthropicReasoningText, and the fields map when implementing
the change so the marshal failure is handled explicitly (log or propagate)
before attempting to set fields["reasoning_content"].

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
internal/providers/anthropic/anthropic.go (1)

1696-1705: ⚠️ Potential issue | 🟠 Major

Delay reasoning item creation until the first real thinking_delta.

The Responses stream still allocates/emits reasoning state before it knows the block contains text. A placeholder thinking block can therefore leave a dangling response.output_item.added, and if the next block is the first real one the stream emits content_index: 1 while response.output_item.done compacts that same block to content[0]. Create the block lazily on the first non-whitespace thinking_delta, and only attach a signature once that block exists.

Possible fix
 func (sc *responsesStreamConverter) setReasoningSignature(anthropicIndex int, signature string) {
 	if signature == "" {
 		return
 	}
-	state := sc.ensureResponsesReasoningState()
-	contentIndex := sc.reasoningContentIndex(anthropicIndex)
-	if contentIndex >= len(state.Blocks) {
+	state := sc.reasoning
+	if state == nil {
+		return
+	}
+	contentIndex, ok := sc.thinkingContentIndices[anthropicIndex]
+	if !ok || contentIndex >= len(state.Blocks) {
 		return
 	}
 	state.Blocks[contentIndex].Signature = signature
 }
@@
 	case "content_block_start":
 		if event.ContentBlock != nil && event.ContentBlock.Type == "thinking" {
-			if !sc.emitReasoningSignatures {
-				return ""
-			}
-			sc.reasoningContentIndex(event.Index)
-			return sc.startReasoningOutput()
+			return ""
 		}
@@
 		case "thinking_delta":
 			if !sc.emitReasoningSignatures || event.Delta.Thinking == "" {
 				return ""
 			}
-			prefix := sc.startReasoningOutput()
-			sc.appendReasoningText(event.Index, event.Delta.Thinking)
-			contentIndex := sc.reasoningContentIndex(event.Index)
+			contentIndex, ok := sc.thinkingContentIndices[event.Index]
+			prefix := ""
+			if !ok {
+				if strings.TrimSpace(event.Delta.Thinking) == "" {
+					return ""
+				}
+				prefix = sc.startReasoningOutput()
+				contentIndex = sc.reasoningContentIndex(event.Index)
+			}
+			sc.appendReasoningText(event.Index, event.Delta.Thinking)
 			return prefix + sc.output.WriteEvent("response.reasoning_text.delta", map[string]any{
 				"type":          "response.reasoning_text.delta",
 				"item_id":       sc.reasoning.ItemID,

Also applies to: 1740-1755, 1802-1807, 1829-1848

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/providers/anthropic/anthropic.go` around lines 1696 - 1705, The
reasoning block is being created eagerly which can emit a placeholder thinking
block before any real text; update setReasoningSignature and the code paths that
create reasoning blocks (e.g., where ensureResponsesReasoningState(),
reasoningContentIndex(), and block append logic are used) to defer creating the
reasoning Block until you see the first non-whitespace thinking_delta;
specifically, only call ensureResponsesReasoningState()/append a new Block when
the incoming thinking_delta contains non-whitespace content and then attach
Signature with setReasoningSignature only if the corresponding block exists
(guard against contentIndex >= len(state.Blocks) before setting Signature).
Apply the same lazy-creation guard to the other similar sites noted (around the
blocks indicated by the reviewer: ~1740-1755, ~1802-1807, ~1829-1848) so no
placeholder block is emitted for empty/whitespace thinking deltas.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/core/json_fields.go`:
- Around line 45-70: The current parsing in internal/core/json_fields.go
inconsistently handles errors: initial token/delimiter errors use continue but
mid-object errors return UnknownJSONFields{}, causing previously merged data
(merged map) to be discarded; change the mid-object error handling in the json
decoding loop (where readJSONObjectKey and dec.Decode(&value) are called) to
mirror the initial behavior by skipping the entire corrupt fieldSet instead of
returning empty — i.e., on failing readJSONObjectKey or dec.Decode, continue to
the next fieldSet, preserve merged data, and only return UnknownJSONFields{} if
no valid entries were ever merged (or alternatively change the function to
return an error so callers can decide), update callers (e.g.,
responses_adapter.go usage of dst.ExtraFields) accordingly to handle either a
nil/unchanged result or an error.

---

Duplicate comments:
In `@internal/providers/anthropic/anthropic.go`:
- Around line 1696-1705: The reasoning block is being created eagerly which can
emit a placeholder thinking block before any real text; update
setReasoningSignature and the code paths that create reasoning blocks (e.g.,
where ensureResponsesReasoningState(), reasoningContentIndex(), and block append
logic are used) to defer creating the reasoning Block until you see the first
non-whitespace thinking_delta; specifically, only call
ensureResponsesReasoningState()/append a new Block when the incoming
thinking_delta contains non-whitespace content and then attach Signature with
setReasoningSignature only if the corresponding block exists (guard against
contentIndex >= len(state.Blocks) before setting Signature). Apply the same
lazy-creation guard to the other similar sites noted (around the blocks
indicated by the reviewer: ~1740-1755, ~1802-1807, ~1829-1848) so no placeholder
block is emitted for empty/whitespace thinking deltas.
🪄 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: ASSERTIVE

Plan: Pro

Run ID: 353c203d-7b48-4f27-a0b9-d4b09d7da98b

📥 Commits

Reviewing files that changed from the base of the PR and between d7263a5 and 26c9129.

📒 Files selected for processing (2)
  • internal/core/json_fields.go
  • internal/providers/anthropic/anthropic.go

Comment on lines +45 to +70
dec := json.NewDecoder(bytes.NewReader(fieldSet.raw))
tok, err := dec.Token()
if err != nil {
continue
}
delim, ok := tok.(json.Delim)
if !ok || delim != '{' {
continue
}

for dec.More() {
key, ok := readJSONObjectKey(dec)
if !ok {
return UnknownJSONFields{}
}

var value json.RawMessage
if err := dec.Decode(&value); err != nil {
return UnknownJSONFields{}
}

if merged == nil {
merged = make(map[string]json.RawMessage)
}
merged[key] = CloneRawJSON(value)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Inconsistent error handling causes silent data loss.

The error handling strategy is inconsistent:

  • Lines 47-52: Initial tokenization/delimiter errors use continue, preserving previously merged data
  • Lines 58 and 63: Mid-object parse errors use return UnknownJSONFields{}, discarding all accumulated data

If fieldSet #1 merges successfully but `fieldSet `#2 has a corrupt key/value mid-object, the function silently returns empty, losing fieldSet #1``'s data. The call sites in responses_adapter.go assign results directly to `dst.ExtraFields` without validation, so this data loss goes undetected.

Consider making error handling consistent—either skip corrupt field sets entirely (like initial errors) or return an error so callers can react:

Option A: Skip corrupt field sets consistently
 		for dec.More() {
 			key, ok := readJSONObjectKey(dec)
 			if !ok {
-				return UnknownJSONFields{}
+				break // skip this corrupt fieldSet, keep accumulated data
 			}

 			var value json.RawMessage
 			if err := dec.Decode(&value); err != nil {
-				return UnknownJSONFields{}
+				break // skip this corrupt fieldSet, keep accumulated data
 			}

 			if merged == nil {
 				merged = make(map[string]json.RawMessage)
 			}
 			merged[key] = CloneRawJSON(value)
 		}
Option B: Return error for caller awareness
-func MergeUnknownJSONFields(fields ...UnknownJSONFields) UnknownJSONFields {
+func MergeUnknownJSONFields(fields ...UnknownJSONFields) (UnknownJSONFields, error) {
 	if len(fields) == 0 {
-		return UnknownJSONFields{}
+		return UnknownJSONFields{}, nil
 	}
 // ... update error sites to return (UnknownJSONFields{}, err)
📝 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.

Suggested change
dec := json.NewDecoder(bytes.NewReader(fieldSet.raw))
tok, err := dec.Token()
if err != nil {
continue
}
delim, ok := tok.(json.Delim)
if !ok || delim != '{' {
continue
}
for dec.More() {
key, ok := readJSONObjectKey(dec)
if !ok {
return UnknownJSONFields{}
}
var value json.RawMessage
if err := dec.Decode(&value); err != nil {
return UnknownJSONFields{}
}
if merged == nil {
merged = make(map[string]json.RawMessage)
}
merged[key] = CloneRawJSON(value)
}
dec := json.NewDecoder(bytes.NewReader(fieldSet.raw))
tok, err := dec.Token()
if err != nil {
continue
}
delim, ok := tok.(json.Delim)
if !ok || delim != '{' {
continue
}
for dec.More() {
key, ok := readJSONObjectKey(dec)
if !ok {
break // skip this corrupt fieldSet, keep accumulated data
}
var value json.RawMessage
if err := dec.Decode(&value); err != nil {
break // skip this corrupt fieldSet, keep accumulated data
}
if merged == nil {
merged = make(map[string]json.RawMessage)
}
merged[key] = CloneRawJSON(value)
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/core/json_fields.go` around lines 45 - 70, The current parsing in
internal/core/json_fields.go inconsistently handles errors: initial
token/delimiter errors use continue but mid-object errors return
UnknownJSONFields{}, causing previously merged data (merged map) to be
discarded; change the mid-object error handling in the json decoding loop (where
readJSONObjectKey and dec.Decode(&value) are called) to mirror the initial
behavior by skipping the entire corrupt fieldSet instead of returning empty —
i.e., on failing readJSONObjectKey or dec.Decode, continue to the next fieldSet,
preserve merged data, and only return UnknownJSONFields{} if no valid entries
were ever merged (or alternatively change the function to return an error so
callers can decide), update callers (e.g., responses_adapter.go usage of
dst.ExtraFields) accordingly to handle either a nil/unchanged result or an
error.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@internal/providers/anthropic/request_translation.go`:
- Around line 207-210: When extracting Anthropic reasoning blocks via
extractAnthropicReasoningBlocksFromExtraFields(msg.ExtraFields), validate that
any presence of compat reasoning fields (reasoning_details / reasoning_content /
reasoning_signature) is only allowed for assistant turns when the Anthropic
compatibility flag is enabled; otherwise return a client-facing invalid request
error using core.NewInvalidRequestError (a core.GatewayError). Concretely: after
calling extractAnthropicReasoningBlocksFromExtraFields, if it found any compat
reasoning fields and msg.Role is not "assistant" OR the Anthropic compatibility
flag (the module/struct bool that controls compat translation) is false,
immediately fail with core.NewInvalidRequestError describing the unsupported
fields instead of continuing.

In `@internal/providers/responses_adapter.go`:
- Around line 470-475: When PreserveAnthropicReasoningCompat is true we still
must parse/validate message-level reasoning extras before allowing them through:
update the branch around containsAnthropicReasoningCompatFields to, even when
opts.PreserveAnthropicReasoningCompat is true, attempt to decode/validate the
assistant message extras keys
(reasoning_details/reasoning_content/reasoning_signature) using the same
validation used later in mergeReasoningIntoAssistantMessage and, on any
parse/validation failure, return a core.NewInvalidRequestError (i.e., a
GatewayError with category invalid_request_error) describing the invalid
reasoning payload; apply the same change to the other similar block referenced
(lines ~560-565) so malformed reasoning JSON is rejected at request validation
time rather than during merge.
🪄 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: ASSERTIVE

Plan: Pro

Run ID: 89c0c8b4-adb8-4c9f-b884-53a16918469d

📥 Commits

Reviewing files that changed from the base of the PR and between 26c9129 and 497b18c.

📒 Files selected for processing (9)
  • .env.template
  • README.md
  • docs/advanced/configuration.mdx
  • internal/providers/anthropic/anthropic.go
  • internal/providers/anthropic/anthropic_test.go
  • internal/providers/anthropic/reasoning_compat.go
  • internal/providers/anthropic/request_translation.go
  • internal/providers/responses_adapter.go
  • internal/providers/responses_adapter_test.go

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant