From a5cdb00a8609c39a7e3d6451b7633800f4ce4cbf Mon Sep 17 00:00:00 2001 From: "claude@clouddev1" Date: Sat, 23 May 2026 21:18:07 +0000 Subject: [PATCH] =?UTF-8?q?docs:=20session=20handoff=2033=20=E2=80=94=20AD?= =?UTF-8?q?R-0033=20sub-phase=203j=20+=20Amendment=203=20+=20/runda=20roun?= =?UTF-8?q?d?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/handoff/20260523-handoff-33.md | 244 ++++++++++++++++++++++++++++ 1 file changed, 244 insertions(+) create mode 100644 docs/handoff/20260523-handoff-33.md diff --git a/docs/handoff/20260523-handoff-33.md b/docs/handoff/20260523-handoff-33.md new file mode 100644 index 0000000..9b5f375 --- /dev/null +++ b/docs/handoff/20260523-handoff-33.md @@ -0,0 +1,244 @@ +# Session handoff — 2026-05-23 (33) + +Thirty-third handover. This session implemented **ADR-0033 sub-phase +3j** (dispatch wiring — making `insert`/`update`/`delete` shared +DSL/SQL entry words), and the implementation surfaced a chain of +cross-cut interactions that were escalated, decided with the user, and +resolved by **ADR-0033 Amendment 3** (the command-identity / mode model ++ a new combined-error UX feature). A user-invoked `/runda` round at the +end found and fixed two more issues. + +The next session does **sub-phase 3k** — the Phase-3 verification sweep +and phase-exit report. See §4. + +## §1. State at handoff + +**Branch:** `main`. **Tests: 1626 passing, 0 failing, 1 ignored.** +**Clippy:** clean (`cargo clippy --all-targets -- -D warnings`). + +**Latest commit (local-only):** + +``` +d5c7f63 grammar+walker: 3j — shared insert/update/delete entry words (ADR-0033 §2 / Amendments 1 & 3) +``` + +`origin/main` is at `6b8888f` (3h), so **8 commits are local-only** +(the six 3i commits, the handoff-32 doc commit `c16196f`, and this +3j commit `d5c7f63`). Unpushed commits are a normal working state; +pushing is the user's step — do not prompt about it. + +## §2. Process lessons (read first) + +1. **The "DSL is the Simple-mode surface" premise was wrong about the + tests.** The plan's 3j exit gate assumed the DSL `insert/update/ + delete` tests run in Simple mode. They do not — the common helpers + (`ok`/`err`/`parse`, `parse_command`, db `parse_filter`, the + `typing_surface` `assess`) default to **Advanced** mode. Pre-3j that + was harmless (those words were DSL-only, so Advanced still hit the + DSL grammar). Once the words became *shared*, Advanced routes them + to SQL. The fix was to **mode-correct the DSL tests to Simple** + (inputs/assertions unchanged), not to rewrite assertions. Recorded + as ADR-0033 Amendment 3. + +2. **The dispatch model needed three escalation rounds with the user + to get right** (recorded as Amendment 3). The user's framing — which + is now the authoritative model — is: + - A command is the **mode-rooted grammar-path outcome**; its identity + is intrinsic (`Command::Insert` vs `Command::SqlInsert` are + distinct, both correct, both tested). + - **Execution is mode-independent** today; the differences are + "visual" (output/diagnostics), so a *valid* command replays / + executes identically regardless of which variant is produced. + - Simple mode **commits the DSL candidate** for a shared word + (surfacing the real DSL error); "this is SQL" is reserved for + entry words with no DSL form (`select`/`with`). + +3. **`/runda` again earned its keep** (last session it found 6 bugs; + this session 2). Lesson re-confirmed: *probe, don't reason* — every + suspicion was checked with a throwaway test that printed the actual + `parse_command` / `ambient_hint` output before any conclusion. + Reasoning alone had me wrong twice (the `--all-rows` "regression" + that wasn't, and the replay "regression" that wasn't). + +4. **Escalate ADR-vs-implementation mismatches; don't "improve" + silently.** The simple-mode behaviour, the replay mode, and the + combined-pointer UX were all escalated to the user before coding and + resolved by Amendment 3. The user values this; keep doing it. + +## §3. What shipped this session (detail) + +All in commit `d5c7f63`; the authoritative record is **ADR-0033 +Amendment 3** (`docs/adr/0033-sql-dml-grammar.md`) — read it. + +### 3j dispatch wiring (the planned work) +- `data.rs`: `SQL_INSERT`/`SQL_UPDATE`/`SQL_DELETE` CommandNodes moved + off the dev words (`sqlinsert`/`sql_update`/`sql_delete`) to the real + entry words `insert`/`update`/`delete`, category `Advanced`, alongside + the `Simple` DSL nodes. `build_sql_{insert,update,delete}` collapsed + to `source.trim()` (the row-source extraction is span-based, so it was + entry-word-agnostic already). +- `walker/mod.rs`: the two REGISTRY entry-word listing sites + (`completion_probe_in_mode`, `expected_at_input_in_mode`) now + de-duplicate the shared primaries. +- Dev-word inputs migrated to the real words across `tests/sql_*.rs`, + the walker diagnostic tests, and `completion.rs` (advanced mode). + +### Dispatch model fixes (the cross-cut chain) +- **`decide` (walker/mod.rs)** — *Simple mode* now commits the DSL + candidate for a shared word (was: emitted "this is SQL" on + DSL-mismatch + SQL-match, which discarded the actionable DSL error + and broke phase-D / completion). *Advanced mode* commits the first + candidate that is `Match` **or** `ValidationFailed` (so an internal- + table rejection on the SQL candidate is surfaced, not masked by the + DSL fallback). Removed the now-dead `scratch_full_match`. +- **Mode-aware helpers** so simple-mode hints stop leaking the advanced + SQL view of a shared word: `classify_input_with_schema_in_mode` + (input_render), `invalid_ident_at_cursor_in_mode` (completion). +- **Test-mode correction:** DSL grammar / completion / phase-D / db + tests parse in **Simple** mode (`parse_command_in_mode(.., Simple)`, + `cands_simple`, `assess` pinned to Simple). `replay` keeps its + **Advanced** model (one stale "parse error" assertion relaxed to + "rejected, state intact"). + +### Combined DSL-error + advanced-SQL pointer (user-requested feature) +A Simple-mode **definite** DSL error whose line would run as SQL in +advanced mode keeps its DSL prose **and** gains the +`advanced_mode.also_valid_sql` suffix — e.g. *"for `Name`: Type a +quoted string … (valid as SQL in advanced mode — `mode advanced` or +prefix `:`)"*. Lives in `input_render::ambient_hint_in_mode` (live +typing) and `App::dispatch_dsl` (submit), both via the shared +`input_render::advanced_alternative_note`. New catalog key +`advanced_mode.also_valid_sql` (keys.rs + en-US.yaml). + +### `/runda` round (two findings) +- **Finding A (fixed):** the combined pointer reached multi-row / + UPSERT in the live hint but **not** `delete … returning` (live + completion shadowed the error) and **not the submit path at all**. + Fixed by adding the pointer to `dispatch_dsl` (submit) — now every + Simple-mode definite-DSL-error-that's-valid-SQL gets it on submit. +- **Finding B (fixed):** simple-mode `insert into __rdbms_* …` parsed + to `Command::Insert` — the DSL data-command target slots had **no** + `reject_internal_table` (advanced SQL rejected them; simple did not). + Added the validator to the three DSL target slots + (`TABLE_NAME_EXISTING` / `_INSERT` / `_WRITES` in data.rs) so + insert/update/delete/show-data/show-table refuse `__rdbms_*` in both + modes (ADR-0030 §6 — "every table-source slot"). + +## §4. Sub-phase 3k — the NEXT job (verification sweep) + +Read `docs/plans/20260520-adr-0033-phase-3.md` "Sub-phase 3k" and the +"Cross-cut verification matrix" section. + +**Goal:** the Phase-3 phase-exit verification. Per the plan: +1. Author/extend the **end-to-end (Tier-3) integration tests** for the + real-world DML shapes listed in 3k's exit gate (INSERT…SELECT + cross-table; multi-row INSERT covering the ten types; UPDATE with + subquery in SET; DELETE with cascade; UPSERT round-trip; RETURNING + on all three; `history.log` replay of every Phase-3 form). +2. **Fill the cross-cut verification matrix** (every "X comes for free" + claim from ADR-0030/0031/0032/0033 → a passing test whose INPUT is + SQL syntax for SQL claims). Mark each row's `file::function`. +3. **Final DA review** — genuine, specific critiques first, verdict + last; rubber-stamp PASS is a process failure. +4. Produce the **phase-exit report** at + `docs/handoff/-phase-3-verification.md`. +5. Confirm zero failures / zero skips / no regression from the Phase-2 + baseline (1446/0/1; this session's count is **1626/0/1** and rising + as tests were added per sub-phase). Clippy clean. + +**3k heads-up — re-run the full diagnostic + dispatch set.** 3j changed +how shared-word inputs route by mode; the new dispatch (Amendment 3) is +covered but exercise the compound forms (INSERT…SELECT…ON CONFLICT… +RETURNING) in *both* modes, and confirm the combined-pointer + internal- +table behaviour on the awkward inputs (per §2 lesson 3, *probe* the +output, don't assume). + +## §5. Decisions settled this session (do not re-litigate) + +Recorded in **ADR-0033 Amendment 3** (Accepted-in-spirit; the ADR is +still "Proposed" overall pending 3k). Key points: + +- **Simple mode = DSL surface; commit the DSL candidate for shared + words.** "This is SQL" (bare) only for `select`/`with`. A shared-word + SQL construct in simple mode → DSL error **+ combined pointer**. +- **Advanced mode = SQL-first, DSL fallback.** Fallback fires on a + structural mismatch (`delete … --all-rows`); a content rejection + (internal table) on the SQL candidate is committed, not fallen back. +- **`update … --all-rows` in advanced does NOT fall back** — the SQL + `SET` expression absorbs `--all-rows` as `42 - -all - rows`; harmless + because the engine treats `--all-rows` as a SQL comment, so it runs as + "all rows" anyway. (Only `delete … --all-rows` falls back.) +- **Replay parses in Advanced mode** (the full surface, same parser as + interactive) — `run_replay` is unchanged in mode; a valid log replays + identically by the §6/§7 parity guarantees, so replay needs **no** + per-line mode. The deferred M4 is *not* a replay prerequisite. +- **Execution-time mode side-channel = deferred (M4).** Three-way `Mode` + (simple / advanced / advanced-one-shot) threaded to the worker for + mode-dependent *output* (e.g. simple `create table` echoing generated + SQL in advanced). Tracked in `requirements.md` **M4**; gets its own + ADR. **Not** a Phase-3 dependency. + +## §6. Tracked deferred items (nothing lost) + +Pick up after Phase 3 (3k) per the user's standing "extra tasks after +phase 3 is complete". + +- **M4 — execution-time mode side-channel** (new this session; + `requirements.md`). See §5. Its own future ADR. +- **TASK #8 — implement ADR-0034 (history journal).** `history.log` + success-only ⇒ failed commands recallable in-session but lost across + sessions. ADR-0034 (`docs/adr/0034-…`, Accepted): complete journal + tagged `ok`/`err`; hydration reads all, replay reads `ok` only. +- **TASK #9 — fix broken replay + add the missing test.** `run_replay` + parses each whole line through the DSL parser with no understanding of + the `||` journal format, so `replay history.log` + fails on line 1. (Note: 3j confirmed replay's *mode* is correct — + Advanced; #9 is about the *line format*, orthogonal.) + +### Lower-priority observations (discussed, acceptable) +- **DDL internal-table rejection** is still absent: `drop table + __rdbms_*`, `add column to __rdbms_* …` etc. are not refused at parse + (Finding B fixed only the *data* commands, which have SQL + counterparts). No SQL DDL exists to compare against; flagged for a + future pass. +- The migrated test helpers `run_sqlinsert` / `run_update` / + `run_delete` (tests/sql_*.rs) keep their dev-word-era names though + they now parse the real words — cosmetic, low priority. +- `also_valid_sql` is *not* appended to mid-typing (incomplete) hints, + by design (avoids noise during normal DSL entry). + +## §7. Process pins (unchanged, still binding) + +- **Confirm every commit.** Propose the message; wait for the go-ahead. + No auto-commit at sub-phase gates (the user confirmed each commit). +- **Push is the user's step.** Never push; never prompt about it. +- **No AI attribution** in commits (global rule). +- **Test-first / probe-then-assert.** Reproduce with a failing (or + printing) test before concluding. `/runda`'s model: probe → confirm → + fix → keep the test. +- **Core walker changes** (`walk_seq` / `walk_choice` / `walk_repeated`) + need explicit user OK. `decide` (the dispatcher) and new post-walk + passes do not — but Amendment-worthy *behaviour* changes are + escalated and recorded (as Amendment 3 was). +- **Escalate ADR-mechanism mismatches; never classify work "out of + scope" without user confirmation.** (Finding B was fixed only after + the user said to; the DDL gap was flagged, not silently dropped.) + +## §8. How to take over + +1. **Read, in order:** this file → + `docs/adr/0033-sql-dml-grammar.md` (**Amendment 3** is the model + this session settled; also Amendment 1 = dispatch, Amendment 2 = + cascade) → `docs/plans/20260520-adr-0033-phase-3.md` "Sub-phase 3k" + + "Cross-cut verification matrix" → `CLAUDE.md` → + `docs/requirements.md` (note new **M4**). +2. **Baseline:** + ``` + cargo test # expect 1626 passing / 0 failing / 1 ignored + cargo clippy --all-targets -- -D warnings # clean + ``` +3. **Start 3k** per §4: end-to-end DML integration tests, fill the + cross-cut matrix, final DA review, phase-exit report at + `docs/handoff/-phase-3-verification.md`. +4. **Escalate** anything not settled in ADR-0033 / its amendments / the + plan; the user wants mismatches surfaced, not silently fixed.