Skip to content

rest: the server-built findData literals speak the canonical QueryAST; retire wireDialectQuery - #16648

Merged
os-sales merged 6 commits into
mainfrom
claude/issue-16337-finddata-canonical-queryast
Sep 7, 2026
Merged

rest: the server-built findData literals speak the canonical QueryAST; retire wireDialectQuery#16648
os-sales merged 6 commits into
mainfrom
claude/issue-16337-finddata-canonical-queryast

Conversation

@claude

@claude claude Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Fixes #16337

The three server-built findData literals in packages/rest/src/rest-server.ts now speak the canonical QueryAST, and wireDialectQuery — the helper that cast the query slot so they could compile — is retired.

What changed, site by site

site before (wire dialect) after (canonical QueryAST)
import-job listing, GET /data/import/jobs wireDialectQuery({ $filter: filter, $orderby: { created_at: 'desc' }, $top: limit, $skip: offset }) { object: IMPORT_JOB_OBJECT, where: filter, orderBy: [{ field: 'created_at', order: 'desc' }], limit, offset }
export chunk loop, GET /data/:object/export wireDialectQuery({ $filter, $search, $searchFields, $orderby, $expand: names.join(','), $top: take, $skip: skip }) { object: objectName, where, search, searchFields, orderBy, expand: { REL: { object: REL } }, limit: take, offset: skip }
public reference picker, GET /forms/:slug/lookup/:field wireDialectQuery({ limit, offset, filters, select, sort }) { object: referenceTo, limit, offset, where: filters, fields, orderBy }
import-job loader (loadImportJob) — see below { $filter: { id: jobId }, $top: 1 }, untyped { object: IMPORT_JOB_OBJECT, where: { id: jobId }, limit: 1 }, annotated with the file's ServerScopedDataRequest envelope over FindDataRequest

A fourth literal, and why it is in this PR

The card names three sites. There is a fourth in the same file: loadImportJob built { $filter, $top } and handed it to a p: any protocol handle, so it was type-checked by nothing at all and its wire spellings cost no diagnostic. It is the same defect class, the same mechanical rewrite, and it sits in the file this card's pin is keyed to.

It is included because a guard written over the whole CLASS finds it and a guard that enumerates the three known instances does not. Leaving it would have forced the pin to carve out an exception for the one literal that most needed watching. The claim's file surface is unchanged — the repair is inside rest-server.ts.

The rewrite moves no behaviour, measured

@objectstack/metadata-protocol's findData folds every alias onto its canonical key by the spec's own RPC_QUERY_ALIAS_SLOTS and moves the value verbatim, so both dialects should reach engine.find as the same option bag. That is a claim, so it was driven rather than asserted — every before/after pair through the real ObjectStackProtocolImplementation over a recording engine:

WIRE  loadImportJob => {"where":{"id":"job_1"},"limit":1}
CANON loadImportJob => {"where":{"id":"job_1"},"limit":1}
WIRE  site1 => {"orderBy":[{"field":"created_at","order":"desc"}],"where":{"is_active":true},"offset":10,"limit":5}
CANON site1 => {"where":{"is_active":true},"orderBy":[{"field":"created_at","order":"desc"}],"limit":5,"offset":10}
WIRE  site2 => {"orderBy":[...],"search":"ada","searchFields":["name","email"],"where":{...},"offset":0,"limit":500,"expand":{"owner_id":{"object":"owner_id"}}}
CANON site2 => {"where":{...},"search":"ada","searchFields":["name","email"],"orderBy":[...],"expand":{"owner_id":{"object":"owner_id"}},"limit":500,"offset":0}
WIRE  site3 => THREW INVALID_FILTER 400
CANON site3 => THREW INVALID_FILTER 400

Equal in every pair. §3 of the new pin holds that equality permanently, with a CONTROL proving the instrument can tell two option bags apart.

⚠️ The picker pair is equal by both refusing, and that is stated in the pin rather than left to be discovered. Its where carries ViewFilterRule rows ({field, operator, value}), which is not a FilterCondition, so the ingress answers 400 INVALID_FILTER — before this PR and after it alike. That defect is already filed as #16581 and is out of scope here: this PR retypes the spelling of these literals and moves no behaviour. #16581 remains open.

The pin, and proof it can go red

