diff --git a/docs/handoff/20260508-handoff-4.md b/docs/handoff/20260508-handoff-4.md new file mode 100644 index 0000000..deaa215 --- /dev/null +++ b/docs/handoff/20260508-handoff-4.md @@ -0,0 +1,484 @@ +# Session handoff — 2026-05-08 (4) + +Fourth handover on what's been a busy day. The previous +session shipped track 2's export/import + --resume + +persistent history + migration scaffold (handoff-3). This +session built on top: pretty-table rendering, B2/C2 column +operations, a small UX polish pass, and an ADR-driven design +for column type-change compatibility (ADR-0017 — the next +session's main job to implement). + +## State at handoff + +**Branch:** `main`. Working tree clean. Commits since +handoff-3: + +``` +c3e5f90 ADR-0017 + ADR-0002 amendment: type-change compatibility + + engine-agnostic posture +7b97786 B2/C2: column drop / rename / change-type DSL commands +41cef53 parser: make `to` and `table` independently optional + in add column +1b27a0c runtime: suppress silent-rebuild banner for empty + projects +5b5e08d ADR-0016 + Iter 5/6 follow-up: pretty table rendering +67d68db Iteration 6: --resume + persistent input history + + migration scaffold +c6cf3df Iteration 5: export / import commands +``` + +**Tests:** **449 passing, 0 failing, 0 skipped** (up from +345 at handoff-2's baseline; +104 over the last two +sessions). + +**Clippy:** clean with `nursery` lints enabled. + +**Release build:** ~7.0 MB single binary (up ~600 KB from +handoff-3's 6.9 MB; the increase is the column-ops code +path and additional tests-only items don't ship). The zip +crate from Iter 5 is unchanged. + +**Branch state vs origin:** 8 commits ahead of `origin/main`. +Push remains the user's call. + +## What's implemented (delta vs. handoff-3) + +The previous handoff covered: Iter 1–6 of track 2 +(file-backed projects, per-command persistence, rebuild, +save/save as/new/load, export/import, --resume, persistent +history, migration scaffold). This session adds: + +### Pretty table rendering (ADR-0016, commit 5b5e08d) + +- New `src/output_render.rs` — hand-rolled, + Unicode-box-drawing renderer with header-only border + style (`┌─┐│└─┘├─┤`). Public surface: + - `render_data_table(&DataResult) -> Vec` + - `render_structure(&TableDescription) -> Vec` +- **Type-aware alignment**: `DataResult` was extended with + `column_types: Vec>`, populated from the + same metadata lookup `describe_table` already does. + Numeric types (int, real, decimal, serial) + right-align; everything else left-aligns. No + string-heuristics: types are the source of truth. +- **NULL rendering**: `(null)`. Cell newlines / tabs / + control chars become `↵` / `→` / `·` for display only; + underlying data is untouched and round-trips through + CSV cleanly. +- 18 new unit tests in `output_render.rs` plus 4 insta + snapshots pinning the canonical layouts. +- Replaces the old pipe-and-dash `render_data_view` in + `app.rs`. The structure rendering now lives entirely in + the renderer module; `handle_dsl_success` just calls it. + +### Silent-rebuild banner fix (commit 1b27a0c) + +A fresh-launch temp project hits the on-startup +"rebuild from text" path (because no `.db` exists yet) +and used to surface "[ok] rebuild — 0 tables and 0 rows +will be reconstructed." Now `runtime::run()` suppresses +the banner when the project has no schema. The explicit +`rebuild` command still always reports — the user +explicitly asked. + +### Parser: optional `to`/`table` in add column (commit 41cef53) + +Both prepositions are now independently optional, so all +four variants parse identically: + +``` +add column to table T: c (text) +add column to T: c (text) +add column table T: c (text) +add column T: c (text) +``` + +Matches the convention elsewhere in the DSL where bare +identifiers are accepted in unambiguous positions. New +helper `optional_keyword(kw)` that the column-ops +parsers (next bullet) reuse. + +### B2/C2: drop / rename / change column commands (commit 7b97786) + +Closes B2 (rebuild-table primitive reused outside +relationships) and C2 (full add/drop/rename/change-type +column operations). Three new DSL commands: + +- **`drop column [from] [table] : `** — uses + SQLite's native `ALTER TABLE … DROP COLUMN` (3.35+) + + metadata cleanup in `__rdbms_playground_columns`. + Refuses PK columns and columns involved in any + declared relationship (drop the relationship first). +- **`rename column [in] [table] : to `** — + uses `ALTER TABLE … RENAME COLUMN` (3.25+); the engine + cascades the rename through FK declarations on other + tables. Mirrors the rename into BOTH metadata tables + (`__rdbms_playground_columns` and + `__rdbms_playground_relationships`) so describes stay + accurate. Refuses identity rename and name collisions. +- **`change column [in] [table] : ()`** + — routes through the rebuild-table primitive + (ADR-0013) since `ALTER TABLE` doesn't support type + changes. **Currently** refuses PK columns, + relationship-involved columns, `serial` target, and + no-op same-type. Data conversion compatibility is + delegated to STRICT typing during the + `INSERT INTO new SELECT FROM old` step — which is the + spec gap that ADR-0017 fills (next session). + +20 new tests (11 parser, 9 db-layer) covering happy +paths, refusals, and metadata propagation. + +### ADR-0017: column type-change compatibility (commit c3e5f90) + +The big design piece of this session. The B2/C2 change +command's "let STRICT decide" placeholder is replaced +with a curated model. Headlines: + +- **Per-cell three-tier outcome model**: every cell + classifies as **clean** (transformer produces a value + the target accepts losslessly), **lossy** (valid + output, but information discarded — e.g. real → int + truncation), or **incompatible** (no transformer can + produce a valid output — e.g. text "abc" → int). +- **Static transformer matrix** for numeric chains, + text↔structured types, and always-clean + stringifications. The text → int chain narrowest-first: + try int parse (clean), fall back to real parse + truncate + (lossy), else incompatible. +- **PK columns refined**: B2/C2's blanket PK refusal + becomes conditional. The cascade only fires when the + new type would change `fk_target_type()` (per + ADR-0011). So `serial → int` and `shortid → text` on + FK-referenced PKs are now **allowed** (both preserve + the FK target type); `int → text` etc. still refuse. +- **Uniqueness-bearing columns** (PK + shortid) get a + post-transformation collision check. Lossy + transformations that collapse distinct source values + to the same target are classified as incompatible + (cross-row, structural; no `--force-conversion` + override). +- **Override flags**: `--force-conversion` accepts loss + but still refuses incompatibles; `--dont-convert` + skips the entire client-side layer (raw values to + STRICT). Mutually exclusive. +- **Pedagogical `[client-side] …` note** in the success + summary whenever the transformer rewrote any cell — + the moment that tells the learner "the tool did this + for you; raw SQL would need a CAST or application + code." +- **Diagnostic row lists rendered via the pretty-table + renderer** (ADR-0016) — bordered, capped at 100 rows, + with the trailing `… and N more` row inside the box. +- **Forward-look** captured for `--default ` / + `--on-incompatible ''` resolution flags + (preserves design space; not in v1). + +Companion change: ADR-0002 gained a new "User-facing +posture" section cementing that the engine choice is an +implementation detail and is never named in user-visible +strings (no "SQLite", no "STRICT", no "PRAGMA"). A +matching bullet was added to `CLAUDE.md`'s working-style +rules so every session picks it up. + +## ADR index (read these before touching the related areas) + +``` +0000 Record architecture decisions (process) +0001 Language and TUI framework (Rust + Ratatui) +0002 Database engine + — amended in this session: User-facing posture + (no engine name in user-visible strings). +0003 Input modes and command dispatch +0004 Project file format + — amended by 0015 +0005 Column type vocabulary +0006 Undo snapshots and replay log (deferred) +0007 Sharing and export + — amended by 0015 amendment 1 (history.log out of + export zip) +0008 Testing approach +0009 DSL command syntax conventions +0010 Database access via worker thread +0011 FK column type compatibility + — referenced by 0017 §4.1 +0012 Internal metadata for user-facing column types +0013 Relationships, naming, and rebuild-table strategy + — primitive reused by ADR-0017's change-column + implementation +0014 Data operations, value literals, and auto-show +0015 Project storage runtime (track 2; fully implemented + through Iterations 1–6) +0016 Pretty table rendering for data and structure views +0017 Column type-change compatibility + — DESIGNED. Implementation pending; the next + session's main task. +``` + +## Pending — proposed next moves (in order) + +### 1. Implement ADR-0017 + +This is the biggest chunk and the obvious next step. The +ADR is fully specified — no further design work is needed +before coding. Estimated scope: ~600–800 lines + tests. + +The work splits naturally into a few pieces: + +1. **Transformer matrix module** (`src/dsl/types.rs` + alongside `fk_target_type()`, or a new + `src/type_change.rs`). Per-pair functions + `transform(src_value, target_type) -> CellOutcome` + where `CellOutcome ∈ {Clean(new), Lossy(new, reason), + Incompatible(reason)}`. Reuse the validators in + `dsl/value.rs` for grammar checks; the lossy/clean + distinction is computed fresh per pair. +2. **Parser additions**: `--force-conversion` and + `--dont-convert` flags on the `change column` parser. + The flags conform to ADR-0009's `--` convention. + Mutual-exclusion check rejects both at parse time. +3. **`do_change_column_type` rewrite** in `db.rs`. Replace + the current "refuse PK / refuse relationship-involved + / call rebuild_table" body with: + - Static refusal preconditions (incl. the refined + `fk_target_type`-aware PK rule per ADR-0017 §4). + - Per-cell dry run that reads each row's value and + classifies via the matrix. + - Post-transformation uniqueness check for PK + + shortid columns. + - If `--dont-convert`: skip dry run, hand raw to + `rebuild_table` (engine STRICT enforces). Wrap any + engine error in a friendly form (no engine name — + CLAUDE.md rule). + - Otherwise: refuse on incompatibles, refuse on lossy + unless `--force-conversion`, proceed with rebuild + using a new `rebuild_table_with_transform` variant + (or a parameter on `rebuild_table`) that streams + rows through the transformer rather than the + identity SQL `SELECT`. +4. **Diagnostic rendering**: a small helper in + `output_render.rs` (or alongside it) that produces + the bordered tables for lossy / incompatible / + uniqueness-collision cases per ADR-0017 §7. +5. **`[client-side]` note**: the success path adds the + note when any cell was non-identity transformed. +6. **Tests**: one per cell-classification per pair + (clean / lossy / incompatible), the FK-cascade + refinement, the uniqueness check (PK + shortid), + `--force-conversion` allowing lossy, `--dont-convert` + bypassing client-side, mutual exclusion of the two + flags. Probably 30–50 new tests. + +The B2/C2 implementation lives in `db.rs` at +`do_change_column_type` (line ~1550 area). That's the +function to rewrite. The B2/C2 tests at `change_column_*` +in `db.rs::tests` will mostly stay relevant; some +assertions about the PK refusal will need updating to +match the new finer-grained rule (PK without inbound FK +is now allowed). + +### 2. Query DSL ADR + implementation + +The biggest remaining design piece. Earlier discussion +landed on: extend `show data` into a SELECT-style command +with WHERE / projection / order; expose generated SQL as a +pedagogical hook; bundle C5a's complex WHERE into one +coherent feature. Needs an ADR (operator subset, NULL +semantics, projection grammar, "see the SQL" UX). Then +QA1 (EXPLAIN QUERY PLAN) becomes meaningful since queries +will exist. + +### 3. CI (TT5) + +Test infrastructure exists; the GitHub Actions workflow +file does not. One file, locks in the green baseline +across Linux / macOS / Windows. + +### 4. Friendly error layer (H1) + +Partial — FK errors are enriched both ways. The remaining +work is a general engine-error → learner-language +translator. Per ADR-0002's new User-facing posture, the +posture is "never expose engine error text verbatim"; +H1's full implementation is what makes that +operationally true. + +### 5. `replay` command (U4) + +`history.log` is already replay-compatible. The command +runs commands from a `history.log` or `.commands` file. +Modest scope. + +### 6. Bigger UX projects + +V4 (session log + Markdown export), V1/V2 pretty +rendering refinements (relationship-rendering ADR — the +"two structures + arrow" view), B2 column operations' +edge cases (e.g. UNIQUE constraints when C3 lands). + +## Sharp edges and subtleties (delta vs. handoff-3) + +Carried-over edges still apply (sync `update`, worker +thread, metadata transactions, rebuild-table primitive, +modal infrastructure, project-switch lock dance, the +`[temp]` cleanup guards, persistence ordering). New ones +this session: + +- **`DataResult` now carries `column_types`.** Tests that + construct `DataResult` directly need the field + populated — `column_types: vec![Some(Type::…), …]` + with one entry per column. Two existing tests in + `walking_skeleton.rs` were updated for this; future + tests follow the same shape. + +- **The pretty-table renderer is `output_render.rs`, not + `ui.rs`.** `ui.rs` is the ratatui Frame renderer for + the whole TUI layout; `output_render` produces text + rows (one `OutputLine`-equivalent per element) for + the output panel. Different concerns; same project. + +- **No engine name in user-facing strings.** `CLAUDE.md` + has a working-style bullet codifying this (per + ADR-0002's new "User-facing posture" section). When + ADR-0017's implementation produces error messages, + do not say "SQLite", "STRICT", "PRAGMA", etc. Say + "the database" or describe the issue in the abstract. + Existing code mostly already follows this; the + occasional drift (e.g. `do_drop_table` may have a + legacy mention — check) should be cleaned up + opportunistically. + +- **B2/C2's PK refusal is about to relax.** When you + implement ADR-0017, the current "PK refused" tests + for change-column-type will need updating: the rule + becomes "refused only when the column has an inbound + FK and the new type changes `fk_target_type`". The + matching `*_refuses_pk` tests should split into a + pair: "refused when FK-referenced and target-type + changes" and "allowed when FK-referenced and target + type preserved (`serial → int` happy path)". + +- **Modern SQLite versions for column ops.** B2/C2 + uses `ALTER TABLE DROP COLUMN` (3.35+) and + `ALTER TABLE RENAME COLUMN` (3.25+). The `rusqlite` + bundled feature pins a recent SQLite, so these are + available. If we ever switch to system-linked SQLite + on a host without these, drop_column / rename_column + would refuse — worth a follow-up if it happens. + +- **`output_render.rs` is the only place new tabular + output should be produced from now on.** Per + ADR-0017 §7's directive (and per the user's + "everything tabular goes through it" rule), any + future feature that lists rows in error / success + output uses `render_data_table` rather than + hand-rolled indented lines. + +## Repository layout (delta vs. handoff-3) + +``` +src/ + output_render.rs — new (ADR-0016) + dsl/ + parser.rs — drop_column / rename_column / + change_column parsers, + optional_keyword helper + command.rs — DropColumn / RenameColumn / + ChangeColumnType variants + db.rs — do_drop_column, + do_rename_column, + do_change_column_type + + Database::* sibling methods + + DataResult.column_types field + app.rs — calls into output_render; + the inline render_data_view + helper + join_padded + + separator_row removed + runtime.rs — DropColumn / RenameColumn / + ChangeColumnType dispatch in + execute_command_typed; + silent-rebuild suppression + snapshots/ — 4 new insta snapshots + (output_render renderings) +docs/ + adr/ + 0002-database-engine.md — User-facing posture amendment + 0016-pretty-table-rendering.md — new (this session) + 0017-column-type-change-compatibility.md — new (design only) + README.md — indexed + handoff/ + 20260508-handoff-4.md — this file +CLAUDE.md — "no engine name" working- + style bullet +``` + +## How to take over + +1. Read this file. +2. Read `CLAUDE.md` for the working-style rules and current + layout. +3. Read `docs/requirements.md` for granular progress. +4. **Read ADR-0017 in full** — that's the spec for the + first task. Also skim ADR-0016 (pretty-table renderer) + and ADR-0002 (User-facing posture) since both inform + the implementation. +5. Run `cargo test` to confirm the 449-test green baseline. +6. `cargo run --release` to see the UI; try a `change + column` against the current B2/C2 implementation so + you have a concrete reference for what changes when + ADR-0017 lands. + +### End-to-end smoke test (current state) + +Demonstrates the new column ops + pretty rendering. Does +NOT exercise ADR-0017 (which is unimplemented): + +``` +$ rm -rf /tmp/handoff4-smoke +$ rdbms-playground --data-dir /tmp/handoff4-smoke + +# Inside the app: +help -- new column ops listed +create table Customers with pk id:serial +add column Customers: Name (text) -- bare-identifier form + -- (handoff-3 → handoff-4 polish) +add column to Customers: Email (text) -- with `to`, no `table` +add column table Customers: Score (int) -- with `table`, no `to` +insert into Customers ('Alice', 'a@b.com', 10) +insert into Customers ('Bob', 'b@b.com', 20) +show data Customers -- rendered via pretty table: + -- ┌────┬───────┬─────────┬───────┐ + -- │ id │ Name │ Email │ Score │ + -- ... + +# Column ops: +rename column Customers: Score to Points +drop column Customers: Email +show table Customers -- pretty structure listing + +# Type change (B2/C2 placeholder — works for safe cases, +# refuses for PK / relationships / serial / no-op): +change column Customers: Points (real) -- succeeds (per-cell + -- compatible: int → real + -- widens cleanly) +quit +``` + +After ADR-0017 lands, the same `change column` will +produce client-side notes for transformed rows, refuse +lossy without `--force-conversion`, render bordered +diagnostic tables for refusals, etc. + +### Manual spot-checks worth running + +- `--help` lists all column ops (drop / rename / change). +- Pretty rendering kicks in for `show data` AND every + schema-mutating command's auto-show. +- `[ok] rebuild — …` no longer fires on a fresh-launch + empty temp project (verify by launching with no args). +- `add column [to] [table] T: c (type)` — try all four + shapes; all parse. +- `change column T: id (text)` on a serial PK refuses + (current B2/C2 behavior; will become conditional under + ADR-0017 — refused only if FK-referenced AND + fk_target_type changes).