fix(run_agent): split concatenated streamed tool-call args - #25346
fix(run_agent): split concatenated streamed tool-call args#25346LeonSGP43 wants to merge 1 commit into
Conversation
alt-glitch
added
type/bug
|
Confirmed reproducing on stable Repro (run 5x per arm)Baseline (without this PR)Sample warning (note the Across 5 trials: most prompts dropped multiple With this PRAll 4 (The "split into 3" + 1 standalone = 4 total tool calls — matches the prompt.) ConclusionThis bug is not limited to the preview model — the stable Happy to provide more repro data on request. |
…nonfinite, coerce booleans, port content-channel promotion
Six pre-existing failure modes in the tool-call argument pipeline, all
surfaced by adversarial probing and online research into upstream
Hermes PRs. 241/241 tests passing in the repair+coerce+transport surface.
A. Concatenated streamed tool-call args — Gemini-3-flash-preview and
some Ollama routes emit ``{"a":1}{"b":2}`` with no delimiter. The
repair layer was falling through to ``{}`` and losing every call.
Fixed by adding ``_split_concatenated_tool_call_arguments`` (matches
upstream PR NousResearch#25346/NousResearch#36039 verbatim so a future merge is conflict-
free), which uses ``json.JSONDecoder().raw_decode(pos=...)`` to
walk left-to-right and peel off complete top-level dicts.
B. JSON buried in surrounding noise — U+2028/U+2029 line separators,
BOM markers, model preamble/postscript, extra ``true``/``null``/
``"excess"`` after valid JSON, etc. All made ``json.loads`` reject
the whole payload. Fixed by adding ``_extract_first_json_object``,
which uses ``raw_decode`` from every ``{`` position in the string
and rescues the first complete dict it finds.
C. NaN / Infinity pass-through — ``json.loads(strict=False)`` accepts
the literal tokens, but re-serialising produces ``NaN``/``Infinity``
literals that strict-validating providers (Anthropic, AWS Bedrock,
Google Vertex) reject with HTTP 400. Fixed by adding
``_scrub_nonfinite_numbers`` which recursively replaces them with
``None``.
D. Integer overflow silent corruption in ``_coerce_number`` —
``'99999999999999999999'`` was being routed through ``float()`` then
``int()``, producing ``100000000000000000000`` (off by ~9 orders of
magnitude). Python's ``int()`` has arbitrary precision, so for
``integer_only`` fields we now try ``int(value)`` first; the float
path is the fallback for decimal strings like ``"3.0"``.
E. ``_coerce_boolean`` accepted only ``"true"`` / ``"false"`` while
M3, DeepSeek, Qwen, and GLM routinely emit ``"1"`` / ``"0"`` /
``"yes"`` / ``"no"`` / ``"on"`` / ``"off"`` for boolean fields.
Extended the match set. Two pre-existing tests asserted the old
wrong behaviour; updated with comments explaining the change.
F. Content-channel tool calls (M3 / MiniMax, Kimi K2, Ollama
qwen2.5-coder, GLM, Gemma) — models emit tool calls in the
response ``content`` field as ``<invoke name="…">…</invoke>`` or
``<tool_call>{…}</tool_call>`` instead of the structured
``tool_calls`` field, causing the call to leak as chat text. Ported
the leaf parser from upstream Hermes PR NousResearch#35129 into a new module
``agent/transports/content_tool_calls.py`` (309 lines, 6 parsers,
exact-name gate, fail-closed, env kill-switch
``HERMES_PROMOTE_TOOLCALLS``) and added a single promotion seam
in ``agent/conversation_loop.py`` at the post-normalization point
in the response flow. The seam is a strict no-op when structured
``tool_calls`` already exist, so native tool-calling paths are
untouched.
Tests:
- 4 new classes in test_repair_tool_call_arguments.py (53 tests) for
the split/extract/scrub helpers and end-to-end coverage of the new
repair stages.
- 1 new class in test_tool_arg_coercion.py (8 tests) for integer
overflow precision.
- 2 pre-existing tests in test_tool_arg_coercion.py updated to assert
the new correct boolean coercion behaviour.
- New file tests/agent/transports/test_content_tool_calls.py (17 tests)
covering all 6 content-channel parsers, dedup, env gates, and
fail-closed behaviour on unknown tool names.
Files changed: 6 modified, 2 new. +506/-13.
Upstream: PRs NousResearch#25346, NousResearch#36039, NousResearch#35129 are still open. If the user
wants to push these locally, the diffs are designed to merge near-zero-
conflict with those PRs.
|
Thanks for the careful lossless parsing approach and the focused regression coverage. This automated hermes-sweeper review is closing this as not planned because the underlying issue has a maintainer product-boundary decision: malformed or concatenated provider/model tool-call output must not be inferred, split, or repaired into executable calls. The owner’s closure of #25333 explicitly identifies splitter-style recovery as out of scope.
A focused provider or adapter fix that prevents invalid structured tool-call output at its source would be the appropriate direction. Closed as not-planned per standing maintainer policy ( |
teknium1
added
the
sweeper:not-planned
4 participants
Summary
{"a":1}{"b":2}into separate streamed tool callsTesting
uv run --frozen pytest -o addopts='' tests/run_agent/test_repair_tool_call_arguments.py tests/run_agent/test_run_agent.py::TestStreamingApiCall::test_concatenated_json_arguments_split_into_parallel_tool_calls tests/run_agent/test_run_agent.py::TestStreamingApiCall::test_ollama_reused_index_separate_tool_calls tests/run_agent/test_run_agent.py::TestStreamingApiCall::test_truncated_tool_call_args_upgrade_finish_reason_to_lengthgit diff --checkuv run --frozen ruff check run_agent.py tests/run_agent/test_repair_tool_call_arguments.py tests/run_agent/test_run_agent.pyAttribution
Known red
Testson recent company PRs andorigin/mainis treated aspreexisting_unrelatedbaseline noise; this candidate's targeted local gate is green.Closes #25333.