From 76ceeee1370bc5013e03c874ef82d5f617d950b6 Mon Sep 17 00:00:00 2001 From: POM Date: Sat, 16 May 2026 01:10:33 +0200 Subject: [PATCH] fix(shannon-worker): strip markdown fences when parsing result_val Claude commonly wraps structured JSON in ```json ... ``` markdown fences. _parse_shannon_output used strict json.loads on the result field and swallowed JSONDecodeError with `continue`, so fenced responses silently fell through and surfaced as "parse_error: output missing required keys" downstream. Fall back to _extract_json_object on JSONDecodeError at all three call sites (result-event loop, assistant-message result field, content-block text). _extract_json_object is a small helper added alongside the parser that handles the fenced shape (via raw_decode from the first `{`) as well as the prose-before-JSON and JSON-then-trailing-prose cases. Add regression tests covering all three call sites for the fenced case. Co-Authored-By: Claude Opus 4.7 (1M context) --- megaplan/shannon_worker.py | 72 ++++++++++++++++++++++++++++++++++--- tests/test_workers.py | 73 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 141 insertions(+), 4 deletions(-) diff --git a/megaplan/shannon_worker.py b/megaplan/shannon_worker.py index f692b59e..5e2580a3 100644 --- a/megaplan/shannon_worker.py +++ b/megaplan/shannon_worker.py @@ -316,6 +316,60 @@ def _ensure_shannon_parent_timeout_control() -> None: # --------------------------------------------------------------------------- +def _extract_json_object(text: str) -> dict[str, Any] | None: + """Best-effort recovery of a JSON object embedded in free-form text. + + Handles the common cases that bare ``json.loads`` rejects: + + * Claude responses wrapped in markdown code fences:: + + ```json + {"plan": "..."} + ``` + + * Prose preceding the JSON object (``Here is the plan: {...}``). + * JSON followed by trailing prose (``{...} Hope that helps!``). + + Returns the decoded dict on success, or ``None`` when no JSON object + could be recovered. Non-object payloads (lists, scalars) also return + ``None`` since callers expect a mapping. + """ + + if not isinstance(text, str): + return None + stripped = text.strip() + if not stripped: + return None + # Already-valid JSON object. + if stripped.startswith("{"): + try: + data = json.loads(stripped) + if isinstance(data, dict): + return data + except json.JSONDecodeError: + pass + # Try raw_decode from the first '{' to consume a leading object even + # with trailing prose. + start = stripped.find("{") + if start != -1: + try: + data, _end = json.JSONDecoder().raw_decode(stripped[start:]) + if isinstance(data, dict): + return data + except json.JSONDecodeError: + pass + # Last-resort: trim to outermost braces. + end = stripped.rfind("}") + if end > start: + try: + data = json.loads(stripped[start : end + 1]) + if isinstance(data, dict): + return data + except json.JSONDecodeError: + return None + return None + + def _parse_shannon_output(raw: str) -> tuple[dict[str, Any], dict[str, Any]]: """Parse Shannon CLI JSON output into ``(envelope, payload)``. @@ -390,7 +444,10 @@ def _parse_shannon_output(raw: str) -> tuple[dict[str, Any], dict[str, Any]]: try: result_val = json.loads(result_val) except json.JSONDecodeError: - continue + extracted = _extract_json_object(result_val) + if extracted is None: + continue + result_val = extracted if isinstance(result_val, dict): return msg, result_val @@ -416,7 +473,10 @@ def _parse_shannon_output(raw: str) -> tuple[dict[str, Any], dict[str, Any]]: try: result_val = json.loads(result_val) except json.JSONDecodeError: - continue + extracted = _extract_json_object(result_val) + if extracted is None: + continue + result_val = extracted if isinstance(result_val, dict): return inner, result_val # content blocks that might embed JSON @@ -430,14 +490,18 @@ def _parse_shannon_output(raw: str) -> tuple[dict[str, Any], dict[str, Any]]: if isinstance(parsed, dict): return inner, parsed except json.JSONDecodeError: - pass + extracted = _extract_json_object(text) + if isinstance(extracted, dict): + return inner, extracted elif isinstance(content, str): try: parsed = json.loads(content) if isinstance(parsed, dict): return inner, parsed except json.JSONDecodeError: - pass + extracted = _extract_json_object(content) + if isinstance(extracted, dict): + return inner, extracted # Fallback: return the last dict in the array if data and isinstance(data[-1], dict): diff --git a/tests/test_workers.py b/tests/test_workers.py index c2c08a6e..9fa3fbc5 100644 --- a/tests/test_workers.py +++ b/tests/test_workers.py @@ -3368,6 +3368,79 @@ def test_parse_shannon_output_prefers_result_event() -> None: assert envelope["total_cost_usd"] == 0.01 +def test_parse_shannon_output_result_event_markdown_fenced_json() -> None: + """Regression: Claude commonly wraps structured JSON in ```json ... ``` fences. + + Prior to the fix, _parse_shannon_output called json.loads on the result + field and `continue`d on JSONDecodeError, so fenced responses fell through + and downstream gates surfaced "parse_error: output missing required + keys: plan". The parser should now fall back to _extract_json_object. + """ + from megaplan.shannon_worker import _parse_shannon_output + + plan_payload = { + "plan": "Step 1: do the thing.", + "questions": [], + "success_criteria": ["it works"], + "assumptions": ["env is sane"], + } + fenced = "```json\n" + json.dumps(plan_payload) + "\n```" + transcript = [ + {"type": "assistant", "message": {"content": [{"type": "text", "text": "thinking..."}]}}, + { + "type": "result", + "subtype": "success", + "is_error": False, + "result": fenced, + "session_id": "shannon-fenced-session", + "total_cost_usd": 0.02, + "usage": {"input_tokens": 12, "output_tokens": 8}, + }, + ] + envelope, payload = _parse_shannon_output(json.dumps(transcript)) + assert isinstance(payload, dict) + assert payload == plan_payload + assert set(payload.keys()) == {"plan", "questions", "success_criteria", "assumptions"} + assert envelope["session_id"] == "shannon-fenced-session" + + +def test_parse_shannon_output_assistant_message_markdown_fenced_json() -> None: + """Regression: fenced JSON in an assistant message's `result` field should also parse.""" + from megaplan.shannon_worker import _parse_shannon_output + + plan_payload = { + "plan": "Do it.", + "questions": ["q?"], + "success_criteria": ["ok"], + "assumptions": [], + } + fenced = "```json\n" + json.dumps(plan_payload) + "\n```" + transcript = [ + {"type": "user", "message": {"content": "Plan this"}}, + {"type": "assistant", "message": {"result": fenced}}, + ] + envelope, payload = _parse_shannon_output(json.dumps(transcript)) + assert payload == plan_payload + + +def test_parse_shannon_output_assistant_content_block_fenced_json() -> None: + """Regression: fenced JSON inside an assistant content-block text should also parse.""" + from megaplan.shannon_worker import _parse_shannon_output + + plan_payload = { + "plan": "P", + "questions": [], + "success_criteria": [], + "assumptions": [], + } + fenced = "Here is the plan:\n\n```json\n" + json.dumps(plan_payload) + "\n```" + transcript = [ + {"type": "assistant", "message": {"content": [{"type": "text", "text": fenced}]}}, + ] + envelope, payload = _parse_shannon_output(json.dumps(transcript)) + assert payload == plan_payload + + def test_parse_shannon_output_invalid_json() -> None: from megaplan.shannon_worker import _parse_shannon_output