ADR-0027: existing-cases sweep + docs (step F)
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.
This commit is contained in:
@@ -178,15 +178,6 @@ not yet implemented:
|
|||||||
1–4 of ADR-0015). Pending pieces: `export` / `import` (Iter
|
1–4 of ADR-0015). Pending pieces: `export` / `import` (Iter
|
||||||
5), `--resume` + persistent input history hydration +
|
5), `--resume` + persistent input history hydration +
|
||||||
migration framework scaffold (Iter 6).
|
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
|
- **SQL handling in advanced mode** (Q1): `sqlparser-rs` parser
|
||||||
+ a defined SQL subset (Q4 — its own ADR).
|
+ a defined SQL subset (Q4 — its own ADR).
|
||||||
- **Column drops/renames/type changes** (B2 / C2 partial): the
|
- **Column drops/renames/type changes** (B2 / C2 partial): the
|
||||||
|
|||||||
@@ -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`
|
column's type, exactly as the pre-ADR `where col = val`
|
||||||
slot did. The operand grammar carries no validators —
|
slot did. The operand grammar carries no validators —
|
||||||
permissive per §7.
|
permissive per §7.
|
||||||
- **Step 5 deferred to ADR-0027.** The §7 *behaviour*
|
- **Step 5 — done, as part of ADR-0027.** The §7
|
||||||
relaxation is done: `bind_where_literal` binds a
|
*behaviour* relaxation landed with steps 1–4:
|
||||||
type-mismatched WHERE literal by its syntactic shape, and
|
`bind_where_literal` binds a type-mismatched WHERE literal
|
||||||
the pre-ADR bind-time rejection is gone. The §7
|
by its syntactic shape, and the pre-ADR bind-time
|
||||||
*diagnostic flagging* — surfacing a type-mismatched
|
rejection is gone. The §7 *diagnostic flagging* — a
|
||||||
comparison or `= NULL` as an error-class highlight + hint
|
type-mismatched comparison or `= NULL` as a flagged
|
||||||
— is the seam with ADR-0027, which designs the walker
|
finding — was folded into ADR-0027's walker
|
||||||
diagnostics-severity model that flagging belongs in (and
|
diagnostics-severity model rather than built as a
|
||||||
whose WARNING severity is defined to have "no triggers
|
standalone mechanism (ADR-0027's WARNING severity is
|
||||||
until ADR-0026 is implemented"). Building the flagging as
|
defined to have "no triggers until ADR-0026 is
|
||||||
a standalone mechanism first would be reworked by
|
implemented" — those triggers are exactly these). It
|
||||||
ADR-0027; the recommendation is to implement it as the
|
surfaces as a `Severity::Warning` `Diagnostic`, computed
|
||||||
first triggers of ADR-0027's model.
|
post-walk from the built `Expr` against the table's
|
||||||
|
column types — see ADR-0027.
|
||||||
|
|
||||||
## See also
|
## See also
|
||||||
|
|
||||||
|
|||||||
@@ -229,6 +229,66 @@ ADR-0026's expression diagnostics (type mismatch, `= NULL`)
|
|||||||
land with that ADR's implementation and feed the WARNING
|
land with that ADR's implementation and feed the WARNING
|
||||||
severity.
|
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
|
## See also
|
||||||
|
|
||||||
- ADR-0003 — input modes; the input field and its mode
|
- ADR-0003 — input modes; the input field and its mode
|
||||||
|
|||||||
+16
-9
@@ -26,13 +26,14 @@ repo is pushed).
|
|||||||
|
|
||||||
## Test baseline
|
## Test baseline
|
||||||
|
|
||||||
After ADR-0026 steps 1–4 (complex WHERE expressions):
|
After ADR-0027 (input-validity indicator): **1096 passing,
|
||||||
**1070 passing, 0 failing, 1 ignored** (`cargo test` — the
|
0 failing, 1 ignored** (`cargo test` — the one ignored test
|
||||||
one ignored test is a long-standing `` ```ignore `` doc-test
|
is a long-standing `` ```ignore `` doc-test in
|
||||||
in `src/friendly/mod.rs`). Clippy clean with the nursery lint
|
`src/friendly/mod.rs`). Clippy clean with the nursery lint
|
||||||
group enabled. (Earlier reference points: 1039 after ADR-0025
|
group enabled. (Earlier reference points: 1079 after ADR-0026
|
||||||
(indexes); 1006 after ADR-0024 + the handoff-14 cleanup; 449
|
(complex WHERE expressions); 1039 after ADR-0025 (indexes);
|
||||||
after B2/C2.)
|
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.
|
for inspecting hints about the current input or last error.
|
||||||
- [ ] **S5** Mode label and distinct border style on the input
|
- [ ] **S5** Mode label and distinct border style on the input
|
||||||
field communicate the current input mode at all times.
|
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,
|
`[ERR]` / `[WRN]` marker at the right edge of the input row,
|
||||||
summarising — before submit — whether the current command
|
summarising — before submit — whether the current command
|
||||||
would run. Backed by a walker diagnostics-severity model
|
would run. Backed by a walker diagnostics-severity model
|
||||||
(ERROR / WARNING). Advisory only — never blocks submission.
|
(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
|
## Input field
|
||||||
|
|
||||||
|
|||||||
@@ -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]
|
#[test]
|
||||||
fn walker_parses_insert_with_explicit_column_list() {
|
fn walker_parses_insert_with_explicit_column_list() {
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
|
|||||||
Reference in New Issue
Block a user