DEV-1541: autopsy one-shot split + AutopsyError capture + silent-fail backfill - #39
Conversation
…ror swallow Pre-DEV-1541, run_autopsy used a single a-interact-shaped LLM-output schema and swallowed every exception with `except Exception: return None`. On the latest livesqlbench-base-lite-sqlite slayer-mode runs this silently dropped 7 of 67 graded failures: the LLM correctly recognised "this is one-shot, no user-sim, no asks" and returned `key_asks: []` with no resolutions arrays, but the schema rejected it as a pydantic ValidationError that was caught and discarded. Three orthogonal fixes: 1. Schema split. New AutopsyAnalysisOneShot / AutopsyPatternOneShot / AutopsyLLMOutputOneShot / _AUTOPSY_TOOL_SCHEMA_ONE_SHOT drop the four ask_user-shaped fields (n_asks, key_asks, disclosed_resolutions, undisclosed_resolutions) and the three ask_user-related pattern values from the one-shot path. AutopsyResult.analysis becomes a discriminated union on `kind`; a model_validator(mode="before") injects kind="a_interact" for legacy on-disk reads. _build_prompt and _map_output branch on is_one_shot. 2. Typed exception capture. New AutopsyError persists kind (validation_error/context_overflow/api_error/network_error/ missing_tool_use/unknown), FQN exception class, capped excerpts, and prompt/KB/trajectory size stats. run_autopsy never returns None on failure; AutopsyResult enforces exactly-one(analysis, error). grade_and_write only promotes autopsy-side decision_point / user_sim to the top level when analysis is populated, so an error-path autopsy can't clobber real grader signal. 3. One-shot user_sim plumbing. _resolve_default_user_sim defaults user_sim_interaction to None on one-shot benchmarks (livesqlbench) in both _build_submission_annotation and _write_harness_confirmed_annotation; _user_sim_interaction_from_trajectory gains a `one_shot` kwarg used by regrade and annotate. SubmissionAnnotation.user_sim_interaction is now Optional but keeps default_factory=UserSimInteraction so legacy a-interact annotations with the field omitted still parse to zero-asks. Adds scripts/regenerate_autopsies.py (thin CLI wrapper) + the src/bird_interact_agents/eval/regenerate_autopsies.py library module for re-running autopsy on persisted annotations whose autopsy is absent or carries an AutopsyError. Trajectory load prefers the runs/ sidecar, falls back to results/<bench>/cloud/<run>/ rows/<inst>/attempt-1.json. exit_code() is non-zero only on io_errors so a cron driver doesn't loop on transient LLM errors. The 7 originally-silent livesqlbench annotations (robot_10 + museum_2/4/5/7/9/10) have been regenerated with Opus 4.7 (Sonnet 4.5 fell short on robot_10's 216k-token trajectory). Those updates live under the gitignored runs/ tree in the main checkout and are not part of this commit. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
DEV-1541 Autopsy machinery: one-shot schema mismatch, silent ValidationError swallow, no error capture, ask_user leak
Summary
Concrete log evidence
The LLM correctly recognised "this is one-shot, no user-sim, no asks" and returned Six concrete issues, one umbrella1. Schema is broken for one-shot
These only make sense when there's a user-sim. One-shot has none. The LLM's correct one-shot output omits them → pydantic ValidationError. 2. Prompt + tool-schema leak ask_user patterns
3. Output mapping always emits UserSimInteraction
4. grade_in_place defaults
5. Silent except Exception
try:
tool_use = next(...)
llm_output = AutopsyLLMOutput.model_validate(tool_use.input)
return _map_output(llm_output)
except Exception:
logger.error(...)
return None
The BadRequestError branch a few lines up is similarly opaque — only context-overflow text matching gets special treatment; everything else gets an 6. No error capture into the annotationToday the analysis script can't distinguish:
All three look the same: Proposal — split fix into 6 surfacesSurface 1: branch the schema by is_one_shotNew Pydantic class class AutopsyLLMOutputOneShot(BaseModel):
pattern: Literal[
"late_mutation_corrupted_result",
"wrong_join_path",
"output_schema_misread",
"slayer_generation_artifact",
"exhausted_budget_guessing",
"other",
]
other_details: Optional[str] = None
narrative: str
remediation: str
decision_point_trajectory_index: Optional[int] = None
decision_point_description: Optional[str] = None
# no n_asks, no key_asks, no resolutionsMirror Surface 2: branch _build_prompt by is_one_shotFor one-shot: drop the three ask_user-related pattern definitions; drop any "key clarification" / "user-sim" phrasing; instruct the LLM to omit the resolutions arrays. Surface 3: plumb is_one_shotAdd Surface 4: AutopsyError schemaNew Pydantic class in class AutopsyError(BaseModel):
kind: Literal[
"validation_error",
"context_overflow",
"api_error",
"network_error",
"missing_tool_use",
"unknown",
]
exception_class: str
message_excerpt: str # cap ~500 chars
traceback_excerpt: str # cap ~2k chars
prompt_chars: int
kb_chars: int
trajectory_items: int
model: str
timestamp: datetime
Surface 5: typed except clausestry:
tool_use = next(b for b in response.content if getattr(b, "type", None) == "tool_use")
SchemaCls = AutopsyLLMOutputOneShot if is_one_shot else AutopsyLLMOutput
llm_output = SchemaCls.model_validate(tool_use.input)
return _map_output(llm_output, is_one_shot=is_one_shot)
except pydantic.ValidationError as exc:
return _autopsy_error_result(kind="validation_error", exc=exc, ...)
except StopIteration as exc:
return _autopsy_error_result(kind="missing_tool_use", exc=exc, ...)
except Exception as exc:
return _autopsy_error_result(kind="unknown", exc=exc, ...)
Surface 6: fix grade_in_place / regrade defaultsWhen the benchmark descriptor's BackfillThe 7 unannotated livesqlbench failures ( Tests
Out of scope
RelatedDEV-1535, DEV-1536, DEV-1537, DEV-1538 (SLayer-side fixes surfaced by the same failure-mode analysis). DEV-1540 (one-shot agent prompt tweaks). Source dataPer-task autopsy logs: |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Lite Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR (DEV-1541) extends the autopsy system to support one-shot runs alongside a-interact benchmarks. It adds typed error handling, branches autopsy behavior on a ChangesOne-Shot Autopsy Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bird_interact_agents/eval/autopsy.py (1)
540-557:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWrap prompt/KB preparation in the autopsy error boundary.
run_autopsy()still does KB loading and prompt construction before the firsttry. If either step raises, both agent callers catch the exception and continue with_autopsy_result = None, so the pre-DEV-1541 “missing autopsy” behavior is still reachable on pre-request failures.Suggested fix
- kb_text = _read_kb_text( - slayer_storage_dir, - task_annotation.selected_database, - task_annotation.external_knowledge, - ) - prompt = _build_prompt( - task_annotation=task_annotation, - trajectory=trajectory, - kb_text=kb_text, - miss_diagnostics=miss_diagnostics, - is_one_shot=is_one_shot, - ) - tool_schema = ( - _AUTOPSY_TOOL_SCHEMA_ONE_SHOT if is_one_shot else _AUTOPSY_TOOL_SCHEMA - ) - schema_cls = ( - AutopsyLLMOutputOneShot if is_one_shot else AutopsyLLMOutput - ) + kb_text = "" + prompt = "" try: + kb_text = _read_kb_text( + slayer_storage_dir, + task_annotation.selected_database, + task_annotation.external_knowledge, + ) + prompt = _build_prompt( + task_annotation=task_annotation, + trajectory=trajectory, + kb_text=kb_text, + miss_diagnostics=miss_diagnostics, + is_one_shot=is_one_shot, + ) + tool_schema = ( + _AUTOPSY_TOOL_SCHEMA_ONE_SHOT if is_one_shot else _AUTOPSY_TOOL_SCHEMA + ) + schema_cls = AutopsyLLMOutputOneShot if is_one_shot else AutopsyLLMOutput client = anthropic.AsyncAnthropic() response = await client.messages.create( model=native_model_id(model),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/bird_interact_agents/eval/autopsy.py` around lines 540 - 557, run_autopsy() currently prepares KB and prompt (calls to _read_kb_text and _build_prompt and selection of tool_schema/schema_cls) before entering the function's try/except, so errors there escape the autopsy error boundary; move the entire block that sets kb_text, prompt, tool_schema, and schema_cls into the existing try block inside run_autopsy() (so exceptions from _read_kb_text or _build_prompt are caught) and ensure the except path sets the same fallback behavior currently used when the inner autopsy call fails (e.g., _autopsy_result = None) so both pre-request and in-request failures are handled uniformly.
🧹 Nitpick comments (1)
tests/test_autopsy.py (1)
100-110: ⚡ Quick winDe-duplicate the full-pattern constants to prevent drift.
The same 9-pattern list is defined twice. Keeping a single source avoids accidental divergence between schema tests and LLM-output contract tests.
Suggested patch
-_ALL_AUTOPSY_PATTERNS_LIST = [ - "never_asked_key_question", - "asked_but_ignored_answer", - "user_sim_misleading", - "late_mutation_corrupted_result", - "wrong_join_path", - "output_schema_misread", - "slayer_generation_artifact", - "exhausted_budget_guessing", - "other", -] +_ALL_AUTOPSY_PATTERNS_LIST = _ALL_AUTOPSY_PATTERNSAlso applies to: 1817-1827
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_autopsy.py` around lines 100 - 110, The list constant _ALL_AUTOPSY_PATTERNS is defined twice; remove the duplicate and centralize to a single canonical definition used by both the schema tests and LLM-output contract tests. Keep one module-level _ALL_AUTOPSY_PATTERNS (the existing list shown) and update the other occurrence to reference that same constant (no duplicate literal), ensuring any test imports or references point to this single symbol so the nine-pattern set cannot drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/regenerate_autopsies.py`:
- Around line 65-81: Update _resolve_runs_root to stop constructing a gitignored
“runs/” path manually: after honoring the explicit arg and BIRD_RUNS_ROOT env,
import bird_interact_agents.paths and return paths.runs_root() unconditionally
(use paths.runs_root() directly instead of the hasattr(paths, "runs_root")
branch and the paths.results_root().parent / "runs" fallback), and remove the
final except branch that returns Path.cwd() / "runs" so we no longer fabricate a
runs directory; keep the explicit --runs-root / $BIRD_RUNS_ROOT behavior intact.
In `@src/bird_interact_agents/eval/grade_in_place.py`:
- Around line 51-77: The failed-submission path still constructs a hard-coded
UserSimInteraction() instead of applying the one-shot default logic; update
write_failed_submission_annotation to call _resolve_default_user_sim(...)
(passing the same benchmark and n_ask_user_calls context and any provided value)
and use its return value for the persisted user_sim_interaction rather than
instantiating UserSimInteraction() directly so one-shot benchmarks correctly
store None.
In `@src/bird_interact_agents/eval/regenerate_autopsies.py`:
- Around line 113-117: The except block that catches failures parsing ann_path
currently only logs and continues, so corrupt annotation files are never
recorded in the RegenerationReport; change the except to capture the exception
(e) and increment/record the unreadable annotation on the RegenerationReport
instance (use RegenerationReport.io_errors or the report instance used in
regenerate_autopsies.py) before continuing, and include the error in the
logger.warning message so unreadable files count toward exit_code().
- Around line 308-316: The regenerate function accepts results_root but leaves
it None so _load_trajectory() can't perform the sidecar→cloud fallback; fix by
resolving results_root at the start of regenerate (when results_root is None)
using bird_interact_agents.paths.results_root() so all downstream calls
(including _load_trajectory and any worklist generation) receive a concrete
Path; update any references in regenerate (and its use of run_id/benchmark) to
use the resolved results_root variable.
In `@tests/test_autopsy.py`:
- Around line 1203-1233: The test
test_build_prompt_one_shot_drops_all_ask_user_language currently only includes
"ask_user" in the _ASK_USER_TOKENS tuple, so add the space-separated variant
"ask user" (and any other common spacing variants like "ask user:" if desired)
to _ASK_USER_TOKENS to ensure the case-insensitive match on prompt_lower catches
spaced phrasing; update the tuple definition for _ASK_USER_TOKENS used by the
test that invokes _build_prompt so the leaked list properly flags either
"ask_user" or "ask user".
---
Outside diff comments:
In `@src/bird_interact_agents/eval/autopsy.py`:
- Around line 540-557: run_autopsy() currently prepares KB and prompt (calls to
_read_kb_text and _build_prompt and selection of tool_schema/schema_cls) before
entering the function's try/except, so errors there escape the autopsy error
boundary; move the entire block that sets kb_text, prompt, tool_schema, and
schema_cls into the existing try block inside run_autopsy() (so exceptions from
_read_kb_text or _build_prompt are caught) and ensure the except path sets the
same fallback behavior currently used when the inner autopsy call fails (e.g.,
_autopsy_result = None) so both pre-request and in-request failures are handled
uniformly.
---
Nitpick comments:
In `@tests/test_autopsy.py`:
- Around line 100-110: The list constant _ALL_AUTOPSY_PATTERNS is defined twice;
remove the duplicate and centralize to a single canonical definition used by
both the schema tests and LLM-output contract tests. Keep one module-level
_ALL_AUTOPSY_PATTERNS (the existing list shown) and update the other occurrence
to reference that same constant (no duplicate literal), ensuring any test
imports or references point to this single symbol so the nine-pattern set cannot
drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Lite
Run ID: 1e4bb882-143d-429e-9ff1-747b02e099e5
📒 Files selected for processing (11)
scripts/regenerate_autopsies.pysrc/bird_interact_agents/agents/claude_sdk_otf/agent.pysrc/bird_interact_agents/agents/claude_sdk_otf_ainteract/agent.pysrc/bird_interact_agents/eval/annotate.pysrc/bird_interact_agents/eval/annotation_schema.pysrc/bird_interact_agents/eval/autopsy.pysrc/bird_interact_agents/eval/grade_in_place.pysrc/bird_interact_agents/eval/regenerate_autopsies.pysrc/bird_interact_agents/eval/regrade.pytests/test_autopsy.pytests/test_regenerate_autopsies.py
Round 2 of DEV-1541 process-reviews. Folds in CodeRabbit + Codex. * autopsy.run_autopsy: move _read_kb_text + _build_prompt + tool_schema selection INSIDE the outer try. A pre-LLM-call exception would have bubbled to the agent caller's `except Exception:` and re-introduced the silent `autopsy=None` regression this PR exists to kill. prompt/kb_text default to "" so _autopsy_error_result still records prompt_chars / kb_chars when prep never completed. * eval.regenerate_autopsies.scan_work_list: accept an on_parse_error callback; regenerate() wires it to RegenerationReport.io_errors so corrupt annotations no longer slip past exit_code(). Broaden _annotation_needs_backfill to mirror SubmissionAnnotation._migrate_invalid_verdict so legacy verdict="invalid" genuine misses are picked up. After a successful regenerated autopsy, promote analysis-side decision_point + user_sim_interaction to the top-level annotation fields (mirrors grade_and_write's guarded-promotion contract). Default results_root=None → paths.results_root() at function entry so the sidecar→cloud-results trajectory fallback is reachable for the normal CLI path. * eval.grade_in_place.write_failed_submission_annotation: use _resolve_default_user_sim instead of hard-coding UserSimInteraction(). One-shot benchmarks failing before grading now persist null user_sim (third site after _build_submission_annotation and _write_harness_confirmed_annotation). * scripts/regenerate_autopsies._resolve_runs_root: drop the manual fallback chain (results_root().parent / "runs", Path.cwd() / "runs") and defer unconditionally to paths.runs_root() after the explicit-arg and $BIRD_RUNS_ROOT channels. Manual reconstruction violated the "always use paths.*_root() helpers" rule. * tests/test_autopsy.py: _ASK_USER_TOKENS now matches "ask user" (space) and "ask-user" (hyphen) variants. _ALL_AUTOPSY_PATTERNS_LIST collapsed to an alias of _ALL_AUTOPSY_PATTERNS. 9 new tests covering each fix; full suite is 2554 passed. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…re parse Two correctness fixes Codex caught in round 3 of process-reviews. * scan_work_list: derive instance_id + run_id from the file path BEFORE json.loads, so a targeted `--instance-ids` / `--run-id` backfill doesn't bump io_errors for unrelated corrupt annotations outside the filter. Test: test_scan_filtered_subset_ignores_unrelated_corrupt_file. * _load_trajectory: thread the annotation's `submission.trajectory_path` through and use it for the cloud-results fallback. The previous implementation hardcoded `rows/<inst>/attempt-1.json` regardless of which attempt the annotation actually referenced, so any resubmit (attempt-2+) would silently regenerate the autopsy from a different agent session than the one being overwritten. Falls back to the hardcoded path only when the annotation has no trajectory_path. Test: test_regenerate_uses_submission_trajectory_path_for_cloud_fallback. Full suite is 2556 passed. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Summary
Closes DEV-1541.
eval/autopsy.pywas broken end-to-end for one-shot benchmarks (livesqlbench) and the failure was silent: 7 of 67 graded failures on the latest slayer-mode runs shippedautopsy=None, indistinguishable from "autopsy didn't run." Root cause: an a-interact-shaped pydantic schema rejected the LLM's correct one-shot output (key_asks: [], no resolutions arrays), andexcept Exception: return Noneatautopsy.py:384swallowed theValidationError. The output that DID succeed was contaminated byask_user-shaped hallucinations on one-shot tasks that have no user-sim.Three orthogonal fixes:
AutopsyAnalysisOneShot/AutopsyPatternOneShot/AutopsyLLMOutputOneShot/_AUTOPSY_TOOL_SCHEMA_ONE_SHOTdrop the 4 ask_user-shaped fields and 3 ask_user-related pattern values from the one-shot path.AutopsyResult.analysisis now a discriminated union onkind; amodel_validator(mode="before")injectskind="a_interact"for legacy on-disk reads._build_promptis positively-framed for one-shot (no mention of removed patterns — even negatively, per Codex review).AutopsyErrorpersistskind(validation_error/context_overflow/api_error/network_error/missing_tool_use/unknown), FQN exception class, capped excerpts (500 / 2000), and prompt/KB/trajectory size stats + tz-aware UTC timestamp.run_autopsynever returnsNoneon failure.AutopsyResultenforces exactly-one(analysis,error).grade_and_writeonly promotes autopsy-sidedecision_point/user_simto the top level whenanalysis is not None, so an error-path autopsy can't clobber real grader signal.user_simplumbing._resolve_default_user_simdefaultsuser_sim_interaction=Noneon one-shot benchmarks in both_build_submission_annotationand_write_harness_confirmed_annotation;_user_sim_interaction_from_trajectorygains aone_shotkwarg threaded throughregradeandannotate.SubmissionAnnotation.user_sim_interactionis now Optional but keepsdefault_factory=UserSimInteractionso legacy a-interact annotations with the field omitted still parse to zero-asks.Also adds
scripts/regenerate_autopsies.py(thin CLI wrapper) +src/bird_interact_agents/eval/regenerate_autopsies.py(library) for re-running autopsy on persisted annotations whose autopsy is absent or carries anAutopsyError. Trajectory load prefers the runs/ sidecar, falls back toresults/<bench>/cloud/<run>/rows/<inst>/attempt-1.json.exit_code()is non-zero only onio_errorsso a cron driver doesn't loop on transient LLM errors.The 7 originally-silent livesqlbench annotations (
robot_10+museum_2/4/5/7/9/10) have been regenerated with Opus 4.7. Those live under the gitignoredruns/tree in the main checkout and are not part of this commit.Test plan
2545 passed, 94 skipped, 50 deselected— no regressions.123 passedcovering the schema split, exactly-one invariant, legacy on-disk back-compat (round-trip through fullSubmissionAnnotation), AutopsyError truncation + FQN + tz-aware timestamp, prompt + tool-schema branching, every typed exception path (incl.APIConnectionErrorordered beforeAPIError,APITimeoutErroras a subclass), agent call-site assertions foris_one_shot=True/False,_resolve_default_user_simplumbing in both annotation builders,grade_and_writeoverwrite guard (error-only autopsy must NOT clobber top-leveluser_sim/decision_point), backfill scan/dry-run/IO-vs-autopsy-error exit-code split, sidecar-takes-precedence-over-cloud-results.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests