Merge pull request #851 from amitvijapur/fix/youtube-failure-count

fix(youtube): log ScrapeCreators transcript rescue instead of masking it

changelog.d/851.fixed.md

@@ -0,0 +1 @@
+YouTube ScrapeCreators transcript rescue is logged instead of being masked as a hard failure.

skills/last30days/scripts/lib/youtube_yt.py

@@ -832,6 +832,17 @@ def fetch_transcript(
     if token and _should_try_sc_transcript(status):
         sc_transcript = _sc_fetch_transcript(video_id, token)
         if sc_transcript:
+            # The keyless cascade (yt-dlp / direct HTTP) already logged its
+            # failure above. Without this line that failure is the last thing
+            # printed for this video, and the batch summary in
+            # fetch_transcripts_parallel() counts it as a plain success —
+            # making a rate-limited/bot-gated run look like nothing went
+            # wrong. Log the rescue and flag it in `status` so the summary
+            # can report it explicitly instead of masking it (#831).
+            _log(f"ScrapeCreators transcript fallback rescued {video_id} "
+                 f"after the keyless fetch cascade failed")
+            if status is not None:
+                status["sc_rescued"] = True
             return sc_transcript
 
     _log(f"No transcript available for {video_id}")
@@ -892,7 +903,20 @@ def fetch_transcripts_parallel(
 
     got = sum(1 for v in results.values() if v)
     errors = sum(1 for v in results.values() if v is None)
-    _log(f"Got transcripts for {got}/{len(video_ids)} videos ({errors} failed)")
+    # `got` includes videos that only succeeded because the ScrapeCreators
+    # fallback rescued a failed keyless fetch — yt-dlp when available, or the
+    # direct HTTP path alone (see fetch_transcript()). Folding
+    # those into a bare "M failed" count previously made a fully rate-limited
+    # yt-dlp run — every fetch failing, silently saved by the fallback — read
+    # as "0 failed", with no trace of the fallback ever having fired (#831).
+    # Surface the split so the summary can't misrepresent a masked failure
+    # as a clean success.
+    sc_rescued = sum(1 for st in statuses.values() if st.get("sc_rescued"))
+    if sc_rescued:
+        _log(f"Got transcripts for {got}/{len(video_ids)} videos "
+             f"({errors} failed, {sc_rescued} rescued via ScrapeCreators fallback)")
+    else:
+        _log(f"Got transcripts for {got}/{len(video_ids)} videos ({errors} failed)")
     return results
 
 

tests/test_youtube_yt.py

@@ -884,6 +884,24 @@ class TestScTranscriptFallback(unittest.TestCase):
         direct_mock.assert_not_called()  # hard error skips the (also-blocked) direct path
         self.assertEqual(result, "scrapecreators transcript text")
 
+    def test_sc_rescue_logged_and_flagged_in_status(self):
+        """A yt-dlp hard failure rescued by ScrapeCreators must be logged and
+        flagged via status['sc_rescued'] — not just returned silently — so
+        fetch_transcripts_parallel() can report the rescue instead of letting
+        the batch summary read as a clean success (#831)."""
+        status = {}
+        logs = []
+        with mock.patch.object(youtube_yt, "is_ytdlp_installed", return_value=True), \
+             mock.patch.object(youtube_yt, "_fetch_transcript_ytdlp",
+                               side_effect=self._ytdlp_hard_fail()), \
+             mock.patch.object(youtube_yt, "_sc_fetch_transcript",
+                               return_value="rescued transcript text"), \
+             mock.patch.object(youtube_yt, "_log", side_effect=lambda m: logs.append(m)):
+            result = youtube_yt.fetch_transcript("vidR", "/tmp/x", status=status, token="key123")
+        self.assertEqual(result, "rescued transcript text")
+        self.assertTrue(status.get("sc_rescued"))
+        self.assertTrue(any("ScrapeCreators" in m and "vidR" in m for m in logs))
+
     def test_sc_not_called_when_ytdlp_succeeds(self):
         """No credit is spent when yt-dlp returns a transcript."""
         with mock.patch.object(youtube_yt, "is_ytdlp_installed", return_value=True), \
@@ -942,6 +960,46 @@ class TestScTranscriptFallback(unittest.TestCase):
             youtube_yt.fetch_transcripts_parallel(["v1", "v2"], token="tok")
         self.assertEqual(captured, {"v1": "tok", "v2": "tok"})
 
+    def test_summary_reports_sc_rescue_not_bare_success(self):
+        """Regression for #831: when every video's yt-dlp fetch fails and the
+        ScrapeCreators fallback rescues all of them, the batch summary must
+        not read as a bare "0 failed" success — it has to say the videos
+        were rescued via the fallback, so a fully rate-limited yt-dlp run
+        doesn't look like nothing went wrong."""
+        logs = []
+
+        def _rescued_fetch_transcript(video_id, temp_dir, status=None, token=None):
+            if status is not None:
+                status["sc_rescued"] = True
+            return "rescued transcript"
+
+        with mock.patch.object(youtube_yt, "fetch_transcript",
+                                side_effect=_rescued_fetch_transcript), \
+             mock.patch.object(youtube_yt, "_log", side_effect=lambda m: logs.append(m)):
+            results = youtube_yt.fetch_transcripts_parallel(["v1", "v2"], token="tok")
+
+        self.assertEqual(results, {"v1": "rescued transcript", "v2": "rescued transcript"})
+        summary = next(m for m in logs if m.startswith("Got transcripts for"))
+        self.assertIn("2/2", summary)
+        self.assertIn("0 failed", summary)
+        self.assertIn("2 rescued via ScrapeCreators fallback", summary)
+
+    def test_summary_omits_rescue_note_when_no_fallback_used(self):
+        """The plain 'N/N (M failed)' format must be unchanged when no video
+        needed the ScrapeCreators fallback — no rescue tag should appear."""
+        logs = []
+
+        def _plain_fetch_transcript(video_id, temp_dir, status=None, token=None):
+            return "a normal transcript"
+
+        with mock.patch.object(youtube_yt, "fetch_transcript",
+                                side_effect=_plain_fetch_transcript), \
+             mock.patch.object(youtube_yt, "_log", side_effect=lambda m: logs.append(m)):
+            youtube_yt.fetch_transcripts_parallel(["v1", "v2", "v3"], token="tok")
+
+        summary = next(m for m in logs if m.startswith("Got transcripts for"))
+        self.assertEqual(summary, "Got transcripts for 3/3 videos (0 failed)")
+
 
 class TestYtdlpFastFail(unittest.TestCase):
     """Fail-fast behavior when a ScrapeCreators key is present (U3)."""