Repository navigation
fix(tui): preserve hanging prefixes in tool cards - #15679
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
The review-summary comment only says the review is running; there is no finding or requested change in the PR thread yet. Which specific change should I make? |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6428084c10
ℹ️ 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 optLabel = renderInlineMarkdown(opt.label, mdTheme, t => uiTheme.fg("muted", t)); | ||
| out.push(` ${uiTheme.fg("dim", optionMarker(uiTheme, multi, false))} ${optLabel}`); | ||
| const marker = ` ${uiTheme.fg("dim", optionMarker(uiTheme, multi, false))} `; | ||
| out.push(...wrapPrefixedLine(marker, " ".repeat(visibleWidth(marker)), optLabel, width)); |
There was a problem hiding this comment.
Preserve option prefixes after Ask completes
When an Ask option has a long label, this wraps the pending call beneath its marker, but the completed card switches to renderAnswerOptionLines, which still emits a single prefixed string for renderOutputBlock to wrap generically. Consequently, submitting the answer makes continuation rows jump back to the card's left edge; apply the same prefixed wrapping to the result path as well.
AGENTS.md reference: AGENTS.md:L241-L246
Useful? React with 👍 / 👎.
|
|
||
| const rail = ` ${theme.fg("mdCodeBlockBorder", theme.boxRound.vertical)} `; | ||
| const contentLines = | ||
| codeBlockMatch && expanded |
There was a problem hiding this comment.
Apply rail wrapping to collapsed LSP hovers
For an LSP hover whose first code line exceeds the card width, collapsing the card bypasses this branch even though renderHover still emits the same code rail in collapsed mode. renderOutputBlock then wraps the whole row generically, so the continuation starts beside the frame rather than beneath the rail; toggling from expanded to collapsed therefore reintroduces the alignment defect.
AGENTS.md reference: AGENTS.md:L241-L246
Useful? React with 👍 / 👎.
|
- Wrapped answered Ask labels at the card content width for single and grouped questions. - Applied hover code-rail wrapping to collapsed as well as expanded LSP cards. Fixes #15677
a055431 to
2b0b800
Compare
There was a problem hiding this comment.
💡 Codex Review
oh-my-pi/packages/tui/src/tools/edit.ts
Line 1147 in 2b0b800
When a streaming Edit preview contains several long diff lines, sliceStreamingDiffTail budgets them using wrapTextWithAnsi(rawLine, innerWidth), but this new render step wraps each highlighted line at innerWidth - gutterWidth so every continuation can consume more rows than the selector counted. For example, lines around two card-widths can be counted as two rows and rendered as three, allowing the live card to exceed previewWindowRows() and reintroducing the stale/duplicated transcript behavior that the tail cap is meant to prevent; use the same gutter-aware row calculation during selection or re-cap the resulting wrapped rows.
ℹ️ 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".
Repro
At width 60, Write preview rows with long English/CJK text resume beside the frame border rather than beneath the line number; a grouped Read URL starts after a bare
├─on the next row. Reproduced before the fix withbun run /tmp/omp-15677-repro.ts(the issue's renderer script using repository imports); the committed regression can be run withbun test packages/tui/test/tool-hanging-wrap.test.ts.Cause
renderOutputBlockandText.renderappliedwrapTextWithAnsito flattened rows afterwriteToolRenderer,ReadToolGroupComponent.#appendSummaryRow, and other renderers joined their structural prefix with content. The generic wrapper had no continuation prefix to retain.Fix
wrapPrefixedLineandwrapTreeLineto hang ANSI-aware content beneath line gutters and tree branches.Verification
bun test packages/tui/test/tool-hanging-wrap.test.ts packages/tui/test/output-block.test.ts packages/tui/test/transcript-committed-release.test.ts packages/tui/test/tools-native-search.test.ts: 45 pass;bun run check:typesinpackages/tui: pass; live width-60 renderer smoke: Write continuations and grouped Read URL hang under their prefixes.gh_push_branchpassedbun run fixandbun check. Skipped PR pre-publish gate:bun run testfails on pre-existingorigin/mainfailures, verified with the same test paths on a cleanorigin/mainsnapshot: outdated native addon 18.8.8 vs 18.10.4 (browser-relay-daemon.test.tsanddebug-server.test.ts), twogrep-internal-urls.test.tsassertions, and oneusage-cli.test.tsassertion;bun run build:nativecannot rebuild here becausecmakeis unavailable. The changed TUI package suite passed 3598 tests with only the same default-branchdebug-server.test.tsnative failure. The ignored HTML tool-view asset was generated before the full-suite attempt. Fixes #15677