fix: advanced CREATE TABLE completion cluster
Three completion / hint bugs in the same advanced-mode grammar
+ walker path:
1. `create table T ` offered only `with` (the DSL fallback) — the
`(` continuation for the SQL column-def list (ADR-0035 §4) was
missing because the shared-entry-word completion merge in
`completion_probe_in_mode` only fired at the entry-word boundary.
Broadened to fire at any cursor depth and to handle
`Expectation::Punct` continuations alongside `Word`/`Literal`. A
shared-entry-word candidate whose grammar has already diverged
(e.g. SQL `CREATE INDEX` past `create table …`) returns
Mismatch and is naturally skipped — the viability check stays the
gate, not the cursor depth.
2. `create table T (` showed only the table-level constraint
keywords (`primary`, `unique`, `check`, `constraint`, `foreign`)
in the ambient hint, leaving the column-name role invisible
because COLUMN_DEF starts with an `Ident::NewName` slot that
produces no concrete candidate. Added a new `HintMode::IntroProse(
&'static str)` variant that surfaces catalog prose at slot entry
without suppressing Tab completion (unlike `ProseOnly`) and
without requiring `typing_name_at_cursor` to fire (unlike
`ForceProse`). Wrapped ELEMENT in `Node::Hinted { mode: IntroProse(
"hint.create_table_element"), … }`, with prose "Type a column
name, or a table-level constraint: `primary`, `unique`, `check`,
`constraint`, `foreign`". Tab still cycles every keyword.
3. The SQL_TYPE position leaked the bare keyword `double` (the
first token of the dedicated `double precision` Choice branch
per ADR-0035 §6.3) alongside the playground's regular type list.
Added `("double", "double precision")` to `COMPOSITE_CANDIDATES`
and extended the keyword filter to drop composite openers so the
composite phrase replaces the bare opener instead of appearing
alongside it. Tab now offers `double precision` as a single
coherent candidate; the partial-typing prose at the same slot is
subsumed by item 2's IntroProse (the user reads "Type a column
name…" while mid-typing, then advances to the clean type list).
Tests added (4): pinning each behavioural promise above plus the
no-leakage assertion at the partial-typing prose position. Full
suite 2035 passed / 0 failed / 0 unexpected skips. Clippy clean.
The new `HintMode::IntroProse` variant is an additive extension to
the ADR-0024 HintMode-per-node model; no behaviour change to
existing modes. An ADR-0024 amendment recording it can follow later
if desired — flagged but not written.
This commit is contained in:
claude@clouddev1
+22
-5
@@ -25,11 +25,22 @@ use crate::mode::Mode;
|
||||
/// one token but which the user types as a single fluent piece.
|
||||
/// Pairs of (walker-expected-literal, full-composite-text).
|
||||
///
|
||||
/// When the walker reports `Expectation::Literal(opener)` at the
|
||||
/// cursor, the engine surfaces the full composite text as a Tab
|
||||
/// candidate. Today the only entry is `1:n` (the opener for
|
||||
/// `add 1:n relationship`) — adding more is a one-line edit.
|
||||
const COMPOSITE_CANDIDATES: &[(&str, &str)] = &[("1", "1:n")];
|
||||
/// When the walker reports `Expectation::Literal(opener)` or
|
||||
/// `Expectation::Word(opener)` at the cursor, the engine surfaces the
|
||||
/// full composite text as a Tab candidate. Used for multi-token
|
||||
/// fragments the user thinks of as a single phrase:
|
||||
///
|
||||
/// - `1:n` — the opener for `add 1:n relationship`.
|
||||
/// - `double precision` — the lone two-word SQL type alias
|
||||
/// (ADR-0035 §6.3; the grammar has a dedicated branch so the per-word
|
||||
/// `Ident` validator never has to make sense of `double` alone).
|
||||
/// Surfacing it as a composite stops bare `double` from appearing in
|
||||
/// the type candidate list alongside `int`/`text`/etc. (issue #5).
|
||||
/// Source 1's keyword filter drops openers that appear here so the
|
||||
/// composite replaces the bare opener rather than appearing
|
||||
/// alongside it.
|
||||
const COMPOSITE_CANDIDATES: &[(&str, &str)] =
|
||||
&[("1", "1:n"), ("double", "double precision")];
|
||||
|
||||
/// Per-project schema lookup cache (ADR-0022 §9, ADR-0024 §Phase D).
|
||||
///
|
||||
@@ -525,6 +536,11 @@ pub fn candidates_at_cursor_with_in_mode(
|
||||
// Declaration order is preserved (matches the canonical
|
||||
// command shape, e.g. `to` before `table` for
|
||||
// `add column [to] [table] …`).
|
||||
//
|
||||
// Composite openers (e.g. `double` for `double precision`) are
|
||||
// filtered out here so Source 1.6 (the composite pipeline) can
|
||||
// surface the full multi-word candidate without the bare opener
|
||||
// also appearing alongside (issue #5).
|
||||
let mut keywords: Vec<String> = expected
|
||||
.iter()
|
||||
.filter_map(|e| match e {
|
||||
@@ -532,6 +548,7 @@ pub fn candidates_at_cursor_with_in_mode(
|
||||
_ => None,
|
||||
})
|
||||
.filter(|w| !w.is_empty() && w.chars().all(|c| c.is_ascii_alphabetic()))
|
||||
.filter(|w| !COMPOSITE_CANDIDATES.iter().any(|(opener, _)| opener == w))
|
||||
.map(str::to_string)
|
||||
.filter(|name| matches_prefix(name))
|
||||
.collect();
|
||||
|
||||
@@ -149,6 +149,15 @@ impl IdentSource {
|
||||
/// - `ForceProse(catalog_key)` — force this prose at the
|
||||
/// catalog key regardless of candidates. Used today by
|
||||
/// `NewName` ident slots ("Type a name, then `(`").
|
||||
/// - `IntroProse(catalog_key)` — show prose at slot entry to
|
||||
/// *introduce* a position whose first-class candidate is an
|
||||
/// ident slot (which would be invisible in a pure-candidate
|
||||
/// render) but whose keyword alternatives are also available.
|
||||
/// Unlike `ProseOnly`, Tab candidates remain available — the
|
||||
/// user still cycles through the keyword set. Used at the
|
||||
/// advanced-mode CREATE TABLE element slot, where the
|
||||
/// column-name `NewName` slot would otherwise be invisible
|
||||
/// alongside the table-level constraint keywords (issue #4).
|
||||
/// - `SuppressProse` — show only candidates; never fall back
|
||||
/// to a prose ladder.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
@@ -156,6 +165,7 @@ pub enum HintMode {
|
||||
Default,
|
||||
ForceProse(&'static str),
|
||||
ProseOnly(&'static str),
|
||||
IntroProse(&'static str),
|
||||
SuppressProse,
|
||||
}
|
||||
|
||||
|
||||
@@ -390,7 +390,18 @@ const TABLE_FK_NAMED: Node = Node::Seq(TABLE_FK_NAMED_NODES);
|
||||
// the same trade real SQL makes with its reserved words.)
|
||||
static ELEMENT_CHOICES: &[Node] =
|
||||
&[TABLE_PK, TABLE_UNIQUE, TABLE_CHECK, TABLE_FK_NAMED, TABLE_FK, COLUMN_DEF];
|
||||
const ELEMENT: Node = Node::Choice(ELEMENT_CHOICES);
|
||||
const ELEMENT_INNER: Node = Node::Choice(ELEMENT_CHOICES);
|
||||
// Issue #4: wrap the element slot in `IntroProse` so a fresh element
|
||||
// position (`create table T (` and after every `,`) surfaces a prose
|
||||
// hint that names the column-name role *and* the table-level
|
||||
// constraint keywords. The bare candidate render shows only the
|
||||
// constraint keywords because the `COLUMN_DEF` branch starts with a
|
||||
// `NewName` ident that has no concrete candidate to offer; the prose
|
||||
// makes the dominant first move visible without suppressing Tab.
|
||||
const ELEMENT: Node = Node::Hinted {
|
||||
mode: crate::dsl::grammar::HintMode::IntroProse("hint.create_table_element"),
|
||||
inner: &ELEMENT_INNER,
|
||||
};
|
||||
|
||||
static COLUMN_LIST_NODES: &[Node] = &[
|
||||
Node::Punct('('),
|
||||
|
||||
+49
-19
@@ -147,13 +147,15 @@ pub fn hint_resolution_at_input_in_mode(
|
||||
// value-literal fallback slot; `ForceProse` covers `NewName`
|
||||
// ident slots ("Type a name").
|
||||
match snap.pending_hint_mode {
|
||||
Some(mode @ (HintMode::ProseOnly(_) | HintMode::ForceProse(_))) => {
|
||||
Some(HintResolution {
|
||||
Some(
|
||||
mode @ (HintMode::ProseOnly(_)
|
||||
| HintMode::ForceProse(_)
|
||||
| HintMode::IntroProse(_)),
|
||||
) => Some(HintResolution {
|
||||
mode,
|
||||
column: None,
|
||||
form_b_autogen_skipped: Vec::new(),
|
||||
})
|
||||
}
|
||||
}),
|
||||
Some(HintMode::SuppressProse | HintMode::Default) | None => None,
|
||||
}
|
||||
}
|
||||
@@ -386,28 +388,39 @@ 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.
|
||||
// ADR-0035 §4i (d/e) + issue #3 (2026-05-28): 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`. Walk every viable
|
||||
// (`Incomplete`) candidate from the entry word to the cursor, union
|
||||
// their next-token continuations, tag each by the producing category
|
||||
// — so completion offers all valid continuations at the current
|
||||
// depth and can colour/order them by mode.
|
||||
//
|
||||
// Originally limited to the entry-word boundary; broadened to every
|
||||
// depth so that e.g. `create table T ` in advanced mode surfaces
|
||||
// both `with` (DSL, ADR-0009) and `(` (SQL, ADR-0035 §4) as
|
||||
// continuations. A candidate whose grammar has already diverged
|
||||
// (e.g. SQL `CREATE INDEX` after `create table …`) returns
|
||||
// Mismatch and is naturally skipped — the viability check is the
|
||||
// gate, not the cursor depth.
|
||||
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()
|
||||
{
|
||||
if let Some((kw_start, kw_end)) = consume_ident(source, s) {
|
||||
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();
|
||||
// Continuations that aren't keyword/literal-shaped
|
||||
// (notably `Punct`, used for `(` at the SQL column-list
|
||||
// boundary — issue #3). Tracked separately because the
|
||||
// tally is keyed by `&'static str`; mode classification
|
||||
// for punctuation defaults to `Both`.
|
||||
let mut punct_tally: Vec<char> = Vec::new();
|
||||
for (_, node, category) in candidates {
|
||||
let mut sctx = context::WalkContext::with_schema(schema);
|
||||
sctx.mode = mode;
|
||||
@@ -419,7 +432,9 @@ pub fn completion_probe_in_mode(
|
||||
};
|
||||
let advanced = category == CommandCategory::Advanced;
|
||||
for e in &cont {
|
||||
if let outcome::Expectation::Word(w) | outcome::Expectation::Literal(w) = e {
|
||||
match e {
|
||||
outcome::Expectation::Word(w)
|
||||
| outcome::Expectation::Literal(w) => {
|
||||
match tally.iter_mut().find(|(kw, _, _)| kw == w) {
|
||||
Some(rec) => {
|
||||
if advanced {
|
||||
@@ -431,9 +446,14 @@ pub fn completion_probe_in_mode(
|
||||
None => tally.push((w, !advanced, advanced)),
|
||||
}
|
||||
}
|
||||
outcome::Expectation::Punct(c) if !punct_tally.contains(c) => {
|
||||
punct_tally.push(*c);
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
if !tally.is_empty() {
|
||||
}
|
||||
if !tally.is_empty() || !punct_tally.is_empty() {
|
||||
// Augment `expected` with merged continuations the
|
||||
// committed node lacked.
|
||||
for &(w, _, _) in &tally {
|
||||
@@ -446,7 +466,17 @@ pub fn completion_probe_in_mode(
|
||||
expected.push(outcome::Expectation::Word(w));
|
||||
}
|
||||
}
|
||||
for &c in &punct_tally {
|
||||
let present = expected
|
||||
.iter()
|
||||
.any(|e| matches!(e, outcome::Expectation::Punct(x) if *x == c));
|
||||
if !present {
|
||||
expected.push(outcome::Expectation::Punct(c));
|
||||
}
|
||||
}
|
||||
// Classify every expectation by the merged tally.
|
||||
// Word/Literal route through the tally; Punct (and
|
||||
// other expectation shapes) default to `Both`.
|
||||
expected_modes = expected
|
||||
.iter()
|
||||
.map(|e| match e {
|
||||
|
||||
@@ -211,6 +211,10 @@ pub const KEYS_AND_PLACEHOLDERS: &[(&str, &[&str])] = &[
|
||||
&["kind", "found"],
|
||||
),
|
||||
("hint.ambient_typing_name", &[]),
|
||||
// Issue #4: introduce the advanced-mode CREATE TABLE element
|
||||
// slot (`create table T (`) so the otherwise-invisible
|
||||
// column-name role reads as the dominant first move.
|
||||
("hint.create_table_element", &[]),
|
||||
("hint.value_literal_slot", &[]),
|
||||
(
|
||||
"hint.ambient_typing_name_then",
|
||||
|
||||
@@ -357,6 +357,10 @@ hint:
|
||||
# falls through to `ambient_typing_name` instead.
|
||||
ambient_typing_name: "Type a name"
|
||||
ambient_typing_name_then: "Type a name, then {next}"
|
||||
# Issue #4 — advanced-mode CREATE TABLE element slot. Surfaced
|
||||
# at `create table T (` so the column-name role is visible
|
||||
# alongside the table-level constraint keywords.
|
||||
create_table_element: "Type a column name, or a table-level constraint: `primary`, `unique`, `check`, `constraint`, `foreign`"
|
||||
# Value-literal slot — `insert ... values (`, `update ... set
|
||||
# col=`, `where col=`. Replaces the misleading "null true
|
||||
# false" keyword candidate list with format guidance for all
|
||||
|
||||
@@ -693,6 +693,17 @@ fn ambient_hint_core_in_mode(
|
||||
return Some(AmbientHint::Prose(text));
|
||||
}
|
||||
}
|
||||
Some(crate::dsl::grammar::HintMode::IntroProse(key)) => {
|
||||
// Slot entry: surface the catalog prose so an
|
||||
// invisible-by-default ident slot (the column-name
|
||||
// `NewName` at the CREATE TABLE element position,
|
||||
// issue #4) reads as the dominant first move with
|
||||
// the keyword alternatives folded into the prose.
|
||||
// Tab candidates remain available via the parallel
|
||||
// completion surface; the user still cycles the
|
||||
// keyword set.
|
||||
return Some(AmbientHint::Prose(crate::friendly::translate(key, &[])));
|
||||
}
|
||||
Some(crate::dsl::grammar::HintMode::SuppressProse | crate::dsl::grammar::HintMode::Default)
|
||||
| None => {}
|
||||
}
|
||||
@@ -1111,6 +1122,143 @@ mod tests {
|
||||
assert!(ambient_hint(" ", 3, None, &empty_cache()).is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn advanced_create_table_element_position_introduces_column_name() {
|
||||
// Issue #4: at `create table T (`, the user is at the
|
||||
// ELEMENT slot of the column-def list. The current candidate
|
||||
// list shows only table-level constraint keywords (`primary`,
|
||||
// `unique`, `check`, `constraint`, `foreign`); a new column
|
||||
// is the dominant first move and is currently invisible
|
||||
// because the COLUMN_DEF branch starts with an `Ident::NewName`
|
||||
// slot which produces no concrete candidate.
|
||||
//
|
||||
// The fix wraps the ELEMENT choice in a `Hinted::IntroProse`
|
||||
// that surfaces a prose hint mentioning the column name first,
|
||||
// with the constraint keywords as the alternative. Tab
|
||||
// candidates remain available.
|
||||
let cache = crate::completion::SchemaCache::default();
|
||||
let input = "create table Orders (";
|
||||
match ambient_hint_in_mode(input, input.len(), None, &cache, Mode::Advanced) {
|
||||
Some(AmbientHint::Prose(p)) => {
|
||||
assert!(
|
||||
p.to_lowercase().contains("column name"),
|
||||
"prose must mention `column name`; got: {p:?}",
|
||||
);
|
||||
// Constraint alternatives should still be mentioned.
|
||||
assert!(
|
||||
p.contains("primary") && p.contains("unique"),
|
||||
"prose should mention constraint alternatives; got: {p:?}",
|
||||
);
|
||||
}
|
||||
other => panic!("expected Prose hint at ELEMENT slot; got: {other:?}"),
|
||||
}
|
||||
// Tab candidates should remain available (the keywords still cycle).
|
||||
let comp = crate::completion::candidates_at_cursor_in_mode(
|
||||
input,
|
||||
input.len(),
|
||||
&cache,
|
||||
Mode::Advanced,
|
||||
)
|
||||
.expect("completion must remain available");
|
||||
let texts: Vec<&str> = comp.candidates.iter().map(|c| c.text.as_str()).collect();
|
||||
for kw in &["primary", "unique"] {
|
||||
assert!(
|
||||
texts.contains(kw),
|
||||
"Tab candidate `{kw}` must remain; got {texts:?}",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn advanced_partial_typing_does_not_leak_bare_double_in_prose() {
|
||||
// Issue #5 (prose half): at `create table Orders (count` (no
|
||||
// trailing space), the user is mid-typing what's
|
||||
// grammatically a column name (`count` could be the start of
|
||||
// `counterparty`). The bare `Word("double")` from the
|
||||
// DOUBLE_PRECISION_NODES branch must not appear in the
|
||||
// ambient hint at this position — the new IntroProse hint
|
||||
// from issue #4 already covers this position by introducing
|
||||
// the element slot ("Type a column name, or a table-level
|
||||
// constraint: …"), and the user discovers the type list
|
||||
// (with `double precision` as a single composite, not bare
|
||||
// `double`) when they advance to the SQL_TYPE slot.
|
||||
let cache = crate::completion::SchemaCache::default();
|
||||
let input = "create table Orders (count";
|
||||
match ambient_hint_in_mode(input, input.len(), None, &cache, Mode::Advanced) {
|
||||
Some(AmbientHint::Prose(p)) => {
|
||||
assert!(
|
||||
!p.contains("`double`"),
|
||||
"bare `double` must not appear in the prose; got: {p:?}",
|
||||
);
|
||||
}
|
||||
other => panic!("expected Prose hint at partial column name; got: {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn advanced_type_position_offers_double_precision_not_bare_double() {
|
||||
// Issue #5: at the SQL_TYPE position (`create table Orders
|
||||
// (count `), the candidate list previously surfaced `double`
|
||||
// as a peer of the playground's regular types — the user
|
||||
// sees a leading "double" alongside int/text/etc. and has
|
||||
// to know it's the start of the two-word `double precision`
|
||||
// alias. The fix surfaces `double precision` as a single
|
||||
// composite candidate and suppresses the bare `double`.
|
||||
let cache = crate::completion::SchemaCache::default();
|
||||
let input = "create table Orders (count ";
|
||||
let comp = crate::completion::candidates_at_cursor_in_mode(
|
||||
input,
|
||||
input.len(),
|
||||
&cache,
|
||||
Mode::Advanced,
|
||||
)
|
||||
.expect("completion expected at the SQL_TYPE position");
|
||||
let texts: Vec<&str> = comp.candidates.iter().map(|c| c.text.as_str()).collect();
|
||||
assert!(
|
||||
!texts.contains(&"double"),
|
||||
"bare `double` must NOT appear as a type candidate; got {texts:?}",
|
||||
);
|
||||
assert!(
|
||||
texts.contains(&"double precision"),
|
||||
"`double precision` should appear as a composite type candidate; got {texts:?}",
|
||||
);
|
||||
// The regular type vocabulary still appears.
|
||||
for t in &["int", "text", "real", "serial"] {
|
||||
assert!(
|
||||
texts.iter().any(|x| x == t),
|
||||
"regular type `{t}` must remain a candidate; got {texts:?}",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn advanced_create_table_offers_open_paren_after_name() {
|
||||
// Issue #3: typing `create table Orders ` in advanced mode
|
||||
// should offer both `with` (DSL form, ADR-0009) and `(` (SQL
|
||||
// form, ADR-0035 §4) as the next-step continuation. Today
|
||||
// only `with` surfaces — the shared-entry-word completion
|
||||
// merge only fires at the entry-word boundary, so deeper
|
||||
// positions show only the committed node's continuations.
|
||||
let cache = crate::completion::SchemaCache::default();
|
||||
let input = "create table Orders ";
|
||||
let comp = crate::completion::candidates_at_cursor_in_mode(
|
||||
input,
|
||||
input.len(),
|
||||
&cache,
|
||||
Mode::Advanced,
|
||||
)
|
||||
.expect("completion expected for advanced create-table after name");
|
||||
let texts: Vec<&str> = comp.candidates.iter().map(|c| c.text.as_str()).collect();
|
||||
assert!(
|
||||
texts.contains(&"("),
|
||||
"advanced mode must offer `(` for the SQL column-def list; got {texts:?}",
|
||||
);
|
||||
assert!(
|
||||
texts.contains(&"with"),
|
||||
"advanced mode must keep `with` for the DSL form; got {texts:?}",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn advanced_mode_ambient_offers_sql_from_slot_candidate() {
|
||||
// ADR-0022 Amendment 1: advanced-mode ambient assistance
|
||||
|
||||
Reference in New Issue
Block a user