Skip to content

feat: mixed cards + files entry search with scope and client parity#5428

Open
FadhlanR wants to merge 2 commits into
mainfrom
cs-11775-search-api-mixed-entry-search-cards-files-with-card-instance
Open

feat: mixed cards + files entry search with scope and client parity#5428
FadhlanR wants to merge 2 commits into
mainfrom
cs-11775-search-api-mixed-entry-search-cards-files-with-card-instance

Conversation

@FadhlanR

@FadhlanR FadhlanR commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Closes CS-11775, CS-11776.

What

Makes the search entry API return card instances and files from a single query, instead of routing all-or-nothing by filter type, and updates every consumer to the new mechanism so the API change and its consumers land together. Kind is discriminated entirely by the caller's filter — a card-type-anchored filter matches only instance rows via types, a FileDef-anchored filter only file rows — so no per-kind routing heuristic is needed and no wire member is added.

API changes (CS-11775)

  • _search gains an 'all' mode (WHERE i.type IN ('instance','file')) beside the existing 'instance'/'file' modes. Nothing is deduped by default: a dual-indexed card .json (an instance row and a file row sharing one url) surfaces as two rows via GROUP BY (url, type); COUNT/LIMIT/OFFSET run over the same grouping so total stays consistent. Instance rows honor includeErrors; file rows only surface healthy.
  • _isCardInstance is a queryable synthetic field: eq: { item._isCardInstance: null } keeps cards + plain files and drops the card .json file row (the key is stamped only on that row). The positive NULL match is load-bearing — not: { eq: { _isCardInstance: true } } would compare NULL on every other row and drop everything.
  • Unified projection + assembly. search() carries the file-only columns in either projection when file rows are in scope; searchEntries folds the former instance and file-meta loops into one loop that branches per row on row.type. The single-URL instance GET pins 'instance' so a bare file URL never resolves as a card entry.
  • The federated fan-out/merge is untouched.

Consumer changes (CS-11776)

Every consumer falls into one of two filter idioms:

  • Cards-only → anchor with { type: baseCardRef } unless a positive type ref is already present (the engine shares one types cross-join alias, so two positive type conditions cannot compose). Shared helper anchorQueryToCardInstances lives in runtime-common/query-field-utils.ts.
  • Mixed-but-deduped → compose eq: { _isCardInstance: null } (legacy spelling) / 'item._isCardInstance': null (wire spelling).
Consumer Fix
Search sheet (query-builder.ts) term filter matches the synthetic _title key so files match by name; sheet queries compose the _isCardInstance dedup
Card choosers (modal + mini) new cardsOnly option on buildSearchQuery/buildRecentsQuery, passed as @cardsOnly through PanelContent; the preselected-card store.search query is anchored defensively
Recents queries (incl. compact) scoped the same way — their cardUrls also name the dual-indexed .json file rows
Host-mode head prefetch 'item._isCardInstance': null in the wire query (its cardUrls matched both rows and the file row's head could win the order tiebreak — the playwright head-tags failure)
create-listing example query same wire-level dedup
cards-grid "All Cards" anchored with { type: baseCardRef }; the "All Files" group owns the file side. (Collateral: cards-grid wasn't in the original ticket scope — the mixed default forces it.)
search-cards AI command anchors an otherwise-unscoped query via the shared helper
Playground, detail-panel, create-specs, sync-openrouter, file-tree no change — already type-anchored (self-discriminating) or file-only by design

When a positive type condition is present (a picked type or a typed base filter), neither anchor nor dedup is added: one kind is already selected, and a card .json matching an explicit file-type filter correctly surfaces once, as a file.

Tests

  • Realm-server engine tests: mixed default returns both kinds; eq item._isCardInstance null dedups; positive card-type anchor stays cards-only; the head-scoped single-URL query mirrors the prefetch dedup.
  • Host: query-builder unit expectations updated for _title + scoping; implicitly cards-only realm/indexing test queries anchored; new "All Cards stays cards-only" integration test.

Deployment & backward compatibility

No transitional wire mechanism ships with this change — compatibility is handled operationally. The asymmetry that drives the plan:

  • Old client → new server degrades softly at worst. NULL semantics keep most existing queries card-only automatically (a positive predicate on a card field, a not: { eq: … }, or a positive type anchor all drop file rows, since file rows lack those search_doc keys / types). Only filter-less/sort-only queries, cardUrls queries (a card .json URL matches both of its rows), and queries on keys files also carry (_title) see mixed rows — extra file-meta entries that current clients already deserialize; no errors.
  • New client → old server is a hard break: the _isCardInstance synthetic field is only queryable as of this PR, so an old realm-server rejects it as an unknown field.

Prerequisite: index state (per environment, staging then prod)

Dedup relies on _isCardInstance: true being stamped on every card-instance .json file row (stamping landed separately with the CS-11774 indexing change; this PR only consumes it, and includes no reindex trigger). Before deploying, verify a full reindex has completed since that stamping deploy — e.g. spot-check boxel_index that file-type rows for card .jsons have search_doc->>'_isCardInstance' = 'true'. If not, force a full reindex and wait. Un-restamped cards appear twice in the search sheet until their realm reindexes.

Deploy order

  1. realm-server first — the mixed default goes live; a not-yet-updated host doesn't send the dedup filter, so the only skew passed through is soft (transient dupes in search results, no errors).
  2. host second, once realm-server is healthy — it starts sending eq: { item._isCardInstance: null } and the skew window closes.

Rolling back is the reverse: host first, then realm-server, so the deployment never passes through new-host + old-realm (the erroring direction). A realm-server rollback needs no index migration — the stamped key is inert to old code.

Post-deploy smoke tests

  • Search sheet: cards and plain files appear; no card appears twice.
  • Cards-grid "All Cards": cards only; errored instances still show their error tile.
  • Card chooser / MiniCardChooser: cards only.
  • search-cards AI command: card instances only.
  • Recents: each entry appears once.
  • Published view: head prefetch resolves a single card.

Client follow-up

boxel-cli's filter-less and cardUrls searches will show the extra file rows until it adopts the dedup filter — tracked in CS-12052, to be released together with or after the realm-server deploy (an old server rejects the new filter).

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Preview deployments

Host Test Results

    1 files  ±0      1 suites  ±0   3h 3m 32s ⏱️ - 2m 2s
3 485 tests ±0  3 470 ✅ +10  15 💤 ±0  0 ❌ ± 0 
3 504 runs  ±0  3 489 ✅ +20  15 💤 ±0  0 ❌  - 10 

Results for commit 3cde39e. ± Comparison against earlier commit 5b89f06.

Realm Server Test Results

    1 files  ±0      1 suites  ±0   11m 24s ⏱️ -45s
1 824 tests ±0  1 824 ✅ +6  0 💤 ±0  0 ❌  - 6 
1 903 runs  ±0  1 903 ✅ +6  0 💤 ±0  0 ❌  - 6 

Results for commit 3cde39e. ± Comparison against earlier commit 5b89f06.

@FadhlanR FadhlanR changed the title Search API: mixed cards + files search with card-instance .json exclusion Search: mixed cards + files entry search; consumers pinned or deduped (CS-11775, CS-11776) Jul 9, 2026
@FadhlanR FadhlanR marked this pull request as ready for review July 10, 2026 08:18
@FadhlanR FadhlanR requested review from a team and habdelra July 10, 2026 08:18

@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: 56029a658b

ℹ️ 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/host/app/utils/card-search/query-builder.ts Outdated
Comment thread docs/cs-11775-11776-mixed-search-plan.md 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

Updates the realm “entry” search pipeline to support mixed card-instance + file results in a single query (entryType: 'all'), and adjusts host + server consumers/tests to either (a) anchor queries to instances only, or (b) intentionally request mixed results while deduping dual-indexed card .json file rows via _isCardInstance: null.

Changes:

  • Expand IndexQueryEngine/RealmIndexQueryEngine to support mixed entry searching (GROUP BY (url, type), deterministic ordering, and distinct (url,type) counting).
  • Add a shared helper anchorQueryToCardInstances() and update host consumers to use either anchoring (cards-only) or _isCardInstance: null (mixed-but-deduped).
  • Extend realm-server and host tests to cover mixed default behavior, dedup behavior, and cards-only invariants.

Reviewed changes

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

Show a summary per file
File Description
packages/runtime-common/realm-index-query-engine.ts Assemble mixed entry results by branching per row (instance vs file) and pin single-URL card lookup to instance scope.
packages/runtime-common/query-field-utils.ts Add anchorQueryToCardInstances() helper to force cards-only results when a query is otherwise untyped.
packages/runtime-common/index-query-engine.ts Add 'all' mode to _search/search, group/count by (url,type), add deterministic type tiebreakers, and make _isCardInstance queryable as a synthetic field.
packages/realm-server/tests/search-entries-engine-test.ts Add coverage for mixed default results, dedup-by-NULL filter, and cards-only anchoring behavior.
packages/host/tests/unit/card-search-query-builder-test.ts Update expectations for _title term matching, dedup filter composition, and cardsOnly scoping.
packages/host/tests/unit/anchor-query-to-card-instances-test.ts New unit tests for anchorQueryToCardInstances() behavior and edge cases.
packages/host/tests/integration/realm-test.gts Anchor test queries to cards-only where the assertions expect card-only result sets.
packages/host/tests/integration/realm-querying-test.gts Anchor the sort-by-card-display-name test to card instances to avoid mixed default interference.
packages/host/tests/integration/realm-indexing-test.gts Anchor indexing-related test queries to cards-only to preserve intent under mixed default.
packages/host/tests/integration/components/operator-mode-ui-test.gts Add integration coverage that cards-grid “All Cards” remains cards-only.
packages/host/app/utils/card-search/query-builder.ts Switch term matching to _title and add scoping logic (cardsOnly anchor vs _isCardInstance: null dedup).
packages/host/app/tools/search-cards.ts Anchor LLM-provided queries to card instances to preserve the tool’s contract.
packages/host/app/services/host-mode-service.ts Add wire-level 'item._isCardInstance': null to avoid head-prefetch ambiguity with dual-indexed .json rows.
packages/host/app/components/operator-mode/create-listing-modal.gts Add wire-level 'item._isCardInstance': null to dedup example-card lookups by cardUrls.
packages/host/app/components/card-search/panel-content.gts Thread @cardsOnly into query building and ensure compact recents don’t double-surface card .json URLs.
packages/host/app/components/card-search/constants.ts Switch A–Z sorting to _title so file rows sort meaningfully in mixed results.
packages/host/app/components/card-chooser/modal.gts Ensure chooser uses cards-only search behavior and defensively anchor preselected-card queries.
packages/host/app/components/card-chooser/mini/index.gts Set @cardsOnly for the mini chooser results.
packages/base/cards-grid.gts Document why “All Cards” stays cards-only without a type anchor (preserves errored instances).
docs/cs-11775-11776-mixed-search-plan.md Adds an internal planning document (currently includes private tracker references and merge-state notes).

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

Comment thread docs/cs-11775-11776-mixed-search-plan.md Outdated
@habdelra

Copy link
Copy Markdown
Contributor

I believe for for "all" type of query it will only work with full text search using the "matches" filter, as there are no fields in common between file-def and card-def. we should return a 400 error if the client uses combines an "all" with something that is not a "matches" filter.

@habdelra

Copy link
Copy Markdown
Contributor

Can you include the new wire format for the "all" mode? I'm having a hard time finding it the tests

@habdelra

Copy link
Copy Markdown
Contributor

what happens when you mix _isCardInstance:null with type: xxx? what does that even mean?

@habdelra

Copy link
Copy Markdown
Contributor

what does _isCardInstance: false mean? why is the guidance to use _isCardInstance: null instead of _isCardCardInstance: false

@FadhlanR

Copy link
Copy Markdown
Contributor Author

[Claude Code 🤖] @habdelra answering the discussion points together, since they are related:

Is there an 'all' wire mode? / 400 for non-matches` filters

There is no new wire member — the request format is unchanged. Searching both kinds is simply the endpoint's default now: with no type filter, a query matches instance and file rows alike. Kind discrimination happens entirely through the filter's type ref — { type: <any card type> } (e.g. the CardDef base ref) matches only card instances via types, a FileDef-anchored ref matches only files. On the response side the only change is that a mixed result can interleave file-meta resources with card resources, in the same envelope as before.

On restricting mixed queries to matches: the two kinds do share queryable fields — the synthetic _title key (card title / file name) is stamped on both row kinds, and the search sheet relies on contains: { _title: term } matching files by name. Full-text matches spans both too. A filter on a card-only field never matches file rows (the key is absent from their search doc), which acts as the intended narrowing rather than an error — and since mixed is the default with no opt-in member, a 400 on non-matches filters would reject every typed query. Filters on genuinely nonexistent fields already error today.

That said — would you prefer an explicit wire member (e.g. scope: 'instance' | 'file' | 'all') over filter-driven discrimination? If you think the implicitness will bite API consumers, adding one is straightforward.

_isCardInstance semantics (null vs false, mixing with type:)

Agreed the current shape is confusing, and we're changing it. The key is stamped on a card .json's file row and really means "this file is a card instance's source", so the plan is to rename it to _isCardSourceFile and make eq: { _isCardSourceFile: false } the dedup spelling — an absent key will be treated as false at query translation, so the load-bearing null match goes away and false means what it reads as. Mixing it with type: xxx is redundant but harmless: a card-type filter already selects instance rows, which never carry the key; our query builders skip the dedup filter whenever a positive type ref is present.

@habdelra

Copy link
Copy Markdown
Contributor

so one thing we floated a bunch of times was using a type of baseDef to mean both card defs and file defs since they both inherit from this type, what did you think of that idea?

@habdelra

Copy link
Copy Markdown
Contributor

(also i have claude working on a more thorough code review as we speak)

@habdelra habdelra 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.

[Claude Code 🤖] Full review, focused on how this lands against the query engine's architecture. TL;DR: the engine-level mechanics are right and I'd have built the _search layer the same way; the feedback is concentrated on the application layer — where kind-selection intent lives, and one engine limitation this PR works around that we should fix instead.

What lands right

The core design respects the engine's underpinnings precisely where it matters:

  • Mixed scope belongs in _search, expressed as row-kind gating (i.type IN via the OR'd branches), not as filter rewriting. The instance/file branches keep their distinct error semantics, and the shared everyCondition means the data and count queries can't disagree.
  • GROUP BY (url, type) is the correct grouping once a dual-indexed card .json puts two rows behind one url — and the count rewrite to COUNT(*) over a DISTINCT (url, type) subquery correctly accounts for SQLite's single-expression COUNT(DISTINCT …) limit. The type tiebreaker added to every ORDER BY path (including the wrapped-projection variant) keeps pagination deterministic. This is exactly the discipline the two-adapter constraint demands.
  • Kind discrimination through the filter is sound because of how the search doc works: a card-only field filter requires an on: type ref to resolve (which brings a types condition that file rows can't match), and file rows simply lack card keys. The self-narrowing is a property of the schema walk, not a heuristic.
  • Deleting the queryTargetsFileMeta all-or-nothing routing is a genuine simplification — a FileDef-typed filter now works through the same path as everything else.
  • The deployment/compat analysis in the description is thorough and honest about the hard direction.

On restricting mixed queries to matches (discussion follow-up)

I'd argue against a 400 for non-matches filters in mixed scope. The two kinds do share a queryable surface — it's just synthetic: _title (contains + sort), matches (GIN full-text), _cardType, and now the dedup key. The search sheet's own mixed query is every: [{not: {type: spec}}, {any: [{matches}, {contains: {_title}}]}, dedup] — a strict matches-only rule would reject it. The synthetic-key set effectively is the schema of a mixed search; rather than policing filters, I'd document that contract (a short section wherever the entry API wire format is documented: "these are the keys both row kinds carry; anything else narrows to one kind by construction").

On the baseDef type-anchor idea (discussion follow-up)

Conceptually attractive — one mechanism for everything — but three mechanical facts work against it today:

  1. BaseDef's key is in neither kind's types array. getTypes (host render/meta.ts) explicitly breaks at baseRef, so instance chains end at CardDef; file rows carry [<file subtype>, FileDef] (file-indexer.ts). A {type: baseDef} filter would match zero rows. Making it work needs an indexer change plus a full reindex of every realm.
  2. Type conditions don't compose. All type refs in a query share one jsonb_array_elements_text(types) cross-join alias, so every: [{type: baseDef}, {not: {type: spec}}] doesn't mean what it reads as (verified against Postgres — see the anchorQueryToCardInstances thread). A baseDef anchor would immediately collide with this.
  3. Rows with empty types vanish whenever the filter references the types alias — errored instances would silently drop out of "everything" queries.

So: if we want baseDef-as-universe eventually, the typeCondition composability fix below is a prerequisite anyway. For now, an explicit wire scope (or the PR's ambient default) is mechanically cleaner.

Recommendations (details in the inline threads)

  1. Expose the scope on the wire (scope: 'cards' | 'files' | 'all', default 'all'). The PR already models scope internally (entryTypeScope) and pins it for cardDocument; wire callers deserve the same verb instead of filter incantations. It pins i.type directly — no shared-alias heuristics, no dependence on index-stamp state, doesn't drop errored instances the way a types anchor does, and it answers "what's the wire format for 'all'?" by making the mode visible in the request. Compat cost is identical to what the PR already accepts for the new synthetic field.
  2. Centralize the dedup idiom behind one named runtime-common export (and its wire spelling) instead of five scattered _isCardInstance: null literals — which also makes the agreed _isCardSourceFile rename a one-file change. The rename's absent-as-false semantics need to live in fieldEqFilter (polarity-aware), not the getField shim — sketch in the inline comment.
  3. File a follow-up: make typeCondition a self-contained membership predicate (pg types @> '["key"]'::jsonb, sqlite EXISTS (SELECT 1 FROM json_each(types) WHERE value = ?) — a resolved per-adapter node, same pattern as JsonContains). This fixes a latent pre-existing bug (not: {type: spec} does not actually exclude Specs — verified), makes positive type conditions AND-composable, and lets anchorQueryToCardInstances drop its skip-heuristic and documented any: leak.
  4. Consider hoisting the cards-only anchor into store.search — its doc comment already promises "instances only," and the helper's skip-on-positive-type behavior means typed (card or file) queries pass through untouched. One choke point instead of per-call-site patches, and future store.search callers can't forget.
  5. Smaller items inline: trim the unused markdown/isolated_html from fileColumns (potentially large per-row payloads the mixed assembly never reads), and converge the duplicated file-row assembly between the new mixed branch and searchEntriesFileMeta.

Tests

The engine tests are well-aimed (mixed default, NULL dedup, positive-anchor narrowing, and the _title sort-direction guard is a nice touch). Two gaps: an 'all' + includeErrors test (the asymmetry — errored instances in, errored files out — is load-bearing and currently only documented in a comment), and an engine-level result test for the anchored not-spec composition, which belongs with the follow-up in (3).

None of this blocks the PR conceptually — (1) and (2) are worth doing before merge since they shape the wire surface consumers will start depending on; (3)–(5) work fine as fast-follows.

Comment thread packages/runtime-common/index-query-engine.ts
Comment thread packages/runtime-common/index-query-engine.ts Outdated
Comment thread packages/runtime-common/query-field-utils.ts Outdated
Comment thread packages/host/app/utils/card-search/query-builder.ts
Comment thread packages/host/app/services/host-mode-service.ts Outdated
Comment thread packages/runtime-common/realm-index-query-engine.ts Outdated
Comment thread packages/runtime-common/realm-index-query-engine.ts
Comment thread packages/host/tests/unit/anchor-query-to-card-instances-test.ts Outdated
@habdelra

Copy link
Copy Markdown
Contributor

(lol, these fable code reviews are fantastic 🤩 )

@habdelra habdelra 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.

[Claude Code 🤖] Follow-up review on local search parity — the client-side counterpart of the query engine. The store runs live queries against in-memory instances for immediate response and reconciles the server's results as they arrive (SearchResource.displayedInstances in host/app/resources/search.ts, backed by runtime-common/instance-filter-matcher.ts, whose header promises it is "kept faithful to the server"). This PR teaches the server three new query idioms without teaching the client matcher any of them, and the failure mode is not "degraded instant results" — it's the reconciler actively removing correct server results.

The mechanics

displayedInstances keeps a server-returned instance only while it doesn't demonstrably fail the filter locally: 'no-match' removes it; 'unresolvable' keeps it. The matcher resolves a filter path via resolvePath, which looks fields up on the hydrated instance. A synthetic search-doc key (_title, _cardType, _isCardInstance) is not a field on any instance, so the path contributes no values and — critically — sawUnresolvable stays false, so the predicate returns 'no-match', not 'unresolvable'. The engine's getField has a shim that fabricates definitions for these keys; the matcher has no counterpart.

The three divergences this PR introduces

  1. eq: { _isCardInstance: null } blanks a reconciled result set. Server: key absent from every instance row → IS NULL → matches all cards. Client: no such field → 'no-match' for every instance → every server result is filtered out of the display and no local candidate can be added. A live SearchResource query using this PR's own documented dedup recipe renders zero rows while holding a correct server response.
  2. contains: { _title: term } does the same for recents-style term filters. The pre-PR spelling cardTitle was a real field and evaluated correctly locally; _title resolves to nothing → 'no-match' for every instance.
  3. sort: [{ by: '_title', on: CardDef }] degrades to URL order locally. sortValue yields null for every instance → NULLS-LAST ties → id tiebreak — and displayedInstances re-sorts the merged set with the local comparator, overriding the server's correct ordering. (Inline comment on constants.ts.)

Why tests haven't caught it: today's consumers of these idioms (search sheet, recents) go over the entry API resource, which doesn't instance-match — so the breakage is latent there. But getSearchResource is precisely the surface handed to cards via @context.store, and the dedup filter is now the documented recipe for mixed-safe queries; the first card author who follows it gets a blank live query. There's also an amusing proof of how idiom-dependent this is: cards-grid's not: { eq: { _cardType: null } } accidentally agrees with the server locally (inner 'no-match' negates to 'match'), while the positive eq: { _isCardInstance: null } — same synthetic-key mechanics, opposite polarity — blanks the set.

Proposed approach

Mirror the engine's own pattern: the engine centralizes synthetic-key knowledge in its getField shim; give the matcher the exact counterpart, in the same file the engine documents them, so the next synthetic key updates both sides or neither.

  1. A synthetic-key resolution step ahead of resolvePath (used by matchEq/matchIn/matchContains and sortValue), returning the value both sides agree the key denotes:
    • _title → the instance's title (cardTitle for cards — the same value the index stamps; a FileDef's name).
    • _cardType → the card type's display name (what the meta route stamps).
    • _isCardInstance → for a hydrated CardDef, always null (the stamp exists only on a card .json's file row, which never hydrates as a CardDef) — so eq: null matches, restoring parity. For a hydrated FileDef the stamp isn't derivable from the resource (it lives in the search doc, not the file's attributes), so return 'unresolvable' — which the reconciler already treats as "trust the server."
  2. A conservative guard in isClientEvaluable: a filter path whose head is an unshimmed underscore-prefixed key should make the query not client-evaluable (server-only passthrough). That turns "someone added a synthetic key on the server and forgot the matcher" from silently-blanked results into merely no-instant-results — the failure mode you want by default.
  3. makeInstanceComparator.sortValue takes the same shim for _title (and _cardType) sorts.
  4. When the agreed _isCardSourceFile rename lands, its matcher shim should ship in the same commit as the engine-side fieldEqFilter rewrite — the two files are one contract, and a cross-reference comment between the engine's getField shim and the matcher would make that hard to forget.

One adjacent structural note for a follow-up rather than this PR: displayedInstances dispatches a single candidate pool (cards or file-meta) by sniffing the filter's top-level type ref or the first server row. Once mixed result sets are the default, a mixed live query can only reconcile one kind, and first-row sniffing becomes order-dependent. Fine to defer — but worth capturing in the same ticket that tracks mixed-search parity, since the sheet will presumably want live file results eventually.

If it's useful I can sketch the shim as a concrete diff — it's small and self-contained (instance-filter-matcher.ts + one guard in isClientEvaluable).

Comment thread packages/host/app/components/card-search/constants.ts

@habdelra habdelra 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.

This PR actually breaks the local search parity in a destructive way. see the followup review comment for the details.

@habdelra

Copy link
Copy Markdown
Contributor

FYI, if it helps I used this skill to help guide this code review #5467

@FadhlanR FadhlanR force-pushed the cs-11775-search-api-mixed-entry-search-cards-files-with-card-instance branch from 00f8008 to 5b89f06 Compare July 13, 2026 10:30
@FadhlanR

Copy link
Copy Markdown
Contributor Author

[Claude Code 🤖] Rebased onto latest main and pushed a revised implementation addressing the review feedback. Summary of what changed since the last review:

Engine — type conditions now compose (root-cause fix, not a workaround)
Replaced the shared `jsonb_array_elements_text(types)` cross-join alias with a self-contained `TypesContains` membership predicate (Postgres `@>`, SQLite `json_each` EXISTS, `COALESCE` so NULL/absent `types` are a definite FALSE). Verified on both adapters: `not:{type:X}` genuinely excludes X, `every:[{type:A},{type:B}]` is a satisfiable intersection, and rows with empty `types` survive negation. This removes the need for the anchor heuristic and its documented `any:` leak — `anchorQueryToCardInstances` is deleted.

Wire `scope` member
Kind selection is now a first-class wire concept: `scope: 'cards' | 'files' | 'all'` (default `'all'`) pins `boxel_index.type` directly, independent of filter shape. It answers "what's the wire format for 'all'?" by making the mode explicit, doesn't drop errored instances the way a `{type}` anchor does, and is immune to the un-restamped-rows window. Consumers (store.search, host-mode head prefetch, create-listing, recents, choosers) pin scope instead of the `_isCardInstance` incantation; the federated-search cache key includes it. `scope: 'all'` returns both rows of a dual-indexed card; deduping stays an explicit filter.

Client search parity (the CHANGES_REQUESTED blocker)
The matcher now understands the synthetic search-doc keys via a shim at the top of `resolvePath` (covers all match predicates and the sort comparator), plus an `isClientEvaluable` guard so an unshimmed underscore key becomes server-only rather than blanking reconciled results. `friendlyCardType` moved to runtime-common so the index stamp and the matcher share one implementation.

Key rename + BaseDef in the type chain
`_isCardInstance` → `_isCardInstanceFile`, canonical spelling `eq: false` (compiles to an adapter-agnostic existence test, fixing the old `eq:true` pg/sqlite divergence). One home in `search-doc-keys.ts` + an `excludeCardInstanceFileRows()` helper. BaseDef now terminates both instance and file type chains, so `{ type: baseRef }` spans both kinds and composes.

Smaller items
cardsOnly choosers hide file types in the type picker; trimmed the unread `markdown`/`isolated_html` from the mixed file projection; folded `searchEntriesFileMeta` into `searchEntries(..., 'file')`; documented `scope` + the mixed-search shared-key contract in `docs/search.md`.

Tests (green locally on both adapters)
Postgres `search-entries-engine` 29/29 and `indexing` 61/61; SQLite engine 79 tests / 168 assertions; matcher parity 32/78; query-builder 17/21; index-writer 52/142. New coverage for the composition fix (not-spec exclusion, intersection), BaseDef, the three scopes, `_isCardInstanceFile` true/false, and the matcher synthetic-key shim.

Deployment note: requires a full reindex (stamp rename + BaseDef in `types`). There is no index-version knob and the deploy auto-reindex only fires on a boxel-ui dist checksum change, so trigger it explicitly post-deploy (`_grafana-full-reindex` or `REALM_SERVER_FULL_INDEX_ON_STARTUP=true`). Host-after-server deploy order (an old server 400s on the new `scope` member).

@FadhlanR FadhlanR changed the title Search: mixed cards + files entry search; consumers pinned or deduped (CS-11775, CS-11776) feat: mixed cards + files entry search with scope and client parity Jul 13, 2026
Extend the realm `entry` search so one query can span card instances and
files, and address the review feedback on how kind selection and client-side
parity work.

Engine
- Replace the shared `jsonb_array_elements_text(types)` cross-join alias with a
  self-contained `TypesContains` membership predicate (pg `@>`, sqlite
  `json_each` EXISTS, COALESCE for NULL/absent types). Type conditions now
  compose correctly: `not:{type}` genuinely excludes, `every:[{type},{type}]`
  intersects, and rows with empty types survive negation.
- Support a mixed 'all' entry scope alongside 'instance'/'file'.

Wire scope
- Add `scope: 'cards' | 'files' | 'all'` (default 'all') to the entry wire
  query; it pins boxel_index.type directly, independent of filter shape. The
  federated-search cache key includes it. Consumers (store.search, host-mode
  head prefetch, create-listing, recents, choosers) pin scope instead of a
  synthetic-key incantation; `anchorQueryToCardInstances` is removed.

Client parity
- Teach the instance-filter matcher the synthetic search-doc keys (`_title`,
  `_cardType`, `_isCardInstanceFile`) via a shim at the top of resolvePath, and
  add an isClientEvaluable guard so an unshimmed underscore key is server-only
  rather than blanking reconciled results. `friendlyCardType` moves to
  runtime-common so the index stamp and matcher share one implementation.

Keys and types
- Name the dual-`.json` marker `_isCardInstanceFile` with canonical `eq: false`
  (existence-test compile, adapter-agnostic); one home in search-doc-keys.ts
  plus an `excludeCardInstanceFileRows()` helper.
- Append BaseDef to instance and file type chains so `{ type: baseRef }` spans
  both kinds and composes.

Cleanup + docs
- cardsOnly choosers hide file types in the type picker; trim unread
  markdown/isolated_html from the mixed file projection; fold
  searchEntriesFileMeta into searchEntries(..., 'file').
- Document scope + the mixed-search shared-key contract in docs/search.md.

Requires a full reindex on deploy (stamp rename + BaseDef in types); there is
no index-version knob, so trigger it explicitly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@FadhlanR FadhlanR force-pushed the cs-11775-search-api-mixed-entry-search-cards-files-with-card-instance branch from 5b89f06 to 3cde39e Compare July 13, 2026 11:29

@habdelra habdelra 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.

[Claude Code 🤖] Follow-up review of the reworked revision. TL;DR: this is an exemplary response to review — every architectural thread from the last pass was adopted and, in several places, improved on. The remaining asks are small: one doc correction, a few comment-tightenings (inline), and one real must-do — the PR description still describes the superseded design and needs a rewrite, including its deployment section.

What this revision gets right

I verified each of these against the branch rather than taking the diff's word for it:

  • Kind selection is now a first-class verb. The wire scope: 'cards' | 'files' | 'all' member (validated with a 400 on bad values, folded into the response cache key) replaces both filter-anchor gymnastics and the queryTargetsFileMeta routing heuristic. Internal callers and wire callers now speak the same language, and the single-URL GET pins scope by accept-header kind through the same parameter.
  • The dedup finally reads as what it does. _isCardInstance: nullexcludeCardInstanceFileRows() / eq: { _isCardInstanceFile: false }, compiled as an existence test (IS [NOT] NULL) — adapter-agnostic, polarity-clean, and eq: true gains a sensible meaning (select exactly the card-source .json rows, now tested). The rename plus search-doc-keys.ts as the single source of the synthetic-key spellings addresses the scattered-literals concern completely.
  • Type conditions now mean what they say. The TypesContains membership node (per-adapter rendering on the JsonContains pattern, with the COALESCE empty-array guard) fixes all three artifacts of the shared cross-join alias: not: { type: X } genuinely excludes, every of two type refs is an intersection, and empty-types rows no longer vanish. The new engine tests pin each of these — including the not-spec case that was silently broken before.
  • BaseDef as the composable universe anchor — with the chains extended consistently in all three getTypes implementations, and the prerender loop smartly skipping the abstract BaseDef ancestor rendering so the index doesn't bloat.
  • One assembly loop. searchEntriesFileMeta is gone; the 'file' scope routes through the unified searchEntries path, so file-row assembly can't drift.
  • Client parity is closed. The matcher shims all three synthetic keys (with friendlyCardType extracted to runtime-common so the _cardType stamp and the shim share one definition), files resolve as unresolvable (trust-the-server), and the isClientEvaluable underscore-guard turns any future unshimmed synthetic key into "no instant results" instead of "reconciler blanks correct results". Tests cover each shim and the guard.
  • The projection trim, the boxel-cli scope passthrough (folding in what was a follow-up ticket), the cards-only type-picker filtering, and the docs/search.md contract section all landed too.

On the clarity of "search both kinds"

That was the open design question, and I think this revision answers it well. There's now a layered, unambiguous vocabulary: scope says which row kinds a query spans (explicit 'all' for both); type refs narrow within that scope (with { type: BaseDef } as the kind-spanning anchor inside filter compositions, now that type conditions compose); excludeCardInstanceFileRows() is the one remaining special case — "each card once" in a mixed result — and its eq: false spelling plus the named helper make the intent legible at every call site. Two documentation nits keep that story crisp (inline on docs/search.md): _cardType is listed as a both-kinds key but is stamped on card rows only, and a sentence on when to reach for the BaseDef anchor vs scope: 'all' would pre-empt the "two spellings for the same thing" question.

The must-do: rewrite the PR description

The body still describes the previous design — "no wire member is added", the _isCardInstance: null idiom, anchorQueryToCardInstances, and a consumer table that no longer matches the code. Since this PR is the reference for how mixed search works, that's worth fixing before merge. In particular the deployment section needs re-deriving, because two things in this revision change the index-state story:

  1. The stamp rename: rows indexed under the old _isCardInstance key don't carry _isCardInstanceFile, so until a realm reindexes, its card .json file rows pass the new dedup filter and appear as duplicates in mixed results.
  2. BaseDef in types: { type: baseRef } matches nothing on rows indexed before this deploy.

Both self-heal through the deploy-triggered full reindex, but the visible window (transient dupes; BaseDef anchors under-matching) is exactly what the operational notes exist to describe — and the current text's "verify a reindex has completed since the stamping deploy" prerequisite is now obsolete (the relevant reindex is the one this deploy triggers).

Small things (inline threads)

store.search's "positive type ref selects a kind" premise now has two exceptions (BaseDef refs, mixed any:) worth naming in its comment; the fieldEqFilter special case implies a _-prefix reservation that should be stated once in search-doc-keys.ts; TableValuedEach and its SQLite adjustSQL rewrites are now dead code; a GIN index on types is the natural perf follow-up now that type filters are @>-served; and the cache key can normalize explicit scope: 'all' to match the absent default.

None of the inline items block merge on their own — the description rewrite is the one thing I'd insist on before this lands.

Comment thread docs/search.md

- `matches` — full-text search over both kinds' content.
- `_title` — the row's display title (a card's `cardTitle`, a file's name); usable in `contains` and `sort`.
- `_cardType` — the card type's display name.

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.

[Claude Code 🤖] One correction and one addition to this key-contract list — worth getting exactly right since this section is now the canonical answer to "how do I query across both kinds":

  1. _cardType doesn't belong under "keys both row kinds carry" — it's stamped only during card render (render/meta.ts); file rows never get it (that asymmetry is exactly what makes cards-grid's not: { eq: { _cardType: null } } a cards-only filter). Listing it here invites someone to sort a mixed result set on _cardType and wonder why every file sinks to the NULLS-LAST tail. I'd move it out of this list and describe it as a card-only synthetic — useful in mixed queries precisely because files lack it.

  2. A sentence on when { type: <BaseDef> } beats scope: 'all' would head off the "we now have two spellings for everything" confusion: scope is query-global (it pins boxel_index.type for the whole request), while a BaseDef anchor is a filter node — composable inside the tree, e.g. one branch of an any: that must span both kinds while a sibling branch is card-typed, or an intersection with another type condition. If a caller just wants "everything", scope: 'all' is the spelling; the BaseDef anchor is for kind-spanning within a filter composition.

? getTypeRefsFromFilter(query.filter)
: undefined;
let hasPositiveType = typeRefs?.some((r) => !r.negated) ?? false;
let scope: SearchEntryScope | undefined = hasPositiveType

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.

[Claude Code 🤖] Making store.search the choke point is the right structure — every caller (search-cards tool, preselected-card query, and anything future) now inherits the pin without per-call-site edits. Two exceptions have crept into the premise "a positive type ref already selects a kind", though, both introduced elsewhere in this same PR:

  1. { type: baseRef } is now a positive ref that selects both kinds (BaseDef terminates both chains as of this PR). A store.search({ filter: { type: baseRef } }) therefore takes the hasPositiveType branch, stays 'all'-scoped, and returns FileDef hydrations plus both rows of every card .json — probably not what a store.search caller ever wants, given the method's "instances only"-flavored contract.
  2. A mixed any:any: [{ type: SomeCard }, { contains: { _title: 'x' } }] counts as positively typed, but its untyped branch still matches file rows in 'all' scope.

Neither has a live caller today, so this is a comment-tightening ask, not a bug report: the doc comment should say the pass-through is for concrete card/file type refs, and name these two shapes as the known gaps. If you want to close gap 1 cheaply, treat a baseRef-equivalent ref as not kind-selecting (pin 'cards' for it) — store.search's contract arguably wants that regardless.

polarity: FilterPolarity = 'positive',
): CardExpression {
if (
currentField(key) === CARD_INSTANCE_FILE_KEY &&

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.

[Claude Code 🤖] The existence-test compilation is the cleanest possible resolution of the null-vs-false ambiguity — eq: false now reads as what it does, IS [NOT] NULL sidesteps the pg-'true'-vs-sqlite-1 rendering divergence entirely, and polarity inverts for free. Nice.

One precedence subtlety worth a sentence in the comment: this special case keys on the leaf name (currentField(key)) and fires before any definition lookup, so a card that defines a real boolean field literally named _isCardInstanceFile gets existence semantics instead of value comparison (eq: false would fail to match a card whose field is explicitly false — the key is present). That's the opposite precedence from the getField shim below, which only fabricates a synthetic definition when the real-field lookup fails. Checking the definition first isn't practical here (this method is sync; lookup is async), and the matcher's isClientEvaluable guard already treats every unshimmed _-prefixed key as server-only — so the system has de facto reserved _-prefixed top-level keys for synthetics. I'd just make that reservation explicit: one sentence in search-doc-keys.ts (and the docs' key-contract section) saying user-defined fields must not use _-prefixed names at the search-doc top level, rather than trying to make this branch definition-aware.

// semantics that miscompose under AND/NOT — this is a single per-row scalar
// predicate, so type conditions compose correctly (a real `NOT` exclusion, an
// AND intersection) and no GROUP BY is needed to recollapse the fan-out.
export interface TypesContains {

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.

[Claude Code 🤖] This node is exactly the right shape — per-adapter rendering like JsonContains, and the COALESCE guard means an empty/never-indexed types is a definite FALSE instead of a vanished row, which fixes the third artifact of the old fan-out (rows disappearing whenever any filter branch referenced the alias). The engine tests pinning not:-exclusion and every:-intersection close out what were previously silent misfires.

Two small follow-ons, neither blocking:

  1. TableValuedEach is now consumer-less. typeCondition, hasInstanceType, and hasFileType were its only emitters; what remains is the node kind, its factory, its expressionToSql branch, and — easy to forget — the SQLite adjustSQL rewrites that existed solely for it (jsonb_array_elements_text(json_each(, the _array_element alias fixup, and the case jsonb_typeof … end strip). Dead per-adapter rewrite machinery is where bit-rot hides; worth deleting, here or as an immediate follow-up.
  2. Postgres can now index-serve type filters. The old cross-join could never use an index; types @> '["key"]'::jsonb can, given CREATE INDEX … USING gin (types jsonb_path_ops) on boxel_index(+_working). Type-anchored queries are the hottest filter shape in the system, so that migration is probably worth a ticket even if perf is acceptable today.

// same query would collide on one ETag/body. Fold only when set (absent ==
// the default 'all', so keying `undefined` would not fragment but omitting
// it keeps the key identical to pre-scope requests).
if (parsed.scope) {

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.

[Claude Code 🤖] Good catch folding scope into the cache key — I'd missed that interaction entirely. One residual asymmetry: an explicit scope: 'all' is truthy, so it keys differently from an absent scope even though the two are semantically identical — same query, two cache entries/ETags. Normalizing the default away keeps the comment's own goal ("absent == the default 'all'") true for both spellings:

if (parsed.scope && parsed.scope !== 'all') {
  cacheKeyOpts.scope = parsed.scope;
}

The sheet's base filter was `any: [CardDef, FieldDef]`, which no file row
satisfies (file type chains run MarkdownDef → FileDef → BaseDef), so a typed
term could never surface a file even though the search runs at the mixed
scope — while picking a file type worked, because a picked type strips the
type-only base filter entirely.

- Base the sheet on `{ type: baseRef }`: BaseDef is the common ancestor
  stamped on both instance and file type chains, so one ref spans cards
  (including specs), field instances, and files.
- Send the wire scope explicitly: `searchScopeForOptions` now returns 'all'
  for the mixed case instead of leaving scope unset.
- Root refs (BaseDef/CardDef/FieldDef/FileDef) no longer suppress the
  card-json dedup filter: they span kinds rather than narrowing to one, so a
  base filter built from them still needs `excludeCardInstanceFileRows()` —
  otherwise every card would surface twice via its dual-indexed `.json` file
  row. A picked (narrowing) type still skips the dedup.
- Add `baseFileRef` to the type picker's root set so the widened base filter
  reads as unconstrained and base FileDef stays out of the picker options.
- Placeholder copy now mentions files.

Tests: new acceptance module covering files-by-term, card dedup, and spec
inclusion in the sheet; query-builder unit coverage for the root-ref dedup
rules; the chooser search test's term becomes "Mark Jackson" since word-AND
full-text over the now-included skills-realm docs also matched "Mark J".

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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