fix(podcasts): stop late, misattributed podcast generation failures - #1239
Open
alefbt wants to merge 4 commits into
Open
fix(podcasts): stop late, misattributed podcast generation failures#1239alefbt wants to merge 4 commits into
alefbt wants to merge 4 commits into
Conversation
…fnovo#1238) Four defects combined into one story: a Gemini/non-English podcast run burned several minutes and a full transcript's worth of tokens, then died with an error that named the wrong cause. Prompt templates (prompts/podcast/{transcript,outline}.jinja): - Restore the `{{ language }}` block. These templates shadow podcast-creator's bundled ones (the library resolves Path.cwd()/prompts/podcast/ before its own package resources) and had lost it, so an episode profile set to he-IL produced an English outline and English segment titles - the field looked supported and did nothing. - Render the JSON example from the episode's real speaker names instead of a fill-in skeleton (`"speaker": "[Actual Speaker Name]"` plus a bare `...`) that models returned verbatim, failing podcast-creator's speaker-name validation and discarding the outline and every segment already generated. A copied example is now valid output. Placeholders and truncation are banned explicitly, and the ```json fence that contradicted the "no code blocks" instruction is gone along with the second copyable example. Voice pre-flight (open_notebook/podcasts/voices.py, SpeakerProfile .validate_voices, generate_podcast_command): - Audio is generated last, so a voice_id the TTS model does not accept failed only after the whole transcript had been generated and paid for - and Gemini answers an unknown voice with `404 Requested entity was not found.`, which reads like a missing model. Voices (including per-speaker voice_model overrides) are now checked against esperanto's catalogue up front, naming the offending voice and the valid ones. - Only voices another provider's catalogue claims fail the run - the profiles seeded by migration 7 carry OpenAI names (nova, echo, shimmer). Unknown voices warn and proceed, because those catalogues go stale (esperanto's OpenAI list predates `ash`), and an unavailable catalogue never blocks a run. Error hints (commands/podcast_commands.py): - The GPT-5 extended-thinking note was the only hint, so `Invalid speaker name` got nothing and a truncated Gemini response was told to switch to gpt-4o. Placeholder speaker names, unusable voices and truncation now each get their own explanation. Tests cover both templates (including a drift check that fails when a variable the bundled template uses is missing from the app's copy), the voice pre-flight and the hint mapping. No network or credentials needed.
alefbt
marked this pull request as ready for review
August 3, 2026 11:01
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 10 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…fnovo#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.
Author
|
Second commit (3579777) addresses a review pass over the first:
Tests now parse the example with |
Contributor
There was a problem hiding this comment.
1 issue found across 7 files (changes from recent commits).
Confidence score: 3/5
- In
open_notebook/podcasts/models.py, per-speaker override validation can reuse a cached provider/model catalogue from a different credential endpoint, so an unsupported voice may pass checks and then fail later during audio generation; scope the cache key by endpoint (or disable cross-endpoint reuse) so validation matches the actual credentials.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="open_notebook/podcasts/models.py">
<violation number="1" location="open_notebook/podcasts/models.py:210">
P2: Per-speaker overrides with identical provider/model names but different credential endpoints can validate against another endpoint's cached catalogue. This can let an unsupported voice through to the late audio failure (or reject a valid one); include catalogue-affecting config in the cache key, or do not share HTTP-backed catalogues across distinct configs.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| ) | ||
|
|
||
| profile_tts: Optional[Tuple[str, str, dict]] = None | ||
| cache = VoiceCatalogueCache() |
Contributor
There was a problem hiding this comment.
P2: Per-speaker overrides with identical provider/model names but different credential endpoints can validate against another endpoint's cached catalogue. This can let an unsupported voice through to the late audio failure (or reject a valid one); include catalogue-affecting config in the cache key, or do not share HTTP-backed catalogues across distinct configs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At open_notebook/podcasts/models.py, line 210:
<comment>Per-speaker overrides with identical provider/model names but different credential endpoints can validate against another endpoint's cached catalogue. This can let an unsupported voice through to the late audio failure (or reject a valid one); include catalogue-affecting config in the cache key, or do not share HTTP-backed catalogues across distinct configs.</comment>
<file context>
@@ -193,15 +193,21 @@ async def validate_voices(self) -> None:
+ )
profile_tts: Optional[Tuple[str, str, dict]] = None
+ cache = VoiceCatalogueCache()
for speaker in self.speakers:
override = speaker.get("voice_model")
</file context>
…model (lfnovo#1238) VoiceCatalogueCache keyed on (provider, model_name), on the assumption that a catalogue is a property of the model. It is a property of the endpoint and account: speakers override `voice_model` individually, and two `model` records sharing a provider and model name can still resolve to different credentials - a second API key, another `base_url`/`endpoint_tts` - and to_esperanto_config() carries exactly those fields. An ElevenLabs voice library is per-account, so the second speaker was validated against the first account's voices: an unusable voice passed through to the late audio failure this pre-flight exists to prevent, and a valid one could be rejected outright if it happened to share a name with a static provider's voice (that path raises rather than warns). The key now includes a digest of the credential config. Hashed rather than stored verbatim, because the config carries API keys and cache keys surface in reprs and tracebacks. Identical configs - the common case, every speaker on the profile's own voice_model - still share one lookup.
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 7 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…e-flight (lfnovo#1238) VoiceCatalogueCache memoized whatever get_known_voice_ids returned, including None. A None means "catalogue unknown" - and a timeout is indistinguishable from a provider esperanto doesn't know - so a single flaky request while checking speaker 1 silently disabled validation for every later speaker resolving to the same provider, model and credential. Caching only successes (the obvious fix) would instead pay CATALOGUE_TIMEOUT_SECONDS per speaker whenever an endpoint is simply down, which is what the cache was added to prevent. So failures are retried, but bounded: MAX_CATALOGUE_ATTEMPTS (2) per key per pass survives a one-off timeout while capping a dead endpoint at two waits instead of one per speaker. Also drop mistral from STATIC_CATALOGUE_PROVIDERS. That list is documented as "literal dict, no credentials, no network", but esperanto's Mistral provider paginates through GET /audio/voices - so attributing an unplaceable voice sent a request (and collected a 401) to a provider the deployment may not even use. Surfaced by the attribution test, which now asserts the per-key bound instead of "no repeats", since a failing catalogue is legitimately retried once.
Author
|
Fourth commit (d51dbec) — the cached-failure note is valid, fixed, plus one thing it led me to. Caching It also surfaced a real bug in my own The attribution test now asserts the per-key bound rather than "no repeats", since a failing catalogue is legitimately retried once. 722 tests pass, ruff and mypy clean; both new tests fail against |
Contributor
There was a problem hiding this comment.
7 issues found across 7 files (changes from recent commits).
Confidence score: 3/5
- In
open_notebook/podcasts/models.py, voice IDs with surrounding whitespace can pass preflight validation but later fail when generation uses the untrimmed profile value, creating a real run-time failure path for users—canonicalize and persist the normalized speaker-profile voice ID (or validate the exact value used downstream). - In
prompts/podcast/transcript.jinja, the embedded example can teach the model to emit too few turns and, for non-English runs, to output English text, which can produce invalid transcript structure and wrong-language TTS content—render the example fromturnsand target language so it cannot contradict constraints. - In
prompts/podcast/outline.jinja, the fixed two-segment example can be copied even whennum_segmentsrequests 3–20, silently shortening episodes against configuration—make the outline example dynamic to the requested segment count. - The checks in
tests/test_podcast_voice_validation.pyandtests/test_podcast_prompt_templates.py(plus wording indocs/7-DEVELOPMENT/podcasts.md) currently leave these behaviors under-guarded, so regressions may slip through despite green tests—tighten exact-once catalogue assertions and relax dialogue validation to accept valid?/!endings while updating docs to match actual template behavior.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/7-DEVELOPMENT/podcasts.md">
<violation number="1" location="docs/7-DEVELOPMENT/podcasts.md:29">
P3: The prompt-template guidance inaccurately describes the outline example: only the transcript example contains real speaker names and speakable dialogue; the outline example contains fixed segment values. Distinguishing the two examples would prevent maintainers from expecting `speaker_names` or dialogue in the outline template.</violation>
</file>
<file name="prompts/podcast/transcript.jinja">
<violation number="1" location="prompts/podcast/transcript.jinja:84">
P2: Non-English podcast runs still receive a valid English transcript example that can be copied into the output despite the language instruction, producing English/irrelevant TTS content instead of the requested segment. A localized example or a rendering strategy that supplies example dialogue in the requested language would avoid this conflicting signal.</violation>
<violation number="2" location="prompts/podcast/transcript.jinja:84">
P2: A model that copies this otherwise-valid example still violates the prompt's minimum transcript length: it emits one entry for a solo segment or two entries for a panel while `turns` is 3, 6, or 10. Rendering the sample with `turns` entries (or otherwise making the example explicitly non-copyable) would prevent short, incomplete segments from being accepted.</violation>
</file>
<file name="tests/test_podcast_voice_validation.py">
<violation number="1" location="tests/test_podcast_voice_validation.py:321">
P3: Duplicate attribution lookups can pass this test even though its name promises that catalogues are not re-enumerated per speaker. An exact-once assertion for a successful attribution catalogue, such as OpenAI, would verify the intended cache behavior while retaining the retry-ceiling check.</violation>
</file>
<file name="tests/test_podcast_prompt_templates.py">
<violation number="1" location="tests/test_podcast_prompt_templates.py:141">
P3: Valid prompt examples ending in `?` or `!` will be rejected even though they are speakable dialogue. The assertion could check non-empty text and the forbidden placeholder forms instead of requiring one punctuation mark.</violation>
</file>
<file name="open_notebook/podcasts/models.py">
<violation number="1" location="open_notebook/podcasts/models.py:225">
P2: Whitespace-padded voice IDs can pass this pre-flight but still fail later: the stripped local value is validated, while audio generation uses the original speaker-profile value. Canonicalizing the profile value or rejecting non-canonical whitespace would keep pre-flight and TTS execution consistent.</violation>
</file>
<file name="prompts/podcast/outline.jinja">
<violation number="1" location="prompts/podcast/outline.jinja:46">
P2: The copied outline example always has two segments even when the requested `num_segments` is 3–20, so it can silently shorten the episode despite the surrounding instructions requiring the configured count. A dynamic example with exactly `num_segments` valid entries, or a format example that cannot be mistaken for complete output, would keep the copy-safe behavior without contradicting the count requirement.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| 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: | ||
|
|
||
| {% if speakers|length == 1 %} | ||
| {"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."}]} |
Contributor
There was a problem hiding this comment.
P2: Non-English podcast runs still receive a valid English transcript example that can be copied into the output despite the language instruction, producing English/irrelevant TTS content instead of the requested segment. A localized example or a rendering strategy that supplies example dialogue in the requested language would avoid this conflicting signal.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At prompts/podcast/transcript.jinja, line 84:
<comment>Non-English podcast runs still receive a valid English transcript example that can be copied into the output despite the language instruction, producing English/irrelevant TTS content instead of the requested segment. A localized example or a rendering strategy that supplies example dialogue in the requested language would avoid this conflicting signal.</comment>
<file context>
@@ -78,18 +78,21 @@ Follow these format requirements strictly:
{% if speakers|length == 1 %}
-{"transcript": [{"speaker": "{{ speaker_names[0] }}", "dialogue": "<the complete words this speaker says out loud, written out in full>"}]}
+{"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."}]}
{% else %}
-{"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>"}]}
</file context>
| 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: | ||
|
|
||
| {% if speakers|length == 1 %} | ||
| {"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."}]} |
Contributor
There was a problem hiding this comment.
P2: A model that copies this otherwise-valid example still violates the prompt's minimum transcript length: it emits one entry for a solo segment or two entries for a panel while turns is 3, 6, or 10. Rendering the sample with turns entries (or otherwise making the example explicitly non-copyable) would prevent short, incomplete segments from being accepted.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At prompts/podcast/transcript.jinja, line 84:
<comment>A model that copies this otherwise-valid example still violates the prompt's minimum transcript length: it emits one entry for a solo segment or two entries for a panel while `turns` is 3, 6, or 10. Rendering the sample with `turns` entries (or otherwise making the example explicitly non-copyable) would prevent short, incomplete segments from being accepted.</comment>
<file context>
@@ -78,18 +78,21 @@ Follow these format requirements strictly:
{% if speakers|length == 1 %}
-{"transcript": [{"speaker": "{{ speaker_names[0] }}", "dialogue": "<the complete words this speaker says out loud, written out in full>"}]}
+{"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."}]}
{% else %}
-{"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>"}]}
</file context>
| # validate_speakers() accepts a present-but-empty voice_id, and an | ||
| # empty voice can't be attributed to any provider below, so it | ||
| # would otherwise slip through to audio generation. | ||
| voice_id = str(speaker.get("voice_id") or "").strip() |
Contributor
There was a problem hiding this comment.
P2: Whitespace-padded voice IDs can pass this pre-flight but still fail later: the stripped local value is validated, while audio generation uses the original speaker-profile value. Canonicalizing the profile value or rejecting non-canonical whitespace would keep pre-flight and TTS execution consistent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At open_notebook/podcasts/models.py, line 225:
<comment>Whitespace-padded voice IDs can pass this pre-flight but still fail later: the stripped local value is validated, while audio generation uses the original speaker-profile value. Canonicalizing the profile value or rejecting non-canonical whitespace would keep pre-flight and TTS execution consistent.</comment>
<file context>
@@ -213,9 +219,20 @@ async def validate_voices(self) -> None:
+ # validate_speakers() accepts a present-but-empty voice_id, and an
+ # empty voice can't be attributed to any provider below, so it
+ # would otherwise slip through to audio generation.
+ voice_id = str(speaker.get("voice_id") or "").strip()
+ if not voice_id:
+ raise ValueError(
</file context>
| ] | ||
| } | ||
| ``` | ||
| {"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"}]} |
Contributor
There was a problem hiding this comment.
P2: The copied outline example always has two segments even when the requested num_segments is 3–20, so it can silently shorten the episode despite the surrounding instructions requiring the configured count. A dynamic example with exactly num_segments valid entries, or a format example that cannot be mistaken for complete output, would keep the copy-safe behavior without contradicting the count requirement.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At prompts/podcast/outline.jinja, line 46:
<comment>The copied outline example always has two segments even when the requested `num_segments` is 3–20, so it can silently shorten the episode despite the surrounding instructions requiring the configured count. A dynamic example with exactly `num_segments` valid entries, or a format example that cannot be mistaken for complete output, would keep the copy-safe behavior without contradicting the count requirement.</comment>
<file context>
@@ -41,13 +41,17 @@ Please create an outline based on this briefing. Your outline should consist of
+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:
-{"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"}]}
+{"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"}]}
- "size" must be exactly one of "short", "medium" or "long".
</file context>
|
|
||
| - 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. | ||
| - The variables available are whatever `podcast_creator.nodes` passes: the transcript template gets `speaker_names`, the outline template does **not**. | ||
| - 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. |
Contributor
There was a problem hiding this comment.
P3: The prompt-template guidance inaccurately describes the outline example: only the transcript example contains real speaker names and speakable dialogue; the outline example contains fixed segment values. Distinguishing the two examples would prevent maintainers from expecting speaker_names or dialogue in the outline template.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/7-DEVELOPMENT/podcasts.md, line 29:
<comment>The prompt-template guidance inaccurately describes the outline example: only the transcript example contains real speaker names and speakable dialogue; the outline example contains fixed segment values. Distinguishing the two examples would prevent maintainers from expecting `speaker_names` or dialogue in the outline template.</comment>
<file context>
@@ -26,13 +26,13 @@ Consequences to keep in mind when touching these files:
- 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.
- The variables available are whatever `podcast_creator.nodes` passes: the transcript template gets `speaker_names`, the outline template does **not**.
-- 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.
+- 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.
## Voice pre-flight
</file context>
Suggested change
| - 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. | |
| - Never show the model a fill-in skeleton it can return verbatim. The transcript example uses 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; the outline example uses fixed, fully written sample segment values. 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. |
| # The resolved model's own catalogue succeeds, so it is fetched once for | ||
| # the whole profile no matter how many speakers need checking. | ||
| assert counts[(GEMINI_TTS[0], GEMINI_TTS[1])] == 1 | ||
| assert max(counts.values()) <= MAX_CATALOGUE_ATTEMPTS |
Contributor
There was a problem hiding this comment.
P3: Duplicate attribution lookups can pass this test even though its name promises that catalogues are not re-enumerated per speaker. An exact-once assertion for a successful attribution catalogue, such as OpenAI, would verify the intended cache behavior while retaining the retry-ceiling check.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_podcast_voice_validation.py, line 321:
<comment>Duplicate attribution lookups can pass this test even though its name promises that catalogues are not re-enumerated per speaker. An exact-once assertion for a successful attribution catalogue, such as OpenAI, would verify the intended cache behavior while retaining the retry-ceiling check.</comment>
<file context>
@@ -111,12 +140,185 @@ async def test_gemini_voices_pass_case_insensitively(self):
+ # The resolved model's own catalogue succeeds, so it is fetched once for
+ # the whole profile no matter how many speakers need checking.
+ assert counts[(GEMINI_TTS[0], GEMINI_TTS[1])] == 1
+ assert max(counts.values()) <= MAX_CATALOGUE_ATTEMPTS
@pytest.mark.asyncio
</file context>
Suggested change
| assert max(counts.values()) <= MAX_CATALOGUE_ATTEMPTS | |
| assert counts[("openai", None)] == 1 | |
| assert max(counts.values()) <= MAX_CATALOGUE_ATTEMPTS |
| example = example_object(render_transcript(), "transcript") | ||
| for entry in example["transcript"]: | ||
| assert "<" not in entry["dialogue"] | ||
| assert entry["dialogue"].endswith(".") |
Contributor
There was a problem hiding this comment.
P3: Valid prompt examples ending in ? or ! will be rejected even though they are speakable dialogue. The assertion could check non-empty text and the forbidden placeholder forms instead of requiring one punctuation mark.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_podcast_prompt_templates.py, line 141:
<comment>Valid prompt examples ending in `?` or `!` will be rejected even though they are speakable dialogue. The assertion could check non-empty text and the forbidden placeholder forms instead of requiring one punctuation mark.</comment>
<file context>
@@ -87,6 +96,59 @@ def render_outline(**overrides) -> str:
+ example = example_object(render_transcript(), "transcript")
+ for entry in example["transcript"]:
+ assert "<" not in entry["dialogue"]
+ assert entry["dialogue"].endswith(".")
+
+ def test_outline_example_parses_with_valid_sizes(self):
</file context>
Suggested change
| assert entry["dialogue"].endswith(".") | |
| assert entry["dialogue"].strip() | |
| assert not re.search( | |
| r"\.\.\.|…|\[[^\]]*\]|<[^>]*>|\bTODO\b", entry["dialogue"] | |
| ) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes the four podcast-generation defects reported in #1238 that live in this repo. Together they made a Gemini / non-English run burn several minutes and a full transcript's worth of tokens before dying with an error that named the wrong cause.
Prompt templates (
prompts/podcast/{transcript,outline}.jinja){{ language }}block. These templates shadow podcast-creator's bundled ones — the library resolvesPath.cwd()/prompts/podcast/<name>.jinjabefore its own package resources — and had lost it, so an episode profile set tohe-ILproduced an English outline and English segment titles. The field looked supported and did nothing."speaker": "[Actual Speaker Name]"plus a bare...). Models returned that skeleton verbatim, which failed podcast-creator's speaker-name validation and discarded the outline plus every segment already generated. A copied example is now valid output. Placeholders and truncation are banned explicitly, and the```jsonfence that contradicted the "do NOT wrap the JSON in code blocks" rule is gone, along with the second copyable{"transcript": [...]}/{"segments": [...]}example.Voice pre-flight (new
open_notebook/podcasts/voices.py,SpeakerProfile.validate_voices(), called fromgenerate_podcast_command)voice_idthe TTS model doesn't accept failed only after the whole transcript had been generated and paid for — and Gemini's 3.x TTS preview answers an unknown voice with404 Requested entity was not found., which reads like a missing model. Voices are now checked against esperanto'savailable_voicesbefore generation starts, honoring per-speakervoice_modeloverrides, and the error names the offending voice and the valid ones.nova,alloy,echo,shimmer,ash). A voice no catalogue knows is logged and allowed through, because those catalogues go stale: esperanto's OpenAI list predatesash, which migration 7 itself seeds and the OpenAI API accepts, so a plain "absent from the catalogue → reject" would break working installs. HTTP-backed catalogues (ElevenLabs, OpenRouter, OpenAI-compatible) are fetched in a worker thread with a 10s timeout, and any failure means "unknown" — an unavailable catalogue never blocks a generation that would otherwise work.Error hints (
commands/podcast_commands.py)Invalid json output/Expecting value. SoInvalid speaker namegot nothing, and a truncated Gemini response was advised to switch togpt-4o. Placeholder speaker names, unusable voices, and unparseable-or-truncated output now each get their own explanation (the truncation one mentions the 5000-token transcript segment cap and the profile'smax_tokens).Not included, since they belong to other repos: podcast-creator classifying
OutputParserException(aValueErrorsubclass) as non-retryable, renderingformat_instructionsfrom the unvalidatedTranscriptmodel, and esperanto's Google TTS collapsing a bad-voice 404 intoGoogle API error: …. All three are described in #1238.Related Issue
Fixes #1238
Type of Change
How Has This Been Tested?
uv run pytest)Test Details:
uv run pytest tests/→ 697 passed (42 of them new),ruff check .clean,uv run python -m mypy .clean. The new tests need no network and no credentials — the providers they exercise return literal dicts fromavailable_voices.tests/test_podcast_prompt_templates.py— renders both templates and asserts the negatives (no[Actual Speaker Name], no{"transcript": [...]}, no fence outside the "no fences" rule itself), that the example carries real speaker names, that the out-of-rangespeaker_names[1]guard holds, and that the language instruction appears with alanguageand not without one. Plus a drift check that fails when a{{ variable }}podcast-creator's bundled template uses is missing from the app's copy — the mechanism that lost the language block in the first place.tests/test_podcast_voice_validation.py— Gemini's catalogue resolves offline and case-insensitively (Kore→kore),echoon Gemini is attributed to OpenAI and raises,ashon Gemini only warns, an unavailable catalogue never raises, and a per-speakervoice_modeloverride is what gets checked.tests/test_podcast_error_hints.py— each failure string maps to its own hint, and an unrecognised failure gets none.I confirmed the template tests are genuine regressions by running the same checks against
main's templates:[Actual Speaker Name]present,{"segments": [...]}present, no language block rendered withlanguage="Hebrew", andlanguagereported missing from both app templates.End-to-end verification of these fixes happened on the reporting deployment (bind-mounted templates + a patched
_is_retryable, described under "Downstream workaround" in #1238): the same 6-segment Hebrew episode that previously aborted at segment 2/6 produced 6 segments, 42 Hebrew dialogue lines across all three speakers and an MP3 in 474 s. I have not re-run a real generation from this branch, so a maintainer smoke test on a live Gemini or OpenAI deployment before merge would be worthwhile — particularly the voice pre-flight, which is the only change that can refuse to start a run.Design Alignment
Explanation: Multi-provider flexibility is the direct target — the podcast pipeline silently assumed OpenAI voices and English output, and both assumptions broke a Gemini deployment with non-English sources. Simplicity: no new configuration, no new endpoints; failures just arrive earlier and say what to change. Async-first: the pre-flight is
await-ed, and the only lookup that can block runs viaasyncio.to_threadwith a timeout.Checklist
Code Quality
Testing
make rufforruff check . --fixmake lintoruv run python -m mypy .Documentation
/docs(if applicable) —docs/7-DEVELOPMENT/podcasts.mdgained two sections: how the app's prompt templates shadow the library's, and what the voice pre-flight does and deliberately does not doDatabase Changes
/migrations) — no schema changesBreaking Changes
Additional Context
Two things worth a maintainer decision, both raised in #1238 and deliberately left out of this PR:
prompts/podcast/*.jinjahave diverged from podcast-creator's in both directions — they gained the solo-speaker branches and the<think>-tag rules, and they lost the language block. Every future library prompt improvement stays invisible here. The drift test in this PR turns that from a silent problem into a failing test, but the real fix is either upstreaming the app-only features and deletingprompts/podcast/, or documenting the fork as intentional.voice_ids) nor offer the provider's voices as a dropdown in the speaker-profile form. Happy to add either if you want them here.Pre-Submission Verification