fix(cli): show_reasoning works independently of streaming setting (#34209) - #34355
Closed
Bartok9 wants to merge 1 commit into
Closed
fix(cli): show_reasoning works independently of streaming setting (#34209)#34355Bartok9 wants to merge 1 commit into
Bartok9 wants to merge 1 commit into
Conversation
…usResearch#34209) In CLI mode, `show_reasoning=true` only displayed reasoning when `streaming=true`. Users who prefer non-streaming output (`streaming: false`) could not see reasoning content even with `show_reasoning: true` explicitly set in ~/.hermes/config.yaml. Root cause: cli.py::_current_reasoning_callback returned None for the combination (show_reasoning=True, streaming_enabled=False), so the agent had no reasoning sink and reasoning tokens were silently dropped before reaching any render path. The CLI already had a buffered-preview render path (_on_reasoning + _flush_reasoning_preview) used today by the (verbose=True, show_reasoning=False) combination \u2014 that path emits compact `[thinking] ...` blocks on natural boundaries. Reusing it for the non-streaming show_reasoning case is the smallest change that restores the expected behavior without changing anything else about the user's CLI output preferences. Coverage matrix after this fix: show_reasoning streaming verbose reasoning render -------------- --------- ------- --------------------------- True True any live token-by-token box True False any buffered [thinking] preview \u2190 NousResearch#34209 False any True minimal [thinking] preview False any False no reasoning render Updated test name + assertion: test_show_reasoning_non_streaming_uses_final_box_only \u2192 test_show_reasoning_non_streaming_uses_buffered_preview (the old assertion enshrined the bug; new assertion locks in the fix) Tests (3 new + 1 updated in TestReasoningDisplayModeSelection): - test_show_reasoning_non_streaming_uses_buffered_preview \u2014 NousResearch#34209 regression: confirms the buffered preview callback is returned for (show_reasoning=True, streaming=False). - test_no_callback_when_all_options_off \u2014 default state guard. - test_no_callback_when_streaming_only \u2014 streaming on but no reasoning flag = no callback (existing behavior pinned). - test_show_reasoning_takes_precedence_over_verbose \u2014 when both show_reasoning and verbose are True with streaming, show_reasoning wins (live-stream callback, not preview). All 58 tests in test_reasoning_command.py pass. Refs: NousResearch#34209 Closes: NousResearch#34209 Co-authored-by: Cursor <cursoragent@cursor.com>
Collaborator
Contributor
Author
|
Closing as duplicate of #34224 — thanks @alt-glitch and the reviewer pipeline for flagging the dup pattern (same as #34376 \u2192 #34241 earlier tonight). @LeonSGP43's PR #34224 (opened ~54 min before mine) makes the same routing fix to #34224 is the canonical fix; closing this one. \u2014 Bartok9 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #34209
Problem
In CLI mode,
show_reasoning=trueonly displayed reasoning whenstreaming=true. Users who prefer non-streaming output (streaming: false) couldn't see reasoning content even withshow_reasoning: trueexplicitly set.The agent had no reasoning sink and reasoning tokens were silently dropped.
Fix
The CLI already has a buffered-preview render path (
_on_reasoning+_flush_reasoning_preview) used today forverbose=True, show_reasoning=False— emits compact[thinking]blocks on natural boundaries.Reusing it for the non-streaming show_reasoning case restores expected behavior without changing the user's CLI output preferences.
Coverage matrix after fix
[thinking]preview ← #34209[thinking]previewTests
test_show_reasoning_non_streaming_uses_final_box_only→...uses_buffered_preview. The old assertion enshrined the bug (assertIsNone); the new one locks in the fix.test_no_callback_when_all_options_off,test_no_callback_when_streaming_only,test_show_reasoning_takes_precedence_over_verbose.$ python -m pytest tests/cli/test_reasoning_command.py === 58 passed in 1.11s ===🎻 Co-authored-by: Cursor cursoragent@cursor.com