Skip to content

docs(engine): correct seven out-of-package comments that still gate formatOutput's timestamp passes on isSqlite - #16817

Merged
os-musk merged 2 commits into
mainfrom
claude/issue-16728-post-16619-comment-drift
Sep 8, 2026
Merged

docs(engine): correct seven out-of-package comments that still gate formatOutput's timestamp passes on isSqlite#16817
os-musk merged 2 commits into
mainfrom
claude/issue-16728-post-16619-comment-drift

Conversation

@os-musk

@os-musk os-musk commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

Fixes #16728

Comment prose only. Not one line of runtime code moves, in any package.

What changed underneath the prose

PR #16619 (card #13973, ADR-0053 D-F1) made both of SqlDriver#formatOutput's timestamp
passes unconditional on every dialect. Verified by reading formatOutput on the current
tree, not by grepping it: the if (this.isSqlite) arm opens at the JSON codec, closes after
the numeric-scalar repair, and both the AUDIT_TIMESTAMP_COLUMNS pass and the
normalizeSqliteDatetimeOutput pass over datetimeFields sit at top level below it. Seven
out-of-package comments still described them as gated on that arm.

packages/rest/src/rest-server.ts was repaired first, per the triage ruling on the card:
it is not stale description but a warning aimed at a future author — "A declared
Field.datetime is therefore NOT protected on Postgres/MySQL" — and the action it invites
(a tolerant coercion at the consumer) is what ADR-0053 and that card expressly forbid. A
paragraph was arguing for the reverse of its own governing ruling.

Deliverable 1 — re-measurement, beside triage's numbers

Triage measured at origin/main 5e53d73d. This branch measured at 56ee7aef1d, after
PR #16716 (#16609) and PR #16710 (#16570) landed on this surface.

git grep -n normalizeSqliteDatetimeOutput -- 'packages/**'18, distribution identical
to triage's 10 + 1 + 7:

bucket count detail
driver-sql home 10 sql-driver.ts 7 (the firing control), sql-driver-timestamp-format.test.ts 1, sql-driver-13973-canonical-iso-read-door.test.ts 1, its CHANGELOG.md 1
change history 1 packages/rest/CHANGELOG.md — untouched
out-of-package 7 the seven sites, all confirmed present

