Skip to content

fix: default Claude hooks to production brain#242

Merged
Gradata merged 6 commits into
mainfrom
gra-104-fix-claude-hook-paths
Jun 3, 2026
Merged

fix: default Claude hooks to production brain#242
Gradata merged 6 commits into
mainfrom
gra-104-fix-claude-hook-paths

Conversation

@Gradata

@Gradata Gradata commented Jun 2, 2026

Copy link
Copy Markdown
Owner

Fixes GRA-104.

Changes:

  • Agent hook installs now default to ~/.gradata/brain instead of the current working directory when no explicit brain is provided.
  • BRAIN_DIR / GRADATA_BRAIN / explicit --brain still override the default.
  • Added stale Claude settings detection for Gradata hook commands whose BRAIN_DIR no longer exists.
  • Hardened adapter tests so installs do not touch real user configs.

Verification:

  • env -u BRAIN_DIR python3 -m pytest tests/test_hook_adapters.py tests/test_hooks_base.py tests/test_hooks_safety.py tests/test_hooks_learning.py tests/test_hooks_intelligence.py tests/test_hook_profile.py tests/test_rule_to_hook.py tests/test_rule_to_hook_promotion.py tests/test_inject_handoff_hook.py tests/test_hermes_hook_event_names.py -q → 225 passed in 24.15s
  • python3 -m pytest tests/test_hook_adapters.py tests/test_hooks_base.py -q → 33 passed in 0.75s

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a precedence-based path resolver and agent-specific brain-root default (~/.gradata/brain), uses it for CLI agent install/uninstall (including reusing recorded manifest signatures), tightens a zip in cmd_prove, detects missing BRAIN_DIR hooks in Claude settings, generalizes adapter hook commands/HOOKS across events, and updates tests and help text.

Changes

Agent Brain Root Resolution and Stale Detection

Layer / File(s) Summary
Path resolution helpers
Gradata/src/gradata/cli.py
Adds _resolve_path_with_precedence(...), _resolve_agent_brain_root(args), reimplements _resolve_brain_root(args) to use the helper, updates _get_brain usage, tightens cmd_prove to use zip(..., strict=True), and updates CLI help text defaults.
CLI install/uninstall integration
Gradata/src/gradata/cli.py
_cmd_install_agent and cmd_uninstall now compute brain_dir via _resolve_agent_brain_root(args) and cmd_uninstall prefers the recorded install manifest signature when present to locate the original brain target.
Stale hook detection for missing brain directories
Gradata/src/gradata/hooks/stale_hook_check.py
Default brain root changed to ~/.gradata/brain; adds JSON parsing of Claude settings, extraction of BRAIN_DIR=... from hook command strings, scanning for missing referenced brain dirs, and prints a “stale Claude settings hook warnings” block with suggested gradata install --agent claude-code --brain ... fixes.
Adapter command modularization and multi-event HOOKS
Gradata/src/gradata/hooks/adapters/_base.py, Gradata/src/gradata/hooks/adapters/claude_code.py, Gradata/src/gradata/hooks/adapters/hermes.py
hook_command accepts a module parameter; Claude and Hermes adapters introduce HOOKS tables and generalize install/uninstall to manage multiple events/modules and to scan/remove entries across event groups.
Test coverage and isolation
Gradata/tests/test_hook_adapters.py, Gradata/tests/test_hooks_base.py, Gradata/tests/test_hermes_hook_event_names.py, Gradata/tests/test_uninstall_command.py
Tests redirect HOME/USERPROFILE to temporary directories, parametrize immutability checks across all agents, validate adapter config path logic with explicit home arg, add tests for agent default brain resolution and env-precedence, add a test asserting stale_hook_check warns on missing configured brain dirs, and simplify SystemExit handling in uninstall test.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • Gradata/gradata#215: Overlaps CLI agent install/uninstall behavior and brain-dir resolution adjustments used when writing/removing hook configs.
  • Gradata/gradata#189: Also modifies the agent install flow in gradata/cli.py and interacts with brain-path resolution and post-install verification.
  • Gradata/gradata#26: Prior changes to CLI brain-root resolution that overlap with this PR’s resolver and defaults.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 47.22% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: default Claude hooks to production brain' directly and concisely describes the main change: modifying the default brain path for Claude hook installs from the current working directory to ~/.gradata/brain.
Description check ✅ Passed The description clearly explains the core fix (agent hook defaults changed to ~/.gradata/brain), environment variable precedence, stale hook detection, test hardening, and includes verification results that confirm the changes work correctly.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch gra-104-fix-claude-hook-paths

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot added the bug Something isn't working label Jun 2, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/cli.py`:
- Around line 1221-1238: Both _resolve_agent_brain_root and _resolve_brain_root
duplicate the same precedence logic (args → env vars → default); extract that
logic into a single helper (e.g., _resolve_path_with_precedence or
_resolve_brain_path) that accepts the args object, the candidate arg names
("brain_dir", "brain"), the env var names ("BRAIN_DIR", "GRADATA_BRAIN"), and
the default subpath (".gradata/brain" or similar), then replace both
_resolve_agent_brain_root and _resolve_brain_root to call this helper so they
only supply the specific names/defaults and return the Path result. Ensure the
helper returns a Path and preserves existing behavior (explicit args override
env, envs checked in order, fallback to Path.home() / default).

