feat(ingest): Recapture mode — re-grab post bodies/links + localize on-disk inline images (#830)
A plain backfill gates post-body capture on the seen-ledger, so a post whose media is already on disk AND whose post key is already seen never gets its body recaptured (operator-flagged: Industrial Lust description missing). Recovery recaptures unconditionally but re-downloads the whole source. New 'recapture' walk mode (4th beside tick/backfill/recovery): bypasses the post-record gate so EVERY post's body + external links are re-captured (detail-fetching empty bodies) WITHOUT re-downloading on-disk media; and surfaces already-present media via a separate non-deleting relink channel so the importer backfills ImageRecord.source_filehash for inline-image localization. - ingest_core: recapture mode + recapture_records gate bypass + relink collect - patreon_downloader: recapture surfaces seen-on-disk as skipped_disk(path), never refetches seen-missing media, still downloads genuinely-new - importer.relink_source_filehash: NULL-only sha256 backfill, never unlinks - download_service: mode derivation + phase-3 relink loop + lifecycle clear - source_service/api: start_recapture + backfill_recapture field + action - frontend: Recapture kebab action + 'Recapturing' badge across SourceActions/ Row/Card/SubscriptionsTab + sources store - tests across ingester/downloader/importer/source_service/api/download_service Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -324,6 +324,32 @@ async def test_backfill_endpoint_recover_arms_bypass(client, artist, db):
|
||||
assert stopped_body["backfill_bypass_seen"] is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_backfill_endpoint_recapture_arms_flag(client, artist, db):
|
||||
"""#830: action='recapture' arms a backfill that re-grabs post bodies/links +
|
||||
localizes on-disk inline images; the response exposes backfill_recapture=True
|
||||
(badge 'Recapturing') and NOT backfill_bypass_seen. Stop clears it."""
|
||||
src = Source(
|
||||
artist_id=artist.id, platform="patreon",
|
||||
url="https://patreon.com/alice-recapture", enabled=True,
|
||||
)
|
||||
db.add(src)
|
||||
await db.commit()
|
||||
sid = src.id
|
||||
|
||||
resp = await client.post(f"/api/sources/{sid}/backfill", json={"action": "recapture"})
|
||||
assert resp.status_code == 200
|
||||
body = await resp.get_json()
|
||||
assert body["backfill_state"] == "running"
|
||||
assert body["backfill_recapture"] is True
|
||||
assert body["backfill_bypass_seen"] is False
|
||||
|
||||
stopped = await client.post(f"/api/sources/{sid}/backfill", json={"action": "stop"})
|
||||
stopped_body = await stopped.get_json()
|
||||
assert stopped_body["backfill_state"] is None
|
||||
assert stopped_body["backfill_recapture"] is False
|
||||
|
||||
|
||||
# --- Plan #703 #2: pre-flight credential verify on backfill/recover arm -----
|
||||
|
||||
|
||||
|
||||
@@ -60,8 +60,10 @@ def _make_fake_dl_result(
|
||||
*, success=True, written_paths=None, quarantined_paths=None,
|
||||
files_downloaded=0, error_type=None, error_message=None,
|
||||
stdout="", stderr="", cursor=None, run_stats=None, posts_processed=0,
|
||||
relink_source_paths=None,
|
||||
):
|
||||
return SimpleNamespace(
|
||||
relink_source_paths=relink_source_paths or [],
|
||||
success=success,
|
||||
url="https://patreon.com/alice",
|
||||
artist_slug="alice",
|
||||
@@ -793,6 +795,31 @@ async def test_recovery_mode_selected_and_flag_cleared_on_complete(
|
||||
assert "_backfill_bypass_seen" not in co
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_recapture_mode_selected_and_flag_cleared_on_complete(
|
||||
db, db_sync, tmp_path, seed_artist_and_source,
|
||||
):
|
||||
"""#830: `_backfill_recapture` alongside a running backfill selects recapture
|
||||
mode. A clean rc=0 walk completes the shared lifecycle AND clears the
|
||||
recapture flag so the next tick is a plain tick again."""
|
||||
_artist, source = seed_artist_and_source
|
||||
source.config_overrides = {"_backfill_state": "running", "_backfill_recapture": True}
|
||||
source.backfill_runs_remaining = 5
|
||||
await db.commit()
|
||||
|
||||
svc, calls = _backfill_svc(db, db_sync, tmp_path, _make_fake_dl_result(
|
||||
success=True, written_paths=[], files_downloaded=0,
|
||||
))
|
||||
await svc.download_source(source.id)
|
||||
|
||||
assert calls[0]["mode"] == "recapture"
|
||||
co = (await db.execute(
|
||||
select(Source.config_overrides).where(Source.id == source.id)
|
||||
)).scalar_one()
|
||||
assert co.get("_backfill_state") == "complete"
|
||||
assert "_backfill_recapture" not in co
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_partial_error_type_maps_to_ok_status(
|
||||
db, db_sync, tmp_path, seed_artist_and_source,
|
||||
|
||||
@@ -206,6 +206,54 @@ def test_skip_disk(tmp_path):
|
||||
assert existing.read_bytes() == b"already here" # untouched
|
||||
|
||||
|
||||
def test_recapture_surfaces_seen_on_disk_without_download(tmp_path):
|
||||
"""#830 recapture: a seen item that IS on disk is surfaced as skipped_disk
|
||||
(with its path, for source_filehash backfill) — NOT tier-1 short-circuited,
|
||||
and never re-downloaded."""
|
||||
session = _FakeSession()
|
||||
dl = _downloader(tmp_path, session=session)
|
||||
post_dir = tmp_path / "artist-x" / "patreon" / "2026-05-01_1001_My Post Title"
|
||||
post_dir.mkdir(parents=True)
|
||||
existing = post_dir / "01_media1.png"
|
||||
existing.write_bytes(b"already here")
|
||||
|
||||
outcomes = dl.download_post(
|
||||
_post(), [_img("media1.png")], "artist-x",
|
||||
is_seen=lambda m: True, recapture=True,
|
||||
)
|
||||
assert outcomes[0].status == "skipped_disk"
|
||||
assert outcomes[0].path == existing
|
||||
assert session.calls == [] # no re-download
|
||||
assert existing.read_bytes() == b"already here" # untouched
|
||||
|
||||
|
||||
def test_recapture_does_not_refetch_seen_missing(tmp_path):
|
||||
"""#830 recapture: a seen item that's NOT on disk is left alone (skipped_seen,
|
||||
no download) — refetching is recovery's job, not recapture's."""
|
||||
session = _FakeSession()
|
||||
dl = _downloader(tmp_path, session=session)
|
||||
outcomes = dl.download_post(
|
||||
_post(), [_img("media1.png")], "artist-x",
|
||||
is_seen=lambda m: True, recapture=True,
|
||||
)
|
||||
assert outcomes[0].status == "skipped_seen"
|
||||
assert outcomes[0].path is None
|
||||
assert session.calls == [] # not refetched
|
||||
|
||||
|
||||
def test_recapture_still_downloads_genuinely_new(tmp_path):
|
||||
"""#830 recapture: media that is neither seen nor on disk still downloads — a
|
||||
recapture walk is thorough, it just doesn't re-pull existing files."""
|
||||
session = _FakeSession()
|
||||
dl = _downloader(tmp_path, session=session)
|
||||
outcomes = dl.download_post(
|
||||
_post(), [_img("media1.png")], "artist-x",
|
||||
is_seen=lambda m: False, recapture=True,
|
||||
)
|
||||
assert outcomes[0].status == "downloaded"
|
||||
assert outcomes[0].path is not None
|
||||
|
||||
|
||||
def test_video_routes_to_ytdlp(tmp_path, monkeypatch):
|
||||
session = _FakeSession()
|
||||
dl = _downloader(tmp_path, session=session)
|
||||
|
||||
@@ -88,16 +88,23 @@ class _FakeDownloader:
|
||||
self.post_records = 0
|
||||
|
||||
def download_post(self, post, media_items, artist_slug, *, is_seen,
|
||||
should_stop=lambda: False):
|
||||
should_stop=lambda: False, recapture=False):
|
||||
outcomes = []
|
||||
for m in media_items:
|
||||
if should_stop():
|
||||
break
|
||||
if is_seen(m):
|
||||
seen = is_seen(m)
|
||||
# Recapture mirrors the real downloader: a seen item isn't tier-1
|
||||
# short-circuited — it falls through so an on-disk file surfaces as
|
||||
# skipped_disk (for source_filehash backfill); a seen file NOT on
|
||||
# disk is left alone (skipped_seen, never refetched).
|
||||
if seen and not recapture:
|
||||
outcomes.append(MediaOutcome(media=m, status="skipped_seen", path=None, error=None))
|
||||
elif _ledger_key(m) in self.on_disk:
|
||||
p = self.tmp_path / f"{m.post_id}_{m.filename}"
|
||||
outcomes.append(MediaOutcome(media=m, status="skipped_disk", path=p, error=None))
|
||||
elif seen:
|
||||
outcomes.append(MediaOutcome(media=m, status="skipped_seen", path=None, error=None))
|
||||
elif _ledger_key(m) in self.error:
|
||||
self.download_calls += 1
|
||||
outcomes.append(MediaOutcome(media=m, status="error", path=None, error="boom"))
|
||||
@@ -581,6 +588,78 @@ async def test_backfill_recaptures_body_for_already_downloaded_post(
|
||||
assert downloader.post_records == 1
|
||||
|
||||
|
||||
def _seed_seen(sync_engine, source_id, key, post_id=None):
|
||||
factory = sessionmaker(sync_engine, expire_on_commit=False)
|
||||
with factory() as s:
|
||||
s.add(PatreonSeenMedia(source_id=source_id, filehash=key, post_id=post_id))
|
||||
s.commit()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_backfill_skips_already_captured_post_but_recapture_forces_it(
|
||||
source_id, sync_engine, tmp_path,
|
||||
):
|
||||
"""#830: a plain backfill GATES post-record capture on the seen-ledger, so a
|
||||
post whose `post:<id>` key is already seen is NOT recaptured (the operator's
|
||||
'Industrial Lust body missing' bug). RECAPTURE mode bypasses that gate and
|
||||
re-captures the body/links even though the post key is seen — without
|
||||
re-downloading the on-disk media."""
|
||||
m1 = _media("p1", 1)
|
||||
# Pre-seed BOTH the media key and the synthetic post key as already seen.
|
||||
_seed_seen(sync_engine, source_id, _ledger_key(m1), post_id="p1")
|
||||
_seed_seen(sync_engine, source_id, "post:p1", post_id="p1")
|
||||
|
||||
# 1) Plain backfill: post key is seen → gate skips body recapture.
|
||||
client = _FakeClient([(None, [("p1", [m1])])])
|
||||
downloader = _FakeDownloader(tmp_path, on_disk={_ledger_key(m1)})
|
||||
ing = _ingester(sync_engine, tmp_path, client, downloader)
|
||||
res_backfill = ing.run(
|
||||
source_id=source_id, campaign_id="c1", artist_slug="ingest",
|
||||
url="https://patreon.com/ingest", mode="backfill",
|
||||
)
|
||||
assert res_backfill.post_record_paths == [] # gated → not recaptured
|
||||
assert downloader.post_records == 0
|
||||
|
||||
# 2) Recapture: gate bypassed → body recaptured; media NOT re-downloaded;
|
||||
# the on-disk file is surfaced for source_filehash relink.
|
||||
client2 = _FakeClient([(None, [("p1", [m1])])])
|
||||
downloader2 = _FakeDownloader(tmp_path, on_disk={_ledger_key(m1)})
|
||||
ing2 = _ingester(sync_engine, tmp_path, client2, downloader2)
|
||||
res_recap = ing2.run(
|
||||
source_id=source_id, campaign_id="c1", artist_slug="ingest",
|
||||
url="https://patreon.com/ingest", mode="recapture",
|
||||
)
|
||||
assert len(res_recap.post_record_paths) == 1 # gate bypassed → recaptured
|
||||
assert downloader2.post_records == 1
|
||||
assert res_recap.files_downloaded == 0 # no media re-download
|
||||
assert downloader2.download_calls == 0
|
||||
# The on-disk media is surfaced as a (path, CDN url) relink pair.
|
||||
assert len(res_recap.relink_source_paths) == 1
|
||||
rel_path, rel_url = res_recap.relink_source_paths[0]
|
||||
assert rel_url == m1.url
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_recapture_does_not_refetch_seen_media_missing_from_disk(
|
||||
source_id, sync_engine, tmp_path,
|
||||
):
|
||||
"""Recapture must NOT re-download a seen item that's absent from disk (that's
|
||||
recovery's job) — it returns skipped_seen and contributes no relink pair."""
|
||||
m1 = _media("p1", 1)
|
||||
_seed_seen(sync_engine, source_id, _ledger_key(m1), post_id="p1")
|
||||
client = _FakeClient([(None, [("p1", [m1])])])
|
||||
# NOT in on_disk → seen + missing.
|
||||
downloader = _FakeDownloader(tmp_path)
|
||||
ing = _ingester(sync_engine, tmp_path, client, downloader)
|
||||
result = ing.run(
|
||||
source_id=source_id, campaign_id="c1", artist_slug="ingest",
|
||||
url="https://patreon.com/ingest", mode="recapture",
|
||||
)
|
||||
assert result.files_downloaded == 0
|
||||
assert downloader.download_calls == 0
|
||||
assert result.relink_source_paths == []
|
||||
|
||||
|
||||
# --- dead-letter ledger (plan #705 #7) ------------------------------------
|
||||
|
||||
|
||||
|
||||
@@ -120,6 +120,44 @@ def test_sidecar_source_url_persists_filehash(importer, import_layout):
|
||||
assert rec.source_filehash == "0123456789abcdef0123456789abcdef"
|
||||
|
||||
|
||||
def test_relink_source_filehash_backfills_existing(importer, import_layout):
|
||||
"""#830 recapture (Part B): relink_source_filehash backfills source_url +
|
||||
source_filehash on an existing on-disk image's ImageRecord (matched by
|
||||
sha256) WITHOUT re-importing or unlinking the file. NULL-only — never
|
||||
clobbers a filehash already set."""
|
||||
import_root, _ = import_layout
|
||||
m = import_root / "Alice" / "img.jpg"
|
||||
_split(m, "v")
|
||||
r = importer.import_one(m)
|
||||
assert r.status == "imported"
|
||||
rec = importer.session.get(ImageRecord, r.image_id)
|
||||
assert rec.source_filehash is None
|
||||
|
||||
url = "https://cdn.test/p/0123456789ABCDEF0123456789abcdef/img.jpg"
|
||||
assert importer.relink_source_filehash(m, url) is True
|
||||
importer.session.refresh(rec)
|
||||
assert rec.source_url == url
|
||||
assert rec.source_filehash == "0123456789abcdef0123456789abcdef"
|
||||
assert m.exists() # never unlinked
|
||||
|
||||
# NULL-only: a second relink with a different url is a no-op.
|
||||
other = "https://cdn.test/p/ffffffffffffffffffffffffffffffff/x.jpg"
|
||||
assert importer.relink_source_filehash(m, other) is False
|
||||
importer.session.refresh(rec)
|
||||
assert rec.source_filehash == "0123456789abcdef0123456789abcdef"
|
||||
|
||||
|
||||
def test_relink_source_filehash_no_match_is_noop(importer, import_layout):
|
||||
"""A path whose bytes match no ImageRecord (never imported) is a clean no-op,
|
||||
not an error."""
|
||||
import_root, _ = import_layout
|
||||
m = import_root / "Alice" / "ghost.jpg"
|
||||
_split(m, "h")
|
||||
assert importer.relink_source_filehash(
|
||||
m, "https://cdn.test/p/0123456789abcdef0123456789abcdef/ghost.jpg"
|
||||
) is False
|
||||
|
||||
|
||||
def test_reimport_same_post_idempotent(importer, import_layout):
|
||||
import_root, _ = import_layout
|
||||
# Threshold 0: only an exact phash match collapses. Orthogonal splits
|
||||
|
||||
@@ -267,6 +267,47 @@ async def test_start_recovery_arms_bypass_flag(db):
|
||||
assert "_backfill_bypass_seen" not in (co2 or {})
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_start_recapture_arms_recapture_flag(db):
|
||||
"""#830: start_recapture arms the backfill state machine PLUS the
|
||||
_backfill_recapture flag, surfaced on the record so the badge can label it
|
||||
'Recapturing'. Mutually exclusive with recovery (clears bypass_seen); stop
|
||||
clears it."""
|
||||
from backend.app.services.source_service import BACKFILL_MAX_CHUNKS
|
||||
|
||||
artist = await _artist(db, "Alice")
|
||||
svc = SourceService(db)
|
||||
rec = await svc.create(
|
||||
artist_id=artist.id, platform="patreon",
|
||||
url="https://patreon.com/alice",
|
||||
)
|
||||
# Pre-arm recovery, then recapture must clear bypass_seen (mutual exclusion).
|
||||
await svc.start_recovery(rec.id)
|
||||
|
||||
updated = await svc.start_recapture(rec.id)
|
||||
assert updated.backfill_state == "running"
|
||||
assert updated.backfill_recapture is True
|
||||
assert updated.backfill_bypass_seen is False
|
||||
assert updated.backfill_runs_remaining == BACKFILL_MAX_CHUNKS
|
||||
|
||||
co = (await db.execute(
|
||||
select(Source.config_overrides).where(Source.id == rec.id)
|
||||
)).scalar_one()
|
||||
assert co.get("_backfill_recapture") is True
|
||||
assert "_backfill_bypass_seen" not in co
|
||||
|
||||
# to_dict carries the new field for the API/UI.
|
||||
assert updated.to_dict()["backfill_recapture"] is True
|
||||
|
||||
# Stop clears the recapture flag too.
|
||||
stopped = await svc.stop_backfill(rec.id)
|
||||
assert stopped.backfill_recapture is False
|
||||
co2 = (await db.execute(
|
||||
select(Source.config_overrides).where(Source.id == rec.id)
|
||||
)).scalar_one()
|
||||
assert "_backfill_recapture" not in (co2 or {})
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_start_backfill_raises_when_source_missing(db):
|
||||
svc = SourceService(db)
|
||||
|
||||
Reference in New Issue
Block a user