fix(planner): attributable step failures, safe narration, headless exit prompt - #212
Open
lfnothias wants to merge 5 commits into
Open
fix(planner): attributable step failures, safe narration, headless exit prompt#212lfnothias wants to merge 5 commits into
lfnothias wants to merge 5 commits into
Conversation
run_cached runs the agent on a worker thread, joins with a deadline, and treats
completed=False as a timeout. The retry loop caught every exception but the two
lines recording it were commented out:
#result['exception'] = e
#result['completed'] = True
So an agent whose three attempts all raised left completed=False and
exception=None. The thread exited, join() returned immediately, and the caller
raised
TimeoutError: Agent 'reconstructor' execution timed out after 18000 seconds
about two minutes into the run, with the real exception discarded — visible
only as a bare print() inside the worker. Every agent-level crash was
misreported as a five-hour timeout.
The discriminator is whether the worker is still alive after the join: alive
past the deadline is a genuine timeout; finished without completing means every
retry raised, so re-raise the last one. A successful attempt clears any earlier
retry's exception, since the caller re-raises whatever is left in the slot.
Observed against stealth/ox-alpha running ASB capsules, where the underlying
cause was the temperature-400 fixed in 6499f03 — invisible behind the timeout.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
(cherry picked from commit 3cfeaf2)
…nstall
Exit code. run_single_thread_eval_loop catches per-row exceptions and continues
— right for a batch — recording success_level: "Error" in execution_history.
Nothing read it, so a run whose every row failed exited 0. Observed: a task
logging "Error in csv row 1: Planner: Execution failed" twice, exit 0. Any
harness scoring by exit code counts that as a pass. papers_mode now reports the
count and exits 1.
Test isolation. Writing the test above surfaced a worse problem. pyproject
force-includes main.py and config.py as top-level modules, so installing the
project drops copies into site-packages, and under pytest those copies won
every import:
import config -> .venv/lib/python3.11/site-packages/config.py (Aug 14)
sources/ is not shipped that way and correctly resolved to the tree, which is
why only main/config were affected — and why it went unnoticed. The effect is
that config_roundtrip_test.py, whose whole purpose is to check which fields
survive dump/load, was checking an install-time snapshot: fields added in the
working tree were invisible to it and it passed regardless.
conftest.py now puts the repo root at the front of sys.path. config_roundtrip
consequently exercises real code for the first time in a while, and still
passes, including over the perspicacite_* fields added in 6499f03.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
(cherry picked from commit 07f7c1b)
The step-execution handler wrapped the cause as
raise Exception(f"❌ Critical error in step execution: {str(e)}") from e
`from e` preserves the chain for a Python caller, but the operator only ever
sees the formatted string. A real run ended with
❌ Critical error in step execution: unhashable type: 'slice'
— no file, no line, no exception type, and nothing in the log to grep for. The
run had already produced its 13.9 kB deliverable and written its ASTRA capsule,
so the bare message made a late failure look like a total loss.
Now logs the traceback and names the exception type, keeping `from e`.
Planner had no logger at all — `self.logger` appeared nowhere in the file — so
logging the traceback needed one added to __init__ first. Without that the fix
would have raised AttributeError from inside an except block, which is a worse
failure than the one it was meant to explain.
The underlying TypeError is not fixed here: it is a dict indexed with a slice
somewhere under run_attempts, and it is not locatable from the message alone.
That is the point — this change is what makes the next occurrence diagnosable.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
(cherry picked from commit ec41bbc)
The p_iimn run of 2026-08-22 (20260822_183340_88cdd1fe) scored 0.760,
wrote reproduction_iimn.md and exported its ASTRA capsule, then died with
Critical error in step execution: unhashable type: 'slice'
and was reported as a 0% success rate with a non-zero exit.
The cause was the TTS summary at the end of run_attempts, which sliced
each element of final_answers. Agents do not answer with strings: every
entry of that run's state_result.json is a dict, so x[:128] indexed a
dict with a slice. Reproduced from the persisted answers on the project
interpreter. On 3.12+, where slices became hashable, the same line
degrades to a KeyError instead — the fix covers both.
Every other consumer already coerced first (planner.py ~397,
evolution_engine.py ~245); this one did not, and the annotations said
list[str] while the data said list[dict]. Corrected both.
The second defect is that the narration sat in the step's success path
at all. A line that is spoken aloud must never decide whether the work
counts, so it moves to _narrate_step_completion and is caught there —
logged with its traceback, not swallowed.
Also anchors the exception-chaining test on the raise rather than on the
message text, which now appears in prose earlier in the module.
(cherry picked from commit 501742f)
The p_iimn re-run of 2026-08-22 reached step 4 of 6 and died with
❌ Planner: Execution failed: EOF when reading a line
Two defects, one behind the other.
request_user_exit called input() on the benchmark path. In a batch run
stdout is redirected and stdin is not a TTY, so the prompt printed into a
log and the read raised EOFError — a message naming neither the question
nor the step. It now raises UserInterventionRequired carrying the
question. Raising rather than exit(1) is deliberate: the CSV harness
counts the row and still prints its summary, which SystemExit from inside
the planner would skip. pricing.py and csv_mode._prompt_with_default were
already headless-safe; this was the last blocking prompt.
It should never have been asked. data_acquisition declared
/workspace/data/ as an output and wrote nine files into it, but
_verify_expected_outputs compares against a scan that yields files only,
so a directory could never match. The step was permanently "missing
outputs" and blocked every step downstream. A declared directory is now
satisfied by any file inside it, matched on the trailing directory name
because plans declare workspace-absolute paths while the scan returns
relative ones.
15 tests, including that an interactive yes still continues and an
interactive no still exits. Suite: 346 passed, the same 16 pre-existing
failures.
(cherry picked from commit 67b4251)
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
1 participant
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.
Slice 4 of 4 from #197. Five commits cherry-picked unchanged from
mimosa_v2_lfx. Each fix names the run it was reproduced from.Problem
run_cachedtreatedcompleted=Falseas a timeout, but the two lines recording the caught exception were commented out. Every agent-level crash was reported as a five-hour timeout two minutes into the run, with the real exception discarded.pyprojectforce-includesmain.pyandconfig.pyas top-level modules, so an installed copy in site-packages won every import during tests.Critical error in step execution: unhashable type: 'slice'with no type, file or traceback; the planner had no logger at all.request_user_exitcalledinput()with stdin redirected and died withEOF when reading a line. The question should not have been asked:_verify_expected_outputscompared a declared directory against a scan that yields files only, so the step was permanently "missing outputs".Solution
papers_modecounts errored rows and exits 1;conftest.pyputs the tree ahead of site-packages.from e.request_user_exitraisesUserInterventionRequiredwhen stdin is not a TTY, so the CSV harness still counts the row and prints its summary. A declared directory is satisfied by any file inside it.Testing
Fresh
uv sync --group dev --python 3.11on this branch:The 17 failures are the pre-existing set from #205. This slice adds 44 passing tests:
agent_failure_reporting_test,papers_mode_exit_code_test,planner_step_error_test,planner_narration_test,headless_planner_test.Backwards compatibility
Interactive runs still get the prompt. #206 defines the same
UserInterventionRequiredclass at the same position, so the two merge cleanly in either order (checked withgit merge-tree); the guards cover different functions (request_user_exithere, plan approval there).🤖 Generated with Claude Code