diff --git a/.forgejo/workflows/baseline.yml b/.forgejo/workflows/baseline.yml index 7d36ac4..4681a35 100644 --- a/.forgejo/workflows/baseline.yml +++ b/.forgejo/workflows/baseline.yml @@ -217,6 +217,42 @@ jobs: # comparison is about whether the models describe the schema. sed -i '0,/^import sqlalchemy as sa$/s//import sqlalchemy as sa\nimport pgvector.sqlalchemy.vector/' alembic/versions/*.py grep -n 'import pgvector' alembic/versions/*.py + # Second generator defect, same class as the missing import. + # + # base.py's naming convention includes %(constraint_name)s for ck, + # which — unlike uq/fk/ix — means the convention is applied even to + # a CheckConstraint that HAS a name. So a model declaring + # name="singleton" correctly becomes ck_ml_settings_singleton in + # the metadata. Autogenerate then writes that RENDERED name into + # the migration, and running the migration applies the convention a + # SECOND time: ck_ml_settings_ck_ml_settings_singleton. + # + # That is round-tripping damage done by the generator, not a claim + # the models make, so it is repaired here rather than counted as a + # schema difference. Undone by removing the ck__ prefix the + # convention will re-add — the exact inverse, and it only fires on + # a name that actually carries its own table's prefix. + python3 - alembic/versions/*.py <<'PYEOF' + import re, sys + + table = None + for path in sys.argv[1:]: + out = [] + for line in open(path): + m = re.search(r"op\.create_table\(\s*[\"']([A-Za-z0-9_]+)[\"']", line) + if m: + table = m.group(1) + if table and "CheckConstraint" in line: + prefix = f"ck_{table}_" + line = re.sub( + r"(name=[\"'])" + re.escape(prefix), + r"\1", + line, + ) + out.append(line) + open(path, "w").writelines(out) + PYEOF + grep -n 'CheckConstraint' alembic/versions/*.py || true ls alembic/versions/*.py DB_NAME=fc_base alembic upgrade head rm -f alembic/versions/*.py @@ -245,6 +281,25 @@ jobs: # these two lines and nothing else. That control is what licenses this # filter — it was observed to be the only false positive, rather than # assumed to be one. + # Column ORDER inside a CREATE TABLE is compared separately from column + # CONTENT, and only content is fatal. + # + # A table built by 87 migrations has its columns in ADD COLUMN order; the + # same table built in one shot has them in declaration order. That is a + # real and permanent difference which no baseline can erase — the + # operator's existing database keeps chain order forever, a fresh install + # gets model order — so a check that fails on it would never pass and + # would teach nothing. FC reaches every column through the ORM by name, + # and `SELECT *` ordering is not depended on anywhere. + # + # So the second pass SORTS the column lines within each CREATE TABLE + # rather than DELETING them. That distinction is the whole point: sorting + # cannot hide a column that exists on one side only, or one whose type, + # nullability or default differs — those still land in the diff. A filter + # could have hidden all three. + # + # Both diffs are reported. The ordered one is informational; the + # order-insensitive one is the verdict. - name: Diff run: | set -eu @@ -256,11 +311,54 @@ jobs: norm chain.sql > a.txt norm baseline.sql > b.txt echo "normalised: chain=$(wc -l < a.txt) lines, current=$(wc -l < b.txt) lines" + + sort_table_columns() { + python3 - "$1" <<'PYEOF' + import re, sys + + lines = open(sys.argv[1]).read().splitlines() + out, block = [], None + for line in lines: + if block is not None: + # ');' on its own closes the CREATE TABLE body. + if line.strip() == ");": + out.extend(sorted(block)) + out.append(line) + block = None + else: + # Drop the list comma before sorting. Only the LAST + # column lacks one, so keeping it would make every + # reordering look like a content change as well — the + # comma is punctuation, and carries no schema meaning. + block.append(line.rstrip().rstrip(",")) + continue + out.append(line) + if re.match(r"CREATE TABLE .*\($", line): + block = [] + if block is not None: # unterminated body: emit it rather than drop it + out.extend(block) + print("\n".join(out)) + PYEOF + } + sort_table_columns a.txt > a.sorted.txt + sort_table_columns b.txt > b.sorted.txt + test "$(wc -l < a.sorted.txt)" = "$(wc -l < a.txt)" + test "$(wc -l < b.sorted.txt)" = "$(wc -l < b.txt)" + if diff -u a.txt b.txt > schema.diff; then - echo "SCHEMAS IDENTICAL — the collapsed chain reproduces the old one." + echo "ORDERED DIFF: identical, column order included." else - echo "SCHEMAS DIFFER — $(grep -cE '^[+-]' schema.diff) changed lines:" + echo "ORDERED DIFF: $(grep -cE '^[+-]' schema.diff) changed lines (informational):" cat schema.diff + fi + echo + echo "================================================================" + echo + if diff -u a.sorted.txt b.sorted.txt > sorted.diff; then + echo "SCHEMAS MATCH — every difference above is column ORDER alone." + else + echo "SCHEMAS DIFFER — $(grep -cE '^[+-]' sorted.diff) changed lines that are NOT ordering:" + cat sorted.diff echo echo "The baseline is wrong, not the database. Do not stamp." exit 1 diff --git a/alembic/versions/0088_reconcile_models_with_schema.py b/alembic/versions/0088_reconcile_models_with_schema.py index 6117dd3..a286c6c 100644 --- a/alembic/versions/0088_reconcile_models_with_schema.py +++ b/alembic/versions/0088_reconcile_models_with_schema.py @@ -14,13 +14,20 @@ created that index. Every autogenerate run since would have proposed adding it; nobody ran one, so the model and the database simply drifted apart and stayed that way. -Deliberately NOT in this migration: making `image_record.sha256` unique. The -model says `unique=True` and `0001` created a plain, non-unique index, so -duplicates are possible today and the ORM believes they are not. Adding the -constraint is a real change that FAILS if duplicates already exist, and if -they do exist the right response is a dedupe decision rather than a constraint -— so it needs an answer about live data before it is written, not after. -Tracked in #3275. +Deliberately NOT in this migration: anything about `image_record.sha256`. An +earlier draft of this file claimed sha256 was not unique in the database and +that duplicate rows were therefore possible. That was WRONG, and it was wrong +because it was read off `op.create_index("ix_image_record_sha256", ...)` at +0001 line 151 without reading line 149 two lines above it: + + sa.UniqueConstraint("sha256", name="uq_image_record_sha256"), + +Uniqueness has been enforced since the initial schema. The database simply +expresses it as a CONSTRAINT plus a separate non-unique lookup index, where +the model expressed it as one `unique=True, index=True` column — the same +guarantee built from different objects, which is why the two schemas did not +line up. The model now declares the constraint and the plain index separately, +so it describes what is actually there. No DDL is needed for it. Revision ID: 0088 Revises: 0087 diff --git a/backend/app/models/head_metrics_snapshot.py b/backend/app/models/head_metrics_snapshot.py index 651344e..dfde05e 100644 --- a/backend/app/models/head_metrics_snapshot.py +++ b/backend/app/models/head_metrics_snapshot.py @@ -19,8 +19,14 @@ class HeadMetricsSnapshot(Base): __tablename__ = "head_metrics_snapshot" id: Mapped[int] = mapped_column(Integer, primary_key=True) - tag_id: Mapped[int] = mapped_column( - ForeignKey("tag.id", ondelete="CASCADE"), index=True + # Nullable, matching alembic 0060, which declared this column without + # `nullable=False`. The model had it as `Mapped[int]` — NOT NULL — which + # was simply never true of the database (#3275). Left nullable rather than + # tightened: a snapshot of a tag that is later hard-deleted is a row worth + # keeping, and the FK is ON DELETE CASCADE, so tightening it would only + # change behaviour, not correct a bug. + tag_id: Mapped[int | None] = mapped_column( + ForeignKey("tag.id", ondelete="CASCADE"), nullable=True, index=True ) # Denormalized so a snapshot stays readable even if the tag is later renamed. name: Mapped[str] = mapped_column(String(255), nullable=False) diff --git a/backend/app/models/image_provenance.py b/backend/app/models/image_provenance.py index fb18178..2ce95de 100644 --- a/backend/app/models/image_provenance.py +++ b/backend/app/models/image_provenance.py @@ -47,8 +47,15 @@ class ImageProvenance(Base): # attachment on the post. NULL for loose downloads and pre-backfill rows. # SET NULL so deleting the archive attachment never destroys the (image, # post) edge — it just forgets which archive it came from. + # FK named explicitly: the convention renders this + # `fk_image_provenance_from_attachment_id_post_attachment`, but alembic + # 0055 created it as `fk_image_provenance_from_attachment` (#3275). from_attachment_id: Mapped[int | None] = mapped_column( - ForeignKey("post_attachment.id", ondelete="SET NULL"), + ForeignKey( + "post_attachment.id", + ondelete="SET NULL", + name="fk_image_provenance_from_attachment", + ), nullable=True, index=True, ) captured_metadata: Mapped[dict | None] = mapped_column(JSON, nullable=True) diff --git a/backend/app/models/image_record.py b/backend/app/models/image_record.py index 3de0a3f..0fae950 100644 --- a/backend/app/models/image_record.py +++ b/backend/app/models/image_record.py @@ -18,6 +18,7 @@ from sqlalchemy import ( Integer, String, Text, + UniqueConstraint, func, text, ) @@ -33,6 +34,12 @@ class ImageRecord(Base): __table_args__ = ( + # alembic 0001. The database enforces sha256 uniqueness with a + # CONSTRAINT and carries a SEPARATE non-unique btree index; the model + # said `unique=True, index=True`, which collapses both into a single + # UNIQUE index under a different name. Same guarantee either way, but + # not the same objects, so autogenerate saw a drop and an add (#3275). + UniqueConstraint("sha256", name="uq_image_record_sha256"), # alembic 0036, and the last thing in this schema that lived only in a # migration. SQLAlchemy CAN express an hnsw index with an operator # class, so there is no reason for it to be invisible to the models — @@ -52,7 +59,9 @@ class ImageRecord(Base): # On-disk identity path: Mapped[str] = mapped_column(Text, nullable=False, unique=True) - sha256: Mapped[str] = mapped_column(String(64), nullable=False, unique=True, index=True) + # index=True only: the UNIQUE half is the named constraint in + # __table_args__ above, matching what 0001 actually created. + sha256: Mapped[str] = mapped_column(String(64), nullable=False, index=True) phash: Mapped[str | None] = mapped_column(String(32), nullable=True, index=True) size_bytes: Mapped[int] = mapped_column(BigInteger, nullable=False) mime: Mapped[str] = mapped_column(String(64), nullable=False) @@ -92,8 +101,15 @@ class ImageRecord(Base): ) # FC-2d-vii-c: canonical per-image artist (the single source of truth # for attribution; provenance posts remain lineage detail). + # FK named explicitly: the naming convention renders this + # `fk_image_record_artist_id_artist`, but alembic 0008 created it as + # `fk_image_record_artist_id` (#3275). artist_id: Mapped[int | None] = mapped_column( - ForeignKey("artist.id", ondelete="SET NULL"), nullable=True, index=True + ForeignKey( + "artist.id", ondelete="SET NULL", name="fk_image_record_artist_id" + ), + nullable=True, + index=True, ) # ML fields (populated by the ml-worker / GPU agent). 1152 = SigLIP-so400m diff --git a/backend/app/models/import_settings.py b/backend/app/models/import_settings.py index 74ac13f..78c0fab 100644 --- a/backend/app/models/import_settings.py +++ b/backend/app/models/import_settings.py @@ -4,7 +4,15 @@ Enforced as a single row via a CHECK (id = 1) constraint. The application always SELECTs id=1 and never inserts/deletes after the initial migration. """ -from sqlalchemy import Boolean, CheckConstraint, Float, Integer, Text, select +from sqlalchemy import ( + Boolean, + CheckConstraint, + Float, + Integer, + Text, + select, + text, +) from sqlalchemy.orm import Mapped, mapped_column from .base import Base @@ -124,7 +132,9 @@ class ImportSettings(Base): # English (e.g. "… WIP Part 1") as a European language at ~0.86. CJK stays # trusted regardless (script-detected). Per-post overrides handle the misses. translation_min_confidence: Mapped[float] = mapped_column( - Float, nullable=False, default=0.9, server_default="0.9", + # text() because alembic 0084 used sa.text(); see ml_settings for why + # the form matters and why it is per-column (#3275). + Float, nullable=False, default=0.9, server_default=text("0.9"), ) # Title-based WIP auto-tagging (task #1458). When a freshly-imported post's diff --git a/backend/app/models/ml_settings.py b/backend/app/models/ml_settings.py index 3e1570a..d70449c 100644 --- a/backend/app/models/ml_settings.py +++ b/backend/app/models/ml_settings.py @@ -11,6 +11,7 @@ from sqlalchemy import ( String, func, select, + text, ) from sqlalchemy.orm import Mapped, mapped_column @@ -110,11 +111,16 @@ class MLSettings(Base): ) presentation_auto_apply_threshold: Mapped[float] = mapped_column( Float, nullable=False, default=0.90, - server_default="0.90", + # text(), not a string, because alembic 0082 used sa.text(): a bare + # string renders DEFAULT '0.90'::double precision while text() renders + # DEFAULT 0.90, and the chain is MIXED — some migrations used one, + # some the other. Same value, different stored expression, so each + # column here mirrors whichever form its own migration used (#3275). + server_default=text("0.90"), ) presentation_conflict_threshold: Mapped[float] = mapped_column( Float, nullable=False, default=0.50, - server_default="0.50", + server_default=text("0.50"), ) # -- Process auto-apply (#1464) ---------------------------------------- # `wip` / `editor screenshot` are PROCESS art — unfinished pieces + program @@ -133,11 +139,11 @@ class MLSettings(Base): ) process_auto_apply_threshold: Mapped[float] = mapped_column( Float, nullable=False, default=0.90, - server_default="0.9", + server_default="0.90", ) process_conflict_threshold: Mapped[float] = mapped_column( Float, nullable=False, default=0.50, - server_default="0.5", + server_default="0.50", ) # Default = SigLIP 2 (so400m, 512px) for new installs (migration 0069); # existing libraries keep their stored value until the operator re-embeds. @@ -172,7 +178,7 @@ class MLSettings(Base): ) detector_person_conf: Mapped[float] = mapped_column( Float, nullable=False, default=0.35, - server_default="0.35", + server_default=text("0.35"), ) # anatomy: booru_yolo anime/furry/NSFW torso components → concept crops. # Default = yolov11m_aa22 (26 classes, best mAP50-95 0.96), committed in the @@ -192,7 +198,7 @@ class MLSettings(Base): ) detector_anatomy_conf: Mapped[float] = mapped_column( Float, nullable=False, default=0.30, - server_default="0.30", + server_default=text("0.30"), ) # panel: comic page → panel regions → concept crops (Apache-2.0, YOLOv12x). detector_panel_enabled: Mapped[bool] = mapped_column( @@ -206,7 +212,7 @@ class MLSettings(Base): ) detector_panel_conf: Mapped[float] = mapped_column( Float, nullable=False, default=0.30, - server_default="0.30", + server_default=text("0.30"), ) # Per-frame caps bound the crop→embed explosion; max_regions is the hard # per-job backstop; dedupe_iou drops near-duplicate crops before the embed. @@ -228,7 +234,7 @@ class MLSettings(Base): ) detector_dedupe_iou: Mapped[float] = mapped_column( Float, nullable=False, default=0.85, - server_default="0.85", + server_default=text("0.85"), ) # -- CCIP character prototypes (#1317) --------------------------------- # The per-character reference set is precomputed + refreshed INCREMENTALLY diff --git a/backend/app/models/series_chapter.py b/backend/app/models/series_chapter.py index ff7698a..87fc5c2 100644 --- a/backend/app/models/series_chapter.py +++ b/backend/app/models/series_chapter.py @@ -16,7 +16,14 @@ title is the optional chapter name; stated_part is the optional operator-facing from datetime import datetime -from sqlalchemy import DateTime, ForeignKey, Integer, Text, func +from sqlalchemy import ( + DateTime, + ForeignKey, + Integer, + Text, + UniqueConstraint, + func, +) from sqlalchemy.orm import Mapped, mapped_column from .base import Base @@ -25,14 +32,26 @@ from .base import Base class SeriesChapter(Base): __tablename__ = "series_chapter" + __table_args__ = ( + # alembic 0047 named the UNIQUE `uq_series_chapter_anchor_page`, not + # the `uq_series_chapter_anchor_page_id` a bare `unique=True` would + # render (#3275). + UniqueConstraint("anchor_page_id", name="uq_series_chapter_anchor_page"), + ) + id: Mapped[int] = mapped_column(Integer, primary_key=True) series_tag_id: Mapped[int] = mapped_column( ForeignKey("tag.id", ondelete="CASCADE"), nullable=False, index=True ) + # Both the UNIQUE (above) and the FK carry the names 0047 gave them; the + # convention would render the FK `fk_series_chapter_anchor_page_id_series_page`. anchor_page_id: Mapped[int] = mapped_column( - ForeignKey("series_page.id", ondelete="CASCADE"), + ForeignKey( + "series_page.id", + ondelete="CASCADE", + name="fk_series_chapter_anchor_page", + ), nullable=False, - unique=True, ) title: Mapped[str | None] = mapped_column(Text, nullable=True) stated_part: Mapped[int | None] = mapped_column(Integer, nullable=True) diff --git a/backend/app/models/series_page.py b/backend/app/models/series_page.py index 0bb0b40..22e19e1 100644 --- a/backend/app/models/series_page.py +++ b/backend/app/models/series_page.py @@ -14,7 +14,14 @@ number parsed from the source post, nullable when unknown. from datetime import datetime -from sqlalchemy import DateTime, ForeignKey, Integer, String, func +from sqlalchemy import ( + DateTime, + ForeignKey, + Integer, + String, + UniqueConstraint, + func, +) from sqlalchemy.orm import Mapped, mapped_column from .base import Base @@ -23,14 +30,22 @@ from .base import Base class SeriesPage(Base): __tablename__ = "series_page" + __table_args__ = ( + # alembic 0005 named this `uq_series_page_image`; a bare `unique=True` + # on the column renders `uq_series_page_image_id` under the naming + # convention, which is a different object from the one the database + # has (#3275). + UniqueConstraint("image_id", name="uq_series_page_image"), + ) + id: Mapped[int] = mapped_column(Integer, primary_key=True) series_tag_id: Mapped[int] = mapped_column( ForeignKey("tag.id", ondelete="CASCADE"), nullable=False, index=True ) + # UNIQUE lives in __table_args__ above, under the name 0005 gave it. image_id: Mapped[int] = mapped_column( ForeignKey("image_record.id", ondelete="CASCADE"), nullable=False, - unique=True, ) # 'placed' = in the series-global run (page_number set); 'pending' = staged # from a post awaiting the operator's sort (page_number NULL). (#789 P2) diff --git a/backend/app/models/source.py b/backend/app/models/source.py index 1c29e6f..8538bda 100644 --- a/backend/app/models/source.py +++ b/backend/app/models/source.py @@ -5,7 +5,16 @@ Multiple sources per artist support creators with cross-platform presence. from datetime import datetime -from sqlalchemy import Boolean, DateTime, ForeignKey, Integer, JSON, String, Text +from sqlalchemy import ( + Boolean, + DateTime, + ForeignKey, + Integer, + JSON, + String, + Text, + UniqueConstraint, +) from sqlalchemy.orm import Mapped, mapped_column, relationship from .base import Base @@ -14,6 +23,20 @@ from .base import Base class Source(Base): __tablename__ = "source" + __table_args__ = ( + # alembic 0010. One row per (artist, platform, url): re-adding a source + # the artist already has is an update, not a second row. The model had + # never declared it (#3275), so autogenerate would have proposed + # DROPPING it — the guarantee existed only in the migration chain. + # + # Named explicitly because the naming convention would render this + # `uq_source_artist_id` (uq keys off column_0_name), which is both + # wrong about the shape and not what the database actually has. + UniqueConstraint( + "artist_id", "platform", "url", name="uq_source_artist_platform_url" + ), + ) + id: Mapped[int] = mapped_column(Integer, primary_key=True) artist_id: Mapped[int] = mapped_column( ForeignKey("artist.id", ondelete="CASCADE"), nullable=False, index=True diff --git a/backend/app/models/tag_suggestion_rejection.py b/backend/app/models/tag_suggestion_rejection.py index 02756de..341f67f 100644 --- a/backend/app/models/tag_suggestion_rejection.py +++ b/backend/app/models/tag_suggestion_rejection.py @@ -19,11 +19,19 @@ class TagSuggestionRejection(Base): # Named explicitly; see tag_alias for why (#3275). Index("ix_tag_suggestion_rejection_tag", "tag_id"), ) + # Both FKs named explicitly. alembic 0003 used a hand-shortened `tsr` + # prefix; the convention would render the full table name (#3275). image_record_id: Mapped[int] = mapped_column( - ForeignKey("image_record.id", ondelete="CASCADE"), primary_key=True + ForeignKey( + "image_record.id", + ondelete="CASCADE", + name="fk_tsr_image_record_id_image_record", + ), + primary_key=True, ) tag_id: Mapped[int] = mapped_column( - ForeignKey("tag.id", ondelete="CASCADE"), primary_key=True + ForeignKey("tag.id", ondelete="CASCADE", name="fk_tsr_tag_id_tag"), + primary_key=True, ) rejected_at: Mapped[datetime] = mapped_column( DateTime(timezone=True), nullable=False, server_default=func.now()