feat(symphony): a strict JSON prefix acceptor keeps the fragment promise

[slin1237]

Draft: waits for bellwether #20 (malformed-argument cases), the fixture ground rule 1 asks for, and for #2806 below it.

Description

Problem

ToolCallArguments promises that the fragments emitted so far always form a valid JSON prefix. The assembler (#2792) and the constrained format (#2806) kept that promise with the ported prefix parser in prefix mode, which is as lenient as the old crate's: it tolerates a bracket closed by the wrong kind, and its whole-word literal rule accepts true in one piece but refuses truex from the first byte. So where fragments stop and InvalidArguments begins depended on how the output was cut: Alex's harnesses found the boundary moving for truex on #2792 and again on #2806, and the contract corpora had to leave out junk that begins like a literal. The decision log carried this as a compromise, to be removed by a strict acceptor.

Solution

json::prefix::scan, a strict acceptor of JSON prefixes: a byte-by-byte run of RFC 8259's grammar over the value's text so far, returning how many leading bytes some JSON value could still continue (valid) and, once the text holds a whole value, where it ends (complete). It is a function of the text alone, so the boundary is a property of the output, not of the chunking. Objects with string keys, arrays, strings with their escapes and the four hex digits after \u, numbers without leading zeros, the three literals, the four whitespace bytes; nesting beyond 32 refused, as the ported parser refused it. Two readings are deliberate and documented: a number at the very end of the text is a valid prefix but never whole, since more digits may come; and a raw control character inside a string is taken as content, as the model wrote it.

The assembler runs each new run of argument bytes through scan instead of the prefix parser, and the constrained format's forced-function mode uses scan for both the prefix check and the end of the whole value, replacing the prefix parser and is_complete. The ported prefix parser stays, for the places that need the value rather than its bytes.

What changes for a client: nothing for well-formed arguments, with one tolerated malformation that moves: a trailing comma before a closing bracket, {"a": 1,}, which the ported parser accepted as a named choice and the old gateway healed into {"a":1}, is refused as RFC 8259 and serde refuse it, so the fragments stop before the bracket and the bracket is malformed, the same in both constrained modes and under every cut (found by the review). Whether a format should heal it instead is recorded as a decision for the maintainer next to the truncated-call question. For other junk, the fragments are exactly the bytes some value could still continue and the rest is malformed, the same under every cut: {"a": nope} gives the fragment {"a": n (n may become null) and malformed ope}; {"a": 1, nope} gives {"a": 1, and malformed nope}, since after a comma an object wants a key; truex gives true and malformed x. The contract corpora take the literal-junk cases back: three for Qwen3, four for the forced function, two for the required list, and chunking invariance holds for all of them.

Stacked on #2806 (base branch feat/symphony-format-constrained); it retargets when that merges.

Changes

• crates/symphony/src/json/prefix.rs: scan, Scan, MAX_DEPTH, the machine, seven tests (every prefix of a whole value is valid and only the whole is complete; the first byte no value could continue ends the valid prefix, twenty cases, the trailing comma in an array and in an object among them; a whole value followed by more ends where the value does; numbers at the end are prefixes but not whole; incomplete strings, literals and containers are prefixes; the depth limit; the valid prefix of every cut agrees with the whole text's).
• crates/symphony/src/json/mod.rs: the module and its exports, one doc sentence.
• crates/symphony/src/json/assembler.rs: scan in place of the prefix parser; the module doc's promise paragraph rewritten.
• crates/symphony/src/formats/constrained.rs: scan in place of the prefix parser and is_complete; one doc phrase; the prefix test extended with the {"a": nope} case.
• crates/symphony/tests/contract.rs: nine literal-junk outputs across the three corpora.

Nothing outside crates/symphony/ changes; Cargo.lock is unchanged.

Test Plan

Run on the head, exit codes checked:

• cargo +nightly fmt -p smg-symphony: 0, no change.
• cargo +1.98.0 clippy -p smg-symphony --all-targets -- -D warnings: 0.
• cargo +1.98.0 test -p smg-symphony: 0, 133 unit tests, two fixture tests on their skip path and 7 contract tests. The assembler's and the constrained format's existing tests pass unchanged except the one extended; the contract's conservation, chunking invariance, well-formedness, order and token identity hold over the widened corpora under every chunking, where the leniency would have failed chunking invariance for truex, nope and falsex.
• RUSTDOCFLAGS="-D warnings" cargo +1.98.0 doc -p smg-symphony --no-deps: 0.
• git diff --stat feat/symphony-format-constrained...HEAD: the five files above.

Checklist

• cargo +nightly fmt passes
• cargo clippy --all-targets -- -D warnings passes for smg-symphony (the workspace-wide run with --all-features needs OpenCV for llm-multimodal on this machine; CI runs it)
• Documentation updated (module docs)
• (Optional) Please join us on Slack #sig-smg to discuss, review, and merge PRs

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

🟡 Nit: scan(&self.value) re-reads the whole value from byte 0 on every delta, so a forced-function call costs O(n²) in its argument length. Here scan is the only per-delta pass (unlike the assembler, which runs outline over the full text anyway). For a large forced output, say a write_file call with ~200 KB of content at ~4 bytes/token, that comes to billions of byte-steps for one request.

This isn't a regression: the old PartialJson::parse + is_complete also re-parsed the whole value each time, and allocated a Value while doing it. But Machine can already resume (it is just state + stack), so a cheap fix fits here: keep a Machine in Single, feed it only self.value[self.scanned..], and stop once it returns Reject/Done. The cost becomes linear, and the result is still a function of the bytes alone, so chunking invariance is unaffected. A follow-up PR is fine for this.

[slin1237]

Agreed, and it is this PR's next change rather than a follow-up: Single keeps the acceptor's machine and feeds it only the bytes of each delta, so a forced function costs time linear in its arguments, and the result stays a function of the bytes alone. It is not in f7a8d64; the tracker lists it as the next unit on this draft, and this thread stays open until it lands.

[hello-alexmcc]

Review on behalf of the maintainers (rule 17 checklist, rules 0, 19 to 24 and 26), at head 423fedd: one commit, feat(symphony): a strict JSON prefix acceptor keeps the fragment promise, stacked on #2806's ff44c4b.

Scope, commit, gates. json/prefix.rs (new), json/mod.rs (the module and its exports), json/assembler.rs and formats/constrained.rs (the two call sites, the prefix parser kept as the value parser), tests/contract.rs (the literal-junk cases taken back into the corpora); Cargo.lock unchanged; event.rs, input.rs and parser.rs untouched. Conventional subject, the author's Signed-off-by, Co-authored-by: Chang Su, no AI trailer. PR body per the template. Gates run here on 423fedd in a clean worktree, exit codes captured: cargo +nightly fmt --all -- --check 0; cargo +1.98.0 clippy -p smg-symphony --all-targets -- -D warnings 0; cargo +1.98.0 test -p smg-symphony 0 (122 unit tests, six contract tests over the three formats); RUSTDOCFLAGS=-D warnings cargo +1.98.0 doc -p smg-symphony --no-deps 0; codespell 2.4.1 over crates/symphony 65, the three flags below.

Reading. scan is RFC 8259 as a byte machine with a frame stack: value, key, colon, after-value, string (escape and four hex), number (the eight positions with whole on the four that may end), literal (the bytes still expected). Done returns as soon as the top-level value ends, so valid equals complete there and the bytes after a whole value are nobody's; a number at the end is a prefix and never whole, by the documented reading; a raw control character inside a string is content, by the other. Rejection can only land on a character's first byte outside a string (continuation bytes are never examined there, since a lead byte is rejected first) and never inside one, so valid is always a character boundary, which the callers slice by. The number machine re-processes the byte that ended a number in the enclosing state, which is what makes [1 2] fail at 2 and 12abc whole at 12. In the assembler, invalid_from = valid.max(emitted) keeps what is out and, since scan is a function of the bytes, the boundary is now the output's; the constrained single mode takes complete for whole_end and valid for the fragment, replacing is_complete. The two readings and MAX_DEPTH are in the module doc.

Harness. Four runs on this head. The acceptor against serde_json: 27 whole values (objects with nested escapes, Japanese and emoji, the surrogate-pair escape, arrays, strings with every escape, the literals, numbers in each form, whitespace around values, a deep array, multi-line objects), every character-boundary prefix of each, 426 prefixes: valid is the whole prefix unless the prefix holds a whole value, complete is exactly where serde's first value ends and never for a bare number at the end, and valid equals complete where there is one; 0 differences. The junk boundaries the PR states, plus others: truex 4 and whole, falsex 5, nullx 4, tru 3 and open, nope 1, {"a": nope} 7, {"a": 1, nope} 9, 12abc and 01 whole at 2 and 1, - and 1. open, +1, .5 and NaN 0, {"a": "x"}junk whole at 10, a bad escape and a bad hex digit rejected where they stand; two of my own expectations were wrong and the acceptor right ({"a" 1} and [1 2] take the whitespace before the byte that fails). The assembler on literal junk and trailing commas, whole and byte by byte: identical fragments and malformed text in every case ([1, truex] is [1, true and x] both ways, where #2792's review saw them differ). The constrained harness with literal junk and trailing commas added to both corpora: 49 outputs, 1815 replays, 0 failures, and the chunking dependence counted at #2806 (12 replays) is gone. The fixture judgement: 785 plans, 216 bitwise, 437 separators only, 132 the two marker cases, per-case verdicts identical to #2806's, 0 failures.

Checks at review time. pre-commit will fail on this head: codespell flags the deliberate literal fragments in prefix.rs, b"alse" in the literal machine (line 171) and the test strings "tru" and "fals" (lines 432 and 433), as misspellings. Not typos, but rule 25 needs every check green, so the head cannot merge as it is; a // codespell:ignore alse and // codespell:ignore tru,fals on those lines clear it, as #2786 did for the same words, and I re-approve the next head on sight since the tree change is a comment.

One note, not blocking. The PR says the change is for junk only, and that is true of the grammar; but one tolerated malformation moves with it. The ported parser accepted a trailing comma before a closing bracket as a named choice ("a trailing comma before a closing bracket is accepted" in partial.rs), and the old gateway healed {"a": 1,} into {"a":1} for the client. The acceptor refuses it, as RFC 8259 and serde do: {"a": 1,} is the fragment {"a": 1, and Malformed("}", InvalidArguments), [1, 2,] the same, in the assembler and in both constrained modes, invariant under the cuts. That is the promise kept (a trailing comma is not a valid prefix) and it agrees with whole_arguments in the adapters, which already refused it; but it is a client-visible change for a quirk models do produce, so the "nothing for well-formed arguments" paragraph should name it, and it belongs on the maintainer's decision list next to the healing question.

Approved. Rule 25 at merge time: stacked on #2806, so it retargets as that lands and merges with every check green on the head, the codespell fix included, the squash body carrying the sign-off and the Chang Su trailer.

[hello-alexmcc]

Review on behalf of the maintainers (rule 17 checklist, rules 0, 19 to 24 and 26), at head d83c3c8: the approved 423fedd plus one commit, documentation and comments only, both points of the previous review taken; re-approved on sight as offered.

The change. git diff 423fedda d83c3c82 touches crates/symphony/src/json/prefix.rs only: // codespell:ignore annotations on the literal machine's b"alse" and on the two test strings, in the form #2786's tests use (the three code lines differ from the approved head by the trailing comment alone, checked by stripping it), and the module doc naming the trailing comma before a closing bracket as the one tolerated malformation the acceptor stops where the old gateway healed it, with the healing question left to the formats' policy and recorded on the maintainer's list. No code changed, so the gates and the four harness runs of the review at 423fedd describe this tree; cargo +nightly fmt --all -- --check 0 and codespell 2.4.1 over crates/symphony 0 on this head, the two gates a comment can touch, which clears the pre-commit failure that blocked the merge.

Commit (rule 19). Conventional subject, the author's Signed-off-by, Co-authored-by: Chang Su, no AI trailer.

Approved. Rule 25 at merge time as before.

[slin1237]

Converted to draft under ground rule 1: this changes where argument fragments stop on malformed input, and no fixture on main has malformed arguments, so none fails before the change and passes after it. Cases are requested as bellwether #20. It also sits on #2806, which waits for bellwether #19 and #21.

[claude]

🟡 Nit: After this change, the contract's "arguments form a valid JSON prefix" check uses the same function the formats use to decide where fragments stop. The assembler and the constrained format both cut at scan(..).valid, and this oracle asserts scan(so_far).valid == so_far.len(), so the check passes by construction. It still proves the formats plumb scan through correctly (nothing is emitted past the boundary). It no longer proves the boundary is right. Say scan regressed and accepted a byte the grammar refuses, e.g. the ,} case flagged earlier at prefix.rs L363. The format would emit {"a": 1,} as fragments, this oracle would agree, and the client-facing contract would stay green. The old PartialJson oracle was at least an independent implementation.

serde_json is already a dependency, so a cheap independent cross-check is possible. When a call ends, if scan says its arguments are whole, serde must parse them:

Event::ToolCallEnd { index, .. } => {
    // ...existing asserts...
    if let Some((_, args)) = arguments.iter().find(|(i, _)| i == index) {
        if let Some(end) = scan(args).complete {
            assert!(
                serde_json::from_str::<serde_json::Value>(&args[..end]).is_ok(),
                "{}: whole by the acceptor but not JSON: {args:?}",
                place()
            );
        }
    }
}

(Raw control characters inside strings would need excluding, since scan takes them deliberately and serde refuses them.) The same serde cross-check over WHOLE in prefix.rs's unit tests would anchor the acceptor itself to a reference parser.

[slin1237]

Right: with scan on both sides the prefix check proves the formats stop where scan says, and no longer that scan is right. An independent check belongs here. It goes into this draft's next change, with the resumable acceptor from the other open thread: when a call ends and scan reports its arguments whole, serde_json must parse them, and when serde_json parses the joined fragments, scan must have reported them whole. This thread stays open until that lands.