fix(spec): EngineQueryOptionsSchema.search accepts the canonical bare query string (#7178) - #7249
Merged
Merged
Conversation
… query string (#7178) `BaseQuerySchema.search` (query.zod.ts, hence QueryAST, hence DriverQuery) has been `z.union([z.string(), FullTextSearchSchema])` since its own drift repair, with a doc comment saying why: the bare string IS the canonical Tier-1 contract (ADR-0061 D1), it is what every surface sends, and it is what the dogfood HTTP proof pins. `EngineQueryOptionsSchema.search` — the options type of IDataEngine.find/findOne — declared the structured form only. The runtime never agreed with that narrowing: `normalizeSearch` in objectql/src/search-filter.ts opens with `if (typeof raw === 'string')`, and protocol-data.test.ts asserts the protocol layer hands the engine a bare string. So the type forbade what the engine serves, and callers paid the standard price: `as any` on the query argument, which does not suppress `search` alone — it switches off checking for where/orderBy/fields in the same literal. This schema is not `.strict()`, so an unknown key there is silently dropped; the cast the divergence forced was precisely the cast check:query-options-erasure exists to stop. Same-family drift REPAIR, not a new dialect — the identical fix BaseQuerySchema.search already carries. On the query side it surfaced as a validation failure when #3899 started validating request bodies; here it surfaced as TS2345 when #6231 retyped DatabaseLoader's read helpers to DriverQuery and the engine branch alone refused to compile. Consumer census before landing (the card's own guard): every site reading object-form members off an engine-options `search` already narrows with `typeof` — engine.ts's `$search` expansion, search-filter.ts's normalizeSearch, and metadata-protocol/protocol.ts's searchFields ingress gate. No consumer needed a guard added and none changes behavior. `count` is untouched: EngineCountOptionsSchema declares no `search` key at all. With the schemas agreed, the casts the divergence forced are deleted — DatabaseLoader's three engine-branch `as any` (real where/orderBy/fields checking recovered on the metadata main read path) and the seven in engine-findone-contract.test.ts that were passing the canonical spelling. query-options-erasure-baseline.json ratcheted down accordingly (the loader file leaves the grandfather list entirely; test surface 256 -> 249). Closes #7178 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ymfZNzYCQpoptQ2sHbwU3
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 107 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 7 release-owned page(s) also reference the affected code. These are read-only:
|
github-actions
Bot
added
documentation
…'s canonical spelling (#7178) `content/docs/kernel/contracts/data-engine.mdx` hand-mirrors the `EngineQueryOptions` interface and declared `search?: FullTextSearch`. That was true of the schema until this branch widened it; leaving it makes the hand-written face say the opposite of the generated face (`references/data/data-engine.mdx`, regenerated in the previous commit) about the same key — the shape that costs AI authors the most. Surfaced by the PR's own docs-drift advisory. It is the ONLY hand-written page in the repo carrying this declaration (`grep 'search?: FullTextSearch'` over content/docs: one hit); the other pages the advisory lists reach @objectstack/spec by package-level fan-out and make no claim about this key. The query-side pages (`objectql/query-syntax`, `data-modeling/queries`) already document the union, since `BaseQuerySchema.search` has carried it since its own repair. Deliberately NOT fixed here: the same block omits `searchFields` entirely. That is #7170's defect (triage deduped it as distinct) and belongs to that card, not this one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ymfZNzYCQpoptQ2sHbwU3
os-zhuang
marked this pull request as ready for review
August 10, 2026 04:57
os-zhuang
pushed a commit
that referenced
this pull request
Aug 10, 2026
…eline on the merged tree Both #7249 and this PR ratcheted scripts/query-options-erasure-baseline.json; the merged tree carries both reductions, so the file is re-measured with --update rather than hand-merged (the resolution rule declared in the PR body). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011SaEx5461eovGX7AS9aLDV
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #7178.
The lane-ruled direction, executed: align
EngineQueryOptionsSchema.searchwithBaseQuerySchema.search— the same-family drift repair whose precedent and rationalequery.zod.tsalready records — then delete the casts the divergence forced.Premise re-measurement (fresh
origin/main@2e4274d)Everything triage verified at
5d24f4bstill held at2e4274d; nothing was repaired in between.packages/spec/src/data/data-engine.zod.ts:119search: FullTextSearchSchema.optional()packages/spec/src/data/query.zod.ts:502z.union([z.string(), FullTextSearchSchema]).optional()packages/metadata/src/loaders/database-loader.ts:228-258as any(_find/_findOne/_count) under aNOTE (#6231)block that names #7178packages/objectql/src/engine-findone-contract.test.tsscripts/query-options-erasure-baseline.jsondatabase-loader.ts: 3,testSurface.sites: 256Deviation from the dispatch, worth a look: the card and the dispatch both say five
as anyinengine-findone-contract.test.tspass the canonical spelling. The real count is seven — lines 144, 150, 163, 164, 169, 179, 296 onorigin/main. All seven hadsearch: <string>as their only off-contract content and all seven compile uncast after the change; the erasure ratchet independently confirms the number (test surface fell exactly 256 → 249). I removed all seven rather than five. No other cast in that file was touched — the ones coveringwhere: [[...]]tuples,filter:, empty bags and theproof.callloop are genuinely off-contract input and stay.Also worth noting: the loader's fourth
as any(_delete,{ where: { id } } as any) is a different shape, unrelated tosearch, and is left alone — it is not one of the three the ratchet counts.Consumer census (the card's own guard, run before landing)
Swept every read of
searchoff a value typedEngineQueryOptions/EngineQueryOptionsParsed/EngineCountOptions/ any enginefind/findOne/countoptions parameter, looking for object-form member access (.query,.fields, destructuring, spread,Object.keys) without atypeofguard.objectql/src/engine.ts:5849-5860(expandSearchOnAst, the one$searchexpansion shared byfind/findOne)raw.fieldstypeof raw === 'object' ? raw?.fields : undefinedobjectql/src/search-filter.ts:59-69(normalizeSearch)o.query,o.fieldsstring/objectdiscrimination; param isunknownobjectql/src/search-filter.ts:102-109(expandSearchToFilter)normalizeSearchmetadata-protocol/src/protocol.ts:5882-5886(#4254searchFieldsingress gate)search.fieldstypeof … === 'object'else(no caller-named fields to validate)metadata-protocol/src/protocol.ts:6063options.search == nullobjectql/src/engine.ts:5865-5866delete (ast as any).searchEngineCountOptionsSchema/ENGINE_COUNT_OPTION_KEYScountdeclares nosearchkey at allResult: zero unguarded object-form consumers, and zero sites in category (b) "behavior would silently change." Every consumer was already written for the union, which is the whole shape of this defect. No guard needed adding, so the STOP condition did not trigger.
Cleared as different types, not engine options:
metadata-manager.ts(MetadataQuerySchema.search, a plainz.string()),client/src/query-builder.ts(a writer),rest-server.ts(raw HTTP query bag, alreadytypeof-guarded), plusURL.search/ view-config booleans. No driver readssearchat all — the key is deleted before the AST leaves the engine.Changes
packages/spec/src/data/data-engine.zod.tssearch: FullTextSearchSchema.optional()→z.union([z.string(), FullTextSearchSchema]).optional(), with a doc comment mirroring the repair rationalequery.zod.tsrecords for the query sidepackages/metadata/src/loaders/database-loader.tsas anydeleted (_find/_findOne/_count); the staleNOTE (#6231)block rewritten to record the close-out and warn against reinstating the castpackages/objectql/src/engine-findone-contract.test.tsas anydeleted — the canonical spelling now passes uncastpackages/spec/src/data/data-engine.test.tsscripts/query-options-erasure-baseline.jsondatabase-loader.tsleaves the grandfather list entirely (was 3, now clean);testSurface.sites256 → 249content/docs/references/data/data-engine.mdxsearchrow now readsstring | { query: string; … }content/docs/kernel/contracts/data-engine.mdxEngineQueryOptionsblock:search?: FullTextSearch→string | FullTextSearch.changeset/engine-query-options-search-union.md@objectstack/specminor,@objectstack/metadatapatchBecause the baseline JSON is the ESLint
ignoreslist (eslint.config.mjs:299reads it), dropping the loader key also un-mutes that file forpnpm lint— it now has to stay clean on its own.On the second commit (hand-written docs)
The PR's own docs-drift advisory listed 107 pages. 106 of them reach
@objectstack/specby package-level fan-out and make no claim about this key — but one was a genuine hit:content/docs/kernel/contracts/data-engine.mdx:113hand-mirrors theEngineQueryOptionsinterface and declaredsearch?: FullTextSearch. Left alone, the hand-written face would say the opposite of the regenerated face about the same key.grep 'search?: FullTextSearch'overcontent/docsconfirms it was the only such page; the query-side pages (objectql/query-syntax,data-modeling/queries) already document the union.Deliberately not fixed there: the same block omits
searchFieldsentirely. That is #7170's defect (triage deduped it as distinct) and belongs to that card.Reverse verification — direction predicted first
Predictions written before measuring, then measured by reverting only
data-engine.zod.tstoorigin/main(git checkout origin/main -- <file>) and running the new pins unchanged. Tests import fromsrc, so no rebuild confound.ZodError: expected object, received stringsearch+searchFieldsin both spellingsBaseQuerySchema.search(siblings agree)QuerySchemaaccepted both, engine options rejected the stringFullTextSearchformsearchthat is neither string norFullTextSearchStated explicitly, since it is the point of the exercise: the last two pins are non-discriminating — they are guard pins that hold the rest of the accept face still (the union widens by exactly one spelling and does not open the key to anything), not evidence for the change. The first three carry the proof, and all three were red on unmodified
origin/mainfor the predicted reason.Compile-side, the card's TS2345 is gone:
DriverQuery→EngineQueryOptionsParsednow assigns bare.tsc --noEmitoverpackages/metadatareports zero errors indatabase-loader.ts(the only hits under that path are pre-existing.test.tsimport-extension / implicit-any debt I did not touch), andpackages/objectqlsrc typechecks clean. Sinceobjectql/tsconfig.jsonexcludes**/*.test.ts, I typecheckedengine-findone-contract.test.tsexplicitly through a throwaway project file (deleted, not committed): no error on any de-casted line — proving the 7 casts were vestigial rather than merely unenforced.Gates
Spec was built before every dist-derived regen, per the stale-dist trap (#7122), and rebuilt once more after the reverse-verification revert/restore so nothing generated read a stale
dist.pnpm --filter @objectstack/spec buildturbo run build --filter=@objectstack/objectql... --filter=@objectstack/metadata...check:query-options-erasure2e4274d)pnpm --filter @objectstack/spec check:generatedcontent/docs/references/**) →gen:docs→ ✅ 11/11check:api-surfacecheck:export-originscheck:authorable-surfacecheck:docscheck:doc-authoringcheck:docs-audit-scopecheck:empty-changeset,check:changeset-gate-self-tests,check:adr-0087-registrationpnpm --filter @objectstack/spec testpnpm --filter @objectstack/objectql testpnpm --filter @objectstack/metadata testeslint --no-inline-configon all changed sourcesThe dual-snapshot pair (
api-surface/+export-origins/) both reported current against the real post-builddist, so neither needed regenerating — the type surface widened a property's type, which is not part of either snapshot's recorded shape. Heavy runs were serialized underflock /tmp/os-heavy-verify.lock; no contention, zero wait (this was the only heavy consumer on the box).Special-inspection items for the PM
countaccepts the value fine becauseEngineCountOptionsis az.inputtype. Measured cause is slightly different and worth recording correctly:EngineCountOptionsSchemadeclares onlywhere+context— it has nosearchkey at all — andENGINE_COUNT_OPTION_KEYS(engine.ts:310) rejects one outright. SocountacceptedDriverQueryvia ordinary non-fresh-object excess-property assignability, not viaz.inputvsz.infer. Post-change the asymmetry is simply gone from the caller's view: all three loader helpers pass uncast. The only place the old wording lived in code was the loader'sNOTE (#6231)block, which this PR rewrites.database-loader.tsis no longer inignores, so any futureany-erasure there failspnpm lintdirectly rather than needing the ratchet. Intended, but it is a real tightening beyond the literal ask.packages/spec/src/data/query.test.ts:1094/1106/1193/1205do unguardedresult.search?.operator/.fuzzy/.highlightonQuerySchema.parse(...), whosesearchhas beenstring | FullTextSearchParsedsince the query-side repair. Latent on the QueryAST side, untouched and unaffected by this PR. Say the word and I will file it as afinding; I did not, to avoid noise on a pin that is green.docs/adr/**andcontent/docs/releases/were not touched. The twocontent/changes are the generatedreferences/data/data-engine.mdxand the one genuinely-stale hand-written line inkernel/contracts/data-engine.mdx.No auto-merge — yours to land serially.