Fix performance of search router in p95 tail by sumo-slonik · Pull Request #98282 · Expensify/App

sumo-slonik · 2026-08-11T11:40:36Z

Explanation of Change

Cuts the ManualOpenSearchRouter p95 tail (Sentry: "Opening Search bar"). Android is 12% of samples but 53% of everything above the global p95, with cold_start=false in 99.6% — the tail is the session's first open landing in post-launch JS-thread contention.

New SearchRouterWarmup/SearchRouterOptionsWarmer, mounted from AuthScreens, move that first-open work off the critical path. After OpenApp is applied, the first idle window (Scheduler.scheduleWhenIdle) evaluates the SearchRouterPage/RightModalNavigator module graphs and builds the empty-query option list into createFilteredOptionList's cache. That cache is keyed on the identity of its Onyx inputs, so the list is only built once they hold still across an idle window — otherwise post-launch writes would immediately invalidate it. The warmer unmounts when done, and skips entirely if the router is already open.

The config those options are built from now lives in searchRouterOptionsConfig.ts, so the warmer and SearchAutocompleteList can't drift apart on the values that cache is keyed on.

An earlier revision also skipped the autocomplete parser for blank queries. Dropped — parsing is 0.07ms per call, and the rest of the delta was a dev-only Hermes compile cost (see the discussion on useAutocompleteSuggestions.ts).

Local measurements (dev builds — absolute ms are not production values, only the relative change matters)

iOS

median main	median branch	delta
940ms	736ms	-204ms (-22%)

Android

median main	median branch	delta
1323ms	1108ms	-215ms (-16%)

Fixed Issues

$ #79353
PROPOSAL:

Tests

Verify that no errors appear in the JS console

Open the app and tap Search — the router opens and shows recent searches and recent reports.
Type a query — autocomplete suggestions appear and the results match the query. Clear the query — the recent searches/reports list comes back.
Tap a recent report — it opens the right report.
Go back to Home, leave the app untouched for half a minute, then tap Search again — it opens with the same list, nothing missing or duplicated.
Tap Search immediately after the app finishes loading, before it has been idle — the router still opens correctly with the same list.
Sign out and sign back in, then tap Search — the router opens with the new account's list.

Offline tests

Unnecessary

QA Steps

Same as tests.

PR Author Checklist

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

…empty

codecov · 2026-08-11T11:45:16Z

Codecov Report

❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.

Files with missing lines	Coverage Δ
src/components/Search/SearchAutocompleteList.tsx	`92.22% <ø> (-0.08%)`	⬇️
...s/Search/SearchRouter/searchRouterOptionsConfig.ts	`100.00% <100.00%> (ø)`
src/libs/Navigation/AppNavigator/AuthScreens.tsx	`82.97% <100.00%> (-0.18%)`	⬇️
...Navigation/AppNavigator/searchRouterLazyLoaders.ts	`100.00% <100.00%> (ø)`
.../Search/SearchRouter/SearchRouterOptionsWarmer.tsx	`96.66% <96.66%> (ø)`
...ponents/Search/SearchRouter/SearchRouterWarmup.tsx	`92.30% <92.30%> (ø)`
...onents/Search/SearchRouter/SearchRouterContext.tsx	`79.48% <66.66%> (-2.34%)`	⬇️
...rch/SearchRouter/SearchRouterPage/index.native.tsx	`15.38% <0.00%> (-1.29%)`	⬇️
... and 15 files with indirect coverage changes

Replace the 15s timer in SearchRouterWarmup with the state it was approximating: wait for OpenApp to be applied, then for an idle window on the JS thread via Scheduler.scheduleWhenIdle. Build the option list only once its Onyx inputs hold still across a full idle window. createFilteredOptionList keys its cache on the identity of those inputs, so a list built while post-launch writes are still landing is discarded before the user can open the router. A connectWithoutView counter tracks that churn without rendering or rebuilding anything, and a cap on the number of idle windows keeps a chatty account from postponing the build forever. Module evaluation is unaffected by Onyx writes, so it still happens in the first idle window. Also: - evict the attachment-parse cache LRU instead of clearing every entry at the cap - share the useFilteredOptions config between SearchAutocompleteList and SearchRouterOptionsWarmer so the cache key cannot drift apart - share the SearchRouterPage/RightModalNavigator require paths between AuthScreens and the warmup - pick the empty-search predicate once per getValidOptions call instead of testing for it per report

