Skip to content

feat(cli): add memory introspection commands#102

Merged
warren618 merged 4 commits into
HKUDS:mainfrom
Teerapat-Vatpitak:feat/memory-introspection-cli
May 13, 2026
Merged

feat(cli): add memory introspection commands#102
warren618 merged 4 commits into
HKUDS:mainfrom
Teerapat-Vatpitak:feat/memory-introspection-cli

Conversation

@Teerapat-Vatpitak

Copy link
Copy Markdown
Contributor

Summary

Adds vibe-trading memory list/show/search/forget so users can inspect
and manage persistent memory without opening ~/.vibe-trading/memory/
by hand. Surfaces real bug signal: two recent memory fixes (#87
snake_case, #95 CJK slugs) were hard to triage without observability
into what is actually saved and what the recall scorer returns.

Diff vs main: +516 / -5 across cli.py, src/memory/persistent.py,
and two test files. 81 tests pass. vibe-trading --version measured
at 1.0s (no regression on cold startup).

CLI surface

vibe-trading memory list   [--type user|feedback|project|reference]
vibe-trading memory show   <name>
vibe-trading memory search <query> [--limit N]
vibe-trading memory forget <name> [-y|--yes]

<name> resolves by exact title first, then by on-disk filename stem
({type}_{slug} or the bare slug), so users can paste either form
from the index.

Exit codes mirror existing commands:

scenario code
success EXIT_SUCCESS (0)
user declines confirm EXIT_SUCCESS (0)
not found / EOF stdin EXIT_USAGE_ERROR (2)
remove failed EXIT_RUN_FAILED (1)

Commits

The branch is split into four commits so each step is reviewable on
its own:

Commit Why
892cd16 feat(cli): add memory introspection commands the actual feature
f8de9d0 perf(cli): avoid loading agent runtime on cli startup hoisting PersistentMemory made --version 7.6s because src.agent.frontmatter transitively loads AgentLoop / SkillsLoader. Reverted to lazy imports inside each cmd_memory_* and a local _MEMORY_TYPES tuple in cli.py so non-memory commands stay fast
fe4fca6 style(cli): revert unintended formatter spillover on unrelated functions a local pre-tool-edit formatter reformatted ~30 unrelated functions in cli.py. This commit restores them to upstream form so the diff only shows memory-feature lines
672e2ba fix(memory): coerce frontmatter values to str in _scan_entries bug fix discovered during heavy E2E — see below

Changes

New (no impact on existing callers)

  • agent/cli.pymemory subparser; cmd_memory_list/show/search/forget;
    _MEMORY_TYPES tuple; _MEMORY_TYPE_STYLES (with module-load assert);
    rich.markup.escape on every user-supplied field; Confirm.ask
    wrapped against EOFError for non-interactive deletes
  • agent/src/memory/persistent.pyMEMORY_TYPES constant; module
    logger; public list_entries() / find(name) / remove_entry(entry).
    Existing remove(title) is unchanged so remember_tool is unaffected
  • agent/tests/test_cli_memory.py — 29 new tests (command logic,
    name resolution incl. true stem fallback, parser wiring, Rich markup
    escape, empty body rendering, EOF path, remove-failure path,
    bracketed-description regression)

One existing-code change: _scan_entries coercion

MemoryEntry annotates title, description, and memory_type as
str, but _scan_entries was passing the raw parse_frontmatter
output through untouched. The parser returns lists for [a, b]
syntax and bools for true/false, so any markdown file with e.g.
description: [red]inject[/red] leaked a list into the dataclass.
The previous CLI never called string ops on these fields, so the
mismatch was latent; the new CLI hits rich.markup.escape(...) and
crashes with TypeError.

Add a small _coerce_str helper and route the three frontmatter
fields through it so the dataclass contract is truthful at the storage
boundary. Existing consumers (find_relevant's tokenization,
remember_tool's rendering, remove(title)'s title comparison) all
silently improve on the edge case and behave identically on the happy
path. tests/test_remember_tool.py (16 tests) is unchanged and still
green.

Pre-existing limitations surfaced by the new CLI (out of scope)

Heavy E2E testing with Thai content surfaced two pre-existing bugs in
persistent.py that this PR intentionally does not fix:

  • _tokenize accepts only CJK Unified Ideographs (一-鿿);
    Thai / Arabic / Hebrew / Cyrillic queries always return zero
    matches because the tokenizer returns an empty set on those scripts
  • The slug regex in add() strips Thai (and other non-CJK scripts)
    to _, producing filenames like feedback______________.md for
    Thai titles

These are follow-ups of #95 and worth a separate issue/PR. The new
CLI happens to make them visible (memory show renders the Thai
title fine, but the on-disk slug is mangled).

Test Plan

  • pytest agent/tests/test_cli_memory.py agent/tests/test_persistent_memory.py agent/tests/test_remember_tool.py agent/tests/test_cli_init.py → 81 pass
  • ruff check agent/cli.py agent/src/memory/persistent.py agent/tests/test_cli_memory.py agent/tests/test_persistent_memory.py → no new errors (14 pre-existing E402/F541/F401 untouched)
  • time vibe-trading --version → 1.0s
  • File-level E2E on a populated ~/.vibe-trading/memory/: list (with and without --type filter), show by title / by full slug / by bare slug, search (Latin and CJK queries), forget with -y, with n decline, with closed stdin (EOF)
  • End-to-end with a real LLM: vibe-trading run → agent invokes remember tool → new entry visible in memory list, retrievable via memory show, matchable by memory search, removable via memory forget -y

Checklist

  • No changes to protected modules (src/agent/, src/session/, src/providers/)
  • No hardcoded values
  • Follows CONTRIBUTING.md
  • Tests added
  • No on-disk format changes; existing remember_tool workflows are byte-for-byte compatible

Add `vibe-trading memory list/show/search/forget` so users can inspect
and manage persistent memory without opening the file store directly.
Two recent memory bugs (#87 underscore tokenization, #95 CJK slug
collisions) were hard to spot without observability into what is
actually saved and recalled.

Also expand the PersistentMemory public surface to keep CLI lookup
logic out of storage internals:

- MEMORY_TYPES tuple as the single source of truth for type names,
  shared by argparse choices and the CLI style table
- list_entries() / find() / remove_entry() back the new commands;
  find() resolves by title first, then by on-disk filename stem so
  users can paste either form from the index
- remove_entry(entry) skips the re-scan that remove(title) does

Existing remove(title) is unchanged; remember_tool stays green.
Hoisting PersistentMemory to top-level cli.py made every CLI invocation
import `src.agent.frontmatter`, which transitively triggers
`src/agent/__init__.py` and eagerly loads AgentLoop, SkillsLoader (74
skills), and ToolRegistry. `vibe-trading --version` measured 7.6s
because of this chain.

- Move `from src.memory.persistent import PersistentMemory` back into
  each `cmd_memory_*` so only memory commands pay the import cost
- Mirror MEMORY_TYPES as a local `_MEMORY_TYPES` tuple inside cli.py so
  argparse `choices` and `_MEMORY_TYPE_STYLES` no longer require the
  module-level import. The module-load assertion still guards against
  drift between the local tuple and the style dict

`vibe-trading --version` drops from 7.6s to 0.66s (11.5x). Memory
commands keep the same latency since they need the module anyway.
The previous two commits inadvertently captured whole-file output from a
local pre-tool-edit formatter, reformatting ~30 functions
(`_print_status_bar`, `_format_tool_call_args`, `_build_benchmark_table`,
`_print_result`, `_build_welcome_panel`, ...) that have nothing to do
with the memory introspection feature.

Restore those functions to upstream `main` while keeping only the
intentional changes for this PR:

- `from datetime import datetime` hoisted to top imports (so the new
  CLI commands can use it without a per-call local import)
- `from rich.markup import escape as rich_escape` for safely rendering
  user-supplied memory titles/descriptions
- New `memory` subparser block in `_build_parser`
- New `cmd_memory_list / show / search / forget` plus the local
  `_MEMORY_TYPES` constant and `_MEMORY_TYPE_STYLES` dict
- Dispatch for `args.command == "memory"` in `main()`
- Drop the now-redundant local `from datetime import datetime` inside
  `cmd_trace`

Net diff for agent/cli.py shrinks from +296/-53 to +170/-1.
The frontmatter parser returns lists for ``[a, b]`` syntax and bools
for ``true``/``false``, but MemoryEntry annotates ``title``,
``description``, and ``memory_type`` as ``str``. The mismatch was latent
until the new memory CLI tried to render those fields through
``rich.markup.escape``, which raised ``TypeError: expected string or
bytes-like object, got 'list'`` for any description that looked like a
YAML list (e.g. ``description: [red]inject[/red]``).

Add a small ``_coerce_str`` helper and route the three frontmatter
fields through it in ``_scan_entries`` so the dataclass type contract is
truthful at the storage boundary. Other consumers (recall scoring,
``remember_tool``) also benefit transparently.

Tests:

- Unit coverage of ``_coerce_str`` for str/None/list/bool inputs
- Storage-layer regression: a markdown file with a bracketed description
  parses to a string-typed description
- CLI regression: ``cmd_memory_list`` no longer crashes on entries with
  a bracketed description
@warren618 warren618 merged commit 8a824fa into HKUDS:main May 13, 2026
1 check passed
@warren618

Copy link
Copy Markdown
Collaborator

Thanks @Teerapat-Vatpitak — this is a clean and useful addition. The memory CLI makes the persistent-memory layer much easier to inspect and debug, and the extra frontmatter coercion fix is a nice catch. Merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants