Skip to content

fix(driver-memory): read a stored ARRAY as its elements in the equality arm, so both filter faces answer one filter one way - #17287

Merged
os-sam merged 2 commits into
mainfrom
claude/issue-16838-memory-matcher-scalar-array
Sep 10, 2026
Merged

fix(driver-memory): read a stored ARRAY as its elements in the equality arm, so both filter faces answer one filter one way#17287
os-sam merged 2 commits into
mainfrom
claude/issue-16838-memory-matcher-scalar-array

Conversation

@os-sam

@os-sam os-sam commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Fixes #16838

Clause-②: yes — a published read verb changes which rows it returns (the accept-set direction), so this waits outside the queue under needs:contract-review. That wait is the safe state; the label is not cleared here.

The defect, in one line

memory-matcher.ts's equality arm ended in value == condition. Loose == converts a stored ARRAY to a primitive — ['a','b'] becomes the string "a,b" — so this package's reference matcher and its live query path (InMemoryDriver.find, through mingo) answered the same filter two different ways, in both directions at once:

filter stored reference matcher, before live query path after
{ tags: 'a' } ['a','b'] false — no row 1 row true
{ tags: 'a,b' } ['a','b'] true — the row 0 rows false
{ tags: 'a' } ['a'] true 1 row true — the firing control, unmoved

The second row is the sharper one: a false positive, a filter written to narrow returning a row it should not, which on an RLS read scope is a permission concern rather than a degraded filter. The first is fail-open the other way and just as silent — if (!rows.length) cannot tell "genuinely none" from "the predicate asked the wrong question". Both come from that one line, which is why both move together here.

What changed

comparandEquals becomes the entry every arm calls (implicit equality, $eq, $ne); its previous body is singleValueEquals, byte-unchanged, deciding one value against one comparand. The new entry adds one composition:

A stored array is read as its ELEMENTS, and each is asked the question the arm asks of a scalar — so the answer for a row storing an array is the OR of the answers for the rows storing its elements.

That is mingo's own composition, which is this file's standing tie-break (#5240, #5324, #5328, #5374): the reference face converges on the path this package's users actually run rather than inventing a third reading. The string-join reading was never a reading — no author writes "a,b" meaning ['a','b'].

One level only, measured rather than reasoned: mingo does not descend into a nested array, so neither does this face. [['a']] against 'a' is no row on both faces now; it used to be a match on this one, by the same join.

Refusing the shape was not available, and the difference from #16810 is structural rather than a preference: a refusal is raised from the FILTER by assertFilterConditionShape, once, before any row is seen. This cell is a property of the stored ROW, so a refusal would fire or not fire depending on the data — the exact record-dependence #5240 moved the shape walk out of the field loop to avoid.

The live query path is untouched. It already answered membership. An array in the COMPARAND position is still refused (INVALID_FILTER / 400) by the shape gate — that is #16810's cell, on the other side of the same operator, and this change does not widen it.

#16810's pins moved deliberately, and were not deleted

memory-matcher-array-and-date-comparand.test.ts pinned these three answers as UNCHANGED so that PR's refusal could not move them by accident. The pin did its job: the change that moved them had to come here and say so. The block is rewritten — the three answers now track the value side's own ruling — and it gains the invariant it existed for, stated directly instead of left to be inferred: the comparand refusal must not reach the value side, so a scalar comparand against any stored array must ANSWER rather than throw, while the comparand position still refuses on the same row.

Evidence

Red before, green aftermemory-matcher-scalar-comparand-array-value.test.ts drives BOTH faces in one process over one fixture: the card's three rows, its firing control and the control's negative twin, $eq/$ne, a null comparand against a null member, and the OR-over-elements property over the whole matrix.

before the fix:  Tests  10 failed | 9 passed (19)   — every LIVE-path case passed, every REFERENCE case failed
after the fix:   Tests  20 passed (20)

Ablation, from the committed state — the array arm removed on disk (marker grep 1 → 0, mutated blob 3b02e17 vs HEAD blob a09ace0), then the new suite and #16810's pins re-run:

ABLATION command exit: 1
 Test Files  2 failed (2)
      Tests  11 failed | 27 passed (38)

Restored with git checkout HEAD -- <path>; restored blob a09ace0 equals the HEAD blob and git diff HEAD is empty.

Package suite, at the merge commit, after rebuilding the dependency closure:

pnpm --filter @objectstack/driver-memory test        Test Files 50 passed (50) · Tests 1211 passed (1211)
pnpm --filter @objectstack/driver-memory typecheck   exit 0

The new test file is in the typecheck program (confirmed with tsc --noEmit --listFiles), so its assertions are type-checked rather than merely advertised.

Gates. All 58 commands derived by node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack were run locally at 17058b6d; 55 exited 0, including check:where-matcher (395 matchers discovered and judged), check:driver-conformance, check:driver-memory-census, check:engine-double-contract, check:cross-package-test-inputs, check:test-source-alias, check:changeset-no-major, check:adr-0087-registration and check:nul-bytes.

Three exited 3 = PREREQUISITE NOT MET = NOT MEASURED, never a pass — each needs a whole-repo pnpm build or a re-measure this seat may not perform, and each is left to CI: check:dual-build-cjs-loads, check:lean-entry-closure, check:type-check-debt.

Changeset

minor on @objectstack/driver-memory. Above the floor the seat ruling sets, and justified rather than defaulted: the change is not merely internal repair. A face this package publishes as its reference evaluator now selects rows it previously did not (the accept-set direction), so a consumer that drives it as a driver double, or that compares find() against it, sees a different row set. That is an envelope change on behaviour rather than a new key or export, which is the minor rung; major is refused by CI and would overstate a change no published type or export makes.

Acceptance notes

🤖 Generated with Claude Code

https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU


Generated by Claude Code

…ty arm, so both filter faces answer one filter one way

`memory-matcher.ts`'s equality arm ended in `value == condition`. Loose `==`
converts a stored ARRAY to a primitive — `['a','b']` becomes the string
`"a,b"` — so the reference matcher and the live query path (`InMemoryDriver
.find`, through mingo) disagreed about the same filter in BOTH directions:

| filter            | stored      | matcher, before | live path |
|-------------------|-------------|-----------------|-----------|
| `{ tags: 'a' }`   | `['a','b']` | no row          | the row   |
| `{ tags: 'a,b' }` | `['a','b']` | the row         | no row    |
| `{ tags: 'a' }`   | `['a']`     | the row         | the row   |

The second row is the sharper one: a false positive, a filter written to
narrow returning a row it should not, which on a read scope is a permission
concern rather than a degraded filter. The first is fail-open the other way
and just as silent.

A stored array is now read as its ELEMENTS, and each is asked the question
the arm asks of a scalar — so an array answers the OR of the answers its
elements would give. That is mingo's composition, which is this file's
standing tie-break: the reference face converges on the path users actually
run instead of inventing a third reading. One level only, measured: mingo
does not descend into a nested array, so neither does this face. Refusing the
shape was not available — a refusal is raised from the FILTER before any row
is seen, and this cell is a property of the stored ROW.

`comparandEquals` becomes the entry every arm calls; the previous body is
`singleValueEquals`, unchanged, deciding one value against one comparand.
The live query path is untouched.

Tests: `memory-matcher-scalar-comparand-array-value.test.ts` drives BOTH
faces in one process over one fixture — the card's three rows, its firing
control and its negative twin, `$eq`/`$ne`, a null comparand against a null
member, and the OR-over-elements property over the whole matrix.

#16810's pin block, which recorded this behaviour as unchanged so its own
refusal could not move it by accident, is rewritten rather than deleted: the
three answers move with the value side's ruling, and the invariant the block
exists for — the comparand refusal must not reach the value side — is now
asserted directly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU
@github-actions

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 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 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 ebf9a4891156a20d09889f6984b7d74d3b0301cepackageMentionDocs.

Which tree this was computed on

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

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

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

@claude

claude Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Contract review — seat self-review under the 2026-09-10 maintainer ruling · VERDICT: PASS

Authority: 「你的车道所有任务不需要 fable,任务结束你自己就可以审核」 — this lane is released from CONTRACT_REVIEW_TIER and the dispatching seat reviews its own deliveries. Recorded in full on the seat post (5612940482), including why contract-review.md:60 is suspended for this lane by ruling rather than reinterpreted, and the self-review standard this verdict is held to.

⚠️ Reviewed by the seat that dispatched it. That is 自查放行 by construction, so the measurements below are the whole of the argument — ⛔ nothing here rests on the PR body's word or on this seat's own dispatch reasoning.


1. Does it widen the accept set? YES — and that confirms the seat's declaration rather than assuming it.

memory-matcher.ts:351:

function comparandEquals(value: any, condition: any): boolean {
    if (Array.isArray(value) && !Array.isArray(condition)) {
        return value.some((element) => !Array.isArray(element) && singleValueEquals(element, condition));
    }
    return singleValueEquals(value, condition);
}

{ tags: 'a' } against a stored ['a','b'] answered false before and true after — a published read verb returning a row it previously did not. Clause-②: yes is correct.

2. Does it enlarge the published surface? NO.

git diff origin/main...pr-17287 | grep -E "^\+.*\bexport\b"no added export lines. Both functions are module-private (function singleValueEquals at :280, function comparandEquals at :351 — neither carries export). src/index.ts is not in the diff at all; the four changed files are the matcher, its two test files and the changeset.

3. Grade minor — correct, and it rests on behaviour alone.

Envelope on a published verb. Since no export moves (2), the minor cannot be borrowed from a new-export rung — it is earned entirely by the behaviour change, which is the honest reading. Meets the minor floor the clause-② yes declaration imposes, so check-changeset-no-major's LEVEL AXIS is satisfied by a real grade rather than by a dodged one.

4. Does the implementation do what the body claims? YES, checked against the code rather than the description. The body's before/after table matches :351 arm for arm.

5. Semantic invention — the risk this review was built around. Discharged, with a stated limitation.

The load-bearing claim is that OR-over-elements 「is MongoDB's array semantics and therefore mingo's」. ⛔ Not accepted from the docblock.

memory-matcher-scalar-comparand-array-value.test.ts:59 imports InMemoryDriver from ./memory-driver.js and :154 constructs a real one (new InMemoryDriver({ persistence: false })), driving the live mingo path alongside match from the reference face in one process. CI on head 17058b6dc7: 33 distinct checks, 0 red, legacy combined status success read separately. So the two faces were measured against real mingo and agree.

That is the right correctness standard here, not an appeal to MongoDB docs: this file's standing tie-break (#5240, #5324, #5328, #5374) makes the live path the authority, because it is what users of this package run. Convergence on mingo IS the criterion.

⚠️ Limitation, stated rather than glossed: this container has no node_modules after a restart, so this seat could not drive mingo independently — a direct probe returned ERR_MODULE_NOT_FOUND. That reading is NOT MEASURED by the reviewer, and NOT MEASURED is never a pass. What carries the point is the PR's own test exercising a real driver under green CI, which is a genuine measurement but is the PR's test, not an independent one. Recorded so a later reader can weigh it correctly.

The stated bound is visible in the code and not merely asserted: !Array.isArray(element) makes the composition one level only, so [['a']] against 'a' does not match.

6. Is the negative direction preserved? YES — and #16810's pin was strengthened, not gutted.

An array comparand never reaches the new composition: the !Array.isArray(condition) guard routes it to singleValueEquals, keeping its prior answer, and the shape gate still refuses it. Verified on the rewritten pin, which gained the invariant it existed for, stated directly instead of left inferrable from three answer values:

for (const stored of [['a','b'], ['a'], [], [null,'b'], [['a']]])
  expect(() => match({ tags: stored }, { tags: 'a' })).not.toThrow();
expect(() => match({ tags: ['a','b'] }, { tags: ['a','b'] })).toThrow(/requires a single comparable value/);

⭐ The values track the value side's ruling; the shape of the assertion — an answer, not an exception — is what #16810 pinned, and that is unchanged. A rewritten pin that comes out stronger is the opposite of a deleted one.


Attacking this seat's own dispatch order first, as the standard requires

  • The fence held. The order fenced this work out of filter-refusal.ts, memory-analytics.ts and memory-analytics-time-granularity.test.ts (fix(driver-memory): an analytics time dimension buckets by its declared granularity (#16178) #17206's files). The diff's four files include none of them.
  • ⚠️ The order's ruling 5 did NOT bind, and the implementer was right to pass it by. That ruling said 「where the evidence is ambiguous, prefer the reading that returns FEWER rows」. This fix returns more rows for {tags:'a'} vs ['a','b'] — but ruling 5's precondition is ambiguity, and the evidence is not ambiguous: mingo is the named authority and it returns the row. A tie-break invoked where there is no tie would have produced a third reading, which is the very thing the card exists to remove. The order's own conditional saved it; had it been stated unconditionally it would have been wrong.
  • The clause-② declaration was the seat's and is confirmed by (1) above rather than by the reasoning that produced it.

Open question from the implementer — ruled, so this PASS is not issued over an unanswered one

Its report raised the PR-body attribution footer conflict (harness/dispatch form vs AGENTS.md:420-426) and reported that the MCP create-PR channel appended the AGENTS.md form unprompted, leaving one of each and no hand-duplication. Ruled: that reading is correct and is recorded as a platform measurement on the seat post (5612106443); the template-vs-AGENTS.md conflict itself is routed to the maintainer (option C), ⛔ not a seat's reconciliation. Nothing owed on this PR.

Filed, not folded in

#17286 — the stored-ARRAY axis is repaired for equality only; $in/$nin, the text family and the ordering family still read a stored array as one opaque value, and the two exclusion arms diverge in the widening direction. Correctly filed rather than swept: mingo matches $gt when ANY element satisfies it and its text arms DO descend into a nested array where its equality arm does not, so a sweep would have invented semantics on cells nobody has ruled. That is the same restraint this PR shows on the comparand side.


VERDICT: PASS. Clause ② yes confirmed by measurement; minor correct and earned by behaviour; no published surface moves; the mingo convergence is measured by the PR's own test against a real driver under green CI, with the reviewer's independent probe recorded as NOT MEASURED; #16810's refusal is preserved and its pin strengthened. Clearing needs:contract-review on both carriers.


Generated by Claude Code

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