Skip to content

Commit 3579777

Browse files
committed
fix(podcasts): harden the voice pre-flight and the prompt examples (#1238)
Review follow-ups on the same four defects. - Reject a blank voice_id outright. `validate_speakers()` accepts a present-but-empty voice, which no catalogue lists and no provider can be said to own, so it fell into the "can't attribute it" branch, warned, and then failed during audio generation - the exact late failure this pre-flight exists to prevent. - Memoize catalogue lookups per validation pass (VoiceCatalogueCache). The catalogue for a (provider, model_name) cannot change between speakers of one profile, but every speaker re-fetched it and every unattributable voice re-enumerated all five static catalogues. For HTTP-backed providers (ElevenLabs, OpenRouter, OpenAI-compatible) that meant one network request per speaker, each able to run to the 10s timeout, before any work started. - Serialize speaker names in the JSON example with `tojson`. A name containing a quote or a backslash produced an invalid example, which is the one thing the example must never be. - Replace the angle-bracket placeholders with fully written sample dialogue and segment values. `<the complete words this speaker says out loud>` was copyable in exactly the way this PR set out to stop, contradicted the template's own "never emit placeholder content" rule, and would have been read aloud by the TTS engine. A copied example is now valid AND speakable. Since the sample is hard-coded English, the language block now labels it as such so it does not pull a non-English episode back toward English. Tests: the example is parsed with json.loads (not string-matched) and checked for speaker fidelity, escaping and speakable dialogue; the angle-bracket placeholders join the forbidden-skeleton list; blank voices, the per-profile lookup count and the no-duplicate-enumeration property are covered; and the unattributable-voice case now asserts the warning is emitted rather than only that nothing raised. All five new assertions fail against the previous commit.
1 parent 35b8e79 commit 3579777

7 files changed

Lines changed: 227 additions & 26 deletions

File tree

docs/7-DEVELOPMENT/podcasts.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,13 @@ Consequences to keep in mind when touching these files:
2626

2727
- Library prompt improvements are invisible here. The `{{ language }}` block was lost exactly this way, which made `EpisodeProfile.language` silently do nothing (#1238). `tests/test_podcast_prompt_templates.py` fails when a variable the bundled template uses is missing from the app's copy.
2828
- The variables available are whatever `podcast_creator.nodes` passes: the transcript template gets `speaker_names`, the outline template does **not**.
29-
- Never show the model a fill-in skeleton it can return verbatim. Examples are rendered from the real speaker names so a copied example is still valid output, and placeholders (`...`, `[like this]`) are banned explicitly — a copied `"speaker": "..."` used to abort the whole episode on podcast-creator's speaker-name validation, discarding the segments already generated.
29+
- Never show the model a fill-in skeleton it can return verbatim. The JSON examples carry the episode's real speaker names (serialized with `tojson`, so a name containing a quote can't break the example) and fully written sample dialogue, so a copied example is valid, speakable output. Placeholders (`...`, `[like this]`, `<like this>`) are banned explicitly — a copied `"speaker": "..."` used to abort the whole episode on podcast-creator's speaker-name validation, discarding the segments already generated. Because the sample is hard-coded English, the language block labels it as such; otherwise it nudges a non-English episode back toward English.
3030

3131
## Voice pre-flight
3232

3333
Audio is generated last, so a `voice_id` the TTS model doesn't accept fails only after the full transcript has been generated and paid for — and the provider message rarely names the voice (Gemini's 3.x TTS preview answers an unknown voice with `404 Requested entity was not found.`). `SpeakerProfile.validate_voices()` runs before generation and checks each speaker's voice (honoring per-speaker `voice_model` overrides) against esperanto's `available_voices` for the resolved model.
3434

