DEV-1462: LiveSQLBench-Base-Lite-SQLite one-shot harness - #5
Merged
ZmeiGorynych merged 10 commits intoMay 28, 2026
Merged
Conversation
Add a new `--mode one-shot` evaluation path that runs the
LiveSQLBench-Base-Lite-SQLite dataset (the unambiguous, one-shot sibling
of mini-interact) against BOTH SLayer agents — `pydantic_ai_recursive`
(KB→memories) and `pydantic_ai_otf_encode` (KB→models). Scored on the
180 SELECT (`category=="Query"`) tasks; no user simulator, no
`ask_user` anywhere in the spawn tree.
Highlights (full plan in DEV-1462's "Final approved plan" section):
* New `--dataset {mini-interact, livesqlbench}` and `--gold-file` flags.
`--mode one-shot` is gated to `--dataset livesqlbench`; `--dataset
livesqlbench` is gated to `--mode {one-shot, oracle}`; one-shot
requires `--slayer-setup on-the-fly`. Guards land both at the CLI
(parser.error) and at the programmatic-entry points
(run_evaluation/make_runner/run_one_task — Codex #1).
* New `scripts/prepare_livesqlbench.py` materialises per-DB
`<db>.sqlite` from `<db>_template.sqlite` (refuses git-LFS pointers
loudly). New `harness.materialize_task_db` gives each LiveSQLBench
task an isolated per-instance `db_file_path` (symlinked template) so
concurrent eval-resets never race the OTF cache build. Stale-symlink
rebuild + defensive `_ephemeral_`/`_process_` refusal (Codex #4/#5).
* New `harness.load_livesqlbench_tasks`: merges the gated gold sidecar
by `instance_id`, maps `query`→`amb_user_query`, stamps a
`task["dataset"]="livesqlbench"` marker (the irreducible source of
truth for DB isolation + the one-shot run_task programmatic guard),
filters `category=="Query"` (logs `_M_` disagreements), asserts
exactly 180 SELECT tasks on a full run, and is `filter_ids`-aware so
partial-gold targeted runs don't trip the empty-`sol_sql` fail-fast
(Codex #6).
* One-shot factories + prompts in BOTH SLayer adapters: `_build_sub_explorer`,
`_build_projection_resolver_oneshot`, `_build_query_constructor_oneshot`
(+ otf_encode `_build_kb_encoder_oneshot`). `_register_spawn_subagent`
+ `_register_kb_to_slayer` parametrized with `eval_mode` to dispatch
the right ask_user-free child. New one-shot prompt variants with NO
ask_user / user-sim language. `agent.run_task` accepts
`eval_mode in {a-interact, one-shot}` with reserve=submit_query only
and a one-shot resolver recovery prompt.
* Per-benchmark artifact separation (user principle: artifacts kept
SEPARATE by benchmark). `paths.slayer_otf_cache_root` and
`slayer_models_otf_root` gain a `benchmark: str | None = None` kwarg;
default returns the legacy mini-interact path (no caller breakage),
`benchmark="livesqlbench"` returns parallel roots
(`slayer_otf_cache_livesqlbench/`, `slayer_models_otf_livesqlbench/`)
with parallel env-var overrides. `_maybe_force_wipe_otf` is
benchmark-aware so `--otf-rebuild` never wipes the wrong root.
* Codex #2 fix: `slayer_otf/reference_build.py::_effective_db_root`
accepts an authoritative `db_root` kwarg that overrides
`$BIRD_DB_PATH` (threaded all the way from `ensure_db_reference`
through `_resolve_datasource_for_build` + `_portabilise_datasource`).
The otf_encode adapter passes `--db-path` as the explicit `db_root`
for LiveSQLBench runs, so the setup-encoder build resolves the right
sqlite even when conftest/CI sets `$BIRD_DB_PATH` to mini-interact.
* README + CLAUDE.md updated with the LiveSQLBench setup +
per-benchmark roots pattern. A `test_cloud_paths_unchanged.py`
regression test pins that the dev-1470 cloud upload-back / post-run
merge keep using the LEGACY (no-kwarg) roots — cloud-side
LiveSQLBench is explicitly out of scope and filed as a follow-up.
Full non-integration suite: 1232 passed, 94 skipped, 19 deselected.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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:
📝 WalkthroughWalkthroughAdds LiveSQLBench one-shot execution: dataset preparation, gold-sidecar merging, per-task SQLite materialization, benchmark-scoped OTF cache/model roots, one-shot agent factories/prompts and budgeting for recursive and otf-encode frameworks, db_root threading through reference-build, CLI/programmatic guards, and broad tests and fixtures. ChangesLiveSQLBench One-Shot Mode
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/agents/pydantic_ai_otf_encode/prompts.py`:
- Around line 436-469: The new KB_ENCODER_ONESHOT_PROMPT removed the
self-tagging/output-contract that sets meta.kb_id={kb_id}, which breaks
downstream dependency discovery used by _live_tagged_entities() and
_format_deps_block(); restore the tagging and output-contract sections from
KB_ENCODER_PROMPT into KB_ENCODER_ONESHOT_PROMPT (keeping only removal of
ask_user-specific instructions) so that encoders still emit meta.kb_id={kb_id}
and any required meta/self-annotation fields and call patterns (e.g.,
submit_encoding(result_json=...)) exactly once, ensuring already-encoded
dependencies are discoverable by _live_tagged_entities() and
_format_deps_block().
In `@src/bird_interact_agents/agents/pydantic_ai_recursive/agent.py`:
- Around line 282-298: run_task currently allows eval_mode="one-shot" even when
self.slayer_setup=="pre-encoded", letting callers bypass the dataset guard; add
an explicit check alongside the existing dataset check to raise ValueError if
is_one_shot and self.slayer_setup != "on-the-fly". Locate the branch around
eval_mode/is_one_shot in run_task (and reference self.slayer_setup) and enforce
that one-shot is only permitted when self.slayer_setup equals the on-the-fly
mode, with a clear error message mentioning both the required slayer_setup and
dataset constraints.
In `@tests/_livesqlbench_fixtures.py`:
- Around line 189-193: The public_task fixture currently uses truthy 'or'
defaults which overrides explicit empty values; change the assignments to use
explicit None checks so empty string "" or empty dict {} are preserved: for the
"query" key use query if query is not None else f"unambiguous request for
{instance_id}", and for the "conditions" key use conditions if conditions is not
None else {"decimal": [], "distinct": False, "order": False}; update the code in
the public_task function where "query" and "conditions" are set accordingly.
In `@tests/test_one_shot_mode.py`:
- Around line 351-352: Split the semicolon-joined statements into separate lines
to satisfy Ruff E702: replace the combined statements that create and write to
"data" and create "db_path" (variables data, db_path, tmp_path in
tests/test_one_shot_mode.py) with two distinct statements — one that assigns
data = tmp_path / "data.jsonl" and calls data.write_text(""), and another that
assigns db_path = tmp_path / "ds" and calls db_path.mkdir() — and do the same
for the other occurrences around the same file (the locations referenced near
the original combined lines).
In `@tests/test_one_shot_run.py`:
- Around line 728-799: The tests install the _spy_materialize spy but never
assert it was invoked, so add an assertion after each instantiation/run in both
OTF one-shot parity tests (the test function shown and the other one around
lines 802-870) to verify DB materialization occurred: after calling
PydanticAIOtfEncodeAgent.run_task, assert that the spy created by
_spy_materialize recorded a call to materialize_task_db (e.g., check spy.called
or spy.call_count > 0 or spy.assert_called_once() depending on the spy API).
Make this change in both test functions referencing _spy_materialize and
materialize_task_db so a regression in one-shot DB materialization is caught.
🪄 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: 476372c4-3570-4134-9d96-b93cf793a62c
📒 Files selected for processing (27)
CLAUDE.mdREADME.mdscripts/prepare_livesqlbench.pysrc/bird_interact_agents/agents/pydantic_ai_otf_encode/agent.pysrc/bird_interact_agents/agents/pydantic_ai_otf_encode/factories.pysrc/bird_interact_agents/agents/pydantic_ai_otf_encode/prompts.pysrc/bird_interact_agents/agents/pydantic_ai_recursive/agent.pysrc/bird_interact_agents/agents/pydantic_ai_recursive/factories.pysrc/bird_interact_agents/agents/pydantic_ai_recursive/prompts.pysrc/bird_interact_agents/harness.pysrc/bird_interact_agents/paths.pysrc/bird_interact_agents/run.pysrc/bird_interact_agents/slayer_otf/reference_build.pytests/_livesqlbench_fixtures.pytests/test_cloud_paths_unchanged.pytests/test_db_isolation.pytests/test_livesqlbench_loader.pytests/test_one_shot_mode.pytests/test_one_shot_otf_encode_factories.pytests/test_one_shot_recursive_factories.pytests/test_one_shot_run.pytests/test_otf_encode_reference_root.pytests/test_otf_rebuild_per_benchmark.pytests/test_otf_rebuild_wiring.pytests/test_paths.pytests/test_prepare_livesqlbench.pytests/test_pydantic_ai_otf_encode_agent.py
* run.py: wire `materialize_task_db` into `run_oracle_task` so
LiveSQLBench oracle runs at `--concurrency > 1` don't race the
shared `<db>.sqlite` the OTF cache reads (B0's docstring said it
was called from both run paths but the oracle wiring was missing
— Codex).
* pydantic_ai_otf_encode/prompts.py: restore the KB SELF-ANNOTATION
contract in `KB_ENCODER_ONESHOT_PROMPT`. Every encoded entity must
carry `label`, the canonical `description` block, and
`meta.kb_id={kb_id}` — downstream `_live_tagged_entities()` +
`_format_deps_block()` discover already-encoded deps via those
tags, so a successfully-encoded KB without the meta.kb_id tag
silently looks "not encoded" to dependents and breaks transitive
`kb_to_slayer` resolution (CodeRabbit).
* pydantic_ai_recursive/agent.py: belt-and-suspenders check that
one-shot ⟹ `slayer_setup == "on-the-fly"`. The check already lives
at the CLI / run_evaluation / make_runner / run_one_task
boundaries; this in-run_task assertion closes the bypass for a
programmatic caller that constructs
`PydanticAIRecursiveAgent(slayer_setup="pre-encoded")` and invokes
`.run_task(eval_mode="one-shot", ...)` directly (CodeRabbit). +
regression test.
Full non-integration suite: 1233 passed (was 1232).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* harness.load_livesqlbench_tasks: the full-run "exactly 180" check is now `if … : raise ValueError(…)`, not `assert`. Production guards must survive `python -O` / `PYTHONOPTIMIZE` — the latter strips assertions, so a truncated dataset would have silently run in exactly the scenario this guard exists for. * README: the LiveSQLBench setup steps used `cd livesqlbench-base- lite-sqlite` then ran `uv run python scripts/prepare_livesqlbench.py`, but `scripts/` lives in **bird-agents**, not the dataset dir. Rewrite step 2 as a subshell (`(cd … && git lfs pull)`) and have step 3 explicitly `cd bird-agents` before running the script. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* tests/_livesqlbench_fixtures.py: `public_task` uses `is not None`
(not `or`) for `query` and `conditions` defaults, so a caller can
build a fixture with an explicit empty `""` / `{}` for edge-case
tests instead of having it silently replaced by the default.
* tests/test_one_shot_mode.py: split semicolon-separated statements
(`data = ...; data.write_text("")`) into two lines each — Ruff E702
flags the one-liner form and would break CI once Ruff is wired in.
* tests/test_one_shot_run.py: both OTF one-shot parity tests (reserve
+ empty-resolver-skip) now capture the `materialize_calls` spy
return and assert it was invoked. The recursive parity tests
already do this; without it a regression in OTF one-shot DB
materialization would slip through these two paths.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous round threaded `db_root` into `ensure_db_reference` →
`_effective_db_root` so the otf_encode **reference build** ignores
`$BIRD_DB_PATH` when an authoritative `--db-path` is in play. But the
**per-task variant copy** (which is what the SLayer MCP server reads
at runtime) is materialised by `build_task_variant_storage`, and that
function delegates to `resolve_committed_connection_string` which
still preferred `$BIRD_DB_PATH` over the supplied root.
Net effect: a one-shot LiveSQLBench run would correctly build/reuse
the LiveSQLBench-scoped reference, then SILENTLY query the
mini-interact sqlite at task time (because conftest / CI / day-to-day
shells often set `BIRD_DB_PATH` to mini-interact).
Mirror the previous fix one more layer:
* `slayer_pipeline/portable_connection.py::resolve_committed_connection_string`
gains an explicit `db_root: Path | None = None` kwarg that overrides
`$BIRD_DB_PATH` when set. Back-compat preserved (no kwarg → legacy
env-wins semantics).
* `hard8_preprocessor.build_task_variant_storage` gains the same
`db_root` kwarg and forwards it into
`resolve_committed_connection_string`.
* otf_encode's `_resolve_otf_task_storage_dir` now passes
`db_root=Path(data_path_base).resolve()` (the same value already
passed to `ensure_db_reference`), so build-time + runtime
re-anchoring see the SAME root.
Recursive flavor unaffected — `runtime._rewrite_datasource_connection_string`
force-overwrites the connection string from `data_path_base` already,
no env consultation.
+ 3 regression tests in test_otf_encode_reference_root.py:
- resolve_committed_connection_string honours db_root over env,
- back-compat: no db_root → env wins,
- build_task_variant_storage forwards db_root through the resolver
(spies the source-module resolver, not the importing-module
binding, because the import lives inside the function body).
Full non-integration suite: 1236 passed (was 1233; +3 new tests).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…he-benchmark-harness-to-run-with-livesqlbench-lite # Conflicts: # src/bird_interact_agents/agents/pydantic_ai_otf_encode/factories.py
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/bird_interact_agents/agents/pydantic_ai_otf_encode/prompts.py (1)
581-632:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAppend the shared encoder style guide to the one-shot KB encoder prompt.
_run_kb_encoder_default()now computesexisting_kb_tagged_entities_blockfor both modes, butKB_ENCODER_ONESHOT_PROMPTnever renders it. That drops the new string-normalisation, peer-dedup, and host-choice instructions exactly on the one-shot path, so the encoder never sees theentity_ref=/reachable_from_host:markers this PR added.♻️ Suggested fix
KB_ENCODER_ONESHOT_PROMPT = """\ ... Budget: {budget} bird-coins remaining. Call `submit_encoding` exactly once when done; reply briefly afterwards to finish. """ + +KB_ENCODER_ONESHOT_PROMPT += "\n\n" + _STYLE_GUIDEAs per coding guidelines "Use
_collect_peer_kb_entriesfrom factories.py to inspect and validate that every kb-tagged peer entity in the rendered prompt block carries labeled markersentity_ref=<db>.<model>[.<leaf>]andreachable_from_host: host(hops), …."🤖 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/pydantic_ai_otf_encode/prompts.py` around lines 581 - 632, KB_ENCODER_ONESHOT_PROMPT is missing the existing_kb_tagged_entities_block so one-shot runs don't receive the new normalization/dedup/host-choice markers; update the prompt definition used by _run_kb_encoder_default() to include the existing_kb_tagged_entities_block (the same way the multi-turn encoder uses it), ensuring the block rendered contains entity_ref= and reachable_from_host: markers produced by _collect_peer_kb_entries in factories.py; specifically, insert the existing_kb_tagged_entities_block placeholder into KB_ENCODER_ONESHOT_PROMPT where dependencies are listed (the {deps_block} area) and verify generated blocks include the labeled markers before calling submit_encoding.src/bird_interact_agents/agents/pydantic_ai_otf_encode/factories.py (1)
1157-1165:⚠️ Potential issue | 🟠 Major | ⚡ Quick winScope
get_modellookups by datasource in_entity_exists()/_resolve_ref()
_entity_exists()and_resolve_ref()callstorage.get_model(...)withoutdata_source, so same-named models across datasources can cause valid entities to be treated as missing (spuriousstatus="error") and prevent setup-ref reuse.♻️ Suggested fix
- model = await storage.get_model(ent.name) + try: + model = await storage.get_model(ent.name, data_source=db_name) + except TypeError: + model = await storage.get_model(ent.name) return model is not None and model.data_source == db_name ... - model = await storage.get_model(ent.host_model) + try: + model = await storage.get_model(ent.host_model, data_source=db_name) + except TypeError: + model = await storage.get_model(ent.host_model) if model is None or model.data_source != db_name: return False ... - model = await storage.get_model(parts[1]) + try: + model = await storage.get_model(parts[1], data_source=db) + except TypeError: + model = await storage.get_model(parts[1]) if model is not None and getattr(model, "data_source", None) == db: return EncodedEntity( kind="model", host_model=None, name=parts[1], entity_ref=ref, ) ... - model = await storage.get_model(model_name) + try: + model = await storage.get_model(model_name, data_source=db) + except TypeError: + model = await storage.get_model(model_name) if model is None or getattr(model, "data_source", None) != db: return 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 `@src/bird_interact_agents/agents/pydantic_ai_otf_encode/factories.py` around lines 1157 - 1165, In _entity_exists() and _resolve_ref(), the code calls storage.get_model(...) without scoping by datasource which causes collisions across datasources; update both locations so that calls to get_model include the data_source/db_name argument (e.g., replace await storage.get_model(ent.name) and await storage.get_model(ent.host_model) with await storage.get_model(ent.name, data_source=db_name) or the appropriate parameter name), and ensure any downstream checks (model is not None and model.data_source == db_name) remain consistent or are simplified because the lookup is already scoped by data_source.
🤖 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.
Outside diff comments:
In `@src/bird_interact_agents/agents/pydantic_ai_otf_encode/factories.py`:
- Around line 1157-1165: In _entity_exists() and _resolve_ref(), the code calls
storage.get_model(...) without scoping by datasource which causes collisions
across datasources; update both locations so that calls to get_model include the
data_source/db_name argument (e.g., replace await storage.get_model(ent.name)
and await storage.get_model(ent.host_model) with await
storage.get_model(ent.name, data_source=db_name) or the appropriate parameter
name), and ensure any downstream checks (model is not None and model.data_source
== db_name) remain consistent or are simplified because the lookup is already
scoped by data_source.
In `@src/bird_interact_agents/agents/pydantic_ai_otf_encode/prompts.py`:
- Around line 581-632: KB_ENCODER_ONESHOT_PROMPT is missing the
existing_kb_tagged_entities_block so one-shot runs don't receive the new
normalization/dedup/host-choice markers; update the prompt definition used by
_run_kb_encoder_default() to include the existing_kb_tagged_entities_block (the
same way the multi-turn encoder uses it), ensuring the block rendered contains
entity_ref= and reachable_from_host: markers produced by
_collect_peer_kb_entries in factories.py; specifically, insert the
existing_kb_tagged_entities_block placeholder into KB_ENCODER_ONESHOT_PROMPT
where dependencies are listed (the {deps_block} area) and verify generated
blocks include the labeled markers before calling submit_encoding.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a5bfcc00-92bc-49a2-b6cd-bb5fc49caea2
📒 Files selected for processing (5)
CLAUDE.mdsrc/bird_interact_agents/agents/pydantic_ai_otf_encode/agent.pysrc/bird_interact_agents/agents/pydantic_ai_otf_encode/factories.pysrc/bird_interact_agents/agents/pydantic_ai_otf_encode/prompts.pytests/test_pydantic_ai_otf_encode_agent.py
✅ Files skipped from review due to trivial changes (1)
- CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_pydantic_ai_otf_encode_agent.py
The dev-1478 merge substantially enriched the a-interact
`KB_ENCODER_PROMPT` (shared `_STYLE_GUIDE`: STRING-NORM, cross-model
access, no-invented-joins, host-choice, peer-KB dedup, sampled-value
caveats + the `{existing_kb_tagged_entities_block}` placeholder). The
one-shot `KB_ENCODER_ONESHOT_PROMPT` predated that and had drifted into
a shortened distillation — so the with-encoding one-shot flavor was
producing lower-fidelity KB encodings than the a-interact flavor on the
SAME KBs, weakening the benchmark comparison. (The merged `.format()`
call passed `existing_kb_tagged_entities_block` to both templates, but
the one-shot prompt had no placeholder for it, so it was silently
dropped.)
Fix: stop hand-maintaining the one-shot encoder prompt and DERIVE it
from the a-interact base instead, so the recipe table / NULL handling /
SQL dialect / `meta.kb_id` self-annotation / `_STYLE_GUIDE` stay in
lockstep forever and can't re-drift:
* Extract the a-interact body into `_KB_ENCODER_BASE`;
`KB_ENCODER_PROMPT = _KB_ENCODER_BASE + "\n\n" + _STYLE_GUIDE`
(behaviour unchanged).
* Build `KB_ENCODER_ONESHOT_PROMPT` from `_KB_ENCODER_BASE` by swapping
exactly the four `ask_user`-bearing segments (NO-DEFERRAL para, the
budget `ask_user: 2` row, workflow Step 2, the output-contract status
line) for an autonomous-decision contract, then appending the shared
`_STYLE_GUIDE`.
* Each swap is assertion-guarded: if a future edit changes the base
wording, import fails loudly rather than silently leaving `ask_user`
text in a prompt whose agent has no such tool. A final assert pins
that the derived prompt contains no `ask_user` (also covered by
tests/test_one_shot_otf_encode_factories.py).
Verified: the one-shot prompt now carries the full recipe table,
`_STYLE_GUIDE` (HOST-CHOICE etc.), the
`{existing_kb_tagged_entities_block}` placeholder, and the meta.kb_id
self-annotation, with zero ask_user/user-sim language; `.format()` with
the factory's kwargs renders cleanly.
Full non-integration suite: 1336 passed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`scripts/prepare_livesqlbench.py` imported `from bird_interact_agents import paths` with no `sys.path` setup, so a bare `python scripts/prepare_livesqlbench.py` from a src-layout checkout without an editable install raised ModuleNotFoundError before any LFS/template check ran (Codex). `uv run python …` worked because the venv carries the editable install, but the convention in `scripts/export_slayer_models.py` is to self-bootstrap. Add the same 3-line `<repo>/src` → sys.path insert before the import (no-op once the package is installed). Verified: bare `python3 scripts/prepare_livesqlbench.py --help` now exits 0 under a cleaned environment. Full non-integration suite: 1336 passed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
DEV-1478 (cloud DB conn re-anchor + Mode-B filter normalization) landed on main. Two files conflicted — both where DEV-1478's connection-string re-anchoring overlaps this branch's `db_root` override (Codex #2 fix that lets a LiveSQLBench `--db-path` win over `$BIRD_DB_PATH`): * portable_connection.py (auto-merged; both helpers now coexist): threaded `db_root` through `reanchor_connection_string` and `expected_connection_string` so the explicit `db_root` wins over `$BIRD_DB_PATH` in BOTH the relative-resolve and absolute-force-rewrite branches. `reanchor_*` is now the single entry point — superset of `resolve_committed_connection_string` (relative resolve) PLUS DEV-1478's absolute cloud-transport rewrite (`<root>/<db>/<db>.sqlite`). * hard8_preprocessor.build_task_variant_storage — call site now uses `reanchor_connection_string(cs, db, root, db_root=db_root)` (keeps the DEV-1462 db_root override while gaining DEV-1478's absolute re-anchor). * slayer_otf/reference_build._resolve_datasource_for_build — docstring reconciled to DEV-1478's reanchor mechanism + DEV-1462's db_root precedence; fixed a latent bug where the merged body called `reanchor_connection_string(...)` WITHOUT `db_root=`, so conftest's `$BIRD_DB_PATH=<mini-interact>` would have mis-anchored a LiveSQLBench build. Now passes `db_root=db_root` so build-time resolve and commit-time portabilise agree on the effective root. Full non-integration suite: 1385 passed, 94 skipped, 19 deselected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…_encode verify
Two major review findings from the DEV-1478 merge, same root cause:
LiveSQLBench and mini-interact share DB names (alien, cross_db), and a
resolve-by-name step wasn't told which benchmark it serves — so it could
silently pick the other benchmark's database.
Fix 1 (Codex) — recursive on-the-fly path now threads an authoritative
`db_root` so the harness's `--db-path` beats `$BIRD_DB_PATH`, mirroring the
otf_encode adapter. The recursive `_resolve_otf_task_storage_dir` computed the
right root (`Path(data_path_base).resolve()`) but handed it to
`prepare_task_storage` only as the fallback positional; downstream
`reanchor_connection_string` then fell back to `$BIRD_DB_PATH` (set to
mini-interact by conftest / dev shells), re-anchoring an overlapping DB name to
the wrong sqlite. Adds `db_root` to `runtime.prepare_task_storage` +
`_rewrite_datasource_connection_string`, forwarded into
`reanchor_connection_string(db_root=...)`; the recursive resolver passes
`db_root=mini_interact_root`. Cloud-safe (there data_path_base == $BIRD_DB_PATH).
Fix 2 (CodeRabbit) — otf_encode's post-encode verifier `_entity_exists` and
setup-ref resolver `_resolve_ref` looked up `storage.get_model(name)` unscoped
then asserted `model.data_source == db`. SLayer resolves a bare name by
datasource priority, so on a name collision the lookup returns the wrong db's
model and the entity is falsely reported missing (spurious status="error" /
lost setup-ref reuse). Now scope all four lookups with `data_source=db`,
matching the sibling lookups already scoped at factories.py L774/L908. (No
try/except — `get_model(name, data_source=None)` definitively exists.)
Tests: tests/test_recursive_runtime_db_root.py (signature + db_root-over-env
re-anchor + reanchor threading + recursive-resolver passes db_root) and
tests/test_otf_encode_get_model_scoping.py (2-datasource collision stub →
_entity_exists/_resolve_ref resolve the correct db). Updated two existing
stubs (fake_prepare, _PermissiveStorage.get_model) to accept the new kwargs.
Full non-integration suite: 1394 passed.
Dismissed CodeRabbit CR-1 (one-shot KB-encoder prompt "missing" the style
guide): false — KB_ENCODER_ONESHOT_PROMPT already appends _STYLE_GUIDE
(prompts.py:676) which carries the {existing_kb_tagged_entities_block}
placeholder (L181), formatted for both modes (factories.py:660). CodeRabbit
cited the replacement-tuple range and missed the L676 assignment.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 28, 2026
Merged
ZmeiGorynych
added a commit
that referenced
this pull request
Jun 4, 2026
- Use paths.*_root() helpers in migrate_data_dirs.py (CodeRabbit #1) - Guard src_mi.rmdir() after partial annotation dir merge (CodeRabbit #2) - Use paths.benchmark_data_root() in ray_app.py + ray_app_annotator.py (CodeRabbit #3, #4) - Add BIRD_AUDITED_GOLD_ROOT to isolation fixture in test_paths.py (CodeRabbit #5) - Assert BIRD_GATED_GOLD_ROOT in download_benchmark_data test (CodeRabbit nitpick) - Add Benchmark.select_full_run_count; pass through load_benchmark_tasks so livesqlbench-base-full / livesqlbench-large (one_shot=True) don't hit the hardcoded 180-assertion (Codex #2) - _build_original_sql_index: use _auto_discover_gold instead of hardcoded livesqlbench filename — works for all gold_required benchmarks (Codex #3) - _check_gold_present helper in driver.py: fail at submit time if gold_required but no *.jsonl found locally (Codex #1, right layer for this check) - Remove Path import from ray_app_annotator.py (no longer used) - Merge conflict resolved: FakeSubmitArgs + _fake_annotate_args in test_driver.py Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
4 tasks
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.
Add a new
--mode one-shotevaluation path that runs theLiveSQLBench-Base-Lite-SQLite dataset (the unambiguous, one-shot
sibling of mini-interact) against BOTH SLayer agents —
pydantic_ai_recursive(KB→memories) andpydantic_ai_otf_encode(KB→models). Scored on the 180 SELECT (
category=="Query") tasks;no user simulator, no
ask_useranywhere in the spawn tree.The full final plan (incl. all Codex findings folded) is in
DEV-1462's "Final approved plan" section.
Highlights
--dataset {mini-interact, livesqlbench}and--gold-fileflags.--mode one-shotis gated to--dataset livesqlbench;--dataset livesqlbenchis gated to
--mode {one-shot, oracle}; one-shot requires--slayer-setup on-the-fly. Guards land both at the CLI(
parser.error) and the programmatic-entry points(
run_evaluation/make_runner/run_one_task).scripts/prepare_livesqlbench.pymaterialises per-DB<db>.sqlitefrom
<db>_template.sqlite(refuses git-LFS pointers loudly). Newharness.materialize_task_dbgives each LiveSQLBench task an isolatedper-instance
db_file_path(symlinked template) so concurrenteval-resets never race the OTF cache build. Stale-symlink rebuild +
defensive
_ephemeral_/_process_refusal.harness.load_livesqlbench_tasks: merges the gated gold sidecarby
instance_id, mapsquery→amb_user_query, stampstask["dataset"]="livesqlbench", filterscategory=="Query", assertsexactly 180 on a full run, and is
filter_ids-aware so partial-goldtargeted runs don't trip the empty-
sol_sqlfail-fast._build_sub_explorer,_build_projection_resolver_oneshot,_build_query_constructor_oneshot(+ otf_encode_build_kb_encoder_oneshot)._register_spawn_subagent+_register_kb_to_slayerparametrized witheval_modeto dispatchthe right ask_user-free child.
agent.run_taskacceptseval_mode in {a-interact, one-shot}with reserve=submit_queryonly and a one-shot resolver recovery prompt.
paths.slayer_otf_cache_rootand
slayer_models_otf_rootgain abenchmark: str | None = Nonekwarg; default returns the legacy mini-interact path (no caller
breakage),
benchmark="livesqlbench"returns parallel roots(
slayer_otf_cache_livesqlbench/,slayer_models_otf_livesqlbench/)with parallel env-var overrides.
_maybe_force_wipe_otfisbenchmark-aware so
--otf-rebuildnever wipes the wrong root.slayer_otf/reference_build.py::_effective_db_rootaccepts an authoritative
db_rootkwarg that overrides$BIRD_DB_PATH(threaded all the way from
ensure_db_reference). The otf_encodeadapter passes
--db-pathas the explicitdb_rootfor LiveSQLBenchruns, so the setup-encoder resolves the right sqlite even when
conftest/CI sets
$BIRD_DB_PATHto mini-interact.per-benchmark roots pattern. A
test_cloud_paths_unchanged.pyregression test pins that the DEV-1470 cloud upload-back / post-run
merge keep using the legacy (no-kwarg) roots — cloud-side
LiveSQLBench is explicitly out of scope and filed as a follow-up.
Tests
165 new tests + extensions to existing ones; full non-integration
suite: 1232 passed, 94 skipped, 19 deselected. TDD: tests landed
first, Codex-reviewed against the plan (all 8 findings folded; the
test_otf_encode_reference_rootwas restructured to be unconditional;the spawn-builder closure was moved back to call-time so the existing
nested-spawn trajectory test still pins parent_idx correctness).
Manual smoke (after merge)
Out of scope (explicit follow-ups)
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
New Features
Documentation
Improvements
Tests