Repository navigation
Conversation
DeepSeek V4 and V4.1 write a call as <｜DSML｜ invoke name="NAME"> and one <｜DSML｜ parameter name="KEY" string="true|false">VALUE</｜DSML｜ parameter> per argument, inside a <｜DSML｜ calls> block that holds one invoke or several. tagged::dsml is the assembler for one invoke: the engine enters it at the invoke tag's opening, so it starts inside the function's name; the string attribute types each value, a string streamed as it arrives and anything else written whole at its close through the value module's inference; no template newline is taken from a value; the invoke's closing tag, read by the engine, is ToolCallEnd's source. It shares the value module and the escaping with the Qwen tagged assembler and will share one core when a third dialect shows the seams. The engine gains Emits::Wrapper, the template's wrapping between a block's calls (whitespace dropped, anything else malformed, run by run), and CallSyntax::Dsml; a call syntax may take the terminal that closed its region as the call's end, in which case the engine does not drop it. Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com> Co-authored-by: Chang Su <8605658+CatherineSue@users.noreply.github.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
github-actions
Bot
added
tests
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| /// A tag between parameters: the parameter tag opens a key, the closing tag has no place. | ||
| fn tag_between(&mut self, tag: usize, out: &mut Events) { | ||
| let bytes = TAGS[tag]; | ||
| if tag == PARAMETER_OPEN { |
There was a problem hiding this comment.
🔴 Important: tag_between opens a parameter even when no call has started, so the assembler can emit ToolCallArguments with no ToolCallStart before it. That breaks the Event contract, and the next call ends up with invalid JSON arguments.
There are two ways to reach Stage::Between while function is still None:
- the empty-name branch in
text(lines 258-260) - the
_arm intag(lines 218-221), when a tag cuts the invoke's name or tail short. One way this happens is a missing>after the invoke tag, with a parameter tag right after it.
From there, the next <｜DSML｜ parameter name="… goes through this branch into open_value/value_text, which pushes {"x": " with index: self.index. Example input:
<｜DSML｜ invoke name="">
<｜DSML｜ parameter name="x" string="true">v</｜DSML｜ parameter>
</｜DSML｜ invoke>
<｜DSML｜ invoke name="g">
<｜DSML｜ parameter name="y" string="false">1</｜DSML｜ parameter>
</｜DSML｜ invoke>
Tracing it:
- The first invoke emits
ToolCallArguments{index: 0}fragments{"x": ",v,". - On close,
started()is false, soEngine::close_callnever incrementscalls. - The second invoke reuses index 0:
ToolCallStart{0, "g"}, then{"y": 1}and}.
Concatenated for index 0, that's {"x": "v"{"y": 1}}. event.rs promises "Concatenated fragments for one index are always a valid JSON prefix", and adapt/chat.rs says arguments for a call that never started "cannot occur". The streaming chat adapter sends the orphan fragments to the client as tool_calls[0].function.arguments deltas, so the client's arguments for g come out corrupt.
The Qwen assembler guards against this with a separate Stage::Opening and PARAMETER_OPEN if between (assembler.rs:289-327). The smallest fix that does the same here is to accept the parameter tag only once a call has started:
| if tag == PARAMETER_OPEN { | |
| if tag == PARAMETER_OPEN && self.started() { |
With that change, the stray tag falls through to the Malformed { TAG_OUT_OF_PLACE } arm. It's worth adding a test for an empty name followed by a parameter, and one for a parameter after a cut-short invoke tag.
| .transition("reasoning", "think_close", "content") | ||
| .transition("content", "calls_open", "calls") | ||
| .transition("calls", "invoke_open", "invoke") | ||
| .transition("invoke", "invoke_close", "calls") |
There was a problem hiding this comment.
🟡 Nit: </｜DSML｜ invoke> is the only way out of invoke, which doesn't match what dsml::Assembler::close says it handles. Its doc (tagged/dsml.rs:34-35) says close covers "the invoke's closing tag, or the block's end before it". But no row lets </｜DSML｜ calls> close an open invoke.
When a model leaves out </｜DSML｜ invoke>, </｜DSML｜ calls> reaches the assembler as text (Malformed). The engine then stays in invoke for the rest of the output, so any prose after the block becomes Malformed { "text between a call's tags" } instead of Content. At end of stream, finish leaves the call open with an unclosed object ({"x": "v").
The Qwen table handles the matching case, where </tool_call> arrives before </function>, through close. One extra row would do the same here, and the engine already supports it: close_call(ByMarker, "</｜DSML｜ calls>") → close takes the bytes as ToolCallEnd's source:
.transition("invoke", "invoke_close", "calls")
.transition("invoke", "calls_close", "content")The fixtures don't contain this case (0 differences), so it isn't blocking. If the row is left out on purpose, the line in close's doc should be removed.
| let mut after = self.carried.split_off(tag_end); | ||
| let close = after.pop(); | ||
| if !after.is_empty() { | ||
| self.drop_carried(out); |
There was a problem hiding this comment.
🟡 Nit: When a parameter tag has an unexpected tail (for example <｜DSML｜ parameter name="x" kind="y">), drop_carried emits everything carried as Dropped { Wrapper }. That isn't just whitespace: it includes the <｜DSML｜ parameter name=" tag bytes and the key x". drop_carried's doc describes it as "Whitespace carried before text that is reported", and start_call handles the same tail case by keeping the tag's bytes as the start's source.
Byte conservation still holds, but source attribution is wrong: a consumer reading Dropped { Wrapper } would treat the parameter's name as template wrapping. One option is to push the head (whitespace + tag + key) as the source of an empty-json ToolCallArguments before the Malformed tail. Another is to fold it into the member's opening fragment and report the tail on its own.
| Self::Qwen3Tagged => KNOWN_TAGGED_DIFFERENCES, | ||
| // DSML's code-fence probe holds Qwen's syntax, which the DSML table never reads as a | ||
| // call: the fence is content, as the reference says. | ||
| Self::DeepSeekV4_1 => &KNOWN_TAGGED_DIFFERENCES[..1], |
There was a problem hiding this comment.
🟡 Nit: &KNOWN_TAGGED_DIFFERENCES[..1] picks entries by position. It only works because reasoning-with-marker-text happens to come first and content-with-marker-in-code-fence second. If someone inserts or reorders entries, DSML silently gets a different list. The harness would then fail with a puzzling "listed … but matching the reference now", or worse, an entry that should apply gets dropped.
Two more readable options:
- a named
const KNOWN_DSML_DIFFERENCES: &[KnownDifference] = &[/* reasoning-with-marker-text */] - filtering by
id
That would also make the comment above match the code more directly: it explains why the code-fence entry is left out. The PR description says the opposite ("with the code-fence probe listed"), which is worth fixing too.
| } | ||
|
|
||
| #[test] | ||
| fn a_calls_block_in_the_prompt_is_not_entered_and_thinking_off_starts_in_content() { |
There was a problem hiding this comment.
🟡 Nit: The test name says "a calls block in the prompt is not entered", but the prompt is <think>\n\n</think>\n\n, which has no <｜DSML｜ calls>. Only the thinking-off half is tested.
Either rename it to something like thinking_off_starts_in_content, or add a prompt that actually holds a calls block, e.g. a prior turn's <｜DSML｜ calls>…</｜DSML｜ calls> followed by <think>. That second case is worth having: a prompt that leaves the replay in calls (an Emits::Wrapper state) is handled by seed's enter, which nothing currently tests.
…ases The design's section 4 example as a table: six terminals, four states (content, reasoning, the calls block's wrapping, one invoke's arguments) and six rows; the prompt opens the thought, and the recorded outputs close it before the calls block. deepseek-v4.1-flash joins the parity table: over bellwether's scale-run fixtures of 2026-10-06, 2,436 cases, 2,433 bitwise, 2 separators only, 1 listed, 0 differences. Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com> Co-authored-by: Chang Su <8605658+CatherineSue@users.noreply.github.com>
ac8be82 to
8dd35d0
Compare
This branch has not been deployed
1 participant
Description
Stacked on #2839 (
feat/symphony-format-table); the base is that branch, so the diff is this step alone: two commits, 3d17de7 and ac8be82. The first family after Qwen through the table, and the design's own section 4 example.Problem
DeepSeek V4 and V4.1 write a call as DSML:
<｜DSML｜ calls>around the turn's calls,<｜DSML｜ invoke name="NAME">around each call, and<｜DSML｜ parameter name="KEY" string="true|false">VALUE</｜DSML｜ parameter>per argument, thestringattribute saying whether the value is text or JSON. Neither the JSON assembler nor the Qwen tagged assembler reads it: the tags are spelled differently, the name ends at a quote, the type comes from the attribute rather than the request's tools, there is no template newline around a value, and one block holds several calls. bellwether's scale run records 2,436 parse cases fordeepseek-ai/DeepSeek-V4.1-Flash, 442 of them with several invokes in one block.Solution
tagged::dsml::Assembler, the second tagged assembler, for one invoke. The engine enters it at the invoke tag's opening (<｜DSML｜ invoke name="is the terminal), so it starts inside the function's name; the name runs to"and the tag to>.string="true"streams as it arrives, like a declared string in the Qwen assembler;string="false"is written whole at the value's close through the value module's inference (null,true,[1, 2],{"a": 1}as written; text that is not JSON is a string holding it). No byte is taken from a value: the template writes it directly between the tags. Inside a value only</｜DSML｜ parameter>is a tag. Two endings as in the Qwen assembler:close(the invoke's closing tag, which the engine reads and hands over asToolCallEnd's source; inside a value it closes an open string and the object and reports what was held) andfinish(a cut stream closes nothing). Text where the syntax has tags isMalformedrun by run with the whitespace before itDropped { Wrapper }; a tag with text after its name reports that tail; a parameter tag without the attribute is read as a string and its tail reported. It sharesvalue::jsonand the two escaping helpers with the Qwen assembler; the two get one core when a third dialect (GLM, MiniMax, Hy4) shows where the seams are.Emits::Wrapper(the template's wrapping between a block's calls: whitespace dropped, anything else malformed, classified run by run as the surplus after a Qwen call already is) andCallSyntax::Dsml. A call syntax may now take the terminal that closed its region as the call's end (Call::endsays whether it did), in which case the engine does not drop it; the JSON and Qwen tagged syntaxes are unchanged.deepseek_v4_1(), the table:<think>/</think>,<｜DSML｜ calls>/</｜DSML｜ calls>, the invoke opening and closing; states content, reasoning,calls(wrapper),invoke(arguments); six rows, among themcalls + invoke_open = invokeandinvoke + invoke_close = calls, so a block with several invokes gives several calls through the same two rows, each with its own index. The template opens the thought in the prompt (<think>, no newline) and an empty thought writes</think>at once, which the engine's prompt replay handles as for Qwen 3.5. No row leaves reasoning on<｜DSML｜ calls>: the recorded outputs close the thought before a call, and a call inside the thought is for the fixtures to raise.deepseek-v4.1-flashjoins the parity table, with the code-fence probe listed (the fence holds Qwen's syntax, which this table never reads as a call) and no corpus allowance.Changes
crates/symphony/src/tagged/dsml.rs(new, 522 lines; its tests are the table's, below),tagged/mod.rs,tagged/assembler.rs(the two escaping helpers becomepub(crate)).crates/symphony/src/engine.rs:Emits::Wrapper,Call::Dsml,Call::endreturning whether it took the terminal,wrappingwith its reason;format.rs:CallSyntax::Dsml,Emits::Wrapper.crates/symphony/src/formats/deepseek_v4_1.rs(new; five tests: a recorded output with two invokes, a string and a JSON value, byte for byte; a string streaming and a JSON value written at its close; every chunking; a cut stream; thinking off),formats/mod.rs,lib.rs.crates/symphony/tests/bellwether_parse_fixtures.rs:Family::DeepSeekV4_1and the slug.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(187 lib tests, 3 fixture tests skipping withoutBELLWETHER_FIXTURES, 7 contract tests),RUSTDOCFLAGS="-D warnings" cargo doc -p smg-symphony --no-deps, codespell with the repository's list: all exit 0 at ac8be82.deepseek-v4.1-flashthroughevery_recorded_qwen_model_parses_like_its_reference: 2,436 cases, 2,433 bitwise, 2 separators only, 1 listed, 0 allowed, 0 differences, every replay conserving its bytes and agreeing with every other chunking (78 s). The 36 Qwen slugs are as on feat(symphony): formats are data, and one engine runs them #2839.\nbetween every pair of tags, no template newline around a value (one value holds its own), all 4,227string="false"values JSON, 5,662string="true", 2,420 of 2,436 outputs starting with</think>and 10 with thinking off.Checklist
cargo +nightly fmtpassescargo clippy --all-targets --all-features -- -D warningspasses