Repository navigation
feat(symphony): the tagged syntax's structural tag, for Qwen 3.5 and Seed-OSS, with the value's spelling in the table - #2884
Conversation
|
Warning Review limit reachedYour organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Next included review available in 5 minutes. View limit detailsLimit details: You’ve used all 4 included reviews currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (12)
Comment |
github-actions
Bot
added
tests
|
👋 The PR description doesn't fully follow
Please update the PR description so reviewers have the context they need. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
hello-alexmcc
left a comment
hello-alexmcc
left a comment
There was a problem hiding this comment.
Rule-17 review of #2884 at f28a81a: comment, changes needed before I approve. The grammar matches what most templates write, but the recorded outputs show a table can't pin one spelling for all its checkpoints. The bot's second thread is right, and one more checkpoint breaks the spelling around the value as well.
Template oracle. I used the parse sets recorded locally from each checkpoint's template, up to 1,000 tool-call cases per set across BFCL, six glaive sets, SWE-bench's call sets and the shapes call sets: 62,761 cases in five groups. Each case's grammar comes from its request's tools, with the reasoning state paired to its output. The cases in the first column below conform to their schemas: the call names a tool of the request, uses only the schema's keys, in order, with every required one, and every value is valid against its property's schema.
| Group | Accepted | Rejected: spelling around the value | Rejected: non-string values in the template's text |
|---|---|---|---|
| qwen3.5-2b | 10,239 | 0 | 611 (True/False) |
| qwen3.6-27b | 12,726 | 0 | 0 (the template writes JSON) |
| qwen-drive-1.0-4b | 11,877 | 0 | 849 (True/False) |
| mimo-v2.5 | 236 | 11,641 (inline: <parameter=drink_id>latte</parameter>) |
849 |
| seed-oss-36b-instruct | 10,602 | 0 | 1,996 (True, and {'size': 'large', …}) |
The spellings across every group that writes this syntax, from the same recordings (17 groups, up to 4,000 cases each):
| Spelling | Booleans | Groups |
|---|---|---|
| Value on its own line | JSON (true) |
nanbeige4.2-3b, qwen-agentworld-35b-a3b, qwen3-coder-next, qwen3.6-27b, qwen3.6-35b-a3b, qwen3.8-2.4t-a95b, qwen3.8-27b, qwen3.8-flash-next |
| Value on its own line | Jinja's text (True) |
qwen-drive-1.0-4b, qwen3-coder-30b-a3b-instruct, qwen3.5-2b, qwen3.5-9b, qwen3.5-27b, qwen3.5-35b-a3b, step-3.5-flash |
| Inline | Jinja's text (True) |
mimo-v2.5 (objects as JSON) |
| Inline | Jinja's text (True) |
seed-oss-36b-instruct (objects as Python's repr) |
What to change:
- MiMo-V2.5 writes values inline but sits on the
qwen3table, which pinsOWN_LINE. As derived, the grammar would force 11,641 of its 11,877 recorded calls into a spelling its template never writes. The spelling has to come with the checkpoint, not the table. Alternatively, the tag can accept both spellings around the value, as the parser already reads both. - Booleans and null: nine of the 17 templates write Jinja's
True/False/None, and the JSON shape refuses them. Groups on the sameqwen3table differ here (Qwen 3.5 vs 3.6), so this too is per checkpoint, or the shape accepts both spellings. - Seed-OSS's objects and arrays are Python's repr, which no JSON shape takes. Any text up to the close is the exact option without a Python-literal grammar.
- The call opener inside a string value (the bot's first thread): agreed. The engine reads
<tool_call>inside a value as a terminal, so the string'sany_textshould exclude it too.
What holds:
- Nothing else is rejected. In the other four groups, every conforming call whose non-string values the template writes as JSON is accepted: 45,444 cases. MiMo's 236 accepted are exactly its calls without arguments, and all 11,641 with arguments are rejected.
- Gates at f28a81a merged on main 21d5e44, Rust 1.98.0,
-j 2: fmt, clippy and doc exit 0; tests 337 lib, 10 fixture, 7 contract and 5 README. - No other table moves. Against #2882's grammars, only
qwen3tagged and Seed-OSS change, fromNoneto a tag. Kimi K3 is still equal to its old builder except for"type": "tag". GLM and DSML are 36 of 36. - Rule 18:
CallSyntax::TaggedgainsSpelling, so the merge is the maintainer's. - Trailers: the squash body has six, and no AI trailer.
|
Round at 8dd859c, from the review and the oracle.
|
ab7f762 to
fb3dffe
Compare
8dd859c to
7d220a7
Compare
fb3dffe to
9e03a58
Compare
52b43ea to
f3ccfe1
Compare
f3ccfe1 to
5f00aa9
Compare
9e03a58 to
9acf6bf
Compare
hello-alexmcc
left a comment
hello-alexmcc
left a comment
There was a problem hiding this comment.
Rule-17 re-review of #2884 at 5f00aa9 (rebased on #2882 9acf6bf): approve. The tag now takes the values as each checkpoint's template writes them. My oracle accepts every schema-conforming recorded call, and all threads are resolved.
-
Tagged oracle: 62,761 recorded cases in five groups, paired with each prompt's reasoning state. Every schema-conforming case is accepted:
Group Accepted qwen3.5-2b 10,850 qwen3.6-27b 12,726 qwen-drive-1.0-4b 12,726 mimo-v2.5 12,726 seed-oss-36b-instruct 12,598 That includes MiMo's inline values (all 11,641 with arguments, rejected in the first round), the
True/FalseandNonethe Qwen 3.5 family writes, and Seed-OSS's Python reprs of objects and lists. Calls that break their schema are still rejected, except where Seed-OSS's text shape now takes any value. -
Numbers: every numeric argument the five templates write is JSON: no inf or nan, and the 203 exponent spellings parse. So the JSON number shape is right throughout.
-
Ling and Hy4, after the change to what ends a value: all 26,758 schema-conforming cases are still accepted, in both reasoning modes.
-
Dump vs #2882: the Qwen3 tagged and Seed-OSS grammars appear (12 lines each). Ling and Hy4 change 8 lines each, from the call's opener now ending a value. No table goes from a tag to
None. -
Gates: fmt, clippy and doc exit 0; tests 340 lib, 10 fixture, 7 contract and 5 README. GLM and DSML 36 of 36.
-
Rule 18:
CallSyntax::Tagged(Spelling)changes what aFormatcarries, so the merge is the maintainer's. Trailers: six in the squash body, and no AI trailer.
9acf6bf to
9009f6a
Compare
5f00aa9 to
57afd44
Compare
hello-alexmcc
left a comment
hello-alexmcc
left a comment
There was a problem hiding this comment.
Rule-17 re-review of #2884 at 57afd44 (rebased on #2882 9009f6a): approve. It's the 5f00aa9 I approved plus the rebase, and every table's grammar is byte-identical to that head's (180 grammars, 0 lines differ).
- Tagged oracle, with each case paired to its prompt's reasoning state: every schema-conforming recorded case is accepted. qwen3.5-2b 10,850, qwen3.6-27b 12,726, qwen-drive-1.0-4b 12,726, mimo-v2.5 12,726, seed-oss-36b-instruct 12,598.
- Ling and Hy4: all 26,758 conforming cases are still accepted.
- Gates: fmt, clippy and doc exit 0; tests 340 lib, 10 fixture, 7 contract and 5 README. GLM and DSML 36 of 36.
- Rule 18:
CallSyntax::Tagged(Spelling)changes what aFormatcarries, so the merge is the maintainer's. Trailers: six in the squash body, and no AI trailer. Threads: all resolved.
…Seed-OSS, with the value's spelling in the table Qwen 3.5 and Qwen3-Coder write a parameter's value on a line of its own, a newline after `<parameter=KEY>` and one before `</parameter>`; Seed-OSS writes it between the two tags directly. The assembler reads both, so the table said nothing about it; the tag has to spell the one the template writes, so `CallSyntax::Tagged` now carries the family's spelling (`tagged::Spelling`, `OWN_LINE` or `INLINE`), and the tables and every match on the syntax name it. The tag itself is written from the markers and the tags: the call's opener, a newline, `<function=NAME>` and a newline; one parameter per property of the schema in its order, optional unless required, the value with the spelling's newlines around it and `</parameter>` and a newline; then `</function>`, a newline and the call's close. The value follows the property's type as the templates write it (a string as it is, up to the parameter's close or a marker that ends the call; the rest as JSON, through the schema module); a property without a type, or a schema without properties, takes any text. With the reasoning open, the prefix excludes the four tags as it excludes the keyed ones. The engines' `qwen_xml_parameter` style writes Qwen's spelling of the parameters alone, so neither family uses it. Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com> Co-authored-by: Chang Su <8605658+CatherineSue@users.noreply.github.com>
…em, and the call's opener ends a value The recorded parse sets of five groups (the review's oracle) showed the tagged syntax's spellings do not follow the table: Qwen 3.5 puts a value on a line of its own and MiMo-V2.5, on the same table, writes it between the tags; nine of seventeen groups' templates write a boolean as Python's word; Seed-OSS writes an object as Python's repr. The parser reads all of it as the JSON it stands for, so the tag does too: a newline is allowed on either side of a value and required on neither; a boolean is JSON or `True`/`False`, null is `null` or `None`; and `CallSyntax::Tagged` now carries how a family writes a value that is not a string, `Json` (Qwen, `tojson`: an object's or a list's schema is pinned) or `Python` (Seed-OSS, `str`: any text up to the close). The call's opener ends a value's text in the tagged and the keyed modules alike, since a call opened before the last closed ends it. Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com> Co-authored-by: Chang Su <8605658+CatherineSue@users.noreply.github.com>
…ned value takes the optional newlines The keyed and tagged tags excluded a fixed list from a value's text: the value's close, the call's opener and close, a block's close. The engine takes inside a call exactly the terminals the table has a row for out of the arguments state, no more (Hy4's rows leave a call on its close alone; Ling's on the next call's opener too), so `CallMarkers` now names those, `call_exits`, and both tags exclude them. The tagged tag allowed a newline on either side of every value; around text that was ambiguous, the text taking the newline itself, and put an optional after an `any_text`, a shape no builder had sent an engine. The optionals now stand only around a pinned value (JSON, a word, one of an enum's), and a value written as text keeps the shape that works, text then its close. Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com> Co-authored-by: Chang Su <8605658+CatherineSue@users.noreply.github.com>
The last round sent every string to the text grammar, an `enum` of strings included, which let a constrained call write any value where the schema allows a few. Only a value written as text goes that way now; an enum keeps its values, with the newlines a pinned value allows. Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com> Co-authored-by: Chang Su <8605658+CatherineSue@users.noreply.github.com>
57afd44 to
25893ba
Compare
hello-alexmcc
left a comment
hello-alexmcc
left a comment
There was a problem hiding this comment.
Rule-17 re-review of #2884 at 25893ba (on main 227bd5b): approve. The rebase onto the merged #2882 changes nothing in crates/symphony relative to 57afd44, which I approved after the full battery. git diff 57afd443 25893bab -- crates/symphony is empty, and main's tree there equals #2882's 9009f6a. Threads are all resolved, and the squash body has six trailers. Rule 18: the merge is the maintainer's.
2 participants
What
Stacked on #2882 (base
feat/symphony-grammar-keyed): the tagged syntax, insidecrates/symphony/only.CallSyntax::Taggedcarries the family's spelling (tagged::Spelling) of a value that is not a string:Json(Qwen 3.5 and later, Qwen3-Coder: an object or a list astojson, a boolean or null as Python's word) orPython(Seed-OSS: every value asstr, an object or a list as its repr). The assembler reads all of it as the JSON it stands for, as before; the tables name theirs (qwen3(CallSyntax::Tagged(Spelling::Json)),seed_oss()withPython), and every match on the syntax names it (engine.rs, the three test suites).tagged::assembler::TAGSbecomes public for the prefix's excludes. Rule 18: this changes the dataFormatcarries, so the merge is the maintainer's.grammar/tagged.rs: a call written from the markers and the tags:{call opener}\n<function=NAME>\n, one parameter per property of the schema in order (optional unless required):<parameter=KEY>, the value (a newline allowed on either side of a pinned value and required on neither: Qwen 3.5 puts the value on its own line, MiMo-V2.5 on the same table writes it inline, and the assembler takes one newline away on each side when it is there; a value written as text takes those newlines into the text),</parameter>\n; then</function>\n{call close}. The value follows the property's type as the models write it and the parser reads it: a string as any text up to</parameter>or a terminal the table has a row for out of the arguments state (CallMarkers::call_exits: the call's close, its opener where a new call ends the one open, a block's close where it ends a call; the keyed tag now reads the same);enumas its values; a number as JSON; a boolean as JSON orTrue/False; null asnullorNone; an object or a list as the property's schema where the family writes JSON and as any text where it writes Python's repr. A property without a type, or a schema without properties, takes any text. Both tables have no block; a tagged table with one would take the block's ways in throughblock_of.<function=,<parameter=,</parameter>,</function>) beside the terminals, as it excludes the keyed tags and DSML's parameter tags.Spellings, from the templates
'<tool_call>\n<function=' + name + '>\n', per argument'<parameter=' + k + '>\n', the value (tojsonfor a mapping or a list, elsestr:True,None),'\n</parameter>\n', then'</function>\n</tool_call>'<tool_call>\n<function=get_weather>\n<parameter=city>\nParis\n</parameter>\n</function>\n</tool_call>toolcall_begin_token + "\n<function=" + name + ">\n", per argument"<parameter=" + k + ">", the value (as it is if a string, elsestr: an object as its repr),"</parameter>\n", then"</function>\n" + toolcall_end_token<seed:tool_call>\n<function=get_weather>\n<parameter=city>Paris</parameter>\n</function>\n</seed:tool_call>No old builder is an oracle (the gateway takes the JSON-schema path for both families). The reviewer's template oracle, the recorded parse sets of five groups (62,761 cases), found the first head too narrow: MiMo-V2.5 writes values inline on the qwen3 table, nine of seventeen tagged groups write booleans as Python's words, Seed-OSS writes objects as Python's repr; the second head takes all of it, as the parser does. The tests pin the derived payloads literally: Qwen's whole payload for two tools (string, integer, boolean, object, a string enum, a tool without properties), Seed-OSS's parts (an object as text), null's two words, and the prefix with the reasoning open.
Gates
By exit code (
.claude/symphony/gates.sh, basefeat/symphony-grammar-keyed): fmt, clippy-D warnings, test (340 lib, 10 fixture, 7 contract, 5 README), doc, width, codespell, all 0.Not in this PR
None.Merge
After #2882, rebased onto main. Squash under rule 25 and rule 18 (
CallSyntax::Taggedgains data; the maintainer merges). Subject: the PR title plus(#<n>). Body, with the authors of the old Kimi K3 parser whose property walk the tagged syntax builds on throughgrammar/schema.rs(rule 27, as on #2881 and #2882):