diff --git a/packages/boxel-cli/src/commands/search.ts b/packages/boxel-cli/src/commands/search.ts index 32f2c71fcb..a34254ff83 100644 --- a/packages/boxel-cli/src/commands/search.ts +++ b/packages/boxel-cli/src/commands/search.ts @@ -7,6 +7,7 @@ import { import { ensureTrailingSlash } from '@cardstack/runtime-common/paths'; import { resolveRealmIdentifier } from '../lib/resolve-realm-identifier.ts'; import { resourceIdentity } from '@cardstack/runtime-common/resource-identity'; +import { CARD_INSTANCE_FILE_KEY } from '@cardstack/runtime-common/search-doc-keys'; import { FG_RED, DIM, RESET } from '../lib/colors.ts'; import { cliLog } from '../lib/cli-log.ts'; @@ -206,6 +207,95 @@ export function itemsFromSearchEntryDoc( return items; } +// A mixed (`scope: 'all'`) entry search returns both rows of a dual-indexed +// card `.json` — an `instance` row and a `file` row — so a card `.json` shows +// up twice unless the filter discriminates. The dedup filter keeps card +// instances and plain files but drops the redundant `.json` file row: the +// `_isCardInstanceFile` key is stamped only on that file row, so `eq: false` +// (which matches an absent key too) keeps every other row and drops it. +// +// Mirrors the host's search-sheet dedup (`excludeCardInstanceFileRows` / +// `scopeFilters` in host `card-search/query-builder.ts`). The type-anchor +// detection mirrors `getTypeRefsFromFilter` / `hasNarrowingPositiveTypeRef` +// there, reimplemented locally — with the root refs as literals — because both +// runtime-common modules pull the `@cardstack/base/*` graph into boxel-cli's +// dependency-light build (the same boundary the wire-translation helpers above +// note). + +// 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 still needs +// the dedup. The base-realm module can be spelled as its URL or its scoped +// alias; both resolve to the same module server-side. +const ROOT_TYPE_REF_MODULES = [ + 'https://cardstack.com/base/card-api', + '@cardstack/base/card-api', +]; +const ROOT_TYPE_REF_NAMES = ['BaseDef', 'CardDef', 'FieldDef', 'FileDef']; + +function isRootTypeRef(ref: unknown): boolean { + if (typeof ref !== 'object' || ref == null) { + return false; + } + let { module, name } = ref as { module?: unknown; name?: unknown }; + return ( + typeof module === 'string' && + typeof name === 'string' && + ROOT_TYPE_REF_MODULES.includes(module) && + ROOT_TYPE_REF_NAMES.includes(name) + ); +} + +// True when the filter carries a positive (non-negated), kind-narrowing +// `type`/`on` anchor. A narrowing type discriminates the kind by itself — a +// card type matches only instance rows (no dupe to drop), a file type must +// stay free to surface a `.json` file row that legitimately matches it — so +// the dedup is skipped. Polarity flips under each enclosing `not`, so a +// negated anchor doesn't count — and neither does a kind-spanning root ref +// (see `isRootTypeRef`). +function hasNarrowingTypeAnchor(filter: unknown, negated = false): boolean { + if (typeof filter !== 'object' || filter == null || Array.isArray(filter)) { + return false; + } + let f = filter as Record; + if (f.on != null || f.type != null) { + return !negated && !isRootTypeRef(f.on ?? f.type); + } + if (f.not != null) { + return hasNarrowingTypeAnchor(f.not, !negated); + } + if (Array.isArray(f.every)) { + return f.every.some((node) => hasNarrowingTypeAnchor(node, negated)); + } + if (Array.isArray(f.any)) { + return f.any.some((node) => hasNarrowingTypeAnchor(node, negated)); + } + return false; +} + +/** + * Make a search query invariant to the mixed entry-search default: compose the + * `_isCardInstanceFile` dedup filter so each card `.json` surfaces once (via its + * instance row) while plain files stay listed. + * + * Skipped when the wire `scope` already pins a single kind (`'cards'`/`'files'`) + * or the filter carries a narrowing positive type anchor — the same rule the + * host applies. Exported for unit testing. + */ +export function composeMixedScopeDedup( + query: Record, +): Record { + if (query.scope === 'cards' || query.scope === 'files') { + return query; + } + if (hasNarrowingTypeAnchor(query.filter)) { + return query; + } + let dedup = { eq: { [CARD_INSTANCE_FILE_KEY]: false } }; + let filter = query.filter == null ? dedup : { every: [query.filter, dedup] }; + return { ...query, filter }; +} + /** * Federated search across one or more realms via the `_federated-search` * server endpoint. @@ -233,6 +323,8 @@ export async function search( let realmServerUrl = active.profile.realmServerUrl.replace(/\/$/, ''); let searchUrl = `${realmServerUrl}/_federated-search`; + query = composeMixedScopeDedup(query); + let realms: string[] = []; for (let realm of Array.isArray(realmUrls) ? realmUrls : [realmUrls]) { let resolved = resolveRealmIdentifier(realm, { profileManager: pm }); diff --git a/packages/boxel-cli/tests/commands/search-query.test.ts b/packages/boxel-cli/tests/commands/search-query.test.ts index 026b7a5c99..618b07af40 100644 --- a/packages/boxel-cli/tests/commands/search-query.test.ts +++ b/packages/boxel-cli/tests/commands/search-query.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect } from 'vitest'; import { searchEntryRequestBody, itemsFromSearchEntryDoc, + composeMixedScopeDedup, } from '../../src/commands/search.ts'; const SkillRef = { @@ -12,6 +13,12 @@ const CardDefRef = { module: 'https://cardstack.com/base/card-api', name: 'CardDef', }; +const BaseDefRef = { + module: 'https://cardstack.com/base/card-api', + name: 'BaseDef', +}; + +const DEDUP = { eq: { _isCardInstanceFile: false } }; describe('searchEntryRequestBody — card-rooted query → entry wire grammar', () => { it('always requests the data-only fieldset and the given realms', () => { @@ -182,3 +189,81 @@ describe('itemsFromSearchEntryDoc — flatten a data-only entry doc to items', ( expect(itemsFromSearchEntryDoc({})).toEqual([]); }); }); + +describe('composeMixedScopeDedup — invariant mixed-scope output', () => { + it('injects the dedup as the sole filter when the query has none', () => { + expect(composeMixedScopeDedup({})).toEqual({ filter: DEDUP }); + }); + + it('injects the dedup for a cardUrls-only lookup (no filter)', () => { + let query = { cardUrls: ['https://realm/a/x.json'] }; + expect(composeMixedScopeDedup(query)).toEqual({ + cardUrls: ['https://realm/a/x.json'], + filter: DEDUP, + }); + }); + + it('ANDs the dedup with an existing anchorless filter', () => { + expect(composeMixedScopeDedup({ filter: { matches: 'hello' } })).toEqual({ + filter: { every: [{ matches: 'hello' }, DEDUP] }, + }); + expect( + composeMixedScopeDedup({ filter: { eq: { _title: 'Mango' } } }), + ).toEqual({ filter: { every: [{ eq: { _title: 'Mango' } }, DEDUP] } }); + }); + + it('leaves a narrowing positive type anchor unchanged', () => { + let byType = { filter: { type: SkillRef } }; + expect(composeMixedScopeDedup(byType)).toBe(byType); + let byOn = { filter: { on: SkillRef, eq: { skillTitle: 'x' } } }; + expect(composeMixedScopeDedup(byOn)).toBe(byOn); + let nested = { + filter: { every: [{ type: SkillRef }, { eq: { status: 'active' } }] }, + }; + expect(composeMixedScopeDedup(nested)).toBe(nested); + }); + + it('still dedups when the only anchor is a kind-spanning root ref', () => { + // Every file row carries BaseDef in its type chain, so a root-ref anchor + // matches both rows of a card `.json` — it doesn't narrow the kind. + expect(composeMixedScopeDedup({ filter: { type: BaseDefRef } })).toEqual({ + filter: { every: [{ type: BaseDefRef }, DEDUP] }, + }); + let byOn = { filter: { on: CardDefRef, eq: { cardTitle: 'x' } } }; + expect(composeMixedScopeDedup(byOn)).toEqual({ + filter: { every: [byOn.filter, DEDUP] }, + }); + // The scoped-alias spelling of the base-realm module counts too. + let aliasRef = { module: '@cardstack/base/card-api', name: 'FileDef' }; + expect(composeMixedScopeDedup({ filter: { type: aliasRef } })).toEqual({ + filter: { every: [{ type: aliasRef }, DEDUP] }, + }); + }); + + it('still dedups when the type anchor is negated', () => { + expect( + composeMixedScopeDedup({ filter: { not: { type: SkillRef } } }), + ).toEqual({ filter: { every: [{ not: { type: SkillRef } }, DEDUP] } }); + }); + + it('skips the dedup when scope already pins a single kind', () => { + let cards = { scope: 'cards' }; + expect(composeMixedScopeDedup(cards)).toBe(cards); + let files = { scope: 'files' }; + expect(composeMixedScopeDedup(files)).toBe(files); + }); + + it('injects the dedup for the explicit mixed scope', () => { + expect(composeMixedScopeDedup({ scope: 'all' })).toEqual({ + scope: 'all', + filter: DEDUP, + }); + }); + + it('translates to the item.-prefixed dedup key on the wire', () => { + let body = searchEntryRequestBody(composeMixedScopeDedup({}), [ + 'https://realm/a/', + ]); + expect(body.filter).toEqual({ eq: { 'item._isCardInstanceFile': false } }); + }); +}); diff --git a/packages/boxel-cli/tests/integration/search.test.ts b/packages/boxel-cli/tests/integration/search.test.ts index 00013df10d..899bc3c87b 100644 --- a/packages/boxel-cli/tests/integration/search.test.ts +++ b/packages/boxel-cli/tests/integration/search.test.ts @@ -56,6 +56,10 @@ beforeAll(async () => { }, }, }), + // A plain (non-card) file: it has only a `file` row, so it must still + // appear in a mixed list-all after the card `.json` file rows are + // deduped away. + 'readme.txt': 'plain file contents', }, permissions: { [ownerUserId]: ['read', 'write', 'realm-owner'], @@ -86,17 +90,47 @@ afterAll(async () => { }); describe('federated search (integration)', () => { - it('returns all results when no filter is supplied', async () => { + it('returns each card once (no `.json` file-row dupe) plus plain files', async () => { let result = await search(realmHref, {}, { profileManager }); expect(result.ok, `search failed: ${result.error}`).toBe(true); - let titles = (result.data ?? []).map( - (entry) => - (entry as { attributes?: { cardTitle?: string } }).attributes - ?.cardTitle, - ); - expect(titles).toEqual( - expect.arrayContaining(['Shared Card', 'Other Card']), + let entries = (result.data ?? []) as { + id?: string; + type?: string; + attributes?: { cardTitle?: string }; + }[]; + + // Each card `.json` is dual-indexed (an instance row + a file row); the + // dedup drops the file row so the card appears exactly once. + let cardTitles = entries + .filter((e) => e.type === 'card') + .map((e) => e.attributes?.cardTitle) + .sort(); + expect(cardTitles).toEqual(['Other Card', 'Shared Card']); + + // No id appears twice. + let ids = entries.map((e) => e.id); + expect(ids.length).toBe(new Set(ids).size); + + // The plain (non-card) file is still listed. + let fileIds = entries + .filter((e) => e.type === 'file-meta') + .map((e) => e.id); + expect(fileIds).toContain(`${realmHref}readme.txt`); + // ...and neither card's `.json` file row leaked in. + expect(fileIds).not.toContain(`${realmHref}shared-card.json`); + expect(fileIds).not.toContain(`${realmHref}other-card.json`); + }); + + it('returns a single entry for a cardUrls `.json` lookup', async () => { + let result = await search( + realmHref, + { cardUrls: [`${realmHref}shared-card.json`] }, + { profileManager }, ); + expect(result.ok, `search failed: ${result.error}`).toBe(true); + let entries = result.data ?? []; + expect(entries.length).toBe(1); + expect((entries[0] as { id?: string }).id).toBe(`${realmHref}shared-card`); }); it('filters by cardTitle and returns only the matching card', async () => {