fix: retry a feedback setup failure instead of ending the run (#164) - #168
Merged
Conversation
FR-016a says a failed evaluation is retried up to 3 times and then marked failed, unblocking the ticker, with the run carrying on. Two steps ran before the retry loop and got none of that: reading the entry report and fetching the evaluation price. Both touch the outside world, so a missing report file or an unreachable Alpaca escaped evaluate(), travelled up through _process_session and stopped the run - leaving the position blocked forever, which is what FR-016a and FR-017 exist to prevent. The entry session may legitimately carry no report path; _run_feedback already passes `html_report_path or ""` for that case, so _extract_thesis opening "" and raising FileNotFoundError is a path the code expects to reach. - Both steps moved inside the retry loop, so they retry like any other failure and end in the normal EVALUATION_FAILED path that unblocks the ticker. - _write_failure accepts no price. candle_close_price is NOT NULL, so the position's own exit price stands in and the reasoning text records that the evaluation price was never obtained, so the row is not mistaken for a real candle close. - Scheduler._run_feedback wraps each position: one that still blows up is logged and emitted as EVALUATION_FAILED, and the loop moves to the next. Nine tests added; five fail against the old code. Closes #164
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.
Fixes #164.
FR-016a says a failed evaluation is retried up to 3 times and then marked failed, which
unblocks the ticker, and the run carries on. Two steps ran before the retry loop and
got none of that protection: reading the entry report and fetching the evaluation price.
Both touch the outside world, so a missing report file or an unreachable Alpaca raised
straight out of
evaluate(), travelled up through_process_sessionand stopped the wholerun - leaving the position blocked forever, which is exactly what FR-016a and FR-017 exist
to prevent.
This is not a hypothetical path. The entry session may legitimately carry no report path;
_run_feedbackalready passeshtml_report_path or ""for that case, so_extract_thesisopening
""and raisingFileNotFoundErroris a path the code expects to reach.What changed
like any other failure and, after 3 attempts, end in the normal
EVALUATION_FAILEDpaththat unblocks the ticker.
_write_failurenow works when no price was ever fetched.candle_close_priceisNOT NULL, so the position's own exit price stands in - a real price for this position -
and the reasoning text records that the evaluation price could not be fetched, so the row
is not mistaken for a genuine candle close.
Scheduler._run_feedbackwraps each position. One that still blows up is logged, echoed,and emitted as
EVALUATION_FAILED, and the loop moves on to the next position and to thesession.
Tests
Nine added. Five of them fail against the old code:
test_a_missing_report_file_does_not_escape_evaluatetest_a_price_fetch_failure_does_not_escape_evaluatetest_a_price_fetch_is_retried_three_timestest_a_price_fetch_that_recovers_on_the_second_attempt_succeedstest_failure_without_a_price_records_the_exit_price_insteadThe rest pin the scheduler safety net and confirm a successful price fetch is still the
price recorded on an LLM-side failure.
Verification