Skip to content

Commit a956a06

Browse files
grypezclaude
andauthored
fix(kernel-agents): use external json parsing (#853)
## Summary - Replace the bracket-counting heuristic in `sample-collector.ts` with the [`partial-json`](https://github.com/promplate/partial-json-parser-js) library to correctly distinguish incomplete-but-valid JSON from malformed JSON during LLM streaming - Fix false positives when streamed JSON contains brace characters inside string values (e.g. `{"think": "add a '}' here"}`) - Use `Allow.OBJ` to reject non-object partial JSON early - Wrap all `partial-json` errors in `SampleGenerationError` for consistent error handling Closes #673 <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes core streaming-response parsing behavior, which can alter when/why `SampleGenerationError` is thrown and may affect downstream agent flows; impact is contained to JSON strategy parsing and is test-covered. > > **Overview** > Replaces the `makeSampleCollector` streaming JSON parsing heuristic (brace counting + `JSON.parse`) with `partial-json` to distinguish **complete**, **incomplete**, and **malformed** buffers during LLM streaming, and to avoid false failures when braces appear inside string values. > > Adds `partial-json` as a dependency and updates tests to reflect the new error behavior (earlier rejection of non-JSON text, adjusted max-chunk test input, and new coverage for braces-in-string handling). > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 418cc32. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent f7b4257 commit a956a06

4 files changed

Lines changed: 61 additions & 34 deletions

File tree

packages/kernel-agents/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,7 @@
187187
"@metamask/kernel-utils": "workspace:^",
188188
"@metamask/logger": "workspace:^",
189189
"@ocap/kernel-language-model-service": "workspace:^",
190+
"partial-json": "^0.1.7",
190191
"ses": "^1.14.0"
191192
}
192193
}

packages/kernel-agents/src/strategies/json/sample-collector.test.ts

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,24 +48,24 @@ describe('makeSampleCollector', () => {
4848
);
4949
});
5050

