test(migrate): close the uuid-PK blind spot; fix two Postgres non-convergence bugs it found

Every PK in every migrate fixture was `field.long` + `@generation: increment`,
so an entire family of uuid-PK migrate bugs was invisible to 5000+ tests. This
adds the missing coverage — and the new gates immediately caught two REAL,
previously-unknown Postgres bugs that make `meta migrate` never converge.

Coverage (test/fixtures/trainer-website-entities.json):
  MediaAsset — a uuid-PK kitchen sink. One shape per shipped bug: a uuid PK, an
  FK from a long-PK table INTO it, a paren-bearing string default, a
  quote-bearing string default, a boolean @default false, a field.enum, an
  @autoset timestamp, a partial (@where) UNIQUE, an expression (@expr) index, a
  @orders:desc index, a @constraintName FK, and a @precision/@scale decimal.
  It feeds the existing SQLite AND Postgres round-trip gates, both of which
  already apply-then-re-diff, so both engines now cover uuid PKs for free.

  test/integration/sqlite-uuid-pk-kitchen-sink.test.ts runs BOTH required gates
  against real libsql: IDEMPOTENCE (apply → introspect → re-diff must be EMPTY)
  and VALUE SEMANTICS (insert a row taking the defaults, then ask the engine what
  it actually stored). Neither alone suffices: idempotence is blind to
  consistently-wrong behavior (emit and introspect happily agreed on
  DEFAULT 'false' while SQLite stored the TEXT "false" and `= 0` matched nothing),
  and value semantics is blind to asymmetry.

Two REAL bugs found by the new gates, both making `meta migrate` exit 1 forever:

  1. Postgres introspection never read numeric precision/scale. The column query
     selected data_type — a bare "numeric" for every NUMERIC(p,s) — and
     character_maximum_length, which is NULL for numerics. So a
     `field.decimal @precision @scale` column diffed as a change-column-type on
     EVERY migrate, and being lossy it was BLOCKED. Any Postgres schema with a
     decimal column could never converge. Now selects numeric_precision /
     numeric_scale and passes them out-of-band. (The existing unit tests only
     ever fed pgTypeToSqlType the inline string "numeric(10,2)" — a form the real
     information_schema query never produces.)

  2. NUMERIC(p) IS NUMERIC(p,0). A precision-only decimal modeled no scale, but
     Postgres stores and reports scale 0, so expected and introspected could
     never converge. expected-schema now models the scale the database will hold.

Test-shape fix: sqlite-roundtrip / postgres-roundtrip called diff() WITHOUT a
dialect — a shape the production CLI never produces. Two behaviors hang off it:
FKs are keyed STRUCTURALLY on sqlite (which stores no FK names, so a
@constraintName can never match by name), and CHECK constraints are only diffed
when a dialect is known — so an enum @values change was invisible. Both gates
were silently disarmed; they now call diff() exactly as the CLI does.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hd5NWZ1rKau63vJzrBSpLb

Author: dmealing

server/typescript/packages/migrate-ts/src/expected-schema.ts

@@ -928,8 +928,13 @@ function subtypeToSqlType(field: MetaData): SqlType {
       if (typeof precision === "number" && typeof scale === "number") {
         return { kind: "numeric", precision, scale };
       }
+      // NUMERIC(p) IS NUMERIC(p,0) — the SQL standard defaults an omitted scale
+      // to zero, and Postgres stores it that way (information_schema reports
+      // numeric_scale = 0, not NULL). Model the scale the database will actually
+      // hold, or expected (bare p) and introspected (p, 0) can never converge and
+      // the column churns change-column-type on every migrate.
       if (typeof precision === "number") {
-        return { kind: "numeric", precision };
+        return { kind: "numeric", precision, scale: 0 };
       }
       return { kind: "numeric" };
     }

server/typescript/packages/migrate-ts/src/introspect/postgres.ts

@@ -76,12 +76,32 @@ export async function introspectPostgres(db: Kysely<Record<string, unknown>>): P
  * before matching.
  *
  * If character_maximum_length is available, callers should pass `maxLength`.
