2g: advanced-mode highlight + engine.* wiring + matrix tests

Cross-cut verification matrix for ADR-0032 Phase 2 is now fully
populated with concrete test references — every row green. Filling
the matrix surfaced three real gaps that this commit closes.

1. Advanced-mode syntax highlighting (ADR-0030 §8 matrix row).
   The `ui.rs` Advanced branch routed through `plain_input_spans`,
   bypassing the highlight walker entirely. In production SQL
   keywords past the entry word rendered as plain identifiers.
   Fix: mode-aware variants of `highlight_runs`,
   `render_input_runs`, `lex_to_runs`, and `input_diagnostics`;
   the Advanced render path now uses the highlighted form with
   `Mode::Advanced`. `plain_input_spans` removed (unused).

2. Engine.* key wiring (ADR-0032 §11.4 / §13 matrix rows + handoff
   §3.3 follow-up). The four Phase-2 engine.* catalog entries
   were authored in 2d but never reached: `translate_generic`
   discarded the engine message and returned a vague catalog
   entry. Fix: pattern-match the engine message text for the four
   Phase-2 categories (aggregate misuse, group-by required,
   compound arity mismatch fallback, scalar-subquery cardinality)
   inside `translate_generic`, routing each to its engine-neutral
   catalog entry.

3. Matrix-coverage tests. Thirteen new tests covering the rows
   that had no explicit coverage:
   - 3 SQL keyword/operator/CASE highlight tests
   - 4 engine.* engine-message tests
   - 3 sql_expr column-completion tests (WHERE, HAVING)
   - 3 predicate-warning slot tests (CASE, ORDER BY, projection)
   - 1 all-10-playground-types recovery test (tests/sql_select.rs)

Plan document (docs/plans/20260520-adr-0032-phase-2.md) updated:
every (TBD) row in the cross-cut matrix replaced with a concrete
test file::function reference and a green status marker.

