docs: session handoff 53 — #15/#16 resolved, ADR-0022 Amendment 6

2026-05-31 16:28:42 +00:00
parent 6d8c9eea36
commit 1a93f0cd01
1 changed files with 252 additions and 0 deletions
@@ -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.