51-
it('throws error for invalid JSON', () => {
52-
const invalidJson = '{"invalid": json}';
51+
it('throws error for malformed JSON', () => {
5352
const collector = makeSampleCollector({});
54-
expect(() => collector(invalidJson)).toThrow(
53+
expect(() => collector('not json at all')).toThrow(
5554
expect.objectContaining({
5655
message: 'LLM generated invalid response.',
5756
cause: expect.objectContaining({
58-
message: expect.stringContaining(invalidJson),
57+
message: expect.stringContaining('at position'),
5958
}),
6059
}),
6160
);
6261
});
6362

6463
it('throws error when max chunk count exceeded', () => {
6564
const collector = makeSampleCollector({ maxChunkCount: 2 });
66-
collector('chunk1');
67-
collector('chunk2');
68-
expect(() => collector('chunk3')).toThrow(
65+
// Use valid partial JSON so partial-json does not flag it early
66+
collector('{"key": "');
67+
collector('aaa');
68+
expect(() => collector('bbb')).toThrow(
6969
expect.objectContaining({
7070
message: 'LLM generated invalid response.',
7171
cause: expect.objectContaining({
@@ -74,4 +74,27 @@ describe('makeSampleCollector', () => {
7474
}),
7575
);
7676
});
77+
78+
it('handles braces inside string values without false errors', () => {
79+
const collector = makeSampleCollector({});
80+
// Simulates an LLM streaming: { think: "I should add a '}' to the..."
81+
// The old bracket-counting heuristic would false-positive here because
82+
// the '}' inside the string balances the opening brace.
83+
expect(collector('{"think": "I should add a ')).toBeNull();
84+
expect(collector("'}' to the")).toBeNull();
85+
expect(collector(' response", "done": true}')).toStrictEqual({
86+
think: "I should add a '}' to the response",
87+
done: true,
88+
});
89+
});
90+
91+
it('detects genuinely malformed JSON early', () => {
92+
const collector = makeSampleCollector({ maxChunkCount: 100 });
93+
// Bare text is structurally invalid — partial-json detects this immediately
94+
expect(() => collector('this is not json')).toThrow(
95+
expect.objectContaining({
96+
message: 'LLM generated invalid response.',
97+
}),
98+
);
99+
});
77100
});

packages/kernel-agents/src/strategies/json/sample-collector.ts

Lines changed: 22 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,16 @@
11
import { SampleGenerationError } from '@metamask/kernel-errors';
22
import type { Logger } from '@metamask/logger';
3+
import { parse, MalformedJSON, PartialJSON } from 'partial-json';
34

45
import type { SampleCollector } from '../../types.ts';
56

67
/**
7-
* A quick and dirty sample collector for a streaming response.
8+
* A sample collector for a streaming JSON object response.
9+
*
10+
* Uses `partial-json` for a three-way check on each chunk:
11+
* - **Complete** — `parse(buffer, 0)` returns the parsed object.
12+
* - **Incomplete** — throws `PartialJSON`, meaning the buffer is valid so far.
13+
* - **Malformed** — throws `MalformedJSON`, meaning the buffer is irrecoverable.
814
*
915
* @param args - The arguments to make the sample collector.
1016
* @param args.prefix - The prefix to prepend to the response
@@ -24,36 +30,25 @@ export const makeSampleCollector = <Result = unknown>({
2430
}): SampleCollector<Result> => {
2531
let response = prefix;
2632
let chunkCount = 0;
27-
let leftBracketCount = prefix.split('{').length - 1;
28-
let rightBracketCount = prefix.split('}').length - 1;
2933
return (delta: string) => {
3034
chunkCount += 1;
31-
const subchunks = delta.split('}');
32-
const lastSubchunk = subchunks.pop() as string;
33-
for (const subchunk of subchunks) {
34-
rightBracketCount += 1;
35-
leftBracketCount += subchunk.split('{').length - 1;
36-
response += `${subchunk}}`;
37-
logger?.info('toParse:', response);
38-
try {
39-
const result = JSON.parse(response);
40-
logger?.info('parsed:', result);
41-
return result;
42-
} catch (cause) {
43-
// XXX There are other ways to detect an irrecoverable state.
44-
// This is the simplest.
45-
if (leftBracketCount === rightBracketCount) {
46-
throw new SampleGenerationError(
47-
response,
48-
cause instanceof Error
49-
? cause
50-
: new Error('Invalid JSON', { cause }),
51-
);
52-
}
35+
response += delta;
36+
logger?.info('toParse:', response);
37+
38+
try {
39+
const result = parse(response, 0);
40+
logger?.info('parsed:', result);
41+
return result;
42+
} catch (error) {
43+
if (error instanceof PartialJSON) {
44+
// Buffer is incomplete but structurally valid.
45+
} else if (error instanceof MalformedJSON) {
46+
throw new SampleGenerationError(response, error);
47+
} else {
48+
throw error;
5349
}
5450
}
55-
leftBracketCount += lastSubchunk.split('{').length - 1;
56-
response += lastSubchunk;
51+
5752
if (maxChunkCount && chunkCount > maxChunkCount) {
5853
throw new SampleGenerationError(
5954
response,

yarn.lock

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3674,6 +3674,7 @@ __metadata:
36743674
eslint-plugin-n: "npm:^17.17.0"
36753675
eslint-plugin-prettier: "npm:^5.2.6"
36763676
eslint-plugin-promise: "npm:^7.2.1"
3677+
partial-json: "npm:^0.1.7"
36773678
prettier: "npm:^3.5.3"
36783679
rimraf: "npm:^6.0.1"
36793680
ses: "npm:^1.14.0"
@@ -12398,6 +12399,13 @@ __metadata:
1239812399
languageName: node
1239912400
linkType: hard
1240012401

12402+
"partial-json@npm:^0.1.7":
12403+
version: 0.1.7
12404+
resolution: "partial-json@npm:0.1.7"
12405+
checksum: 10/6fb8d305638e4747fccfc0649b8a8de99314bfafcc4e3136a3a27c0632857c40b62e6e6bb5f17af00472387393f1d8a58a7865242ca20382af31b593a7bb1f89
12406+
languageName: node
12407+
linkType: hard
12408+
1240112409
"path-browserify@npm:^1.0.1":
1240212410
version: 1.0.1
1240312411
resolution: "path-browserify@npm:1.0.1"

0 commit comments

Comments
 (0)