Skip to content

fix(driver-sql): make the SQLite Field.json codec injective — one encoding across all three dialects - #12401

Merged
os-warren merged 2 commits into
mainfrom
claude/issue-12380-sqlite-json-injective
Aug 26, 2026
Merged

fix(driver-sql): make the SQLite Field.json codec injective — one encoding across all three dialects#12401
os-warren merged 2 commits into
mainfrom
claude/issue-12380-sqlite-json-injective

Conversation

@os-warren

Copy link
Copy Markdown
Collaborator

Fixes#12380

Implements route A of the 2026-08-26 maintainer ruling: JSON.stringify on write, JSON.parse on read, unconditionally, matching what Postgres and MySQL already do. This deletes a dialect branch rather than adding one. B (declare the current behaviour the contract) and C (narrow what can be written) are not taken.

What was wrong, measured live

Measured through the driver boundary on live SQLite (better-sqlite3), live Postgres 16.13 (server TZ Asia/Shanghai) and live MySQL 8.0.46 (server TZ +08:00), process TZ America/New_York, with every stored cell read back through a separate raw catalog query (typeof() / json_typeof() / json_type()) — never the emitted DDL.

Three independent mechanisms, only two of them reversible. Each was verified against origin/main before being built on:

#mechanismreversible
ARead-sideformatOutputJSON.parses every string in a json columnyes
BWrite-side — the column is declared type json, which under SQLite's affinity rules resolves to NUMERIC affinity, converting number-like strings to INTEGER/REAL before storageno
CNative booleanstrue read back as number 1; formatOutput's booleanFields pass is keyed to declared Field.boolean columns, not to booleans inside a json payloadyes

The contract settles which dialect is right, not strictness. json's stored contract is z.unknown() because "openness is now an explicit decision, not an accident of nobody checking" (packages/spec/src/data/field-value.zod.ts). An open contract admits both 123 and '123' as legal values of one field ⇒ no driver may collapse them.

Mechanism B decided the design, and it is pinned live rather than reasoned

No DDL change was needed. The column stays declared json, so NUMERIC affinity is still in force — and the encoded form defeats it, because a string's encoding carries its quotes ('123'"123", which is not a well-formed numeric literal).

That is not an argument in the PR body; it is a test. sql-driver-12380-json-roundtrip.test.ts binds the pre-fix value and the encoded value into the same column in the same INSERT statement, so nothing but SQLite's own affinity rule can account for the difference:

raw '0123' → integer (the affinity, caught in the act)
JSON.stringify('0123') → text "0123" (the encoding defeating it)

…for each of '123', ' 123 ', '0123', '1e5', '1.0', '-0'.

What changes on disk

For new writes the delta is exactly two classes: strings (now quoted JSON text) and booleans (now TEXT true/false instead of INTEGER 1/0). Objects, arrays, nulland numbers are byte-identical to before — 123 bound as a number and "123" bound as text both land as INTEGER 123.

⚠️An out-of-band reader of a SQLite file — anything reading the table with its own SQL rather than through this driver — now sees quoted JSON text where it saw a bare value.

The migration, and the limits it states plainly

backfillCanonicalJsonEncoding runs on syncSchema/initObjects for existing tables, in the same shape and posture as the backfillCanonicalDatetimes / backfillCanonicalTimes storage-format migrations beside it: one UPDATE per column, failures logged and swallowed, correctness never contingent on it having run.

It converts the one on-disk class the pre-fix encoding left unambiguous — a TEXT cell that is not valid JSON, which nothing but a stored plain string could have produced.

It does not guess, and the ruling's unrecoverable class is named rather than discovered:

  • INTEGER/REAL cells — a number, a boolean, and a number-like string eaten by NUMERIC affinity are the same bytes. 123 the number and '123' the string are one INTEGER 123; boolean true is one INTEGER 1 alongside the number 1. Cannot be restored, only stopped from growing.
  • TEXT cells that already parse — a stored object {"a":1} and a stored string'{"a":1}' were byte-identical before this change. Re-quoting would turn every legacy object and array into a string, corrupting the common case to guess at the rare one.

⇒ Those rows read after this change exactly as they read before it. The class stops growing; it is not retroactively repaired.

