fix(pixiv): capture ugoira frame timings in the post record (ordering bug)

The core writes the post record BEFORE extract_media, but the ugoira frame
delays were only memoized DURING extract_media — so write_post_record never saw
them and ugoira_frames was always empty in the record. Extract a memoized
_ugoira_meta (frames + zip url share ONE /v1/ugoira/metadata call regardless of
order) and inject client.fetch_ugoira_frames into the downloader (mirrors
Patreon's content_fetcher) so write_post_record populates the frames itself.
Zero extra API calls — the fetch is shared/memoized with extract_media. A
recapture now backfills the timings onto existing ugoira posts.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CDgx8bQS5YrGRK76v8HUnM

backend/app/services/pixiv_client.py

@@ -435,24 +435,59 @@ class PixivClient:
             )
         ]
 
-    def _ugoira_media(self, work: dict, pid: str) -> list[MediaItem]:
+    def _ugoira_meta(self, work: dict, pid: str) -> dict | None:
-        """The ugoira frame zip (gallery-dl's default non-original mode):
+        """Fetch + memoize the ugoira metadata (frames + zip urls) for a work.
-        `/v1/ugoira/metadata` → zip_urls.medium with the 600x600→1920x1080
+
-        swap. Frame delays are memoized onto the work so the post record
+        Idempotent and cached on the work dict, so the post record and the
-        captures them (a future ugoira→video conversion needs the timings —
+        media extraction share ONE `/v1/ugoira/metadata` call regardless of
-        the zip alone has none). A metadata failure downgrades to 'no media'
+        which runs first (the core writes the post record BEFORE it extracts
-        with a warning (matching gallery-dl) instead of failing the walk —
+        media). Returns None — and caches the miss — on a non-auth failure
-        except auth failures, which stay loud."""
+        (matching gallery-dl's downgrade); auth failures stay loud."""
+        if "_ugoira_meta" in work:
+            return work["_ugoira_meta"]
         try:
             body = self._call("/v1/ugoira/metadata", {"illust_id": pid})
             meta = body["ugoira_metadata"]
-            zip_url = meta["zip_urls"]["medium"]
         except PixivAuthError:
             raise
         except (PixivAPIError, KeyError, TypeError) as exc:
             log.warning("Pixiv ugoira metadata failed for %s: %s", pid, exc)
-            return []
+            work["_ugoira_meta"] = None
+            return None
+        work["_ugoira_meta"] = meta
+        # Frame delays: a future ugoira→video conversion needs the timings (the
+        # zip alone has none), so the post record captures them.
         work["_ugoira_frames"] = meta.get("frames") or []
+        return meta
+
+    def fetch_ugoira_frames(self, post: dict) -> None:
+        """Populate `post['_work']['_ugoira_frames']` for an ugoira post (no-op
+        otherwise). The core writes the post record BEFORE extract_media, so
+        without this the frame timings would never reach the record; this
+        fetches (and memoizes, so extract_media reuses it) the metadata. Injected
+        into the downloader by the ingester, mirroring Patreon's content_fetcher.
+        Auth errors propagate; other failures leave frames unset."""
+        work = post.get("_work") or {}
+        if work.get("type") != "ugoira":
+            return
+        pid = str(post.get("id") or "")
+        if pid:
+            self._ugoira_meta(work, pid)
+
+    def _ugoira_media(self, work: dict, pid: str) -> list[MediaItem]:
+        """The ugoira frame zip (gallery-dl's default non-original mode):
+        `/v1/ugoira/metadata` → zip_urls.medium with the 600x600→1920x1080
+        swap. A metadata failure downgrades to 'no media' with a warning
+        (matching gallery-dl) instead of failing the walk — except auth
+        failures, which stay loud."""
+        meta = self._ugoira_meta(work, pid)
+        if meta is None:
+            return []
+        try:
+            zip_url = meta["zip_urls"]["medium"]
+        except (KeyError, TypeError) as exc:
+            log.warning("Pixiv ugoira zip url missing for %s: %s", pid, exc)
+            return []
         url = zip_url.replace("_ugoira600x600", "_ugoira1920x1080", 1)
         return [
             MediaItem(

backend/app/services/pixiv_downloader.py

@@ -102,11 +102,16 @@ class PixivDownloader(BaseNativeDownloader):
         validate: bool = True,
         rate_limit: float = 0.0,
         session: requests.Session | None = None,
+        ugoira_frames_fetcher: Callable[[dict], None] | None = None,
     ):
         super().__init__(
             images_root, cookies_path, platform="pixiv",
             validate=validate, rate_limit=rate_limit, session=session,
         )
+        # Injected by the ingester (client.fetch_ugoira_frames) so write_post_record
+        # can populate frame timings — which extract_media memoizes, but the core
+        # writes the post record FIRST. Mirrors Patreon's content_fetcher.
+        self._ugoira_frames_fetcher = ugoira_frames_fetcher
         if session is None:
             # i.pximg.net 403s any GET without the app Referer; mirror the
             # client's full app-header profile (gallery-dl serves media off
@@ -248,7 +253,16 @@ class PixivDownloader(BaseNativeDownloader):
                 "account": user.get("account"),
                 "name": user.get("name"),
             }
-        # Memoized by extract_media's ugoira branch; the zip has no timings.
+        # Ugoira frame timings. extract_media memoizes these, but the core writes
+        # the post record BEFORE extracting media, so fetch them here (shared +
+        # idempotent via the client's memoization) so the record actually keeps
+        # them — the zip carries no timings.
+        if (
+            work.get("type") == "ugoira"
+            and not work.get("_ugoira_frames")
+            and self._ugoira_frames_fetcher is not None
+        ):
+            self._ugoira_frames_fetcher(post)
         frames = work.get("_ugoira_frames")
         if frames:
             data["ugoira_frames"] = frames

backend/app/services/pixiv_ingester.py

@@ -81,6 +81,10 @@ class PixivIngester(Ingester):
             if downloader is not None
             else PixivDownloader(
                 self.images_root, cookies_path, validate=validate, rate_limit=rate_limit,
+                # write_post_record runs before extract_media in the core, so it
+                # fetches ugoira frame timings via the SAME client (shared,
+                # memoized) — else the record's ugoira_frames stays empty.
+                ugoira_frames_fetcher=resolved_client.fetch_ugoira_frames,
             )
         )
         super().__init__(

tests/test_pixiv_client.py

@@ -200,6 +200,38 @@ def test_extract_media_ugoira_zip_swap(client, page1, monkeypatch):
     assert post["_work"]["_ugoira_frames"] == frames
 
 
+def test_fetch_ugoira_frames_memoizes_and_shares_one_call(client, page1, monkeypatch):
+    # fetch_ugoira_frames (called by write_post_record, which the core runs
+    # BEFORE extract_media) populates frames; extract_media then REUSES the
+    # memoized metadata — exactly one /v1/ugoira/metadata call total.
+    frames = [{"file": "000000.jpg", "delay": 90}]
+    calls = []
+
+    def fake_call(endpoint, params):
+        calls.append(endpoint)
+        return {"ugoira_metadata": {
+            "zip_urls": {"medium": (
+                "https://i.pximg.net/img-zip-ugoira/x/333_ugoira600x600.zip"
+            )},
+            "frames": frames,
+        }}
+
+    monkeypatch.setattr(client, "_call", fake_call)
+    post = _post_for(client, page1, 333)
+    client.fetch_ugoira_frames(post)
+    assert post["_work"]["_ugoira_frames"] == frames
+    items = client.extract_media(post, {})          # reuses memoized meta
+    assert items[0].media_id == "ugoira"
+    assert calls == ["/v1/ugoira/metadata"]          # ONE fetch, not two
+
+
+def test_fetch_ugoira_frames_noop_for_non_ugoira(client, page1, monkeypatch):
+    monkeypatch.setattr(client, "_call", lambda e, p: pytest.fail("should not fetch"))
+    post = _post_for(client, page1, 222)             # a single-page illust
+    client.fetch_ugoira_frames(post)
+    assert "_ugoira_frames" not in post["_work"]
+
+
 def test_extract_media_ugoira_metadata_failure_downgrades(
     client, page1, monkeypatch
 ):

tests/test_pixiv_downloader.py

@@ -268,6 +268,40 @@ def test_write_post_record_includes_ugoira_frames(tmp_path):
     assert data["ugoira_frames"] == frames
 
 
+def test_write_post_record_fetches_ugoira_frames_when_absent(tmp_path):
+    # The core writes the post record BEFORE extract_media, so frames aren't
+    # memoized yet — the injected fetcher must populate them so the record keeps
+    # the timings (regression: they were silently always empty).
+    post = _post_only(333)
+    assert "_ugoira_frames" not in post["_work"]
+    frames = [{"file": "000000.jpg", "delay": 120}]
+    calls = []
+
+    def fetcher(p):
+        calls.append(p)
+        p["_work"]["_ugoira_frames"] = frames
+
+    dl = PixivDownloader(
+        tmp_path, validate=False, session=_FakeSession(),
+        ugoira_frames_fetcher=fetcher,
+    )
+    outcome = dl.write_post_record(post, "artist-a")
+    data = json.loads(outcome.path.read_text())
+    assert data["ugoira_frames"] == frames
+    assert len(calls) == 1
+
+
+def test_write_post_record_non_ugoira_does_not_call_fetcher(tmp_path):
+    post = _post_only(111)                       # a plain multi-page illust
+    calls = []
+    dl = PixivDownloader(
+        tmp_path, validate=False, session=_FakeSession(),
+        ugoira_frames_fetcher=lambda p: calls.append(p),
+    )
+    dl.write_post_record(post, "artist-a")
+    assert calls == []
+
+
 def test_write_post_record_without_id(tmp_path):
     dl = _downloader(tmp_path)
     outcome = dl.write_post_record({"id": None, "attributes": {}}, "artist-a")