Skip to content

perf(renderer): reduce UI runtime churn#572

Open
JustYannicc wants to merge 9 commits into
SuperCmdLabs:mainfrom
JustYannicc:codex/consolidate-ui-runtime-wave2
Open

perf(renderer): reduce UI runtime churn#572
JustYannicc wants to merge 9 commits into
SuperCmdLabs:mainfrom
JustYannicc:codex/consolidate-ui-runtime-wave2

Conversation

@JustYannicc

Copy link
Copy Markdown
Collaborator

What changed

  • Avoid building the linear list virtual-row array while List is rendering the emoji-grid layout, and keep emoji-grid keyboard selection visible by scrolling the rendered selected cell.
  • Move useForm error updates through guarded form error-map helpers so no-op clears and same-value validation errors keep object identity.
  • Cache inline emoji picker matches from the current rendered query so left/right navigation and enter/tab insertion do not rescan emoji data.
  • Add focused harnesses for the clean-base list-row adaptation, useForm no-op error churn, and inline emoji picker match caching.
  • Add the small guarded form error helper module required for the useForm change to be self-contained on origin/main, and type the list-detail fragment props that focused LSP diagnostics surfaced in the touched file.

Why

These paths sit in renderer/runtime hot loops: large emoji-heavy Raycast lists, form field changes, validation writes, and inline emoji picker navigation. The consolidation removes repeated work while keeping the current Raycast-compatible behavior and avoiding the unrelated integration-stack commits carried under the smaller source branches.

Compatibility impact

Raycast list, form, and inline emoji picker APIs are unchanged. Normal list virtualization still builds and renders the same flat rows. Emoji-grid mode still renders the same grouped items and now scrolls the actual selected rendered cell into view instead of relying on discarded linear-row layout data. Form errors still clear on value changes and publish changed validation errors; no-op cases now return the previous error map. Emoji picker match order, wraparound navigation, zero-match hide, insertion, and cache clearing on hide are preserved.

How tested

  • node --test scripts/test-list-conditional-virtual-rows.mjs scripts/test-use-form-error-state-noop.mjs scripts/test-inline-emoji-picker-match-cache.mjs passed: 7 tests, 0 failures.
  • node scripts/test-list-conditional-virtual-rows.mjs --report passed and emitted the row-work report below.
  • node scripts/test-use-form-error-state-noop.mjs --report passed and emitted the identity-churn report below.
  • ./node_modules/.bin/tsc -p tsconfig.main.json --noEmit --pretty false passed.
  • ./node_modules/.bin/tsc -p tsconfig.renderer.json --noEmit --pretty false failed with exit code 2 on existing upstream-main renderer errors outside this change, including src/renderer/src/App.tsx, CameraExtension.tsx, LiquidGlassSurface.tsx, useLauncherCommandModel.ts, useLauncherKeyboardControls.ts, i18n/runtime.ts, QuickLinkManager.tsx, action-runtime-overlay.tsx, context-scope-runtime.ts, detail-runtime.tsx, use-cached-promise.ts, icon-runtime-phosphor.tsx, index.tsx, list-runtime-renderers.tsx, oauth/with-access-token.tsx, settings/AITab.tsx, settings/StoreTab.tsx, SnippetManager.tsx, and utils/quicklink-icons.tsx. The prior focused list-runtime.tsx diagnostic was fixed in this branch.
  • Codex LSP error diagnostics: no diagnostics for src/main/main.ts, src/renderer/src/raycast-api/hooks/use-form.ts, src/renderer/src/raycast-api/form-runtime-state.ts, or src/renderer/src/raycast-api/list-runtime.tsx.
  • git diff --check origin/main..HEAD passed.

Performance evidence

  • List conditional row harness: 5,000 emoji items previously built 5,001 discarded linear rows, now 0; 20,000 emoji items previously built 20,001 discarded linear rows, now 0. Normal list row counts are preserved.
  • useForm no-error field changes: 10/100/1000 changes previously produced 10/100/1000 error-object identity changes; after this change all three cases produce 0. Same-error validation sets keep identity, changed errors publish, and clearing an existing field preserves unrelated errors.
  • Inline emoji picker harness with 1,870 emoji entries, query smi, 500 sequences of 20 right-arrow steps: legacy model made 20,000 search calls in 2515.564 ms total; cached model made 500 search calls in 61.619 ms total; 40x search-call reduction with matching checksum 113000.

Stack validation

This branch starts from upstream origin/main at 9bdd3d2a99cbb30a7688e85784c3464a706ae7b0. It carefully applies only the scoped top changes from #559, #562, and #567, plus the minimal helper/type/test adjustments needed to make those changes self-contained and validated on the clean base. It does not add coordinator notes, prompt files, local playbooks, or generated package-manager metadata.

