# 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 | `ins::single_row_insert_persists_and_counts` | ✅ | | ADR-0033 §1 | Multi-row VALUES INSERT runs end-to-end | `ins::multi_row_insert_persists_both_rows` | ✅ | | ADR-0033 §1 | INSERT with `(column_name_list)` runs | `ins::single_row_insert_persists_and_counts` | ✅ | | ADR-0033 §1 | INSERT without column list (full table arity) runs | `ins::no_column_list_full_arity_insert_persists` | ✅ | | ADR-0033 §4 | `INSERT … SELECT` runs | `ins::insert_select_copies_rows_and_persists`, `e2e::e2e_insert_select_cross_table_copies_rows_and_persists_both` | ✅ | | ADR-0033 §4 / R4 | `INSERT … WITH … SELECT` row source runs (Amendment-2 carry-through) | `ins::with_prefixed_insert_select_runs_and_persists` | ✅ | | ADR-0033 §1 | UPDATE with WHERE runs | `upd::single_column_update_with_where_persists` | ✅ | | ADR-0033 §1 / §12 | UPDATE without WHERE runs (no `--all-rows` rail) | `upd::update_without_where_runs_across_all_rows` | ✅ | | ADR-0033 §1 | UPDATE with multi-column SET | `upd::multi_column_update_persists` | ✅ | | ADR-0033 §1 | UPDATE with sql_expr in SET (function call, subquery, CASE) | `upd::update_with_sql_expr_in_set`, `e2e::e2e_update_with_subquery_in_set` | ✅ | | ADR-0033 §1 | DELETE with WHERE runs | `del::delete_with_where_persists` | ✅ | | ADR-0033 §1 / §12 | DELETE without WHERE runs (no `--all-rows` rail) | `del::delete_without_where_runs_across_all_rows` | ✅ | | **Dispatch (§2)** | | | | | ADR-0033 §2 / 3a | Guard fires BEFORE Seq commits (R1 invariant on synthetic markers) [a] | `walker::advanced_mode_dsl_input_falls_back_to_dsl` | ✅ | | ADR-0033 §2 | Simple mode + DSL shared word → DSL branch | `parser::simple_dsl_delete_stays_dsl` | ✅ | | ADR-0033 §2 [b] | Simple mode + shared word w/ SQL-only construct → DSL parse error + combined pointer | `parser::simple_shared_word_with_sql_construct_is_a_dsl_parse_error`, `input_render::ambient_hint_combines_dsl_error_with_advanced_sql_pointer`, `app::simple_mode_submit_of_sql_construct_appends_advanced_pointer`; SQL-only *entry* word → `parser::simple_sql_only_entry_word_points_at_advanced_mode` | ✅ | | ADR-0033 §2 | Advanced + structurally-ambiguous (`delete from t where id = 1`) → SQL branch | `parser::advanced_ambiguous_delete_routes_to_sql` | ✅ | | ADR-0033 §2 | Advanced + DSL-only (`--all-rows`) → DSL branch (falls through) | `parser::advanced_dsl_only_delete_falls_back_to_dsl` | ✅ | | ADR-0033 §2 | Same for UPDATE shared entry word (+ `--all-rows` SQL-absorb counter-example) | `parser::advanced_ambiguous_update_routes_to_sql`, `e2e::e2e_update_all_rows_in_advanced_does_not_fall_back_to_dsl` | ✅ | | ADR-0033 §2 | Same for INSERT shared entry word | `parser::advanced_ambiguous_insert_routes_to_sql` | ✅ | | **RETURNING (§5 / §12)** | | | | | ADR-0033 §5 | INSERT … RETURNING * surfaces DataResult | `ins::insert_returning_star_returns_inserted_row`, `e2e::e2e_returning_on_insert_update_delete` | ✅ | | ADR-0033 §5 | UPDATE … RETURNING cols surfaces DataResult | `upd::update_returning_yields_modified_columns`, `e2e::e2e_returning_on_insert_update_delete` | ✅ | | ADR-0033 §5 | DELETE … RETURNING * surfaces DataResult (pre-DELETE row) | `del::delete_returning_yields_predelete_row`, `e2e::e2e_returning_on_insert_update_delete` | ✅ | | ADR-0033 §5 | DELETE … RETURNING preserves cascade summary alongside result set | `del::delete_returning_with_cascade_surfaces_both` | ✅ | | ADR-0033 §12 | Result-column type recovery for each of ten playground types via RETURNING | `e2e::e2e_multirow_insert_all_ten_types_roundtrips_and_returning_recovers_each_type` (all 10), `ins::insert_returning_recovers_multiple_bare_column_types` (5) | ✅ | | ADR-0033 §12 | Computed expression in RETURNING stays typeless | `ins::insert_returning_computed_expression_is_typeless` | ✅ | | ADR-0033 §5 / R3 | Multi-row INSERT … RETURNING: column-origin same for all rows | `ins::multirow_autofill_returning_yields_distinct_generated_ids`, `e2e::e2e_multirow_insert_all_ten_types_…` | ✅ | | **shortid auto-fill (§6)** | | | | | ADR-0033 §6 | Single-row VALUES auto-fills omitted shortid PK | `ins::values_autofills_omitted_shortid_pk` | ✅ | | ADR-0033 §6 | Multi-row VALUES yields DISTINCT shortids per row | `ins::values_multirow_autofills_distinct_shortids` | ✅ | | ADR-0033 §6 | INSERT … SELECT auto-fills shortids per yielded row | `ins::insert_select_autofills_distinct_shortids` | ✅ | | ADR-0033 §6 | Explicit value for shortid column respected (override; warning in §8.2) | `ins::explicit_shortid_value_is_respected` | ✅ | | ADR-0033 §6 | serial + shortid combo: no double-fill collision | `ins::combined_serial_and_shortid_autofill` | ✅ | | **Cascade summary (§7)** | | | | | ADR-0033 §7 | DSL parity: same schema/data, SQL DELETE produces same per-relationship summary | `del::cascade_parity_with_dsl` | ✅ | | ADR-0033 §7 / R2 [c] | WHERE-with-subquery: cascade correct (count-diff, R2 withdrawn) | `del::r2_where_with_subquery` | ✅ | | ADR-0033 §7 [c] | Cascade detection by count-diff in a transaction (before/after) | `del::cascade_delete_reports_summary_and_repersists_child` | ✅ | | ADR-0033 §7 | Cascade-affected child tables re-persisted | `del::cascade_delete_reports_summary_and_repersists_child`, `e2e::e2e_delete_with_cascade_reports_summary_and_repersists_children` | ✅ | | ADR-0033 §7 | Cascade-summary formatter shared (one render path, DSL + SQL) | `del::cascade_parity_with_dsl` | ✅ | | **Diagnostics (§8)** | | | | | ADR-0033 §8.1 | `insert_arity_mismatch` positive (single-row) | `walker::insert_arity_mismatch_single_row_fires` | ✅ | | ADR-0033 §8.1 | `insert_arity_mismatch` negative (matched arity) | `walker::insert_arity_match_is_silent` | ✅ | | ADR-0033 §8.1 | `insert_arity_mismatch` per-row (multi-row VALUES) | `walker::insert_arity_mismatch_emits_per_row` | ✅ | | ADR-0033 §8.1 | `insert_arity_mismatch` on INSERT…SELECT (projection arity) | `walker::insert_select_arity_mismatch_fires` | ✅ | | ADR-0033 §8.2 | `auto_column_overridden` positive (serial) | `walker::auto_column_overridden_fires_on_explicit_serial` | ✅ | | ADR-0033 §8.2 | `auto_column_overridden` positive (shortid) | `walker::auto_column_overridden_fires_on_explicit_shortid` | ✅ | | ADR-0033 §8.2 | `auto_column_overridden` negative (omitted) | `walker::auto_column_overridden_silent_when_omitted` | ✅ | | ADR-0033 §8.3 | `not_null_missing` positive | `walker::not_null_missing_fires_when_required_column_omitted` | ✅ | | ADR-0033 §8.3 | `not_null_missing` negative | `walker::not_null_missing_silent_when_included` | ✅ | | ADR-0033 §8.3 | `not_null_missing` does NOT fire for NOT-NULL-with-default | `walker::not_null_missing_silent_when_column_has_default` | ✅ | | **UPSERT (§9)** | | | | | ADR-0033 §9 | ON CONFLICT (col) DO NOTHING runs; no-ops on conflict | `ins::on_conflict_do_nothing_keeps_existing_row`, `e2e::e2e_upsert_round_trip_do_update_then_do_nothing` | ✅ | | ADR-0033 §9 | ON CONFLICT (col) DO UPDATE SET … = excluded.… runs; row updated | `ins::on_conflict_do_update_applies_excluded`, `e2e::e2e_upsert_round_trip_…` | ✅ | | ADR-0033 §9 | ON CONFLICT DO NOTHING (no target spec) accepts any conflict | `ins::on_conflict_do_nothing_without_target` | ✅ | | ADR-0033 §9 | `excluded.col` completes to target table's columns inside DO UPDATE | `completion::excluded_prefix_completes_to_target_columns` | ✅ | | ADR-0033 §9 | `excluded` rejected outside DO UPDATE (scoping) | `walker::excluded_outside_do_update_is_unknown_qualifier`, `walker::excluded_in_values_flagged_even_when_do_update_present` | ✅ | | **Inherited Phase-2 diagnostics on DML slots (§8.4)** | | | | | ADR-0032 §11 | `schema_existence` fires on INSERT VALUES / col-list column | `walker::insert_column_list_unknown_column_is_flagged` | ✅ | | ADR-0032 §11 | `schema_existence` fires on UPDATE SET | `walker::sql_update_unknown_set_column_is_error` | ✅ | | ADR-0032 §11 | `schema_existence` fires on UPDATE / DELETE WHERE | `walker::sql_update_where_unknown_column_is_error`, `walker::sql_delete_where_unknown_column_is_error` | ✅ | | ADR-0032 §11 | `schema_existence` fires on RETURNING projection | `walker::returning_ref_unknown_column_is_flagged`, `walker::returning_ref_in_plain_insert_validated_against_target` | ✅ | | ADR-0032 §11.6 | Predicate warning `eq_null` fires on UPDATE WHERE | `walker::sql_update_eq_null_in_where_warns` | ✅ | | ADR-0032 §11.6 | Predicate warning `like_numeric` fires on DELETE WHERE | `walker::sql_delete_where_like_numeric_warns` | ✅ | | ADR-0032 §11.6 [d] | Predicate warning fires on row-scoped DML sql_expr slots (UPDATE SET CASE, INSERT…SELECT WHERE) — VALUES has no row scope | `walker::sql_update_set_case_predicate_warns`, `walker::insert_select_where_predicate_warns` | ✅ | | **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) | `highlight::sql_dml_keywords_classified` | ✅ | | ADR-0030 §8 | Tab completion works for SQL DML entry keywords | `completion::empty_input_offers_all_command_entry_keywords` | ✅ | | ADR-0030 §8 | Tab completion works for target table after `INSERT INTO ` | `completion::insert_into_offers_table_names_at_target_slot` | ✅ | | ADR-0030 §8 | Tab completion works for column names inside `(column_list)` | `completion::insert_into_open_paren_offers_current_table_columns` | ✅ | | ADR-0030 §8 | Tab completion works for column names in UPDATE SET / WHERE | `completion::update_set_offers_only_current_table_columns`, `completion::update_where_offers_only_current_table_columns` | ✅ | | ADR-0030 §8 [f] | Hint-panel assistance appears at representative DML slots | `input_render::advanced_mode_ambient_offers_dml_slot_candidates` (advanced SQL), `input_render::ambient_hint_at_insert_first_value_shows_int_prose` / `…at_update_set_shows_per_column_prose` / `…at_where_mentions_column_name` (simple DSL prose) | ✅ | | ADR-0030 §8 | `[ERR]`/`[WRN]` validity indicator fires for SQL DML diagnostics | `e2e::e2e_validity_indicator_fires_for_sql_dml_diagnostic`, `walker::input_verdict_eq_null_is_warning`, `walking_skeleton::validity_indicator_renders_err_and_wrn_labels` | ✅ | | ADR-0030 §8 | Per-command parse-error usage fires for SQL DML shared words | `parse_error_pedagogy::insert_partial_renders_insert_usage_template`, `…update_partial_renders_update_usage_template` | ✅ | | **Engine-neutrality + safety** | | | | | ADR-0030 §6 / ADR-0033 §1 | `__rdbms_*` rejection at INSERT target slot | `ins::parse_path_rejects_internal_target_table` | ✅ | | ADR-0030 §6 / ADR-0033 §1 | `__rdbms_*` rejection at UPDATE target slot | `sql_update.rs::internal_target_table_rejected` | ✅ | | ADR-0030 §6 / ADR-0033 §1 | `__rdbms_*` rejection at DELETE target slot | `del::internal_target_table_rejected_at_parse` | ✅ | | ADR-0030 §6 / ADR-0033 §1 | `__rdbms_*` rejection in INSERT…SELECT row source's FROM | `sql_insert.rs::select_row_source_rejects_internal_from_table` | ✅ | | ADR-0030 §7 / ADR-0033 §11 | Engine UNIQUE constraint failure surfaces via friendly layer (engine-neutral) | `friendly::enrich_unique_insert_resolves_table_column_value_and_pinpoint`, `ins::failed_insert_rolls_back_and_does_not_repersist` | ✅ | | ADR-0030 §7 / ADR-0033 §11 | Engine NOT NULL constraint failure surfaces via friendly layer | `friendly::enrich_not_null_resolves_table_and_column` | ✅ | | ADR-0030 §7 / ADR-0033 §11 | Engine FK constraint failure on DELETE-without-cascade surfaces | `del::delete_violating_fk_fails_and_persists_nothing` | ✅ | | **Persistence + history (§10–§11)** | | | | | ADR-0030 §11 | INSERT writes history.log + re-persists target CSV | `ins::insert_appends_literal_line_to_history` | ✅ | | ADR-0030 §11 | UPDATE writes history.log + re-persists target CSV | `upd::update_appends_literal_line_to_history` | ✅ | | ADR-0030 §11 | DELETE writes history.log + re-persists target + cascade-affected CSVs | `del::delete_appends_literal_line_to_history`, `e2e::e2e_delete_with_cascade_…` | ✅ | | ADR-0030 §11 | Every Phase-3 statement form replays from history.log faithfully | `e2e::e2e_replay_phase3_dml_forms_from_a_script` | ✅ | | ADR-0033 §10 | INSERT failure does NOT re-persist CSV | `ins::failed_insert_rolls_back_and_does_not_repersist` | ✅ | | ADR-0006 / ADR-0033 §10 [e] | Auto-snapshot fires for SQL DML the same way it does for DSL DML | — (ADR-0006 unimplemented for BOTH paths; deferred) | N/A | | **OOS rejections (§13)** | | | | | ADR-0033 §13 OOS-1 | `INSERT INTO t DEFAULT VALUES` parse-rejects | `e2e::e2e_out_of_scope_dml_forms_parse_reject` | ✅ | | ADR-0033 §13 OOS-2 | `INSERT OR REPLACE` / `OR IGNORE` parse-rejects | `e2e::e2e_out_of_scope_dml_forms_parse_reject` | ✅ | | ADR-0033 §13 OOS-3 | `UPDATE … FROM other_table` parse-rejects | `e2e::e2e_out_of_scope_dml_forms_parse_reject` | ✅ | | ADR-0033 §13 OOS-4 | `WITH x AS (…) UPDATE …` parse-rejects | `e2e::e2e_out_of_scope_dml_forms_parse_reject` | ✅ | | ADR-0033 §13 OOS-4 | `WITH x AS (…) DELETE …` parse-rejects | `e2e::e2e_out_of_scope_dml_forms_parse_reject` | ✅ | | ADR-0033 §13 OOS-5 | `INDEXED BY` / `NOT INDEXED` table modifiers parse-reject | `e2e::e2e_out_of_scope_dml_forms_parse_reject` | ✅ | | ADR-0033 §13 OOS-6 | Multi-statement batch (`…; …`) parse-rejects (single `;`-tail still parses) | `e2e::e2e_out_of_scope_dml_forms_parse_reject`, `e2e::e2e_single_dml_statement_with_trailing_semicolon_parses` | ✅ | **Test-location legend.** `ins::` = `tests/sql_insert.rs`, `upd::` = `tests/sql_update.rs`, `del::` = `tests/sql_delete.rs`, `e2e::` = `tests/sql_dml_e2e.rs` (new in 3k), `walker::` = `src/dsl/walker/mod.rs::tests`, `highlight::` = `src/dsl/walker/highlight.rs::tests`, `completion::` = `src/completion.rs::tests`, `parser::` = `src/dsl/parser.rs::tests`, `input_render::` = `src/input_render.rs::tests`, `app::` = `src/app.rs::tests`, `friendly::` = `tests/friendly_enrichment.rs`, `parse_error_pedagogy::` = `tests/parse_error_pedagogy.rs`, `walking_skeleton::` = `tests/walking_skeleton.rs`, `sql_update.rs` / `sql_insert.rs` (bare) = the `#[cfg(test)]` module inside `src/dsl/grammar/sql_update.rs` / `sql_insert.rs`. **Footnotes (resolutions surfaced during 3k).** - **[a]** ADR-0033 **Amendment 1** replaced `Node::Guard` with category-grouped, mode-aware dispatch in `walker::walk`; the R1 invariant ("the SQL branch failing must not strand the DSL candidate") is now the Advanced-mode fall-through, proven on the dispatch smoke registry by `advanced_mode_dsl_input_falls_back_to_dsl` and on real DML by `parser::advanced_dsl_only_delete_falls_back_to_dsl`. - **[b]** ADR-0033 **Amendment 3** supersedes the original "bare `ValidationFailed`" expectation for shared words: a shared entry word in Simple mode commits the **DSL candidate** and surfaces the real DSL parse error, with the `advanced_mode.also_valid_sql` pointer combined at the render layer. The bare `advanced_mode.sql_in_simple` hint is reserved for SQL-only *entry words* (`select` / `with`). - **[c]** ADR-0033 **Amendment 2** withdrew R2: the cascade summary is produced by before/after **count-diff in a transaction** (DSL parity), not WHERE-byte pre-count injection. The subquery-in-WHERE case is correct by construction. - **[d]** **Matrix claim corrected (user-confirmed, 3k).** Predicate warnings fire on every DML `sql_expr` slot that carries **row scope** (UPDATE/DELETE WHERE, UPDATE SET incl. CASE, INSERT…SELECT projection & WHERE). Plain `INSERT … VALUES` has no row scope, so a bare-column predicate is inapplicable there — consistent with ADR-0033 §8.4 ("WHERE and CASE expressions"). The original example ("inside INSERT VALUES") was an over-statement of the §8.4 claim. - **[e]** **N/A — deferred (user-confirmed, 3k).** ADR-0006 auto-snapshot / undo is unimplemented for **both** the DSL and SQL DML paths (the U-series, per `CLAUDE.md` "Things deliberately deferred"). The "same way as DSL" parity therefore holds vacuously and Phase 3 introduces no regression. ADR-0033 §10's auto-snapshot consequence is contingent on the deferred ADR-0006 work; this row is N/A, not red, and does not block the phase exit. - **[f]** **Attribution corrected (`/runda` round).** The ambient hint-panel claim sits under the §8 *advanced* surface, but the hint-prose tests run via `ambient_hint` (which defaults to `Mode::Simple` — the DSL value-slot prose). 3k's `/runda` round added `advanced_mode_ambient_offers_dml_slot_candidates` so the row has a genuine **advanced-mode** DML hint-panel attribution (column candidates at an `UPDATE … SET` LHS and inside an `INSERT … (` column list); the simple-mode DSL prose tests remain as the DSL-surface complement. 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.