From 1c8cbc19839509b4584c054d4323cc2fa68a7df6 Mon Sep 17 00:00:00 2001 From: "claude@clouddev1" Date: Thu, 21 May 2026 20:25:16 +0000 Subject: [PATCH] completion+hint: F1/F2 advanced-mode completion fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F1: the hint panel is the completion UI, so a premature "no such table/ column" ERROR on the token the user is still typing must not shadow its completion. ambient_hint now suppresses an under-cursor error diagnostic when a completion exists for the (non-empty) partial it overlaps, and falls through to the candidates. Genuinely-unknown names (no prefix match) still show the error; WARNINGs are unaffected. Both modes. F2: projection-before-FROM ("select 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. When the look-ahead finds no scope, retry with a neutral 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 used. Test-first: 3 F1 tests (mid-typed completes, unknown still errors, simple- mode DSL) + 1 F2 multi-table narrowing test. 1469 baseline green. --- src/completion.rs | 28 ++++++++- src/input_render.rs | 138 +++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 157 insertions(+), 9 deletions(-) diff --git a/src/completion.rs b/src/completion.rs index e821432..d504871 100644 --- a/src/completion.rs +++ b/src/completion.rs @@ -279,7 +279,33 @@ pub fn candidates_at_cursor_with_in_mode( && probe.cte_bindings.is_empty() && input.len() > leading.len() { - Some(crate::dsl::walker::completion_probe_in_mode(input, cache, mode)) + let direct = crate::dsl::walker::completion_probe_in_mode(input, cache, mode); + if direct.from_scope.is_empty() && direct.cte_bindings.is_empty() { + // The slot at the cursor is empty/incomplete — e.g. the + // projection list of `select from T` after the + // user deleted `*` — so the full-input walk never + // reached FROM and recovered no scope. Repair by + // inserting a neutral expression placeholder at the + // cursor and re-walking, so the trailing FROM/CTE scope + // is recovered for column narrowing (ADR-0032 §10.6). + // Only the repaired walk's `from_scope` / `cte_bindings` + // are consumed (table + columns), so the inserted token + // doesn't perturb the expected set, which comes from the + // leading probe. + let mut repaired = String::with_capacity(input.len() + 2); + repaired.push_str(&input[..start]); + repaired.push_str("1 "); + repaired.push_str(&input[start..]); + let repaired_probe = + crate::dsl::walker::completion_probe_in_mode(&repaired, cache, mode); + if repaired_probe.from_scope.is_empty() && repaired_probe.cte_bindings.is_empty() { + Some(direct) + } else { + Some(repaired_probe) + } + } else { + Some(direct) + } } else { None }; diff --git a/src/input_render.rs b/src/input_render.rs index a490751..62a436c 100644 --- a/src/input_render.rs +++ b/src/input_render.rs @@ -265,6 +265,15 @@ pub fn ambient_hint_in_mode( selected: Some(m.selection_idx), }); } + // Completion candidates at the cursor, computed once: both the + // diagnostic-shadow check below and the candidate ladder + // further down consume them. `candidates_at_cursor_in_mode` + // narrows column candidates to the active table and runs the + // §10.6 look-ahead, so it is the authoritative "what can go + // here" set. + let completion = + crate::completion::candidates_at_cursor_in_mode(input, cursor, cache, mode); + // Schema-aware diagnostics (ADR-0027 §2). `input_diagnostics` // is non-empty only for a command that *structurally parses* // — so a non-empty result means "this command is complete @@ -276,9 +285,26 @@ pub fn ambient_hint_in_mode( // (the panel explains where the user is looking), else the // most severe one. The error overlay still marks every // flagged token; this panel carries the *why*. + // + // F1 (ADR-0022): the hint panel *is* the completion UI, so a + // premature "unknown table/column" ERROR on the very token the + // user is still typing must not shadow its completion. When an + // under-cursor ERROR overlaps the (non-empty) partial a + // candidate would replace, prefer the candidates. let diagnostics = crate::dsl::walker::input_diagnostics_in_mode(input, Some(cache), mode); if let Some(diag) = pick_hint_diagnostic(&diagnostics, cursor.min(input.len())) { - return Some(AmbientHint::Prose(diag.message.clone())); + let typing_over_diag = diag.severity == crate::dsl::walker::Severity::Error + && completion.as_ref().is_some_and(|c| { + let (replace_start, replace_end) = c.replaced_range; + let (diag_start, diag_end) = diag.span; + replace_end > replace_start + && !c.candidates.is_empty() + && diag_start < replace_end + && replace_start < diag_end + }); + if !typing_over_diag { + return Some(AmbientHint::Prose(diag.message.clone())); + } } // Resolve the walker-side `HintMode` at the cursor. This // detects value-literal-slot and NewName-slot positions @@ -362,13 +388,11 @@ pub fn ambient_hint_in_mode( // Candidates win when any exist — the panel surfaces them // directly because they're more actionable than prose // framings. - // Candidate completion runs through the `mode`-aware walker - // view (ADR-0022 Amendment 1): in advanced mode SQL keywords - // and schema candidates surface; in simple mode `select` is - // gated as "this is SQL" (ADR-0030 §2). - if let Some(comp) = - crate::completion::candidates_at_cursor_in_mode(input, cursor, cache, mode) - { + // Candidate completion (computed once above) runs through the + // `mode`-aware walker view (ADR-0022 Amendment 1): in advanced + // mode SQL keywords and schema candidates surface; in simple + // mode `select` is gated as "this is SQL" (ADR-0030 §2). + if let Some(comp) = completion { return Some(AmbientHint::Candidates { items: comp.candidates, selected: None, @@ -821,6 +845,104 @@ mod tests { ); } + // ---- F1: mid-typed token completes, not flagged (both modes) ---- + + #[test] + fn f1_mid_typed_table_prefix_shows_completion_not_error() { + // "select * from c" — `c` prefix-matches `Customers`. The + // hint must offer the completion, not "no such table c". + let cache = + schema_with_columns("Customers", &[("id", crate::dsl::types::Type::Int)]); + match ambient_hint_in_mode( + "select * from c", + "select * from c".len(), + None, + &cache, + crate::mode::Mode::Advanced, + ) { + Some(AmbientHint::Candidates { items, .. }) => assert!( + items.iter().any(|c| c.text == "Customers"), + "expected Customers completion, got {items:?}", + ), + other => panic!("F1: expected completion candidates, got {other:?}"), + } + } + + #[test] + fn f1_genuinely_unknown_table_still_shows_error() { + // "zzz" matches no table prefix — the error must still show. + let cache = + schema_with_columns("Customers", &[("id", crate::dsl::types::Type::Int)]); + match ambient_hint_in_mode( + "select * from zzz", + "select * from zzz".len(), + None, + &cache, + crate::mode::Mode::Advanced, + ) { + Some(AmbientHint::Prose(s)) => { + assert!(s.contains("zzz"), "expected unknown-table error, got {s:?}"); + } + other => panic!("F1: expected unknown-table error prose, got {other:?}"), + } + } + + #[test] + fn f1_simple_mode_dsl_mid_typed_table_completes() { + // The same shadowing affects DSL commands in simple mode: + // "show data c" must offer Customers, not "no such table c". + let cache = + schema_with_columns("Customers", &[("id", crate::dsl::types::Type::Int)]); + match ambient_hint_in_mode( + "show data c", + "show data c".len(), + None, + &cache, + crate::mode::Mode::Simple, + ) { + Some(AmbientHint::Candidates { items, .. }) => assert!( + items.iter().any(|c| c.text == "Customers"), + "expected Customers completion, got {items:?}", + ), + other => panic!("F1 (simple): expected completion candidates, got {other:?}"), + } + } + + // ---- F2: projection-before-FROM narrows to the FROM table ---- + + #[test] + fn f2_empty_projection_narrows_to_from_table() { + use crate::completion::TableColumn; + use crate::dsl::types::Type; + // Two tables; cursor in the EMPTY projection of + // "select from Orders" must offer Orders' column, NOT + // Customers' column. + let mut cache = schema_with_columns("Customers", &[("cust_col", Type::Text)]); + cache.tables.push("Orders".to_string()); + cache.columns.push("order_col".to_string()); + cache.table_columns.insert( + "Orders".to_string(), + vec![TableColumn { name: "order_col".to_string(), user_type: Type::Int }], + ); + + let comp = crate::completion::candidates_at_cursor_in_mode( + "select from Orders", + 7, + &cache, + crate::mode::Mode::Advanced, + ) + .expect("candidates at projection cursor"); + let texts: Vec = comp.candidates.iter().map(|c| c.text.clone()).collect(); + assert!( + texts.iter().any(|t| t == "order_col"), + "F2: should offer the FROM table's column; got {texts:?}", + ); + assert!( + !texts.iter().any(|t| t == "cust_col"), + "F2: must NOT offer the other table's column; got {texts:?}", + ); + } + // ---- Phase D typed-slot hints (end-to-end) ------- fn schema_with_columns(