diff --git a/docs/handoff/20260525-handoff-40.md b/docs/handoff/20260525-handoff-40.md new file mode 100644 index 0000000..4247103 --- /dev/null +++ b/docs/handoff/20260525-handoff-40.md @@ -0,0 +1,239 @@ +# Session handoff — 2026-05-25 (40) + +Fortieth handover. This session **shipped ADR-0035 Phase-4 sub-phases +4f and 4g** (each test-first, with planning escalation and a *combined* +`/runda` finished-slice critique at the end) **plus a `/runda`-found +rebuild fix**. The next session **implements 4h — +`ALTER TABLE … RENAME TO`** (the §6 new low-level op), then **4i — the +verification sweep**. After 4h+4i, Phase 4 (ADR-0035) is complete. + +## §1. State at handoff + +**Branch:** `main`. **HEAD `50a889e`.** **Tests: 1885 passing, 0 failing, +0 skipped, 1 ignored** (the unchanged `friendly/mod.rs` ` ```ignore ` +doctest). **Clippy:** clean (`cargo clippy --all-targets -- -D warnings`). + +**This session's commits** (oldest → newest, local): + +``` +5b76315 feat: ADR-0035 4f — ALTER TABLE … ALTER COLUMN TYPE +6ff97f6 feat: ADR-0035 4g — ALTER TABLE add/drop constraint + add FK +50a889e fix: ADR-0035 4g — reconstruct table-CHECK metadata on rebuild +``` + +**Git state — no divergence.** `origin/main` is at **`5b76315`** (4f), +so **`6ff97f6` (4g) + `50a889e` (the fix) are unpushed** — a normal +working state. (The earlier `a2fc3c9` docs commit + `5b76315` 4f were +pushed at some point this session.) Push is the user's step; nothing +blocks. Working tree clean. + +## §2. What shipped this session + +- **4f — `ALTER TABLE ALTER COLUMN TYPE `** (`5b76315`). + A fourth `AlterTableAction::AlterColumnType`, **runtime-decomposed to + `change_column_type(…, ChangeColumnMode::ForceConversion)`** — which + **is** the ADR-0035 §7 advanced policy (lossy converts *with a note*, + no force flag; incompatible + the ADR-0017 static refusals still + refuse; **`int → serial` is *allowed*** per ADR-0018 §8 — the §7 + "→serial refused" summary is looser than the code). Builder + discriminates the branch by the **`type` keyword** (unique — ADD + COLUMN's type is an ident; checked **first** because `alter column …` + also contains `column`). Internal-`__rdbms_*` guard folded into + `do_change_column_type`. +- **4g — `ALTER TABLE` add/drop constraint + add FK** (`6ff97f6`). + `ADD [CONSTRAINT ] (CHECK | composite UNIQUE | FOREIGN KEY)` + + `DROP CONSTRAINT `. **User-confirmed scope:** ADD = CHECK + FK + + composite UNIQUE; `ADD PRIMARY KEY` and a *named* UNIQUE are refused + (composite UNIQUE is anonymous in our model). Each ADD reuses a + low-level path with a dry-run guard (table-CHECK/UNIQUE rebuild; FK → + `do_add_relationship`, bare `REFERENCES

