bird-agents: drop --ingest-on-startup on OTF slayer MCP launch (DEV-1508)

[ZmeiGorynych]

Summary

• Fixes DEV-1508: per-task slayer stdio MCP stuck in status='pending' for 5 of 10 LiveSQLBench museum tasks under claude_sdk_otf. Agent silently lost every mcp__slayer__* tool; ~$17 / $39 burned on a single run.
• Root cause: slayer_mcp_stdio_config unconditionally passed --ingest-on-startup. On a warm OTF cache that's 30-50s of wasted schema reflection + SELECT DISTINCT sample-value refresh + embedding hash-walks. The Claude Agent SDK has no MCP startup-timeout knob (unlike pydantic-ai's MCPServerStdio(timeout=…)), so it proceeds with pending — no recovery.
• Fix: opt-out ingest_on_startup kwarg (default True; committed-reference adapters unchanged). Three OTF call sites pass False: claude_sdk_otf, pydantic_ai_otf_encode, pydantic_ai_recursive (gated on slayer_setup).
• Safety net (Codex review): persist a second _impl_fp.txt marker carrying only impl-version components (slayer.__version__ + active embedding model). On reuse, the full fingerprint stays presence-gated (cloud lifecycle preserved), but the impl half IS recomputed and forces a rebuild on drift.

Test plan

• Unit: tests/test_slayer_mcp_config.py — 4 new tests pin the kwarg contract (default includes flag; False omits it; env unaffected; keyword-only).
• Unit: tests/test_slayer_otf_cache.py — 7 new tests cover the impl-fp split (marker written; helper excludes inputs/root; embed-model change; slayer-version mismatch rebuild — all 3 phases; embed-model mismatch rebuild — all 3 phases; happy-path fast reuse; no full-fp recompute regression).
• Unit: tests/test_mcp_startup_timeout.py — _build_shared_slayer_server tests for pydantic_ai_otf_encode and pydantic_ai_recursive; AST-level call-site test on pydantic_ai_recursive.run_task that enforces the slayer_setup != "on-the-fly" polarity.
• Unit: tests/test_claude_sdk_otf_agent.py — drives run_task with a spy that captures slayer_mcp_stdio_config(**kw); asserts ingest_on_startup is False.
• Full non-integration suite: 1515 passed, 94 skipped, 19 deselected.
• Cloud smoke (per acceptance criteria): re-run museum_{1,2,3,4,10} with claude_sdk_otf + Opus + one-shot + --slayer-setup on-the-fly. Assert every init SystemMessage reports mcp_servers.slayer.status == 'connected'; ≥1 mcp__slayer__inspect_model per trajectory; no Ingesting datasource 'museum'… lines in slayer stderr.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Refactor

  • Make SLAYER server startup ingestion configurable (can disable ingest-on-startup)
  • Tighten cache reuse with an implementation-fingerprint marker to avoid incompatible reuse

• Tests

  • Add/expand tests covering startup-ingest behavior, server config, and cache impl-fingerprint races
  • Add import-isolation and performance helpers to test harness

• Documentation

  • Clarify startup timeout and parameter behavior in SLAYER startup docs

[linear]

DEV-1508

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a keyword-only ingest_on_startup flag to slayer MCP stdio config (default True) and threads False from on-the-fly agent paths; introduces an implementation-only _impl_fp.txt marker and matching reuse logic in the OTF cache; updates tests and fixtures accordingly.

Changes

On-the-fly encoding optimization