The firing control holds: sql-driver.ts still answers 7, so the query hits where it can
hit and the out-of-package 7 is a real reading, not a dead query. Nothing had been repaired
by anyone else, and no site had moved into packages/metadata-protocol/src/protocol.ts (held
by PR #16796 — untouched here, confirmed by file list). Census is byte-stable after the fix:
still 18, same distribution, driver-sql and both CHANGELOGs untouched.

The premise is WIDER than the card, in one direction the symbol query cannot see

The census keys on normalizeSqliteDatetimeOutput. Prose that asserts the same false gating
without naming that symbol is invisible to it. Sweeping the symbol that cannot be split by
line-wrapping either — if (this.isSqlite) — finds 11 further occurrences in 10 more files,
same defect class, all false since PR #16619, all outside this card's declared file surface:

packages/cli/src/commands/migrate/duplicates.ts · duplicates.created-at-canonical.test.ts ·
packages/metadata-protocol/src/protocol.ts (x2, held by PR #16796) ·
protocol-14038-list-commits-created-at-iso.test.ts · protocol.commit-timeline-instant-order.test.ts ·
sys-metadata-repository-13997-authored-at-canonicalisation.test.ts ·
packages/metadata/src/loaders/database-loader.test.ts ·
packages/metadata/src/migrations/migrate-sys-notification-to-event.ts ·
packages/services/service-storage/src/stranded-orphan-inventory.ts · its .test.ts

Not repaired here — outside the declared file surface, and protocol.ts is another PR's. Filed
separately; see the acceptance notes.

Three of the seven files also carried a second occurrence of the same false claim, invisible
to the symbol census for the same reason. Those are in-file and in scope, so the diff rewrites
11 sentences across the 7 declared files, not 7.

The seven sites — before to after

Each was verified by reading, and each rewrite states the same three facts: the passes are
unconditional since #13973 / ADR-0053 D-F1; withPostgresCalendarDayAsText is untouched
(D-F2); the Invalid Date residue stands (D-F3).

  1. packages/rest/src/rest-server.ts (the ruled-first one) — "both sit INSIDE its
    if (this.isSqlite) arm, so neither runs here. A declared Field.datetime is therefore NOT
    protected on Postgres/MySQL"
    to: both passes run on every dialect, the driver folds the
    client's Date at its own read boundary, and exactly one Date shape still arrives — an
    Invalid Date, which keeps the arm live rather than dead. The sibling bullet claiming the
    canonical-text arm is SQLite-only was corrected with it; leaving it would have contradicted
    the new text in the same docblock.
  2. packages/rest/src/import-job-dto-timestamp-canonical.test.ts — the defect narrative moved
    to past tense and gained the reason these cases are still live (mongo Date, Invalid Date);
    the "deliberately NOT claimed" section no longer cites formatOutput's "isSqlite bracketing"
    as the current dialect fact.
  3. packages/metadata/src/loaders/database-loader.ts (x2 — the canonicalIsoInstant docblock
    and the Five more adapter-boundary sites cast a driver Date into a declared ISO-string timestamp — MetadataEvent.ts, MetadataHistoryRecord.recordedAt, MetadataRecord.createdAt/updatedAt #14037 one) — both now say the pass was lifted out of the gate, and that this
    producer-side arm became a no-op, not a conflict, under the B1 ruling.
  4. packages/metadata/src/loaders/database-loader-14037-adapter-boundary-iso.test.ts — same,
    plus why the planted Date is still a shape a driver produces.
  5. packages/metadata-protocol/src/sys-metadata-repository.ts (x3 — the canonicalIsoInstant
    docblock, the rowToEvent one, and the listDrafts body comment). The first also quoted a
    clause from withPostgresCalendarDayAsText — that Field.datetime "depends on" the Date
    materialisation — which [finding] Sweep: which consumers compare or format a value whose runtime type differs between the Date-materialising drivers and the ISO-text ones #13973 checked against the tree, found did not hold, and removed; the
    comment was quoting deleted prose, so the rewrite says so. The listDrafts comment cited
    "[finding] Sweep: which consumers compare or format a value whose runtime type differs between the Date-materialising drivers and the ISO-text ones #13973's two standing prohibitions"; one of the two (no unilateral reshape at the driver's
    read door) was discharged by the B1 ruling, which made that reshape the central fix. The
    surviving prohibition — no tolerant ?? at a consumer — is kept and marked as the one that stands.
  6. packages/metadata-protocol/src/sys-metadata-repository-14037-event-ts-canonicalisation.test.ts
  7. packages/metadata-protocol/src/sys-metadata-repository-14938-list-drafts-updated-at.test.ts
    — its section heading "Why the value is a Date on the live dialects" was itself the false
    claim, so the heading changed too.

Five of the seven cited sql-driver-13567-audit-stamp-materialisation.test.ts as pinning "both
column classes arrive as a Date". Reading that fossil first, as the standing instruction
requires: its own header says §B "now pins that contract" and "is therefore inverted on purpose".
Those citations were doubly false — the claim, and the pin invoked for it. Each now points at the
inverted §B and says what it pins today. The fossil agrees with this direction; no conflict to report.

Both un-flattenable items survive — verified mechanically

  1. withPostgresCalendarDayAsText is still untouched (D-F2). All six sites that mentioned it
    still do (the seventh never did); every one keeps "the client hands back a Date" as true
    and corrects only "nothing folds it". driver-sql is not in this diff at all.
  2. The Invalid Date residue stands (D-F3). All seven files carry the carve-out explicitly.
    Repo-wide grep for the overstatement PR feat(driver-sql): the read door presents datetime values and audit stamps as canonical ISO-Z text on every dialect (#13973, B1 narrow) #16619's own contract review returned a finding for —
    any claim that the read door never hands out a Date — matches zero lines.

Changeset route — decided on a measurement, not on size

Governing text: .github/workflows/pr-automation.yml, the WHICH LEVEL block (lines ~660-690).

Text rejected: route 2, the skip-changeset label. Its scope is "it releases nothing", and
AGENTS.md:1028 sets the floor — that label is for "a diff that publishes nothing from any
released package". Measured against built output, that is false here:

  • .d.ts of all three packages: no amended text reaches one — zero occurrences of
    isSqlite, D-F3 or any amended phrase in any dist/*.d.ts / .d.cts. No public surface moves.
  • packages/rest/dist/index.js and .cjs, and packages/metadata-protocol/dist/index.{js,cjs}:
    carry the amended comment text verbatim. D-F3 occurs zero times in either package's src
    at the merge base and appears in both bundles after the change, so the published bytes do change.
  • packages/metadata/dist: JSDoc blocks are stripped by its bundle (control: 288 // lines survive,
    the touched block comments do not appear). Its published output is unchanged, so it is deliberately
    not named in the changeset.

Route taken: route 1, a patch changeset naming @objectstack/metadata-protocol and
@objectstack/rest.
patch and not minor because the WHICH LEVEL block reserves minor for a
purely additive widening of a published public surface, and nothing here widens one.

Verification

Gate coverage, reported verbatim from node scripts/pm/dispatch-gates.mjs --ran ... --repo objectstack-ai/objectstack:

Run reconciliation — 58 derived, 58 run, 0 NOT-MEASURED, 0 UNRUN.
✓ dispatch-gates --ran: 58 derived famil(ies) accounted for — 58 run, 0 NOT-MEASURED.

That line accounts for coverage, not verdicts. Verdicts, stated separately:

  • 56 of 58 exited 0.
  • 2 exited 3 = PREREQUISITE NOT MET, which is NOT MEASURED and is not a pass:
    pnpm check:dual-build-cjs-loads ("this gate reads built output, and some package has no dist/…
    This is NOT a pass: nothing was measured") and pnpm check:type-check-debt ("--re-measure cannot
    run: 6 workspace dependenc(ies) … have no built type entry point on disk"). Both need a
    whole-workspace build, which is CI's run, not this branch's. Declared narrowing, not a skip.

Tests and typecheck, at 6e6f67d884, all under scripts/pm/os-verify-lock.sh (VERDICT command-exit 0):

package test typecheck
@objectstack/rest 184 files / 3057 passed tsc --noEmit + check:test-typecheck (0 files / 0 errors)
@objectstack/metadata-protocol 169 files passed, 2 skipped / 2433 passed, 10 skipped tsc --noEmit Done
@objectstack/metadata 52 files / 797 passed tsc --noEmit Done

Dependency closure built first (pnpm --filter 'PKG^...' build, each package's upstream closure),
then the three packages themselves, so the dist measurement above reads real emitted bytes rather
than a stale tree. No ablation: this diff adds no guard and moves none, so there is nothing whose
failure could be demonstrated by reverting it.

验收备注

Clause-②: no — comment prose only, no exported symbol, no key on a published payload. The
measurement above agrees with the seat's declaration: zero amended text reaches any dist/*.d.ts.


Generated by Claude Code

…t's timestamp passes

Since #13973 / PR #16619 (ADR-0053 D-F1) both of `SqlDriver#formatOutput`'s
timestamp passes -- the AUDIT_TIMESTAMP_COLUMNS pass and the
normalizeSqliteDatetimeOutput pass over datetimeFields -- run on every dialect.
Seven out-of-package comments still described them as gated on
`if (this.isSqlite)`.

The rest-server.ts one was not merely stale: it warned future authors that "a
declared Field.datetime is therefore NOT protected on Postgres/MySQL", which
invites the tolerant consumer-side coercion ADR-0053 / #16619 expressly forbid.
Corrected first, per the triage ruling on the card.

Both carve-outs are preserved rather than flattened:
  - withPostgresCalendarDayAsText is untouched (D-F2) -- the client still hands
    back a Date; only "nothing folds it" was false.
  - the Invalid `Date` residue still stands (D-F3) -- no sentence claims the
    read door never hands out a Date.

Comments only; no runtime behaviour changes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
…e changes

Measured, not assumed: the amended comment text reaches
packages/rest/dist/index.{js,cjs} and packages/metadata-protocol/dist/index.{js,cjs}
verbatim, so those two publish changed bytes. @objectstack/metadata is deliberately
not named -- its edits are all JSDoc blocks, which its bundle strips.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
@github-actions github-actions Bot added the size/m label Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/metadata-protocol, @objectstack/metadata, @objectstack/rest, touching 1 documentable anchor(s). ⚠️ 2 changed file(s) yielded no anchor (packages/metadata/src/loaders/database-loader.ts, packages/rest/src/rest-server.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/concepts/metadata-lifecycle.mdx (via SysMetadataRepository (symbol, a top-level class))
What this run could not see
  • 2 changed file(s) yielded no anchor (packages/metadata/src/loaders/database-loader.ts, packages/rest/src/rest-server.ts) — pages documenting those are invisible to this run
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 23 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 30b0990787ef633cfff724951ceeee08c4fc05b9packageMentionDocs.

Which tree this was computed on

This run read content/docs from f4e42e59934eae1ce965d732ccdc874451fc8041 — the merge of head 6e6f67d88447bd897c7d5f221c83f21a768053a3 into base 30b0990787ef633cfff724951ceeee08c4fc05b9, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin f4e42e59934eae1ce965d732ccdc874451fc8041 && git checkout f4e42e59934eae1ce965d732ccdc874451fc8041
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 30b0990787ef633cfff724951ceeee08c4fc05b9 6e6f67d88447bd897c7d5f221c83f21a768053a3 && git checkout -B drift-repro 30b0990787ef633cfff724951ceeee08c4fc05b9 && git merge --no-ff 6e6f67d88447bd897c7d5f221c83f21a768053a3

node scripts/docs-audit/affected-docs.mjs --json 30b0990787ef633cfff724951ceeee08c4fc05b9

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 30b0990787ef633cfff724951ceeee08c4fc05b9 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 8, 2026
@os-musk
os-musk marked this pull request as ready for review September 8, 2026 09:29
@os-musk
os-musk enabled auto-merge September 8, 2026 09:29
@os-musk
os-musk added this pull request to the merge queue Sep 8, 2026
Merged via the queue into main with commit ce478db Sep 8, 2026
35 checks passed
@os-musk
os-musk deleted the claude/issue-16728-post-16619-comment-drift branch September 8, 2026 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants