Fix narration cache identity and partial subtitle offsets

Address the fresh review on PR #2214 after the #2275 integration. Drew approved fixing the two reproduced bugs and keeping the specified auto/on/off verification semantics.

Cache accepted narration by normalized text plus effective engine, voice, and synthesis model. Resolve voice defaults before rendering, invalidate entries without settings, and retain requested ASR checks on cache hits. Exclude rejected clips as before.

Treat manual subtitle offsets as start-time overrides. Only assembly offsets JSON selects scenes in the cut, including when manual timing overrides are also supplied. Empty narrated cuts write an empty SRT without crashing.

Make the gated Unix example pass --verify on and document the actual local ASR modes. Auto remains permissive if ASR is unavailable; on remains strict.

Validation: observed the new cache and subtitle regressions fail before the fixes; all 23 focused narration and subtitle-text tests now pass. Synthesis, duration probing, and ASR are mocked. No media inspection or live ASR was performed; Drew retains final video acceptance.

skills/proving-it-works-with-a-movie/SKILL.md

@@ -43,7 +43,7 @@ The Unix sequence:
 # $SKILL_DIR is this skill's own directory - the "Base directory for this
 # skill" path printed when it loads. Installed as a plugin that is
 # $CLAUDE_PLUGIN_ROOT/skills/proving-it-works-with-a-movie
-"$SKILL_DIR/scripts/narrate"        scenes.yaml narration/   # voice, gated
+"$SKILL_DIR/scripts/narrate"        scenes.yaml narration/ --verify on
 "$SKILL_DIR/scripts/assemble"       scenes.yaml silent-cut.mp4
 "$SKILL_DIR/scripts/make-subtitles" narration/manifest.json movie.srt \
                                     --offsets-json segments/offsets.json

skills/proving-it-works-with-a-movie/narrating.md

@@ -19,7 +19,7 @@ word your movie is about is worse than no narration.
 
 ## Use the script
 
-`scripts/narrate scenes.yaml narration/` renders one clip per scene and
+`scripts/narrate scenes.yaml narration/ --verify on` renders one clip per scene and
 picks its engine automatically: a cloud voice when a key is there, Piper
 when there isn't. It writes `manifest.json` with the exact text and the
 *measured* duration of every clip — which is what make-subtitles and the
@@ -30,10 +30,19 @@ buys the best prosody and pays for it with ad-libs, so it is gated below.
 
 ## The gate runs even without a key
 
-`narrate` listens back to every clip it renders and compares what it hears
-against the script. With a key it can use a cloud transcriber; without one
-it uses a local ASR (faster-whisper) in its own environment. The gate is not
-something you only get when you're online.
+`narrate --verify on` transcribes every clip, including cached clips, with
+local faster-whisper in its own environment and compares the result against
+the script. It needs no API key. Missing or failed transcription is a failure;
+the first run needs network access to download dependencies and the ASR model.
+
+| Mode | Local transcription behavior |
+|---|---|
+| `--verify on` | Required for every engine. Unavailable ASR or detected drift returns nonzero and excludes the failed clip from the manifest. Use this for the gated workflow above. |
+| `--verify auto` (CLI default) | Tries ASR for Piper and `openai-chat`; reports unavailable ASR but allows the clip. Skips ASR for `openai`. Detected drift still fails. |
+| `--verify off` | Skips ASR. |
+
+The `openai-chat` engine's returned transcript is also checked when rendering,
+regardless of the ASR mode. That transcript does not prove what the WAV contains.
 
 What it measures is **missing or invented content**, not exact words, and
 that distinction is load-bearing. A small ASR mangles unusual names — ours
@@ -47,8 +56,10 @@ preamble, a clip that came out empty.
 It will not catch a single dropped word in a jargon-heavy line. For those,
 listen to one clip yourself when you pick the voice.
 
-Editing a line re-renders it: `narrate` records the text each clip was made
-from, and a clip whose script has changed is regenerated rather than reused.
+`narrate` records each clip's text, engine, voice, and synthesis model.
+Changing any of these re-renders the clip. Clips without recorded synthesis
+settings also re-render; an unchanged clip can be reused and still receives
+any requested ASR verification.
 
 ## The verbatim gate — required
 

skills/proving-it-works-with-a-movie/scripts/make-subtitles

