fix: dedup card .json file rows in boxel-cli search output#5514
Conversation
The realm-server's mixed entry-search default now returns both an `instance` row and a `file` row for every dual-indexed card `.json`. Compose the `_isCardInstanceFile` dedup filter into `boxel search` queries that lack a narrowing positive type anchor (and aren't already pinned to a single kind via `scope`), so each card surfaces once while plain files stay listed. Mirrors the host search-sheet dedup. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The `@cardstack/runtime-common/constants` import pulled runtime-common's `@cardstack/base/*` type graph into boxel-cli's `tsc --noEmit`, breaking CI lint. Detect a positive type anchor with a local filter walk instead, using the ticket's rule (skip dedup on any positive type/on anchor) — `_isCardInstanceFile` still comes from the import-clean search-doc-keys. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Root refs (BaseDef/CardDef/FieldDef/FileDef) span kinds rather than narrowing to one — every file row carries BaseDef in its type chain, so a type: BaseDef query matches both rows of a card .json and would leak duplicates when it suppressed the dedup. Match the host's hasNarrowingPositiveTypeRef rule, with the root refs as local literals (URL and scoped-alias module spellings) so the dependency-light build stays free of the runtime-common constants import. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1420f14e9
ℹ️ 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".
| return f.every.some((node) => hasNarrowingTypeAnchor(node, negated)); | ||
| } | ||
| if (Array.isArray(f.any)) { | ||
| return f.any.some((node) => hasNarrowingTypeAnchor(node, negated)); |
There was a problem hiding this comment.
Require every OR branch to narrow before skipping dedup
When an any filter has only one narrowing branch, this some makes composeMixedScopeDedup skip the _isCardInstanceFile filter for the whole mixed search. For example, { any: [{ type: SomeCard }, { contains: { _title: "Mango" } }] } can still match both the card instance row and its .json file row through the untyped _title branch, so the duplicate/leaked file-meta row returns despite this change's goal. Only skip the dedup when the entire filter is kind-narrowing, not merely one OR alternative.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR updates boxel search (boxel-cli) to deduplicate “mixed-scope” entry search results so dual-indexed card .json files no longer surface twice (instance row + file row) under the realm-server’s mixed entry-search default, while still preserving plain file results.
Changes:
- Add
composeMixedScopeDedup(query)to inject an_isCardInstanceFile: falsefilter when the query is mixed-scope and not already kind-narrowed by a positive, non-root type anchor. - Apply the dedup composition at the start of the CLI’s federated
search()so all callers benefit. - Extend unit + integration tests to cover list-all,
cardUrlslookups, scope/type-anchor skip rules, and ensuring plain files still appear.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/boxel-cli/src/commands/search.ts | Adds mixed-scope dedup filter composition and applies it to federated search requests. |
| packages/boxel-cli/tests/commands/search-query.test.ts | Adds unit tests for composeMixedScopeDedup behavior and wire translation of the synthetic dedup key. |
| packages/boxel-cli/tests/integration/search.test.ts | Adds integration coverage ensuring cards are not duplicated and plain files still appear in mixed list-all; verifies cardUrls .json lookup returns one entry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Background and Goal
CS-11775 shipped the realm-server "mixed entry search" default:
_search/_federated-searchnow return both aninstancerow and afilerow for every dual-indexed card.json, unless the caller's filter discriminates. That leaves threeboxel searchcases returning duplicated/leaked rows: filter-less list-all,cardUrlslookups (a card.jsonURL matches both of its rows), and term searches on keys files also carry (_title).This makes
boxel searchoutput invariant to that default — each card surfaces once (via its instance row), plain files stay listed, and type-anchored searches are unchanged.Where to start
packages/boxel-cli/src/commands/search.ts— newcomposeMixedScopeDedup(query)composes the_isCardInstanceFilededup filter ({ eq: { _isCardInstanceFile: false } }, which matches an absent key too and drops the redundant.jsonfile row). It's applied at the top ofsearch(), so bothboxel searchandBoxelCLIClient.searchare covered.Key decisions and non-obvious mechanics
excludeCardInstanceFileRows/scopeFiltersinpackages/host/app/utils/card-search/query-builder.ts). The dedup is skipped whenscopealready pins a single kind ('cards'/'files') or the filter carries a narrowing positive type anchor. Root refs (BaseDef/CardDef/FieldDef/FileDef) don't count as narrowing — they span kinds, so atype: baseRefquery still needs the dedup — matchinghasNarrowingPositiveTypeRefon the host.tsc --noEmit, no glint/content-tag), while@cardstack/baseis the base realm's.gtscontent: it only resolves through the host's tsconfigpathsmapping (@cardstack/base/*/https://cardstack.com/base/*→packages/base/*.gts) plus the Ember/Glint toolchain that can parse<template>tags — and its runtime graph (@glimmer/component,ember-concurrency, …) doesn't execute under plain Node. The refs aren't defined there anyway; the nearest source isruntime-common/constants, but its type imports reach the runtime-common index barrel, which drags content-tag and the virtual@cardstack/base/*modules into the CLI's type-check (TS2307). So, like the existing localtoItemFilter/SearchEntryDoc, the type-anchor detection is reimplemented locally: onlyCARD_INSTANCE_FILE_KEY(fromsearch-doc-keys, zero imports) is imported, and the root refs are literals. The literals also cover both base-realm module spellings (URL and@cardstack/base/alias) — CLI users author raw JSON and may write either, whereasconstantscarries only the alias form. These refs are part of the public query grammar, as stable as theon/typekeywords themselves; if a single source of truth is wanted later, the move is extracting them into an import-clean runtime-common subpath alongsidesearch-doc-keys, as a separate refactor.{ eq: { _isCardInstanceFile: false } }with no explicit anchor does not restrict to card types — the engine only emits atypes containsrestriction from an expliciton/type, so the existence test compiles toIS NULLand plain file rows survive.Release ordering
The
_isCardInstanceFilefield ships with the realm-server (CS-11775, PR #5428); an old server rejects it as unknown. This CLI release must go out with or after that realm-server deploy.Tests
composeMixedScopeDedupcovers no-filter,cardUrls-only, anchorless-filter AND-compose, positive type anchor (skip), root ref (dedup), negated type (dedup),scope: cards|files(skip) /scope: all(dedup), and the wireitem._isCardInstanceFiletranslation.readme.txt, no.jsonfile-row leaks; acardUrls.jsonlookup returns a single entry.🤖 Generated with Claude Code