From 007592827c74245332e019a6c58e1d9fd8497381 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 24 Apr 2026 10:49:23 -0400 Subject: [PATCH] fix(suggestions): parse (Fandom) suffix on accept + heal legacy rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two-part fix for the reported bug where accepting a character suggestion named 'Jinx (League Of Legends)' created a single malformed character tag with fandom_id=NULL and the whole suffix embedded in the name, and never attached the fandom tag to the image. Root cause: WD14 predictions store names like 'jinx_(league_of_legends)'. app.services.tag_suggestions ._canonicalize_wd14_name transforms that to 'Jinx (League Of Legends)' for character/copyright categories. The suggestion chip renders that canonicalized string. On ✓, accept_image_suggestion looked for an existing character with name = 'Jinx (League Of Legends)' — post-refactor no such row exists (names are bare) — fell into the 0-candidate branch, and created a fresh malformed character. Going forward (main.py): accept_image_suggestion's character branch now splits a '(Fandom)' suffix off the incoming name before the 0/1/N lookup. If it splits, find-or-create the fandom tag, then find-or-create a character with (kind='character', name=bare, fandom_id=). If no suffix, unchanged 0/1/N behavior. Backfill for already-corrupted rows (tasks/maintenance.py): New _heal_malformed_character_names pass runs before the existing sync-fandoms-to-images logic. Finds (kind='character', fandom_id IS NULL, name LIKE '% (%)%'), parses the suffix via the same regex, and either promotes the row to (name=bare, fandom_id=) or merges into an existing canonical character. tag_reference_embedding rows (string PK) are cascade-renamed alongside. Same auto-merge semantics as migration j26042101 Phase 2. Implementation is all raw SQL to sidestep an ORM autoflush ordering quirk: ORM-level fandom INSERT + subsequent tag UPDATE referencing its fresh id hit a transient FK violation in worker task context. Raw SQL with an explicit commit after fandom insert avoids the ambiguity. Verified locally: - Fresh accept of 'CanonicalChar (Canonical Fandom)' against a pre-existing canonical character: resolved to the canonical row, fandom attached to image. - Seeded malformed 'LegacyJinx (League Of Legends)' character with fandom_id=NULL attached to an image: sync task healed it to bare 'LegacyJinx' + fandom_id, attached the fandom row to image_tags, final counts {healed:1, skipped:0, links_added:1}. Co-Authored-By: Claude Opus 4.7 --- app/main.py | 65 +++++++++++----- app/tasks/maintenance.py | 158 +++++++++++++++++++++++++++++++++++---- 2 files changed, 187 insertions(+), 36 deletions(-) diff --git a/app/main.py b/app/main.py index d55b2d9..1c5deab 100644 --- a/app/main.py +++ b/app/main.py @@ -872,27 +872,52 @@ def accept_image_suggestion(image_id): if tag is None: return jsonify(ok=False, error="tag_not_found"), 404 elif kind == "character": - candidates = Tag.query.filter_by(kind="character", name=name).all() - if len(candidates) == 1: - tag = candidates[0] - elif len(candidates) == 0: - tag = Tag(kind="character", name=name, fandom_id=None) - db.session.add(tag) - db.session.flush() + # WD14 emits character names in 'bare_(fandom)' form. After canonical + # case/underscore handling the incoming name may look like + # 'Soraka (League Of Legends)'. Before the post-refactor 0/1/N + # lookup, split the '(Fandom)' suffix and resolve the fandom to a + # fandom_id — otherwise the lookup misses every existing bare-name + # character and we'd create a new character whose entire name is + # 'Soraka (League Of Legends)' with fandom_id=NULL. + import re + fandom_suffix_match = re.match(r'^(.+?) \(([^()]+)\)$', name) + if fandom_suffix_match: + bare_name = fandom_suffix_match.group(1).strip() + suffix_fandom_name = fandom_suffix_match.group(2).strip() + f_tag = _ensure_fandom_tag(suffix_fandom_name) + candidates = ( + Tag.query + .filter_by(kind="character", name=bare_name, fandom_id=f_tag.id) + .all() + ) + if len(candidates) == 1: + tag = candidates[0] + else: + tag = Tag(kind="character", name=bare_name, fandom_id=f_tag.id) + db.session.add(tag) + db.session.flush() else: - return jsonify( - ok=False, - error="ambiguous", - candidates=[ - { - "id": c.id, - "display_name": c.display_name, - "fandom_id": c.fandom_id, - "fandom_name": c.fandom.name if c.fandom else None, - } - for c in candidates - ], - ), 409 + candidates = Tag.query.filter_by(kind="character", name=name).all() + if len(candidates) == 1: + tag = candidates[0] + elif len(candidates) == 0: + tag = Tag(kind="character", name=name, fandom_id=None) + db.session.add(tag) + db.session.flush() + else: + return jsonify( + ok=False, + error="ambiguous", + candidates=[ + { + "id": c.id, + "display_name": c.display_name, + "fandom_id": c.fandom_id, + "fandom_name": c.fandom.name if c.fandom else None, + } + for c in candidates + ], + ), 409 else: effective_kind = kind if kind else "user" tag = Tag.query.filter_by(kind=effective_kind, name=name).first() diff --git a/app/tasks/maintenance.py b/app/tasks/maintenance.py index 1429b89..fc49477 100644 --- a/app/tasks/maintenance.py +++ b/app/tasks/maintenance.py @@ -5,6 +5,7 @@ blocklisted tag names from the library. The earlier sync_character_fandoms task was removed on 2026-04-21 as part of the bare-name refactor. """ import logging +import re from celery import shared_task from sqlalchemy import text @@ -14,28 +15,152 @@ from app.models import Tag log = logging.getLogger(__name__) +_FANDOM_SUFFIX_RE = re.compile(r'^(.+?) \(([^()]+)\)$') + @shared_task( name='app.tasks.maintenance.sync_character_fandoms_to_images', soft_time_limit=300, time_limit=600, ) -def sync_character_fandoms_to_images() -> dict: - """For every character tag with a non-null fandom_id, ensure the fandom - tag is attached to every image that has the character attached. +def _heal_malformed_character_names() -> dict: + """Find characters whose name still contains a '(Fandom)' suffix and + whose fandom_id is NULL (typically created by a pre-fix suggestion + accept that didn't parse the WD14 suffix). Parse the suffix, ensure + the fandom tag exists, and either promote the malformed row to + (name=bare, fandom_id=) or merge it into an existing canonical + character with the same (bare_name, fandom_id) pair. - Backfills the gap left by migration j26042101 — the migration set - tag.fandom_id from the old '(Fandom)' suffix but did not retroactively - add fandom rows to image_tags. Set operations made AFTER the migration - via /api/tag//set-fandom do this inline, so running this task once - (or whenever the user suspects drift) is sufficient. - - Additive only — never removes fandom attachments. Idempotent via - ON CONFLICT DO NOTHING. - - Returns a summary with per-character counts so the result is - introspectable. + Same auto-merge semantics as migration j26042101 Phase 2. Uses raw + SQL throughout to avoid ORM autoflush ordering quirks between the + fandom INSERT and the subsequent tag UPDATE inside a single worker + task session. """ + rows = db.session.execute(text(""" + SELECT id, name FROM tag + WHERE kind = 'character' AND fandom_id IS NULL AND name LIKE '% (%)%' + """)).fetchall() + + healed = 0 + merged = 0 + skipped = 0 + + for row in rows: + tag_id = row[0] + old_name = row[1] + m = _FANDOM_SUFFIX_RE.match(old_name) + if not m: + skipped += 1 + continue + bare_name = m.group(1).strip() + fandom_name = m.group(2).strip() + if not bare_name or not fandom_name: + skipped += 1 + continue + + try: + # Find-or-create the fandom via raw SQL, commit so the FK from + # the subsequent character UPDATE can resolve. + fandom_row = db.session.execute( + text("SELECT id FROM tag WHERE kind='fandom' AND name=:n"), + {'n': fandom_name}, + ).fetchone() + if fandom_row is None: + fandom_id = db.session.execute( + text("INSERT INTO tag (kind, name) VALUES ('fandom', :n) RETURNING id"), + {'n': fandom_name}, + ).scalar_one() + else: + fandom_id = fandom_row[0] + db.session.commit() + + # Collision check against the post-refactor partial index. + canonical_row = db.session.execute( + text(""" + SELECT id FROM tag + WHERE kind='character' AND name=:n AND fandom_id=:f AND id<>:me + """), + {'n': bare_name, 'f': fandom_id, 'me': tag_id}, + ).fetchone() + + if canonical_row is not None: + canonical_id = canonical_row[0] + # Reassign image_tags onto the canonical, delete malformed's + # own image_tags rows. + db.session.execute(text(""" + INSERT INTO image_tags (image_id, tag_id) + SELECT DISTINCT image_id, :to_id FROM image_tags WHERE tag_id = :from_id + ON CONFLICT DO NOTHING + """), {'from_id': tag_id, 'to_id': canonical_id}) + db.session.execute( + text("DELETE FROM image_tags WHERE tag_id = :tid"), + {'tid': tag_id}, + ) + _cascade_ref_embedding_rename(old_name, bare_name) + db.session.execute( + text("DELETE FROM tag WHERE id = :tid"), + {'tid': tag_id}, + ) + merged += 1 + else: + _cascade_ref_embedding_rename(old_name, bare_name) + db.session.execute( + text("UPDATE tag SET name=:n, fandom_id=:f WHERE id=:tid"), + {'n': bare_name, 'f': fandom_id, 'tid': tag_id}, + ) + healed += 1 + db.session.commit() + except Exception as e: + db.session.rollback() + log.warning( + "heal_malformed_character_names: tag %s (%r) failed: %s", + tag_id, old_name, e, + ) + skipped += 1 + + return {'malformed_scanned': len(rows), 'healed': healed, 'merged': merged, 'skipped': skipped} + + +def _cascade_ref_embedding_rename(old_name: str, new_name: str) -> None: + """Rename tag_reference_embedding rows from old_name to new_name where + new_name doesn't already have a row for the same model_version; delete + leftover old_name rows after. Matches the pattern used by migration + j26042101.""" + if old_name == new_name: + return + db.session.execute(text(""" + UPDATE tag_reference_embedding SET tag_name = :new_name + WHERE tag_name = :old_name + AND NOT EXISTS ( + SELECT 1 FROM tag_reference_embedding w + WHERE w.tag_name = :new_name AND w.model_version = tag_reference_embedding.model_version + ) + """), {'old_name': old_name, 'new_name': new_name}) + db.session.execute( + text("DELETE FROM tag_reference_embedding WHERE tag_name = :old_name"), + {'old_name': old_name}, + ) + + +def sync_character_fandoms_to_images() -> dict: + """Idempotent two-pass maintenance: + + Pass A — heal characters whose 'name' still embeds a '(Fandom)' suffix + and whose fandom_id is NULL. These come from pre-fix suggestion + accepts that didn't parse the WD14 output. Splits the suffix, + ensures the fandom tag exists, promotes or merges the row. + + Pass B — for every character with a non-null fandom_id, attach the + fandom tag to every image that has the character attached. + Backfills the gap left by migration j26042101 (which populated + tag.fandom_id from old suffixes but didn't walk image_tags). + + Additive for image_tags; destructive only for malformed character rows + that merge into a canonical row. Safe to re-run — both passes are + idempotent. + """ + pass_a = _heal_malformed_character_names() + chars = ( Tag.query .filter_by(kind='character') @@ -68,10 +193,11 @@ def sync_character_fandoms_to_images() -> dict: char.id, char.name, e, ) log.info( - "sync_character_fandoms_to_images: processed %d characters, added %d links, %d failures", - characters_processed, total_links_added, failures, + "sync_character_fandoms_to_images: heal=%s, char_attach=%d processed, %d links added, %d failures", + pass_a, characters_processed, total_links_added, failures, ) return { + 'heal': pass_a, 'characters_scanned': len(chars), 'characters_processed': characters_processed, 'links_added': total_links_added,