Skip to content

Commit ef2b6ba

Browse files
committed
[Feat][Router] Migrate custom Chat Completions structs to official SDK types
- Replace 21 custom Chat Completions structs across pkg/responseapi, pkg/classification, pkg/modelselection, and pkg/mcp with official Go SDK types for protocol compatibility - Use composition for router-specific extensions (vLLM extra_body, provider error wrapping) per API Type Contracts guardrail - Handle multimodal content (text + image) via SDK union types - Preserve ToolChoice forwarding through SDK type mapping - Fix API error detection in benchmark runner: separate error envelope check from SDK deserialization (ChatCompletion.UnmarshalJSON ignores extra fields) - Add warnings for silently dropped image content and malformed history - Add 67 compatibility tests including unit, round-trip mock backend, and wire-format compliance tests across all affected packages - Add API Type Contracts guardrail to architecture-guardrails.md - Track deferred pkg/cache migration as tech debt Signed-off-by: asaadbalum <asaad.balum@gmail.com>
1 parent 269c1c8 commit ef2b6ba

File tree

15 files changed

+2048
-362
lines changed

15 files changed

+2048
-362
lines changed

docs/agent/architecture-guardrails.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,20 @@
5656
- `plugin` performs post-decision or post-selection processing
5757
- `global` carries intentionally cross-cutting behavior
5858

59+
## API Type Contracts
60+
61+
- Use official SDK types for OpenAI and Anthropic request/response handling:
62+
- `github.com/openai/openai-go` for OpenAI-shaped data
63+
- `github.com/anthropics/anthropic-sdk-go` for Anthropic-shaped data
64+
- Do not define custom structs that duplicate what the SDK provides
65+
(e.g., custom `ChatCompletionRequest`, `ChatCompletionResponse`)
66+
- Exceptions: packages that intentionally avoid the SDK dependency for
67+
isolation (e.g., E2E fixtures, standalone training tools) should document
68+
the reason in a comment and keep their custom types minimal
69+
- When the SDK type does not cover a field the router needs, extend via
70+
composition (`type ExtendedReq struct { openai.ChatCompletionNewParams; Extra string }`)
71+
rather than reimplementing the whole struct
72+
5973
## Avoid
6074

6175
- giant managers

docs/agent/tech-debt/README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ Keep the numeric index unique within `docs/agent/tech-debt/`.
9090
- [TD034 Runtime and Dashboard State Surfaces Still Lack a Coherent Durability, Recovery, and Telemetry Contract](td-034-runtime-and-dashboard-state-durability-and-telemetry-contract.md)
9191
- [TD035 Projection Partition Default Coverage Contract Is No Longer Declarative Only](td-035-signal-group-default-coverage-contract-gap.md)
9292
- [TD036 Decision Tree Authoring Cannot Round-Trip Through Runtime Config](td-036-decision-tree-authoring-roundtrip-gap.md)
93+
- [TD037 Custom Chat Completions Structs Duplicate Official OpenAI SDK Types](td-037-custom-chat-completions-structs.md)
9394

9495
## Architecture Review Coverage Map
9596

