perf(design-system): hoist NSColor bridging out of adaptiveColor hot path (LUM-709)

[ashleeradka]

Prompt / plan

Investigate LUM-709 (App Hang during view rendering — culprit adaptiveColor), which Sentry attributes to the dynamic-provider closure in clients/shared/DesignSystem/Tokens/ColorTokens.swift — the largest active hang in the macOS backlog (33 events / 13 users on vellum-macos@0.8.0, last seen 2026-05-11). LUM-468 is the duplicate-fingerprint partner where the watchdog catches _allocateUninitializedArray<T> inside the same closure.

After investigation we considered: (A) asset-catalog migration, (B) hoist out of the closure, (C) drop bestMatch for appearance.name, (D) \.colorScheme-env per view, (E) notification-observer cache, (F) custom ShapeStyle. Recommendation approved: Option B — targeted hot-path fix without disturbing VTheme.applyTheme's NSApp.appearance-based theme machinery or the ~3,926 VColor.X consumer call sites. Asset catalog remains the right long-term Apple-recommended pattern but is a separate workstream.

What changed

adaptiveColor(light:dark:) previously did three things on every closure invocation:

1. allocated a fresh [NSAppearance.Name] literal ([.darkAqua, .aqua]) — the _allocateUninitializedArray<T> leaf in LUM-468,
2. called appearance.bestMatch(from:),
3. ran NSColor(SwiftUI.Color) on whichever branch matched — the NSColor thunk culprit in event d40d12be….

