Repository navigation
⌘K global-search palette recall is strictly narrower than the $search executor (no pinyin/companion recall) #7643
Description
Symptom
The ⌘K command palette's recall is strictly narrower than the $search executor's. hnkj / huaningkeji return 0 hits from the global endpoint while POST /api/v1/data/showcase_account/query {search:…} returns the CJK row; the seeded CJK account is unreachable by huaning from the palette. Net user effect: pinyin recall works in the list quick-search but not in the ⌘K command palette.
The rest of the item passes: exactly one GET /api/v1/search (no per-object fanout), grouped hits (#3371 does not reproduce), Enter navigates, empty / 1-char input fire zero requests, and the RLS deny side is clean on both API and UI.
Root cause
packages/metadata-protocol/src/protocol.ts searchAll builds its own filter (TEXT_TYPES + f.searchable===true, falling back to the title field) instead of calling resolveSearchFields / expandSearchToFilter, and never ORs the SEARCH_COMPANION_FIELD — so full-pinyin / initials recall via the __search companion is absent from this path. Its comment claiming "case-insensitive substring matching via $contains" is also stale — the same $contains case bug tracked in #7641 (shared $contains/case-fold root cause).
Confirmed still present on origin/main (76d74ecb): searchAll (lines ~7177–7291) still constructs its own TEXT_TYPES + searchable===true filter with the stale $contains comment at line 7291, and references neither SEARCH_COMPANION_FIELD nor resolveSearchFields / expandSearchToFilter.
Reproduction
GET /api/v1/search?q=hnkj → 0 hits
GET /api/v1/search?q=huaningkeji → 0 hits
POST /api/v1/data/showcase_account/query {"search":"hnkj"} → returns the CJK row
Source
Extracted from the QA run #7629 (framework 92f26f7, console 6314e87f).
Activity
hotlong commentedon Aug 14, 2026
Claim: PM loop round 8
Session:
session_01XeQRiAa7vYRVX5Fog7Zby8Branch:
claude/issue-7643-searchall-companion-recallWorktree:
objectstack-issue-7643Domain:
domain:metadataFile surface:
packages/metadata-protocol/src/protocol.ts— region:searchAllonly, ~7177–7411 — plus its tests (stop on breach; explain in the report)Container & model:
M,mode:subagent,model: opusSerial constraints cleared: region-level declaration, because the file is contended. PR #8770 (#8421) is open and unlanded on this same file, but in disjoint regions:
canonicalizeMetaRequestTypeat:211, its six call sites, and thesaveMetaItemdoor. This card is confined tosearchAll. Both dispatches carry a merge-main-before-PR instruction and the queue arbitrates if I have judged the region separation wrong.📌 Folding in #7850's rows 2–3 — same function, same sentences
Measured before dispatching: #7850's rows 2 and 3 are
protocol.ts:7293and:7411, i.e. inside this card's region — and this card's own root-cause section already names one of them ("Its comment claiming 'case-insensitive substring matching via$contains' is also stale").Two agents in one 230-line span to fix two halves of one sentence would be a self-inflicted conflict. So the
searchAlldev corrects those two comments as part of this work and reports that it did.⛔ #7850 row 1 is NOT folded in —
packages/qa/dogfood/test/search-conformance.ledger.ts:23belongs to the spec seat and stays with #7850, which remains open for it. The correction to make is the one #4706 Q2 = A requires:$containsis case-SENSITIVE,$icontainsis the insensitive one, so prose naming$containsfor case-insensitive matching is self-contradictory.⛔ Also out of scope, from #7850:
packages/objectql/src/search-filter.ts:18("Matching: case-insensitive") is correct as a declaration — the executor is what must move, and that is #7641. Do not "fix" it toward current behaviour.Generated by Claude Code
hotlong commentedon Aug 15, 2026
{ "issue": 7643, "status": "done", "branch": "claude/issue-7643-searchall-companion-recall", "pr": "https://github.com/objectstack-ai/objectstack/pull/8787", "premise_still_valid": true, "summary": "Re-measured on current main: the defect holds, but every line number in the card is stale — searchAll is at protocol.ts:8520-8700, not ~7177-7411. searchAll now hands engine.find the query TEXT (search: q) per object instead of building its own filter, so the ADR-0061 expansion that serves the list quick-search is the single producer of search clauses; resolveSearchFieldResolution (from @objectstack/spec/data) is still consulted, but only for the skip guard and snippet fields. PM mechanism assumption HALF-FALSIFIED and reported in the PR: expandSearchToFilter cannot be imported into metadata-protocol because objectql DEPENDS on it (package cycle), so routing happens one level up at engine.find — which is the stronger shape anyway (one producer, not two callers of one helper). No per-object assumption blocked it: the cross-object sweep was already a loop of per-object engine.find calls. Body confirmed NOT sanitizer-truncated. #7850 fold-in: row 3 (:7411) was ALREADY corrected on main by #7641's landing, so this card's own root-cause paragraph naming it stale is itself now stale; row 2 (:7293, 'case-insensitive LIKE') was still present and is corrected. Row 1 and search-filter.ts:18 left alone as instructed.", "tests": "Union re-run AFTER the final commit, at HEAD=663d95e1b. New suite packages/objectql/src/global-search-palette-recall.test.ts (13 cases, all pass) — placed in objectql because that is the only package where both real implementations meet; asserts PARITY (palette hit-ids === executor hit-ids, plus a written expectation so 'both broke together' cannot pass) and compares the driver-level filter trees the two paths emit. Suites: metadata-protocol 90 files/1352 tests pass; objectql 207 files/3642 tests pass; downstream consumers via the PREFIX filter '...@objectstack/metadata-protocol' — rest 116 files/1921 tests pass, runtime 157 files/2391 tests pass. REVERSE VERIFICATION, direction predicted before running: revert protocol.ts, expect 5 red / 8 green (the $icontains pin predicted GREEN, since #7641 already landed that operator in the old code). Measured 6 red / 7 green — the extra red found a fourth divergence axis (below). The FIRST ablation attempt came back all-green and was INVALID: this suite resolves @objectstack/metadata-protocol through its built dist/, so the ablation only bites after a REBUILD. Both legs re-run with a rebuild and scripts/ablation-dist-preflight.mjs proving the mutation reached the artifact (--absent leaving, present returning); restore was byte-identical (git status clean vs the commit). Gates: derived via scripts/pm/dispatch-gates.mjs against the real changed files (three-dot origin/main...HEAD) — all 13 derived families PASS plus check:nul-bytes. check:type-check-debt --re-measure (the ratchet) initially caught a +1 IN MY OWN NEW TEST (Array.prototype.at is outside this package's tsc lib); FIXED rather than raised — ledger flat at 1968, none above recorded. origin/main merged immediately before the PR; no conflict with PR #8770's regions.", "open_questions": [], "out_of_scope_findings": [ "filed as #8786: searchAll's renderTitle reads only the deprecated displayNameField, never the canonical nameField, while provisionPrimary stamps only nameField — so an object whose primary title is not a conventional name titles its ⌘K hits with the raw record id. Titling, not recall; searched first, no duplicate.", "NOT filed, reported here: the old palette scanned PRIMARY KEYS. Its fallback chain ended in 'the first text-typed field', which on an object whose only text column is id selects id — every keystroke ran {id: {$icontains: term}} against system/junction/log tables. This is #4483's defect, which the executor had fixed and this path had not; it is REPAIRED by this PR as a consequence of sharing the resolution, and is called out in the changeset as a user-visible narrowing. No issue filed because it is fixed here, not left behind." ] }Generated by Claude Code
hotlong commentedon Aug 15, 2026
ACCEPT — PR #8787.
My mechanism assumption was not just wrong, it was unbuildable
I dispatched this with: "route
searchAllthrough the sameresolveSearchFields/expandSearchToFilterthe$searchexecutor uses", and invited falsification on whether per-object helpers could serve a cross-object sweep.The per-object worry was unfounded — "the sweep was already a loop of per-object
engine.findcalls, so the per-object expansion fits exactly where the old per-object filter sat." But the import half is impossible:So the routing went one level up instead:
searchAllhandsengine.findthe query text (search: q, a declaredfindoption) per object, and the engine expands it. That leaves one producer of search clauses rather than two that agree by maintenance — which is strictly better than the shared-helper shape I proposed, and it is the version that survives someone changing the expansion later.Also: my region declaration was stale. I claimed
searchAllat~:7177–7411; it is at:8520–8700. The dev re-derived from the tree rather than trusting my claim comment, which is what the rule says to do and why no conflict occurred.A fourth axis, found by the ablation
The reverse verification predicted 5 red / 8 green and measured 6 red / 7 green — divergence declared rather than smoothed. The extra red is how this was found:
That is #4483's defect, which the executor had already fixed and this path had not. So the PR carries a narrowing alongside the three widenings, and it exists because an ablation disagreed with a prediction. Those objects are now skipped.
The pin measures the right thing
That is the direction I asked to be protected, and it is protected by comparing both paths against each other and against a written expectation, with the per-object filter each hands the driver compared tree-to-tree. A test that only asserted "pinyin now works" would have passed on a change that broke the executor.
The first ablation came back all-green and was declared invalid — the
distresolution trap again — and both legs were re-run withscripts/ablation-dist-preflight.mjsproving the mutation reached the artifact. Fourth dev tonight to hit that trap; every one of them disclosed it rather than banking the false green.#7850, and a pleasing recursion
Row 3 ("case-insensitive substring matching via
$contains") was already corrected onmainby #7641's landing — so the card's own root-cause paragraph naming it as stale is itself now stale. Row 2 ("case-insensitive LIKE") was still there and is corrected:LIKEnames no operator in this vocabulary and, since #6518's LIKE→GLOB change, not even the SQL the compilers emit.Left alone as instructed: #7850 row 1 (spec seat's, card stays open for it) and
search-filter.ts:18, correct as a declaration under #4706 Q2 = A.Housekeeping
searchAll's title rendering reads only the deprecateddisplayNameField, so a canonically-designated primary title never titles a ⌘K hit #8786 filed, not fixed —renderTitlein this same function reads only the deprecateddisplayNameField, never the canonicalnameField, whileprovisionPrimarystamps only the latter. Titling, not recall. Correct call to leave it.check:type-check-debtcaught a +1 in the dev's own new test (Array.prototype.atoutside this package's tsclib) — fixed rather than raised. Ledger flat at 1968.metadata-protocol,objectql,rest,runtime. That is the sweep whose absence cost us the fix(metadata): refuse a /meta type name the platform does not have, instead of minting a namespace for it (#8421) #8770 regression tonight.Gates were starting at review time; I'll take job conclusions before marking ready, then auto-merge in that order.
Generated by Claude Code
hotlong commentedon Aug 15, 2026
Docs check — no change needed, and the reason is worth recording: the published docs already describe the behaviour this PR ships.
I checked this one properly rather than waving the advisory away, because unlike tonight's other PRs it genuinely changes what a published endpoint returns — and because the drift check demonstrably missed the documenting page on #8421 today.
Every
searchablehit in the docs is object-level, inside theenable: { … }capability block ("Include in global search index" / "Index records for global search"). That is the object sweep, which this PR leaves unchanged. No doc describes the field-levelsearchable: trueflag as selecting which fields global search matches — which is the thing this PR stops honouring.And
content/docs/data-modeling/schema-design.mdx:86-88states the field-selection rule outright:That is precisely what this PR implements — declared
searchableFields, else the auto-default. So the docs were already right andsearchAllwas wrong; this PR brings the code into line with the published contract rather than moving the contract.That is a useful corroboration of the PR's direction from a source the dev did not cite:
references/data/object.mdx:140likewise already named global search as one ofsearchableFields' three consumers, which the PR body notes it makes true.(Reverse control on the search: 97 doc files mention "search", so the absence of a field-level claim is a real absence, not a broken grep.)
Generated by Claude Code
fix(metadata-protocol): route searchAll through the engine's $search …
fix(metadata-protocol): title ⌘K hits from the canonical nameField (o…