From 4928a4c4fd367f44da17b5ad6eab9a089e1a70dd Mon Sep 17 00:00:00 2001 From: "claude@clouddev1" Date: Wed, 27 May 2026 10:58:30 +0000 Subject: [PATCH] =?UTF-8?q?docs:=20session=20handoff=2044=20=E2=80=94=20AD?= =?UTF-8?q?R-0036=20complete=20(Phases=202/3a/3b=20+=20carryover)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/handoff/20260527-handoff-44.md | 216 ++++++++++++++++++++++++++++ 1 file changed, 216 insertions(+) create mode 100644 docs/handoff/20260527-handoff-44.md diff --git a/docs/handoff/20260527-handoff-44.md b/docs/handoff/20260527-handoff-44.md new file mode 100644 index 0000000..a948986 --- /dev/null +++ b/docs/handoff/20260527-handoff-44.md @@ -0,0 +1,216 @@ +# Session handoff — 2026-05-27 (44) + +Forty-fourth handover. This session **completed ADR-0036** — value handling +for advanced-mode SQL DML — by shipping Phase 2, Phase 3a, and Phase 3b +(picking up from handoff-43, which had landed Phase 1), then closing the +one Phase-1 carryover gap. ADR-0036 is now fully implemented; the next +session starts from a clean slate on it. + +## §1. State at handoff + +**Branch:** `main`. **HEAD `d987171`.** **Tests: 1948 passing, 0 failing, +0 skipped, 1 ignored** (the `friendly/mod.rs` ` ```ignore ` doctest). +**Clippy:** clean (`cargo clippy --all-targets -- -D warnings`). + +This session's commits (newest first): + +``` +d987171 fix: ADR-0036 — name the offending value for natural-order SQL INSERTs +8906661 feat: ADR-0036 Phase 3b — live typed-slot hints + highlighting for INSERT VALUES +49ea03b feat: ADR-0036 Phase 3a — live typed-slot hints + highlighting for SQL SET values +8c3b13b feat: ADR-0036 Phase 2 — validate advanced-mode UPDATE SET literals + retain the value +``` + +## §2. What shipped — ADR-0036 complete + +Read **`docs/adr/0036-typed-dml-values-vs-verbatim.md`** in full, and +especially **Amendment 1**, which is now the authoritative record of the +Phase 3 mechanism (it supersedes the original §5 sketch — see §3 below). + +The four ADR-0036 phases, end to end: + +- **Phase 1 (handoff-43) — `INSERT … VALUES` literal validation.** + Capture each literal value position at parse onto + `Command::SqlInsert.literal_rows`; `do_sql_insert` validates them + against column types (shared `impl_value_for`) before the verbatim + insert; the error enricher names the offending value. No grammar + change. +- **Phase 2 (`8c3b13b`) — `UPDATE … SET` literal validation.** The same + capture-at-parse technique on the SET assignment list: + `capture_set_literals` (`src/dsl/grammar/data.rs`) classifies each + top-level `SET col = ` into `(col, Some(Value))` (a bare literal, + incl. a signed number) or `(col, None)` (an expression), using paren + depth so a comma in a function call or a `where` in a subquery isn't + a boundary, and excluding the trailing top-level `WHERE`. + `Command::SqlUpdate` gained `set_literals`; `do_sql_update` validates; + `user_value_for_column` reads them. `WHERE` is deliberately not + validated. +- **Phase 3a (`49ea03b`) — live typed-slot hints + highlighting for + `SET col = `.** The `SET` column ident now sets + `writes_column: true` (so `current_column` / `pending_value_column` + are populated per assignment); the RHS became the shared + `shared::SET_VALUE` slot — a **boundary-aware lookahead** that routes a + *lone literal* to the column-typed slot (live hint + numeric-shape + highlight, shared with the DSL) and any expression to `sql_expr`. + Covers `sql_update`'s assignment list and `sql_insert`'s + `ON CONFLICT … DO UPDATE SET`. +- **Phase 3b (`8906661`) — live per-position typed slots for + `INSERT … VALUES (…)`** (single/multi-row, Form A and Form B). The + hard half — see §3. + +### Phase-1 carryover closed (`d987171`) + +A no-column-list (natural-order / Form B) SQL `INSERT` that hit a +UNIQUE/CHECK violation used to degrade to the neutral "that value" +because `user_value_for_column` only resolved the offending value for the +explicit-column-list form. `user_value_for_column_with_schema` +(`src/runtime.rs`) now maps each `VALUES` position to the schema's columns +in **declaration order — all columns** (advanced-mode Form B auto-fills +nothing, so every column has a value), single-row only (multi-row stays +ambiguous → neutral). This was the only tracked ADR-0036 follow-up; it is +done. + +## §3. Phase 3 design — the load-bearing details + +Two design forks were escalated to and decided by the user; both are +recorded in **ADR-0036 Amendment 1**. Future work on the grammar must +respect them. + +### The mechanism is a boundary-aware lookahead, NOT a naive `Choice` + +ADR-0036 §5 originally sketched Phase 3 as `Choice(typed-literal-slot, +sql_expr)` at each value position. **That is wrong and would regress valid +SQL.** The walker's `Node::Choice` is first-match-wins with no +cross-branch backtrack, so a typed slot would greedily match the leading +`1` of `1 + 2` and commit, leaving `+ 2` dangling — breaking, e.g., the +existing `values (1, 1 + 2)` test. The correction (Amendment 1): a +**lookahead** peeks the whole value position and routes a literal to the +typed slot *only when the literal fills the position* (up to the next +`,` / `)` / `;` / `where` / `returning` / end); everything else goes to +`sql_expr`. `shared::SET_VALUE` (`set_value_node` + `set_rhs_is_lone_literal`) +is that primitive; it's reused by both 3a and 3b. Empty positions route +to the typed slot too, so the hint shows from the moment the cursor lands. + +### Phase 3b's new `Node::SetColumn` primitive + arity reconciliation + +`INSERT … VALUES` positions are **positional** (no per-position column +ident) and **multi-row**, so the user approved adding a grammar primitive +(the "full parity" option): + +- **`Node::SetColumn(&'static TableColumn)`** (`src/dsl/grammar/mod.rs`, + walker arm in `src/dsl/walker/driver.rs`) — a **zero-width** node that + sets `current_column` + `pending_value_column`, like an + `Ident { writes_column: true }` but without consuming input. One enum + variant, one walker arm (only one exhaustive `Node` match site exists). +- The factory **`sql_insert::sql_value_list`** emits `SetColumn(colᵢ)` + then `SET_VALUE` per position. Column mapping mirrors `do_sql_insert`: + **Form A → listed columns in user order; Form B → ALL columns in + declaration order.** +- **Auto-fill clarification (corrects a loose statement made while + scoping):** advanced mode **does** auto-fill an *omitted* `shortid` in + **Form A** (`plan_shortid_autofill`) — that column has no `VALUES` + position and is correctly absent from the mapping; it does **not** + auto-fill `serial` (the **X4** gap); and **Form B auto-fills nothing** + (the function returns early on an empty column list). +- **Arity reconciliation (the subtle part).** A fixed-length typed `Seq` + would *reject* a wrong-arity tuple, suppressing the friendly per-tuple + `insert_arity_mismatch` diagnostic (ADR-0033 §8.1), which is a + post-walk pass over the matched path and so needs the tuple to be + accepted. So the tuple value list is an **arity-gating lookahead** + (`tuple_value_list` + `count_tuple_values`): a *closed* tuple uses the + typed `Seq` only on an exact value/column match; an *open* (mid-typing) + tuple uses it while `count <= columns` (so the hint shows from `(` + onward); every other case falls back to the type-blind + `Repeated(sql_expr)`, leaving §8.1 to fire unchanged. Correct-arity + tuples (the common case) get full live typed feedback — including a + wrong-*kind* lone literal like `('text')` into an `int` column — while + wrong-arity tuples keep the friendly arity message. + +**Known limitation (all phases, matches the DSL):** `date` / `shortid` / +`datetime` *format* is still not validated at parse — those slots accept +any quoted string; format is checked at bind/execution time. The live +highlight catches *numeric-shape* mismatches (`int`/`decimal`/`bool`); the +column-type *hint* shows for every type. + +## §4. Where the code lives (ADR-0036 surface) + +- **Parse-time capture / typed slots:** `src/dsl/grammar/data.rs` + (`capture_literal_rows`, `capture_set_literals`, `build_sql_insert`, + `build_sql_update`); `src/dsl/grammar/shared.rs` (`SET_VALUE`, + `set_value_node`, `set_rhs_is_lone_literal`, `next_is_set_boundary`, + the typed `*_SLOT`s, `current_column_value`); + `src/dsl/grammar/sql_insert.rs` (`sql_value_list`, `tuple_value_list`, + `count_tuple_values`, `target_value_columns`, `fallback_value_list`, + `VALUE_TUPLE`); `src/dsl/grammar/sql_update.rs` (`ASSIGN_COLUMN`, + `SET_VALUE` RHS). +- **The primitive:** `Node::SetColumn` in `src/dsl/grammar/mod.rs`; its + arm in `src/dsl/walker/driver.rs`. +- **Worker validation:** `src/db.rs` `do_sql_insert` / `do_sql_update` + (validate the captured literals via `impl_value_for`). +- **Error enrichment:** `src/runtime.rs` `user_value_for_column` / + `user_value_for_column_with_schema`. +- **Tests:** `tests/sql_update.rs` (Phase 2 + 3a, incl. the advanced-mode + typing-surface helpers `schema_cache` + `ambient_hint_in_mode` / + `classify_input_with_schema_in_mode` against `Mode::Advanced`); + `tests/sql_insert.rs` (Phase 3b, `vschema` + `prose_at`); + `tests/friendly_enrichment.rs` (offending-value enrichment for + SqlUpdate + natural-order SqlInsert). + +## §5. Open / tracked work (none from ADR-0036) + +- **X4 — `serial` non-PK auto-fill difference (possible bug, the most + concrete next item).** `requirements.md` Cross-cutting. Simple-mode + `do_insert` auto-fills an omitted non-PK `serial` with `MAX(col)+1`; + advanced-mode (`plan_shortid_autofill`) auto-fills only `shortid`, + never non-PK `serial`. Likely unintended. Decide whether advanced mode + should match (probably yes) and align ADR-0018. Untouched by ADR-0036 + (surgical by design). +- **X5 — framework cohesion / restructuring (strategic).** + `requirements.md` Cross-cutting. The grammar/execution framework grew + organically; a *descriptive* ADR (map what exists + the + share-a-mechanic-not-a-command reuse-boundary rule) is the likely + cheaper first step before any restructure. ADR-0036 is a worked example + of the rule in practice. Not scheduled. +- **Long-tail (carried from handoff-39 §8 / handoff-43 §5):** + app-lifecycle runtime-failure journalling; M4 execution-time mode + side-channel; the `blob` value literal (belongs in the literal layer); + CI/TT5 (test infra exists, no workflow yet); the DSL→SQL teaching echo + (ADR-0030 Phase 5); H1 full SQL→English error translation; + H1a syntax-help in parse errors; tutorial/lesson system (needs its own + ADR); V3/V4 (ER diagram export, session log + Markdown export); the I- + series input UX (readline shortcuts, multi-line, tab completion, syntax + highlighting). + +## §6. Process pins (reinforced this session) + +- **Confirm every commit** (propose the message, wait). No AI attribution. +- **Escalate grammar-touching / architectural forks; don't decide for the + user.** This session escalated two Phase-3 forks (the lookahead + mechanism correction; the 3a/3b split and the 3b full-parity-vs- + pragmatic choice) — both materially changed the work and were the + user's call. The arity-diagnostic conflict surfaced *during* 3b + implementation (an exploration gap) and was reconciled within the + approved option without regressing §8.1 — flagged to the user rather + than absorbed silently. +- **`/runda`-style verification at planning exit earns its keep**, and so + does re-checking assumptions against the code: the "advanced mode + doesn't auto-fill serial/shortid" claim was wrong for `shortid`, caught + by the user and confirmed against `plan_shortid_autofill`. +- **Keep docs lockstep** — ADR body + Amendment + README index move with + the code in the same change. +- **ADRs aren't re-litigated casually** — when a decision needs to change + (the Phase 3 mechanism), write an Amendment. Amendment 1 is the live + Phase 3 record. + +## §7. How to take over + +1. **Read, in order:** this file → `CLAUDE.md` → + `docs/adr/0036-typed-dml-values-vs-verbatim.md` (esp. **Amendment 1**) + → `docs/requirements.md` X4/X5. +2. **Baseline:** `cargo test` (1948 / 0 / 0 / 1 ignored) + + `cargo clippy --all-targets -- -D warnings` (clean). +3. Pick up **X4** (the most concrete tracked bug) or a long-tail item + with the user. For X4, the relevant code is `db.rs` `do_insert` + (simple-mode `MAX+1` serial fill) vs `plan_shortid_autofill` + (advanced-mode shortid-only fill), and ADR-0018 is the decision to + align.