Inline image + Mermaid diagram rendering in the preview pane #2

Merged
xenarathon merged 10 commits from worktree-mermaid-image-rendering into main 2026-08-01 13:55:45 -04:00
Owner

Renders ![]() images and fenced mermaid diagrams as real terminal graphics (Kitty/iTerm2 protocols) in the assembled-post preview pane, falling back to labeled text placeholders in unsupported terminals.

Highlights

  • New internal/graphics package: protocol detection, image decode/encode, mermaid rendering (mmdc CLI first, HTTP API fallback), a thread-safe LRU cache for remote fetches.
  • Sentinel-substitution content pipeline in bodyPreview() (glamour sanitizes raw-HTML nodes, so escapes are spliced in as text after rendering, not through the AST).
  • All network/subprocess work is wired through bubbletea's async tea.Cmd/Msg pattern so the render path never blocks View().
  • Privacy: remote image fetches are off by default (ui.preview.allow_remote_images), and placeholder text never leaks a URL's query string or an underlying error's raw text.
  • Scroll-safety: Kitty placement deletes are scoped per-ID (never a window-wide delete-all) and now fire on every preview exit, not just on scroll.

Review trail

Spec + plan each went through two independent external reviews before implementation (a multi-model consult and a manual second-opinion consult) — see docs/superpowers/specs/2026-08-01-mermaid-image-rendering-design.md and the companion -BUFFY-CONSULT.md file. All 7 implementation tasks passed individual review clean. The final whole-branch review found 2 Critical + 5 Important cross-task issues (cursor-movement policy desyncing the renderer, raw error text leaking into placeholders, two missing scan/cleanup wiring paths, and an incorrect tmux-passthrough assumption) — all fixed in one follow-up wave and independently re-verified.

Known open item

Step 10 of the plan (manual verification in a real Kitty/WezTerm terminal — scroll a post with 2+ images, confirm no ghosting) could not be performed in the agentic environment (no interactive terminal available). Recommended before relying on this in daily use.

🤖 Generated with subagent-driven-development (Claude Code)

