TUI rendering (zeroline): glamour markdown, syntax highlighting, themed surfaces

[gnanam1990]

Module of N — TUI rendering (zeroline)

Reviewable slice of the runtime-core split (off main). Scope: internal/zeroline only (6 files, ~1,988 lines incl. tests). Pure presentation layer — no internal deps; a clean new package (no drift).

Naming: the reskin's working name was zenline; this PR lands it as zeroline — dropping the zenline name so it reads as ZERO's own default polished terminal surface.

What's in it

• glamour-rendered markdown with a bounded mdCache so we don't re-render every frame, plus chroma syntax highlighting (transitive via glamour).
• Colored diffs, full-bleed background, and two surfaces — a Zen home page and a vim/powerline-style statusline chat page — sharing 5 switchable color themes (theme.go).
• Pure presentation: callers (the TUI model) build the data structs from live agent state; zeroline turns them into styled terminal frames.

Forward code

No caller on main yet — the TUI model wires this in a later module; here it's exercised by render_test.go / markdown_test.go / render_overlay_test.go.

Dependencies (called out)

Adds github.com/charmbracelet/glamour v1.0.0, which pulls alecthomas/chroma/v2, yuin/goldmark(+emoji), microcosm-cc/bluemonday, golang.org/x/net, golang.org/x/term (and bumps golang.org/x/text) transitively. lipgloss/bubbletea/termenv were already present. Resolved via go mod tidy.

Testing

go build ./..., go vet ./..., go test ./..., go test -race ./internal/zeroline/ all green; no behavior change to existing packages.

Part of decomposing #101 (draft); subagents excluded.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Markdown rendering with syntax-highlighted fenced code in terminal transcripts.
  • Five selectable color themes for the terminal UI.
  • Full text-based TUI screens: boot, home, and chat views with overlays.
  • Permission prompts, questionnaires, and picker/suggestion overlays with deterministic layout.
  • Assistant output: completed messages rendered as styled markdown; streaming content shown verbatim.

• Tests

  • Expanded coverage for rendering, overlays, clipping, diff handling, and permission/modal behaviors.

[github-actions]

Zero automated PR review

Verdict: No blockers found

Blockers

• None found.

Validation

• [pass] Diff hygiene: git diff --check
• [pass] Tests: go test ./...
• [pass] Build: go run ./cmd/zero-release build
• [pass] Smoke build: go run ./cmd/zero-release smoke

Scope

Head: a59a61538f27
Changed files (8): go.mod, go.sum, internal/zeroline/markdown.go, internal/zeroline/markdown_test.go, internal/zeroline/render.go, internal/zeroline/render_overlay_test.go, internal/zeroline/render_test.go, internal/zeroline/theme.go

This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality.

[anandh8x]

What's good

• Bounded caches that prevent a resize drag from blowing out memory. mdOutCacheMax = 512 and mdCacheMax = 256 are constants with explicit comments: "a resize drag (many widths) can't grow it forever" and "a resize mints a width key per frame". TestMarkdownRendererCacheBounded exercises the cap by inserting mdCacheMax * 3 width keys and asserts the cache never exceeds the bound. The reset is a simple mdCache = map[mdKey]...{} ("cheap to repopulate") — that's the right call for a key-space that is at most variants * 2 * widths, well below the cap in practice.

• Cache key includes variant, dark, width so theme/mode/resize all get a fresh renderer. The comment is explicit: "A renderer bakes in the theme colors AND the word-wrap width, so any change to variant, mode or width needs a fresh one." TestMarkdownRendererCachedPerKey covers all three axes (identical key returns same pointer; width change gets a new one; variant change gets a new one). This is the right call — glamour's renderer bakes the colors and the wrap width in.

• Non-reentrant mutex handling around the cache. renderMarkdown releases mdMu before calling markdownRenderer (which takes the same mutex), with a comment that says sync.Mutex is not reentrant. The pattern is lookup → unlock → render → re-lock → insert so the slow glamour render runs without the lock held. The same pattern protects the output cache. Correct.

• Lockstep guarantee between PermLayout and permModalLines. permModalLines builds exactly permModalRows = 8 lines with the buttons on permBtnRow = 5, and panics on drift: panic("permModalLines: modal line count drifted from permModalRows"). The constants live next to each other and the comment names the relationship: "PermLayout places the button hitboxes assuming this exact shape, so keep the count and the buttons row in lockstep with the constants." PermLayout re-applies the same width/height clamps as RenderChat (floors to 40 wide and 8 tall, bodyH = height-3 floored to 1), so a sub-floor size still produces a hitbox that lands on the actual button row.

• Permission hitbox correctly disabled when buttons would be clipped. PermLayout returns Active: false when top + permBtnRow >= bodyH, with a comment: "the modal is clipped above the buttons — disable the hitboxes so a click can't resolve to allow/deny on a row where no button is drawn." TestPermLayoutDisablesHitboxesWhenButtonsClipped exercises this at height=8 (where bodyH=5 < top+5). Without this guard a click on a non-button row would silently resolve to allow/deny.

• Permission hitbox disabled at sub-floor sizes. TestPermLayoutMatchesRenderClamped runs PermLayout and RenderChat at sizes {10,11}, {20,12}, {30,14}, {38,24} — all below the 40-wide floor — and asserts three things: (1) the button row contains both "allow" and "deny" labels, (2) the rendered "[ a" bracket sits at exactly g.Allow.X + 2 columns, (3) no other row carries both labels. The comment names the bug this guards: "raw width shifts the box by a column at sub-40 widths and the click would miss."

• Sub-floor-height clamp is tested explicitly. TestPermLayoutMatchesRenderClamped then runs PermLayout(50, h) for h ∈ {1, 4, 7} and asserts g.Allow.Y < frameH. The comment names the threat: "the hitbox never points past the frame."

• Overlay is suppressed when a permission prompt is up. overlayRegion returns "" when d.Perm != nil (or maxRows <= 0), with a comment: "PermLayout (the mouse hit-test) lays the modal out assuming no overlay rows, so showing one here would drift the rendered buttons away from their hitboxes." TestPermLayoutMatchesRenderWithOverlayPresent confirms that an active picker + suggestions overlay is NOT rendered while a permission prompt is up, and the frame is still exactly h rows tall.

• Frame height is invariant under overlay cap. TestOverlayCappedKeepsFrameHeight (40 suggestions, frame height 16) and TestPickerOverlayCappedKeepsFrameHeight (50 items, frame height 14) both assert len(lines) == h AND a "… N more" summary row is present. capRows reserves a row for the summary when it has to truncate, so the visible count leaves room for it (visible + 1 <= maxRows). A non-positive budget shows nothing.

• Clip budgets by display width, not rune count. clip calls lipgloss.Width to measure and ansi.Truncate to clip, so wide runes (CJK = 2 cells, emoji = 2 cells) never exceed the budget. The comment names the threat: "a naive rune-count clip would let the line exceed its width budget." TestClipBudgetsByDisplayWidth covers 3 inputs (世 × 20, 🚀 × 15, mixed) × 4 widths (4/8/12/20) and asserts lipgloss.Width(got) <= w for every combination. Excellent coverage.