@@ -101,6 +102,7 @@ Use this map when turning scale-out architecture findings into debt work. Reuse
101102
- extproc request and response phase collapse: [TD023](td-023-extproc-request-pipeline-phase-collapse.md), [TD029](td-029-extproc-response-pipeline-phase-collapse.md)
102103
- restart-sensitive runtime state and control-plane telemetry semantics: [TD034](td-034-runtime-and-dashboard-state-durability-and-telemetry-contract.md)
103104
- remaining hotspot-ratchet debt across router and binding hotspots: [TD006](td-006-structural-rule-target-vs-legacy-hotspots.md)
105+
- remaining custom Chat Completions struct consolidation: [TD037](td-037-custom-chat-completions-structs.md)
104106
- Dashboard frontend and backend
105107
- frontend route shell, editor control plane, and large UI containers: [TD030](td-030-dashboard-frontend-config-and-interaction-slice-collapse.md)
106108
- dashboard backend training, evaluation, and model-research contract seams: [TD032](td-032-training-evaluation-artifact-contract-drift.md)
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
# TD037: Custom Chat Completions Structs Duplicate Official OpenAI SDK Types
2+
3+
## Status
4+
5+
Partially resolved — major packages migrated, remaining surfaces tracked below.
6+
7+
## Scope
8+
9+
`src/semantic-router/pkg/responseapi`, `pkg/classification`, `pkg/modelselection`,
10+
`pkg/mcp`, `pkg/extproc`, `pkg/anthropic`, `pkg/cache`, `pkg/memory`
11+
12+
## Summary
13+
14+
Several packages defined their own `ChatCompletionRequest`, `ChatCompletionResponse`,
15+
`ChatMessage`, `Choice`, `Usage`, and similar types instead of using the official
16+
`openai-go` SDK types. This duplication risks schema drift against the upstream
17+
OpenAI API and creates maintenance burden when fields are added or changed.
18+
19+
## Evidence
20+
21+
Packages that **have been migrated** (this PR):
22+
23+
| Package | Removed Structs | Now Uses |
24+
|---------|----------------|----------|
25+
| `pkg/responseapi` | 8 structs (ChatCompletionRequest, ChatMessage, ToolCall, FunctionCall, ChatTool, ChatCompletionResponse, Choice, CompletionUsage) | `openai.ChatCompletionNewParams`, `openai.ChatCompletion` |
26+
| `pkg/classification` | 6 structs (ChatCompletionRequest, ChatMessage, ChatCompletionResponse, Choice, Message, Usage) | `openai.ChatCompletion`, `openai.ChatCompletionMessageParamUnion` with composition for `ExtraBody` |
27+
| `pkg/modelselection` | 3 structs (ChatCompletionRequest, ChatMessage, ChatCompletionResponse) | `openai.ChatCompletion`, `openai.ChatCompletionMessageParamUnion` with composition for error fields |
28+
| `pkg/mcp` | 4 structs (OpenAITool, OpenAIToolFunction, OpenAIToolCall, OpenAIToolCallFunction) | `openai.ChatCompletionToolParam`, `openai.ChatCompletionMessageToolCall` |
29+
30+
Packages with **remaining custom types** (lower priority):
31+
32+
| Package | Custom Types | Notes |
33+
|---------|-------------|-------|
34+
| `pkg/cache` | Message, Usage structs | Used for cache serialization; migration deferred to avoid cache format break |
35+
| `pkg/memory` | Message struct | Used for memory store serialization |
36+
37+
## Why It Matters
38+
39+
- Schema drift: custom structs miss new fields added to the OpenAI API
40+
- Maintenance burden: changes must be replicated across multiple struct definitions
41+
- Testing gap: custom types can silently diverge from what clients actually send
42+
- PR #1070 reviewer explicitly flagged this as needed work
43+
44+
## Desired End State
45+
46+
All OpenAI-shaped request/response handling uses `openai-go` SDK types. Custom
47+
structs exist only where composition is needed for router-specific extensions
48+
(e.g., vLLM `extra_body`, provider error wrapping), documented with a comment
49+
explaining why the extension is necessary.
50+
51+
## Exit Criteria
52+
53+
- [ ] `pkg/cache` serialization types migrated or documented as intentional exceptions
54+
- [ ] `pkg/memory` serialization types migrated or documented as intentional exceptions
55+
- [ ] Zero custom `ChatCompletion*` type definitions remain outside documented exceptions
56+
- [ ] Compatibility tests cover all conversion paths
Lines changed: 279 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,279 @@
1+
package anthropic
2+
3+
import (
4+
"encoding/json"
5+
"testing"
6+
7+
"github.com/anthropics/anthropic-sdk-go"
8+
"github.com/openai/openai-go"
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
)
12+
13+
// These tests verify that the OpenAI ↔ Anthropic conversion layer produces
14+
// JSON output that is wire-compatible with the respective official SDKs.
15+
// They serve as a regression guard against schema drift when either SDK is
16+
// upgraded or when internal conversion logic is refactored.
17+
18+
func TestRoundTrip_SimpleRequest(t *testing.T) {
19+
orig := &openai.ChatCompletionNewParams{
20+
Model: "claude-sonnet-4-5",
21+
Messages: []openai.ChatCompletionMessageParamUnion{
22+
{OfUser: &openai.ChatCompletionUserMessageParam{
23+
Content: openai.ChatCompletionUserMessageParamContentUnion{
24+
OfString: openai.String("Explain photosynthesis."),
25+
},
26+
}},
27+
},
28+
Temperature: openai.Float(0.5),
29+
}
30+
31+
anthropicBody, err := ToAnthropicRequestBody(orig)
32+
require.NoError(t, err)
33+
34+
var parsed anthropic.MessageNewParams
35+
require.NoError(t, json.Unmarshal(anthropicBody, &parsed))
36+
37+
assert.Equal(t, anthropic.Model("claude-sonnet-4-5"), parsed.Model)
38+
assert.Equal(t, DefaultMaxTokens, parsed.MaxTokens)
39+
require.Len(t, parsed.Messages, 1)
40+
assert.Equal(t, anthropic.MessageParamRoleUser, parsed.Messages[0].Role)
41+
}
42+
43+
func TestRoundTrip_SystemSeparation(t *testing.T) {
44+
orig := &openai.ChatCompletionNewParams{
45+
Model: "claude-sonnet-4-5",
46+
Messages: []openai.ChatCompletionMessageParamUnion{
47+
{OfSystem: &openai.ChatCompletionSystemMessageParam{
48+
Content: openai.ChatCompletionSystemMessageParamContentUnion{
49+
OfString: openai.String("Be concise."),
50+
},
51+
}},
52+
{OfUser: &openai.ChatCompletionUserMessageParam{
53+
Content: openai.ChatCompletionUserMessageParamContentUnion{
54+
OfString: openai.String("Hi"),
55+
},
56+
}},
57+
},
58+
}
59+
60+
body, err := ToAnthropicRequestBody(orig)
61+
require.NoError(t, err)
62+
63+
var parsed anthropic.MessageNewParams
64+
require.NoError(t, json.Unmarshal(body, &parsed))
65+
66+
require.Len(t, parsed.System, 1)
67+
assert.Equal(t, "Be concise.", parsed.System[0].Text)
68+
require.Len(t, parsed.Messages, 1, "system must not appear in messages array")
69+
}
70+
71+
func TestRoundTrip_MaxTokensVariants(t *testing.T) {
72+
tests := []struct {
73+
name string
74+
req *openai.ChatCompletionNewParams
75+
expected int64
76+
}{
77+
{
78+
name: "MaxCompletionTokens takes priority",
79+
req: &openai.ChatCompletionNewParams{
80+
Model: "claude-sonnet-4-5",
81+
MaxCompletionTokens: openai.Int(512),
82+
MaxTokens: openai.Int(1024),
83+
Messages: simpleUserMsg("hi"),
84+
},
85+
expected: 512,
86+
},
87+
{
88+
name: "fallback to MaxTokens",
89+
req: &openai.ChatCompletionNewParams{
90+
Model: "claude-sonnet-4-5",
91+
MaxTokens: openai.Int(2048),
92+
Messages: simpleUserMsg("hi"),
93+
},
94+
expected: 2048,
95+
},
96+
{
97+
name: "default when neither set",
98+
req: &openai.ChatCompletionNewParams{
99+
Model: "claude-sonnet-4-5",
100+
Messages: simpleUserMsg("hi"),
101+
},
102+
expected: DefaultMaxTokens,
103+
},
104+
}
105+
106+
for _, tt := range tests {
107+
t.Run(tt.name, func(t *testing.T) {
108+
body, err := ToAnthropicRequestBody(tt.req)
109+
require.NoError(t, err)
110+
111+
var parsed anthropic.MessageNewParams
112+
require.NoError(t, json.Unmarshal(body, &parsed))
113+
assert.Equal(t, tt.expected, parsed.MaxTokens)
114+
})
115+
}
116+
}
117+
118+
func TestResponse_StopReasonMapping(t *testing.T) {
119+
tests := []struct {
120+
anthropicReason anthropic.StopReason
121+
expectedOpenAI string
122+
}{
123+
{anthropic.StopReasonEndTurn, "stop"},
124+
{anthropic.StopReasonMaxTokens, "length"},
125+
{anthropic.StopReasonToolUse, "tool_calls"},
126+
{"unknown_future_reason", "stop"},
127+
}
128+
129+
for _, tt := range tests {
130+
t.Run(string(tt.anthropicReason), func(t *testing.T) {
131+
resp := anthropic.Message{
132+
ID: "msg_test",
133+
Content: []anthropic.ContentBlockUnion{{Type: "text", Text: "ok"}},
134+
StopReason: tt.anthropicReason,
135+
Usage: anthropic.Usage{InputTokens: 1, OutputTokens: 1},
136+
}
137+
138+
raw, err := json.Marshal(resp)
139+
require.NoError(t, err)
140+
141+
out, err := ToOpenAIResponseBody(raw, "claude-sonnet-4-5")
142+
require.NoError(t, err)
143+
144+
var oai openai.ChatCompletion
145+
require.NoError(t, json.Unmarshal(out, &oai))
146+
assert.Equal(t, tt.expectedOpenAI, oai.Choices[0].FinishReason)
147+
})
148+
}
149+
}
150+
151+
func TestResponse_UsageMapping(t *testing.T) {
152+
resp := anthropic.Message{
153+
ID: "msg_usage",
154+
Content: []anthropic.ContentBlockUnion{{Type: "text", Text: "hi"}},
155+
StopReason: anthropic.StopReasonEndTurn,
156+
Usage: anthropic.Usage{InputTokens: 42, OutputTokens: 17},
157+
}
158+
159+
raw, err := json.Marshal(resp)
160+
require.NoError(t, err)
161+
162+
out, err := ToOpenAIResponseBody(raw, "test-model")
163+
require.NoError(t, err)
164+
165+
var oai openai.ChatCompletion
166+
require.NoError(t, json.Unmarshal(out, &oai))
167+
168+
assert.Equal(t, int64(42), oai.Usage.PromptTokens)
169+
assert.Equal(t, int64(17), oai.Usage.CompletionTokens)
170+
assert.Equal(t, int64(59), oai.Usage.TotalTokens)
171+
}
172+
173+
func TestResponse_OutputIsValidOpenAIJSON(t *testing.T) {
174+
resp := anthropic.Message{
175+
ID: "msg_valid",
176+
Role: "assistant",
177+
Content: []anthropic.ContentBlockUnion{
178+
{Type: "text", Text: "Hello world"},
179+
},
180+
StopReason: anthropic.StopReasonEndTurn,
181+
Usage: anthropic.Usage{InputTokens: 5, OutputTokens: 3},
182+
}
183+
raw, err := json.Marshal(resp)
184+
require.NoError(t, err)
185+
186+
out, err := ToOpenAIResponseBody(raw, "claude-sonnet-4-5")
187+
require.NoError(t, err)
188+
189+
var oai openai.ChatCompletion
190+
require.NoError(t, json.Unmarshal(out, &oai),
191+
"output must unmarshal cleanly into the official openai.ChatCompletion type")
192+
193+
assert.Equal(t, "chat.completion", string(oai.Object))
194+
assert.Equal(t, "msg_valid", oai.ID)
195+
assert.Equal(t, "claude-sonnet-4-5", oai.Model)
196+
require.Len(t, oai.Choices, 1)
197+
assert.Equal(t, "assistant", string(oai.Choices[0].Message.Role))
198+
assert.Equal(t, "Hello world", oai.Choices[0].Message.Content)
199+
assert.NotZero(t, oai.Created)
200+
}
201+
202+
func TestRequest_EmptyMessagesRejected(t *testing.T) {
203+
req := &openai.ChatCompletionNewParams{
204+
Model: "claude-sonnet-4-5",
205+
Messages: nil,
206+
}
207+
208+
body, err := ToAnthropicRequestBody(req)
209+
require.NoError(t, err)
210+
211+
var parsed anthropic.MessageNewParams
212+
require.NoError(t, json.Unmarshal(body, &parsed))
213+
assert.Empty(t, parsed.Messages)
214+
}
215+
216+
func TestResponse_EmptyContentBlocks(t *testing.T) {
217+
resp := anthropic.Message{
218+
ID: "msg_empty",
219+
Content: []anthropic.ContentBlockUnion{},
220+
StopReason: anthropic.StopReasonEndTurn,
221+
Usage: anthropic.Usage{InputTokens: 1, OutputTokens: 0},
222+
}
223+
raw, err := json.Marshal(resp)
224+
require.NoError(t, err)
225+
226+
out, err := ToOpenAIResponseBody(raw, "model")
227+
require.NoError(t, err)
228+
229+
var oai openai.ChatCompletion
230+
require.NoError(t, json.Unmarshal(out, &oai))
231+
assert.Empty(t, oai.Choices[0].Message.Content)
232+
}
233+
234+
func TestRequest_StopSequences(t *testing.T) {
235+
t.Run("string array", func(t *testing.T) {
236+
req := &openai.ChatCompletionNewParams{
237+
Model: "claude-sonnet-4-5",
238+
Messages: simpleUserMsg("hi"),
239+
Stop: openai.ChatCompletionNewParamsStopUnion{
240+
OfStringArray: []string{"END", "DONE"},
241+
},
242+
}
243+
244+
body, err := ToAnthropicRequestBody(req)
245+
require.NoError(t, err)
246+
247+
var parsed anthropic.MessageNewParams
248+
require.NoError(t, json.Unmarshal(body, &parsed))
249+
assert.Equal(t, []string{"END", "DONE"}, parsed.StopSequences)
250+
})
251+
252+
t.Run("single string", func(t *testing.T) {
253+
req := &openai.ChatCompletionNewParams{
254+
Model: "claude-sonnet-4-5",
255+
Messages: simpleUserMsg("hi"),
256+
Stop: openai.ChatCompletionNewParamsStopUnion{
257+
OfString: openai.String("STOP"),
258+
},
259+
}
260+
261+
body, err := ToAnthropicRequestBody(req)
262+
require.NoError(t, err)
263+
264+
var parsed anthropic.MessageNewParams
265+
require.NoError(t, json.Unmarshal(body, &parsed))
266+
assert.Equal(t, []string{"STOP"}, parsed.StopSequences)
267+
})
268+
}
269+
270+
// simpleUserMsg builds a minimal message slice for tests.
271+
func simpleUserMsg(text string) []openai.ChatCompletionMessageParamUnion {
272+
return []openai.ChatCompletionMessageParamUnion{
273+
{OfUser: &openai.ChatCompletionUserMessageParam{
274+
Content: openai.ChatCompletionUserMessageParamContentUnion{
275+
OfString: openai.String(text),
276+
},
277+
}},
278+
}
279+
}

0 commit comments

Comments
 (0)