Skip to content

Commit 2f122b6

Browse files
fix(objectql)!: having resolves placeholders through the where resolver, and the per-aggregation filter refusals name aggregations[i].filter (#20334) (#20368)
Fixes #20334 Clause-②: no (narrowing) ## What this does Two `where` behaviours the other filter positions of `engine.aggregate` lacked, per triage 5861202636's three execution notes, and a third change the seat's answer 5863946181 ordered as a patch round on this PR (open question 1 = B). 1. **`having` resolves `{placeholder}` tokens through the resolver `where` uses.** `ObjectQL.resolveWhereTokens` now takes the AST slot as a parameter (`'where'` by default, `'having'` from `aggregate`), and `aggregate` calls it once for `having`, after the per-aggregation filters resolve and before the middleware chain. ⛔ No second resolver: the call is the same method, through the same stage function (`resolveWhereFilterTokens` → `@objectstack/core`'s `resolveFilterTokens`) that `where` and the judge use. An unknown token is `FILTER_TOKEN_UNKNOWN` / 400 and a context token with no value is `FILTER_TOKEN_UNRESOLVED` / 400, in `where`'s words, before any driver read. A known token compares as the value it names. 2. **The per-aggregation `filter`'s refusals name their position.** `assertTemporalComparandsInterpretable` and `assertTextOperatorTargetsAreStringCapable` take an optional `path` (default `'where'`), and the per-aggregation loop passes `` `aggregations[${i}].filter` ``, the root its list-shape and comparand-type doors already pass. 3. **The `having` temporal refusal's remedy is `where`'s** (patch round). `assertHavingTemporalComparandsInterpretable` now ends its refusal in `REMEDY[hit.kind]`, the table the `where` and per-aggregation refusals read, and the parallel `HAVING_REMEDY` table and its docblock are deleted (net 1 line added, 16 removed). A string a `date` or `datetime` column cannot read now gets the remedy naming the relative-date placeholder (`"{30_days_ago}" / "{current_month_start}"`), which `having` resolves from item 1 on. `DATE_YEAR_REMEDY` is untouched, and the `time` kind's words are the same string as before (`HAVING_REMEDY.time` was `REMEDY.time`). `REMEDY`'s own docblock names no reader, so it does not misstate one and is not edited. Files: `packages/objectql/src/engine.ts` (`resolveWhereTokens`, `aggregate`), `temporal-comparand-door.ts` and `text-operator-declared-type-door.ts` (the `path` parameter; in the first, also the `having` remedy of item 3), two new pins (`objectql/src/engine-aggregate-positions.test.ts`, `rest/src/rest-aggregate-positions.test.ts`), one updated pin (`objectql/src/engine-aggregate-having-temporal-door.test.ts`: the unknown-token row replaced, the remedy case flipped), `.changeset/20334-aggregate-positions.md` (new: `minor`, BREAKING narrowing, ADR-0087 `not-required (no-migration-prescription)`), and a DELIBERATE CORRECTION to `.changeset/20263-having-temporal-comparand-door.md` (two sentences; each rewrite is listed under Deviations). Both door functions are package-internal: only `engine.ts` imports them, and neither `src/index.ts` nor `src/core.ts` re-exports them. ## How `having` reaches the one resolver (H2) - At base, `resolveWhereTokens(ast, execCtx)` read `ast.where` alone: `if (!ast || ast.where == null) return; ast.where = resolveWhereFilterTokens(ast.where, execCtx);`. The per-aggregation `filter` resolved through its own call to core's `resolveFilterTokens`, and `having` reached no resolver at all. - **The resolver needs no field type.** Core's walk replaces values and never reads keys (`for (const [k, v] of Object.entries(node)) out[k] = walk(v)`), and a token resolves from the request context alone (`now`, `timezone`, `userId`, `orgId`). So a `having` keyed by aggregate aliases resolves exactly as a `where` keyed by fields does. The one field-type-aware step, reading a resolved day by a column's storage rule, is `applyHaving`'s, as it already is for a literal day. - **Order against the temporal door.** On `where` the door runs inside `lowerWhereFilterArray`, before `resolveWhereTokens`, and steps around any `classifyFilterToken` hit (core's `isUninterpretableTemporalComparand`). `having`'s door (#20263) runs in the same position relative to the new call: every `having` door first, then resolution. So `{ last_placed: { $lt: 'not-a-date' }, first_opened: { $gte: '{not_a_token}' } }` is the temporal door's `INVALID_FILTER`, and an earlier door's refusal (`totl`, #20123) keeps its words. Both are pinned. As on `where`, a resolved value is not judged again by the door. - Both `applyHaving` doors (native `driver.aggregate()` and the rows fallback) read `ast.having`, so one call covers both paths, before `executeWithMiddleware` and any driver read. The caller's `having` object is not written back: the resolver is copy-on-write, and the AST is the engine's own object (pinned). ## The path (H3) - At base the per-aggregation loop called `assertTemporalComparandsInterpretable(object, 'aggregate', schema, aggFilter)` with no path, and the walk rooted at its default `'where'`. - The doors that already name their position there, quoted from the base: the list-shape door, "Received string ("2026-01-10") at aggregations[1].filter.placed_on.$in"; the comparand-type door, "Filter comparand at aggregations[1].filter.amount.$eq is a plain object". The walker's refusals name `` `aggregations[1].filter` ``, and the materializable door names no path at all. - **Partly falsified: a second door had the same defect.** The text-operator declared-type door (#15661) was also called with no path: `$contains` on a number field in `aggregations[1].filter` said `at where.amount.$contains` at base, on all three drivers and both doors. It is fixed here the same way, as a bounded in-place fix (see Deviations). Head: `at aggregations[1].filter.placed_on.$gt`, `at aggregations[1].filter.$or[1].placed_on.$lt`, `at aggregations[1].filter.amount.$contains`, and `at aggregations[2].filter.…` when the filter sits on the third aggregation. ## Measured: base `26daf0b036` and head, three drivers, both doors, both `having` paths InMemoryDriver, SqlDriver on SQLite and SqlDriver on PostgreSQL 16.13 (a private role and database on the system cluster, database timezone `Asia/Shanghai`, process `TZ=America/New_York`), through `engine.aggregate` and `POST /api/v1/data/:object/query` (JSON round-tripped), four groups c1–c4 (`max(placed_on)` 2026-01-10 / 03-01 / 01-15 / 02-01, `sum(amount)` 500 / 1200 / 50 / 20). 432 cells per tree; driver reads counted. The head runtime is the objectql source at `f78b1e0c9d`. The later commits of the first round change tests, changesets and one doc comment, and `engine.ts` is byte-identical (blob `1c4f6d0a41c2`) at `a1a42d4c4a` and at the patch round's head `8b950b8e`. The patch round changes one runtime string, the `having` remedy, measured in its own section below. The three drivers and both doors agree on every row below unless the row says otherwise. | position · input | base | head | `where` twin | |:--|:--|:--|:--| | `having` `{ last_placed: { $gt: '{current_year_start}' } }` | 200, no group | 200, c1–c4 (the literal `'2026-01-01'` twin: c1–c4) | 200, c1–c4 | | `having` `{ last_placed: { $gte: '{not_a_token}' } }` | 200, no group, 1 read | `FILTER_TOKEN_UNKNOWN` / 400, 0 reads | `FILTER_TOKEN_UNKNOWN` / 400 | | `having` `'{TODAY}'` (near miss), an unknown token on `count`, under `$and` or `$or` | 200 (no group; `$or` kept c2) | `FILTER_TOKEN_UNKNOWN` / 400, 0 reads | — | | `having` `{today}` `$lte` / `{30_days_ago}` `$gt` / `{current_month_start}` `$lt` | c1–c4 / none / c1–c4 (text order) | c1–c4 / none / c1–c4 (resolved; the same groups by this data) | resolved | | `having` `{7_months_ago}` `$gt`, `$between` `['{current_year_start}', '2026-02-01']`, `{current_year_start}` `$gte` on `min(opened_at)`, `$not` of a token | none, none, none, c1–c4 | c2; c1, c3, c4; c1–c4; none | — | | `having` `$or: [{ total: { $gt: 1000 } }, { last_placed: { $gt: '{current_year_start}' } }]` | c2 | c1–c4 | — | | `having` `{ customer_id: '{current_user_id}' }`, user c2 | none | c2 | c2 | | `having` the same with no user · `{current_org_id}` with no org (engine) · `{record_id}` | none | `FILTER_TOKEN_UNRESOLVED` / 400, 0 reads (REST with no user: 401 before and after) | `FILTER_TOKEN_UNRESOLVED` / 400 | | `having` `{current_org_id}`, user c2, no org, both doors | none | `FILTER_TOKEN_UNRESOLVED` / 400 | — | | `having` `{ total: { $gt: '{today}' } }` on `sum` | none | none, except PostgreSQL native: c1, c3 | — | | `aggregations[1].filter` `{ placed_on: { $gt: 'not-a-date' } }` | 400 `INVALID_FILTER` `at where.placed_on.$gt` | 400 `INVALID_FILTER` `at aggregations[1].filter.placed_on.$gt` | 400 `at where.placed_on.$gt` | | `aggregations[1].filter` the same under `$or`, and the number for 10000-01-01 | `at where.…` | `at aggregations[1].filter.…` | `at where.…` | | `aggregations[1].filter` `{ amount: { $contains: '5' } }`, `{ placed_on: { $startsWith: '2026' } }`, and under `$or` | `at where.amount.$contains` … | `at aggregations[1].filter.amount.$contains` … | `at where.amount.$contains` | The PostgreSQL native `sum` cell is not a new divergence. Its literal twin `{ total: { $gt: 'TODAY-AS-A-DAY' } }` keeps c1, c3 there on the base too (PostgreSQL's native aggregate returns `sum` as a string, #20307's out-of-scope finding 3, the region #20335 holds). The resolved token compares exactly as that literal does. ## Collateral (H4) - **`where`: 48 of 48 cells byte-identical** base → head (status, code, message, groups, reads): tokens resolved and refused, the temporal and text-operator refusals, `{current_user_id}` with and without a user. - **`having` without a token: 96 of 96 cells byte-identical** at `a1a42d4c4a`, including the temporal refusals in their words (the patch round then moves one thing among them, the `date` / `datetime` temporal refusal's remedy, measured in its section below), a nested `$or` refusal, `a{b}c` (braces inside a string, not a placeholder), a `{ $field }` reference and the literal twins. - **Per-aggregation `filter`, every other door: 36 of 36 byte-identical** (list-shape, comparand-type, `$median`, `$field`, the unknown-token refusal and a resolved token's counts). - **The per-aggregation temporal and text-operator refusals: code, status and reads unchanged**, and at the engine the message differs in the `at where.` → `at aggregations[1].filter.` root alone (18 of 18 cells). Over REST the message is the engine's under the existing 500-character bound (`truncateClientMessage`), on every cell of both trees. The two `'not-a-date'` refusals (489 and 496 characters at base) now cross the bound and lose the end of their remedy (`"{current_month_s…`). The year-class and text-operator refusals were already cut at base. The changeset says so. - **A known token now compares as its resolved value, not as text**: pinned as "the token keeps exactly the groups the value written out keeps" on both paths, for ten shapes (a comparand, `$in`, both `$between` endpoints, a bare day on `min(datetime)`, `$and` / `$or` / `$not`) and `{current_user_id}`. ## Declaration and who is reached (H5) - The claim's `Clause-②: no (narrowing)` holds. An unknown token and an unresolvable context token on `having` answered 200 and now answer 400, which narrows the accept set. A known token that compared as text now resolves, which is a changed answer rather than a narrowed accept set. `node scripts/check-changeset-no-major.mjs --base origin/main`: "✓ This diff introduces no `major` bump." `node scripts/check-adr-0087-registration.mjs --base origin/main`: "✓ … 1 declared-breaking changeset(s), each carrying an ADR-0087 disposition", `.changeset/20334-aggregate-positions.md [BREAKING+bang+clause-②-narrowing] not-required (no-migration-prescription)`. - **Shipped authors of a `{…}` string on `having`: none.** There are five `having` clauses in `content/docs` and `skills/`: `queries.mdx` twice, `query-syntax.mdx` and `skills/objectstack-query/rules/aggregation.md` twice. Each compares `order_count` / `total_spent` / `total` with a number. Outside the engine and the spec that declare it, no runtime code, example app or `apps/` file composes a `having`. The control grep, a `where` / `filter` carrying a token in `examples/`, hits 16. ## Patch round: the `having` remedy (seat answer 5863946181), measured Before is `c599f758` (the merge of `main` at `c577e66635` into `a1a42d4c4a`, `HAVING_REMEDY` still read). After is `8b950b8e`, whose `temporal-comparand-door.ts` is the `db334ac4` blob `3ee7abf0c722`. - **Where the words are built (H1): one site, as hypothesised.** It is `assertHavingTemporalComparandsInterpretable` in `packages/objectql/src/temporal-comparand-door.ts`, the only line carrying "The `having` was NOT applied.", and it read `HAVING_REMEDY[hit.kind]` once. It now reads `REMEDY[hit.kind]`, and `git grep HAVING_REMEDY` answers 0 at `8b950b8e`. The year-class branch still reads `DATE_YEAR_REMEDY`. - **The pin sweep (H2), partly falsified.** Terms swept across this repository (every tracked file) and `../objectui`: `HAVING_REMEDY`, "The `having` was NOT applied", "a 200 indistinguishable from a real answer", "keep no group or every group", the two old remedy strings, "names no placeholder", "literal forms only", "remedy names none", `assertHavingTemporalComparandsInterpretable`, and `{30_days_ago}` / `current_month_s` in test files. Only one pin read the old remedy: `engine-aggregate-having-temporal-door.test.ts`, whose remedy case asserted `not.toContain('{30_days_ago}')`. It is flipped (Deviations). `packages/rest/src/data-query-having-temporal-door.test.ts` asserts only the column clause of the REST `error` (`toContain(column)`), which sits before the remedy, and no remedy words, so it has nothing to flip. The other having message-equality pins (`engine-aggregate-positions.test.ts`, `rest-aggregate-positions.test.ts`, `engine-aggregate-having-comparand-shape.test.ts`) compare token or walker refusals, not this door's. No doc, skill or `objectui` text quotes the having remedy. Every rejection assertion for a genuinely illegal shape is unchanged. - **What moved, at the engine.** 16 `having` cells (object `ledger_having`, SqlDriver on SQLite, both `having` paths) plus the two `where` twins were each read at both commits. 26 of the 26 moved cell-paths differ from before in the remedy alone: substituting the new remedy for the old one in the before message gives the after message byte for byte. The 8 unmoved cell-paths are the `time` column, the year class and the `where` twins. Both paths give one message in every cell. The `having` remedy equals the `where` twin's remedy byte for byte, on a `date` and on a `datetime` column. - **Over REST, the 500-character bound (H3), confirmed.** The bound (`CLIENT_MESSAGE_MAX`, a message of 500 or more characters is cut to 499 plus `…`) is unchanged. A `date` column's fixed words plus its new remedy take 409 characters, which leaves 90 for the object name, the column, what it aggregates, the quoted comparand and the path. A `datetime` column's take 502, so every such refusal is cut. "Whole" below means the REST `error` equals the engine message byte for byte. | `having` cell (`date` unless stated) | engine characters, before → after | REST, before → after | |:--|:--|:--| | `'not-a-date'` `$lt` on `max(placed_on)` | 382 → 481 | whole → whole | | `'+010000-01-01T00:00:00.000Z'` `$gt` on `max(placed_on)` | 399 → 498 | whole → whole (the longest measured) | | an `$in` member, a `$between` endpoint, the implicit-equality slot, under `$or`, under `$not` | 385, 390, 378, 389, 387 → 99 more each | whole → whole | | a `placed_on` groupBy key, a `day` bucket of `opened_at` | 391, 375 → 490, 474 | whole → whole | | `'not-a-date'` `$lt` on `min(opened_at)` (`datetime`) | 480 → 576 | whole → cut: `…epoch milliseconds, or a relative-date pl…` | | `'not-a-date'` on an `opened_at` groupBy key (`datetime`) | 487 → 583 | whole → cut: `…milliseconds, or a relative-…` | | `'last_30_days'` on `max(placed_on)` / `min(opened_at)` | 385 / 483 → 484 / 579 | `VALIDATION_FAILED` / 400 from the query schema, before and after: REST never reaches the engine with a preset name | | control: `'not-a-date'` / `'noon'` on `max(slot)` (`time`) | 412 / 406, unchanged | whole | | control: the number for 10000-01-01 on `max(placed_on)` (year class) | 555, unchanged | cut, before and after | | control, `where`: `'not-a-date'` on `placed_on` / `opened_at` | 485 / 578, unchanged | whole / cut at `…or a relative-date …`, before and after | The changeset states this: every measured `date` refusal arrives whole, and a `datetime` refusal is cut inside the remedy, as `where`'s already was. - **Ablation of the flipped pin, at `8b950b8e`.** It ran through `scripts/ablation-replace.mjs` (WRAP) under one verify lock. The anchor `` The \`having\` was NOT applied. ${REMEDY[hit.kind]} `` was replaced by the old literal-only remedy table, inlined behind the marker `ABLATION-20334-LITERAL-ONLY`. The tool read the anchor x1 → x0, the replacement x0 → x1, and the blob `3ee7abf0c722` → `bfe6275fe997`. Result: `engine-aggregate-having-temporal-door.test.ts` had 1 failed / 50 passed, the failure being the flipped case: `AssertionError: expected 'Write a "YYYY-MM-DD" calendar day.' to contain '"{30_days_ago}"'`. The direction is red, as predicted. The restore was proven: blob == HEAD `3ee7abf0c722`, `git diff HEAD` empty, tree clean. The test imports `./engine.js`, the source, so the leg needed no build. The built `dist/` (from `db334ac4`, the same `objectql` source) carries 0 hits of the marker. The whole `objectql` suite ran green again after the restore (below). ## Tests and evidence, patch round (head `8b950b8e`, after merging `main` at `c577e66635`) `main` at `c577e66635` brought no change to `objectql`, `core` or the drivers. It changed `packages/rest/src` (the draft-read builder gate), `packages/core/src/qa` and `spec`. The merge is `c599f758`, a true merge commit with parents `a1a42d4c4a` and `c577e66635`, through `scripts/pm/os-regen-merge.sh`: no generated artefact was pending, and main's side was taken for every `os-regen` path. The rest closure was rebuilt from it (`turbo run build --filter='@objectstack/rest^...'`, 24 tasks, exit 0), and `objectql` was rebuilt after the fix (dist carries the new remedy strings and 0 of the old). - `pnpm --filter @objectstack/objectql exec vitest run --project local --project repo --maxWorkers=2`: 324 files, 5890 tests passed. - `pnpm --filter @objectstack/rest exec vitest run --project local --project repo --maxWorkers=2`: 213 files, 3845 passed, 2 skipped (main added two files). - `engine-aggregate-having-temporal-door.test.ts` alone: 51 passed. The flipped remedy case is one of them, and it goes red under the ablation above. - `pnpm --filter @objectstack/objectql run typecheck`: exit 0, with the test layer: "40 file(s) / 234 error(s) / 65 pinned signature(s) held", unchanged. `tsc -p tsconfig.test.json --listFiles` lists `engine-aggregate-having-temporal-door.test.ts` (1 hit). `pnpm --filter @objectstack/rest run typecheck`: exit 0. - ESLint, narrowed and proven, at `8b950b8e`: `eslint --no-inline-config --format json` over the 6 `.ts` files this PR changes against the merge base counts 6 files in the JSON, 0 errors and 0 warnings. The population is read from `eslint.config.mjs` (`packages/**/*.{ts,tsx,mts,cts}`). Invariance: the config "never enables type-aware linting (no `parserOptions.project`, no typed `@typescript-eslint` rules)" (its own note), so this diff cannot move a verdict on an untouched file. - The REST measurement above ran as a scratch test file placed in `packages/rest/src` for one run at each commit and deleted after it. It was never committed, and the tree was clean after each run. ## Tests and evidence, first round (head `a1a42d4c4a`, after merging `main` at `29720975b6`, which touched none of these packages) - `pnpm --filter @objectstack/objectql exec vitest run --project local --project repo --maxWorkers=2`: 324 files, 5890 tests passed. - `pnpm --filter @objectstack/rest exec vitest run --project local --project repo --maxWorkers=2`: 211 files, 3819 passed, 2 skipped. - `pnpm --filter @objectstack/driver-memory exec vitest run --maxWorkers=2`: 57 files, 1374 passed. - `driver-sql`'s nine aggregate test files (`src/*aggregat*`), with `OS_TEST_POSTGRES_URL` on the private PostgreSQL 16 database and `TZ=America/New_York`: 9 files, 174 passed, 4 skipped. The suite's own summary: `live postgres RAN`, `live mysql NOT RUN` (no MySQL here; CI's Temporal Conformance job runs it). - New pins: `engine-aggregate-positions.test.ts` 28 passed, `rest-aggregate-positions.test.ts` 6 passed. The updated `engine-aggregate-having-temporal-door.test.ts` passes 51 and `data-query-having-temporal-door.test.ts` passes 11. - `pnpm --filter @objectstack/objectql run typecheck` and `pnpm --filter @objectstack/rest run typecheck`: exit 0, test layers included (objectql debt held at 40 files / 234 errors, unchanged; rest 0). `tsc --listFiles` on each `tsconfig.test.json` lists the new and updated test files (objectql 2, rest 1). - ESLint, narrowed and proven: `eslint --no-inline-config --format json` over the 6 changed `.ts` files counts 6 files in the JSON, 0 errors and 0 warnings. The population is read from `eslint.config.mjs` (`packages/**/*.{ts,tsx,mts,cts}`). Invariance: that config enables no type-aware linting (its own note), so this diff cannot move a verdict on an untouched file. **Ablations, at `93170f5468`, through `scripts/ablation-replace.mjs` (WRAP) under one verify lock each, with the restore proven.** Each leg ran mutate → `objectql` rebuilt → `ablation-dist-preflight` marker present in 4 built files → pins. Its restore leg ran blob == HEAD `1c4f6d0a41c2` → `git diff HEAD` empty → rebuilt → preflight `--absent` over all 14 built files → tree clean → pins green again (79/79 objectql, 17/17 REST). My first attempt at leg 1 never ran: its lock call timed out (exit 99) before the build, and the tool restored the file. Only the landing runs are reported. | leg (anchor replaced by a marker) | objectql (2 files) | REST (2 files) | what went red | |:--|:--|:--|:--| | the `having` resolution call | 19 failed / 60 passed | 4 failed / 13 passed | every resolved and every refused `having` cell, plus the #20263 file's unknown-token row; the door-order, braces, caller-copy and per-aggregation cells stayed green | | the temporal door's `path` argument | 4 failed / 75 passed | 1 failed / 16 passed | the three temporal rows and the `aggregations[2]` index | | the text-operator door's `path` argument | 2 failed / 77 passed | 1 failed / 16 passed | the two text-operator rows | ## Gates, patch round (head `8b950b8e`) `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` at `8b950b8e` derives the same 65 commands, against the same 8 paths versus merge base `c577e66635`. All 65 ran at `8b950b8e`, each exit code written to a file before any pipe. `--ran` reconciliation: "✓ dispatch-gates --ran: 65 derived famil(ies) accounted for — 63 run, 2 NOT-MEASURED (2 DERIVED from a recorded exit 3)." 0 were unrun, and 62 exited 0. - Red by design: `node scripts/check-empty-changeset.mjs --base origin/main` exits 1 on the DELIBERATE CORRECTION of `.changeset/20263-having-temporal-comparand-door.md`. Its lines: "✓ No empty-frontmatter changeset introduced by this diff (2 declaring changeset(s) added)." and "… DELIBERATE CORRECTION -- your change may have made this PENDING release note false, and you rewrote it in the same stroke. Remedy: do NOT restore it -- say so on the PR and get it confirmed …". This PR says so under Deviations. - NOT MEASURED: `pnpm check:dual-build-cjs-loads` and `pnpm check:type-check-debt`, reason: PREREQUISITE NOT MET (exit 3). They need the whole workspace's built `dist`, which this worktree does not hold ("Run `pnpm build` first"; "7 workspace dependenc(ies) of the ledgered packages have no built type entry point on disk"). CI builds it. - Two gates refused on this clone's shallow history (`check-engine-split-ratio.mjs --days 90`, exit 2; `check-plugin-teardown-shape.mjs --self-test`, exit 3, "cannot read the positive control at 621a487"). The clone was deepened as they prescribe (`git fetch --unshallow origin main`), and both were re-run at `8b950b8e`: exit 0 each ("48 cases pass" for the self-test). The reconciliation records those re-runs. - `check-changeset-no-major.mjs --base origin/main`: "✓ This diff introduces no `major` bump." `check-adr-0087-registration.mjs --base origin/main`: "✓ check-adr-0087-registration: 1 declared-breaking changeset(s), each carrying an ADR-0087 disposition." `check-issue-citations.mjs`: "✅ … every citation this change adds resolves". `check:nul-bytes`: "OK (… no raw ASCII control bytes)". `check:doc-authoring`: all clean. - `origin/main` moved on during the round, to `db74b169d` (4 commits, among them #20370 in `objectql/src/validation/record-validator.ts`). None of them touches a file this PR changes, and the branch is not merged again. The gates read three-dot from the merge base `c577e66635`, so the moving pointer did not enter their change set. ## Gates, first round `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` at `a1a42d4c4a` derives 65 commands, the same 65 as the dispatch's list. All ran at `a1a42d4c4a`. The `--ran` reconciliation found 65 derived, 63 run, 2 NOT-MEASURED and 0 unrun. - NOT MEASURED: `pnpm check:dual-build-cjs-loads` and `pnpm check:type-check-debt`, reason: PREREQUISITE NOT MET (exit 3). Both read whole-workspace built artefacts this worktree does not hold, and CI builds them. - Red by design: `node scripts/check-empty-changeset.mjs --base origin/main` exits 1 on the one DELIBERATE CORRECTION below. In its own words, the fix is to "say so on the PR … and get it confirmed … this gate stays red either way". Its other line: "✓ No empty-frontmatter changeset introduced by this diff (2 declaring changeset(s) added)". - `node scripts/check-issue-citations.mjs --base 2972097`: exit 0, "✅ … every citation this change adds resolves", 8 citations. - Also run, as the derivation flagged their rosters under this diff's directories: `node scripts/check-changeset-fixed.mjs`, `check:authz-resolver`, `check:error-code-casing`, `check:filter-alias-parity`, all exit 0. ## Deviations - **DELIBERATE CORRECTION, `.changeset/20263-having-temporal-comparand-door.md`**: two sentences, numstat 2/2 against `main`. `Check Changeset` stays red by design. Every rewrite, in order: 1. First round, the `What is judged` clause. Before (on `main`): "`having` does not resolve placeholders, and did not before, so the refusal's remedy names none." After (`a1a42d4c4a`): "`having` resolves placeholders from the same release (#20334), after this door, and the refusal's remedy names none." 2. Patch round, the same clause again, because its last half is false at `8b950b8e`. Before: the `a1a42d4c4a` text above. After: "`having` resolves placeholders from the same release (#20334), after this door, so the refusal's remedy on a `date` or `datetime` column is the `where` refusal's and names them, e.g. `{30_days_ago}` / `{current_month_start}`." 3. Patch round, the note's own **Fix.** line gains the placeholder form. Before: "**Fix.** Compare a `date` column with a `YYYY-MM-DD` day, a `datetime` column with an ISO-8601 instant, a bare day or epoch milliseconds, and a `time` column with an `HH:MM` or `HH:MM:SS` wall clock." After: "**Fix.** Compare a `date` column with a `YYYY-MM-DD` day, a `datetime` column with an ISO-8601 instant, a bare day or epoch milliseconds, either one with a relative-date placeholder the resolver knows (`{30_days_ago}`, `{current_month_start}`; `having` resolves them from the same release, #20334), and a `time` column with an `HH:MM` or `HH:MM:SS` wall clock." Its "Unchanged" list ("`{today}`-style placeholders, known or not" and "every existing `having` refusal, in its words") is a before-and-after statement about #20263's own door, so it stays TRUE and is not touched. The sentence "The refusal follows the `where` door's words" is truer now and is not touched either. - **This PR's own `.changeset/20334-aggregate-positions.md`, patch round** (not a correction: the file is new in this PR). Added: the paragraph "**The `having` temporal refusal's remedy is `where`'s.** …", which states the change and the REST bound reading in the patch-round section above. Rewritten: "…keeps that refusal and its words." became "…keeps that refusal, in that door's words.", and the Unchanged clause "every `having` that carries no placeholder, its refusals in their words;" became "every `having` that carries no placeholder, and its refusals in their words other than the `date` / `datetime` temporal refusal's remedy above;". Both old sentences read as "the words did not move", which is false at `8b950b8e`. - **Bounded in-place fix beyond the claimed surface: `packages/objectql/src/text-operator-declared-type-door.ts`**, its `path` parameter and the `path` field's doc line only. All four conditions hold. ① It is ruling 2's defect class exactly: a per-aggregation refusal naming `where`. ② The fix is mechanical and its shape is pinned by the temporal door's. ③ No open PR touches the file (a census of the 13 open PRs' file lists, including #20309's branch; #20335's branch has no commits). ④ The same pins and gate families cover it. The first round's claim did not list this file; the takeover claim 5863879760 now does (open question 2 = A). - **`temporal-comparand-door.ts` beyond "its path parameter only"**: in the first round the `HAVING_REMEDY` doc comment was corrected with no runtime byte moved, and whether the remedy should name a placeholder was put to the seat. The seat answered B (5863946181), so the patch round deletes `HAVING_REMEDY` and its doc comment and the `having` refusal reads `REMEDY`. This is item 1 of that answer, not a surface breach. - **The #20263 pin that held the defect is replaced, not re-spelled.** Its row `'an unknown {placeholder} too'` asserted 200 with no group for `{not_a_token}` on `having`, which is the branch this change removes. It is now a case asserting `FILTER_TOKEN_UNKNOWN` / 400 with zero reads, not the door's `INVALID_FILTER`. In the first round the remedy case's title "having resolves none" was retitled with its assertion unchanged. In the patch round the case is flipped and retitled "the remedy names the placeholder the resolver knows, in the where twin's words". For a `date` and a `datetime` column it asserts the envelope (`INVALID_FILTER` / 400 through `expectHavingRefusal`, both paths, empty or populated, no read), the message's first sentence, `"{30_days_ago}"` in the remedy, and the remedy byte-equal to the `where` twin's. The ablation below turns it red. - **"Pin on the three drivers"** (ruling 3) is executed as #20307 executed it. No driver reads `having` and every refusal precedes the driver, so the engine is pinned on both path shapes with counting drivers (objectql), and the engine and REST doors over a real SqlDriver on SQLite (rest), with the `where` twin as the control everywhere. InMemoryDriver and PostgreSQL were measured (432 cells per tree, above), not pinned. No package in the claim's surface holds the engine together with those drivers: objectql has neither, and rest has `driver-sql` but no `driver-memory` and no CI job hands it a PostgreSQL URL. Adding either is outside the surface. ## Acceptance notes (observations, not filed) - The first round's note that `HAVING_REMEDY` named only literal forms is resolved by the patch round (open question 1 = B). - Over REST, the 500-character bound now cuts a `datetime` column's `having` refusal inside the remedy, before the placeholder it names (`…epoch milliseconds, or a relative-date pl…`). The `where` refusal for a `datetime` field was already cut at the same place on `main` (578 characters, in the patch-round table above). The engine message is whole, and the order kept the bound as it is. So the placeholder half of the remedy reaches an in-process caller, and a REST caller only on a `date` column. Noted, not filed. - The per-aggregation `filter` still resolves tokens through a direct call to core's `resolveFilterTokens` with `filterTokenContextFrom`, which is the same resolver in a second spelling of the stage function `resolveWhereFilterTokens`. It is behaviour-identical and not changed here. - The per-aggregation temporal refusal keeps `where`'s consequence sentence ("reach the driver as written … return 200 with an empty result"). For a per-aggregation filter the consequence is a wrong count for that one aggregation. Ruling 2 moved only the path, so the wording is left as it was. - `.changeset/20148-aggregation-filter-where-doors.md` says the per-aggregation filter is "refused by the temporal-comparand door `where` takes, run unchanged on this position, in its words". I judge it TRUE as scoped to #20148: the same door and sentence, now with this position's location clause. It is not corrected, and the seat may re-judge. ## Open questions (answered by the seat) - **Q1, the `having` remedy's words: B, in this PR** (seat answer 5863946181). `having`'s temporal refusal reads `REMEDY` and `HAVING_REMEDY` retires, done in this patch round (item 3 above). - **Q2, the claim's file surface: A** (seat answer 5863946181). The takeover claim 5863879760 amends its `File surface:` to list `text-operator-declared-type-door.ts`, its path parameter and one doc line. ## Out-of-scope findings None filed or proposed. The only divergence seen, PostgreSQL's native `sum` returned as a string, is #20307's finding 3, which #20335 holds. Authored in two rounds. The first was an os-dev run under `session_01Bvd69VPa6puiNzzPUroDBx` (report 5862763145). The patch round was an os-dev run under the seat session `session_01N8TPEsoJxPsdSdNKGnNGEN` (takeover claim 5863879760; this round's report is on #20334). --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ab6fb02 commit 2f122b6

8 files changed

Lines changed: 686 additions & 31 deletions

‎.changeset/20263-having-temporal-comparand-door.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,14 @@ What is judged:
2828

2929
- The same walk and the same predicate, `isUninterpretableTemporalComparand` in `@objectstack/core`, that the door runs on `where` and on each per-aggregation `filter`. A change to that rule reaches `having` with it.
3030
- The kind is the aggregated column's class, the one the `addDays` rule already reads: `min` / `max` of a `date`, `datetime` or `time` field keeps that kind, a groupBy projection of such a field takes its kind, and a `day` bucket is a `date`. `count`, `count_distinct`, `sum` and `avg`, a `week` / `month` / `quarter` / `year` bucket, and every other column are not temporal, so they are not judged.
31-
- Every comparison and set operator's comparand, each `$in` / `$nin` member and `$between` endpoint, and the implicit-equality slot, under `$and`, `$or` and `$not`. As on `where`, a `{placeholder}` string, the empty string, `null` and a `{ $field }` reference are not judged. `having` does not resolve placeholders, and did not before, so the refusal's remedy names none.
31+
- Every comparison and set operator's comparand, each `$in` / `$nin` member and `$between` endpoint, and the implicit-equality slot, under `$and`, `$or` and `$not`. As on `where`, a `{placeholder}` string, the empty string, `null` and a `{ $field }` reference are not judged. `having` resolves placeholders from the same release (#20334), after this door, so the refusal's remedy on a `date` or `datetime` column is the `where` refusal's and names them, e.g. `{30_days_ago}` / `{current_month_start}`.
3232
- The text operators (`$contains`, `$notContains`, `$startsWith`, `$endsWith`, `$icontains`) are not judged. On `where` the text-operator declared-type door answers them first, and that door does not front `having`.
3333
- The door runs after every other `having` door, so a clause one of them refuses (an unknown operator, a key naming no column, a comparand of no comparable type, an `addDays` pair, an array in the equality slot) keeps that refusal and its words.
3434

3535
The refusal follows the `where` door's words. It names the `having` path, the column, what the column aggregates and its kind, and the comparand, for example: `` `having` on 'last_placed' (max(placed_on), a date column) compares against "not-a-date" at having.last_placed.$lt ``. Like the `where` refusal, it names the column's kind, and otherwise only what the query carries.
3636

3737
**Who is affected.** `having` is a request-only key (`QuerySchema.having`, `EngineAggregateOptions.having`), and no metadata type stores it. Every `having` in this repository's docs and published skills compares a numeric aggregation alias, which is not judged. Callers of `engine.aggregate` and of the REST aggregate query in a deployment were NOT measured.
3838

39-
**Fix.** Compare a `date` column with a `YYYY-MM-DD` day, a `datetime` column with an ISO-8601 instant, a bare day or epoch milliseconds, and a `time` column with an `HH:MM` or `HH:MM:SS` wall clock.
39+
**Fix.** Compare a `date` column with a `YYYY-MM-DD` day, a `datetime` column with an ISO-8601 instant, a bare day or epoch milliseconds, either one with a relative-date placeholder the resolver knows (`{30_days_ago}`, `{current_month_start}`; `having` resolves them from the same release, #20334), and a `time` column with an `HH:MM` or `HH:MM:SS` wall clock.
4040

4141
**Unchanged**, measured identical before and after on the three drivers, both paths and both doors: every `where` and per-aggregation `filter` answer; every `having` on a temporal column whose comparand the rule reads (a `YYYY-MM-DD` day, an ISO instant, an epoch-millisecond number or string, an in-range `Date`, a zone-naive instant, a wall clock, an extended-year instant on a `datetime` column, which that rule reads); `{today}`-style placeholders, known or not; the empty and the whitespace-only string; `null`, `$exists`, `$in` / `$nin`, `$between`, `$not` / `$or` / `$and` and `{ $field }` references; `$contains` and `$startsWith`; every `count` / `sum` / `avg` column, a string comparand included; a `month` bucket; and every existing `having` refusal, in its words.
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
---
2+
"@objectstack/objectql": minor
3+
---
4+
5+
fix(objectql)!: `having` on `engine.aggregate` resolves `{placeholder}` tokens through the resolver `where` uses, so an unknown one is refused `FILTER_TOKEN_UNKNOWN` / 400 instead of keeping no group with a 200; and the per-aggregation `filter`'s temporal and text-operator refusals name `aggregations[i].filter` instead of `where` (#20334)
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) this change adds no transition to migrate. A refused `having` comparand is a `{placeholder}` the resolver cannot resolve: a token outside the vocabulary, which `having` compared as its own literal text, or a context token the request carries no value for. Neither was ever an accepted spelling with a meaning to preserve, and there is no spelling it can be mechanically rewritten to: which token or which literal the author meant is an authoring decision, and the refusal lists the resolvable tokens. `having` is a request-only key that no metadata type stores, so there is no stored document for `objectstack migrate meta` to rewrite. The tables below record the answer each input had and has; they prescribe no rewrite. -->
10+
11+
**BREAKING**: this narrows what `having` accepts on `engine.aggregate`, and on the REST aggregate query (`POST /data/:object/query`) that forwards it there. A `{placeholder}` in `having` is now resolved by the same resolver, with the same refusals, as one in `where`, once per query, before any driver is asked for a row, on both the native `driver.aggregate()` path and the in-memory fallback. It ships as `minor` under the launch-window convention for accept-set narrowings.
12+
13+
Measured before and after through `engine.aggregate` and `POST /data/:object/query`, on InMemoryDriver, SqlDriver on SQLite and SqlDriver on PostgreSQL 16, on both `having` paths, four groups whose `max(placed_on)` falls between 2026-01-02 and 2026-03-01:
14+
15+
| `having` | before | now | the same token in `where` |
16+
|:--|:--|:--|:--|
17+
| an unknown token: `{ last_placed: { $gte: '{not_a_token}' } }`, or a near miss such as `'{TODAY}'`, on any column, under `$and` / `$or` / `$not` or as an `$in` member | 200, keeps no group | `FILTER_TOKEN_UNKNOWN` / 400, no read | the same refusal, in the same words |
18+
| `'{current_user_id}'` with no user on the request, `'{current_org_id}'` with no active organization, `'{record_id}'` always | 200, keeps no group | `FILTER_TOKEN_UNRESOLVED` / 400, no read (a REST request with no user is answered 401 before it reaches the engine, as before) | the same refusal |
19+
| a known token: `{ last_placed: { $gt: '{current_year_start}' } }` on `max(placed_on)` | compared as its own text, which sorts after every digit: keeps no group | compares as `'2026-01-01'`: keeps all four | resolved |
20+
| any known token, as a comparand, an `$in` member or a `$between` endpoint, under `$and` / `$or` / `$not` | compared as its own text (on a date or datetime column, `$gt` / `$gte` kept no group and `$lt` / `$lte` every group) | the groups the resolved value keeps, the same as that value written out | resolved |
21+
| `{ customer_id: '{current_user_id}' }` on a groupBy key, as user `c2` | keeps no group | keeps `c2` | resolved |
22+
23+
Tokens resolve the way they do in `where`: a date macro to a `YYYY-MM-DD` day (a sub-day macro to an ISO instant) in the request's timezone, `{current_user_id}` and `{current_org_id}` from the request. A string that only contains braces (`'a{b}c'`) is not a placeholder and compares as written, as in `where`. The `having` doors run first, as `where`'s do: a clause an earlier `having` door refuses (an unknown key, an unknown operator, a comparand its temporal column cannot read) keeps that refusal, in that door's words.
24+
25+
**The per-aggregation `filter`'s refusals name their position.** A comparand a declared temporal field cannot read, and a text operator aimed at a field that never holds a string, in `aggregations[1].filter` said `at where.placed_on.$gt` / `at where.amount.$contains`, a `where` the author did not write. They now say `at aggregations[1].filter.placed_on.$gt` / `at aggregations[1].filter.amount.$contains`, as that filter's list-shape and comparand-type refusals already did. The code (`INVALID_FILTER`), the status (400) and every other word are unchanged at the engine. Over REST, the message keeps its existing 500-character bound: a `date` field's refusal of a string such as `'not-a-date'`, which fitted within it, now loses the end of its remedy (`"{current_month_s…` at the shortest path), and the refusals that already exceeded the bound still do.
26+
27+
**The `having` temporal refusal's remedy is `where`'s.** A string a `date` or `datetime` aggregated column cannot read (`{ last_placed: { $lt: 'last_30_days' } }` on `max(placed_on)`) was refused with a remedy that named the literal forms only (`Write a "YYYY-MM-DD" calendar day.` on a `date` column), written when `having` resolved no placeholder. Now that it resolves them, the refusal ends in the remedy the same comparand gets in `where`, which names the placeholder: `Write a "YYYY-MM-DD" calendar day, or a relative-date placeholder the resolver knows, e.g. "{30_days_ago}" / "{current_month_start}".` on a `date` column, and the `where` remedy for a `datetime` field on a `datetime` column. The code (`INVALID_FILTER`), the status (400), what is refused and every word before the remedy are unchanged, and so are a `time` column's refusal and the refusal of a number or `Date` whose year falls outside 0000 to 9999. Over REST the message keeps its 500-character bound, which the longer remedy now reaches, measured with an object named `ledger_having`. A `date` column's refusal still arrives whole in every cell measured, the longest at 498 characters (`'+010000-01-01T00:00:00.000Z'` on `max(placed_on)`), and it stays whole while the object name, the column, what it aggregates, the comparand as quoted and its path take at most 90 characters together. A `datetime` column's refusal no longer fits whatever those are, because its fixed words and remedy alone take 502 characters: over REST it now ends inside the remedy, before the placeholder it names (`…epoch milliseconds, or a relative-date pl…` on `min(opened_at)`), as the `where` refusal for a `datetime` field already did. A preset name such as `'last_30_days'` does not reach this refusal over REST: the query schema refuses it first (`VALIDATION_FAILED`), before and after.
28+
29+
**Who is affected.** `having` is a request-only key (`QuerySchema.having`, `EngineAggregateOptions.having`), and no metadata type stores it. The five `having` clauses in this repository's docs and published skills compare a numeric aggregation alias with a number, and none carries a placeholder; no runtime code or example app in this repository composes a `having`. Callers of `engine.aggregate` and of the REST aggregate query in a deployment were NOT measured.
30+
31+
**Fix.** For an unknown token, write one the resolver knows (the refusal lists them: `{today}`, `{current_quarter_start}`, `{30_days_ago}`, `{current_user_id}`, …) or the literal value. For `{current_user_id}` / `{current_org_id}`, send the request with a user or an active organization.
32+
33+
**Unchanged**, measured identical before and after on the three drivers, both paths and both doors: every `where` answer, placeholders and refusals included; every `having` that carries no placeholder, and its refusals in their words other than the `date` / `datetime` temporal refusal's remedy above; every per-aggregation `filter` answer other than the two refusals' paths above, placeholders included.

‎packages/objectql/src/engine-aggregate-having-temporal-door.test.ts‎

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -247,9 +247,32 @@ describe('[#20263] having — a comparand its column cannot read is refused befo
247247
await expectHavingRefusal(() => ({ d: { $gt: Y10000 } }), [{ field: 'opened_at', dateGranularity: 'day', alias: 'd' }]);
248248
});
249249

