perf: precompute render span cell widths

[benvinegar]

Summary

• precompute terminal cell widths when Pierre render spans are constructed
• reuse planned span widths during render-window slicing and wrapping
• preserve behavior by clearing cached width metadata when sanitization changes span text

Validation

• bun test src/lib/terminalText.test.ts src/ui/diff/pierre.test.ts src/ui/lib/ui-lib.test.ts
• bun run typecheck

This PR description was generated by Pi using OpenAI GPT-5

[greptile-apps]

Greptile Summary

This PR precomputes terminal cell widths (cellWidth) at RenderSpan construction time in pierre.ts and threads those cached values through the sliceSpansWindow and wrapSpans render paths, replacing per-render measureTextWidth calls with cheap integer reads. Sanitization correctly clears the cached width when span text is mutated by sanitizeTerminalSpans.

• createRenderSpan and updated mergeSpan in pierre.ts measure each span exactly once at construction and propagate the summed width through coalesced runs.
• measureRenderSpanWidth in renderRows.tsx reads cellWidth when available and falls back to measureTextWidth otherwise; sliced and wrapped sub-spans set their own cellWidth from the measured slice width.
• sanitizeTerminalSpans clears cellWidth when sanitization changes span text, which is safe but slightly conservative since measureTextWidth already sanitizes internally and would return the same value.

Confidence Score: 4/5

Safe to merge; the optimization correctly threads precomputed widths through the render pipeline without changing visible output, and falls back to live measurement in all edge cases.

The core logic is sound: widths are measured once at construction and reused at render time. The fallback to measureTextWidth is always available, so no incorrect widths can reach the screen. Two observations hold back a clean bill: appendRenderSpan drops both spans' cached widths when either is missing, which is more conservative than the parallel mergeSpan in pierre.ts and can cause redundant re-measurement after sanitization; and sanitizeTerminalSpans adds a RenderSpan-specific field to a generic utility via an as T cast. Neither produces wrong output, but the first works against the optimization's goal in the sanitized-span case and the second is a latent type-safety gap.

src/ui/diff/renderRows.tsx — specifically the appendRenderSpan merge logic and the zero-width span pass-through path.

Important Files Changed

Filename	Overview
src/lib/terminalText.ts	Clears cellWidth when sanitization mutates span text; minor type-safety concern from adding a RenderSpan-specific field inside a generic function.
src/ui/diff/pierre.ts	Adds cellWidth field to RenderSpan; introduces createRenderSpan helper and updates mergeSpan to precompute and propagate terminal-cell widths correctly during span construction and coalescing.
src/ui/diff/renderRows.tsx	Adds measureRenderSpanWidth helper and threads precomputed widths through sliceSpansWindow and wrapSpans; appendRenderSpan drops cached width when either merged span lacks it, which is more conservative than mergeSpan's approach in pierre.ts.

Sequence Diagram

sequenceDiagram
    participant HAST as HAST Node
    participant FL as flattenHighlightedLine
    participant MS as makeSplitCell/makeStackCell
    participant STS as sanitizeTerminalSpans
    participant SSW as sliceSpansWindow/wrapSpans
    participant Render as renderInlineSpans

    HAST->>FL: text node value
    FL->>FL: "createRenderSpan(text) → cellWidth = measureTextWidth(text)"
    FL->>FL: mergeSpan() → accumulate cellWidth on coalesced runs
    FL-->>MS: RenderSpan[] with cellWidth (cached)
    MS-->>Render: cell.spans (shared cache reference)

    Render->>STS: sanitizeTerminalSpans(spans)
    STS-->>Render: spans (unchanged if text clean) OR new span with cellWidth:undefined
    Render->>SSW: sliceSpansWindow(sanitized, offset, width)
    SSW->>SSW: measureRenderSpanWidth → span.cellWidth ?? measureTextWidth
    SSW->>SSW: "sliceTextByWidth → { text, width }"
    SSW-->>Render: "sliced spans with cellWidth = visible.width"

