diff --git a/backend/ai_runtime.py b/backend/ai_runtime.py index 8302634..c40690c 100644 --- a/backend/ai_runtime.py +++ b/backend/ai_runtime.py @@ -1,11 +1,13 @@ -"""Shared sync→async bridge for the Claude-agent subsystems (REFACTOR-003). +"""Shared runtime helpers for the Claude-agent subsystems (REFACTOR-003, AI-006). The fundamentals, technical, and 67-Ka-Funda agents each need to run one Agent -SDK coroutine to completion from synchronous (Streamlit) code. All three used -to carry a private copy of the same bridge; the copies drifted once (the -technical agent's lost the context fix below), so the logic now lives here and -the agents delegate. Design rationale and the options weighed are in the ADR: -``docs/architecture/refactor-003-ai-runtime.md``. +SDK coroutine to completion from synchronous (Streamlit) code, and to pull the +verdict JSON object out of the model's final message. All three used to carry +private copies of both helpers; the bridge copies drifted once (the technical +agent's lost the context fix below), so the logic now lives here and the +agents delegate. Design rationale and the options weighed are in the ADR: +``docs/architecture/refactor-003-ai-runtime.md`` (the extractor was folded in +by its AI-006 amendment). Two subtleties are handled, both easy to get wrong: @@ -35,13 +37,66 @@ import asyncio import concurrent.futures import contextvars +import json +import re import sys from collections.abc import Awaitable -from typing import TypeVar +from typing import Any, NoReturn, TypeVar T = TypeVar("T") +def _reject_non_json_constant(value: str) -> NoReturn: + """Reject Python's optional NaN/Infinity JSON decoder extensions. + + Beginner note: + The JSON standard only permits finite numbers, but ``json.loads`` accepts + ``NaN`` and infinities unless a ``parse_constant`` callback rejects them. + Agent output is persisted and signed as strict JSON, so accepting those + values here would merely move the failure to the later cache boundary. + """ + raise ValueError(f"Non-standard JSON numeric constant: {value}") + + +def extract_json_object(text: str) -> dict[str, Any] | None: + """Pull a single JSON object out of an agent's final message text. + + Each agent instructs the model to emit ONLY a JSON object (AgentVerdict, + TechnicalVerdict, or the 67-Ka-Funda verdict), but real models + occasionally wrap it in a ```json fence or add a stray sentence. This + helper is tolerant: it first looks for a fenced block, then falls back to + the outermost {...} span. Returns None when nothing parses. + + Beginner note: + All three agents used to carry a logic-identical private copy of this + function — the same drift-prone shape as the sync bridge below, which DID + drift once. AI-006 moved the single implementation here; each agent + imports it under its old private name (``_extract_json_object``) so call + sites and per-agent parse-fallback behavior stay exactly as they were. + """ + if not text: + return None + + fenced = re.search(r"```(?:json)?\s*(\{.*\})\s*```", text, re.DOTALL) + if fenced: + candidate = fenced.group(1) + else: + start = text.find("{") + end = text.rfind("}") + if start == -1 or end == -1 or end < start: + return None + candidate = text[start : end + 1] + + try: + parsed = json.loads(candidate, parse_constant=_reject_non_json_constant) + except (ValueError, RecursionError): + # JSONDecodeError subclasses ValueError. RecursionError is also a + # model-controlled parse failure when the response is deeply nested; + # both should enter the agents' normal retry path instead of escaping. + return None + return parsed if isinstance(parsed, dict) else None + + def run_agent_coroutine(coro: Awaitable[T]) -> T: """Run one agent coroutine to completion from sync code and return its result. diff --git a/backend/ai_validation.py b/backend/ai_validation.py index dd92225..1a7bf29 100644 --- a/backend/ai_validation.py +++ b/backend/ai_validation.py @@ -42,9 +42,14 @@ class StrictAIModel(BaseModel): - """Strict base class for model-produced JSON.""" + """Strict base class for finite, model-produced JSON values. - model_config = ConfigDict(strict=True, extra="forbid") + ``allow_inf_nan=False`` repeats the runtime decoder's finite-number rule at + the schema boundary. This defense in depth also protects callers that + validate an already-built Python mapping instead of parsing JSON text. + """ + + model_config = ConfigDict(strict=True, extra="forbid", allow_inf_nan=False) class AIValidationError(RuntimeError): diff --git a/backend/fundamentals/fundamental_agent.py b/backend/fundamentals/fundamental_agent.py index 58101b2..82a5f6c 100644 --- a/backend/fundamentals/fundamental_agent.py +++ b/backend/fundamentals/fundamental_agent.py @@ -40,7 +40,6 @@ import contextvars import json import logging -import re from collections.abc import Awaitable, Callable from dataclasses import dataclass from datetime import UTC, datetime @@ -48,6 +47,10 @@ from pydantic import Field, ValidationError, ValidationInfo, field_validator +# The aliased import keeps this module's call sites unchanged: the shared +# extractor pulls the AgentVerdict JSON out of the model's final message +# (AI-006 — one tolerant implementation for all three agents). +from backend.ai_runtime import extract_json_object as _extract_json_object from backend.ai_runtime import run_agent_coroutine from backend.ai_validation import StrictAIModel, parse_with_retry from backend.config import get_ai_max_attempts @@ -668,34 +671,6 @@ def _data_date_from_payload(data: dict[str, Any]) -> str: return datetime.now(UTC).date().isoformat() -def _extract_json_object(text: str) -> dict[str, Any] | None: - """Pull the AgentVerdict JSON object out of the model's final message. - - The model is instructed to emit ONLY a JSON object, but real models - occasionally wrap it in a ```json fence or add a stray sentence. This - helper is tolerant: it first looks for a fenced block, then falls back to - the outermost {...} span. Returns None when nothing parses. - """ - if not text: - return None - - fenced = re.search(r"```(?:json)?\s*(\{.*\})\s*```", text, re.DOTALL) - if fenced: - candidate = fenced.group(1) - else: - start = text.find("{") - end = text.rfind("}") - if start == -1 or end == -1 or end < start: - return None - candidate = text[start : end + 1] - - try: - parsed = json.loads(candidate) - except json.JSONDecodeError: - return None - return parsed if isinstance(parsed, dict) else None - - def _build_user_prompt(symbol: str, mode: str, model: str) -> str: """Build the per-stock kickoff message for the agent.""" return ( diff --git a/backend/sixty_seven/agent.py b/backend/sixty_seven/agent.py index bf72d4e..0ba4301 100644 --- a/backend/sixty_seven/agent.py +++ b/backend/sixty_seven/agent.py @@ -29,7 +29,6 @@ import hashlib import json import logging -import re from collections.abc import Awaitable, Callable from datetime import UTC, datetime from typing import Any, Literal @@ -42,6 +41,11 @@ sign_cache_envelope, verify_cache_envelope, ) + +# The aliased import keeps this module's call sites unchanged: the shared +# extractor pulls the 67-Ka-Funda verdict JSON out of the model's final message +# (AI-006 — one tolerant implementation for all three agents). +from backend.ai_runtime import extract_json_object as _extract_json_object from backend.ai_runtime import run_agent_coroutine from backend.ai_validation import StrictAIModel, parse_with_retry from backend.config import get_agent_fast_mode, get_ai_max_attempts, get_fundamentals_model @@ -250,32 +254,6 @@ class SixtySevenEvaluationResult: """ -def _extract_json_object(text: str) -> dict[str, Any] | None: - """Pull the verdict JSON object out of the model's final message. - - Tolerant of a stray ```json fence or a leading sentence: it looks for a fenced - block first, then falls back to the outermost {...} span. Returns None when - nothing parses. (Mirrors the fundamental / technical agents' extractor; kept - local so the three agents stay independent.) - """ - if not text: - return None - fenced = re.search(r"```(?:json)?\s*(\{.*\})\s*```", text, re.DOTALL) - if fenced: - candidate = fenced.group(1) - else: - start = text.find("{") - end = text.rfind("}") - if start == -1 or end == -1 or end < start: - return None - candidate = text[start : end + 1] - try: - parsed = json.loads(candidate) - except json.JSONDecodeError: - return None - return parsed if isinstance(parsed, dict) else None - - def _candidate_hash(candidate: DrawdownCandidate) -> str: """Return a short, stable digest of the candidate's deterministic price facts. diff --git a/backend/technical/technical_agent.py b/backend/technical/technical_agent.py index b5d7bb8..929fb39 100644 --- a/backend/technical/technical_agent.py +++ b/backend/technical/technical_agent.py @@ -35,7 +35,6 @@ import hashlib import json import logging -import re from collections.abc import Awaitable, Callable from dataclasses import dataclass from datetime import UTC, datetime @@ -49,6 +48,11 @@ sign_cache_envelope, verify_cache_envelope, ) + +# The aliased import keeps this module's call sites unchanged: the shared +# extractor pulls the TechnicalVerdict JSON out of the model's final message +# (AI-006 — one tolerant implementation for all three agents). +from backend.ai_runtime import extract_json_object as _extract_json_object from backend.ai_runtime import run_agent_coroutine from backend.ai_validation import StrictAIModel, parse_with_retry from backend.config import get_ai_max_attempts @@ -333,39 +337,6 @@ class TechnicalEvaluationResult: _FINAL_OUTPUT_INSTRUCTION = FINAL_OUTPUT_INSTRUCTION -# --------------------------------------------------------------------------- -# Helpers -# --------------------------------------------------------------------------- - - -def _extract_json_object(text: str) -> dict[str, Any] | None: - """Pull the TechnicalVerdict JSON object out of the model's final message. - - Tolerant of a stray ```json fence or a leading sentence: it looks for a - fenced block first, then falls back to the outermost {...} span. Returns - None when nothing parses. (Mirrors the fundamentals agent's extractor; kept - local so the two agents stay independent.) - """ - if not text: - return None - - fenced = re.search(r"```(?:json)?\s*(\{.*\})\s*```", text, re.DOTALL) - if fenced: - candidate = fenced.group(1) - else: - start = text.find("{") - end = text.rfind("}") - if start == -1 or end == -1 or end < start: - return None - candidate = text[start : end + 1] - - try: - parsed = json.loads(candidate) - except json.JSONDecodeError: - return None - return parsed if isinstance(parsed, dict) else None - - # --------------------------------------------------------------------------- # Agent # --------------------------------------------------------------------------- diff --git a/docs/architecture/refactor-003-ai-runtime.md b/docs/architecture/refactor-003-ai-runtime.md index 9b8d72e..45cbefe 100644 --- a/docs/architecture/refactor-003-ai-runtime.md +++ b/docs/architecture/refactor-003-ai-runtime.md @@ -1,6 +1,6 @@ # ADR — REFACTOR-003: one shared sync bridge for the Claude-agent subsystems -**Status:** Accepted +**Status:** Accepted — amended by AI-006 (2026-07-10, see the Amendment section) **Date:** 2026-07-07 **Deciders:** repo maintainer (PR review); authored by Claude (Fable) during the July 2026 whole-app review **Relates to:** [audit-2026-06.md](audit-2026-06.md) "Deferred follow-ups" (Agent SDK boilerplate dedup) · TEST-003 (context-propagation bug class) @@ -91,3 +91,27 @@ safety gain. Option A takes exactly the shared-and-dangerous part and nothing el 2. [x] `tests/test_ai_runtime.py`: contextvar propagation, result/exception passthrough, sequential reuse. 3. [x] Migrate agents one commit each (technical → sixty_seven → fundamental), keeping delegating `_run_sync` staticmethods. 4. [x] Full gate suite + the three agent test suites green. + +## Amendment — AI-006 (2026-07-10): the JSON extractor joins the shared runtime + +Option A's accepted con was that everything outside the bridge "stays per-agent". +For the options-construction blocks that remains the right call (they differ for real +domain reasons — see the trade-off analysis above). But one of the leftover blocks, +`_extract_json_object`, failed the same three-part test that justified moving the +bridge: it was (a) **logic-identical** across all three agents (verified line-by-line +before the move — only docstrings differed), (b) **relied on for correctness** of every +verdict parse, and (c) **drift-prone in exactly the bridge's way** — two of the three +copies carried a "kept local so the agents stay independent" comment while being +character-for-character the same logic, which is copy-paste drift waiting for its +first inconsistent fix. + +AI-006 therefore moved the single implementation into `backend/ai_runtime.py` as +`extract_json_object(text)`. Each agent imports it under its old private name +(`from backend.ai_runtime import extract_json_object as _extract_json_object`), so +call sites, per-agent parse-fallback behavior, and the agent test suites are all +unchanged. Extractor unit tests (fenced block, missing language tag, surrounding +prose, no-JSON/invalid-JSON/reversed-brace edges) live in `tests/test_ai_runtime.py`. + +This does **not** reopen Option B: the extractor, like the bridge, is shared-and- +identical; the options-construction/retry/error-taxonomy blocks remain genuinely +different per agent and stay where they are. diff --git a/tests/test_ai_runtime.py b/tests/test_ai_runtime.py index 0e8e343..bcd1f08 100644 --- a/tests/test_ai_runtime.py +++ b/tests/test_ai_runtime.py @@ -1,4 +1,4 @@ -"""Tests for the shared Claude-agent sync bridge (REFACTOR-003). +"""Tests for the shared Claude-agent runtime helpers (REFACTOR-003, AI-006). ``backend.ai_runtime.run_agent_coroutine`` is the one place that bridges sync (Streamlit) code into the Agent SDK's async world. These tests lock the two @@ -10,6 +10,10 @@ 2. The coroutine runs on a fresh event loop that supports subprocess transports on Windows (the Agent SDK spawns the Claude CLI as a subprocess, which Tornado's selector loop cannot do). + +``extract_json_object`` (AI-006) is the shared tolerant verdict-JSON extractor +all three agents import under their old private ``_extract_json_object`` name; +its tests below lock the tolerance behaviors the agents rely on. """ from __future__ import annotations @@ -20,7 +24,8 @@ import pytest -from backend.ai_runtime import run_agent_coroutine +import backend.ai_runtime as ai_runtime +from backend.ai_runtime import extract_json_object, run_agent_coroutine _PROBE: contextvars.ContextVar[str] = contextvars.ContextVar("ai_runtime_probe", default="unset") @@ -75,3 +80,75 @@ async def _loop_name() -> str: # Windows; Tornado installs the selector policy, so the bridge must # build the right loop explicitly rather than inherit the policy. assert first == "ProactorEventLoop" + + +# --------------------------------------------------------------------------- +# extract_json_object (AI-006) — the shared tolerant verdict extractor +# --------------------------------------------------------------------------- + + +def test_extracts_bare_json_object(): + assert extract_json_object('{"approved": true, "confidence": 8}') == { + "approved": True, + "confidence": 8, + } + + +def test_extracts_from_json_fence(): + text = 'Here is my verdict:\n```json\n{"rating": "BUY"}\n```\nDone.' + assert extract_json_object(text) == {"rating": "BUY"} + + +def test_extracts_from_fence_without_language_tag(): + assert extract_json_object('```\n{"rating": "SELL"}\n```') == {"rating": "SELL"} + + +def test_extracts_outermost_span_despite_surrounding_prose(): + text = 'Sure! The verdict is {"rating": "HOLD", "nested": {"depth": 2}} — hope that helps.' + assert extract_json_object(text) == {"rating": "HOLD", "nested": {"depth": 2}} + + +def test_returns_none_for_empty_text(): + assert extract_json_object("") is None + + +def test_returns_none_when_no_braces_present(): + assert extract_json_object("The model rambled and produced no JSON at all.") is None + + +def test_returns_none_when_braces_do_not_parse(): + assert extract_json_object("{not: valid json}") is None + + +@pytest.mark.parametrize("constant", ["NaN", "Infinity", "-Infinity"]) +def test_returns_none_for_non_finite_json_numbers(constant): + """Model-only numeric extensions must not enter strict cache payloads. + + Beginner note: + Python's JSON decoder accepts these JavaScript-style constants by default, + even though the JSON standard does not. Returning ``None`` here makes the + normal agent retry path handle them like any other malformed response. + """ + assert extract_json_object(f'{{"confidence": {constant}}}') is None + + +def test_returns_none_when_json_decoder_hits_recursion_limit(monkeypatch): + """A decoder recursion failure is a parse miss on every Python version. + + Beginner note: the nesting depth that triggers ``RecursionError`` differs + between Python's JSON decoder implementations. Raising at the decoder + boundary tests our behavior directly instead of assuming a magic depth that + happens to fail on one interpreter but succeeds on another. + """ + + def _raise_recursion(*_args, **_kwargs): + raise RecursionError("decoder nesting limit") + + monkeypatch.setattr(ai_runtime.json, "loads", _raise_recursion) + + assert extract_json_object('{"value": 1}') is None + + +def test_returns_none_for_reversed_braces(): + # rfind("}") lands BEFORE find("{"): the span is invalid, not an exception. + assert extract_json_object("} stray close then open {") is None diff --git a/tests/test_technical_analysis_agent.py b/tests/test_technical_analysis_agent.py index 7b7bcbd..80c9224 100644 --- a/tests/test_technical_analysis_agent.py +++ b/tests/test_technical_analysis_agent.py @@ -149,6 +149,15 @@ def test_technical_verdict_rejects_coercion_and_unknown_fields(): TechnicalVerdict.model_validate(nested) +def test_technical_verdict_rejects_non_finite_numbers(): + """The shared strict model rejects NaN even when built outside JSON parsing.""" + payload = _sample_verdict().model_dump(mode="json") + payload["key_levels"] = [float("nan")] + + with pytest.raises(Exception): + TechnicalVerdict.model_validate(payload) + + # --------------------------------------------------------------------------- # Agent behaviour (driven by the fake runner) # --------------------------------------------------------------------------- @@ -516,6 +525,82 @@ async def __call__( assert result.verdict.pattern == "at_support" +def test_agent_retries_non_finite_json_before_cache_signing(tmp_path, monkeypatch): + """A model-controlled NaN must be retried before the signing boundary. + + The first response uses Python's historically accepted non-standard JSON + constant. The second response is valid. If NaN escapes parsing, strict + cache canonicalization raises instead of returning a normal evaluation. + """ + monkeypatch.setenv("SCANNER_AI_MAX_ATTEMPTS", "2") + cache = FundamentalsCache(cache_dir=tmp_path) + valid_json = json.dumps(_sample_verdict().model_dump(mode="json")) + + class _NonFiniteThenValidRunner(_FakeRunner): + async def __call__( + self, prompt, *, system_prompt, model, max_turns, tool_context=None + ): + self.calls += 1 + if self.calls == 1: + return AgentRunResult( + text=valid_json.replace('"key_levels": [95.0]', '"key_levels": [NaN]') + ) + return AgentRunResult(text=valid_json) + + runner = _NonFiniteThenValidRunner(_sample_verdict()) + agent = TechnicalAnalysisAgent(model="test-model", cache=cache, runner=runner) + + result = agent.evaluate("DEMO", _sample_candles(), _sample_levels()) + + assert runner.calls == 2 + assert result.error_type is None + assert result.verdict is not None + assert result.verdict.key_levels == [95.0] + data_date = str(_sample_candles().iloc[-1]["timestamp"])[:10] + assert cache.get_verdict( + "DEMO", + agent._cache_model_key("DEMO", _sample_candles(), _sample_levels(), None), + data_date, + ) is not None + + +def test_agent_turns_persistent_non_finite_json_into_error_without_cache( + tmp_path, monkeypatch +): + """Exhausted unsafe-number retries produce a receipt and skip signing.""" + monkeypatch.setenv("SCANNER_AI_MAX_ATTEMPTS", "2") + cache = FundamentalsCache(cache_dir=tmp_path) + unsafe_json = json.dumps(_sample_verdict().model_dump(mode="json")).replace( + '"key_levels": [95.0]', '"key_levels": [Infinity]' + ) + + class _PersistentNonFiniteRunner(_FakeRunner): + async def __call__( + self, prompt, *, system_prompt, model, max_turns, tool_context=None + ): + self.calls += 1 + return AgentRunResult(text=unsafe_json) + + signing_calls = [] + monkeypatch.setattr( + technical_agent_module, + "sign_cache_envelope", + lambda *args, **kwargs: signing_calls.append((args, kwargs)), + ) + runner = _PersistentNonFiniteRunner(_sample_verdict()) + agent = TechnicalAnalysisAgent(model="test-model", cache=cache, runner=runner) + + result = agent.evaluate("DEMO", _sample_candles(), _sample_levels()) + + assert runner.calls == 2 + assert result.verdict is None + assert result.error_type == "AIValidationError" + assert result.provenance.verdict == "error" + assert result.validated_verdict_json == {} + assert signing_calls == [] + assert list(tmp_path.glob("*_verdict_*.json")) == [] + + def test_agent_rejects_verdict_missing_required_fields(tmp_path, monkeypatch): """A verdict JSON missing a required field exhausts the retry → AIValidationError.""" monkeypatch.setenv("SCANNER_AI_MAX_ATTEMPTS", "2")