Skip to content

Fix tool call reasoning boundary - #6411

Merged
nvcsathe merged 2 commits into
NVIDIA:mainfrom
nvcsathe:parser-tool-call-fix
Aug 13, 2026
Merged

Fix tool call reasoning boundary#6411
nvcsathe merged 2 commits into
NVIDIA:mainfrom
nvcsathe:parser-tool-call-fix

Conversation

@nvcsathe

@nvcsathe nvcsathe commented Aug 10, 2026

Copy link
Copy Markdown
Contributor
  • I, the PR author, have personally reviewed every line of this PR.

What does this PR do?

⚠️ For major changes (either in lines of code or in its impact), please make sure to first share a design doc with the team. If you're unsure what's the best way to do so, contact @NVIDIA/mcore-oncall.

Issue tracking

For PRs from open-source community contributors:

  • New features: a linked issue is required. Please open a feature request and reference it here before submitting the PR.
  • Small updates (bug fixes, minor improvements): a linked issue is recommended and will accelerate the PR review process.

Linked issue:

Contribution process

Pre-checks

  • I have added relevant unit tests
  • I have added relevant functional tests
  • I have added proper typing to my code Typing guidelines
  • I have added relevant documentation
  • I have run the autoformatter.sh on my PR

Code review

Feel free to message or comment @NVIDIA/mcore-oncall to help accelerate your merge into main. The less complex your PR is, the faster it will be approved and merged!

All PRs start as draft. If you open a non-draft PR, it will be automatically converted to draft.

Step 1: Mark PR as "Ready for Review"

  1. When your PR is ready, click Ready for Review.
  2. An oncall reviewer is auto-assigned and expert reviewers are notified based on your changes.
    • Some PRs may jump straight to step 2. This is determined by .github/CODEOWNERS.

⚠️ Only mark as ready once merge-conflicts are resolved and the CI is passing.
Final Review might get declined if these requirements are not fulfilled.

Step 2: Final Review

For PRs that change megatron/core, once all expert reviewers have approved, the Final Review label is applied automatically and final reviewers are assigned.

For PRs outside megatron/core, this step is skipped.

Step 3: Approved

Once all required reviewers have approved, the Approved label is applied automatically.

Merge

Any member of mcore-engineers will be able to merge your PR.

@copy-pr-bot

copy-pr-bot Bot commented Aug 10, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@nvcsathe
nvcsathe marked this pull request as ready for review August 10, 2026 17:32
@nvcsathe
nvcsathe requested review from a team as code owners August 10, 2026 17:32
dimapihtar
dimapihtar requested changes Aug 13, 2026
View reviewed changes

@dimapihtar dimapihtar 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.

Hi, could you add unit tests for newly added logic/function to the tokenizers? Or it will be covered by funcitonal tests?

@dimapihtar

dimapihtar commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Hi, could you add unit tests for newly added logic/function to the tokenizers? Or it will be covered by funcitonal tests?

oh sorry, you've already added. Just missed it. LGTM.

@dimapihtar
dimapihtar self-requested a review August 13, 2026 12:20
@svcnvidia-nemo-ci svcnvidia-nemo-ci added the Approved All necessary approvals have been made label Aug 13, 2026
@nvcsathe
Fix tool call reasoning boundary
5a8c84e 
Signed-off-by: Chaitra Sathe <csathe@nvidia.com>
@nvcsathe
fix formatting for linting
ca9feee 
Signed-off-by: Chaitra Sathe <csathe@nvidia.com>
@nvcsathe
nvcsathe force-pushed the parser-tool-call-fix branch from 2f42c5a to ca9feee Compare August 13, 2026 16:08
@nvcsathe
nvcsathe enabled auto-merge August 13, 2026 16:08
@nvcsathe
nvcsathe added this pull request to the merge queue Aug 13, 2026
@svcnvidia-nemo-ci

svcnvidia-nemo-ci commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

🔄 Merge queue validation started!

You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/31725348823

