diff --git a/docs/adr/0027-input-validity-indicator.md b/docs/adr/0027-input-validity-indicator.md index 15180ba..4d47490 100644 --- a/docs/adr/0027-input-validity-indicator.md +++ b/docs/adr/0027-input-validity-indicator.md @@ -273,23 +273,72 @@ choices and deviations from the sketch above: 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; +- **WARNING spans are per-literal.** Each `Operand` AST node + carries the byte span of the terminal it was built from, so + an expression WARNING is anchored to exactly the offending + literal. *(The initial ship used a coarse whole-WHERE-clause + span; made precise in the Follow-up section below.)* +- **Highlight & hint wiring — complete.** §2's "highlighting + and the hint panel read the individual diagnostics" is + wired — see the Follow-up section below. *(The initial ship + had the indicator and the model only; the build order did + not enumerate the wiring as a step.)* +- **Debounce decision logic is unit-tested** — it is the + `IndicatorDebounce` state machine in `runtime` (Follow-up + section). The `tokio::time::timeout` interaction itself + stays integration-level. The pure verdict + (`input_verdict` / `input_validity_verdict`) and the + indicator rendering were covered from the initial ship; `input_verdict` has a parse-outcome / schema-existence / expression-warning / existing-cases-sweep test set. +## Follow-up — highlight & hint wiring, precise spans (2026-05-19) + +The As-built notes above left three items open; this pass +closes them. §2's design — "the indicator is the summary; +highlighting and the hint panel read the individual +diagnostics for *where* and *why*" — is now fully realised. + +- **`Operand` carries a source span.** Each WHERE-expression + operand records the byte range of the terminal it was built + from. `Operand`'s `PartialEq` is hand-written to *ignore* + the span — it is editor metadata — so `Command` equality, + and the large `Expr` test corpus, stay whitespace- and + position-independent. Expression WARNINGs are now anchored + to exactly the offending literal, not the whole clause. +- **`walker::input_diagnostics`** is the shared entry point + both UI layers read — the sibling of `input_verdict` that + returns the diagnostic set rather than its severity summary. +- **Highlight overlay.** `render_input_runs` overlays the + schema-aware diagnostics on the input field: an unknown + table / column ERROR in the error colour, an expression + WARNING in `theme.warning`. The overlay is *global* — every + flagged token is coloured wherever it sits, not only under + the cursor, which is precisely the problem §Context + describes. The pre-existing cursor-local invalid-identifier + overlay is kept (it covers in-progress identifiers, which + produce no diagnostics); the two are additive and + idempotent. `overlay_span` recolours a token's whole byte + range (the older `overlay_error` hit only a single byte). +- **Hint panel.** `ambient_hint` surfaces a diagnostic's + message. `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". It + is checked early, ahead of slot hints and completions, so a + flawed-but-parsing command no longer gets the misleading + "Submit with Enter" prose. The diagnostic under the cursor + wins (the panel explains where the user is looking); + otherwise the most severe. +- **Debounce.** The indicator-debounce decision logic moved + out of the event loop into the `IndicatorDebounce` state + machine, unit-tested for the keystroke / settle cycle. + +The one piece still integration-level: a PTY-tier test of the +actual `tokio` debounce *timing* (as distinct from the +now-tested decision logic) — consistent with the four-tier +strategy (ADR-0008), where Tier 4 is wired only for the +critical flows. + ## Amendment 1 — `LIKE` on a numeric column (2026-05-19) §1 defined the WARNING set as exactly a type-mismatched diff --git a/docs/handoff/20260519-handoff-19.md b/docs/handoff/20260519-handoff-19.md new file mode 100644 index 0000000..9c02825 --- /dev/null +++ b/docs/handoff/20260519-handoff-19.md @@ -0,0 +1,239 @@ +# Session handoff — 2026-05-19 (19) + +Nineteenth handover. A focused **implementation run** that +**finished ADR-0027**: the §4 "known follow-ups" from +handoff-18 are all done. ADR-0027's §2 always specified that +"highlighting and the hint panel read the individual +diagnostics" — handoff-18 shipped the indicator and the model +but left that wiring as a follow-up. It is now wired. + +**Headline: the diagnostics are visible where they happen.** +An unknown table / column (ERROR) and a dubious comparison +(WARNING — type mismatch, `= NULL`, and now `LIKE` on a +numeric column) are overlaid on the input field *globally* +and explained in the hint panel — not just summarised by the +`[ERR]` / `[WRN]` indicator. + +## State at handoff + +**Branch:** `main`. Working tree clean. **7 commits** since +handoff-18 (`39b92a7`), all local — push asynchronously, not +blocking. + +``` + docs: handoff 19 + ADR-0027 follow-up/amendment notes +c1c9f6c runtime: extract the indicator debounce into a tested state machine +400fb71 ui: surface diagnostics in the ambient hint panel (ADR-0027 §2) +bbfb70c ui: overlay diagnostic spans on the input field (ADR-0027 §2) +437b2f2 walker: flag LIKE on a numeric column (ADR-0027 Amendment 1) +3912fb5 walker: precise per-literal spans for expression WARNINGs +426e801 command: Operand carries a source span +``` + +**Tests:** **1125 passing, 0 failing, 1 ignored** (`cargo +test` — up from 1100 at handoff-18). The ignored test is the +long-standing `` ```ignore `` doc-test in +`src/friendly/mod.rs`. Typing-surface matrix: **161 cells**, +unchanged (one cell's *input* was corrected — see §3). + +**Clippy:** clean (`cargo clippy --all-targets -- -D +warnings`, nursery group). + +## §1. What this run did — the three §4 follow-ups + +handoff-18 §4 listed three "known follow-ups (none blocking)". +The user reviewed them, judged §4-item-1 to be unfinished +ADR-0027 *scope* (the §2 Decision text, not an optional +extra), and asked for **all three** — including the small ADR +amendment item 3 needs. All three are done. + +### Item 1 — diagnostic highlight + hint wiring + +§2 of ADR-0027: the indicator is the *summary*; highlighting +and the hint panel carry the *where* and *why*. Built across +four commits: + +- **`Operand` carries a source span** (`426e801`). Each + WHERE-expression operand now records the byte range of the + terminal it was built from. `Operand`'s `PartialEq` is + hand-written to **ignore** the span — it is editor + metadata — so `Command` equality stays whitespace- and + position-independent and the large `Expr` test corpus + needed no assertion changes. +- **Precise per-literal WARNING spans** (`3912fb5`). The old + WARNING span was coarse — the whole WHERE clause. + `predicate_warnings` now anchors each WARNING to exactly + the offending literal operand; `where_clause_span` is + gone. +- **Highlight overlay** (`bbfb70c`). `render_input_runs` + overlays the walker's schema-aware diagnostics: ERROR in + the error colour, WARNING in `theme.warning`. New + `overlay_span` covers a token's whole byte range (the + older `overlay_error` hit only the run at a single byte). + The overlay is **global** — every flagged token is + coloured wherever it sits, not just under the cursor, + which is exactly the gap §Context described. The + pre-existing cursor-local invalid-identifier overlay is + kept (it covers in-progress idents, which produce no + diagnostics); the two are additive and idempotent. +- **Hint panel** (`400fb71`). `ambient_hint` surfaces a + diagnostic's message. `walker::input_diagnostics` is + non-empty *only* for a command that structurally parses, + so a non-empty result means "complete and submittable, + but wrong" — checked early, ahead of slot hints and + completions, so a flawed-but-parsing command no longer + shows the misleading "Submit with Enter". The diagnostic + under the cursor wins, else the most severe. + +### Item 2 — debounce test coverage (`c1c9f6c`) + +The indicator debounce was two locals in the event loop with +no unit coverage. The decision logic is now the +`IndicatorDebounce` state machine in `runtime` — `note_event` +/ `settle` / `is_armed` / `visible`, 7 unit tests for the +keystroke / settle cycle (including: a background event +mid-typing must not cancel the owed recompute). No behaviour +change; the `tokio` timer and terminal stay in the loop. A +PTY-tier test of the *actual timing* remains the one +integration-level gap (consistent with ADR-0008's four-tier +strategy). + +### Item 3 — `LIKE` on a numeric column (`437b2f2`) + +A genuinely new WARNING trigger, so it needed an ADR change: +**ADR-0027 Amendment 1**. `LIKE` is a text-pattern match; +against an `int` / `real` / `decimal` / `serial` column it +runs but is almost never intended. New `Type::is_numeric`, +catalog key `diagnostic.like_numeric`. Scope is deliberately +numeric-only — `bool` and the text-/blob-backed types are +not flagged; rationale is in the amendment. `docs/adr/ +README.md` updated per the index-upkeep rule. + +## §2. ⚠️ Two manual-testing bugs — NOT fixed, queued next + +Mid-run the user reported two bugs from manual testing and +explicitly deferred the fixes. **These are the top of the +next session's queue.** They are unrelated to ADR-0027. + +1. **`add 1:n relationship` — missing completion + wrong + usage hint.** Typing `add 1:n relationship from + Artists.album_id to Albums.id --` does **not** offer + `--create-fk`. Separately, the ambient hint at the end of + `add 1:n relationship from … to …` shows the **wrong** + usage block — `add column …` — because + `grammar::usage_key_for_input` (commit `151ed08`) + disambiguates `add column` / `add index` but not the + `1:n relationship` form, so it falls back to the first + `add` form. Same *class* of bug `151ed08` fixed; the fix + is to teach `usage_key_for_input` the relationship form, + plus wire the `--create-fk` flag completion. + *(The user confirmed the `[ERR]` shown for the + non-existent column in that input is correct and welcome + — that is this run's schema-existence diagnostic working. + Do not touch it.)* +2. **`--resume` points at an unpersisted temp project.** An + empty temp project is created on launch but, by design + (ADR-0015), not persisted when it has no content — yet + `last_project` still records its path, so a later + `--resume` fails with a confusing "path does not exist". + The user asked for: (a) `last_project` updated only once + a path is **truly persisted with content**; (b) a + friendly error when `--resume` finds no `last_project` + at all. + +Both are recorded as tasks (#7, #8 in this session's task +list) with full repro detail. + +## §3. Architectural delta (vs. handoff-18) + +### Command AST + +- `Operand` is now a struct-variant enum — + `Column { name, span }` / `Literal { value, span }` — + carrying a byte `span`. **`PartialEq` is hand-written to + ignore `span`** (see the type docs); `Eq` still derived. + `Operand::span()` accessor; `Operand::NO_SPAN` for + programmatically-built operands (`RowFilter::eq`). + +### Walker + +- `walker::input_diagnostics(source, schema) -> Vec` + — the sibling of `input_verdict`; the shared entry point + for the highlight overlay and the hint panel. +- `predicate_warnings` emits per-literal spans; + `pair_type_mismatch` returns `(message, span)`; + `where_clause_span` removed. +- `like_numeric_warning` — the `LIKE`-on-numeric WARNING + (Amendment 1). `Predicate::Like` is no longer a no-op arm. + +### ui / input_render + +- `render_input_runs` overlays diagnostics (`overlay_span`, + new). `ambient_hint` has a diagnostic branch right after + the Tab-cycle memo; `pick_hint_diagnostic` chooses + cursor-local, else most-severe. + +### runtime + +- `IndicatorDebounce` state machine replaces the + `indicator_pending` local; `app.input_indicator` mirrors + its `visible` for the renderer. + +### Types / catalog + +- `Type::is_numeric()`. New catalog key + `diagnostic.like_numeric` (+ `friendly::keys` registry + entry). + +## §4. Docs touched + +- **ADR-0027** — new "Follow-up" section (the §2 wiring + completed) and "Amendment 1" (`LIKE` on a numeric column); + the three stale "As-built notes" bullets updated to point + at them. +- `docs/adr/README.md` — ADR-0027 line notes Amendment 1. +- `docs/requirements.md` — test baseline (→ 1125) and the + `S6` entry note the completed wiring + amendment. + +## §5. What's next + +1. **The two §2 bugs** — `add 1:n relationship` completion / + usage hint, and `--resume` / `last_project`. The user + deferred these explicitly; they are first in line. +2. **ADR-0028 — query plans (`explain`).** Still the last + unimplemented member of the handoff-16 design trio; + **Accepted**, not started. `show data … where` is the + filtered query whose plan flips between a full scan and + an index search. + +Other open clusters unchanged from handoff-16/17/18 +(snapshot/undo `U`-series; constraints `C3`; `C4` m:n; +`C3a`; `C1` table rename; `H1`; `SD1`; `TT5` CI; `V4`; `I1`; +`TU1`). Prioritisation is a user product decision — ask. + +## §6. How to take over + +1. **Read this file, then handoff-18** (ADR-0026/0027 + implementation), then handoff-17 / handoff-16 as needed. +2. **Read `CLAUDE.md`** — working-style rules. +3. **Read `docs/adr/0027-input-validity-indicator.md`** — + especially the new "Follow-up" and "Amendment 1" + sections. +4. **Run `cargo test`** — 1125 passing, 0 failing, 1 ignored. +5. **Run `cargo clippy --all-targets -- -D warnings`** — clean. +6. **Start with §5.1 — the two manual-testing bugs.** They + were deferred mid-run with the user's agreement; they are + the committed next step, ahead of ADR-0028. + +### Note on the typing-surface matrix + +`tests/typing_surface/` is **161 cells**, unchanged. One +cell — `where_expression::complex_and_or_expression_parses` +— used a column (`t`) that does not exist in its schema; the +new schema-existence diagnostic correctly flagged it, which +revealed the cell was not testing what it claimed. Its input +was corrected to a real column and the snapshot regenerated. +The matrix-snapshot discipline from handoff-17/18 still +applies: a failing cell with *correct* new behaviour → +update its snapshot; with *wrong* behaviour → the cell +earned its keep. diff --git a/docs/requirements.md b/docs/requirements.md index e63e7bc..2964ed3 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -26,14 +26,16 @@ repo is pushed). ## Test baseline -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 +After the ADR-0027 highlight / hint follow-up (precise WARNING +spans, the diagnostic overlay + hint wiring, the +`LIKE`-on-numeric WARNING, the debounce state machine): +**1125 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.) +group enabled. (Earlier reference points: 1100 after ADR-0027's +initial ship; 1079 after ADR-0026 (complex WHERE expressions); +1039 after ADR-0025 (indexes); 1006 after ADR-0024 + the +handoff-14 cleanup; 449 after B2/C2.) --- @@ -72,7 +74,11 @@ B2/C2.) 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.)* + `warning` theme colour. A follow-up pass completed §2's + highlight + hint wiring — diagnostics overlaid on the input + field and surfaced in the hint panel, with precise + per-literal WARNING spans — and Amendment 1 adds a + `LIKE`-on-numeric-column WARNING.)* ## Input field