Skip to content

docs: correct nine more out-of-package comments that still gated formatOutput's timestamp passes on isSqlite - #16866

Merged
os-musk merged 5 commits into
mainfrom
claude/issue-16818-isqlite-comment-drift-sweep
Sep 8, 2026
Merged

docs: correct nine more out-of-package comments that still gated formatOutput's timestamp passes on isSqlite#16866
os-musk merged 5 commits into
mainfrom
claude/issue-16818-isqlite-comment-drift-sweep

Conversation

@os-musk

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

Copy link
Copy Markdown
Collaborator

Fixes #16818

Nine out-of-package comments still described SqlDriver#formatOutput's two timestamp
passes as gated on if (this.isSqlite). They are not. Comments only — zero runtime
behaviour, no exported symbol and no public type changes.

All readings below are pinned to this PR's head 08f3a9cff0.

Clause-②: no

What the tree actually says

Measured by walking formatOutput in packages/drivers/driver-sql/src/sql-driver.ts,
not recalled from the card:

formatOutput                       opens :16947   closes :17125
  if (this.isSqlite) {             opens :16965   closes :17026
      jsonFields   (JSON codec)          :16996   INSIDE the arm
      numericFields (scalar repair)      :17015   INSIDE the arm
  AUDIT_TIMESTAMP_COLUMNS loop           :17046   OUTSIDE — top level
  datetimeFields loop                    :17061   OUTSIDE — top level
    normalizeSqliteDatetimeOutput(...)   :17065   OUTSIDE — top level

The surviving if (this.isSqlite) arm covers the JSON codec and the numeric-scalar
repair only, and it closes twenty lines above the first timestamp pass.

The fossil, quoted before rewriting

Three recorded constraints are carried through rather than flattened. Each is quoted from
the tree at 08f3a9cff0.

D-F2 — withPostgresCalendarDayAsText is still untouched. sql-driver.ts:5372:

timestamptz / timestamp are deliberately untouched here: those are instants, and a
Date is the right materialisation for them at the CLIENT layer, where the wire text is
parsed. It is not, however, what the driver hands out of its read doors.

So "the client hands back a Date" stays TRUE everywhere it appears; only "nothing folds
it" was false. Every rewrite says which layer it is talking about.

D-F3 — the Invalid Date residue survives. isoFromValidDate, sql-driver.ts:324:

function isoFromValidDate(value: Date): unknown {
  return Number.isNaN(value.getTime()) ? value : value.toISOString();
}

with its docblock recording that both live dialects produce that shape from rows already
on disk. No sentence in this diff claims the read door never hands out a Date; every
rewritten site carries the carve-out explicitly. This is the over-generalisation PR #16619's
own contract review returned a finding against, and it is deliberately not repeated.

B1 — the ruled-B consumer arms stay. Under the B1 ruling those arms are no-ops for the
valid-Date case, not conflicts. No arm is retired here; only the prose explaining why each
one exists is corrected. Measured, not assumed — the three helpers this prose documents
contain no tolerant ?? fallback and no alias over a mis-spelled key, only per-shape
normalisation of one value:

function usableCreatedAt(value: unknown): string | undefined { ... }        // service-storage
export function canonicalHolderCreatedAt(value: unknown): string | null {} // cli
function canonicalTimestampText(value: unknown): string { ... }            // metadata

That is also the card's written re-grade trigger, and it is NOT met: no prose site here
was measured to have caused a defensive coercion of the kind ADR-0053 forbids.

Three-list reconciliation, re-run on current origin/main

The card's list was measured at 56ee7aef1d; PR #16817 has landed since, so the census was
re-run rather than inherited. Landing was verified by content — every file of PR #16817
byte-identical to origin/main — with a comparator proven in both directions in the same
run (PR #16796 still differs on all six of its files).

list count disposition
issue #16728's seven (PR #16817) 7 already repaired on main; re-read each, none redone
triage's three extras 3 folded into PR #16817 before it landed; already repaired
this card's eleven 11 9 repaired here, 2 deferred (below)

The three extras are at sys-metadata-repository.ts:96/:178/:1196 and
database-loader.ts:42/:118 on today's tree — triage's :167 / :1182 / :41 drifted, as
the card warned. Nothing was anchored on a line number; every site was located by reading
the sentence.

The union was not assumed to be the whole set. An independent instrument was built to
test that: it classifies every out-of-package if (this.isSqlite) occurrence by the tense
of the verb tying it to the gate. Its firing control is that on origin/main it must name
the card's eleven and nothing else — it does, name for name, correctly excluding the nine
already-repaired blocks that still contain the token inside a true past-tense sentence.

FIRING CONTROL @ origin/main 7f745c3ffc     -> 11 false sites (== the card's eleven)
POST-EDIT      @ this branch                ->  2 false sites (== the deferred pair)

Sentence-level sweeps beyond the token — on normalizeSqliteDatetimeOutput,
AUDIT_TIMESTAMP_COLUMNS, repairNaiveUtcAuditTimestamp, formatOutput,
withPostgresCalendarDayAsText, "as a JS Date", and the SQLite-gated/SQLite-only prose
forms — surfaced no further site making this claim. What they did surface is recorded in
the acceptance notes below.

Deferred, declared

packages/metadata-protocol/src/protocol.ts — two sites, at :1641 and :1692 on
origin/main. Held by open draft PR #16796 (head bbe4590c0e), which was verified by
content to be un-landed and which does not itself correct them (they survive at :1719 and
:1770 on its head). Not touched here. Issue #16702 is the card behind that PR; it is not
addressed by this one and stays open.

packages/metadata/src/migrations/migrate-sys-notification-to-event.ts — this one was
deferred at the start and then un-deferred: PR #16834 landed mid-run and was verified by
content (all four of its files byte-identical to origin/main, same comparator, same firing
control). The site is therefore repaired here, at :495 on the post-#16834 tree — which is
exactly the disagreement the dispatch flagged. Both readings were right at their own
commits: :437 before PR #16834, :495 after. Issue #16312 is not addressed here and
stays open.

Which two were load-bearing

  • stranded-orphan-inventory.ts drew a conclusion for a live read door from the false
    premise, and went further: it stated that folding at the driver's read door "would reverse
    the deliberate withPostgresCalendarDayAsText decision" — which is what the ADR-0053 D-F1
    ruling authorised and did. Both halves are corrected.
  • migrate-sys-notification-to-event.ts wrote it as a numbered step in migration reasoning.
    Its conclusion is TRUE and is kept: that path reads through the raw-SQL seam, so
    formatOutput never runs on it and the dialect divergence genuinely survives there. Only
    the attached reason was wrong, so the step now names the seam instead of a gate.

The same true-half care applies to both packages/cli sites, for the same reason.

Evidence

Per-site absence of the removed claim, each with a firing control in the same run — the
exact removed string counted at HEAD/origin/main (must be 1) and in the worktree (must
be 0), plus a negative control string that was never in the tree:

[1] stranded-orphan-inventory.ts       HEAD=1 -> control FIRES ; worktree=0 -> GONE   PASS
[2] stranded-orphan-inventory.ts       HEAD=1 -> control FIRES ; worktree=0 -> GONE   PASS   (the D-F2 over-claim)
[3] stranded-orphan-inventory.test.ts  HEAD=1 -> control FIRES ; worktree=0 -> GONE   PASS
[4] cli/duplicates.ts                  HEAD=1 -> control FIRES ; worktree=0 -> GONE   PASS
[5] cli/duplicates.created-at-canonical.test.ts
                                       HEAD=1 -> control FIRES ; worktree=0 -> GONE   PASS
[6] protocol-14038-list-commits-created-at-iso.test.ts
                                       HEAD=1 -> control FIRES ; worktree=0 -> GONE   PASS
[7] protocol.commit-timeline-instant-order.test.ts
                                       HEAD=1 -> control FIRES ; worktree=0 -> GONE   PASS
[8] sys-metadata-repository-13997-authored-at-canonicalisation.test.ts
                                       HEAD=1 -> control FIRES ; worktree=0 -> GONE   PASS
[9] database-loader.test.ts            HEAD=1 -> control FIRES ; worktree=0 -> GONE   PASS
[10] migrate-sys-notification-to-event.ts
                              origin/main=1 -> control FIRES ; worktree=0 -> GONE   PASS
NEGATIVE CONTROL (a claim never in the tree) = 0 -> the instrument can answer NO

Tests and typechecks — real commands, real summary lines:

pnpm --filter @objectstack/service-storage test
    Test Files 38 passed (38)                   Tests 593 passed (593)
pnpm --filter @objectstack/metadata-protocol test
    Test Files 170 passed | 2 skipped (172)     Tests 2442 passed | 10 skipped (2452)
pnpm --filter @objectstack/metadata test
    Test Files 52 passed (52)                   Tests 799 passed (799)
pnpm --filter @objectstack/cli exec vitest run --project unit
    Test Files 186 passed (186)                 Tests 2555 passed | 6 expected fail (2561)
pnpm --filter @objectstack/cli exec vitest run --project integration src/commands/migrate/duplicates.created-at-canonical.test.ts
    Test Files 1 passed (1)                     Tests 6 passed (6)
pnpm --filter @objectstack/cli exec vitest run test/vitest-tiers-partition.test.ts
    Test Files 1 passed (1)                     Tests 22 passed (22)
pnpm --filter ... typecheck   (all four packages)   tsc --noEmit -> Done, x4
    check:test-typecheck: OK — @objectstack/cli's test layer compiles under tsconfig.test.json
    check:test-typecheck: OK — @objectstack/service-storage's test layer compiles under tsconfig.test.json

duplicates.created-at-canonical.test.ts is in the integration tier by the package's own
predicate (it value-imports @objectstack/driver-sql), so it was run there as well as
through the unit tier; the partition pin is green, confirming the comment-only edits move
no file between tiers (the predicate reads comment-masked source).

Gates — derived with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack,
all run, then reconciled with --ran. The reconciliation line, verbatim:

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

⚠️ That line accounts for COVERAGE, not verdicts. Verdicts, stated separately: 61 of 61
exit 0.
On the first pass 59 exited 0 and two exited 3 — PREREQUISITE NOT MET, which is
NOT MEASURED and not a pass
: check:dual-build-cjs-loads (12 packages had no dist/) and
check:i18n-coverage (12 of 13 configs could not lint). Both named their build closure, both
closures were built, and both were re-run to a real verdict:

pnpm check:dual-build-cjs-loads   EXIT=0
  ✓ 104 published require entry point(s) across 67 package(s) load; 620 emitted CommonJS
    file(s) parse; 102 require condition(s) resolve a CommonJS-flavoured `types` that exists.
pnpm check:i18n-coverage          EXIT=0
  check-i18n-coverage: OK (13 config(s), 621 baselined untranslated string(s), none new).

Every exit code above was captured before any pipe, per those gates' own warning.

Lint was run targeted over the nine touched TypeScript files
(eslint --no-inline-config --format json): 9 files linted, 0 errors, 0 warnings, exit 0.
The repo-wide pnpm lint sweep is CI's run, not this PR's local obligation.

The changeset, decided on a measurement

Route 2 (skip-changeset) does not apply: the label is for a diff that publishes
nothing from any released package, and this one publishes. Measured per package by grepping
each built dist/ for a distinctive phrase from the new prose, each with a firing control
proving the grep works on that dist/:

package what I touched reaches dist? named?
@objectstack/cli duplicates.ts (source) YES — verbatim in dist/commands/migrate/duplicates.js and .d.ts patch
@objectstack/service-storage stranded-orphan-inventory.ts (source) NO — bundle strips the internal usableCreatedAt JSDoc no
@objectstack/metadata migrate-sys-notification-to-event.ts (source) NO — bundle strips the internal canonicalTimestampText JSDoc no
@objectstack/metadata-protocol test files only NO — tests are not published no

Firing controls for the three NO rows: the enclosing identifier IS found in the same
dist/ (usableCreatedAt, canonicalTimestampText), and an exported declaration's JSDoc
DOES survive into dist/index.d.ts — so the absence is comment stripping, not a broken
probe. Source maps were checked too: no sourcesContent in any of the four, so no comment
text ships that way either. Same shape and same conclusion PR #16817 reached for its own
packages.

验收备注

Findings from the sentence-level sweep that are outside this card's class and are not
touched here:

  • .changeset/listdrafts-updated-at-canonical-iso.md:9 — a pending, unreleased
    changeset carries the same false sentence verbatim ("SqlDriver#formatOutput repairs them
    ... only inside its if (this.isSqlite) arm"). It will publish into
    @objectstack/metadata-protocol's CHANGELOG.md at the next release. Left alone on the
    boundary rule that a factual error on a release surface is a dedicated docs-only change,
    never a rider — flagged so somebody can decide before the next version cut.
  • A neighbouring, weaker drift class exists that this card does not cover: comments
    asserting the downstream consequence ("on Postgres and MySQL it materialises as a JS
    Date") without naming the gate. Examples read but not touched:
    sys-metadata-repository.ts:554 and :1915, db-queue-adapter.ts:35-52,
    db-queue-adapter-13993-idempotency-window-materialisation.test.ts:10,
    migrate-sys-notification-to-event.test.ts:264, temporal-conformance.ts:434. These are
    partly true (D-F2 at the client layer, D-F3 at the door) and they document ruled-B arms,
    which must not be retired as tidying — so they need a judgement, not a sweep.
  • stranded-orphan-inventory.test.ts:559 names a test "a JS Date (the Postgres/MySQL
    shape)". Test titles were left alone throughout, matching PR docs(engine): correct seven out-of-package comments that still gate formatOutput's timestamp passes on isSqlite #16817, which renamed none.

Noted, not filed. None of the three is a reproducible defect, a declared-contract
violation, or a metadata-authoring trap.


Generated by Claude Code

…tput's timestamp passes on isSqlite

WIP checkpoint before the verification laps.

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

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels 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/cli, @objectstack/metadata, @objectstack/service-storage, touching 1 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/metadata/src/migrations/migrate-sys-notification-to-event.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

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

  • content/docs/deployment/cli.mdx (via os migrate duplicates (command, read off packages/cli/src/commands/migrate/duplicates.ts))
  • content/docs/deployment/seed-tenancy-repair.mdx (via os migrate duplicates (command, read off packages/cli/src/commands/migrate/duplicates.ts))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17.mdx (via os migrate duplicates (command, read off packages/cli/src/commands/migrate/duplicates.ts))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/metadata/src/migrations/migrate-sys-notification-to-event.ts) — pages documenting those are invisible to this run
  • 1 anchor(s) matched too much of the corpus to be a work list: created_at (literal, 33 pages)
  • 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 — 34 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 8b672726ac0ae8f93e2009d8b53635c7b8d3df71packageMentionDocs.

Which tree this was computed on

This run read content/docs from 1496ec1ad4e0429badcf7c9d9aaaf3e3a228ffc9 — the merge of head 08f3a9cff0543c2759329b6c400c5c149de111b1 into base 8b672726ac0ae8f93e2009d8b53635c7b8d3df71, 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 1496ec1ad4e0429badcf7c9d9aaaf3e3a228ffc9 && git checkout 1496ec1ad4e0429badcf7c9d9aaaf3e3a228ffc9
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 8b672726ac0ae8f93e2009d8b53635c7b8d3df71 08f3a9cff0543c2759329b6c400c5c149de111b1 && git checkout -B drift-repro 8b672726ac0ae8f93e2009d8b53635c7b8d3df71 && git merge --no-ff 08f3a9cff0543c2759329b6c400c5c149de111b1

node scripts/docs-audit/affected-docs.mjs --json 8b672726ac0ae8f93e2009d8b53635c7b8d3df71

⚠️ 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 8b672726ac0ae8f93e2009d8b53635c7b8d3df71 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-musk
os-musk marked this pull request as ready for review September 8, 2026 12:26
@os-musk
os-musk enabled auto-merge September 8, 2026 12:26
@os-musk
os-musk added this pull request to the merge queue Sep 8, 2026
Merged via the queue into main with commit c5b7d84 Sep 8, 2026
38 checks passed
@os-musk
os-musk deleted the claude/issue-16818-isqlite-comment-drift-sweep branch September 8, 2026 12:52
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