In `@Gradata/src/gradata/hooks/stale_hook_check.py`:
- Around line 144-147: The current except branch falls back to naive
command.split(), which loses shell quoting semantics; change the except
ValueError handler in the shlex.split block (the try that assigns to parts) to
return None (or otherwise signal parsing ambiguity) instead of doing
command.split(), so the function preserves its safe behavior for validation;
update any callers of the function if needed to handle a None result.
- Around line 266-275: The suggested fix command always uses "--brain
~/.gradata/brain"; update the printing logic in stale_hook_check.py (the block
that calls _missing_hook_brain_dirs() and prints the "fix:" suggestion) to
detect the user's configured brain path from environment variables (check
BRAIN_DIR, then GRADATA_BRAIN) and use that value in the printed command,
falling back to "~/.gradata/brain" if neither is set; ensure you only change the
printed suggestion string (not behavior of install) and reference the existing
function/name _missing_hook_brain_dirs() to locate the block to modify.
🪄 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: 4ec2e3e1-1a2f-4346-b075-f6ba459bf477

📥 Commits

Reviewing files that changed from the base of the PR and between be4df94 and 2fdcedb.

📒 Files selected for processing (4)
  • Gradata/src/gradata/cli.py
  • Gradata/src/gradata/hooks/stale_hook_check.py
  • Gradata/tests/test_hook_adapters.py
  • Gradata/tests/test_hooks_base.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 windows-latest / py3.11
  • GitHub Check: pytest (py3.12)
  • GitHub Check: pytest ubuntu-latest / py3.11
  • GitHub Check: pytest ubuntu-latest / py3.12
  • GitHub Check: pytest macos-latest / py3.12
  • GitHub Check: pytest (py3.11)
  • GitHub Check: pytest macos-latest / py3.11
🧰 Additional context used
📓 Path-based instructions (2)
Gradata/tests/**/*.py

📄 CodeRabbit inference engine (Gradata/AGENTS.md)

Gradata/tests/**/*.py: Set BRAIN_DIR environment variable via tmp_path in conftest.py for test isolation — ensure _paths.py module cache refreshes when calling Brain.init() directly inside tests
Add unit tests in tests/test_*.py for every CI push without LLM calls (deterministic); mark integration tests with @pytest.mark.integration and skip them by default (they hit real LLM APIs)

Files:

  • Gradata/tests/test_hooks_base.py
  • Gradata/tests/test_hook_adapters.py
Gradata/src/**/*.py

📄 CodeRabbit inference engine (Gradata/AGENTS.md)

