Skip to content

fix(objectql): gate having's $icontains comparand shape (#7158) - #7236

Merged
os-help merged 1 commit into
mainfrom
claude/issue-7158-having-icontains-gate
Aug 10, 2026
Merged

fix(objectql): gate having's $icontains comparand shape (#7158)#7236
os-help merged 1 commit into
mainfrom
claude/issue-7158-having-icontains-gate

Conversation

@os-help

@os-help os-help commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Fixes #7158

having was the sixth JS evaluation face of one filter vocabulary and the only one with no comparand-shape gate. It now refuses an $icontains comparand that is not a non-empty string, with the same gate driver-memory's icontainsComparandError (filter-refusal.ts:670) and driver-sql's word-for-word twin make: both rejections in one constructor, INVALID_FILTER / 400, ADR-0112 envelope.

Premise re-verified on origin/main @ 1bd6525 (post-#7161), by execution

The card's three measured expressions, re-run before the first edit — all three reproduce exactly as filed:

empty scalar   : true          # matchesHaving({ name: 'ACME Corp' }, { name: { $icontains: '' } })
empty applyRows: 9 of 9        # applyHaving(FILTER_TEXT_ROWS, { name: { $icontains: '' } })
nonstr scalar  : false         # matchesHaving({ name: 'ACME Corp' }, { name: { $icontains: 42 } })
nonstr applyRow: 0

The empty-comparand row is the sharp one: every string contains the empty substring, so the predicate constrained nothing and the author got the UNFILTERED aggregate back with no error — a filter that widens rather than narrows (#3948). The non-string row is the quieter half: 42 was answered "no rows" for a shape StringOperatorSchema does not permit.

The change

packages/objectql/src/having-filter.ts

  • icontainsComparandError(field, value, path) — the twin's constructor, message verbatim after the position clause. The position is spelled from the having root (having.$and[0].name.$icontains) where the driver faces spell it from where; the clause is the difference, and a caller reading a 400 needs to know which of the two they mis-wrote. It calls the invalidFilterError helper fix(objectql): put having's operator refusals inside the ADR-0112 envelope (#7047) #7161 established for this face, so the two objectql refusal sites still answer one mistake with one envelope (the finding: rest-server.ts 里三个相邻 /meta handler 的错误信封是三种不同形状,其中两种不符合 ADR-0112 #7035 defect class).
  • The gate sits above the no-value early exit in checkCondition, deliberately. A comparand's shape is a property of the FILTER, not of the row being judged; below the exit, { missing_column: { $icontains: '' } } would be refused only for rows that carry the column and answered false for the rest — one filter, two verdicts, decided by the data.
  • matchesHaving gained an optional third path argument (default 'having') so the refusal can name the position; every existing two-argument call is unaffected.
  • The $icontains arm's typeof target !== 'string' limb is hoisted, not relaxed. It was this face's entire opinion about a bad comparand and its opinion was return false; with the gate above it, it was dead code that read as a check. The guard on the COLUMN VALUE stays — a value that is not text cannot contain a substring, which is a judgement about the row, not about the filter.

packages/objectql/src/having-filter-text-conformance.test.ts — the two rows #7047 could not enrol are enrolled. UNENROLLED_REJECTION_CASES and the namesRetiredOperator partition are gone because the partition existed to name an exclusion and there is none; REJECTION_CASES is now every rejection row the table declares, and the guard test lists all five names. The pins that measured the exclusion are kept, flipped rather than deleted: the same four expressions, now asserted to refuse.

Both directions, as counts

before after
FILTER_TEXT_CASES evaluation rows driven, green 13 13, verbatim unchanged
FILTER_TEXT_CASES rejection rows enrolled 3 of 5 5 of 5
conformance-file tests 20 22

The accepted surface does not shrink: all 13 evaluation rows pass with the same expected id lists, and having-icontains.test.ts's 11 accept cases are untouched (its one non-string case is a non-string COLUMN VALUE with a valid comparand — a different judgement, deliberately left).

Rejection-case assertions

code + status + mustMention, never toThrow() alone — a throw-only assertion carries one bit where the defect has two, and is exactly what stayed green through #7047's missing envelope. One shared expectRefusal helper drives all five rows, so the retired-operator rows and the comparand rows are held to the identical envelope.

Reverse verification

Predicted direction: red, and red only on the two comparand rows. Took the gate out with git checkout origin/main -- packages/objectql/src/having-filter.ts and re-ran the conformance file:

Tests  5 failed | 17 passed (22)

AssertionError: Every row contains the empty substring, so evaluating it is a predicate
  that constrains nothing …: expected undefined to be an instance of Error
AssertionError: Coercing 42 to "42" would answer a query nobody wrote; the declared
  comparand type is string.: expected undefined to be an instance of Error
… (×2 more, the matchesHaving/applyHaving entry points)
AssertionError: expected null to be an instance of Error      ← the position test

The 13 evaluation rows, the 3 retired-operator rows and the table guard all stayed green — the gate is load-bearing for exactly the two rows it claims, and for nothing else.

Consumer sweep

applyHaving / matchesHaving have one non-test caller in the repo, packages/objectql/src/engine.ts (lines 8283 and 8292, the aggregate path); nothing outside packages/objectql imports either. Grepped the repo for tests pinning the tolerated behaviour in any spelling — literal, escaped and cross-line: the only ones were having-filter-text-conformance.test.ts's two pins, and they are flipped rather than deleted per the card.

Tests

pnpm --filter '@objectstack/objectql^...' build          # build closure first
pnpm --filter @objectstack/objectql test     → Test Files 167 passed (167), Tests 2908 passed (2908)
pnpm --filter @objectstack/objectql typecheck → clean
node scripts/check-nul-bytes.mjs             → OK (6615 files, no raw control bytes)

2908 vs 2906 on the pre-change tree, i.e. exactly the +2 the conformance file gains; no test was lost in the restructure.

Changeset: .changeset/silly-jars-shake.md, patch on @objectstack/objectql, stating the client-visible effect plainly — two having filters that used to return rows now return a 400.


Generated by Claude Code

An empty comparand matched EVERY aggregated row — a predicate that
constrains nothing widens the result set rather than narrowing it
(#3948) — and a non-string comparand was answered "no rows" for a
shape `StringOperatorSchema` does not permit. Both are now refused
with INVALID_FILTER / 400 in the ADR-0112 envelope, the same gate
`driver-memory` and `driver-sql`'s `icontainsComparandError` make.

The two FILTER_TEXT_CASES rejection rows #7047 could not enrol are
enrolled: all five rejection rows are driven now, and the pins that
measured the exclusion are kept in their flipped direction.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016R9de1FqP7NvwKvqXi92Gh
@vercel

vercel Bot commented Aug 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 10, 2026 3:27am

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql.

15 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/concepts/metadata-lifecycle.mdx (via @objectstack/objectql)
  • content/docs/data-modeling/formulas.mdx (via packages/objectql)
  • content/docs/deployment/migration-from-objectql.mdx (via @objectstack/objectql)
  • content/docs/deployment/vercel.mdx (via @objectstack/objectql)
  • content/docs/kernel/contracts/data-engine.mdx (via @objectstack/objectql)
  • content/docs/kernel/runtime-services/examples.mdx (via packages/objectql)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/objectql)
  • content/docs/kernel/services.mdx (via @objectstack/objectql)
  • content/docs/permissions/authentication.mdx (via @objectstack/objectql)
  • content/docs/permissions/system-context.mdx (via packages/objectql)
  • content/docs/plugins/index.mdx (via @objectstack/objectql)
  • content/docs/plugins/packages.mdx (via @objectstack/objectql)
  • content/docs/protocol/kernel/index.mdx (via @objectstack/objectql)
  • content/docs/protocol/objectql/query-syntax.mdx (via packages/objectql)
  • content/docs/protocol/objectql/state-machine.mdx (via @objectstack/objectql)

1 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx (via @objectstack/objectql)

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Aug 10, 2026
@os-help
os-help marked this pull request as ready for review August 10, 2026 03:39
@os-help
os-help added this pull request to the merge queue Aug 10, 2026
Merged via the queue into main with commit 1f6ed16 Aug 10, 2026
26 checks passed
@os-help
os-help deleted the claude/issue-7158-having-icontains-gate branch August 10, 2026 04:16
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

Development

Successfully merging this pull request may close these issues.

objectql having has no $icontains comparand-shape gate — an empty comparand matches EVERY row (2 of 5 FILTER_TEXT_CASES rejection rows unenrollable)

2 participants