diff --git a/CLAUDE.md b/CLAUDE.md index 33aa4f1..bfd358d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -73,8 +73,11 @@ Current decisions at a glance (each backed by an ADR): - **Sharing:** `export` command produces a zip without the `.db`; no hosted publishing (ADR-0007). - **Testing:** four-tier strategy from `cargo test` units up to - PTY-based end-to-end (ADR-0008). Tiers 1–3 are active; Tier 4 - is wired only for the listed critical flows. + PTY-based end-to-end (ADR-0008). Tiers 1–3 are active; **Tier 4 + is not yet wired** — ADR-0008 specifies the PTY harness and the + four critical flows, but no PTY deps or tests exist yet + (verified 2026-06-07; corrects an earlier "wired only for the + listed critical flows" claim). Tracked as `requirements.md` TT4. - **DSL syntax conventions:** required clauses use keyword grammar (`with pk`, `to table` optional, `from..to`, `set`, `where`); `--` flags are reserved for opt-in choices; one @@ -163,6 +166,17 @@ Key invariants in the code: until it settles. The ADR-0000 index-upkeep rule applies: every ADR change updates `docs/adr/README.md` in the same edit. +- **Issue tracking.** Bugs and enhancements are filed as Gitea + issues (see *Issue tracking — Gitea via `tea`* below). + `docs/requirements.md` and the ADRs remain the source of truth + for **scope and decisions**; issues are the lightweight tracker + for **discrete work items**, cross-referenced from commits and + handoffs (e.g. `fix: … (#12)`). The project is near completion + of its initial requirements, so no heavyweight planning workflow + is run — the document-based requirements are augmented with + issue references as work proceeds. A change that touches a + *decided* area still earns an ADR; the issue references the ADR, + it does not replace it. - **Testing.** Per the user's global standards, tests are established before changes, bugs are reproduced with failing tests before being fixed, and "all green, no skips" is the @@ -187,6 +201,64 @@ Key invariants in the code: `git commit` is preceded by an explicit message proposal and user approval. No AI attribution in commit messages. +## Issue tracking — Gitea via `tea` + +Extends (does not replace) the generic Gitea/`tea` safety rules in +the global `CLAUDE.md`. Use `tea` to manage Gitea issues; `tea +--help`, `tea issues --help`, etc. for command reference. + +**Repo coordinates.** This repo lives on the self-hosted Gitea at +`git.lazyeval.net` as `oli/rdbms-playground`. `tea` **auto-detects +it correctly off the git remote** — verified — so plain `tea issues` +works here even though the machine's *default* `tea` login is a +different host (`git.oliversturm.com`). Pass `--login +git.lazyeval.net --repo oli/rdbms-playground` only as a fallback if +auto-detection ever slips. **Never** fall back to raw API calls +(`curl`/`fetch`) when `tea` misbehaves — tokens leak into shell +history; fix `tea` instead (usually `--login`/`--repo`). + +**Labels.** Preconfigured (`bug`, `enhancement` are in use). +**Ask the user before creating new labels.** Create with `tea +labels create --name --color --description `. + +### Critical gotchas + +- **`tea` blocks on stdin in a non-TTY → hangs.** `tea comment`, + `tea issue … --comments`, and similar **wait on stdin** when not + attached to a terminal, so they hang silently. **Always append + `< /dev/null`**, and wrap in `timeout 30` as a safety net: + `timeout 30 tea comment "$body" < /dev/null`. Verify the + write landed afterwards (re-fetch); don't trust a clean exit alone. +- **Multi-line comment / description bodies**: heredocs do **not** + work with `--description` / the comment-body arg. Write the + markdown to a temp file and pass it via shell substitution: `tea + comment "$(cat /tmp/body.md)" < /dev/null` (same for `tea + issues edit --description "$(cat …)"`). +- **Read an issue's RAW body** (for editing): the default/`--output + yaml` view is a lossy rendered box. Use JSON: `tea issue + --fields body --output json < /dev/null | jq -r '.body'`. **`tea + issues edit --description` replaces the WHOLE body** — splice + surgically and keep the raw backup before applying. +- **Reopen**: use `tea issues reopen `, NOT `tea issues edit + --state open`. +- **Milestones** (not currently used here, but if introduced): set + with `tea issues edit --milestone "" ` (empty string + clears it). **Options MUST precede the ``** — flag-after-index + silently no-ops. The `tea issues create --milestone …` flag is + **unreliable** — set the milestone with a follow-up `edit` and + verify. +- **Display blind-spot — don't loop on this.** `tea issue + --fields milestone` and `--fields comments` render `None`/`0` + **even when set** — they are NOT a source of truth. Confirm a + **milestone** via the filtered list (`tea issues list --milestones + "" --limit 100 | grep `; presence = set); confirm a + **posted comment** via `tea issue --comments` (NOT the + `comments` count field). Labels/state/title DO render correctly on + the single-issue fetch; only milestone + comments don't. +- **Pagination**: default ~50 issues. Use `--limit 100` (or more) + for full lists; `--state all` to include closed; `--output + tsv`/`json` for parseable output. + ## Build hygiene `target/` is git-ignored and 100% regenerable, but it grows diff --git a/Cargo.toml b/Cargo.toml index b11af84..f3b74d9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ version = "0.1.0" edition = "2024" description = "A cross-platform TUI playground for learning relational databases." license = "MIT OR Apache-2.0" -repository = "https://github.com/sturm/rdbms-playground" +repository = "https://git.lazyeval.net/oli/rdbms-playground" readme = "README.md" publish = false diff --git a/docs/adr/0001-language-and-tui-framework.md b/docs/adr/0001-language-and-tui-framework.md index 3832f08..69f66a9 100644 --- a/docs/adr/0001-language-and-tui-framework.md +++ b/docs/adr/0001-language-and-tui-framework.md @@ -45,3 +45,25 @@ package managers (`cargo binstall`, Homebrew, Scoop, `winget`). - TUI styling will require explicit work to match the polish that Bubble Tea / Lipgloss give for free; budget for it in the design pass. + +## Amendment 1 — Distribution channel is open after the Gitea migration (2026-06-09) + +The *Decision* block above assumed prebuilt binaries would ship "via +GitHub releases plus package managers." Since then the repository has +been migrated off GitHub to a self-hosted Gitea instance +(`git.lazyeval.net/oli/rdbms-playground`), and `tea` is the forge CLI +in use. The "GitHub releases" half of that sentence is therefore no +longer a settled assumption. + +This amendment does **not** pick a replacement. Binary distribution is +not built yet (no release pipeline, no CI — `requirements.md` TT5/E* +remain open), so the channel for prebuilt binaries is an **open +choice** — Gitea releases, a GitHub mirror's releases, or both — to be +settled by a dedicated ADR when distribution is actually implemented. +The package-manager channels named in the Decision (`cargo binstall`, +Homebrew, Scoop, `winget`) are independent of the forge and are +unaffected. + +(For the same supersede-don't-rewrite reason, the Decision block also +still names `sqlparser-rs`, which ADRs 0030–0036 replaced with a +hand-rolled grammar; that is recorded there, not by editing this ADR.) diff --git a/docs/adr/0020-tokenization-layer-for-the-dsl-parser.md b/docs/adr/0020-tokenization-layer-for-the-dsl-parser.md index fab1700..58595cf 100644 --- a/docs/adr/0020-tokenization-layer-for-the-dsl-parser.md +++ b/docs/adr/0020-tokenization-layer-for-the-dsl-parser.md @@ -2,7 +2,29 @@ ## Status -Accepted. +**Superseded by ADR-0024** (2026-05-14). Accepted then superseded +without being implemented. + +> **Superseding note (2026-06-03).** This ADR was never built. It +> specifies a `chumsky`-over-tokens architecture — a separate lexer +> producing `Vec`, a `define_keywords!` macro, and chumsky +> grammar combinators consuming `&[Token]`. ADR-0024 (unified grammar +> tree) instead adopted a **scannerless hand-rolled walker** that +> operates directly on source bytes, and **removed chumsky from the +> project entirely** (it is no longer a dependency). The lexer, +> `keyword.rs`, and the token model described below do not exist. +> +> What this ADR got *right* survives in ADR-0024: the +> expected-set aggregation it wanted (one branch's report no longer +> swallowing the others) is delivered by the walker's structural +> `expected` derivation, and the I3 (completion) / I4 (highlighting) +> hooks it anticipated are served by the same walker. Read ADR-0024 +> for the architecture as built; this ADR remains as institutional +> memory of the path not taken and the reasoning that led there. + +--- + +*Original status (historical):* Accepted. Amends ADR-0001 (language and TUI framework) by adding a tokenization layer between the source string and the chumsky diff --git a/docs/adr/0021-parser-as-source-of-truth-for-h1a.md b/docs/adr/0021-parser-as-source-of-truth-for-h1a.md index 244f7f4..ff1e4c7 100644 --- a/docs/adr/0021-parser-as-source-of-truth-for-h1a.md +++ b/docs/adr/0021-parser-as-source-of-truth-for-h1a.md @@ -2,7 +2,37 @@ ## Status -Accepted. +**Mechanism superseded by ADR-0024; H1a scope continued in ADR-0042.** +Accepted then superseded. + +> **Superseding note (2026-06-03).** The *intent* of this ADR — surface +> the grammar of the command at the point of error, not just the next +> token — survived and is largely delivered. The *mechanism* did not. +> This ADR specifies a `chumsky`-based design: a separate `UsageEntry` +> registry in `src/dsl/usage.rs`, `parse.token.*` catalog keys driven +> by chumsky's `RichPattern` expected sets, and a renderer over +> chumsky output. ADR-0024 (unified grammar tree) replaced chumsky with +> a scannerless walker and **folded usage info onto the grammar nodes +> themselves**: `usage_ids` live on each `CommandNode`, the per-command +> `parse.usage.*` templates and the `parse.available_commands` fallback +> ship as designed here, and the expected-set vocabulary +> (`format_expectation` in `parser.rs`) renders directly from walker +> `Expectation` variants — no `UsageEntry` registry, no `parse.token.*` +> keys, no `src/dsl/usage.rs`. +> +> So: the §1 usage registry, §3 "deepest consumed keyword" mechanism, +> §4 `parse.token.*` catalog, and §7 validator details below describe +> code that does not exist. What shipped equivalently: §1's per-command +> templates (as `usage_ids` + `parse.usage.*`), §2's three-block render +> (echo+caret / structural error / usage), and §5's available-commands +> fallback. **ADR-0042 picks up H1a from here** — it records what is +> actually shipped and defines the remaining systematic-pass scope +> against the grammar-tree architecture. Read ADR-0042 for the live +> plan; this ADR remains as the design rationale for the pedagogy goal. + +--- + +*Original status (historical):* Accepted. Builds on ADR-0020 (tokenization layer). Addresses H1a from `requirements.md` — the parse-error pedagogy gap that diff --git a/docs/adr/0042-h1a-parse-error-pedagogy-grammar-tree.md b/docs/adr/0042-h1a-parse-error-pedagogy-grammar-tree.md new file mode 100644 index 0000000..927f34e --- /dev/null +++ b/docs/adr/0042-h1a-parse-error-pedagogy-grammar-tree.md @@ -0,0 +1,385 @@ +# ADR-0042: H1a parse-error pedagogy in the grammar-tree era + +## Status + +**Accepted** — 2026-06-03. + +Continues H1a (`requirements.md`) from **ADR-0021**, whose +chumsky-based mechanism was superseded by **ADR-0024** (unified +grammar tree). ADR-0021's *intent* — surface the grammar of the +command at the point of error, not just the next token — is +re-stated here against the architecture as actually built, with +an inventory of what already ships and a definition of done for +the remaining work. + +Cross-references ADR-0019 (friendly-error layer + i18n catalog +conventions; H1a output shares the catalog), ADR-0022 (ambient +typing assistance, which shares the walker's expected-set +machinery), ADR-0024 (the grammar tree), and ADR-0009 (DSL +surface conventions; usage templates render in the documented +surface form). + +## Context + +### Why a new ADR rather than amending ADR-0021 + +ADR-0021 specifies a `UsageEntry` registry in `src/dsl/usage.rs`, +`parse.token.*` catalog keys, and a renderer over chumsky's +`RichPattern` expected sets. None of that exists. ADR-0024 +removed chumsky from the project, deleted `usage.rs`, and folded +usage information onto the grammar nodes themselves. Amending +ADR-0021 in place would force every reader to mentally translate a +dead mechanism; a fresh ADR records the live state directly. +ADR-0020 and ADR-0021 keep their superseding notes and remain as +institutional memory. + +### What H1a is + +When a learner types something near-correct, the error should +*name the missing keyword or clause* and *show the shape of the +command*, rather than point a caret at the unexpected character. +The user-reported gap: typing `create` once produced +`parse error: after \`create\`, expected \`table\`` — structurally +true, pedagogically silent. + +### What already ships (the baseline — do not re-build) + +Verified against code on 2026-06-03. The grammar-tree migration +delivered most of ADR-0021's intent through different machinery: + +1. **Per-command usage block.** Every `CommandNode` carries + `usage_ids: &'static [&'static str]` + (`src/dsl/grammar/mod.rs`). On any parse error the renderer + emits a `usage:` block listing every form of the matched + command family — 38 templates under `parse.usage.*` + (`src/friendly/strings/en-US.yaml:499-571`), resolved by + `grammar::usage_keys_for_input` and rendered by + `render_usage_block` (`src/app.rs:2560`). + +2. **Available-commands fallback.** When no command keyword was + consumed, the block becomes + `available commands: …` (`parse.available_commands`, + `en-US.yaml:493`; `app.rs:2593`). + +3. **Structural error names the consumed prefix and expected + set.** `format_walker_error` (`src/dsl/parser.rs:289`) renders + `after \`\`, expected , found `, distinguishing incomplete-at-EOF (`at_eof = true`, + more input would help) from a definite mid-input mismatch. + +4. **Friendly slot labels for identifiers.** `format_expectation` + (`src/dsl/parser.rs:262`) renders `Ident` slots by source — + "table name", "column name", "relationship name", "index + name", "type" — instead of a bare "identifier" (ADR-0022 stage + 8c). + +5. **Curated custom messages** for high-value near-misses under + `parse.custom.*` (`en-US.yaml:443-478`): `create_table_needs_pk`, + `insert_form_a_missing_values` ("looks like Form A — add + `values (...)`"), `change_column_flags_exclusive`, + `bind_type_mismatch`, the redundant-constraint and + alter-add-primary-key cases, etc. + +6. **Schema-aware pre-flight diagnostics** that light the + `[ERR]` validity indicator *at typing time* (ADR-0027 / + ADR-0033 / ADR-0036): INSERT arity for Forms A/B/C, unknown + table/column, type mismatch, `= NULL`, NOT-NULL-missing, and — + on the advanced-SQL surface — `cte_arity_mismatch`, + `compound_arity_mismatch`, and `projection_alias_misplaced` + (`diagnostic.*`, `en-US.yaml:577-620`; walker logic in + `src/dsl/walker/mod.rs`). + +7. **Ambient "Next:" hints** and the **simple→advanced cross-mode + pointer** (ADR-0022 / `advanced_alternative_note`, + `src/input_render.rs`). + +So H1a is *substantially* delivered at the intent level. The +handoff's two canonical examples already behave: `insert into T +('Oli')` → custom Form-A message; `update T set x=1` → structural +"expected `where` or `--all-rows`" + usage block. + +### What remains — the genuine gap + +The remaining work is **systematic verification plus targeted +polish**, not a missing feature: + +- **No enumerated coverage guarantee.** Coverage is curated + case-by-case; nothing asserts that *every required slot in every + command* produces a pedagogically-sound near-miss message. +- **Literal expectations render terse.** `Word`/`Literal`/`Punct`/ + `Flag` slots come out as backticked literals (`` `where` ``, + `` `=` ``, `` `--all-rows` ``). Correct, but a learner is helped + more by a short prose gloss in select high-value positions. +- **Advanced-mode SQL parse pedagogy is thinner** than the DSL + surface (RETURNING scope, CTE-arity diagnostic positioning, + `CROSS JOIN … ON`, INSERT…SELECT column-count). No other ADR or + open issue covers this (ADR-0019 §OOS-2 covers advanced-SQL + *engine-error sanitisation* — a different layer). + +## Decision + +### 1. Definition of done — a verified near-miss matrix + +H1a is "done" when there is a test matrix that, for **every +command in the REGISTRY**, exercises its salient near-miss inputs +and asserts the rendered output reads pedagogically. "Salient +near-misses" per command means at minimum: + +- the bare entry keyword alone (`create`, `add`, `update`); +- each required clause omitted (e.g. `update T set x=1` with no + filter rail; `insert into T (cols)` with no `values`); +- a wrong token where a specific slot is expected (e.g. a number + where a table name belongs); +- the zero-prefix / unknown-command case (available-commands + fallback). + +The matrix lives in the existing surfaces — `tests/typing_surface/` +(snapshot-based, the standalone `typing_surface_matrix` binary) for +the typing-time hint/validity view, and +`tests/it/parse_error_pedagogy.rs` (the consolidated `it` binary) +for the submit-time rendered three-block output. New integration +tests go in `tests/it/` per the handoff-57 §3 layout rule — **not** +as new top-level `tests/*.rs`. + +Work is **test-first**: add the matrix entry, observe the current +rendering, and only then adjust wording/labels where it reads +poorly. A near-miss whose current rendering is already good is +locked by a snapshot, not rewritten. + +### 2. Friendlier literal expectation labels + +`format_expectation` gains, for high-value keyword/punct positions, +an optional prose gloss while **always keeping the exact literal +visible** — a learner must still see the precise token to type. +The principle: a label may *add* role context, never *replace* the +literal. + +Illustrative target (final wording settled per-case against the +matrix, as is normal for pedagogical text): + +- `expected \`where\` or \`--all-rows\`` → + `expected a filter clause: \`where …\` or \`--all-rows\`` +- `expected \`values\`` (after a Form-A column list) → + already covered by `parse.custom.insert_form_a_missing_values`; + the matrix confirms it fires. + +Mechanism (illustrative, finalised at implementation time): a +grammar `Word`/`Punct` node may carry an optional expectation-label +key, mirroring how `Ident` slots derive a label from +`IdentSource`. Absent an override, rendering is unchanged (the +backticked literal). This keeps the change additive and per-slot — +no blanket reword that would churn the anchor-phrase tests +needlessly. + +New glosses are catalog-sourced (`parse.expect.*` or reuse of +`parse.usage.*` fragments — chosen at implementation time) so +wording stays in `en-US.yaml`, not in code, consistent with +ADR-0019. + +### 3. Advanced-mode SQL parse pedagogy — in scope + +The same matrix discipline (§1) extends to the advanced-mode SQL +surface. Two of the relevant arity diagnostics **already exist** and +must not be re-built — `cte_arity_mismatch` and +`compound_arity_mismatch` (`en-US.yaml:590-591`); for these the work +is matrix coverage and, for CTE, auditing whether the diagnostic is +*positioned at the CTE name* (easiest to fix) rather than the body. +The remaining items were re-checked empirically at implementation +time (2026-06-05) and **most turned out already handled** — see the +Implementation-outcome section's advanced-SQL paragraph for the +corrected picture. The `:` one-shot escape (a simple-mode line run +once in advanced mode) is part of the advanced surface and is +covered by the mode-aware usage threading (G3). + +This stays clear of ADR-0019 §OOS-2 (advanced-SQL *engine-error* +sanitisation): §OOS-2 reworks errors raised by *executing* SQL; +H1a here concerns errors raised while *parsing* it. If a near-miss +turns out to be an engine error rather than a parse error, it is +out of H1a scope and noted against §OOS-2 instead. + +### 4. Catalog and anchor-phrase discipline + +All new or reworded user-facing strings go through the i18n catalog +(`en-US.yaml`) and the `KEYS_AND_PLACEHOLDERS` validator, per +ADR-0019. No engine vocabulary in any string (CLAUDE.md). + +Two anchor styles constrain §2's glosses and both are preserved by +its "literal always visible" rule: + +- The **substring assertions** in `src/dsl/parser.rs` tests + ("after `…`", "expected table name", "found end of input", + "unknown type", "expected one of"). +- The **substring assertions** in `tests/it/parse_error_pedagogy.rs`, + which check for backticked literals and usage fragments + (e.g. `` `column` ``, `` `1` ``, "create table", "with pk"). This + test is `.contains()`-based, not snapshot-based, so a §2 gloss + that dropped the bare literal would fail it — which is precisely + the regression §2's rule prevents. + +The snapshot-based `tests/typing_surface/` matrix will re-baseline +on any §2 wording change (expected; reviewed via `cargo insta`), +but the two substring suites above must stay green without edits to +their assertions. + +## Implementation outcome (2026-06-05) + +The baseline capture (§Implementation notes step 1) triaged four +gaps; all four are fixed test-first, locked by the near-miss matrix +in `tests/it/parse_error_pedagogy.rs`: + +- **G1** — the bare `1` cardinality literal opening `add 1:n + relationship …` rendered cryptically. Render it as + `` `1:n relationship` `` in `format_expectation` (error wording + only; completion still offers the literal `1`). +- **G2** — bare `select` dumped the 14-item expression first-set. + Collapse it to "a projection: `*`, a column, or an expression" + in `format_walker_error`, detected by the `distinct`+`all` + quantifier pair being *jointly* expectable — a signature unique + to a projection start (empirically verified not to misfire at + `count(`, `union`, `union all`, `select distinct`, or mid-list). + Render-only; the completion/hint layer still expands the full + set. +- **G3** — the usage block was mode-blind (`render_usage_block` + resolved shared entry words to the first-registered Simple node). + `usage_key(s)_for_input` gain mode-aware `_in_mode` variants. + + **Decision (user-confirmed, after the DA pass).** In advanced + mode the DSL forms remain *valid input* via fallback — verified: + `create table Foo with pk`, `drop column from table T: c`, + `drop relationship r`, `add column …` all parse and dispatch in + advanced mode. So the advanced usage block shows **every form + valid in the mode, mode-primary (SQL) first, then the DSL + fallback forms** — a usage hint must never hide input that works. + (An initial implementation that showed SQL-only was flagged by + the DA pass as hiding `create table … with pk` / `drop column …` + and corrected.) Simple mode shows DSL forms only — the SQL-only + forms hit the "this is SQL" rail and are unreachable. + +- **G4** — `with` borrowed `select`'s usage; it gains its own + `parse.usage.with` CTE template. + +**Advanced-SQL pedagogy (§3) — empirical re-check (2026-06-05).** +§3 (drafted from a code survey) listed `RETURNING` scope, +`CROSS JOIN … ON`, and INSERT…SELECT column-count as absences. +Verifying each against the running app **reversed two of three**: + +- **INSERT…SELECT column-count** is *already handled* — a count + mismatch fires `verdict = Error` with "the column list names N + column(s) but M value(s) are given" (walker test + `insert_select_arity_mismatch_fires`). It is a structural + list-vs-list check, so it fires even without a schema. Not a gap. + *Caveat (pre-existing, not addressed here):* a `SELECT *` + projection is not expanded for arity, so `insert into T (one_col) + select * from Multi` is not pre-caught — the engine rejects it at + execution. Star-expansion for pre-flight arity would be a separate + enhancement (and brushes ADR-0019 §OOS-2 engine-error territory). +- **RETURNING scope** is *already handled* — at a bare `returning` + position completion offers the table's columns; `returning + ` fires the `unknown_column` diagnostic. Not a gap. +- **`CROSS JOIN … ON`** *was* a genuine residual: the grammar + rejects the `on` but the structural error said only "expected end + of input". **Fixed** — `parse.cross_join_no_on` renders "a CROSS + JOIN has no ON clause — …" when the failing token is `on` and the + most recent consumed join is a CROSS join (a precise signature: + every other join requires `on`, so there `on` is expected, not a + failure). Render-only, no grammar change; two misfire guards + (plain join still asks for `on`; a stray `on` with no join does + not fire). The CTE/compound arity diagnostics noted above remain + present and correct. + +**Known low-priority residual (user-confirmed to defer).** At +*submit* time, an incomplete expression position that is not a +SELECT projection (bare `where `, `returning `, `having `, `set +col=`) still renders the raw ~14-item expression first-set; only the +SELECT projection is glossed (G2, keyed on the `distinct`+`all` +quantifier pair). This is low-impact because *typing*-time +completion already offers the correct candidates (columns, +functions, expression keywords) at those positions. Generalising the +gloss was considered and deferred — the payoff is small and a +broader render-side collapse adds misfire surface. + +Coverage: the matrix covers, in both modes, every entry word's bare +/ missing-clause / wrong-token near-misses, the app-lifecycle +trailing-junk cases, **and** the committed *multi-form* variants +(`add index` / `add constraint` / `add 1:n relationship`, `drop +index` / `drop constraint` / `drop relationship`, `show table`, +`change column …`, `create index`, `alter table … add` / `… drop`). +The committed forms were audited 2026-06-05 and each renders its own +form-specific missing-keyword message + usage (e.g. `add index` → +"expected `on` or `as`"; `drop constraint` → "expected `not`, +`unique`, `default`, or `check`"), regression-locked in +`near_miss_matrix_committed_multiforms`. + +## Out of scope + +1. **Advanced-SQL engine-error sanitisation** — ADR-0019 §OOS-2. +2. **Tab completion (I3) and syntax highlighting (I4)** as + features — they share the walker but are separate ADRs. +3. **Schema-aware "did you mean `Customers`?" spell-correction** — + ADR-0021's out-of-scope §2; belongs with I3. +4. **Multi-error reporting.** The walker reports the first error + and stops; unchanged. +5. **`messages`-style verbosity gating of the usage block.** Per + ADR-0021 §8 the usage block is always shown; parse errors are + exactly when pedagogical surface should be maximal. Unchanged. +6. **Auto-generating usage/help text from the grammar.** ADR-0024 + left help prose hand-curated; templates stay hand-written. + +## Consequences + +### Positive + +- H1a gains an explicit, enumerated definition of done instead of + an open-ended "systematic pass still pending". +- The matrix becomes a regression lock: future grammar changes + that degrade a near-miss message fail a snapshot. +- Literal-label glosses close the last terse-wording gap without a + blanket reword. +- The advanced-SQL surface reaches parity with the DSL surface for + the audience that has switched to raw SQL. + +### Costs + +- Wording iteration across many near-miss cases — but cheap, + catalog-driven, and snapshot-guarded. +- The §2 per-node label field is one more annotation a new command + may set (optional; default unchanged). +- Snapshot volume grows; acceptable given the existing ~160-entry + typing-surface matrix. + +### Neutral + +- No public API change. `parse_command*` signatures, the + `ParseError` shape, and the three-block render path are all + unchanged; this ADR adds wording, labels, and tests within them. + +## Implementation notes + +Order of operations (test-first throughout): + +1. Enumerate the per-command near-miss matrix (§1) as failing/asserting + tests in `tests/typing_surface/` + `tests/it/parse_error_pedagogy.rs`. + Capture current rendering as the starting baseline. +2. Triage: which entries read poorly? Only those get wording work. +3. Add the optional expectation-label mechanism (§2) and apply it + to the high-value keyword/punct positions surfaced in triage. +4. Advanced-SQL near-miss audit + fixes (§3), distinguishing parse + from engine errors as they arise. +5. Catalog validator + anchor-phrase checks stay green (§4). +6. Update `requirements.md` H1a with the matrix as the done-marker; + flip to `[x]` only when the matrix is complete and green. + +## See also + +- ADR-0021 — Parser-as-source-of-truth for H1a (mechanism + superseded; intent continued here). +- ADR-0020 — Tokenization layer (superseded by the scannerless + walker). +- ADR-0024 — Unified grammar tree (the architecture H1a is built + on). +- ADR-0022 — Ambient typing assistance (shares the expected-set + machinery). +- ADR-0019 — Friendly-error layer and i18n catalog (§OOS-2 is the + adjacent engine-error scope). +- ADR-0009 — DSL command-syntax conventions (usage surface form). +- `requirements.md` — H1a tracking entry. diff --git a/docs/adr/0043-compound-pk-foreign-key-references.md b/docs/adr/0043-compound-pk-foreign-key-references.md new file mode 100644 index 0000000..55ea21d --- /dev/null +++ b/docs/adr/0043-compound-pk-foreign-key-references.md @@ -0,0 +1,339 @@ +# ADR-0043: Compound-primary-key foreign-key references (T3) + +## Status + +**Accepted + implemented** — 2026-06-09. Implementation landed the +same day: the relationship model went list-based through all six +layers (refactor commit `b14f019`, single-column preserved), then +the DSL + SQL grammars gained multi-column parsing and the +executor the full-PK/auto-expand/per-pair-type-compat/auto-name/ +`--create-fk`-per-column logic. Verified by 12 integration tests in +`tests/it/compound_fk.rs` (parse both surfaces, engine-enforced FK, +arity + partial-PK refusal, `--create-fk`, single-column +preserved) on top of the existing single-column relationship +suite. `requirements.md` **T3** is `[x]`. All four genuine forks +confirmed by the user at the recommended option: **F-A** full PK in order, **F-B** +house-style uniform column lists (no migration; back-compat not +required), **F-C** parenthesized DSL lists, **F-D** bare table-level +SQL FK auto-expands to the parent's full PK. Closes the one open +leg of +`requirements.md` **T3** ("compound primary keys handled +end-to-end (DSL, storage, display, **FK reference**)"): a foreign +key that *references* a compound (multi-column) primary key. + +Cross-references **ADR-0011** (FK column type compatibility — +`Type::fk_target_type`), **ADR-0013** (relationships, naming, the +rebuild-table strategy, and the `__rdbms_playground_relationships` +metadata table), **ADR-0035 §4b** (the SQL `FOREIGN KEY` surface), +**ADR-0004 / ADR-0015** (`project.yaml` as the authoritative +format; `playground.db` is a derived artifact), and **ADR-0009** +(DSL surface conventions). + +## Context + +Compound PRIMARY KEYs are declared, stored, and displayed today +(`create table T with pk a(int), b(int)` → `primary_key: +Vec`). The missing leg is the *reference*: a child table +whose foreign key points at a parent's compound PK. A 2026-06-09 +codebase audit found single-column FK is a pervasive assumption — +~15–20 sites across 6+ files: + +- **Metadata** — `__rdbms_playground_relationships` stores scalar + `parent_column TEXT` / `child_column TEXT` + (`PRIMARY KEY (child_table, child_column)`). +- **Persistence** — `RelationshipSchema { parent_column: String, + child_column: String }`; `project.yaml` `RawEndpoint { table, + column }`. +- **Grammar** — `add 1:n relationship … from

. to + .` (one ident per side); SQL `FOREIGN KEY () + REFERENCES

()` (parens that hold exactly one ident). +- **AST** — `Command::AddRelationship { parent_column: String, + child_column: String }`; `SqlForeignKey { child_column: String, + parent_column: Option }`. +- **Executor** — `schema_to_ddl` emits a single-column + `FOREIGN KEY (c) REFERENCES P(p)`; `check_fk_type_compat` + compares one parent type to one child type; bare + `REFERENCES

` on a compound-PK parent is refused as + ambiguous (`resolve_create_table_fks`, + `do_alter_add_foreign_key`). +- **Display** — `RelationshipEnd { other_column: String, + local_column: String }`. + +This is not a sweep-sized change, which is why it earns an ADR +rather than an inline build. The decisions below also turn the +audit's worst-case framing (a metadata-schema + yaml-format +migration via the F3 framework) into a **no-migration** change. + +### Why no migration is needed + +**Decision input (user, 2026-06-09): back-compatibility with +existing saved projects is not required.** The project is +pre-release; there is no installed base of `project.yaml` / +`playground.db` files to preserve. This removes the only force +that would have demanded an F3 migrator or a version bump, and — +more importantly — it lets the representation be chosen for +*cleanliness and consistency* rather than for byte-identical +back-compat. The consequence is explicit and accepted: a +`project.yaml` written before this change that contains +relationships will not load under the new format. + +Freed of back-compat, the storage follows the convention the file +**already uses** for ordered column lists rather than inventing a +new one: + +- `project.yaml` already writes `primary_key: [id]` (a compound PK + is `primary_key: [a, b]`) and index `columns: [a, b]` + (`RawIndex { columns: Vec }`). The relationship endpoint + is the lone multi-column-capable slot still using a scalar + `column:`. It joins the house style (D5). +- The metadata columns are `TEXT`; SQLite has no array type, so a + list lives in a text cell as JSON regardless. That JSON is now a + *uniform* encoding (a one-element array for the single-column + case), not a "bare-name-or-JSON, sniff which" fallback — the + fallback only existed to keep old rows identical, which is no + longer a goal. + +So this is not a clever back-compat dodge; it is "use the existing +list convention, uniformly." No version bump, no F3 migrator. + +## Decision + +Support a foreign key that references a parent's **full** compound +primary key, matched **positionally** to an equal-length child +column list, with per-pair type compatibility — across both the +DSL and SQL surfaces — using format-flexible storage that needs no +migration. + +### D1 — Matching policy: the full PK, in order + +A compound-PK FK references **all** columns of the parent's +primary key, in PK declaration order, matched 1:1 to the child's +column list (same length). Referencing a *subset* of a compound PK +is **out of scope**: SQL/SQLite require FK parent columns to form a +PK or UNIQUE key, and a strict subset of a compound PK is not +itself unique unless separately constrained. Teaching-clean rule: +*a foreign key to a compound key names every column of that key.* + +A length mismatch (child supplies N columns, parent PK has M ≠ N) +is a friendly error naming both counts. + +### D2 — Type compatibility: per pair, positional + +Each child column's type must satisfy +`parent_pk_col.fk_target_type() == child_col` for the +corresponding pair (the existing ADR-0011 rule, applied +element-wise in order). `check_fk_type_compat` generalises to walk +the pairs and report the **first** offending pair with the same +wording it uses today. + +### D3 — DSL syntax: parenthesized column lists + +`add 1:n relationship [as ] + from

.(, ) to .(, ) + [on delete …] [on update …] [--create-fk]` + +The single-column form `from

. to .` is unchanged +(no parens) — back-compatible and the common case. The +parenthesized list is the multi-column form. Both sides must use +the same arity (enforced as a D1 length check). Parentheses mirror +the existing compound-PK *declaration* syntax (`with pk a(int), +b(int)` uses parens around the per-column type; the FK list uses +parens around the column names) and the SQL `FOREIGN KEY (…)` +shape, so the surface stays internally consistent. + +### D4 — SQL syntax: extend the existing lists + +`FOREIGN KEY (, ) REFERENCES

(, )` — the grammar's +child and parent column slots become comma-separated **lists** +(today capped at one). Inline ` REFERENCES

(, +)` stays single-child-column (one inline column can't match a +2-column key) — a compound FK uses the table-level form. Bare +table-level `FOREIGN KEY (x, y) REFERENCES

` (no parent +columns) **auto-expands to the parent's full PK** when the arities +match; bare inline ` REFERENCES

