docs: session handoff 43 — ADR-0035 Am1 follow-ups + ADR-0036 (surgical) Phase 1
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.
This commit is contained in:
claude@clouddev1
@@ -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_<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.
|
||||
Reference in New Issue
Block a user