253 lines
13 KiB
Markdown
253 lines
13 KiB
Markdown
# 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.
|