From a3268495e2ed2baf20b8dfce1e66283d105e4a00 Mon Sep 17 00:00:00 2001 From: "claude@clouddev1" Date: Tue, 19 May 2026 07:35:06 +0000 Subject: [PATCH] ADR-0027: existing-cases sweep + docs (step F) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sweep: input_verdict tests confirm the schema-existence check fires across the identifier-taking commands — unknown table on drop / show / add column, unknown column on drop column / update — and that known references stay clean. The Step B check is grammar-generic, so this is verification + coverage rather than new code. Docs: requirements.md S6 -> [x], baseline 1096; CLAUDE.md deferred list reconciled (C5a and S6 are done — removed); ADR-0026's as-built note updated (step 5 shipped via ADR-0027); ADR-0027 gains an As-built notes section recording the post-walk diagnostics realization, the pre-rendered message, the timeout-based debounce, coarse WARNING spans, and the deferred highlight/hint wiring. --- CLAUDE.md | 9 ---- docs/adr/0026-complex-where-expressions.md | 27 +++++----- docs/adr/0027-input-validity-indicator.md | 60 ++++++++++++++++++++++ docs/requirements.md | 25 +++++---- src/dsl/walker/mod.rs | 57 ++++++++++++++++++++ 5 files changed, 147 insertions(+), 31 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index fadc957..18159fc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -178,15 +178,6 @@ not yet implemented: 1–4 of ADR-0015). Pending pieces: `export` / `import` (Iter 5), `--resume` + persistent input history hydration + migration framework scaffold (Iter 6). -- **Complex WHERE expressions** (C5a): implemented through - ADR-0026 steps 1–4 — the stratified expression grammar - (AND/OR/NOT, the six comparisons, LIKE/IN/BETWEEN/IS NULL, - parentheses) reached through a new `Subgrammar` node, the - recursive `Expr` AST + parameterised SQL, and `where` / - `limit` on `show data`. Type-mismatched comparisons run - permissively (§7). Still pending: the §7 advisory - *flagging* of type mismatches / `= NULL`, the seam with - ADR-0027's diagnostics-severity model. - **SQL handling in advanced mode** (Q1): `sqlparser-rs` parser + a defined SQL subset (Q4 — its own ADR). - **Column drops/renames/type changes** (B2 / C2 partial): the diff --git a/docs/adr/0026-complex-where-expressions.md b/docs/adr/0026-complex-where-expressions.md index 364a048..a3d9232 100644 --- a/docs/adr/0026-complex-where-expressions.md +++ b/docs/adr/0026-complex-where-expressions.md @@ -462,19 +462,20 @@ choices, and where they deviate from the design sketch above: column's type, exactly as the pre-ADR `where col = val` slot did. The operand grammar carries no validators — permissive per §7. -- **Step 5 deferred to ADR-0027.** The §7 *behaviour* - relaxation is done: `bind_where_literal` binds a - type-mismatched WHERE literal by its syntactic shape, and - the pre-ADR bind-time rejection is gone. The §7 - *diagnostic flagging* — surfacing a type-mismatched - comparison or `= NULL` as an error-class highlight + hint - — is the seam with ADR-0027, which designs the walker - diagnostics-severity model that flagging belongs in (and - whose WARNING severity is defined to have "no triggers - until ADR-0026 is implemented"). Building the flagging as - a standalone mechanism first would be reworked by - ADR-0027; the recommendation is to implement it as the - first triggers of ADR-0027's model. +- **Step 5 — done, as part of ADR-0027.** The §7 + *behaviour* relaxation landed with steps 1–4: + `bind_where_literal` binds a type-mismatched WHERE literal + by its syntactic shape, and the pre-ADR bind-time + rejection is gone. The §7 *diagnostic flagging* — a + type-mismatched comparison or `= NULL` as a flagged + finding — was folded into ADR-0027's walker + diagnostics-severity model rather than built as a + standalone mechanism (ADR-0027's WARNING severity is + defined to have "no triggers until ADR-0026 is + implemented" — those triggers are exactly these). It + surfaces as a `Severity::Warning` `Diagnostic`, computed + post-walk from the built `Expr` against the table's + column types — see ADR-0027. ## See also diff --git a/docs/adr/0027-input-validity-indicator.md b/docs/adr/0027-input-validity-indicator.md index ebd528c..9748335 100644 --- a/docs/adr/0027-input-validity-indicator.md +++ b/docs/adr/0027-input-validity-indicator.md @@ -229,6 +229,66 @@ ADR-0026's expression diagnostics (type mismatch, `= NULL`) land with that ADR's implementation and feed the WARNING severity. +## As-built notes (2026-05-19) + +All five build-order steps are implemented, and — because +ADR-0026 deferred its own step 5 — the ADR-0026 §7 +expression diagnostics (type mismatch, `= NULL`) were folded +in here as the WARNING severity's first triggers. Realization +choices and deviations from the sketch above: + +- **Diagnostics are computed post-walk, not emitted during + the walk.** §2 says "diagnostics are emitted there [in the + walker]". As built, the walk produces the `MatchedPath` and + the `Command` as before; `walk()` then runs two post-walk + passes — `schema_existence_diagnostics` over the matched + path, and `expr_warnings` over the built `Expr` — and + stores the result in `WalkResult::diagnostics`. This keeps + diagnostics off the walker's speculative-rollback paths + (a `Choice` branch that matches an ident then rolls back + would otherwise leak a diagnostic); the post-walk passes + see only the surviving terminals. Same single schema-aware + context, one logical pass later. +- **`Diagnostic` carries a pre-rendered `message: String`,** + not the `{severity, span, message_key}` of §2. Each + diagnostic source catalog-translates at creation. This + sidesteps the awkwardness of a parse-style message having + no single static key, and the model stays "roughly + `{severity, span, message}`". +- **The parse outcome is not a synthetic `Diagnostic`.** The + indicator reads it straight from `WalkOutcome` (§2's + "highest severity across the outcome *and* the + diagnostics"); `diagnostics` carries only the schema-aware + findings layered on a structurally-valid parse. Schema- + existence therefore runs only on a `Match`. +- **`MatchedKind::Ident` gained an `IdentSource` field** so + the post-walk schema check can tell a table reference from + a column reference from an invented name without + hard-coding role strings. +- **The debounce uses `tokio::time::timeout` per event-loop + iteration,** not `tokio::select!` — it keeps the existing + ~100-line event-handler body un-reindented. `recv` is + time-boxed only while a recompute is owed; an idle session + still blocks plainly. Any event resets the window (not + only keystrokes) — a slight over-reset, harmless in + practice. +- **WARNING spans are coarse** — the whole WHERE clause. The + `Expr` AST carries no source spans, and the indicator + reads only severity. Precise per-literal spans (for a + future warning-highlight overlay) are a refinement. +- **Highlight / hint integration beyond the indicator** — + §2 notes highlighting and the hint panel "read the + individual diagnostics". The indicator and the model + ship; routing diagnostic spans into the per-byte highlight + overlay, and diagnostic messages into the hint panel + beyond the existing parse-error path, is a follow-up (the + build order did not enumerate it as a step). +- **Debounce timing is not unit-tested** — it is async + event-loop glue. `App::input_validity_verdict` (the pure + verdict) and the indicator rendering are covered; + `input_verdict` has a parse-outcome / schema-existence / + expression-warning / existing-cases-sweep test set. + ## See also - ADR-0003 — input modes; the input field and its mode diff --git a/docs/requirements.md b/docs/requirements.md index 5886228..e63e7bc 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -26,13 +26,14 @@ repo is pushed). ## Test baseline -After ADR-0026 steps 1–4 (complex WHERE expressions): -**1070 passing, 0 failing, 1 ignored** (`cargo test` — the -one ignored test is a long-standing `` ```ignore `` doc-test -in `src/friendly/mod.rs`). Clippy clean with the nursery lint -group enabled. (Earlier reference points: 1039 after ADR-0025 -(indexes); 1006 after ADR-0024 + the handoff-14 cleanup; 449 -after B2/C2.) +After ADR-0027 (input-validity indicator): **1096 passing, +0 failing, 1 ignored** (`cargo test` — the one ignored test +is a long-standing `` ```ignore `` doc-test in +`src/friendly/mod.rs`). Clippy clean with the nursery lint +group enabled. (Earlier reference points: 1079 after ADR-0026 +(complex WHERE expressions); 1039 after ADR-0025 (indexes); +1006 after ADR-0024 + the handoff-14 cleanup; 449 after +B2/C2.) --- @@ -60,12 +61,18 @@ after B2/C2.) for inspecting hints about the current input or last error. - [ ] **S5** Mode label and distinct border style on the input field communicate the current input mode at all times. -- [ ] **S6** Input-field validity indicator: a debounced +- [x] **S6** Input-field validity indicator: a debounced `[ERR]` / `[WRN]` marker at the right edge of the input row, summarising — before submit — whether the current command would run. Backed by a walker diagnostics-severity model (ERROR / WARNING). Advisory only — never blocks submission. - Designed in ADR-0027; implementation pending. + *(ADR-0027: `Severity` / `Diagnostic` on `WalkResult`; + `input_verdict` combines the parse outcome, schema-existence + ERRORs — unknown table / column — and the ADR-0026 §7 + expression WARNINGs — type mismatch, `= NULL`. The runtime + debounces the indicator's display ~1 s; the rightmost six + columns of the input row are reserved unconditionally. New + `warning` theme colour.)* ## Input field diff --git a/src/dsl/walker/mod.rs b/src/dsl/walker/mod.rs index 57d44bd..3d71bb7 100644 --- a/src/dsl/walker/mod.rs +++ b/src/dsl/walker/mod.rs @@ -1679,6 +1679,63 @@ mod tests { ); } + // ---- Existing-cases sweep (ADR-0027 §6) ------------------- + + #[test] + fn input_verdict_sweep_unknown_table_across_commands() { + let schema = schema_with("Customers", &[("id", Type::Int)]); + for input in [ + "drop table NoSuchTable", + "show table NoSuchTable", + "show data NoSuchTable", + "add column to NoSuchTable: x (int)", + ] { + assert_eq!( + super::input_verdict(input, Some(&schema)), + Some(super::Severity::Error), + "unknown table in {input:?} should be flagged", + ); + } + } + + #[test] + fn input_verdict_sweep_unknown_column_across_commands() { + let schema = schema_with( + "Customers", + &[("id", Type::Int), ("Name", Type::Text)], + ); + for input in [ + "drop column from table Customers: NoSuchCol", + "update Customers set NoSuchCol = 1 where id = 1", + ] { + assert_eq!( + super::input_verdict(input, Some(&schema)), + Some(super::Severity::Error), + "unknown column in {input:?} should be flagged", + ); + } + } + + #[test] + fn input_verdict_known_entities_across_commands_are_clean() { + let schema = schema_with( + "Customers", + &[("id", Type::Int), ("Name", Type::Text)], + ); + for input in [ + "show table Customers", + "drop table Customers", + "add column to Customers: Email (text)", + "drop column from table Customers: Name", + ] { + assert_eq!( + super::input_verdict(input, Some(&schema)), + None, + "{input:?} references only known entities — clean", + ); + } + } + #[test] fn walker_parses_insert_with_explicit_column_list() { assert_eq!(