feat(tui): lean detail + artifact into OpenTUI rich components (#192)

docs/superpowers/specs/2026-05-28-tui-192-detail-artifact-opentui-design.md

@@ -0,0 +1,84 @@
+# TUI #192 — Lean detail + artifact into OpenTUI rich components
+
+**Issue:** [windoliver/grove#192](https://github.com/windoliver/grove/issues/192)
+**Date:** 2026-05-28
+**Scope decision:** Focused first cycle — `detail.tsx` + `artifact-preview.tsx` only. One PR. No other surfaces; no terminal-rendering swap. Remaining surfaces (dag, dashboard, lists, terminal) are out of scope and become follow-up issues.
+
+## Problem
+
+Grove uses OpenTUI but several surfaces still behave like a manually managed table dashboard. The two highest value-per-effort, lowest-risk surfaces:
+
+- **`detail.tsx`** — a flat stack of `<box>`/`<text>` with no scrolling, no focus/selection state, and silent `.slice()` truncation of long content (`description.slice(0,500)`, ancestor/child summaries `.slice(0,50)`, context `.slice(0,300)`).
+- **`artifact-preview.tsx`** — already uses `<scrollbox>`/`<markdown>`/`<code>`, but its diff path hand-rolls an LCS unified diff (`computeUnifiedDiff`, ~50 lines) rendered as uncolored plain text, ignoring OpenTUI's `<diff>` intrinsic that the codebase already wraps in `SplitDiff`.
+
+## Goals (from issue acceptance criteria, scoped)
+
+- Detail and artifact surfaces are easier to inspect in-terminal.
+- Focus and selection states are consistently obvious.
+- High-value transitions improve orientation instead of adding noise.
+- These two surfaces feel native to the OpenTUI stack.
+
+## Non-goals
+
+- Changes to `terminal.tsx`, `dag.tsx`, `dashboard.tsx`, lists, or other views.
+- Replacing the manual xterm cell-rendering with `ghostty-opentui`.
+- Any provider/server changes. This is pure TUI render + local UI state.
+
+## Architecture context
+
+Keyboard input is centralized: `use-keyboard-handler.ts` maps keys → named actions (e.g. `artifact_prev`, `artifact_next`, `artifact_diff`, `terminal_scroll_up`). UI/nav state (`showArtifactDiff`, `artifactIndex`, scroll offsets) lives in `use-navigation` and flows as props through `panel-manager.tsx` into the views. New interactions therefore follow a fixed pattern: **new nav state → new action in the keyboard handler → new prop into the view.** This design adds nothing outside that pattern.
+
+OpenTUI primitives relied on (all already present in the codebase): `<scrollbox>`, `<diff>` (via `SplitDiff`), `useTimeline` (declared in `opentui.d.ts`), and theme color-depth resolution (`truecolor`/`256`/`16`).
+
+## Design
+
+### 1. `detail.tsx` — scrollable, focus-aware surface
+
+- Wrap the body in `<scrollbox flexGrow={1}>`.
+- **Remove all `.slice()` truncations.** Full content (description, ancestor/child summaries, context JSON) becomes reachable via scroll rather than silently cut. Existing large-content caps that exist for *render-stall protection* (not present in detail today) are not introduced here; if a stall is observed during implementation, a generous cap is added with a visible "truncated" marker — never a silent cut.
+- Treat the existing blocks as an ordered list of **focusable sections**: Summary, Scores, Relations, Artifacts, Ancestors, Children, Discussion, Context. Sections with no data are **skipped in the focus ring** (you cannot focus an absent "Scores").
+- A new `focusedSection` index (nav state, passed as a prop) drives:
+  - an accent border + `>` marker on the focused section, using `theme.focus`;
+  - **scroll-into-view**: the scrollbox scroll position is set so the focused section is visible.
+- `j`/`k` and arrow keys move `focusedSection` when the Detail panel is focused. New actions `detail_section_next` / `detail_section_prev` in `use-keyboard-handler.ts`, mirroring the existing `artifact_prev`/`artifact_next` wiring. Focus index wraps at the ends.
+
+### 2. `artifact-preview.tsx` — real diff rendering
+
+> **API correction (post-implementation).** The probe (Task 0) + adversarial verification proved the real `@opentui/core` `<diff>` API is **`diff` (a unified-diff *string*) + `view: "unified" | "split"` + `filetype`** — there is **no** `mode`/`oldContent`/`newContent`. The bullets below describe the *as-shipped* behavior, which supersedes the original draft. The `SplitDiff` component is **not** used (it passes the nonexistent `oldContent`/`newContent`/`mode` props — a pre-existing bug tracked as a follow-up). `computeUnifiedDiff` is **kept** (its output is exactly the `diff` string the intrinsic consumes) and emits a valid `@@ -1,m +1,n @@` hunk header so the intrinsic's `parsePatch` accepts it.
+
+- Render the diff via the `<diff>` intrinsic inside a `<scrollbox>`:
+  - `diffMode === "inline"` → `<diff diff={unifiedString} view="unified" filetype={lang} />`
+  - `diffMode === "split"` → `<diff diff={unifiedString} view="split" filetype={lang} />`
+  - `unifiedString = computeUnifiedDiff(parentText, childText, ...)`, headered with `--- / +++ / @@`. Diff inputs are capped at `MAX_DIFF_LINES = 2000` (visible truncation marker, never a silent cut).
+- New `diffMode: "inline" | "split"` (default `"inline"`; inline fits narrow panes).
+- Keep the existing `[d]` toggle for diff on/off. Add `[s]` to flip split/inline. New nav state `artifactDiffMode`, new `diffMode` prop, `s` handled in the Artifact-panel keyboard block. Header reflects state: `[d]iff` and `[DIFF <mode>]  [s]plit/inline`. (The original plan also added a named `artifact_diff_mode` keymap action; it was dropped as unreachable dead code since no keymap binds it — `s` routes via the hardcoded Artifact handler.)
+
+### 3. Minimal transition
+
+- A single `useTimeline` accent pulse (~150ms ease) on:
+  - the focused section's border in detail when `focusedSection` changes, and
+  - the artifact header when `artifactIndex` changes.
+- No slide/scroll animation. Degrades to static color on `16`-color terminals (theme already resolves color depth at load).
+
+## Components / boundaries
+
+| Unit | Responsibility | Depends on |
+| --- | --- | --- |
+| `detail.tsx` | Render contribution detail; accept `focusedSection`; manage scroll-into-view + pulse | `<scrollbox>`, `useTimeline`, theme |
+| `artifact-preview.tsx` | Render artifact content; render diff via `<diff>`/`SplitDiff` per `diffMode` | `SplitDiff`, `<diff>`, theme |
+| `use-keyboard-handler.ts` | Map `j/k` (detail), `s` (artifact) to new actions | existing action dispatch |
+| `use-navigation` | Hold `focusedSection`, `artifactDiffMode` | — |
+| `panel-manager.tsx` | Thread new state into views as props | above |
+
+## Testing (TDD)
+
+- **detail:** focus ring skips empty sections; `j`/`k` advance and wrap; truncation removed (assert full text present, not the old capped length); focused section carries the accent marker.
+- **artifact:** `diffMode` selects `<diff mode="inline">` vs `SplitDiff`; `[s]` toggles `artifactDiffMode`; removing `computeUnifiedDiff` does not break the no-parent / diff-off / no-diff-data paths.
+- **keyboard-handler:** unit tests for `detail_section_next`, `detail_section_prev`, `artifact_diff_mode`, following existing `artifact_*` test patterns.
+- All changes via test-first. The two runtime unknowns below are resolved by a failing test before relying on them.
+
+## Risks / open verifications
+
+- **Scrollbox scroll-into-view API.** If OpenTUI's `<scrollbox>` exposes no controllable scroll position, fallback is manual windowing — slice the visible sections around `focusedSection`. Decided during the first TDD step, not assumed.
+- **`<diff mode="inline">` prop.** `SplitDiff` proves `mode="split"` works; confirm `"inline"` is a valid mode before relying on it. If not, inline falls back to `<code language="diff">` over the computed unified text (kept only for that fallback).
+- Low overall risk: no server/provider changes, no other surfaces touched.

src/tui/app-reducer.test.ts

@@ -0,0 +1,32 @@
+import { describe, expect, test } from "bun:test";
+import { INITIAL_KEYBOARD_STATE, tuiReducer } from "./app-reducer.js";
+
+describe("artifact diff mode (#192)", () => {
+  test("defaults to inline", () => {
+    expect(INITIAL_KEYBOARD_STATE.artifactDiffMode).toBe("inline");
+  });
+  test("ARTIFACT_DIFF_MODE_TOGGLE flips inline <-> split", () => {
+    const once = tuiReducer(INITIAL_KEYBOARD_STATE, { type: "ARTIFACT_DIFF_MODE_TOGGLE" });
+    expect(once.artifactDiffMode).toBe("split");
+    const twice = tuiReducer(once, { type: "ARTIFACT_DIFF_MODE_TOGGLE" });
+    expect(twice.artifactDiffMode).toBe("inline");
+  });
+});
+
+describe("detail section focus (#192)", () => {
+  test("defaults to 0", () => {
+    expect(INITIAL_KEYBOARD_STATE.detailFocusedSection).toBe(0);
+  });
+  test("NEXT increments, PREV decrements (may go negative; view applies modulo)", () => {
+    const a = tuiReducer(INITIAL_KEYBOARD_STATE, { type: "DETAIL_SECTION_NEXT" });
+    expect(a.detailFocusedSection).toBe(1);
+    const b = tuiReducer(a, { type: "DETAIL_SECTION_PREV" });
+    expect(b.detailFocusedSection).toBe(0);
+    const c = tuiReducer(b, { type: "DETAIL_SECTION_PREV" });
+    expect(c.detailFocusedSection).toBe(-1);
+  });
+  test("RESET returns to 0", () => {
+    const a = tuiReducer(INITIAL_KEYBOARD_STATE, { type: "DETAIL_SECTION_NEXT" });
+    expect(tuiReducer(a, { type: "DETAIL_SECTION_RESET" }).detailFocusedSection).toBe(0);
+  });
+});

src/tui/app-reducer.ts

@@ -6,6 +6,7 @@ export interface TuiKeyboardState {
   readonly vfsNavigateTrigger: number;
   readonly artifactIndex: number;
   readonly showArtifactDiff: boolean;
+  readonly artifactDiffMode: "inline" | "split";
   readonly paletteIndex: number;
   readonly paletteQuery: string;
   readonly searchQuery: string;
@@ -21,6 +22,7 @@ export interface TuiKeyboardState {
   readonly zoomLevel: ZoomLevel;
   readonly terminalScrollOffset: number;
   readonly layoutMode: "grid" | "tab";
+  readonly detailFocusedSection: number;
 }
 
 /** Actions for the TUI keyboard state reducer. */
@@ -29,6 +31,7 @@ export type TuiAction =
   | { readonly type: "ARTIFACT_PREV" }
   | { readonly type: "ARTIFACT_NEXT" }
   | { readonly type: "ARTIFACT_DIFF_TOGGLE" }
+  | { readonly type: "ARTIFACT_DIFF_MODE_TOGGLE" }
   | { readonly type: "PALETTE_UP" }
   | { readonly type: "PALETTE_DOWN"; readonly maxIndex: number }
   | { readonly type: "PALETTE_RESET" }
@@ -61,12 +64,16 @@ export type TuiAction =
   | { readonly type: "TERMINAL_SCROLL_UP" }
   | { readonly type: "TERMINAL_SCROLL_DOWN" }
   | { readonly type: "TERMINAL_SCROLL_BOTTOM" }
-  | { readonly type: "LAYOUT_TOGGLE" };
+  | { readonly type: "LAYOUT_TOGGLE" }
+  | { readonly type: "DETAIL_SECTION_NEXT" }
+  | { readonly type: "DETAIL_SECTION_PREV" }
+  | { readonly type: "DETAIL_SECTION_RESET" };
 
 export const INITIAL_KEYBOARD_STATE: TuiKeyboardState = {
   vfsNavigateTrigger: 0,
   artifactIndex: 0,
   showArtifactDiff: false,
+  artifactDiffMode: "inline",
   paletteIndex: 0,
   paletteQuery: "",
   searchQuery: "",
@@ -82,6 +89,7 @@ export const INITIAL_KEYBOARD_STATE: TuiKeyboardState = {
   zoomLevel: "normal",
   terminalScrollOffset: 0,
   layoutMode: "tab",
+  detailFocusedSection: 0,
 };
 
 /** Pure reducer for TUI keyboard state - testable and serializable. */
@@ -95,6 +103,11 @@ export function tuiReducer(state: TuiKeyboardState, action: TuiAction): TuiKeybo
       return { ...state, artifactIndex: state.artifactIndex + 1 };
     case "ARTIFACT_DIFF_TOGGLE":
       return { ...state, showArtifactDiff: !state.showArtifactDiff };
+    case "ARTIFACT_DIFF_MODE_TOGGLE":
+      return {
+        ...state,
+        artifactDiffMode: state.artifactDiffMode === "inline" ? "split" : "inline",
+      };
     case "PALETTE_UP":
       return { ...state, paletteIndex: Math.max(0, state.paletteIndex - 1) };
     case "PALETTE_DOWN":
@@ -195,5 +208,11 @@ export function tuiReducer(state: TuiKeyboardState, action: TuiAction): TuiKeybo
       return state.terminalScrollOffset === 0 ? state : { ...state, terminalScrollOffset: 0 };
     case "LAYOUT_TOGGLE":
       return { ...state, layoutMode: state.layoutMode === "tab" ? "grid" : "tab" };
+    case "DETAIL_SECTION_NEXT":
+      return { ...state, detailFocusedSection: state.detailFocusedSection + 1 };
+    case "DETAIL_SECTION_PREV":
+      return { ...state, detailFocusedSection: state.detailFocusedSection - 1 };
+    case "DETAIL_SECTION_RESET":
+      return state.detailFocusedSection === 0 ? state : { ...state, detailFocusedSection: 0 };
   }
 }

src/tui/app.tsx

@@ -278,6 +278,11 @@ export function App({
     [provider, topology, selectedSession, nav.detailCid, ks.layoutMode, showError],
   );
 
+  // biome-ignore lint/correctness/useExhaustiveDependencies: nav.detailCid is an intentional reset trigger — not read in the body, but a change means a new contribution opened, so the focused detail section must reset to the top.
+  useEffect(() => {
+    dispatch({ type: "DETAIL_SECTION_RESET" });
+  }, [nav.detailCid]);
+
   useEffect(() => {
     for (const diagnostic of mergedActionRegistry.diagnostics) {
       showError(diagnostic.message);
@@ -890,6 +895,9 @@ export function App({
       onArtifactPrev: () => dispatch({ type: "ARTIFACT_PREV" }),
       onArtifactNext: () => dispatch({ type: "ARTIFACT_NEXT" }),
       onArtifactDiffToggle: () => dispatch({ type: "ARTIFACT_DIFF_TOGGLE" }),
+      onArtifactDiffModeToggle: () => dispatch({ type: "ARTIFACT_DIFF_MODE_TOGGLE" }),
+      onDetailSectionNext: () => dispatch({ type: "DETAIL_SECTION_NEXT" }),
+      onDetailSectionPrev: () => dispatch({ type: "DETAIL_SECTION_PREV" }),
       onCompareToggle: () => dispatch({ type: "COMPARE_TOGGLE" }),
       onCompareSelect: (cid: string) => dispatch({ type: "COMPARE_SELECT", cid }),
       onCompareAdopt: (side: "a" | "b") => {
@@ -1217,6 +1225,8 @@ export function App({
           vfsNavigateTrigger={ks.vfsNavigateTrigger}
           artifactIndex={ks.artifactIndex}
           showArtifactDiff={ks.showArtifactDiff}
+          artifactDiffMode={ks.artifactDiffMode}
+          detailFocusedSection={ks.detailFocusedSection}
           activeClaims={activeClaims ?? undefined}
           searchQuery={
             panels.state.mode === InputMode.SearchInput ? ks.searchBuffer : ks.searchQuery

src/tui/hooks/use-accent-pulse.test.tsx

@@ -0,0 +1,95 @@
+/**
+ * Tests for {@link useAccentPulse} (#192) — the shared focus-change accent
+ * pulse hook extracted from <DetailView> and <ArtifactPreviewView>.
+ *
+ * The hook must be test-safe under react-test-renderer: all timeline/timer
+ * calls live inside useEffect, so a plain mount + trigger update is a no-op
+ * (no throw, no timer leak). It returns a boolean pulse flag.
+ */
+
+import { describe, expect, test } from "bun:test";
+import type React from "react";
+import TestRenderer, { act } from "react-test-renderer";
+import { useAccentPulse } from "./use-accent-pulse.js";
+
+(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
+
+// ---------------------------------------------------------------------------
+// Tiny harness component: exposes the returned pulse flag as a string leaf so
+// the test can read it from the rendered tree, and exposes the raw value for
+// type assertions via a captured ref.
+// ---------------------------------------------------------------------------
+
+let lastPulse: unknown;
+
+function Harness({ trigger }: { readonly trigger?: number | undefined }): React.ReactNode {
+  const pulse = useAccentPulse(trigger);
+  lastPulse = pulse;
+  return <text>{String(pulse)}</text>;
+}
+
+describe("useAccentPulse (#192)", () => {
+  test("renders without throwing and returns a boolean", async () => {
+    let renderer!: TestRenderer.ReactTestRenderer;
+    await act(async () => {
+      renderer = TestRenderer.create((<Harness trigger={0} />) as React.ReactElement);
+    });
+    expect(renderer.toJSON()).toBeDefined();
+    expect(typeof lastPulse).toBe("boolean");
+    renderer.unmount();
+  });
+
+  test("renders without throwing when trigger is undefined", async () => {
+    let renderer!: TestRenderer.ReactTestRenderer;
+    await act(async () => {
+      renderer = TestRenderer.create((<Harness />) as React.ReactElement);
+    });
+    expect(renderer.toJSON()).toBeDefined();
+    expect(typeof lastPulse).toBe("boolean");
+    renderer.unmount();
+  });
+
+  test("mounting then updating the trigger re-renders without throwing", async () => {
+    let renderer!: TestRenderer.ReactTestRenderer;
+    await act(async () => {
+      renderer = TestRenderer.create((<Harness trigger={0} />) as React.ReactElement);
+    });
+    await act(async () => {
+      await Promise.resolve();
+      await Promise.resolve();
+    });
+    await act(async () => {
+      renderer.update((<Harness trigger={1} />) as React.ReactElement);
+    });
+    expect(renderer.toJSON()).toBeDefined();
+    expect(typeof lastPulse).toBe("boolean");
+    renderer.unmount();
+  });
+
+  // Regression (F3 / round-1 review): after a trigger change the pulse must
+  // return to false within ~durationMs. The original effect listed the
+  // per-render `timeline` instance in its deps, so setPulse(true)'s re-render
+  // re-ran the effect, cleared the fallback timer, then early-returned —
+  // leaving the pulse stuck on permanently. With deps [trigger, durationMs]
+  // the fallback timer survives and clears the pulse.
+  test("pulse returns to false within ~durationMs after a trigger change (no stuck-on)", async () => {
+    let renderer!: TestRenderer.ReactTestRenderer;
+    await act(async () => {
+      renderer = TestRenderer.create((<Harness trigger={0} />) as React.ReactElement);
+    });
+    await act(async () => {
+      await Promise.resolve();
+      await Promise.resolve();
+    });
+    // Change the trigger -> pulse flips true.
+    await act(async () => {
+      renderer.update((<Harness trigger={1} />) as React.ReactElement);
+    });
+    // Wait past the fallback clear (durationMs default 150 + 20 slack).
+    await act(async () => {
+      await new Promise((resolve) => setTimeout(resolve, 220));
+    });
+    expect(lastPulse).toBe(false);
+    renderer.unmount();
+  });
+});