Gradata/src/**/*.py: Prefer sentence-transformers for local embeddings, google-genai for Gemini embeddings, cryptography for AES-GCM encrypted system.db, bm25s for BM25 rule ranking, and mem0ai for external memory adapters — guard all optional dependency imports with try / except ImportError at 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 bare except: pass — use typed exceptions or at minimum logger.warning(...) with exc_info=True to avoid silent failure in a memory product
Never import from out-of-scope sibling directories ../Sprites/ or ../Hausgem/ within gradata/* 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 inside gradata/*
Use atomic-write helper when writing JSON files to prevent corruption from mid-write crashes

Files:

  • Gradata/src/gradata/hooks/stale_hook_check.py
  • Gradata/src/gradata/cli.py
🔇 Additional comments (8)
Gradata/src/gradata/cli.py (2)

470-470: LGTM!


576-576: LGTM!

Also applies to: 2057-2057, 2090-2090

Gradata/src/gradata/hooks/stale_hook_check.py (2)

129-133: LGTM!


156-190: LGTM!

Gradata/tests/test_hook_adapters.py (3)

20-21: LGTM!


57-73: LGTM!


75-87: LGTM!

Gradata/tests/test_hooks_base.py (1)

19-50: LGTM!

Comment thread Gradata/src/gradata/cli.py
Comment thread Gradata/src/gradata/hooks/stale_hook_check.py Outdated
Comment thread Gradata/src/gradata/hooks/stale_hook_check.py

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
Gradata/src/gradata/cli.py (1)

35-52: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Consider refactoring _get_brain to use _resolve_brain_root.

_get_brain duplicates the same precedence logic as _resolve_brain_root (args → env → default). Since the docstring already states "Precedence mirrors :func:_resolve_brain_root exactly", consolidating them would eliminate duplication and ensure they stay in sync.

♻️ Proposed refactor
 def _get_brain(args):
     """Resolve brain directory from env, args, or cwd.
 
     Precedence mirrors :func:`_resolve_brain_root` exactly —
     explicit args > ``BRAIN_DIR`` env > ``GRADATA_BRAIN`` env > cwd — so both helpers
     always target the same brain (important for export, tests with tmp
     brains, etc.).
     """
     from gradata import Brain
-
-    brain_dir = (
-        getattr(args, "brain_dir", None)
-        or getattr(args, "brain", None)
-        or env_str("BRAIN_DIR")
-        or env_str("GRADATA_BRAIN")
-        or Path.cwd()
-    )
-    return Brain(brain_dir)
+    return Brain(_resolve_brain_root(args))
🤖 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 35 - 52, The _get_brain function
duplicates precedence logic; replace its manual resolution with a call to
_resolve_brain_root so both helpers stay in sync: have _get_brain call
_resolve_brain_root(args) (importing or referencing the existing function) to
obtain the brain root and then return Brain(resolved_root), removing the
duplicated getattr/env_str/Path.cwd logic in _get_brain; ensure
imports/references to Brain and _resolve_brain_root are present and adjust the
docstring to reflect the delegation.
Gradata/src/gradata/hooks/adapters/claude_code.py (1)

93-100: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Stale docstring: uninstall now spans all events, not just PreToolUse.

The implementation iterates every event in the hooks map (Lines 112-125), but the docstring still describes dropping "the signature-matching PreToolUse entry". Update it to reflect the multi-event removal so future readers aren't misled.

🤖 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 93 - 100, The
uninstall docstring is outdated (it claims only PreToolUse is removed) but the
function uninstall actually iterates all events in the hooks map and removes
signature-matching entries for each event; update the docstring for uninstall to
state that it scans every event in the hooks map and removes any entries
matching our signature (preserving user-owned entries and pruning empty
containers), and keep existing notes about idempotence and preservation of
non-signed entries; reference the uninstall function and the loop over
hooks/events when making the docstring change.
Gradata/src/gradata/hooks/adapters/hermes.py (1)

260-275: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Uninstall leaves orphaned post_tool_call and on_session_end hooks.

install() now writes signature-bearing entries to all three HOOKS events (pre_tool_call, post_tool_call, on_session_end), but uninstall only scans ("pre_tool_call", "pre_tool_use"). Since removed becomes nonzero from pre_tool_call, the config is rewritten and the post_tool_call/on_session_end Gradata entries persist — pointing at a brain dir the user just tried to remove. Iterate all event lists (as claude_code.uninstall does) so removal is symmetric with install.

🐛 Proposed fix to remove signature entries across all events
         removed = 0
-        # Check both current and legacy event names.
-        for key in ("pre_tool_call", "pre_tool_use"):
-            entries = hooks.get(key)
-            if not isinstance(entries, list):
-                continue
-            kept = []
-            for entry in entries:
-                if sig in str(entry):
-                    removed += 1
-                    continue
-                kept.append(entry)
-            if kept:
-                hooks[key] = kept
-            else:
-                hooks.pop(key, None)
+        # Scan every hook event (current + legacy names) so all entries
+        # written by install() are removed.
+        for key in list(hooks):
+            entries = hooks.get(key)
+            if not isinstance(entries, list):
+                continue
+            kept = []
+            for entry in entries:
+                if sig in str(entry):
+                    removed += 1
+                    continue
+                kept.append(entry)
+            if kept:
+                hooks[key] = kept
+            else:
+                hooks.pop(key, None)

Also update the docstring on Line 240, which still says removal targets only hooks.pre_tool_call.

🤖 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/hermes.py` around lines 260 - 275, The
uninstall routine only scans ("pre_tool_call","pre_tool_use") so
signature-bearing entries added by install to "post_tool_call" and
"on_session_end" remain; update the removal loop in the uninstall function (the
block that uses sig and hooks) to iterate over all hook event names (e.g., the
HOOKS constant or list of its keys) instead of the hard-coded tuple so it
removes signature entries from pre_tool_call, post_tool_call, on_session_end,
etc.; keep the same entries list filtering logic (checking isinstance(entries,
list) and comparing sig in str(entry)), update hooks[key] or pop(key)
accordingly, and update the uninstall function docstring (the docstring
currently referencing removal of only hooks.pre_tool_call) to accurately say it
removes signature entries from all registered hook events.
🤖 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/cli.py`:
- Around line 35-52: The _get_brain function duplicates precedence logic;
replace its manual resolution with a call to _resolve_brain_root so both helpers
stay in sync: have _get_brain call _resolve_brain_root(args) (importing or
referencing the existing function) to obtain the brain root and then return
Brain(resolved_root), removing the duplicated getattr/env_str/Path.cwd logic in
_get_brain; ensure imports/references to Brain and _resolve_brain_root are
present and adjust the docstring to reflect the delegation.

In `@Gradata/src/gradata/hooks/adapters/claude_code.py`:
- Around line 93-100: The uninstall docstring is outdated (it claims only
PreToolUse is removed) but the function uninstall actually iterates all events
in the hooks map and removes signature-matching entries for each event; update
the docstring for uninstall to state that it scans every event in the hooks map
and removes any entries matching our signature (preserving user-owned entries
and pruning empty containers), and keep existing notes about idempotence and
preservation of non-signed entries; reference the uninstall function and the
loop over hooks/events when making the docstring change.

In `@Gradata/src/gradata/hooks/adapters/hermes.py`:
- Around line 260-275: The uninstall routine only scans
("pre_tool_call","pre_tool_use") so signature-bearing entries added by install
to "post_tool_call" and "on_session_end" remain; update the removal loop in the
uninstall function (the block that uses sig and hooks) to iterate over all hook
event names (e.g., the HOOKS constant or list of its keys) instead of the
hard-coded tuple so it removes signature entries from pre_tool_call,
post_tool_call, on_session_end, etc.; keep the same entries list filtering logic
(checking isinstance(entries, list) and comparing sig in str(entry)), update
hooks[key] or pop(key) accordingly, and update the uninstall function docstring
(the docstring currently referencing removal of only hooks.pre_tool_call) to
accurately say it removes signature entries from all registered hook events.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: dede9943-0219-462c-a30a-98d230127b6d

📥 Commits

Reviewing files that changed from the base of the PR and between 2fdcedb and 06b8597.

📒 Files selected for processing (7)
  • Gradata/src/gradata/cli.py
  • Gradata/src/gradata/hooks/adapters/_base.py
  • Gradata/src/gradata/hooks/adapters/claude_code.py
  • Gradata/src/gradata/hooks/adapters/hermes.py
  • Gradata/src/gradata/hooks/stale_hook_check.py
  • Gradata/tests/test_hook_adapters.py
  • Gradata/tests/test_hooks_base.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.11
  • GitHub Check: pytest (py3.12)
  • GitHub Check: pytest macos-latest / py3.12
  • GitHub Check: pytest (py3.11)
  • GitHub Check: pytest ubuntu-latest / py3.12
  • GitHub Check: pytest ubuntu-latest / py3.11
  • GitHub Check: pytest windows-latest / py3.12
  • GitHub Check: pytest macos-latest / py3.11
🧰 Additional context used
📓 Path-based instructions (2)
Gradata/src/**/*.py

📄 CodeRabbit inference engine (Gradata/AGENTS.md)

Gradata/src/**/*.py: Prefer sentence-transformers for local embeddings, google-genai for Gemini embeddings, cryptography for AES-GCM encrypted system.db, bm25s for BM25 rule ranking, and mem0ai for external memory adapters — guard all optional dependency imports with try / except ImportError at 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 bare except: pass — use typed exceptions or at minimum logger.warning(...) with exc_info=True to avoid silent failure in a memory product
Never import from out-of-scope sibling directories ../Sprites/ or ../Hausgem/ within gradata/* 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 inside gradata/*
Use atomic-write helper when writing JSON files to prevent corruption from mid-write crashes

Files:

  • Gradata/src/gradata/hooks/adapters/_base.py
  • Gradata/src/gradata/hooks/adapters/hermes.py
  • Gradata/src/gradata/hooks/adapters/claude_code.py
  • Gradata/src/gradata/cli.py
  • Gradata/src/gradata/hooks/stale_hook_check.py
Gradata/tests/**/*.py

📄 CodeRabbit inference engine (Gradata/AGENTS.md)

Gradata/tests/**/*.py: Set BRAIN_DIR environment variable via tmp_path in conftest.py for test isolation — ensure _paths.py module cache refreshes when calling Brain.init() directly inside tests
Add unit tests in tests/test_*.py for every CI push without LLM calls (deterministic); mark integration tests with @pytest.mark.integration and skip them by default (they hit real LLM APIs)

Files:

  • Gradata/tests/test_hooks_base.py
  • Gradata/tests/test_hook_adapters.py
🧠 Learnings (4)
📓 Common learnings
Learnt from: Gradata
Repo: Gradata/gradata PR: 0
File: :0-0
Timestamp: 2026-04-17T17:18:07.439Z
Learning: In PR `#102` (gradata/gradata), Round 2 addressed: cli.py env-first brain resolution (GRADATA_BRAIN > --brain-dir > cwd), _tenant.py corrupt .tenant_id overwrite, _env_int default clamping to minimum, and _events.py tenant-scoped fallback SELECT for dedup. All ruff and 99 tests green after these fixes.
📚 Learning: 2026-05-01T15:50:32.772Z
Learnt from: CR
Repo: Gradata/gradata PR: 0
File: Gradata/AGENTS.md:0-0
Timestamp: 2026-05-01T15:50:32.772Z
Learning: Applies to Gradata/tests/**/*.py : Set `BRAIN_DIR` environment variable via `tmp_path` in conftest.py for test isolation — ensure `_paths.py` module cache refreshes when calling `Brain.init()` directly inside tests

Applied to files:

  • Gradata/tests/test_hooks_base.py
  • Gradata/tests/test_hook_adapters.py
  • Gradata/src/gradata/cli.py
  • Gradata/src/gradata/hooks/stale_hook_check.py
📚 Learning: 2026-04-17T17:18:07.439Z
Learnt from: Gradata
Repo: Gradata/gradata PR: 0
File: :0-0
Timestamp: 2026-04-17T17:18:07.439Z
Learning: In PR `#102` (gradata/gradata), Round 2 addressed: cli.py env-first brain resolution (GRADATA_BRAIN > --brain-dir > cwd), _tenant.py corrupt .tenant_id overwrite, _env_int default clamping to minimum, and _events.py tenant-scoped fallback SELECT for dedup. All ruff and 99 tests green after these fixes.

Applied to files:

  • Gradata/src/gradata/cli.py
  • Gradata/src/gradata/hooks/stale_hook_check.py
📚 Learning: 2026-05-01T15:50:32.772Z
Learnt from: CR
Repo: Gradata/gradata PR: 0
File: Gradata/AGENTS.md:0-0
Timestamp: 2026-05-01T15:50:32.772Z
Learning: Use `from gradata import Brain` as the public entry point — `brain.correct()` is THE entry point for the headline product promise

Applied to files:

  • Gradata/src/gradata/hooks/stale_hook_check.py
🔇 Additional comments (14)
Gradata/src/gradata/cli.py (4)

880-880: LGTM!


1207-1223: LGTM!


1226-1228: LGTM!


1231-1238: LGTM!

Gradata/src/gradata/hooks/stale_hook_check.py (2)

140-153: LGTM!


266-276: LGTM!

Gradata/src/gradata/hooks/adapters/_base.py (1)

132-136: LGTM!

Gradata/src/gradata/hooks/adapters/claude_code.py (2)

62-90: LGTM!


20-26: ⚡ Quick win

Hook modules exist and are runnable via python -m gradata.hooks.<module>inject_brain_rules, auto_correct, session_close, pre_compact, and context_inject all exist as importable modules under gradata.hooks, each with an if __name__ == "__main__" entrypoint.

Gradata/src/gradata/hooks/adapters/hermes.py (1)

171-182: LGTM!

Gradata/tests/test_hook_adapters.py (3)

58-74: LGTM!


77-90: LGTM!


18-23: ⚡ Quick win

Env-only isolation is sufficient for adapter_config_path
adapter_config_path(agent) resolves its home/config root via _resolve_home_dir() in Gradata/src/gradata/hooks/adapters/_base.py, which uses only HOME or USERPROFILE (fallback os.path.expanduser("~"))—it does not consult XDG_CONFIG_HOME or APPDATA. Therefore, in Gradata/tests/test_hook_adapters.py (lines 18-23), patching HOME/USERPROFILE before adapter_config_path(agent) is sufficient to keep the idempotent install() writes confined to the test’s temp directory.

Gradata/tests/test_hooks_base.py (1)