` on a compound-PK parent +keeps today's friendly refusal, with the message pointing at the +table-level multi-column form. + +### D5 — Storage: uniform column lists, matching the house style + +Both stores hold an **ordered column list**, uniformly (a +one-element list for the single-column case), following the +convention `project.yaml` already uses for `primary_key` and index +`columns`. + +- **`project.yaml`**: `RawEndpoint` becomes `{ table, columns: + Vec }` and writes `columns: [a, b]` (single-column → + `columns: [id]`), exactly parallel to `primary_key: [id]`. No + scalar `column:` form, no dual-shape reader. +- **Metadata** (`__rdbms_playground_relationships`): no + `CREATE TABLE` change (the `TEXT` columns and + `PRIMARY KEY (child_table, child_column)` are untouched). + `parent_column` / `child_column` store the list **comma-joined** + in the same text cell (`a,b`; a single column is just its bare + name). *As-built note:* the ADR first said "JSON array"; the + implementation uses a comma delimiter, which is safe because + column identifiers are `[A-Za-z0-9_]+` (no commas — `parser.rs`) + and simpler (no `serde_json` dependency). This is an internal + encoding detail below fork F-B — the user-visible `project.yaml` + is still the `columns: [a, b]` list. + The actual enforced FK lives on the rebuilt child table's DDL + (`FOREIGN KEY (a, b) REFERENCES P(x, y)`), emitted by + `schema_to_ddl`, exactly as the single-column FK is today via the + rebuild-table primitive (ADR-0013) — one relationship, one undo + step. + +### D6 — In-memory model: `Vec` column lists + +`Command::AddRelationship`, `SqlForeignKey`, `RelationshipSchema`, +the internal `ReadForeignKey`, and `RelationshipEnd` (display) all +carry `parent_columns: Vec` / `child_columns: Vec` +(or `Option>` for the bare-SQL parent case). A +one-element vec is the single-column case; nothing about the +single-column UX changes. + +## Genuine forks (escalated for sign-off) + +These are decisions, not facts. Recommendations are marked; the +user confirms before this ADR moves to Accepted. + +- **F-A — matching policy.** Full PK only (D1, *recommended*) vs. + allow a subset (needs a separate UNIQUE key; larger, less + teaching-clean). +- **F-B — storage encoding.** Uniform column lists in the existing + house style — `columns: [a, b]` in yaml (like `primary_key`), + JSON-array in the unchanged metadata `TEXT` columns; no + back-compat, no migration (D5, *recommended*) vs. a normalized + relationship-columns child table (more "correct" but a schema + change with joins on read, no learner-visible payoff). Premise: + no existing projects to preserve (confirmed). +- **F-C — DSL multi-column syntax.** `from P.(a, b) to C.(x, y)` + parenthesized (D3, *recommended*) vs. a repeated-dotted form + (`from P.a, P.b to C.x, C.y`, more ambiguous to parse and read). +- **F-D — bare table-level SQL FK auto-expansion.** Auto-expand + `FOREIGN KEY (x,y) REFERENCES P` to P's full PK when arities + match (D4, *recommended*) vs. always require explicit parent + columns. + +## Implementation sketch (change sites) + +Grouped; each lands behind tests. No migration step. + +1. **AST** — `AddRelationship` + `SqlForeignKey` column fields → + `Vec` / `Option>` (`command.rs`). +2. **Grammar** — DSL endpoint column slot → optional + parenthesized list (`ddl.rs`); SQL child/parent column slots → + comma lists (`sql_create_table.rs`). Builders collect lists. +3. **Metadata** — `insert_relationship_metadata` / + `read_all_relationships` encode/decode bare-or-JSON + (`db.rs`); no `CREATE TABLE` change. +4. **Persistence** — `RelationshipSchema` → `Vec`; + `RawEndpoint` becomes `{ table, columns: Vec }`, written + `columns: [a, b]` like `primary_key` + (`persistence/mod.rs`, `persistence/yaml.rs`). +5. **Executor** — `do_add_relationship` / + `resolve_create_table_fks` / `do_alter_add_foreign_key` walk + column lists; `schema_to_ddl` emits multi-column `FOREIGN KEY + (…) REFERENCES P(…)`; `check_fk_type_compat` loops pairs; + bare-reference paths auto-expand to the full PK (D4) or refuse + with the improved message; the default relationship-name + generator (`db.rs:6850`) joins the column lists; `--create-fk` + creates one child column per parent PK column (`db.rs`). +6. **Display** — `RelationshipEnd` → column lists; `describe` + renders `(a, b) → (x, y)` symmetrically (outbound + inbound, + ADR-0013) (`db.rs`, `output_render.rs`). +7. **Teaching echo (ADR-0038)** — `render_add_relationship` and + `render_add_relationship_create_fk` (`echo.rs`) go multi-column: + the FK line emits `FOREIGN KEY (a, b) REFERENCES P (x, y)`, and + `--create-fk` emits **one `ADD COLUMN` line per newly-created + child column** (each typed to the matching parent PK column's + `fk_target_type`) before the FK line. Copy-paste contract + (ADR-0038) holds: every echoed line is runnable advanced SQL. +8. **Tests** — parse (DSL + SQL: single-col still works; multi + parses; arity mismatch errors; empty `()` rejected; inline + `col REFERENCES P(a,b)` rejected with the table-level pointer); + worker round-trip (declare a 2-col FK, rebuild, the FK is + **enforced** — an insert violating it is refused; per-pair + type-mismatch refused; bare-FK **auto-expand** to the parent PK; + `--create-fk` creates both child columns); persistence + round-trip (a single-col relationship writes `columns: [id]` and + reads back; a 2-col writes `columns: [a, b]` and reads back; + full save→rebuild reconstructs the FK); **undo** (add a 2-col + relationship, undo, it is gone — one step); display + (`describe` shows `(a, b) → (x, y)` both directions). + +## Implementation-readiness notes (DA pass, 2026-06-09) + +Verified against the code before build; folded in so the plan is +complete. + +- **SQLite precondition holds.** A FK's parent columns must be a + PK or a UNIQUE-indexed set. A SQLite `PRIMARY KEY (a, b)` creates + the requisite unique index, so `FOREIGN KEY (x, y) REFERENCES + P(a, b)` is valid against a compound PK with no extra index. + STRICT tables do not change FK rules. (F-A's "full PK" therefore + always targets a valid key; a subset would not be unique — the + reason F-A excludes it.) +- **Explicit parent columns must be exactly the PK set.** Under + F-A, `REFERENCES P()` is accepted iff `` is the + parent's PK column **set**; any ordering is accepted and maps + positionally to the child list (SQLite matches the set to the + unique index; the child↔parent pairing is positional). A + non-PK, partial, or super-set list is refused with a friendly + message naming the parent's actual PK (subset/UNIQUE targets are + OOS). +- **Arity + emptiness.** Child and parent lists must be equal, + non-zero length; a mismatch reports both counts + ("N child column(s) but M in `P`'s key"). An empty `()` list is + a parse error. Inline single-column `col REFERENCES P(a, b)` is + refused (one inline column can't satisfy a 2-column key) with a + pointer to the table-level `FOREIGN KEY (…)` form (D4). +- **DSL `from P.(a)` (single in parens)** is accepted — equivalent + to bare `from P.a` — so the parenthesized form is uniform across + arities; the bare form stays the idiomatic single-column + spelling. +- **`--create-fk` is per-column.** When child columns are missing, + one is created per parent PK column, each typed to that parent + column's `fk_target_type` (ADR-0011) — generalising today's + single-column behaviour; the echo mirrors this (sketch step 7). +- **Metadata identity unchanged.** `PRIMARY KEY (child_table, + child_column)` still holds with the JSON-array string as the + key — so a child column **set** still participates in at most one + relationship (pre-existing behaviour, now per-set). Distinct + sets on the same child table are distinct keys. +- **Auto-name generation** (`db.rs:6850`, the `[as ]`-less + default) is single-column today + (`{parent_table}_{parent_column}_to_{child_table}_{child_column}`) + — it must join the column lists (e.g. + `Orders_a_b_to_Customers_x_y`). A found change site the first + sketch missed; added to the executor step. +- **Undo / batch unchanged.** One `add 1:n relationship` is one + rebuild = one undo step (ADR-0013/0006), independent of arity. + +## Consequences + +- T3 closes; a learner can model a real composite-key relationship + end to end. +- No migration, and the on-disk representation gets *more* + consistent: the relationship endpoint joins the `primary_key: + [...]` / index `columns: [...]` list convention. The in-app + single-column UX is untouched (one-element vecs). +- Accepted trade-off (user, 2026-06-09): a `project.yaml` written + before this change that contains relationships will not load + under the new format. There is no installed base to preserve, so + this is a clean cutover, not data loss. +- The relationship model becomes list-based throughout, which is + the natural foundation if subset/UNIQUE-targeted FKs are ever + wanted (explicitly OOS here). +- A modest, broad refactor (the `Vec` field change ripples through + the 6 layers) — methodical, not deep; locked by tests at each + layer. + +## Out of scope + +- Subset/non-PK FK targets (referencing a UNIQUE key that isn't + the PK) — possible later on this list-based foundation. +- Any change to single-column behaviour, the rebuild-table + primitive, or the undo model (one relationship = one undo step + stands). +- A `project.yaml` version bump or F3 migrator (not needed — + no installed base to migrate; clean cutover per D5). diff --git a/docs/adr/0042-public-website-and-documentation-site.md b/docs/adr/0044-public-website-and-documentation-site.md similarity index 94% rename from docs/adr/0042-public-website-and-documentation-site.md rename to docs/adr/0044-public-website-and-documentation-site.md index 29fd71a..49dd24d 100644 --- a/docs/adr/0042-public-website-and-documentation-site.md +++ b/docs/adr/0044-public-website-and-documentation-site.md @@ -1,9 +1,14 @@ -# ADR-0042: Public website and documentation site +# ADR-0044: Public website and documentation site ## Status Accepted (2026-06-04). Implementation plan: -[`docs/plans/20260604-adr-0042-website.md`](../plans/20260604-adr-0042-website.md). +[`docs/plans/20260604-adr-0044-website.md`](../plans/20260604-adr-0044-website.md). + +> Renumbered from ADR-0042 to ADR-0044 when the `website` branch merged +> `main` (2026-06-09): `main` had independently used 0042 for the H1a +> parse-error ADR and 0043 for compound-PK FK references. Content is +> unchanged from the original draft. ## Context diff --git a/docs/adr/README.md b/docs/adr/README.md index a86de90..5ebab87 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -6,7 +6,7 @@ This directory contains the project's ADRs, recorded per ## Index - [ADR-0000 — Record architecture decisions](0000-record-architecture-decisions.md) -- [ADR-0001 — Language and TUI framework](0001-language-and-tui-framework.md) +- [ADR-0001 — Language and TUI framework](0001-language-and-tui-framework.md) — **Amendment 1 (2026-06-09)**: after the GitHub→Gitea migration (`git.lazyeval.net`), the prebuilt-binary distribution channel named in the Decision ("GitHub releases") is reopened as an undecided choice, to be settled by a future distribution ADR; package-manager channels unaffected - [ADR-0002 — Database engine](0002-database-engine.md) - [ADR-0003 — Input modes and command dispatch](0003-input-modes-and-command-dispatch.md) — the persistent `Simple`/`Advanced` mode and the `:` one-shot escape. The **startup mode is no longer always `simple`**: it is restored from the project's stored mode and overridable with `--mode` (see **ADR-0015 Amendment 1**, issue #14). The app-command registry gains **`copy`** (ADR-0041, issue #11) - [ADR-0004 — Project file format](0004-project-file-format.md) @@ -25,8 +25,8 @@ This directory contains the project's ADRs, recorded per - [ADR-0017 — Column type-change compatibility](0017-column-type-change-compatibility.md) - [ADR-0018 — Auto-fill contracts for `serial` and `shortid` columns](0018-auto-fill-contracts-for-serial-and-shortid.md) - [ADR-0019 — Friendly error layer (H1) and i18n message catalog](0019-friendly-error-layer-and-i18n.md) -- [ADR-0020 — Tokenization layer for the DSL parser](0020-tokenization-layer-for-the-dsl-parser.md) -- [ADR-0021 — Parser-as-source-of-truth for H1a (per-command usage in parse errors)](0021-parser-as-source-of-truth-for-h1a.md) +- [ADR-0020 — Tokenization layer for the DSL parser](0020-tokenization-layer-for-the-dsl-parser.md) — **Superseded by ADR-0024 (never implemented).** Specified a `chumsky`-over-tokens architecture (separate lexer, `define_keywords!`, `&[Token]` grammar). ADR-0024 adopted a scannerless hand-rolled walker and removed `chumsky` entirely; the lexer/keyword/token model here does not exist. Kept as institutional memory of the path not taken. +- [ADR-0021 — Parser-as-source-of-truth for H1a (per-command usage in parse errors)](0021-parser-as-source-of-truth-for-h1a.md) — **Mechanism superseded by ADR-0024; H1a scope continued in ADR-0042.** The *intent* (show the command's grammar at the point of error) shipped — `usage_ids` on each `CommandNode`, the `parse.usage.*` templates, and the `available_commands` fallback all exist — but via grammar nodes, not the `chumsky` `UsageEntry` registry / `parse.token.*` keys described here (which were never built). - [ADR-0022 — Ambient typing assistance: colour, hint panel, completion (I3 + I4)](0022-ambient-typing-assistance.md) — **Amendment 1 supersedes §12's simple-mode-only carve-out**: the unified mode-aware walker (ADR-0030/0031/0032) now speaks SQL, so advanced-mode ambient assistance is re-enabled. `ambient_hint_in_mode` + `hint_resolution_at_input_in_mode` + `expected_for_hint_snapshot` thread `Mode`; `render_hint_panel` calls ambient for all modes (no more advanced-mode `None`); the one-shot `:` sigil is stripped before the ambient walk. Fixes a live bug where advanced-mode SQL hinting/completion-preview were dead despite Phase 2 marking them green (validated at the engine layer, not the UI). Simple-mode gating, highlighting, and the §13 performance posture are unchanged; covered by an app-level render test plus ambient-layer regression locks; **Amendment 2 reverses the handoff-14 keywords-first candidate ordering** — schema identifiers (table/column/relationship names) now sort *before* keywords so a name the user would have to look up stays visible in the single-row, window-scrolled candidate line (keywords are learned over time; the `tok_identifier`/`tok_keyword` colour split marks the boundary); shipped with a `walk_repeated` fix that surfaces a list item's trailing optionals at a clean boundary (`order by Name ` → `asc`/`desc`, `select Name ` → `as`, `create table … Code(text) ` → `not`/`unique`/`default`/`check`; the `,` separator deliberately not surfaced); records a deferred two-line hint box for growing lists; **Amendment 3 makes the ambient-hint fallback rung schema-aware** — Amendment 1's bottom-rung `parse_command_in_mode` was schemaless while every earlier rung was not, so between-values insert hints pointed at `)` (type-blind close) instead of `,` and wrong-arity closed tuples read "submit with Enter" for an input the schema-aware parse rejects (issue #2); now uses `parse_command_with_schema_in_mode`, no extra walk, with the friendly arity diagnostic still winning at its higher rung; **Amendment 4 gives column types a dedicated highlight class** — both `Node::Ident.highlight_override` *and* the `Word.highlight_override` field were dead (driver destructured the former to `_`, `walk_word` hardcoded `Keyword`); now both wired through, with a new `HighlightClass::Type` + eighth `Theme` field `tok_type` (a pink/deep-magenta distinct from both keyword purple and identifier teal) so types no longer render identically to identifiers (issue #8); the three `IdentSource::Types` slots opt in via `Some(Type)` (advanced-mode single-word SQL aliases — `float`, `varchar`, … per ADR-0035 §3 — ride along for free), and the two-word `double precision` alias opts in via the new `Word::type_keyword` constructor so it matches its synonyms; **Amendment 5 lets the hint panel grow for long prose hints** — a fixed one-row panel clipped long field-value/usage hints past the first line (issue #12); `resolve_hint_lines` now pre-wraps prose and `render_right_column` sizes the panel to the line count (1 row default, up to `MAX_HINT_ROWS`=3, reclaimed when short) with a `clamp_wrapped` ellipsis backstop; the candidate list still scrolls horizontally on one row (Amendment 2's deferred two-line candidate box stays deferred); also shortens the 299-char `parse.usage.sql_create_table` synopsis to a terse one-liner (full grammar remains in `help.ddl.sql_create_table`); **Amendment 6 adds a curated SQL function-name list** (`src/dsl/sql_functions.rs`, `KNOWN_SQL_FUNCTIONS` — aggregates + common + broader scalars; `cast` deliberately excluded as its `CAST(x AS type)` syntax isn't a plain-call shape) as the single source of truth shared by two consumers at the `sql_expr_ident` slot (ADR-0031 §1): **issue #15** offers the functions as Tab candidates under a new `CandidateKind::Function` + ninth `Theme` colour `tok_function` (a blue distinct from keyword/identifier/type, parallel to Amendment 4's `tok_type`) so a learner discovers `sum`/`upper`/…; **issue #16** restores the typing-time column-typo flag the issue-#6 fix had dropped wholesale at this slot — `invalid_ident_at_cursor` now bails only when the partial prefix-matches a known function, else falls through to the schema-column check, so `select Agx` warns again at typing time while `select sum` does not (the issue-#6 lockdown tests + the submit-time `unknown_column` diagnostic path are untouched, and the no-validation-allowlist posture stands); see ADR-0031's status note for the grammar-side anchor - [ADR-0023 — Unified declarative grammar tree](0023-unified-grammar-tree.md) — direction (superseded for execution detail by ADR-0024) - [ADR-0024 — Unified grammar tree: execution plan](0024-unified-grammar-tree-execution-plan.md) — **Accepted**, the executable spec — implemented (Phases A–F; Phase F shipped "minimal", `parser.rs` retained as the router — see the ADR's Phase F implementation note) @@ -47,4 +47,6 @@ This directory contains the project's ADRs, recorded per - [ADR-0039 — EXPLAIN over advanced-mode SQL queries](0039-explain-over-advanced-sql.md) — **Accepted** (2026-05-27), **implemented 2026-05-30 (issue #7)**, **supersedes ADR-0030 §13 OOS-2**. Lets `explain` wrap the advanced SQL commands (`Select`/`SqlInsert`/`SqlUpdate`/`SqlDelete`, plus `with`/CTE which builds a `Select`) in addition to the DSL `ShowData`/`Update`/`Delete` it already covers (ADR-0028), running `EXPLAIN QUERY PLAN` over the validated SQL text through the existing ADR-0028 span-styled plan tree (advanced mode only; DSL `explain` unchanged in both modes). Implemented via a second `Advanced` `explain` CommandNode (`EXPLAIN_SQL`) registered under the shared `explain` entry word — reusing the established `insert`/`update`/`delete` shared-word dispatch (`decide`: SQL-first / DSL-fallback), so `explain show data …` and DSL-only `--all-rows` still reach the DSL node; rejected a `DynamicSubgrammar` mode-gate (its resolution cache key omits `mode`). `build_explain_sql` slices the inner SQL off the source (excludes `explain`) and reuses the existing SQL builders; `do_explain_plan` runs the carried text verbatim, no params. Advanced `explain update`/`delete` now route through SQL (identical plan, full SQL syntax); DSL-explain tests pinned to simple mode. Reframed OOS-2 as a *deferred* exclusion (per ADR-0000's out-of-scope discipline), not a rejection. OOS (deferred): EXPLAIN of DDL (no query plan exists) - [ADR-0040 — A per-command completion marker (✓/✗) replaces the `[ok]` summary line](0040-completion-marker-replaces-ok-summary.md) — **Accepted 2026-05-30 (issue #9)**, amends ADR-0014 / ADR-0028 / ADR-0019 output conventions, builds on ADR-0037's mode-tagged echo. An audit of the whole command surface found the `[ok] ` summary line duplicates the echo line above it (verb+subject) everywhere; its only unique contribution is the success-vs-error signal (and `explain select` even rendered `[ok] explain` with an empty subject post-ADR-0039). Decision: drop the `[ok]` line and the symmetric `"…" failed:` prefix; the echo line gains a trailing inline **✓** (green, success) / **✗** (red, failure) — `running:` becomes a pending state that resolves to ` ✓/✗` on completion (status set via the existing `rfind(Echo)` lookup). Content (row counts, structure, data, plan tree, teaching echo) unchanged. Scoped to the DSL/data/SQL family that has the redundant echo+`[ok]` pair; app-command `[ok]` lines (`rebuild`/`export`/`now editing`) are payload-bearing, have no echo to mark, and stay as-is. `ok.summary` retired; `dsl.failed` reduced to the rendered reason. Broad but mechanical snapshot churn. OOS: app-command `[ok]` lines, the `[WRN]` validity indicator, and the tag colours (issue #10) - [ADR-0041 — Copy the output panel to the system clipboard](0041-copy-output-to-clipboard.md) — **Accepted 2026-06-02 (issue #11)**, amends ADR-0003's app-command registry (adds **`copy`** / `copy all` / `copy last`). The friction it removes: filing a bug report meant terminal-selecting the output panel and fighting wrapping/borders. New **app-level command** (sigil-free, both modes): `copy` / `copy all` copy the whole panel; `copy last` copies from the most recent echo line to the end. **Mechanism — OSC 52 *and* native (`arboard`), always both**, because OSC 52 acceptance is undetectable (no terminal ack), so a true "fall back when unsupported" can't be built: emit the OSC 52 escape (no new dep — `base64`+`crossterm`; works over SSH; tmux-passthrough-wrapped via `$TMUX`), then a best-effort native write whose failure is ignored (headless host — OSC 52 carried it); the two carry identical content. **Format — plain text verbatim as rendered** (tags, `✓`/`✗`, box-drawing) joined by `\n`, without viewport padding/wrapping; a drift-lock test pins `OutputLine::plain_text` to `render_output_line`. `arboard` added **`--no-default-features`** (drops the `image` crate; X11-only on Linux — `wayland-data-control` deliberately omitted as it ~doubles the dep tree and OSC 52 covers native-Wayland). Security: write-only, scans clean for arboard's tree (cargo audit / osv-scanner / grype), 1Password-maintained, minimal surface. OOS: Markdown export, selection/range, a keybinding, OSC 52 read, `screen` passthrough -- [ADR-0042 — Public website and documentation site](0042-public-website-and-documentation-site.md) — **Accepted 2026-06-04**. The first public website: a marketing landing page plus the **canonical** user docs. Stack **Astro 6 + Starlight + Tailwind v4** (chosen over SvelteKit + Tailwind for a docs-heavy + marketing site; interactive bits as Astro islands). Showcase demos are **asciinema** `.cast` recordings (scripted-input driver for paced, re-recordable sessions — *not* `history.log` replay), reused inline in docs. The **in-page WASM playground is deferred** (OOS: deferred) behind a stable `Demo` seam, with the portable-core (`dsl`/`app`/`ui`, in-memory `rusqlite` via `ffi-sqlite-wasm-rs`) vs native-edge (Tokio/worker-thread/`crossterm`/persistence/backup-API) boundary recorded for a future ADR + iteration plan. Portable **static build** (Vercel target, but host-agnostic); **no CI yet**; **monorepo** (`website/`). Docs cover the **full supported feature set** with "planned" callouts for the unshipped minority; two wording rules bind user-facing copy — **no engine name** (continues ADR-0002) and **no "DSL"** ("simple mode" / "advanced mode"). Install docs cover **prebuilt binaries + package managers** (D1–D3 track the release tooling). Plan: `docs/plans/20260604-adr-0042-website.md` +- [ADR-0042 — H1a parse-error pedagogy in the grammar-tree era](0042-h1a-parse-error-pedagogy-grammar-tree.md) — **Accepted 2026-06-03.** Continues **H1a** from ADR-0021 against the ADR-0024 grammar tree (ADR-0021's chumsky mechanism is dead). Records the **baseline already shipped** — per-command `usage:` block (38 `parse.usage.*` templates), available-commands fallback, structural "after `…`, expected …" wording, source-derived ident slot labels ("table name"/"column name"), curated `parse.custom.*` near-miss messages, and the ADR-0027/0033/0036 schema-aware `[ERR]` diagnostics — so H1a is *substantially* delivered at the intent level. Defines the remaining work as **(1)** a verified per-command **near-miss matrix** (`tests/typing_surface/` + `tests/it/parse_error_pedagogy.rs`) as the definition of done, test-first; **(2)** **friendlier literal expectation labels** — optional prose glosses on `Word`/`Punct`/`Flag` positions that *add* role context while always keeping the exact literal visible (e.g. "a filter clause: `where …` or `--all-rows`"); **(3)** **advanced-mode SQL** near-miss parity (RETURNING scope, CTE-arity positioning, `CROSS JOIN … ON`, INSERT…SELECT count) — **in scope**, kept distinct from ADR-0019 §OOS-2 which covers advanced-SQL *engine*-error sanitisation, a different layer. Catalog/anchor-phrase discipline (ADR-0019) preserved; no public API change. OOS: I3/I4, spell-correction, multi-error reporting, verbosity-gating the usage block +- [ADR-0043 — Compound-primary-key foreign-key references (T3)](0043-compound-pk-foreign-key-references.md) — **Accepted + implemented 2026-06-09** (all four forks confirmed at the recommended option: full-PK matching, house-style uniform lists, parenthesized DSL syntax, bare-SQL-FK auto-expansion). Closes `requirements.md` **T3** `[x]` — the relationship model went list-based across six layers (single-column preserved, no migration), DSL `from P.(a,b) to C.(x,y)` + SQL `FOREIGN KEY (a,b) REFERENCES P(x,y)` parse/execute/enforce, 12 tests in `tests/it/compound_fk.rs`. Closes the open leg of `requirements.md` **T3**: a foreign key that *references* a parent's compound primary key. A 2026-06-09 audit found single-column FK woven through ~15–20 sites (metadata table, `RelationshipSchema`, `project.yaml` `RawEndpoint`, both grammar surfaces, executor FK-DDL emission, per-column type-compat, display) — earns an ADR, not an inline build. **Decision:** reference the parent's **full** compound PK, matched **positionally** to an equal-length child column list, per-pair `fk_target_type` compat (ADR-0011, element-wise); DSL `from

