[LUM-947] Cache sizeThatFits measurements in display text views

[devin-ai-integration]

Memoizes the size measurements produced by three NSViewRepresentable display text views (VSelectableTextView, CodeTextView, VCodeTextView) so cells inside LazyVStack don't rerun NSLayoutManager.ensureLayout(for:) on every scroll/resize pass.

Why this is needed

NSLayoutManager.ensureLayout(for:) is O(n) in glyph count and must run on the main thread. SwiftUI calls sizeThatFits (and the static VSelectableTextView.measureSize) on every visible cell for every LazyVStack layout pass, which means scroll/resize cascades were re-computing the full TextKit layout for every bubble in a long conversation. With hundreds of messages and/or large code blocks the compounded cost stalls the main thread long enough that the sidebar and message list render feels sluggish or momentarily blank.

Mirrors the cache landed for ComposerTextEditor in #26054, adapted for these read-only display paths.

Benefits

• First-pass measurement still runs; subsequent identical queries return in constant time.
• Scroll and window-resize cascades no longer re-run TextKit layout per cell.
• Fixes a latent correctness issue in CodeTextView / VCodeTextView: the previous single-slot cachedHeight only invalidated on text change. When SwiftUI proposed a new wrapping width with the same content the cached height was reused even though it no longer matched the proposal — this keyed cache catches that.

Why it is safe

• Caches are additive: the first measurement for any (content, width) pair always runs the real layout. The cache only short-circuits repeat queries with identical keys.
• Every text-storage mutation site invalidates: applyAttributedString / reset (VSelectableTextView), updateNSView text replacement / textDidChange (CodeTextView), applyText and the async syntax-highlight completion (VCodeTextView).
• VSelectableTextView.MeasurementKey retains the NSAttributedString so Dictionary equality falls through to NSAttributedString.isEqual(_:), avoiding any risk that hash collisions surface as wrong-height cells. Memory is bounded by the 256-entry cap.
• No change to text rendering or text selection — only the ensureLayout + usedRect pair is elided.

References

• NSLayoutManager.ensureLayout(for:)
• NSViewRepresentable.sizeThatFits(_:nsView:context:)
• NSAttributedString.isEqual(_:)
• WWDC21 — What's new in AppKit — TextKit 2 / TextKit 1 interoperability; these views continue to pin a TextKit 1 stack because TextKit 2 is not yet a drop-in replacement for attributed-string measurement inside a representable.
• Reference implementation: fix(macos): coalesce ComposerTextEditor.measureHeight and skip redundant ensureLayout passes (LUM-827) #26054 (LUM-827, ComposerTextEditor).

Alternatives considered and rejected

• Port to TextKit 2 / NSTextLayoutManager. More invasive, introduces a separate set of correctness risks (selection, link handling, line-break semantics), and would not by itself memoize repeat queries from SwiftUI. Caching measurements is orthogonal and composes with a future TextKit 2 migration.
• Single shared NSCache across the three views. Rejected because the cache keys and invalidation points are view-specific — VSelectableTextView can invalidate on attribute changes via isEqual(_:), while CodeTextView / VCodeTextView rely on explicit invalidation at known mutation sites and use the cheaper (textStorage.length, width) key. Sharing would force the lowest-common-denominator key and make invalidation harder to reason about.
• Precompute from the SwiftUI layer with .frame(width:height:). Already supported in VSelectableTextView via useExternalSizing; the caller in MarkdownSegmentView uses it. The instance-level cache is there to protect the useExternalSizing = false path and the static measureSize call.
• DispatchWorkItem-based coalescer from fix(macos): coalesce ComposerTextEditor.measureHeight and skip redundant ensureLayout passes (LUM-827) #26054. Intentionally omitted: sizeThatFits must return a size synchronously, so debouncing into a later runloop turn would return stale or default sizes and break layout. Only the skip-guard half of the reference pattern applies here.
• LRU eviction on the static cache. Bulk eviction when the cap is hit was preferred over LRU bookkeeping (which adds per-entry cost on every read) because the working set in a single conversation is small relative to the cap and rehydration is cheap.

Root cause analysis

How the code got here

• VSelectableTextView landed with a shared measurement NSLayoutManager but no memoization. Its intended use via measureSize was to precompute a size from the SwiftUI side once per bubble — the caller in MarkdownSegmentView does have its own NSCache, which masked the missing internal cache. The instance-level sizeThatFits path (used when the caller does not opt into useExternalSizing) had no cache at all.
• CodeTextView / VCodeTextView did have a single-slot cachedHeight, which worked while they lived in contexts that handed them a fixed width (file viewer panels). Once they were composed into chat message bubbles streaming inside a LazyVStack, proposed widths started oscillating (streaming layout, window resize, typing in the composer), and the cache silently stopped helping.

Mistakes / decisions that led to it

• Caches were designed against the containing view's behavior at the time of writing, not against the NSViewRepresentable contract that SwiftUI re-queries size on every layout pass.
• The cache keys were incomplete: cachedHeight did not key on width.
• fix(macos): coalesce ComposerTextEditor.measureHeight and skip redundant ensureLayout passes (LUM-827) #26054 identified and fixed the same pathology for ComposerTextEditor but the fix was applied to one call site instead of generalized to the sibling display views.

Warning signs we missed

• The same-length-different-width cache hit was a silent bug — it produced a stale size but still rendered text, so the symptom was only "things feel slow in long conversations," not a visible regression.
• fix(macos): coalesce ComposerTextEditor.measureHeight and skip redundant ensureLayout passes (LUM-827) #26054 should have been a prompt to audit the other ensureLayout(for:) call sites. There was no cross-reference.

