claude@clouddev1
290 lines
16 KiB
Markdown
290 lines
16 KiB
Markdown
# 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.
|