• Glamour output is clipped per line. renderAssistantMarkdown calls ansi.Truncate(ln, clipW, "…") on every line. The comment names the threat: "glamour word-wraps at wrapW but does NOT hard-break unbreakable tokens (a long URL or fenced-code line), so it can emit a line far wider than the frame. Clip each output line to the budget … with an ANSI-aware truncate so styled escapes stay intact." TestAssistantMarkdownClipsLongLines (300-char URL) and TestAssistantMarkdownClipsLongFencedCode (300-char x line) assert every emitted line fits the budget. A real bug — without the clip, a single long line would blow out the fixed-height/full-bleed layout.

• Tool result collapsing is the right default. resultSummary per-tool: read_file → "N lines" (never dumps content), list_directory → "N entries", glob → "N matches", grep → "0 matches" or "N matches" with body cap of 4 lines, edit_file/apply_patch → show body (diffs) with cap 8, bash → show body with cap 10, errors → cap 3 of the error body. TestToolResultRenderingCollapsesAndShows asserts each path: read_file shows "217 lines" and never the raw content; list_directory shows "3 entries"; edit_file shows the diff; errors show the failure text. This is the right summary-vs-body policy for a coding agent transcript.

• Diff colorization is bounded. diffMaxLines = 40 is the cap; long diffs surface a "… N more lines" footer. The cap protects the transcript from huge patches. TestColorizeDiffCapsLongDiffs writes 100 added lines and asserts the rendered output has a footer and lines <= diffMaxLines + 2. The colorizeDiff helper is exported with a clean (detail, p Pal) string signature for callers outside the package.

• Colorized diff uses theme Red/Green, not a literal color. diffLine switches on the leading marker (@@/+++ /--- /diff /index → mute, + → green, - → red, default → dim). TestColorizeDiffColorsAddsAndDels uses lipgloss.NewStyle().Foreground(p.Green).Render("+added line") and asserts the rendered output contains that exact string, so the test is theme-aware (if a theme's Green were the same as its Red the test pre-fails with a clear message). The theme is the source of truth for the colors.

• Theme structure is clean and small. Theme has Name, Swt, Dark, Light; Pal has 14 fields (Bg, Panel, Panel2, Fg, Dim, Mute, Line, Line2, Accent, Accent2, Green, Amber, Red, Sel). mkPal takes 14 hex strings and returns a Pal. Resolve clamps the variant into bounds. The 5 themes (Phosphor/Cyan/Sage/Violet/Mono) each have distinct dark/light palettes. TestThemesCount asserts len(Themes) == 5 and that each has a name and both Bg colors set; TestRenderHomeAllThemes renders the home page in each.

• Boot splash is a thoughtful micro-animation. RenderBoot reveals the ZERO wordmark line-by-line with explicit reveal = []int{1,3,5,7,9} ("per-line reveal frames ~120ms each"), then the tagline at frame 11, then the loading spinner at frame 8. The wordmark is rendered as 5 lines of fixed-width ASCII art. Pure presentation; no state.

• Statusline top bar / bottom bar have a clean three-section layout. topBar shows mode + branch + cwd + model + prov on the left and prov + ctx + cost on the right, with a bar() helper that fills the gap with a Panel-colored spacer. botBar shows mode + utf-8 + tok/s on the left and ctx-gauge + theme-name + theme-hint on the right. The gauge is a 8-cell ▕██████░░▏ bar.

• Run-state derivation is a clean switch. RenderChat derives run from Perm/AskUser (blocked) → Working (work) → hasAssistant(rows) (done) → normal. The mode is rendered as a colored pill (⟳ WORKING amber, ✓ DONE green, ⚠ BLOCKED red, NORMAL accent) in the top bar. Clear state machine.

• Ask-user takes the place of the thinking spinner. The transcript switch renders askUserLines(a, tw) when d.AskUser != nil and the comment says: "A pending questionnaire takes the place of the thinking/streaming line: show the focused question, not a misleading spinner." TestRenderChatAskUserShowsQuestionNotSpinner asserts the focused question appears AND the thinking line is suppressed. Mirrors the default skin's focused questionnaire so the zeroline surface doesn't show a misleading "working…".

• Streaming text is kept plain on purpose. renderAssistant routes to renderAssistantPlain when markdown == false, and the comment names the reason: "live streaming text (markdown=false) stays plain because partial markdown renders badly mid-stream." TestStreamingAssistantStaysPlain asserts the streaming text appears verbatim. Correct — glamour would render half a **bold badly.

• Streaming caret block is on the last streaming line. s.block() returns a 1-cell Background(Accent) " " and the transcript appends it to the last streaming line. Small but real UX detail that mirrors the default skin's streaming indicator.

• forceColor test helper is correctly scoped. TestColorizeDiffColorsAddsAndDels calls forceColor(t) to set lipgloss.SetColorProfile(termenv.TrueColor) for the test, with t.Cleanup restoring the previous profile. A subsequent test that depends on no-color output won't be affected. Clean pattern.

• stripANSI test helper handles the common case. It strips ESC (0x1b) ... m ANSI sequences. Doesn't handle cursor moves or other CSI, but the test assertions only check for colored text content, so it's sufficient. A future test that needs more could use a regex library.

• Diff detection is conservative. looksLikeDiff triggers on @@, +++ , --- , or any +/- line in the body. A +just an addition text would be misclassified as a diff, but the TestLooksLikeDiff table-driven test confirms the trade-off. The body-cap of 4 lines for grep, 8 for edits, 10 for bash means a misclassification only over-colorizes a few lines.

• Empty input is handled everywhere. renderMarkdown returns "" for r == nil (the comment handles it via the if cached, ok := mdOutput[key] path with empty value fallback to plain). codeLine returns "" for empty input. permModal returns empty for nil Perm (via the early return in RenderChat). No panics on empty data.

• Dependencies are called out explicitly. The body lists the transitive deps: chroma, goldmark(+emoji), bluemonday, golang.org/x/net, golang.org/x/term, plus a golang.org/x/text bump. lipgloss/bubbletea/termenv were already present. go mod tidy was run. The package is "pure presentation: callers build the data structs from live agent state; zeroline turns them into styled terminal frames" — no internal deps.

• The forward-code boundary is clean. No caller on main yet. The TUI model wires this in a later module; here it's exercised by render_test.go / markdown_test.go / render_overlay_test.go. That's the right slicing for the runtime-core split.

Observations (non-blocking)

1. Naive cache reset on overflow (mdCache = map[mdKey]...{}) drops ALL entries. A real LRU (e.g., a doubly-linked list keyed by recency) would preserve hot entries across overflow. The comment correctly says "simple bounded reset; cheap to repopulate" — and in practice the cache is bounded by variants * 2 * widths which is small. The simpler reset is the right call here.

2. mdOutCacheMax = 512 and mdCacheMax = 256 are package-level vars in markdown.go. If a future caller wanted a per-renderer budget (e.g., the TUI tightens it in a constrained mode), it would need a function parameter. The current global is fine; future refactor could pass them in.

3. renderMarkdown is a package-private function with a var cache. A future caller outside zeroline would call RenderChat and get the cache. The internal mdMu is shared across callers, which is correct for a memoized renderer.

4. The glamour version is pinned at v1.0.0 — a stable release. Bumping to a newer v1.x is a future concern. The transitive deps (chroma, goldmark, bluemonday) are all current majors.

5. newStyles and newCanvasStyles differ only in the background of the foreground styles. newStyles builds f(c) = NewStyle().Foreground(c); newCanvasStyles builds f(c) = NewStyle().Foreground(c).Background(p.Bg). The two paths exist because text inside status bars (topBar/botBar) sits on a Panel background and should NOT bake in a different background of its own, while text on the home / boot splash sits on the full-bleed Bg and must bake it in. The comment names the bug this guards: "otherwise the text shows the terminal's own background, producing a visible 'card' against the themed margins." Two functions is the right call.

6. PermLayout is exported and permModalLines is not. The TUI model needs the hitboxes for mouse handling; it doesn't need the raw modal lines. Correct API surface.

7. permBoxWidth clamps the modal width to [38, 52] and to w-2. The 38-floor is a minimum readable width; the 52-cap is the design max. The w-2 clamp prevents the modal from exceeding the frame. TestPermLayoutMatchesRenderClamped exercises all three clamps.

8. bar() fills the gap between left and right sections with a Panel-colored spacer. A simpler implementation could use lipgloss Width(w).Align(lipgloss.Right) but that doesn't render styled background. The current approach (build left, build right, build spacer, concat) is correct and explicit.

9. The wordmark is a 5-line ASCII art constant. A future enhancement could load it from a file or accept a custom one. The current constant is fine for a fixed brand identity.

10. spinFrames is a 10-element Braille spinner. A future enhancement could use a different spinner style per theme. The current set is sufficient.

11. hasAssistant is a small helper that scans rows for the first assistant row. A future enhancement could memoize the last-seen run-state to avoid the scan on every frame. The current scan is O(n) where n is the visible transcript, which is bounded by the frame height.

12. clip returns "" for w <= 0 and the input verbatim if it already fits. The function is a thin wrapper around ansi.Truncate for the overflow case, with a wide-rune-correctness test. TestClipLeavesShortStringsAndZeroWidth covers the boundary.

13. padRight measures lipgloss.Width of the input and pads to w with a Background(fill) cell. The fill color is the parameter, which lets the same helper pad to Panel (status bar) or Bg (transcript). A future enhancement could also expose a left-pad variant.

14. wrap is a simple word-wrap that splits on whitespace and respects the width budget. It doesn't handle hyphenation, soft breaks, or long-word breaking. Good enough for prose. Code blocks bypass wrap (they're rendered with renderCodeBlock which clips per line).

15. shortPath collapses paths longer than 3 segments to .../last/segment. A future enhancement could use a config-driven depth. The current 2-segment tail is a reasonable default.

16. orDash returns the input trimmed or an em-dash for empty. Used for Branch/Model/Provider to avoid showing a literal empty field. Small UX detail that matches what a coding agent wants to see.

17. resultSummary for unknown tools clips the first 56 chars of the first line. A future enhancement could call into the tool's own Summary() method. The current default is the safe fallback.

18. TestPermLayoutMatchesRender is the canonical hitbox test — it asserts the rendered button row contains both "allow" and "deny" labels, and a click in the middle of each button resolves correctly via g.Hit(x, y). A click at (0, 0) returns "". The hit-test is correct.

19. TestRenderChatLiveData exercises 4 transcript states — done (with assistant row), working+thinking (with spinner), streaming (with live text), permission (with modal). All four states are distinct visible behaviors. Comprehensive.

20. The package doc comment is a one-paragraph mission statement that names the package's role ("renders the ZERO 'Zeroline' terminal surface"), the two surfaces ("Zen home page and a Statusline chat page"), the theming ("5 switchable color themes"), and the boundary ("pure presentation: callers build the data structs from live agent state; zeroline turns them into styled terminal frames"). A future reader can grep the doc to find the package's contract.

21. forceColor is a test helper that doesn't appear in production code. The t.Cleanup restore is correct — a test that doesn't restore would leak the color profile into the next test, producing color-dependent test order issues.

22. No benchmark for RenderChat (the hot path — every frame, every keystroke, every spinner tick). The cache and the clip helpers are the obvious targets. A benchmark suite would catch future regressions where someone adds an O(n) scan per frame.

23. glamour.NewTermRenderer returns an error on init failure. markdownRenderer returns nil on error, and renderMarkdown falls back to the raw text. TestRenderMarkdownStripsSyntax exercises the happy path; the error path is not tested. A test could force a renderer failure (e.g., by passing an invalid style) to assert the fallback works.

24. themes.go doesn't include the Sel (selection) color in any visible spot. A future surface (e.g., the picker overlay) could use it as the selection color. Currently Sel is defined but unreferenced from the render code — probably reserved for the future picker hitbox fill.

25. themeStyleConfig overrides most of the glamour default colors but does not override the heading underline color or the link underline color. A future enhancement could add cfg.Heading.Underline and cfg.Link.Underline to match the theme. The current behavior inherits glamour defaults which are usually fine.

26. clip uses ansi.Truncate(..., "…") — the ellipsis is a single character. For very narrow widths (e.g., 2 cells) the ellipsis is most of the output. A future enhancement could use a thinner ellipsis (...) for very narrow budgets.

27. PermGeometry.Hit is inclusive on the left/top edges and exclusive on the right/bottom (x >= r.X && x < r.X + r.W && y >= r.Y && y < r.Y + r.H). Standard convention. Adjacent buttons don't overlap. A click exactly at the boundary resolves to the left/top button. The hitboxes are designed with gaps (Allow at x+2, width 13; Always at x+17, width 14; Deny at x+33, width 12) so a click on the gap returns "".

28. colorizeDiff (the package-level helper) hardcodes variant=0, dark=true. A future caller (e.g., a different theme) would need to pass them in. The current helper is for tests; the real renderer uses the per-call styles.colorizeDiff(detail, tw) which uses the call's theme.

Approving — this is a substantial presentation layer with thorough test coverage. The mdCache and mdOutput are bounded (with tests proving the bound), keyed by variant+dark+width so theme changes and resizes get fresh renderers, and the non-reentrant mutex pattern is correct. The PermLayout/permModalLines lockstep guarantee is enforced at construction time (with a panic if the modal line count drifts) and tested at sub-floor sizes for both the width clamp and the height clamp. The overlay is correctly suppressed when a permission prompt is up, the frame height is invariant under overlay cap, and the clip helper is wide-rune-correct (CJK/emoji). The diff colorization uses the theme Red/Green (tested) and caps at 40 lines. The tool result collapsing is the right default for a coding agent transcript. The 5 themes are clean and small. The go.mod additions are explicit and called out in the body. The forward-code boundary is clean (no caller on main yet). The follow-up is the TUI model wiring, which lands in a later module.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 237599de-614f-4256-a2cb-3ee241fb888c

📥 Commits

Reviewing files that changed from the base of the PR and between 4c886a3 and a59a615.

📒 Files selected for processing (2)

• internal/zeroline/render.go
• internal/zeroline/render_test.go

🚧 Files skipped from review as they are similar to previous changes (2)

• internal/zeroline/render_test.go
• internal/zeroline/render.go

---

Walkthrough

Adds a complete zeroline TUI renderer: five color themes, markdown->ANSI rendering with caching, RenderBoot/RenderHome/RenderChat surfaces, permission modal layout/hit-testing, overlay management, diff-aware tool output rendering, and extensive tests for clipping and overlays.

Changes

Zeroline TUI Rendering System

Layer / File(s)	Summary
Theme System & Dependencies internal/zeroline/theme.go, go.mod	Define five color themes with dark/light palettes and lookup functions; update go.mod to require github.com/charmbracelet/glamour, bump lipgloss, and add explicit terminal/color/sys deps.
Markdown Rendering with Caching internal/zeroline/markdown.go, internal/zeroline/markdown_test.go	Render CommonMark via glamour/chroma with theme-derived ansi.StyleConfig, fenced-code highlighting, and two bounded mutex-protected caches (renderers and rendered outputs). Tests cover syntax stripping, code-fence rendering, renderer caching/bounds, diff heuristics and colorization, and markdown vs streaming modes.
Boot & Home Rendering internal/zeroline/render.go (boot/home sections), internal/zeroline/render_test.go (home tests)	Implement RenderBoot animated reveal and RenderHome centered wordmark/tagline, header stripe, recent sessions, input box, and suggestion/picker overlay.
Chat Layout & Overlays internal/zeroline/render.go	Implement RenderChat orchestration: top/body/cmd/bottom regions, run-mode selection, overlay reservation/capping, overlay suppression during Perm/AskUser, and frame-width clamping.
Permission Modal & Hit-testing internal/zeroline/render.go, internal/zeroline/render_test.go	Deterministic PermLayout and PermGeometry.Hit mapping, permission modal line construction with keyboard-only fallback, and behavior when hitboxes are disabled by clipping.
Transcript, Assistant Modes, Diffs & Tools internal/zeroline/render.go	Render transcript rows (user/assistant/toolcall/toolresult/permission/system/error); split assistant rendering into glamour-markdown and streaming plain modes; detect/colorize diffs; render clipped/capped code blocks; summarize tool results.
Overlays & Display-Width Clipping Tests internal/zeroline/render_overlay_test.go, internal/zeroline/render.go	Tests ensuring overlays are suppressed during permission/question prompts, suggestion/picker overlays capped with “… more” summary, and clip/wrap respect display-cell widths (CJK/emoji). Includes long-token clipping tests.
Render Test Suite & Helpers internal/zeroline/render_test.go, internal/zeroline/render_overlay_test.go	Extensive tests for themes, RenderHome across variants, RenderChat live/working/streaming states, ask_user behavior, perm geometry/clamped cases, tool-result collapsing/diff rendering, assistant clipping for long URLs and fenced code, and ANSI-stripping helper.

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 37.68% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and specifically summarizes the main additions: glamour markdown rendering, syntax highlighting via chroma, and themed terminal surfaces for the new zeroline TUI package.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch tui-rendering

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 4

🤖 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 `@internal/zeroline/markdown_test.go`:
- Around line 177-186: The test TestStreamingAssistantStaysPlain currently only
checks for "partial" and can pass even if markdown markers are stripped; update
the assertion to verify the exact raw streaming fragment is preserved by
checking that stripANSI(RenderChat(d)) contains "partial **incomplete" (or both
"partial " and "**incomplete" together) so the markdown markers are not removed;
locate the test and the ChatData/RenderChat/stripANSI usage and replace the
current contains-check with an assertion that expects the full literal fragment
"partial **incomplete".

In `@internal/zeroline/render.go`:
- Around line 413-421: permBoxWidth can shrink too small causing the permission
modal's three-button row to overflow and leaving active hitboxes behind;
increase the minimum box width in permBoxWidth (replace the hard 38 minimum) to
at least the button-row requirement (e.g. 44) so the modal cannot be sized to
cause wrapping, and also guard the button-rendering/hitbox publishing in
permModalLines and the hitbox-publish section (the code around where hitboxes
are created/published) to skip rendering/publishing the button row when the
available width is less than the required button-row width. Ensure you reference
and update permBoxWidth and the conditional branches in permModalLines / the
hitbox publishing code so rendering and hitbox state remain consistent at narrow
widths.
- Around line 292-300: The overlay rendering currently only skips when d.Perm !=
nil or maxRows <= 0, but it must also suppress any picker/suggestions when
AskUser is active; update the early-return condition in the render block that
checks d.Perm and maxRows to also return when d.AskUser is true (so that
d.Picker, d.Suggestions, and d.SelectedIdx are not rendered while AskUser is
set), ensuring the check references d.AskUser alongside d.Perm and maxRows.
- Around line 1068-1092: The wrap function uses byte/char lengths
(len(cur)/len(word)) to decide line breaks, causing CJK/emoji width errors;
update the logic in wrap to compute widths with lipgloss.Width for cur and word
when comparing to w (e.g., replace len(...) checks with lipgloss.Width(...)),
and add unit tests exercising wrap directly (including CJK/emoji cases) similar
to existing TestClip* tests to verify display-cell-aware wrapping; ensure you
import github.com/charmbracelet/lipgloss and update any test names to cover the
plain assistant path for wrap.

🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 03e63cf0-1fb4-47b0-b38c-29692479ac8d

📥 Commits

Reviewing files that changed from the base of the PR and between f59654a and bc708e1.

⛔ Files ignored due to path filters (1)

