Skip to content

Commit 162200e

Browse files
HatriGtHatriGt
committed
fix(translator): address review feedback on Anthropic block normalization
- Preserve typed Anthropic server tools (e.g. web_search_20250305): only wrap tools that have NO "type" field as OpenAI function tools, so typed server tools are left intact for downstream mapping instead of being rewritten into a custom tool with no schema (codex review P2). - Use string accumulators with sjson.Set/SetRaw instead of []byte + SetBytes/SetRawBytes to avoid unnecessary allocations (gemini review). - Add TestNormalizeAnthropicRequestBlocks_TypedToolNotWrapped regression test.
1 parent 5ee0504 commit 162200e

2 files changed

Lines changed: 88 additions & 36 deletions

File tree

‎internal/translator/claude/openai/chat-completions/claude_openai_normalize_test.go‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,53 @@ func TestConvertOpenAIRequestToClaude_BareAnthropicTools(t *testing.T) {
139139
}
140140
}
141141

142+
func TestNormalizeAnthropicRequestBlocks_TypedToolNotWrapped(t *testing.T) {
143+
// Typed Anthropic server tools (type present, not "function") must NOT be
144+
// rewritten into OpenAI function tools by the normalizer; only bare tools
145+
// (no "type") are wrapped. This guards against corrupting server tools like
146+
// web_search_20250305 into a wrong custom tool with no schema.
147+
inputJSON := `{
148+
"model": "claude-sonnet-4-5",
149+
"messages": [{"role": "user", "content": "search"}],
150+
"tools": [
151+
{"type": "web_search_20250305", "name": "web_search", "max_uses": 5},
152+
{"name": "read_file", "description": "Read a file", "input_schema": {"type": "object"}}
153+
]
154+
}`
155+
156+
normalized := normalizeAnthropicRequestBlocks([]byte(inputJSON))
157+
normJSON := gjson.ParseBytes(normalized)
158+
tools := normJSON.Get("tools").Array()
159+
160+
if len(tools) != 2 {
161+
t.Fatalf("Expected 2 tools after normalize, got %d: %s", len(tools), normJSON.Get("tools").Raw)
162+
}
163+
164+
// Typed server tool kept verbatim (still its original type, not "function").
165+
typed := normJSON.Get(`tools.#(name=="web_search")`)
166+
if !typed.Exists() {
167+
t.Fatalf("Expected web_search tool preserved, got: %s", normJSON.Get("tools").Raw)
168+
}
169+
if got := typed.Get("type").String(); got != "web_search_20250305" {
170+
t.Fatalf("Expected typed tool type preserved as web_search_20250305, got %q (%s)", got, typed.Raw)
171+
}
172+
if got := typed.Get("max_uses").Int(); got != 5 {
173+
t.Fatalf("Expected typed tool fields preserved (max_uses=5), got %d", got)
174+
}
175+
176+
// Bare tool wrapped into OpenAI function shape.
177+
bare := normJSON.Get(`tools.#(function.name=="read_file")`)
178+
if !bare.Exists() {
179+
t.Fatalf("Expected read_file wrapped into function tool, got: %s", normJSON.Get("tools").Raw)
180+
}
181+
if got := bare.Get("type").String(); got != "function" {
182+
t.Fatalf("Expected wrapped tool type function, got %q", got)
183+
}
184+
if !bare.Get("function.parameters").Exists() {
185+
t.Fatalf("Expected wrapped tool parameters, got: %s", bare.Raw)
186+
}
187+
}
188+
142189
func TestConvertOpenAIRequestToClaude_StandardOpenAIUnchanged(t *testing.T) {
143190
// A normal OpenAI payload must pass through normalization untouched.
144191
inputJSON := `{

‎internal/translator/claude/openai/chat-completions/claude_openai_request.go‎

Lines changed: 41 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -358,13 +358,13 @@ func normalizeAnthropicRequestBlocks(rawJSON []byte) []byte {
358358

359359
// Rebuild messages array if any message carries Anthropic content blocks.
360360
if messages := root.Get("messages"); messages.Exists() && messages.IsArray() {
361-
newMessages := []byte("[]")
361+
newMessages := "[]"
362362
messages.ForEach(func(_, message gjson.Result) bool {
363363
role := message.Get("role").String()
364364
content := message.Get("content")
365365

366366
if !content.IsArray() || !messageHasAnthropicBlocks(content) {
367-
newMessages, _ = sjson.SetRawBytes(newMessages, "-1", []byte(message.Raw))
367+
newMessages, _ = sjson.SetRaw(newMessages, "-1", message.Raw)
368368
return true
369369
}
370370

@@ -390,29 +390,29 @@ func normalizeAnthropicRequestBlocks(rawJSON []byte) []byte {
390390
// tool_result blocks become standalone role:"tool" messages so
391391
// the downstream translator pairs them with the prior tool_calls.
392392
for _, tr := range toolResults {
393-
toolMsg := []byte(`{"role":"tool","tool_call_id":"","content":""}`)
394-
toolMsg, _ = sjson.SetBytes(toolMsg, "tool_call_id", tr.Get("tool_use_id").String())
393+
toolMsg := `{"role":"tool","tool_call_id":"","content":""}`
394+
toolMsg, _ = sjson.Set(toolMsg, "tool_call_id", tr.Get("tool_use_id").String())
395395
trContent := tr.Get("content")
396396
if trContent.IsArray() {
397-
toolMsg, _ = sjson.SetRawBytes(toolMsg, "content", []byte(trContent.Raw))
397+
toolMsg, _ = sjson.SetRaw(toolMsg, "content", trContent.Raw)
398398
} else {
399-
toolMsg, _ = sjson.SetBytes(toolMsg, "content", flattenAnthropicToolResultText(trContent))
399+
toolMsg, _ = sjson.Set(toolMsg, "content", flattenAnthropicToolResultText(trContent))
400400
}
401-
newMessages, _ = sjson.SetRawBytes(newMessages, "-1", toolMsg)
401+
newMessages, _ = sjson.SetRaw(newMessages, "-1", toolMsg)
402402
}
403403

404404
// Remaining text / passthrough parts stay as a user message.
405405
if len(textParts) > 0 || len(passthrough) > 0 {
406-
userMsg := []byte(`{"role":"user","content":[]}`)
406+
userMsg := `{"role":"user","content":[]}`
407407
for _, t := range textParts {
408-
textPart := []byte(`{"type":"text","text":""}`)
409-
textPart, _ = sjson.SetBytes(textPart, "text", t)
410-
userMsg, _ = sjson.SetRawBytes(userMsg, "content.-1", textPart)
408+
textPart := `{"type":"text","text":""}`
409+
textPart, _ = sjson.Set(textPart, "text", t)
410+
userMsg, _ = sjson.SetRaw(userMsg, "content.-1", textPart)
411411
}
412412
for _, p := range passthrough {
413-
userMsg, _ = sjson.SetRawBytes(userMsg, "content.-1", []byte(p.Raw))
413+
userMsg, _ = sjson.SetRaw(userMsg, "content.-1", p.Raw)
414414
}
415-
newMessages, _ = sjson.SetRawBytes(newMessages, "-1", userMsg)
415+
newMessages, _ = sjson.SetRaw(newMessages, "-1", userMsg)
416416
}
417417

418418
case "assistant":
@@ -430,60 +430,65 @@ func normalizeAnthropicRequestBlocks(rawJSON []byte) []byte {
430430
return true
431431
})
432432

433-
asstMsg := []byte(`{"role":"assistant","content":""}`)
434-
asstMsg, _ = sjson.SetBytes(asstMsg, "content", strings.Join(textParts, ""))
433+
asstMsg := `{"role":"assistant","content":""}`
434+
asstMsg, _ = sjson.Set(asstMsg, "content", strings.Join(textParts, ""))
435435
if len(toolUses) > 0 {
436-
asstMsg, _ = sjson.SetRawBytes(asstMsg, "tool_calls", []byte("[]"))
436+
asstMsg, _ = sjson.SetRaw(asstMsg, "tool_calls", "[]")
437437
for _, tu := range toolUses {
438-
toolCall := []byte(`{"id":"","type":"function","function":{"name":"","arguments":"{}"}}`)
439-
toolCall, _ = sjson.SetBytes(toolCall, "id", tu.Get("id").String())
440-
toolCall, _ = sjson.SetBytes(toolCall, "function.name", tu.Get("name").String())
438+
toolCall := `{"id":"","type":"function","function":{"name":"","arguments":"{}"}}`
439+
toolCall, _ = sjson.Set(toolCall, "id", tu.Get("id").String())
440+
toolCall, _ = sjson.Set(toolCall, "function.name", tu.Get("name").String())
441441
if input := tu.Get("input"); input.Exists() && input.IsObject() {
442-
toolCall, _ = sjson.SetBytes(toolCall, "function.arguments", input.Raw)
442+
toolCall, _ = sjson.Set(toolCall, "function.arguments", input.Raw)
443443
}
444-
asstMsg, _ = sjson.SetRawBytes(asstMsg, "tool_calls.-1", toolCall)
444+
asstMsg, _ = sjson.SetRaw(asstMsg, "tool_calls.-1", toolCall)
445445
}
446446
}
447-
newMessages, _ = sjson.SetRawBytes(newMessages, "-1", asstMsg)
447+
newMessages, _ = sjson.SetRaw(newMessages, "-1", asstMsg)
448448

449449
default:
450-
newMessages, _ = sjson.SetRawBytes(newMessages, "-1", []byte(message.Raw))
450+
newMessages, _ = sjson.SetRaw(newMessages, "-1", message.Raw)
451451
}
452452
return true
453453
})
454-
out, _ = sjson.SetRawBytes(out, "messages", newMessages)
454+
out, _ = sjson.SetRawBytes(out, "messages", []byte(newMessages))
455455
}
456456

457457
// Wrap bare Anthropic tool definitions into OpenAI function tools.
458+
// Only tools with NO "type" field are treated as bare custom tools. Typed
459+
// Anthropic server tools (e.g. {"type":"web_search_20250305","name":...})
460+
// are left untouched so the downstream mapper can handle them correctly.
458461
if tools := root.Get("tools"); tools.Exists() && tools.IsArray() && len(tools.Array()) > 0 {
459462
needsWrap := false
460463
tools.ForEach(func(_, tool gjson.Result) bool {
461-
if tool.Get("type").String() != "function" && tool.Get("name").Exists() {
464+
if !tool.Get("type").Exists() && tool.Get("name").Exists() {
462465
needsWrap = true
463466
return false
464467
}
465468
return true
466469
})
467470

468471
if needsWrap {
469-
newTools := []byte("[]")
472+
newTools := "[]"
470473
tools.ForEach(func(_, tool gjson.Result) bool {
471-
if tool.Get("type").String() == "function" {
472-
newTools, _ = sjson.SetRawBytes(newTools, "-1", []byte(tool.Raw))
474+
// Pass through anything that already has a type (OpenAI function
475+
// tools and typed Anthropic server tools) unchanged.
476+
if tool.Get("type").Exists() {
477+
newTools, _ = sjson.SetRaw(newTools, "-1", tool.Raw)
473478
return true
474479
}
475-
wrapped := []byte(`{"type":"function","function":{"name":"","description":""}}`)
476-
wrapped, _ = sjson.SetBytes(wrapped, "function.name", tool.Get("name").String())
477-
wrapped, _ = sjson.SetBytes(wrapped, "function.description", tool.Get("description").String())
480+
wrapped := `{"type":"function","function":{"name":"","description":""}}`
481+
wrapped, _ = sjson.Set(wrapped, "function.name", tool.Get("name").String())
482+
wrapped, _ = sjson.Set(wrapped, "function.description", tool.Get("description").String())
478483
if schema := tool.Get("input_schema"); schema.Exists() {
479-
wrapped, _ = sjson.SetRawBytes(wrapped, "function.parameters", []byte(schema.Raw))
484+
wrapped, _ = sjson.SetRaw(wrapped, "function.parameters", schema.Raw)
480485
} else if schema := tool.Get("parameters"); schema.Exists() {
481-
wrapped, _ = sjson.SetRawBytes(wrapped, "function.parameters", []byte(schema.Raw))
486+
wrapped, _ = sjson.SetRaw(wrapped, "function.parameters", schema.Raw)
482487
}
483-
newTools, _ = sjson.SetRawBytes(newTools, "-1", wrapped)
488+
newTools, _ = sjson.SetRaw(newTools, "-1", wrapped)
484489
return true
485490
})
486-
out, _ = sjson.SetRawBytes(out, "tools", newTools)
491+
out, _ = sjson.SetRawBytes(out, "tools", []byte(newTools))
487492
}
488493
}
489494

@@ -510,7 +515,7 @@ func anthropicBlocksPresent(root gjson.Result) bool {
510515
if tools := root.Get("tools"); tools.Exists() && tools.IsArray() {
511516
found := false
512517
tools.ForEach(func(_, tool gjson.Result) bool {
513-
if tool.Get("type").String() != "function" && tool.Get("name").Exists() {
518+
if !tool.Get("type").Exists() && tool.Get("name").Exists() {
514519
found = true
515520
return false
516521
}

0 commit comments

Comments
 (0)