Repository navigation
feat: record history meta-command metadata from the REPL's dispatch result - #312
Merged
Merged
Conversation
Command history entries now carry structured metadata in the SQLite `more_info` column. `FuzzyHistory::save` goes through reedline 0.50's typed `SqliteBackedHistory::save_with_extra`, storing a `HistoryExtraInfo` whose `meta_command` flag marks lines the REPL treats as meta commands (`:cd`, `:help`, ...). Meta-command detection mirrors `process_meta_command`: a line qualifies when its leading-whitespace-trimmed form starts with `:`. Reading the flag back and offering an option to exclude meta commands from Ctrl+R and the history browser is left to a follow-up; nothing in the REPL behaves differently yet. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Command history metadata was decided lexically at save time: a line was flagged as a meta command when its text began with `:`. That agreed with the REPL only by coincidence, and it mis-flagged two paths that write history without dispatching anything -- input typed at a menu prompt (`readline()`, `menu()`, `browser()`) and code injected over IPC. It also could not survive user-defined commands, where which `:` lines are meta commands depends on runtime configuration. Metadata now records what the REPL actually dispatched. Interactive submissions are inserted with `more_info` as SQL NULL, and the row is finalized once the routing is known. A failed finalization leaves NULL, which honestly means "arf does not know"; it never falls back to writing `false`. To reach reedline's typed metadata API, arf now owns the database through `HistoryStore` and hands reedline an adapter that delegates to it. The adapter records each save in a receipt, so the row being evaluated is tracked by explicit ID rather than through reedline's internal last-run ID. That fixes a bug: reedline overwrites that ID on every submission, including at a menu prompt, so a command that prompted for input recorded its exit status against the answer rather than against itself. The adapter also stops propagating save errors, which reedline turns into a panic, and `HistoryExtraInfo` now retains the whole JSON object so that fields written by a newer arf survive a typed update instead of being dropped. History import still ignores metadata; that is handled separately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reworking the history layer routed IPC-injected code exclusively through the SQLite store, so it was dropped entirely when no store exists -- under `--no-history`, or when the database fails to open. Ordinary typed input still reaches the in-memory backend reedline installs by default in that case, so injected commands silently fell out of in-session recall while typed ones stayed. Fall back to that backend when there is no store. The branches are exclusive: with a store the adapter is already installed on the editor, and saving through it as well would write the row to SQLite twice. The fallback returns no ID. The in-memory backend hands back a deque index that shifts as entries are evicted, refuses `update`, and returns nothing when an entry repeats the previous one, so it cannot address a row for the later exit-status write. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Refactors history persistence so metadata reflects actual REPL dispatch while preserving accurate command status attribution.
Changes:
- Introduces an arf-owned SQLite history store and reedline adapter.
- Records dispatch-based meta-command metadata and preserves unknown JSON fields.
- Makes history-save failures non-fatal and tracks command rows explicitly.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
CHANGELOG.md |
Documents history behavior and fixes. |
crates/arf-console/src/app/headless.rs |
Uses the owned history store. |
crates/arf-console/src/history/metadata.rs |
Adds lossless typed metadata. |
crates/arf-console/src/history/mod.rs |
Exposes the new history components. |
crates/arf-console/src/history/reedline_adapter.rs |
Adapts the store for reedline and fuzzy search. |
crates/arf-console/src/history/search.rs |
Removes the superseded search wrapper. |
crates/arf-console/src/history/storage.rs |
Removes the superseded metadata model. |
crates/arf-console/src/history/store.rs |
Implements synchronized SQLite storage. |
crates/arf-console/src/ipc/mod.rs |
Persists headless history through the store. |
crates/arf-console/src/repl/history.rs |
Adds setup, IPC-save, and finalization helpers. |
crates/arf-console/src/repl/mod.rs |
Integrates history handles across REPL modes. |
crates/arf-console/src/repl/read_console.rs |
Finalizes metadata from routing results. |
crates/arf-console/src/repl/state.rs |
Tracks stores and explicit pending row IDs. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The receipt held whichever row the adapter saved last, not the row for the submission being routed. reedline returns a successful signal without calling `History::save` in two cases -- an empty buffer, and a buffer matching a configured history exclusion prefix, which is diverted to an excluded-item slot instead -- and in either case the stale value would have finalized the previous command's row. Reading the receipt now removes the value, so a submission that saved nothing yields nothing. Each successful-signal arm takes it once, before any branching, and passes the result to both consumers: the metadata finalization and the pending context that later records the exit status. This removes the emptiness check that guarded the first case by replicating reedline's own save condition inside arf. Duplicating an invariant owned by another crate was the wrong shape for the fix, and it never covered the exclusion case at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
3 of 5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Command history now records whether an entry was a meta command, taken from what
the REPL actually dispatched rather than from the text of the line.
Why this is more than storing a flag
The obvious implementation — test for a leading
:when saving — is wrong. Itagrees with the REPL only by coincidence; it mis-flags the two paths that write
history without dispatching anything, namely input typed at a
readline()/menu()/browser()prompt and code injected over IPC; and it cannot surviveuser-defined commands as requested in #310, where which
:lines are metacommands depends on runtime configuration.
So the write layer was redesigned rather than patched:
HistoryStoreand hands reedline anadapter that delegates to it. That is what makes reedline's typed metadata API
reachable — the
Historytrait deals in type-erased items and cannot roundtrip
more_info.more_infoas SQL NULL andfinalized once routing is known. A failed finalization leaves NULL, which
honestly means "arf does not know"; it never falls back to writing
false.reedline returns a successful signal without saving in two cases — an empty
buffer, and a buffer matching a history exclusion prefix — so a receipt that
could be read twice would attribute one command's row to the next.
HistoryExtraInfoalso retains the whole JSON object, so fields written by anewer version of arf survive a typed update instead of being dropped. Every
typed update is a deserialize/serialize round trip, not just import.
Also fixed
answer typed at that prompt instead of against the command itself. reedline
overwrites its internal last-run ID on every submission, including at such a
prompt, so the row being evaluated is now tracked by explicit ID.
turns an error returned from
History::saveinto a panic, so the adapterabsorbs it and keeps the session alive.
Not in this PR
History import still ignores metadata. Carrying it across import, degrading
malformed rows individually, and backfilling duplicates are tracked separately,
as are several things this work turned up:
--no-historycontract — its help text promises more than theimplementation delivers
addition to reedline
:history clearbypassing the owned store to open its own connectionTest plan
cargo fmt --all --checkcargo clippy --workspace --all-targets -- -D warningscargo test -p arf-console— 927 unit tests plus the integration and PTYsuites
shell and standalone routing, receipt consumption, unknown-field
preservation, save failure, and exit-status attribution after a menu
prompt