fix: one shared response reader, so feedback can parse a real reply - #178
Merged
Merged
Conversation
The feedback agent ran for the first time on 2026-08-11 and failed all
three retries with "Expecting value: line 1 column 1 (char 0)". The model
had returned a complete, correct evaluation; Alphoryn discarded it and
filed the position EVALUATION_FAILED - the exact outcome FR-016a exists to
prevent.
Captured from the live run, the response was two parts:
[0] thought=True 4077-char reasoning summary
[1] thought=None ```json\n{ "outcome_judgment": "CORRECT" ... }
feedback_agent skipped the thought correctly, then handed the fenced
string straight to json.loads. main_agent has stripped fences since it was
written, and also skips parts whose text is empty; feedback_agent did
neither. The same logic had been written twice and the copies drifted.
Rather than paste the two missing guards into the second copy, both agents
now call alphoryn/agents/responses.py:extract_response_json, which skips
thoughts, skips empty parts and strips fences in one place. Two copies of
this is what caused the bug; one copy cannot drift.
Verified by replaying the exact bytes from the failed run: the old path
reproduces the production error, the new path parses and yields
outcome_judgment='CORRECT'.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (5 files)
Reviewed by step-3.7-flash · Input: 82.7K · Output: 7.5K · Cached: 84.7K |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What happened
The feedback agent ran for the first time ever today, in the live run following #176.
It failed all three retries with:
The position was filed
EVALUATION_FAILED. But the model had produced a complete,correct evaluation - Alphoryn threw it away. That is precisely the outcome FR-016a
exists to prevent.
Root cause
Captured from the live run's OTel logs, the response was two parts:
thought[0]True[1]None```json\n{ "outcome_judgment": "CORRECT" ... }feedback_agentskipped the thought correctly (that was #174's fix, and it works),then handed the fenced string straight to
json.loads, which fails at character 0.main_agenthas stripped fences since it was written, and also skips parts whose textis empty.
feedback_agentdid neither:The same logic had been written twice and the two copies drifted.
The fix
Rather than paste the two missing guards into the second copy, both agents now call
alphoryn/agents/responses.py:extract_response_json, which does all three things -skip thoughts, skip empty parts, strip fences - in one place.
_strip_fencesmoved outof
main_agentinto that module.Two copies of this logic is what caused the bug. One copy cannot drift.
Verified against the bytes that actually failed
Replaying the exact response captured from the failed run:
The old path reproduces the production error exactly; the new path recovers the
evaluation that was discarded.
Tests
700 passed, 100% coverage,
ruff check alphoryn/ tests/clean, verified under CI'scredential-less environment.
tests/unit/test_responses.pyis new and covers the shared reader directly, includingtest_a_fenced_answer_after_a_thought_is_the_live_failure, which encodes this exactproduction response shape. The four
_strip_fencestests moved out oftest_main_agent.pyinto it.Note
This is the third defect in a row that only a live run could find (#176 was two). All
three were invisible to a green 637-test suite because every test stubs the LLM and the
broker. Worth remembering when judging how much a passing suite proves here.