# Implementation plan — ADR-0032 (ADR-0030 Phase 2) **Plan author date:** 2026-05-20 **Driving ADR:** [ADR-0032 — The full SQL `SELECT` grammar](../adr/0032-sql-select-grammar.md) **Parent ADR:** [ADR-0030 — Advanced mode: the standard-SQL surface](../adr/0030-advanced-mode-sql-surface.md) ## Purpose ADR-0032 is the *decision*: what Phase 2 covers, how it composes, and what consequences fall out. This plan is the *mechanics*: the build order, the per-step exit gates, the cross-cut verifications that prevent the kind of silent gap Phase 1 shipped (the SQL-expression predicate-warning gap closed in ADR-0032 §11.6). The plan does **not** re-litigate ADR-0032's decisions. Any finding during implementation that suggests a decision should change is escalated to the user and resolved by ADR amendment (or a new ADR) before code proceeds — never by silent drift. ## Why this plan exists Phase 1 of ADR-0030 shipped with a structurally green test suite (1260 passing, 0 failing) and a believed-but-unverified "ambient assistance comes for free" claim. The verification *missed* the fact that ADR-0027's predicate warnings — emitted by a pass walking the DSL `Expr` AST — could not fire on SQL expressions because `sql_expr.rs` builds no AST (ADR-0031 §2). ADR-0032 §11.6 documents the gap and closes it; this plan exists to make sure Phase 2 does not ship with an analogous unnoticed gap. The two failure modes the plan is built to prevent: - **The "free for free" claim shipping without a test that proves the claim.** Every "X works for SQL automatically" in ADR-0030/0031/0032 needs an explicit cross-cut test. - **A sub-phase reporting green because its own scope is covered, while a behaviour it enables for downstream code is silently broken.** Each sub-phase's exit gate includes the cross-cut behaviours it unlocks, not just its own internal correctness. ## Working-mode reminder Per `CLAUDE.md`'s "Working mode — solo with sub-agents by default": this work proceeds in solo mode unless the user explicitly invokes `/team`. The phased structure below maps to solo-mode's Phase 1 → 5 discipline within each sub-phase: - Requirements extraction (the sub-phase's scope below) is Phase 1. - Divergent exploration is rarely needed for individual sub-phases here (ADR-0032 already selected the approach), but if implementation reveals an ambiguity, it lands as a Phase-2 exploration before code proceeds. - The DA hat is worn at every sub-phase gate. The DA critique is written, not silent (CLAUDE.md "Devil's Advocate discipline"). - Escalation rather than guessing applies throughout. ## Standing authorizations and continuation policy The user has granted standing authorizations for this plan, overriding the corresponding `CLAUDE.md` defaults *for this plan's scope only*. They are recorded here verbatim so any implementer (the original session, a future session, or a fresh agent) operates on the same terms. ### 1. Walk the plan without interruption when no interruption is needed The plan's sub-phases (2a–2g) have explicit, written exit gates. Within a sub-phase, the implementer proceeds through the build steps and verifies the gate without asking for permission at each step. Between sub-phases, the implementer proceeds to the next one without asking, *provided* the previous gate is green. The implementer does NOT interrupt to: - Confirm "should I continue to the next sub-phase?" - Confirm minor implementation choices that match surrounding code (`CLAUDE.md`'s code-style discipline applies; pick the consistent option and move on). - Confirm test naming, file organisation, commit granularity within a sub-phase. - Report progress mid-step. A status update arrives at gate boundaries — not every five minutes. ### 2. When the implementer MUST escalate The escalation rule from `CLAUDE.md` ("Escalate ambiguity — do not decide for the user") is **not** relaxed. Standing authorization to walk uninterrupted is *not* authorization to guess on design questions. The implementer escalates if: - An exit gate cannot be made green by work in the current sub-phase's scope — a test fails for a reason that suggests the ADR is wrong, or a behaviour the gate requires turns out to need machinery not in ADR-0032. - An ambiguity arises that the ADR and this plan do not settle. See the "Open questions to escalate before code starts" section at the end of this plan — those are pre-identified. Anything similar that surfaces is also escalation territory. - An autonomous design decision is required (a choice outside ADR-0032 §§1–13's settled list and the plan's per-sub-phase scope). Per `CLAUDE.md`'s "Out of scope and non-blocking require user confirmation": the default assumption is that the work is in scope and is blocking unless the user has said otherwise. - A "real blocker" is encountered — environment, dependency, or test-infrastructure failure that prevents progress and cannot be resolved within the sub-phase. (A flaky test is not a real blocker — investigate per `CLAUDE.md`'s "no excuses" rule on test infrastructure.) ### 3. Commits are pre-authorized at standard granularity The user has explicitly pre-authorized commits within this plan's scope, overriding `CLAUDE.md`'s "All commits MUST be confirmed by the user first" for this plan. The implementer: - **Commits at natural break points within each sub-phase** and always at sub-phase gate boundaries. Granular is better than monolithic. - **Writes detailed commit messages** in the project's existing style — `area: short subject` first line, then a body explaining what changed and why. Recent precedent: ``` grammar: SQL SELECT end-to-end (ADR-0030 Phase 1) tests: Phase 1 SQL SELECT integration tests app: mode-threaded completion, overlay, and validity indicator ``` Body should reference the ADR section being implemented (e.g., "ADR-0032 §10.2 — Node::ScopedSubgrammar variant added") so a reader can trace each commit back to a decision. - **Includes NO AI attribution** of any form (`Co-Authored-By`, generated-by lines, AI footers, …). This is a global rule (`CLAUDE.md`); the standing authorization here is for *committing*, not for relaxing attribution discipline. ### 4. Pushes remain a user step `CLAUDE.md`'s "Push is a user step — agents cannot and shall never do it" is **not** overridden. The implementer commits freely; the user pushes when they choose. Unpushed commits are a normal working state, and the implementer never describes work as blocked on a push. ### 5. End-of-plan handoff At the end of 2g, after the verification report is produced and the DA's final PASS verdict is recorded, the implementer **stops and surfaces a summary to the user** — exact test counts, the filled cross-cut matrix, the requirements-to-test mapping, any autonomous decisions made and their justification. The user confirms acceptance before the plan is considered done. (This is the one explicit hand-back point.) If the user wants follow-up work — adjustments, polish, additional tests — the implementer addresses it and re-runs the gate before re-surfacing. The plan does not declare itself complete; the user does. ## Sub-phase overview | Sub-phase | Title | Test-suite gate | |-----------|--------------------------------------|-------------------------------------------------| | **2a** | Grammar fragment | All §1 productions accept; OOS shapes reject | | **2b** | `ScopedSubgrammar` + scope accumulators | Subquery scope isolation; CTE self-reference visible | | **2c** | Phase-1 grammar migration | All 7 Phase-1 SQL SELECT tests still green | | **2d** | Diagnostics passes | Every catalog key has positive + negative tests; Phase-1 gap closure verified | | **2e** | Completion + post-walk fixup | Per-slot completion matrix green; projection-before-FROM scenario verified | | **2f** | Type resolution | Each of 10 playground types preserved through bare ref | | **2g** | Integration sweep + cross-cut verification | All cross-cut tests green; final verification report | Each sub-phase ships independently: green tests, clippy clean, a written DA gate review, and (per project rules) a user-confirmed commit message before commit. --- ## Sub-phase 2a — Grammar fragment ### Scope (in) - Author `src/dsl/grammar/sql_select.rs` exporting `pub static SQL_SELECT_STATEMENT: Node` and `pub static SQL_SELECT_COMPOUND: Node`. - Encode the full §1 BNF: `with_clause`, `compound_select`, `select_core`, the projection list with `DISTINCT`/`ALL`/`*`/ `t.*`/bare-alias, the `from_clause`/`join_clause` chain with the five JOIN flavours, `where_clause`, `group_by_clause`, `having_clause`, `order_by_clause`, `limit_clause` with `OFFSET`. - Wire `sql_expr::SQL_OR_EXPR` into every expression slot via `Subgrammar`. - Author the structural one-line hint key per slot (per ADR-0032 Consequences "Hint-panel prose" bullet); the keys are placed in the catalog with placeholder text. Rich teaching prose is Phase 6. - Encode the `reject_internal_table` validator on every new table-source slot. ### Scope (out — explicit) - The `ScopedSubgrammar` variant — that is 2b. - The Phase-1 `data::SELECT` migration — that is 2c. - Any new `WalkContext` field — that is 2b. ### Build steps 1. Stub `sql_select.rs` with the static node skeleton and re-export through `grammar/mod.rs`. 2. Author each production tier as a named `static Node` in loosest-to-tightest order, matching the §1 BNF. 3. For each production, add a unit test in the file (the `expr.rs` / `sql_expr.rs` test pattern) that walks a representative input. 4. Author the OOS reject tests (NATURAL JOIN, comma-FROM, `LIMIT m, n`, `VALUES`, `LATERAL`, window-function `OVER`). 5. `cargo test` and `cargo clippy --all-targets -- -D warnings`. ### Exit gate **Required green tests:** - Every JOIN flavour (`INNER`, bare `JOIN`, `LEFT [OUTER]`, `RIGHT [OUTER]`, `FULL [OUTER]`, `CROSS`) accepts. - Every set op (`UNION`, `UNION ALL`, `INTERSECT`, `EXCEPT`) accepts in a compound chain. - Both CTE forms (`WITH`, `WITH RECURSIVE`) accept; both column- list and no-column-list shapes accept. - `DISTINCT`/`ALL` accepts and rejects each other co-occurring. - `*`, `t.*`, bare-alias projection, `AS alias` projection all accept. - Qualified column refs (`t.c`) accept in WHERE/projection/ ORDER BY/HAVING/GROUP BY. - `LIMIT n` and `LIMIT n OFFSET m` both accept; `LIMIT m, n` rejects. - Every §13 OOS-1 to OOS-7 shape rejects with a parse error (not a silent accept). **Required negative tests:** absence of bare `RECURSIVE` (must be `WITH RECURSIVE`); absence of `JOIN` after `INNER` (must be `INNER JOIN`); etc. — the obvious parse-failure modes for each production. **Test-suite count:** ≥ 50 new unit tests in `sql_select.rs` (rough order; the actual count is dictated by production coverage). **Other gates:** - `cargo test` total: 1260 (baseline) + ≥ 50 (new) = ≥ 1310. Zero failures, one ignored (the unchanged doctest), zero skipped. - `cargo clippy --all-targets -- -D warnings` clean. ### DA gate (written) - Does every §1 BNF production have at least one positive test? - Does every §13 OOS shape have at least one reject test? - Did the implementation introduce any structure not covered by ADR-0032 §1 or §10–§11 (an autonomous decision)? If yes, escalate to the user before the gate is signed. - Are the OOS rejects emitting *engine-neutral* parse errors, or are they leaking SQLite messages? --- ## Sub-phase 2b — `ScopedSubgrammar` + scope accumulators ### Implementation status (2026-05-20) Sub-phase 2b shipped in five commits (`4f89106`, `98a74b2`, `b522d09`, `4ff054c`, and earlier 2a foundations). The scope-accumulator infrastructure — `Node::ScopedSubgrammar`, `ScopeFrame`, `from_scope_stack`, the new Ident flags (`writes_table_alias`, `writes_cte_name`, `writes_projection_alias`), and the sql_expr §5 / §6 additive extensions — is complete and exercised by 26 driver-level tests. **Deferral, explicitly user-approved:** §10.3 stage 2 (the six CTE output-column derivation rules) is NOT implemented in 2b. Stage 1 (placeholder CTE binding push) IS implemented; stage 2 will fold into 2d (where the new arity-check pass needs declared-vs-derived column counts) and 2e (where qualified- prefix completion needs CTE columns). Until then, a CTE binding's `columns` stays empty after the body exits, and qualified-prefix completion past `cte_alias.|` returns an empty candidate list. The CTE-name is still visible as a table source from inside the body (WITH RECURSIVE self-reference works) and from outside (downstream CTE-name validators see it). The 2g cross-cut matrix rows for §10.3 derivation cases are deferred along with the harvest; they will land when 2d/2e implements the rules. ### Scope (in) - Add the new walker node variant `Node::ScopedSubgrammar(&Node)`. - Add the `ScopeFrame` struct and `from_scope_stack: Vec` to `WalkContext`. Add `from_scope`, `cte_bindings`, `projection_aliases` as fields *inside* `ScopeFrame`. - Teach the walker driver to push/pop a fresh frame on `ScopedSubgrammar` entry/exit. - Rewire every Subgrammar reference to `&SQL_SELECT_COMPOUND` in `sql_expr.rs` and `sql_select.rs` to use the new variant. - Teach `from_clause` / `join_clause` walks to populate the current frame's `from_scope`. - Teach `with_clause` walks to populate the outer frame's `cte_bindings` (placeholder push pre-body per §10.3 stage 1). - Teach `projection_item` walks to populate the current frame's `projection_aliases`. - Implement §10.3 stage 2: at the CTE body's frame exit, derive output columns from the body's projection and rewrite the placeholder binding in the outer frame. - Keep `current_table` / `current_table_columns` as derived helpers (top-frame single-binding view) — verify the DSL paths still see them correctly. ### Scope (out — explicit) - Qualified-prefix completion narrowing — that is 2e. - The post-walk fixup pass for projection identifiers — that is 2e. - The arity-check ERROR pass (uses the frame-exit hook from §10.3 stage 2 but adds a new pass) — that is 2d. ### Build steps 1. Add the variant to `Node`; add walker driver match arm (push, recurse, pop). 2. Add `ScopeFrame` and `from_scope_stack` to `WalkContext`; rewrite `current_table` / `current_table_columns` accessors to derive from the top frame. 3. Rewire Subgrammar references to ScopedSubgrammar in `sql_select.rs` (subquery primary, IN/EXISTS tails, CTE bodies — except for the ladder-internal recursions in `sql_expr.rs` that stay as `Subgrammar`). 4. Teach the `from_clause` / `join_clause` walker handlers to build and push `TableBinding`s. 5. Teach the `with_clause` handler to push the placeholder `CteBinding` before entering the body. 6. Teach the `projection_item` handler to record aliases. 7. Implement the body-frame-exit hook that derives CTE output columns per §10.3 stage 2 rules. 8. Unit tests for each. ### Exit gate **Required green tests:** - An empty query through the walker still produces the baseline output (no spurious push). - A DSL `UPDATE T …` still sets `current_table` correctly via the derived helper (DSL paths untouched). - A subquery's FROM does not leak: parse `SELECT * FROM a WHERE id IN (SELECT id FROM b)`, then probe a cursor outside the subquery — only `a` is in `from_scope`. - A correlated reference resolves: parse `SELECT * FROM a WHERE EXISTS (SELECT 1 FROM b WHERE b.x = a.x)`, then probe inside the subquery — `a` is visible via outer frame. - A CTE name is visible inside its own body: parse `WITH r AS (SELECT 1 UNION ALL SELECT n+1 FROM r) SELECT * FROM r`, then probe inside the body — `r` is in `cte_bindings`. - A CTE body's `SELECT *` derives the right output columns: parse `WITH x AS (SELECT * FROM users) …`, verify `x`'s derived columns match `users`' columns by name and playground type. - A CTE body's `SELECT a, b AS bb FROM t` derives two columns with the right names and types (`a`: source type; `bb`: source type since underlying is a bare ref). - A CTE body's `SELECT a + 1 FROM t` derives one column with `name = None`. - A CTE column list `(c1, c2)` renames positionally; arity mismatch is detectable at frame exit (the diagnostic is emitted by 2d but the *detection* runs here). - The depth cap (`MAX_SUBGRAMMAR_DEPTH = 64`) still fires on pathological nesting — exercised through both variants. **Other gates:** - All 2a tests still green. - All 1260 baseline tests still green (DSL paths verified insulated). - `cargo clippy --all-targets -- -D warnings` clean. ### DA gate (written) - Did any DSL test regress? If yes, the `current_table` helper is not deriving correctly — fix before gate signs. - Is the `ScopedSubgrammar` push/pop happening on every entry and every exit, including error paths? Read the walker driver's match arm and verify. - Does the CTE column derivation handle all six §10.3 rules? Walk the rule table and find a test for each. - Did 2b introduce any structure ADR-0032 doesn't sanction? --- ## Sub-phase 2c — Phase-1 grammar migration ### Scope (in) - Move the Phase-1 SQL `SELECT` grammar nodes from `src/dsl/grammar/data.rs` into `src/dsl/grammar/sql_select.rs` (or remove them if redundant with the new §1 productions). - Rebuild the `data::SELECT` `CommandNode` so its body is a reference to `SQL_SELECT_STATEMENT`. - Confirm the seven Phase-1 SQL `SELECT` integration tests still pass without modification. ### Scope (out — explicit) - Any behaviour change. This is a refactor: the Phase-1 surface must be preserved exactly. ### Build steps 1. Identify the Phase-1 SQL `SELECT` static nodes in `data.rs` (`SELECT_PROJECTION_*`, `SELECT_WHERE_*`, `SELECT_ORDER_*`, `SELECT_LIMIT_*` if separate, plus any sub-tree they reference). 2. Move them into `sql_select.rs` or remove if the §1 productions cover them. 3. Rewrite the `data::SELECT` `CommandNode` to reference `SQL_SELECT_STATEMENT`. 4. Run the seven Phase-1 SQL `SELECT` integration tests. ### Exit gate **Required green tests:** - All seven Phase-1 SQL `SELECT` integration tests (`tests/sql_select_*.rs` or equivalent) pass without modification. - All 2a and 2b tests still green. - All 1260 baseline tests still green. **Other gates:** - `cargo clippy --all-targets -- -D warnings` clean. - Dead code: confirm via `cargo check` that the old `data.rs` Phase-1 SELECT nodes are either removed or re-exported, not orphaned. ### DA gate (written) - Were any Phase-1 SELECT behaviours quietly altered? Diff the Phase-1 integration test outputs (text and structural) — they must be identical, not "equivalent". - Did the migration produce dead code? Removing dead code is fine; *leaving* it isn't. --- ## Sub-phase 2d — Diagnostics passes ### Scope (in) - Implement the three diagnostic passes per ADR-0032 §11.7: - Schema-existence ERROR pass (extended for multi-binding `from_scope`, qualified-reference resolution, ambiguity detection). - Arity-check ERROR pass (new, hooked to CTE-body and compound-query frame-exits from 2b). - Predicate-warnings pass (extended with a MatchedPath-walking variant for `sql_expr` per §11.6). - Author the five new `diagnostic.*` catalog keys (§11.5): `unknown_qualifier`, `ambiguous_column`, `projection_alias_misplaced`, `cte_arity_mismatch`, `compound_arity_mismatch`. - Author the eight new `engine.*` catalog keys for friendly- error translations (§11.5). ### Scope (out — explicit) - The post-walk projection-list fixup pass (§10.6) — that is 2e. - Detection of engine-rejected cases (§11.4) — those stay engine-side. ### Build steps 1. Extend the schema-existence pass to iterate every `from_scope` binding and the active `cte_bindings`. 2. Add qualified-reference resolution: when a `name_or_call` match has a `'.' identifier` tail, resolve the qualifier against `from_scope` aliases first, then table names. 3. Add ambiguity detection: an unqualified column ref that resolves into multiple bindings. 4. Add projection-alias-misplaced check: walk `sql_expr` slots in WHERE/GROUP BY/HAVING; identifier matches against the current frame's `projection_aliases` → ERROR. 5. Author the arity-check pass: at every `ScopedSubgrammar` exit, if the frame is a CTE body, compare declared vs derived column counts; if the parent context is a compound-query chain, compare against the previous leg's arity. 6. Author the MatchedPath-walking predicate-warnings variant: identify `LIKE` / `=`/`!=` with `NULL` / mismatched-type comparison predicate-tails by node-name labels; emit the corresponding catalog key with the offending operand's span. 7. Author the catalog entries in the i18n catalog file with engine-neutral wording. 8. Author tests per §11.2 and §11.6. ### Exit gate **Required green tests:** - **One positive and one negative test per new ERROR catalog key** (the negative confirms the key does *not* fire on a well-formed equivalent): - `diagnostic.unknown_qualifier`: `SELECT t.c FROM u` (where `t` is not in scope) fires; `SELECT u.c FROM u` doesn't. - `diagnostic.ambiguous_column`: `SELECT id FROM a JOIN b ON a.id = b.id` fires for the bare `id`; `SELECT a.id FROM a JOIN b ON a.id = b.id` doesn't. - `diagnostic.projection_alias_misplaced`: `SELECT a + b AS x FROM t WHERE x > 0` fires; `… ORDER BY x` doesn't (alias is bound by ORDER BY time). - `diagnostic.cte_arity_mismatch`: `WITH x(a, b) AS (SELECT 1) SELECT * FROM x` fires; matched arity doesn't. - `diagnostic.compound_arity_mismatch`: `SELECT 1, 2 UNION SELECT 1` fires; matched arity doesn't. - **The Phase-1 gap closure** explicitly verified: `SELECT * FROM products WHERE price LIKE 5` fires `diagnostic.like_numeric` (the test that would have caught the Phase-1 gap). Sister tests: `WHERE name = NULL` fires `diagnostic.eq_null`; `WHERE age = 'old'` fires `diagnostic.type_mismatch`. - **Predicate warnings extend across every SQL slot**: the same `LIKE`-on-numeric pattern fires in `HAVING`, `ON`, a `CASE WHEN` clause, a projection item, and an `ORDER BY` item — one test per slot. - Engine.* keys: integration tests that drive the engine to the error condition verify the friendly-error layer surfaces the engine-neutral wording (one test per key, run-and-assert shape). **Other gates:** - All 2a–2c tests still green. - All 1260 baseline tests still green. - Existing DSL predicate warnings (the DSL `Expr` AST variant) still fire correctly — verify by re-running the DSL test suite section that exercises them. - `cargo clippy --all-targets -- -D warnings` clean. ### DA gate (written) - Does every new catalog key have *both* a positive and a negative test, or did any slip in with only one? - Was the Phase-1 gap closure verified, or did the test slip past in "well, it should work now"? - Are the engine.* messages truly engine-neutral, or does any one leak SQLite product/feature names? (ADR-0030 §7.) - Did 2d cover every Phase-2 case in §11.2, or were any silently downgraded to "engine handles"? --- ## Sub-phase 2e — Completion + post-walk fixup ### Scope (in) - Add qualified-prefix completion narrowing (§10.5): when the matched path immediately preceding an `IdentSource::Columns` slot ends with `Ident '.'`, the completer narrows candidates to the named binding's columns. - Implement the post-walk fixup pass for projection-list identifiers (§10.6): collect projection `Ident` terminals during the walk; after walk completion, re-resolve each against the final `from_scope`; rewrite the highlight class and validity diagnostic in the walker's accumulated output. - The pass runs as a final stage of the walk itself (§10.6 integration-point convention). ### Scope (out — explicit) - Completion behaviour beyond column-name narrowing (keyword completion / function names / table names) — those come for free from the walker's expected-set machinery and need only cross-cut verification, not new code. ### Build steps 1. Add a "prefix qualifier" hint to the completion API that the walker passes when the cursor is immediately after `Ident '.'`. 2. Extend `IdentSource::Columns` completion to honour the qualifier hint when present. 3. Implement the projection-`Ident` recording during the walk (append a small struct per projection identifier to a walker-local list). 4. Implement the fixup pass that runs after the main walk: re-resolve each recorded identifier; rewrite the highlight run; update the diagnostics vector. 5. Tests per the exit gate. ### Exit gate **Required green tests:** - **Qualified-prefix completion:** at the cursor in `SELECT t.|`, candidates are `t`'s columns; at `SELECT u.|` where `u` is not in scope, candidates are empty. - **Alias-prefix completion:** at the cursor in `SELECT FROM a AS x JOIN b AS y ON x.|`, candidates are `x`'s (i.e. `a`'s) columns. - **Qualified-prefix narrowing for CTE:** at `WITH x AS (SELECT a, b FROM t) SELECT x.|`, candidates are `a` and `b` (the derived columns). - **Qualified-prefix narrowing for CTE with `SELECT *`:** at `WITH x AS (SELECT * FROM users) SELECT x.|`, candidates are `users`' columns (per §10.3 derivation). - **Projection-before-FROM:** typing `SELECT col1, col2` produces a generic-identifier highlight on `col1`/`col2` (no `from_scope` to resolve against); typing ` FROM t` after produces a column highlight on `col1`/`col2` if they exist in `t`, or an unknown-identifier diagnostic if they don't — verified within one debounce/walk cycle. - **`ORDER BY` alias resolution:** parses `SELECT a + b AS total FROM t ORDER BY total`; the alias resolves without a fixup pass (single-pass walk handles it per §10.6). - **Completion across every Phase-2 slot:** Tab at `WHERE |`, `GROUP BY |`, `HAVING |`, `ON |`, projection `|`, and `ORDER BY |` all offer expected candidates from the union of `from_scope` bindings. **Other gates:** - All 2a–2d tests still green. - All 1260 baseline tests still green. - `cargo clippy --all-targets -- -D warnings` clean. ### DA gate (written) - Does the projection-before-FROM verification actually exercise the full re-walk cycle, or does it just call the fixup pass directly? (The user-observable behaviour is the re-walk on debounce; that is what the test must exercise.) - Does the qualified-prefix narrowing produce empty candidates when the qualifier doesn't resolve, or does it accidentally fall back to the global column set? - Did 2e introduce any priority/ordering for outer-frame candidates? ADR-0032 §10.5 explicitly says ordering is completion-tier polish, not specified here — if 2e added priority, that is an autonomous decision and escalates. --- ## Sub-phase 2f — Type resolution ### Scope (in) - Add `"column_metadata"` to the rusqlite feature list in `Cargo.toml` (alongside `"bundled"`). - Implement the worker-side type-resolution helper that runs after `prepare`, before iterating rows: for each result column, query `RawStatement::column_table_name(i)` / `column_origin_name(i)`; if both return Some, look up the playground type from `SchemaCache.columns_for_table`; otherwise leave the slot as `None`. - Call the helper from `do_run_select` before constructing the `DataResult`. ### Scope (out — explicit) - Any new walker / grammar work. This is a worker-side change only. ### Build steps 1. Update `Cargo.toml`. `cargo build` to confirm the feature resolves through `libsqlite3-sys` and the bundled SQLite is rebuilt with `SQLITE_ENABLE_COLUMN_METADATA`. 2. Add the resolver method on `Database` (probably `resolve_select_column_types(&Statement, &SchemaCache) -> Vec>`). 3. Wire into `do_run_select`. 4. Tests per the exit gate. ### Exit gate **Required green tests:** - **One test per playground type** (all ten: `text`, `int`, `real`, `decimal`, `bool`, `date`, `datetime`, `blob`, `serial`, `shortid`) — a bare-column SELECT of a column of that type produces `column_types[0] = Some()` and the renderer formats accordingly. - **`bool` renders as `true`/`false`** (the pedagogically- important case lifting Phase-1 §4.5). - **Computed expression stays typeless:** `SELECT a + 1 FROM t` produces `column_types[0] = None`; the renderer uses neutral alignment. - **Multi-column mixed test:** `SELECT name, age, is_active, age + 1 FROM users` produces `[Some(text), Some(int), Some(bool), None]`. - **CTE pass-through:** `WITH x AS (SELECT name FROM users) SELECT name FROM x` recovers `Some(text)` — this verifies the engine's column-origin metadata follows through the CTE in the bundled SQLite build (verified, not assumed). - **Subquery result:** `SELECT (SELECT name FROM users WHERE id = 1)` — the engine's column-origin metadata may or may not follow through a scalar subquery; whichever it does, the test asserts the actual behaviour (not the wished-for behaviour) and an explanatory comment captures the engine's posture. - **Unknown-table SELECT:** `SELECT * FROM nonexistent` — the resolver does not panic; the parse-time ERROR already fired in 2d. **Other gates:** - All 2a–2e tests still green. - All 1260 baseline tests still green. - `cargo clippy --all-targets -- -D warnings` clean. ### DA gate (written) - Does the resolver handle every result-column case (named column, expression, NULL literal, CTE pass-through) without panicking? - Is the rusqlite feature flag in place? `grep column_metadata Cargo.toml`. - Did 2f surface any cases where engine-side metadata *doesn't* follow through (CTEs, views, subqueries)? Each such finding must be documented as an honest limitation, with the test capturing the actual behaviour. --- ## Sub-phase 2g — Integration sweep + cross-cut verification ### Scope (in) The catch-all sub-phase. Verifies the full end-to-end Phase 2 surface and every cross-cut "X comes for free" claim. Produces the **final phase-exit verification report** (next section). ### Build steps 1. Author the end-to-end integration tests (Tier 3) for real-world-shape queries (the exit-gate list below). 2. Author or extend the cross-cut verification matrix (next section) for every "X comes for free" claim from ADR-0030/0031/0032. 3. Run the full test suite. Confirm 1260 baseline + all sub-phase 2a–2f additions + the 2g integration tests. 4. Run `cargo clippy --all-targets -- -D warnings`. 5. Produce the final verification report. ### Exit gate **Required end-to-end integration tests:** - **JOIN with WHERE:** `SELECT a.name, b.total FROM customers a JOIN orders b ON a.id = b.customer_id WHERE a.country = 'DE'` — runs, returns expected rows, the data-table renderer emits a clean output. - **Recursive CTE:** a small tree traversal (e.g., generate integers 1..10 via `WITH RECURSIVE`) — runs, returns 10 rows, renderer clean. - **UNION of two SELECTs:** runs, returns the union, renderer clean. - **Correlated EXISTS subquery:** `SELECT name FROM customers c WHERE EXISTS (SELECT 1 FROM orders o WHERE o.customer_id = c.id)` — runs, returns expected rows, renderer clean. - **`history.log` replay:** every Phase-2 statement form written above replays from the log faithfully (ADR-0030 §11 unchanged). - **`reject_internal_table`:** `FROM __rdbms_columns` parse- rejects; `WITH __rdbms_x AS (…)` parse-rejects; a `FROM __rdbms_*` inside a CTE body parse-rejects. **Cross-cut verification matrix** (next section): every claim must have a passing test. **Other gates:** - Total test count: ≥ 1310 + (additions from 2b–2g). The exact count depends on per-step authorship; the gate is "every required-test row above and in the cross-cut matrix has at least one green test", not a magic number. - Zero failures, zero skipped (excluding the unchanged doctest). - `cargo clippy --all-targets -- -D warnings` clean. ### DA gate (written) The final DA review, per CLAUDE.md Phase-5 verification: - Does every requirement on the ADR-0032 checklist have a test? Walk the next-section matrix row by row. - Are there any "free for free" claims from ADR-0030/0031/0032 that have no explicit test? (The Phase-1 gap pattern.) Each must have one. - Did any sub-phase silently downgrade a requirement to "later"? Phase-1 §4.1–§4.5 were autonomous decisions flagged at handoff; any Phase-2 autonomous decision must also be flagged with the same discipline. - Are ALL tests green AND zero-skipped? If not, the verdict is FAIL and 2g loops back. --- ## Cross-cut verification matrix Each row is a claim from ADR-0030, ADR-0031, or ADR-0032 that needs an explicit test, with the test's location. The matrix is filled in during 2g; the gate is "every row green". | Claim source | Claim | Test location | Status | |---------------------------|---------------------------------------------------------------|---------------|--------| | ADR-0030 §8 | Syntax highlighting works for SQL keywords | (TBD) | (TBD) | | ADR-0030 §8 | Tab completion works for SQL keywords | (TBD) | (TBD) | | ADR-0030 §8 | Hint-panel prose appears at every SQL grammar slot | (TBD) | (TBD) | | ADR-0030 §8 | `[ERR]`/`[WRN]` validity indicator fires for SQL | (TBD) | (TBD) | | ADR-0030 §8 | Per-command parse-error usage fires for SQL | (TBD) | (TBD) | | ADR-0030 §9 | Out-of-subset construct (`OVER (…)`) produces engine-neutral parse error | (TBD) | (TBD) | | ADR-0030 §11 | Every Phase-2 SQL statement form logs to `history.log` and replays | (TBD) | (TBD) | | ADR-0031 §5 | Highlighting works for SQL expression operators / `CASE` / function calls | (TBD) | (TBD) | | ADR-0031 §5 | Column completion works for `IdentSource::Columns` slots in `sql_expr` | (TBD) | (TBD) | | ADR-0031 §5 | Hint prose appears at every `sql_expr` slot | (TBD) | (TBD) | | ADR-0032 §10.2 | DSL `Subgrammar` recursion (ADR-0026) does NOT push scope | (TBD) | (TBD) | | ADR-0032 §10.2 | `sql_expr` ladder `Subgrammar` recursion does NOT push scope | (TBD) | (TBD) | | ADR-0032 §10.2 | `ScopedSubgrammar` (SELECT) DOES push scope | (TBD) | (TBD) | | ADR-0032 §10.3 | `SELECT *` CTE body derives output columns from from_scope | (TBD) | (TBD) | | ADR-0032 §10.3 | Computed CTE projection without alias yields unnamed slot | (TBD) | (TBD) | | ADR-0032 §10.3 | CTE `(col-list)` renames positionally | (TBD) | (TBD) | | ADR-0032 §10.3 | Compound CTE body takes columns from first leg | (TBD) | (TBD) | | ADR-0032 §10.3 | Recursive CTE body takes columns from non-recursive leg | (TBD) | (TBD) | | ADR-0032 §10.6 | Projection-before-FROM re-resolves on full re-walk | (TBD) | (TBD) | | ADR-0032 §11.6 | **Phase-1 gap: `LIKE`-on-numeric fires on SQL `WHERE`** | (TBD) | (TBD) | | ADR-0032 §11.6 | `= NULL` fires on SQL `WHERE` | (TBD) | (TBD) | | ADR-0032 §11.6 | Type-mismatch fires on SQL `WHERE` | (TBD) | (TBD) | | ADR-0032 §11.6 | Predicate warnings fire on `HAVING`, `ON`, `CASE`, projection, `ORDER BY` | (TBD) | (TBD) | | ADR-0032 §11.4 / §13 | Aggregate-in-WHERE rejected by engine, surfaced engine-neutral | (TBD) | (TBD) | | ADR-0032 §11.4 / §13 | Non-aggregated-column-with-GROUP-BY engine-neutral | (TBD) | (TBD) | | ADR-0032 §9 | Depth cap fires on pathological SELECT nesting (≥ 64 frames) | (TBD) | (TBD) | | ADR-0032 §12 | Engine column-origin metadata follows through CTE | (TBD) | (TBD) | | ADR-0032 §12 | All 10 playground types recovered via bare ref | (TBD) | (TBD) | | ADR-0032 §13 | Every OOS shape rejects (NATURAL, USING, comma-FROM, comma-LIMIT, window, LATERAL, VALUES) | (TBD) | (TBD) | The implementer fills in `Test location` and `Status` (green checkmark or red cross) as 2g proceeds. A row marked red blocks the phase exit. --- ## Final phase-exit verification report Produced at the end of 2g. A markdown document at `docs/handoff/-phase-2-verification.md` (or similar). The report contains: 1. **Test suite totals** — passed / failed / skipped / ignored, exact numbers from `cargo test` output. Compared against the Phase-1 baseline (1260 / 0 / 0 / 1). 2. **The full cross-cut verification matrix** with every row green. 3. **A requirements-to-test mapping** — every numbered decision in ADR-0032 §§1–13 with the test(s) that prove it. 4. **A list of any autonomous decisions** made during implementation, with rationale and explicit user-confirmation pointer for each. (Per CLAUDE.md: an autonomous decision without explicit user confirmation is a verdict FAIL.) 5. **The DA's written final review** — PASS or FAIL, no "conditional" verdicts. Only after the report is produced and signed off does the user-facing "Phase 2 complete" message issue. ## Open questions to escalate before code starts These are matters the plan deliberately leaves to the implementer to escalate when reached, *not* to decide silently: 1. **Subquery type-resolution through scalar subqueries.** The 2f exit gate notes the engine's column-origin metadata may or may not follow through a scalar subquery. The test asserts the actual behaviour; if the behaviour is "doesn't follow", that becomes an honest limitation in §12 — the user is informed, the limitation is documented. 2. **Duplicate CTE-name detection.** §11.4 leaves this as engine-rejected (no pre-flight diagnostic). If 2g's verification finds the engine message is confusing, escalate: the user may want a pre-flight `diagnostic.duplicate_cte` ERROR, which would mean a small extension to 2d. 3. **Function name completion / allowlist.** §11.4 / ADR-0030 §13 OOS-3 leaves no allowlist. If real-world testing reveals learners are confused by silently-admitted-then-engine-rejected function names, escalate for an explicit ADR. ## What this plan does NOT contain - Time estimates. Phase 2 is large and ADR-0032 is the first pass through the problem; calibration is poor. Milestones, not hours. - Sub-phase commit granularity beyond "ship green tests at each gate". The implementer commits at natural break points within a sub-phase; the gate is what gets verified. - Re-statements of ADR-0032's decisions. The plan refers to ADR-0032 by section; the implementer reads both.