From bd3f99658237454ee481150d900e7f9c6105a516 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 27 May 2026 15:35:25 -0400 Subject: [PATCH] fix(sidecar): correct external_post_id + post_url derivation for non-Patreon platforms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit of one sample sidecar per platform on the operator's /mnt/Data/Patreon/ archive surfaced three parser bugs that have been silently corrupting non-Patreon Posts since FC-3 shipped: 1. SubscribeStar `id` vs `post_id` confusion. gallery-dl puts the per-attachment id in `id` (e.g. 711509) and the actual post id in `post_id` (e.g. 360360). FC's external_post_id chain had `id` winning, so every multi-image SubscribeStar post was fragmented into N Post rows in the database. Reorder the chain to `("post_id", "id", "index", "message_id")` — Patreon/Pixiv (no `post_id`), HF (uses `index`), Discord (uses `message_id`) all unaffected. 2. Discord `message` field not captured. Discord posts put the body in `message`, not `content`. Append it to the description fallback chain `("content", "description", "caption", "message")`. 3. post_url is the file URL on SubscribeStar/Pixiv/HF/Discord. New `_derive_post_url(platform, data)` helper synthesizes proper permalinks from per-platform fields: subscribestar → https://www.subscribestar.com/posts/ pixiv → https://www.pixiv.net/artworks/ hentaifoundry → https://www.hentai-foundry.com/pictures/user// discord → https://discord.com/channels/// Patreon's bare `url` IS a real permalink and is used as-is. For the four file-URL platforms, the bare `url` is NEVER trusted: derive or return None rather than persist a CDN URL. Tests: - `test_parse_core_fields_and_id_priority` flipped to assert post_id wins over id. - New `test_parse_id_used_when_no_post_id` covers the Patreon real shape. - New `test_parse_message_used_as_description_fallback` covers Discord bodies. - Five new tests cover per-platform post_url derivation (SubscribeStar/Pixiv/HF/Discord/Patreon-untouched + missing-fields → None). Cleanup migration alembic 0025_fix_subscribestar_post_ids: - For each SubscribeStar Post: find a related ImageRecord.path, walk to its sidecar JSON, read `post_id`, overwrite Post.external_post_id + post_url with the corrected values. - After all updates, every group of Posts under one source sharing the same NEW external_post_id is a fragment-set — merge to a canonical row using the same ImageProvenance pre-delete + repoint dance as alembic 0022 (banked pattern). - Pure-SQL backfill of Pixiv post_url: replace any `i.pximg.net`-shape url with the derived `/artworks/` permalink. - HF and Discord post_url backfills skipped — HF would need the `user` field (not stored on Post), Discord needs server/channel triple. Both will be corrected by a deep-scan re-applying sidecars through the new parser. Idempotent: re-running on already-corrected data is a no-op. --- .../0025_fix_subscribestar_post_ids.py | 250 ++++++++++++++++++ backend/app/utils/sidecar.py | 65 ++++- tests/test_sidecar_util.py | 102 ++++++- 3 files changed, 412 insertions(+), 5 deletions(-) create mode 100644 alembic/versions/0025_fix_subscribestar_post_ids.py diff --git a/alembic/versions/0025_fix_subscribestar_post_ids.py b/alembic/versions/0025_fix_subscribestar_post_ids.py new file mode 100644 index 0000000..fb4fa49 --- /dev/null +++ b/alembic/versions/0025_fix_subscribestar_post_ids.py @@ -0,0 +1,250 @@ +"""subscribestar: correct external_post_id + post_url, merge fragmented Posts + +Revision ID: 0025 +Revises: 0024 +Create Date: 2026-05-27 + +Closes the operator-flagged 2026-05-27 SubscribeStar data-fragmentation +issue: gallery-dl writes the per-attachment id in `id` and the actual +post id in `post_id`. FC's sidecar parser had `id` winning the +external_post_id fallback chain, so every multi-image SubscribeStar post +was fragmented into N Post rows (one per attachment) in the database. + +The parser fix (sidecar.py, same commit) reorders the chain so `post_id` +wins. This migration cleans up the existing fragmented data. + +Strategy per SubscribeStar Post in the database: + 1. Find a related ImageRecord via ImageProvenance to get an on-disk + path. Walk to its sidecar JSON (gallery-dl's NN_ numbering prefix + convention is honored). + 2. Read the sidecar; if it has `post_id`, that's the correct + external_post_id. + 3. Update Post.external_post_id (= post_id) and Post.post_url (= + derived https://www.subscribestar.com/posts/). + +After all rows are updated, every group of Posts under the same source +that share the same NEW external_post_id is a duplicate set — +fragments of the same actual post. Merge them: pick canonical (lowest +id), repoint ImageProvenance + ImageRecord.primary_post_id + +PostAttachment to canonical, dedupe ImageProvenance against alembic +0021's UNIQUE, delete drop Posts. + +Also: pure-SQL backfill of Pixiv post_url (Pixiv sidecars also write a +file URL in `url`; the post permalink is /artworks/ and FC's parser +now derives it). Pixiv external_post_id is already correct (gallery-dl's +`id` matches the URL pattern) so no per-row sidecar IO needed. + +HF + Discord post_url backfills are skipped here — HF needs the user +field (not stored on Post directly), Discord needs server/channel +triple. Both will eventually be corrected by a deep-scan that +re-applies sidecars via the new parser. + +Idempotent: re-running on already-corrected data is a no-op (the post_id +in sidecar matches what's already in DB). +""" +from __future__ import annotations + +import json +import re +from pathlib import Path +from typing import Sequence, Union + +from alembic import op +from sqlalchemy import text + +revision: str = "0025" +down_revision: Union[str, None] = "0024" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +# Mirror of sidecar._NUMBERING_PREFIX. Kept inline so the migration is +# self-contained (the operator's banked rule: +# reference_postgres_enum_swap_drop_checks.md says migrations shouldn't +# import from runtime app code). +_NUMBERING_PREFIX = re.compile(r"^\d+_(.+)$") + + +def _find_sidecar(media_path: Path) -> Path | None: + """gallery-dl writes the sidecar under the unprefixed stem + (`HOLLOW-ICHIGO.json`) while the media file gets a NN_ ordering + prefix (`01_HOLLOW-ICHIGO.png`). Try in order: + 1. .json next to the media + 2. .json next to the media (full-name variant) + 3. strip the NN_ prefix from the stem, then .json + """ + if not media_path: + return None + cand = media_path.with_suffix(".json") + if cand.is_file(): + return cand + cand = media_path.parent / f"{media_path.name}.json" + if cand.is_file(): + return cand + m = _NUMBERING_PREFIX.match(media_path.stem) + if m: + cand = media_path.parent / f"{m.group(1)}.json" + if cand.is_file(): + return cand + return None + + +def upgrade() -> None: + conn = op.get_bind() + + # ── PART 1: SubscribeStar post_id correction ───────────────────── + ss_posts = conn.execute(text(""" + SELECT p.id, p.external_post_id, p.source_id, p.post_url + FROM post p + JOIN source s ON s.id = p.source_id + WHERE s.platform = 'subscribestar' + """)).fetchall() + + sidecars_read = 0 + posts_updated = 0 + posts_missing_sidecar = 0 + for post_row in ss_posts: + path = _first_attachment_path(conn, post_row.id) + if not path: + posts_missing_sidecar += 1 + continue + sidecar = _find_sidecar(Path(path)) + if sidecar is None: + posts_missing_sidecar += 1 + continue + try: + data = json.loads(sidecar.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError): + posts_missing_sidecar += 1 + continue + sidecars_read += 1 + post_id = data.get("post_id") + if not isinstance(post_id, (str, int)) or isinstance(post_id, bool): + continue + post_id_str = str(post_id).strip() + if not post_id_str: + continue + new_url = f"https://www.subscribestar.com/posts/{post_id_str}" + # Idempotent: skip if already correct. + if (post_row.external_post_id == post_id_str + and post_row.post_url == new_url): + continue + conn.execute( + text(""" + UPDATE post + SET external_post_id = :epid, post_url = :url + WHERE id = :id + """), + {"epid": post_id_str, "url": new_url, "id": post_row.id}, + ) + posts_updated += 1 + + print( + f"0025: subscribestar — read {sidecars_read} sidecars, " + f"updated {posts_updated} Posts, " + f"{posts_missing_sidecar} Posts had no resolvable sidecar" + ) + + # ── PART 2: Merge SubscribeStar fragments now sharing epid ─────── + # After Part 1, each group of Posts under one source with the SAME + # new external_post_id is a fragment-set of the same actual post. + # Merge to one canonical row. Pre-handle the same ImageProvenance + # collision pattern as alembic 0022 (uq_image_provenance_image_post). + fragment_groups = conn.execute(text(""" + SELECT source_id, external_post_id, ARRAY_AGG(id ORDER BY id ASC) AS post_ids + FROM post p + JOIN source s ON s.id = p.source_id + WHERE s.platform = 'subscribestar' + AND p.external_post_id IS NOT NULL + GROUP BY source_id, external_post_id + HAVING COUNT(*) > 1 + """)).fetchall() + + merged = 0 + for grp in fragment_groups: + post_ids = list(grp.post_ids) + keep_id, *drop_ids = post_ids + for drop_id in drop_ids: + # Pre-DELETE colliding ImageProvenance under drop_ that + # already exist under keep (alembic 0022 banked the pattern). + conn.execute( + text(""" + DELETE FROM image_provenance + WHERE post_id = :drop_ + AND image_record_id IN ( + SELECT image_record_id FROM image_provenance + WHERE post_id = :keep + ) + """), + {"keep": keep_id, "drop_": drop_id}, + ) + conn.execute( + text(""" + UPDATE image_provenance SET post_id = :keep + WHERE post_id = :drop_ + """), + {"keep": keep_id, "drop_": drop_id}, + ) + conn.execute( + text(""" + UPDATE image_record SET primary_post_id = :keep + WHERE primary_post_id = :drop_ + """), + {"keep": keep_id, "drop_": drop_id}, + ) + conn.execute( + text(""" + UPDATE post_attachment SET post_id = :keep + WHERE post_id = :drop_ + """), + {"keep": keep_id, "drop_": drop_id}, + ) + conn.execute( + text("DELETE FROM post WHERE id = :drop_"), + {"drop_": drop_id}, + ) + merged += 1 + print(f"0025: subscribestar — merged {merged} duplicate Post fragments") + + # ── PART 3: Pixiv post_url backfill (pure SQL) ─────────────────── + # Pixiv's external_post_id is already correct (gallery-dl's `id` is + # the post id). Only post_url needs derivation: replace anything + # under i.pximg.net (the file URL) with the /artworks/ permalink. + pixiv_updated = conn.execute(text(""" + UPDATE post p + SET post_url = 'https://www.pixiv.net/artworks/' || p.external_post_id + FROM source s + WHERE p.source_id = s.id + AND s.platform = 'pixiv' + AND p.external_post_id IS NOT NULL + AND (p.post_url IS NULL + OR p.post_url LIKE 'https://i.pximg.net/%' + OR p.post_url LIKE 'http://i.pximg.net/%') + """)).rowcount + print(f"0025: pixiv — backfilled post_url on {pixiv_updated} Posts") + + +def _first_attachment_path(conn, post_id: int) -> str | None: + """Return any ImageRecord.path attached to this post (via + ImageProvenance). Lowest-id row keeps the migration deterministic + so re-running on the same DB picks the same sidecar.""" + row = conn.execute( + text(""" + SELECT ir.path + FROM image_provenance ip + JOIN image_record ir ON ir.id = ip.image_record_id + WHERE ip.post_id = :pid + ORDER BY ip.id ASC + LIMIT 1 + """), + {"pid": post_id}, + ).first() + return row[0] if row else None + + +def downgrade() -> None: + # Lossy: external_post_id values were overwritten with the correct + # post_id; original per-attachment ids weren't preserved. Post-merge + # also deleted drop rows. No safe restore. To roll back the schema + # invariant, fork from 0024 and re-run sidecar imports. + pass diff --git a/backend/app/utils/sidecar.py b/backend/app/utils/sidecar.py index 19b1878..13ced7c 100644 --- a/backend/app/utils/sidecar.py +++ b/backend/app/utils/sidecar.py @@ -111,8 +111,16 @@ def parse_sidecar(data: dict) -> SidecarData: cat = data.get("category") platform = cat if isinstance(cat, str) and cat.strip() else None + # external_post_id lookup order: post_id MUST come before id. + # SubscribeStar gallery-dl writes the per-attachment id in `id` + # (e.g. 711509) and the actual post id in `post_id` (e.g. 360360); + # picking `id` first fragments every multi-image subscribestar post + # into N distinct Post rows in FC. Patreon/Pixiv have no `post_id` + # so `id` still wins for them; HF uses `index`, Discord uses + # `message_id` — all reached via the remaining chain entries. + # Operator-flagged 2026-05-27 during the sidecar audit. external_post_id = None - for k in ("id", "post_id", "index", "message_id"): + for k in ("post_id", "id", "index", "message_id"): v = data.get(k) if isinstance(v, bool): continue @@ -138,7 +146,11 @@ def parse_sidecar(data: dict) -> SidecarData: if post_date is not None: break - description = _first_str(data, ("content", "description", "caption")) + # `message` is Discord gallery-dl's body field (no `content`); added + # 2026-05-27 to the description fallback chain. + description = _first_str( + data, ("content", "description", "caption", "message"), + ) # SubscribeStar posts always write `title: ""` and put the leading # sentence inside `content` (confirmed against the operator's @@ -150,13 +162,60 @@ def parse_sidecar(data: dict) -> SidecarData: if post_title is None and description: post_title = _first_line_text(description) + # post_url derivation: SubscribeStar/Pixiv/HF/Discord put the FILE + # download URL in `url`, not a post permalink. Synthesize the + # permalink from per-platform fields when possible. Patreon's `url` + # IS a permalink and is used as-is. For the four file-URL platforms, + # the bare `url` is NEVER trusted — derive or return None rather + # than persist a CDN URL in post.post_url. + if platform in _DERIVED_URL_PLATFORMS: + post_url = _derive_post_url(platform, data) + else: + post_url = _first_str(data, ("url", "post_url")) + return SidecarData( platform=platform, external_post_id=external_post_id, - post_url=_first_str(data, ("url", "post_url")), + post_url=post_url, post_title=post_title, description=description, attachment_count=attachment_count, post_date=post_date, raw=data, ) + + +_DERIVED_URL_PLATFORMS = frozenset({ + "subscribestar", "pixiv", "hentaifoundry", "discord", +}) + + +def _derive_post_url(platform: str, data: dict) -> str | None: + """Synthesize the post-permalink URL from per-platform metadata. + + gallery-dl writes the file-download URL in `url` for these four + platforms; we need a real permalink for the PostCard "open original" + button. Returns None if the platform-specific fields are missing + (rare in well-formed sidecars but defensive). + """ + if platform == "subscribestar": + pid = data.get("post_id") + if isinstance(pid, (str, int)) and str(pid).strip(): + return f"https://www.subscribestar.com/posts/{pid}" + elif platform == "pixiv": + pid = data.get("id") + if isinstance(pid, (str, int)) and not isinstance(pid, bool) and str(pid).strip(): + return f"https://www.pixiv.net/artworks/{pid}" + elif platform == "hentaifoundry": + user = _first_str(data, ("user", "artist")) + idx = data.get("index") + if user and isinstance(idx, (str, int)) and not isinstance(idx, bool) and str(idx).strip(): + return f"https://www.hentai-foundry.com/pictures/user/{user}/{idx}" + elif platform == "discord": + sid = data.get("server_id") + cid = data.get("channel_id") + mid = data.get("message_id") + if all(isinstance(v, (str, int)) and not isinstance(v, bool) and str(v).strip() + for v in (sid, cid, mid)): + return f"https://discord.com/channels/{sid}/{cid}/{mid}" + return None diff --git a/tests/test_sidecar_util.py b/tests/test_sidecar_util.py index 2edd259..747e944 100644 --- a/tests/test_sidecar_util.py +++ b/tests/test_sidecar_util.py @@ -78,6 +78,10 @@ def test_parse_empty_dict_all_none(): def test_parse_core_fields_and_id_priority(): + """`post_id` MUST win over `id` (SubscribeStar duplicate-post fix). + Patreon sidecars in the wild don't expose post_id; this test uses + `category=patreon` but synthetically sets both fields to pin the + parser's precedence.""" sd = parse_sidecar({ "category": "patreon", "id": 12345, "post_id": 999, @@ -88,7 +92,7 @@ def test_parse_core_fields_and_id_priority(): "published_at": "2023-08-01T04:20:02Z", }) assert sd.platform == "patreon" - assert sd.external_post_id == "12345" # 'id' wins over 'post_id' + assert sd.external_post_id == "999" # 'post_id' wins over 'id' assert sd.post_url == "https://patreon.com/posts/12345" assert sd.post_title == "Hello" assert sd.description == "

