fix(parse): accept DeepSeek DSML markers on the tool_call tag family - #21
Conversation
…call tags The tagged grammar now matches tool call tags that carry a `DeepSeek` DSML marker (e.g., `<|DSML|tool_call>`) or use the fullwidth pipe `|` that chat templates actually emit, both of which were previously missed. This prevents a `deepseek` turn that uses the tag form from being parsed as prose and silently dropping the call. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add test coverage for DeepSeek's DSML marker on the `tool_call` tag, including fullwidth bar variants and the ASCII spelling, to ensure these formats are correctly parsed as tool calls rather than prose. Also verify that the plural `<|DSML|tool_calls>` wrapper element is not mistakenly treated as a call marker, which would cause the inner call to be lost. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Reformat the `TAG_RE` regex construction and the `dsml_marker_variants_on_the_tag_parse` test assertion to use multi-line formatting, improving code readability without changing any behavior. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Tiny Sweeper reviewTiny Sweeper reviewed this change across 6 lane(s) and found 2 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below. State: Changes requested Review snapshot
Completeness: Complete What changedThe review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below. FeaturesNone identified with supported citations. TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. Findings
Before merge
How this fits togetherflowchart LR
n0["a_bare_trailing_opener_is_dropped_not_shown<br/>changed"]:::changed
n1["parse"]:::impacted
n2["probe_decided"]:::impacted
n3["ParseOptions"]:::impacted
n4["next_opener"]:::impacted
n5["swallow_extra_closers"]:::impacted
n6["len"]:::impacted
n0 -->|calls| n1
n2 -->|uses| n3
n2 -->|calls| n4
n2 -->|calls| n5
n2 -->|calls| n6
n4 -->|calls| n6
n5 -->|calls| n6
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches📝 Generate docstrings
Comment |
Picks up tinyhumansai/tinytools#21: the tagged-JSON grammar now accepts DeepSeek's DSML marker and fullwidth pipes on the `tool_call` tag family, the way `invoke_xml` already did for `<invoke>`. Without it a `<|DSML|tool_call>` block parsed as narrative and the call was dropped silently — seen live from `deepseek-v4-flash` on the code dialect. Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
Requesting changes: 2 lane(s) blocking, worst finding is high.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0134 · 118,566 in / 20,588 out · 17,478 cached (15%) · ladder/vectors, gpt-5.6-luna, deepseek-v4-flash · 359 embedded
critique: $0.0090 · 48,487 in / 3,018 out · 2,118 cached (4%) · gpt-5.6-luna, deepseek-v4-flash
security: $0.0033 · 29,302 in / 921 out · 0 cached (0%) · gpt-5.6-luna
tests: $0.0007 · 32,048 in / 8,596 out · 14,336 cached (45%) · deepseek-v4-flash
description: $0.0003 · 5,353 in / 4,991 out · 1,024 cached (19%) · deepseek-v4-flash
| /// attribute list. `<tool_calls>` (plural, a JSON key, and the DSML wrapper | ||
| /// element) and `<tool_callable>` do not match: the name must end at a pipe, | ||
| /// slash, whitespace, or `>`. | ||
| static TAG_RE: LazyLock<Option<Regex>> = LazyLock::new(|| { |
There was a problem hiding this comment.
Update openers and probe for DSML and fullwidth variants
The new regex accepts <|DSML|tool_call>, <|DSML|tool_call>, and fullwidth bar variants, but openers() (used to hold partial markers during streaming) and the probe() pending_opener call only list ASCII-bar forms like <|tool_call. Without these entries, a DSML-prefixed or fullwidth-bar tag split across chunks is not held, so the incomplete tag fragment escapes as narrative and the call may be silently dropped. Add the corresponding strings (e.g., <|DSML|tool_call, <|tool_call, <|DSML|tool_call) to both lists.
Additional critique observation
Update opener detection for DSML tag markers
[RULE] incomplete-parser-integration
Adding DSML spellings only to TAG_RE is insufficient if probe/openers first locate candidate tags using a fixed literal list. The new inputs such as <|DSML|tool_call> and <|DSML|tool_call> can therefore remain prose and never reach this regex, contradicting the added tests and the stated behavior. Add the DSML opener variants (including doubled bars and any supported whitespace forms) to the grammar's candidate detection, or make probing use the same regex when searching for the next marker.
[RULE] incomplete-streaming-support ·
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b47ccd1a58
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// slash, whitespace, or `>`. | ||
| static TAG_RE: LazyLock<Option<Regex>> = LazyLock::new(|| { | ||
| Regex::new( | ||
| r"(?i)<[|\u{ff5c}/\s]*(?:DSML[|\u{ff5c}/\s]*)?tool[_-]?call(?:[|\u{ff5c}/\s]*|\s+[^>]*)>", |
There was a problem hiding this comment.
Classify fullwidth-pipe closing markers
When the newly accepted fullwidth-pipe spelling puts the slash after the pipe (<|/tool_call|>), TAG_RE matches it but is_closing_marker still strips only ASCII |. In a doubled block followed by narrative, swallow_extra_closers therefore treats the second closer as a new opener and leaves <|/tool_call|> in the user-visible text; extend closer classification to handle U+FF5C as well.
Useful? React with 👍 / 👎.
| //! * a `DeepSeek` DSML marker on the tag itself, | ||
| //! `<|DSML|tool_call>…</|DSML|tool_call>` — the same marker |
There was a problem hiding this comment.
Update the grammar table for DSML tagged calls
This adds a new TaggedJson spelling, but crates/tinytools-agent/README.md still omits DSML from the TaggedJson row and mentions it only under InvokeXml. Update the consumer-facing grammar table in this commit so it remains aligned with the parser and module documentation.
AGENTS.md reference: AGENTS.md:L204-L205
Useful? React with 👍 / 👎.
Same base main already pins (3c9ba00, the session-todo-list work this tree depends on), plus the tinytools bump from tinyhumansai/tinyagents#196 → tinyhumansai/tinytools#21: a `<|DSML|tool_call>` block parsed as narrative and the call was dropped silently. `deepseek-v4-flash` emits that form on the code dialect, which is the path this branch's integration work now leans on. Repoint to the merge commit once #196 lands. Co-authored-by: Medulla <medulla@tinyhumans.ai>
Picks up tinyhumansai/tinytools#21: the tagged-JSON grammar now accepts DeepSeek's DSML marker and fullwidth pipes on the `tool_call` tag family, the way `invoke_xml` already did for `<invoke>`. Without it a `<|DSML|tool_call>` block parsed as narrative and the call was dropped silently — seen live from `deepseek-v4-flash` on the code dialect. Branched from the commit `openhuman` main pins (3c9ba00), not tinyagents `main`, so the bump applies on top of the session-todo-list work that openhuman already depends on. Co-authored-by: Medulla <medulla@tinyhumans.ai>
What
invoke_xmlaccepts a DSML prefix (<|DSML|invoke name="…">) and has a test corpus for it. The tagged-JSON grammar did not: its marker regex allowed the ASCII|only, with no fullwidth|and noDSMLmarker. So the tag form DeepSeek also emits —— matched nothing, the block parsed as narrative, and the call was dropped with no error anywhere.
Before this change, that input parses to 0 calls.
How it showed up
Live, from
deepseek-v4-flashdriving the code (Python) dialect in OpenHuman: the turn ended with the model's lead-in ("I'll look up your Calendly event types now.") and the tool never ran. The model had emitted a DSMLtool_callblock that the parser did not recognise as a call.The change
TAG_REnow accepts the fullwidth|those chat templates actually emit and an optionalDSMLmarker — the same prefixinvoke_xmlalready honours.The plural
<|DSML|tool_calls>wrapper deliberately still does not match: the name must end at a pipe, slash, whitespace or>. A negative test pins that, because treating the wrapper as a marker would open a block on it and close it on the first inner tag, losing the call inside.Tests
Three new tests in
parse/test/tagged.rs: the plain fullwidth form, the doubled-bar and ASCII variants, and the plural-wrapper negative case.cargo test -p tinytools-agent— 310 passed, 0 failed.cargo fmt --checkandcargo clippy --all-targetsclean.Co-authored-by: Medulla medulla@tinyhumans.ai