.(a, b) to .(x, y)` (single form unchanged), SQL `FOREIGN KEY (x, y) REFERENCES P(a, b)` (extend the existing one-cap lists; bare table-level FK auto-expands to the parent PK when arities match). **Storage — no migration (back-compat not required, user-confirmed 2026-06-09; no installed base):** the relationship endpoint joins the list convention `project.yaml` *already* uses — `columns: [a, b]` like `primary_key: [id]` and index `columns: [...]` (the endpoint was the lone scalar `column:` holdout); the metadata `TEXT` columns are unchanged and store the list **comma-joined** (`a,b`; the bare name for single — safe because identifiers are `[A-Za-z0-9_]+`). No F3 migrator, no version bump; accepted trade-off is that a pre-change `project.yaml` with relationships won't load (clean cutover). In-memory model goes list-based (`Vec`) through all six layers; the enforced FK is the rebuilt child-table DDL (`FOREIGN KEY (a,b) REFERENCES P(x,y)`), one relationship = one undo step (ADR-0013). Genuine forks escalated: matching policy (full-PK vs subset), storage (house-style uniform lists vs normalized table), DSL syntax (parenthesized vs repeated-dotted), bare-SQL-FK auto-expansion. OOS: subset/non-PK (UNIQUE-targeted) FK references; any single-column behaviour change +- [ADR-0044 — Public website and documentation site](0044-public-website-and-documentation-site.md) — **Accepted 2026-06-04** (originally drafted as ADR-0042 on the `website` branch; renumbered on merge to avoid colliding with the H1a ADR-0042). The first public website: a marketing landing page plus the **canonical** user docs. Stack **Astro 6 + Starlight + Tailwind v4** (chosen over SvelteKit + Tailwind for a docs-heavy + marketing site; interactive bits as Astro islands). Showcase demos are **asciinema** `.cast` recordings (scripted-input driver for paced, re-recordable sessions — *not* `history.log` replay), reused inline in docs. The **in-page WASM playground is deferred** (OOS: deferred) behind a stable `Demo` seam, with the portable-core (`dsl`/`app`/`ui`, in-memory `rusqlite` via `ffi-sqlite-wasm-rs`) vs native-edge (Tokio/worker-thread/`crossterm`/persistence/backup-API) boundary recorded for a future ADR + iteration plan. Portable **static build** (Vercel target, but host-agnostic); **no CI yet**; **monorepo** (`website/`). Docs cover the **full supported feature set** with "planned" callouts for the unshipped minority; two wording rules bind user-facing copy — **no engine name** (continues ADR-0002) and **no "DSL"** ("simple mode" / "advanced mode"). Install docs cover **prebuilt binaries + package managers** (D1–D3 track the release tooling). Plan: `docs/plans/20260604-adr-0044-website.md` diff --git a/docs/handoff/20260606-handoff-58.md b/docs/handoff/20260606-handoff-58.md new file mode 100644 index 0000000..d8ecd08 --- /dev/null +++ b/docs/handoff/20260606-handoff-58.md @@ -0,0 +1,185 @@ +# Session handoff — 2026-06-06 (58) + +Fifty-eighth handover. Continues from handoff-57, whose **next job +was H1a**. This session did exactly that, end to end: **H1a +(strong syntax-help in parse errors) is now done and marked `[x]`** +in `requirements.md`, via a new ADR-0042 systematic pass. The arc +was audit → ADR → test-first matrix → four gap fixes → two +adversarial review passes that each caught a real defect. See §2. + +## §1. State at handoff + +**Branch:** `main`. **HEAD `c305dc7`.** **origin/main at `10f8c2a`** +(pushed mid-session) → **4 commits unpushed** (`649fdcb`, `1d4923b`, +`d6e229f`, `c305dc7`). Push is the user's step. + +**Tests: 2158 passing / 0 failing / 1 ignored** (lib 1578, it 388, +typing_surface_matrix 192; the 1 ignored is the pre-existing one). +**Clippy clean** (nursery, all targets, `-D warnings`). + +This session's commits (since handoff-57's `a8d0138`): + +``` +c305dc7 docs: mark H1a done via the ADR-0042 systematic pass +d6e229f feat: H1a CROSS JOIN ON teaching message; advanced-SQL gaps re-verified (ADR-0042) +1d4923b fix: H1a G3 advanced usage shows all valid forms; complete near-miss matrix (ADR-0042) +649fdcb feat: H1a parse-error gaps G2–G4 + advanced near-miss matrix (ADR-0042) +10f8c2a test: H1a near-miss matrix + friendlier `add 1:n relationship` label (ADR-0042) +0e6f767 docs: ADR-0042 — continue H1a parse-error pedagogy on the grammar tree +``` + +## §2. What happened this stretch — H1a, start to finish + +### Audit first (the big correction) + +The handoff-57 §6 pointer said "read ADR-0021 + ADR-0020". **Both +are obsolete.** They specify a `chumsky`-over-tokens mechanism +(`UsageEntry` registry, `parse.token.*` keys, a lexer) that +**ADR-0024 deleted** — chumsky is not a dependency; the parser is +the scannerless unified grammar-tree walker. ADR-0020/0021 are now +marked **Superseded** (status notes + README), kept as memory. +ADR-0021's *intent* survived and was already ~80% shipped via the +grammar tree. **Read `docs/adr/0042-h1a-parse-error-pedagogy-grammar-tree.md`, +not 0020/0021, for the live H1a design.** + +### ADR-0042 + the user's three scope decisions + +Wrote **ADR-0042** (continues H1a against the grammar tree). Three +forks escalated and decided by the user: (1) ADR hygiene = +superseded-notes + new ADR; (2) scope = matrix-verify + friendlier +literal labels; (3) advanced-SQL **in scope**. + +### The near-miss matrix (the definition of done) + +`tests/it/parse_error_pedagogy.rs` now holds a per-command +near-miss matrix, built **test-first** from an empirical baseline +capture. Covers, in both modes: every entry word's bare / +missing-clause / wrong-token cases, the app-lifecycle trailing-junk +cases, and the **committed multi-forms** (`add index`, +`add constraint`, `add 1:n relationship`, `drop index/constraint/ +relationship`, `show table`, `change column`, `create index`, +`alter table add/drop`). Tests: `near_miss_matrix_simple_mode`, +`near_miss_matrix_advanced_mode`, `near_miss_matrix_committed_multiforms`, +plus per-gap tests. + +### Four gap fixes (each test-first) + +- **G1** — bare `1` cardinality literal → `` `1:n relationship` `` in + `format_expectation` (render-only; completion still offers `1`). +- **G2** — bare `select`'s 14-item expression first-set → "a + projection: `*`, a column, or an expression", detected by the + `distinct`+`all` quantifier pair (unique to a projection start; + empirically verified non-misfiring). Render-only in + `format_walker_error`. +- **G3** — usage block was mode-blind. Added `usage_*_in_mode` + (`src/dsl/grammar/mod.rs`) + mode threading through + `render_usage_block` (`app.rs`) and the ambient usage + (`input_render.rs`). **Decision (user, after review):** advanced + mode shows **all forms valid in the mode, SQL-first then the DSL + fallback forms** — DSL forms (`create table … with pk`, + `drop column …`) remain valid input in advanced mode (verified), + so a usage hint must not hide them. Simple mode = DSL only. +- **G4** — `with` got its own `parse.usage.with` CTE template + (was borrowing `select`'s). +- **CROSS JOIN ON** — `parse.cross_join_no_on` teaches "a CROSS JOIN + has no ON clause …" when the failing token is `on` and the most + recent consumed join is a CROSS join. `is_cross_join_on` in + `parser.rs`; render-only. + +### Two adversarial review passes earned their keep + +- Pass 1 caught **G3 over-correction**: an initial "SQL-only" + advanced usage block *hid* valid DSL fallback forms. Escalated → + user chose "show all valid forms" → fixed (`1d4923b`). +- "Verify, don't trust the survey" **reversed two of three** + advanced-SQL "gaps": INSERT…SELECT count and RETURNING scope were + *already handled* (the Explore-survey list was wrong, twice). + Only CROSS JOIN ON was real. +- The matrix itself caught a regression mid-work (advanced + insert/update/delete falling back to available-commands because + the SQL nodes have empty `usage_ids`; fixed with a union fallback). + +### Deferred by decision (low-priority residual) + +At **submit** time, a non-projection expression position (bare +`where `, `returning `, `having `, `set col=`) still renders the raw +~14-item expression first-set; only the SELECT projection is glossed +(G2). Low-impact because **typing-time completion already offers the +right candidates** there. User chose to leave it. Documented in +ADR-0042 + `requirements.md`. + +Plus one **pre-existing caveat** (not this session's work, noted in +ADR-0042): `insert into T (one_col) select * from Multi` isn't +pre-caught for arity — `SELECT *` isn't expanded; the engine rejects +it at execution (brushes ADR-0019 §OOS-2 engine-error territory). + +## §3. ⚠️ Where parse-error pedagogy lives now (read before touching) + +- **Usage templates:** `parse.usage.*` in `src/friendly/strings/en-US.yaml`; + `usage_ids` on each `CommandNode` (`src/dsl/grammar/mod.rs`). + Mode-aware selection: `usage_keys_for_input_in_mode` / + `usage_key_for_input_in_mode`. +- **Structural error wording:** `format_walker_error` + + `format_expectation` in `src/dsl/parser.rs` (this is where the G1 + label, G2 projection gloss, and CROSS JOIN message live — + render-only, they do **not** mutate the `Expectation` set the + completion/hint layer consumes). +- **Catalog discipline (ADR-0019):** every new key goes in + `en-US.yaml` **and** `src/friendly/keys.rs::KEYS_AND_PLACEHOLDERS` + (the `keys_validate_against_catalog` test enforces it). +- **Tests:** integration parse-error tests live in `tests/it/` per + the handoff-57 §3 rule — drop the file in `tests/it/` + add a + `mod` line to `tests/it/main.rs`. Schema-aware diagnostics are + tested at the walker level with a `SchemaCache` (`vschema` helper + in `tests/it/sql_insert.rs`). + +## §4. Carried / unchanged + +- **arboard decisions** (handoff-55 §3): X11-only on Linux; `copy` + reproduces `[system]` tags. One-line changes if revisited. +- No open GitHub issues (`gh issue list` empty; the project's issue + tracker is GitHub, not the gitea host `tea` is configured for). + +## §5. Other tracks (from `requirements.md`) + +Unchanged: Track 2 Iter 6 leftovers (history.log input-history +hydration polish, migration-framework exercise); C3a modify +relationship; C4 m:n convenience; **H1 done** (ADR-0019), **H1a +done** (ADR-0042); H2 `hint`; H3 `help` (partial — general +reference + `help ` still missing); V4 session-log / +Markdown export; I1/I1b multi-line + readline; I3 tab completion / +I4 syntax highlighting (the walker already exposes the hooks); TU1 +tutorial (needs ADR); TT5 CI (not configured). + +## §6. Next job — pick one + +No single forced next step. Candidates, roughly by readiness: + +1. **H3 `help` completion** — the grammar tree already iterates the + REGISTRY for the command list; the missing pieces are a general + reference and `help ` per-command detail. The + `help_id` per `CommandNode` is the hook. Small-to-medium. +2. **I3 tab completion** — the walker's expected-set + completion + candidates already exist (used by ambient hints); I3 is the + **UI/UX** (cursor handling, menu, accept). Needs its own ADR. +3. **The deferred H1a residual** (§2) — generalise the projection + gloss to other expression positions. Low value (completion + already covers typing-time); only if it bugs you. +4. **CI (TT5)** — test infra is solid (2158 green); no workflow yet. + +## §7. How to take over + +1. Read handoffs 56 → 57 → 58, then `CLAUDE.md`, then + `docs/requirements.md` (H1 and **H1a now `[x]`**), + `docs/adr/README.md`. +2. **For anything parse-error/pedagogy: read ADR-0042, not + ADR-0020/0021** (those are superseded; chumsky is gone). +3. Codebase on `main` at `c305dc7`, clean, 4 unpushed. +4. Process pins that paid off this arc, again: **audit before + assuming** (ADR-0021 was obsolete; H1a was mostly already + shipped), **verify empirically — don't trust the docs/survey** + (the advanced-SQL gap list was wrong twice; a regression hid in + "looks fine"), **escalate genuine forks** (the G3 all-forms + decision was the user's, not mine), and **test-first + matrix as + a regression lock** (it caught a regression I introduced). + Commits user-confirmed, append-only, no AI attribution. diff --git a/docs/handoff/20260609-handoff-59.md b/docs/handoff/20260609-handoff-59.md new file mode 100644 index 0000000..04476ca --- /dev/null +++ b/docs/handoff/20260609-handoff-59.md @@ -0,0 +1,182 @@ +# Session handoff — 2026-06-09 (59) + +Fifty-ninth handover. Continues from handoff-58 (H1a done). This +session did **three** distinct things, in order: (1) a +**tracking-integrity audit + reconciliation** of `requirements.md` +against the actual code; (2) a **partials sweep** closing V5, H3, +and a new V5a; (3) the big one — **T3 compound-PK foreign-key +references**, designed in **ADR-0043**, `/runda`-verified twice, +and implemented end-to-end across both surfaces. + +## §1. State at handoff + +**Branch:** `main`. **HEAD `4752ba2`.** 8 commits this session +(`28e7596` → `4752ba2`); push is the user's step. + +**Tests: 2193 passing / 0 failing / 1 ignored** (lib 1578, it 423, +typing_surface_matrix 192; the 1 ignored is the long-standing +doc-test). **Clippy clean** (nursery, all targets). + +**Requirements markers now:** `[ ]` 19, `[/]` 6, `[x]` 59, +`[~]` 11, `[-]` 3. (The `[/]` "partial / in progress" marker is +**new this session** — see §2.) + +This session's commits: +``` +4752ba2 feat: compound-PK foreign-key references — grammar + tests (ADR-0043) +b14f019 refactor: relationship model to column lists for compound FK (ADR-0043) +b688592 docs: ADR-0043 implementation-readiness notes from /runda DA pass +274e2b1 docs: ADR-0043 compound-PK foreign-key references (T3); accepted +1d898ad feat: V5a show relationship/index detail views +757711f feat: H3 help per-command detail + general reference +8dec784 feat: V5 show tables / relationships / indexes list commands +28e7596 docs: reconcile requirements tracking with verified code state +``` + +## §2. The tracking reconciliation (read this — it changes how you plan) + +Planning off handoffs/`requirements.md` was **unreliable**: an +audit of all 35 `[ ]` items against the source found **~46 % +mis-marked**, overwhelmingly *under*-claimed (tab completion `I3` +and syntax highlighting `I4` were shipped but marked "not yet +implemented"). Root cause: the **binary legend** — a shipped +feature, a half-built one, and an untouched one all wore the same +`[ ]`. + +Fix (commit `28e7596`): added a **`[/]` "partial / in progress"** +marker to the legend, with a dated reconciliation note. Re-marked: +- **7 shipped-but-`[ ]` → `[x]`:** S1, S4, S5, I1a, I3, I4, C1. +- **9 substantially-built → `[/]`** with explicit gap notes: S3, + A1, V1, V2, V5, T3, H3, DOC1, X1. +- Fixed a **false `CLAUDE.md` claim** ("Tier 4 is wired") — no PTY + tests exist; TT4 is spec-only. + +**Discipline going forward:** move items `[ ]` → `[/]` → `[x]`; +keep the gap note current. Don't trust a marker without checking +the code — verify empirically. + +## §3. The partials sweep (V5, H3, V5a) + +Three small/medium, no-ADR partials closed (the others — V1, S3/V2, +A1, DOC1, X1 — need an ADR / output-model redesign / new features, +so were left `[/]`, see §5): + +- **V5 `[x]`** (`8dec784`): `show tables` / `show relationships` / + `show indexes` as one `Command::ShowList { kind }`; read-only + worker `show_list`. 10 tests. +- **H3 `[x]`** (`757711f`): `help ` per-command detail + (HELP node takes an optional `BarePath` topic; `note_help_topic` + renders every command sharing the entry word) + `help types` + + friendly unknown-topic pointer. 9 tests. +- **V5a `[x]`** (`1d898ad`): singular `show relationship ` / + `show index ` detail — folded into `ShowList { kind, name: + Option }`. Raised + closed this session (it was the + honest tracked home for V5's `[]` clause). + +## §4. T3 — compound-PK foreign-key references (the big one) + +**ADR-0043** (`docs/adr/0043-compound-pk-foreign-key-references.md`, +Accepted + implemented). The open leg of T3: a FK that *references* +a parent's compound PK. The audit found single-column FK woven +through ~15–20 sites; it earned an ADR, not an inline build. + +**Four forks, all user-confirmed at the recommended option:** +- **F-A** reference the parent's **full** PK (subset/UNIQUE-target + OOS), matched **positionally** to an equal-length child list. +- **F-B** **house-style uniform lists, no migration** — the + decisive simplifier: the user confirmed **back-compat is not + required** (pre-release, no installed base), so `project.yaml` + endpoints became `columns: [a, b]` (like `primary_key: [id]`), + metadata `TEXT` cells store the list **comma-joined** (bare for + single), and **no F3 migrator / version bump** was needed. +- **F-C** parenthesized DSL `from P.(a, b) to C.(x, y)` (single + bare form unchanged). +- **F-D** bare table-level SQL `FOREIGN KEY (x,y) REFERENCES P` + auto-expands to the parent's full PK when arities match. + +**Implementation (two commits):** +- `b14f019` — **the refactor**: relationship model `String → + Vec` through all six layers (`AddRelationship` / + `SqlForeignKey` AST, `RelationshipSchema`, metadata, + `project.yaml`, `ReadForeignKey`, `RelationshipEnd`). **Single- + column behaviour preserved** (one-element vecs); the existing + single-column relationship suite was the regression net. This is + the safe green checkpoint. +- `4752ba2` — **the grammar + tests**: multi-column parsing on both + surfaces (`AR_PARENT_COLS`/`AR_CHILD_COLS` Choice in `ddl.rs`; + `FK_*_COL_LIST` Repeated in `sql_create_table.rs`; + `consume_fk_reference` + the table-level/ALTER parsers collect + lists). Executor: `resolve_fk_parent_columns` (F-A validation + + F-D auto-expand + arity), per-pair `check_fk_type_compat`, + `schema_to_ddl` multi-column emission, `pragma_foreign_key_list` + read **grouped by `id`, ordered by `seq`** (positional pairing), + auto-name joins lists, `--create-fk` per-column, multi-column + teaching echo. 12 tests in `tests/it/compound_fk.rs`. + +**`/runda` ran twice** (once on the ADR pre-build, once on the +finished code). The post-build pass **found 3 coverage gaps and +they were closed**: save→rebuild round-trip (the riskiest — proves +comma-encode/decode + yaml + FK reconstruction survive a text→db +rebuild), undo (one step), per-pair type-mismatch. + +**Two residuals, confirmed non-blocking (noted in code):** +1. inline `col REFERENCES P(a,b)` gives a *generic* arity error, + not the ADR's tailored "use the table-level form" pointer. +2. a compound-FK *violation*'s friendly error names only the first + column pair (the ADR-0019 facts model is single-column). +Both are messaging-only; FK correctness/enforcement is unaffected. + +**Process note (a real scare):** a broad regex script over test +files (to wrap `add_relationship` call args + struct fields in +`vec![]`) **over-reached into lib structs** it shouldn't have +(`RelationshipSelector::Endpoints`, `FailureContext`/ +`TranslateContext` — all deliberately single-column), briefly +breaking the lib build. Recovered by reverting those specific +sites. Lesson: scripted edits across `src/` are dangerous when a +field name (`parent_column`) is shared between the struct you're +migrating and ones you're not — prefer compiler-guided manual fixes +or scope the script tightly. + +## §5. Remaining open landscape (now trustworthy) + +**Still `[/]` partial (need design/ADR or are larger):** +- **V1** relationship line-art visualization — **needs a + visualization ADR** (the most-requested deferred item). +- **S3 / V2** multi-result tabs — output-model redesign (single + buffer → tab abstraction). +- **A1** app-commands — blocked on `seed` (SD1) + `hint` (H2), + both unbuilt features. +- **DOC1** reference docs — writing effort. +- **X1** logging density — broad/mechanical (~25 `tracing` sites + today; `CLAUDE.md` wants "log liberally"). + +**`[ ]` not started:** H2 `hint`, SD1 `seed`, C4 m:n, B3 +query-timeout, I1 multi-line, I1b readline, I5 cancellation, TT5 +CI, TT4 PTY (spec-only), D1–D3 distribution, NFR-1…7. + +## §6. Next job — candidates + +No forced next step. By readiness: +1. **V1 relationship visualization** — design-first (its own ADR). + The user has flagged it repeatedly; it's the prominent open + design item. +2. **X1 logging** — mechanical, no ADR; brings the codebase to the + `CLAUDE.md` "log liberally" bar. +3. **TT5 CI** — test infra is solid (2193 green); no workflow yet. +4. The two T3 residuals (§4) — small messaging polish if they bug + you. + +## §7. How to take over + +1. Read handoffs 57 → 58 → 59, then `CLAUDE.md`, + `docs/requirements.md` (now with the `[/]` marker + the §2 + reconciliation note), `docs/adr/README.md`. +2. **For relationships/FK: read ADR-0043.** The model is list-based + (`Vec`); single-column is the one-element case. +3. Codebase on `main` at `4752ba2`, clean, 8 commits unpushed. +4. Process pins that paid off: **verify the tracker against code, + don't trust the marker**; **escalate genuine forks** (the four + ADR-0043 forks were the user's); **`/runda` after planning AND + after implementation** (the post-build pass found 3 real gaps); + **scope scripted edits tightly** (§4 scare). Commits + user-confirmed, append-only, no AI attribution. diff --git a/docs/plans/20260604-adr-0042-website.md b/docs/plans/20260604-adr-0044-website.md similarity index 97% rename from docs/plans/20260604-adr-0042-website.md rename to docs/plans/20260604-adr-0044-website.md index 79ebdbf..86bcea6 100644 --- a/docs/plans/20260604-adr-0042-website.md +++ b/docs/plans/20260604-adr-0044-website.md @@ -3,7 +3,7 @@ **Date:** 2026-06-04 · **Status:** ready to build Decisions for this work are recorded in -[ADR-0042](../adr/0042-public-website-and-documentation-site.md): Astro 6 + +[ADR-0044](../adr/0044-public-website-and-documentation-site.md): Astro 6 + Starlight + Tailwind v4; asciinema demos reusable in docs; the in-page WASM playground deferred behind a stable demo seam; portable static hosting (Vercel target); monorepo (`website/`); website is the canonical docs home; @@ -128,7 +128,7 @@ project lifecycle + undo/history → Tier 3 teaching echo + EXPLAIN + errors + completion/highlighting → Tier 4 clipboard + hints + editing. Conventions live in the **living style guide** `website/STYLE.md` (binding -rules from ADR-0042 §7 — no engine name, **no "DSL"**, "planned" callouts — +rules from ADR-0044 §7 — no engine name, **no "DSL"**, "planned" callouts — plus finer conventions and an open-decisions log for depth/splitting/example dataset/etc. as they settle). Sources to mine: `src/dsl/command.rs`, `src/dsl/grammar/*`, the REGISTRY, `en-US.yaml`, `docs/adr/*`, @@ -144,7 +144,7 @@ static host. `website/README.md` notes the Vercel preset (root dir `Demo.astro` exposes a stable contract (`{ src, title, height, autoplay }`). At launch it renders `Cast.astro`; later a `Playground.astro` WASM island swaps in behind the same props on the landing page and in docs, with zero -call-site changes. Boundary details are in ADR-0042 §3. +call-site changes. Boundary details are in ADR-0044 §3. ## Verification @@ -155,7 +155,7 @@ call-site changes. Boundary details are in ADR-0042 §3. - Starlight link-check passes (broken internal links fail the build). - Docs grep clean of forbidden terms: **no "DSL"**, no engine name. - A `dist/` static deploy works on Vercel (manual import) — confirms - portability. (No CI gate yet, per ADR-0042 §4.) + portability. (No CI gate yet, per ADR-0044 §4.) ## Notes / recommendations (non-blocking) diff --git a/docs/requirements.md b/docs/requirements.md index 938eae2..0cac2a5 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -13,17 +13,32 @@ is a process failure. **Scope.** The list is intentionally coarse — each item is a unit of "satisfied / not satisfied" judgement. When an item is taken up for implementation, it is decomposed further in a -backlog (initially in this repo, later in GitHub issues once the -repo is pushed). +backlog (initially in this repo, now tracked as Gitea issues). ## Status legend -- `[ ]` — open, not yet implemented +- `[ ]` — open, not yet started +- `[/]` — partial / in progress: some of it is built and tested, + but named gaps remain. The entry states what works and what is + still missing. (Distinct from `[ ]`, which is genuinely + untouched, and from `[~]`, which is *deliberately* deferred + pending an ADR rather than half-built.) - `[x]` — satisfied (implemented + tested) - `[~]` — deferred, awaiting an ADR or further design before any implementation - `[-]` — explicitly out of scope (rationale at the bottom) +> **Reconciliation note (2026-06-07).** A full audit of every +> `[ ]` item against the source found ~46 % of them mis-marked — +> overwhelmingly *under*-claimed (e.g. tab completion `I3` and +> syntax highlighting `I4` were shipped but marked "not yet +> implemented"). The binary legend was the root cause: a +> shipped feature, a half-built one, and an untouched one all +> wore the same `[ ]`. The `[/]` marker above was added to fix +> this, and the audited items were re-marked. When you implement +> against an item, move it `[ ]` → `[/]` → `[x]` rather than +> jumping straight to `[x]`, and keep the gap note current. + ## Test baseline After ADR-0022 Amendment 6 (the curated SQL function-name list — @@ -54,20 +69,34 @@ since ADR-0027.) ## TUI shell -- [ ] **S1** Three-region layout: items list (left), output +- [x] **S1** Three-region layout: items list (left), output panel (right), input field (bottom). + *(Verified 2026-06-07: `ui.rs:26-58` lays out a horizontal + split — items panel left, right column subdivided into output + panel / input field / hint panel; rendered every frame.)* - [x] **S2** Items list shows tables and per-table indexes; designed to extend to additional element kinds (relations, views, etc.) without restructuring. *(ADR-0025: the items panel renders a nested list — each table with its index names indented beneath it. The nested model is the extension point for future element kinds.)* -- [ ] **S3** Output panel renders a visualization of the +- [/] **S3** Output panel renders a visualization of the currently selected item and supports multiple tabs. -- [ ] **S4** Hint area below the input field; keyboard-toggleable + *(Partial, verified 2026-06-07: single-element structure + visualisation renders (`output_render.rs:82-180`); **multiple + tabs are not implemented** — the output is one line buffer, no + tab abstraction. Same multi-tab gap as V2.)* +- [x] **S4** Hint area below the input field; keyboard-toggleable for inspecting hints about the current input or last error. -- [ ] **S5** Mode label and distinct border style on the input + *(Verified 2026-06-07: `ui.rs:1088-1110` `render_hint_panel` / + `resolve_hint_lines` — a dynamic 1–`MAX_HINT_ROWS` panel below + the input showing ambient hints, candidates, or the last error.)* +- [x] **S5** Mode label and distinct border style on the input field communicate the current input mode at all times. + *(Verified 2026-06-07: `ui.rs:896-934` `render_input_panel` — + a coloured, bold mode label plus a mode-distinct border colour + (`border` vs `border_advanced`), tracking the three-way + `EffectiveMode` incl. the one-shot `:` state.)* - [x] **S6** Input-field validity indicator: a debounced `[ERR]` / `[WRN]` marker at the right edge of the input row, summarising — before submit — whether the current command @@ -89,12 +118,17 @@ since ADR-0027.) - [ ] **I1** Multi-line entry that auto-expands; Ctrl-Enter (or equivalent) submits, plain Enter inserts a newline. -- [ ] **I1a** In-line cursor editing in the input field: Left / +- [x] **I1a** In-line cursor editing in the input field: Left / Right arrows move the cursor by character (UTF-8 boundaries honoured), Home / End jump to the extremes, Delete removes the character at the cursor, Backspace removes the character - before. Insertion happens at the cursor position. *(Implemented; - multi-line editing per I1 still pending.)* + before. Insertion happens at the cursor position. + *(Verified 2026-06-07: `app.rs:973-1005` `cursor_left` / + `cursor_right` (with `is_char_boundary` checks), Home/End, + Delete, Backspace, `insert_at_cursor`. This is single-line + cursor editing and is complete on its own terms; the separate + **multi-line** entry goal is tracked under I1, which is + genuinely not started.)* - [ ] **I1b** Readline-style cursor shortcuts: Ctrl-A / Ctrl-E as aliases for Home / End for users on keyboards without those keys (and for ergonomics in command-driven workflows). Likely @@ -106,15 +140,33 @@ since ADR-0027.) navigable history is hydrated from the tail of `history.log` up to the same in-memory cap (Iter 6). Global rolling history is out of scope per OOS-6 / N4.)* -- [ ] **I3** Tab completion for app commands, DSL keywords, table +- [x] **I3** Tab completion for app commands, DSL keywords, table names, column names, and SQL keywords. - *(Refinement 2026-05-30, issue #15: SQL expression slots - (`sql_expr_ident`) now also offer a curated set of SQL function + *(Verified 2026-06-07 — this was mis-marked `[ ]` despite being + shipped: `src/completion.rs` is 2852 lines with ~100 tests; + `app.rs:898` binds Tab → `completion_tab_forward` (and + BackTab → `_backward`) with forward/backward cycling through a + candidate memo and a colour-coded candidate line in the hint + panel (`ui.rs:1125` `render_candidate_line`). All five + candidate categories work — app commands (via REGISTRY), DSL + keywords (walker `Expectation`), table names + column names + (`SchemaCache`), and SQL keywords/functions in advanced mode. + Refinement 2026-05-30, issue #15: SQL expression slots + (`sql_expr_ident`) also offer a curated set of SQL function names — `KNOWN_SQL_FUNCTIONS` in `src/dsl/sql_functions.rs`, - surfaced as a new `CandidateKind::Function` — ADR-0022 Amendment 6. - The broad tab-completion goal stays open.)* -- [ ] **I4** Syntax highlighting for both the DSL and SQL. - *(Refinement 2026-05-29, issue #8: column data types now carry a + surfaced as `CandidateKind::Function` (ADR-0022 Amendment 6). + The original "broad tab-completion" goal is met; any further + polish is incremental, not a missing core.)* +- [x] **I4** Syntax highlighting for both the DSL and SQL. + *(Verified 2026-06-07 — mis-marked `[ ]` despite being shipped: + `input_render.rs:64-113` lexes the input to styled byte-range + runs (`lex_to_runs_in_mode`) and renders them per-mode (DSL in + simple, SQL in advanced), with nine token classes in `theme.rs` + (`tok_keyword`, `tok_identifier`, `tok_string`, `tok_punct`, + `tok_flag`, `tok_error`, `tok_function`, `tok_type`) and + diagnostics overlaid (error/warning spans). Both surfaces are + highlighted; the core goal is met. + Refinement 2026-05-29, issue #8: column data types now carry a dedicated `HighlightClass::Type` / `tok_type` colour, distinct from identifiers and clause keywords — ADR-0022 Amendment 4; a further refinement 2026-05-30, issue #15: SQL function-name candidates carry @@ -172,24 +224,27 @@ since ADR-0027.) ## App-level commands (per ADR-0003) -- [ ] **A1** All canonical app-level commands implemented and +- [/] **A1** All canonical app-level commands implemented and available in both modes: `save`, `save as`, `load`, `new`, `rebuild`, `export`, `import`, `seed`, `replay`, `undo`, `redo`, `mode`, `help`, `hint`, `quit`. - *(Progress: `quit`/`q`, `mode simple|advanced`, `help`, - `save`, `save as`, `load`, `new`, `rebuild`, `export`, - `import`, `replay` all implemented (Iterations 4 + 5; - `replay` via ADR-0024 Phase E — see U4). `seed` in the - seeding iteration; `undo` / `redo` in the U-series; `hint` - with H2.)* + *(Partial, verified 2026-06-07: 13 of 15 implemented and + available in both modes — `quit`/`q`, `mode simple|advanced`, + `help`, `save`, `save as`, `load`, `new`, `rebuild`, `export`, + `import`, `replay`, `undo`, `redo` (REGISTRY in + `grammar/app.rs:249-333`). **Missing: `seed`** (tracked as SD1) + **and `hint`** (tracked as H2) — neither is registered. A1 + closes when SD1 + H2 land.)* ## DSL data commands -- [ ] **C1** Table operations: create / drop / rename. - *(Progress: create + drop done; **rename done on the advanced - surface** — `ALTER TABLE … RENAME TO`, ADR-0035 §6 / 4h. A simple-mode - rename-table verb is deliberately not provided — table rename is - advanced-mode only.)* +- [x] **C1** Table operations: create / drop / rename. + *(Verified 2026-06-07: create + drop done; **rename done on the + advanced surface** — `ALTER TABLE … RENAME TO`, ADR-0035 §6 / 4h + (`do_rename_table`, `db.rs:4821`). A simple-mode rename-table + verb is **deliberately** not provided — table rename is + advanced-mode only — so the requirement is satisfied as + designed, not partial.)* - [x] **C2** Column operations: add / drop / rename / change type. `drop column` and `rename column` use SQLite native ALTER TABLE (3.35+ / 3.25+); `change column` routes through @@ -343,29 +398,54 @@ since ADR-0027.) *(Implemented per ADR-0014; auto-fills omitted shortid columns and validates user-supplied values against the same alphabet and length range.)* -- [ ] **T3** Compound primary keys handled end-to-end (DSL, +- [x] **T3** Compound primary keys handled end-to-end (DSL, storage, display, FK reference). - *(Progress: DSL grammar (`with pk a(int),b(int)`), storage, and - table-info description are all present; the FK iteration - references single-column PKs only — compound-key FK references - remain pending.)* + *(Done 2026-06-09 via **ADR-0043**: the FK-reference leg now + works on both surfaces — DSL `add 1:n relationship from + P.(a, b) to C.(x, y)` and SQL `FOREIGN KEY (a, b) REFERENCES + P(x, y)` (bare `REFERENCES P` auto-expands to the full PK). + References the parent's full compound PK matched positionally, + per-pair type-compat (ADR-0011); the FK is engine-enforced, + persisted (`columns: [a, b]` in `project.yaml`, comma-joined in + metadata), shown symmetrically by `describe`, and `--create-fk` + creates one child column per parent PK column. The relationship + model went list-based through all six layers, single-column + behaviour preserved (commit `b14f019`); 12 integration tests in + `tests/it/compound_fk.rs` plus the existing single-column suite + as the regression net. The earlier-noted (2026-06-07) breakdown:* + *compound-PK **declaration** (`with pk a(int),b(int)`), + **storage** (`primary_key: Vec`), and **display** were + already present and tested. The FK-reference leg — once an + ADR-first ~15–20-site change across the relationship model — is + what ADR-0043 delivered (back-compat dropped by user decision, so + no migration was needed). Subset / non-PK (UNIQUE-target) FK + references stay out of scope.)* ## Visualizations -- [ ] **V1** Single-element views render in the output pane: a +- [/] **V1** Single-element views render in the output pane: a selected table as its structure (columns, types, keys, constraints); a selected relationship as two tables joined by a line. - *(Progress: a basic structure view (column rows with SQLite - type names) is rendered after each successful DDL; pretty - rendering, selection nav, and relationship line-art pending — - see V4 for the broader direction.)* -- [ ] **V2** SQL query results render as a dynamic table view in + *(Partial, verified 2026-06-07: the **table-structure** half is + done — `output_render.rs:82-180` renders columns / types / + constraints / indexes in a box-drawing table, with relationship + metadata as `References:` / `Referenced by:` prose + (`A.col → B.col`). The **relationship-as-line-art** half — two + tables drawn side by side with a connecting line — is **not + implemented** (deferred per `output_render.rs` §5 OOS-1, ADR + pending). This is the relationship-visualisation piece that has + been repeatedly pushed away; it is the substantive open part of + V1. Selection-nav and the broader journal direction live in V4.)* +- [/] **V2** SQL query results render as a dynamic table view in the output pane, with multiple result tabs supported. - *(Progress: a basic aligned-column data view is rendered for - `show data` and after every write (ADR-0014). Pretty - box-drawing tables with truncation/scroll handling, plus - multi-tab support, remain in V4 territory.)* + *(Partial, verified 2026-06-07: the **table view** is done — + `output_render.rs:38-72` `render_data_table` renders a + box-drawing frame with aligned columns (numeric right, text + left) and NULL/control-char sanitisation, for `show data` and + after every write (ADR-0014). **Missing: multiple result tabs** + — the output is a single `VecDeque` with no tab + abstraction (same gap as S3). Multi-tab sits in V4 territory.)* - [~] **V3** Full ER-diagram export (whole-database graph, viewed outside the TUI) — low priority; design and ADR pending. - [~] **V4** Output panel as a *scrollable per-session log* with @@ -381,10 +461,34 @@ since ADR-0027.) buffer is in, with new output snapping the view to the most recent. The full V4 scope — smart structure rendering, log styling, Markdown export, scroll indicator — remains pending.)* -- [ ] **V5** `show []` family of commands for - redisplaying schema info on demand. *(Progress: `show table - ` and `show data ` implemented; - `show tables`, `show relationships`, etc. pending.)* +- [x] **V5** `show []` family of commands for + redisplaying schema info on demand. + *(Done 2026-06-07: `show table ` + `show data
` + (single-item) plus the list-all family `show tables` / + `show relationships` / `show indexes` — the latter three landed + as `Command::ShowList { kind }` (one variant, `grammar/data.rs`), + a read-only worker `show_list` formatting count-headed lists from + the same helpers the items panel uses, with help + parse-usage + entries and 10 integration tests (`tests/it/show_list.rs`). The + one remaining member of the `[]` clause — singular + per-item detail for relationships/indexes — is split out as + **V5a** below so it has a tracked home rather than living as a + footnote here.)* +- [x] **V5a** Singular per-item detail views `show relationship + ` / `show index ` — the `[]` half of V5 for + the relationship and index kinds (the table kind already has + `show table `). + *(Done 2026-06-07: folded into `Command::ShowList { kind, name: + Option }` — `name: Some(_)` is the singular form. Two + grammar branches (`relationship ` / `index `, + reusing the `Relationships`/`Indexes` completion sources for the + name slot), a worker `do_show_one` rendering a labelled detail + block (endpoints + ON DELETE/UPDATE for a relationship; table, + columns, uniqueness for an index) or a friendly "No relationship/ + index named `X`." line, reusing the V5 `ShowList` render path. + Help + parse-usage entries + two ADR-0042 near-miss matrix rows; + 5 added integration tests. V5's `[]` clause is now + complete across all three kinds.)* - [x] **V6** Copy the output panel to the system clipboard (issue #11, ADR-0041). `copy` / `copy all` copy the whole panel; `copy last` copies the most recent command's output. @@ -557,14 +661,14 @@ since ADR-0027.) migration sweep of all other user-facing strings, advanced-mode SQL-error sanitization (§OOS-2), and `messages` persistence (§OOS-3, awaits the settings ADR).)* -- [ ] **H1a** Strong syntax-help in parse errors. When the user +- [x] **H1a** Strong syntax-help in parse errors. When the user types something near-correct (e.g. `insert into T ('Oli')` — forgotten `values`; or `update T set x=1` — missing WHERE), the error should *name the missing keyword or clause* rather than just point at the unexpected character. This is a separate effort from H1 (which targets database errors); it - targets parser errors. Pending — multiple targeted fixes - shipping piecemeal so far (e.g. `values` becoming optional in + targets parser errors. *(Done via the **ADR-0042** systematic + pass, 2026-06-06.)* Built piecemeal first (e.g. `values` becoming optional in INSERT removes one such case; ADR-0024's typed value slots give per-column-type rejection wording; `insert into T (col)` with no `values` clause now flags "looks like Form A — add @@ -589,19 +693,41 @@ since ADR-0027.) `col`…"* message at typing time, counted against the user-fillable columns, with `serial`/`shortid` auto-fill named; new keys `diagnostic.insert_arity_mismatch_form_b_simple` / - `diagnostic.insert_arity_mismatch_all_auto`). A systematic pass is - still pending. + `diagnostic.insert_arity_mismatch_all_auto`). The **ADR-0042 + systematic pass** then closed it: a per-command near-miss matrix + (`tests/it/parse_error_pedagogy.rs`) locks every entry word's + bare / missing-clause / wrong-token cases plus the committed + multi-forms, in both modes; friendlier labels landed (`add` → + `1:n relationship`; bare `select` → "a projection: …"); the + usage block became mode-aware (advanced shows the SQL forms + plus the still-valid DSL fallback forms, SQL-first); `with` + got its own CTE template; and `cross join … on` now teaches + that a CROSS JOIN takes no ON clause. The advanced-SQL + diagnostics that the survey thought missing (INSERT…SELECT + count, RETURNING column scope) were verified already present. + One low-priority residual is deferred by decision: at *submit* + time a non-projection expression position (bare `where `, + `returning `) still shows the raw expression first-set — + typing-time completion already offers the right candidates + there, so the payoff is small. - [ ] **H2** `hint` provides contextual help for the current input or the most recent error. -- [ ] **H3** `help` provides general reference and per-command +- [x] **H3** `help` provides general reference and per-command help. - *(Progress: the `help` command lists currently-supported - commands + DSL grammar reference + types. As of ADR-0024 - §help_id it is assembled by iterating the command REGISTRY - and translating each `CommandNode.help_id`, so a new command - appears automatically. A general reference and `help - `-style detailed per-command help are still the - missing pieces.)* + *(Done 2026-06-07: the **general reference** is `help` (no arg) — + intro + the full command list (REGISTRY × `help_id`, so new + commands appear automatically) + the type reference + a footer + pointing at the focused form. **Per-command help** is `help + ` (H3's new piece): the HELP node took an optional + single-word topic (`BarePath`), `AppCommand::Help { topic }`, + and `note_help_topic` renders the block(s) of every command + sharing that entry word — so `help create` covers both create + forms — plus `help types` for the type reference and a friendly + "no help for `X`" pointer for an unknown topic. Help/usage + strings catalogued + key-registered; 9 integration tests + (`tests/it/help_command.rs`). A richer *narrative* overview + (modes, the `:` escape, syntax conventions) is reference-docs + scope, tracked under **DOC1** — not part of H3.)* ## CLI @@ -636,7 +762,7 @@ since ADR-0027.) ## Documentation -- [ ] **DOC1** User- and student-facing reference +- [/] **DOC1** User- and student-facing reference documentation under `docs/`: the DSL command surface, the type system, and the boundaries of simple mode. `docs/simple-mode-limitations.md` is the first piece — @@ -644,6 +770,11 @@ since ADR-0027.) reference. Distinct from in-app `help` (`H3`), the interactive tutorial system (`TU1`), and the sharing recipes under `E2`. + *(Partial, verified 2026-06-07: `docs/simple-mode-limitations.md` + exists (~55 lines, covers the WHERE-expression and + table-creation boundaries). **Missing:** a DSL + command-surface reference and a standalone type-system + reference under `docs/`.)* ## Testing (per ADR-0008) @@ -657,14 +788,29 @@ since ADR-0027.) - [~] **TT4** Tier 4: PTY-based end-to-end for the four critical flows named in ADR-0008 (cold launch → DDL → quit; save → reopen; export → import → rebuild; undo after DROP). + *(Verified 2026-06-07: **nothing is wired** — no + `portable-pty` / `expectrl` / `vt100` dependencies, no PTY test + files; ADR-0008 §Tier-4 is a specification only. The Tier-3 + `tests/it/*_e2e.rs` files are synthetic event-loop tests, not + PTY. Correcting a stale `CLAUDE.md` line that read "Tier 4 is + wired only for the listed critical flows" — it was not wired at + all. Genuinely deferred.)* - [ ] **TT5** CI runs all tiers on Linux, macOS, and Windows on stable Rust. ## Cross-cutting -- [ ] **X1** Comprehensive logging via the project's logging +- [/] **X1** Comprehensive logging via the project's logging infrastructure per `CLAUDE.md` (decision points, parameter values, fallback paths). + *(Partial, verified 2026-06-07: the logging **harness** is + wired — `src/logging.rs` sets up file-backed `tracing` with an + env filter — but instrumentation is **sparse**: ~25 `tracing::` + call sites across the tree, concentrated in `runtime.rs` and + `undo.rs` and mostly error/warning on failure paths. The + decision-point / parameter-value / fallback-path coverage the + `CLAUDE.md` "log liberally" standard calls for — especially in + `db.rs`, the parser, and the executors — is largely absent.)* - [~] **X2** Language: English-only for v1; multi-language is an open question to revisit later. - [~] **X3** Accessibility: TUI screen-reader support is diff --git a/src/app.rs b/src/app.rs index 6e6989c..02c890b 100644 --- a/src/app.rs +++ b/src/app.rs @@ -605,6 +605,15 @@ impl App { self.handle_dsl_explain_success(&command, &plan); Vec::new() } + AppEvent::DslShowListSucceeded { command, lines } => { + // Mark the echo ✓ (ADR-0040), then emit the + // worker-formatted list as system output lines. + self.note_ok_summary(&command); + for line in lines { + self.note_system(line); + } + Vec::new() + } AppEvent::DslInsertSucceeded { command, result } => { self.handle_dsl_insert_success(&command, &result); Vec::new() @@ -1312,8 +1321,11 @@ impl App { use crate::dsl::{AppCommand, MessagesValue, ModeValue}; match cmd { AppCommand::Quit => vec![Action::Quit], - AppCommand::Help => { - self.note_help(); + AppCommand::Help { topic } => { + match &topic { + Some(t) => self.note_help_topic(t), + None => self.note_help(), + } Vec::new() } AppCommand::Rebuild => vec![Action::PrepareRebuild], @@ -1338,11 +1350,11 @@ impl App { }, ), AppCommand::Import { path, target } => { - // The path-bearing import goes through the - // pre-chumsky source-slice (parser.rs), which - // already validated non-empty path. Bare - // `import` returns from chumsky with an empty - // path string — surface the usage error. + // A path-bearing import carries a non-empty path + // from the walker. Bare `import` parses with an + // empty path string — surface the usage hint here + // at dispatch (not a parse error; ADR-0024 replaced + // the old chumsky source-slice path). if path.is_empty() { self.note_error(crate::t!("project.import_usage")); return Vec::new(); @@ -1521,7 +1533,7 @@ impl App { for note in notes { self.note_error(note); } - self.note_error(render_usage_block(input)); + self.note_error(render_usage_block(input, mode)); return vec![Action::JournalFailure { source: input.to_string(), }]; @@ -1601,7 +1613,7 @@ impl App { // known command-entry keyword was consumed) or // the available-commands fallback (§5). if let ParseError::Invalid { .. } = &err { - self.note_error(render_usage_block(input)); + self.note_error(render_usage_block(input, mode)); } // ADR-0034 §1/§2: a submitted line that failed to // parse is journalled `err` so it is recallable @@ -1956,12 +1968,14 @@ impl App { ), C::AddRelationship { parent_table, - parent_column, + parent_columns, .. } => ( Operation::AddRelationship, Some(parent_table.as_str()), - Some(parent_column.as_str()), + // Single-column facts model (ADR-0019): the first PK + // column for a compound FK (ADR-0043). + parent_columns.first().map(String::as_str), ), C::DropRelationship { selector } => match selector { RelationshipSelector::Endpoints { @@ -2007,6 +2021,9 @@ impl App { C::ShowData { name, .. } | C::ShowTable { name } => { (Operation::Query, Some(name.as_str()), None) } + // A `show ` list spans no single table; a failure + // routes through Query with no table fallback. + C::ShowList { .. } => (Operation::Query, None, None), // A SQL `SELECT` carries only its statement text — // no single table name to fall back on. A query // failure routes through `Operation::Query`. @@ -2393,6 +2410,49 @@ impl App { .lines() .map(str::to_string), ); + // H3: point at the focused per-command form. + lines.push(crate::t!("help.detail_hint")); + for line in lines { + self.note_system(line); + } + } + + /// Focused per-command help (H3): `help `, where `topic` + /// is a command entry word (`insert`, `create`, `show`, …) or + /// the special `types`. Renders the help block(s) of every + /// command sharing that entry word — so `help create` covers + /// both the DSL and SQL create forms — or a friendly pointer + /// back to `help` when nothing matches. + fn note_help_topic(&mut self, topic: &str) { + use crate::dsl::grammar::REGISTRY; + + let topic = topic.trim(); + // `help types` re-shows just the type reference. + if topic.eq_ignore_ascii_case("types") { + for line in crate::t!("help.types_reference").lines() { + self.note_system(line.to_string()); + } + return; + } + + let mut lines: Vec = Vec::new(); + for (command, _category) in REGISTRY { + let Some(help_id) = command.help_id else { + continue; + }; + if command.entry.matches(topic) { + let key = format!("help.{help_id}"); + let body = crate::friendly::translate(&key, &[]); + lines.extend(body.lines().map(str::to_string)); + } + } + + if lines.is_empty() { + // No command owns that entry word — name it and point + // back at the full list rather than failing silently. + self.note_system(crate::t!("help.unknown_topic", topic = topic)); + return; + } for line in lines { self.note_system(line); } @@ -2557,16 +2617,18 @@ fn parse_error_message(err: &ParseError) -> String { /// renders every catalog template — multi-form families like /// `drop` show every variant. Otherwise the fallback lists every /// entry keyword alphabetically. -fn render_usage_block(input: &str) -> String { +fn render_usage_block(input: &str, mode: Mode) -> String { // A multi-form command that has committed to a form // (`add index …`) shows only that form's usage; a bare // multi-form entry word (`add`) shows the whole family. + // Mode-aware (ADR-0042 G3): in advanced mode a shared entry + // word shows its SQL forms, not the DSL templates. let catalog_keys: Vec<&'static str> = - crate::dsl::grammar::usage_key_for_input(input) + crate::dsl::grammar::usage_key_for_input_in_mode(input, mode) .map(|key| vec![key]) .or_else(|| { - crate::dsl::grammar::usage_keys_for_input(input) - .map(|(_word, all)| all.to_vec()) + crate::dsl::grammar::usage_keys_for_input_in_mode(input, mode) + .map(|(_word, all)| all) }) .unwrap_or_default(); if !catalog_keys.is_empty() { @@ -2820,13 +2882,23 @@ mod tests { #[test] fn tab_cycles_forward_through_multi_candidate_set() { + // `show ` offers five subcommands in grammar order: + // data / table / tables / relationships / indexes (V5). let mut app = App::new(); type_str(&mut app, "show "); - app.update(key(KeyCode::Tab)); - assert_eq!(app.input, "show data"); - app.update(key(KeyCode::Tab)); - assert_eq!(app.input, "show table"); - // Wrap-around. + for expected in [ + "show data", + "show table", + "show tables", + "show relationships", + "show indexes", + "show relationship", + "show index", + ] { + app.update(key(KeyCode::Tab)); + assert_eq!(app.input, expected); + } + // Wrap-around back to the first. app.update(key(KeyCode::Tab)); assert_eq!(app.input, "show data"); } @@ -2835,12 +2907,12 @@ mod tests { fn shift_tab_cycles_backward_starting_from_last() { let mut app = App::new(); type_str(&mut app, "show "); - app.update(key(KeyCode::BackTab)); - assert_eq!(app.input, "show table"); - app.update(key(KeyCode::BackTab)); - assert_eq!(app.input, "show data"); - app.update(key(KeyCode::BackTab)); - assert_eq!(app.input, "show table"); + // Backward starts from the last candidate (`index`, the + // V5a singular form). + for expected in ["show index", "show relationship", "show indexes"] { + app.update(key(KeyCode::BackTab)); + assert_eq!(app.input, expected); + } } #[test] diff --git a/src/completion.rs b/src/completion.rs index 10c9132..ef74daa 100644 --- a/src/completion.rs +++ b/src/completion.rs @@ -1661,9 +1661,23 @@ mod tests { } #[test] - fn show_offers_data_and_table_alphabetised() { + fn show_offers_all_subcommands() { + // `show` branches: data / table (singular) plus the V5 + // list-all forms tables / relationships / indexes, in + // grammar-declaration order. let cs = cands("show ", 5); - assert_eq!(cs, vec!["data".to_string(), "table".to_string()]); + assert_eq!( + cs, + vec![ + "data".to_string(), + "table".to_string(), + "tables".to_string(), + "relationships".to_string(), + "indexes".to_string(), + "relationship".to_string(), + "index".to_string(), + ], + ); } #[test] diff --git a/src/db.rs b/src/db.rs index 661e5f6..4adc288 100644 --- a/src/db.rs +++ b/src/db.rs @@ -114,10 +114,11 @@ pub struct RelationshipEnd { pub name: String, /// The other table involved. pub other_table: String, - /// The column on the other table. - pub other_column: String, - /// The column on *this* table. - pub local_column: String, + /// The column(s) on the other table (ordered, paired with + /// `local_columns`; one element for single-column — ADR-0043). + pub other_columns: Vec, + /// The column(s) on *this* table. + pub local_columns: Vec, pub on_delete: ReferentialAction, pub on_update: ReferentialAction, } @@ -554,6 +555,17 @@ enum Request { ListTables { reply: oneshot::Sender, DbError>>, }, + /// List every item of a schema kind (tables / relationships / + /// indexes) as pre-formatted display lines for the `show + /// ` commands (V5). Read-only; formats in the worker + /// from the same helpers the items panel and describe view use. + ShowList { + kind: crate::dsl::command::ShowListKind, + /// `None` lists all items of the kind; `Some(name)` shows + /// one named relationship/index's detail (V5a). + name: Option, + reply: oneshot::Sender, DbError>>, + }, DescribeTable { name: String, source: Option, @@ -562,9 +574,9 @@ enum Request { AddRelationship { name: Option, parent_table: String, - parent_column: String, + parent_columns: Vec, child_table: String, - child_column: String, + child_columns: Vec, on_delete: ReferentialAction, on_update: ReferentialAction, create_fk: bool, @@ -1317,6 +1329,18 @@ impl Database { recv.await.map_err(|_| DbError::WorkerGone)? } + /// Pre-formatted display lines for `show tables` / + /// `show relationships` / `show indexes` (V5). Read-only. + pub async fn show_list( + &self, + kind: crate::dsl::command::ShowListKind, + name: Option, + ) -> Result, DbError> { + let (reply, recv) = oneshot::channel(); + self.send(Request::ShowList { kind, name, reply }).await?; + recv.await.map_err(|_| DbError::WorkerGone)? + } + pub async fn describe_table( &self, name: String, @@ -1337,9 +1361,9 @@ impl Database { &self, name: Option, parent_table: String, - parent_column: String, + parent_columns: Vec, child_table: String, - child_column: String, + child_columns: Vec, on_delete: ReferentialAction, on_update: ReferentialAction, create_fk: bool, @@ -1349,9 +1373,9 @@ impl Database { self.send(Request::AddRelationship { name, parent_table, - parent_column, + parent_columns, child_table, - child_column, + child_columns, on_delete, on_update, create_fk, @@ -2245,6 +2269,9 @@ fn handle_request( Request::ListTables { reply } => { let _ = reply.send(do_list_tables(conn)); } + Request::ShowList { kind, name, reply } => { + let _ = reply.send(do_show_list(conn, kind, name.as_deref())); + } Request::DescribeTable { name, source, @@ -2260,9 +2287,9 @@ fn handle_request( Request::AddRelationship { name, parent_table, - parent_column, + parent_columns, child_table, - child_column, + child_columns, on_delete, on_update, create_fk, @@ -2275,9 +2302,9 @@ fn handle_request( source.as_deref(), name.as_deref(), &parent_table, - &parent_column, + &parent_columns, &child_table, - &child_column, + &child_columns, on_delete, on_update, create_fk, @@ -2977,9 +3004,9 @@ fn read_all_relationships(conn: &Connection) -> Result, Ok(RelationshipSchema { name: row.get(0)?, parent_table: row.get(1)?, - parent_column: row.get(2)?, + parent_columns: decode_rel_columns(row.get::<_, String>(2)?.as_str()), child_table: row.get(3)?, - child_column: row.get(4)?, + child_columns: decode_rel_columns(row.get::<_, String>(4)?.as_str()), on_delete: parse_action_from_sqlite(row.get::<_, String>(5)?.as_str()), on_update: parse_action_from_sqlite(row.get::<_, String>(6)?.as_str()), }) @@ -3131,6 +3158,15 @@ fn row_value_to_cell(row: &rusqlite::Row<'_>, idx: usize) -> Result String { + cols.iter() + .map(|c| quote_ident(c)) + .collect::>() + .join(", ") +} + fn quote_ident(name: &str) -> String { let mut out = String::with_capacity(name.len() + 2); out.push('"'); @@ -3430,9 +3466,9 @@ fn do_create_table( for fk in &resolved_fks { ddl.push_str(&format!( ", FOREIGN KEY ({child}) REFERENCES {parent}({pcol}) ON DELETE {od} ON UPDATE {ou}", - child = quote_ident(&fk.child_column), + child = quote_cols(&fk.child_columns), parent = quote_ident(&fk.parent_table), - pcol = quote_ident(&fk.parent_column), + pcol = quote_cols(&fk.parent_columns), od = fk.on_delete.sql_clause(), ou = fk.on_update.sql_clause(), )); @@ -3471,9 +3507,9 @@ fn do_create_table( &tx, &fk.name, &fk.parent_table, - &fk.parent_column, + &fk.parent_columns, name, - &fk.child_column, + &fk.child_columns, fk.on_delete, fk.on_update, )?; @@ -5834,6 +5870,145 @@ fn do_list_tables(conn: &Connection) -> Result, DbError> { Ok(out) } +/// Pre-formatted display lines for the `show ` list commands +/// (V5). A count header followed by one indented item per line, or a +/// single friendly "none yet" line for an empty collection. Reuses +/// the same helpers the items panel / describe view read from, so the +/// list never drifts from those views. Engine-neutral wording per the +/// ADR-0002 user-facing posture. +fn do_show_list( + conn: &Connection, + kind: crate::dsl::command::ShowListKind, + name: Option<&str>, +) -> Result, DbError> { + use crate::dsl::command::ShowListKind; + // V5a: a named item shows one relationship/index's detail. + if let Some(name) = name { + return do_show_one(conn, kind, name); + } + let mut lines = Vec::new(); + match kind { + ShowListKind::Tables => { + let tables = do_list_tables(conn)?; + if tables.is_empty() { + lines.push("No tables in this project yet.".to_string()); + } else { + lines.push(format!("Tables ({}):", tables.len())); + for name in tables { + lines.push(format!(" {name}")); + } + } + } + ShowListKind::Relationships => { + let rels = read_all_relationships(conn)?; + if rels.is_empty() { + lines.push("No relationships in this project yet.".to_string()); + } else { + lines.push(format!("Relationships ({}):", rels.len())); + for r in rels { + let mut line = format!( + " {}: {} → {}", + r.name, + fmt_rel_endpoint(&r.parent_table, &r.parent_columns), + fmt_rel_endpoint(&r.child_table, &r.child_columns), + ); + if r.on_delete != ReferentialAction::default_action() { + line.push_str(&format!(" on delete {}", r.on_delete.keyword())); + } + if r.on_update != ReferentialAction::default_action() { + line.push_str(&format!(" on update {}", r.on_update.keyword())); + } + lines.push(line); + } + } + } + ShowListKind::Indexes => { + // Each table's user-created indexes (origin "c"), the + // same set the items panel shows. Ordered by table, then + // by index name (read_table_indexes orders by name). + let tables = do_list_tables(conn)?; + let mut entries: Vec = Vec::new(); + for table in &tables { + for ix in read_table_indexes(conn, table)? { + let unique = if ix.unique { " [unique]" } else { "" }; + entries.push(format!( + " {}.{} ({}){unique}", + table, + ix.name, + ix.columns.join(", ") + )); + } + } + if entries.is_empty() { + lines.push("No indexes in this project yet.".to_string()); + } else { + lines.push(format!("Indexes ({}):", entries.len())); + lines.extend(entries); + } + } + } + Ok(lines) +} + +/// Detail lines for one named relationship or index (V5a): a +/// labelled block, or a friendly "no such item" line. `Tables` is +/// never routed here (the table singular is `ShowTable`); the +/// defensive arm keeps the match total without a panic. +fn do_show_one( + conn: &Connection, + kind: crate::dsl::command::ShowListKind, + name: &str, +) -> Result, DbError> { + use crate::dsl::command::ShowListKind; + let mut lines = Vec::new(); + match kind { + ShowListKind::Relationships => match read_all_relationships(conn)? + .into_iter() + .find(|r| r.name == name) + { + None => lines.push(format!("No relationship named `{name}`.")), + Some(r) => { + lines.push(format!("Relationship `{}`:", r.name)); + lines.push(format!( + " {} → {}", + fmt_rel_endpoint(&r.parent_table, &r.parent_columns), + fmt_rel_endpoint(&r.child_table, &r.child_columns), + )); + lines.push(format!(" on delete {}", r.on_delete.keyword())); + lines.push(format!(" on update {}", r.on_update.keyword())); + } + }, + ShowListKind::Indexes => { + // Find the user-created index by name across all tables. + let mut found = None; + for table in do_list_tables(conn)? { + if let Some(ix) = read_table_indexes(conn, &table)? + .into_iter() + .find(|ix| ix.name == name) + { + found = Some((table, ix)); + break; + } + } + match found { + None => lines.push(format!("No index named `{name}`.")), + Some((table, ix)) => { + lines.push(format!("Index `{}` on {table}:", ix.name)); + lines.push(format!(" columns: {}", ix.columns.join(", "))); + lines.push(format!( + " unique: {}", + if ix.unique { "yes" } else { "no" } + )); + } + } + } + ShowListKind::Tables => { + lines.push(format!("No relationship or index named `{name}`.")); + } + } + Ok(lines) +} + /// Internal full schema of a table, sufficient to regenerate /// its `CREATE TABLE` statement during the rebuild dance. #[derive(Debug, Clone)] @@ -5884,8 +6059,11 @@ struct ReadColumn { #[derive(Debug, Clone)] struct ReadForeignKey { parent_table: String, - parent_column: String, - child_column: String, + /// Parent + child column lists, positionally paired; one + /// element for single-column, ordered list for a compound FK + /// (ADR-0043). + parent_columns: Vec, + child_columns: Vec, on_delete: ReferentialAction, on_update: ReferentialAction, } @@ -5946,30 +6124,50 @@ fn read_schema(conn: &Connection, table: &str) -> Result { } } - // Foreign keys from pragma_foreign_key_list. + // Foreign keys from pragma_foreign_key_list. A *compound* FK + // (ADR-0043) is reported as several rows sharing one `id`, in + // `seq` order — so we group consecutive same-`id` rows into one + // `ReadForeignKey` with positionally-ordered column lists. let mut fk_stmt = conn .prepare( - "SELECT \"table\", \"from\", \"to\", on_delete, on_update \ + "SELECT id, \"table\", \"from\", \"to\", on_delete, on_update \ FROM pragma_foreign_key_list(?1) \ ORDER BY id, seq;", ) .map_err(DbError::from_rusqlite)?; let fk_rows = fk_stmt .query_map([table], |row| { - let on_delete_str: String = row.get(3)?; - let on_update_str: String = row.get(4)?; - Ok(ReadForeignKey { - parent_table: row.get(0)?, - child_column: row.get(1)?, - parent_column: row.get(2)?, - on_delete: parse_action_from_sqlite(&on_delete_str), - on_update: parse_action_from_sqlite(&on_update_str), - }) + Ok(( + row.get::<_, i64>(0)?, // id — groups a compound FK's columns + row.get::<_, String>(1)?, // parent table + row.get::<_, String>(2)?, // child column (`from`) + row.get::<_, String>(3)?, // parent column (`to`) + parse_action_from_sqlite(&row.get::<_, String>(4)?), + parse_action_from_sqlite(&row.get::<_, String>(5)?), + )) }) .map_err(DbError::from_rusqlite)?; - let mut foreign_keys = Vec::new(); + let mut foreign_keys: Vec = Vec::new(); + let mut current_id: Option = None; for row in fk_rows { - foreign_keys.push(row.map_err(DbError::from_rusqlite)?); + let (id, parent_table, child_col, parent_col, on_delete, on_update) = + row.map_err(DbError::from_rusqlite)?; + if current_id == Some(id) { + let fk = foreign_keys + .last_mut() + .expect("a same-id group always follows its first row"); + fk.child_columns.push(child_col); + fk.parent_columns.push(parent_col); + } else { + current_id = Some(id); + foreign_keys.push(ReadForeignKey { + parent_table, + child_columns: vec![child_col], + parent_columns: vec![parent_col], + on_delete, + on_update, + }); + } } // Table-level CHECK constraints (ADR-0035 §4a.3) come from their @@ -6491,12 +6689,15 @@ fn schema_to_ddl(table: &str, schema: &ReadSchema) -> String { } for fk in &schema.foreign_keys { + // Multi-column FK (ADR-0043): emit the positionally-paired + // column lists `FOREIGN KEY (a, b) REFERENCES P(x, y)`. A + // single-column FK is the one-element case. clauses.push(format!( "FOREIGN KEY ({child}) REFERENCES {parent_table}({parent_col}) \ ON DELETE {od} ON UPDATE {ou}", - child = quote_ident(&fk.child_column), + child = quote_cols(&fk.child_columns), parent_table = quote_ident(&fk.parent_table), - parent_col = quote_ident(&fk.parent_column), + parent_col = quote_cols(&fk.parent_columns), od = fk.on_delete.sql_clause(), ou = fk.on_update.sql_clause(), )); @@ -6680,12 +6881,21 @@ where fn resolve_relationship_name( name: Option<&str>, parent_table: &str, - parent_column: &str, + parent_columns: &[String], child_table: &str, - child_column: &str, + child_columns: &[String], ) -> String { + // Auto-name joins each side's column list with `_` (ADR-0043): + // a compound FK becomes `Parent_a_b_to_Child_x_y`; a + // single-column FK is unchanged (`Parent_a_to_Child_x`). name.map_or_else( - || format!("{parent_table}_{parent_column}_to_{child_table}_{child_column}"), + || { + format!( + "{parent_table}_{p}_to_{child_table}_{c}", + p = parent_columns.join("_"), + c = child_columns.join("_"), + ) + }, ToString::to_string, ) } @@ -6718,9 +6928,9 @@ fn insert_relationship_metadata( conn: &Connection, name: &str, parent_table: &str, - parent_column: &str, + parent_columns: &[String], child_table: &str, - child_column: &str, + child_columns: &[String], on_delete: ReferentialAction, on_update: ReferentialAction, ) -> Result<(), DbError> { @@ -6733,9 +6943,9 @@ fn insert_relationship_metadata( [ name, parent_table, - parent_column, + &encode_rel_columns(parent_columns), child_table, - child_column, + &encode_rel_columns(child_columns), on_delete.keyword(), on_update.keyword(), ], @@ -6744,6 +6954,31 @@ fn insert_relationship_metadata( Ok(()) } +/// Encode a relationship's column list into a metadata `TEXT` cell +/// (ADR-0043 D5): comma-joined. Safe because column identifiers are +/// `[A-Za-z0-9_]+` (no commas — parser.rs); a single-column FK +/// stores just the bare name, a compound one stores `a,b`. +fn encode_rel_columns(cols: &[String]) -> String { + cols.join(",") +} + +/// Inverse of [`encode_rel_columns`]: split a metadata cell back +/// into the ordered column list (one element for single-column). +fn decode_rel_columns(s: &str) -> Vec { + s.split(',').map(str::to_string).collect() +} + +/// Format a relationship endpoint for display (ADR-0043): `T.col` +/// for a single column, `T.(a, b)` for a compound one — mirroring +/// the DSL `from P.(a, b)` syntax. +fn fmt_rel_endpoint(table: &str, cols: &[String]) -> String { + if cols.len() == 1 { + format!("{table}.{}", cols[0]) + } else { + format!("{table}.({})", cols.join(", ")) + } +} + /// Validate that an FK child column's type is compatible with the /// referenced parent column's type — it must equal the parent type's /// `fk_target_type()` (ADR-0011). Engine-neutral mismatch error. @@ -6767,15 +7002,74 @@ fn check_fk_type_compat( Ok(()) } +/// Resolve a foreign key's parent (referenced) column list and check +/// it pairs with the child column list (ADR-0043). +/// +/// - **F-A (full PK):** an *explicit* `REFERENCES P(...)` list must be +/// exactly the parent's primary-key column **set** (any order; +/// paired positionally with the child list). A subset, super-set, +/// or non-PK column is refused (UNIQUE-target / subset FKs are OOS). +/// - **F-D (auto-expand):** a *bare* `REFERENCES P` (`explicit == +/// None`) expands to the parent's full PK in PK order, requiring the +/// child arity to match. +/// - Arity: the returned parent list and the child list must be +/// equal, non-zero length. +fn resolve_fk_parent_columns( + parent_table: &str, + parent_pk: &[String], + explicit: Option<&[String]>, + child_arity: usize, +) -> Result, DbError> { + if child_arity == 0 { + return Err(DbError::Unsupported( + "a foreign key needs at least one column.".to_string(), + )); + } + let parent_columns = match explicit { + None => { + if parent_pk.is_empty() { + return Err(DbError::Unsupported(format!( + "`{parent_table}` has no primary key to reference." + ))); + } + parent_pk.to_vec() + } + Some(cols) => { + use std::collections::BTreeSet; + let pk: BTreeSet<&String> = parent_pk.iter().collect(); + let given: BTreeSet<&String> = cols.iter().collect(); + if pk != given { + return Err(DbError::Unsupported(format!( + "a foreign key must reference `{parent_table}`'s full primary \ + key ({pk_list}); got ({given_list}). Referencing a subset, or \ + a non-primary-key column, is not supported.", + pk_list = parent_pk.join(", "), + given_list = cols.join(", "), + ))); + } + cols.to_vec() + } + }; + if parent_columns.len() != child_arity { + return Err(DbError::Unsupported(format!( + "{child_arity} foreign-key column(s) on the child side, but \ + `{parent_table}`'s key has {n}. A foreign key references every \ + column of the key, paired in order.", + n = parent_columns.len(), + ))); + } + Ok(parent_columns) +} + /// A `CREATE TABLE` foreign key after resolution + validation /// (ADR-0035 §5, sub-phase 4b): the bare-`REFERENCES` parent column is /// resolved, the relationship name is decided, and PK-target / /// type-compat are checked. struct ResolvedFk { name: String, - child_column: String, + child_columns: Vec, parent_table: String, - parent_column: String, + parent_columns: Vec, on_delete: ReferentialAction, on_update: ReferentialAction, } @@ -6817,73 +7111,58 @@ fn resolve_create_table_fks( ) }; - // Explicit referenced column, or the parent's single-column PK - // for the bare `REFERENCES ` form. - let parent_column = match &fk.parent_column { - Some(c) => c.clone(), - None => { - if parent_pk.len() == 1 { - parent_pk[0].clone() - } else { - return Err(DbError::Unsupported(format!( - "`{parent}` has a composite primary key, so a bare \ - reference is ambiguous — name the referenced column, \ - e.g. `REFERENCES {parent}()`.", - parent = fk.parent_table, - ))); - } - } - }; - - // The referenced column must be a primary key (ADR-0011/0013). - if !parent_pk.contains(&parent_column) { - return Err(DbError::Unsupported(format!( - "column `{}.{}` is not a primary key. Foreign keys must \ - reference a primary key (UNIQUE-target FKs land in a later \ - iteration).", - fk.parent_table, parent_column - ))); - } - let parent_type = parent_cols - .iter() - .find(|(n, _)| n == &parent_column) - .and_then(|(_, t)| *t) - .ok_or_else(|| DbError::Sqlite { - message: format!("no such column: {}.{}", fk.parent_table, parent_column), - kind: SqliteErrorKind::NoSuchColumn, - })?; - - // The child column must be one of the columns being defined. - let child = columns - .iter() - .find(|c| c.name == fk.child_column) - .ok_or_else(|| DbError::Sqlite { - message: format!("no such column: {}.{}", table_name, fk.child_column), - kind: SqliteErrorKind::NoSuchColumn, - })?; - check_fk_type_compat( + // Resolve the parent column list: explicit must be the full PK + // set (F-A); bare auto-expands to the PK (F-D). Arity-checked + // against the child column count (ADR-0043). + let parent_columns = resolve_fk_parent_columns( &fk.parent_table, - &parent_column, - parent_type, - table_name, - &fk.child_column, - child.ty, + &parent_pk, + fk.parent_columns.as_deref(), + fk.child_columns.len(), )?; + // Each child column must be one of the columns being defined, + // and each pair must be type-compatible (ADR-0011, per pair). + for (child_col, parent_col) in fk.child_columns.iter().zip(&parent_columns) { + let child = columns + .iter() + .find(|c| &c.name == child_col) + .ok_or_else(|| DbError::Sqlite { + message: format!("no such column: {table_name}.{child_col}"), + kind: SqliteErrorKind::NoSuchColumn, + })?; + let parent_type = parent_cols + .iter() + .find(|(n, _)| n == parent_col) + .and_then(|(_, t)| *t) + .ok_or_else(|| DbError::Sqlite { + message: format!("no such column: {}.{parent_col}", fk.parent_table), + kind: SqliteErrorKind::NoSuchColumn, + })?; + check_fk_type_compat( + &fk.parent_table, + parent_col, + parent_type, + table_name, + child_col, + child.ty, + )?; + } + let resolved_name = resolve_relationship_name( fk.name.as_deref(), &fk.parent_table, - &parent_column, + &parent_columns, table_name, - &fk.child_column, + &fk.child_columns, ); ensure_relationship_name_unique(conn, &resolved_name)?; out.push(ResolvedFk { name: resolved_name, - child_column: fk.child_column.clone(), + child_columns: fk.child_columns.clone(), parent_table: fk.parent_table.clone(), - parent_column, + parent_columns, on_delete: fk.on_delete, on_update: fk.on_update, }); @@ -6898,9 +7177,9 @@ fn do_add_relationship( source: Option<&str>, name: Option<&str>, parent_table: &str, - parent_column: &str, + parent_columns: &[String], child_table: &str, - child_column: &str, + child_columns: &[String], on_delete: ReferentialAction, on_update: ReferentialAction, create_fk: bool, @@ -6915,102 +7194,115 @@ fn do_add_relationship( let parent_table = canonical_parent.as_str(); let canonical_child = require_canonical_table(conn, child_table)?; let child_table = canonical_child.as_str(); - // 1. Read parent schema; verify the referenced column is a PK. + // 1. Read parent schema; resolve+validate the referenced columns + // are the parent's full PK (F-A), arity-matched to the child + // list (ADR-0043). The DSL always supplies explicit columns. let parent_schema = read_schema(conn, parent_table)?; - let parent_col = parent_schema - .columns - .iter() - .find(|c| c.name == parent_column) - .ok_or_else(|| DbError::Sqlite { - message: format!("no such column: {parent_table}.{parent_column}"), - kind: SqliteErrorKind::NoSuchColumn, - })?; - if !parent_col.primary_key { - return Err(DbError::Unsupported(format!( - "column `{parent_table}.{parent_column}` is not a primary key. \ - Foreign keys must reference a primary key (UNIQUE-target FKs \ - land in a later iteration)." - ))); - } + let parent_columns = resolve_fk_parent_columns( + parent_table, + &parent_schema.primary_key, + Some(parent_columns), + child_columns.len(), + )?; - // 2. Read child schema; verify the FK column or auto-create. + // 2. Read child schema; refuse missing columns unless --create-fk. let mut child_schema = read_schema(conn, child_table)?; - let needs_create_column = child_schema - .columns + let missing: Vec<&String> = child_columns .iter() - .all(|c| c.name != child_column); - if needs_create_column && !create_fk { + .filter(|c| child_schema.columns.iter().all(|col| &col.name != *c)) + .collect(); + if !missing.is_empty() && !create_fk { + let list = missing + .iter() + .map(|c| format!("`{child_table}.{c}`")) + .collect::>() + .join(", "); return Err(DbError::Unsupported(format!( - "column `{child_table}.{child_column}` does not exist. \ - Add it first, or use `--create-fk` to create it automatically." + "column(s) {list} do not exist. Add them first, or use \ + `--create-fk` to create them automatically." ))); } - // 3. Determine child column type. Either the existing one's - // user_type, or the parent's fk_target_type for auto-create. - let parent_user_type = parent_col.user_type.ok_or_else(|| DbError::Unsupported( - "parent column has no user type metadata".to_string(), - ))?; - let expected_child_type = parent_user_type.fk_target_type(); + // 3. Per pair: resolve the parent column's user type, then either + // validate the existing child column's type (ADR-0011 per pair) + // or synthesise a new child column typed to the parent's + // fk_target_type (--create-fk, one per missing column). + let mut created: Vec<(String, Type)> = Vec::new(); + for (child_col, parent_col) in child_columns.iter().zip(&parent_columns) { + let pcol = parent_schema + .columns + .iter() + .find(|c| &c.name == parent_col) + .ok_or_else(|| DbError::Sqlite { + message: format!("no such column: {parent_table}.{parent_col}"), + kind: SqliteErrorKind::NoSuchColumn, + })?; + let parent_user_type = pcol.user_type.ok_or_else(|| { + DbError::Unsupported("parent column has no user type metadata".to_string()) + })?; + match child_schema.columns.iter().find(|c| &c.name == child_col) { + Some(child_col_row) => { + let actual = child_col_row.user_type.ok_or_else(|| { + DbError::Unsupported("child column has no user type metadata".to_string()) + })?; + check_fk_type_compat( + parent_table, + parent_col, + parent_user_type, + child_table, + child_col, + actual, + )?; + } + None => created.push((child_col.clone(), parent_user_type.fk_target_type())), + } + } - if needs_create_column { - // Synthesise the column row for the new schema. + // Synthesise the new child columns into the old schema (so rebuild + // builds the new table with them), mirroring the single-column path. + for (col, ty) in &created { child_schema.columns.push(ReadColumn { - name: child_column.to_string(), - sqlite_type: expected_child_type.sqlite_strict_type().to_string(), + name: col.clone(), + sqlite_type: ty.sqlite_strict_type().to_string(), notnull: false, primary_key: false, unique: false, default_sql: None, check: None, - user_type: Some(expected_child_type), + user_type: Some(*ty), }); - } else { - // Validate type compatibility against the existing column. - let child_col = child_schema - .columns - .iter() - .find(|c| c.name == child_column) - .expect("checked above"); - let actual = child_col.user_type.ok_or_else(|| DbError::Unsupported( - "child column has no user type metadata".to_string(), - ))?; - check_fk_type_compat( - parent_table, - parent_column, - parent_user_type, - child_table, - child_column, - actual, - )?; } // 4. Determine relationship name (auto-gen or supplied) and // check uniqueness against the metadata table. - let resolved_name = - resolve_relationship_name(name, parent_table, parent_column, child_table, child_column); + let resolved_name = resolve_relationship_name( + name, + parent_table, + &parent_columns, + child_table, + child_columns, + ); ensure_relationship_name_unique(conn, &resolved_name)?; - // 5. Build the new schema with the FK appended. + // 5. Build the new schema with the (single, multi-column) FK appended. let mut new_schema = child_schema.clone(); new_schema.foreign_keys.push(ReadForeignKey { parent_table: parent_table.to_string(), - parent_column: parent_column.to_string(), - child_column: child_column.to_string(), + parent_columns: parent_columns.clone(), + child_columns: child_columns.to_vec(), on_delete, on_update, }); // 6. Rebuild, with metadata updates inside the transaction. - let column_user_type_kw = expected_child_type.keyword(); rebuild_table(conn, child_table, &child_schema, &new_schema, |tx| { - if needs_create_column { + for (col, ty) in &created { tx.execute( &format!( "INSERT INTO {META_TABLE} (table_name, column_name, user_type) \ VALUES (?1, ?2, ?3);" ), - [child_table, child_column, column_user_type_kw], + [child_table, col.as_str(), ty.keyword()], ) .map_err(DbError::from_rusqlite)?; } @@ -7018,9 +7310,9 @@ fn do_add_relationship( tx, &resolved_name, parent_table, - parent_column, + &parent_columns, child_table, - child_column, + child_columns, on_delete, on_update, )?; @@ -7091,7 +7383,9 @@ fn do_drop_relationship( // Read child schema; build new schema without the FK. let old_schema = read_schema(conn, &child_table)?; let mut new_schema = old_schema.clone(); - new_schema.foreign_keys.retain(|fk| fk.child_column != child_column); + new_schema + .foreign_keys + .retain(|fk| fk.child_columns != decode_rel_columns(&child_column)); let child_table_for_persist = child_table.clone(); rebuild_table(conn, &child_table, &old_schema, &new_schema, |tx| { @@ -7424,37 +7718,35 @@ fn do_alter_add_foreign_key( ) -> Result { reject_internal_table_name(child_table)?; reject_internal_table_name(&fk.parent_table)?; - let parent_column = match &fk.parent_column { - Some(c) => c.clone(), - None => { - let ps = read_schema(conn, &fk.parent_table)?; - if ps.primary_key.len() == 1 { - ps.primary_key[0].clone() - } else { - return Err(DbError::Unsupported(format!( - "`{parent}` has a composite primary key, so a bare reference \ - is ambiguous — name the referenced column, e.g. \ - `REFERENCES {parent}()`.", - parent = fk.parent_table, - ))); - } + // Resolve the parent columns: explicit must be the full PK (F-A); + // a bare `REFERENCES P` auto-expands to the full PK in PK order + // (F-D), arity-matched to the child list (ADR-0043). + let parent_pk = read_schema(conn, &fk.parent_table)?.primary_key; + let parent_columns = resolve_fk_parent_columns( + &fk.parent_table, + &parent_pk, + fk.parent_columns.as_deref(), + fk.child_columns.len(), + )?; + // Every child column must already exist for `ALTER … ADD FOREIGN + // KEY` — there is no SQL spelling to auto-create one (`--create-fk` + // is the simple-mode `add relationship` surface only). Pre-check + // here so the refusal speaks SQL, not the DSL flag (ADR-0035 + // Amendment 1, gap C). A missing child *table* is left to + // `do_add_relationship`'s own "no such table". + if let Ok(child_schema) = read_schema(conn, child_table) { + let missing: Vec<&String> = fk + .child_columns + .iter() + .filter(|c| child_schema.columns.iter().all(|col| &col.name != *c)) + .collect(); + if let Some(child) = missing.first() { + return Err(DbError::Unsupported(format!( + "column `{child_table}.{child}` does not exist — add it first \ + (`alter table {child_table} add column {child} `), then \ + add the foreign key." + ))); } - }; - // The child column must already exist for `ALTER … ADD FOREIGN KEY` — - // there is no SQL spelling to auto-create it (the `--create-fk` option - // is the simple-mode `add relationship` surface only). Pre-check here - // so the refusal speaks SQL, not the DSL flag (ADR-0035 Amendment 1, - // gap C). A missing child *table* is left to `do_add_relationship`'s - // own "no such table". - if let Ok(child_schema) = read_schema(conn, child_table) - && child_schema.columns.iter().all(|c| c.name != fk.child_column) - { - return Err(DbError::Unsupported(format!( - "column `{child_table}.{child}` does not exist — add it first \ - (`alter table {child_table} add column {child} `), then \ - add the foreign key.", - child = fk.child_column, - ))); } do_add_relationship( conn, @@ -7462,9 +7754,9 @@ fn do_alter_add_foreign_key( source, name, &fk.parent_table, - &parent_column, + &parent_columns, child_table, - &fk.child_column, + &fk.child_columns, fk.on_delete, fk.on_update, false, @@ -9415,8 +9707,8 @@ fn read_relationships_outbound( Ok(RelationshipEnd { name: row.get(0)?, other_table: row.get(1)?, - other_column: row.get(2)?, - local_column: row.get(3)?, + other_columns: decode_rel_columns(row.get::<_, String>(2)?.as_str()), + local_columns: decode_rel_columns(row.get::<_, String>(3)?.as_str()), on_delete: on_delete .parse::() .unwrap_or(ReferentialAction::NoAction), @@ -9452,8 +9744,8 @@ fn read_relationships_inbound( Ok(RelationshipEnd { name: row.get(0)?, other_table: row.get(1)?, - other_column: row.get(2)?, - local_column: row.get(3)?, + other_columns: decode_rel_columns(row.get::<_, String>(2)?.as_str()), + local_columns: decode_rel_columns(row.get::<_, String>(3)?.as_str()), on_delete: on_delete .parse::() .unwrap_or(ReferentialAction::NoAction), @@ -9595,12 +9887,14 @@ fn do_rebuild_from_text( )) .map_err(DbError::from_rusqlite)?; for rel in &snapshot.relationships { + let parent_cols = encode_rel_columns(&rel.parent_columns); + let child_cols = encode_rel_columns(&rel.child_columns); stmt.execute([ rel.name.as_str(), rel.parent_table.as_str(), - rel.parent_column.as_str(), + parent_cols.as_str(), rel.child_table.as_str(), - rel.child_column.as_str(), + child_cols.as_str(), rel.on_delete.keyword(), rel.on_update.keyword(), ]) @@ -9731,8 +10025,8 @@ fn build_read_schema(table: &TableSchema, relationships: &[RelationshipSchema]) .filter(|r| r.child_table == table.name) .map(|r| ReadForeignKey { parent_table: r.parent_table.clone(), - parent_column: r.parent_column.clone(), - child_column: r.child_column.clone(), + parent_columns: r.parent_columns.clone(), + child_columns: r.child_columns.clone(), on_delete: r.on_delete, on_update: r.on_update, }) @@ -10385,9 +10679,9 @@ mod tests { db.add_relationship( None, "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "cust_id".to_string(), + vec!["cust_id".to_string()], ReferentialAction::NoAction, ReferentialAction::NoAction, false, @@ -10745,9 +11039,9 @@ mod tests { db.add_relationship( None, "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "cust_id".to_string(), + vec!["cust_id".to_string()], ReferentialAction::NoAction, ReferentialAction::NoAction, false, @@ -10772,7 +11066,7 @@ mod tests { let outbound = orders .outbound_relationships .iter() - .find(|r| r.local_column == "buyer_id"); + .find(|r| r.local_columns == vec!["buyer_id".to_string()]); assert!( outbound.is_some(), "expected outbound rel on `buyer_id`, got {:?}", @@ -10787,7 +11081,7 @@ mod tests { let inbound = customers .inbound_relationships .iter() - .find(|r| r.other_column == "buyer_id"); + .find(|r| r.other_columns == vec!["buyer_id".to_string()]); assert!( inbound.is_some(), "expected inbound rel referencing `buyer_id`, got {:?}", @@ -10905,9 +11199,9 @@ mod tests { db.add_relationship( None, "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "cust_id".to_string(), + vec!["cust_id".to_string()], ReferentialAction::NoAction, ReferentialAction::NoAction, false, @@ -10949,9 +11243,9 @@ mod tests { db.add_relationship( None, "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "cust_id".to_string(), + vec!["cust_id".to_string()], ReferentialAction::NoAction, ReferentialAction::NoAction, false, @@ -11328,9 +11622,9 @@ mod tests { db.add_relationship( None, "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "cust_id".to_string(), + vec!["cust_id".to_string()], ReferentialAction::NoAction, ReferentialAction::NoAction, false, @@ -11637,9 +11931,9 @@ mod tests { db.add_relationship( None, "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "cust_id".to_string(), + vec!["cust_id".to_string()], ReferentialAction::NoAction, ReferentialAction::NoAction, false, @@ -11721,9 +12015,9 @@ mod tests { db.add_relationship( None, "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], ReferentialAction::NoAction, ReferentialAction::NoAction, false, @@ -11733,9 +12027,9 @@ mod tests { let orders = db.describe_table("Orders".to_string(), None).await.unwrap(); assert_eq!(orders.outbound_relationships.len(), 1); let rel = &orders.outbound_relationships[0]; - assert_eq!(rel.local_column, "CustId"); + assert_eq!(rel.local_columns, vec!["CustId".to_string()]); assert_eq!(rel.other_table, "Customers"); - assert_eq!(rel.other_column, "id"); + assert_eq!(rel.other_columns, vec!["id".to_string()]); assert_eq!(rel.name, "Customers_id_to_Orders_CustId"); } @@ -11746,9 +12040,9 @@ mod tests { db.add_relationship( None, "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], ReferentialAction::NoAction, ReferentialAction::NoAction, false, @@ -11758,9 +12052,9 @@ mod tests { let customers = db.describe_table("Customers".to_string(), None).await.unwrap(); assert_eq!(customers.inbound_relationships.len(), 1); let rel = &customers.inbound_relationships[0]; - assert_eq!(rel.local_column, "id"); + assert_eq!(rel.local_columns, vec!["id".to_string()]); assert_eq!(rel.other_table, "Orders"); - assert_eq!(rel.other_column, "CustId"); + assert_eq!(rel.other_columns, vec!["CustId".to_string()]); } #[tokio::test] @@ -11770,9 +12064,9 @@ mod tests { db.add_relationship( Some("cust_orders".to_string()), "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], ReferentialAction::Cascade, ReferentialAction::SetNull, false, @@ -11808,9 +12102,9 @@ mod tests { db.add_relationship( None, "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], ReferentialAction::NoAction, ReferentialAction::NoAction, true, // --create-fk @@ -11851,9 +12145,9 @@ mod tests { .add_relationship( None, "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], ReferentialAction::NoAction, ReferentialAction::NoAction, false, @@ -11889,9 +12183,9 @@ mod tests { .add_relationship( None, "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], ReferentialAction::NoAction, ReferentialAction::NoAction, false, @@ -11932,9 +12226,9 @@ mod tests { .add_relationship( None, "Customers".to_string(), - "Name".to_string(), + vec!["Name".to_string()], "Orders".to_string(), - "CustName".to_string(), + vec!["CustName".to_string()], ReferentialAction::NoAction, ReferentialAction::NoAction, false, @@ -11942,7 +12236,7 @@ mod tests { .await .unwrap_err(); match err { - DbError::Unsupported(msg) => assert!(msg.contains("not a primary key"), "{msg}"), + DbError::Unsupported(msg) => assert!(msg.contains("primary key"), "{msg}"), other => panic!("unexpected error: {other:?}"), } } @@ -11954,9 +12248,9 @@ mod tests { db.add_relationship( Some("cust_orders".to_string()), "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], ReferentialAction::NoAction, ReferentialAction::NoAction, false, @@ -11981,9 +12275,9 @@ mod tests { db.add_relationship( None, "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], ReferentialAction::NoAction, ReferentialAction::NoAction, false, @@ -12009,9 +12303,9 @@ mod tests { db.add_relationship( None, "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], ReferentialAction::NoAction, ReferentialAction::NoAction, false, @@ -12034,9 +12328,9 @@ mod tests { db.add_relationship( None, "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], ReferentialAction::NoAction, ReferentialAction::NoAction, false, @@ -12060,9 +12354,9 @@ mod tests { db.add_relationship( Some("dup".to_string()), "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], ReferentialAction::NoAction, ReferentialAction::NoAction, false, @@ -12073,9 +12367,9 @@ mod tests { .add_relationship( Some("dup".to_string()), "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "OtherCust".to_string(), + vec!["OtherCust".to_string()], ReferentialAction::NoAction, ReferentialAction::NoAction, false, @@ -12100,9 +12394,9 @@ mod tests { db.add_relationship( None, "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], ReferentialAction::Cascade, ReferentialAction::NoAction, false, @@ -13330,9 +13624,9 @@ mod tests { db.add_relationship( None, "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], ReferentialAction::NoAction, ReferentialAction::NoAction, false, @@ -13373,9 +13667,9 @@ mod tests { db.add_relationship( None, "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], ReferentialAction::Cascade, ReferentialAction::NoAction, false, @@ -13428,9 +13722,9 @@ mod tests { db.add_relationship( Some("parent_of".to_string()), "T".to_string(), - "id".to_string(), + vec!["id".to_string()], "T".to_string(), - "ParentId".to_string(), + vec!["ParentId".to_string()], ReferentialAction::Cascade, ReferentialAction::NoAction, false, @@ -13940,9 +14234,9 @@ mod tests { db.add_relationship( Some("cust_orders".to_string()), "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], crate::dsl::ReferentialAction::NoAction, crate::dsl::ReferentialAction::NoAction, false, diff --git a/src/dsl/command.rs b/src/dsl/command.rs index 805028a..a5b6b07 100644 --- a/src/dsl/command.rs +++ b/src/dsl/command.rs @@ -29,15 +29,20 @@ pub struct SqlForeignKey { /// FK or an unnamed table FK (auto-named at execution per /// ADR-0013). pub name: Option, - /// The column in the table being created that holds the FK. - pub child_column: String, + /// The column(s) in the table being created that hold the FK. + /// One element for a single-column FK; ordered list for a + /// compound FK (ADR-0043). Positionally paired with + /// `parent_columns`. + pub child_columns: Vec, /// The referenced (parent) table — may be the table being created /// (a self-referencing FK). pub parent_table: String, - /// The referenced parent column. `None` for the bare - /// `REFERENCES ` form, resolved at execution to the - /// parent's single-column primary key (ADR-0035 §4b, user-confirmed). - pub parent_column: Option, + /// The referenced parent column(s), positionally paired with + /// `child_columns`. `None` for the bare `REFERENCES ` + /// form, resolved at execution to the parent's primary key — + /// the single-column PK, or (ADR-0043 F-D) the full compound PK + /// when the child arity matches. + pub parent_columns: Option>, pub on_delete: ReferentialAction, pub on_update: ReferentialAction, } @@ -253,9 +258,14 @@ pub enum Command { AddRelationship { name: Option, parent_table: String, - parent_column: String, + /// Parent (referenced) PK column(s); one element for a + /// single-column FK, ordered list for a compound FK + /// (ADR-0043). Positionally paired with `child_columns`. + parent_columns: Vec, child_table: String, - child_column: String, + /// Child (referencing) column(s), positionally paired with + /// `parent_columns`; equal, non-zero length. + child_columns: Vec, on_delete: ReferentialAction, on_update: ReferentialAction, create_fk: bool, @@ -332,6 +342,17 @@ pub enum Command { ShowTable { name: String, }, + /// Re-display a schema collection in the output (V5/V5a). With + /// `name: None`, the whole collection — every table, + /// relationship, or index (the list-all forms). With + /// `name: Some(_)`, one named item's detail — `show + /// relationship ` / `show index ` (V5a); the table + /// singular is the separate `ShowTable`, so a named `Tables` + /// never occurs. Read-only; pure display, no schema change. + ShowList { + kind: ShowListKind, + name: Option, + }, /// Insert a single row. `columns` is `None` for the natural- /// order short form (`insert into T values (...)`); the /// executor fills in the column list by walking the schema. @@ -485,8 +506,14 @@ pub enum Command { pub enum AppCommand { /// Exit cleanly. Accepts the `q` alias. Quit, - /// Show in-app help. Body comes from `help.in_app_body`. - Help, + /// Show in-app help (H3). With no `topic`, the full command + /// list + types reference; with a `topic` (a command entry + /// word like `insert` / `create` / `show`, or `types`), the + /// focused detail for that command (or command group sharing + /// the entry word). + Help { + topic: Option, + }, /// Rebuild `playground.db` from `project.yaml` + data/, with /// confirmation modal. Rebuild, @@ -746,6 +773,36 @@ pub enum IndexSelector { Columns { table: String, columns: Vec }, } +/// Which schema collection a `show ` list command displays (V5). +/// +/// The bare plural forms list every item of the kind across the +/// project; the singular `show table ` (a separate +/// `Command::ShowTable`) shows one. The singular `show +/// relationship ` / `show index ` forms are not yet +/// provided — only the list-all forms land here. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ShowListKind { + /// `show tables` — every user table (internal `__rdbms_*` + /// tables excluded, as in the items panel). + Tables, + /// `show relationships` — every declared FK relationship. + Relationships, + /// `show indexes` — every index across all tables. + Indexes, +} + +impl ShowListKind { + /// The full command name for the `name()` / echo surface. + #[must_use] + pub const fn command_name(self) -> &'static str { + match self { + Self::Tables => "show tables", + Self::Relationships => "show relationships", + Self::Indexes => "show indexes", + } + } +} + /// The action of an advanced-mode `ALTER TABLE` (ADR-0035 §4). /// /// Sub-phase 4e carries the column actions; 4f adds `AlterColumnType`; @@ -860,6 +917,17 @@ impl Command { Self::AddConstraint { .. } => "add constraint", Self::DropConstraint { .. } => "drop constraint", Self::ShowTable { .. } => "show table", + // A named item reports the singular verb; the list-all + // forms report the plural (`kind.command_name()`). + Self::ShowList { + kind: ShowListKind::Relationships, + name: Some(_), + } => "show relationship", + Self::ShowList { + kind: ShowListKind::Indexes, + name: Some(_), + } => "show index", + Self::ShowList { kind, .. } => kind.command_name(), Self::Insert { .. } => "insert into", Self::Update { .. } => "update", Self::Delete { .. } => "delete from", @@ -872,7 +940,7 @@ impl Command { Self::SqlDelete { .. } => "delete from", Self::App(app) => match app { AppCommand::Quit => "quit", - AppCommand::Help => "help", + AppCommand::Help { .. } => "help", AppCommand::Rebuild => "rebuild", AppCommand::Save => "save", AppCommand::SaveAs => "save as", @@ -948,6 +1016,10 @@ impl Command { // result renders as a data view, not a structure // view, so an empty target is correct here. Self::Select { .. } => "", + // A `show ` list spans every table (or none) — + // there is no single structure-target table; it renders + // as a list, not a structure view. + Self::ShowList { .. } => "", // A SQL `INSERT` carries its parsed target table (for // CSV re-persistence and ok-summary subject). Self::SqlInsert { target_table, .. } @@ -970,11 +1042,26 @@ impl Command { match self { Self::AddRelationship { parent_table, - parent_column, + parent_columns, child_table, - child_column, + child_columns, .. - } => format!("from {parent_table}.{parent_column} to {child_table}.{child_column}"), + } => { + // `from P.col to C.col` (single) or `from P.(a, b) to + // C.(x, y)` (compound — ADR-0043), mirroring the DSL. + let fmt = |cols: &[String]| { + if cols.len() == 1 { + cols[0].clone() + } else { + format!("({})", cols.join(", ")) + } + }; + format!( + "from {parent_table}.{} to {child_table}.{}", + fmt(parent_columns), + fmt(child_columns), + ) + } Self::DropRelationship { selector } => match selector { RelationshipSelector::Named { name } => name.clone(), RelationshipSelector::Endpoints { diff --git a/src/dsl/grammar/app.rs b/src/dsl/grammar/app.rs index a40254a..8bc4361 100644 --- a/src/dsl/grammar/app.rs +++ b/src/dsl/grammar/app.rs @@ -80,6 +80,11 @@ const IMPORT_PATH_AND_TARGET: Node = Node::Seq(&[Node::BarePath, IMPORT_AS_TARGE const EXPORT_PATH_OPT: Node = Node::Optional(&Node::BarePath); const IMPORT_BODY_OPT: Node = Node::Optional(&IMPORT_PATH_AND_TARGET); +// `help []` (H3): an optional single-word topic — a command +// entry word (`insert`, `create`, `show`, …) or `types`. Captured +// as a `BarePath` so any keyword-shaped word is accepted verbatim. +const HELP_TOPIC_OPT: Node = Node::Optional(&Node::BarePath); + // `mode `: known keywords are surfaced as `Word` children // so they appear in the walker's expected set (and feed the // completion engine's keyword candidates). The trailing `Ident` @@ -154,8 +159,15 @@ const fn build_quit(_path: &MatchedPath, _source: &str) -> Result Result { - Ok(Command::App(AppCommand::Help)) +fn build_help(path: &MatchedPath, _source: &str) -> Result { + // Optional single-word topic (a command entry word, or + // `types`) captured as a `BarePath` — `help insert`, + // `help create`, `help show`. Multi-word commands share an + // entry word, so `help create` covers every create form. + let topic = path + .find(|i| matches!(i.kind, MatchedKind::BarePath)) + .map(|i| i.text.clone()); + Ok(Command::App(AppCommand::Help { topic })) } const fn build_rebuild(_path: &MatchedPath, _source: &str) -> Result { @@ -255,7 +267,7 @@ pub static QUIT: CommandNode = CommandNode { pub static HELP: CommandNode = CommandNode { entry: Word::keyword("help"), - shape: EMPTY_SEQ, + shape: HELP_TOPIC_OPT, ast_builder: build_help, help_id: Some("app.help"), usage_ids: &["parse.usage.help"],}; diff --git a/src/dsl/grammar/data.rs b/src/dsl/grammar/data.rs index c33e0ca..b6a8a34 100644 --- a/src/dsl/grammar/data.rs +++ b/src/dsl/grammar/data.rs @@ -24,7 +24,7 @@ //! later swap that capture for the same typed slots used here, adding //! live hints/highlighting. -use crate::dsl::command::{Command, Expr, RowFilter}; +use crate::dsl::command::{Command, Expr, RowFilter, ShowListKind}; use crate::dsl::grammar::{ CommandNode, IdentSource, Node, NumberValidator, ValidationError, Word, expr, shared::{ @@ -99,7 +99,62 @@ const SHOW_TABLE_NODES: &[Node] = &[ ]; const SHOW_TABLE: Node = Node::Seq(SHOW_TABLE_NODES); -const SHOW_CHOICES: &[Node] = &[SHOW_DATA, SHOW_TABLE]; +// `show tables` / `show relationships` / `show indexes` — the +// list-all forms (V5). Each is a single keyword with no argument; +// the executor lists every item of the kind. Distinct keyword +// tokens (`tables` ≠ `table`), so Choice ordering is irrelevant. +const SHOW_TABLES: Node = Node::Word(Word::keyword("tables")); +const SHOW_RELATIONSHIPS: Node = Node::Word(Word::keyword("relationships")); +const SHOW_INDEXES: Node = Node::Word(Word::keyword("indexes")); + +// `show relationship ` / `show index ` — singular +// per-item detail (V5a). The name slot reuses the existing +// completion sources (relationship / index names). Distinct +// keyword tokens from the plurals (`relationship` ≠ +// `relationships`), so Choice ordering is irrelevant. +const SHOW_RELATIONSHIP_NAME: Node = Node::Ident { + source: IdentSource::Relationships, + role: "relationship_name", + validator: None, + highlight_override: None, + writes_table: false, + writes_column: false, + writes_user_listed_column: false, + writes_table_alias: false, + writes_cte_name: false, + writes_projection_alias: false, +}; +const SHOW_RELATIONSHIP_NODES: &[Node] = &[ + Node::Word(Word::keyword("relationship")), + SHOW_RELATIONSHIP_NAME, +]; +const SHOW_RELATIONSHIP: Node = Node::Seq(SHOW_RELATIONSHIP_NODES); + +const SHOW_INDEX_NAME: Node = Node::Ident { + source: IdentSource::Indexes, + role: "index_name", + validator: None, + highlight_override: None, + writes_table: false, + writes_column: false, + writes_user_listed_column: false, + writes_table_alias: false, + writes_cte_name: false, + writes_projection_alias: false, +}; +const SHOW_INDEX_NODES: &[Node] = + &[Node::Word(Word::keyword("index")), SHOW_INDEX_NAME]; +const SHOW_INDEX: Node = Node::Seq(SHOW_INDEX_NODES); + +const SHOW_CHOICES: &[Node] = &[ + SHOW_DATA, + SHOW_TABLE, + SHOW_TABLES, + SHOW_RELATIONSHIPS, + SHOW_INDEXES, + SHOW_RELATIONSHIP, + SHOW_INDEX, +]; const SHOW_SHAPE: Node = Node::Choice(SHOW_CHOICES); // ================================================================= @@ -552,10 +607,35 @@ fn build_show(path: &MatchedPath, _source: &str) -> Result None, }) .nth(1); - let name = require_ident(path, "table_name")?; match sub { Some("data") => build_show_data(path, _source), - Some("table") => Ok(Command::ShowTable { name }), + // `name` is resolved only for the forms that carry one; the + // list-all forms (`tables` / `relationships` / `indexes`) + // have no table argument. + Some("table") => Ok(Command::ShowTable { + name: require_ident(path, "table_name")?, + }), + Some("tables") => Ok(Command::ShowList { + kind: ShowListKind::Tables, + name: None, + }), + Some("relationships") => Ok(Command::ShowList { + kind: ShowListKind::Relationships, + name: None, + }), + Some("indexes") => Ok(Command::ShowList { + kind: ShowListKind::Indexes, + name: None, + }), + // V5a singular per-item detail — carry the named item. + Some("relationship") => Ok(Command::ShowList { + kind: ShowListKind::Relationships, + name: Some(require_ident(path, "relationship_name")?), + }), + Some("index") => Ok(Command::ShowList { + kind: ShowListKind::Indexes, + name: Some(require_ident(path, "index_name")?), + }), _ => Err(ValidationError { message_key: "parse.error_wrapper", args: vec![("detail", "unknown show subcommand".to_string())], @@ -1362,7 +1442,15 @@ pub static SHOW: CommandNode = CommandNode { shape: SHOW_SHAPE, ast_builder: build_show, help_id: Some("data.show"), - usage_ids: &["parse.usage.show_data", "parse.usage.show_table"],}; + usage_ids: &[ + "parse.usage.show_data", + "parse.usage.show_table", + "parse.usage.show_tables", + "parse.usage.show_relationships", + "parse.usage.show_indexes", + "parse.usage.show_relationship", + "parse.usage.show_index", + ],}; pub static INSERT: CommandNode = CommandNode { entry: Word::keyword("insert"), @@ -1445,7 +1533,7 @@ pub static WITH: CommandNode = CommandNode { shape: Node::Subgrammar(&sql_select::SQL_WITH_TAIL), ast_builder: build_select, help_id: None, - usage_ids: &["parse.usage.select"],}; + usage_ids: &["parse.usage.with"],}; /// SQL `INSERT` — the `Advanced`-category node of the shared /// `insert` entry word (ADR-0033 §2, Amendment 1, sub-phase 3j). diff --git a/src/dsl/grammar/ddl.rs b/src/dsl/grammar/ddl.rs index 7c16ca8..35b15b5 100644 --- a/src/dsl/grammar/ddl.rs +++ b/src/dsl/grammar/ddl.rs @@ -385,6 +385,36 @@ const ADD_COLUMN: Node = Node::Seq(ADD_COLUMN_NODES); // `writes_table: true` on each endpoint's table ident so the // `.` slot narrows to that table's columns (handoff-13 // §2.2 follow-up — mirrors DR_PARENT / DR_CHILD). +// A single FK-endpoint column ident (narrows to the endpoint +// table's columns via the table ident's `writes_table: true`). +const AR_PARENT_COL: Node = Node::Ident { + source: IdentSource::Columns, + role: "parent_column", + validator: None, + highlight_override: None, + writes_table: false, + writes_column: false, + writes_user_listed_column: false, + writes_table_alias: false, + writes_cte_name: false, + writes_projection_alias: false, +}; +// Compound endpoint: `( a, b, … )` — a comma-separated column list +// in parens (ADR-0043). Same role as the single form, so the +// builder collects either shape uniformly. +const AR_PARENT_COL_LIST: Node = Node::Repeated { + inner: &AR_PARENT_COL, + separator: Some(&Node::Punct(',')), + min: 1, +}; +const AR_PARENT_COLS_PAREN_NODES: &[Node] = + &[Node::Punct('('), AR_PARENT_COL_LIST, Node::Punct(')')]; +const AR_PARENT_COLS_PAREN: Node = Node::Seq(AR_PARENT_COLS_PAREN_NODES); +// `from P.(a, b)` (compound) or `from P.col` (single) — Choice on +// the first post-`.` token (`(` vs an ident), so order is safe. +const AR_PARENT_COLS_CHOICES: &[Node] = &[AR_PARENT_COLS_PAREN, AR_PARENT_COL]; +const AR_PARENT_COLS: Node = Node::Choice(AR_PARENT_COLS_CHOICES); + const AR_PARENT_NODES: &[Node] = &[ Node::Ident { source: IdentSource::Tables, @@ -394,25 +424,37 @@ const AR_PARENT_NODES: &[Node] = &[ writes_table: true, writes_column: false, writes_user_listed_column: false, - writes_table_alias: false, - writes_cte_name: false, - writes_projection_alias: false, + writes_table_alias: false, + writes_cte_name: false, + writes_projection_alias: false, }, Node::Punct('.'), - Node::Ident { - source: IdentSource::Columns, - role: "parent_column", - validator: None, - highlight_override: None, - writes_table: false, - writes_column: false, - writes_user_listed_column: false, + AR_PARENT_COLS, +]; +const AR_PARENT: Node = Node::Seq(AR_PARENT_NODES); + +const AR_CHILD_COL: Node = Node::Ident { + source: IdentSource::Columns, + role: "child_column", + validator: None, + highlight_override: None, + writes_table: false, + writes_column: false, + writes_user_listed_column: false, writes_table_alias: false, writes_cte_name: false, writes_projection_alias: false, - }, -]; -const AR_PARENT: Node = Node::Seq(AR_PARENT_NODES); +}; +const AR_CHILD_COL_LIST: Node = Node::Repeated { + inner: &AR_CHILD_COL, + separator: Some(&Node::Punct(',')), + min: 1, +}; +const AR_CHILD_COLS_PAREN_NODES: &[Node] = + &[Node::Punct('('), AR_CHILD_COL_LIST, Node::Punct(')')]; +const AR_CHILD_COLS_PAREN: Node = Node::Seq(AR_CHILD_COLS_PAREN_NODES); +const AR_CHILD_COLS_CHOICES: &[Node] = &[AR_CHILD_COLS_PAREN, AR_CHILD_COL]; +const AR_CHILD_COLS: Node = Node::Choice(AR_CHILD_COLS_CHOICES); const AR_CHILD_NODES: &[Node] = &[ Node::Ident { @@ -423,23 +465,12 @@ const AR_CHILD_NODES: &[Node] = &[ writes_table: true, writes_column: false, writes_user_listed_column: false, - writes_table_alias: false, - writes_cte_name: false, - writes_projection_alias: false, + writes_table_alias: false, + writes_cte_name: false, + writes_projection_alias: false, }, Node::Punct('.'), - Node::Ident { - source: IdentSource::Columns, - role: "child_column", - validator: None, - highlight_override: None, - writes_table: false, - writes_column: false, - writes_user_listed_column: false, - writes_table_alias: false, - writes_cte_name: false, - writes_projection_alias: false, - }, + AR_CHILD_COLS, ]; const AR_CHILD: Node = Node::Seq(AR_CHILD_NODES); @@ -785,12 +816,24 @@ fn build_add_relationship(path: &MatchedPath, _source: &str) -> Result Result { + // Inline FK is single-column (the column it sits on); + // a compound FK uses the table-level form (ADR-0043 D4). let child_column = columns.last().map_or_else(String::new, |c| c.name.clone()); - foreign_keys.push(consume_fk_reference(&mut items, None, child_column)); + foreign_keys.push(consume_fk_reference(&mut items, None, vec![child_column])); } // Table-level `[constraint ] foreign key () // references [()] [on …]` (ADR-0035 §5, 4b). @@ -1520,11 +1565,21 @@ fn build_sql_create_table(path: &MatchedPath, source: &str) -> Result )` + // `( [, ]* )` — a compound + // FK lists multiple child columns (ADR-0043). if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Punct('('))) { items.next(); } - let child_column = items.next().map_or_else(String::new, |it| it.text.clone()); + let mut child_columns = Vec::new(); + while let Some(it) = items.peek() { + match &it.kind { + MatchedKind::Punct(')') => break, + MatchedKind::Punct(',') => { + items.next(); + } + _ => child_columns.push(items.next().expect("peeked").text.clone()), + } + } if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Punct(')'))) { items.next(); } @@ -1532,7 +1587,7 @@ fn build_sql_create_table(path: &MatchedPath, source: &str) -> Result( items: &mut std::iter::Peekable, name: Option, - child_column: String, + child_columns: Vec, ) -> SqlForeignKey where I: Iterator, { let parent_table = items.next().map_or_else(String::new, |it| it.text.clone()); - // Optional `( )`. - let mut parent_column = None; - if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Punct('('))) { - items.next(); // `(` - if let Some(it) = items.next() { - parent_column = Some(it.text.clone()); - } - if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Punct(')'))) { - items.next(); // `)` - } - } + // Optional `( [, ]* )` — a + // compound FK references multiple parent columns (ADR-0043). + // `None` for the bare `REFERENCES ` form. + let parent_columns: Option> = + if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Punct('('))) { + items.next(); // `(` + let mut cols = Vec::new(); + while let Some(it) = items.peek() { + match &it.kind { + MatchedKind::Punct(')') => break, + MatchedKind::Punct(',') => { + items.next(); + } + _ => cols.push(items.next().expect("peeked").text.clone()), + } + } + if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Punct(')'))) { + items.next(); // `)` + } + Some(cols) + } else { + None + }; // `on ` clauses, in either order, 0..2. let mut on_delete = ReferentialAction::default_action(); let mut on_update = ReferentialAction::default_action(); @@ -1680,9 +1747,9 @@ where } SqlForeignKey { name, - child_column, + child_columns, parent_table, - parent_column, + parent_columns, on_delete, on_update, } @@ -2370,14 +2437,24 @@ fn build_alter_fk(path: &MatchedPath) -> SqlForeignKey { if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Punct('('))) { items.next(); } - let child_column = items.next().map_or_else(String::new, |it| it.text.clone()); + // `( [, ]* )` — compound FK (ADR-0043). + let mut child_columns = Vec::new(); + while let Some(it) = items.peek() { + match &it.kind { + MatchedKind::Punct(')') => break, + MatchedKind::Punct(',') => { + items.next(); + } + _ => child_columns.push(items.next().expect("peeked").text.clone()), + } + } if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Punct(')'))) { items.next(); } if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Word("references"))) { items.next(); } - consume_fk_reference(&mut items, None, child_column) + consume_fk_reference(&mut items, None, child_columns) } pub static SQL_ALTER_TABLE: CommandNode = CommandNode { @@ -3202,9 +3279,9 @@ mod sql_alter_table_tests { assert_eq!(name, None); match *constraint { TableConstraint::ForeignKey(fk) => { - assert_eq!(fk.child_column, "pid"); + assert_eq!(fk.child_columns, vec!["pid".to_string()]); assert_eq!(fk.parent_table, "P"); - assert_eq!(fk.parent_column.as_deref(), Some("id")); + assert_eq!(fk.parent_columns, Some(vec!["id".to_string()])); } other => panic!("expected ForeignKey, got {other:?}"), } @@ -3216,7 +3293,7 @@ mod sql_alter_table_tests { assert_eq!(name.as_deref(), Some("fk_p")); match *constraint { TableConstraint::ForeignKey(fk) => { - assert_eq!(fk.parent_column, None, "bare reference resolves at execution"); + assert_eq!(fk.parent_columns, None, "bare reference resolves at execution"); } other => panic!("expected ForeignKey, got {other:?}"), } diff --git a/src/dsl/grammar/mod.rs b/src/dsl/grammar/mod.rs index f39fdc5..d68fc3b 100644 --- a/src/dsl/grammar/mod.rs +++ b/src/dsl/grammar/mod.rs @@ -533,13 +533,89 @@ pub struct CommandNode { /// Returns the canonical (primary-form) entry literal and the /// `usage_ids` list, or `None` if no entry word matches. #[must_use] -pub fn usage_keys_for_input(source: &str) -> Option<(&'static str, &'static [&'static str])> { +pub fn usage_keys_for_input(source: &str) -> Option<(&'static str, Vec<&'static str>)> { + usage_keys_for_input_in_mode(source, crate::mode::Mode::Simple) +} + +/// Mode-aware variant of [`usage_keys_for_input`] (ADR-0042 G3). +/// +/// A shared entry word (`create`, `drop`, `insert`, …) registers a +/// `Simple` DSL node *and* one or more `Advanced` SQL nodes. The +/// usage block must reflect the surface the user is actually typing: +/// the SQL forms in `Advanced` mode, the DSL forms in `Simple` mode +/// — otherwise advanced-mode `create` shows the DSL `create table … +/// with pk …` template, which is not valid SQL. +/// +/// Selection prefers candidates whose [`CommandCategory`] matches +/// the mode; if the entry word has none in that category (an +/// app-lifecycle command is `Simple`-only yet usable in both modes), +/// every candidate is used. The returned keys are the union of the +/// selected nodes' `usage_ids`, de-duplicated in registry order — so +/// advanced `create` shows both `sql_create_table` and +/// `sql_create_index`. +#[must_use] +pub fn usage_keys_for_input_in_mode( + source: &str, + mode: crate::mode::Mode, +) -> Option<(&'static str, Vec<&'static str>)> { use crate::dsl::walker::lex_helpers::{consume_ident, skip_whitespace}; let start = skip_whitespace(source, 0); let (kw_start, kw_end) = consume_ident(source, start)?; let word = &source[kw_start..kw_end]; - let (_, node) = command_for_entry_word(word)?; - Some((node.entry.primary, node.usage_ids)) + let candidates = commands_for_entry_word(word); + if candidates.is_empty() { + return None; + } + let union = |nodes: &[(usize, &'static CommandNode, CommandCategory)]| -> Vec<&'static str> { + let mut keys: Vec<&'static str> = Vec::new(); + for (_, node, _) in nodes { + for k in node.usage_ids { + if !keys.contains(k) { + keys.push(*k); + } + } + } + keys + }; + // Advanced mode: every candidate form is reachable — the SQL + // nodes are primary, and the DSL nodes remain valid via fallback + // (verified: `create table … with pk` and `drop column …` both + // run in advanced mode). Show them all, mode-primary (Advanced) + // first, so the usage hint never hides input that works. Simple + // mode: only the DSL forms — the SQL-only forms hit the "this is + // SQL" rail and are not reachable. (ADR-0042 G3.) + let selected: Vec<(usize, &'static CommandNode, CommandCategory)> = + if mode == crate::mode::Mode::Advanced { + let mut v: Vec<_> = candidates + .iter() + .copied() + .filter(|(_, _, c)| *c == CommandCategory::Advanced) + .collect(); + v.extend( + candidates + .iter() + .copied() + .filter(|(_, _, c)| *c != CommandCategory::Advanced), + ); + v + } else { + candidates + .iter() + .copied() + .filter(|(_, _, c)| *c == CommandCategory::Simple) + .collect() + }; + // Degenerate guard: an advanced-only word in simple mode (not + // normally reachable — it hits the SQL rail first) leaves + // `selected` empty; fall back to all candidates so a usage block + // still renders rather than the available-commands fallback. + let pick = if selected.is_empty() { candidates } else { selected }; + let keys = union(&pick); + if keys.is_empty() { + return None; + } + let entry = pick[0].1.entry.primary; + Some((entry, keys)) } /// The single usage template most relevant to `source`, when @@ -555,8 +631,19 @@ pub fn usage_keys_for_input(source: &str) -> Option<(&'static str, &'static [&'s /// show the whole family or nothing. #[must_use] pub fn usage_key_for_input(source: &str) -> Option<&'static str> { + usage_key_for_input_in_mode(source, crate::mode::Mode::Simple) +} + +/// Mode-aware variant of [`usage_key_for_input`] (ADR-0042 G3) — +/// disambiguates the single most-relevant usage key from the +/// mode-selected key set. +#[must_use] +pub fn usage_key_for_input_in_mode( + source: &str, + mode: crate::mode::Mode, +) -> Option<&'static str> { use crate::dsl::walker::lex_helpers::{consume_ident, skip_whitespace}; - let (_entry, keys) = usage_keys_for_input(source)?; + let (_entry, keys) = usage_keys_for_input_in_mode(source, mode)?; let first = *keys.first()?; if keys.len() == 1 { return Some(first); diff --git a/src/dsl/grammar/sql_create_table.rs b/src/dsl/grammar/sql_create_table.rs index 0cfb9f7..1a09883 100644 --- a/src/dsl/grammar/sql_create_table.rs +++ b/src/dsl/grammar/sql_create_table.rs @@ -182,7 +182,15 @@ const FK_PARENT_COLUMN: Node = Node::Ident { writes_cte_name: false, writes_projection_alias: false, }; -static FK_PARENT_COL_NODES: &[Node] = &[Node::Punct('('), FK_PARENT_COLUMN, Node::Punct(')')]; +// `( a [, b]* )` — a compound FK references multiple parent columns +// (ADR-0043). The `Repeated` separator handles the commas; a +// single-column FK is the one-element case. +const FK_PARENT_COL_LIST: Node = Node::Repeated { + inner: &FK_PARENT_COLUMN, + separator: Some(&Node::Punct(',')), + min: 1, +}; +static FK_PARENT_COL_NODES: &[Node] = &[Node::Punct('('), FK_PARENT_COL_LIST, Node::Punct(')')]; const FK_PARENT_COL_OPT: Node = Node::Optional(&Node::Seq(FK_PARENT_COL_NODES)); // `REFERENCES [ ( ) ] [on delete/update …]` — the inline @@ -333,6 +341,13 @@ const FK_CHILD_COLUMN: Node = Node::Ident { writes_cte_name: false, writes_projection_alias: false, }; +// `( a [, b]* )` — a compound FK lists multiple child columns +// (ADR-0043); single-column is the one-element case. +const FK_CHILD_COL_LIST: Node = Node::Repeated { + inner: &FK_CHILD_COLUMN, + separator: Some(&Node::Punct(',')), + min: 1, +}; const FK_NAME: Node = Node::Ident { source: IdentSource::NewName, role: "fk_name", @@ -354,7 +369,7 @@ static FOREIGN_KEY_BODY_NODES: &[Node] = &[ Node::Word(Word::keyword("foreign")), Node::Word(Word::keyword("key")), Node::Punct('('), - FK_CHILD_COLUMN, + FK_CHILD_COL_LIST, Node::Punct(')'), Node::Word(Word::keyword("references")), FK_PARENT_TABLE, @@ -374,7 +389,7 @@ static TABLE_FK_NAMED_NODES: &[Node] = &[ Node::Word(Word::keyword("foreign")), Node::Word(Word::keyword("key")), Node::Punct('('), - FK_CHILD_COLUMN, + FK_CHILD_COL_LIST, Node::Punct(')'), Node::Word(Word::keyword("references")), FK_PARENT_TABLE, @@ -984,9 +999,9 @@ mod builder_tests { assert_eq!(fks.len(), 1); let fk = &fks[0]; assert_eq!(fk.name, None, "inline FK is auto-named at execution"); - assert_eq!(fk.child_column, "pid"); + assert_eq!(fk.child_columns, vec!["pid".to_string()]); assert_eq!(fk.parent_table, "parent"); - assert_eq!(fk.parent_column.as_deref(), Some("id")); + assert_eq!(fk.parent_columns, Some(vec!["id".to_string()])); assert_eq!(fk.on_delete, ReferentialAction::NoAction); assert_eq!(fk.on_update, ReferentialAction::NoAction); } @@ -994,9 +1009,9 @@ mod builder_tests { #[test] fn bare_inline_reference_has_no_parent_column() { let fks = parse_sct_fks("create table t (id int, pid int references parent)"); - assert_eq!(fks[0].parent_column, None, "bare REFERENCES — resolved at execution"); + assert_eq!(fks[0].parent_columns, None, "bare REFERENCES — resolved at execution"); assert_eq!(fks[0].parent_table, "parent"); - assert_eq!(fks[0].child_column, "pid"); + assert_eq!(fks[0].child_columns, vec!["pid".to_string()]); } #[test] @@ -1026,9 +1041,9 @@ mod builder_tests { parse_sct_fks("create table t (id int, pid int, foreign key (pid) references parent(id))"); assert_eq!(fks.len(), 1); assert_eq!(fks[0].name, None); - assert_eq!(fks[0].child_column, "pid"); + assert_eq!(fks[0].child_columns, vec!["pid".to_string()]); assert_eq!(fks[0].parent_table, "parent"); - assert_eq!(fks[0].parent_column.as_deref(), Some("id")); + assert_eq!(fks[0].parent_columns, Some(vec!["id".to_string()])); } #[test] @@ -1038,7 +1053,7 @@ mod builder_tests { constraint fk_parent foreign key (pid) references parent(id))", ); assert_eq!(fks[0].name.as_deref(), Some("fk_parent")); - assert_eq!(fks[0].child_column, "pid"); + assert_eq!(fks[0].child_columns, vec!["pid".to_string()]); } #[test] @@ -1048,8 +1063,8 @@ mod builder_tests { foreign key (a) references p(id), foreign key (b) references q(id))", ); assert_eq!(fks.len(), 2); - assert_eq!((fks[0].child_column.as_str(), fks[0].parent_table.as_str()), ("a", "p")); - assert_eq!((fks[1].child_column.as_str(), fks[1].parent_table.as_str()), ("b", "q")); + assert_eq!((fks[0].child_columns[0].as_str(), fks[0].parent_table.as_str()), ("a", "p")); + assert_eq!((fks[1].child_columns[0].as_str(), fks[1].parent_table.as_str()), ("b", "q")); } #[test] @@ -1057,8 +1072,8 @@ mod builder_tests { let fks = parse_sct_fks("create table emp (id int primary key, mgr int references emp(id))"); assert_eq!(fks[0].parent_table, "emp", "self-reference"); - assert_eq!(fks[0].child_column, "mgr"); - assert_eq!(fks[0].parent_column.as_deref(), Some("id")); + assert_eq!(fks[0].child_columns, vec!["mgr".to_string()]); + assert_eq!(fks[0].parent_columns, Some(vec!["id".to_string()])); } #[test] @@ -1080,7 +1095,7 @@ mod builder_tests { } => { assert_eq!(primary_key, vec!["id".to_string()]); assert_eq!(foreign_keys.len(), 1); - assert_eq!(foreign_keys[0].child_column, "pid"); + assert_eq!(foreign_keys[0].child_columns, vec!["pid".to_string()]); // the column-level CHECK still attaches to `pid` assert_eq!( columns.iter().find(|c| c.name == "pid").unwrap().check_sql.as_deref(), diff --git a/src/dsl/mod.rs b/src/dsl/mod.rs index 16da2ed..0c6b795 100644 --- a/src/dsl/mod.rs +++ b/src/dsl/mod.rs @@ -23,7 +23,7 @@ pub use action::ReferentialAction; pub use command::{ AlterTableAction, AppCommand, ChangeColumnMode, ColumnSpec, Command, CompareOp, CopyScope, Expr, IndexSelector, MessagesValue, ModeValue, Operand, Predicate, RelationshipSelector, RowFilter, - SqlForeignKey, + ShowListKind, SqlForeignKey, }; pub use parser::{ParseError, parse_command}; pub use types::Type; diff --git a/src/dsl/parser.rs b/src/dsl/parser.rs index b95a699..2a1e650 100644 --- a/src/dsl/parser.rs +++ b/src/dsl/parser.rs @@ -263,6 +263,16 @@ fn format_expectation(e: &crate::dsl::walker::outcome::Expectation) -> String { use crate::dsl::grammar::IdentSource; use crate::dsl::walker::outcome::Expectation; match e { + // ADR-0042 G1: the bare `1` that opens `add 1:n + // relationship …` is the project's only `Literal("1")` + // (grammar `ddl.rs`); on its own in an expected-set it is + // cryptic — a learner cannot know it begins a + // relationship. Render it as the named construct in error + // wording. This is render-only: completion/hints read the + // raw `Expectation::Literal("1")` directly (offering the + // literal `1` to type), so the candidate surface is + // unchanged. + Expectation::Literal("1") => "`1:n relationship`".to_string(), Expectation::Word(w) | Expectation::Literal(w) => format!("`{w}`"), Expectation::Ident { source, .. } => match source { // Match `IdentSlot::expected_label` outputs so the @@ -286,14 +296,79 @@ fn format_expectation(e: &crate::dsl::walker::outcome::Expectation) -> String { } } +/// ADR-0042 G2: a projection start (`select |`, or the projection +/// position inside a subquery / CTE body) expects the full +/// expression first-set — 14 alternatives — plus the SELECT +/// quantifiers `distinct` and `all`. Those two quantifiers are +/// jointly expectable *only* at a projection start, so their joint +/// presence is a precise signature for collapsing the noisy list +/// into one gloss. Render-only: this fires inside +/// `format_walker_error` (the error message), not in the expected +/// set the completion/hint layer consumes. +fn is_select_projection_start(expected: &[crate::dsl::walker::outcome::Expectation]) -> bool { + use crate::dsl::walker::outcome::Expectation; + let has_word = |w: &str| { + expected + .iter() + .any(|e| matches!(e, Expectation::Word(x) if x.eq_ignore_ascii_case(w))) + }; + has_word("distinct") && has_word("all") +} + +/// ADR-0042 §3: detect the `… CROSS JOIN
on …` mistake. A +/// CROSS JOIN takes no `ON` clause; the grammar rejects the `on`, +/// but the bare structural error ("expected end of input") doesn't +/// teach why. `on` is unexpected at this position *only* when the +/// most recent join is a CROSS join — every other join flavour +/// requires `on`, so there `on` would be in the expected set, not a +/// failure. Detection: the failing token is the keyword `on`, and +/// the last `join` word in the consumed prefix is immediately +/// preceded by `cross`. Render-only; no grammar change. +fn is_cross_join_on(source: &str, position: usize) -> bool { + let rest = source[position.min(source.len())..].trim_start(); + let next_is_on = { + let mut chars = rest.chars(); + let starts_on = rest.len() >= 2 && rest[..2].eq_ignore_ascii_case("on"); + let boundary = chars + .nth(2) + .is_none_or(|c| !c.is_ascii_alphanumeric() && c != '_'); + starts_on && boundary + }; + if !next_is_on { + return false; + } + let consumed = &source[..position.min(source.len())]; + let words: Vec<&str> = consumed + .split(|c: char| !c.is_ascii_alphanumeric() && c != '_') + .filter(|w| !w.is_empty()) + .collect(); + match words.iter().rposition(|w| w.eq_ignore_ascii_case("join")) { + Some(i) if i > 0 => words[i - 1].eq_ignore_ascii_case("cross"), + _ => false, + } +} + fn format_walker_error( source: &str, position: usize, at_eof: bool, expected: &[crate::dsl::walker::outcome::Expectation], ) -> String { - let parts: Vec = expected.iter().map(format_expectation).collect(); - let joined = oxford_join(&parts); + if is_cross_join_on(source, position) { + let consumed = source[..position.min(source.len())].trim_end(); + let prefix = if consumed.is_empty() { + String::new() + } else { + format!("after `{consumed}`, ") + }; + return format!("{prefix}{}", crate::t!("parse.cross_join_no_on")); + } + let joined = if is_select_projection_start(expected) { + crate::t!("parse.expect.select_projection") + } else { + let parts: Vec = expected.iter().map(format_expectation).collect(); + oxford_join(&parts) + }; // Mirror the chumsky-side wording: "after ``, // expected …" when the parser already consumed something @@ -862,9 +937,9 @@ mod tests { Command::AddRelationship { name: name.map(String::from), parent_table: parent.0.to_string(), - parent_column: parent.1.to_string(), + parent_columns: vec![parent.1.to_string()], child_table: child.0.to_string(), - child_column: child.1.to_string(), + child_columns: vec![child.1.to_string()], on_delete, on_update, create_fk, diff --git a/src/dsl/walker/mod.rs b/src/dsl/walker/mod.rs index bf6d34e..d22e750 100644 --- a/src/dsl/walker/mod.rs +++ b/src/dsl/walker/mod.rs @@ -3088,7 +3088,7 @@ mod tests { #[test] fn walker_parses_help() { - assert_eq!(parse("help").unwrap(), Command::App(AppCommand::Help)); + assert_eq!(parse("help").unwrap(), Command::App(AppCommand::Help { topic: None })); } #[test] @@ -3514,9 +3514,9 @@ mod tests { Command::AddRelationship { name: None, parent_table: "Customers".to_string(), - parent_column: "id".to_string(), + parent_columns: vec!["id".to_string()], child_table: "Orders".to_string(), - child_column: "customer_id".to_string(), + child_columns: vec!["customer_id".to_string()], on_delete: ReferentialAction::default_action(), on_update: ReferentialAction::default_action(), create_fk: false, @@ -3535,9 +3535,9 @@ mod tests { Command::AddRelationship { name: Some("cust_orders".to_string()), parent_table: "Customers".to_string(), - parent_column: "id".to_string(), + parent_columns: vec!["id".to_string()], child_table: "Orders".to_string(), - child_column: "customer_id".to_string(), + child_columns: vec!["customer_id".to_string()], on_delete: ReferentialAction::Cascade, on_update: ReferentialAction::SetNull, create_fk: true, @@ -6644,7 +6644,7 @@ mod dispatch_3a_tests { // Distinct dummy commands so a test can tell which node a walk // committed to (the outcome alone doesn't distinguish them). fn dsl_builder(_: &MatchedPath, _: &str) -> Result { - Ok(Command::App(AppCommand::Help)) + Ok(Command::App(AppCommand::Help { topic: None })) } fn sql_builder(_: &MatchedPath, _: &str) -> Result { Ok(Command::App(AppCommand::Quit)) @@ -6729,7 +6729,7 @@ mod dispatch_3a_tests { ); let (outcome, cmd) = dispatch("smk dsltail", Mode::Simple, &cands); assert!(matches!(outcome, WalkOutcome::Match { .. }), "got {outcome:?}"); - assert_eq!(cmd, Some(Command::App(AppCommand::Help))); + assert_eq!(cmd, Some(Command::App(AppCommand::Help { topic: None }))); } // ---- Exit-gate case 2: Advanced + SQL input → SQL match ---- @@ -6805,7 +6805,7 @@ mod dispatch_3a_tests { ); let (outcome, cmd) = dispatch("smk dsltail", Mode::Advanced, &cands); assert!(matches!(outcome, WalkOutcome::Match { .. }), "got {outcome:?}"); - assert_eq!(cmd, Some(Command::App(AppCommand::Help))); + assert_eq!(cmd, Some(Command::App(AppCommand::Help { topic: None }))); } /// In advanced mode a non-shared DSL entry word (no Advanced diff --git a/src/echo.rs b/src/echo.rs index 8bd6adb..e320cb7 100644 --- a/src/echo.rs +++ b/src/echo.rs @@ -264,12 +264,16 @@ pub(crate) fn render_drop_index(name: &str) -> String { pub(crate) fn render_add_relationship( name: &str, parent_table: &str, - parent_column: &str, + parent_columns: &[String], child_table: &str, - child_column: &str, + child_columns: &[String], on_delete: ReferentialAction, on_update: ReferentialAction, ) -> String { + // Multi-column FK (ADR-0043): comma-join each side; a + // single-column FK is the one-element case. + let child_column = child_columns.join(", "); + let parent_column = parent_columns.join(", "); let mut s = format!( "ALTER TABLE {child_table} ADD CONSTRAINT {name} FOREIGN KEY ({child_column}) REFERENCES {parent_table} ({parent_column})" ); @@ -325,28 +329,31 @@ pub(crate) fn render_drop_column_cascade( pub(crate) fn render_add_relationship_create_fk( name: &str, parent_table: &str, - parent_column: &str, + parent_columns: &[String], child_table: &str, - child_column: &str, + child_columns: &[String], on_delete: ReferentialAction, on_update: ReferentialAction, - new_child_column_type: crate::dsl::types::Type, + // The child columns `--create-fk` newly creates, with their types + // (ADR-0043: one per missing column, typed to the matching parent + // PK column's `fk_target_type`). Columns that already existed are + // omitted — no `ADD COLUMN` line for them. + new_columns: &[(String, crate::dsl::types::Type)], ) -> Vec { - vec![ - format!( - "ALTER TABLE {child_table} ADD COLUMN {child_column} {}", - new_child_column_type.keyword() - ), - render_add_relationship( - name, - parent_table, - parent_column, - child_table, - child_column, - on_delete, - on_update, - ), - ] + let mut lines: Vec = new_columns + .iter() + .map(|(col, ty)| format!("ALTER TABLE {child_table} ADD COLUMN {col} {}", ty.keyword())) + .collect(); + lines.push(render_add_relationship( + name, + parent_table, + parent_columns, + child_table, + child_columns, + on_delete, + on_update, + )); + lines } /// Append the `NOT NULL` / `UNIQUE` / `DEFAULT` / `CHECK` column-constraint @@ -953,9 +960,9 @@ mod tests { let sql = render_add_relationship( "Orders_CustId_to_Customers_id", "Customers", - "id", + &["id".to_string()], "Orders", - "CustId", + &["CustId".to_string()], ReferentialAction::NoAction, ReferentialAction::NoAction, ); @@ -971,9 +978,9 @@ mod tests { let sql = render_add_relationship( "places", "Customers", - "id", + &["id".to_string()], "Orders", - "CustId", + &["CustId".to_string()], ReferentialAction::Cascade, ReferentialAction::SetNull, ); @@ -1029,14 +1036,14 @@ mod tests { let lines = render_add_relationship_create_fk( "Customers_id_to_Orders_CustId", "Customers", - "id", + &["id".to_string()], "Orders", - "CustId", + &["CustId".to_string()], ReferentialAction::Cascade, ReferentialAction::NoAction, // Parent PK is `serial` → child FK column is `int` // (`Type::fk_target_type` strips auto-gen semantics; ADR-0011). - crate::dsl::types::Type::Int, + &[("CustId".to_string(), crate::dsl::types::Type::Int)], ); assert_eq!( lines.as_slice(), @@ -1055,12 +1062,12 @@ mod tests { let lines = render_add_relationship_create_fk( "Items_code_to_Lines_code", "Items", - "code", + &["code".to_string()], "Lines", - "code", + &["code".to_string()], ReferentialAction::NoAction, ReferentialAction::NoAction, - crate::dsl::types::Type::Text, + &[("code".to_string(), crate::dsl::types::Type::Text)], ); assert_eq!(lines[0], "ALTER TABLE Lines ADD COLUMN code text"); // No referential clauses when both default. @@ -1181,7 +1188,7 @@ mod tests { // advanced` (verb + payload). for app in [ AppCommand::Quit, - AppCommand::Help, + AppCommand::Help { topic: None }, AppCommand::Rebuild, AppCommand::Save, AppCommand::New, diff --git a/src/event.rs b/src/event.rs index 9266a09..6bde025 100644 --- a/src/event.rs +++ b/src/event.rs @@ -73,6 +73,9 @@ pub enum AppEvent { /// An `explain …` command succeeded (ADR-0028). `plan` /// carries the captured query plan; nothing was executed. DslExplainSucceeded { command: Command, plan: QueryPlan }, + /// A `show ` list command (V5) — carries pre-formatted + /// display lines (tables / relationships / indexes). + DslShowListSucceeded { command: Command, lines: Vec }, DslInsertSucceeded { command: Command, result: InsertResult, diff --git a/src/friendly/keys.rs b/src/friendly/keys.rs index 1cae166..4855855 100644 --- a/src/friendly/keys.rs +++ b/src/friendly/keys.rs @@ -174,6 +174,8 @@ pub const KEYS_AND_PLACEHOLDERS: &[(&str, &[&str])] = &[ ("help.intro", &[]), ("help.dsl_section", &[]), ("help.types_reference", &[]), + ("help.detail_hint", &[]), + ("help.unknown_topic", &["topic"]), ("help.app.quit", &[]), ("help.app.help", &[]), ("help.app.rebuild", &[]), @@ -306,7 +308,15 @@ pub const KEYS_AND_PLACEHOLDERS: &[(&str, &[&str])] = &[ ("parse.usage.select", &[]), ("parse.usage.show_data", &[]), ("parse.usage.show_table", &[]), + ("parse.usage.show_tables", &[]), + ("parse.usage.show_relationships", &[]), + ("parse.usage.show_indexes", &[]), + ("parse.usage.show_relationship", &[]), + ("parse.usage.show_index", &[]), ("parse.usage.update", &[]), + ("parse.usage.with", &[]), + ("parse.expect.select_projection", &[]), + ("parse.cross_join_no_on", &[]), // ---- Project lifecycle event notes ---- ("project.export_failed", &["error"]), ("project.export_ok", &["path"]), diff --git a/src/friendly/strings/en-US.yaml b/src/friendly/strings/en-US.yaml index 0916eb1..2f2e432 100644 --- a/src/friendly/strings/en-US.yaml +++ b/src/friendly/strings/en-US.yaml @@ -238,6 +238,10 @@ help: # per line so scroll math stays accurate. intro: "Supported commands:" dsl_section: "DSL data commands (in simple mode):" + # H3: footer on the full `help` list, and the not-found note + # for `help `. `{topic}` is the word the user typed. + detail_hint: "Type `help ` for detail on one command (e.g. `help insert`), or `help types` for the type reference." + unknown_topic: "No help for `{topic}`. Type `help` for the full command list, or `help types` for the type reference." # Per-command help, keyed by `CommandNode.help_id`. Block # scalars (`|-`) so the column alignment survives — the # double-quoted form trips a libyml scanner bug on long @@ -248,6 +252,7 @@ help: quit — exit the app help: |- help — show this command list + help — detailed help for one command (e.g. `help insert`) rebuild: |- rebuild — rebuild the project database from project.yaml + data/ (with confirmation) save: |- @@ -317,6 +322,11 @@ help: show: |- show table — show a table's structure show data — show a table's rows + show tables — list all tables + show relationships — list all relationships + show indexes — list all indexes + show relationship — show one relationship's detail + show index — show one index's detail insert: |- insert into [(cols)] [values] (vals) — add a row update: |- @@ -491,6 +501,21 @@ parse: # command-keyword renderings (each from # `parse.token.keyword.*`). available_commands: "available commands: {commands}" + # ADR-0042 G2: collapse the SELECT projection-start expression + # first-set (14 expression-starters plus the `distinct`/`all` + # quantifiers) into one learner-sized gloss in the error + # message. The detector keys on `distinct` AND `all` being + # jointly expectable, which only happens at a projection start — + # so the raw set is replaced *in the error line only*; + # completion/hints still expand the full first-set. + expect: + select_projection: "a projection: `*`, a column, or an expression" + # ADR-0042 §3: a CROSS JOIN pairs every row and takes no ON + # clause. The grammar rejects a following `on`; this message + # (rendered in place of the generic structural error when the + # most recent join is a CROSS join and the failing token is `on`) + # teaches the distinction instead of just "expected end of input". + cross_join_no_on: "a CROSS JOIN has no ON clause — it pairs every row; for a join condition use `JOIN … ON`, or filter with `WHERE`" # Per-command usage templates (ADR-0021 §1). Rendered under a # "usage:" prefix when a parse fails after consuming a # known command-entry keyword. The bracket convention `[...]` @@ -539,6 +564,11 @@ parse: [--force-conversion | --dont-convert] show_data: "show data
" show_table: "show table
" + show_tables: "show tables" + show_relationships: "show relationships" + show_indexes: "show indexes" + show_relationship: "show relationship " + show_index: "show index " insert: "insert into
[([, ...])] [values] ([, ...])" update: "update
set =[, ...] (where = | --all-rows)" delete: "delete from
(where = | --all-rows)" @@ -550,6 +580,10 @@ parse: replay: "replay | replay ''" # SQL `SELECT` (advanced mode; ADR-0030 / ADR-0031). select: "select (* | [ as ][, ...]) from
[where ] [order by [ asc|desc][, ...]] [limit ]" + # SQL `WITH` / CTE (advanced mode; ADR-0032). G4 (ADR-0042): + # its own template — `with` previously borrowed `select`'s, + # which never showed the CTE shape. + with: "with [recursive] [([, ...])] as ()[, ...] select ..." # App-lifecycle commands (per ADR-0003, surfaced through # the parser so they participate in usage templates + # completion). Templates here describe the surface @@ -557,7 +591,7 @@ parse: # listing in `help.in_app_body` carries the user-facing # description. quit: "quit" - help: "help" + help: "help []" rebuild: "rebuild" save: "save | save as" new: "new" diff --git a/src/input_render.rs b/src/input_render.rs index 6f87742..e0bac70 100644 --- a/src/input_render.rs +++ b/src/input_render.rs @@ -866,7 +866,9 @@ fn ambient_hint_core_in_mode( // The form the user has committed to drives the // usage template — `add index …` shows the // `add index` usage, not the first `add` form. - let usage = crate::dsl::grammar::usage_key_for_input(input) + // Mode-aware (ADR-0042 G3): advanced-mode shared + // entry words show their SQL form, not the DSL one. + let usage = crate::dsl::grammar::usage_key_for_input_in_mode(input, mode) .map(|key| crate::friendly::translate(key, &[])); Some(AmbientHint::Prose(match usage { Some(u) => crate::t!( @@ -2048,9 +2050,21 @@ mod tests { } #[test] - fn ambient_hint_at_word_boundary_after_show_returns_data_table() { + fn ambient_hint_at_word_boundary_after_show_returns_all_subcommands() { + // data / table plus the V5 list-all forms, grammar order. let cs = cands_hint("show ", 5).expect("candidate hint"); - assert_eq!(cs, vec!["data".to_string(), "table".to_string()]); + assert_eq!( + cs, + vec![ + "data".to_string(), + "table".to_string(), + "tables".to_string(), + "relationships".to_string(), + "indexes".to_string(), + "relationship".to_string(), + "index".to_string(), + ], + ); } #[test] diff --git a/src/output_render.rs b/src/output_render.rs index f318604..a3251b0 100644 --- a/src/output_render.rs +++ b/src/output_render.rs @@ -78,6 +78,16 @@ pub fn render_data_table(data: &DataResult) -> Vec { /// — `References:` / `Referenced by:` blocks below as plain /// indented text (relationship visualization is its own /// future ADR per §5 OOS-1). +/// Display a relationship-endpoint column list (ADR-0043): the bare +/// column for a single-column FK, `(a, b)` for a compound one. +fn cols_disp(cols: &[String]) -> String { + if cols.len() == 1 { + cols[0].clone() + } else { + format!("({})", cols.join(", ")) + } +} + #[must_use] pub fn render_structure(desc: &TableDescription) -> Vec { let mut out: Vec = Vec::new(); @@ -112,9 +122,9 @@ pub fn render_structure(desc: &TableDescription) -> Vec { for r in &desc.outbound_relationships { out.push(format!( " {} → {}.{} ({}, on delete {}, on update {})", - r.local_column, + cols_disp(&r.local_columns), r.other_table, - r.other_column, + cols_disp(&r.other_columns), r.name, r.on_delete, r.on_update, @@ -127,8 +137,8 @@ pub fn render_structure(desc: &TableDescription) -> Vec { out.push(format!( " {}.{} → {} ({}, on delete {}, on update {})", r.other_table, - r.other_column, - r.local_column, + cols_disp(&r.other_columns), + cols_disp(&r.local_columns), r.name, r.on_delete, r.on_update, @@ -769,8 +779,8 @@ mod tests { inbound_relationships: vec![RelationshipEnd { name: "cust_orders".to_string(), other_table: "Orders".to_string(), - other_column: "cust_id".to_string(), - local_column: "id".to_string(), + other_columns: vec!["cust_id".to_string()], + local_columns: vec!["id".to_string()], on_delete: ReferentialAction::Cascade, on_update: ReferentialAction::NoAction, }], diff --git a/src/persistence/mod.rs b/src/persistence/mod.rs index fb0be07..5197b4f 100644 --- a/src/persistence/mod.rs +++ b/src/persistence/mod.rs @@ -245,9 +245,13 @@ pub struct IndexSchema { pub struct RelationshipSchema { pub name: String, pub parent_table: String, - pub parent_column: String, + /// Parent PK column(s); one element for single-column, ordered + /// list for a compound-PK FK (ADR-0043). Paired positionally + /// with `child_columns`. + pub parent_columns: Vec, pub child_table: String, - pub child_column: String, + /// Child column(s), positionally paired with `parent_columns`. + pub child_columns: Vec, pub on_delete: ReferentialAction, pub on_update: ReferentialAction, } diff --git a/src/persistence/yaml.rs b/src/persistence/yaml.rs index 4e72718..518a288 100644 --- a/src/persistence/yaml.rs +++ b/src/persistence/yaml.rs @@ -188,20 +188,31 @@ fn write_relationship(out: &mut String, rel: &RelationshipSchema) { let _ = writeln!(out, " - name: {}", quote_if_needed(&rel.name)); let _ = writeln!( out, - " parent: {{ table: {}, column: {} }}", + " parent: {{ table: {}, columns: [{}] }}", quote_if_needed(&rel.parent_table), - quote_if_needed(&rel.parent_column), + write_col_list(&rel.parent_columns), ); let _ = writeln!( out, - " child: {{ table: {}, column: {} }}", + " child: {{ table: {}, columns: [{}] }}", quote_if_needed(&rel.child_table), - quote_if_needed(&rel.child_column), + write_col_list(&rel.child_columns), ); let _ = writeln!(out, " on_delete: {}", action_keyword(rel.on_delete)); let _ = writeln!(out, " on_update: {}", action_keyword(rel.on_update)); } +/// Format a column list for an inline yaml flow sequence — `a, b` +/// (the caller wraps in `[…]`), each element quoted if needed. +/// Matches the `primary_key: [...]` / index `columns: [...]` house +/// style (ADR-0043 D5). One element for a single-column endpoint. +fn write_col_list(cols: &[String]) -> String { + cols.iter() + .map(|c| quote_if_needed(c)) + .collect::>() + .join(", ") +} + const fn action_keyword(action: ReferentialAction) -> &'static str { match action { ReferentialAction::NoAction => "no_action", @@ -309,9 +320,9 @@ pub(crate) fn parse_schema(body: &str) -> Result { relationships.push(RelationshipSchema { name: r.name, parent_table: r.parent.table, - parent_column: r.parent.column, + parent_columns: r.parent.columns, child_table: r.child.table, - child_column: r.child.column, + child_columns: r.child.columns, on_delete, on_update, }); @@ -502,7 +513,10 @@ struct RawRelationship { #[derive(Deserialize)] struct RawEndpoint { table: String, - column: String, + /// FK endpoint column list (ADR-0043): `columns: [a, b]`, one + /// element for a single-column endpoint — matching the + /// `primary_key` / index `columns` house style. + columns: Vec, } #[derive(Deserialize)] @@ -551,9 +565,9 @@ mod tests { relationships: vec![RelationshipSchema { name: "Customers_id_to_Orders_CustId".to_string(), parent_table: "Customers".to_string(), - parent_column: "id".to_string(), + parent_columns: vec!["id".to_string()], child_table: "Orders".to_string(), - child_column: "CustId".to_string(), + child_columns: vec!["CustId".to_string()], on_delete: ReferentialAction::Cascade, on_update: ReferentialAction::NoAction, }], @@ -578,8 +592,8 @@ mod tests { assert!(body.contains("{ name: id, type: serial }")); assert!(body.contains("{ name: Name, type: text }")); assert!(body.contains("- name: Customers_id_to_Orders_CustId")); - assert!(body.contains("parent: { table: Customers, column: id }")); - assert!(body.contains("child: { table: Orders, column: CustId }")); + assert!(body.contains("parent: { table: Customers, columns: [id] }")); + assert!(body.contains("child: { table: Orders, columns: [CustId] }")); assert!(body.contains("on_delete: cascade")); assert!(body.contains("on_update: no_action")); assert!(body.contains("- name: Orders_CustId_idx")); @@ -934,8 +948,8 @@ project: tables: [] relationships: - name: R - parent: { table: A, column: id } - child: { table: B, column: aid } + parent: { table: A, columns: [id] } + child: { table: B, columns: [aid] } on_delete: blow_up on_update: no_action "; diff --git a/src/runtime.rs b/src/runtime.rs index d29b874..05543b7 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -1403,6 +1403,10 @@ fn spawn_dsl_dispatch( echo, } } + Ok(CommandOutcome::ShowList(lines)) => AppEvent::DslShowListSucceeded { + command: command.clone(), + lines, + }, Ok(CommandOutcome::QueryPlan(plan)) => AppEvent::DslExplainSucceeded { command: command.clone(), plan, @@ -1587,16 +1591,16 @@ struct EchoLookups { /// teaching playground). drop_relationship: Option<(String, String)>, /// For `Command::AddRelationship { create_fk: true, .. }` — the - /// type of the child column the `--create-fk` flag will create, *if* - /// the column did not already exist (`Some(ty)` → newly created → - /// multi-line echo; `None` → already existed → single-line echo). - /// The type is derived from the parent's PK column type via - /// `Type::fk_target_type` (ADR-0011: `serial → int`, `shortid → - /// text`, others identity). The outer `Option` is `None` for - /// not-applicable commands (not a `--create-fk` add, or simple mode, - /// or a pre-execution lookup failed); the inner option encodes the - /// existed-vs-created distinction. - add_rel_create_fk_new_column_type: Option>, + /// child columns the `--create-fk` flag will newly create, each with + /// its type (ADR-0043: one per child column that did **not** already + /// exist, typed to the matching parent PK column's `fk_target_type` — + /// ADR-0011: `serial → int`, `shortid → text`, others identity). An + /// **empty** vec means every child column already existed → + /// single-line echo; a non-empty vec → multi-line (one `ADD COLUMN` + /// per element). The outer `Option` is `None` for not-applicable + /// commands (not a `--create-fk` add, simple mode, or a + /// pre-execution lookup failed). + add_rel_create_fk_new_columns: Option>, } /// Resolve drop-target names and `--create-fk` pre-state **before** @@ -1634,9 +1638,13 @@ async fn collect_echo_lookups( } => { if let Ok(desc) = database.describe_table(child_table.clone(), None).await && let Some(rel) = desc.outbound_relationships.iter().find(|r| { + // The Endpoints drop selector is single-column + // (ADR-0043 keeps DROP by-endpoints single-column; + // compound relationships drop by name) — match a + // one-column relationship by its sole columns. r.other_table == *parent_table - && r.other_column == *parent_column - && r.local_column == *child_column + && r.other_columns.as_slice() == std::slice::from_ref(parent_column) + && r.local_columns.as_slice() == std::slice::from_ref(child_column) }) { out.drop_relationship = Some((rel.name.clone(), child_table.clone())); @@ -1664,41 +1672,37 @@ async fn collect_echo_lookups( Command::AddRelationship { create_fk: true, parent_table, - parent_column, + parent_columns, child_table, - child_column, + child_columns, .. } => { - // Two pre-state facts feed the multi-line `--create-fk` echo - // (ADR-0038 §7 Bucket B, category 2): whether the child - // column already exists (determines single- vs multi-line) - // and the parent PK column's user type (determines the - // newly-created child column's type via - // `Type::fk_target_type`). Both are looked up post-exec from - // the description for `add relationship` (no `--create-fk`), - // but the `--create-fk` multi-line case needs them *before* - // execution to know whether to emit an `ADD COLUMN` line. - let parent_pk_type = database - .describe_table(parent_table.clone(), None) - .await - .ok() - .and_then(|d| { - d.columns + // Pre-state for the multi-line `--create-fk` echo (ADR-0038 + // §7 Bucket B, category 2 / ADR-0043): the subset of child + // columns that do NOT already exist, each typed to the + // matching parent PK column's `fk_target_type`. Needed + // *before* execution to know which `ADD COLUMN` lines to + // emit. The parent columns here are the explicit DSL list, + // paired positionally with the child list. + let parent_desc = database.describe_table(parent_table.clone(), None).await; + let child_desc = database.describe_table(child_table.clone(), None).await; + if let (Ok(parent_desc), Ok(child_desc)) = (parent_desc, child_desc) { + let mut new_columns: Vec<(String, crate::dsl::types::Type)> = Vec::new(); + for (child_col, parent_col) in child_columns.iter().zip(parent_columns) { + let already = child_desc.columns.iter().any(|c| c.name == *child_col); + if already { + continue; + } + if let Some(parent_ty) = parent_desc + .columns .iter() - .find(|c| c.name == *parent_column) + .find(|c| c.name == *parent_col) .and_then(|c| c.user_type) - }); - let child_column_existed = database - .describe_table(child_table.clone(), None) - .await - .ok() - .map(|d| d.columns.iter().any(|c| c.name == *child_column)); - if let (Some(parent_ty), Some(existed)) = (parent_pk_type, child_column_existed) { - out.add_rel_create_fk_new_column_type = Some(if existed { - None - } else { - Some(parent_ty.fk_target_type()) - }); + { + new_columns.push((child_col.clone(), parent_ty.fk_target_type())); + } + } + out.add_rel_create_fk_new_columns = Some(new_columns); } } _ => {} @@ -1751,9 +1755,9 @@ fn build_schema_echo( Command::AddRelationship { name, parent_table, - parent_column, + parent_columns, child_table, - child_column, + child_columns, on_delete, on_update, create_fk, @@ -1762,57 +1766,55 @@ fn build_schema_echo( // relationships (target_table for AddRelationship is the // parent — `database.add_relationship` returns the parent's // description per ADR-0013), falling back to the command's - // explicit `name` when the description is unavailable. + // explicit `name` when the description is unavailable. Match + // on the column lists (ADR-0043), the child as `other` and + // the parent as `local` from the parent's perspective. let resolved = description .and_then(|d| { d.inbound_relationships.iter().find(|r| { r.other_table == *child_table - && r.other_column == *child_column - && r.local_column == *parent_column + && r.other_columns == *child_columns + && r.local_columns == *parent_columns }) }) .map(|r| r.name.clone()) .or_else(|| name.clone())?; if *create_fk { - // Multi-line iff the child column was newly created - // (`--create-fk`'s pre-state, captured pre-execution - // into `add_rel_create_fk_new_column_type`). When the - // column already existed the echo collapses to the - // single-line FK form — the SQL `ADD COLUMN` would be - // a no-op-with-error otherwise, and the catalogue is - // explicit: "one line if the column already existed". - Some(lookups.add_rel_create_fk_new_column_type?.map_or_else( - || { - vec![crate::echo::render_add_relationship( - &resolved, - parent_table, - parent_column, - child_table, - child_column, - *on_delete, - *on_update, - )] - }, - |new_ty| { - crate::echo::render_add_relationship_create_fk( - &resolved, - parent_table, - parent_column, - child_table, - child_column, - *on_delete, - *on_update, - new_ty, - ) - }, - )) + // The pre-execution lookup captured which child columns + // `--create-fk` newly creates (ADR-0043). An empty list + // → every column existed → single-line FK echo; a + // non-empty list → one `ADD COLUMN` per new column then + // the FK line. + let new_columns = lookups.add_rel_create_fk_new_columns.as_ref()?; + if new_columns.is_empty() { + Some(vec![crate::echo::render_add_relationship( + &resolved, + parent_table, + parent_columns, + child_table, + child_columns, + *on_delete, + *on_update, + )]) + } else { + Some(crate::echo::render_add_relationship_create_fk( + &resolved, + parent_table, + parent_columns, + child_table, + child_columns, + *on_delete, + *on_update, + new_columns, + )) + } } else { Some(vec![crate::echo::render_add_relationship( &resolved, parent_table, - parent_column, + parent_columns, child_table, - child_column, + child_columns, *on_delete, *on_update, )]) @@ -2009,17 +2011,19 @@ async fn enrich_fk_violation( }; facts.table = Some(table.clone()); for rel in outbound { - let value = user_value_for_column_with_schema( - database, - command, - table, - &rel.local_column, - ) - .await; + // The friendly FK-error facts model is single-column + // (ADR-0019); for a compound FK (ADR-0043) we enrich + // from the first column pair — the error still surfaces, + // richer multi-column enrichment is a later refinement. + let Some(local_col) = rel.local_columns.first().cloned() else { + continue; + }; + let value = + user_value_for_column_with_schema(database, command, table, &local_col).await; if let Some(v) = value { - facts.column = Some(rel.local_column); + facts.column = Some(local_col); facts.parent_table = Some(rel.other_table); - facts.parent_column = Some(rel.other_column); + facts.parent_column = rel.other_columns.into_iter().next(); facts.value = Some(v.to_string()); break; } @@ -2244,6 +2248,10 @@ enum CommandOutcome { /// — skipped" note. SchemaCreateIndexSkipped(String), Query(DataResult), + /// A `show ` list (V5) — pre-formatted display lines from + /// the worker (table / relationship / index names). Pure + /// display, no schema change. + ShowList(Vec), QueryPlan(QueryPlan), Insert(InsertResult), Update(UpdateResult), @@ -2607,9 +2615,9 @@ async fn execute_command_typed( Command::AddRelationship { name, parent_table, - parent_column, + parent_columns, child_table, - child_column, + child_columns, on_delete, on_update, create_fk, @@ -2617,9 +2625,9 @@ async fn execute_command_typed( .add_relationship( name, parent_table, - parent_column, + parent_columns, child_table, - child_column, + child_columns, on_delete, on_update, create_fk, @@ -2766,6 +2774,10 @@ async fn execute_command_typed( .describe_table(name, src) .await .map(|d| CommandOutcome::Schema(Some(d))), + Command::ShowList { kind, name } => database + .show_list(kind, name) + .await + .map(CommandOutcome::ShowList), Command::Insert { table, columns, @@ -3181,9 +3193,9 @@ mod tests { .add_relationship( None, "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], ReferentialAction::Cascade, ReferentialAction::NoAction, false, @@ -3194,9 +3206,9 @@ mod tests { let add_rel_cmd = Command::AddRelationship { name: None, parent_table: "Customers".to_string(), - parent_column: "id".to_string(), + parent_columns: vec!["id".to_string()], child_table: "Orders".to_string(), - child_column: "CustId".to_string(), + child_columns: vec!["CustId".to_string()], on_delete: ReferentialAction::Cascade, on_update: ReferentialAction::NoAction, create_fk: false, @@ -3354,9 +3366,9 @@ mod tests { let add_fk_cmd = Command::AddRelationship { name: None, parent_table: "Customers".to_string(), - parent_column: "id".to_string(), + parent_columns: vec!["id".to_string()], child_table: "Orders".to_string(), - child_column: "CustId".to_string(), + child_columns: vec!["CustId".to_string()], on_delete: ReferentialAction::Cascade, on_update: ReferentialAction::NoAction, create_fk: true, @@ -3366,17 +3378,17 @@ mod tests { let pre_lookups = super::collect_echo_lookups(&db, &add_fk_cmd, EffectiveMode::AdvancedPersistent).await; assert_eq!( - pre_lookups.add_rel_create_fk_new_column_type, - Some(Some(Type::Int)), + pre_lookups.add_rel_create_fk_new_columns, + Some(vec![("CustId".to_string(), Type::Int)]), "pre-exec captures `serial → int` for the newly-created child column", ); let parent_desc = db .add_relationship( None, "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], ReferentialAction::Cascade, ReferentialAction::NoAction, true, @@ -3423,17 +3435,17 @@ mod tests { let pre_lookups = super::collect_echo_lookups(&db, &add_fk_cmd, EffectiveMode::AdvancedPersistent).await; assert_eq!( - pre_lookups.add_rel_create_fk_new_column_type, - Some(None), + pre_lookups.add_rel_create_fk_new_columns, + Some(vec![]), "pre-exec records the child column already existed → single-line echo", ); let parent_desc = db .add_relationship( None, "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], ReferentialAction::Cascade, ReferentialAction::NoAction, true, diff --git a/tests/it/column_op_guards.rs b/tests/it/column_op_guards.rs index 9fb0714..bc9f43e 100644 --- a/tests/it/column_op_guards.rs +++ b/tests/it/column_op_guards.rs @@ -132,9 +132,9 @@ fn add_relationship_refuses_internal_tables() { .block_on(db.add_relationship( None, internal.clone(), - "name".to_string(), + vec!["name".to_string()], "C".to_string(), - "x".to_string(), + vec!["x".to_string()], ReferentialAction::NoAction, ReferentialAction::NoAction, false, @@ -161,9 +161,9 @@ fn add_relationship_refuses_internal_tables() { .block_on(db.add_relationship( None, "P".to_string(), - "id".to_string(), + vec!["id".to_string()], internal, - "x".to_string(), + vec!["x".to_string()], ReferentialAction::NoAction, ReferentialAction::NoAction, false, diff --git a/tests/it/compound_fk.rs b/tests/it/compound_fk.rs new file mode 100644 index 0000000..e72ce1e --- /dev/null +++ b/tests/it/compound_fk.rs @@ -0,0 +1,549 @@ +//! Integration tests for compound-primary-key foreign-key +//! references (T3 / ADR-0043) — the DSL `add 1:n relationship` +//! surface end to end. +//! +//! Covers: parse of the parenthesized multi-column endpoint; +//! worker round-trip (declare a 2-column FK, FK is enforced, +//! per-pair type-mismatch refused, arity mismatch refused); +//! persistence round-trip (`columns: [a, b]`); display; and undo. + +use rdbms_playground::db::Database; +use rdbms_playground::dsl::{ + parse_command, ColumnSpec, Command, ReferentialAction, SqlForeignKey, Type, Value, +}; +use rdbms_playground::persistence::Persistence; +use rdbms_playground::project; + +// ---- parse layer ------------------------------------------------ + +#[test] +fn parenthesized_compound_endpoint_parses_to_column_lists() { + let cmd = parse_command( + "add 1:n relationship from Parent.(a, b) to Child.(x, y)", + ) + .expect("parses"); + match cmd { + Command::AddRelationship { + parent_table, + parent_columns, + child_table, + child_columns, + .. + } => { + assert_eq!(parent_table, "Parent"); + assert_eq!(parent_columns, vec!["a".to_string(), "b".to_string()]); + assert_eq!(child_table, "Child"); + assert_eq!(child_columns, vec!["x".to_string(), "y".to_string()]); + } + other => panic!("expected AddRelationship, got {other:?}"), + } +} + +#[test] +fn single_column_endpoint_still_parses_unparenthesized() { + let cmd = parse_command("add 1:n relationship from Parent.id to Child.pid") + .expect("parses"); + match cmd { + Command::AddRelationship { + parent_columns, + child_columns, + .. + } => { + assert_eq!(parent_columns, vec!["id".to_string()]); + assert_eq!(child_columns, vec!["pid".to_string()]); + } + other => panic!("expected AddRelationship, got {other:?}"), + } +} + +// ---- SQL surface (advanced mode) -------------------------------- + +#[test] +fn sql_table_level_compound_fk_parses_to_lists() { + let cmd = parse_command( + "create table City (country int, region_code int, \ + foreign key (country, region_code) references Region(country, code))", + ) + .expect("parses"); + match cmd { + Command::SqlCreateTable { foreign_keys, .. } => { + assert_eq!(foreign_keys.len(), 1); + assert_eq!( + foreign_keys[0].child_columns, + vec!["country".to_string(), "region_code".to_string()], + ); + assert_eq!( + foreign_keys[0].parent_columns, + Some(vec!["country".to_string(), "code".to_string()]), + ); + } + other => panic!("expected SqlCreateTable, got {other:?}"), + } +} + +#[test] +fn sql_bare_compound_reference_parses_with_no_parent_columns() { + // `FOREIGN KEY (a, b) REFERENCES P` (no parent cols) — auto-expanded + // to the parent's full PK at execution (F-D). + let cmd = parse_command( + "create table City (country int, region_code int, \ + foreign key (country, region_code) references Region)", + ) + .expect("parses"); + match cmd { + Command::SqlCreateTable { foreign_keys, .. } => { + assert_eq!( + foreign_keys[0].child_columns, + vec!["country".to_string(), "region_code".to_string()], + ); + assert_eq!(foreign_keys[0].parent_columns, None); + } + other => panic!("expected SqlCreateTable, got {other:?}"), + } +} + +#[test] +fn sql_create_table_compound_fk_executes_and_enforces() { + let (_p, db, _dir) = open_project_db(); + let rt = rt(); + rt.block_on(async { + // Parent with a compound PK. + db.create_table( + "Region".to_string(), + vec![ + ColumnSpec::new("country", Type::Int), + ColumnSpec::new("code", Type::Int), + ], + vec!["country".to_string(), "code".to_string()], + None, + ) + .await + .expect("create Region"); + // Child via the SQL path with a multi-column FK referencing the + // full compound PK (resolve_create_table_fks). + db.sql_create_table( + "City".to_string(), + vec![ + ColumnSpec::new("country", Type::Int), + ColumnSpec::new("region_code", Type::Int), + ], + vec![], + vec![], + vec![], + vec![SqlForeignKey { + name: None, + child_columns: vec!["country".to_string(), "region_code".to_string()], + parent_table: "Region".to_string(), + parent_columns: Some(vec!["country".to_string(), "code".to_string()]), + on_delete: ReferentialAction::NoAction, + on_update: ReferentialAction::NoAction, + }], + false, + None, + ) + .await + .expect("create City with compound FK"); + + db.insert( + "Region".to_string(), + Some(vec!["country".to_string(), "code".to_string()]), + vec![Value::Number("1".to_string()), Value::Number("10".to_string())], + None, + ) + .await + .expect("insert parent"); + let bad = db + .insert( + "City".to_string(), + Some(vec!["country".to_string(), "region_code".to_string()]), + vec![Value::Number("9".to_string()), Value::Number("9".to_string())], + None, + ) + .await; + assert!(bad.is_err(), "compound FK violation refused by the engine"); + }); +} + +// ---- worker round-trip ------------------------------------------ + +fn rt() -> tokio::runtime::Runtime { + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("tokio rt") +} + +fn open_project_db() -> (project::Project, Database, tempfile::TempDir) { + let dir = tempfile::tempdir().expect("create tempdir"); + let project = + project::open_or_create(None, Some(dir.path())).expect("open or create project"); + let persistence = Persistence::new(project.path().to_path_buf()); + let db = Database::open_with_persistence(project.db_path(), persistence) + .expect("open db with persistence"); + (project, db, dir) +} + +/// `Region(country int, code int)` compound PK + `City(country int, +/// region_code int, name text)` — the child FK columns matching the +/// parent PK by type (int → int). +async fn seed_compound(db: &Database) { + db.create_table( + "Region".to_string(), + vec![ + ColumnSpec::new("country", Type::Int), + ColumnSpec::new("code", Type::Int), + ], + vec!["country".to_string(), "code".to_string()], + None, + ) + .await + .expect("create Region"); + db.create_table( + "City".to_string(), + vec![ + ColumnSpec::new("country", Type::Int), + ColumnSpec::new("region_code", Type::Int), + ColumnSpec::new("name", Type::Text), + ], + vec!["country".to_string()], + None, + ) + .await + .expect("create City"); +} + +#[test] +fn compound_fk_declares_enforces_and_round_trips() { + let (_p, db, _dir) = open_project_db(); + let rt = rt(); + rt.block_on(async { + seed_compound(&db).await; + // Declare the compound FK: City.(country, region_code) → + // Region.(country, code). + db.add_relationship( + Some("city_region".to_string()), + "Region".to_string(), + vec!["country".to_string(), "code".to_string()], + "City".to_string(), + vec!["country".to_string(), "region_code".to_string()], + ReferentialAction::NoAction, + ReferentialAction::NoAction, + false, + None, + ) + .await + .expect("add compound relationship"); + + // The FK is enforced: a parent row exists for (1, 10); a + // child referencing it inserts, one referencing (9, 9) is + // refused by the engine. + db.insert( + "Region".to_string(), + Some(vec!["country".to_string(), "code".to_string()]), + vec![Value::Number("1".to_string()), Value::Number("10".to_string())], + None, + ) + .await + .expect("insert parent row"); + db.insert( + "City".to_string(), + Some(vec![ + "country".to_string(), + "region_code".to_string(), + "name".to_string(), + ]), + vec![Value::Number("1".to_string()), Value::Number("10".to_string()), Value::Text("Metropolis".to_string())], + None, + ) + .await + .expect("child row referencing an existing parent key inserts"); + let violation = db + .insert( + "City".to_string(), + Some(vec![ + "country".to_string(), + "region_code".to_string(), + "name".to_string(), + ]), + vec![Value::Number("9".to_string()), Value::Number("9".to_string()), Value::Text("Nowhere".to_string())], + None, + ) + .await; + assert!( + violation.is_err(), + "a child row with no matching compound parent key must be refused", + ); + + // describe shows the compound endpoints symmetrically. + let city = db.describe_table("City".to_string(), None).await.unwrap(); + let outbound = &city.outbound_relationships[0]; + assert_eq!( + outbound.local_columns, + vec!["country".to_string(), "region_code".to_string()], + ); + assert_eq!( + outbound.other_columns, + vec!["country".to_string(), "code".to_string()], + ); + }); +} + +#[test] +fn compound_fk_create_fk_makes_both_child_columns() { + let (_p, db, _dir) = open_project_db(); + let rt = rt(); + rt.block_on(async { + // Region(country, code) compound PK; City has neither FK column. + db.create_table( + "Region".to_string(), + vec![ + ColumnSpec::new("country", Type::Int), + ColumnSpec::new("code", Type::Int), + ], + vec!["country".to_string(), "code".to_string()], + None, + ) + .await + .expect("create Region"); + db.create_table( + "City".to_string(), + vec![ColumnSpec::new("name", Type::Text)], + vec![], + None, + ) + .await + .expect("create City"); + // --create-fk creates both missing child columns, typed to the + // matching parent PK columns' fk_target_type (int → int). + db.add_relationship( + None, + "Region".to_string(), + vec!["country".to_string(), "code".to_string()], + "City".to_string(), + vec!["c_country".to_string(), "c_code".to_string()], + ReferentialAction::NoAction, + ReferentialAction::NoAction, + true, + None, + ) + .await + .expect("add compound relationship with --create-fk"); + let city = db.describe_table("City".to_string(), None).await.unwrap(); + for col in ["c_country", "c_code"] { + assert!( + city.columns.iter().any(|c| c.name == col), + "--create-fk created `{col}`: {:?}", + city.columns.iter().map(|c| &c.name).collect::>(), + ); + } + }); +} + +#[test] +fn compound_fk_arity_mismatch_is_refused() { + let (_p, db, _dir) = open_project_db(); + let rt = rt(); + rt.block_on(async { + seed_compound(&db).await; + // Two parent columns, one child column → arity mismatch. + let err = db + .add_relationship( + None, + "Region".to_string(), + vec!["country".to_string(), "code".to_string()], + "City".to_string(), + vec!["country".to_string()], + ReferentialAction::NoAction, + ReferentialAction::NoAction, + false, + None, + ) + .await; + assert!(err.is_err(), "mismatched child/parent arity must be refused"); + }); +} + +#[test] +fn compound_fk_type_mismatch_per_pair_is_refused() { + let (_p, db, _dir) = open_project_db(); + let rt = rt(); + rt.block_on(async { + db.create_table( + "Region".to_string(), + vec![ + ColumnSpec::new("country", Type::Int), + ColumnSpec::new("code", Type::Int), + ], + vec!["country".to_string(), "code".to_string()], + None, + ) + .await + .expect("create Region"); + // `bad` is `text` — incompatible with the `int` PK column it + // would pair with (per-pair type-compat, ADR-0011). + db.create_table( + "City".to_string(), + vec![ + ColumnSpec::new("country", Type::Int), + ColumnSpec::new("bad", Type::Text), + ], + vec![], + None, + ) + .await + .expect("create City"); + let err = db + .add_relationship( + None, + "Region".to_string(), + vec!["country".to_string(), "code".to_string()], + "City".to_string(), + vec!["country".to_string(), "bad".to_string()], + ReferentialAction::NoAction, + ReferentialAction::NoAction, + false, + None, + ) + .await; + assert!(err.is_err(), "a type-incompatible column pair must be refused"); + }); +} + +#[test] +fn compound_fk_survives_rebuild_from_text() { + // The riskiest round-trip: comma-encoded metadata + yaml + // `columns: [a, b]` → rebuild reconstructs the compound FK DDL. + let dir = tempfile::tempdir().expect("tempdir"); + let project = project::open_or_create(None, Some(dir.path())).expect("open project"); + let path = project.path().to_path_buf(); + let rt = rt(); + { + let db = Database::open_with_persistence( + project.db_path(), + Persistence::new(path.clone()), + ) + .expect("open db"); + rt.block_on(async { + seed_compound(&db).await; + db.add_relationship( + Some("city_region".to_string()), + "Region".to_string(), + vec!["country".to_string(), "code".to_string()], + "City".to_string(), + vec!["country".to_string(), "region_code".to_string()], + ReferentialAction::NoAction, + ReferentialAction::NoAction, + false, + Some("add 1:n relationship".to_string()), + ) + .await + .expect("add compound relationship"); + }); + } + // Reopen and rebuild the database purely from the persisted + // project.yaml + data/. + let db = Database::open_with_persistence(project.db_path(), Persistence::new(path.clone())) + .expect("reopen db"); + rt.block_on(async { + db.rebuild_from_text(path.clone(), None) + .await + .expect("rebuild from text"); + // The compound FK is reconstructed and still enforced. + db.insert( + "Region".to_string(), + Some(vec!["country".to_string(), "code".to_string()]), + vec![Value::Number("1".to_string()), Value::Number("10".to_string())], + None, + ) + .await + .expect("insert parent after rebuild"); + let bad = db + .insert( + "City".to_string(), + Some(vec!["country".to_string(), "region_code".to_string()]), + vec![Value::Number("9".to_string()), Value::Number("9".to_string())], + None, + ) + .await; + assert!(bad.is_err(), "compound FK still enforced after rebuild from text"); + // Endpoints survived the round-trip intact. + let city = db.describe_table("City".to_string(), None).await.unwrap(); + assert_eq!( + city.outbound_relationships[0].other_columns, + vec!["country".to_string(), "code".to_string()], + ); + }); +} + +#[test] +fn compound_fk_undo_removes_the_relationship() { + let dir = tempfile::tempdir().expect("tempdir"); + let project = project::open_or_create(None, Some(dir.path())).expect("open project"); + let db = Database::open_with_persistence_and_undo( + project.db_path(), + Persistence::new(project.path().to_path_buf()), + true, + ) + .expect("open db with undo"); + let rt = rt(); + rt.block_on(async { + seed_compound(&db).await; + db.add_relationship( + Some("city_region".to_string()), + "Region".to_string(), + vec!["country".to_string(), "code".to_string()], + "City".to_string(), + vec!["country".to_string(), "region_code".to_string()], + ReferentialAction::NoAction, + ReferentialAction::NoAction, + false, + // A user-command source records one undo snapshot. + Some("add 1:n relationship".to_string()), + ) + .await + .expect("add compound relationship"); + assert_eq!( + db.describe_table("City".to_string(), None) + .await + .unwrap() + .outbound_relationships + .len(), + 1, + ); + // One undo step removes the whole relationship (ADR-0013/0006). + db.undo().await.unwrap().expect("undo applied"); + assert!( + db.describe_table("City".to_string(), None) + .await + .unwrap() + .outbound_relationships + .is_empty(), + "undo removed the compound relationship in one step", + ); + }); +} + +#[test] +fn compound_fk_partial_pk_reference_is_refused() { + let (_p, db, _dir) = open_project_db(); + let rt = rt(); + rt.block_on(async { + seed_compound(&db).await; + // Referencing only one column of Region's 2-column PK (F-A: + // must reference the full PK). + let err = db + .add_relationship( + None, + "Region".to_string(), + vec!["country".to_string()], + "City".to_string(), + vec!["country".to_string()], + ReferentialAction::NoAction, + ReferentialAction::NoAction, + false, + None, + ) + .await; + assert!(err.is_err(), "a partial-PK reference must be refused (F-A)"); + }); +} diff --git a/tests/it/friendly_enrichment.rs b/tests/it/friendly_enrichment.rs index f726952..ff289c9 100644 --- a/tests/it/friendly_enrichment.rs +++ b/tests/it/friendly_enrichment.rs @@ -420,9 +420,9 @@ fn enrich_fk_insert_resolves_parent_table_column_and_value() { db.add_relationship( None, "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], ReferentialAction::NoAction, ReferentialAction::NoAction, false, @@ -496,9 +496,9 @@ fn enrich_fk_insert_natural_order_multi_value_resolves_via_schema() { db.add_relationship( None, "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], ReferentialAction::NoAction, ReferentialAction::NoAction, false, @@ -570,9 +570,9 @@ fn enrich_fk_delete_resolves_child_table() { db.add_relationship( None, "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], ReferentialAction::NoAction, ReferentialAction::NoAction, false, diff --git a/tests/it/help_command.rs b/tests/it/help_command.rs new file mode 100644 index 0000000..b455f60 --- /dev/null +++ b/tests/it/help_command.rs @@ -0,0 +1,141 @@ +//! Integration tests for `help` and `help ` (H3). +//! +//! Covers: +//! - Parse layer: `help` → `Help { topic: None }`; `help insert` +//! → `Help { topic: Some("insert") }`. +//! - App behaviour: the full `help` ends with the detail hint; +//! `help ` renders that command's block (and every +//! form sharing the entry word); `help types` renders the type +//! reference; an unknown topic gets a friendly pointer back. + +use crossterm::event::{KeyCode, KeyEvent, KeyEventKind, KeyModifiers}; + +use rdbms_playground::app::App; +use rdbms_playground::dsl::{parse_command, AppCommand, Command}; +use rdbms_playground::event::AppEvent; + +const fn key(code: KeyCode) -> AppEvent { + AppEvent::Key(KeyEvent { + code, + modifiers: KeyModifiers::NONE, + kind: KeyEventKind::Press, + state: crossterm::event::KeyEventState::NONE, + }) +} + +fn type_str(app: &mut App, s: &str) { + for c in s.chars() { + app.update(key(KeyCode::Char(c))); + } +} + +/// Submit `input` to a fresh app and collect all output text. +fn output_for(input: &str) -> Vec { + let mut app = App::new(); + type_str(&mut app, input); + app.update(key(KeyCode::Enter)); + app.output.iter().map(|l| l.text.clone()).collect() +} + +// ================================================================= +// Parse layer +// ================================================================= + +#[test] +fn bare_help_parses_with_no_topic() { + assert_eq!( + parse_command("help").expect("parses"), + Command::App(AppCommand::Help { topic: None }), + ); +} + +#[test] +fn help_with_topic_captures_the_word() { + assert_eq!( + parse_command("help insert").expect("parses"), + Command::App(AppCommand::Help { + topic: Some("insert".to_string()) + }), + ); +} + +#[test] +fn help_topic_is_a_single_word_multi_word_is_a_parse_error() { + // Entry-word topics cover multi-word commands (`help create`), + // so a second word is trailing junk, not a longer topic. + assert!(parse_command("help foo bar").is_err()); +} + +// ================================================================= +// App behaviour +// ================================================================= + +#[test] +fn full_help_lists_commands_and_ends_with_the_detail_hint() { + let out = output_for("help"); + assert!( + out.iter().any(|l| l == "Supported commands:"), + "intro present: {out:?}", + ); + assert!( + out.iter().any(|l| l.contains("help ")), + "detail-hint footer present: {out:?}", + ); +} + +#[test] +fn help_insert_renders_the_insert_block() { + let out = output_for("help insert"); + assert!( + out.iter().any(|l| l.contains("insert into")), + "insert help shown: {out:?}", + ); + // Focused: it must NOT dump the whole list — the intro header + // belongs to the full `help` only. + assert!( + !out.iter().any(|l| l == "Supported commands:"), + "focused help omits the full-list intro: {out:?}", + ); +} + +#[test] +fn help_create_covers_every_form_sharing_the_entry_word() { + // `create` is the entry word for both the DSL `create table` + // and the advanced SQL `CREATE TABLE` — `help create` shows + // both blocks. + let out = output_for("help create"); + let joined = out.join("\n"); + assert!( + joined.contains("create table"), + "DSL create form shown: {out:?}", + ); + assert!( + joined.to_lowercase().matches("create").count() >= 2, + "more than one create form shown: {out:?}", + ); +} + +#[test] +fn help_types_renders_the_type_reference() { + let out = output_for("help types"); + let joined = out.join("\n").to_lowercase(); + // The type reference names the playground types. + assert!( + joined.contains("serial") || joined.contains("shortid"), + "type reference shown: {out:?}", + ); +} + +#[test] +fn help_unknown_topic_points_back_to_the_full_list() { + let out = output_for("help wibble"); + assert!( + out.iter() + .any(|l| l.contains("No help for") && l.contains("wibble")), + "names the unknown topic: {out:?}", + ); + assert!( + out.iter().any(|l| l.contains("Type `help`")), + "points back at the full list: {out:?}", + ); +} diff --git a/tests/it/iteration2_persistence.rs b/tests/it/iteration2_persistence.rs index 9a66f99..22b70f1 100644 --- a/tests/it/iteration2_persistence.rs +++ b/tests/it/iteration2_persistence.rs @@ -192,9 +192,9 @@ fn delete_with_cascade_rewrites_both_csvs() { db.add_relationship( None, "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], ReferentialAction::Cascade, ReferentialAction::NoAction, false, @@ -424,9 +424,9 @@ fn project_yaml_carries_relationship_after_add() { db.add_relationship( None, "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], ReferentialAction::Cascade, ReferentialAction::NoAction, false, diff --git a/tests/it/iteration3_rebuild.rs b/tests/it/iteration3_rebuild.rs index 40cea86..4361dfb 100644 --- a/tests/it/iteration3_rebuild.rs +++ b/tests/it/iteration3_rebuild.rs @@ -185,9 +185,9 @@ fn rebuild_restores_relationships_and_cascade_behaviour() { db.add_relationship( None, "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], ReferentialAction::Cascade, ReferentialAction::NoAction, false, diff --git a/tests/it/main.rs b/tests/it/main.rs index 3bd4270..f2e9a57 100644 --- a/tests/it/main.rs +++ b/tests/it/main.rs @@ -9,8 +9,10 @@ mod case_insensitive_names; mod column_op_guards; +mod compound_fk; mod engine_vocabulary_audit; mod friendly_enrichment; +mod help_command; mod iteration2_persistence; mod iteration3_rebuild; mod iteration4a_rebuild_command; @@ -30,5 +32,6 @@ mod sql_drop_table; mod sql_insert; mod sql_select; mod sql_update; +mod show_list; mod undo_snapshots; mod walking_skeleton; diff --git a/tests/it/parse_error_pedagogy.rs b/tests/it/parse_error_pedagogy.rs index d93f01e..8cab3b5 100644 --- a/tests/it/parse_error_pedagogy.rs +++ b/tests/it/parse_error_pedagogy.rs @@ -13,6 +13,7 @@ use crossterm::event::{KeyCode, KeyEvent, KeyEventKind, KeyModifiers}; use rdbms_playground::action::Action; use rdbms_playground::app::{App, OutputKind}; use rdbms_playground::event::AppEvent; +use rdbms_playground::mode::Mode; const fn key(code: KeyCode) -> AppEvent { AppEvent::Key(KeyEvent { @@ -59,6 +60,308 @@ fn dump(input: &str, lines: &[String]) -> String { ) } +/// The simple-mode near-miss matrix (ADR-0042 §1). Each row is a +/// near-correct input plus substrings that MUST appear across its +/// rendered error lines — the structural "name the missing +/// keyword/clause" message and the per-command usage template. +#[test] +fn near_miss_matrix_simple_mode() { + // (input, required-substrings-across-error-lines) + let matrix: &[(&str, &[&str])] = &[ + // app-lifecycle arg errors. The arg-less commands all reject + // trailing junk with "expected end of input" + their usage + // (audited 2026-06-05); locked here as regression insurance. + ("quit now", &["after `quit`, expected end of input", " quit"]), + // `help` now takes an optional single-word topic (H3), so + // `help foo` parses (topic lookup); only a *multi-word* + // topic is the near-miss that rejects trailing junk. + ("help foo bar", &["after `help foo`, expected end of input", "help []"]), + ("rebuild now", &["after `rebuild`, expected end of input", " rebuild"]), + ("new foo", &["after `new`, expected end of input", " new"]), + ("load foo", &["after `load`, expected end of input", " load"]), + ("undo foo", &["after `undo`, expected end of input", " undo"]), + ("redo foo", &["after `redo`, expected end of input", " redo"]), + ("export foo bar", &["after `export foo`, expected end of input", "export []"]), + ("import a b c", &["after `import a`, expected end of input", "import "]), + ("save sideways", &["after `save`, expected end of input", "save | save as"]), + ("mode sideways", &["unknown mode 'sideways'", "mode simple | mode advanced"]), + ("messages louder", &["unknown messages mode 'louder'", "messages short"]), + ("copy everything", &["unknown copy target 'everything'", "copy all"]), + // DDL bare + missing-slot + ("create", &["after `create`, expected `table`", "create table with pk"]), + ("create table", &["after `create table`, expected identifier", "create table with pk"]), + ("create table T", &["with pk", "create table with pk"]), + // G1: relationship cardinality reads as the named construct. + ("add", &["after `add`, expected `column`, `1:n relationship`", "add 1:n relationship"]), + ("drop table", &["after `drop table`, expected table name", "drop table "]), + ("add column", &["after `add column`, expected table name", "add column [to] [table]"]), + ("rename", &["after `rename`, expected `column`", "rename column [in] [table]"]), + ("rename column", &["after `rename column`, expected table name", "rename column [in] [table]"]), + ("change", &["after `change`, expected `column`", "change column [in] [table]"]), + ("change column", &["after `change column`, expected table name", "change column [in] [table]"]), + // data bare + missing-clause + ("insert", &["after `insert`, expected `into`", "insert into
"]), + ("insert into", &["after `insert into`, expected table name", "insert into
"]), + ("insert into T", &["after `insert into T`, expected `values` or `(`", "insert into
"]), + ("update", &["after `update`, expected table name", "update
set"]), + ("update T", &["after `update T`, expected `set`", "update
set"]), + ("update T set x=1", &["expected `where` or `--all-rows`", "update
set"]), + ("delete", &["after `delete`, expected `from`", "delete from
"]), + ("delete from", &["after `delete from`, expected table name", "delete from
"]), + ("delete from T", &["expected `where` or `--all-rows`", "delete from
"]), + ("replay", &["after `replay`, expected string literal or path", "replay "]), + ("explain", &["after `explain`, expected `show`, `update`, or `delete`", "explain show data"]), + // advanced-only entry word typed in simple mode → "this is SQL" rail + ("select * from T", &["`select` is SQL", "mode advanced"]), + ("alter table T add column c int", &["`alter` is SQL", "mode advanced"]), + ]; + for (input, needles) in matrix { + let lines = error_lines_for(input); + let dump_msg = dump(input, &lines); + assert!( + lines.iter().any(|l| l.starts_with("parse error")), + "missing `parse error` line for {input:?}\n{dump_msg}", + ); + let joined = lines.join("\n"); + for needle in *needles { + assert!( + joined.contains(needle), + "near-miss {input:?} missing expected substring {needle:?}\n{dump_msg}", + ); + } + } +} + +/// Helper: advanced-mode error lines for `input`. +fn advanced_error_lines_for(input: &str) -> Vec { + let mut app = App::new(); + app.mode = Mode::Advanced; + type_str(&mut app, input); + let _ = submit(&mut app); + app.output + .iter() + .filter(|l| l.kind == OutputKind::Error) + .map(|l| l.text.clone()) + .collect() +} + +/// Committed multi-form near-misses (ADR-0042 §1). A user who has +/// already chosen a form (`add index …`, `drop constraint …`, +/// `alter table T add …`) must get that form's specific +/// missing-keyword/clause message and usage — not the whole family +/// generically. Audited 2026-06-05; these render well today and are +/// locked here as the residual systematic-pass tail. +#[test] +fn near_miss_matrix_committed_multiforms() { + // (input, advanced?, required-substrings) + let matrix: &[(&str, bool, &[&str])] = &[ + // add / drop multi-forms (simple) + ("add index", false, &["after `add index`, expected `on` or `as`", "add index [as ] on"]), + ("add index on T", false, &["after `add index on T`, expected `(`", "add index [as ] on"]), + ("add constraint", false, &["after `add constraint`, expected `not`, `unique`, `default`, or `check`", "add constraint not null to"]), + ("add constraint not null", false, &["after `add constraint not null`, expected `to`", "add constraint not null to"]), + ("add 1:n relationship", false, &["after `add 1:n relationship`, expected `from` or `as`", "add 1:n relationship"]), + ("add 1:n relationship from", false, &["after `add 1:n relationship from`, expected table name", "from ."]), + ("drop constraint", false, &["after `drop constraint`, expected `not`, `unique`, `default`, or `check`", "drop constraint (not null"]), + ("drop constraint not null", false, &["after `drop constraint not null`, expected `from`", "drop constraint (not null"]), + ("drop index", false, &["after `drop index`, expected `on` or index name", "drop index ", "drop index on
"]), + ("drop index on T", false, &["after `drop index on T`, expected `(`", "drop index on
"]), + ("drop relationship", false, &["after `drop relationship`, expected `from` or relationship name", "drop relationship "]), + ("show table", false, &["after `show table`, expected table name", "show table
"]), + ("show relationship", false, &["after `show relationship`, expected relationship name", "show relationship "]), + ("show index", false, &["after `show index`, expected index name", "show index "]), + ("change column in table T: c", false, &["after `change column in table T: c`, expected `(`", "change column [in] [table]"]), + // advanced committed multi-forms + ("create index on", true, &["after `create index on`, expected table name", "create [unique] index"]), + ("create unique index", true, &["after `create unique index`, expected `on`, identifier, or `if`", "create [unique] index"]), + ("alter table T add", true, &["after `alter table T add`, expected `column`, `constraint`, `check`, `unique`, `foreign`, or `primary`", "alter table
add column"]), + ("alter table T drop", true, &["after `alter table T drop`, expected `column` or `constraint`", "alter table
drop column"]), + ]; + for (input, advanced, needles) in matrix { + let lines = if *advanced { + advanced_error_lines_for(input) + } else { + error_lines_for(input) + }; + let dump_msg = dump(input, &lines); + assert!( + lines.iter().any(|l| l.starts_with("parse error")), + "missing `parse error` line for {input:?}\n{dump_msg}", + ); + assert!( + !lines.iter().any(|l| l.starts_with("available commands:")), + "committed form {input:?} fell back to available-commands\n{dump_msg}", + ); + let joined = lines.join("\n"); + for needle in *needles { + assert!( + joined.contains(needle), + "committed form {input:?} missing expected substring {needle:?}\n{dump_msg}", + ); + } + } +} + +#[test] +fn advanced_bare_select_collapses_projection_first_set() { + // ADR-0042 G2: bare `select` dumped the full 14-item + // expression first-set ("`not`, `-`, …, `case`, column name, + // `distinct`, or `all`"). Collapse it to a learner-sized + // projection gloss in the error MESSAGE only — completion + // still expands the raw set (locked by the typing-surface + // matrix). + let lines = advanced_error_lines_for("select"); + let joined = lines.join("\n"); + let dump_msg = dump("select", &lines); + assert!( + joined.contains("a projection: `*`, a column, or an expression"), + "bare `select` should collapse to the projection gloss\n{dump_msg}", + ); + let err_line = lines + .iter() + .find(|l| l.starts_with("parse error")) + .expect("parse error line"); + assert!( + !err_line.contains("`exists`") && !err_line.contains("`case`"), + "projection gloss should replace the raw expression first-set\n{dump_msg}", + ); +} + +#[test] +fn advanced_mode_usage_block_shows_sql_and_dsl_forms() { + // ADR-0042 G3: `render_usage_block` was mode-blind — it + // resolved shared entry words to the first-registered (Simple) + // node, so advanced-mode `create` showed ONLY the DSL `create + // table … with pk …` template and none of the SQL forms. + // Mode-aware selection shows every form valid in the mode, + // SQL-primary first. In advanced mode the DSL forms remain + // valid input (verified: `create table Foo with pk` parses and + // runs in advanced mode), so they MUST still appear — a usage + // hint never hides input that works. + let lines = advanced_error_lines_for("create"); + let joined = lines.join("\n"); + let dump_msg = dump("create", &lines); + assert!( + joined.contains("create table [if not exists]"), + "advanced `create` should show the SQL create-table usage\n{dump_msg}", + ); + assert!( + joined.contains("create [unique] index"), + "advanced `create` should show the SQL create-index usage\n{dump_msg}", + ); + assert!( + joined.contains("create table with pk"), + "advanced `create` should ALSO show the DSL `with pk` form (valid in advanced mode)\n{dump_msg}", + ); + // Ordering: the SQL form is listed before the DSL form + // (mode-primary first). + let sql_at = joined.find("create table [if not exists]").unwrap(); + let dsl_at = joined.find("create table with pk").unwrap(); + assert!(sql_at < dsl_at, "SQL form should precede the DSL form\n{dump_msg}"); +} + +#[test] +fn advanced_cross_join_with_on_teaches_no_on_clause() { + // ADR-0042 §3: a CROSS JOIN has no ON clause. The grammar + // rejects a following `on`, but the bare structural error + // ("expected end of input") does not teach why. `on` is + // unexpected here only because the most recent join is a CROSS + // join — every other join flavour *requires* `on` — so the case + // is precisely detectable and gets a teaching message. + let input = "select * from a cross join b on x = y"; + let lines = advanced_error_lines_for(input); + let joined = lines.join("\n"); + let dump_msg = dump(input, &lines); + assert!( + joined.contains("a CROSS JOIN has no ON clause"), + "cross join + on should teach that CROSS JOIN takes no ON\n{dump_msg}", + ); + // Misfire guard 1: a plain JOIN missing its ON still asks for `on`. + let plain = advanced_error_lines_for("select * from a join b").join("\n"); + assert!( + plain.contains("expected `on`") && !plain.contains("CROSS JOIN"), + "a plain join must still ask for ON, not the cross-join message: {plain}", + ); + // Misfire guard 2: a stray `on` with no join present must NOT + // claim a CROSS JOIN. + let stray = advanced_error_lines_for("select * from a on x = y").join("\n"); + assert!( + !stray.contains("CROSS JOIN has no ON"), + "no cross join present — must not fire: {stray}", + ); +} + +/// The advanced-mode near-miss matrix (ADR-0042 §1/§3). Mirrors +/// the simple-mode matrix for the SQL surface. Every row must show +/// a per-command `usage:` block (never the available-commands +/// fallback — that is for unconsumed entry words only). +#[test] +fn near_miss_matrix_advanced_mode() { + let matrix: &[(&str, &[&str])] = &[ + // SQL select / with (G2, G4) + ("select", &["expected a projection: `*`, a column, or an expression", "select (* |"]), + ("select * from", &["after `select * from`, expected table name", "select (* |"]), + ("with", &["after `with`, expected identifier or `recursive`", "with [recursive]", "as ("]), + // create / drop / alter — SQL forms AND the still-valid DSL + // fallback forms, SQL-primary first (G3). + ("create", &["after `create`, expected `table`", "create table [if not exists]", "create [unique] index", "create table with pk"]), + ("create table", &["after `create table`, expected identifier or `if`", "create table [if not exists]"]), + ("create index", &["after `create index`, expected `on`", "create [unique] index"]), + ("drop", &["after `drop`, expected `table`", "drop table [if exists]", "drop column [from]", "drop relationship"]), + ("alter", &["after `alter`, expected `table`", "alter table
add column"]), + ("alter table T", &["expected `add`, `drop`, `rename`, or `alter`", "alter table
"]), + // shared insert/update/delete — must show usage, not the + // available-commands fallback (regression guard for the + // empty-usage_ids SQL nodes). + ("insert into T", &["after `insert into T`, expected `values`, `with`, `select`, or `(`", "insert into
"]), + ("update T", &["after `update T`, expected `set`", "update
set"]), + ("delete from", &["after `delete from`, expected table name", "delete from
"]), + ]; + for (input, needles) in matrix { + let lines = advanced_error_lines_for(input); + let dump_msg = dump(input, &lines); + assert!( + lines.iter().any(|l| l.starts_with("parse error")), + "missing `parse error` line for {input:?}\n{dump_msg}", + ); + // A consumed entry word must yield a usage block, never the + // available-commands fallback. + assert!( + !lines.iter().any(|l| l.starts_with("available commands:")), + "advanced {input:?} fell back to available-commands instead of a usage block\n{dump_msg}", + ); + let joined = lines.join("\n"); + for needle in *needles { + assert!( + joined.contains(needle), + "advanced near-miss {input:?} missing expected substring {needle:?}\n{dump_msg}", + ); + } + } +} + +#[test] +fn with_alone_renders_cte_usage_not_select() { + // ADR-0042 G4: `with` (advanced-only CTE entry word) borrowed + // the `select` usage template, which never mentions the CTE + // shape. It now carries its own `parse.usage.with`. + let mut app = App::new(); + app.mode = Mode::Advanced; + type_str(&mut app, "with"); + let _ = submit(&mut app); + let lines: Vec = app + .output + .iter() + .filter(|l| l.kind == OutputKind::Error) + .map(|l| l.text.clone()) + .collect(); + let dump_msg = dump("with", &lines); + assert!( + lines.iter().any(|l| l.trim_start().starts_with("with ") && l.contains("as (")), + "missing CTE-specific `with … as (…)` usage template\n{dump_msg}", + ); +} + #[test] fn create_alone_renders_create_table_usage() { let lines = error_lines_for("create"); @@ -81,15 +384,18 @@ fn create_alone_renders_create_table_usage() { fn add_alone_renders_both_add_family_usages() { let lines = error_lines_for("add"); let dump_msg = dump("add", &lines); - // Aggregation across `choice` (ADR-0020): the structural - // error line lists both add-family entries. + // Aggregation across the top-level choice: the structural + // error line lists every add-family branch. ADR-0042 G1: the + // relationship branch renders as the friendly `1:n + // relationship` rather than the cryptic bare `1` cardinality + // literal. assert!( lines.iter().any(|l| { l.starts_with("parse error") - && l.contains("`1`") + && l.contains("`1:n relationship`") && l.contains("`column`") }), - "expected aggregated `1` or `column` in structural error\n{dump_msg}", + "expected aggregated `1:n relationship` and `column` in structural error\n{dump_msg}", ); // Usage block (ADR-0021): both add-* templates surface. assert!( @@ -229,3 +535,7 @@ fn caret_aligns_under_offending_token() { "caret should sit at column 9 (under `f` of `frobulate` after the `running: ` prefix); got {leading_spaces} spaces in {caret:?}", ); } + + + + diff --git a/tests/it/show_list.rs b/tests/it/show_list.rs new file mode 100644 index 0000000..ea6f31b --- /dev/null +++ b/tests/it/show_list.rs @@ -0,0 +1,359 @@ +//! Integration tests for the `show ` list commands (V5): +//! `show tables`, `show relationships`, `show indexes`. +//! +//! Covers: +//! - Parse layer: each form parses to `Command::ShowList { kind }` +//! in both simple and advanced mode (the forms are +//! `CommandCategory::Simple`, available in both). +//! - Worker round-trip: `Database::show_list` returns the correct +//! pre-formatted lines after real DDL (tables, a relationship, +//! an index), and the empty-collection wording. +//! - App end-to-end: submitting `show tables` reaches the output +//! panel as system lines and marks the echo complete. + +use crossterm::event::{KeyCode, KeyEvent, KeyEventKind, KeyModifiers}; + +use rdbms_playground::action::Action; +use rdbms_playground::app::App; +use rdbms_playground::db::Database; +use rdbms_playground::dsl::{ + parse_command, ColumnSpec, Command, ReferentialAction, ShowListKind, Type, +}; +use rdbms_playground::event::AppEvent; +use rdbms_playground::mode::Mode; +use rdbms_playground::persistence::Persistence; +use rdbms_playground::project; + +// ================================================================= +// Parse layer +// ================================================================= + +#[test] +fn show_tables_parses_as_show_list_tables() { + assert_eq!( + parse_command("show tables").expect("parses"), + Command::ShowList { + kind: ShowListKind::Tables, + name: None, + }, + ); +} + +#[test] +fn show_relationships_parses_as_show_list_relationships() { + assert_eq!( + parse_command("show relationships").expect("parses"), + Command::ShowList { + kind: ShowListKind::Relationships, + name: None, + }, + ); +} + +#[test] +fn show_indexes_parses_as_show_list_indexes() { + assert_eq!( + parse_command("show indexes").expect("parses"), + Command::ShowList { + kind: ShowListKind::Indexes, + name: None, + }, + ); +} + +#[test] +fn show_relationship_singular_parses_with_name() { + assert_eq!( + parse_command("show relationship orders_customer").expect("parses"), + Command::ShowList { + kind: ShowListKind::Relationships, + name: Some("orders_customer".to_string()), + }, + ); +} + +#[test] +fn show_index_singular_parses_with_name() { + assert_eq!( + parse_command("show index idx_orders_customer").expect("parses"), + Command::ShowList { + kind: ShowListKind::Indexes, + name: Some("idx_orders_customer".to_string()), + }, + ); +} + +#[test] +fn show_table_singular_still_parses_as_show_table() { + // The new plural keyword must not shadow the singular + // `show table ` form — `table` ≠ `tables`. + assert_eq!( + parse_command("show table Customers").expect("parses"), + Command::ShowTable { + name: "Customers".to_string() + }, + ); +} + +// ================================================================= +// Worker round-trip — real execution +// ================================================================= + +fn rt() -> tokio::runtime::Runtime { + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("tokio rt") +} + +fn open_project_db() -> (project::Project, Database, tempfile::TempDir) { + let dir = tempfile::tempdir().expect("create tempdir"); + let project = + project::open_or_create(None, Some(dir.path())).expect("open or create project"); + let persistence = Persistence::new(project.path().to_path_buf()); + let db = Database::open_with_persistence(project.db_path(), persistence) + .expect("open db with persistence"); + (project, db, dir) +} + +/// Create two related tables plus an index, so each list kind has +/// content. Returns once the worker has applied everything. +async fn seed_schema(db: &Database) { + db.create_table( + "Customers".to_string(), + vec![ + ColumnSpec::new("id", Type::Serial), + ColumnSpec::new("Name", Type::Text), + ], + vec!["id".to_string()], + None, + ) + .await + .expect("create Customers"); + db.create_table( + "Orders".to_string(), + vec![ + ColumnSpec::new("id", Type::Serial), + ColumnSpec::new("customer_id", Type::Int), + ], + vec!["id".to_string()], + None, + ) + .await + .expect("create Orders"); + db.add_relationship( + Some("orders_customer".to_string()), + "Customers".to_string(), + vec!["id".to_string()], + "Orders".to_string(), + vec!["customer_id".to_string()], + ReferentialAction::Cascade, + ReferentialAction::NoAction, + false, + None, + ) + .await + .expect("add relationship"); + db.add_index( + Some("idx_orders_customer".to_string()), + "Orders".to_string(), + vec!["customer_id".to_string()], + None, + ) + .await + .expect("add index"); +} + +#[test] +fn show_tables_lists_all_user_tables_with_count_header() { + let (_p, db, _dir) = open_project_db(); + let rt = rt(); + rt.block_on(seed_schema(&db)); + let lines = rt + .block_on(db.show_list(ShowListKind::Tables, None)) + .expect("show tables"); + assert_eq!(lines[0], "Tables (2):", "count header"); + assert!( + lines.iter().any(|l| l == " Customers"), + "Customers listed: {lines:?}", + ); + assert!( + lines.iter().any(|l| l == " Orders"), + "Orders listed: {lines:?}", + ); +} + +#[test] +fn show_relationships_lists_name_endpoints_and_nondefault_action() { + let (_p, db, _dir) = open_project_db(); + let rt = rt(); + rt.block_on(seed_schema(&db)); + let lines = rt + .block_on(db.show_list(ShowListKind::Relationships, None)) + .expect("show relationships"); + assert_eq!(lines[0], "Relationships (1):"); + // Name, both endpoints, and the non-default ON DELETE CASCADE + // (ON UPDATE NO ACTION is the default and is omitted). + assert_eq!( + lines[1], + " orders_customer: Customers.id → Orders.customer_id on delete cascade", + "relationship summary line: {lines:?}", + ); +} + +#[test] +fn show_indexes_lists_qualified_name_and_columns() { + let (_p, db, _dir) = open_project_db(); + let rt = rt(); + rt.block_on(seed_schema(&db)); + let lines = rt + .block_on(db.show_list(ShowListKind::Indexes, None)) + .expect("show indexes"); + assert_eq!(lines[0], "Indexes (1):"); + assert_eq!( + lines[1], " Orders.idx_orders_customer (customer_id)", + "index summary line: {lines:?}", + ); +} + +#[test] +fn show_lists_report_empty_collections_with_friendly_lines() { + let (_p, db, _dir) = open_project_db(); + let rt = rt(); + // No schema seeded — every kind is empty. + assert_eq!( + rt.block_on(db.show_list(ShowListKind::Tables, None)).unwrap(), + vec!["No tables in this project yet.".to_string()], + ); + assert_eq!( + rt.block_on(db.show_list(ShowListKind::Relationships, None)) + .unwrap(), + vec!["No relationships in this project yet.".to_string()], + ); + assert_eq!( + rt.block_on(db.show_list(ShowListKind::Indexes, None)).unwrap(), + vec!["No indexes in this project yet.".to_string()], + ); +} + +// ================================================================= +// V5a — singular per-item detail +// ================================================================= + +#[test] +fn show_one_relationship_renders_detail_block() { + let (_p, db, _dir) = open_project_db(); + let rt = rt(); + rt.block_on(seed_schema(&db)); + let lines = rt + .block_on(db.show_list(ShowListKind::Relationships, Some("orders_customer".to_string()))) + .expect("show relationship"); + assert_eq!(lines[0], "Relationship `orders_customer`:"); + assert_eq!(lines[1], " Customers.id → Orders.customer_id"); + assert!( + lines.iter().any(|l| l == " on delete cascade"), + "on-delete shown: {lines:?}", + ); +} + +#[test] +fn show_one_index_renders_detail_block() { + let (_p, db, _dir) = open_project_db(); + let rt = rt(); + rt.block_on(seed_schema(&db)); + let lines = rt + .block_on(db.show_list(ShowListKind::Indexes, Some("idx_orders_customer".to_string()))) + .expect("show index"); + assert_eq!(lines[0], "Index `idx_orders_customer` on Orders:"); + assert!( + lines.iter().any(|l| l == " columns: customer_id"), + "columns shown: {lines:?}", + ); + assert!( + lines.iter().any(|l| l == " unique: no"), + "uniqueness shown: {lines:?}", + ); +} + +#[test] +fn show_one_unknown_name_reports_friendly_not_found() { + let (_p, db, _dir) = open_project_db(); + let rt = rt(); + rt.block_on(seed_schema(&db)); + assert_eq!( + rt.block_on(db.show_list(ShowListKind::Relationships, Some("nope".to_string()))) + .unwrap(), + vec!["No relationship named `nope`.".to_string()], + ); + assert_eq!( + rt.block_on(db.show_list(ShowListKind::Indexes, Some("nope".to_string()))) + .unwrap(), + vec!["No index named `nope`.".to_string()], + ); +} + +// ================================================================= +// App end-to-end +// ================================================================= + +const fn key(code: KeyCode) -> AppEvent { + AppEvent::Key(KeyEvent { + code, + modifiers: KeyModifiers::NONE, + kind: KeyEventKind::Press, + state: crossterm::event::KeyEventState::NONE, + }) +} + +fn type_str(app: &mut App, s: &str) { + for c in s.chars() { + app.update(key(KeyCode::Char(c))); + } +} + +#[test] +fn app_show_tables_dispatches_show_list_command() { + let mut app = App::new(); + app.mode = Mode::Simple; + type_str(&mut app, "show tables"); + let actions = app.update(key(KeyCode::Enter)); + let dispatched = actions.iter().any(|a| { + matches!( + a, + Action::ExecuteDsl { + command: Command::ShowList { + kind: ShowListKind::Tables, + name: None, + }, + .. + } + ) + }); + assert!(dispatched, "submit dispatches ShowList(Tables): {actions:?}"); +} + +#[test] +fn app_renders_show_list_lines_as_system_output() { + // Feed the success event directly so the test stays + // self-contained (the worker round-trip is covered above). + let mut app = App::new(); + app.output.push_back(rdbms_playground::app::OutputLine::echo( + "show tables", + Mode::Simple, + )); + app.update(AppEvent::DslShowListSucceeded { + command: Command::ShowList { + kind: ShowListKind::Tables, + name: None, + }, + lines: vec!["Tables (1):".to_string(), " Customers".to_string()], + }); + assert!( + app.output.iter().any(|l| l.text == "Tables (1):"), + "header line rendered", + ); + assert!( + app.output.iter().any(|l| l.text == " Customers"), + "item line rendered", + ); +} diff --git a/tests/it/sql_create_table.rs b/tests/it/sql_create_table.rs index a4d48d0..2d0b097 100644 --- a/tests/it/sql_create_table.rs +++ b/tests/it/sql_create_table.rs @@ -834,9 +834,9 @@ fn dropping_a_column_a_table_check_references_fails_cleanly() { fn fk(child_column: &str, parent_table: &str, parent_column: Option<&str>) -> SqlForeignKey { SqlForeignKey { name: None, - child_column: child_column.to_string(), + child_columns: vec![child_column.to_string()], parent_table: parent_table.to_string(), - parent_column: parent_column.map(str::to_string), + parent_columns: parent_column.map(|c| vec![c.to_string()]), on_delete: ReferentialAction::NoAction, on_update: ReferentialAction::NoAction, } @@ -929,7 +929,7 @@ fn foreign_key_creates_named_relationship_visible_in_describe() { let rel = &child.outbound_relationships[0]; assert_eq!(rel.name, "parent_id_to_child_pid", "auto-named per ADR-0013"); assert_eq!(rel.other_table, "parent"); - assert_eq!(rel.local_column, "pid"); + assert_eq!(rel.local_columns, vec!["pid".to_string()]); let parent = r.block_on(db.describe_table("parent".to_string(), None)).expect("describe parent"); assert_eq!(parent.inbound_relationships.len(), 1, "parent is referenced by child"); @@ -974,7 +974,7 @@ fn bare_references_resolves_to_parent_single_column_pk() { )) .expect("create child with bare REFERENCES"); let child = r.block_on(db.describe_table("child".to_string(), None)).expect("describe"); - assert_eq!(child.outbound_relationships[0].other_column, "id", "resolved to parent PK"); + assert_eq!(child.outbound_relationships[0].other_columns, vec!["id".to_string()], "resolved to parent PK"); } #[test] @@ -1341,7 +1341,7 @@ fn bare_self_reference_resolves_to_own_pk() { )) .expect("create self-referential emp with a bare reference"); let emp = r.block_on(db.describe_table("emp".to_string(), None)).expect("describe"); - assert_eq!(emp.outbound_relationships[0].other_column, "id", "bare self-ref resolved to own PK"); + assert_eq!(emp.outbound_relationships[0].other_columns, vec!["id".to_string()], "bare self-ref resolved to own PK"); // Enforced: a non-existent manager is rejected. r.block_on(db.insert( "emp".to_string(), diff --git a/tests/it/sql_delete.rs b/tests/it/sql_delete.rs index ab64307..43f3a23 100644 --- a/tests/it/sql_delete.rs +++ b/tests/it/sql_delete.rs @@ -100,9 +100,9 @@ fn cascade_fixture(db: &Database, rt: &tokio::runtime::Runtime) { rt.block_on(db.add_relationship( Some("places".to_string()), "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], ReferentialAction::Cascade, ReferentialAction::NoAction, false, @@ -289,9 +289,9 @@ fn cascade_to_two_children_reports_both() { rt.block_on(db.add_relationship( Some(name.to_string()), "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], child.to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], ReferentialAction::Cascade, ReferentialAction::NoAction, false, @@ -358,9 +358,9 @@ fn delete_violating_fk_fails_and_persists_nothing() { rt.block_on(db.add_relationship( Some("places".to_string()), "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], ReferentialAction::NoAction, // on delete: reject if referenced ReferentialAction::NoAction, false, @@ -395,9 +395,9 @@ fn self_referential_cascade_counts_only_cascaded_rows() { rt.block_on(db.add_relationship( Some("parent_of".to_string()), "T".to_string(), - "id".to_string(), + vec!["id".to_string()], "T".to_string(), - "ParentId".to_string(), + vec!["ParentId".to_string()], ReferentialAction::Cascade, ReferentialAction::NoAction, false, diff --git a/tests/it/sql_dml_e2e.rs b/tests/it/sql_dml_e2e.rs index 844a053..313d643 100644 --- a/tests/it/sql_dml_e2e.rs +++ b/tests/it/sql_dml_e2e.rs @@ -318,9 +318,9 @@ fn cascade_fixture(db: &Database, rt: &tokio::runtime::Runtime) { rt.block_on(db.add_relationship( Some("places".to_string()), "Customers".to_string(), - "id".to_string(), + vec!["id".to_string()], "Orders".to_string(), - "CustId".to_string(), + vec!["CustId".to_string()], ReferentialAction::Cascade, ReferentialAction::NoAction, false, diff --git a/tests/it/sql_drop_table.rs b/tests/it/sql_drop_table.rs index 2c7f578..b71be7a 100644 --- a/tests/it/sql_drop_table.rs +++ b/tests/it/sql_drop_table.rs @@ -104,9 +104,9 @@ fn dropping_a_referenced_parent_is_refused() { vec![], vec![SqlForeignKey { name: None, - child_column: "pid".to_string(), + child_columns: vec!["pid".to_string()], parent_table: "parent".to_string(), - parent_column: Some("id".to_string()), + parent_columns: Some(vec!["id".to_string()]), on_delete: rdbms_playground::dsl::ReferentialAction::NoAction, on_update: rdbms_playground::dsl::ReferentialAction::NoAction, }], diff --git a/tests/it/walking_skeleton.rs b/tests/it/walking_skeleton.rs index db442d2..2be6ea4 100644 --- a/tests/it/walking_skeleton.rs +++ b/tests/it/walking_skeleton.rs @@ -420,9 +420,9 @@ fn add_relationship_flow_shows_parent_side_with_inbound_section() { &Command::AddRelationship { name: None, parent_table: "Customers".to_string(), - parent_column: "Id".to_string(), + parent_columns: vec!["Id".to_string()], child_table: "Orders".to_string(), - child_column: "CustId".to_string(), + child_columns: vec!["CustId".to_string()], on_delete: ReferentialAction::Cascade, on_update: ReferentialAction::NoAction, create_fk: false, @@ -449,8 +449,8 @@ fn add_relationship_flow_shows_parent_side_with_inbound_section() { inbound_relationships: vec![RelationshipEnd { name: "Customers_Id_to_Orders_CustId".to_string(), other_table: "Orders".to_string(), - other_column: "CustId".to_string(), - local_column: "Id".to_string(), + other_columns: vec!["CustId".to_string()], + local_columns: vec!["Id".to_string()], on_delete: ReferentialAction::Cascade, on_update: ReferentialAction::NoAction, }], @@ -462,9 +462,9 @@ fn add_relationship_flow_shows_parent_side_with_inbound_section() { command: Command::AddRelationship { name: None, parent_table: "Customers".to_string(), - parent_column: "Id".to_string(), + parent_columns: vec!["Id".to_string()], child_table: "Orders".to_string(), - child_column: "CustId".to_string(), + child_columns: vec!["CustId".to_string()], on_delete: ReferentialAction::Cascade, on_update: ReferentialAction::NoAction, create_fk: false, @@ -504,8 +504,8 @@ fn add_relationship_flow_shows_inbound_section_on_parent() { inbound_relationships: vec![RelationshipEnd { name: "Customers_Id_to_Orders_CustId".to_string(), other_table: "Orders".to_string(), - other_column: "CustId".to_string(), - local_column: "Id".to_string(), + other_columns: vec!["CustId".to_string()], + local_columns: vec!["Id".to_string()], on_delete: ReferentialAction::Cascade, on_update: ReferentialAction::NoAction, }], diff --git a/tests/typing_surface/mod.rs b/tests/typing_surface/mod.rs index 51e94b5..4551501 100644 --- a/tests/typing_surface/mod.rs +++ b/tests/typing_surface/mod.rs @@ -233,6 +233,7 @@ fn command_kind_label(cmd: &rdbms_playground::dsl::Command) -> String { AddConstraint { .. } => "AddConstraint".into(), DropConstraint { .. } => "DropConstraint".into(), ShowTable { .. } => "ShowTable".into(), + ShowList { kind, name } => format!("ShowList({kind:?}, {})", name.is_some()), Insert { .. } => "Insert".into(), Update { .. } => "Update".into(), Delete { .. } => "Delete".into(), @@ -245,7 +246,7 @@ fn command_kind_label(cmd: &rdbms_playground::dsl::Command) -> String { SqlDelete { .. } => "SqlDelete".into(), App(app) => match app { AppCommand::Quit => "App(Quit)".into(), - AppCommand::Help => "App(Help)".into(), + AppCommand::Help { .. } => "App(Help)".into(), AppCommand::Rebuild => "App(Rebuild)".into(), AppCommand::Save => "App(Save)".into(), AppCommand::SaveAs => "App(SaveAs)".into(), diff --git a/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__add_relationship__after_child_table_dot_narrows_to_child_columns@after_child_dot.snap b/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__add_relationship__after_child_table_dot_narrows_to_child_columns@after_child_dot.snap index a148fd6..200a828 100644 --- a/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__add_relationship__after_child_table_dot_narrows_to_child_columns@after_child_dot.snap +++ b/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__add_relationship__after_child_table_dot_narrows_to_child_columns@after_child_dot.snap @@ -1,5 +1,6 @@ --- source: tests/typing_surface/add_relationship.rs +assertion_line: 64 description: "input=\"add 1:n relationship from Customers.id to Orders.\" cursor=49" expression: "& a" --- @@ -25,6 +26,11 @@ Assessment { kind: Identifier, mode: Both, }, + Candidate { + text: "(", + kind: Punct, + mode: Both, + }, ], selected: None, }, @@ -52,6 +58,11 @@ Assessment { kind: Identifier, mode: Both, }, + Candidate { + text: "(", + kind: Punct, + mode: Both, + }, ], }, ), diff --git a/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__add_relationship__after_parent_table_dot_narrows_to_parent_columns@after_parent_dot.snap b/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__add_relationship__after_parent_table_dot_narrows_to_parent_columns@after_parent_dot.snap index 0165d55..357eef6 100644 --- a/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__add_relationship__after_parent_table_dot_narrows_to_parent_columns@after_parent_dot.snap +++ b/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__add_relationship__after_parent_table_dot_narrows_to_parent_columns@after_parent_dot.snap @@ -1,5 +1,6 @@ --- source: tests/typing_surface/add_relationship.rs +assertion_line: 51 description: "input=\"add 1:n relationship from Customers.\" cursor=36" expression: "& a" --- @@ -20,6 +21,11 @@ Assessment { kind: Identifier, mode: Both, }, + Candidate { + text: "(", + kind: Punct, + mode: Both, + }, ], selected: None, }, @@ -42,6 +48,11 @@ Assessment { kind: Identifier, mode: Both, }, + Candidate { + text: "(", + kind: Punct, + mode: Both, + }, ], }, ), diff --git a/website/STYLE.md b/website/STYLE.md index c90cf7d..5315985 100644 --- a/website/STYLE.md +++ b/website/STYLE.md @@ -4,7 +4,7 @@ This is the **living** home for documentation authoring conventions for the RDBMS Playground website. It grows as we write. - **Binding rules** come from - [ADR-0042 §7](../docs/adr/0042-public-website-and-documentation-site.md); + [ADR-0044 §7](../docs/adr/0044-public-website-and-documentation-site.md); this guide must not contradict them. If a convention is significant, durable, or contested, it is decided in an **ADR** (new or amended), and this guide references it. Finer, settled conventions live here directly. @@ -17,7 +17,7 @@ Status tags used below: **[DECIDED]** (binding or settled) · --- -## Terminology & wording [DECIDED — ADR-0042 §7] +## Terminology & wording [DECIDED — ADR-0044 §7] - **No "DSL".** It is internal jargon. Use **simple mode** (the playground's keyword command language) and **advanced mode** (SQL). @@ -60,7 +60,7 @@ Ground every reference page in source — `parse.usage.*` and `help.*` in `src/friendly/strings/en-US.yaml`, `src/dsl/command.rs`, `src/dsl/types.rs` — never paraphrase grammar from memory. -## "Planned / not yet available" callouts [DECIDED — ADR-0042 §7] +## "Planned / not yet available" callouts [DECIDED — ADR-0044 §7] Any capability that is not yet fully implemented is **omitted** or carries a clear callout — never presented as shipped. Standard form: a Starlight aside @@ -104,7 +104,7 @@ a small standalone example, not by complicating this schema. - Pair a hero/landing cast with a text transcript or the equivalent docs snippet (accessibility + SEO). - Recorded via a scripted-input driver for paced, re-recordable sessions - (ADR-0042 §2; recipe in `README.md`). **[OPEN]**: cast file naming, + (ADR-0044 §2; recipe in `README.md`). **[OPEN]**: cast file naming, fixed terminal size, light/dark theme handling. ## Formatting [DECIDED, refine] diff --git a/website/astro.config.mjs b/website/astro.config.mjs index d5ac274..54728d6 100644 --- a/website/astro.config.mjs +++ b/website/astro.config.mjs @@ -16,7 +16,7 @@ export default defineConfig({ // public home. Omitted for now rather than linking the wrong repo. // social: [{ icon: 'github', label: 'GitHub', href: '…' }], customCss: ['./src/styles/global.css'], - // Pragmatic structure (ADR-0042 §7 / website/STYLE.md): Getting + // Pragmatic structure (ADR-0044 §7 / website/STYLE.md): Getting // started, Guides, Reference, Concepts. Autogenerated per directory; // in-section order is controlled by each page's `sidebar.order` // frontmatter.