19-51: LGTM!

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/tests/test_hermes_hook_event_names.py`:
- Around line 45-52: The test uses brittle exact two-space indentation checks
(lines.count("  pre_tool_call:") etc.); change the assertions in
test_hermes_hook_event_names.py to ignore leading whitespace by counting
stripped lines or parsing the YAML: replace the lines.count(...) checks with
something like sum(1 for l in lines if l.strip() == "pre_tool_call:") == 1 (and
similarly for "post_tool_call:" and "on_session_end:"), or load the file with a
YAML parser and assert the keys exist once; update the assertions referencing
variable lines and the string literals accordingly.

In `@Gradata/tests/test_uninstall_command.py`:
- Around line 163-165: The test drains capsys twice causing stderr to be lost;
call capsys.readouterr() only once and capture its return (e.g., result =
capsys.readouterr()) then combine result.out and result.err when asserting;
update the block around cmd_uninstall(args) that currently does two readouterr()
calls so it reads once and uses that single result for both stdout and stderr
checks.
🪄 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: f1f46a36-1c76-4227-915c-e3a1c1351e95

📥 Commits

Reviewing files that changed from the base of the PR and between 06b8597 and 7db910c.

📒 Files selected for processing (3)
  • Gradata/src/gradata/hooks/adapters/hermes.py
  • Gradata/tests/test_hermes_hook_event_names.py
  • Gradata/tests/test_uninstall_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). (2)
  • GitHub Check: pytest windows-latest / py3.12
  • GitHub Check: pytest windows-latest / py3.11
🧰 Additional context used
📓 Path-based instructions (2)
Gradata/tests/**/*.py

📄 CodeRabbit inference engine (Gradata/AGENTS.md)

Gradata/tests/**/*.py: Set BRAIN_DIR environment variable via tmp_path in conftest.py for test isolation — ensure _paths.py module cache refreshes when calling Brain.init() directly inside tests
Add unit tests in tests/test_*.py for every CI push without LLM calls (deterministic); mark integration tests with @pytest.mark.integration and skip them by default (they hit real LLM APIs)

Files:

  • Gradata/tests/test_uninstall_command.py
  • Gradata/tests/test_hermes_hook_event_names.py
Gradata/src/**/*.py

📄 CodeRabbit inference engine (Gradata/AGENTS.md)

Gradata/src/**/*.py: Prefer sentence-transformers for local embeddings, google-genai for Gemini embeddings, cryptography for AES-GCM encrypted system.db, bm25s for BM25 rule ranking, and mem0ai for external memory adapters — guard all optional dependency imports with try / except ImportError at 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 bare except: pass — use typed exceptions or at minimum logger.warning(...) with exc_info=True to avoid silent failure in a memory product
Never import from out-of-scope sibling directories ../Sprites/ or ../Hausgem/ within gradata/* 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 inside gradata/*
Use atomic-write helper when writing JSON files to prevent corruption from mid-write crashes

Files:

  • Gradata/src/gradata/hooks/adapters/hermes.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: Gradata
Repo: Gradata/gradata PR: 0
File: :0-0
Timestamp: 2026-04-17T17:18:07.439Z
Learning: In PR `#102` (gradata/gradata), Round 2 addressed: cli.py env-first brain resolution (GRADATA_BRAIN > --brain-dir > cwd), _tenant.py corrupt .tenant_id overwrite, _env_int default clamping to minimum, and _events.py tenant-scoped fallback SELECT for dedup. All ruff and 99 tests green after these fixes.
🔇 Additional comments (4)
Gradata/src/gradata/hooks/adapters/hermes.py (3)

20-24: LGTM!


171-181: LGTM!


261-277: LGTM!

Gradata/tests/test_uninstall_command.py (1)

12-12: LGTM!

Comment thread Gradata/tests/test_hermes_hook_event_names.py Outdated
Comment thread Gradata/tests/test_uninstall_command.py Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
Gradata/src/gradata/cli.py (3)

1198-1214: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Preserve the repo's env-first brain resolution contract.

Line 1206 now prefers CLI args over env vars, so _get_brain() regresses the previously-fixed GRADATA_BRAIN override behavior and can resolve a different brain than commands that still follow the old contract.

♻️ Proposed fix
 def _resolve_path_with_precedence(
     args,
     *,
     arg_names: tuple[str, ...] = ("brain_dir", "brain"),
-    env_names: tuple[str, ...] = ("BRAIN_DIR", "GRADATA_BRAIN"),
+    env_names: tuple[str, ...] = ("GRADATA_BRAIN", "BRAIN_DIR"),
     default: Path,
 ) -> Path:
-    """Resolve a path from CLI args, then env vars, then a caller default."""
-    for name in arg_names:
-        value = getattr(args, name, None)
-        if value:
-            return Path(value)
+    """Resolve a path from env vars, then CLI args, then a caller default."""
     for name in env_names:
         value = env_str(name)
         if value:
             return Path(value)
+    for name in arg_names:
+        value = getattr(args, name, None)
+        if value:
+            return Path(value)
     return default

Based on learnings: "In PR #102 (gradata/gradata), Round 2 addressed: cli.py env-first brain resolution (GRADATA_BRAIN > --brain-dir > cwd), _tenant.py corrupt .tenant_id overwrite, _env_int default clamping to minimum, and _events.py tenant-scoped fallback SELECT for dedup."

🤖 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 1198 - 1214, The path resolution
currently prefers CLI args over environment variables in
_resolve_path_with_precedence, which breaks the repo's env-first contract;
change the logic in _resolve_path_with_precedence (or call sites like
_get_brain) to check env_names (ENV: "BRAIN_DIR", "GRADATA_BRAIN") before
arg_names ("brain_dir", "brain") so that env_str(name) wins over getattr(args,
name) and only fall back to the caller default Path when neither env nor CLI
provide a value.

2045-2050: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Mention GRADATA_BRAIN in both agent help strings.

Lines 2049 and 2082 only advertise BRAIN_DIR, but _resolve_agent_brain_root() also honors GRADATA_BRAIN. The CLI help should match the actual override behavior.

📝 Proposed fix
-        help="Brain directory for agent hook config (default: BRAIN_DIR or ~/.gradata/brain)",
+        help="Brain directory for agent hook config (default: GRADATA_BRAIN, BRAIN_DIR, or ~/.gradata/brain)",

Apply the same change to the uninstall help text.

Also applies to: 2078-2083

🤖 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 2045 - 2050, Update the help text
for the "--brain" argument in both p_install.add_argument (the install command)
and the corresponding uninstall parser (p_uninstall.add_argument) so it mentions
the GRADATA_BRAIN environment variable in addition to BRAIN_DIR and the default
path; modify the help strings referenced in the "--brain" argument declarations
to read something like "Brain directory for agent hook config (default:
GRADATA_BRAIN or BRAIN_DIR or ~/.gradata/brain)" so the CLI help matches the
behavior of _resolve_agent_brain_root().

563-616: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Reuse the install-time brain target during uninstall
The install manifest records a per-agent signature that embeds the install-time brain path (along with config_path and sha256_after_install). cmd_uninstall recomputes brain_dir from current args/env (_resolve_agent_brain_root(args)) instead of using the persisted install-time targeting, so uninstall can search for a hook signature that doesn’t match what was originally installed (e.g., when --brain or the implicit CWD differs).

🤖 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 563 - 616, cmd_uninstall recomputes
brain_dir via _resolve_agent_brain_root(args) instead of using the install-time
brain target recorded in the manifest, so uninstall may look up the wrong hook
signature; fix by, after calling get_record(name), extracting the install-time
brain path from record.signature (the stored install-time target) and use that
as the brain_dir passed to adapter.uninstall when a record exists, falling back
to _resolve_agent_brain_root(args) only if no record is present; update calls
referencing brain_dir (notably the adapter.uninstall invocation) to use the
chosen install_brain_dir and keep dropping the manifest via drop_record
unchanged.
🤖 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/cli.py`:
- Around line 1198-1214: The path resolution currently prefers CLI args over
environment variables in _resolve_path_with_precedence, which breaks the repo's
env-first contract; change the logic in _resolve_path_with_precedence (or call
sites like _get_brain) to check env_names (ENV: "BRAIN_DIR", "GRADATA_BRAIN")
before arg_names ("brain_dir", "brain") so that env_str(name) wins over
getattr(args, name) and only fall back to the caller default Path when neither
env nor CLI provide a value.
- Around line 2045-2050: Update the help text for the "--brain" argument in both
p_install.add_argument (the install command) and the corresponding uninstall
parser (p_uninstall.add_argument) so it mentions the GRADATA_BRAIN environment
variable in addition to BRAIN_DIR and the default path; modify the help strings
referenced in the "--brain" argument declarations to read something like "Brain
directory for agent hook config (default: GRADATA_BRAIN or BRAIN_DIR or
~/.gradata/brain)" so the CLI help matches the behavior of
_resolve_agent_brain_root().
- Around line 563-616: cmd_uninstall recomputes brain_dir via
_resolve_agent_brain_root(args) instead of using the install-time brain target
recorded in the manifest, so uninstall may look up the wrong hook signature; fix
by, after calling get_record(name), extracting the install-time brain path from
record.signature (the stored install-time target) and use that as the brain_dir
passed to adapter.uninstall when a record exists, falling back to
_resolve_agent_brain_root(args) only if no record is present; update calls
referencing brain_dir (notably the adapter.uninstall invocation) to use the
chosen install_brain_dir and keep dropping the manifest via drop_record
unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5ec0b9c8-8eb8-469d-aa98-4617504af8b0

📥 Commits

Reviewing files that changed from the base of the PR and between 7db910c and c5a9a42.

📒 Files selected for processing (3)
  • Gradata/src/gradata/cli.py
  • Gradata/src/gradata/hooks/adapters/claude_code.py
  • Gradata/src/gradata/hooks/adapters/hermes.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 ubuntu-latest / py3.11
  • GitHub Check: pytest windows-latest / py3.12
  • GitHub Check: pytest macos-latest / py3.12
  • GitHub Check: pytest windows-latest / py3.11
  • GitHub Check: pytest ubuntu-latest / py3.12
  • GitHub Check: pytest macos-latest / py3.11
  • GitHub Check: pytest (py3.12)
  • GitHub Check: pytest (py3.11)
🧰 Additional context used
📓 Path-based instructions (1)
Gradata/src/**/*.py

📄 CodeRabbit inference engine (Gradata/AGENTS.md)

Gradata/src/**/*.py: Prefer sentence-transformers for local embeddings, google-genai for Gemini embeddings, cryptography for AES-GCM encrypted system.db, bm25s for BM25 rule ranking, and mem0ai for external memory adapters — guard all optional dependency imports with try / except ImportError at 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 bare except: pass — use typed exceptions or at minimum logger.warning(...) with exc_info=True to avoid silent failure in a memory product
Never import from out-of-scope sibling directories ../Sprites/ or ../Hausgem/ within gradata/* 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 inside gradata/*
Use atomic-write helper when writing JSON files to prevent corruption from mid-write crashes

Files:

  • Gradata/src/gradata/hooks/adapters/hermes.py
  • Gradata/src/gradata/hooks/adapters/claude_code.py
  • Gradata/src/gradata/cli.py
🧠 Learnings (4)
📓 Common learnings
Learnt from: Gradata
Repo: Gradata/gradata PR: 0
File: :0-0
Timestamp: 2026-04-17T17:18:07.439Z
Learning: In PR `#102` (gradata/gradata), Round 2 addressed: cli.py env-first brain resolution (GRADATA_BRAIN > --brain-dir > cwd), _tenant.py corrupt .tenant_id overwrite, _env_int default clamping to minimum, and _events.py tenant-scoped fallback SELECT for dedup. All ruff and 99 tests green after these fixes.
📚 Learning: 2026-04-17T17:18:07.439Z
Learnt from: Gradata
Repo: Gradata/gradata PR: 0
File: :0-0
Timestamp: 2026-04-17T17:18:07.439Z
Learning: In PR `#102` (gradata/gradata), Round 2 addressed: cli.py env-first brain resolution (GRADATA_BRAIN > --brain-dir > cwd), _tenant.py corrupt .tenant_id overwrite, _env_int default clamping to minimum, and _events.py tenant-scoped fallback SELECT for dedup. All ruff and 99 tests green after these fixes.

