From be8a4f514d223229d4c5ee5e58779f2d33bffb96 Mon Sep 17 00:00:00 2001 From: "claude@clouddev1" Date: Tue, 19 May 2026 21:55:09 +0000 Subject: [PATCH] =?UTF-8?q?docs:=20handoff=2025=20=E2=80=94=20ADR-0030=20P?= =?UTF-8?q?hase=201=20+=20ADR-0031=20complete?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implementation handoff: a SQL `select` typed in advanced mode parses, runs, and renders end to end; the same line in simple mode lights up the precise "this is SQL" hint instead. ADR-0031 (the SQL expression grammar) and ADR-0030 Phase 1 ("Foundations + first SELECT") landed across five commits. Tests 1240 → 1260, clippy clean. The handoff records: the walker mode gate + `is_advanced_only` set, the `ast_builder` source-param sweep, `Command::Select` carrying the validated SQL text, the `data::SELECT` shape, the worker `Request::RunSelect` round-trip, the ambient mode threading through completion / overlay / validity indicator, the autonomous calls made during execution (FROM optional, implicit alias unsupported, etc.), and the seams the next session uses to take up ADR-0030 Phase 2 (full SELECT) — which gets its own focused ADR before code, per ADR-0030 §3. --- docs/handoff/20260519-handoff-25.md | 338 ++++++++++++++++++++++++++++ 1 file changed, 338 insertions(+) create mode 100644 docs/handoff/20260519-handoff-25.md diff --git a/docs/handoff/20260519-handoff-25.md b/docs/handoff/20260519-handoff-25.md new file mode 100644 index 0000000..83cabff --- /dev/null +++ b/docs/handoff/20260519-handoff-25.md @@ -0,0 +1,338 @@ +# Session handoff — 2026-05-19 (25) + +Twenty-fifth handover. **Implementation session.** ADR-0030 +Phase 1 ("Foundations + first `SELECT`") and ADR-0031 (the SQL +expression grammar) landed end to end: a SQL `select` typed in +advanced mode parses, runs against the database, and renders +through the existing data-table renderer; the same line in +simple mode lights up the precise "this is SQL" hint instead of +running, and Tab completion / live overlay / `[ERR]` indicator +agree. + +## §1. State at handoff + +**Branch:** `main`. Working tree clean. + +**Tests:** **1260 passing, 0 failing, 1 ignored** (the +unchanged doctest). Up from the handoff-24 baseline of 1240 by ++20 — 13 sql-expression unit tests, 7 SQL-`SELECT` integration +tests. **Clippy:** clean under `-D warnings`. + +**New commits since handoff-24** (`a049ff9..HEAD`): + +``` +cd6371a tests: Phase 1 SQL SELECT integration tests +83e0ddc app: mode-threaded completion, overlay, and validity indicator +6369066 grammar: SQL SELECT end-to-end (ADR-0030 Phase 1) +c93f939 grammar: SQL expression grammar fragment (ADR-0031) +81793a3 docs: ADR-0031 — SQL expression grammar +``` + +All local; push asynchronously, not blocking. + +## §2. What was built — the shape now + +Read **`docs/adr/0031-sql-expression-grammar.md`** for the +grammar spec (Accepted) and **`docs/adr/0030-advanced-mode-sql- +surface.md`** for the surrounding architecture. + +### 2.1. ADR-0031: the SQL expression grammar + +`src/dsl/grammar/sql_expr.rs` — a parallel fragment to +`grammar/expr.rs`. Stratified ladder, one named `static Node` +per precedence tier: + +``` +or_expr → and_expr → not_expr → predicate → additive → + multiplicative → unary → primary +``` + +Predicate set: `cmp_op` / `LIKE` / `BETWEEN` / `IN` / `IS NULL` +(with the ADR-0026 factoring — operand prefix matched once, +infix `NOT` as an explicit branch). Primary covers literals, +`( or_expr )`, `case_expr` (searched + simple), and +`name_or_call` — the identifier-prefix shared between column +refs and function calls factored into one `Ident` followed by +an `Optional` `( call_args )` tail (the same hazard-avoidance +shape `predicate_tail` uses). `count(*)` and `count(distinct +…)` both fall out of `call_args`. + +**No AST builder** — every Phase-1 consumer (SELECT projection, +WHERE) runs validated SQL as text per ADR-0030 §4/§6. The +fragment's output is the matched-terminal `MatchedPath` (drives +highlight / completion / hints) plus accept/reject. ADR-0031 +§2 documents the rationale. + +Drop-in: `pub static SQL_EXPRESSION: Node = Subgrammar(&SQL_OR_EXPR);`. +SQL `CommandNode` shapes embed it the way DSL shapes embed +`expr::EXPRESSION`. Recursion goes through `Node::Subgrammar` +with ADR-0026's `MAX_SUBGRAMMAR_DEPTH = 64` cap reused +unchanged — no new walker capability. + +### 2.2. Walker mode gate (ADR-0030 §2) + +- `WalkContext` gains `mode: Mode`. `Mode` derives `Default` + (`= Simple`, matching the app's startup mode). +- `grammar::is_advanced_only` keys the advanced-only entry-word + set. Phase 1 has one entry: `select`. +- In `walker::walk()`, immediately after `command_for_entry_word` + resolves the `CommandNode`, the gate short-circuits when + `ctx.mode == Simple && is_advanced_only(entry)` — returning a + `WalkResult` whose outcome is `ValidationFailed { error: { key: + "advanced_mode.sql_in_simple", … } }`. The entry word still + highlights as a keyword; the parse-error layer translates the + catalog key into the "switch with `mode advanced`, or prefix + the line with `:`" hint; the validity indicator goes ERROR. +- `parser::parse_command_with_schema_in_mode` (and the + schemaless `parse_command_in_mode`) thread `mode` into + `WalkContext`. Old `parse_command*` keep delegating with + `Mode::Advanced` (most permissive — back-compat for tests + + any caller that doesn't care about gating). + +### 2.3. Builder signature sweep (ADR-0031 §2) + +`CommandNode.ast_builder` is now +`fn(&MatchedPath, &str) -> Result` — +the walker forwards the parse `source`. `build_select` reads it +to put the validated SQL text into `Command::Select`; the 21 +pre-existing builders accept it as `_source`. The walker's call +site flips from `(&path)` to `(&path, source)` in one place. + +### 2.4. `Command::Select` + execution + +- `Command::Select { sql: String }` — every exhaustive + `match Command` updated (`verb`, `target_table`, + `build_translate_context`, `execute_command_typed`, + `typing_surface_matrix`'s `command_kind_label`). +- `grammar::data::SELECT` — the `CommandNode`: + ``` + projection ('*' | [ as ][, ...]) + [ from ] + [ where ] + [ order by [ asc|desc][, ...] ] + [ limit ] + [ ; ] + ``` + Expression slots reference `sql_expr::SQL_OR_EXPR` via + `Subgrammar`. The `FROM` table-name slot carries a + `reject_internal_table` validator that refuses any `__rdbms_*` + reference at parse time (ADR-0030 §6). +- Worker: `Request::RunSelect { sql, source, reply }` + + `Database::run_select` + `do_run_select_request` / + `do_run_select`. The latter prepares the statement, collects + rows into a `DataResult` with `column_types: Vec` per + ADR-0030 §6, and appends the literal source line to + `history.log` so replay re-runs it (ADR-0030 §11). +- Runtime: `execute_command_typed` gains the `Command::Select` + arm → `database.run_select(sql, src)` → + `CommandOutcome::Query` → `AppEvent::DslDataSucceeded` → + `render_data_table`. Reuses the existing `show data` + rendering path end to end. + +### 2.5. Dispatch unification + ambient mode threading + +- `App::submit` is unified: both modes go through + `dispatch_dsl(&effective_input, effective_mode)`, which parses + with the line's effective mode. The placeholder advanced-mode + echo branch is gone (and the `advanced_mode.not_implemented` + catalog entry is dead code now — left in place; remove on a + passing future cleanup). +- `EffectiveMode::as_mode()` collapses the persistent / + one-shot distinction into the plain `Mode` the walker reads. +- Walker exposes `completion_probe_in_mode`, + `expected_at_input_in_mode`, `input_verdict_in_mode`. The + empty-input / unknown-entry fallbacks in the first two filter + the `REGISTRY` listing by `is_advanced_only` — Tab in simple + mode does not offer `select`. +- Completion exposes `candidates_at_cursor_in_mode` / + `candidates_at_cursor_with_in_mode`. Internally they route the + `parse_command` completeness probe and the `completion_probe` + / `expected_at` calls through the mode-aware variants. +- `App::input_validity_verdict` and the Tab handlers + (`start_or_complete_at` / `_last`) pass `Mode::Simple` + (validity indicator only fires in plain simple mode) and + `self.effective_mode().as_mode()` (Tab handlers). + `input_render::render_input_runs` and `ambient_hint` are + invoked only when effective mode is plain `Simple` (ui.rs + gates), so their internal calls hardcode `Mode::Simple`. + +### 2.6. Catalog (ADR-0019) + +- `advanced_mode.sql_in_simple` `{command}` — the walker gate + hint. +- `select.internal_table` `{table}` — the `__rdbms_*` + rejection. +- `parse.usage.select` — the parse-error usage template. + +## §3. Behaviour now (user-visible) + +- **Advanced-mode `select`** runs: `select 1`, `select * from t`, + `select Name from Customers where Age > 18 order by Name limit + 10` — all parse, dispatch as `Command::Select`, run via the + worker, render as a data table. +- **Simple-mode `select`** does not run: the walker gates it, + the dispatch path renders the catalog hint + (`advanced_mode.sql_in_simple`), the validity indicator + lights ERROR before submit, the live overlay marks the input + as a definite error, Tab does not offer it as a completion. +- **`:select …`** in simple mode → one-shot Advanced, dispatches + the SELECT; persistent mode unchanged. +- **`__rdbms_*` in `FROM`** → parse rejected with + `select.internal_table`. +- **`history.log`** records every executed SELECT (literal + line); `replay` re-runs them through the same dispatch path. + +The data-table renderer treats SELECT results as a typeless +view — `column_types: Vec`. Pedagogically that means +neutral alignment for everything; one specific UX note: bool +columns SELECTed render as `0`/`1` rather than `true`/`false`. +Phase 2 can enrich plain-column refs back to their source type. + +## §4. Autonomous decisions during execution + +Per CLAUDE.md, these were judgement calls outside the original +approach — they are flagged here so the next session can +sanction or override: + +1. **`FROM` clause is optional.** ADR-0030 Phase 1 says + "single-table SELECT"; standard SQL also admits `select 1` / + `select upper('x')` (zero-table constant or function-call + form). Included because it is the canonical learner probe; + documented in code at the SELECT shape declaration. +2. **Implicit projection aliasing (`select a x`) deliberately + unsupported.** A bare alias and the `from` keyword are + structurally ambiguous; only `select a as x` is admitted. +3. **`help_id: None` on the SELECT `CommandNode`.** The `help` + listing skips `select` for now — the `help sql` page is + ADR-0030 Phase 6. +4. **Some walker entries still default to internal + `Mode::Simple` via `WalkContext::new()`** — + `hint_mode_at_input*`, `hint_resolution_at_input`, + `input_diagnostics`, `highlight_runs`. Production paths that + consume them are only invoked when effective mode is plain + `Simple` (ui.rs gates), so the default is behaviourally + correct. Full threading lands when advanced mode grows its + own ambient assistance (Phase 2-ish). +5. **SELECT result bool columns render as `0`/`1`.** All result + `column_types` are `None` (ADR-0030 §6 — typeless view). + Phase 2 can resolve plain column refs back to their schema + type and recover the DSL `show data` rendering. + +## §5. What's next — ADR-0030 Phases 2–6 + +ADR-0030's plan continues. Each later phase is independently +shippable; the two large grammar slices (Phase 2's full +`SELECT`; Phase 3's SQL DML expression set if it grows) each +warrant their own focused ADR (precedent: ADR-0026, ADR-0031). +The order in the ADR is the suggested execution order; the +user decides what to take next. + +1. **Phase 2 — `SELECT` (full).** `JOIN`s (inner / left / + right), `GROUP BY` / `HAVING`, aggregate functions, + subquery expressions (`(SELECT …)` as a primary; + `IN (SELECT …)`; `EXISTS (…)`), `UNION` / `INTERSECT` / + `EXCEPT`, common table expressions (`WITH`), `LIMIT … OFFSET`, + qualified column refs (`t.c`, `alias.c`). Its own focused + ADR before authoring — same way ADR-0031 preceded the + fragment. The expression grammar (ADR-0031) is shaped to + receive the subquery and qualified-ref branches additively; + no restructuring foreseen there. + +2. **Phase 3 — DML.** SQL `INSERT` (single- and multi-row), + `UPDATE`, `DELETE`. Execute as validated SQL (ADR-0030 §4) + via the same `Command:: { sql }`-as-text pattern as + `Command::Select` — the worker's "run validated DML + re- + persist the table" request is new. Settle multi-row + `INSERT` and `shortid` auto-fill on a SQL `INSERT` (open + question, escalate to the user when reached). + +3. **Phase 4 — DDL.** SQL `CREATE`/`DROP`/`ALTER TABLE`, + `CREATE`/`DROP INDEX`. DDL routes through the typed + `Command` executor (ADR-0030 §4) — the `CommandNode`'s + `ast_builder` translates SQL → `Command::{CreateTable, + AddColumn, …}`, then the existing executor keeps metadata, + the type vocabulary (ADR-0005) and `STRICT` intact. ADR-0030 + §5 maps standard SQL type names (`integer`, `varchar(255)`, + `numeric`, …) onto the ten-type vocabulary. `CHECK` / + `DEFAULT` expressions reuse the ADR-0031 fragment but their + *storage* is text (ADR-0030 §11) — the `Constraint::Check` + shape may need a text-carrying variant; escalate when + reached. Table-rename (`C1`) may fall out here. + +4. **Phase 5 — DSL → SQL teaching echo.** A DSL command run in + advanced mode prints its equivalent SQL beneath the `[ok]` + summary (ADR-0030 §10). It is a `Command` → SQL renderer; + uses the `OutputLine` styled-runs mechanism (ADR-0028). + App-level commands have no SQL form and are not echoed. + +5. **Phase 6 — Polish.** `help sql`; engine-neutral error + sweep (ADR-0030 §7); typing-surface / matrix coverage for + the SQL surface; the `DOC1` SQL-surface reference page. + +## §6. Seams for the next implementer + +- **`sql_expr.rs` is the grammar evolution point.** Phase 2's + subquery `primary` branch and qualified-column-ref tail are + additive — a new `Choice` branch in `primary`, recursing + through `Subgrammar` into the SELECT fragment, and a + `[ '.' identifier ]` tail on `name_or_call`'s ident. The + ADR-0031 §7 OOS list calls these out by name. +- **`grammar/data.rs` SELECT is the model for SQL DML/DDL + CommandNodes** (Phases 3-4) — same `Subgrammar(&SQL_OR_EXPR)` + for expression slots, same `reject_internal_table` style + validator on table-name slots, same `_NODES` / `_SHAPE` / + `CommandNode` layout. +- **`Command:: { sql: String }` pattern** is the Phase-3 + template for DML — `build_(_path, source)` packages the + validated text; the runtime arm sends it to a new + "run + re-persist" worker request. +- **`WalkContext.mode` is the gate**; per-`Choice`-branch + tagging (ADR-0030 §2 end state) is what arrives with the + shared DSL/SQL entry words (`create`, `insert`, …) in Phase + 3-4. The whole-command gating via `is_advanced_only` covers + Phase 1 cleanly; per-branch tagging is a new mechanism on top + — design it when the first shared entry word lands. +- **`Database::run_select`** is the read-only `conn.prepare` + + collect-rows pattern. The Phase-3 DML "run validated SQL + re- + persist the affected table" worker request reuses the + `conn.prepare` half and adds re-persist (the existing + `do_query_data_request` persistence call is a model). +- **`ast_builder` now receives `source: &str`** uniformly — SQL + DML/DDL builders in Phases 3-4 use it freely; existing DSL + builders ignore it as `_source`. +- **`parse_command*_in_mode`** is the right entry for any new + schema/mode-sensitive call site; the unmoded `parse_command*` + default to `Mode::Advanced` for back-compat. New production + call sites should always pass an explicit mode. + +## §7. How to take over + +1. **Read this file, then `docs/adr/0030-advanced-mode-sql- + surface.md` and `docs/adr/0031-sql-expression-grammar.md`.** + Then `CLAUDE.md` (working-style rules) and `docs/ + requirements.md` (Q1/Q2 partially satisfied; Q4 ticked). +2. **`cargo test`** — 1260 passing, 0 failed, 1 ignored. +3. **`cargo clippy --all-targets -- -D warnings`** — clean. +4. **Pick the next phase with the user.** Phase 2 (full SELECT) + is the natural next step and gets its own ADR before code, + per ADR-0030 §3 — write the focused grammar ADR first + (ADR-0026 / ADR-0031 are the model). +5. **Escalate, do not guess**, on the open per-phase questions + ADR-0030 left — multi-row `INSERT`, `shortid` on SQL + `INSERT`, the `Constraint::Check` text-carrying variant, any + walker-taxonomy extension that changes the walker's contract. + +## §8. What else is open + +Unchanged from handoff-24 §7 (prioritisation is a **user +decision**): snapshot/undo `U`-series (ADR-0006 written, +unbuilt); m:n convenience `C4`; modify-relationship `C3a`; the +`show` family `V5`; rename-table `C1` (may fall out of ADR-0030 +Phase 4); friendly-error sweep `H1`; CI `TT5`; session-log / +Markdown export `V4`. + +Phase 1's specific deferrals (§4 of this file) — `help_id: None` +on SELECT (Phase 6), the few walker entries still defaulting to +internal Simple mode, bool rendering in SELECT results — are +documented; the user decides whether to revisit any in Phase 2 +or leave until Phase 6 polish.