Skip to content

fix(driver-sql): richtext and code take an unbounded TEXT column, restoring the declared Rich Content grouping - #11876

Merged
hotlong merged 3 commits into
mainfrom
claude/issue-11794-richtext-text-family-emission
Aug 25, 2026
Merged

fix(driver-sql): richtext and code take an unbounded TEXT column, restoring the declared Rich Content grouping#11876
hotlong merged 3 commits into
mainfrom
claude/issue-11794-richtext-text-family-emission

Conversation

@huangyiirene

Copy link
Copy Markdown
Collaborator

Closes#11794

createColumn's text-family case listed text / textarea / html / markdown. richtext — the third member of the spec's own "Rich Content" grouping in field.zod.ts — was in neither that case nor JSON_COLUMN_TYPES, so it fell through to the catch-all's table.string(name): knex's varchar(255). Two of a three-member spec group got an unbounded column; the third got 255 characters. code had the identical defect and moves with it.

⭐ The clause ② judgment, measured against the fence

Triage ruled the design question in-fix and fenced it: "pick the direction that restores the declared grouping and state it in the PR; if it turns out to widen accepted physical shapes beyond the declared contract, stop and report."

The first pass of this branch took the wider option — richtext + code + signature + qrcode. That was measured against the fence and two of the four failed it. This PR ships two, not four.

The test that decides membership

A type may take an unbounded TEXT column exactly when the WRITE SEAM enforces its declared maxLength. That is not a new rule invented here — schema-drift.ts already rests on it, in as many words:

A TEXT column refuses nothing a maxLength allows, so there is no divergence to plan an ALTER for; the bound is enforced at the write seam.

If that premise is false for a type, an unbounded column stops being a restoration of the contract and becomes a widening of it.

Measured, not read off the source — validateRecord with { type: T, maxLength: 64 } and a 100-character value:

typewrite seamverdict
text / textarea / markdown / htmlREFUSED VALIDATION_FAILED "f must be ≤ 64 characters (got 100)"already in the family
richtextREFUSED, same envelopemoves — contract restored
codeREFUSED, same envelopemoves — contract restored
signatureACCEPTED — no max_length branch⛔ stays put
qrcodeACCEPTED — no max_length branch⛔ stays put

maxLength is a plain optional key on FieldSchema, admitted on every type, so Field.signature({ maxLength: 64 }) parses and binds nothing. For those two an unbounded column would trade an under-accepting column for an over-accepting one — over-accepting past the declaration on every dialect. richtext and code keep a field-named ADR-0112 refusal before the column is reached, so their declared contract (valueSchemaForz.string(), unbounded) is what the column now expresses.

Every candidate type, before and after, on every dialect

Read back from information_schema / PRAGMAnever from the emitter — on live MySQL 8.0.46 (STRICT_TRANS_TABLES, utf8mb4) and Postgres 16. The write column is a 1000-character value.

typedialectbeforewriteafterwrite
richtextsqlitevarchar(255)accepted¹TEXTaccepted
richtextpostgrescharacter varying(255)REFUSED 22001textaccepted
richtextmysqlvarchar(255)REFUSED ER_DATA_TOO_LONGtextaccepted
codesqlitevarchar(255)accepted¹TEXTaccepted
codepostgrescharacter varying(255)REFUSED 22001textaccepted
codemysqlvarchar(255)REFUSED ER_DATA_TOO_LONGtextaccepted
signaturepostgrescharacter varying(255)REFUSED 22001character varying(255)REFUSED 22001
signaturemysqlvarchar(255)REFUSED ER_DATA_TOO_LONGvarchar(255)REFUSED ER_DATA_TOO_LONG
qrcodepostgrescharacter varying(255)REFUSED 22001character varying(255)REFUSED 22001
qrcodemysqlvarchar(255)REFUSED ER_DATA_TOO_LONGvarchar(255)REFUSED ER_DATA_TOO_LONG

¹ SQLite enforces no varchar width, which is why the enforcing dialects carry the claim.

Positive control — the grouping already held for two of three, and this change moved only what it claims: markdown / html / text are text on postgres and mysql and TEXT on sqlite, before and after, with the 1000-character write accepted in every one of those six cells. Negative controlssecret / color / select / string stay varchar(255) and are still refused by the server at 1000 characters, in the same runs — which is what makes the acceptance above non-vacuous rather than a lenient session.

The half left open, and it is asserted rather than hidden