Applied to files:

  • Gradata/src/gradata/cli.py
📚 Learning: 2026-05-01T15:50:32.772Z
Learnt from: CR
Repo: Gradata/gradata PR: 0
File: Gradata/AGENTS.md:0-0
Timestamp: 2026-05-01T15:50:32.772Z
Learning: Use `from gradata import Brain` as the public entry point — `brain.correct()` is THE entry point for the headline product promise

Applied to files:

  • Gradata/src/gradata/cli.py
📚 Learning: 2026-05-01T15:50:32.772Z
Learnt from: CR
Repo: Gradata/gradata PR: 0
File: Gradata/AGENTS.md:0-0
Timestamp: 2026-05-01T15:50:32.772Z
Learning: Applies to Gradata/tests/**/*.py : Set `BRAIN_DIR` environment variable via `tmp_path` in conftest.py for test isolation — ensure `_paths.py` module cache refreshes when calling `Brain.init()` directly inside tests

Applied to files:

  • Gradata/src/gradata/cli.py
🔇 Additional comments (5)
Gradata/src/gradata/cli.py (3)

35-43: LGTM!


457-462: LGTM!


867-872: LGTM!

Gradata/src/gradata/hooks/adapters/claude_code.py (1)

94-99: LGTM!

Gradata/src/gradata/hooks/adapters/hermes.py (1)

240-244: LGTM!

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
Gradata/src/gradata/hooks/stale_hook_check.py (1)

274-283: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Match the repair command’s env precedence to the CLI resolver.

This block still prefers BRAIN_DIR before GRADATA_BRAIN, but cli.py now resolves the agent brain in the opposite order. When both vars are set, the warning can suggest reinstalling hooks to a different brain than gradata install --agent will actually use.

♻️ Proposed fix
-        target_brain = env_str("BRAIN_DIR") or env_str("GRADATA_BRAIN") or "~/.gradata/brain"
+        target_brain = env_str("GRADATA_BRAIN") or env_str("BRAIN_DIR") or "~/.gradata/brain"
🤖 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/stale_hook_check.py` around lines 274 - 283, The
warning builds target_brain with env_str("BRAIN_DIR") before
env_str("GRADATA_BRAIN"), which mismatches cli.py's resolver; change the
precedence so target_brain uses env_str("GRADATA_BRAIN") first, then
env_str("BRAIN_DIR") (i.e., swap the two env_str calls used to set target_brain
in stale_hook_check.py), keep the fallback "~/.gradata/brain" and still wrap the
final value with shlex.quote when composing the suggested gradata install
command.
Gradata/src/gradata/cli.py (1)

1236-1243: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Resolve agent brain roots to an absolute path before persisting them.

This resolver currently returns the raw CLI/env value. If a user installs hooks with a relative --brain or GRADATA_BRAIN, that relative path gets embedded into the agent config and manifest signature, so later hook executions resolve it against whatever project cwd the agent is running from. That can send corrections to the wrong brain and makes uninstall non-symmetric for the same install.

♻️ Proposed fix
 def _resolve_agent_brain_root(args):
     """Resolve the brain path embedded into agent hook configs.
@@
-    return _resolve_path_with_precedence(args, default=Path.home() / ".gradata" / "brain")
+    return (
+        _resolve_path_with_precedence(args, default=Path.home() / ".gradata" / "brain")
+        .expanduser()
+        .resolve()
+    )
🤖 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 1236 - 1243, The resolver
_resolve_agent_brain_root currently returns whatever
_resolve_path_with_precedence yields (which may be a relative path); update
_resolve_agent_brain_root to take the returned value and convert it to an
absolute, user-expanded path (e.g., Path(result).expanduser().resolve()) before
returning so that any CLI/env-provided brain (or the default
Path.home()/".gradata"/"brain") is persisted as an absolute path; keep using
_resolve_path_with_precedence for precedence logic but ensure the final return
is the resolved absolute Path.
🤖 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/tests/test_uninstall_command.py`:
- Around line 153-168: Add a pre-check asserting the installed manifest
signature exists in the config before calling cmd_uninstall: after calling
claude_code.install(installed, cfg) and record_install("claude-code", cfg,
hook_signature("claude-code", installed)), assert that
hook_signature("claude-code", installed) is present in
cfg.read_text(encoding="utf-8") to ensure the test fails if the install step
didn't actually write the expected signature; then proceed with
cmd_uninstall(SimpleNamespace(agent="claude-code", brain=str(wrong_arg))) and
the existing post-uninstall assertion.

---

Outside diff comments:
In `@Gradata/src/gradata/cli.py`:
- Around line 1236-1243: The resolver _resolve_agent_brain_root currently
returns whatever _resolve_path_with_precedence yields (which may be a relative
path); update _resolve_agent_brain_root to take the returned value and convert
it to an absolute, user-expanded path (e.g.,
Path(result).expanduser().resolve()) before returning so that any
CLI/env-provided brain (or the default Path.home()/".gradata"/"brain") is
persisted as an absolute path; keep using _resolve_path_with_precedence for
precedence logic but ensure the final return is the resolved absolute Path.

In `@Gradata/src/gradata/hooks/stale_hook_check.py`:
- Around line 274-283: The warning builds target_brain with env_str("BRAIN_DIR")
before env_str("GRADATA_BRAIN"), which mismatches cli.py's resolver; change the
precedence so target_brain uses env_str("GRADATA_BRAIN") first, then
env_str("BRAIN_DIR") (i.e., swap the two env_str calls used to set target_brain
in stale_hook_check.py), keep the fallback "~/.gradata/brain" and still wrap the
final value with shlex.quote when composing the suggested gradata install
command.
🪄 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: fbfa4a18-6278-4c85-9edd-e4211e9e8640

📥 Commits

Reviewing files that changed from the base of the PR and between c5a9a42 and d1e55bc.

📒 Files selected for processing (5)
  • Gradata/src/gradata/cli.py
  • Gradata/src/gradata/hooks/stale_hook_check.py
  • Gradata/tests/test_hermes_hook_event_names.py
  • Gradata/tests/test_hook_adapters.py
  • Gradata/tests/test_uninstall_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 (py3.12)
  • GitHub Check: pytest (py3.11)
  • GitHub Check: pytest macos-latest / py3.12
  • GitHub Check: pytest windows-latest / py3.12
  • GitHub Check: pytest ubuntu-latest / py3.12
  • GitHub Check: pytest ubuntu-latest / py3.11
  • GitHub Check: pytest windows-latest / py3.11
  • GitHub Check: pytest macos-latest / py3.11
🧰 Additional context used
📓 Path-based instructions (2)
Gradata/tests/**/*.py

📄 CodeRabbit inference engine (Gradata/AGENTS.md)

Gradata/tests/**/*.py: Set BRAIN_DIR environment variable via tmp_path in conftest.py for test isolation — ensure _paths.py module cache refreshes when calling Brain.init() directly inside tests
Add unit tests in tests/test_*.py for every CI push without LLM calls (deterministic); mark integration tests with @pytest.mark.integration and skip them by default (they hit real LLM APIs)

Files:

  • Gradata/tests/test_uninstall_command.py
  • Gradata/tests/test_hook_adapters.py
  • Gradata/tests/test_hermes_hook_event_names.py
Gradata/src/**/*.py

📄 CodeRabbit inference engine (Gradata/AGENTS.md)

Gradata/src/**/*.py: Prefer sentence-transformers for local embeddings, google-genai for Gemini embeddings, cryptography for AES-GCM encrypted system.db, bm25s for BM25 rule ranking, and mem0ai for external memory adapters — guard all optional dependency imports with try / except ImportError at 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 bare except: pass — use typed exceptions or at minimum logger.warning(...) with exc_info=True to avoid silent failure in a memory product
Never import from out-of-scope sibling directories ../Sprites/ or ../Hausgem/ within gradata/* 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 inside gradata/*
Use atomic-write helper when writing JSON files to prevent corruption from mid-write crashes

Files:

  • Gradata/src/gradata/hooks/stale_hook_check.py
  • Gradata/src/gradata/cli.py
🧠 Learnings (4)
📓 Common learnings
Learnt from: Gradata
Repo: Gradata/gradata PR: 0
File: :0-0
Timestamp: 2026-04-17T17:18:07.439Z
Learning: In PR `#102` (gradata/gradata), Round 2 addressed: cli.py env-first brain resolution (GRADATA_BRAIN > --brain-dir > cwd), _tenant.py corrupt .tenant_id overwrite, _env_int default clamping to minimum, and _events.py tenant-scoped fallback SELECT for dedup. All ruff and 99 tests green after these fixes.
📚 Learning: 2026-04-17T17:18:07.439Z
Learnt from: Gradata
Repo: Gradata/gradata PR: 0
File: :0-0
Timestamp: 2026-04-17T17:18:07.439Z
Learning: In PR `#102` (gradata/gradata), Round 2 addressed: cli.py env-first brain resolution (GRADATA_BRAIN > --brain-dir > cwd), _tenant.py corrupt .tenant_id overwrite, _env_int default clamping to minimum, and _events.py tenant-scoped fallback SELECT for dedup. All ruff and 99 tests green after these fixes.

Applied to files:

  • Gradata/src/gradata/hooks/stale_hook_check.py
  • Gradata/tests/test_hook_adapters.py
  • Gradata/src/gradata/cli.py
📚 Learning: 2026-05-01T15:50:32.772Z
Learnt from: CR
Repo: Gradata/gradata PR: 0
File: Gradata/AGENTS.md:0-0
Timestamp: 2026-05-01T15:50:32.772Z
Learning: Applies to Gradata/tests/**/*.py : Set `BRAIN_DIR` environment variable via `tmp_path` in conftest.py for test isolation — ensure `_paths.py` module cache refreshes when calling `Brain.init()` directly inside tests

Applied to files:

  • Gradata/src/gradata/hooks/stale_hook_check.py
  • Gradata/tests/test_hook_adapters.py
  • Gradata/src/gradata/cli.py
  • Gradata/tests/test_hermes_hook_event_names.py
📚 Learning: 2026-05-01T15:50:32.772Z
Learnt from: CR
Repo: Gradata/gradata PR: 0
File: Gradata/AGENTS.md:0-0
Timestamp: 2026-05-01T15:50:32.772Z
Learning: Use `from gradata import Brain` as the public entry point — `brain.correct()` is THE entry point for the headline product promise

Applied to files:

  • Gradata/src/gradata/cli.py
🔇 Additional comments (6)
Gradata/tests/test_hermes_hook_event_names.py (1)

50-52: The brittle exact-indentation assertions were replaced with whitespace-insensitive line.strip() comparisons, which resolves the earlier concern.

Gradata/tests/test_uninstall_command.py (1)

183-184: capsys.readouterr() is now captured once and reused for both stdout and stderr, fixing the previously-flagged double-drain bug.

Gradata/tests/test_hook_adapters.py (2)

14-74: LGTM!


77-115: LGTM!

Gradata/src/gradata/cli.py (1)

567-585: LGTM!

Also applies to: 1201-1228, 2060-2097

Gradata/src/gradata/hooks/stale_hook_check.py (1)

140-161: LGTM!

Comment thread Gradata/tests/test_uninstall_command.py

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant