mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-05 08:10:14 +00:00
fix: ensure coerced follow_up suggestions always include mode field
The tool schema requires mode on each suggestion item (required: ["text", "mode"]). When follow_up was coerced from a string, the mode field was missing entirely. Some model Jinja templates (e.g. Qwen via llama.cpp) fail with "Cannot convert value of type Optional<Any> to Jinja Value" when a required field is absent from tool call arguments in conversation history. Now all coercion paths (coerceFollowUp helper + NativeToolCallParser partial and finalization handlers) normalize every suggestion to include mode: null when not explicitly set.
This commit is contained in:
parent
967fa19890
commit
d012af9a0e
3 changed files with 66 additions and 21 deletions
|
|
@ -479,11 +479,18 @@ export class NativeToolCallParser {
|
|||
if (!Array.isArray(coercedPartialFollowUp) && typeof coercedPartialFollowUp === "string") {
|
||||
try {
|
||||
const parsed = JSON.parse(coercedPartialFollowUp)
|
||||
coercedPartialFollowUp = Array.isArray(parsed) ? parsed : undefined
|
||||
coercedPartialFollowUp = Array.isArray(parsed)
|
||||
? parsed.map((s: any) => ({ text: s.text, mode: s.mode ?? null }))
|
||||
: undefined
|
||||
} catch {
|
||||
coercedPartialFollowUp = undefined
|
||||
}
|
||||
} else if (!Array.isArray(coercedPartialFollowUp)) {
|
||||
} else if (Array.isArray(coercedPartialFollowUp)) {
|
||||
coercedPartialFollowUp = coercedPartialFollowUp.map((s: any) => ({
|
||||
text: s.text,
|
||||
mode: s.mode ?? null,
|
||||
}))
|
||||
} else {
|
||||
coercedPartialFollowUp = undefined
|
||||
}
|
||||
nativeArgs = {
|
||||
|
|
@ -837,11 +844,18 @@ export class NativeToolCallParser {
|
|||
if (trimmed.length > 0) {
|
||||
try {
|
||||
const parsed = JSON.parse(trimmed)
|
||||
coercedFinalFollowUp = Array.isArray(parsed) ? parsed : [{ text: trimmed }]
|
||||
coercedFinalFollowUp = Array.isArray(parsed)
|
||||
? parsed.map((s: any) => ({ text: s.text, mode: s.mode ?? null }))
|
||||
: [{ text: trimmed, mode: null }]
|
||||
} catch {
|
||||
coercedFinalFollowUp = [{ text: trimmed }]
|
||||
coercedFinalFollowUp = [{ text: trimmed, mode: null }]
|
||||
}
|
||||
}
|
||||
} else if (Array.isArray(coercedFinalFollowUp)) {
|
||||
coercedFinalFollowUp = coercedFinalFollowUp.map((s: any) => ({
|
||||
text: s.text,
|
||||
mode: s.mode ?? null,
|
||||
}))
|
||||
}
|
||||
nativeArgs = {
|
||||
question: args.question,
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ import { BaseTool, ToolCallbacks } from "./BaseTool"
|
|||
|
||||
interface Suggestion {
|
||||
text: string
|
||||
mode?: string
|
||||
mode?: string | null
|
||||
}
|
||||
|
||||
interface AskFollowupQuestionParams {
|
||||
|
|
@ -27,7 +27,7 @@ interface AskFollowupQuestionParams {
|
|||
*/
|
||||
export function coerceFollowUp(value: unknown): Suggestion[] | undefined {
|
||||
if (Array.isArray(value)) {
|
||||
return value
|
||||
return normalizeSuggestions(value)
|
||||
}
|
||||
|
||||
if (typeof value === "string" && value.trim().length > 0) {
|
||||
|
|
@ -35,19 +35,31 @@ export function coerceFollowUp(value: unknown): Suggestion[] | undefined {
|
|||
try {
|
||||
const parsed = JSON.parse(value)
|
||||
if (Array.isArray(parsed)) {
|
||||
return parsed
|
||||
return normalizeSuggestions(parsed)
|
||||
}
|
||||
} catch {
|
||||
// Not valid JSON -- fall through to plain-string wrapping
|
||||
}
|
||||
|
||||
// Wrap plain string as a single suggestion
|
||||
return [{ text: value }]
|
||||
return [{ text: value, mode: null }]
|
||||
}
|
||||
|
||||
return undefined
|
||||
}
|
||||
|
||||
/**
|
||||
* Ensure every suggestion has an explicit `mode` field (defaulting to `null`).
|
||||
* The tool schema requires `mode` on each item (`required: ["text", "mode"]`),
|
||||
* and some model Jinja templates fail if a required field is absent.
|
||||
*/
|
||||
function normalizeSuggestions(items: Suggestion[]): Suggestion[] {
|
||||
return items.map((s) => ({
|
||||
text: s.text,
|
||||
mode: s.mode ?? null,
|
||||
}))
|
||||
}
|
||||
|
||||
export class AskFollowupQuestionTool extends BaseTool<"ask_followup_question"> {
|
||||
readonly name = "ask_followup_question" as const
|
||||
|
||||
|
|
|
|||
|
|
@ -45,7 +45,7 @@ describe("askFollowupQuestionTool", () => {
|
|||
|
||||
expect(mockCline.ask).toHaveBeenCalledWith(
|
||||
"followup",
|
||||
expect.stringContaining('"suggest":[{"answer":"Option 1"},{"answer":"Option 2"}]'),
|
||||
expect.stringContaining('"suggest":[{"answer":"Option 1","mode":null},{"answer":"Option 2","mode":null}]'),
|
||||
false,
|
||||
)
|
||||
})
|
||||
|
|
@ -105,7 +105,7 @@ describe("askFollowupQuestionTool", () => {
|
|||
expect(mockCline.ask).toHaveBeenCalledWith(
|
||||
"followup",
|
||||
expect.stringContaining(
|
||||
'"suggest":[{"answer":"Regular option"},{"answer":"Plan architecture","mode":"architect"}]',
|
||||
'"suggest":[{"answer":"Regular option","mode":null},{"answer":"Plan architecture","mode":"architect"}]',
|
||||
),
|
||||
false,
|
||||
)
|
||||
|
|
@ -189,7 +189,7 @@ describe("askFollowupQuestionTool", () => {
|
|||
// Plain string should be coerced to [{ text: "not an array" }]
|
||||
expect(mockCline.ask).toHaveBeenCalledWith(
|
||||
"followup",
|
||||
expect.stringContaining('"suggest":[{"answer":"not an array"}]'),
|
||||
expect.stringContaining('"suggest":[{"answer":"not an array","mode":null}]'),
|
||||
false,
|
||||
)
|
||||
})
|
||||
|
|
@ -217,7 +217,9 @@ describe("askFollowupQuestionTool", () => {
|
|||
// JSON string should be parsed into a proper array
|
||||
expect(mockCline.ask).toHaveBeenCalledWith(
|
||||
"followup",
|
||||
expect.stringContaining('"suggest":[{"answer":"Option A"},{"answer":"Option B","mode":"code"}]'),
|
||||
expect.stringContaining(
|
||||
'"suggest":[{"answer":"Option A","mode":null},{"answer":"Option B","mode":"code"}]',
|
||||
),
|
||||
false,
|
||||
)
|
||||
})
|
||||
|
|
@ -248,18 +250,32 @@ describe("askFollowupQuestionTool", () => {
|
|||
})
|
||||
|
||||
describe("coerceFollowUp helper", () => {
|
||||
it("should return arrays as-is", () => {
|
||||
it("should normalize arrays to include mode: null when missing", () => {
|
||||
const input = [{ text: "Option 1" }, { text: "Option 2" }]
|
||||
expect(coerceFollowUp(input)).toEqual(input)
|
||||
expect(coerceFollowUp(input)).toEqual([
|
||||
{ text: "Option 1", mode: null },
|
||||
{ text: "Option 2", mode: null },
|
||||
])
|
||||
})
|
||||
|
||||
it("should preserve existing mode values in arrays", () => {
|
||||
const input = [{ text: "Option 1", mode: "code" }, { text: "Option 2" }]
|
||||
expect(coerceFollowUp(input)).toEqual([
|
||||
{ text: "Option 1", mode: "code" },
|
||||
{ text: "Option 2", mode: null },
|
||||
])
|
||||
})
|
||||
|
||||
it("should parse a JSON string containing an array", () => {
|
||||
const input = '[{"text":"A"},{"text":"B","mode":"code"}]'
|
||||
expect(coerceFollowUp(input)).toEqual([{ text: "A" }, { text: "B", mode: "code" }])
|
||||
expect(coerceFollowUp(input)).toEqual([
|
||||
{ text: "A", mode: null },
|
||||
{ text: "B", mode: "code" },
|
||||
])
|
||||
})
|
||||
|
||||
it("should wrap a plain string as a single suggestion", () => {
|
||||
expect(coerceFollowUp("some option")).toEqual([{ text: "some option" }])
|
||||
it("should wrap a plain string as a single suggestion with mode: null", () => {
|
||||
expect(coerceFollowUp("some option")).toEqual([{ text: "some option", mode: null }])
|
||||
})
|
||||
|
||||
it("should return undefined for null", () => {
|
||||
|
|
@ -278,9 +294,9 @@ describe("askFollowupQuestionTool", () => {
|
|||
expect(coerceFollowUp(" ")).toBeUndefined()
|
||||
})
|
||||
|
||||
it("should wrap a JSON string that parses to a non-array as a suggestion", () => {
|
||||
it("should wrap a JSON string that parses to a non-array as a suggestion with mode: null", () => {
|
||||
// A JSON string like '{"text":"hello"}' is valid JSON but not an array
|
||||
expect(coerceFollowUp('{"text":"hello"}')).toEqual([{ text: '{"text":"hello"}' }])
|
||||
expect(coerceFollowUp('{"text":"hello"}')).toEqual([{ text: '{"text":"hello"}', mode: null }])
|
||||
})
|
||||
})
|
||||
|
||||
|
|
@ -400,7 +416,7 @@ describe("askFollowupQuestionTool", () => {
|
|||
follow_up: Array<{ text: string; mode?: string }>
|
||||
}
|
||||
expect(nativeArgs.question).toBe("Pick one")
|
||||
expect(nativeArgs.follow_up).toEqual([{ text: "Option A" }])
|
||||
expect(nativeArgs.follow_up).toEqual([{ text: "Option A", mode: null }])
|
||||
}
|
||||
})
|
||||
|
||||
|
|
@ -422,7 +438,10 @@ describe("askFollowupQuestionTool", () => {
|
|||
follow_up: Array<{ text: string; mode?: string }>
|
||||
}
|
||||
expect(nativeArgs.question).toBe("Pick one")
|
||||
expect(nativeArgs.follow_up).toEqual([{ text: "A" }, { text: "B" }])
|
||||
expect(nativeArgs.follow_up).toEqual([
|
||||
{ text: "A", mode: null },
|
||||
{ text: "B", mode: null },
|
||||
])
|
||||
}
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue