Sidecar synthetic anchor cleanup + tier-gated classifier fix #39

Merged
bvandeusen merged 2 commits from dev into main 2026-06-01 00:16:58 -04:00
8 changed files with 380 additions and 23 deletions
@@ -0,0 +1,190 @@
"""collapse-sidecar-synthetic: repoint Posts/ImageProvenance/DownloadEvents
from `sidecar:<platform>:<slug>` synthetic Source anchors onto the real
Source for the same (artist, platform) when one exists, then delete the
synthetic.
Revision ID: 0028
Revises: 0027
Create Date: 2026-05-31
Background: alembic 0022 (2026-05-26) consolidated the old per-post-URL
Source rows into one canonical Source per (artist, platform). When NO
real campaign URL was salvageable among the candidates, it rewrote the
canonical row to url='sidecar:<platform>:<slug>' enabled=false as a
disabled anchor for any Posts already attached.
That was fine while it was the only Source for that artist+platform.
But: the unique constraint on Source is (artist_id, platform, url), not
(artist_id, platform). When the operator later added the real
subscription via the UI / extension / etc., a SECOND row landed —
the real one — with id > the synthetic. Both coexisted.
Two follow-on problems surfaced 2026-05-31:
1. The Subscriptions UI listed both rows. The synthetic was disabled
so the scheduler never polled it, but it looked like a phantom
subscription. (Fixed in same commit by SourceService.list filter.)
2. importer._source_for_sidecar picked Source by `ORDER BY id ASC
LIMIT 1`, so EVERY gallery-dl download since the real Source was
added attached its Post to the SYNTHETIC anchor, not the real
Source. (Fixed in same commit by preferring non-sidecar URLs.)
This migration is the data half of the cleanup: for every (artist,
platform) with both a synthetic AND a real Source, repoint the
synthetic's children (Posts, ImageProvenance, DownloadEvents) onto the
real Source and delete the synthetic. Reuses the same epid/provenance
collision dance from alembic 0022 because the same uniqueness
constraints fire row-by-row during bulk UPDATEs.
Lone synthetic anchors — those where no real Source for the same
(artist, platform) exists (e.g., filesystem-imported artist with no
subscription added) — are LEFT INTACT. They anchor real imported
content; deleting them would CASCADE-delete the Posts the operator
imported. The SourceService.list filter hides them from the UI; the
operator can delete them by hand if they want the underlying imports
gone.
"""
from typing import Sequence, Union
from alembic import op
from sqlalchemy import text
revision: str = "0028"
down_revision: Union[str, None] = "0027"
branch_labels: Union[str, Sequence[str], None] = None
depends_on: Union[str, Sequence[str], None] = None
def upgrade() -> None:
conn = op.get_bind()
# Find (artist_id, platform) groups where BOTH a sidecar synthetic
# and at least one real Source exist.
groups = conn.execute(text("""
SELECT artist_id, platform
FROM source
GROUP BY artist_id, platform
HAVING bool_or(url LIKE 'sidecar:%')
AND bool_or(url NOT LIKE 'sidecar:%')
""")).fetchall()
for artist_id, platform in groups:
rows = conn.execute(
text("""
SELECT id, url FROM source
WHERE artist_id = :a AND platform = :p
ORDER BY id ASC
"""),
{"a": artist_id, "p": platform},
).fetchall()
synthetic_ids = [sid for sid, url in rows if url.startswith("sidecar:")]
real_rows = [(sid, url) for sid, url in rows if not url.startswith("sidecar:")]
if not synthetic_ids or not real_rows:
continue # belt+suspenders; the GROUP BY already filtered
# Canonical real: lowest-id non-sidecar Source.
canonical_id = real_rows[0][0]
# STEP A: PRE-merge Post collisions on (canonical, external_post_id).
# Mirror alembic 0022's pre-merge logic — when synth has Post X
# epid=N and real has Post Y epid=N, the bulk UPDATE below would
# trip uq_post_source_external_id row-by-row. Group all Posts
# under (canonical + synthetics) by epid; for any group >1,
# pick a keep (prefer one already under canonical, else lowest
# id) and merge the rest into it.
all_posts = conn.execute(
text("""
SELECT external_post_id, id, source_id
FROM post
WHERE source_id = :canonical OR source_id = ANY(:synths)
ORDER BY external_post_id, id
"""),
{"canonical": canonical_id, "synths": synthetic_ids},
).fetchall()
by_epid: dict = {}
for epid, post_id, src_id in all_posts:
by_epid.setdefault(epid, []).append((post_id, src_id))
for _epid, posts in by_epid.items():
if len(posts) <= 1:
continue
canonical_side = [p for p in posts if p[1] == canonical_id]
keep_id = canonical_side[0][0] if canonical_side else posts[0][0]
drop_ids = [p[0] for p in posts if p[0] != keep_id]
for drop_id in drop_ids:
# Pre-delete image_provenance rows under drop_ whose
# image_record_id already has provenance under keep —
# avoids tripping uq_image_provenance_image_post (0021)
# row-by-row during the repoint UPDATE.
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("DELETE FROM post WHERE id = :drop_"),
{"drop_": drop_id},
)
# STEP B: Bulk reparent the remaining Posts off the synthetics.
conn.execute(
text("""
UPDATE post SET source_id = :canonical
WHERE source_id = ANY(:synths)
"""),
{"canonical": canonical_id, "synths": synthetic_ids},
)
# STEP C: Reparent ImageProvenance.source_id (denormalized FK;
# no UNIQUE on source_id, safe bulk).
conn.execute(
text("""
UPDATE image_provenance SET source_id = :canonical
WHERE source_id = ANY(:synths)
"""),
{"canonical": canonical_id, "synths": synthetic_ids},
)
# STEP D: Reparent any DownloadEvent.source_id. Synthetics are
# enabled=false so the scheduler never created events for them;
# this is belt+suspenders for any rows planted by manual force
# or older code paths.
conn.execute(
text("""
UPDATE download_event SET source_id = :canonical
WHERE source_id = ANY(:synths)
"""),
{"canonical": canonical_id, "synths": synthetic_ids},
)
# STEP E: Drop the now-empty synthetics.
conn.execute(
text("DELETE FROM source WHERE id = ANY(:synths)"),
{"synths": synthetic_ids},
)
def downgrade() -> None:
# Lossy migration — synthetic Sources deleted, Posts repointed and
# potentially merged. No safe downgrade.
pass
+21 -8
View File
@@ -42,13 +42,16 @@ class ErrorType(StrEnum):
UNKNOWN_ERROR = "unknown_error" UNKNOWN_ERROR = "unknown_error"
# Pinned to download_source's Celery soft_time_limit (900s, see # 30 seconds shy of download_source's Celery soft_time_limit (900s, see
# tasks/download.py:32). Anything larger and Celery kills the task before # tasks/download.py:32). subprocess.run MUST raise TimeoutExpired before
# subprocess.run can raise TimeoutExpired — leaving the DownloadEvent # Celery raises SoftTimeLimitExceeded — otherwise Celery wins the race,
# stranded for the recovery sweep instead of capturing a clean timeout # SIGKILLs the worker, in-memory stdout/stderr is lost, and the
# error. Per-source bumps live in source.config_overrides for legitimately # DownloadEvent ends up empty-logged with "stranded by recovery sweep"
# long syncs. Operator-confirmed 2026-05-30 (~40-min hang investigation). # 18 minutes later (operator-flagged 2026-05-31, Knuxy event #38275).
_DEFAULT_GDL_TIMEOUT_SECONDS = 900 # The 30s buffer absorbs scheduler jitter / GC pauses without making
# legitimately-long-running syncs timeout-friendlier. Per-source bumps
# still live in source.config_overrides for legitimately long syncs.
_DEFAULT_GDL_TIMEOUT_SECONDS = 870
@dataclass @dataclass
@@ -369,7 +372,17 @@ class GalleryDLService:
if return_code in (1, 4) and (skip_line_count > 0 or has_skip_text) and not has_actual_error: if return_code in (1, 4) and (skip_line_count > 0 or has_skip_text) and not has_actual_error:
return ErrorType.NO_NEW_CONTENT, "No new content to download" return ErrorType.NO_NEW_CONTENT, "No new content to download"
if return_code in (1, 4) and not has_actual_error: # Tier-gated classification used to require `return_code in (1, 4)`,
# which silently fell through to UNKNOWN_ERROR when gallery-dl
# returned a different exit code for mixed-failure runs (e.g.
# paywall warnings + a missing yt-dlp dep flipping the exit bits).
# The artist then surfaced as "needs attention" purely because a
# paywall blocked posts the operator wasn't paying to see —
# operator-flagged 2026-05-31. Now: if no source-level error
# category fired AND tier-gated warnings are present, classify
# as TIER_LIMITED regardless of return code. Same priority order
# as before (auth/rate/access/not_found/network/http still win).
if not has_actual_error:
tier_gated_lines = [ tier_gated_lines = [
line for line in combined.split("\n") line for line in combined.split("\n")
if "][warning]" in line and "not allowed to view post" in line if "][warning]" in line and "not allowed to view post" in line
+34 -14
View File
@@ -261,24 +261,44 @@ class Importer:
def _source_for_sidecar( def _source_for_sidecar(
self, *, artist_id: int, platform: str, artist_slug: str, self, *, artist_id: int, platform: str, artist_slug: str,
) -> Source: ) -> Source:
"""Filesystem-import sidecar Source resolver. """Sidecar-import Source resolver. Used by both filesystem imports
and gallery-dl downloads (both write sidecar JSON, both flow through
_apply_sidecar / _capture_attachment).
Source represents a subscription feed (one per artist+platform — the Source represents a subscription feed (one per artist+platform — the
gallery-dl URL polled by the FC-3 downloader). The filesystem importer URL polled by the FC-3 downloader). The filesystem importer used to
used to call _find_or_create_source(url=sd.post_url), which created call _find_or_create_source(url=sd.post_url), creating one Source
one Source row per post URL — 100s of junk Sources per artist, all row per post URL — 100s of junk Sources per artist, all with
with enabled=True, polluting the artist detail page and tricking the enabled=True, polluting the artist detail page and tricking the
subscription checker into trying to poll patreon post URLs as feeds. subscription checker into trying to poll patreon post URLs as feeds.
Operator-flagged 2026-05-26. Operator-flagged 2026-05-26; consolidated via alembic 0022.
New behaviour: if any Source row exists for (artist_id, platform), Resolution order: prefer a real (non-sidecar) Source over a
reuse it regardless of its URL — the artist's real subscription Source synthetic anchor. When alembic 0022 ran, it may have rewritten
(created by the downloader / extension / UI) is the canonical per-post Sources into `sidecar:<platform>:<slug>` synthetic
attachment point for filesystem-imported posts. If none exists, create anchors. If the operator later added the real subscription, both
ONE synthetic anchor with url='sidecar:<platform>:<artist_slug>' and rows now coexist. A naive `ORDER BY id ASC LIMIT 1` lookup would
enabled=False (so the subscription checker doesn't poll it). pick the older synthetic and silently attach every gallery-dl
download to the wrong Source — operator-flagged 2026-05-31 after
the Subscriptions UI surfaced the phantom anchors. Pick the real
one when one exists; fall back to the synthetic; only create a
new synthetic when nothing exists for (artist, platform).
""" """
stmt = ( real_stmt = (
select(Source)
.where(
Source.artist_id == artist_id,
Source.platform == platform,
~Source.url.like("sidecar:%"),
)
.order_by(Source.id.asc())
.limit(1)
)
real = self.session.execute(real_stmt).scalar_one_or_none()
if real is not None:
return real
any_stmt = (
select(Source) select(Source)
.where( .where(
Source.artist_id == artist_id, Source.artist_id == artist_id,
@@ -288,7 +308,7 @@ class Importer:
.limit(1) .limit(1)
) )
return self._get_or_create( return self._get_or_create(
stmt, any_stmt,
lambda: Source( lambda: Source(
artist_id=artist_id, artist_id=artist_id,
platform=platform, platform=platform,
+8
View File
@@ -151,10 +151,18 @@ class SourceService:
async def list( async def list(
self, artist_id: int | None = None, failing: bool = False, self, artist_id: int | None = None, failing: bool = False,
include_synthetic: bool = False,
) -> list[SourceRecord]: ) -> list[SourceRecord]:
stmt = select(Source, Artist).join(Artist, Artist.id == Source.artist_id) stmt = select(Source, Artist).join(Artist, Artist.id == Source.artist_id)
if artist_id is not None: if artist_id is not None:
stmt = stmt.where(Source.artist_id == artist_id) stmt = stmt.where(Source.artist_id == artist_id)
if not include_synthetic:
# Filesystem-import sidecar anchors (importer._source_for_sidecar)
# have url='sidecar:<platform>:<slug>' and exist only to give
# imported Posts a NOT-NULL Source FK. They aren't pollable
# feeds; the Subscriptions UI used to render them as phantom
# subscriptions. Hide by default.
stmt = stmt.where(~Source.url.like("sidecar:%"))
if failing: if failing:
# Worst-first so the rollup card surfaces the loudest failures. # Worst-first so the rollup card surfaces the loudest failures.
stmt = stmt.where(Source.consecutive_failures > 0).order_by( stmt = stmt.where(Source.consecutive_failures > 0).order_by(
+5
View File
@@ -22,6 +22,11 @@ imagehash>=4.3,<4.4
# Gallery-dl wrapper (lands in FC-3) # Gallery-dl wrapper (lands in FC-3)
gallery-dl>=1.32,<1.33 gallery-dl>=1.32,<1.33
# Video extractor backend for gallery-dl. Without it, every video post
# attachment fails with `[downloader.ytdl][error] Cannot import yt-dlp`
# → `[download][error] Failed to download NN_video.mp4`. Operator-flagged
# 2026-05-31 after Patreon video posts produced empty downloads.
yt-dlp>=2025.1
# Utilities # Utilities
python-dotenv>=1.2,<2.0 python-dotenv>=1.2,<2.0
+52
View File
@@ -164,6 +164,58 @@ async def test_validation_quarantines_truncated_files(tmp_path, monkeypatch):
assert result.error_type == ErrorType.VALIDATION_FAILED assert result.error_type == ErrorType.VALIDATION_FAILED
@pytest.mark.asyncio
async def test_download_tier_limited_classified_as_success(gdl, monkeypatch):
"""Pure tier-gated stderr (paywall warnings only, no source-level
errors). Should be TIER_LIMITED with success=True so the
DownloadEvent doesn't surface as 'needs attention' for an operator
who's just not paying the top tier. Pre-fix: this fell through to
UNKNOWN_ERROR when the return code wasn't 1 or 4."""
stderr = (
"[patreon][warning] Not allowed to view post 159421258\n"
"[patreon][warning] Not allowed to view post 159247879\n"
"[patreon][warning] Not allowed to view post 158111214\n"
)
# Use a non-(1,4) return code to prove the gate widened.
monkeypatch.setattr(
"backend.app.services.gallery_dl.subprocess.run",
lambda *a, **k: _proc(stdout="", stderr=stderr, returncode=128),
)
result = await gdl.download(
url="https://patreon.com/alice", artist_slug="alice", platform="patreon",
source_config=SourceConfig(),
)
assert result.error_type == ErrorType.TIER_LIMITED
assert result.success is True
assert "3 posts" in (result.error_message or "")
@pytest.mark.asyncio
async def test_download_tier_limited_with_per_item_video_failures(gdl, monkeypatch):
"""Real operator log shape (2026-05-31): hundreds of paywall warnings
+ a handful of per-item video errors caused by missing yt-dlp. The
video errors are classified as per-item (not source-level), so the
classifier should still pick TIER_LIMITED. Once yt-dlp is installed
the video errors stop firing; until then, the source-health doesn't
flap to 'needs attention' over paywalled content."""
stderr = (
"[patreon][warning] Not allowed to view post 159421258\n"
"[patreon][warning] Not allowed to view post 159247879\n"
"[downloader.ytdl][error] Cannot import yt-dlp or youtube-dl\n"
"[download][error] Failed to download 02_video.mp4\n"
)
monkeypatch.setattr(
"backend.app.services.gallery_dl.subprocess.run",
lambda *a, **k: _proc(stdout="", stderr=stderr, returncode=1),
)
result = await gdl.download(
url="https://patreon.com/alice", artist_slug="alice", platform="patreon",
source_config=SourceConfig(),
)
assert result.error_type == ErrorType.TIER_LIMITED
assert result.success is True
def test_compute_run_stats(gdl): def test_compute_run_stats(gdl):
stats = gdl._compute_run_stats( stats = gdl._compute_run_stats(
return_code=0, return_code=0,
+33
View File
@@ -210,3 +210,36 @@ def test_source_for_sidecar_distinct_platforms_distinct_anchors(
assert p.id != x.id assert p.id != x.id
assert p.platform == "patreon" assert p.platform == "patreon"
assert x.platform == "pixiv" assert x.platform == "pixiv"
def test_source_for_sidecar_prefers_real_over_synthetic_when_both_exist(
importer, artist_row, db_sync,
):
"""When BOTH a synthetic anchor AND a real Source exist for the same
(artist, platform), the resolver must return the REAL one. This is the
fix for the 2026-05-31 phantom-subscription bug: alembic 0022 had
rewritten an older per-post Source row into a sidecar synthetic, and
the operator later added the real subscription. The old `ORDER BY
id ASC LIMIT 1` lookup picked the older synthetic (lower id),
silently attaching every gallery-dl download to the wrong Source.
"""
synthetic = Source(
artist_id=artist_row.id, platform="patreon",
url=f"sidecar:patreon:{artist_row.slug}", enabled=False,
)
db_sync.add(synthetic)
db_sync.flush()
real = Source(
artist_id=artist_row.id, platform="patreon",
url="https://www.patreon.com/testartist", enabled=True,
)
db_sync.add(real)
db_sync.flush()
assert synthetic.id < real.id # ordering precondition
resolved = importer._source_for_sidecar(
artist_id=artist_row.id, platform="patreon",
artist_slug=artist_row.slug,
)
assert resolved.id == real.id
assert resolved.url == "https://www.patreon.com/testartist"
+37 -1
View File
@@ -1,7 +1,7 @@
import pytest import pytest
from sqlalchemy import select from sqlalchemy import select
from backend.app.models import Artist from backend.app.models import Artist, Source
from backend.app.services.source_service import ( from backend.app.services.source_service import (
KNOWN_PLATFORMS, KNOWN_PLATFORMS,
ArtistNotFoundError, ArtistNotFoundError,
@@ -133,3 +133,39 @@ async def test_update_changes_fields(db):
updated = await svc.update(rec.id, enabled=False, config_overrides={"videos": False}) updated = await svc.update(rec.id, enabled=False, config_overrides={"videos": False})
assert updated.enabled is False assert updated.enabled is False
assert updated.config_overrides == {"videos": False} assert updated.config_overrides == {"videos": False}
@pytest.mark.asyncio
async def test_list_hides_sidecar_synthetic_anchors(db):
"""Filesystem-import synthetic Sources (url='sidecar:<platform>:<slug>',
enabled=False — see importer._source_for_sidecar) used to leak into the
Subscriptions UI as phantom subscriptions because list() didn't filter
them. They aren't pollable feeds; hide by default."""
artist = await _artist(db, "Alice")
real = Source(
artist_id=artist.id, platform="patreon",
url="https://patreon.com/alice", enabled=True, config_overrides={},
)
synthetic = Source(
artist_id=artist.id, platform="patreon",
url="sidecar:patreon:alice", enabled=False, config_overrides={},
)
db.add_all([real, synthetic])
await db.commit()
svc = SourceService(db)
visible = await svc.list()
visible_urls = {s.url for s in visible}
assert "https://patreon.com/alice" in visible_urls
assert "sidecar:patreon:alice" not in visible_urls
# Same filter applies to the artist-scoped list path (the artist detail
# page hits /api/sources?artist_id=N).
artist_scoped = await svc.list(artist_id=artist.id)
assert {s.url for s in artist_scoped} == {"https://patreon.com/alice"}
# include_synthetic=True opts back in for admin tooling.
everything = await svc.list(include_synthetic=True)
assert {s.url for s in everything} >= {
"https://patreon.com/alice", "sidecar:patreon:alice",
}