rdbms-playground/docs/handoff/20260522-handoff-31.md

# Session handoff — 2026-05-22 (31)

Thirty-first handover. This session shipped, in order:

1. **Finished the advanced-mode completion interlude** left open by
   handoff 30 — the **F5** core `walk_seq` fix, plus a follow-up
   `walk_repeated` fix and a candidate-ordering reversal that grew
   out of manual testing.
2. **Advanced ADR-0033 Phase 3 through sub-phases 3c, 3d, 3e** —
   SQL `INSERT … SELECT`, `shortid` auto-fill, and SQL `UPDATE`.

The next session should **start sub-phase 3f** (`DELETE` +
cascade summary) — the largest remaining DML sub-phase — which is
why we're handing off first. See §4 for a key 3f heads-up.

## §1. State at handoff

**Branch:** `main`. **Tests: 1524 passing, 0 failing, 1 ignored.**
**Clippy:** clean (`cargo clippy --all-targets -- -D warnings`).

**Commits this session** (newest first). `origin/main` is at
`6ff9144` (3c), so the **last three are local-only**:

```
53808ed grammar+db: 3e — SQL UPDATE grammar + execution (ADR-0033 §2)   (local)
18d34d0 db: 3d fix — don't let shortid auto-fill mask INSERT arity mismatch  (local)
78ad476 db+grammar: 3d — shortid auto-fill for SQL INSERT (ADR-0033 §6)  (local)
6ff9144 grammar: 3c — INSERT … SELECT row source (ADR-0033 §4)          (origin/main)
7f68a53 walker+completion: surface list trailing-optionals + identifiers-first ordering (ADR-0022 Amendment 2)
43c49f4 walker: F5 — drop preceding-clause keywords from committed-child Incomplete sets
```

