diff --git a/.changeset/tall-jars-invent.md b/.changeset/tall-jars-invent.md new file mode 100644 index 0000000000..712099056c --- /dev/null +++ b/.changeset/tall-jars-invent.md @@ -0,0 +1,18 @@ +--- +'@objectstack/metadata-protocol': patch +--- + +Global search (`GET /api/v1/search`) now resolves searchable fields the same way `$search` does, so the ⌘K palette recalls what the list quick-search recalls (#7643) + +`searchAll` built its own filter instead of going through the engine's ADR-0061 `$search` expansion, which made the palette's recall a strict subset of the executor's. It now hands the engine `search: ` per object and lets one expansion resolve the fields and compile the clause. + +What a caller observes changing on `GET /api/v1/search` — both are widenings; no query that returned a hit before returns fewer: + +- **Pinyin/initials recall now works on this endpoint.** Where the deployment provisions the hidden `__search` companion column (`OS_SEARCH_PINYIN_ENABLED`), latin terms are OR-ed against it, so `hnkj` and `huaningkeji` now return the CJK-named record that `POST /api/v1/data/:object/query {"search":"hnkj"}` already returned. Previously: 0 hits. +- **Which columns are scanned now follows the object, not a field flag.** Resolution is the object's declared `searchableFields`, else the auto-default (display/name field plus short-text and enum fields) — the set `searchableFields` documents itself as governing. The endpoint previously scanned only text-typed fields carrying the field-level `searchable: true` flag, falling back to the title field alone, so most objects were searched on one column. Hits from a second column (an email, a description, a select's label) are new. +- Enum (`select`/`status`) columns are now matched by option LABEL, and virtual `formula` fields are excluded, both as on the executor path. +- **The endpoint no longer substring-scans primary keys.** An object whose only text-typed column is `id` — system tables, junction tables, append-only logs — used to fall through to "the first text-typed field" and be queried as `{id: {$icontains: term}}` on every keystroke. Such objects are now skipped, as `$search` already skipped them (#4483). Callers relying on a bare `id` fragment matching through this endpoint will no longer get that hit; query the record by id instead. + +Unchanged: which objects are swept and their opt-outs (`enable.searchable`, `enable.apiEnabled`, the `sys_*` skips), the per-object and overall caps, ordering, RLS/RBAC enforcement, and the response shape. The `$search` executor path itself is untouched. A record matched only through the pinyin companion has no `snippet` — no source column contains the typed term. + +Also corrects the stale case declaration on this path (#7850): the doc comment said "case-insensitive LIKE" while the sentence below it named `$contains`, which #4706 Q2 = A defines as case-**sensitive**. Matching folds case via `$icontains`; behaviour is unchanged by that edit. diff --git a/packages/metadata-protocol/src/protocol.search-case-fold.test.ts b/packages/metadata-protocol/src/protocol.search-case-fold.test.ts index f6b6e69cef..c2f4030a4d 100644 --- a/packages/metadata-protocol/src/protocol.search-case-fold.test.ts +++ b/packages/metadata-protocol/src/protocol.search-case-fold.test.ts @@ -1,22 +1,37 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. // -// [#7641] `searchAll` — the global-search palette — is the SECOND producer of -// search clauses, and it carried the same declared≠enforced defect as -// objectql's `search-filter.ts`: it built its AND-of-OR from `$contains` under -// a comment asserting that `$contains` was "case-insensitive substring -// matching". It is not — #4706 Q2 = A rules the `$contains` family case- -// SENSITIVE, and `$icontains` is the operator that folds. +// [#7641 → #7643] `searchAll` — the global-search palette — used to be the +// SECOND producer of search clauses, and it carried the same declared≠enforced +// defect as objectql's `search-filter.ts`: it built its AND-of-OR from +// `$contains` under a comment asserting that `$contains` was "case-insensitive +// substring matching". It is not — #4706 Q2 = A rules the `$contains` family +// case-SENSITIVE, and `$icontains` is the operator that folds. #7641 fixed the +// operator in both producers. // -// The two producers were found and fixed together, but they are NOT one code -// path: `search-filter.ts` serves per-object `find({ $search })` and this one -// serves `GET /api/v1/search`. `search.console-global-search`'s knownGaps had -// already recorded that the palette inherits the gap and that #7641 owns it. +// #7643 removed the second producer instead: the palette now hands the engine +// `search: ` and the ADR-0061 expansion resolves the fields and compiles the +// clause, so there is one definition of recall rather than two that agreed by +// maintenance. That moves what THIS file can honestly assert. // -// Why this asserts on the FILTER handed to `engine.find` rather than on matched -// rows: the fake below deliberately does not implement filtering, so a row -// assertion here would pass against any filter at all. The operator IS the -// contract this producer is responsible for — every backend that executes it is -// separately conformance-checked against `$icontains` (`FILTER_TEXT_CASES`). +// ## What moved, and where it went +// +// The operator pin — "$icontains on source columns, never $contains" — is now a +// claim about a filter the ENGINE produces, so it cannot be observed here: this +// suite's fake engine has no expansion, and asserting on the options bag would +// only re-state the delegation. It is pinned end-to-end, over a real `ObjectQL` +// and a real driver, in `objectql/src/global-search-palette-recall.test.ts` +// ("source columns still compile to $icontains"), alongside the companion-recall +// parity that was #7643's actual defect. +// +// ## What is pinned HERE +// +// The DELEGATION itself, which is this package's own contract surface and is +// invisible from the engine side: `searchAll` must hand the engine the query +// TEXT and no filter of its own. A regression that rebuilt a local `where` — +// the exact shape of the #7643 defect — would restore a second producer, and +// every parity test one package over would keep passing right up until the two +// definitions drifted again. That is what this file exists to catch, and why it +// was rewritten rather than deleted. import { describe, it, expect, vi } from 'vitest'; import { ObjectStackProtocolImplementation } from './protocol.js'; @@ -27,11 +42,10 @@ interface SearchRow { updated_at: string; } -/** The shape `searchAll` builds: `{$or:[…]}`, or `{$and:[{$or:[…]}, …]}`. */ -type SearchFilter = Record; - interface FindOptions { - where?: SearchFilter; + where?: Record; + search?: unknown; + searchFields?: unknown; orderBy?: Array<{ field: string; order?: string }>; limit?: number; } @@ -43,16 +57,16 @@ const ROWS: SearchRow[] = [ const CONTACT = { name: 'contact', - fields: { name: { name: 'name', type: 'text', searchable: true } }, + fields: { name: { name: 'name', type: 'text' } }, }; function makeProtocol(): { p: ObjectStackProtocolImplementation; find: ReturnType; } { - // No filtering: see the header — what is under test is the filter this - // producer EMITS, so the double must not be able to satisfy an assertion - // by filtering correctly on its own. + // No filtering and no `$search` expansion: this double stands in for the + // engine BOUNDARY, not for the engine. What is under test is what the + // protocol hands across it. const find = vi.fn(async (_object: string, _opts: FindOptions = {}) => ROWS); const engine = { registry: { @@ -64,47 +78,54 @@ function makeProtocol(): { return { p: new ObjectStackProtocolImplementation(engine as never), find }; } -/** The filter the protocol handed to `engine.find` on its first call. */ -function filterFrom(find: ReturnType): SearchFilter { - const opts = find.mock.calls[0][1] as FindOptions; - return opts.where as SearchFilter; +/** The options bag the protocol handed to `engine.find` on its first call. */ +function optionsFrom(find: ReturnType): FindOptions { + return find.mock.calls[0][1] as FindOptions; } -describe('[#7641] searchAll compiles to the case-folding operator', () => { - it('emits $icontains — never the case-SENSITIVE $contains', async () => { +describe('[#7643] searchAll delegates recall to the engine', () => { + it('sends the query TEXT and builds no filter of its own', async () => { const { p, find } = makeProtocol(); await p.searchAll({ q: 'retail', perObject: 5 }); - expect(filterFrom(find)).toEqual({ $or: [{ name: { $icontains: 'retail' } }] }); - // Spelled separately so a regression reads as "went back to the - // case-sensitive operator" rather than as an object-shape diff. - expect(JSON.stringify(filterFrom(find))).not.toContain('$contains'); + const opts = optionsFrom(find); + expect(opts.search).toBe('retail'); + // The whole point: a locally compiled predicate is what #7643 removed. + expect(opts.where).toBeUndefined(); + }); + + it('passes the query verbatim — no pre-tokenising, no per-field fan-out', async () => { + // A multi-term query used to become `{$and:[{$or:[…]},{$or:[…]}]}` here. + // Term semantics are unchanged, but they are now the engine's to apply; + // splitting the text before handing it over would be a second + // definition of "what a term is" in the very place this card merged. + const { p, find } = makeProtocol(); + await p.searchAll({ q: 'acme retail', perObject: 5 }); + + const opts = optionsFrom(find); + expect(opts.search).toBe('acme retail'); + expect(opts.where).toBeUndefined(); }); - it('picks the operator by field type, not by the term\'s own casing', async () => { - // The defect was invisible whenever the term's casing happened to match - // the stored value. Both spellings must compile the same way — folding - // is the operator's job, not the caller's. - const lower = makeProtocol(); - await lower.p.searchAll({ q: 'retail', perObject: 5 }); - const upper = makeProtocol(); - await upper.p.searchAll({ q: 'Retail', perObject: 5 }); + it('sends no searchFields — the palette wants the object\'s full default reach', async () => { + // `searchFields` can only NARROW the resolved set (ADR-0061). Sending + // one would reintroduce a palette-specific field policy through the + // back door, which is the same defect wearing the engine's key. + const { p, find } = makeProtocol(); + await p.searchAll({ q: 'retail', perObject: 5 }); - expect(filterFrom(lower.find)).toEqual({ $or: [{ name: { $icontains: 'retail' } }] }); - expect(filterFrom(upper.find)).toEqual({ $or: [{ name: { $icontains: 'Retail' } }] }); + expect(optionsFrom(find).searchFields).toBeUndefined(); }); - it('folds every term of a multi-term query, which stays AND-of-OR', async () => { + it('still caps and orders per object', async () => { + // Unchanged by #7643 and asserted so the delegation cannot quietly take + // the cross-object half — the one part that IS this method's own — with + // it. `order`, not `direction` (#4674). const { p, find } = makeProtocol(); - await p.searchAll({ q: 'acme retail', perObject: 5 }); + await p.searchAll({ q: 'retail', perObject: 3 }); - // Term semantics are untouched by #7641 — asserted here so the operator - // change is pinned as operator-only. - expect(filterFrom(find)).toEqual({ - $and: [ - { $or: [{ name: { $icontains: 'acme' } }] }, - { $or: [{ name: { $icontains: 'retail' } }] }, - ], - }); + const opts = optionsFrom(find); + expect(opts.limit).toBe(3); + expect(opts.orderBy).toEqual([{ field: 'updated_at', order: 'desc' }]); }); }); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 6e21b596d4..7921ac807d 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -8506,16 +8506,48 @@ export class ObjectStackProtocolImplementation implements // Global Search (M10.5) // ========================================== /** - * Cross-object substring search across all registered objects that opt in - * via `enable.searchable !== false` and `enable.apiEnabled !== false`. - * Searches text-like fields (text/textarea/email/url/phone/markdown/html/string) - * whose `searchable: true` flag is set, falling back to the object's - * `displayNameField` (or `name`) when no fields are explicitly searchable. - * - * The query is split into whitespace-separated terms; each term must match - * (case-insensitive LIKE) at least one searchable field. RBAC/RLS is - * enforced by forwarding the caller's `context` to `engine.find` so users - * only see records they are entitled to read. + * Cross-object search across all registered objects that opt in via + * `enable.searchable !== false` and `enable.apiEnabled !== false`. + * + * ## [#7643] WHICH columns are searched is not decided here + * + * The per-object query is handed `search: ` and the ENGINE resolves the + * fields and compiles the clause (`expandSearchOnAst` → + * `objectql/src/search-filter.ts` `expandSearchToFilter`, ADR-0061) — the + * same one expansion that serves the record picker and the list + * quick-search. This method used to resolve its own set (text-typed fields + * carrying the field-level `searchable: true` flag, falling back to the + * title field) and build its own AND-of-OR, which made the ⌘K palette's + * recall a STRICT SUBSET of the executor's on two axes at once: + * + * - it never ORed the hidden `__search` companion column, so full-pinyin + * (`huaningkeji`) and initials (`hnkj`) matched the seeded CJK row + * through `POST /data/:object/query {search}` and returned NOTHING from + * `GET /api/v1/search` — #2486 recall, absent on this door only; and + * - it read the field-level `searchable` flag, which `$search` has never + * read, instead of the object's `searchableFields` — whose own declared + * contract already names global search as one of its three consumers + * ("the record picker, list quick-search and global search", + * `object.zod.ts`). So the palette was a SECOND, narrower definition of + * a recall rule the spec says is one rule. + * + * Only the CROSS-OBJECT half is this method's own: which objects are swept, + * the per-object cap, hit ranking and title/snippet rendering. Everything + * inside one object's query now has a single producer. + * + * ## Matching + * + * Whitespace-separated terms are AND-ed, fields OR-ed, and matching folds + * case via `$icontains`. All three are the engine expansion's decisions and + * this paragraph only DESCRIBES them — it is never a second declaration. + * [#7850] It previously read "case-insensitive LIKE": `LIKE` names no + * operator in this vocabulary (and since #6518's LIKE→GLOB change, not even + * the SQL the compilers emit), while the sentence one screen down named + * `$contains`, which #4706 Q2 = A defines as case-SENSITIVE. Both halves of + * that contradiction are gone; `$icontains` is the operator that folds. + * + * RBAC/RLS is enforced by forwarding the caller's `context` to + * `engine.find` so users only see records they are entitled to read. */ async searchAll(request: { q: string; @@ -8547,7 +8579,12 @@ export class ObjectStackProtocolImplementation implements ? new Set(request.objects) : null; - // Tokenise: each token must match (LIKE %term%) at least one searchable field + // [#7643] SNIPPET tokens only. The engine does its own tokenisation + // inside the `$search` expansion (same split, same AND-of-terms rule), + // so these no longer decide what MATCHES — they decide where the + // excerpt is cut. Kept as a separate local rather than fed to the + // engine: the query text is what the contract carries, and a second + // pre-tokenised channel is exactly the divergence this card closed. const terms = q.split(/\s+/).filter(Boolean).slice(0, 8); const allObjects = (this.engine as any).registry?.getAllObjects?.() ?? []; @@ -8580,7 +8617,6 @@ export class ObjectStackProtocolImplementation implements : (fieldsRaw && typeof fieldsRaw === 'object' ? Object.entries(fieldsRaw).map(([name, f]: [string, any]) => ({ name, ...(f || {}) })) : []); - const TEXT_TYPES = new Set(['text', 'textarea', 'string', 'email', 'url', 'phone', 'markdown', 'html']); const fieldByName = new Map(fields.map(f => [f.name, f])); const hasField = (n: string) => fieldByName.has(n); // Resolve title for a record using titleFormat → displayNameField → @@ -8613,37 +8649,44 @@ export class ObjectStackProtocolImplementation implements return String(row.id); }; - const titleFieldName = obj.displayNameField - || (hasField('name') ? 'name' : undefined) - || (hasField('title') ? 'title' : undefined) - || fields.find(f => TEXT_TYPES.has(f.type))?.name; - - let searchableFields = fields - .filter(f => f && TEXT_TYPES.has(f.type) && f.searchable === true) - .map(f => f.name as string); - - // Fallback: if no field is explicitly searchable, scan the title field - if (searchableFields.length === 0 && titleFieldName) { - searchableFields = [titleFieldName]; - } + // [#7643] The SAME resolution the engine's `$search` expansion will + // apply to this object one call down — `@objectstack/spec/data` owns + // it precisely so the layers that must agree read one function + // instead of each carrying a copy (#4254 moved it there for the + // ingress gate; this is the third face). It is consulted here for + // two things, and NEITHER of them is building the filter: + // + // 1. the SKIP below — an object with nothing to scan is passed + // over. This is load-bearing, not tidiness: `expandSearchToFilter` + // answers an empty field set with `null`, `expandSearchOnAst` + // then leaves `where` unset, and a `find` with no `where` returns + // the object's FIRST `perObject` rows — a global palette + // answering every query with unrelated records. The old code's + // equivalent `continue` guarded the same cliff one layer up. + // 2. the snippet source fields further down, so the excerpt is cut + // from a column the search actually scanned. + // + // `expandSearchToFilter` itself is deliberately NOT imported: it + // lives in `@objectstack/objectql`, which DEPENDS on this package, + // so importing it would close a package cycle. Handing the engine + // `search` instead of a pre-built `where` routes through it anyway — + // and leaves ONE producer of search clauses rather than two callers + // of one helper, which is the stronger form of the same fix. + const fieldMetaByName: Record = {}; + for (const f of fields) if (f?.name) fieldMetaByName[f.name] = f; + const { allowed: searchableFields } = resolveSearchFieldResolution({ + fields: fieldMetaByName, + searchableFields: obj.searchableFields, + // [ADR-0079] `nameField` is the canonical primary-title pointer; + // `displayNameField` is the deprecated alias (still honored). + // Same precedence as `expandSearchOnAst` and the #4254 gate — + // a third spelling here would re-split what this card merged. + displayField: obj.nameField ?? obj.displayNameField, + }); if (searchableFields.length === 0) continue; objectsScanned++; - // Build AND-of-OR filter: every term must hit at least one field. - // [#7641] Case-insensitive substring matching is `$icontains`, NOT - // `$contains` — the comment this replaced asserted the opposite and - // was the same declared≠enforced defect as `search-filter.ts`'s - // (`$contains` is contractually case-SENSITIVE, #4706 Q2 = A). The - // global-search palette is a SECOND producer of search clauses and - // was wrong the same way; `search.global-search`'s knownGaps already - // recorded it as this issue's to fix. Neither operator's semantics - // changed — only which one the palette compiles to. - const andClauses = terms.map(term => ({ - $or: searchableFields.map(f => ({ [f]: { $icontains: term } })), - })); - const where = andClauses.length === 1 ? andClauses[0] : { $and: andClauses }; - try { // `order`, NOT `direction` — see the audit-history query above. // Ascending here returned the STALEST `perObject` matches and @@ -8651,7 +8694,16 @@ export class ObjectStackProtocolImplementation implements // likely to want (#4674). Typed rather than `any` so the // contract rejects the wrong key at the call site. const opts: EngineQueryOptionsParsed = { - where, + // [#7643] The bare string IS the ADR-0061 Tier-1 contract + // ("the client sends only the query text; the server + // resolves which fields to search from object metadata"), + // and `search` is a declared `find` option + // (`EngineQueryOptionsSchema`, `ENGINE_FIND_OPTION_KEYS`) — + // so this is the engine's published door, not a private one. + // No `searchFields`: that key only ever NARROWS the resolved + // set (ADR-0061), and the palette wants the object's full + // default reach. + search: q, limit: perObject, orderBy: [{ field: 'updated_at', order: 'desc' }], }; @@ -8661,7 +8713,14 @@ export class ObjectStackProtocolImplementation implements for (const row of rows || []) { if (hits.length >= overallLimit) break; const title = renderTitle(row); - // Build snippet from first searchable field that contains a term + // Build snippet from first searchable field that contains a + // term. [#7643] `undefined` is a CORRECT answer here, not a + // miss: a companion (pinyin) hit matches the normalized + // `__search` blob, so no source column literally contains + // `hnkj` — the row is a real hit with nothing to excerpt. + // The companion is never a snippet source itself; it is + // stripped from rows on the way out (#7642) and its content + // is machine-normalized text no user typed. let snippet: string | undefined; for (const f of searchableFields) { const v = row[f]; diff --git a/packages/objectql/src/global-search-palette-recall.test.ts b/packages/objectql/src/global-search-palette-recall.test.ts new file mode 100644 index 0000000000..69e66e2651 --- /dev/null +++ b/packages/objectql/src/global-search-palette-recall.test.ts @@ -0,0 +1,415 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#7643] The ⌘K global-search palette and the `$search` executor have ONE + * definition of recall. + * + * ## The defect + * + * `GET /api/v1/search` is served by `MetadataProtocol.searchAll`, which used to + * resolve its own searchable set (text-typed fields carrying the field-level + * `searchable: true` flag, falling back to the title field) and compile its own + * AND-of-OR. `POST /api/v1/data/:object/query {search}` goes through the + * engine's ADR-0061 expansion (`search-filter.ts` `expandSearchToFilter`). + * Two producers, and the palette's was a STRICT SUBSET on two axes: + * + * - it never ORed the hidden `__search` companion, so `hnkj` / `huaningkeji` + * returned 0 hits from the palette while the executor returned 华宁科技 + * (the QA repro of #7629, verbatim); and + * - it read a flag `$search` has never read, instead of the object's + * `searchableFields` — whose own spec description already names global + * search as one of its three consumers. + * + * ## Why this suite lives in `objectql` and not next to `searchAll` + * + * The claim under test is a RELATION between two paths, so a double that + * satisfies one of them proves nothing. `metadata-protocol` cannot import the + * engine (`objectql` depends on IT — importing back would close a package + * cycle), so the only place both real implementations meet is here. Everything + * below runs a real `ObjectQL` over a real driver: the recall answers are + * executed, not asserted about. + * + * ## What is pinned, and in which direction + * + * The load-bearing assertion is PARITY, not "the palette finds pinyin" — a + * palette widened by narrowing the executor would satisfy the latter and is a + * failure. So each probe compares the two paths' hit sets against each other AND + * against an explicitly written expectation, and the per-object filter the two + * paths hand the driver is compared tree-to-tree. + * + * The #7641 operator pin (`$icontains`, never the case-SENSITIVE `$contains`, + * on source columns) MOVED here rather than disappearing: the palette no longer + * compiles an operator of its own, so the pin now sits on the filter it causes + * the engine to produce. `protocol.search-case-fold.test.ts` — which asserted + * that operator on a `where` the palette built itself — is rewritten to pin the + * delegation, and points here for the operator. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import type { ServiceObject } from '@objectstack/spec/data'; + +import { ObjectQL } from './engine.js'; +import { SEARCH_COMPANION_FIELD, provisionSearchCompanion } from './search-companion.js'; + +// --------------------------------------------------------------------------- +// A driver that really filters, and records the AST it was handed. +// --------------------------------------------------------------------------- + +type Row = Record; + +interface DriverAst { + where?: Record; + fields?: string[]; + limit?: number; + offset?: number; + orderBy?: Array<{ field: string; order?: string }>; +} + +interface Capture { + object: string; + where: Record | undefined; +} + +function makeStoreDriver(): { + driver: unknown; + seed(object: string, row: Row): void; + stored(object: string, id: string): Row | undefined; + captures: Capture[]; +} { + const rows = new Map>(); + const captures: Capture[] = []; + const tableFor = (o: string): Map => { + let t = rows.get(o); + if (!t) { t = new Map(); rows.set(o, t); } + return t; + }; + + const matches = (row: Row, where: Record | undefined): boolean => { + if (!where) return true; + for (const [k, v] of Object.entries(where)) { + // `$and` / `$or` are CONJOINED with their siblings, never `return`ed — + // a `return` would discard every sibling key the loop has not reached + // (#7620 / #8494), which on this suite would silently widen recall. + if (k === '$and') { + if (!(v as Array>).every((w) => matches(row, w))) return false; + continue; + } + if (k === '$or') { + if (!(v as Array>).some((w) => matches(row, w))) return false; + continue; + } + if (k.startsWith('$')) continue; + if (v !== null && typeof v === 'object') { + const cmp = v as Record; + // The two operators are executed with DIFFERENT case rules on purpose. + // `$contains` folding here would make the companion clause pass for a + // reason it does not have in production, and would hide a regression + // that swapped the source-column operator back to `$contains` + // (#4706 Q2 = A: `$contains` is case-SENSITIVE). + if ('$icontains' in cmp) { + const needle = String(cmp.$icontains).toLowerCase(); + if (!String(row[k] ?? '').toLowerCase().includes(needle)) return false; + continue; + } + if ('$contains' in cmp) { + if (!String(row[k] ?? '').includes(String(cmp.$contains))) return false; + continue; + } + if ('$in' in cmp) { + if (!(cmp.$in as unknown[]).some((x) => x === row[k])) return false; + continue; + } + if ('$eq' in cmp) { + if (row[k] !== cmp.$eq) return false; + continue; + } + continue; + } + if ((row[k] ?? null) !== (v ?? null)) return false; + } + return true; + }; + + const run = (object: string, ast: DriverAst | undefined): Row[] => { + captures.push({ object, where: ast?.where }); + let out = Array.from(tableFor(object).values()).filter((r) => matches(r, ast?.where)); + if (typeof ast?.offset === 'number' && ast.offset > 0) out = out.slice(ast.offset); + if (typeof ast?.limit === 'number' && ast.limit >= 0) out = out.slice(0, ast.limit); + return Array.isArray(ast?.fields) && ast.fields.length > 0 + ? out.map((r) => Object.fromEntries(ast.fields!.map((f) => [f, r[f]]))) + : out.map((r) => ({ ...r })); + }; + + let seq = 0; + const driver = { + name: 'store', version: '0.0.0', supports: {}, + async connect(): Promise {}, + async disconnect(): Promise {}, + async checkHealth(): Promise { return true; }, + async execute(): Promise { return null; }, + async find(object: string, ast?: DriverAst): Promise { return run(object, ast); }, + async findOne(object: string, ast?: DriverAst): Promise { return run(object, ast)[0] ?? null; }, + async create(object: string, data: Row): Promise { + seq += 1; + const id = (data.id as string | undefined) ?? `r_${seq}`; + const row: Row = { ...data, id }; + tableFor(object).set(id, row); + return { ...row }; + }, + async update(object: string, id: string, data: Row): Promise { + const table = tableFor(object); + const current = table.get(id); + if (!current) throw new Error(`not found: ${object}/${id}`); + const next: Row = { ...current, ...data, id }; + table.set(id, next); + return { ...next }; + }, + async delete(object: string, id: string): Promise { return tableFor(object).delete(id); }, + async count(object: string, ast?: DriverAst): Promise { return run(object, ast).length; }, + async bulkCreate(object: string, batch: Row[]): Promise { + const out: Row[] = []; + for (const r of batch) out.push(await driver.create(object, r)); + return out; + }, + async beginTransaction(): Promise<{ commit: () => Promise; rollback: () => Promise }> { + return { commit: async () => {}, rollback: async () => {} }; + }, + async commit(): Promise {}, + async rollback(): Promise {}, + }; + + return { + driver, + captures, + seed: (object, row) => { tableFor(object).set(String(row.id), { ...row }); }, + stored: (object, id) => tableFor(object).get(id), + }; +} + +// --------------------------------------------------------------------------- +// Objects — the showcase shapes the QA run measured. +// --------------------------------------------------------------------------- + +const ACCOUNT = 'showcase_account'; +/** An object with nothing scannable: neither path may answer from it. */ +const LEDGER = 'showcase_ledger_entry'; + +/** + * NOTE the absent field-level `searchable: true`. That flag is what the old + * `searchAll` keyed on, and `$search` has never read it — an object shaped like + * this is the ordinary case, not a contrived one (the showcase objects do not + * set it either). Under the old palette rule this object fell back to scanning + * the title field alone. + */ +const accountBase: ServiceObject = { + name: ACCOUNT, + label: 'Account', + fields: { + id: { type: 'text' }, + name: { type: 'text' }, + billing_email: { type: 'email' }, + annual_revenue: { type: 'number' }, + }, +}; + +const ledgerBase: ServiceObject = { + name: LEDGER, + label: 'Ledger Entry', + fields: { + id: { type: 'text' }, + amount: { type: 'number' }, + posted: { type: 'boolean' }, + }, +}; + +/** `OS_SEARCH_PINYIN_ENABLED=true` — the registry declares `__search`. */ +const accountProvisioned = provisionSearchCompanion(accountBase); + +/** + * The seeded CJK account, byte-for-byte the fixture the showcase seeds and the + * companion projection suite reuses: 华宁科技 = U+534E U+5B81 U+79D1 U+6280, + * with the normalizer's output for it. + */ +const CJK_NAME = '华宁科技'; +const CJK_BLOB = 'huaningkeji hnkj'; + +interface Harness { + engine: ObjectQL; + protocol: ObjectStackProtocolImplementation; + store: ReturnType; +} + +async function makeHarness(opts?: { companion?: boolean }): Promise { + const engine = new ObjectQL(); + const store = makeStoreDriver(); + engine.registerDriver(store.driver as never, true); + await engine.init(); + engine.registry.registerObject( + opts?.companion === false ? accountBase : accountProvisioned, 'test'); + engine.registry.registerObject(ledgerBase, 'test'); + + const protocol = new ObjectStackProtocolImplementation(engine as never); + + store.seed(ACCOUNT, { + id: 'acc_cjk', name: CJK_NAME, billing_email: 'billing@huaning.example', + annual_revenue: 36_000_000, updated_at: '2024-03-01T00:00:00.000Z', + ...(opts?.companion === false ? {} : { [SEARCH_COMPANION_FIELD]: CJK_BLOB }), + }); + store.seed(ACCOUNT, { + id: 'acc_nw', name: 'Northwind', billing_email: 'ap@northwind.example', + annual_revenue: 5_400_000, updated_at: '2024-02-01T00:00:00.000Z', + }); + store.seed(LEDGER, { id: 'led_1', amount: 42, posted: true, updated_at: '2024-01-01T00:00:00.000Z' }); + + return { engine, protocol, store }; +} + +/** Ids the `$search` EXECUTOR recalls — `POST /data/:object/query {search}`. */ +async function executorIds(h: Harness, q: string): Promise { + const rows = await h.engine.find(ACCOUNT, { search: q, limit: 25 }); + return rows.map((r: Row) => String(r.id)).sort(); +} + +/** Ids the ⌘K PALETTE recalls — `GET /api/v1/search`. */ +async function paletteIds(h: Harness, q: string): Promise { + const res = await h.protocol.searchAll({ q, objects: [ACCOUNT], perObject: 25, limit: 25 }); + return res.hits.map((hit) => String(hit.id)).sort(); +} + +describe('[#7643] palette recall === executor recall', () => { + let h: Harness; + beforeEach(async () => { h = await makeHarness(); }); + + it('the fixture is real: the stored CJK row carries the companion blob', () => { + // Guards every pinyin assertion below from passing vacuously — they are all + // "the companion was consulted", which is worthless if it holds no value. + expect(h.store.stored(ACCOUNT, 'acc_cjk')).toHaveProperty(SEARCH_COMPANION_FIELD, CJK_BLOB); + expect(h.store.stored(ACCOUNT, 'acc_cjk')).toHaveProperty('name', CJK_NAME); + }); + + // Each row: [probe, the ids BOTH paths must return]. The expectation is + // written out rather than derived from either path, so a change that breaks + // both paths identically still fails. + const PROBES: Array<[label: string, q: string, expected: string[]]> = [ + ['initials via the companion — the QA repro', 'hnkj', ['acc_cjk']], + ['full pinyin via the companion', 'huaningkeji', ['acc_cjk']], + ['a latin substring of a source column', 'northwind', ['acc_nw']], + ['the same term in the other casing (folding is the operator\'s job)', 'NORTHWIND', ['acc_nw']], + ['a non-title source column (email)', 'huaning.example', ['acc_cjk']], + ['the CJK original, typed directly', CJK_NAME, ['acc_cjk']], + ['a term nothing carries', 'zzzznope', []], + ]; + + it.each(PROBES)('%s', async (_label, q, expected) => { + const [palette, executor] = [await paletteIds(h, q), await executorIds(h, q)]; + // The relation first — this is the card's actual claim. + expect(palette).toEqual(executor); + // …then the absolute answer, so "both broke together" cannot pass. + expect(palette).toEqual(expected); + }); + + it('the two paths hand the driver the SAME filter tree, not merely the same rows', async () => { + // Row-level parity can be reached by two different filters that happen to + // agree on this fixture. Tree equality is the statement that there is one + // definition of recall, which is what the card asked for. + h.store.captures.length = 0; + await h.engine.find(ACCOUNT, { search: 'hnkj', limit: 25 }); + // Index arithmetic, not `.at(-1)`: this package's tsc program targets a + // `lib` without `Array.prototype.at`, and the TEST_DEBT ratchet re-measures + // that program — so `.at()` here is a +1 on a shrink-only ledger. + const executorWhere = h.store.captures[h.store.captures.length - 1]?.where; + + h.store.captures.length = 0; + await h.protocol.searchAll({ q: 'hnkj', objects: [ACCOUNT], perObject: 25, limit: 25 }); + const paletteWhere = h.store.captures.find((c) => c.object === ACCOUNT)?.where; + + expect(paletteWhere).toEqual(executorWhere); + expect(paletteWhere).toBeDefined(); + }); +}); + +describe('[#7643] the companion OR is what the palette gained', () => { + it('the palette filter really contains the companion clause', async () => { + // Positive assertion of the FIX, so the parity suite above cannot be + // satisfied by both paths losing companion recall together. + const h = await makeHarness(); + h.store.captures.length = 0; + await h.protocol.searchAll({ q: 'hnkj', objects: [ACCOUNT], perObject: 25, limit: 25 }); + const where = h.store.captures.find((c) => c.object === ACCOUNT)?.where; + + expect(JSON.stringify(where)).toContain(SEARCH_COMPANION_FIELD); + // The companion clause stays `$contains` DELIBERATELY: the column is a + // normalized blob, lowercase on both sides by construction, so a + // case-sensitive operator over two folded values is exact rather than a + // case bug (`search-filter.ts` spells this out). Pinned so an "align the + // operators" sweep has to read that reasoning first. + expect(where).toMatchObject({ $or: expect.arrayContaining([ + { [SEARCH_COMPANION_FIELD]: { $contains: 'hnkj' } }, + ]) }); + }); + + it('[#7641] source columns still compile to $icontains, never the case-SENSITIVE $contains', async () => { + // The operator pin, MOVED from `protocol.search-case-fold.test.ts`: the + // palette no longer compiles an operator itself, so the claim is now about + // the filter it causes the engine to produce. Asserted on a latin term so + // the companion clause (legitimately `$contains`) is not in the tree. + const h = await makeHarness(); + h.store.captures.length = 0; + await h.protocol.searchAll({ q: CJK_NAME, objects: [ACCOUNT], perObject: 25, limit: 25 }); + const where = h.store.captures.find((c) => c.object === ACCOUNT)?.where; + + expect(JSON.stringify(where)).toContain('$icontains'); + expect(JSON.stringify(where)).not.toContain('$contains"'); + expect(where).toMatchObject({ $or: expect.arrayContaining([ + { name: { $icontains: CJK_NAME } }, + ]) }); + }); + + it('with the companion NOT provisioned, both paths lose pinyin recall together', async () => { + // The capability is deployment-gated (`OS_SEARCH_PINYIN_ENABLED`). The + // contract is parity, not "pinyin always works" — so the flag-off state is + // pinned as parity too, and this is what stops the fix from being read as + // "the palette must special-case pinyin". + const h = await makeHarness({ companion: false }); + expect(await paletteIds(h, 'hnkj')).toEqual([]); + expect(await executorIds(h, 'hnkj')).toEqual([]); + // …while ordinary recall is untouched by the flag, on both paths. + expect(await paletteIds(h, 'northwind')).toEqual(['acc_nw']); + expect(await executorIds(h, 'northwind')).toEqual(['acc_nw']); + }); +}); + +describe('[#7643] an object with nothing scannable is SKIPPED, never dumped', () => { + it('contributes no hits, and is never queried at all', async () => { + // TWO different facts, and the second was measured rather than assumed — + // an ablation run predicted this test would stay green against the old + // code and it went RED, which is how the third divergence axis was found. + // + // 1. The cliff: `expandSearchToFilter` answers an empty field set with + // `null`, the engine then leaves `where` unset, and an unfiltered `find` + // returns the object's first rows — a palette answering every query with + // unrelated records. Hence the skip before the query is built. + // + // 2. `showcase_ledger_entry` HAS no scannable column — its only text-typed + // field is `id` — and the old palette queried it anyway. Its fallback + // chain ended in "the first text-typed field", which on such an object + // selects the PRIMARY KEY, so every keystroke ran + // `{id: {$icontains: term}}` against every system/junction/log table. + // That is #4483's defect exactly (`SEARCH_AUTO_EXCLUDED_FIELDS` names + // `id` for precisely this reason), which the executor had fixed and this + // path had not. Sharing the resolution retires it here too — so the + // `captures` assertion below is the pin on a behaviour that CHANGED, not + // on one that was preserved. + const h = await makeHarness(); + const res = await h.protocol.searchAll({ q: 'zzzznope', perObject: 25, limit: 25 }); + + expect(res.hits.map((hit) => hit.object)).not.toContain(LEDGER); + expect(res.totalHits).toBe(0); + // The ledger row exists and would have come back had the object been swept + // with no filter — the assertion above is not vacuous. + expect(h.store.stored(LEDGER, 'led_1')).toBeDefined(); + // And no query was ever issued against it. + expect(h.store.captures.some((c) => c.object === LEDGER)).toBe(false); + }); +});