Measured on the SearchRouter open path with the changes stacked one at a time (Android emulator, high-traffic account, 3 cold opens per state): adding this cache moved the median from 1840ms to 1936ms, i.e. no gain, with the run ranges fully overlapping. The reason is visible in the data: the cache lookup sits behind the `data-expensify-source` fast path, and on the test account only 4 of 635 reports carry that attribute, so the parser barely runs on this path at all. Keeping the cache would mean retaining message HTML strings for a saving that does not exist here. If it is worth having, it belongs in a change aimed at the report view, justified by measurements of that path.

Same stacked measurement as the previous commit: adding this step moved the median from 1957ms (main) to 1985ms, with the run ranges fully overlapping main's, so it buys nothing on the SearchRouter open path. Building and deburring the search text for an empty query is genuinely redundant, but at this size it does not show up next to the option-list build that dominates the open, so it is not worth the review surface in a perf PR.

… flag

staszekscp

So far looking great!

The blank-query short-circuit skipped the parser's first-use module evaluation, which measured at ~76ms on an Android dev build. That cost is Hermes compiling the generated parser at runtime, which a release build does not pay - its bundle is precompiled to bytecode. The parsing itself is 0.07ms per call, so the guard is not worth the extra branch.

melvin-bot · 2026-08-17T10:45:11Z

