feat(tui): interactive diff view with unified/side-by-side/plain modes

[engalar]

Summary

• Add interactive DiffView component with sticky line numbers, hunk navigation (]c/[c), horizontal scroll, and search
• Add three display modes: Unified, Side-by-Side, and Plain Diff (LLM-friendly unified diff for yanking)
• Introduce View interface and ViewStack to replace scattered boolean visibility flags in app.go
• Add type-prefixed search in FuzzyList (e.g. e:Customer to filter entities)
• Fix preview auto-trigger, scroll behavior, and hunk navigation

Code Review Fixes (post-review commit)

• Remove dead leftOffset/rightOffset/syncScroll/focus scaffold fields
• Replace time.Sleep with tea.Tick in flash-clear cmd
• Consolidate duplicate stripANSI/stripAnsi into single function
• Extract scrollbarGeometry/scrollbarChar helpers (3× duplication removed)
• Move diff color palette to theme.go as AdaptiveColor pairs for light/dark support
• Add y:yank to DiffViewHints; add hslice OSC limitation comment
• go mod tidy — promote sergi/go-diff to direct dependency

Test Plan

• go test ./cmd/mxcli/tui/... passes
• Unified mode: scroll, hunk nav, search, yank work
• Side-by-side mode: both panes render correctly
• Plain Diff mode: output matches standard unified diff format
• Tab cycles through all three modes
• y copies plain diff to clipboard
• Type-prefixed search (e.g. e:, mf:) filters correctly in FuzzyList

[ako]

Code Review: feat(tui): interactive diff view with unified/side-by-side/plain modes

30 files, +4880/-629, 5 commits. Adds DiffView component with three display modes, introduces View interface/ViewStack pattern, adds type-prefixed fuzzy search.

---

Critical Issues

1. Overlay Tab-switching is broken (browserview.go / app.go)

BrowserView.handleKey for "b" and "m" keys sets bv.overlayQName, bv.overlayNodeType, bv.overlayIsNDSL on the value receiver, then emits OpenOverlayMsg. But App handles OpenOverlayMsg with:

ov := NewOverlayView(msg.Title, msg.Content, a.width, a.height, OverlayViewOpts{})

The opts are always empty — Switchable is false, QName is empty. NDSL/MDL Tab-switching in the overlay is dead.

2. DiffView search broken in PlainDiff mode (diffview.go)

buildMatchLines() searches dv.result.Lines (structural diff lines), but PlainDiff mode renders dv.plainLines which have different indices (includes @@ headers, ---/+++ lines). Search match indices don't correspond to visible lines, causing scroll-to-match to jump to wrong positions.

3. Key hint mismatch: d shown but D required (hintbar.go / compare.go)

CompareHints shows {Key: "d", Label: "diff"} but the handler checks case "D":. Users will press d and nothing happens.

---

Moderate Issues

4. hslice/truncateToWidth don't handle wide characters (diffrender.go)

hslice increments visW by 1 per rune. East Asian characters take 2 columns. mattn/go-runewidth is added as a dependency but not used in these functions. Diff view will misalign with CJK text.

5. handleBrowserAppKeys returns nil-cmd goroutines (app.go)

Many cases return func() tea.Msg { return nil } as a "handled" sentinel. Each spawns a goroutine that does nothing. Should use a no-op constant or change the return signature.

6. CompareView visible field redundant with ViewStack (compare.go)

updateInternal starts with if !c.visible { return c, nil } — all updates are no-ops unless Show() was called. This is redundant with being on the ViewStack and fragile if someone forgets to call Show().

---

Dead Code

7. BrowserView.overlayQName, overlayNodeType, overlayIsNDSL fields — set but never read (browserview.go)

These are set in handleKey but the OpenOverlayMsg handler creates overlays with empty opts. Remove these fields.

8. BrowserView.handleKey "c" and "r" cases unreachable (browserview.go)

Both keys are intercepted by App.handleBrowserAppKeys before reaching BrowserView.Update. The BrowserView cases are dead.

9. BrowserView.compareItems field — set but never read (browserview.go)

Assigned in syncBrowserView() and LoadTreeMsg handler but never used by BrowserView.

---

Minor Issues

10. Magic numbers

• 150ms debounce delay (miller.go)
• 3 for divider width in side-by-side (diffrender.go)
• 8 for horizontal scroll step (diffview.go)

11. renderContextSummary naive pluralization (app.go)

strings.ToLower(k)+"s" produces wrong plurals for some types (e.g., "Index" → "indexs").

12. Duplicated loadBsonNDSL across BrowserView and CompareView

Identical implementations in both views.

---

What Looks Good

• The View interface + ViewStack pattern is a solid architectural improvement over the scattered boolean flags — makes view routing declarative and composable
• DiffEngine with word-level inline highlighting using SequenceMatcher + sergi/go-diff is well-implemented
• Type-prefixed fuzzy search (e:Customer, mf:query) is a nice UX touch using dynamic fuzzyScore matching
• 40 unit tests for DiffEngine, DiffRenderer, and FuzzyList
• The post-review cleanup commit addressing dead fields, time.Sleep → tea.Tick, scrollbar dedup, and theme extraction shows good self-review discipline
• Plain Diff mode for LLM-friendly output is a thoughtful addition

---

Workflow Note

The AI review workflow ran but returned an empty response from models.github.ai — likely the 6k-line diff exceeded the request size limit. The pull-requests: write permission was also not granted (only Contents: read, Metadata: read, Models: read appeared in the token). Will fix the workflow separately.

---

Recommendation

Request changes — fix the critical issues (#1-3) before merging. The overlay Tab-switching being completely broken and the search/hint mismatches are user-facing bugs. The dead code (#7-9) should also be cleaned up since it was identified during the ViewStack refactor.

🤖 Generated with Claude Code