fix(desktop): restore emoji recents

[wesbillman]

Summary

• normalize emoji-mart's poisoned literal {} recents state before initialization
• preserve the established idle emoji-index prewarm so first picker open stays responsive
• keep the mounted quick-reaction tray behavior unchanged from main
• cover full-picker same-session recents and deterministically guard the prewarm ordering/callback contract

Validation

• deterministic prewarm test passed and mutation-check failed when idle scheduling was removed
• focused full-picker lifecycle plus existing custom-emoji/tray smoke passed 2/2 on the preceding runtime-equivalent head
• pre-push desktop-check, desktop-typecheck, and full desktop unit suite passed (5,023/5,023) at a80f690413a79a751af6606e8c1c6fbceae6a3cf
• independent Royal Court scope review and headless field test passed the full-picker journey on the preceding runtime-equivalent head

[jedwards27]

Verdict: REQUEST CHANGES

Reviewed: 40f1dac6913d04c87d72610a69ed53bd12377b84..7439c0e85fbcba3a9db2679c5241885cf7c7f0f7 (exact head 7439c0e85fbcba3a9db2679c5241885cf7c7f0f7)

Risk: medium — user-visible picker startup and reaction-control stability, with local persistence scoped by community.

Blocking findings

1. desktop/src/features/messages/ui/useQuickReactionEmojis.ts:248-250,285-301 — immediate reranking breaks the stable hover-tray contract and the required smoke gate. Every successful reaction now invalidates the session cache and broadcasts to all mounted action bars, so the quick buttons move under/after the pointer. The existing journey at desktop/tests/e2e/custom-emoji.spec.ts:404-409 intentionally requires the newly selected :react: not to enter the mounted tray; the base implementation likewise documented “Keep the current hover tray stable.” At this head, both an isolated local run and GitHub job 95870870158 fail deterministically at line 409: expected count 0, received 1 on all attempts. The aggregate Desktop check is therefore red. Keep the mounted tray stable and refresh at a safe lifecycle boundary, or explicitly replace that product contract and add interaction proof that active pointer/focus targets cannot jump.

2. desktop/src/features/custom-emoji/ui/EmojiPicker.tsx:1-19 — restoring recents removes the first-open responsiveness fix. PR #1089 added idle init({ data }) specifically because emoji-mart synchronously builds its ~1.8k-emoji search index on first picker mount and caused the macOS busy-cursor freeze. This patch deletes that prewarm while the picker still initializes on mount at :116-137; the new correctness test does not measure latency. Preserve recents without returning index construction to the first reaction/status/composer open, and add a first-open responsiveness regression that fails when prewarm is removed.

Non-blocking follow-up

desktop/src/features/messages/ui/MessageReactions.tsx:341-347,428-437 records usage before the reaction mutation succeeds. Immediate invalidation makes failed attempts visible as “frequent,” unlike the action-bar/system-message paths that record in .then(). Record only confirmed successes and cover rejection while resolving the blocking tray behavior.

Behavior/contracts traced: emoji-mart persisted Frequent repair and module-global initialization; quick-reaction localStorage/session cache; community-scoped invalidation and listener cleanup; regular/system reaction call sites; existing E2E product contract; VISION.md and TESTING.md. The four-file PR scope is otherwise contained, and no cross-community leak was found in the searched quick-reaction path.

Validation at matching clean HEAD:

• pnpm install --frozen-lockfile && cd desktop && pnpm test — PASS, 5,023/5,023.
• pnpm build:e2e — PASS.
• pnpm exec playwright test tests/e2e/custom-emoji.spec.ts --project=smoke — FAIL, 15 passed / 1 failed; deterministic contract failure above.
• git diff --check 40f1dac6913d04c87d72610a69ed53bd12377b84..HEAD — PASS.
• GitHub run 32186273600: Desktop Core, macOS build, and integration shards pass; Smoke E2E (2) and aggregate Desktop fail.

Manual/native evidence: inspected the exact-head Playwright failure screenshot/video; no native Buzz GUI was launched. Residual risk: macOS first-open latency was not remeasured locally, so the performance regression is established from removal of the dedicated #1089 mitigation and its documented synchronous boundary, not a new timing receipt.

[wesbillman]

Addressed the changes-requested review at 5f83caf71aa6658e4a73d7b4fa0ac21c0179684b.

• Restored the established idle init({ data }) prewarm, preserving first-open responsiveness.
• The poisoned literal {} state is now normalized before that prewarm runs, so eager initialization no longer removes the module-global Frequent category.
• Quick-row runtime and test files remain identical to main; tray stability remains toHaveCount(0).
• Focused picker lifecycle + existing custom-emoji/tray smoke pass 2/2; pre-push check/typecheck/unit suite pass 5,022/5,022.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

[jedwards27]

Verdict: REQUEST CHANGES

Reviewed: 40f1dac6913d04c87d72610a69ed53bd12377b84..5f83caf71aa6658e4a73d7b4fa0ac21c0179684b (exact head 5f83caf71aa6658e4a73d7b4fa0ac21c0179684b)

Risk: medium — local persistence repair shares the initialization boundary that prevents a user-visible first-picker-open freeze.

Resolved from the prior review

• Mounted quick-reaction stability is restored. useQuickReactionEmojis.ts and its unit test are byte-identical to base. recordQuickReactionEmoji now only persists (desktop/src/features/messages/ui/useQuickReactionEmojis.ts:210-232); there is no same-document invalidation, and desktop/tests/e2e/custom-emoji.spec.ts:404-409 again requires the mounted tray to keep zero :react: controls.
• The runtime prewarm is restored. desktop/src/features/custom-emoji/ui/EmojiPicker.tsx:9-39 removes the poisoned literal {}/last state before scheduling the established idle init({ data }), preserving both the Frequent category and the prebuilt emoji search index.

