docs: handoff 20 — ADR-0028 step 1 done, steps 2-5 planned

Interim handoff. ADR-0028 (query plans / explain) is started:
step 1 (the styled-output-line mechanism, 03d8a09) is done and
committed. handoff-20 carries the full validated build plan
for steps 2-5 with file:line anchors and three implementation
gotchas (the const/static Subgrammar wrinkle, build_show's
positional dispatch, and why steps 2+3 must land as one
commit) so a fresh session implements them without
re-exploring.
This commit is contained in:
claude@clouddev1
2026-05-19 11:47:15 +00:00
parent 03d8a09457
commit c1fcf28e04
+255
View File
@@ -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 25 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.
```
<this file> 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<Vec<OutputSpan>>` 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 25 — 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<Command> }`.
- `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<ExplainRow> }`.
- `Request::ExplainPlan { query: Command, reply:
oneshot::Sender<Result<QueryPlan, DbError>> }` (`Request` at
`:409`); dispatch arm in `handle_request`.
- `Database::explain_query_plan(query: Command) ->
Result<QueryPlan, DbError>` 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<rusqlite Value>)`
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 <sql>`
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 <pk>` 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 25** 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.