Skip to content

feat(spec,driver-sql,cli): one physical representation for the NUMERIC column family, read by all three producers - #16887

Draft
os-musk wants to merge 7 commits into
mainfrom
claude/issue-16318-numeric-column-representation-table
Draft

feat(spec,driver-sql,cli): one physical representation for the NUMERIC column family, read by all three producers#16887
os-musk wants to merge 7 commits into
mainfrom
claude/issue-16318-numeric-column-representation-table

Conversation

@os-musk

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

Copy link
Copy Markdown
Collaborator

Fixes #16318

Clause-②: yes

Ruled C ∩ ④ (director seat, decision batch 86, 2026-09-08). One explicit per-field-type
physical-representation table lives in packages/spec, and all three producers read it:
SqlDriver.createColumn, os generate migration --format sql, and the typescript format.
New tables only — no existing column is retyped, no migration is planned, no backfill runs.

The two measurements the ruling named as pre-work

Both were taken before any number was chosen. Every reading below carries a firing control in
the same run, and every count was read from the harness's own printed count, never from a pipe.
Every reading is pinned to the PR head c0d4ed6f9f.

1. The three-producer table, re-run on PostgreSQL 16.13

One object, seven plain numeric declarations, three producers (initObjects; --format sql via
db.raw; the typescript format imported and its up(db) called), read back out of
information_schema.columns — with numeric_precision / numeric_scale, which is the half the
original report's bare data_type read hid.

BEFORE (c930f8597, the branch point):

column        driver                sql gen               ts gen
f_number      real                  numeric(18,2)         numeric(8,2)     DIVERGE
f_currency    real                  numeric(18,2)         numeric(8,2)     DIVERGE
f_percent     real                  numeric(5,2)          numeric(8,2)     DIVERGE
f_slider      real                  numeric(18,2)         numeric(8,2)     DIVERGE
f_summary     real                  numeric(18,2)         numeric(8,2)     DIVERGE
f_progress    real                  numeric(5,2)          numeric(8,2)     DIVERGE
f_rating      real                  integer               integer          DIVERGE
COUNT NUMERIC: diverge=7 agree=0 probed=7

