Skip to content

fix(objectql): refuse an uninterpretable temporal filter comparand at the engine door (#8690) - #8808

Merged
hotlong merged 5 commits into
mainfrom
claude/issue-8690-temporal-comparand-refusal
Aug 15, 2026
Merged

fix(objectql): refuse an uninterpretable temporal filter comparand at the engine door (#8690)#8808
hotlong merged 5 commits into
mainfrom
claude/issue-8690-temporal-comparand-refusal

Conversation

@hotlong

@hotlonghotlong commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Part of #8690 — the B half only. Merging this does not close the card: the C half (refuse the declared preset vocabulary at publish time, packages/spec + @objectstack/lint, claude-fable-5 tier by the ruling's own last line) is carved out of this dispatch and tracked on #8793. #8690 remains open after this lands.

Implements the maintainer ruling of 2026-08-15 (delegated adjudication), option B, explicitly not A.

The defect

A datetime field filtered with a bare string the API cannot take literally was bound as written all the way to the driver, where the comparison is false for every row — HTTP 200, empty result set, no diagnostic. An unknown {placeholder} in the same position was already refused loudly, so one API answered two shapes of unusable comparand two different ways.

Reachable rather than theoretical: last_7_days / last_30_days / last_90_days are declared preset names in the dashboard schema. The console lowers them to {N_days_ago} macros, so the console path was always safe — a saved report, an integration, an MCP client or an AI-authored query sends the preset name itself.

Where the refusal lands, and why

lowerWhereFilterArray is the engine's single filter collection point and the one seam holding the caller's comparand and the field's declared type at the same moment. It is reached by every verb (find / findOne / count / aggregate / update / delete) through both spellings (the array sugar and the already-lowered condition the protocol face hands over), so all four backends inherit one answer.

The two seams triage originally named were measured and cannot host it: packages/core's token resolver is field-agnostic by construction, and packages/rest binds no comparands at all. The driver layer holds both facts but is four frozen packages whose pass-through is a deliberate, counter-pinned contract shared with the write path — rejected by name.

PM mechanism assumption: confirmed by measurement.lowerWhereFilterArray(object, operation, bag, schema) is already handed this._registry.getObject(object) at all six call sites, and its existing neighbour assertFilterIsMaterializable already reads schema.fields[name].type. Nothing new had to be threaded.

packagechange
@objectstack/coreisUninterpretableTemporalComparand(kind, value) — the VALUE half, shared so the rule cannot exist twice in two packages that do not depend on each other. Interpretability is defined by the drivers' own total functions, so the door refuses exactly what a driver would hand back unchanged.
@objectstack/objectqlthe door: INVALID_FILTER / 400, naming field, declared kind, value, key path and the spellings that work.
@objectstack/service-analyticsNativeSQLStrategy.canHandledeclines an uninterpretable temporal comparand so raw-SQL paths fall through to the door instead of binding it into their own statement.

Deviation to flag, deliberately not silent

The ruling says the analytics decline should arrive "via a new StrategyContext hook". Measured, StrategyContext is declared in packages/spec (contracts/analytics-service.ts) — which this dispatch forbids, and which is where the carved-out C half lands. Rather than edit spec or declare an undeclared hook on a shared contract, the decline classifies on metadata StrategyContextalready carries: a cube dimension declares type: 'time', resolved through the same lookupMember every other member lookup in the strategy uses. Same shape as the 2026-08-12 Q1=B ruling one seam over, which rejected new StrategyContext hooks for exactly this decision. Cost, recorded in the code: a temporal column filtered without being a declared time dimension is not classified, so it keeps today's behaviour on the raw-SQL path — strictly smaller than "every raw-SQL query bypasses the door", and it fails in the safe direction, since a missed decline degrades to today's behaviour rather than a new wrong answer. Reviewed and accepted; the residual precision is carried onto #8793, which is already opening packages/spec.

Scope boundaries, each by ruling

  • Non-empty strings only. The empty-string cell stays its own card and is pinned unchanged (measured: it binds as '' and returns 51 of 51 — the card table's 38 is a transcription error its own prose corrects).
  • {placeholder} strings are stepped around. The door runs before token resolution because the refusal must precede the driver, so judging one would refuse {30_days_ago}. Unknown tokens keep FILTER_TOKEN_UNKNOWN / 400.
  • Non-string comparands untouched — a number is epoch milliseconds, a Date is an instant.
  • ⛔ No packages/drivers change. ⛔ No packages/rest consumer-side patch. ⛔ No packages/spec edit.

Verification

Union re-run after the final commit, at fd917ec42 (main merged in), clean tree.

  • @objectstack/objectql — 209 files, 3664 tests passed; typecheck (tsc --noEmit) clean.
  • @objectstack/core — 34 files, 831 passed. @objectstack/service-analytics — 77 files, 1722 passed (includes [finding] service-analytics carries its own copies of the comparand-type allow-list the #7872 door now single-sources — reconcile membership and message wording to the door #8186's comparand-door-single-source suite, which arrived on main and touches the same file this PR extends).
  • Refusal pin + positive control in one test, as required: the four preset comparands each assert code === 'INVALID_FILTER'andstatus === 400 with zero driver reads, and the same it() asserts {30_days_ago} still returns 38 rows on the card's 51-row / 38-in-window dataset shape, with the resolved floor read back off the driver AST.
  • Raw-SQL bypass pinned: the decline routes four uninterpretable comparands away from executeRawSql, and an over-decline control proves 2026-07-15, an ISO instant and {30_days_ago} all keep the P1 fast path.
  • Reverse verification, direction predicted before running: with the two door call sites ablated, 5 of 8 red, 3 green — every refusal cell lost its refusal (expected null not to be null, i.e. the silent zero returned), while the three controls (token resolver, scoped-out cells, registry-less no-verdict) stayed green because none of them is the door's doing. Restored with git checkout HEAD -- packages/objectql/src/engine.ts from the commit that already carried the fix.
  • Gates at this head: check:nul-bytes, check:error-code-casing, check:durability-log-level, check:kernel-hook-pairs, check:stack-collection-maps, check:test-source-alias, check:type-source-resolution, check:query-options-erasure, check:type-check-coverage, check:changeset-gate-self-tests, check:objectui-changeset, plus check-adr-0087-registration, check-changeset-no-major, check-empty-changeset, check-engine-split-ratio — all PASS.

check:type-check-debt — the red this PR was kicked for, and the fix

The first push went red on @objectstack/objectql's TEST_DEBT: recorded 355, measured 356. The +1 was reads.at(-1) in the new pin — TS2550, because this package's lib target predates Array.prototype.at. It was invisible to pnpm --filter @objectstack/objectql typecheck because objectql's tsconfig.json excludes **/*.test.ts, so tsc --noEmit never compiled the file; only the TEST_DEBT re-measure, which drops that exclusion, sees it.

Fixed at the source — indexed access, same assertion, no @ts-expect-error, no skipped case, ledger not raised.

All three implicated entries re-measured at fd917ec42, each with the same project shape the gate constructs (fidelity confirmed: this replication reproduced CI's 356 exactly before the fix):

ledger entryrecordedmeasuredfrom this PR's files
@objectstack/objectql (TEST_DEBT)3553550
@objectstack/core (DEBT)98980
@objectstack/service-analytics (DEBT)10100

Both new test files were confirmed inside the programs those numbers are measured from (tsc --listFiles), so the zeros are real coverage rather than a file nothing read. The full --re-measure sweep runs every ledger entry sequentially and exceeds one call window locally; CI runs it whole.


Generated by Claude Code

… the engine door (#8690)
A bare string a temporal field cannot interpret — `last_30_days`,
`not-a-date-at-all` — was bound as written, compared false for every row,
and answered 200 with an empty result and no diagnostic, while an unknown
`{placeholder}` was refused loudly one branch over.
Refuse it at the ObjectQL engine's single filter collection point, per the
maintainer ruling of 2026-08-15 (option B): `lowerWhereFilterArray` is the
one seam holding the caller's comparand and the field's declared type at the
same moment, on every verb and through both doors.
- `@objectstack/core`: the value-half predicate, shared so the rule cannot
exist twice; interpretability is defined by the drivers' own totals.
- `@objectstack/objectql`: the door, `INVALID_FILTER` / 400.
- `@objectstack/service-analytics`: `NativeSQLStrategy.canHandle` declines an
uninterpretable temporal comparand so raw-SQL paths fall through to the
door instead of binding it directly.
Scoped to non-empty strings by ruling: the empty-string cell stays its own
card, and `{placeholder}` strings keep their existing loud refusal.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XeQRiAa7vYRVX5Fog7Zby8
@vercel

vercelBot commented Aug 15, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 15, 2026 4:03am

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/core, @objectstack/objectql, @objectstack/service-analytics.

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

  • content/docs/ai/actions-as-tools.mdx(via @objectstack/core)
  • content/docs/ai/knowledge-rag.mdx(via @objectstack/core)
  • content/docs/ai/natural-language-queries.mdx(via @objectstack/core)
  • content/docs/api/data-api.mdx(via @objectstack/service-analytics)
  • content/docs/api/index.mdx(via @objectstack/service-analytics)
  • content/docs/automation/webhooks.mdx(via @objectstack/core)
  • content/docs/concepts/metadata-lifecycle.mdx(via @objectstack/objectql)
  • content/docs/concepts/north-star.mdx(via packages/core)
  • content/docs/data-modeling/formulas.mdx(via packages/objectql)
  • content/docs/deployment/migration-from-objectql.mdx(via @objectstack/core, @objectstack/objectql)
  • content/docs/deployment/vercel.mdx(via @objectstack/objectql)
  • content/docs/kernel/contracts/data-engine.mdx(via @objectstack/objectql)
  • content/docs/kernel/contracts/index.mdx(via @objectstack/core)
  • content/docs/kernel/runtime-services/examples.mdx(via @objectstack/core, packages/objectql)
  • content/docs/kernel/services-checklist.mdx(via @objectstack/core, @objectstack/objectql, @objectstack/service-analytics)
  • content/docs/kernel/services.mdx(via @objectstack/core, @objectstack/objectql)
  • content/docs/permissions/authentication.mdx(via @objectstack/core, @objectstack/objectql)
  • content/docs/permissions/authorization.mdx(via packages/core)
  • content/docs/permissions/sharing-rules.mdx(via @objectstack/service-analytics)
  • content/docs/permissions/system-context.mdx(via packages/objectql)
  • content/docs/plugins/anatomy.mdx(via @objectstack/core)
  • content/docs/plugins/development.mdx(via @objectstack/core)
  • content/docs/plugins/index.mdx(via @objectstack/core, @objectstack/objectql)
  • content/docs/plugins/packages.mdx(via @objectstack/core, @objectstack/objectql, @objectstack/service-analytics)
  • content/docs/protocol/kernel/index.mdx(via @objectstack/core, @objectstack/objectql)
  • content/docs/protocol/kernel/lifecycle.mdx(via @objectstack/core)
  • content/docs/protocol/kernel/plugin-spec.mdx(via @objectstack/core)
  • content/docs/protocol/objectql/query-syntax.mdx(via packages/objectql)
  • content/docs/protocol/objectql/state-machine.mdx(via @objectstack/objectql)

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

  • content/docs/releases/implementation-status.mdx(via @objectstack/core, @objectstack/objectql, @objectstack/service-analytics)
  • content/docs/releases/v12.mdx(via @objectstack/core)
  • content/docs/releases/v15.mdx(via @objectstack/core)
  • content/docs/releases/v17.mdx(via @objectstack/core, @objectstack/service-analytics)
  • content/docs/releases/v9.mdx(via @objectstack/service-analytics)

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.

…in the temporal-door pin (#8690)
`reads.at(-1)` is TS2550 under this package's lib target, and objectql's
tsconfig hides `**/*.test.ts` from its own `typecheck` script — so the error
was invisible to `tsc --noEmit` and surfaced only in the TEST_DEBT re-measure,
pushing the shrink-only ledger 355 -> 356.
Indexed access instead. Same assertion, same read, ledger back at 355.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XeQRiAa7vYRVX5Fog7Zby8
@hotlong
hotlong marked this pull request as ready for review August 15, 2026 04:20
@hotlong
hotlong added this pull request to the merge queueAug 15, 2026
Merged via the queue into main with commit 402c125Aug 15, 2026
29 checks passed
@hotlong
hotlong deleted the claude/issue-8690-temporal-comparand-refusal branch August 15, 2026 04:34
os-project-manager pushed a commit that referenced this pull request Aug 15, 2026
…he +2 TEST_DEBT drift (#8793)
Same class as #8808's TS2550: invisible to 'pnpm --filter @objectstack/lint
typecheck' because tsconfig excludes **/*.test.ts; only the TEST_DEBT
re-measure compiles the file. The two TS7006s were downstream of the one
unresolved import (TS2835). Ledger not raised; re-measured 19 vs recorded 20
on the fully built workspace closure.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fgvh1iEJfxetei7aNVdtJt
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/xlteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@hotlong@claude