The migration changes no read. A legacy plain string reads back as that string both before it runs (via formatOutput's parse fallback, kept deliberately and now documented as the pre-#12380 read-side repair) and after. It is a canonicalisation that makes storage injective going forward, not a repair of something that reads wrong today.

Idempotence is by construction, and pinned. The WHERE is the exact complement of the SET's output: json_quote(X) of a TEXT value is a quoted JSON string, for which json_valid() is 1, so a converted row cannot match again. The suite runs the migration three times and asserts the disk bytes are identical after runs 2 and 3 — specifically that "America/New_York" never becomes "\"America/New_York\"".

Blast radius, read from source

sys_setting.value is Field.json and the settings service persists verbatim with no re-coercion by declared type, so the driver's answer is what the caller gets — on the dialect tenant environments actually run. driver-turso and driver-sqlite-wasm extend SqlDriver and override neither half of the codec (checked); both inherit the fix and their suites run green against the built dist.

Filters are unaffected: every scalar comparison operator on a json column is already refused by the driver (JSON_COLUMN_INCOMPATIBLE_OPERATORS$eq/$ne/$gt/$in/$nin/$between and their spellings), so no predicate could have been keyed to the old stored text.

Verification

Gate union derived, not recalled, at 56c22c0node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (re-derived after merging origin/main, because the first run reported a STALE TREE). Every matched family run in the foreground; every exit code captured before any pipe. All green. Each gate's own verdict line:

  • check-adr-0087-registration: 1 declared-breaking changeset(s), each carrying an ADR-0087 disposition. — the gate accepted not-required (no-migration-prescription). This is a declared-breaking change; the disposition is that no authorable metadata key is removed, renamed or re-shaped, so there is nothing for objectstack migrate meta to rewrite — the migration is over data rows and runs automatically.
  • check-driver-conformance: OK — 45 covered cell(s), 0 in the DEBT ledger, 0 exempt. / 9 of 9 dialect-scored cell(s) have a matrix-routed suite.
  • check-nul-bytes: OK (scanned 6859 text file(s) … no raw ASCII control bytes).
  • check-type-check-coverage --re-measure: OK — 32 ledger entr(ies) re-measured in 247.2s, 1843 raw tsc error(s) total, none above its recorded number. (needed the full workspace closure built; it refused outright before that rather than measuring a different world)
  • ✓ This diff introduces no 'major' bump. · ✓ No empty-frontmatter changeset introduced by this diff (1 declaring changeset(s) added).
  • check-engine-double-contract: OK — 415 pinned · where-matcher conformance holds: 302 matcher(s) · check-test-source-alias OK · OK: 18 package(s) read outside themselves, all declared — plus check:objectui-changeset, check:page-declaration-shape, check:published-files, check:slot-lookup, check:type-source-resolution, check:query-options-erasure, check:ci-filter-parity, check:comment-mask-adoption, check:plugin-teardown-shape, both docs-audit gates and release-rehearsal-clone --self-test, all exit 0.

Suites, all foreground:

suiteresult
new sql-driver-12380-json-roundtrip.test.ts103 passed (24 cases × 3 dialects + disk/read collision + catalog + affinity + migration)
driver-sql full145 files, 3020 passed, 1 skipped
driver-turso1006 passed
driver-sqlite-wasm395 passed
service-settings519 passed
platform-objects496 passed
driver-sql typecheckexit 0

The four consumer suites ran against a built driver-sql/dist carrying this change. Their first run failed to resolve@objectstack/driver-sql on an unbuilt worktree — reported here because that shape reads exactly like a regression and was not one.

Ablation — direction predicted in writing first, then measured

Predicted: restoring the deleted pre-fix branch goes RED, asymmetrically — SQLite only: 14 of the 24 round-trip cases on the SQLite cell (the 12 JSON-looking/number-like strings + n_true/n_false), the controls s_empty/s_nan/s_tz/s_bad/n_str and all numbers/objects/arrays/null staying green, plus SQLite's whole-matrix, disk-collision and read-collision tests and the migration suite's fresh-table test — 18 failures, with PG and MySQL 100% green.

Measured: 18 failed | 85 passed (103), and 0 FAIL lines on the postgres or mysql cells — all 18 in (sqlite) or the SQLite-only migration suite. Prediction matched exactly, and the PG/MySQL regression control fires: those dialects were 17/17 faithful before this change and are unaffected by both the fix and its removal.

Mutation proven on disk before any result was read, by single-line anchored grep counts: ABLATION-12380-MUTANT0 → 1, ONE encoding, every dialect (#12380)1 → 0, git diff --numstat10 42. No rebuild was involved or needed — the suite imports ./sql-driver.js, resolved to source, so no dist was in the loop and none could be poisoned; the result changing at all is itself the proof that the subject is the source file. The script carried a trap … EXIT INT TERM restore; the restore is verified byte-identical (git status empty, fix marker back to 1, mutant marker 0).

Out of scope

The VALUE_ROUNDTRIP census case-set (option D) is ruled in but sequenced second as a separate card — it is cross-driver by design and would ship red on SQLite by construction if it landed before this. Not added here; this fix carries its own pin instead.


Generated by Claude Code

…oding across all three dialects
`formatInput` now `JSON.stringify`s every `Field.json` value on every dialect and
`formatOutput` parses it back, deleting the SQLite branch rather than adding one.
Postgres and MySQL are untouched: this makes SQLite match what they already did.
Measured live (SQLite / PG 16.13 / MySQL 8.0.46), stored cell read back through a
separate raw catalog query: PG and MySQL were 17/17 faithful, SQLite 13/17
type-changed, through three mechanisms — the read-side parse, SQLite's NUMERIC
affinity on a `json`-declared column eating number-like strings before storage,
and native booleans landing as INTEGER 1/0. The declared contract decides which
dialect is right: `json`'s stored contract is `z.unknown()`, an explicitly open
contract that admits both `123` and `'123'` as legal values of one field.
`backfillCanonicalJsonEncoding` converges existing SQLite rows on `syncSchema`,
in the same shape and posture as the datetime/time storage-format backfills
beside it. It converts the one unambiguous on-disk class (TEXT that is not valid
JSON) and refuses to guess at the two that the pre-fix encoding made ambiguous.
Part of #12380
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W6HFzyH98W1YaQXhJUJt6o
@github-actionsgithub-actionsBot added size/l documentation Improvements or additions to documentation tests tooling labels Aug 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/driver-sql, touching 5 documentable anchor(s).

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

  • content/docs/data-modeling/drivers.mdx(via SqlDriver (symbol))
  • content/docs/data-modeling/index.mdx(via SqlDriver (symbol))
  • content/docs/plugins/packages.mdx(via SqlDriver (symbol))
  • content/docs/protocol/kernel/index.mdx(via SqlDriver (symbol))
  • content/docs/protocol/kernel/lifecycle.mdx(via SqlDriver (symbol))
  • content/docs/protocol/objectql/query-syntax.mdx(via SqlDriver (symbol))

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

  • content/docs/releases/v17.mdx(via SqlDriver (symbol))

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
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 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; 102 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 — 9 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 cdbd9204b65ae4ebec63f90f61669fb498efaa55packageMentionDocs.

Which tree this was computed on

This run read content/docs from 8fd0062ab7ff05fdd7e6aa8d377552ca5be564ba — the merge of head 56c22c04534c7183f2357c0ae9236f610103d029 into base cdbd9204b65ae4ebec63f90f61669fb498efaa55, 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 8fd0062ab7ff05fdd7e6aa8d377552ca5be564ba && git checkout 8fd0062ab7ff05fdd7e6aa8d377552ca5be564ba
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin cdbd9204b65ae4ebec63f90f61669fb498efaa55 56c22c04534c7183f2357c0ae9236f610103d029 && git checkout -B drift-repro cdbd9204b65ae4ebec63f90f61669fb498efaa55 && git merge --no-ff 56c22c04534c7183f2357c0ae9236f610103d029
node scripts/docs-audit/affected-docs.mjs --json cdbd9204b65ae4ebec63f90f61669fb498efaa55

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

@os-steveClaude

Copy link
Copy Markdown
Collaborator

Contract review (clause-② chain): PASS at head 56c22c04534c7183f2357c0ae9236f610103d029 — verdict and evidence recorded on #12380. needs:contract-review cleared on the card; the enqueue gate is satisfied for this head. If the head moves after this note, the gate re-arms. Landing remains with the domain:engine seat's normal review.


Generated by Claude Code

@os-warren
os-warren marked this pull request as ready for review August 26, 2026 02:31
@os-warren
os-warren added this pull request to the merge queueAug 26, 2026
Merged via the queue into main with commit 4045b95Aug 26, 2026
34 checks passed
@os-warren
os-warren deleted the claude/issue-12380-sqlite-json-injective branch August 26, 2026 02:49
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

3 participants

@os-warren@os-steve@claude