signature and qrcode have the same live defect (a data-URI PNG is refused at 255 characters today) and it is not fixed here. The live-dialect suite now asserts that refusal out loud — "records the STILL-OPEN half" — so the cost of the choice is in the test file rather than in a comment nobody runs. The contract decision they need is filed separately as #11875, which is out of scope for this branch and is not addressed by it.

⚠️ The #11565 agreement pin — the owed change turned out not to be owed

The dispatch and the claim comment both said sql-driver-11565-row-byte-budget.test.ts ("agrees with createColumn about every FieldType the spec declares") "currently pins the present behaviour, this defect included" and needed its expectation updated here.

Checked on origin/main: that premise is wrong, and I am flagging it rather than quietly not doing it. The pin is agreement-shaped, not behaviour-shaped — it reads FieldType.options from the spec and compares varcharColumnChars against columnInfo() column by column. It hard-codes no per-type physical shape, so there is no expectation to edit; it stays green as long as both declaration sites move together, and turns red by design if only one does. Verified by running it, not by reading it: 10/10 green on the pre-change tree, 10/10 green after, and 10/10 green under the ablation below. The file is unchanged by this PR.

That safety net is why both sites move here: the varchar-width mirror (varcharColumnChars) and createColumn itself.

Closing the root cause, not just the instance

The card's own diagnosis is that "the hand-maintained list is what let one member of a three-member spec group diverge silently." A one-type fix leaves that intact. So the set of types that take an unbounded column when unkeyed is now pinned as a whole, over every FieldType the spec declares — adding a type to the text family or to JSON_COLUMN_TYPES fails until the new membership is stated on purpose. It carries its own non-vacuity guards (the registry really was read; the filter really matched).

#11374 semantics preserved for the new members

Unchanged for richtext / code: keyed and bounded ⇒ varchar(maxLength) so a declared index still keys on MySQL; keyed and unbounded ⇒ TEXT and the named explainUnkeyableTextColumn refusal, never a silently weaker constraint; unkeyed ⇒ TEXT, bound or not. Nothing plans an ALTER on an existing column — createColumn runs on CREATE TABLE and ALTER TABLE ADD COLUMN, so the column it sizes is always empty.

A pre-existing factual error corrected in passing

The catch-all's own comment listed code among the option-valued types whose stored value is not the declared value. Measured in field-zoo, code stores the editor's contents verbatim. The comment now says so instead of leaving a stale sentence next to changed behaviour.

Verification — all at 161cd39112

Ablation. Direction predicted before the run: removing 'richtext' + 'code' from both lists turns this file RED on the SQLite physical-shape test, the set pin, the keyed-semantics test and both live cells; the SQLite round-trip stays GREEN (SQLite enforces no varchar width); the #11565 pin stays GREEN (it pins agreement, not shape). Mutation proven on disk before any result was read, by anchored grep counts on the text I intended to change — ^ case 'richtext':$ 2 ⇒ 0 and ^ case 'code': 2 ⇒ 0, with the injected bare-brace anchor present — because a zero-hit edit exits 0 and reads like a successful ablation. Restore under trap … EXIT INT TERM from a byte copy taken before the mutation; clean tree verified afterwards (git status --porcelain empty, counts back to 2 / 2). Observed: exactly the prediction — 5 failed / 14 passed across the three files; SQLite round-trip green, #11565 green. No rebuild leg exists for this pair and that is a property of the resolution path, not an omission: the suite imports ../src/index.js and the package's vitest.config.ts aliases @objectstack/spec to src, so nothing here resolves through dist.

Gate union — derived, not recalled: node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack at 161cd39112 (3 paths vs merge base 589758d22). Every family it named was run, exit codes captured before any pipe; all green:

check:changeset-gate-self-tests · check:driver-conformance · check:objectui-changeset · check:published-files · check:slot-lookup · check:test-source-alias · check:type-source-resolution · check-adr-0087-registration · check-changeset-no-major · check-empty-changeset · check-plugin-teardown-shape · docs-audit/check-affected-docs · docs-audit/check-drift-comment · pm/release-rehearsal-clone --self-test — plus the convention-triggered set for "adds or edits a test file": check:query-options-erasure · check:type-check-coverage · check:engine-double-contract · check:cross-package-test-inputs · check:where-matcher · check-nul-bytes, and the ratchet check:type-check-debt on the built workspace closure (check-type-check-coverage --re-measure: OK — 32 ledger entries re-measured, 1898 raw tsc errors total, none above its recorded number).

