Repository navigation
Commit 9f13c94
fix(objectql)!: a string comparand against a boolean field is narrowed to its boolean, or refused 400, at the engine filter door (#21372)
Fixes #21333
Clause-②: yes (narrowing)
## What this does
A comparand against a declared `boolean` / `toggle` field (or a
`formula` returning `boolean`) is now judged at the engine's one
field-aware filter walk, the same walk that judges number comparands:
- `true` / `false` reach the driver as written (the controls);
- `1` / `0`, `"1"` / `"0"` and `"true"` / `"false"` are **narrowed** to
`true` / `false`, copy-on-write, so every driver receives the one
boolean each spelling names;
- any other string (`"yes"`, `"TRUE"`, `" true "`, `""`, a
`{placeholder}`) is **refused** `INVALID_FILTER` / 400, naming the field
and its declared type, before any driver is resolved.
That holds at `where` (object form and `FilterArray` sugar), the
per-aggregation `filter` and `having`, on every verb that collects a
filter (`find` / `findOne` / `count` / `aggregate` / `update` /
`delete`, plus `judgeFilter`). Every REST spelling reaches that one walk
unchanged (the POST body `where`, `?filter=` JSON, `?$filter=`, the
filter AST, and bare query parameters), so there is no per-door
coercion.
The accepted set is exactly the one the record validator's boolean arm
admits on WRITE (`record-validator.ts`), per the triage ruling
(5946403646).
## Files
- `packages/spec/src/data/filter-boolean-comparand-declared-type.ts`
(new, exported from `@objectstack/spec/data`). This is the contract:
`BOOLEAN_COMPARAND_SPELLINGS`, `readBooleanComparand`,
`booleanComparandFieldVerdict` / `booleanComparandDoorVerdict`,
`booleanComparandRefusalMessage`, the reading table, the fixture and the
derived `BOOLEAN_COMPARAND_DOOR_CASES`. It is additive. The judged
positions are the number door's lists by identity (pinned).
- `packages/objectql/src/boolean-comparand-declared-type-door.ts` (new).
This is the boolean arm: the field meta, the routed verdict and the
words. ⛔ It walks nothing.
- `packages/objectql/src/number-comparand-declared-type-door.ts`. The
existing `walkCondition` asks the boolean arm at every field key the
number arm does not judge. `judgeFieldSpec` now takes the judging arm,
so both arms judge at the same positions by construction. ⛔ No second
walker.
- `packages/objectql/src/engine.ts`. This file only gets `[#21333]`
notes at the four existing call sites (where, lowered where,
per-aggregation filter, having). No new call site.
- Tests: `filter-boolean-comparand-declared-type.test.ts` (spec) and
`engine-boolean-comparand-declared-type-door.test.ts` (objectql). The
number engine suite gets a named partition for its two census rows on
`f_boolean` / `f_toggle`: they pass the number verdict and are now
refused by the boolean arm, and that partition is pinned in the
direction it answers.
- `packages/spec/api-surface/data.json`,
`packages/spec/export-origins/data.json`: regenerated (26 additions, 0
removals).
- Two changesets: `objectql` `minor`, BREAKING, `Clause-②: yes
(narrowing)`, with an ADR-0087 `not-required
(no-migration-prescription)` disposition; and `spec` `minor`, additive,
`Clause-②: yes`, with no ADR-0087 marker (it is not a breaking
changeset). See the review round below.
## The card's table, measured before and after, on both drivers
Two rows (one `true`, one `false`). Measured with a one-time harness
through `engine.find`, `engine.aggregate` (`where` count and
per-aggregation `filter` count) and `findData` for all five REST
spellings, on InMemoryDriver and SqlDriver/SQLite. The harness used
freshly built dists: base `6c5bef5f4`, and after on the merged head
`1c184d7695`. Every cell is a 200 unless it says otherwise.
| comparand | memory before | SQLite before | memory after | SQLite
after |
|:--|:--|:--|:--|:--|
| `"true"` (implicit, `$eq`, `$in`, all 5 REST spellings, aggregate
count) | 0 rows | 0 rows | 1 (true row) | 1 (true row) |
| `$ne "true"` / `$nin ["true"]` | **2 rows** | **2 rows** | 1 (false
row) | 1 (false row) |
| `"false"` / `$ne "false"` | 0 / 2 | 0 / 2 | 1 / 1 | 1 / 1 |
| `"yes"` / `$ne "yes"` / `"TRUE"` | 0 / 2 / 0 | 0 / 2 / 0 | 400
`INVALID_FILTER` | 400 `INVALID_FILTER` |
| control `1` / `"1"` / `0` / `"0"` | **0 rows** | 1 | 1 | 1 |
| control `$ne 1` / `$ne "1"` | **2 rows** | 1 | 1 | 1 |
| control `true` / `false` / `$ne true` / `$in [true]` | 1 | 1 | 1 | 1 |
| per-aggregation `filter`: `"true"` / `$ne "true"` | 0 / 2 | 0 / 2 | 1
/ 1 | 1 / 1 |
| `having` over `groupBy f_boolean`: `"true"` / `$ne "true"` / `"yes"` |
no group / both | no group / both | true group / false group / 400 |
true group / false group / 400 |
The after-run had zero mismatches against the card's correct column on
either driver. ⚠ The ruling's premise that `1` / `0` / `"1"` / `"0"` are
"already answered correctly" holds on SQLite only: InMemoryDriver
answered them with no row (strict `true` vs `1`). Narrowing every
accepted spelling to its boolean is what makes the ruling's own pin
("the card's table answers its correct column on both drivers") true
there. That is a measured refinement of the premise, ⛔ not a switch of
the ruling.
## Mechanism hypotheses (dispatch Zone 2)
- **H1, holds.** The number door's walk is shareable. The boolean twin
is an arm of `walkCondition`, not a copied walker, and the four engine
sites run both arms with no new call.
- **H2, holds.** `GET /api/v1/data/:object` hands `req.query` to
`findData`, which folds leftover keys into an implicit `where` of
strings (`metadata-protocol` `protocol.ts`, the implicit-filters block)
and calls `engine.find`. The engine door therefore sees `"true"`, and no
REST-side coercion is needed. The GET-door rows are pinned through
`findData` in objectql's suite (bare-param spelling included), so no
`packages/rest` pin was added.
- **H3, holds.** `$ne`, `$in`, `$nin` (and `$between`) members follow
the same narrowing. The baseline `$ne "true"` is 2 rows on both drivers,
as in the table.
- **H4.** The contract could live entirely in objectql without weakening
"one door": every surface reaches the one engine walk, and the walk is
the only runtime consumer today. It lives in `packages/spec/src/data/`
anyway, beside the number contract, for three reasons. The record
validator's write arm carries the identical accepted set as a literal,
and one grammar both sides can import belongs where both can reach it
(the `parseNumericString` precedent). The refusal words and case table
are the public contract the door answers in. And a consumer outside
objectql (see H5) can read it without importing the engine. The module
is additive and exported.
- **H5.** Two consumers answer the card's rows wrongly at doors this PR
does not touch. They are reported below as out-of-scope findings, ⛔ not
widened into.
## Reverse verification
The implementation was committed first (HEAD `1c184d7695`). The arm was
then ablated through `scripts/ablation-replace.mjs` (anchor `const
booleanMeta = booleanArmFieldMeta(facts.boolean);`, replaced by `null`;
anchor 1 to 0, blob `cc4b1198` to `b001e9b3`), and both engine suites
were run:
- **26 red**: every refusal pin and every narrowing pin of the boolean
suite (25: the case table's refusals and narrowings, the card's table
and refusals, every verb, FilterArray, nested structure, placeholder,
`judgeFilter`, aggregate `where`, per-aggregation `filter`, `having`,
and all ten REST-door cases), plus the number suite's boolean-arm
partition (1).
- **37 green**: the controls (`true` / `false` / `null` / flag /
`$field` cases reaching the driver unchanged, the by-reference guard),
the boolean suite's pure partition guard, and all 36 other number-door
pins.
- Restore proven by the tool: blob after restore equals the HEAD blob
`cc4b1198`, and `git diff HEAD` is empty.
## Tests and gates (all at HEAD `1c184d7695`, freshly built dists)
- `@objectstack/spec`: `test` 599 files / 17541 passed (1 todo),
`test:repo` 48 / 849, exit 0. `typecheck` exit 0. The new test is in
`tsconfig.test.json`'s program (`--listFiles`).
- `@objectstack/objectql`: `test` 363 files / 7301, `test:repo` 1 / 5,
exit 0. `typecheck` exit 0 (new files in `tsconfig.test.json`'s
program).
- `@objectstack/rest`: `test` 254 files / 4805 passed (316 skipped),
`test:repo` 5 / 177 (1 skipped), exit 0. `typecheck` exit 0.
- `@objectstack/driver-memory`: 70 files / 1718, exit 0. `typecheck`
exit 0.
- `@objectstack/driver-sql`: 215 files passed (11 skipped) / 3594 passed
(202 skipped), exit 0. `typecheck` exit 0.
- `@objectstack/dogfood`: 166 files passed (1 skipped) / 1369 passed (3
skipped), exit 0. `typecheck` exit 0.
- `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack
--commands` derived 91 families on this head and all 91 ran; `--ran`
reconciles 91 / 91, 0 unrun. Each family's exit code was captured before
any pipe. All were 0 except `check:dual-build-cjs-loads`, whose first
run was `PREREQUISITE NOT MET` (eight unrelated packages had no `dist/`;
nothing measured); after building those eight it ran to exit 0. The run
includes `check:dispatcher-error-vocabulary` (no new code:
`INVALID_FILTER` through the existing `invalidFilterError`),
`check:adr-0087-registration`, `check:empty-changeset`, `spec
check:generated`, `check:nul-bytes`, `check:driver-memory-census` and
`check:test-source-alias`.
## Acceptance notes
1. **Live-driver pins.** The committed pins drive a recording driver.
The arm answers before any driver is resolved, and a narrowed comparand
is pinned to reach the driver as the byte-identical filter its boolean
control does. The per-driver row counts above were measured with a
one-time harness, not committed. A committed SQLite cell would be a
`packages/rest` pin (`domain:cli`). A committed InMemoryDriver cell
needs a ruling on the closed `check:driver-memory-census`. objectql
depends on neither driver.
2. **Not ruled, left as written.** A number other than `1` / `0`, a
`Date`, or an array member against a boolean field passes the verdict
and reaches the driver as written (no stored boolean equals `2`). The
ruling refuses strings only. Whether to close the set is the analogue of
the number door's later widening, so it is an open question for the
maintainer, ⛔ not done here.
3. **One grammar, two sides, one literal.** `record-validator.ts`'s
boolean write arm still spells the accepted set as a literal rather than
reading `BOOLEAN_COMPARAND_SPELLINGS`. They are equal today, and the
spec test pins the set's content. Carrier: none named.
## Out-of-scope findings (for the seat to file; ⛔ not fixed here)
- **RLS `using` predicates compare a boolean as written.** The compiled
policy filter is composed after the caller's filter door, by design, so
`record.flag != 'true'` keeps the true row and `== 'true'` keeps none.
Measured after this change through `SecurityPlugin`'s real middleware
over a real engine on SqlDriver/SQLite, with `engine.find` and a member
context, on two rows: `== true` gives the true row, `== 'true'` gives
none, `!= 'true'` gives **both**, `== 'yes'` gives none (silently).
Reach: exception (security: a policy's exclusion is not applied). No
in-repo producer writes such a predicate today.
- **Analytics NativeSQL strategy compiles `runtimeFilter` itself.**
`POST /api/v1/analytics/dataset/query` over SqlDriver/SQLite
(`AnalyticsServicePlugin` composition, NativeSQL answered) gives:
`{"flag":"true"}` 200 count 0 (should be 1), `{"flag":{"$ne":"true"}}`
200 count 2 (should be 1), `{"flag":"yes"}` 200 count 0 (the engine door
refuses it 400). Reach: public door measured. Seam:
`spec:booleanComparandDoorVerdict` to runtime `service-analytics`
NativeSQL `where` compilation.
## Review round 1 (head `6f74eb444c`, after the at-tier contract review
FAIL 5948769828)
Section added by the `domain:engine#1` seat, from the dev's patch-round
report (5949409460 on #21333):
- **The FAIL was on ② alone.** `Clause-②: no (narrowing)` was false. The
26 additive `@objectstack/spec/data` exports widen the public surface,
and the line was the seat's own false declaration on the claim,
corrected by the claim amendments 5948515811 and 5948799813. This body's
first lines now read `Clause-②: yes (narrowing)`,
`scripts/pm/clause2-line.mjs`'s spelling for a diff that widens one
surface and narrows another.
- **The changesets** (commit `6f74eb444c`, the only change in this
round; no code moved):
- `objectql`: `Clause-②: yes (narrowing)`, still BREAKING. Six sentences
are scoped to what was measured: `InMemoryDriver` and `SqlDriver` over
SQLite, at `findData` rather than the HTTP route, and "every door that
reaches the engine's filter walk". One of them is the sentence the
review named.
- `spec`: `Clause-②: yes`. The BREAKING banner, the narrowing arm and
the ADR-0087 marker are removed, which is `b285508188`'s shape. Its
"before" and "remedy" sentences, which described `objectql`'s engine,
are replaced by "What moves for consumers".
- **Gates at `6f74eb444c`:** 91 derived, 91 run, all exit 0.
- `check-adr-0087-registration` lists one declared-breaking changeset
(`objectql`'s).
- `check-changeset-no-major`'s level axis, driven offline with this
body's line, exits 0.
- `check-widening-tells --diff` exits 0 under `--declaration yes`, and
exits 4 under the old `no` with 31 tells (26 × T3, 5 × T2). That
confirms the review.
- No suite was re-run, because the code is unchanged since `1c184d7695`.
- **The review's escalations:**
- closing the accepted set to non-string comparands is a follow-up card.
The patch round measured it on four drivers: PostgreSQL 16 answers `500
DATABASE_ERROR` at every slot, and an array `$in` member splits 200 /
400 across drivers;
- the RLS seam and analytics NativeSQL positions are filed as #21376;
- a CI-visible `packages/rest` SQLite cell is an acceptance note.
- **Lane.** Triage's answer 5948860364 on #21333: the spec lane lands
this PR whole, with the `objectql` half declared. The `domain:engine`
seat hands the card over without marking this PR ready or enqueueing it.
---
_Generated by [Claude
Code](https://claude.ai/code/session_017xfMoEjKUuSh2xYB8sCozp)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 6d487d2 commit 9f13c94
12 files changed
Lines changed: 1697 additions & 17 deletions
File tree
- .changeset
- packages
- objectql/src
- spec
- api-surface
- export-origins
- src/data
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
Lines changed: 111 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
0 commit comments