bird-agents: pre-copy template into per-task working sqlite (DEV-1509) - #13
Conversation
The dry-run gate (agents/_submit.py::_dry_run_sql) opens the per-task LiveSQLBench <db>.sqlite with ?mode=ro. Before this change, materialize_task_db only symlinked the template and left the working file uncreated — so anything that touched the path RW first (e.g. upstream get_db_connection's default-RW sqlite3.connect) left an empty SQLite there. The gate then returned OperationalError: no such table, which the agent misdiagnosed as a casing problem and burned 40-100 turns on. Pre-copying the template (atomically via tmp + os.replace) makes the invariant "when db_file_path is set, the file is a real SQLite with the template's data" hold from the moment materialize_ task_db returns. The template entry in the per-task dir stays a symlink (DEV-1462 storage invariant); only the WORKING <db>.sqlite is copied (one per task instance). The fast-path idempotence guard now also requires the SQLite magic header so 0-byte and foreign-shape (LFS pointer) files trigger a rebuild. Both the symlink and working-file installs use per-call uuid-suffixed tmp paths + os.replace so two concurrent materialize_task_db calls on the same instance_id cannot collide. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 54 minutes and 18 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review now! |
|
✅ Actions performedFull review triggered. |
Summary
OperationalError: no such table: artifactscorefrom the dry-run gate (agents/_submit.py::_dry_run_sql) and burned 40-100 turns rationalising it as a casing problem. The Linear issue (DEV-1509) framed it as a SLayer-introspection-vs-SQLite casing mismatch; investigation showed that framing is wrong.harness.materialize_task_dbonly symlinked the template at<id>/<db>_template.sqliteand left the working<id>/<db>.sqliteuncreated. Anything that touched the path RW before the upstream's firstreset_and_restore_database(most likely upstreamget_db_connection's default-RWsqlite3.connect, which creates the file if missing) left an empty SQLite atdb_file_path. The gate'ssqlite3.connect("file:<db_file_path>?mode=ro", uri=True)then produced preciselyno such table: <name>for every referenced table — empirically reproduced.materialize_task_dbnow pre-copies the canonical template to the working<db>.sqliteviashutil.copy2+os.replace(atomic). The per-task<db>_template.sqliteentry remains a SYMLINK (DEV-1462 storage invariant: 18 templates × N tasks would blow storage). The fast-path idempotence guard additionally checks the SQLite magic header (catches 0-byte AND foreign shapes like LFS pointers). Both the symlink and working-file installs use per-call uuid-suffixed tmp paths +os.replace, so two concurrent materialize_task_db calls on the same instance_id cannot collide.ensure_db_cache) is intentionally skipped — slayer ingest already preserves SQLite-stored casing verbatim and SQLite identifier lookup is case-insensitive. A new testtest_slayer_ingest_preserves_table_casing_and_sqlite_case_insensitivepins both invariants as documentation-via-code so future readers understand why the casing framing was rejected.Tests
15 new tests in
tests/test_db_isolation_pre_copy.py. All 9 existingtests/test_db_isolation.pytests still pass. Full non-integration suite: 1514 passed, 94 skipped.Coverage map:
_dry_run_sql+sqlite3.connect(?mode=ro)succeed against the materialised filedb_file_pathset)reset_and_restore_databasestill works after pre-copyshutil.copy2failure leavesexpected_db_fileuntouchedTest plan
pytest tests/test_db_isolation_pre_copy.py tests/test_db_isolation.py -v— 24/24 passenv -u SSH_AUTH_SOCK uv run --extra all --extra dev --extra pydantic-ai pytest— 1514 passed20260530t0958-claudes-slayer-b278f1and confirm the casing rabbit hole does not reappear🤖 Generated with Claude Code