feat(hooks): add Claude PreCompact snapshot hook#224
Conversation
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.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📜 Recent 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). (2)
🧰 Additional context used📓 Path-based instructions (2)Gradata/tests/**/*.py📄 CodeRabbit inference engine (Gradata/AGENTS.md)
Files:
Gradata/src/**/*.py📄 CodeRabbit inference engine (Gradata/AGENTS.md)
Files:
🔇 Additional comments (3)
📝 WalkthroughWalkthroughThis PR adds a PreCompact snapshot mechanism that atomically writes JSON snapshots (with sanitized session IDs and captured context files) under .precompact-snapshots/, provides command/signature helpers, integrates PreCompact hook install/uninstall in the claude-code adapter, and includes tests for adapter wiring and end-to-end snapshot creation. ChangesPre-compact hook snapshot and integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labelsfeature 🚥 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.22.0)OpenGrep fatal error (exit code 2): �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m Comment |
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.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/hooks/adapters/claude_code.py`:
- Around line 133-147: The current loop over pre_tool removes entire entries
when sig is found in the entry string, which can drop user hooks; instead, parse
each entry (the dict/object in pre_tool) and mutate its internal hooks list to
remove only hook items that match our sig (and ignore/prevent removing items
that contain pre_compact_sig), then append the pruned entry to kept if it still
has non-empty hooks; if hooks become empty, skip the entry (and remove the
PreToolUse key if no kept entries remain). Apply the same targeted-pruning
approach to the PreCompact handling (the analogous loop that uses
pre_compact_sig) so only matching Gradata hooks are removed rather than deleting
mixed entries wholesale.
In `@Gradata/src/gradata/hooks/pre_compact.py`:
- Around line 52-53: In hooks/pre_compact.py there are except blocks (e.g., the
except OSError that currently just returns None and another at lines ~102-105)
that silently swallow exceptions; update those handlers to log the exception
before returning by calling the module logger (e.g., logger.warning or
logger.exception) with a contextual message and exc_info=True so the stacktrace
is recorded, then continue to return None as before; locate the handlers in
pre_compact.py and replace the bare silent returns with a logging call that
includes exc_info=True.
- Around line 51-56: The code currently calls path.read_text() which loads the
entire file into memory before truncating; change to streaming-read only up to
MAX_FILE_CHARS+1 bytes to avoid memory spikes: open the file via path.open(...)
and call read(MAX_FILE_CHARS + 1) into text, set truncated = len(text) >
MAX_FILE_CHARS, and if truncated slice text = text[:MAX_FILE_CHARS]; preserve
the same exception handling for OSError and keep variable names (path,
MAX_FILE_CHARS, truncated, text) so the rest of the function remains unchanged.
🪄 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: eeccb7bf-97e7-4675-9a5a-ab2741d3b9fb
📒 Files selected for processing (5)
Gradata/src/gradata/hooks/adapters/_base.pyGradata/src/gradata/hooks/adapters/claude_code.pyGradata/src/gradata/hooks/pre_compact.pyGradata/tests/test_hook_adapters.pyGradata/tests/test_pre_compact_hook.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). (4)
- GitHub Check: pytest ubuntu-latest / py3.11
- GitHub Check: pytest ubuntu-latest / py3.12
- GitHub Check: pytest windows-latest / py3.12
- GitHub Check: pytest windows-latest / py3.11
🧰 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/claude_code.pyGradata/src/gradata/hooks/adapters/_base.pyGradata/src/gradata/hooks/pre_compact.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_pre_compact_hook.pyGradata/tests/test_hook_adapters.py
🔇 Additional comments (5)
Gradata/src/gradata/hooks/pre_compact.py (1)
36-45: LGTM!Also applies to: 76-95, 99-101, 106-106
Gradata/src/gradata/hooks/adapters/_base.py (1)
139-143: LGTM!Gradata/src/gradata/hooks/adapters/claude_code.py (1)
15-15: LGTM!Also applies to: 21-21, 28-30, 64-107, 112-126
Gradata/tests/test_hook_adapters.py (1)
3-3: LGTM!Also applies to: 69-92
Gradata/tests/test_pre_compact_hook.py (1)
1-45: LGTM!
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.
Summary
Verification
Closes GRA-1210