docs: session handoff 47 + M4 update — DSL→SQL echo Phase 1 (Bucket A) done
This commit is contained in:
claude@clouddev1
@@ -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 <pk> 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 <pk>` 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 <auto> 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>` | resolved name from the pre-execution describe (the executor knows it) |
|
||||
| `add 1:n relationship [as N] …` | `ALTER TABLE C ADD CONSTRAINT <name> 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 <name>` | resolved name + child table |
|
||||
| `add 1:n relationship … --create-fk` (child col created) | `ALTER TABLE C ADD COLUMN cc <ty>` ⏎ `ALTER TABLE C ADD CONSTRAINT <name> 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 <ix1>` ⏎ … ⏎ `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<String>`** (one statement per line)
|
||||
to generalise `Option<String>` 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<String>` 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 <name>`
|
||||
(→ `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<String>` → `Vec<String>` 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.
|
||||
Reference in New Issue
Block a user