fix(service-analytics): the draft preview refuses a filter operator it cannot evaluate instead of answering every row - #19833
Conversation
…t cannot evaluate `preview-evaluator.ts`'s `matchOp` switch carried ten cases and then `default: return true; // unknown operator — permissive (preview, reads only)`. Every other declared operator — `$icontains`, `$notContains`, `$startsWith`, `$endsWith`, `$null`, `$exists`, the staged `$like` / `$ilike`, and any typo — therefore matched EVERY row: a drafted chart with `name $icontains 'acme'` charted the whole dataset and looked like a working chart, then changed at publish, where the real filter doors apply the operator. A predicate that answers true for every row does not narrow a query, it WIDENS it (#3948, #4286/ADR-0078, #5345). The switch becomes ONE table whose keys ARE the face's vocabulary — `memory-analytics`' `MONGO_TO_CUBE_OPERATOR` shape, taken for this same defect — and anything outside it is refused in the ADR-0112 `INVALID_FILTER` / 400 envelope this package's `where` door already speaks, through `filter-normalizer`'s exported `invalidFilterError`. No new error code, no new export. Refused rather than excluded: an excluded row makes the preview merely DIFFERENT from publish, the silent shape #16322 abolished on this same evaluator; a refusal makes the divergence visible to the author who can fix it. The gate is row-independent, so an empty seed draft refuses too. The ten evaluated arms are unchanged, byte-for-byte. Claude-Session: https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF Co-authored-by: Claude <noreply@anthropic.com>
…evaluable filter operator Claude-Session: https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift Check7 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not 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
Coarse fallback — 9 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 198b4aee6e5bec9849cdde9a2d6768bba2e8ea5f && git checkout 198b4aee6e5bec9849cdde9a2d6768bba2e8ea5f
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 1cacfe4a425dc0c2cdd4eb5d0088d5b3f5a16c92 2deda8dab5b44d9056035ae9c24a8f1188122ee3 && git checkout -B drift-repro 1cacfe4a425dc0c2cdd4eb5d0088d5b3f5a16c92 && git merge --no-ff 2deda8dab5b44d9056035ae9c24a8f1188122ee3
node scripts/docs-audit/affected-docs.mjs --json 1cacfe4a425dc0c2cdd4eb5d0088d5b3f5a16c92 |
Contract review (retroactive — PR landed as a6a4361 before this record was written)Served-tier:
① Derived judgmentsThe changeset prose was checked claim by claim. Every factual claim is TRUE:
② Semver level
③ Boundary flagsZero Implemented-by: VERDICT: PASS Non-blocking defects:
Generated by Claude Code |
…preview (objectstack-ai#19896) Closes objectstack-ai#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 objectstack-ai#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 objectstack-ai#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>
Fixes #19810
Clause-②: no
The analytics draft-data preview (
packages/services/service-analytics/src/preview-evaluator.ts, the ADR-0037 P3 Live Canvas path) answered true for every row for anywhereoperator its switch had no case for. A drafted chart therefore silently IGNORED those filters and CHANGED at publish, where the real filter doors apply them.The enumeration, read at source before any edit
At
origin/mainc1dfa5241b,matchOp's switch carried exactly TEN cases and one default arm:$eq,$ne,$gt,$gte,$lt,$lte,$between,$in,$nin,$contains:109)default: return true; // unknown operator — permissive (preview, reads only)$notContains,$startsWith,$endsWith,$icontains,$null,$exists(the rest ofFILTER_OPERATORS), the staged$like/$ilike, and any typoThe card's premise holds exactly as filed. The combinators
$and/$or/$notwere and remain handled bymatchesWhereitself.The repair, and which contract it matches
Fail-closed by REFUSING —
INVALID_FILTER/400, throughfilter-normalizer.ts's already-exportedinvalidFilterError. No new error code and no new exported symbol (the module's five exports are byte-identical before and after).Three candidate behaviours, and why refusal:
lowerPreviewDateRangeabolished on this same evaluator (Analytics drivers (memory, SQL) aligndateRangestring handling to the closed preset vocabulary (driver half of #16041) #16322), so it trades one invisible divergence for another;The yardstick is what the real filter doors do, read rather than invented:
driver-memory'suncompilableFieldOperatorError(driver-memory 的 analytics 面静默丢弃大半个 filter:$or/$not整条丢,$between/$startsWith/$null/$regex因无 cube 映射而丢 —— 聚合结果被放大 #5345): "It is refused rather than dropped: a predicate that compiles to nothing does not narrow the query, it WIDENS it — the aggregate is then computed over rows the filter excluded, and a chart drawn over them looks like a working chart (A filter with an operator outside VALID_AST_OPERATORS is silently dropped, not rejected — single-condition views return unfiltered results #3948, [P2] data:QueryASTdeclares 12 members no executor runs — the liveness ledger governs metadata types, not the request surface #4286/ADR-0078)."service-analyticsalready refuses$like/$ilikethis way. From theFILTER_OPERATORSdocblock's own face table: "driver-mongodb,objectqlhaving,service-analytics— REFUSE, loudly, in the ADR-0112INVALID_FILTERenvelope". This change puts the preview face on the posture its own package already holds.ViewFilterRuleSchemaaccepts two shapes every consumer refuses, andObjectGridProps.defaultFiltersisz.unknown()so nothing gates it at all — the protocol half of objectui#9050's ruling C′ #19514 gave the two filter doors for an empty or non-string$icontainscomparand. It is kept the strong way round: the preview does not evaluate$icontainsat all, so there is no comparand for it to disagree about — every spelling of it refuses.Two structural points, both copied from how this defect class was closed elsewhere:
Mapwhose keys ARE what this face accepts — the shapememory-analytics'MONGO_TO_CUBE_OPERATORtook for the identical defect: "adding a row here is the only way to widen what this face accepts, and forgetting to add one is a loud refusal rather than a wrong number." AMapand not an object literal, so a constraint key naming anObject.prototypemember cannot resolve to an inherited function and be called as a predicate.wheretree is walked once before any row is read, the waydriver-memory'sassertFilterConditionShaperuns ahead of that driver's lowering.⛔ No case was added, deliberately. The default arm is the defect; adding
$icontains,$startsWithand$endsWithwould have left the next unhandled operator in exactly the same state, which is why the table and not a case list is the repair. Growing the arms is separate work with its own ordering already ruled: theFILTER_OPERATORSdocblock's #6520 constraint — a name must not land ahead of its evaluators — reads the same in this direction, so an arm joins the table in the PR that measures it against the shared text/temporal conformance kits. This face is not enrolled inFILTER_TEXT_CASEStoday, and its one shipped text arm ($contains) is itself off that contract (see Acceptance notes).Evidence — both directions
Command, identical in both states:
BEFORE —
preview-evaluator.tsrestored toc1dfa5241bon disk (blob227496afconfirmed on disk againstgit rev-parse BASE:path; marker countsdefault: return true= 1,PREVIEW_FIELD_OPERATORS= 0), the test file and everything else at HEAD:expected [ 'Acme Corp', 'Globex' ] to not include 'Globex'is the card's claim measured:Globexdoes not matchname $icontains 'acme', and the preview charted it anyway.The restore leg was verified by hash, not by an exit code: on-disk blob back to
0e1bf302=HEAD:path,git diff HEADempty.AFTER — same command, tree at HEAD:
The unchanged direction. The 17 tests that pass in BOTH states are the regression guard, and they are meant to: a fail-closed default that starts rejecting rows which used to match correctly is the mirror-image defect. They assert both directions (a matching row still matches, a non-matching row still does not) for every one of the ten evaluated arms, plus the
$ltebare-day rule (#3777), implicit equality,$and,$or,$not, and an absentwhere. Every predicate body is byte-for-byte thecaseit replaces.Checks run locally at
2deda8dab5pnpm --filter @objectstack/service-analytics test— 114 files / 2442 tests passedpnpm --filter @objectstack/service-analytics run typecheck— exit 0pnpm --filter '@objectstack/service-analytics^...' build— exit 0 (dependency closure)node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --ran ...— 60 derived families accounted for: 57 run green, 3 NOT MEASURED (check:dual-build-cjs-loads,check:lean-entry-closure,check:type-check-debteach exit 3, PREREQUISITE NOT MET — they read a whole-workspacepnpm build, which is CI's Build Core job). Among the 57:check:where-matcher(417 matchers, 0 silently-wrong),check:nul-bytes,check:issue-citations,check:doc-authoring,check:empty-changeset,check:test-source-alias,check:undeclared-dep-imports.check-changeset-fixed,check:authz-resolver,check:error-code-casing,check:filter-alias-parity— all exit 0.eslint . --no-inline-config— the whole repo, not a narrowing: 7022 files, 0 errors, 0 warnings, run at this PR's final commit.A changeset is included:
@objectstack/service-analyticsis published and this changes its runtime behaviour.Acceptance notes — found in passing, NOT fixed here
$containsin this same file folds case, and the contract says it must not.matchOp's$containsarm isString(value ?? '').toLowerCase().includes(String(expected ?? '').toLowerCase()).filter-text-conformance.tsrecords that$contains/$notContains/$startsWith/$endsWith"compare CASE-SENSITIVELY" and thatdriver-memorymoved its two folding faces onto the case-exact answer in drivers(memory, mongodb): the$containsfamily still folds case — the last two backends left on the wrong side of #4706 Q2 = A #6682. The same line also coerces a non-string stored value, which the driver-memory's reference matcher answers$notContainsNO for every valued NON-STRING row — the live mingo path answers YES #14079 ruling type-gates. So the preview answers$containsdifferently from every published face — the same preview-vs-publish divergence this card is about, one arm over. ⛔ Deliberately untouched: this PR's second evidence direction is that the ten evaluated arms do not change behaviour.$-key in a NODE position is silently read as a field name.matchesWherehandles$and/$or/$notand falls through everything else to implicit equality, so{ $nor: [...] }comparesrow['$nor']and excludes every row without a word. The published path refuses it (unknownLogicalOperatorError). Fails closed rather than open, so it is not this card's harm — but it is silent.{ name: {} }matches every row. No operator keys, so the inner loop never runs.driver-memoryrefuses this shape (emptyFieldConstraintError,{ field: {} }(零个操作符的字段约束)在同仓有三个答案:driver-sql 组合子内 TRUE、顶层抛 INVALID_FILTER、formula/driver-memory FALSE #5240): "{ status: {} }did not mean 'no rows', it meant 'rows whose status is anything'". This one IS answer-true-shaped, on the same evaluator, and is outside the operator vocabulary this card closes.Generated by Claude Code