Skip to content

Commit ae50c73

Browse files
committed
fix(metadata): collapse four isoFromValidDate call sites onto the shared canonical-ISO spelling (#16422)
The per-site `isoFromValidDate` helper rewrote exactly one shape (a valid JS `Date` becomes ISO text) and handed every other input back untouched, so four adapter boundaries fed a `null`, a `number`, an opaque column and an Invalid `Date` into fields declared `z.string()` / `z.string().datetime()`, each behind an `as string` cast asserting the opposite. Measured over the seven inputs that distinguish the two helpers, the declared schemas refused 21 of 35 values. Those four sites now read `canonicalIsoInstant`, whose return type IS `string | undefined`, so all four casts are deleted rather than restated, and both sibling definitions of the retired helper are gone. The terminal value is chosen per site from that site's declared schema: `undefined` for the two `.optional()` fields on `MetadataRecord`, and the epoch (`recordedAtFallback`) for the REQUIRED `MetadataHistoryRecord.recordedAt`, which had no legal answer at all before this change. Refusals over the same inputs: 21 -> 8. `listCommits` in metadata-protocol keeps its copy on purpose — it promises callers the RAW value back, and the shared spelling would erase an Invalid `Date` from the response and reorder the commit timeline. That site is byte-identical on all seven inputs. `SqlDriver`'s same-named helper takes `Date`, not `unknown`, and is the producer-side fold ADR-0053 D-F3 governs; it is not part of this family and is untouched. The three neutrality pins are dispositioned individually: two rewritten as ruled pins, the `listCommits` one kept verbatim because its behaviour did not move. A fourth section the card did not name (`database-loader-14078`'s composition pin) is rewritten too, and records the one composed behaviour that changed: `stat()`'s `updatedAt ?? createdAt` now falls through for an unreadable `updated_at`, publishing a stored `created_at` instead of a fabricated `new Date()`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU
1 parent 8838301 commit ae50c73

1 file changed

Lines changed: 80 additions & 0 deletions

File tree

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
---
2+
"@objectstack/metadata": patch
3+
"@objectstack/metadata-protocol": patch
4+
---
5+
6+
fix(metadata): four `isoFromValidDate` call sites collapse onto the shared canonical-ISO spelling; `MetadataHistoryRecord.recordedAt` gets the terminal value it never had (#16422)
7+
8+
## What was wrong
9+
10+
`#14037`/`#14038` landed a narrow per-site helper, `isoFromValidDate`, beside
11+
the shared `canonicalIsoInstant` spelling. It rewrote exactly one shape — a
12+
valid JS `Date` becomes ISO text — and handed **every other input back
13+
untouched**. Four adapter boundaries used it, and each fed a field declared
14+
`z.string()` or `z.string().datetime()`:
15+
16+
| site | declared as |
17+
|:--|:--|
18+
| `SysMetadataRepository.rowToEvent``MetadataEvent.ts` | `z.string()` |
19+
| `DatabaseLoader.rowToRecord``MetadataRecord.createdAt` / `.updatedAt` | `z.string().datetime().optional()` |
20+
| `DatabaseLoader.getHistoryRecord``MetadataHistoryRecord.recordedAt` | `z.string().datetime()`**required** |
21+
| `DatabaseLoader.queryHistory` → the same field, the other door | `z.string().datetime()`**required** |
22+
23+
So a `null`, a `number`, an opaque column and an Invalid `Date` all arrived at a
24+
field declared `string`, each wearing an `as string` / `as string | undefined`
25+
cast that asserted the opposite. Measured over the seven inputs that
26+
distinguish the two helpers, the declared schemas refused **21 of 35** produced
27+
values.
28+
29+
`recordedAt` was the sharp end: a REQUIRED `z.string().datetime()` for which
30+
none of the three available answers was legal — the visible text
31+
`"Invalid Date"` fails the refinement, `undefined` fails the required field, and
32+
the pass-through fed it the `Date` object, which fails both.
33+
34+
## What it does now
35+
36+
Those four sites read `canonicalIsoInstant`, whose return type **is**
37+
`string | undefined`, so all four casts are deleted rather than restated. Both
38+
sibling definitions of `isoFromValidDate` are gone. The terminal value is chosen
39+
per site, from the site's own declared schema:
40+
41+
- `MetadataRecord.createdAt` / `.updatedAt` are `.optional()``undefined`, the
42+
branch an absent column already took. ⛔ No default is invented for a field the
43+
schema lets be absent.
44+
- `MetadataHistoryRecord.recordedAt` is required → the **epoch**, via a named
45+
`recordedAtFallback()` shared by both history doors. ⛔ Not `new Date()`: a
46+
`now` stamp is a plausible-looking recording instant nobody measured, and it
47+
sorts a version recorded years ago to the top of a newest-first timeline. The
48+
epoch invents no fact and sorts to the oldest end. It is also the answer the
49+
sibling reader of this same `sys_metadata_history.recorded_at` column already
50+
gives (`rowToEvent` and `history()`, both `?? new Date(0).toISOString()`).
51+
52+
Schema refusals over the same seven inputs: **21 → 8**. The eight that remain
53+
are a `number` and an opaque object at four sites — shapes no driver is measured
54+
to materialise for these columns. They now arrive as the declared *type* (a
55+
string) that simply is not a valid datetime, so the producer's bug stays visible
56+
instead of being papered over.
57+
58+
## One behaviour change worth reading twice
59+
60+
`DatabaseLoader.stat()` computes `record.updatedAt ?? record.createdAt`. An
61+
Invalid `updated_at` used to WIN that `??` — a `Date` is truthy and not nullish —
62+
so a row with an unreadable `updated_at` and a good `created_at` published
63+
`new Date()` as its `mtime`. It now folds to `undefined` one step earlier and
64+
loses the `??`, so the row publishes its `created_at`: a stored instant in place
65+
of a fabricated one, and exactly the "same `?? <default>` chain an absent column
66+
takes" that `#14078`'s own ruling text prescribes for the shape.
67+
68+
## What deliberately did NOT collapse
69+
70+
`listCommits` in `@objectstack/metadata-protocol` keeps its copy. Its docblock
71+
promises callers the RAW value back for a non-`Date`, and the shared spelling
72+
rewrites the whole domain: swapping it in would ERASE an Invalid `Date` from the
73+
response (`undefined` — the one answer ADR-0053 D-F3 refuses, because it silently
74+
drops a value that is on disk) and hand a `number` or an opaque object to the
75+
commit-timeline sort as `String(value)` rather than verbatim. Measured, that site
76+
is byte-identical on all seven inputs before and after this change.
77+
78+
`SqlDriver`'s same-named helper is not part of this family at all: it takes
79+
`Date` (not `unknown`), both its call sites narrow with `instanceof Date` first,
80+
and it is the PRODUCER-side fold ADR-0053 D-F3 governs. It is untouched.

0 commit comments

Comments
 (0)