db: the baseline was missing the three system-tag seeds (#3266)
CI / lint (push) Successful in 3s
CI / extension-version (push) Successful in 4s
CI / frontend-build (push) Successful in 22s
CI / backend-lint-and-test (push) Successful in 30s
CI / integration (push) Successful in 3m41s
Build images / sign-extension (push) Successful in 3s
Build images / build-agent (push) Successful in 7s
Build images / build-web (push) Successful in 6s
Build images / build-ml (push) Successful in 27s
CI / lint (push) Successful in 3s
CI / extension-version (push) Successful in 4s
CI / frontend-build (push) Successful in 22s
CI / backend-lint-and-test (push) Successful in 30s
CI / integration (push) Successful in 3m41s
Build images / sign-extension (push) Successful in 3s
Build images / build-agent (push) Successful in 7s
Build images / build-web (push) Successful in 6s
Build images / build-ml (push) Successful in 27s
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
This commit is contained in:
@@ -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
|
Extensions are database objects, not table metadata, so no model can carry
|
||||||
them. `IF NOT EXISTS` because a re-run must not fail.
|
them. `IF NOT EXISTS` because a re-run must not fail.
|
||||||
|
|
||||||
2. **The two settings singleton rows.** 0002 and 0003 did not only build
|
2. **Three seed inserts** — the two settings singletons (0002, 0003) and the
|
||||||
schema — they each inserted a row:
|
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)
|
Distinguishing these from the other data statements in the chain is the
|
||||||
INSERT INTO ml_settings (id) VALUES (1)
|
whole trick, and the rule turns out to be mechanical:
|
||||||
|
|
||||||
and nothing in the application ever creates them. `ImportSettings.load()`
|
* `INSERT ... VALUES (...)` with literal values is a SEED. It creates
|
||||||
and `MLSettings.load()` are `select(...).scalar_one()`, which RAISES
|
something the product ships. It must be carried.
|
||||||
`NoResultFound` on an empty result rather than returning None. So a baseline
|
* `INSERT ... SELECT ... FROM <table>` is a BACKFILL. It derives rows
|
||||||
built from models alone would produce those two tables empty and crash a
|
from rows that already exist, so on an empty database it inserts
|
||||||
fresh install on its first settings access.
|
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 category is invisible to every automated check this project has:
|
||||||
this project has: `baseline.yml` compares SCHEMA, and the schema would have
|
`baseline.yml` compares SCHEMA, and a baseline missing all three seeds still
|
||||||
been a perfect match. Only running the application against a fresh database
|
produces a byte-identical schema and a perfectly green diff. What caught the
|
||||||
finds it.
|
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
|
3. **The `pgvector` import.** Autogenerate emits qualified
|
||||||
`pgvector.sqlalchemy.vector.VECTOR(...)` references without importing the
|
`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 import_settings (id) VALUES (1)")
|
||||||
op.execute("INSERT INTO ml_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:
|
def downgrade() -> None:
|
||||||
"""Deliberately not implemented.
|
"""Deliberately not implemented.
|
||||||
|
|||||||
Reference in New Issue
Block a user