Skip to content

fix(openai): preserve reasoning in mixed content deltas - #526

Merged
hvagadia merged 2 commits into
mlcommons:mainfrom
hvagadia:fix/openai-mixed-reasoning-content
Oct 5, 2026
Merged

hvagadia merged 2 commits into
mlcommons:mainfrom
hvagadia:fix/openai-mixed-reasoning-content

Conversation

@hvagadia

@hvagadia hvagadia commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator

When an OpenAI streaming delta contains both content and reasoning, the accumulator's if/elif drops the reasoning. Accumulate both fields independently and preserve both in emitted chunks.

Adds regression coverage for reasoning_content and reasoning, with first-chunk-only and full streaming. All 60 OpenAI unit tests and all pre-commit checks pass.

@hvagadia
hvagadia requested a review from a team October 5, 2026 16:22
@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

@github-actions github-actions Bot added the size/normal PR Review Policy: <=500 non-test lines & <=20 files label Oct 5, 2026
@codecov-commenter

codecov-commenter commented Oct 5, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@f1100cf). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #526   +/-   ##
=======================================
  Coverage        ?   81.35%           
=======================================
  Files           ?      157           
  Lines           ?    22494           
  Branches        ?        0           
=======================================
  Hits            ?    18301           
  Misses          ?     4193           
  Partials        ?        0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@leopck leopck left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

valid issue, I've manually checked on vLLM that this is how it is working and this is truly missing the contents

@arekay-nv arekay-nv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM.
Followup PR with more coverage.

@hvagadia

hvagadia commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

cc: @tianmu-li might have slight perf impact, please check your runs.

@hvagadia
hvagadia merged commit 8ec2d58 into mlcommons:main Oct 5, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/normal PR Review Policy: <=500 non-test lines & <=20 files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants