-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix: preserve whitespace in thinking content for stream-json output format #1365
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📋 Review SummaryThis PR fixes an important whitespace preservation issue in thinking content for stream-json output format. The problem was caused by unnecessary trim() calls that removed leading/trailing spaces when thinking content was split across multiple stream events. The solution correctly removes the trim() calls while maintaining the filtering of empty values, and includes comprehensive tests to verify the fix. 🔍 General Feedback
🟡 High
🟢 Medium
✅ Highlights
|
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
|
This pr has fixed #1356 for me locally. |
TLDR
This PR fixes missing whitespace in thinking content when using
stream-jsonorjsonoutput formats. The issue was caused bytrim()calls on streaming fragments, which removed leading/trailing spaces when thinking content was split across multiple stream events.Dive Deeper
Problem
In issue #1356, users reported that thinking content in
stream-jsonoutput had all whitespace removed:Expected (text format):
Actual (stream-json format):
{"thinking":"Theuserjustsaid\"Hello\".Thisisasimplegreeting,..."}Root Cause
The
appendThinkingmethod inBaseJsonOutputAdapter.tswas calling.trim()on bothsubjectanddescriptionparameters before joining them:When thinking content was streamed in multiple fragments like:
"The user just"" said \"Hello\""The trim() removed the leading space from Fragment 2, resulting in
"The user justsaid \"Hello\"".Solution
Modified the logic to build fragments without trimming:
This preserves all whitespace in the original content while still filtering out empty values.
Reviewer Test Plan
Unit Tests
Added comprehensive test cases to verify whitespace preservation:
"The user just"and" said \"Hello\""Integration Test
Test the fix with the exact scenario from issue #1356:
npx @qwen-code/qwen-code@latest -p "Hello" --output-format stream-jsonVerify the thinking content shows:
{"thinking":"The user just said \"Hello\". This is a simple greeting,..."}Instead of:
{"thinking":"Theuserjustsaid\"Hello\".Thisisasimplegreeting,..."}Testing Matrix
Linked issues / bugs
Fixes #1356