Skip to content

Stop a lowered operator key from clobbering a sibling — one rule for the class, on both document-shaped drivers and the analytics face - #13550

Merged
zhuangjianguo merged 3 commits into
mainfrom
claude/issue-13524-operator-key-clobber
Aug 30, 2026
Merged

Stop a lowered operator key from clobbering a sibling — one rule for the class, on both document-shaped drivers and the analytics face#13550
zhuangjianguo merged 3 commits into
mainfrom
claude/issue-13524-operator-key-clobber

Conversation

@zhuangjianguo

Copy link
Copy Markdown
Collaborator

Fixes#13524

A field operator whose lowering reuses another operator's key silently clobbered it. Both document-shaped translators wrote every lowered key into ONE object literal, so two constraints on one field landed on one key and the second assignment won — no error, no warning, nothing in the emitted query to see it by. WHICH constraint disappeared was decided by the author's key order, because that is the order Object.keys walks. On an RLS read scope, a dropped constraint is a widened one.

The card's table, reproduced on origin/main at 50cf2940b9 before anything was written

Fixture {id:'1',name:'a'}, {id:'2',name:'b'}, {id:'3',name:null}, driven through InMemoryDriver.find(), with the reference matcher (memory-matcher.tsmatch()) as the oracle:

filterlive pathreference matcher
{name: {$null: false, $ne: 'b'}}['1','3']['1']
{name: {$ne: 'b', $null: false}}['1','2']['1']

Exactly the card's numbers. The premise holds.

The must-answer question, answered by ENUMERATION

Every operator in the declared vocabulary was probed ALONE and its lowered key set recorded, then the sets were intersected. This is measurement, not reading — the probe is now the executable table in mongodb-operator-key-clobber.test.ts.

Declared authorable field operators, n=18 (SUPPORTED_FIELD_OPERATORS = FILTER_OPERATORS plus $like and $ilike): $eq $ne $gt $gte $lt $lte $in $nin $between $contains $notContains $startsWith $endsWith $icontains $null $exists $like $ilike.

Contested lowered keys — a key more than one authorable operator writes:

lowered keywritten bydriver-memorydriver-mongodb
$eq$eq, $null: true, $exists: falseclobberedclobbered
$ne$ne, $null: false, $exists: trueclobberedclobbered
$gte$gte, $betweenclobberedclobbered
$lte$lte, $betweenclobberedclobbered
$lt$lt, $lte on a bare calendar day, $between (bare-day max)clobberedclobbered
$regex$contains, $startsWith, $endsWith, $icontains (+ $like, $ilike on memory)already promoted via _multiRegexclobbered
$not$notContains — and NOTHING elsenot reachablenot reachable

Three results the card did not have:

  1. There IS a fourth member: $lte with a bare YYYY-MM-DD comparand. The whole-day rewrite compiles it half-open, onto $lt, which an author writes too. Measured: {d: {$lte: '2026-07-28', $lt: '2026-07-02'}} answered ['1'] and its key-swapped twin ['1','2'], oracle ['1'].
  2. There is a fifth, and it is driver-mongodb's alone: the whole $regex string family.driver-memory has promoted its string family to $and branches for years; this face never did. Measured: {name: {$startsWith: 'a', $endsWith: 'z'}} emitted {name: {$regex: 'z$'}} and the swap emitted {name: {$regex: '^a'}} — one anchor silently gone in each direction.
  3. $notContains is NOT reachable. Nothing else writes $not, and $not is a LOGICAL operator absent from the field vocabulary, so it cannot be authored beside it on one field. The card's third named member was reasoned, not executed. It is covered here by construction rather than curatively — which is the point of a rule for the class.

One rule for the class, generalising the guard #13195 landed

#13195 hit this class from the other side and gave $exists a guard: a lowered write whose key is free merges inline, a write whose key is taken becomes its own $and branch. That guard was deliberately scoped to one operator. This generalises it rather than putting a second idiom beside it: every arm now records its writes through one accumulator, and one shared assembly applies the same rule to all of them. _presenceAnd is subsumed by a _extraAnd list, since one field constraint can contest more than one key.

