diff --git a/docs/handoff/20260528-handoff-50.md b/docs/handoff/20260528-handoff-50.md new file mode 100644 index 0000000..c05c48a --- /dev/null +++ b/docs/handoff/20260528-handoff-50.md @@ -0,0 +1,293 @@ +# Session handoff — 2026-05-28 (50) + +Fiftieth handover. **A bug-report triage + fix session** — different +shape from the recent feature-development handovers. The user surfaced +fourteen distinct issues from a single session of using the app; the +session triaged all of them into GitHub issues, then resolved six +clusters. Three commits, three closed issues (six original tickets +folded into them), two enhancement trackers spawned for follow-up +work outside bug-fix scope. **ADR-0033 gained Amendment 5.** + +## §1. State at handoff + +**Branch:** `main`. **HEAD `24c2685`.** **Tests: 2040 passing, 0 +failing, 0 unexpected skips, 1 pre-existing ignored** (the same +ignored doctest as previous baselines). **Clippy: clean.** + +Commits since handoff-49's `9468324`: + +``` +24c2685 fix: SQL function-call names not flagged as columns +6f87ad1 fix: advanced CREATE TABLE completion cluster +c12ed1d fix: INSERT Form B value-count UX (ADR-0033 Amendment 5) +``` + +Test count moved 2020 → 2040 (+20 across all three commits). + +## §2. The shape of this session + +This session was a **bug-report intake + triage + fix** loop. The +user dumped a list of fourteen distinct observations from a session +of actually using the app. The session: + +1. Confirmed via `gh` CLI that GitHub issue tracking is available + on `oliversturm/rdbms-playground` (the project intends to migrate + to Gitea eventually, but `gh` works for now). +2. **Investigated each observation** before filing — grounded every + ticket body in concrete file:line references the explore agent + surfaced or that direct grep confirmed. +3. **Filed all fourteen as GitHub issues** with sensible grouping: + - Some bundled (undo dialog width + undo dialog wording → one + ticket). + - Some split (the three CREATE TABLE completion bugs → three + separate tickets because each closes cleanly on its own). + - Some recategorised (the `[error]` vs `[system]` tag colour + was on the fence between "documented design" and "real defect"; + filed as enhancement after user clarification). +4. **Picked tickets in clusters of related code paths** to amortise + the exploration cost. +5. **Followed CLAUDE.md solo-mode discipline** at each step: + requirements check → failing test first → implement → green + + clippy → DA pass (often via `/runda`) → commit-with-approval → + close-with-summary → enhancement spinoffs for adjacent ideas. + +### User-preferred commit-message style — pinned + +The user explicitly **does not want issue numbers in commit messages** +(the project will migrate trackers; numbers won't point at anything +later). Issue numbers in **issue bodies and close-comments** are fine +(those migrate with the tracker). Cross-referencing issues inside the +tracker is encouraged. + +## §3. Closed issues + +### #1 — INSERT Form B value-count UX (commit `c12ed1d`) + +The user's original bug case: `insert into Customers values('Oli', +52, 3)` against a 4-column table (`id serial PK, Name text, Age int, +SerNo serial`) failed in simple mode AND in advanced mode, and the +cross-mode pointer ("valid as SQL in advanced mode") fired +misleadingly. Three layered fixes plus **ADR-0033 Amendment 5**: + +- **Walker diagnostic** (`dml_insert_arity_diagnostics`): the + function's own doc-comment recorded the Form B (no-column-list) + branch as "deferred". This commit closed that gap — Form B + mismatches now emit a new `diagnostic.insert_arity_mismatch_form_b` + ERROR per offending tuple, keyed off the schema cache's column + count. The `[ERR]` validity indicator (ADR-0027) now lights up at + typing time for the user's reported scenario. +- **Cross-mode pointer gate** (`advanced_alternative_note` in + `src/input_render.rs`): refactored from a hand-rolled count check + to a single `input_verdict_in_mode(input, schema, Mode::Advanced)` + call. The pointer fires only when the verdict is `None` — the + ADR-0027 sense of "valid". Any future static check added to the + verdict pipeline participates automatically. +- **Teaching notes** for the submit path: + - simple-mode submit: `insert.form_b_extra_values_note` covers + under-, in-window, and over-supply. + - advanced-mode dispatch pre-flight: + `insert.form_b_positional_count_mismatch_note`. + +The `advanced_mode.also_valid_sql` pointer was reworded to *"trying +to write SQL? switch with `mode advanced`, or prefix `:` to run +once"* (one insta snapshot regenerated). **ADR-0033 Amendment 5** +records the gate semantics with an explicit definition of "valid". +`docs/requirements.md` H1a cites the three new pedagogical strings. + +The /runda round on this issue surfaced and fixed a sub-issue: my +initial gate was Form-B-specific, but the user pointed out that +validity-verdict-driven is the principled answer. That led to the +walker diagnostic (so the verdict picks up the mismatch) + the +broader gate refactor. + +### #3 + #4 + #5 — Advanced CREATE TABLE completion cluster (commit `6f87ad1`) + +Three bugs in the same advanced-mode grammar + walker path: + +- **#3:** `create table T ` (trailing space, advanced) offered only + `with` — `(` for the SQL column-def list was missing. Root cause: + the shared-entry-word completion merge in + `completion_probe_in_mode` only fired at the entry-word boundary. + Fix: broadened to fire at any cursor depth + handle + `Expectation::Punct` continuations alongside `Word`/`Literal`. +- **#4:** `create table T (` listed only the table-level constraint + keywords (`primary`, `unique`, `check`, `constraint`, `foreign`) — + the column-name role was invisible because `COLUMN_DEF` starts + with an `Ident::NewName` slot that produces no concrete candidate. + Fix: added a new **`HintMode::IntroProse(&'static str)`** variant + that surfaces catalog prose at slot entry without suppressing Tab + completion (unlike `ProseOnly`) and without requiring + `typing_name_at_cursor` to fire (unlike `ForceProse`). Wrapped + `ELEMENT` in `Node::Hinted { mode: IntroProse( + "hint.create_table_element"), … }`. The prose reads *"Type a + column name, or a table-level constraint: `primary`, `unique`, + `check`, `constraint`, `foreign`"*. +- **#5:** `create table T (count` and `create table T (count ` both + leaked the bare keyword `double` (the first token of the dedicated + `double precision` Choice branch per ADR-0035 §6.3) alongside the + regular type list. Fix: added `("double", "double precision")` to + `COMPOSITE_CANDIDATES` + extended the keyword filter to drop + composite openers. The partial-typing prose case is subsumed by + #4's IntroProse (user reads "Type a column name…" while + mid-typing, then advances to the clean type list). + +**Worth flagging for future readers:** the new `HintMode::IntroProse` +variant is an additive extension to the ADR-0024 HintMode-per-node +model. No behaviour change to existing modes. **No ADR amendment +written** — the variant is currently used at one slot and the +extension is straightforward. If a second use case lands, an ADR-0024 +amendment recording the variant alongside `ProseOnly` / `ForceProse` +would be the right move. + +### #6 — SQL function-call names not flagged as columns (commit `24c2685`) + +`select sum(Age) from Customers` runs cleanly at the engine but the +validator was treating `sum` as a column reference and flagging "no +such column `sum` on table `Customers`". Two layered fixes for the +same bug class: + +1. **Walker** (`schema_existence_diagnostics`): the bare-column check + on `sql_expr_ident` items now skips when the ident is immediately + followed by `(` — it's a function-call name, not a column + reference. New helper `is_followed_by_call_args` mirrors the + existing `is_followed_by_qualified_ref` guard. Cascades to the + `[ERR]` indicator, the red highlight overlay, and the diagnostic- + driven ambient hint. +2. **Typing-time** (`invalid_ident_at_cursor_in_mode`): early-return + at `sql_expr_ident` positions. The partial could resolve to either + a column reference or a function-call name; without lookahead for + a trailing `(` we can't tell. Submit-time still catches genuine + column typos via the schema-existence diagnostic and + `pick_hint_diagnostic`. + +The scope question in the original ticket body (which aggregate set +to enumerate?) turned out to be moot — the fix is **all function +calls**, matching ADR-0031 §1's stated posture (*"the grammar admits +the call shape structurally; it does not know which names are +aggregates"*). No grammar change; no ADR amendment. + +**Trade-off worth knowing:** typing `select Agx` (no FROM yet) is +now silent until FROM is added; previously the typing-time path +flagged it as "No such column". This makes typing-time **consistent** +with submit-time — the schema-existence pass already silently skipped +no-FROM expressions ("no FROM in scope — engine catches"). For any +expression-with-scope (SELECT with FROM, WHERE, etc.) the +diagnostic-driven hint still fires for column typos. Pinned by the +test `genuine_column_typo_in_complete_select_still_hints_via_diagnostic`. + +Two **enhancement trackers spawned** from the /runda DA pass on this +issue — see §5 below. + +## §4. ADR work + +**ADR-0033 Amendment 5** (2026-05-28, in `c12ed1d`): +`advanced_mode.also_valid_sql` (Amendment 3's cross-mode pointer) +fires on **validity**, not just **parse** — *"valid"* meaning +`input_verdict_in_mode(input, schema, Mode::Advanced) == None` in +the ADR-0027 sense. The amendment text spells out the exact +condition so future readers can't drift back to a syntactic-only +reading. Index entry in `docs/adr/README.md` updated. The user +explicitly asked for the precise definition to be inlined, and +agreed with the term **"valid"** (over "clean", "fully valid", etc.) +because it ties cleanly to ADR-0027's existing vocabulary. + +No other ADRs touched. ADR-0024's `HintMode` got a new variant +(`IntroProse`) without an amendment — flagged in §3 above for the +next session to consider if a second use case lands. + +`docs/requirements.md` H1a (strong syntax-help in parse errors) got +a citation line listing the three new pedagogical strings +(`insert.form_b_extra_values_note`, +`insert.form_b_positional_count_mismatch_note`, +`diagnostic.insert_arity_mismatch_form_b`) as concrete contributions. + +## §5. What's open + +### Bug reports filed this session, still open + +| # | Title | Label | +|---|---|---| +| [2](https://github.com/oliversturm/rdbms-playground/issues/2) | VALUES-list completion stops giving useful hints once the first value is a string literal | bug | +| [7](https://github.com/oliversturm/rdbms-playground/issues/7) | Advanced mode: `explain` not yet supported | enhancement | +| [8](https://github.com/oliversturm/rdbms-playground/issues/8) | Advanced-mode syntax highlighting: identifiers and type keywords share the same teal color | bug | +| [9](https://github.com/oliversturm/rdbms-playground/issues/9) | `[ok] explain ` is terse; reconsider whether `[ok]` line duplicates the SQL-echo line above it | bug | +| [10](https://github.com/oliversturm/rdbms-playground/issues/10) | Output panel: `[error]` tag color identical to `[system]` — gap in ADR-0037 | enhancement | +| [11](https://github.com/oliversturm/rdbms-playground/issues/11) | Copy output panel contents to the system clipboard | enhancement | +| [12](https://github.com/oliversturm/rdbms-playground/issues/12) | Long input hints overflow horizontally | bug | +| [13](https://github.com/oliversturm/rdbms-playground/issues/13) | Undo confirmation dialog: too narrow and language polish | bug | +| [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 in expression positions | enhancement | +| [16](https://github.com/oliversturm/rdbms-playground/issues/16) | Restore typing-time column-typo hint for SQL expressions via known-function list | enhancement | + +#15 and #16 were spawned from #6's /runda pass — both want a curated +SQL-function list as a single source of truth; they should be +tackled together. + +### Categorisation (for next session's triage) + +- **Spec-heavy (need discussion before code):** #9 (`[ok] explain` + redundancy), #10 (`[error]` tag colour — needs ADR-0037 + amendment), #14 (`--resume` restore last mode — ties to ADR-0015 + Iter 6). +- **Bounded code fixes:** #2 (VALUES completion regression), #8 + (identifier vs type colour), #13 (undo dialog), #7 (advanced + `explain` — feature gap, more substantial). +- **Substantial scope:** #11 (clipboard copy — user named it ~80% + bug-report-friction reduction), #12 (long hint overflow — design + call: wrap vs cap vs resize). +- **Function-list cluster:** #15 + #16 (do together). + +### Other tracks (unchanged from handoff-49, still pending) + +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), H1a syntax-help in parse + errors (this session added the INSERT Form B contributions). +- Tutorial / lesson system (needs its own ADR). +- V4 session log + Markdown export. +- I1 / I1b multi-line + readline shortcuts, I3 tab completion + polish, I4 syntax highlighting beyond input echo. +- ADR-0039 "explain over advanced SQL" — design agreed, + implementation deferred. + +## §6. Process pins (carried forward) + +- **Confirm every commit** (propose message, wait). **No AI + attribution.** **No issue numbers in commit messages** (this + session's specific user preference — issue bodies and close + comments may freely reference issue numbers). +- **Test-first**; green + clippy-clean is the only acceptable end + state; current baseline **2040 / 0 / 1**. +- **Follow CLAUDE.md solo-mode phases** explicitly — requirements + extraction → divergent exploration → evaluation → execution → + verification. Each phase produces written artifacts. /runda is + the standard tool for the DA pass between/after phases on + non-trivial work; in this session every closed issue went through + at least one /runda round before committing. +- **Round-trip every catalogue row** (the ADR-0038 §1 contract; for + multi-statement, per line) — unchanged from prior handovers. +- **GitHub issue tracking is in use for now**, migrating to Gitea + later. `gh` CLI works. `tea` CLI is the future-state per + CLAUDE.md's Gitea section. + +## §7. How to take over + +1. **Read, in order:** this file → `requirements.md` for the open + tracks → `docs/adr/README.md` for the ADR index → the relevant + ADR for whatever feature/bug you pick up. +2. **Baseline:** `cargo test` (2040 / 0 / 1) + `cargo clippy + --all-targets` (clean). +3. **For a bug fix from the open list:** start with the issue body, + then probe the actual behaviour (the explore agent + targeted + `panic!()` probe-tests in the relevant module are the fast path — + see `simple_mode_submit_of_form_b_extra_value_teaches_serial_skip` + in `src/app.rs` for a clean template). Then failing test → fix → + green → /runda → commit-with-approval → close-with-summary. +4. **For a fresh feature:** write the ADR (or extend an existing + one), run `/runda` over the design before building, then build + test-first. +5. **Pick the next ticket** by checking §5's categorisation. If + tackling #15 / #16, do them together — they share the + known-function-list infrastructure.