437b2f2e91
LIKE is a text-pattern match; against a numeric column (int, real, decimal, serial) it runs but is almost never intended. predicate_warnings now emits a WARNING for it, spanned at the target column. New Type::is_numeric; catalog key diagnostic.like_numeric; ADR-0027 gains "Amendment 1" and the adr/README index line is updated per the index-upkeep rule. bool and the text-/blob-backed types are deliberately not flagged — see the amendment for the rationale. 3 walker tests (int, decimal NOT LIKE, text-column clean). 1108 passing, clippy clean.
341 lines
14 KiB
Markdown
341 lines
14 KiB
Markdown
# ADR-0027: Input-field validity indicator
|
||
|
||
## Status
|
||
|
||
Accepted
|
||
|
||
## Context
|
||
|
||
While the user types, the app already flags problems in
|
||
two immediate ways (ADR-0022): per-token syntax
|
||
highlighting, and an ambient hint panel describing the slot
|
||
under the cursor. Both are *local* — they speak about the
|
||
spot being edited.
|
||
|
||
What is missing is a *global* signal: a single, glanceable
|
||
answer to "if I press Enter now, will this command run?" A
|
||
learner can type something whose error is highlighted ten
|
||
columns back, move the cursor to the end, and submit it
|
||
without noticing.
|
||
|
||
This ADR adds a **validity indicator** at the right edge of
|
||
the input row, and — because a meaningful indicator needs a
|
||
consistent notion of "what is wrong" — a small
|
||
**diagnostics-severity model** behind it.
|
||
|
||
The indicator is **advisory**. It never blocks submission:
|
||
that posture is settled (ADR-0022 — the app indicates, it
|
||
does not refuse; ADR-0026 §7 — even a flagged WHERE still
|
||
runs if submitted).
|
||
|
||
## Decision
|
||
|
||
### 1. Two severities
|
||
|
||
- **ERROR** — the input is *known* to fail. Either it does
|
||
not parse (incomplete, or a mismatched / invalid token),
|
||
or it parses but names something that does not exist (an
|
||
unknown table or column).
|
||
- **WARNING** — the input is valid and *will* run, but is
|
||
very likely not what a knowledgeable user wants: a
|
||
type-mismatched comparison, or `= NULL` (both from
|
||
ADR-0026 §7). Amendment 1 adds a third trigger — `LIKE`
|
||
against a numeric column.
|
||
|
||
The split is *certainty of failure* versus *likely
|
||
misleading*. The indicator shows the highest severity
|
||
present; clean input shows nothing at all.
|
||
|
||
### 2. The diagnostics model
|
||
|
||
The walk produces a set of **diagnostics** alongside its
|
||
`WalkOutcome`. A diagnostic is, roughly,
|
||
`{ severity, span, message_key }` — the severity drives the
|
||
indicator, the span drives highlighting, the message key
|
||
drives the hint text (ADR-0019 catalog).
|
||
|
||
Diagnostics come from three sources, all reachable within
|
||
the single schema-aware walk:
|
||
|
||
- **The parse outcome.** `Incomplete`, `Mismatch`, and
|
||
`ValidationFailed` each yield an ERROR diagnostic; a
|
||
plain `Match` yields none.
|
||
- **Schema existence.** A matched `IdentSource::Tables` or
|
||
`IdentSource::Columns` token whose name is absent from
|
||
the schema cache yields an ERROR diagnostic. *This is new
|
||
behaviour.* Today `walk_ident` returns `Matched` for any
|
||
identifier-shaped token and consults the schema only to
|
||
populate context (`current_table`, `current_column`); an
|
||
unknown table parses cleanly and fails only at execution.
|
||
The check is cheap — the schema cache is already in
|
||
`WalkContext` — but it is genuinely new code, and it must
|
||
run for every Tables / Columns ident, not only the
|
||
`writes_*` ones.
|
||
- **Expression flagging (ADR-0026).** A type-mismatched
|
||
comparison and `= NULL` yield WARNING diagnostics.
|
||
|
||
The walker is the one schema-aware pass, so diagnostics are
|
||
emitted there, not in a separate re-resolution pass.
|
||
`WalkResult` gains a `diagnostics` field. The indicator
|
||
reads the highest severity across the outcome and the
|
||
diagnostics; highlighting and the hint panel read the
|
||
individual diagnostics for *where* and *why*. The indicator
|
||
is the summary; the existing layers remain the detail.
|
||
|
||
### 3. "Would Enter succeed now?" — and the debounce
|
||
|
||
The indicator answers exactly one question: *if you pressed
|
||
Enter right now, what would happen?* — runs clean (nothing
|
||
shown), runs but flagged (`[WRN]`), or will not run
|
||
(`[ERR]`).
|
||
|
||
An incomplete, still-being-typed command is in the `[ERR]`
|
||
bucket: pressing Enter on it fails. The indicator does not
|
||
special-case "still typing".
|
||
|
||
Flicker is prevented not by suppressing states but by a
|
||
**debounce**:
|
||
|
||
- On every keystroke the indicator is hidden immediately.
|
||
- After roughly one second with no typing, it reappears,
|
||
showing the current verdict.
|
||
|
||
So: typing → blank; a pause → the verdict. The user gets a
|
||
settled, glance-before-submit signal without a marker that
|
||
thrashes on every key. Highlighting and hints stay
|
||
immediate and unchanged — only the *indicator's display* is
|
||
debounced; diagnostics themselves are still computed every
|
||
keystroke to feed those immediate layers.
|
||
|
||
The indicator's display is therefore time-gated. Per the
|
||
`update()`-is-pure invariant, the debounce timer lives in
|
||
the runtime / event loop; `App` holds the indicator's
|
||
visible state, which the runtime sets when the quiet
|
||
interval elapses or a keystroke arrives. The interval is a
|
||
single tunable constant (~1 s).
|
||
|
||
### 4. Rendering
|
||
|
||
The indicator is a fixed-width five-column label —
|
||
`[ERR]` or `[WRN]` — or nothing when the input is clean. A
|
||
positive "all good" state is deliberately omitted: absence
|
||
*is* the all-clear.
|
||
|
||
It sits in the input row, at the right end. The rightmost
|
||
strip — five columns for the label plus one column of gap,
|
||
six in all — is **reserved unconditionally**: the text area
|
||
is always `width − 6`, whether or not the indicator is
|
||
currently visible. The label appears and disappears within
|
||
its already-reserved strip, so the text / horizontal-scroll
|
||
boundary never moves and the label never collides with the
|
||
typed command.
|
||
|
||
A *conditionally* reserved strip was rejected: it would
|
||
make a long, horizontally-scrolled command jump up to six
|
||
columns sideways on every typing-pause transition.
|
||
Unconditional reservation costs ~6 columns of typing width
|
||
— negligible — for a stable layout.
|
||
|
||
Border-mounting the indicator (as chrome on the field's
|
||
frame, like the mode label) was also considered and not
|
||
chosen: a right-edge border decoration reads differently
|
||
from a top-border label, and the in-row position is the
|
||
intended look.
|
||
|
||
`[ERR]` uses the existing `theme.error`. `[WRN]` needs a
|
||
new **`warning`** theme colour — an orange / amber —
|
||
defined for both the light and dark themes (NFR-5: colour
|
||
conveys information; NFR-7: legible on either background).
|
||
|
||
### 5. The indicator never blocks submission
|
||
|
||
Enter always submits, whatever the indicator shows. A user
|
||
who wants to run a flagged command — or even a doomed one,
|
||
to see what the engine does — may; the error lands in the
|
||
output as usual. The indicator's job is to make "this will
|
||
not do what you think" visible *before* submission, not to
|
||
prevent it. This is the same advisory posture as ADR-0022
|
||
and ADR-0026 §7.
|
||
|
||
### 6. The existing-cases sweep, and discoverability
|
||
|
||
Implementation includes a **sweep** of the current command
|
||
surface for failures that are detectable before submission
|
||
but not yet surfaced as diagnostics — chiefly unknown table
|
||
and column references, across every command that takes an
|
||
identifier. Each becomes an ERROR diagnostic via §2. (The
|
||
WARNING set is thin until ADR-0026 lands; type mismatch and
|
||
`= NULL` are its first members.)
|
||
|
||
The diagnostics model is documented as *the* route for any
|
||
future "we can tell, before it runs, that this is wrong or
|
||
dubious" case. This ADR is cross-referenced from the
|
||
diagnostic-producing code and from related ADRs, so new
|
||
cases plug into the model rather than becoming one-off
|
||
checks.
|
||
|
||
### 7. Out of scope
|
||
|
||
- **Blocking submission** — never; the indicator is
|
||
advisory (§5).
|
||
- **A positive `[OK]` state** — clean input shows nothing.
|
||
- **Raw advanced-mode SQL** — there is no SQL parser yet
|
||
(`Q1`). The indicator covers simple-mode DSL and the
|
||
app-level commands the walker parses; when SQL parsing
|
||
lands, SQL diagnostics route through this same model.
|
||
- **Per-diagnostic display in the indicator itself** — the
|
||
indicator is a one-glyph summary; *where* and *why* stay
|
||
with highlighting and the hint panel.
|
||
|
||
## Consequences
|
||
|
||
- `WalkResult` gains a `diagnostics` field; the walker
|
||
emits diagnostics (parse-outcome and schema-existence) as
|
||
it walks.
|
||
- A new schema-existence check on `IdentSource::Tables` /
|
||
`Columns` matches. Small, but genuinely new — today an
|
||
unknown identifier parses cleanly and fails only at
|
||
execution.
|
||
- The event loop gains a debounce timer for the indicator's
|
||
display. It lives in the runtime, so `update()` stays
|
||
pure.
|
||
- A new `warning` theme colour; the input field reserves a
|
||
fixed six-column right strip.
|
||
- The indicator is advisory; submission is never gated.
|
||
- A reusable diagnostics-severity model that future
|
||
pre-submit checks — and, eventually, advanced-mode SQL —
|
||
extend.
|
||
- The WARNING severity has no triggers until ADR-0026 is
|
||
implemented. The indicator may ship ERROR-only first and
|
||
gain WARNING with C5a, or ship after C5a; the model
|
||
carries both severities regardless.
|
||
|
||
## Implementation notes
|
||
|
||
A sensible order, each step test-guarded:
|
||
|
||
1. The `Diagnostic` type and the `diagnostics` field on
|
||
`WalkResult`; map the parse outcome to diagnostics.
|
||
2. The schema-existence check in `walk_ident` for
|
||
`Tables` / `Columns` idents.
|
||
3. The `warning` theme colour; the fixed six-column input
|
||
strip; the `[ERR]` / `[WRN]` rendering.
|
||
4. The debounce in the runtime, and the indicator's
|
||
visible state on `App`.
|
||
5. The sweep — confirm every identifier-taking command
|
||
surfaces unknown-name diagnostics; cross-reference this
|
||
ADR from the diagnostic sites.
|
||
|
||
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.
|
||
|
||
## Amendment 1 — `LIKE` on a numeric column (2026-05-19)
|
||
|
||
§1 defined the WARNING set as exactly a type-mismatched
|
||
comparison and `= NULL`. A third trigger is added:
|
||
|
||
- **`LIKE` against a numeric column.** `LIKE` is a
|
||
text-pattern match — `%` / `_` wildcards over characters.
|
||
Applied to a column of a numeric type (`int`, `real`,
|
||
`decimal`, `serial`) it still runs — the engine matches the
|
||
pattern against the value's text form — but is almost never
|
||
what the user wants; they most likely mean a comparison or
|
||
`BETWEEN`. It is a WARNING, consistent with the advisory
|
||
posture (§5): the command still submits. The negation is
|
||
irrelevant — `NOT LIKE` on a numeric column is just as
|
||
dubious.
|
||
|
||
Scope is deliberately narrow — only *numeric* target columns
|
||
are flagged:
|
||
|
||
- `bool` is integer-backed (0/1) but excluded; the handoff
|
||
item this implements named numeric columns specifically.
|
||
- The text-backed types (`text`, `shortid`, `date`,
|
||
`datetime`) are not flagged: `LIKE 'A%'` on text is its
|
||
intended use, and a prefix match on an ISO `date` /
|
||
`datetime` string is genuinely useful.
|
||
- `blob` is left unflagged.
|
||
|
||
Widening to those types is a future model extension if a need
|
||
appears.
|
||
|
||
This is exactly the kind of "we can tell, before it runs,
|
||
that this is dubious" case §6 anticipated: the trigger plugs
|
||
into the existing model rather than being a one-off check. It
|
||
lives alongside the others in `walker::predicate_warnings`
|
||
(the `Predicate::Like` arm, via `like_numeric_warning`); the
|
||
target column operand's span drives the highlight; the
|
||
message key is `diagnostic.like_numeric`.
|
||
|
||
## See also
|
||
|
||
- ADR-0003 — input modes; the input field and its mode
|
||
label.
|
||
- ADR-0019 — the friendly-message catalog the diagnostic
|
||
message keys resolve through.
|
||
- ADR-0022 — ambient typing assistance: the immediate
|
||
highlighting and hint layers this indicator summarises.
|
||
- ADR-0026 — complex WHERE expressions; the type-mismatch
|
||
and `= NULL` WARNING diagnostics.
|