Skip to content

fix(driver-memory): a row with no value is not inside $gt / $gte / $lt / $lte - #14077

Merged
zhuangjianguo merged 2 commits into
mainfrom
claude/issue-13553-ordering-arms-no-value
Sep 1, 2026
Merged

fix(driver-memory): a row with no value is not inside $gt / $gte / $lt / $lte#14077
zhuangjianguo merged 2 commits into
mainfrom
claude/issue-13553-ordering-arms-no-value

Conversation

@zhuangjianguo

Copy link
Copy Markdown
Collaborator

Fixes #13553

The reference matcher (packages/drivers/driver-memory/src/memory-matcher.ts) compared a stored null instead of first deciding whether the comparison meant anything. On a numeric column JS coerces null to 0, so null >= -1 is a true comparison between two numbers and the no-value row lands inside the bound.

Why this card exists: after #13549 landed, this file contradicted itself

$between routes through valueWithinRange and excludes a null-valued numeric row. But this package's live path compiles $between INTO $gte + $lte (memory-driver.ts). So on main before this PR, one face answered

  • {n: {$between: [-1, 1]}} with ['2'], and
  • {n: {$gte: -1, $lte: 1}} with ['2','3']

on the same rows — although they are one predicate to the live path. That is the same face giving two answers to one query, and it is what this PR resolves.

Measured, before and after