+ *
+ * `numeric` carries its qualifier OUT-OF-BAND: `data_type` is a bare "numeric"
+ * for both NUMERIC and NUMERIC(9,4), so callers reading information_schema must
+ * pass `numeric.precision` / `numeric.scale` (from numeric_precision /
+ * numeric_scale) or the qualifier is lost. An inline "numeric(9,4)" string is
+ * still parsed — that form arrives from format_type()/pg_catalog callers and
+ * from unit tests — but it is NOT what information_schema produces.
+ *
+ * Only the numeric/decimal branch consults the qualifier: information_schema
+ * also populates numeric_precision for INTEGER columns (int4 → 32, radix 2),
+ * which has nothing to do with a NUMERIC qualifier.
  */
-export function pgTypeToSqlType(dataType: string, maxLength?: number | null, udtName?: string): SqlType {
+export function pgTypeToSqlType(
+  dataType: string,
+  maxLength?: number | null,
+  udtName?: string,
+  numeric?: { precision: number | null; scale: number | null },
+): SqlType {
   const dt = dataType.toLowerCase().trim();
 
   // Array columns: information_schema reports data_type "ARRAY" and the element
   // type in udt_name with a leading underscore (e.g. "_uuid", "_text", "_varchar").
+  // The element is deliberately left UNQUALIFIED: information_schema reports no
+  // qualifiers for array elements (numeric_precision/character_maximum_length are
+  // NULL when data_type = 'ARRAY'), and the expected side builds unqualified
+  // elements to match (see arrayElementSqlType in expected-schema).
   if (dt === "array" && typeof udtName === "string") {
     const elemUdt = udtName.replace(/^_/, "").toLowerCase();
     return { kind: "array", element: pgTypeToSqlType(elemUdt, maxLength) };
@@ -117,12 +137,20 @@ export function pgTypeToSqlType(dataType: string, maxLength?: number | null, udt
   if (dt === "float4" || dt === "real") return { kind: "real4" };
   if (dt === "float8" || dt === "double precision") return { kind: "real" };
 
-  // Arbitrary-precision numeric — "numeric(p,s)" or bare "numeric"/"decimal"
+  // Arbitrary-precision numeric — "numeric(p,s)" or bare "numeric"/"decimal".
+  // An inline qualifier wins when present; otherwise fall back to the
+  // out-of-band numeric_precision/numeric_scale, which is the only form
+  // information_schema ever gives us. An unconstrained NUMERIC reports both as
+  // NULL and stays a bare { kind: "numeric" }.
   const numMatch = /^(?:numeric|decimal)(?:\((\d+)(?:,\s*(\d+))?\))?$/.exec(dt);
   if (numMatch) {
     const out: SqlType = { kind: "numeric" };
-    if (numMatch[1]) out.precision = parseInt(numMatch[1], 10);
-    if (numMatch[2]) out.scale = parseInt(numMatch[2], 10);
+    const precision = numMatch[1] !== undefined ? parseInt(numMatch[1], 10) : numeric?.precision ?? null;
+    const scale = numMatch[1] !== undefined
+      ? (numMatch[2] !== undefined ? parseInt(numMatch[2], 10) : null)
+      : numeric?.scale ?? null;
+    if (precision !== null) out.precision = precision;
+    if (scale !== null) out.scale = scale;
     return out;
   }
 
@@ -290,18 +318,29 @@ interface RawColumn {
   data_type: string;
   udt_name: string;
   character_maximum_length: number | null;
+  numeric_precision: number | null;
+  numeric_scale: number | null;
   is_nullable: string;   // 'YES' | 'NO'
   column_default: string | null;
 }
 
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 async function readColumns(k: Kysely<any>, schema: string, tableName: string): Promise<ColumnDescriptor[]> {
+  // numeric_precision/_scale are the ONLY place a NUMERIC(p,s) qualifier survives:
+  // information_schema.columns.data_type reports a bare "numeric" for every
+  // NUMERIC column regardless of its qualifier, and character_maximum_length is
+  // NULL for numerics. Selecting them is what lets a `field.decimal @precision
+  // @scale` column converge — without them the expected side says NUMERIC(9,4),
+  // introspection reads back a bare NUMERIC, and the diff reports a (lossy, so
+  // BLOCKED) change-column-type on every single migrate, forever.
   const rows = await sql<RawColumn>`
     SELECT
       column_name,
       data_type,
       udt_name,
       character_maximum_length,
+      numeric_precision,
+      numeric_scale,
       is_nullable,
       column_default
     FROM information_schema.columns
@@ -311,7 +350,10 @@ async function readColumns(k: Kysely<any>, schema: string, tableName: string): P
   `.execute(k);
 
   return rows.rows.map((r) => {
-    const sqlType = pgTypeToSqlType(r.data_type, r.character_maximum_length, r.udt_name);
+    const sqlType = pgTypeToSqlType(r.data_type, r.character_maximum_length, r.udt_name, {
+      precision: r.numeric_precision,
+      scale: r.numeric_scale,
+    });
     const col: ColumnDescriptor = {
       name: r.column_name,
       sqlType,

server/typescript/packages/migrate-ts/test/fixtures/trainer-website-entities.json

@@ -70,7 +70,38 @@
             { "field.string": { "name": "title", "@required": true } },
             { "field.int": { "name": "durationSeconds" } },
             { "field.timestamp": { "name": "createdAt", "@required": true, "@default": "CURRENT_TIMESTAMP" } },
-            { "identity.primary": { "name": "pk", "@fields": ["id"], "@generation": "increment" } }
+            { "field.uuid": { "name": "thumbnailAssetId" } },
+            { "identity.primary": { "name": "pk", "@fields": ["id"], "@generation": "increment" } },
+            { "identity.reference": {
+                "name": "ref_thumbnail",
+                "@fields": ["thumbnailAssetId"],
+                "@references": "MediaAsset",
+                "@constraintName": "fk_video_thumbnail_asset"
+            } },
+            { "relationship.association": { "name": "thumbnail", "@cardinality": "one", "@objectRef": "MediaAsset" } }
+          ]
+        }
+      },
+      {
+        "object.entity": {
+          "name": "MediaAsset",
+          "children": [
+            { "field.uuid": { "name": "id" } },
+            { "field.string": { "name": "storageKey", "@required": true } },
+            { "field.string": { "name": "caption", "@default": "n/a (unknown)" } },
+            { "field.string": { "name": "altText", "@default": "don't know" } },
+            { "field.boolean": { "name": "archived", "@required": true, "@default": "false" } },
+            { "field.decimal": { "name": "fileSizeMb", "@precision": 12, "@scale": 3 } },
+            { "field.enum": { "name": "kind", "@required": true, "@values": ["IMAGE", "VIDEO", "AUDIO"] } },
+            { "field.timestamp": { "name": "createdAt", "@required": true, "@autoSet": "onCreate" } },
+            { "identity.primary": { "name": "pk", "@fields": ["id"], "@generation": "uuid" } },
+            { "identity.secondary": {
+                "name": "uniqueStorageKey",
+                "@fields": ["storageKey"],
+                "@where": "caption IS NOT NULL"
+            } },
+            { "index.lookup": { "name": "byCaptionLower", "@fields": ["caption"], "@expr": "lower(caption)" } },
+            { "index.lookup": { "name": "byCreated", "@fields": ["createdAt"], "@orders": ["desc"] } }
           ]
         }
       },

server/typescript/packages/migrate-ts/test/integration/postgres-roundtrip.test.ts

@@ -122,11 +122,14 @@ describe("PG round-trip — trainer-website fixture", () => {
 
     // Step 1: Build the expected schema from metadata.
     const expected = buildExpectedSchema(metadata);
-    expect(expected.tables).toHaveLength(6);
+    // 7 = the six original entities + MediaAsset, the uuid-PK kitchen sink.
+    expect(expected.tables).toHaveLength(7);
 
     // Step 2: Diff against empty DB — should want to create everything.
+    // `dialect` is what the CLI passes; it is also what switches CHECK-constraint
+    // diffing on, so without it an enum @values change is invisible.
     const actual0 = await introspectPostgres(kysely);
-    const initial = await diff(expected, actual0);
+    const initial = await diff({ expected, actual: actual0, dialect: "postgres" });
     expect(initial.blocked).toEqual([]);
     expect(initial.changes.length).toBeGreaterThan(0);
 
@@ -139,7 +142,7 @@ describe("PG round-trip — trainer-website fixture", () => {
 
     // Step 5: Re-introspect and re-diff — MUST yield no changes.
     const actual1 = await introspectPostgres(kysely);
-    const followup = await diff(expected, actual1);
+    const followup = await diff({ expected, actual: actual1, dialect: "postgres" });
 
     if (followup.changes.length > 0) {
       console.error("ROUND-TRIP FAILURE — remaining changes after apply:");

server/typescript/packages/migrate-ts/test/integration/sqlite-roundtrip.test.ts

@@ -66,7 +66,13 @@ describe("SQLite round-trip — trainer-website fixture", () => {
     // equal the introspected `timestamp{withTimezone:false}`.
     const expected = buildExpectedSchema(metadata, { dialect: "sqlite" });
     const actual0 = await introspectSqlite(k);
-    const initial = await diff(expected, actual0);
+    // `dialect` is REQUIRED to reproduce the production diff. The CLI always passes
+    // it, and two diff behaviors hang off it: FKs are keyed STRUCTURALLY (SQLite
+    // stores no FK names, so a @constraintName / composite FK can never match by
+    // name), and CHECK constraints are only diffed when a dialect is known (so an
+    // enum @values change is otherwise invisible). Omitting it here tested a shape
+    // the real pipeline never produces — and silently disarmed both gates.
+    const initial = await diff({ expected, actual: actual0, dialect: "sqlite" });
     expect(initial.blocked).toEqual([]);
 
     const { up } = emit(initial.changes, {
@@ -77,7 +83,7 @@ describe("SQLite round-trip — trainer-website fixture", () => {
     await applyRaw(k, up);
 
     const actual1 = await introspectSqlite(k);
-    const followup = await diff(expected, actual1);
+    const followup = await diff({ expected, actual: actual1, dialect: "sqlite" });
     if (followup.changes.length > 0) {
       console.error("ROUND-TRIP FAILURE — remaining changes:");
       for (const c of followup.changes) console.error(`  - ${c.kind}:`, JSON.stringify(c, null, 2));
@@ -89,7 +95,7 @@ describe("SQLite round-trip — trainer-website fixture", () => {
     const metadata1 = await loadFixture("trainer-website-entities");
     const expected1 = buildExpectedSchema(metadata1, { dialect: "sqlite" });
     {
-      const initial = await diff(expected1, await introspectSqlite(k));
+      const initial = await diff({ expected: expected1, actual: await introspectSqlite(k), dialect: "sqlite" });
       const { up } = emit(initial.changes, { dialect: "sqlite", expectedSchema: expected1 });
       await applyRaw(k, up);
     }
@@ -105,12 +111,12 @@ describe("SQLite round-trip — trainer-website fixture", () => {
     const metadata2 = (await new MetaDataLoader().load([new InMemoryStringSource(JSON.stringify(json))])).root;
     const expected2 = buildExpectedSchema(metadata2, { dialect: "sqlite" });
 
-    const second = await diff(expected2, await introspectSqlite(k));
+    const second = await diff({ expected: expected2, actual: await introspectSqlite(k), dialect: "sqlite" });
     expect(second.blocked).toEqual([]);
     const { up: up2 } = emit(second.changes, { dialect: "sqlite", expectedSchema: expected2 });
     await applyRaw(k, up2);
 
-    const followup = await diff(expected2, await introspectSqlite(k));
+    const followup = await diff({ expected: expected2, actual: await introspectSqlite(k), dialect: "sqlite" });
     if (followup.changes.length > 0) {
       console.error("ROUND-TRIP FAILURE (add-column) — remaining changes:");
       for (const c of followup.changes) console.error(`  - ${c.kind}:`, JSON.stringify(c, null, 2));