Skip to content

CS-11121: dedup concurrent same-key store.search calls during prerender#4883

Merged
habdelra merged 3 commits into
mainfrom
cs-11121-store-side-in-flight-search-dedup-during-prerender-host-spa
May 19, 2026
Merged

CS-11121: dedup concurrent same-key store.search calls during prerender#4883
habdelra merged 3 commits into
mainfrom
cs-11121-store-side-in-flight-search-dedup-during-prerender-host-spa

Conversation

@habdelra
Copy link
Copy Markdown
Contributor

Summary

Adds an in-flight Map<key, Promise<doc>> on StoreService so concurrent same-(realms, query) store.search calls during a prerender share a single _federated-search round-trip. Mirrors RealmIndexQueryEngine.#inFlightSearch (CS-11115 Phase 1) on the host SPA.

  • Gated on __boxelRenderContext so live user searches stay uncoalesced — write-then-read freshness story unchanged outside prerender.
  • Refactors fetchSearchData and fetchAndHydrateSearchResults onto a shared fetchSearchDoc HTTP+JSON path so both benefit.
  • Entries self-clear on .finally() with identity check (same shape as the server-side reference).
  • clearInFlightSearch() wired to render-route deactivate so an in-flight entry doesn't leak across visits.

This is the host-side Phase 1. The sibling resolved-doc job-scoped cache (CS-11175) stacks on top of this layer; it answers from already-resolved bytes so sequential repeats inside a render get a sync hit, while this PR closes the concurrent-overlap window.

Linear: CS-11121

Test plan

  • CI: Unit | Utility | searchInFlightKey (host) passes — eight key-composition cases (order invariance, distinct queries / realms / pagination, etc.).
  • CI: Integration | search resource > in-flight search dedup module passes — five behavior cases:
    • Concurrent same-key calls share a single fetch inside a prerender.
    • Concurrent same-key calls do NOT coalesce outside a prerender.
    • Different filters do not coalesce even inside a prerender.
    • Sequential same-key calls fall through after the first resolves (in-flight only, no resolved-doc cache yet).
    • clearInFlightSearch() drops pending entries so new same-key callers re-fetch.
  • Manual: a local prerender visit logs fewer _federated-search HTTP requests for cohort-shaped fan-out (verifiable once stacked with CS-11175 against the /ambitious/prianha bench).

🤖 Generated with Claude Code

Add an in-flight Map on StoreService keyed by (realms, query) so
multiple Glimmer components rendering on the same prerender page that
fire identical store.search calls share one `_federated-search`
round-trip instead of each issuing its own.

Mirrors the server-side `RealmIndexQueryEngine.#inFlightSearch`
landed in CS-11115 Phase 1, with the same identity-checked finally
cleanup. Gated on `__boxelRenderContext` so live user searches stay
uncoalesced — write-then-read freshness story unchanged outside
prerender.

Refactor `fetchSearchData` and `fetchAndHydrateSearchResults` onto a
shared `fetchSearchDoc` HTTP+JSON path so both benefit from the
dedup.

Wire `clearInFlightSearch()` to render-route deactivate so an
in-flight entry doesn't leak across visits. The sibling resolved-doc
job-scoped cache (CS-11175) stacks on top of this layer.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@habdelra habdelra requested a review from Copilot May 19, 2026 14:50
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 19, 2026

Preview deployments

Host Test Results

    1 files  ±    0      1 suites  ±0   3h 21m 2s ⏱️ + 1h 48m 36s
2 686 tests +  197  2 671 ✅ +  197  15 💤 ± 0  0 ❌ ±0 
5 286 runs  +2 779  5 256 ✅ +2 764  30 💤 +15  0 ❌ ±0 

Results for commit 18320f8. ± Comparison against earlier commit b8a9b60.

Realm Server Test Results

    1 files  ±0      1 suites  ±0   9m 0s ⏱️ + 1m 7s
1 414 tests +3  1 414 ✅ +4  0 💤 ±0  0 ❌  - 1 
1 501 runs  +3  1 501 ✅ +4  0 💤 ±0  0 ❌  - 1 

