add handoff-11: ADR-0024 Phase F/D fully landed; HintMode/Ranker/round-5 closed

2026-05-15 17:53:14 +00:00
parent 8188fa5ee1
commit 124c1d33e9
1 changed files with 442 additions and 0 deletions
@@ -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<String>` bridge. New helper
+`walker::expected_at_input(source) -> Vec<Expectation>` 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<Candidate>) -> Vec<Candidate>;`
+- `pub const fn identity_ranker(c) -> Vec<Candidate>` 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<HintMode>`
+  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<String,
+   Vec<TableColumn>>`. `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<Expectation>` 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<Box<Node>>` 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.