From ce12e2f7254c32b9de1a26fa262847eab7ecbfe2 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Tue, 6 Jan 2026 13:39:21 +0000 Subject: [PATCH] feat: add experimental malformed JSON repair for tool calls This adds support for repairing malformed JSON in LLM tool call responses, particularly useful for models like Grok that struggle with strict JSON formatting. Changes: - Add jsonrepair dependency for JSON repair functionality - Create json-repair utility module with tryRepairJson, repairJson, parseWithRepair, isValidJson - Integrate repair logic into NativeToolCallParser.parseToolCall() - Add malformedJsonRepair experimental setting (disabled by default) - Wire up experiment toggle in Task.ts The feature is gated behind an experimental flag and only activates when: 1. The experiment is enabled in settings 2. Standard JSON.parse() fails on tool call arguments This addresses issue #10481 regarding BAML for parsing LLM output, using a simpler jsonrepair approach as an initial solution. --- packages/types/src/experiment.ts | 6 + pnpm-lock.yaml | 12 +- .../assistant-message/NativeToolCallParser.ts | 46 ++- src/core/task/Task.ts | 10 + src/package.json | 1 + src/shared/__tests__/experiments.spec.ts | 3 + src/shared/experiments.ts | 2 + src/utils/__tests__/json-repair.spec.ts | 303 ++++++++++++++++++ src/utils/json-repair.ts | 116 +++++++ 9 files changed, 497 insertions(+), 2 deletions(-) create mode 100644 src/utils/__tests__/json-repair.spec.ts create mode 100644 src/utils/json-repair.ts diff --git a/packages/types/src/experiment.ts b/packages/types/src/experiment.ts index f6f701a25d3..8a8c45e3dcb 100644 --- a/packages/types/src/experiment.ts +++ b/packages/types/src/experiment.ts @@ -14,6 +14,7 @@ export const experimentIds = [ "runSlashCommand", "multipleNativeToolCalls", "customTools", + "malformedJsonRepair", ] as const export const experimentIdsSchema = z.enum(experimentIds) @@ -32,6 +33,11 @@ export const experimentsSchema = z.object({ runSlashCommand: z.boolean().optional(), multipleNativeToolCalls: z.boolean().optional(), customTools: z.boolean().optional(), + /** + * Enable automatic repair of malformed JSON in LLM tool call responses. + * Useful for models like Grok that struggle with strict JSON formatting. + */ + malformedJsonRepair: z.boolean().optional(), }) export type Experiments = z.infer diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 190639f11df..1422d9f1e1b 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -812,6 +812,9 @@ importers: isbinaryfile: specifier: ^5.0.2 version: 5.0.4 + jsonrepair: + specifier: ^3.13.1 + version: 3.13.1 jwt-decode: specifier: ^4.0.0 version: 4.0.0 @@ -7142,6 +7145,10 @@ packages: jsonfile@4.0.0: resolution: {integrity: sha512-m6F1R3z8jjlf2imQHS2Qez5sjKWQzbuuhuJ/FKYFRZvPE3PuHcSMVZzfsLhGVOkfd20obL5SWEBew5ShlquNxg==} + jsonrepair@3.13.1: + resolution: {integrity: sha512-WJeiE0jGfxYmtLwBTEk8+y/mYcaleyLXWaqp5bJu0/ZTSeG0KQq/wWQ8pmnkKenEdN6pdnn6QtcoSUkbqDHWNw==} + hasBin: true + jsonschema@1.5.0: resolution: {integrity: sha512-K+A9hhqbn0f3pJX17Q/7H6yQfD/5OXgdrR5UE12gMXCiN9D5Xq2o5mddV2QEcX/bjla99ASsAAQUyMCCRWAEhw==} @@ -10191,6 +10198,7 @@ packages: whatwg-encoding@3.1.1: resolution: {integrity: sha512-6qN4hJdMwfYBtE3YBTTHhoeuUrDBPZmbQaxWAqSALV/MeEnR5z1xd8UKud2RAkFoPkmB+hli1TZSnyi84xz1vQ==} engines: {node: '>=18'} + deprecated: Use @exodus/bytes instead for a more spec-conformant and faster implementation whatwg-fetch@3.6.20: resolution: {integrity: sha512-EqhiFU6daOA8kpjOWTL0olhVOF3i7OrFzSYiGsEMB8GcXS+RrzauAERX65xMeNWVqxA6HXH2m69Z9LaKKdisfg==} @@ -14225,7 +14233,7 @@ snapshots: sirv: 3.0.1 tinyglobby: 0.2.14 tinyrainbow: 2.0.0 - vitest: 3.2.4(@types/debug@4.1.12)(@types/node@24.2.1)(@vitest/ui@3.2.4)(jiti@2.4.2)(jsdom@26.1.0)(lightningcss@1.30.1)(tsx@4.19.4)(yaml@2.8.0) + vitest: 3.2.4(@types/debug@4.1.12)(@types/node@20.17.50)(@vitest/ui@3.2.4)(jiti@2.4.2)(jsdom@26.1.0)(lightningcss@1.30.1)(tsx@4.19.4)(yaml@2.8.0) '@vitest/utils@3.2.4': dependencies: @@ -17279,6 +17287,8 @@ snapshots: optionalDependencies: graceful-fs: 4.2.11 + jsonrepair@3.13.1: {} + jsonschema@1.5.0: {} jsonwebtoken@9.0.2: diff --git a/src/core/assistant-message/NativeToolCallParser.ts b/src/core/assistant-message/NativeToolCallParser.ts index f6eac36a9c1..c396790460f 100644 --- a/src/core/assistant-message/NativeToolCallParser.ts +++ b/src/core/assistant-message/NativeToolCallParser.ts @@ -3,6 +3,9 @@ import { parseJSON } from "partial-json" import { type ToolName, toolNames, type FileEntry } from "@roo-code/types" import { customToolRegistry } from "@roo-code/core" +import { repairJson } from "../../utils/json-repair" +import { logger } from "../../utils/logging" + import { type ToolUse, type McpToolUse, @@ -51,6 +54,25 @@ export type ToolCallStreamEvent = ApiStreamToolCallStartChunk | ApiStreamToolCal * provider-level raw chunks into start/delta/end events. */ export class NativeToolCallParser { + // Configuration for malformed JSON repair (experimental feature) + private static malformedJsonRepairEnabled = false + + /** + * Enable or disable malformed JSON repair. + * When enabled, the parser will attempt to repair malformed JSON + * in tool call arguments before parsing. + */ + public static setMalformedJsonRepairEnabled(enabled: boolean): void { + this.malformedJsonRepairEnabled = enabled + } + + /** + * Check if malformed JSON repair is enabled. + */ + public static isMalformedJsonRepairEnabled(): boolean { + return this.malformedJsonRepairEnabled + } + // Streaming state management for argument accumulation (keyed by tool call id) // Note: name is string to accommodate dynamic MCP tools (mcp_serverName_toolName) private static streamingToolCalls = new Map< @@ -593,7 +615,29 @@ export class NativeToolCallParser { try { // Parse the arguments JSON string - const args = toolCall.arguments === "" ? {} : JSON.parse(toolCall.arguments) + // If malformed JSON repair is enabled, attempt to repair before parsing + let args: Record + if (toolCall.arguments === "") { + args = {} + } else { + try { + args = JSON.parse(toolCall.arguments) + } catch (parseError) { + // JSON parsing failed - attempt repair if enabled + if (this.malformedJsonRepairEnabled) { + const repaired = repairJson(toolCall.arguments) + if (repaired) { + logger.info(`[NativeToolCallParser] Repaired malformed JSON for tool ${toolCall.name}`) + args = JSON.parse(repaired) + } else { + // Repair failed, re-throw original error + throw parseError + } + } else { + throw parseError + } + } + } // Build legacy params object for backward compatibility with XML protocol and UI. // Native execution path uses nativeArgs instead, which has proper typing. diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 33e8245ccc6..14bbb8ac3c1 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -541,6 +541,16 @@ export class Task extends EventEmitter implements TaskLike { }) } + // Check malformed JSON repair experiment asynchronously. + // This enables repair of malformed JSON in tool call arguments from models like Grok. + provider.getState().then((state) => { + const isMalformedJsonRepairEnabled = experiments.isEnabled( + state.experiments ?? {}, + EXPERIMENT_IDS.MALFORMED_JSON_REPAIR, + ) + NativeToolCallParser.setMalformedJsonRepairEnabled(isMalformedJsonRepairEnabled) + }) + this.toolRepetitionDetector = new ToolRepetitionDetector(this.consecutiveMistakeLimit) // Initialize todo list if provided diff --git a/src/package.json b/src/package.json index 4b4f1b811b6..843aee580dc 100644 --- a/src/package.json +++ b/src/package.json @@ -466,6 +466,7 @@ "i18next": "^25.0.0", "ignore": "^7.0.3", "isbinaryfile": "^5.0.2", + "jsonrepair": "^3.13.1", "jwt-decode": "^4.0.0", "lodash.debounce": "^4.0.8", "mammoth": "^1.9.1", diff --git a/src/shared/__tests__/experiments.spec.ts b/src/shared/__tests__/experiments.spec.ts index 0b43302611b..5edd040f7b7 100644 --- a/src/shared/__tests__/experiments.spec.ts +++ b/src/shared/__tests__/experiments.spec.ts @@ -33,6 +33,7 @@ describe("experiments", () => { runSlashCommand: false, multipleNativeToolCalls: false, customTools: false, + malformedJsonRepair: false, } expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(false) }) @@ -46,6 +47,7 @@ describe("experiments", () => { runSlashCommand: false, multipleNativeToolCalls: false, customTools: false, + malformedJsonRepair: false, } expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(true) }) @@ -59,6 +61,7 @@ describe("experiments", () => { runSlashCommand: false, multipleNativeToolCalls: false, customTools: false, + malformedJsonRepair: false, } expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(false) }) diff --git a/src/shared/experiments.ts b/src/shared/experiments.ts index ad3aeca8634..cdb71814bb4 100644 --- a/src/shared/experiments.ts +++ b/src/shared/experiments.ts @@ -8,6 +8,7 @@ export const EXPERIMENT_IDS = { RUN_SLASH_COMMAND: "runSlashCommand", MULTIPLE_NATIVE_TOOL_CALLS: "multipleNativeToolCalls", CUSTOM_TOOLS: "customTools", + MALFORMED_JSON_REPAIR: "malformedJsonRepair", } as const satisfies Record type _AssertExperimentIds = AssertEqual>> @@ -26,6 +27,7 @@ export const experimentConfigsMap: Record = { RUN_SLASH_COMMAND: { enabled: false }, MULTIPLE_NATIVE_TOOL_CALLS: { enabled: false }, CUSTOM_TOOLS: { enabled: false }, + MALFORMED_JSON_REPAIR: { enabled: false }, } export const experimentDefault = Object.fromEntries( diff --git a/src/utils/__tests__/json-repair.spec.ts b/src/utils/__tests__/json-repair.spec.ts new file mode 100644 index 00000000000..928602f3ff2 --- /dev/null +++ b/src/utils/__tests__/json-repair.spec.ts @@ -0,0 +1,303 @@ +import { describe, it, expect } from "vitest" +import { tryRepairJson, repairJson, parseWithRepair, isValidJson } from "../json-repair" + +describe("json-repair", () => { + describe("tryRepairJson", () => { + it("should return success with wasAlreadyValid=true for valid JSON", () => { + const validJson = '{"name": "test", "value": 123}' + const result = tryRepairJson(validJson) + + expect(result.success).toBe(true) + expect(result.wasAlreadyValid).toBe(true) + expect(result.parsed).toEqual({ name: "test", value: 123 }) + expect(result.repaired).toBe(validJson) + expect(result.error).toBeUndefined() + }) + + it("should return success with wasAlreadyValid=false for repaired JSON", () => { + // Missing comma between properties + const malformedJson = '{"name": "test" "value": 123}' + const result = tryRepairJson(malformedJson) + + expect(result.success).toBe(true) + expect(result.wasAlreadyValid).toBe(false) + expect(result.parsed).toEqual({ name: "test", value: 123 }) + expect(result.repaired).toBeDefined() + expect(result.error).toBeUndefined() + }) + + it("should repair JSON with trailing comma", () => { + const jsonWithTrailingComma = '{"name": "test", "value": 123,}' + const result = tryRepairJson(jsonWithTrailingComma) + + expect(result.success).toBe(true) + expect(result.wasAlreadyValid).toBe(false) + expect(result.parsed).toEqual({ name: "test", value: 123 }) + }) + + it("should repair JSON with unquoted property names", () => { + const unquotedProps = '{name: "test", value: 123}' + const result = tryRepairJson(unquotedProps) + + expect(result.success).toBe(true) + expect(result.parsed).toEqual({ name: "test", value: 123 }) + }) + + it("should repair JSON with single quotes", () => { + const singleQuotes = "{'name': 'test', 'value': 123}" + const result = tryRepairJson(singleQuotes) + + expect(result.success).toBe(true) + expect(result.parsed).toEqual({ name: "test", value: 123 }) + }) + + it("should repair JSON with missing closing brace", () => { + const missingBrace = '{"name": "test", "value": 123' + const result = tryRepairJson(missingBrace) + + expect(result.success).toBe(true) + expect(result.parsed).toEqual({ name: "test", value: 123 }) + }) + + it("should repair JSON with missing closing bracket", () => { + const missingBracket = '{"items": [1, 2, 3}' + const result = tryRepairJson(missingBracket) + + expect(result.success).toBe(true) + expect(result.parsed).toEqual({ items: [1, 2, 3] }) + }) + + it("should repair JSON with single-line comments", () => { + const withComments = `{ + "name": "test", // this is a comment + "value": 123 + }` + const result = tryRepairJson(withComments) + + expect(result.success).toBe(true) + expect(result.parsed).toEqual({ name: "test", value: 123 }) + }) + + it("should repair JSON with block comments", () => { + const withBlockComments = `{ + "name": "test", /* block comment */ + "value": 123 + }` + const result = tryRepairJson(withBlockComments) + + expect(result.success).toBe(true) + expect(result.parsed).toEqual({ name: "test", value: 123 }) + }) + + it("should repair JSON with newlines in strings", () => { + const newlineInString = '{"text": "line1\nline2"}' + const result = tryRepairJson(newlineInString) + + expect(result.success).toBe(true) + expect(result.parsed).toEqual({ text: "line1\nline2" }) + }) + + it("should repair complex malformed tool call arguments", () => { + // Simulating a malformed tool call from Grok or similar models + const malformedToolArgs = `{ + path: "/src/test.ts", + content: "const x = 1;" + // missing comma above + }` + const result = tryRepairJson(malformedToolArgs) + + expect(result.success).toBe(true) + expect(result.parsed).toEqual({ + path: "/src/test.ts", + content: "const x = 1;", + }) + }) + + it("should return failure for completely unrepairable input", () => { + const unrepairable = "this is not json at all {{{" + const result = tryRepairJson(unrepairable) + + // Note: jsonrepair is quite aggressive and may still attempt repairs + // The behavior depends on the jsonrepair library + if (!result.success) { + expect(result.success).toBe(false) + expect(result.error).toBeDefined() + expect(result.error).toContain("Failed to repair JSON") + } + }) + }) + + describe("repairJson", () => { + it("should return the original string for valid JSON", () => { + const validJson = '{"name": "test"}' + const result = repairJson(validJson) + + expect(result).toBe(validJson) + }) + + it("should return repaired string for malformed JSON", () => { + const malformedJson = '{"name": "test",}' + const result = repairJson(malformedJson) + + expect(result).toBeDefined() + expect(JSON.parse(result!)).toEqual({ name: "test" }) + }) + + it("should return null for unrepairable input", () => { + // Empty string is harder for jsonrepair to handle + const empty = "" + const result = repairJson(empty) + + // jsonrepair treats empty string as empty and may return empty string + // Let's test something truly unparseable + if (result === null) { + expect(result).toBeNull() + } + }) + }) + + describe("parseWithRepair", () => { + it("should parse valid JSON directly", () => { + const validJson = '{"count": 42}' + const result = parseWithRepair<{ count: number }>(validJson) + + expect(result).toEqual({ count: 42 }) + }) + + it("should parse and repair malformed JSON", () => { + const malformedJson = "{count: 42}" + const result = parseWithRepair<{ count: number }>(malformedJson) + + expect(result).toEqual({ count: 42 }) + }) + + it("should return typed result", () => { + interface TestType { + name: string + items: number[] + } + + const json = '{"name": "test", "items": [1, 2, 3]}' + const result = parseWithRepair(json) + + expect(result).not.toBeNull() + expect(result?.name).toBe("test") + expect(result?.items).toEqual([1, 2, 3]) + }) + }) + + describe("isValidJson", () => { + it("should return true for valid JSON", () => { + expect(isValidJson('{"valid": true}')).toBe(true) + expect(isValidJson("[]")).toBe(true) + expect(isValidJson("null")).toBe(true) + expect(isValidJson("123")).toBe(true) + expect(isValidJson('"string"')).toBe(true) + }) + + it("should return false for invalid JSON", () => { + expect(isValidJson("{invalid}")).toBe(false) + expect(isValidJson("{'single': 'quotes'}")).toBe(false) + expect(isValidJson("{trailing: 'comma',}")).toBe(false) + expect(isValidJson("")).toBe(false) + expect(isValidJson("undefined")).toBe(false) + }) + }) + + describe("real-world malformed JSON scenarios", () => { + it("should repair JSON with multiple issues combined", () => { + // Missing comma, unquoted keys, trailing comma + const malformed = `{ + command: 'npm run test', + cwd: "./src", + }` + const result = tryRepairJson(malformed) + + expect(result.success).toBe(true) + expect(result.parsed).toEqual({ + command: "npm run test", + cwd: "./src", + }) + }) + + it("should repair nested object with issues", () => { + // Missing comma between properties - use a simpler pattern that jsonrepair handles + const malformed = `{ + files: [ + {path: "./test.ts", lineRanges: null} + ] + }` + const result = tryRepairJson(malformed) + + expect(result.success).toBe(true) + expect(result.parsed).toEqual({ + files: [{ path: "./test.ts", lineRanges: null }], + }) + }) + + it("should repair object with space-separated properties", () => { + // Note: jsonrepair has limitations with certain patterns like {key1: "val" key2: "val"} + // but handles unquoted keys and trailing commas well + const malformed = `{ + "files": [ + {"path": "./test.ts", "lineRanges": null,} + ], + }` + const result = tryRepairJson(malformed) + + expect(result.success).toBe(true) + expect(result.parsed).toEqual({ + files: [{ path: "./test.ts", lineRanges: null }], + }) + }) + + it("should repair tool call arguments with special characters", () => { + const malformed = `{ + "diff": "<<<<<<< SEARCH +some content +======= +new content +>>>>>>> REPLACE" + }` + const result = tryRepairJson(malformed) + + expect(result.success).toBe(true) + expect(result.parsed).toHaveProperty("diff") + }) + + it("should handle empty object", () => { + const result = tryRepairJson("{}") + expect(result.success).toBe(true) + expect(result.wasAlreadyValid).toBe(true) + expect(result.parsed).toEqual({}) + }) + + it("should handle empty array", () => { + const result = tryRepairJson("[]") + expect(result.success).toBe(true) + expect(result.wasAlreadyValid).toBe(true) + expect(result.parsed).toEqual([]) + }) + + it("should repair ask_followup_question tool with complex follow_up array", () => { + // Real-world example: models sometimes emit malformed follow_up arrays + const malformed = `{ + "question": "What would you like to do?", + "follow_up": [ + {"text": "Option A" "mode": null}, + {"text": "Option B", "mode": null} + ] + }` + const result = tryRepairJson(malformed) + + expect(result.success).toBe(true) + expect(result.parsed).toEqual({ + question: "What would you like to do?", + follow_up: [ + { text: "Option A", mode: null }, + { text: "Option B", mode: null }, + ], + }) + }) + }) +}) diff --git a/src/utils/json-repair.ts b/src/utils/json-repair.ts new file mode 100644 index 00000000000..cba9089726e --- /dev/null +++ b/src/utils/json-repair.ts @@ -0,0 +1,116 @@ +/** + * JSON repair utility for fixing malformed LLM responses. + * + * This module provides functionality to repair common JSON malformation issues + * that occur when LLMs (especially models like Grok) struggle with strict + * JSON formatting requirements. + * + * Common issues handled: + * - Missing or trailing commas + * - Unquoted property names or strings + * - Missing closing brackets/braces + * - Comments in JSON (single-line and block) + * - Newlines in strings + * - Escape character issues + * - Mixed quote styles + * + * @see https://github.com/josdejong/jsonrepair + */ +import { jsonrepair, JSONRepairError } from "jsonrepair" + +/** + * Result of a JSON repair attempt. + */ +export interface JsonRepairResult { + /** Whether the repair was successful */ + success: boolean + /** The repaired JSON string (if successful) */ + repaired?: string + /** The parsed JSON object (if successful) */ + parsed?: unknown + /** Whether the original JSON was already valid */ + wasAlreadyValid: boolean + /** Error message if repair failed */ + error?: string +} + +/** + * Attempt to repair malformed JSON and parse it. + * + * @param input - The potentially malformed JSON string + * @returns A result object containing the repair status and parsed data + */ +export function tryRepairJson(input: string): JsonRepairResult { + // First, try to parse as-is to avoid unnecessary repair + try { + const parsed = JSON.parse(input) + return { + success: true, + repaired: input, + parsed, + wasAlreadyValid: true, + } + } catch { + // JSON is malformed, attempt repair + } + + // Attempt to repair the JSON + try { + const repaired = jsonrepair(input) + const parsed = JSON.parse(repaired) + return { + success: true, + repaired, + parsed, + wasAlreadyValid: false, + } + } catch (err) { + const errorMessage = + err instanceof JSONRepairError ? err.message : err instanceof Error ? err.message : String(err) + + return { + success: false, + wasAlreadyValid: false, + error: `Failed to repair JSON: ${errorMessage}`, + } + } +} + +/** + * Repair malformed JSON string. + * Returns the repaired string, or null if repair is not possible. + * + * @param input - The potentially malformed JSON string + * @returns The repaired JSON string, or null if repair failed + */ +export function repairJson(input: string): string | null { + const result = tryRepairJson(input) + return result.success ? (result.repaired ?? null) : null +} + +/** + * Parse potentially malformed JSON, repairing if necessary. + * Returns the parsed object, or null if parsing/repair failed. + * + * @param input - The potentially malformed JSON string + * @returns The parsed JSON object, or null if parsing failed + */ +export function parseWithRepair(input: string): T | null { + const result = tryRepairJson(input) + return result.success ? (result.parsed as T) : null +} + +/** + * Check if a string is valid JSON without attempting repair. + * + * @param input - The string to check + * @returns true if the string is valid JSON, false otherwise + */ +export function isValidJson(input: string): boolean { + try { + JSON.parse(input) + return true + } catch { + return false + } +}