Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 92 additions & 0 deletions packages/boxel-cli/src/commands/search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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<string, unknown>;
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));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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 👍 / 👎.

}
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<string, unknown>,
): Record<string, unknown> {
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.
Expand Down Expand Up @@ -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 });
Expand Down
85 changes: 85 additions & 0 deletions packages/boxel-cli/tests/commands/search-query.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { describe, it, expect } from 'vitest';
import {
searchEntryRequestBody,
itemsFromSearchEntryDoc,
composeMixedScopeDedup,
} from '../../src/commands/search.ts';

const SkillRef = {
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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 } });
});
});
50 changes: 42 additions & 8 deletions packages/boxel-cli/tests/integration/search.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand Down Expand Up @@ -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 () => {
Expand Down
Loading