Skip to content

Commit 402c125

Browse files
hotlongclaude
andauthored
fix(objectql): refuse an uninterpretable temporal filter comparand at the engine door (#8690) (#8808)
* fix(objectql): refuse an uninterpretable temporal filter comparand at 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 * test(analytics): pin the raw-SQL temporal decline; changeset (#8690) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XeQRiAa7vYRVX5Fog7Zby8 * fix(objectql): index the recorded read instead of Array.prototype.at 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 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 60ade58 commit 402c125

9 files changed

Lines changed: 1111 additions & 1 deletion

File tree

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
---
2+
"@objectstack/core": patch
3+
"@objectstack/objectql": patch
4+
"@objectstack/service-analytics": patch
5+
---
6+
7+
fix(objectql): a temporal filter comparand the platform cannot interpret is refused at the engine door instead of answering 200 with zero rows (#8690)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) Nothing authorable is
10+
renamed, retired or tombstoned — no spec schema is touched at all. The change
11+
is a new runtime refusal at the engine's filter collection point, plus the
12+
routing decline that stops the raw-SQL analytics path bypassing it. -->
13+
14+
A `datetime` / `date` / `time` field filtered with a bare string the platform
15+
cannot read — `last_30_days`, `not-a-date-at-all` — was bound **as written**
16+
all the way to the driver, where the comparison is false for every row. The
17+
caller received `HTTP 200`, an empty result set, and nothing to indicate the
18+
filter was meaningless. An unknown `{placeholder}` in the same position was
19+
already refused loudly (`FILTER_TOKEN_UNKNOWN` / 400, listing the resolvable
20+
tokens), so one API answered two shapes of unusable comparand two different
21+
ways.
22+
23+
It is concretely reachable rather than theoretical: `last_7_days` /
24+
`last_30_days` / `last_90_days` are **declared preset names** in the dashboard
25+
schema. The shipped console lowers them to `{N_days_ago}` macros before they
26+
reach the API, so the console path was always safe — but a saved report, an
27+
integration, an MCP client or an AI-authored query sends the preset name itself
28+
and got a silent zero. An empty chart is the hardest failure to debug: it is
29+
indistinguishable from "there is genuinely no data".
30+
31+
Such a comparand is now refused at the ObjectQL engine's single filter
32+
collection point, with `code: 'INVALID_FILTER'` and `status: 400`, naming the
33+
field, the value, the key path and the spellings that would work. That seam is
34+
the one place holding the caller's comparand and the field's **declared type**
35+
at the same moment, and every verb (`find` / `findOne` / `count` / `aggregate`
36+
/ `update` / `delete`) and both filter spellings (the array sugar and the
37+
lowered condition) pass through it, so all four backends inherit one answer
38+
rather than four. `NativeSQLStrategy` additionally **declines** such a query so
39+
the raw-SQL analytics path falls through to that door instead of binding the
40+
value into its own statement.
41+
42+
Deliberately unchanged, each by ruling: a `{placeholder}` keeps its existing
43+
refusal one layer down (the door runs before token resolution and steps around
44+
them, so `{30_days_ago}` still resolves normally); non-string comparands are
45+
untouched (a number is epoch milliseconds, a `Date` is an instant); and the
46+
**empty string** keeps today's behaviour exactly — it binds as `''` and matches
47+
every non-null row, which is a separate question that remains its own card.

‎packages/core/src/index.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,12 @@ export * from './utils/migration-journal.js';
5252
// Export the runtime filter-placeholder resolver (framework#3582)
5353
export * from './utils/filter-tokens.js';
5454

55+
// [#8690] Can a temporal column's storage rule read this comparand? The VALUE
56+
// half of the field-typed judgement behind the engine's temporal-comparand door
57+
// and the analytics raw-SQL decline — one rule, two packages that do not depend
58+
// on each other.
59+
export * from './utils/temporal-comparand.js';
60+
5561
// Export the shared single-record 404 (#4435/#5138, moved down here in #7867) —
5662
// the one `RECORD_NOT_FOUND` envelope `protocol.updateData`/`deleteData`,
5763
// `callData`'s ObjectQL fallback and the engine's own by-id write gate answer
Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#8690] Can a temporal field's storage rule READ this comparand at all?
5+
*
6+
* The one predicate behind the engine's temporal-comparand door
7+
* (`@objectstack/objectql`, `temporal-comparand-door.ts`) and the analytics
8+
* raw-SQL decline (`@objectstack/service-analytics`,
9+
* `NativeSQLStrategy.canHandle`). It lives in `core` because those two packages
10+
* do not depend on each other and a rule that exists twice is a rule that will
11+
* disagree with itself — the shape the 2026-08-12 ruling named by name.
12+
*
13+
* ## The defect it exists to close
14+
*
15+
* A `datetime` field filtered with a bare string the API cannot take literally
16+
* — `last_30_days`, `not-a-date-at-all` — was bound AS-IS all the way to the
17+
* driver, where the comparison is false for every row. Measured end to end on
18+
* `InMemoryDriver` with 51 rows seeded / 38 in-window:
19+
*
20+
* ```
21+
* $gte "last_30_days" HTTP 200 count=0 <- silent zero (the defect)
22+
* $gte "not-a-date-at-all" HTTP 200 count=0 <- silent zero
23+
* $gte "{30_days_ago}" HTTP 200 count=38 <- the positive control
24+
* $gte "{TODAY}" REFUSED FILTER_TOKEN_UNKNOWN / 400
25+
* ```
26+
*
27+
* A `{placeholder}` the resolver does not know is refused loudly; a bare string
28+
* that is not a date was not validated at all. An empty chart is the hardest
29+
* failure to debug — it is indistinguishable from "there is genuinely no data".
30+
*
31+
* ## Why the KIND is an argument, and not something this file works out
32+
*
33+
* "Uninterpretable comparand" is a field-TYPED judgement, and `packages/spec`
34+
* says so outright (`filter.zod.ts`): a filter schema "is field-AGNOSTIC … it
35+
* never sees which column the operator is applied to". This module therefore
36+
* answers only the VALUE half — given a kind, can that kind's storage rule read
37+
* this value — and the caller, which owns object metadata, supplies the kind.
38+
* That split is what lets the rule sit below both consumers without dragging
39+
* field metadata down with it.
40+
*
41+
* ## Interpretability is defined by the DRIVERS' own totals, deliberately
42+
*
43+
* Each predicate below mirrors the total function that would receive the value
44+
* if the door let it through — `storageDatetimeValue` / `storageDateValue` /
45+
* `storageTimeValue` in `driver-memory`, and their `SqlDriver` twins. Those
46+
* functions are total on purpose: an input they cannot interpret is returned
47+
* UNCHANGED rather than becoming an invented instant. So "the driver would
48+
* return it unchanged" IS the definition of uninterpretable, and defining it
49+
* any other way would refuse comparands that work today.
50+
*
51+
* That is why this is not `utcInstantMs` (`@objectstack/spec/data`), which is
52+
* the stricter canonical reader: it rejects a bare epoch-millisecond string and
53+
* every non-ISO spelling `Date.parse` accepts, both of which the drivers read
54+
* correctly today. Refusing those would be a narrowing this card did not ask
55+
* for and no measurement supports.
56+
*
57+
* ## Two things it deliberately does NOT judge
58+
*
59+
* - **Non-string comparands.** A number is epoch milliseconds, a `Date` is an
60+
* instant, `null` is a null test. The refusal scopes to strings by ruling.
61+
* - **The EMPTY string.** Measured, `$gte ""` binds as `''` and every canonical
62+
* UTC text sorts at or above it, so it returns every non-null row — a third
63+
* behaviour again, and one the maintainer ruled stays its own card: "B and C
64+
* scope to non-empty strings and must not decide it in passing". A
65+
* whitespace-only string is the same cell (the drivers `trim()` before they
66+
* test), so it is left alone too.
67+
*/
68+
69+
import { classifyFilterToken } from '@objectstack/spec/data';
70+
71+
/** Which temporal storage rule a declared field takes. */
72+
export type TemporalComparandKind = 'datetime' | 'date' | 'time';
73+
74+
/**
75+
* The kind a declared field's `type` takes, or `null` for every non-temporal
76+
* field.
77+
*
78+
* The same three-way split `driver-memory`'s `indexTemporalFields` and
79+
* `SqlDriver.temporalFieldKind` make, so the door and the drivers cannot
80+
* disagree about which fields are temporal at all.
81+
*/
82+
export function temporalComparandKind(fieldType: unknown): TemporalComparandKind | null {
83+
if (fieldType === 'datetime') return 'datetime';
84+
if (fieldType === 'date') return 'date';
85+
if (fieldType === 'time') return 'time';
86+
return null;
87+
}
88+
89+
/**
90+
* `storageDatetimeValue`'s reading, as a yes/no.
91+
*
92+
* Mirrors it step for step: a bare integer in either sign is epoch
93+
* milliseconds; a bare `YYYY-MM-DD` is midnight UTC; a zone-naive
94+
* `YYYY-MM-DD[ T]HH:MM[:SS[.fff]]` has its wall clock read AS UTC; anything
95+
* else is handed to `Date.parse` exactly as the drivers hand it over.
96+
*/
97+
function readsAsInstant(s: string): boolean {
98+
if (/^-?\d+$/.test(s)) return Number.isFinite(new Date(Number(s)).getTime());
99+
const iso = /^\d{4}-\d{2}-\d{2}$/.test(s)
100+
? `${s}T00:00:00.000Z`
101+
: /^\d{4}-\d{2}-\d{2}[ T]\d{2}:\d{2}(:\d{2}(\.\d+)?)?$/.test(s)
102+
? `${s.replace(' ', 'T')}Z`
103+
: s;
104+
return Number.isFinite(Date.parse(iso));
105+
}
106+
107+
/**
108+
* `storageDateValue`'s reading of a STRING: its leading `YYYY-MM-DD`, and
109+
* nothing else.
110+
*
111+
* Narrower than {@link readsAsInstant} on purpose, because the rule it mirrors
112+
* is narrower: `storageDateValue` collapses a leading calendar day and returns
113+
* every other string untouched. `2026/07/15` is therefore uninterpretable for a
114+
* `date` column even though `Date.parse` reads it — today it survives to the
115+
* driver and compares as text against `YYYY-MM-DD` values, which is the silent
116+
* wrong answer this door exists to stop.
117+
*/
118+
function readsAsCalendarDay(s: string): boolean {
119+
return /^\d{4}-\d{2}-\d{2}/.test(s);
120+
}
121+
122+
/**
123+
* `storageTimeValue`'s reading: a bare wall clock whose components are in
124+
* range. Out-of-range (`25:00`) is uninterpretable — the rule it mirrors
125+
* returns such a value untouched rather than wrapping it.
126+
*/
127+
function readsAsWallClock(s: string): boolean {
128+
const m = /^(\d{2}):(\d{2})(?::(\d{2})(?:\.(\d+))?)?$/.exec(s);
129+
if (!m) return false;
130+
return Number(m[1]) <= 23 && Number(m[2]) <= 59 && Number(m[3] ?? '0') <= 59;
131+
}
132+
133+
/**
134+
* Is `value` a comparand that a `kind` column's storage rule cannot read?
135+
*
136+
* `true` ONLY for a non-empty, non-placeholder STRING that the kind's rule
137+
* would hand back unchanged. Everything else — a number, a `Date`, `null`, a
138+
* `{ $field }` reference, filter structure, the empty string, a `{token}` —
139+
* answers `false`, each for a reason recorded in the module note or below.
140+
*
141+
* A `{placeholder}` is stepped around rather than judged because it is another
142+
* layer's vocabulary and that layer already refuses the unknown ones loudly
143+
* (`FILTER_TOKEN_UNKNOWN` / 400, with the resolvable tokens listed). Both doors
144+
* that call this run BEFORE token resolution, so judging a placeholder here
145+
* would refuse `{30_days_ago}` — the platform's own correct spelling, and the
146+
* positive control this fix is pinned against.
147+
*/
148+
export function isUninterpretableTemporalComparand(
149+
kind: TemporalComparandKind,
150+
value: unknown,
151+
): boolean {
152+
if (typeof value !== 'string') return false;
153+
const s = value.trim();
154+
// The empty-string cell is its own card — see the module note.
155+
if (s === '') return false;
156+
// Another layer's vocabulary, and it has its own loud refusal.
157+
if (classifyFilterToken(value) !== null) return false;
158+
if (kind === 'datetime') return !readsAsInstant(s);
159+
if (kind === 'date') return !readsAsCalendarDay(s);
160+
return !(readsAsWallClock(s) || readsAsInstant(s));
161+
}

0 commit comments

Comments
 (0)