One addition to that shape: which writer keeps the inline slot is decided by the spec's declared operator order, not by the author's key order. Author order would keep the ANSWER correct — $and is commutative — while leaving the emitted DOCUMENT a function of key order, which is the property this card exists to remove. Ranking by FILTER_OPERATORS order also reproduces #13195's landed documents byte for byte, because $exists is last in that list.

Writes are collected in author order and assembled afterwards, so refusals still fire in the order the author wrote them and an uncontested key keeps its original insertion position. A filter with no contested key emits exactly what it emitted before — which is why every pre-existing test in both packages passes unchanged, with nothing re-baselined.

The analytics face — measured separately, and it is a different, wider defect

MemoryAnalyticsService.query() built its $match as matchStage[fieldPath] = builder(...). That is not a per-key clobber but a wholesale one: the stage is keyed by field path alone, so a second predicate on a member replaced the first ENTIRELY, for every operator pair. Measured: {name: {$contains: 'a', $ne: 'b'}} aggregated ['1','3'] and its key-swapped twin ['1'], oracle ['1'] — and neither operator shares a lowered key with the other, so the translators never lost this one.

Worse, flattenFilterCondition folds $and into the same flat list, so {$and: [{name: {$contains:'a'}}, {name: {$ne:'b'}}]} — the shape a dashboard actually authors — lost a constraint too.

The remedy is the same RULE one level up (free member inline, taken member becomes its own $and branch of the same $match) but a different mechanism, keyed by field path rather than by lowered key, and it needs no ranking because nothing is being overwritten. It is eight lines and lives in the file the card names, so it is here rather than deferred. Also measured and pinned: generateSql pushes into a LIST and never clobbered, so before this the echoed statement and the executed answer described different filters.

Both key orders, and the ablation that proves the order cases bite

Every repaired cell has a case in each key order. Three ablations, each mutated on a committed implementation, proven on disk by grep counts of both the injected marker and the deleted text, restored under an EXIT INT TERM trap with an absolute repo root, and each restore proven by an empty git diff HEAD plus a HEAD-blob hash match:

ablationwhat it breaksdriver-memorydriver-mongodb
A1 — guard removed entirely (the original clobber)everything8 of 13 RED7 of 32 RED
A2 — ranking kept, promotion removed (order-INDEPENDENT but lossy)the oracle half5 of 13 RED7 of 32 RED
A3 — promotion kept, ranking removed (correct but order-dependent documents)the key-order half only13 of 13 GREEN6 of 32 RED

A3 is the one worth reading. It is semantically correct in both directions, so every row-set assertion passes; only the two-direction DOCUMENT assertions catch it. And the before-picture shows the same thing from the other side: $between + $gte was right by accident when the range was written first and wrong when it was written second, while $between + $lte failed in the OPPOSITE direction. Neither one-direction suite catches the class.

The sweep is written against the declared operator set rather than a hand-list, and fails loudly if the comparand table stops covering it, so a nineteenth operator cannot join the vocabulary without this coverage being told.

Ablation note: both new suites import their subject through RELATIVE paths inside their own package, so vitest compiles the source directly and no dist sits between the mutation and the reading. Every leg was nevertheless verified on disk before it was measured.

Verification, all at 3b7b4d4c4b

  • pnpm --filter @objectstack/driver-memory exec vitest run — 35 files, 959 tests passed
  • pnpm --filter @objectstack/driver-mongodb exec vitest run — 24 files, 542 passed, 143 skipped (the gated real-mongod suites)
  • Downstream consumer sweep, prefix filter (...@objectstack/driver-memory, ...@objectstack/driver-mongodb) — 16 of 20 packages ran to completion, all green: runtime 2973, rest 2709, plugin-auth 1744, driver-turso 1108, service-datasource 591, client 405, cloud-connection 340, http-conformance 86, hono 74, service-sms 70, verify 48, client-react 34, plugin-dev 58, embed-objectql 2, plus the two changed packages. Roughly 11,700 tests, zero failures.
  • packages/cli and the three example apps exceeded the container's foreground ceiling as a batch. Narrowed and declared: the four packages/cli test files that name driver-memory or InMemoryDriver were run directly — 90 tests, all passing.
  • pnpm lint (eslint . --no-inline-config, the whole repo, not narrowed) — exit 0
  • pnpm check:nul-bytes — OK, 7518 text files scanned; plus a direct control-character grep over every touched path, no hits
  • The gate family derived by node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack from the real change set — 34 commands, 32 exit 0. The two non-green are declared PREREQUISITE NOT MET by the gates themselves and measured nothing: check-test-completeness needs a saved turbo run test log, and check-half-states needs a real GitHub credential this container does not have. check:dual-build-cjs-loads and check:type-check-debt both refused until the workspace closure was built, then passed: 102 require entry points across 66 packages load, and 29 ledger entries re-measured with none above its recorded number.

Scope

Untouched: packages/spec, docs/adr, .claude, skills, AGENTS.md, CLAUDE.md, content/docs/releases. No test was skipped, disabled, quarantined, allow-listed, deleted or re-baselined — every pre-existing test passes as written.

Filed while measuring, out of scope here: #13549 — the reference matcher matches a null-VALUED row against a well-formed bounded $between while the live path excludes it. Distinct from the two queued matcher cards, both of which were read before filing: #13495 is the null-BOUND axis of the same arm, #13494 the $eq: null guard. Neither of those is addressed here, and both remain open.


Generated by Claude Code

zhuangjianguoand others added 3 commits August 30, 2026 21:31
…lobbering a sibling
Generalise #13195's per-operator guard into one rule for the class: a lowered
write whose key is free merges inline, a write whose key is taken becomes its
own $and branch. Ranked by the spec's declared operator order so the emitted
document is a function of the constraint set, not of the author's key order.
Also promotes the analytics face's wholesale per-member clobber the same way.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L
The vocabulary sweep is written against the declared operator set rather than a
hand-list, so a new operator cannot join without this coverage being told.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L
@github-actionsgithub-actionsBot added size/xl documentation Improvements or additions to documentation tests tooling labels Aug 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

7 anchor(s) derived from 2 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 name(s) were too generic to anchor anything (single lowercase words)
  • 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 50cf2940b9668154ab4d2ba8b08336cf5d93a470packageMentionDocs.

Which tree this was computed on

This run read content/docs from 611bb213e8ae717317911e489fa993b5d11769ea — the merge of head 3b7b4d4c4b57a0978901d014c5c3ed3a0d14b681 into base 50cf2940b9668154ab4d2ba8b08336cf5d93a470, 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 611bb213e8ae717317911e489fa993b5d11769ea && git checkout 611bb213e8ae717317911e489fa993b5d11769ea
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 50cf2940b9668154ab4d2ba8b08336cf5d93a470 3b7b4d4c4b57a0978901d014c5c3ed3a0d14b681 && git checkout -B drift-repro 50cf2940b9668154ab4d2ba8b08336cf5d93a470 && git merge --no-ff 3b7b4d4c4b57a0978901d014c5c3ed3a0d14b681
node scripts/docs-audit/affected-docs.mjs --json 50cf2940b9668154ab4d2ba8b08336cf5d93a470

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

@zhuangjianguoClaude

Copy link
Copy Markdown
CollaboratorAuthor

PM review — ACCEPT, landing held until CI converges (11 success · 2 skipped · 16 running · 0 red at time of writing). My bar is every check green. ⛔ I do not merge it.

⭐ The enumeration was the point of this card, and it returned more than the class was thought to hold

Triage required enumeration, not example. What came back is a measurement, not a list: each of the 18 declared authorable operators translated alone through both drivers, with a second comparand wherever the lowering is comparand-dependent, and the emitted key sets intersected pairwise. It found:

  • A fourth member$lte on a bare YYYY-MM-DD comparand lowers onto $lt. Measured ['1'] against ['1','2'] on the two key orders, oracle ['1'].
  • A fifth, driver-mongodb's alone — the $regex string family, which driver-memory already promoted via _multiRegex and mongodb never did: {$startsWith:'a', $endsWith:'z'} emitted {$regex:'z$'} and the swap emitted {$regex:'^a'}.
  • A negative result, which is the sign of a real enumeration: $notContains is not reachable. It writes $not, nothing else writes $not, and $not is a logical operator absent from the field vocabulary — so it cannot be authored beside itself. Covered by construction, and that fact is pinned rather than assumed.

⇒ The card named three members. The class is five, minus one that was never reachable. A list would not have found that; only walking the vocabulary did.

⭐ And it is durable: the sweeps assert against the declared operator set with a coverage assertion, so a nineteenth operator fails loudly instead of being silently skipped.

The key-order requirement, and the ablation that actually proves it

Triage: 只测一个方向的用例,恰好是本缺陷能活下来的原因. Every repaired cell is asserted in both orders, plus an 18-operator × 306-ordered-pair × 2-order sweep on memory and 16 × 240 × 2 on mongodb.

⭐ The ablation that proves the order cases bite is A3 — promotion kept, ranking removed: semantically correct in both directions, so all 13 memory row-set tests stay GREEN while 6 of 32 mongodb two-direction document assertions go RED. That isolates exactly the property under test: only the two-direction document assertions catch order-dependence. And independently, $between+$gte and $between+$lte fail in opposite directions on the broken code — so neither one-direction suite could have caught the class.

Three ablation legs, each mutation proven on disk by grep counts of both the injected marker and the deleted text, each restore proven by an empty git diff HEADand a per-file HEAD-blob hash match.

The analytics face — measured, and the card's suspicion confirmed

A different and wider defect, proven with a pair that shares no contested key: {name:{$contains:'a', $ne:'b'}} aggregated ['1','3'], the key-swapped twin ['1'], oracle ['1'] — a pair both translators always handled correctly. ⇒ Not the per-key clobber; a wholesale per-member one.

⭐ And a consequence nobody had asked about: generateSql pushes into a list and never clobbered, so before this the echoed statement and the executed answer described different filters. That asymmetry is now pinned.

Same rule, different mechanism, eight lines, in the file the card names ⇒ correctly in this PR rather than deferred.

One precision I checked and would state differently

The PR describes the rank as read off "the spec's declaration order, not a hand-copy of it." Verified on main: SUPPORTED_FIELD_OPERATORS lives in driver-memory's own filter-refusal.ts and is [...FILTER_OPERATORS, '$like', '$ilike'] — spec's array spread, plus two names spec's own docblock (filter.zod.ts:2199) says driver-memory "widens by hand".

⇒ The property holds — the rank has one source and anything in that array is ranked the day it lands — but the source is driver-memory's array derived from spec, not spec's array directly. Worth saying precisely, because "the spec's declaration order" invites a reader to look in the wrong file. ⛔ Not a defect and not a change request.

Fences and the oracle

Clean: no content/docs/releases/, packages/spec/, docs/adr/, .claude/, skills/. ⭐ memory-matcher.ts is untouched — it is this card's oracle, not a target, and it was re-confirmed before the first edit and again in the final diff. So no collision with the queued #13494 / #13495.

Clause ② re-checked by the executor and holds: FieldOperatorsSchema declares each operator independently and says nothing about how two on one field compose, so this conforms to the reference matcher's existing answer rather than changing what the contract accepts.

Zone 2 — C broke (the three known members were not the whole class) and D broke in one direction (the analytics clobber is wider, but that argued for including it, not deferring it). A held and was used; B held and was re-confirmed twice.

946 + 510 pre-existing tests passed with zero edits — nothing re-baselined, skipped or deleted.

⇒ On full green I flip ready and enqueue. #13549 (a reference-matcher $between divergence on the value axis, distinct from #13495's null-bound axis) → pm:retriage.


Generated by Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/xlteststooling

Projects

None yet

1 participant

@zhuangjianguo