Skip to content

Commit 1548d4f

Browse files
committed
fix: enhance JSON parsing to handle multiple objects and add corresponding tests
1 parent 5339bcc commit 1548d4f

File tree

9 files changed

+152
-40
lines changed

9 files changed

+152
-40
lines changed

deno.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/cli/deno.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@mcpc/cli",
3-
"version": "0.1.6",
3+
"version": "0.1.7",
44
"repository": {
55
"type": "git",
66
"url": "git+https://github.com/mcpc-tech/mcpc.git"

packages/core/deno.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@mcpc/core",
3-
"version": "0.2.5",
3+
"version": "0.2.6",
44
"repository": {
55
"type": "git",
66
"url": "git+https://github.com/mcpc-tech/mcpc.git"

packages/core/src/executors/sampling/agentic-sampling-executor.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -189,10 +189,10 @@ export class SamplingExecutor extends BaseSamplingExecutor {
189189
const taskPrompt = `
190190
191191
## Current Task
192-
I will now use agentic sampling to complete the following task: "${userRequest}"${contextInfo}
192+
You will now use agentic sampling to complete the following task: "${userRequest}"${contextInfo}
193193
194-
When I need to use a tool, I should specify the tool name in 'action' and provide tool-specific parameters as additional properties.
195-
When the task is complete, I should use "action": "complete".`;
194+
When you need to use a tool, specify the tool name in 'action' and provide tool-specific parameters as additional properties.
195+
When the task is complete, use "action": "complete".`;
196196

197197
// Use JSON instruction injection pattern
198198
return this.injectJsonInstruction({

packages/core/src/executors/sampling/base-sampling-executor.ts

Lines changed: 17 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
22
import type { ComposableMCPServer } from "../../compose.ts";
33
import type { SamplingConfig } from "../../types.ts";
4-
import { CompiledPrompts } from "../../prompts/index.ts";
54
import { Ajv } from "ajv";
65
import { AggregateAjvError } from "@segment/ajv-human-errors";
76
import addFormats from "ajv-formats";
@@ -103,7 +102,7 @@ export abstract class BaseSamplingExecutor {
103102
content: {
104103
type: "text",
105104
text:
106-
'Return ONLY raw JSON (no code fences or explanations). The JSON MUST include action and decision. Example: {"action":"<tool>","decision":"proceed|complete","<tool>":{}}',
105+
'Return ONE AND ONLY ONE raw JSON object (no code fences, explanations, or multiple objects). The JSON MUST include action and decision. Example: {"action":"<tool>","decision":"proceed|complete","<tool>":{}}',
107106
},
108107
}];
109108

@@ -160,15 +159,14 @@ export abstract class BaseSamplingExecutor {
160159
continue;
161160
}
162161

163-
if (parsedData) {
164-
this.conversationHistory.push({
165-
role: "assistant",
166-
content: {
167-
type: "text",
168-
text: JSON.stringify(parsedData, null, 2),
169-
},
170-
});
171-
}
162+
// Always show LLM what we parsed - this allows self-correction
163+
this.conversationHistory.push({
164+
role: "assistant",
165+
content: {
166+
type: "text",
167+
text: JSON.stringify(parsedData, null, 2),
168+
},
169+
});
172170

173171
const action = parsedData["action"];
174172

@@ -285,28 +283,18 @@ export abstract class BaseSamplingExecutor {
285283
}
286284

287285
protected addParsingErrorToHistory(
288-
responseText: string,
286+
_responseText: string,
289287
parseError: unknown,
290288
): void {
291-
this.conversationHistory.push({
292-
role: "assistant",
293-
content: {
294-
type: "text",
295-
text: `JSON parsing failed. Response was: ${responseText}`,
296-
},
297-
});
289+
const errorMsg = parseError instanceof Error
290+
? parseError.message
291+
: String(parseError);
298292

299293
this.conversationHistory.push({
300294
role: "user",
301295
content: {
302296
type: "text",
303-
text: CompiledPrompts.errorResponse({
304-
errorMessage: `JSON parsing failed: ${
305-
parseError instanceof Error
306-
? parseError.message
307-
: String(parseError)
308-
}\n\nPlease respond with valid JSON.`,
309-
}),
297+
text: `Invalid JSON: ${errorMsg}\n\nRespond with valid JSON.`,
310298
},
311299
});
312300
}
@@ -513,11 +501,11 @@ ${history}`,
513501
schema,
514502
schemaPrefix = "JSON schema:",
515503
schemaSuffix = `STRICT REQUIREMENTS:
516-
1. Return ONLY raw JSON that passes JSON.parse() - no markdown, code blocks, explanatory text, or extra characters
504+
1. Return ONE AND ONLY ONE raw JSON object that passes JSON.parse() - no markdown, code blocks, explanatory text, or multiple JSON objects
517505
2. Include ALL required fields with correct data types and satisfy ALL schema constraints (anyOf, oneOf, allOf, not, enum, pattern, min/max, conditionals)
518-
3. Your response must be the JSON object itself, nothing else
506+
3. Your response must be a single JSON object, nothing else
519507
520-
INVALID: \`\`\`json{"key":"value"}\`\`\` or "Here is: {"key":"value"}"
508+
INVALID: \`\`\`json{"key":"value"}\`\`\` or "Here is: {"key":"value"}" or {"key":"value"}{"key":"value"}
521509
VALID: {"key":"value"}`,
522510
}: {
523511
prompt?: string;

packages/mcp-sampling-ai-provider/deno.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@mcpc/mcp-sampling-ai-provider",
3-
"version": "0.1.7",
3+
"version": "0.1.8",
44
"repository": {
55
"type": "git",
66
"url": "git+https://github.com/mcpc-tech/mcpc.git"

packages/utils/deno.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@mcpc/utils",
3-
"version": "0.2.2",
3+
"version": "0.2.3",
44
"repository": {
55
"type": "git",
66
"url": "git+https://github.com/mcpc-tech/mcpc.git"
@@ -15,6 +15,7 @@
1515
},
1616
"imports": {
1717
"@modelcontextprotocol/sdk": "npm:@modelcontextprotocol/sdk@^1.8.0",
18+
"@std/assert": "jsr:@std/assert@^1",
1819
"@std/http": "jsr:@std/http@^1.0.14",
1920
"cheerio": "npm:cheerio@^1.0.0",
2021
"jsonrepair": "npm:jsonrepair@^3.13.0"

packages/utils/src/json.ts

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,54 @@ function stripMarkdownAndText(text: string): string {
1717
"",
1818
);
1919

20-
// Try to find JSON object/array boundaries if there's surrounding text
21-
const jsonMatch = text.match(/(\{[\s\S]*\}|\[[\s\S]*\])/);
22-
if (jsonMatch) {
23-
text = jsonMatch[1];
20+
// Try to extract the FIRST complete JSON object or array
21+
// This handles cases where LLM returns multiple JSON objects
22+
const firstJsonIndex = text.search(/[\{\[]/);
23+
if (firstJsonIndex >= 0) {
24+
text = text.slice(firstJsonIndex);
25+
26+
// Find the end of the first complete JSON structure
27+
let depth = 0;
28+
let inString = false;
29+
let escapeNext = false;
30+
const startChar = text[0];
31+
const endChar = startChar === "{" ? "}" : "]";
32+
33+
for (let i = 0; i < text.length; i++) {
34+
const char = text[i];
35+
36+
if (escapeNext) {
37+
escapeNext = false;
38+
continue;
39+
}
40+
41+
if (char === "\\") {
42+
escapeNext = true;
43+
continue;
44+
}
45+
46+
if (char === '"' && !inString) {
47+
inString = true;
48+
continue;
49+
}
50+
51+
if (char === '"' && inString) {
52+
inString = false;
53+
continue;
54+
}
55+
56+
if (inString) continue;
57+
58+
if (char === startChar) {
59+
depth++;
60+
} else if (char === endChar) {
61+
depth--;
62+
if (depth === 0) {
63+
// Found the end of the first complete JSON structure
64+
return text.slice(0, i + 1);
65+
}
66+
}
67+
}
2468
}
2569

2670
return text.trim();
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
import { assertEquals } from "@std/assert";
2+
import { parseJSON } from "../src/json.ts";
3+
4+
Deno.test("parseJSON - handles multiple JSON objects by taking the first one", () => {
5+
const duplicateJson =
6+
'{"action":"filesystem_read_file","decision":"proceed","nextAction":"filesystem_read_file","filesystem_read_file":{"path":"codex-rs/core/src/mcp_connection_manager.rs"}}{"action":"filesystem_read_file","decision":"proceed","nextAction":"filesystem_read_file","filesystem_read_file":{"path":"codex-rs/core/src/mcp_connection_manager.rs"}}';
7+
8+
const result = parseJSON(duplicateJson, true);
9+
10+
assertEquals(result, {
11+
action: "filesystem_read_file",
12+
decision: "proceed",
13+
nextAction: "filesystem_read_file",
14+
filesystem_read_file: {
15+
path: "codex-rs/core/src/mcp_connection_manager.rs",
16+
},
17+
});
18+
});
19+
20+
Deno.test("parseJSON - handles nested objects correctly", () => {
21+
const nestedJson =
22+
'{"outer":{"inner":{"deep":"value"},"array":[1,2,3]},"next":"field"}{"should":"ignore"}';
23+
24+
const result = parseJSON(nestedJson, true);
25+
26+
assertEquals(result, {
27+
outer: {
28+
inner: {
29+
deep: "value",
30+
},
31+
array: [1, 2, 3],
32+
},
33+
next: "field",
34+
});
35+
});
36+
37+
Deno.test("parseJSON - handles JSON arrays", () => {
38+
const arrayJson = '[{"a":1},{"b":2}][{"c":3}]';
39+
40+
const result = parseJSON(arrayJson, true);
41+
42+
assertEquals(result, [{ a: 1 }, { b: 2 }]);
43+
});
44+
45+
Deno.test("parseJSON - handles strings with quotes correctly", () => {
46+
const stringWithQuotes =
47+
'{"message":"He said \\"hello\\" to me"}{"ignore":"this"}';
48+
49+
const result = parseJSON(stringWithQuotes, true);
50+
51+
assertEquals(result, {
52+
message: 'He said "hello" to me',
53+
});
54+
});
55+
56+
Deno.test("parseJSON - handles markdown code fences with multiple objects", () => {
57+
const markdownJson =
58+
'```json\n{"action":"test","value":123}{"extra":"object"}\n```';
59+
60+
const result = parseJSON(markdownJson, true);
61+
62+
assertEquals(result, {
63+
action: "test",
64+
value: 123,
65+
});
66+
});
67+
68+
Deno.test("parseJSON - handles explanatory text before JSON", () => {
69+
const textBeforeJson =
70+
'Here is the response: {"action":"test","value":123}{"extra":"object"}';
71+
72+
const result = parseJSON(textBeforeJson, true);
73+
74+
assertEquals(result, {
75+
action: "test",
76+
value: 123,
77+
});
78+
});

0 commit comments

Comments
 (0)