Skip to content

fix(streaming): merge tool-call fragments when providers reuse index with fresh per-fragment ids - #45164

Open
MichaelZelbel wants to merge 1 commit into
NousResearch:mainfrom
MichaelZelbel:fix/streaming-toolcall-fragment-fresh-id
Open

fix(streaming): merge tool-call fragments when providers reuse index with fresh per-fragment ids#45164
MichaelZelbel wants to merge 1 commit into
NousResearch:mainfrom
MichaelZelbel:fix/streaming-toolcall-fragment-fresh-id

Conversation

@MichaelZelbel

Copy link
Copy Markdown

Summary

Some providers stream a single tool call with index held at 0 but a fresh id on every argument fragment (the function name only on the first chunk). The streaming accumulator's "different id at the same index → new slot" rule treats each fragment as a separate tool call, shattering one call into many invalid-JSON pieces. Downstream this trips has_truncated_tool_args, which upgrades finish_reason to "length", so the user gets "Response truncated due to output length limit" even though nothing was truncated.

This affects Kimi kimi-for-coding/K2.6 (https://api.kimi.com/coding/v1) and LM-Studio (cf. #5331). It only happens on tool-calling turns, which makes it look intermittent — plain-text replies are fine.

Root cause (captured deltas)

A single skill_view call streamed from Kimi:

index=0 id='tool_OEUd…' name='skill_view' args=''
index=0 id='tool_Cfy6…' name=None        args='{"'
index=0 id='tool_dR9g…' name=None        args='name'
index=0 id='tool_haJT…' name=None        args='":'
… (one fresh id per fragment, name only on the first) …
index=0 id='tool_xG1v…' name=None        args='"}'

Result: 11 tool-call slots, each holding an invalid-JSON fragment; completion_tokens was ~230 (no real length truncation).

Fix

Open a new slot only when the delta carries a function.name. Per the OpenAI streaming convention only the first chunk of a tool call carries the name; argument-continuation chunks never do. This preserves the existing Ollama parallel-call detection (a genuine new call always begins with a name) and is more robust than an arguments-length heuristic (it doesn't depend on fragment size).

_delta_has_name = bool(tc_delta.function is not None and tc_delta.function.name)
if (delta_id and raw_idx in _last_id_at_idx
        and delta_id != _last_id_at_idx[raw_idx]
        and _delta_has_name):
    new_slot = max(tool_calls_acc, default=-1) + 1
    _active_slot_by_idx[raw_idx] = new_slot

Test

Adds test_kimi_fresh_id_per_fragment_merges_into_one_tool_call. Verified it fails without the guard (shatters into 11 calls) and passes with it; existing test_ollama_reused_index_* and the MiniMax/accumulator tests stay green (11 passed).

Related

Same root cause as #5331 (LM-Studio) — this proposes a smaller, provider-agnostic condition (gate on name vs. an len(args) < 100 heuristic) and adds Kimi coverage. Also adjacent to #25046 (Gemini streaming JSON). Refs #18742.

🤖 Generated with Claude Code

…with fresh per-fragment ids

Kimi (kimi-for-coding/K2.6) and LM-Studio stream a single tool call with
the index held at 0 but a FRESH id on every argument fragment; the
function name only appears on the first chunk. The existing "different id
at the same index -> new slot" rule treated each fragment as a separate
tool call, shattering one call into many invalid-JSON pieces. Downstream
that trips has_truncated_tool_args, which upgrades finish_reason to
"length", so the user gets "Response truncated due to output length
limit" even though nothing was actually truncated (only happens on
tool-calling turns, which makes it look intermittent).

Only open a new slot when the delta also carries a function.name: per the
OpenAI streaming convention only the first chunk of a tool call carries
the name; argument-continuation chunks never do. This keeps the existing
Ollama parallel-call detection (a new call always begins with a name) and
adds a regression test for the Kimi/LM-Studio fragment pattern.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@liuhao1024

Copy link
Copy Markdown
Contributor

Verified: this is a clean, well-scoped fix with a clear root-cause analysis and solid test coverage.

What's correct:

  • The _delta_has_name guard correctly implements the OpenAI streaming convention: only the first chunk of a tool call carries function.name; argument-continuation chunks carry only function.arguments.
  • Adding and _delta_has_name to the existing Ollama re-use-index condition is the minimal correct change — it doesn't affect the Ollama case (which always sends the name on the first chunk of a genuinely new call).
  • The test (test_kimi_fresh_id_per_fragment_merges_into_one_tool_call) exercises the exact failure mode: 6 fragments at index=0 with fresh IDs, only the first carrying a name. The assertion on finish_reason == "tool_calls" confirms the stream isn't misreported as truncated.

No issues found. The fix is surgical — one boolean guard + one test, no behavioral side effects on existing providers.

@khamis1992 khamis1992 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hermes Agent Review — APPROVE ✅

Verdict: Ready to merge.

What it does

Fixes streaming tool-call accumulation for providers (Kimi, LM-Studio) that keep index=0 fixed but emit a fresh ID on every argument fragment. Previously each fragment opened a new slot, shattering one call into many invalid-JSON pieces. The fix adds a guard: only treat a changed ID as a new tool call when the delta also carries a function.name (per OpenAI convention, only the first chunk carries the name).

Review

  • Correctness: ✅ The fix correctly follows the OpenAI streaming convention. The name guard is the right discriminator.
  • Security: ✅ No concerns.
  • Testing: ✅ New test test_kimi_fresh_id_per_fragment_merges_into_one_tool_call with 7 fragments across 6 different IDs — verifies they merge into one call with valid JSON. Also asserts finish_reason stays tool_calls (not upgraded to length truncation).
  • Blast radius: LOW — affects only the streaming tool-call accumulator, and only when a provider emits fresh IDs per fragment.
  • Code quality: ✅ Clear comment explaining the rationale. Minimal change.

Suggestions (non-blocking)

  • None.

Reviewed by Hermes Agent (builder profile)

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint provider/kimi Kimi / Moonshot P2 Medium — degraded but workaround exists labels Jun 12, 2026
@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused regression coverage. Current main still creates a new accumulator slot for every changed ID at a reused index in agent/chat_completion_helpers.py:2396-2402; that directly permits the reported fresh-ID, argument-only fragments to shatter one call.

The proposed name guard is consistent with current behavior: genuine same-index second calls are named in tests/run_agent/test_run_agent.py:6891-6913, while continued arguments are unnamed in :6915-6933. The added Kimi case covers the currently untested fresh-ID continuation variant and asserts that the assembled response remains tool_calls rather than being upgraded to length.

The target blocks remain materially unchanged despite later line movement, so this is a high-salvageability mechanical transplant onto current main.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added the area/streaming Streaming responses: gateway delivery, provider wire label Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/streaming Streaming responses: gateway delivery, provider wire comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists provider/kimi Kimi / Moonshot sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants