feat(reports): BIRD-INTERACT-1.0 a-Interact submission generator (DEV-1553) - #45
Open
ZmeiGorynych wants to merge 11 commits into
Open
feat(reports): BIRD-INTERACT-1.0 a-Interact submission generator (DEV-1553)#45ZmeiGorynych wants to merge 11 commits into
ZmeiGorynych wants to merge 11 commits into
Conversation
…-1553) Adds `bird-interact-cloud submission` that reconstructs a BIRD-INTERACT-1.0 a-Interact submission directory (submission.jsonl + email_title.txt + manifest.json) from existing cloud runs — no harness-runtime changes. Reconstruction is offline-only: * Walk runs/<bench>/<db>/<id>/<run>.trajectory.json via the claude_sdk_otf_* adapter (one Turn per tool_use, pure-text folded into next turn). * Canonicalise tool calls to upstream eval_react names (submit / execute / ask + the bird-interact-tools getters); unknown tools fall through. * Tokenise action args + observations via Anthropic count_tokens with envelope-baseline subtraction so the Section VI 250/1000 thresholds are contract-exact. * Replay Section VI costs (fixed ask=2 / submit=3 / execute=1; token-aware otherwise) — this is the LEADERBOARD contract, not the harness's internal ACTION_COSTS. * total_budget mirrors harness.calculate_budget(task_data, patience, mode='a-interact'); remaining_budget = max(0, total - cumulative). Hard errors: duplicate selection.jsonl instance_id, missing/stub trajectory.json, missing results.db, incomplete coverage without `--allow-partial`, instance_id not in the benchmark split. CLI extras: `--no-thinking` strips thinking blocks; `--check-leakage` adds per-instance gold-SQL substring counts to manifest (diagnostic only, never redacts). 124 new tests (full repo suite: 2810 passed, 0 failed). 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 a submission generator: CLI subcommand, selection/coverage/source resolution, adapters and Turn model, trajectory→SubmissionRow conversion (tokens/costs/budgets, phase splitting, canonical actions), leakage checks, output writers, and broad tests. ChangesSubmission Report Generation & CLI Integration
Sequence Diagram(s)sequenceDiagram
participant CloudCLI as cloud.cli
participant ReportsCLI as bird_interact_agents.reports.cli
participant Sources as bird_interact_agents.reports.sources
participant Converter as bird_interact_agents.reports.converter
participant Output as bird_interact_agents.reports.output
CloudCLI->>ReportsCLI: run_submission(args)
ReportsCLI->>Sources: resolve_sources(selection/run_id)
ReportsCLI->>Converter: build_submission_row(instance trajectory)
ReportsCLI->>Output: write_submission(rows, plan, out_dir)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
tests/reports/test_selection.py (2)
171-171: ⚡ Quick winPrefix unused unpacked value with underscore.
The
runs_rootvariable is unpacked but onlyresults_rootis used. Prefix the unused value with_.🧹 Proposed fix
- runs_root, results_root = stage( + _runs_root, results_root = stage(🤖 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/reports/test_selection.py` at line 171, The variable runs_root is unpacked from the call to stage but never used; change the unpacking to prefix the unused value with an underscore (e.g., replace "runs_root, results_root = stage(...)" with "_, results_root = stage(...)") so the linter knows the first value is intentionally ignored; update the unpacking at the call site where stage(...) is used in tests/reports/test_selection.py and keep the results_root usage unchanged.
85-85: ⚡ Quick winPrefix unused unpacked values with underscore.
The
runs_rootandresults_rootvariables are unpacked but never used. Prefix unused values with_to clarify intent.🧹 Proposed fix
- runs_root, results_root = stage( + _runs_root, _results_root = stage(🤖 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/reports/test_selection.py` at line 85, The variables runs_root and results_root returned from stage(...) are unpacked but not used; rename them to _runs_root and _results_root (or prefix with a single underscore) where they are unpacked to signal they are intentionally unused and silence lint warnings—update the unpacking site that currently reads "runs_root, results_root = stage(...)" to use those underscored names.src/bird_interact_agents/reports/adapters/__init__.py (2)
43-43: 💤 Low valueConsider sorting
__all__alphabetically.Ruff (RUF022) suggests sorting
__all__entries alphabetically for consistency. While the current logical grouping is reasonable, alphabetical sorting makes it easier to verify completeness and avoid duplicates in larger modules.♻️ Proposed fix
-__all__ = ["Turn", "get_adapter", "UnknownFrameworkError"] +__all__ = ["Turn", "UnknownFrameworkError", "get_adapter"]🤖 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/reports/adapters/__init__.py` at line 43, The __all__ export list should be alphabetized to satisfy RUF022; update the __all__ definition that currently lists "Turn", "get_adapter", "UnknownFrameworkError" so the entries are in alphabetical order (reference symbols: Turn, get_adapter, UnknownFrameworkError) and ensure the final __all__ contains the same symbols but sorted.
37-40: ⚡ Quick winUse exception chaining for better debugging context.
The raised
UnknownFrameworkErrorshould preserve the originalKeyErrorusingraise ... from errto maintain the full traceback.♻️ Proposed fix
try: return _REGISTRY[framework] except KeyError: raise UnknownFrameworkError( f"no submission-report adapter registered for framework " f"{framework!r}; supported: {sorted(_REGISTRY)}" - ) + ) from NoneNote: Using
from Noneis appropriate here since theKeyErroritself is not meaningful to the caller—only theUnknownFrameworkErrorwith the helpful message matters.🤖 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/reports/adapters/__init__.py` around lines 37 - 40, The code currently raises UnknownFrameworkError without exception chaining; update the exception raise in the adapter lookup so it preserves the caught KeyError (err) by using "raise UnknownFrameworkError(f\"no submission-report adapter registered for framework {framework!r}; supported: {sorted(_REGISTRY)}\") from err" — modify the raise in the block that catches the KeyError so UnknownFrameworkError is raised from err (reference symbols: UnknownFrameworkError, _REGISTRY, framework, err).src/bird_interact_agents/reports/selection.py (1)
33-36: ⚡ Quick winUse exception chaining for better debugging context.
The re-raised
KeyErrorshould preserve the original exception chain usingraise ... from eto maintain the full traceback and distinguish this exception from errors in the exception handler itself.♻️ Proposed fix
try: inst = obj["instance_id"] run = obj["run_id"] except KeyError as e: raise KeyError( f"{p}:{line_no} selection entry missing required " f"field {e.args[0]!r}: {obj!r}" - ) + ) from e🤖 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/reports/selection.py` around lines 33 - 36, The KeyError raised in selection.py currently loses the original exception context; update the re-raise to use exception chaining by changing the raise to include "from e" so the new KeyError preserves the original exception (the caught variable named e) and full traceback; locate the raise KeyError(...) in the selection entry validation block and append "from e" to the raise expression.
🤖 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/reports/cli.py`:
- Around line 48-67: In _read_patience_for_instance, the call to int(pat) can
raise ValueError for non-numeric patience values; wrap the conversion of pat to
int in a try/except ValueError (alongside the existing JSON/OSError handling)
and return (None, "default") on failure so malformed patience in obj or
obj["submission"] does not crash; ensure you still return f"runs:{p.name}" only
when conversion succeeds.
---
Nitpick comments:
In `@src/bird_interact_agents/reports/adapters/__init__.py`:
- Line 43: The __all__ export list should be alphabetized to satisfy RUF022;
update the __all__ definition that currently lists "Turn", "get_adapter",
"UnknownFrameworkError" so the entries are in alphabetical order (reference
symbols: Turn, get_adapter, UnknownFrameworkError) and ensure the final __all__
contains the same symbols but sorted.
- Around line 37-40: The code currently raises UnknownFrameworkError without
exception chaining; update the exception raise in the adapter lookup so it
preserves the caught KeyError (err) by using "raise UnknownFrameworkError(f\"no
submission-report adapter registered for framework {framework!r}; supported:
{sorted(_REGISTRY)}\") from err" — modify the raise in the block that catches
the KeyError so UnknownFrameworkError is raised from err (reference symbols:
UnknownFrameworkError, _REGISTRY, framework, err).
In `@src/bird_interact_agents/reports/selection.py`:
- Around line 33-36: The KeyError raised in selection.py currently loses the
original exception context; update the re-raise to use exception chaining by
changing the raise to include "from e" so the new KeyError preserves the
original exception (the caught variable named e) and full traceback; locate the
raise KeyError(...) in the selection entry validation block and append "from e"
to the raise expression.
In `@tests/reports/test_selection.py`:
- Line 171: The variable runs_root is unpacked from the call to stage but never
used; change the unpacking to prefix the unused value with an underscore (e.g.,
replace "runs_root, results_root = stage(...)" with "_, results_root =
stage(...)") so the linter knows the first value is intentionally ignored;
update the unpacking at the call site where stage(...) is used in
tests/reports/test_selection.py and keep the results_root usage unchanged.
- Line 85: The variables runs_root and results_root returned from stage(...) are
unpacked but not used; rename them to _runs_root and _results_root (or prefix
with a single underscore) where they are unpacked to signal they are
intentionally unused and silence lint warnings—update the unpacking site that
currently reads "runs_root, results_root = stage(...)" to use those underscored
names.
🪄 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: 1f74732b-73ec-456e-8c9a-745552feac55
📒 Files selected for processing (35)
src/bird_interact_agents/cloud/cli.pysrc/bird_interact_agents/paths.pysrc/bird_interact_agents/reports/__init__.pysrc/bird_interact_agents/reports/action_canonicalize.pysrc/bird_interact_agents/reports/adapters/__init__.pysrc/bird_interact_agents/reports/adapters/base.pysrc/bird_interact_agents/reports/adapters/claude_sdk_otf.pysrc/bird_interact_agents/reports/budget.pysrc/bird_interact_agents/reports/cli.pysrc/bird_interact_agents/reports/converter.pysrc/bird_interact_agents/reports/cost.pysrc/bird_interact_agents/reports/coverage.pysrc/bird_interact_agents/reports/leakage.pysrc/bird_interact_agents/reports/output.pysrc/bird_interact_agents/reports/phase_split.pysrc/bird_interact_agents/reports/schema.pysrc/bird_interact_agents/reports/selection.pysrc/bird_interact_agents/reports/sources.pysrc/bird_interact_agents/reports/tokens.pytests/cloud/test_submission_cli.pytests/reports/__init__.pytests/reports/_fixtures.pytests/reports/conftest.pytests/reports/test_action_canonicalize.pytests/reports/test_adapter_claude_sdk_otf.pytests/reports/test_budget.pytests/reports/test_converter.pytests/reports/test_cost.pytests/reports/test_coverage.pytests/reports/test_leakage.pytests/reports/test_output.pytests/reports/test_paths_reports_root.pytests/reports/test_phase_split.pytests/reports/test_selection.pytests/reports/test_tokens.py
…tience guard Codex + CodeRabbit findings on PR #45. * action_canonicalize: register `mcp__bird-interact-tools__ask_user` as `ask` — the SLayer agent exposes ask_user through the MCP server, so the bare-name entry alone missed real trajectories and dropped the Section VI fixed cost of 2. * adapters: drop `_raw` / `_ainteract_raw` from the registry. Raw runs persist trajectory `data` as a Python-repr STRING, not a dict; the dict-based walker would crash. The lookup now raises clearly at source-resolution time until a string-repr parser lands. * reports/cli `_read_patience_for_instance`: guard `int(pat)` with try/except — non-numeric patience falls back to (None, "default") consistently with the surrounding file-IO / JSON defensiveness. * test_selection: prefix unused `runs_root` unpacks with `_`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ce phase warnings Codex findings on PR #45. * Real cloud runs persist `framework="claude_sdk"` in results.db (the `bird-interact-cloud submit --framework claude_sdk` CLI flag — the SLayer-vs-raw / one-shot-vs-a-interact distinction lives in `query_mode` and `mode`). The adapter registry was keyed by the internal class name `claude_sdk_otf` and rejected every real run. Re-key by `(framework, query_mode)`: - `("claude_sdk", "slayer")` → walker (real cloud runs). - Internal names `claude_sdk_otf` / `_ainteract` stay registered for forward-compat if a future schema promotes them. - `query_mode="raw"` is now rejected at lookup time (data is a Python-repr string the dict walker can't read). `resolve_sources` reads `query_mode` + `mode` from the task_results row and passes both through `InstanceSource`. * CLI mode gate: refuse to run when any selected instance has `mode != "a-interact"`. The DEV-1553 spec is a-Interact-only; the gate lists every offender so a mis-pointed run-id fails fast. * Phase-split warnings (Codex finding #2): `split_phases` returned missing-marker / inconsistent-ordering warnings, but `build_submission_row` discarded them. `build_submission_row` now returns `(row, warnings)` and the CLI merges them with the results.db cross-check warnings into `manifest.warnings_by_instance`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/reports/test_adapter_claude_sdk_otf.py (2)
253-260: ⚡ Quick winCatch the specific exception type for test precision.
The test catches
(KeyError, ValueError), but per the implementation in__init__.pylines 60–64,get_adapteralways raisesUnknownFrameworkError(aValueErrorsubclass) when the lookup fails. Import and assertUnknownFrameworkErrordirectly for better test documentation and failure messages.♻️ Proposed fix
def test_adapter_registry_unknown_framework_errors(): import pytest - from bird_interact_agents.reports.adapters import get_adapter + from bird_interact_agents.reports.adapters import ( + UnknownFrameworkError, + get_adapter, + ) - with pytest.raises((KeyError, ValueError)): + with pytest.raises(UnknownFrameworkError): get_adapter("pydantic_ai", query_mode="slayer")🤖 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/reports/test_adapter_claude_sdk_otf.py` around lines 253 - 260, Update the test_adapter_registry_unknown_framework_errors test to assert the specific exception type raised by get_adapter: import UnknownFrameworkError from the package (the error class raised by get_adapter) and replace the broad pytest.raises((KeyError, ValueError)) with pytest.raises(UnknownFrameworkError) so the test checks for the concrete UnknownFrameworkError thrown by get_adapter.
262-274: ⚡ Quick winCatch the specific exception type for test precision.
The test catches
(KeyError, ValueError), butget_adapteralways raisesUnknownFrameworkErrorwhen the(framework, query_mode)combo is unsupported. Import and assertUnknownFrameworkErrordirectly to make the test intention clearer and improve failure diagnostics.♻️ Proposed fix
def test_adapter_registry_rejects_raw_query_mode(): """``query_mode='raw'`` runs persist trajectory `data` as a Python repr STRING, not a dict — the SLayer walker would crash. Until a string-repr parser lands the lookup must error clearly so the failure surfaces at source-resolution time (not mid-walk with a confusing AttributeError).""" import pytest - from bird_interact_agents.reports.adapters import get_adapter + from bird_interact_agents.reports.adapters import ( + UnknownFrameworkError, + get_adapter, + ) - with pytest.raises((KeyError, ValueError)): + with pytest.raises(UnknownFrameworkError): get_adapter("claude_sdk", query_mode="raw")🤖 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/reports/test_adapter_claude_sdk_otf.py` around lines 262 - 274, The test test_adapter_registry_rejects_raw_query_mode currently asserts a broad (KeyError, ValueError) but get_adapter("claude_sdk", query_mode="raw") raises UnknownFrameworkError; update the test to import UnknownFrameworkError from the module that defines it (e.g., the adapter exceptions) and change the pytest.raises to expect UnknownFrameworkError when calling get_adapter in this test so it asserts the precise exception from get_adapter.
🤖 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/reports/test_adapter_claude_sdk_otf.py`:
- Around line 253-260: Update the test_adapter_registry_unknown_framework_errors
test to assert the specific exception type raised by get_adapter: import
UnknownFrameworkError from the package (the error class raised by get_adapter)
and replace the broad pytest.raises((KeyError, ValueError)) with
pytest.raises(UnknownFrameworkError) so the test checks for the concrete
UnknownFrameworkError thrown by get_adapter.
- Around line 262-274: The test test_adapter_registry_rejects_raw_query_mode
currently asserts a broad (KeyError, ValueError) but get_adapter("claude_sdk",
query_mode="raw") raises UnknownFrameworkError; update the test to import
UnknownFrameworkError from the module that defines it (e.g., the adapter
exceptions) and change the pytest.raises to expect UnknownFrameworkError when
calling get_adapter in this test so it asserts the precise exception from
get_adapter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8f0969b1-9bb1-4d5b-9361-8813a3beb9b1
📒 Files selected for processing (9)
src/bird_interact_agents/reports/adapters/__init__.pysrc/bird_interact_agents/reports/cli.pysrc/bird_interact_agents/reports/converter.pysrc/bird_interact_agents/reports/sources.pytests/cloud/test_submission_cli.pytests/reports/_fixtures.pytests/reports/test_adapter_claude_sdk_otf.pytests/reports/test_converter.pytests/reports/test_selection.py
🚧 Files skipped from review as they are similar to previous changes (5)
- src/bird_interact_agents/reports/sources.py
- tests/reports/test_selection.py
- tests/cloud/test_submission_cli.py
- tests/reports/test_converter.py
- tests/reports/_fixtures.py
…_results row Codex findings on PR #45. * Patience comes from the run-level cloud manifest, NOT the per-instance submission-annotation sidecar. Real cloud runs stamp `--patience 250` (or 500 for smokes) into `<results>/<benchmark>/cloud/<run-id>/ manifest.json`; the previous code only checked the per-instance sidecar (which never has patience) and fell back to the CLI default of 3 — replaying budgets with the wrong total and emitting incorrect `remaining_budget` in every prompt_flow row. New `_read_patience_for_ run` checks the run-level manifest (with the legacy `results/cloud/` path as a secondary candidate). Lookup order is now per-instance sidecar → run-level manifest → `--patience` default. * Selection entries that point to a trajectory.json present on disk but to an instance_id with NO matching `task_results` row now hard-error via `MissingTaskResultsError`. Previously the lookup fell back to scanning `runs/` and produced an InstanceSource with empty `mode`, which silently bypassed the a-Interact gate (the gate skips empty modes). Listing every offender is more useful than a single error. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… DSL Codex finding on PR #45. In SLayer mode the agent passes a SlayerQuery JSON DSL to `submit_query` (not SQL); the server compiles it to SQL and persists the compiled SQL as `trajectory.submitted_sql`. The previous converter slurped `tool_input["query_json"]` verbatim into `subtask_K_predicted_sql`, emitting JSON DSL where the leaderboard expects SQL — every real SLayer submission would have been ungradeable. New extraction: * If the submit's tool_input looks like JSON DSL (starts with `{`), pull the compiled SQL from `trajectory.submitted_sql` — but only for the LAST overall submit (the only one the harness persists). * Earlier-phase compiled SQL is unrecoverable from the trajectory (the harness overwrites `successful_phase1_sql` only in-memory and records just the final SQL on disk). Emit `[""]` plus a manifest warning so the operator sees the gap. Tests pin both the single- phase happy path AND the two-phase phase-1-lost case. * Raw-mode submits keep using the agent's literal SQL argument (`query` / `query_json` / `sql`), unchanged. The `action` field still records the JSON DSL the agent literally called — that's the audit trail of the call. Only `subtask_K_predicted_sql` becomes the compiled SQL. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/reports/test_converter.py (1)
92-103: 💤 Low valueRedundant re-imports inside test function.
The fixtures (
assistant_msg,build_trajectory,system_msg,tool_result_msg,tool_use_block,user_text_msg) are already imported at module level (lines 9-20). The same applies to lines 139-146.🤖 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/reports/test_converter.py` around lines 92 - 103, Remove the redundant local imports of fixtures inside the test function test_phase_sql_slayer_mode_uses_trajectory_submitted_sql and the similar duplicate import block around lines 139-146; the fixtures assistant_msg, build_trajectory, system_msg, tool_result_msg, tool_use_block, and user_text_msg are already imported at module scope, so delete the inner "from tests.reports._fixtures import (...)" statements and rely on the module-level imports instead to avoid duplicate imports.
🤖 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/reports/test_converter.py`:
- Around line 92-103: Remove the redundant local imports of fixtures inside the
test function test_phase_sql_slayer_mode_uses_trajectory_submitted_sql and the
similar duplicate import block around lines 139-146; the fixtures assistant_msg,
build_trajectory, system_msg, tool_result_msg, tool_use_block, and user_text_msg
are already imported at module scope, so delete the inner "from
tests.reports._fixtures import (...)" statements and rely on the module-level
imports instead to avoid duplicate imports.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4f105a85-d967-4ec5-afc4-ded4c4fcaabe
📒 Files selected for processing (2)
src/bird_interact_agents/reports/converter.pytests/reports/test_converter.py
…ts guard Codex findings on PR #45. * SLayer `submit_query.query_json` accepts either a single SlayerQuery object (`{...}`) OR a nested-DAG ARRAY of stage objects (`[...]` — see claude_sdk/agent.py:374). The compiled-SQL extraction detector previously only checked `startswith("{")`, so nested-DAG submits silently fell into the raw-SQL branch and emitted the JSON DSL string into `subtask_K_predicted_sql`. Detector now also matches `[` — both shapes route through `trajectory.submitted_sql`. * `task_results` carries a composite key including framework/mode/query_mode; in principle a corrupted DB could land multiple rows per `(run_id, instance_id)`. The previous code's dict- by-instance silently picked whichever row SQLite returned last, potentially landing on the wrong mode and flipping the a-Interact gate / adapter choice. Detect duplicates and raise listing every affected instance_id. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codex findings on PR #45. * The a-Interact mode gate previously short-circuited when `src.mode` was empty (`if src.mode and src.mode != "a-interact"`), so a row whose `mode` column is null/empty would silently pass. Round 3's `MissingTaskResultsError` already catches the no-row-at-all case; this is defense in depth for a row that EXISTS but has a blank mode. Change the gate to require `src.mode == "a-interact"` outright. * The duplicate `(run_id, instance_id)` detector in `_read_results_db` was added in round 5 but raised a bare `ValueError`, which the CLI catch-list did not cover. Promote it to a dedicated `DuplicateTaskResultsError(ValueError)` and add to the CLI's exception wrapper so corrupted DBs produce the same clean `error: ...` + exit-2 path as missing trajectories / missing rows rather than an uncaught traceback. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…e tool_uses Codex findings on PR #45. * CLI now hard-gates `query_mode == "slayer"` alongside the mode gate. Without it, an empty `query_mode` silently defaulted to "slayer" at the converter call site, and a truthy unsupported value like "raw" crashed inside `build_submission_row` with an uncaught `UnknownFrameworkError` traceback (the mode gate passed on a-interact). Both cases now produce the same clean SystemExit(2) with offenders listed. * Claude SDK trajectory walker preserves shared context across multiple `tool_use` blocks in the same AssistantMessage. Real runs typically emit one tool_use per assistant message, but the SDK allows multiple; the previous walker reset prompt/thinking/text after the FIRST tool_use, leaving subsequent Turns with an empty prompt and missing thinking/text. Now we collect all tool_uses from the message, then emit one Turn per tool sharing the same prompt + thinking + text — context reset happens after the whole message, not per-tool. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… path Codex findings on PR #45. * `--allow-partial` no longer permits a zero-instance submission. An empty selection file, or a `--run-id` whose results.db has no task_results rows, previously resolved to `selection=[]` → empty submission.jsonl + manifest.n_instances=0 (silently unsubmittable). Now aborts up front with a clear error. * Selection-file parse errors (DuplicateInstanceError, missing field, malformed JSON, OSError on open) now route through the same SystemExit(2) + stderr message path as resolve_sources / coverage errors instead of producing an uncaught traceback. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ry payload Codex finding on PR #45. The converter previously emitted whatever ``instance_id`` was stamped into the trajectory payload (`trajectory_obj["instance_id"]`). Coverage and source resolution are keyed off the SELECTION's instance_id; a stale or mis-copied trajectory file could therefore pass coverage for one id while emitting a submission row tagged with a different id (or an empty id), making the JSONL ungradeable or attributed to the wrong task. `build_submission_row` now accepts an optional `instance_id` parameter — the trusted id from the CLI's source resolution. When supplied: * The emitted SubmissionRow carries that id, not the trajectory's. * A mismatch between the trusted id and the trajectory's stamped id produces a manifest warning so the operator sees the stale-file gap. Unit tests that call the converter directly without a CLI source- resolution step continue to work by passing `instance_id=None`, in which case the trajectory's stamped id is the fallback. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…t observations SLayer-mode submits embed the compiled SQL in every tool_result as `Generated SQL:\n<sql>\n\nResult: ...` (_submit.py:806), so phase-1 SQL is recoverable per submit rather than only via the trajectory's final `submitted_sql`. Parse that marker first and fall back to `submitted_sql` for the last submit when it is absent, narrowing the manifest warning to the genuinely unrecoverable case. Recovered from uncommitted work lost in the 2026-07-31 disk-corruption incident; salvaged from an orphaned worktree on the backup drive. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Summary
bird-interact-cloud submission— converts an existing cloud run (or a per-instance selection across runs) into a BIRD-INTERACT-1.0 a-Interact submission directory (submission.jsonl+email_title.txt+manifest.json) ready to mail tobird.bench25@gmail.com.runs/<bench>/<db>/<id>/<run-id>.trajectory.json+results/<bench>/cloud/<run-id>/results.db— no harness-runtime changes; every historical cloud run becomes submittable on day 1.claude_sdk_otf_*framework family (4 agents, one shared adapter); benchmarksbird-interact-lite-exp/bird-interact-full/mini-interact.Key contract
ask=2 / submit=3 / execute=1; everything else token-aware (in<250 AND out<1000 → 0.5, else1.0). We do NOT export the harness's internalACTION_COSTStable — Section VI is the auditor-visible cost surface.total_budgetmirrorsharness.calculate_budget(task_data, patience, mode='a-interact') = 6 + 2*amb + 2*patienceso the leaderboard sees the same budget envelope the agent saw at runtime.Phase 1 SQL Correct! …/Phase 2 SQL Correct! …/Submitted SQL failed test case in Phase {1|2}. …). Missing/contradictory markers → per-instance manifest warning, never error.subtask_K_predicted_sqlis the literal-spec list type: length 1 with the final SQL for the phase,[]when the phase never ran.CLI extras
--no-thinkingstrips Claudethinkingblocks from each step'sresponsefield.--check-leakagescans everypromptfor case-insensitive substring matches againstground_truth_sql(≥12 chars). Per-instance count goes intomanifest.leakage_check. Diagnostic only — NEVER redacts; the leaderboard's 15-expert user-sim review is the authoritative leakage check.--patience N(default 3) is the fallback when the per-instance json carries no patience.--allow-partialis required when the selected instance set doesn't equal the full benchmark split (both--run-idand--selectionpaths).Hard errors
instance_idin selection.jsonl.(instance_id, run_id).trajectoryarray — older mini-interact placeholders).results.dbfor any run-id.--allow-partial.instance_idnot in the benchmark split (always, regardless of--allow-partial— that's a typo, not a partial run).Test plan
tests/reports/+tests/cloud/test_submission_cli.pycover: action canonicalisation, Section VI cost classifier (incl. AND-semantics at 250/1000 boundary), phase split (incl. missing/inconsistent markers), budget parity vs harness, claude_sdk_otf adapter walk, converter end-to-end, selection load + duplicate detection, source resolution + missing/stub errors, coverage gates (run-id + selection paths), output writer + JSONL schema validity, manifest provenance, token wrapper baseline subtraction + LRU cache,paths.reports_root()worktree-safety, leakage diagnostic, CLI argparse contract,--no-thinking,--check-leakage,--patience.Reviewed via /spec
--run-idpartial coverage tests, default include_thinking test, structural envelope check for thinking-strip, multi-submit inconsistent-marker case).🤖 Generated with Claude Code
Summary by CodeRabbit