fix(serve): repair invalid backslash escapes in typed tool-call parameters

[gregtakacs]

Concrete problem

A strictly-typed (array/object/...) tool-call parameter whose JSON value contains a backslash the model didn't double for JSON escaping makes parse_parameter() in src/serve/tool_call_parser.cpp reject the entire tool call and fall back to returning the raw <tool_call>... text as plain content, discarding an otherwise completely well-formed call.

Concretely: a run_commands tool with an array-typed commands parameter, where the model emits a shell command containing a literal regex/grep backslash escape (e.g. grep -n '^\*\|^* '). \* and \| aren't legal JSON string escapes (JSON only allows \" \\ \/ \b \f \n \r \t \uXXXX), so Json::parse discards the value, parse_parameter returns false, and the caller falls all the way back to fallback(text) — even though the tool-call structure itself (tags, function name, other parameters) was perfectly valid.

This is a different failure mode from #10 (structural tag/wrapper drift): the JSON here is syntactically complete, just contains one un-escaped backslash inside an otherwise well-formed value, and it's the first call in the response, so #10's tolerant recovery (which only engages once at least one call has already been successfully parsed) never gets a chance to help.

Reproduced against a real qwen3_8_27b_nvfp4 NVFP4 artifact under Cline-style long agentic tool-use sessions.

Design and why it's appropriate

In the strict (non-string, contract-typed) decode branch of parse_parameter, retry once against a repaired copy of the value before giving up:

Json parsed = Json::parse(value, nullptr, false);
if (parsed.is_discarded()) {
    parsed = Json::parse(repair_invalid_json_escapes(value), nullptr, false);
    if (parsed.is_discarded()) { return false; }
}
args[key] = std::move(parsed);

repair_invalid_json_escapes is a narrow, single-pass repair: it tracks whether it's inside a JSON string and doubles a backslash only when it isn't already the first character of a legal JSON escape (\" \\ \/ \b \f \n \r \t \u). Legal escapes are copied through byte-for-byte untouched. It never touches structural characters outside strings, and it's applied only as a fallback after strict decode already failed — the common (already-valid) case never runs it, and genuinely broken JSON (truncated strings, mismatched brackets — not just an escaping quirk) still fails to parse after repair and correctly falls through to the existing rejection path unchanged.

This is deliberately scoped to the exact failure observed rather than a general lenient-JSON parser: it repairs only the specific defect (an un-doubled backslash), so it can't silently accept or misinterpret other classes of malformed input.

Affected behavior/contract

parse_qwen_tool_call_output's typed-parameter decode path in src/serve/tool_call_parser.cpp only. No CLI flag, no public API change, no schema version bump — this is purely an internal parsing robustness fix. Untyped ("legacy"/no-contract) parameters were already unaffected by this failure (they degrade to a string on parse failure rather than rejecting the call), so this only changes behavior for tools with explicit non-string JSON Schema parameter types.

Verification

cmake -S . -B build -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTING=ON
cmake --build build --parallel --target ninfer_tool_call_parser_test
./build/tests/ninfer_tool_call_parser_test

Result: ok (all cases pass), built clean against current master (2190c4a1) in an nvidia/cuda:13.3.1-devel-ubuntu24.04 container.

Added test_typed_parameter_repairs_invalid_backslash_escape in tests/test_tool_call_parser.cpp, covering:

• a typed array parameter with an un-doubled \*/\| is now recovered, decoding to the correct literal-backslash string;
• a sibling array element with genuinely legal escapes (\n, \") round-trips untouched by the repair pass;
• a structurally truncated/unterminated value (not just an escaping defect) still correctly fails and falls back to raw text — repair does not mask real errors.

No performance claim is made; this is a correctness fix on an already-failing path, not a modification to a hot loop (only reached once, after a strict decode has already failed).

Not run

GPU-dependent tests (anything requiring an actual device/artifact) were not run — this change doesn't touch engine/runtime code, only the OpenAI-facing tool-call text parser, and the added test is a pure CPU unit test.

AI usage disclosure

This implementation and its test were generated entirely by Claude Sonnet 5 (claude-sonnet-5). I (the submitter) reviewed the complete diff, reasoned through the escape-handling edge cases (legal-escape preservation, string-boundary tracking, genuinely-broken-JSON non-recovery) and the verification above, and take full responsibility for this change.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 59a4ad0100

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Validate full Unicode escapes before preserving them

When a typed object or array contains an undoubled backslash followed by u but not four hexadecimal digits—for example the model output C:\users—this condition classifies \u as legal without inspecting the required digits. The repaired text is therefore unchanged, the retry still fails JSON parsing, and the entire otherwise valid tool call falls back to plain content. Preserve \u only when a complete four-hex-digit escape follows; otherwise double the backslash like the other invalid escapes.

Useful? React with 👍 / 👎.

[gregtakacs]

Good catch, fixed in 8ef19f2 — now requires exactly 4 hex digits after \u before treating it as legal (matching JSON's actual \uXXXX grammar); anything else, including a bare \u, now gets the backslash doubled like any other illegal escape. Added regression coverage for both the Windows-path case (C:\users\greg) and a genuine \uXXXX escape to confirm it still round-trips untouched.

[Neroued]

Thanks for the report. However, ninfer intentionally does not repair malformed model output. It follows the declared tool call contract and json schema instead.

[gregtakacs]

Understood, thanks for the quick response — makes sense as a deliberate design boundary. We'll keep this as a local patch on our end. Appreciate the review either way, including catching the \u edge case.