Merged via the queue into NVIDIA:main with commit f7b2209 Aug 13, 2026
89 checks passed
@nvcsathe
nvcsathe deleted the parser-tool-call-fix branch August 13, 2026 19:45
@lhb8125 lhb8125 mentioned this pull request Aug 27, 2026
Open
ksivaman pushed a commit to ksivaman/Megatron-LM that referenced this pull request Sep 1, 2026
@nvcsathe
Fix tool call reasoning boundary (NVIDIA#6411)
513b87a 
Signed-off-by: Chaitra Sathe <csathe@nvidia.com>
ArEsKay3 added a commit to ArEsKay3/Megatron-LM that referenced this pull request Sep 14, 2026
@ArEsKay3 @claude
[Inference] Make the tool-call implicit reasoning end opt-in
38bb3fd 
NVIDIA#6411 taught the qwen3-coder tool parser to advertise <tool_call> as an
implicit end of reasoning, so a tool call emitted before </think> is
split out of the reasoning text and parsed. That matches vLLM's combined
qwen3 parser engine, but not vLLM's standalone reasoning parsers
(deepseek_r1 and its subclasses) paired with the qwen3_coder tool
parser, which leave such a tool call inside reasoning_content. Anyone
serving the same model on both engines with a separate reasoning parser
now gets different tool calls out, and RL rewards that penalise an
unparsed <tool_call> see the two engines disagree.

Keep qwen3-coder-tool strict (no implicit end), matching the separate-
parser pairing, and add qwen3-coder-tool-combined carrying the marker
for callers who want the combined-engine behaviour. The choice is made
in the parser list, where it is visible in the server config, rather
than implied by which tool parser is loaded. Streaming markers are
unchanged on both.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Robert Kirby <rkirby@nvidia.com>
@ArEsKay3 ArEsKay3 mentioned this pull request Sep 14, 2026
Closed
4 of 6 tasks
sidsingh-nvidia pushed a commit to sidsingh-nvidia/Megatron-LM that referenced this pull request Sep 17, 2026
@ArEsKay3 @sidsingh-nvidia
[Inference] Make the tool-call implicit reasoning end opt-in
ec4a436 
PR NVIDIA#6411 made qwen3-coder-tool declare <tool_call> an implicit end of
reasoning, so a tool call emitted before </think> is split out of the
reasoning text and parsed. That matches vLLM's combined qwen3 parser but
not its standalone reasoning parsers paired with the qwen3_coder tool
parser, which leave such a tool call in reasoning_content. Serving the
same model on both engines with a separate reasoning parser now yields
different tool calls.

Keep qwen3-coder-tool strict and add qwen3-coder-tool-combined carrying
the marker, so the behaviour is chosen in the parser list. Streaming
markers are unchanged on both.

Folded into the Qwen3-Coder parser-parity branch alongside the grammar and
coercion parity fixes (NVIDIA#7320); relevant to NVIDIA#7313.

Signed-off-by: Robert Kirby <rkirby@nvidia.com>
(cherry picked from commit c09b3fe)
Signed-off-by: Siddharth Singh <sidsingh@nvidia.com>
sidsingh-nvidia pushed a commit to sidsingh-nvidia/Megatron-LM that referenced this pull request Sep 18, 2026
@ArEsKay3 @sidsingh-nvidia
[Inference] Make the tool-call implicit reasoning end opt-in
12126c1 
PR NVIDIA#6411 made qwen3-coder-tool declare <tool_call> an implicit end of
reasoning, so a tool call emitted before </think> is split out of the
reasoning text and parsed. That matches vLLM's combined qwen3 parser but
not its standalone reasoning parsers paired with the qwen3_coder tool
parser, which leave such a tool call in reasoning_content. Serving the
same model on both engines with a separate reasoning parser now yields
different tool calls.

Keep qwen3-coder-tool strict and add qwen3-coder-tool-combined carrying
the marker, so the behaviour is chosen in the parser list. Streaming
markers are unchanged on both.

Folded into the Qwen3-Coder parser-parity branch alongside the grammar and
coercion parity fixes (NVIDIA#7320); relevant to NVIDIA#7313.

Signed-off-by: Robert Kirby <rkirby@nvidia.com>
(cherry picked from commit c09b3fe)
Signed-off-by: Siddharth Singh <sidsingh@nvidia.com>

This branch was previously deployed

2 inactive deployments
public — ca9feeef Deployed Aug 13, 2026 by copy-pr-bot[bot] via finalize / publish-docs #6020
test — ca9feeef Deployed Aug 13, 2026 by copy-pr-bot[bot] via cicd-wait-in-queue #18924
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kvareddy kvareddy kvareddy approved these changes
@dimapihtar dimapihtar dimapihtar approved these changes
@shanmugamr1992 shanmugamr1992 shanmugamr1992 approved these changes
@sidsingh-nvidia sidsingh-nvidia sidsingh-nvidia approved these changes

Assignees

No one assigned

Labels

Approved All necessary approvals have been made complexity: low

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@nvcsathe @dimapihtar @svcnvidia-nemo-ci @kvareddy @shanmugamr1992 @sidsingh-nvidia