AppKit invokes the dynamic provider whenever any method on the color needs component values (currentDrawingAppearance, alphaComponent, cgColor, etc., per Apple's NSColor.init(name:dynamicProvider:) docs) — i.e. on every SwiftUI render/layout pass that touches a design token. With ~56 adaptive tokens × multiple component queries per token per render pass, this dominates main-thread time and crosses the 2-second app-hang watchdog.

This PR:

• Moves the match-list literal to a module-level private let, so the closure no longer allocates a fresh [NSAppearance.Name] per invocation.
• Pre-bridges the light/dark Color arguments to NSColor once at token-init time, each under the appropriate drawing appearance via NSAppearance.performAsCurrentDrawingAppearance(_:). The closure body reduces to one bestMatch + ternary picking between two captured references — the shape Apple's NSColor.init(name:dynamicProvider:) docs use as the canonical example.
• Wraps the pre-bridge in performAsCurrentDrawingAppearance so that MeadowTokens entries which compose nested adaptive arguments (VColor.surfaceOverlay.opacity(…), VColor.surfaceBase.opacity(…), VColor.surfaceActive.opacity(…)) still resolve their inner dynamic providers in the correct branch's appearance instead of whatever ambient appearance happens to be set at module-init time.

Diff is +29/-3 in one file.

Why this is safe

• VTheme.applyTheme(_:) mutates NSApp.appearance (and each window's appearance) to implement the user's Light/Dark/System override. That mechanism propagates the chosen appearance into currentDrawingAppearance at draw time, which the (still-dynamic) outer NSColor consumes via bestMatch. Behavior is preserved end-to-end — per-window dark mode, system theme switching, and the user's manual setting all still work.
• The pre-bridged NSColors are immutable and captured by reference in the closure. NSColor is documented thread-safe.
• For the 53 plain tokens in ColorTokens.swift (whose arguments are Color(hex:) / Color(.sRGB, …) — appearance-independent), the pre-bridge produces the same NSColor regardless of drawing appearance.
• For the 3 nested-adaptive MeadowTokens entries (panelBackground, panelBorder, and the captionText egg-glow pair), performAsCurrentDrawingAppearance ensures each branch resolves its inner dynamic provider under the right appearance.
• No new caches with mutable state, no cross-@Observable-store reads, no new Layout types — none of the failure modes from recent perf PRs in this area (perf(layout): override explicitAlignment to stop O(n×depth) cascade in custom Layout wrappers (LUM-1167) #28691 LUM-1167, Cache derived properties on ConversationListStore to eliminate observation cascades #29898) apply.
• Wire/protocol impact: none. This is a purely internal SwiftUI render-path change inside the macOS client. No IPC payloads, gateway endpoints, SSE schemas, or NotificationCenter contracts are touched — safe under both old macOS + new platform and new macOS + old platform skews.

Test plan

• CI: bunx tsc, repo lint, design-token guardrail (clients/scripts/check-design-tokens.sh — passes locally; we don't change any token values).
• Local Xcode build required before merge — the repo has no macOS build in CI (no Xcode runner), so public-API access-level issues and AppKit/SwiftUI compile errors only surface in a local build. The diff is small and access levels are unchanged (adaptiveColor stays public, new helpers are private).
• Visual verification in Component Gallery under both Light and Dark appearance, plus toggling VTheme.applyTheme("light"/"dark"/"system") while the gallery is open, to confirm:
  • All token swatches render at their previous values.
  • MeadowTokens.panelBackground / panelBorder / captionText still pick the right VColor.surface* opacity in dark mode (these are the nested-adaptive cases the performAsCurrentDrawingAppearance wrapper is designed to preserve).
• Post-merge Sentry validation: monitor LUM-709 fingerprint and LUM-468 volume on the next release. The recommendation memo predicts a ≥30% drop on the next release if the closure was the dominant cost. If volume stays flat, the closure was hot but not dominant, and the bigger architectural follow-up (asset-catalog migration) should be scoped.

Root cause analysis

1. How did the code get into this state? The function was introduced in #2957 ("feat: add light theme support, redesign onboarding, and use full-window panels", 2026-02-15, Anita Kirkovska) when the design system gained light-theme support. The closure body is functionally correct — it just inlines the per-call work that Apple's documented example does once. Untouched since.

2. What mistakes or decisions led to it? Apple's NSColor.init(name:dynamicProvider:) docs say the value is "calculated on first use," which reads as if AppKit will cache. The clarification underneath ("When methods on a color need component values, AppKit calls the provider with currentDrawingAppearance") is easy to miss — and in practice means the closure is the SwiftUI render hot path. The original implementation reasonably assumed it would only fire on theme changes.

3. Were there warning signs we missed? LUM-468 (_allocateUninitializedArray<T> inside adaptiveColor's closure) has been live since at least 2026-03; the team treated it as a duplicate of LUM-709 once both surfaced. The _allocateUninitializedArray<T> leaf was the strongest signal that the closure body was running far more often than expected — anything per-call allocating on the render hot path warrants a closer look.

4. What can prevent recurrence? The pattern to internalize: bodies of NSColor(name:dynamicProvider:) / UIColor(dynamicProvider:) closures should be trivially fast (precomputed-reference selection only) because AppKit/UIKit invoke them on every component-getter call during draw — they are not memoized for you. The same rule applies to any other AppKit/UIKit "provider" closure that AppKit calls during layout/render (e.g. NSTextStorage attribute providers, CAMetalLayer.drawableSize blocks).

5. Should we add a guideline to clients/AGENTS.md? Yes — a one-sentence note in the existing "Performance and Resource Management → View Bodies and Rendering" section is enough, since that section already establishes the "view-graph hot path must stay trivial" principle. Proposed addition (not in this PR; happy to fold in if reviewers want):

   Dynamic NSColor / UIColor provider closures are part of the render hot path. AppKit/UIKit invoke NSColor(name:dynamicProvider:) and UIColor(dynamicProvider:) blocks on every component-getter call during draw, not only on appearance change. Hoist allocations and NSColor(SwiftUI.Color) bridging out of the closure and capture precomputed NSColor references — the body should be a trivial branch on appearance. Ref: Apple NSColor.init(name:dynamicProvider:).

Apple refs checked (2026-05-11):

• NSColor.init(name:dynamicProvider:)
• NSAppearance.performAsCurrentDrawingAppearance(_:)
• NSAppearance overview
• Color overview

Closes LUM-709
Resolves duplicate fingerprint LUM-468

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

---

[linear]

LUM-709

[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

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

[vex-assistant-bot]

Value

Kills the largest active hang in the macOS backlog (LUM-709, 33 events / 13 users, v0.8.0). Targeted +29/-3 hot-path fix in one file that closes the watchdog gap without touching VTheme.applyTheme's NSApp.appearance machinery or any of the ~3,752 VColor.* consumer call sites. LUM-468 is the duplicate-fingerprint partner (same _allocateUninitializedArray<T> in the same closure) and gets cleared by the same edit. Approach (Option B) is correctly chosen over the asset-catalog migration, which remains the right long-term Apple-recommended pattern but is a separate workstream.

Verification

Claim	Verified
~56 adaptive tokens	✅ exact — 53 in ColorTokens.swift + 3 in MeadowTokens.swift
Originating PR #2957	✅ confirmed via git log --follow
adaptiveColor scope	✅ only clients/shared/DesignSystem/Tokens/{ColorTokens,MeadowTokens}.swift
3 nested-adaptive Meadow entries	✅ panelBackground, panelBorder, captionText — all compose VColor.X.opacity(…)
Design-token CI guard compatibility	✅ Rule B excludes shared/DesignSystem/Tokens/; new private helper resolveSwiftUIColor lives inside that scope
Apple NSColor.init(name:dynamicProvider:) semantics	✅ closure is invoked per component-getter call (alphaComponent, cgColor, etc.), making it a render hot path
Per-window appearance overrides preserved	✅ outer closure still runs under the drawing context's appearance, bestMatch selects between the two captured NSColor refs

Why the pre-bridge is safe

• 53 plain tokens (light/dark args are Color(hex:) / Color(.sRGB, …)): appearance-independent. Pre-bridging produces the same NSColor regardless of which performAsCurrentDrawingAppearance we wrap it in.
• 3 nested-adaptive Meadow tokens: the Color → NSColor bridge is what triggers each nested VColor.X's own dynamic provider. performAsCurrentDrawingAppearance(.aqua) for the light branch forces that nested resolution under the right appearance; .darkAqua for the dark branch. Without the explicit wrapper, the nested provider would resolve under whatever ambient appearance happened to be set at module-init time — the silent correctness bug this PR avoids.

Memory footprint is negligible (~56 × 2 = 112 NSColor instances retained, vs. on-demand resolution before).

Anti-pattern sweep

Clean against data/codebase/anti-patterns.md and data/codebase/swiftui-patterns.md. This PR is the removal of a hot-path allocation pattern, not the introduction of one. Worth distilling into a new entry alongside the existing repeatForever / motionVectors rules:

AppKit/UIKit dynamic color providers are a SwiftUI render hot path. NSColor.init(name:dynamicProvider:)'s closure runs on every component-getter (alphaComponent, cgColor, currentDrawingAppearance, …), which means per-render-pass × N tokens. Keep the closure body trivial — no allocations, no NSColor(SwiftUI.Color) thunks, no bestMatch(from:) array literals. Pre-bridge at init time and capture references.

Happy to land that as a follow-up to anti-patterns.md + the AGENTS.md note this PR proposed.

Concurrency / cross-platform

• No async/concurrent code touched. NSColor is documented thread-safe. performAsCurrentDrawingAppearance(_:) runs the closure synchronously.
• macOS-only (AppKit). No iOS / web blast radius.

Non-blocking observations

1. resolveSwiftUIColor uses var resolved: NSColor! and relies on the (documented) synchronous semantics of performAsCurrentDrawingAppearance. Defensively this could be var resolved: NSColor? with return resolved ?? NSColor(color), but the current form matches Apple's own examples and isn't a correctness risk.
2. NSAppearance(named:) lookup happens twice per token at init time — could be cached at module scope like the match list, but it's init-time work so the perf delta is invisible.

Merge gate

• Vex ✅ this review
• Devin Review: "No Issues Found" ✅
• Codex 👍 on PR description ✅
• CI: Socket Security ×2 ✅, FlexFrame Lint ✅; macOS Build/Tests SKIPPED (expected for clients/shared/ paths)
• PR description ✅ — options considered, alternatives ruled out, safety analysis, Apple docs cited

Boss QA on a local build is the gating factor. Validation steps:

1. cd ~/Development/vellum-assistant/clients/macos && ./build.sh clean && ./build.sh run
2. Visual sweep across light + dark mode (window-level + system-level appearance flips) to confirm Meadow onboarding panels still render correctly under each branch.
3. Instruments → Time Profiler over a long ChatView session — _allocateUninitializedArray<T> should disappear from the closure leaf, render-pass main-thread time on dynamic colors should drop.
4. Confirm [adaptiveColor:33] and _allocateUninitializedArray Sentry signals go quiet on the cherry-picked build.

Merge-ready once Boss QA passes. ✦