Skip to content

fix(service-analytics): the draft preview refuses a filter operator it cannot evaluate instead of answering every row - #19833

Merged
huangyiirene merged 2 commits into
mainfrom
claude/issue-19810-preview-matcher-unknown-operators
Sep 23, 2026
Merged

huangyiirene merged 2 commits into
mainfrom
claude/issue-19810-preview-matcher-unknown-operators

Conversation

@huangyiirene

Copy link
Copy Markdown
Collaborator

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 any where operator 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/main c1dfa5241b, matchOp's switch carried exactly TEN cases and one default arm:

Evaluated (10) $eq, $ne, $gt, $gte, $lt, $lte, $between, $in, $nin, $contains
Default arm (:109) default: return true; // unknown operator — permissive (preview, reads only)
Answered for EVERY row $notContains, $startsWith, $endsWith, $icontains, $null, $exists (the rest of FILTER_OPERATORS), the staged $like / $ilike, and any typo

The card's premise holds exactly as filed. The combinators $and / $or / $not were and remain handled by matchesWhere itself.

The repair, and which contract it matches

Fail-closed by REFUSING — INVALID_FILTER / 400, through filter-normalizer.ts's already-exported invalidFilterError. 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:

The yardstick is what the real filter doors do, read rather than invented:

Two structural points, both copied from how this defect class was closed elsewhere:

  1. The vocabulary and the evaluator are ONE table. The switch becomes a Map whose keys ARE what this face accepts — the shape memory-analytics' MONGO_TO_CUBE_OPERATOR took 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." A Map and not an object literal, so a constraint key naming an Object.prototype member cannot resolve to an inherited function and be called as a predicate.
  2. The gate is row-independent. A per-row refusal only fires if some row reaches it, so a pending seed draft holding ZERO rows — the state a draft is authored in — would have answered an empty chart for a filter it cannot evaluate. The where tree is walked once before any row is read, the way driver-memory's assertFilterConditionShape runs ahead of that driver's lowering.

⛔ No case was added, deliberately. The default arm is the defect; adding $icontains, $startsWith and $endsWith would 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: the FILTER_OPERATORS docblock'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 in FILTER_TEXT_CASES today, and its one shipped text arm ($contains) is itself off that contract (see Acceptance notes).

Evidence — both directions

Command, identical in both states:

pnpm --filter @objectstack/service-analytics exec vitest run --maxWorkers=2 \
  src/__tests__/preview-unevaluable-operator.test.ts

BEFORE — preview-evaluator.ts restored to c1dfa5241b on disk (blob 227496af confirmed on disk against git rev-parse BASE:path; marker counts default: return true = 1, PREVIEW_FIELD_OPERATORS = 0), the test file and everything else at HEAD:

 Test Files  1 failed (1)
      Tests  14 failed | 17 passed (31)

 FAIL  ... > does NOT answer the row that `name $icontains "acme"` excludes
AssertionError: expected [ 'Acme Corp', 'Globex' ] to not include 'Globex'
 FAIL  ... > refuses it in the ADR-0112 `INVALID_FILTER` / 400 envelope
AssertionError: expected undefined to be an instance of Error
 FAIL  ... > refuses over an EMPTY seed draft too — the walk is not a function of the data
 FAIL  ... > refuses inside `$or`, `$and` and `$not` arms
 FAIL  ... > refuses a constraint key that names an Object.prototype member
 FAIL  ... > $notContains / $startsWith / $endsWith / $icontains / $null / $exists / $like / $ilike
            is refused, never answered for every row   (8 rows)
 FAIL  ... > refuses the drafted selection instead of charting every seed row
AssertionError: promise resolved "{ rows: [ { …(2) }, { …(2) } ], …(1) }" instead of rejecting

expected [ 'Acme Corp', 'Globex' ] to not include 'Globex' is the card's claim measured: Globex does not match name $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 HEAD empty.

AFTER — same command, tree at HEAD:

 Test Files  1 passed (1)
      Tests  31 passed (31)

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 $lte bare-day rule (#3777), implicit equality, $and, $or, $not, and an absent where. Every predicate body is byte-for-byte the case it replaces.

Checks run locally at 2deda8dab5

  • pnpm --filter @objectstack/service-analytics test — 114 files / 2442 tests passed
  • pnpm --filter @objectstack/service-analytics run typecheck — exit 0
  • pnpm --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-debt each exit 3, PREREQUISITE NOT MET — they read a whole-workspace pnpm 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.
  • The four roster gates whose ledger sits under a directory this diff touches, read and run rather than assumed silent: 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.
  • Control-character self-scan over all three changed files: no hits.

A changeset is included: @objectstack/service-analytics is published and this changes its runtime behaviour.

Acceptance notes — found in passing, NOT fixed here

  1. $contains in this same file folds case, and the contract says it must not. matchOp's $contains arm is String(value ?? '').toLowerCase().includes(String(expected ?? '').toLowerCase()). filter-text-conformance.ts records that $contains / $notContains / $startsWith / $endsWith "compare CASE-SENSITIVELY" and that driver-memory moved its two folding faces onto the case-exact answer in drivers(memory, mongodb): the $contains family 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 $notContains NO for every valued NON-STRING row — the live mingo path answers YES #14079 ruling type-gates. So the preview answers $contains differently 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.
  2. An undeclared $-key in a NODE position is silently read as a field name. matchesWhere handles $and / $or / $not and falls through everything else to implicit equality, so { $nor: [...] } compares row['$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.
  3. An empty field constraint { name: {} } matches every row. No operator keys, so the inner loop never runs. driver-memory refuses 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

…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>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 23, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

7 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
  • 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 9 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 1cacfe4a425dc0c2cdd4eb5d0088d5b3f5a16c92 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 198b4aee6e5bec9849cdde9a2d6768bba2e8ea5f — the merge of head 2deda8dab5b44d9056035ae9c24a8f1188122ee3 into base 1cacfe4a425dc0c2cdd4eb5d0088d5b3f5a16c92, 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 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

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Contract review (retroactive — PR landed as a6a4361 before this record was written)

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 2deda8dab5b44d9056035ae9c24a8f1188122ee3

⚠️ Why this record is late: the domain:services seat enqueued this PR without the served-tier contract review that .claude/skills/pm-dispatch/references/contract-review.md:8 owes to .changeset prose and :25 requires on file before landing. That lapse is the seat's own. The review below was run afterwards by an isolated reviewer at the served tier. It saw only the card, its comments, and the landed diff, which is identical to the reviewed head. The seat adopts the verdict.

① Derived judgments

The changeset prose was checked claim by claim. Every factual claim is TRUE:

  • The switch before the fix: the parent matchOp had ten arms plus default: return true at parent :109.
  • The operators it answered as "every row": the six FILTER_OPERATORS members were filter.zod.ts:2739-2751, and the staged $like/$ilike were :2710-2718.
  • The new refusal: it reuses the INVALID_FILTER/400 envelope through the exported invalidFilterError (filter-normalizer.ts:403-408).
  • No new export: the five export lines are the same before and after.
  • Zero-row seeds refuse too: the pre-row gate (:206-221 called at :551) catches them.
  • The ten evaluated arms are unchanged, byte for byte.

Clause-②: no is RIGHT. The change is 「拉回已声明契约」 (references/execution-duties.md:99): no new export, no new key, no widening.

② Semver level

patch on @objectstack/service-analytics is consistent (AGENTS.md:1081-1085). It is not breaking.

③ Boundary flags

Zero packages/spec, zero content/docs/**, zero CHANGELOG, zero governed surface. The scope matches the card. All 36 head check-runs finished: 33 success, 3 roster skips.

Implemented-by: claude/issue-19810-preview-matcher-unknown-operators
Reviewed-by: session_01AhQASwqJr2Z7XfGWUdvnbF

VERDICT: PASS

Non-blocking defects:

  1. Attribution overreach in the changeset prose. It cites the JS 求值面全体拒收 $icontains(driver-memory 两面 / driver-mongodb / objectql having / formula)—— SQL 族已实现,同一 filter 在内存 double 上抛错 #6520 FILTER_OPERATORS ruling (filter.zod.ts:2694-2696) for a scope decision that ruling does not make. The ruling concerns the order in which names land.
  2. Imprecision in the changeset prose. 「the published chart applied the filter」 does not hold for $like/$ilike. The published face refuses those two (filter-normalizer.ts:418-435, :1041), so before the fix the divergence for them was "every row" versus "refuse".
  3. A stale sibling comment at analytics-service.ts:2330-2337 still argues from "would newly refuse them on the draft-preview path".
  4. The preview face is not enrolled in filter-text-conformance.ts. Its $contains arm case-folds. This one is already known and owed.

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