fix(openai): repair streamed tool-call indices for OpenAI-compatible providers#260
fix(openai): repair streamed tool-call indices for OpenAI-compatible providers#260danny-avila wants to merge 4 commits into
Conversation
…providers OpenAI-compatible endpoints don't always assign a distinct `index` to each parallel tool call in a streamed response. Ollama streams `index: 0` for every call in a batch (ollama/ollama#15457) and older builds omit `index` entirely (ollama/ollama#7881). LangChain merges streamed tool-call chunks by `index`, so when sibling calls share (or lack) an index and their arguments arrive in fragments, the continuations collapse into the first call — the other calls end up with empty/garbled args and fail tool-input schema validation with "Received tool input did not match expected schema" before any request is sent. This surfaced as LibreChat #13885 (parallel MCP tool calls fail client-side; sequential calls succeed). Repair the indices in the chat-completions stream before LangChain merges: key on the provider's tool-call `id` (which the affected providers still emit), falling back to call-start signals when the `id` is absent. Repair only engages once the provider's indices prove unreliable (two ids sharing one index, or a call start with no index), so well-behaved streams — official OpenAI, modern Ollama, first-party Azure — pass through unchanged. Gated to non-official endpoints.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 76b3cfc863
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } else if (isCallStart) { | ||
| assigned = state.next++; | ||
| } else { | ||
| assigned = state.lastIndex ?? 0; |
There was a problem hiding this comment.
When a provider emits multiple call-start deltas before the argument fragments (for example one chunk starts a and b, both with index: 0, and the following argument deltas omit id as OpenAI-style streams commonly do), lastIndex is left pointing at b. The next id-less fragment for a is then rewritten to index 1, so LangChain merges a's arguments into b and leaves a empty/invalid. This is still a fragmented parallel tool-call stream, so the fallback needs to avoid assigning unanchored fragments to the most recently seen call or track enough per-call ordering to route them correctly.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed the realistic form in 7287c00: fragments are now routed to the in-progress call (state.current), and any fragment that carries an id or index is attributed directly — so interleaved parallel streams reconstruct correctly.
The specific residual you describe — multiple call starts emitted before any of their argument fragments, where those later fragments then carry neither id nor index — is inherently unattributable: there's no signal tying such a fragment to a non-current call. In a streaming tool-call protocol a call's argument fragments arrive before the next call starts; providers that batch starts send complete arguments inline, and those that fragment typically repeat the id on continuations (which we key on). So this exact shape isn't produced by real OpenAI-compatible streams, and I left the fallback as best-effort (route to the current call) rather than buffer with no key to disambiguate on. Happy to revisit if there's a provider that actually emits it.
| const state = createToolCallReindexState(); | ||
| for await (const chunk of stream) { | ||
| for (const choice of chunk.choices) { | ||
| const toolCalls = choice.delta.tool_calls; | ||
| if (toolCalls != null && toolCalls.length > 0) { |
There was a problem hiding this comment.
For streamed requests that return multiple choices (for example n > 1), tool_calls[].index is scoped to each choice, but this single state is shared across all choices in the chunk stream. If choice 0 and choice 1 each start their first tool call at provider index 0, the second choice activates repair as a duplicate and later id-less fragments can be rewritten using the other choice's lastIndex, corrupting tool-call assembly for otherwise valid completions. Key the reindex state by choice.index so choices do not influence each other.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7287c00 — repair state is now kept per choice.index (a Map<number, ToolCallReindexState> in _reindexToolCallStream), so choices in an n > 1 stream no longer influence one another. Added a test where two choices each open a call at provider index 0 and both correctly remain at index 0 (shared state would have bumped the second to 1).
| if (isCallStart && providerIndex == null) { | ||
| state.active = true; | ||
| } else if (id != null && providerIndex != null) { |
There was a problem hiding this comment.
When a compatible provider omits tool-call ids but still sends a repeated index on each call start, this branch never records or compares the start because id is null, so repair never activates and every sibling keeps the provider's duplicate index. That leaves a sequential stream such as start/args for call 1 followed by start/args for call 2 collapsed under index 0, even though the function-name start signal is enough to assign the second call a new repaired index.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7287c00 — a call start is now detected by id or function.name, and repair activates when a start reuses an already-opened provider index (tracked in startedProviderIndices), regardless of whether an id is present. So an id-less stream that stamps index 0 on every name start now assigns the second call a fresh repaired index. Covered by a new test (id-less starts, duplicate index 0 → alpha/beta separated).
| if (!state.active) { | ||
| if (providerIndex != null) { | ||
| state.lastIndex = providerIndex; | ||
| } | ||
| continue; |
There was a problem hiding this comment.
If a compatible provider includes distinct indices on the tool-call starts but omits index on later argument fragments, this early continue leaves those fragments unrepaired because they are not call starts. In a parallel stream such as call 0 start, call 0 args, call 1 start, call 1 args-without-index, LangChain buckets the last fragment under the default/first index and corrupts call 0 instead of using the last known call index.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7287c00 — argument fragments that omit index are now anchored to the in-progress call (state.current) instead of being left unindexed, even on an otherwise-reliable stream where the starts carried distinct indices. New test covers start(index 0) / arg(no index) / start(index 1) / arg(no index) → reconstructs alpha/beta correctly.
Rework the repair so a streamed tool call is modeled as a start delta (id and/or function.name) followed by argument fragments, with one repaired index per start and id-less fragments routed to the in-progress call. This addresses the review findings: - Per-choice state: keep repair state keyed by choice.index so concurrent choices (n > 1) never influence each other. - Name-signal starts: a second call start that reuses an already-opened index now activates repair even when the provider omits tool-call ids (function.name alone marks a new call), so id-less duplicate-index streams are separated. - Index-less argument fragments: anchor fragments that omit index to the in-progress call instead of leaving them unindexed, even when call starts carried distinct indices. Well-behaved streams (official OpenAI, modern Ollama, first-party Azure) still pass through unchanged. Adds unit coverage for each case.
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Avoid the extra work the first pass added: - Reindex now runs inside the existing filterOpenAIChatCompletionStream pass instead of wrapping it in a second async generator, so chunks aren't hopped through an extra layer (and text-only / official-endpoint streams are untouched as before). - Drop the startedProviderIndices Set: a reused/duplicate start index is just `providerIndex < next` (the high-water mark already tracked). Behavior unchanged; reindex unit tests and the openai suite still pass.
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Problem
OpenAI-compatible endpoints don't always assign a distinct
indexto each parallel tool call in a streamed response:index: 0for every call in a parallel batch (ollama/ollama#15457)indexentirely (ollama/ollama#7881)LangChain merges streamed tool-call chunks by
index. When sibling calls share (or lack) an index and their arguments arrive in fragments, the continuation fragments collapse into the first call — the other calls end up with empty/garbled args and fail tool-input schema validation withReceived tool input did not match expected schemabefore any request is sent.This is the client-side mechanism behind LibreChat #13885 (parallel MCP tool calls fail; the same calls succeed sequentially — a lone call has no sibling to collapse with).
Fix
Repair the indices in the chat-completions stream before LangChain merges chunks (
_reindexToolCallStreaminsrc/llm/openai/index.ts):id(which the affected providers still emit), falling back to call-start signals when theidis absent;Tests
src/llm/openai/toolCallReindex.test.ts(4 tests): repairsindex: 0-for-all and missing-index streams, leaves well-behaved streams untouched, and documents the collapse without repair.Validation
Reproduced #13885 end-to-end against the real
@modelcontextprotocol/server-everythingecho tool through the agent graph's event-drivenON_TOOL_EXECUTEpath. With this build, the previously-failing shapes (fragmented args + shared/missing index) now execute and reach the server; well-formed streams (real current Ollama, which emits proper indices) are unaffected.