Layer / File(s)	Summary
MCP ingest_on_startup flag implementation src/bird_interact_agents/harness.py	slayer_mcp_stdio_config now accepts keyword-only ingest_on_startup: bool = True and appends --ingest-on-startup to args only when True; startup timeout docs updated.
MCP ingest_on_startup wiring across agent adapters src/bird_interact_agents/agents/claude_sdk_otf/agent.py, src/bird_interact_agents/agents/claude_sdk_otf_ainteract/agent.py, src/bird_interact_agents/agents/pydantic_ai_otf_encode/agent.py, src/bird_interact_agents/agents/pydantic_ai_recursive/agent.py	Claude OTF adapters call slayer_mcp_stdio_config(..., ingest_on_startup=False) for per-task MCP servers; pydantic recursive helper accepts ingest_on_startup and passes self.slayer_setup != "on-the-fly" to it; pydantic OTF encode adds DEV-1508 comments.
MCP configuration unit tests tests/test_slayer_mcp_config.py	Tests added for default inclusion of --ingest-on-startup, omission when ingest_on_startup=False, env/command invariance, and keyword-only enforcement.
Claude SDK OTF agent MCP tests tests/test_claude_sdk_otf_agent.py	Test stubs updated to capture slayer_mcp_stdio_config kwargs and a new test asserts the agent sets ingest_on_startup=False; import-isolation test refactored to use fixture.
Claude OTF ainteract agent tests tests/test_claude_sdk_otf_ainteract_agent.py	Stubs capture MCP kwargs; new test asserts ingest_on_startup=False; import-isolation test refactored to shared fixture.
MCP ingest_on_startup integration tests tests/test_mcp_startup_timeout.py	Monkeypatch spy helper and tests verify pydantic OTF encode default True, recursive helper forwards False for on-the-fly, preserves True by default, and an AST-based test confirms the self.slayer_setup != "on-the-fly" wiring in run_task.
Test fixtures and import isolation tests/conftest.py	Adds session-scoped caching for YAMLStorage reads and an import_isolation_results fixture that runs three import checks in a subprocess and reports results to tests.
Implementation fingerprint marker and reuse validation src/bird_interact_agents/slayer_otf/cache.py	Adds _impl_fp.txt (_IMPL_MARKER) and _impl_fingerprint_of() (slayer version + embedding model); ensure_db_cache requires both _cache_fp.txt and matching _impl_fp.txt for reuse; writes _impl_fp.txt before _cache_fp.txt and tightens concurrency/rename handling.
Implementation fingerprint marker tests tests/test_slayer_otf_cache.py	Extensive tests added: peer-rename race cases that require matching impl marker, stale-peer re-raise, marker write and migration tests, reuse rebuilds on slayer/version or embedding-model mismatches, fast-path no-rebuild when impl matches, and legacy-cache migration behavior.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• MotleyAI/bird-agents#10: touches Claude SDK on-the-fly agent implementations and MCP wiring similar to the ingest_on_startup changes.
• MotleyAI/bird-agents#9: earlier PR introducing ClaudeSDKOtfAgent; related to the same run_task MCP setup path.
• MotleyAI/bird-agents#2: modifies on-the-fly SLAYER cache reuse logic and is code-level related to the impl-fingerprint changes.

Poem

🐰 I dug through caches, flags in paw,
Wrote tiny markers that say what I saw,
MCP whispers "don't ingest" for OTF runs,
Fingerprints split so reuse hums,
Hooray — faster hops for encoding fun!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 68.18% 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 accurately describes the main objective: dropping the --ingest-on-startup flag for on-the-fly (OTF) slayer MCP launches, and references the ticket DEV-1508. The changeset shows this flag is being made configurable and disabled for OTF workflows in multiple adapters, which directly aligns with the title's claim.
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.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch egor/dev-1508-slayer-mcp-fails-to-start-on-livesqlbench-museum-tasks-4-of

---

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

[coderabbitai]

🧹 Nitpick comments (1)

tests/test_slayer_otf_cache.py (1)

559-570: 💤 Low value

Consider adding a test for legacy cache migration (no _impl_fp.txt).

The _impl_ok() helper returns False when _impl_fp.txt is missing (treating legacy caches as mismatched). An explicit test for this scenario would strengthen coverage: create a cache dir with only _cache_fp.txt and _kb_rows.json (simulating a pre-split cache), then verify ensure_db_cache triggers a rebuild.

🤖 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_slayer_otf_cache.py` around lines 559 - 570, Add a test that
simulates a legacy cache missing IMPL_MARKER by creating a temp cache directory
containing only the old `_cache_fp.txt` and `_kb_rows.json` files and asserting
that `_impl_ok()` returns False and `ensure_db_cache(...)` performs a rebuild
(e.g., replaces files or returns the rebuild indicator). Locate helpers
`_impl_ok()` and `ensure_db_cache()` in the test module, create the legacy files
with valid contents matching the old fingerprint, call `_impl_ok(cache_dir)` to
assert False, then call `ensure_db_cache(cache_dir, ...)` and assert the
expected rebuild behavior (new `_impl_fp.txt` created or rebuild
flag/side-effect observed).

🤖 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/test_slayer_otf_cache.py`:
- Around line 559-570: Add a test that simulates a legacy cache missing
IMPL_MARKER by creating a temp cache directory containing only the old
`_cache_fp.txt` and `_kb_rows.json` files and asserting that `_impl_ok()`
returns False and `ensure_db_cache(...)` performs a rebuild (e.g., replaces
files or returns the rebuild indicator). Locate helpers `_impl_ok()` and
`ensure_db_cache()` in the test module, create the legacy files with valid
contents matching the old fingerprint, call `_impl_ok(cache_dir)` to assert
False, then call `ensure_db_cache(cache_dir, ...)` and assert the expected
rebuild behavior (new `_impl_fp.txt` created or rebuild flag/side-effect
observed).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1dc01174-8d44-489f-9eda-026fb9ebbbd9

