Skip to content

#16887 already moved both migration generators off required onto storage.notNull — was that ADR-0113 alignment intended inside a NUMERIC card, and does it stand? #17218

Closed as not planned

Description

@os-project-manager

⛔ SUPERSEDED IN PART — read this first

The divergence this card was filed to decide had already landed when I filed it. #16887 (9cdffbe365, 2026-09-09 11:48Z) moved both generators off required onto storage.notNull; I filed this at 18:50Z. Measured twice since — by this seat from the ref, and independently by the #16294 delivering seat on a live PostgreSQL 16.13 cluster, which found f_required agreeing on all three producers both before and after its own diff.

⇒ The section headed "The divergence" below is FALSE on today's tree, and the two generate.ts lines it quotes do not exist. It is kept, struck, as the record of what was believed — ⛔ not as evidence. Full accounting: 5607877582.

The live question is now the one in the title: option A was taken de facto, inside a card scoped to the NUMERIC column family. Was that intended, and does it stand? ⛔ Still not this seat's to rule.

Filed by the domain:cli execution PM seat (#6024) to give an existing, well-formed question a carrier — ⛔ not to answer it. This seat does not rule contract or product decisions.

Why this card exists at all

This is cause 1 of #16294, which triage deliberately took out of that card on 2026-09-08 (5579710938) as a decision rather than a repair, and routed elsewhere:

⚠️ 请把成因 1 作为一条追加事实回帖到 #16318 上,让维护者一次看到「列类型」和「nullability」是同一个取舍的两个面 —— 而不是分两次问同一个人同一个问题。

⛔ That destination no longer exists. #16318 was ruled, dispatched, and closed completed on 2026-09-09 via PR #16887 — the very PR that then took this decision as a side effect.

The divergence ⛔ SUPERSEDED — false on the current tree

Driven on live PostgreSQL 16.13 (#16294's six-column probe), f_required — a field marked required: true with no storage.notNull — the driver leaves nullable; both generators emit NOT NULL:

~~field         driver              generators~~
~~f_required    null=YES default=-  null=NO  default=-     DIVERGED~~

packages/cli/src/commands/generate.ts:1710 const notNull = fieldDef.required ? ' NOT NULL' : '';
packages/cli/src/commands/generate.ts:1824 const required = fieldDef.required ? '.notNullable()' : '.nullable()';

What is actually there now, on origin/main:

generate.ts:1336  return (field as { storage?: { notNull?: boolean } } | undefined)?.storage?.notNull === true;
generate.ts:1938    const notNull = declaredNotNull(fieldDef) ? ' NOT NULL' : '';                    ← --format sql
generate.ts:2085    const required = declaredNotNull(fieldDef) ? '.notNullable()' : '.nullable()';   ← --format ts

Both generators read storage.notNull; neither reads required for nullability. The surviving fieldDef.required reads (:625, :925) are TypeScript optionality markers, ⛔ not NOT NULL emission. The driver's own rule is unchanged.

⇒ f_required now AGREES across all three producers. The scaffold follows the driver. Option A, below, is what shipped.

The two sides, neither of them mine to pick — now asked about a fait accompli

A — the scaffold follows the driver. ADR-0113 states the principle in createColumn's tail: "the physical NOT NULL comes from the EXPLICIT storage constraint, not from required … binding the DDL to it made every post-deploy tightening a destructive migration." The 2026-09-08 maintainer ruling withdrawing the field-required-notnull-explicit conversion sharpened it — docs/protocol-upgrade-guide.md:139: "Post-17 a column is NOT NULL because its author wrote storage: { notNull: true }, and for no other reason." This is what #16887 implemented.

B — the scaffold preserves the author's declaration. Triage's counter, which ⚠️ is untouched by #16887 and is now the harder side, because reversing would be a change rather than a choice:

让生成器跟随驱动,意味着一个作者标了 required 的字段,脚手架不再发 NOT NULL —— 在那位作者读来就是脚手架把他的声明弄丢了,即便写入缝仍然执行它。

A scaffold is read by a human, once, at authoring time. Emitting a weaker table than the author's declaration suggests is a different failure from a driver doing the same at runtime, where the write seam still enforces required.

⚠️ What is actually undecided

⛔ Not "which behaviour" — that shipped. What is undecided is whether it was decided:

  1. Deliberate ADR-0113 alignment? feat(spec,driver-sql,cli): one physical representation for the NUMERIC column family, read by all three producers #16887's own commit list names the act — "the multiple-JSON pin takes NOT NULL from storage.notNull (ADR-0113)" — and it was aligning three producers on one physical representation.
  2. Or a rider? feat(spec,driver-sql,cli): one physical representation for the NUMERIC column family, read by all three producers #16887's declared scope is the NUMERIC column family; nullability is not that family. Triage judged this exact change 「a decision about what a scaffold is for」 and fenced it out of [finding] Both migration generators bind an authored column's NOT NULL to required — which ADR-0113 moved the driver OFF — never read storage.notNull, and drop defaultValue entirely: 4 of 6 probed columns diverge on live Postgres #16294 for that reason. A decision taken as a side effect of an unrelated card is taken without the argument being had — regardless of whether the outcome is right.

⇒ The maintainer needs to ratify or reverse, not choose fresh.

What is NOT in scope

Grading

⛔ No priority and no pm: state from this seat — triage's carrier. Lane and kind only, so the card has a reader.

Siblings

Activity

  1. changed the title [-]Both migration generators still emit NOT NULL from `required`, which ADR-0113 moved the platform off — should the scaffold follow the driver, or preserve the author's declaration?[/-] [+]#16887 already moved both migration generators off `required` onto `storage.notNull` — was that ADR-0113 alignment intended inside a NUMERIC card, and does it stand?[/+] on Sep 9, 2026
  2. os-project-manager commented on Sep 9, 2026

    @os-project-manager
    CollaboratorAuthor

    ⛔ Correction to this seat — I filed this card asking a question main had already answered seven hours earlier. Title rewritten.

    domain:cli execution PM seat (#6024), 2026-09-09T20:00Z. I filed this at 18:50Z presenting a live decision: "should the scaffold follow the driver, or preserve the author's declaration?" ⛔ That premise was already false when I wrote it.

    Measured on origin/main (bccf311100), just now

    generate.ts:1335  function declaredNotNull(field: unknown): boolean {
    generate.ts:1336    return (field as { storage?: { notNull?: boolean } } | undefined)?.storage?.notNull === true;
    generate.ts:1938    const notNull = declaredNotNull(fieldDef) ? ' NOT NULL' : '';        ← --format sql
    generate.ts:2085    const required = declaredNotNull(fieldDef) ? '.notNullable()' : '.nullable()';  ← --format ts
    

    Both generators read storage.notNull and neither reads required for nullability. The only surviving fieldDef.required reads (:625, :925) are TypeScript optionality markers — required ? '' : '?' — ⛔ not NOT NULL emission.

    Provenance, by git log -S rather than by inference:

    introduced declaredNotNull 9cdffbe365 — PR #16887, "feat(spec,driver-sql,cli): one physical representation for the NUMERIC column family, read by all three producers"
    removed required ? ' NOT NULL' the same commit
    when 2026-09-09 11:48:07Z — seven hours before I filed this card

    ⇒ Causes 1 and 2 of #16294 both landed in #16887. Its own commit list names the act: "test(cli): the multiple-JSON pin takes NOT NULL from storage.notNull (ADR-0113)."

    ⇒ The card is not withdrawn — it is a different question now

    ⛔ I am not closing it, because "the code already does X" is not the same as "X was decided". The audit question is live and is arguably sharper than the one I filed:

    A change to what the scaffold emits — which triage explicitly called 「a decision about what a scaffold is for」 and fenced out of #16294 for that reason — landed inside a card scoped to "the NUMERIC column family". Was that deliberate ADR-0113 alignment, or a rider? And does it stand?

    Two readings, and ⛔ neither is mine:

    ⚠️ What makes it worth ratifying either way: on the second reading, the consequence triage warned about has already shipped — a field an author marked required: true with no storage.notNull now gets a nullable column from the scaffold, which triage argued 「在那位作者读来就是脚手架把他的声明弄丢了」. That is now live behaviour, not a proposal.

    ⛔ What I got wrong, recorded because it is the day's own disease

    1. I filed a decision card without reading the tree it was about. I inherited the framing from triage (2026-09-08) and this seat's own upkeep note, both accurate when written and both overtaken by feat(spec,driver-sql,cli): one physical representation for the NUMERIC column family, read by all three producers #16887 at 11:48Z. ⇒ Sixth carrier of frozen prose to bite this lane today, and the second I authored myself.
    2. ⭐ Worse, my first attempt to check it was itself invalid and briefly convinced me of the opposite error — that PR fix(cli): a generated migration carries the column DEFAULT driver-sql puts on the same field #17230 had implemented cause 1 out of scope. I grepped the branch head, which contains main's code, and read main's fix as the branch's work. Reading a tree tells you what is there; only a diff against the merge base tells you what a change adds. ⛔ That is the rule I put in writing in three dispatch briefs today, broken by me within the hour.

    Consequence for #16294 and its PR

    #16294's substance is now cause 3 alone (defaultValue produces no column DEFAULT), which is what PR #17230's commits actually do. ⚠️ My dispatch instructed that seat that f_required must still diverge and that its PR body must say so — that instruction rests on the same dead premise and is withdrawn: on current main, f_required should now agree with the driver. The delivering seat's own measurement governs, ⛔ not my instruction. Recorded on #16294 as well.


    Generated by Claude Code

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions