feat: extract tviewmd markdown renderer and wire into tui2 - #183
Conversation
Replace glamour+TranslateANSI in tui2 with native tview color-tag rendering via the published github.com/buchenberg/tviewmd module (v0.1.0). The tviewmd module (14 files, MIT) provides: - CommonMark + GFM parsing via goldmark - chroma syntax highlighting for fenced code blocks - tview color-tag backend (no ANSI round-trip) - 23 tests + fuzz (315k+ executions, zero panics) Adds architecture review (docs/architecture-review.md) and implementation plan (.agents/plans/tui2-hardening/PLAN.md).
📝 WalkthroughWalkthroughThe change documents tui2 hardening and architecture findings, adds Changestui2 hardening and markdown rendering
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Replace glamour+TranslateANSI in tui2 with native tview color-tag rendering via the published github.com/buchenberg/tviewmd module (v0.1.0). The tviewmd module (14 files, MIT) provides: - CommonMark + GFM parsing via goldmark - chroma syntax highlighting for fenced code blocks - tview color-tag backend (no ANSI round-trip) - 23 tests + fuzz (315k+ executions, zero panics) Adds architecture review (docs/architecture-review.md) and implementation plan (.agents/plans/tui2-hardening/PLAN.md).
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
internal/tui2/markdown.go (1)
10-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd an integration test for the Markdown wrapper.
Test empty input, fenced code, literal bracket text, and a narrow width. These cases verify the
TUI2.Messagesboundary and protect the new color-tag contract.🤖 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/tui2/markdown.go` around lines 10 - 14, Add an integration test covering the Markdown wrapper around renderMarkdown and the TUI2.Messages boundary. Verify empty input, fenced code, literal bracket text, and narrow-width rendering, including the expected color-tag contract; keep the existing renderMarkdown behavior unchanged..agents/plans/tui2-hardening/PLAN.md (2)
32-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd language identifiers to both diagram fences.
Markdownlint reports MD040 at Line 32 and Line 255. Use
textfor both fences.Proposed fix
-``` +```textApply this change at both opening fences.
As per static analysis, these fences currently lack language identifiers.
Also applies to: 255-255
🤖 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 @.agents/plans/tui2-hardening/PLAN.md at line 32, Add the `text` language identifier to both opening diagram code fences in the plan document, including the fences near the sections associated with lines 32 and 255, while leaving their enclosed diagram content unchanged.Source: Linters/SAST tools
350-357: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winResolve the Phase 1 scope contradiction.
The phase says “No tables, no chroma yet” and then says “Wire chroma code highlighting.” Either defer Chroma or include it in the declared Phase 1 scope and exit gate.
🤖 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 @.agents/plans/tui2-hardening/PLAN.md around lines 350 - 357, Resolve the contradictory Phase 1 checklist by choosing one scope: either remove/defer the “Wire chroma code highlighting” item and related exit-gate expectations, or explicitly add Chroma support to the declared Phase 1 feature list and validation criteria. Ensure the resulting scope consistently states whether Chroma is included.
🤖 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 @.agents/plans/tui2-hardening/PLAN.md:
- Around line 144-148: Update the extraction procedure near the git mv command
to remove the invalid outside-worktree move. Document one tested approach using
either git filter-repo or copying the md module into a newly initialized
repository, then retain the follow-up go.work removal, publication, and yaah
dependency steps.
- Around line 82-100: Align both .agents/plans/tui2-hardening/PLAN.md sections
at lines 82-100 and 102-114 with github.com/buchenberg/tviewmd v0.1.0: replace
the custom table model with TableCell/TableData using TableData.Aligns, add
ListItem.Index, and set Block.Table to TableData. Define Options with Width,
NoHighlight, and Theme, reference DefaultTheme(), and document that zero values
enable highlighting and default to width 80.
- Around line 29-40: Rewrite A.1 and A.6 in .agents/plans/tui2-hardening/PLAN.md
to depend on the published github.com/buchenberg/tviewmd v0.1.0 instead of
creating, locally resolving, or extracting an md module; update the related
go.mod dependency entry at go.mod:11 accordingly. Revise A.2 and the planned API
references in internal/tui2/markdown.go:4-14 to use v0.1.0’s DefaultTheme,
NoHighlight, TableData, and TableCell symbols, removing references to DarkTheme,
Highlight, and the planned Table shape. Remove the no-longer-applicable
go.work/md module structure from the plan at PLAN.md:29-40 and PLAN.md:142-152.
In `@docs/architecture-review.md`:
- Line 75: Update the Markdown-related entry in the architecture review table to
document tui2’s current tviewmd.Render integration with native tview color tags
instead of Glamour, preserving the table’s existing format.
- Around line 84-86: Update the missing-feature list in architecture-review.md
to remove claims that model-picker data wiring is unimplemented, since
CtrlModelList population and OnModelSelect are already handled by FetchAllModels
in tui2.go. Retain only genuinely missing model-picker UI gaps at the referenced
entries.
In `@internal/tui2/markdown.go`:
- Around line 10-14: Update renderMarkdown and all its callers to accept the
messages-pane width instead of hard-coding 80; in refreshMessages, capture
t.Messages.GetInnerRect().Dx() once and pass it through both streaming and
assistant-response rendering paths. Extend tviewmd table rendering so generated
tables fit the supplied Options.Width, preserving row alignment and avoiding
wrapping by TextView.
---
Nitpick comments:
In @.agents/plans/tui2-hardening/PLAN.md:
- Line 32: Add the `text` language identifier to both opening diagram code
fences in the plan document, including the fences near the sections associated
with lines 32 and 255, while leaving their enclosed diagram content unchanged.
- Around line 350-357: Resolve the contradictory Phase 1 checklist by choosing
one scope: either remove/defer the “Wire chroma code highlighting” item and
related exit-gate expectations, or explicitly add Chroma support to the declared
Phase 1 feature list and validation criteria. Ensure the resulting scope
consistently states whether Chroma is included.
In `@internal/tui2/markdown.go`:
- Around line 10-14: Add an integration test covering the Markdown wrapper
around renderMarkdown and the TUI2.Messages boundary. Verify empty input, fenced
code, literal bracket text, and narrow-width rendering, including the expected
color-tag contract; keep the existing renderMarkdown behavior unchanged.
🪄 Autofix
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: 8c9c0586-f9e0-40e7-b94e-3cdacb0d8595
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
.agents/plans/tui2-hardening/PLAN.mddocs/architecture-review.mdgo.modinternal/tui2/markdown.go
| // Block is one block-level element. Exactly one field is set. | ||
| type Block struct { | ||
| Kind BlockKind // Heading / Paragraph / List / CodeBlock / Table / BlockQuote / ThematicBreak | ||
| Level int // heading level 1-6 | ||
| Segments []Segment // paragraph / heading / quote inline content | ||
| Items []ListItem | ||
| Code CodeBlock | ||
| Table Table | ||
| } | ||
|
|
||
| type ListItem struct { | ||
| Segments []Segment | ||
| Children []ListItem // nesting | ||
| Ordered bool | ||
| } | ||
|
|
||
| type CodeBlock struct{ Lang, Source string } | ||
| type Table struct{ Header []Segment; Rows [][]Segment; Align []TextAlign } | ||
| ``` |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
curl -fsSL \
https://raw.githubusercontent.com/buchenberg/tviewmd/v0.1.0/ast.go \
> "$tmp/ast.go"
curl -fsSL \
https://raw.githubusercontent.com/buchenberg/tviewmd/v0.1.0/render.go \
> "$tmp/render.go"
rg -n \
'type (TableCell|TableData|ListItem|Options)|Index|NoHighlight|DefaultTheme|func Render' \
"$tmp/ast.go" "$tmp/render.go"Repository: buchenberg/yaah
Length of output: 1005
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- plan ---'
sed -n '70,125p' .agents/plans/tui2-hardening/PLAN.md
echo '--- selected module declarations ---'
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
curl -fsSL https://raw.githubusercontent.com/buchenberg/tviewmd/v0.1.0/ast.go > "$tmp/ast.go"
curl -fsSL https://raw.githubusercontent.com/buchenberg/tviewmd/v0.1.0/render.go > "$tmp/render.go"
sed -n '1,115p' "$tmp/ast.go"
sed -n '1,90p' "$tmp/render.go"
echo '--- repository module references ---'
rg -n 'tviewmd|TableCell|TableData|NoHighlight|DefaultTheme|type Block|type ListItem|type Options' \
--glob '!vendor/**' --glob '!node_modules/**' .Repository: buchenberg/yaah
Length of output: 7076
Align the plan with github.com/buchenberg/tviewmd v0.1.0.
The pinned module uses TableCell/TableData, TableData.Aligns, and ListItem.Index. Set Block.Table to TableData. Define Options with Width, NoHighlight, and Theme; use DefaultTheme(), and document that zero values enable highlighting and use width 80. Update both cited sections.
📍 Affects 1 file
.agents/plans/tui2-hardening/PLAN.md#L82-L100(this comment).agents/plans/tui2-hardening/PLAN.md#L102-L114
🤖 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 @.agents/plans/tui2-hardening/PLAN.md around lines 82 - 100, Align both
.agents/plans/tui2-hardening/PLAN.md sections at lines 82-100 and 102-114 with
github.com/buchenberg/tviewmd v0.1.0: replace the custom table model with
TableCell/TableData using TableData.Aligns, add ListItem.Index, and set
Block.Table to TableData. Define Options with Width, NoHighlight, and Theme,
reference DefaultTheme(), and document that zero values enable highlighting and
default to width 80.
| func renderMarkdown(md string) string { | ||
| initRenderer() | ||
| if renderer == nil { | ||
| return md | ||
| } | ||
| out, err := renderer.Render(md) | ||
| if err != nil { | ||
| return md | ||
| if md == "" { | ||
| return "" | ||
| } | ||
| out = strings.TrimSpace(out) | ||
| return tview.TranslateANSI(out) | ||
| return tviewmd.Render(md, tviewmd.Options{Width: 80}) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 \
'renderMarkdown\s*\(|GetInnerRect\(\)\.Dx\(\)|SetWrap\(true\)' \
internal/tui2 || trueRepository: buchenberg/yaah
Length of output: 2507
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- internal/tui2/markdown.go ---'
cat -n internal/tui2/markdown.go
printf '%s\n' '--- internal/tui2/tui2.go relevant range ---'
sed -n '240,330p' internal/tui2/tui2.go
printf '%s\n' '--- internal/tui2/helpers_msg.go ---'
cat -n internal/tui2/helpers_msg.go
printf '%s\n' '--- module and plan references ---'
rg -n -C 5 'tviewmd|GetInnerRect\(\)|refreshMessages|width propagation|Width: 80' go.mod go.sum .agents/plans/tui2-hardening/PLAN.md internal/tui2
printf '%s\n' '--- dependency files if present ---'
git ls-files | rg '(^|/)(render_tview\.go|tviewmd|go\.mod|go\.sum)$' || trueRepository: buchenberg/yaah
Length of output: 26427
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
curl -fsSL \
https://raw.githubusercontent.com/buchenberg/tviewmd/v0.1.0/render_tview.go \
-o "$tmpdir/render_tview.go"
curl -fsSL \
https://raw.githubusercontent.com/buchenberg/tviewmd/v0.1.0/render.go \
-o "$tmpdir/render.go"
printf '%s\n' '--- tviewmd Options and render implementation ---'
cat -n "$tmpdir/render.go"
cat -n "$tmpdir/render_tview.go"
printf '%s\n' '--- all renderMarkdown call sites ---'
rg -n -C 3 'renderMarkdown\s*\(' internal/tui2
printf '%s\n' '--- Messages construction and layout ---'
rg -n -C 8 'Messages\s*=|messages\.Build|AddItem\(.*Messages|NewFlex|GetInnerRect' internal/tui2
printf '%s\n' '--- deterministic dependency checks ---'
python3 - "$tmpdir/render.go" "$tmpdir/render_tview.go" <<'PY'
import re
import sys
from pathlib import Path
render = Path(sys.argv[1]).read_text()
tview = Path(sys.argv[2]).read_text()
options = re.search(r"type\s+Options\s+struct\s*\{(?P<body>.*?)\}", render, re.S)
width_uses = [(m.start(), tview[max(0, m.start()-100):m.end()+100])
for m in re.finditer(r"\bWidth\b", tview)]
print("options_width_declared:",
bool(options and re.search(r"\bWidth\s+int\b", options.group("body"))))
print("render_tview_width_reference_count:", len(width_uses))
for i, (_, context) in enumerate(width_uses, 1):
print(f"width_reference_{i}:", " ".join(context.split()))
print("table_identifier_count:",
len(re.findall(r"\b[Tt]able\b|\bCell\b|\bcolumn\b", tview)))
PYRepository: buchenberg/yaah
Length of output: 23384
Pass the messages-pane width through every Markdown render path.
renderMarkdown hard-codes 80. The renderer uses this value for thematic breaks, so rules wrap in narrow panes and stop at 80 columns in wider panes. Its table renderer ignores Options.Width, so tables wider than the pane are wrapped by TextView and lose row alignment. Capture t.Messages.GetInnerRect().Dx() once in refreshMessages, pass it to streaming and assistant-response rendering, and add table-fitting logic to tviewmd.
🤖 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/tui2/markdown.go` around lines 10 - 14, Update renderMarkdown and
all its callers to accept the messages-pane width instead of hard-coding 80; in
refreshMessages, capture t.Messages.GetInnerRect().Dx() once and pass it through
both streaming and assistant-response rendering paths. Extend tviewmd table
rendering so generated tables fit the supplied Options.Width, preserving row
alignment and avoiding wrapping by TextView.
…o feat/tviewmd # Conflicts: # .agents/plans/tui2-hardening/PLAN.md
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 @.agents/plans/tui2-hardening/PLAN.md:
- Line 436: Update the table row in PLAN.md near the “Committed on branch
feat/tviewmd” entry to add the missing trailing pipe delimiter, ensuring the row
conforms to markdownlint MD055.
🪄 Autofix
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: 77237df2-6f6e-438b-a013-23228bb6eba3
📒 Files selected for processing (1)
.agents/plans/tui2-hardening/PLAN.md
| | 2026-08-07 | 1 | Module scaffolded, 7 source + 4 test files (14 total), 23 tests + fuzz | | ||
| | 2026-08-07 | A.6 | Extracted to GitHub `buchenberg/tviewmd` v0.1.0; local `md/` + `go.work` removed | | ||
| | 2026-08-07 | — | yaah `go.mod` wired to published module; build/vet/test/staticcheck/gofmt all clean | | ||
| | 2026-08-07 | — | Committed on branch `feat/tviewmd` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the missing trailing table delimiter.
Line 436 lacks the trailing | required by markdownlint MD055.
Proposed fix
-| 2026-08-07 | — | Committed on branch `feat/tviewmd`
+| 2026-08-07 | — | Committed on branch `feat/tviewmd` |📝 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.
| | 2026-08-07 | — | Committed on branch `feat/tviewmd` | |
| | 2026-08-07 | — | Committed on branch `feat/tviewmd` | |
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 436-436: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
🤖 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 @.agents/plans/tui2-hardening/PLAN.md at line 436, Update the table row in
PLAN.md near the “Committed on branch feat/tviewmd” entry to add the missing
trailing pipe delimiter, ensuring the row conforms to markdownlint MD055.
Source: Linters/SAST tools
…anup - Thread pane width through renderMarkdown via GetInnerRect().Dx() instead of hard-coded 80 columns (CodeRabbit #6) - Update architecture review: tui2 markdown entry now says tviewmd instead of glamour (#4), model-picker wiring is partial not missing (#5) - Add language identifiers to unlabeled code fences in plan (#markdownlint) - Phase 1 text already replaced by checkmarked completion in amended commit
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 `@internal/tui2/markdown.go`:
- Line 18: Fix table sizing in renderMarkdown by upgrading or patching the
pinned tviewmd dependency so table column widths honor the supplied
Options.Width instead of only cell-content widths. Add a regression test
covering a table rendered at a narrow width and verify rows remain aligned
without TextView wrapping.
🪄 Autofix
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: d920cb4b-c0a2-42f4-8327-eeb797f15333
📒 Files selected for processing (6)
.agents/plans/tui2-hardening/PLAN.mddocs/architecture-review.mdinternal/tui2/events.gointernal/tui2/helpers_msg.gointernal/tui2/markdown.gointernal/tui2/tui2.go
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/architecture-review.md
- .agents/plans/tui2-hardening/PLAN.md
| if width <= 0 { | ||
| width = 80 | ||
| } | ||
| return tviewmd.Render(md, tviewmd.Options{Width: width}) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
Fix table sizing in the width-aware renderer.
renderMarkdown passes width to tviewmd, but tviewmd v0.1.0 calculates table columns from cell contents and does not use Options.Width. Wide tables are therefore wrapped by TextView, which breaks row alignment. (raw.githubusercontent.com)
This is the same unresolved issue from the previous review. Patch or upgrade the dependency, and add a narrow-width table regression test.
Verification script
#!/bin/bash
set -euo pipefail
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
curl -fsSL \
https://raw.githubusercontent.com/buchenberg/tviewmd/v0.1.0/render.go \
-o "$tmpdir/render.go"
curl -fsSL \
https://raw.githubusercontent.com/buchenberg/tviewmd/v0.1.0/render_tview.go \
-o "$tmpdir/render_tview.go"
rg -n -C 3 'Width|func renderTable|opts\.Width' "$tmpdir"This assessment uses the pinned tviewmd v0.1.0 implementation and the previous review finding.
🤖 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/tui2/markdown.go` at line 18, Fix table sizing in renderMarkdown by
upgrading or patching the pinned tviewmd dependency so table column widths honor
the supplied Options.Width instead of only cell-content widths. Add a regression
test covering a table rendered at a narrow width and verify rows remain aligned
without TextView wrapping.
Expand the architecture review with a comprehensive §4 section covering `internal/tui2`'s package topology (~19 subpackages), the flat-factory component pattern, and an analysis of the imperative widget tree model with its strengths (simplicity, zero abstraction overhead) and weaknesses (no lifecycle standardization, lack of isolation via shared *App). Update the §3 recommendation to reference the new deep-dive section.
Replace glamour+TranslateANSI in tui2 with native tview color-tag rendering via the published github.com/buchenberg/tviewmd module (v0.1.0).
The tviewmd module (14 files, MIT) provides:
Adds architecture review (docs/architecture-review.md) and implementation plan (.agents/plans/tui2-hardening/PLAN.md).
Summary by CodeRabbit
New Features
Documentation