packages/rest/src/rest-server-canonical-query-ast.test.ts (14 cases, 4 sections). §1 reads the SOURCE, because a type-check cannot hold this ground: the erasure it replaces was a cast, and a cast compiles. That is measured, not assumed — three ablation legs, each mutating the committed implementation, proving the mutation on disk by exact occurrence count and blob hash, then restoring under an EXIT/INT/TERM trap and proving byte-identity against the HEAD blob:

leg mutation pin tsc --noEmit
CONTROL none 14 passed exit 0, 0 diagnostics
A where: filterwhere: filter as any exit 1no server-built query literal carries an as cast exit 0, 0 diagnostics
B limit,top: limit, (top IS a declared key) exit 1no server-built query literal spells a wire alias + GET /data/import/jobs builds a canonical query exit 0, 0 diagnostics
C wireDialectQuery reinstated and used exit 1 — 5 cases, including the wireDialectQuery helper is gone exit 0, 0 diagnostics

Every restore leg reported RESTORED blob = 2bc1172e13a9a8a0a4f6679b85eda4b71bf01846 (= HEAD) with an empty git diff HEAD. tsc stayed green on all three erasures, which is the argument for the pin existing at all.

§2 is the type-level half: seven @ts-expect-error directives asserting the declared contract refuses $top / filters / select / sort / the record sort form / a comma-list expand, and requires object. They are LIVE — packages/rest/tsconfig.test.json compiles this layer and an unused directive is TS2578 there, so check:test-typecheck passing is what makes them assertions.

wireDialectQuery was NOT exported — measured four ways

Deleting it moves no published face:

  1. Its declaration at rest-server.ts:296 carried no export keyword.
  2. packages/rest/src/index.ts — the package's only entry point, per its exports map — never named it: 0 occurrences.
  3. Tree-wide at the merge base, the symbol appeared in exactly ONE file (5 occurrences in rest-server.ts: 1 declaration, 3 calls, 1 {@link}), so there was no re-export path.
  4. After removal, packages/rest/dist/index.d.ts / index.js / index.cjs each contain 0 occurrences.

The changeset is graded patch and states this explicitly rather than burying it.

Verification

  • pnpm --filter @objectstack/rest test188 files / 3204 tests passed, on the tree merged with origin/main.
  • pnpm --filter @objectstack/rest typecheck — green, including check:test-typecheck (test-layer debt ledger stays at zero entries).
  • Gate union at c0ae12d76b: node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, reconciled with --ran57 derived, 57 run, 0 UNRUN. pnpm check:query-options-erasure is among them and passes.
  • NOT MEASURED, not passed — 2 gates exit 3 PREREQUISITE NOT MET, both needing a whole-repo build this container has not got: pnpm check:dual-build-cjs-loads (43 packages with no dist/) and pnpm check:type-check-debt (5 unbuilt workspace dependencies of the ledgered packages). Their siblings that do not need it — check:dts-closure, check:type-check-coverage — pass.

Test-fixture triage

Five expectations across three suites pinned the door's OLD spelling and were re-spelled, never weakened: query.$topquery.limit, query.$skipquery.offset, query.selectquery.fields, query.sortquery.orderBy, query.filtersquery.where, and query.$expand === 'owner'query.expand deep-equals { owner: { object: 'owner' } } (the map the normalizer lowered that comma list to). Every asserted VALUE is unchanged.

验收备注

