Skip to content

Commit 44ce049

Browse files
fix(analytics): refuse a zero-operator field constraint on the draft preview (#19896)
Closes #19835 Clause-②: no ## What `packages/services/service-analytics/src/preview-evaluator.ts`: the draft-data preview now **refuses** a field constraint with zero operators (`{ name: {} }`). Before this change it matched every row. **The defect.** `matchesWhere`'s per-field arm iterated `Object.entries(cond)`. An empty object has no entries, so the loop body never ran and the row fell through to the closing `return true`. The card's probe `matchesWhere({ name: 'Globex' }, { name: {} })` answered `true`. The operator-vocabulary refusal that PR #19833 added cannot reach this case: with no key there is nothing to look up. All three shipped drivers refuse the shape (`driver-memory` `filter-refusal.ts`, `driver-mongodb` `mongodb-filter.ts`, and `driver-sql` both at the top level and inside combinators). This package's own `where` door also refuses it (`filter-normalizer`'s wrapper arm). So the drafted chart showed every row for a filter that publish refuses outright. **The repair**, using the refusal plumbing PR #19833 added: - `isEmptyFieldConstraint`, **mirrored locally**. It matches a plain object with zero own keys. The prototype check keeps a `Date` / `RegExp` / class instance out of it, because those are comparands. The predicate has the same shape as the drivers' copies. The exported copy lives in `driver-memory`, which `service-analytics` does not depend on. The card says not to add a cross-package dependency for this, so none was added. - `previewEmptyFieldConstraintError`: the ADR-0112 `INVALID_FILTER` / 400 envelope through the existing `invalidFilterError`. No new error code and no new export. The wording follows `emptyFieldConstraintError`: it names the constraint and its position, and gives the two legal repairs (name an operator, or write a direct comparand). It carries the ruled reasoning that the shape means neither "every row" nor "no rows". Per `check:doc-authoring`, the runtime string has no tracker number. - **Where it fires.** (1) `assertPreviewCanEvaluate` is the row-independent gate that runs before any row is read, and it now walks the whole tree. It tracks the path (`where.$or[1].amount`), so `$and` / `$or` / `$not` nesting cannot route around it. This includes an `$or` arm that a matching row would short-circuit past, and a seed draft with zero rows. (2) `matchesWhere`'s field arm also refuses, so a direct caller gets the same answer. This mirrors the drivers, which judge the shape at every depth. - ⛔ The constraint is **not** read as "matches zero rows". That is the other silent reading, and the ruling behind the drivers declined it. ## Evidence New file `src/__tests__/preview-empty-field-constraint.test.ts`, 11 cases: - The card probe, top level, empty seed, `$and`, `$or` (the short-circuited arm), `$not`, deep nesting, and direct `matchesWhere` under `$and`. Each asserts `code: 'INVALID_FILTER'` + `status: 400`, never a bare `toThrow()`. - Unchanged cases: operator constraints, implicit equality, and an empty **node** (`where: {}` / `$and: [{}]`, which is the identity and not a field constraint). **Reverse verification** (fix committed first, at `3adb37d427`): - Mutation: the pre-fix `preview-evaluator.ts` from base `dabf8d795e` was written to disk, and the landing was proven by `grep -c isEmptyFieldConstraint` = 0. - Result: **8 failed | 3 passed (11)**, vitest exit 1. All 8 refusal cases went red. The 3 unchanged cases stayed green, as expected, since they pin behaviour that has not moved. - Restore was `git checkout HEAD -- path` inside a trap. It was proven by the blob hash `b4aeff76fa` equalling HEAD and by an empty `git diff HEAD`. - The test imports `src` by relative path, so no `dist` was involved. **Package runs at `3adb37d427`:** - `pnpm --filter @objectstack/service-analytics run test`: **115 files / 2453 tests passed**. - `typecheck` (`tsc --noEmit`): exit 0. `--listFiles` includes the new test file. - eslint `--no-inline-config --format json` on both touched `.ts` files: 2 files, 0 errors, 0 warnings. **Derived gate families**: `node scripts/pm/dispatch-gates.mjs` gave 60 derived. Reconciled with `--ran`: 57 exit 0, 3 NOT MEASURED, 0 unrun. The NOT MEASURED ones are `check:dual-build-cjs-loads`, `check:lean-entry-closure` and `check:type-check-debt`. Each exited 3 with `PREREQUISITE NOT MET` because it needs the whole-repo build. That narrowing is declared and left to CI. `check:where-matcher`, `check:doc-authoring`, `check:nul-bytes`, `check:test-source-alias` and `check:published-files` are all green. **Lint narrowing, declared.** The population is read from `eslint.config.mjs`: both files fall under the `packages/**/*.{ts,...}` objects, and neither is ignored (0 "file ignored" warnings). The count is 2 files from the JSON output. Invariance: the config never enables type-aware linting (no `parserOptions.project`), so this diff cannot move any verdict on an untouched file. The repo-wide `pnpm lint` belongs to CI. ## What it costs A drafted chart whose `where` carries `{ field: {} }` now returns `400 INVALID_FILTER` in preview. Before, it rendered a number computed over every row, and that number changed at publish. To fix a filter, name the intended operator: `{ status: { $eq: 'open' } }` or `{ status: 'open' }`. Changeset: `patch` for `@objectstack/service-analytics`. ## Acceptance notes 1. **Not measured**: an end-to-end reproduction through a rendered Live Canvas draft chart. The evidence is the evaluator-level tests above, the same scope as the card. 2. **Noted, not filed.** A `Date` in implicit-equality position has the same enumerate-to-nothing fall-through, and this PR leaves it alone. Probe: `matchesWhere({ d: 'zzz' }, { d: new Date('2026-05-01') })` answers `true`. The `Date` falls into the operator-map arm, and its zero entries become a match. It is deliberately outside this refusal, because the drivers treat a `Date` as a comparand, not a constraint. What it *should* answer (an instant equality through `compare`) is not pinned, and reachability through `queryDataset` (a JSON wire body) was not established. Owner: none. --- _Generated by [Claude Code](https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3230308 commit 44ce049

3 files changed

Lines changed: 239 additions & 5 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@objectstack/service-analytics": patch
3+
---
4+
5+
The draft-data preview **refuses** a field constraint with zero operators (`{ name: {} }`) instead of answering it with every row, so a drafted chart no longer shows rows for a filter publish refuses outright (#19835).
6+
7+
`preview-evaluator.ts`'s `matchesWhere` iterated a field constraint's entries; an empty object has none, so the loop never ran and the row fell through to a MATCH. `matchesWhere({ name: 'Globex' }, { name: {} })` answered `true`. Every data driver refuses this shape (`driver-memory`, `driver-mongodb`, and `driver-sql` at the top level and inside `$and`/`$or`/`$not`), and so does this package's own `where` door, so the preview and publish gave opposite answers to the same filter.
8+
9+
- **Refused in the ADR-0112 `INVALID_FILTER` / 400 envelope**, through the same `invalidFilterError` the preview already uses for an operator it cannot evaluate. No new error code and no new exported symbol. The message follows the drivers' wording: it names the constraint and its position (`where.$or[1].amount`), and gives the two legal repairs (name an operator, or write a direct comparand).
10+
- **Not answered as "matches zero rows" either.** `{ status: {} }` does not mean "no rows". Read literally it means "rows whose status is anything", and the shape is almost always an authoring accident: a filter builder that recorded a field but never its operator. Only a refusal names the constraint to repair.
11+
- **Nesting cannot route around it.** The check walks the whole `where` before any row is read, `$and` / `$or` / `$not` arms included. So a constraint in an `$or` arm that a matching row would short-circuit past still refuses, and so does a seed draft holding zero rows.
12+
- ⚠️ **What it costs**: a drafted chart whose filter carries `{ field: {} }` now returns `400 INVALID_FILTER` in preview, where before it rendered a number computed over every row. Fix: name the operator the constraint was meant to carry, e.g. `{ status: { $eq: 'open' } }` or `{ status: 'open' }`.
13+
- **Unchanged**: constraints that name an operator, implicit-equality comparands, and an empty *node* (`where: {}` or `$and: [{}]`, which is the identity and not a field constraint).
Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#19835] The draft-preview matcher answered a field constraint with ZERO
5+
* operators — `{ name: {} }` — with EVERY row.
6+
*
7+
* `matchesWhere`'s per-field loop iterates the constraint's entries; an empty
8+
* object has none, so the loop never ran and the row fell through to the
9+
* closing `return true`. Probe at `origin/main` before the repair:
10+
* `matchesWhere({ name: 'Globex' }, { name: {} })` → `true`. The #19810
11+
* operator-vocabulary refusal could not reach it: no key, no lookup to fail.
12+
*
13+
* Every shipped driver refuses the shape (`driver-memory` / `driver-mongodb`
14+
* `emptyFieldConstraintError`, `driver-sql` at the top level and inside
15+
* combinators) — ruled on #5240, refused everywhere. So the preview charted
16+
* every row for a filter publish refuses outright.
17+
*
18+
* ## What this file pins
19+
*
20+
* 1. **Refused** — `INVALID_FILTER` / 400 (ADR-0112), pinned by `code` +
21+
* `status`, never by a bare `toThrow()`. ⛔ Not "matches zero rows": that
22+
* is the other silent reading the ruling declined.
23+
* 2. **Not bypassable by nesting** — under `$and`, `$or` (including the arm a
24+
* short-circuit would never reach) and `$not`, and over an EMPTY seed.
25+
* 3. **Unchanged** — a constraint that names an operator, an implicit
26+
* comparand, and an empty NODE (`{}` as the whole `where` or as a
27+
* combinator arm — the identity, not a field constraint) answer exactly
28+
* what they answered before.
29+
*/
30+
31+
import { describe, it, expect } from 'vitest';
32+
import { DatasetSchema } from '@objectstack/spec/ui';
33+
import type { Cube } from '@objectstack/spec/data';
34+
import { AnalyticsService } from '../analytics-service.js';
35+
import { evaluateAnalyticsQueryOverRows, matchesWhere } from '../preview-evaluator.js';
36+
37+
type Refusal = Error & { code?: string; status?: number };
38+
39+
const ROWS: Record<string, unknown>[] = [
40+
{ id: '1', name: 'Acme Corp', amount: 1200 },
41+
{ id: '2', name: 'Globex', amount: 800 },
42+
];
43+
44+
const CUBE: Cube = new AnalyticsService().registerDataset(
45+
DatasetSchema.parse({
46+
name: 'expense_ds',
47+
label: 'Expense',
48+
object: 'expense',
49+
dimensions: [{ name: 'name', field: 'name', type: 'string', label: 'Name' }],
50+
measures: [{ name: 'count', aggregate: 'count' }],
51+
}),
52+
).cube;
53+
54+
function run(where: Record<string, unknown>, rows = ROWS) {
55+
return evaluateAnalyticsQueryOverRows(
56+
{ cube: 'expense_ds', measures: ['count'], dimensions: ['name'], where },
57+
CUBE,
58+
rows,
59+
);
60+
}
61+
62+
function refusalFor(thunk: () => unknown): Refusal | undefined {
63+
try {
64+
thunk();
65+
return undefined;
66+
} catch (e) {
67+
return e as Refusal;
68+
}
69+
}
70+
71+
function namesAnswered(where: Record<string, unknown>): string[] {
72+
return run(where).rows.map((r) => String(r.name)).sort();
73+
}
74+
75+
describe('[#19835] a field constraint with zero operators on the draft preview', () => {
76+
it('the card probe — `matchesWhere({ name: "Globex" }, { name: {} })` — refuses instead of matching', () => {
77+
const err = refusalFor(() => matchesWhere({ name: 'Globex' }, { name: {} }));
78+
expect(err).toBeInstanceOf(Error);
79+
expect(err).toMatchObject({ code: 'INVALID_FILTER', status: 400 });
80+
});
81+
82+
it('refuses a whole preview query at the top level, naming the field and its position', () => {
83+
const err = refusalFor(() => run({ name: {} }));
84+
expect(err).toMatchObject({ code: 'INVALID_FILTER', status: 400 });
85+
expect(err?.message).toContain('where.name');
86+
expect(err?.message).toContain('{ "name": {} }');
87+
});
88+
89+
it('refuses over an EMPTY seed draft — the walk is not a function of the data', () => {
90+
expect(refusalFor(() => run({ name: {} }, []))).toMatchObject({ code: 'INVALID_FILTER', status: 400 });
91+
});
92+
93+
describe('nesting cannot route around it', () => {
94+
it('inside `$and`', () => {
95+
const err = refusalFor(() => run({ $and: [{ name: 'Globex' }, { amount: {} }] }));
96+
expect(err).toMatchObject({ code: 'INVALID_FILTER', status: 400 });
97+
expect(err?.message).toContain('where.$and[1].amount');
98+
});
99+
100+
it('inside `$or`, in the arm a short-circuit would never reach for a matching row', () => {
101+
// Row `Globex` satisfies arm 0, so a per-row walk would `some()` past arm
102+
// 1 and answer it; the refusal must not depend on which row is tested.
103+
const err = refusalFor(() => run({ $or: [{ name: 'Globex' }, { amount: {} }] }));
104+
expect(err).toMatchObject({ code: 'INVALID_FILTER', status: 400 });
105+
expect(err?.message).toContain('where.$or[1].amount');
106+
});
107+
108+
it('under `$not`', () => {
109+
const err = refusalFor(() => run({ $not: { name: {} } }));
110+
expect(err).toMatchObject({ code: 'INVALID_FILTER', status: 400 });
111+
expect(err?.message).toContain('where.$not.name');
112+
});
113+
114+
it('several combinators deep', () => {
115+
const err = refusalFor(() => run({ $and: [{ $or: [{ $not: { name: {} } }] }] }));
116+
expect(err).toMatchObject({ code: 'INVALID_FILTER', status: 400 });
117+
});
118+
119+
it('through `matchesWhere` directly, under `$and`', () => {
120+
const err = refusalFor(() => matchesWhere({ name: 'Globex' }, { $and: [{ name: {} }] }));
121+
expect(err).toMatchObject({ code: 'INVALID_FILTER', status: 400 });
122+
});
123+
});
124+
125+
describe('non-empty constraints answer exactly as before', () => {
126+
it('an operator constraint', () => {
127+
expect(namesAnswered({ name: { $eq: 'Globex' } })).toEqual(['Globex']);
128+
expect(namesAnswered({ amount: { $gt: 1000 } })).toEqual(['Acme Corp']);
129+
expect(matchesWhere({ name: 'Globex' }, { name: { $ne: 'Globex' } })).toBe(false);
130+
});
131+
132+
it('an implicit-equality comparand', () => {
133+
expect(namesAnswered({ name: 'Globex' })).toEqual(['Globex']);
134+
});
135+
136+
it('an empty NODE — the whole `where`, or a combinator arm — is the identity, not a field constraint', () => {
137+
expect(namesAnswered({})).toEqual(['Acme Corp', 'Globex']);
138+
expect(namesAnswered({ $and: [{}] })).toEqual(['Acme Corp', 'Globex']);
139+
expect(matchesWhere({ name: 'Globex' }, {})).toBe(true);
140+
});
141+
});
142+
});

‎packages/services/service-analytics/src/preview-evaluator.ts‎

Lines changed: 84 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@
2222
// live only in the pending seed draft, so there is no live path to hand the
2323
// query to. It is REFUSED — `INVALID_FILTER` / 400, the envelope this package's
2424
// `where` door already speaks — and never answered true. See
25-
// PREVIEW_FIELD_OPERATORS.
25+
// PREVIEW_FIELD_OPERATORS. [#19835] So is a field constraint carrying ZERO
26+
// operators (`{ name: {} }`), at any depth — see isEmptyFieldConstraint.
2627

2728
import {
2829
calendarPartsInTzOrUtc,
@@ -193,6 +194,66 @@ function previewUnevaluableOperatorError(op: string, field: string): Error {
193194
);
194195
}
195196

197+
/**
198+
* [#19835] Is this field spec `{}` — a field constrained by ZERO operators?
199+
*
200+
* A plain object with no own enumerable keys, and nothing else — the predicate
201+
* `driver-memory`, `driver-mongodb`, `driver-sql` and `@objectstack/formula`
202+
* each apply under the same name. A `Date`, a `RegExp` or a class instance also
203+
* enumerates to nothing, but it is a COMPARAND rather than a constraint, so the
204+
* prototype check keeps it out of this refusal exactly as it does there.
205+
*
206+
* Mirrored locally, not imported: the exported copy lives in `driver-memory`
207+
* (`filter-refusal.ts`), a package this service does not depend on, and a
208+
* dependency on a driver for a four-line predicate is the wrong direction.
209+
*/
210+
function isEmptyFieldConstraint(spec: unknown): boolean {
211+
if (spec === null || typeof spec !== 'object' || Array.isArray(spec)) return false;
212+
const proto = Object.getPrototypeOf(spec);
213+
if (proto !== Object.prototype && proto !== null) return false;
214+
return Object.keys(spec as Row).length === 0;
215+
}
216+
217+
/**
218+
* [#19835] `{ field: {} }` — a field constrained by ZERO operators — REFUSED,
219+
* in the same `INVALID_FILTER` / 400 envelope as
220+
* {@link previewUnevaluableOperatorError}.
221+
*
222+
* ⛔ It used to MATCH EVERY ROW, and not by anyone's decision: the per-field
223+
* loop in {@link matchesWhere} iterates the constraint's entries, an empty
224+
* object has none, so the loop body never ran and the row fell through to the
225+
* function's closing `return true`. The operator-vocabulary refusal (#19810)
226+
* could not reach it either — with no key there is no operator to look up and
227+
* no lookup to fail.
228+
*
229+
* Every shipped backend already refuses this shape (#5240 ruled it: refused
230+
* everywhere, one wording): `driver-memory`'s and `driver-mongodb`'s
231+
* `emptyFieldConstraintError`, and `driver-sql`'s at the top level and inside
232+
* `$and`/`$or`/`$not` alike. This package's own `where` door refuses it too
233+
* (`filter-normalizer`'s wrapper arm). So the draft preview charted every row
234+
* for a filter publish never answers at all — the preview/publish divergence
235+
* this evaluator's other refusals exist to make visible.
236+
*
237+
* ⛔ Refused, NOT answered as "matches nothing". `{ status: {} }` does not mean
238+
* "no rows" — read literally it means "rows whose status is anything", and the
239+
* shape is almost always an authoring accident (a filter builder that recorded
240+
* a field and never its operator). Either silent reading hands the author a row
241+
* count they never asked for; only the refusal names the constraint to repair.
242+
*
243+
* The message follows the drivers' wording (constraint, position, the two legal
244+
* repairs) and carries no tracker number, per the runtime-string rule.
245+
*/
246+
function previewEmptyFieldConstraintError(field: string, path: string): Error {
247+
return invalidFilterError(
248+
`[analytics] Field constraint at ${path} carries zero operators ({ "${field}": {} }). A field ` +
249+
`constraint must name at least one operator (e.g. { "${field}": { "$eq": "value" } }) or be a ` +
250+
`direct comparand (e.g. { "${field}": "value" }). It is refused rather than evaluated: the ` +
251+
`draft-data preview used to answer it with EVERY row, while every data driver and the live ` +
252+
`analytics filter refuse it — so the drafted chart was drawn over rows the published one never ` +
253+
`returns. It does not mean "no rows" either; name the operator the constraint was meant to carry.`,
254+
);
255+
}
256+
196257
/**
197258
* [#19810] Refuse a `where` this face cannot evaluate BEFORE any row is read.
198259
*
@@ -206,17 +267,30 @@ function previewUnevaluableOperatorError(op: string, field: string): Error {
206267
*
207268
* It mirrors {@link matchesWhere}'s own traversal exactly, malformed shapes
208269
* included — a non-array `$and` is left for `matchesWhere` to fault on as it
209-
* always has, so this gate widens no refusal beyond the operator vocabulary.
270+
* always has, so this gate widens no refusal beyond the operator vocabulary
271+
* and the zero-operator constraint.
272+
*
273+
* [#19835] The zero-operator constraint is judged HERE, for the whole tree,
274+
* rather than only where {@link matchesWhere} meets it: that walk
275+
* short-circuits (`every`/`some`, and a node returns on its first false entry),
276+
* so a `{ $or: [{ name: 'Globex' }, { amount: {} }] }` would refuse or answer
277+
* depending on which ROW was being tested. A malformed filter is refused for
278+
* every row or none — the posture `@objectstack/formula`'s `assertFilterShape`
279+
* takes for the same shape — and nesting under `$and`/`$or`/`$not` cannot
280+
* route around it, the position `driver-sql` once dropped it in.
210281
*/
211-
function assertPreviewCanEvaluate(where: Record<string, unknown> | undefined): void {
282+
function assertPreviewCanEvaluate(where: Record<string, unknown> | undefined, path = 'where'): void {
212283
if (!where) return;
213284
for (const [key, cond] of Object.entries(where)) {
285+
const here = `${path}.${key}`;
214286
if (key === '$and' || key === '$or') {
215-
if (Array.isArray(cond)) for (const arm of cond) assertPreviewCanEvaluate(arm as Row);
287+
if (Array.isArray(cond)) cond.forEach((arm, i) => assertPreviewCanEvaluate(arm as Row, `${here}[${i}]`));
216288
} else if (key === '$not') {
217289
if (cond !== null && typeof cond === 'object' && !Array.isArray(cond)) {
218-
assertPreviewCanEvaluate(cond as Row);
290+
assertPreviewCanEvaluate(cond as Row, here);
219291
}
292+
} else if (isEmptyFieldConstraint(cond)) {
293+
throw previewEmptyFieldConstraintError(key, here);
220294
} else if (cond !== null && typeof cond === 'object' && !Array.isArray(cond)) {
221295
for (const op of Object.keys(cond as Row)) {
222296
if (!PREVIEW_FIELD_OPERATORS.has(op)) throw previewUnevaluableOperatorError(op, key);
@@ -240,6 +314,11 @@ export function matchesWhere(row: Row, where: Record<string, unknown> | undefine
240314
if (!(cond as Row[]).some((c) => matchesWhere(row, c as Row))) return false;
241315
} else if (key === '$not') {
242316
if (matchesWhere(row, cond as Row)) return false;
317+
} else if (isEmptyFieldConstraint(cond)) {
318+
// [#19835] Zero entries would leave the loop below unrun and fall through
319+
// to a MATCH. Refused here too, so a direct caller of this matcher gets
320+
// the same answer the row-independent gate gives a whole query.
321+
throw previewEmptyFieldConstraintError(key, key);
243322
} else if (cond !== null && typeof cond === 'object' && !Array.isArray(cond)) {
244323
for (const [op, expected] of Object.entries(cond as Row)) {
245324
if (!matchOp(row[key], op, expected, key)) return false;

0 commit comments

Comments
 (0)