Skip to content

fix(llm): emit role once in chat streams - #12639

Merged
rmccorm4 merged 5 commits into
mainfrom
rmccormick/dis-2622-deduplicate-stream-roles
Aug 5, 2026
Merged

fix(llm): emit role once in chat streams#12639
rmccorm4 merged 5 commits into
mainfrom
rmccormick/dis-2622-deduplicate-stream-roles

Conversation

@rmccorm4

@rmccorm4 rmccorm4 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Advance the chat delta generator's message counter so it emits delta.role only on the first generated chunk.
  • Deduplicate roles per choice after tool and reasoning parser transformations, which can synthesize new role-bearing chunks.
  • Preserve the initial role-only chunk instead of filtering it as empty.

Both layers are required: fixing only the base generator does not cover parser-generated chunks, while boundary-only normalization would hide the generator defect.

Validation

  • cargo fmt --all -- --check
  • cargo test -p dynamo-llm --lib test_role_is_emitted_only_in_first_delta
  • cargo test -p dynamo-llm --lib test_deduplicate_stream_roles_preserves_only_first_role_per_choice

Tracking

Internal: DIS-2622

Summary by CodeRabbit

  • Bug Fixes
    • Improved chat response streaming so assistant role information is preserved when received in a standalone update.
    • Prevented repeated role information from appearing in subsequent streamed chunks.
    • Improved handling of multiple choices so each choice emits its role correctly and independently.
    • Updated streaming behavior to avoid incorrectly discarding meaningful role-only updates.

Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
@github-actions github-actions Bot added fix frontend `python -m dynamo.frontend` and `dynamo-run in=http|text|grpc` labels Aug 4, 2026
@datadog-official

This comment has been minimized.

@rmccorm4
rmccorm4 marked this pull request as ready for review August 4, 2026 18:58
@rmccorm4
rmccorm4 requested a review from a team as a code owner August 4, 2026 18:58
devin-ai-integration[bot]

This comment was marked as resolved.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Chat streaming now treats role-only deltas as meaningful. It preserves the first role per choice, removes repeated roles before empty-chunk filtering, and updates delta-generation and stream-processing tests.

Changes

Chat role streaming

Layer / File(s) Summary
First-delta role generation
lib/llm/src/protocols/openai/chat_completions/delta.rs
DeltaGenerator::create_choice increments its message counter after creating a choice. Tests verify that only the first streaming choice includes the assistant role.
Chat SSE role deduplication
lib/llm/src/http/service/openai.rs
Chat stream emptiness checks include roles. The handler tracks emitted roles per choice and removes repeated roles before SSE conversion. Tests retain role-only chunks and verify per-choice deduplication.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary change: emitting the chat stream role only once.
Description check ✅ Passed The description explains the changes, validation commands, and tracking issue, with sufficient detail for review despite different section headings.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

coderabbitai[bot]

This comment was marked as resolved.

Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
@pull-request-size pull-request-size Bot added size/L and removed size/M labels Aug 4, 2026
devin-ai-integration[bot]

This comment was marked as resolved.

Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
devin-ai-integration[bot]

This comment was marked as resolved.

Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
Comment thread lib/llm/src/preprocessor.rs Outdated

@indrajit96 indrajit96 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
@rmccorm4
rmccorm4 enabled auto-merge (squash) August 5, 2026 00:28
@rmccorm4
rmccorm4 merged commit 1a83d24 into main Aug 5, 2026
114 of 115 checks passed
@rmccorm4
rmccorm4 deleted the rmccormick/dis-2622-deduplicate-stream-roles branch August 5, 2026 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix frontend `python -m dynamo.frontend` and `dynamo-run in=http|text|grpc` size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants