do_rebuild_from_text re-emitted table-level CHECKs into the recreated
DDL (so they stayed enforced) but never repopulated __rdbms_playground_
table_checks. A fresh rebuild (missing .db, reconstructed from
project.yaml) therefore left the CHECK metadata empty: DROP CONSTRAINT,
describe, and a later save would lose it — including a named CHECK's
name. In-place rebuilds only worked because the wipe never touched the
table. (Latent since 4a.3 for unnamed checks; exposed by 4g's named
round-trip claim.)
Rebuild now wipes and repopulates CHECK_TABLE from the yaml snapshot
(name + seq + expr), like META/REL, and adds the 4g `name` column if a
pre-4g table predates it (the rebuild-only migration). Regression test:
a named CHECK's metadata survives a fresh rebuild (DROP CONSTRAINT by
name resolves).
ALTER TABLE <T> ADD [CONSTRAINT <name>] (CHECK | UNIQUE | FOREIGN KEY)
and DROP CONSTRAINT <name>. ADD = table-CHECK + composite UNIQUE + FK
(ADD PRIMARY KEY and a named UNIQUE 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 -> add_relationship, bare
REFERENCES -> parent single PK). DROP CONSTRAINT resolves the name to a
named table-CHECK then a child-side FK, else refuses. One undo step each.
Named table-CHECKs round-trip: a nullable `name` column on
__rdbms_playground_table_checks (rebuild-only arrival; a named add on a
pre-4g project is refused with a "rebuild first" hint) plus a project.yaml
check_constraints {expr, name} extension (bare-string form still reads).
The internal-__rdbms_* guard was folded into do_add_constraint /
do_add_relationship, completing that guard class.
Grammar: the action Choice keeps one branch per verb (add/drop/rename/
alter) with an inner Choice fanning out on the distinct second keyword,
since the walker's Choice does not backtrack between same-led branches.
Tests: 7 Tier-1 parse + 2 yaml round-trip + 1 internal-guard + 9 Tier-3
e2e. Help/usage refreshed; ADR-0035 §13 4g + README + requirements.md in
lockstep.
Fourth AlterTableAction (AlterColumnType), runtime-decomposed to the
existing change_column_type executor with ForceConversion — which IS the
§7 advanced policy: lossy converts with a note (no force flag),
incompatible + the ADR-0017 static refusals (↔blob, same-type,
date↔datetime, non-int→serial) still refuse, while int→serial is allowed
(auto-fills nulls + UNIQUE, ADR-0018 §8). No new mode/note/persistence;
undo is the advanced safety net.
Grammar adds a fourth action branch leading on `alter`, discriminated in
the builder by the `type` keyword (unique — ADD COLUMN's type is an
ident); the type slot reuses SQL_TYPE. The internal-__rdbms_* guard was
folded into do_change_column_type (user-confirmed), closing the simple
`change column` exposure.
Tests: 7 Tier-3 e2e via run_replay + 4 Tier-1 parse (incl. a column-named-
`type` discriminator probe) + the simple-surface guard. Help/usage
refreshed; ADR-0035 §13 4f + README + requirements.md in lockstep.
- docs/plans/20260525-adr-0035-sql-ddl-4f.md — ALTER TABLE ALTER COLUMN
TYPE plan (/runda'd; forks resolved). Advanced lossy = ForceConversion
(reuse do_change_column_type; existing transformed_lossy note); the
internal-__rdbms_* guard folds into do_change_column_type both surfaces
(user-confirmed); int->serial is ALLOWED (ADR-0018 §8), only non-int
->serial / blob / date<->datetime are static-refused.
- docs/handoff/20260525-handoff-39.md — 4d/4e shipped; 4f is next
(plan ready).
Advanced-only `alter` entry word; ALTER TABLE <T> ADD COLUMN <col> <type>
[constraints] | DROP COLUMN <col> | RENAME COLUMN <old> TO <new> ->
SqlAlterTable, runtime-decomposed to the existing column executors
(do_add_column / do_drop_column / do_rename_column) — one undo step each,
no new worker layer. The COLUMN keyword is required (reserves bare
RENAME TO for 4h, ADD CONSTRAINT for 4g).
- ADD COLUMN takes NOT NULL / UNIQUE / DEFAULT / CHECK (no PK / inline
REFERENCES). do_add_column extended to consume the SQL raw-text
default_sql / check_sql (sql_expr is validate-only, the 4a.2
mechanism), reaching parity with CREATE TABLE's column constraints.
- Drop/rename column refuse a column any CHECK references — table-level
AND column-level (incl. a column's own self-check on rename) — the
4a.3 deferral, detected up-front by tokenizing the raw CHECK text
(skipping string literals). In the shared executors, so it guards both
the simple and SQL surfaces and fixes a latent rename-drift bug that
desynced the stored CHECK text and broke rebuild.
- SQL DROP COLUMN refuses an index-covered column (no --cascade SQL
spelling — matches SQLite + the simple default).
- The column executors and do_add_index gained an internal-__rdbms_*
guard (refuse as "no such table"), closing a pre-existing exposure on
both surfaces. (do_change_column_type / do_add_constraint /
do_add_relationship are a tracked follow-up.)
- `alter` is advanced-only; AlterTableAction::AddColumn is boxed
(clippy::large_enum_variant).
Docs: ADR-0035 status + §13 4e; ADR README; requirements.md Q1. Plan:
docs/plans/20260525-adr-0035-sql-ddl-4e.md.
Tests: 1854 passing / 0 failing / 0 skipped / 1 ignored; clippy clean.
Advanced-mode SQL CREATE [UNIQUE] INDEX [IF NOT EXISTS] [<name>] ON
<T> (cols) -> SqlCreateIndex and DROP INDEX [IF EXISTS] <name> ->
SqlDropIndex, both reusing the ADR-0025 executors (do_add_index /
do_drop_index), like 4c reused do_drop_table.
- CREATE UNIQUE INDEX admitted in advanced mode (ADR-0025 Amendment 1):
ADR-0025 deferred UNIQUE indexes for the simple-mode DSL, but advanced
mode trusts the user like SQL does. Adds an additive IndexSchema.unique
flag (project.yaml, serde-default, version stays 1); rebuild re-emits
CREATE UNIQUE INDEX; the redundant-set guard keys on (columns, unique).
Simple-mode `add unique index` stays deferred.
- IF [NOT] EXISTS on both forms reuses the 4c no-op-with-note skip
(journalled, not snapshotted) via CreateIndexOutcome / DropIndexOutcome.
- Unnamed CREATE INDEX auto-named (ADR-0025 convention); the [UNIQUE]
prefix is a concrete-keyword Choice and the optional name an on-led-first
selector (the drop-index selector precedent) — trap-safe.
- create/drop each gain a second advanced node; the existing all-candidates
dispatch handles it (locked by parse tests).
- Unique indexes marked [unique] in the structure view and items panel.
- do_add_index refuses internal __rdbms_* tables as "no such table",
closing a latent exposure on both the simple `add index` and the new
SQL CREATE INDEX surfaces (ADR-0025 Amendment 1).
Docs: ADR-0035 status + §13 4d + 4i; ADR-0025 Amendment 1; ADR README;
requirements.md Q1/C3. Plan: docs/plans/20260525-adr-0035-sql-ddl-4d.md.
Tests: 1834 passing / 0 failing / 0 skipped / 1 ignored; clippy clean.
Three Phase-4 sub-phases shipped this session (all local-only on main,
1805 tests passing, clippy clean):
- 4a.3 table-level/multi-column CHECK (new __rdbms_playground_table_checks)
- 4b foreign keys in CREATE TABLE (ADR-0013 named relationships)
- 4c DROP TABLE [IF EXISTS] (DropOutcome::Skipped no-op-with-note)
Handoff records the growing 4i deferral list (a–e, canonical in
ADR-0035 §13 4i) and flags the 4d escalation (IndexSchema.unique model
extension for CREATE UNIQUE INDEX).
Add advanced-mode SQL `DROP TABLE [IF EXISTS] <name>` -> SqlDropTable,
executing through the existing do_drop_table (cascade / inbound-
relationship refusal / metadata cleanup) — full parity with the simple
`drop table`. The only new behaviour is `IF EXISTS` as a
no-op-with-note: a new DropOutcome::Skipped mirroring
CreateOutcome::Skipped (journalled, no snapshot), rendered via a new
ddl.drop_skipped_absent note + DslDropSkipped event.
- Grammar: SQL_DROP_TABLE node (entry `drop`, shape `table [if exists]
<name> [;]`), registered Advanced. SQL-first dispatch: `drop table T`
-> SqlDropTable in advanced; `drop column`/`relationship`/`index`/
`constraint` fall back to the simple `drop` node (and still execute).
- Worker: Request::SqlDropTable + db.sql_drop_table; the if-exists-and-
absent arm journals + replies Skipped without a snapshot, else
snapshot_then(do_drop_table) -> Dropped.
- Completion: advanced `drop ` now surfaces the SQL `table` (the
shared-entry-word behaviour from `create`); test split into simple
(full DSL list) + advanced (SQL surface).
Known shared-entry-word completion unevenness (advanced `drop ` offers
only `table`; partial `drop rel` returns an empty list) deferred to 4i
(merge candidate sets for shared entry words) along with a flagged user
request to visually distinguish simple- vs advanced-mode completions in
the hint UI — tracked in ADR §13 4i (d)/(e), the 4c plan, and the
completion test. The DSL drops still parse + execute via fallback.
10 new tests (parse/builder + Tier-3: drop existing + one-undo-step +
restore, IF EXISTS skip + journal, plain-absent error, inbound refusal).
Docs: ADR-0035 Status/§13, README, requirements.md Q1.
Tests: 1805 passing, 0 failing, 1 ignored. Clippy clean.
Add foreign keys to advanced-mode SQL CREATE TABLE — the SQL spelling of
an ADR-0013 named relationship, created in the same transaction as the
table (one undo step).
- Grammar: inline `<col> … REFERENCES <parent>[(<col>)] [ON DELETE/UPDATE
…]` (a new column constraint) and table-level `[CONSTRAINT <name>]
FOREIGN KEY (<col>) REFERENCES …` (two new element branches — both
start on a concrete keyword, never a leading Optional, which would
abort the element Choice). Referential clauses reuse
shared::REFERENTIAL_CLAUSES.
- Builder: greedy FK-clause consumption (parens consumed internally so
they don't perturb the 4a.3 element-boundary depth tracker); inline FK
auto-named, table FK takes an optional CONSTRAINT name.
- Worker: do_create_table resolves + validates each FK before building
the DDL (self-ref validates against the in-statement columns/PK; bare
REFERENCES resolves to the parent's single-column PK, composite ->
error; PK-target + Type::fk_target_type compatibility), emits the
FOREIGN KEY clause identically to schema_to_ddl, and writes the
relationship metadata in the create transaction.
- Reuse: name/uniqueness/metadata-insert/type-compat factored into shared
helpers; do_add_relationship refactored to use them.
- FKs round-trip via the existing relationship plumbing (no new
persistence structures); describe surfaces the relationship.
Self-references and bare `REFERENCES <parent>` supported (user-confirmed).
Self-ref pre-submit indicator wrinkle deferred to 4i (tracked in ADR §13,
a code comment, and the plan).
DA/runda round added cross-cutting probes (FK survives the add-column
rebuild + a later rebuild_from_text; referential actions survive rebuild;
drop-child clears the relationship; drop-parent refused; bare self-ref
resolves to own PK) — all green, no fixes needed.
27 new tests (grammar/builder + Tier-3). Docs: ADR-0035 Status/§13,
README, requirements.md Q1.
Tests: 1795 passing, 0 failing, 1 ignored. Clippy clean.
Add table-level CHECK (e.g. `CREATE TABLE t (a int, b int, CHECK (a < b))`)
to advanced-mode SQL CREATE TABLE. Since SQLite exposes no PRAGMA for CHECK
constraints, a table-level CHECK cannot be read back from the engine and
becomes the source of truth in a new internal metadata table
`__rdbms_playground_table_checks (table_name, seq, check_expr)`.
- Grammar: new TABLE_CHECK element in ELEMENT_CHOICES.
- Builder: distinguishes a table-level CHECK from a column-level one by
element position (no column-def open in the element), using depth-aware
boundary tracking so a length-arg comma (`numeric(10,2)`) or a
table-PRIMARY KEY's inner comma is not mistaken for an element separator.
- Worker: do_create_table emits the CHECK clauses and writes the metadata
rows in its transaction; schema_to_ddl emits them identically on rebuild;
read_schema / read_schema_snapshot read them from the metadata table;
do_drop_table clears them.
- Persistence: TableSchema.check_constraints round-trips through project.yaml
(#[serde(default)], optional on read), mirroring unique_constraints.
- Composite UNIQUE deliberately stays PRAGMA-detected (engine-reportable,
unlike CHECK) — user-confirmed.
DA/runda round added cross-cutting tests and a forward-looking doc fix:
- table CHECK survives a rebuild triggered by `add column`, and a later
rebuild_from_text (the ADR-0013 rebuild primitive uses a raw DROP, so the
metadata rows keyed on the final name are preserved);
- dropping a column a table CHECK references fails cleanly (rollback, table
intact); detection is 4e, friendly wording is H1;
- dropping a table clears its CHECK metadata (no orphan rows on re-create);
- amended ADR §6 so 4h's RENAME also updates the new metadata table.
20 Tier-3 + 9 grammar/builder + 2 YAML tests. Docs: ADR-0035 Status/§13/§6,
README index, requirements.md Q1. Help/usage skeleton + describe display of
table-level constraints deferred to 4i (symmetric with 4a.2).
Tests: 1769 passing, 0 failing, 1 ignored. Clippy clean.
Advanced-mode SQL CREATE TABLE implemented through sub-phase 4a.2
(columns/types/aliases, NOT NULL/UNIQUE/PRIMARY KEY, IF NOT EXISTS,
per-column CHECK/DEFAULT, composite UNIQUE), ADR-0035 flipped to
Accepted, /runda pass on 4a fixed two defects. Handoff details the next
step (4a.3 — table-level CHECK + a new __rdbms_* metadata table), the
remaining Phase-4 sub-phases (4b–4i), the cross-cutting patterns (two
DDL generators must stay in sync; round-trip via PRAGMA-or-metadata;
the litmus test; raw-text capture), and process pins. Baseline
1752/0/0/1, clippy clean.
Advanced-mode SQL CREATE TABLE gains the constraints that need no new
internal table (the 4a.2 slice):
- Grammar (sql_create_table.rs): column-level DEFAULT/CHECK and
table-level UNIQUE(cols). DEFAULT is a literal or a *parenthesised*
expression (standard SQL) — a bare sql_expr greedily eats a following
NOT (NOT IN/LIKE/BETWEEN), breaking `DEFAULT 0 NOT NULL`; the parens
bound it. CHECK is paren-bounded already.
- Builder (ddl.rs): captures CHECK/DEFAULT raw SQL text by byte span
(sql_expr builds no AST) via capture_parenthesised_span /
capture_expr_span; routes single-column table UNIQUE into the
column's flag and composite UNIQUE into unique_constraints.
- Command/worker: ColumnSpec gains check_sql/default_sql (raw, preferred
over the typed Expr/Value); Command::SqlCreateTable + Request +
do_create_table gain unique_constraints; do_create_table emits raw
CHECK/DEFAULT and composite UNIQUE clauses.
- Round-trip (part D): ReadSchema/TableSchema gain unique_constraints;
read_schema detects composite UNIQUE via PRAGMA index_list origin 'u'
(single-column still folds to the column flag); schema_to_ddl emits
them; YAML RawTable/write_table round-trips (optional-on-read).
CHECK round-trips via __rdbms_playground_columns.check_expr, DEFAULT
via PRAGMA table_info — no new metadata table.
Table-level/multi-column CHECK remains 4a.3 (rejected "not yet
supported"); FK is 4b.
Tests: +7 builder (raw-text capture incl. the DEFAULT 0 NOT NULL
boundary the fix was found by; single/composite UNIQUE routing) and +4
Tier-3 (CHECK enforced, DEFAULT applied, composite UNIQUE enforced, and
all three survive a rebuild — the part-D round-trip). 1752 pass / 0 fail
/ 1 ignored; clippy clean. Plan + requirements.md updated.
Survey of the constraint persistence machinery revealed that
table-level/multi-column CHECK needs a NEW __rdbms_* metadata table
(SQLite exposes no PRAGMA for CHECK), unlike per-column CHECK/DEFAULT
(reuse __rdbms_playground_columns.check_expr + PRAGMA dflt_value) and
composite UNIQUE (PRAGMA index_list origin 'u' + a TableSchema field).
User-confirmed split: 4a.2 = per-column CHECK/DEFAULT (raw sql_expr
text) + composite UNIQUE(a,b), no new internal table; 4a.3 = table-level
CHECK + the new metadata table. ADR §13 and README updated in lockstep;
4a.2 plan doc added.
Command + builder + worker for advanced-mode SQL CREATE TABLE
(sub-phase 4a), executed structurally through do_create_table:
- Command::SqlCreateTable + build_sql_create_table (ddl.rs): aliases via
from_sql_name (incl. double precision), column- and table-level
PRIMARY KEY, redundant-flag de-dup off a sole PK, IF NOT EXISTS.
Advanced REGISTRY entry on the shared `create` word (SQL-first, DSL
fallback); no-PK tables allowed (user-confirmed).
- Worker (db.rs): Request::SqlCreateTable + CreateOutcome + snapshot_then
(one undo step); IF NOT EXISTS no-op (no snapshot, but journalled, like
read-only commands). do_create_table inline-PK rule aligned with the
rebuild generator schema_to_ddl — no round-trip DDL drift; serial
autoincrement is independent of inline-PK (verified by round-trip
tests).
- Runtime/App: dispatch + CommandOutcome::SchemaSkipped +
AppEvent::DslCreateSkipped (structure + "already exists — skipped"
note). Friendly catalog keys added (engine-neutral).
DEFAULT/CHECK/table-level UNIQUE are absent from the 4a grammar (parse
error with usage skeleton; friendly message + support land in the 4a.2
constraint slice) — user-confirmed.
Tests: type resolver, grammar shape, builder (incl. the PK
detection bug they caught), and tests/sql_create_table.rs (worker
round-trip, serial autoincrement first/non-first across rebuild, IF NOT
EXISTS no-op + journalling, no-PK table, one undo step) + a replay-as-
write test. 1739 pass / 0 fail / 1 ignored; clippy clean.
Exit gate: ADR-0035 Proposed -> Accepted (validated end-to-end by 4a);
README + requirements.md Q1 updated.
Advanced-mode SQL type slot accepts the ten playground keywords plus the
standard-SQL aliases (integer/varchar/timestamp/numeric/float/double
precision/binary/..., case-insensitive). Simple-mode FromStr is unchanged
(rejects aliases). Unknown names -> None for the friendly diagnostic.
Three design questions settled during 4a implementation (plan + ADR §13
+ README in lockstep):
- CHECK/DEFAULT defer to the 4a.2 constraint slice: sql_expr is
validate-only (no Expr AST), so they need raw-SQL-text storage on a
separate path, not do_create_table's Expr->compile reuse. 4a.2 now
also covers composite UNIQUE / multi-column table CHECK.
- double precision (the lone two-word alias) handled via a keyword-pair
branch; single-word aliases + discarded (len) cover the rest.
- serial sole-PK in a multi-column table must inline PRIMARY KEY to keep
autoincrement (worker-step do_create_table extension).
4a core narrows to columns + types + NOT NULL/UNIQUE/PRIMARY KEY +
IF NOT EXISTS; everything else errors "not yet supported".
Add the sub-phase 4a implementation plan (docs/plans/), test-first,
mirroring the ADR-0033 DML sub-phase model: SqlCreateTable as its own
command executed structurally through the existing do_create_table
helper; shared-entry-word dispatch (SQL-first, simple fallback); the
type-alias resolver; IF NOT EXISTS no-op-with-note (CreateOutcome
enum); INTEGER PRIMARY KEY -> plain int; one-undo-step wiring.
Records the user-confirmed 4a/4a.2 split: composite UNIQUE(a,b) and
multi-column table CHECK move to a dedicated slice because they are the
first structures TableSchema cannot already represent, so they need a
persistence-model + round-trip extension rather than parse+execute
reuse. ADR-0035 §13 gains 4a.2; README sub-phase line updated in
lockstep.
Pre-implementation /runda round settled two open micro-calls before 4a,
both user-confirmed:
- IF [NOT] EXISTS admitted (no-op-that-succeeds-with-a-note), not
refused — a near-universal cross-vendor idiom (PostgreSQL, MySQL,
SQLite, Oracle 23ai), reclassified into scope rather than treated as
an engine-specific spelling. Touches §3/§4/§12/§13 (4a, 4c).
- INTEGER PRIMARY KEY maps to a plain int PK, not auto-increment;
serial stays the sole auto-increment type (§3).
README index updated in the same edit per the lockstep rule.
Phase 4 of the ADR-0030 roadmap; clarifies §4. Advanced-mode
CREATE/DROP/ALTER TABLE + CREATE/DROP INDEX get their own
per-statement Sql* commands, executed structurally (not verbatim)
so the playground's types, named relationships, and STRICT stay
intact. Full surface (no pre-emptive cuts): constraints, compound
PK, FK -> named relationships (one statement = one undo step),
ALTER incl. advanced-only table rename (C1), [UNIQUE] indexes.
Unified column-type-conversion: lossy refuses in simple mode but
proceeds-with-a-note in advanced, with undo as the safety net.
Integration (parser/hint/completion/diagnostics/history/replay/undo)
is structural via the unified grammar; replay treats DDL as a write.
Nine sub-phases (4a-4i). Updates the ADR README index.
Status: Proposed (design agreed; implementation pending).
/runda found silent data loss: with the non-fatal snapshot-failure
policy, a committed mutation whose snapshot couldn't be staged left
the redo stack stale (redo-clear was only a side effect of finalize),
so a later redo silently discarded the new work. Same gap in batches.
- SnapshotStore::clear_redo() drops the redo stack + payloads
- snapshot_then / end_batch call it when committed user work has no
staged snapshot; for disk-full it succeeds where a full backup
couldn't (tiny index write + payload deletes)
- unit test + integration regression (forced staging failure)
- ADR-0006 implementation note records the fix + residual edge
1698 passed / 0 failed / 1 ignored; clippy clean.
Three Tier-3 flows through the real worker:
- undo/redo steps back across interleaved DSL insert, SQL insert,
and SQL delete — proving SQL DML snapshots like DSL (R22)
- undo restores the database read model AND the on-disk CSV
together (consistent (db, csv) pair)
- the snapshot ring persists across a close + reopen of the project
(undo works in a later session)
1696 passed / 0 failed / 1 ignored; clippy clean.
The undo ring is local working state, handled at all three
project-file seams (R13):
- .gitignore template ignores /.snapshots/
- export excludes .snapshots/ (like playground.db / history.log)
- safely_delete_temp_project allowlists .snapshots/ so a temp that
was modified then undone back to empty stays auto-deletable
- undo::SNAPSHOTS_DIR is now a pub const referenced by all three
- tests: gitignore content, export exclusion, cleanup allowlist
1693 passed / 0 failed / 1 ignored; clippy clean.
- snapshot_then() brackets all 19 mutating dispatch arms: stage a
pre-op snapshot, finalise on success / discard on rollback; gated
on a user command source (internal ops like open-time rebuild are
not snapshotted) and on undo being enabled
- BatchState + BeginBatch/EndBatch requests: a batch takes one
boundary snapshot, suppresses per-command snapshots, and finalises
iff a mutation committed (one undo step per replay/batch)
- Undo/Redo/PeekUndo/PeekRedo requests handled in worker_loop with
&mut conn for the restore; cleanup() sweeps crash leftovers on open
- Database::{undo,redo,peek_undo,peek_redo,begin_batch,end_batch} +
open_with_persistence_and_undo(); snapshot failures are non-fatal
(logged), restore failures surface
- 6 Tier-3 integration tests through the real worker
1680 passed / 0 failed / 1 ignored; clippy clean.
Settles the undo/snapshot half (U1/U2) before implementation:
- every-mutation single-step undo (supersedes destructive-only model)
- hybrid whole-project snapshot (db backup API + yaml/csv copy),
reconciling ADR-0006 with ADR-0015's derived-db model
- persisted N=50 ring; redo discarded on new work
- batch ops (replay + future) record one undo step; import excluded
- --no-undo disable switch
Adds the implementation plan and updates README index, requirements
U1/U2, and CLAUDE.md in lockstep.
Records this session's close-out: ADR-0033 Phase 3 marked Accepted; ADR-0034 (history journal + replay filter, incl. Amendment 1 replay app-command skip) implemented and verified. Tees up ADR-0006's undo/snapshot half (U1/U2) as the next job with scope considerations and open design calls.
Replay (§3): run_replay parses <ts>|<status>|<source> journal records — runs ok, skips non-ok — while still accepting bare .commands scripts (prefix-detected so a | inside a bare command isn't misread). Fixes replay history.log, which died on line 1.
Journal failures (§1/§2): failed commands are recorded err via a new Action::JournalFailure, emitted by the pure-sync App for both parse failures and worker-execution failures (runtime appends best-effort, never fatal). Hydration reads all records so typo'd/rejected commands are recallable across sessions.
Amendment 1 — replay filters app-lifecycle commands: a working replay history.log exposed that the journal also records save as/load/new/export/import/rebuild/mode (which would panic the worker dispatch or abort replay). Replay now re-applies only schema/data writes and skips every app-lifecycle command + nested replay, classified by entry word so modal/incomplete forms (save as, bare mode) and quit skip uniformly rather than aborting. All skips continue (reversing the nested-replay refusal); import and nested replay warn. replay.error_nested removed; replay.skipped_import/_replay added; ReplayCompleted carries warnings. requirements.md U3/U4 updated; app-command runtime-failure journalling tracked as a follow-up.
1659 passing / 0 failing / 0 skipped / 1 ignored. Clippy clean.
Two-sub-task test-first plan mapping 1:1 to ADR-0034's named sub-tasks: (1) journal failures + per-consumer filtering (status-tagged append, best-effort err writes, hydration reads all), (2) replay parses the journal format (ok-only filter, dual-shape input). Opens with a headline failing test that reproduces the live replay history.log bug.
Phase 3 of the ADR-0030 SQL surface (DML) is implemented and verified through sub-phase 3k; mark ADR-0033 Accepted in the ADR and the README index (index-upkeep rule). Add handoff 34 tracking the close-out and teeing up ADR-0034 (history journal + replay filter) as the next job.
1645 passing / 0 failing / 0 skipped / 1 ignored. Clippy clean.
Wire `insert`/`update`/`delete` as shared DSL/SQL entry words through the
category-grouped dispatcher (ADR-0033 Amendment 1): the Advanced SQL nodes
move off the dev words (`sqlinsert`/`sql_update`/`sql_delete`) to the real
keywords, registered alongside the Simple DSL nodes. Remove the dev-word
scaffold; collapse build_sql_{insert,update,delete} to source.trim();
de-duplicate the two REGISTRY entry-word listing sites.
Dispatch model (ADR-0033 Amendment 3, written this round):
- A command is the mode-rooted grammar-path outcome; identity is intrinsic.
Advanced mode tries SQL first, falling back to the Simple DSL command when
no SQL branch matches a token (`delete … --all-rows` falls back;
`update … --all-rows` does not — the SET expression absorbs it, harmless
since the engine treats `--all-rows` as a comment).
- Simple mode commits the DSL candidate for a shared word, surfacing the real
DSL error; bare "this is SQL" is reserved for SQL-only entry words
(`select`/`with`). A content rejection on the SQL candidate (internal
table) is committed, never masked by the DSL fallback.
Combined DSL-error + advanced-SQL pointer (ADR-0033 Amendment 3): a Simple-mode
definite DSL error that would run as SQL in advanced mode gains the
`advanced_mode.also_valid_sql` suffix — in the live hint (ambient_hint_in_mode)
and on submit (dispatch_dsl), via the shared advanced_alternative_note — so the
actionable DSL fix and the mode pointer coexist (submit covers constructs that
surface only on submit, e.g. `delete … returning`).
Internal-table rejection symmetrised (/runda finding B, ADR-0030 §6): the DSL
data-command target slots (insert/update/delete/show data/show table) gained
reject_internal_table, so `__rdbms_*` tables are refused in Simple mode too —
previously only the advanced SQL grammar rejected them.
Mode-awareness: classify_input_with_schema_in_mode and
invalid_ident_at_cursor_in_mode stop leaking the advanced SQL view into
simple-mode hints for shared words.
Tests: dev-word inputs migrated to the real words (advanced); DSL grammar /
completion / phase-D / db tests parse in Simple mode (the DSL surface); replay
keeps its advanced-mode model (one stale assertion fixed); dispatcher routing,
combined-pointer, and internal-table tests added. Suite 1626 pass / 0 fail /
1 ignored; clippy --all-targets -D warnings clean.
Defer M4 (execution-time mode side-channel; tracked in requirements.md) to its
own ADR.
A focused adversarial round (/runda) found a single root cause with
six manifestations, all pre-existing latent false-positives: the
INSERT target is recorded under the `insert_target_table` role, not
as a diagnostic `bindings` entry, so refs that should resolve to the
*target* row were instead checked against the statement's bindings —
which for an `INSERT … SELECT` are the SELECT's *source* tables (the
wrong scope), producing false unknown_column / unknown_qualifier
diagnostics on valid input.
New helper bare_ref_insert_target re-scopes a ref onto the INSERT
target when it sits in a target-referencing region: the UPSERT
DO UPDATE action (byte range) or an INSERT's RETURNING list. Applied
across every ref form:
1. INSERT column list (insert_column) — validated vs the target,
skipped in the bare-column branch (was checked vs SELECT source).
2. ON CONFLICT (col) target (conflict_target_column) — same.
3. DO UPDATE SET RHS / WHERE bare refs — validated vs the target
(also closes the #12 residual for VALUES upserts).
4. RETURNING bare refs — validated vs the target.
5. target-qualified refs `t.col` in DO UPDATE / RETURNING — the
unified `excluded` / target-qualifier resolution in the
qualified-ref None branch.
6. target-qualified star `t.*` in RETURNING — same re-scoping in
the qualified-star handler.
Each fix has a positive (resolves cleanly) and negative (genuinely
unknown column / unrelated qualifier still flagged) test; the
`excluded` leak guard and all prior diagnostics remain green.
1613 pass / 0 fail / 1 ignored. Clippy clean.
DA pass on 3i. Fix: build_schema_cache set not_null = c.notnull ||
c.primary_key, which would false-flag an omitted `int` PK as a
not_null_missing WARNING — but an int PK is an INTEGER PRIMARY KEY
rowid alias that auto-fills (and SQLite's PK-NULL quirk means a PK
isn't implicitly NOT NULL anyway). Use c.notnull alone (ADR-0033
§8.3 "declared NOT NULL"): faithful and false-positive-free.
Arity-walk hardening (same class as the ON CONFLICT regression the
existing tests caught mid-3i): RETURNING after VALUES is a depth-0
keyword that ends the tuple list (only the real tuple is flagged),
and a comma nested in a function-call value (depth ≥ 2) does not
inflate the tuple's value count.
Tests (+2). 1598 pass / 0 fail / 1 ignored. Clippy clean.
New dml_target_column_diagnostics pass: an ERROR for an unknown column
in the INSERT column list or the UPSERT DO UPDATE SET (validated
directly against the insert_target_table). The INSERT target isn't a
flat-scope `bindings` entry, so the existing schema-existence pass
didn't cover these; a targeted pass avoids the false INSERT…SELECT
ambiguity a global binding would cause.
Closes the 3i cross-cut "schema-existence fires on INSERT VALUES"
gate item, and closes the DA finding #12 (UPSERT DO UPDATE SET column
now flagged like a top-level UPDATE's SET column). Residual: bare
sql_expr_ident refs in the DO UPDATE SET RHS / WHERE remain
unvalidated for upserts (the documented flat-scope limitation).
Tests (+5): unknown INSERT column flagged + known silent; unknown
DO UPDATE SET column flagged + known/excluded silent; predicate
warning (= NULL) fires on a SQL UPDATE WHERE (cross-cut). 1596 pass /
0 fail / 1 ignored. Clippy clean.
Extend SchemaCache TableColumn with not_null + has_default (with a
TableColumn::new constructor for the common no-constraint case),
populated in build_schema_cache from ColumnDescription (a PK column
counts as not-null). New dml_not_null_missing_diagnostics pass: a
WARNING when a SQL INSERT's explicit column list omits a column that
is NOT NULL with no DEFAULT — advisory (the engine enforces it).
serial/shortid (auto-filled) and defaulted columns are excluded.
Anchored on the target-table ident (no token for the omitted column).
Catalog key diagnostic.not_null_missing (engine-neutral). Tests (+4):
fires on omitted required column; silent when included, when
defaulted, and for auto-gen serial/shortid. ~24 TableColumn literal
sites updated for the two new fields (build clean). 1591 pass / 0
fail / 1 ignored. Clippy clean.
All three ADR-0033 §8 DML diagnostics now implemented. Remaining 3i:
cross-cut verification + #12 UPSERT DO UPDATE validation.
New dml_insert_arity_diagnostics pass (ERROR): when an explicit
(column_name_list) arity disagrees with a row's arity. VALUES tuples
are checked per-row (each offending tuple emits its own diagnostic on
its span; matched rows stay silent). INSERT … SELECT compares the
first SELECT leg's projection arity, anchored on the first projection
item; a WITH-prefixed row source is skipped (engine still reports it —
a false positive would be worse). No-column-list form deferred
(needs schema; outside the 3i gate).
The VALUES walk stops at the first depth-0 keyword so an ON CONFLICT
(col) conflict target / RETURNING tail is not mis-counted as a value
tuple (caught by the existing upsert_excluded tests during dev).
Catalog key diagnostic.insert_arity_mismatch (engine-neutral).
Tests (+7): single-row + matched + per-row multi-row; INSERT…SELECT
mismatch + matched; ON CONFLICT interaction (only the real tuple
flagged, clean case silent). 1587 pass / 0 fail / 1 ignored. Clippy
clean. Remaining 3i: not_null_missing (needs TableColumn
not_null+default), cross-cut verification, #12 UPSERT DO UPDATE
validation.
New dml_auto_column_diagnostics pass: a WARNING when a SQL INSERT's
explicit column list names a serial/shortid (auto-generated) column —
the explicit value bypasses the auto-counter/generator and may collide
with later auto-generated values. Advisory only (ADR-0027 §1); the
statement still runs. Conflict-target columns (distinct
conflict_target_column role) are not mistaken for inserted columns.
Catalog key diagnostic.auto_column_overridden (engine-neutral).
Tests (+4): serial + shortid fire; omitted is silent; ON CONFLICT
target not falsely flagged. 1580 pass / 0 fail / 1 ignored. Clippy
clean. Remaining 3i: insert_arity_mismatch, not_null_missing (needs
TableColumn not_null+default), cross-cut verification, #12 UPSERT
DO UPDATE validation.
on_conflict_clause on SQL_INSERT_SHAPE: optional (col,…) conflict
target (distinct conflict_target_column role so it never enters
listed_columns), DO NOTHING / DO UPDATE SET … [WHERE …]. `do` is
factored out of the action Choice so nothing/update disambiguate
without tripping the walk_seq/walk_choice shared-prefix trap
(ADR-0033 Amendment 1). Worker runs the UPSERT verbatim (SQLite
native); no new execution path.
build_sql_insert: row_source now stops before the FIRST trailing
clause — ON CONFLICT (3h) or RETURNING (3g) — and do_sql_insert's
shortid auto-fill rewrite re-appends the whole trailing tail, so an
auto-filled INSERT keeps its ON CONFLICT / RETURNING.
excluded pseudo-table (§9): resolves to the target's columns inside
the DO UPDATE action and completes at `excluded.|`, but stays flagged
as unknown_qualifier in VALUES / RETURNING / non-upsert statements.
Diagnostic pass scopes it by the DO UPDATE byte-range (update token →
RETURNING/end); completion resolves it against the INSERT target's
current_table_columns. NOTE: scoping uses byte-range rather than the
plan's prescribed from_scope TableBinding push — same behaviour, no
walker scope-frame change.
Tests (+13): grammar accept/reject; DO NOTHING / DO UPDATE-excluded /
no-target execution + persistence; auto-fill × ON CONFLICT with a
REAL unique conflict (proves the clause survives the rewrite, not a
no-op); excluded resolves in DO UPDATE SET + WHERE, flagged in VALUES
(incl. same statement), unknown column under excluded; excluded.|
completion; conflict-target not in listed_columns. 1576 pass / 0 fail
/ 1 ignored. Clippy clean. Dev sql_insert entry word still removed in
3j.
Known follow-up (tracked for 3i): UPSERT DO UPDATE bare column refs
(SET LHS / WHERE) are not schema-validated, unlike regular UPDATE —
the INSERT target isn't a diagnostic binding. Fits 3i's cross-cut
SET/WHERE validation scope.
Shared RETURNING_CLAUSE (reuses Phase-2 PROJECTION_LIST, now
pub(crate)) as an optional tail on all three SQL DML shapes.
`returning: bool` on the Command variants, set by the ast-builders
and threaded to the worker. run_returning collects the returned rows
as a DataResult (RETURNING mutates + yields in one pass), reusing
resolve_select_column_types for bare-column type recovery; computed
projections stay typeless. DeleteResult gains a `data` field rendered
alongside the cascade summary.
Follow-set fix: `returning` is added to the table-source and
projection bare-alias follow-sets so an INSERT … SELECT row source
stops before RETURNING instead of reading it as a table alias.
Auto-fill × RETURNING: build_sql_insert stops row_source before the
RETURNING token (keeping it preparable for shortid materialisation),
and plan_shortid_autofill re-appends the RETURNING tail so generated
shortids surface in RETURNING *.
Tests (+17): grammar accept on all three; INSERT/UPDATE/DELETE
RETURNING incl. *, aliases, multi-row, type recovery + computed-
typeless; auto-fill × RETURNING (single + multi-row distinct ids);
INSERT…SELECT…RETURNING execution; UPDATE…RETURNING zero-match;
DELETE…RETURNING cascade+rows; app-level render of both. Dev
sql_insert/sql_update/sql_delete entry words still removed in 3j.
1562 pass / 0 fail / 1 ignored. Clippy clean.
Found while implementing 3f: history.log is success-only, but the
in-memory Up/Down recall ring records every submission — and the ring
is re-seeded from the log on open, so failed commands are recallable
in-session yet lost across sessions. Replay and recall also want
different inputs (state-builders vs everything-typed), which one
success-only file can't serve. And replay never parsed the pipe
format (run_replay parses whole lines), so `replay history.log` fails
on line 1 with no test covering it.
Decision: history.log becomes a complete journal tagged ok/err;
hydration reads all, replay reads ok-only and learns the format.
Amends ADR-0006 + ADR-0015 §5/§12. Code deferred to two tracked
sub-tasks. No migration for existing all-ok logs.
A self-referential ON DELETE CASCADE FK (e.g. T.ParentId -> T.id) is
returned by read_relationships_inbound as a child whose table IS the
delete target. The before/after row-count diff then includes the
directly-deleted rows (already in rows_affected), so deleting a chain
root reported 3 cascaded rows when only 2 were removed via the
self-reference.
Fix in both do_delete (DSL) and do_sql_delete (SQL): when the child
table equals the target, subtract rows_affected from the diff and
guard on the corrected count (a leaf delete no longer reports a
phantom 0-row self-cascade); the target's CSV is already queued, so a
self-ref child is not re-added to rewritten_tables. Pre-existing in
do_delete; surfaced by the 3f DA pass, fixed in both paths to keep
DSL/SQL parity. Behaviour: report only the rows removed via the
self-reference (user-confirmed).
Also adds an app-level render test for the SQL DELETE path
(handle_dsl_delete_success via CommandOutcome::Delete) — the shared
renderer's ok-summary + per-relationship cascade line were exercised
only through the DSL path before.
Test-first: self_referential_cascade_counts_only_cascaded_rows added
for both paths (asserted 2, failed at 3 before the fix). 1545 pass /
0 fail / 1 ignored. Clippy clean.
New src/dsl/grammar/sql_delete.rs (FROM <table> [WHERE] [;]),
Command::SqlDelete, Request::RunSqlDelete, do_sql_delete worker.
do_sql_delete mirrors the DSL do_delete: detect FK cascade by
before/after child row-count diffing, re-persist target + every
cascade-affected child, history-on-success inside the tx. Reuses
CommandOutcome::Delete -> handle_dsl_delete_success, so the
per-relationship cascade summary formatter is shared, not duplicated.
ADR-0033 Amendment 2: supersedes §7's WHERE-injected pre-count. Its
premise (DSL handler builds pre-counts from the typed Expr) was wrong
— do_delete uses count-diff. The pre-count would also have broken the
§2 parity promise by reporting SET NULL the DSL path doesn't. Count-
diff gives exact parity, no WHERE-byte extraction, and withdraws R2.
SET NULL reporting deferred for both paths (user-confirmed).
Tests: +6 grammar unit, +12 integration (cascade parity with DSL,
both R2 subquery cases, before-execute order, no-WHERE, FK-rejection
rollback, childless-parent, two-child cascade). 1542 pass / 0 fail /
1 ignored. Clippy clean. Dev sql_delete entry word removed in 3j.
New src/dsl/grammar/sql_update.rs: SQL_UPDATE_SHAPE =
<table> SET col = sql_expr (',' …)* [WHERE sql_expr] [';'], the
__rdbms_* target rejection, and the shared sql_expr on both the
assignment RHS and the predicate. No --all-rows rail — a SQL
UPDATE without WHERE runs as written (ADR-0030 §12). Reuses
sql_select::WHERE_CLAUSE (now pub(crate)) so the predicate
diagnostics are identical. The target uses the shared `table_name`
ident role (not a bespoke one) so the Phase-2 schema-existence and
predicate-warning passes collect it as a scope binding and check
the SET / WHERE columns for free — a bespoke role left them
unchecked (the cross-cut tests caught this).
Command::SqlUpdate { sql, target_table }; Request::RunSqlUpdate +
do_sql_update (execute validated SQL via execute_with_fk_enrichment,
re-persist the target CSV, append history.log). 3e surfaces the
affected-row count only; precise row output is RETURNING (3g), so
the update-success render skips a column-less data set rather than
showing a misleading "(no rows)" band. Behind the dev `sql_update`
entry word until 3j.
Tests: grammar accept/reject; integration (single/multi-col,
no-WHERE all-rows, sql_expr in SET, scalar subquery in SET,
zero-match success, history); walker cross-cut (unknown SET column
→ unknown_column, `= NULL` in WHERE → eq_null warning); app-level
render-guard both ways (column-less → count only; with columns →
table renders). 1524 green, clippy clean.
claude@clouddev1