Renders `![]()` images and fenced ```mermaid``` diagrams as real terminal graphics (Kitty/iTerm2 protocols) in the assembled-post preview pane, falling back to labeled text placeholders in unsupported terminals. ## Highlights - New `internal/graphics` package: protocol detection, image decode/encode, mermaid rendering (mmdc CLI first, HTTP API fallback), a thread-safe LRU cache for remote fetches. - Sentinel-substitution content pipeline in `bodyPreview()` (glamour sanitizes raw-HTML nodes, so escapes are spliced in as text after rendering, not through the AST). - All network/subprocess work is wired through bubbletea's async `tea.Cmd`/`Msg` pattern so the render path never blocks `View()`. - Privacy: remote image fetches are off by default (`ui.preview.allow_remote_images`), and placeholder text never leaks a URL's query string or an underlying error's raw text. - Scroll-safety: Kitty placement deletes are scoped per-ID (never a window-wide delete-all) and now fire on every preview exit, not just on scroll. ## Review trail Spec + plan each went through two independent external reviews before implementation (a multi-model consult and a manual second-opinion consult) — see `docs/superpowers/specs/2026-08-01-mermaid-image-rendering-design.md` and the companion `-BUFFY-CONSULT.md` file. All 7 implementation tasks passed individual review clean. The final whole-branch review found 2 Critical + 5 Important cross-task issues (cursor-movement policy desyncing the renderer, raw error text leaking into placeholders, two missing scan/cleanup wiring paths, and an incorrect tmux-passthrough assumption) — all fixed in one follow-up wave and independently re-verified. ## Known open item Step 10 of the plan (manual verification in a real Kitty/WezTerm terminal — scroll a post with 2+ images, confirm no ghosting) could not be performed in the agentic environment (no interactive terminal available). Recommended before relying on this in daily use. 🤖 Generated with subagent-driven-development (Claude Code)
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Rewrites bodyPreview() to substitute images/mermaid diagrams into the
glamour-rendered preview as sentinel tokens, then splice in real
terminal graphics (or privacy-safe placeholders) after rendering.
Adds findGraphicsRefs() (shared with Task 6's future scan),
Model.graphicsCache/graphicsProtocol/pendingPreviewPlacementIDs, and
the line-height-reservation + partial-visibility guard so a scrolled
viewport never shows a half-clipped image escape sequence.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- View() now deletes exactly the Kitty placement IDs it drew last
  frame (m.previewPlacementIDs) before writing the new viewport
  frame, then rotates m.pendingPreviewPlacementIDs (Task 5) into
  m.previewPlacementIDs for the next call. Scoped per-ID deletes
  avoid clobbering images placed by any other program sharing the
  terminal window.
- Fixes a gap where phasePreview scrolling (pgup/pgdown/up/down)
  moved the viewport's YOffset without calling refreshViewport(),
  so bodyPreview() never re-ran on scroll -- silently no-opping
  both the partial-visibility guard (Task 5) and this task's
  delete-before-redraw. refreshViewport() after scroll is safe:
  viewport.SetContent() (bubbles v1.0.0) does not reset YOffset
  unless new content is shorter than the current offset.
- Manual terminal verification (Step 10 of the task brief: scroll a
  real Kitty/WezTerm session with 2+ images, confirm no ghosting)
  was not performed -- no interactive terminal available in this
  environment. Needs human confirmation before shipping.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Finding 1 (critical): Kitty and iTerm2 escapes never set "don't move
cursor", so the terminal advanced an extra Rows on top of the Rows-1
blank-line padding spliceGraphics already reserves — desyncing every
line below a placement. Sets DoNotMoveCursor on both kitty.Options
(C=1) and iterm2.File (doNotMoveCursor=1). Noted in the iTerm2 comment
that WezTerm honors doNotMoveCursor=1 while iTerm2 proper does not;
that is a gap in the protocol itself, not something more code fixes.

Finding 6: Kitty transmission was never chunked, so a full base64 PNG
went out in one APC regardless of the protocol's 4096-byte payload
limit. Sets Chunk: true; kitty.EncodeGraphics does the splitting.

Finding 8: adds Quite: 2 to suppress Kitty's OK and error responses,
which bubbletea's input parser (no APC handling) could otherwise
misdecode as stray key events.

Finding 5: tmux passthrough detection was non-functional. tmux's
allow-passthrough gates tmux's OWN DCS-wrapped escapes, not the raw
APC sequences emitted here, so a user following the documented
`set -g allow-passthrough on` advice would still get a blank gap —
exactly the silent failure this project's error handling forbids.
detect() now returns ProtocolNone whenever $TMUX is set; the
tmuxAllowPassthrough helper and its sentinel error are removed as dead
code. Tests assert ProtocolNone under tmux for every protocol.

New tests read the emitted escape bytes for C=1, q=2, m=1/m=0 chunk
markers and doNotMoveCursor=1.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Finding 2 (critical): graphicsPlaceholderText interpolated
entry.err.Error() verbatim, leaking whatever the failing layer wrapped
into the visible placeholder — fetchImageBytes wraps the full image src
(query string and tracking tokens included), renderMermaidAPI wraps a
*url.Error carrying the API endpoint plus the base64-encoded diagram
source, and renderMermaidCLI wraps mmdc's entire CombinedOutput. That
violates the spec's explicit privacy rule. The reason text now comes
from graphicsFailureReason, a closed set of fixed literals: known
sentinels (ErrImageTooLarge, ErrNoBackend, errRemoteImagesDisabled,
context.DeadlineExceeded/Canceled) map to short human-readable strings
and everything else collapses to "render failed". The underlying error
stays on the cache entry for future logging but never reaches the UI.
Newlines are stripped defensively, so a placeholder can never span more
than the single row resolveGraphicsSentinel reserves for it.

The new test drives a REAL failing fetch against an httptest server
(404, URL carrying ?token=SECRET, error body echoing the request line)
rather than an injected errors.New — the synthetic-error test shape is
precisely what let this bug through seven reviews, since a fabricated
error can never carry a query string. It asserts the underlying error
does contain the token (so the test is testing something real) and
that m.View() contains neither "SECRET" nor "token=".

Finding 3: commitSection returned a nil Cmd when the last accepted
section completed the post, so the normal "finish writing → auto-land
in preview" flow never fired the graphics scan and every image sat on
"loading…" until the user manually scrolled or re-entered preview. Now
returns m.scanGraphics(), matching the ctrl+p path.

Finding 4: View() gated both the delete-emission loop and the placement
ID rotation on phasePreview, so leaving preview (ctrl+p / q / esc) never
emitted deletes for what was on screen and Kitty kept those images
composited over the review, publish or quit screen. Deletes are now
emitted unconditionally and moved ahead of the phase branch so they also
cover the edit and file-picker views; only the rotation stays phase
gated, clearing previewPlacementIDs to nil outside preview so stale
deletes are never re-emitted.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
redakt: config example + NOTICE attribution — final-review findings 7, 9
Some checks failed
CI / build / test / lint (pull_request) Failing after 5m58s
a2321c6a4c
Finding 7: redakt.toml.example documented every config section except
the new one. Adds a [ui.preview] table (matching the file's existing
separate-TOML-table convention) covering mermaid_api_url with the
mermaid.ink default and how to self-host it, mermaid_cli_path and its
blank-means-auto-detect-mmdc-on-$PATH behavior with the note that the
CLI is tried before the API, and allow_remote_images with why it
defaults to false. Also records that tmux sessions always fall back to
text placeholders regardless of allow-passthrough (see finding 5), and
carries the same correction into the design spec's tmux-guard section,
which described the now-removed passthrough query as the mechanism that
unlocks rendering.

Verified the example still parses and decodes cleanly into
config.Config via go-toml/v2 (throwaway test, not committed).

Finding 9: this branch promoted github.com/charmbracelet/x/ansi and
github.com/yuin/goldmark from indirect to direct in go.mod, but
NOTICE.md still listed both under Transitive dependencies. Moves them
into the Direct dependencies tables — x/ansi into the Charmbracelet
stack table, goldmark into the second table (heading extended with
"Markdown") — and removes them from the transitive MIT list, honoring
the file's own "never both" rule. The directDeps slice in
cmd/redakt/main.go, which `redakt license` prints and which documents
itself as mirroring go.mod's first require block, had the same drift
and gains both entries.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
xenarathon deleted branch worktree-mermaid-image-rendering 2026-08-01 13:55:47 -04:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
xenarathon/redakt!2
No description provided.