250-
it('the remedy names no placeholder: having resolves none, so it would send the author to a literal', async () => {
251-
for (const having of [{ last_placed: { $lt: 'x' } }, { first_opened: { $lt: 'x' } }]) {
252-
expect(await expectHavingRefusal(() => having)).not.toContain('{30_days_ago}');
250+
// [#20334] `having` resolves placeholders through `where`'s resolver, so its
251+
// remedy is `where`'s: it names the relative-date placeholder that a caller
252+
// holding a preset name (`last_30_days`) needs, on the two kinds that take one.
253+
it('the remedy names the placeholder the resolver knows, in the where twin\'s words', async () => {
254+
const CASES: ReadonlyArray<readonly [Record<string, unknown>, Record<string, unknown>, string]> = [
255+
[{ last_placed: { $lt: 'last_30_days' } }, { placed_on: { $lt: 'last_30_days' } },
256+
"`having` on 'last_placed' (max(placed_on), a date column) compares against \"last_30_days\" at "
257+
+ 'having.last_placed.$lt, which is not a date value this platform can interpret.'],
258+
[{ first_opened: { $lt: 'last_30_days' } }, { opened_at: { $lt: 'last_30_days' } },
259+
"`having` on 'first_opened' (min(opened_at), a datetime column) compares against \"last_30_days\" at "
260+
+ 'having.first_opened.$lt, which is not a datetime value this platform can interpret.'],
261+
];
262+
const after = (message: string, marker: string): string => {
263+
expect(message).toContain(marker);
264+
return message.slice(message.indexOf(marker) + marker.length);
265+
};
266+
for (const [having, where, firstSentence] of CASES) {
267+
// INVALID_FILTER / 400 on both paths, empty or populated, no read.
268+
const message = await expectHavingRefusal(() => having);
269+
expect(message.startsWith(`aggregate('${OBJECT}'): ${firstSentence} `)).toBe(true);
270+
const remedy = after(message, 'The `having` was NOT applied. ');
271+
expect(remedy).toContain('"{30_days_ago}"');
272+
const twin = await whereTwinOf(where);
273+
expect(twin.err?.code).toBe('INVALID_FILTER');
274+
expect(twin.err?.status).toBe(400);
275+
expect(remedy).toBe(after(twin.err!.message, 'The filter was NOT applied. '));
253276
}
254277
});
255278
});
@@ -306,7 +329,6 @@ describe('[#20263] having — what the door leaves alone answers exactly as befo
306329
['a string on count — not temporal', { n: { $gt: 'not-a-date' } }, []],
307330
['a string on avg — not temporal', { mean: { $gt: 'not-a-date' } }, []],
308331
['a {placeholder} is stepped around, as on where', { last_placed: { $lte: '{today}' } }, ['c1', 'c2', 'c3', 'c4']],
309-
['an unknown {placeholder} too', { last_placed: { $gte: '{not_a_token}' } }, []],
310332
['the empty string (its own card)', { last_placed: { $gt: '' } }, ['c1', 'c2', 'c3', 'c4']],
311333
['null in the equality slot', { last_placed: null }, []],
312334
['$exists', { last_placed: { $exists: true } }, ['c1', 'c2', 'c3', 'c4']],
@@ -325,6 +347,19 @@ describe('[#20263] having — what the door leaves alone answers exactly as befo
325347
});
326348
}
327349

350+
// [#20334] An unknown one is stepped around by this door too, and is then
351+
// refused one layer down by the token resolver, in its own code, as on
352+
// `where` (it kept no group with a 200 before `having` resolved tokens).
353+
it('an unknown {placeholder} is not this door\'s verdict: FILTER_TOKEN_UNKNOWN from the resolver, before any read', async () => {
354+
for (const path of ['native', 'rows'] as const) {
355+
const { engine, reads } = await makeEngine(path, ROWS);
356+
const { err } = await outcome(() => engine.aggregate(OBJECT, query(path, { last_placed: { $gte: '{not_a_token}' } })));
357+
expect(err?.code, path).toBe('FILTER_TOKEN_UNKNOWN');
358+
expect(err?.status, path).toBe(400);
359+
expect(reads, path).toEqual({ aggregate: 0, find: 0 });
360+
}
361+
});
362+
328363
it('a coarser bucket is a text label, and is not judged', async () => {
329364
const kept = await keptGroups(
330365
{ m: { $gt: 'not-a-date' } },

0 commit comments

Comments
 (0)