diff --git a/alphoryn/agents/feedback_agent.py b/alphoryn/agents/feedback_agent.py index d002336..55062e4 100644 --- a/alphoryn/agents/feedback_agent.py +++ b/alphoryn/agents/feedback_agent.py @@ -20,7 +20,8 @@ from google.genai import types as genai_types from alphoryn.agents.prompts import FEEDBACK_AGENT_SYSTEM_PROMPT -from alphoryn.agents.thinking import is_thought_part, thinking_enabled_config +from alphoryn.agents.responses import extract_response_json +from alphoryn.agents.thinking import thinking_enabled_config from alphoryn.market_data.client import MarketDataClient from alphoryn.memory.bank import MemoryBank from alphoryn.memory.schema import FeedbackEvaluation @@ -196,11 +197,7 @@ def _call_agent( ), ): if event.is_final_response() and event.content and event.content.parts: - for part in event.content.parts: - if is_thought_part(part): - continue - raw_json = part.text - break + raw_json = extract_response_json(event.content.parts) if raw_json is None: _logger.error("feedback_agent produced no final response (attempt %d)", attempt) diff --git a/alphoryn/agents/main_agent.py b/alphoryn/agents/main_agent.py index 8873091..d813e2a 100644 --- a/alphoryn/agents/main_agent.py +++ b/alphoryn/agents/main_agent.py @@ -18,7 +18,8 @@ from google.genai import types as genai_types from alphoryn.agents.prompts import MAIN_AGENT_SYSTEM_PROMPT -from alphoryn.agents.thinking import is_thought_part, thinking_enabled_config +from alphoryn.agents.responses import extract_response_json +from alphoryn.agents.thinking import thinking_enabled_config from alphoryn.execution.agent import AssetDecision, SessionDecision from alphoryn.market_data.client import MarketDataClient from alphoryn.telemetry.logger import TelemetryLogger @@ -109,13 +110,7 @@ def decide( session_id=session_id, ) if event.is_final_response() and event.content and event.content.parts: - for part in event.content.parts: - if is_thought_part(part): - continue - text = getattr(part, "text", None) - if text and text.strip(): - raw_json = _strip_fences(text.strip()) - break + raw_json = extract_response_json(event.content.parts) if raw_json is None: _logger.error("main_agent produced no final response for session %s", session_id) @@ -143,16 +138,6 @@ def decide( return decision -def _strip_fences(text: str) -> str: - """Strip markdown code fences from LLM output (e.g. ```json ... ```).""" - if text.startswith("```"): - lines = text.splitlines() - # drop first line (```json or ```) and trailing ``` line - inner = lines[1:-1] if lines[-1].strip() == "```" else lines[1:] - return "\n".join(inner).strip() - return text - - def _build_prompt( session_id: str, tickers: list[str], diff --git a/alphoryn/agents/responses.py b/alphoryn/agents/responses.py new file mode 100644 index 0000000..266324f --- /dev/null +++ b/alphoryn/agents/responses.py @@ -0,0 +1,48 @@ +"""Reading a JSON answer out of a Gemini response. + +Every agent here asks the model for JSON and gets back a list of parts that +needs the same three things done to it: skip the thought summaries, skip the +parts that carry no text, and strip the markdown fences the model wraps its +answer in. Miss any one and `json.loads` fails at character 0. + +This module exists because that logic was written twice - once in main_agent +and once in feedback_agent - and the two copies drifted. feedback_agent's copy +skipped thoughts but did neither of the other two, so it could never parse a +real reply: on 2026-08-11 it discarded a complete, correct evaluation three +times and filed the position as EVALUATION_FAILED, which is precisely what +FR-016a exists to prevent. One reader, used by both, cannot drift again. +""" + +from collections.abc import Iterable +from typing import Any + +from alphoryn.agents.thinking import is_thought_part + + +def strip_fences(text: str) -> str: + """Strip markdown code fences from LLM output (e.g. ```json ... ```). + + Only a fence at the very start counts. Text that merely contains backticks + - a JSON string value, say - is returned untouched. + """ + if not text.startswith("```"): + return text + lines = text.splitlines() + # Drop the opening ```json / ``` line, and the closing ``` if there is one. + inner = lines[1:-1] if lines[-1].strip() == "```" else lines[1:] + return "\n".join(inner).strip() + + +def extract_response_json(parts: Iterable[Any]) -> str | None: + """Return the first part that holds the model's answer, fences removed. + + Returns None when no part carries usable text, which callers treat as + "the model produced no final response" rather than as an empty answer. + """ + for part in parts: + if is_thought_part(part): + continue + text = getattr(part, "text", None) + if text and text.strip(): + return strip_fences(text.strip()) + return None diff --git a/tests/unit/test_main_agent.py b/tests/unit/test_main_agent.py index f77a208..30ead24 100644 --- a/tests/unit/test_main_agent.py +++ b/tests/unit/test_main_agent.py @@ -11,7 +11,6 @@ MainAgentError, _build_prompt, _parse_decision, - _strip_fences, ) # --------------------------------------------------------------------------- @@ -394,29 +393,8 @@ def test_parse_decision_non_list_decisions_raises_main_agent_error() -> None: _parse_decision(bad_data) -# --------------------------------------------------------------------------- -# _strip_fences -# --------------------------------------------------------------------------- - - -def test_strip_fences_plain_json_unchanged() -> None: - raw = '{"a": 1}' - assert _strip_fences(raw) == raw - - -def test_strip_fences_removes_json_code_fence() -> None: - raw = '```json\n{"a": 1}\n```' - assert _strip_fences(raw) == '{"a": 1}' - - -def test_strip_fences_removes_plain_code_fence() -> None: - raw = '```\n{"a": 1}\n```' - assert _strip_fences(raw) == '{"a": 1}' - - -def test_strip_fences_fence_without_closing_tick() -> None: - raw = '```json\n{"a": 1}' - assert _strip_fences(raw) == '{"a": 1}' +# Fence-stripping moved to alphoryn/agents/responses.py, shared with the +# feedback agent; its tests live in tests/unit/test_responses.py. # --------------------------------------------------------------------------- diff --git a/tests/unit/test_responses.py b/tests/unit/test_responses.py new file mode 100644 index 0000000..7addc66 --- /dev/null +++ b/tests/unit/test_responses.py @@ -0,0 +1,111 @@ +"""Unit tests for alphoryn/agents/responses.py. + +One shared reader for every agent that parses a JSON answer out of a Gemini +response. It exists because main_agent and feedback_agent each grew their own +copy, the copies drifted, and the feedback agent's copy could never parse a +real reply - see test_a_fenced_answer_after_a_thought_is_the_live_failure. +""" + +from unittest.mock import MagicMock + +from alphoryn.agents.responses import extract_response_json, strip_fences + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _part(text: str | None, *, thought: bool | None = None) -> MagicMock: + part = MagicMock() + part.text = text + part.thought = thought + return part + + +# --------------------------------------------------------------------------- +# strip_fences +# --------------------------------------------------------------------------- + + +def test_plain_json_is_unchanged() -> None: + assert strip_fences('{"a": 1}') == '{"a": 1}' + + +def test_json_tagged_fence_is_removed() -> None: + assert strip_fences('```json\n{"a": 1}\n```') == '{"a": 1}' + + +def test_untagged_fence_is_removed() -> None: + assert strip_fences('```\n{"a": 1}\n```') == '{"a": 1}' + + +def test_fence_without_a_closing_tick_is_removed() -> None: + assert strip_fences('```json\n{"a": 1}') == '{"a": 1}' + + +def test_text_that_merely_contains_backticks_is_untouched() -> None: + assert strip_fences('{"a": "```"}') == '{"a": "```"}' + + +# --------------------------------------------------------------------------- +# extract_response_json +# --------------------------------------------------------------------------- + + +def test_a_single_plain_part_is_returned() -> None: + assert extract_response_json([_part('{"a": 1}')]) == '{"a": 1}' + + +def test_a_thought_part_is_skipped() -> None: + parts = [_part("my reasoning", thought=True), _part('{"a": 1}')] + assert extract_response_json(parts) == '{"a": 1}' + + +def test_an_empty_part_is_skipped() -> None: + """A tool-call part carries no text; taking it yields '' and fails at char 0.""" + assert extract_response_json([_part(""), _part('{"a": 1}')]) == '{"a": 1}' + + +def test_a_whitespace_only_part_is_skipped() -> None: + assert extract_response_json([_part(" \n "), _part('{"a": 1}')]) == '{"a": 1}' + + +def test_a_part_with_no_text_attribute_is_skipped() -> None: + assert extract_response_json([_part(None), _part('{"a": 1}')]) == '{"a": 1}' + + +def test_surrounding_whitespace_is_trimmed() -> None: + assert extract_response_json([_part('\n {"a": 1} \n')]) == '{"a": 1}' + + +def test_nothing_usable_returns_none() -> None: + assert extract_response_json([_part(""), _part(None)]) is None + + +def test_no_parts_at_all_returns_none() -> None: + assert extract_response_json([]) is None + + +def test_only_a_thought_returns_none() -> None: + assert extract_response_json([_part("reasoning", thought=True)]) is None + + +def test_the_first_usable_part_wins() -> None: + assert extract_response_json([_part('{"a": 1}'), _part('{"b": 2}')]) == '{"a": 1}' + + +def test_a_fenced_answer_after_a_thought_is_the_live_failure() -> None: + """Regression for the 2026-08-11 run. + + Gemini returned part[0] = a 4077-char thought summary and part[1] = the + answer wrapped in ```json fences. feedback_agent skipped the thought + correctly but then handed the fenced string straight to json.loads, which + fails at character 0. A complete, correct evaluation was discarded three + times and the position was filed EVALUATION_FAILED - the exact outcome + FR-016a exists to prevent. + """ + parts = [ + _part("Alright, let's break down what I'm thinking here...", thought=True), + _part('```json\n{\n "outcome_judgment": "CORRECT"\n}\n```'), + ] + assert extract_response_json(parts) == '{\n "outcome_judgment": "CORRECT"\n}'