body

" @@ -96,13 +100,25 @@ def test_parse_core_fields_and_id_priority(): assert sd.post_date.year == 2023 and sd.post_date.tzinfo is not None +def test_parse_id_used_when_no_post_id(): + """Without post_id (Patreon/Pixiv/Discord real shape), `id` wins.""" + sd = parse_sidecar({"id": 12345, "url": "https://example.test/p/1"}) + assert sd.external_post_id == "12345" + + def test_parse_description_precedence_and_images_count(): sd = parse_sidecar({"description": "d", "caption": "c", "images": [1, 2, 3]}) - assert sd.description == "d" # content>description>caption + assert sd.description == "d" # content>description>caption>message assert sd.attachment_count == 3 # len(images) fallback +def test_parse_message_used_as_description_fallback(): + """Discord posts have `message` not `content`; FC must surface it.""" + sd = parse_sidecar({"category": "discord", "message": "hello channel"}) + assert sd.description == "hello channel" + + def test_parse_date_epoch_and_unparseable_and_naive(): assert parse_sidecar({"timestamp": 1690857602}).post_date.tzinfo is not None assert parse_sidecar({"date": "not-a-date"}).post_date is None @@ -143,6 +159,88 @@ def test_parse_title_no_fallback_when_no_content(): assert sd.post_title is None +def test_parse_subscribestar_post_url_derived_and_post_id_wins(): + """SubscribeStar gallery-dl puts the per-attachment id in `id` and + the actual post id in `post_id`. The bare `url` is the file URL — + must be ignored and a derived permalink used instead.""" + sd = parse_sidecar({ + "category": "subscribestar", + "id": 711509, "post_id": 360360, + "url": "/post_uploads?payload=opaque", + "title": "", + "content": "
hello
", + }) + assert sd.external_post_id == "360360" + assert sd.post_url == "https://www.subscribestar.com/posts/360360" + + +def test_parse_pixiv_post_url_derived(): + """Pixiv's `url` is the image URL (i.pximg.net); must be replaced + with the post permalink under /artworks/.""" + sd = parse_sidecar({ + "category": "pixiv", + "id": 140466853, + "url": "https://i.pximg.net/img-original/img/2026/01/28/10/28/24/140466853_p0.jpg", + "title": "Nerissa x Jailbird", + }) + assert sd.external_post_id == "140466853" + assert sd.post_url == "https://www.pixiv.net/artworks/140466853" + + +def test_parse_hentaifoundry_post_url_derived(): + """HF sidecars omit `url` entirely and use `index`+`user` for the + post's natural key. Synthesize the canonical /pictures/user// + permalink.""" + sd = parse_sidecar({ + "category": "hentaifoundry", + "index": 1182595, + "user": "HolyMeh", + "title": "Annigosa", + }) + assert sd.external_post_id == "1182595" + assert sd.post_url == "https://www.hentai-foundry.com/pictures/user/HolyMeh/1182595" + + +def test_parse_discord_post_url_derived_and_message_id_wins(): + """Discord posts use `message_id` for the post key and the + server/channel/message triple for the permalink.""" + sd = parse_sidecar({ + "category": "discord", + "message_id": "1195924119762505818", + "channel_id": "968315530597498880", + "server_id": "771088957849075793", + "message": "channel body text", + "url": "https://cdn.discordapp.com/attachments/file.png", + }) + assert sd.external_post_id == "1195924119762505818" + assert sd.post_url == ( + "https://discord.com/channels/771088957849075793/" + "968315530597498880/1195924119762505818" + ) + assert sd.description == "channel body text" + + +def test_parse_patreon_post_url_kept_as_is(): + """Patreon's bare `url` IS a real permalink — must not be replaced.""" + sd = parse_sidecar({ + "category": "patreon", + "id": 47074733, + "url": "https://www.patreon.com/posts/barbara-genshin-47074733", + }) + assert sd.post_url == "https://www.patreon.com/posts/barbara-genshin-47074733" + + +def test_parse_derived_url_returns_none_when_fields_missing(): + """If the per-platform fields needed to derive the URL are missing, + return None rather than fall back to the file `url`.""" + sd = parse_sidecar({ + "category": "subscribestar", + "url": "/post_uploads?payload=opaque", + # no post_id + }) + assert sd.post_url is None + + def test_parse_ignores_non_str_junk(): sd = parse_sidecar({"category": 5, "title": 7, "page_count": "x", "id": True})