From 55b784597a41a8e59f3d8178b446ace29a94a794 Mon Sep 17 00:00:00 2001 From: "claude@clouddev1" Date: Sun, 24 May 2026 09:23:03 +0000 Subject: [PATCH] docs: ADR-0033 Accepted (Phase 3 complete) + session handoff 34 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. --- docs/adr/0033-sql-dml-grammar.md | 8 +- docs/adr/README.md | 2 +- docs/handoff/20260523-handoff-34.md | 195 ++++++++++++++++++++++++++++ 3 files changed, 203 insertions(+), 2 deletions(-) create mode 100644 docs/handoff/20260523-handoff-34.md diff --git a/docs/adr/0033-sql-dml-grammar.md b/docs/adr/0033-sql-dml-grammar.md index 7a39f2c..a9150fd 100644 --- a/docs/adr/0033-sql-dml-grammar.md +++ b/docs/adr/0033-sql-dml-grammar.md @@ -2,7 +2,13 @@ ## Status -Proposed +Accepted + +Phase 3 implemented and verified through sub-phase 3k +(2026-05-23); see `docs/handoff/20260523-phase-3-verification.md` +for the phase-exit report and the filled cross-cut matrix in +`docs/plans/20260520-adr-0033-phase-3.md`. Amendments 1–3 below +are part of this acceptance. ## Context diff --git a/docs/adr/README.md b/docs/adr/README.md index 3c801fa..0e4b580 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -38,5 +38,5 @@ This directory contains the project's ADRs, recorded per - [ADR-0030 — Advanced mode: the standard-SQL surface](0030-advanced-mode-sql-surface.md) — **Accepted**, SQL added as grammar *within the unified grammar tree* (ADR-0024), not a separate batch parser — so SQL gets the same completion / highlighting / hints / parse-errors as the DSL; mode gates the SQL forms; DDL routes through the typed `Command` executor (metadata + type vocabulary preserved), DML and `SELECT` execute as validated SQL; engine-neutral posture, the DSL→SQL teaching echo; supersedes ADR-0001's `sqlparser-rs` reservation; phased plan (`Q1` / `Q2` / `Q4`) - [ADR-0031 — The SQL expression grammar](0031-sql-expression-grammar.md) — **Accepted**, the stratified SQL expression grammar fragment commissioned by ADR-0030 §3: a single precedence ladder (`OR`/`AND`/`NOT`, the comparison/`LIKE`/`IN`/`BETWEEN`/`IS NULL` predicate set, arithmetic incl. `||`, function calls, `CASE`) — the superset of ADR-0026's DSL `WHERE` grammar, authored as a parallel fragment so simple mode is untouched; pure validation, builds **no** AST (consumers run/store SQL as text per ADR-0030 §4/§6); reuses ADR-0026's `Subgrammar` recursion + depth cap unchanged; subquery expressions and qualified column refs deferred to ADR-0030 Phase 2 - [ADR-0032 — The full SQL `SELECT` grammar](0032-sql-select-grammar.md) — **Accepted**, the Phase-2 grammar commissioned by ADR-0030 §3: full `SELECT` with `INNER`/`LEFT`/`RIGHT`/`FULL OUTER`/`CROSS` joins, `GROUP BY`/`HAVING`, all four set ops (`UNION`/`UNION ALL`/`INTERSECT`/`EXCEPT`), `WITH` and `WITH RECURSIVE` CTEs, `LIMIT … OFFSET`, `DISTINCT`, `t.*`, and bare-alias projection (lifting Phase-1 §4.2); additive extensions to ADR-0031's `sql_expr` for scalar subqueries, `IN (SELECT …)`, `[NOT] EXISTS`, and qualified column refs (redeeming ADR-0031 §7 OOS-1/OOS-2); grammar-recursion via `Subgrammar(&SQL_SELECT_COMPOUND)` reuses ADR-0026's `MAX_SUBGRAMMAR_DEPTH = 64` cap unchanged; **softens ADR-0030 §8's "ambient assistance comes for free" claim**: completion scope needs new `WalkContext` accumulators (a `from_scope_stack` of `ScopeFrame`s holding `from_scope` / `cte_bindings` / `projection_aliases`), a **new walker node variant `Node::ScopedSubgrammar(&Node)`** as the push/pop trigger (existing `Node::Subgrammar` unchanged so DSL `Expr` and `sql_expr` recursion are unaffected), qualified-prefix completion narrowing, body-projection-derived CTE column resolution (so `SELECT *` and explicit-projection CTE bodies both yield real column completion past `cte_alias.|`), and a **post-walk fixup pass** that re-resolves projection-list identifier highlighting/validity once `FROM` is parsed (the projection-before-FROM problem); classifies every Phase-2 validation case against ADR-0027's ERROR/WARNING guideline (§11): five new `diagnostic.*` keys for parse-time-detectable cases (unknown qualifier, ambiguous column, projection-alias misplaced, CTE/compound arity mismatch) plus eight `engine.*` translation keys; a MatchedPath-walking predicate-warnings variant that closes the Phase-1 gap where SQL `WHERE` expressions emitted no `LIKE`-on-numeric / `= NULL` / type-mismatch warnings (ADR-0027 Amendment 1 finally extends to the SQL surface); adds a worker-side post-prepare type-resolution pass via engine column-origin metadata so bare column refs recover their playground type (partially lifting Phase-1 §4.5, the bool→0/1 case) — `Cargo.toml` gains `column_metadata` to rusqlite features (verified against pinned 0.39.0); `__rdbms_*` rejection extended to every new table-source slot; Amendment 1 narrows §12's resolution rule from a grammar-side structural classification to "trust the engine's column-origin metadata verbatim" after an empirical probe showed origin metadata follows through non-recursive CTEs, scalar subqueries, derived tables, set ops, and joins — the one structural exception is recursive CTE result columns, which return None and stay typeless; Amendment 2 records that §10.6's "rewrite the highlight class" prescription is realised via the two-pass schema-existence diagnostic + the renderer's diagnostic-overlay path (no separate per-byte rewrite step needed; no new HighlightClass variant), and that the projection-before-FROM completion narrowing has been improved by an `src/completion.rs` look-ahead probe when the leading walk's `from_scope` is empty but the full input parses -- [ADR-0033 — The full SQL DML grammar (`INSERT` / `UPDATE` / `DELETE`)](0033-sql-dml-grammar.md) — **Proposed**, the Phase-3 grammar commissioned by ADR-0030 §3: single- and multi-row `INSERT` (incl. `INSERT … SELECT` recursing through ADR-0032's `SQL_SELECT_COMPOUND`), `UPDATE` with `SET` assignment list, `DELETE`, all three optionally followed by `RETURNING projection_list`, plus full `ON CONFLICT … DO NOTHING / DO UPDATE` UPSERT on INSERT; **fixes the DSL-vs-SQL dispatch architecture for shared entry words (`insert`/`update`/`delete`)**: SQL-first / DSL-fallback in Advanced mode via a `Choice(SQL_shape, DSL_shape)` per shape, gated by a new walker capability `Node::Guard(fn)` — a zero-byte-consumption gating node that fails the enclosing Seq with a `ValidationError`; carries `Command::SqlInsert` / `SqlUpdate` / `SqlDelete` variants and `do_sql_*` worker handlers each of which knows the target table (for re-persistence) and the `returning: bool` flag (for DataResult routing); `shortid` auto-fill mirrors the DSL `do_insert` mechanism via worker post-fill; SQL DELETE produces the same per-relationship cascade summary the DSL DELETE does (ADR-0014 parity); three new walker diagnostics (`insert_arity_mismatch` ERROR, `auto_column_overridden` WARNING, `not_null_missing` WARNING) with positive + negative tests each; OOS list explicitly carves out `DEFAULT VALUES` (the project's planned seed feature), SQLite-specific `OR REPLACE` / `OR IGNORE` / `OR ABORT` / `OR FAIL` / `OR ROLLBACK` prefixes, `UPDATE FROM` multi-table updates, and WITH-prefixed DML; the `excluded` keyword inside `ON CONFLICT DO UPDATE` is a deliberate carve-out from ADR-0030 §7's engine-neutral posture (no standard-SQL UPSERT spelling exists that SQLite and PostgreSQL share); eleven phased sub-phases each with explicit exit gates + written DA gate, opening with the dispatch mechanism before any DML grammar lands; initial DA review recorded seven critiques that were resolved before status moved to Proposed; **Amendment 1 supersedes §2's dispatch mechanism**: the originally-chosen `Node::Guard(fn)` + `Choice(SQL_shape, DSL_shape)` was found during 3a to be unworkable as framed (any guard-in-`Choice` mechanism forces a `walk_choice` change — `walk_choice` only falls through on `NoMatch`, so Simple-mode valid-DSL would wrongly surface "this is SQL", and `walk_seq` treats a `NoMatch` past `idx 0` as a hard `Failed`, breaking Advanced-mode DSL fall-through); replaced by **category-grouped, mode-aware dispatch** in `walker::walk` (each `REGISTRY` entry tagged `CommandCategory::{Simple, Advanced}`, generalising the existing whole-command `is_advanced_only` gate), shared entry words carrying a node in both groups, no `Node::Guard` and no `walk_choice`/`walk_seq` change, advanced-mode completion SQL-first with DSL as a full-line fallback; **Amendment 2 (sub-phase 3f) supersedes §7's cascade mechanism**: the WHERE-injected per-child pre-count rested on a premise that was factually wrong about the DSL handler (which detects cascades by before/after row-count diffing inside a transaction, not by `Expr`-derived pre-count subqueries) and would have broken the §2 parity promise by reporting `SET NULL` the DSL path doesn't; replaced by mirroring `do_delete`'s count-diff exactly (verbatim DELETE executes, child-count diff observes the cascade — `ON DELETE CASCADE` row removals only, SET NULL deferred for both paths to preserve parity), which shares the render-layer formatter for free via `CommandOutcome::Delete` and **withdraws risk R2** (no WHERE-byte extraction, no N+1 subquery); **Amendment 3 (sub-phase 3j) records the command-identity model and defers the execution-mode side-channel**: a command is the typed outcome of a *mode-rooted* grammar path and its identity is intrinsic (Advanced mode tries SQL first, falls back to the *Simple* DSL command when no SQL branch matches a token, e.g. `delete … --all-rows`; note `update … --all-rows` does *not* fall back — the SQL `SET` expression eats `--all-rows`, harmless since the engine treats it as a comment); **Simple mode commits the DSL candidate for shared words** so the *real* DSL error surfaces, and when that line would also run in advanced mode the rendering layer **combines** them — DSL error **plus** an `advanced_mode.also_valid_sql` pointer ("… (valid as SQL in advanced mode)") — keeping the actionable DSL fix while pointing at advanced mode; bare "this is SQL" is reserved for entry words with no DSL form (`select`/`with`); a fully-overlapping input (`insert … values …`) legitimately yields *two distinct commands* (`Command::Insert` typed-AST vs `Command::SqlInsert` validated-text) that do the same thing but execute differently (ADR-0030 §4), so each is tested in the mode that produces it; **corrects the plan's 3j exit-gate premise** that the DSL DML tests run in Simple mode (they call `parse_command`, which defaults to Advanced) — the real invariant is "Simple-mode behaviour unchanged, Advanced mode SQL-first, DSL grammar tested in Simple mode, both variants tested in their producing mode", with §6/§7 parity keeping the paths observably equivalent; and **defers to its own future ADR** the execution-time mode side-channel (three-way `Mode`: simple/advanced/advanced-one-shot threaded through `Action`→worker, for mode-dependent *output* like echoing generated SQL) — today only the *rendering* side-channel `OutputLine.mode_at_submission` exists, and the three-way distinction is not required for Phase 3 dispatch correctness +- [ADR-0033 — The full SQL DML grammar (`INSERT` / `UPDATE` / `DELETE`)](0033-sql-dml-grammar.md) — **Accepted** (implemented + verified through sub-phase 3k, 2026-05-23; phase-exit report `docs/handoff/20260523-phase-3-verification.md`), the Phase-3 grammar commissioned by ADR-0030 §3: single- and multi-row `INSERT` (incl. `INSERT … SELECT` recursing through ADR-0032's `SQL_SELECT_COMPOUND`), `UPDATE` with `SET` assignment list, `DELETE`, all three optionally followed by `RETURNING projection_list`, plus full `ON CONFLICT … DO NOTHING / DO UPDATE` UPSERT on INSERT; **fixes the DSL-vs-SQL dispatch architecture for shared entry words (`insert`/`update`/`delete`)**: SQL-first / DSL-fallback in Advanced mode via a `Choice(SQL_shape, DSL_shape)` per shape, gated by a new walker capability `Node::Guard(fn)` — a zero-byte-consumption gating node that fails the enclosing Seq with a `ValidationError`; carries `Command::SqlInsert` / `SqlUpdate` / `SqlDelete` variants and `do_sql_*` worker handlers each of which knows the target table (for re-persistence) and the `returning: bool` flag (for DataResult routing); `shortid` auto-fill mirrors the DSL `do_insert` mechanism via worker post-fill; SQL DELETE produces the same per-relationship cascade summary the DSL DELETE does (ADR-0014 parity); three new walker diagnostics (`insert_arity_mismatch` ERROR, `auto_column_overridden` WARNING, `not_null_missing` WARNING) with positive + negative tests each; OOS list explicitly carves out `DEFAULT VALUES` (the project's planned seed feature), SQLite-specific `OR REPLACE` / `OR IGNORE` / `OR ABORT` / `OR FAIL` / `OR ROLLBACK` prefixes, `UPDATE FROM` multi-table updates, and WITH-prefixed DML; the `excluded` keyword inside `ON CONFLICT DO UPDATE` is a deliberate carve-out from ADR-0030 §7's engine-neutral posture (no standard-SQL UPSERT spelling exists that SQLite and PostgreSQL share); eleven phased sub-phases each with explicit exit gates + written DA gate, opening with the dispatch mechanism before any DML grammar lands; initial DA review recorded seven critiques that were resolved before status moved to Proposed; **Amendment 1 supersedes §2's dispatch mechanism**: the originally-chosen `Node::Guard(fn)` + `Choice(SQL_shape, DSL_shape)` was found during 3a to be unworkable as framed (any guard-in-`Choice` mechanism forces a `walk_choice` change — `walk_choice` only falls through on `NoMatch`, so Simple-mode valid-DSL would wrongly surface "this is SQL", and `walk_seq` treats a `NoMatch` past `idx 0` as a hard `Failed`, breaking Advanced-mode DSL fall-through); replaced by **category-grouped, mode-aware dispatch** in `walker::walk` (each `REGISTRY` entry tagged `CommandCategory::{Simple, Advanced}`, generalising the existing whole-command `is_advanced_only` gate), shared entry words carrying a node in both groups, no `Node::Guard` and no `walk_choice`/`walk_seq` change, advanced-mode completion SQL-first with DSL as a full-line fallback; **Amendment 2 (sub-phase 3f) supersedes §7's cascade mechanism**: the WHERE-injected per-child pre-count rested on a premise that was factually wrong about the DSL handler (which detects cascades by before/after row-count diffing inside a transaction, not by `Expr`-derived pre-count subqueries) and would have broken the §2 parity promise by reporting `SET NULL` the DSL path doesn't; replaced by mirroring `do_delete`'s count-diff exactly (verbatim DELETE executes, child-count diff observes the cascade — `ON DELETE CASCADE` row removals only, SET NULL deferred for both paths to preserve parity), which shares the render-layer formatter for free via `CommandOutcome::Delete` and **withdraws risk R2** (no WHERE-byte extraction, no N+1 subquery); **Amendment 3 (sub-phase 3j) records the command-identity model and defers the execution-mode side-channel**: a command is the typed outcome of a *mode-rooted* grammar path and its identity is intrinsic (Advanced mode tries SQL first, falls back to the *Simple* DSL command when no SQL branch matches a token, e.g. `delete … --all-rows`; note `update … --all-rows` does *not* fall back — the SQL `SET` expression eats `--all-rows`, harmless since the engine treats it as a comment); **Simple mode commits the DSL candidate for shared words** so the *real* DSL error surfaces, and when that line would also run in advanced mode the rendering layer **combines** them — DSL error **plus** an `advanced_mode.also_valid_sql` pointer ("… (valid as SQL in advanced mode)") — keeping the actionable DSL fix while pointing at advanced mode; bare "this is SQL" is reserved for entry words with no DSL form (`select`/`with`); a fully-overlapping input (`insert … values …`) legitimately yields *two distinct commands* (`Command::Insert` typed-AST vs `Command::SqlInsert` validated-text) that do the same thing but execute differently (ADR-0030 §4), so each is tested in the mode that produces it; **corrects the plan's 3j exit-gate premise** that the DSL DML tests run in Simple mode (they call `parse_command`, which defaults to Advanced) — the real invariant is "Simple-mode behaviour unchanged, Advanced mode SQL-first, DSL grammar tested in Simple mode, both variants tested in their producing mode", with §6/§7 parity keeping the paths observably equivalent; and **defers to its own future ADR** the execution-time mode side-channel (three-way `Mode`: simple/advanced/advanced-one-shot threaded through `Action`→worker, for mode-dependent *output* like echoing generated SQL) — today only the *rendering* side-channel `OutputLine.mode_at_submission` exists, and the three-way distinction is not required for Phase 3 dispatch correctness - [ADR-0034 — `history.log` as a complete command journal; replay reads success-only](0034-history-journal-and-replay-filter.md) — **Accepted**, resolves a three-way tension in `history.log`'s roles found while implementing ADR-0033 3f: (1) the persistent log is success-only while the in-memory Up/Down recall ring records *every* submission (success or failure, "so users can recall and edit typo'd commands"), and the ring is re-seeded from the log on project open — so **failed commands are recallable within a session but silently lost across sessions**; (2) replay wants the state-building (successful) commands while recall wants everything typed, which one success-only file cannot serve; (3) `replay history.log` never actually worked — `run_replay` parses each whole line through the DSL parser with no understanding of the `||` record shape, so a real log fails on line 1, and **no test ever fed the pipe format to replay** (the `replay_history_log_records_subcommands_only` test only checks what replay *writes*, never replays the log as input). Decision: `history.log` becomes a **complete journal** — every submission recorded, tagged `ok`/`err` via the status field the format already reserved (ADR-0015 §5) — and **each consumer filters**: hydration reads all records (cross-session recall matches in-session), replay reads `ok` only (and learns the journal format, while still accepting bare-command `.commands` scripts; detection by the leading timestamp+status prefix so a `|` inside a bare command isn't misread). Successful commands stay journalled transactionally by the worker; failed commands are journalled `err` best-effort from the runtime/app error path (a parse failure never reaches the worker). Amends ADR-0006's "successfully executed" wording and ADR-0015 §5 ("status always `ok`") / §12 (hydration). Code deferred to two tracked test-first sub-tasks (journal-failures+filtering; replay-parses-journal-format); existing all-`ok` logs need no migration diff --git a/docs/handoff/20260523-handoff-34.md b/docs/handoff/20260523-handoff-34.md new file mode 100644 index 0000000..e7697ac --- /dev/null +++ b/docs/handoff/20260523-handoff-34.md @@ -0,0 +1,195 @@ +# Session handoff — 2026-05-23 (34) + +Thirty-fourth handover. This session completed **ADR-0033 +sub-phase 3k** — the Phase-3 verification sweep — closing Phase 3 +of the ADR-0030 advanced-mode SQL surface. A user-invoked +`/runda` round hardened the matrix attributions and closed two +small coverage gaps. **ADR-0033 is now Accepted.** + +The next session (or the continuation of this one) starts the +**ADR-0034** implementation. See §4. + +## §1. State at handoff + +**Branch:** `main`. **Tests: 1645 passing, 0 failing, 0 skipped, +1 ignored** (the unchanged `friendly/mod.rs` doctest). **Clippy:** +clean (`cargo clippy --all-targets -- -D warnings`). + +**Latest committed (local-only):** + +``` +380c423 test+docs: 3k Phase-3 verification sweep — e2e DML + filled cross-cut matrix +``` + +`origin/main` is at `6b8888f` (3h), so **10 commits are +local-only** (the 3i series, handoff-32, 3j `d5c7f63`, handoff-33 +`a5cdb00`, and this session's 3k `380c423`). Unpushed commits are +a normal working state; pushing is the user's step — do not prompt +about it. + +**Uncommitted at the moment of writing this handoff** (the +ADR-0033 acceptance flip + this handoff doc): + +- `docs/adr/0033-sql-dml-grammar.md` — Status `Proposed` → + `Accepted`. +- `docs/adr/README.md` — ADR-0033 index line flipped to + `Accepted` (same-edit index-upkeep rule). +- `docs/handoff/20260523-handoff-34.md` — this file. + +These three should be committed together (proposed message in §8). + +## §2. What shipped this session (3k) + +Authoritative detail lives in the phase-exit report: +**`docs/handoff/20260523-phase-3-verification.md`** — read it for +the filled matrix, the requirements-to-test mapping, and the DA +review. Summary: + +- **`tests/sql_dml_e2e.rs` (new)** — 11 Tier-3 end-to-end DML + tests (parse-in-Advanced → worker → persist/history): INSERT… + SELECT cross-table, all-ten-type multi-row INSERT + `RETURNING` + type recovery, UPDATE-with-subquery-in-SET, cascade DELETE, + UPSERT round-trip, `RETURNING` ×3, `history.log` replay of + Phase-3 forms, the full §13 OOS rejections, a trailing-`;` guard, + and the `[WRN]` validity indicator driven by a SQL DML diagnostic. +- **Cross-cut gap-fill** in `src/`: `walker/mod.rs` (5 + inherited-diagnostic cross-cuts — DELETE/UPDATE WHERE + schema-existence, like-numeric on DELETE WHERE, the corrected + row-scoped predicate slots), `highlight.rs` (DML-keyword + highlight), `completion.rs` (`INSERT INTO` target-table), + `input_render.rs` (advanced-mode DML hint-panel — added in the + `/runda` round). +- **Cross-cut matrix filled** in + `docs/plans/20260520-adr-0033-phase-3.md` — **85 ✅, 1 N/A, 0 + ❌**, every row a verified `file::function`. +- **ADR-0033 → Accepted** (this handoff's commit). + +## §3. Decisions settled this session (do not re-litigate) + +Both escalated and **user-confirmed** before any matrix/test edit +(via `AskUserQuestion`); recorded in the phase-exit report §4 and +the matrix footnotes [d]/[e]: + +1. **Predicate-warning-in-VALUES (matrix I7) — claim corrected.** + Predicate warnings (`eq_null` / `like_numeric` / `type_mismatch`) + fire on every DML `sql_expr` slot with **row scope** (UPDATE/ + DELETE WHERE, UPDATE SET incl. CASE, INSERT…SELECT projection & + WHERE) but **not** in plain `INSERT … VALUES` — a VALUES literal + has no row to compare against. This matches ADR-0033 §8.4 ("WHERE + and CASE"); the original "(e.g. = with NULL in VALUES)" example + was an over-statement. Backed by new row-scoped tests. + +2. **Auto-snapshot (ADR-0006 / ADR-0033 §10) — N/A, deferred.** + ADR-0006 auto-snapshot/undo is unimplemented for **both** the + DSL and SQL paths (the U-series). The "same as DSL" parity holds + vacuously; Phase 3 introduces no regression. The matrix row is + N/A, not red. + +The `/runda` round additionally **corrected the A6 attribution** +(the hint-panel tests ran in Simple mode via `ambient_hint`, which +defaults to `Mode::Simple`; added an advanced-mode DML hint test) +and **closed OOS-5/OOS-6** (INDEXED BY / multi-statement — both +parse-reject; a single trailing `;` still parses). + +## §4. ADR-0034 — the NEXT job + +**Implement ADR-0034** (`docs/adr/0034-history-journal-and-replay-filter.md`, +**Accepted**). This is the user's standing "extra tasks after +Phase 3" item; it subsumes the old TASK #8 + TASK #9 and fixes a +**real, currently-broken behaviour** verified this session: + +> `replay history.log` fails on line 1. `run_replay` +> (`src/runtime.rs:1630`) feeds each whole raw line straight to +> `parse_command_with_schema` and only skips blank/`#` lines — it +> does **not** understand the `||` journal +> format that `history.rs` already writes. So replay only works on +> hand-written bare-command `.commands` scripts, never on the +> actual journal. No test catches this (existing replay tests + +> the new 3k e2e replay test all use bare-command scripts). + +Concrete work (ADR-0034 §§1–4): + +1. **Writer records every command, tagged `ok` / `err`** (today + `history.rs` hardcodes `ok`). `err` lines are journalled + best-effort from the runtime/app error path (a parse failure + never reaches the worker); `ok` lines stay worker-journalled, + transactionally coupled to the state change. +2. **Hydration reads all records** (ok + err) so cross-session + recall matches the in-session ring (req I2 is marked done but + only covers `ok` today — err lines are lost across sessions). +3. **Replay learns the journal format**: detect the + `||` prefix, extract+unescape ``, skip + non-`ok`; bare commands still replay as-is. Detection by the + leading timestamp+status prefix so a `|` inside a bare command + isn't misread. +4. Escape/unescape parity with the existing `history.rs` helpers. + +**Test-first (global rule):** the first move is a **failing** test +that replays an actual `history.log` (with the `|ok|` prefix) +and currently dies on line 1 — then make it green. Also add an +`err`-line round-trip test (write → hydrate-reads-it → +replay-skips-it). + +**Open question to settle with the user first:** ADR-0034 has **no +plan doc** (unlike Phases 2–3). It is smaller; a lightweight +phased pass in one session may suffice. The user said we can +"continue in this session afterwards with the ADR-0034 plan" — so +the immediate next action is to produce that plan (or confirm a +plan-less test-first pass), then implement. + +## §5. Other tracked deferred items (nothing lost) + +- **ADR-0006 auto-snapshot / undo** (U-series) — surfaced by 3k's + N/A matrix row; implement for **both** DSL and SQL worker + handlers so parity is real, not vacuous. +- **M4 — execution-time mode side-channel** (three-way `Mode` + threaded `Action`→worker, for mode-dependent *output*). Needs + its own ADR (ADR-0033 Amendment 3; `requirements.md` M4). Not a + prerequisite for anything above. +- **Phase 4 — DDL as SQL** (`CREATE`/`DROP`/`ALTER TABLE`, + `CREATE`/`DROP INDEX`). The next ADR-0030 roadmap phase, but + **no ADR exists yet** (latest is 0034) — needs its own ADR + before implementation. +- **`blob` value literal** — `src/dsl/value.rs` has no blob + literal yet; the 3k all-types test inserts NULL for blob (type + still round-trips). Pre-existing gap. +- **Q1 / Q2** (SQL subset completeness; polished out-of-subset + rejection message naming the construct) — multi-phase, still + `[ ]`; the 3k OOS tests assert rejection only, not message + quality (Q2 is "Implementation pending"). +- Cosmetic: `tests/sql_*.rs` helper names (`run_sqlinsert` etc.) + keep their dev-word-era spelling though they parse the real + words. + +## §6. Process pins (unchanged, still binding) + +- **Confirm every commit.** Propose the message; wait for the + go-ahead. (The 3k commit `380c423` and the acceptance flip were + both user-approved.) +- **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 catalog sub-agents + fabricated test names and mis-attributed dispatch/inherited- + diagnostic rows; every matrix row was re-verified by reading the + actual test. The `/runda` round caught a mode mismatch the same + way. Reproduce/print before concluding. +- **Escalate ADR-vs-implementation mismatches; never classify work + "out of scope" without user confirmation.** Both 3k findings + (I7, auto-snapshot) were escalated, not silently resolved. + +## §7. How to take over + +1. **Read, in order:** this file → + `docs/handoff/20260523-phase-3-verification.md` (the Phase-3 + close-out) → `docs/adr/0034-history-journal-and-replay-filter.md` + (**the next job**) → `CLAUDE.md` → `docs/requirements.md`. +2. **Baseline:** + ``` + cargo test # expect 1645 passing / 0 failing / 0 skipped / 1 ignored + cargo clippy --all-targets -- -D warnings # clean + ``` +3. **Start ADR-0034** per §4: settle plan-vs-no-plan with the + user, then a test-first pass beginning with the failing + `replay history.log` reproduction. +4. **Escalate** anything not settled in ADR-0034; the user wants + mismatches surfaced, not silently fixed.