fix(llm): repair \u escapes that lack four hex digits in streaming JSON#88226
Conversation
openclaw-barnacle
Bot
added
size: XS
proof: supplied
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR addresses two edge cases where formatting/parsing could produce invalid or non-parseable outputs: token counts rounding to 1000k, and JSON strings containing invalid \u escapes.
Changes:
- Prevent
formatTokenShortfrom emitting"1000k"by falling through to the million formatter at the rollover boundary. - Make
repairJsonhandle invalid\uescapes by escaping the backslash so the JSON becomes parseable. - Add targeted unit tests covering both boundary/repair behaviors (including a streaming JSON scenario).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/shared/subagents-format.ts | Avoids "1000k" by falling through to "1m" when rounding thousands reaches 1000. |
| src/shared/subagents-format.test.ts | Adds tests for the thousands→millions rollover boundary. |
| src/llm/utils/json-parse.ts | Fixes repairJson to escape invalid \u sequences so parsing succeeds. |
| src/llm/utils/json-parse.test.ts | Adds tests for invalid/valid \u handling and streaming args recovery. |
|
Codex review: needs maintainer review before merge. Reviewed May 30, 2026, 3:39 PM ET / 19:39 UTC. Summary PR surface: Source +5, Tests +147, Other +146. Total +298 across 7 files. Reproducibility: yes. Source inspection shows current main falls through from an invalid short Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the parser repair once maintainers are satisfied that the CI shard split is intentional and freshly verified. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection shows current main falls through from an invalid short Is this the best way to solve the issue? Yes for the parser repair: treating a short AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 37c6e2dfa0bf. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +5, Tests +147, Other +146. Total +298 across 7 files. View PR surface stats
Acceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
clawsweeper
Bot
added
rating: 🦐 gold shrimp
51d8c26 to
17fd128
Compare
|
@clawsweeper re-review I split this PR down to a single purpose. The Your prior review already judged the parser proof sufficient ("Copied terminal output sufficiently supports the parser repair"); the gold-shrimp/needs-proof rating was driven by the formatter behavior, which is now removed. Please re-review the current single-purpose head ( |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
17fd128 to
7610282
Compare
clawsweeper
Bot
added
proof: sufficient
7610282 to
18c802b
Compare
openclaw-barnacle
Bot
removed
the
proof: sufficient
clawsweeper
Bot
added
proof: sufficient
18c802b to
845c0cd
Compare
openclaw-barnacle
Bot
removed
the
proof: sufficient
repairJson treated a backslash-u not followed by four hex digits as a valid
escape ("u" is in VALID_JSON_ESCAPES) and re-emitted it un-doubled, so the
output still failed JSON.parse — defeating the repair. parseStreamingJson then
fell through to an empty object, silently dropping tool-call arguments.
This hits the streaming tool-call argument path for both Anthropic and OpenAI
(parseStreamingJson) whenever a model emits an unescaped backslash-u string,
e.g. a Windows path (C:\users), LaTeX (\underline), or a regex.
Double the backslash for an invalid \u, matching the other invalid-escape
handling. Add regression tests (the file had no coverage).
Co-Authored-By: Claude Opus 4.8 <[email protected]>
clawsweeper
Bot
added
the
proof: sufficient
845c0cd to
c8f0af6
Compare
openclaw-barnacle
Bot
added
scripts
openclaw-barnacle
Bot
removed
the
proof: sufficient
clawsweeper
Bot
added
rating: 🌊 off-meta tidepool
clawsweeper
Bot
added
proof: sufficient
openclaw-barnacle
Bot
removed
the
proof: sufficient
clawsweeper
Bot
added
the
proof: sufficient
openclaw-barnacle
Bot
removed
the
proof: sufficient
|
Verification before merge: Behavior addressed: invalid streaming JSON Unicode escapes are repaired without corrupting valid
|
Summary
repairJsoninsrc/llm/utils/json-parse.tsis meant to fix malformed JSON string literals (it escapes raw control chars and doubles backslashes before invalid escapes). But it mishandled a backslash-uthat is not followed by four hex digits:\uXXXXpath (4-hex check) is skipped, then control falls through to the generic valid-escape branch — and"u"is inVALID_JSON_ESCAPES, so the broken\uis re-emitted un-doubled. The output still failsJSON.parse, defeating the repair.parseJsonWithRepaironly retries whenrepairJsonactually changed the string, so it throws;parseStreamingJsonthen falls through its fallbacks to an empty object, silently dropping the tool call's arguments.User impact:
parseStreamingJsonparses streaming tool-call arguments for both Anthropic and OpenAI (src/llm/providers/anthropic.ts,src/llm/providers/openai-responses-shared.ts,openai-completions.ts). Whenever a model emits a string value containing an unescaped backslash-u— a Windows path (C:\users), LaTeX (\underline), a regex, etc. — the tool is invoked with empty/dropped arguments.Fix
When a
\uis not followed by four hex digits, double the backslash (treat it as an invalid escape), matching how the function already handles other invalid escapes.VALID_JSON_ESCAPESis left intact. Added regression tests (the file had no coverage).Real behavior proof (required for external PRs)
Behavior addressed: streaming tool-call arguments containing an unescaped backslash-
ustring were silently reduced to{}becauserepairJsonre-emitted the broken escape instead of repairing it. After this patch the arguments parse and are recovered.Real environment tested: local macOS checkout on fresh main. The proof drives the real production functions
repairJson/parseJsonWithRepair/parseStreamingJson— the exact functions the Anthropic and OpenAI streaming transports call to parse tool-call arguments. No mocks of the functions under test.Exact steps or command run after this patch: I added regression rows for the invalid-
\ucases and the valid\uXXXXcase, ran them against the unpatchedrepairJson(the invalid-\urows threw / returned{}), then applied the fix and re-ran. Command:Evidence after fix: copied live console output from
node scripts/run-vitest.mjs—Observed result after fix:
parseStreamingJson('{"cmd":"\underline{x}"}')now returns{ cmd: "\underline{x}" }instead of{}, andparseJsonWithRepair('{"path":"C:\users"}')returns{ path: "C:\users" }; the validAescape still decodes to"A".What was not tested: I did not run a live streaming model emitting such tool arguments end-to-end; the proof drives the exact parser functions the Anthropic/OpenAI transports call, with the offending inputs.
Tests and validation
node scripts/run-vitest.mjs src/llm/utils/json-parse.test.ts-> 3 passed (new file; invalid-\urepair, streaming-arg recovery, valid\uXXXXpreserved).src/llm/utils/suite -> 7 files / 34 tests pass (no regressions).\urows fail on the unpatched source and pass with the fix.oxfmt --checkclean,oxlintclean,tsgo:coreexit 0,tsgo:core:testexit 0,git diff --checkclean.