Skip to content

fix(cli): a generated migration carries the field-level unique index driver-sql creates - #17208

Merged
os-project-manager merged 3 commits into
mainfrom
claude/issue-16317-generate-migration-emits-indexes
Sep 9, 2026
Merged

fix(cli): a generated migration carries the field-level unique index driver-sql creates#17208
os-project-manager merged 3 commits into
mainfrom
claude/issue-16317-generate-migration-emits-indexes

Conversation

@claude

@claude claude Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Fixes #16317

Clause-②: no

A generated migration emitted the table and none of the object's declared uniqueness. The key set was not missing — generate.ts already computes it to size the keyed text family's columns (#16091); only the index it implies was never emitted. The file said so in its own comment at indexKeyColumns: "A generated migration still emits no CREATE INDEX". That sentence is now false, and it has been rewritten rather than left standing.

The acceptance, re-run — live PostgreSQL 16.13

Triage did not re-run the card's measurement, so it was re-run here from scratch: a scratch cluster (initdb + pg_ctl, PostgreSQL 16.13), the card's own probe object driven through all three producers into three schemas, pg_indexes read back per schema.

{ name: 'probe', fields: { keyed_unique: { type: 'text', unique: true, maxLength: 100 } } }
producer before after
driver-sql via initObjects probe_pkey, uniq_probe_keyed_unique unchanged
os generate migration --format sql probe_pkey probe_pkey, uniq_probe_keyed_unique
--format ts (module imported, up(db) called) probe_pkey probe_pkey, uniq_probe_keyed_unique

The card's baseline reproduced exactly, and the card's consequence with it. A second row carrying the same keyed_unique value, offered to each table:

before   driver REFUSED (23505 ... violates unique constraint "uniq_probe_keyed_unique")
         sql gen ACCEPTED · ts gen ACCEPTED
after    driver REFUSED · sql gen REFUSED · ts gen REFUSED   — all three naming the same constraint

② The positive control — the column width — is unmoved. keyed_unique reads character varying(100) on all three producers both before and after, and so does every other column on the table (id varchar(255), both audit columns timestamp with time zone). #16298's result is not spent.

One layer the acceptance did not ask for and got anyway: pg_constraint agrees too — uniq_probe_keyed_unique/u on all three — because the sql format emits an inline table constraint rather than a following CREATE UNIQUE INDEX, which is what knex's table.unique(columns, { indexName }) compiles to on PostgreSQL.

What the change is

  • --format sql emits CONSTRAINT "NAME" UNIQUE (COLUMNS) inside the existing CREATE TABLE IF NOT EXISTS. Chosen over a following ALTER TABLE ... ADD CONSTRAINT because that spelling has no IF NOT EXISTS, and over a bare CREATE UNIQUE INDEX because the constraint form is what the driver's own knex call produces, so both catalogs agree.
  • --format ts emits table.unique([COLUMNS], { indexName: 'NAME' }) — the driver's own line, with table bound to the create-table builder instead of an alter-table one. The indexName is load-bearing: it is what makes syncDeclaredIndexes recognise the constraint as already present on its first boot against a generated table, instead of adding a second under its own name and then reporting the generated one as an orphan to drop.
  • ④ No driver import. buildIndexName, the __global__ sentinel and the ADR-0120 D1/D3 scoping rule are transcribed, following the precedent the column widths set (objectstack dev 在工作区未构建时刷 12 段无关命令的 MODULE_NOT_FOUND,唯一可执行的那条却指向错误修法 #5726 forbids a static driver value-import from a CLI production module, and these generators are synchronous). Every transcription is pinned against the driver's own export.

What it deliberately does not emit — and now says so out loud

Two shapes stay unemitted. They used to be dropped silently; each is now named in the generated file itself, with its key parts and the reason:

-- NOT EMITTED: UNIQUE index "uniq_probe_org_organization_id_keyed_unique" on (organization_id, keyed_unique) — its
-- organization key part is COALESCE("organization_id", '__global__'), an expression key part this format does not
-- emit; the platform creates it at boot.
  1. The ADR-0120 D3 organization-scoped composite (step 2, not dispatched). Emitting the bare composite instead would be worse than emitting nothing: under SQL's NULL-distinct UNIQUE, (organization_id, field) constrains no row that has no organization — on a single-tenant stack, every row (driver-sql: 单租户栈上 organization_id 恒为 NULL,#3696 的 (tenant, col) 复合 UNIQUE 因 NULL-distinct 而完全不生效 —— 字段级 unique: true 静默零约束 #5030). That is a constraint advertised and not delivered.
  2. An index over a column no field materialises (a virtual formula field). The driver skips exactly this case with a warning; emitting it produces DDL that cannot run at all.

Object-level indexes[] remains unemitted by both formats. It is normalized by normalizeDeclaredIndex, which reads the same unique: true token differently (verbatim as global, a maintainer ruling), so it is a second transcription with a second pin — not a loop added to this one. The new pin asserts it is still unemitted, so whoever adds it lands there and reads why first.

⭐ The step-2 measurement triage asked for: can knex express a COALESCE(...) key part?

Measured rather than assumed, knex 3.3.0 against the same live cluster:

attempt result
table.unique([knex.raw("COALESCE(...)"), 'f'], { indexName }) knex compiles ALTER TABLE ... ADD CONSTRAINT ... UNIQUE (COALESCE(...), "f") — PostgreSQL refuses: syntax error at or near "(". A UNIQUE constraint takes no expression key part; only a unique index does.
table.unique(["COALESCE(...)", 'f'], { indexName }) knex quotes the string as an identifier — column "COALESCE(""organization_id"", '__global__')" named in key does not exist
db.raw('CREATE UNIQUE INDEX ... (COALESCE("organization_id", \'__global__\'), "f")') accepted, materialised as CREATE UNIQUE INDEX k_c ON t USING btree (COALESCE(organization_id, '__global__'::character varying), f)

Conclusion, for whoever takes step 2: the two formats are not unequally capable. knex's schema builder cannot express the key part in either format — but the emitted up(db) receives a knex handle, and db.raw is exactly the seam SqlDriver.createNullSafeUniqueIndex already uses for this. So the scoped form costs a raw statement in the ts format rather than another table.unique(...) line; it does not cost a capability. One detail for that work: the driver's index materialises the sentinel with a cast taken from the organization column's own type ('__global__'::character varying above), so name-and-definition convergence should be measured, not assumed.

The pin, and its ablation

packages/cli/src/commands/generate-declared-unique-index.pin.test.ts (18 cases). Its authority is the real chain, not a transcribed literal: all three producers driven into one in-memory better-sqlite3 database, one table each, indexes read back out of the database's own catalog (PRAGMA index_list / index_info, primary key excluded by SQLite's own origin), the duplicate row offered to each table, and every expected name recomputed from driver-sql's exported uniqueIndexesFromFields / buildIndexName / GLOBAL_TENANT. Underneath it sits the leaf differential over a 12-shape corpus.

⚠️ One SQLite artifact is named in the file rather than papered over: SQLite ignores the identifier on a table-level CONSTRAINT NAME UNIQUE (...) and materialises sqlite_autoindex_TABLE_N. The sql format's DDL is a PostgreSQL claim by #15521; what the SQLite chain carries for that format is the key parts and the enforcement, while its constraint name is asserted textually against the driver's computed one, plus on the live cluster above.

⑥ Ablation — five legs, directions predicted in writing before any leg ran, all five as predicted (RED). Each leg proved the mutation reached disk (anchor occurrence count 1 to 0, plus a blob hash off the HEAD blob), rebuilt the package and ran scripts/ablation-dist-preflight.mjs in both directions, and proved the restore clean (git diff HEAD empty, blob equal to HEAD's, whole-tree git status --porcelain empty, dist/commands/generate.js back to its pre-ablation digest), under a trap ... EXIT INT TERM with absolute paths.

leg mutation predicted measured
L1 delete the sql format's constraint emission RED 9 failed / 18
L2 delete the ts format's table.unique emission RED 9 failed / 18
L3 drop the organization-scoped exclusion (emit the bare composite) RED 5 failed / 18
L4 widen the identifier budget so no name is hash-truncated RED 3 failed / 18
L5 drop the "no column materialised" skip RED 2 failed / 18

Baseline, unmutated: 18 passed / 18.

Verification

Run at 074a7b56e9 (this branch's head, after git merge origin/main).

  • pnpm --filter '@objectstack/cli...' --filter '@objectstack/driver-sql...' build — exit 0.
  • pnpm --filter @objectstack/cli test230 files / 2983 tests passed, both tiers (this diff adds an integration-tier file, so the integration project was run locally rather than declared to CI).
  • pnpm --filter @objectstack/cli typecheck — exit 0; tsc --listFiles confirms the new pin file is in the program.
  • node scripts/pm/dispatch-gates.mjs --commands derived 60 families; all 60 run, exit code captured by redirect-then-$? (never through a pipe). Two first returned a PREREQUISITE NOT MET refusal (exit 3, not a pass) — check:dual-build-cjs-loads and check:i18n-coverage, both wanting a whole-workspace dist/. After pnpm build both were re-run and are genuinely green, so 0 NOT MEASURED. --ran reconciles: 60 derived, 60 run, 0 NOT-MEASURED, 0 UNRUN.
  • pnpm lint — the whole repo, eslint . --no-inline-config, exit 0. No narrowing, so no narrowing evidence is owed.

Scope

Held to the dispatched half. ⛔ The NUMERIC arm is untouched (#16318 is needs-user-decision and orthogonal); ⛔ absent/unknown type defaulting is untouched (#16319); --format sql stays PostgreSQL-only (#15521); no packages/spec path, no *.zod.ts, no error-code ledger, and no new export, authorable key or declared-payload key — the ② stop condition did not fire.

⑤ Single-writer re-measured from the open PR list, never from remote branches: each of the 18 open PRs diffed against its own merge base. packages/cli/src/commands/generate.ts is held by none. Positive control from the same run, so the detector is demonstrably not blind to this subtree: packages/cli/src/commands/db/clean.ts is held by #17073 and packages/cli/package.json by #17076. #16887 and #17143 are both merged and both ancestors of this branch's base (compare reports behind_by: 0 for each), so generate.ts was read as they left it.


Generated by Claude Code

…the driver creates

Both migration formats emitted the table and none of the object's declared
uniqueness. Measured on live PostgreSQL 16.13, one object through all three
producers, pg_indexes per schema:

  driver   probe_pkey, uniq_probe_keyed_unique
  sql gen  probe_pkey
  ts gen   probe_pkey

Two rows with the same value in a `unique: true` field were refused by the
platform's table and accepted by both generated ones, with nothing reporting
it. The key set was already computed here for #16091's column widths; only
the index it implies was missing.

The sql format emits an inline `CONSTRAINT <name> UNIQUE (...)` — what knex's
`table.unique(columns, { indexName })` compiles to on PostgreSQL, so both
pg_indexes and pg_constraint agree with the driver — and the ts format emits
that knex call itself. Names come from a transcription of driver-sql's
`buildIndexName` (#5726 forbids a static driver import from a CLI production
module), pinned against the driver's own export.

Two shapes stay unemitted and are now NAMED in the generated file rather than
dropped: the ADR-0120 D3 organization-scoped composite, whose
COALESCE key part knex's schema builder cannot express, and object-level
`indexes[]`, which a second normalizer reads with different token semantics.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
@github-actions github-actions Bot added the size/l label Sep 9, 2026
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/cli, touching 11 documentable anchor(s).

4 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/deployment/cli.mdx (via os generate (command, read off packages/cli/src/commands/generate.ts))
  • content/docs/protocol/kernel/lifecycle.mdx (via os generate (command, read off packages/cli/src/commands/generate.ts))
  • content/docs/protocol/objectql/types.mdx (via os generate (command, read off packages/cli/src/commands/generate.ts))

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

  • content/docs/releases/v17/17-4.mdx (via os generate (command, read off packages/cli/src/commands/generate.ts))

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
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 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 — 23 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 22c94e53b652d95289e93aaa78c71e9c8436ec92packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 22c94e53b652d95289e93aaa78c71e9c8436ec92

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

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 9, 2026
@os-project-manager
os-project-manager marked this pull request as ready for review September 9, 2026 17:39
@os-project-manager
os-project-manager added this pull request to the merge queue Sep 9, 2026
Merged via the queue into main with commit 3c5f3c5 Sep 9, 2026
35 checks passed
@os-project-manager
os-project-manager deleted the claude/issue-16317-generate-migration-emits-indexes branch September 9, 2026 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

2 participants