Fixture (the card's): {id:'1', n:5} {id:'2', n:0} {id:'3', n:null} {id:'4'} — key ABSENT on row 4. Both faces driven per cell; origin/main at 8b04c75d7d.

filter live mingo matcher BEFORE matcher AFTER
{n: {$gte: -1}} ['1','2'] ['1','2','3'] ['1','2']
{n: {$gt: -1}} ['1','2'] ['1','2','3'] ['1','2']
{n: {$lte: 1}} ['2'] ['2','3'] ['2']
{n: {$lt: 1}} ['2'] ['2','3'] ['2']

A full sweep of all 18 declared operators on this numeric fixture went from 9 divergences to 3; the 3 that remain are pre-existing, out of scope, and measured byte-identical across the change (below).

The repair

The four ordering operators are named as a set and consulted from the pre-switch guard — the place that already decides the other reading of "no value" (value === undefined). Both readings now land on one answer, EXCLUDE, side by side. That is the ruling on file: the live mingo path excludes such a row, and the platform's settled reading admits a no-value row only for the negation-carrying operators ($ne / $nin / $notContains, #5298 option A, re-affirmed 2026-08-10).

Two things deliberately do not move, and both are enforced by assertions in this PR:

  1. $between is not a member of the set. It decides the no-value case itself, and its answer is the opposite one — the degenerate range whose both ends are absent selects the no-value rows (driver-memory's reference matcher answers {$between: [null, null]} with EVERY valued row — the range arm's two comparisons are both false against a null bound, so a bounded range stops bounding #13495). Adding it to the set would return false before valueWithinRange ran and silently move a ruled cell.
  2. A no-value COMPARAND is excluded from the guard, so {$gte: null} and its three siblings keep today's answer exactly. See the next section — this was the design decision of the card.

The one scope decision, made by measurement

The natural-looking repair is to extract valueWithinRange's comparability half and reuse it for the four arms. Two measurements say not to:

So the guard is scoped to the no-value row — exactly what the card and its triage ruled — and the comparand cells are left byte-identical rather than decided in an operator arm. Their divergence from the live path is reported on the issue as an open question for the maintainer, not resolved here.

Tests

packages/drivers/driver-memory/src/memory-matcher-null-value-and-comparand.test.ts — the file that already pins this class — gains a #13553 block. Every cell asserts the row set on both faces in one call, the file's standing discipline.

  • the card's four cells, on the NUMERIC fixture;
  • the discriminator: {id:'2', n:0} stays IN on every arm. A "repair" that excluded everything falsy — the shape a reader reaches for once told that null coerces to 0 — would drop the zero row too and still turn the four cells above green. This is what stops the pin going vacuous;
  • both readings of "no value" now agree with each other, asserted per arm;
  • $between and its own two bounds answer one row the same way, over three ranges;
  • valued rows keep every answer they had;
  • the string fixture cells are asserted unmoved — they agreed before (null >= '2026-07-01' compares 0 against NaN) and still do, which is why three earlier cards passed over this defect;
  • $between's degenerate all-absent cell is asserted unmoved, on both fixtures.

The {$gte: null} cells are deliberately not pinned, in either direction — pinning them would prejudge a ruling nobody has made. The header note says so, in the same terms the file already uses for #13357's arms.

Verification

Run at 43b799bc66 (the final commit, origin/main merged in):

  • pnpm --filter @objectstack/driver-memory test38 files, 1025 tests passed, TEST_EXIT=0
  • pnpm --filter @objectstack/driver-memory typecheckTYPECHECK_EXIT=0, and tsc --listFiles confirms both edited files are in the program (no *.test.ts exclusion), so that reading covers the new tests
  • dependency closure built first (pnpm --filter '@objectstack/driver-memory^...' build)
  • gate family re-derived from the change set after the final commit (scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack): 34 families, 31 exit 0. Three exited 3 = NOT MEASURED, each by its own printed verdict and none a red: check-test-completeness (grades a saved turbo test log; none exists locally, unreachable in CI), check:dual-build-cjs-loads (needs every package's dist/), check:type-check-debt (needs the full workspace closure built)

Ablation — per-arm separability

Mutating the committed implementation, each leg proved on disk (git hash-object differing from the HEAD blob plus an anchor count) and restored under trap ... EXIT INT TERM with absolute paths pinned to HEAD, each restore proved by a matching blob and an empty git diff HEAD. No rebuild leg applies: the probe imports ./memory-matcher.js relatively, inside the same package, so no dist wall is crossed — evidenced by the source edit changing the measured answers with no build of driver-memory between.

leg mutation predicted observed
A the whole guard removed all four arms red, $between control green 4 failed, 1 passed — exactly the four arms
B only $gte dropped from the set only the $gte arm red 1 failed, 4 passed — ARM $gte
C only $lt dropped from the set only the $lt arm red 1 failed, 4 passed — ARM $lt

A first attempt at leg B was voided by its own on-disk proof and re-run: the anchor '$gte' also occurs in the docblock prose, so the count check failed although the mutation had landed. It is recorded rather than quietly retried.

Invariance of the cells this PR does not decide

Same sweep, before and after, byte-identical in both runs:

filter live mingo matcher before matcher after
{n: {$in: [null]}} ['3','4'] ['3'] ['3']
{n: {$nin: [null]}} ['1','2'] ['1','2','4'] ['1','2','4']
{n: {$gte: null}} ['3'] ['1','2','3'] ['1','2','3']
{n: {$lte: null}} ['3'] ['2','3'] ['2','3']
{n: {$gt: null}} [] ['1'] ['1']
{n: {$lt: null}} [] [] []

The first two are #13357's cells, refused at the contract door since 2026-08-31 and reachable only by a caller that skips the compile face — the boundary memory-null-list-member-unreachable.test.ts states in its own docblock. The guard is written over the OPERATOR set, which is what keeps them outside its blast radius.


Generated by Claude Code

…$lte` (#13553)

The reference matcher compared a stored `null` instead of deciding first
whether the comparison was meaningful. On a NUMERIC column `null` coerces to
`0`, so `null >= -1` is a true comparison between two numbers and the no-value
row landed inside the bound — while this package's live mingo path excluded it.

The four ordering operators now exclude a no-value row on BOTH readings of "no
value" (a stored `null` and an absent key), decided in the pre-switch guard
beside the reading that was already handled there. `$between` is deliberately
not a member of the guarded set: it decides the case itself in
`valueWithinRange`, and its answer for the degenerate all-absent range is the
opposite one. A no-value COMPARAND in an ordering position is excluded from the
guard too, so those cells keep today's answer rather than being decided here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L
@github-actions github-actions Bot added the size/m label Sep 1, 2026
@github-actions

github-actions Bot commented Sep 1, 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
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 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 — 8 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 8dc22d68ddcb52e0c3a1659f967685ddbc1a2b64packageMentionDocs.

Which tree this was computed on

This run read content/docs from 496ce3761e4ba2566cb23f0da33a879afcd64c1b — the merge of head 43b799bc66c22c3e7401f4b781786558981c77b4 into base 8dc22d68ddcb52e0c3a1659f967685ddbc1a2b64, 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 496ce3761e4ba2566cb23f0da33a879afcd64c1b && git checkout 496ce3761e4ba2566cb23f0da33a879afcd64c1b
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 8dc22d68ddcb52e0c3a1659f967685ddbc1a2b64 43b799bc66c22c3e7401f4b781786558981c77b4 && git checkout -B drift-repro 8dc22d68ddcb52e0c3a1659f967685ddbc1a2b64 && git merge --no-ff 43b799bc66c22c3e7401f4b781786558981c77b4

node scripts/docs-audit/affected-docs.mjs --json 8dc22d68ddcb52e0c3a1659f967685ddbc1a2b64

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

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/m tests tooling

Projects

None yet

2 participants