• go.sum is excluded by !**/*.sum

📒 Files selected for processing (7)

• go.mod
• internal/zeroline/markdown.go
• internal/zeroline/markdown_test.go
• internal/zeroline/render.go
• internal/zeroline/render_overlay_test.go
• internal/zeroline/render_test.go
• internal/zeroline/theme.go

[anandh8x]

Revising my earlier APPROVE — CodeRabbit's CHANGES_REQUESTED review found 4 real issues I missed on my first pass. Walking through each:

1. TestStreamingAssistantStaysPlain has a weak assertion (real bug — test is silently weak)

Location: internal/zeroline/markdown_test.go:177-186

The test only asserts strings.Contains(out, "partial") on the rendered output. That passes even if the streaming fragment had its ** markdown markers stripped. The streaming path is supposed to keep the raw fragment verbatim (per the renderAssistantPlain contract), so the test should pin the exact fragment:

if !strings.Contains(out, "partial **incomplete") {
    t.Errorf("streaming text must keep raw markdown markers verbatim, got: %q", out)
}

A weak assertion is worse than no assertion — it gives false confidence. The whole point of renderAssistantPlain is "partial markdown renders badly mid-stream"; the test should prove the markers are kept.

2. permBoxWidth minimum of 38 is too small for the button row (real bug — buttons can render past the modal edge)

Location: internal/zeroline/render.go — permBoxWidth and the hitbox-publish section in permModalLines.

The button hitboxes are:

• Allow: bx + 2, width 13 → ends at bx + 15
• Always: bx + 17, width 14 → ends at bx + 31
• Deny: bx + 33, width 12 → ends at bx + 45

The minimum modal width is 38, so bx = (width - 38) / 2. At bw = 38, the Deny button at bx + 33 extends to bx + 45, which is 7 cells past the right border of the modal. The button label and the hitbox would render outside the modal frame. TestPermLayoutMatchesRenderClamped runs at {38, 24} which exercises this exact path — the test passes because the "[ d · deny ]" label happens to start within the modal, but the right edge of the label and the hitbox W=12 are not actually verifiable inside the modal's clip.

Fix: raise the minimum to at least 45 (or the actual button-row width requirement) and guard the hitbox-publish so the buttons are inactive when the box is narrower than the button row. The lockstep guard in permModalLines (if len(lines) != permModalRows { panic(...) }) protects line count but not width. Add an if bw < minButtonRowWidth { return PermGeometry{Active: false} } guard in PermLayout mirroring the height-clamp guard at line if top+permBtnRow >= bodyH.

3. Overlay must also be suppressed when AskUser is active (real bug)

Location: internal/zeroline/render.go:292-300 — overlayRegion early-return condition.

Current code:

if d.Perm != nil || maxRows <= 0 {
    return ""
}

When d.AskUser != nil, the picker/suggestions overlay would still render between the command line and the bottom bar. The AskUser questionnaire already takes the place of the thinking/streaming line in the transcript, but a picker overlay above the cmd line would compete for screen space. Same lockstep concern as the permission prompt: the overlay regions are budgeted out of the body, so an active overlay during AskUser would shift the focused-question lines up/down.

Fix: add d.AskUser != nil to the early-return:

if d.Perm != nil || d.AskUser != nil || maxRows <= 0 {
    return ""
}

A test could exercise the new path: TestOverlaySuppressedDuringAskUser (mirror of TestPermLayoutMatchesRenderWithOverlayPresent) would assert that an active picker overlay is NOT rendered while AskUser is set.

4. wrap uses len() which is wrong for CJK/emoji (real bug — over-wraps wide runes)

Location: internal/zeroline/render.go — wrap(text string, w int) []string.

The function does Fields(text) and then len(cur)+1+len(word) <= w to decide joining. len() is byte length, not display width. CJK runes are 3 bytes UTF-8 but 2 display cells; emoji are 4 bytes UTF-8 but 2 display cells. So wrap over-wraps CJK text — the line that should fit is broken too early.

Example: with w=10, words ["世", "界", "世", "界"]:

• After "世" (len 3, width 2): cur len 3.
• "界" (len 3, width 2): 3+1+3=7 <= 10 → join. cur len 7, width 5.
• "世" (len 3, width 2): 7+1+3=11 > 10 → break. cur len 7 stays.
• But display width check: 5+1+2=8 <= 10 → should fit!

So wrap returns 3 lines where it should return 2, and the second line is shorter than necessary. Real layout bug for any CJK assistant output.

Fix: replace len(cur)+1+len(word) <= w with lipgloss.Width(cur)+1+lipgloss.Width(word) <= w. The package already imports lipgloss. Add unit tests on wrap directly (mirroring TestClipBudgetsByDisplayWidth): exercise wrap on CJK and emoji inputs and assert no emitted line exceeds w cells. A test like TestWrapBudgetsByDisplayWidth would catch this.

Note: clip already uses lipgloss.Width correctly (tested in TestClipBudgetsByDisplayWidth); wrap was missed. The two helpers should be consistent.

---

All four are real bugs that my first pass missed. The PR's overall design (bounded caches, lockstep modal, overlay cap, theme-aware diff colorization) is solid, but these four are blockers. Switching to CHANGES_REQUESTED pending fixes. Once addressed (with the wrap test in particular locking the new behavior), this is a clean approve.

[Vasanthdev2004]

Updating my verdict after the later review comments: request changes.

Validation is green (go test ./..., go run ./cmd/zero-release build, and go run ./cmd/zero-release smoke all pass locally), but I agree the current rendering layer still has a few TUI-contract blockers:

1. AskUser should suppress overlays just like permissions do. RenderChat treats AskUser as the blocking surface, but overlayRegion only skips on Perm, so stale suggestions/pickers can still render and consume body height while a focused question is active. Please add the d.AskUser != nil guard and cover AskUser + Suggestions/Picker with a regression test.

2. The permission modal can overflow at narrow widths. permBoxWidth floors to 38, while the three-button row is wider than the modal content budget, and PermLayout can still publish active hitboxes. Please derive/enforce a minimum width from the rendered button row or suppress/publish hitboxes consistently when the buttons cannot fit, then add a small-width line-width/hitbox test.

3. Plain assistant wrapping still uses byte length and does not hard-clip long words/tokens. Markdown output is clipped, but streaming/plain output can exceed the frame for CJK/emoji or a long URL-like token. Please switch wrap decisions to display width and make the plain path guarantee every rendered line stays within budget.

The streaming verbatim test comment is a good minor cleanup too: assert the literal partial **incomplete fragment so the test proves markdown is not applied while streaming.

[gnanam1990]

Addressed all review feedback (@coderabbitai @anandh8x @Vasanthdev2004) — all 4 findings plus the extra long-token case:

1. AskUser overlay suppression — overlayRegion now returns early on d.AskUser != nil (regression test TestOverlaySuppressedDuringAskUser).
2. Permission modal narrow-width — derived permMinBoxWidth = 47 from the button row; PermLayout disables hitboxes below it (mirrors the vertical-clip guard). Rewrote TestPermLayoutMatchesRenderClamped (narrow → inactive, wide+height-clamped → lockstep).
3. wrap display width — uses lipgloss.Width not byte length (TestWrapBudgetsByDisplayWidth).
4. Plain path hard-clips long tokens (@Vasanthdev2004 item 3) — renderAssistantPlain clips each wrapped prose line so a long URL / unbroken CJK can't overflow (TestPlainAssistantClipsLongUnbrokenToken).
5. Streaming verbatim test strengthened to assert partial **incomplete survives.

build/vet/-race/full-suite + GOOS=windows build green. @coderabbitai review

[coderabbitai]

@gnanam1990 Thanks for the thorough follow-up — all five fixes look well-scoped:

1. AskUser overlay suppression — early return guard + regression test is the right pattern.
2. permMinBoxWidth = 47 — deriving the floor from the button row keeps the geometry consistent with the rendered output.
3. wrap display width — lipgloss.Width is the correct budget for multibyte/wide-rune safety.
4. Plain path hard-clip — clipping each wrapped prose line prevents URL/CJK overflow on the plain code path.
5. Streaming verbatim test — asserting partial **incomplete survives gives strong regression coverage.

Kicking off a fresh review now.

---

🧠 Learnings used

Learnt from: CR
Repo: Gitlawb/openclaude PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-06-06T00:19:44.234Z
Learning: Verify that the review calls out the risk surface and whether it introduces a blocker if the PR touches auth, provider routing, permissions, outbound network behavior, background execution, startup/config-home behavior, skills/plugins/MCP, CI permissions, or release scripts.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

🧹 Nitpick comments (3)

internal/zeroline/render_overlay_test.go (1)

37-41: 💤 Low value

Consider verifying picker suppression as well.

The test data includes both Suggestions and Picker, but only verifies that suggestion content is suppressed. For completeness, you could also assert that the picker items ("a", "b", "c") or title ("pick") don't appear.

🤖 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 `@internal/zeroline/render_overlay_test.go` around lines 37 - 41, The test
currently checks that suggestion text is suppressed by inspecting full :=
stripANSI(out) and asserting absence of "show help" and "switch model"; extend
this to also assert the picker content is suppressed by verifying that the
picker title "pick" and the picker items "a", "b", and "c" do not appear in full
(use strings.Contains on full as with the existing checks and t.Errorf on
failure) so both Suggestions and Picker outputs are validated as suppressed.

internal/zeroline/markdown_test.go (1)

79-81: ⚖️ Poor tradeoff

Consider making mdCache injectable for testing.

The test directly manipulates the global mdCache and mdMu to verify cache bounding. While this works (Go tests in the same package run sequentially by default), it creates coupling to internal implementation details and would break if tests were parallelized.

Consider adding a resetMarkdownCache() test helper or making the cache injectable through a package-level function. However, given that this test suite doesn't use t.Parallel() and the current approach works reliably, this is purely a nice-to-have.

🤖 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 `@internal/zeroline/markdown_test.go` around lines 79 - 81, The test mutates
package globals mdCache and mdMu which couples tests to implementation; add a
small package-level test helper such as resetMarkdownCache() (or
SetMarkdownCache(map[mdKey]*glamour.TermRenderer) to inject a cache) and use
that in tests instead of directly locking and assigning mdCache/mdMu; implement
resetMarkdownCache() to lock mdMu, replace mdCache with an empty map, and
unlock, or provide an injectable constructor (e.g.,
NewMarkdownRendererWithCache) so tests can supply an isolated cache.

internal/zeroline/render_test.go (1)

271-286: 💤 Low value

stripANSI only handles SGR sequences.

The helper correctly strips color codes (SGR sequences ending in 'm') from lipgloss output, which is its intended purpose. However, it would get stuck in escape mode if the input contained other CSI sequences like cursor movement (ending in 'H', 'J', etc.). This is fine for current test usage but worth noting as a limitation.

More robust ANSI stripper (if needed beyond lipgloss output)

 func stripANSI(s string) string {
 	var b strings.Builder
 	inEsc := false
 	for _, r := range s {
 		switch {
 		case r == 0x1b:
 			inEsc = true
-		case inEsc && (r == 'm'):
+		case inEsc && (r >= '@' && r <= '~'):
+			// CSI sequences end with a byte in range 0x40-0x7E
 			inEsc = false
 		case inEsc:
 		default:
 			b.WriteRune(r)
 		}
 	}
 	return b.String()
 }

🤖 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 `@internal/zeroline/render_test.go` around lines 271 - 286, stripANSI currently
only stops escape mode on SGR ('m') sequences and will hang if other CSI
sequences appear; update the stripANSI function to detect CSI sequences (escape
followed by '[') and consume runes until the CSI final byte (a rune in the range
'@'..'~') instead of only checking for 'm', so inEsc is cleared for any CSI
sequence; keep the existing handling for plain ESC and SGR but broaden the
switch in stripANSI to recognize the '[' after 0x1b and skip until the CSI
terminator to avoid getting stuck on cursor/movement sequences.

🤖 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.

Nitpick comments:
In `@internal/zeroline/markdown_test.go`:
- Around line 79-81: The test mutates package globals mdCache and mdMu which
couples tests to implementation; add a small package-level test helper such as
resetMarkdownCache() (or SetMarkdownCache(map[mdKey]*glamour.TermRenderer) to
inject a cache) and use that in tests instead of directly locking and assigning
mdCache/mdMu; implement resetMarkdownCache() to lock mdMu, replace mdCache with
an empty map, and unlock, or provide an injectable constructor (e.g.,
NewMarkdownRendererWithCache) so tests can supply an isolated cache.

In `@internal/zeroline/render_overlay_test.go`:
- Around line 37-41: The test currently checks that suggestion text is
suppressed by inspecting full := stripANSI(out) and asserting absence of "show
help" and "switch model"; extend this to also assert the picker content is
suppressed by verifying that the picker title "pick" and the picker items "a",
"b", and "c" do not appear in full (use strings.Contains on full as with the
existing checks and t.Errorf on failure) so both Suggestions and Picker outputs
are validated as suppressed.

In `@internal/zeroline/render_test.go`:
- Around line 271-286: stripANSI currently only stops escape mode on SGR ('m')
sequences and will hang if other CSI sequences appear; update the stripANSI
function to detect CSI sequences (escape followed by '[') and consume runes
until the CSI final byte (a rune in the range '@'..'~') instead of only checking
for 'm', so inEsc is cleared for any CSI sequence; keep the existing handling
for plain ESC and SGR but broaden the switch in stripANSI to recognize the '['
after 0x1b and skip until the CSI terminator to avoid getting stuck on
cursor/movement sequences.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: bf70eb43-dbe5-463e-9cfc-ac51de7fab9a

📥 Commits

Reviewing files that changed from the base of the PR and between bc708e1 and 014fa2e.

📒 Files selected for processing (4)

• internal/zeroline/markdown_test.go
• internal/zeroline/render.go
• internal/zeroline/render_overlay_test.go
• internal/zeroline/render_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• internal/zeroline/render.go

[anandh8x]

What's good

All four issues from my earlier CHANGES_REQUESTED are addressed, plus a 5th fix that the author caught while resolving them. Walking through each:

• #1 TestStreamingAssistantStaysPlain strengthened. The assertion now checks strings.Contains(out, "partial **incomplete") — pins the literal fragment, so a regression that strips the ** markers (i.e., a future refactor that accidentally routes streaming through glamour) would fail. The error message names the threat: "streaming text not verbatim (markdown markers stripped?)". A weak assertion is worse than no assertion; this is now a real test.

• #2 permMinBoxWidth = 47 constant with derived rationale. The comment is explicit: "43 button-row cells ([ a · allow ] 13 + 2 gap + [ y · always ] 14 + 2 gap + [ d · deny ] 12) plus a 1-cell border and 1-cell padding on each side". My earlier math said 44/45; the author derived 47 from the actual button strings in permModalLines (the always/deny labels are 14/12 wide including the bracket pair, not the visible-character count I used). The new guard in PermLayout:

  if bw < permMinBoxWidth {
      return PermGeometry{Active: false}
  }

  Mirrors the existing vertical-clip guard and is placed in lockstep with it. The comment names the bug this guards: "the button row would overflow the box border and can clip at the terminal edge, so the rendered buttons no longer reliably match these hitboxes. Disable them (the keyboard shortcuts still work) — mirrors the vertical-clip guard below." A future vertically-stacked narrow layout is correctly flagged as "a future enhancement".

• #3 overlayRegion now suppresses on AskUser too. The early-return is now d.Perm != nil || d.AskUser != nil || maxRows <= 0. The doc comment above the function is updated to name both cases: "For an ask_user questionnaire, RenderChat treats it as the focused blocking surface, so overlay rows would consume bodyH and push the question offscreen." TestOverlaySuppressedDuringAskUser (new) feeds an AskUser plus an active Suggestions list with a /zzsuggest token and asserts the suggestion is NOT in the rendered output. A future overlay type that needs to render during AskUser (unlikely) would have to be added explicitly.

• #4 wrap now uses lipgloss.Width. The switch case changed from len(cur)+1+len(word) <= w to lipgloss.Width(cur)+1+lipgloss.Width(word) <= w. TestWrapBudgetsByDisplayWidth (new) exercises "你好 世界" at w=10 and asserts the function returns one line, not two — proving the function uses display width (4+1+4=9 ≤ 10) instead of byte length (6+1+6=13, which would force a wrap). The comment in the test is a model: "wrap must use display width like clip does". The fix is consistent with clip, padRight, and the other width-aware helpers.

• #5 (bonus) renderAssistantPlain clips the wrapped line too. A long unbroken token (URL or unbroken CJK run) is not broken by wrap (it splits on whitespace), so the new line out = append(out, " "+s.fg.Render(clip(wl, tw-9))) applies clip per wrapped line. The comment is clear: "clip the wrapped line too: wrap splits on spaces and does not break a single word longer than the budget, so a long URL-like token or an unbroken CJK run would otherwise overflow the frame." TestPlainAssistantClipsLongUnbrokenToken (new) feeds a 400-char "x" repeat through streaming, scans for the rendered token, and asserts no line containing it exceeds the frame width. Without this fix, the previous TestAssistantMarkdownClipsLongLines (glamour path) was fine but the plain path could still overflow.

Test quality of the new tests

• TestWrapBudgetsByDisplayWidth uses strings.Repeat and a single line of assertion: a focused regression test that would catch a future revert to byte-length checks.
• TestOverlaySuppressedDuringAskUser uses a /zzsuggest token (deliberately obscure) so a future test run can't accidentally match a real command. The token is asserted absent, not present — the right polarity for a suppression test.
• TestPlainAssistantClipsLongUnbrokenToken iterates the rendered output and checks every line containing the token. A future regression that adds a single overflow line would fail. The test scope comment is precise: "Scoped to the prose line carrying the token (the statusline footer width is a separate concern)." That's a thoughtful scope-bound.
• TestPermLayoutMatchesRenderClamped was rewritten cleanly: it now has two explicit blocks — "too narrow → inactive" and "wide enough → lockstep" — with the lockstep block unchanged. The inactive-block comment names the new guard: "the buttons would overflow the modal frame / clip at the terminal edge, so hitboxes are disabled (keyboard still works)". The sizes {10,24}, {40,24}, {48,24} all yield bw < 47 after permBoxWidth clamping (38/38/46 respectively), so all three must be inactive. Good coverage of the new branch.

Other observations (non-blocking)

1. permMinBoxWidth = 47 is a package-private const next to permModalRows and permBtnRow. The trio of constants now lives together and the new comment derives 47 from the actual button strings. If the button labels change in the future, the constant needs a re-derivation. A test that asserts the constant matches the actual button width (a structural assertion) would catch this, but a unit test for "43 + 4 = 47" is overkill. The current code is fine.

2. permBoxWidth still floors to 38 for non-permission widths (e.g., when called from RenderHome's input box). The 38 minimum there is a different concern (readability of the home input). Keeping the two floors separate is correct.

3. The test TestPermLayoutMatchesRenderClamped for the inactive case is short (3 sizes, all assert !g.Active). A future enhancement could also assert the frame height stays correct when hitboxes are inactive — RenderChat should still render a usable modal that looks the same, just with the hitboxes disabled. The current test scope is fine.

4. TestPlainAssistantClipsLongUnbrokenToken only checks the line containing the token (if strings.Contains(ln, "xxx")). A future regression that adds a second line that overflows (e.g., from a wrap call elsewhere) would not be caught. The test's scope is the streaming path, so this is OK.

5. TestWrapBudgetsByDisplayWidth is a single-case test. The original CodeRabbit feedback suggested "a test exercising wrap directly (including CJK/emoji cases) similar to existing TestClip* tests". A more comprehensive test (e.g., mixing CJK and ASCII, testing wrap at narrow widths, testing wrap with emoji) would be a nice future enhancement. The current test pins the bug fix; additional cases are nice-to-have.

6. The wrap change ripples into renderAssistantPlain but the test is in render_overlay_test.go (named for the overlay tests but the file groups other tests too). A future refactor could move the wrap test into its own wrap_test.go. Minor.

7. clip and wrap are now consistent — both use lipgloss.Width and both have a regression test pinning the behavior. Good consistency. Future helpers in this file should follow the same pattern.

8. The doc comment for wrap could be updated to mention that it uses display width (matching the clip doc comment's explicit mention of display width vs rune count). A one-line addition would help future maintainers.

9. CI is green on all platforms. Build/vet/-race/full-suite + GOOS=windows build all pass per the commit message. CodeRabbit status check shows SUCCESS. No regression risk visible.

10. The commit message is well-structured — it lists the 5 fixes in order, names the test added for each, and confirms green CI. A future bisect on this PR can read the commit message and know exactly what changed and why.

Approving — the four issues from my first pass are all addressed with focused fixes and regression tests, and the author caught a 5th issue (long unbroken token overflow in the plain path) while resolving them. The lockstep guarantee between PermLayout and the rendered modal is now enforced on BOTH axes (height-clamp and width-clamp), the overlay suppression is correct on both Perm and AskUser, and the wrap/clip/renderAssistantPlain helpers are all display-width-aware. The new tests pin the new behavior so a future regression would fail. CI is green. Clean approve.

[Vasanthdev2004]

Re-review on 014fa2e: most of the previous blockers are fixed, but I still need to request changes for one remaining render-contract issue.

The AskUser overlay suppression, display-width wrapping, plain-path clipping, and streaming-verbatim test are fixed and look good. Local validation is green:

• go test ./...
• go run ./cmd/zero-release build
• go run ./cmd/zero-release smoke

Remaining blocker:

internal/zeroline/render.go:632-643 still renders the full three-button permission row even when PermLayout disables hitboxes for narrow widths. For example, at terminal width 40, permBoxWidth returns 38, but buttons is 43 cells and content(buttons) adds borders/padding without clipping or replacing it. That makes the rendered row wider than the frame even though the hitboxes are inactive. The fix should keep render + hit-test behavior aligned: when bw < permMinBoxWidth, render a compact keyboard-only hint/blank row instead of the full buttons, or otherwise guarantee every modal line fits the frame. Please add a regression test that renders RenderChat(... Width: 40, Perm: ...) and asserts every line stays within the frame width while PermLayout(...).Active == false.

[gnanam1990]

@Vasanthdev2004 fixed the remaining blocker:

• permModalLines no longer renders the full button row when too narrow — below permMinBoxWidth it renders a compact keyboard-only hint clipped to the content width, so render + hit-test are aligned (PermLayout already disables the hitboxes there).
• Added clampFrameWidth in RenderChat — a final pass that clips every composed line to the frame width, so the chrome (top/bottom bars) and modal/body can't overflow the terminal at narrow widths (this also fixes the header/statusline-footer overflow I'd flagged as separate).
• Regression test TestPermModalFitsFrameAtNarrowWidth: RenderChat(Width:40, Perm:...) asserts every line ≤ 40 while PermLayout(40,24).Active == false — exactly the test you asked for.

build/vet/-race/full-suite + GOOS=windows build green. (CodeRabbit and @anandh8x already re-approved the prior round.)

[Vasanthdev2004]

Approved on 4c886a3.

The narrow permission modal blocker is fixed: permModalLines now swaps the overflowing button row for a compact keyboard hint when bw < permMinBoxWidth, PermLayout keeps hitboxes inactive for that same narrow state, and TestPermModalFitsFrameAtNarrowWidth covers Width: 40 by asserting every rendered line stays within the frame.

I also rechecked the earlier fixes and they still look good: AskUser suppresses overlays, plain wrapping/clipping is display-width safe, and streaming preserves raw markdown markers.

Local validation passed:

• go test ./internal/zeroline -run TestPermModalFitsFrameAtNarrowWidth -count=1
• go test ./...
• go run ./cmd/zero-release build
• go run ./cmd/zero-release smoke

GitHub Actions are green. CodeRabbit is still finishing its latest run at the moment of this review, but I don’t see any remaining human-review blockers.

[coderabbitai]

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 `@internal/zeroline/render.go`:
- Around line 652-658: When rendering the narrow fallback (bw < permMinBoxWidth)
remove the mouse/click hint and only include the keyboard hints by gating any
"click a choice" prompt on PermLayout(w, d.Height).Active; update the code path
that builds buttons (the clip(...) call using s.acc.Render and s.dim.Render) and
ensure cmdRegion only emits a click prompt when PermLayout(...).Active is true.
Also add/adjust the unit TestPermModalFitsFrameAtNarrowWidth to assert
PermLayout(width, d.Height).Active == false for the tested narrow width and
verify the command hint does not advertise clicking (i.e., no "click" or
mouse-choice text) so the test locks the expected narrow behavior.

🪄 Autofix (Beta)

✅ Autofix completed

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 4fad6d1e-9f2a-4930-89c3-298d4d665cf2

📥 Commits

Reviewing files that changed from the base of the PR and between 014fa2e and 4c886a3.

📒 Files selected for processing (2)

• internal/zeroline/render.go
• internal/zeroline/render_test.go

[coderabbitai]

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

Fixes Applied Successfully

Fixed 2 file(s) based on 1 unresolved review comment.

Files modified:

• internal/zeroline/render.go
• internal/zeroline/render_test.go

Commit: a59a61538f274ac793f95d20ea6c00817d4922c4

The changes have been pushed to the tui-rendering branch.

Time taken: 4m 19s

[gnanam1990]

@coderabbitai review — gated the command hint on PermLayout(w, d.Height).Active: when the hitboxes are disabled (narrow/clipped modal) the hint no longer says "click a choice", only keys a/y/d · Esc cancel. Locked with an assertion in TestPermModalFitsFrameAtNarrowWidth. build/vet/-race/full-suite + windows green. (@Vasanthdev2004 already approved the prior round.)

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Fetch relevant sections of render.go — permModal, permModalLines, clampFrameWidth, RenderChat hint logic
rg -n "PermLayout|permMinBoxWidth|clampFrameWidth|keyboard|click|keys a|hint|Active" internal/zeroline/render.go | head -80

echo "---permModalLines---"
ast-grep --pattern 'func permModalLines($$$) $_ {
  $$$
}' internal/zeroline/render.go

echo "---RenderChat hint block---"
rg -n -A5 -B2 "click a choice\|keys a/y" internal/zeroline/render.go

echo "---TestPermModalFitsFrameAtNarrowWidth---"
rg -n -A40 "TestPermModalFitsFrameAtNarrowWidth" internal/zeroline/render_test.go

Length of output: 3524

---

---

@gnanam1990 The implementation is clean and the fix is correct.

What I verified:

Area	Finding
Hint gating (render.go line 577)	PermLayout(w, d.Height).Active correctly branches between "click a choice · keys …" and "keys …" — the two render paths (modal body permModalLines and the command hint in RenderChat) independently check the same predicate (bw < permMinBoxWidth / .Active) and agree on omitting click language.
clampFrameWidth	Provides a hard safety net after all composition — any chrome/modal/body overflow is caught regardless of path.
TestPermModalFitsFrameAtNarrowWidth	Pre-asserts PermLayout(40, 24).Active == false, checks every ANSI-stripped line ≤ 40 display cells via lipgloss.Width, and asserts "click" is absent. Covers the stated fix completely.
Risk surface	Pure TUI presentation layer; no callers on main yet. No auth, outbound network, background execution, CI permissions, or release script surface introduced. ✅

One trivial observation: PermLayout(w, d.Height) is called twice inside RenderChat (hint block + potentially elsewhere). Since it's pure arithmetic it's negligible, but worth a named local if the function grows. Not a blocker.

No outstanding issues — this is good to merge.

[approve]

---

🧠 Learnings used

Learnt from: CR
Repo: Gitlawb/openclaude PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-06-06T00:19:44.234Z
Learning: Verify that the review calls out the risk surface and whether it introduces a blocker if the PR touches auth, provider routing, permissions, outbound network behavior, background execution, startup/config-home behavior, skills/plugins/MCP, CI permissions, or release scripts.