diff --git a/docs/handoff/20260531-handoff-53.md b/docs/handoff/20260531-handoff-53.md new file mode 100644 index 0000000..42f2be9 --- /dev/null +++ b/docs/handoff/20260531-handoff-53.md @@ -0,0 +1,252 @@ +# Session handoff — 2026-05-31 (53) + +Fifty-third handover. **A two-issue pair off handoff-52's open list.** +Closed **#15 + #16** in a single commit — they share one piece of +infrastructure (a curated SQL-function-name list), exactly as +handoff-52 §5 predicted ("do **together**"). One ADR amendment +authored: **ADR-0022 Amendment 6**, plus an ADR-0031 status note. + +The session is worth reading for **one process save**: the +implementation `/runda` pass (DA hat) found a **real duplicate-candidate +defect** that the green test suite had not — a column named like a +function (`count`, `date`) was listed twice in the completion line. It +also caught a clippy regression I'd introduced and a whitespace/EOF +wart. None of these were visible in "tests pass." See §2. + +## §1. State at handoff + +**Branch:** `main`. **HEAD `6d8c9ee`.** **Ahead of `origin/main` by +2** — handoff-52's `8311de4` (#9) was still unpushed at the start of +this session, and `6d8c9ee` (this session) is on top. **Tests: 2107 +passing / 0 failing / 1 ignored** (full `cargo test`; the lib-only +figure is **1538** — see the baseline note below). The 1 ignored is +the same long-standing `` ```ignore `` doctest in `src/friendly/mod.rs` +as every prior baseline. **Clippy: clean** (nursery group). Push is the +user's step. + +Commit since handoff-52's `8311de4`: + +``` +6d8c9ee feat: curated SQL function list — Tab completion (#15) + typing-time typo hint (#16) +``` + +**Test-baseline bookkeeping (read once).** `requirements.md`'s "Test +baseline" had drifted: it still read **1131** (the ADR-0027-era lib +figure) because the intervening issue fixes #8/#13/#12/#7/#9 landed +tests without bumping it — the doc historically moves the number at ADR +milestones, not per issue. With the user's confirmation this session +**refreshed it to 1538 lib / 2107 full**, with an Amendment-6 entry +appended to the running list and a note explaining the prior drift. +Going forward the two figures are: **`cargo test --lib` = 1538**, +**`cargo test` (all binaries) = 2107**. + +## §2. The process save that paid off (read this) + +Both issues went: read issue bodies + the ADRs they name → trace the +exact symbols (`completion.rs`, `sql_expr.rs`, the lockdown tests) → +**escalate the genuine design choices to the user** (function set, +colour, ADR-or-not) → test-first build → **`/runda` on the +implementation** → fix findings red-first → commit. + +The implementation `/runda` (DA hat) is where the value was. Three +findings, none visible in a green suite: + +1. **Duplicate-candidate defect (real).** A column named exactly like a + function — a column called `count` or `date`, both plausible — was + listed **twice** in the completion line (`count/Identifier` + + `count/Function`), distinguishable only by colour. The existing code + deduped identifiers-vs-keywords but I had added no + functions-vs-identifiers dedup. **Fixed:** `functions.retain(|f| + !identifiers.iter().any(|i| i.eq_ignore_ascii_case(f)))` — the + column wins (same spirit as the keyword-wins rule), case-insensitive + so a `Count` column also suppresses the `count` function. Proven + **red-first**: the new test fails without the retain, printing the + exact `[count/Identifier, count/Function]` duplication. +2. **#16 fix not yet proven load-bearing.** Reverted the `invalid_ident` + change and confirmed the typo test **FAILs** without it — the fix is + doing the work, not coinciding with another path. +3. **Clippy regression + EOF whitespace** I'd introduced (a + `field_reassign_with_default` in the new test, and stray trailing + blank lines from a throwaway-probe removal). Both fixed; EOF + normalised to be **byte-identical to HEAD** rather than sneaking in a + cleanup of the pre-existing trailing-blank convention. + +Lesson reinforced (matches handoff-52 §2.2): **`/runda` on the +implementation, not just the design.** A green suite is necessary, not +sufficient. + +## §3. Resolved issues + +### #15 + #16 — curated SQL function list (`6d8c9ee`, ADR-0022 Amendment 6) + +Both hinge on the same gap: the advanced-mode SQL expression grammar +(ADR-0031 §1) admits a function-call *shape* at its `sql_expr_ident` +slot but, by design, does not know which names are real functions (the +slot is `IdentSource::Columns`, optimised for the column-reference +common case; the walker is a structural matcher, not an evaluator). So +both issues wanted the same thing — *one source of truth for "what SQL +function names does this playground recognise."* + +**New file — `src/dsl/sql_functions.rs`:** + +- `KNOWN_SQL_FUNCTIONS` — 23 names, **sorted + lowercase** (a pinned + invariant). The **broader curated set** (user choice): aggregates + (`count`/`sum`/`avg`/`min`/`max`), common scalars + (`length`/`upper`/`lower`/`trim`/`substr`/`coalesce`/`abs`/`round`), + and a broader scalar tier + (`date`/`datetime`/`strftime`/`hex`/`ifnull`/`nullif`/`replace`/ + `instr`/`typeof`/`random`). +- `is_known_function_prefix(partial)` — case-insensitive prefix test, + empty prefix matches all. +- **`cast` deliberately excluded** — a faithfulness finding during + build: SQLite's `CAST(expr AS type)` is **not** a plain `name(args)` + call the expression grammar parses (`expr AS type` is not a valid + argument expression), so offering `cast` would surface a candidate + that cannot complete. Documented + test-locked (`cast_is_excluded`, + `cast_is_not_offered_as_a_function_candidate`). + +**#15 — Tab completion** (`completion.rs`, "Source 1.8"): at a +`sql_expr_ident` slot the curated functions are offered as candidates +(prefix-filtered like every other source), ordered after keywords/types +(a learner reads clause keywords first, then discovers callables). New +`CandidateKind::Function` + a ninth `Theme` colour `tok_function` (a +blue, distinct from keyword purple / identifier teal / type pink-magenta +in both `dark()` and `light()`) — parallel to how #8 gave types +`tok_type`. `ui.rs` maps the new kind. + +**#16 — typing-time typo hint restored** (`completion.rs`, +`invalid_ident_at_cursor`): the issue-#6 fix had dropped the "no such +column" flag *wholesale* at `sql_expr_ident` slots (to stop the false +positive on names like `sum`). Now it bails **only** when the partial +prefix-matches a known function; otherwise it falls through to the +existing schema-column check. So `select Agx` (no FROM yet — schema +diagnostic silent) warns at typing time again, while `select sum` stays +quiet. The issue-#6 lockdown tests +(`genuine_column_typo_in_complete_select_…`, +`advanced_select_partial_function_name_not_flagged_…`) still pass, and +the submit-time `unknown_column` diagnostic path is untouched. + +**Dedup** (the DA finding, §2): a column named like a function appears +once; the column wins. + +**Coverage added** (15 tests): +`sql_functions::{list_is_sorted_and_lowercase, list_has_no_duplicates, +cast_is_excluded, prefix_match_is_case_insensitive, +empty_prefix_matches_all, unknown_prefix_does_not_match}`; +`completion::{sql_expr_slot_offers_known_function_candidates, +projection_slot_offers_known_function_candidates, +sql_function_candidates_filter_by_prefix, +sql_function_candidates_carry_function_kind, +function_candidates_absent_at_non_expression_slots, +function_candidate_deduped_against_a_like_named_column, +cast_is_not_offered_as_a_function_candidate, +invalid_ident_fires_for_genuine_typo_at_sql_expr_slot, +invalid_ident_does_not_fire_for_function_prefix_at_sql_expr_slot, +invalid_ident_does_not_fire_for_column_prefix_at_sql_expr_slot}`; +`input_render::advanced_select_genuine_column_typo_before_from_warns_at_typing_time`; +`theme::function_colour_is_distinct_from_keyword_identifier_and_type`. + +**Mode-gating confirmed sound** (DA probe): DSL where-expressions use +role `expr_column`; only SQL uses `sql_expr_ident`. Functions therefore +**never leak into simple mode**. Slot coverage probed empirically — +functions surface at `WHERE`, comparison RHS, `HAVING`, projection list, +and post-`AND`. + +## §4. ADR / docs work + +- **ADR-0022 Amendment 6 — new** (`6d8c9ee`): the curated function + list, the `CandidateKind::Function` + `tok_function` colour, the #16 + invalid-ident narrowing, the dedup, and the **no-validation-allowlist + posture** (the list drives completion + the typo hint **only**, never + parse-time acceptance — an unknown function still parses and surfaces + an engine-neutral execution error, preserving ADR-0031 §6/§7). + Coverage list inline. +- **ADR-0031 status note** (`6d8c9ee`): records that Amendment 6 layers + the known-function list *on top of* the `sql_expr_ident` slot — + grammar unchanged, no-allowlist posture intact; softens §5's + "function names are not completed" line to "completed from a curated + pedagogical list, not a validation allowlist." +- **README index** (`6d8c9ee`): ADR-0022 entry gains the Amendment-6 + paragraph; ADR-0031 entry gains the status-note reference. +- **requirements.md** (`6d8c9ee`): I3 (tab-completion) + I4 + (highlighting) refinement notes for #15; **test baseline refreshed** + (see §1). + +## §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 | + +**Closed this session:** #15, #16 (commit `6d8c9ee` — **close them on +GitHub once pushed**). + +### Categorisation / notes for pickup + +- **#10 (`[error]`/`[system]` tag colour)** is the most adjacent and + was already flagged "most adjacent" in handoff-52: ADR-0040 kept the + `[error]` tag on failure-reason lines, so the colour collision is + still live. Needs an **ADR-0037 amendment** (it owns output tag + styling). Smallest of the three; a clean next pick. +- **#14** ties to ADR-0015 Iter 6 (`--resume` + persistent input + history); small, builds on the existing `last_project` machinery. +- **#11** clipboard copy — self-contained but pulls in a new + cross-platform clipboard crate (most surface area of the three). + +### 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 (this session advanced it for SQL + function names); I4 highlighting beyond input echo. + +## §6. Process pins (carried forward + this session's reinforcement) + +- **`/runda` on the implementation, not just the design.** This + session's save (§2): the impl pass found a real duplicate-candidate + defect, a clippy regression, and an EOF whitespace wart — none + visible in a green suite. Held from handoff-52 §2.2. +- **Red-first proof for every fix.** Each fix this session was proven + by reverting it and watching the relevant test FAIL before + restoring. (Global rule: reproduce bugs with a failing test first.) +- **Escalate genuine design choices — do not decide for the user.** + The function set, the candidate colour, and whether to write an ADR + were all surfaced via `AskUserQuestion` before building; the stale + test-baseline refresh was also confirmed, not assumed. +- **Faithfulness over convenience.** `cast` looks like an obvious + member of "common scalars" but does not fit the call-shape grammar — + excluded with a documented reason + a lock test, rather than shipped + as a candidate that fails to complete. +- **Match HEAD, don't reformat unrelated things.** The EOF fix restored + the file's pre-existing trailing-blank convention byte-for-byte + rather than "tidying" it — keeps the diff to the actual change (the + user's append-only / no-gratuitous-reformat git philosophy). +- **Reproduce-first** (handoff-51 §2.1) and **no easy-way-out on + architectural discrepancies** (handoff-51 §2.2): both still stand. +- **Push is the user's step.** Commits and pushes are decoupled; + unpushed commits are a normal working state. Two are unpushed now + (`8311de4`, `6d8c9ee`). + +## §7. How to take over + +1. Read this note, then `CLAUDE.md`, then `docs/requirements.md` + (note the refreshed baseline), then `docs/adr/README.md`. +2. The codebase is on `main` at `6d8c9ee`, working tree clean, **2 commits + ahead of origin** (both unpushed — see §1). +3. If continuing the bug-list run: **#10** is the smallest and most + adjacent next pick (needs an ADR-0037 amendment for output tag + styling); then **#14**, then **#11**. +4. The tooling note from earlier this session: an intermittent + empty-output bug in a newer Claude Code build was the cause of the + "Bash/Read returning nothing" symptom — the user reverted to a + working build mid-session. If output goes blank again, suspect the + harness, not the repo. +5. Honour the process pins in §6 — `/runda` on implementation + + red-first proof are the two that earned their keep this session.