Skip to content

fix(analytics): refuse a zero-operator field constraint on the draft preview - #19896

Merged
huangyiirene merged 1 commit into
mainfrom
claude/issue-19835-preview-empty-field-constraint
Sep 23, 2026
Merged

huangyiirene merged 1 commit into
mainfrom
claude/issue-19835-preview-empty-field-constraint

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

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

…preview

`matchesWhere` iterated a field constraint's entries; `{ name: {} }` has
none, so the row fell through to a match and the draft preview charted
every row for a filter all three shipped drivers refuse. Refuse it in the
INVALID_FILTER / 400 envelope the preview already speaks, from the
row-independent gate (whole tree, nested combinators included) and from
matchesWhere's field arm.

Claude-Session: https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

4 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 dabf8d795ee9279c18b4b00bfabb322544106b2f → packageMentionDocs.

Which tree this was computed on

This run read content/docs from eb6f2dc0822af699288a76a99b14f2381f106229 — the merge of head 3adb37d4276dcc3fd3b41149df851b1b3b632fdc into base dabf8d795ee9279c18b4b00bfabb322544106b2f, 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 eb6f2dc0822af699288a76a99b14f2381f106229 && git checkout eb6f2dc0822af699288a76a99b14f2381f106229
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin dabf8d795ee9279c18b4b00bfabb322544106b2f 3adb37d4276dcc3fd3b41149df851b1b3b632fdc && git checkout -B drift-repro dabf8d795ee9279c18b4b00bfabb322544106b2f && git merge --no-ff 3adb37d4276dcc3fd3b41149df851b1b3b632fdc

node scripts/docs-audit/affected-docs.mjs --json dabf8d795ee9279c18b4b00bfabb322544106b2f

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 3adb37d4276dcc3fd3b41149df851b1b3b632fdc

An isolated reviewer ran at the served tier. It saw only the card #19835, the triage direction 5793123019, and this PR. The domain:services seat adopts its verdict. All 34 head check-runs completed; the 3 skips are on the roster.

① Derived judgments

11 claims checked. All RIGHT.

  • All three shipped drivers refuse { field: {} }:
    • driver-memory/src/filter-refusal.ts:870
    • driver-mongodb/src/mongodb-filter.ts:262
    • driver-sql/src/sql-driver.ts:3987, :15025-15026
  • "Every data driver" also holds. sqlite-wasm and turso-local inherit the SqlDriver refusal, and turso remote refuses at remote-transport.ts:3136-3138.
  • The live analytics door refuses it too: filter-normalizer.ts:911-926.
  • The detector matches the drivers exactly: plain object, zero own keys, Date/RegExp excluded (preview-evaluator.ts:210-215 vs filter-refusal.ts:240-255).
  • Nesting cannot bypass the refusal. The row-independent gate (:282-300, called at :632 before rows.filter) recurses through $and/$or/$not. It catches the $or short-circuit arm and a zero-row seed.
  • Nothing is read as "matches zero rows": both sites throw.
  • No legal shape is newly refused: where: {}, $and: [{}] and $not: {} are unchanged.
  • Narrowing goes only toward what every published path already refuses. No new export, no new payload key, no new error code: INVALID_FILTER already exists and both helpers are module-private.

Non-blocking:

  • A direct matchesWhere refusal names the bare field instead of a full path. The shipped path reports the full path.
  • Two pre-existing items are out of this card. An unknown $-prefixed node key is read as a field. A Date in implicit-equality position matches every row; this is already in the PR acceptance notes and can't be reached over JSON.

② Semver level

patch for @objectstack/service-analytics with Clause-②: no is consistent with AGENTS.md:1081-1083. The change is a bug fix that brings a face back onto the declared contract. The mechanical floor is not tripped. It is not breaking, so no ADR-0087 marker is owed.

③ Boundary flags

Zero packages/spec, zero content/docs/releases/**, zero CHANGELOG edits. The scope matches the card and the claim's declared file face.

Implemented-by: claude/issue-19835-preview-empty-field-constraint
Reviewed-by: session_01AhQASwqJr2Z7XfGWUdvnbF

VERDICT: PASS


Generated by Claude Code

@huangyiirene
huangyiirene marked this pull request as ready for review September 23, 2026 16:07
@huangyiirene
huangyiirene added this pull request to the merge queue Sep 23, 2026
Merged via the queue into main with commit 44ce049 Sep 23, 2026
36 checks passed
@huangyiirene
huangyiirene deleted the claude/issue-19835-preview-empty-field-constraint branch September 23, 2026 16:33
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