diff --git a/docs/handoff/20260520-handoff-29.md b/docs/handoff/20260520-handoff-29.md new file mode 100644 index 0000000..49cc3bc --- /dev/null +++ b/docs/handoff/20260520-handoff-29.md @@ -0,0 +1,228 @@ +# Session handoff — 2026-05-20 (29) + +Twenty-ninth handover. This session **closed ADR-0032 Phase 2** +(see `docs/handoff/20260520-handoff-28.md`) and **drafted +ADR-0033** as the planning artifact for ADR-0030 Phase 3 (SQL +DML in Advanced mode). The next session starts implementation +at **sub-phase 3a** — see §3 below for the concrete entry point. + +## §1. State at handoff + +**Branch:** `main`. The Phase-2 commits were pushed by the user +mid-session; the Phase-3 planning commits in this session are +local-only and ready to be pushed at the user's convenience. + +**Tests:** **1446 passing, 0 failing, 1 ignored** (unchanged +from handoff-28 — Phase 3 is planning-only this session). + +**Clippy:** clean. + +**This session's commits (Phase 3 planning):** + +``` + + +fa417a4 docs: session handoff 28 — Phase 2 closed; Phase 3 is next +``` + +(All Phase-2 implementation commits are already pushed; see +handoff-28 §1 for the full list.) + +## §2. Phase 3 design — settled + +Ten design questions were discussed and settled in this session. +The full rationale lives in **`docs/adr/0033-sql-dml-grammar.md`**; +this is the index: + +| Q | Topic | Decision | +|-----|----------------------------------------------------------------|---------------------------------------------------------------------------------------------------------| +| Q1 | `INSERT … SELECT` as a row source | **In scope** — recurses through `SQL_SELECT_COMPOUND` | +| Q2 | `INSERT INTO t DEFAULT VALUES` | **OOS** — the planned seed feature covers this case | +| Q3 | `RETURNING` clause on INSERT / UPDATE / DELETE | **In scope** on all three | +| Q4 | UPSERT (`ON CONFLICT … DO NOTHING / DO UPDATE`) | **Full UPSERT in scope** — including `excluded` pseudo-table | +| Q5 | `INSERT OR REPLACE / IGNORE / ABORT / FAIL / ROLLBACK` | **OOS** — SQLite-specific | +| Q6 | `UPDATE table FROM other_table` (multi-table) | **OOS** — SQLite/PostgreSQL extension, confusing pedagogically | +| Q7 | `shortid` auto-fill on SQL INSERT | **Worker post-fill** (parity with DSL `do_insert`) | +| Q8a | INSERT column-count arity mismatch | **Walker ERROR diagnostic** — `diagnostic.insert_arity_mismatch` | +| Q8b | Auto-gen column override warning | **Walker WARNING** — `diagnostic.auto_column_overridden` | +| Q8c | NOT-NULL-missing-on-INSERT | **Walker WARNING (advisory)** — `diagnostic.not_null_missing` | +| Q9 | Cascade summary on SQL DELETE | **Parity with DSL** — shared formatter, WHERE-clause byte-range injection into pre-count subqueries | +| Q10 | `Command` shape for DML execution | **Three typed variants** — `SqlInsert` / `SqlUpdate` / `SqlDelete` | + +**Dispatch architecture (the hardest call):** Advanced mode runs +SQL-first with DSL fallback via `Choice(SQL_shape, DSL_shape)` +per shared entry word. The mode-gating mechanism requires a new +walker capability — `Node::Guard(fn)`, a zero-byte-consumption +gating node — which is the very first sub-phase's work (§3a in +the ADR). + +## §3. Where the next session starts + +**Sub-phase 3a — `Node::Guard` + mode-gated Choice mechanism.** +This is the foundation; if it doesn't work cleanly, the +mitigation paths (alternative dispatch mechanisms) are evaluated +before any DML grammar lands. ADR-0033 §"Open implementation +risks" R1 budgets for this explicitly. + +**Concrete first-day tasks:** + +1. **Read** the following, in order: + - This handoff (you're already doing it). + - `docs/handoff/20260520-handoff-28.md` (Phase 2 closing + state + the four non-blocking observations). + - `docs/adr/0030-advanced-mode-sql-surface.md` (the source + roadmap). + - `docs/adr/0033-sql-dml-grammar.md` (the Phase-3 grammar + ADR — the controlling document for all DML decisions). + - `CLAUDE.md` (project conventions; DA discipline reminder + in particular). + +2. **Run baseline:** + ``` + cargo test # expect 1446 / 0 / 1 + cargo clippy --all-targets -- -D warnings # clean + ``` + +3. **Write the Phase-3 implementation plan doc** at + `docs/plans/-adr-0033-phase-3.md`, modelled on + `docs/plans/20260520-adr-0032-phase-2.md`. ADR-0033's + per-sub-phase exit gates are detailed enough that the + plan doc can be focused on the **cross-cut verification + matrix scaffold** (the rows to fill in during 3k). Suggested + matrix sources: + - ADR-0033 §1 (statement shapes) — one row per shape per + row-source variant (VALUES single, VALUES multi-row, + INSERT…SELECT, …). + - ADR-0033 §2 (dispatch) — one row per mode × DSL/SQL + branch. + - ADR-0033 §5 (RETURNING) — one row per statement kind. + - ADR-0033 §6 (shortid auto-fill) — single-row, multi-row, + INSERT…SELECT, override case. + - ADR-0033 §7 (cascade summary) — DSL parity row + WHERE + subquery case (R2 mitigation). + - ADR-0033 §8 (diagnostics) — positive + negative per key. + - ADR-0033 §9 (UPSERT) — DO NOTHING + DO UPDATE rows. + +4. **Implement sub-phase 3a** per ADR-0033 §Implementation + notes: + - Add `Node::Guard(fn(&WalkContext) -> Result<(), + ValidationError>)` to `src/dsl/grammar/mod.rs::Node`. + - Add `walk_guard` to `src/dsl/walker/driver.rs`. + - Add the `reject_sql_in_simple_mode` guard function. + - Build the smoke-test grammar (a single experimental + `CommandNode` with a `Choice(SQL_branch_gated, + DSL_branch)` shape that uses two distinguishable tokens). + - Verify the four exit-gate test cases in ADR-0033 §3a. + +5. **DA gate (sub-phase 3a):** apply the written DA gate from + ADR-0033 §3a verbatim. The "Does the Guard actually fire + BEFORE the Seq commits?" question is the make-or-break + test — write that test explicitly. If it fails, R1's + mitigation reopens sub-phase 3a with mechanism option (b) + or (c). + +6. **Commit** with a message that explicitly names the + mechanism choice (a/b/c) actually used. If you had to fall + back to option (b) or (c), update ADR-0033 §2's "Default + choice" sentence in the same commit. + +## §4. Process pins from this session + +These are NOT new project rules — they're lessons reinforced +this session that the next session should keep in mind. + +### 4.1. DA discipline — first pass is rubber-stamp risk + +Phase 2's verification report shipped with a rubber-stamp PASS +on the first DA pass. The user had to challenge it to get +specific critiques. This session's ADR-0033 DA review took two +passes: the first pass produced seven specific critiques, all +addressed in the body; the second pass surfaced an eighth real +gap (the `Node::Guard` capability didn't exist) and added it. + +**Pin for sub-phase exits:** DA review lists specific critiques +first, concludes after. A clean PASS without critiques is a +process failure even if the work is sound — the DA hat has to +PROVE engagement. + +### 4.2. Defer-trap reflexes + +Phase 2 surfaced this pattern repeatedly: when scope expanded, +the implementer's reflex was to label items "out of scope" or +"non-blocking" without user approval, per CLAUDE.md's explicit +prohibition. The user corrected this several times. + +**Pin for Phase 3:** every "out of scope" classification must +either (a) be already declared OOS by an ADR, or (b) be +escalated to the user. The default is "in scope, in this +sub-phase, now." + +### 4.3. Tests-first on gap closure + +Phase 2's 2g rework added 13 tests AFTER the gaps were +discovered, not before. The work was sound but the ordering was +backwards. + +**Pin for Phase 3:** when a sub-phase's exit gate exposes a gap +in an earlier sub-phase, write the failing test FIRST, then fix. +This is the bug-fix discipline from CLAUDE.md applied to +gap-closure. + +### 4.4. Matrix attribution must be SQL tests, not DSL tests + +Phase 2's matrix row "validity indicator fires for SQL" was +attributed to a DSL test (the user-visible bug it left was +fixed in the rework). Generalisable lesson: + +**Pin for Phase 3:** when filling the matrix in sub-phase 3k, +each row pointing at a SQL surface must point at a test whose +INPUT is SQL syntax. Cross-check at attribution time, not at +DA-review time. + +## §5. Pinned non-blocking observations from Phase 2 + +These items are from `docs/handoff/20260520-handoff-28.md` §3. +**They're not blockers for Phase 3** but a Phase-3 implementer +may decide to address one or more if they're nearby: + +- §3.1 `translate_generic`'s "group by" pattern overbreadth in + `src/friendly/translate.rs`. +- §3.2 Look-ahead probe cost in `src/completion.rs` — second + walk per Tab press when leading scope is empty; not + benchmarked. +- §3.3 Process: tests-after-code on matrix coverage. +- §3.4 Process: matrix attribution wasn't verified row-by-row. + +## §6. Open work — unchanged from handoff-28 + +Same list as handoff-28 §6. Phase 3 (this session's planning +target) is one of the items; the others remain: + +- Project storage Iter 5/6. +- Snapshot / replay / undo (U-series). +- m:n convenience (C4); modify relationship (C3a); rename + table (C1); column drops/renames/type changes (B2/C2). +- Friendly-error sweep (H1); strong syntax-help in parse + errors (H1a). +- CI (TT5). +- Session log + Markdown export (V4). +- Readline shortcuts (I1b), multi-line input (I1), tab + completion polish (I3), syntax highlighting (I4). +- ER diagram export (V3). +- Tutorial / lesson system. + +## §7. How to take over + +1. **Read this file in full.** Then handoff-28, then + ADR-0033, then `CLAUDE.md`. +2. **`cargo test`** — expect **1446 passing, 0 failed, 1 + ignored**. +3. **`cargo clippy --all-targets -- -D warnings`** — clean. +4. **First task: write the Phase-3 plan doc** (per §3 step 3). + Don't skip this — the cross-cut matrix is the scaffolding + that lets sub-phases 3b–3k know what they're aiming at. +5. **Second task: sub-phase 3a** (`Node::Guard` + mode-gated + Choice mechanism). Per §3 steps 4–6. +6. **Escalate** any ambiguity not already settled in ADR-0033. + Don't decide silently; don't classify items "out of scope" + without checking.