feat(history): mode-tagged history + top-of-chain journaling (#30)
Record the submission mode per history entry so advanced commands are reusable in simple mode, and fix the bug where a ':'-one-shot command lost its ':' across sessions (ADR-0052, closing #30). Format: the history.log status token gains an optional ':adv' suffix (ok / ok:adv / err / err:adv); 'source' stays last and canonical, so replay is unaffected. The in-memory ring (still Vec<String>) stores advanced entries ': '-prefixed; recall strips the ':' in advanced mode and keeps it in simple; hydration reconstructs the prefix from the tag. Journaling moved from the worker to the dispatch layer (spawn_dsl_- dispatch / run_replay / app-command sites), where the mode is in scope with no worker plumbing; finalize_persistence writes only yaml/csv (commit-db-last still atomic for state). The journal write is now best-effort (command already committed), consistent with the failure path. App commands journal simple, so they recall bare. Journaling is now uniform (every successful command, per ADR-0034) — closing a gap where show tables/relationships/explain didn't journal. Amends ADR-0034 (status tag + journaling location), ADR-0015 §6 (history.log out of the worker tx), ADR-0040 (journal-write best-effort). 15 worker-level journaling tests retired, re-covered at the new layer (history.rs format, app.rs recall matrix, iteration6 cross-session regression, replay). 2471 pass / 0 fail / 0 skip, clippy clean.
This commit is contained in:
+137
-20
@@ -874,13 +874,16 @@ impl App {
|
||||
error,
|
||||
facts,
|
||||
source,
|
||||
advanced,
|
||||
} => {
|
||||
self.handle_dsl_failure(&command, error, facts);
|
||||
// ADR-0034 §1/§2: an execution failure is journalled
|
||||
// `err` so it is recallable across sessions (the
|
||||
// worker only journals successful commands). The App
|
||||
// emits the intent; the runtime does the append.
|
||||
vec![Action::JournalFailure { source }]
|
||||
// emits the intent; the runtime does the append. The
|
||||
// mode rides along (ADR-0052) so an advanced failure
|
||||
// tags `err:adv`.
|
||||
vec![Action::JournalFailure { source, advanced }]
|
||||
}
|
||||
AppEvent::TablesRefreshed(tables) => {
|
||||
trace!(count = tables.len(), "tables refreshed");
|
||||
@@ -1648,11 +1651,27 @@ impl App {
|
||||
Some(i) => i - 1,
|
||||
};
|
||||
self.history_cursor = Some(next_index);
|
||||
self.input = self.history[next_index].clone();
|
||||
let stored = self.history[next_index].clone();
|
||||
self.input = self.recall_display(&stored);
|
||||
self.input_cursor = self.input.len();
|
||||
self.input_scroll_offset = 0;
|
||||
}
|
||||
|
||||
/// The display form of a stored history entry for the current mode
|
||||
/// (ADR-0052, issue #30). An advanced entry is stored in its
|
||||
/// `:`-prefixed simple-mode runnable form; in **advanced** mode the
|
||||
/// `:` is stripped so it runs as bare SQL, while in **simple** mode it
|
||||
/// stays prefixed and runs via the one-shot escape. A simple entry
|
||||
/// (never starting with `:`) is returned unchanged in either mode.
|
||||
fn recall_display(&self, stored: &str) -> String {
|
||||
if self.mode == Mode::Advanced
|
||||
&& let Some(rest) = stored.strip_prefix(':')
|
||||
{
|
||||
return rest.trim_start().to_string();
|
||||
}
|
||||
stored.to_string()
|
||||
}
|
||||
|
||||
/// Move forwards in history (towards newer entries; eventually
|
||||
/// returning to the user's saved draft).
|
||||
fn history_forward(&mut self) {
|
||||
@@ -1661,7 +1680,8 @@ impl App {
|
||||
};
|
||||
if i + 1 < self.history.len() {
|
||||
self.history_cursor = Some(i + 1);
|
||||
self.input = self.history[i + 1].clone();
|
||||
let stored = self.history[i + 1].clone();
|
||||
self.input = self.recall_display(&stored);
|
||||
} else {
|
||||
// Past the most recent entry — restore the draft and
|
||||
// exit navigation mode.
|
||||
@@ -1709,10 +1729,6 @@ impl App {
|
||||
if trimmed.is_empty() {
|
||||
return Vec::new();
|
||||
}
|
||||
// Record the original (trimmed) line in history regardless
|
||||
// of whether it parses, so users can recall and edit
|
||||
// typo'd commands.
|
||||
self.push_history(trimmed);
|
||||
|
||||
// `:` one-shot escape: in simple mode, a leading `:` means
|
||||
// treat *this single submission* as advanced. The persistent
|
||||
@@ -1729,6 +1745,9 @@ impl App {
|
||||
};
|
||||
|
||||
if effective_input.is_empty() {
|
||||
// A bare `:` (one-shot with nothing after it) executes
|
||||
// nothing and is not recorded — the push moved below the
|
||||
// strip (ADR-0052), so it no longer lands in history.
|
||||
return Vec::new();
|
||||
}
|
||||
|
||||
@@ -1739,16 +1758,31 @@ impl App {
|
||||
"submit"
|
||||
);
|
||||
|
||||
// Parse-first: app-level commands and DSL commands now
|
||||
// share the chumsky parser (per the round-5 refactor).
|
||||
// App commands work in both modes — they're not gated by
|
||||
// `effective_mode`. Anything that parses to a non-App
|
||||
// variant falls through to the existing mode-specific
|
||||
// path: simple → DSL execution; advanced → SQL placeholder.
|
||||
// Anything that fails to parse falls through too — the
|
||||
// simple-mode path renders the friendly parse error, the
|
||||
// advanced-mode path renders the SQL placeholder.
|
||||
if let Ok(Command::App(app_cmd)) = parse_command(&effective_input) {
|
||||
// Parse-first: app-level commands and DSL commands share the
|
||||
// parser. App commands work in both modes — they're not gated by
|
||||
// `effective_mode`. Anything that parses to a non-App variant (or
|
||||
// fails to parse) falls through to the mode-specific path.
|
||||
let parsed = parse_command(&effective_input);
|
||||
|
||||
// ADR-0052 (issue #30): record the command for cross-mode recall.
|
||||
// An **advanced** (SQL) command is stored in its `:`-prefixed
|
||||
// simple-mode runnable form, so it can be recalled and re-run in
|
||||
// simple mode (recall strips the `:` again in advanced mode). A
|
||||
// simple command — and **any app command**, which runs in either
|
||||
// mode and so must not gain a `:` — is stored bare. Recorded
|
||||
// regardless of whether it parses, so typo'd commands stay
|
||||
// recallable. The canonical (un-prefixed) text is what reaches
|
||||
// the journal via `ExecuteDsl.source`.
|
||||
let is_app = matches!(&parsed, Ok(Command::App(_)));
|
||||
let advanced = submission_mode.is_advanced() && !is_app;
|
||||
let ring_line = if advanced {
|
||||
format!(": {effective_input}")
|
||||
} else {
|
||||
effective_input.clone()
|
||||
};
|
||||
self.push_history(&ring_line);
|
||||
|
||||
if let Ok(Command::App(app_cmd)) = parsed {
|
||||
return self.dispatch_app_command(app_cmd, &effective_input);
|
||||
}
|
||||
|
||||
@@ -1961,6 +1995,7 @@ impl App {
|
||||
self.note_error(note);
|
||||
return vec![Action::JournalFailure {
|
||||
source: input.to_string(),
|
||||
advanced: submission_mode.is_advanced(),
|
||||
}];
|
||||
}
|
||||
// Issue #17: simple-mode (DSL) counterpart. A wrong-count
|
||||
@@ -1988,6 +2023,7 @@ impl App {
|
||||
self.note_error(render_usage_block(input, mode));
|
||||
return vec![Action::JournalFailure {
|
||||
source: input.to_string(),
|
||||
advanced: submission_mode.is_advanced(),
|
||||
}];
|
||||
}
|
||||
self.push_output(OutputLine::echo(input, mode));
|
||||
@@ -2074,6 +2110,7 @@ impl App {
|
||||
// append; the App only emits the intent.
|
||||
vec![Action::JournalFailure {
|
||||
source: input.to_string(),
|
||||
advanced: submission_mode.is_advanced(),
|
||||
}]
|
||||
}
|
||||
}
|
||||
@@ -5493,6 +5530,7 @@ mod tests {
|
||||
},
|
||||
facts: crate::friendly::FailureContext::default(),
|
||||
source: String::new(),
|
||||
advanced: false,
|
||||
});
|
||||
let last = app.output.back().unwrap();
|
||||
assert_eq!(last.kind, OutputKind::Error);
|
||||
@@ -5551,6 +5589,7 @@ mod tests {
|
||||
error: err,
|
||||
facts,
|
||||
source: String::new(),
|
||||
advanced: false,
|
||||
});
|
||||
let body = app
|
||||
.output
|
||||
@@ -5600,6 +5639,7 @@ mod tests {
|
||||
error: err,
|
||||
facts,
|
||||
source: String::new(),
|
||||
advanced: false,
|
||||
});
|
||||
let body = app
|
||||
.output
|
||||
@@ -5632,6 +5672,7 @@ mod tests {
|
||||
error: err(),
|
||||
facts: crate::friendly::FailureContext::default(),
|
||||
source: String::new(),
|
||||
advanced: false,
|
||||
});
|
||||
let verbose_text = app
|
||||
.output
|
||||
@@ -5652,6 +5693,7 @@ mod tests {
|
||||
error: err(),
|
||||
facts: crate::friendly::FailureContext::default(),
|
||||
source: String::new(),
|
||||
advanced: false,
|
||||
});
|
||||
let short_text = app
|
||||
.output
|
||||
@@ -6327,7 +6369,7 @@ mod tests {
|
||||
assert!(
|
||||
matches!(
|
||||
actions.as_slice(),
|
||||
[Action::JournalFailure { source }] if source == "florp glorp"
|
||||
[Action::JournalFailure { source, .. }] if source == "florp glorp"
|
||||
),
|
||||
"expected JournalFailure for the typo'd line; got {actions:?}",
|
||||
);
|
||||
@@ -6350,11 +6392,12 @@ mod tests {
|
||||
},
|
||||
facts: crate::friendly::FailureContext::default(),
|
||||
source: "drop table Ghost".to_string(),
|
||||
advanced: false,
|
||||
});
|
||||
assert!(
|
||||
matches!(
|
||||
actions.as_slice(),
|
||||
[Action::JournalFailure { source }] if source == "drop table Ghost"
|
||||
[Action::JournalFailure { source, .. }] if source == "drop table Ghost"
|
||||
),
|
||||
"expected JournalFailure carrying the source; got {actions:?}",
|
||||
);
|
||||
@@ -6483,6 +6526,80 @@ mod tests {
|
||||
assert_eq!(app.input, "drop table AX");
|
||||
}
|
||||
|
||||
// ---- ADR-0052 (issue #30): mode-aware history recall ----
|
||||
|
||||
#[test]
|
||||
fn one_shot_advanced_command_recalls_with_colon_in_simple_mode() {
|
||||
// The bug: a `:`-one-shot advanced command must recall WITH the
|
||||
// `:` so it re-runs in simple mode (in-session and, via the
|
||||
// `:`-prefixed ring form, across sessions too).
|
||||
let mut app = App::new();
|
||||
type_str(&mut app, ": select 1");
|
||||
submit(&mut app);
|
||||
app.update(key(KeyCode::Up));
|
||||
assert_eq!(app.input, ": select 1");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn persistent_advanced_command_recalls_with_colon_back_in_simple_mode() {
|
||||
// The feature: a command typed in *persistent* advanced mode
|
||||
// recalls into simple mode with a `:` so it stays runnable.
|
||||
let mut app = App::new();
|
||||
app.mode = Mode::Advanced;
|
||||
type_str(&mut app, "select 1");
|
||||
submit(&mut app);
|
||||
// Switch back to simple and recall.
|
||||
app.mode = Mode::Simple;
|
||||
app.update(key(KeyCode::Up));
|
||||
assert_eq!(app.input, ": select 1");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn advanced_command_recalls_bare_in_advanced_mode() {
|
||||
// In advanced mode the stored `:`-prefix is stripped so it runs
|
||||
// as bare SQL.
|
||||
let mut app = App::new();
|
||||
app.mode = Mode::Advanced;
|
||||
type_str(&mut app, "select 1");
|
||||
submit(&mut app);
|
||||
app.update(key(KeyCode::Up));
|
||||
assert_eq!(app.input, "select 1");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn simple_command_recalls_bare_in_either_mode() {
|
||||
let mut app = App::new();
|
||||
type_str(&mut app, "drop table T");
|
||||
submit(&mut app);
|
||||
app.update(key(KeyCode::Up));
|
||||
assert_eq!(app.input, "drop table T");
|
||||
app.mode = Mode::Advanced;
|
||||
app.update(key(KeyCode::Down)); // back to draft
|
||||
app.update(key(KeyCode::Up));
|
||||
assert_eq!(app.input, "drop table T");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn app_command_recalls_bare_even_when_typed_with_colon() {
|
||||
// An app command runs in any mode, so it must NOT gain a `:` on
|
||||
// recall even when entered via the one-shot escape.
|
||||
let mut app = App::new();
|
||||
type_str(&mut app, ": mode advanced");
|
||||
submit(&mut app);
|
||||
app.update(key(KeyCode::Up));
|
||||
assert_eq!(app.input, "mode advanced");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_bare_colon_is_not_recorded_in_history() {
|
||||
let mut app = App::new();
|
||||
type_str(&mut app, ":");
|
||||
submit(&mut app);
|
||||
// Nothing recallable.
|
||||
app.update(key(KeyCode::Up));
|
||||
assert_eq!(app.input, "");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn add_column_with_text_type_emits_execute_action() {
|
||||
let mut app = App::new();
|
||||
|
||||
Reference in New Issue
Block a user