diff --git a/docs/handoff/20260525-handoff-37.md b/docs/handoff/20260525-handoff-37.md new file mode 100644 index 0000000..b27619d --- /dev/null +++ b/docs/handoff/20260525-handoff-37.md @@ -0,0 +1,255 @@ +# Session handoff — 2026-05-25 (37) + +Thirty-seventh handover. This session **implemented ADR-0035 Phase 4 +sub-phases 4a and 4a.2** (advanced-mode SQL `CREATE TABLE`), flipped +ADR-0035 to **Accepted**, and ran a `/runda` pass on 4a that found and +fixed two real defects. The next session **implements sub-phase 4a.3 — +table-level / multi-column `CHECK`** (the one constraint that needs a +new internal metadata table). See §4. + +## §1. State at handoff + +**Branch:** `main`. **Tests: 1752 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):** `c0f5626` (4a.2). `origin/main` is at +`df6aa69`; **everything since is local-only** (10 commits: handoff-36's +two + this session's eight). Unpushed commits are a normal working +state; pushing is the user's step — do not prompt about it. + +**This session's commits** (oldest → newest): + +``` +19d3cd3 docs: ADR-0035 — record two /runda refinements +093496f docs: ADR-0035 4a plan + 4a.2 split +94ec87b docs: ADR-0035 4a — refine scope +58386d7 feat: ADR-0035 4a — SQL type-alias resolver +8031092 feat: ADR-0035 4a — SQL CREATE TABLE grammar shape +631074f feat: ADR-0035 4a — command, worker, and exit gate +1c50133 docs: ADR-0035 4a.2 plan + split table-level CHECK to 4a.3 +c0f5626 feat: ADR-0035 4a.2 — per-column CHECK/DEFAULT + composite UNIQUE +``` + +## §2. What shipped this session + +**Advanced-mode SQL `CREATE TABLE` is live**, executed *structurally* +(ADR-0035 §1) through the existing `do_create_table` — an +advanced-created table is a first-class playground object. + +- **4a** (`58386d7`/`8031092`/`631074f`): `Command::SqlCreateTable` + + grammar (`src/dsl/grammar/sql_create_table.rs`) + worker. Surface: + columns + the type alias map (`Type::from_sql_name`, incl. the + two-word `double precision` + ignored length args) + + `NOT NULL`/`UNIQUE`/column- & table-level `PRIMARY KEY` + + `IF NOT EXISTS` (no-op-with-note via `CreateOutcome::Skipped`). + Shared-`create`-word dispatch (SQL-first, DSL fallback). No-PK tables + allowed. One undo step. +- **4a.2** (`c0f5626`): per-column `DEFAULT`/`CHECK` (raw `sql_expr` + text, captured by byte span — `sql_expr` builds no AST) + composite + `UNIQUE(a,b)`. CHECK round-trips via + `__rdbms_playground_columns.check_expr`; DEFAULT via `PRAGMA + table_info`; composite UNIQUE via a new `TableSchema.unique_constraints` + field detected from `PRAGMA index_list` origin `u`. **No new internal + table.** +- **ADR-0035 → Accepted** (validated end-to-end by 4a); README + + `requirements.md` Q1 updated; plan docs + `docs/plans/20260524-adr-0035-sql-ddl-4a.md` and + `…-4a2.md`. +- **`/runda` on 4a found + fixed two defects** (probe, don't reason): + (1) a do_create_table-vs-`schema_to_ddl` inline-PK **round-trip + drift** (the "serial needs inline PK" premise was wrong — `serial` + auto-fill is independent of rowid-alias; aligned both generators to + the first-column rule); (2) the `IF NOT EXISTS` no-op **wasn't + journalled**. Both regression-tested. + +## §3. Design decisions settled this session (do not re-litigate) + +All user-confirmed unless marked implementer-call: + +- `IF [NOT] EXISTS` **admitted** (no-op-with-note) — near-universal + cross-vendor idiom, not engine-specific (verified by web search). +- `INTEGER PRIMARY KEY` → **plain `int`** (not auto-increment); `serial` + is the sole auto-increment type. +- **No-PK tables allowed** in advanced mode (standard SQL; the §7 trust + posture), unlike simple mode. +- `DEFAULT`/`CHECK`/table-level `UNIQUE`/`CHECK` **deferred** (now + 4a.2/4a.3); until landed they're **parse errors** (the usage skeleton + shows the supported surface — a friendly bespoke "not yet supported" + message was judged unnecessary for a deferred form). +- `double precision` (implementer): a keyword-pair branch in the type + slot; the lone two-word alias. +- inline-PK rule (implementer): `do_create_table` matches + `schema_to_ddl` (inline only a **first-column** single PK) — keeps the + create and rebuild DDL identical (no round-trip drift). +- redundant PK constraints (implementer): advanced mode accepts + `id int primary key not null` and silently de-dups the flag. +- **4a.2 / 4a.3 split** — table-level `CHECK` is the only constraint + that needs a new internal table (SQLite has no PRAGMA for CHECK), so + it earns its own slice. +- `DEFAULT` is a **literal or a parenthesised expression** (standard + SQL), not a bare `sql_expr` — a bare expr greedily eats a following + `NOT` (`NOT IN`/`LIKE`/`BETWEEN`), breaking `DEFAULT 0 NOT NULL`. + +## §4. The NEXT job — sub-phase 4a.3 (table-level / multi-column `CHECK`) + +**Goal:** `CREATE TABLE t (a int, b int, CHECK (a < b))` — a +table-level CHECK referencing multiple columns. Plan it like 4a/4a.2 +(short plan doc, test-first). The defining difficulty: **SQLite exposes +no PRAGMA for CHECK constraints**, so a table-level CHECK *cannot be read +back from the engine* and must live in a **new `__rdbms_*` metadata +table** as its source of truth (the ADR-0012/0013 pattern). This is the +whole reason it was split out. + +**Sketch (confirm specifics with the user as they arise):** + +1. **Grammar** (`sql_create_table.rs`): add a table-level + `CHECK ( sql_expr )` element to `ELEMENT_CHOICES` (today only + `TABLE_PK`, `TABLE_UNIQUE`, `COLUMN_DEF`). Update the shape test + `table_level_check_and_fk_still_rejected` — table CHECK becomes + accepted; **FK stays rejected** (4b). +2. **Command**: `SqlCreateTable` gains `check_constraints: Vec` + (raw inner SQL texts). The builder captures each via the existing + `capture_parenthesised_span`; distinguish a **table-level** CHECK + (element position — appears where a column name would start) from a + **column-level** CHECK (after a column's type — already handled). +3. **New metadata table** — e.g. `__rdbms_playground_table_constraints + (table_name TEXT, seq INT, check_expr TEXT, PRIMARY KEY(table_name, + seq))`. Create it in the `configure_connection` `__rdbms_*` setup + (it's auto-filtered from `list_tables` by the `__rdbms_` prefix). It + is the source of truth — `read_schema` reads table CHECKs from it + (not PRAGMA), exactly as `check_expr` works for column CHECKs. +4. **Both DDL generators** (§6.1): `do_create_table` AND `schema_to_ddl` + must emit `, CHECK (expr)` table clauses identically, and + `do_create_table` must write the metadata rows in its transaction. +5. **Round-trip**: `ReadSchema` + `TableSchema` gain `check_constraints`; + `read_schema` reads from the metadata table; `read_schema_snapshot` + maps it; YAML `RawTable`/`write_table`/`parse_schema` round-trip it + (`#[serde(default)]`, optional-on-read — mirror `unique_constraints`). +6. **Tests**: builder (table CHECK captured, distinct from column + CHECK) + Tier-3 (enforced + **survives rebuild** — the part-D proof) + + a YAML round-trip unit test for the metadata. + +Open question to escalate when reached: whether composite `UNIQUE` +should *also* move into the new metadata table for uniformity, or stay +PRAGMA-detected (current). Default: leave UNIQUE on PRAGMA (it works); +each constraint uses the simplest correct mechanism. + +## §5. Everything else remaining in Phase 4 (ADR-0035 §13) + +In order after 4a.3: + +- **4b — Foreign keys in `CREATE TABLE`.** Inline `REFERENCES` + + table-level `FOREIGN KEY` → ADR-0013 **relationship metadata** (one + statement = one undo step). The grammar rejects FK today (the §4-step + test asserts it). Builder routes FK clauses to the relationship + machinery; `Type::fk_target_type` (ADR-0011) governs compatibility. +- **4c — `DROP TABLE [IF EXISTS]`** → `SqlDropTable` (cascade parity + with the DSL `drop table`; `IF EXISTS` no-op-with-note — mirror the + `CreateOutcome::Skipped` pattern with a `DropOutcome`). +- **4d — `CREATE [UNIQUE] INDEX` / `DROP INDEX`** → `SqlCreateIndex` / + `SqlDropIndex`. **`CREATE UNIQUE INDEX` needs an `IndexSchema.unique` + flag** — ADR-0025 deferred unique indexes (a model extension, same + class as 4a.2/4a.3). Escalate the index-model extension when reached. +- **4e — `ALTER TABLE` add/drop/rename column** (builds on the ADR-0013 + rebuild-table primitive). +- **4f — `ALTER TABLE … ALTER COLUMN TYPE`** — the ADR §7 conversion + model: advanced mode **performs lossy conversions with a post-op note + and relies on `undo`** (no force flag), unlike simple mode's + refuse-by-default. +- **4g — `ALTER TABLE` add/drop constraint, add FK.** +- **4h — `ALTER TABLE … RENAME TO`** — the `C1` table-rename: a + **genuinely new low-level op** (rename the table + its `data/.csv` + + both ends of every relationship row), advanced-mode only. +- **4i — Verification sweep.** Typing-surface + **matrix coverage**, + engine-neutral error pass, undo-parity (one step per statement), + `help`/usage for the new forms. **Deferred to here from 4a:** the + Tier-2 insta snapshot (skipped as redundant) and the typing-surface + matrix entries for the new commands. A full `/runda` pass is planned + at the **end of Phase 4** (user's call). + +## §6. Patterns the implementer must not forget + +1. **Two DDL generators must stay in sync.** `do_create_table` + (`src/db.rs`, create path) and `schema_to_ddl` (rebuild path) both + emit a table's DDL. **Any new constraint must be emitted by BOTH, + identically**, or the table drifts between create and rebuild — the + exact 4a `serial` bug `/runda` caught. There is no shared helper yet; + when they diverge, round-trip tests are the safety net. +2. **Round-trip path:** `read_schema_snapshot` (db → `SchemaSnapshot`, + called by `finalize_persistence` after every mutation) → `project.yaml` + → `build_read_schema` (yaml → `ReadSchema`, on rebuild) → + `schema_to_ddl`. A new structure must be **detectable on read** — + from PRAGMA where the engine reports it (composite UNIQUE), from a + `__rdbms_*` metadata table where it doesn't (CHECK). +3. **The litmus test (the recurring rule):** a DDL feature needs new + model / metadata / execution work *only* when it introduces a + structure simple mode could never produce, or one the engine can't + report. Most of advanced DDL is syntax-only + reuse. +4. **Undo:** SQL DDL is `SqlCreateTable`/`Sql*`, already wrapped in + `snapshot_then` (one undo step). New mutating `Sql*` worker variants + must be wrapped too; `create`/`drop`/`alter` are **writes**, not in + ADR-0034's app-lifecycle skip set (they replay). +5. **Raw-text capture:** `sql_expr` builds no AST; capture expression + text by byte span via `capture_parenthesised_span` / + `capture_expr_span` (`src/dsl/grammar/ddl.rs`). Watch the greedy-`NOT` + trap — bound expressions with parens (the DEFAULT lesson). +6. **Catalog + keys lockstep:** every new `help_id`/`usage_id`/ + diagnostic key needs a `keys.rs` entry **and** an `en-US.yaml` body + (`keys_validate_against_catalog` enforces both directions); + engine-neutral wording (the vocab audit enforces it). +7. **Persistence struct churn:** adding a field to `ColumnSpec` / + `TableSchema` / `ReadSchema` breaks struct-literal sites — the + compiler finds them; test fixtures need the field too. + +## §7. 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; + needs its own ADR). +- **`blob` value literal** — `Value` (`src/dsl/value.rs`) has no blob + variant; pre-existing gap. +- **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. + +## §8. 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). +- **Probe, don't reason.** This session's two real bugs (round-trip + drift, `DEFAULT 0 NOT NULL`) were found by *running probes / writing + the failing test*, not by reasoning. Reproduce before concluding; + delete throwaway probes (or promote them to real tests) before + committing. +- **Escalate ambiguity / new cost.** Every scope split this session + (4a.2, 4a.3, the CHECK/DEFAULT defer) came from surfacing a newly + discovered cost to the user, not deciding silently. +- **Keep docs lockstep.** ADR status / scope changes update + `docs/adr/README.md` and `requirements.md` in the same edit. +- **DA hat any time; `/runda` at phase-4 end.** Per the user: the + end-of-Phase-4 `/runda` is the formal gate, but wear the DA hat to + verify each slice (write the critique down). +- **Terminology:** the **DSL is the one unified grammar**; the real + axis is **mode-availability** (simple / advanced / both). + +## §9. How to take over + +1. **Read, in order:** this file → `docs/adr/0035-advanced-mode-sql-ddl.md` + (§13 sub-phase list; 4a.3 is next) → the two plan docs + (`docs/plans/20260524-adr-0035-sql-ddl-4a.md`, + `…-4a2.md`) → `CLAUDE.md` → `docs/requirements.md` (`Q1`/`Q4`/`C1`). +2. **Baseline:** + ``` + cargo test # expect 1752 passing / 0 failing / 0 skipped / 1 ignored + cargo clippy --all-targets -- -D warnings # clean + ``` +3. **Start 4a.3** per §4: short plan doc, escalate the metadata-table + shape + the composite-UNIQUE-uniformity question, implement + test-first (grammar → command/builder → metadata table → both DDL + generators → round-trip tests). +4. Mirror the established slices: `tests/sql_create_table.rs` (Tier-3), + `builder_tests` in `sql_create_table.rs` (Tier-1).