1fad29c0f9a0574a0f48c73678be98c2bae20e42
320 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 claude@clouddev1
|
55b784597a |
docs: ADR-0033 Accepted (Phase 3 complete) + session handoff 34
Phase 3 of the ADR-0030 SQL surface (DML) is implemented and verified through sub-phase 3k; mark ADR-0033 Accepted in the ADR and the README index (index-upkeep rule). Add handoff 34 tracking the close-out and teeing up ADR-0034 (history journal + replay filter) as the next job. 1645 passing / 0 failing / 0 skipped / 1 ignored. Clippy clean. |
||
|
claude@clouddev1
|
380c4238ef |
test+docs: 3k Phase-3 verification sweep — e2e DML + filled cross-cut matrix
Sub-phase 3k of ADR-0033. Adds the Tier-3 end-to-end DML suite (tests/sql_dml_e2e.rs) and the cross-cut gap-fill tests, fills the verification matrix (every row a verified file::function), and produces the phase-exit report. - tests/sql_dml_e2e.rs: INSERT…SELECT cross-table, all-ten-type multi-row INSERT + RETURNING type recovery, UPDATE-with-subquery-in-SET, cascade DELETE, UPSERT round-trip, RETURNING x3, history.log replay, OOS rejections (full §13 table), validity-indicator-from-SQL-DML. - walker/mod.rs, highlight.rs, completion.rs, input_render.rs: inherited-diagnostic, DML-keyword highlight, INSERT INTO completion, and advanced-mode DML hint-panel cross-cuts. - Matrix correction (user-confirmed): predicate warnings fire on row-scoped DML slots; INSERT VALUES has no row scope (ADR-0033 §8.4). - Auto-snapshot row marked N/A (user-confirmed): ADR-0006 unimplemented for both paths; deferred. /runda round: added an advanced-mode DML hint-panel test (A6 was attributed to simple-mode prose under the §8 advanced heading); extended OOS coverage to the full ADR-0033 §13 table (OOS-5 INDEXED BY / OOS-6 multi-statement) + a trailing-semicolon guard. 1645 passing / 0 failing / 0 skipped / 1 ignored. Clippy clean. |
||
|
claude@clouddev1
|
a5cdb00a86 | docs: session handoff 33 — ADR-0033 sub-phase 3j + Amendment 3 + /runda round | ||
|
claude@clouddev1
|
d5c7f63513 |
grammar+walker: 3j — shared insert/update/delete entry words (ADR-0033 §2 / Amendments 1 & 3)
Wire `insert`/`update`/`delete` as shared DSL/SQL entry words through the
category-grouped dispatcher (ADR-0033 Amendment 1): the Advanced SQL nodes
move off the dev words (`sqlinsert`/`sql_update`/`sql_delete`) to the real
keywords, registered alongside the Simple DSL nodes. Remove the dev-word
scaffold; collapse build_sql_{insert,update,delete} to source.trim();
de-duplicate the two REGISTRY entry-word listing sites.
Dispatch model (ADR-0033 Amendment 3, written this round):
- A command is the mode-rooted grammar-path outcome; identity is intrinsic.
Advanced mode tries SQL first, falling back to the Simple DSL command when
no SQL branch matches a token (`delete … --all-rows` falls back;
`update … --all-rows` does not — the SET expression absorbs it, harmless
since the engine treats `--all-rows` as a comment).
- Simple mode commits the DSL candidate for a shared word, surfacing the real
DSL error; bare "this is SQL" is reserved for SQL-only entry words
(`select`/`with`). A content rejection on the SQL candidate (internal
table) is committed, never masked by the DSL fallback.
Combined DSL-error + advanced-SQL pointer (ADR-0033 Amendment 3): a Simple-mode
definite DSL error that would run as SQL in advanced mode gains the
`advanced_mode.also_valid_sql` suffix — in the live hint (ambient_hint_in_mode)
and on submit (dispatch_dsl), via the shared advanced_alternative_note — so the
actionable DSL fix and the mode pointer coexist (submit covers constructs that
surface only on submit, e.g. `delete … returning`).
Internal-table rejection symmetrised (/runda finding B, ADR-0030 §6): the DSL
data-command target slots (insert/update/delete/show data/show table) gained
reject_internal_table, so `__rdbms_*` tables are refused in Simple mode too —
previously only the advanced SQL grammar rejected them.
Mode-awareness: classify_input_with_schema_in_mode and
invalid_ident_at_cursor_in_mode stop leaking the advanced SQL view into
simple-mode hints for shared words.
Tests: dev-word inputs migrated to the real words (advanced); DSL grammar /
completion / phase-D / db tests parse in Simple mode (the DSL surface); replay
keeps its advanced-mode model (one stale assertion fixed); dispatcher routing,
combined-pointer, and internal-table tests added. Suite 1626 pass / 0 fail /
1 ignored; clippy --all-targets -D warnings clean.
Defer M4 (execution-time mode side-channel; tracked in requirements.md) to its
own ADR.
|
||
|
claude@clouddev1
|
c16196fc7f | docs: session handoff 32 — Phase 3 (3f–3i) + ADR-0034 + /runda round | ||
|
claude@clouddev1
|
8d17583fe0 |
walker: 3i /runda DA round — fix INSERT-target scope confusion (6 cases)
A focused adversarial round (/runda) found a single root cause with
six manifestations, all pre-existing latent false-positives: the
INSERT target is recorded under the `insert_target_table` role, not
as a diagnostic `bindings` entry, so refs that should resolve to the
*target* row were instead checked against the statement's bindings —
which for an `INSERT … SELECT` are the SELECT's *source* tables (the
wrong scope), producing false unknown_column / unknown_qualifier
diagnostics on valid input.
New helper bare_ref_insert_target re-scopes a ref onto the INSERT
target when it sits in a target-referencing region: the UPSERT
DO UPDATE action (byte range) or an INSERT's RETURNING list. Applied
across every ref form:
1. INSERT column list (insert_column) — validated vs the target,
skipped in the bare-column branch (was checked vs SELECT source).
2. ON CONFLICT (col) target (conflict_target_column) — same.
3. DO UPDATE SET RHS / WHERE bare refs — validated vs the target
(also closes the #12 residual for VALUES upserts).
4. RETURNING bare refs — validated vs the target.
5. target-qualified refs `t.col` in DO UPDATE / RETURNING — the
unified `excluded` / target-qualifier resolution in the
qualified-ref None branch.
6. target-qualified star `t.*` in RETURNING — same re-scoping in
the qualified-star handler.
Each fix has a positive (resolves cleanly) and negative (genuinely
unknown column / unrelated qualifier still flagged) test; the
`excluded` leak guard and all prior diagnostics remain green.
1613 pass / 0 fail / 1 ignored. Clippy clean.
|
||
|
claude@clouddev1
|
4fa0aa06e9 |
db+walker: 3i DA pass — not_null PK false-positive fix + arity hardening
DA pass on 3i. Fix: build_schema_cache set not_null = c.notnull || c.primary_key, which would false-flag an omitted `int` PK as a not_null_missing WARNING — but an int PK is an INTEGER PRIMARY KEY rowid alias that auto-fills (and SQLite's PK-NULL quirk means a PK isn't implicitly NOT NULL anyway). Use c.notnull alone (ADR-0033 §8.3 "declared NOT NULL"): faithful and false-positive-free. Arity-walk hardening (same class as the ON CONFLICT regression the existing tests caught mid-3i): RETURNING after VALUES is a depth-0 keyword that ends the tuple list (only the real tuple is flagged), and a comma nested in a function-call value (depth ≥ 2) does not inflate the tuple's value count. Tests (+2). 1598 pass / 0 fail / 1 ignored. Clippy clean. |
||
|
claude@clouddev1
|
cfd925c24a |
grammar+db: 3i — DML column-existence + cross-cut verification (ADR-0033 §8)
New dml_target_column_diagnostics pass: an ERROR for an unknown column in the INSERT column list or the UPSERT DO UPDATE SET (validated directly against the insert_target_table). The INSERT target isn't a flat-scope `bindings` entry, so the existing schema-existence pass didn't cover these; a targeted pass avoids the false INSERT…SELECT ambiguity a global binding would cause. Closes the 3i cross-cut "schema-existence fires on INSERT VALUES" gate item, and closes the DA finding #12 (UPSERT DO UPDATE SET column now flagged like a top-level UPDATE's SET column). Residual: bare sql_expr_ident refs in the DO UPDATE SET RHS / WHERE remain unvalidated for upserts (the documented flat-scope limitation). Tests (+5): unknown INSERT column flagged + known silent; unknown DO UPDATE SET column flagged + known/excluded silent; predicate warning (= NULL) fires on a SQL UPDATE WHERE (cross-cut). 1596 pass / 0 fail / 1 ignored. Clippy clean. |
||
|
claude@clouddev1
|
2d1112d0f3 |
grammar+db: 3i — not_null_missing diagnostic + TableColumn constraints (ADR-0033 §8.3)
Extend SchemaCache TableColumn with not_null + has_default (with a TableColumn::new constructor for the common no-constraint case), populated in build_schema_cache from ColumnDescription (a PK column counts as not-null). New dml_not_null_missing_diagnostics pass: a WARNING when a SQL INSERT's explicit column list omits a column that is NOT NULL with no DEFAULT — advisory (the engine enforces it). serial/shortid (auto-filled) and defaulted columns are excluded. Anchored on the target-table ident (no token for the omitted column). Catalog key diagnostic.not_null_missing (engine-neutral). Tests (+4): fires on omitted required column; silent when included, when defaulted, and for auto-gen serial/shortid. ~24 TableColumn literal sites updated for the two new fields (build clean). 1591 pass / 0 fail / 1 ignored. Clippy clean. All three ADR-0033 §8 DML diagnostics now implemented. Remaining 3i: cross-cut verification + #12 UPSERT DO UPDATE validation. |
||
|
claude@clouddev1
|
6db8253c25 |
grammar+db: 3i — insert_arity_mismatch diagnostic (ADR-0033 §8.1)
New dml_insert_arity_diagnostics pass (ERROR): when an explicit (column_name_list) arity disagrees with a row's arity. VALUES tuples are checked per-row (each offending tuple emits its own diagnostic on its span; matched rows stay silent). INSERT … SELECT compares the first SELECT leg's projection arity, anchored on the first projection item; a WITH-prefixed row source is skipped (engine still reports it — a false positive would be worse). No-column-list form deferred (needs schema; outside the 3i gate). The VALUES walk stops at the first depth-0 keyword so an ON CONFLICT (col) conflict target / RETURNING tail is not mis-counted as a value tuple (caught by the existing upsert_excluded tests during dev). Catalog key diagnostic.insert_arity_mismatch (engine-neutral). Tests (+7): single-row + matched + per-row multi-row; INSERT…SELECT mismatch + matched; ON CONFLICT interaction (only the real tuple flagged, clean case silent). 1587 pass / 0 fail / 1 ignored. Clippy clean. Remaining 3i: not_null_missing (needs TableColumn not_null+default), cross-cut verification, #12 UPSERT DO UPDATE validation. |
||
|
claude@clouddev1
|
be63315e61 |
grammar+db: 3i — auto_column_overridden diagnostic (ADR-0033 §8.2)
New dml_auto_column_diagnostics pass: a WARNING when a SQL INSERT's explicit column list names a serial/shortid (auto-generated) column — the explicit value bypasses the auto-counter/generator and may collide with later auto-generated values. Advisory only (ADR-0027 §1); the statement still runs. Conflict-target columns (distinct conflict_target_column role) are not mistaken for inserted columns. Catalog key diagnostic.auto_column_overridden (engine-neutral). Tests (+4): serial + shortid fire; omitted is silent; ON CONFLICT target not falsely flagged. 1580 pass / 0 fail / 1 ignored. Clippy clean. Remaining 3i: insert_arity_mismatch, not_null_missing (needs TableColumn not_null+default), cross-cut verification, #12 UPSERT DO UPDATE validation. |
||
|
claude@clouddev1
|
6b8888f105 |
grammar+db: 3h — UPSERT ON CONFLICT DO NOTHING / DO UPDATE (ADR-0033 §9)
on_conflict_clause on SQL_INSERT_SHAPE: optional (col,…) conflict target (distinct conflict_target_column role so it never enters listed_columns), DO NOTHING / DO UPDATE SET … [WHERE …]. `do` is factored out of the action Choice so nothing/update disambiguate without tripping the walk_seq/walk_choice shared-prefix trap (ADR-0033 Amendment 1). Worker runs the UPSERT verbatim (SQLite native); no new execution path. build_sql_insert: row_source now stops before the FIRST trailing clause — ON CONFLICT (3h) or RETURNING (3g) — and do_sql_insert's shortid auto-fill rewrite re-appends the whole trailing tail, so an auto-filled INSERT keeps its ON CONFLICT / RETURNING. excluded pseudo-table (§9): resolves to the target's columns inside the DO UPDATE action and completes at `excluded.|`, but stays flagged as unknown_qualifier in VALUES / RETURNING / non-upsert statements. Diagnostic pass scopes it by the DO UPDATE byte-range (update token → RETURNING/end); completion resolves it against the INSERT target's current_table_columns. NOTE: scoping uses byte-range rather than the plan's prescribed from_scope TableBinding push — same behaviour, no walker scope-frame change. Tests (+13): grammar accept/reject; DO NOTHING / DO UPDATE-excluded / no-target execution + persistence; auto-fill × ON CONFLICT with a REAL unique conflict (proves the clause survives the rewrite, not a no-op); excluded resolves in DO UPDATE SET + WHERE, flagged in VALUES (incl. same statement), unknown column under excluded; excluded.| completion; conflict-target not in listed_columns. 1576 pass / 0 fail / 1 ignored. Clippy clean. Dev sql_insert entry word still removed in 3j. Known follow-up (tracked for 3i): UPSERT DO UPDATE bare column refs (SET LHS / WHERE) are not schema-validated, unlike regular UPDATE — the INSERT target isn't a diagnostic binding. Fits 3i's cross-cut SET/WHERE validation scope. |
||
|
claude@clouddev1
|
fd8b74ba5e |
grammar+db: 3g — RETURNING on INSERT/UPDATE/DELETE (ADR-0033 §5)
Shared RETURNING_CLAUSE (reuses Phase-2 PROJECTION_LIST, now pub(crate)) as an optional tail on all three SQL DML shapes. `returning: bool` on the Command variants, set by the ast-builders and threaded to the worker. run_returning collects the returned rows as a DataResult (RETURNING mutates + yields in one pass), reusing resolve_select_column_types for bare-column type recovery; computed projections stay typeless. DeleteResult gains a `data` field rendered alongside the cascade summary. Follow-set fix: `returning` is added to the table-source and projection bare-alias follow-sets so an INSERT … SELECT row source stops before RETURNING instead of reading it as a table alias. Auto-fill × RETURNING: build_sql_insert stops row_source before the RETURNING token (keeping it preparable for shortid materialisation), and plan_shortid_autofill re-appends the RETURNING tail so generated shortids surface in RETURNING *. Tests (+17): grammar accept on all three; INSERT/UPDATE/DELETE RETURNING incl. *, aliases, multi-row, type recovery + computed- typeless; auto-fill × RETURNING (single + multi-row distinct ids); INSERT…SELECT…RETURNING execution; UPDATE…RETURNING zero-match; DELETE…RETURNING cascade+rows; app-level render of both. Dev sql_insert/sql_update/sql_delete entry words still removed in 3j. 1562 pass / 0 fail / 1 ignored. Clippy clean. |
||
|
claude@clouddev1
|
b935090d7b |
docs: ADR-0034 — history.log as a complete journal; replay reads ok-only
Found while implementing 3f: history.log is success-only, but the in-memory Up/Down recall ring records every submission — and the ring is re-seeded from the log on open, so failed commands are recallable in-session yet lost across sessions. Replay and recall also want different inputs (state-builders vs everything-typed), which one success-only file can't serve. And replay never parsed the pipe format (run_replay parses whole lines), so `replay history.log` fails on line 1 with no test covering it. Decision: history.log becomes a complete journal tagged ok/err; hydration reads all, replay reads ok-only and learns the format. Amends ADR-0006 + ADR-0015 §5/§12. Code deferred to two tracked sub-tasks. No migration for existing all-ok logs. |
||
|
claude@clouddev1
|
62f09bebc5 |
db: fix self-referential cascade over-count + SQL-delete render test
A self-referential ON DELETE CASCADE FK (e.g. T.ParentId -> T.id) is returned by read_relationships_inbound as a child whose table IS the delete target. The before/after row-count diff then includes the directly-deleted rows (already in rows_affected), so deleting a chain root reported 3 cascaded rows when only 2 were removed via the self-reference. Fix in both do_delete (DSL) and do_sql_delete (SQL): when the child table equals the target, subtract rows_affected from the diff and guard on the corrected count (a leaf delete no longer reports a phantom 0-row self-cascade); the target's CSV is already queued, so a self-ref child is not re-added to rewritten_tables. Pre-existing in do_delete; surfaced by the 3f DA pass, fixed in both paths to keep DSL/SQL parity. Behaviour: report only the rows removed via the self-reference (user-confirmed). Also adds an app-level render test for the SQL DELETE path (handle_dsl_delete_success via CommandOutcome::Delete) — the shared renderer's ok-summary + per-relationship cascade line were exercised only through the DSL path before. Test-first: self_referential_cascade_counts_only_cascaded_rows added for both paths (asserted 2, failed at 3 before the fix). 1545 pass / 0 fail / 1 ignored. Clippy clean. |
||
|
claude@clouddev1
|
2c86a1313e |
grammar+db: 3f — SQL DELETE + cascade summary (ADR-0033 §1/§7)
New src/dsl/grammar/sql_delete.rs (FROM <table> [WHERE] [;]), Command::SqlDelete, Request::RunSqlDelete, do_sql_delete worker. do_sql_delete mirrors the DSL do_delete: detect FK cascade by before/after child row-count diffing, re-persist target + every cascade-affected child, history-on-success inside the tx. Reuses CommandOutcome::Delete -> handle_dsl_delete_success, so the per-relationship cascade summary formatter is shared, not duplicated. ADR-0033 Amendment 2: supersedes §7's WHERE-injected pre-count. Its premise (DSL handler builds pre-counts from the typed Expr) was wrong — do_delete uses count-diff. The pre-count would also have broken the §2 parity promise by reporting SET NULL the DSL path doesn't. Count- diff gives exact parity, no WHERE-byte extraction, and withdraws R2. SET NULL reporting deferred for both paths (user-confirmed). Tests: +6 grammar unit, +12 integration (cascade parity with DSL, both R2 subquery cases, before-execute order, no-WHERE, FK-rejection rollback, childless-parent, two-child cascade). 1542 pass / 0 fail / 1 ignored. Clippy clean. Dev sql_delete entry word removed in 3j. |
||
|
claude@clouddev1
|
70ecf5535e | docs: session handoff 31 — Phase 3 (3c/3d/3e) + interlude close | ||
|
claude@clouddev1
|
53808ed9d7 |
grammar+db: 3e — SQL UPDATE grammar + execution (ADR-0033 §2)
New src/dsl/grammar/sql_update.rs: SQL_UPDATE_SHAPE =
<table> SET col = sql_expr (',' …)* [WHERE sql_expr] [';'], the
__rdbms_* target rejection, and the shared sql_expr on both the
assignment RHS and the predicate. No --all-rows rail — a SQL
UPDATE without WHERE runs as written (ADR-0030 §12). Reuses
sql_select::WHERE_CLAUSE (now pub(crate)) so the predicate
diagnostics are identical. The target uses the shared `table_name`
ident role (not a bespoke one) so the Phase-2 schema-existence and
predicate-warning passes collect it as a scope binding and check
the SET / WHERE columns for free — a bespoke role left them
unchecked (the cross-cut tests caught this).
Command::SqlUpdate { sql, target_table }; Request::RunSqlUpdate +
do_sql_update (execute validated SQL via execute_with_fk_enrichment,
re-persist the target CSV, append history.log). 3e surfaces the
affected-row count only; precise row output is RETURNING (3g), so
the update-success render skips a column-less data set rather than
showing a misleading "(no rows)" band. Behind the dev `sql_update`
entry word until 3j.
Tests: grammar accept/reject; integration (single/multi-col,
no-WHERE all-rows, sql_expr in SET, scalar subquery in SET,
zero-match success, history); walker cross-cut (unknown SET column
→ unknown_column, `= NULL` in WHERE → eq_null warning); app-level
render-guard both ways (column-less → count only; with columns →
table renders). 1524 green, clippy clean.
|
||
|
claude@clouddev1
|
18d34d0d36 |
db: 3d fix — don't let shortid auto-fill mask INSERT arity mismatch
plan_shortid_autofill read exactly listed_columns.len() cells from the materialised row source. When the row source produced a different column count than the user's list, the extra columns were silently dropped (wider → wrong data, insert succeeded) or read out of range (narrower). Guard: if the materialised statement's column_count differs from the listed-column count, skip auto-fill and execute the verbatim statement so the engine reports the mismatch — matching the non-auto-fill path. A friendly pre-flight diagnostic remains sub-phase 3i. Tests: VALUES with too many values; INSERT…SELECT with a wider and a narrower projection — each rejected with nothing persisted. |
||
|
claude@clouddev1
|
78ad476d24 |
db+grammar: 3d — shortid auto-fill for SQL INSERT (ADR-0033 §6)
When an INSERT's column list omits one or more shortid columns, the worker now fills them. Command::SqlInsert gains listed_columns and row_source, captured in build_sql_insert from the matched path (the row source is located by the first values/select/with Word token, so a string literal like 'select' can't be mistaken for the keyword). do_sql_insert calls plan_shortid_autofill, which — per the user-confirmed Option B — materialises the row source by running it as a query, generates a distinct shortid per row via the existing generate_shortid_batch (deduped against stored values), and reconstructs a parameterised multi-row INSERT over the listed columns plus the omitted shortid columns. Uniform for VALUES and INSERT…SELECT, and handles multiple omitted shortids in one row (each gets its own batch). No explicit list, no omitted shortid, or a zero-row source → execute verbatim (the 3b path). serial stays engine-filled via rowid. history.log keeps the original line, never the rewrite (§11). Tests: VALUES single/multi-row distinct; explicit override honoured; INSERT…SELECT distinct fills; combined serial(engine) + shortid(worker); two shortids (PK + non-PK) both fill; one provided + one omitted; compound-PK shortid member; mixed-case column name (ADR-0009 DA gate); original-source-in-history on the rewrite path. Still behind the dev `sqlinsert` entry word (3j). 1503 green, clippy clean. |
||
|
claude@clouddev1
|
6ff9144c7a |
grammar: 3c — INSERT … SELECT row source (ADR-0033 §4)
Make the INSERT row source a Choice between the VALUES clause and Subgrammar(&sql_select::SQL_SELECT_COMPOUND). SQL_SELECT_COMPOUND is itself a Choice that admits a leading WITH, so a WITH-prefixed SELECT row source (R4) parses through it for free; the two branches start on disjoint keywords (values vs select/with) so the Choice never ambiguously commits. No worker change — do_sql_insert already executes the validated SQL and re-persists, and the engine handles insert-from-query. Tests: grammar accept (plain / column-list+projection / WITH- prefixed / trailing-semi) and reject (__rdbms_* on the SELECT's FROM slot, incomplete select); integration parse-path lowering + worker round-trip (rows land, CSV re-persisted) incl. R4 WITH end- to-end; walker cross-cut that the Phase-2 unknown_column diagnostic fires on the INSERT…SELECT projection; DA-gate test that a self- sourced INSERT…SELECT runs as a plain insert (no cascade summary — that is DELETE-only). Still behind the dev `sqlinsert` entry word (shared `insert` is 3j). 1493 tests green, clippy clean. |
||
|
claude@clouddev1
|
7f68a53f86 |
walker+completion: surface list trailing-optionals + identifiers-first ordering (ADR-0022 Amendment 2)
walk_repeated discarded the last matched item's trailing-optional expectations at a clean item boundary, so a comma-separated list offered no continuation after a complete item: `order by Name ` gave no asc/desc, `select Name ` no `as`, `create table … Code(text) ` no not/unique/default/check. Capture the last item's skipped set and surface it when the list ends at an item boundary (the separator `,` itself is deliberately not surfaced). That fix made expression-position candidate lists long, which exposed a visibility problem: the hint panel's candidate line is single-row and window-scrolls on overflow, centring on item 0 when nothing is selected — so with keywords-first, schema identifiers scrolled off behind the `>` marker. Reverse the ordering: schema identifiers (table/column/relationship names) now sort before keywords, since a name the user would have to look up is the highest-value completion and must stay visible (keywords are learned over time; the tok_identifier/tok_keyword colour split marks the boundary). This reverses the handoff-14 keywords-first call, now recorded in ADR-0022 Amendment 2. Tests: walker expected-set + completion-layer regressions for the trailing-optionals and the ordering; candidate_ordering.rs header invariant inverted; ~20 typing-surface snapshots re-baselined; a two-line hint box recorded as a deferred follow-up. |
||
|
claude@clouddev1
|
43c49f4d1b |
walker: F5 — drop preceding-clause keywords from committed-child Incomplete sets
walk_seq's Incomplete arm unconditionally merged the accumulated skipped-Optional expectations (pending_skipped) into the child's expected set. When a child committed terminals before going Incomplete (e.g. `order by` consumed, now awaiting a sort item), this leaked ~13 clause keywords from clauses positioned *before* the committed child — WHERE/GROUP BY/HAVING, the FROM's JOIN options, set-ops — into the ORDER BY completion list, shoving the actual columns off-screen. Merge pending_skipped only when the Incomplete-producing child consumed nothing (path length unchanged): the cursor still sits at the optional boundary, so those optionals are genuine alternatives. A committed child means the cursor is past them. Tests: walker expected-set guard (+ over-correction guard) and a full-stack completion-layer regression test. |
||
|
claude@clouddev1
|
5b88ccd2c3 |
docs: session handoff 30 — Phase 3 (3a/3b) + advanced-mode completion interlude
3a (grouping dispatch / ADR-0033 Amendment 1) and 3b (SQL INSERT) done; advanced-mode ambient assistance re-enabled (ADR-0022 Amendment 1) plus F1/F2 completion fixes. F3/F4/case-sensitivity triaged as non-issues; F5 (walk_seq keyword pollution) remains and needs user OK. Old-project migration deferred. Resume at F5, then Phase 3 sub-phase 3c. |
||
|
claude@clouddev1
|
1c8cbc1983 |
completion+hint: F1/F2 advanced-mode completion fixes
F1: the hint panel is the completion UI, so a premature "no such table/
column" ERROR on the token the user is still typing must not shadow its
completion. ambient_hint now suppresses an under-cursor error diagnostic
when a completion exists for the (non-empty) partial it overlaps, and
falls through to the candidates. Genuinely-unknown names (no prefix match)
still show the error; WARNINGs are unaffected. Both modes.
F2: projection-before-FROM ("select <cursor> from T" after deleting *)
offered the global column list instead of T's columns, because the §10.6
look-ahead's full-input walk can't reach FROM through an empty projection.
When the look-ahead finds no scope, retry with a neutral placeholder
inserted at the cursor so the trailing FROM/CTE scope is recovered for
narrowing. Only the repaired walk's from_scope/cte_bindings are used.
Test-first: 3 F1 tests (mid-typed completes, unknown still errors, simple-
mode DSL) + 1 F2 multi-table narrowing test. 1469 baseline green.
|
||
|
claude@clouddev1
|
ed40445828 |
ui: re-enable advanced-mode ambient assistance (ADR-0022 Amendment 1)
Advanced-mode hinting + completion-preview were dead: render_hint_panel returned None for advanced mode (stale ADR-0022 §12 gate, predating the SQL grammar) and the hint resolver/ambient_hint never threaded Mode, so a SQL statement was gated as "this is SQL". The unified walker (ADR-0030/ 0031/0032) speaks SQL, so this lifts the gate. - ambient_hint_in_mode + hint_resolution_at_input_in_mode + expected_for_hint_snapshot(mode); candidate/diagnostic/parse sub-calls run in the active mode. - render_hint_panel calls ambient for all modes; one-shot `:` sigil stripped (strip_one_shot_prefix) so `: sel` hints `select`. - ADR-0022 Amendment 1 + README index. Found by manual advanced-mode testing; Phase 2 marked SQL hint/completion green at the engine layer but never exercised the UI. App-level render test (advanced_mode_hint_panel_surfaces_sql_candidates) + ambient-layer regression locks. 1466 baseline green. |
||
|
claude@clouddev1
|
c87363168f |
grammar+db: 3b — SQL INSERT grammar + minimal execution (ADR-0033 §1)
SQL_INSERT_SHAPE (INTO <table> [(cols)] VALUES tuple(s)) with __rdbms_*
target rejection; Command::SqlInsert{sql,target_table}; Request::RunSqlInsert
+ do_sql_insert worker (tx-guarded: execute, then finalize_persistence for
CSV + history before commit, so failures roll back and don't re-persist).
Auto-show is best-effort via last_insert_rowid range.
Isolated behind a dev `sqlinsert` entry word (Advanced) so the SQL path is
testable without making `insert` a shared word yet (that's 3j, after 3d
auto-fill parity). Command::SqlInsert carries only sql+target_table; the
plan's listed_columns/returning land in 3d/3g where they're read.
6 grammar accept/reject tests + 8 integration tests (single/multi-row,
column-list, full-arity, history, rollback-on-failure, multi-row atomicity,
parse-path reconstruction, internal-table rejection). 1452 baseline green.
|
||
|
claude@clouddev1
|
4e16d97fe0 |
walker: 3a — category-grouped mode-aware dispatch (ADR-0033 Amendment 1)
Replaces ADR-0033 §2's original Node::Guard + Choice(SQL,DSL) mechanism,
which was found during 3a to be unworkable: any guard-in-Choice approach
forces a walk_choice change (walk_choice falls through only on NoMatch, so
simple-mode valid-DSL would wrongly surface "this is SQL"), and walk_seq
treats a NoMatch past idx 0 as a hard Failed, breaking advanced-mode DSL
fall-through.
Mechanism (Amendment 1): each REGISTRY entry is tagged
CommandCategory::{Simple, Advanced}, generalising the whole-command
is_advanced_only gate. walk() becomes a thin dispatcher over decide()
(mode-aware candidate selection: simple commits the DSL node or emits the
"this is SQL" hint; advanced tries SQL first, DSL as a full-line fallback)
and an extracted walk_one_command(); speculative match-testing runs on a
scratch WalkContext so the caller's context is only touched by the
committed walk. No Node::Guard, no walk_choice/walk_seq change.
6 dispatch smoke tests on a shared-entry-word smoke registry; 1446 baseline
green; clippy clean.
|
||
|
claude@clouddev1
|
a37a0b7d40 |
docs: ADR-0033 Phase 3 — implementation plan + cross-cut matrix
Build-order plan (sub-phases 3a–3k) with per-sub-phase scope, exit gates, and written DA gates, modelled on docs/plans/20260520-adr-0032-phase-2.md. Centrepiece is the cross-cut verification matrix scaffold (~75 rows) grouped by ADR-0033 section (statement shapes, dispatch, RETURNING, shortid, cascade, diagnostics, UPSERT, inherited Phase-2 diagnostics, ambient assistance, engine-neutrality, persistence, OOS rejections), to be filled in during 3k. Carries the handoff-29 §4 process pins into the relevant sections: DA critiques listed before verdict, no silent out-of-scope classification, and matrix attribution requiring SQL-input tests for SQL claims. Records four open questions to escalate before code starts (shortid SELECT row-source path, R1 mechanism fallback, cascade pre-count construction, UPSERT catalog wording). |
||
|
claude@clouddev1
|
ded1ca8e26 |
docs: session handoff 29 — Phase 3 planning done; sub-phase 3a is next
Twenty-ninth handover. This session drafted ADR-0033 as the planning artifact for ADR-0030 Phase 3 (SQL DML in Advanced mode). Documents the ten settled design decisions (Q1-Q10), the dispatch architecture (SQL-first / DSL-fallback via the new Node::Guard mechanism), the eleven phased sub-phases (3a-3k) each with their exit gates, and the open implementation risks (R1-R4). Points the next session at sub-phase 3a (Node::Guard scaffolding) as the concrete entry point, with the plan-doc cross-cut matrix as the immediate prerequisite task. Pins four process lessons from the Phase-2 session (DA rubber-stamp risk, defer-trap reflexes, tests-first on gap closure, matrix attribution verification). State: 1446 / 0 / 1 passing (unchanged — planning-only session). Clippy clean. |
||
|
claude@clouddev1
|
555149da3c |
docs: ADR-0033 — SQL DML grammar (INSERT / UPDATE / DELETE)
Phase 3 of ADR-0030's SQL-surface roadmap. Status: Proposed. Statement shapes (§1): single- and multi-row INSERT, INSERT…SELECT (recursing through ADR-0032's SQL_SELECT_COMPOUND), UPDATE with SET assignment list, DELETE, all three optionally followed by RETURNING projection_list. Full UPSERT (ON CONFLICT … DO NOTHING / DO UPDATE with the SQLite/PostgreSQL `excluded` pseudo-table) on INSERT. Dispatch (§2): SQL-first / DSL-fallback in Advanced mode via Choice(SQL_shape, DSL_shape) per shared entry word. Requires a new walker capability — Node::Guard(fn), a zero-byte-consumption gating node — landed as the first sub-phase's work (R1 mitigation budgeted). Execution (§10): three typed Command variants (SqlInsert / SqlUpdate / SqlDelete) carrying target_table, listed_columns, and a returning: bool flag. Worker handlers know per-kind specialisations: shortid auto-fill (§6, parity with DSL), cascade summary (§7, WHERE byte-range injection into pre-count subqueries), DataResult routing on RETURNING (§5). Diagnostics (§8): three new keys (insert_arity_mismatch ERROR, auto_column_overridden WARNING, not_null_missing WARNING) with positive + negative test requirements. OOS list (§13): DEFAULT VALUES (seed feature), SQLite OR-prefixes, UPDATE FROM, WITH-prefixed DML, indexed-by hints, multi-statement batches. Implementation notes: eleven phased sub-phases (3a–3k) each with explicit exit gates + written DA gates. Ordering puts Node::Guard scaffolding (3a) FIRST so the dispatch mechanism is proven before DML grammar lands on top. Initial DA review (Initial DA review section) recorded seven critiques that were resolved before status moved to Proposed; a second-pass DA surfaced an eighth (Node::Guard wasn't an existing walker capability) and added it to §2 + sub-phase 3a's scope. |
||
|
claude@clouddev1
|
fa417a47cc |
docs: session handoff 28 — Phase 2 closed; Phase 3 is next
Twenty-eighth handover. Captures this session's Phase-2 completion (sub-phases 2d.1, 2e, 2g + DA-driven rework) and points the next session at ADR-0030 Phase 3 (DML in Advanced mode) as the natural continuation. Pinned items for the next session: - Four non-blocking DA observations from this session's verification report (group-by pattern overbreadth, look-ahead probe cost, tests-after-code on matrix coverage, matrix attribution wasn't row-by-row verified). - A process lesson on DA discipline: rubber-stamp PASS verdicts must die. Next DA review lists critiques first, concludes after. - Phase 3 design considerations: UPDATE/DELETE table-source scope, INSERT…SELECT, RETURNING, UPSERT, DML-specific diagnostics. State: 1446 / 0 / 1 passing. Clippy clean. Phase 2 fully pushed. |
||
|
claude@clouddev1
|
1c42e78d92 |
docs: ADR-0032 Phase 2 — phase-exit verification report
The §5 deliverable from the implementation plan, this time with
a non-rubber-stamp DA review.
Documents:
- Final test state (1446 / 0 / 1 — clippy clean).
- Cross-cut matrix outcome (29 rows, all green per the plan doc).
- Requirements-to-test mapping for ADR-0032 §§1–13 + both
Amendments.
- Autonomous-decision audit (7 implementation decisions, each
with explicit user-confirmation pointer).
- DA's written final review with three blocking critiques
(now closed in commit
|
||
|
claude@clouddev1
|
05884bd13a |
2g rework: address DA findings on type recovery + engine routing + UI
Three DA critiques from the Phase-2 verification flagged real gaps; this commit closes them. 1. Type recovery row-independence (critique #1). The all-10-types test left col_blob NULL because the DSL Value enum has no Blob variant. The DA flagged this as a potential row-dependence gap. Added `database_run_select_type_recovery_works_on_empty_table` that proves column-origin metadata works on Text AND Blob columns with zero rows, pinning the invariant. The all-types test now carries an explicit comment referencing it. 2. Engine.* pattern matching against real SQLite output (critique #2). The pre-rework tests fed `translate_generic` hand-coded strings; never verified that the pinned SQLite version actually produces those wordings. Added three engine-routing tests in `tests/sql_select.rs` that produce real engine errors via `run_select` and assert catalog routing. Aggregate-in-WHERE confirms end-to-end. GROUP-BY-required and scalar-subquery are SQLite-permissive (no real error on the natural triggers), so those tests verify the matcher doesn't false-positive on benign queries + that synthetic messages route correctly. 3. Manual TUI verification (critique #3) surfaced an additional gap: `App::input_validity_verdict()` was hard-coded silent in Advanced mode, so SQL predicate warnings emitted but never reached the [WRN] indicator. Wired the verdict through to the active effective mode; updated two pre-existing tests that pinned the now-superseded "silent in Advanced" behavior; added one new test confirming a SQL `LIKE`-on-numeric warning fires the indicator. Launched the TUI, typed a representative warning-triggering SELECT, confirmed SELECT/FROM/WHERE/LIKE highlight as keyword colour AND the [WRN] indicator appears. Test totals: 1441 → 1446 passing (+5). Clippy clean. |
||
|
claude@clouddev1
|
ed881eea59 |
2g: advanced-mode highlight + engine.* wiring + matrix tests
Cross-cut verification matrix for ADR-0032 Phase 2 is now fully populated with concrete test references — every row green. Filling the matrix surfaced three real gaps that this commit closes. 1. Advanced-mode syntax highlighting (ADR-0030 §8 matrix row). The `ui.rs` Advanced branch routed through `plain_input_spans`, bypassing the highlight walker entirely. In production SQL keywords past the entry word rendered as plain identifiers. Fix: mode-aware variants of `highlight_runs`, `render_input_runs`, `lex_to_runs`, and `input_diagnostics`; the Advanced render path now uses the highlighted form with `Mode::Advanced`. `plain_input_spans` removed (unused). 2. Engine.* key wiring (ADR-0032 §11.4 / §13 matrix rows + handoff §3.3 follow-up). The four Phase-2 engine.* catalog entries were authored in 2d but never reached: `translate_generic` discarded the engine message and returned a vague catalog entry. Fix: pattern-match the engine message text for the four Phase-2 categories (aggregate misuse, group-by required, compound arity mismatch fallback, scalar-subquery cardinality) inside `translate_generic`, routing each to its engine-neutral catalog entry. 3. Matrix-coverage tests. Thirteen new tests covering the rows that had no explicit coverage: - 3 SQL keyword/operator/CASE highlight tests - 4 engine.* engine-message tests - 3 sql_expr column-completion tests (WHERE, HAVING) - 3 predicate-warning slot tests (CASE, ORDER BY, projection) - 1 all-10-playground-types recovery test (tests/sql_select.rs) Plan document (docs/plans/20260520-adr-0032-phase-2.md) updated: every (TBD) row in the cross-cut matrix replaced with a concrete test file::function reference and a green status marker. Test totals: 1428 → 1441 passing (+13 new). Clippy clean. |
||
|
claude@clouddev1
|
ee0dafd86b |
docs: ADR-0032 Amendment 2 + §10.6 regression tests
Amendment 2 records the §10.6 fixup-pass mechanism choice. §10.6 prescribes "rewriting the highlight class" on projection-list idents at end-of-walk; the actual implementation uses a different mechanism that achieves the identical user-visible behavior: 1. 2d's two-pass schema-existence diagnostic collects every FROM binding from the matched path first, then resolves projection idents against the complete scope. The post-walk re-resolve §10.6 calls for, just embedded in the diagnostic emitter. 2. input_render.rs's diagnostic-overlay path colors each diagnostic span Error/Warning, achieving the visual change §10.6 describes without needing a new HighlightClass variant. The completion-mid-typing piece is improved by the §10.5 look-ahead probe (sub-phase 2e earlier). Four new regression tests in `projection_before_from_tests` pin the behavior so a future refactor can't silently regress it: correct ident resolves silently, unknown ident flags via diagnostic on its span, multi-projection only flags unknowns, projection-without-FROM is silent. ADR index entry updated to reference Amendment 2. Test totals: 1424 → 1428 passing (+4). Clippy clean. |
||
|
claude@clouddev1
|
0fc7b082b2 |
completion: §10.5 qualified-prefix + edit-scenario look-ahead
ADR-0032 §10.5 — at the cursor, an `<ident>.` prefix narrows column candidates to that qualifier's binding columns. Resolves through from_scope aliases first, then table names, then cte_bindings (for `cte_alias.|`). Falls back to the schema cache for DSL paths (`from <Table>.<col>`). Unresolved qualifier → empty column list; the structural error path surfaces the unresolved-prefix message. Look-ahead probe — the "edit an existing query" workflow. When the cursor is mid-projection but FROM exists after the cursor, a second walk on the full input populates from_scope and the column candidates narrow accordingly. Gated on the leading walk producing no scope so cursor-past-FROM positions pay no cost. The full input must parse for this to work; an unparseable mid-edit state falls back to the §10.6 global posture. CompletionProbe now exposes `from_scope` (top-frame table bindings) and `cte_bindings` (union of in-scope CTE bindings, innermost-first dedupe). The walker drains these at the cursor position; the completion engine reads them for qualifier resolution and unqualified narrowing. Test totals: 1415 → 1424 passing (+9: 5 qualified-prefix + 4 look-ahead). Clippy clean. |
||
|
claude@clouddev1
|
fd259048da |
grammar: admit WITH inside subqueries / CTE bodies (ADR-0032 §10.3)
ADR-0032 §10.3 says cte_bindings lives on the scope frame, with inner subqueries free to declare their own CTEs that shadow outer ones. The grammar didn't actually admit nested WITH inside SQL_SELECT_COMPOUND — a real ADR-vs-implementation gap. Closes the gap by making SQL_SELECT_COMPOUND a Choice between a WITH-prefixed form and a plain form. The naive Optional-prefix approach silently broke the paren-vs-subquery dispatch in sql_expr.rs's PAREN_GROUP: Optional matches 0 bytes, committing the Seq, so SELECT_CORE's NoMatch on `(a + b)` became Failed and the Choice couldn't fall through to or_expr. The Choice-fronted form keeps the fast NoMatch on non-WITH non-SELECT first tokens. Side effect: scalar subquery / IN / EXISTS / derived-table bodies now admit a leading WITH too, which matches standard SQL. Updated two tests that were guarding the old `(WITH …)` rejection behavior. Added one new harvest test exercising nested-WITH inside a CTE body — the harvest's `expand_binding` mechanism already handled the data correctly; the grammar gap was the sole blocker. Test totals: 1414 → 1415 passing (+1 nested-with-in-cte test). Clippy clean. |
||
|
claude@clouddev1
|
dd37a1cbfc |
walker: 2e prereq — §10.3 stage-2 CTE harvest + cte_arity_mismatch
Implements the six ADR-0032 §10.3 output-column derivation rules at CTE body-frame exit, populating the placeholder CteBinding's columns. Unblocks `diagnostic.cte_arity_mismatch` (which compares declared col-list arity vs derived projection arity) and the upcoming qualified-prefix completion in 2e proper. - `WalkContext::pending_cte_harvest`: bookkeeping for an in-progress CTE harvest, armed by writes_cte_name + extended by cte_column idents, consumed by the next walk_scoped_subgrammar invocation (CTE syntax has no intervening ScopedSubgrammar, so timing is deterministic). Cleared on every walk_scoped_subgrammar entry to prevent stale state surviving a speculative walk rollback. - `run_cte_harvest`: post-walk path-scan classifier that reconstructs the body's first leg's projection-list and applies the six derivation rules. Compound bodies take columns from the first leg per spec; recursive CTE bodies take the non-recursive (first) leg. Optional (col-list) renames positionally with preserved types. - `expand_binding`: bridges a TableBinding to a CteColumn list, resolving CTE-source bindings (empty columns + table-name matches an in-scope CteBinding) through to the CTE's harvested columns. Enables sibling CTEs to project correctly: in \`WITH a AS (...), b AS (SELECT * FROM a) ...\`, b's harvest sees a's derived columns through the body's from_scope binding. - `WalkContext::pending_diagnostics`: accumulator for diagnostics emitted DURING the walk by node handlers with context the post-walk passes can't reconstruct. Drained by the top-level walk function on both match and non-match paths so a re-used context can't leak entries between walks. Test totals: 1399 → 1414 passing (+15: 10 derivation rules + 1 sibling CTE + 4 arity match/mismatch tests). Clippy clean. |
||
|
claude@clouddev1
|
c20c6e05ca |
walker: 2d.1 — projection-alias misplaced + compound-arity ERROR passes
Closes the two diagnostics deferred by sub-phase 2d that were not attached to a user-approved deferral. `cte_arity_mismatch` stays deferred — it depends on the §10.3 stage-2 CTE harvest, which IS a user-approved deferral. - `diagnostic.projection_alias_misplaced` (ADR-0032 §11.2): emitted when a projection alias is referenced from `WHERE` / `HAVING` / `GROUP BY`. `ORDER BY` references are allowed and silent. The pass is integrated into `schema_existence_diagnostics`: when a bare-column ref doesn't resolve to any binding's column but DOES match a projection alias in the current SELECT leg, the new diagnostic pre-empts the misleading `unknown_column` that would otherwise fire on the same span. Real-column-shadowed-by-alias cases (engine resolves to the table column) stay silent. Subquery scopes (paren depth > 0) keep their own implicit alias bag — outer aliases don't leak into inner WHERE. - `diagnostic.compound_arity_mismatch` (ADR-0032 §11.2 / §11.7): a new MatchedPath-walking pass that counts projection items per SELECT leg by tallying top-level commas at the leg's own paren- depth, then compares adjacent legs across `UNION` / `UNION ALL` / `INTERSECT` / `EXCEPT` operators. The diagnostic anchors on the operator span. Per-depth book-keeping lets chained compound queries inside CTE bodies / subqueries report independently. Function-call argument commas (deeper depth) are correctly ignored. Test totals: 1385 → 1399 passing (+14), 0 failed, 1 ignored. Clippy clean. |
||
|
claude@clouddev1
|
5d716e64ab |
docs: handoff 27 — ADR-0032 Phase 2 sub-phases 2a / 2b / 2c / 2d / 2f shipped
Session summary covering nine implementation commits since handoff-26 (e032f01..0c3847a). Records the user-approved deferral of §10.3 stage 2 (CTE column-derivation harvest), flags three further 2d diagnostic-key deferrals that need user confirmation, and points the next implementer at the seams for sub-phases 2e (qualified-prefix completion + post-walk fixup) and 2g (verification sweep + final report). Test totals at handoff: 1385 passing, 0 failed, 1 ignored (+125 from the 1260 baseline at handoff-26). |
||
|
claude@clouddev1
|
0c3847a5b9 |
db: column-origin type recovery in SELECT results (sub-phase 2f)
`Cargo.toml`: add `column_metadata` to rusqlite's feature list. This pulls in the SQLite `SQLITE_ENABLE_COLUMN_METADATA` compile flag and surfaces `sqlite3_column_table_name` / `sqlite3_column_origin_name` on prepared statements via rusqlite's `Statement::columns_with_metadata()`. `do_run_select` in db.rs now calls a new `resolve_select_column_types(conn, stmt)` helper after `prepare`. The helper walks each result-column's origin metadata; when both `table_name` and `origin_name` come back populated (the result column traces back to a base-table column), it looks up the playground type in `__rdbms_playground_columns`. The per-column types thread through to `format_cell(value, ty)` so the data-table renderer (ADR-0016) gets the same per-type rendering it applies to `show data` results. Effect: ADR-0030 Phase-1 §4.5 (bool SELECT results render as `0` / `1`) is lifted for any bare-column reference whose origin the engine carries through — per ADR-0032 Amendment 1 (2026-05-20 empirical probe), that means all non-recursive CTE bodies, scalar subqueries (aliased or not), derived tables, set ops, and JOINs. Computed projections and recursive-CTE result columns remain typeless (the engine populates no origin), which the renderer handles via neutral alignment. The lookup is engine-driven verbatim — no grammar-side structural classification (ADR-0032 Amendment 1 replaces §12's original "structurally a single column reference" rule with "trust column_table_name / column_origin_name"). Tests (3 new in `tests/sql_select.rs`, all green): - `database_run_select_recovers_bool_column_type` — the Phase-1 §4.5 case: `SELECT Active FROM Products` returns `column_types = [Some(Bool)]` and rows render as `true` / `false`. - `database_run_select_recovers_text_type_through_alias` — `SELECT Name AS n FROM Users` remaps the result column name to `n` but the origin metadata still resolves the playground type to `Some(Text)`. - `database_run_select_computed_expression_stays_typeless` — `SELECT Score + 1 FROM T` keeps `column_types[0] = None`, the documented Amendment-1 exception. The CTE pass-through, scalar subquery, set-op, and JOIN cases all work for free given the empirical findings; their behaviour is asserted by the Amendment-1 probe results recorded in the ADR, so no per-case integration tests are duplicated here. Test totals: 1382 → 1385 passing (+3), 0 failed, 1 ignored. Clippy clean. |
||
|
claude@clouddev1
|
c5cf03b152 |
walker: SQL diagnostics — multi-binding scope, qualified refs, Phase-1 gap closure (sub-phase 2d)
Implements the bulk of ADR-0032 §11 diagnostics. The
schema-existence pass becomes multi-binding-aware; the SQL
predicate-warning pass closes the Phase-1 carry-over gap
named in §11.6; pre-flight duplicate-CTE detection lands
(user-approved Plan §Open-2); a `data::WITH` CommandNode
makes WITH-prefixed statements dispatch through the registry.
Catalog (`src/friendly/strings/en-US.yaml`, `src/friendly/keys.rs`):
- Six new `diagnostic.*` keys: ambiguous_column,
compound_arity_mismatch, cte_arity_mismatch, duplicate_cte,
projection_alias_misplaced, unknown_qualifier.
- Eight new `engine.*` translation keys (ADR-0032 §11.5) for
the friendly-error layer to render engine messages in
engine-neutral wording. The catalog entries are authored;
wiring them into the engine-error path is deferred (the
friendly layer reads these by key when reached).
Schema-existence diagnostic (`schema_existence_diagnostics`)
extended per ADR-0032 §11.2:
- A pre-pass collects all `table_name` / `cte_name` / table-
alias idents into a `PassBinding` vec + a CTE name list,
sidestepping the projection-before-FROM ordering problem
(§10.6). The main pass then resolves identifiers against the
complete scope.
- Bare column references resolve against any binding's
columns. Zero matches → `diagnostic.unknown_column` (the
table arg lists all in-scope tables in the multi-binding
case). Two-or-more matches → `diagnostic.ambiguous_column`.
- Qualified `t.c` refs detect their qualifier via a look-ahead
on the matched path (Punct '.' + Ident{role:
sql_expr_qualified_ref} after the leading Ident). Unknown
qualifier → `diagnostic.unknown_qualifier`; the column check
then runs against the resolved binding's table.
- The `t.*` qualified-wildcard's `qualified_star_qualifier`
ident also resolves through the same pass.
- CTE-name references in table-source slots accept silently
(the CTE binding's columns are unknown until the deferred
§10.3 stage-2 harvest lands, so bare column refs into a
CTE binding short-circuit to "accept silently").
- Duplicate CTE names in the same `WITH` block emit
`diagnostic.duplicate_cte` on the second occurrence
(Plan §Open-2).
Phase-1 gap closure (`sql_predicate_warnings`, ADR-0032 §11.6):
A new MatchedPath-walking pass that identifies predicate-tail
shapes by node-name labels and emits the same `diagnostic.*`
keys the DSL `Expr` AST pass already emitted (`eq_null`,
`like_numeric`, `type_mismatch`). Scoped to bare column refs
in `<column> <op> <literal>` form — qualified-ref and
expression-operand cases stay un-flagged in this minimal pass,
which is a safe false-negative posture (the warning is
advisory; the engine still runs). Runs alongside the schema-
existence pass on every successful SQL parse — WHERE,
HAVING, JOIN ON, projection, ORDER BY all get warnings
uniformly. Tests cover all three keys plus the negative
"compatible types don't warn" case.
WITH dispatch (`data::WITH`):
`with x as (…) select * from x` now dispatches via the registry
with entry word `with`. Shape: `SQL_WITH_TAIL`, the post-`WITH`
portion of a statement (optional `RECURSIVE`, the cte_def
list, the trailing compound_select, optional `;`). Both
`data::SELECT` and `data::WITH` route to `build_select` and
produce `Command::Select { sql: source }` — execution is
grammar-as-text, so the entry-word split doesn't fork the
exec path. `is_advanced_only` extended to include `with`.
Deferred per the 2d-scoped DA review (documented as a
`(TBD)` in the cross-cut matrix for 2g):
- `diagnostic.projection_alias_misplaced` — requires clause
detection (the matched-path is flat).
- `diagnostic.compound_arity_mismatch` — needs per-leg
projection counting.
- `diagnostic.cte_arity_mismatch` — depends on §10.3 stage-2
harvest, which 2b deferred.
- `engine.*` key wiring into the friendly-error layer — the
catalog entries are authored; the engine-error path reads
them by key when reached, but no proactive enhancement of
the layer here.
Test totals: 1366 → 1382 passing (+16: 10 schema-existence
multi-binding + diagnostic tests, 7 Phase-1 gap closure
tests, minus duplicates from prior runs), 0 failed, 1 ignored.
Clippy clean.
|
||
|
claude@clouddev1
|
a491df32a0 |
grammar: migrate Phase-1 SELECT to the ADR-0032 fragment (sub-phase 2c)
The Phase-1 SQL `SELECT` grammar nodes that used to live in `src/dsl/grammar/data.rs` retire — 22 statics / consts and the `reject_internal_table` validator copy are removed, ~150 lines of grammar machinery gone. `data::SELECT.shape` now references the post-`SELECT` portion of the ADR-0032 fragment via a thin `Node::Subgrammar(&sql_select::SQL_SELECT_TAIL)`. `SQL_SELECT_TAIL` is a new export from `sql_select.rs`, parallel to `SQL_SELECT_STATEMENT`. It represents what a top-level `SELECT` statement looks like AFTER the registry's entry-word dispatch has already consumed the leading `SELECT` keyword: the DISTINCT/ALL prefix, projection list, optional FROM / WHERE / GROUP BY / HAVING, the compound set-op chain (each subsequent leg's `SELECT` is part of `SET_OP_TAIL`), outer ORDER BY / LIMIT, and a tolerated trailing `;`. WITH-prefixed statements (`WITH x AS (…) SELECT * FROM x`) are NOT in 2c's scope — they need a separate `data::WITH` `CommandNode` so the entry-word dispatch routes correctly. For now, top-level WITH continues to fall through to the chumsky parser route (the same as in Phase 1). The `SQL_SELECT_STATEMENT` static (which includes the optional WITH prefix) stays available for use by that future CommandNode or by any other consumer that needs the full statement shape. All seven Phase-1 SQL `SELECT` integration tests (`tests/sql_select.rs`) pass without modification, satisfying the 2c exit gate's "behaviour preserved" requirement. The 70 fragment unit tests and the 26 driver-level scope tests also pass — the migration is a refactor, no new tests required. Behaviour change explicitly sanctioned by ADR-0032 §8: Phase-1's `LIMIT_VALIDATOR` (positive-int-only, parse-time) is superseded by the full `sql_expr` admission. `LIMIT max(10, x)` and similar now parse; the engine constrains the value at execution time per the ADR's "grammar admits, engine rejects" posture. Plan §2b status note: the 2026-05-20 deferral of §10.3 stage 2 (CTE output-column harvest derivation) is recorded in `docs/plans/20260520-adr-0032-phase-2.md` per the user-approved deferral. Test totals: 1366 passing (unchanged), 0 failed, 1 ignored. Clippy clean. data.rs loses ~150 lines of dead grammar; the single source of truth for the SQL `SELECT` shape is now `sql_select.rs`. |
||
|
claude@clouddev1
|
4ff054ca75 |
walker: populate cte_bindings placeholders + projection_aliases (ADR-0032 §10.3 stage 1 / §10.4)
Sub-phase 2b checkpoints 4 and 5 combined — adds the placeholder CTE binding push (§10.3 stage 1) and the projection alias accumulator (§10.4). Node::Ident gains two more flags, mechanically applied to every existing site: - `writes_cte_name: bool` — push a placeholder `CteBinding` (name only, empty columns) onto the top `ScopeFrame`'s `cte_bindings`. Set on `CTE_NAME_IDENT` in sql_select.rs. Fires BEFORE the body's `ScopedSubgrammar` enters (the CTE-def Seq's ident slot precedes the body's `(`), so the body can self-reference the CTE name as a valid table source (WITH RECURSIVE). - `writes_projection_alias: bool` — append the matched name to the top frame's `projection_aliases`. Set on `PROJECTION_BARE_ALIAS_IDENT` so both the AS-form (`a AS alpha`) and bare-form (`a alpha`) paths capture cleanly. The ident is shared by both paths through `PROJECTION_AS_ALIAS` and the lookahead factory, so capturing on the ident itself covers both forms with no duplication. The §10.3 stage-2 harvest (deriving CTE output columns from the body's projection per the six derivation rules in the ADR's table) is structurally deferred — the placeholder's `columns` stays empty until the harvest is wired. This is intentional scope honesty: the placeholder-name presence is sufficient for the schema-existence diagnostic (2d) to recognize CTE names as valid table sources, and the qualified-prefix completion (2e) will populate the columns when the harvest hook is added there. Tests below assert the placeholder-name behavior; the column-derivation tests from plan §2b's exit gate will be satisfied incrementally as later sub-phases need them. Tests (8 new, all green): - Single CTE → one placeholder binding with the matched name. - Multiple CTEs → placeholders in declaration order. - Recursive CTE → name visible inside body (the body's `from r` reference parses; verified by the walk completing). - Projection aliases via AS form → captured into the top frame's `projection_aliases`. - Projection aliases via bare form → captured. - Mixed alias forms → captured in projection order, with unaliased projection items absent from the alias list. - No aliases → empty `projection_aliases`. - CTE body aliases do not leak to outer scope (the body's frame pops on `ScopedSubgrammar` exit, taking its projection_aliases with it). All 1358 previous tests still pass. Test totals: 1366 passing, 0 failed, 1 ignored. Clippy clean. This closes out the scope-accumulator side of sub-phase 2b. The remaining 2b-style work — full CTE column-derivation harvest per §10.3's six rules — folds into 2d (where the arity-check pass needs declared-vs-derived column counts) and 2e (where qualified-prefix completion needs CTE columns). |
||
|
claude@clouddev1
|
b522d09f5a |
walker: populate from_scope table bindings (ADR-0032 §10.1)
Sub-phase 2b checkpoint 3 — the `writes_table` / `writes_table_alias` flags now drive the multi-binding `from_scope` accumulator on the top `ScopeFrame`. Node::Ident gains `writes_table_alias: bool`. When set on an ident-name slot, the matched name lands on the most-recently- pushed `TableBinding`'s `alias`. All 46 existing Ident sites across the codebase are updated to `writes_table_alias: false` (mechanical — no behavioral change for DSL paths). walk_ident's `writes_table` semantics extend: - `IdentSource::Tables` matches with `writes_table: true` still populate `current_table` / `current_table_columns` as before (preserved for DSL paths that read those fields directly via the dynamic-subgrammar / column-writes machinery), AND now also push a fresh `TableBinding` onto the top ScopeFrame's `from_scope`. The two mechanisms coexist additively — current_table reflects the most-recent `writes_table` write (single-binding view, as before); from_scope is the authoritative multi-binding accumulator that SQL JOINs, subqueries, and CTE bodies use. sql_select.rs splits the alias slot into two ident variants: - `PROJECTION_BARE_ALIAS_IDENT` (role `projection_alias`) — no scope writes; capture into `projection_aliases` is 2b-5. - `TABLE_SOURCE_BARE_ALIAS_IDENT` (role `table_alias`, `writes_table_alias: true`) — sets the top binding's alias. The `AS alias` form likewise splits into PROJECTION_AS_ALIAS and TABLE_SOURCE_AS_ALIAS so each path threads through the correct ident. The bare-alias lookahead factories return the projection or table-source ident accordingly. `TABLE_NAME_IDENT` in sql_select.rs gets `writes_table: true` so each FROM / JOIN table source pushes a binding. The schema-resolved columns are stored on the TableBinding for later use by qualified-prefix completion (2e) and the schema-existence diagnostic (2d). Tests (9 new, all green): - single from-table → one binding - AS alias / bare alias on from-table → alias captured - two-way JOIN → two bindings, correct order - two-way JOIN with both aliased → two bindings with aliases - three-way JOIN (left + bare) → three bindings in order - subquery from_scope does not leak to outer scope (the ScopedSubgrammar push/pop discipline at work) - CTE body from_scope does not leak to outer scope (the outer scope sees only the CTE-name reference, not the body's internals) - SELECT without FROM → empty from_scope All 1351 previous tests still pass — DSL paths untouched. Test totals: 1358 passing, 0 failed, 1 ignored. Clippy clean. Frame is_cte_body marker, body-projection harvest, and projection_aliases population are the remaining 2b work (2b-4 and 2b-5). |
||
|
claude@clouddev1
|
98a74b23d3 |
grammar: sql_expr additive extensions for §5/§6, CTE body rewires to ScopedSubgrammar
Sub-phase 2b checkpoint 2 — closes the recursion loop between sql_expr.rs and sql_select.rs so subquery expressions and qualified column refs become structurally valid in every SQL context where they belong. sql_expr.rs: - §5 qualified-ref tail. `name_or_call` gains a `.identifier` suffix as a Choice sibling of the function-call `(args)` tail. The leading identifier is still matched once (per ADR-0031 §1's factoring); the optional tail dispatches between the two suffixes by their first character (`.` vs `(`). - §6.1 scalar subquery as primary. The `(or_expr)` and `(SELECT …)` branches share the leading `(`; the first inside token (`SELECT` → subquery, anything else → expression) discriminates. The subquery recurses through `Node::ScopedSubgrammar(&sql_select::SQL_SELECT_COMPOUND)`. - §6.2 IN (subquery) predicate. Sibling of the existing IN-value-list; same `(` factoring, same dispatch. - §6.3 [NOT] EXISTS primary. Bare `EXISTS (compound_select)` lives in `primary`; `NOT EXISTS` falls out via the existing `not_expr := NOT not_expr` tier above `primary`. sql_select.rs: - CTE body recursion rewires `Node::Subgrammar` → `Node::ScopedSubgrammar`, matching §10.2. The top-level statement's COMPOUND embedding stays plain Subgrammar — the implicit bottom frame is the right scope for a statement- level SELECT. Structural side-effect — const-eval cycle workaround: Closing the sql_expr ⇄ sql_select reference loop made Rust's const-evaluator follow the cycle through every `const Node` that transitively reaches it. Mirroring sql_expr.rs's existing pattern, composition Nodes in sql_select.rs (Seq / Choice / Optional / Repeated / Lookahead) are now `static Node` and appear in slice positions through `Node::Subgrammar(&NAME)` wraps; only leaf items (Punct, Word, Ident) remain `const`. Same workaround applies to data.rs's SELECT_PROJ_LIST / SELECT_PROJECTION chain and the inlined `SQL_EXPR` reference. Statics resolve lazily at link time, so the cycle is valid; const-eval is not, and the named `const SQL_EXPR` alias is gone in both files (replaced with the inline `Node::Subgrammar (&sql_expr::SQL_OR_EXPR)` expression at every use site). Test coverage: - sql_expr.rs gains 11 new tests for qualified refs, scalar subquery, IN-subquery, EXISTS / NOT EXISTS, nested subqueries, and the existing IN-value-list form (regression). - sql_select.rs gains 7 new tests for qualified refs in WHERE, scalar subqueries in WHERE / projection, IN / EXISTS / NOT EXISTS in WHERE, nested subqueries, and qualified refs inside CTE bodies. - All 70 prior sql_select tests still pass; the 2a baseline is preserved. `(WITH x AS (…) SELECT * FROM x)` is explicitly NOT admitted as a scalar subquery — ADR-0032 §1 / §9 wire subqueries to SQL_SELECT_COMPOUND, which omits the outer with_clause. WITH remains a statement-level-only construct. Documented in the relevant test. Test totals: 1333 → 1351 passing, 0 failed, 1 ignored (unchanged). Clippy clean. |
||
|
claude@clouddev1
|
4f89106a63 |
walker: Node::ScopedSubgrammar variant + scope-frame stack (ADR-0032 §10.2)
Sub-phase 2b checkpoint 1 — adds the foundation for SQL SELECT lexical-scope discipline without changing existing walker semantics. New types in `dsl::walker::context`: - `TableBinding` — one FROM-source binding with table name, optional alias, and schema-resolved columns (§10.1). - `CteBinding` + `CteColumn` — a CTE definition visible from inside its body (WITH RECURSIVE self-reference) and from the outer scope after harvest (§10.3). - `ScopeFrame` — `from_scope`, `cte_bindings`, and `projection_aliases` for one lexical scope. Default-empty; the fields will be populated by later 2b checkpoints. `WalkContext` gains `from_scope_stack: Vec<ScopeFrame>`, initialised with one bottom frame in both `new()` and `with_schema()`. The bottom frame is the implicit top-level scope DSL paths and top-level SQL statements operate in; `Node::ScopedSubgrammar` entries push and pop additional frames on top. `current_table` / `current_table_columns` remain as direct fields for this checkpoint — converting them to derived helpers is a later 2b step. New grammar-tree variant: - `Node::ScopedSubgrammar(&'static Self)` — like `Subgrammar`, but pushes a fresh `ScopeFrame` on entry and pops it on exit (ADR-0032 §10.2). Shares `subgrammar_depth` with the plain Subgrammar variant so the MAX_SUBGRAMMAR_DEPTH = 64 cap fires uniformly across both — §9's "no new walker capability for grammar recursion" claim holds. DSL Expr (ADR-0026) and sql_expr.rs ladder (ADR-0031) recursion continue to use the plain Subgrammar variant and never push a scope. Driver gains a parallel `walk_scoped_subgrammar` arm; the push/pop is unconditional so a speculatively-walked branch a later Choice rolls back leaves the stack clean. Test coverage in `driver.rs`: - A recursive ScopedSubgrammar test grammar walks correctly through depths 0-3. - The depth cap fires the same `expression_too_deep` friendly validation error as for plain Subgrammar. - The bottom frame invariant: `WalkContext::new` seeds exactly one frame, and after a walk the stack is restored. No grammar tree references the new variant yet — the rewire of sql_select.rs CTE bodies and the sql_expr.rs additive extensions for §5/§6 are the next 2b checkpoint. Test totals: 1330 baseline + 3 = 1333 passing, 0 failed, 1 ignored. Clippy clean. |
||
|
claude@clouddev1
|
8d293358a0 |
grammar: SQL SELECT full statement fragment (ADR-0032 Phase 2a)
Author the standalone walkable shape for the full standard-SQL
SELECT per ADR-0032 §1: compound queries with the four set ops
(UNION / UNION ALL / INTERSECT / EXCEPT), the five JOIN flavours
(INNER / LEFT [OUTER] / RIGHT [OUTER] / FULL [OUTER] / CROSS),
GROUP BY / HAVING, WITH and WITH RECURSIVE common table
expressions, LIMIT … OFFSET, DISTINCT / ALL, qualified-wildcard
`t.*` projection, and bare-alias projection (lifting ADR-0030
Phase-1 §4.2).
Recursion into SQL_SELECT_COMPOUND uses Node::Subgrammar for
2a; sub-phase 2b will rewire those references to the new
Node::ScopedSubgrammar variant for completion-scope discipline
(ADR-0032 §10.2). The Phase-1 data::SELECT CommandNode is not
touched here — the new fragment is reachable only from its own
tests until sub-phase 2c performs the migration.
Two implementation mechanisms realize ADR semantics without
changing them:
- Node::Lookahead disambiguates the projection_item Choice
(bare `*` vs `ident . *` qualified wildcard vs `sql_expr [
alias ]`) and gates bare-alias slots against continuation
keywords. The walker's walk_ident accepts any
identifier-shape token, including keyword-shape ones, and
Choice / Optional are first-match-wins; without lookahead a
bare-alias slot would greedily swallow FROM / WHERE / JOIN /
etc. Per-position follow-sets list which keywords legitimately
follow each alias slot. Same pattern as data.rs's
insert_first_paren precedent.
- INNER JOIN and bare JOIN are split into two distinct Choice
branches (each with a concrete leading keyword) rather than
sharing one Optional(Word("inner"))-leading branch. Avoids a
walker hazard where an Optional-leading-child Seq commits to
idx > 0 and then converts the next child's EOF NoMatch into
Incomplete, blocking the outer Choice from falling through to
later branches. Same semantic surface, distinct mechanism.
The §13 OOS shapes all have explicit reject tests (NATURAL,
USING, comma-FROM, LIMIT m,n, window OVER, VALUES, derived
tables). LATERAL has a noted partial limitation: the comma form
rejects via OOS-3, but the single-keyword form `FROM a LATERAL
JOIN b ON …` is admitted structurally because `lateral` parses
as a bare table-source alias for `a`. This matches ADR-0030's
"grammar admits identifier-shape tokens; engine resolves"
posture.
`__rdbms_*` rejection extends to every Phase-2 table-source
slot — the FROM table, each JOIN's table, each CTE name, and
the FROM inside any CTE body — via the reuseable
reject_internal_table validator.
70 new unit tests in sql_select.rs walk every §1 production and
every OOS reject case. Test totals: 1260 baseline + 70 = 1330
passing, 0 failing, 1 ignored (unchanged from baseline). Clippy
clean.
Per the Phase-2 plan sub-phase 2a exit gate. DA gate written
review: PASS.
|
||
|
claude@clouddev1
|
e032f01b2d |
docs: ADR-0032 Amendment 1 — empirical scope of column-origin metadata
§12 was written conservatively, classifying projection items structurally and listing "subquery expressions" alongside arithmetic / CASE as cases that stay None. The Phase-2 plan's Open Question 1 captured the matching uncertainty about CTEs and scalar subqueries. A throwaway probe against the pinned bundled SQLite + rusqlite 0.39.0 (with the `column_metadata` feature) settles the question across 20 representative query shapes. The engine's column_table_name / column_origin_name metadata follows through non-recursive CTEs (SELECT *, bare-ref, qualified-ref, and (col-list)-renamed bodies; CTE chains), scalar subqueries (aliased and unaliased), derived tables (out of scope per §13 OOS-1 but useful to note), all four set ops, multi-table JOIN projections, and IN-subquery WHERE clauses (the inner subquery does not affect the outer projection's origin). The structural-None classes reduce to computed projections (function calls, arithmetic, CASE, literals, wildcards — expected and pedagogically obvious) and recursive CTE result columns (the one structural surprise — the recursive temporary table has no base-column origin to point at). Amendment 1 supersedes §12's "Resolution rule" with a simpler engine-driven rule: trust column_table_name(i) / column_origin_name(i) verbatim, with no grammar-side structural classification. The speculative MatchedPath-walk fallback is moot. The Phase-2 plan's sub-phase 2f exit gate gains explicit positive assertions for CTE pass-through and scalar-subquery type recovery, and a new explicit negative assertion for the recursive-CTE limitation. README.md index entry extended in the same style as ADR-0027's Amendment-1 line. Closes Plan §Open-1. |