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 =

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.