refactor: handle complex streaming/scrolling with better rendering#8214
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e3e22e0a6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const headerH = 2; | ||
| const inputBarH = showInputBar ? (queuedMessages.length > 0 ? 4 : 3) : 0; | ||
| const historyBarH = isViewingHistory ? 2 : 0; | ||
| const viewportHeight = Math.max( | ||
| termHeight - PAD_Y * 2 - headerH - inputBarH - historyBarH, | ||
| 3, |
There was a problem hiding this comment.
Compute viewport height from measured layout
The new fixed headerH/inputBarH/historyBarH math assumes those sections are always 2/3/4 lines, but several of those rows can wrap (e.g., long status text such as failed: ..., queued-message text on narrow terminals, or turn counters). When wrapping happens, viewportHeight is overestimated, so total rendered rows exceed the terminal height and bottom UI (input/history/footer) gets clipped or overwritten. The previous flexGrow/measured approach adapted to actual rendered height, so this is a regression in constrained widths or long-status scenarios.
Useful? React with 👍 / 👎.
DOsinga
left a comment
There was a problem hiding this comment.
Solid refactor — net code reduction, fixes streaming/scrolling bugs by moving to explicit line-by-line rendering, removes dead code. The Codex comment about fixed header/input bar heights is a valid but low-priority edge case for very narrow terminals; the whole point of this PR is to move away from measureElement which was causing the bugs in the first place. LGTM.
* origin/main: (85 commits) docs: standardize on ~/.agents/skills/ as canonical skill path (#8239) feat: bump versions (already published) (#8240) Revert "fix: rename bin for the tui (#8231)" (#8238) docs: update reusable recipes docs (#8232) docs: add GOOSE_SHELL env var (#8233) feat(security): add egress logging inspector (#8149) blog: Adversary Mode — A Second Pair of Eyes on Every Tool Call (#8220) fix: rename bin for the tui (#8231) feat: goose serve (#8209) refactor: make --text mode independent of the normal TUI (#8210) refactor: handle complex streaming/scrolling with better rendering (#8214) Add Inference Mesh settings tab (#8094) only run windows compile on main branch (#8216) used simplified privacy info modal and removed unnecessary components (#8200) removed unused welcome route (#8196) fix: use build_host_url for GCP Vertex AI location fallback (#8185) fix: remove ui/acp/.npmrc to allow auth token inheritance fix(openai): use safely_parse_json for streaming tool arguments (#8208) feat: add copilot-acp provider (#8154) chore(deps): bump path-to-regexp from 8.3.0 to 8.4.0 in /evals/open-model-gym/mcp-harness (#8178) ... # Conflicts: # ui/desktop/.gitignore # ui/desktop/src/components/Layout/CondensedRenderer.tsx # ui/desktop/src/components/Layout/navigation/ChatSessionsDropdown.tsx # ui/desktop/src/components/Layout/navigation/SessionsList.tsx # ui/desktop/src/components/ParameterInputModal.tsx # ui/desktop/src/components/parameter/ParameterInput.tsx # ui/desktop/src/components/recipes/RecipeActivityEditor.tsx # ui/desktop/src/components/recipes/shared/CreateSubRecipeInline.tsx # ui/desktop/src/main.ts
Fixes a number of issues with scrolling when streaming is also in play. Before we didn't track height/lines very well so tool call box rendering and scrolling were very buggy with text overlaps etc. Now it does a much better job.