feat: proxy transform layer correctness (018)#22
Merged
john-zhh merged 22 commits intofeat/v3.0.1from Mar 9, 2026
Merged
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ness - Add TDD test task T037 for US5 debugLogger verification - Convert edge case questions to behavioral specifications - Clarify FR-017 scope: remove unconditional logger, defer optional debug - Move optional debug logging to Out of Scope section - Standardize phase naming: Phase A-E → Phase 1-5 in plan.md - Update task numbering: T037-T043 after new US5 test insertion - Update all cross-references to new task numbers Fixes: C1 (Constitution TDD), A1 (Ambiguity), I1 (Inconsistency), I2 (Scope), U1 (Coverage) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Phase 1: Setup - Add FormatAnthropicMessages, FormatOpenAIChat, FormatOpenAIResponses constants - Update NeedsTransform to handle fine-grained format identifiers - Update detectClientFormat to return new format constants Phase 2: Foundational - Remove init() function and debugLogger from anthropic.go - Remove all debugLogger.Printf() call sites - Verify clean build Tasks: T001-T005 complete Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add TestDetectClientFormat with all three format constants - Add TestNeedsTransformWithNewFormats covering new format identifiers - Add TestStreamTransformerRouting for openai-chat vs openai-responses - Update StreamTransformer.TransformSSEStream routing logic Tasks: T006-T008 complete, T009 in progress Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tion Phase 3: User Story 1 (Protocol Routing) - Add transformAnthropicToOpenAIChat for Chat Completions format - Rename transformAnthropicToOpenAI to transformAnthropicToOpenAIResponses - Update StreamTransformer routing to distinguish openai-chat vs openai-responses - Update AnthropicTransformer to handle both OpenAI formats distinctly - Add transformToChatCompletions and transformToResponsesAPI methods - Update OpenAITransformer to use normalizeFormat for anthropic-messages Tasks: T009-T013 complete Checkpoint: Chat Completions and Responses API produce correct shapes Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add TestOpenAITransformer_ToolsTransformation for OpenAI→Anthropic tools - Add TestAnthropicTransformer_ToolUseToToolCalls for Anthropic→OpenAI tool_calls - Add TestStreamTransformer_AnthropicToolUseToOpenAIChat for streaming tool deltas - Add TestStreamTransformer_AnthropicToolUseToOpenAIResponses for Responses API Tasks: T014-T017 complete (tests written, expecting failures) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Phase 4: User Story 2 (Tool Calls) - Add json import to anthropic.go - Implement tool_use → tool_calls in transformToChatCompletions - Add content_block_start handling for tool_use in transformAnthropicEventToChat - Add input_json_delta handling for streaming tool arguments - Implement function_call_arguments.delta for Responses API - Fix test to use AnthropicTransformer instead of OpenAITransformer Tasks: T014-T023 complete (T022-T023 covered by T021 implementation) Checkpoint: Tool calls transform correctly in streaming and non-streaming modes Note: Some pre-existing tests failing, will fix in next commit Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Handle empty clientFormat as default (anthropic for AnthropicTransformer, openai for OpenAITransformer) - Default legacy "openai" clientFormat to Responses API for backward compatibility - Restore transformResponsesAPIToAnthropic routing branch - Move response.created initialization from content_block_start to message_start - Remove debug logging from tests Fixes: TestAnthropicTransformer_TransformRequest_NoTransform, TestStreamTransformer_AnthropicToOpenAI Remaining: TestTransformResponsesAPIToAnthropic tests still failing (investigation needed) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Problem: openai-responses was normalized to "openai", causing it to match the generic openai→anthropic route instead of the specific transformResponsesAPIToAnthropic route. Solution: Check specific ProviderFormat == FormatOpenAIResponses before normalized comparisons to ensure correct routing. Fixes: TestTransformResponsesAPIToAnthropic_Text, TestTransformResponsesAPIToAnthropic_ToolCall All transform tests now passing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Implements User Story 3 (T024-T031): - Add writeStreamError helper to emit protocol-native error events - Check scanner.Err() after all streaming loops - Emit error events instead of fake completions on stream errors - Support all three streaming paths: Anthropic→OpenAI Chat, Anthropic→OpenAI Responses, OpenAI→Anthropic Tests verify truncated streams emit error events, clean EOF emits completion. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Implements User Story 4 (T032-T036): - Define TransformError type to distinguish transform failures from provider errors - Return TransformError from forwardRequest when request transform fails - Handle response transform errors with HTTP 500 - Detect TransformError in tryProviders and skip provider health marking - Transform errors no longer penalize provider health Tests verify TransformError type exists and can be detected via errors.As. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Implements Phase 7 (US5) and Phase 8: - T037: Add test verifying no debugLogger references exist - T038: Verify no debugLogger in codebase (grep check passed) - T039: Verify transform.log not created (old file from previous runs) - T040: Test coverage 92.7% ✅ (≥ 80% target) - T041: No race conditions detected ✅ - T042: Clean build successful ✅ - T043: All changes committed ✅ All 43 tasks (T001-T043) completed across 8 phases. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…orms Fixes two blocking issues from code review: 1. TransformPath now uses NormalizeFormat to handle fine-grained formats - anthropic-messages -> openai now correctly transforms /v1/messages -> /v1/chat/completions - openai-chat/openai-responses -> anthropic now correctly transforms paths 2. copyResponseFromResponsesAPI now uses NormalizeFormat for format detection - Anthropic clients (anthropic-messages) now correctly receive Anthropic format responses - Both streaming and non-streaming Responses API transforms work correctly Changes: - Export normalizeFormat as NormalizeFormat for use in server.go - Add empty string handling in NormalizeFormat (defaults to "anthropic") - Update TransformPath to use NormalizeFormat for both client and provider formats - Update copyResponseFromResponsesAPI to use NormalizeFormat for format detection - Add comprehensive tests for TransformPath with fine-grained formats All tests passing ✅ Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…formation Addresses non-blocking issues from code review: 1. Transform error responses now return proper JSON with correct Content-Type - Use json.NewEncoder instead of http.Error with string formatting - Set Content-Type: application/json explicitly - Properly escape special characters in error messages - Add test to verify valid JSON output 2. Complete OpenAI tool_calls to Anthropic tool_use transformation - Transform tool_calls array to tool_use content blocks - Parse function arguments JSON and include in tool_use input - Support responses with both text content and tool_calls - Add comprehensive tests for tool_calls transformation Changes: - Update tryProviders to return proper JSON for TransformError - Update copyResponse to return proper JSON for response transform errors - Add tool_calls transformation in OpenAITransformer.TransformResponse - Add encoding/json import to openai.go - Add tests for JSON error format and tool_calls transformation All tests passing ✅ Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Addresses all blocking issues from final code review: 1. Complete Anthropic -> OpenAI request transformation - Transform tool_use blocks to OpenAI tool_calls format - Transform tool_result blocks to OpenAI tool role messages - Serialize tool input as JSON string in arguments field - Expand tool_result blocks into separate tool messages - Add comprehensive tests for tool_use and tool_result transformation 2. Complete OpenAI Chat SSE streaming tool_calls transformation - Transform streaming tool_calls delta to Anthropic content blocks - Emit content_block_start with tool_use type when tool call starts - Emit input_json_delta events for function arguments - Emit content_block_stop when tool call completes - Support mixed content (text followed by tool_calls) - Add tests for streaming tool_calls scenarios 3. Fix Codex client type detection - Prioritize path-based detection over client type - Allow Codex to use Responses API if path matches /responses - Only default to openai-chat for Codex when path is ambiguous - Add tests for Codex with different API paths Changes: - Add transformAnthropicMessageToOpenAI helper in openai.go - Add transformAssistantMessage for tool_use conversion - Add transformUserMessageWithToolResults for tool_result conversion - Add tool_calls delta handling in transformOpenAIChatToAnthropic - Reorder detectClientFormat to check path before client type - Add encoding/json import to openai.go - Add comprehensive tests for all three blocking issues All tests passing ✅ Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ate machine Addresses all blocking issues from final code review round 2: 1. Complete OpenAI -> Anthropic request transformation - Transform OpenAI system messages to Anthropic system field - Transform OpenAI assistant.tool_calls to Anthropic tool_use blocks - Transform OpenAI tool role messages to Anthropic tool_result blocks - Merge multiple system messages - Add comprehensive tests for full tool loop transformation 2. Fix streaming state machine reliability - Track current block state (index, type) instead of global boolean - Close previous block with correct index when switching blocks - Support text -> tool_use transitions correctly - Add blockState struct to track opened blocks 3. Fix streaming termination semantics - Avoid duplicate termination events from finish_reason and [DONE] - Store finalStopReason when finish_reason is received - Set messageStopped flag to prevent duplicate message_stop - Only send termination from [DONE] if not already stopped 4. Fix Anthropic -> OpenAI semantic preservation - Concatenate all text blocks instead of keeping only last one - Preserve text content mixed with tool_results - Add _anthropic_text_content marker for mixed messages - Emit user message before tool messages when text is present - Concatenate multiple text parts with newlines Changes: - Add transformOpenAIMessagesToAnthropic in anthropic.go - Add transformOpenAIAssistantMessage and transformOpenAIToolMessage - Refactor transformOpenAIToAnthropic with blockState tracking - Add finalStopReason and messageStopped flags - Update transformAssistantMessage to concatenate text blocks - Update transformUserMessageWithToolResults to preserve mixed content - Add comprehensive tests for all four blocking issues All tests passing ✅ Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fixes three blocking issues identified in Round 3 code review:
1. Parallel tool_calls OpenAI -> Anthropic merging:
- Changed transformOpenAIMessagesToAnthropic to accumulate consecutive
tool messages and merge them into a single user message with multiple
tool_result blocks
- Removed transformOpenAIToolMessage function (no longer needed)
- This ensures parallel tool calls produce correct Anthropic structure
2. Text/tool_result ordering preservation:
- Updated transformUserMessageWithToolResults to preserve original
content block ordering via _anthropic_content_blocks marker
- Modified expansion logic in TransformRequest to emit messages in
original order (text -> tool_result -> text)
- Prevents semantic reordering where tail text was moved before tool results
3. Streaming parallel tool calls support:
- Replaced single global currentBlock with blocksByIndex map to track
multiple parallel blocks
- Updated tool_calls delta handling to manage blocks by index
- Updated finish_reason and [DONE] handling to close all open blocks
- Enables correct handling of interleaved deltas from multiple tool calls
Updated test expectations to match new correct behavior that preserves
ordering instead of concatenating text blocks.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fixes the blocking issue where OpenAI tool_call indices were incorrectly used as Anthropic content block indices, causing index conflicts when text and tool blocks were mixed. Problem: - OpenAI tool_calls[].index represents position in tool_calls array - Anthropic content[].index represents position in entire content array - Previous implementation directly used OpenAI index as Anthropic index - This caused text (index=0) and first tool (index=0) to conflict Solution: - Introduced nextAnthropicIndex counter for global content block indexing - Separated tracking: toolBlocksByOpenAIIndex maps OpenAI indices to Anthropic block states with correct anthropicIndex - Text blocks now get sequential indices from the global counter - Tool blocks get sequential indices from the global counter - Result: text=0, tool=1, tool=2, etc. (correct Anthropic semantics) Added comprehensive test to verify: - Text block starts at index 0 - First tool block starts at index 1 - content_block_stop events use correct indices This ensures Claude can correctly reconstruct the streaming content structure without index conflicts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fixes the protocol issue where multiple content blocks could remain open simultaneously, violating Anthropic Messages SSE semantics. Problem: - When switching from text to tool, text block was not closed first - When switching from tool to text, tool blocks were not closed first - This allowed multiple blocks to be in "opened" state simultaneously - Anthropic Messages SSE expects strict "close-then-open" lifecycle Solution: - Before opening a tool block: close any open text block - Before opening a text block: close all open tool blocks - Ensures only one block is open at any time - Maintains strict sequential lifecycle: start → delta → stop → start Example correct sequence: content_block_start(text, index=0) content_block_delta(text, index=0) content_block_stop(index=0) // Close text first content_block_start(tool_use, index=1) // Then open tool content_block_delta(tool_use, index=1) content_block_stop(index=1) Updated test to verify lifecycle ordering: - Verifies text block is stopped before tool block starts - Checks event positions: start(text) < stop(text) < start(tool) - Ensures no overlapping block lifetimes This matches the expected consumption pattern for Anthropic Messages SSE and prevents protocol violations that could confuse stream consumers. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fixes the protocol issue where multiple parallel tool_use blocks could
remain open simultaneously, violating strict sequential lifecycle semantics.
Problem:
- When starting a new tool block, only closed the tool at the same OpenAI index
- Did not close other open tool blocks from different OpenAI indices
- This allowed multiple tool_use blocks to be open at the same time
- Example violation:
tool_calls[index=0] start (Anthropic index=1)
tool_calls[index=0] arguments delta
tool_calls[index=1] start (Anthropic index=2) // tool 0 still open!
Solution:
- Before opening any new tool block: close ALL open tool blocks
- Ensures strict sequential lifecycle even for parallel tool calls
- Only one content block (text or tool) is open at any given time
Correct sequence:
tool_calls[index=0] start (Anthropic index=1)
tool_calls[index=0] arguments delta
content_block_stop(index=1) // Close tool 0 first
tool_calls[index=1] start (Anthropic index=2) // Then open tool 1
tool_calls[index=1] arguments delta
content_block_stop(index=2)
Added comprehensive test for parallel tool calls:
- Verifies tool 0 is stopped before tool 1 starts
- Checks event positions: start(tool0) < stop(tool0) < start(tool1) < stop(tool1)
- Ensures no overlapping tool block lifetimes
- Verifies sequential Anthropic content indices
This ensures Anthropic Messages SSE consumers can rely on strict
sequential block lifecycle without handling concurrent open blocks.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…treams Fixes the critical bug where interleaved tool call deltas could be sent to already-closed content blocks, violating Anthropic SSE semantics. Problem: - When opening tool 1, all previous tool blocks (including tool 0) are closed - But toolBlocksByOpenAIIndex map still contains the closed block states - If subsequent deltas arrive for tool 0 (interleaved scenario), code would retrieve the closed block and send content_block_delta to its anthropicIndex - This violates Anthropic SSE: cannot send deltas to closed blocks Real-world interleaved scenario: tool 0 start (Anthropic index=1) tool 0 args part1 tool 1 start (Anthropic index=2) → closes tool 0 tool 1 args part1 tool 0 args part2 → BUG: sends delta to closed index=1! Solution: - When closing a tool block, delete it from toolBlocksByOpenAIIndex map - Subsequent deltas for that OpenAI tool index will find no block and be skipped - This prevents sending deltas to closed Anthropic content blocks Correct behavior: tool 0 start (Anthropic index=1) tool 0 args part1 → delta to index=1 tool 1 start (Anthropic index=2) → closes and removes tool 0 from map tool 1 args part1 → delta to index=2 tool 0 args part2 → no block found, delta skipped (correct!) Added comprehensive test for interleaved deltas: - Simulates real parallel tool call scenario with interleaved arguments - Verifies tool 0 only receives deltas before being closed - Verifies tool 1 receives all its deltas - Confirms no deltas sent to closed blocks This ensures safe handling of parallel/interleaved tool call streams that are common in real-world OpenAI Chat Completions SSE responses. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fixes the critical bug where late tool call deltas arriving after switching to text could still be sent to closed tool blocks. Problem: - When switching to text (line 752), all tool blocks are closed: started = false - But blocks remain in toolBlocksByOpenAIIndex map - When processing tool deltas (line 731), code only checks exists, not started - Late tool deltas are sent to closed anthropicIndex, violating SSE semantics Scenario: tool 0 start (Anthropic index=1) tool 0 args part1 text delta → closes tool 0, starts text (index=2) tool 0 args part2 → BUG: sends delta to closed index=1! Root cause: - Tool → tool transition: blocks are deleted from map (correct) - Tool → text transition: blocks are only marked started=false (incorrect) - Delta handler checks exists but not started (incorrect) Solution: - Add started check when sending tool deltas - Only send delta if block exists AND block.started == true - Late deltas for closed blocks are silently skipped Correct behavior: tool 0 start (Anthropic index=1) tool 0 args part1 → delta to index=1 text delta → closes tool 0, starts text (index=2) tool 0 args part2 → block exists but !started, delta skipped (correct!) text delta → delta to index=2 Added comprehensive test for tool → text → late tool delta: - Simulates tool starting, text interrupting, late tool delta arriving - Verifies tool block only receives deltas before text starts - Verifies text block receives all its deltas - Confirms late tool delta is NOT sent to closed tool block This completes the strict lifecycle enforcement for all transition paths: - tool → tool: closed blocks deleted from map - tool → text: closed blocks kept but marked !started, deltas skipped - text → tool: text block closed before tool starts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
1 participant
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.
Summary
Implements complete proxy transform layer correctness improvements across 5 user stories and 43 tasks.
Changes
User Story 1: Protocol-Aware Request Transformation (P0)
anthropic-messages,openai-chat,openai-responsesUser Story 2: Complete Tool Call Transformation (P0)
content_block_start(tool_use)+input_json_deltaUser Story 3: Safe SSE Stream Error Handling (P0)
writeStreamErrorhelper for protocol-native error eventsscanner.Err()after all streaming loopsUser Story 4: Transform Error Classification (P1)
TransformErrortype to distinguish transform failures from provider errorsUser Story 5: Remove Transform Hot Path Logging (P1)
debugLoggerreferences and file I/O from transform packageMetrics
Testing
All 43 tasks completed with TDD approach:
Backward Compatibility
Related Issues
Closes #18 (if exists)
Checklist