[https://nvbugs/6240584][fix] Attempt json.loads before early-return in tool parser#14777
[https://nvbugs/6240584][fix] Attempt json.loads before early-return in tool parser#14777tensorrt-cicd wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughThe PR adds a fallback JSON parsing path to ChangesQwen3 Bare JSON Tool Call Recovery
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
4bb40c8 to
6a579e6
Compare
json.loads(text.strip()) and feed the result |
/bot run --disable-fail-fast |
|
PR_Github #57124 [ run ] triggered by Bot. Commit: |
|
PR_Github #57124 [ run ] completed with state
|
|
/bot run |
|
PR_Github #57520 [ run ] triggered by Bot. Commit: |
|
PR_Github #57520 [ run ] completed with state |
|
/bot run |
|
PR_Github #57893 [ run ] triggered by Bot. Commit: |
|
PR_Github #57893 [ run ] completed with state |
|
/bot run --disable-fail-fast |
|
PR_Github #57930 [ run ] triggered by Bot. Commit: |
|
PR_Github #57930 [ run ] completed with state
|
… in detect_and_parse
Some Qwen3 chat templates (notably Qwen3.6 FP8 served with
--reasoning_parser qwen3_5 --tool_parser qwen3) emit tool calls as bare
JSON objects without the <tool_call>...</tool_call> wrapper once the
reasoning parser strips the </think> block. detect_and_parse() previously
returned calls=[] unconditionally whenever the bot_token ("<tool_call>\n")
was missing, so those tool calls leaked out as message.content with
finish_reason="stop".
Try json.loads + parse_base_json before falling through to the original
normal_text return. Streaming (parse_streaming_increment) and
has_tool_call() are intentionally unchanged.
Signed-off-by: tensorrt-cicd <90828364+tensorrt-cicd@users.noreply.github.com>
…SON fallback
This stacks on the initial fix in this PR and closes four gaps:
1) detect_and_parse: tighten exception handling. Replace
`except (JSONDecodeError, AttributeError)` with an explicit
`isinstance(parsed, (dict, list))` guard so bare JSON scalars
(e.g. "42", null) fall through cleanly without relying on
AttributeError.
2) parse_streaming_increment: extend the streaming path with the same
bare-JSON fallback. When the stream never contains `<tool_call>`
but the accumulated buffer parses as a JSON object matching
`{"name": ..., "arguments"/"parameters": ...}`, emit the tool
call using the base-class streaming pattern (name-first with empty
params, then arguments). Unblocks streaming clients on Qwen3.6 FP8
which otherwise saw the tool call leak into `delta.content` with
`finish_reason="stop"`. Wrapped happy path is preserved by
routing to the existing base implementation once `<tool_call>` is
detected.
3) has_tool_call() semantics unchanged. It still strictly gates on
`<tool_call>` for callers using it as a wrapper detector; the
bare-JSON fallback lives inside detect_and_parse /
parse_streaming_increment.
4) Tests. Unit tests exercise the bare-JSON fallback on
detect_and_parse (dict, list, `parameters` key, non-JSON text,
JSON scalar, malformed JSON) and on parse_streaming_increment
(single chunk, split chunks, no content leak, non-JSON flush,
wrapped-form regression). An integration test wires the qwen3_5
reasoning parser plus qwen3 tool parser through the postprocess
pipeline using the exact bug-report input
`<think>...</think>\\n{...}` and asserts `has_tool_call[0]` is
True (so downstream flips `finish_reason` to `tool_calls`).
Non-goals honored: no changes to other tool parsers, no changes to
openai_protocol.py or the API stability YAML.
Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
13c2198 to
c752d0c
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #61185 [ run ] triggered by Bot. Commit: |
|
PR_Github #61185 [ run ] completed with state
|
Summary
<tool_call>\nand returns calls=[] for any input lacking that tag, even when the text is valid bare JSON the base class's parse_base_json could handle.json.loads(text.strip())and feed the result throughself.parse_base_json(parsed, tools); on success return those calls with empty normal_text. Streaming path and has_tool_call are untouched.Test plan
Links
Summary by CodeRabbit