Skip to content

test: add install smoke matrix#228

Open
Gradata wants to merge 3 commits into
mainfrom
test/gra-1680-install-smoke-matrix
Open

test: add install smoke matrix#228
Gradata wants to merge 3 commits into
mainfrom
test/gra-1680-install-smoke-matrix

Conversation

@Gradata
Copy link
Copy Markdown
Owner

@Gradata Gradata commented May 26, 2026

Summary

  • Add a pytest smoke matrix for the current gradata install --agent ... path.
  • Covers mocked-home installs for Claude Code, Codex, Hermes, and OpenCode.
  • Pins host config artifacts and supported hook event names; documents why deprecated npm installer is excluded.

Test Plan

  • python3 -m pytest tests/test_install_smoke_matrix.py -q → 4 passed
  • python3 -m pytest tests/test_cli_install_agent.py tests/test_hook_adapters.py tests/test_install_smoke_matrix.py -q → 17 passed

Closes GRA-1680

Copy link
Copy Markdown

@greptile-apps greptile-apps Bot left a comment

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
Copy link
Copy Markdown

coderabbitai Bot commented May 26, 2026

Review Change Stack

📝 Walkthrough
  • Adds new test module Gradata/tests/test_install_smoke_matrix.py: parametrized pytest smoke matrix for gradata install --agent across hosts claude-code, codex, hermes, opencode (cursor entry present but skipped as MCP-only).
  • Introduces HOST_MATRIX constant, helper _run_install(tmp_path, host, brain), and test test_cli_install_smoke_matrix_writes_host_hook_config.
  • Tests run installs in a sandboxed/mock-home environment (clears GRADATA_* env vars, sets HOME/USERPROFILE/XDG_CONFIG_HOME to tmp_path, and PYTHONPATH to src).
  • Validates CLI exit code, host name output, and creation of host-specific config files at host-relative paths containing gradata:<host>: and correct BRAIN_DIR.
  • Host-specific assertions:
    • Claude Code: JSON hooks include expected events and non-empty entries.
    • Codex: TOML pre_tool hook id prefix gradata:codex: and command includes gradata.hooks.inject_brain_rules.
    • Hermes: only supported event names appear; legacy Claude-style event names (pre_tool_use/post_tool_use/session_end) are absent.
    • OpenCode: JSON preTool hook id prefix gradata:opencode: and command includes gradata.hooks.inject_brain_rules.
  • Test results: smoke matrix tests 4 passed; related suite run reported 17 passed.
  • CLI robustness fix: install verification now uses TemporaryDirectory(ignore_cleanup_errors=True), creates the verification brain via Brain.init(...), and ensures the verification Brain is explicitly closed in a finally block (addresses Windows-specific close issue).
  • No breaking changes, no security fixes, and no new public API surface introduced by these changes.

Walkthrough

Parametrized pytest smoke test runs python -m gradata.cli install in an isolated sandbox for multiple hook hosts, verifies successful execution, ensures a host-specific config file is written, and asserts host-specific hook/event contents per format.

Changes

CLI Install Smoke Test

Layer / File(s) Summary
Install verification lifecycle
Gradata/src/gradata/cli.py
Reworked the install verification to use TemporaryDirectory(ignore_cleanup_errors=True), obtain the verification Brain via Brain.init(...), run correct(...) and search(...), and ensure verification_brain.close() in a finally block.
CLI install smoke test matrix
Gradata/tests/test_install_smoke_matrix.py
Adds a parametrized test matrix and helper _run_install() that scrubs env and redirects HOME/PYTHONPATH, runs the installer for each host, asserts success and host presence in stdout, verifies the per-host config file exists containing gradata:<host>: and BRAIN_DIR, and performs host-specific content checks for claude-code, codex, hermes, and opencode.

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs:

  • Gradata/gradata#132: Adds CI workflows running pytest across OS/Python matrices that would exercise the new tests.
  • Gradata/gradata#215: Modifies gradata/cli.py's install path (install manifest changes), touching the same CLI area.
  • Gradata/gradata#189: Previously adjusted transient verification Brain usage in the install verification flow.

Suggested labels:
bug

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% 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 'test: add install smoke matrix' accurately reflects the main change—adding a new pytest smoke test matrix for the install command.
Description check ✅ Passed The description clearly relates to the changeset, detailing the smoke matrix addition, test coverage for multiple agent types, and test plan results.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/gra-1680-install-smoke-matrix

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):
┌──────────────┐
│ Opengrep CLI │
└──────────────┘

