From 7299e82c22e595430343f2e534e63cf1cd0f0582 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 1 Nov 2026 03:52:16 +0100 Subject: [PATCH] fix(downloads): a Facebook download titled only "Facebook" takes the post's text Since 2026-10-28 a signed-in read of a reel gives the placeholder title and no description, so the adapter had nothing to fall back on and every such download (and its name) read "Facebook". A placeholder with nothing behind it now gets one signed-out, metadata-only read of the page. The backfill retitles the downloads already made, and every name that only repeated the placeholder; a name its owner gave stays. --- backend/app/downloads/adapters.py | 13 +- backend/app/downloads/backfill.py | 50 ++++++- backend/app/downloads/pipeline.py | 6 +- backend/tests/test_facebook_titles.py | 180 ++++++++++++++++++++++++++ backend/tests/test_failure_lines.py | 4 +- backend/tests/test_upload.py | 1 + 6 files changed, 245 insertions(+), 9 deletions(-) create mode 100644 backend/tests/test_facebook_titles.py diff --git a/backend/app/downloads/adapters.py b/backend/app/downloads/adapters.py index 7878bffe..ee693f6c 100644 --- a/backend/app/downloads/adapters.py +++ b/backend/app/downloads/adapters.py @@ -66,6 +66,11 @@ class SourceAdapter: """The download's raw title, before `titles.normalize_title` cleans it. Generic: yt-dlp's.""" return info.get("title") + def is_placeholder_title(self, title: str | None) -> bool: + """Whether `title` is a stand-in the source gives in place of the real one (worth one more, + signed-out read of the page). Generic: never.""" + return False + def uploader_url(self, info: dict) -> str | None: """The channel/uploader page URL. Generic: whatever yt-dlp exposes directly (YouTube gives `channel_url`), else None (reddit / a raw media URL have no derivable channel page).""" @@ -201,11 +206,17 @@ class FacebookAdapter(SourceAdapter): # Measured (2026-09-14): a SIGNED-IN read gives the placeholder "Video" (plain and as Chrome), and an # 18+ reel only readable as Chrome gives "Facebook"; the post's text is still in `description`. A # signed-out read titles the page with that same text behind the engagement counts, cut short. + # Measured (2026-11-01, yt-dlp 2026.08.19): since 2026-10-28 every signed-in reel read gives + # "Facebook" and NO description, while a signed-out read of the same reel still carries the text — + # so a placeholder left with nothing behind it gets one signed-out read (`backfill.public_title_info`). _PLACEHOLDER_TITLES = frozenset({"video", "facebook"}) + def is_placeholder_title(self, title: str | None) -> bool: + return (title or "").strip().lower() in self._PLACEHOLDER_TITLES + def title(self, info: dict) -> str | None: raw = info.get("title") - if (raw or "").strip().lower() not in self._PLACEHOLDER_TITLES: + if not self.is_placeholder_title(raw): return raw line = next( (ln.strip() for ln in (info.get("description") or "").splitlines() if ln.strip()), "" diff --git a/backend/app/downloads/backfill.py b/backend/app/downloads/backfill.py index ae400902..8a34d485 100644 --- a/backend/app/downloads/backfill.py +++ b/backend/app/downloads/backfill.py @@ -5,7 +5,9 @@ auto-detected channel becomes a clickable link) and `poster_path` (an ffmpeg fra thumbnail-less sources, so the card / watch page / link preview still shows an image). Best-effort and safe to re-run: it only touches assets still missing the field, and only re-fetches YouTube / Facebook sources (a direct media URL like a reddit HLS manifest carries no channel and its signed -URL may have expired — those just get a locally-generated poster instead). +URL may have expired — those just get a locally-generated poster instead). A Facebook download +titled with the source's placeholder ("Facebook", "Video") is retitled from a signed-out read of its +page, and so is each download name that only repeated the placeholder. Run once per environment: docker exec siftlode-worker-1 python -c "from app.downloads import backfill; print(backfill.run())" @@ -15,6 +17,7 @@ from urllib.parse import urlsplit import yt_dlp from sqlalchemy import select +from sqlalchemy.orm import Session from app import logs from app.config import settings @@ -22,12 +25,14 @@ from app.db import SessionLocal from app.downloads import adapters from app.downloads import edit as editmod from app.downloads import formats, service -from app.downloads.models import MediaAsset +from app.downloads.models import DownloadJob, MediaAsset +from app.models import DISPLAY_TEXT_MAX +from app.titles import normalize_title log = logs.get_logger(__name__) -def _extract_meta(url: str, adapter: adapters.SourceAdapter, asset_id: int) -> dict | None: +def extract_meta(url: str, adapter: adapters.SourceAdapter, asset_id: int) -> dict | None: """Metadata-only yt-dlp extraction, using the source adapter's client sets + options (YouTube needs the POT/player-client strategy even just to extract).""" with formats.hiding(url): @@ -62,8 +67,37 @@ def _extract_meta_scoped(url: str, adapter: adapters.SourceAdapter, asset_id: in return None +def public_title_info(info: dict, url: str, adapter: adapters.SourceAdapter, asset_id: int) -> dict: + """`info` to title a download from: itself, unless its title is a source's placeholder with + nothing behind it (a signed-in Facebook read: "Facebook", no description). Then one signed-out, + metadata-only read of the page lends its text — the description, else its title — as the + description the adapter takes a placeholder's title from. Nothing better there: `info` as it was.""" + if not adapter.is_placeholder_title(adapter.title(info)): + return info + public = extract_meta(url, adapter, asset_id) + text = public and (public.get("description") or public.get("title")) + return {**info, "description": text} if text else info + + +def _retitle(db: Session, a: MediaAsset, adapter: adapters.SourceAdapter, url: str) -> bool: + """Swap a placeholder title kept from an earlier download for the page's text, and the download + names that only repeated it. A name its owner gave (anything else) stays.""" + if not adapter.is_placeholder_title(a.title): + return False + info = public_title_info({"title": a.title, "description": None}, url, adapter, a.id) + named = normalize_title(adapter.title(info)) + if not named or adapter.is_placeholder_title(named): + return False + jobs = db.execute(select(DownloadJob).where(DownloadJob.asset_id == a.id)).scalars() + for job in jobs: + if job.display_name == a.title: + job.display_name = named[:DISPLAY_TEXT_MAX] + a.title = named + return True + + def run() -> dict: - filled_url = filled_poster = 0 + filled_url = filled_poster = retitled = 0 with SessionLocal() as db: assets = ( db.execute( @@ -78,7 +112,7 @@ def run() -> dict: adapter = adapters.resolve_adapter(a.source_kind, a.source_ref) if a.uploader_url is None and adapter.wants_metadata_backfill(): url = a.source_webpage_url or service.source_url(a.source_kind, a.source_ref) - info = _extract_meta(url, adapter, a.id) + info = extract_meta(url, adapter, a.id) uu = adapter.uploader_url(info) if info else None if uu: a.uploader_url = uu @@ -86,6 +120,11 @@ def run() -> dict: log.info( "asset %s uploader_url filled", a.id ) # the URL names what they downloaded + if adapter.wants_metadata_backfill(): + url = a.source_webpage_url or service.source_url(a.source_kind, a.source_ref) + if _retitle(db, a, adapter, url): + retitled += 1 + log.info("asset %s retitled", a.id) # the title names what they downloaded if a.poster_path is None: # Record a self-hosted poster for every download (the reliable og:image source): # ensure_poster returns the existing .jpg thumbnail sidecar, or cuts a frame. @@ -99,6 +138,7 @@ def run() -> dict: "assets": len(assets), "filled_uploader_url": filled_url, "filled_poster": filled_poster, + "retitled": retitled, } log.info("backfill done: %s", result) return result diff --git a/backend/app/downloads/pipeline.py b/backend/app/downloads/pipeline.py index 6dd29a92..215bdc44 100644 --- a/backend/app/downloads/pipeline.py +++ b/backend/app/downloads/pipeline.py @@ -16,6 +16,7 @@ from app.downloads import clip as clipmod from app.downloads import edit as editmod from app.downloads import errors as dlerrors from app.downloads import adapters, credentials, formats, mediakind, playable, probe, quota +from app.downloads import backfill from app.downloads import service, storage from app.downloads import urlguard from app.downloads.jobdb import ( @@ -585,9 +586,12 @@ def _download( # missing duration hid the trim action. Probed before the sidecars so the .nfo runtime has it # too. Same probe the upload ingest uses. vcodec, acodec, width, height, duration_s = probe.resolve_media_facts(produced, info) + # A placeholder title with nothing behind it ("Facebook" from a signed-in read) borrows the + # page's text from one signed-out read; the download itself stays the signed-in one. + titled = backfill.public_title_info(info, url, adapter, asset_id) meta = storage.MediaMeta( video_id=info.get("id") or source_ref, - title=normalize_title(adapter.title(info)) or source_ref, + title=normalize_title(adapter.title(titled)) or source_ref, uploader=info.get("uploader") or info.get("channel") or "Unknown Channel", upload_date=_parse_upload_date(info.get("upload_date")), uniqueid_type=adapter.nfo_uniqueid_type(), diff --git a/backend/tests/test_facebook_titles.py b/backend/tests/test_facebook_titles.py new file mode 100644 index 00000000..eb09ecf9 --- /dev/null +++ b/backend/tests/test_facebook_titles.py @@ -0,0 +1,180 @@ +"""A Facebook download's title when the signed-in read gives only a placeholder. + +Measured on prod (2026-11-01): every Facebook download since 2026-10-28 is titled "Facebook" — the +signed-in read (the owner's cookie) now gives that placeholder and NO description, so the adapter had +no post text to fall back on, and the download name copied the placeholder too. A signed-out read of +the same reel (yt-dlp 2026.08.19, as on prod) still carries the post's text in its description. +""" + +from app.config import settings +from app.downloads import adapters, backfill, pipeline +from app.downloads.models import DownloadJob, MediaAsset + +_URL = "https://www.facebook.com/reel/1860682468422402/" +# The measured signed-out read of that reel. +_PUBLIC = { + "title": "163K reactions · 14K shares | sorry not helping #workhumor #officelife | Work dot exe", + "description": "sorry not helping\n\n#workhumor #officelife", + "uploader": "Work dot exe", +} + + +def _fake_ydl(monkeypatch, outcome): + """yt-dlp faked at its `YoutubeDL` boundary: records each read's options, answers `outcome`.""" + seen: list[dict] = [] + + class FakeYDL: + def __init__(self, opts): + seen.append(opts) + + def __enter__(self): + return self + + def __exit__(self, *exc): + return False + + def extract_info(self, url, download): + return outcome + + monkeypatch.setattr(backfill.yt_dlp, "YoutubeDL", FakeYDL) + return seen + + +def test_a_placeholder_with_nothing_behind_it_takes_the_signed_out_pages_text(monkeypatch): + seen = _fake_ydl(monkeypatch, _PUBLIC) + fb = adapters.FacebookAdapter() + + info = backfill.public_title_info({"title": "Facebook", "description": None}, _URL, fb, 7) + + assert fb.title(info) == "sorry not helping" + # One metadata-only read, and signed out: no cookie goes with it. + assert len(seen) == 1 + assert seen[0]["skip_download"] is True + assert not seen[0].get("cookiefile") + + +def test_a_real_title_reads_nothing_more(monkeypatch): + seen = _fake_ydl(monkeypatch, _PUBLIC) + info = {"title": "Funny Dark Humor Dad Joke!", "description": None} + + assert backfill.public_title_info(info, _URL, adapters.FacebookAdapter(), 7) is info + assert seen == [] + + +def test_a_placeholder_the_signed_out_page_cannot_better_stays(monkeypatch): + _fake_ydl(monkeypatch, {"title": "Facebook", "description": None}) + fb = adapters.FacebookAdapter() + + info = backfill.public_title_info({"title": "Facebook", "description": None}, _URL, fb, 7) + + assert fb.title(info) == "Facebook" + + +def test_another_source_is_never_read_again(monkeypatch): + seen = _fake_ydl(monkeypatch, _PUBLIC) + info = {"title": "Video", "description": None} + + assert backfill.public_title_info(info, _URL, adapters.SourceAdapter(), 7) is info + assert seen == [] + + +def test_the_backfill_retitles_a_placeholder_download_and_the_names_that_repeated_it( + db, make_user, monkeypatch +): + _fake_ydl(monkeypatch, _PUBLIC) + owner, other = make_user(), make_user() + asset = MediaAsset( + source_kind="url", + source_ref=_URL, + format_sig="fb", + status="ready", + rel_path="facebook/x.mp4", + poster_path="facebook/x.jpg", + uploader_url="https://www.facebook.com/1", + title="Facebook", + ref_count=2, + ) + db.add(asset) + db.flush() + copied = DownloadJob( + user_id=owner.id, + asset_id=asset.id, + source_kind="url", + source_ref=_URL, + status="done", + queue_pos=0, + profile_snapshot={}, + display_name="Facebook", + ) + renamed = DownloadJob( + user_id=other.id, + asset_id=asset.id, + source_kind="url", + source_ref=_URL, + status="done", + queue_pos=0, + profile_snapshot={}, + display_name="Office jokes", + ) + db.add_all([copied, renamed]) + db.commit() + + assert backfill.run()["retitled"] == 1 + + db.expire_all() + assert db.get(MediaAsset, asset.id).title == "sorry not helping" + assert db.get(DownloadJob, copied.id).display_name == "sorry not helping" + assert db.get(DownloadJob, renamed.id).display_name == "Office jokes" + # Run again: nothing left to retitle, and no page read for it. + assert backfill.run()["retitled"] == 0 + + +def test_a_download_titled_facebook_is_named_from_the_signed_out_page( + db, make_user, monkeypatch, tmp_path +): + # The whole download path with yt-dlp and the media tools faked: the signed-in read gives only + # the placeholder, and the file and the download's name get the post's text. + _fake_ydl(monkeypatch, _PUBLIC) + monkeypatch.setattr(settings, "files_root", str(tmp_path)) + monkeypatch.setattr(pipeline.urlguard, "check_url", lambda url: None) + monkeypatch.setattr(pipeline, "_ensure_phone_safe", lambda _j, produced, *_a: produced) + monkeypatch.setattr( + pipeline.probe, "resolve_media_facts", lambda produced, info: ("h264", "aac", 720, 1280, 9) + ) + monkeypatch.setattr(pipeline.editmod, "thumbnail_to_jpg", lambda *a: None) + monkeypatch.setattr(pipeline.editmod, "ensure_poster", lambda *a: None) + owner = make_user() + asset = MediaAsset( + source_kind="url", source_ref=_URL, format_sig="fb", status="downloading", ref_count=1 + ) + db.add(asset) + db.flush() + job = DownloadJob( + user_id=owner.id, + asset_id=asset.id, + source_kind="url", + source_ref=_URL, + status="running", + queue_pos=0, + profile_snapshot={}, + ) + db.add(job) + db.commit() + + def signed_in_read(job_id, asset_id, url, spec, staging, adapter, **kw): + f = staging / "1860682468422402.mp4" + f.write_bytes(b"v" * 100) + return { + "id": "1860682468422402", + "title": "Facebook", + "uploader": "Work dot exe", + "requested_downloads": [{"filepath": str(f)}], + } + + monkeypatch.setattr(pipeline, "_extract_with_fallback", signed_in_read) + pipeline._download(job.id, asset.id, "url", _URL, {}, owner.id) + + db.expire_all() + assert db.get(DownloadJob, job.id).status == "done" + assert db.get(MediaAsset, asset.id).title == "sorry not helping" + assert db.get(DownloadJob, job.id).display_name == "sorry not helping" diff --git a/backend/tests/test_failure_lines.py b/backend/tests/test_failure_lines.py index 4e43e501..e5122140 100644 --- a/backend/tests/test_failure_lines.py +++ b/backend/tests/test_failure_lines.py @@ -368,7 +368,7 @@ def test_the_metadata_backfill_names_the_asset_and_not_the_download(sink, monkey ) monkeypatch.setattr(backfill.yt_dlp, "YoutubeDL", _Refusing) - assert backfill._extract_meta(url, adapters.YouTubeAdapter(), 11) is None + assert backfill.extract_meta(url, adapters.YouTubeAdapter(), 11) is None lines = [o for o in sink.objects() if "meta extract failed" in o["msg"]] assert lines and all("dQw4w9WgXcQ" not in json.dumps(o) for o in lines) @@ -393,7 +393,7 @@ def test_the_metadata_backfill_takes_out_a_generic_name_whatever_it_holds(sink, raise yt_dlp.utils.DownloadError("ERROR: [generic] Our Trip: Day 1: Unable to download") monkeypatch.setattr(backfill.yt_dlp, "YoutubeDL", _Refusing) - assert backfill._extract_meta(url, adapters.SourceAdapter(), 11) is None + assert backfill.extract_meta(url, adapters.SourceAdapter(), 11) is None lines = [o for o in sink.objects() if "meta extract failed" in o["msg"]] assert lines and all("Day 1" not in json.dumps(o) for o in lines) diff --git a/backend/tests/test_upload.py b/backend/tests/test_upload.py index 0b385594..7462d1b3 100644 --- a/backend/tests/test_upload.py +++ b/backend/tests/test_upload.py @@ -596,6 +596,7 @@ DISPLAY_WRITE_SITES = { ("downloads/service.py", "enqueue_clip"): {"display_name or asset.title[:DISPLAY_TEXT_MAX]"}, ("downloads/pipeline.py", "_download"): {"meta.title[:DISPLAY_TEXT_MAX]"}, ("downloads/pipeline.py", "_process_clip"): {"named[:DISPLAY_TEXT_MAX]"}, + ("downloads/backfill.py", "_retitle"): {"named[:DISPLAY_TEXT_MAX]"}, ("worker.py", "_finish_from_cache"): {"asset.title[:DISPLAY_TEXT_MAX]"}, ("downloads/upload.py", "ingest_upload"): {"display_text(display_name)"}, ("routes/auth.py", "callback"): {"display_text(userinfo.get('name'))"},