fix: dedupe repeated scalar response_metadata in streamed chunks#295
Conversation
Providers that emit `finish_reason` on more than one streamed chunk (e.g. OpenRouter / custom OpenAI-compatible endpoints) made LangChain core's `_mergeDicts` concatenate the scalar fields `@langchain/openai` stamps alongside finish_reason, producing `finish_reason: "stopstop"` and duplicated `model_name` in both the aggregated graph message and the Langfuse trace (core keeps `id`/`name`/`model_provider` last, but not these). Fold a keep-first dedupe into the shared `delayStreamChunks` wrapper so repeated `model_name`/`finish_reason`/`service_tier`/`system_fingerprint` are dropped at the chunk source, before either the core `_streamIterator` aggregation (handleLLMEnd / Langfuse) or any host-side concat runs. Single occurrences are untouched. Covers ChatOpenAI/Azure/DeepSeek/XAI, and ChatOpenRouter/ChatMoonshot via inheritance.
|
@codex review |
|
@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: 4c08f446f2
ℹ️ 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".
| if (!seen.has(field)) { | ||
| seen.add(field); |
There was a problem hiding this comment.
When a streamed request asks for multiple choices (n > 1), chunks for different completions share this one seen set even though each chunk carries its own generationInfo.completion and the caller later aggregates by that index. After completion 0's final chunk records finish_reason/model_name, completion 1's final chunk has those fields deleted here, so the second generated message/LLMEnd output loses its finish reason and model metadata. Key the seen state by completion index as well as field.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I don't think this holds for this SDK. The streaming path collapses all choices into a single message: attemptInvoke returns { messages: [finalChunk] } (src/llm/invoke.ts) and core's _streamIterator builds one generationChunk for handleLLMEnd — nothing in invoke.ts / Graph.ts / stream.ts aggregates by generationInfo.completion (it's read only to pass handleLLMNewToken token indices, never to split messages), and n>1 isn't wired. So there is no 'second generated message' to lose metadata.
Keying seen by completion index would actually reintroduce the bug: with everything concatenated into one message, letting choice 1's finish_reason/model_name survive alongside choice 0's is exactly what produces stopstop again. At n=1 (the only reachable case) completion index is always 0, so it's a no-op. Happy to add per-index scoping if/when the SDK grows real multi-choice aggregation, but today it would regress the single collapsed message.
| lastYieldedAt = Date.now(); | ||
| await emitStreamChunkCallback(outputChunk, runManager); | ||
| signal?.throwIfAborted(); | ||
| dropRepeatedScalarMetadata(outputChunk, seenScalarMetadata); |
There was a problem hiding this comment.
This runs after emitStreamChunkCallback, so handleLLMNewToken callbacks receive the original chunk with repeated scalar metadata even though the yielded chunk is later cleaned. In environments that build traces or streamed state from token callbacks, repeated finish chunks can still surface duplicated values such as stopstop; move the cleanup before emitting the callback so all consumers observe the same de-duplicated chunk.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 448f61f. Moved dropRepeatedScalarMetadata ahead of emitStreamChunkCallback so token callbacks, the yielded chunk, and both downstream aggregations all observe the same cleaned metadata. Keep-first means the first finish chunk retains its scalars and prompt/completion token indices are untouched. Regression now asserts handleLLMNewToken sees finish_reason exactly once.
Move dropRepeatedScalarMetadata ahead of emitStreamChunkCallback so token callbacks observe the same cleaned chunk as the yielded/aggregated one. Keep-first means the first finish chunk retains its metadata; prompt/completion token indices are untouched. Adds a token-callback assertion to the regression.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 448f61f2cd
ℹ️ 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".
| runManager?: CallbackManagerForLLMRun | ||
| ): AsyncGenerator<ChatGenerationChunk> { | ||
| let lastYieldedAt: number | undefined; | ||
| const seenScalarMetadata = new Set<string>(); |
There was a problem hiding this comment.
When a chat-completions request streams multiple choices (n > 1), this Set is shared across the entire stream even though chunks carry their choice in generationInfo.completion and _generate aggregates each completion separately. After choice 0 emits finish_reason/model_name, the finish metadata for choice 1+ is deleted before yielding, so later generated choices and token callbacks lose their stop reason/model metadata; track seen fields per completion index instead of globally.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Accepted in 3d455f6 — you're right that this belongs keyed by completion index. My earlier pushback was scoped to the SDK's .stream() path (which collapses all choices into one message), but the helper is on the general-purpose class and _generate aggregates per choice, so seen is now a Map<completionIndex, Set<field>> keyed off generationInfo.completion. No-op at n=1 (the only in-SDK-reachable case); correct for n>1.
| lastYieldedAt = Date.now(); | ||
| // Dedupe before emitting so token callbacks and the yielded chunk | ||
| // (and both downstream aggregations) observe the same cleaned metadata. | ||
| dropRepeatedScalarMetadata(outputChunk, seenScalarMetadata); |
There was a problem hiding this comment.
For ChatOpenRouter, this de-dupe runs inside super._streamResponseChunks() before the subclass sees the chunk, but the OpenRouter override uses generationInfo.finish_reason as the signal to flush accumulated reasoning_details. In the repeated-finish OpenRouter streams this patch targets, any later finish chunk that also carries reasoning/encrypted details now reaches that override with finish_reason deleted and goes through the non-final branch, so those final details are deleted instead of attached; keep an internal finish marker or run this cleanup after subclass finalization.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in 3d455f6. The de-dup no longer runs in the shared super path. delayStreamChunks de-duplicates only when handed a seen-map (the terminal emit); ChatOpenAI exposes a raw _streamRawResponseChunks variant, and ChatOpenRouter now consumes that so it reads the un-stripped finish_reason for its reasoning flush, then de-duplicates before its own emitStreamChunkCallback. Live-verified on deepseek/deepseek-r1 via OpenRouter: reasoning details are still attached and finish_reason/model_name are de-duplicated in both aggregations.
| continue; | ||
| } | ||
| if (inGenerationInfo) { | ||
| delete generationInfo[field]; |
There was a problem hiding this comment.
When ChatDeepSeek splits one raw final chunk into several synthetic chunks (for example a short <think>...</think>answer fallback-tag response), _createDeepSeekStreamChunk reuses the original generationInfo/response_metadata objects for each piece. Deleting from the second piece here mutates the same object still referenced by the first already-emitted piece, so consumers that keep chunk references can see the only finish_reason/model_name disappear after emission; clone the metadata object before removing repeated fields.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 3d455f6. dropRepeatedScalarMetadata now computes the fields to drop, then deletes from a shallow clone of generationInfo/response_metadata and reassigns it on the repeat chunk — so a ChatDeepSeek split that shares those objects across pieces no longer loses metadata from an already-emitted sibling. Added a unit test asserting the shared object and the first piece keep finish_reason while the repeat gets a cleaned copy.
…subclass finish signal Addresses Codex P2/P3 findings on the scalar-metadata de-dup: - Key `seen` by completion index so multi-choice (n>1) responses, which `_generate` aggregates per choice, keep each choice's finish metadata. - Clone generationInfo/response_metadata before deleting, so providers that share those objects across split chunks (ChatDeepSeek) don't lose metadata from an already-emitted sibling. - De-duplicate at each terminal emit rather than in the shared super path: delayStreamChunks dedupes only when given a seen-map; ChatOpenAI exposes a raw variant, and ChatOpenRouter consumes it so its finish_reason-driven reasoning flush runs before de-dup. Verified live on openai/gpt-chat-latest and deepseek/deepseek-r1 (reasoning details preserved, metadata de-duped).
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? 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
Summary
Fixes duplicated streamed
response_metadata—finish_reason: "stopstop"and doubledmodel_name— surfaced downstream in LibreChat Langfuse traces (danny-avila/LibreChat#14086).Root cause
LangChain core's
_mergeDictsconcatenates repeatedresponse_metadatastring values (merged[key] += value). Its keep-last allowlist is onlyid/name/output_version/model_provider, somodel_name/finish_reason/service_tier/system_fingerprintget concatenated when they appear on more than one chunk.@langchain/openaistamps that scalar group onto every chunk whosechoice.finish_reasonis non-null. A conformant OpenAI stream carriesfinish_reasonexactly once, so this never triggers there. Providers that repeat it (verified live across every OpenRouter route:openai/gpt-chat-latest,openai/gpt-4o-mini,deepseek/deepseek-chat) make the merge double the values. Direct OpenAI does not reproduce.The corruption is produced by two independent aggregations, both via
AIMessageChunk.concat:_streamIteratorgenerationChunk.concat→handleLLMEnd→ Langfuse GENERATION observation modelconcatof yielded chunks → graph message → trace outputSo it must be fixed at the chunk source, before either runs.
Fix
Fold a keep-first dedupe into the shared module-local
delayStreamChunkswrapper (src/llm/openai/index.ts), which every OpenAI-family class already funnels through. It drops repeatedmodel_name/finish_reason/service_tier/system_fingerprintafter the first occurrence, before yielding to core, so both accumulators see clean chunks. A field that only appears on a later chunk (e.g.service_tier) still survives, and a conformant single-finish_reasonstream is untouched. CoversChatOpenAI/AzureChatOpenAI/ChatDeepSeek/ChatXAI, andChatOpenRouter/ChatMoonshotvia inheritance. Mirrors the existing BedrockenrichChunkprecedent (strips fields "to prevent_mergeDictsconflicts").Testing
src/llm/openai/streamMetadataDedup.spec.tsdrives the real.stream()path (only the HTTP boundary mocked) and asserts bothhandleLLMEndand a host-sideconcatare de-duplicated; plus a "conformant single-finish stream untouched" case.openai/gpt-chat-latest:stopstop→stop, doubled model → single, in both accumulators; direct OpenAI unchanged.src/llm/openaisuite green (139 tests).