Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions alphoryn/agents/feedback_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
21 changes: 3 additions & 18 deletions alphoryn/agents/main_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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],
Expand Down
48 changes: 48 additions & 0 deletions alphoryn/agents/responses.py
Original file line number Diff line number Diff line change
@@ -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
26 changes: 2 additions & 24 deletions tests/unit/test_main_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
MainAgentError,
_build_prompt,
_parse_decision,
_strip_fences,
)

# ---------------------------------------------------------------------------
Expand Down Expand Up @@ -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.


# ---------------------------------------------------------------------------
Expand Down
111 changes: 111 additions & 0 deletions tests/unit/test_responses.py
Original file line number Diff line number Diff line change
@@ -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}'
Loading