Skip to content

fix: handle whitespace between <tool_call> tag and JSON in Hermes handler#88

Merged
leehack merged 2 commits intoleehack:mainfrom
ipmsteven:tool-calling-fix
Mar 17, 2026
Merged

fix: handle whitespace between <tool_call> tag and JSON in Hermes handler#88
leehack merged 2 commits intoleehack:mainfrom
ipmsteven:tool-calling-fix

Conversation

@ipmsteven
Copy link
Copy Markdown
Contributor

@ipmsteven ipmsteven commented Mar 15, 2026

Summary

  • Fix Hermes handler parse() failing to extract tool calls when whitespace (space/newline) exists between <tool_call> and the JSON object
  • This format is produced by Qwen models per their Jinja template: <tool_call>\n{"name":...}\n</tool_call>

Root Cause

_openRegex group(3) captures leading whitespace as part of the match (e.g., {"name"). When computing jsonStart, the offset pointed to the whitespace character instead of {, causing extractLeadingJsonValue() to return null and the entire tool call to fall back as plain content.

Fix

Skip leading whitespace after computing jsonStart to find the actual { before calling _extractJsonObjectRange(). This is consistent with how Granite, Mistral, and Magistral handlers already handle whitespace before their extractLeadingJsonValue() calls.

Test plan

  • 3 new test cases: space, newline, and mixed whitespace between tag and JSON
  • All 1027 existing tests pass
  • Verified end-to-end with Qwen3.5-2B on iOS simulator

…dler

The Hermes handler's parse() failed to extract tool calls when there
was whitespace (space or newline) between <tool_call> and the JSON
object, e.g. `<tool_call> {"name":...}` or `<tool_call>\n{"name":...}`.

Root cause: _openRegex group(3) captures leading whitespace as part of
the match (e.g., ` {"name"`). When computing jsonStart, the offset
pointed to the whitespace character instead of '{', causing
extractLeadingJsonValue() to return null and the entire tool call to
fall back as plain content.

Fix: skip leading whitespace after computing jsonStart to find the
actual '{' before calling _extractJsonObjectRange(). This is consistent
with how Granite, Mistral, and Magistral handlers already handle
whitespace before their extractLeadingJsonValue() calls.
@leehack
Copy link
Copy Markdown
Owner

leehack commented Mar 16, 2026

Thanks for the contribution, @ipmsteven!

It seems there's a very minor issue. Would you mind fixing the formatting? I can merge it as soon as that's addressed.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.90%. Comparing base (ff0be57) to head (95190f9).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #88      +/-   ##
==========================================
- Coverage   77.23%   76.90%   -0.34%     
==========================================
  Files          67       67              
  Lines        8338     8368      +30     
==========================================
- Hits         6440     6435       -5     
- Misses       1898     1933      +35     
Flag Coverage Δ
unittests 76.90% <100.00%> (-0.34%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ipmsteven
Copy link
Copy Markdown
Contributor Author

Thanks for the contribution, @ipmsteven!

It seems there's a very minor issue. Would you mind fixing the formatting? I can merge it as soon as that's addressed.

formatting issue should be fixed now 👍

@leehack leehack merged commit 5f8e0e3 into leehack:main Mar 17, 2026
6 checks passed
@ipmsteven ipmsteven deleted the tool-calling-fix branch March 18, 2026 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants