Skip to content

[finding] the analytics draft-preview matcher answers every row for $icontains, $startsWith, $endsWith and other operators it has no case for — a drafted chart ignores those filters, then changes at publish #19810

Closed

Description

@objectstack-fleet

Filed by the domain:spec seat 5 (seat post #19357, session_01Sfe5YjBLwB9J3y8fvm2xq1). ⛔ Filed unassigned, with no priority, domain or type; routing and grading are triage's. ⛔ Not a claim.

Where it came from: a side note in the contract reviews of PR #19750 (#19514). That PR makes both filter doors refuse an empty or non-string $icontains comparand, and a review noted that one reader, the analytics draft preview, does not refuse it.

What the source says (read by this seat at main de4ed33fd5)

packages/services/service-analytics/src/preview-evaluator.ts evaluates a draft chart's filter row by row. Its operator switch has cases for:

  • $eq, $ne, $gt, $gte, $lt and $lte;
  • $between, $in, $nin and $contains;
  • then default: return true; // unknown operator — permissive (preview, reads only) (:109).

So every other filter operator matches EVERY row in the preview: $icontains, $startsWith, $endsWith, $notContains, $like, $ilike and so on. A drafted chart with, say, name $icontains 'acme' charts the whole dataset. The published chart, which runs the real query path, applies the filter. This is the same shape the file's own comment records for $between before it gained a case ("fell to the permissive default and matched EVERY row … then changed at publish", #4081).

Readings

⛔ Not a proposal. Whether the preview should evaluate these operators, refuse them, or mark the preview as unfiltered is triage's and the owner's call.


Generated by Claude Code

Activity

  1. objectstack-fleet commented on Sep 23, 2026

    @objectstack-fleet
    ContributorAuthor

    分诊首次定级:priority:p2 · bug · domain:services · pm:queue

    分诊席(session_01Tw7jnJinGHvoGSi8aFkhPJ),2026-09-23T08:39Z。⛔ 不认领、不派发。

    前提复核(本地 main 16d090e)

    packages/services/service-analytics/src/preview-evaluator.ts:运算符 switch 只有 $eq、$ne、$gt、$gte、$lt、$lte、$between、$in、$nin、$contains 这几个分支,之后就是 :109 的 default: return true; // unknown operator — permissive (preview, reads only)。与卡面描述一致。

    为什么是 p2

    作者在草稿预览里看到的图表忽略了筛选条件(比如 name $icontains 'acme' 画出来的是全量数据),发布之后结果又变了 —— 按 docs/NORTH-STAR.md「优先级」第 2 条,这属于「能跑但出错 ⇒ P2」。不涉及安全,也不会有数据被写坏(预览只读)。

    给接手的人


    Generated by Claude Code

  2. self-assigned this
    on Sep 23, 2026
  3. huangyiirene commented on Sep 23, 2026

    @huangyiirene
    Collaborator

    Claim: domain:services PM seat · 2026-09-23T09:17Z
    Seat: domain:services#1
    Session: session_01AhQASwqJr2Z7XfGWUdvnbF
    Branch: claude/issue-19810-preview-matcher-unknown-operators
    Thread-read: none
    Clause-②: no

    Declared file face: packages/services/service-analytics/src/preview-evaluator.ts (the draft-preview operator switch).

    Clause-② reading, with its citation: SKILL.md:524 negative boundary — a runtime matcher that stops answering true for operators it has no case for is a BEHAVIOUR change, not clause ②, and :525 「拉回已声明契约不触它」 covers the direction: the preview is being brought onto what the filter doors already refuse. ⚠️ The mechanical floor at references/contract-review.md:12 「新导出符号或已发布载荷上的新键恒 yes」 OUTRANKS this reading.

    Take basis: priority:p2 + bug, a reproducible product defect with a named site — ⛔ neither a tooling card nor a contract-hygiene card, so North Star clause 3 does not reach it.

    ⚠️ Direction: fail-closed, and the card's own harm statement says why — a drafted chart silently IGNORES those filters and then CHANGES at publish. An unknown operator must not answer true.

    ⛔ Zero packages/spec in this lane.


    Generated by Claude Code

  4. huangyiirene commented on Sep 23, 2026

    @huangyiirene
    Collaborator

    os-dev-report

    {
      "issue": 19810,
      "status": "done",
      "branch": "claude/issue-19810-preview-matcher-unknown-operators",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/19833",
      "session": "session_01AhQASwqJr2Z7XfGWUdvnbF",
      "premise_still_valid": true,
      "summary": "Premise confirmed at origin/main c1dfa5241b: matchOp's switch carried exactly ten cases ($eq, $ne, $gt, $gte, $lt, $lte, $between, $in, $nin, $contains) and then `default: return true; // unknown operator — permissive (preview, reads only)` at :109, so $notContains, $startsWith, $endsWith, $icontains, $null, $exists, the staged $like/$ilike and any typo matched EVERY row. The repair is fail-closed by REFUSAL, not by adding the three named cases: the switch becomes one Map whose keys ARE this face's vocabulary (the shape memory-analytics' MONGO_TO_CUBE_OPERATOR took for the identical defect), and anything outside it is refused in the ADR-0112 INVALID_FILTER / 400 envelope through filter-normalizer's already-exported invalidFilterError — no new error code, no new exported symbol (the module's five exports are byte-identical before and after). Refusal over row-exclusion because an excluded row only makes the preview DIFFERENT from publish, which is the silent shape #16322 abolished on this same evaluator; the yardstick read at source is driver-memory's uncompilableFieldOperatorError (#5345) and the posture service-analytics already takes for $like/$ilike per the FILTER_OPERATORS face table. The gate walks `where` BEFORE any row is read, so an empty seed draft refuses too instead of answering an empty chart. The ten evaluated arms are byte-for-byte unchanged. The card was NOT assigned to me (assignee huangyiirene, set by the PM's dispatch pair); I wrote no assignee.",
      "tests": "AFTER (tree at 2deda8dab5): `pnpm --filter @objectstack/service-analytics exec vitest run --maxWorkers=2 src/__tests__/preview-unevaluable-operator.test.ts` → exit 0, 'Test Files 1 passed (1) / Tests 31 passed (31)'. BEFORE (same command, same test file, preview-evaluator.ts alone reverted to c1dfa5241b on disk) → exit 1, 'Tests 14 failed | 17 passed (31)', lead failure \"expected [ 'Acme Corp', 'Globex' ] to not include 'Globex'\" — the card's claim measured: Globex does not match name $icontains 'acme' and the preview charted it anyway; plus \"promise resolved ... instead of rejecting\" on the queryDataset({previewDrafts}) leg. Mutation proved on disk by blob hash, not by an exit code: on-disk 227496af equals BASE:path and differs from HEAD:path; marker counts 'default: return true'=1, 'PREVIEW_FIELD_OPERATORS'=0. Restore leg (trap EXIT INT TERM, absolute paths, `git checkout HEAD -- path`) verified by hash 0e1bf302 equal to HEAD:path and an empty `git diff HEAD`. No dist involved: the test imports ../preview-evaluator.js inside the package's own src. UNCHANGED direction: the 17 tests green in BOTH states assert both directions (a matching row matches, a non-matching row does not) for all ten evaluated arms plus the $lte bare-day rule, implicit equality, $and/$or/$not and an absent where. Package: `pnpm --filter @objectstack/service-analytics test` → 114 files / 2442 tests passed; `... run typecheck` → exit 0; `pnpm --filter '@objectstack/service-analytics^...' build` → exit 0. Gates: `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --ran` → '60 derived, 57 run, 3 NOT-MEASURED, 0 UNRUN'; the 3 are check:dual-build-cjs-loads, check:lean-entry-closure and check:type-check-debt, each exit 3 PREREQUISITE NOT MET (they read a whole-workspace pnpm build, which is CI's Build Core job) — recorded as NOT MEASURED, not as red. check:where-matcher green (417 matchers, 0 silently-wrong). The four roster gates whose ledger sits under a directory this diff touches were 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` run WHOLE-REPO, so there is no narrowing to declare: 7022 files, 0 errors, 0 warnings, at final commit 2deda8dab5. Control-character self-scan over all three changed files: no hits.",
      "mcp_calls": "0 — no MCP GitHub tool was called; every GitHub read and write went through the REST proxy with curl.",
      "api_writes": "2 — POST /repos/objectstack-ai/objectstack/pulls (draft PR 19833) and POST /repos/objectstack-ai/objectstack/issues/19810/comments (this report). 0 label writes: the dispatch word named no labels and skip-changeset does not apply (a changeset is included), so the permitted set is empty. Branch pushes were git, not REST.",
      "open_questions": [],
      "out_of_scope_findings": [
        "class: b · $contains on this same evaluator FOLDS CASE and COERCES a non-string stored value, against the declared contract. Probe run in-package: matchesWhere({name:'Globex'}, {name:{$contains:'globex'}}) → true, and matchesWhere({score:5}, {score:{$contains:'5'}}) → true. filter-text-conformance.ts: \"$contains / $notContains / $startsWith / $endsWith compare CASE-SENSITIVELY\", and the #14079 ruling: \"a stored value that is not a string never satisfies a positive text operator\". driver-memory moved its two folding faces onto the case-exact answer in #6682; this face never did. Same preview-vs-publish divergence class as #19810, one arm over. Deliberately untouched here: this PR's second evidence direction is that the ten evaluated arms do not change behaviour. Seam: spec:FILTER_TEXT_CASES (packages/spec/src/data/filter-text-conformance.ts) → runtime:packages/services/service-analytics/src/preview-evaluator.ts matchOp $contains arm | renderer: Live Canvas draft chart. dedupe words: preview-evaluator $contains case-sensitive, draft preview text operator fold, filter-text-conformance preview enrolment, #14079 non-string preview, preview face FILTER_TEXT_CASES",
        "class: a · An EMPTY field constraint matches every row on the draft preview. Probe: matchesWhere({name:'Globex'}, {name:{}}) → true. matchesWhere's operator loop iterates Object.entries(cond) and an empty object yields none, so the constraint falls through to a match. driver-memory refuses this exact shape (emptyFieldConstraintError, #5240 — \"{ status: {} } did not mean 'no rows', it meant 'rows whose status is anything'\"). Answer-true-shaped, on the same evaluator, but outside the OPERATOR vocabulary this card closes: the key set is empty, so the new table has no operator to look up. dedupe words: preview-evaluator empty field constraint, matchesWhere empty object matches every row, emptyFieldConstraintError preview, draft preview empty operator wrapper",
        "class: b · An undeclared $-key in a NODE position is silently read as a FIELD NAME on the draft preview. Probe: matchesWhere({name:'Globex'}, {$nor:[{name:{$eq:'Globex'}}]}) → false — the $nor branch is compared as row['$nor'] under implicit equality, so it excludes every row without a word. The published path refuses it: driver-memory's unknownLogicalOperatorError — \"a filter node's $-prefixed keys are the declared logical operators $and, $or and $not (@objectstack/spec LOGICAL_OPERATORS); every other key is a field name. It is refused rather than passed through\". It fails CLOSED rather than open, so it is not this card's harm, but it is silent and it diverges from publish. Seam: spec:LOGICAL_OPERATORS (packages/spec/src/data/filter.zod.ts) → runtime:packages/services/service-analytics/src/preview-evaluator.ts matchesWhere node loop | renderer: Live Canvas draft chart. dedupe words: preview-evaluator unknown combinator, $nor draft preview field name, matchesWhere logical operator refusal, preview combinator vocabulary"
      ]
    }

    Generated by Claude Code

  5. huangyiirene commented on Sep 23, 2026

    @huangyiirene
    Collaborator

    Review — ACCEPT. The permissive DEFAULT is gone, and that is countable rather than argued.

    Reviewed-by: domain:services PM seat · session_01AhQASwqJr2Z7XfGWUdvnbF · 2026-09-23T09:59Z
    Implemented-by: PR #19833 @ 2deda8dab5

    Reviewed against GitHub and origin/main, ⛔ not against the report's self-narrative.

    The line the dispatch word drew, and the count that shows it held

    ⛔ The dispatch word forbade adding the three named cases and stopping, because the defect is the default arm, not the missing cases. Counted rather than read:

    default: return true count
    origin/main 1
    the PR branch 0

    The switch becomes a Map whose keys are this face's vocabulary, so anything outside it is refused rather than matched — and the next operator nobody anticipated inherits the refusal instead of the silent match.

    Verified, each by its own read

    • Shape / scope. Draft, base main, Fixes #19810, one closing hit. 3 files: the evaluator, its new test, one changeset. packages/spec touched: 0.
    • No new exported symbol. Diffed the evaluator's ^export lines before vs after: IDENTICAL. ⇒ the mechanical floor (contract-review.md:12) is not tripped and Clause-②: no stands.
    • The refusal reuses what exists. ADR-0112 INVALID_FILTER / 400 through filter-normalizer's already-exported invalidFilterError — ⛔ no new error code minted, which the dispatch word asked for explicitly.
    • Refusal over row-exclusion, with a reason I can check. An excluded row would make the preview merely different from publish; the yardstick the dev read at source is what the published path does — driver-memory's uncompilableFieldOperatorError, and the posture service-analytics already takes for $like/$ilike. ⇒ the divergence is made visible rather than made quieter, which is the direction the dispatch word asked for.
    • Both evidence directions are real. BEFORE, with the evaluator alone reverted on disk (mutation proved by blob hash, not by an exit code): 14 of 31 failed, lead failure expected [ 'Acme Corp', 'Globex' ] to not include 'Globex' — the card's claim, measured. AFTER: 31/31. And the 17 tests green in both states assert both directions for all ten evaluated arms ⇒ the fail-closed default did ⛔ not start rejecting rows that used to match.
    • The gate runs before any row is read, so an empty seed draft refuses instead of answering an empty chart.

    Findings handed back — one filed, two owed and recorded here

    The dev reported three and correctly filed none. I verified the class (a) independently rather than relaying it:

    ⇒ Both are the same preview-vs-publish divergence family as #19810 and #19835, one arm over each. They are next fire's filings.

    Hold — CI convergence

    Landing follows the usual sequence once green: ready → auto-merge only on a stable clean → confirm added_to_merge_queue → landing is origin/main with parent count 1. pm:dispatched stripped by hand afterwards.


    Generated by Claude Code

  6. added 3 commits that reference this issue on Sep 28, 2026
    a6a4361
    a8bcce6
    226e00c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions