rdbms-playground/docs/handoff/20260521-handoff-30.md

Session handoff — 2026-05-21 (30)

Thirtieth handover. This session did two things:

1. Started ADR-0033 Phase 3 (SQL DML) — wrote the Phase-3 implementation plan, then shipped sub-phases 3a (DSL/SQL dispatch mechanism) and 3b (SQL INSERT grammar + minimal execution).
2. Ran an advanced-mode completion/hint bug-fixing interlude triggered by manual testing — fixed the dead advanced-mode ambient assistance plus two completion bugs (F1, F2), and triaged several other reports.

The next session should finish the interlude (one bug left, F5, which needs the user's OK because it's a core walker change) and then resume Phase 3 at sub-phase 3c (INSERT … SELECT).

§1. State at handoff

Branch: main. Tests: 1473 passing, 0 failing, 1 ignored. Clippy: clean (cargo clippy --all-targets -- -D warnings).

Commits this session (newest first; the last three are local-only, origin/main is at 4e16d97):

1c8cbc1 completion+hint: F1/F2 advanced-mode completion fixes      (local)
ed40445 ui: re-enable advanced-mode ambient assistance (ADR-0022 Amendment 1)  (local)
c873631 grammar+db: 3b — SQL INSERT grammar + minimal execution (ADR-0033 §1)  (local)
4e16d97 walker: 3a — category-grouped mode-aware dispatch (ADR-0033 Amendment 1)
a37a0b7 docs: ADR-0033 Phase 3 — implementation plan + cross-cut matrix

(Unpushed commits are a normal working state; pushing is the user's step — do not prompt about it.)

§2. Phase 3 — the main job (ADR-0033, SQL DML)

Controlling docs (read both):

• docs/adr/0033-sql-dml-grammar.md — the decision. Read Amendment 1 especially (it supersedes §2's dispatch mechanism — see below).
• docs/plans/20260520-adr-0033-phase-3.md — the build order (sub-phases 3a–3k), per-sub-phase exit gates + DA gates, the cross-cut verification matrix (filled in during 3k), and the standing authorizations (commits pre-authorized within the plan — but see §5 below: the user has since asked to approve each commit and watch context, so confirm commits).

2.1. Done

Sub-phase 3a — DSL/SQL dispatch mechanism (commit 4e16d97). The ADR originally specified Node::Guard + Choice(SQL, DSL). That was found unworkable during 3a (any guard-in-Choice forces a walk_choice change; walk_choice only falls through on NoMatch, and walk_seq treats a NoMatch past idx 0 as a hard Failed). It was replaced — with the user's explicit approval — by category-grouped, mode-aware dispatch, recorded in ADR-0033 Amendment 1:

• Each REGISTRY entry is tagged CommandCategory::{Simple, Advanced} (in src/dsl/grammar/mod.rs), generalising the old whole-command is_advanced_only gate.
• walker::walk is now a thin dispatcher over decide + walk_one_command + scratch helpers (src/dsl/walker/mod.rs). Simple mode commits the DSL node (or emits "this is SQL"); Advanced tries Advanced candidate(s) first, DSL as fallback.
• A shared entry word carries a node in both groups; the dispatcher selects by mode. No Node::Guard, no walk_choice/walk_seq change.

Sub-phase 3b — SQL INSERT grammar + minimal execution (commit c873631).

• src/dsl/grammar/sql_insert.rs::SQL_INSERT_SHAPE — `INTO [ (cols) ] VALUES tuple(s)`, `__rdbms_*` target rejection (reuses `sql_select::reject_internal_table`, now `pub(crate)`).
• Command::SqlInsert { sql, target_table }; Request::RunSqlInsert + do_sql_insert worker (tx-guarded: execute → finalize_persistence for CSV + history → commit, so failures roll back and don't re-persist). Auto-show is best-effort via last_insert_rowid range.
• Isolated behind a temporary dev entry word sqlinsert (Advanced category) so the SQL path is testable WITHOUT making insert a shared word yet. Tests use sqlinsert into ….
• Deviations from the plan's literal Command::SqlInsert field list (to avoid dead-code under -D warnings): listed_columns is deferred to 3d (shortid auto-fill) and returning to 3g — the sub-phases that actually read them.
• Tests: 6 grammar accept/reject (sql_insert.rs) + 8 integration (tests/sql_insert.rs).

2.2. Remaining sub-phases (resume here after the interlude)

Per the plan, in order: 3c INSERT … SELECT → 3d shortid auto-fill (adds listed_columns) → 3e UPDATE → 3f DELETE + cascade summary → 3g RETURNING (adds returning flag) → 3h UPSERT (ON CONFLICT) → 3i diagnostics (insert_arity_mismatch, auto_column_overridden, not_null_missing) → 3j dispatch wiring → 3k verification sweep.

Things to remember for later sub-phases:

• 3j is where the dev sqlinsert word is removed and insert becomes a real shared word (register the SQL INSERT node as Advanced alongside the DSL insert Simple node). With the grouping mechanism this is mostly registry wiring. 3j must also add de-duplication to the completion entry-word lists (completion_probe_in_mode / expected_at_input_in_mode in src/dsl/walker/mod.rs): once insert appears twice in REGISTRY, the entry-word candidate lists would show it twice. (Flagged in ADR-0033 Amendment 1.)
• 3j must also confirm advanced-mode DSL inserts don't regress through the (now-shared) SQL path — which is why 3j comes after 3d's shortid-auto-fill parity.

§3. The advanced-mode completion/hint interlude

Triggered by the user manually testing advanced mode. Paused Phase 3 to harden it because broken ambient assistance poisons every advanced-mode test we'd write.

3.1. Fixed and committed

Ambient assistance re-enabled in advanced mode (ADR-0022 Amendment 1, commit ed40445). render_hint_panel hard- returned None for advanced mode (a stale ADR-0022 §12 gate predating the SQL grammar), and the hint resolver defaulted to Mode::Simple. Now: ambient_hint_in_mode + hint_resolution_at_input_in_mode + expected_for_hint_snapshot thread Mode; render_hint_panel calls ambient for all modes; the one-shot : sigil is stripped (strip_one_shot_prefix) before the ambient walk. App-level render test (ui::advanced_mode_hint_panel_surfaces_sql_candidates) + ambient-layer locks. (Phase 2 had marked SQL hint/completion "green" but only at the engine layer, never the UI — the gap this fix closes.)

F1 + F2 (commit 1c8cbc1).

• F1 — a premature "no such table/column" ERROR shadowed the completion for the token being typed (the hint panel is the completion UI; there is no separate popup). Fix in input_render.rs::ambient_hint_in_mode: suppress an under-cursor ERROR diagnostic when a completion exists for the (non-empty) partial it overlaps; fall through to candidates. Genuinely-unknown names still error; WARNINGs unaffected; both modes.
• F2 — select <cursor> from T (after deleting *) offered the global column list instead of T's columns, because the §10.6 look-ahead's full-input walk can't reach FROM through an empty projection. Fix in completion.rs::candidates_at_cursor_with_in_mode: when the look-ahead finds no scope, retry with a neutral 1 placeholder inserted at the cursor so the trailing FROM/CTE scope is recovered for narrowing (only the repaired walk's from_scope/cte_bindings are consumed).

3.2. Triaged as NOT bugs (no work needed)

• F3 ("order by Name" flagged red / no suggestions) — was a downstream symptom of the old-project migration gap (see §4), not an ORDER BY grammar bug. On a fresh db it does not occur (order by <valid column> parses clean, narrows correctly). Confirmed by reproduction.
• F4 ("only one error span highlights") — multi-span highlighting works (two simultaneous unknown columns both get flagged). The "des red while typing" the user saw is as-you-type harshness on a partial keyword (desc mid-typed reads as trailing junk → parse error), not a single-span limit.
• Case-sensitivity (table-name case vs. column narrowing) — not a bug. columns_for_table (completion.rs:90) uses eq_ignore_ascii_case; verified from orders / from ORDERS / from Orders all narrow correctly. An earlier DA-gate "flag" about this was an unverified guess and was retracted.

3.3. REMAINING — F5 (needs the user's OK before coding)

F5 — the order by (and projection) candidate list is padded with ~13 invalid clause keywords (as, left, right, full, cross, inner, join, where, group, having, union, intersect, except). The columns are present and correctly filtered, but the noise shoves them off-screen. The user is fine with keywords-before-columns ordering as long as the keyword list is reasonably filtered — i.e. fix the spurious keywords.

Confirmed mechanism (probed): these are stale skipped-optional clause keywords from the clauses positioned between the last-consumed token and order by (the FROM's JOIN options, WHERE/GROUP BY/HAVING, set-ops). They accumulate in walk_seq's pending_skipped and get merged into ORDER BY's Incomplete expected set — even though ORDER BY consumed bytes and committed past those optionals. Control inputs proved it: … where id = 1 order by drops where+JOIN keywords (consumed), leaving group/having/set-ops; select 1 order by (no FROM) adds from.

Proposed fix (CORE walker change — get user OK): in src/dsl/walker/driver.rs walk_seq, the Incomplete arm should NOT merge the accumulated pending_skipped when the child that produced the Incomplete consumed bytes (i.e. committed past those optionals). The NoMatch arm (child consumed nothing → optionals are genuine alternatives at the cursor) keeps merging. Blast radius: all parsing (1469+ tests). This is exactly the kind of core change the project is careful about and the user asked to approve it first. Do it test-first: a failing test that asserts the order by expected set excludes where/group/join/union/etc., then the fix, then full suite + clippy.

§4. Deferred — old-project migration (do NOT fix now)

The user's first repro used an old project (~/.local/share/rdbms-playground/projects/20260521-repro-1/, created_at: 2026-05-09) whose __rdbms_playground_columns is the pre-check_expr 3-column schema. Current code's read_schema (db.rs:4197) hard-references m.check_expr, so:

• show table crashes (no such column: m.check_expr);
• describe_table fails for every table → build_schema_cache (runtime.rs:992) silently swallows the error → table_columns ends up empty → degraded completion (this was the real cause of the "F3" symptoms above).

The user explicitly deferred this — "having a migration story at the end is perfectly fine." So: no migration work now. When the migration framework lands (ADR-0015 Iter 6, pending), the targeted fix is to ensure-current-metadata-columns on open (idempotent ALTER TABLE … ADD COLUMN check_expr TEXT), make read_schema not hard-crash on a missing internal column, and stop build_schema_cache from silently swallowing the describe error (it masked this for a long time). ADR-0015 would get an amendment. Recorded here so it isn't lost.

§5. Process pins (important for the next session)

• Confirm every commit. The ADR-0033 plan pre-authorized commits, but mid-session the user asked to approve each commit and watch context use — so propose the commit message and wait for the go-ahead. No auto-commit at sub-phase gates.
• Push is the user's step. Never push; never prompt about it.
• No AI attribution in commits (global rule).
• Test-first discipline held throughout this session and should continue: reproduce a bug with a FAILING test first, then fix. For new features, exit-gate tests before "done."
• Don't trust unverified diagnoses — this session twice produced wrong guesses (the case-sensitivity "bug"; F3 as an ORDER BY grammar bug) that empirical probes disproved. Probe before fixing.
• DA discipline: specific critiques first, verdict last; a clean PASS with no critiques is a process failure.
• Core walker changes (walk_seq/walk_choice) need explicit user OK before coding (the F5 fix is the pending example).

§6. How to take over

1. Read, in order: this file → docs/adr/0033-sql-dml-grammar.md (esp. Amendment 1) → docs/plans/20260520-adr-0033-phase-3.md → docs/adr/0022-ambient-typing-assistance.md Amendment 1 → CLAUDE.md.
2. Baseline:

   cargo test    # expect 1473 passing / 0 failing / 1 ignored
   cargo clippy --all-targets -- -D warnings   # clean
   

3. First: finish the interlude — F5. Confirm with the user that the core walk_seq change is OK (it's the only blocker), then implement test-first per §3.3.
4. Then: resume Phase 3 at sub-phase 3c (INSERT … SELECT) per the plan, continuing 3c → 3k. Remember the 3j notes in §2.2 (remove sqlinsert dev word, make insert shared, add completion-list de-dup) and the deferred listed_columns/ returning Command fields (3d/3g).
5. Escalate anything not settled in ADR-0033 / the plan; don't classify work "out of scope" without user confirmation.