docs(adr-0005): Amendment 2 — drop blob from the type vocabulary

blob is a dead-end type: no literal in either mode, seed-unsupported, so
a blob column can only ever hold NULL. It rode along as the engine's
fourth storage class and never got the pedagogical scrutiny UUIDs
(excluded) and compound PKs (defended) received — and there's nothing
natural to do with binary data in a TUI teaching tool. Drop it → a
nine-type vocabulary.

Records the removal scope (Type::Blob + Value/CellValue::Blob + the
base64 CSV path + completion/grammar slot + the help/hint user-facing
strings + the binder refusal, whose message also carried a "DSL"
copy-rule bug subsumed by deletion) and the backward-compat migration:
the ADR-0015 framework's first real use — a v1→v2 migrator converts
type: blob → text and forces a .db rebuild from the migrated text.
convert-to-text chosen over drop-the-column (CSV rewrites) and
refuse-to-load. Updates the ADR index.
This commit is contained in:
claude@clouddev1
2026-06-22 20:00:53 +00:00
parent 64818c08f6
commit 1a2002dbf6
2 changed files with 143 additions and 1 deletions
+142
View File
@@ -132,3 +132,145 @@ verbatim, trailing zeros and all (`100.10`). Exact decimal
require rewriting the user's standard-SQL operators into function require rewriting the user's standard-SQL operators into function
calls, defeating both the "validated SQL runs verbatim" model and calls, defeating both the "validated SQL runs verbatim" model and
the goal of teaching ordinary SQL. the goal of teaching ordinary SQL.
## Amendment 2 — drop `blob` from the vocabulary (2026-06-22)
**This supersedes the `blob` row of the Decision table.** The type
set is now **nine types**: `text`, `int`, `real`, `decimal`, `bool`,
`date`, `datetime`, `serial`, `shortid`.
### Why `blob` is removed
The original Decision listed `blob` purely to mirror the engine's
storage classes (its only note was "Binary data."). Unlike the two
other vocabulary judgements made here — compound PKs *defended*
("skipping them would teach the wrong lesson") and true UUIDs
*excluded* ("in favour of TUI legibility") — `blob` never received the
same pedagogical scrutiny. It rode along as the fourth storage class
and was never re-examined.
In practice `blob` is a **dead-end type** in this application:
- **No value can be entered for it through the command surface.**
There is no blob literal in either mode — neither the simplified
syntax nor advanced SQL has one (ADR-0014: "DSL literal not
supported this iteration"; ADR-0038: "blob has no literal … neither
the DSL nor advanced SQL has a blob-literal syntax"). The binder
refuses outright (`src/dsl/value.rs`).
- **`seed` cannot generate it** (ADR-0048: `blob → unsupported`), and a
`NOT NULL blob` column is a hard refusal.
- So a blob column can only ever hold NULL via normal use. The only
way bytes get in at all is hand-editing the base64 cell in a CSV —
not something the app teaches or surfaces.
The missing literal was filed as a perpetual "pre-existing gap" across
many handoffs and never actioned. The right resolution is not to build
a blob-entry affordance — **there is nothing natural to *do* with
binary data in a terminal teaching tool, and nothing about the `blob`
type that serves this app's learning goals** (pedagogy wins ties; this
app is not the place to teach storage-class breadth). Removing it makes
the type set smaller (still "teachable in five minutes", now with one
less dead-end), removes a standing copy-rule bug (the binder's refusal
message named the internal "DSL" term — flagged 2026-06-22), and
simplifies the type-change matrix (ADR-0017's `↔ blob` static refusals
vanish) and the seed generator.
### What this changes
- `Type::Blob` and its keyword/aliases (`blob`, `binary`, `varbinary`)
are removed. Because the variant set is the single source of truth,
this clears both parse paths at once — `from_sql_name` (the
alias-aware command path) *and* `FromStr`/`parse::<Type>()` (the
keyword-only path the `.db` metadata read uses) — once the explicit
`blob` arms are deleted (compiler-guided). The type slot offered by
completion and the advanced-SQL grammar (`BLOB_SLOT`) loses `blob`;
`output_render`/`describe` lose the `BLOB` rendering. The data-layer
value plumbing it fed (`Value::Blob` / `CellValue::Blob(Vec<u8>)` and
the base64 CSV encode/decode in `persistence::csv_io`) is removed with
it.
- **User-facing strings** lose their blob entries: the `help types`
reference (catalog `help.types_reference`) drops `blob` from the
ten-name list, and the blob value-slot hint ("Type a quoted blob
literal or null", `hint.value_slot_blob`) — plus its `keys.rs`
declaration and the walker mapping — is removed. The binder's blob
refusal in `src/dsl/value.rs` (whose message named the internal "DSL"
term — the copy-rule bug that surfaced this whole question) is deleted
outright, so that fix needs no separate rewording.
- The `base64` dependency **stays**`clipboard.rs` uses it for OSC-52,
independent of `blob`.
- ADR-0011 (FK target-type compatibility) and ADR-0017 (type-change
compatibility) lose their `blob` rows; ADR-0014/0038/0048's
blob-literal/seed notes become moot. Prose that says "ten-type"
(ADR-0030/0033/0035, `CLAUDE.md`, the website type reference) becomes
"nine-type".
### Backward compatibility — a project migration
`blob` was a creatable column type, so a saved project may name it. The
type appears in **two artifacts**, and the migration must reconcile
both:
1. **`project.yaml`** (authoritative) — a column declared `type: blob`.
On load this is parsed by `from_sql_name`, which returns `None` once
the type is gone → the project would fail to open.
2. **The derived `playground.db`** (ADR-0004/0015) — which carries the
type *twice over*: the user-facing string `"blob"` in the
`__rdbms_playground_columns` metadata table, **and** an actual
`STRICT … BLOB` column in the SQLite schema. Crucially, load uses an
existing `.db` **as-is**`rebuild_from_text` runs only when the
`.db` is *missing* (`runtime.rs`, ADR-0015 §7). Worse, the metadata
read parses the stored type with `parse::<Type>().ok()` and falls
back to `Type::Text` on failure — so a stale blob `.db` would *not*
error; it would silently read the column as `text` while the engine
column stays `BLOB`: an inconsistent, broken state.
**So a YAML-only type-swap is insufficient.** The migration is:
1. **Migrate the YAML** via the ADR-0015 framework
(`src/persistence/migrations.rs`): register the first real migrator
(v1 → v2) that rewrites every `type: blob` to `type: text`. This is
the framework's first production use — note its by-design
consequence: bumping `latest_version` to 2 re-versions **every**
project on next load (a `project.yaml.v1.bak` is written, the body
rewritten) — harmless for blob-free projects (a pure version bump),
but it does touch every file once.
2. **Force a rebuild of the `.db` from the migrated YAML** whenever the
migration actually changed a column type (i.e. a `blob` was present)
— reusing the existing `rebuild_from_text` path so the engine schema
and metadata are regenerated as `text`, not left stale. A pure
version bump with no schema change does **not** force a rebuild.
**Conversion target: `text`** (decided 2026-06-22). Non-destructive —
the CSV already stores blob cells as base64 *text*, so any bytes survive
as a recoverable string, and an empty column (the overwhelmingly common
case) simply becomes an empty `text` column. It is constraint-safe:
`text` is a valid primary-key and FK-target type (`fk_target_type` is
identity for both), so a former-blob PK or FK target stays valid. And it
keeps the migration **CSV-free**: the column keeps its name and position,
so every CSV header still matches the rebuilt schema (the rebuild's
strict header==schema check, `do_rebuild_from_text`) and the existing
cells decode as `text`.
The considered alternatives were rejected: **drop the column** is
cleaner semantically (there is no legitimately-entered content to keep)
but the strict header check means it would force the migration to *also*
rewrite every affected `data/<table>.csv` to strip the column — not
worth the machinery for a type that has existed unusably for a few days
with very few users; and **refuse to load** strands the project. The
rebuild-on-migration step (above) is required regardless of target,
because the stale `.db` keeps a `STRICT … BLOB` *engine* column that can
neither hold text nor be reported as anything usable until regenerated.
### Consequences
- A smaller, fully-usable type set: every remaining type can be both
created *and* populated through the command surface.
- The type-change matrix and `seed` shed their blob special-cases; the
binder sheds its blob refusal (and its copy-rule violation).
- The ADR-0015 migration framework gets its first real exercise,
validating that path end-to-end.
- A breaking change for the rare project carrying a `blob` column,
mitigated to a graceful, non-destructive load by the migration.
- Re-introducing `blob` later (should a concrete teaching use appear)
is a new ADR superseding this amendment — the same bar every other
type change clears.
+1 -1
View File
@@ -17,7 +17,7 @@ This directory contains the project's ADRs, recorded per
- [ADR-0002 — Database engine](0002-database-engine.md) - [ADR-0002 — Database engine](0002-database-engine.md)
- [ADR-0003 — Input modes and command dispatch](0003-input-modes-and-command-dispatch.md) — the persistent `Simple`/`Advanced` mode and the `:` one-shot escape. The **startup mode is no longer always `simple`**: it is restored from the project's stored mode and overridable with `--mode` (see **ADR-0015 Amendment 1**, issue #14). The app-command registry gains **`copy`** (ADR-0041, issue #11) - [ADR-0003 — Input modes and command dispatch](0003-input-modes-and-command-dispatch.md) — the persistent `Simple`/`Advanced` mode and the `:` one-shot escape. The **startup mode is no longer always `simple`**: it is restored from the project's stored mode and overridable with `--mode` (see **ADR-0015 Amendment 1**, issue #14). The app-command registry gains **`copy`** (ADR-0041, issue #11)
- [ADR-0004 — Project file format](0004-project-file-format.md) - [ADR-0004 — Project file format](0004-project-file-format.md)
- [ADR-0005 — Column type vocabulary](0005-column-type-vocabulary.md) — the ten-type set (`text`/`int`/`real`/`decimal`/`bool`/`date`/`datetime`/`blob`/`serial`/`shortid`), compound PKs, no true UUIDs; `decimal` stored as exact TEXT. **Amendment 1, 2026-06-12** (issue #32): SQLite has no native decimal/BCD type, so arithmetic/aggregation over a TEXT `decimal` is implicitly coerced to an IEEE-754 double and the computed (typeless) result leaked float noise (`298.59999999999997` for `298.60`); floating-point values are now rounded to **15 significant figures for display only** (`format_real_display` in `db.rs`, wired into `format_cell` — the result-set/`show data` cell formatter, the only surface where arithmetic noise surfaces) while every other f64→string path keeps full precision because the distinction is *semantic*: persistence (`csv_io::format_real`) stays byte-exact for round-trip; `render_value` is a *canonical identity key* for the uniqueness dry-runs (`dry_run_unique` ADR-0029 §5, `check_uniqueness_collisions` ADR-0017 §4.3) so rounding it would report collisions the exact-valued engine wouldn't; FK-key matching and EXPLAIN-SQL literals likewise stay exact — so stored `real`/`decimal` round-trips stay byte-exact and raw `decimal` columns render verbatim - [ADR-0005 — Column type vocabulary](0005-column-type-vocabulary.md) — the ten-type set (`text`/`int`/`real`/`decimal`/`bool`/`date`/`datetime`/`blob`/`serial`/`shortid`), compound PKs, no true UUIDs; `decimal` stored as exact TEXT. **Amendment 1, 2026-06-12** (issue #32): SQLite has no native decimal/BCD type, so arithmetic/aggregation over a TEXT `decimal` is implicitly coerced to an IEEE-754 double and the computed (typeless) result leaked float noise (`298.59999999999997` for `298.60`); floating-point values are now rounded to **15 significant figures for display only** (`format_real_display` in `db.rs`, wired into `format_cell` — the result-set/`show data` cell formatter, the only surface where arithmetic noise surfaces) while every other f64→string path keeps full precision because the distinction is *semantic*: persistence (`csv_io::format_real`) stays byte-exact for round-trip; `render_value` is a *canonical identity key* for the uniqueness dry-runs (`dry_run_unique` ADR-0029 §5, `check_uniqueness_collisions` ADR-0017 §4.3) so rounding it would report collisions the exact-valued engine wouldn't; FK-key matching and EXPLAIN-SQL literals likewise stay exact — so stored `real`/`decimal` round-trips stay byte-exact and raw `decimal` columns render verbatim; **Amendment 2, 2026-06-22** (supersedes the `blob` row): drops `blob` from the vocabulary → a **nine-type** set. `blob` was a dead-end — no literal in either mode (ADR-0014/0038), `seed`-unsupported (ADR-0048), so a blob column could only ever hold NULL; it rode along as the engine's fourth storage class and never got the pedagogical scrutiny UUIDs (excluded) and compound PKs (defended) did, and there is nothing natural to *do* with binary data in a TUI teaching tool. Removes `Type::Blob` + `Value::Blob`/`CellValue::Blob` + the base64 CSV path + the completion/grammar `BLOB_SLOT` + the `help types`/`value_slot_blob` user-facing strings + the binder's blob refusal (whose message carried a "DSL" copy-rule bug — subsumed by deletion); simplifies ADR-0017's type-change matrix (`↔ blob` static refusals vanish) and the seed generator; `base64` stays (clipboard OSC-52). **Backward compat = the migration framework's (ADR-0015) first real use**: a v1→v2 migrator rewrites `type: blob``type: text` in `project.yaml` (re-versioning every project once, `.bak` written) and forces a `.db` rebuild from the migrated text when a column actually changed (the stale `.db` keeps a `STRICT … BLOB` engine column + `"blob"` metadata that load uses as-is); **convert-to-text** chosen over drop-the-column (drop would force CSV rewrites past the strict header check — not worth it) and refuse-to-load; non-destructive + PK/FK-safe + CSV-free. Knocks `ten-type``nine-type` across ADR-0030/0033/0035, `CLAUDE.md`, website type reference
- [ADR-0006 — Undo snapshots and replay log](0006-undo-snapshots-and-replay-log.md) — **Accepted**. The **replay/journal half** (U3/U4) shipped via ADR-0034; the **undo/snapshot half** (U1/U2) is settled by **Amendment 1 (2026-05-24)** and **implemented 2026-05-24** (plan: `docs/plans/20260524-adr-0006-undo-snapshots.md`; ring in `src/undo.rs`, worker hook in `src/db.rs`). Amendment 1 **supersedes the original "snapshots only before destructive operations" model**: a snapshot is taken before **every** data/schema mutation (DSL + SQL) for familiar single-step (Ctrl-Z) undo — so the confirmation collapses to *naming the one command being undone* (no db-diff). Snapshot is a **hybrid whole-project copy** — database via the online backup API **plus** `project.yaml`/`data/*.csv` as files — reconciling this ADR with ADR-0015's "text is authoritative, db is derived"; undo restores all three directly. Staged before the mutation's transaction, finalised after the db commit (preserves ADR-0015 §6 commit-db-last); rolled-back ops leave no snapshot. **Persisted** ring under `.snapshots/`, **N = 50** (raised from 10), git-ignored + export-excluded + temp-cleanup-aware. `redo` supported, **redo stack discarded on new work**. **Batch ops record one undo step** (`replay` + future batch via a Begin/EndBatch worker primitive); **`import` is outside undo** (it switches projects per ADR-0015 §11, leaving the current project untouched). A **`--no-undo` CLI flag** disables snapshotting (hardware escape hatch). Adds the `backup` feature to `rusqlite` - [ADR-0006 — Undo snapshots and replay log](0006-undo-snapshots-and-replay-log.md) — **Accepted**. The **replay/journal half** (U3/U4) shipped via ADR-0034; the **undo/snapshot half** (U1/U2) is settled by **Amendment 1 (2026-05-24)** and **implemented 2026-05-24** (plan: `docs/plans/20260524-adr-0006-undo-snapshots.md`; ring in `src/undo.rs`, worker hook in `src/db.rs`). Amendment 1 **supersedes the original "snapshots only before destructive operations" model**: a snapshot is taken before **every** data/schema mutation (DSL + SQL) for familiar single-step (Ctrl-Z) undo — so the confirmation collapses to *naming the one command being undone* (no db-diff). Snapshot is a **hybrid whole-project copy** — database via the online backup API **plus** `project.yaml`/`data/*.csv` as files — reconciling this ADR with ADR-0015's "text is authoritative, db is derived"; undo restores all three directly. Staged before the mutation's transaction, finalised after the db commit (preserves ADR-0015 §6 commit-db-last); rolled-back ops leave no snapshot. **Persisted** ring under `.snapshots/`, **N = 50** (raised from 10), git-ignored + export-excluded + temp-cleanup-aware. `redo` supported, **redo stack discarded on new work**. **Batch ops record one undo step** (`replay` + future batch via a Begin/EndBatch worker primitive); **`import` is outside undo** (it switches projects per ADR-0015 §11, leaving the current project untouched). A **`--no-undo` CLI flag** disables snapshotting (hardware escape hatch). Adds the `backup` feature to `rusqlite`
- [ADR-0007 — Sharing and export](0007-sharing-and-export.md) - [ADR-0007 — Sharing and export](0007-sharing-and-export.md)
- [ADR-0008 — Testing approach](0008-testing-approach.md) — the four-tier strategy (pure-logic units → `TestBackend` render/`insta` snapshots → synthetic event-loop integration → PTY black-box). **Amendment 1 (2026-06-22): Tier 4 realized (TT4).** `tests/e2e_pty.rs` drives the actual built binary in a real pseudo-terminal — `portable-pty` + `vt100`, **`expectrl` dropped** (conflicting PTY layer; replaced by a hand-rolled `wait_for` on the vt100 grid). Fixed 100×30, per-test temp `--data-dir`, serial, tight fail-fast 3 s waits; table presence read from the Tables **sidebar** region (the Output panel echoes commands); commands **paced to completion** (a command sent mid-rebuild can misparse against a stale schema cache, issue #39). The four flows: cold-launch→DDL→quit; create→reopen (`--resume`, column round-trips); export→import→rebuild (schema **+** data); undo-after-DROP through the modal. Runs by default in `cargo test` → the Linux gate exercises it (**advances TT5**); PTY-in-container validated; Windows execution + nightly broader coverage stay out of scope. Also piggybacks NFR-1/NFR-3 measurement (ADR-0057) - [ADR-0008 — Testing approach](0008-testing-approach.md) — the four-tier strategy (pure-logic units → `TestBackend` render/`insta` snapshots → synthetic event-loop integration → PTY black-box). **Amendment 1 (2026-06-22): Tier 4 realized (TT4).** `tests/e2e_pty.rs` drives the actual built binary in a real pseudo-terminal — `portable-pty` + `vt100`, **`expectrl` dropped** (conflicting PTY layer; replaced by a hand-rolled `wait_for` on the vt100 grid). Fixed 100×30, per-test temp `--data-dir`, serial, tight fail-fast 3 s waits; table presence read from the Tables **sidebar** region (the Output panel echoes commands); commands **paced to completion** (a command sent mid-rebuild can misparse against a stale schema cache, issue #39). The four flows: cold-launch→DDL→quit; create→reopen (`--resume`, column round-trips); export→import→rebuild (schema **+** data); undo-after-DROP through the modal. Runs by default in `cargo test` → the Linux gate exercises it (**advances TT5**); PTY-in-container validated; Windows execution + nightly broader coverage stay out of scope. Also piggybacks NFR-1/NFR-3 measurement (ADR-0057)