@@ -78,11 +78,11 @@ def main():
     ap.add_argument("manifest", type=Path)
     ap.add_argument("out", type=Path)
     ap.add_argument("--offsets", nargs="*", default=[],
-                    help="SCENE=SECONDS start overrides; without these, scenes "
-                         "are assumed to run back to back in manifest order")
+                    help="SCENE=SECONDS start overrides; other scenes run "
+                         "back to back in manifest order")
     ap.add_argument("--offsets-json", type=Path, default=None,
-                    help="segments/offsets.json from assemble - the reliable "
-                         "way to time cues against the finished cut")
+                    help="segments/offsets.json from assemble - selects scenes "
+                         "in the finished cut and sets their start times")
     ap.add_argument("--max-chars", type=int, default=MAX_CHARS)
     ap.add_argument("--max-secs", type=float, default=MAX_SECS)
     args = ap.parse_args()
@@ -92,14 +92,12 @@ def main():
     if args.offsets_json:
         overrides.update({k: float(v) for k, v in
                           json.loads(args.offsets_json.read_text(encoding="utf-8-sig")).items()})
+        # Assembly offsets identify the cut's scenes; manual offsets only retime them.
+        manifest = [e for e in manifest if e["id"] in overrides]
     for spec in args.offsets:
         k, _, v = spec.partition("=")
         overrides[k] = float(v)
 
-    # a scene with no offset and no place in the cut would silently land at
-    # the wrong time; skip it rather than mistime it
-    if overrides:
-        manifest = [e for e in manifest if e["id"] in overrides]
     cues, clock = [], 0.0
     for entry in manifest:
         start = overrides.get(entry["id"], clock)
@@ -120,7 +118,8 @@ def main():
             b = a + MIN_SECS
         lines += [str(i), f"{ts(a)} --> {ts(b)}", text, ""]
     args.out.write_text("\n".join(lines), encoding="utf-8")
-    print(f"{len(cues)} cues, ends at {ts(cues[-1][1])} -> {args.out}")
+    end = cues[-1][1] if cues else 0.0
+    print(f"{len(cues)} cues, ends at {ts(end)} -> {args.out}")
     return 0
 
 

skills/proving-it-works-with-a-movie/scripts/narrate

