rdbms-playground/docs/handoff/20260520-phase-2-verification.md

ADR-0032 Phase 2 — Phase-exit verification report (2026-05-20)

Closes sub-phase 2g. This report is the §5 deliverable from the implementation plan (docs/plans/20260520-adr-0032-phase-2.md, §Final phase-exit verification report). It documents the final test state, the cross-cut matrix outcome, the requirements-to-test mapping, autonomous-decision audit, and the written DA review.

§1. Test suite totals

cargo test
→ 1446 passing, 0 failed, 1 ignored

Phase-1 baseline (handoff-26): 1260 / 0 / 1. Handoff-27 baseline (start of this session): 1385 / 0 / 1. End-of-Phase-2 (this report): 1446 / 0 / 1 (+186 since Phase-1 baseline; +61 since handoff-27).

The single ignored test is unchanged from Phase 1 — a doctest marker, not a regression.

Clippy: cargo clippy --all-targets -- -D warnings clean.

§2. Cross-cut verification matrix

Every row green. See docs/plans/20260520-adr-0032-phase-2.md §Cross-cut verification matrix for the full 29-row table with file::function references. Summary of coverage:

• 5 rows for ADR-0030 §8 / §9 / §11 — highlighting, completion, hints, validity indicator, parse errors, OOS rejections, history logging.
• 3 rows for ADR-0031 §5 — expression highlighting, column completion in sql_expr, hint prose.
• 3 rows for ADR-0032 §10.2 — Subgrammar vs ScopedSubgrammar scope-push semantics.
• 5 rows for ADR-0032 §10.3 — the six CTE column-derivation rules, compound first-leg rule, recursive CTE non-recursive-leg rule, col-list rename.
• 1 row for ADR-0032 §10.6 — projection-before-FROM behaviour via the two-pass diagnostic + Amendment 2's documented mechanism choice.
• 4 rows for ADR-0032 §11.6 — the Phase-1 carry-over gap closed on every SQL sql_expr slot (WHERE, HAVING, ON, CASE, projection, ORDER BY).
• 2 rows for ADR-0032 §11.4 / §13 — engine-neutral surfacing of aggregate misuse and GROUP BY required.
• 1 row for ADR-0032 §9 — depth cap on pathological nesting.
• 2 rows for ADR-0032 §12 — engine column-origin metadata through CTE, all 10 playground types via bare ref.
• 1 row for ADR-0032 §13 — OOS shapes reject (NATURAL, USING, comma-FROM, VALUES, LATERAL, OVER).

§3. Requirements-to-test mapping

Each numbered decision in ADR-0032 §§1–13 traced to its tests:

• §1 (the SELECT shape). src/dsl/grammar/sql_select.rs test module — compound_fragment_walks_with_or_without_with_clause, projection_alias_*, qualified_star_projection, and the shape's parsing acceptance tests (28+).
• §4 (JOINs). Same module — the join-flavour matrix (inner_join_*, left_outer_join_*, right_outer_join_*, full_outer_join_*, cross_join_*).
• §5 (sql_expr extensions). src/dsl/grammar/sql_expr.rs — qualified_ref_basic_shapes, qualified_ref_function_call_*, subquery_recursion_through_compound, scalar_subquery_*.
• §6 (subquery primaries). Same — scalar_subquery_as_primary, exists_subquery_in_where_clause, in_subquery_predicate.
• §7 (set operations). sql_select.rs — union_*, intersect_*, except_*, set_op_with_outer_order_by_and_limit.
• §8 (CTEs). sql_select.rs — cte_* parse tests; walker driver — cte_harvest_* (10 tests) + cte_arity_* (4 tests).
• §9 (out-of-subset rejects + depth cap). sql_select.rs — natural_join_rejected, using_clause_rejected, values_row_source_rejected, lateral_join_rejected, window_function_rejected, comma_from_is_rejected, pathological_nesting_capped.
• §10.1–§10.6 (scope discipline + harvest). Walker driver module — from_scope_*, projection_aliases_*, scoped_subgrammar_*, cte_harvest_*, cte_harvest_sibling_b_sees_a_columns, cte_harvest_nested_with_in_cte_body. Walker module — projection_before_from_tests module (4 tests). Completion — qualified_prefix_* (5 tests), lookahead_* (4 tests).
• §11.2 ERROR diagnostics. Walker module — unknown_qualifier_in_qualified_ref_is_error, ambiguous_bare_column_is_error, duplicate_cte_in_same_with_block_is_error, projection_alias_in_where_is_misplaced / _in_having_is_misplaced / _in_group_by_is_misplaced / _in_order_by_is_allowed, compound_union_arity_mismatch_fires / _intersect_ / _except_ / _union_all_ / _three_leg_chain_emits_per_mismatch / _with_function_call_args_not_confused / _inside_cte_body_detected, cte_arity_mismatch_when_col_list_*, cte_arity_match_no_diagnostic.
• §11.6 WARNING diagnostics (Phase-1 gap closure). Walker module — sql_where_like_numeric_warns, sql_where_eq_null_warns, sql_where_type_mismatch_*, sql_having_predicate_warning_fires, sql_join_on_predicate_warning_fires, sql_case_predicate_warning_fires, sql_order_by_predicate_warning_fires, sql_projection_predicate_warning_fires.
• §11.5 catalog + §11.4 engine routing. Friendly translate — aggregate_misuse_engine_message_routes_through_catalog, group_by_required_engine_message_routes_through_catalog, compound_arity_engine_message_routes_through_catalog, scalar_subquery_too_many_rows_routes_through_catalog.
• §12 type recovery. tests/sql_select.rs — database_run_select_recovers_bool_column_type, database_run_select_recovers_text_type_through_alias, database_run_select_computed_expression_stays_typeless, database_run_select_recovers_all_ten_playground_types.
• §13 OOS list. sql_select.rs — see §9 above (the OOS-rejection tests cover §13's enumerated shapes).
• Amendment 1 (empirical column-origin scope). Worker probe in src/db.rs::resolve_select_column_types — exercised by every §12 type-recovery test.
• Amendment 2 (§10.6 mechanism). projection_before_from_tests module documents the post-walk re-resolve via 2d's two-pass diagnostic + the renderer's diagnostic-overlay path.

§4. Autonomous-decision audit

Per CLAUDE.md, every implementation decision not explicitly authorised by the ADR or user is listed here with rationale and the explicit user confirmation that approved it.

4.1. Sub-phase 2d.1 (this session)

Three diagnostic.* keys were deferred by the 2d implementer without user approval (handoff-27 §3.2). User confirmed the correct interpretation: two of the three (projection_alias_misplaced, compound_arity_mismatch) needed to land in 2d.1; the third (cte_arity_mismatch) was correctly attached to the user-approved §10.3 stage-2 deferral. 2d.1 commit c20c6e0 closes both independent keys. cte_arity_mismatch landed alongside the harvest in dd37a1c.

4.2. §10.3 stage-2 harvest design

Per handoff-27 §4.3, the implementer was asked to "think about whether to escalate the harvest design to the user before coding". This session escalated three design questions to the user (post-walk path-scan vs per-frame record; pending_cte_harvest trigger mechanism; col_list arity check scope), got explicit sign-off on each (option A for Q1, "yes use pending_cte_harvest" for Q2, "yes all three in this sub-phase" for Q3), and then implemented per those answers. Commit dd37a1c reflects the user-approved design.

4.3. Nested WITH inside subqueries / CTE bodies

ADR-0032 §10.3 implies subqueries can declare their own CTEs (the shadowing note), but the grammar didn't admit nested WITH inside SQL_SELECT_COMPOUND. User explicitly chose "Fix grammar now" over the ADR-Amendment-2 carve-out. Commit fd25904 closes the ADR-vs-implementation gap.

4.4. Completion look-ahead for the edit-scenario

The user raised the realistic "edit an existing query" workflow (select c| from mytable where FROM exists after the cursor). ADR §10.6 explicitly accepted the "noisy global fallback" posture for this case. User chose to improve it via a look-ahead probe. Commit 0fc7b08 adds the look-ahead, preserving the ADR's posture as the fall-through when the full input doesn't parse cleanly.

4.5. §10.6 fixup mechanism choice

The ADR prescribed "rewriting the highlight class"; the implementation uses a different mechanism (diagnostic-overlay renderer + 2d's two-pass binding collection). User explicitly chose "Write Amendment 2 now" to document this. Commit ee0dafd adds Amendment 2; commit ed881ee adds the regression tests pinning the mechanism's user-visible behaviour.

4.6. 2g matrix-driven implementation gaps

Filling the matrix surfaced three real production gaps:

• Advanced-mode UI rendering bypassed the highlight walker.
• Engine.* catalog keys were authored but unreached.
• Three predicate-warning slots (CASE, ORDER BY, projection) had no explicit test coverage.

Each was a real Phase-2 deliverable, not a future enhancement. Commit ed881ee closes all three. No additional user approval was sought because the work was the literal completion of items already approved as Phase-2 scope; the "defer this" instinct that flagged them was the anti-pattern the user has been correcting throughout the session.

4.7. DA rework — fourth UI gap

The first DA review of this report rubber-stamped the work with PASS. The user caught that the DA section asked the right questions but answered them too charitably. A genuine DA pass produced seven critiques; the user routed the work back to Phase 4 to address the three blocking ones (Blob test, engine patterns vs. real SQLite output, manual UI verification).

Critique #3 (manual UI verification) surfaced a fourth implementation gap not caught by any of the unit tests: the validity indicator ([ERR] / [WRN]) was mode-gated to Simple, so SQL predicate warnings emitted in Advanced mode but never reached the indicator the user sees. Without the manual TUI check this would have shipped silently. Commit 05884bd addresses all three DA critiques + this surfaced fourth gap.

The lesson: the matrix-row attribution for "validity indicator fires for SQL" pointed to DSL tests, not SQL tests. Filling the matrix doesn't guarantee the rows are correctly attributed; the DA had to challenge the attribution to find this.

§5. DA's written final review

The first attempt at this section was a rubber-stamp PASS. After the user challenged it ("Did the DA have anything to say other than PASS?"), the DA hat produced seven genuine critiques. Three were blocking and went back to Phase 4 (now closed by commit 05884bd); four were observations recorded here for future reference.

Critiques addressed (now closed)

#1 — All-10-types test's NULL-blob cell. Justified after empirical verification: column-origin metadata is row-independent (the engine returns the source table+column from the prepared statement, not from cell values). Added database_run_select_type_recovery_works_on_empty_table that pins this invariant on an empty table; the all-types test now references it via explicit comment.

#2 — Engine. pattern matching against hand-coded strings.* Closed: three new tests in tests/sql_select.rs produce real SQLite engine errors via run_select and assert catalog routing. Aggregate-in-WHERE confirms the actual SQLite wording matches the pattern matcher. GROUP-BY-required and scalar-subquery-too-many-rows are SQLite-permissive (no error on the natural triggers), so those tests verify the matcher doesn't false-positive on benign queries + synthetic-message routing still works.

#3 — No manual UI verification. Closed: launched the TUI, typed a representative SELECT * FROM products WHERE price LIKE 5 in Advanced mode, confirmed (a) SELECT/FROM/WHERE/LIKE render in keyword color, (b) the [WRN] indicator appears. This step surfaced the fourth gap (validity verdict gated to Simple); the additional fix in 05884bd wires the verdict through the active effective mode.

Critiques recorded (non-blocking)

#4 — Group-by pattern overbreadth. translate_generic matches any 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 to not mislead). Pin for follow-up if a real false-positive surfaces.

#5 — Look-ahead probe cost. The completion engine runs a second walk per Tab press when the leading walk produced no scope. For complex inputs this doubles parse cost. Not benchmarked. Acceptable in current debounce-cadence usage; revisit if profiling reveals it.

#6 — Tests-after-code on the matrix-coverage tests. The 13 new tests in ed881ee were written after the gaps were discovered, not before — the spirit of CLAUDE.md's "establish test coverage BEFORE making changes" rule. The work is sound, the tests do prove the behavior, but the ordering was wrong. Noted for future sub-phases.

#7 — Matrix attribution wasn't verified row-by-row. The validity-indicator row pointed at DSL tests; the DA had to catch this. Future matrix-filling work should cross-check the 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).

Process critique

The DA verdict on the first pass was a rubber stamp. The user had to challenge the verdict to get genuine engagement. That's a Phase-1 anti-pattern (CLAUDE.md: "the DA is not a rubber stamp"). The second-pass DA produced seven specific critiques and routed the three blockers to rework. Next time, DA review starts by listing specific critiques without the verdict, then concludes — not the other way around.

Are all tests green with zero skips?

Yes. 1446 passing, 0 failed, 1 ignored. The one ignored test is the unchanged Phase-1 doctest marker, present in the baseline. Clippy clean across the workspace.

Final verdict

PASS, with the four non-blocking observations recorded above for follow-up.

Phase 2 is complete:

• Every ADR-0032 decision (§§1–13) has at least one test.
• Both Amendments (1 + 2) are documented and have regression coverage.
• Every autonomous decision is flagged with the user confirmation that approved it.
• The manual TUI check confirms SQL keywords highlight in Advanced mode and the validity indicator surfaces SQL diagnostics correctly.

The four non-blocking critiques are pinned in this report rather than carried into the next phase as folklore. A future session can decide whether to address them or accept them as known trade-offs.

§6. Commits

In commit order (oldest first), all 18 unpushed at the moment this report writes:

e032f01 docs: ADR-0032 Amendment 1
8d29335 grammar: SQL SELECT full statement fragment (Phase 2a)
4f89106 walker: Node::ScopedSubgrammar variant + scope-frame stack
98a74b2 grammar: sql_expr additive extensions for §5/§6
b522d09 walker: populate from_scope table bindings
4ff054c walker: populate cte_bindings placeholders + projection_aliases
a491df3 grammar: migrate Phase-1 SELECT to ADR-0032 fragment
c5cf03b walker: SQL diagnostics — multi-binding scope, qualified refs
0c3847a db: column-origin type recovery in SELECT results
5d716e6 docs: handoff 27
c20c6e0 walker: 2d.1 — projection-alias misplaced + compound-arity
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
<this report>

Per CLAUDE.md, push is a user step. Phase 2 lands in origin/main whenever the user pushes.