Replaces

@shobhit99

Copy link
Copy Markdown
Contributor

Reviewed the full diff and traced each hot-path change against its consumers. This is a tight, behavior-preserving perf pass that also fixes a latent emoji-grid scroll bug.

Strengths

  • list-runtime.tsx flat-row short-circuit is safe and correct. flatRows is only consumed by the linear virtualization branch (rowMetrics, visibleStart/End, itemIdxToRowIdx, and the flatRows.slice() render at ~L463). The emoji-grid branch (~L419) renders groupedItems directly, so returning [] in grid mode discards only unused work. Adding shouldUseEmojiGridValue to the memo deps is right, and since it's a memoized boolean the value-compare is stable.
  • The emoji-grid scroll branch is a genuine bug fix, not just perf. Previously the scroll-into-view effect always used itemIdxToRowIdx + rowMetrics.offsets (36px linear-row math) even in grid mode, which scrolls to the wrong pixel offset for an 8-col/96px grid. The new querySelector('[data-idx="${selectedIdx}"]')?.scrollIntoView({ block:'nearest' }) targets the actual rendered cell. Verified ListEmojiGridItemRenderer emits data-idx={dataIdx} (list-runtime-renderers.tsx:132) with dataIdx={globalIdx} and globalIdx === selectedIdx for the selection, so the selector resolves. Mirrors the existing grid-runtime.tsx:195 pattern; behavior:'auto' matches the linear path's anti-jitter choice.
  • main.ts emoji-match cache is consistent. emojiPickerCurrentMatches is set alongside emojiPickerCurrentQuery in renderEmojiPicker (the only place the query changes) and cleared in hideEmojiPicker, so the cached array is always exactly what was rendered. Nav never mutates the query, so updateEmojiPickerSelection/handleEmojiTriggerNav reading the cache is equivalent to (and slightly more correct than) re-searching. The enter/tab path captures pick, queryLen, prefixLen into locals before hideEmojiPicker() clears the cache, so insertion is unaffected. 40x fewer scans on held arrow keys.
  • useForm identity guards are behavior-preserving. clearFormFieldError returns prev when the field has no error (no-op) and otherwise clones+deletes exactly as before; setFormFieldError returns prev on same-value writes. Truthy-narrowing keeps the err arg typed string. validate()/reset() still replace the whole map. The Fragment detail prop cast (~L393) is a pure type narrowing.

Concerns / potential bugs

  • None blocking. I could not construct a stale-UI, dropped-condition, or leak scenario: no effect cleanup or listeners were touched, and every consumer of the short-circuited/cached values takes the correct branch.
  • Minor calibration: the setValue error-identity guard yields no render savings on its own, because setValues still allocates a new object every keystroke and the itemProps memo depends on values, so itemProps recomputes regardless. The guard saves an allocation (and matters for the setValidationError/onBlur same-error case, which does avoid a re-render). Perf claim is real but the setValue portion is GC-only.

Suggestions (optional)

🟡 Approve with minor comments

🤖 Automated review by Claude Code (Opus 4.8).

@JustYannicc

JustYannicc commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the remaining actionable review suggestion from the top-level feedback.

What changed:

  • Replaced brittle exact source-substring assertions in scripts/test-list-conditional-virtual-rows.mjs with TypeScript AST checks for the flatRows emoji-grid short-circuit, dependency array wiring, and selected rendered-cell scroll behavior.
  • Replaced the useForm harness string/regex source checks in scripts/test-use-form-error-state-noop.mjs with TypeScript transpilation plus AST checks for guarded helper imports and setErrors updater usage.
  • Kept runtime code unchanged; this is scoped to hardening the new harnesses against harmless formatting changes.

Review-thread state checked:

  • GraphQL reviewThreads: 0 threads.
  • Submitted reviews: 0.
  • Only top-level feedback found was the optional harness brittleness suggestion plus non-code merge-order/wording calibration.

Verification:

  • node scripts/test-list-conditional-virtual-rows.mjs --report ✅ 1 passed; 5,000 emoji items avoid 5,001 discarded linear rows and 20,000 avoid 20,001, with normal row counts preserved
  • node scripts/test-use-form-error-state-noop.mjs --report ✅ helper AST wiring found; 10/100/1000 no-error changes now produce 0 error-map identity changes
  • npm exec -- tsc -p tsconfig.main.json --noEmit --pretty false
  • git diff --check origin/main..HEAD

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants