From c485189da8ba7629e120cd7e285ff9133fde1f42 Mon Sep 17 00:00:00 2001 From: "claude@clouddev1" Date: Fri, 15 May 2026 18:33:52 +0000 Subject: [PATCH] ADR-0024 Phase D: include column name in value-slot hint prose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User-facing improvement: typing into a value slot now surfaces the column name in the hint. The hint at `insert into Customers values (` (first column id:int) reads "for `id`: Type an integer (e.g. 42, -7) or null" instead of the generic "Type an integer …" prose. After `1, ` the panel updates to the second column ("for `Name`: Type a quoted string …"). The same applies to `update T set Email=` and `delete from T where ts=` — the catalog wrapper threads the column name through. Implementation: **`Node::TypedValueSlot.column_name: Option<&'static str>`** (new field, `src/dsl/grammar/mod.rs`). When `Some`, walker writes `WalkContext::pending_value_column` on entry; clears along with `pending_value_type` on inner success. **Walker driver writes both names** (`src/dsl/walker/driver.rs`): - `Node::TypedValueSlot` dispatch reads `column_name` and populates `pending_value_column`. - `Ident { writes_column: true }` dispatch also writes `pending_value_column` (using the schema-canonical name when available, falling back to the user's spelling) so update set / where positions surface the column name. **Shared sub-grammars** (`src/dsl/grammar/shared.rs`): - New `slot_for_column(ty, name)` builds a `TypedValueSlot` with the embedded leaked column name. Used by `column_value_list`. - New `slot_inner_for_type(ty)` returns just the Choice (without TypedValueSlot wrapper) for slot_for_column to rebuild. - `column_value_list` factory now constructs per-column slots via `slot_for_column(col.user_type, &col.name)`. Each slot leaks its column name string with the same per-walk Box::leak pattern the rest of dynamic dispatch uses. **`WalkContext::pending_value_column: Option`** (new field, `src/dsl/walker/context.rs`). Pairs with `pending_value_type` to give the hint resolver both pieces. **Single-walk hint resolver** (`src/dsl/walker/mod.rs`): - New `HintResolution { mode: HintMode, column: Option }` struct. - New `hint_resolution_at_input(source, schema) -> Option< HintResolution>` runs one walk and reports both pieces. The ambient_hint dispatch composes per-column prose from the result. - Existing `hint_mode_at_input` / `hint_mode_at_input_with_schema` preserved as thinner wrappers for tests / future callers that don't need the column name. **Catalog wrapper** (`src/friendly/strings/en-US.yaml`, `src/friendly/keys.rs`): - New `hint.value_slot_for_column: "for `{column}`: {detail}"` prefixes the per-type prose with the actual column name when the walker has it bound. Schemaless fallback continues to use the generic value-literal prose with no column prefix. **ambient_hint composes** (`src/input_render.rs`): consults `hint_resolution_at_input`; when `column` is `Some`, wraps the type prose through `hint.value_slot_for_column`; otherwise emits the bare type prose. Tests (846 total, 0 failing): - 4 new input_render tests assert column names appear in the prose at insert/update/where positions plus the second-insert-value position (proves column tracking advances with comma). - All existing tests pass unchanged — the column-name addition is layered on top of the type-only prose path. Clippy clean. --- src/dsl/grammar/mod.rs | 13 ++-- src/dsl/grammar/shared.rs | 68 ++++++++++++++++++--- src/dsl/walker/context.rs | 17 ++++++ src/dsl/walker/driver.rs | 30 ++++++++-- src/dsl/walker/mod.rs | 92 +++++++++++++++++++++++++++- src/friendly/keys.rs | 1 + src/friendly/strings/en-US.yaml | 4 ++ src/input_render.rs | 102 ++++++++++++++++++++++++++++++-- 8 files changed, 304 insertions(+), 23 deletions(-) diff --git a/src/dsl/grammar/mod.rs b/src/dsl/grammar/mod.rs index 352044c..d2e8449 100644 --- a/src/dsl/grammar/mod.rs +++ b/src/dsl/grammar/mod.rs @@ -287,12 +287,17 @@ pub enum Node { /// column type in `WalkContext::pending_value_type` so the /// hint resolver can emit per-type catalog prose ("Type an /// integer", "Type a date as 'YYYY-MM-DD'", …) at empty - /// prefix at this slot. The recorded type clears on a - /// successful inner match — so positions BETWEEN typed - /// slots (`insert into T values (1` mid-input) don't carry - /// a stale hint type. + /// prefix at this slot. When `column_name` is `Some`, the + /// walker also writes `pending_value_column` so the hint + /// can be rendered with the actual column name (e.g. "for + /// `Email`: Type a quoted string …") rather than a generic + /// type hint. The recorded values clear on a successful + /// inner match — so positions BETWEEN typed slots + /// (`insert into T values (1` mid-input) don't carry stale + /// hint state. TypedValueSlot { ty: crate::dsl::types::Type, + column_name: Option<&'static str>, inner: &'static Self, }, } diff --git a/src/dsl/grammar/shared.rs b/src/dsl/grammar/shared.rs index 49fe365..1478184 100644 --- a/src/dsl/grammar/shared.rs +++ b/src/dsl/grammar/shared.rs @@ -189,10 +189,12 @@ const INT_SLOT_CHOICES: &[Node] = &[ const INT_SLOT_INNER: Node = Node::Choice(INT_SLOT_CHOICES); const INT_SLOT: Node = Node::TypedValueSlot { ty: Type::Int, + column_name: None, inner: &INT_SLOT_INNER, }; const SERIAL_SLOT: Node = Node::TypedValueSlot { ty: Type::Serial, + column_name: None, inner: &INT_SLOT_INNER, }; @@ -200,6 +202,7 @@ const REAL_SLOT_CHOICES: &[Node] = &[Node::NumberLit { validator: None }, NULL_W const REAL_SLOT_INNER: Node = Node::Choice(REAL_SLOT_CHOICES); const REAL_SLOT: Node = Node::TypedValueSlot { ty: Type::Real, + column_name: None, inner: &REAL_SLOT_INNER, }; @@ -212,6 +215,7 @@ const DECIMAL_SLOT_CHOICES: &[Node] = &[ const DECIMAL_SLOT_INNER: Node = Node::Choice(DECIMAL_SLOT_CHOICES); const DECIMAL_SLOT: Node = Node::TypedValueSlot { ty: Type::Decimal, + column_name: None, inner: &DECIMAL_SLOT_INNER, }; @@ -223,6 +227,7 @@ const BOOL_SLOT_CHOICES: &[Node] = &[ const BOOL_SLOT_INNER: Node = Node::Choice(BOOL_SLOT_CHOICES); const BOOL_SLOT: Node = Node::TypedValueSlot { ty: Type::Bool, + column_name: None, inner: &BOOL_SLOT_INNER, }; @@ -230,6 +235,7 @@ const TEXT_SLOT_CHOICES: &[Node] = &[Node::StringLit, NULL_WORD]; const TEXT_SLOT_INNER: Node = Node::Choice(TEXT_SLOT_CHOICES); const TEXT_SLOT: Node = Node::TypedValueSlot { ty: Type::Text, + column_name: None, inner: &TEXT_SLOT_INNER, }; @@ -238,30 +244,36 @@ const TEXT_SLOT: Node = Node::TypedValueSlot { // the YYYY-MM-DD / YYYY-MM-DD HH:MM:SS format examples. const DATE_SLOT: Node = Node::TypedValueSlot { ty: Type::Date, + column_name: None, inner: &TEXT_SLOT_INNER, }; const DATETIME_SLOT: Node = Node::TypedValueSlot { ty: Type::DateTime, + column_name: None, inner: &TEXT_SLOT_INNER, }; const BLOB_SLOT: Node = Node::TypedValueSlot { ty: Type::Blob, + column_name: None, inner: &TEXT_SLOT_INNER, }; // shortid columns store base58 text (ADR-0011 fk_target_type // shortid → text); the slot accepts a quoted-text literal or -// null. The pre-Phase-D plain-Choice scaffolding had this -// mapped to INT_SLOT (a holdover from the chumsky-side generic -// VALUE_LITERAL fallback). Per-type dispatch corrects that. +// null. const SHORTID_SLOT: Node = Node::TypedValueSlot { ty: Type::ShortId, + column_name: None, inner: &TEXT_SLOT_INNER, }; /// Dispatch a value slot per user-facing type -/// (ADR-0024 §slot_for_type). Returns the same node every time -/// for a given Type — fine to call from within a -/// `DynamicSubgrammar` factory. +/// (ADR-0024 §slot_for_type). +/// +/// Returns the same node every time for a given Type — fine to +/// call from within a `DynamicSubgrammar` factory. The +/// returned slot does not carry a column name; callers that +/// have one (e.g. `column_value_list` building per-column +/// slots) should use [`slot_for_column`] instead. #[must_use] pub const fn slot_for_type(ty: Type) -> Node { match ty { @@ -278,6 +290,45 @@ pub const fn slot_for_type(ty: Type) -> Node { } } +/// Look up just the inner Choice (no `TypedValueSlot` wrapper) +/// for a given user-facing type. Used by `slot_for_column` to +/// rebuild a TypedValueSlot with an embedded column name. +const fn slot_inner_for_type(ty: Type) -> &'static Node { + match ty { + Type::Int | Type::Serial => &INT_SLOT_INNER, + Type::Real => &REAL_SLOT_INNER, + Type::Decimal => &DECIMAL_SLOT_INNER, + Type::Bool => &BOOL_SLOT_INNER, + Type::Text | Type::Date | Type::DateTime | Type::Blob | Type::ShortId => { + &TEXT_SLOT_INNER + } + } +} + +/// Build a typed value slot with an embedded column name +/// (ADR-0024 §Phase D §typed-value-slots). Used by +/// `column_value_list` to attach each column's name so the +/// hint resolver can render "for ``:" prefixes. +/// +/// The walker writes the (leaked) name into +/// `WalkContext::pending_value_column` on entry to the slot +/// and clears it on successful inner match. +#[must_use] +fn slot_for_column(ty: Type, name: &str) -> Node { + // `Box::leak`: column names from the schema cache need a + // `&'static str`-compatible lifetime to plug into the + // static Node enum. The leak is per dynamic walk (factory + // invocation), bounded by the column count — consistent + // with the `DynamicSubgrammar` Box::leak in the walker + // driver. + let leaked: &'static str = Box::leak(name.to_string().into_boxed_str()); + Node::TypedValueSlot { + ty, + column_name: Some(leaked), + inner: slot_inner_for_type(ty), + } +} + // ================================================================= // Dynamic sub-grammar: column_value_list // ================================================================= @@ -329,12 +380,15 @@ pub fn column_value_list(ctx: &WalkContext) -> Node { return FALLBACK_VALUE_LIST; } // Build a Seq of typed slots interleaved with commas. + // Each slot embeds its column name so the hint resolver + // can mention the column by name ("for `Email`: Type a + // quoted string …"). let mut children: Vec = Vec::with_capacity(cols.len() * 2); for (i, col) in cols.iter().enumerate() { if i > 0 { children.push(Node::Punct(',')); } - children.push(slot_for_type(col.user_type)); + children.push(slot_for_column(col.user_type, &col.name)); } Node::Seq(Box::leak(children.into_boxed_slice())) } diff --git a/src/dsl/walker/context.rs b/src/dsl/walker/context.rs index 979d521..f474e4c 100644 --- a/src/dsl/walker/context.rs +++ b/src/dsl/walker/context.rs @@ -38,6 +38,22 @@ pub struct WalkContext<'a> { /// emit per-type prose ("Type an integer", "Type a date as /// 'YYYY-MM-DD'", …) at empty prefix at typed value slots. pub pending_value_type: Option, + /// The column name (if known) the walker is about to + /// consume a value for. + /// + /// Populated by: + /// - `Ident { source: Columns, writes_column: true }` for + /// `update set =` and `where =` positions, where + /// the column ident matches in the path immediately + /// before the value slot. + /// - `Node::TypedValueSlot { column_name: Some(name), … }` + /// for the per-column typed slots in `column_value_list` + /// (insert-into-T-values positions, where the column name + /// is keyed by position in the table's column list). + /// + /// Cleared on successful inner match alongside + /// `pending_value_type`. + pub pending_value_column: Option, } impl<'a> WalkContext<'a> { @@ -60,6 +76,7 @@ impl<'a> WalkContext<'a> { current_table_columns: None, current_column: None, pending_value_type: None, + pending_value_column: None, } } } diff --git a/src/dsl/walker/driver.rs b/src/dsl/walker/driver.rs index 4528f5f..48f6f6c 100644 --- a/src/dsl/walker/driver.rs +++ b/src/dsl/walker/driver.rs @@ -130,17 +130,27 @@ pub fn walk_node( let resolved: &'static Node = Box::leak(Box::new(factory(ctx))); walk_node(source, pos, resolved, ctx, path, per_byte) } - Node::TypedValueSlot { ty, inner } => { + Node::TypedValueSlot { + ty, + column_name, + inner, + } => { // ADR-0024 §Phase D §typed-value-slots. Tag the // pending column type so the hint resolver can emit - // per-type prose at empty prefix. Clear on - // successful inner match — positions BETWEEN typed - // slots (post-comma, between values) don't carry a - // stale hint type. + // per-type prose at empty prefix. If a column name + // is embedded (insert column_value_list path), tag + // that too so the hint can mention the column by + // name. Clear on successful inner match — positions + // BETWEEN typed slots (post-comma, between values) + // don't carry stale hint state. ctx.pending_value_type = Some(*ty); + if let Some(name) = column_name { + ctx.pending_value_column = Some((*name).to_string()); + } let result = walk_node(source, pos, inner, ctx, path, per_byte); if matches!(result, NodeWalkResult::Matched { .. }) { ctx.pending_value_type = None; + ctx.pending_value_column = None; } result } @@ -268,6 +278,16 @@ fn walk_ident( .find(|c| c.name.eq_ignore_ascii_case(&text)) .cloned() }); + // Surface the column name to the hint resolver too — + // this is the `update set =` / `where =` + // path. The matching column's canonical name (from the + // schema) wins over the user's spelling so the hint + // mirrors what's in the schema. + ctx.pending_value_column = ctx + .current_column + .as_ref() + .map(|c| c.name.clone()) + .or_else(|| Some(text.clone())); } path.push(MatchedItem { kind: MatchedKind::Ident { role }, diff --git a/src/dsl/walker/mod.rs b/src/dsl/walker/mod.rs index 223bfbd..21ffbdd 100644 --- a/src/dsl/walker/mod.rs +++ b/src/dsl/walker/mod.rs @@ -64,6 +64,80 @@ pub fn hint_mode_at_input_with_schema( hint_mode_at_input_inner(source, Some(schema)) } +/// Resolution of the hint-panel mode at the cursor, plus the +/// column name (if known) the cursor's value slot is keyed on. +/// +/// Returned by [`hint_resolution_at_input`]. The renderer +/// composes per-column prose ("for `Email`: Type a quoted +/// string …") when `column` is `Some`. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct HintResolution { + pub mode: crate::dsl::grammar::HintMode, + pub column: Option, +} + +/// Single-walk hint resolver (ADR-0024 §Phase D §typed-value-slots). +/// +/// Walks `source` against `schema`, then reports both the +/// resolved `HintMode` and the walker's `pending_value_column` +/// (if any). Returns `None` when no HintMode applies. +#[must_use] +pub fn hint_resolution_at_input( + source: &str, + schema: Option<&crate::completion::SchemaCache>, +) -> Option { + use crate::dsl::grammar::{HintMode, IdentSource}; + use crate::dsl::walker::outcome::Expectation; + + let (expected, pending_type, pending_column) = + expected_for_hint_with_full_ctx(source, schema); + if expected.is_empty() { + return None; + } + + if let Some(ty) = pending_type { + return Some(HintResolution { + mode: HintMode::ProseOnly(catalog_key_for_value_type(ty)), + column: pending_column, + }); + } + + let has_word = |w: &str| { + expected + .iter() + .any(|e| matches!(e, Expectation::Word(x) if *x == w)) + }; + let value_literal_slot = has_word("null") + && has_word("true") + && has_word("false") + && expected.iter().any(|e| matches!(e, Expectation::NumberLit)) + && expected.iter().any(|e| matches!(e, Expectation::StringLit)); + if value_literal_slot { + return Some(HintResolution { + mode: HintMode::ProseOnly("hint.value_literal_slot"), + column: None, + }); + } + + let new_name_slot = expected.iter().any(|e| { + matches!( + e, + Expectation::Ident { + source: IdentSource::NewName, + .. + } + ) + }); + if new_name_slot { + return Some(HintResolution { + mode: HintMode::ForceProse("hint.ambient_typing_name"), + column: None, + }); + } + + None +} + fn hint_mode_at_input_inner( source: &str, schema: Option<&crate::completion::SchemaCache>, @@ -209,6 +283,18 @@ fn expected_for_hint_with_ctx( source: &str, schema: Option<&crate::completion::SchemaCache>, ) -> (Vec, Option) { + let (expected, ty, _col) = expected_for_hint_with_full_ctx(source, schema); + (expected, ty) +} + +fn expected_for_hint_with_full_ctx( + source: &str, + schema: Option<&crate::completion::SchemaCache>, +) -> ( + Vec, + Option, + Option, +) { use crate::dsl::grammar::REGISTRY; if source.trim().is_empty() { @@ -216,7 +302,7 @@ fn expected_for_hint_with_ctx( .iter() .map(|c| outcome::Expectation::Word(c.entry.primary)) .collect(); - return (expected, None); + return (expected, None, None); } let mut ctx = schema.map_or_else(context::WalkContext::new, |s| { context::WalkContext::with_schema(s) @@ -227,7 +313,7 @@ fn expected_for_hint_with_ctx( .iter() .map(|c| outcome::Expectation::Word(c.entry.primary)) .collect(); - return (expected, None); + return (expected, None, None); }; let expected = match result.outcome { outcome::WalkOutcome::Match { .. } | outcome::WalkOutcome::ValidationFailed { .. } => { @@ -236,7 +322,7 @@ fn expected_for_hint_with_ctx( outcome::WalkOutcome::Incomplete { expected, .. } | outcome::WalkOutcome::Mismatch { expected, .. } => expected, }; - (expected, ctx.pending_value_type) + (expected, ctx.pending_value_type, ctx.pending_value_column) } /// Public walk entry. `bound` is `EndOfInput` for parse; diff --git a/src/friendly/keys.rs b/src/friendly/keys.rs index 262b9ec..91c698a 100644 --- a/src/friendly/keys.rs +++ b/src/friendly/keys.rs @@ -149,6 +149,7 @@ pub const KEYS_AND_PLACEHOLDERS: &[(&str, &[&str])] = &[ ("hint.value_slot_serial", &[]), ("hint.value_slot_shortid", &[]), ("hint.value_slot_text", &[]), + ("hint.value_slot_for_column", &["column", "detail"]), // ---- Parse error rendering ---- ("parse.available_commands", &["commands"]), ("parse.caret", &["padding"]), diff --git a/src/friendly/strings/en-US.yaml b/src/friendly/strings/en-US.yaml index da9668c..b3d23b2 100644 --- a/src/friendly/strings/en-US.yaml +++ b/src/friendly/strings/en-US.yaml @@ -303,6 +303,10 @@ hint: value_slot_blob: "Type a quoted blob literal or null" value_slot_serial: "Type an integer (or omit to auto-generate) or null" value_slot_shortid: "Type a quoted shortid (or omit to auto-generate) or null" + # Wrapper that prefixes a per-column-type slot hint with the + # actual column name so the user sees "for `Email`: Type a + # quoted string …" instead of the generic type prose. + value_slot_for_column: "for `{column}`: {detail}" parse: # Wrapper around chumsky's structural error message. The diff --git a/src/input_render.rs b/src/input_render.rs index 3a8f50f..6c079fb 100644 --- a/src/input_render.rs +++ b/src/input_render.rs @@ -191,9 +191,11 @@ pub fn ambient_hint( let leading = hint_leading_slice(input, cursor); // ADR-0024 §Phase D §typed-value-slots: pass the schema so // the resolver can narrow value-slot prose per column type - // (Date → "Type a date as 'YYYY-MM-DD'", etc.). - let hint_mode = crate::dsl::walker::hint_mode_at_input_with_schema(leading, cache); - match hint_mode { + // (Date → "Type a date as 'YYYY-MM-DD'", etc.) and surface + // the column name when the walker has it bound. + let resolution = + crate::dsl::walker::hint_resolution_at_input(leading, Some(cache)); + match resolution.as_ref().map(|r| r.mode) { Some(crate::dsl::grammar::HintMode::ProseOnly(key)) => { // The cursor sits at a slot where Tab candidates // would be actively misleading. Surface the catalog @@ -201,7 +203,16 @@ pub fn ambient_hint( // once the user starts typing a partial, normal // candidate completion (e.g. `n` → `null`) applies. if cursor_partial_is_empty(input, cursor) { - return Some(AmbientHint::Prose(crate::friendly::translate(key, &[]))); + let detail = crate::friendly::translate(key, &[]); + let composed = match resolution.and_then(|r| r.column) { + Some(column) => crate::t!( + "hint.value_slot_for_column", + column = column, + detail = detail + ), + None => detail, + }; + return Some(AmbientHint::Prose(composed)); } } Some(crate::dsl::grammar::HintMode::ForceProse(_key)) => { @@ -662,6 +673,89 @@ mod tests { } } + #[test] + fn ambient_hint_at_insert_first_value_mentions_column_name() { + use crate::dsl::types::Type; + let cache = schema_with_columns( + "Customers", + &[("id", Type::Int), ("Name", Type::Text)], + ); + let input = "insert into Customers values ("; + match ambient_hint(input, input.len(), None, &cache) { + Some(AmbientHint::Prose(p)) => { + assert!(p.contains("id"), "expected column name `id`, got {p:?}"); + assert!( + p.contains("integer"), + "expected int prose, got {p:?}", + ); + } + other => panic!("expected Prose, got {other:?}"), + } + } + + #[test] + fn ambient_hint_at_update_set_mentions_column_name() { + use crate::dsl::types::Type; + let cache = schema_with_columns( + "Customers", + &[("id", Type::Int), ("Email", Type::Text)], + ); + let input = "update Customers set Email="; + match ambient_hint(input, input.len(), None, &cache) { + Some(AmbientHint::Prose(p)) => { + assert!( + p.contains("Email"), + "expected column name `Email`, got {p:?}", + ); + assert!( + p.contains("quoted string"), + "expected text prose, got {p:?}", + ); + } + other => panic!("expected Prose, got {other:?}"), + } + } + + #[test] + fn ambient_hint_at_where_mentions_column_name() { + use crate::dsl::types::Type; + let cache = schema_with_columns("Events", &[("ts", Type::DateTime)]); + let input = "delete from Events where ts="; + match ambient_hint(input, input.len(), None, &cache) { + Some(AmbientHint::Prose(p)) => { + assert!(p.contains("ts"), "expected column name `ts`, got {p:?}"); + assert!( + p.contains("YYYY-MM-DD"), + "expected datetime prose, got {p:?}", + ); + } + other => panic!("expected Prose, got {other:?}"), + } + } + + #[test] + fn ambient_hint_at_second_insert_value_mentions_second_column() { + use crate::dsl::types::Type; + let cache = schema_with_columns( + "Customers", + &[("id", Type::Int), ("Name", Type::Text)], + ); + let input = "insert into Customers values (1, "; + match ambient_hint(input, input.len(), None, &cache) { + Some(AmbientHint::Prose(p)) => { + assert!( + p.contains("Name"), + "expected second column `Name`, got {p:?}", + ); + assert!( + p.contains("quoted string"), + "expected text prose, got {p:?}", + ); + } + other => panic!("expected Prose, got {other:?}"), + } + } + #[test] fn ambient_hint_at_value_slot_falls_back_to_generic_without_schema() { // Empty cache: the walker can't resolve the column type