diff --git a/docs/handoff/20260519-handoff-20.md b/docs/handoff/20260519-handoff-20.md new file mode 100644 index 0000000..3a731ff --- /dev/null +++ b/docs/handoff/20260519-handoff-20.md @@ -0,0 +1,255 @@ +# Session handoff — 2026-05-19 (20) + +Twentieth handover. **Interim** handoff: this session +finished ADR-0027 (§1), fixed two manual-testing bugs (§1), +and **started ADR-0028** — query plans / `explain`. ADR-0028 +**step 1 of 5 is done and committed**; steps 2–5 are planned +in full detail (§3) and not yet built. A fresh session can +implement them from this file + the ADR without re-exploring. + +## State at handoff + +**Branch:** `main`. Working tree clean. **12 commits** since +handoff-18 (`39b92a7`), all local — push asynchronously, not +blocking. + +``` + docs: handoff 20 — ADR-0028 step 1 done, 2-5 planned +03d8a09 ui: styled-output-line mechanism (ADR-0028 step 1) +a1e4932 docs: handoff 19 update — both manual-testing bugs fixed +3a40ae2 runtime: don't record an unmodified temp as the --resume target +f239ca5 walker: keep optional trailing flags completable after `--` +0e5f226 docs: handoff 19 — ADR-0027 highlight/hint wiring finished +c1c9f6c runtime: extract the indicator debounce into a tested state machine +400fb71 ui: surface diagnostics in the ambient hint panel (ADR-0027 §2) +bbfb70c ui: overlay diagnostic spans on the input field (ADR-0027 §2) +437b2f2 walker: flag LIKE on a numeric column (ADR-0027 Amendment 1) +3912fb5 walker: precise per-literal spans for expression WARNINGs +426e801 command: Operand carries a source span +``` + +**Tests:** **1133 passing, 0 failing, 1 ignored** (`cargo +test`). The ignored test is the long-standing `` ```ignore `` +doc-test in `src/friendly/mod.rs`. Typing-surface matrix: +**161 cells**, unchanged. + +**Clippy:** clean (`cargo clippy --all-targets -- -D +warnings`, nursery group). + +## §1. Done before ADR-0028 — see handoff-19 + +This session first **finished ADR-0027** (the §2 highlight / +hint wiring + precise WARNING spans + the `LIKE`-on-numeric +Amendment 1 + the debounce state machine) and **fixed two +manual-testing bugs** (optional trailing-flag completion after +`--`; the `--resume` unpersisted-temp pointer). All of that is +documented in **handoff-19** — read it for that context. This +file is about ADR-0028. + +## §2. ADR-0028 step 1 — done (`03d8a09`) + +ADR-0028's build-order step 1: the **styled-output-line +mechanism**. `OutputLine` gained an optional +`styled_runs: Option>` payload; +`render_output_line` (`src/ui.rs`) gained a branch that, when +the payload is present, renders the line span-by-span — each +`OutputSpan { byte_range, class }`'s semantic +`OutputStyleClass` (`Neutral` / `Efficient` / `Expensive` / +`AutomaticIndex`) resolved to a theme colour by +`output_span_style`. The echo path is untouched; a line with +no payload still renders whole-line by `kind`. + +`theme.plan_efficient` was added — a green distinct from +`theme.system` (ADR-0028 §6); `theme.warning` is reused for +"expensive". `OutputLine::styled(...)` is the constructor for +payload-bearing lines. + +No user-visible change yet — nothing produces styled lines. +Two `ui` tests cover the new branch and the no-payload +fallback. + +## §3. ADR-0028 steps 2–5 — the plan + +Read `docs/adr/0028-query-plans.md` first — it is the spec. +Below is the validated build plan with file:line anchors, so +no re-exploration is needed. + +### Three gotchas found during exploration + +1. **`Subgrammar` needs `&'static Node`; the query shapes are + `const`.** `SHOW_DATA` / `UPDATE_SHAPE` / `DELETE_SHAPE` + (`src/dsl/grammar/data.rs:68,300,311`) are `const`. A + `const` cannot be referenced as `&'static`; and promoting + them to `static` breaks `const SHOW_CHOICES` + (`data.rs:76`), which embeds `SHOW_DATA` *by value*. + **Fix:** add three thin wrapper `static`s over the existing + `_NODES` slices — + `static EXPLAIN_SHOW_DATA: Node = Node::Seq(SHOW_DATA_NODES);` + etc. — and have `explain` reference *those*. A `static` may + read a `const`, and a `const` array may take `&` of a + `static`. No churn to the existing shapes. +2. **`build_show` is positional.** `build_show` + (`data.rs:347`) reads `words.nth(1)` to tell `data` from + `table` — index 1 assumes `show` is the entry word at + index 0. Under `explain` the indices shift. **Fix:** + extract a role-based `build_show_data(path)` helper (it + only needs `require_ident(path,"table_name")`, + `build_show_filter`, `build_show_limit` — all already + position-independent); `build_show`'s `data` arm calls it. + `build_update` / `build_delete` are already role-based, so + they need nothing. +3. **Steps 2 and 3 must land in one commit.** Adding the + `Command::Explain` enum variant breaks every exhaustive + `match Command` — notably `execute_command_typed` + (`src/runtime.rs:1685`). So the grammar/AST (ADR step 2) + and the worker path (ADR step 3) cannot be separate green + commits. Combine them: one commit landing `explain` + end-to-end with a **plain** tree render; step 4 then adds + the styled / annotated tree. + +### Step 2 + 3 (one commit) — `explain` end to end + +**Grammar — `src/dsl/grammar/data.rs`:** +- Add `static EXPLAIN_SHOW_DATA / EXPLAIN_UPDATE / + EXPLAIN_DELETE = Node::Seq(SHOW_DATA_NODES / UPDATE_NODES / + DELETE_NODES)`. +- `const EXPLAIN_SHOW_NODES = &[Word("show"), + Subgrammar(&EXPLAIN_SHOW_DATA)]`; likewise update / delete. + `const EXPLAIN_CHOICES = &[Seq(EXPLAIN_SHOW_NODES), …]`; + `const EXPLAIN_SHAPE = Node::Choice(EXPLAIN_CHOICES)`. The + surface is `explain show data …` / `explain update …` / + `explain delete from …` (ADR §1 — the inner entry word is + part of the surface). +- Extract `build_show_data(path)`; have `build_show`'s + `Some("data")` arm call it. +- `build_explain(path)`: the words are `[explain, show|update| + delete, …]`; `words.nth(1)` discriminates → call + `build_show_data` / `build_update` / `build_delete`, box the + result: `Command::Explain { query: Box::new(inner) }`. +- `pub static EXPLAIN: CommandNode { entry: + Word::keyword("explain"), shape: EXPLAIN_SHAPE, ast_builder: + build_explain, help_id: None, usage_ids: + &["parse.usage.explain"] }`. + +**Registry:** add `&data::EXPLAIN` to `REGISTRY` +(`src/dsl/grammar/mod.rs:466`). `"explain"` is a free entry +word. + +**Catalog:** add `parse.usage.explain` to +`src/friendly/strings/en-US.yaml` (the `parse: usage:` +section) **and** `("parse.usage.explain", &[])` to +`src/friendly/keys.rs` `KEYS_AND_PLACEHOLDERS` (sorted) — the +`keys_validate_against_catalog` test enforces this. + +**Command AST — `src/dsl/command.rs`:** +- `Command::Explain { query: Box }`. +- `verb()` (`:442`, exhaustive) — add `Self::Explain { .. } => + "explain"`. +- `target_table()` (`:481`, exhaustive) — add `Self::Explain + { query } => query.target_table()`. +- `display_subject()` (`:530`) has a `_` arm — no change + needed. + +**DB worker — `src/db.rs`:** +- New `ExplainRow { id: i64, parent: i64, detail: String }` + and `QueryPlan { display_sql: String, rows: Vec }`. +- `Request::ExplainPlan { query: Command, reply: + oneshot::Sender> }` (`Request` at + `:409`); dispatch arm in `handle_request`. +- `Database::explain_query_plan(query: Command) -> + Result` async method (mirror the + existing `oneshot` request methods). +- **Separate SQL construction from execution.** Extract + `build_query_data_sql` / `build_update_sql` / + `build_delete_sql` returning `(String, Vec)` + from `do_query_data` (`:4730`) / `do_update` (`:4539`) / + `do_delete` (`:4642`) — the SQL string + params are already + fully assembled before the first `conn.prepare` / + `execute_with_fk_enrichment` in each, so the cut is clean. +- `do_explain_plan(conn, query)` — match the inner command, + call the shared SQL builder, run `EXPLAIN QUERY PLAN ` + with the params bound (template: `read_table_indexes`, + `:3305` — the same multi-row read shape), collect + `ExplainRow`s. Also build the **display SQL** (ADR §3): + standard SQL with double-quoted idents (`quote_ident`, + `:1713` — make it `pub(crate)` or duplicate), WHERE literals + *inline* (a `compile_operand` variant that renders the + literal instead of pushing a `?` param), `<>` for + inequality (`compare_op_sql` already emits `<>`), and the + implicit `ORDER BY ` that `limit` adds. + +**Result flow:** +- `CommandOutcome::QueryPlan(QueryPlan)` + (`src/runtime.rs:1490`). +- `AppEvent::DslExplainSucceeded { command: Command, plan: + QueryPlan }` (`src/event.rs:17`). +- `execute_command_typed` (`runtime.rs:1685`) arm: + `Command::Explain { query } => + database.explain_query_plan(*query).await + .map(CommandOutcome::QueryPlan)`. +- `spawn_dsl_dispatch` (`runtime.rs:1129`) — map + `CommandOutcome::QueryPlan` → `AppEvent::DslExplainSucceeded`. +- `App::update` — handle `DslExplainSucceeded`: for this + commit render a **plain** tree (indented `detail` strings + + the display SQL line), pushed as `OutputKind::System` + lines. Step 4 swaps in the styled renderer. + +### Step 4 (one commit) — `render_explain_plan` + taxonomy + +- `render_explain_plan` in `src/output_render.rs` (alongside + `render_structure` / `render_data_table`). Builds the + box-drawing tree from `id` / `parent`; node text is the + `detail` string **verbatim**; the display SQL is shown above + the tree; the command echo precedes it. +- Annotation taxonomy (ADR §4): a small substring-pattern + table classifying each `detail` into Full scan / Index + search / Covering index / PK lookup / Automatic index / + Temp B-tree / Neutral. Unmatched → Neutral. +- The renderer emits `OutputLine`s with `styled_runs: + Some(...)` — only the category-bearing keywords of each + `detail` get an `OutputStyleClass` (Efficient → green, + Expensive → amber, AutomaticIndex → amber + bold + a + distinct marker per ADR §6); connectors and names stay + Neutral. The mechanism for this is **already built** + (step 1). +- `App::update`'s `DslExplainSucceeded` handler switches from + the plain render to `render_explain_plan`. + +### Step 5 (one commit) — matrix + verification + +- Typing-surface matrix cells for `explain` in + `tests/typing_surface/` (parse / completion / hint of + `explain show data …` etc.). +- Full suite green vs. the 1133 baseline; clippy clean; + matrix-snapshot discipline. +- Final handoff (handoff-21). + +## §4. What's next + +1. **ADR-0028 steps 2–5** per §3 — `explain` is the only + active feature in flight. +2. After ADR-0028: the handoff-16 design trio is then fully + implemented. Other open clusters unchanged from + handoff-16/17/18/19 (snapshot/undo `U`-series; constraints + `C3`; `C4` m:n; `C3a`; `C1`; `H1`; `SD1`; `TT5` CI; `V4`; + `I1`; `TU1`). Prioritisation is a user decision — ask. + +## §5. How to take over + +1. **Read this file, then `docs/adr/0028-query-plans.md`** + (the spec), then **handoff-19** (ADR-0027 + the two bugs). +2. **Read `CLAUDE.md`** — working-style rules. +3. **Run `cargo test`** — 1133 passing, 0 failing, 1 ignored. +4. **Run `cargo clippy --all-targets -- -D warnings`** — clean. +5. **Implement ADR-0028 §3** — step 2+3 as one commit, then + step 4, then step 5. The file:line anchors in §3 are from + this session's exploration; the three gotchas are the + non-obvious parts. + +### Note on the typing-surface matrix + +`tests/typing_surface/` is **161 cells**. After the `explain` +grammar lands (step 2+3), add cells for it (step 5) and apply +the matrix-snapshot discipline from handoff-17/18/19: a +failing cell with *correct* new behaviour → update its +snapshot; with *wrong* behaviour → the cell earned its keep.