feat(symphony): keyed arguments, the third tagged assembler, with Hy4, Ling and IQuest

[slin1237]

Description

Stacked on #2841 (feat/symphony-recorded-families); the base is that branch, so the diff is this step alone. The third tagged assembler, and the three recorded families that write its syntax; GLM writes it too and joins when bellwether records it.

Problem

GLM, Hy4, Ling and IQuest write a call as its name followed by <arg_key>K</arg_key><arg_value>V</arg_value> pairs, each family under its own call markers and tag spellings (Hy4 suffixes every tag :opensource and wraps the turn's calls in <tool_calls:opensource>; Ling writes a newline after the name and after </arg_key>; IQuest writes none). Neither the Qwen tagged assembler (a <function= tag, a > ending the name, template newlines around values) nor DSML's (a quoted name attribute, a string attribute typing values) reads it. bellwether's scale run holds 2,436 parse cases for each of the three.

Solution

• tagged::keyed::Assembler, for one call after its opening marker: the name is the text before the first <arg_key> (or before the call's end, for a call without arguments), less the whitespace around it, which goes into the next event's source; the engine's closing marker is the call's end and ToolCallEnd's bytes. A value is typed by the request's tools as in the Qwen assembler: a declared string streams as it arrives, a nullable one once null is ruled out, everything else is written whole at </arg_value> through the value module (the templates write non-strings with tojson). Nothing is taken from a value. Inside a value only </arg_value> is a tag. The four tags are a [Tags] the format passes (PLAIN, HY4). The three tagged assemblers now share the value module and the escaping; the seams for one core are visible and named in the module doc.
• The engine counts a call that is named only at its end (a keyed call with no arguments): the end's events are read for ToolCallStart before the count. CallSyntax::Keyed(Tags, Declared).
• Three tables: hy4() (think, a calls wrapper state, call arguments), ling() and iquest() (think and call); each with two tests (a recorded shape with two calls, typed values and the thought; every chunking).
• Three rows in the parity table, with the prompt shapes the fixtures show: Ling and IQuest open the thought in the prompt and honour thinking off; Hy4 opens it whatever the request says. The prompt tail is now spelled in each family's own markers (Family::think_markers), which the Hy4 and Seed-OSS rows needed.

Changes

• crates/symphony/src/tagged/keyed.rs (new), tagged/mod.rs; format.rs (CallSyntax::Keyed), engine.rs (Call::Keyed, the count at the end).
• crates/symphony/src/formats/{hy4,ling,iquest}.rs (new, two tests each), formats/mod.rs, lib.rs.
• crates/symphony/tests/bellwether_parse_fixtures.rs: three rows, Family::{Hy4, Ling, IQuest}, think_markers, the tail as a String.

Test Plan

• cargo +nightly fmt -p smg-symphony --check, cargo clippy -p smg-symphony --all-targets --all-features -- -D warnings, cargo test -p smg-symphony (203 lib tests, 3 fixture tests skipping without BELLWETHER_FIXTURES, 7 contract tests), RUSTDOCFLAGS="-D warnings" cargo doc -p smg-symphony --no-deps, codespell with the repository's list: all exit 0 at 4ca17b1.

• Preview against bellwether's scale-run fixtures of 2026-10-06 (local, not CI), the three slugs through every_recorded_qwen_model_parses_like_its_reference: 0 differences, every replay conserving its bytes and agreeing with every other chunking.

  Slug	Cases	Bitwise	Separators only	Listed	Allowed
  hy4-preview	2436	2321	0	0	115
  ling-3.0-flash	2436	2317	2	2	115
  iquest-q1	2436	2320	0	0	115

  "Allowed" is the corpus class bellwether Extract multimodal module into standalone llm-multimodal crate #56 now refuses at record time. Two things the run established: Hy4's template opens the thought even with enable_thinking: false (the first run's 11 differences were those cases), and a keyed call with no arguments was not counted before the engine fix (27 and 28 cases of Ling and IQuest with finish: stop).

• The 48 slugs of feat(symphony): formats are data, and one engine runs them #2839, feat(symphony): DeepSeek's DSML through the table, the second tagged assembler #2840 and feat(symphony): eleven more recorded families through the tables, Seed-OSS's among them #2841 are unchanged.

Checklist

• cargo +nightly fmt passes
• cargo clippy --all-targets --all-features -- -D warnings passes
• Documentation updated (module docs)
• (Optional) Please join us on Slack #sig-smg to discuss, review, and merge PRs

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Team
• Run ID: b6860804-3b1d-43b2-9462-b6bfabdca914

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

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

---

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

[claude]

🔴 Important: A call block with no name can emit ToolCallArguments without ever emitting ToolCallStart, and the next real call then reuses the same index.

name_call sets Stage::Between even when the name is empty (line 346), so after that the stage no longer tells you the call never started. Here KEY_OPEN opens a key whether or not self.started(). Trace through <tool_call><arg_key>a</arg_key><arg_value>1</arg_value><arg_key>b</arg_key><arg_value>2</arg_value></tool_call>:

1. The first <arg_key> ends an empty name and is reported as Malformed. The stage is now Between, with nothing started.
2. a, </arg_key>, <arg_value>, 1 and </arg_value> all come back as Malformed (they arrive in Between).
3. The second <arg_key> reaches this branch and moves to Stage::Key. b is read as the key, the value 2 is held as Whole, and close_value pushes ToolCallArguments { index: 0, json: "{\"b\": 2}" }.
4. close sees !started(), so it reports the terminal and pushes no ToolCallEnd. close_call doesn't count the call either, so the next <tool_call> also gets index 0.

In streaming chat, adapt/chat.rs:77 sends that fragment as tool_calls[{index: 0, function: {arguments: "{\"b\": 2}"}}] before any id or name. The client then appends the real index-0 call's arguments to it, which gives invalid JSON ({"b": 2}{...}). In the non-streaming path, the orphan fragment goes into content instead (chat.rs:172).

The Qwen assembler avoids this by keeping Opening separate from Between (PARAMETER_OPEN if between). The smallest fix is to gate this branch on self.started():

if tag == KEY_OPEN && self.started() {

You could also keep a separate stage for a block with no name.

[claude]

🟡 Nit: When the call ends inside a streamed string, the bytes the scanner was holding end up silently in the closing quote's source. The module doc says they are reported (lines 33–35: "inside a value it closes an open string and the object and reports what was held").

Before this match, close() appends scanner.held() to carried. For Streaming { opened: true }, all of carried then becomes the source of the " fragment. Take the value a < followed by </tool_call>: the scanner holds < as a possible <arg_key>, so the client receives "a ", and the < exists only in the source of the " fragment. Nothing is flagged.

The Qwen assembler reports held bytes in the same situation as Malformed(CLOSED_EARLY) before it closes the string. Two ways to fix it here:

• Report carried first (for example with report(TAG_CUT_SHORT, …)) and then push " with an empty source.
• Or stream the held bytes as value text before closing.

[claude]

🟡 Nit: This assembler has no unit tests. The Qwen tagged::Assembler has around 20, and DSML has its own.

The only CI coverage is the three format tests in hy4.rs, ling.rs and iquest.rs. Each runs one well-formed output with two calls, plus a single cut at every position. The bellwether fixture test skips without BELLWETHER_FIXTURES.

None of these paths run in CI:

• EMPTY_KEY
• TAG_CUT_SHORT (a tag inside a key, a key or value cut short by the call end)
• WITHOUT_A_NAME
• the NullableString path: Undecided, then split_off, then Streaming { opened: false }
• an empty declared string (Streaming { opened: false } at </arg_value>)
• finish() against close()
• text between tags (reported, with the Dropped whitespace before it)
• escaping of keys and values

A small #[cfg(test)] module in the style of assembler.rs would have caught the nameless-call bug above. It would also pin the byte accounting for each malformed shape.

[claude]

🟡 Nit: Some leftover parameters and wrappers:

• text() takes declared only to discard it (let _ = declared;).
• open_value does the same with out (line 364, let _ = out;).
• leftover (line 487) only forwards to report.

Dropping the unused parameters, and folding leftover into report, would make the signatures match what each function actually uses. Tags could also derive Copy, since it is four &'static str. That would let Call::new in engine.rs drop the tags.clone().

[claude]

🟡 Nit: This push removes Copy from the public CallSyntax enum. The only reason is that keyed::Tags doesn't derive it. Because of that, open_call in engine.rs:309 changes from .copied() to .cloned().

Tags is just four &'static str, so it can derive Copy. That keeps CallSyntax Copy and puts .copied() back:

// keyed.rs
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub struct Tags { … }

Suggested change

	#[derive(Clone, Debug, PartialEq, Eq)]
	#[derive(Clone, Copy, Debug, PartialEq, Eq)]

[claude]

🔴 Important: When the stream is cut, finish() never pushes ToolCallEnd for a call that already started. That breaks the parser contract this PR's base now enforces.

The rebase onto #2841 (fd60518) changed DSML's finish() to end a started call, documented as "a call that started still ends, with no bytes of its own, as every assembler ends one". The Qwen tagged::Assembler::finish (assembler.rs:210) already does the same. The keyed assembler is now the only tagged one that leaves the call open.

tests/contract.rs:336 asserts started.len() == ended.len() ("a call without an end") under every chunking. Take <tool_call>f<arg_key>a</arg_key><arg_value>1 with finish: length, which happens with any max_tokens cut in the middle of a call. Hy4, Ling and IQuest emit ToolCallStart { index: 0 } and Finish { tool_calls: 1 }, but never a ToolCallEnd. CI doesn't catch it because none of the three keyed formats is a Subject in contract.rs.

What clients see: for the same cut, the Responses adapter marks the call item completed for Qwen and DSML, but leaves it at the response's status for these three families. It also emits function_call_arguments.done only at the terminal event, not when the call ends.

Fix, matching the other two:

        if !self.carried.is_empty() {
            out.push(Event::Malformed { … });
        }
        if self.started() {
            out.push(Event::ToolCallEnd {
                index: self.index,
                source: Text::default(),
            });
        }

Also update the doc on finish (lines 222–224), and the module doc at lines 32–35, which say "nothing is closed". It would also help to add one keyed subject to contract.rs (for example ling() with a cut output, a call with no arguments and a nameless block). That puts this invariant, and the nameless-block index issue flagged earlier at line 295, under the contract checks.

[claude]

🟡 Nit: This nameless block has only one key/value pair, so it never reaches the nameless-call bug flagged earlier at keyed.rs:302. That bug is still open in this push (tag_between still opens a key without checking self.started()).

With one pair, everything after the first <arg_key> arrives in Between and is reported as Malformed, so the contract sees nothing wrong. A second pair is what triggers the bug. The second <arg_key> goes into Stage::Key, and close_value then pushes ToolCallArguments { index: 0, json: "{\"b\": 2" } with no ToolCallStart. The contract's well-formedness check ("arguments and an end only for a call that has started") would then fail on this subject.

Adding a second pair turns this case into a regression test for that fix:

Suggested change

	"</think><iquest_tool_call><arg_key>city</arg_key><arg_value>Paris</arg_value>\
	</iquest_tool_call>",
	"</think><iquest_tool_call><arg_key>city</arg_key><arg_value>Paris</arg_value>\
	<arg_key>days</arg_key><arg_value>3</arg_value></iquest_tool_call>",

With this case, the test will fail until keyed.rs:302 is gated (if tag == KEY_OPEN && self.started()).