Prevention

1. When fixing a layout-perf issue in an NSViewRepresentable, grep for the same API in sibling representables before closing the ticket.
2. Cache keys that involve width must include width explicitly. A cachedX field without a composite key is a smell in any sizeThatFits path.
3. For any NSViewRepresentable that hosts a TextKit stack and is placed inside a cell type (LazyVStack, LazyHStack, List, ScrollView), sizeThatFits must memoize.

clients/AGENTS.md guideline (committed in this PR)

Added as a bullet under § "View Bodies and Rendering" and as a row in the pitfalls table so future changes to any TextKit-backed NSViewRepresentable inherit the expectation:

NSViewRepresentable views that host a TextKit stack and are placed inside LazyVStack / LazyHStack / List cells must memoize sizeThatFits output. NSLayoutManager.ensureLayout(for:) is O(glyph count) and runs on the main thread; SwiftUI re-queries size on every layout pass. Cache the last-measured size keyed on at least (textStorage.length, proposalWidth) (or NSAttributedString.isEqual(_:) for attribute-sensitive paths) and invalidate at every text-storage mutation site.

Test plan

• Manual Xcode build against main — verified locally (warnings limited to unrelated VAvatarImage.swift actor-isolation, no errors in the three modified files).
• Text rendering and selection in VSelectableTextView unchanged — only ensureLayout + usedRect are elided on cache hits; the first measurement and every text-mutation path still re-runs layout.
• Cache invalidation audited per file: applyAttributedString / reset (VSelectableTextView), updateNSView text replacement / textDidChange (CodeTextView), applyText plain apply + async highlight completion (VCodeTextView).

Note on CI

macOS / iOS builds are skipped in CI for this repo. Xcode build verified locally against main before merging.

Human review checklist

• Cache invalidation in applyAttributedString, reset, updateNSView, textDidChange, applyText, and the async highlight completion are each correct for their view.
• MeasurementKey equality via isEqual(_:) is the right identity for attribute-sensitive paths (e.g. markdown styling changes).
• 256-entry cap + bulk eviction is acceptable given the working-set size you expect in a single long conversation.
• clients/AGENTS.md addition reads as a durable pattern-level rule, not a change note.

Link to Devin session: https://app.devin.ai/sessions/97bf309bde854c228a2ff11fb059a459
Requested by: @ashleeradka

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment and CI monitoring

[linear]

LUM-947 Perf: SelectableTextView / HighlightedTextView / VCodeView unguarded ensureLayout in LazyVStack cells

[vex-assistant-bot]

Approved ✦

Solid performance optimization — mirrors the battle-tested pattern from #26054 (ComposerTextEditor) and extends it to all three display-path ensureLayout call sites. The approach is sound: skip-guards keyed on (textStorage.length, width) for instance-level sizeThatFits, plus a bounded FIFO cache for the static measureSize path.

What I verified:

Cache invalidation completeness:

• SelectableTextView: applyAttributedString and reset both call invalidateMeasurementCache() — these are the only two mutation paths for read-only display cells. ✅
• VCodeView (VCodeTextView): applyText invalidates on plain text set, and the async syntax-highlight completion invalidates again (font variants can change line height). Search highlights only modify .backgroundColor attributes, not text content, so they correctly don't invalidate. ✅
• HighlightedTextView (CodeTextView): updateNSView text replacement and textDidChange both invalidate. ✅

Cache key safety:

• textStorage.length as key: length-collision with different content is theoretically possible but moot — all content changes flow through methods that explicitly invalidate. These are read-only display views; users can't type into them.
• Static measureSize cache uses MeasurementKey with custom Hashable that falls through to NSAttributedString.isEqual(_:) for equality — hash collisions can't produce wrong heights. Good defensive design.

FIFO eviction in measureSize:

• Cliff eviction at 256 entries (removeAll then re-add). Simple, bounded memory, and the PR description correctly notes "Dropping is cheap and avoids per-entry bookkeeping; the next measurement rehydrates the working set." For a display cache in a scrolling list, the working set is small relative to the limit. Fine.

Width tracking upgrade:

• The old cachedHeight in VCodeView/HighlightedTextView only invalidated on text change, not width change. This PR correctly keys on (length, width) — fixes silent degradation when proposed widths oscillate during streaming/resize. This was the actual bug the old cache missed.

Minor note (non-blocking):

• The measurementSizeCache is static on VSelectableTextView — it persists for the app lifetime. The 256-entry FIFO bound keeps this reasonable, but worth noting that NSAttributedString references are retained in the dictionary keys. For long conversations, after the cliff eviction the old strings become eligible for release, so no leak concern.

Clean extension of an established pattern to the remaining call sites. Ship it.

[vex-assistant-bot]

@devin review this PR

[vex-assistant-bot]

@devin review this PR

[vex-assistant-bot]

Re-approved after reviewing the three new commits. ✦

Post-review changes address Devin Review feedback cleanly:

1. isEqual for cache keys — MeasurementKey uses NSAttributedString.isEqual instead of hash alone, eliminating collision risk for wrong-height cells.
2. invalidateMeasurementCache() helper — replaces old cachedHeight=nil with atomic 3-field reset, consistent across all 3 files.
3. Comment rephrasing — standalone descriptions, no PR iteration references.

Core approach unchanged and solid. All invalidation points verified across HighlightedTextView, SelectableTextView, and VCodeView. Ship it.

[vex-assistant-bot]

@devin review this PR