@aimane-chnaif Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6864931e12

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-08-17T10:49:15Z

    const openSearchRouter = (query?: string, isFromSearchPageSearchButton?: boolean) => {
+        isSearchRouterOpenOrOpening = true;


Reset the open flag when native navigation dismisses the route

When the native Search Router is dismissed through Android hardware Back or an iOS navigation gesture, React Navigation removes SCREENS.SEARCH_ROUTER.ROOT without calling closeSearchRouter, so this module-level flag remains true until sign-out. The native useSyncModalWithHistory is a no-op, and the route's beforeRemove listener in AuthScreens.tsx only updates modal state and cancels requests. Consequently, if the user opens Search before the initial idle warmup and dismisses it through navigation—or if a later reconnect needs to rewarm invalidated data—both warmers keep treating the closed router as open and skip the optimization for subsequent opens.

Useful? React with 👍 / 👎.

github-actions · 2026-08-17T10:49:59Z

+import {getIsSearchRouterOpenOrOpening} from './SearchRouterContext';
+
+type OptionsWarmerProps = {
+    onDone: () => void;


❌ CONSISTENCY-13 (docs)

The newly added OptionsWarmerProps type declares the onDone prop with no /** ... */ block comment. Per STYLE.md, every component prop should be documented with a JSDoc block comment so its purpose is clear at the definition site. Note the sibling SearchRouterOptionsWarmerProps type already documents the same prop.

Add a block comment above the prop:

type OptionsWarmerProps = { /** Called once the option list is cached (or the warm is no longer needed), so the parent can unmount this component. */ onDone: () => void; };

Reviewed at: 6864931 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.

github-actions · 2026-08-17T10:50:01Z

+    onDone: () => void;
+};
+
+// Stop waiting for quiet after this many idle windows; a chatty account never fully stops writing.


❌ CONSISTENCY-16 (docs)

This comment uses a semicolon to join two independent clauses. Comments should read as plain sentences, so split it into two separate sentences instead of using a semicolon.

// Stop waiting for quiet after this many idle windows. A chatty account never fully stops writing. const MAX_QUIET_WAIT_ATTEMPTS = 5;

Reviewed at: 6864931 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.

The native route mounts on a deep link or a restored last-visited path without openSearchRouter, and hardware Back and the iOS gesture pop it without closeSearchRouter, so set and reset the module-level flag from the page itself.

MelvinBot · 2026-08-17T16:39:08Z

Review

Overall this is a solid, well-reasoned change — the design (idle warm-up gated on Onyx inputs holding still, config extracted so the warmer and the live screen key the cache identically, unmount-when-done) is sound and unusually well-commented. The earlier bot findings (native back/gesture flag leak, comment style, prop docs) all look addressed. A few things worth a look before merge:

1. No tests for the new warm-up logic (main gap)

The 8-file diff adds no test, and Codecov flags a coverage drop — yet the author checklist claims unit tests were added. The pieces most likely to regress silently are pure logic and easy to cover:

The cache-hit invariant: after the warmer builds, the first empty-query open should hit createFilteredOptionList's cache rather than rebuild. That's the whole point of the PR and nothing guards it — a future change to the config or the key inputs could quietly turn the warm into dead work with no test failing.
The config-sharing invariant: searchRouterOptionsConfig.ts exists precisely so the warmer and SearchAutocompleteList produce the same cache key. A test asserting both call sites yield the same buildFilteredOptionListCacheKey inputs (maxRecentReports, includeP2P, isSearching, deferContactsUntilSearch) would lock that down.
The quiet-wait state machine in SearchRouterOptionsWarmer (reschedule while churn advances, cap at MAX_QUIET_WAIT_ATTEMPTS, bail when the router is already open) is testable with a fake scheduler.

2. Churn heuristic covers a subset of the cache inputs (non-blocking)

The warmer waits for reports/policies/personalDetails/reportAttributes/sortedActions to settle (here), but the cache key/inputs in createFilteredOptionList also include privateIsArchivedMap, conciergeReportID, isTrackIntentUser, and the locales. If one of those flips reference between the warm build and the first open, the cache simply misses and the live screen rebuilds — correct, just a missed optimization. Not a bug, but a one-line comment noting the wait is best-effort (and doesn't watch every invalidation source) would help the next reader.

3. Re-arm on every `IS_LOADING_APP → false` (confirm, non-blocking)

SearchRouterWarmup keys its effect on isLoadingApp, so every reconnect/OpenApp cycle that toggles the flag re-schedules an idle warm and re-mounts the options warmer. That's the intended "rewarm invalidated data" behavior, but on a flaky connection it means repeated churn-waits + builds. It self-limits (bails when the router is open, and cache hits are cheap), so likely fine — just worth confirming it doesn't thrash.

On the failing checks

The only red checks are the reviewer/author checklist gates and Codecov (item 1). Nothing in the actual failures points at a code defect in this diff — adding the tests above would clear Codecov and cover the highest-risk paths at once.

Scope of this review

Code review only — I did not exercise the app at runtime. The paths most worth a manual pass are the author's own Test steps 4–6 (idle-then-open, open-immediately-after-load-before-idle, and sign-out/sign-in), since those are exactly the cache-hit and cache-invalidation transitions this PR introduces.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8cd9dcca95

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-08-17T16:39:25Z

+                if (churnVersionRef.current !== lastSeenChurn && attempts < MAX_QUIET_WAIT_ATTEMPTS) {
+                    lastSeenChurn = churnVersionRef.current;
+                    attempts += 1;
+                    waitForQuiet();


Yield between quiet-period checks

When React Scheduler still has budget, an idle-priority callback scheduled from a currently running idle callback can execute in the same scheduler work loop; Scheduler.scheduleWhenIdle only adds a competing 300 ms fallback and does not guarantee a new event-loop window. Consequently, this recursive call can immediately observe the same churnVersionRef, enable the build, and unmount the warmer before queued post-OpenApp Onyx deliveries run. On accounts whose updates arrive just after that scheduler flush, the next delivery invalidates the transient cache entry, so the first Search open still performs the full build while startup also paid for the discarded warmup. Introduce an actual yield between checks or restart a quiet-period timer when an input changes.

Useful? React with 👍 / 👎.

Confirmed: a recursive scheduleWhenIdle re-runs inside the same scheduler work loop (8 recursions in one macrotask, 0.085 ms), so the quiet-wait barely waits. I built the real debounce (300 ms per watched input, 2 s cap) and A/B'd it against this branch on Android — interleaved variants, 6 paired rounds, cold launch, tap after a 25 s settle: the debounce variant lands +8.6% slower than this branch (median 1028 ms vs 943 ms, slower in 5/6 rounds), so making the wait real buys nothing and I'd rather not grow the warm-up code for it here.
Cache hit rate is the remaining gap and I'll take it in a follow-up: the warm entry is still invalidated by Onyx writes right after launch, exactly as you point out, so this is better than before but not yet ideal.

MelvinBot · 2026-08-17T16:40:00Z

⚠️ Something went wrong. This may be a transient issue — try again. If it persists, check status.anthropic.com.

Run details

aimane-chnaif

Please pull main.
And check bot comments. It's fine to skip edge cases.

MelvinBot · 2026-08-17T17:02:52Z

Code-quality review

Quality is high overall — every non-obvious decision has a why comment, magic values are named, and the earlier bot nits (prop JSDoc, comment style, native flag reset) are all fixed. A few refinements, none blocking:

1. Duplicated prop type across the two warmer files — CONSISTENCY-3

SearchRouterWarmup.tsx:14-17 declares OptionsWarmerProps ({ onDone: () => void }), which is a verbatim copy of SearchRouterOptionsWarmerProps in SearchRouterOptionsWarmer.tsx:16-19 — including the doc comment. Since the warmer is pulled in via require, export the prop type from SearchRouterOptionsWarmer and reuse it for the ComponentType<…> annotation (require<{default: ComponentType<SearchRouterOptionsWarmerProps>}>), so the two contracts can't drift.

2. The churn / quiet-wait loop wants to be its own hook — CLEAN-REACT-PATTERNS-4

In SearchRouterOptionsWarmer.tsx:40-71, the churn-version ref, the idle reschedule loop, the attempt cap, and the shouldBuild gate are one self-contained concern (“build once the inputs go quiet”) sitting alongside the actual useFilteredOptions build and the completion signal. Extracting it into a focused, named hook — e.g. useShouldBuildWhenQuiet(churnDeps) returning a boolean — isolates the state machine, gives it a name, and makes it unit-testable against a fake Scheduler. That directly covers the highest-risk untested logic I flagged in the prior review, so it buys quality and coverage at once.

3. `require` path style is inconsistent (minor)

SearchRouterWarmup.tsx:42 requires via the @components/… alias, while the sibling loaders in searchRouterLazyLoaders.ts use relative ../../../…. Pick one for consistency — the alias is the nicer of the two.

4. Funnel the module-flag writes through the setter (optional)

SearchRouterContext.tsx mutates isSearchRouterOpenOrOpening by bare assignment in five spots, but also exports setIsSearchRouterOpenOrOpening, which the native route path uses. Routing the in-file writes through the same setter (or a pair of markOpening()/markClosed() helpers) gives every mutation one greppable choke point — handy for a global whose whole risk is "did we forget to reset it somewhere."

5. Defensive `useCallback` on the inline `onDone` (minor)

SearchRouterWarmup.tsx:52 passes an inline () => setOptionsWarmer(null), and the child's quiet-wait effect lists onDone in its deps. React Compiler memoizes it today, but the coupling is implicit and cross-platform compiler agreement isn't guaranteed — a useCallback makes the stable-identity contract explicit.

What I checked and deliberately did not flag

The effect density in SearchRouterOptionsWarmer is not a PERF-9/PERF-10 violation: the effects synchronize with the external idle scheduler and an async build (things that can't be computed inline), and onDone signals a lifecycle unmount rather than passing state up — both are explicit carve-outs in those rules.
Naming (searchRouterOptionsConfig.ts → SEARCH_ROUTER_OPTIONS_CONFIG), JSDoc, comment-why, and the new-file headers all conform.
Code review only; I did not run the app.

MelvinBot · 2026-08-17T17:17:38Z

Regression hunt

Bottom line: no blocking regression found. I exercised the full search flow at runtime on web and statically traced the behavioral changes — everything that could break still works. Two things are worth a second look (neither reproduced), and one PR-checklist claim (no console errors) I couldn't verify because the web driver can't read the JS console.

Runtime results (web)

All six flow steps passed — open shows recent chats (non-empty, no duplicates), typing returns matching results, Clear restores the exact prior list, clicking a recent report opens the right one, and reopening after a ~30s idle window (warm-up runs) shows the same list with no duplication or loss. Opening before the first idle also worked.

	Evidence
Open
Query
Report opened
After idle

Static analysis — regression vectors checked and cleared

SearchAutocompleteList config refactor — SEARCH_ROUTER_OPTIONS_CONFIG reproduces the previous inline values exactly (enabled:true, deferContactsUntilSearch:true, maxRecentReports:100, batchSize:500) and is spread before isSearching, so no key collision. Behaviorally identical to before.
Moved lazy loaders — the relative require paths in searchRouterLazyLoaders.ts resolve to the same modules from their new location, and AuthScreens.tsx:291 still wires them as getComponent. Screen registration unchanged.
Module-level isSearchRouterOpenOrOpening — only read by the warmers; the writes added to the open/close paths are side-assignments that don't alter existing open/close logic. It's reset on provider unmount and the option-list cache is cleared via registerSessionCleanupCallback, so no state or data leaks across sign-out / into a different account.
Cache correctness — createFilteredOptionList keys on reference-equality of its Onyx inputs, so a warmed entry is only served when every input still matches at open time; any mismatch rebuilds. A stale or wrong-account list can't be served from the warm.

Worth a second look (not reproduced)

New "Recent searches" entry after the idle wait — on reopen after 30s, a type:expense from:me recent-search row appeared that wasn't on the very first open (stable, no duplication on further reopens). Recent searches render from a separate data source (recentSearchesMetadata), not the option-list cache this PR warms, so this is almost certainly a default/suggested search settling in on a ~60s-old account rather than the warm-up. Quick confirm on an established account would close it out.
Startup Onyx read cost on large accounts — SearchRouterOptionsWarmer subscribes to the full REPORT/POLICY collections + personal details during the post-launch idle window. That's the intended trade, but it's heavy work landing right after OpenApp — the fresh test account can't surface contention here, so the high-traffic-account run in the checklist is the real signal. Also note the warmer re-arms on every IS_LOADING_APP → false, so reconnects re-trigger it (intended rewarm, but repeated on flaky connections).

Verification gap

The web driver has no JS-console access, so the PR's "no console errors" test step is unverified — no visible error banners or broken UI appeared, but I can't confirm a clean console. Worth a manual console check during QA.

# Conflicts: # src/components/Search/SearchAutocompleteList.tsx

…p95-tail

sumo-slonik · 2026-08-18T13:48:51Z

Hi @aimane-chnaif, I think this PR is ready for a second round of review.

sumo-slonik added 4 commits August 11, 2026 12:14

Skip search-text normalization in getValidOptions when the search is …

3a124fe

…empty

Skip the autocomplete parser for blank queries

5064258

Cache attachment-message parse results by HTML

3d1e807

Warm the SearchRouter open path while the app is idle

50f9533

melvin-bot Bot assigned sumo-slonik Aug 11, 2026

sumo-slonik added 3 commits August 12, 2026 15:54

sumo-slonik changed the title ~~Perf/search router p95 tail~~ Fix performance of search router in p95 tail Aug 12, 2026

sumo-slonik added 3 commits August 12, 2026 16:57

Trim the comments in the SearchRouter warmup changes

e7f08da

Track warmup input churn through useOnyx and gate on an explicit open…

3c32afa

… flag

Shorten the recent-reports cap comment

ab916e5

staszekscp reviewed Aug 13, 2026

View reviewed changes

Comment thread src/hooks/useAutocompleteSuggestions.ts Outdated

sumo-slonik marked this pull request as ready for review August 17, 2026 10:45

sumo-slonik requested review from a team as code owners August 17, 2026 10:45

melvin-bot Bot requested review from aimane-chnaif and flaviadefaria and removed request for a team and flaviadefaria August 17, 2026 10:45

melvin-bot Bot removed the request for review from a team August 17, 2026 10:45

chatgpt-codex-connector Bot reviewed Aug 17, 2026

View reviewed changes

github-actions Bot reviewed Aug 17, 2026

View reviewed changes

Track the native SearchRouter route as the open flag

8cd9dcc

The native route mounts on a deep link or a restored last-visited path without openSearchRouter, and hardware Back and the iOS gesture pop it without closeSearchRouter, so set and reset the module-level flag from the page itself.

chatgpt-codex-connector Bot reviewed Aug 17, 2026

View reviewed changes

aimane-chnaif suggested changes Aug 17, 2026

View reviewed changes

Comment thread src/components/Search/SearchRouter/SearchRouterOptionsWarmer.tsx

Comment thread src/components/Search/SearchRouter/SearchRouterWarmup.tsx Outdated

Comment thread src/components/Search/SearchRouter/SearchRouterOptionsWarmer.tsx Outdated

sumo-slonik added 5 commits August 18, 2026 10:51

Merge branch 'main' into perf/search-router-p95-tail

bb473ea

Share the warmer prop type and drop the display names

824ce95

Merge branch 'main' into perf/search-router-p95-tail

e43ec01

# Conflicts: # src/components/Search/SearchAutocompleteList.tsx

Keep every shared option-list value in the search-router config

8ab2d73

Merge remote-tracking branch 'upstream/main' into perf/search-router-…

d5bac35

…p95-tail

		const openSearchRouter = (query?: string, isFromSearchPageSearchButton?: boolean) => {
		isSearchRouterOpenOrOpening = true;

Uh oh!

Conversation

sumo-slonik commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation of Change

Local measurements (dev builds — absolute ms are not production values, only the relative change matters)

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

codecov Bot commented Aug 11, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

staszekscp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

melvin-bot Bot commented Aug 17, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

sumo-slonik Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 17, 2026

Choose a reason for hiding this comment

❌ CONSISTENCY-13 (docs)

Uh oh!

sumo-slonik Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 17, 2026

Choose a reason for hiding this comment

❌ CONSISTENCY-16 (docs)

Uh oh!

sumo-slonik Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

MelvinBot commented Aug 17, 2026

Review

1. No tests for the new warm-up logic (main gap)

2. Churn heuristic covers a subset of the cache inputs (non-blocking)

3. Re-arm on every IS_LOADING_APP → false (confirm, non-blocking)

On the failing checks

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

sumo-slonik Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

MelvinBot commented Aug 17, 2026

Uh oh!

aimane-chnaif left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MelvinBot commented Aug 17, 2026

Code-quality review

1. Duplicated prop type across the two warmer files — CONSISTENCY-3

2. The churn / quiet-wait loop wants to be its own hook — CLEAN-REACT-PATTERNS-4

3. require path style is inconsistent (minor)

4. Funnel the module-flag writes through the setter (optional)

5. Defensive useCallback on the inline onDone (minor)

Uh oh!

sumo-slonik commented Aug 11, 2026 •

edited

Loading

codecov Bot commented Aug 11, 2026 •

edited by github-actions Bot

Loading

3. Re-arm on every `IS_LOADING_APP → false` (confirm, non-blocking)

3. `require` path style is inconsistent (minor)

5. Defensive `useCallback` on the inline `onDone` (minor)