docs: session handoff 51 — #2/#17/#18 resolved, ADR-0022 Am3 + ADR-0036 Am2

docs/handoff/20260529-handoff-51.md

@@ -0,0 +1,289 @@
+# Session handoff — 2026-05-29 (51)
+
+Fifty-first handover. **Continuation of handoff-50's bug-fix loop.**
+The user steered this session twice with strategic judgement that
+proved correct both times (see §2). Three issues resolved — two bug
+fixes and one substantial cross-mode unification — plus two follow-up
+trackers spawned from #6's earlier /runda pass were tackled (#18
+closed benign-but-hardened, #17 fixed). **ADR-0022 gained Amendment 3;
+ADR-0036 gained Amendment 2.**
+
+## §1. State at handoff
+
+**Branch:** `main`. **HEAD `10e5197`.** **Tests: 2051 passing, 0
+failing, 0 unexpected skips, 1 pre-existing ignored** (the same
+ignored doctest as prior baselines). **Clippy: clean.** Nothing
+pushed (per the user's rules — push is the user's step).
+
+Commits since handoff-50's `abc9bb6`:
+
+```
+10e5197 feat: bring simple-mode insert arity diagnostics to parity with advanced
+7cccf4e refactor: make completion-drop gate schema-aware for consistency
+fa5d0dc fix: insert VALUES between-values hint points at comma not close-paren
+```
+
+Test count moved 2040 → 2051 (+11 net across the three commits).
+
+## §2. The two user steers that shaped the session
+
+This session is worth reading for **how the user's judgement
+redirected the work** — both calls were right and both uncovered real
+problems a naive pass would have missed.
+
+1. **"Most of the open issues were reported at the same time — the
+   fixes we already applied may have changed the outcome of other
+   reports."** This re-validation instinct was correct. Acting on it,
+   every issue picked up got a *confirm-it-still-reproduces* first
+   step. Result: **#2's reported root cause was wrong** (the report
+   blamed string literals; it was not literal-type-specific at all),
+   and **#18 turned out benign** (the divergence it described cannot
+   reach the code path it gates). Neither would have surfaced without
+   re-probing against current HEAD first. **This is now the standing
+   agreement: the first step of every bug pickup is to reproduce it on
+   current code before designing a fix.**
+
+2. **On #17, the user rejected the "easy way out."** When implementation
+   of the chosen Approach A revealed a larger blast radius, the lead
+   floated reverting to the lighter Approach B. The user pushed back:
+   *"this feels like mistakes were made in the past which resulted in
+   this discrepancy, and approach B is the easy way out … I'd rather
+   stick to A and get this sorted really cleanly — we're still in the
+   original dev phase, it'd be good to create a strong basis for future
+   maintenance, even if it means more work now."* This was **exactly
+   right**: there genuinely was a past discrepancy (issue #1's
+   simple-mode submit teaching note was a *workaround* for the very gap
+   #17 names), and Approach A — done properly — unified both modes onto
+   one model rather than piling a third bespoke surface on top.
+   Sticking with A also surfaced (and closed) a genuine
+   **execution-safety regression** that the lighter path would have left
+   buried.
+
+## §3. Resolved issues
+
+### #2 — VALUES between-values hint (commit `fa5d0dc`)
+
+**Re-characterised on investigation.** The report's hypothesis (per-
+column inference abandoned *when the first literal doesn't match the
+first column's type role*) was **wrong** — probing `values ('Oli'`,
+`values (42`, `values (3.5` all reproduced identically. Not literal-
+type-specific.
+
+**Real root cause:** the ambient-hint ladder's bottom rung
+(`ambient_hint_core_in_mode`, `src/input_render.rs`) parsed
+**schemalessly** (`parse_command_in_mode`) while every higher rung was
+already schema-aware. Type-blind, the grammar closes an `insert …
+values (…)` tuple after one value, so the "Next:" hint pointed at `)`
+when the schema-aware walk (knowing the remaining columns) expects `,`.
+The same schemaless parse also reported wrong-arity *closed* tuples as
+complete → the hint read "submit with Enter" for an input the real
+parse rejects.
+
+**Fix:** the fallback rung now calls
+`parse_command_with_schema_in_mode(input, cache, mode)`. One-line
+change (the cache was already in scope). Verified the friendly arity
+diagnostic is **not masked** — in advanced mode it fires at a higher
+rung and still wins (guard test
+`advanced_mode_wrong_arity_insert_keeps_friendly_diagnostic_over_fallback`).
+
+Three `typing_surface` snapshots re-baselined (form_a one-value
+`Next:` `)`→`,`; form_b too-few and form_c wrong-count: the misleading
+"Submit with Enter" → the accurate parse error). **ADR-0022
+Amendment 3** records the schema-aware fallback.
+
+### #18 — completion-drop schemaless gate (commit `7cccf4e`, closed benign-but-hardened)
+
+The ticket flagged `completion.rs:336`'s `input_parses_complete` gate
+using the schemaless parse (sibling of #2). **Investigated as the
+ticket asked: it's benign with the current grammar.** The gate drives
+exactly one thing — dropping a candidate that *exactly equals* the
+trailing partial (don't re-suggest `pk` at the end of `create table T
+with pk`). The schemaless/schema-aware parse *does* diverge (e.g.
+`insert into T values (1, 1, 'x')` — int into a bool column), but the
+divergence is confined to **value type/arity**, while the drop only
+ever removes a re-offered **keyword** == partial — and those positions
+never coincide. Probed every plausible co-occurrence; candidate output
+was identical with and without the fix.
+
+**Disposition (user chose "keep as hardening"):** applied the one-line
+schema-aware fix anyway (matches #2, strictly safe — can only ever
+*retain* a candidate the schemaless gate would have dropped). No
+behavioural test possible (no observable bug); the suite staying green
+is the regression cover. Rationale recorded in a code comment.
+
+### #17 — simple-mode arity diagnostic parity (commit `10e5197`)
+
+**The substantial one.** Simple-mode wrong-count inserts showed a bare
+`expected `,`/`)`` while advanced mode showed a friendly *"N column(s)
+… M value(s)"* message. Root cause: simple `insert` and advanced
+`insert` are different grammar nodes (ADR-0033 Am1). Advanced routes a
+wrong-count tuple to a type-blind fallback (`tuple_value_list`) so it
+**structurally matches** and `dml_insert_arity_diagnostics` fires; the
+simple DSL value list (`column_value_list`) built a fixed-length typed
+`Seq`, so a wrong-count tuple **Mismatched** and the diagnostic never
+ran.
+
+**Fix — unified onto the one ADR-0027 model (structural parse + ERROR
+diagnostic), advanced left byte-for-byte unchanged:**
+
+1. **Grammar:** new `dsl_insert_value_list` gate (`data.rs`) routes a
+   wrong-count DSL insert tuple to the shared `FALLBACK_VALUE_LIST` so
+   it matches and the diagnostic fires. **Gated to `Mode::Simple`** —
+   in advanced the DSL node stays strict (otherwise a non-SQL shape
+   like Form C `insert into T (1,2)` would spuriously match and be
+   accepted in advanced; `sql_insert.rs` owns advanced inserts).
+   `count_tuple_values` (moved to `shared.rs`) and a new
+   `insert_target_columns` (extracted from `column_value_list`) are now
+   **shared** by both grammars so the gate and the typed slots never
+   disagree.
+2. **Diagnostic is mode-aware:** `dml_insert_arity_diagnostics` gained a
+   `mode` param. Advanced Form B expects **all** columns (auto-fills
+   nothing); simple Form B/C expects the **user-fillable** columns
+   (`serial`/`shortid` auto-fill, ADR-0018 §3). It now also counts the
+   DSL Form A role (`insert_first_item`, distinct from SQL
+   `insert_column`) and scans the keyword-less Form C tuple. New
+   catalog keys `diagnostic.insert_arity_mismatch_form_b_simple` (names
+   the fillable *and* the auto-generated columns) and
+   `diagnostic.insert_arity_mismatch_all_auto`.
+3. **Submit safety (the regression that A surfaced):** a wrong-count
+   DSL insert now parses `Ok` + carries the ERROR diagnostic, so it
+   would otherwise have **dispatched and executed**. A unified Ok-arm
+   pre-flight (`dsl_insert_count_mismatch_notes`, `input_render.rs`)
+   now **blocks `ExecuteDsl`** and shows the teaching note — mirroring
+   the advanced Ok-arm pre-flight. The issue #1 Err-arm note retires.
+   `advanced_alternative_note`'s gate now reads the validity verdict
+   (not just `DefiniteErrorAt`) so the cross-mode pointer still fires
+   for the new parse-Ok-with-ERROR shape, but **only** when the line is
+   genuinely valid in advanced (Form B 3-val → pointer; Form C / too-few
+   → no pointer).
+
+**Scope guard (important for future readers):** this is *arity-
+diagnostic UX parity only*. It does **not** consolidate value-handling,
+execution, or `serial`/`shortid` auto-fill across modes — ADR-0036's
+deliberate mode-distinctness stands. The per-mode count difference is a
+*consequence* of the auto-fill difference, not a violation of it.
+
+**Documented as ADR-0036 Amendment 2** (cross-ref ADR-0033 §8.1) +
+README index + requirements.md H1a.
+
+## §4. ADR / docs work
+
+- **ADR-0022 Amendment 3** (in `fa5d0dc`): the ambient-hint fallback
+  rung is now schema-aware. Records that Amendment 1's listing of
+  `parse_command_in_mode` as the fallback was an oversight (every other
+  rung was schema-aware), and that the friendly arity diagnostic still
+  wins at its higher rung. README index updated same-edit.
+- **ADR-0036 Amendment 2** (in `10e5197`): simple-mode arity-diagnostic
+  parity (full detail in §3 #17). README index updated same-edit.
+- **requirements.md H1a** gained citations for both #2 and #17
+  contributions.
+- **#18** is a code-comment-only decision (benign hardening); no ADR.
+
+## §5. What's open
+
+### Bug reports still open (filed handoff-50)
+
+| # | Title | Label |
+|---|---|---|
+| [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 teal colour | bug |
+| [9](https://github.com/oliversturm/rdbms-playground/issues/9) | `[ok] explain <Table>` is terse; reconsider the `[ok]`/SQL-echo duplication | bug |
+| [10](https://github.com/oliversturm/rdbms-playground/issues/10) | Output panel: `[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 |
+| [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 + 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 |
+
+**Closed this session:** #2, #17, #18. **Spawned + closed within the
+session:** none new (#17/#18 were spawned in handoff-50 from #6's
+/runda pass).
+
+### Categorisation (carried from handoff-50, still current)
+
+- **Bounded code fixes:** #8 (identifier vs type colour), #13 (undo
+  dialog).
+- **Spec-heavy (discuss first):** #9 (`[ok] explain` redundancy), #10
+  (`[error]` tag colour — needs ADR-0037 amendment), #14 (`--resume`
+  restore mode — ties to ADR-0015 Iter 6).
+- **Substantial scope:** #7 (advanced `explain` — feature gap; ADR-0039
+  design agreed, impl deferred), #11 (clipboard copy), #12 (long-hint
+  overflow — design call: wrap vs cap vs resize). **Note:** #12 is
+  adjacent to this session's hint work — the field-value hints it cites
+  are exactly the arity / per-column prose touched in #2 and #17, which
+  are now *longer* (the #17 messages name multiple columns). #12 may be
+  more visible now; worth checking the hint-panel width handling when
+  picked up.
+- **Function-list cluster:** #15 + #16 — do **together** (shared
+  curated SQL-function-list infrastructure; both spawned from #6).
+
+### Other tracks (unchanged, 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 (this session
+  added the #2 + #17 contributions).
+- Tutorial / lesson system (needs its own ADR).
+- V4 session log + Markdown export; I1/I1b multi-line + readline;
+  I3 tab-completion polish; I4 syntax highlighting beyond input echo.
+- ADR-0039 "explain over advanced SQL" — design agreed, impl deferred
+  (this is issue #7).
+
+## §6. Process pins (carried forward, with this session's additions)
+
+- **NEW — reproduce-first.** Per the user's §2.1 steer: the first step
+  of every bug pickup is to **confirm it still reproduces on current
+  HEAD** before designing a fix. Several handoff-50 reports were filed
+  in one batch; earlier fixes may have changed or eliminated others.
+- **NEW — no easy-way-out on architectural discrepancies.** Per §2.2:
+  when implementation reveals a fix touches a *past discrepancy* (a
+  workaround papering over a deeper gap), prefer the unifying fix that
+  removes the discrepancy over a lighter patch that deepens it — the
+  project is still in its original dev phase and a strong basis is
+  worth the extra work. Escalate the trade-off; don't silently take the
+  lighter path.
+- **Confirm every commit** (propose message, wait). **No AI
+  attribution.** **No issue numbers in commit messages** (issue bodies
+  + close comments may reference freely).
+- **Test-first**; green + clippy-clean is the only acceptable end
+  state; current baseline **2051 / 0 / 1**.
+- **Follow CLAUDE.md solo-mode phases** explicitly. `/runda` is the
+  standard DA tool between/after phases on non-trivial work — this
+  session ran it over the #2 fix and the #17 *design* (the design pass
+  caught the DSL-Form-A-role mismatch and surfaced the message-wording
+  fork before any code was written). The second-round DA discipline
+  (the `/runda` re-prompt) caught real issues each time.
+- **cargo-insta is NOT installed** as a subcommand. Accept pending
+  snapshots by `mv -f <file>.snap.new <file>.snap` (review the diff
+  first — these are behaviour changes to verify, not rubber-stamp).
+- **GitHub issue tracking** (`gh` CLI) is in use; migrating to Gitea
+  (`tea`) later.
+
+## §7. How to take over
+
+1. **Read, in order:** this file → `requirements.md` for open tracks →
+   `docs/adr/README.md` for the ADR index → the relevant ADR for
+   whatever you pick up. For insert/value-list work, **ADR-0036
+   (esp. Amendment 2)** and **ADR-0033 §8.1** are now the authority on
+   the arity-diagnostic model — both modes share it.
+2. **Baseline:** `cargo test` (2051 / 0 / 1) + `cargo clippy
+   --all-targets` (clean).
+3. **For a bug fix:** **reproduce it first on current HEAD** (process
+   pin above). The `panic!()` probe-test technique (write a throwaway
+   test that prints the actual behaviour, run, observe, delete) was the
+   fast path all session — see how #2 and #17 were investigated. Then
+   failing test → fix → green → /runda → commit-with-approval →
+   close-with-summary.
+4. **Pick the next ticket** from §5's categorisation. #8 or #13 are the
+   bounded quick wins. #15 + #16 should be done together. If picking a
+   hint-related ticket (#12), note this session lengthened the
+   field-value hint strings.
+5. **Architectural note for future insert work:** the insert value-list
+   path now has a clean shared spine — `count_tuple_values` and
+   `insert_target_columns` in `grammar::shared`, consumed by both the
+   DSL gate (`data.rs::dsl_insert_value_list`, simple-only) and the SQL
+   gate (`sql_insert.rs::tuple_value_list`). The arity diagnostic
+   (`dml_insert_arity_diagnostics`) is mode-aware. Keep that symmetry
+   if you extend either side.