(Unpushed commits are a normal working state; pushing is the
user's step — do not prompt about it.)

## §2. THE process lesson (read this first)

This session's clearest signal is about **Devil's Advocate
discipline**, not code. Three times the lead produced a DA
"verdict" that merely restated the exit gate — and three times the
*user* had to prompt before a genuine adversarial pass happened.
Each real pass then found something concrete:

- "What about more than one shortid in a row?" → an **untested**
  multi-shortid path (it worked, but had zero coverage).
- "Did you check with the DA?" → the **arity-masking bug** (3d
  auto-fill silently dropped extra columns / could read out of
  range). Real correctness bug; fixed in `18d34d0`.
- "Sounds like the DA had nothing to add." → the **untested render
  guard** in 3e (`handle_dsl_update_success`).

**Takeaway for the next agent:** do the adversarial work
*proactively* and *in writing*, and make it produce a failing test
or a concrete change — not a paragraph that clears everything.
"The exit-gate tests pass" is not a DA review. Attack the code:
untested branches, edge cases the happy-path tests skip, behaviour
that diverges from the non-SQL path, things that only break when an
input is malformed. A clean PASS with no findings on a non-trivial
sub-phase should itself be suspicious.

## §3. Phase 3 — what shipped (3c / 3d / 3e)

Controlling docs (read both):
- `docs/adr/0033-sql-dml-grammar.md` — the decision (read
  **Amendment 1** for the dispatch mechanism).
- `docs/plans/20260520-adr-0033-phase-3.md` — build order
  (3a–3k), per-sub-phase exit + DA gates.

### 3c — `INSERT … SELECT` (`6ff9144`)
Grammar-only: the INSERT row source became a
`Choice(VALUES_CLAUSE, Subgrammar(&sql_select::SQL_SELECT_COMPOUND))`.
`SQL_SELECT_COMPOUND` is itself a `Choice` that admits a leading
`WITH`, so the R4 (WITH-prefixed) row source parses for free. No
worker change — `do_sql_insert` already executes the validated SQL
and the engine handles insert-from-query.

### 3d — `shortid` auto-fill (`78ad476`) + arity fix (`18d34d0`)
`Command::SqlInsert` gained `listed_columns` and `row_source`,
extracted in `build_sql_insert` from the matched path (the row
source is found by the **first `values`/`select`/`with` Word
token**, which is path-based so a string literal like `'select'`
can't be mistaken for the keyword). `plan_shortid_autofill`
(`db.rs`) implements the **user-chosen Option B**: when the column
list omits a `shortid` column, materialise the row source by
running it as a query, generate a distinct id per row via the
existing `generate_shortid_batch` (deduped against stored values),
and reconstruct a parameterised multi-row INSERT. Uniform for
VALUES and SELECT; handles multiple omitted shortids (one batch per
column). `serial` stays engine-filled via rowid. `history.log`
keeps the original line, never the rewrite (§11).

**Arity guard (`18d34d0`):** the auto-fill path read exactly
`listed_columns.len()` cells per row — a column/value arity
mismatch silently dropped extra columns or read out of range. Now:
if the materialised statement's `column_count() != listed_count`,
skip auto-fill and run verbatim so the engine reports the mismatch
(a friendly pre-flight is 3i).

Minor deviation from the plan: the plan's 3d step said "turn on
`writes_user_listed_column`". We did **not** flip that flag — the
worker only needs the column *names*, collected by role in
`build_sql_insert`. The flag drives completion-narrowing of VALUES
against the listed columns, which nothing needs yet. It remains
`false`; flipping it is a separate completion enhancement, not a
blocker.

### 3e — SQL `UPDATE` (`53808ed`)
New `src/dsl/grammar/sql_update.rs`: `SQL_UPDATE_SHAPE =
<table> SET col = sql_expr (',' …)* [WHERE sql_expr] [';']`. No
`--all-rows` rail (ADR-0030 §12). `Command::SqlUpdate { sql,
target_table }`, `Request::RunSqlUpdate`, `do_sql_update` (execute
verbatim, re-persist target, history). 3e surfaces the
**affected-row count only**; precise rows are RETURNING (3g).

**Two findings that matter for later sub-phases:**

1. **Cross-cut diagnostics are NOT automatically free.** The
   schema-existence + predicate-warning passes (`mod.rs`) build
   their scope from Tables idents whose **role is `"table_name"`**
   (the pre-pass at `schema_existence_diagnostics`). A bespoke role
   (`update_target_table`) left the SET/WHERE columns unchecked
   (`diag_keys` returned `[]`). Fix: the UPDATE target uses the
   shared `"table_name"` role. **3f's DELETE target must do the
   same** to get the predicate diagnostics on its WHERE for free.
2. **Render guard.** A column-less `UpdateResult` would render a
   misleading `(no rows)` band. `handle_dsl_update_success` now
   skips `render_data_table` when `result.data.columns.is_empty()`.
   The DSL UPDATE always has columns, so it's unaffected. Covered
   by app-level tests both ways.

`sql_select::WHERE_CLAUSE` is now `pub(crate)` so the DML
statements reuse the exact predicate clause.

## §4. Sub-phase 3f — the next job (DELETE + cascade)

Per the plan (`docs/plans/20260520-adr-0033-phase-3.md`, "Sub-phase
3f"): new `src/dsl/grammar/sql_delete.rs` (`DELETE FROM table
[WHERE] [';']`), `Command::SqlDelete` / `Request::RunSqlDelete` /
`do_sql_delete`, cascade-summary pre-count (ADR-0033 §7), the
`format_cascade_summary` formatter **shared** with the DSL
`do_delete`, and **multi-table persistence** (target + every
cascade-affected child).

**Heads-up — the WHERE-byte-extraction problem is tractable for
DELETE.** In 3e I worried that a flat token path can't distinguish
a statement-level WHERE from a subquery's WHERE (a `where` token
can appear before *or* after the statement one). That ambiguity
comes from UPDATE's `SET` clause possibly holding a subquery-with-
WHERE *before* the statement WHERE. **DELETE has no SET** — nothing
before the statement WHERE can contain a subquery — so the
statement WHERE is simply the **first `where` Word token after the
target table**, and the predicate text is `source[where_start..]`
(trim trailing `;`). The R2 invariant
(`DELETE … WHERE x IN (SELECT … WHERE …)`) is fine: the nested
subquery WHERE is *inside* the predicate, which is exactly what the
cascade pre-count wants to inject. So mirror the
`build_sql_insert` row_source / `build_sql_update` extraction: find
the first `where` token, capture the clause text into
`Command::SqlDelete`, and inject it into
`SELECT <pk> FROM <target> <where_clause>` for each child
pre-count. When there is no WHERE, the pre-count is unbounded
(all target rows).

Other 3f gotchas from the plan's DA gate: the cascade pre-count
must run **before** the DELETE (the rows being counted are the ones
about to vanish); cascade-affected child CSVs must all be
re-persisted; and the SQL path's per-relationship summary must
match the DSL path's on the same schema/data (shared formatter).

## §5. Established patterns (reuse these in 3f–3i)

- **Dev entry word per sub-phase.** SQL DML is isolated behind
  `sqlinsert` / `sql_update` (and `sql_delete` next) entry words,
  `CommandCategory::Advanced` in `REGISTRY`
  (`src/dsl/grammar/mod.rs`). The `build_sql_*` ast-builder
  reconstructs the real keyword (`insert`/`update`/`delete`) +
  matched tail. **3j removes all dev words** and makes
  `insert`/`update`/`delete` shared DSL/SQL entry words; 3j must
  also de-dup the completion entry-word lists once a word appears
  twice in `REGISTRY` (flagged in ADR-0033 Amendment 1).
- **`table_name` role for any target whose WHERE/SET columns need
  schema diagnostics** (see §3e finding 1).
- **Static-vs-const in grammar files.** A `Node` referenced *by
  value* in a `static [...]` array must be `const` (so it inlines);
  a `Node` referenced via `&NODE` can be `static`. Getting this
  wrong gives "cannot move out of a shared reference" (hit twice
  in 3e).
- **Worker result + render.** SQL DML reuses the DSL
  `CommandOutcome::{Insert,Update,Delete}` and the
  `handle_dsl_*_success` renderers. Any new `Command` variant must
  be added to: `command.rs` `verb()` + `target_table()`, the
  `runtime.rs` dispatch, `app.rs` `build_translate_context`, and
  the `tests/typing_surface/mod.rs` `command_kind_label` match
  (all are non-exhaustive checks that will fail to compile until
  covered — a useful forcing function).

## §6. Escalations settled this session (do not re-litigate)

- **Identifiers-first candidate ordering** (ADR-0022 Amendment 2):
  schema identifiers sort *before* keywords, globally — the user
  explicitly chose this over the handoff-14 keywords-first
  invariant, after seeing that long SQL keyword runs pushed column
  names off the single-row, window-scrolled candidate line. A
  **two-line hint box** is recorded as a deferred follow-up.
- **3d SELECT shortid strategy = Option B** (materialise + dedup +
  reinsert), user-confirmed.
- **`auto_column_overridden` WARNING stays INSERT-only** (the plan
  default). 3e did not extend it to UPDATE; if 3f/3i wants to, the
  plan says escalate.

## §7. Still deferred (tracked, not lost)

- **RETURNING** (3g) — precise DML row output; 3e/3f surface counts
  (+ cascade summary) only until then.
- **UPSERT `ON CONFLICT`** (3h), **diagnostics** `insert_arity_mismatch`
  / `auto_column_overridden` / `not_null_missing` (3i),
  **dispatch wiring / dev-word removal** (3j), **verification
  sweep** (3k).
- **Old-project migration** (from handoff 30 §4) — the pre-
  `check_expr` 3-column `__rdbms_playground_columns` schema; the
  user deferred a migration story to "the end". Targeted fix when
  the migration framework (ADR-0015 Iter 6) lands.
- **Two-line hint box** (ADR-0022 Amendment 2).
- **`writes_user_listed_column`** flag (see §3d) — VALUES-against-
  listed-columns completion narrowing.

## §8. Process pins (unchanged, still binding)

- **Confirm every commit.** Propose the message and wait for the
  go-ahead. No auto-commit at sub-phase gates.
- **Push is the user's step.** Never push; never prompt about it.
- **No AI attribution** in commits (global rule).
- **Test-first.** Reproduce a bug with a FAILING test before
  fixing; for features, exit-gate tests before "done". This is
  also how the real DA findings surfaced this session — write the
  cross-cut/edge test and watch it fail.
- **Core walker changes** (`walk_seq` / `walk_choice` /
  `walk_repeated`) need explicit user OK before coding. (F5 and the
  3d-era `walk_repeated` change both went through this.)
- **Escalate ambiguity; never classify work out of scope without
  user confirmation.**

## §9. How to take over

1. **Read, in order:** this file →
   `docs/adr/0033-sql-dml-grammar.md` (esp. §7 cascade + §2
   UPDATE, already shipped) →
   `docs/plans/20260520-adr-0033-phase-3.md` "Sub-phase 3f" →
   `CLAUDE.md`.
2. **Baseline:**
   ```
   cargo test    # expect 1524 passing / 0 failing / 1 ignored
   cargo clippy --all-targets -- -D warnings   # clean
   ```
3. **Start 3f** per §4. Mirror `sql_update.rs` for the grammar and
   `do_sql_update` for the worker shell; the new work is the
   cascade pre-count + the shared `format_cascade_summary` refactor
   + multi-table persistence. Use the `table_name` role for the
   DELETE target. Capture the WHERE clause text via the first
   `where` token (tractable for DELETE — §4).
4. **Do the DA pass for real** (§2). On a sub-phase this size,
   expect to find at least one untested branch or edge case;
   if you don't, look harder.
5. **Escalate** anything not settled in ADR-0033 / the plan.