@@ -129,7 +129,7 @@ def post(url, key, body, want_json=True):
 
 def say_openai(key, text, out_wav, voice):
     data = post("https://api.openai.com/v1/audio/speech", key,
-                {"model": OPENAI_TTS_MODEL, "voice": voice or "nova",
+                {"model": OPENAI_TTS_MODEL, "voice": voice,
                  "input": text, "response_format": "wav"}, want_json=False)
     out_wav.write_bytes(data)
     return None                       # deterministic engine: nothing to gate
@@ -139,7 +139,7 @@ def say_openai_chat(key, text, out_wav, voice):
     doc = post("https://api.openai.com/v1/chat/completions", key, {
         "model": OPENAI_CHAT_MODEL,
         "modalities": ["text", "audio"],
-        "audio": {"voice": voice or "nova", "format": "wav"},
+        "audio": {"voice": voice, "format": "wav"},
         "messages": [{"role": "user", "content":
                       "Read this narration aloud, warm and clear, verbatim, "
                       "and say nothing else:\n\n" + text}],
@@ -155,7 +155,7 @@ def say_piper(text, out_wav, voice):
     home = Path(os.environ.get("PIPER_VOICE_DIR",
                                Path.home() / ".cache" / "piper-voices"))
     home.mkdir(parents=True, exist_ok=True)
-    name = voice or PIPER_VOICE
+    name = voice
     onnx = home / f"{name}.onnx"
     if not onnx.exists():
         print(f"  downloading local voice {name} (one time)…")
@@ -194,9 +194,9 @@ def main():
     ap.add_argument("--voice", default=None)
     ap.add_argument("--force", action="store_true")
     ap.add_argument("--verify", default="auto", choices=["auto", "on", "off"],
-                    help="listen back to each clip with a local ASR and flag "
-                         "missing or invented content (default: on when the "
-                         "engine can't tell you what it said)")
+                    help="local ASR: on requires verification; auto tries it "
+                         "for piper/openai-chat but allows unavailable ASR; "
+                         "off skips ASR (default: auto)")
     ap.add_argument("--asr-model", default="base.en")
     args = ap.parse_args()
 
@@ -213,19 +213,24 @@ def main():
         die("no OPENAI_API_KEY (and `llm keys get openai` found nothing). "
             "Use --engine piper for a local voice.")
     print(f"engine: {engine}" + ("" if key or engine == "piper" else ""))
+    voice = args.voice or (PIPER_VOICE if engine == "piper" else "nova")
+    synthesis = {"engine": engine, "voice": voice, "model": {
+        "openai": OPENAI_TTS_MODEL,
+        "openai-chat": OPENAI_CHAT_MODEL,
+        "piper": voice,
+    }[engine]}
 
     # a deterministic cloud endpoint reads exactly what you send it, so the
     # ear-check is optional there; anything else gets listened to by default
     verify = args.verify == "on" or (args.verify == "auto" and engine != "openai")
 
     args.outdir.mkdir(parents=True, exist_ok=True)
-    # what the cached clips were rendered FROM: editing a line and keeping
-    # its old audio is a silent lie, and the movie will contradict itself
+    # Cache only clips rendered from the requested text and synthesis settings.
     prior = {}
     prior_path = args.outdir / "manifest.json"
     if prior_path.exists():
         try:
-            prior = {e["id"]: e.get("text", "") for e in
+            prior = {e["id"]: e for e in
                      json.loads(prior_path.read_text(encoding="utf-8-sig"))}
         except Exception:  # noqa: BLE001 - a corrupt manifest just means no cache
             prior = {}
@@ -235,20 +240,23 @@ def main():
         sid = sc["id"]
         text = " ".join((sc["narration"] or "").split())
         wav = args.outdir / f"{sid}.wav"
-        cached = wav.exists() and not args.force and prior.get(sid) == text
+        previous = prior.get(sid, {})
+        cached = (wav.exists() and not args.force
+                  and previous.get("text") == text
+                  and previous.get("synthesis") == synthesis)
         if cached:
             print(f"{sid}: cached")
         elif wav.exists() and not args.force and sid in prior:
-            print(f"{sid}: text changed since this clip was rendered - redoing")
+            print(f"{sid}: text or synthesis settings changed - redoing")
         for attempt in ((1,) if cached else (1, 2)):
             claimed = None
             if not cached:
                 if engine == "openai":
-                    claimed = say_openai(key, text, wav, args.voice)
+                    claimed = say_openai(key, text, wav, voice)
                 elif engine == "openai-chat":
-                    claimed = say_openai_chat(key, text, wav, args.voice)
+                    claimed = say_openai_chat(key, text, wav, voice)
                 else:
-                    claimed = say_piper(text, wav, args.voice)
+                    claimed = say_piper(text, wav, voice)
 
             # Preserve the engine transcript gate and the ASR drift thresholds.
             if claimed is not None:
@@ -284,7 +292,7 @@ def main():
         if sid in failures:
             continue
         manifest.append({"id": sid, "text": text, "wav": wav.name,
-                         "duration": duration(wav)})
+                         "duration": duration(wav), "synthesis": synthesis})
 
     (args.outdir / "manifest.json").write_text(json.dumps(manifest, indent=2), encoding="utf-8")
     total = sum(m["duration"] for m in manifest)

tests/proving-it-works-with-a-movie/test_narration.py

@@ -1,4 +1,6 @@
 import io
+import json
+import sys
 import tempfile
 from contextlib import redirect_stderr, redirect_stdout
 import unittest
@@ -70,7 +72,8 @@ class NarrationDriftRegression(unittest.TestCase):
             (output / "clip.wav").write_bytes(b"cached audio fixture")
             (output / "manifest.json").write_text(json.dumps([
                 {"id": "clip", "text": "Read this sentence.", "wav": "clip.wav",
-                 "duration": 1.0}
+                 "duration": 1.0, "synthesis": {"engine": "piper",
+                 "voice": module.PIPER_VOICE, "model": module.PIPER_VOICE}}
             ]), encoding="utf-8")
             scenes = work / "scenes.yaml"
             scenes.write_text(json.dumps({"scenes": [
@@ -82,6 +85,7 @@ class NarrationDriftRegression(unittest.TestCase):
             with patch.object(sys, "argv", argv), \
                  patch.object(module, "openai_key", return_value=None), \
                  patch.object(module, "duration", return_value=1.0), \
+                 patch.object(module, "say_piper", side_effect=AssertionError("expected cached clip")), \
                  patch.object(module, "transcribe_local", return_value=None), \
                  redirect_stdout(stdout), redirect_stderr(stderr):
                 self.assertNotEqual(module.main(), 0)
@@ -195,5 +199,126 @@ class TranscriptionProtocolRegression(unittest.TestCase):
                     self.assertIn("clip: required verification unavailable", stderr.getvalue())
 
 
+class NarrationCacheRegression(unittest.TestCase):
+    def setUp(self):
+        self.module = fixtures.load_script("narrate")
+        directory = tempfile.TemporaryDirectory()
+        self.addCleanup(directory.cleanup)
+        self.root = Path(directory.name)
+        self.scenes = self.root / "scenes.yaml"
+        self.text = "Read this sentence exactly."
+        self.write_scenes(self.text)
+        self.output = self.root / "voice"
+        self.renders = []
+
+        def synthesize(*args):
+            text, wav, voice = args[-3:]
+            self.renders.append((text, voice))
+            wav.write_bytes(f"render {len(self.renders)}".encode())
+            return text
+
+        for name, options in (
+            ("openai_key", {"return_value": "test-key"}),
+            ("duration", {"return_value": 1.0}),
+            ("transcribe_local", {"return_value": None}),
+            ("say_piper", {"side_effect": synthesize}),
+            ("say_openai", {"side_effect": synthesize}),
+            ("say_openai_chat", {"side_effect": synthesize}),
+        ):
+            mocked = patch.object(self.module, name, **options)
+            mocked.start()
+            self.addCleanup(mocked.stop)
+
+    def write_scenes(self, text):
+        self.scenes.write_text(json.dumps({"scenes": [
+            {"id": "clip", "narration": text}
+        ]}), encoding="utf-8")
+
+    def narrate(self, *options, verify="off", expected_exit=0):
+        argv = ["narrate", str(self.scenes), str(self.output),
+                "--verify", verify, *options]
+        stdout, stderr = io.StringIO(), io.StringIO()
+        with patch.object(sys, "argv", argv), \
+             redirect_stdout(stdout), redirect_stderr(stderr):
+            self.assertEqual(self.module.main(), expected_exit, stderr.getvalue())
+        return json.loads((self.output / "manifest.json").read_text(encoding="utf-8"))
+
+    def test_engine_and_voice_changes_rerender(self):
+        for index, options in enumerate((
+            ("--engine", "piper", "--voice", "voice-a"),
+            ("--engine", "piper", "--voice", "voice-b"),
+            ("--engine", "openai", "--voice", "voice-b"),
+            ("--engine", "openai-chat", "--voice", "voice-b"),
+        ), 1):
+            with self.subTest(options=options):
+                self.narrate(*options)
+                self.assertEqual(len(self.renders), index)
+                self.assertEqual((self.output / "clip.wav").read_bytes(),
+                                 f"render {index}".encode())
+
+    def test_cloud_model_changes_rerender(self):
+        for engine, constant in (("openai", "OPENAI_TTS_MODEL"),
+                                 ("openai-chat", "OPENAI_CHAT_MODEL")):
+            with self.subTest(engine=engine):
+                self.narrate("--engine", engine)
+                count = len(self.renders)
+                with patch.object(self.module, constant, "another-model"):
+                    self.narrate("--engine", engine)
+                self.assertEqual(len(self.renders), count + 1)
+
+    def test_implicit_and_explicit_defaults_share_cache(self):
+        for engine, voice in (("piper", self.module.PIPER_VOICE),
+                              ("openai", "nova"), ("openai-chat", "nova")):
+            with self.subTest(engine=engine):
+                self.narrate("--engine", engine)
+                count = len(self.renders)
+                self.narrate("--engine", engine, "--voice", voice)
+                self.assertEqual(len(self.renders), count)
+        self.narrate("--engine", "openai")
+        count = len(self.renders)
+        self.narrate()
+        self.assertEqual(len(self.renders), count)
+        with patch.object(self.module, "openai_key", return_value=None):
+            self.narrate()
+        self.assertEqual(len(self.renders), count + 1)
+
+    def test_cache_without_synthesis_settings_rerenders(self):
+        manifest = self.narrate()
+        manifest[0].pop("synthesis", None)
+        (self.output / "manifest.json").write_text(json.dumps(manifest), encoding="utf-8")
+        self.narrate()
+        self.assertEqual(len(self.renders), 2)
+
+    def test_changed_text_force_and_missing_wav_rerender(self):
+        self.narrate()
+        self.write_scenes("Read a different sentence exactly.")
+        manifest = self.narrate()
+        self.assertEqual(manifest[0]["text"], "Read a different sentence exactly.")
+        self.assertEqual(len(self.renders), 2)
+        self.narrate("--force")
+        self.assertEqual(len(self.renders), 3)
+        (self.output / "clip.wav").unlink()
+        self.narrate()
+        self.assertEqual(len(self.renders), 4)
+
+    def test_cached_clip_is_reverified_with_requested_asr_model(self):
+        self.narrate()
+        with patch.object(self.module, "transcribe_local", return_value=self.text) as asr:
+            self.narrate("--asr-model", "small.en", verify="on")
+        asr.assert_called_once_with(self.output / "clip.wav", "small.en")
+        self.assertEqual(len(self.renders), 1)
+
+    def test_unavailable_asr_respects_each_verification_mode(self):
+        for engine in ("piper", "openai", "openai-chat"):
+            for mode in ("auto", "on", "off"):
+                with self.subTest(engine=engine, mode=mode):
+                    self.module.transcribe_local.reset_mock()
+                    manifest = self.narrate("--engine", engine, verify=mode,
+                                            expected_exit=1 if mode == "on" else 0)
+                    self.assertEqual(bool(manifest), mode != "on")
+                    self.assertEqual(self.module.transcribe_local.call_count,
+                                     int(mode == "on" or (mode == "auto" and engine != "openai")))
+
+
 if __name__ == "__main__":
     unittest.main()

tests/proving-it-works-with-a-movie/test_subtitles.py

@@ -1,4 +1,5 @@
 import tempfile
+import json
 import sys
 import io
 from contextlib import redirect_stderr, redirect_stdout
@@ -50,6 +51,61 @@ class SubtitlePathRegression(unittest.TestCase):
                 self.assertEqual(module.main(), 1)
             self.assertIn("burn failed", stderr.getvalue())
 
+class SubtitleOffsetRegression(unittest.TestCase):
+    def subtitles(self, *manual, offsets=None):
+        module = fixtures.load_script("make-subtitles")
+        with tempfile.TemporaryDirectory() as directory:
+            root = Path(directory)
+            manifest, output = root / "manifest.json", root / "captions.srt"
+            manifest.write_text(json.dumps([
+                {"id": scene, "text": scene, "duration": 1.0}
+                for scene in ("intro", "body", "end")
+            ]), encoding="utf-8")
+            argv = ["make-subtitles", str(manifest), str(output)]
+            if offsets is not None:
+                path = root / "offsets.json"
+                path.write_text(json.dumps(offsets), encoding="utf-8")
+                argv += ["--offsets-json", str(path)]
+            if manual:
+                argv += ["--offsets", *manual]
+            with patch.object(sys, "argv", argv), redirect_stdout(io.StringIO()):
+                self.assertEqual(module.main(), 0)
+            cues = []
+            for block in output.read_text(encoding="utf-8").strip().split("\n\n"):
+                if not block:
+                    continue
+                _, timing, text = block.split("\n", 2)
+                times = []
+                for timestamp in timing.split(" --> "):
+                    h, m, s = timestamp.replace(",", ".").split(":")
+                    times.append(int(h) * 3600 + int(m) * 60 + float(s))
+                cues.append((*times, text))
+            return cues
+
+    def test_default_scenes_run_back_to_back(self):
+        self.assertEqual(self.subtitles(),
+                         [(0, 1, "intro"), (1, 2, "body"), (2, 3, "end")])
+
+    def test_partial_manual_offsets_preserve_other_scenes(self):
+        self.assertEqual(self.subtitles("intro=2"),
+                         [(2, 3, "intro"), (3, 4, "body"), (4, 5, "end")])
+        self.assertEqual(self.subtitles("body=4"),
+                         [(0, 1, "intro"), (4, 5, "body"), (5, 6, "end")])
+
+    def test_assembly_offsets_select_scenes_in_the_cut(self):
+        self.assertEqual(self.subtitles(offsets={"intro": 2, "end": 8}),
+                         [(2, 3, "intro"), (8, 9, "end")])
+
+    def test_manual_offsets_change_timing_without_changing_cut_membership(self):
+        self.assertEqual(self.subtitles("intro=3", "body=5", offsets={"intro": 2, "end": 8}),
+                         [(3, 4, "intro"), (8, 9, "end")])
+
+    def test_cut_without_narrated_scenes_has_no_cues(self):
+        for offsets in ({}, {"silent": 2}):
+            with self.subTest(offsets=offsets):
+                self.assertEqual(self.subtitles(offsets=offsets), [])
+
+
 class SubtitleIntegrationRegression(unittest.TestCase):
     def test_bom_manifest_and_offsets_write_utf8_under_legacy_console(self):
         import json