From 46a31284c5ebbf2e61f0a8fdf235811cc5b33760 Mon Sep 17 00:00:00 2001 From: "claude@clouddev1" Date: Fri, 29 May 2026 21:00:18 +0000 Subject: [PATCH] =?UTF-8?q?docs:=20session=20handoff=2051=20=E2=80=94=20#2?= =?UTF-8?q?/#17/#18=20resolved,=20ADR-0022=20Am3=20+=20ADR-0036=20Am2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/handoff/20260529-handoff-51.md | 289 ++++++++++++++++++++++++++++ 1 file changed, 289 insertions(+) create mode 100644 docs/handoff/20260529-handoff-51.md diff --git a/docs/handoff/20260529-handoff-51.md b/docs/handoff/20260529-handoff-51.md new file mode 100644 index 0000000..a63bcfd --- /dev/null +++ b/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 ` 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 .snap.new .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.