35-
It fails the run **only** for a voice another provider's catalogue claims — the case of the migration-7 profiles, seeded with OpenAI voices (`nova`, `echo`, `shimmer`, …), against a Gemini voice model. A voice no catalogue knows is logged and allowed through, because those catalogues go stale (esperanto's OpenAI list predates `ash`), and an unavailable catalogue never blocks generation.
35+
A blank voice always fails (no provider can speak it). Otherwise it fails the run **only** for a voice another provider's catalogue claims — the case of the migration-7 profiles, seeded with OpenAI voices (`nova`, `echo`, `shimmer`, …), against a Gemini voice model. A voice no catalogue knows is logged and allowed through, because those catalogues go stale (esperanto's OpenAI list predates `ash`), and an unavailable catalogue never blocks generation. `VoiceCatalogueCache` memoizes each `(provider, model_name)` lookup for the duration of one pass — HTTP-backed providers (ElevenLabs, OpenRouter) otherwise pay a request per speaker, each able to run to the 10s timeout.
3636

3737
## Job lifecycle and the retry policy
3838

open_notebook/podcasts/models.py

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -193,15 +193,21 @@ async def validate_voices(self) -> None:
193193
transcript has been generated and paid for - with a provider message
194194
that names the wrong cause (#1238).
195195
196-
Only certain mismatches raise: a voice missing from the model's
197-
catalogue but present in another provider's. A voice no catalogue
198-
knows about is logged and allowed through, because esperanto's
199-
hard-coded lists go stale (see open_notebook.podcasts.voices). Raises
200-
ValueError so the podcast command treats it as permanent (no retry).
196+
A blank voice always raises: no provider can speak it. Otherwise only
197+
certain mismatches raise - a voice missing from the model's catalogue
198+
but present in another provider's. A voice no catalogue knows about is
199+
logged and allowed through, because esperanto's hard-coded lists go
200+
stale (see open_notebook.podcasts.voices). Raises ValueError so the
201+
podcast command treats it as permanent (no retry).
201202
"""
202-
from open_notebook.podcasts.voices import find_voice_mismatch, format_voice_list
203+
from open_notebook.podcasts.voices import (
204+
VoiceCatalogueCache,
205+
find_voice_mismatch,
206+
format_voice_list,
207+
)
203208

204209
profile_tts: Optional[Tuple[str, str, dict]] = None
210+
cache = VoiceCatalogueCache()
205211
for speaker in self.speakers:
206212
override = speaker.get("voice_model")
207213
if override:
@@ -213,9 +219,20 @@ async def validate_voices(self) -> None:
213219
profile_tts = await self.resolve_tts_config()
214220
provider, model_name, config = profile_tts
215221

216-
voice_id = str(speaker.get("voice_id") or "")
222+
# validate_speakers() accepts a present-but-empty voice_id, and an
223+
# empty voice can't be attributed to any provider below, so it
224+
# would otherwise slip through to audio generation.
225+
voice_id = str(speaker.get("voice_id") or "").strip()
226+
if not voice_id:
227+
raise ValueError(
228+
f"Speaker '{speaker.get('name')}' in speaker profile "
229+
f"'{self.name}' has no voice. Pick one of the voices "
230+
f"{provider}/{model_name} provides in Settings -> Speaker "
231+
"Profiles."
232+
)
233+
217234
mismatch = await find_voice_mismatch(
218-
provider, model_name, config, voice_id
235+
provider, model_name, config, voice_id, cache=cache
219236
)
220237
if mismatch is None:
221238
continue

open_notebook/podcasts/voices.py

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
"""
3030

3131
import asyncio
32-
from typing import NamedTuple, Optional, Set
32+
from typing import Dict, NamedTuple, Optional, Set, Tuple
3333

3434
from loguru import logger
3535

@@ -108,14 +108,48 @@ def _lookup() -> Set[str]:
108108
return voice_ids or None
109109

110110

111+
class VoiceCatalogueCache:
112+
"""Memoizes catalogues for one validation pass.
113+
114+
A catalogue can't change between the speakers of a single profile, but the
115+
HTTP-backed providers charge a request (up to CATALOGUE_TIMEOUT_SECONDS) for
116+
every lookup, and the cross-provider attribution below enumerates five more
117+
catalogues per mismatch. Without this, a 4-speaker ElevenLabs profile made
118+
the same request four times before generation could start.
119+
120+
Deliberately per-pass rather than process-wide: a fetched catalogue reflects
121+
the credentials in use, and a run should see voices added since the last one.
122+
"""
123+
124+
def __init__(self) -> None:
125+
self._catalogues: Dict[Tuple[str, Optional[str]], Optional[Set[str]]] = {}
126+
127+
async def get(
128+
self, provider: str, model_name: Optional[str] = None, config: Optional[dict] = None
129+
) -> Optional[Set[str]]:
130+
key = (provider, model_name)
131+
if key not in self._catalogues:
132+
self._catalogues[key] = await get_known_voice_ids(
133+
provider, model_name, config
134+
)
135+
return self._catalogues[key]
136+
137+
111138
async def find_voice_mismatch(
112139
provider: str,
113140
model_name: str,
114141
config: Optional[dict],
115142
voice_id: str,
143+
cache: Optional[VoiceCatalogueCache] = None,
116144
) -> Optional[VoiceMismatch]:
117-
"""Report a voice the model does not list, or None when it looks usable."""
118-
known_voices = await get_known_voice_ids(provider, model_name, config)
145+
"""Report a voice the model does not list, or None when it looks usable.
146+
147+
Pass a shared `cache` when checking several speakers so each catalogue is
148+
fetched once for the whole profile.
149+
"""
150+
cache = cache or VoiceCatalogueCache()
151+
152+
known_voices = await cache.get(provider, model_name, config)
119153
if not known_voices:
120154
return None
121155
if voice_id.lower() in known_voices:
@@ -125,9 +159,7 @@ async def find_voice_mismatch(
125159
for other in STATIC_CATALOGUE_PROVIDERS:
126160
if other == provider:
127161
continue
128-
other_voices = await get_known_voice_ids(
129-
other, config=dict(_CATALOGUE_ONLY_CONFIG)
130-
)
162+
other_voices = await cache.get(other, config=dict(_CATALOGUE_ONLY_CONFIG))
131163
if other_voices and voice_id.lower() in other_voices:
132164
other_providers.add(other)
133165

prompts/podcast/outline.jinja

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,17 @@ Please create an outline based on this briefing. Your outline should consist of
4141
7. This is a whole podcast so no need to reintroduce speakers or topics on each segment. Segments are just markers for us to know to change the topics, nothing else.
4242
8. Include an introduction segment at the beginning and a conclusion or wrap-up segment at the end.
4343

44-
Return exactly one JSON object with a single root key "segments" whose value is a list of {{ num_segments }} entries. Each entry has exactly three keys, "name", "description" and "size". The line below is the SHAPE only - replace every angle-bracket description with real content:
44+
Return exactly one JSON object with a single root key "segments" whose value is a list of {{ num_segments }} entries. Each entry has exactly three keys, "name", "description" and "size". The line below shows the required structure, filled in with sample values - keep the structure and write your own segments from the briefing:
4545

46-
{"segments": [{"name": "<the real title of this segment>", "description": "<what is discussed in this segment, including the key points and questions to cover>", "size": "short"}, {"name": "<the real title of the next segment>", "description": "<what is discussed in that segment, including the key points and questions to cover>", "size": "medium"}]}
46+
{"segments": [{"name": "Setting the scene", "description": "Introduce the subject and the questions this episode sets out to answer.", "size": "short"}, {"name": "Working through the detail", "description": "Take the main points from the briefing in turn, with the specifics that matter most.", "size": "medium"}]}
4747

4848
- "size" must be exactly one of "short", "medium" or "long".
49-
- Never emit placeholder or elided content: no "..." or "…", no "[like this]", no "TODO", no empty strings, no trailing commas.
49+
- The sample values above show the structure only - write segments drawn from the briefing instead of reusing them.
50+
- Never emit placeholder or elided content: no "..." or "…", no "[like this]", no "<like this>", no "TODO", no empty strings, no trailing commas.
5051
- Write out all {{ num_segments }} segments in full; never shorten or truncate the list.
52+
{% if language %}
53+
- The sample values above are in English only to show the structure; every segment name and description you write must be in {{ language }}.
54+
{% endif %}
5155

5256
Formatting instructions:
5357
{{ format_instructions}}

prompts/podcast/transcript.jinja

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,18 +78,21 @@ Follow these format requirements strictly:
7878
{% endif %}
7979

8080

81-
Return exactly one JSON object with a single root key "transcript" whose value is a list of entries. Each entry has exactly two keys, "speaker" and "dialogue". The line below is the SHAPE only - replace every angle-bracket description with real content:
81+
Return exactly one JSON object with a single root key "transcript" whose value is a list of entries. Each entry has exactly two keys, "speaker" and "dialogue". The line below shows the required structure, filled in with sample dialogue - keep the structure and write your own words for this segment:
8282

8383
{% if speakers|length == 1 %}
84-
{"transcript": [{"speaker": "{{ speaker_names[0] }}", "dialogue": "<the complete words this speaker says out loud, written out in full>"}]}
84+
{"transcript": [{"speaker": {{ speaker_names[0]|tojson }}, "dialogue": "Let's pick up where we left off, because this is where the material really starts to come together."}]}
8585
{% else %}
86-
{"transcript": [{"speaker": "{{ speaker_names[0] }}", "dialogue": "<the complete words this speaker says out loud, written out in full>"}, {"speaker": "{{ speaker_names[1] if speaker_names|length > 1 else speaker_names[0] }}", "dialogue": "<the complete words this speaker says out loud, written out in full>"}]}
86+
{"transcript": [{"speaker": {{ speaker_names[0]|tojson }}, "dialogue": "Let's pick up where we left off, because this is where the material really starts to come together."}, {"speaker": {{ (speaker_names[1] if speaker_names|length > 1 else speaker_names[0])|tojson }}, "dialogue": "Agreed, and the detail I keep coming back to is the one that changes how you read everything before it."}]}
8787
{% endif %}
8888

8989
- Every "speaker" value must be copied character-for-character from this list: {{ speaker_names|join(', ') }}
90-
- Every "dialogue" value must be the finished words the speaker says out loud; it is sent straight to a text-to-speech engine.
91-
- Never emit placeholder or elided content: no "..." or "…", no "[like this]", no "TODO", no empty strings, no trailing commas.
90+
- Every "dialogue" value must be the finished words the speaker says out loud; it is sent straight to a text-to-speech engine. The sample dialogue above shows the structure only - write dialogue for this segment instead of reusing it.
91+
- Never emit placeholder or elided content: no "..." or "…", no "[like this]", no "<like this>", no "TODO", no empty strings, no trailing commas.
9292
- Never shorten or truncate the list. Write out every entry in full.
93+
{% if language %}
94+
- The sample dialogue above is in English only to show the structure; every "dialogue" value you write must be in {{ language }}.
95+
{% endif %}
9396

9497
Formatting instructions:
9598
{{ format_instructions}}

tests/test_podcast_prompt_templates.py

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
Note the outline template receives `speakers` but NOT `speaker_names`.
1919
"""
2020

21+
import json
2122
import re
2223
from pathlib import Path
2324

@@ -40,13 +41,21 @@
4041
]
4142

4243
# Strings that must never reach the model: each one is copyable as content.
44+
# Angle-bracket descriptions count too - a model that copies
45+
# "<the complete words this speaker says out loud>" into a dialogue value sends
46+
# that straight to the TTS engine, and the templates' own rules ban placeholders.
4347
COPYABLE_SKELETONS = (
4448
"[Actual Speaker Name]",
4549
"[Speaker's dialogue based on their personality and expertise]",
4650
"[Segment Name]",
4751
"[Description of the segment content]",
4852
'{"transcript": [...]}',
4953
'{"segments": [...]}',
54+
"<the complete words this speaker says out loud, written out in full>",
55+
"<the real title of this segment>",
56+
"<the real title of the next segment>",
57+
"<what is discussed in this segment, including the key points and questions to cover>",
58+
"<what is discussed in that segment, including the key points and questions to cover>",
5059
)
5160

5261

@@ -87,6 +96,59 @@ def render_outline(**overrides) -> str:
8796
return render("outline", **data)
8897

8998

99+
def example_object(rendered: str, root_key: str) -> dict:
100+
"""Parse the JSON example the prompt shows the model.
101+
102+
The example is the contract the model imitates, so it has to be valid JSON
103+
in its own right - a speaker name carrying a quote or a backslash would
104+
otherwise hand the model a broken example to copy.
105+
"""
106+
prefix = '{"' + root_key + '":'
107+
for line in rendered.splitlines():
108+
if line.startswith(prefix):
109+
return json.loads(line)
110+
raise AssertionError(f"no {root_key} example found in the rendered prompt")
111+
112+
113+
class TestExampleIsValidAndComplete:
114+
"""Whatever the model copies from the example must be usable output."""
115+
116+
def test_transcript_example_parses_and_names_the_speakers(self):
117+
example = example_object(render_transcript(), "transcript")
118+
assert [entry["speaker"] for entry in example["transcript"]] == [
119+
"Marcus Thompson",
120+
"Elena Vasquez",
121+
]
122+
123+
def test_transcript_example_survives_json_special_characters(self):
124+
r"""A name like Dr. "Alex" Chen\ must be escaped, not interpolated raw."""
125+
speakers = [
126+
{"name": 'Dr. "Alex" Chen\\', "backstory": "b", "personality": "p"},
127+
{"name": "Jamie\tRodriguez", "backstory": "b", "personality": "p"},
128+
]
129+
example = example_object(render_transcript(speakers=speakers), "transcript")
130+
assert [entry["speaker"] for entry in example["transcript"]] == [
131+
'Dr. "Alex" Chen\\',
132+
"Jamie\tRodriguez",
133+
]
134+
135+
def test_transcript_example_dialogue_is_speakable(self):
136+
"""Dialogue goes straight to TTS, so the example must not contain a
137+
description of what to write - a copied one would be read aloud."""
138+
example = example_object(render_transcript(), "transcript")
139+
for entry in example["transcript"]:
140+
assert "<" not in entry["dialogue"]
141+
assert entry["dialogue"].endswith(".")
142+
143+
def test_outline_example_parses_with_valid_sizes(self):
144+
example = example_object(render_outline(), "segments")
145+
assert example["segments"]
146+
for segment in example["segments"]:
147+
assert segment["size"] in {"short", "medium", "long"}
148+
assert "<" not in segment["name"]
149+
assert "<" not in segment["description"]
150+
151+
90152
class TestNoCopyableSkeletons:
91153
"""Whatever the model copies from the prompt must be valid output."""
92154

@@ -154,6 +216,14 @@ def test_outline_includes_the_language_instruction(self):
154216
assert "IMPORTANT LANGUAGE INSTRUCTION" in rendered
155217
assert "segment names, descriptions" in rendered
156218

219+
@pytest.mark.parametrize(
220+
"renderer", [render_transcript, render_outline], ids=["transcript", "outline"]
221+
)
222+
def test_english_sample_is_flagged_as_english(self, renderer):
223+
"""The example is hard-coded English; say so, or it nudges the model
224+
back toward English for a non-English episode."""
225+
assert "in English only to show the structure" in renderer(language="Hebrew")
226+
157227
@pytest.mark.parametrize(
158228
"renderer", [render_transcript, render_outline], ids=["transcript", "outline"]
159229
)

0 commit comments

Comments
 (0)