From 400fb71460beb7af98f31ef5f1a4fbbe752ce1f2 Mon Sep 17 00:00:00 2001 From: "claude@clouddev1" Date: Tue, 19 May 2026 09:39:58 +0000 Subject: [PATCH] =?UTF-8?q?ui:=20surface=20diagnostics=20in=20the=20ambien?= =?UTF-8?q?t=20hint=20panel=20(ADR-0027=20=C2=A72)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ambient_hint now reads the walker's schema-aware diagnostics. input_diagnostics is non-empty only for a command that structurally parses — so a non-empty result means "complete and submittable, but wrong or dubious". That is checked early (right after the Tab-cycle memo), ahead of slot hints and completions: a command that parses but is flawed no longer gets the misleading "Submit with Enter" prose, it gets the diagnostic's why. pick_hint_diagnostic prefers the diagnostic under the cursor, else the most severe. The cursor-local invalid-ident hint is kept for genuinely incomplete commands (no Match → no diagnostics). 5 ambient_hint tests (unknown table, type-mismatch over submit-prose, LIKE-numeric, clean command still submittable, cursor-following). The complex_and_or matrix cell referenced a non-existent column `t`; fixed to a real column so it tests a valid expression as intended. 1118 passing, clippy clean. --- src/input_render.rs | 121 +++++++++++++++++- ...d_or_expression_parses@complex_and_or.snap | 6 +- tests/typing_surface/where_expression.rs | 6 +- 3 files changed, 126 insertions(+), 7 deletions(-) diff --git a/src/input_render.rs b/src/input_render.rs index 5f4c438..b7f8bb0 100644 --- a/src/input_render.rs +++ b/src/input_render.rs @@ -223,6 +223,21 @@ pub fn ambient_hint( selected: Some(m.selection_idx), }); } + // 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 + // and submittable, but wrong or dubious". That is the single + // most important thing to tell the user, ahead of slot hints + // and completions, so it is checked early — right after the + // Tab-cycle memo. An unknown name is an ERROR, a dubious + // comparison a WARNING; the diagnostic under the cursor wins + // (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*. + let diagnostics = crate::dsl::walker::input_diagnostics(input, Some(cache)); + if let Some(diag) = pick_hint_diagnostic(&diagnostics, cursor.min(input.len())) { + return Some(AmbientHint::Prose(diag.message.clone())); + } // Resolve the walker-side `HintMode` at the cursor. This // detects value-literal-slot and NewName-slot positions // declaratively (ADR-0024 §HintMode-per-node) — the @@ -418,6 +433,26 @@ fn oxford_or(items: &[String]) -> String { } } +/// Choose which diagnostic the hint panel surfaces: the one +/// whose span contains the cursor (the panel explains where the +/// user is looking), else the most severe — ERROR over WARNING, +/// ties broken leftmost. +fn pick_hint_diagnostic( + diagnostics: &[crate::dsl::walker::Diagnostic], + cursor: usize, +) -> Option<&crate::dsl::walker::Diagnostic> { + diagnostics + .iter() + .find(|d| d.span.0 <= cursor && cursor <= d.span.1) + .or_else(|| { + diagnostics.iter().max_by(|a, b| { + a.severity + .cmp(&b.severity) + .then_with(|| b.span.0.cmp(&a.span.0)) + }) + }) +} + fn overlay_error(runs: &mut [StyledRun], error_byte: usize, theme: &Theme) { // Failing tokens have their byte_range starting exactly at // `error_byte`. Override the fg colour while preserving any @@ -958,7 +993,9 @@ mod tests { #[test] fn ambient_hint_for_invalid_identifier_says_no_such() { use crate::completion::SchemaCache; - // Schema knows "Customers"; user typed "Custp" — no match. + // `show data Custp` is a complete command naming a table + // that does not exist — surfaced by the ADR-0027 + // diagnostic branch (the schema-existence ERROR). let cache = SchemaCache { tables: vec!["Customers".to_string()], ..SchemaCache::default() @@ -966,8 +1003,8 @@ mod tests { match ambient_hint("show data Custp", 15, None, &cache) { Some(AmbientHint::Prose(p)) => { assert!( - p.contains("No such table"), - "expected 'No such table' wording, got {p:?}", + p.to_lowercase().contains("no such table"), + "expected 'no such table' wording, got {p:?}", ); assert!(p.contains("Custp"), "should name the bad ident, got {p:?}"); } @@ -975,6 +1012,84 @@ mod tests { } } + // ---- diagnostic hints (ADR-0027 hint wiring) ---- + + #[test] + fn ambient_hint_surfaces_unknown_table_diagnostic() { + use crate::dsl::types::Type; + let cache = schema_with_columns("Customers", &[("id", Type::Int)]); + match ambient_hint("show data Missing", 17, None, &cache) { + Some(AmbientHint::Prose(p)) => { + assert!(p.contains("Missing"), "got {p:?}"); + assert!( + p.to_lowercase().contains("no such table"), + "got {p:?}", + ); + } + other => panic!("expected Prose, got {other:?}"), + } + } + + #[test] + fn ambient_hint_surfaces_type_mismatch_over_submit_prose() { + use crate::dsl::types::Type; + // The command parses cleanly — without the diagnostic + // branch this shows the misleading "press Enter" prose. + let cache = schema_with_columns("Events", &[("Count", Type::Int)]); + let input = "delete from Events where Count = 'oops'"; + match ambient_hint(input, input.len(), None, &cache) { + Some(AmbientHint::Prose(p)) => { + assert!(!p.contains("Enter"), "should not invite submit: {p:?}"); + assert!(p.contains("Count"), "should name the column: {p:?}"); + } + other => panic!("expected Prose, got {other:?}"), + } + } + + #[test] + fn ambient_hint_surfaces_like_numeric_warning() { + use crate::dsl::types::Type; + let cache = schema_with_columns("Events", &[("Count", Type::Int)]); + let input = "delete from Events where Count like '9%'"; + match ambient_hint(input, input.len(), None, &cache) { + Some(AmbientHint::Prose(p)) => { + assert!(p.contains("LIKE"), "should mention LIKE: {p:?}"); + } + other => panic!("expected Prose, got {other:?}"), + } + } + + #[test] + fn ambient_hint_clean_command_still_invites_submit() { + use crate::dsl::types::Type; + let cache = schema_with_columns("Events", &[("Count", Type::Int)]); + let input = "delete from Events where Count = 7"; + match ambient_hint(input, input.len(), None, &cache) { + Some(AmbientHint::Prose(p)) => { + assert!(p.contains("Enter"), "clean command invites submit: {p:?}"); + } + other => panic!("expected Prose, got {other:?}"), + } + } + + #[test] + fn ambient_hint_diagnostic_follows_the_cursor() { + use crate::dsl::types::Type; + // Two type-mismatch WARNINGs; the hint names the column + // whose offending literal the cursor sits in. + let cache = + schema_with_columns("Events", &[("a", Type::Int), ("b", Type::Int)]); + let input = "delete from Events where a = 'x' or b = 'y'"; + let on_x = input.find("'x'").expect("'x' literal") + 1; + let on_y = input.find("'y'").expect("'y' literal") + 1; + let prose_at = |cursor| match ambient_hint(input, cursor, None, &cache) { + Some(AmbientHint::Prose(p)) => p, + other => panic!("expected Prose, got {other:?}"), + }; + assert!(prose_at(on_x).contains("`a`"), "cursor on 'x' → column a"); + assert!(prose_at(on_y).contains("`b`"), "cursor on 'y' → column b"); + } + #[test] fn ambient_hint_with_memo_carries_selected_index() { use crate::completion::{Candidate, CandidateKind, LastCompletion}; diff --git a/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__where_expression__complex_and_or_expression_parses@complex_and_or.snap b/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__where_expression__complex_and_or_expression_parses@complex_and_or.snap index 3b019d0..d7fb269 100644 --- a/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__where_expression__complex_and_or_expression_parses@complex_and_or.snap +++ b/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__where_expression__complex_and_or_expression_parses@complex_and_or.snap @@ -1,11 +1,11 @@ --- source: tests/typing_surface/where_expression.rs -description: "input=\"delete from Things where k > 1 and t like 'a%' or k = 9\" cursor=55" +description: "input=\"delete from Things where k > 1 and note like 'a%' or k = 9\" cursor=58" expression: "& a" --- Assessment { - input: "delete from Things where k > 1 and t like 'a%' or k = 9", - cursor: 55, + input: "delete from Things where k > 1 and note like 'a%' or k = 9", + cursor: 58, state: Valid, hint: Some( Prose( diff --git a/tests/typing_surface/where_expression.rs b/tests/typing_surface/where_expression.rs index 7bd9130..4725c92 100644 --- a/tests/typing_surface/where_expression.rs +++ b/tests/typing_surface/where_expression.rs @@ -61,8 +61,12 @@ fn in_list_open_paren_expects_an_item() { #[test] fn complex_and_or_expression_parses() { let schema = schema_every_type(); + // `note` is the text column — `t` is not a column of + // `Things`, and the schema-existence diagnostic (ADR-0027) + // would now correctly flag it; this cell tests a *valid* + // complex expression, so it uses a real column. let a = assess_at_end( - "delete from Things where k > 1 and t like 'a%' or k = 9", + "delete from Things where k > 1 and note like 'a%' or k = 9", &schema, ); assert!(matches!(a.state, InputState::Valid));