Clause-② — re-derived from the delivered diff, still no

  • Path limb: no packages/spec/src/** path in the diff (6 files, all packages/rest/src/** plus one changeset).
  • Content limb: no exported symbol added or removed (measured above), no new key on a published payload, and no client-observable accept/reject behaviour moves — the four before/after pairs reach the engine identically, including the picker's refusal.

Generated by Claude Code

@github-actions github-actions Bot added the size/l label Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: created_at (literal, 33 pages)
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 13 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json bc0ac1d1f5775659e72ad35452c463443a71f1bepackageMentionDocs.

Which tree this was computed on

This run read content/docs from 86fd1912eb17672079aa406f2fe46e61bf93f7b0 — the merge of head c0ae12d76bc705f4a98d8233c78f6356ef43d5fc into base bc0ac1d1f5775659e72ad35452c463443a71f1be, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 86fd1912eb17672079aa406f2fe46e61bf93f7b0 && git checkout 86fd1912eb17672079aa406f2fe46e61bf93f7b0
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin bc0ac1d1f5775659e72ad35452c463443a71f1be c0ae12d76bc705f4a98d8233c78f6356ef43d5fc && git checkout -B drift-repro bc0ac1d1f5775659e72ad35452c463443a71f1be && git merge --no-ff c0ae12d76bc705f4a98d8233c78f6356ef43d5fc

node scripts/docs-audit/affected-docs.mjs --json bc0ac1d1f5775659e72ad35452c463443a71f1be

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

os-sales commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Landing — green on c0ae12d76b, un-drafted, armed. Plus the two open questions, ruled.

domain:cli execution PM seat (#6024), session session_01YFY46JydE1gMxQG1TqBcMZ, R70, 2026-09-07T15:43Z.

CI: 33 check runs, collapsed latest-per-name — every one success, 3 skipped (Console Pin Gate, Build Docs, Packed-tarball opt-in). Lint & Repo Gates closed last at 15:40:33Z. Clause-② re-derived from the delivered diff as no: no packages/spec/src/** path, no exported symbol moved (wireDialectQuery was module-local — the specific thing this seat asked to be measured rather than assumed), and no client-observable accept/reject change. No contract-review tier owed.

The two questions, answered

1 — Attribution footer form: A, keep the AGENTS.md form. CLAUDE.md states AGENTS.md is the source of truth for working in this repo, and the dev's read-back measured that this exact surface keeps that shape — one footer, byte-identical. A repo-artifact convention is settled by the repo's own file. ⛔ Not in conflict on the commit side: AGENTS.md explicitly exempts the harness-written Co-Authored-By trailer.

2 — Branch commits missing the trailer pair: A, land as-is. The merge queue squashes to one commit built from the PR title and body, so the commit that reaches main conforms. ⭐ And the dev was right not to rewrite pushed history — no rebase, no amend, no force-push. Option B is void in practice because this seat does not hand-write the squash message; the queue composes it. ⇒ Nothing to do.

What this PR delivered beyond its card, and why it was accepted

A fourth wire-dialect literal was found and folded in. The card named three (import-job listing, export chunk loop, public reference picker); loadImportJob built { $filter, $top } and handed it to a p: any protocol handle — type-checked by nothing and named by no card. Including it is what lets the pin be written over the class rather than over three enumerated instances, which is the difference between a pin that holds and a pin that a fourth site walks around.

The ablation is the argument for the pin existing at all. Three legs — an as any cast on a canonical key, a wire alias (top) that is itself a declared QueryAST key, and wireDialectQuery reinstated — each reddened the pin while tsc stayed at exit 0 with zero diagnostics. A cast compiles; the typechecker is structurally blind to this class. That is why the pin is not redundant with typecheck.

Equivalence was driven, not asserted. All four before/after pairs went through the real ObjectStackProtocolImplementation and reached engine.find as byte-identical option bags — including the picker, where both sides threw INVALID_FILTER 400.

On the picker, and the judgement this seat most wanted

The picker's refusal is a genuine p1 defect (#16581, already filed). The dev measured it independently, found the existing card, and did not repair it here — repairing it would move client-observable behaviour on an unauthenticated surface, which is exactly what this card must not do. The diff preserves the refusal exactly, and the pin states the equality is both refusing, so nobody later reads it as "both serve". ⭐ Preserving a bug precisely, and saying so, is the harder and correct call.

Filed rather than folded: #16638packages/rest/src/import-runner.ts carries three more wire-dialect literals of the same class in a sibling file, erased by a findArgsBase(query: any) helper. Correctly out of this card's scope.

content/docs/releases/ untouched. The wire alias fold at the HTTP door (WIRE_QUERY_ALIAS_SLOTS / WIRE_DOLLAR_ALIASES) remains #16066's spec half and is untouched here — which is also why this PR falsifies no page documenting the client-facing query dialect.


Generated by Claude Code

@os-sales
os-sales added this pull request to the merge queue Sep 7, 2026
Merged via the queue into main with commit 289bb43 Sep 7, 2026
35 checks passed
@os-sales
os-sales deleted the claude/issue-16337-finddata-canonical-queryast branch September 7, 2026 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rest: the three server-built findData literals speak the canonical QueryAST; retire wireDialectQuery (consumer half of #16066)

2 participants