fix(prd): parse fenced stress-test JSON and bound the walk (#927)

codeframe/cli/app.py

@@ -1713,6 +1713,7 @@ def prd_stress_test(
     from codeframe.core.llm_resolution import resolve_llm_settings, create_provider
     from codeframe.cli.validators import require_api_key_for_provider
     from codeframe.core.prd_stress_test import (
+        StressTestError,
         stress_test_prd,
         resolve_ambiguities_into_prd,
     )
@@ -1749,7 +1750,13 @@ def prd_stress_test(
 
     # Run stress test
     console.print(f"[dim]Recursively decomposing (max depth: {max_depth})...[/dim]")
-    result = stress_test_prd(record.content, provider, max_depth=max_depth)
+    try:
+        result = stress_test_prd(record.content, provider, max_depth=max_depth)
+    except StressTestError as e:
+        # extract_goals now raises rather than returning [] (#927). Without this
+        # the CLI shows a traceback where every other failure here is a red line.
+        console.print(f"[red]Error:[/red] {e}")
+        raise typer.Exit(1)
 
     # Show ambiguity report
     if result.ambiguities:
@@ -1791,7 +1798,11 @@ def prd_stress_test(
             )
             # Re-run stress test on updated PRD to reflect resolved ambiguities
             console.print("[dim]Re-analyzing updated PRD...[/dim]")
-            result = stress_test_prd(new_record.content, provider, max_depth=max_depth)
+            try:
+                result = stress_test_prd(new_record.content, provider, max_depth=max_depth)
+            except StressTestError as e:
+                console.print(f"[red]Error:[/red] {e}")
+                raise typer.Exit(1)
         else:
             console.print("[yellow]Warning:[/yellow] Failed to create new PRD version.")
 
@@ -1806,6 +1817,11 @@ def prd_stress_test(
     # Summary
     node_count = _count_nodes(result.tree)
     console.print(f"\n[bold]Summary:[/bold] {len(result.tree)} goals, {node_count} nodes, {len(result.ambiguities)} ambiguities")
+    if result.partial:
+        console.print(
+            "[yellow]Warning:[/yellow] LLM call budget exhausted — the walk is "
+            "partial. Ambiguities listed are real, but others may be unreported."
+        )
     if result.ambiguities and not interactive:
         console.print("[dim]Tip: Run with --interactive to resolve ambiguities and update the PRD[/dim]")
 

codeframe/core/llm_json.py

@@ -0,0 +1,62 @@
+"""Parsing JSON out of an LLM response (#927).
+
+A **leaf module**: stdlib only, so every consumer can converge on it.
+
+Models routinely wrap JSON in a markdown fence, and local / OpenAI-compatible
+providers do it more often than Anthropic. Four call sites in this repo each
+grew their own fence stripper, in two subtly different shapes — and
+``prd_stress_test`` grew none at all, so it did a raw ``json.loads``, swallowed
+the failure, and reported "No ambiguities found — PRD is well-specified" after
+the user had paid for the call.
+
+The lesson in that bug is the reason this module exists: a parser that returns a
+falsy default on failure turns a provider quirk into a clean bill of health.
+``parse_json_response`` raises instead, and callers decide what to do about it.
+"""
+
+from __future__ import annotations
+
+import json
+import re
+from typing import Any
+
+__all__ = ["LLMJsonError", "parse_json_response", "strip_code_fence"]
+
+#: A fenced block, with or without a language tag, anywhere in the response.
+#: Non-greedy so the *first* complete block wins when a model emits several.
+_FENCE_RE = re.compile(r"```[ \t]*[A-Za-z0-9_+-]*[ \t]*\r?\n(.*?)```", re.DOTALL)
+
+
+class LLMJsonError(ValueError):
+    """An LLM response could not be parsed as JSON."""
+
+
+def strip_code_fence(content: str) -> str:
+    """Return the contents of the first markdown fence, or the input unchanged.
+
+    Tolerates prose around the block ("Sure! Here you go:"), which local models
+    add routinely, and a fence with no language tag.
+    """
+    match = _FENCE_RE.search(content)
+    return match.group(1).strip() if match else content.strip()
+
+
+def parse_json_response(content: str, *, what: str = "response") -> Any:
+    """Parse an LLM response as JSON, stripping any markdown fence.
+
+    Raises:
+        LLMJsonError: If the content is empty or is not JSON once unfenced.
+            Deliberately an exception rather than a falsy default — the caller
+            must not be able to mistake a parse failure for an empty result.
+    """
+    if not content or not content.strip():
+        raise LLMJsonError(f"Empty {what} — nothing to parse")
+
+    stripped = strip_code_fence(content)
+    try:
+        return json.loads(stripped)
+    except (json.JSONDecodeError, TypeError) as exc:
+        preview = stripped[:200].replace("\n", " ")
+        raise LLMJsonError(
+            f"Could not parse {what} as JSON: {exc}. Content began: {preview!r}"
+        ) from exc

codeframe/core/prd_stress_test.py

@@ -9,14 +9,14 @@
 """
 
 import asyncio
-import json
 import logging
 import uuid
 from dataclasses import dataclass
 from enum import Enum
-from typing import AsyncGenerator, Literal, Optional
+from typing import AsyncGenerator, Callable, Literal, Optional
 
 from codeframe.adapters.llm.base import Purpose
+from codeframe.core.llm_json import LLMJsonError, parse_json_response
 
 logger = logging.getLogger(__name__)
 
@@ -65,6 +65,9 @@ class StressTestResult:
     ambiguities: list[Ambiguity]
     tech_spec_markdown: str
     ambiguity_report: str
+    # True when the call budget ran out mid-walk: the ambiguities found are
+    # real, but absence of others is not evidence of their absence (#927).
+    partial: bool = False
 
 
 # ---------------------------------------------------------------------------
@@ -120,6 +123,67 @@ class StressTestResult:
 # ---------------------------------------------------------------------------
 
 
+#: A model can return an arbitrarily long ``children`` list. The depth cap alone
+#: does not bound the walk: breadth multiplies at every level, and each node is
+#: one paid LLM call (#927).
+MAX_CHILDREN_PER_NODE = 12
+
+#: Total classification calls for one stress-test run. Depth 10 (the API
+#: maximum) with even modest breadth is thousands of calls; this is the ceiling
+#: the user's bill actually cares about.
+MAX_LLM_CALLS = 200
+
+
+class StressTestError(RuntimeError):
+    """The stress test could not produce a usable result.
+
+    Raised rather than returning an empty result: the CLI and web UI report
+    "No ambiguities found — PRD is well-specified" for an empty run, so a
+    silent failure reads to the user as a passing grade (#927).
+    """
+
+
+@dataclass
+class _Budget:
+    """Bounds one stress-test walk: total calls, plus caller cancellation.
+
+    ``is_cancelled`` is polled before every call so a disconnected SSE client
+    stops paying for work nobody will read (#927).
+    """
+
+    max_calls: int = MAX_LLM_CALLS
+    is_cancelled: Optional[Callable[[], bool]] = None
+    spent: int = 0
+    exhausted: bool = False
+    cancelled: bool = False
+
+    def take(self) -> bool:
+        """Claim one LLM call. False means stop walking."""
+        if self.is_cancelled is not None and self.is_cancelled():
+            self.cancelled = True
+            return False
+        if self.spent >= self.max_calls:
+            self.exhausted = True
+            return False
+        self.spent += 1
+        return True
+
+    @property
+    def stopped_early(self) -> bool:
+        return self.exhausted or self.cancelled
+
+
+def _cap_children(children: list[dict]) -> list[dict]:
+    """Truncate a model-supplied children list to a sane breadth."""
+    if len(children) <= MAX_CHILDREN_PER_NODE:
+        return children
+    logger.warning(
+        "Model returned %d children; keeping the first %d",
+        len(children), MAX_CHILDREN_PER_NODE,
+    )
+    return children[:MAX_CHILDREN_PER_NODE]
+
+
 def extract_goals(prd_content: str, provider) -> list[str]:
     """Extract high-level deliverable goals from a PRD."""
     response = provider.complete(
@@ -130,13 +194,26 @@ def extract_goals(prd_content: str, provider) -> list[str]:
         temperature=0.0,
     )
     try:
-        goals = json.loads(response.content)
-        if isinstance(goals, list):
-            return [str(g) for g in goals]
-        logger.warning("Goal extraction returned non-list: %s", type(goals).__name__)
-    except (json.JSONDecodeError, TypeError) as exc:
-        logger.warning("Failed to parse goal extraction response: %s", exc)
-    return []
+        goals = parse_json_response(response.content, what="goal extraction")
+    except LLMJsonError as exc:
+        # Never return [] here. The caller reads an empty list as "no goals to
+        # analyse" and reports "No ambiguities found — PRD is well-specified",
+        # so a provider that fenced its JSON produced a clean bill of health
+        # after the user had paid for the call (#927).
+        raise StressTestError(f"Could not read the model's goal list: {exc}") from exc
+
+    if not isinstance(goals, list):
+        raise StressTestError(
+            f"Goal extraction returned {type(goals).__name__}, expected a list"
+        )
+
+    extracted = [str(g) for g in goals if str(g).strip()]
+    if not extracted:
+        raise StressTestError(
+            "The model returned no goals for this PRD. That is not a "
+            "well-specified PRD — it is an unusable response."
+        )
+    return extracted
 
 
 def classify_and_decompose(
@@ -170,11 +247,21 @@ def classify_and_decompose(
     )
 
     try:
-        data = json.loads(response.content)
-    except (json.JSONDecodeError, TypeError) as exc:
+        data = parse_json_response(response.content, what=f"classification of {title!r}")
+    except LLMJsonError as exc:
+        # A single unparseable node degrades to a leaf rather than failing the
+        # whole run — unlike goal extraction, where an empty result is
+        # indistinguishable from success (#927).
         logger.warning("Failed to parse classification for '%s': %s", title, exc)
         return Classification.ATOMIC, [], None, "Low"
 
+    if not isinstance(data, dict):
+        logger.warning(
+            "Classification for '%s' returned %s, expected an object",
+            title, type(data).__name__,
+        )
+        return Classification.ATOMIC, [], None, "Low"
+
     raw_cls = data.get("classification", "atomic").lower()
     try:
         cls = Classification(raw_cls)
@@ -184,10 +271,10 @@ def classify_and_decompose(
     complexity = data.get("complexity_hint", "Low")
     raw_children = data.get("children", []) if cls == Classification.COMPOSITE else []
     # Validate children are dicts with expected keys
-    children = [
+    children = _cap_children([
         c for c in raw_children
         if isinstance(c, dict) and ("title" in c or "description" in c)
-    ]
+    ])
 
     ambiguity = None
     if cls == Classification.AMBIGUOUS:
@@ -214,10 +301,19 @@ def recursive_decompose(
     max_depth: int,
     ambiguities: list[Ambiguity],
     provider,
+    budget: Optional["_Budget"] = None,
 ) -> DecompositionNode:
-    """Recursively decompose a goal, collecting ambiguities along the way."""
-    # Force leaf at max depth
-    if depth >= max_depth:
+    """Recursively decompose a goal, collecting ambiguities along the way.
+
+    ``budget`` bounds the total number of LLM calls and lets a disconnected
+    caller stop the walk. A run that stops early returns the partial tree built
+    so far rather than raising — the ambiguities already found are real (#927).
+    """
+    if budget is None:
+        budget = _Budget()
+
+    # Force leaf at max depth, when the call budget is spent, or on cancellation
+    if depth >= max_depth or not budget.take():
         return DecompositionNode(
             id=str(uuid.uuid4()),
             title=title,
@@ -249,8 +345,11 @@ def recursive_decompose(
                 max_depth,
                 ambiguities,
                 provider,
+                budget,
             )
             children.append(child_node)
+            if budget.stopped_early:
+                break
 
     return DecompositionNode(
         id=str(uuid.uuid4()),
@@ -402,7 +501,14 @@ def stress_test_prd(
     tree: list[DecompositionNode] = []
     ambiguities: list[Ambiguity] = []
 
+    # One budget for the whole run, matching the streaming path. Letting each
+    # goal default to its own ``_Budget()`` would bound the walk at
+    # ``len(goals) × MAX_LLM_CALLS`` instead of the documented total (#927).
+    budget = _Budget()
+
     for goal in goals:
+        if budget.stopped_early:
+            break
         node = recursive_decompose(
             title=goal,
             description=goal,
@@ -412,6 +518,7 @@ def stress_test_prd(
             max_depth=max_depth,
             ambiguities=ambiguities,
             provider=provider,
+            budget=budget,
         )
         tree.append(node)
 
@@ -435,11 +542,15 @@ def stress_test_prd(
         ambiguities=ambiguities,
         tech_spec_markdown=tech_spec,
         ambiguity_report=amb_report,
+        partial=budget.stopped_early,
     )
 
 
 async def stress_test_prd_stream(
-    prd_content: str, provider, max_depth: int = 3
+    prd_content: str,
+    provider,
+    max_depth: int = 3,
+    is_cancelled: Optional[Callable[[], bool]] = None,
 ) -> AsyncGenerator[dict, None]:
     """Async streaming variant of :func:`stress_test_prd`.
 
@@ -465,7 +576,15 @@ async def stress_test_prd_stream(
         ambiguities: list[Ambiguity] = []
         tree: list[DecompositionNode] = []
 
+        # One budget for the whole run: the total-call ceiling and the
+        # cancellation check both live inside the recursion, so a disconnected
+        # client stops paying at the next node rather than at the next
+        # top-level goal (#927).
+        budget = _Budget(is_cancelled=is_cancelled)
+
         for goal in goals:
+            if budget.stopped_early:
+                break
             node = await asyncio.to_thread(
                 recursive_decompose,
                 goal,            # title
@@ -476,6 +595,7 @@ async def stress_test_prd_stream(
                 max_depth,
                 ambiguities,
                 provider,
+                budget,
             )
             tree.append(node)
             yield {
@@ -493,6 +613,9 @@ async def stress_test_prd_stream(
             "ambiguities": [ambiguity_to_dict(a) for a in ambiguities],
             "tech_spec_markdown": tech_spec,
             "ambiguity_report": amb_report,
+            # Honest about a truncated walk: the ambiguities found are real,
+            # but absence of others is not evidence of their absence (#927).
+            "partial": budget.stopped_early,
         }
     except Exception as exc:  # noqa: BLE001 — surface any failure to the client
         logger.warning("Stress test stream failed: %s", exc, exc_info=True)