Skip to content

Commit 3566c08

Browse files
fix(metadata-protocol): seed replay compares time, datetime and date fields in their stored form (#24617)
Fixes #24562 Clause-②: no ## What changed `SeedLoaderService.isNoOpReplay` (`packages/metadata-protocol/src/seed-loader.ts`) now compares a field the object declares `time`, `datetime` or `date` in its **stored form**. Both sides go through `temporalStorageForm` (`@objectstack/core`), the rule `SqlDriver` and `InMemoryDriver` already apply on write (ADR-0053). Every other field keeps the comparison it had. - The kinds come from the object definition `loadDataset` already resolves (`temporalFieldKinds`, next to `seedValuesInStoredForm`). They are passed explicitly to both write paths that call `isNoOpReplay`: the batched `decideWriteAction` (all three call sites) and the sequential `writeRecord` used by self-referencing objects. The new parameter is required, so no call site can leave it out. - "Any doubt reads as changed" still holds. `temporalStorageForm` returns a value it cannot read unchanged, so an unparseable value equals only its own identical spelling. A non-text result (an Invalid Date, a structure) never compares equal. - Nothing a seed may declare changes, and nothing any write stores changes. This is a comparison inside the replay only (the triage ruling). No consumer-side tolerance was added anywhere. ## Measured: mechanism before and after, on a real driver This used a scratch harness (not committed): a real `ObjectQL` and `SqlDriver` (`better-sqlite3`) on one SQLite **file** DB. Each boot is a new driver, engine and loader. Results: ``` scratch object (time / datetime / date), seed spelled short stored after boot 1: '09:15' -> '09:15:00'; '2026-06-17T14:30:00Z' -> '2026-06-17T14:30:00.000Z'; date written '2026-06-17T00:00:00Z' -> '2026-06-17' base 2667e24 boot 2 (same seed): updated=3 skipped=1 (short-time, short-datetime, iso-date rewritten) fix 09b324c boot 2 (same seed): updated=0 skipped=4 fix 09b324c boot 3 (time+instant changed): updated=2 skipped=2 (exactly the two changed rows) showcase: every examples/app-showcase object definition + every ShowcaseSeedData dataset on a showcase-owned object (127 rows) base 2667e24 boot 2: engine.update on 5 rows -- Specimen — Full, INV-1001 · Consulting hours, INV-1004 · Consulting hours, and their two parent invoices (summary recompute) per-key causes: lines -> service_start '09:15' vs '09:15:00' (and '13:40'); Specimen — Full -> f_datetime, f_time AND f_password fix 09b324c boot 2: engine.update on 1 row -- Specimen — Full, f_password only ``` The `date` kind has the same spelling gap (an instant written to a `date` field), so it is covered too. **What this PR does not fix:** Specimen — Full still gets a new `updated_at` on every boot. The cause is a second, independent mechanism. The loader reads existing rows through `engine.find`, which masks a `password` field to `••••••••` in every context, so the seed's plaintext `hunter2` never equals the stored value. It is not a temporal-spelling issue. Fixing it means deciding how a replay compares a credential it cannot read, so it is reported to the seat as a separate finding and left out of this PR. So of the card's 5 churning showcase rows, 4 stop churning here; the fifth keeps churning until that finding has its own fix. The full `objectstack dev` two-boot on the stock showcase is **NOT MEASURED**: the CLI's build closure is 63 packages, which is too much for this container's shared verify lock. The harness above loads the showcase's own object definitions and seed data through the same `SeedLoaderService`, `ObjectQL` and `SqlDriver`, and on base it reproduces the card's exact 5 rows. ## Tests - New pins in `packages/metadata-protocol/src/seed-loader-replay.test.ts`, reusing the file's pinned faithful engine double. Its `insert` and `update` are overridden to write temporal fields in the driver's stored form and to stamp `updated_at`. Each case asserts which write path ran (a `writeRecord` spy): - replaying the unchanged short-spelled seed (`'09:15'`, `'…T14:30:00Z'`, zone-naive `'2026-06-17 14:30'`, an instant on a `date` field, plus a stored-spelling control) issues **no update** and leaves the store, `updated_at` included, deep-equal. 4 cases: batched and sequential paths, modes `upsert` and `update`; - positive control: a changed wall clock, instant and day are each rewritten, **only** their rows, and stored in the new value's form (2 cases); - an unparseable time and datetime still read as changed (2 cases). - `pnpm --filter @objectstack/metadata-protocol exec vitest run --maxWorkers=2 src/seed-loader-replay.test.ts`: 13 passed (5 existing + 8 new). - `pnpm --filter @objectstack/metadata-protocol typecheck && pnpm --filter @objectstack/metadata-protocol exec vitest run --maxWorkers=2` at `7cad49c76` (the final head, after merging `origin/main` `5a871e4e4` and rebuilding the package closure): typecheck exit 0; 248 files passed, 3 skipped; 20731 tests passed, 19 skipped. The package `tsconfig` includes `src/**/*`; `--listFiles` counts the test file. - **Ablation** (one-shot, committed fix at `09b324c5b`, through `scripts/ablation-replace.mjs`; tests import `./seed-loader.js` from source, so no `dist/` is involved). The `if (kind)` stored-form block was deleted: anchor 1 to 0, blob `ee3eb38f87de` to `206fd67db529`. Predicted direction: all 8 new cases red, 5 existing green. Observed: **8 failed, 5 passed**. The replay negatives failed with "expected not to be called, called 4 times" (the 4 short-spelled rows); the control and unparseable cases failed by rewriting extra rows. Restored: blob equals HEAD `ee3eb38f87de`, `git diff HEAD` empty. ## Gates Locally, the derived set from `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`, run at the final head `7cad49c76`. `--ran` reconciliation: **64 derived, 64 run, 0 NOT-MEASURED, 0 UNRUN**, every one exit 0. - That includes `check:engine-double-contract` (OK, 1031 pinned), `check:durability-log-level`, `check:nul-bytes`, `check:type-check-debt`, `check:dual-build-cjs-loads` (111 require entry points load), `check:test-source-alias`, `check:cross-package-test-inputs`, `check:published-files` and `check-adr-0087-registration` (1 non-breaking changeset). - `check-plugin-teardown-shape --self-test` needs a positive-control commit that lay outside this shallow clone. That commit was fetched with `--depth=1`, and the self-test then passed with 48 cases. - `test:repo` and the whole-repo `lint`: not run locally, declared to CI. ## Acceptance notes - Changeset: `.changeset/24562-seed-replay-time-equality.md`, `@objectstack/metadata-protocol` `patch`. - `seedValuesInStoredForm` (a `Date` resolved by a CEL envelope, put in its stored form before the write) is untouched. The new helper reads the definition's field types the same way. - `@objectstack/metadata-protocol` has no dependency on a driver package, so the pin uses the existing double with the drivers' own storage rule, not a real driver. The real-driver reading is the measurement above. It was a one-shot and is not a permanent test, by the dispatch file surface. --- _Generated by [Claude Code](https://claude.ai/code/session_01AHeUBbe6S3JRawcpbLejXD)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent c395405 commit 3566c08

3 files changed

Lines changed: 245 additions & 15 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@objectstack/metadata-protocol': patch
3+
---
4+
5+
fix(metadata-protocol): a seed replay no longer rewrites an unchanged row whose `time`, `datetime` or `date` value the seed spells differently from the stored form (#24562)
6+
7+
Clause-②: no
8+
9+
The replay's no-op check (`isNoOpReplay`) compared a seed value with the stored one as text, so `'09:15'` against the stored `'09:15:00'`, `'2026-06-17T14:30:00Z'` against `'2026-06-17T14:30:00.000Z'`, and a `date` written as an instant against its stored `YYYY-MM-DD` all read as changed. Every boot rewrote those rows: `updated_at` moved, lifecycle validation ran again, history tracking recorded an edit nobody made, and a parent's summary recompute moved the parent's `updated_at` too.
10+
11+
A field the object declares `time`, `datetime` or `date` is now compared in its stored form, by the rule the drivers apply on write (`temporalStorageForm`, `@objectstack/core`). Two spellings of the same wall clock, instant or calendar day skip the row. A different value is still rewritten, and a value that rule cannot read still counts as changed. What a seed may declare, and what any write stores, is unchanged.

‎packages/metadata-protocol/src/seed-loader-replay.test.ts‎

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { describe, it, expect, vi } from 'vitest';
44
import { SeedLoaderService } from './seed-loader.js';
55
import type { IDataEngine, IMetadataService } from '@objectstack/spec/contracts';
66
import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate } from '@objectstack/metadata-core';
7+
import { temporalComparandKind, temporalStorageForm } from '@objectstack/core';
78

89
/**
910
* Replay regression: seeds with lookup natural keys must survive a dev-server
@@ -322,3 +323,171 @@ describe('seed replay (restart) — lookup natural keys on the update path', ()
322323
expect(store.my_app_interview.find((r) => r.name === 'iv-alice-1')!.candidate).toBe(aliceId);
323324
});
324325
});
326+
327+
/**
328+
* [#24562] A `time` / `datetime` / `date` value the seed spells short, replayed
329+
* over the row a driver stored for it.
330+
*
331+
* The faithful engine above stores exactly what it is handed; a real driver
332+
* does not. `SqlDriver` and `InMemoryDriver` both write a temporal field
333+
* through `temporalStorageForm` (`@objectstack/core`, ADR-0053), so measured
334+
* on a SQLite file DB `'09:15'` lands as `'09:15:00'`,
335+
* `'2026-06-17T14:30:00Z'` as `'2026-06-17T14:30:00.000Z'`, and a `date`
336+
* written as an instant as its `YYYY-MM-DD`. The engine below does the same,
337+
* and stamps `updated_at` on every write the way the audit stamp does, so a
338+
* replay that rewrites an unchanged row shows as a moved stamp.
339+
*
340+
* Both write paths compare through `isNoOpReplay`: the batched
341+
* `decideWriteAction`, and the sequential `writeRecord` a self-referencing
342+
* object takes (`hasSelfRef`) — each case below asserts which one ran.
343+
*/
344+
describe('seed replay — a temporal value spelled short is the value the driver stored', () => {
345+
const ENTRY_FIELDS = {
346+
name: { type: 'text' },
347+
starts_at: { type: 'time' },
348+
logged_at: { type: 'datetime' },
349+
worked_on: { type: 'date' },
350+
};
351+
const ENTRY = { name: 'tz_replay_entry', fields: ENTRY_FIELDS };
352+
const TREE_ENTRY = {
353+
name: 'tz_replay_tree_entry',
354+
fields: { ...ENTRY_FIELDS, parent: { type: 'lookup', reference: 'tz_replay_tree_entry' } },
355+
};
356+
const PATHS = [
357+
{ path: 'batched decideWriteAction', object: ENTRY, sequential: false },
358+
{ path: 'sequential writeRecord', object: TREE_ENTRY, sequential: true },
359+
];
360+
361+
const SHORT_SPELLINGS: Array<Record<string, unknown>> = [
362+
{ name: 'short-time', starts_at: '09:15' },
363+
{ name: 'short-datetime', logged_at: '2026-06-17T14:30:00Z' },
364+
{ name: 'zone-naive-datetime', logged_at: '2026-06-17 14:30' },
365+
{ name: 'instant-date', worked_on: '2026-06-17T00:00:00Z' },
366+
// Control: already spelled the way the driver stores it.
367+
{ name: 'stored-spelling', starts_at: '09:05:30', logged_at: '2026-06-17T14:30:00.000Z', worked_on: '2026-06-17' },
368+
];
369+
370+
function createStoringEngine(object: { name: string; fields: Record<string, { type: string }> }) {
371+
const { engine, store } = createFaithfulEngine();
372+
let clock = 0;
373+
const stored = (row: Record<string, unknown>) => {
374+
const out: Record<string, unknown> = { ...row, updated_at: `stamp-${++clock}` };
375+
for (const [key, value] of Object.entries(row)) {
376+
const kind = temporalComparandKind(object.fields[key]?.type);
377+
if (kind && value != null) out[key] = temporalStorageForm(value, kind);
378+
}
379+
return out;
380+
};
381+
const insert = (engine.insert as any).getMockImplementation();
382+
(engine.insert as any).mockImplementation(async (o: string, data: any, opts?: any) =>
383+
insert(o, Array.isArray(data) ? data.map(stored) : stored(data), opts));
384+
const update = (engine.update as any).getMockImplementation();
385+
(engine.update as any).mockImplementation(async (o: string, data: any, opts?: any) => update(o, stored(data), opts));
386+
return { engine, store };
387+
}
388+
389+
function metadataOf(object: { name: string }): IMetadataService {
390+
return {
391+
getObject: vi.fn(async (name: string) => (name === object.name ? object : undefined)),
392+
listObjects: vi.fn(async () => [object]),
393+
register: vi.fn(async () => {}),
394+
get: vi.fn(async (_t: string, name: string) => (name === object.name ? object : undefined)),
395+
list: vi.fn(async () => []),
396+
unregister: vi.fn(async () => {}),
397+
exists: vi.fn(async () => false),
398+
listNames: vi.fn(async () => []),
399+
} as unknown as IMetadataService;
400+
}
401+
402+
async function load(
403+
engine: IDataEngine,
404+
object: { name: string },
405+
mode: string,
406+
records: Array<Record<string, unknown>>,
407+
) {
408+
const writeRecord = vi.spyOn(SeedLoaderService.prototype as any, 'writeRecord');
409+
try {
410+
const result = await new SeedLoaderService(engine, metadataOf(object), createLogger()).load({
411+
seeds: [{ object: object.name, externalId: 'name', mode, env: ['prod', 'dev', 'test'], records }] as any,
412+
config: CONFIG,
413+
});
414+
return { result, writeRecordCalls: writeRecord.mock.calls.length };
415+
} finally {
416+
writeRecord.mockRestore();
417+
}
418+
}
419+
420+
const rowOf = (store: Record<string, any[]>, object: { name: string }, name: string) =>
421+
store[object.name].find((r) => r.name === name)!;
422+
const updatedNames = (engine: IDataEngine, store: Record<string, any[]>, object: { name: string }) =>
423+
(engine.update as any).mock.calls
424+
.map(([, data]: [string, any]) => store[object.name].find((r) => r.id === data.id)?.name)
425+
.sort();
426+
427+
it.each(PATHS.flatMap((p) => ['upsert', 'update'].map((mode) => ({ ...p, mode }))))(
428+
'$path, mode $mode: replaying the unchanged seed issues no update and moves no updated_at',
429+
async ({ object, sequential, mode }) => {
430+
const { engine, store } = createStoringEngine(object);
431+
const first = await load(engine, object, 'upsert', SHORT_SPELLINGS);
432+
expect(first.result.summary.totalInserted).toBe(5);
433+
// The premise: what the column holds is the driver's spelling, not the seed's.
434+
expect(rowOf(store, object, 'short-time').starts_at).toBe('09:15:00');
435+
expect(rowOf(store, object, 'short-datetime').logged_at).toBe('2026-06-17T14:30:00.000Z');
436+
expect(rowOf(store, object, 'zone-naive-datetime').logged_at).toBe('2026-06-17T14:30:00.000Z');
437+
expect(rowOf(store, object, 'instant-date').worked_on).toBe('2026-06-17');
438+
(engine.update as any).mockClear();
439+
const before = structuredClone(store);
440+
441+
const replay = await load(engine, object, mode, SHORT_SPELLINGS);
442+
443+
expect(replay.writeRecordCalls > 0).toBe(sequential);
444+
expect(engine.update).not.toHaveBeenCalled();
445+
expect(replay.result.summary.totalUpdated).toBe(0);
446+
expect(replay.result.summary.totalSkipped).toBe(5);
447+
expect(replay.result.summary.totalErrored).toBe(0);
448+
expect(store).toEqual(before);
449+
},
450+
);
451+
452+
it.each(PATHS)('$path: a changed wall clock, instant or day is still rewritten, and only its row', async ({ object, sequential }) => {
453+
const { engine, store } = createStoringEngine(object);
454+
await load(engine, object, 'upsert', SHORT_SPELLINGS);
455+
(engine.update as any).mockClear();
456+
const before = structuredClone(store);
457+
const changed = SHORT_SPELLINGS.map((r) =>
458+
r.name === 'short-time' ? { ...r, starts_at: '09:16' }
459+
: r.name === 'short-datetime' ? { ...r, logged_at: '2026-06-17T14:30:01Z' }
460+
: r.name === 'instant-date' ? { ...r, worked_on: '2026-06-18T00:00:00Z' }
461+
: r);
462+
463+
const replay = await load(engine, object, 'upsert', changed);
464+
465+
expect(replay.writeRecordCalls > 0).toBe(sequential);
466+
expect(updatedNames(engine, store, object)).toEqual(['instant-date', 'short-datetime', 'short-time']);
467+
expect(replay.result.summary.totalUpdated).toBe(3);
468+
expect(replay.result.summary.totalSkipped).toBe(2);
469+
expect(rowOf(store, object, 'short-time').starts_at).toBe('09:16:00');
470+
expect(rowOf(store, object, 'short-datetime').logged_at).toBe('2026-06-17T14:30:01.000Z');
471+
expect(rowOf(store, object, 'instant-date').worked_on).toBe('2026-06-18');
472+
for (const name of ['zone-naive-datetime', 'stored-spelling']) {
473+
expect(rowOf(store, object, name)).toEqual(before[object.name].find((r) => r.name === name));
474+
}
475+
});
476+
477+
it.each(PATHS)('$path: a value the storage rule cannot read still reads as changed', async ({ object, sequential }) => {
478+
const { engine, store } = createStoringEngine(object);
479+
await load(engine, object, 'upsert', SHORT_SPELLINGS);
480+
(engine.update as any).mockClear();
481+
const unreadable = SHORT_SPELLINGS.map((r) =>
482+
r.name === 'short-time' ? { ...r, starts_at: 'quarter past nine' }
483+
: r.name === 'short-datetime' ? { ...r, logged_at: 'not-a-date' }
484+
: r);
485+
486+
const replay = await load(engine, object, 'upsert', unreadable);
487+
488+
expect(replay.writeRecordCalls > 0).toBe(sequential);
489+
expect(updatedNames(engine, store, object)).toEqual(['short-datetime', 'short-time']);
490+
expect(replay.result.summary.totalUpdated).toBe(2);
491+
expect(replay.result.summary.totalSkipped).toBe(3);
492+
});
493+
});

0 commit comments

Comments
 (0)