Collapse alembic 0001..0089 into one baseline #245

Merged

bvandeusen merged 2 commits from dev into main 2026-09-01 01:30:52 -04:00

Conversation 0 Commits 2 Files Changed 102

+867 -6890

bvandeusen commented 2026-09-01 01:30:47 -04:00

Owner

Copy Link

Copy Source

89 files / 6,300 lines → 1 file / 807 lines. Nothing about the resulting schema changes; what goes away is the requirement that a new installation replay our development history to arrive at it.

The migration strategy is the revision id

revision      = "0089"
down_revision = None

That pairing is the strategy, not a detail of it. A deployed database already has alembic_version = '0089' from running the real 0089, so alembic reads the version table, sees head reached, and does nothing. No stamp is required — which matters, because alembic stamp writes a version string without validating anything about the schema it writes it against, and a wrong stamp is indistinguishable from a right one until the next migration fails.

An empty database runs the file and records 0089. Both paths converge. The next migration is 0090, exactly as it would have been — the numbering is continuous across the collapse on purpose.

Why this attempt worked when the first was reverted

The first attempt failed because autogenerate silently dropped eleven indexes and three uniqueness guarantees. #3275 put those on the models first — the HNSW index with its opclass, the COALESCE expression index, the partial uniques, 107 server_defaults, the enum CHECKs. Doing the reconciliation before the squash rather than after is the entire reason this one reproduces the schema.