CONTROL (the #16091 character family + boolean + date — expect all AGREE)
f_text        text                  text                  text             AGREE
f_email       character varying     character varying     character varying AGREE
f_boolean     boolean               boolean               boolean          AGREE
f_date        date                  date                  date             AGREE
COUNT CONTROL: diverge=0 agree=4 probed=4

AFTER, same command, same database:

f_number      numeric(65,30)        numeric(65,30)        numeric(65,30)   AGREE
f_currency    numeric(65,30)        numeric(65,30)        numeric(65,30)   AGREE
f_percent     numeric(65,30)        numeric(65,30)        numeric(65,30)   AGREE
f_slider      numeric(65,30)        numeric(65,30)        numeric(65,30)   AGREE
f_summary     numeric(65,30)        numeric(65,30)        numeric(65,30)   AGREE
f_progress    numeric(65,30)        numeric(65,30)        numeric(65,30)   AGREE
f_rating      integer               integer               integer          AGREE
COUNT NUMERIC: diverge=0 agree=7 probed=7   |   COUNT CONTROL: diverge=0 agree=4 probed=4

⭐ The divergence is wider than the card reported: six of the seven were a THREE-way split,
not a two-way one. table.decimal(name) with no arguments is knex's decimal(8, 2), so the two
halves of one command never agreed with each other either. The card's data_type read could not
see it.

2. The DECIMAL(5,2) truncation — measured, and the inference is CORRECTED

The order is explicit that this was an inference off the DDL literal and that a rounding or a
validating generator path re-opens the whole weighting. Executed on PostgreSQL 16.13:

value                                      numeric(5,2)   numeric(18,2)  real     numeric(65,30)
33.333   (whole-percent storage)           33.33 ALTERED  33.33 ALTERED  EXACT    EXACT
0.33333  (FRACTION storage — the default)  0.33  ALTERED  0.33  ALTERED  EXACT    EXACT
33.336   (rounds UP if it rounds at all)   33.34 ALTERED  33.34 ALTERED  EXACT    EXACT
12.34    (FIRING CONTROL — fits all)       EXACT          EXACT          EXACT    EXACT
COUNT control_exact=5/5

⚠️ The direction named was wrong and the substance holds. PostgreSQL rounds half-up
(33.336 arrives as 33.34); it does not truncate. The loss is silent either way.

And nothing upstream prevents it — this is the half that decides whether the weighting re-opens.
Driven through validateRecord itself:

percent,  NO declared scale   <- 0.33333        ACCEPTED   value UNCHANGED
percent,  max:100, no scale   <- 33.333         ACCEPTED   value UNCHANGED
currency, NO declared scale   <- 1234567.89     ACCEPTED   value UNCHANGED
number,   NO declared scale   <- 33.333         ACCEPTED   value UNCHANGED
CONTROL percent, scale: 2     <- 0.33333        REFUSED code=VALIDATION_FAILED
CONTROL number,  scale: 2     <- 33.333         REFUSED code=VALIDATION_FAILED
COUNT undeclared_accepted=4/4  control_refused=2/2  any_value_mutated=0/6

The generator path neither rounds nor validates. For a field that declares no scale the
COLUMN is the only thing deciding, and a narrow one silently alters data. The weighting that made
option A insufficient stands; only the word "truncate" was wrong. record-validator.ts states the
platform's own position on exactly this (#7501, maintainer ruling 2026-08-11): an over-scale value
"is refused the way an out-of-range one is; silent rounding is silently altering data".

⚠️ summary has NO seam at all — it is platform-computed and validateRecord's type door
excludes it — so for that member the column is the only guard there has ever been.

Where 65,30 comes from — both numbers are dialect maxima, not taste

Nine-value corpus, live PostgreSQL 16.13, written and read back through the driver's own pg type
parsing. Control: 12.5, a dyadic rational every candidate holds exactly — 0 of 5 lost it.

   real            3/9 altered   <- the driver today
   numeric(8,2)    9/9 altered   <- the typescript format today
   numeric(18,2)   7/9 altered   <- the sql format today
   numeric(38,17)  2/9 altered
   numeric(65,30)  0/9 altered   <- this table

real's three are the ones that matter: 1234567.89 reads back 1234567.9 and
Number.MAX_SAFE_INTEGER reads back 9007199000000000. That is the money-fidelity defect the
card named, as a reading rather than an argument.

65 and 30 are MySQL's documented DECIMAL maxima — the binding constraint among the dialects
this platform speaks (PostgreSQL's ceiling is 1000 digits, SQLite has none). ⚠️ An unconstrained
numeric is NOT portable: measured through knex's compilers, decimal(name, null) compiles to
decimal on pg, to float on better-sqlite3, and THROWS on mysql2 ("Specifying no
precision on decimal columns is not supported"). A stated pair is the only spelling all three
accept, which is what the ruling asked for.

The residual bound, stated rather than assumed. An exact decimal is bounded where a float is
not: magnitudes below 1e-30 round to zero and magnitudes at or above 1e35 are REFUSED, where
real kept about seven significant digits out to ~1e38. A refusal is loud and the rounding it
replaces was not, and the sql format's numeric(18,2) already refuses everything at or above
1e16 today.

THE FOSSILS, quoted

SqlDriver.createColumn's float arm — the fossil for the SQLite affinity choice:

rating/slider/progress are authored as numeric scalars (a star count, a slider position,
a percent-of-completion). Without an explicit case they fell to default → table.string,
giving the column TEXT affinity so SQLite coerced the written number to a string ('4' not 4) —
a silent type-fidelity leak the value-loss tests didn't catch. REAL affinity round-trips them
as JS numbers (#field-zoo).

The ADR-0113 reasoning at the end of createColumn, for the nullability half:

ADR-0113: the physical NOT NULL comes from the EXPLICIT storage constraint, not from
requiredrequired is the write-time contract enforced by the record validator at the
engine seam, and binding the DDL to it made every post-deploy tightening a destructive
migration.

Neither fossil contradicts these instructions. Both are carried into the new arms verbatim rather
than summarised away.

SQLite, per type — the constraint the card raised, answered

knex 3.3.0 / better-sqlite3, compiled DDL and live storage class:

   table.float(c)           -> float          real:4      real:4.5  real:33.333  real:0.33333
   table.decimal(c, 65, 30) -> float          real:4      real:4.5  real:33.333  real:0.33333
   table.integer(c)         -> integer        integer:4   real:4.5  real:33.333  real:0.33333
   table.string(c)          -> varchar(255)   text:4.0    <- the fossil's own leak, still there
COUNT identical_float_vs_decimal=1/1   COUNT text_affinity_leaks_outside_string_column=0

Per type, for every type moved out of the float arm:

  • number / currency / percent / slider / progress / summary — BYTE-IDENTICAL
    SQLite DDL to the float arm they leave. ColumnCompiler_SQLite3.prototype.decimal is the
    literal 'float', the same string floating resolves to. They keep REAL affinity, the fossil's
    leak stays defeated, and SQLite applies no precision and no scale — so the exactness this table
    buys is a PostgreSQL/MySQL property and SQLite behaves exactly as it does today.
  • rating — moves to INTEGER affinity. 4 is stored as the integer 4 rather than the real
    4.0, and 4.5 is still accepted as a REAL: SQLite refuses no fractional value, so nothing
    this dialect accepts today stops being accepted. The refusal rating gains is
    PostgreSQL/MySQL-only.

The table.string control in the same run still compiled to varchar(255) and still stored
text:4.0, so "identical" above is a discriminating reading and not a constant.

⭐ A blocking consequence nobody had measured: the READ path

Moving the driver's numeric columns to an exact decimal changes the JS type they read back as.
Measured on live PostgreSQL 16.13:

  c_real   typeof=number   value=12.5
  c_num    typeof=string   value="12.500000000000000000000000000000"
  c_int    typeof=number   value=4
  c_dp     typeof=number   value=12.5
COUNT numeric_is_string=1/1  real_is_number=1/1  (control: `real` still reports `number`)

node-postgres parses numeric to a STRING, and mysql2 does the same for DECIMAL. Driven
end-to-end through driver.create / driver.find, the first implementation returned 3 of 4
numeric fields as strings — a wire-contract break, since valueSchemaFor gives the whole class
z.number().finite().

formatOutput's numericFields pass already exists and already repairs exactly this, but it sat
inside the SQLite-only arm, on a premise stated in readPresentationKind's docblock: "The numeric
repair stays SQLite-only: it exists for legacy TEXT-affinity columns, which no other dialect has."
This change falsified that premise, so the pass now runs on every dialect and the fossil
sentence is corrected in place rather than left standing. After that, end-to-end:

  f_number    wrote 0.1234567890123456   read 0.1234567890123456   typeof=number   OK
  f_currency  wrote 1234567.89           read 1234567.89           typeof=number   OK
  f_percent   wrote 0.33333              read 0.33333              typeof=number   OK
  f_rating    wrote 4                    read 4                    typeof=number   OK
COUNT wrong=0/4

1234567.89 is the discriminating value: on the real column it read back 1234567.9.

The nullability half — landed HERE, not in a paired PR

Stated as the order requires. It lands in this PR because both generators' emitters are edited a
few lines apart from the numeric arms, and splitting them would put two halves of one file's
rewrite in two reviews. Both formats now take the physical NOT NULL from storage.notNull and
never from required. Driven on live PostgreSQL 16.13, four declaration shapes, three producers:

column            driver    sql gen   ts gen
f_required_only   YES       YES       YES   AGREE
f_storage_only    NO        NO        NO    AGREE
f_both            NO        NO        NO    AGREE
f_neither         YES       YES       YES   AGREE
COUNT nullability_diverge=0/4
CONTROL: the probe distinguishes its two verdicts (f_storage_only=NO, f_neither=YES)

This unblocks #16294 cause 1. That card's other two causes are not addressed here and stay
on it.

The "new tables only" bound, verified rather than asserted

A table built exactly as the pre-change driver built it (real numeric columns, rows in it) was
put in front of detectManagedDrift alongside a control table carrying the stale-textual shape
the detector DOES report:

COUNT legacy_findings=1  control_findings=2  total=3
  LEGACY   type_mismatch  name    (the pre-existing varchar(255) under a `text` field — unrelated)
  CONTROL  type_mismatch  f_tags  (multi-value field over a varchar column — the #11535 shape)
  CONTROL  type_mismatch  name    (the same unrelated varchar finding)
COUNT numeric_column_findings=0

0 drift findings for the four pre-existing real numeric columns, while the control fired
twice in the same run. Nothing here retypes an existing column, plans a migration, or starts
reporting drift over the difference — the additive sync only ever ADDS columns.

⚠️ The consequence that follows, stated because a reviewer will ask: a table that gains a NEW
numeric field after this lands carries a numeric(65,30) column beside its older real ones.
That is what "new columns only" means, and it is the shape the ruling chose — the alternative is
the migration of existing data it explicitly declined (「不考虑现有数据」).

Gaps that stay OPEN — stated, not silently assumed

  1. The value ranges in existing currency columns are unknown. No deployment was surveyed.
    The triage seat's 「无拉动」 is absence of evidence, not evidence of absence, and nothing here
    changes that.
  2. MySQL was not executed. Every MySQL claim in this PR is knex's compiled DDL plus MySQL's
    documented DECIMAL caps; no MySQL server was driven. PostgreSQL 16.13 and better-sqlite3 were.
  3. SQLite is measured for affinity and storage class, not for scale. SQLite applies neither
    precision nor scale, so "the six members are unchanged there" is a statement about affinity and
    DDL bytes — it is not a claim that SQLite gained exactness.
  4. --format sql remains a PostgreSQL-only claim (os generate migration's audit-stamp columns diverge from driver-sql — the generators emit NOT NULL where the driver emits nullable, and the SQL format emits TIMESTAMP where both knex paths yield timestamptz #15521). Neither generator reproduces the
    driver's dialect branching, and this PR does not change that.

Full file surface

Cross-lane by design — the type table cannot be split across two PRs without producing two
spellings of it.

packages/spec/src/data/numeric-column-representation.ts        NEW  the table + resolver
packages/spec/src/data/numeric-column-representation.test.ts   NEW  the totality pin
packages/spec/src/data/index.ts                                     one export line
packages/spec/api-surface/data.json                                 regenerated
packages/spec/export-origins/data.json                              regenerated
packages/drivers/driver-sql/src/sql-driver.ts                       createColumn + the read pass
packages/drivers/driver-sql/src/sql-driver-16318-numeric-representation.test.ts  NEW
packages/cli/src/commands/generate.ts                               both formats + nullability
packages/cli/src/commands/generate-numeric-column-representation.pin.test.ts     NEW
packages/cli/src/commands/generate-multiple-json-column.pin.test.ts  vehicle updated, see below
.changeset/numeric-column-representation.md                    NEW

⛔ No governed path is touched: nothing under docs/adr/, .claude/, skills/, AGENTS.md,
CLAUDE.md or content/docs/releases/.

⚠️ generate-multiple-json-column.pin.test.ts — the VEHICLE changed, the subject did not. That
pin is about multiple not deciding nullability; required was merely how a NOT NULL was spelled
when it was written. It now spells the constrained case with storage.notNull and asserts the
required-only case is nullable BESIDE it, which is the half that would catch this change being
silently reverted.

#16693 is in flight on adjacent text — left alone

Card #16693 has commits pushed against
packages/spec/src/conversions/registry.ts's field-required-notnull-explicit entry and the
ADR-0113 sentence in sql-driver.ts. ⛔ Neither was edited here. This PR quotes the ADR-0113
sentence and adds a separate #16318 note elsewhere in the file; the conversion registry is not
in this diff at all. If that text moves under this branch, the two land independently.

Verification

Red-before / green-after with the same command, both outputs quoted above (7 of 7 diverge → 0 of
7), plus:

  • Ablation of the new pin. NUMERIC_COLUMN_SCALE mutated 30 → 2 in the spec source, the
    mutation proved on disk (blob hash moved off the HEAD blob; the anchor count moved 1→0 and
    0→1), the pin run RED (exit 1, 1 failed / 4 passed), restored with git checkout HEAD --, the
    restore proved byte-identical to the HEAD blob with an empty git diff HEAD, and the restored
    leg run GREEN (exit 0, 5 passed). Trap-guarded on EXIT/INT/TERM with an absolute path.
  • pnpm --filter @objectstack/driver-sql test — 162 files passed, 10 skipped; 2449 tests passed.
  • pnpm --filter @objectstack/cli exec vitest run --project unit — 186 files, 2561 tests, all
    passing after the pin above was updated (it was RED first, on exactly the assertion this change
    reverses).
  • pnpm --filter @objectstack/cli exec vitest run --project integration on the generator pins —
    36 tests; the new pin is integration-tier (it value-imports the driver) and runs 8 of them.
  • pnpm --filter @objectstack/spec test — 465 files passed, 1 skipped; 12969 tests passed. (The
    new pin is 5 of them.)
  • typecheck for all three packages — exit 0.
  • pnpm --filter @objectstack/spec check:generated — was red on api-surface and
    export-origins, regenerated with the two commands it named, and both are committed.

Gate coverage was derived, not guessed —
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack, all 83 run,
then reconciled. The reconciliation line, verbatim:

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

⚠️ That line is COVERAGE, not verdicts. The verdicts, stated separately:
81 of 83 exit 0, and 2 exit 3 — NOT MEASURED, never a pass:

  • pnpm check:dual-build-cjs-loadsPREREQUISITE NOT MET — this gate reads built output, and some package has no dist/. It names 12 packages this diff does not touch (studio,
    client-react, the connectors, the plugins). It needs a whole-monorepo pnpm build, which does
    not fit this box's foreground budget; CI builds everything and runs it there. Declared, not
    skipped.
  • pnpm check:i18n-coverage — same cause, one layer out: os lint could not load
    examples/app-showcase/objectstack.config.ts because @objectstack/connector-mcp has no build
    output in this worktree. Its own text says "Nothing was compared… this result says NOTHING".

⚠️ A third gate, pnpm check:type-check-debt, first exited 3 with a V8
FATAL ERROR: Ineffective mark-compacts near heap limit under --max-old-space-size=4096 on this
shared box — its own text says that is "NOT a pass and NOT a finding". Re-measured at 8192 it
exits 0. The pass above is that second run, not the OOM.

The gate list was re-derived against a freshly fetched origin/main after the sweep: identical, 83
families, none added and none dropped, so nothing newly landed upstream is owed.

Level

minor on all three packages — a widening takes at least minor, and major is refused in the
launch window, so breaking-ness rides the BREAKING banner in the changeset plus its ADR-0087
disposition (not-required (no-migration-prescription): nothing an author writes is removed or
renamed, so there is no FROM → TO edit to prescribe). packages/drivers/driver-sql/src/ is a
NESTED package dir, which the level axis cannot see (#16713) — it is graded minor here anyway,
and that grading is the author's, not the gate's.

🤖 Generated with Claude Code

https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg

The container restart that killed the previous os-dev left this in the
worktree, uncommitted. Committed verbatim so it is not the only copy;
every number in it is re-measured before anything is claimed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
… for the NUMERIC column family

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

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
…(ADR-0113)

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

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
@github-actions github-actions Bot added size/xl documentation Improvements or additions to documentation protocol:data 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/driver-sql, @objectstack/spec, touching 17 documentable anchor(s). ⚠️ 3 changed file(s) yielded no anchor (packages/spec/api-surface/data.json, packages/spec/export-origins/data.json, packages/spec/src/data/index.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

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

  • content/docs/api/data-flow.mdx (via os generate (command, read off packages/cli/src/commands/generate.ts))
  • content/docs/data-modeling/drivers.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/data-modeling/index.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/deployment/cli.mdx (via os generate (command, read off packages/cli/src/commands/generate.ts))
  • content/docs/permissions/tenant-audit-census.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/plugins/packages.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/protocol/kernel/index.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/protocol/kernel/lifecycle.mdx (via SqlDriver (symbol, a top-level class), os generate (command, read off packages/cli/src/commands/generate.ts))
  • content/docs/protocol/objectql/query-syntax.mdx (via SqlDriver (symbol, a top-level class))

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

  • content/docs/releases/v17.mdx (via SqlDriver (symbol, a top-level class))

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
  • 3 changed file(s) yielded no anchor (packages/spec/api-surface/data.json, packages/spec/export-origins/data.json, packages/spec/src/data/index.ts) — pages documenting those are invisible to this run
  • 9 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 216 client-bound route-ledger rows — the other 156 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 156: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • 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 — 137 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 c1d8f98a5cdb6948d9ee2286e7f26f7d4b4922bapackageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json c1d8f98a5cdb6948d9ee2286e7f26f7d4b4922ba

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

Copy link
Copy Markdown
Contributor

Contract review (claude-fable-5-1, isolated seat) — PR #16887 @ c0d4ed6f9

Verdict: CHANGES REQUIRED. The code does what the ruling ordered — one table in packages/spec, all three producers resolve through it, no residual literal, no ALTER, no migration — and the SQLite pins are honest about what SQLite can measure. Two things the PR ships disagree with the repo: a protocol doc that now states the opposite of the new DDL, and a changelog sentence about MySQL that was never executed and that MySQL's documented conversion contradicts, while the live PG+MySQL harness the repo already runs in CI went unused.

Governed-surface check: docs/adr/**, .claude/**, skills/**, AGENTS.md, CLAUDE.md, content/docs/releases/** — none touched (git diff --name-only origin/main...c0d4ed6f9, 11 files, all under packages/** and .changeset/). packages/spec/src/conversions/registry.ts (#16693) not in the diff; the ADR-0113 sentence at the tail of createColumn is quoted, not edited. git merge-tree --write-tree origin/main refs/review/16887 → clean (origin/main = 142c01c88).

Findings

  1. blocking — the protocol doc now states the opposite of the DDL, and was not edited. content/docs/protocol/objectql/types.mdx:266-268 — "SQL driver: a floating-point column (REAL on PostgreSQL/SQLite, FLOAT on MySQL). precision/scale are validation and display metadata — the DDL does not emit NUMERIC(precision, scale)"; :312 the same for currency; the mapping table at :1177 (number/currency/percentREAL/FLOAT/REAL) and :1185 (summaryREAL). content/docs/references/api/sortability.mdx:68 — "summary is an engine-maintained table.float". After this PR a new column is numeric(65,30) on PG/MySQL and rating is integer. The Docs Drift bot on this PR says in its own caveat that types.mdx is exactly the page an emitter-only diff cannot list ("it was the page that diff falsified, in four places" — fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430), so the bot's green is not coverage here. The PR's whole argument is "one declaration, one column"; the protocol doc is now a fourth answer. content/docs/protocol/** is hand-written and ungoverned (AGENTS.md → Documentation Guardrails row for content/docs/<tree>/), so it belongs in this PR, not in a rider. Same fossil, lower stakes: packages/services/service-analytics/src/measure-result-type.ts:161 still records "driver-sql's DDL answers col = table.float(name)" as the reason summary needs no correction.

  2. blocking — the changeset asserts a MySQL refusal that was never executed, and MySQL's documented conversion says the opposite. .changeset/numeric-column-representation.md bullet 1: "rating is an INTEGER column on PostgreSQL and MySQL, so a fractional star count is now REFUSED there". The PR body's own gap 2: "MySQL was not executed … no MySQL server was driven." PostgreSQL refuses '4.5' into integer (text-param input syntax error). MySQL does not: a fractional value assigned to an integer column is rounded (4.5 → 5), with no error in strict mode, and a DECIMAL scale overflow is rounded with a Note (1265). That is a silent alteration on the dialect the changeset names as refusing — the exact class (A number field's declared scale is never enforced — values with more decimals are accepted and stored verbatim (min/max on the same field are enforced) #7501, "silent rounding is silently altering data") the table was chosen to end. The instrument exists in this repo: packages/drivers/driver-sql/src/live-dialect-matrix.testkit.ts (+ .globalsetup.ts) keyed on OS_TEST_POSTGRES_URL, used by 16 test files (e.g. sql-driver-aggregation-conformance.test.ts, sql-driver-column-order-dialects.test.ts), and run by the required check "Temporal Conformance (live PG + MySQL)" (.github/workflows/ci.yml:894, PG service + MySQL service conformance). Ask: either measure rating/DECIMAL on MySQL through that harness and pin it, or rewrite the bullet to say PostgreSQL refuses and MySQL rounds. A published changelog cannot carry a dialect claim the author marks unmeasured two sections later.

  3. record — no live pin for the property the PR is about. All three new tests are better-sqlite3 (sql-driver-16318-numeric-representation.test.ts, generate-numeric-column-representation.pin.test.ts, spec pin) and say so honestly ("⛔ Do not read the oracle as a precision check"). The numeric(65,30) DDL, the information_schema three-producer table, the nullability four-shape table and the read-path string→number repair on PG are all PR-body prose from a session, reproducible by nobody from the tree. The env-gated harness in finding 2 is where a numeric_precision/numeric_scale read and a typeof === 'number' read belong; on SQLite the read-path pin at sql-driver-16318-numeric-representation.test.ts:139-149 cannot fire (SQLite never hands back a string for a float column), so the every-dialect move in formatOutput has zero automated coverage on the two dialects it was made for.

  4. record — the read seam is binary64, and that bounds "money fidelity" end-to-end; the every-dialect pass also touches EXISTING columns. sql-driver.ts:17092-17099 does Number(v) on every string in numericFields on every dialect. Measured (node): for a value the driver itself wrote from a JS double the round-trip is exact — '1234567.890000000000000000000000000000'1234567.89, '12.500…'12.5 — because the shortest repr is what was stored. For anything not born as a JS double it is lossy and silent: '1234567890123456.123'1234567890123456; '9007199254740993.000…'9007199254740992; a full 30-scale '0.123456789012345678901234567890'0.12345678901234568. So the column is exact and find() is not: SQL-side writers, summary sums computed in SQL, anything ≥ 2^53. Not new as a wire limit (z.number().finite()), but the PR's "0/9 altered" is a column reading, not a find() reading, and the changeset should say the fidelity gained is exact-column-through-double. Second half: numericFields is seeded from NUMERIC_SCALAR_TYPES (sql-driver.ts:262), which includes the driver aliases integer/int/float for external/introspected columns. node-postgres returns int8/bigint as a string; before this PR that string left formatOutput untouched on PG, after it is Number()-ed — an existing external bigint column under a numeric field type silently rounds above 2^53 on PG. That is a read-path change to existing deployments outside "new tables only", unmeasured and unstated; state it or scope the pass.

  5. record — the residual bound understates its onset. PR/spec docblock: "magnitudes below 1e-30 round to zero". True, but PG rounds any digit past the 30th place silently, so a full-precision double loses trailing significant digits from about |x| < 1e-13 (1.2345678901234567e-15 stores as 0.000000000000001234567890123457). The bound to state is "≤ 30 fractional digits are kept", not "below 1e-30".

  6. record — ADR-0087 disposition rides the detector miss the gate's own docblock warns about. scripts/check-adr-0087-registration.mjs:141-147: no-migration-prescription is refused only when the body carries a prescription, and :199-206 (fix(service-package): classify a publish driver fault as 5xx and stop returning driver text as caller data (#8131) #8277) says an exemption held because the detector found nothing is the failure mode. Changeset bullet 3 is a prescription in substance: "A generated migration no longer emits NOT NULL for a field marked only required: true. Declare storage: { notNull: true } for a physical constraint" — a FROM (required: true) → TO (storage.notNull) edit a consumer must make to keep the column they had, written without the arrow/table framing the detector matches. "Nothing authorable moves" is true of keys; the output of os generate migration for an unchanged input moves (types and NOT NULL), which is what a consumer with checked-in migrations diffs. Either argue the exemption positively in the marker (the ledger is for migrate meta rewrites; this is an addition, not a rename — defensible) or register. Check Changeset is green on all matrix legs, consistent with a miss rather than a finding.

  7. observation — ruling compliance, itemised. (a) One table in packages/specnumeric-column-representation.ts:196-233. (b) Three producers read it ✓ — sql-driver.ts:16483-16489, generate.ts:1134-1136,1212,1243-1245 via numericSqlType, generate.ts:1985-1992; grep of both files at the ref finds no DECIMAL(/decimal(/float( literal for a FieldType: the remaining table.float (:16436) and table.integer (:16429) arms are the driver aliases float/integer/int, which are not in the FieldType enum (field.zod.ts:40-100). (c) New tables only ✓ — the three alterTable sites (:10128,12193,12462) add columns through createColumn; no type alter, no drift reader (schema-drift.ts has no numeric expectation; nullability drift reads storage.notNull, :849). (d) Both pre-work measurements are in the PR body, with controls; the 5,2 inference corrected to rounding. (e) rating integer with the half-star search stated in changeset ✓. (f) Nullability landed here rather than paired — the ruling says "same PR or a paired one", the dispatch says "the dev's call, stated either way"; stated ✓, within the ruling. (g) Deviation worth naming: the ruling asked per-type answers ("precision chosen for money", "a stated scale", "a scale that does not truncate"); the table is per-type in shape and uniform in value — 65,30 for all six, justified as the portable maximum. It satisfies each constraint; it is not what "chosen for money" reads as, and the maintainer should see that it was answered with "the ceiling".

  8. observation — totality and the pin. The pin (numeric-column-representation.test.ts:25-27) enumerates from NUMERIC_VALUE_TYPES, not a second hand list ✓, and NUMERIC_VALUE_TYPES is satisfies readonly FieldType[] (field-value.zod.ts:63-65) — a subset check, itself a hand list. A numeric type added to the FieldType enum but not to that set still falls to default → table.string in the driver; pre-existing, out of this table's reach, worth a sentence. summary belongs in the table (it is stored, was table.float, is in NUMERIC_VALUE_TYPES); "no seam" is true as stated — COMPUTED_VALUE_TYPES excludes it from validateRecord's type door — and the aggregateSummaryValue verbatim min/max over a text child (measure-result-type.ts) is now refused by an exact column exactly as real refused it.

  9. observation — numericSqlType runs at module evaluation. FIELD_TYPE_SQL_MAP is a module const (generate.ts:1134), so a missing entry throws when generate.ts is imported — every os command, not only generate migration. Loud by design; the blast radius is the CLI, and the spec pin fails first in CI.

  10. observation — tests, surface, hygiene. No .skip/.only/.todo in the diff. Ablation: NUMERIC_COLUMN_SCALE 30→2 fails exactly toBe(30) / toBeGreaterThan(2) / >= 5 in one it (:57-64); the other four compare against the constants and stay green — "1 failed / 4 passed" follows. generate-multiple-json-column.pin.test.ts keeps its subject (multiple does not decide nullability) and adds the required-only-is-nullable half. api-surface/data.json / export-origins/data.json: additions only — NUMERIC_COLUMN_PRECISION, NUMERIC_COLUMN_REPRESENTATION, NUMERIC_COLUMN_SCALE (const), NumericColumnRepresentation (type), numericColumnFor (function). Changesets minor ×3 with BREAKING banner; check-changeset-no-major has nothing to refuse; the packages/drivers/driver-sql/src/ level-axis blind spot ([finding] The changeset LEVEL axis is blind to every NESTED package: packages/*/src/** matches one segment, so 51 of 74 workspace packages (all drivers/services/adapters) can pair Clause-②: yes with patch and stay green #16713) is declared and the grade is the author's. Fixes #16318 in the body only; seven commits, none carries a closing trailer. CI at c0d4ed6f9: 55 of 56 check runs completed, all success or skipped (Build Core, Test Core 1–6, Temporal Conformance, Dogfood ×4, Type Check ×4, Check Changeset ×6, Governed Surface Queue Guard, Spec property liveness); "Lint & Repo Gates" still in_progress at read time. mergeable_state: blocked — draft, needs:contract-review with no approving review, one required check unfinished.

Maintainer-only merge: yes. This decides what every NEW table gets for money, percent and rating on PostgreSQL and MySQL — numeric(65,30) read through a JS double, integer for stars — under a ruling recorded as the director seat's reading of 「继续决策」 with "the maintainer may overturn on a word"; it is cross-lane (engine + cli), Clause-②: yes, and finding 2 puts a dialect claim in a shipped changelog that the maintainer's own MySQL deployments would contradict. The maintainer's eyes are warranted, and finding 1 and 2 should be fixed before they look.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants