claude@clouddev1
ded1ca8e26
Twenty-ninth handover. This session drafted ADR-0033 as the planning artifact for ADR-0030 Phase 3 (SQL DML in Advanced mode). Documents the ten settled design decisions (Q1-Q10), the dispatch architecture (SQL-first / DSL-fallback via the new Node::Guard mechanism), the eleven phased sub-phases (3a-3k) each with their exit gates, and the open implementation risks (R1-R4). Points the next session at sub-phase 3a (Node::Guard scaffolding) as the concrete entry point, with the plan-doc cross-cut matrix as the immediate prerequisite task. Pins four process lessons from the Phase-2 session (DA rubber-stamp risk, defer-trap reflexes, tests-first on gap closure, matrix attribution verification). State: 1446 / 0 / 1 passing (unchanged — planning-only session). Clippy clean.
229 lines
11 KiB
Markdown
229 lines
11 KiB
Markdown
# Session handoff — 2026-05-20 (29)
|
||
|
||
Twenty-ninth handover. This session **closed ADR-0032 Phase 2**
|
||
(see `docs/handoff/20260520-handoff-28.md`) and **drafted
|
||
ADR-0033** as the planning artifact for ADR-0030 Phase 3 (SQL
|
||
DML in Advanced mode). The next session starts implementation
|
||
at **sub-phase 3a** — see §3 below for the concrete entry point.
|
||
|
||
## §1. State at handoff
|
||
|
||
**Branch:** `main`. The Phase-2 commits were pushed by the user
|
||
mid-session; the Phase-3 planning commits in this session are
|
||
local-only and ready to be pushed at the user's convenience.
|
||
|
||
**Tests:** **1446 passing, 0 failing, 1 ignored** (unchanged
|
||
from handoff-28 — Phase 3 is planning-only this session).
|
||
|
||
**Clippy:** clean.
|
||
|
||
**This session's commits (Phase 3 planning):**
|
||
|
||
```
|
||
<this handoff>
|
||
<ADR-0033 + README index commit, pending>
|
||
fa417a4 docs: session handoff 28 — Phase 2 closed; Phase 3 is next
|
||
```
|
||
|
||
(All Phase-2 implementation commits are already pushed; see
|
||
handoff-28 §1 for the full list.)
|
||
|
||
## §2. Phase 3 design — settled
|
||
|
||
Ten design questions were discussed and settled in this session.
|
||
The full rationale lives in **`docs/adr/0033-sql-dml-grammar.md`**;
|
||
this is the index:
|
||
|
||
| Q | Topic | Decision |
|
||
|-----|----------------------------------------------------------------|---------------------------------------------------------------------------------------------------------|
|
||
| Q1 | `INSERT … SELECT` as a row source | **In scope** — recurses through `SQL_SELECT_COMPOUND` |
|
||
| Q2 | `INSERT INTO t DEFAULT VALUES` | **OOS** — the planned seed feature covers this case |
|
||
| Q3 | `RETURNING` clause on INSERT / UPDATE / DELETE | **In scope** on all three |
|
||
| Q4 | UPSERT (`ON CONFLICT … DO NOTHING / DO UPDATE`) | **Full UPSERT in scope** — including `excluded` pseudo-table |
|
||
| Q5 | `INSERT OR REPLACE / IGNORE / ABORT / FAIL / ROLLBACK` | **OOS** — SQLite-specific |
|
||
| Q6 | `UPDATE table FROM other_table` (multi-table) | **OOS** — SQLite/PostgreSQL extension, confusing pedagogically |
|
||
| Q7 | `shortid` auto-fill on SQL INSERT | **Worker post-fill** (parity with DSL `do_insert`) |
|
||
| Q8a | INSERT column-count arity mismatch | **Walker ERROR diagnostic** — `diagnostic.insert_arity_mismatch` |
|
||
| Q8b | Auto-gen column override warning | **Walker WARNING** — `diagnostic.auto_column_overridden` |
|
||
| Q8c | NOT-NULL-missing-on-INSERT | **Walker WARNING (advisory)** — `diagnostic.not_null_missing` |
|
||
| Q9 | Cascade summary on SQL DELETE | **Parity with DSL** — shared formatter, WHERE-clause byte-range injection into pre-count subqueries |
|
||
| Q10 | `Command` shape for DML execution | **Three typed variants** — `SqlInsert` / `SqlUpdate` / `SqlDelete` |
|
||
|
||
**Dispatch architecture (the hardest call):** Advanced mode runs
|
||
SQL-first with DSL fallback via `Choice(SQL_shape, DSL_shape)`
|
||
per shared entry word. The mode-gating mechanism requires a new
|
||
walker capability — `Node::Guard(fn)`, a zero-byte-consumption
|
||
gating node — which is the very first sub-phase's work (§3a in
|
||
the ADR).
|
||
|
||
## §3. Where the next session starts
|
||
|
||
**Sub-phase 3a — `Node::Guard` + mode-gated Choice mechanism.**
|
||
This is the foundation; if it doesn't work cleanly, the
|
||
mitigation paths (alternative dispatch mechanisms) are evaluated
|
||
before any DML grammar lands. ADR-0033 §"Open implementation
|
||
risks" R1 budgets for this explicitly.
|
||
|
||
**Concrete first-day tasks:**
|
||
|
||
1. **Read** the following, in order:
|
||
- This handoff (you're already doing it).
|
||
- `docs/handoff/20260520-handoff-28.md` (Phase 2 closing
|
||
state + the four non-blocking observations).
|
||
- `docs/adr/0030-advanced-mode-sql-surface.md` (the source
|
||
roadmap).
|
||
- `docs/adr/0033-sql-dml-grammar.md` (the Phase-3 grammar
|
||
ADR — the controlling document for all DML decisions).
|
||
- `CLAUDE.md` (project conventions; DA discipline reminder
|
||
in particular).
|
||
|
||
2. **Run baseline:**
|
||
```
|
||
cargo test # expect 1446 / 0 / 1
|
||
cargo clippy --all-targets -- -D warnings # clean
|
||
```
|
||
|
||
3. **Write the Phase-3 implementation plan doc** at
|
||
`docs/plans/<date>-adr-0033-phase-3.md`, modelled on
|
||
`docs/plans/20260520-adr-0032-phase-2.md`. ADR-0033's
|
||
per-sub-phase exit gates are detailed enough that the
|
||
plan doc can be focused on the **cross-cut verification
|
||
matrix scaffold** (the rows to fill in during 3k). Suggested
|
||
matrix sources:
|
||
- ADR-0033 §1 (statement shapes) — one row per shape per
|
||
row-source variant (VALUES single, VALUES multi-row,
|
||
INSERT…SELECT, …).
|
||
- ADR-0033 §2 (dispatch) — one row per mode × DSL/SQL
|
||
branch.
|
||
- ADR-0033 §5 (RETURNING) — one row per statement kind.
|
||
- ADR-0033 §6 (shortid auto-fill) — single-row, multi-row,
|
||
INSERT…SELECT, override case.
|
||
- ADR-0033 §7 (cascade summary) — DSL parity row + WHERE
|
||
subquery case (R2 mitigation).
|
||
- ADR-0033 §8 (diagnostics) — positive + negative per key.
|
||
- ADR-0033 §9 (UPSERT) — DO NOTHING + DO UPDATE rows.
|
||
|
||
4. **Implement sub-phase 3a** per ADR-0033 §Implementation
|
||
notes:
|
||
- Add `Node::Guard(fn(&WalkContext) -> Result<(),
|
||
ValidationError>)` to `src/dsl/grammar/mod.rs::Node`.
|
||
- Add `walk_guard` to `src/dsl/walker/driver.rs`.
|
||
- Add the `reject_sql_in_simple_mode` guard function.
|
||
- Build the smoke-test grammar (a single experimental
|
||
`CommandNode` with a `Choice(SQL_branch_gated,
|
||
DSL_branch)` shape that uses two distinguishable tokens).
|
||
- Verify the four exit-gate test cases in ADR-0033 §3a.
|
||
|
||
5. **DA gate (sub-phase 3a):** apply the written DA gate from
|
||
ADR-0033 §3a verbatim. The "Does the Guard actually fire
|
||
BEFORE the Seq commits?" question is the make-or-break
|
||
test — write that test explicitly. If it fails, R1's
|
||
mitigation reopens sub-phase 3a with mechanism option (b)
|
||
or (c).
|
||
|
||
6. **Commit** with a message that explicitly names the
|
||
mechanism choice (a/b/c) actually used. If you had to fall
|
||
back to option (b) or (c), update ADR-0033 §2's "Default
|
||
choice" sentence in the same commit.
|
||
|
||
## §4. Process pins from this session
|
||
|
||
These are NOT new project rules — they're lessons reinforced
|
||
this session that the next session should keep in mind.
|
||
|
||
### 4.1. DA discipline — first pass is rubber-stamp risk
|
||
|
||
Phase 2's verification report shipped with a rubber-stamp PASS
|
||
on the first DA pass. The user had to challenge it to get
|
||
specific critiques. This session's ADR-0033 DA review took two
|
||
passes: the first pass produced seven specific critiques, all
|
||
addressed in the body; the second pass surfaced an eighth real
|
||
gap (the `Node::Guard` capability didn't exist) and added it.
|
||
|
||
**Pin for sub-phase exits:** DA review lists specific critiques
|
||
first, concludes after. A clean PASS without critiques is a
|
||
process failure even if the work is sound — the DA hat has to
|
||
PROVE engagement.
|
||
|
||
### 4.2. Defer-trap reflexes
|
||
|
||
Phase 2 surfaced this pattern repeatedly: when scope expanded,
|
||
the implementer's reflex was to label items "out of scope" or
|
||
"non-blocking" without user approval, per CLAUDE.md's explicit
|
||
prohibition. The user corrected this several times.
|
||
|
||
**Pin for Phase 3:** every "out of scope" classification must
|
||
either (a) be already declared OOS by an ADR, or (b) be
|
||
escalated to the user. The default is "in scope, in this
|
||
sub-phase, now."
|
||
|
||
### 4.3. Tests-first on gap closure
|
||
|
||
Phase 2's 2g rework added 13 tests AFTER the gaps were
|
||
discovered, not before. The work was sound but the ordering was
|
||
backwards.
|
||
|
||
**Pin for Phase 3:** when a sub-phase's exit gate exposes a gap
|
||
in an earlier sub-phase, write the failing test FIRST, then fix.
|
||
This is the bug-fix discipline from CLAUDE.md applied to
|
||
gap-closure.
|
||
|
||
### 4.4. Matrix attribution must be SQL tests, not DSL tests
|
||
|
||
Phase 2's matrix row "validity indicator fires for SQL" was
|
||
attributed to a DSL test (the user-visible bug it left was
|
||
fixed in the rework). Generalisable lesson:
|
||
|
||
**Pin for Phase 3:** when filling the matrix in sub-phase 3k,
|
||
each row pointing at a SQL surface must point at a test whose
|
||
INPUT is SQL syntax. Cross-check at attribution time, not at
|
||
DA-review time.
|
||
|
||
## §5. Pinned non-blocking observations from Phase 2
|
||
|
||
These items are from `docs/handoff/20260520-handoff-28.md` §3.
|
||
**They're not blockers for Phase 3** but a Phase-3 implementer
|
||
may decide to address one or more if they're nearby:
|
||
|
||
- §3.1 `translate_generic`'s "group by" pattern overbreadth in
|
||
`src/friendly/translate.rs`.
|
||
- §3.2 Look-ahead probe cost in `src/completion.rs` — second
|
||
walk per Tab press when leading scope is empty; not
|
||
benchmarked.
|
||
- §3.3 Process: tests-after-code on matrix coverage.
|
||
- §3.4 Process: matrix attribution wasn't verified row-by-row.
|
||
|
||
## §6. Open work — unchanged from handoff-28
|
||
|
||
Same list as handoff-28 §6. Phase 3 (this session's planning
|
||
target) is one of the items; the others remain:
|
||
|
||
- Project storage Iter 5/6.
|
||
- Snapshot / replay / undo (U-series).
|
||
- m:n convenience (C4); modify relationship (C3a); rename
|
||
table (C1); column drops/renames/type changes (B2/C2).
|
||
- Friendly-error sweep (H1); strong syntax-help in parse
|
||
errors (H1a).
|
||
- CI (TT5).
|
||
- Session log + Markdown export (V4).
|
||
- Readline shortcuts (I1b), multi-line input (I1), tab
|
||
completion polish (I3), syntax highlighting (I4).
|
||
- ER diagram export (V3).
|
||
- Tutorial / lesson system.
|
||
|
||
## §7. How to take over
|
||
|
||
1. **Read this file in full.** Then handoff-28, then
|
||
ADR-0033, then `CLAUDE.md`.
|
||
2. **`cargo test`** — expect **1446 passing, 0 failed, 1
|
||
ignored**.
|
||
3. **`cargo clippy --all-targets -- -D warnings`** — clean.
|
||
4. **First task: write the Phase-3 plan doc** (per §3 step 3).
|
||
Don't skip this — the cross-cut matrix is the scaffolding
|
||
that lets sub-phases 3b–3k know what they're aiming at.
|
||
5. **Second task: sub-phase 3a** (`Node::Guard` + mode-gated
|
||
Choice mechanism). Per §3 steps 4–6.
|
||
6. **Escalate** any ambiguity not already settled in ADR-0033.
|
||
Don't decide silently; don't classify items "out of scope"
|
||
without checking.
|