DEV-1649: persist + reuse the agent's edited slayer models per task - #84
Merged
ZmeiGorynych merged 6 commits intoJul 9, 2026
Merged
Conversation
Add two local CLI flags for on-the-fly slayer runs: * --save-edited-models: after a SUCCESSFUL run (in-task audited phase1_passed) that actually changed the per-task SLayer store, snapshot the whole store as runs/<bench>/<db>/<iid>/edited_models.tar.gz (latest-wins overwrite). No-op runs are skipped via a content-manifest baseline captured post re-anchor + post HARD-8 mask. * --apply-edited-models: start each task from that saved store instead of the bare deterministic cache. Untars, validates _STORE_META (deleted_kb_ids + cache fingerprint), re-anchors the datasource connection, and does NOT re-encode KB memories (would clobber the agent's edits). Falls back to the fresh cache on any miss/mismatch. Stored as a single .tar.gz so no loose *.json leaks into the recursive runs/ annotation walkers. Provenance stamped onto results.db (edited_models_saved_path / _applied_from) + the OTF event log; the SubmissionAnnotation schema is left untouched. Scope: all five on-the-fly slayer interact agents (they funnel through slayer_otf.prepare_task_storage) via a shared finalize_with_edited_models_save hook; rejected for raw / --pre-encoded-models / *_otf_encode. Local only; cloud submit never sets the flags (workers read them with .get(..., False)). Extract the datasource-reanchor helper into slayer_otf.datasource_reanchor (runtime aliases it) to break the runtime<->edited_models import cycle. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
DEV-1649 Save edited models after each benchmark run
Right now, in a slayer-mode run, an agent typically modifies the models (encoding kb items it needs to complete its task). Right now the modified models are discarded after the run. I want to add capability, enabled by a flag in the CLI, to persist the modified models and use them in the next run for this task (without spamming the disk with duplicate information). Store the diffs as part of the run record perhaps, and optionally apply when the flag is set? |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds edited-model save/apply support for on-the-fly SLayer runs, including archive persistence, resolver integration, agent finalization wiring, CLI and runner propagation, result provenance columns, and tests covering archive, routing, and validation behavior. ChangesEdited Models Save/Apply Feature
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/bird_interact_agents/run.py (2)
2700-2718: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCLI's early fail-fast validation doesn't thread
save_edited_models/apply_edited_models— bad combos slip past the fast-fail guard.This
_validate_slayer_setupcall (used specifically to fail fast "before any task starts" per its own docstring, and before_maybe_bootstrap_local_postgres/_maybe_sync_annotationsrun at Lines 2779-2781) omits the two new flags, so they default toFalsehere regardless of what the user actually passed via--save-edited-models/--apply-edited-models. An invalid combination (e.g.--apply-edited-models --pre-encoded-models otf, or--save-edited-models --query-mode raw) will pass this guard silently, trigger postgres bootstrap and annotation sync, and only fail later insiderun_evaluation's own_validate_slayer_setupcall (Line 1530-1536) — after those expensive/non-idempotent side effects have already run. This defeats the exact purpose this early check exists for.🐛 Proposed fix
_validate_slayer_setup( slayer_setup=args.slayer_setup, framework=args.framework, query_mode=args.query_mode, mode=args.mode, pre_encoded_source=args.pre_encoded_source, + save_edited_models=args.save_edited_models, + apply_edited_models=args.apply_edited_models, )🤖 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/run.py` around lines 2700 - 2718, The early fail-fast validation in run.py is not passing the new edited-model flags into _validate_slayer_setup, so invalid --save-edited-models/--apply-edited-models combinations can bypass the CLI guard and only fail later after side effects start. Update the _validate_slayer_setup call in the CLI validation block to thread through args.save_edited_models and args.apply_edited_models, matching the programmatic validation path in run_evaluation. Keep the existing validation order and parser.error handling so bad flag combinations are rejected before any bootstrap/sync work begins.
121-169: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject edited-models flags in
oraclemode._validate_slayer_setup()runs before_make_runner(), so--mode oracle --query-mode slayer ... --save-edited-models/--apply-edited-modelsstill passes here and is then silently ignored by the oracle early-return. Add amode == "oracle"check in this guard.🤖 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/run.py` around lines 121 - 169, The _validate_slayer_setup guard in run.py still allows --save-edited-models and --apply-edited-models when mode is oracle, even though _make_runner() later skips them; add an explicit mode == "oracle" rejection in this validation path alongside the existing query_mode/framework checks so edited-models flags are refused before runner creation.src/bird_interact_agents/agents/claude_sdk_otf_ainteract/agent.py (1)
311-332: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winException path loses
edited_models_applied_fromprovenance.The success path (
finalize_with_edited_models_save) stampsrow["edited_models_applied_from"]fromtask_data["_edited_models_applied_from"], but theexceptblock at Line 532 still calls plainfinalize_result_row, so a task that started from an applied archive and then crashed produces a result row with no record of which archive was in use. Given the PR's stated goal of provenance tracking, this asymmetry undermines debuggability of failed apply-runs.🩹 Proposed fix
result = (ctx_dict or {}).get("result") or {} - return finalize_result_row( + row = finalize_result_row( { "task_id": instance_id, ... }, deleted_kb_ids=deleted_kb_ids, slayer_storage_dir=slayer_storage_dir, ) + applied_from = task_data.get("_edited_models_applied_from") + if applied_from: + row["edited_models_applied_from"] = applied_from + return rowAlso applies to: 521-567
🤖 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/agents/claude_sdk_otf_ainteract/agent.py` around lines 311 - 332, The exception path currently drops `edited_models_applied_from` provenance, unlike `finalize_with_edited_models_save`, so failed apply-runs lose the archive origin. Update the `except`/failure handling in `agent.py` to pass through `task_data["_edited_models_applied_from"]` when present and include it in the row built via `finalize_result_row`, matching the success path’s stamping behavior. Keep the fallback behavior unchanged for tasks without that provenance.
🧹 Nitpick comments (3)
tests/test_edited_models_apply.py (1)
120-137: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnused unpacked variable
deleted.Static analysis flags
deletedas unused. Prefix with underscore for clarity.🧹 Proposed fix
- storage, deleted = asyncio.run( + storage, _deleted = asyncio.run(🤖 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_edited_models_apply.py` around lines 120 - 137, The test assigns the second return value from runtime.resolve_otf_task_storage_dir to deleted, but it is never used; update the unpacking in test_runtime_resolver_uses_saved_store to use an underscore-prefixed name instead so the intent is clear and the static analysis warning is resolved.Source: Linters/SAST tools
src/bird_interact_agents/slayer_otf/runtime.py (1)
127-145: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant archive-path recomputation.
run_edited_models_archive(...)is called here purely to re-derive the path thatapply_or_nonealready computed internally (and returned asNone/scratch, discarding the archive path). Consider havingapply_or_none/materialize_from_saved_storereturn the archive path alongside the scratch dir to avoid recomputing it at each call site and to keep the "resolve archive location" logic in one place.🤖 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/slayer_otf/runtime.py` around lines 127 - 145, The edited-models apply flow is recomputing the archive path instead of reusing the value already known by the apply logic. Update `apply_or_none` and/or `materialize_from_saved_store` to return the archive path together with the scratch/applied result, then have the runtime block in `runtime.py` use that returned path for `task_data["_edited_models_applied_from"]` instead of calling `run_edited_models_archive(...)` again. Keep the archive-location resolution centralized and adjust the `log_otf_event`/return handling around `applied` accordingly.src/bird_interact_agents/slayer_otf/edited_models.py (1)
189-207: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the
import ioto the top of the module.
import iois buried insidesave_edited_store; it's a stdlib import used only here, so hoisting it to the top-level import block is a small readability win.🤖 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/slayer_otf/edited_models.py` around lines 189 - 207, Move the stdlib io import out of save_edited_store and into the module’s top-level import block alongside the other imports. Keep the tarfile handling in save_edited_store unchanged, but remove the inline import io near the TarInfo/meta_bytes logic so the function only contains the save flow and the module-level imports are easier to scan.
🤖 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 `@src/bird_interact_agents/slayer_otf/edited_models.py`:
- Around line 144-156: The _checkpoint_wal helper is swallowing sqlite3.Error
and silently continuing, which can leave archived embeddings.db files missing
WAL-backed data; update this function to surface checkpoint failures instead of
ignoring them. Use log_otf_event from edited_models.py to record the db_file and
exception details when sqlite3.connect or PRAGMA wal_checkpoint(TRUNCATE) fails,
while keeping the existing loop over scratch.rglob("*.db") and the non-sqlite
fallback behavior intact.
- Around line 220-280: The fallback extraction in materialize_from_saved_store
uses tarfile.extractall with filter="data", which is not safe on all Python 3.11
runtimes currently allowed by the project. Update this path to either guard the
filter argument based on the interpreter version (and use a compatible fallback
on 3.11.0–3.11.3) or raise the minimum Python requirement so this code only runs
where filter="data" is supported; keep the change localized to
materialize_from_saved_store and its tarfile.open/extractall flow.
---
Outside diff comments:
In `@src/bird_interact_agents/agents/claude_sdk_otf_ainteract/agent.py`:
- Around line 311-332: The exception path currently drops
`edited_models_applied_from` provenance, unlike
`finalize_with_edited_models_save`, so failed apply-runs lose the archive
origin. Update the `except`/failure handling in `agent.py` to pass through
`task_data["_edited_models_applied_from"]` when present and include it in the
row built via `finalize_result_row`, matching the success path’s stamping
behavior. Keep the fallback behavior unchanged for tasks without that
provenance.
In `@src/bird_interact_agents/run.py`:
- Around line 2700-2718: The early fail-fast validation in run.py is not passing
the new edited-model flags into _validate_slayer_setup, so invalid
--save-edited-models/--apply-edited-models combinations can bypass the CLI guard
and only fail later after side effects start. Update the _validate_slayer_setup
call in the CLI validation block to thread through args.save_edited_models and
args.apply_edited_models, matching the programmatic validation path in
run_evaluation. Keep the existing validation order and parser.error handling so
bad flag combinations are rejected before any bootstrap/sync work begins.
- Around line 121-169: The _validate_slayer_setup guard in run.py still allows
--save-edited-models and --apply-edited-models when mode is oracle, even though
_make_runner() later skips them; add an explicit mode == "oracle" rejection in
this validation path alongside the existing query_mode/framework checks so
edited-models flags are refused before runner creation.
---
Nitpick comments:
In `@src/bird_interact_agents/slayer_otf/edited_models.py`:
- Around line 189-207: Move the stdlib io import out of save_edited_store and
into the module’s top-level import block alongside the other imports. Keep the
tarfile handling in save_edited_store unchanged, but remove the inline import io
near the TarInfo/meta_bytes logic so the function only contains the save flow
and the module-level imports are easier to scan.
In `@src/bird_interact_agents/slayer_otf/runtime.py`:
- Around line 127-145: The edited-models apply flow is recomputing the archive
path instead of reusing the value already known by the apply logic. Update
`apply_or_none` and/or `materialize_from_saved_store` to return the archive path
together with the scratch/applied result, then have the runtime block in
`runtime.py` use that returned path for
`task_data["_edited_models_applied_from"]` instead of calling
`run_edited_models_archive(...)` again. Keep the archive-location resolution
centralized and adjust the `log_otf_event`/return handling around `applied`
accordingly.
In `@tests/test_edited_models_apply.py`:
- Around line 120-137: The test assigns the second return value from
runtime.resolve_otf_task_storage_dir to deleted, but it is never used; update
the unpacking in test_runtime_resolver_uses_saved_store to use an
underscore-prefixed name instead so the intent is clear and the static analysis
warning is resolved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7c9f7def-9470-4f18-97d3-ba0b283021fa
📒 Files selected for processing (24)
src/bird_interact_agents/agents/_edited_models_hook.pysrc/bird_interact_agents/agents/claude_sdk_otf/agent.pysrc/bird_interact_agents/agents/claude_sdk_otf_ainteract/agent.pysrc/bird_interact_agents/agents/claude_sdk_otf_ainteract_v1/agent.pysrc/bird_interact_agents/agents/claude_sdk_otf_v1/agent.pysrc/bird_interact_agents/agents/pydantic_ai_recursive/agent.pysrc/bird_interact_agents/cloud/ray_app.pysrc/bird_interact_agents/eval/annotation_io.pysrc/bird_interact_agents/results_db.pysrc/bird_interact_agents/run.pysrc/bird_interact_agents/slayer_otf/datasource_reanchor.pysrc/bird_interact_agents/slayer_otf/edited_models.pysrc/bird_interact_agents/slayer_otf/runtime.pytests/_edited_models_fixtures.pytests/test_claude_sdk_otf_ainteract_v1_agent.pytests/test_claude_sdk_otf_v1_agent.pytests/test_edited_models.pytests/test_edited_models_agent_wiring.pytests/test_edited_models_apply.pytests/test_paths_run_edited_models.pytests/test_recursive_runtime_db_root.pytests/test_results_db_edited_models_cols.pytests/test_run_edited_models_flags.pytests/test_runs_no_annotation_walker_pickup.py
Process-reviews fixes (Codex + CodeRabbit) on PR #84: * collation._row_to_task_result_row now carries edited_models_saved_path / _applied_from — the DEFAULT local process-pool path (and cloud fetch) build the row here, so without it those columns were NULL on every path except the legacy in-process one (Codex, major). * run.py main() fail-fast _validate_slayer_setup now threads the two flags, so a bad combo (e.g. --save-edited-models --query-mode raw) is rejected at the CLI before postgres bootstrap, not deep in run_evaluation (CodeRabbit, major). * edited_models._checkpoint_wal logs a warning on sqlite failure instead of silently swallowing it (a failed checkpoint could drop committed rows from the "self-contained" archive) (CodeRabbit, major). * edited_models materialize gates extractall(filter="data") on tarfile.data_filter presence — PEP 706 landed in 3.11.4 but requires-python admits 3.11.0-3.11.3 (CodeRabbit, minor). * Hoist `import io` to module top; rename unused test unpack to _deleted. Added coverage: collation row round-trip + CLI fast-fail rejection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/test_run_edited_models_flags.py (1)
181-183: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the fail-fast assertion prove evaluation was not reached.
This only checks “some
SystemExit”. Assert code2and makerun_evaluationfail if called, matching the test’s docstring and fail-fast contract.🧪 Proposed test tightening
+ async def fail_run_evaluation(**_kwargs): + pytest.fail("run_evaluation should not be called for invalid flags") + + monkeypatch.setattr(run_module, "run_evaluation", fail_run_evaluation) monkeypatch.setattr(sys, "argv", argv) - with pytest.raises(SystemExit): + with pytest.raises(SystemExit) as exc_info: run_module.main() + assert exc_info.value.code == 2🤖 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_run_edited_models_flags.py` around lines 181 - 183, The fail-fast test around run_module.main currently only asserts that a SystemExit happened, which does not prove evaluation was skipped. Tighten this by asserting the exit code is 2 and by patching run_evaluation in the same test so it raises if invoked, using the existing run_module.main and run_evaluation symbols to verify argument validation stops execution before evaluation starts.
🤖 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 `@src/bird_interact_agents/slayer_otf/edited_models.py`:
- Around line 258-264: The fallback branch in edited_models.py still calls
tar.extractall(work_dir) without any PEP 706 safety checks, so you need to
harden the pre-3.11.4 path in the tar extraction logic. Update the code around
the tarfile.data_filter check to validate all archive members before extraction
on the fallback branch, ensuring paths stay within work_dir and unsafe links or
traversal entries are rejected before calling TarFile.extractall.
---
Nitpick comments:
In `@tests/test_run_edited_models_flags.py`:
- Around line 181-183: The fail-fast test around run_module.main currently only
asserts that a SystemExit happened, which does not prove evaluation was skipped.
Tighten this by asserting the exit code is 2 and by patching run_evaluation in
the same test so it raises if invoked, using the existing run_module.main and
run_evaluation symbols to verify argument validation stops execution before
evaluation starts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 43367d09-60b1-4f2c-a5d7-b1dbae4f37cf
📒 Files selected for processing (6)
src/bird_interact_agents/cloud/collation.pysrc/bird_interact_agents/run.pysrc/bird_interact_agents/slayer_otf/edited_models.pytests/test_edited_models_apply.pytests/test_results_db_edited_models_cols.pytests/test_run_edited_models_flags.py
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/test_results_db_edited_models_cols.py
- tests/test_edited_models_apply.py
- src/bird_interact_agents/run.py
…metry
* materialize_from_saved_store: the pre-3.11.4 fallback now extracts via
_safe_extractall (validates every member stays under work_dir + rejects
links) instead of a bare tar.extractall — closes Ruff S202 / CWE-22 path
traversal (CodeRabbit). filter="data" path unchanged on 3.11.4+/3.12+.
* _checkpoint_wal now emits log_otf_event("otf.edited_models.wal_checkpoint_failed")
on the module's telemetry channel instead of logging.warning, matching the
module idiom (log_otf_event for saved/applied/save_failed) (CodeRabbit).
* Tighten the CLI fail-fast test to assert exit code 2 AND that run_evaluation
is never reached (CodeRabbit nitpick).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
test_trajectory_has_agents_key_and_final_output_excerpt ran in a-interact
mode with model="test" (TestModel), which auto-invokes the ask_user tool.
ask_user's impl (_submit.ask_user_impl) fires a REAL user-simulator LLM
call via the default user_sim_model (haiku) — so this "unit" test (no
integration marker) was silently making a paid, network-dependent Anthropic
call. It only looked green while the account had credits; it 400'd once
they ran out ("credit balance is too low").
Stub factories.ask_user_impl to a canned answer so the test is truly
offline + deterministic (TestModel covered the AGENT model but not the
user-sim). Swept the whole non-integration suite with an autouse blocker on
usage._acompletion + litellm.acompletion/completion: this was the ONLY
offender — 4235 pass with all real completion entrypoints blocked.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds an autouse fixture (sibling to _disable_real_embeddings) that patches the completion seams — usage._acompletion (the user-sim / tracked entry) and litellm.acompletion / litellm.completion — to raise in any test NOT marked @pytest.mark.integration. Tests that legitimately exercise these wrappers stub the same seam themselves (their setattr runs after this autouse fixture and wins); anything that reaches a live call now fails fast with an actionable message instead of silently making a paid, network-dependent Anthropic call. This makes the DEV-1649 postmortem class of bug unrepeatable: a unit test that reaches the wire can no longer masquerade as green while credits last. Integration-marked tests are exempted so they can still hit the real API. Full non-integration suite: 4235 pass with the guard active. Does NOT cover the claude_sdk subprocess path (spawns the `claude` Node CLI, not litellm) — those tests stub the SDK client or are integration-marked. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/conftest.py (1)
301-311: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSilent
except Exception: passmay mask real failures, not just "module not present."Both blocks swallow all exceptions, including genuine bugs (e.g., a broken import chain in
usage), not just the "attribute doesn't exist" case thatraising=Falsealready handles. Ifusageimport ever breaks for an unrelated reason, this guard silently degrades instead of surfacing the problem. Ruff also flags this (S110).🪵 Suggested fix: log instead of silently swallowing
try: from bird_interact_agents import usage as _usage monkeypatch.setattr(_usage, "_acompletion", _aboom, raising=False) - except Exception: # noqa: BLE001 — never block collection on a defensive guard - pass + except Exception: # noqa: BLE001 — never block collection on a defensive guard + logging.getLogger(__name__).warning("Could not patch usage._acompletion", exc_info=True) try: import litellm as _litellm monkeypatch.setattr(_litellm, "acompletion", _aboom, raising=False) monkeypatch.setattr(_litellm, "completion", _boom, raising=False) - except Exception: # noqa: BLE001 — litellm always present; defensive - pass + except Exception: # noqa: BLE001 — litellm always present; defensive + logging.getLogger(__name__).warning("Could not patch litellm.acompletion/completion", exc_info=True)🤖 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/conftest.py` around lines 301 - 311, The defensive try/except blocks in the conftest monkeypatch setup are swallowing all exceptions, which can hide real import or setup failures; update the logic around the bird_interact_agents usage import and the litellm patching so only the expected missing-attribute/module cases are tolerated, and surface or log unexpected exceptions instead of using bare except Exception: pass. Use the existing _usage, _litellm, _acompletion, _aboom, and _boom symbols to locate the affected test fixture code, and keep the monkeypatch.setattr calls with raising=False for the intended fallback behavior.Source: Linters/SAST tools
🤖 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.
Nitpick comments:
In `@tests/conftest.py`:
- Around line 301-311: The defensive try/except blocks in the conftest
monkeypatch setup are swallowing all exceptions, which can hide real import or
setup failures; update the logic around the bird_interact_agents usage import
and the litellm patching so only the expected missing-attribute/module cases are
tolerated, and surface or log unexpected exceptions instead of using bare except
Exception: pass. Use the existing _usage, _litellm, _acompletion, _aboom, and
_boom symbols to locate the affected test fixture code, and keep the
monkeypatch.setattr calls with raising=False for the intended fallback behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dad8a613-61f5-41b8-9241-7fcd89728f38
📒 Files selected for processing (1)
tests/conftest.py
usage / litellm are non-optional core deps, so the defensive try/except: pass around the guard's monkeypatches only hid genuine breakage (a broken import chain would silently disable the guard) and tripped Ruff S110. Drop the catch — a real failure now surfaces loudly. Imports stay inside the fixture to match this conftest's lazy-import style. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jul 14, 2026
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.
What
Two local
bird-interactCLI flags for on-the-fly slayer runs (DEV-1649):--save-edited-models— after a successful run (in-task auditedphase1_passed) that actually changed the per-task SLayer store, snapshot the whole store toruns/<bench>/<db>/<iid>/edited_models.tar.gz(latest-wins overwrite). No-op runs are skipped via a content-manifest baseline captured after re-anchor + HARD-8 mask.--apply-edited-models— start each task from that saved store instead of the bare deterministic cache. Untars, validates_STORE_META(deleted_kb_ids+ cache fingerprint), re-anchors the datasource connection, and does not re-encode KB memories (that would clobber the agent's edits). Falls back to the fresh cache on any miss/mismatch.Design notes
runs/annotation walkers (iter_run_annotations/latest_run_per_instance) userglob("*.json")and the scratch contains_kb_rows.json; a single opaque.tar.gzleaks no*.json, so every current/future walker is inherently safe.claude_sdk_otf{,_v1},claude_sdk_otf_ainteract{,_v1},pydantic_ai_recursive) via a sharedfinalize_with_edited_models_savehook. Rejected for raw /--pre-encoded-models/*_otf_encode.submitnever sets the flags; workers read them with.get(..., False), so cloud behaviour is unchanged. Cloud follow-up left for a later issue.edited_models_saved_path/edited_models_applied_fromstamped ontoresults.db(via the additive-migration path) + the OTF event log.SubmissionAnnotationschema left untouched.slayer_otf.datasource_reanchor(runtime aliases it) to break theruntime↔edited_modelsimport cycle.Tests
51 new tests (7 files + fixtures). Full non-integration suite green (4233 passed). Two prior Codex reviews (plan + tests) folded.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
--save-edited-modelsand--apply-edited-modelsCLI options (with validation for incompatible modes).Bug Fixes
Tests