📥 Commits

Reviewing files that changed from the base of the PR and between 0fb3ad5 and 6a02056.

📒 Files selected for processing (9)

• src/bird_interact_agents/agents/claude_sdk_otf/agent.py
• src/bird_interact_agents/agents/pydantic_ai_otf_encode/agent.py
• src/bird_interact_agents/agents/pydantic_ai_recursive/agent.py
• src/bird_interact_agents/harness.py
• src/bird_interact_agents/slayer_otf/cache.py
• tests/test_claude_sdk_otf_agent.py
• tests/test_mcp_startup_timeout.py
• tests/test_slayer_mcp_config.py
• tests/test_slayer_otf_cache.py

[coderabbitai]

🧹 Nitpick comments (1)

src/bird_interact_agents/harness.py (1)

699-699: 💤 Low value

Consider using paths.*_root() for repo root resolution.

Line 699 manually calculates the repo root as Path(__file__).resolve().parent.parent.parent. As per coding guidelines, reachable-from-Python paths should be resolved via bird_interact_agents.paths.*_root() helpers which anchor at the main checkout via git's common dir. This ensures the code works correctly from any checkout (canonical or git worktree).

♻️ Proposed refactor using paths helper

Check if there's an existing paths.repo_root() or similar helper. If available:

-    repo_root = Path(__file__).resolve().parent.parent.parent
+    from bird_interact_agents import paths
+    repo_root = paths.repo_root()

If no such helper exists, the current approach is acceptable for this localized use case.

As per coding guidelines: "All reachable-from-Python data paths must be resolved via bird_interact_agents.paths.*_root() helpers which anchor at the main checkout via git's common dir."

🤖 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/harness.py` at line 699, Replace the manual repo
root computation (repo_root = Path(__file__).resolve().parent.parent.parent)
with the central path helper; locate where repo_root is set in harness.py and
call the appropriate bird_interact_agents.paths.*_root() helper (e.g.,
bird_interact_agents.paths.repo_root()) so the repo root is anchored via git
common dir and works with worktrees.

🤖 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 `@src/bird_interact_agents/harness.py`:
- Line 699: Replace the manual repo root computation (repo_root =
Path(__file__).resolve().parent.parent.parent) with the central path helper;
locate where repo_root is set in harness.py and call the appropriate
bird_interact_agents.paths.*_root() helper (e.g.,
bird_interact_agents.paths.repo_root()) so the repo root is anchored via git
common dir and works with worktrees.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 19e8e88a-bcbf-44ca-84dc-03e8f211cdba

📥 Commits

Reviewing files that changed from the base of the PR and between 6a02056 and 6bc84d3.

📒 Files selected for processing (11)

• src/bird_interact_agents/agents/claude_sdk_otf/agent.py
• src/bird_interact_agents/agents/claude_sdk_otf_ainteract/agent.py
• src/bird_interact_agents/agents/pydantic_ai_otf_encode/agent.py
• src/bird_interact_agents/harness.py
• src/bird_interact_agents/slayer_otf/cache.py
• tests/conftest.py
• tests/test_claude_sdk_otf_agent.py
• tests/test_claude_sdk_otf_ainteract_agent.py
• tests/test_harness_imports.py
• tests/test_mcp_startup_timeout.py
• tests/test_slayer_otf_cache.py

✅ Files skipped from review due to trivial changes (1)

• src/bird_interact_agents/agents/pydantic_ai_otf_encode/agent.py

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

• src/bird_interact_agents/agents/claude_sdk_otf/agent.py
• src/bird_interact_agents/slayer_otf/cache.py