From aa71cbbdbf1240aaf93fecb8480328ce3e86c293 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 1 Sep 2026 01:20:42 -0400 Subject: [PATCH] db: the baseline was missing the three system-tag seeds (#3266) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_017QHszn9H8VBvx5Ke8x1hvw --- alembic/versions/0089_baseline.py | 55 +++++++++++++++++++++++-------- 1 file changed, 42 insertions(+), 13 deletions(-) diff --git a/alembic/versions/0089_baseline.py b/alembic/versions/0089_baseline.py index b9d89d5..f961ee1 100644 --- a/alembic/versions/0089_baseline.py +++ b/alembic/versions/0089_baseline.py @@ -39,22 +39,31 @@ Four things still had to be added by hand, because they are not in the models: Extensions are database objects, not table metadata, so no model can carry them. `IF NOT EXISTS` because a re-run must not fail. -2. **The two settings singleton rows.** 0002 and 0003 did not only build - schema — they each inserted a row: +2. **Three seed inserts** — the two settings singletons (0002, 0003) and the + three hygiene system tags (0075). Some migrations did not only build schema; + they inserted rows the product needs in order to function, and nothing in + the application ever creates them. Every consumer reads them with + `scalar_one()`, which RAISES `NoResultFound` on an empty result rather than + returning None, so their absence is a crash and not a degradation. - INSERT INTO import_settings (id) VALUES (1) - INSERT INTO ml_settings (id) VALUES (1) + Distinguishing these from the other data statements in the chain is the + whole trick, and the rule turns out to be mechanical: - and nothing in the application ever creates them. `ImportSettings.load()` - and `MLSettings.load()` are `select(...).scalar_one()`, which RAISES - `NoResultFound` on an empty result rather than returning None. So a baseline - built from models alone would produce those two tables empty and crash a - fresh install on its first settings access. + * `INSERT ... VALUES (...)` with literal values is a SEED. It creates + something the product ships. It must be carried. + * `INSERT ... SELECT ... FROM ` is a BACKFILL. It derives rows + from rows that already exist, so on an empty database it inserts + nothing and carrying it would be pointless. 0034 (artist_visit), 0040 + and 0047 (series_chapter) are all of this shape and are correctly + absent here. - This is worth dwelling on, because it is invisible to every automated check - this project has: `baseline.yml` compares SCHEMA, and the schema would have - been a perfect match. Only running the application against a fresh database - finds it. + This category is invisible to every automated check this project has: + `baseline.yml` compares SCHEMA, and a baseline missing all three seeds still + produces a byte-identical schema and a perfectly green diff. What caught the + system tags was the integration suite — 36 tests failing on + `NoResultFound` — after a first version of this file shipped with only the + two settings rows. A first-run check against the real application is the + only thing that finds this class of defect. 3. **The `pgvector` import.** Autogenerate emits qualified `pgvector.sqlalchemy.vector.VECTOR(...)` references without importing the @@ -794,6 +803,26 @@ def upgrade() -> None: op.execute("INSERT INTO import_settings (id) VALUES (1)") op.execute("INSERT INTO ml_settings (id) VALUES (1)") + # The three hygiene system tags, from 0075. These are PRODUCT data, not + # operator configuration — 0075's own docstring says so: "the fix keys on + # SYSTEM tags the product ships". The presentation and process auto-apply + # sweeps look them up with scalar_one(), so without these rows those + # features raise NoResultFound rather than degrading. + # + # 0075 adopted an existing same-name general tag before inserting, because + # an operator might already have tagged `wip` by hand. That cannot happen + # on the empty database this file runs against, but the guard is kept: it + # costs nothing and makes the statement safe to re-run. + for _name in ("wip", "banner", "editor screenshot"): + op.execute( + sa.text( + "INSERT INTO tag (name, kind, is_system) " + "SELECT :name, 'general', true WHERE NOT EXISTS (" + " SELECT 1 FROM tag WHERE lower(name) = lower(:name)" + ")" + ).bindparams(name=_name) + ) + def downgrade() -> None: """Deliberately not implemented.