Remaining blocking evidence gap

desktop/tests/e2e/custom-emoji.spec.ts:312-345 does not protect first-open responsiveness. It proves that Frequently used exists and updates, but it has no assertion on the prewarm boundary, long-task behavior, or first-open latency. Deleting EmojiPicker.tsx:22-39 leaves this new regression green; that exact deletion already occurred in this PR and removed the dedicated #1089 busy-cursor mitigation. Add a deterministic guard for the idle prewarm contract or a measured first-open journey with an explicit budget, then mutation-prove that it fails when the prewarm is removed. Source inspection establishes that today's code is repaired; it does not make the performance regression durable.

Non-blocking

• desktop/src/features/messages/ui/MessageReactions.tsx:339-348,428-437 still records before the reaction mutation confirms success, so rejected attempts can enter persisted recents after reload or a cross-document refresh. This is byte-identical to base now and no longer immediately reranks mounted controls; fix as follow-up by recording only confirmed success and covering rejection.
• Refresh the PR summary: it still claims immediate scoped quick-reaction refresh, while the final implementation intentionally preserves the mounted tray and defers that refresh.

Behavior/contracts traced: emoji-mart persisted Frequent repair and module-global initialization; first-open prewarm from #1089; mounted quick-tray/session-cache behavior; cross-document storage listener and cleanup; reaction recording call sites; VISION.md and TESTING.md. Net scope is two Desktop files, with no relay, schema, identity, community-isolation, mobile, or release-runtime change.

Validation at matching clean HEAD:

• cd desktop && pnpm test — PASS, 5,022/5,022.
• pnpm check && pnpm typecheck — PASS; only existing warnings outside this two-file diff.
• pnpm build:e2e — PASS.
• Focused rebuilt smoke for Frequently used plus custom-reaction tray stability — PASS, 2/2 locally.
• Independent rebuilt full custom-emoji.spec.ts --project=smoke --workers=1 — PASS, 16/16; repeated focused rows passed 6/6 after one isolated prior count failure. Exact-head GitHub smoke shards were still completing when this review was submitted; completed shards were green.
• git diff --check 40f1dac6913d04c87d72610a69ed53bd12377b84..HEAD — PASS.

Manual/native evidence: no native Buzz GUI was launched. Residual risk: no exact-artifact macOS first-open timing receipt or mutation-proven prewarm regression exists; that is the requested blocking evidence above.

[wesbillman]

Addressed the remaining review at a80f690413a79a751af6606e8c1c6fbceae6a3cf.

• Added a deterministic startup test for poisoned-state normalization followed by idle init({ data }) scheduling.
• Tied the data consumed by EmojiPicker to that prewarm module, so the contract is on the picker's actual import path.
• Mutation proof: replacing idle scheduling with the fallback timer fails the test (idleCalls.length: expected 1, received 0).
• Avoided a wall-clock picker-latency assertion, which would be environment-sensitive and would not identify where index construction occurred.
• Updated the PR summary to match the final full-picker-only scope.
• Pre-push checks passed at this exact head: desktop check/typecheck and 5,023/5,023 unit tests.

The failed-reaction recents concern remains valid but is byte-identical to main and should be handled as a separate follow-up.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

[jedwards27]

:bot: Jude’s code review agent

Request changes

[P2] Mutation-protect the production picker → prewarm connection

Production is correct at this head: desktop/src/features/custom-emoji/ui/EmojiPicker.tsx:6,108 consumes emojiMartData from emojiMartPrewarm.ts, whose module initialization normalizes storage and schedules idle init({ data }) at emojiMartPrewarm.ts:8-25. The new regression test does not protect that production boundary, however: emojiMartPrewarm.test.mjs:23 imports the helper directly and never imports or observes EmojiPicker or the app entry path.

Two independent reviews mutation-tested this boundary by changing EmojiPicker.tsx back to direct @emoji-mart/data consumption and removing its only prewarm-module import. The claimed regression test still passed 1/1 (exit 0) in both runs. In that mutant, normal app startup no longer evaluates the prewarm module and first-picker synchronous initialization can return while the guard remains green. Both positive controls were effective: disrupting idle scheduling failed the test at the idleCalls.length assertion, and the restored exact-head control passed.

Please add a production-boundary test that imports/renders EmojiPicker and observes prewarm scheduling/data consumption, or a focused architecture assertion tying EmojiPicker to emojiMartPrewarm, then mutation-prove that removing the production import fails. Keep the helper-level test; it correctly covers normalization, ordering, and callback payload.

User consequence: a routine import cleanup can silently restore the macOS first-open busy-cursor freeze—the exact regression this guard is intended to prevent.

Revalidated at a80f690413a79a751af6606e8c1c6fbceae6a3cf

• Mounted-tray stability is resolved: the relevant hook/test and MessageReactions.tsx are unchanged from base; rebuilt custom-emoji.spec.ts passed 16/16.
• The PR summary now accurately states that mounted tray behavior is unchanged and describes prewarm preservation.
• Failed-attempt accounting remains pre-existing, non-blocking debt: MessageReactions.tsx:339-348,428-437 records before mutation confirmation, but this file is unchanged from base and no longer reranks mounted controls.
• Full Desktop units passed 5,023/5,023; pnpm check, pnpm typecheck, production build, E2E build, and git diff --check passed on a clean matching head.
• The exact-head GitHub matrix is green across 24 applicable checks, including Desktop Core, smoke/integration shards, macOS, Windows, units, security, relay E2E, and cross-compiles.
• VISION.md and TESTING.md were reviewed; no scope or product conflict found.

No native GUI was launched. Runtime correctness today is supported by source and rebuilt smoke coverage; the remaining blocker is the regression guard’s failure to cover the production connection.