claude@clouddev1
c1fcf28e04
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.
256 lines
11 KiB
Markdown
256 lines
11 KiB
Markdown
# 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.
|
||
|
||
```
|
||
<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 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<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 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.
|