Twenty-eighth handover. Captures this session's Phase-2 completion (sub-phases 2d.1, 2e, 2g + DA-driven rework) and points the next session at ADR-0030 Phase 3 (DML in Advanced mode) as the natural continuation. Pinned items for the next session: - Four non-blocking DA observations from this session's verification report (group-by pattern overbreadth, look-ahead probe cost, tests-after-code on matrix coverage, matrix attribution wasn't row-by-row verified). - A process lesson on DA discipline: rubber-stamp PASS verdicts must die. Next DA review lists critiques first, concludes after. - Phase 3 design considerations: UPDATE/DELETE table-source scope, INSERT…SELECT, RETURNING, UPSERT, DML-specific diagnostics. State: 1446 / 0 / 1 passing. Clippy clean. Phase 2 fully pushed.
13 KiB
Session handoff — 2026-05-20 (28)
Twenty-eighth handover. ADR-0032 Phase 2 closed. This session
shipped sub-phases 2d.1 (the two non-cte arity diagnostics that
prior agent deferred), 2e (qualified-prefix completion + the
§10.3 stage-2 CTE harvest + §10.6 fixup-pass mechanism), and 2g
(verification sweep, including a DA-driven rework round that
surfaced four real production gaps). Phase 2 verification report
landed at docs/handoff/20260520-phase-2-verification.md.
§1. State at handoff
Branch: main, fully pushed (user pushed after this
session). Working tree clean.
Tests: 1446 passing, 0 failing, 1 ignored (up from the start-of-session baseline of 1385 / 0 / 1 — a net +61 tests this session).
Clippy: clean (last verified at end of session, after the DA rework commits).
Commits this session (in commit order, all now on
origin/main):
c20c6e0 walker: 2d.1 — projection-alias misplaced + compound-arity ERROR passes
dd37a1c walker: 2e prereq — §10.3 stage-2 CTE harvest + cte_arity_mismatch
fd25904 grammar: admit WITH inside subqueries / CTE bodies (ADR-0032 §10.3)
0fc7b08 completion: §10.5 qualified-prefix + edit-scenario look-ahead
ee0dafd docs: ADR-0032 Amendment 2 + §10.6 regression tests
ed881ee 2g: advanced-mode highlight + engine.* wiring + matrix tests
05884bd 2g rework: address DA findings on type recovery + engine routing + UI
1c42e78 docs: ADR-0032 Phase 2 — phase-exit verification report
§2. What's done — Phase 2 in full
The Phase-2 surface delivered by this and the prior session is
documented in detail in
docs/handoff/20260520-phase-2-verification.md (the §5 deliverable
from the implementation plan). Headline summary:
2.1. Grammar
- Full SELECT statement: all five JOIN flavours, all four set
ops,
WITH/WITH RECURSIVECTEs with optional(col-list),GROUP BY/HAVING,LIMIT … OFFSET,DISTINCT,t.*, bare-alias projection (lifting Phase-1 §4.2). - sql_expr extensions for scalar subqueries,
IN (SELECT …),[NOT] EXISTS, qualified column refs. - Nested
WITHadmitted inside subqueries / CTE bodies (commitfd25904closed the ADR-vs-implementation gap; ADR-0032 §10.3 implied this all along). - OOS rejections for
NATURAL JOIN,JOIN … USING, comma-FROM, derived-tables-in-FROM, window functions,LATERAL,VALUES.
2.2. Walker
Node::ScopedSubgrammar(&Node)— the new node variant that pushes/pops a freshScopeFrameon entry/exit.WalkContext::from_scope_stack: Vec<ScopeFrame>— always non-empty, bottom frame is the implicit top-level scope.- Each
ScopeFramecarriesfrom_scope: Vec<TableBinding>,cte_bindings: Vec<CteBinding>,projection_aliases: Vec<String>. - §10.3 stage-2 harvest: CTE body's projection items derive
output columns at body-frame exit via post-walk path-scan
classifier (commit
dd37a1c). expand_bindingbridges CTE-source TableBindings to their CteBinding's columns (sibling and nested CTEs work).
2.3. Diagnostics
- Three passes run on every successful parse:
schema_existence_diagnostics— multi-binding scope, qualified refs, projection_alias_misplaced, duplicate_cte, unknown_qualifier, ambiguous_column.sql_predicate_warnings— MatchedPath-walking variant of the predicate-warning pass; closes the Phase-1 carry-over gap (= NULL, type-mismatch,LIKE-on-numeric on every SQLsql_exprslot: WHERE, HAVING, ON, CASE, projection, ORDER BY).compound_arity_diagnostics— pre-flight catch forSELECT 1, 2 UNION SELECT 1-style mismatches.
cte_arity_mismatchemitted from the harvest when col-list arity differs from derived arity.
2.4. Completion
CompletionProbenow exposesfrom_scopeandcte_bindingsat the cursor.- Qualified-prefix completion (§10.5) —
t.|/cte.|narrows to that binding's columns. - Look-ahead probe (the "edit an existing query" workflow): when the leading walk produces no scope but the full input has a FROM after the cursor, a second walk on the full input populates from_scope and column candidates narrow accordingly. Falls back to the §10.6 noisy-global posture when the full input doesn't parse cleanly.
2.5. UI rendering
- Mode-aware highlighting:
lex_to_runs_in_mode,render_input_runs_in_mode,highlight_runs_in_mode. The Advanced render path inui.rsnow uses the highlight walker withMode::Advanced, so SQL keywords colour correctly. - The validity indicator
[ERR]/[WRN]is wired through to the active effective mode. SQL predicate warnings reach the indicator (previously gated to Simple only — a real gap the DA rework surfaced). plain_input_spans(the old un-highlighted Advanced fallback) removed.
2.6. Type recovery
do_run_selectcallsresolve_select_column_types(conn, stmt)afterprepareand threads the per-column types through toformat_cell.- ADR-0030 Phase-1 §4.5 (bool rendering as
0/1) is lifted for every Phase-2 shape per Amendment 1's empirical findings (non-recursive CTEs, scalar subqueries, derived tables, set ops, JOINs all work; recursive CTE result columns stay typeless).
2.7. Friendly-error layer
- Four new
engine.*keys (aggregate_misuse, group_by_required, compound_arity_mismatch, scalar_subquery_too_many_rows) are authored AND wired:translate_genericpattern-matches the engine message text and routes to the engine-neutral catalog entry. Aggregate-in-WHERE confirmed end-to-end against actual SQLite output; GROUP-BY-required and scalar-subquery are SQLite-permissive (no real error on the natural triggers), but the routing tests verify both no-false-positive on benign queries and synthetic-message routing.
2.8. Documentation
- ADR-0032 Amendment 2 — records that §10.6's "rewrite the highlight class" prescription is realised via the two-pass schema-existence diagnostic + the renderer's diagnostic- overlay path (no separate per-byte rewrite step needed; no new HighlightClass variant).
- Cross-cut verification matrix in
docs/plans/20260520-adr-0032-phase-2.md— all 29 rows populated with concretefile::functiontest references. - Phase-exit verification report at
docs/handoff/20260520-phase-2-verification.mdwith the full requirements-to-test mapping, autonomous-decision audit, and DA review (PASS, with 4 non-blocking observations).
§3. The four non-blocking DA observations
These were recorded in the verification report (§5) and are pinned here so the next session can decide whether to address them or accept them as known trade-offs. None block Phase 3 work.
§3.1. Group-by pattern overbreadth
translate_generic in src/friendly/translate.rs matches any
engine error containing "group by" — could route unrelated
future SQLite errors through engine.group_by_required. Low
risk in practice (SQLite's other "group by" mentions are rare
and the catalog wording is generic enough not to mislead).
Tighten if a real false-positive surfaces.
§3.2. Look-ahead probe cost
The completion engine in src/completion.rs runs a second walk
per Tab press when the leading walk produced no scope. For
complex inputs this doubles parse cost on those keystrokes. Not
benchmarked. Acceptable in current debounce-cadence usage;
revisit if profiling reveals it.
§3.3. Tests-after-code on the matrix-coverage tests
The 13 new tests in commit ed881ee were written after the
gaps were discovered, not before. The spirit of CLAUDE.md's
"establish test coverage BEFORE making changes" rule was
violated. The tests do prove the behaviour, but the ordering
was wrong. Noted for future sub-phases — when matrix-driven
gap-closure happens, write failing tests first.
§3.4. Matrix attribution wasn't verified row-by-row
The "validity indicator fires for SQL" matrix row pointed at
DSL tests (input_verdict_eq_null_is_warning exercises a
DELETE FROM command, not a SELECT). The DA had to catch
this — the rest of the matrix may have similar mis-attributions.
Future matrix-filling work should cross-check that each
attributed test actually exercises the claimed surface (e.g., a
"fires for SQL" row should point at a SQL-input test, not a
DSL test that happens to share the same code path).
§4. Process lesson — DA discipline
The first DA pass on the verification report was a rubber stamp. The user had to explicitly challenge it ("Did the DA have anything to say other than PASS?") to get genuine engagement. The second-pass DA produced seven specific critiques and routed three blockers to rework (Phase 4 loop-back). Next session: DA review starts by listing specific critiques without the verdict, then concludes — not the other way around. A "conditional PASS" or "PASS with caveats" is a FAIL per CLAUDE.md, but only if the DA actually surfaces the caveats.
§5. What's next — ADR-0030 Phase 3
ADR-0030's roadmap from §3:
- Phase 1 ✅ — Validation grammar for SELECT in Advanced mode.
- Phase 2 ✅ — Full SELECT grammar (this session closed it).
- Phase 3 ← NEXT — DML in Advanced mode (
INSERT/UPDATE/DELETEas SQL). - Phase 4 — DDL in Advanced mode.
- Phase 5 — DSL→SQL teaching echo.
- Phase 6 — Polish.
Phase 3 planning is the next session's primary task. The infrastructure built for Phase 2 directly applies:
ScopedSubgrammar+ scope-frame stack — UPDATE / DELETE have table-source slots that need from_scope, same machinery.- sql_expr —
INSERT … (col, …) VALUES (…)value lists, UPDATE SET assignments, WHERE predicates — all already speak sql_expr. - Diagnostics — multi-binding schema-existence and the predicate-warning pass apply to DML WHEREs uniformly.
- Engine-neutral parse errors via the friendly-error layer.
What needs fresh thinking:
- Scope of DML's table-source slot. UPDATE / DELETE accept
a single table (no JOIN in standard SQL). SQLite extends this
via
UPDATE FROM. ADR-0030 Phase 3 needs a posture. INSERT … SELECT— pulls in the full Phase-2 SELECT surface as a subquery. Should fall out for free from ScopedSubgrammar recursion.RETURNINGclause — SQLite supports it. Phase-3 may or may not include it; needs a decision.UPSERT(INSERT … ON CONFLICT …) — same.- Diagnostics new to Phase 3: e.g., assigning a column not in the target table on INSERT, or a NOT-NULL violation that the schema cache can pre-flight.
Recommended next-session entry point:
- Read this handoff in full.
- Read
docs/adr/0030-advanced-mode-sql-surface.md(the source roadmap). - Read
docs/adr/0032-sql-select-grammar.md(the Phase-2 template — the same shape applies to Phase 3). - Read this session's Phase-2 verification report
(
docs/handoff/20260520-phase-2-verification.md) for the patterns that worked and the four non-blocking observations that may want attention. - Draft ADR-0033 (placeholder number) for ADR-0030 Phase 3 — the SQL DML surface. Follow ADR-0032's structure: scope, sub-grammars, completion scope, diagnostics, type recovery, OOS list, sub-phase plan.
§6. Other open work — unchanged from prior handoffs
Same list as handoff-27 §8, minus the items Phase 2 closed:
- Project storage Iter 5/6 — export/import,
--resume, persistent input history hydration, migration framework scaffold. - Snapshot / replay / undo (U-series) — designed in ADR-0006, not implemented.
- m:n convenience (C4) — auto-generated junction tables.
- Modify relationship (C3a) — drop+add covers the case today.
- Column drops / renames / type changes (B2 / C2 partial) — rebuild-table primitive (ADR-0013) is in place; grammar + dispatch are pending.
- Rename table (C1) — needs its own ADR.
- Friendly-error sweep (H1) — partial today; ADR-0032 §11.5
added four
engine.*keys. - Strong syntax-help in parse errors (H1a) — point users at missing keywords rather than the unexpected character.
- CI (TT5) — test infrastructure exists; CI workflow not yet configured.
- Session log + Markdown export (V4) — scrollable session journal + save-as-markdown.
- Readline shortcuts (I1b), multi-line input (I1), tab completion polish (I3), syntax highlighting (I4) — the last two received Phase-2 advancement this session.
- ER diagram export (V3).
- Tutorial / lesson system — needs its own ADR.
§7. How to take over
- Read this file in full. Then the Phase-2 verification
report and
CLAUDE.md. cargo test— expect 1446 passing, 0 failed, 1 ignored.cargo clippy --all-targets -- -D warnings— clean.- The user's stated next step is "plan Phase 3, then drop a new handoff that explicitly points to the next work step". The Phase-3 planning happens in the same session as this handoff was written, so handoff-29 will likely follow with the concrete code-starts-here pointer.
- Escalate before re-litigating decisions. Phase 2's ADR + Amendments are accepted. If implementation reveals they're wrong, write a new Amendment — don't drift.