diff --git a/CHANGELOG.md b/CHANGELOG.md index 6252b92e5b..11a01fc8be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,8 +11,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **New "Quiet Green" design foundation** — the first of three PRs applying the visual identity co-designed with the community in Discussion #1202. The design token system lands in `globals.css` (core palette, neutral reading surfaces, ink ramp, hairline borders, content-type hues, evidence/citation classes, context-state colors, a squared 4–6px radius scale and a near-flat shadow scale where popovers own the only real shadow); fonts move from Inter to Bricolage Grotesque (display) + Instrument Sans (UI) + Spline Sans Mono (data) via `next/font`; the shadcn primitives are restyled to the foundation's rules (fern green primary actions, red reserved exclusively for destructive/error, clay for warnings, teal as the AI/system voice, underline tabs, quiet neutral badges, teal focus ring); and hard-coded palette colors across 38 components were swept into the semantic tokens, which re-resolve automatically per theme. A dev-only living styleguide at `/dev/design` (404 in production) renders every token and primitive in both themes and serves as the review reference for the follow-up screen-by-screen PRs. Purely visual — no behavior, navigation or i18n changes - **Screen-by-screen reskin in the "Quiet Green" language** (the second redesign PR): the app shell gets the recomposed tri-hue pebble wordmark (fern/gold/teal — no red left), a fern spine on the active nav item and destination-hued nav icons; the notebook workspace gets display-type titles, panel headers with identity ticks (sources sage / notes gold / chat teal), one-line source-card metadata with the overflow menu at top-right, and de-washed chat bubbles (the AI speaks in teal accents, never washed backgrounds); the notebooks home separates compact recently-viewed rows from active-notebook cards; the sources table gets the library treatment (content-type pebbles, quiet embedded pills, hover that raises the surface); dialogs and the source viewer are flattened (no card-in-card boxes, uniform breathing room, no ⋮/close collision); and Models/Settings/Podcasts/Transformations/Ask-Search get the badge diet, quiet audio-player containers, mono for data and teal AI accents. Still purely visual — no behavior, columns, navigation or i18n changes +### Fixed +- **Podcast episode profiles honor their `language` again.** The app's `prompts/podcast/{outline,transcript}.jinja` shadow podcast-creator's bundled templates (the library resolves `Path.cwd()/prompts/podcast/` before its own resources) and had lost the `{{ language }}` block, so a profile set to `he-IL` produced an English outline and English segment titles — the field looked supported and did nothing. A regression test now fails when a variable the bundled template uses is missing from the app's copy (#1238) +- **A copied prompt placeholder no longer aborts a podcast mid-generation.** Both podcast templates showed the model a fill-in JSON skeleton (`"speaker": "[Actual Speaker Name]"`, a bare `...`) inside a ```json fence while also instructing it not to use fences; models returned the skeleton verbatim, which failed podcast-creator's speaker-name validation and discarded the outline and every segment already generated. The examples are now rendered from the episode's real speaker names (so a verbatim copy is valid output), placeholders and truncation are banned explicitly, and the contradictory fences and second copyable example are gone (#1238) +- **A speaker voice that the TTS model can't use now fails immediately instead of after the whole transcript.** Audio is generated last, so the profiles seeded on install — which carry OpenAI voice names (`nova`, `echo`, `shimmer`, …) — died on a Gemini voice model with `Google API error: Requested entity was not found.`, which reads like a missing model, once the full transcript had already been generated and paid for. Generation now pre-flights each speaker's voice (including per-speaker `voice_model` overrides) against the provider's voice catalogue and names the offending voice and the valid ones. Only voices that demonstrably belong to another provider fail the run; unknown voices are logged and allowed through, since provider catalogues can be out of date (#1238) + ### Changed - Community contribution intake now separates exploration from execution: feature requests, product/design/architecture ideas and contribution proposals start in GitHub Discussions, while Issues are reserved for reproducible bugs and maintainer-approved work items. The Issue chooser routes contributors accordingly, a structured Ideas Discussion form starts from user goals and outcomes, and the contributor/maintainer docs plus PR template now describe the Discussion → Issue → PR graduation path (#1204). +- Podcast generation failures now carry a hint that matches the failure: a placeholder/renamed speaker name, a `voice_id` its TTS model doesn't support, and a response that was truncated rather than swallowed by `` tags each get their own explanation. The GPT-5 extended-thinking note used to be the only hint, so the two most common real failures got either nothing or advice about the wrong provider (#1238) - Release image gate gained a `probe` scenario (`make release-test` runs it as part of `all`): container-level checks that a Python test suite can't cover because they depend on the shipped image's process supervision — `OPEN_NOTEBOOK_WORKER_MAX_TASKS` reaching the in-image worker (the supervisord `sh -c` expansion), and the worker surviving startup with `HTTP_PROXY` set while a user's `NO_PROXY` value is preserved (the internal SurrealDB websocket not being tunneled). Both were manual probes during the v1.14.0 release; they now run automatically. Release-process docs gained the post-tag re-cut sequence and a note on never leaving the version bump uncommitted (v1.14.0 retro) ## [1.14.0] - 2026-07-20 diff --git a/commands/podcast_commands.py b/commands/podcast_commands.py index 95dd3b9336..f38c84c1c4 100644 --- a/commands/podcast_commands.py +++ b/commands/podcast_commands.py @@ -41,6 +41,49 @@ def build_episode_output_dir(podcasts_folder: str = PODCASTS_FOLDER) -> tuple[st return episode_dir_name, output_dir +def explain_generation_failure(error_msg: str) -> Optional[str]: + """Map a podcast-generation failure to an actionable hint, or None. + + Ordered most specific first. The GPT-5 extended-thinking hint used to be + the only one, so the two most common real failures got either nothing + (`Invalid speaker name`) or advice about the wrong provider - a truncated + Gemini response was told to switch to gpt-4o (#1238). + """ + if "Invalid speaker name" in error_msg: + return ( + "The transcript model returned a speaker name that is not in the " + "speaker profile - usually a placeholder copied from the prompt " + 'such as "..." rather than an invented person. Speaker names must ' + "match the profile exactly. Retrying the episode often succeeds, " + "since each attempt is a fresh sample." + ) + + if "Requested entity was not found" in error_msg or ( + "Voice name" in error_msg and "not supported" in error_msg + ): + return ( + "The speaker profile's voice_id is not valid for its TTS model - " + "Google returns 'Requested entity was not found' for an unknown " + "voice, which reads like a missing model. Check the voices in " + "Settings -> Speaker Profiles against the ones your voice model " + "provides (the profiles seeded on install use OpenAI voice names)." + ) + + if "Invalid json output" in error_msg or "Expecting value" in error_msg: + return ( + "The model's response could not be parsed as JSON. Two common " + "causes: (1) the response was truncated - podcast-creator caps a " + "transcript segment at 5000 output tokens unless the episode " + "profile sets max_tokens, which is tight for long segments or " + "token-expensive languages, so raise max_tokens or use fewer and " + "shorter segments; (2) a model using extended thinking (e.g. " + "GPT-5) put all of its output inside tags, leaving nothing " + "to parse - try gpt-4o, gpt-4o-mini or gpt-4-turbo instead." + ) + + return None + + class PodcastGenerationInput(CommandInput): episode_profile: str # Speaker profile record ID or name (the API boundary resolves the @@ -142,6 +185,13 @@ async def generate_podcast_command( f"tts: {tts_provider}/{tts_model_name}" ) + # Pre-flight the speaker voices against the resolved TTS model. Audio + # is generated last, so an unusable voice_id (migration 7 seeds OpenAI + # voice names) otherwise fails only after the full transcript has been + # generated and paid for, with a provider message that names the wrong + # cause: #1238. + await speaker_profile.validate_voices() + # 4. Load all profiles and configure podcast-creator episode_profiles = await repo_query("SELECT * FROM episode_profile") speaker_profiles = await repo_query("SELECT * FROM speaker_profile") @@ -365,11 +415,8 @@ async def generate_podcast_command( logger.exception(e) error_msg = str(e) - if "Invalid json output" in error_msg or "Expecting value" in error_msg: - error_msg += ( - "\n\nNOTE: This error commonly occurs with GPT-5 models that use extended thinking. " - "The model may be putting all output inside tags, leaving nothing to parse. " - "Try using gpt-4o, gpt-4o-mini, or gpt-4-turbo instead in your episode profile." - ) + hint = explain_generation_failure(error_msg) + if hint: + error_msg += f"\n\nNOTE: {hint}" raise RuntimeError(error_msg) from e diff --git a/docs/7-DEVELOPMENT/podcasts.md b/docs/7-DEVELOPMENT/podcasts.md index 67da079e16..e650e93df1 100644 --- a/docs/7-DEVELOPMENT/podcasts.md +++ b/docs/7-DEVELOPMENT/podcasts.md @@ -18,6 +18,22 @@ The legacy string fields (`tts_provider`, `outline_provider`, …) that predated `PodcastEpisode` stores `episode_profile` and `speaker_profile` as **dicts (snapshots)**, not references. Editing a profile never retroactively changes past episodes — that's intentional. Corollary: deleting a profile does not cascade to episodes. +## Prompt templates shadow podcast-creator's + +`prompts/podcast/{outline,transcript}.jinja` are **not** just this app's copies of the library's prompts — they replace them. podcast-creator resolves templates as inline config → `prompts_dir` config → `Path.cwd()/prompts/podcast/.jinja` → its own package resources, and this app configures only profiles, so the working directory wins and the bundled prompts are never read. + +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. 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]`, ``) 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 + +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. + +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 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. The key is `(provider, model_name, digest of the credential config)`: speakers override `voice_model` individually, and two `model` records sharing a provider and name can still point at different accounts or endpoints, whose voice libraries differ. Only successful lookups are memoized — a failure and an absent catalogue are indistinguishable here, so caching the first failure would switch validation off for the rest of the profile after one flaky request — and a failing catalogue is retried at most `MAX_CATALOGUE_ATTEMPTS` times per pass, so a dead endpoint can't charge the 10s timeout once per speaker. + ## Job lifecycle and the retry policy Generation runs as a `generate_podcast_command` job on the surreal-commands worker: diff --git a/open_notebook/podcasts/models.py b/open_notebook/podcasts/models.py index ac628ee195..44973cbac6 100644 --- a/open_notebook/podcasts/models.py +++ b/open_notebook/podcasts/models.py @@ -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() + 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() + 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( + 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""" diff --git a/open_notebook/podcasts/voices.py b/open_notebook/podcasts/voices.py new file mode 100644 index 0000000000..c185a00563 --- /dev/null +++ b/open_notebook/podcasts/voices.py @@ -0,0 +1,227 @@ +"""Voice-id pre-flight against the TTS provider's own voice catalogue. + +Speaker profiles store a free-text `voice_id` per speaker while the live TTS +model is whatever `voice_model` points at, and nothing ties the two together: +`SpeakerProfile.validate_speakers` only checks that the key exists. Migration 7 +seeds three profiles with OpenAI voice names (`nova`, `alloy`, `echo`, +`shimmer`, `ash`), so a deployment whose voice model is Gemini inherits +speakers that can only work with OpenAI. + +Without a pre-flight the mismatch surfaces only when the first audio clip is +generated - after the whole transcript has been generated and paid for - and +the provider error rarely names the voice: Gemini's 3.x TTS preview answers an +unknown voice with `404 Requested entity was not found.`, which reads like a +missing model (see #1238). + +esperanto publishes each provider's catalogue as `available_voices`, so the +check is free for providers that hard-code their voice list (OpenAI, +Google/Gemini, Vertex, xAI, Mistral, Azure). For providers that fetch it over +HTTP (ElevenLabs, OpenRouter, OpenAI-compatible endpoints) the lookup runs in a +worker thread with a timeout, and any failure means "catalogue unknown": an +unavailable catalogue must never block a generation that would otherwise work. + +Those catalogues can also be stale - esperanto's OpenAI list predates `ash`, +`coral` and friends, which the OpenAI API accepts - so "absent from the +catalogue" alone is NOT treated as an error. Only a voice that another +provider's catalogue does list is reported as a mismatch: that is the seeded +OpenAI-voice-on-Gemini case, and it cannot work. Anything else is logged and +allowed through. +""" + +import asyncio +import hashlib +from typing import Dict, NamedTuple, Optional, Set, Tuple + +from loguru import logger + +# The catalogue is a hard-coded dict for most providers; only the HTTP-backed +# ones can block, and a podcast run is not worth delaying for a pre-flight. +CATALOGUE_TIMEOUT_SECONDS = 10.0 + +# How often one validation pass will try a catalogue that keeps failing. Two +# attempts survive a one-off timeout without letting a dead endpoint charge +# CATALOGUE_TIMEOUT_SECONDS once per speaker (see VoiceCatalogueCache.get). +MAX_CATALOGUE_ATTEMPTS = 2 + +# Providers whose esperanto catalogue is a literal dict, so it can be +# enumerated without credentials and without a network call. Used only to +# attribute a voice to the provider it actually belongs to. +# +# Mistral is deliberately absent: its available_voices paginates through +# GET /audio/voices, so listing it here sent a request (and a 401) to a provider +# the deployment may not even use, every time a voice couldn't be placed. +STATIC_CATALOGUE_PROVIDERS = ("openai", "google", "vertex", "xai") + +# Any non-empty string satisfies the providers' constructor key check; the +# catalogue is a literal dict, so no request is ever made with it. +_CATALOGUE_ONLY_CONFIG = {"api_key": "unused-catalogue-lookup"} + + +class VoiceMismatch(NamedTuple): + """A speaker voice_id the resolved TTS model does not list.""" + + voice_id: str + provider: str + model_name: str + known_voices: Set[str] + # Providers whose catalogue DOES list this voice. Non-empty means the + # mismatch is certain (the voice belongs to someone else), which is the + # case worth failing the run for. + other_providers: Set[str] + + @property + def confident(self) -> bool: + return bool(self.other_providers) + + +async def get_known_voice_ids( + provider: str, model_name: Optional[str] = None, config: Optional[dict] = None +) -> Optional[Set[str]]: + """Return the lower-cased voice ids `provider`/`model_name` accepts. + + Returns None when the catalogue can't be determined (provider unknown to + esperanto, credentials missing, HTTP lookup failing or timing out, empty + catalogue). Callers must treat None as "skip validation", never as "no + voice is valid". + + Ids are lower-cased because providers accept them case-insensitively: + Gemini lists `achernar` and accepts `Kore` (its documented capitalisation) + for the same voice. + """ + + def _lookup() -> Set[str]: + from esperanto import AIFactory + + model = AIFactory.create_text_to_speech( + provider, model_name, config=config or {} + ) + catalogue = model.available_voices or {} + ids = {str(key).lower() for key in catalogue} + ids.update( + str(voice.id).lower() + for voice in catalogue.values() + if getattr(voice, "id", None) + ) + return ids + + try: + voice_ids = await asyncio.wait_for( + asyncio.to_thread(_lookup), timeout=CATALOGUE_TIMEOUT_SECONDS + ) + except Exception as e: + logger.debug( + f"Skipping voice validation for {provider}/{model_name}: " + f"voice catalogue unavailable ({e})" + ) + return None + + return voice_ids or None + + +def config_fingerprint(config: Optional[dict]) -> str: + """Digest the credential config so two endpoints never share a catalogue. + + Hashed rather than stored verbatim: the config carries API keys, and this + value ends up in cache keys that can surface in reprs and tracebacks. + """ + if not config: + return "" + items = sorted((str(key), repr(value)) for key, value in config.items()) + return hashlib.sha256(repr(items).encode()).hexdigest()[:16] + + +class VoiceCatalogueCache: + """Memoizes catalogues for one validation pass. + + A catalogue can't change between the speakers of a single profile, but the + HTTP-backed providers charge a request (up to CATALOGUE_TIMEOUT_SECONDS) for + every lookup, and the cross-provider attribution below enumerates five more + catalogues per mismatch. Without this, a 4-speaker ElevenLabs profile made + the same request four times before generation could start. + + The key includes the credential config, not just provider and model name: + speakers can override `voice_model` individually, and two Model records + sharing a provider and model name may still point at different endpoints or + accounts (`base_url`, `endpoint_tts`, a second API key). An ElevenLabs voice + library is per-account, so reusing one account's catalogue for another would + validate a speaker against voices it cannot use. + + Deliberately per-pass rather than process-wide: a fetched catalogue reflects + the credentials in use, and a run should see voices added since the last one. + """ + + def __init__(self) -> None: + self._catalogues: Dict[Tuple[str, Optional[str], str], Set[str]] = {} + self._attempts: Dict[Tuple[str, Optional[str], str], int] = {} + + async def get( + self, provider: str, model_name: Optional[str] = None, config: Optional[dict] = None + ) -> Optional[Set[str]]: + """Return the catalogue, fetching it at most MAX_CATALOGUE_ATTEMPTS times. + + Only successes are memoized. A failure is indistinguishable from an + absent catalogue here - get_known_voice_ids() returns None for a timeout + and for a provider esperanto doesn't know alike - so remembering the + first failure would silently disable validation for every later speaker + after one flaky request. Retrying without a bound would instead pay + CATALOGUE_TIMEOUT_SECONDS per speaker whenever the endpoint is simply + down, which is what this cache exists to avoid. + """ + key = (provider, model_name, config_fingerprint(config)) + cached = self._catalogues.get(key) + if cached is not None: + return cached + + if self._attempts.get(key, 0) >= MAX_CATALOGUE_ATTEMPTS: + return None + self._attempts[key] = self._attempts.get(key, 0) + 1 + + voice_ids = await get_known_voice_ids(provider, model_name, config) + if voice_ids: + self._catalogues[key] = voice_ids + return voice_ids + + +async def find_voice_mismatch( + provider: str, + model_name: str, + config: Optional[dict], + voice_id: str, + cache: Optional[VoiceCatalogueCache] = None, +) -> Optional[VoiceMismatch]: + """Report a voice the model does not list, or None when it looks usable. + + Pass a shared `cache` when checking several speakers so each catalogue is + fetched once for the whole profile. + """ + cache = cache or VoiceCatalogueCache() + + known_voices = await cache.get(provider, model_name, config) + if not known_voices: + return None + if voice_id.lower() in known_voices: + return None + + other_providers = set() + for other in STATIC_CATALOGUE_PROVIDERS: + if other == provider: + continue + other_voices = await cache.get(other, config=dict(_CATALOGUE_ONLY_CONFIG)) + if other_voices and voice_id.lower() in other_voices: + other_providers.add(other) + + return VoiceMismatch( + voice_id=voice_id, + provider=provider, + model_name=model_name, + known_voices=known_voices, + other_providers=other_providers, + ) + + +def format_voice_list(voice_ids: Set[str], limit: int = 25) -> str: + """Render a catalogue for an error message, capped so it stays readable.""" + ordered = sorted(voice_ids) + if len(ordered) <= limit: + return ", ".join(ordered) + return ", ".join(ordered[:limit]) + f", … ({len(ordered)} total)" diff --git a/prompts/podcast/outline.jinja b/prompts/podcast/outline.jinja index 0ae13a7fee..941371c893 100644 --- a/prompts/podcast/outline.jinja +++ b/prompts/podcast/outline.jinja @@ -25,7 +25,11 @@ The podcast will feature the following speakers: Personality: {{ speaker.personality }} {% endfor %} +{% if language %} +IMPORTANT LANGUAGE INSTRUCTION: You MUST generate ALL content in {{ language }}. This includes segment names, descriptions, and all text in your response. Do not use English unless the content itself contains English terms. The entire output must be written in {{ language }}. + +{% endif %} Please create an outline based on this briefing. Your outline should consist of {{ num_segments }} main segments for the podcast episode, along with a description of each segment. Follow these guidelines: 1. Read the briefing carefully and identify the main topics and themes. @@ -37,30 +41,17 @@ Please create an outline based on this briefing. Your outline should consist of 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. 8. Include an introduction segment at the beginning and a conclusion or wrap-up segment at the end. -Format your outline using the following structure: +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: -```json -{ - "segments": [ - { - "name": "[Segment Name]", - "description": "[Description of the segment content]", - "size": "short" - }, - { - "name": "[Segment Name]", - "description": "[Description of the segment content]", - "size": "medium" - }, - { - "name": "[Segment Name]", - "description": "[Description of the segment content]", - "size": "long" - }, - ... - ] -} -``` +{"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". + - The sample values above show the structure only - write segments drawn from the briefing instead of reusing them. + - Never emit placeholder or elided content: no "..." or "…", no "[like this]", no "", no "TODO", no empty strings, no trailing commas. + - Write out all {{ num_segments }} segments in full; never shorten or truncate the list. +{% if language %} + - The sample values above are in English only to show the structure; every segment name and description you write must be in {{ language }}. +{% endif %} Formatting instructions: {{ format_instructions}} @@ -76,8 +67,6 @@ IMPORTANT OUTPUT FORMAT: - If you use extended thinking with tags, put ALL your reasoning inside tags - Put the final JSON output OUTSIDE and AFTER any tags - Do NOT wrap the JSON in ```json code blocks - return the raw JSON object only -- Example correct format: - Let me analyze the briefing... - {"segments": [...]} +- Correct format: any reasoning inside tags, then the JSON object described above, and nothing else Please provide your outline now, following the format and guidelines provided above. diff --git a/prompts/podcast/transcript.jinja b/prompts/podcast/transcript.jinja index 974b307391..c016e7f121 100644 --- a/prompts/podcast/transcript.jinja +++ b/prompts/podcast/transcript.jinja @@ -27,7 +27,11 @@ The podcast features the following speakers: Personality: {{ speaker.personality }} {% endfor %} +{% if language %} +IMPORTANT LANGUAGE INSTRUCTION: You MUST generate ALL dialogue and content in {{ language }}. Every speaker's dialogue must be written entirely in {{ language }}. Do not use English unless quoting specific English terms. The entire transcript must be in {{ language }}. + +{% endif %} Next, examine the outline produced by our director: {{ outline }} @@ -74,17 +78,21 @@ Follow these format requirements strictly: {% endif %} -```json -{ - "transcript": [ - { - "speaker": "[Actual Speaker Name]", - "dialogue": "[Speaker's dialogue based on their personality and expertise]" - }, - ... - ] -} -``` +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."}]} +{% else %} +{"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."}]} +{% endif %} + + - Every "speaker" value must be copied character-for-character from this list: {{ speaker_names|join(', ') }} + - 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. + - Never emit placeholder or elided content: no "..." or "…", no "[like this]", no "", no "TODO", no empty strings, no trailing commas. + - Never shorten or truncate the list. Write out every entry in full. +{% if language %} + - The sample dialogue above is in English only to show the structure; every "dialogue" value you write must be in {{ language }}. +{% endif %} Formatting instructions: {{ format_instructions}} @@ -119,9 +127,7 @@ IMPORTANT OUTPUT FORMAT: - If you use extended thinking with tags, put ALL your reasoning inside tags - Put the final JSON output OUTSIDE and AFTER any tags - Do NOT wrap the JSON in ```json code blocks - return the raw JSON object only -- Example correct format: - Let me plan the dialogue... - {"transcript": [...]} +- Correct format: any reasoning inside tags, then the JSON object described above, and nothing else When you're ready, provide the transcript. {% if speakers|length == 1 %} diff --git a/tests/test_podcast_error_hints.py b/tests/test_podcast_error_hints.py new file mode 100644 index 0000000000..264d127586 --- /dev/null +++ b/tests/test_podcast_error_hints.py @@ -0,0 +1,57 @@ +"""Hints attached to podcast generation failures (#1238). + +The GPT-5 extended-thinking hint used to be the only one, keyed on +`Invalid json output` / `Expecting value`. The two most common real failures +therefore got either nothing (`Invalid speaker name`) or advice about the wrong +provider - a truncated Gemini response was told to switch to gpt-4o. +""" + +import pytest + +from commands.podcast_commands import explain_generation_failure + + +class TestExplainGenerationFailure: + def test_placeholder_speaker_name_is_explained(self): + hint = explain_generation_failure( + "Failed to parse ValidatedTranscript from completion " + '{"transcript": [{"speaker": "...", "dialogue": "..."}]}. Got: ' + "1 validation error for ValidatedTranscript transcript.0.speaker " + "Value error, Invalid speaker name '...'. Must be one of: " + "Marcus Thompson, Elena Vasquez" + ) + assert hint is not None + assert "speaker" in hint + assert "gpt-4o" not in hint + + def test_google_bad_voice_points_at_the_speaker_profile(self): + hint = explain_generation_failure( + "Google API error: Requested entity was not found." + ) + assert hint is not None + assert "voice_id" in hint + assert "Speaker Profiles" in hint + + def test_unsupported_voice_name_points_at_the_speaker_profile(self): + hint = explain_generation_failure( + "Google API error: Voice name echo is not supported. Allowed voice " + "names are: achernar, achird, algenib" + ) + assert hint is not None + assert "voice_id" in hint + + @pytest.mark.parametrize( + "message", + [ + "Invalid json output: {'transcript': [{'speaker'", + "Expecting value: line 1 column 1 (char 0)", + ], + ) + def test_unparseable_output_mentions_truncation_and_thinking(self, message): + hint = explain_generation_failure(message) + assert hint is not None + assert "max_tokens" in hint + assert "" in hint + + def test_unrecognised_failure_gets_no_hint(self): + assert explain_generation_failure("Connection reset by peer") is None diff --git a/tests/test_podcast_prompt_templates.py b/tests/test_podcast_prompt_templates.py new file mode 100644 index 0000000000..8eaabcfb45 --- /dev/null +++ b/tests/test_podcast_prompt_templates.py @@ -0,0 +1,268 @@ +"""Regression tests for the app's podcast prompt templates (#1238). + +Two defects, neither visible without rendering the templates: + +1. The templates showed the model a fill-in JSON skeleton + (`"speaker": "[Actual Speaker Name]"` plus a bare `...`) inside a ```json + fence, while also instructing it NOT to use fences. Gemini returned the + skeleton verbatim - `{"transcript": [{"speaker": "...", "dialogue": "..."}]}` + - which failed podcast-creator's speaker-name validation and aborted the + episode mid-run, discarding the segments already generated. + +2. These templates shadow podcast-creator's bundled ones (the library resolves + `Path.cwd()/prompts/podcast/.jinja` before its own package resources) + and never referenced `{{ language }}`, so `episode_profile.language` looked + supported and did nothing: Hebrew sources produced an English outline. + +Variable names follow podcast_creator.nodes, which renders these templates. +Note the outline template receives `speakers` but NOT `speaker_names`. +""" + +import json +import re +from pathlib import Path + +import pytest +from jinja2 import Environment, FileSystemLoader + +PROMPTS_DIR = Path(__file__).parent.parent / "prompts" / "podcast" + +SPEAKERS = [ + { + "name": "Marcus Thompson", + "backstory": "Former consultant", + "personality": "Strategic", + }, + { + "name": "Elena Vasquez", + "backstory": "Serial entrepreneur", + "personality": "Pragmatic", + }, +] + +# Strings that must never reach the model: each one is copyable as content. +# Angle-bracket descriptions count too - a model that copies +# "" into a dialogue value sends +# that straight to the TTS engine, and the templates' own rules ban placeholders. +COPYABLE_SKELETONS = ( + "[Actual Speaker Name]", + "[Speaker's dialogue based on their personality and expertise]", + "[Segment Name]", + "[Description of the segment content]", + '{"transcript": [...]}', + '{"segments": [...]}', + "", + "", + "", + "", + "", +) + + +def render(template_name: str, **data) -> str: + env = Environment(loader=FileSystemLoader(str(PROMPTS_DIR))) + return env.get_template(f"{template_name}.jinja").render(**data) + + +def render_transcript(speakers=None, speaker_names=None, **overrides) -> str: + speakers = SPEAKERS if speakers is None else speakers + data = { + "briefing": "A briefing", + "context": "Some content", + "speakers": speakers, + "speaker_names": [s["name"] for s in speakers] + if speaker_names is None + else speaker_names, + "outline": "An outline", + "segment": "A segment", + "turns": 6, + "is_final": False, + "transcript": [], + "format_instructions": "Return JSON", + } + data.update(overrides) + return render("transcript", **data) + + +def render_outline(**overrides) -> str: + data = { + "briefing": "A briefing", + "context": "Some content", + "speakers": SPEAKERS, + "num_segments": 6, + "format_instructions": "Return JSON", + } + data.update(overrides) + return render("outline", **data) + + +def example_object(rendered: str, root_key: str) -> dict: + """Parse the JSON example the prompt shows the model. + + The example is the contract the model imitates, so it has to be valid JSON + in its own right - a speaker name carrying a quote or a backslash would + otherwise hand the model a broken example to copy. + """ + prefix = '{"' + root_key + '":' + for line in rendered.splitlines(): + if line.startswith(prefix): + return json.loads(line) + raise AssertionError(f"no {root_key} example found in the rendered prompt") + + +class TestExampleIsValidAndComplete: + """Whatever the model copies from the example must be usable output.""" + + def test_transcript_example_parses_and_names_the_speakers(self): + example = example_object(render_transcript(), "transcript") + assert [entry["speaker"] for entry in example["transcript"]] == [ + "Marcus Thompson", + "Elena Vasquez", + ] + + def test_transcript_example_survives_json_special_characters(self): + r"""A name like Dr. "Alex" Chen\ must be escaped, not interpolated raw.""" + speakers = [ + {"name": 'Dr. "Alex" Chen\\', "backstory": "b", "personality": "p"}, + {"name": "Jamie\tRodriguez", "backstory": "b", "personality": "p"}, + ] + example = example_object(render_transcript(speakers=speakers), "transcript") + assert [entry["speaker"] for entry in example["transcript"]] == [ + 'Dr. "Alex" Chen\\', + "Jamie\tRodriguez", + ] + + def test_transcript_example_dialogue_is_speakable(self): + """Dialogue goes straight to TTS, so the example must not contain a + description of what to write - a copied one would be read aloud.""" + 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): + example = example_object(render_outline(), "segments") + assert example["segments"] + for segment in example["segments"]: + assert segment["size"] in {"short", "medium", "long"} + assert "<" not in segment["name"] + assert "<" not in segment["description"] + + +class TestNoCopyableSkeletons: + """Whatever the model copies from the prompt must be valid output.""" + + @pytest.mark.parametrize("skeleton", COPYABLE_SKELETONS) + def test_transcript_has_no_placeholder_skeleton(self, skeleton): + assert skeleton not in render_transcript() + + @pytest.mark.parametrize("skeleton", COPYABLE_SKELETONS) + def test_outline_has_no_placeholder_skeleton(self, skeleton): + assert skeleton not in render_outline() + + def test_transcript_example_uses_real_speaker_names(self): + rendered = render_transcript() + assert '{"transcript": [{"speaker": "Marcus Thompson"' in rendered + assert '"speaker": "Elena Vasquez"' in rendered + + def test_solo_transcript_example_uses_the_only_speaker(self): + rendered = render_transcript(speakers=[SPEAKERS[0]]) + assert '{"transcript": [{"speaker": "Marcus Thompson"' in rendered + assert "Elena Vasquez" not in rendered + + def test_second_speaker_is_guarded_against_a_short_name_list(self): + """An out-of-range speaker_names[1] would render as an empty string + under Jinja's default undefined, putting an invalid example in the + prompt - exactly the failure this template is meant to prevent.""" + rendered = render_transcript( + speakers=SPEAKERS, speaker_names=["Marcus Thompson"] + ) + assert '"speaker": ""' not in rendered + assert rendered.count('"speaker": "Marcus Thompson"') == 2 + + def test_transcript_bans_placeholder_content(self): + rendered = render_transcript() + assert "Never emit placeholder or elided content" in rendered + assert "Never shorten or truncate the list" in rendered + + def test_outline_bans_placeholder_content(self): + rendered = render_outline() + assert "Never emit placeholder or elided content" in rendered + assert "never shorten or truncate the list" in rendered + + @pytest.mark.parametrize( + "renderer", [render_transcript, render_outline], ids=["transcript", "outline"] + ) + def test_no_code_fence_contradicts_the_no_fence_instruction(self, renderer): + """The only ``` left may be inside the "no code blocks" rule itself: + an example wrapped in a fence contradicts that rule and pushes the + model toward pattern matching over instruction following.""" + rendered = renderer() + no_fence_rule = "Do NOT wrap the JSON in ```json code blocks" + assert no_fence_rule in rendered + assert "```" not in rendered.replace(no_fence_rule, "") + + +class TestLanguageInstruction: + """episode_profile.language must actually reach the model (#1238).""" + + def test_transcript_includes_the_language_instruction(self): + rendered = render_transcript(language="Hebrew") + assert "IMPORTANT LANGUAGE INSTRUCTION" in rendered + assert rendered.count("Hebrew") >= 3 + + def test_outline_includes_the_language_instruction(self): + rendered = render_outline(language="Hebrew") + assert "IMPORTANT LANGUAGE INSTRUCTION" in rendered + assert "segment names, descriptions" in rendered + + @pytest.mark.parametrize( + "renderer", [render_transcript, render_outline], ids=["transcript", "outline"] + ) + def test_english_sample_is_flagged_as_english(self, renderer): + """The example is hard-coded English; say so, or it nudges the model + back toward English for a non-English episode.""" + assert "in English only to show the structure" in renderer(language="Hebrew") + + @pytest.mark.parametrize( + "renderer", [render_transcript, render_outline], ids=["transcript", "outline"] + ) + def test_no_language_instruction_without_a_language(self, renderer): + assert "IMPORTANT LANGUAGE INSTRUCTION" not in renderer() + assert "IMPORTANT LANGUAGE INSTRUCTION" not in renderer(language=None) + + +class TestNoDriftFromBundledTemplates: + """The app's copies shadow podcast-creator's, so library prompt work is + invisible here. The language block was lost exactly this way. Fail when a + variable the bundled template uses is missing from the app's copy.""" + + @staticmethod + def _variables(path: Path) -> set: + text = path.read_text() + used = set(re.findall(r"\{\{-?\s*([a-zA-Z_][a-zA-Z0-9_]*)", text)) + used |= set( + re.findall(r"\{%-?\s*(?:if|elif)\s+([a-zA-Z_][a-zA-Z0-9_]*)", text) + ) + loop_locals = set(re.findall(r"\{%-?\s*for\s+([a-zA-Z_][a-zA-Z0-9_]*)", text)) + return used - loop_locals + + @pytest.mark.parametrize("template", ["transcript", "outline"]) + def test_app_template_uses_every_bundled_variable(self, template): + import podcast_creator + + bundled = ( + Path(podcast_creator.__file__).parent + / "resources" + / "prompts" + / "podcast" + / f"{template}.jinja" + ) + missing = self._variables(bundled) - self._variables( + PROMPTS_DIR / f"{template}.jinja" + ) + assert not missing, ( + f"prompts/podcast/{template}.jinja shadows podcast-creator's copy " + f"but ignores {sorted(missing)}. Either use them or delete the " + "app template so the library's is used." + ) diff --git a/tests/test_podcast_voice_validation.py b/tests/test_podcast_voice_validation.py new file mode 100644 index 0000000000..cceb39f057 --- /dev/null +++ b/tests/test_podcast_voice_validation.py @@ -0,0 +1,354 @@ +"""Voice-id pre-flight for podcast speaker profiles (#1238). + +Migration 7 seeds speaker profiles with OpenAI voice names (`nova`, `alloy`, +`echo`, `shimmer`, `ash`) while the live TTS model is whatever `voice_model` +points at. With a Gemini voice model every audio clip fails - after the whole +transcript has been generated and paid for - with `Google API error: Requested +entity was not found.`, which reads like a missing model rather than a bad +voice. + +`SpeakerProfile.validate_voices()` runs before generation instead, but must not +be trigger-happy: esperanto's hard-coded catalogues go stale (its OpenAI list +predates `ash`, which the API accepts), so only a voice another provider's +catalogue claims is treated as an error. + +No network and no credentials: the providers exercised here return literal +dicts from `available_voices`. +""" + +from unittest.mock import AsyncMock, patch + +import pytest + +from open_notebook.podcasts.models import SpeakerProfile +from open_notebook.podcasts.voices import ( + VoiceCatalogueCache, + find_voice_mismatch, + format_voice_list, + get_known_voice_ids, +) + +GEMINI_TTS = ("google", "gemini-3.1-flash-tts-preview", {"api_key": "unused"}) + + +def make_profile(speakers, name="business_panel"): + return SpeakerProfile( + name=name, + voice_model="model:tts", + speakers=[ + {"backstory": "b", "personality": "p", **speaker} for speaker in speakers + ], + ) + + +class TestVoiceCatalogue: + @pytest.mark.asyncio + async def test_gemini_catalogue_is_available_offline_and_lowercased(self): + voices = await get_known_voice_ids(*GEMINI_TTS) + assert voices is not None + # Gemini documents its voices capitalised (Kore); esperanto lists them + # lower-cased. Both must match, since the API accepts either. + assert "kore" in voices + assert "echo" not in voices + + @pytest.mark.asyncio + async def test_unknown_provider_skips_validation(self): + assert await get_known_voice_ids("not-a-provider", "x", {}) is None + + @pytest.mark.asyncio + async def test_openai_voice_is_attributed_to_openai(self): + mismatch = await find_voice_mismatch(*GEMINI_TTS, "echo") + assert mismatch is not None + assert mismatch.confident + assert mismatch.other_providers == {"openai"} + + @pytest.mark.asyncio + async def test_unattributable_voice_is_not_confident(self): + """`ash` is a real OpenAI voice missing from esperanto's list; nothing + can be concluded from its absence, so the run must not be blocked.""" + mismatch = await find_voice_mismatch(*GEMINI_TTS, "ash") + assert mismatch is not None + assert not mismatch.confident + + @pytest.mark.asyncio + async def test_valid_voice_reports_no_mismatch(self): + assert await find_voice_mismatch(*GEMINI_TTS, "Kore") is None + + @pytest.mark.asyncio + async def test_cache_keeps_endpoints_apart(self): + """Two Model records can share a provider and model name while pointing + at different accounts or endpoints (a second API key, another + `base_url`/`endpoint_tts`). An ElevenLabs voice library is per-account, + so sharing one catalogue across them validates a speaker against voices + it cannot use.""" + + async def per_account(provider, model_name=None, config=None): + return {"voice-" + (config or {}).get("api_key", "none")} + + cache = VoiceCatalogueCache() + with patch("open_notebook.podcasts.voices.get_known_voice_ids", per_account): + account_a = await cache.get( + "elevenlabs", "eleven_turbo_v2", {"api_key": "acct-a"} + ) + account_b = await cache.get( + "elevenlabs", "eleven_turbo_v2", {"api_key": "acct-b"} + ) + repeat_a = await cache.get( + "elevenlabs", "eleven_turbo_v2", {"api_key": "acct-a"} + ) + + assert account_a == {"voice-acct-a"} + assert account_b == {"voice-acct-b"} + # Same config still hits the memo rather than the provider. + assert repeat_a is account_a + + def test_voice_list_is_capped(self): + formatted = format_voice_list({f"v{i:02d}" for i in range(30)}, limit=3) + assert formatted == "v00, v01, v02, … (30 total)" + + +class TestValidateVoices: + @pytest.mark.asyncio + async def test_seeded_openai_voice_on_gemini_fails_immediately(self): + profile = make_profile( + [ + {"name": "Marcus Thompson", "voice_id": "echo"}, + {"name": "Elena Vasquez", "voice_id": "shimmer"}, + ] + ) + with patch.object( + SpeakerProfile, "resolve_tts_config", AsyncMock(return_value=GEMINI_TTS) + ): + with pytest.raises(ValueError) as exc_info: + await profile.validate_voices() + + message = str(exc_info.value) + # Names the speaker, the voice, the model and the valid alternatives - + # everything "Requested entity was not found" left the operator to guess. + assert "Marcus Thompson" in message + assert "'echo'" in message + assert "google/gemini-3.1-flash-tts-preview" in message + assert "kore" in message + assert "Speaker Profiles" in message + + @pytest.mark.asyncio + async def test_gemini_voices_pass_case_insensitively(self): + profile = make_profile([{"name": "Speaker", "voice_id": "Kore"}]) + with patch.object( + SpeakerProfile, "resolve_tts_config", AsyncMock(return_value=GEMINI_TTS) + ): + await profile.validate_voices() + + @pytest.mark.asyncio + async def test_unattributable_voice_warns_instead_of_raising(self): + """`ash` is a real OpenAI voice absent from esperanto's list, so the run + proceeds - but it has to say so, otherwise the eventual provider error + arrives with nothing in the log pointing at the voice. (loguru doesn't + propagate to caplog, hence patching the module's logger.)""" + profile = make_profile([{"name": "Johny Bing", "voice_id": "ash"}]) + with patch.object( + SpeakerProfile, "resolve_tts_config", AsyncMock(return_value=GEMINI_TTS) + ): + with patch("open_notebook.podcasts.models.logger") as mock_logger: + await profile.validate_voices() + + mock_logger.warning.assert_called_once() + warning = mock_logger.warning.call_args.args[0] + assert "Johny Bing" in warning + assert "'ash'" in warning + assert "google/gemini-3.1-flash-tts-preview" in warning + + @pytest.mark.asyncio + @pytest.mark.parametrize("blank", ["", " ", None]) + async def test_blank_voice_fails_immediately(self, blank): + """A blank voice matches no catalogue and belongs to no provider, so it + would otherwise fall into the "can't attribute it" branch and only warn - + then fail during audio generation like the bug this pre-flight fixes.""" + profile = make_profile([{"name": "Speaker", "voice_id": blank}]) + with patch.object( + SpeakerProfile, "resolve_tts_config", AsyncMock(return_value=GEMINI_TTS) + ): + with pytest.raises(ValueError, match="has no voice"): + await profile.validate_voices() + + @pytest.mark.asyncio + async def test_catalogue_is_fetched_once_per_profile(self): + """Catalogues can't change between speakers of one profile, and an + HTTP-backed provider charges a request (up to 10s) per lookup.""" + profile = make_profile( + [ + {"name": "A", "voice_id": "voice-a"}, + {"name": "B", "voice_id": "voice-b"}, + {"name": "C", "voice_id": "voice-c"}, + ] + ) + catalogue = AsyncMock(return_value={"voice-a", "voice-b", "voice-c"}) + with patch.object( + SpeakerProfile, + "resolve_tts_config", + AsyncMock(return_value=("elevenlabs", "eleven_turbo_v2", {})), + ): + with patch( + "open_notebook.podcasts.voices.get_known_voice_ids", catalogue + ): + await profile.validate_voices() + + assert catalogue.await_count == 1 + + @pytest.mark.asyncio + async def test_speakers_on_different_accounts_are_checked_separately(self): + """Two speakers overriding voice_model to the same provider and model + name but different credentials must each be checked against their own + account's voices - not the first one's.""" + profile = make_profile( + [ + { + "name": "A", + "voice_id": "voice-acct-a", + "voice_model": "model:account_a", + }, + { + "name": "B", + "voice_id": "voice-acct-b", + "voice_model": "model:account_b", + }, + ] + ) + configs = { + "model:account_a": ("elevenlabs", "eleven_turbo_v2", {"api_key": "acct-a"}), + "model:account_b": ("elevenlabs", "eleven_turbo_v2", {"api_key": "acct-b"}), + } + + async def per_account(provider, model_name=None, config=None): + return {"voice-" + (config or {}).get("api_key", "none")} + + with patch( + "open_notebook.podcasts.models._resolve_model_config", + AsyncMock(side_effect=lambda model_id: configs[model_id]), + ): + with patch( + "open_notebook.podcasts.voices.get_known_voice_ids", per_account + ): + with patch("open_notebook.podcasts.models.logger") as mock_logger: + await profile.validate_voices() + + # Both voices are valid for their own account: nothing to warn about. + mock_logger.warning.assert_not_called() + + @pytest.mark.asyncio + async def test_one_off_lookup_failure_does_not_disable_the_rest_of_the_pass(self): + """A timeout and an unknown provider both surface as None, so caching + the first failure would let one flaky request switch validation off for + every later speaker.""" + from open_notebook.podcasts import voices as voices_module + + real_lookup = voices_module.get_known_voice_ids + attempts = {"n": 0} + + async def flaky(provider, model_name=None, config=None): + attempts["n"] += 1 + if attempts["n"] == 1: + return None + return await real_lookup(provider, model_name, config) + + profile = make_profile( + [{"name": "A", "voice_id": "echo"}, {"name": "B", "voice_id": "echo"}] + ) + with patch.object( + SpeakerProfile, "resolve_tts_config", AsyncMock(return_value=GEMINI_TTS) + ): + with patch("open_notebook.podcasts.voices.get_known_voice_ids", flaky): + with pytest.raises(ValueError) as exc_info: + await profile.validate_voices() + + # Speaker A was skipped, but B's lookup succeeded and caught the voice. + assert "Speaker 'B'" in str(exc_info.value) + + @pytest.mark.asyncio + async def test_a_dead_catalogue_is_not_retried_once_per_speaker(self): + """The retry above must stay bounded: an endpoint that is simply down + would otherwise cost CATALOGUE_TIMEOUT_SECONDS for every speaker.""" + from open_notebook.podcasts.voices import MAX_CATALOGUE_ATTEMPTS + + profile = make_profile( + [{"name": name, "voice_id": "voice-x"} for name in "ABCD"] + ) + catalogue = AsyncMock(return_value=None) + with patch.object( + SpeakerProfile, + "resolve_tts_config", + AsyncMock(return_value=("elevenlabs", "eleven_turbo_v2", {})), + ): + with patch( + "open_notebook.podcasts.voices.get_known_voice_ids", catalogue + ): + await profile.validate_voices() + + assert catalogue.await_count == MAX_CATALOGUE_ATTEMPTS + + @pytest.mark.asyncio + async def test_attribution_catalogues_are_not_re_enumerated_per_speaker(self): + """Attributing an unknown voice enumerates the static catalogues; two + speakers with the same problem must not pay for them twice. A catalogue + that fails to resolve (Vertex without a project id) is the one thing + retried, and only up to MAX_CATALOGUE_ATTEMPTS.""" + from collections import Counter + + from open_notebook.podcasts import voices as voices_module + from open_notebook.podcasts.voices import MAX_CATALOGUE_ATTEMPTS + + real_lookup = voices_module.get_known_voice_ids + lookups = [] + + async def spy(provider, model_name=None, config=None): + lookups.append((provider, model_name)) + return await real_lookup(provider, model_name, config) + + profile = make_profile( + [{"name": "A", "voice_id": "ash"}, {"name": "B", "voice_id": "ash"}] + ) + with patch.object( + SpeakerProfile, "resolve_tts_config", AsyncMock(return_value=GEMINI_TTS) + ): + with patch("open_notebook.podcasts.voices.get_known_voice_ids", spy): + await profile.validate_voices() + + counts = Counter(lookups) + assert counts + # 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 + async def test_unavailable_catalogue_never_blocks_generation(self): + profile = make_profile([{"name": "Speaker", "voice_id": "whatever"}]) + with patch.object( + SpeakerProfile, + "resolve_tts_config", + AsyncMock(return_value=("elevenlabs", "eleven_turbo_v2", {})), + ): + with patch( + "open_notebook.podcasts.voices.get_known_voice_ids", + AsyncMock(return_value=None), + ): + await profile.validate_voices() + + @pytest.mark.asyncio + async def test_per_speaker_voice_model_override_is_used(self): + """A speaker may override the profile's voice model, so the voice has + to be checked against the override, not the profile default.""" + profile = make_profile( + [{"name": "Speaker", "voice_id": "echo", "voice_model": "model:openai_tts"}] + ) + with patch.object( + SpeakerProfile, "resolve_tts_config", AsyncMock(return_value=GEMINI_TTS) + ) as profile_config: + with patch( + "open_notebook.podcasts.models._resolve_model_config", + AsyncMock(return_value=("openai", "gpt-4o-mini-tts", {"api_key": "x"})), + ) as override_config: + await profile.validate_voices() + + override_config.assert_awaited_once_with("model:openai_tts") + profile_config.assert_not_awaited()