Skip to content

Commit c6d5360

Browse files
easonLiangWorldedtecheasonliang28edelauna
authored
refactor(providers): map xAI tool_choice to the Responses API shape with a typed request body (#2036)
* refactor(providers): map xAI tool_choice to the Responses API shape with a typed request body Extracted from the abort-signal series unit (PR 1293) at reviewer request so that unit stays abort-wiring only: - xai: replace the `as any` tool_choice cast with an explicit mapToolChoice mapping (Chat Completions nests named-tool references under function/custom; the Responses API puts the name at the top level; allowed_tools entries are flattened). - xai: type the request body as ResponseCreateParamsStreaming (drops five lint suppressions) and send max_output_tokens/temperature unconditionally - getModel() always resolves them. - anthropic-vertex: drop the no-op content_block_stop case (the comment itself noted no action is needed; NativeToolCallParser handles tool-call completion). Verification: 75/75 green in xai.spec.ts + anthropic-vertex.spec.ts; tsc --noEmit 0 errors; eslint --max-warnings=0 clean on touched files (xai.ts suppression count 7 -> 2); prettier clean. * Address review: reject malformed allowed_tools function entries Chat Completions allowlist entries are open records, so a function entry can arrive with a missing or non-string name. Forwarding such an entry unchanged would send the nested Chat Completions shape to the Responses API, which cannot express that allowlist entry, and how xAI handles the malformed payload is unspecified. mapToolChoice now rejects function entries without a valid string name at the boundary (before responses.create); non-function entries still pass through unchanged. The pass-through test now pins only the non-function case; a new test rejects four malformed function-entry shapes (null name, null ref, function value, non-string name) and asserts no request is issued (negative control verified: reverting the throw to a pass-through fails the new test). Verification at this head: 25/25 green in xai.spec.ts; tsc --noEmit 0 errors; eslint --max-warnings=0 clean on touched files; prettier clean. * chore(lint): take upstream baseline suppressions for xai.ts The merge brings main xai responses-create code whose as-any casts main suppresses (count 7 in upstream eslint-suppressions.json). Locally the auto-merge kept the older count 2, so compile lint flagged the inherited casts. Use the upstream baseline instead of leaving the branch red. * chore(lint): right-size the xai suppression to the merged any count The merge keeps five as-any occurrences in xai.ts (two in the mapResponseTools signature, three in the responses create call); the upstream baseline of seven would leave stale suppressions, which eslint rejects. Set the count to the actual five. * test(xai): assert request options without the extra call-argument shape The merge with main makes responses.create take a second options argument. Rewriting the assertions to inspect mock.calls[0][0] left the old second expectation argument in place, which failed type-check (toEqual takes one argument). Each assertion now checks the request body and the separate options argument explicitly (signal/timeout, forwarded timeout, absent options). xai.spec.ts 49 passed; tsc clean for the file. * fix(xai): bridge the abort signal only after request construction Review follow-up: mapToolChoice throws for malformed allowed_tools entries, and the abort bridge cleanup only covers the request that follows. The bridge now registers after the whole request body (including the tool-choice mapping) is built, so a construction failure cannot leave a listener on metadata.abortSignal. The malformed-entry test now passes an abortSignal and asserts addEventListener was never called. xai.spec.ts 49 passed; tsc clean. * chore(lint): flag unneeded type assertions in api/ --------- Co-authored-by: Eason Liang <easonliang28@gmail.com> Co-authored-by: Elliott de Launay <edelauna@gmail.com>
1 parent a90133d commit c6d5360

12 files changed

Lines changed: 341 additions & 89 deletions

‎src/api/providers/__tests__/xai.spec.ts‎

Lines changed: 228 additions & 38 deletions
Large diffs are not rendered by default.

‎src/api/providers/anthropic-vertex.ts‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ export class AnthropicVertexHandler extends BaseProvider implements SingleComple
143143
for await (const chunk of stream) {
144144
switch (chunk.type) {
145145
case "message_start": {
146-
const usage = chunk.message!.usage
146+
const usage = chunk.message.usage
147147

148148
// `output_tokens_details.thinking_tokens` decomposes the billed
149149
// output into internal reasoning tokens; surfaced for per-turn
@@ -155,19 +155,19 @@ export class AnthropicVertexHandler extends BaseProvider implements SingleComple
155155
cacheWriteTokens: usage.cache_creation_input_tokens || undefined,
156156
cacheReadTokens: usage.cache_read_input_tokens || undefined,
157157
...(typeof usage.output_tokens_details?.thinking_tokens === "number"
158-
? { reasoningTokens: usage.output_tokens_details!.thinking_tokens }
158+
? { reasoningTokens: usage.output_tokens_details.thinking_tokens }
159159
: {}),
160160
}
161161

162162
break
163163
}
164164
case "message_delta": {
165-
const deltaThinkingTokens = chunk.usage!.output_tokens_details?.thinking_tokens
165+
const deltaThinkingTokens = chunk.usage.output_tokens_details?.thinking_tokens
166166

167167
yield {
168168
type: "usage",
169169
inputTokens: 0,
170-
outputTokens: chunk.usage!.output_tokens || 0,
170+
outputTokens: chunk.usage.output_tokens || 0,
171171
...(typeof deltaThinkingTokens === "number"
172172
? { reasoningTokens: deltaThinkingTokens }
173173
: {}),
@@ -176,17 +176,17 @@ export class AnthropicVertexHandler extends BaseProvider implements SingleComple
176176
break
177177
}
178178
case "content_block_start": {
179-
switch (chunk.content_block!.type) {
179+
switch (chunk.content_block.type) {
180180
case "text": {
181-
if (chunk.index! > 0) {
181+
if (chunk.index > 0) {
182182
yield { type: "text", text: "\n" }
183183
}
184184

185-
yield { type: "text", text: chunk.content_block!.text }
185+
yield { type: "text", text: chunk.content_block.text }
186186
break
187187
}
188188
case "thinking": {
189-
if (chunk.index! > 0) {
189+
if (chunk.index > 0) {
190190
yield { type: "reasoning", text: "\n" }
191191
}
192192

@@ -198,8 +198,8 @@ export class AnthropicVertexHandler extends BaseProvider implements SingleComple
198198
yield {
199199
type: "tool_call_partial",
200200
index: chunk.index,
201-
id: chunk.content_block!.id,
202-
name: chunk.content_block!.name,
201+
id: chunk.content_block.id,
202+
name: chunk.content_block.name,
203203
arguments: undefined,
204204
}
205205
break
@@ -209,9 +209,9 @@ export class AnthropicVertexHandler extends BaseProvider implements SingleComple
209209
break
210210
}
211211
case "content_block_delta": {
212-
switch (chunk.delta!.type) {
212+
switch (chunk.delta.type) {
213213
case "text_delta": {
214-
yield { type: "text", text: chunk.delta!.text }
214+
yield { type: "text", text: chunk.delta.text }
215215
break
216216
}
217217
case "thinking_delta": {

‎src/api/providers/anthropic.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import {
3232
// Pre-sorted list of known Anthropic model IDs (lowercased) by length (descending) for case-insensitive substring matching.
3333
const ANTHROPIC_MODEL_IDS_SORTED_LOWER = (Object.keys(anthropicModels) as AnthropicModelId[])
3434
.map((id) => id.toLowerCase())
35-
.sort((a, b) => b.length - a.length) as string[]
35+
.sort((a, b) => b.length - a.length)
3636

3737
// Original-case mapping: lowercase key → original AnthropicModelId for lookup.
3838
const ANTHROPIC_MODEL_ID_LOWER_TO_ORIGINAL = Object.fromEntries(
@@ -405,7 +405,7 @@ export class AnthropicHandler extends BaseProvider implements SingleCompletionHa
405405
return anthropicModels[anthropicDefaultModelId]
406406
}
407407

408-
const originalId = ANTHROPIC_MODEL_ID_LOWER_TO_ORIGINAL[matchedLower] as AnthropicModelId
408+
const originalId = ANTHROPIC_MODEL_ID_LOWER_TO_ORIGINAL[matchedLower]
409409
return anthropicModels[originalId]
410410
}
411411

‎src/api/providers/nanogpt.ts‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import {
99
providerIdentifiers,
1010
type ModelInfo,
1111
type ModelRecord,
12-
type NanoGptRoutingPreference,
1312
type ReasoningEffortExtended,
1413
} from "@roo-code/types"
1514

@@ -115,10 +114,7 @@ export class NanoGptHandler extends RouterProvider implements SingleCompletionHa
115114
}
116115

117116
private getRequestModelId(canonicalModelId: string): string {
118-
return applyNanoGptRoutingPreference(
119-
canonicalModelId,
120-
this.options.nanoGptRoutingPreference as NanoGptRoutingPreference | undefined,
121-
)
117+
return applyNanoGptRoutingPreference(canonicalModelId, this.options.nanoGptRoutingPreference)
122118
}
123119

124120
private createSafeError(operation: string, error: unknown): Error {

‎src/api/providers/openai-codex.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -627,7 +627,7 @@ export class OpenAiCodexHandler extends BaseProvider implements SingleCompletion
627627
if (block.type === "text") {
628628
content.push({ type: "input_text", text: block.text })
629629
} else if (block.type === "image") {
630-
const image = block as Anthropic.Messages.ImageBlockParam
630+
const image = block
631631
if (image.source.type === "base64") {
632632
const imageUrl = `data:${image.source.media_type};base64,${image.source.data}`
633633
content.push({ type: "input_image", image_url: imageUrl })

‎src/api/providers/openai-native.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,7 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio
145145
usage.cache_read_input_tokens ?? usage.cache_read_tokens ?? usage.cached_tokens ?? cachedFromDetails ?? 0
146146

147147
// Resolve effective tier: prefer actual tier from response; otherwise requested tier
148-
const effectiveTier =
149-
this.lastServiceTier || (this.options.openAiNativeServiceTier as ServiceTier | undefined) || undefined
148+
const effectiveTier = this.lastServiceTier || this.options.openAiNativeServiceTier || undefined
150149
const effectiveInfo = this.applyServiceTierPricing(model.info, effectiveTier)
151150

152151
// Pass total input tokens directly to calculateApiCostOpenAI
@@ -486,7 +485,7 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio
486485
if (block.type === "text") {
487486
content.push({ type: "input_text", text: block.text })
488487
} else if (block.type === "image") {
489-
const image = block as Anthropic.Messages.ImageBlockParam
488+
const image = block
490489
if (image.source.type === "base64") {
491490
const imageUrl = `data:${image.source.media_type};base64,${image.source.data}`
492491
content.push({ type: "input_image", image_url: imageUrl })
@@ -1551,7 +1550,7 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio
15511550

15521551
// Include text.verbosity only when the model explicitly supports it
15531552
if (model.info.supportsVerbosity === true) {
1554-
requestBody.text = { verbosity: (verbosity || "medium") as VerbosityLevel }
1553+
requestBody.text = { verbosity: verbosity || "medium" }
15551554
}
15561555

15571556
// Enable extended prompt cache retention for eligible models

‎src/api/providers/xai.ts‎

Lines changed: 74 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,61 @@ export class XAIHandler extends BaseProvider implements SingleCompletionHandler
6262
return { id, info, ...params }
6363
}
6464

65+
/**
66+
* Map a Chat Completions tool choice to the Responses API shape so TypeScript
67+
* validates the provider payload (the APIs use different object forms for the
68+
* named-tool choices: Chat Completions nests the name under `function`/`custom`,
69+
* Responses API puts it at the top level). String options (auto/required/none)
70+
* are identical in both APIs and pass through unchanged. `allowed_tools`
71+
* entries keep their allowlist mode, but Chat Completions function references
72+
* ({ type: "function", function: { name } }) are flattened to the Responses
73+
* API shape ({ type: "function", name }); other entry types pass through
74+
* unchanged.
75+
*/
76+
private mapToolChoice(
77+
toolChoice: NonNullable<OpenAI.Chat.ChatCompletionCreateParams["tool_choice"]>,
78+
): OpenAI.Responses.ResponseCreateParamsStreaming["tool_choice"] {
79+
if (typeof toolChoice === "string") {
80+
return toolChoice
81+
}
82+
switch (toolChoice.type) {
83+
case "function":
84+
return { type: "function", name: toolChoice.function.name }
85+
case "custom":
86+
return { type: "custom", name: toolChoice.custom.name }
87+
case "allowed_tools":
88+
return {
89+
type: "allowed_tools",
90+
mode: toolChoice.allowed_tools.mode,
91+
tools: toolChoice.allowed_tools.tools.map((entry) => {
92+
// Chat Completions allowlist entries nest the function reference
93+
// ({ type: "function", function: { name } }); the Responses API
94+
// expects the name at the top level ({ type: "function", name }).
95+
// The Chat Completions type allows open records, so a function
96+
// entry can arrive malformed (missing or non-string name).
97+
// Forwarding it unchanged would send the nested Chat Completions
98+
// shape to the Responses API, which cannot express that allowlist
99+
// entry, and how xAI handles the malformed payload is unspecified;
100+
// reject it at the boundary instead.
101+
if (entry["type"] === "function") {
102+
const functionRef = entry["function"]
103+
const name =
104+
functionRef != null && typeof functionRef === "object"
105+
? (functionRef as Record<string, unknown>)["name"]
106+
: undefined
107+
if (typeof name !== "string") {
108+
throw new Error(
109+
'xAI: allowed_tools entry of type "function" is missing a valid function name',
110+
)
111+
}
112+
return { type: "function", name }
113+
}
114+
return entry
115+
}),
116+
}
117+
}
118+
}
119+
65120
/**
66121
* Convert tools from OpenAI Chat Completions format to Responses API format.
67122
* Chat Completions: { type: "function", function: { name, description, parameters } }
@@ -101,18 +156,11 @@ export class XAIHandler extends BaseProvider implements SingleCompletionHandler
101156
// Convert directly from Anthropic format to Responses API input format
102157
const input = convertToResponsesApiInput(messages)
103158
const responseTools = this.mapResponseTools(metadata?.tools)
104-
105-
// Bridge the external abort signal from request metadata into a per-request
106-
// controller so the SDK call is cancelled when the owning request is aborted
107-
// (or when the signal is already aborted). Without an external signal the
108-
// client-level timeout configured in the constructor remains the only
109-
// cancellation mechanism, preserving the existing behavior.
110-
const externalAbortSignal = metadata?.abortSignal
111-
const { signal: abortSignal, dispose: removeExternalAbortListener } =
112-
bridgeExternalAbortSignal(externalAbortSignal)
159+
const toolChoice = metadata?.tool_choice
160+
const parallelToolCalls = metadata?.parallelToolCalls
113161

114162
// Build request options
115-
const requestBody: Record<string, any> = {
163+
const requestBody: OpenAI.Responses.ResponseCreateParamsStreaming = {
116164
model: model.id,
117165
instructions: systemPrompt,
118166
input: input,
@@ -121,19 +169,16 @@ export class XAIHandler extends BaseProvider implements SingleCompletionHandler
121169
include: ["reasoning.encrypted_content"],
122170
}
123171

124-
if (model.maxTokens) {
125-
requestBody.max_output_tokens = model.maxTokens
126-
}
127-
128-
if (model.temperature !== undefined) {
129-
requestBody.temperature = model.temperature
130-
}
172+
// Model params are always resolved by getModel(); send them unconditionally.
173+
requestBody.max_output_tokens = model.maxTokens
174+
requestBody.temperature = model.temperature
131175

132176
if (responseTools) {
133177
requestBody.tools = responseTools
134-
// Cast tool_choice since metadata uses Chat Completions types but Responses API has its own type
135-
requestBody.tool_choice = (metadata?.tool_choice ?? "auto") as any
136-
requestBody.parallel_tool_calls = metadata?.parallelToolCalls ?? true
178+
// Metadata carries a Chat Completions tool choice; the Responses API
179+
// uses its own shape, so map it explicitly instead of casting.
180+
requestBody.tool_choice = this.mapToolChoice(toolChoice === undefined ? "auto" : toolChoice)
181+
requestBody.parallel_tool_calls = parallelToolCalls ?? true
137182
}
138183

139184
// Pass reasoning effort for models that support it (e.g., grok-4.5, grok-3-mini).
@@ -143,11 +188,19 @@ export class XAIHandler extends BaseProvider implements SingleCompletionHandler
143188
requestBody.reasoning = { effort: model.reasoning.reasoning_effort }
144189
}
145190

191+
// Bridge the external abort signal only after every throw-prone part of the request
192+
// construction (notably mapToolChoice, which rejects malformed allowed_tools entries)
193+
// has run: the bridge's cleanup covers the request below, not construction failures,
194+
// so registering earlier would leak a listener on metadata.abortSignal.
195+
const externalAbortSignal = metadata?.abortSignal
196+
const { signal: abortSignal, dispose: removeExternalAbortListener } =
197+
bridgeExternalAbortSignal(externalAbortSignal)
198+
146199
try {
147200
let stream: AsyncIterable<any>
148201
try {
149202
stream = (await this.client.responses.create(
150-
{ ...requestBody, stream: true } as any,
203+
requestBody,
151204
abortSignal ? { signal: abortSignal } : undefined,
152205
)) as unknown as AsyncIterable<any>
153206
} catch (error) {

‎src/api/transform/model-params.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ export function getModelParams({
137137
// Here we simply propagate the resolved effort into the params, while
138138
// still treating "disable" as an omission.
139139
if (effort && effort !== "disable") {
140-
reasoningEffort = effort as ReasoningEffortExtended
140+
reasoningEffort = effort
141141
}
142142
}
143143

‎src/api/transform/openai-format.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,7 @@ export function convertToOpenAiMessages(
465465
] as OpenAI.Chat.ChatCompletionToolMessageParam
466466
if (lastToolMessage?.role === "tool") {
467467
const additionalText = filteredNonToolMessages
468-
.map((part) => sanitizeSurrogates((part as Anthropic.TextBlockParam).text))
468+
.map((part) => sanitizeSurrogates(part.text))
469469
.join("\n")
470470
lastToolMessage.content = `${lastToolMessage.content}\n\n${additionalText}`
471471
}

‎src/api/transform/reasoning.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ export const getOpenRouterReasoning = ({
6363
? { max_tokens: reasoningBudget }
6464
: shouldUseReasoningEffort({ model, settings })
6565
? reasoningEffort && reasoningEffort !== "disable"
66-
? { effort: reasoningEffort as ReasoningEffortExtended }
66+
? { effort: reasoningEffort }
6767
: undefined
6868
: undefined
6969

@@ -185,8 +185,7 @@ export const getGeminiReasoning = ({
185185
// We intentionally ignore enableReasoningEffort here so that explicitly chosen
186186
// efforts in the UI (e.g. "High" for gemini-3-pro-preview) always translate
187187
// into a thinkingConfig, regardless of legacy boolean flags.
188-
const selectedEffort = (settings.reasoningEffort ?? model.reasoningEffort) as
189-
ReasoningEffortExtended | "disable" | undefined
188+
const selectedEffort = settings.reasoningEffort ?? model.reasoningEffort
190189

191190
// Respect "off" / unset semantics from the effort selector itself.
192191
if (!selectedEffort || selectedEffort === "disable") {

0 commit comments

Comments
 (0)