feat(cli): add 'gradata forget' subcommand (GRA-1207 / GH #206)#209
Conversation
Wraps Brain.forget() in a CLI surface with interactive confirmation. Lets users undo lessons without writing Python. Selector syntax: gradata forget # most recent active lesson gradata forget last 3 # last 3 active lessons gradata forget all TONE # every active lesson in a category gradata forget casual # fuzzy description substring match gradata forget last --yes # skip confirmation (scripts) Preview-before-apply: shows matched lessons + asks 'Proceed? [y/N]'. Pass --yes / -y to skip for scripts. Soft-delete semantics: - Lessons flipped to KILLED state (not hard-deleted) - LESSON_CHANGE event emitted with action='rolled_back' and kill_reason='manual_forget' - Rule cache invalidated so next apply_brain_rules() reflects the change - Sync pipeline replays the events to cloud automatically Replaces plugin /gradata:forget slash command (GRA-1198 epic). Test plan: pytest tests/test_forget_command.py => 6 passed in 3.29s 6 cases: last (single), last N, all CATEGORY, fuzzy description match, no-match clean error, unknown-category clean error. All seed via the public Brain.correct() API so the events table matches production exactly — no hand-crafted SQL inserts. Layering: cmd_forget delegates to Brain.forget() (Layer 2 public API). Layer 0 imports (LessonState, parse_lessons) are used for preview only, not for state mutation. No Layer 0 -> 2 imports.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 Walkthrough
WalkthroughThis PR introduces a ChangesForget Command Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.21.0)OpenGrep fatal error (exit code 2): �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Gradata/src/gradata/cli.py`:
- Around line 592-595: The "last N" branch in the CLI parsing (where wl ==
"last" or wl.startswith("last ")) currently parses n with int(parts[1]) but
allows 0 which makes active[-0:] return all entries; update the validation in
this branch (and the similar branch around the other occurrence referenced) to
reject non-positive values: parse parts[1], ensure it is a digit and that
int(parts[1]) >= 1, and if not raise/print a clear CLI error or return without
performing preview/apply instead of using n=1; modify the code around the wl
handling and the variable n (and the subsequent use of preview = [l for _, l in
active[-n:]]) to only execute when n >= 1.
In `@Gradata/tests/test_forget_command.py`:
- Around line 98-99: The test unpacks "brain, brain_dir = seeded_brain" but the
"brain" variable is never used (RUF059); update the test(s) that call
seeded_brain (the occurrences around the _run_forget calls) to only unpack the
needed value (e.g., "_, brain_dir = seeded_brain" or "brain_dir =
seeded_brain[1]") or change the assignment to a single-variable unpacking so
that only brain_dir is assigned where _run_forget is invoked; adjust both
occurrences referenced (around lines with _run_forget(brain_dir,
"nonexistent-phrase-zzz", ...)) accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 29524b2a-e068-4ab3-922c-7e761cc55d04
📒 Files selected for processing (2)
Gradata/src/gradata/cli.pyGradata/tests/test_forget_command.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: pytest windows-latest / py3.12
- GitHub Check: pytest macos-latest / py3.11
- GitHub Check: pytest ubuntu-latest / py3.11
- GitHub Check: pytest ubuntu-latest / py3.12
- GitHub Check: pytest windows-latest / py3.11
- GitHub Check: pytest macos-latest / py3.12
- GitHub Check: pytest (py3.11)
- GitHub Check: pytest (py3.12)
🧰 Additional context used
📓 Path-based instructions (2)
Gradata/tests/**/*.py
📄 CodeRabbit inference engine (Gradata/AGENTS.md)
Gradata/tests/**/*.py: SetBRAIN_DIRenvironment variable viatmp_pathin conftest.py for test isolation — ensure_paths.pymodule cache refreshes when callingBrain.init()directly inside tests
Add unit tests intests/test_*.pyfor every CI push without LLM calls (deterministic); mark integration tests with@pytest.mark.integrationand skip them by default (they hit real LLM APIs)
Files:
Gradata/tests/test_forget_command.py
Gradata/src/**/*.py
📄 CodeRabbit inference engine (Gradata/AGENTS.md)
Gradata/src/**/*.py: Prefersentence-transformersfor local embeddings,google-genaifor Gemini embeddings,cryptographyfor AES-GCM encrypted system.db,bm25sfor BM25 rule ranking, andmem0aifor external memory adapters — guard all optional dependency imports withtry / except ImportErrorat the call site, never at module level
Maintain strict layering: Layer 0 (Primitives: _types.py, _db.py, _events.py, _paths.py, _file_lock.py; Patterns: contrib/patterns/) must never import from Layer 1 (Enhancements: enhancements/, rules/) or Layer 2 (Public API: brain.py, cli.py, daemon.py, mcp_server.py)
Never use bareexcept: pass— use typed exceptions or at minimumlogger.warning(...)withexc_info=Trueto avoid silent failure in a memory product
Never import from out-of-scope sibling directories../Sprites/or../Hausgem/withingradata/*code — that is a layering bug
Never leak private-sibling paths into public docs/code — no references to../Sprites/,../Hausgem/, email addresses, OneDrive paths, or Sprites-specific examples from insidegradata/*
Use atomic-write helper when writing JSON files to prevent corruption from mid-write crashes
Files:
Gradata/src/gradata/cli.py
🪛 Ruff (0.15.13)
Gradata/tests/test_forget_command.py
[warning] 98-98: Unpacked variable brain is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
[warning] 104-104: Unpacked variable brain is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
🔇 Additional comments (3)
Gradata/src/gradata/cli.py (2)
340-341: LGTM!Also applies to: 353-357
1669-1686: LGTM!Also applies to: 1878-1878
Gradata/tests/test_forget_command.py (1)
16-97: LGTM!Also applies to: 106-124
| if wl == "last" or wl.startswith("last "): | ||
| parts = wl.split() | ||
| n = int(parts[1]) if len(parts) == 2 and parts[1].isdigit() else 1 | ||
| preview = [l for _, l in active[-n:]] |
There was a problem hiding this comment.
Reject invalid last N values before preview/apply.
last 0 currently resolves to active[-0:] (all active lessons), which can cause unintended mass rollback.
Suggested fix
wl = what.lower()
+ normalized_what = what
preview: list = []
if wl == "last" or wl.startswith("last "):
parts = wl.split()
- n = int(parts[1]) if len(parts) == 2 and parts[1].isdigit() else 1
+ if len(parts) == 1:
+ n = 1
+ elif len(parts) == 2 and parts[1].isdigit() and int(parts[1]) >= 1:
+ n = int(parts[1])
+ else:
+ print("Error: selector must be 'last' or 'last N' where N >= 1")
+ sys.exit(2)
+ normalized_what = "last" if n == 1 else f"last {n}"
preview = [l for _, l in active[-n:]]
@@
- result = brain.forget(what)
+ result = brain.forget(normalized_what)Also applies to: 627-627
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Gradata/src/gradata/cli.py` around lines 592 - 595, The "last N" branch in
the CLI parsing (where wl == "last" or wl.startswith("last ")) currently parses
n with int(parts[1]) but allows 0 which makes active[-0:] return all entries;
update the validation in this branch (and the similar branch around the other
occurrence referenced) to reject non-positive values: parse parts[1], ensure it
is a digit and that int(parts[1]) >= 1, and if not raise/print a clear CLI error
or return without performing preview/apply instead of using n=1; modify the code
around the wl handling and the variable n (and the subsequent use of preview =
[l for _, l in active[-n:]]) to only execute when n >= 1.
| brain, brain_dir = seeded_brain | ||
| out = _run_forget(brain_dir, "nonexistent-phrase-zzz", yes=True, capsys=capsys) |
There was a problem hiding this comment.
Remove unused brain unpacking in no-match tests.
These unpacked variables are unused and trigger Ruff RUF059.
Suggested fix
def test_forget_no_matches_returns_clean_error(seeded_brain, capsys):
- brain, brain_dir = seeded_brain
+ _, brain_dir = seeded_brain
out = _run_forget(brain_dir, "nonexistent-phrase-zzz", yes=True, capsys=capsys)
assert "No active lessons match" in out or "No matches" in out
def test_forget_unknown_category_says_no_matches(seeded_brain, capsys):
- brain, brain_dir = seeded_brain
+ _, brain_dir = seeded_brain
out = _run_forget(brain_dir, "all NONEXISTENT", yes=True, capsys=capsys)
assert "No matches" in out or "No active lessons in" in outAlso applies to: 104-105
🧰 Tools
🪛 Ruff (0.15.13)
[warning] 98-98: Unpacked variable brain is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Gradata/tests/test_forget_command.py` around lines 98 - 99, The test unpacks
"brain, brain_dir = seeded_brain" but the "brain" variable is never used
(RUF059); update the test(s) that call seeded_brain (the occurrences around the
_run_forget calls) to only unpack the needed value (e.g., "_, brain_dir =
seeded_brain" or "brain_dir = seeded_brain[1]") or change the assignment to a
single-variable unpacking so that only brain_dir is assigned where _run_forget
is invoked; adjust both occurrences referenced (around lines with
_run_forget(brain_dir, "nonexistent-phrase-zzz", ...)) accordingly.
Resolves conflicts in cli.py between cmd_prove (this branch) and cmd_forget (PR #209 already merged to main). Both commands kept side-by-side. All 17 tests pass (status + forget + prove).
…RA-1198) (#211) Replaces the two-path 'Claude Code via /plugin marketplace OR Python SDK via pipx' branching with a single canonical install: pipx + gradata install --agent <host>. Why: the plugin marketplace path was a duplicate surface that did the same thing the SDK install command already does (apply hooks + slash commands to the host config). Two paths created onboarding friction ('which one am I supposed to use?') for zero functional gain. Council voted Option A 'kill the plugin' on 2026-05-01. Also surfaces the 6 first-class subcommands the SDK now ships: status, correct, forget, prove, recall, doctor. Three of those were shipped earlier today (PR #208 status, #209 forget, #210 prove) and replaced the equivalent plugin slash commands. Removed: - '/plugin marketplace add Gradata/gradata' + '/plugin install gradata' - The 'pick one' framing - .claude-plugin/ from the repo layout (manifest stays in tree until PR retiring the directory ships separately — keeps the layout description accurate as of THIS commit) Parent: GRA-1198 (kill the plugin epic) GH: Gradata/gradata #206 Co-authored-by: data-engineer <data-engineer@gradata.ai>
…-1198) (#214) The .claude-plugin/ directory itself was already removed in a prior cleanup (see CHANGELOG: 'Remove orphaned gradata-plugin/ subdir (#54)'). What remained were stale string references in docs and examples now that the SDK ships all subcommands directly (PRs #208/#209/#210/#211 + #213). Changes: - .dockerignore: removed dead .claude-plugin exclude line - examples/with_claude_code.py: replaced '/plugin install gradata' language with the canonical 'gradata install --agent claude-code' - examples/README.md: fix broken link to .claude-plugin/README.md - CHANGELOG.md: BREAKING entry under Unreleased documenting the retirement This closes out the kill-the-plugin epic (GRA-1198 / GH #206) from the references side. Anyone who installed via /plugin marketplace before 2026-05-20 must migrate to the SDK install path. Verified: - pip install /home/olive/work/gradata-sdk/Gradata in a fresh venv succeeds - gradata install --agent claude-code --brain /tmp/test-brain --help works - pytest tests/ -x -q passes (816 tests, 7 skipped, 1 known-skip on test_byo_key_provider for missing httpx in dev env unrelated to this) - ruff check clean on touched files - grep for 'claude-plugin|gradata-plugin' on src/ + docs/ shows only the intentional CHANGELOG entries (current BREAKING note + historical refs) Branch authored by delegate_task subagent (hit max_iterations on PR-open); parent agent verified + extracted clean diff + opened PR. Co-authored-by: data-engineer <data-engineer@gradata.ai>
1 participant
Summary
Adds
gradata forget <selector>— undo lessons from the brain via CLI without writing Python.Wraps the existing
Brain.forget()public API. Preview-before-apply UX with interactive confirmation;--yesskips for scripts.Selector syntax
Soft-delete semantics
KILLEDstate (not hard-deleted)action='rolled_back'andkill_reason='manual_forget'apply_brain_rules()reflects the changeTest plan
6 cases: last (single), last N, all CATEGORY, fuzzy description, no-match clean error, unknown-category clean error. Tests seed via the public
Brain.correct()API so the events table matches production exactly.Epic context
GRA-1198 (kill the plugin) / GH #206. Replaces plugin
/gradata:forgetslash command. Companion to PR #208 (gradata status).Layering
cmd_forgetdelegates toBrain.forget()(Layer 2 public API). Layer 0 imports (LessonState, parse_lessons) used for preview only, not state mutation. No Layer 0 → 2 imports.Risk
Low. Pure additive CLI wrapper. Underlying
Brain.forget()already has production usage.