History brings back all commands in both modes #30

Closed
opened 2026-06-12 14:03:28 +01:00 by oli · 2 comments
Owner

Executing simple mode commands from history while in advanced mode is possible - no problem. But advanced modes can't be executed in simple mode - if we can distinguish them, we could enable reuse of advanced history commands in simple mode by prepending :.

Executing simple mode commands from history while in advanced mode is possible - no problem. But advanced modes can't be executed in simple mode - if we can distinguish them, we could enable reuse of advanced history commands in simple mode by prepending `:`.
Author
Owner

It could be that this is a bug. Observed this behavior:

  • in simple mode, run advanced command with : prefix
  • hit up arrow, command comes back WITH : and prompt changes to temporary advanced - correct
  • quit and resume
  • hit up arrow, now the same command comes back WITHOUT : and is therefore unusable without manual fixing.
It could be that this is a bug. Observed this behavior: - in simple mode, run advanced command with `:` prefix - hit up arrow, command comes back WITH `:` and prompt changes to temporary advanced - correct - quit and resume - hit up arrow, now the same command comes back WITHOUT `:` and is therefore unusable without manual fixing.
Collaborator

Implemented in 4aeea55ADR-0052 (mode-tagged history + top-of-chain journaling). Closes both parts: the feature (advanced history reusable in simple mode) and the bug in the comment (the : one-shot prefix lost across sessions).

Root cause

History carried no mode, and the in-memory ring stored the raw :select 1 while the worker journalled the stripped select 1 — so the : was lost on disk, and persistent-advanced commands couldn't be told from simple DSL.

Fix

  • Format: the history.log status token gains an optional :adv suffix (ok / ok:adv / err / err:adv). source stays last + canonical, so replay is unaffected (status_is_ok keys off the base token). Old logs read as simple — back-compatible.
  • In-memory: the ring (still Vec<String>) stores advanced entries in their : -prefixed simple-mode runnable form (a leading : unambiguously marks advanced, since simple DSL never starts with :). Recall strips the : in advanced mode (runs as bare SQL) and keeps it in simple (runs via the one-shot escape). Hydration reconstructs the prefix from the tag — so cross-session matches in-session.
  • App commands (undo, mode advanced, …) journal simple and submit excludes them from the ring's advanced flag, so they recall bare (no redundant :).

The architectural turn (your call)

The first draft kept journaling in the worker and threaded the mode down (~30-site plumbing). You asked the right question: why is the journal written deep in the worker when the failure path already journals at the top of the chain? It shouldn't — history.log is a journal of typed commands, not state. So success journaling moved to the dispatch layer (spawn_dsl_dispatch / run_replay / app-command sites), next to the failure path; finalize_persistence now writes only yaml/csv (commit-db-last still atomic for state); the journal write is best-effort (the command is already committed — consistent with the failure path).

DA findings (resolved)

  • Uniform journaling (accepted): the spawn journals every successful command, so show tables / show relationships / show relationship <name> / explain — which didn't journal before (a pre-existing inconsistency vs show table/show data/select) — now do, matching ADR-0034 §1. Harmless on replay (reads/explain don't mutate).
  • No double-journal (Ok→spawn, ErrJournalFailure are exclusive); replay itself still not journalled; undo/redo untouched (history.log isn't in the snapshot).

Amends

ADR-0034 (status tag + journaling location), ADR-0015 §6 (history.log out of the worker tx), ADR-0040 (journal-write best-effort, not fatal).

Tests

The 15 worker-level journaling tests retired (worker no longer journals — yaml/csv/operation checks kept) and re-covered at the new layer: history.rs status-tag + :-reconstruct, app.rs recall matrix (one-shot / persistent-advanced / simple / app-command), the cross-session regression in iteration6, and the replay tests for run_replay journaling. 2471 pass / 0 fail / 0 skip (1 ignored), clippy clean. Two /runda passes (design + implementation).

Follow-up (noted in ADR-0052): the now-vestigial worker source plumbing (_source params + thin *_request wrappers) can be unwound in a clean follow-up.

Implemented in `4aeea55` — **ADR-0052** (mode-tagged history + top-of-chain journaling). Closes both parts: the feature (advanced history reusable in simple mode) and the bug in the comment (the `:` one-shot prefix lost across sessions). ## Root cause History carried no mode, and the in-memory ring stored the raw `:select 1` while the worker journalled the *stripped* `select 1` — so the `:` was lost on disk, and persistent-advanced commands couldn't be told from simple DSL. ## Fix - **Format:** the `history.log` status token gains an optional `:adv` suffix (`ok` / `ok:adv` / `err` / `err:adv`). `source` stays last + canonical, so replay is unaffected (`status_is_ok` keys off the base token). Old logs read as simple — back-compatible. - **In-memory:** the ring (still `Vec<String>`) stores advanced entries in their `: `-prefixed simple-mode runnable form (a leading `:` unambiguously marks advanced, since simple DSL never starts with `:`). Recall **strips the `:` in advanced mode** (runs as bare SQL) and keeps it in simple (runs via the one-shot escape). Hydration reconstructs the prefix from the tag — so cross-session matches in-session. - **App commands** (`undo`, `mode advanced`, …) journal simple and `submit` excludes them from the ring's advanced flag, so they recall bare (no redundant `:`). ## The architectural turn (your call) The first draft kept journaling in the worker and threaded the mode down (~30-site plumbing). You asked the right question: why is the journal written deep in the worker when the *failure* path already journals at the top of the chain? It shouldn't — `history.log` is a journal of typed commands, not state. So **success journaling moved to the dispatch layer** (`spawn_dsl_dispatch` / `run_replay` / app-command sites), next to the failure path; `finalize_persistence` now writes only yaml/csv (commit-db-last still atomic for state); the journal write is **best-effort** (the command is already committed — consistent with the failure path). ## DA findings (resolved) - **Uniform journaling** (accepted): the spawn journals every successful command, so `show tables` / `show relationships` / `show relationship <name>` / `explain` — which didn't journal before (a pre-existing inconsistency vs `show table`/`show data`/`select`) — now do, matching ADR-0034 §1. Harmless on replay (reads/explain don't mutate). - No double-journal (`Ok`→spawn, `Err`→`JournalFailure` are exclusive); `replay` itself still not journalled; undo/redo untouched (history.log isn't in the snapshot). ## Amends ADR-0034 (status tag + journaling location), ADR-0015 §6 (history.log out of the worker tx), ADR-0040 (journal-write best-effort, not fatal). ## Tests The 15 worker-level journaling tests retired (worker no longer journals — yaml/csv/operation checks kept) and re-covered at the new layer: history.rs status-tag + `:`-reconstruct, app.rs recall matrix (one-shot / persistent-advanced / simple / app-command), the cross-session regression in `iteration6`, and the replay tests for `run_replay` journaling. **2471 pass / 0 fail / 0 skip (1 ignored), clippy clean.** Two `/runda` passes (design + implementation). **Follow-up (noted in ADR-0052):** the now-vestigial worker `source` plumbing (`_source` params + thin `*_request` wrappers) can be unwound in a clean follow-up.
Sign in to join this conversation.