-
Notifications
You must be signed in to change notification settings - Fork 4.2k
fix(podcasts): stop late, misattributed podcast generation failures #1239
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
35b8e79
3579777
4102c8b
d51dbec
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -183,6 +183,81 @@ async def resolve_tts_config(self) -> Tuple[str, str, dict]: | |
| ) | ||
| return await _resolve_model_config(self.voice_model) | ||
|
|
||
| async def validate_voices(self) -> None: | ||
| """Reject voice_ids that plainly belong to another TTS provider. | ||
|
|
||
| validate_speakers() can only check that a `voice_id` key exists - it | ||
| never sees the model, and nothing else checks either. So a profile | ||
| seeded with OpenAI voices (migration 7) against a Gemini voice_model | ||
| fails only once the first audio clip is requested - after the whole | ||
| transcript has been generated and paid for - with a provider message | ||
| that names the wrong cause (#1238). | ||
|
|
||
| A blank voice always raises: no provider can speak it. Otherwise only | ||
| certain mismatches raise - a voice missing from the model's catalogue | ||
| but present in another provider's. A voice no catalogue knows about is | ||
| logged and allowed through, because esperanto's hard-coded lists go | ||
| stale (see open_notebook.podcasts.voices). Raises ValueError so the | ||
| podcast command treats it as permanent (no retry). | ||
| """ | ||
| from open_notebook.podcasts.voices import ( | ||
| VoiceCatalogueCache, | ||
| find_voice_mismatch, | ||
| format_voice_list, | ||
| ) | ||
|
|
||
| profile_tts: Optional[Tuple[str, str, dict]] = None | ||
| cache = VoiceCatalogueCache() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
| for speaker in self.speakers: | ||
| override = speaker.get("voice_model") | ||
| if override: | ||
| provider, model_name, config = await _resolve_model_config( | ||
| str(override) | ||
| ) | ||
| else: | ||
| if profile_tts is None: | ||
| profile_tts = await self.resolve_tts_config() | ||
| provider, model_name, config = profile_tts | ||
|
|
||
| # 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
| if not voice_id: | ||
| raise ValueError( | ||
| f"Speaker '{speaker.get('name')}' in speaker profile " | ||
| f"'{self.name}' has no voice. Pick one of the voices " | ||
| f"{provider}/{model_name} provides in Settings -> Speaker " | ||
| "Profiles." | ||
| ) | ||
|
|
||
| mismatch = await find_voice_mismatch( | ||
|
cubic-dev-ai[bot] marked this conversation as resolved.
|
||
| provider, model_name, config, voice_id, cache=cache | ||
| ) | ||
| if mismatch is None: | ||
| continue | ||
|
|
||
| valid_voices = format_voice_list(mismatch.known_voices) | ||
| if not mismatch.confident: | ||
| logger.warning( | ||
| f"Speaker '{speaker.get('name')}' in speaker profile " | ||
| f"'{self.name}' uses voice '{voice_id}', which is not in " | ||
| f"{provider}/{model_name}'s known voice list " | ||
| f"({valid_voices}). Generating anyway - the list may be " | ||
| "out of date - but audio generation will fail if the " | ||
| "provider rejects it." | ||
| ) | ||
| continue | ||
|
|
||
| raise ValueError( | ||
| f"Speaker '{speaker.get('name')}' in speaker profile " | ||
| f"'{self.name}' uses voice '{voice_id}', which belongs to " | ||
| f"{', '.join(sorted(mismatch.other_providers))} and is not " | ||
| f"supported by {provider}/{model_name}. Valid voices: " | ||
| f"{valid_voices}. Update the voice in Settings -> Speaker " | ||
| "Profiles, or pick a voice model that provides this voice." | ||
| ) | ||
|
|
||
| @classmethod | ||
| async def get_by_name(cls, name: str) -> Optional["SpeakerProfile"]: | ||
| """Get speaker profile by name""" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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_namesor dialogue in the outline template.Prompt for AI agents