From 9b1b0562f215da20312e083696c21dccb73eacac Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 03:28:57 +0000 Subject: [PATCH] fix(search): $search compiles to $icontains so textual matching actually folds case `$search` was case-SENSITIVE on textual fields, contrary to three places that all declared the opposite: the `search.cross-field-object-search` checklist item title, `search-filter.ts`'s own docblock ("Matching: case-insensitive"), and the `search-conformance` ledger row. Searching "Retail" returned "Acme Retail"; searching "retail" returned nothing. Declared != enforced, and the declaration is what three places already chose. The cause was operator CHOICE, not operator behaviour. `fieldClausesForTerm` emitted `{field: {$contains: term}}`, and `$contains` is contractually case-SENSITIVE (#4706 Q2 = A) -- `$icontains` is the case-insensitive one. SQLite's `LIKE` folded ASCII incidentally and hid the mismatch until #6518's `LIKE`->`GLOB` change removed that accident. Nothing about either operator changed. `$contains` remains case-sensitive and `$icontains` remains its ASCII-folding twin; only which one `$search` compiles to moved, which is what keeps this orthogonal to however #4706 Q2 finally lands. Fixed in BOTH producers of search clauses. `search-filter.ts` was the one the card named; `searchAll` -- the global-search palette behind GET /api/v1/search -- turned out to be a second producer with the same defect, building its AND-of-OR from `$contains` under a comment asserting `$contains` was the case-insensitive operator. `search.console-global-search`'s knownGaps had already recorded that path as this issue's to fix. Deliberately unchanged: the select label->value path (`optionValuesMatching` folds in JS and emits an exact-value `$in`), and the `__search` companion clause, which stays `$contains` because the column is lowercase by construction and the term is lowercased before it is compared -- a case-sensitive operator over two already-folded values is exact, not a case bug. The select RAW-VALUE fallback DID move, since it compares against raw stored text like any textual clause. All 6.5 filter faces verified to implement `$icontains` by reading the implementation rather than counting greps -- including the two thin ones, where service-analytics' cube lowering maps `$icontains` to a cube op that both strategy renderers honour. driver-memory / driver-mongodb are frozen (#5499), untouched, and already answer it. Test placement follows the gap the card named: the dogfood pin stayed green through the whole defect because its only case assertion was a select LABEL, which passes on a case-sensitive build. It now carries the ['name']-narrowed lowercase-vs-capitalized assertion over the real HTTP API, so the pin and the checklist's textual clause finally cover the same mechanism. Reverse-verified by reverting each producer independently: 7 search-filter unit cases + the new dogfood assertion go red (the latter with `expected [] to include 'Acme Retail'`, the issue's exact symptom) while the 4 pre-existing dogfood cases stay green, and 3 searchAll cases go red on the palette side. The 16 downstream tests that pinned `$contains` are updated. Several were fake matchers that folded BOTH sides while keyed on `$contains` -- they implemented `$icontains` semantics under the case-sensitive operator's name, which is part of why no unit test on this path ever noticed. Fixes #7641 Co-authored-by: Claude --- .../search-case-insensitive-icontains.md | 40 +++++++ docs/qa/platform-checklist/areas/search.json | 26 +++-- .../src/protocol.search-case-fold.test.ts | 110 ++++++++++++++++++ packages/metadata-protocol/src/protocol.ts | 11 +- .../src/engine-author-state-query.test.ts | 8 +- .../src/engine-findone-contract.test.ts | 20 +++- packages/objectql/src/engine.test.ts | 6 +- .../src/query-expression-conformance.test.ts | 14 ++- .../objectql/src/search-companion.test.ts | 9 +- packages/objectql/src/search-filter.test.ts | 97 ++++++++++++--- packages/objectql/src/search-filter.ts | 35 +++++- .../dogfood/test/search-conformance.ledger.ts | 7 +- .../test/showcase-search.dogfood.test.ts | 30 +++++ 13 files changed, 362 insertions(+), 51 deletions(-) create mode 100644 .changeset/search-case-insensitive-icontains.md create mode 100644 packages/metadata-protocol/src/protocol.search-case-fold.test.ts diff --git a/.changeset/search-case-insensitive-icontains.md b/.changeset/search-case-insensitive-icontains.md new file mode 100644 index 0000000000..1bdb9cae6a --- /dev/null +++ b/.changeset/search-case-insensitive-icontains.md @@ -0,0 +1,40 @@ +--- +'@objectstack/objectql': patch +'@objectstack/metadata-protocol': patch +--- + +fix(search): `$search` compiles to `$icontains`, so textual matching is actually case-insensitive + +`$search` was case-SENSITIVE on textual fields, contrary to three places that +all declared the opposite: the `search.cross-field-object-search` checklist item +title, `search-filter.ts`'s own docblock (*"Matching: case-insensitive"*), and +the `search-conformance` ledger row. Searching `Retail` returned "Acme Retail"; +searching `retail` returned nothing. + +The cause was operator choice, not operator behaviour. `fieldClausesForTerm` +emitted `{field: {$contains: term}}`, and `$contains` is contractually +case-SENSITIVE (#4706 Q2 = A) — `$icontains` is the case-insensitive one. +SQLite's `LIKE` used to fold ASCII incidentally and hid the mismatch; #6518's +`LIKE`→`GLOB` change removed that accident and exposed it. + +**Nothing about either operator changed.** `$contains` remains case-sensitive +and `$icontains` remains the ASCII-folding twin; only which one `$search` +compiles to moved. Every filter backend already answers `$icontains` (#6520 / +#6682), so no driver changes were needed. + +Fixed in both producers of search clauses: + +- `objectql` `search-filter.ts` — per-object `find({ $search })`, for textual + fields and for the select raw-value fallback. +- `metadata-protocol` `searchAll` — the global-search palette behind + `GET /api/v1/search`, which built the same AND-of-OR from `$contains` under a + comment asserting `$contains` was the case-insensitive operator. + +Deliberately unchanged: the select label→value path (`optionValuesMatching` +folds in JS and emits an exact-value `$in`), and the `__search` companion +clause, which stays `$contains` because both of its sides are already lowercase. + +The three declarations are reconciled with the behaviour, and the dogfood pin +that stayed green through the whole defect — its only case assertion was a +select label, which passes on a case-sensitive build — now carries the +`['name']`-narrowed lowercase-vs-capitalized assertion that catches it. diff --git a/docs/qa/platform-checklist/areas/search.json b/docs/qa/platform-checklist/areas/search.json index 2eba58caaf..feeb05caf4 100644 --- a/docs/qa/platform-checklist/areas/search.json +++ b/docs/qa/platform-checklist/areas/search.json @@ -8,7 +8,7 @@ "title": "$search is a server-resolved cross-field match: terms AND-ed, fields OR-ed, case-insensitive, select labels mapped to values", "since": "v15", "status": "active", - "revision": 2, + "revision": 3, "priority": "P1", "surface": "api", "personas": ["seeded admin (admin@objectos.ai)"], @@ -18,8 +18,8 @@ "the seeded accounts: Northwind (industry 'retail', name does NOT contain 'retail'), Acme Retail (name-hit control), Contoso (cross-term control) — examples/app-showcase/src/data/seed/index.ts" ], "knownGaps": [ - "the textual case-fold clause FAILS on builds carrying #7641: search-filter.ts fieldClausesForTerm emits { field: { $contains: term } } for textual types and `$contains` is contractually case-SENSITIVE (#4706 Q2 = A), so a lowercase term misses a capitalized name (run #7629, framework 92f26f75, reproduced on two fresh boots; the #6518 SQLite LIKE→GLOB change removed the incidental ASCII fold that used to hide it). Case-insensitive stays the DECLARED truth — this item states the contract and #7641 moves the executor onto `$icontains`. A run records the FAIL against #7641; it does NOT re-declare the semantics to match a red build", - "the automated pin (packages/qa/dogfood/test/showcase-search.dogfood.test.ts) stayed green through #7641 because its only case assertion is a select LABEL, which survives via optionValuesMatching's JS-side lowercasing. A run may not treat that pin as covering the textual case-fold clause — that clause needs its own lowercase-name assertion" + "CLOSED by #7641 (was: the textual case-fold clause FAILED because search-filter.ts fieldClausesForTerm emitted { field: { $contains: term } } for textual types, and `$contains` is contractually case-SENSITIVE per #4706 Q2 = A, so a lowercase term missed a capitalized name — run #7629, framework 92f26f75, reproduced on two fresh boots; #6518's SQLite LIKE→GLOB change removed the incidental ASCII fold that had hidden it). fieldClausesForTerm now emits `$icontains` for textual types and for the select raw-value fallback, so the executor honours the declaration this item transcribes. Neither operator's own semantics moved — `$contains` remains case-SENSITIVE. A run that still sees the lowercase spelling miss is on a build predating #7641; score it FAIL against the declaration, not against this item", + "the automated pin (packages/qa/dogfood/test/showcase-search.dogfood.test.ts) used to stay green on a case-SENSITIVE build because its only case assertion was a select LABEL, which survives via optionValuesMatching's JS-side lowercasing. #7641 closed that blind spot: the pin now carries the ['name']-narrowed lowercase-vs-capitalized assertion this item's textual clause needs, so the pin and the clause finally cover the same mechanism" ] }, "steps": [ @@ -47,13 +47,13 @@ { "clause": "matching is case-insensitive on TEXTUAL fields: narrowed to ['name'], both 'retail' and 'Retail' return the name-hit control (Acme Retail) — the case-fold is a property of the matching itself, not a side effect of the select-label path", "oracle": "api", - "verify": "the two ['name']-narrowed responses both carry Acme Retail. This is the DECLARED contract: `$contains` is case-SENSITIVE (#4706 Q2 = A), so a conforming executor emits the case-insensitive `$icontains` for textual fields. On a build where search-filter.ts still emits `$contains`, the lowercase spelling returns [] — FAIL this clause against #7641 rather than softening the declaration", + "verify": "the two ['name']-narrowed responses both carry Acme Retail. This is the DECLARED contract: `$contains` is case-SENSITIVE (#4706 Q2 = A), so a conforming executor emits the case-insensitive `$icontains` for textual fields — which search-filter.ts fieldClausesForTerm does since #7641. On a build predating it the lowercase spelling returns [] — FAIL this clause against #7641 rather than softening the declaration", "evidence": "the two ['name']-narrowed response bodies" }, { "clause": "select labels map to option values: 'Retail' (label case) matches rows storing the value 'retail' — an independent mechanism from the textual case-fold above", "oracle": "api", - "verify": "the capitalized search still returns Northwind (optionValuesMatching lowercases both sides in JS before emitting $in; raw-value $contains fallback). Ticking this clause says nothing about the textual clause — they are verified separately on purpose", + "verify": "the capitalized search still returns Northwind (optionValuesMatching lowercases both sides in JS before emitting $in; raw-value $icontains fallback since #7641). Ticking this clause says nothing about the textual clause — they are verified separately on purpose", "evidence": "the response body" }, { @@ -65,8 +65,8 @@ ], "negative": [ "'retail contoso' returning Contoso (terms OR-ed instead of AND-ed) is a FAIL against the declared matching semantics", - "an empty result for 'retail' means the executor silently dropped $search — the exact pre-ADR-0061 no-op this surface replaced; FAIL, not thin data. Rule out the #7641 case-fold gap first: if 'Retail' hits where 'retail' misses, the executor did not drop $search, it failed to case-fold", - "a lowercase term missing a capitalized name ('retail' → [] while 'Retail' → Acme Retail) is a case-fold FAIL, not thin data and not a fixture problem — the declaration is the truth and the executor is what must move (#7641)" + "an empty result for 'retail' means the executor silently dropped $search — the exact pre-ADR-0061 no-op this surface replaced; FAIL, not thin data. Distinguish it from the case-fold gap #7641 closed: if 'Retail' hits where 'retail' misses, the executor did not drop $search, it failed to case-fold (a build predating #7641)", + "a lowercase term missing a capitalized name ('retail' → [] while 'Retail' → Acme Retail) is a case-fold FAIL, not thin data and not a fixture problem — the declaration is the truth and the executor is what must move; #7641 moved it onto `$icontains`, so on a current build this spelling must hit" ], "variants": [ "multi-term AND", @@ -78,13 +78,14 @@ "automated": { "kind": "api", "ref": "packages/qa/dogfood/test/showcase-search.dogfood.test.ts" }, "source": [ "packages/qa/dogfood/test/search-conformance.ledger.ts (rows search-executor, search-select-label-mapping — the variants list is the enforced behavior set)", - "packages/objectql/src/search-filter.ts (docblock line 18 — matching semantics: terms AND-ed, fields OR-ed, case-insensitive, label mapping; the DECLARATION this item transcribes, which fieldClausesForTerm does not yet honor for textual types — #7641)", + "packages/objectql/src/search-filter.ts (docblock 'Matching:' paragraph — matching semantics: terms AND-ed, fields OR-ed, case-insensitive, label mapping; the DECLARATION this item transcribes, honored for textual types by fieldClausesForTerm since #7641 moved it onto `$icontains`)", "packages/objectql/src/engine.ts expandSearchOnAst (the executor site the ledger names)", "#4706 Q2 = A (the operator ruling that fixes the direction: `$contains` is a case-SENSITIVE substring test and `$icontains` is the case-insensitive one — so 'case-insensitive $search' means $icontains, and the declaration is what the executor must satisfy)" ], "history": [ { "revision": 1, "date": "2026-08-07", "change": "new item transcribed from the search-conformance ledger and its HTTP-level dogfood proof, seeded names (Northwind/Acme Retail/Contoso) verified in the showcase seed", "ref": "claude/platform-test-checklist-ocwugl" }, - { "revision": 2, "date": "2026-08-11", "change": "case-fold made separately assertable, and aligned TOWARD the declaration per the #4706 Q2 = A ruling (case-insensitive is the declared truth; the product-side conformance gap is #7641's job, not this file's). Run #7629 scored the old clause FAIL and showed why it had gone unnoticed for a release: one clause conflated two mechanisms — textual case-fold and select label→value mapping — and the label half passes on a case-SENSITIVE build because optionValuesMatching lowercases in JS, so a green tick (and a green dogfood pin) said nothing about the textual half. Now split into two clauses with a ['name']-narrowed probe that pins the textual case-fold away from the select path, a step and a negative for the lowercase-name assertion the run identified as the missing one, and knownGaps recording the #7641 expected FAIL plus the pin's blind spot. The variant string 'case-insensitive $contains' was self-contradictory under Q2 = A and now names $icontains semantics. Title, docblock citation and the case-insensitive declaration itself are deliberately UNCHANGED", "ref": "#7647" } + { "revision": 2, "date": "2026-08-11", "change": "case-fold made separately assertable, and aligned TOWARD the declaration per the #4706 Q2 = A ruling (case-insensitive is the declared truth; the product-side conformance gap is #7641's job, not this file's). Run #7629 scored the old clause FAIL and showed why it had gone unnoticed for a release: one clause conflated two mechanisms — textual case-fold and select label→value mapping — and the label half passes on a case-SENSITIVE build because optionValuesMatching lowercases in JS, so a green tick (and a green dogfood pin) said nothing about the textual half. Now split into two clauses with a ['name']-narrowed probe that pins the textual case-fold away from the select path, a step and a negative for the lowercase-name assertion the run identified as the missing one, and knownGaps recording the #7641 expected FAIL plus the pin's blind spot. The variant string 'case-insensitive $contains' was self-contradictory under Q2 = A and now names $icontains semantics. Title, docblock citation and the case-insensitive declaration itself are deliberately UNCHANGED", "ref": "#7647" }, + { "revision": 3, "date": "2026-08-12", "change": "the executor caught up with the declaration, so the item records a CLOSED gap instead of an open one. #7641 moved search-filter.ts fieldClausesForTerm from `$contains` to `$icontains` for textual types (and for the select raw-value fallback); the label→value `$in` path is untouched, and the `__search` companion clause deliberately stays `$contains` because both of its sides are already lowercase. No clause substance changed and the case-insensitive declaration is again deliberately UNCHANGED — this revision only flips the tense of knownGaps, the textual clause's verify note, the two negatives and the search-filter.ts source citation, which all read as if the gap were still open. The second knownGap is closed by the same PR: the dogfood pin now carries the ['name']-narrowed lowercase-vs-capitalized assertion it was missing, so the pin and the textual clause finally cover the same mechanism", "ref": "#7641" } ] }, { @@ -384,7 +385,7 @@ "title": "The console global search — ⌘K palette, header Search button, and /search page — drives ONE path (GET /api/v1/search): hits group under object headings, RLS hides invisible rows, Enter opens the record, empty input shows recents", "since": "v16", "status": "active", - "revision": 2, + "revision": 3, "priority": "P1", "surface": "mixed", "personas": [ @@ -401,7 +402,7 @@ "knownGaps": [ "the /_console bundle is vendored and may be stale — verify the palette/page behavior against current objectui app-shell or a fresh build (stale-console-bundle, RUNNER §2)", "when the search plugin is ABSENT, searchAll's GET /api/v1/search answers 404 and useRecordSearch degrades to the per-object find({ $search }) fanout — record WHICH path served from the network trace rather than assuming the global endpoint", - "the /search path inherits the same declared-vs-actual case gap as search.cross-field-object-search: matching is DECLARED case-insensitive (#4706 Q2 = A makes `$icontains` the case-insensitive operator) and searchAll's filter still emits `$contains`, so a query whose case differs from the stored value can miss on a build carrying #7641. Vary case deliberately, or query in the stored case, before scoring a miss as a palette/UI defect — the declaration stands and #7641 owns the executor side" + "CLOSED by #7641 (was: the /search path inherited the same declared-vs-actual case gap as search.cross-field-object-search — matching is DECLARED case-insensitive, #4706 Q2 = A makes `$icontains` the case-insensitive operator, and searchAll's filter emitted `$contains`, so a query whose case differed from the stored value could miss). searchAll is a SECOND producer of search clauses and #7641 fixed it alongside search-filter.ts: metadata-protocol/src/protocol.ts searchAll now emits `$icontains`, and the comment there that claimed `$contains` was the case-insensitive operator is gone. A case-varying miss on a current build is a real palette/UI defect, not this gap; on a build predating #7641 it is the executor" ] }, "steps": [ @@ -486,7 +487,8 @@ ], "history": [ { "revision": 1, "date": "2026-08-08", "change": "new item: console global-search UI (⌘K palette + header button ADR-0054 C1 + /search page) over GET /api/v1/search — object-grouped hits, RLS parity, Enter-navigates, empty-shows-recents; grounded in objectui app-shell + the framework search route ledger", "ref": "claude/platform-test-checklist-ocwugl" }, - { "revision": 2, "date": "2026-08-11", "change": "two text corrections from run #7629, no clause substance changed. (1) The RLS-parity fixture carried the same stale invoice count as search.rls-both-personas — the seed ships 12 rows INV-1001..INV-1012, not 8; INV-1003's owner (linus) is now named so the invisible-row premise is checkable in place. (2) knownGaps records that this path inherits the declared-case-insensitive contract and the #7641 executor gap, since searchAll also emits `$contains` — without it a run scores a case-driven miss as a palette defect. Direction is per the #4706 Q2 = A ruling: case-insensitive is the declared truth, so the checklist reconciles toward the declaration and #7641 owns the product side", "ref": "#7647" } + { "revision": 2, "date": "2026-08-11", "change": "two text corrections from run #7629, no clause substance changed. (1) The RLS-parity fixture carried the same stale invoice count as search.rls-both-personas — the seed ships 12 rows INV-1001..INV-1012, not 8; INV-1003's owner (linus) is now named so the invisible-row premise is checkable in place. (2) knownGaps records that this path inherits the declared-case-insensitive contract and the #7641 executor gap, since searchAll also emits `$contains` — without it a run scores a case-driven miss as a palette defect. Direction is per the #4706 Q2 = A ruling: case-insensitive is the declared truth, so the checklist reconciles toward the declaration and #7641 owns the product side", "ref": "#7647" }, + { "revision": 3, "date": "2026-08-12", "change": "the executor gap this item's third knownGap recorded is CLOSED, so the gap flips to past tense. #7641 found searchAll to be a SECOND producer of search clauses with the same declared≠enforced defect — metadata-protocol/src/protocol.ts built its AND-of-OR from `$contains` under a comment asserting `$contains` was the case-insensitive operator — and moved it onto `$icontains` in the same PR as search-filter.ts. No clause substance changed; a case-varying miss on a current build is now a real palette/UI defect rather than an expected executor FAIL, which is the scoring instruction this revision corrects", "ref": "#7641" } ] } ] diff --git a/packages/metadata-protocol/src/protocol.search-case-fold.test.ts b/packages/metadata-protocol/src/protocol.search-case-fold.test.ts new file mode 100644 index 0000000000..f6b6e69cef --- /dev/null +++ b/packages/metadata-protocol/src/protocol.search-case-fold.test.ts @@ -0,0 +1,110 @@ +// 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. +// +// 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. +// +// 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`). + +import { describe, it, expect, vi } from 'vitest'; +import { ObjectStackProtocolImplementation } from './protocol.js'; + +interface SearchRow { + id: string; + name: string; + updated_at: string; +} + +/** The shape `searchAll` builds: `{$or:[…]}`, or `{$and:[{$or:[…]}, …]}`. */ +type SearchFilter = Record; + +interface FindOptions { + where?: SearchFilter; + orderBy?: Array<{ field: string; order?: string }>; + limit?: number; +} + +const ROWS: SearchRow[] = [ + { id: 'c1', name: 'Acme Retail', updated_at: '2024-01-01T00:00:00.000Z' }, + { id: 'c2', name: 'Northwind', updated_at: '2024-02-01T00:00:00.000Z' }, +]; + +const CONTACT = { + name: 'contact', + fields: { name: { name: 'name', type: 'text', searchable: true } }, +}; + +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. + const find = vi.fn(async (_object: string, _opts: FindOptions = {}) => ROWS); + const engine = { + registry: { + getObject: (n: string) => (n === 'contact' ? CONTACT : undefined), + getAllObjects: () => [CONTACT], + }, + find, + }; + 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; +} + +describe('[#7641] searchAll compiles to the case-folding operator', () => { + it('emits $icontains — never the case-SENSITIVE $contains', 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'); + }); + + 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 }); + + expect(filterFrom(lower.find)).toEqual({ $or: [{ name: { $icontains: 'retail' } }] }); + expect(filterFrom(upper.find)).toEqual({ $or: [{ name: { $icontains: 'Retail' } }] }); + }); + + it('folds every term of a multi-term query, which stays AND-of-OR', async () => { + const { p, find } = makeProtocol(); + await p.searchAll({ q: 'acme retail', perObject: 5 }); + + // 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' } }] }, + ], + }); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 8a6bd78b2c..7389444a2c 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -7408,9 +7408,16 @@ export class ObjectStackProtocolImplementation implements objectsScanned++; // Build AND-of-OR filter: every term must hit at least one field. - // ObjectQL exposes case-insensitive substring matching via `$contains`. + // [#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]: { $contains: term } })), + $or: searchableFields.map(f => ({ [f]: { $icontains: term } })), })); const where = andClauses.length === 1 ? andClauses[0] : { $and: andClauses }; diff --git a/packages/objectql/src/engine-author-state-query.test.ts b/packages/objectql/src/engine-author-state-query.test.ts index a1bd513b41..884bffeea7 100644 --- a/packages/objectql/src/engine-author-state-query.test.ts +++ b/packages/objectql/src/engine-author-state-query.test.ts @@ -71,8 +71,12 @@ function makeRecordingDriver() { if (!(v as any).$in.map(String).includes(String(row[k]))) return false; continue; } - if (v && typeof v === 'object' && '$contains' in (v as any)) { - const needle = String((v as any).$contains).toLowerCase(); + // [#7641] `$icontains` — what `$search` compiles to. This arm folded + // both sides while keyed on `$contains`, so the double already + // answered case-insensitively under the case-SENSITIVE operator's + // name; only the key moved. + if (v && typeof v === 'object' && '$icontains' in (v as any)) { + const needle = String((v as any).$icontains).toLowerCase(); if (!String(row[k] ?? '').toLowerCase().includes(needle)) return false; continue; } diff --git a/packages/objectql/src/engine-findone-contract.test.ts b/packages/objectql/src/engine-findone-contract.test.ts index fc5a9e52ca..50b5a2a452 100644 --- a/packages/objectql/src/engine-findone-contract.test.ts +++ b/packages/objectql/src/engine-findone-contract.test.ts @@ -68,8 +68,14 @@ function makeRecordingDriver() { if (k === '$and') return (v as any[]).every((w) => matches(row, w)); if (k === '$or') return (v as any[]).some((w) => matches(row, w)); if (k.startsWith('$')) continue; - if (v && typeof v === 'object' && '$contains' in (v as any)) { - const needle = String((v as any).$contains).toLowerCase(); + // [#7641] `$icontains` — the operator `$search` compiles to. This + // arm folded BOTH sides while it was still keyed on `$contains`, + // i.e. it implemented `$icontains` semantics under the case- + // SENSITIVE operator's name. That is part of why no unit test on + // this path noticed the compiler was emitting the wrong operator: + // the double answered the way the fixed compiler asks for. + if (v && typeof v === 'object' && '$icontains' in (v as any)) { + const needle = String((v as any).$icontains).toLowerCase(); if (!String(row[k] ?? '').toLowerCase().includes(needle)) return false; continue; } @@ -146,11 +152,14 @@ describe('findOne executes what it declares and refuses an empty predicate (#441 expect(row?.id).not.toBe(one.id); }); - it('the search term reaches the driver as a $contains predicate — `search` never does', async () => { + it('the search term reaches the driver as an $icontains predicate — `search` never does', async () => { await engine.findOne('crm_account', { search: 'Two' }); const { ast } = lastRead(); expect(ast.where).toBeTruthy(); - expect(JSON.stringify(ast.where)).toContain('$contains'); + // [#7641] `$icontains`, not `$contains`: the latter is contractually + // case-SENSITIVE (#4706 Q2 = A), so `$search` compiling to it made + // matching case-sensitive against three declarations saying otherwise. + expect(JSON.stringify(ast.where)).toContain('$icontains'); expect(ast.search).toBeUndefined(); expect(ast.searchFields).toBeUndefined(); }); @@ -332,7 +341,8 @@ describe('findOne executes what it declares and refuses an empty predicate (#441 call: { search: 'Two' }, expect: ({ ast }) => { expect(ast.search).toBeUndefined(); - expect(JSON.stringify(ast.where)).toContain('$contains'); + // [#7641] the case-folding operator, not the case-sensitive one. + expect(JSON.stringify(ast.where)).toContain('$icontains'); }, }, searchFields: { diff --git a/packages/objectql/src/engine.test.ts b/packages/objectql/src/engine.test.ts index 1ac2b30981..d3adcbf6f2 100644 --- a/packages/objectql/src/engine.test.ts +++ b/packages/objectql/src/engine.test.ts @@ -246,8 +246,10 @@ describe('ObjectQL Engine', () => { await engine.find('account', { search: 'retail' }); const ast = (mockDriver.find as any).mock.calls.at(-1)[1]; + // [#7641] `$icontains` on the textual field — the case-folding + // operator — while the select label→value path stays an exact `$in`. expect(ast.where).toEqual({ $or: [ - { name: { $contains: 'retail' } }, + { name: { $icontains: 'retail' } }, { industry: { $in: ['retail'] } }, ] }); expect(ast.search).toBeUndefined(); @@ -264,7 +266,7 @@ describe('ObjectQL Engine', () => { const ast = (mockDriver.find as any).mock.calls.at(-1)[1]; expect(ast.where.$and).toContainEqual({ status: 'active' }); - expect(ast.where.$and).toContainEqual({ $or: [{ name: { $contains: 'acme' } }] }); + expect(ast.where.$and).toContainEqual({ $or: [{ name: { $icontains: 'acme' } }] }); }); }); diff --git a/packages/objectql/src/query-expression-conformance.test.ts b/packages/objectql/src/query-expression-conformance.test.ts index 127293f932..08c0785296 100644 --- a/packages/objectql/src/query-expression-conformance.test.ts +++ b/packages/objectql/src/query-expression-conformance.test.ts @@ -125,8 +125,11 @@ function makeStubDriver() { if (!v.every((arm) => matchesWhere(row, arm))) return false; continue; } - // [#4254] `$or` + `$contains` are the shape the engine expands - // `search` into (an `$or` of case-insensitive `$contains`, ADR-0061). + // [#4254] `$or` + `$icontains` are the shape the engine expands + // `search` into (an `$or` of case-insensitive `$icontains`, + // ADR-0061). [#7641] It read "`$contains`" until the compiler moved + // onto the operator that actually folds — `$contains` is + // contractually case-SENSITIVE (#4706 Q2 = A). // Without them the driver would MATCH EVERY ROW for any search, and // the "searchFields really narrows the row set" controls below would // hold vacuously against a search that never filtered anything. @@ -139,9 +142,12 @@ function makeStubDriver() { if (!(v as any).$in.map(String).includes(String(row[k]))) return false; continue; } - if (v && typeof v === 'object' && '$contains' in (v as any)) { + // [#7641] Keyed on `$icontains` since the compiler emits it. The + // body is unchanged — it already folded both sides, which is + // `$icontains`' semantics wearing `$contains`' name. + if (v && typeof v === 'object' && '$icontains' in (v as any)) { const haystack = String(row[k] ?? '').toLowerCase(); - if (!haystack.includes(String((v as any).$contains).toLowerCase())) return false; + if (!haystack.includes(String((v as any).$icontains).toLowerCase())) return false; continue; } const expected = (v && typeof v === 'object' && '$eq' in (v as any)) ? (v as any).$eq : v; diff --git a/packages/objectql/src/search-companion.test.ts b/packages/objectql/src/search-companion.test.ts index 5e2bd0cafa..2d9a90e04b 100644 --- a/packages/objectql/src/search-companion.test.ts +++ b/packages/objectql/src/search-companion.test.ts @@ -142,9 +142,14 @@ describe('expandSearchToFilter with companion column (query-time, additive)', () it('ORs the companion clause for latin terms (lowercased)', () => { const filter = expandSearchToFilter('ZhangWei', { fields }); + // [#7641] The two clauses use DIFFERENT operators, on purpose. The + // companion is a normalized blob — lowercase by construction on the column + // side and lowercased here on the term side — so case-SENSITIVE + // `$contains` over two already-folded values is exact. expect(filter.$or).toContainEqual({ [SEARCH_COMPANION_FIELD]: { $contains: 'zhangwei' } }); - // Source-field clauses are untouched alongside it. - expect(filter.$or).toContainEqual({ name: { $contains: 'ZhangWei' } }); + // The SOURCE field compares against raw stored text, so it needs the + // folding operator; it carries the term at the caller's own casing. + expect(filter.$or).toContainEqual({ name: { $icontains: 'ZhangWei' } }); }); it('skips the companion clause for CJK and letterless terms', () => { diff --git a/packages/objectql/src/search-filter.test.ts b/packages/objectql/src/search-filter.test.ts index ad0be63192..53b57ef5e3 100644 --- a/packages/objectql/src/search-filter.test.ts +++ b/packages/objectql/src/search-filter.test.ts @@ -1,7 +1,15 @@ import { describe, it, expect } from 'vitest'; import { expandSearchToFilter, resolveSearchFields, normalizeSearch } from './search-filter'; -const accountFields = { +/** + * The compiler's output shape, named so the assertions below are typed rather + * than reaching into an `any`. `expandSearchToFilter` is declared `any` at the + * source (a driver-agnostic filter tree), so this is the test's own reading of + * the contract it pins. + */ +type SearchFilter = Record; + +const accountFields: Record }> = { name: { type: 'text' }, industry: { type: 'select', options: [ { label: 'Technology', value: 'technology' }, @@ -80,39 +88,96 @@ describe('expandSearchToFilter', () => { expect(expandSearchToFilter('x', { fields: {} })).toBeNull(); }); - it('single term → $or of $contains across resolved fields', () => { - const f = expandSearchToFilter('acme', { fields: accountFields, searchableFields: ['name', 'website'] }); + it('single term → $or of $icontains across resolved fields', () => { + const f: SearchFilter | null = expandSearchToFilter('acme', { fields: accountFields, searchableFields: ['name', 'website'] }); expect(f).toEqual({ $or: [ - { name: { $contains: 'acme' } }, - { website: { $contains: 'acme' } }, + { name: { $icontains: 'acme' } }, + { website: { $icontains: 'acme' } }, ] }); }); it('maps a select label to stored option values ($in)', () => { - const f = expandSearchToFilter('retail', { fields: accountFields, searchableFields: ['name', 'industry'] }); + const f: SearchFilter | null = expandSearchToFilter('retail', { fields: accountFields, searchableFields: ['name', 'industry'] }); expect(f).toEqual({ $or: [ - { name: { $contains: 'retail' } }, + { name: { $icontains: 'retail' } }, { industry: { $in: ['retail'] } }, ] }); }); it('multi-term → AND across terms, OR across fields', () => { - const f = expandSearchToFilter('acme tech', { fields: accountFields, searchableFields: ['name', 'industry'] }); - expect(f.$and).toHaveLength(2); - // first term "acme": no industry label matches → falls back to $contains - expect(f.$and[0]).toEqual({ $or: [ - { name: { $contains: 'acme' } }, - { industry: { $contains: 'acme' } }, + const f: SearchFilter | null = expandSearchToFilter('acme tech', { fields: accountFields, searchableFields: ['name', 'industry'] }); + const and: SearchFilter[] = (f as { $and: SearchFilter[] }).$and; + expect(and).toHaveLength(2); + // first term "acme": no industry label matches → falls back to $icontains + expect(and[0]).toEqual({ $or: [ + { name: { $icontains: 'acme' } }, + { industry: { $icontains: 'acme' } }, ] }); // second term "tech": matches the "Technology" label → $in - expect(f.$and[1]).toEqual({ $or: [ - { name: { $contains: 'tech' } }, + expect(and[1]).toEqual({ $or: [ + { name: { $icontains: 'tech' } }, { industry: { $in: ['technology'] } }, ] }); }); it('case-insensitive label match', () => { - const f = expandSearchToFilter('ACTIVE', { fields: accountFields, searchableFields: ['status'] }); + const f: SearchFilter | null = expandSearchToFilter('ACTIVE', { fields: accountFields, searchableFields: ['status'] }); expect(f).toEqual({ $or: [{ status: { $in: ['active'] } }] }); }); + + /** + * [#7641] The case-fold clause, asserted on the OPERATOR the compiler picks + * rather than on a matched row — this module's output IS the contract, and + * every filter face downstream is separately conformance-checked against + * `$icontains` (`FILTER_TEXT_CASES`). + * + * `$contains` is contractually case-SENSITIVE (#4706 Q2 = A), so emitting it + * for a textual field made the docblock's "Matching: case-insensitive" a + * declaration nothing enforced. These cases fail on the pre-#7641 compiler in + * exactly the way the HTTP repro did: the term's own spelling reaches the + * driver under an operator that will not fold it. + */ + describe('[#7641] textual fields compile to the case-folding operator', () => { + it('emits $icontains — never the case-SENSITIVE $contains — for text fields', () => { + const f: SearchFilter | null = expandSearchToFilter('retail', { fields: accountFields, searchableFields: ['name'] }); + expect(f).toEqual({ $or: [{ name: { $icontains: 'retail' } }] }); + // Spelled as its own assertion so a regression reads as "went back to the + // case-sensitive operator", not as an opaque object-shape diff. + expect(JSON.stringify(f)).not.toContain('$contains'); + }); + + it('picks the operator by field TYPE, not by the term\'s own casing', () => { + // The pre-#7641 defect was invisible to a capitalized term, because the + // stored value happened to match it. Both spellings must compile + // identically — the fold is the operator's job, not the caller's. + const lower: SearchFilter | null = expandSearchToFilter('retail', { fields: accountFields, searchableFields: ['name'] }); + const upper: SearchFilter | null = expandSearchToFilter('Retail', { fields: accountFields, searchableFields: ['name'] }); + expect(lower).toEqual({ $or: [{ name: { $icontains: 'retail' } }] }); + expect(upper).toEqual({ $or: [{ name: { $icontains: 'Retail' } }] }); + }); + + it('folds the select RAW-VALUE fallback too, but leaves the label→value $in exact', () => { + // "zzz" matches no industry label, so the fallback clause is what runs. + const fallback: SearchFilter | null = expandSearchToFilter('zzz', { fields: accountFields, searchableFields: ['industry'] }); + expect(fallback).toEqual({ $or: [{ industry: { $icontains: 'zzz' } }] }); + // …while a term that DOES hit a label still compiles to an exact-value + // $in: that path folds in JS (`optionValuesMatching`) and #7641 did not + // touch it. + const mapped: SearchFilter | null = expandSearchToFilter('RETAIL', { fields: accountFields, searchableFields: ['industry'] }); + expect(mapped).toEqual({ $or: [{ industry: { $in: ['retail'] } }] }); + }); + + it('leaves the `__search` companion clause on $contains (both sides already lowercase)', () => { + const withCompanion: Record = { ...accountFields, __search: { type: 'text' } }; + const f: SearchFilter | null = expandSearchToFilter('Retail', { fields: withCompanion, searchableFields: ['name'] }); + // The companion is a normalized blob: the column is lowercase by + // construction and the term is lowercased here, so a case-SENSITIVE + // operator over two folded values is exact rather than a case bug. This + // is a different mechanism from the source-column clause above. + expect(f).toEqual({ $or: [ + { name: { $icontains: 'Retail' } }, + { __search: { $contains: 'retail' } }, + ] }); + }); + }); }); diff --git a/packages/objectql/src/search-filter.ts b/packages/objectql/src/search-filter.ts index a69064bb0b..948538dd5f 100644 --- a/packages/objectql/src/search-filter.ts +++ b/packages/objectql/src/search-filter.ts @@ -5,9 +5,9 @@ * * The picker / list / command-palette surfaces all send a `$search` string; * historically the data layer dropped it (a silent no-op). This module turns - * that string into a driver-agnostic `$or` of `$contains` predicates across the + * that string into a driver-agnostic `$or` of `$icontains` predicates across the * object's *server-resolved* searchable fields — every driver already executes - * `$or` + `$contains`, so no driver changes are needed. + * `$or` + `$icontains`, so no driver changes are needed. * * Field resolution (server-side, never client-trusted) lives in * `@objectstack/spec/data` (`search-fields.ts`) since #4254, because the REST @@ -18,7 +18,20 @@ * Matching: case-insensitive; multiple whitespace-separated terms are AND-ed * (every term must hit some field); fields are OR-ed. `select`/`status` columns * store a value but users type the label, so the term is mapped to option - * values whose label matches (with a raw-value `$contains` fallback). + * values whose label matches (with a raw-value `$icontains` fallback). + * + * [#7641] The case-insensitive operator is `$icontains`, NOT `$contains`. + * `$contains` is contractually case-SENSITIVE (#4706 Q2 = A), so the + * `$contains` this module emitted until #7641 made the sentence above a + * declaration the executor did not honour on textual fields: SQLite's `LIKE` + * used to fold ASCII incidentally and hid it, and #6518's `LIKE`→`GLOB` change + * removed that accident. Nothing about either OPERATOR changed here — only + * which one `$search` compiles to. Every filter face already answers + * `$icontains` (#6520 / #6682): both `driver-sql` compilers (and + * `driver-sqlite-wasm` / turso-local by inheritance), turso's independent + * `RemoteTransport`, service-analytics' read-scope and cube lowerings, + * `formula`'s RLS matcher, objectql's own HAVING evaluator, and the frozen + * `driver-memory` / `driver-mongodb` (#5499). * * Pinyin recall (#2486): when the object carries the hidden `__search` * companion column (provisioned by the SchemaRegistry when @@ -87,10 +100,14 @@ function optionValuesMatching(meta: SearchFieldMeta, term: string): unknown[] { function fieldClausesForTerm(field: string, term: string, meta: SearchFieldMeta): any[] { if (SEARCHABLE_ENUM_TYPES.has(meta?.type ?? '')) { const values = optionValuesMatching(meta, term); + // The label→value path is already case-insensitive in JS (see + // `optionValuesMatching`) and emits an exact-value `$in` — untouched by + // #7641. Only the raw-value FALLBACK below is an operator clause, and it + // folds for the same reason the textual clause does. if (values.length > 0) return [{ [field]: { $in: values } }]; - return [{ [field]: { $contains: term } }]; + return [{ [field]: { $icontains: term } }]; } - return [{ [field]: { $contains: term } }]; + return [{ [field]: { $icontains: term } }]; } /** @@ -115,6 +132,14 @@ export function expandSearchToFilter(raw: unknown, opts: ExpandSearchOptions): a // stores lowercase normalized forms, so the term is lowercased; CJK terms // skip the clause (they hit the source columns directly and can never match // the ASCII companion). + // + // [#7641] This clause deliberately stays `$contains`: the companion is a + // NORMALIZED blob that is already lowercase on BOTH sides (the column by + // construction, the term by `.toLowerCase()` below), so a case-SENSITIVE + // operator over two folded values is exact, not a case bug. This is a + // different mechanism from the source-column clauses in + // `fieldClausesForTerm`, which compare against raw stored text and therefore + // need `$icontains`. Do not "align" the two. const hasCompanion = !!opts.fields[SEARCH_COMPANION_FIELD]; const andClauses = terms.map((term) => { const clauses = searchFields.flatMap((f) => fieldClausesForTerm(f, term, opts.fields[f] || {})); diff --git a/packages/qa/dogfood/test/search-conformance.ledger.ts b/packages/qa/dogfood/test/search-conformance.ledger.ts index 39b0f26191..8ebc8cebfc 100644 --- a/packages/qa/dogfood/test/search-conformance.ledger.ts +++ b/packages/qa/dogfood/test/search-conformance.ledger.ts @@ -20,7 +20,12 @@ import type { ConformanceRow } from '@objectstack/verify'; export const SEARCH_SURFACE: ConformanceRow[] = [ { id: 'search-executor', - summary: '`$search` server-resolved cross-field executor (terms AND-ed, fields OR-ed, case-insensitive `$contains`)', + // [#7641] "case-insensitive `$contains`" was self-contradictory under + // #4706 Q2 = A, which rules `$contains` case-SENSITIVE. The executor now + // emits `$icontains` — the operator that actually folds — so the row names + // it. Neither operator's own semantics moved; only what `$search` compiles + // to did. + summary: '`$search` server-resolved cross-field executor (terms AND-ed, fields OR-ed, case-insensitive via `$icontains`)', surface: 'spec/api/query.zod.ts:$search (QueryParams `search`)', state: 'enforced', enforcement: 'objectql/src/engine.ts (find AST expansion) → objectql/src/search-filter.ts expandSearchToFilter', diff --git a/packages/qa/dogfood/test/showcase-search.dogfood.test.ts b/packages/qa/dogfood/test/showcase-search.dogfood.test.ts index 486a112c06..cce3cf5ac3 100644 --- a/packages/qa/dogfood/test/showcase-search.dogfood.test.ts +++ b/packages/qa/dogfood/test/showcase-search.dogfood.test.ts @@ -56,6 +56,36 @@ describe('showcase: $search over the HTTP API (ADR-0061 conformance proof)', () expect(records.map((r) => r.name)).not.toContain('Northwind'); }); + /** + * [#7641] The TEXTUAL case-fold, pinned away from the select path. + * + * This pin stayed green through the whole defect because its only case + * assertion was the select LABEL above ("Retail" → Northwind), which passes + * on a case-SENSITIVE build: `optionValuesMatching` lowercases both sides in + * JS before emitting `$in`, so the label half never touches the operator. + * The textual half does, and it was broken — `fieldClausesForTerm` emitted + * `$contains`, which #4706 Q2 = A rules case-SENSITIVE. + * + * Narrowing to `['name']` is what makes this assertion load-bearing: it keeps + * the label→value mapping out of the verdict, so the only thing that can + * satisfy it is the operator folding case. Asserted over the real HTTP API, + * so it covers the whole chain — compiler, engine, and the driver actually + * executing `$icontains` — not just the emitted filter tree. + */ + it('textual case-fold: "retail" and "Retail" both match the capitalized name "Acme Retail"', async () => { + // The seed stores the name CAPITALIZED, so a lowercase term can only hit + // via a case-insensitive operator. This is the exact HTTP repro from #7641. + const lower = await query({ search: 'retail', searchFields: ['name'] }); + expect(lower.map((r) => r.name)).toContain('Acme Retail'); + // The capitalized spelling matched even on the broken build; asserting both + // is what makes a regression read as "the fold went away" rather than "the + // fixture moved". + const upper = await query({ search: 'Retail', searchFields: ['name'] }); + expect(upper.map((r) => r.name)).toContain('Acme Retail'); + // Same rows either way — case is not allowed to change the result set. + expect(lower.map((r) => r.name).sort()).toEqual(upper.map((r) => r.name).sort()); + }); + it('terms AND: "retail northwind" still matches; "retail contoso" does not', async () => { const both = await query({ search: 'retail northwind' }); expect(both.map((r) => r.name)).toContain('Northwind');