From 3db292c795f2a9232efbffe44b4610488fe75837 Mon Sep 17 00:00:00 2001 From: "claude@clouddev1" Date: Wed, 20 May 2026 10:29:04 +0000 Subject: [PATCH] =?UTF-8?q?docs:=20handoff=2026=20=E2=80=94=20ADR-0032=20+?= =?UTF-8?q?=20Phase=202=20plan=20accepted?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Twenty-sixth handover. Design session, no code touched. Tests unchanged at 1260 / 0 / 1. Captures: the Phase 2 grammar decisions (ADR-0032 accepted), the implementation plan at docs/plans/20260520-adr-0032-phase-2.md with seven sub-phases and a cross-cut verification matrix that explicitly names every "X comes for free" claim from ADR-0030/ 0031/0032, and the Phase-1 carry-over finding the warning/error guideline check surfaced — SQL WHERE expressions currently emit no LIKE-on-numeric / = NULL / type-mismatch warnings because sql_expr builds no AST. ADR-0032 §11.6 closes the gap; the plan's cross-cut matrix has a named row to prevent regression. The next session is Phase 2 sub-phase 2a (grammar fragment) per the plan; standing authorizations apply. Status: ready to hand over. --- docs/handoff/20260520-handoff-26.md | 332 ++++++++++++++++++++++++++++ 1 file changed, 332 insertions(+) create mode 100644 docs/handoff/20260520-handoff-26.md diff --git a/docs/handoff/20260520-handoff-26.md b/docs/handoff/20260520-handoff-26.md new file mode 100644 index 0000000..5612207 --- /dev/null +++ b/docs/handoff/20260520-handoff-26.md @@ -0,0 +1,332 @@ +# Session handoff — 2026-05-20 (26) + +Twenty-sixth handover. **Design session.** ADR-0032 (the full +SQL `SELECT` grammar — ADR-0030 Phase 2) is **Accepted** and an +**implementation plan** sits next to it under a new +`docs/plans/` directory. No code changed this session; the next +session is implementation territory. + +## §1. State at handoff + +**Branch:** `main`. Working tree clean. One unpushed commit +(`be8a4f5..a7db7dd`). + +**Tests:** **1260 passing, 0 failing, 1 ignored** — unchanged +from handoff-25 (no code touched). + +**Clippy:** clean (last verified at handoff-25; no code changed +since). + +**New commit since handoff-25:** + +``` +a7db7dd docs: ADR-0032 + Phase 2 plan — full SQL SELECT grammar +``` + +A single docs commit; ADR + plan + index update ship as a +coherent unit because the plan is Phase-2's mechanics for +ADR-0032's decisions. + +## §2. What got designed — the shape now + +Read in order: + +1. **`docs/adr/0032-sql-select-grammar.md`** — the grammar + decisions for Phase 2. +2. **`docs/plans/20260520-adr-0032-phase-2.md`** — the + implementation plan walking the work in seven sub-phases. + +### 2.1. ADR-0032 in one screen + +Phase 2's grammar surface (§§1–9 of the ADR): + +- Five JOIN flavours: `INNER`, `LEFT [OUTER]`, `RIGHT [OUTER]`, + `FULL [OUTER]`, `CROSS`. NATURAL/USING/comma-FROM are OOS. +- All four set ops: `UNION`, `UNION ALL`, `INTERSECT`, `EXCEPT`. +- CTEs: `WITH` and `WITH RECURSIVE`, with optional `(col-list)`. +- Subquery expressions as additive `sql_expr` branches: scalar + `(SELECT …)` primary, `IN (SELECT …)`, `[NOT] EXISTS (…)`. + Redeems ADR-0031 §7 OOS-1. +- Qualified column refs `t.c` / `alias.c` as a `name_or_call` + tail. Redeems ADR-0031 §7 OOS-2. +- `LIMIT n [OFFSET m]`; legacy comma form OOS. +- `DISTINCT` / `ALL`, `t.*` projection, bare-alias projection + (lifts Phase-1 §4.2's autonomous decision). +- One new grammar file: `src/dsl/grammar/sql_select.rs`, + parallel to `sql_expr.rs`, exporting `SQL_SELECT_STATEMENT` + and `SQL_SELECT_COMPOUND`. + +Walker-capability honesty (§10) — the part where ADR-0030 §8's +"ambient assistance comes for free" got softened: + +- **Grammar recursion** still needs nothing new — `Subgrammar` + + ADR-0026's depth cap unchanged. +- **Completion scope** needs a new node variant + `Node::ScopedSubgrammar(&Node)` alongside the existing + `Node::Subgrammar`, plus a `from_scope_stack: Vec` + on `WalkContext`. Each `ScopeFrame` holds `from_scope`, + `cte_bindings`, `projection_aliases`. Existing DSL `Expr` + and `sql_expr` recursion stays on the non-scope-pushing + variant and is unaffected. +- **CTE column resolution** (§10.3): `SELECT *` and explicit- + projection CTE bodies both yield real column completion past + `cte_alias.|` via a body-projection derivation rule that + runs at the body's `ScopedSubgrammar` exit. The earlier + "honest limitation" posture is replaced. +- **Qualified-prefix completion** (§10.5): at `t.|`, candidates + are scoped to `t`'s binding. +- **Projection-before-FROM problem** (§10.6): the debounced + re-walk catches up via a post-walk fixup pass that re- + resolves projection-list identifiers against the final + `from_scope`, rewriting highlight class and validity + diagnostic in the walker's accumulated output. Runs as a + final stage of the walk itself — same convention applies to + the §11.6 predicate-warnings pass. + +Diagnostics (§11) — every Phase-2 validation case classified +against ADR-0027's ERROR/WARNING guideline: + +- **Five new `diagnostic.*` keys** (parse-time-detectable): + `unknown_qualifier`, `ambiguous_column`, + `projection_alias_misplaced`, `cte_arity_mismatch`, + `compound_arity_mismatch`. +- **Eight new `engine.*` keys** for friendly-error layer + translations of engine messages. +- **Three diagnostic passes by end of Phase 2** (§11.7): + schema-existence (extended for multi-binding scope), arity- + check (new), predicate-warnings (extended with a + MatchedPath-walking variant — §11.6). + +Result-column type resolution (§12): + +- `rusqlite` 0.39.0 exposes `column_table_name` / + `column_origin_name` / `column_database_name` on + `RawStatement` behind a `column_metadata` feature. + **Verified** during this session by reading the rusqlite + source. `Cargo.toml` change is one line — add + `"column_metadata"` to the feature list. +- Bare column refs recover their playground type via that + metadata; partially lifts Phase-1 §4.5's bool→0/1 deferral. + +### 2.2. The implementation plan + +`docs/plans/20260520-adr-0032-phase-2.md` (881 lines) breaks +Phase 2 into seven sub-phases: + +- **2a** — Grammar fragment (`sql_select.rs`, no walker + changes yet). +- **2b** — `Node::ScopedSubgrammar` variant + the scope + accumulators on `WalkContext`. +- **2c** — Phase-1 grammar migration: move Phase-1 SELECT + nodes from `data.rs` into `sql_select.rs`; the 7 Phase-1 + integration tests are the safety net. +- **2d** — Diagnostics passes + catalog keys. +- **2e** — Completion + post-walk fixup pass. +- **2f** — Type resolution via rusqlite `column_metadata`. +- **2g** — Integration sweep + the cross-cut verification + matrix (the Phase-1-gap-prevention mechanism). + +Each sub-phase has explicit exit gates with required tests and +a DA-prompt checklist for the gate review. The plan also +encodes the user's standing authorizations: + +- **Walk uninterrupted between gates** unless there's a real + ambiguity, autonomous decision, blocker, or in-the-plan-open- + question trigger. +- **Commits pre-authorized** at gate boundaries and natural + break points within a sub-phase. Standard `area: subject` + message style, detailed body, no AI attribution. +- **Pushes remain user-only.** +- **End-of-plan hand-back** is the one explicit pause: the + verification report + DA PASS verdict get surfaced to the + user before Phase 2 is declared complete. + +The cross-cut verification matrix (29 rows) names every "X +comes for free" claim from ADR-0030/0031/0032 with a test- +location placeholder. The Phase-1 SQL-expression predicate- +warning gap is row 20 by name — making sure an analogous +silent gap cannot ship undetected. + +## §3. The Phase-1 finding worth carrying forward + +While walking ADR-0027 Amendment 1's existing-cases inventory +during this session, the warning-vs-error guideline check +surfaced a **Phase-1 carry-over gap**: + +ADR-0027 Amendment 1's `LIKE`-on-numeric warning — together +with ADR-0026 §7's `= NULL` and type-mismatch warnings — is +emitted by a pass that walks the **DSL `Expr` AST**. Phase 1's +`sql_expr.rs` deliberately builds **no AST** (ADR-0031 §2). +The consequence: **SQL `WHERE` expressions today emit none of +these warnings.** `select * from t where name like 5` parses, +the engine runs it, and the learner gets the engine's verdict, +not the friendly pre-flight nudge Amendment 1 promised. + +Phase 1 shipped structurally green (1260 / 0 / 1) but the +validity indicator was effectively dormant on SQL expressions — +the test suite never asked "does this warning fire on a SQL +WHERE input?" because there was no test for the cross-cut "the +free-for-free promise still holds for SQL" claim. + +ADR-0032 §11.6 closes the gap (Phase 2's diagnostics work +implements a MatchedPath-walking predicate-warnings variant +covering every `sql_expr` slot — WHERE, HAVING, ON, CASE, +projection, ORDER BY) and the implementation plan's cross-cut +matrix has it as a named row to prevent a regression. The plan +is structured so this kind of gap cannot ship undetected again. + +**Worth carrying forward to future phase planning:** every +"comes for free" claim in any ADR is also a candidate for an +unnoticed silent gap. The cross-cut matrix pattern in the plan +generalises. + +## §4. Autonomous decisions during this session + +Per CLAUDE.md, decisions outside ADR-0030's settled scope are +flagged here. None are urgent; all were either confirmed by the +user via explicit AskUserQuestion gates or are routine detail. + +- **Catalog key naming.** `diagnostic.*` for parse-time + diagnostics and `engine.*` for friendly-error translations + — chosen by following the existing pattern + (`diagnostic.unknown_table`, `select.internal_table`); no + new naming convention introduced. +- **Subgrammar push-trigger mechanism** (§10.2) — `Node::Scoped­ + Subgrammar(&Node)` chosen via explicit user + AskUserQuestion gate over two alternatives (flag on existing + variant; driver-side identity registry). Recommended option; + user picked Recommended. +- **CTE body column derivation** (§10.3) — six derivation rules + covering `*`, `t.*`, bare refs, qualified refs, aliased + expressions, and computed-no-alias; first leg / non-recursive + leg dictates for compound / recursive bodies. Per standard + SQL throughout. +- **`Node::ScopedSubgrammar` data model** — `ScopeFrame` is a + struct holding `from_scope` / `cte_bindings` / + `projection_aliases` together rather than three parallel + stacks. Cleaner data model; no semantic difference. Settled + during writing. + +Every scope question that affected expressiveness or surface +area went through an `AskUserQuestion` gate. The full list of +those gates appears in the conversation log; the answers shape +the ADR's §§1–10 directly. + +## §5. What's next — Phase 2 implementation + +Per ADR-0030's phasing, Phase 2 is the next slice. The user +has signed off on the plan and authorized walking it without +interruption between gates. The implementer for Phase 2 should: + +1. **Read** in order: this handoff, ADR-0032, the Phase 2 + plan, then `CLAUDE.md` (working-style rules unchanged) and + `docs/requirements.md` (`Q1`/`Q2` advance further; `Q4` + already ticked). +2. **Begin sub-phase 2a** — the grammar fragment. The plan's + "Scope (in)" / "Build steps" / "Exit gate" / "DA gate" sections + for 2a are the spec. +3. **Commit at gate boundaries** with detailed messages in the + `area: subject` style (recent precedent: `grammar: SQL + SELECT end-to-end (ADR-0030 Phase 1)`, etc.). +4. **Escalate when the plan says to** — see §2 of the plan's + "Standing authorizations" for the four trigger conditions. + The plan's "Open questions to escalate before code starts" + section pre-identifies three known triggers (subquery type- + resolution through scalar subqueries; duplicate CTE-name + detection; function name allowlist). +5. **At end of 2g**, produce the verification report and + surface to the user; do NOT declare Phase 2 complete + without the user's confirmation. This is the one explicit + pause-point per §5 of the plan's standing authorizations. + +After Phase 2 ships and is accepted, ADR-0030 Phase 3 (DML — +INSERT / UPDATE / DELETE in SQL) is the next focus area. + +## §6. Seams for the implementer + +These are the file-and-section pointers for Phase 2 work, +copied across from the plan for fast access: + +- **`src/dsl/grammar/sql_select.rs`** (new) — the §1 grammar. + Parallel to `sql_expr.rs` (ADR-0031). Exports + `SQL_SELECT_STATEMENT` (the full statement with optional + `WITH`) and `SQL_SELECT_COMPOUND` (the embedded form + subqueries recurse into). +- **`src/dsl/grammar/sql_expr.rs`** — three additive `Choice` + branches (scalar subquery `primary`, `IN (subquery)`, `[NOT] + EXISTS (subquery)`) and one additive `name_or_call` tail + (qualified ref `t.c`). All recurse through + `Node::ScopedSubgrammar(&SQL_SELECT_COMPOUND)`. +- **`src/dsl/grammar/mod.rs`** — `Node` enum gets the new + `ScopedSubgrammar(&'static Node)` variant alongside the + existing `Subgrammar`. +- **`src/dsl/walker/context.rs`** — `WalkContext` gains + `from_scope_stack: Vec`. `current_table` / + `current_table_columns` become derived helpers over the top + frame (DSL paths must stay green — the existing 1240+ test + surface verifies this). +- **`src/dsl/walker/driver.rs`** (or wherever the Subgrammar + match arm lives) — new arm for `Node::ScopedSubgrammar` that + pushes/pops a `ScopeFrame`. +- **`src/dsl/walker/mod.rs`** — `schema_existence_diagnostics` + extended to walk every `from_scope` binding; new + arity-check pass; existing `predicate_warnings` gets a + MatchedPath-walking variant for `sql_expr` slots. +- **`src/db.rs`** — `do_run_select` calls a new + `resolve_select_column_types` helper before constructing the + `DataResult`. The helper uses + `RawStatement::column_table_name(i)` / + `column_origin_name(i)` per result column. +- **`Cargo.toml`** — add `"column_metadata"` to rusqlite's + feature list. One-line change. +- **`src/dsl/grammar/data.rs`** — Phase-1 SQL SELECT static + nodes either move into `sql_select.rs` or get removed + outright (the `data::SELECT` `CommandNode` is rebuilt + against `SQL_SELECT_STATEMENT`); see plan sub-phase 2c. +- **Catalog file** (wherever the i18n keys live; the existing + `diagnostic.unknown_table` neighbours) — five new + `diagnostic.*` keys + eight new `engine.*` keys per ADR-0032 + §11.5. + +## §7. How to take over + +1. **Read this file, then `docs/adr/0032-sql-select-grammar.md` + and `docs/plans/20260520-adr-0032-phase-2.md`.** Then + `CLAUDE.md` and `docs/requirements.md`. +2. **`cargo test`** — expect 1260 passing, 0 failed, 1 ignored + (the baseline). +3. **`cargo clippy --all-targets -- -D warnings`** — clean. +4. **Start sub-phase 2a** per the plan. Standing + authorizations (plan §§1–5) apply. +5. **Escalate per CLAUDE.md when the plan's triggers fire; + never decide silently** on a question the ADR/plan doesn't + settle. + +## §8. What else is open + +Unchanged from handoff-25 §8 (prioritisation is a **user +decision**): snapshot/undo `U`-series (ADR-0006 written, +unbuilt); m:n convenience `C4`; modify-relationship `C3a`; +`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`. + +ADR-0030's later phases — DML (Phase 3), DDL (Phase 4), the +DSL→SQL teaching echo (Phase 5), polish (Phase 6) — remain on +the roadmap behind Phase 2. + +Phase-1's specific deferrals (§4.1–§4.5 in handoff-25) are +addressed as follows by ADR-0032: + +- **§4.1 — `FROM` optional.** Stands. ADR-0032 §1 notes + `SELECT 1` and `SELECT upper('x')` continue to parse. +- **§4.2 — implicit projection aliasing (`select a x`).** + Lifted by ADR-0032 §1 — admitted in Phase 2. +- **§4.3 — `help_id: None` on SELECT.** Stands; Phase 6. +- **§4.4 — walker entries defaulting to internal `Mode::Simple`.** + Stands; revisited when advanced mode grows its own ambient + assistance (Phase 6). +- **§4.5 — bool SELECT results render as `0`/`1`.** Partially + lifted by ADR-0032 §12 — bare bool column refs now recover + their type via engine column-origin metadata. Computed + bool expressions stay typeless until a future enhancement.