diff --git a/docs/handoff/20260525-handoff-38.md b/docs/handoff/20260525-handoff-38.md new file mode 100644 index 0000000..13fdad2 --- /dev/null +++ b/docs/handoff/20260525-handoff-38.md @@ -0,0 +1,283 @@ +# Session handoff — 2026-05-25 (38) + +Thirty-eighth handover. This session **shipped three ADR-0035 Phase-4 +sub-phases — 4a.3, 4b, 4c** — each test-first with a `/runda` (4a.3, 4b) +or written-DA (4c) gate, all green. The next session **implements 4d — +`CREATE [UNIQUE] INDEX` / `DROP INDEX`** (§4), which carries one +escalation: the `IndexSchema.unique` model extension (ADR-0025 deferred +unique indexes). A **growing 4i list (a–e)** is now the running tail of +deferred polish — see §6, tracked canonically in ADR-0035 §13 4i. + +## §1. State at handoff + +**Branch:** `main`. **Tests: 1805 passing, 0 failing, 0 skipped, +1 ignored** (the unchanged `friendly/mod.rs` ` ```ignore ` doctest). +**Clippy:** clean (`cargo clippy --all-targets -- -D warnings`). + +**HEAD (local-only):** `e52e90c` (4c). `origin/main` is at `1991fb4` +(handoff-37); **the 3 commits since are local-only**. Unpushed commits +are a normal working state; pushing is the user's step — do not prompt. + +**This session's commits** (oldest → newest): + +``` +60111f6 feat: ADR-0035 4a.3 — table-level / multi-column CHECK +76d6059 feat: ADR-0035 4b — foreign keys in CREATE TABLE +e52e90c feat: ADR-0035 4c — DROP TABLE [IF EXISTS] +``` + +(Plus this handoff commit.) + +## §2. What shipped this session + +Advanced-mode SQL `CREATE TABLE` is now feature-complete for constraints ++ FKs, and `DROP TABLE` is live. + +- **4a.3 — table-level / multi-column `CHECK`** (`60111f6`). `CREATE + TABLE t (a int, b int, CHECK (a < b))`. SQLite exposes **no PRAGMA for + CHECK**, so a table-level CHECK is stored in a **new internal table + `__rdbms_playground_table_checks (table_name, seq, check_expr)`** as + its source of truth (read by `read_schema`/`read_schema_snapshot`, + cleared by `do_drop_table`, round-tripped through `project.yaml` via + `TableSchema.check_constraints`). The builder distinguishes a + **table-level** CHECK from a **column-level** one by **element + position** (no column-def open), using a depth-aware tracker so a + length-arg comma (`numeric(10,2)`) or a table-`PRIMARY KEY (a,b)` comma + is not mistaken for an element separator. Composite `UNIQUE` stays + PRAGMA-detected (user-confirmed). `/runda` added the + rebuild-survival + drop-clean + drop-column-referenced cross-cutting + guards. +- **4b — foreign keys in `CREATE TABLE`** (`76d6059`). Inline + ` … REFERENCES [()] [ON DELETE/UPDATE …]` and + table-level `[CONSTRAINT ] FOREIGN KEY () REFERENCES …`, + each an ADR-0013 **named relationship** created in the create + transaction (one undo step). Validation/naming/metadata-insert/ + type-compat factored into **shared helpers** that `do_add_relationship` + was refactored to use (this **pre-pays 4g**). `do_create_table` emits + the `FOREIGN KEY` clause identically to `schema_to_ddl`. **Self-refs** + (validated against the in-statement columns/PK) and **bare + `REFERENCES `** (resolves to the parent's single-column PK; + composite → error) supported (user-confirmed). FKs round-trip via the + existing relationship plumbing — no new persistence structures. + `/runda` added 5 cross-cutting probes (FK survives the rebuild + primitive + a later `rebuild_from_text`; actions survive rebuild; drop + semantics; bare self-ref). +- **4c — `DROP TABLE [IF EXISTS]`** (`e52e90c`). `SqlDropTable` reuses + `do_drop_table` (cascade / inbound-relationship refusal / metadata + cleanup) — full parity with simple `drop table`. `IF EXISTS` on an + absent table is a **no-op-with-note** via a new `DropOutcome { Dropped, + Skipped }` mirroring `CreateOutcome` (journalled, no snapshot), with a + new `ddl.drop_skipped_absent` note + `DslDropSkipped` event. `drop` is + now a **shared entry word** (SQL-first; `drop column`/`relationship`/ + `index`/`constraint` fall back to simple and still execute). + +ADR-0035 stays **Accepted**; README + `requirements.md` Q1 updated each +slice; plan docs `docs/plans/20260525-adr-0035-sql-ddl-4a3.md`, +`…-4b.md`, `…-4c.md`. + +## §3. Design decisions settled this session (do not re-litigate) + +All user-confirmed unless marked implementer-call: + +- **4a.3 metadata table** `__rdbms_playground_table_checks` — focused, + not a generic `table_constraints` (expand for named CHECK in 4g). +- **Composite `UNIQUE` stays PRAGMA-detected** — engine-reportable, + unlike CHECK; the PRAGMA/metadata split is principled and stable. +- **4b self-referencing FK supported** — validated against the + in-statement columns/PK when the parent is the table being created. +- **4b bare `REFERENCES ` supported** — resolves to the + parent's single-column PK; composite-PK parent → "name the column" + error. (The user leaned standard-SQL here.) +- **Inline FK auto-named; only the table-level form takes `CONSTRAINT + `** (ADR §4/§5). PK-target only; UNIQUE-target stays deferred. +- **4c `drop` dispatch:** `drop table T` → `SqlDropTable` in advanced + (equivalent execution to the simple `DropTable`); other `drop` forms + fall back to simple. `IF EXISTS` skip is journalled, not snapshotted. +- **Drop-column-referenced-by-table-CHECK** (4a.3 finding): fails + cleanly (rollback, table intact). Up-front detection is **4e**; + friendly wording is **H1** (user-confirmed split). +- (implementer) The grammar **leading-`Optional` trap**: a `Choice` + branch / element must **start on a concrete keyword**, never an + `Optional` — `walk_seq` advances `idx` past a skipped Optional, so a + later mismatch becomes a hard `Failed` that aborts the enclosing + `Choice` (caught when `TABLE_FK` and `SQL_DROP_TABLE` shapes broke all + `CREATE TABLE` parsing). Hence `TABLE_FK`/`TABLE_FK_NAMED` are split + on `foreign`/`constraint`. + +## §4. The NEXT job — 4d (`CREATE [UNIQUE] INDEX` / `DROP INDEX`) + +**Goal:** `CREATE [UNIQUE] INDEX [] ON (, …)` → +`SqlCreateIndex`, and `DROP INDEX ` → `SqlDropIndex`, mapped to the +ADR-0025 index machinery. Plan it like the others (short plan doc, +test-first, escalate the one open call below). + +**The escalation (surface FIRST, ADR-0035 §13 4d / ADR-0025):** +`CREATE UNIQUE INDEX` needs an **`IndexSchema.unique` flag** — ADR-0025 +**deferred unique indexes**, so the index *model* has no uniqueness slot. +This is a model extension of the same class as 4a.2/4a.3 (a structure +the model doesn't yet carry). **Escalate the index-model extension to +the user before implementing** — it touches `IndexSchema` +(`src/persistence/mod.rs`), the YAML round-trip, and how `read_…` +detects a unique index (PRAGMA `index_list` reports `unique`/origin `c`). + +**Reuse / patterns that carry in:** +- **Shared entry words gain a *second* advanced node.** `drop` already + has `SQL_DROP_TABLE`; 4d adds `SQL_DROP_INDEX` (also entry `drop`), and + `create` gains `SQL_CREATE_INDEX` (entry `create`) alongside + `SQL_CREATE_TABLE`. **Verify the dispatch tries *all* advanced + candidates for an entry word** (`commands_for_entry_word` returns + them; confirm the walker/`completion_probe` actually walks each). This + is new — until now each shared entry word had exactly one advanced + node. Add a parse test: `drop index ix` → `SqlDropIndex`, `drop table + T` → `SqlDropTable`, both in advanced mode. +- **`DROP INDEX IF EXISTS` reuses the 4c skip path verbatim** — the + `DropOutcome::Skipped` + journalled-no-snapshot pattern + the + `ddl.drop_skipped_absent` note (or an index-specific note key). +- Existing index ops: `add index` / `drop index` (ADR-0025) already + exist as DSL commands (`Command::AddIndex` / `DropIndex`, + `do_add_index` / `do_drop_index` in `db.rs`). 4d's SQL forms should + **reuse those low-level executors**, like 4c reused `do_drop_table`. +- Index name auto-generation already exists (the `__idx` + convention, ADR-0025) — mirror it for an unnamed `CREATE INDEX`. + +## §5. Everything else remaining in Phase 4 (ADR-0035 §13) + +In order after 4d: + +- **4e — `ALTER TABLE` add/drop/rename column** (the ADR-0013 rebuild + primitive). **Includes** the drop-column-referenced-by-table-CHECK + up-front detection (the 4a.3 deferral; friendly wording is H1). +- **4f — `ALTER TABLE … ALTER COLUMN TYPE`** — the §7 conversion model; + advanced mode performs lossy conversions with a note + relies on undo + (no force flag). +- **4g — `ALTER TABLE` add/drop constraint, add FK.** The add-FK path + **reuses the 4b shared relationship helpers**; named CHECK adds a + `name` column to `__rdbms_playground_table_checks`. +- **4h — `ALTER TABLE … RENAME TO`** — the §6 new low-level op (rename + table + `data/.csv` + relationship metadata + the **`table_checks` + rows** — the §6 amendment this session added). Advanced-only (`C1`). +- **4i — Verification sweep** (the running deferral tail — §6 below). + +## §6. The 4i list (the running deferral tail — READ THIS) + +**Canonical location: ADR-0035 §13 4i bullet, items (a)–(e).** Every +other mention (the two newest plan docs, the `NOTE (4i)` code comments +in `src/dsl/grammar/sql_create_table.rs` and `src/completion.rs`, the +README) points back to it — there is no independent list. Reproduced +here so the next session sees it without digging: + +- **(a)** Refresh the `CREATE TABLE` help/usage skeleton for the 4a.2 + `DEFAULT`/`CHECK`/composite-`UNIQUE`, 4a.3 table-`CHECK`, and 4b FK + forms (each deferred its skeleton update; 4c's were *added* since the + node is new). Add 4d's index forms too when they land. +- **(b)** `describe` display of **table-level** constraints (composite + `UNIQUE` + table `CHECK`) — `TableDescription` surfaces neither today + (symmetric gap, not a regression). +- **(c)** 4b **self-ref FK pre-submit indicator** — `references ` + parses + executes fine, but the schema-existence diagnostic falsely + flags the not-yet-created self table (FK parent slot is + `IdentSource::Tables`). Teach the diagnostic about the `CREATE TABLE` + target. +- **(d)** **Shared-entry-word completion merge** — advanced-mode `drop ` + offers only `table`; a partial DSL keyword (`drop rel`) returns an + *empty* list (mid-word dead end). The DSL drops still parse + execute. + Merge all candidate nodes' continuations for a shared entry word + (`drop ` → table + column + relationship + index + constraint; `drop + rel` → relationship); verify `create`/`insert`/`update`/`delete` + completion stays sensible. **(4d will widen this** — `drop`/`create` + gain a second advanced node, so the merge matters more.) +- **(e)** **Discussion flag (user):** before/with (d), **discuss + visually distinguishing simple- vs advanced-mode completions in the + hint UI (likely by colour)** — a UX design conversation, not just the + mechanical merge. + +**Watch item:** that 4i bullet is getting long. If 4d–4h keep adding +deferrals, consider promoting it to a dedicated short "4i scope" doc or +a `requirements.md` checklist (flagged to the user, not yet needed). + +## §7. Patterns the implementer must not forget + +1. **Two DDL generators stay in sync.** `do_create_table` and + `schema_to_ddl` both emit a table's DDL; **any new clause must be + emitted by BOTH, identically** (the 4a `serial` lesson). 4b's FK + clause and 4a.3's CHECK clause follow this; round-trip/rebuild tests + are the net. +2. **Reuse low-level executors.** SQL DDL commands wrap the existing + helpers (`do_create_table`, `do_drop_table`, `do_add_index`, the + rebuild primitive, the relationship helpers) — they do not fork. + 4c reused `do_drop_table`; 4d should reuse `do_add_index`/ + `do_drop_index`. +3. **Metadata survives the rebuild primitive** because it uses a **raw + `DROP`** (not `do_drop_table`, which clears `__rdbms_*` rows) + + `schema_to_ddl` re-emit. Proven for CHECK (4a.3) and FK (4b). Any + new metadata keyed by table name inherits this — but **4h RENAME must + update every such table** (the §6 list now includes `table_checks`). +4. **Undo:** every mutating `Sql*` worker variant wraps in + `snapshot_then` (one undo step). The `IF [NOT] EXISTS` skip arms + journal **without** a snapshot (no-op = nothing to undo). +5. **Grammar leading-`Optional` trap** (§3) — element/`Choice` branches + start on a concrete keyword. +6. **Catalog lockstep:** new `help_id`/`usage_id`/note keys need a + `keys.rs` entry **and** an `en-US.yaml` body (the + `keys_validate_against_catalog` test enforces both ways); + engine-neutral wording (the vocab audit enforces it). Help keys carry + a `help.` prefix in `keys.rs`. +7. **Exhaustive matches:** a new `Command`/`Request` variant forces arms + in `command.rs` (verb/`target_table`), `app.rs` (failure-translate), + `runtime.rs` (dispatch + outcome→event), and + `tests/typing_surface/mod.rs` (the matrix label). The compiler finds + them. + +## §8. Other tracked deferred items (nothing lost) + +- **(A)** App-lifecycle-command runtime-failure journalling (ADR-0034 + follow-up). +- **M4** — execution-time mode side-channel (ADR-0033 Amendment 3). +- **`blob` value literal** — `Value` has no blob variant (pre-existing). +- **Undo residual edge** (ADR-0006 note): an entirely-unwritable + `.snapshots/` can leave a stale redo — accepted. +- **CI / TT5**, **DSL→SQL teaching echo** (ADR-0030 Phase 5, after DDL), + then the §6 polish phase. +- Friendlier FK/CHECK error messages (e.g. "create the parent first", + "can't drop a column a CHECK references") — **H1** (the friendly-error + layer is partial; current messages are engine-*neutral* but terse). + +## §9. Process pins (unchanged, still binding) + +- **Confirm every commit.** Propose the message; wait for the go-ahead. +- **Push is the user's step.** Never push; never prompt about it. +- **No AI attribution** in commits (global rule overrides any default). +- **Probe, don't reason.** This session's real findings — the grammar + leading-`Optional` trap, the FK-survives-rebuild invariant, the + drop-column-referenced clean-failure, the uneven `drop ` completion — + all came from *running probes*, not reasoning. Reproduce before + concluding; delete throwaway probes (or promote them) before + committing. +- **Escalate ambiguity / new cost.** 4b's self-ref + bare-ref and 4c's + completion deferral were all surfaced to the user, not decided + silently. 4d's `IndexSchema.unique` extension is the next escalation. +- **Keep docs lockstep.** ADR status/§13 + README + `requirements.md` + update in the same edit; the 4i list lives in ADR §13 4i (§6). +- **DA hat each slice; `/runda` at phase-4 end.** The user ran `/runda` + on 4a.3 and 4b this session (both PASS, both added cross-cutting + tests); 4c got a written DA pass. + +## §10. How to take over + +1. **Read, in order:** this file → `docs/adr/0035-advanced-mode-sql-ddl.md` + (§4/§5 FK, §13 sub-phase list incl. the 4i (a)–(e) tail; **4d is + next**) → the three plan docs (`…-4a3.md`, `…-4b.md`, `…-4c.md`) → + `CLAUDE.md` → `docs/requirements.md` (`Q1`/`Q4`/`C1`). +2. **Baseline:** + ``` + cargo test # expect 1805 passing / 0 failing / 0 skipped / 1 ignored + cargo clippy --all-targets -- -D warnings # clean + ``` +3. **Start 4d** per §4: **escalate the `IndexSchema.unique` model + extension first**, then short plan doc, then implement test-first + (grammar → command/builder → worker reusing `do_add_index`/ + `do_drop_index` → round-trip). Verify the shared-entry-word dispatch + handles a **second** advanced node per entry word. +4. Mirror the established slices: `tests/sql_drop_table.rs` (Tier-3), + the in-crate `sql_drop_table_tests` (parse), and the + `builder_tests`/Tier-3 split used by `CREATE TABLE`.