DEV-1550: cap removal + autopsy retry + N1=ex_base + embedding truncation

[ZmeiGorynych]

Summary

Three layered correctness fixes on top of the DEV-1550 branch.

1. Per-task wall-clock cap default → 0 (uncapped)

• src/bird_interact_agents/run.py: was 900 s (DEV-1535), now 0 (no cap). Rate-limited cloud runs were converting recoverable LLM back-offs into permanent eval_failed rows. BIRD_INTERACT_PER_TASK_TIMEOUT_S still re-enables the cap when needed.
• tests/cloud/test_run_one_task.py: new regression test_per_task_timeout_default_is_uncapped.

2. Autopsy: Pydantic-derived tool schema + corrective retry

• src/bird_interact_agents/eval/autopsy.py: tool schemas now generated from AutopsyLLMOutput / AutopsyLLMOutputOneShot via _pydantic_to_tool_schema ($defs/$ref inlined). Pydantic is the single source of truth.
• run_autopsy does one corrective retry on pydantic.ValidationError, echoing the validation message back as a tool_result with is_error=True so the model can self-correct. Repro: archeology_10 with 353 k-char prompt dropping the required pattern field — recovers cleanly.
• tests/test_autopsy.py: three new tests (retry recovers, retry exhaustion still surfaces validation_error, generated schemas match Pydantic is_required() + no $ref/$defs leftovers).

3. Cascade N1 = upstream ex_base (mini-interact + livesqlbench)

• New src/bird_interact_agents/eval/upstream_ex_base.py wraps both upstream graders (mini-interact / livesqlbench test_utils.py) behind compare_pred_vs_gold_ex_base(). Applies the full test_case_default cleanup (remove_comments / remove_distinct / remove_round), dispatches on benchmark, and conn.rollback()s after every Postgres-side call so pred mutations cannot leak.
• N1 dispatch in tolerant_grader.py calls the shim for {mini-interact, livesqlbench-base-lite-sqlite, livesqlbench-base-lite, livesqlbench-base-full, livesqlbench-large}; falls back to legacy _set_equal on ExBaseUnavailableError, missing SQL, or missing local DB (preserves stubbed-executor test paths).
• Deliberate deviation from upstream: BOTH preprocessed lists empty → True (matches legacy _set_equal([], [])); pinned by a regression test.
• scripts/regrade_n1_ex_base.py: one-shot mini-interact backfill. Re-runs the FULL cascade per stored result so audited tiers / tolerance booleans / verdict / failure_classification all get recomputed consistently. Skips mutation-bearing SQL (is_mutation_sql at statement start) with status state-sensitive, skips missing DB / annotation as missing_inputs. --dry-run flag. Idempotent.

4. Embedding-text truncation

• src/bird_interact_agents/slayer_otf/cache.py: new _truncate_for_embedding pre-truncates rendered memory text to 7800 tokens via tiktoken before embed_batch. Strips provider prefix (openai/...), falls back to cl100k_base for unknown models, fails closed with a logged warning + 28 000-char hard cap when tiktoken is unavailable or the encoder load fails.
• _EMBEDDING_BUILDER_VERSION hashed into _impl_fingerprint_of so already-built caches invalidate automatically on bump — no manual rm -rf _cache_fp.txt.
• Per-truncation logger.warning with memory id, db, original/capped char counts, and model.

Test plan

• pytest tests/cloud/test_run_one_task.py — 10/10
• pytest tests/test_autopsy.py — 123/123 (3 new)
• pytest tests/eval/test_upstream_ex_base.py — 44/44 (new)
• pytest tests/eval/test_n1_dispatch.py — 4/4 (new)
• pytest tests/eval/test_regrade_n1_ex_base.py — 10/10 (new)
• pytest tests/test_slayer_otf_cache_embed_truncate.py — 15/15 (new)
• Full non-integration suite — 2848 passed, 94 skipped, 50 deselected
• Local backfill on archeology_10 (validation_error) recovered with a populated autopsy.analysis.
• Cloud verification: re-submit on one of the 6 embedding-error DBs after rm -rf slayer_otf_cache/<db>/_cache_fp.txt to confirm the truncation path + rebuild.
• Mini-interact N1 backfill: uv run python scripts/regrade_n1_ex_base.py --dry-run, then apply, then re-run aggregate_cascading_latest('mini-interact') and confirm the N6 epsilon delta drops to 0 (the 6+3 cases now pass at N1).

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Autopsy now surfaces and classifies LLM/SDK failures, validates tool inputs, performs one corrective retry on schema validation, and returns richer capped error reports.
  • Per-task wall-clock timeout is uncapped by default.

