docs: handoff 25 — ADR-0030 Phase 1 + ADR-0031 complete
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.
This commit is contained in:
@@ -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<Command, ValidationError>` —
|
||||||
|
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 ('*' | <expr>[ as <alias>][, ...])
|
||||||
|
[ from <Table> ]
|
||||||
|
[ where <sql_expr> ]
|
||||||
|
[ order by <expr>[ asc|desc][, ...] ]
|
||||||
|
[ limit <n> ]
|
||||||
|
[ ; ]
|
||||||
|
```
|
||||||
|
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<None>` 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<None>`. 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::<X> { 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::<X> { sql: String }` pattern** is the Phase-3
|
||||||
|
template for DML — `build_<x>(_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.
|
||||||
Reference in New Issue
Block a user