fix: thinking-box polish — six small correctness fixes#19
Merged
Conversation
ThinkingBoxControl was always created with its default expand_key
("c-t"), so a custom AppInfo.expand_key changed the actual binding
but the truncation hint still read "ctrl-t to expand". The manager
now takes expand_key and forwards it to every box it creates.
StreamingContent.set_line(-5, ...) on shorter content fell through normalization still negative, skipped the extend loop, and failed on the list assignment with an error naming the normalized index. It now raises immediately with the index the caller passed, and leaves the content untouched.
finish_all() now reports each box's max_collapsed_lines, and finish_thinking() uses it instead of the session-wide max_thinking_height — matching the per-box finish path used by ThinkingContext.finish(). Boxes created with a custom max_lines were previously truncated to the wrong limit on the deprecated path.
Display.clear() raw-printed \033[2J\033[H to stdout, bypassing prompt_toolkit's renderer. With the app running, the renderer's idea of what is on screen goes stale and the next repaint draws against the wrong baseline. Screen clearing now lives in ThinkingPromptSession.clear(): app.renderer.clear() when running, escape-code fallback when not. Display.clear() handles only its own state (history buffer + pending output).
get_line_count measured raw len(line), so SGR escape sequences in ansi-format content inflated the wrapping estimate and the collapsed box was sized taller than its visible content. Escape sequences are now stripped before width math.
The header line was rendered at a hardcoded 80 columns, so it never spanned wider terminals and overflowed narrower ones. The width now comes from the running app's output size, falling back to 80 outside a running app.
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
1 participant
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.
Six independent fixes, one commit each:
ThinkingBoxControlwas always created with its default (c-t), so a customAppInfo.expand_keychanged the binding but the hint still readctrl-t to expand. The manager now forwardsexpand_keyto every box.StreamingContent.set_line()negative-index bounds — an out-of-range negative index now raises a clearIndexErrornaming the caller's index, leaving content untouched.finish_thinking()—finish_all()reports each box'smax_collapsed_lines(5th tuple element) andfinish_thinking()uses it instead of the session-widemax_thinking_height, matchingThinkingContext.finish().clear()— screen clearing goes throughapp.renderer.clear()while running (raw\033[2Jwrites desynced the renderer); escape-code fallback when not running.Display.clear()now handles only its own state.get_line_count()strips SGR escapes before width math, so styled ansi-format content no longer renders an oversized box.Each fix has a regression test. Identified in docs/notes/project_review_20260612.md (findings 5–7).