fix(podcasts): make the failure hints reachable and scope the not-found one (#1238)

Review follow-ups on this PR.

- The hint mapper never ran for the failures it was written for. `except
  ValueError: raise` sits ahead of the handler that attaches hints, and both
  LangChain's OutputParserException and json.JSONDecodeError are ValueError
  subclasses - so a placeholder speaker name, `Invalid json output` and a
  truncated response all left untouched. That includes the pre-existing GPT-5
  extended-thinking note, which has been dead for exactly the case it
  describes. The ValueError branch now attaches the hint and re-raises a
  ValueError, keeping the command layer's permanent-failure contract intact.
- `Requested entity was not found` no longer claims the voice is at fault.
  Google returns it for any missing resource, including a model id on the
  outline or transcript call, so the hint now names both candidates and says
  which is likelier when audio had already started. The voice-specific hint
  stays keyed on `Voice name ... is not supported`, which does name the voice.
- A solo speaker profile renders a one-entry example; the prose called it a
  two-entry excerpt and told the model to return more than "the two shown".
  Both now follow speakers|length.
- The template drift check counted Jinja's `not` as a variable, so
  `{% if not language %}` would fail the check the moment one template negated
  a test the other didn't.

Author: Yehuda-Ness

commands/podcast_commands.py

@@ -58,15 +58,24 @@ def explain_generation_failure(error_msg: str) -> Optional[str]:
             "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
-    ):
+    if "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)."
+            "The speaker profile's voice_id is not valid for its TTS 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 "Requested entity was not found" in error_msg:
+        return (
+            "Google returns this for any resource it cannot find, without "
+            "naming which one. Two candidates, likeliest first: a speaker "
+            "profile voice_id that its TTS model doesn't provide (the profiles "
+            "seeded on install use OpenAI voice names, which Gemini voice "
+            "models reject with exactly this message), or a model id in the "
+            "episode profile that doesn't exist for its provider. If the "
+            "transcript finished and the failure came during audio, it is the "
+            "voice."
         )
 
     if "Invalid json output" in error_msg or "Expecting value" in error_msg:
@@ -400,8 +409,19 @@ async def generate_podcast_command(
             processing_time=processing_time,
         )
 
-    except ValueError:
-        raise
+    except ValueError as e:
+        # ValueError is the command layer's "permanent failure, do not retry"
+        # signal (retry config uses stop_on=[ValueError]), so the type has to
+        # survive - but LangChain's OutputParserException and
+        # json.JSONDecodeError are ValueError subclasses too. Every parser
+        # failure therefore left through here, past the hint mapper below:
+        # the placeholder speaker name and the truncated-JSON cases reached
+        # the user with no guidance at all (#1238).
+        hint = explain_generation_failure(str(e))
+        if not hint:
+            raise
+        logger.error(f"Podcast generation failed: {e}")
+        raise ValueError(f"{e}\n\nNOTE: {hint}") from e
 
     except Exception as e:
         logger.error(f"Podcast generation failed: {e}")

prompts/podcast/transcript.jinja

@@ -80,7 +80,7 @@ Follow these format requirements strictly:
 
 Return exactly one JSON object with a single root key "transcript" whose value is a list of at least {{ turns }} entries. Each entry has exactly two keys, "speaker" and "dialogue", both strings.
 {% if not language %}
-The line below is a two-entry EXCERPT showing that structure with sample dialogue - keep the structure, write your own words for this segment, and return at least {{ turns }} entries rather than the two shown:
+The line below is a {% if speakers|length == 1 %}one-entry{% else %}two-entry{% endif %} EXCERPT showing that structure with sample dialogue - keep the structure, write your own words for this segment, and return at least {{ turns }} entries rather than the {% if speakers|length == 1 %}one{% else %}two{% endif %} shown:
 
 {% 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."}]}

tests/test_podcast_error_hints.py

@@ -6,9 +6,16 @@
 provider - a truncated Gemini response was told to switch to gpt-4o.
 """
 
+from unittest.mock import AsyncMock, patch
+
 import pytest
+from langchain_core.exceptions import OutputParserException
 
-from commands.podcast_commands import explain_generation_failure
+from commands.podcast_commands import (
+    PodcastGenerationInput,
+    explain_generation_failure,
+    generate_podcast_command,
+)
 
 
 class TestExplainGenerationFailure:
@@ -24,21 +31,26 @@ def test_placeholder_speaker_name_is_explained(self):
         assert "speaker" in hint
         assert "gpt-4o" not in hint
 
-    def test_google_bad_voice_points_at_the_speaker_profile(self):
+    def test_generic_not_found_names_both_candidates(self):
+        """Google returns this for any missing resource, so the hint must not
+        send someone to the voice settings when the real fault is a model id
+        on the outline or transcript call."""
         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
+        assert "model id" in hint
 
     def test_unsupported_voice_name_points_at_the_speaker_profile(self):
+        """This message names the voice, so the hint can be specific."""
         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
+        assert "Speaker Profiles" in hint
 
     @pytest.mark.parametrize(
         "message",
@@ -55,3 +67,57 @@ def test_unparseable_output_mentions_truncation_and_thinking(self, message):
 
     def test_unrecognised_failure_gets_no_hint(self):
         assert explain_generation_failure("Connection reset by peer") is None
+
+
+class TestHintsReachTheUser:
+    """The mapper is only useful if the failure it describes passes through it.
+
+    `except ValueError: raise` guards the command layer's permanent-failure
+    contract, and LangChain's OutputParserException plus json.JSONDecodeError
+    are both ValueError subclasses - so every parser failure, which is exactly
+    what these hints are for, used to skip the mapper entirely.
+    """
+
+    @staticmethod
+    def make_input():
+        return PodcastGenerationInput(
+            episode_profile="Test Episode Profile",
+            episode_name="Test Episode",
+            content="Some content",
+        )
+
+    @pytest.mark.asyncio
+    async def test_parser_failure_keeps_its_type_and_gains_the_hint(self):
+        parse_error = OutputParserException(
+            "Failed to parse ValidatedTranscript from completion "
+            '{"transcript": [{"speaker": "...", "dialogue": "..."}]}. Got: '
+            "Value error, Invalid speaker name '...'."
+        )
+        assert isinstance(parse_error, ValueError)
+
+        with patch(
+            "commands.podcast_commands.EpisodeProfile.get_by_name",
+            AsyncMock(side_effect=parse_error),
+        ):
+            with pytest.raises(ValueError) as exc_info:
+                await generate_podcast_command(self.make_input())
+
+        message = str(exc_info.value)
+        assert "Invalid speaker name" in message
+        assert "NOTE:" in message
+        assert "Speaker names must match the profile exactly" in message
+        # Still a ValueError, so the command layer keeps treating it as
+        # permanent rather than retrying a failure that will repeat.
+        assert not isinstance(exc_info.value, RuntimeError)
+
+    @pytest.mark.asyncio
+    async def test_unrecognised_value_error_is_re_raised_untouched(self):
+        with patch(
+            "commands.podcast_commands.EpisodeProfile.get_by_name",
+            AsyncMock(side_effect=ValueError("Episode profile 'x' not found")),
+        ):
+            with pytest.raises(ValueError) as exc_info:
+                await generate_podcast_command(self.make_input())
+
+        assert str(exc_info.value) == "Episode profile 'x' not found"
+        assert "NOTE:" not in str(exc_info.value)

tests/test_podcast_prompt_templates.py

@@ -166,6 +166,14 @@ def test_transcript_example_points_at_the_turn_minimum(self):
         assert "two-entry EXCERPT" in rendered
         assert "return at least 6 entries rather than the two shown" in rendered
 
+    def test_solo_excerpt_is_not_called_a_two_entry_one(self):
+        """A solo profile renders one entry; calling it a two-entry excerpt
+        contradicts the example directly above it."""
+        rendered = render_transcript(speakers=[SPEAKERS[0]], turns=3)
+        assert "one-entry EXCERPT" in rendered
+        assert "two-entry" not in rendered
+        assert "rather than the one shown" in rendered
+
     def test_outline_example_points_at_the_segment_count(self):
         rendered = render_outline(num_segments=6)
         assert "two-entry EXCERPT" in rendered
@@ -286,15 +294,27 @@ class TestNoDriftFromBundledTemplates:
     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:
+    # Operators and literals a condition can open with: `{% if not language %}`
+    # names no variable called "not".
+    JINJA_KEYWORDS = frozenset(
+        {"not", "and", "or", "is", "in", "if", "else", "true", "false", "none"}
+    )
+
+    @classmethod
+    def _variables(cls, 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
+        return used - loop_locals - cls.JINJA_KEYWORDS
+
+    def test_jinja_keywords_are_not_treated_as_variables(self):
+        """`{% if not language %}` names no variable called "not" - counting it
+        would fail the drift check the moment one template negates a test the
+        other doesn't."""
+        assert "not" not in self._variables(PROMPTS_DIR / "transcript.jinja")
 
     @pytest.mark.parametrize("template", ["transcript", "outline"])
     def test_app_template_uses_every_bundled_variable(self, template):