Suites.pnpm --filter @objectstack/driver-sql exec vitest run135 files passed, 2713 passed / 1 skipped, with the live cells provisioned as CI does (OS_EXPECT_LIVE_DIALECT_MATRIX=1, MySQL time_zone='+08:00', TZ=America/New_York; without that provisioning the temporal-conformance and MySQL-datetime suites correctly refuse to prove anything on a UTC server). pnpm --filter @objectstack/driver-sql typecheck clean. Repo-wide pnpm lint (eslint . --no-inline-config) clean in 85s — run whole, so no narrowing to justify.

Driver-conformance ledger (lane standing promise) — read before and after: OK — 45 covered cell(s), 0 in the DEBT ledger, 0 exempt, identical on both sides. The "before" reading was taken on a detached worktree pinned at 589758d22c, since removed.

Serial / region

Re-enumerated at this push, not reused: all open PR heads fetched (+refs/pull/*/head:refs/remotes/pr/*) and read with git diff --name-only origin/main...refs/remotes/pr/<n>. Exactly one open PR touches packages/drivers/driver-sql/src/sql-driver.ts#11785, at @@ 7750 / 7765 / 7794 (aggregate lowering). This branch's hunks are @@ 13385 / 13679 / 13828: ~5,500 lines apart, no shared symbol. #11827 is no longer open. Positive control: the same matcher returns 11 files for #11699, so it is not silently matching nothing. aggregate(), introspectUniqueConstraints and the dialect getters are untouched.

Same-switch neighbours #11431 and #11374 are not addressed here and remain open; neither has a live PR in this file.


Generated by Claude Code

…11794)
INCOMPLETE — preserved so it is not lost with the container. Not for review.
Done:
- `richtext`, `code`, `signature`, `qrcode` added to BOTH declaration sites:
the varchar-width mirror and `createColumn`. Adding to only one turns the
mirror/createColumn agreement pin red by design.
- `secret` (opaque sys_secret ref, ADR-0100) and `color` deliberately left in
the catch-all, with reasons recorded at the site.
- Corrects a pre-existing factual error: the catch-all comment listed `code`
as option-valued; measured in field-zoo it stores editor contents verbatim.
- 199-line test file, not yet run in a full suite.
Still owed:
- the #11565 agreement pin expectation update
- changeset, gate union, ablation
- the clause-2 judgment on whether admitting code/signature/qrcode widens the
accepted physical surface beyond the declared contract. Triage's fence:
if it does, stop and report.
Resume this branch; do not restart from scratch.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VK8rFDtg8eREaxBGX99Csn
`createColumn`'s text-family case listed `text` / `textarea` / `html` /
`markdown`. `richtext` — the third member of the spec's own "Rich Content"
grouping in `field.zod.ts` — was in neither that case nor `JSON_COLUMN_TYPES`,
so it fell through to the catch-all's `table.string(name)`: knex's
varchar(255). Measured at 1000 characters on live MySQL 8.0.46 and Postgres
16, the write was refused by the server (`ER_DATA_TOO_LONG` under
`STRICT_TRANS_TABLES`, `22001`) while the same body in a `markdown` field on
the same table was accepted. `code` had the identical defect and moves with
it.
Membership is now decided by a stated, measured test rather than by the
hand-maintained case list that let one member of a three-member spec group
diverge in the first place: a type may take an unbounded TEXT column exactly
when the WRITE SEAM enforces its declared `maxLength` — the invariant
`schema-drift.ts` already rests on. objectql's record-validator applies its
`max_length` branch to `text` / `textarea` / `email` / `url` / `phone` /
`password` / `markdown` / `html` / `richtext` / `code` and to nothing else.
`signature` and `qrcode` are deliberately NOT moved: nothing enforces their
declared `maxLength` at the write seam, so an unbounded column would accept
values the declaration forbids — a physical surface wider than the contract
rather than a restoration of it. Their own defect stays open and is asserted
out loud in the live-dialect suite instead of being left undocumented.
Both declaration sites move together — the varchar-width mirror and
`createColumn` — because their agreement is pinned by
`sql-driver-11565-row-byte-budget.test.ts`. The set of types that take an
unbounded column when unkeyed is now pinned as a whole, so the next addition
has to be stated on purpose.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VK8rFDtg8eREaxBGX99Csn
@github-actionsgithub-actionsBot added size/m documentation Improvements or additions to documentation tests tooling labels Aug 24, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 45 of 222 client-bound route-ledger rows — the other 177 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: 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 4019e16cdc9678517a2b44f2123403deef703ac4packageMentionDocs.

Which tree this was computed on

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

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

@huangyiireneClaude

Copy link
Copy Markdown
CollaboratorAuthor

Ready for triage review — ⛔ deliberately NOT enqueued

domain:engine PM seat, session session_01VK8rFDtg8eREaxBGX99Csn. ACCEPTed at 5402476458. CI 31/31, zero failures at head 161cd39112 (⚠️ including Test Core (1/6), the shard #11808 kills — it passed at 22:47:21Z). Flipped to ready with draft: false read back. ⛔ Auto-merge deliberately not enabled — the maintainer directed this to triage review rather than being enqueued by this seat.

⭐ What a reviewer should look at first — the fence firing IS the result

This card was dispatched below CONTRACT_REVIEW_TIER by an explicit maintainer decision (quota exhausted; ruling recorded verbatim at issue comment 5401803355, ⛔ not a precedent). The compensation was that the one unmeasured contract judgment got fenced harder — and it changed the answer.

An earlier pass of this branch had taken the wider option: richtext + code + signature + qrcode. Measured against triage's fence for the first time, 2 of the 4 failed, and the two were reverted. ⇒ Had this shipped as first written, it would have widened the accepted physical surface past the declared contract — exactly what triage forbade.

The criterion is a mechanism, not an analogy — worth verifying independently

A type may take an unbounded TEXT column exactly when the write seam enforces its declared maxLength.

⛔ Not invented here — schema-drift.ts already rests on it: "A TEXT column refuses nothing a maxLength allows … the bound is enforced at the write seam."

The one-line check a reviewer can run: packages/objectql/src/validation/record-validator.ts:554 lists text / textarea / email / url / phone / password / richtext / code (with markdown / html) — and ⛔ notsignature, ⛔ notqrcode. So for those two an unbounded column would trade an under-accepting column for an over-accepting one, since Field.signature({ maxLength: 64 }) parses and binds nothing.

Three things that make this reviewable rather than assertive

  1. Every candidate enumerated before/after on all three dialects, read from information_schema / PRAGMA, ⛔ never the emitter — including the two that did not move, so the revert is visible as a measurement.
  2. Non-vacuity is built into the live cells: the same oversized value into a column deliberately left at varchar(255) is refused by the server in the same run — which is what stops a mis-provisioned lenient MySQL session from passing the acceptance while measuring nothing.
  3. The still-open half is asserted, not hidden: a test named "records the STILL-OPEN half" pins that an oversized signature is refused today, with a comment saying ⛔ it is "not a wish and not a quarantine" and naming what turns it red. The contract decision those two need is filed separately as signature / qrcode have no maxLength enforcement anywhere, so they cannot join the TEXT family — a data-URI signature is refused at 255 chars and the declared bound binds nothing #11875.

⚠️ One thing the PR corrects about MY instructions

My dispatch briefs stated — twice — that the #11565 agreement pin "currently pins the present behaviour, this defect included" and needed its expectation updated. That was wrong. The pin is agreement-shaped: it reads FieldType.options and compares varcharColumnChars against columnInfo(), hard-coding no per-type shape, so it stays green when both declaration sites move and goes red when only one does. The dev pushed back and verified by running it 10/10 before, after, and under ablation rather than reasoning about it. ⇒ The file is correctly untouched; ⛔ do not expect a diff there.

Scope

Closes #11794. Three files. Same-switch neighbours #11431 and #11374 are not addressed and remain open. Serial re-enumerated at this push (⛔ not reused): the only open PR touching sql-driver.ts is #11785 at 7750/7765/7794 against this branch's 13385/13679/13828 — ~5,500 lines apart, no shared symbol, positive control included.


Generated by Claude Code

@hotlong
hotlong added this pull request to the merge queueAug 25, 2026
Merged via the queue into main with commit b826390Aug 25, 2026
32 checks passed
@hotlong
hotlong deleted the claude/issue-11794-richtext-text-family-emission branch August 25, 2026 05:41
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

driver-sql: richtext is emitted as varchar(255) while its markdown/html siblings get TEXT — a rich-text body is capped at 255 characters

3 participants

@huangyiirene@hotlong@claude