From dfd3c51643361751c0addd3edfe5829e0cf110a0 Mon Sep 17 00:00:00 2001 From: "claude@clouddev1" Date: Mon, 18 May 2026 23:21:23 +0000 Subject: [PATCH] =?UTF-8?q?chore:=20handoff=2017=20=E2=80=94=20ADR-0026=20?= =?UTF-8?q?complex=20WHERE=20expressions=20implemented?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/handoff/20260518-handoff-17.md | 245 ++++++++++++++++++++++++++++ 1 file changed, 245 insertions(+) create mode 100644 docs/handoff/20260518-handoff-17.md diff --git a/docs/handoff/20260518-handoff-17.md b/docs/handoff/20260518-handoff-17.md new file mode 100644 index 0000000..8850790 --- /dev/null +++ b/docs/handoff/20260518-handoff-17.md @@ -0,0 +1,245 @@ +# Session handoff — 2026-05-18 (17) + +Seventeenth handover. This session was an **implementation +run**: it built **ADR-0026 (complex WHERE expressions)** — the +keystone the handoff-16 design run set up. Steps 1–4 and 6 of +the ADR's build order are done, committed, tested, and clippy- +clean. **Step 5 (the §7 diagnostic *flagging*) is deferred to +ADR-0027** — a deliberate sequencing call; see §3, which is +the one thing in this handoff that may want your input. + +**Headline: complex WHERE expressions work end to end.** +`update` / `delete` / `show data` take a full boolean +expression — `AND` / `OR` / `NOT`, the six comparisons, +`LIKE` / `IN` / `BETWEEN` / `IS [NOT] NULL`, parentheses; it +compiles to parameterised SQL; `show data` gained `where` and +`limit`. The functional `C5a` requirement is satisfied. + +## State at handoff + +**Branch:** `main`. Working tree clean. Four commits since +handoff-16 (`ac41938`), all local — push asynchronously, do +not treat as blocking: + +``` +a50c6cd WHERE expressions: matrix cells + predicate_tail grammar fix (ADR-0026 step 6) +f75f71b WHERE expressions: wire into update/delete/show data + SQL gen (ADR-0026 steps 3-4) +59e6a54 grammar: WHERE-expression fragment + Expr AST + build_expr (ADR-0026 step 2) +f0b2043 walker: add Subgrammar node + recursion-depth cap (ADR-0026 step 1) +``` + +**Tests:** **1079 passing, 0 failing, 1 ignored** (`cargo +test` — up from 1039). The ignored test is the long-standing +`` ```ignore `` doc-test in `src/friendly/mod.rs`. The +typing-surface matrix is **161 cells** (was 152 — `+9` in the +new `tests/typing_surface/where_expression.rs`). + +**Clippy:** clean (`cargo clippy --all-targets -- -D +warnings`, nursery group). + +## §1. What shipped — ADR-0026 steps 1–4 + 6 + +### Step 1 — `Subgrammar` node + depth cap (`f0b2043`) + +New `Node::Subgrammar(&'static Node)` variant: a named +`static` grammar fragment recurses through a reference (a +`Seq` / `Choice` embeds children by value and cannot close a +cycle). `WalkContext::subgrammar_depth` counts active frames; +past `MAX_SUBGRAMMAR_DEPTH` (64) the walk fails friendly +(`parse.custom.expression_too_deep`) instead of overflowing +the stack. Depth is saved/restored per frame so a +rolled-back `Choice` branch leaves no residue. + +### Step 2 — expression grammar + `Expr` AST + `build_expr` (`59e6a54`) + +`src/dsl/grammar/expr.rs` (new): the **stratified** +WHERE-expression grammar — `or` / `and` / `not` / +`bool_primary` / `predicate` tiers as named `static` `Node` +fragments, recursing through `Subgrammar`. New recursive +`Expr` / `Predicate` / `Operand` / `CompareOp` AST in +`dsl::command`. `build_expr` folds the flat matched-terminal +slice into an `Expr`. + +### Steps 3–4 — wiring + SQL generation (`f75f71b`) + +- **Grammar** (`grammar/data.rs`): `update` / `delete` + `where` is now `Subgrammar(&expr::OR_EXPR)`; `show data` + gained optional `where ` and `limit `. +- **AST**: `RowFilter::Where{column,value}` → + `RowFilter::Where(Expr)`; `ShowData` gained + `filter: Option` + `limit: Option`. A + `RowFilter::eq` convenience constructor keeps + simple-equality call sites readable. +- **SQL** (`db.rs`): `compile_expr` lowers `Expr` to a + parameterised WHERE (every literal a `?` placeholder, `<>` + for inequality). `show data … limit n` emits `LIMIT ?` + with an implicit primary-key `ORDER BY`. +- **§8 hints**: the expression's right-hand operands resolve + through a schema-aware `DynamicSubgrammar` + (`where_rhs_operand`) so the hint panel narrows to the + compared column's type — but the operand grammar carries + **no validators**, so a type-mismatched literal parses + (§7 permissive). +- 11 db integration tests cover AND/OR/NOT, BETWEEN, IN, + LIKE, filtered `show data`, limit ordering. + +### Step 6 — matrix cells + a grammar fix (`a50c6cd`) + +`tests/typing_surface/where_expression.rs` (9 cells). Writing +them caught a real bug: `predicate_tail`'s `[NOT] negatable` +branch started with `Optional(not)`, and an `Optional`-first +`Seq` always "commits" — so the walker's `Choice` returned +that branch's `Incomplete` early and **discarded the sibling +branches' expected sets**, dropping `is` and the comparison +operators from completion after a column. Fixed by splitting +into explicit `NOT negatable` + bare `negatable` branches; +the matched terminal sequence is unchanged so `build_expr` +was untouched. + +## §2. The §3 builder realization (settled with the user) + +ADR-0026 §3 admitted two realizations of "the fragment-builder +the walker invokes". The project owner chose **option 1 +("reconstruct in builder")** before implementation: + +- The stratified grammar is walked normally; terminals flow + into the flat `MatchedPath` unchanged (driving highlight / + completion / the expected-set). `build_expr` then folds + that flat slice into the `Expr` — a deterministic recursive + descent, run only at submit-time dispatch. +- Two honest deviations from §3's wording, recorded in + ADR-0026's new **"As-built notes"** section: there is **no + `MatchedKind::Expr` variant** (`MatchedPath` stays purely + terminals; the `Expr` is built in the command + `ast_builder`s), and there **is** a second structural pass + (scoped to dispatch — "no second parse" read as "no + separate parser framework"). + +Do not re-litigate; build to the as-built notes. + +## §3. ⚠️ Step 5 is deferred — this is the one open call + +ADR-0026 step 5 is "schema-aware type-mismatch and `= NULL` +flagging in the walker". It splits in two: + +- **The §7 *behaviour* relaxation — DONE.** A type-mismatched + WHERE literal (`where Age = 3.14`, `Age` an int) used to be + *rejected* at bind time. It no longer is: `bind_where_literal` + binds it by its syntactic shape and the command runs. Test: + `walker::tests::phase_d_delete_where_permits_decimal_at_int_column`. +- **The §7 *diagnostic flagging* — DEFERRED to ADR-0027.** + 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** the + flagging belongs in. ADR-0027 itself says its WARNING + severity "has **no triggers** until ADR-0026 is + implemented" — i.e. ADR-0026's type-mismatch / `= NULL` + conditions *are* those triggers. + +**Why deferred, not done badly now:** building the flagging +as a standalone ad-hoc mechanism (a bespoke `HighlightClass:: +Error` pass + a bespoke hint) before ADR-0027's severity +model exists would almost certainly be reworked when that +model lands. The clean sequencing is: implement ADR-0027's +diagnostics-severity model, and make ADR-0026 §7's +type-mismatch + `= NULL` detection the first triggers feeding +it. + +**This is a sequencing judgement made while you were away.** +If you'd rather have a quick standalone §7 flag now, say so; +otherwise the recommendation is to fold it into ADR-0027. +It's recorded in ADR-0026's "As-built notes", `requirements.md` +`C5a`, and `CLAUDE.md`'s deferred list. + +## §4. What's next + +1. **ADR-0027 — input-field validity indicator.** Now + genuinely unblocked: ADR-0026 supplies the WARNING-severity + triggers (type mismatch, `= NULL`). Implementing ADR-0027 + should also land ADR-0026 step 5's flagging as its first + triggers (see §3). +2. **ADR-0028 — query plans (`explain`).** Also unblocked — + `show data … where` (now real) is the filtered query whose + plan flips between a full scan and an index search. +3. Either order after ADR-0027, per handoff-16. + +Other open clusters are unchanged from handoff-16 §3 +(snapshot/undo `U1`/`U2`; constraints `C3` — `CHECK` can now +reuse ADR-0026's expression grammar; `C4` m:n; `C3a`; `C1` +table rename; `H1`; `SD1`; `TT5` CI; `V4`; `I1`; `TU1`). +Prioritisation is a user product decision — ask. + +## §5. Architectural delta (vs. handoff-16) + +### Grammar / walker + +- `Node::Subgrammar(&'static Node)` — new variant; static + counterpart of `DynamicSubgrammar`. +- `WalkContext::subgrammar_depth: usize`; + `walker::driver::MAX_SUBGRAMMAR_DEPTH = 64`. +- `CompletionProbe` gained `pending_hint_mode` — completion + now suppresses keyword candidates at a grammar-declared + `ProseOnly` slot (the WHERE operand, which also accepts a + column ident, would otherwise surface the misleading + `null`/`true`/`false` trio). +- `src/dsl/grammar/expr.rs` — the whole expression fragment + + `build_expr`. Entry point: `pub static expr::OR_EXPR`. + +### Command AST + +- `Expr` / `Predicate` / `Operand` / `CompareOp` — new, + recursive; in `dsl::command`, re-exported from `dsl`. +- `RowFilter::Where(Expr)` (was `Where{column,value}`); + `RowFilter::eq(col, val)` convenience constructor. +- `Command::ShowData` gained `filter: Option`, + `limit: Option`. + +### db + +- `Request::QueryData` / `Database::query_data` gained + `filter` + `limit`; signature is now + `query_data(table, filter, limit, source)`. +- `compile_expr` / `compile_predicate` / `compile_operand` / + `bind_where_literal` / `syntactic_bound` — `Expr` → SQL. +- `do_query_data` builds `WHERE` / `ORDER BY` / `LIMIT`. + +### Catalog + +- New key `parse.custom.expression_too_deep` (depth cap). + +### A latent bug fixed in passing + +`completion::invalid_ident_at_cursor` treated a digit-led +token (`1`) as an identifier — now that the WHERE operand +slot accepts a column ref *and* a literal, it mis-flagged +`where id=1`'s `1` as an unknown column. It now skips +digit-led tokens. Regression test: +`completion::tests::numeric_literal_in_where_is_not_flagged_as_invalid_column`. + +## §6. How to take over + +1. **Read this file, then handoff-16** for the design-run + context that set ADR-0026 up. +2. **Read `CLAUDE.md`** — working-style rules. +3. **Read `docs/adr/0026-complex-where-expressions.md`** — + especially the new **"As-built notes"** section (§3 + builder realization, the deviations, the step-5 deferral). +4. **Read `docs/adr/0027-input-validity-indicator.md`** — the + recommended next ADR; §3 of this handoff explains how + ADR-0026 step 5 folds into it. +5. **Run `cargo test`** — 1079 passing, 0 failing, 1 ignored. +6. **Run `cargo clippy --all-targets -- -D warnings`** — clean. +7. **Decide the §3 question** (standalone §7 flag now vs. + fold into ADR-0027), then start ADR-0027. + +### Note on the typing-surface matrix + +`tests/typing_surface/` is now **161 cells**. After any +grammar/walker change: a failing cell with *correct* new +behaviour → update its snapshot +(`INSTA_UPDATE=always cargo test --test typing_surface_matrix +`); a failing cell with *wrong* behaviour → the cell +earned its keep. This session: 5 `delete_with_where` / +`update_with_where` snapshots were correctly updated (the +WHERE operand surface widened), and the new +`where_expression` family added 9 cells.