Results for commit 18320f8. ± Comparison against earlier commit b8a9b60.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reduces duplicate _federated-search HTTP round-trips during prerender by coalescing concurrent store.search(realms, query) calls onto a single in-flight promise (gated to prerender via __boxelRenderContext). It also refactors search fetching so both hydrated and data-only search paths share the same HTTP/JSON implementation, and ensures in-flight entries are cleared on render-route deactivation.

Changes:

  • Add prerender-only in-flight deduping for concurrent same-key searches in StoreService.
  • Refactor search fetching to a shared fetchSearchDoc() path used by both hydration and data-only callers.
  • Add unit + integration tests for key stability and coalescing behavior; clear in-flight search entries on render route deactivate.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/host/app/services/store.ts Adds inflightSearch map + clearInFlightSearch(), refactors search fetching into fetchSearchDoc() with prerender-gated coalescing.
packages/host/app/routes/render.ts Clears store in-flight search map on deactivate() to prevent cross-visit coalescing.
packages/host/app/lib/search-in-flight-key.ts Introduces a stable (realms, query) signature helper for in-flight dedup keys.
packages/host/tests/unit/lib/search-in-flight-key-test.ts Unit tests for key stability and differentiation across realms/query/pagination.
packages/host/tests/integration/resources/search-test.ts Integration tests verifying coalescing behavior in prerender vs live-SPA and clearInFlightSearch() behavior.
Comments suppressed due to low confidence (4)

packages/host/tests/integration/resources/search-test.ts:772

  • This assertion relies on setTimeout(10) to give the two store.search calls time to reach the fetch hook. That introduces timing-based flakiness. Use a deterministic wait (like waitUntil(() => fetchCalls === 2)) before asserting.

      await new Promise((r) => setTimeout(r, 10));

packages/host/tests/integration/resources/search-test.ts:802

  • The test waits using a fixed setTimeout(10) before asserting fetchCalls. On slow machines the fetch interception may not have been hit yet, causing intermittent failures. Consider waiting until fetchCalls reaches the expected value instead of sleeping.
      let p1 = storeService.search(bookQuery, [testRealmURL]);
      let p2 = storeService.search(otherQuery, [testRealmURL]);

      await new Promise((r) => setTimeout(r, 10));

packages/host/tests/integration/resources/search-test.ts:842

  • Using setTimeout(10) to ensure the first fetch is in-flight can be flaky if the async chain hasn't executed within 10ms. Prefer a deterministic wait (e.g., waitUntil(() => fetchCalls === 1)) before asserting.
      let p1 = storeService.search(bookQuery, [testRealmURL]);
      await new Promise((r) => setTimeout(r, 10));
      assert.strictEqual(fetchCalls, 1, 'first fetch in-flight');

packages/host/tests/integration/resources/search-test.ts:849

  • This check again depends on setTimeout(10) for scheduling. To avoid timing-related flakes, wait until fetchCalls is observed to be 2 (or at least >= 2) before asserting.
      let p2 = storeService.search(bookQuery, [testRealmURL]);
      await new Promise((r) => setTimeout(r, 10));
      assert.strictEqual(

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/host/tests/integration/resources/search-test.ts Outdated
habdelra and others added 2 commits May 19, 2026 11:01
Address PR feedback: replace `setTimeout(10)` waits with synchronous
assertions, and narrow the fetch counter to `_federated-search` URLs.

The whole sync portion of `search → fetchSearchData → fetchSearchDoc
→ maybeAuthedFetchForRealms` runs before each `storeService.search()`
expression returns. The wrapped fetch's `fetchCalls++` is the first
line before any `await`, so by the time both invocations have
returned their Promises the counter reflects every fetch that's been
committed. No timeout needed — and a broken dedup would tick the
counter immediately too, so the test is strictly more sensitive than
the timed variant.

The URL filter scopes the counter to the store-search hot path, so
unrelated `maybeAuthedFetchForRealms` traffic (auth probes, registry
pings, future callers) can't inflate it and turn the dedup assertion
flaky.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…n-flight-search-dedup-during-prerender-host-spa

# Conflicts:
#	packages/host/app/services/store.ts
@habdelra habdelra requested a review from a team May 19, 2026 15:43
@habdelra habdelra merged commit 411d8db into main May 19, 2026
100 of 103 checks passed
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.

3 participants