` → parent single PK, + `create_fk = false`). `DROP CONSTRAINT` resolves the name to a named + table-CHECK then a child-side named FK, else refuses. **Named-CHECK + round-trip:** a nullable `name` column on + `__rdbms_playground_table_checks` (**rebuild-only** arrival) **and** a + `project.yaml` `check_constraints` extension to a `{expr, name}` + mapping (the bare-string form still reads — back-compatible). The + internal-`__rdbms_*` guard was folded into `do_add_constraint` / + `do_add_relationship`, **completing the 4d/4e/4f guard class**. ADD + CHECK also refuses a name colliding with an existing CHECK *or* + relationship (keeps DROP unambiguous). **Grammar restructured:** the + walker's `Choice` does **not** backtrack between two same-leading- + keyword branches, so the action `Choice` keeps **one branch per verb** + (`add`/`drop`/`rename`/`alter`) and `add`/`drop` fan out to an **inner + `Choice`** whose branches each lead on a *distinct* second keyword + (column / constraint / check / unique / foreign / primary). +- **Rebuild fix** (`50a889e`, `/runda`-found). `do_rebuild_from_text` + re-emitted table-CHECKs into the recreated DDL (so enforced) but + **never repopulated `CHECK_TABLE`** — so a *fresh* rebuild (missing + `.db`, reconstructed from `project.yaml`) lost the CHECK metadata + (incl. the name): `DROP CONSTRAINT` / `describe` / a later save would + drop it. In-place rebuilds only worked because the wipe never touched + the table. (Latent since 4a.3 for unnamed checks.) Rebuild now wipes + + repopulates `CHECK_TABLE` from yaml (name + seq + expr), like + META/REL, and adds the `name` column if a pre-4g table predates it + (the rebuild-only migration). Regression test: + `e2e_named_check_metadata_survives_a_fresh_rebuild`. + +ADR-0035 stays **Accepted**; README + `requirements.md` Q1 + ADR §13 +(4f, 4g) updated each slice. + +## §3. The NEXT job — 4h (`ALTER TABLE … RENAME TO`) + +**No plan written yet** — 4h needs its own plan (Phase 1→3 + a planning +`/runda`/written-DA gate) before building, because it is the **one +genuinely new low-level op** in Phase 4 (ADR-0035 §6), not a reuse. + +**Spec (ADR-0035 §6):** `ALTER TABLE RENAME TO ` is +**advanced-mode only** (no simple-mode rename-table verb). Within **one +transaction**: +1. Rename the table in the database. +2. Rename its `data/.csv` → `data/.csv` (persistence layer). +3. Update **every** metadata row that names it: + - `__rdbms_playground_columns` rows (`table_name`), + - **both ends** of any `__rdbms_playground_relationships` row + (`parent_table` *and* `child_table`), + - **`__rdbms_playground_table_checks` rows** (`table_name`, added in + 4a.3, keyed by table name). +4. Name validation + `__rdbms_*` rejection apply to the **target**. + +Closes the rename half of `C1` for the advanced surface. + +**Grammar:** a new `AlterTableAction::RenameTo { new: String }` (or +similar) — a **fifth** action branch in `AT_ACTION_CHOICES`. It leads on +`rename` — **but `rename column` already leads on `rename`** (the walker +`Choice` does not backtrack same-led branches; see §7.4). So `rename` +must follow the §2 pattern: one `rename` branch with an inner `Choice` +on the distinct second token (`column` → rename-column; `to` → rename- +table). Mirror the `add`/`drop` restructure 4g did. The builder keys on +`contains_word("column")` (already the §4f/§4g discriminator order) vs a +bare `rename … to`. + +**Executor:** a genuinely new `do_rename_table` (no existing reuse) — the +CSV rename is a persistence-layer concern (cf. how `do_drop_table` cleans +CSV + metadata; ADR-0015 §6 ordering, commit-db-last). One undo step +(snapshot is whole-project). Internal-`__rdbms_*` guard on the target +(and refuse renaming an internal table as source — `reject_internal_ +table_name` on both). + +**Tests:** Tier-1 parse (rename-table vs rename-column dispatch); Tier-3 +e2e via `run_replay` — rename a table that has rows (CSV follows), a +relationship (both ends update), and a table-level CHECK (the +`table_checks` rows follow); **survives rebuild** (the renamed table + +its metadata round-trip through yaml); one undo step; refuse rename to an +existing name / to an `__rdbms_*` name. + +## §4. Then 4i — verification sweep (the running deferral tail) + +Canonical list: ADR-0035 §13 4i. Unchanged from handoff-39 except 4f/4g +now carry their own help/usage (so the *CREATE TABLE* skeleton refresh is +the only help/usage debt left). The tail: +- **(a)** Refresh the **`CREATE TABLE` help/usage skeleton** for the + 4a.2 `DEFAULT`/`CHECK`/composite-`UNIQUE`, 4a.3 table-`CHECK`, 4b FK + forms (4d/4e/4f/4g forms already carry their own help/usage). +- **(b)** `describe` display of **table-level constraints** (composite + `UNIQUE` + table `CHECK`, now incl. **named** CHECKs from 4g) — the + unique-*index* `[unique]` marker shipped in 4d, so only the table- + level *constraint* display remains. +- **(c)** 4b **self-ref FK pre-submit indicator** (the diagnostic + false-flags a `references ` parent as unknown). +- **(d)** **shared-entry-word completion merge** — `create`/`drop` (and + now constraint forms) should offer every valid continuation across the + SQL + DSL nodes; 4d/4g widened this. +- **(e)** **user discussion flag (2026-05-25):** visually distinguish + simple- vs advanced-mode completions in the hint UI (likely colour) — + a UX design conversation, before/with (d). +- Plus the §4i staples: typing-surface + matrix coverage, engine-neutral + error pass, undo-parity (one step per statement). + +## §5. Tracked follow-ups (not lost) + +- **H1 wording — `--create-fk` leak (4g).** `ALTER … ADD FOREIGN KEY` on + a *missing* child column reuses `do_add_relationship`'s error, which + mentions the DSL flag `--create-fk` — off-key in an SQL context. Fix + the wording for the SQL path (or branch the message). H1. +- **Coverage gap — pre-4g 3-column `CHECK_TABLE` migration (4g/fix).** + The rebuild-time `ALTER … ADD COLUMN name` migration and the named- + CHECK-on-old-project refusal (`check_table_has_name_column`) are + logically sound + guarded, but the **genuinely-old 3-column table** + in-place path is **untested** — a 3-col `CHECK_TABLE` can't be + fabricated through the public API (no raw-SQL exec). The common case + (old project whose `.db` is deleted → fresh rebuild → 4-col table) IS + tested. A test helper (raw-SQL exec, or a fixture `.db`) would close + it; flagged to the user, not yet decided. +- **H1 friendly wording** for the 4e CHECK-guard refusals and the 4f + type-conversion diagnostics (engine-neutral but terse) — carried. +- The §8 items from handoff-39 (app-lifecycle-command runtime-failure + journalling; M4 execution-time mode side-channel; `blob` value + literal; CI/TT5; DSL→SQL teaching echo) remain open. + +## §6. Patterns the implementer must not forget + +1. **Walker `Choice` does not backtrack between same-leading-keyword + branches.** This bit 4g (two `add` branches "expected column"). Keep + **one branch per leading verb**, fan out on a **distinct** second + token via an inner `Choice`. 4h's `rename` must join `rename column` + under one `rename` branch the same way. +2. **Reuse low-level executors; the runtime decomposes `SqlAlterTable`.** + 4f wrapped `change_column_type`; 4g wrapped `do_add_relationship` + + added `do_alter_add_table_check`/`do_alter_add_unique`/ + `do_drop_constraint_by_name`. 4h is the exception — a genuinely **new** + `do_rename_table` (§6). +3. **Two DDL generators stay in sync** (`do_create_table` / + `schema_to_ddl`). 4g added the `CONSTRAINT CHECK` emission to + `schema_to_ddl`; `do_create_table`'s table-CHECK insert writes + `name = NULL` (CREATE-TABLE CHECKs stay unnamed — an ALTER-only + naming, a noted consistency item). +4. **`do_rebuild_from_text` reconstructs ALL metadata from yaml.** It now + wipes + repopulates `META` / `REL` / **`CHECK_TABLE`** and recreates + indexes. **A new metadata table or column MUST be added to both the + wipe and the repopulation** or a fresh rebuild silently drops it (the + exact bug `50a889e` fixed). 4h touches metadata *values* (renames), so + verify a renamed table round-trips through a fresh rebuild. +5. **Exhaustive matches:** a new `AlterTableAction` variant forces arms + in `runtime.rs` (inner `match action`) and `app.rs` + (`build_translate_context`'s inner match). The compiler finds them. +6. **Catalog lockstep + vocab audit** — refresh `sql_alter_table` + help/usage for the rename form; keep wording engine-neutral + (`engine_vocabulary_audit` enforces it). +7. **Probe, don't reason** — the combined `/runda` caught the rebuild bug + precisely because a test was written for the *fresh*-rebuild path the + original e2e skipped. For 4h, test the CSV-rename + both-relationship- + ends + table_checks-follow paths explicitly, and a fresh rebuild. + +## §7. Process pins (unchanged, still binding) + +- **Confirm every commit.** Propose the message; wait for the go-ahead. + No AI attribution. +- **Push is the user's step.** Never push/force-push. Unpushed commits + are normal (see §1). +- **Escalate ambiguity / new cost.** 4g's three scope forks (ADD set, + CHECK migration, DROP scope) and the discovered `project.yaml`-format + implication were all surfaced + user-decided before building. Do the + same for 4h's open calls (e.g. rename-to-existing-name behaviour, any + describe/echo wording). +- **DA hat each slice; `/runda` at the planning exit AND on the finished + slice.** The user opted to **combine** the finished-slice `/runda` + across 4f+4g this session — it found a real bug (the rebuild metadata + loss). Budget for at least one finished-slice `/runda` covering 4h + (+4i); it pays off. +- **Keep docs lockstep** — ADR status/§13 + README + `requirements.md` + in the same edit. + +## §8. How to take over + +1. **Read, in order:** this file → + `docs/adr/0035-advanced-mode-sql-ddl.md` (§6 rename, §13 4h/4i) → + `CLAUDE.md` → `docs/requirements.md` (`Q1`, `C1`). The 4f/4g plans + (`docs/plans/20260525-adr-0035-sql-ddl-4f.md`, `…-4g.md`) are good + models for the **4h plan you must write**. +2. **Baseline:** + ``` + cargo test # expect 1885 passing / 0 failing / 0 skipped / 1 ignored + cargo clippy --all-targets -- -D warnings # clean + ``` +3. **Plan 4h** (Phase 1→3 + planning `/runda`), surfacing any open calls + to the user; then implement test-first per §3 (grammar restructure of + the `rename` branch → `do_rename_table` executor → runtime/app arms → + Tier-1/Tier-3 tests incl. a fresh rebuild → catalog + docs lockstep → + finished-slice `/runda` → commit proposal). +4. **Then 4i** (§4) — the verification sweep closes Phase 4.