• New Features

  • CLI to backfill/regrade stored mini-interact results.
  • Optional upstream grading shim to route N1 comparisons to upstream semantics when safe.
  • Pre-truncation of texts before embedding to bound token usage; cache fingerprinting updated.
  • Grade pipeline now forwards optional grading conditions.

• Tests

  • Expanded coverage for autopsy, timeout default, N1 dispatch/upstream shim, regrade script, embedding truncation, and related integration tests.

[linear]

DEV-1550 bird-agents: integrate slayer compact-by-default — populate Memory.description + drill-in prompt nudge

Context

Follow-up to DEV-1549 (slayer compact-by-default search + models_summary + Memory.description field). This issue tracks the bird-agents-side changes that depend on that SLayer release being pinned in uv.lock.

Blocked by: DEV-1549. Do not start until that release ships and is consumable.

Sibling: DEV-1548 (the SLayer-independent disallow list — ships independently, no dependency).

Two coordinated changes

A2. Populate Memory.description during OTF ingestion

Site: src/bird_interact_agents/slayer_otf/kb_memory_encoder.py:104-132 (_build_one).

The function currently builds memory dicts from KB jsonl rows. KB rows already carry a description field (verified by inspection of <db>_kb.jsonl shape). Add one key to the returned dict:

"description": row.get("description", ""),

That's the entire data plumbing — row["description"] already exists; we just thread it through.

Why this matters: DEV-1549's compact mode renders MemoryHit.text as Memory.description (with a first-paragraph fallback when description is empty). Without populating description during OTF ingestion, every bird-interact memory hits the fallback path — wasteful and fragile. Populating it gives the agent the clean, intentional one-line summary on every search hit.

Empty fallback: rows missing description (defensive — shouldn't happen in practice for bird-interact KBs, but possible for adapters consuming other corpora) get an empty string, NOT a KeyError. SLayer's compact renderer then falls back to first-paragraph extraction from learning.

A3. Prompt nudge for entity-filter drill-in

Once search defaults to compact=True, the agent's drill-in pattern (when it needs the full Memory.learning body for a specific memory it has already identified) is:

search(entities=["memory:<id>"], max_memories=1, compact=False)

Three things make this pattern correct:

• entities=["memory:<id>"] — search is already indexed under canonical id memory:<id> (slayer/search/index.py:142), so this acts as an id filter without a new tool.
• max_memories=1 — without this, search returns up to the default 5 hits, polluting the response with adjacent matches.
• compact=False — opts out of the compact default to get the full learning body.

Add a sentence documenting this drill-in pattern to _AINTERACT_SLAYER_TOOLS in src/bird_interact_agents/agents/claude_sdk_otf_ainteract/prompts.py near line 74 (currently the constant that tells the agent to "Call help FIRST to learn the query syntax").

If claude_sdk_otf/prompts.py (one-shot) shares the same SLayer-tools nudge constant or has an analogous one, mirror the addition there.

Snapshot tests in tests/test_shared_otf_prompts.py get rebaselined — SHA-256 of SLAYER_OTF_AINTERACT (and SLAYER_OTF_ONE_SHOT if also touched).

Per feedback_no_prompt_content_tests in memory: no prompt-content behaviour test (no grep on the new wording). The snapshot hash bump is the contract.

Tests

A2 tests

New tests/test_kb_memory_encoder_description.py:

• description field is populated from row["description"] on a happy-path KB row.
• Empty fallback: row without description key produces description: "" (NOT KeyError).
• description is independent of learning — the learning body still gets the verbatim KB block, unchanged from before.
• No regression on entities / id / query / created_at shape.

A3 tests

• Snapshot rebase in tests/test_shared_otf_prompts.py — update both _AINTERACT_SHA256 and (if applicable) _ONE_SHOT_SHA256.
• No new prompt-content assertions.

Verification

1. Full non-integration pytest suite passes:

   env -u SSH_AUTH_SOCK uv run --extra all --extra dev --extra pydantic-ai pytest
   

2. Cloud smoke with the new SLayer version pinned:

   bird-interact-cloud submit --subscription-auth \
     --framework claude_sdk --query-mode slayer --mode a-interact \
     --dataset mini-interact --agent-model anthropic/claude-opus-4-7 \
     --user-sim-model anthropic/claude-sonnet-4-6 --reasoning-effort high \
     --use-audited-gold-sql --slayer-setup on-the-fly \
     --instance-ids alien_1 --workers 1 --actors-per-worker 1 \
     --worker-type e2-standard-8 --max-runtime-hours 1 --detach
   

   Once the run lands:

   • Confirm search tool results in the trajectory show only MemoryHit.description-sized payloads (~one-line summary per hit), NOT the verbose verbatim KB body.
   • Confirm the agent's drill-in pattern (when it needs a full memory) uses search(entities=["memory:<id>"], max_memories=1, compact=False) and gets the full learning body back.

3. Cost comparison vs the pre-DEV-1549 slayer/correct baseline:

   • Expected ~25-30% combined cache_read reduction (DEV-1548 ~4% + DEV-1549's compact ~21-26%).
   • Expected ~$0.50 / correct slayer task savings.
   • Caveat: the savings number depends on DEV-1549's compaction actually being as tight as projected; informational, not a gate.

Critical files

• src/bird_interact_agents/slayer_otf/kb_memory_encoder.py — A2 (one-line change in _build_one)
• src/bird_interact_agents/agents/claude_sdk_otf_ainteract/prompts.py — A3
• src/bird_interact_agents/agents/claude_sdk_otf/prompts.py — A3 (only if the relevant constant lives there too)
• tests/test_kb_memory_encoder_description.py (new) — A2 pinning
• tests/test_shared_otf_prompts.py — A3 snapshot rebase

Coordination with DEV-1549 release

Order of operations:

1. DEV-1549 lands in SLayer + a release is cut.
2. uv.lock bumps to the new SLayer version on this branch.
3. A2 + A3 land.
4. Verification smoke + cost-comparison.

If uv.lock bump produces unexpected breakage outside the compact-mode change (e.g., other SLayer API changes), file a fix issue on the SLayer side rather than working around in bird-agents.

DEV-1557 slayer.embeddings.client.embed_batch: over-cap inputs wipe the whole batch — needs per-text truncation + per-input fallback

Problem

slayer.embeddings.client.embed_batch (slayer/embeddings/client.py:91-130) calls litellm.aembedding(model=..., input=texts) once for the whole batch and catches any exception by returning [None] * len(texts). A single over-cap input poisons the entire batch — every memory loses its embedding row, not just the offender.

Hit this on the bird-interact-agents side for six content-heavy mini-interact databases (robot, fake, crypto, cold_chain_pharma_compliance, exchange_traded_funds, reverse_logistics). OpenAI's text-embedding-3-small caps at 8192 tokens; one rendered memory per DB crosses the line. litellm propagates the OpenAI 400:

litellm.BadRequestError: OpenAIException - Error code: 400 -
{'error': {'message': "Invalid 'input[N]': maximum input length is 8192 tokens."}}

…the bare except Exception swallows it, embed_batch returns all-None. Downstream OTF cache build (bird_interact_agents.slayer_otf.cache._materialise_cache_memories) sees no embeddings persisted, the agent task then fails with eval_failed: no submitted_sql / selected_database because the SLayer setup wasn't created.

We band-aided bird-agents-side by pre-truncating each text via tiktoken to 7800 tokens before calling embed_batch (MotleyAI/bird-agents#46). That's a workaround — SLayer is the right place to handle this. Downstream callers shouldn't need to know token budgets for every embedding model.

Suggested fixes (one of these, or a combination)

1. Per-text token-aware truncation inside embed_batch

• Look up the model's context cap via litellm.utils.get_max_tokens(resolved_model) — fall back to 8192 for unknown embedding models.
• Per text, encode with tiktoken (encoding_for_model → fall back to cl100k_base), slice tokens to cap - margin, decode.
• Identity-return when already short, so short inputs incur no encode/decode round-trip.

2. Per-input retry on BadRequestError (length-related only)

When the batch raises litellm.BadRequestError, fall back to embedding each text individually so good inputs survive. Optionally pair with (1) so retry sees pre-truncated text.

3. Surface the failure shape so callers can disambiguate

Catch length errors separately from rate-limit / auth / network errors, log distinctly, and consider returning a more typed result (e.g. EmbeddingInputTooLong(idx)) so the cache builder can act per-input.

Repro

import asyncio
from slayer.embeddings.client import embed_batch

short = "hi"
overlong = "word " * 12_000  # > 8192 tokens for text-embedding-3-small

vectors = asyncio.run(embed_batch([short, overlong], model="openai/text-embedding-3-small"))
print(vectors)  # both None — `short` is collateral damage

Cross-reference

• bird-agents workaround: bird_interact_agents/slayer_otf/cache.py:_truncate_for_embedding + per-batch warning log with memory id + db. PR: MotleyAI/bird-agents#46

Why this matters operationally

The all-None batch silently turns into an eval_failed task downstream, which on a long benchmark run looks like an agent failure (no submitted SQL) when it's actually an embedding-time pipeline failure. Six entire DBs of our mini-interact benchmark got knocked out by this until the workaround landed.

Review in Linear

[coderabbitai]

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Updates autopsy schema generation and retry/error handling, adds upstream EX-Base N1 grading plus a mini-interact regrade script and tests, removes the default per-task timeout cap, and truncates cache embedding text before embedding with fingerprint-aware rebuilds.

Changes

Autopsy validation retry and schema generation

Layer / File(s)	Summary
Pydantic-derived tool schema generation src/bird_interact_agents/eval/autopsy.py	Adds helpers that inline Pydantic JSON schema refs and derive Anthropic tool input schemas for the standard and one-shot autopsy outputs.
run_autopsy retry and error handling src/bird_interact_agents/eval/autopsy.py	Preinitializes reporting state, guards setup in a prep phase, validates tool input with Pydantic, retries once with an error tool_result after the first validation failure, and returns categorized AutopsyError results for failures.
Autopsy retry and validation tests tests/test_autopsy.py	Adds coverage for the one-shot corrective retry, repeated validation failure returning kind="validation_error", and schema generation matching Pydantic-required fields without unresolved refs.

Per-task timeout reconfiguration

Layer / File(s)	Summary
Default timeout configuration src/bird_interact_agents/run.py	Sets _DEFAULT_PER_TASK_TIMEOUT_S to 0.0 instead of 900.0 and updates the timeout docstring to describe uncapped default behavior.
Timeout default test tests/cloud/test_run_one_task.py	Adds a test verifying _per_task_timeout_s() stays uncapped when BIRD_INTERACT_PER_TASK_TIMEOUT_S is unset.

EX-Base shim, tolerant grader dispatch, and regrade script

Layer / File(s)	Summary
EX-Base shim and mutation detection src/bird_interact_agents/eval/upstream_ex_base.py	Adds the upstream EX-Base shim, benchmark-specific lazy loading, mutation SQL detection, loader cache isolation for db_utils, rollback and commit handling, and the empty-results pass case.
N1 dispatch into upstream ex_base src/bird_interact_agents/eval/tolerant_grader.py	Adds _compute_n1, forwards conditions, and routes supported N1 comparisons through the upstream shim while falling back to legacy _set_equal when gating checks fail.
Regrade CLI and core processing scripts/regrade_n1_ex_base.py	Adds a CLI that scans stored mini-interact results, reloads task inputs from task JSON or canonical JSONL, skips missing or state-sensitive cases, recomputes grading, and rewrites changed JSON unless --dry-run is used.
N1 dispatch tests tests/eval/test_n1_dispatch.py	Adds tests for supported benchmark routing, unsupported benchmark fallback, unavailable EX-Base fallback, forwarded conditions, Postgres db_path.stem naming, and mutation-based skip behavior.
Regrade script tests tests/eval/test_regrade_n1_ex_base.py	Adds fixtures and subprocess-based coverage for cascade flips, idempotency, mutation skips, non-mini-interact refusal, DB path resolution, dry-run behavior, stale tolerance recomputation, annotation fallback, and missing DB handling.
EX-Base shim tests tests/eval/test_upstream_ex_base.py	Adds tests for comparison semantics, cleanup symmetry, dispatch routing, rollback behavior, loader error mapping, and module cache isolation across upstream trees.

Embedding truncation and cache fingerprinting

Layer / File(s)	Summary
Embedding truncation and fingerprint src/bird_interact_agents/slayer_otf/cache.py	Adds _truncate_for_embedding, truncates rendered memory text before embed_batch, logs truncation warnings, and includes the embed-builder version in _impl_fingerprint_of.
Embedding truncation tests tests/test_slayer_otf_cache_embed_truncate.py	Adds unit and integration tests for token-budget truncation, fallback behavior when tiktoken is unavailable or fails, warning logs, truncated embed inputs, and fingerprint stability and invalidation.

Sequence Diagram(s)

sequenceDiagram
  participant run_autopsy
  participant Anthropic as AnthropicClient
  participant Pydantic as PydanticValidator
  run_autopsy->>run_autopsy: prepare KB, prompt, tool schema
  run_autopsy->>Anthropic: send tool request
  Anthropic-->>run_autopsy: tool_use output
  run_autopsy->>Pydantic: validate tool input
  alt first validation error
    run_autopsy->>Anthropic: send tool_result is_error=True
    Anthropic-->>run_autopsy: corrected tool_use output
    run_autopsy->>Pydantic: validate retry input
  end
  alt retry still invalid
    run_autopsy-->>run_autopsy: return validation_error result
  else valid input
    run_autopsy-->>run_autopsy: map output and return analysis
  end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• MotleyAI/bird-agents#39: The main PR’s run_autopsy changes (Pydantic-derived one-shot tool schemas, typed AutopsyError, and retry/validation-error handling in src/bird_interact_agents/eval/autopsy.py) are directly aligned with DEV-1541’s autopsy one-shot split and error capture.
• MotleyAI/bird-agents#20: Both PRs modify grade_and_write/grade_one_submission wiring; this PR adds conditions forwarding which touches the same callsites as the referenced PR.

"I'm a rabbit in a noisy code glen,
I inlined schemas and fixed the when.
One retry later the output was right,
N1 got routed and regraded at night.
Embeds trimmed short, caches rebuilt bright."

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 73.95% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically summarizes the four main changes: cap removal, autopsy retry logic, N1 grading via upstream ex_base, and embedding truncation.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

tests/test_autopsy.py (1)

1459-1460: 💤 Low value

Consider splitting compound statements for readability.

While syntactically valid, combining statements with semicolons reduces readability in test code where clarity is paramount.

♻️ Optional refactor for clarity

-    r1 = _stub_tool_use(bad_input_1); r1.content[0].id = "tu_1"
-    r2 = _stub_tool_use(bad_input_2); r2.content[0].id = "tu_2"
+    r1 = _stub_tool_use(bad_input_1)
+    r1.content[0].id = "tu_1"
+    r2 = _stub_tool_use(bad_input_2)
+    r2.content[0].id = "tu_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_autopsy.py` around lines 1459 - 1460, Split the compound
statements into separate lines for clarity: replace "r1 =
_stub_tool_use(bad_input_1); r1.content[0].id = 'tu_1'" with two statements
assigning r1 then setting r1.content[0].id, and do the same for
r2/_stub_tool_use(bad_input_2) so each operation is on its own line (references:
_stub_tool_use, r1, r2, bad_input_1, bad_input_2).

🤖 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/eval/autopsy.py`:
- Around line 475-497: In _inline_refs, add defensive handling when resolving a
$ref that points to a non-existent $defs entry: inside resolve (used by
_inline_refs) check that the extracted key exists in defs before returning
defs[key]; if it's missing, raise a clear exception (e.g., ValueError or
KeyError) with a message that includes the missing ref key and the list of
available defs keys to aid debugging, or alternatively use defs.get and raise a
descriptive error; ensure this change references the existing symbols:
_inline_refs, defs, resolve, and the local variable key.

---

Nitpick comments:
In `@tests/test_autopsy.py`:
- Around line 1459-1460: Split the compound statements into separate lines for
clarity: replace "r1 = _stub_tool_use(bad_input_1); r1.content[0].id = 'tu_1'"
with two statements assigning r1 then setting r1.content[0].id, and do the same
for r2/_stub_tool_use(bad_input_2) so each operation is on its own line
(references: _stub_tool_use, r1, r2, bad_input_1, bad_input_2).

🪄 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: b8ed5e5d-0e29-4048-bd13-b10f7768c871

📥 Commits

Reviewing files that changed from the base of the PR and between 710ed30 and 62f7d16.

📒 Files selected for processing (4)

• src/bird_interact_agents/eval/autopsy.py
• src/bird_interact_agents/run.py
• tests/cloud/test_run_one_task.py
• tests/test_autopsy.py

[coderabbitai]

Actionable comments posted: 4

🤖 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/regrade_n1_ex_base.py`:
- Around line 291-299: The code only increments report.processed after detecting
a flip, so unchanged (idempotent) regrades are not counted; update the logic in
the block around flipped/_evaluation_diffs so that report.processed is
incremented for every regrade attempt (either move report.processed += 1 before
the flipped check or add report.processed += 1 inside the branch where
report.regraded_unchanged is incremented), keeping the existing dry_run handling
(n1_before = before.evaluation.phase1_against_original_gold) intact; adjust in
scripts/regrade_n1_ex_base.py around the flipped variable and
report.regraded_unchanged/dry_run usage.

In `@src/bird_interact_agents/eval/tolerant_grader.py`:
- Around line 239-277: The call to compare_pred_vs_gold_ex_base in _compute_n1
currently hardcodes conditions=None and db_name=str(db_path); change it to
forward benchmark-specific EX-Base args by reading attributes off the benchmark
object (e.g., use db_name = getattr(benchmark, "db_name", None) or fallback to
str(db_path) only if benchmark.db_name is not set, and conditions =
getattr(benchmark, "ex_base_conditions", None)); then call
compare_pred_vs_gold_ex_base with those variables (keep conn, pred_sqls,
sol_sqls as-is) so ordered comparisons and Postgres/database-name semantics are
preserved.

In `@src/bird_interact_agents/eval/upstream_ex_base.py`:
- Around line 99-115: The loader currently mutates sys.path and then execs the
upstream module, causing bare "from db_utils import ..." to resolve to a shared
sys.modules["db_utils"]; modify _load_module_from_file to isolate each upstream
tree's db_utils by temporarily swapping sys.modules["db_utils"] (or inserting a
tree-specific key like f"db_utils_{name}") before calling
spec.loader.exec_module(mod) and restoring the original entry afterward (ensure
restoration on exceptions); reference _load_module_from_file,
sys.modules["db_utils"], and spec.loader.exec_module(mod) when making the
change.

In `@tests/test_slayer_otf_cache_embed_truncate.py`:
- Around line 428-431: The test currently uses substring checks if "a" in msg
and if "b" in msg which can match unrelated text; change these to exact ID
checks by matching the token or pattern for the memory id instead (replace if
"a" in msg / if "b" in msg with either equality checks against the specific log
token or a regex with word boundaries like r'\ba\b' / r'\bb\b'), updating the
ids_seen set only when the exact memory-id is found; refer to the existing
ids_seen and msg variables and replace those two conditional checks accordingly.

🪄 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: 16908019-80c2-4006-8d76-d695be3554ce

📥 Commits

Reviewing files that changed from the base of the PR and between 62f7d16 and 397493c.

📒 Files selected for processing (8)

• scripts/regrade_n1_ex_base.py
• src/bird_interact_agents/eval/tolerant_grader.py
• src/bird_interact_agents/eval/upstream_ex_base.py
• src/bird_interact_agents/slayer_otf/cache.py
• tests/eval/test_n1_dispatch.py
• tests/eval/test_regrade_n1_ex_base.py
• tests/eval/test_upstream_ex_base.py
• tests/test_slayer_otf_cache_embed_truncate.py

[coderabbitai]

🧹 Nitpick comments (2)

tests/eval/test_n1_dispatch.py (1)

183-220: 💤 Low value

Minor comment inaccuracy on line 205.

The comment states "presence required by the conn-fallback guard" but since conn=MagicMock() is truthy, the if conn is None block at _compute_n1 line 290 is skipped entirely. The file creation is defensive (good habit) but not actually required for this test to pass.

🤖 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/eval/test_n1_dispatch.py` around lines 183 - 220, The inline comment on
the db_path.write_bytes call in
test_n1_dispatch_uses_db_stem_for_postgres_benchmarks is inaccurate: because the
test passes conn=MagicMock() the conn-is-None branch in _compute_n1 is never
executed, so the file creation is only defensive, not required; update the
comment to reflect that (e.g., "file presence is defensive; conn is provided so
conn-fallback is skipped") or remove the write_bytes call if you prefer to keep
the test minimal; reference the test function name
test_n1_dispatch_uses_db_stem_for_postgres_benchmarks and the helper _compute_n1
when making the change.

tests/eval/test_upstream_ex_base.py (1)

636-656: ⚡ Quick win

Save and restore prior sys.modules["db_utils"] state for consistency.

Test 1 (test_load_module_from_file_isolates_db_utils_between_upstream_trees) follows the defensive pattern of saving any pre-existing sys.modules["db_utils"] before manipulation and restoring it in the finally block. This test should adopt the same pattern for consistency and test isolation robustness.

Currently, if another test leaves a db_utils module in sys.modules, this test overwrites it (line 638) and never restores it (lines 655–656 unconditionally pop it).

🛡️ Proposed fix to match test 1's save/restore pattern

+    prior_db_utils = sys.modules.get("db_utils")
     sentinel = types.ModuleType("db_utils")
     sentinel.ORIGIN = "caller_sentinel"  # type: ignore[attr-defined]
     sys.modules["db_utils"] = sentinel
 
     tree = tmp_path / "tree"
     tree.mkdir()
     (tree / "db_utils.py").write_text("ORIGIN = 'tree_internal'\n")
     (tree / "test_utils.py").write_text(
         "from db_utils import ORIGIN\nWHICH = ORIGIN\n"
     )
     try:
         m = mod._load_module_from_file(
             "test_utils_iso", tree / "test_utils.py",
             sys_path_addition=tree,
         )
         assert m.WHICH == "tree_internal"
         # Post-load: the caller's sentinel is restored.
         assert sys.modules["db_utils"] is sentinel
     finally:
-        sys.modules.pop("db_utils", None)
+        if prior_db_utils is not None:
+            sys.modules["db_utils"] = prior_db_utils
+        else:
+            sys.modules.pop("db_utils", None)
         sys.modules.pop("test_utils_iso", None)

🤖 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/eval/test_upstream_ex_base.py` around lines 636 - 656, This test
overwrites sys.modules["db_utils"] without preserving any pre-existing module;
save the prior value (e.g., original_db_utils = sys.modules.get("db_utils"))
before assigning sentinel, then in the finally block restore it: if
original_db_utils is not None reassign sys.modules["db_utils"] =
original_db_utils else pop it (sys.modules.pop("db_utils", None)); keep the
existing cleanup of "test_utils_iso" and ensure the restoration happens
regardless of load success (the block around mod._load_module_from_file and the
sentinel assignment should reference the sentinel and original_db_utils
symbols).

🤖 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/eval/test_n1_dispatch.py`:
- Around line 183-220: The inline comment on the db_path.write_bytes call in
test_n1_dispatch_uses_db_stem_for_postgres_benchmarks is inaccurate: because the
test passes conn=MagicMock() the conn-is-None branch in _compute_n1 is never
executed, so the file creation is only defensive, not required; update the
comment to reflect that (e.g., "file presence is defensive; conn is provided so
conn-fallback is skipped") or remove the write_bytes call if you prefer to keep
the test minimal; reference the test function name
test_n1_dispatch_uses_db_stem_for_postgres_benchmarks and the helper _compute_n1
when making the change.

In `@tests/eval/test_upstream_ex_base.py`:
- Around line 636-656: This test overwrites sys.modules["db_utils"] without
preserving any pre-existing module; save the prior value (e.g.,
original_db_utils = sys.modules.get("db_utils")) before assigning sentinel, then
in the finally block restore it: if original_db_utils is not None reassign
sys.modules["db_utils"] = original_db_utils else pop it
(sys.modules.pop("db_utils", None)); keep the existing cleanup of
"test_utils_iso" and ensure the restoration happens regardless of load success
(the block around mod._load_module_from_file and the sentinel assignment should
reference the sentinel and original_db_utils symbols).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6a283353-508a-43ee-8e26-d6e495a5cba4

📥 Commits

Reviewing files that changed from the base of the PR and between 397493c and 7983a15.

📒 Files selected for processing (8)

• scripts/regrade_n1_ex_base.py
• src/bird_interact_agents/eval/autopsy.py
• src/bird_interact_agents/eval/tolerant_grader.py
• src/bird_interact_agents/eval/upstream_ex_base.py
• tests/eval/test_n1_dispatch.py
• tests/eval/test_regrade_n1_ex_base.py
• tests/eval/test_upstream_ex_base.py
• tests/test_slayer_otf_cache_embed_truncate.py

🚧 Files skipped from review as they are similar to previous changes (5)

• scripts/regrade_n1_ex_base.py
• src/bird_interact_agents/eval/autopsy.py
• src/bird_interact_agents/eval/upstream_ex_base.py
• tests/test_slayer_otf_cache_embed_truncate.py
• tests/eval/test_regrade_n1_ex_base.py

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

tests/eval/test_regrade_n1_ex_base.py (1)

552-552: 💤 Low value

Optional: Make assertion more explicit for easier debugging.

The combined and assertion works but can be split for clearer failure messages.

💡 Proposed refactor for clarity

-    assert seen_conditions and seen_conditions[0] == {"order": True}
+    assert seen_conditions, "grade_submission was never called"
+    assert seen_conditions[0] == {"order": True}, (
+        f"Expected conditions={{'order': True}}, got {seen_conditions[0]}"
+    )

This makes failures self-documenting: the first assertion explains that the function wasn't called, and the second shows exactly what was passed.

🤖 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/eval/test_regrade_n1_ex_base.py` at line 552, Split the combined
assertion into two clear assertions: first check that seen_conditions is truthy
(e.g., assert seen_conditions with a message like "handler was not called;
seen_conditions is empty"), then assert the exact value with assert
seen_conditions[0] == {"order": True} (optionally adding a message like
f"unexpected conditions: {seen_conditions[0]}"). Update the assertions around
seen_conditions in test_regrade_n1_ex_base.py so failures report whether the
handler was never invoked versus the wrong payload.

🤖 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 `@tests/eval/test_regrade_n1_ex_base.py`:
- Around line 537-540: The test mutates sys.path and imports regrade_n1_ex_base
without cleanup or reload, causing pollution and stale cached modules; fix by
inserting the path and importing inside a try/finally where you remove the
inserted path in the finally block, and before using
importlib.import_module("regrade_n1_ex_base") call importlib.reload if the
module is already present in sys.modules to ensure a fresh import (check
sys.modules for "regrade_n1_ex_base"), or pop the module from sys.modules before
importing; ensure these changes are applied around the existing
sys.path.insert(0, ...) and importlib.import_module calls to guarantee
isolation.

---

Nitpick comments:
In `@tests/eval/test_regrade_n1_ex_base.py`:
- Line 552: Split the combined assertion into two clear assertions: first check
that seen_conditions is truthy (e.g., assert seen_conditions with a message like
"handler was not called; seen_conditions is empty"), then assert the exact value
with assert seen_conditions[0] == {"order": True} (optionally adding a message
like f"unexpected conditions: {seen_conditions[0]}"). Update the assertions
around seen_conditions in test_regrade_n1_ex_base.py so failures report whether
the handler was never invoked versus the wrong payload.

🪄 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: 76b1f640-58b6-438e-af93-68d764cfb106

📥 Commits

Reviewing files that changed from the base of the PR and between 7983a15 and 4e847b2.

📒 Files selected for processing (6)

• scripts/regrade_n1_ex_base.py
• src/bird_interact_agents/eval/grade_in_place.py
• src/bird_interact_agents/eval/upstream_ex_base.py
• tests/eval/test_n1_dispatch.py
• tests/eval/test_regrade_n1_ex_base.py
• tests/eval/test_upstream_ex_base.py

🚧 Files skipped from review as they are similar to previous changes (1)

• scripts/regrade_n1_ex_base.py