docs: handoff 26 — ADR-0032 + Phase 2 plan accepted
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.
This commit is contained in:
claude@clouddev1
@@ -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<ScopeFrame>`
|
||||
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<ScopeFrame>`. `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.
|
||||
Reference in New Issue
Block a user