fix(tui): degrade daxnuts image to 256-color on non-truecolor terminals#6362
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR documents a comprehensive audit of the TUI system's structural and correctness issues, then implements six Phase 0 "stop-the-bleeding" fixes: clamping selection indices to prevent out-of-bounds navigation, preventing negative-slice text corruption, using Unicode code points for string matching, emitting terminal-compatible color escapes, refactoring markdown list rendering to use structural metadata, and disabling render-caching short-circuits when overlays were present in the prior frame. ChangesTUI Audit and Phase 0 Stop-the-Bleeding Fixes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🟡 PR Risk Report — MEDIUM
Affected Systems
File Breakdown
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/tui-audit.md`:
- Around line 158-159: Update the Phase 0 doc entry B4 to reflect the actual
remediation for daxnuts.ts: instead of instructing to replace RGB escapes with
theme.fg() tokens, state that daxnuts should perform RGB quantization/fallback
(emit truecolor when the terminal supports it, otherwise map to the best
256-color approximation) and preserve the existing quantization logic; reference
the file/name "daxnuts.ts" and the term "RGB escapes" in the doc so future
changes implement truecolor-first with 256-color fallback rather than replacing
colors with theme.fg().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 506057f7-6013-4a0c-b505-24487d9b4b20
📒 Files selected for processing (7)
docs/tui-audit.mdpackages/pi-coding-agent/src/modes/interactive/components/daxnuts.tspackages/pi-tui/src/components/input.tspackages/pi-tui/src/components/markdown.tspackages/pi-tui/src/components/select-list.tspackages/pi-tui/src/fuzzy.tspackages/pi-tui/src/tui.ts
| 4. **B4** `daxnuts.ts` — replace RGB escapes with `theme.fg()` tokens. | ||
| 5. **B5** `user-message.ts` — gate OSC 133 emission behind a terminal-capability |
There was a problem hiding this comment.
Phase 0 item B4 currently recommends the wrong remediation.
This line says to replace RGB escapes with theme.fg() tokens, but the current fix strategy for daxnuts is RGB quantization/fallback (truecolor when available, 256-color otherwise). Please update this item to match that behavior so future work doesn’t regress image rendering.
Suggested doc patch
-4. **B4** `daxnuts.ts` — replace RGB escapes with `theme.fg()` tokens.
+4. **B4** `daxnuts.ts` — keep RGB source colors, but emit 256-color SGR via
+ quantization when terminal color mode is not truecolor.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 4. **B4** `daxnuts.ts` — replace RGB escapes with `theme.fg()` tokens. | |
| 5. **B5** `user-message.ts` — gate OSC 133 emission behind a terminal-capability | |
| 4. **B4** `daxnuts.ts` — keep RGB source colors, but emit 256-color SGR via | |
| quantization when terminal color mode is not truecolor. | |
| 5. **B5** `user-message.ts` — gate OSC 133 emission behind a terminal-capability |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/tui-audit.md` around lines 158 - 159, Update the Phase 0 doc entry B4 to
reflect the actual remediation for daxnuts.ts: instead of instructing to replace
RGB escapes with theme.fg() tokens, state that daxnuts should perform RGB
quantization/fallback (emit truecolor when the terminal supports it, otherwise
map to the best 256-color approximation) and preserve the existing quantization
logic; reference the file/name "daxnuts.ts" and the term "RGB escapes" in the
doc so future changes implement truecolor-first with 256-color fallback rather
than replacing colors with theme.fg().
The doRender() short-circuit skipped all post-processing when component output was byte-identical to the previous frame. If the previous frame composited an overlay onto the screen, an identical next frame with no overlay would short-circuit and never erase it, leaving the overlay stuck on screen. Track _lastFrameHadOverlays and force a redraw when the previous frame drew an overlay, even if base component output is unchanged.
When a filter matched no items, selectUp set selectedIndex to filteredItems.length - 1 (i.e. -1), corrupting list state until the next setFilter() reset it. Skip navigation and selection entirely when the filtered list is empty; still honor cancel.
renderList() identified nested-list lines with the regex /^\s+\x1b\[36m[-\d]/ — assuming the list bullet is always ANSI cyan. Any theme with a different bullet color silently broke nested-list indentation. renderListItem() now returns ListItemLine[] with a 'nested' flag set on lines produced by recursive renderList() calls, so renderList() keys off structure instead of pattern-matching theme output.
yankPop() computed the deletion start as cursor - prevText.length with no floor. A negative index passed to String.slice() counts from the end of the string, so an underflow would silently corrupt the input value rather than delete nothing. Clamp the start index to 0.
fuzzyMatch() indexed strings with [i], which yields UTF-16 code units. Astral-plane characters (emoji, rare CJK) occupy two code units, so a single such character was compared as two lone surrogates and never matched. Iterate over Array.from() code-point arrays instead.
The daxnuts easter-egg image emitted 24-bit truecolor SGR sequences (\x1b[38;2;r;g;bm) unconditionally. On 256/16-color terminals (many containers, remote sessions, Apple Terminal) those render incorrectly. Add an rgbTo256() quantizer; rgb() now emits 256-color SGR when theme.getColorMode() reports the terminal is not truecolor.
jeremymcs
force-pushed
the
tui-p0/06-daxnuts-256color
branch
from
a253e93 to
0b52798
Compare
1 participant
Stack — TUI Phase 0 (7/8)
Builds on #6361. Stacked branch — diff also includes commits below it until they merge.
Problem
The daxnuts easter-egg image emitted 24-bit truecolor SGR sequences (
\x1b[38;2;r;g;bm) unconditionally. On 256/16-color terminals (many containers, remote sessions, Apple Terminal) those render incorrectly.Fix
Add an
rgbTo256()quantizer.rgb()now emits 256-color SGR whentheme.getColorMode()reports the terminal is not truecolor. (Theme tokens cannot represent an arbitrary 32×32 photo — graceful colour-depth degradation is the correct fix.)Summary by CodeRabbit
Bug Fixes
Documentation