Test totals: 1428 → 1441 passing (+13 new). Clippy clean.
This commit is contained in:
claude@clouddev1
2026-05-20 21:38:08 +00:00
parent ee0dafd86b
commit ed881eea59
8 changed files with 456 additions and 96 deletions
+29
View File
@@ -2138,6 +2138,35 @@ mod tests {
assert!(cs.contains(&"total".to_string()));
}
#[test]
fn sql_expr_column_completion_inside_where() {
// ADR-0031 §5 — column completion works for
// IdentSource::Columns slots inside SQL expressions.
// At `select * from a where i|`, the partial prefix `i`
// walks `where`'s sql_expr, expects an Ident{Columns},
// and offers `id` (a's column starting with `i`).
let cache = two_table_schema();
let input = "select * from a where i";
let cursor = input.len();
let cs = cands_with(input, cursor, &cache);
assert!(
cs.contains(&"id".to_string()),
"expected `id` candidate via sql_expr WHERE column slot; got {cs:?}",
);
}
#[test]
fn sql_expr_column_completion_inside_having() {
let cache = two_table_schema();
let input = "select * from a group by id having n";
let cursor = input.len();
let cs = cands_with(input, cursor, &cache);
assert!(
cs.contains(&"name".to_string()),
"expected `name` candidate via sql_expr HAVING column slot; got {cs:?}",
);
}
#[test]
fn lookahead_with_partial_prefix_filters_correctly() {
// `select na| from a` — narrowing via look-ahead +
+85 -4
View File
@@ -31,13 +31,28 @@ use crate::dsl::walker::outcome::{ByteClass, WalkBound};
/// Produce the per-byte highlight classes for `source`.
///
/// On a successful walk this is exactly the walker's recorded
/// classes. On partial / unmatched input the byte-shape scanner
/// fills the gap so the renderer keeps colouring through trailing
/// tokens and unknown-command inputs.
/// Defaults to `Mode::Simple`. Callers in advanced-mode UIs
/// should use [`highlight_runs_in_mode`] so SQL keywords get
/// matched and highlighted past the entry word (the simple-mode
/// gate at the dispatcher truncates the walker on advanced-only
/// commands, ADR-0030 §2).
#[must_use]
pub fn highlight_runs(source: &str) -> Vec<ByteClass> {
highlight_runs_in_mode(source, crate::mode::Mode::Simple)
}
/// Mode-aware [`highlight_runs`] (ADR-0032 §10.6 follow-up).
///
/// In `Mode::Advanced` the walker matches every Phase-2 SQL
/// token, producing the keyword classes the renderer needs to
/// colour `select` / `from` / `where` / `union` / `case` / etc.
#[must_use]
pub fn highlight_runs_in_mode(
source: &str,
mode: crate::mode::Mode,
) -> Vec<ByteClass> {
let mut ctx = WalkContext::new();
ctx.mode = mode;
let (result, _cmd) = super::walk(source, WalkBound::EndOfInput, &mut ctx);
let mut classes: Vec<ByteClass> = result
.map(|r| r.per_byte_class)
@@ -316,4 +331,70 @@ mod tests {
assert_eq!(runs[0].2, HighlightClass::String);
assert_eq!(runs[0].1, "'café'".len());
}
// ---- ADR-0030 §8 / ADR-0032 — SQL keyword highlighting ----
fn run_advanced(input: &str) -> Vec<(usize, usize, HighlightClass)> {
highlight_runs_in_mode(input, crate::mode::Mode::Advanced)
.into_iter()
.map(|c| (c.start, c.end, c.class))
.collect()
}
#[test]
fn sql_select_keywords_classified() {
// ADR-0030 §8 — `select` / `from` get keyword class in
// Advanced mode (Simple mode gates SELECT out at the
// dispatcher, so only the entry word would highlight).
let runs = run_advanced("select * from t");
assert!(
runs.iter().any(|(s, e, c)| {
*c == HighlightClass::Keyword && (*s, *e) == (0, 6)
}),
"expected `select` keyword span 0..6; got {runs:?}",
);
assert!(
runs.iter().any(|(s, e, c)| {
*c == HighlightClass::Keyword && (*s, *e) == (9, 13)
}),
"expected `from` keyword span 9..13; got {runs:?}",
);
}
#[test]
fn sql_expression_operators_classified_as_keywords() {
// ADR-0031 §5: LIKE / BETWEEN / IN / IS / AND / OR / NOT
// are part of the predicate ladder. Walker matches them
// as Word nodes; highlight class = Keyword.
let input = "select * from t where a like 'x' and b between 1 and 5";
let runs = run_advanced(input);
let keywords: Vec<&str> = runs
.iter()
.filter(|(_, _, c)| *c == HighlightClass::Keyword)
.map(|(s, e, _)| &input[*s..*e])
.collect();
assert!(keywords.contains(&"like"), "no `like`; got {keywords:?}");
assert!(keywords.contains(&"and"), "no `and`; got {keywords:?}");
assert!(
keywords.contains(&"between"),
"no `between`; got {keywords:?}",
);
}
#[test]
fn sql_case_expression_keywords_classified() {
let input = "select case when a = 1 then 'one' else 'other' end from t";
let runs = run_advanced(input);
let keywords: Vec<&str> = runs
.iter()
.filter(|(_, _, c)| *c == HighlightClass::Keyword)
.map(|(s, e, _)| &input[*s..*e])
.collect();
for kw in ["case", "when", "then", "else", "end"] {
assert!(
keywords.contains(&kw),
"missing `{kw}` keyword; got {keywords:?}",
);
}
}
}
+59 -1
View File
@@ -30,7 +30,7 @@ use crate::dsl::walker::outcome::{
};
pub use context::ColumnInfo;
pub use highlight::highlight_runs;
pub use highlight::{highlight_runs, highlight_runs_in_mode};
pub use outcome::{Diagnostic, Severity};
/// Resolve the hint-panel mode at the end of `source`
@@ -436,6 +436,18 @@ pub fn input_verdict_in_mode(
pub fn input_diagnostics(
source: &str,
schema: Option<&crate::completion::SchemaCache>,
) -> Vec<outcome::Diagnostic> {
input_diagnostics_in_mode(source, schema, crate::mode::Mode::Simple)
}
/// Mode-aware [`input_diagnostics`]. Advanced mode lets the
/// Phase-2 SQL-side diagnostics (ADR-0032 §11) emit alongside
/// the existing DSL diagnostics.
#[must_use]
pub fn input_diagnostics_in_mode(
source: &str,
schema: Option<&crate::completion::SchemaCache>,
mode: crate::mode::Mode,
) -> Vec<outcome::Diagnostic> {
if source.trim().is_empty() {
return Vec::new();
@@ -444,6 +456,7 @@ pub fn input_diagnostics(
context::WalkContext::new,
context::WalkContext::with_schema,
);
ctx.mode = mode;
let (result, _cmd) = walk(source, outcome::WalkBound::EndOfInput, &mut ctx);
result.map_or_else(Vec::new, |r| r.diagnostics)
}
@@ -3832,6 +3845,51 @@ mod tests {
);
}
#[test]
fn sql_case_predicate_warning_fires() {
// ADR-0032 §11.6 — predicate warning fires inside
// `CASE WHEN <bare-col> <op> <literal>` shapes too.
let schema = typed_schema();
let diags = diag_keys(
"select case when price like 5 then 1 else 0 end from products",
&schema,
);
assert!(
diags.iter().any(|d| d.contains("LIKE")),
"expected like_numeric warning inside CASE; got {diags:?}",
);
}
#[test]
fn sql_order_by_predicate_warning_fires() {
// Predicate-shape inside ORDER BY's sql_expr — same
// pass, same warning.
let schema = typed_schema();
let diags = diag_keys(
"select * from products order by price like 5",
&schema,
);
assert!(
diags.iter().any(|d| d.contains("LIKE")),
"expected like_numeric warning inside ORDER BY; got {diags:?}",
);
}
#[test]
fn sql_projection_predicate_warning_fires() {
// Predicate shape used as a projection item (returns
// 0/1). Same warning surface.
let schema = typed_schema();
let diags = diag_keys(
"select price like 5 from products",
&schema,
);
assert!(
diags.iter().any(|d| d.contains("LIKE")),
"expected like_numeric warning inside projection; got {diags:?}",
);
}
#[test]
fn sql_join_on_predicate_warning_fires() {
// Phase-1 gap also affects JOIN ON.
+104 -1
View File
@@ -612,9 +612,48 @@ fn translate_already_exists(message: &str, ctx: &TranslateContext) -> FriendlyEr
// ---- Generic catch-all -----------------------------------------
fn translate_generic(_message: &str, ctx: &TranslateContext) -> FriendlyError {
fn translate_generic(message: &str, ctx: &TranslateContext) -> FriendlyError {
// Engine message is intentionally NOT surfaced — ADR-0002
// posture. The catalog provides the abstract wording.
//
// ADR-0032 §11.5 engine-error translations: pattern-match
// the engine's message text for the four Phase-2 cases that
// arrive as `SqliteErrorKind::Other` and route each to its
// engine-neutral catalog entry. The classifier
// intentionally doesn't grow new SqliteErrorKind variants
// for these — they share a single fallback bucket and are
// distinguished by text pattern at translation time.
let lower = message.to_ascii_lowercase();
if lower.contains("misuse of aggregate") {
return headline_only(t!("engine.aggregate_misuse", name = "?"));
}
if lower.contains("group by")
|| lower.contains("must appear in")
{
return headline_only(t!("engine.group_by_required"));
}
if (lower.contains("union")
|| lower.contains("intersect")
|| lower.contains("except"))
&& lower.contains("result columns")
{
// Last-resort safety net — the pre-flight pass in 2d.1
// catches this in most cases; if the engine surfaces it
// anyway, route it through the engine-neutral key.
return headline_only(t!(
"engine.compound_arity_mismatch",
op = "set operator"
));
}
if lower.contains("scalar subquery") || lower.contains("more than one row") {
return headline_only(t!("engine.scalar_subquery_too_many_rows"));
}
if lower.contains("recursive")
&& (lower.contains("cte") || lower.contains("union"))
{
return headline_only(t!("engine.recursive_cte_malformed"));
}
let operation = ctx
.operation
.map_or("operation", Operation::keyword);
@@ -1044,4 +1083,68 @@ mod tests {
assert_eq!(extract_quoted("column `T.x` already exists"), Some("T.x"));
assert_eq!(extract_quoted("no backticks here"), None);
}
// ---- ADR-0032 §11.5 engine.* keys ----
#[test]
fn aggregate_misuse_engine_message_routes_through_catalog() {
let err = sqlite(
"misuse of aggregate function COUNT()",
SqliteErrorKind::Other,
);
let f = translate(&err, &TranslateContext::default());
assert!(
f.headline.contains("aggregate"),
"expected engine.aggregate_misuse wording; got {}",
f.headline,
);
// Engine name (SQLite) must not appear (ADR-0002 posture).
assert!(
!f.headline.to_lowercase().contains("sqlite"),
"headline leaks engine name: {}",
f.headline,
);
}
#[test]
fn group_by_required_engine_message_routes_through_catalog() {
let err = sqlite(
"column must appear in the GROUP BY clause or be used in an aggregate function",
SqliteErrorKind::Other,
);
let f = translate(&err, &TranslateContext::default());
assert!(
f.headline.contains("GROUP BY"),
"expected engine.group_by_required wording; got {}",
f.headline,
);
}
#[test]
fn compound_arity_engine_message_routes_through_catalog() {
let err = sqlite(
"SELECTs to the left and right of UNION do not have the same number of result columns",
SqliteErrorKind::Other,
);
let f = translate(&err, &TranslateContext::default());
assert!(
f.headline.contains("number of columns"),
"expected engine.compound_arity_mismatch wording; got {}",
f.headline,
);
}
#[test]
fn scalar_subquery_too_many_rows_routes_through_catalog() {
let err = sqlite(
"scalar subquery returned more than one row",
SqliteErrorKind::Other,
);
let f = translate(&err, &TranslateContext::default());
assert!(
f.headline.contains("more than one row"),
"expected engine.scalar_subquery_too_many_rows wording; got {}",
f.headline,
);
}
}
+45 -21
View File
@@ -67,28 +67,39 @@ pub fn render_input_runs(
theme: &Theme,
cache: &crate::completion::SchemaCache,
) -> Vec<StyledRun> {
let mut runs = lex_to_runs(input, theme);
// `render_input_runs` is invoked from `ui.rs` only when the
// effective mode is plain `Simple` (advanced rendering uses
// `plain_input_spans`), so the classification must use the
// simple-mode walker view (ADR-0030 §2): a SQL form here
// surfaces as a definite error overlay, consistent with the
// dispatch path's "this is SQL" hint on submit.
render_input_runs_in_mode(input, cursor_byte, theme, cache, Mode::Simple)
}
/// Mode-aware [`render_input_runs`] (ADR-0030 §8).
///
/// Advanced mode runs the highlight walker with `Mode::Advanced`
/// so SQL keywords get matched and coloured, and the
/// definite-error / schema-existence overlays use the
/// advanced-mode parse view.
#[must_use]
pub fn render_input_runs_in_mode(
input: &str,
cursor_byte: usize,
theme: &Theme,
cache: &crate::completion::SchemaCache,
mode: Mode,
) -> Vec<StyledRun> {
let mut runs = lex_to_runs_in_mode(input, theme, mode);
if let InputState::DefiniteErrorAt(pos) =
classify_parse_result(parse_command_with_schema_in_mode(input, cache, Mode::Simple))
classify_parse_result(parse_command_with_schema_in_mode(input, cache, mode))
{
overlay_error(&mut runs, pos, theme);
}
if let Some(inv) = crate::completion::invalid_ident_at_cursor(input, cursor_byte, cache) {
overlay_error(&mut runs, inv.range.0, theme);
}
// Schema-aware diagnostics (ADR-0027 §2): an unknown table
// or column (ERROR), or a dubious comparison (WARNING), is
// overlaid wherever it sits in the input — not only under
// the cursor, so a problem the user has typed past stays
// visible. `input_diagnostics` is empty on a parse failure,
// so this never fights the definite-error overlay above.
for diag in walker::input_diagnostics(input, Some(cache)) {
// Schema-aware diagnostics (ADR-0027 §2): unknown table /
// column (ERROR), or a dubious comparison (WARNING), is
// overlaid wherever it sits — not only under the cursor —
// so a problem the user has typed past stays visible. The
// mode-aware walk picks up the SQL-specific diagnostics from
// ADR-0032 in advanced mode.
for diag in walker::input_diagnostics_in_mode(input, Some(cache), mode) {
let colour = match diag.severity {
walker::Severity::Error => theme.tok_error,
walker::Severity::Warning => theme.warning,
@@ -503,15 +514,28 @@ fn overlay_span(runs: &mut [StyledRun], span: (usize, usize), colour: Color) {
/// no cursor to show.
#[must_use]
pub fn lex_to_runs(input: &str, theme: &Theme) -> Vec<StyledRun> {
base_runs(input, theme)
lex_to_runs_in_mode(input, theme, Mode::Simple)
}
fn base_runs(input: &str, theme: &Theme) -> Vec<StyledRun> {
/// Mode-aware [`lex_to_runs`]. Advanced mode runs the walker
/// with `Mode::Advanced` so SQL keywords past the entry word
/// match and get highlighted (ADR-0030 §8).
#[must_use]
pub fn lex_to_runs_in_mode(
input: &str,
theme: &Theme,
mode: Mode,
) -> Vec<StyledRun> {
base_runs(input, theme, mode)
}
fn base_runs(input: &str, theme: &Theme, mode: Mode) -> Vec<StyledRun> {
// Walker-driven highlighting (ADR-0024 §architecture, Phase F).
// `walker::highlight_runs` returns per-byte classes for every
// token shape in the source; whitespace gaps are not represented
// and we fill them with the default foreground colour below.
let classes = walker::highlight_runs(input);
// `walker::highlight_runs_in_mode` returns per-byte classes for
// every token shape in the source; whitespace gaps are not
// represented and we fill them with the default foreground
// colour below.
let classes = walker::highlight_runs_in_mode(input, mode);
let mut runs = Vec::with_capacity(classes.len() * 2);
let mut pos = 0;
for class in classes {
+15 -40
View File
@@ -655,25 +655,25 @@ fn render_input_panel(app: &App, theme: &Theme, frame: &mut Frame<'_>, area: Rec
// inverted so the cursor is visible without enabling a real
// terminal cursor.
//
// Simple-mode input gets per-token colouring (ADR-0022 §3)
// via input_render::render_input_runs. Advanced-mode input
// — DSL lexer doesn't speak SQL — renders plain (§12), with
// the same before/under/after cursor shape we always had.
// Per-token colouring (ADR-0022 §3 / ADR-0030 §8) in both
// modes — `render_input_runs_in_mode` runs the highlight
// walker with the active mode so SQL keywords / operators /
// CASE / function calls colour correctly in Advanced mode.
let cursor = app.input_cursor.min(app.input.len());
let spans = match effective {
EffectiveMode::Simple => {
let runs = crate::input_render::render_input_runs(
&app.input,
cursor,
theme,
&app.schema_cache,
);
runs_to_spans(&app.input, &runs)
}
let mode_for_render = match effective {
EffectiveMode::Simple => crate::mode::Mode::Simple,
EffectiveMode::AdvancedPersistent | EffectiveMode::AdvancedOneShot => {
plain_input_spans(&app.input, cursor, theme)
crate::mode::Mode::Advanced
}
};
let runs = crate::input_render::render_input_runs_in_mode(
&app.input,
cursor,
theme,
&app.schema_cache,
mode_for_render,
);
let spans = runs_to_spans(&app.input, &runs);
// ADR-0027 §4: the rightmost six columns of the input row
// (a five-column label plus a one-column gap) are reserved
// unconditionally, so the text area is always
@@ -725,31 +725,6 @@ fn runs_to_spans<'a>(
.collect()
}
/// Plain (no token highlighting) input rendering for advanced
/// mode. Same before/under/after cursor shape as the
/// pre-ADR-0022 input panel; here as a deliberate fallback.
fn plain_input_spans<'a>(input: &'a str, cursor: usize, theme: &Theme) -> Vec<Span<'a>> {
let cursor = cursor.min(input.len());
let before = &input[..cursor];
let (under, after) = if cursor < input.len() {
let mut end = cursor + 1;
while end < input.len() && !input.is_char_boundary(end) {
end += 1;
}
(&input[cursor..end], &input[end..])
} else {
(" ", "")
};
vec![
Span::styled(before, Style::default().fg(theme.fg)),
Span::styled(
under,
Style::default().fg(theme.fg).add_modifier(Modifier::REVERSED),
),
Span::styled(after, Style::default().fg(theme.fg)),
]
}
fn render_hint_panel(app: &App, theme: &Theme, frame: &mut Frame<'_>, area: Rect) {
let block = Block::default()
.borders(Borders::ALL)