From 124c1d33e9cbfbb03df0aa2055f2de3aea088c31 Mon Sep 17 00:00:00 2001 From: "claude@clouddev1" Date: Fri, 15 May 2026 17:53:14 +0000 Subject: [PATCH] add handoff-11: ADR-0024 Phase F/D fully landed; HintMode/Ranker/round-5 closed --- docs/handoff/20260515-handoff-11.md | 442 ++++++++++++++++++++++++++++ 1 file changed, 442 insertions(+) create mode 100644 docs/handoff/20260515-handoff-11.md diff --git a/docs/handoff/20260515-handoff-11.md b/docs/handoff/20260515-handoff-11.md new file mode 100644 index 0000000..e8db0b4 --- /dev/null +++ b/docs/handoff/20260515-handoff-11.md @@ -0,0 +1,442 @@ +# Session handoff — 2026-05-15 (11) + +Eleventh handover. Continues from handoff-10. This session +completed every deferred item from handoffs 9 + 10: **ADR-0024 +Phase F (full) step 5 (walker-driven completion), the Ranker +hook scaffolding, the HintMode dispatch layer, Phase D (full) +end-to-end schema-aware value typing, and the round-5 +optional-suffix completion gap.** Six commits total (five +feature commits + this handoff). + +The codebase now reaches the steady-state ADR-0024 envisioned: +one declaration per command, walker as single source of truth, +typed value slots rejecting mis-shaped input at parse time. + +## State at handoff + +**Branch:** `main`. Working tree clean. **Local HEAD is +`8188fa5`**, ahead of `origin/main` by ten commits across two +sessions (user pushes asynchronously). + +Commits since handoff-10's baseline (`044173b`): + +``` +bbe1252 ADR-0024 Phase F (full) step 5: walker-driven completion +7ae1a0f ADR-0024 ranker hook scaffolding +8581779 ADR-0024 HintMode dispatch via walker_hint_mode_at_input +abebd79 ADR-0024 Phase D (full): schema-aware value typing +8188fa5 ADR-0024 round-5 follow-up: surface tail-Optional expectations +``` + +**Tests:** **830 passing, 0 failing, 1 ignored** (up from 806 +at handoff-10's baseline). Net +24 — these all under +`dsl::walker::tests::phase_d_*`, `dsl::walker::tests::hint_mode_*`, +`completion::tests::ranker_*`, and the round-5 optional-suffix +tests. The ignored test is still the same `\`\`\`ignore` +doc-test in `src/friendly/mod.rs`. + +**Clippy:** clean with `nursery` lints + `-D warnings`. + +## What shipped this session + +### Step 5 — Walker-driven completion (commit bbe1252) + +The completion engine no longer round-trips through the +`ParseError::Invalid::expected: Vec` bridge. New helper +`walker::expected_at_input(source) -> Vec` returns +structured expectations with full `IdentSource + role` info +attached. The completion engine pattern-matches `Expectation` +variants directly: + +- `Word(w)` / `Literal(s)` → keyword candidates (alphabetic + filter). +- `Ident { source: IdentSource::Types }` → type names. +- `Flag(body)` → `--{body}` candidates. +- `Literal("1")` → composite-literal opener (`1:n`). +- `Ident { source, .. }` where `source.completes_from_schema()` → + schema identifier candidates. + +`is_value_literal_signature` now matches on `Expectation::Word` +literals + `NumberLit` + `StringLit` variants instead of +backticked strings. `invalid_ident_at_cursor` and +`typing_name_at_cursor` adopt the same structured path. + +Zero observable-behaviour change: every existing completion +test passes unchanged. The win is clean code and lossless +information flow — the chumsky-era string round-trip is gone. + +### Ranker hook (commit 7ae1a0f) + +ADR-0024 §ranker-layer scaffolding: + +- `pub type Ranker = fn(Vec) -> Vec;` +- `pub const fn identity_ranker(c) -> Vec` returns + its input unchanged. +- `candidates_at_cursor_with(input, cursor, cache, ranker)` + applies a custom ranker; the default + `candidates_at_cursor(input, cursor, cache)` delegates with + `identity_ranker`. + +Three tests cover identity preservation, custom reordering, and +the empty-list-collapses-to-None edge. + +No production caller passes a non-identity ranker yet — hooks +for frequency-based ranking, content-aware priors, and recency +plug in here without touching grammar declarations. + +### HintMode dispatch (commit 8581779) + +ADR-0024 §HintMode-per-node specified a dispatch layer between +the walker's expected-set and the hint-panel renderer. This +session lands the dispatch shape: + +- `walker::hint_mode_at_input(source) -> Option` + pattern-matches the walker's expected-set for the + value-literal-slot signature (returns + `ProseOnly("hint.value_literal_slot")`) and for + `Ident { source: NewName }` (returns + `ForceProse("hint.ambient_typing_name")`). +- `input_render::ambient_hint` consults the hint mode before + the candidate / prose ladder. `ProseOnly` emits the catalog + prose at empty prefix; `ForceProse` consults + `typing_name_at_cursor` for the post-name probe and emits + the typing-name prose. +- A separate `expected_for_hint(source)` returns empty on + `WalkOutcome::Match` so the hint resolver doesn't surface + prose at the end of a valid command (where the completion + engine offers optional-suffix candidates instead). + +This is the structural shape the ADR specified; the actual +detection inside `hint_mode_at_input` is signature-matching +against the expected-set (same logic the previous post-hoc +detectors used, relocated to one place). The detection becomes +node-attached in Phase D (already wired below). + +`HintMode` gains `PartialEq + Eq` for tests; docstring +rewritten to describe live semantics. + +8 new walker tests pin the hint resolver across +value-literal-slot positions, NewName-slot positions, and the +non-fire cases. + +### Phase D (full) — schema-aware value typing (commit abebd79) + +The central design claim of ADR-0024 §Phase D. Insert / update +/ delete value slots dispatch on the user-facing column type at +parse time, rejecting mis-shaped input with localised wording +instead of waiting for the bind-time error. + +End-to-end pipeline: + +1. **SchemaCache extension** (`src/completion.rs`): new + `TableColumn { name, user_type }` per-table type metadata + stored in `SchemaCache.table_columns: HashMap>`. `columns_for_table(name)` is the + case-insensitive lookup helper. +2. **WalkContext schema plumbing** (`src/dsl/walker/context.rs`): + `WalkContext<'a>` gains a lifetime and a + `schema: Option<&'a SchemaCache>`. `WalkContext::new()` is + the schemaless default; `with_schema(s)` is the schema-aware + constructor. +3. **Schema-aware parse entry point** + (`src/dsl/parser.rs`): `parse_command_with_schema(input, + schema)` is the new public variant. `parse_command(input)` + delegates with `None` for back-compat. +4. **`Node::Ident` writes_table/writes_column** flags + (`src/dsl/grammar/mod.rs`): when set, the walker writes the + matched identifier's resolution into `WalkContext`. +5. **Walker driver `DynamicSubgrammar` dispatch** + (`src/dsl/walker/driver.rs`): the factory resolves the inner + Node at walk time; the result is `Box::leak`ed so its + static-slice fields (Choice/Seq) have the lifetime the + walker expects (per ADR-0024 §sub-grammars). +6. **Typed value slot factories** + (`src/dsl/grammar/shared.rs`): one per `Type` — + `int_slot` (integer-only NumberLit + null), `real_slot`, + `decimal_slot`, `bool_slot`, `text_slot`, `date_slot`, + `datetime_slot`, `blob_slot`. `slot_for_type(ty)` is the + dispatcher. `current_column_value(ctx)` reads + `current_column` and dispatches. `column_value_list(ctx)` + reads `current_table_columns` and emits a Seq of typed + slots separated by commas. +7. **Data-command grammar uses the typed slots** + (`src/dsl/grammar/data.rs`): `INSERT_VALUES_LIST` becomes + `DynamicSubgrammar(column_value_list)`. `UPDATE_ASSIGNMENT` + and `WHERE_CLAUSE` use + `PER_COLUMN_VALUE = DynamicSubgrammar(current_column_value)`. + Insert / update / delete table-name slots set + `writes_table: true`; update / delete column-name slots set + `writes_column: true`. +8. **Runtime populates schema-with-types** (`src/runtime.rs`): + `refresh_schema_cache` calls `describe_table` for each + table and populates `SchemaCache::table_columns`. Live + typing benefits immediately on project load and after + every successful DDL. +9. **App dispatches with schema** (`src/app.rs`): + `dispatch_dsl` routes through `parse_command_with_schema`. + +Catalog: new `parse.custom.bind_type_mismatch` entry with +`{found}` / `{expected}` placeholders, surfaced by the +int_slot and decimal_slot validators. + +Fallback semantics: when the schema can't resolve a table (or +the walker is schemaless), `DynamicSubgrammar` factories fall +back to the schemaless `VALUE_LITERAL` choice — the +pre-Phase-D behaviour, so every existing test passes +unchanged. + +11 new walker tests cover schema-aware insert / update / +delete: typed acceptance per column, decimal rejection at int +columns, null acceptance at every slot, multi-assignment +per-column dispatch, schemaless fallback. + +### Round-5 follow-up — tail_expected (commit 8188fa5) + +Closes the long-standing "save Tab offers `as`" gap. Pre-this +session, `save ` parsed as a complete `save` command, the +walker returned `Match { command_idx }`, and the completion +engine had nothing to mine. + +`WalkResult` gains a `tail_expected: Vec` field. +The walker's top-level `Matched` branch copies the outer +shape's skipped-Optional expectations into it. +`expected_at_input` returns `tail_expected` on `Match`. +Completion now surfaces `as` (for `save `), `short` / +`verbose` (for `messages `), and any other Optional suffix +expectation as Tab candidates. + +`hint_mode_at_input` deliberately does NOT consume +`tail_expected` (a separate `expected_for_hint` returns empty +on Match). Reasoning: completion offering optional suffixes is +helpful ("you could continue with `as`"); the hint panel +showing prose like "Type a name" at the end of a valid command +would be misleading ("the command is complete, but you're +forcing me to continue"). + +Two new tests: `save_space_offers_as_via_tail_expected`, +`messages_space_offers_short_and_verbose_via_tail_expected`. + +## What's still pending — for future work + +### 1. `CommandNode.help_id` not consumed + +Every CommandNode declares `help_id: Option<&'static str>` (e.g. +`app.quit`, `data.insert`) pointing into a catalog namespace +that doesn't exist yet. The current help system reads +`help.in_app_body` — one big hand-curated catalog entry. + +Wiring `help_id` would mean: +- Add 20 per-command help catalog entries. +- Replace the `help.in_app_body` lookup with iteration over + REGISTRY → translate each `help_id`. +- The in_app_body composition logic lives in + `app::note_help`. + +Benefit: new commands auto-include in help without YAML edits. +Cost: medium refactor; user-facing strings need careful +rewording. + +Not blocking; not user-visible regression. + +### 2. `Box::leak` memory growth per dynamic walk + +`Node::DynamicSubgrammar` dispatch uses `Box::leak` in the +walker driver. Per-walk leak is bounded by command-shape +complexity (~6 Node allocations for a typical +`insert into T values (...)` with 5 columns). For +per-keystroke completion polling, the leak grows over time. + +ADR-0024 §sub-grammars sketched a per-walk arena as the +alternative. Implementation would replace the Box::leak with +a `WalkContext::arena: Vec>` that lives for the +walk's duration. Worth doing if memory growth becomes +measurable (it isn't today). + +### 3. Replay path is schemaless + +`runtime::run_replay` calls `parse_command(line)` without a +schema, so typed-slot rejections don't fire during replay — +bind-time errors still catch the same cases, so this is a UX +nicety not a correctness gap. The schema mutates during replay +(create / add columns), so the simplest "thread schema through" +approach would need a per-line re-fetch. + +Worth doing once a user actually trips this. Today it just +means a `replay` that contains `insert into T values (3.14)` at +an int column fails at bind time instead of at parse time. + +### 4. HintMode is detection-based, not node-attached + +The ADR design says HintMode annotations should live on +grammar nodes (so `date_slot` could carry +`ProseOnly("hint.date_format")` and the resolver dispatches on +each expected node's HintMode). Today the resolver +pattern-matches the walker's expected-set signature for the +value-literal slot and the NewName ident slot. + +Functionally equivalent — but the structural form would let +typed value slots carry per-type prose hints (a date slot +shows "Type a date as 'YYYY-MM-DD'" instead of the generic +value-literal hint). Phase D's typed slots are ready to receive +the annotations; just plumb HintMode through Expectation when +emitting from `Choice` / `Ident` / `NumberLit` / `StringLit` +nodes. + +### 5. Ranker is scaffolding-only + +No production caller passes a non-identity ranker. Frequency +ranking, content-aware priors, recency hooks — all future +work that plugs in via `candidates_at_cursor_with`. + +### 6. Dead `parse.token.*` catalog entries + +The five structural-class entries +(`parse.token.identifier/number/string_literal/flag/end_of_input`) +and three lex-error entries +(`parse.token.error.{bad_flag,unknown_char,unterminated_string}`) +remain in the catalog. Today's walker doesn't surface them +(structural classification feeds highlighting, not catalog +lookup). Carried from handoff-10 §8 — conservative call: leave +in. They cost nothing and reintroducing them would be cheap if +a future need arises. + +## Sharp edges and invariants + +These are the load-bearing decisions worth remembering: + +- **Phase D fallback is behaviour-preserving.** A + `DynamicSubgrammar` factory called with no schema in + WalkContext, or with a table the schema doesn't know about, + returns the schemaless `VALUE_LITERAL` choice. Every test + that called `parse_command(input)` without a schema continues + to pass with the existing semantics. +- **`Box::leak` per walk.** See pending §2 above. +- **`tail_expected` is for completion, not hint.** Surfacing + optional-suffix expectations through the hint resolver would + display prose like "Type a name" at the end of a complete + command. The split via `expected_for_hint` enforces this + invariant. +- **`writes_table` resolves columns at the same site.** The + walker writes `current_table = Some(matched)` AND resolves + `current_table_columns` from `schema.columns_for_table`. If + the schema lookup misses, `current_table_columns` is None and + downstream DynamicSubgrammar fallbacks engage. The column + list is a *snapshot* at the moment of the table ident match — + if the schema cache refreshes mid-walk (it doesn't today), + the walk would use the old snapshot. This is a feature, not a + bug — schema-cache consistency belongs to the runtime, not + the walker. +- **Schema-aware parse cost.** Every `dispatch_dsl` call now + walks the schema-aware path with a borrowed SchemaCache. The + cost is one `HashMap::get(table)` per Tables ident write and + one `Vec::clone` for the column list. Negligible. +- **`Repeated(UPDATE_ASSIGNMENT, ',', 1)` preserves + current_column write semantics.** Each repetition writes the + matched column name, overwriting the previous iteration's + value. The `=` and the typed value slot dispatch against the + most recent write. Correct. + +## ADR index (delta vs. handoff-10) + +``` +0014 Data operations, value literals, and auto-show + — Phase D now rejects mis-shaped values at parse time + instead of at bind time. Same final behaviour; better UX. +0024 Unified grammar tree: execution plan (ACCEPTED) + — A through F (full all 5 steps) + D (full) landed. + HintMode dispatch landed (detection-based; node-attached + form is non-blocking future work). + Ranker hook landed (scaffolding). +``` + +## Repository layout (delta vs. handoff-10) + +Files significantly modified: + +``` +src/dsl/grammar/mod.rs — Node::Ident writes_table/ + writes_column flags +src/dsl/grammar/data.rs — insert / update / delete use + DynamicSubgrammar value slots +src/dsl/grammar/shared.rs — typed value slots + (int_slot, decimal_slot, …), + slot_for_type, + current_column_value, + column_value_list +src/dsl/walker/context.rs — WalkContext gains lifetime + and schema reference; + current_column changes from + ColumnInfo to TableColumn +src/dsl/walker/driver.rs — DynamicSubgrammar dispatch + via Box::leak; writes_table/ + writes_column semantics in + walk_ident +src/dsl/walker/mod.rs — expected_at_input, + hint_mode_at_input, + expected_for_hint; + WalkResult::tail_expected + surfacing on Match +src/dsl/walker/outcome.rs — WalkResult::tail_expected +src/dsl/parser.rs — parse_command_with_schema +src/completion.rs — SchemaCache::table_columns, + columns_for_table, TableColumn; + structured Expectation + consumption throughout; + Ranker / identity_ranker; + candidates_at_cursor_with +src/input_render.rs — HintMode dispatch via + walker_hint_mode_at_input; + hint_leading_slice helper +src/app.rs — parse_command_with_schema in + dispatch_dsl +src/runtime.rs — describe_table loop in + refresh_schema_cache +src/friendly/strings/en-US.yaml — parse.custom.bind_type_mismatch +src/friendly/keys.rs — keys.rs declaration +``` + +Files added: none (only new tests inside existing modules). + +## How to take over + +1. **Read this file.** +2. **Read handoff-10** (`20260515-handoff-10.md`) for the prior + session's Phase F (full) work that landed steps 1-4 and + set the stage. +3. **Read `CLAUDE.md`** for the working-style rules. +4. **Read ADR-0024** for the design intent now fully realised. +5. **Skim `src/dsl/grammar/mod.rs`** — Node enum, IdentSource, + HintMode, CommandNode contract. +6. **Skim `src/dsl/walker/mod.rs`** — walk(), expected_at_input, + hint_mode_at_input, tail_expected propagation. +7. **Skim `src/dsl/grammar/shared.rs`** — typed value slots + and the two dynamic sub-grammars. +8. **Run `cargo test`** — should report 830 passing, 0 + failing, 1 ignored. +9. **Run `cargo clippy --all-targets -- -D warnings`** — + clean. +10. **If picking up pending items**: see §1-§6 above for the + open work. §1 (help_id wiring) and §4 (node-attached + HintMode) are the most tractable; the rest are background + notes more than action items. + +### End-to-end smoke (post-this-session) + +User-visible improvements vs. handoff-10: + +- **Typed value rejection at parse time.** With a populated + schema, `insert into T values (3.14)` at an int column fires + a parse-time error ("value '3.14' is not a valid integer") + instead of waiting for the bind-time `bind_type_mismatch` + error. Same wording, earlier surface. +- **Save Tab offers `as`.** Type `save ` and press Tab → the + completion menu offers `as` (closing the round-5 gap). +- **Messages Tab offers `short` / `verbose`.** Same shape. +- **Walker-direct completion.** Same user-visible candidates + as before; the round-trip through formatted strings is gone. + Future hint improvements (per-column-type prose, + frequency-based ranking) plug in without touching grammar + declarations. + +Everything else is internal — code health, clearer +abstractions, fewer surprises for the next maintainer.