Loading

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
src/ui/diff/renderRows.tsx:59-62
`appendRenderSpan` clears `cellWidth` when either merged span lacks it, but `mergeSpan` in `pierre.ts` always preserves the merged width by falling back to `measureTextWidth`. This asymmetry means that any span flowing through `sanitizeTerminalSpans` (which nulls out `cellWidth` when text changes) and then being merged here will cause the combined span to lose its precomputed width and force a later `measureTextWidth` fallback — the opposite of this PR's goal. Using the same fallback-or-measure pattern as `mergeSpan` would keep widths cached across the sanitization boundary.

```suggestion
    previous.cellWidth =
      previous.cellWidth !== undefined || span.cellWidth !== undefined
        ? (previous.cellWidth ?? measureTextWidth(previous.text)) +
          (span.cellWidth ?? measureTextWidth(span.text))
        : undefined;
```

### Issue 2 of 2
src/lib/terminalText.ts:78
`sanitizeTerminalSpans` is generic over `T extends { text: string }`, but the new branch adds `cellWidth: undefined` — a `RenderSpan`-specific field — to the spread and hides it behind an `as T` cast. Any future caller passing a different span type satisfying `{ text: string }` would silently gain a spurious `cellWidth` property. Consider narrowing the constraint (e.g. `T extends { text: string; cellWidth?: number }`) or extracting the property assignment to a dedicated `RenderSpan`-typed overload so the coupling is explicit.

_{Reviews (1): Last reviewed commit: "perf: precompute render span cell widths" | Re-trigger Greptile}

[greptile-apps]

appendRenderSpan clears cellWidth when either merged span lacks it, but mergeSpan in pierre.ts always preserves the merged width by falling back to measureTextWidth. This asymmetry means that any span flowing through sanitizeTerminalSpans (which nulls out cellWidth when text changes) and then being merged here will cause the combined span to lose its precomputed width and force a later measureTextWidth fallback — the opposite of this PR's goal. Using the same fallback-or-measure pattern as mergeSpan would keep widths cached across the sanitization boundary.

Suggested change

	previous.cellWidth =
	previous.cellWidth !== undefined && span.cellWidth !== undefined
	? previous.cellWidth + span.cellWidth
	: undefined;
	previous.cellWidth =
	previous.cellWidth !== undefined || span.cellWidth !== undefined
	? (previous.cellWidth ?? measureTextWidth(previous.text)) +
	(span.cellWidth ?? measureTextWidth(span.text))
	: undefined;

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/ui/diff/renderRows.tsx
Line: 59-62

Comment:
`appendRenderSpan` clears `cellWidth` when either merged span lacks it, but `mergeSpan` in `pierre.ts` always preserves the merged width by falling back to `measureTextWidth`. This asymmetry means that any span flowing through `sanitizeTerminalSpans` (which nulls out `cellWidth` when text changes) and then being merged here will cause the combined span to lose its precomputed width and force a later `measureTextWidth` fallback — the opposite of this PR's goal. Using the same fallback-or-measure pattern as `mergeSpan` would keep widths cached across the sanitization boundary.

```suggestion
    previous.cellWidth =
      previous.cellWidth !== undefined || span.cellWidth !== undefined
        ? (previous.cellWidth ?? measureTextWidth(previous.text)) +
          (span.cellWidth ?? measureTextWidth(span.text))
        : undefined;
```

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[greptile-apps]

sanitizeTerminalSpans is generic over T extends { text: string }, but the new branch adds cellWidth: undefined — a RenderSpan-specific field — to the spread and hides it behind an as T cast. Any future caller passing a different span type satisfying { text: string } would silently gain a spurious cellWidth property. Consider narrowing the constraint (e.g. T extends { text: string; cellWidth?: number }) or extracting the property assignment to a dedicated RenderSpan-typed overload so the coupling is explicit.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/lib/terminalText.ts
Line: 78

Comment:
`sanitizeTerminalSpans` is generic over `T extends { text: string }`, but the new branch adds `cellWidth: undefined` — a `RenderSpan`-specific field — to the spread and hides it behind an `as T` cast. Any future caller passing a different span type satisfying `{ text: string }` would silently gain a spurious `cellWidth` property. Consider narrowing the constraint (e.g. `T extends { text: string; cellWidth?: number }`) or extracting the property assignment to a dedicated `RenderSpan`-typed overload so the coupling is explicit.

How can I resolve this? If you propose a fix, please make it concise.