docs: session handoff 38 — ADR-0035 4a.3 + 4b + 4c shipped; 4d next

Three Phase-4 sub-phases shipped this session (all local-only on main,
1805 tests passing, clippy clean):
- 4a.3 table-level/multi-column CHECK (new __rdbms_playground_table_checks)
- 4b foreign keys in CREATE TABLE (ADR-0013 named relationships)
- 4c DROP TABLE [IF EXISTS] (DropOutcome::Skipped no-op-with-note)

Handoff records the growing 4i deferral list (a–e, canonical in
ADR-0035 §13 4i) and flags the 4d escalation (IndexSchema.unique model
extension for CREATE UNIQUE INDEX).
This commit is contained in:
claude@clouddev1
2026-05-25 17:38:48 +00:00
parent e52e90c45b
commit 44248fb8bb
+283
View File
@@ -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 (ae)** 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
`<col> … REFERENCES <parent>[(<col>)] [ON DELETE/UPDATE …]` and
table-level `[CONSTRAINT <name>] FOREIGN KEY (<col>) 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 <parent>`** (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 <parent>` 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
<name>`** (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 [<name>] ON <table> (<col>, …)` →
`SqlCreateIndex`, and `DROP INDEX <name>` → `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 `<T>_<cols>_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/<t>.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 <self>`
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 4d4h 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`.