docs: session handoff 52 — #8/#13/#12/#7/#9 resolved, ADR-0039 impl + ADR-0040

docs/handoff/20260530-handoff-52.md

@@ -0,0 +1,253 @@
+# Session handoff — 2026-05-30 (52)
+
+Fifty-second handover. **A five-issue UI/UX bug-and-feature run** off
+handoff-51's open list (#7–#16). Closed **#8, #13, #12, #7, #9** —
+two bounded fixes, one feature implementation (advanced-mode
+`explain`), and one first-principles output-convention redesign. Two
+ADRs touched: **ADR-0039 implemented**, **ADR-0040 authored**; plus
+**ADR-0022 Amendment 4**.
+
+The session is worth reading for **process**: every substantial change
+went reproduce-first → design (often via the Plan agent) → `/runda` on
+the design → test-first build → **live-app verification in a real TUI**
+→ `/runda` on the implementation → commit → close. The live-app and
+implementation-`/runda` passes each caught real defects a green test
+suite had not (see §3).
+
+## §1. State at handoff
+
+**Branch:** `main`. **HEAD `8311de4`.** **Ahead of `origin/main` by
+1** — only #9's commit (`8311de4`) is unpushed; #8/#13/#12/#7 were
+pushed by the user between issues. **Tests: 2089 passing / 0 failing /
+1 ignored** (the same pre-existing ignored doctest as prior baselines).
+**Clippy: clean.** Push is the user's step.
+
+Commits since handoff-51's `46a3128`:
+
+```
+8311de4 feat: replace the [ok] summary line with a ✓/✗ echo marker      (#9)
+f62ccce feat: support explain over advanced-mode SQL queries            (#7)
+f7ca288 fix: grow the hint panel for long prose hints                   (#12)
+5ea69db fix: widen undo dialog and polish its summary line              (#13)
+d20f765 feat: give column data types a dedicated syntax-highlight colour (#8)
+```
+
+Test count moved 2051 → 2089 (+38) across the five commits.
+
+## §2. Two process habits that paid off (read this)
+
+1. **Live-app verification catches what tests miss.** Driving the real
+   binary in a tmux pane (per the standing agreement) surfaced things
+   green tests didn't: it confirmed #7's plan-tree rendering end-to-end
+   and validated #9's `✓/✗` across success/failure/skip/parse-error in
+   one pass. **Caveat (recurring):** the login shell is fish, which
+   rejects `VAR=value cmd` and bash-isms; **send `exec bash` to the
+   pane first**, then launch with `--data-dir <tmpdir> --log-file …`.
+   Isolate with `tmux -L <name>` + a `mktemp -d` data dir; tear down
+   after.
+
+2. **`/runda` on the *implementation*, not just the design.** Both #7
+   and #9 had a design `/runda`; running it **again on the finished
+   diff** caught real holes each time — #7 was missing `explain insert`
+   execution coverage and had a duplicate-`help` entry; #9's
+   `PersistenceFatal` path left a dangling `running:` echo. None failed
+   a test. Treat the implementation `/runda` as mandatory for
+   substantial work.
+
+## §3. Resolved issues
+
+### #8 — type keywords get a dedicated highlight colour (`d20f765`)
+
+Root cause: `Node::Ident.highlight_override` **and** the `Word`
+struct's `highlight_override` were both **dead** — the walker driver
+discarded the Ident's and `walk_word` hardcoded `Keyword`. So column
+types (`int`, `serial`) rendered identical to identifiers. Wired both
+through; added `HighlightClass::Type` + a dedicated `tok_type` theme
+colour (pink/deep-magenta, distinct from keyword-purple and
+identifier-teal — user chose a dedicated colour over reusing the
+keyword colour). The three `IdentSource::Types` slots + the two-word
+`double precision` alias (via a new `Word::type_keyword` constructor)
+opt in. **ADR-0022 Amendment 4.** The `double precision` thread also
+clarified that the advanced-mode SQL type *aliases* (`integer`,
+`varchar`, `float`, … → the ten playground types) live in **ADR-0035
+§3** (restating ADR-0030 §5) — they are accepted input spellings, not
+members of the type set.
+
+### #13 — undo dialog width + language polish (`5ea69db`)
+
+Dialog capped at 60 cols and wrapped even a short insert; lowercase
+`snapshot`/`yes`/`no`; raw ISO timestamp. Now: content-aware width
+(`undo_dialog_width`, 34–100, never exceeds area); capitalised
+`Snapshot`/`Yes`/`No`; **local-time, human-formatted timestamp**
+(`24 May 2026, 11:00`) via a **new `chrono` dependency** (user chose
+the real date crate; `clock` feature only, no `unstable-locales`).
+`Esc cancel` was already inline (resolved in a prior session — no
+change). Reproduce-first confirmed each sub-item.
+
+### #12 — hint panel grows for long prose hints (`f7ca288`)
+
+The Hint panel was a fixed one row; long **prose** hints (insert
+field-value hints, `parse.usage.*`) wrapped but were clipped — the
+useful tail hidden. (The candidate list already scrolled horizontally.)
+Now: dynamic height 1→**3 rows** (user's cap, "see how it lands"),
+reclaimed when short, with an **ellipsis backstop** on row 3.
+`resolve_hint_lines` pre-wraps; `render_right_column` sizes the panel.
+**ADR-0022 Amendment 5.** Also **shortened** the standout offender,
+`parse.usage.sql_create_table` (299 chars → terse one-liner; full
+grammar stays in `help.ddl.sql_create_table`) — measured all hint
+strings first and the user chose to shorten that one.
+
+### #7 — `explain` over advanced-mode SQL (`f62ccce`, ADR-0039 implemented)
+
+`explain` now wraps `select` / `with` (CTE) / `insert` / `update` /
+`delete` in advanced mode, reusing the ADR-0028 plan tree.
+**Mechanism:** a second `Advanced` `EXPLAIN_SQL` CommandNode under the
+shared `explain` entry word, reusing the established `insert`/`update`/
+`delete` shared-word dispatch (`decide`: SQL-first, DSL-fallback) — no
+new grammar machinery. **Rejected** a `DynamicSubgrammar` mode-gate
+(its resolution cache key omits `mode`, so it would serve stale
+wrong-mode nodes — a real bug the Plan agent found).
+`build_explain_sql` slices the inner SQL off the source (excludes the
+`explain` prefix) and reuses the existing SQL builders; `do_explain_
+plan` runs the carried text verbatim, no params. Advanced
+`explain update`/`delete` now route through SQL (identical plan, full
+SQL syntax); DSL-explain tests pinned to simple mode. WITH/CTE included
+(it's a `Select`, in scope). The implementation `/runda` added 2
+`explain insert` execution tests + a REGISTRY `help_id`-uniqueness
+invariant test, and fixed a duplicate-`help` entry + stale usage/help
+text.
+
+### #9 — `[ok]` summary line → `✓/✗` echo marker (`8311de4`, ADR-0040)
+
+**The substantial one.** Began as "`[ok] explain` is terse" (and
+post-#7, `explain select` rendered `[ok] explain` with an *empty*
+subject). The user steered it to a first-principles audit: the
+`[ok] <verb> <subject>` line **duplicated** the echo line above it on
+every command; its only unique signal was success-vs-error.
+
+**Decision (ADR-0040):** retire the `[ok]` summary line and the
+symmetric `"<verb> <subject>" failed:` prefix. A command's echo line
+resolves `running: <input>` → `<input> ✓` (success) / `<input> ✗`
+(runtime failure) on completion; the failure reason still renders
+beneath (`[error]`-tagged). Content lines unchanged.
+
+**Mechanics:** `EchoStatus { Pending, Ok, Err }` on the echo
+`OutputLine`; executed commands push `Pending` (via `OutputLine::echo`)
+and the result event resolves the **oldest** pending echo
+(`mark_oldest_pending_echo`) — correct under interleaving because the
+db worker is FIFO. **Scope:** executed commands only (success/runtime-
+failure/skip/replay); **parse-time and pre-flight rejections keep their
+`running:` + caret rendering** (they never run, and their caret is
+pinned to the `running:` prefix). **App-command `[ok]` lines**
+(`rebuild`/`export`/`now editing`/`replay — N`) are payload-bearing,
+have no echo to mark, and stay as-is. `ok.summary` retired from catalog
++ keys; `dsl.failed` reduced to `{rendered}`.
+
+Implementation `/runda` caught the `PersistenceFatal` arm leaving a
+dangling `running:` echo (now marks `✗` before the quit banner).
+
+## §4. ADR / docs work
+
+- **ADR-0022 Amendment 4** (`d20f765`): dedicated type highlight class;
+  both dead `highlight_override` fields wired through. README index +
+  requirements.md I4.
+- **ADR-0022 Amendment 5** (`f7ca288`): dynamic hint-panel height.
+  README index + requirements.md QA1-area.
+- **ADR-0039 status → implemented** (`f62ccce`): + an Implementation
+  section recording the two-CommandNode mechanism and the rejected
+  `DynamicSubgrammar`. README index, requirements.md QA1, CLAUDE.md.
+- **ADR-0040 — new** (`8311de4`): the completion-marker decision, the
+  audit, scope, and an Implementation-notes section. README index,
+  requirements.md (ADR-0038 echo line note), CLAUDE.md untouched (no
+  `[ok]` reference there).
+
+## §5. What's open
+
+### Bug reports still open (filed handoff-50)
+
+| # | Title | Label |
+|---|---|---|
+| [10](https://github.com/oliversturm/rdbms-playground/issues/10) | `[error]` tag colour identical to `[system]` (ADR-0037 gap) | enhancement |
+| [11](https://github.com/oliversturm/rdbms-playground/issues/11) | Copy output panel contents to the system clipboard | enhancement |
+| [14](https://github.com/oliversturm/rdbms-playground/issues/14) | `--resume` should restore the last-used input mode | enhancement |
+| [15](https://github.com/oliversturm/rdbms-playground/issues/15) | Tab completion: offer common SQL function names | enhancement |
+| [16](https://github.com/oliversturm/rdbms-playground/issues/16) | Restore typing-time column-typo hint via known-function list | enhancement |
+
+**Closed this session:** #8, #13, #12, #7, #9.
+
+### Categorisation / notes for pickup
+
+- **#10 (`[error]`/`[system]` tag colour)** is now the most adjacent:
+  ADR-0040 kept the `[error]` tag on failure-reason lines, so the
+  colour collision is still live and pairs naturally with the marker
+  work. Needs an **ADR-0037 amendment** (it owns output tag styling).
+- **#15 + #16** are a pair — do **together** (shared curated
+  SQL-function-list infrastructure; both spawned from #6).
+- **#14** ties to ADR-0015 Iter 6 (`--resume` + persistent input
+  history).
+- **#11** clipboard copy — self-contained.
+
+### Other tracks (from requirements.md)
+
+- Track 2 project storage Iter 5/6 — export/import + `--resume` +
+  persistent input history + migration framework.
+- C3a (modify relationship), C4 (m:n convenience).
+- H1 friendly DB-error layer (partial); tutorial/lesson system (needs
+  its own ADR); V4 session log + Markdown export; I1/I1b multi-line +
+  readline; I3 tab-completion polish; I4 highlighting beyond input echo.
+
+## §6. Process pins (carried forward + this session's reinforcements)
+
+- **Reproduce-first.** Confirm every bug still reproduces on current
+  HEAD before designing a fix (handoff-51 §2.1). Held all session.
+- **`/runda` on BOTH design and implementation** for substantial work
+  (this session's §2.2). Each impl pass found a real defect.
+- **Live-app verification** for user-facing changes — drive the real
+  TUI (tmux, `exec bash` first, isolated `--data-dir`). It finds things
+  tests miss.
+- **No easy-way-out on architectural discrepancies** (handoff-51 §2.2):
+  prefer the unifying fix. #8 unified both dead `highlight_override`
+  fields; #7 reused the shared-word dispatch rather than bolting on a
+  `DynamicSubgrammar`; #9 redesigned the convention rather than patch
+  one verb.
+- **Escalate genuine design forks** — this session asked the user on
+  every real fork (type colour, date-crate scope, hint mechanism +
+  cap, WITH-in-explain, marker scope, attribution robustness). The
+  AskUserQuestion previews were useful for visual choices.
+- **Confirm every commit** (propose message, wait). **No AI
+  attribution. No issue numbers in commit messages** (issue bodies +
+  close comments reference freely). **Push is the user's step**; the
+  user pushed between issues and then said "close" — close-with-summary
+  after that.
+- **cargo-insta is NOT installed** as a subcommand. Accept pending
+  snapshots with `mv -f <f>.snap.new <f>.snap` after reviewing the diff.
+- **GitHub issues** via `gh` CLI (Gitea/`tea` migration still later).
+- Baseline to beat: **2089 / 0 / 1**, clippy clean.
+
+## §7. How to take over
+
+1. **Read, in order:** this file → `docs/requirements.md` (open
+   tracks) → `docs/adr/README.md` → the relevant ADR. For output
+   conventions, **ADR-0040** is now the authority on the `✓/✗` marker;
+   **ADR-0037** owns the `[error]`/`[system]` tag colours (#10's home).
+   For `explain`, **ADR-0039** (implemented) + **ADR-0028**.
+2. **Baseline:** `cargo test` (2089 / 0 / 1) + `cargo clippy
+   --all-targets` (clean).
+3. **For a bug fix:** reproduce on current HEAD first (probe-test or
+   live app), then failing test → fix → green → `/runda` (design and,
+   for substantial work, implementation) → live-verify if user-facing →
+   commit-with-approval → user pushes → close-with-summary.
+4. **Pick next from §5.** #10 is the natural follow-on to #9 (tag
+   colours, ADR-0037 amendment). #15+#16 go together.
+5. **Architectural notes for future readers:**
+   - **Echo lifecycle (ADR-0040):** echo lines carry `EchoStatus`;
+     `mark_oldest_pending_echo` resolves them on result events. Any new
+     command-result event arm MUST mark its echo (success→Ok,
+     failure→Err, skip→Ok) or it strands a `running:` line. The
+     renderer's Echo branch + the height calc both key on status.
+   - **`explain` (ADR-0039):** `EXPLAIN_SQL` is a second `Advanced`
+     CommandNode on the shared `explain` word; `build_explain_sql`
+     slices the inner SQL and reuses the SQL builders. Keep that.
+   - **Highlight overrides (ADR-0022 Am4):** both `Node::Ident` and
+     `Word` honour `highlight_override` now; `Word::type_keyword` is the
+     typed-keyword constructor.