Skip to content

Unified search: collapse federated runners, mergers & handlers into one#5167

Merged
habdelra merged 3 commits into
mainfrom
cs-11432-unified-search-collapse-federated-runners-mergers-handlers
Jun 9, 2026
Merged

Unified search: collapse federated runners, mergers & handlers into one#5167
habdelra merged 3 commits into
mainfrom
cs-11432-unified-search-collapse-federated-runners-mergers-handlers

Conversation

@habdelra

@habdelra habdelra commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Phase 1 / Stream A step 2 of Unify prerendered-card-search and card-search: collapse the duplicated federated runners, mergers, and HTTP handlers onto the single projection-parametrized search(). No wire-shape change — both federated endpoints still emit today's two doc shapes (the existing endpoint suites are the parity goldens); the prefer-HTML emission turns on in the next step.

Stacked on #5157. Review the top commit here; base retargets to main as the stack lands.

The change

runtime-common/search-utils.ts

  • One shared fanOutRealmSearch fan-out (filter dead realms → settle concurrently → log per-realm failures → return fulfilled docs in realm order). searchRealms (live) and searchPrerenderedRealms (prerendered) are now thin wrappers over it, differing only in the per-realm method they call, the failure label, and the merge.
  • combineSearchResults dedupes included by the JSON:API identity pair (type, id) instead of id alone — so a card and its rendered-html (which share the card URL as id) can coexist, while a css (hash id) or a linked card shared across realms travels once. Parity-preserving for today's card/file-meta included.

realm-server/handlers/

  • The job-scoped cache + ETag/304 protocol is extracted into one respondWithJobScopedSearchCache core that both handlers call; handle-search-prerendered.ts shrinks to parse + run + delegate.
  • handle-search.ts now parses the unified request (render / dataOnly / cardUrls) and folds those members into the cache key, so the two endpoints can't collide on a shared key and a render/dataOnly difference segregates entries + ETags. Job-id + consuming-realm gating and the 304 path are unchanged.

Tests

  • Merge dedup: a shared css and a shared linked card across two realms appear once in included; a card and its same-id rendered-html both survive ((type, id) discriminates where id alone would collapse them).
  • Cache-key segregation (/_federated-search): requests differing only on render.format / render.renderType / dataOnly / cardUrls get distinct entries + ETags; an explicit default render and a plain query share one entry; identical requests still hit — and the render members don't yet change the response body.
  • The existing /_federated-search + /_federated-search-prerendered suites (federation, caching, ETag/304, auth, errors) pass unchanged as the parity goldens.

Locally: runtime-common + realm-server type-check clean; unified-search contracts 20/20, job-scoped-search-cache 16/16, the two federated endpoint suites 89/89.

🤖 Generated with Claude Code

Collapse the duplicated federated runners, mergers, and HTTP handlers onto the
single projection-parametrized search(). No wire-shape change — both federated
endpoints still emit today's two doc shapes; the prefer-HTML emission lands in
the next step.

- search-utils.ts: one fanOutRealmSearch fan-out shared by searchRealms (live)
  and searchPrerenderedRealms (now thin wrappers, differing only in the
  per-realm method, failure label, and merge). combineSearchResults dedupes
  included by the identity pair (type, id) rather than id alone, so a card and
  its rendered-html (same id) coexist while a shared css or linked card travels
  once.
- realm-server handlers: extract the job-scoped cache + ETag/304 protocol into
  one respondWithJobScopedSearchCache core both handlers call;
  handle-search-prerendered shrinks to parse + run + delegate. handle-search
  parses the unified request and folds render/dataOnly/cardUrls into the cache
  key, so the two endpoints can't collide and a render/dataOnly difference
  segregates entries + ETags. Job-id + consuming-realm gating and 304 path
  unchanged.

Tests: merge dedup over (type, id); live cache-key segregation by
render/dataOnly/cardUrls; the existing endpoint suites pass unchanged as the
parity goldens.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 28607d7d30

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/runtime-common/search-utils.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Collapses duplicated federated search fan-out/merge logic and shared job-scoped caching behavior into single, projection-parameterized primitives, as part of the “unified search” refactor while preserving existing endpoint wire shapes.

Changes:

  • Refactors federated realm fan-out into a shared fanOutRealmSearch and updates the unified merge to dedupe included by (type, id).
  • Extracts job-scoped cache + ETag/304 behavior into respondWithJobScopedSearchCache and reuses it in both federated handlers.
  • Adds targeted tests for (type,id) included dedupe and cache-key/ETag segregation across unified request members.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/runtime-common/search-utils.ts Introduces unified merge semantics and shared federated fan-out helper used by both live and prerendered runners.
packages/realm-server/handlers/handle-search.ts Parses unified request members and folds them into the job-scoped cache key; extracts shared cache/ETag logic.
packages/realm-server/handlers/handle-search-prerendered.ts Reuses the shared job-scoped cache/ETag helper to remove duplicated caching logic.
packages/realm-server/tests/unified-search-contracts-test.ts Adds unit coverage for unified combineSearchResults included-dedupe behavior.
packages/realm-server/tests/server-endpoints/search-test.ts Adds endpoint coverage ensuring cache-key/ETag segregation by unified request members.

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

Comment thread packages/runtime-common/search-utils.ts Outdated
Comment thread packages/realm-server/handlers/handle-search.ts Outdated
…the cache key

- combineSearchResults: write the (type, id) identity separator as a backslash
  escape rather than a raw NUL byte, so the source stays ASCII. A raw NUL made
  text tooling (rg/grep) classify the whole file as binary; the runtime
  separator is unchanged.
- handle-search: fold cardUrls into the job-scoped cache key only when
  non-empty. An empty array is a no-op filter, so folding it would fragment the
  cache against an equivalent request that omits cardUrls. The segregation test
  now asserts cardUrls: [] shares the plain-query entry.
- Reword the comments introduced in this change to state the current contract.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Host Test Results

    1 files  ± 0      1 suites  ±0   1h 50m 51s ⏱️ - 3m 6s
2 997 tests +47  2 982 ✅ +47  15 💤 ±0  0 ❌ ±0 
3 016 runs  +47  3 001 ✅ +47  15 💤 ±0  0 ❌ ±0 

Results for commit a01b7a1. ± Comparison against earlier commit 7dc6516.

Realm Server Test Results

    1 files  ±0      1 suites  ±0   10m 44s ⏱️ +12s
1 579 tests +9  1 579 ✅ +9  0 💤 ±0  0 ❌ ±0 
1 670 runs  +9  1 670 ✅ +9  0 💤 ±0  0 ❌ ±0 

Results for commit a01b7a1. ± Comparison against earlier commit 7dc6516.

@habdelra
habdelra changed the base branch from cs-11431-unified-search-projection-parametrized-search-sql to main June 9, 2026 21:47
…ch-collapse-federated-runners-mergers-handlers

# Conflicts:
#	packages/realm-server/handlers/handle-search-prerendered.ts
#	packages/runtime-common/search-utils.ts
@habdelra
habdelra merged commit b8e7a7c into main Jun 9, 2026
65 of 66 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