claude@clouddev1
257 lines
13 KiB
Markdown
257 lines
13 KiB
Markdown
# Session handoff — 2026-05-28 (48)
|
||
|
||
Forty-eighth handover. **ADR-0038 is feature-complete except for the §4
|
||
styled-runs rendering polish.** Buckets A + B and the §6 category-3 prose
|
||
all ship, every catalogue row round-trips per line, docs are in sync,
|
||
and a /runda Devil's-Advocate pass landed cleanup. The session built
|
||
the renderer out from the handoff-46 walking skeleton through Phases
|
||
1, 2, and 3 in one go; if you continue, the polish is the only open
|
||
item. If you start a different feature, ADR-0038 won't surprise you.
|
||
|
||
## §1. State at handoff
|
||
|
||
**Branch:** `main`. **HEAD `5cb105b`.** **Tests: 2015 passing, 0 failing,
|
||
0 skipped, 1 ignored.** **Clippy: clean** (`--all-targets -D warnings`,
|
||
nursery).
|
||
|
||
Commits since handoff-47's `c60026c`:
|
||
|
||
```
|
||
5cb105b docs(adr): /runda DA cleanup — reflect Phases 1-3 done, pin Bucket C
|
||
e6ad1ae feat: DSL→SQL teaching echo — Phase 3 cat-3 caveat (ADR-0038)
|
||
275c726 feat: DSL→SQL teaching echo — Bucket B renderer (ADR-0038 Phase 2)
|
||
558cfae docs(adr): promote ADR-0037 + ADR-0038 to Accepted (Phase 1 shipped)
|
||
```
|
||
|
||
(Plus handoff-47 itself, `c60026c`, immediately prior.)
|
||
|
||
## §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 | ✅ **Accepted** — channel done (handoff-46 `04c8e42`), validated end-to-end by all four ADR-0038 commits |
|
||
| **ADR-0038** | the echo + full catalogue | ✅ **Accepted** — Phases 1-3 done (`90479cb` / `275c726` / `e6ad1ae`); **§4 styled-runs polish remains** |
|
||
| **ADR-0039** | EXPLAIN over advanced SQL | ⏸ deferred follow-up — separate feature |
|
||
|
||
ADR-0037 and ADR-0038 were promoted to **Accepted** with implementation
|
||
notes in `558cfae`; the /runda cleanup (`5cb105b`) refreshed both
|
||
entries to reflect every shipping commit.
|
||
|
||
## §3. What landed since handoff-47
|
||
|
||
### §3a. Phase 2 — Bucket B renderers (`275c726`, +1049 / −53 across 4 files)
|
||
|
||
Two new echo-build paths the handoff-46 ⚠️ gotcha foreshadowed:
|
||
|
||
- **`collect_echo_lookups`** (pre-execution, runtime) — resolves names
|
||
the dropped thing or not-yet-created column would erase post-
|
||
execution: positional `drop index` (describe the table, find the
|
||
index by exact column-set match), `drop relationship` in both
|
||
selector forms (`Endpoints` via child describe + matching by
|
||
endpoints; `Named` via a small `list_tables` scan acceptable for
|
||
teaching-playground schemas), and the `--create-fk` pre-state
|
||
(whether the child column already existed + the parent's PK type to
|
||
derive the new column type via `Type::fk_target_type` — `serial →
|
||
int`, `shortid → text`).
|
||
- **`build_schema_echo`** (post-execution, runtime) — subsumes the
|
||
Bucket A pure-`Command` schema cases (`create table`, `rename
|
||
column`, `add`/`drop constraint` — delegates to `command_to_sql`)
|
||
*and* renders Bucket B from the description (`add index` resolved
|
||
from `desc.indexes` by column set; `add relationship` from the
|
||
parent's `inbound_relationships` by endpoints) and the lookups.
|
||
|
||
Plus a **payload refactor** — `Option<String>` → `Option<Vec<String>>`
|
||
on all 7 success events + arms + render path, motivated by the
|
||
category-2 multi-statement echoes (`drop column --cascade`, `add
|
||
relationship --create-fk`). Each catalogue row's pure renderer (and
|
||
its round-trip) lives in `src/echo.rs`; the runtime composes them.
|
||
|
||
The `DropColumn` arm gained `build_drop_column_cascade_echo`, which
|
||
reads `DropColumnResult::dropped_indexes` to emit the multi-line
|
||
cascade echo; non-cascade falls through to the pre-execution Bucket A
|
||
echo unchanged.
|
||
|
||
### §3b. Phase 3 — category-3 prose (`e6ad1ae`, +130 / −8 across 5 files)
|
||
|
||
Phase 3 turned out to be small: the existing `client_side.*` notes
|
||
already covered **shortid generation** (`auto_fill_add_shortid`,
|
||
`auto_fill_transition`) and **type-conversion transforms**
|
||
(`transformed`, `transformed_lossy`), and they already render in the
|
||
right position (between the echo and the structure). Only the **`change
|
||
column … --dont-convert` caveat** was actually missing — the one
|
||
Bucket A caveat (every other category-3 line is *illuminating*).
|
||
|
||
Wiring: a new `dont_convert_caveat: bool` on `DslChangeColumnSucceeded`,
|
||
set by the runtime when `submission_mode.is_advanced() && matches!(cmd,
|
||
ChangeColumnType { mode: DontConvert, .. })`, rendered by
|
||
`handle_dsl_change_column_success` between the client-side notes and
|
||
the structure. Gated on advanced mode because the caveat references
|
||
"the line above" — the echo, which only fires in advanced mode. New
|
||
i18n key `client_side.dont_convert_caveat` (registered in
|
||
`keys::KEYS_AND_PLACEHOLDERS`).
|
||
|
||
### §3c. /runda DA cleanup (`5cb105b`, +128 / −32 across 4 files)
|
||
|
||
Devil's-Advocate audit at the end of the session caught three doc-drift
|
||
bugs the per-phase commits left behind (requirements.md M4 and both
|
||
ADR-0038 Status block + README index entry still said "Phase 2 / Phase
|
||
3 remain") and one test gap — Bucket C cases (`show table`, `explain`,
|
||
`replay`, every `Command::App` variant) flowed through the catch-all
|
||
`_ => None` without explicit no-echo coverage. The new
|
||
`bucket_c_no_echo_commands_all_return_none` test pins all four; a
|
||
future renderer arm at the wrong place can no longer silently leak.
|
||
|
||
## §4. The architecture in shape now
|
||
|
||
Two echo construction paths now run in `spawn_dsl_dispatch`, side by
|
||
side:
|
||
|
||
- **Pre-execution** (`echo::echo_for` → `echo::command_to_sql`) — the
|
||
outer `echo` var, used by the **non-Schema** arms (`Update`,
|
||
`Delete`, `AddColumn`, `DropColumn` non-cascade, `ChangeColumn`).
|
||
Pure function of the `Command`. Returns `Option<Vec<String>>`
|
||
(single-element for the Bucket A single-line cases).
|
||
- **Post-execution** — three builders, each in the arm matching its
|
||
outcome:
|
||
- `build_schema_echo(command, mode, description, lookups)` for
|
||
`Schema` outcomes (subsumes Bucket A pure-Command schema +
|
||
Bucket B `add index` / `drop index` / `add`/`drop relationship`).
|
||
- `build_show_data_echo(database, command, mode)` for `Query`
|
||
outcomes (only `show data` echoes; the runtime describes the table
|
||
when the query is limited to source the `ORDER BY` PK).
|
||
- `build_drop_column_cascade_echo(command, mode, result)` for the
|
||
`DropColumn` arm when `cascade: true` (reads
|
||
`DropColumnResult::dropped_indexes`); non-cascade falls through to
|
||
the pre-execution echo.
|
||
|
||
The pre-execution lookup helper `collect_echo_lookups` runs once at the
|
||
top of the dispatch, captures whatever drops or `--create-fk` pre-state
|
||
will need post-execution, and passes through to `build_schema_echo`.
|
||
Replay still bypasses the whole spawn (`run_replay` calls
|
||
`execute_command_typed` directly) so no echo fires there — per
|
||
ADR-0038 §3.
|
||
|
||
Rendering: `note_ok_summary` loops over `pending_echo: Vec<String>`,
|
||
emitting one `Executing SQL: <line>` per entry via `note_system`.
|
||
Plain `[system]` lines today — the polish replaces this with
|
||
ADR-0028 styled runs.
|
||
|
||
## §5. The one open item — §4 styled-runs polish
|
||
|
||
ADR-0038 §4: "*The App renders it as one or more **de-emphasised**
|
||
`OutputLine`s beneath the `[ok]` summary, using the ADR-0028 styled-
|
||
runs mechanism (a dimmed `Executing SQL:` prefix; the SQL in a code-ish
|
||
run). One statement per line (§6 category 2).*"
|
||
|
||
Today the echo + the DontConvert caveat surface as plain `note_system`
|
||
lines (a single style). The polish needs:
|
||
|
||
- Read **ADR-0028** for the styled-runs `OutputLine` shape — there is
|
||
already a mechanism (used by the explain plan tree rendering — see
|
||
`output_render::render_explain_plan` for the established pattern of
|
||
pushing pre-built `OutputLine`s via `push_output`).
|
||
- Replace `note_system(crate::t!("echo.executing_sql", sql = line))` in
|
||
`note_ok_summary` with a styled `OutputLine`: dimmed `Executing
|
||
SQL:` prefix run + a code-ish run for the SQL itself. Decide the
|
||
exact style (italic? dim? distinct fg colour?) consistent with
|
||
ADR-0028's existing styled-runs surface; check both `theme::light()`
|
||
and `theme::dark()` for legibility.
|
||
- The DontConvert caveat is also a category-3 prose line — apply the
|
||
same de-emphasised treatment (or a complementary one — caveat could
|
||
warrant a `WARNING`/yellow accent, since it flags non-equivalence).
|
||
- Multi-statement echoes: each line in the Vec becomes its own styled
|
||
`OutputLine`. Today the rendering already emits one line per entry;
|
||
the polish is per-line styling, not per-line splitting.
|
||
|
||
Tests will need adjustment: the App-level tests (`bucket_a_success_*`,
|
||
`bucket_b_multi_line_*`, `change_column_dont_convert_renders_*`)
|
||
currently assert `texts[…].contains("Executing SQL: …")` from
|
||
`note_system` output. After the polish, the same content will live in
|
||
styled-runs — the tests should still find the substring within the
|
||
output line text but may need to look at `OutputLine` styled-run
|
||
content rather than the flat `text` field. Decide style-vs-text
|
||
assertion shape early.
|
||
|
||
The work is rendering-only — no Command-level changes, no event-shape
|
||
changes, no worker changes. Estimated scope: small. The biggest care
|
||
point is consistency with ADR-0028's existing styled-runs usage.
|
||
|
||
## §6. Catalogue cheat-sheet (authoritative: ADR-0038 §7)
|
||
|
||
Unchanged since handoff-47. **Every Bucket A and Bucket B row is now
|
||
implemented and round-tripped** (`echo::tests::*` + the runtime
|
||
end-to-end tests `bucket_b_resolved_name_echoes_against_real_worker` +
|
||
`bucket_b_multi_statement_echoes_against_real_worker`). Bucket C
|
||
(no-echo) is explicitly pinned by
|
||
`bucket_c_no_echo_commands_all_return_none`. Category-3 lines flow
|
||
through the existing `client_side.*` infrastructure plus the new
|
||
`dont_convert_caveat`.
|
||
|
||
## §7. Known follow-ups outside the ADR-0038 scope
|
||
|
||
These are **not** the styled-runs polish — they are separate items
|
||
the /runda surfaced or that emerged during the session, captured here
|
||
so they aren't lost.
|
||
|
||
- **Unquoted-identifier round-trip caveat.** The echo style (`render_*`
|
||
in `src/echo.rs`) emits bare identifiers — matching the existing
|
||
`render_create_table` aesthetic. A user-created column or table
|
||
named after a SQL keyword (e.g. `ORDER`, `SELECT`, `WHERE`) would
|
||
produce an echo whose round-trip is fragile. Pre-existing limitation
|
||
(the create-table echo had it from handoff-46); the §1 contract is
|
||
only formally violated for this edge case. If it ever bites a
|
||
learner, the fix is to add identifier quoting (probably backticks or
|
||
double-quotes per the advanced grammar's accepted spelling, plus a
|
||
conservative "needs quoting" predicate).
|
||
- **No full-spawn integration test for the echo pipeline.** Coverage
|
||
today is layered: pure renderers + runtime-helper unit tests (with
|
||
real `Database`) + App-level rendering tests. The unwired glue is
|
||
small (~3 lines per arm in `spawn_dsl_dispatch`). A Tier-3 test
|
||
that drives the full submit → runtime → App pipeline would complete
|
||
the picture; not blocking.
|
||
- **Pure-Command Schema commands double-compute echo.** `echo_for` is
|
||
called for every command, and the Schema arm then computes
|
||
`build_schema_echo` (whose catch-all redelegates to
|
||
`command_to_sql`). Minor inefficiency, no behaviour bug. Could be
|
||
optimized later; not worth the code complexity now.
|
||
- **ADR-0039 (EXPLAIN over advanced SQL)** — separate deferred
|
||
follow-up, not part of ADR-0038. Recorded 2026-05-27 (handoff-45/46);
|
||
pick up when prioritised.
|
||
|
||
## §8. 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 **2015 / 0 / 1**.
|
||
- **Keep docs lockstep**: ADR + `README.md` index + `requirements.md`.
|
||
This session's `5cb105b` was the corrective pass when the prior
|
||
per-phase commits left M4 / ADR-0038 status drifted.
|
||
- **Round-trip every catalogue row** (the §1 copy-paste contract; for
|
||
multi-statement, *per line*).
|
||
- **Pre-execution lookups** for any future Bucket-B-style additions
|
||
whose echo needs info the dropped/created thing erases. Mirror the
|
||
`collect_echo_lookups` shape (best-effort, fail-soft) and consume
|
||
in the post-execution arm.
|
||
|
||
## §9. How to take over
|
||
|
||
1. **Read, in order:** this file → **ADR-0038** (§4 styled-runs
|
||
reference is the relevant bit for the polish) → **ADR-0028**
|
||
(`OutputLine` styled-runs mechanism; `output_render::render_*` for
|
||
established patterns) → **`src/echo.rs`** (the renderer surface
|
||
itself is settled — but read to understand the catalogue shape if
|
||
you'll touch rendering) → **`src/app.rs`**'s `note_ok_summary` and
|
||
`handle_dsl_change_column_success` (the two consumers of the echo
|
||
+ caveat strings — these are what the polish replaces).
|
||
2. **Baseline:** `cargo test` (2015 / 0 / 1) + `cargo clippy
|
||
--all-targets -- -D warnings` (clean).
|
||
3. **If continuing on ADR-0038**: the polish is the only open item.
|
||
See §5. Decide the App-test assertion shape early (style-vs-text).
|
||
4. **If picking up something else**: ADR-0038 is done modulo the
|
||
polish; the §4 line of the ADR remains the formal outstanding
|
||
item but does not block other work. ADR-0039 (EXPLAIN over
|
||
advanced SQL) is a self-contained deferred follow-up if you want
|
||
the next feature.
|