Repository navigation
[https://nvbugs/6958731][fix] Use the tokenizer chat template when the processor has none - #20880
Conversation
…e processor has none resolve_hf_chat_template returned processor.chat_template for every request without tools whenever a processor was loaded, even when that attribute was None. apply_chat_template then raised "No chat template found for the given tokenizer and tools.", which trtllm-serve returns as HTTP 400 for every chat completion. MiniMax-M3-NVFP4 hits this: AutoProcessor loads its remote-code MiniMaxVLProcessor with chat_template=None, while its tokenizer carries the checkpoint's chat_template.jinja. Use the processor's template only when it has one and otherwise fall through to the tokenizer. Add tests for both cases: a processor without a template falls back to the tokenizer's, and a processor template is still preferred. Signed-off-by: chenfeiz0326 <203214996+chenfeiz0326@users.noreply.github.com>
|
/bot run --disable-fail-fast |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe chat-template resolver now falls back to tokenizer lookup when the processor does not provide a template. Added tests check tokenizer fallback and processor-template precedence. ChangesChat template resolution
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~8 minutes Change: Bug fix Possibly related PRs
Merge Risk: ⚪ Minimal · up to
|
|
PR_Github #80097 [ run ] triggered by Bot. Commit: |
|
GB300 end-to-end result with this change: the MiniMax-M3 tp4 con1 AgentX lane passed on oci-jhb. I ran the build 3150 wheel with this fix overlaid and the unmodified client, under CI's own
|
JunyiXu-nv
left a comment
There was a problem hiding this comment.
Approve with a nit in test.
|
Automatically added "ci: full pre-merge approved" because this PR has satisfied the required GitHub review approvals. Unresolved review conversations and other required checks remain independent merge requirements. |
… which template is used The fake tokenizer now returns the template it is asked to render, so each test asserts on the rendered result itself rather than on a captured argument. Signed-off-by: chenfeiz0326 <203214996+chenfeiz0326@users.noreply.github.com>
|
/bot run --disable-fail-fast |
|
PR_Github #80134 [ run ] triggered by Bot. Commit: |
|
PR_Github #80097 [ run ] completed with state |
|
/bot run --disable-fail-fast |
|
PR_Github #80179 [ run ] triggered by Bot. Commit: |
|
PR_Github #80134 [ run ] completed with state |
|
/bot run --disable-fail-fast |
|
PR_Github #80183 [ run ] triggered by Bot. Commit: |
|
PR_Github #80179 [ run ] completed with state |
brnguyen2
left a comment
There was a problem hiding this comment.
The resolver change matches how the native-template guard in apply_chat_template already treats a processor whose chat_template is None, and the two new tests cover both resolution orders. I have no changes to request.
|
PR_Github #80183 [ run ] completed with state |
Description
nvbugs/6958731: the three MiniMax-M3 AgentX lanes in GB300 post-merge perf-sanity (
aggr_upload-gb300_minimax_m3_fp4_agentx_grace_blackwell-*) have failed on every run since they were added. trtllm-serve answered everyPOST /v1/chat/completionswith HTTP 400, so the readiness probe never completed one token and each lane timed out after 900 s. With #20749 in place, the probe reports the server's reason:Root cause.
resolve_hf_chat_templatereturnedprocessor.chat_templatefor every request without tools whenever a processor was loaded, even when that attribute wasNone.apply_chat_templatethen raisedValueError("No chat template found for the given tokenizer and tools."), andopenai_chatturns that into HTTP 400 without logging a traceback.AutoProcessorfor every checkpoint. For MiniMax-M3-NVFP4 that is the checkpoint's remote-codeMiniMaxVLProcessor, whosechat_templateisNone. The tokenizer, and the processor's own tokenizer, carry the checkpoint'schat_template.jinja(11,752 characters).{"messages":[{"role":"user","content":"hi"}],"max_tokens":1}.Fix. Use the processor's template only when it has one; otherwise fall through to the tokenizer's template. vLLM resolves the same way:
resolve_chat_templateinvllm/renderers/hf.pytakes the processor's template only when it is notNone, then falls back to the tokenizer.Test Coverage
TestProcessorWithoutChatTemplateintests/unittest/inputs/test_chat_template_dispatch.py, which iscpu_onlyand runs in l0_cpu through theunittest/inputsentry:ValueErrorthe server returns.pytest.iniwith pytest-threadleak): the new fallback test fails against the installed module and passes with this change. With the change,test_chat_template_dispatch.pyandtest_chat_template_kwargs_guard.pypass (78 tests).b833b7c9) and CI's config for the tp4 con1 lane:/v1/chat/completionsreturned the 400 above, while/v1/completionsreturned 200 with a generated token.AutoProcessorgaveMiniMaxVLProcessorwithchat_template=None, and the tokenizer held the 11,752-character template.apply_chat_templateraised theValueError. With the fixed resolver it rendered the MiniMax-M3 prompt, identical totokenizer.apply_chat_template.test_e2e[aggr-gb300_minimax_m3_fp4_agentx_grace_blackwell-minimax_m3_fp4_tp4_eagle3_mtp7_con1_agentx], unmodified AgentX client. The lane passed (1 passed, 1 h 20 min).1-token completion OK.health OK: 267 requests over 3614.2s, submission_valid=true. One empty-content response was within the 0.5% tolerance.PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
If PR introduces API changes, an appropriate PR label is added - either
api-compatibleorapi-breaking. Forapi-breaking, includeBREAKINGin the PR title.Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.🤖 Generated with Claude Code
Dev Engineer Review
resolve_hf_chat_templateuses a non-Noneprocessor chat template when applicable. If the processor template is missing orNone, resolution falls back to the tokenizer’s template. No current review findings were supplied.QA Engineer Review
tests/unittest/inputs/test_chat_template_dispatch.pyaddsTestProcessorWithoutChatTemplatetests for tokenizer fallback and processor-template precedence. The tests cover the reported missing-template behavior. No integration test-list change appears applicable to this unit test. The PR description reports 78 passing tests and a passing GB300 MiniMax-M3 end-to-end run. Coverage verdict: sufficient for the changed resolution behavior.Per-File QA Perspective
tensorrt_llm/inputs/utils.py: Verify that a present processor template takes precedence and that a missing orNonetemplate falls back to tokenizer lookup. Confirm explicit-template and tool-aware behavior remains unchanged.tests/unittest/inputs/test_chat_template_dispatch.py: Covers processor-template precedence and tokenizer fallback. The unit test does not require an integration CI or manual-QA list entry.