claude@clouddev1
2f0af31b3b
F1–F3 + F2-broad + gaps B/C/D shipped; ADR-0036 opened, /runda'd, reframed to the surgical "validate-and-retain, verbatim" principle, and Phase 1 (advanced-mode INSERT literal validation + offending-value retention) implemented. Phases 2–3, X4 (serial auto-fill bug), X5 (framework cohesion), and the natural-order error-value display remain. 1930 pass / 0 fail / 0 skip.
176 lines
9.3 KiB
Markdown
176 lines
9.3 KiB
Markdown
# 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_<cols>` (recomputed live, nothing persisted), reusing the
|
||
existing `DROP CONSTRAINT <name>` 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 = <literal>`; skip `SET col = <expr>` 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.
|