diff --git a/docs/handoff/20260514-handoff-8.md b/docs/handoff/20260514-handoff-8.md new file mode 100644 index 0000000..b4a1589 --- /dev/null +++ b/docs/handoff/20260514-handoff-8.md @@ -0,0 +1,521 @@ +# Session handoff — 2026-05-14 (8) + +Eighth handover. This session ran round 5 + round 6 of the +testing-and-feedback cycle plus a substantial architecture +discussion that produced **ADR-0023** (direction) and +**ADR-0024** (accepted execution plan). The next agent's +single most important job is to execute ADR-0024 — and is +expected to run it through **as non-interactively as +possible**: the design is fully settled, escalation is only +warranted for genuine ambiguity the ADR doesn't cover. + +## State at handoff + +**Branch:** `main`. Working tree clean. **Local HEAD is +`74c3ec1`**, ahead of `origin/main` by several commits +(check `git status` at session start to confirm). The user +pushes asynchronously; do not be blocked by unpushed state. + +Commits since handoff-7's baseline (`c247f55`), in +chronological order: + +``` +1e06490 round-5 follow-up: completion + i18n sweep +6ca2975 round-5 follow-up r2: migrate all thiserror Display attributes to catalog +a55b6a7 remove `q` quit alias +d6e1381 add ADR-0023 (proposed): unified declarative grammar tree +3b36bbb hint: replace misleading "null true false" suggestions at value slots +74c3ec1 add ADR-0024: unified grammar tree execution plan (accepted) +``` + +**Tests:** **777 passing, 0 failing, 1 ignored** (up from 760 +at handoff-7's baseline; +17 over this session). The ignored +test is still the same `\`\`\`ignore` doc-test in +`src/friendly/mod.rs` — not new debt. + +**Clippy:** clean with `nursery` lints enabled and +`-D warnings`. + +**Cargo.toml:** `thiserror` dependency removed (no longer +used after the r2 migration). `chumsky` still listed — Phase F +of ADR-0024 removes it. + +## What shipped this session (delta vs. handoff-7) + +### Round-5 testing follow-up (commit `1e06490`) + +Four user-reported gaps from round-4 testing addressed: + +- **Empty-prompt hint reworded** from `(no active hint)` to + `"Type a command — press Tab for options, \`help\` for a + list"`. At 80-col snapshot width it truncates after + `\`help\` for`; real terminals are wider and render fully. + 6 snapshots regenerated. +- **App-lifecycle commands folded into the DSL parser.** + Quit (and `q` alias, removed two commits later), help, + rebuild, save / save as, new, load, export, import, mode, + messages. 15 new keywords + catalog entries + `Command::App(AppCommand)` + AST variant with 11 variants. `dispatch_input` refactored + to parse-first — app commands work in both simple and + advanced modes. Pre-chumsky source-slice for `export ` + and `import [as ]` mirrors the replay + precedent. `UsageEntry` registry entries for all 11. `mode ` + / `messages ` use `try_map` for friendly + "unknown mode/messages" wording. +- **DSL completion gaps fixed.** `1:n` surfaces as a + composite candidate at `add `. `--all-rows`, + `--create-fk`, `--force-conversion`, `--dont-convert` + surface as new `CandidateKind::Flag` candidates (coloured + with `tok_flag` in the hint panel). `filter_clause`'s + `.labelled()` wrap removed so chumsky's expected-set + surfaces individual options. +- **Hardcoded user-facing strings migrated to catalog** + (Tier 1): the four parser custom errors (including the + "tables need at least one column" wart noted in + handoff-7), `UnknownType` Display, UI panel titles + (Output / Hint) + mode labels (SIMPLE / ADVANCED / + Advanced:), app.rs cascade rendering (action labels + + summary), runtime `--resume` CLI stderr, db.rs + change-column diagnostic tables (7 headers + 3 wrapper + summaries + force-conversion hint). + +### Round-5 r2: complete thiserror Display migration (commit `6ca2975`) + +All 12 remaining `#[error("...")]` Display attributes +across the codebase migrated to manual `Display` impls that +call `crate::t!()`. Twelve error types: `UnknownAction`, +`ParseError`, `ValueError`, `CsvError`, `PersistenceError`, +`YamlError`, `MigrateError`, `LockError`, `NamingError`, +`UserNameError`, `ProjectError`, `SafeDeleteError`, +`ArchiveError`, `ArgsError`, `DbError`. Roughly 54 user- +visible English strings now live in `en-US.yaml`. The +`thiserror` dependency was dropped from `Cargo.toml`. The +crate fully owns its error wording via the catalog now. + +### `q` removal (commit `a55b6a7`) + +The `q` alias for `quit` was introduced in round 5 as a peer +`Keyword` variant. Per the architectural critique that +became ADR-0023's framing, that shape was wrong — `q` +surfaced as a standalone command in completion (the only one +of its kind). Removed; if it comes back it'll be as an alias +annotation on the `Word("quit")` node per ADR-0024, not as a +peer keyword. + +### ADR-0023 proposed (commit `d6e1381`) + +Captured the architectural critique from the `q`-removal +session. Documented the "10-place edit" scatter problem, +the proposed direction (unified declarative grammar tree), +trade-offs, migration cost estimate, and the rationale for +deferring. Status was Proposed at this commit — the round-6 +design pass later moved it to "Accepted in direction, +superseded for execution by ADR-0024." + +### Round-6: value-literal hint fix (commit `3b36bbb`) + +The hint panel was surfacing `null` / `true` / `false` as +the candidate list at value-literal slots +(`insert into T values (`, `update T set col=`, +`where col=`, comma-separated value positions). Actively +misleading — the user is usually entering a number or +quoted text or date. Suppress those keyword candidates at +empty prefix; emit a prose hint with format guidance: +`"Type a value: number, 'text', true/false, null (dates +as 'YYYY-MM-DD', datetimes as 'YYYY-MM-DDTHH:MM:SS')"`. +Once the user starts typing a prefix (`n`, `tr`, `fa`), +normal keyword completion still applies. + +This is a stopgap; ADR-0024 makes the schema-aware version +(narrow to *only* the format for the actual column's type) +the natural shape. The stopgap is what's live today. + +### ADR-0024 accepted (commit `74c3ec1`) + +The full execution plan, 701 lines. Resolves all open +design questions from ADR-0023 across four rounds of +discussion. Two refinements beyond ADR-0023's sketch: + +- **Lexer dissolves entirely.** Walker operates on bytes + directly (scannerless). The current `dsl/lexer.rs` and + `dsl/keyword.rs` go away in Phase F. +- **Schema-aware parse from day one** (not phased). Typed + value slots reject mis-shaped input at parse time with + localised wording. Completion narrows per column type. + +ADR-0023 was renamed (`0023-proposed-unified-grammar-tree.md` +→ `0023-unified-grammar-tree.md`) and its status updated to +note the direction-accepted-but-superseded relationship +with ADR-0024. + +## Next move: execute ADR-0024 — **non-interactively** + +This is the next session's primary work. ADR-0024 specifies +the migration in full detail. The next agent should: + +1. **Read ADR-0024 carefully before doing anything else.** + It's the executable spec. Every design decision is + already made. +2. **Execute Phases A-F in sequence**, batching one commit + per phase. The user has explicitly said they want this + to run through with minimal back-and-forth — they have + limited time and the design is settled. +3. **Escalate only for genuine ambiguity** the ADR doesn't + cover. Examples of what to escalate: a command's + existing test reveals behaviour the ADR's spec doesn't + exactly match (and there's a choice to make about which + wins); a constraint turns out to be harder to express in + the trie than expected and the ADR's spec needs to flex; + error wording would change in a user-visible way that + wasn't explicitly approved. Examples of what NOT to + escalate: implementation details within the ADR's spec + (which file to put a helper in, naming choices, internal + function signatures), mechanical migration of one + command following the established pattern of earlier + commands, refactoring code that the migration touches in + passing. +4. **Per-phase test discipline:** + - Full test suite must be green before each commit. + - Walker-specific tests for trie-only features (schema- + aware narrowing, `WalkContext` propagation, dynamic + sub-grammar expansion, `HintMode` per-node) are added + as those features land. + - Differential test scaffolding (chumsky vs walker on + the existing input corpus, for migrated commands) + lands in Phase A and is removed in Phase F. + - `cargo clippy --all-targets -- -D warnings` clean + before each commit. +5. **Per-phase commit message** proposes batched scope and + reports test delta. The user will approve or redirect. +6. **The chumsky parser stays in place** as the fallback + router target until Phase F. Migrated commands route + through the walker; everything else falls through to + chumsky. This keeps every commit shippable. + +### Phase plan (from ADR-0024 §5) + +- **Phase A — walker scaffolding + app-lifecycle commands.** + Build the walker driver, `WalkContext`, `WalkOutcome`, + `MatchedPath`, terminal consume functions. Migrate quit, + help, rebuild, save, save as, new, load, export, import, + mode, messages. No schema dependency yet. +- **Phase B — DDL without value literals.** drop table / + column / relationship, rename column, add column (no + value), add 1:n relationship, change column. Schema + lookups via `Ident { source: Tables/Columns/Relationships }`. +- **Phase C — `create table`.** Exercises `Repeated` with + separator for the with-pk column list. +- **Phase D — data commands with full schema awareness.** + insert, update, delete, show data, show table. Full + `WalkContext` propagation. Dynamic sub-grammar + `column_value_list` reads schema and unfolds typed slots + per column. +- **Phase E — replay.** `BarePath` end-to-end. +- **Phase F — cleanup.** Delete `dsl/parser.rs`, + `dsl/lexer.rs`, `dsl/keyword.rs`, `dsl/ident_slot.rs`, + `dsl/usage.rs::REGISTRY`. Drop the `chumsky` dependency. + Simplify catalog (`parse.token.keyword.*` collapse to a + format helper). Remove differential test scaffolding. + +Estimated total: ~4 sessions. Phase A is the heaviest because +it builds the framework; B-D are migration-pattern work; E +is small; F is the cleanup pass. + +### When the user does need to be involved + +- **At phase boundaries.** Each phase ends with a commit + message proposal. The user reviews the scope + the + proposed message before the commit lands. +- **If the differential test catches a divergence** that + isn't a clear walker bug. E.g., the walker produces a + better error message than chumsky did at some input — + is the new wording acceptable? Ask. +- **If a behaviour spec in ADR-0024 turns out to be + underspecified** mid-implementation. Don't guess. +- **At Phase F before deleting modules.** Confirm the + user wants the final cleanup landing as one commit vs + several. Default: one commit. + +The user explicitly does NOT want involvement in: + +- Internal naming / module organisation decisions within + the ADR's spec. +- Per-command mechanical migration once the pattern is + established. +- Cosmetic test refactoring (e.g., test arrays that + hardcoded keyword lists can iterate the new registry — + this is a free win during migration). + +## Sharp edges to know about + +- **Scannerless walker.** Replaces both the lexer and + chumsky parser. Character-level helpers (identifier + shape, digit-shape, string-escape) live in a new + `src/dsl/walker/lex_helpers.rs`, internally similar to + the current lexer's logic but invoked per-position by + the walker rather than as a pre-pass. +- **Schema-aware parse from day one.** A DSL command that + references a non-existent table fails at parse time, not + at execute time as today. This matches the user's mental + model — but it does mean tests that exercised parser + behaviour in isolation (without setting up a schema + cache) may need updating. Most parser tests in + `src/dsl/parser.rs::tests` will move into the new + walker module; the input corpus stays the same. +- **`WalkContext` writes during walk.** `Ident { source: + Tables, writes_table: true }` writes `current_table` and + resolves `current_table_columns` from the schema cache + on match. Subsequent dynamic sub-grammars read these. + Test the propagation explicitly in Phase D. +- **`DynamicSubgrammar` expansion.** Returns owned `Node` + values at walk time. ADR-0024 sketches a `Box::leak` + tactic — alternative is a small per-walk arena. Either + works; pick at implementation time. Document the choice + in the Phase D commit. +- **`Optional` continuations** close the round-5 "save Tab + can't offer as" limitation automatically. Walker at + `save ` sees `Optional(Word("as"))` and offers `as`. + Same for `messages ` → `short` / `verbose`. Tests + removed in round 5 (because chumsky couldn't surface + them) can come back in Phase A. +- **Aliases on `Word` nodes.** `q` doesn't come back + during this migration. If the user later wants it (or + any other alias), it's one line: `aliases: &["q"]` on + the `quit` `Word` node. The walker handles it without + surfacing in completion candidates (matching the + round-5 user expectation). +- **`IdentSource::Types`.** Replaces the magic-string + `TYPE_SLOT_LABEL = "type"` constant in `completion.rs`. + Type names lex as identifiers in the scannerless walker + too; the `Types` source carries the closed-set + validation against `Type::all()`. +- **Path-bearing commands changed UX.** The user agreed + in round 6 to drop the "paths with spaces don't need + quoting" feature. `BarePath` becomes a routine + non-whitespace terminal. Paths with spaces use the + quoted form via `StringLit`. This UX change ships as + part of Phase E (replay) and Phase A (import / export); + the existing tests for these commands may need updating. +- **Help text stays hand-curated.** Grammar tree carries + `help_id` per node, but the help catalog body + (`help.cli_banner`, `help.in_app_body`) stays as prose. + Auto-generation was deferred. +- **The differential test scaffolding** lands in Phase A + as a test helper that runs both parsers on every input + in the existing corpus and asserts identical `Command` + output for migrated commands. Removed in Phase F. Don't + delete it earlier — it's the safety net. + +## Smaller items still on the table (deferred during this session) + +These remain valid feature work for after ADR-0024 lands: + +- **A1 (CI workflow).** Deferred indefinitely per the user + ("testing and dev is confined to one environment, little + value; also unsure of CI tech since GitHub may not be + the platform"). +- **I1 multi-line input.** Pure input handling, no grammar + interaction. Modest scope. After ADR-0024 lands, this + becomes attractive as a small UX win. +- **I1b readline shortcuts** (Ctrl-A/E, Ctrl-W/K/U). Same + shape as I1 — no grammar interaction. +- **Query DSL ADR + implementation.** The bigger design + piece. Adds substantial new commands; doing it before + ADR-0024 would be paying scatter cost we'd just have to + undo. After ADR-0024 lands, this becomes "one block per + new command" rather than seven-file scatter. +- **C3 constraint management.** UNIQUE / CHECK / NOT NULL + DDL. Same argument — wait for ADR-0024. +- **V-series UX projects.** V4 (session log + Markdown + export), V1/V2 (relationship rendering), V3 (ER diagram + export). All non-grammar; can land in parallel with + ADR-0024 if priorities shift. +- **C4 m:n convenience** (auto-junction-table). After + ADR-0024. +- **Settings persistence.** Feeds deferred `messages` + persistence and (when it arrives) any user-configurable + theme picker. +- **U-series undo/snapshot.** ADR-0006 designed; not yet + implemented. + +## ADR index (read these before touching the related areas) + +``` +0000 Record architecture decisions (process) +0001 Language and TUI framework (Rust + Ratatui) + — chumsky dependency dropped in ADR-0024 Phase F +0002 Database engine (User-facing posture) +0003 Input modes and command dispatch +0004 Project file format (amended by 0015) +0005 Column type vocabulary +0006 Undo snapshots and replay log (designed, not impl) +0007 Sharing and export (amended by 0015 amendment 1) +0008 Testing approach +0009 DSL command syntax conventions +0010 Database access via worker thread +0011 FK column type compatibility +0012 Internal metadata for user-facing column types +0013 Relationships, naming, and rebuild-table strategy +0014 Data operations, value literals, and auto-show +0015 Project storage runtime +0016 Pretty table rendering for data and structure views +0017 Column type-change compatibility +0018 Auto-fill contracts for serial and shortid columns +0019 Friendly error layer (H1) and i18n message catalog + — `parse.token.keyword.*` entries collapse in Phase F +0020 Tokenization layer for the DSL parser + — superseded by the scannerless walker in ADR-0024 +0021 Parser-as-source-of-truth for H1a + — usage info migrates from registry to grammar nodes + in ADR-0024 +0022 Ambient typing assistance (I3 + I4 unified) + — walker subsumes the expected-set introspection +0023 Unified declarative grammar tree (direction) + — superseded for execution by ADR-0024; institutional memory +0024 Unified grammar tree: execution plan (ACCEPTED) + — the spec to execute. Read first. +``` + +## Repository layout (delta vs. handoff-7) + +Files that will be created during ADR-0024 execution (none +exist yet at handoff): + +``` +src/dsl/ + grammar/ + mod.rs — Node enum, IdentSource, HintMode, + HighlightClass, MatchedPath, + CommandNode, REGISTRY top-level + data.rs — data commands (Phase D) + ddl.rs — DDL commands (Phases B + C) + app.rs — app-lifecycle commands (Phase A) + shared.rs — typed value slots, qualified_column, + where_clause, action_keyword, + column_value_list (dynamic) + validators.rs — integer_only_validator, + date_format_validator, etc. + walker/ + mod.rs — walk() entry, orchestration (Phase A) + driver.rs — per-node-kind dispatch (Phase A) + context.rs — WalkContext (Phase A) + outcome.rs — WalkOutcome, MatchedPath, WalkResult (Phase A) + lex_helpers.rs — identifier/digit/string-escape helpers (Phase A) +``` + +Files that will be deleted in Phase F: + +``` +src/dsl/parser.rs — chumsky combinators +src/dsl/lexer.rs — bytes → tokens pre-pass +src/dsl/keyword.rs — Keyword enum + macros +src/dsl/ident_slot.rs — IdentSlot enum (migrated to IdentSource) +src/dsl/usage.rs — REGISTRY (deleted) + matched_entry + (may go entirely if no consumers remain) +``` + +ADRs gained this session: + +``` +docs/adr/ + 0023-unified-grammar-tree.md — direction + 0024-unified-grammar-tree-execution-plan.md — accepted spec +``` + +(ADR-0023 was renamed from +`0023-proposed-unified-grammar-tree.md`.) + +## How to take over + +1. **Read this file.** +2. **Read `CLAUDE.md`** for the working-style rules. Note + the "Escalate ambiguity — do not decide for the user" + rule; in this case, ADR-0024 settles most ambiguities, + so the threshold for escalation is "the ADR doesn't + cover this question," not "I'm not sure which choice is + better." +3. **Read ADR-0024 carefully** + (`docs/adr/0024-unified-grammar-tree-execution-plan.md`). + This is the spec. Every design decision is in there. +4. **Read ADR-0023** for the institutional context — *why* + this refactor is happening. Helpful for understanding + when to question the design vs trust it. +5. **Skim ADR-0020 and ADR-0022** for what the walker + replaces. ADR-0020 documents the current lexer + token- + parser model; ADR-0022 documents the current completion + + highlighting infrastructure. Both inform what the + walker has to reproduce behaviourally. +6. **Run `cargo test`** to confirm the 777-test baseline. + Note: lib test count is 636; the rest are integration + + doctests. Total should be 777 passed, 0 failed, 1 + ignored. +7. **Run `cargo clippy --all-targets -- -D warnings`** to + confirm clean baseline. +8. **Start Phase A.** Build the walker scaffolding + + app-lifecycle commands. Each phase is one commit; the + commit message names which phase landed and the test + delta. +9. **Stay in flow.** The user wants this to run through + non-interactively. Escalate only for genuine ambiguity. + Per-phase commit reviews are the user touchpoint; + everything else proceeds autonomously. + +### End-to-end smoke test (current state, post-round-6) + +Replaces handoff-7's smoke test for what's user-visible. +Demonstrates that the existing chumsky-based architecture +still works correctly for everything the walker will eventually +replace. + +``` +$ rm -rf /tmp/handoff8-smoke +$ rdbms-playground --data-dir /tmp/handoff8-smoke + +# Inside the app — empty input. Hint panel shows: +# "Type a command — press Tab for options, `help` for a list" + +# Tab on empty input — cycles through entry keywords +# alphabetically (add, change, create, delete, drop, export, +# help, import, insert, load, messages, mode, new, quit, +# rebuild, rename, replay, save, show, update). + +# App commands work in both simple and advanced modes: +help -- shows in-app help +mode advanced -- switches mode +:quit -- one-shot escape to advanced, + then `quit` works there too + (advanced mode is still SQL- + placeholder; only the app- + commands work there). + +# Insert with value-literal hint. After `insert into T values (`, +# the hint reads: +# "Type a value: number, 'text', true/false, null +# (dates as 'YYYY-MM-DD', datetimes as 'YYYY-MM-DDTHH:MM:SS')" +# rather than the misleading "null true false" cycling list. +# Tab is a no-op at this position (the user types the value). +# But typing `n` and pressing Tab still completes to `null`. + +# Cycling at multi-candidate position: +add -- Tab inserts `column` + (multi-candidate, memo alive + with `column`, `1:n`) + -- input becomes `add 1:n` + -- commits as `add 1:n ` + +# Aliases removed: `q` no longer accepted (round-5 r3). +q -- parse error +quit -- works (canonical form) + +# All thiserror Display attributes go through the catalog now. +# Try a malformed CLI arg from outside the app to see: +$ rdbms-playground --bogus +rdbms-playground: unknown argument: --bogus + ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + this string is catalog-driven (cli.unknown_argument). +``` + +After ADR-0024 Phase A lands, the smoke test extends with +walker-specific observations (each migrated command's parse +goes through the trie); the user-visible behaviour shouldn't +change for any migrated command.