diff --git a/docs/handoff/20260526-handoff-43.md b/docs/handoff/20260526-handoff-43.md new file mode 100644 index 0000000..5c8b9f9 --- /dev/null +++ b/docs/handoff/20260526-handoff-43.md @@ -0,0 +1,175 @@ +# Session handoff — 2026-05-26 (43) + +Forty-third handover. This session started from "let's handle the F1–F3 +follow-ups" (handoff-42 §3) and ended up going much deeper: it shipped the +F1–F3 work and the carried message-gap follow-ups, then — driven by the +user's questions — opened, drafted, `/runda`'d, **reframed**, and began +**implementing a new ADR (0036)** about value handling in advanced-mode +SQL DML. **ADR-0036 Phase 1 is implemented.** Phases 2–3 and several +tracked items remain — hence the handoff. + +## §1. State at handoff + +**Branch:** `main`. **HEAD `1d5534b`.** **Tests: 1930 passing, 0 failing, +0 skipped, 1 ignored** (the `friendly/mod.rs` ` ```ignore ` doctest). +**Clippy:** clean (`cargo clippy --all-targets -- -D warnings`). + +**Unpushed commits from this session (newest first; all normal — push is +the user's step, never prompt about it):** + +``` +1d5534b feat: ADR-0036 Phase 1 — validate advanced-mode INSERT literals + show the value +dc9a475 docs: ADR-0036 revised to surgical "validate-and-retain"; +X4/X5 open questions +3e3a2fb docs: ADR-0036 (Proposed) — bind literal DML values, verbatim text only for expressions/queries +f8a91f4 feat: ADR-0035 Amendment 1 follow-up — enrich replay errors + close message gaps +cb8ff8a feat: ADR-0035 Amendment 1 — drop composite UNIQUE; friendlier drop-column + generic-error wording +``` + +(Plus the handoff-42 unpushed set below them. All unpushed; normal.) + +## §2. What shipped + +### ADR-0035 Amendment 1 + follow-ups (commits `cb8ff8a`, `f8a91f4`) + +The whole-Phase-4 `/runda` follow-ups F1–F3, then the broader F2 + gaps +B/C/D: + +- **F3** — drop a composite UNIQUE via a **derived, engine-neutral name** + `unique_` (recomputed live, nothing persisted), reusing the + existing `DROP CONSTRAINT ` grammar. Ambiguous names refused. One + undo step. `describe` annotates each UNIQUE (`unique_a_b: unique (a, b)`). + ADR-0035 **Amendment 1**. +- **F1** — dropping a column a composite UNIQUE covers is refused up-front + with the derived name + the drop command. +- **F2** — generic-error `{table}` leak fixed (`error.generic.hint_no_table`); + then **F2-broad**: replay failures now enrich via `enrich_dsl_failure` + + the shared `App::translate_context_for`, and the `ctx_*` fallbacks became + neutral prose (no raw `{name}` anywhere). +- **Gap C** — SQL `ALTER … ADD FOREIGN KEY` on a missing child column no + longer suggests the DSL-only `--create-fk`. +- **Gap B** — dropping a single-column-UNIQUE column refuses pointing at + `drop constraint unique from T.col`. +- **Gap D** — the 4e CHECK-guard + 4f change-type-FK refusals reworded to + explain *why* (wording only; `static_refusal` left as-is). + +### ADR-0036 — value validation for advanced-mode DML (commits `3e3a2fb`, `dc9a475`, `1d5534b`) + +This is the session's main arc, and the next session should read +**`docs/adr/0036-typed-dml-values-vs-verbatim.md`** in full. The short +version: + +- **The finding.** Advanced-mode SQL `INSERT`/`UPDATE` hands literal data + values to the engine as text, so it gets **none** of the DSL's value + feedback — no type validation (a malformed `date` `2025/01/15` is + silently stored), and the offending value can't appear in errors. The + DSL never did this: `do_insert` parses each value, validates it + (`bind_for_column` → `validate_date`/`shortid::validate`), and **binds** + it as a parameter. Proven by a characterization test. +- **The arc.** First instinct was to *consolidate* (bind literals via the + DSL path, even emit `Command::Insert` from the advanced surface). The + user pushed back on fusing the modes, and a concrete **auto-fill + difference** (simple-mode fills an omitted non-PK `serial` with + `MAX+1`; advanced-mode does not) confirmed the modes are **not** + identical even for single-row literals. So the ADR was **reframed** + (commit `dc9a475`) to the **surgical** principle: + + > **Validate the literal values (sharing the per-type validators) and + > retain them for error reporting; execute verbatim, and keep auto-fill, + > execution, and command identity mode-specific.** Share a *mechanic*, + > not a *command* (`requirements.md` X5). It **augments** (does not + > supersede) ADR-0030 §4 / ADR-0033 §10; ADR-0033 **Amendment 3 stands**. + +- **Phase 1 (implemented, `1d5534b`).** `build_sql_insert` captures each + `VALUES` position from the matched path (`Some(Value)` for a literal incl. + signed numbers; `None` for an expression) onto a new `literal_rows` field + on `Command::SqlInsert` — **no grammar change, no reparse**. + `do_sql_insert` validates those literals against their column types + (reusing `impl_value_for`) **before** the still-verbatim insert. + `user_value_for_column` reads `literal_rows` so a constraint error shows + the real value. Runtime uses `run_sql_insert_with_literals`; the 6-arg + `run_sql_insert` is the no-capture raw entry (kept so worker-level tests + didn't churn). + +## §3. What's next (ADR-0036 Phases 2–3 — the reason for the handoff) + +Read `docs/adr/0036-…md` §5 (phasing) and §6 (non-goals) first. + +- **Phase 2 — `UPDATE … SET` literal validation.** Same capture-at-parse + technique on the SET assignment list (`SqlUpdate` currently carries no + captured literals — it would gain a `literal_rows`-equivalent). Validate + `SET col = `; skip `SET col = ` and `WHERE` (deliberately + text). Mirror Phase 1's structure. +- **Phase 3 — completion hinting / highlighting.** The **only** part that + needs a grammar change: `Choice(typed-literal-slot, sql_expr)` at each + value position, reusing the DSL's live `column_value_list` / + `TypedValueSlot`s (`src/dsl/grammar/data.rs:141`/`189`/`269`, + `shared.rs`). When it lands it supersedes only Phase 1's literal + *detection* (the validation/enrichment on top is permanent — a + deliberate, documented small throwaway). + +## §4. Tracked follow-ups (in `requirements.md` Cross-cutting) + +- **X4 — serial auto-fill difference (possible bug, investigate).** + Simple-mode `do_insert` auto-fills an omitted non-PK `serial` with + `MAX(col)+1`; advanced-mode (`plan_shortid_autofill`) only fills + `shortid`, never non-PK `serial`. Likely unintended. Decide whether + advanced mode should match (probably yes) and align ADR-0018. **Untouched + by ADR-0036** (which is surgical). +- **X5 — framework cohesion / restructuring (strategic).** The + grammar/execution framework (lexer → walker `Node`s → grammar tree → + `Command`s → executors) grew organically and was never specified as a + whole; commands are coupled to execution, which tempts command-reuse to + get execution-reuse. Desired: **unique commands per unique case; share + execution mechanics as helpers, never by fusing identity.** A + *descriptive* ADR (map what exists + the reuse-boundary rule) is likely + the cheaper first step before any restructure. Not scheduled. + +### Minor / deferred from ADR-0036 Phase 1 + +- **Natural-order error-value *display* deferred.** Validation works for a + no-column-list insert (maps positions to schema columns), but + `user_value_for_column` only resolves the offending value for the + **explicit column-list** case; natural-order falls back to the neutral + "that value". Small follow-up (the `_with_schema` enricher could resolve + it). + +## §5. Carried follow-ups (still open from handoff-42 §3/§4) + +- **F2-narrow vs broad:** done (broad shipped). +- Composite-UNIQUE F3 message wording — the `describe` shows + `unique_a_b: unique (a, b)` (lowercased to match sibling lines); the user + saw `UNIQUE` in the preview but accepted the rendered form. If they want + uppercase or a bracket form, it's a one-line tweak in `output_render.rs`. +- The handoff-39 §8 long-tail items (app-lifecycle runtime-failure + journalling; M4 execution-time mode side-channel; `blob` value literal — + which ADR-0036 notes belongs in the literal layer; CI/TT5; DSL→SQL + teaching echo, ADR-0030 Phase 5) remain. + +## §6. Process pins (unchanged, and reinforced this session) + +- **Confirm every commit** (propose message, wait). No AI attribution. + **Push is the user's step.** Unpushed commits are normal. +- **`/runda` at planning exit earns its keep.** This session's `/runda` on + ADR-0036 caught a misattributed citation, an overstated matrix, and three + unsettled boundary forks — *before* any code. +- **Escalate design forks; don't consolidate by reflex.** The ADR-0036 arc + is the cautionary tale: the clean-looking "emit `Command::Insert`" idea + would have silently changed auto-fill semantics. The user's "keep the + modes apart" instinct was right; the resolution was to share *one + mechanic* (validators), not the command. Default to a distinct command + per case (X5). +- **Keep docs lockstep** — ADR + README index + forward-notes + + `requirements.md` in the same change. + +## §7. How to take over + +1. **Read, in order:** this file → `CLAUDE.md` → + `docs/adr/0036-typed-dml-values-vs-verbatim.md` (the full arc + the + surgical principle + phasing) → `docs/requirements.md` X4/X5. +2. **Baseline:** `cargo test` (1930 / 0 / 0 / 1 ignored) + `cargo clippy + --all-targets -- -D warnings` (clean). +3. Pick up **ADR-0036 Phase 2** (`UPDATE … SET` literal validation, + mirroring Phase 1), or take a tracked item (X4 the most concrete bug), + with the user. Phase 1's shape (`src/dsl/grammar/data.rs` + `capture_literal_rows` + `do_sql_insert` validation + `runtime.rs` + `user_value_for_column`) is the template to follow.