test: snapshot Claude Code install hook coverage#233
Conversation
![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 extends the Gradata hook system to support Claude Code integration by introducing four new hook command builders, reworking the Claude Code adapter's installation to wire five lifecycle hooks (PreToolUse, PostToolUse, Stop, PreCompact, UserPromptSubmit), and adding comprehensive snapshot-based tests that validate the installation output, idempotency, coexistence with user hooks, and coverage of all required lifecycles. ChangesClaude Code Hook Installation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Gradata/src/gradata/hooks/adapters/claude_code.py (1)
147-191:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
uninstall()must remove all 5 lifecycles, not just PreToolUse.The
install()function now adds hooks to five lifecycle lists (PreToolUse,PostToolUse,Stop,PreCompact,UserPromptSubmit), butuninstall()only removesPreToolUseentries. After uninstall, four orphaned Gradata hooks remain active in the config.The docstring claims to "Reverse
install()" but only handles 20% of the installed hooks.🔧 Extend uninstall() to remove all 5 hook lifecycles
Apply similar signature-removal logic to the other four lifecycle lists:
def uninstall(brain_dir: Path, agent_config_path: Path) -> InstallResult: - """Reverse ``install()``: drop the signature-matching PreToolUse entry. + """Reverse ``install()``: drop signature-matching hook entries from all lifecycles. Idempotent — calling on an already-clean config returns ``already_present`` (semantically: 'already in the desired absent state'). Empty containers - are pruned. User-owned PreToolUse entries (without our signature) are - preserved verbatim. + are pruned. User-owned hook entries (without our signature) are preserved. """ try: if not agent_config_path.is_file(): return InstallResult( AGENT, agent_config_path, "already_present", "config file does not exist" ) sig = hook_signature(AGENT, brain_dir) data = read_json(agent_config_path) hooks = data.get("hooks") if not isinstance(hooks, dict): return InstallResult(AGENT, agent_config_path, "already_present", "no hooks block") - pre_tool = hooks.get("PreToolUse") - if not isinstance(pre_tool, list): - return InstallResult(AGENT, agent_config_path, "already_present", "no PreToolUse") removed = 0 - kept: list = [] - for entry in pre_tool: - entry_str = str(entry) - if sig in entry_str: - # Either the entry's `hooks[].id` carries our sig, or the - # whole entry was ours. Drop it. - removed += 1 - continue - kept.append(entry) + for lifecycle in ("PreToolUse", "PostToolUse", "Stop", "PreCompact", "UserPromptSubmit"): + entries = hooks.get(lifecycle) + if not isinstance(entries, list): + continue + kept: list = [] + for entry in entries: + if sig in str(entry): + removed += 1 + continue + kept.append(entry) + if kept: + hooks[lifecycle] = kept + else: + hooks.pop(lifecycle, None) + if removed == 0: return InstallResult(AGENT, agent_config_path, "already_present", "hook not present") - if kept: - hooks["PreToolUse"] = kept - else: - hooks.pop("PreToolUse", None) if not hooks: data.pop("hooks", None) write_json(agent_config_path, data) - return InstallResult(AGENT, agent_config_path, "removed", f"removed {removed} hook entry") + return InstallResult(AGENT, agent_config_path, "removed", f"removed {removed} hook entries") except Exception as exc: return failure(AGENT, agent_config_path, exc)🤖 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/hooks/adapters/claude_code.py` around lines 147 - 191, uninstall() currently only removes entries from the "PreToolUse" lifecycle but install() adds hooks to five lifecycles; update uninstall() to iterate the five lifecycle keys ("PreToolUse", "PostToolUse", "Stop", "PreCompact", "UserPromptSubmit") and apply the same signature-match removal logic using the existing sig variable: for each lifecycle, if hooks.get(lifecycle) is a list, build a kept list skipping entries containing sig, count removals, set hooks[lifecycle]=kept or pop the lifecycle if kept is empty, and aggregate removals to decide whether to return "already_present" or "removed"; preserve existing behavior for non-dict hooks and ensure you still prune the hooks block, call write_json(agent_config_path, data), and return InstallResult or failure(...) as before.
🤖 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.
Outside diff comments:
In `@Gradata/src/gradata/hooks/adapters/claude_code.py`:
- Around line 147-191: uninstall() currently only removes entries from the
"PreToolUse" lifecycle but install() adds hooks to five lifecycles; update
uninstall() to iterate the five lifecycle keys ("PreToolUse", "PostToolUse",
"Stop", "PreCompact", "UserPromptSubmit") and apply the same signature-match
removal logic using the existing sig variable: for each lifecycle, if
hooks.get(lifecycle) is a list, build a kept list skipping entries containing
sig, count removals, set hooks[lifecycle]=kept or pop the lifecycle if kept is
empty, and aggregate removals to decide whether to return "already_present" or
"removed"; preserve existing behavior for non-dict hooks and ensure you still
prune the hooks block, call write_json(agent_config_path, data), and return
InstallResult or failure(...) as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 55381c19-42f4-4f98-b826-3a85a11e1c6b
📒 Files selected for processing (4)
Gradata/src/gradata/hooks/adapters/_base.pyGradata/src/gradata/hooks/adapters/claude_code.pyGradata/tests/snapshots/install_claude_code_settings.jsonGradata/tests/test_install_claude_code_snapshot.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
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/hooks/adapters/_base.pyGradata/src/gradata/hooks/adapters/claude_code.py
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_install_claude_code_snapshot.py
🔇 Additional comments (7)
Gradata/src/gradata/hooks/adapters/_base.py (1)
139-164: LGTM!Gradata/src/gradata/hooks/adapters/claude_code.py (1)
59-144: LGTM!Gradata/tests/test_install_claude_code_snapshot.py (4)
123-148: LGTM!
150-176: LGTM!
178-244: LGTM!
246-277: LGTM!Gradata/tests/snapshots/install_claude_code_settings.json (1)
1-63: LGTM!
1 participant
Summary
gradata install --agent claude-codesettings outputTest plan
python3 -m pytest tests/test_install_claude_code_snapshot.py tests/test_cli_install_agent.py tests/test_uninstall_command.py tests/test_hook_adapters.py -qCloses GRA-1211. Supports GH #206.