Skip to content

Commit 35accbf

Browse files
authored
feat(spec,ci): temporal hooks onto the IDataDriver contract; conformance job with live non-UTC servers (#3979)
Closes the two debts the datetime storage work (#3912/#3942/#3954) left open in ADR-0053. D-A2 — contract promotion. temporalFilterValue and temporalFilterColumnSql were duck-typed: analytics probed `typeof driver.x === 'function'` against a locally-invented interface, and nothing at the type level said a driver must implement both or neither. #3912's lesson is precisely that coercing the comparand without normalising the column reintroduces half the bug, so a driver implementing one hook alone would silently regress. Both are now optional members of IDataDriver, documented as a PAIR with "absent = identity" semantics for drivers whose storage form is the wire form. SqlDriver `implements IDataDriver`, so its signatures are compile-checked from here on; analytics Picks the contract instead of inventing a local shape, keeping the runtime typeof guards as the correct way to consume an optional member. coerceFilterValueForSql already sits behind the hook as the last-resort boolean/number recovery — the demotion D-A2 asked for — and stays in exactly that role. D-A3 — the conformance backstop. The live-server suites from #3912/#3942 are opt-in (OS_TEST_POSTGRES_URL / OS_TEST_MYSQL_URL) and skip without a server, so nothing ran them in CI and the seam could regress silently. New `temporal-conformance` job: postgres:16 and mysql:8.0 service containers, servers pointed at +08:00 post-start (services cannot override the image command), the Node process at America/New_York, and assertions in UTC — the three-way skew that caught every bug in this family, with both suites asserting a non-UTC server so a mis-provisioned service fails loudly rather than passing vacuously. The WHOLE driver-sql suite runs under the skewed zone, so a TZ-sensitive assumption anywhere in the driver's tests fails there before it ships. The job's first run on its own PR confirmed it is not vacuous: the MySQL suite ran 15 tests against MySQL 8.0.46 and the Postgres suite 5 against PG 16.14 with `timezone changed to "Asia/Shanghai"` in the server log — 47 files, 480/480, zero skips. That also completes the #3942 compatibility claim, which had only been verified by hand on MariaDB 10.11.
1 parent ad303ed commit 35accbf

5 files changed

Lines changed: 223 additions & 26 deletions

File tree

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/service-analytics": patch
4+
---
5+
6+
feat(spec): promote the temporal storage hooks onto the IDataDriver contract (ADR-0053 D-A2)
7+
8+
`temporalFilterValue` and `temporalFilterColumnSql` — the pair that closed
9+
#3912's storage-form drift — were duck-typed: analytics probed
10+
`typeof driver.x === 'function'` against a locally-invented interface, and
11+
nothing at the type level said a driver must implement both or neither. The
12+
lesson of #3912 is precisely that coercing the comparand without normalising
13+
the column reintroduces half the bug, so a driver implementing one hook alone
14+
would silently regress.
15+
16+
Both are now optional members of `IDataDriver`
17+
(`@objectstack/spec/contracts`), documented as a pair with "absent = identity"
18+
semantics for drivers whose storage form is the wire form (memory, mongo).
19+
`SqlDriver implements IDataDriver`, so its signatures are compile-checked from
20+
here on; analytics derives its driver seam by `Pick`-ing the contract instead
21+
of a local duck type. Runtime `typeof` guards remain — that is the correct way
22+
to consume an optional contract member — but the shape they guard now has one
23+
authoritative definition.
24+
25+
No runtime behaviour change. ADR-0053 D-A2 is recorded as resolved.

.github/workflows/ci.yml

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,126 @@ jobs:
196196
path: .turbo/cache
197197
key: ${{ runner.os }}-turbo-${{ github.job }}-${{ github.ref_name }}-${{ github.sha }}
198198

199+
200+
# ── Temporal conformance against live, non-UTC servers (ADR-0053 D-A3) ─────
201+
#
202+
# The datetime storage work (#3912/#3942) was verified against real servers
203+
# because every one of its bugs was invisible on all-UTC infrastructure: a
204+
# zone-naive write resolved in the SERVER's zone on Postgres, mysql2 rendered
205+
# a Date in the HOST's zone, and a bare YYYY-MM-DD comparand meant a
206+
# different midnight per dialect. The committed suites are opt-in
207+
# (OS_TEST_POSTGRES_URL / OS_TEST_MYSQL_URL) and skip without a server, so
208+
# without this job they would never run in CI and the seam could regress
209+
# silently — D-A3's exact concern.
210+
#
211+
# Every timezone here is deliberately DIFFERENT: servers at +08:00, the Node
212+
# process at America/New_York, assertions in UTC. Both suites assert they
213+
# are pointed at a non-UTC server, so a mis-provisioned service fails loudly
214+
# instead of letting the job pass vacuously.
215+
temporal-conformance:
216+
name: Temporal Conformance (live PG + MySQL)
217+
needs: filter
218+
if: needs.filter.outputs.core == 'true'
219+
runs-on: ubuntu-latest
220+
permissions:
221+
contents: read
222+
223+
services:
224+
postgres:
225+
image: postgres:16
226+
env:
227+
POSTGRES_PASSWORD: postgres
228+
ports:
229+
- 5432:5432
230+
options: >-
231+
--health-cmd="pg_isready -U postgres"
232+
--health-interval=5s
233+
--health-timeout=5s
234+
--health-retries=12
235+
mysql:
236+
# Real MySQL 8.0. The hands-on verification of #3942 ran on MariaDB
237+
# 10.11 — the stricter dialect for datetime literals — so this job is
238+
# the other half of the compatibility claim. `-h 127.0.0.1` forces the
239+
# ping over TCP: the image's init phase runs mysqld with networking
240+
# disabled, so a socket ping would report healthy before init finishes.
241+
image: mysql:8.0
242+
env:
243+
MYSQL_ROOT_PASSWORD: root
244+
MYSQL_DATABASE: conformance
245+
ports:
246+
- 3306:3306
247+
options: >-
248+
--health-cmd="mysqladmin ping -h 127.0.0.1 -uroot -proot"
249+
--health-interval=5s
250+
--health-timeout=5s
251+
--health-retries=24
252+
253+
steps:
254+
- name: Checkout repository
255+
uses: actions/checkout@v7
256+
257+
# Service containers cannot override the image command, so the non-UTC
258+
# zones are set post-start through each server's own mechanism. Echoed
259+
# back so a provisioning failure is visible in the log — though the
260+
# suites' own non-UTC guards are the real gate.
261+
- name: Point both servers at a non-UTC timezone
262+
run: |
263+
docker exec ${{ job.services.postgres.id }} psql -U postgres -c "ALTER SYSTEM SET timezone='Asia/Shanghai'"
264+
docker exec ${{ job.services.postgres.id }} psql -U postgres -c "SELECT pg_reload_conf()"
265+
docker exec ${{ job.services.postgres.id }} psql -U postgres -tAc "SHOW timezone"
266+
docker exec ${{ job.services.mysql.id }} mysql -uroot -proot -e "SET GLOBAL time_zone = '+08:00'"
267+
docker exec ${{ job.services.mysql.id }} mysql -uroot -proot -N -e "SELECT @@global.time_zone"
268+
269+
- name: Setup Node.js
270+
uses: actions/setup-node@v7
271+
with:
272+
node-version: '22'
273+
274+
- name: Enable Corepack
275+
run: corepack enable
276+
277+
- name: Get pnpm store directory
278+
shell: bash
279+
run: |
280+
echo "STORE_PATH=$(pnpm store path --silent)" >> $GITHUB_ENV
281+
282+
- name: Setup pnpm cache
283+
uses: actions/cache@v6
284+
with:
285+
path: ${{ env.STORE_PATH }}
286+
key: ${{ runner.os }}-pnpm-store-v3-${{ hashFiles('**/pnpm-lock.yaml') }}
287+
restore-keys: |
288+
${{ runner.os }}-pnpm-store-v3-
289+
290+
# Restore-only (same policy as every other job); falls back to the Test
291+
# Core namespace because that job builds a superset of what this one
292+
# needs and its cache is seeded from main.
293+
- name: Restore Turbo cache
294+
uses: actions/cache/restore@v6
295+
with:
296+
path: .turbo/cache
297+
key: ${{ runner.os }}-turbo-${{ github.job }}-${{ github.ref_name }}-${{ github.sha }}
298+
restore-keys: |
299+
${{ runner.os }}-turbo-${{ github.job }}-
300+
${{ runner.os }}-turbo-test-${{ github.ref_name }}-
301+
${{ runner.os }}-turbo-test-
302+
303+
- name: Install dependencies
304+
run: pnpm install --frozen-lockfile
305+
306+
- name: Build driver-sql and its dependencies
307+
run: pnpm exec turbo run build --filter=@objectstack/driver-sql... --concurrency=4
308+
309+
# The whole driver-sql suite runs under the skewed process zone — not
310+
# just the live-server files — so a TZ-sensitive assumption anywhere in
311+
# the driver's tests fails here before it can ship.
312+
- name: Run driver-sql suite against both live servers
313+
env:
314+
TZ: America/New_York
315+
OS_TEST_POSTGRES_URL: postgres://postgres:postgres@127.0.0.1:5432/postgres
316+
OS_TEST_MYSQL_URL: mysql://root:root@127.0.0.1:3306/conformance
317+
run: pnpm --filter @objectstack/driver-sql test
318+
199319
dogfood:
200320
# Sharded 2-way: the suite is ~60 independent test files, each booting its
201321
# own in-process app, and a single 4-vCPU runner needed ~7½ minutes for the

docs/adr/0053-date-and-datetime-semantics.md

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# ADR-0053: `date` is a timezone-naive calendar day; `datetime` is an instant rendered in a reference timezone
22

3-
**Status**: Accepted (2026-06-16) — Phase 1 + addendum D-A1 implemented (`sql-driver.ts` `toDateOnly` write/read/filter normalization; analytics `coerceTemporalFilterValue`), Phase 2 landing incrementally; D-A2 (`temporalFilterValue` promotion onto the `IDataDriver` contract) still open as the ADR predicted. **Partly superseded (2026-07-29, addendum D-B1..D-B4):** Phase 1's "`Field.datetime` stays stored as UTC epoch ms" is replaced by one canonical UTC instant per dialect — `YYYY-MM-DDTHH:MM:SS.sssZ` text on SQLite, `timestamptz` on Postgres, `DATETIME(3)` on MySQL — applied on write and to filter comparands alike (#3912, #3942).
3+
**Status**: Accepted (2026-06-16) — Phase 1 + addendum D-A1 implemented (`sql-driver.ts` `toDateOnly` write/read/filter normalization; analytics `coerceTemporalFilterValue`), Phase 2 landing incrementally; D-A2 resolved 2026-07-30: `temporalFilterValue` + `temporalFilterColumnSql` are optional `IDataDriver` contract members with identity semantics, and analytics types its driver seam from the contract. **Partly superseded (2026-07-29, addendum D-B1..D-B4):** Phase 1's "`Field.datetime` stays stored as UTC epoch ms" is replaced by one canonical UTC instant per dialect — `YYYY-MM-DDTHH:MM:SS.sssZ` text on SQLite, `timestamptz` on Postgres, `DATETIME(3)` on MySQL — applied on write and to filter comparands alike (#3912, #3942).
44
**Deciders**: ObjectStack Protocol Architects
55
**Builds on**: [ADR-0032](./0032-unified-expression-layer.md) (unified expression layer — CEL dialect, `today()`/`daysFromNow()`), [ADR-0014](./0014-record-form-field-type.md) (field types)
66
**Consumers**: `@objectstack/spec` (`Field.date`/`Field.datetime`), `@objectstack/driver-sql` (`coerceFilterValue`, `formatInput`/`formatOutput`, `dateFields`/`datetimeFields`), `@objectstack/formula` (`stdlib` time functions, `cel-engine` hydration), `@objectstack/objectql` (`applyFormulaPlan`), schedule/cron executors, report/analytics date bucketing, `sys-user-preference.timezone`.
@@ -387,6 +387,19 @@ that is not on the driver contract. Promote it to a first-class
387387
retire it)** once the contract method is universal. (If an in-flight
388388
`IDataDriver`-interface change is open, align this with it; do not block on it.)
389389

390+
> **Resolved (2026-07-30).** Both hooks — `temporalFilterValue` and the D-B
391+
> column-side companion `temporalFilterColumnSql` — are optional members of
392+
> `IDataDriver` (`spec/contracts/data-driver.ts`), documented as a PAIR:
393+
> implementing one without the other reintroduces half of #3912, so the
394+
> contract says implement both or neither, with "absent = identity" semantics
395+
> for drivers whose storage form is the wire form. `SqlDriver implements
396+
> IDataDriver`, so its signatures are compile-checked; analytics derives its
397+
> driver seam by `Pick`-ing the contract instead of a local duck type, keeping
398+
> the runtime `typeof` guards as the (correct) way to consume an optional
399+
> member. `coerceFilterValueForSql` already sits behind the hook as the
400+
> last-resort boolean/number recovery for non-temporal columns — the demotion
401+
> this decision asked for — and is retained for exactly that role.
402+
390403
**D-A3 — Add a temporal conformance matrix as the runtime regression backstop.**
391404
Cover `field-type {date, datetime} × operator {eq, gte/lte/gt/lt, in, dateRange} ×
392405
relative-token {today, N_days_ago, N_months_ago, …} × driver {SQLite, Postgres at
@@ -400,8 +413,9 @@ time, the matrix proves runtime correctness across drivers.
400413
- The `datetime`-on-raw-SQL filter bug is closed at the driver boundary, mirroring
401414
Phase 1's "align the consumer with the driver's existing contract rather than
402415
inventing a semantic" stance. No change to Phase 2's reference-timezone plan.
403-
- Until D-A2 lands, the hook depends on a duck-typed driver method — a known,
404-
intentionally-temporary seam tracked here.
416+
- ~~Until D-A2 lands, the hook depends on a duck-typed driver method — a known,
417+
intentionally-temporary seam tracked here.~~ Landed 2026-07-30; see the
418+
resolution note under D-A2.
405419

406420
---
407421

@@ -503,7 +517,8 @@ is clean.
503517
companion threaded to analytics as
504518
`StrategyContext.coerceTemporalFilterColumn`. Coercing the value alone is not
505519
sufficient on a mixed-form column, so any surface that binds a comparand into
506-
raw SQL must wrap its column reference too. Both remain duck-typed until D-A2.
520+
raw SQL must wrap its column reference too. Both promoted onto the contract
521+
2026-07-30 — see the resolution note under D-A2.
507522
- D-A3's conformance matrix should gain a **storage-form** axis (canonical,
508523
legacy-epoch, legacy-naive) and a **server-timezone** axis, since both are now
509524
known to have produced dialect-divergent row results.

packages/services/service-analytics/src/plugin.ts

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import type { Plugin, PluginContext } from '@objectstack/core';
44
import type { Cube, FilterCondition } from '@objectstack/spec/data';
55
import type { ExecutionContext } from '@objectstack/spec/kernel';
6-
import type { IAnalyticsService } from '@objectstack/spec/contracts';
6+
import type { IAnalyticsService, IDataDriver } from '@objectstack/spec/contracts';
77
import { AnalyticsService } from './analytics-service.js';
88
import type { AnalyticsServiceConfig } from './analytics-service.js';
99
import type { DriverCapabilities } from './strategies/types.js';
@@ -45,31 +45,27 @@ interface DataEngineLike {
4545
} | undefined;
4646
/**
4747
* Resolve the storage driver backing an object (public ObjectQL accessor).
48-
* Used to delegate temporal filter-value coercion to the driver, which is the
48+
* Used to delegate temporal storage-form coercion to the driver, which is the
4949
* single source of truth for how a `Field.date`/`Field.datetime` is stored on
50-
* the active dialect. The driver may expose `temporalFilterValue(object, field,
51-
* value)` (SqlDriver does); when absent we leave the value untouched.
50+
* the active dialect. When the hooks are absent, values and column SQL pass
51+
* through untouched — the contract's identity semantics.
5252
*/
53-
getDriverForObject?(objectName: string): DriverLike | undefined;
53+
getDriverForObject?(objectName: string): TemporalDriverSurface | undefined;
5454
}
5555

56-
/** Minimal driver surface the analytics layer probes for temporal coercion. */
57-
interface DriverLike {
58-
/**
59-
* Coerce a filter comparand to the column's on-disk storage form
60-
* (SQLite `Field.datetime` → epoch ms; `Field.date` → YYYY-MM-DD; native
61-
* timestamp / non-temporal → unchanged). Optional — only SqlDriver implements it.
62-
*/
63-
temporalFilterValue?(objectName: string, field: string, value: unknown): unknown;
64-
/**
65-
* Normalise the column reference to that same storage form. Required alongside
66-
* `temporalFilterValue` on any dialect whose column is mixed-form — a SQLite
67-
* `Field.datetime` holds an INTEGER epoch and ISO TEXT at once, so coercing
68-
* only the value fixes one half and leaves the other empty (#3912). Optional —
69-
* only SqlDriver implements it.
70-
*/
71-
temporalFilterColumnSql?(objectName: string, field: string, columnSql: string): string;
72-
}
56+
/**
57+
* The slice of the `IDataDriver` CONTRACT the analytics layer consumes —
58+
* `temporalFilterValue` / `temporalFilterColumnSql` are first-class contract
59+
* members since ADR-0053 D-A2, no longer a duck-typed local invention. Picked
60+
* (rather than using `IDataDriver` whole) because `getDriverForObject` hands
61+
* back whatever the engine registered, and this seam only needs the temporal
62+
* surface; the runtime `typeof` guards below remain the correct way to consume
63+
* an optional contract member.
64+
*/
65+
type TemporalDriverSurface = Pick<
66+
IDataDriver,
67+
'temporalFilterValue' | 'temporalFilterColumnSql'
68+
>;
7369

7470
/**
7571
* Configuration for AnalyticsServicePlugin.

packages/spec/src/contracts/data-driver.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,47 @@ export interface IDataDriver {
127127
/** Delete multiple records matching a query (optional) */
128128
deleteMany?(object: string, query: QueryAST, options?: DriverOptions): Promise<number>;
129129

130+
// ===========================================================================
131+
// Temporal Storage Convention (ADR-0053 D-A1/D-A2, #3912)
132+
// ===========================================================================
133+
//
134+
// A driver is the single source of truth for how a `Field.date` /
135+
// `Field.datetime` value is physically stored on its dialect. Any surface
136+
// that builds queries OUTSIDE the driver's own find()/filter path — the
137+
// analytics native-SQL strategy today, any future raw-query strategy — must
138+
// route its temporal comparands AND its column references through these two
139+
// hooks rather than re-deriving the storage form from the value's textual
140+
// shape (the drift that produced #3912).
141+
//
142+
// The two hooks are a pair on purpose: #3912's lesson is that coercing the
143+
// comparand alone is NOT sufficient on a dialect whose stored form can
144+
// diverge from the bound form — the column side of the comparison has to be
145+
// normalised by the same authority. A driver that implements one without
146+
// the other reintroduces half the bug, so implement both or neither.
147+
//
148+
// Both are optional with identity semantics: a driver whose storage form IS
149+
// the wire form (memory, mongo) simply omits them, and callers treat
150+
// "absent" exactly like "returns the input unchanged".
151+
152+
/**
153+
* Coerce a filter comparand to the on-disk storage form of `field` on
154+
* `objectName` — e.g. an ISO instant for a canonical-text datetime column,
155+
* `YYYY-MM-DD` text for a `Field.date`, a dialect-spelled datetime literal
156+
* where the dialect cannot parse ISO-8601. Non-temporal fields and
157+
* uninterpretable values are returned unchanged.
158+
*/
159+
temporalFilterValue?(objectName: string, field: string, value: unknown): unknown;
160+
161+
/**
162+
* The companion for the LEFT side of the same comparison: given the SQL the
163+
* caller was going to emit for the column (an already-quoted, possibly
164+
* qualified reference), return the SQL it must emit instead so the column
165+
* reads in the same storage form {@link temporalFilterValue} coerces the
166+
* comparand into. Returns `columnSql` verbatim for every column that needs
167+
* no normalisation — which is every column on a fully-converged database.
168+
*/
169+
temporalFilterColumnSql?(objectName: string, field: string, columnSql: string): string;
170+
130171
// ===========================================================================
131172
// Transaction Management
132173
// ===========================================================================

0 commit comments

Comments
 (0)