Skip to content

fix(parsers): tools are leaking in the reasoning content (deepseek) - #144

Merged
indrajit96 merged 6 commits into
ai-dynamo:mainfrom
slivanovich:dsml-problem
Jul 30, 2026
Merged

indrajit96 merged 6 commits into
ai-dynamo:mainfrom
slivanovich:dsml-problem

Conversation

@slivanovich

Copy link
Copy Markdown
Contributor

Overview:

Improves DeepSeek V4 reasoning parsing when a model starts emitting DSML tool-call markup before closing an open <think> block. The parser now treats the DeepSeek DSML tool section marker as a reasoning exit point, preventing tool-call markup from being counted or streamed as reasoning content.

Details:

Rust - parsers/v1/src/reasoning/mod.rs

  • Added DEEPSEEK_TOOL_SECTION_BEGIN for the DeepSeek DSML tool-call marker
  • Configured the DeepSeek V4 reasoning parser to force-exit reasoning when <|DSML| starts
  • Added regression coverage for:
    • Batch parsing where DSML starts before </think>
    • Dangling partial DSML prefixes at the end of reasoning text
    • Streaming chunks where the DSML marker is split across deltas
    • Non-DSML lone < text that should remain reasoning content

Rust - parsers/v1/src/reasoning/base_parser.rs

  • Updated truncated reasoning handling to withhold suffix bytes that may still become a configured tool-start marker
  • Adjusted streaming overlap handling so even a one-character tool marker prefix is buffered until the next chunk disambiguates it
  • Preserves existing behavior for parsers without tool-start markers

Rust - parsers/v1/src/tool_calling/dsml/parser.rs

  • Changed DSML end-position detection to return None until the closing DSML block marker arrives
  • Updated tests so incomplete DSML blocks stay buffered instead of reporting chunk.len() as a completed boundary

Rust - parsers/v1/src/tool_calling/parsers.rs

  • Propagated the DSML parser's Option<usize> end-position result through the generic tool parser dispatch

Where should the reviewer start?

  • parsers/v1/src/reasoning/mod.rs:23 - DeepSeek DSML marker constant.
  • parsers/v1/src/reasoning/mod.rs:238-242 - DeepSeek V4 reasoning parser registration with the DSML tool-start marker.
  • parsers/v1/src/reasoning/base_parser.rs:231-240 and parsers/v1/src/reasoning/base_parser.rs:368-378 - Partial tool-start buffering in batch and streaming reasoning paths.
  • parsers/v1/src/tool_calling/dsml/parser.rs:51-63 - DSML end-position detection returning None for incomplete blocks.
  • parsers/v1/src/reasoning/mod.rs:378-438 - DeepSeek V4 regression tests covering DSML reasoning exits and split-marker streaming.

Related PRs:

Signed-off-by: slivanovich <skuralenok@172.24.29.171-spb.dhcp.yndx.net>
@slivanovich
slivanovich requested a review from a team as a code owner July 26, 2026 17:47
@copy-pr-bot

copy-pr-bot Bot commented Jul 26, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions Bot added external-contribution PR originates from a fork fix labels Jul 26, 2026
@slivanovich slivanovich changed the title fix: tools are leaking in the reasoning content (deepseek) fix(parsers): tools are leaking in the reasoning content (deepseek) Jul 26, 2026
dynamo-review-agent[bot]

This comment was marked as resolved.

@rmccorm4

rmccorm4 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Hi @slivanovich, thanks for raising this!

Two quick questions:

  1. Do you have some e2e curl requests that reproduce the leak? (@KrishnanPrash @indrajit96 we need to improve coverage here)
  2. If you try setting DYN_ENABLE_EXPERIMENTAL_PARSERS_V2=1 in your deployment, do you still see the leak in your setup?

@slivanovich

slivanovich commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

Hi @slivanovich, thanks for raising this!

Two quick questions:

  1. Do you have some e2e curl requests that reproduce the leak? (@KrishnanPrash @indrajit96 we need to improve coverage here)
  2. If you try setting DYN_ENABLE_EXPERIMENTAL_PARSERS_V2=1 in your deployment, do you still see the leak in your setup?

Thanks for the quick response!

  1. This problem seems to be probabilistic and appear while processing long context requests. So there is no exact curl that can show the bug. In out installation of DeepSeek-V4-Flash i've reproduced this on the very long context load test (N sessions up to 1M and more context).
  2. No, i've not tried to use v2 parsers yet. However, as i can see (probably missing smth) there is only v2 tool parser for DeepSeek models (no reasoning parser). This fix is about "reasoning do not end while receiving tool start in it". Also the tool start token for DeepSeek models (dsml token) is not "atomic" in terms of generation process ("<", "|DSML|", ...), so part of this pr handles this particular case (cause empirically considered that .with_tool_start_token(DEEPSEEK_TOOL_SECTION_BEGIN) is not enough).

@rmccorm4

Copy link
Copy Markdown
Contributor

/ok to test 6c720ca

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

@github-actions

github-actions Bot commented Jul 26, 2026

Copy link
Copy Markdown

📊 Conformance matrix rendered — view in CI summary

Comment thread parsers/v1/src/reasoning/base_parser.rs Outdated
dynamo-review-agent[bot]

This comment was marked as resolved.

dynamo-review-agent[bot]

This comment was marked as resolved.

Signed-off-by: slivanovich <skuralenok@172.24.29.171-spb.dhcp.yndx.net>
@slivanovich

slivanovich commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Have resolved @dynamo-review-agent[bot] comments

Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
Comment thread parsers/v1/src/reasoning/base_parser.rs Outdated
Signed-off-by: Ryan McCormick <mccormick.codes@gmail.com>

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

Open in Devin Review

Comment thread parsers/v1/src/reasoning/mod.rs
Comment thread parsers/v1/src/reasoning/mod.rs

@indrajit96 indrajit96 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CHanges look good to me, just one small request for the additional prompt-injected batch scenario test described above.
Thanks for finding and fixing this!
We also recently merged foundational Kimi K3 changes that touch the shared parser logic.
Could you merge the latest main into this branch and run the parser tests again to verify everything works together?

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

Open in Devin Review

Comment thread parsers/v1/src/tool_calling/parsers.rs
Signed-off-by: slivanovich <skuralenok@10.215.217.43-vpn.dhcp.yndx.net>
@slivanovich

slivanovich commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

CHanges look good to me, just one small request for the additional prompt-injected batch scenario test described above. Thanks for finding and fixing this! We also recently merged foundational Kimi K3 changes that touch the shared parser logic. Could you merge the latest main into this branch and run the parser tests again to verify everything works together?

Hi, thanks for the response!
Done: test for the tool call without opener, also add some into jail.rs part (V3.2 uses quite the same DSML notation, but anyway)

yeah, base parser fix has merged faster)

@indrajit96

Copy link
Copy Markdown
Contributor

/ok to test d3f8d64

@indrajit96
indrajit96 enabled auto-merge (squash) July 30, 2026 04:20
@indrajit96
indrajit96 merged commit 45dab5d into ai-dynamo:main Jul 30, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

external-contribution PR originates from a fork fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants