Skip to content

Commit 10d05bb

Browse files
os-muskclaudeos-musk
authored
fix(driver-memory)!: refuse an array comparand and compare Date comparands by time value (#16840)
* fix(driver-memory)!: refuse an array comparand and compare Date comparands by time value `checkCondition` routed both `Date` and `Array` into `value == condition` and called it "exact match" two lines above. Between two objects `==` compares REFERENCES, so it is neither: a deep-equal array and an equal-instant `Date` both answered false, fail-closed and silent. The two halves get different dispositions, and the difference is the contract's: - `Date` is a member of `ACCEPTED_FILTER_COMPARAND_TYPES` and the conformance table requires a Date comparand to "pass the door and execute everywhere", so it is EVALUATED — by time value, arm for arm with `@objectstack/formula`'s `looseEq`, including the Date-against-stored-ISO-text case this driver's own datetime canonicalisation produces. - An array in an implicit or scalar-operator position is a cell the spec's comparand door names and declines to rule; `ACCEPTED_FILTER_COMPARAND_TYPES` has no array member and `driver-sql` refuses one. So it is REFUSED, from `assertFilterConditionShape` — the one gate every face of this package runs — so the live query path, the matcher and the analytics face answer alike. Measured before the change, one row `{ tags: ['a','b'] }`, filter `{ tags: ['a','b'] }`: the live path returned the row, the reference matcher returned none. One filter, one package, two answers. A scalar comparand against a stored ARRAY is deliberately untouched and now pinned: that is the value side, which the comparand door does not judge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg * docs(changeset): declare the array-comparand refusal and the Date equality repair Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg * fix(driver-memory): keep the tracker id out of the refusal's runtime string `check:doc-authoring` red on the new message: a runtime string reaches authors, operators and generated surfaces, none of whom can resolve `#NNNN`. The id stays in the adjacent docblock, where the reader who can resolve it reads the source. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg * docs(changeset): grade @objectstack/spec minor to match this PR's clause-② declaration The level axis of scripts/check-changeset-no-major.mjs refuses a `patch` on a package whose packages/*/src/** the PR moves while the PR body declares `Clause-②: yes`. Raising the level is route 1 of the two the gate names; the declaration is the claim's and is not touched here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: os-musk <elon@objectstack.ai>
1 parent b37f0b1 commit 10d05bb

8 files changed

Lines changed: 408 additions & 14 deletions
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
"@objectstack/driver-memory": minor
3+
---
4+
5+
fix(driver-memory)!: an array comparand is refused instead of compared by reference, and a `Date` comparand is compared by time value
6+
7+
<!-- adr-0087: not-required (no-migration-prescription) An accept-set narrowing and an evaluation repair inside one driver's filter gate: no metadata key, spec symbol, Zod schema, object definition or stored representation is added, removed or renamed. `packages/spec`'s half of this diff is a docblock sentence and one test comment — `filter-comparand-type.ts`'s list of the cases its door deliberately does not rule, corrected to state what the drivers now do — and no schema member, accepted key or exported symbol moves with it, so `check:api-surface` and `check:authorable-surface` see an unchanged surface. What moves is which FILTERS `@objectstack/driver-memory` accepts, which is authored at a call site or in a stored `where`, never a key `objectstack migrate meta` could visit: there is no old spelling to rewrite into a new one, because the refused shape has no replacement spelling that means the same thing — an array comparand never had agreed semantics to preserve, which is the whole reason the cell is refused rather than converged. Measured on this tree, the authored corpus that could carry the shape holds none of it: a structural scan of every tracked JSON and YAML file with a `where` / `filters` / `filter` object (10 JSON candidates, 1 YAML candidate — the latter a GitHub Actions job named `filter`, not a filter) finds ZERO bare-array comparands, and across the repository's own suites exactly one case authored the shape (`{ code: [] }`, in this package's face-agreement table), moved in this diff to assert the refusal. Whether an out-of-repo host authors one is NOT measured and is not claimed to be zero. -->
8+
9+
`checkCondition` routed both `Date` and `Array` into `value == condition`, under a comment two lines above calling that "exact match" — and a comment seven lines below giving `==`'s reasons, all three of which are about primitives ("undefined/null mismatch", "string/number coercion", "convenient in weakly typed queries"). Between two objects `==` does no conversion at all: it compares REFERENCES. So the arm was neither exact nor a match, and the two adjacent comments described something the code could not do.
10+
11+
**BREAKING** accept-set narrowing on a published driver, shipped as `minor` under the repo's launch-window convention for breaking changes (`scripts/check-changeset-no-major.mjs`): a purely additive widening takes at least `minor`, and a narrowing is not the additive bucket, so `patch` is refused here — the public surface really does move. The two halves take DIFFERENT dispositions, and the difference is the contract's rather than this driver's:
12+
13+
**An ARRAY comparand is refused.** `@objectstack/spec`'s comparand door names an array outside `$in`/`$nin`/`$between` as a position it deliberately does not rule, leaving it "to the layers that already answer it"; `ACCEPTED_FILTER_COMPARAND_TYPES` has no array member, and `driver-sql` refuses one with its own message. This driver now answers the same way, in the ADR-0112 envelope its every other filter refusal speaks (`code: 'INVALID_FILTER'`, `status: 400`), with a message naming the field, the received shape, the accepted set and the operators that DO take a list. The refusal is raised from `assertFilterConditionShape` — the one gate every face of this package runs before it evaluates anything — so the live query path, the reference matcher and the analytics face cannot disagree about it.
14+
15+
Why refused rather than made to work, measured on one row `{ tags: ['a','b'] }` with the filter `{ tags: ['a','b'] }` before this change: the live query path returned the row (mingo deep-equals arrays) and the reference matcher returned none (`==` compared references). One filter, one package, two answers, neither reported — the same shape as this package's `{ field: {} }`, malformed-`$between` and non-boolean-`$null` refusals. Converging UP to deep equality was available and is not what landed: the contract declines to rule the cell, and every sibling that has answered it declines the semantics.
16+
17+
**A `Date` comparand is EVALUATED, not refused** — the opposite disposition, because the contract does rule this one. `Date` is a member of `ACCEPTED_FILTER_COMPARAND_TYPES`, and `FILTER_COMPARAND_TYPE_CASES` requires that a Date comparand "must pass the door and execute everywhere". It is now compared by time value, arm for arm with `@objectstack/formula`'s `looseEq` — the sibling record-at-a-time matcher this face's conformance suites are held against — which also settles the case that actually reaches a stored row: a declared `datetime` is canonicalised to ISO TEXT on write (ADR-0053 D-B1), so a `Date` comparand meets a STRING, and `==` stringified the Date to `"Wed Jan 01 2026 …"`, which no ISO value equals. `$eq` / `$ne` take the same equality, so one predicate no longer answers two ways depending on which spelling the author used. An Invalid Date has no time value and so equals nothing, itself included.
18+
19+
**What is deliberately NOT changed**, and is now pinned so a later edit cannot take it away by accident: a SCALAR comparand against a stored ARRAY. `{ tags: 'a' }` against a row storing `['a','b']` still answers false, and `{ tags: 'a,b' }` against that row still answers true, because `==` stringifies the stored array to `"a,b"`. That is a third bad direction of the same operator, but it is on the VALUE side, and the comparand door judges comparands. It is recorded, not repaired. The list operators (`$in` / `$nin` / `$between`), for which an array is the declared comparand, are untouched; so is the text family, whose comparand disposition this package's refusal gate had already recorded as deliberately fail-closed.
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
"@objectstack/spec": minor
3+
---
4+
5+
docs(spec): the comparand door's array parenthetical states what the drivers do today
6+
7+
`filter-comparand-type.ts`'s list of the cases its door deliberately does not rule described the array cell as "`driver-sql` refuses it with its own message; the document stores give it array-equality semantics". Measured on this tree, that second clause is no longer a true statement about `@objectstack/driver-memory`: its reference matcher compared an array comparand by REFERENCE (matching nothing), while its live query path deep-equalled it (matching the row) — one package, two answers, neither of them a stable "array-equality semantics" a reader could build on. With that driver's cell now refused, the sentence names the two dispositions that exist: `driver-sql` and `driver-memory` refuse it, each with its own message, and `driver-mongodb` hands it to MongoDB and inherits that engine's array semantics.
8+
9+
The paragraph's point is unchanged and deliberately kept: the door does not rule this position, the matrix did not measure it, and it is left to the layers that already answer it. Only the description of what those layers do is corrected — a stale factual clause is how the next reader re-derives "the spec promises array-equality" from a passage that explicitly promises nothing. The neighbouring pass-through pin's comment carried the same stale characterisation and is corrected the same way; its assertion (that `parseFilterAST` leaves `{ tags: ['a','b'] }` untouched) is unchanged and still pins the door not judging.

packages/drivers/driver-memory/src/filter-refusal.ts

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@
3131
*/
3232

3333
import { FILTER_OPERATORS, LOGICAL_OPERATORS, RETIRED_FILTER_OPERATORS } from '@objectstack/spec/data';
34+
// [#16810] The accepted comparand-type set's own sentence, quoted rather than
35+
// hand-copied — the same single-sourcing `driver-sql`'s refusals use, so the
36+
// two backends cannot describe the accepted set differently.
37+
import { ACCEPTED_FILTER_COMPARAND_TYPES_SENTENCE } from '@objectstack/spec/data';
3438
// [#7536] The `$like` pattern language's shared gate, so this driver refuses
3539
// the same malformed patterns as every other face.
3640
import { hasDanglingLikeEscape } from '@objectstack/spec/data';
@@ -553,6 +557,85 @@ export function retiredFilterOperatorError(
553557
);
554558
}
555559

560+
/**
561+
* [#16810] The comparison operators whose comparand is a SINGLE value.
562+
*
563+
* The list operators (`$in` / `$nin` / `$between`) are deliberately absent: an
564+
* array is their DECLARED comparand. The text family (`$contains`,
565+
* `$startsWith`, `$icontains`, `$like`, …) is absent too, and that absence is a
566+
* decision this gate already recorded — "a stringified comparand for the LIKE
567+
* family" is on its own "deliberately NOT refused" list, fail-closed and left
568+
* alone. `driver-sql` refuses an array there as well (it cannot bind one); this
569+
* driver keeps its recorded disposition rather than widening a refusal past the
570+
* cell that was ruled.
571+
*/
572+
const SINGLE_VALUE_COMPARISON_OPERATORS: ReadonlySet<string> = new Set([
573+
'$eq', '$ne', '$gt', '$gte', '$lt', '$lte',
574+
]);
575+
576+
/**
577+
* [#16810] An ARRAY where a comparison expects one comparable value —
578+
* `{ tags: ['a','b'] }` and `{ tags: { $eq: ['a','b'] } }`.
579+
*
580+
* ## The cell, and who already answers it
581+
*
582+
* `@objectstack/spec`'s comparand door names this position and steps around it:
583+
* an array outside `$in`/`$nin`/`$between` "is answered per driver today … the
584+
* ruling does not name it, so the door leaves it to the layers that already
585+
* answer it". `ACCEPTED_FILTER_COMPARAND_TYPES` — the six types the door DOES
586+
* rule — has no array member, and `driver-sql` refuses one with
587+
* `unbindableComparandError`. This is that same answer, in this package's
588+
* envelope.
589+
*
590+
* ## Why refused rather than deep-equalled — the two answers, measured
591+
*
592+
* This package's faces did not agree, and the disagreement is the whole reason
593+
* a refusal beats either silent answer. On one row `{ tags: ['a','b'] }`, with
594+
* the filter `{ tags: ['a','b'] }`:
595+
*
596+
* | face | answer |
597+
* |---|---|
598+
* | the live query path (`InMemoryDriver.find` → mingo) | the row — mingo deep-equals arrays |
599+
* | the reference matcher (`memory-matcher.ts`) | NO row |
600+
*
601+
* The matcher's arm routed an array into `value == condition`, and `==` between
602+
* two objects compares REFERENCES: a deep-equal array is never that reference,
603+
* so the predicate could not match any row it had not been handed literally.
604+
* Fail-closed and silent — fewer rows, no error, no warning — which is the
605+
* `#5240` / `#5328` / `#5347` shape exactly: one filter, one package, two
606+
* answers, and `if (!rows.length)` cannot tell "genuinely none" from "the
607+
* predicate never ran".
608+
*
609+
* Converging them UP (deep equality on both faces) was available and is not
610+
* what landed: the spec's door does not rule the cell, and every sibling that
611+
* has ruled it declines the semantics — `driver-sql` refuses it,
612+
* `@objectstack/formula`'s record-at-a-time matcher answers `false` by an
613+
* explicit "a bare array value is not a valid field spec" arm, and objectui's
614+
* `ValueDataSource` was resolved the same way. A driver that calls itself a
615+
* Reference Implementation inventing array-equality alone, on a cell the
616+
* contract declined to rule, is how backends drift apart.
617+
*
618+
* ⚠️ The refusal is about the COMPARAND, never the stored value:
619+
* `{ tags: 'a' }` against a row storing `['a','b']` is a scalar comparand and
620+
* is untouched by this rule — the value side is not a position this door
621+
* judges.
622+
*/
623+
export function arrayComparandError(field: string, value: unknown, path: string, op?: string): Error {
624+
const position = op
625+
? `Operator "${op}" on field "${field}"`
626+
: `The implicit-equality comparand on field "${field}"`;
627+
return unsupportedFilterError(
628+
`${position} requires a single comparable value, but received an array ` +
629+
`(${safeShapePreview(value)}) at ${path}. Use ${ACCEPTED_FILTER_COMPARAND_TYPES_SENTENCE}; ` +
630+
`for a list use $in/$nin, and for a range use $between. It is refused rather than compared ` +
631+
`because this package's two faces answered it differently and neither said so — the live ` +
632+
`query path deep-equalled the array and returned the row, while the reference matcher ` +
633+
`compared it by REFERENCE (== between two objects is a reference test) and returned none. ` +
634+
`@objectstack/spec's comparand door leaves this position to the driver, and driver-sql ` +
635+
`refuses it too.`,
636+
);
637+
}
638+
556639
/** [#5324] `$and`/`$or` take a list of nodes; anything else is refused. */
557640
export function filterNodeListExpectedError(key: string, value: unknown, path: string): Error {
558641
return unsupportedFilterError(
@@ -669,6 +752,11 @@ function assertFieldConstraintShape(
669752
path: string,
670753
capabilities: FilterFaceCapabilities,
671754
): void {
755+
// [#16810] The IMPLICIT-equality position, checked before the plain-object
756+
// test below because an array is not a filter node and would otherwise leave
757+
// this walk unjudged — which is how it reached the matcher's `==` arm and the
758+
// live path's deep equality with nobody reconciling the two.
759+
if (Array.isArray(spec)) throw arrayComparandError(field, spec, path);
672760
if (!isFilterNode(spec)) return;
673761
// [#5240] The zero-operator constraint keeps its own predicate rather than an
674762
// inlined `keys.length === 0`, so the reasoning for what does and does not
@@ -702,6 +790,15 @@ function assertFieldConstraintShape(
702790
if (op === '$null' && typeof spec[op] !== 'boolean') {
703791
throw nonBooleanNullComparandError(field, spec[op], `${path}.$null`);
704792
}
793+
// [#16810] An ARRAY comparand on a single-value comparison — the operator
794+
// spelling of the implicit-equality position refused at the top of this
795+
// function, and the same cell `@objectstack/spec`'s comparand door leaves
796+
// to the driver. The fourth comparand-shape rule this gate makes, for the
797+
// reason the three above it give: a shape the operator cannot evaluate was
798+
// being answered silently, and differently, by each face.
799+
if (SINGLE_VALUE_COMPARISON_OPERATORS.has(op) && Array.isArray(spec[op])) {
800+
throw arrayComparandError(field, spec[op], `${path}.${op}`, op);
801+
}
705802
// [#6520] `$icontains`' comparand is a NON-EMPTY string, the third
706803
// comparand-shape rule and the twin of `driver-sql`'s
707804
// `icontainsComparandError` — deliberately the same two rejections in one

packages/drivers/driver-memory/src/memory-driver-filter-logic-conformance.test.ts

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -568,7 +568,11 @@ const OPERATOR_CASES: Array<[name: string, where: FilterCondition, expected: str
568568
// the operator table's inability to say "this operator takes the whole list".
569569
['an empty $in selects nothing', { code: { $in: [] } } as FilterCondition, []],
570570
['an empty $nin selects everything', { code: { $nin: [] } } as FilterCondition, ['1', '2', '3']],
571-
['an empty implicit-equality list selects nothing', { code: [] } as unknown as FilterCondition, []],
571+
// [#16810] `{ code: [] }` — the empty implicit-equality ARRAY — left this
572+
// table when the cell was ruled. It is not a row-set case any more: an
573+
// array in an implicit-equality position is REFUSED on every face, and the
574+
// property this entry carried (the faces agree about it) is asserted below
575+
// as an agreement about the REFUSAL instead of about the empty row set.
572576
['a non-empty $in still selects its members', { code: { $in: ['100'] } } as FilterCondition, ['1', '3']],
573577
];
574578

@@ -652,6 +656,42 @@ describe('[#5374] operator semantics — the analytics face against the live que
652656
});
653657
}
654658

659+
/**
660+
* [#16810] An ARRAY comparand in an implicit-equality position — refused, and
661+
* refused by BOTH faces with one envelope.
662+
*
663+
* This replaces the row-set case `{ code: [] }` held in `OPERATOR_CASES`
664+
* above. The empty array was the harmless-looking member of the cell: its
665+
* two silent answers happened to coincide at "no rows", which is why nobody
666+
* noticed that a NON-empty one did not. Measured before the refusal landed,
667+
* on a row storing `['a','b']` with the filter `{ tags: ['a','b'] }`: the
668+
* live query path returned the row (mingo deep-equals arrays) and the
669+
* reference matcher returned nothing (`==` between two objects is a
670+
* reference test). One filter, one package, two answers — the #5240 shape.
671+
*
672+
* The assertion is the AGREEMENT, which is what the retired entry was for.
673+
*/
674+
it('an implicit-equality ARRAY comparand is refused, and both faces refuse it alike', async () => {
675+
const where = { code: [] } as unknown as FilterCondition;
676+
const nonEmpty = { code: ['100'] } as unknown as FilterCondition;
677+
678+
for (const [label, filter] of [['empty', where], ['non-empty', nonEmpty]] as const) {
679+
await expect(findIds(filter), `${label}: the live query path did not refuse it`).rejects.toMatchObject({
680+
code: 'INVALID_FILTER',
681+
status: 400,
682+
});
683+
await expect(analyticsIds(filter), `${label}: the analytics face did not refuse it`).rejects.toMatchObject({
684+
code: 'INVALID_FILTER',
685+
status: 400,
686+
});
687+
}
688+
689+
// The list operators are the declared spelling and are untouched — the
690+
// refusal must not read as "this driver stopped accepting arrays".
691+
expect(await findIds({ code: { $in: [] } } as FilterCondition)).toEqual([]);
692+
expect(await findIds({ code: { $nin: [] } } as FilterCondition)).toEqual(['1', '2', '3']);
693+
});
694+
655695
/**
656696
* `contains` and `notContains` partition the table, stated as a predicate over
657697
* every comparand rather than case by case. An inert `$notContains` fails this

0 commit comments

Comments
 (0)