�[32m✔�[39m �[1mOpengrep OSS�[0m
�[32m✔�[39m Basic security coverage for first-party code vulnerabilities.

�[1m Loading rules from local config...�[0m
[00.15][ERROR]: Error: exception Glob.Lexer.Syntax_error("malformed glob pattern: missing ']'")
Raised at Glob__Lexer.syntax_error in file "libs/glob/Lexer.mll", line 8, characters 2-26
Called from Glob__Lexer.__ocaml_lex_token_rec in file "libs/glob/Lexer.mll", line 29, characters 26-53
Cal


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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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_install_smoke_matrix.py`:
- Around line 105-108: The assertions that check legacy event names use exact
line matching against legacy_event_lines and miss variants like "pre_tool_use:
[]" or indented keys; update the checks that reference legacy_event_lines and
the string literals "pre_tool_use:", "post_tool_use:", and "session_end:" to
perform substring or regex-style matching against each line (e.g., assert not
any('pre_tool_use:' in line or re.match(r'^\s*pre_tool_use\b', line) for line in
legacy_event_lines)) so the test fails on any line containing those event names
regardless of trailing value or leading whitespace.
🪄 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: a48acd42-ce48-4e9a-8c09-f31ab900049c

📥 Commits

Reviewing files that changed from the base of the PR and between a197bff and 22182e0.

📒 Files selected for processing (1)
  • Gradata/tests/test_install_smoke_matrix.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.11)
  • GitHub Check: pytest (py3.12)
  • GitHub Check: pytest macos-latest / py3.12
  • GitHub Check: pytest ubuntu-latest / py3.12
  • GitHub Check: pytest windows-latest / py3.12
  • GitHub Check: pytest macos-latest / py3.11
  • GitHub Check: pytest ubuntu-latest / py3.11
  • GitHub Check: pytest windows-latest / py3.11
🧰 Additional context used
📓 Path-based instructions (1)
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_install_smoke_matrix.py
🔇 Additional comments (5)
Gradata/tests/test_install_smoke_matrix.py (5)

13-38: LGTM!


60-89: LGTM!


90-104: LGTM!

Also applies to: 109-115


7-7: ⚡ Quick win

Confirm tomllib compatibility (Python >= 3.11)

Project metadata sets requires-python = ">=3.11", so import tomllib is supported and no tomli backport is required.


41-57: Ensure BRAIN_DIR isn’t needed for the subprocess install smoke test when --brain is passed

_run_install() strips existing GRADATA_* env vars and sets HOME/USERPROFILE/XDG_CONFIG_HOME, but does not set BRAIN_DIR. In src/gradata/cli.py, the install path imports Brain and derives a brain_dir from CLI args, but the code shown doesn’t establish whether it relies solely on the --brain argument (vs falling back to BRAIN_DIR). If src/gradata/_paths.py/Brain can consult BRAIN_DIR when resolving the brain root, the test should set it (or the guideline should be scoped specifically to in-process Brain.init() usage).

Comment thread Gradata/tests/test_install_smoke_matrix.py Outdated
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot left a comment

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[bot]
coderabbitai Bot previously approved these changes May 26, 2026
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot left a comment

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 added the bug Something isn't working label May 29, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 522-523: The call to Brain.init(verification_dir) can spawn
interactive onboarding and must be forced non-interactive during install
verification; change the call site that assigns verification_brain to call
Brain.init with an explicit non-interactive flag (e.g.
Brain.init(verification_dir, non_interactive=True) or the equivalent parameter
your Brain.init API exposes such as
interactive=False/force_non_interactive=True) so onboarding will not read from
TTY and the install --agent verification path will not block.
🪄 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: 51670d10-51f3-4163-8aea-097c5f3a04e4

📥 Commits

Reviewing files that changed from the base of the PR and between 051a7fd and b2cbe1f.

📒 Files selected for processing (1)
  • Gradata/src/gradata/cli.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.12
  • GitHub Check: pytest ubuntu-latest / py3.11
  • GitHub Check: pytest macos-latest / py3.11
  • GitHub Check: pytest (py3.12)
  • GitHub Check: pytest macos-latest / py3.12
  • GitHub Check: pytest windows-latest / py3.12
  • GitHub Check: pytest windows-latest / py3.11
  • 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/cli.py

Comment on lines +522 to +523
verification_brain = Brain.init(verification_dir)
try:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Force non-interactive Brain.init in install verification path.

Brain.init(verification_dir) may enter interactive onboarding on TTY stdin, which can stall install --agent when GRADATA_VERIFY_INSTALL is enabled. This verification bootstrap should be explicitly non-interactive.

Proposed fix
-                    verification_brain = Brain.init(verification_dir)
+                    verification_brain = Brain.init(verification_dir, interactive=False)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
verification_brain = Brain.init(verification_dir)
try:
verification_brain = Brain.init(verification_dir, interactive=False)
try:
🤖 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 522 - 523, The call to
Brain.init(verification_dir) can spawn interactive onboarding and must be forced
non-interactive during install verification; change the call site that assigns
verification_brain to call Brain.init with an explicit non-interactive flag
(e.g. Brain.init(verification_dir, non_interactive=True) or the equivalent
parameter your Brain.init API exposes such as
interactive=False/force_non_interactive=True) so onboarding will not read from
TTY and the install --agent verification path will not block.

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