fix(anthropic): deterministic tool schema serialization for prompt caching - #2082
Merged
akshaydeo merged 1 commit intoMar 15, 2026
Merged
Conversation
|
|
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughNormalize and deterministically sort JSON Schema key ordering across Anthropic provider flows: tool InputSchema values are shallow-normalized, request payloads are marshaled with a sorted routine, top-level keys can be excluded without reordering nested content, and OrderedMap/ToolFunctionParameters gain sorting utilities. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
Contributor
|
❤️ for this @Edward-Upton - @Pratham-Mishra04 was going to look into this but thank you - ill review this in a bit and get in |
Contributor
|
one quick question I have here @Edward-Upton - We have noticed that, specifically with OpenAI - order of the keys (not sorted/unsorted but for example where is the required field for JSON response schema) matters a lot on how much model hallucinates. Apparently JSON parser in models is serialized parser so it does not get entire schema and considers that for response generation. Anything you observed with anthropic? |
Contributor
Author
Haven't noticed, but potentially not ran enough evals of this comparing with vs without. It was so expensive to run evals without this fix that I'm not sure we will even be able to compare. Is there any public information about ordering of keys we could try to replicate? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
core/schemas/chatcompletions.go (1)
417-419: Consider a nil guard inNormalizeKeyOrder.If this utility is ever called on a nil receiver, it will panic at Line 418. A tiny guard makes it safer for reuse.
Suggested patch
func (t *ToolFunctionParameters) NormalizeKeyOrder() { + if t == nil { + return + } t.keyOrder = JSONKeyOrder{} t.Properties.SortKeys()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/schemas/chatcompletions.go` around lines 417 - 419, Add a nil receiver guard at the start of ToolFunctionParameters.NormalizeKeyOrder to avoid panics when the method is called on a nil receiver: check if t == nil and return early; also defensively ensure t.Properties is non-nil before calling t.Properties.SortKeys() (and still reset t.keyOrder = JSONKeyOrder{} only when t is non-nil). This touches the method ToolFunctionParameters.NormalizeKeyOrder and the fields keyOrder and Properties and the JSONKeyOrder type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/providers/anthropic/chat.go`:
- Around line 137-139: The conversion in ToAnthropicChatRequest mutates the
original schema via anthropicTool.InputSchema.NormalizeKeyOrder(), which can
modify nested structures referenced from bifrostReq.Params.Tools[...] and causes
side effects; instead, deep-copy anthropicTool.InputSchema before calling
NormalizeKeyOrder so the converter remains pure—locate ToAnthropicChatRequest
and where it accesses anthropicTool.InputSchema, create a deep copy of the
schema object (preserving maps/slices) and call NormalizeKeyOrder on that copy,
then use the copied/normalized schema in the constructed Anthropic request,
ensuring bifrostReq.Params.Tools and the original anthropicTool remain
unchanged.
In `@core/providers/anthropic/responses.go`:
- Around line 4546-4548: The code calls
anthropicTool.InputSchema.NormalizeKeyOrder() in-place which can mutate a shared
pointer (e.g., tool.ResponsesToolFunction.Parameters) and leak state; replace
this by deep-copying the schema before normalizing: create a new schema instance
(clone/copy) from anthropicTool.InputSchema (or from
tool.ResponsesToolFunction.Parameters if that's the shared source), assign that
copy back to anthropicTool.InputSchema, then call NormalizeKeyOrder() on the
copy so the original caller-owned schema is not modified. Ensure the copy is a
full/deep copy of the schema structure to avoid shared references.
In `@core/providers/anthropic/utils.go`:
- Line 119: The branch that handles excludeFields re-marshals the modified map
with sonic.Marshal which can produce non-deterministic key order; replace that
sonic.Marshal call with providerUtils.MarshalSorted (the same deterministic
marshaller used where jsonBody is set) so that after you delete keys from
requestBody the final jsonBody is produced via
providerUtils.MarshalSorted(requestBody), and propagate/handle the returned
error similarly to the other branch (ensure variables like requestBody,
excludeFields, jsonBody and the error are used consistently).
---
Nitpick comments:
In `@core/schemas/chatcompletions.go`:
- Around line 417-419: Add a nil receiver guard at the start of
ToolFunctionParameters.NormalizeKeyOrder to avoid panics when the method is
called on a nil receiver: check if t == nil and return early; also defensively
ensure t.Properties is non-nil before calling t.Properties.SortKeys() (and still
reset t.keyOrder = JSONKeyOrder{} only when t is non-nil). This touches the
method ToolFunctionParameters.NormalizeKeyOrder and the fields keyOrder and
Properties and the JSONKeyOrder type.
🪄 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: f43517c4-a3a6-4944-951e-59191cf649cd
📒 Files selected for processing (5)
core/providers/anthropic/chat.gocore/providers/anthropic/responses.gocore/providers/anthropic/utils.gocore/schemas/chatcompletions.gocore/schemas/orderedmap.go
| } | ||
| } | ||
| jsonBody, err = sonic.Marshal(requestBody) | ||
| jsonBody, err = providerUtils.MarshalSorted(requestBody) |
Contributor
There was a problem hiding this comment.
Apply deterministic re-marshal in the exclude-fields branch too.
Line 119 fixes raw-body determinism, but Line 174 still re-marshals a map[string]interface{} via sonic.Marshal after deletions. That branch can still destabilize key order when excludeFields is non-empty, weakening cache-key reuse.
Suggested patch
@@
- // Re-marshal the map
- jsonBody, err = sonic.Marshal(jsonMap)
+ // Re-marshal deterministically to preserve stable cache keys
+ jsonBody, err = providerUtils.MarshalSorted(jsonMap)
if err != nil {
return nil, providerUtils.NewBifrostOperationError(schemas.ErrProviderRequestMarshal, err, providerName)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@core/providers/anthropic/utils.go` at line 119, The branch that handles
excludeFields re-marshals the modified map with sonic.Marshal which can produce
non-deterministic key order; replace that sonic.Marshal call with
providerUtils.MarshalSorted (the same deterministic marshaller used where
jsonBody is set) so that after you delete keys from requestBody the final
jsonBody is produced via providerUtils.MarshalSorted(requestBody), and
propagate/handle the returned error similarly to the other branch (ensure
variables like requestBody, excludeFields, jsonBody and the error are used
consistently).
akshaydeo
approved these changes
Mar 15, 2026
Contributor
There was a problem hiding this comment.
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 `@core/providers/anthropic/chat.go`:
- Around line 1091-1097: deepCopyToolFunctionParameters is using encoding/json's
json.Marshal and json.Unmarshal (variables p and copied of type
*schemas.ToolFunctionParameters); replace those calls with
github.com/bytedance/sonic's sonic.Marshal and sonic.Unmarshal for performance
in the hot path, and update imports to remove encoding/json and ensure sonic is
imported; keep the same error handling and return values while marshaling p with
sonic.Marshal and unmarshaling into copied with sonic.Unmarshal.
🪄 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: 5bc92db0-4f1a-4acd-8e71-f4e81caa2776
📒 Files selected for processing (5)
core/providers/anthropic/chat.gocore/providers/anthropic/responses.gocore/providers/anthropic/utils.gocore/schemas/chatcompletions.gocore/schemas/orderedmap.go
🚧 Files skipped from review as they are similar to previous changes (3)
- core/providers/anthropic/utils.go
- core/schemas/chatcompletions.go
- core/providers/anthropic/responses.go
Edward-Upton
force-pushed
the
ed/fix-anthropic-cache-determinism
branch
from
March 15, 2026 07:50
c50d745 to
c9a1b09
Compare
Contributor
There was a problem hiding this comment.
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 `@core/schemas/orderedmap.go`:
- Around line 342-349: The loop only recurses into *OrderedMap and
[]interface{}, so nested plain map[string]interface{} values (e.g., produced by
OrderedMapFromMap) are left unsorted; update the recursion to also detect
map[string]interface{} (and slices containing map[string]interface{}) and either
convert them to OrderedMap and call SortKeys() or call a helper that sorts those
plain maps in-place; specifically extend the cases in the loop over om.values
and in sortOrderedMapsInSlice to handle map[string]interface{} (and
[]map[string]interface{} / []interface{} elements that are maps) by invoking
OrderedMapFromMap(...) followed by SortKeys() or by delegating to a new
recursive function that sorts plain maps, and apply the same change to the other
occurrences of the same pattern (the other loops that currently only check
*OrderedMap and []interface{}).
🪄 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: d0612799-578d-469d-8e3f-41ba50fbd081
📒 Files selected for processing (5)
core/providers/anthropic/chat.gocore/providers/anthropic/responses.gocore/providers/anthropic/utils.gocore/schemas/chatcompletions.gocore/schemas/orderedmap.go
🚧 Files skipped from review as they are similar to previous changes (3)
- core/providers/anthropic/utils.go
- core/providers/anthropic/chat.go
- core/providers/anthropic/responses.go
Edward-Upton
force-pushed
the
ed/fix-anthropic-cache-determinism
branch
from
March 15, 2026 08:19
c9a1b09 to
4c2eebe
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
core/providers/anthropic/utils.go (1)
1641-1654: Usesonicinstead ofencoding/jsonfor marshaling/unmarshaling.The function correctly preserves nested byte ordering via
json.RawMessage, but usesencoding/jsonfor marshal/unmarshal operations. Since this is called in the request body construction path, it should usesonicfor performance per coding guidelines. Thejson.RawMessagetype can remain fromencoding/jsonas it's just a type alias for[]byte.♻️ Proposed refactor to use sonic
// excludeTopLevelJSONKeys removes the specified top-level keys from a JSON // object without parsing nested values. Nested bytes are preserved as-is, // which keeps the original key ordering within tool schemas and other // deeply nested structures. func excludeTopLevelJSONKeys(data []byte, keys []string) ([]byte, error) { var raw map[string]json.RawMessage - if err := json.Unmarshal(data, &raw); err != nil { + if err := sonic.Unmarshal(data, &raw); err != nil { return nil, fmt.Errorf("anthropic: unmarshalling for field exclusion: %w", err) } for _, k := range keys { delete(raw, k) } - return json.Marshal(raw) + return sonic.Marshal(raw) }As per coding guidelines: "JSON marshaling in hot paths must use github.com/bytedance/sonic for performance, not encoding/json"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/providers/anthropic/utils.go` around lines 1641 - 1654, The excludeTopLevelJSONKeys function currently uses encoding/json for Unmarshal/Marshal; switch to github.com/bytedance/sonic's Unmarshal and Marshal for performance: replace json.Unmarshal(data, &raw) with sonic.Unmarshal(data, &raw) and json.Marshal(raw) with sonic.Marshal(raw), keep using json.RawMessage as the byte alias, and update imports to include sonic while removing direct encoding/json Marshal/Unmarshal usages; ensure error wrapping and return semantics remain identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@core/providers/anthropic/utils.go`:
- Around line 1641-1654: The excludeTopLevelJSONKeys function currently uses
encoding/json for Unmarshal/Marshal; switch to github.com/bytedance/sonic's
Unmarshal and Marshal for performance: replace json.Unmarshal(data, &raw) with
sonic.Unmarshal(data, &raw) and json.Marshal(raw) with sonic.Marshal(raw), keep
using json.RawMessage as the byte alias, and update imports to include sonic
while removing direct encoding/json Marshal/Unmarshal usages; ensure error
wrapping and return semantics remain identical.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b05347dd-4398-445f-a025-e530c024fdf2
📒 Files selected for processing (6)
core/providers/anthropic/chat.gocore/providers/anthropic/responses.gocore/providers/anthropic/utils.gocore/schemas/chatcompletions.gocore/schemas/orderedmap.gocore/schemas/orderedmap_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- core/providers/anthropic/chat.go
- core/providers/anthropic/responses.go
…ching
Anthropic's prompt caching is prefix-based — when clients send tool
definitions with non-deterministic JSON key ordering across turns, the
cache key changes every turn, causing repeated cache writes instead of
reads.
Changes:
- Add OrderedMap.SortKeys() with JSON Schema priority ordering (type,
description, properties, required first, then alphabetically) so LLMs
see type information before descriptions and constraints.
- Add OrderedMap.SortedCopy() which returns a new tree with sorted keys
sharing primitive values — avoids expensive Marshal/Unmarshal cloning.
- Add ToolFunctionParameters.Normalized() that returns a shallow copy
with all nested OrderedMaps sorted via SortedCopy, replacing the old
Clone() + NormalizeKeyOrder() pattern.
- Call Normalized() in both Chat Completions and Responses API Anthropic
converters before serialization.
- Replace map[string]interface{} roundtrip in excludeFields path with
map[string]json.RawMessage to preserve nested byte ordering.
- Passthrough mode uses MarshalSorted for deterministic map key output.
Made-with: Cursor
Edward-Upton
force-pushed
the
ed/fix-anthropic-cache-determinism
branch
from
March 15, 2026 08:32
4c2eebe to
92aa3ab
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
core/providers/anthropic/utils.go (1)
1646-1655:⚠️ Potential issue | 🟠 MajorUse deterministic marshal after top-level key exclusion.
Line 1654 uses
sonic.Marshal(raw)on a Go map, so top-level key order can still vary between requests and weaken cache-key reuse whenexcludeFieldsis active.Suggested patch
func excludeTopLevelJSONKeys(data []byte, keys []string) ([]byte, error) { var raw map[string]json.RawMessage if err := sonic.Unmarshal(data, &raw); err != nil { return nil, fmt.Errorf("anthropic: unmarshalling for field exclusion: %w", err) } for _, k := range keys { delete(raw, k) } - return sonic.Marshal(raw) + // Keep nested RawMessage bytes intact, but sort top-level keys deterministically. + return providerUtils.MarshalSorted(raw) }#!/bin/bash # Verify current implementation in excludeTopLevelJSONKeys still uses non-deterministic marshal. rg -n 'func excludeTopLevelJSONKeys|return sonic.Marshal|MarshalSorted' core/providers/anthropic/utils.go -A5 -B5🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/providers/anthropic/utils.go` around lines 1646 - 1655, The function excludeTopLevelJSONKeys currently marshals the map with sonic.Marshal which can produce non-deterministic top-level key ordering; update excludeTopLevelJSONKeys to produce a deterministic JSON encoding by using a stable serializer (e.g., sonic.MarshalSorted or otherwise sorting the top-level keys before marshaling) so the returned byte slice is stable across runs when keys are excluded; locate the function excludeTopLevelJSONKeys and replace the sonic.Marshal(raw) call with a deterministic marshal call (MarshalSorted or manual key-sorting + marshal) and preserve the existing error wrapping behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/schemas/chatcompletions.go`:
- Around line 422-463: Normalized() currently only shallow-copies
ToolFunctionParameters.Default which leaves embedded maps/arrays with
non-deterministic key/item order; update ToolFunctionParameters.Normalized to
detect when Default is object-shaped (e.g., an OrderedMap or map-like value) or
array-shaped and produce a normalized copy: for object-shaped defaults call
SortedCopy (or the equivalent OrderedMap.SortedCopy) and assign the result to
out.Default, and for array-shaped defaults iterate items and SortedCopy any
map-like elements into a new slice before assigning to out.Default so serialized
defaults become deterministic; modify the Normalized method in
ToolFunctionParameters to perform these transformations alongside the existing
Properties/Defs/Items handling.
---
Duplicate comments:
In `@core/providers/anthropic/utils.go`:
- Around line 1646-1655: The function excludeTopLevelJSONKeys currently marshals
the map with sonic.Marshal which can produce non-deterministic top-level key
ordering; update excludeTopLevelJSONKeys to produce a deterministic JSON
encoding by using a stable serializer (e.g., sonic.MarshalSorted or otherwise
sorting the top-level keys before marshaling) so the returned byte slice is
stable across runs when keys are excluded; locate the function
excludeTopLevelJSONKeys and replace the sonic.Marshal(raw) call with a
deterministic marshal call (MarshalSorted or manual key-sorting + marshal) and
preserve the existing error wrapping behavior.
🪄 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: c0e7be63-3783-45f9-b9a4-bb33cb04aa09
📒 Files selected for processing (6)
core/providers/anthropic/chat.gocore/providers/anthropic/responses.gocore/providers/anthropic/utils.gocore/schemas/chatcompletions.gocore/schemas/orderedmap.gocore/schemas/orderedmap_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- core/providers/anthropic/chat.go
- core/schemas/orderedmap_test.go
- core/providers/anthropic/responses.go
Comment on lines
+422
to
+463
| func (t *ToolFunctionParameters) Normalized() *ToolFunctionParameters { | ||
| if t == nil { | ||
| return nil | ||
| } | ||
| out := *t | ||
| out.keyOrder = JSONKeyOrder{} | ||
| out.Properties = t.Properties.SortedCopy() | ||
| out.Defs = t.Defs.SortedCopy() | ||
| out.Definitions = t.Definitions.SortedCopy() | ||
| out.Items = t.Items.SortedCopy() | ||
| if len(t.AnyOf) > 0 { | ||
| out.AnyOf = make([]OrderedMap, len(t.AnyOf)) | ||
| for i := range t.AnyOf { | ||
| if cp := t.AnyOf[i].SortedCopy(); cp != nil { | ||
| out.AnyOf[i] = *cp | ||
| } | ||
| } | ||
| } | ||
| if len(t.OneOf) > 0 { | ||
| out.OneOf = make([]OrderedMap, len(t.OneOf)) | ||
| for i := range t.OneOf { | ||
| if cp := t.OneOf[i].SortedCopy(); cp != nil { | ||
| out.OneOf[i] = *cp | ||
| } | ||
| } | ||
| } | ||
| if len(t.AllOf) > 0 { | ||
| out.AllOf = make([]OrderedMap, len(t.AllOf)) | ||
| for i := range t.AllOf { | ||
| if cp := t.AllOf[i].SortedCopy(); cp != nil { | ||
| out.AllOf[i] = *cp | ||
| } | ||
| } | ||
| } | ||
| if t.AdditionalProperties != nil && t.AdditionalProperties.AdditionalPropertiesMap != nil { | ||
| out.AdditionalProperties = &AdditionalPropertiesStruct{ | ||
| AdditionalPropertiesBool: t.AdditionalProperties.AdditionalPropertiesBool, | ||
| AdditionalPropertiesMap: t.AdditionalProperties.AdditionalPropertiesMap.SortedCopy(), | ||
| } | ||
| } | ||
| return &out | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify `Normalized()` does not currently normalize `Default`.
rg -n 'type ToolFunctionParameters struct|Default interface|func \(t \*ToolFunctionParameters\) Normalized|out\.Default' core/schemas/chatcompletions.go -A80 -B20Repository: maximhq/bifrost
Length of output: 9719
🏁 Script executed:
#!/bin/bash
# Find where Normalized() is called
rg -n '\.Normalized\(\)' core/ --type goRepository: maximhq/bifrost
Length of output: 279
🏁 Script executed:
#!/bin/bash
# Check if there are any examples or tests that use Default with map/array values
rg -n 'Default.*:=' core/schemas/ -A2 -B2 --type go | head -50Repository: maximhq/bifrost
Length of output: 302
🏁 Script executed:
#!/bin/bash
# Look at OrderedMap and understand how SortedCopy works
rg -n 'func.*SortedCopy|type OrderedMap' core/schemas/ --type go -A5 | head -80Repository: maximhq/bifrost
Length of output: 1155
🏁 Script executed:
#!/bin/bash
# Look at the full SortedCopy implementation
rg -n 'func.*SortedCopy' core/schemas/orderedmap.go -A30Repository: maximhq/bifrost
Length of output: 912
🏁 Script executed:
#!/bin/bash
# Check if Default field is ever populated with maps in tests or examples
rg -n '"default"' core/schemas/chatcompletions.go -B2 -A2Repository: maximhq/bifrost
Length of output: 211
🏁 Script executed:
#!/bin/bash
# Look at how OrderedMapFromMap works
rg -n 'func OrderedMapFromMap' core/schemas/ --type go -A20Repository: maximhq/bifrost
Length of output: 1181
🏁 Script executed:
#!/bin/bash
# Check the Anthropic provider usage to understand the context
rg -n 'InputSchema.*Normalized' core/providers/anthropic/ -B5 -A5 --type goRepository: maximhq/bifrost
Length of output: 1699
🏁 Script executed:
#!/bin/bash
# Get the complete SortedCopy implementation
sed -n '376,430p' core/schemas/orderedmap.goRepository: maximhq/bifrost
Length of output: 1358
🏁 Script executed:
#!/bin/bash
# Look for actual usage of Default field with structured values in tests
rg -n 'Default.*map\[|Default.*\[\]|Default.*{' core/ --type go | head -20Repository: maximhq/bifrost
Length of output: 200
🏁 Script executed:
#!/bin/bash
# Check JSON Schema specs - Default field can be any JSON value
# Let's see if there are any existing helpers for normalizing arbitrary values
rg -n 'func normalize' core/schemas/ --type goRepository: maximhq/bifrost
Length of output: 405
🏁 Script executed:
#!/bin/bash
# Check the normalizeForSortedMarshal function - it may already handle this
rg -n 'func normalizeForSortedMarshal' core/schemas/ -A20 --type goRepository: maximhq/bifrost
Length of output: 2385
🏁 Script executed:
#!/bin/bash
# Look for actual test cases that use Default with complex values
rg -n '"default"' core/schemas/chatcompletions_test.go -B3 -A3Repository: maximhq/bifrost
Length of output: 135
🏁 Script executed:
#!/bin/bash
# Check if there's a stack of related PRs or if this is localized
git log --oneline -20Repository: maximhq/bifrost
Length of output: 142
🏁 Script executed:
#!/bin/bash
# Check what the recent commit actually changed
git show --stat 92aa3ab2 | head -50Repository: maximhq/bifrost
Length of output: 2875
🏁 Script executed:
#!/bin/bash
# Check if there are any tests that populate Default with map or array values
find core -name "*.go" -type f -exec grep -l "Default" {} \; | xargs grep -l "map\|OrderedMap" | head -5Repository: maximhq/bifrost
Length of output: 211
🏁 Script executed:
#!/bin/bash
# Look at the actual diff of the recent commit to understand what was normalized
git show 92aa3ab2 core/schemas/chatcompletions.go 2>/dev/null | head -100Repository: maximhq/bifrost
Length of output: 4471
🏁 Script executed:
#!/bin/bash
# Check if Default is actually used with complex values anywhere
rg -n 'Default:' core/ --type go -B2 -A2 | head -40Repository: maximhq/bifrost
Length of output: 2478
🏁 Script executed:
#!/bin/bash
# Confirm the patch would compile and check if similar patterns are already tested
rg -n 'sortedCopySlice\|normalizeForSortedMarshal' core/schemas/ --type go -B2 -A10 | head -60Repository: maximhq/bifrost
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Check if there's any test that verifies Normalized() determinism
rg -n 'Normalized.*deterministic\|deterministic.*Normalized' core/ --type go -B3 -A3Repository: maximhq/bifrost
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Verify that OrderedMapFromMap + SortedCopy pattern is sound
rg -n 'OrderedMapFromMap.*SortedCopy' core/ --type goRepository: maximhq/bifrost
Length of output: 223
Normalize Default when it is object/array-shaped.
Normalized() currently leaves Default untouched (shallow copy only). If default contains maps, serialization can still vary by key order, so normalized schemas are not fully deterministic.
Suggested patch
func (t *ToolFunctionParameters) Normalized() *ToolFunctionParameters {
if t == nil {
return nil
}
out := *t
out.keyOrder = JSONKeyOrder{}
out.Properties = t.Properties.SortedCopy()
out.Defs = t.Defs.SortedCopy()
out.Definitions = t.Definitions.SortedCopy()
out.Items = t.Items.SortedCopy()
@@
if t.AdditionalProperties != nil && t.AdditionalProperties.AdditionalPropertiesMap != nil {
out.AdditionalProperties = &AdditionalPropertiesStruct{
AdditionalPropertiesBool: t.AdditionalProperties.AdditionalPropertiesBool,
AdditionalPropertiesMap: t.AdditionalProperties.AdditionalPropertiesMap.SortedCopy(),
}
}
+ out.Default = normalizeDeterministicSchemaValue(t.Default)
return &out
}
+
+func normalizeDeterministicSchemaValue(v interface{}) interface{} {
+ switch x := v.(type) {
+ case *OrderedMap:
+ return x.SortedCopy()
+ case map[string]interface{}:
+ return OrderedMapFromMap(x).SortedCopy()
+ case []interface{}:
+ out := make([]interface{}, len(x))
+ for i := range x {
+ out[i] = normalizeDeterministicSchemaValue(x[i])
+ }
+ return out
+ default:
+ return v
+ }
+}📝 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
| func (t *ToolFunctionParameters) Normalized() *ToolFunctionParameters { | |
| if t == nil { | |
| return nil | |
| } | |
| out := *t | |
| out.keyOrder = JSONKeyOrder{} | |
| out.Properties = t.Properties.SortedCopy() | |
| out.Defs = t.Defs.SortedCopy() | |
| out.Definitions = t.Definitions.SortedCopy() | |
| out.Items = t.Items.SortedCopy() | |
| if len(t.AnyOf) > 0 { | |
| out.AnyOf = make([]OrderedMap, len(t.AnyOf)) | |
| for i := range t.AnyOf { | |
| if cp := t.AnyOf[i].SortedCopy(); cp != nil { | |
| out.AnyOf[i] = *cp | |
| } | |
| } | |
| } | |
| if len(t.OneOf) > 0 { | |
| out.OneOf = make([]OrderedMap, len(t.OneOf)) | |
| for i := range t.OneOf { | |
| if cp := t.OneOf[i].SortedCopy(); cp != nil { | |
| out.OneOf[i] = *cp | |
| } | |
| } | |
| } | |
| if len(t.AllOf) > 0 { | |
| out.AllOf = make([]OrderedMap, len(t.AllOf)) | |
| for i := range t.AllOf { | |
| if cp := t.AllOf[i].SortedCopy(); cp != nil { | |
| out.AllOf[i] = *cp | |
| } | |
| } | |
| } | |
| if t.AdditionalProperties != nil && t.AdditionalProperties.AdditionalPropertiesMap != nil { | |
| out.AdditionalProperties = &AdditionalPropertiesStruct{ | |
| AdditionalPropertiesBool: t.AdditionalProperties.AdditionalPropertiesBool, | |
| AdditionalPropertiesMap: t.AdditionalProperties.AdditionalPropertiesMap.SortedCopy(), | |
| } | |
| } | |
| return &out | |
| } | |
| func (t *ToolFunctionParameters) Normalized() *ToolFunctionParameters { | |
| if t == nil { | |
| return nil | |
| } | |
| out := *t | |
| out.keyOrder = JSONKeyOrder{} | |
| out.Properties = t.Properties.SortedCopy() | |
| out.Defs = t.Defs.SortedCopy() | |
| out.Definitions = t.Definitions.SortedCopy() | |
| out.Items = t.Items.SortedCopy() | |
| if len(t.AnyOf) > 0 { | |
| out.AnyOf = make([]OrderedMap, len(t.AnyOf)) | |
| for i := range t.AnyOf { | |
| if cp := t.AnyOf[i].SortedCopy(); cp != nil { | |
| out.AnyOf[i] = *cp | |
| } | |
| } | |
| } | |
| if len(t.OneOf) > 0 { | |
| out.OneOf = make([]OrderedMap, len(t.OneOf)) | |
| for i := range t.OneOf { | |
| if cp := t.OneOf[i].SortedCopy(); cp != nil { | |
| out.OneOf[i] = *cp | |
| } | |
| } | |
| } | |
| if len(t.AllOf) > 0 { | |
| out.AllOf = make([]OrderedMap, len(t.AllOf)) | |
| for i := range t.AllOf { | |
| if cp := t.AllOf[i].SortedCopy(); cp != nil { | |
| out.AllOf[i] = *cp | |
| } | |
| } | |
| } | |
| if t.AdditionalProperties != nil && t.AdditionalProperties.AdditionalPropertiesMap != nil { | |
| out.AdditionalProperties = &AdditionalPropertiesStruct{ | |
| AdditionalPropertiesBool: t.AdditionalProperties.AdditionalPropertiesBool, | |
| AdditionalPropertiesMap: t.AdditionalProperties.AdditionalPropertiesMap.SortedCopy(), | |
| } | |
| } | |
| out.Default = normalizeDeterministicSchemaValue(t.Default) | |
| return &out | |
| } | |
| func normalizeDeterministicSchemaValue(v interface{}) interface{} { | |
| switch x := v.(type) { | |
| case *OrderedMap: | |
| return x.SortedCopy() | |
| case map[string]interface{}: | |
| return OrderedMapFromMap(x).SortedCopy() | |
| case []interface{}: | |
| out := make([]interface{}, len(x)) | |
| for i := range x { | |
| out[i] = normalizeDeterministicSchemaValue(x[i]) | |
| } | |
| return out | |
| default: | |
| return v | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@core/schemas/chatcompletions.go` around lines 422 - 463, Normalized()
currently only shallow-copies ToolFunctionParameters.Default which leaves
embedded maps/arrays with non-deterministic key/item order; update
ToolFunctionParameters.Normalized to detect when Default is object-shaped (e.g.,
an OrderedMap or map-like value) or array-shaped and produce a normalized copy:
for object-shaped defaults call SortedCopy (or the equivalent
OrderedMap.SortedCopy) and assign the result to out.Default, and for
array-shaped defaults iterate items and SortedCopy any map-like elements into a
new slice before assigning to out.Default so serialized defaults become
deterministic; modify the Normalized method in ToolFunctionParameters to perform
these transformations alongside the existing Properties/Defs/Items handling.
18 tasks
occcat
pushed a commit
to occcat/bifrost
that referenced
this pull request
Sep 2, 2026
…ching (maximhq#2082) Anthropic's prompt caching is prefix-based — when clients send tool definitions with non-deterministic JSON key ordering across turns, the cache key changes every turn, causing repeated cache writes instead of reads. Changes: - Add OrderedMap.SortKeys() with JSON Schema priority ordering (type, description, properties, required first, then alphabetically) so LLMs see type information before descriptions and constraints. - Add OrderedMap.SortedCopy() which returns a new tree with sorted keys sharing primitive values — avoids expensive Marshal/Unmarshal cloning. - Add ToolFunctionParameters.Normalized() that returns a shallow copy with all nested OrderedMaps sorted via SortedCopy, replacing the old Clone() + NormalizeKeyOrder() pattern. - Call Normalized() in both Chat Completions and Responses API Anthropic converters before serialization. - Replace map[string]interface{} roundtrip in excludeFields path with map[string]json.RawMessage to preserve nested byte ordering. - Passthrough mode uses MarshalSorted for deterministic map key output. Made-with: Cursor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Anthropic's prompt caching is prefix-based — it derives a cache key from the serialized request. When clients send tool definitions with JSON object fields (e.g.
properties,$defs), the key ordering within those objects can vary non-deterministically between turns.Because tool definitions are part of the request prefix, this means the cache key changes every turn, causing repeated cache writes instead of cache reads. In practice, only the system prompt portion (~16k tokens) was ever cache-hit; the full tool schema (~25k tokens) was re-written on every single LLM call.
Impact
We measured this across multiple production agent runs (80+ LLM turns each). After the fix:
Cache read tokens went from plateauing early to growing correctly with each turn, confirming the prefix cache is working as intended.
Fix
Ensures all tool schema JSON is serialized with deterministic, LLM-optimal key ordering across all code paths:
OrderedMap.SortKeys()— sorts keys using JSON Schema priority ordering (type,description,properties,requiredfirst, then remaining keys alphabetically). This ensures LLMs see type information before descriptions and constraints in nested tool schemas.OrderedMap.SortedCopy()— returns a new OrderedMap tree with sorted keys, sharing primitive values with the original. This is much cheaper than a full Marshal/Unmarshal clone since it only allocates new key slices and value maps.ToolFunctionParameters.Normalized()— returns a shallow struct copy with all nested OrderedMaps sorted viaSortedCopy(). Replaces the oldClone()+NormalizeKeyOrder()pattern, avoiding a full JSON roundtrip per tool per request.Chat Completions + Responses API — both Anthropic converters call
Normalized()on tool input schemas before serialization.excludeTopLevelJSONKeys()— the Responses APIexcludeFieldspath now usesmap[string]json.RawMessageinstead ofmap[string]interface{}, preserving nested byte ordering instead of re-sorting everything alphabetically.Passthrough mode — uses
MarshalSortedfor deterministic map key output when forwarding raw request bodies.Test plan
core/module compilesmake test-core PROVIDER=anthropic)