diff --git a/docs/plans/20260520-adr-0033-phase-3.md b/docs/plans/20260520-adr-0033-phase-3.md new file mode 100644 index 0000000..42e3a3a --- /dev/null +++ b/docs/plans/20260520-adr-0033-phase-3.md @@ -0,0 +1,1472 @@ +# Implementation plan — ADR-0033 (ADR-0030 Phase 3) + +**Plan author date:** 2026-05-20 +**Driving ADR:** [ADR-0033 — The full SQL DML grammar (INSERT / UPDATE / DELETE)](../adr/0033-sql-dml-grammar.md) +**Parent ADR:** [ADR-0030 — Advanced mode: the standard-SQL surface](../adr/0030-advanced-mode-sql-surface.md) + +## Purpose + +ADR-0033 is the *decision*: what Phase 3 covers, how dispatch +works on shared entry words, what diagnostics land, what the +execute-path Command variants look like. This plan is the +*mechanics*: the build order (3a → 3k), the per-sub-phase exit +gates, the cross-cut verifications that prevent the kind of +silent gap Phase 1 shipped (ADR-0032 §11.6) and the kind of +matrix-attribution slippage Phase 2's first DA pass let through +(handoff-28 §3.4). + +The plan does **not** re-litigate ADR-0033's decisions. Any +finding during implementation that suggests a decision should +change is escalated to the user and resolved by ADR amendment +(or a new ADR) before code proceeds — never by silent drift. + +## Why this plan exists + +Phase 2 closed clean but the closing DA review needed a second +pass to surface four production gaps. Phase 3 introduces a +structural risk Phase 2 didn't have: **a new walker capability +(`Node::Guard`) sits at the foundation**, and every later +sub-phase depends on it working. If 3a ships broken-but-believed- +green, 3b–3j build on broken scaffolding. The plan exists to +make 3a's verification rigorous and to keep every "X comes for +free" claim from ADR-0033 explicit and tested. + +The three failure modes the plan is built to prevent: + +- **The Guard mechanism shipping without proof that it fires + BEFORE the Seq commits.** R1 in ADR-0033's Open Implementation + Risks; 3a's DA gate makes this the make-or-break test. +- **A "free for free" claim shipping without a test that proves + the claim.** Every "X works for SQL DML automatically" in + ADR-0030/0031/0032/0033 needs an explicit cross-cut test. +- **Matrix attribution pointing at a DSL test for a SQL claim.** + Handoff-28 §3.4: rows pointing at a SQL surface must point at + a test whose INPUT is SQL syntax. Cross-checked at attribution + time, not at DA-review time. + +## Working-mode reminder + +Per `CLAUDE.md`'s "Working mode — solo with sub-agents by +default": this work proceeds in solo mode unless the user +explicitly invokes `/team`. The phased structure below maps to +solo-mode's Phase 1 → 5 discipline within each sub-phase: + +- Requirements extraction (the sub-phase's scope below) is + Phase 1. +- Divergent exploration is rarely needed for individual + sub-phases here (ADR-0033 already selected the approach), but + if implementation reveals an ambiguity, it lands as a Phase-2 + exploration before code proceeds. +- The DA hat is worn at every sub-phase gate. The DA critique + is **written** with specific findings BEFORE the verdict; a + clean PASS without listed findings is a process failure even + if the work is sound (handoff-28 §4.1; handoff-29 §4.1). +- Escalation rather than guessing applies throughout. "Out of + scope" / "non-blocking" require user confirmation + (handoff-29 §4.2). + +## Standing authorizations and continuation policy + +The user granted these authorizations for Phase 2; absent a +fresh statement to the contrary, they extend to Phase 3 +because the work pattern is identical. They are recorded here +verbatim from `docs/plans/20260520-adr-0032-phase-2.md` so any +implementer (the original session, a future session, or a +fresh agent) operates on the same terms. **If the user has +narrowed these for Phase 3, the narrowest interpretation +wins.** + +### 1. Walk the plan without interruption when no interruption is needed + +The plan's sub-phases (3a–3k) have explicit, written exit +gates. Within a sub-phase, the implementer proceeds through +the build steps and verifies the gate without asking for +permission at each step. Between sub-phases, the implementer +proceeds to the next one without asking, *provided* the +previous gate is green. + +The implementer does NOT interrupt to: + +- Confirm "should I continue to the next sub-phase?" +- Confirm minor implementation choices that match surrounding + code (`CLAUDE.md`'s code-style discipline applies; pick the + consistent option and move on). +- Confirm test naming, file organisation, commit granularity + within a sub-phase. +- Report progress mid-step. A status update arrives at gate + boundaries — not every five minutes. + +### 2. When the implementer MUST escalate + +The escalation rule from `CLAUDE.md` ("Escalate ambiguity — do +not decide for the user") is **not** relaxed. Standing +authorization to walk uninterrupted is *not* authorization to +guess on design questions. The implementer escalates if: + +- An exit gate cannot be made green by work in the current + sub-phase's scope — a test fails for a reason that suggests + the ADR is wrong, or a behaviour the gate requires turns out + to need machinery not in ADR-0033. +- An ambiguity arises that the ADR and this plan do not settle. + See "Open questions to escalate before code starts" at the + end of this plan — those are pre-identified. Anything + similar that surfaces is also escalation territory. +- An autonomous design decision is required (a choice outside + ADR-0033 §§1–13's settled list and the plan's per-sub-phase + scope). Per `CLAUDE.md`'s "Out of scope and non-blocking + require user confirmation": the default assumption is that + the work is in scope and is blocking unless the user has + said otherwise (handoff-29 §4.2 pin). +- A "real blocker" is encountered — environment, dependency, + or test-infrastructure failure that prevents progress and + cannot be resolved within the sub-phase. A flaky test is + not a real blocker — investigate per `CLAUDE.md`'s "no + excuses" rule on test infrastructure. + +### 3. Commits are pre-authorized at standard granularity + +The user has explicitly pre-authorized commits within this +plan's scope, overriding `CLAUDE.md`'s "All commits MUST be +confirmed by the user first" for this plan. The implementer: + +- **Commits at natural break points within each sub-phase** and + always at sub-phase gate boundaries. Granular is better than + monolithic. +- **Writes detailed commit messages** in the project's existing + style — `area: short subject` first line, then a body + explaining what changed and why. Body should reference the + ADR section being implemented (e.g., "ADR-0033 §2 — Guard + mechanism, option (a) chosen") so a reader can trace each + commit back to a decision. +- **Includes NO AI attribution** of any form (`Co-Authored-By`, + generated-by lines, AI footers, …). This is a global rule + (`CLAUDE.md`); the standing authorization here is for + *committing*, not for relaxing attribution discipline. +- **Names the mechanism choice in 3a's commit message** — + option (a) / (b) / (c) per ADR-0033 §2. If 3a fell back to + (b) or (c), update ADR-0033 §2's "Default choice" sentence + in the same commit (per handoff-29 §3 step 6). + +### 4. Pushes remain a user step + +`CLAUDE.md`'s "Push is a user step — agents cannot and shall +never do it" is **not** overridden. The implementer commits +freely; the user pushes when they choose. Unpushed commits are +a normal working state, and the implementer never describes +work as blocked on a push. + +### 5. End-of-plan handoff + +At the end of 3k, after the verification report is produced +and the DA's final PASS verdict is recorded (with specific +critiques listed first, not a rubber-stamp), the implementer +**stops and surfaces a summary to the user** — exact test +counts, the filled cross-cut matrix, the requirements-to-test +mapping, any autonomous decisions made and their +justification. The user confirms acceptance before the plan +is considered done. This is the one explicit hand-back point. + +If the user wants follow-up work — adjustments, polish, +additional tests — the implementer addresses it and re-runs +the gate before re-surfacing. The plan does not declare +itself complete; the user does. + +## Sub-phase overview + +| Sub-phase | Title | Test-suite gate | +|-----------|------------------------------------------------|----------------------------------------------------------------------------------| +| **3a** | `Node::Guard` + mode-gated Choice mechanism | Smoke-test grammar: 4 mode × shape combinations green; R1 invariant proven | +| **3b** | INSERT grammar + minimal execution | Single + multi-row VALUES end-to-end; CSV re-persisted; `__rdbms_*` rejected | +| **3c** | INSERT … SELECT | Compound row source + WITH-prefixed SELECT row source end-to-end | +| **3d** | `shortid` auto-fill (worker) | Auto-fill on VALUES (single/multi) + INSERT…SELECT; override warning path | +| **3e** | UPDATE grammar + execution | Multi-column SET, with/without WHERE; sql_expr in SET works | +| **3f** | DELETE grammar + execution + cascade summary | Cascade parity with DSL; WHERE-with-subquery case (R2) | +| **3g** | RETURNING | All three statement kinds; result-column type recovery via Phase-2 path | +| **3h** | UPSERT (`ON CONFLICT … DO NOTHING / DO UPDATE`) | Both branches; `excluded.col` resolves to target table's columns | +| **3i** | Diagnostics | Three new keys: positive + negative each; per-row arity emit verified | +| **3j** | Dispatch wiring on shared entry words | Existing DSL tests still green; SQL inputs route via shared CommandNode | +| **3k** | Verification sweep | Cross-cut matrix every row green; phase-exit verification report | + +Each sub-phase ships independently: green tests, clippy clean, +a written DA gate review with specific critiques listed first, +and a commit (pre-authorized per §3 above). + +--- + +## Sub-phase 3a — `Node::Guard` + mode-gated Choice mechanism + +**This sub-phase is the foundation.** If the Guard mechanism +doesn't work cleanly, ADR-0033 §2's mitigation paths +(mechanism options (b) or (c)) get evaluated **before** any +DML grammar lands on top. + +### Scope (in) + +- Add a new walker node variant + `Node::Guard(fn(&WalkContext) -> Result<(), ValidationError>)` + to `src/dsl/grammar/mod.rs::Node` — zero-byte consumption, + fails the enclosing Seq with `ValidationFailed` when the + validator returns Err. +- Implement `walk_guard` in `src/dsl/walker/driver.rs`. Match + arm in `walk_node_inner`. +- Add the `reject_sql_in_simple_mode` guard function. Returns + `ValidationError { message_key: "advanced_mode.sql_in_simple", + args: … }` in Simple mode, `Ok(())` in Advanced. +- Add the catalog key `advanced_mode.sql_in_simple` with + engine-neutral wording matching the existing `select` / + `with` gating pattern (ADR-0030 §2). +- Build the smoke-test grammar: a single experimental + `CommandNode` whose `shape` is + `Choice([Seq[Guard(reject_sql_in_simple_mode), + SQL_marker_token], DSL_marker_token])` using two + distinguishable, non-DML marker tokens (e.g., synthetic + `__sql_marker` / `__dsl_marker`) so the smoke test exercises + the mechanism without depending on any DML grammar. +- Verify the four mode × input cases listed in Exit gate + below. + +### Scope (out — explicit) + +- Any real DML grammar — that is 3b onwards. +- Wiring `data::INSERT/UPDATE/DELETE` shapes — that is 3j. +- Any other guard-based gating beyond `reject_sql_in_simple_mode` + — the mechanism is reusable but only this guard is + required by this sub-phase. + +### Build steps + +1. Add `Node::Guard` variant. Cover the match-arm in every + exhaustive `match Node` in the walker / grammar / hint + helpers — clippy will surface incomplete arms. +2. Implement `walk_guard` in the driver. Push zero bytes; + return `WalkOutcome::ValidationFailed` from the Err path. +3. Author `reject_sql_in_simple_mode` in + `src/dsl/grammar/mod.rs` (or a sibling module if it fits + better there). +4. Author the catalog key + an engine-neutral i18n message. +5. Build the smoke-test grammar in a temporary file + (`src/dsl/grammar/_guard_smoke.rs` or similar; remove + before 3a's commit, OR keep it as a unit-test fixture in + the walker driver tests — implementer's choice as long as + it doesn't leak into the runtime CommandNode registry). +6. Author the four exit-gate tests. +7. Author the R1 invariant test (DA gate below). +8. `cargo test`, `cargo clippy --all-targets -- -D warnings`. + +### Exit gate + +**Required green tests:** + +- **Smoke-test grammar — Simple mode + DSL-shape input:** the + DSL branch matches. (Baseline: Choice fall-through works.) +- **Smoke-test grammar — Advanced mode + SQL-shape input:** + the SQL branch matches. +- **Smoke-test grammar — Simple mode + SQL-shape input:** + `WalkOutcome::ValidationFailed` with + `message_key: "advanced_mode.sql_in_simple"`. (The Guard + fires; no other branch is attempted because the SQL marker + consumed bytes after the Guard pushed empty.) +- **Smoke-test grammar — Advanced mode + DSL-shape input:** + the DSL branch matches. (Choice fall-through works even + when the SQL branch is admissible — the Guard returned Ok + but the marker token didn't match the input, so the SQL + Seq fails NoMatch and Choice falls through to DSL.) + +**R1 invariant test (this is the make-or-break case):** + +- **Advanced mode + an input whose first tokens match the SQL + branch's Guard-then-marker pattern but the SQL branch's + *later* tokens fail, while the DSL branch's tokens would + match the full input.** The expected outcome is: + - If the Choice falls through to DSL → R1 mechanism (a) + works as designed; 3a is green. + - If the Choice commits to SQL and reports Failed → R1 + mechanism (a) is broken in the same way ADR-0033 §2's + open risk anticipated; sub-phase REOPENS with mechanism + option (b) or (c). + + Note: in 3a's smoke-test grammar the SQL and DSL markers + are different, so the simplest exercise of this invariant + uses a 2-token SQL branch (`SQL_marker_A`, `SQL_tail_X`) + and an input `SQL_marker_A SQL_tail_Y` where `SQL_tail_Y` + isn't admissible to the SQL branch but is admissible to + the DSL branch. The Choice MUST fall through. + +**Other gates:** + +- `cargo test` total: 1446 (baseline) + the new smoke-test + + R1 invariant tests; expect ≥ 1450 (rough order). +- Zero failures, one ignored (the unchanged doctest), zero + skipped. +- `cargo clippy --all-targets -- -D warnings` clean. + +### DA gate (written) + +The DA hat is worn explicitly here. Specific critiques listed +first, verdict last (handoff-29 §4.1 pin): + +- **Does the Guard actually fire BEFORE the Seq commits?** + Read `walk_seq` / `walk_choice` in + `src/dsl/walker/driver.rs`. Trace the path: Choice picks + branch 0 → walk_seq enters → walk_guard runs → if Err, + Seq's outcome is Failed → does walk_choice see Failed and + fall through to branch 1, or does it commit on branch + 0's "tried but failed"? The R1 invariant test above is + the empirical answer; the DA reads the code and confirms + the test's assertion matches the code path. +- **Is the Guard mechanism reusable beyond DML?** Phase 4 + (DDL) is the likely future consumer. If yes, document the + pattern in `src/dsl/grammar/mod.rs` doc comments so the + next phase doesn't reinvent it. +- **Does `reject_sql_in_simple_mode`'s catalog key match the + existing `select` / `with` entry-word gate's wording + posture?** Inconsistent UX between entry-word gates and + branch gates is a real gap; the DA reads + `src/dsl/grammar/mod.rs::ADVANCED_ONLY_ENTRIES` and + confirms the catalog key + i18n string land at the same + user-facing message shape. +- **Did 3a introduce any structure ADR-0033 §2 doesn't + sanction?** A new node variant added; a new validator + signature shape; a new catalog key. If anything beyond + these landed, that's an autonomous decision to surface. +- **If the R1 invariant test fails, was the fallback path + taken cleanly?** The fallback path means: (a) ADR-0033 §2 + amended with the chosen mechanism, (b) the smoke-test + grammar rewritten on (b) or (c), (c) the gate's tests + pass on the new mechanism, (d) the commit message names + the mechanism that landed. + +--- + +## Sub-phase 3b — INSERT grammar + minimal execution + +### Scope (in) + +- Author `src/dsl/grammar/sql_insert.rs` exporting + `pub static SQL_INSERT_SHAPE: Node` (the per ADR-0033 §1 + shape minus row-source SELECT, RETURNING, UPSERT — those + land in 3c/3g/3h). +- The shape covers: `INSERT INTO table_name + [ '(' column_name_list ')' ] VALUES value_tuple ( ',' value_tuple )* [ ';' ]`. +- The `__rdbms_*` rejection on the table-name slot (per + ADR-0030 §6 / ADR-0032 §1's `reject_internal_table` + validator). +- `Command::SqlInsert { sql, source, target_table, + listed_columns, returning: false }` — `returning` always + false in 3b. +- `Request::RunSqlInsert` + `do_sql_insert` worker handler + (no shortid auto-fill yet — explicit values required for + every column). +- Persistence write-through on the target table after + execution (ADR-0030 §11). +- History-log of the literal submitted line (ADR-0030 §11). +- A standalone development CommandNode (e.g., `sql_insert`) + in the registry so 3b–3i can exercise the SQL INSERT path + without depending on 3j's dispatch wiring on shared entry + words. Removed in 3j. + +### Scope (out — explicit) + +- `INSERT … SELECT` — that is 3c. +- `RETURNING` — that is 3g. +- `UPSERT` — that is 3h. +- `shortid` auto-fill — that is 3d. +- Walker diagnostics specific to INSERT (`insert_arity_mismatch`, + `auto_column_overridden`, `not_null_missing`) — that is 3i. +- DSL-vs-SQL dispatch wiring on `data::INSERT` — that is 3j. + +### Build steps + +1. Author `sql_insert.rs` with the value-list shape, plus + reject-internal-table on the target slot. +2. Add `Command::SqlInsert` with the field list above. +3. Add `Request::RunSqlInsert` and `do_sql_insert`. +4. Wire history-log write-before-execute (mirror + `do_run_select`'s pattern). +5. Wire persistence write-through after execute. +6. Build the development CommandNode (`sql_insert`) and + register it. +7. Unit + integration tests per the exit gate. + +### Exit gate + +**Required green tests:** + +- **Single-row INSERT runs end-to-end** — + `INSERT INTO orders (customer_id, total) VALUES (1, 99.50)` + succeeds; affected-row count surfaces; CSV is re-persisted + with the new row. +- **Multi-row INSERT runs end-to-end** — + `INSERT INTO orders VALUES (1, 'a'), (2, 'b')` succeeds; + both rows land; CSV reflects both. +- **Column-list variant** — + `INSERT INTO t (a, b) VALUES (1, 'x')` succeeds; the + unmentioned columns get their defaults / NULLs. +- **No column-list variant** — + `INSERT INTO t VALUES (…)` accepts the table's full column + arity in the tuple. +- **`__rdbms_*` rejection** at the INSERT target-table slot + (per ADR-0030 §6) — parse-rejects. +- **History line** is the literal submitted SQL. +- **Failed INSERT (engine error)** does NOT re-persist the + CSV — invariant: persistence happens only on engine + success. +- All Phase-2 tests (1446 baseline) still green. + +**Other gates:** + +- `cargo clippy --all-targets -- -D warnings` clean. + +### DA gate (written) + +- **Does an INSERT failing mid-multi-row leave persistence in + a consistent state?** SQLite's default is per-statement + transactional — verify the worker doesn't re-persist when + the engine returns an error. +- **Does the `target_table` extraction handle case-folding + the same way the rest of the codebase does?** ADR-0009: + identifiers are case-preserving but ADR-0012's metadata + uses canonical casing for lookups. Verify the path. +- **Are the existing DSL INSERT tests still green?** Any + DSL-side regression means 3b broke something it shouldn't + have. Run the DSL INSERT test module explicitly. +- **Does the history-log line capture pre-engine state?** + ADR-0030 §11: the literal submitted line is logged before + execution. Verify the order (log → execute) and that a + failing INSERT still leaves a log entry (for replay). +- **Did 3b introduce any structure ADR-0033 §1 doesn't + sanction?** + +--- + +## Sub-phase 3c — INSERT … SELECT + +### Scope (in) + +- Extend `SQL_INSERT_SHAPE`'s row-source slot to be a Choice + between `values_clause` and + `Subgrammar(&sql_select::SQL_SELECT_COMPOUND)`. +- The Phase-2 SELECT machinery applies for free — CTEs + (Amendment 2 admits leading `WITH`), JOINs, set ops, + subqueries, scope-frame harvest. +- `do_sql_insert` handles the SELECT-driven path; the engine + handles the insert-from-query flow (no special composite + preparation needed). + +### Scope (out — explicit) + +- Arity diagnostic (`insert_arity_mismatch`) — that is 3i; + 3c's only verification of arity is what the engine reports + via the friendly layer. +- Schema-existence on the SELECT's projection — comes for + free from Phase-2 machinery (ADR-0032 §11); 3c's job is to + cross-cut-verify it fires on a DML SELECT slot, not to + re-implement. + +### Build steps + +1. Wrap the existing values_clause in a Choice with the + subgrammar reference. +2. Verify the development CommandNode admits both row + sources. +3. Tests per the exit gate, including the WITH-prefixed + case (R4 invariant). + +### Exit gate + +**Required green tests:** + +- **Plain INSERT … SELECT** — + `INSERT INTO archive SELECT * FROM orders WHERE created < '2025-01-01'` + runs; the rows land in archive; archive's CSV reflects + them. +- **Column-list + projection** — + `INSERT INTO target (a, b) SELECT x, y FROM source` + runs. +- **WITH-prefixed SELECT row source (R4)** — + `INSERT INTO archive WITH t AS (SELECT * FROM orders) + SELECT * FROM t` runs. (This proves the Amendment-2 + carry-through.) +- **Schema-existence diagnostic on the SELECT's projection + (cross-cut)** — + `INSERT INTO archive SELECT nonexistent_col FROM orders` + fires `diagnostic.unknown_column` (or the equivalent + Phase-2 key) on the projection — the Phase-2 pass + applies here for free; the test pins the claim. +- All 3a–3b tests still green; baseline 1446 still green. + +**Other gates:** + +- `cargo clippy --all-targets -- -D warnings` clean. + +### DA gate (written) + +- **Does the WITH-prefixed SELECT row source actually parse + through `SQL_SELECT_COMPOUND`?** Or does the INSERT prefix + break the Phase-2 recursion? The R4 test above pins this; + the DA reads `sql_select::SQL_SELECT_COMPOUND` to confirm + the entry shape admits `WITH`. +- **Does the cascade-summary path (3f DELETE) accidentally + fire for INSERTs whose SELECT touches the target table?** + It must not — INSERTs have no cascade output. 3c's tests + shouldn't see a cascade summary even when the source is + the same as the target. +- **Does a SELECT-driven INSERT respect the + `__rdbms_*` rejection on the SELECT's `FROM` slot?** + Phase-2 already gates this; 3c verifies it doesn't + silently regress on the DML path. + +--- + +## Sub-phase 3d — `shortid` auto-fill (worker) + +### Scope (in) + +- Worker-side post-fill in `do_sql_insert` (per ADR-0033 §6). +- Detect omitted `shortid` columns by intersecting the + user's `listed_columns` against the target table's + auto-gen column list from `__rdbms_playground_columns`. +- Synthesise fresh shortids per row (the existing + `shortid::generate` path). +- Rewrite the executed SQL to bind the synthesised values + positionally (or use parameter binding — implementer's + choice, but the original submitted SQL stays unchanged in + `history.log`). +- Cover both row sources: VALUES (single + multi-row) and + SELECT (each yielded row gets a fresh shortid for the + omitted auto-gen columns). + +### Scope (out — explicit) + +- The `auto_column_overridden` walker WARNING — that is 3i. +- `serial` auto-fill — handled by SQLite rowid; the worker + has no work to do for `serial` columns. + +### Build steps + +1. Locate the existing DSL `do_insert`'s shortid auto-fill + logic. Refactor as needed so the SQL path can call the + same generator (the generator itself, not the surrounding + typed `Command::Insert` plumbing). +2. In `do_sql_insert`, after parsing `listed_columns` vs + table schema, identify omitted auto-gen columns. +3. For VALUES, synthesise per-row shortids and bind them. +4. For SELECT, the strategy needs an explicit design call: + - **Option A:** Rewrite SQL to wrap the SELECT, projecting + a synthesised shortid alongside (heavier; touches the + SQL text). + - **Option B:** Run the SELECT first into a result set, + synthesise per-row shortids in the worker, then INSERT + the augmented rows row-by-row (simpler; loses the + engine's set-based optimisation). + - **ESCALATE this choice to the user.** Both options are + valid; ADR-0033 §6 doesn't pick. Plan default if the + user is unavailable: **Option B** (simpler, matches the + pedagogical posture; performance is not a goal). +5. Unit + integration tests per the exit gate. + +### Exit gate + +**Required green tests:** + +- **VALUES single-row auto-fill** — + `INSERT INTO t (name) VALUES ('x')` where `t` has `id + shortid pk` auto-fills `id` with a fresh shortid; + `SELECT * FROM t` shows the synthesised id. +- **VALUES multi-row distinct shortids** — + `INSERT INTO t (name) VALUES ('a'), ('b'), ('c')` + yields three rows with three DISTINCT shortid values. +- **Explicit value respected (override)** — + `INSERT INTO t (id, name) VALUES ('hardcoded', 'x')` + preserves `'hardcoded'`; the warning fires in 3i but + the value is honoured here. +- **INSERT … SELECT auto-fills shortids** — + `INSERT INTO target (name) SELECT name FROM source` + where `target.id` is shortid produces fresh shortids + for every yielded row (verified distinct). +- **Combined `serial` + `shortid` columns** — + on a table with both, `serial` is engine-filled, `shortid` + is worker-filled, no collision. +- All 3a–3c tests still green; baseline 1446 still green. + +**Other gates:** + +- `cargo clippy --all-targets -- -D warnings` clean. + +### DA gate (written) + +- **Are the synthesised shortids actually unique across a + multi-row INSERT?** The generator must be called per-row, + not once and reused. The multi-row distinct test pins + this. +- **Does the auto-fill respect the column's case-preserved + name?** ADR-0009. Verify with a mixed-case shortid column + name. +- **What happens if the table has both `serial` and + `shortid` PK columns?** Spec: serial via SQLite rowid; + shortid via worker post-fill. Verify no double-fill + collision and no double-insertion. +- **Did the SELECT row-source path choose option A or B, + and was it escalated to the user before landing?** If the + implementer chose without escalating, that's an autonomous + decision and 3d's DA verdict is FAIL until the user + confirms. + +--- + +## Sub-phase 3e — UPDATE grammar + execution + +### Scope (in) + +- Author `src/dsl/grammar/sql_update.rs` exporting + `pub static SQL_UPDATE_SHAPE: Node`: + `UPDATE table_name SET assignment_list [ where_clause ] [ ';' ]`. +- The `__rdbms_*` rejection on the target slot. +- `assignment_list` reuses `sql_expr::SQL_OR_EXPR` for the + RHS of each assignment. +- `Command::SqlUpdate`, `Request::RunSqlUpdate`, + `do_sql_update`. +- Persistence write-through on the target table. +- A development CommandNode `sql_update` until 3j. +- No `--all-rows` rail (ADR-0030 §12: SQL UPDATE without + WHERE executes as written). + +### Scope (out — explicit) + +- `RETURNING` — that is 3g. +- `UPDATE … FROM other_table` — OOS-3 in ADR-0033 §13. +- Walker WARNING on UPDATE-touching-auto-gen-column — that + warning lands in 3i if the implementer extends + `auto_column_overridden`'s scope; the spec leaves it as an + INSERT-only diagnostic per ADR-0033 §8.2. If the + implementer wants to extend, escalate. + +### Build steps + +1. Author the shape. +2. Add Command/Request/handler. +3. Wire history-log + persistence. +4. Register the dev CommandNode. +5. Tests per the exit gate. + +### Exit gate + +**Required green tests:** + +- **Single-column UPDATE with WHERE** — + `UPDATE t SET v = 'x' WHERE id = 1` runs; affected-row + count surfaces; CSV reflects the change. +- **Multi-column UPDATE** — + `UPDATE t SET a = 1, b = 2 WHERE id = 1` runs. +- **UPDATE without WHERE** — + `UPDATE t SET active = false` runs across all rows (per + ADR-0030 §12, no `--all-rows` gate); CSV reflects all + rows changed. +- **UPDATE with sql_expr in SET** — + `UPDATE t SET total = price * quantity WHERE …` runs; + the engine evaluates the expression. +- **Schema-existence diagnostic fires** on unknown columns + in SET (cross-cut from Phase-2 machinery). +- **Predicate warning fires** on a bad WHERE + (cross-cut from Phase-2 machinery — `= NULL` in the + WHERE.) +- All 3a–3d tests still green; baseline 1446 still green. + +**Other gates:** + +- `cargo clippy --all-targets -- -D warnings` clean. + +### DA gate (written) + +- **Does the SET assignment expression accept the full + `sql_expr` (function calls, subqueries, CASE)?** Verify + with `SET v = (SELECT max(other) FROM other_table)` and + `SET v = CASE WHEN x > 0 THEN x ELSE 0 END`. +- **Does an UPDATE without WHERE actually run as written + with no rail?** Per ADR-0030 §12. The test pins the claim. +- **Does the persistence write-through fire even when + affected-row count is 0?** Edge case: an UPDATE that + matches no rows. Worker should still treat the operation + as successful (engine returned OK); CSV doesn't need to + rewrite if nothing changed, but the path mustn't crash. +- **Did 3e regress any DSL UPDATE behaviour?** + +--- + +## Sub-phase 3f — DELETE grammar + execution + cascade summary + +### Scope (in) + +- Author `src/dsl/grammar/sql_delete.rs` exporting + `pub static SQL_DELETE_SHAPE: Node`: + `DELETE FROM table_name [ where_clause ] [ ';' ]`. +- The `__rdbms_*` rejection on the target slot. +- `Command::SqlDelete`, `Request::RunSqlDelete`, + `do_sql_delete`. +- Cascade-summary pre-count per ADR-0033 §7: extract WHERE + bytes from the matched path, inject into pre-count + subqueries for each child table FK with `ON DELETE + CASCADE` / `SET NULL`. +- Cascade-summary formatter shared with `do_delete` (DSL + path) — one formatter, two callers (ADR-0033 §7's + shared-formatter promise). +- Persistence write-through on target + every cascade- + affected child table (multi-table persistence is new for + SQL DML; the DSL path already does this). +- Development CommandNode `sql_delete` until 3j. + +### Scope (out — explicit) + +- `RETURNING` — that is 3g; 3f leaves the cascade summary + + affected-count as the only output for now. +- `DELETE … FROM other_table` — not a thing in standard + SQL; OOS by silence. + +### Build steps + +1. Author the shape. +2. Extract the WHERE-byte-range capture in the walker's + ast-builder for `SQL_DELETE_SHAPE` — the matched path + spans the WHERE node; the ast-builder pulls the source + bytes covered. +3. Refactor `format_cascade_summary` from `do_delete` so it + accepts both the DSL typed-Expr path and a string-WHERE + path. Implementer's call whether to extract via two + pre-count strategies or one unified pre-count constructor + that accepts either input form. Plan default: extract the + pre-count constructor, keep formatting unified. +4. Wire `do_sql_delete`: pre-count → execute → format + summary → re-persist target + every child whose row + count changed. +5. Tests per the exit gate, including R2's WHERE-with- + subquery case. + +### Exit gate + +**Required green tests:** + +- **Plain DELETE with WHERE** — + `DELETE FROM orders WHERE id = 1` runs; affected-row + count + cascade summary surface; target CSV re-persisted. +- **DELETE without WHERE** — + `DELETE FROM orders` runs across all rows; cascade + summary covers all child rows; target + every cascade- + affected child CSV re-persisted. +- **Cascade parity with DSL** — on the same schema and + data, running `DELETE FROM customers WHERE id = 1` via + the SQL path produces the same per-relationship summary + as running the DSL `delete customers where id = 1`. +- **R2 invariant: WHERE-with-subquery** — + `DELETE FROM orders WHERE customer_id IN (SELECT id FROM + customers WHERE country = 'DE')` runs; the cascade pre- + count uses the same WHERE bytes (the subquery runs N+1 + times — once for the count, once for the DELETE; the + test pins correctness, not performance). +- **Cascade pre-count runs BEFORE execute** — verify order + by exercising a case where the pre-count would yield 0 + if it ran after the DELETE (i.e., the rows being deleted + are exactly the ones whose children are pre-counted). +- **Cascade-affected children CSVs re-persisted** — after + a cascading DELETE, every affected child table's CSV + reflects the post-state (rows gone for `ON DELETE + CASCADE`, NULL for `SET NULL`). +- **`__rdbms_*` rejection** at the DELETE target slot. +- All 3a–3e tests still green; baseline 1446 still green. + +**Other gates:** + +- `cargo clippy --all-targets -- -D warnings` clean. + +### DA gate (written) + +- **R2: does the predicate-extraction byte-range injection + work on a WHERE containing a subquery?** Test + `DELETE FROM a WHERE id IN (SELECT id FROM b WHERE …)`. + The pre-count SQL becomes + `SELECT count(*) FROM child WHERE fk_col IN (SELECT pk_col + FROM a WHERE id IN (SELECT id FROM b WHERE …))`. The + subquery runs twice (R2's noted performance concern). + The DA reads `do_sql_delete`'s pre-count construction to + confirm the byte range covers the entire WHERE (including + the inner subquery) and not just the top-level predicate. +- **Does the cascade pre-count run in a way that doesn't + see the post-DELETE state?** Order matters: pre-count + BEFORE execute. Verified by the order test above; DA + reads the code to confirm. +- **Is the cascade-summary formatter actually shared?** No + duplicate copy of the formatter — one function, two + callers (DSL + SQL). +- **Are cascade-affected child CSVs identified by querying + the metadata tables for `ON DELETE CASCADE` / `SET NULL` + relationships, or by walking the result of the cascade?** + Implementer's choice; DA confirms whichever path is + taken is sound and matches DSL `do_delete`. + +--- + +## Sub-phase 3g — RETURNING + +### Scope (in) + +- Add `RETURNING projection_list` as an optional tail on + all three SQL DML shapes (SQL_INSERT_SHAPE, + SQL_UPDATE_SHAPE, SQL_DELETE_SHAPE). +- `projection_list` is the Phase-2 projection list, + imported unchanged. +- Walker's `ast_builder` for each shape sets + `returning: bool` true when the RETURNING tail matched. +- Worker handlers branch on `returning`: + - `true` → prepare + step + collect rows into a + `DataResult`. Reuse the SELECT result-column type + recovery (`resolve_select_column_types` in `db.rs`, + ADR-0032 §12 + Amendment 1). + - `false` → execute, collect affected-row count + (current 3b/3e/3f behaviour). +- DELETE … RETURNING preserves the cascade summary + alongside the result set — both surface to the user. + +### Scope (out — explicit) + +- The Command variant shape changes — `returning: bool` is + already in the spec (ADR-0033 §10); 3b set it false, 3g + flips it. + +### Build steps + +1. Add the RETURNING tail to each shape. +2. Update each ast_builder to set `returning`. +3. Update each worker handler to branch. +4. Reuse `resolve_select_column_types` for the prepared + statement's column-origin metadata. +5. For DELETE + RETURNING: ensure cascade pre-count still + runs and its summary surfaces alongside the rows. +6. Tests per the exit gate. + +### Exit gate + +**Required green tests:** + +- **INSERT … RETURNING \*** — + `INSERT INTO t (a) VALUES (1) RETURNING *` returns the + inserted row as a DataResult; affected-row count is + implicit (rows.len() = 1). +- **INSERT multi-row … RETURNING id** — + `INSERT INTO t (a) VALUES (1), (2), (3) RETURNING id` + returns three rows; ids distinct. +- **UPDATE … RETURNING id, new_value** — + `UPDATE t SET v = 'x' WHERE id = 1 RETURNING id, v` + returns the modified columns of the affected row. +- **DELETE … RETURNING \*** — + `DELETE FROM t WHERE id = 1 RETURNING *` returns the + pre-DELETE row (BEFORE it's gone). +- **DELETE … RETURNING + cascade summary** — + on a parent DELETE with cascade children, the user sees + both the RETURNING result set AND the cascade + per-relationship summary. +- **Result-column type recovery on RETURNING** — for each + of the ten playground types, a bare-column RETURNING + reference recovers the playground type via the Phase-2 + column-origin path. (At least one test per type; bundle + into a single parametric test for compactness.) +- **Computed expression in RETURNING stays typeless** — + `RETURNING a + 1` yields `column_types[0] = None`. +- **R3: RETURNING on multi-row INSERT** — column-origin is + the same for all yielded rows (per ADR-0032 Amendment 1); + one test pins it. +- All 3a–3f tests still green; baseline 1446 still green. + +**Other gates:** + +- `cargo clippy --all-targets -- -D warnings` clean. + +### DA gate (written) + +- **Does the RETURNING result-set rendering use the same + DataResult path SELECT uses?** Or did a DML-specific + copy sneak in? Read the renderer; confirm one path, not + two. +- **Does `RETURNING expr AS alias` admit aliases the same + way the projection list does?** The projection list is + Phase-2 machinery; 3g imports unchanged. Verify via a + test. +- **For DELETE + RETURNING, does the cascade pre-count + fire correctly?** The cascade summary is per-relationship + child rows affected — independent of whether the user + asked for RETURNING. Both must surface. +- **Result-column type recovery test count: ten types?** + All ten playground types covered (text, int, real, + decimal, bool, date, datetime, blob, serial, shortid). + +--- + +## Sub-phase 3h — UPSERT (`ON CONFLICT … DO NOTHING / DO UPDATE`) + +### Scope (in) + +- Add `on_conflict_clause` to `SQL_INSERT_SHAPE`: + - Optional `(column_name_list)` conflict target. + - `DO NOTHING` branch. + - `DO UPDATE SET assignment_list [ where_clause ]` branch. +- `excluded` admitted as a table alias inside DO UPDATE + expressions only — its columns are the target table's + columns (per ADR-0033 §9). Scoped to the DO UPDATE + branch. +- Walker captures the UPSERT shape; worker executes the + validated SQL as-is (no special pre-processing). + +### Scope (out — explicit) + +- `INSERT OR REPLACE / IGNORE / ABORT / FAIL / ROLLBACK` — + OOS-2 in ADR-0033 §13. + +### Build steps + +1. Author the on_conflict_clause node tree. +2. Wire the conflict target column-name list to use the + same `Columns` ident slot as the column-name list at the + top of INSERT. +3. Author the `excluded` pseudo-alias machinery: a special + `TableBinding` pushed onto the current frame's + from_scope ONLY while walking the DO UPDATE expressions. + The binding's columns are sourced from the target + table's schema cache entry. +4. Tests per the exit gate. + +### Exit gate + +**Required green tests:** + +- **DO NOTHING on conflict** — + `INSERT INTO t (id, name) VALUES (1, 'x') + ON CONFLICT (id) DO NOTHING` on a pre-existing `id = 1` + runs; affected-row count is 0; CSV unchanged. +- **DO UPDATE with excluded** — + `INSERT INTO t (id, name) VALUES (1, 'new') + ON CONFLICT (id) DO UPDATE SET name = excluded.name` + on a pre-existing `id = 1` runs; the row's name is + updated to `'new'`; CSV reflects the change. +- **DO NOTHING without target spec** — + `INSERT INTO t … ON CONFLICT DO NOTHING` handles any + conflict. +- **`excluded.col` resolves to the target table's + columns** (cross-cut completion / highlight test: + Tab at `… DO UPDATE SET name = excluded.|` lists + the target table's columns). +- **`excluded` does NOT leak outside DO UPDATE** — + parsing `INSERT INTO t VALUES (excluded.name, …)` (with + `excluded` used in the value list, not inside DO UPDATE) + fires `diagnostic.unknown_qualifier` or equivalent. The + alias is strictly scoped to the DO UPDATE branch. +- **Persistence write-through on DO UPDATE** — yes; the + target row is modified. +- All 3a–3g tests still green; baseline 1446 still green. + +**Other gates:** + +- `cargo clippy --all-targets -- -D warnings` clean. + +### DA gate (written) + +- **Does ON CONFLICT … DO UPDATE trigger persistence + write-through?** Yes; the target row is modified. The + worker doesn't accidentally bypass re-persistence on the + upsert path. +- **Is the `excluded` alias scoped to ONLY the DO UPDATE + branch, not leaking into the outer INSERT?** The leak + test above pins this. +- **Is `excluded` engine-neutral?** No — it's SQLite + + PostgreSQL spelling (standard SQL uses `MERGE … WHEN + MATCHED`). ADR-0033 §9 carves this out explicitly; the + DA confirms the carve-out is justified (it is) and that + the catalog wording referencing UPSERT doesn't leak + product names. +- **Does the conflict target column list reject + `__rdbms_*` columns?** The columns are owned by the + target table; if the target is rejected at the outer + table slot, the inner column list is by construction + fine. Sanity-verify. + +--- + +## Sub-phase 3i — Diagnostics + +### Scope (in) + +- `diagnostic.insert_arity_mismatch` (ERROR, ADR-0033 §8.1): + walker pre-flight when `(column_name_list)`'s arity + disagrees with a value_tuple's arity (per-row for + multi-row VALUES), or with the SELECT's projection arity + for INSERT…SELECT. +- `diagnostic.auto_column_overridden` (WARNING, ADR-0033 §8.2): + walker emit when INSERT explicitly assigns a value to a + `serial` / `shortid` column. +- `diagnostic.not_null_missing` (WARNING, ADR-0033 §8.3): + walker emit when INSERT's `(column_name_list)` omits a + NOT-NULL-no-default column. +- Per-key positive + negative tests (the ADR-0032 §2d + pattern reused). +- Catalog entries with engine-neutral wording. + +### Scope (out — explicit) + +- Engine-side translations (FK / UNIQUE / NOT-NULL on + execute) — ADR-0033 §11 reuses existing keys; no new + ones needed. +- Predicate warnings on DML's sql_expr slots — inherited + from Phase-2 machinery; 3i cross-cuts-verifies they + fire on DML slots but doesn't re-implement. + +### Build steps + +1. Author the three walker diagnostic passes (or extend + existing passes if cleaner — implementer's call). +2. Author the catalog keys + i18n strings. +3. Author the positive + negative tests per key. +4. Verify per-row emit for multi-row arity mismatch. +5. Cross-cut-verify the inherited Phase-2 passes fire on + DML sql_expr slots. + +### Exit gate + +**Required green tests:** + +- **`insert_arity_mismatch` positive (single-row)** — + `INSERT INTO t (a, b) VALUES (1, 2, 3)` fires. +- **`insert_arity_mismatch` negative (matched arity)** — + `INSERT INTO t (a, b) VALUES (1, 2)` doesn't fire. +- **`insert_arity_mismatch` per-row emit (multi-row)** — + `INSERT INTO t (a, b) VALUES (1, 2), (3, 4, 5), (6), (7, 8)` + fires on rows 2 and 3, NOT on rows 1 and 4 (two + diagnostics, two right rows silent). +- **`insert_arity_mismatch` on INSERT…SELECT** — + `INSERT INTO t (a) SELECT a, b FROM source` fires; + anchored on the SELECT's first projection_item span. +- **`auto_column_overridden` positive** — + `INSERT INTO t (id, name) VALUES (5, 'x')` (id is + `serial`) fires WARNING; the statement still runs. +- **`auto_column_overridden` negative** — omitting `id` + doesn't fire. +- **`auto_column_overridden` on shortid** — same as serial + but `id` is `shortid`. +- **`not_null_missing` positive** — + `INSERT INTO t (a) VALUES (1)` (b is NOT NULL no + default) fires WARNING; the statement still parses (the + engine rejects on execute). +- **`not_null_missing` negative** — including `b` doesn't + fire. +- **`not_null_missing` does NOT fire for columns with a + default value** — even if NOT NULL. +- **Cross-cut: schema-existence diagnostic fires on + INSERT VALUES** — `INSERT INTO t (nonexistent) VALUES (1)` + fires the Phase-2 key on the column name. +- **Cross-cut: schema-existence fires on UPDATE SET** — + `UPDATE t SET nonexistent = 1 WHERE id = 1` fires. +- **Cross-cut: schema-existence fires on UPDATE / DELETE + WHERE** — already verified in 3e/3f exit gates; + re-reference here. +- **Cross-cut: predicate warnings fire on DML WHERE / SET + / VALUES** — `UPDATE t SET v = 1 WHERE x = NULL` fires + `eq_null`. +- All 3a–3h tests still green; baseline 1446 still green. + +**Other gates:** + +- `cargo clippy --all-targets -- -D warnings` clean. + +### DA gate (written) + +- **Does each new key have BOTH a positive and a negative + test?** Walk the keys; verify both rows above. +- **Is `auto_column_overridden` a Warning, not an Error?** + Read the diagnostic's severity field; confirm. +- **Is `not_null_missing` a Warning, not an Error?** Same. +- **Are the catalog wordings engine-neutral?** No "SQLite", + "PRAGMA", "STRICT", or "rusqlite" leak. Verify by + reading the i18n strings. +- **Does multi-row arity-mismatch actually emit per-row?** + The test pins it; the DA reads the code to confirm the + emit happens inside the per-row visit, not once for the + values_clause as a whole. + +--- + +## Sub-phase 3j — Dispatch wiring on shared entry words + +### Scope (in) + +- Convert `data::INSERT`, `data::UPDATE`, `data::DELETE` + CommandNode shapes from their current DSL-only form to + `Choice([SQL__SHAPE, DSL__SHAPE])` per ADR-0033 §2. +- Wire `Node::Guard(reject_sql_in_simple_mode)` at the + head of each SQL__SHAPE's Seq (the 3a mechanism). +- Remove the development CommandNodes (`sql_insert` / + `sql_update` / `sql_delete`) from the registry. +- Verify the public test surface: existing DSL tests use + unchanged inputs and see unchanged outputs; new SQL + tests reach the SQL branch via the shared entry words. + +### Scope (out — explicit) + +- Any behaviour change. This is a wiring refactor: + observable behaviour must be identical before and after + 3j. + +### Build steps + +1. Convert each shape. The Choice declares SQL first, DSL + second (per ADR-0033 §2). +2. Insert the Guard at the head of each SQL branch's Seq. +3. Update every test that referenced `sql_insert` / + `sql_update` / `sql_delete` to use `insert` / `update` / + `delete` instead. +4. Remove the development CommandNodes. +5. Run the full suite. + +### Exit gate + +**Required green tests:** + +- **All existing DSL INSERT/UPDATE/DELETE tests still + green** — unmodified inputs, unmodified outputs. +- **All Phase-3 SQL DML tests still green via the new + dispatch path** — the SQL branch is reached through + `data::INSERT` etc., not through the development + CommandNodes. Verifiable: grep for `sql_insert` / + `sql_update` / `sql_delete` and confirm zero remaining + references in src/dsl/grammar/. +- **Structurally-ambiguous input routes SQL-first in + Advanced** — + `delete from t where id = 1` in Advanced mode goes + through the SQL DELETE branch (verifiable: a SQL-only + side effect fires, e.g., the cascade-summary code path + shared by both — pin by checking a unique-to-SQL trace + point, OR by verifying that a SQL-syntax-only construct + on the same input matches). +- **DSL-specific input falls through to DSL in Advanced** — + `delete from t --all-rows` in Advanced matches the DSL + branch (the `--all-rows` flag is DSL-only, so the SQL + Seq's NoMatch lets Choice fall through). +- **Simple mode + SQL input fires the Guard** — + `delete from t where id = 1 returning *` (DML+RETURNING, + clearly SQL) in Simple mode produces + `ValidationFailed` with + `message_key: "advanced_mode.sql_in_simple"`. +- **R1 invariant holds in real grammar** — the smoke-test + from 3a was minimal; 3j is the first time the Guard + mechanism sees real DML branches with overlapping + prefixes. A specific test: in Advanced mode, an INSERT + that the SQL branch parses partway and then rejects + (e.g., a malformed `RETURNING` tail that's also not + a valid DSL fragment) — the Choice's behaviour is what + ADR-0033 §2 dictates: SQL > DSL ordering means the SQL + branch's Failed propagates as the Choice's outcome (no + fallback because the DSL branch wouldn't match either). + The test pins the user-facing error wording. +- All 3a–3i tests still green; baseline 1446 still green. + +**Other gates:** + +- `cargo clippy --all-targets -- -D warnings` clean. + +### DA gate (written) + +- **Verify that no test was changed to make 3j pass.** Only + the CommandNode shape's structure changes; the + observable behaviour at the public test surface is + identical for inputs that worked before. Grep the diff + for tests whose `INPUT` strings changed in 3j — there + should be none for existing DSL tests; the Phase-3 SQL + tests' inputs are the same (only their dispatch path + differs). +- **Is the test count higher than before 3j?** It should + be the same or higher (no tests removed, possibly a + handful added for the structurally-ambiguous + fall- + through cases above). +- **R1 hold-up in real grammar?** The 3a smoke-test pinned + the invariant on synthetic tokens; the DA confirms it + holds on real DML. + +--- + +## Sub-phase 3k — Verification sweep + +### Scope (in) + +The catch-all sub-phase. Verifies the full end-to-end Phase 3 +surface and every cross-cut "X comes for free" claim. Produces +the **final phase-exit verification report** (next section). + +### Build steps + +1. Author the end-to-end integration tests (Tier 3) for + real-world-shape DML queries (the exit-gate list below). +2. Author or extend the cross-cut verification matrix (next + section) for every "X comes for free" claim from + ADR-0030/0031/0032/0033. +3. Run the full test suite. Confirm 1446 baseline + all + sub-phase 3a–3j additions + the 3k integration tests. +4. Run `cargo clippy --all-targets -- -D warnings`. +5. Produce the final verification report. + +### Exit gate + +**Required end-to-end integration tests:** + +- **INSERT … SELECT cross-table** — + `INSERT INTO archive SELECT * FROM orders WHERE created + < '2025-01-01'` — runs, archive has the rows, both CSVs + reflect state, renderer clean. +- **Multi-row INSERT with all ten playground types** — + every type lands in a row via a multi-row INSERT; + values round-trip correctly. +- **UPDATE with subquery in SET** — + `UPDATE customers SET last_order_date = (SELECT max(date) + FROM orders WHERE customer_id = customers.id)` runs; + values updated; CSV reflects. +- **DELETE with cascade** — end-to-end including + per-relationship summary, multi-table CSV re-persistence. +- **UPSERT round-trip** — DO UPDATE updates an existing + row; DO NOTHING on a separate conflict no-ops; both + surface their expected outcomes. +- **RETURNING on each of INSERT / UPDATE / DELETE** — each + produces a DataResult; result-column types recovered. +- **`history.log` replay**: every Phase-3 statement form + written above replays from the log faithfully (ADR-0030 + §11 unchanged for DML). + +**Cross-cut verification matrix** (next section): every +claim must have a passing test, with the test's INPUT +being SQL syntax for SQL claims (handoff-29 §4.4 pin). + +**Other gates:** + +- Total test count: ≥ 1446 + (additions from 3a–3k). The + exact count depends on per-step authorship; the gate is + "every required-test row above and in the cross-cut + matrix has at least one green test", not a magic number. +- Zero failures, zero skipped (excluding the unchanged + doctest). +- `cargo clippy --all-targets -- -D warnings` clean. + +### DA gate (written) + +The final DA review, per CLAUDE.md Phase-5 verification and +handoff-29 §4.1 pin (specific critiques first, verdict +last; rubber-stamp PASS is a process failure): + +- **Walk the cross-cut matrix row-by-row.** Every row's + test must (a) exist, (b) be green, (c) have INPUT that + matches the row's claim source (SQL claims → SQL + inputs). +- **Are there any "free for free" claims from + ADR-0030/0031/0032/0033 that have no explicit test?** + Phase-1 gap pattern, Phase-2's four DA findings. Each + must have one. +- **Did any sub-phase silently downgrade a requirement to + "later"?** Phase-2's "out of scope" defer-trap reflex. + Any "not in scope" call by the implementer that wasn't + user-confirmed is a verdict FAIL until escalation + closes (handoff-29 §4.2 pin). +- **Did any sub-phase make an autonomous decision that's + not in ADR-0033?** Specific items to check: the 3d + Option A/B choice (must be user-confirmed if Option A + was chosen unilaterally); any new catalog keys beyond + the three in §8.1–§8.3; any new Command variants beyond + the three in §10. +- **Are ALL tests green AND zero-skipped?** If not, the + verdict is FAIL and 3k loops back to whichever sub-phase + introduced the gap. + +--- + +## Cross-cut verification matrix + +Each row is a claim from ADR-0030, ADR-0031, ADR-0032, or +ADR-0033 that needs an explicit test, with the test's +location. The matrix is filled in during 3k; the gate is +"every row green AND every SQL claim's test takes SQL +input" (handoff-29 §4.4 pin). + +| Claim source | Claim | Test location | Status | +|------------------------|--------------------------------------------------------------------------------|---------------|--------| +| **Statement shapes (§1)** | | | | +| ADR-0033 §1 | Single-row VALUES INSERT runs end-to-end | TBD (3b) | ⏳ | +| ADR-0033 §1 | Multi-row VALUES INSERT runs end-to-end | TBD (3b) | ⏳ | +| ADR-0033 §1 | INSERT with `(column_name_list)` runs | TBD (3b) | ⏳ | +| ADR-0033 §1 | INSERT without column list (full table arity) runs | TBD (3b) | ⏳ | +| ADR-0033 §4 | `INSERT … SELECT` runs | TBD (3c) | ⏳ | +| ADR-0033 §4 / R4 | `INSERT … WITH … SELECT` row source runs (Amendment-2 carry-through) | TBD (3c) | ⏳ | +| ADR-0033 §1 | UPDATE with WHERE runs | TBD (3e) | ⏳ | +| ADR-0033 §1 / §12 | UPDATE without WHERE runs (no `--all-rows` rail) | TBD (3e) | ⏳ | +| ADR-0033 §1 | UPDATE with multi-column SET | TBD (3e) | ⏳ | +| ADR-0033 §1 | UPDATE with sql_expr in SET (function call, subquery, CASE) | TBD (3e) | ⏳ | +| ADR-0033 §1 | DELETE with WHERE runs | TBD (3f) | ⏳ | +| ADR-0033 §1 / §12 | DELETE without WHERE runs (no `--all-rows` rail) | TBD (3f) | ⏳ | +| **Dispatch (§2)** | | | | +| ADR-0033 §2 / 3a | Guard fires BEFORE Seq commits (R1 invariant on synthetic markers) | TBD (3a) | ⏳ | +| ADR-0033 §2 | Simple mode + DSL INSERT → DSL branch | TBD (3j) | ⏳ | +| ADR-0033 §2 | Simple mode + SQL INSERT (RETURNING) → ValidationFailed `advanced_mode.sql_in_simple` | TBD (3j) | ⏳ | +| ADR-0033 §2 | Advanced + structurally-ambiguous (`delete from t where id = 1`) → SQL branch | TBD (3j) | ⏳ | +| ADR-0033 §2 | Advanced + DSL-only (`--all-rows`) → DSL branch (Choice falls through) | TBD (3j) | ⏳ | +| ADR-0033 §2 | Same for UPDATE shared entry word | TBD (3j) | ⏳ | +| ADR-0033 §2 | Same for INSERT shared entry word | TBD (3j) | ⏳ | +| **RETURNING (§5 / §12)** | | | | +| ADR-0033 §5 | INSERT … RETURNING * surfaces DataResult | TBD (3g) | ⏳ | +| ADR-0033 §5 | UPDATE … RETURNING cols surfaces DataResult | TBD (3g) | ⏳ | +| ADR-0033 §5 | DELETE … RETURNING * surfaces DataResult (pre-DELETE row) | TBD (3g) | ⏳ | +| ADR-0033 §5 | DELETE … RETURNING preserves cascade summary alongside result set | TBD (3g) | ⏳ | +| ADR-0033 §12 | Result-column type recovery for each of ten playground types via RETURNING | TBD (3g) | ⏳ | +| ADR-0033 §12 | Computed expression in RETURNING stays typeless | TBD (3g) | ⏳ | +| ADR-0033 §5 / R3 | Multi-row INSERT … RETURNING: column-origin same for all rows | TBD (3g) | ⏳ | +| **shortid auto-fill (§6)** | | | | +| ADR-0033 §6 | Single-row VALUES auto-fills omitted shortid PK | TBD (3d) | ⏳ | +| ADR-0033 §6 | Multi-row VALUES yields DISTINCT shortids per row | TBD (3d) | ⏳ | +| ADR-0033 §6 | INSERT … SELECT auto-fills shortids per yielded row | TBD (3d) | ⏳ | +| ADR-0033 §6 | Explicit value for shortid column respected (override; warning in §8.2) | TBD (3d) | ⏳ | +| ADR-0033 §6 | serial + shortid combo: no double-fill collision | TBD (3d) | ⏳ | +| **Cascade summary (§7)** | | | | +| ADR-0033 §7 | DSL parity: same schema/data, SQL DELETE produces same per-relationship summary | TBD (3f) | ⏳ | +| ADR-0033 §7 / R2 | WHERE-with-subquery: cascade pre-count uses correct byte range | TBD (3f) | ⏳ | +| ADR-0033 §7 | Pre-count runs BEFORE execute | TBD (3f) | ⏳ | +| ADR-0033 §7 | Cascade-affected child tables re-persisted | TBD (3f) | ⏳ | +| ADR-0033 §7 | Cascade-summary formatter shared (one function, two callers) | TBD (3f) | ⏳ | +| **Diagnostics (§8)** | | | | +| ADR-0033 §8.1 | `insert_arity_mismatch` positive (single-row) | TBD (3i) | ⏳ | +| ADR-0033 §8.1 | `insert_arity_mismatch` negative (matched arity) | TBD (3i) | ⏳ | +| ADR-0033 §8.1 | `insert_arity_mismatch` per-row (multi-row VALUES) | TBD (3i) | ⏳ | +| ADR-0033 §8.1 | `insert_arity_mismatch` on INSERT…SELECT (projection arity) | TBD (3i) | ⏳ | +| ADR-0033 §8.2 | `auto_column_overridden` positive (serial) | TBD (3i) | ⏳ | +| ADR-0033 §8.2 | `auto_column_overridden` positive (shortid) | TBD (3i) | ⏳ | +| ADR-0033 §8.2 | `auto_column_overridden` negative (omitted) | TBD (3i) | ⏳ | +| ADR-0033 §8.3 | `not_null_missing` positive | TBD (3i) | ⏳ | +| ADR-0033 §8.3 | `not_null_missing` negative | TBD (3i) | ⏳ | +| ADR-0033 §8.3 | `not_null_missing` does NOT fire for NOT-NULL-with-default | TBD (3i) | ⏳ | +| **UPSERT (§9)** | | | | +| ADR-0033 §9 | ON CONFLICT (col) DO NOTHING runs; no-ops on conflict | TBD (3h) | ⏳ | +| ADR-0033 §9 | ON CONFLICT (col) DO UPDATE SET … = excluded.… runs; row updated | TBD (3h) | ⏳ | +| ADR-0033 §9 | ON CONFLICT DO NOTHING (no target spec) accepts any conflict | TBD (3h) | ⏳ | +| ADR-0033 §9 | `excluded.col` completes to target table's columns inside DO UPDATE | TBD (3h) | ⏳ | +| ADR-0033 §9 | `excluded` rejected outside DO UPDATE (scoping) | TBD (3h) | ⏳ | +| **Inherited Phase-2 diagnostics on DML slots (§8.4)** | | | | +| ADR-0032 §11 | `schema_existence` fires on INSERT VALUES column | TBD (3i) | ⏳ | +| ADR-0032 §11 | `schema_existence` fires on UPDATE SET | TBD (3i) | ⏳ | +| ADR-0032 §11 | `schema_existence` fires on UPDATE / DELETE WHERE | TBD (3i) | ⏳ | +| ADR-0032 §11 | `schema_existence` fires on RETURNING projection | TBD (3i / 3g) | ⏳ | +| ADR-0032 §11.6 | Predicate warning `eq_null` fires on UPDATE WHERE | TBD (3i) | ⏳ | +| ADR-0032 §11.6 | Predicate warning `like_numeric` fires on DELETE WHERE | TBD (3i) | ⏳ | +| ADR-0032 §11.6 | Predicate warning fires inside INSERT VALUES sql_expr (e.g., `=` with NULL) | TBD (3i) | ⏳ | +| **Ambient assistance (§§ from ADR-0030 §8)** | | | | +| ADR-0030 §8 | Syntax highlighting works for SQL DML keywords (INSERT, UPDATE, DELETE, INTO, VALUES, SET, RETURNING, ON, CONFLICT) | TBD (3k) | ⏳ | +| ADR-0030 §8 | Tab completion works for SQL DML entry keywords in Advanced mode | TBD (3k) | ⏳ | +| ADR-0030 §8 | Tab completion works for target table after `INSERT INTO ` | TBD (3k) | ⏳ | +| ADR-0030 §8 | Tab completion works for column names inside `(column_list)` | TBD (3k) | ⏳ | +| ADR-0030 §8 | Tab completion works for column names in UPDATE SET / WHERE | TBD (3k) | ⏳ | +| ADR-0030 §8 | Hint-panel prose appears at representative DML slots | TBD (3k) | ⏳ | +| ADR-0030 §8 | `[ERR]`/`[WRN]` validity indicator fires for SQL DML diagnostics | TBD (3k) | ⏳ | +| ADR-0030 §8 | Per-command parse-error usage fires for SQL DML | TBD (3k) | ⏳ | +| **Engine-neutrality + safety** | | | | +| ADR-0030 §6 / ADR-0033 §1 | `__rdbms_*` rejection at INSERT target slot | TBD (3b) | ⏳ | +| ADR-0030 §6 / ADR-0033 §1 | `__rdbms_*` rejection at UPDATE target slot | TBD (3e) | ⏳ | +| ADR-0030 §6 / ADR-0033 §1 | `__rdbms_*` rejection at DELETE target slot | TBD (3f) | ⏳ | +| ADR-0030 §6 / ADR-0033 §1 | `__rdbms_*` rejection in INSERT…SELECT row source's FROM | TBD (3c) | ⏳ | +| ADR-0030 §7 / ADR-0033 §11 | Engine UNIQUE constraint failure surfaces via friendly layer (engine-neutral) | TBD (3k) | ⏳ | +| ADR-0030 §7 / ADR-0033 §11 | Engine NOT NULL constraint failure surfaces via friendly layer | TBD (3k) | ⏳ | +| ADR-0030 §7 / ADR-0033 §11 | Engine FK constraint failure on DELETE-without-cascade surfaces | TBD (3k) | ⏳ | +| **Persistence + history (§10–§11)** | | | | +| ADR-0030 §11 | INSERT writes history.log + re-persists target CSV | TBD (3b) | ⏳ | +| ADR-0030 §11 | UPDATE writes history.log + re-persists target CSV | TBD (3e) | ⏳ | +| ADR-0030 §11 | DELETE writes history.log + re-persists target + cascade-affected CSVs | TBD (3f) | ⏳ | +| ADR-0030 §11 | Every Phase-3 statement form replays from history.log faithfully | TBD (3k) | ⏳ | +| ADR-0033 §10 | INSERT failure does NOT re-persist CSV | TBD (3b) | ⏳ | +| ADR-0006 / ADR-0033 §10 | Auto-snapshot fires for SQL DML the same way it does for DSL DML | TBD (3k) | ⏳ | +| **OOS rejections (§13)** | | | | +| ADR-0033 §13 OOS-1 | `INSERT INTO t DEFAULT VALUES` parse-rejects | TBD (3b) | ⏳ | +| ADR-0033 §13 OOS-2 | `INSERT OR REPLACE` parse-rejects | TBD (3b) | ⏳ | +| ADR-0033 §13 OOS-3 | `UPDATE … FROM other_table` parse-rejects | TBD (3e) | ⏳ | +| ADR-0033 §13 OOS-4 | `WITH x AS (…) UPDATE …` parse-rejects | TBD (3e) | ⏳ | +| ADR-0033 §13 OOS-4 | `WITH x AS (…) DELETE …` parse-rejects | TBD (3f) | ⏳ | + +The implementer fills in `Test location` and `Status` +(✅ / ❌) as 3k proceeds. A row marked red blocks the +phase exit. + +**Attribution rule (handoff-29 §4.4 pin):** for any row +whose Claim source is a SQL claim, the `Test location` must +point at a test whose INPUT is SQL syntax. Cross-checked +at attribution time, not at DA-review time. + +--- + +## Final phase-exit verification report + +Produced at the end of 3k. A markdown document at +`docs/handoff/-phase-3-verification.md` (matching the +Phase-2 naming pattern). The report contains: + +1. **Test suite totals** — passed / failed / skipped / + ignored, exact numbers from `cargo test` output. + Compared against the Phase-2 baseline (1446 / 0 / 0 / 1). +2. **The full cross-cut verification matrix** with every + row green. +3. **A requirements-to-test mapping** — every numbered + decision in ADR-0033 §§1–13 with the test(s) that prove + it. +4. **A list of any autonomous decisions** made during + implementation, with rationale and explicit user- + confirmation pointer for each. (Per CLAUDE.md: an + autonomous decision without explicit user confirmation + is a verdict FAIL.) +5. **The DA's written final review** — with specific + critiques listed first and the PASS / FAIL verdict + last. No "conditional" verdicts (handoff-28 §4.1 pin). + +Only after the report is produced and signed off does the +user-facing "Phase 3 complete" message issue. + +## Open questions to escalate before code starts + +These are matters the plan deliberately leaves to the +implementer to escalate when reached, *not* to decide +silently: + +1. **shortid auto-fill on INSERT … SELECT (3d).** Option A + (SQL rewrite) vs Option B (worker materialisation). + ADR-0033 §6 doesn't pick. Plan default if the user is + unavailable: Option B (simpler, matches pedagogical + posture). Implementer escalates BEFORE 3d code lands; + if user unavailable, choice is recorded in the + verification report under "autonomous decisions" for + retroactive confirmation. +2. **R1 mechanism fallback (3a).** If option (a) (Guard + validator) doesn't work, options (b) and (c) per + ADR-0033 §2's enumeration. The plan does NOT pre-pick; + if (a) fails, the implementer evaluates (b) and (c) + and brings a recommendation to the user. +3. **Cascade-summary pre-count construction (3f).** Two + pre-count strategies (typed-Expr from DSL, byte-range + string from SQL) vs one unified constructor. Plan + default: unified constructor that accepts either input + form, with the formatter alone shared. Implementer's + call; not an escalation unless a third option surfaces. +4. **UPSERT engine-neutral catalog wording (3h).** The + `excluded` keyword is engine-neutral-carved-out per + ADR-0033 §9; the surrounding catalog wording (e.g., + the help-text for an UPSERT statement, the error + message if `excluded` is used outside DO UPDATE) must + not leak SQLite product names. If the implementer + isn't sure, escalate before writing the i18n string. + +## What this plan does NOT contain + +- Time estimates. Phase 3 is medium-sized but introduces + R1's structural risk; calibration is poor. Milestones, + not hours. +- Sub-phase commit granularity beyond "ship green tests + at each gate". The implementer commits at natural break + points within a sub-phase; the gate is what gets + verified. +- Re-statements of ADR-0033's decisions. The plan refers + to ADR-0033 by section; the implementer reads both. +- Process pins from the prior session that don't change + anything mechanical. They land in the implementer's + context via handoff-29 §4, not via duplication here.