diff --git a/docs/plans/20260526-adr-0035-sql-ddl-4i.md b/docs/plans/20260526-adr-0035-sql-ddl-4i.md new file mode 100644 index 0000000..280dc62 --- /dev/null +++ b/docs/plans/20260526-adr-0035-sql-ddl-4i.md @@ -0,0 +1,157 @@ +# Plan: ADR-0035 Phase 4, sub-phase 4i — verification sweep (completes Phase 4) + +The closing sub-phase. Canonical scope: **ADR-0035 §13 4i**. Items: + +- **(a)** Refresh the `CREATE TABLE` help/usage skeleton for the 4a.2 + `DEFAULT`/`CHECK`/composite-`UNIQUE`, 4a.3 table-`CHECK`, and 4b FK + forms (the only help/usage debt — 4d/4e/4f/4g/4h carry their own). +- **(b)** `describe` display of table-level constraints (composite + `UNIQUE` + table `CHECK`, incl. **named** CHECKs from 4g). +- **(c)** 4b self-ref FK pre-submit indicator: stop false-flagging a + `references ` parent as unknown. +- **(d)** shared-entry-word completion merge. +- **(e)** visually distinguish simple- vs advanced-mode completions. +- **staples:** typing-surface + matrix coverage, engine-neutral error + pass, undo-parity (one step per statement). + +**User-chosen sequencing (2026-05-26):** the **(d)/(e) design conversation +first**, then build. (d)/(e) are designed (below); (a)/(b)/(c) + staples +follow. + +## 1. Baseline + +- HEAD `ca64434`; **1909 passing / 0 failing / 0 skipped / 1 ignored**; + clippy clean. + +## 2. (d) + (e) — settled design (user-confirmed 2026-05-26) + +### (d) Shared-entry-word completion merge + +**Bug:** in advanced mode the walker's `decide()` (`walker/mod.rs`) +commits **one** candidate node for a shared entry word, so only that +node's continuations reach completion — `drop ` offers only `table`, and +`drop rel` returns empty, even though the DSL drops parse via fallback. + +**Fix:** in `completion_probe_in_mode`, for an **advanced-mode** shared +entry word (the entry word resolves to **>1 candidate**), walk **each** +candidate node speculatively (the existing `scratch`/`walk_one_command` +machinery on a fresh context), extract each one's expected continuations +(the same `WalkOutcome`→`expected` match the single-walk path uses), and +**union** them. Simple mode is unchanged (its single DSL node already +offers all DSL continuations; SQL isn't offered). A candidate that +mismatches the current input contributes an empty set, so as the input +disambiguates (`drop table …`) the union naturally narrows to the +surviving candidate. + +### (e) Visual simple-vs-advanced distinction + +Each merged continuation is classified by **which categories produced +it** — a new `ModeClass`: + +- `Both` — produced by ≥1 `Advanced` **and** ≥1 `Simple` candidate (a + continuation valid either way, e.g. `drop table`). +- `Advanced` — only `Advanced` candidates (SQL-only, e.g. `create index`). +- `Simple` — only `Simple` candidates (DSL-only, e.g. `drop relationship`). + +**Colour + order apply only when the candidate list is *mixed*** (>1 +distinct `ModeClass`) — the only place the signal is informative; a +single-mode list (deep inside any command) keeps today's token-kind +colours, no mode tint (avoids redundant noise duplicating the mode +indicator). When mixed: + +- **Colour:** `Both` → today's token-kind colour (neutral); `Advanced` → + `theme.mode_advanced` (orange); `Simple` → `theme.mode_simple` (cyan). + These two mode colours already exist (used by the mode indicator). +- **Order (user refinement):** group the continuations into contiguous + colour **blocks** in the order **`Both` → `Advanced` → `Simple`**, so + `Advanced` sits between the other two and each colour reads as one block + rather than interleaving. + +## 3. Architecture & change list (d/e) + +- **`src/dsl/walker/outcome.rs`** (or `CompletionProbe`): add a parallel + `expected_modes: Vec` (same length/order as `expected`), + defaulting to all `Both` (so the single-walk path is neutral). New + `ModeClass` enum (likely in `completion.rs`, re-exported). +- **`src/dsl/walker/mod.rs`** `completion_probe_in_mode`: factor the + expected-extraction (lines 333-356) into a helper; add the advanced-mode + multi-candidate branch that walks each candidate (mode `Advanced`), + classifies each expectation by the producing category, and fills + `expected` + `expected_modes`. +- **`src/completion.rs`**: `Candidate` gains `mode: ModeClass`; the + keyword-building (`Expectation::Word`) carries the parallel `ModeClass` + through prefix filtering; when the keyword set is mixed, **block-order** + by `ModeClass` (Both→Advanced→Simple) within the keyword group; + non-keyword kinds and single-mode lists are all `Both`. +- **`src/ui.rs`** `render_candidate_line` (≈907): when the candidate list + contains >1 `ModeClass`, colour each by its `ModeClass` + (Both=kind-colour, Advanced=`mode_advanced`, Simple=`mode_simple`); + otherwise unchanged. +- **Tests:** `tests/typing_surface/` — `drop ` (advanced) offers + `table·index·column·relationship·constraint`; `drop rel` → + `relationship`; `create ` → `table` (Both) + `index` (Advanced); + block-order assertion (Both then Advanced then Simple); simple mode + unchanged. A `ui.rs` snapshot for the mixed-mode coloured hint line. + +## 4. (a)/(b)/(c) — outline (build after d/e) + +- **(a)** Extend the `sql_create_table` help/usage strings + (`friendly/strings/en-US.yaml`) for `DEFAULT`/`CHECK`/composite-`UNIQUE` + (4a.2), table-`CHECK` (4a.3), and inline + table-level `FOREIGN KEY` + (4b). Catalog-lockstep + vocab-audit guard wording. +- **(b)** `describe` (`do_describe_table` / the structure renderer): show + table-level composite `UNIQUE` and table `CHECK` constraints (named + CHECKs show their name). Tier-2 snapshot + Tier-3 describe assertions. +- **(c)** The pre-submit schema-existence diagnostic: treat a FK parent + equal to the in-statement `CREATE TABLE` target as valid (self-ref), so + the `[ERR]`/`[WRN]` indicator stops lying. Tier-1/typing-surface test. + +## 5. Phase 2/3 — candidates & selection (d/e) + +Settled via the design conversation (§2). The merge alternative +("modify `decide`/`walk` to merge in the core parser") was **rejected** — +it would change the shared parser used everywhere; doing the merge in the +completion-only `completion_probe_in_mode` is lower-risk. The colour +alternatives (always-by-mode; marker tag; defer) were presented to the +user, who chose **mode-colour-when-mixed + block ordering**. + +## 6. Devil's Advocate review of this plan + +- **Forks escalated?** The (e) visual treatment was put to the user with + four options + previews; they chose option 1 with the block-ordering + refinement. (d)'s behaviour is ADR/handoff-specified. ✓ +- **Core-parser risk?** The merge lives in `completion_probe_in_mode` + (completion-only), not `walk`/`decide` — the parse path is untouched, so + no risk to execution/dispatch. ✓ +- **Simple mode unaffected?** The merge branch is advanced-only; + simple-mode completion keeps the single-DSL-node path. A test pins it. ✓ +- **Noise?** Colour/order apply only when the list is genuinely mixed; + single-mode lists are visually unchanged. ✓ +- **Perf?** Completion walks each candidate per keystroke — candidates per + entry word are few (≤ ~3), and completion is already a per-keystroke + walk; negligible. ✓ +- **Ordering vs existing kind-order?** Block-ordering applies *within* the + keyword group (where shared-entry continuations live); the + identifiers-first kind ordering is preserved. ✓ + +## 7. Implementation sequence (test-first) + +1. **(d) merge** — `ModeClass` + `expected_modes`; the advanced-mode + multi-candidate walk + union in `completion_probe_in_mode`; thread + through `completion.rs` (functional merge, classes computed but not yet + coloured). Typing-surface tests (merge + simple-mode-unchanged) → green. +2. **(e) order + colour** — block-ordering when mixed; `render_candidate_line` + colours by `ModeClass`. Ordering test + a UI snapshot → green. +3. **(c)** self-ref FK indicator → test-first. +4. **(b)** describe table-level constraints → snapshot/e2e. +5. **(a)** CREATE TABLE help/usage skeleton + catalog lockstep. +6. **Staples** — typing-surface/matrix sweep, engine-neutral error pass, + undo-parity spot-check. +7. **Full sweep** + finished-slice `/runda` → commit proposal(s). + +## 8. Exit gate + +All §13 4i items done or explicitly deferred-with-user-confirmation; all +tiers green, zero skips; no regression from 1909; clippy clean; written-DA +/ `/runda` PASS; ADR-0035 §13 4i + README + requirements lockstep. **4i +completes ADR-0035 Phase 4** — flip the ADR Status from "4i pending". diff --git a/src/completion.rs b/src/completion.rs index d2d85d4..8a24846 100644 --- a/src/completion.rs +++ b/src/completion.rs @@ -167,6 +167,35 @@ pub const fn identity_ranker(candidates: Vec) -> Vec { candidates } +/// Which input mode(s) a shared-entry-word continuation belongs to. +/// +/// (ADR-0035 §4i d/e.) Computed only where a shared entry word's +/// candidate nodes are merged; everywhere else a continuation is `Both` +/// (neutral). Drives the contiguous colour-block ordering (and, in the +/// UI, the colour) when a candidate list mixes modes. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ModeClass { + /// Valid in both simple (DSL) and advanced (SQL) mode — neutral. + Both, + /// Advanced/SQL-only continuation. + Advanced, + /// Simple/DSL-only continuation. + Simple, +} + +impl ModeClass { + /// Block-ordering key: `Both` (0) → `Advanced` (1) → `Simple` (2), so + /// each colour reads as one contiguous block (user-confirmed order). + #[must_use] + pub const fn block_order(self) -> u8 { + match self { + Self::Both => 0, + Self::Advanced => 1, + Self::Simple => 2, + } + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum CandidateKind { /// One of the parser's expected keywords. @@ -504,6 +533,35 @@ pub fn candidates_at_cursor_with_in_mode( let mut seen_kw = std::collections::HashSet::new(); keywords.retain(|k| seen_kw.insert(k.clone())); + // (ADR-0035 §4i e) Block-order the keyword continuations by + // mode-class when a shared entry word merged simple + advanced forms, + // so the colours read as contiguous blocks Both → Advanced → Simple + // (user-confirmed order). A single-mode list (all `Both`) is left in + // its declaration order. `sort_by_key` is stable, preserving the + // intra-block order. + let kw_mode = |kw: &str| -> ModeClass { + probe + .expected + .iter() + .zip(&probe.expected_modes) + .find_map(|(e, m)| match e { + Expectation::Word(w) | Expectation::Literal(w) if w.eq_ignore_ascii_case(kw) => { + Some(*m) + } + _ => None, + }) + .unwrap_or(ModeClass::Both) + }; + let mixed = keywords + .iter() + .map(|k| kw_mode(k.as_str()).block_order()) + .collect::>() + .len() + > 1; + if mixed { + keywords.sort_by_key(|k| kw_mode(k.as_str()).block_order()); + } + // Source 1.5: type-name candidates when the walker expects // a column-type slot. Type names are a closed set sourced // from `Type::all()` (ADR-0005 declaration order: @@ -1177,9 +1235,41 @@ mod tests { #[test] fn at_token_boundary_offers_next_expected_keyword() { - // After `create ` the parser expects `table`. + // After `create ` advanced mode offers `table` (valid in both + // modes) plus the SQL-only `unique` (`create unique index`) and + // `index` — the shared-entry-word merge (ADR-0035 §4i d). + // `table` (Both) blocks before the Advanced-only `unique`/`index`. let cs = cands("create ", 7); - assert_eq!(cs, vec!["table".to_string()]); + assert_eq!( + cs, + vec!["table".to_string(), "unique".to_string(), "index".to_string()] + ); + } + + #[test] + fn shared_entry_word_drop_merges_all_continuations() { + // ADR-0035 §4i (d): in advanced mode `drop` is a shared entry + // word (SQL `drop table`/`drop index` + the DSL drops). The + // completion must offer EVERY valid continuation, not just the + // first-decided node's. Block order: Both (table, index) then + // Simple-only (column, relationship, constraint). + let cs = cands("drop ", 5); + for kw in ["table", "index", "column", "relationship", "constraint"] { + assert!(cs.contains(&kw.to_string()), "`drop ` should offer `{kw}`; got {cs:?}"); + } + // Both-mode continuations block before the simple-only ones. + let pos = |k: &str| cs.iter().position(|c| c == k).unwrap(); + assert!(pos("table") < pos("column"), "Both block precedes Simple block: {cs:?}"); + assert!(pos("index") < pos("relationship"), "Both block precedes Simple block: {cs:?}"); + } + + #[test] + fn shared_entry_word_drop_partial_keeps_matching_continuation() { + // A partial second keyword (`drop rel`) used to dead-end at an + // empty list (only the SQL node walked); the merge keeps the + // DSL `relationship` continuation. + let cs = cands("drop rel", 8); + assert_eq!(cs, vec!["relationship".to_string()]); } #[test] @@ -1489,20 +1579,23 @@ mod tests { } #[test] - fn drop_in_advanced_mode_surfaces_the_sql_drop_table_completion() { + fn drop_in_advanced_mode_surfaces_all_merged_continuations() { // ADR-0035 §4c: `drop` gained an advanced SQL node - // (`DROP TABLE [IF EXISTS]`). As with the `create`/`insert`/ - // `update`/`delete` shared entry words (ADR-0033 Amendment 3), - // advanced mode surfaces the SQL grammar's completion — here - // just `table` — rather than the DSL subcommands. The DSL drops - // (`drop column` etc.) still parse via fallback; only the - // completion hint differs (and a partial DSL keyword like - // `drop rel` returns an empty list — a mid-word dead end). - // ADR-0035 §13 4i (d)/(e) tracks merging the candidate sets for - // shared entry words, and the user's request to visually - // distinguish simple- vs advanced-mode completions in the hint - // UI (likely by colour); this expectation grows when 4i lands. - assert_eq!(cands("drop ", 5), vec!["table".to_string()]); + // (`DROP TABLE [IF EXISTS]`). With the 4i (d) shared-entry-word + // merge, advanced-mode `drop ` now offers EVERY valid + // continuation across the SQL and DSL nodes, block-ordered Both → + // Advanced → Simple: `table`/`index` (valid in both modes) before + // the DSL-only `column`/`relationship`/`constraint`. + assert_eq!( + cands("drop ", 5), + vec![ + "table".to_string(), + "index".to_string(), + "column".to_string(), + "relationship".to_string(), + "constraint".to_string(), + ] + ); } #[test] diff --git a/src/dsl/walker/mod.rs b/src/dsl/walker/mod.rs index eef1833..1873fe1 100644 --- a/src/dsl/walker/mod.rs +++ b/src/dsl/walker/mod.rs @@ -242,6 +242,11 @@ const fn catalog_key_for_value_type(ty: crate::dsl::types::Type) -> &'static str #[derive(Debug, Clone)] pub struct CompletionProbe { pub expected: Vec, + /// Mode-class for each entry in `expected` (same length / order). + /// All `Both` except where a shared entry word's candidate nodes were + /// merged (ADR-0035 §4i d/e), so the completion engine can block-order + /// (and the UI colour) mixed simple/advanced continuations. + pub expected_modes: Vec, /// Columns of `current_table` resolved at the cursor (set /// by an `Ident { source: Tables, writes_table: true }` /// earlier in the walk). `None` when the walker is @@ -310,8 +315,10 @@ pub fn completion_probe_in_mode( }; if source.trim().is_empty() { + let entries = mode_filtered_entries(); return CompletionProbe { - expected: mode_filtered_entries(), + expected_modes: vec![crate::completion::ModeClass::Both; entries.len()], + expected: entries, current_table_columns: None, pending_hint_mode: None, from_scope: Vec::new(), @@ -322,15 +329,17 @@ pub fn completion_probe_in_mode( ctx.mode = mode; let (result, _cmd) = walk(source, outcome::WalkBound::EndOfInput, &mut ctx); let Some(result) = result else { + let entries = mode_filtered_entries(); return CompletionProbe { - expected: mode_filtered_entries(), + expected_modes: vec![crate::completion::ModeClass::Both; entries.len()], + expected: entries, current_table_columns: None, pending_hint_mode: None, from_scope: Vec::new(), cte_bindings: Vec::new(), }; }; - let expected = match result.outcome { + let mut expected = match result.outcome { outcome::WalkOutcome::Match { .. } => result.tail_expected, // A trailing-junk Mismatch (the shape matched, then the // user kept typing) still carries the outer shape's @@ -377,8 +386,92 @@ pub fn completion_probe_in_mode( } (top_from, ctes) }; + // ADR-0035 §4i (d/e): shared-entry-word completion merge. In advanced + // mode an entry word like `create` / `drop` has several candidate + // nodes (SQL forms + the DSL fallback), but the walk above committed + // to ONE, so only that node's continuations are in `expected`. At the + // entry-word boundary (nothing typed after the entry word yet — the + // divergence point), walk every candidate, keep the viable + // (Incomplete) ones, and union their next-keyword continuations, + // tagging each by the producing category — so completion offers all + // valid continuations and can colour/order them by mode. Deeper + // positions keep the committed walk's `expected` untouched. + let mut expected_modes = vec![crate::completion::ModeClass::Both; expected.len()]; + if mode == crate::mode::Mode::Advanced { + let s = skip_whitespace(source, 0); + if let Some((kw_start, kw_end)) = consume_ident(source, s) + && skip_whitespace(source, kw_end) >= source.len() + { + let entry = &source[kw_start..kw_end]; + let candidates = grammar::commands_for_entry_word(entry); + if candidates.len() > 1 { + use crate::dsl::grammar::CommandCategory; + // (continuation word, produced-by-simple, produced-by-advanced) + let mut tally: Vec<(&'static str, bool, bool)> = Vec::new(); + for (_, node, category) in candidates { + let mut sctx = context::WalkContext::with_schema(schema); + sctx.mode = mode; + let (res, _) = + walk_one_command(source, source, kw_start, kw_end, 0, node, &mut sctx); + let outcome::WalkOutcome::Incomplete { expected: cont, .. } = res.outcome + else { + continue; + }; + let advanced = category == CommandCategory::Advanced; + for e in &cont { + if let outcome::Expectation::Word(w) | outcome::Expectation::Literal(w) = e { + match tally.iter_mut().find(|(kw, _, _)| kw == w) { + Some(rec) => { + if advanced { + rec.2 = true; + } else { + rec.1 = true; + } + } + None => tally.push((w, !advanced, advanced)), + } + } + } + } + if !tally.is_empty() { + // Augment `expected` with merged continuations the + // committed node lacked. + for &(w, _, _) in &tally { + let present = expected.iter().any(|e| { + matches!(e, + outcome::Expectation::Word(x) | outcome::Expectation::Literal(x) + if *x == w) + }); + if !present { + expected.push(outcome::Expectation::Word(w)); + } + } + // Classify every expectation by the merged tally. + expected_modes = expected + .iter() + .map(|e| match e { + outcome::Expectation::Word(w) | outcome::Expectation::Literal(w) => { + tally.iter().find(|(kw, _, _)| kw == w).map_or( + crate::completion::ModeClass::Both, + |&(_, simple, adv)| match (simple, adv) { + (true, true) | (false, false) => { + crate::completion::ModeClass::Both + } + (false, true) => crate::completion::ModeClass::Advanced, + (true, false) => crate::completion::ModeClass::Simple, + }, + ) + } + _ => crate::completion::ModeClass::Both, + }) + .collect(); + } + } + } + } CompletionProbe { expected, + expected_modes, current_table_columns: ctx.current_table_columns, pending_hint_mode: ctx.pending_hint_mode, from_scope,