claude@clouddev1
240 lines
13 KiB
Markdown
240 lines
13 KiB
Markdown
# 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 <T> ALTER COLUMN <col> TYPE <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 <name>] (CHECK | composite UNIQUE | FOREIGN KEY)` +
|
|
`DROP CONSTRAINT <name>`. **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 <P>` → 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 <old> RENAME TO <new>` is
|
|
**advanced-mode only** (no simple-mode rename-table verb). Within **one
|
|
transaction**:
|
|
1. Rename the table in the database.
|
|
2. Rename its `data/<old>.csv` → `data/<new>.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 <self>` 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 <name> 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.
|