Skip to content

execute_sql: a batch door, the model memoized, the connection kept open (ACE-137) - #323

Open
sandeep-agami wants to merge 2 commits into
ACE-136-report-built-by-codefrom
ACE-137-execute-sql-batch
Open

execute_sql: a batch door, the model memoized, the connection kept open (ACE-137)#323
sandeep-agami wants to merge 2 commits into
ACE-136-report-built-by-codefrom
ACE-137-execute-sql-batch

Conversation

@sandeep-agami

Copy link
Copy Markdown
Collaborator

Spec: ACE-137 (agami-sdlc projects/agami-core-extension/specs/reconcile-evidence/ACE-137-execute-sql-batch.md), lane high (the execution chokepoint).

Stacked on #322 (ACE-136). The last of round 9.

The finding

About 16 s per statement on the execute_sql tier, none of it the query: import 2 s, _model_safety 7 to 10 s, connect 4.3 s, query 0.09 s. Neither the resolved model nor the connection was reused within a process (four warm in-process calls: 22.9, 19.7, 16.5, 16.7 s), _resolve_guard_model ran twice per statement, and Phase 1.5 issues about ten statements per row, one process each.

The change (execute_sql.py, mirrored to plugins/agami/lib)

  • --batch <plan.json> [--manifest]: a JSON list of {id, sql | sql_file, out, area?}. Every item goes through execute_guarded on its own (guard, scope gates, refusal, recording); nothing is vetted once for many. Each CSV goes to out (empty when refused or failed, which is how the ledger reads a probe that did not run), the outcome to <out>.run.json in run.json's shape, and a manifest lists them all (stdout carries it). Exit 0 when every item ran, else the first non-ok item's single-statement exit code; a plan that is not a list of runnable items runs nothing (exit 2).
  • The semantic model memoized per process on the disk path: key (profile, root, YAML count and newest mtime), under a lock, never caching an absent or unreadable model, at most eight entries. The hosted (store) path is not memoized.
  • The connection kept open across a batch for Postgres, Redshift, Supabase (psycopg2) and SQLite (check_same_thread=False, since every statement runs on its own bounded thread), dropped after a statement that failed or was cancelled so the next item reconnects. Outside a batch every statement connects and closes exactly as before; the other engines connect per item inside a batch too.
  • statement-check.md step 8 runs a row's probes as one plan on this tier; SKILL.md 1.5d says so; connection-reference.md's CLI surface lists the door. CHANGELOG under Added.

Security notes for the reviewer

The batch is a loop over the chokepoint: the read-only guard, the model pass, the scope gates and the recording run per item, and a refusal is an item result, never a stop. The memo caches a parsed model object keyed by the files that produced it; None is never cached, so fail-closed behaviour is unchanged (test_ace051_fail_closed holds). Statement byte-identity (test_ace093) is untouched. The .run.json carries the classifier's message and the refusal rule, never the statement or the driver's text.

Tests

tests/test_execute_sql_batch.py: a three-item plan over SQLite opens one connection, writes three CSVs, three .run.json files and a manifest, exits 0, and the single door still connects per call; a refused write and a failed missing table leave empty CSVs with the rule or the kind (never the statement or the driver text) while the other items run; a bad plan runs nothing; a broken connection makes the next item reconnect; the model is resolved once until a YAML changes and an unreadable model is never cached. 352 passed across the executor seam, sanitization, exit codes, timeout, fail-closed, byte-identity, mirror, recording and in-process suites.

🤖 Generated with Claude Code

…n kept open across a batch

Every statement through the execute_sql tier paid an interpreter start, a full semantic-model load
(twice: the safety pass and the engine check) and a connect, about sixteen seconds for a query of a
tenth of a second, and reconcile's statement check issues about ten per row. `--batch <plan.json>`
runs a list of `{id, sql | sql_file, out, area?}` in one process: every item still goes through
`execute_guarded` on its own, its CSV goes to `out` (empty when refused or failed), its outcome to
`<out>.run.json` in run.json's shape, and a manifest lists them all. The resolved semantic model is
memoized per process on the disk path, keyed by the YAML files' count and newest mtime, never caching
an absent model; the hosted path is unchanged. The connection is kept open across a batch for the
Postgres family and SQLite and dropped after a statement that broke it; outside a batch every
statement connects and closes as before. The statement check runs a row's probes this way on the
execute_sql tier.

Spec: ACE-137

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The shipped references carry no spec ids; the one this round left in step 8 is gone.

Spec: ACE-137

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant