diff --git a/docs/handoff/20260528-handoff-47.md b/docs/handoff/20260528-handoff-47.md new file mode 100644 index 0000000..7c78b52 --- /dev/null +++ b/docs/handoff/20260528-handoff-47.md @@ -0,0 +1,245 @@ +# Session handoff — 2026-05-28 (47) + +Forty-seventh handover. **Phase 1 of the DSL → SQL teaching echo +(ADR-0038) is complete.** Bucket A — every single-statement DDL row plus +`show data` and the `--all-rows` fall-throughs — is implemented, +round-tripped, and verified. Read §4 if you'll touch the echo path +(there's one structural addition since handoff-46 you need to know +about); §5 is the next-slice plan. + +## §1. State at handoff + +**Branch:** `main`. **HEAD `90479cb`.** **Tests: 2000 passing, 0 failing, +0 skipped, 1 ignored.** **Clippy: clean** (`--all-targets -D warnings`, +nursery). + +Commits since handoff-46's `04c8e42`: + +``` +90479cb feat: DSL→SQL teaching echo — Bucket A renderer (ADR-0038 Phase 1) +``` + +(`+1049 / −25`, five files: `src/echo.rs` `src/event.rs` +`src/runtime.rs` `src/app.rs` `tests/walking_skeleton.rs`.) + +## §2. ADR status + +| ADR | What | Build status | +|---|---|---| +| **ADR-0035 Am2** | standard-first dialect + `ALTER COLUMN` gap-fill | ✅ done (handoff-46 `338dc8a`) | +| **ADR-0033 Am4** | `update … --all-rows` falls back to DSL | ✅ done (handoff-46 `9a23e28`) | +| **ADR-0037** | `EffectiveMode` execution-time side-channel | ✅ done (handoff-46 `04c8e42`) | +| **ADR-0038** | the echo + full catalogue | 🚧 **Phase 1 done** (`90479cb`); Phase 2 (Bucket B) + Phase 3 (cat-3 prose) + the styled-runs polish remain | +| **ADR-0039** | EXPLAIN over advanced SQL | ⏸ deferred follow-up — separate feature | + +ADR-0037 / ADR-0038's status fields still read **Proposed**. The designs +are unchanged and `/runda`'d (handoff-46 §2); whether to promote either +to **Accepted** with an implementation note (matching the pattern of +ADR-0033 / ADR-0035) is a small status-bookkeeping decision worth +raising explicitly rather than amending unilaterally. + +## §3. What Phase 1 delivered (the renderer, in code) + +The echo now covers the full single-statement catalogue. Every row +round-trips through the advanced walker (ADR-0038 §1 copy-paste contract): + +- `add column to T: c (ty) [not null][unique][default v][check e]` + → `ALTER TABLE T ADD COLUMN c ty …` +- `drop column from T: c` (non-cascade) → `ALTER TABLE T DROP COLUMN c` +- `rename column in T: a to b` → `ALTER TABLE T RENAME COLUMN a TO b` +- `change column in T: c (ty)` → `ALTER TABLE T ALTER COLUMN c SET DATA TYPE ty` + (headline only — every conversion mode; the `--dont-convert` caveat is + category-3 prose, Phase 3) +- `add constraint not null/default/unique/check` + → `SET NOT NULL` / `SET DEFAULT v` / `ADD UNIQUE (c)` / `ADD CHECK (e)` +- `drop constraint not null/default` → `DROP NOT NULL` / `DROP DEFAULT` +- `show data T [where …] [limit n]` + → `SELECT * FROM T [WHERE …] [ORDER BY LIMIT n]` +- `delete from T --all-rows` → `DELETE FROM T` +- `update T set … --all-rows` → `UPDATE T SET …` + +Bucket C correctly returns `None`: `drop constraint unique/check` +(anonymous, ADR-0035 Am2 residual gap), `Sql*` / `Select` (already SQL), +and where-filtered `update`/`delete` (route SQL-first in advanced mode +per ADR-0033 Am3). + +Supporting machinery added: + +- **`Value → SQL-literal`** (ADR-0038 §5): `NULL` uppercase, otherwise + reuses `Value::Display` (`'O''Hara'`, bare numbers, `true`/`false`, + quoted ISO dates). +- **`Expr → SQL`** mirrors the worker's `compile_expr` operator spellings + (`<>`, `LIKE`, `BETWEEN`, `IN`, `IS NULL`, parenthesised `AND` / `OR` + / `NOT`) but emits bare identifiers and inlined literals — the echo's + unquoted / runnable style (matching the existing `render_create_table` + aesthetic). +- **One `append_constraints` helper** shared by `create table` and + `add column` for the `NOT NULL` / `UNIQUE` / `DEFAULT` / `CHECK` + suffix. + +### §3a. Two execution-time decisions worth flagging + +1. **Create-table echo gap fixed (deviation from handoff-46's "done").** + The handoff-46 skeleton's `render_create_table` silently dropped + per-column `default` / `check` — but simple-mode + `create table T with pk age(int) check (age >= 0)` does parse those + (ADR-0029), so the echo was non-equivalent. The fix: render the full + ADR-0029 constraint suffix on create-table columns too, sharing the + new `append_constraints` helper with `add column`. Not new scope — + it's catalogue row 1 done correctly. +2. **`show data … limit n` primary-key sourcing.** The + `ORDER BY ` isn't on the `Command`. The pure renderer + (`echo::show_data_to_sql` / `echo::echo_for_query`) takes the PK as a + parameter; the runtime sources it post-execution via + `database.describe_table` — gated on advanced mode + `limit.is_some()`, + mirroring the existing `enrich_dsl_failure` describe pattern (zero + struct churn, localised one-call lookup, rare path). An end-to-end + test (`runtime::tests::show_data_echo_orders_by_resolved_primary_key_when_limited`) + pins the describe → PK → `ORDER BY` glue against a real worker. + +## §4. The architectural shift you need to know about + +Handoff-46's skeleton computed the echo **before** execution +(`echo_for(&command, mode)` once, threaded into every success arm). +That's still the path for everything *except* `show data`, where +Phase 1 added a **second, post-execution path**: + +- **Pre-execution** (`echo::echo_for`, `echo::command_to_sql`) builds + the echo from the `Command` alone. Covers every Bucket A row whose + echo is a pure function of `Command`. +- **Post-execution for Query outcomes** (`echo::echo_for_query`): + `show data` is the only DSL-form query that echoes; its limited form + needs the table PK. The runtime's `Query` arm calls + `build_show_data_echo(&database, &command, submission_mode)` — which + early-returns `None` for simple mode, then `describe_table`s only + when the query is limited, then hands `(command, mode, pk)` to + `echo_for_query`. `Command::Select` returns `None` (already SQL). + +This is the architecture the handoff-46 ⚠️ gotcha foreshadowed for +Bucket B / category-3 ("move/augment the echo build to AFTER +execution"). Phase 1 needed it one row early (for `show data`'s PK), +and it's there now — the pattern is established. **Phase 2 will need to +move the *resolved-name* and *multi-statement* renders to similar +post-execution paths** (see §5). + +### Event wiring + +The six remaining success events now carry an `echo` field: +`DslDataSucceeded`, `DslUpdateSucceeded`, `DslDeleteSucceeded`, +`DslChangeColumnSucceeded`, `DslAddColumnSucceeded`, +`DslDropColumnSucceeded`. (`RenameColumn` / `AddConstraint` / +`DropConstraint` flow through the already-wired `DslSucceeded`.) Each +runtime arm sets `echo` from the pre-execution `echo` var (or the +post-execution `build_show_data_echo` for the Query arm); each `update()` +arm stashes `self.pending_echo = echo` before its handler runs. All +handlers call `note_ok_summary` first — the existing consumer of +`pending_echo`. One App-level test +(`bucket_a_success_events_render_the_teaching_echo_beneath_ok`) guards +every new arm's stash + ordering. + +## §5. What's next (the actual work) + +### Phase 2 — Bucket B (resolved names + multi-statement) + +Per ADR-0038 §7. Each row needs an addition to the **post-execution +arm** of the relevant outcome — the runtime has both `Command` and the +worker's result there. + +| DSL command | Echo | Sources | +|---|---|---| +| `add index on T (cols)` (auto-named) | `CREATE INDEX ON T (cols)` | resolved name from `CommandOutcome::Schema(desc)` — `desc.indexes` matching the requested column set | +| `add index as N on T (cols)` | `CREATE INDEX N ON T (cols)` | pure `Command` — could land in Phase 1 by name, kept in Phase 2 here for uniformity | +| `drop index on T(cols)` (positional) | `DROP INDEX ` | resolved name from the pre-execution describe (the executor knows it) | +| `add 1:n relationship [as N] …` | `ALTER TABLE C ADD CONSTRAINT FOREIGN KEY (cc) REFERENCES P (pc) [ON DELETE X] [ON UPDATE Y]` | resolved name; ADR-0013 metadata | +| `drop relationship (from P.pc to C.cc \| N)` | `ALTER TABLE C DROP CONSTRAINT ` | resolved name + child table | +| `add 1:n relationship … --create-fk` (child col created) | `ALTER TABLE C ADD COLUMN cc ` ⏎ `ALTER TABLE C ADD CONSTRAINT FOREIGN KEY (cc) REFERENCES P (pc) …` | category-2 multi-statement (one line if column already existed) | +| `drop column T.c --cascade` (drops covering indexes) | `DROP INDEX ` ⏎ … ⏎ `ALTER TABLE T DROP COLUMN c` | category-2 multi-statement; index names from `DropColumnResult::dropped_indexes` | + +Mechanical changes Phase 2 will need: + +- **Echo payload likely becomes `Vec`** (one statement per line) + to generalise `Option` for category-2 multi-statement rows. + `note_ok_summary` would render each line beneath `[ok]`. Some + alternatives — e.g. one joined string with embedded newlines, or + keeping `Option` and joining with `\n` — work but are less + flexible if the styled-runs polish (§7) wants to style each line + separately. Decide at the start of Phase 2. +- **Resolved-name sources** vary by command: `DropColumnResult` already + carries `dropped_indexes`; for `add relationship` the resolved name + comes from the post-execution describe of the child table; for + `drop index` (positional) and `drop relationship` (by endpoints) the + executor resolves a name during execution but doesn't surface it in + the current result types — those may need small `CommandOutcome` + enrichments (a name field), or a post-execution lookup. Choose per + row. + +### Phase 3 — category-3 prose expansion + +Three lines (ADR-0038 §6): + +- **`shortid` generation** *(illuminating)* — built from + `client_side_notes` on `AddColumnResult` / `InsertResult` / + `ChangeColumnTypeResult` (the existing auto-fill notes). +- **type-conversion transform** *(illuminating)* — from + `ChangeColumnTypeResult::client_side` (`transformed`, `lossy`). +- **`change column … --dont-convert`** *(caveat)* — the headline is + *not* equivalent in this case; the prose line states the divergence. + +All three render from existing worker `client_side.*` notes — no new +worker plumbing needed; just route the notes into the echo channel. + +### Polish — de-emphasised styled-runs rendering + +The echo line is currently a plain `[system]` line via `note_system`. +ADR-0038 §4 wants it de-emphasised (ADR-0028 styled-runs: a dimmed +`Executing SQL:` prefix; the SQL in a code-ish run). This is a +rendering change in `app.rs`, not a renderer change. Still pending. + +### ADR status bookkeeping (small) + +ADR-0037 and ADR-0038 still read **Proposed**. Now that ADR-0037 is +fully implemented and ADR-0038 has its first phase shipped, an +`Accepted (implemented …)` / `Accepted (Phase 1 implemented …)` update +in the style of ADR-0033 / ADR-0035 would match the repo's pattern. +Not done in this session — a one-line decision worth raising with the +user before touching either ADR's Status block. + +## §6. Catalogue cheat-sheet (authoritative: ADR-0038 §7) + +Unchanged since handoff-46. **Echoes** (DSL-only spellings in advanced +mode): all DDL forms + `show data` + `delete`/`update … --all-rows`. +**Not echoed**: `insert` / `update … where` / `delete … where` +(SQL-first → `Sql*` = already SQL), `drop table`, `drop index ` +(→ `Sql*`), `show table`, `explain`, `replay`, app commands, +column-level UNIQUE / CHECK *drop* (ADR-0035 Am2 residual). +`change column --dont-convert` will echo the headline plus a category-3 +**caveat** (Phase 3). + +## §7. Process pins (unchanged) + +- **Confirm every commit** (propose message, wait). **No AI attribution.** +- **Test-first**; green + clippy-clean is the only acceptable end state; + current baseline **2000 / 0 / 1**. +- **Keep docs lockstep**: ADR + `README.md` index + `requirements.md`. + M4 was updated this session to reflect the now-implemented + side-channel + Phase 1 echo. +- **Round-trip every catalogue row** (the §1 copy-paste contract). +- **Heed the §4 architectural pattern**: Bucket B / category-3 rows + build their echo in the *post-execution* arm of the outcome they + produce, reading the result — `show data` is the established example. + +## §8. How to take over + +1. **Read, in order:** this file → **ADR-0038** (§1 contract, §6 + categories, §7 catalogue, §8 phasing) → **`src/echo.rs`** (the + renderer; the Phase 1 surface is the model for Phase 2) → + **`src/runtime.rs`**'s `spawn_dsl_dispatch` + `build_show_data_echo` + (the post-execution pattern). +2. **Baseline:** `cargo test` (2000 / 0 / 1) + `cargo clippy + --all-targets -- -D warnings` (clean). +3. **Continue** with §5 Phase 2 (Bucket B), test-first, one command at + a time, round-tripping each catalogue row. Decide the + `Option` → `Vec` echo payload question up front. + Phase 3 (category-3 prose) and the styled-runs polish follow. +4. **ADR-0039** (EXPLAIN over advanced SQL) is a separate deferred + follow-up, not this feature. diff --git a/docs/requirements.md b/docs/requirements.md index 1f695d7..347b06a 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -120,19 +120,29 @@ handoff-14 cleanup; 449 after B2/C2.) one-shot advanced escape (with the prompt label updated). The `mode simple` / `mode advanced` command switches modes persistently. -- [~] **M4** Execution-time mode side-channel — deferred, awaiting - its own ADR (ADR-0033 Amendment 3). Every command should know, at - execution time, which of three modes it ran under — `simple`, - `advanced`, or `advanced-one-shot` (the `:` escape) — so execution - can adjust *output* without changing command *identity* (e.g. a - simple-mode `create table` echoing the generated SQL when run in - advanced mode, silent in simple). Today only the *rendering* - side-channel exists (`OutputLine.mode_at_submission`); the `Mode` - enum is two-way, the one-shot distinction is collapsed at - submission, and neither `Action::ExecuteDsl` nor the worker carries - any mode. The work widens `Mode` to three-way and threads it - through the `Action` → worker interface. Not required for Phase 3 - dispatch correctness; tracked here so it is not lost. +- [x] **M4** Execution-time mode side-channel — **implemented** via + ADR-0037 (the channel) and its motivating consumer ADR-0038 (the + DSL → SQL teaching echo). Every command knows, at execution time, + which of three modes it ran under — the three-way + `EffectiveMode` { `Simple`, `AdvancedPersistent`, + `AdvancedOneShot` } resolves at submit time and threads through + `Action::ExecuteDsl` → runtime; the persistent `Mode` enum stays + two-way (the transient one-shot `:` lives on the channel where it + belongs, not in persistent state). The runtime gates the + ADR-0030 §10 teaching echo on it: a DSL-form command run in + advanced/one-shot mode renders the equivalent advanced-mode SQL + beneath `[ok]`; simple-mode and SQL-entered submissions stay + silent. Echo coverage progress: **Bucket A complete** (Phase 1 — + single-statement DDL + `show data` + `--all-rows` fall-throughs, + every row round-tripped per ADR-0038 §1; handoff-47); + **Bucket B** (resolved-name + multi-line echoes — `add index`, + relationship add/drop, `drop column --cascade`, + `add relationship --create-fk`) and **category-3 prose** + (`shortid` generation, type-conversion transforms, + `change column --dont-convert` caveat) are deferred to Phase + 2 / Phase 3 per ADR-0038 §8. The de-emphasised styled-runs + rendering polish (ADR-0038 §4 / ADR-0028) is also still pending — + the echo currently surfaces as a plain `[system]` line. ## App-level commands (per ADR-0003)