diff --git a/docs/handoff/20260530-handoff-52.md b/docs/handoff/20260530-handoff-52.md new file mode 100644 index 0000000..3339c69 --- /dev/null +++ b/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 --log-file …`. + Isolate with `tmux -L ` + 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] ` 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 `" " failed:` prefix. A command's echo line +resolves `running: ` → ` ✓` (success) / ` ✗` +(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 .snap.new .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.