Hand-added, because none of it can live in a model: the two CREATE EXTENSION statements, the pgvector import autogenerate forgets to write (its own output won't run without it), and three seed inserts.

The defect CI caught, and the check that couldn't

The first version of this branch carried two of three seeds. Integration failed 36 tests on NoResultFound from _system_tag(db, "banner") — 0075 seeds three hygiene system tags (wip, banner, editor screenshot) and they were missing.

baseline.yml passed on that broken version (run 5139). It compares SCHEMA, and a baseline missing every seed row still produces a byte-identical schema and a perfectly green diff. That is not a bug in the workflow — it is the boundary of what a schema diff can mean, and the workflow header now says so.

My scan for data statements used a regex requiring INSERT immediately after the opening quote. It matched op.execute("INSERT INTO ml_settings ...") but not 0075, which builds the statement through sa.text() across several lines with bound parameters. The narrow pattern found two of three and reported itself complete — worse than finding none, because it looked like a finished answer.

The rule that actually separates them, now in the baseline's docstring:

shape	meaning	action
INSERT … VALUES (literals)	seed — product data	carry it → 0002, 0003, 0075
INSERT … SELECT … FROM tbl	backfill — derives from existing rows	omit → 0034, 0040, 0047

Also removed

The 10 test_migration_*.py files (they assert intermediate states and backfills that no longer exist; that a column exists is already the model tests' job) and backend/app/utils/artist_backfill.py, whose only importer was 0008 — verified no other consumer anywhere in backend/ or tests/.

downgrade() raises. A baseline's downgrade is "drop every table", which is a data-loss event wearing a migration as a disguise; offering it as one invites someone to run it.

A correction to an earlier claim

The CheckConstraint repair I added to baseline.yml never fired, and b979062's commit message asserting it was "still correct and still needed" was wrong. Autogenerate wraps names in op.f(), which marks them already-formatted and blocks the naming convention from re-applying — tested against the real candidate line, the regex matches nothing. 0088's renames alone fixed the mismatch. The doubling is a real hazard of hand-writing a pre-prefixed name, not a generator defect. Dead code and its false comment are gone.

The workflow header is rewritten for the same reason: it claimed 87 revisions and that the HNSW index could not be expressed in a model, which 0089's own generated output disproves. chain_ref now defaults to 725bf15, the last commit carrying a full chain to compare against.

Verification

• baseline.yml mode: chain vs 725bf15 (run 5143): chain=1122 lines, current=1122 lines → SCHEMAS MATCH — every difference above is column ORDER alone.
• ci.yml (run 5142): all five lanes green, including integration, which runs alembic upgrade head against a real pgvector Postgres from empty.
• build.yml (run 5141): green after a rerun; the first build-ml failure was the act_runner shared-action-cache race (dist/index.js failing at line 1 while build-web used the same action successfully in the same run), not this change.

Follow-up this makes concrete

Nothing automated catches a missing seed row. Integration caught this one only because tests happen to exercise those system tags; a seed with no test coverage would have shipped through both checks clean. A first-run check against the published image on a fresh database is the real gate — belongs in #3271, before anyone installs this.

Scribe: #3266, milestone 328.

**89 files / 6,300 lines → 1 file / 807 lines.** Nothing about the resulting schema changes; what goes away is the requirement that a new installation replay our development history to arrive at it. ## The migration strategy is the revision id ```python revision = "0089" down_revision = None ``` That pairing *is* the strategy, not a detail of it. A deployed database already has `alembic_version = '0089'` from running the real `0089`, so alembic reads the version table, sees head reached, and **does nothing**. No stamp is required — which matters, because `alembic stamp` writes a version string without validating anything about the schema it writes it against, and a wrong stamp is indistinguishable from a right one until the next migration fails. An empty database runs the file and records `0089`. Both paths converge. The next migration is `0090`, exactly as it would have been — the numbering is continuous across the collapse on purpose. ## Why this attempt worked when the first was reverted The first attempt failed because autogenerate silently dropped eleven indexes and three uniqueness guarantees. #3275 put those on the models *first* — the HNSW index with its opclass, the COALESCE expression index, the partial uniques, 107 `server_default`s, the enum CHECKs. Doing the reconciliation before the squash rather than after is the entire reason this one reproduces the schema. Hand-added, because none of it can live in a model: the two `CREATE EXTENSION` statements, the `pgvector` import autogenerate forgets to write (its own output won't run without it), and **three seed inserts**. ## The defect CI caught, and the check that couldn't The first version of this branch carried two of three seeds. Integration failed **36 tests** on `NoResultFound` from `_system_tag(db, "banner")` — `0075` seeds three hygiene system tags (`wip`, `banner`, `editor screenshot`) and they were missing. **`baseline.yml` passed on that broken version** (run 5139). It compares SCHEMA, and a baseline missing every seed row still produces a byte-identical schema and a perfectly green diff. That is not a bug in the workflow — it is the boundary of what a schema diff can mean, and the workflow header now says so. My scan for data statements used a regex requiring `INSERT` immediately after the opening quote. It matched `op.execute("INSERT INTO ml_settings ...")` but not `0075`, which builds the statement through `sa.text()` across several lines with bound parameters. The narrow pattern found two of three and reported itself complete — worse than finding none, because it looked like a finished answer. The rule that actually separates them, now in the baseline's docstring: | shape | meaning | action | |---|---|---| | `INSERT … VALUES (literals)` | **seed** — product data | carry it → `0002`, `0003`, `0075` | | `INSERT … SELECT … FROM tbl` | **backfill** — derives from existing rows | omit → `0034`, `0040`, `0047` | ## Also removed The 10 `test_migration_*.py` files (they assert intermediate states and backfills that no longer exist; that a column exists is already the model tests' job) and `backend/app/utils/artist_backfill.py`, whose only importer was `0008` — verified no other consumer anywhere in `backend/` or `tests/`. `downgrade()` raises. A baseline's downgrade is "drop every table", which is a data-loss event wearing a migration as a disguise; offering it as one invites someone to run it. ## A correction to an earlier claim The `CheckConstraint` repair I added to `baseline.yml` **never fired**, and `b979062`'s commit message asserting it was "still correct and still needed" was wrong. Autogenerate wraps names in `op.f()`, which marks them already-formatted and blocks the naming convention from re-applying — tested against the real candidate line, the regex matches nothing. `0088`'s renames alone fixed the mismatch. The doubling is a real hazard of hand-writing a pre-prefixed name, not a generator defect. Dead code and its false comment are gone. The workflow header is rewritten for the same reason: it claimed 87 revisions and that the HNSW index could not be expressed in a model, which `0089`'s own generated output disproves. `chain_ref` now defaults to `725bf15`, the last commit carrying a full chain to compare against. ## Verification - `baseline.yml mode: chain` vs `725bf15` (run 5143): `chain=1122 lines, current=1122 lines` → **`SCHEMAS MATCH — every difference above is column ORDER alone.`** - `ci.yml` (run 5142): all five lanes green, including integration, which runs `alembic upgrade head` against a real pgvector Postgres from empty. - `build.yml` (run 5141): green after a rerun; the first `build-ml` failure was the act_runner shared-action-cache race (`dist/index.js` failing at line 1 while `build-web` used the same action successfully in the same run), not this change. ## Follow-up this makes concrete Nothing automated catches a missing seed row. Integration caught this one only because tests happen to exercise those system tags; a seed with no test coverage would have shipped through both checks clean. A first-run check against the published image on a fresh database is the real gate — belongs in #3271, before anyone installs this. Scribe: #3266, milestone 328.

bvandeusen added 2 commits 2026-09-01 01:30:47 -04:00

db: collapse alembic 0001..0089 into one baseline (#3266) ... 973db73221

89 files and 6,300 lines become one file of 807. Nothing about the
resulting schema changes; what goes away is the requirement that a new
installation replay our development history to arrive at it.

revision = "0089", down_revision = None. That pairing IS the migration
strategy for existing installs, not a detail of it: a deployed database
already has alembic_version = '0089' from running the real 0089, so
alembic reads the version table, sees head reached, and does nothing. No
stamp is required — which matters, because `alembic stamp` writes a
version string without validating anything about the schema it is writing
it against, and a wrong stamp is indistinguishable from a right one until
the next migration fails. An empty database runs the file and records
0089. Both paths converge. The next migration is 0090, as it would have
been; the numbering is continuous across the collapse on purpose.

Autogenerate produced nearly all of this unaided, which was NOT true of
the first attempt — that one was reverted because the generator silently
dropped eleven indexes and three uniqueness guarantees. #3275 put those
on the models first, so the HNSW index with its opclass, the COALESCE
expression index, the partial uniques, 107 server_defaults and the enum
CHECKs are all emitted now. Doing the reconciliation before the squash,
rather than after, is what made this work.

Hand-added, because none of it can live in a model:

  * CREATE EXTENSION vector / tsm_system_rows (0001, 0004) — database
    objects, not table metadata.
  * The pgvector import. Autogenerate writes qualified
    pgvector.sqlalchemy.vector.VECTOR references without importing the
    package, so its own output cannot run (run 4988).
  * THE TWO SEED ROWS. 0002 and 0003 did not only build schema — each
    inserted a settings singleton, and nothing in the app ever creates
    them: ImportSettings.load() and MLSettings.load() are
    select(...).scalar_one(), which RAISES NoResultFound rather than
    returning None. A models-only baseline would leave both tables empty
    and crash a fresh install on first settings access, while
    baseline.yml reported a perfect schema match. Only running the app
    against a new database finds that.

Not carried over: 0023's DELETE FROM tag and 0047's series deletes, which
are historical cleanups operating on rows an empty database lacks.

downgrade() raises. A baseline's downgrade is "drop every table", which
is a data-loss event wearing a migration as a disguise; offering it as
one invites someone to run it. Restore from a backup.

Also removed, per the plan: the 10 test_migration_*.py files (they assert
intermediate states and backfills that no longer exist — a test that a
column exists is already the model tests' job) and
backend/app/utils/artist_backfill.py, whose only importer was 0008.
Verified no other consumer anywhere in backend/ or tests/.

baseline.yml changes with it. chain_ref now DEFAULTS to 725bf15, since
the tree no longer carries a chain to compare against — that pinned
commit is the last one that does.

And the CheckConstraint repair is removed, because it never fired. I
added it claiming autogenerate re-doubles a constraint name on the round
trip and asserted in b979062 that it was "still correct and still
needed". It is not: autogenerate wraps names in op.f(), which marks them
already-formatted and blocks the convention from re-applying. Tested
against the real candidate line — the regex matches nothing. What
actually fixed the mismatch was 0088's renames alone. The doubling is a
real hazard, but of hand-writing a pre-prefixed name, not of the
generator; the comment asserting otherwise was worse than the dead code
under it.

The header comment is rewritten for the same reason — it described 87
revisions, and claimed the HNSW index could not be expressed in a model,
which #3275 disproved. It now also states plainly what this check CANNOT
see: it compares schema, so a green run means the schema is right, not
that the baseline is.

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

db: the baseline was missing the three system-tag seeds (#3266) ... aa71cbbdbf

Integration caught it: 36 tests failing with NoResultFound, all on
_system_tag(db, "banner") and its siblings. 0075 seeds three hygiene
system tags — wip, banner, editor screenshot — and the first version of
the baseline carried only the two settings singletons.

This is the same defect class the baseline's own docstring warns about,
which I then walked into anyway. The reason is worth recording: my scan
for data statements used a regex requiring INSERT to sit immediately
after the opening quote, so it saw

    op.execute("INSERT INTO ml_settings (id) VALUES (1)")

and missed 0075, which builds the statement through sa.text() across
several lines with bound parameters. The narrow pattern found two of
three seeds and reported itself complete.

The wider scan — grep for insert/bulk_insert across every revision in
725bf15 — turns up six data-writing migrations, and they separate
mechanically:

  INSERT ... VALUES (literals)     = SEED.     Product data. Carry it.
    0002 import_settings, 0003 ml_settings, 0075 system tags
  INSERT ... SELECT ... FROM tbl   = BACKFILL. Derives from existing
    rows, inserts nothing on an empty database, correctly omitted.
    0034 artist_visit, 0040 and 0047 series_chapter

That rule is now in the docstring, because the next person collapsing a
chain needs the rule more than they need the answer.

0075's adopt-before-insert guard is kept as WHERE NOT EXISTS. It cannot
fire on the empty database this file runs against — it existed because an
operator might already have hand-tagged `wip` — but it makes the
statement re-runnable for free.

Worth stating plainly: baseline.yml passed on the version without these
rows, and would pass again. It compares schema, and a baseline missing
every seed still produces a byte-identical schema. The integration suite
is what caught this, which is the argument for the first-run check that
#3271 should carry.

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

bvandeusen merged commit b93a5fc5b4 into main 2026-09-01 01:30:52 -04:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

renovate-bot bvandeusen

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: bvandeusen/FabledCurator#245