Skip to content

feat(cli): add 'gradata prove' subcommand (GRA-1209 / GH #206)#210

Merged
Gradata merged 2 commits into
mainfrom
feat/gradata-prove-subcommand
May 20, 2026
Merged

feat(cli): add 'gradata prove' subcommand (GRA-1209 / GH #206)#210
Gradata merged 2 commits into
mainfrom
feat/gradata-prove-subcommand

Conversation

@Gradata
Copy link
Copy Markdown
Owner

@Gradata Gradata commented May 20, 2026

Summary

Adds gradata prove — statistical evidence the brain is improving output quality, designed as a CI signal: gradata prove && deploy.

What it computes

  • Corrections-per-session slope via linear regression: negative = converging (brain is learning), positive = diverging (brain needs tuning)
  • Rule application count in the window — if corrections >5 and applications =0, warns that rule injection may be broken (diagnostic gold)
  • Top 5 most-applied rules — proves the brain is actually being USED, not just stored
  • Top 5 most-failed rules — with tune/forget recommendation

Exit codes (CI-usable)

Exit When
0 Converging (slope < -0.05) OR stable OR not enough data
1 Diverging (slope > 0.05) — CI failure

Use as: gradata prove --window 7d && deploy

Live output on oliver-admin

Brain: /home/olive/.gradata/brain
Window: 30d (30d)

Corrections per session:
  Sessions: 1
  Total corrections: 50
  Mean: 50.0/session
  Trend: need >=3 sessions to estimate

Rule applications: 0 in window
  WARNING: lots of corrections but zero rule applications — rules may
  not be reaching session-start injection. Run `gradata doctor` to debug.

The warning correctly identified a real issue in oliver-admin's rule-injection pipeline. That's the diagnostic value of this command.

Test plan

pytest tests/test_prove_command.py
=> 6 passed in 1.16s

6 cases: no-events handling, converging exits 0, diverging exits 1, top rules surface, warning fires on apps=0 corr>5, window filter excludes old events.

Epic context

GRA-1198 (kill the plugin) / GH #206. Companion to PRs #208 (status) and #209 (forget). Replaces plugin /gradata:prove slash command.

Layering

Only reads from system.db via raw sqlite3 (Layer 0) + uses brain.db_path (Layer 2 public attribute). No Layer 0 → 2 imports.

Risk

Low. Pure read-only diagnostic command. Pre-existing lint warnings in cli.py are NOT touched by this PR.

Statistical evidence the brain is improving output quality. Designed as
a CI signal: 'gradata prove && deploy' fails when the brain is diverging.

Reads CORRECTION + LESSON_APPLIED + RULE_FAILURE events from system.db
and computes:
- Corrections-per-session linear regression slope
  (negative = converging, positive = diverging)
- Total rule applications in the window
- Top 5 most-applied rules (proves the brain is USED, not just stored)
- Top 5 most-failed rules (with 'tune/forget' recommendation)

Window flag: --window {7d,30d,90d,all} (default 30d).

Exit codes:
- 0: converging (slope < -0.05) OR stable OR not enough data (n<3)
- 1: diverging (slope > 0.05) — CI failure signal

Diagnostic value: when the brain has corrections but ZERO rule
applications, prints a 'WARNING: rules may not be reaching session-start
injection' line — this surfaces the same install/hook-firing bugs that
'gradata doctor' covers but in the throughput-trend context.

Replaces plugin /gradata:prove slash command (GRA-1198 epic).

Test plan: pytest tests/test_prove_command.py
=> 6 passed in 1.16s

6 cases: no events, converging exits 0, diverging exits 1, top rules
appear, warning fires when apps=0 corr>5, window filter excludes old.

Live verification on oliver-admin brain (50 corrections, 1 session,
0 rule applications) — the warning correctly fires, identifying a real
issue in the rule-injection pipeline that warrants a follow-up.

Layering: only reads from system.db via raw sqlite3 (Layer 0 primitive)
and uses brain.db_path (Layer 2 public attribute). No Layer 0 -> 2
imports.
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 20, 2026

Review Change Stack

📝 Walkthrough
  • New gradata prove CLI subcommand for CI/CD signals (e.g., gradata prove --window 7d && deploy) to assess brain quality improvement
  • CI-friendly exit codes: 0 = converging / stable / insufficient data (n<3), 1 = diverging (slope > 0.05)
  • Computes corrections-per-session trend via linear regression over system.db events (CORRECTION, LESSON_APPLIED, RULE_FAILURE)
  • Reports rule application count, top 5 most-applied rules, top 5 most-failed rules with tune/forget guidance, and warns when corrections > 5 but applications = 0
  • Supports window filtering: --window {7d,30d,90d,all}
  • Read-only, low-risk implementation: reads SQLite system.db via sqlite3 (Layer 0) and uses public brain.db_path (Layer 2); no Layer 0→2 imports
  • Tests added: 6 pytest cases covering no-events, converging/diverging detection (exit codes), top-rule reporting, warning on apps=0, and window filtering
  • New public function: cmd_prove(args) (CLI handler); test helper functions added in tests
  • Minor changes: installation verification path imports and datetime UTC usage
  • No breaking changes or security fixes noted

Walkthrough

Adds a prove CLI command that analyzes brain SQLite events over a configurable time window, computes a corrections-per-session trend via linear regression, reports rule application/failure statistics, and returns exit code 0 or 1; accompanied by pytest coverage that seeds events and asserts outputs and exit codes.

Changes

Prove command: brain event analysis and trend reporting

Layer / File(s) Summary
Prove command implementation and CLI wiring
Gradata/src/gradata/cli.py
Adds cmd_prove to open system.db, compute a time cutoff, query CORRECTION/LESSON_APPLIED/RULE_FAILURE events, aggregate per-session correction counts and rule apply/failure descriptions, compute a linear-regression slope, print a multi-section report (trend, top applied/failed rules, warnings), and exit 0/1 based on slope and data sufficiency. Also adds UTC import, updates cmd_install verification imports and marker-detection to use lowercased substring matching, registers prove subcommand with --window {7d,30d,90d,all}, and routes dispatch to cmd_prove.
Prove command test suite
Gradata/tests/test_prove_command.py
New pytest module with helpers (_seed_brain, _add_event, _run_prove) that create a temporary SQLite events table, insert CORRECTION/LESSON_APPLIED events, run cmd_prove while capturing output, and assert six scenarios: no events, converging trend (exit 0), diverging trend (exit 1), top-applied rule reporting, warning when corrections exist but no applications, and time-window filtering excluding old events.
sequenceDiagram
  participant CmdProve
  participant SystemDB
  participant TrendAnalyzer
  participant Stdout
  CmdProve->>SystemDB: query events within window
  SystemDB-->>CmdProve: rows (session, ts, type, data)
  CmdProve->>TrendAnalyzer: aggregate sessions, compute slope, collect rule stats
  TrendAnalyzer-->>CmdProve: slope, top-applied, top-failed
  CmdProve->>Stdout: print report and warnings
  CmdProve->>CmdProve: exit with code (0 or 1)
Loading

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • Gradata/gradata#208: Modifies Gradata/src/gradata/cli.py near the same install verification flow and also adds CLI subcommand changes that query correction events from system.db.

Suggested labels

feature

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 38.46% 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 clearly describes the main change: adding a new 'gradata prove' CLI subcommand with relevant ticket/PR references.
Description check ✅ Passed The description is comprehensive and directly related to the changeset, explaining the command's purpose, functionality, exit codes, test coverage, and risk assessment.
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 feat/gradata-prove-subcommand

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):
┌──────────────┐
│ 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.17][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.

@coderabbitai coderabbitai Bot added the feature label May 20, 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: 4

🤖 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 567-587: The code opens the DB with sqlite3.connect(db_path) and
runs a SELECT without protecting against creation or missing tables; change the
connection call to open read-only using URI mode (use db_path as a URI with
uri=True and mode='ro') so the DB is not created if missing, and wrap the query
execution on cur.execute(...)/.fetchall() in a try-except that catches
_sqlite3.OperationalError (or _sqlite3.Error) to print a friendly "Could not
read brain db or missing schema" message and sys.exit(1); update references to
db_path, con, cur, and the SELECT block (the rows =
cur.execute(...)...fetchall()) accordingly.

In `@Gradata/tests/test_prove_command.py`:
- Around line 105-137: The test test_prove_shows_top_applied_rules currently
only seeds LESSON_APPLIED events, so the RULE_FAILURE branch of the prove output
is never exercised; update this test to also seed one or more RULE_FAILURE
events by calling _add_event with etype="RULE_FAILURE" (and appropriate ts,
session, and data like {"lesson_description": "..."}), then call
_run_prove(brain_dir, capsys) as before and add assertions that the output
contains the failure section header (e.g., "top failed rules" or the exact
string used by the command) and the failing lesson_description values so the
RULE_FAILURE report path is covered.
- Around line 69-79: The test uses a hardcoded base_ts "2026-05-15T" which will
expire; change the fixture generation to compute recent timestamps from
datetime.now(timezone.utc) (or datetime.utcnow() with tzinfo) and format into
the same string pattern used by _add_event so the timestamps fall inside the
default 30d window; update the block that sets base_ts and the loop that calls
_add_event (referencing base_ts, pattern, _add_event, and _run_prove) to build
each event timestamp from the current UTC date plus desired day/hour/minute
offsets so the test is deterministic for CI without relying on a fixed calendar
date.
- Around line 12-15: Remove the try/except that swallows ImportError so missing
gradata.cli fails the test suite; replace the guarded import with a direct
import of cmd_prove (i.e. remove the try/except and the pytest.skip call so the
statement "from gradata.cli import cmd_prove" runs normally), ensuring an
ImportError surfaces during CI rather than skipping all tests.
🪄 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: 25b48272-4664-4b90-bec9-83ef4735baeb

📥 Commits

Reviewing files that changed from the base of the PR and between 92a4c81 and 3f72430.

📒 Files selected for processing (2)
  • Gradata/src/gradata/cli.py
  • Gradata/tests/test_prove_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 macos-latest / py3.12
  • GitHub Check: pytest windows-latest / py3.11
  • GitHub Check: pytest windows-latest / py3.12
  • GitHub Check: pytest ubuntu-latest / py3.11
  • GitHub Check: pytest macos-latest / py3.11
  • GitHub Check: pytest ubuntu-latest / py3.12
  • GitHub Check: pytest (py3.11)
  • GitHub Check: pytest (py3.12)
🧰 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/cli.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_prove_command.py
🪛 Ruff (0.15.13)
Gradata/src/gradata/cli.py

[warning] 622-622: zip() without an explicit strict= parameter

Add explicit value for parameter strict=

(B905)


[error] 626-626: f-string without any placeholders

Remove extraneous f prefix

(F541)


[error] 633-633: f-string without any placeholders

Remove extraneous f prefix

(F541)


[error] 635-635: f-string without any placeholders

Remove extraneous f prefix

(F541)


[error] 637-637: f-string without any placeholders

Remove extraneous f prefix

(F541)


[error] 639-639: f-string without any placeholders

Remove extraneous f prefix

(F541)

🔇 Additional comments (1)
Gradata/src/gradata/cli.py (1)

1742-1752: LGTM!

Also applies to: 1945-1945

Comment on lines +567 to +587
db_path = str(brain.db_path)
try:
con = _sqlite3.connect(db_path)
cur = con.cursor()
except _sqlite3.OperationalError as exc:
print(f"Could not open brain db: {exc}")
sys.exit(1)

cutoff = datetime.now(UTC) - timedelta(days=window_days)
cutoff_iso = cutoff.isoformat()

# Per-session correction counts in the window
rows = cur.execute(
"""
SELECT session, ts, type
FROM events
WHERE type IN ('CORRECTION', 'LESSON_APPLIED', 'RULE_FAILURE')
AND ts >= ?
""",
(cutoff_iso,),
).fetchall()
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

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the file around the specified lines
head -n 600 Gradata/src/gradata/cli.py | tail -n 100

Repository: Gradata/gradata

Length of output: 3694


🏁 Script executed:

# Also check the full cmd_prove function to understand context
grep -n "def cmd_prove" Gradata/src/gradata/cli.py

Repository: Gradata/gradata

Length of output: 84


🏁 Script executed:

# Search for any error handling after the lines mentioned
sed -n '567,650p' Gradata/src/gradata/cli.py

Repository: Gradata/gradata

Length of output: 3062


🏁 Script executed:

# Check if there's any error handling around the query execution
sed -n '587,650p' Gradata/src/gradata/cli.py

Repository: Gradata/gradata

Length of output: 2468


🏁 Script executed:

# Check how system.db is initialized/created in the codebase
rg -n "system\.db" Gradata/src/gradata/cli.py | head -20

Repository: Gradata/gradata

Length of output: 296


🏁 Script executed:

# Look at brain.db_path definition to understand where it points
rg -n "db_path" Gradata/src/gradata/brain.py | head -20

Repository: Gradata/gradata

Length of output: 1301


🌐 Web query:

sqlite3.connect() default mode behavior read-only vs read-write create database

💡 Result:

In Python, sqlite3.connect(database) opens the SQLite database file in read-write mode (creating it if it does not already exist), unless you explicitly request read-only via the SQLite URI parameter “mode=ro”. 1) Default behavior of sqlite3.connect - Python’s documentation shows that calling sqlite3.connect("tutorial.db") “implicitly creat[es] it if it does not exist” [1]. - At the SQLite C API level, sqlite3_open (which is what the driver uses for “plain file paths” rather than URI-controlled mode) corresponds to opening with READWRITE|CREATE: “This is the behavior that is always used for sqlite3_open and sqlite3_open16.” [2]. So the net effect is: default connect attempts read-write and will create the file if needed. 2) What happens if the file can’t actually be written? - SQLite states that for SQLITE_OPEN_READWRITE it opens for reading and writing “if possible, or reading only if the file is write protected by the operating system.” It also provides sqlite3_db_readonly to detect whether it actually ended up read-only. [2]. Meaning: even though the default is intended to be read-write, the OS/file permissions can cause SQLite to fall back to read-only, so writes may fail at runtime. 3) How to explicitly open read-only - Python’s sqlite3 docs provide an explicit example using a SQLite URI: sqlite3.connect("file:tutorial.db?mode=ro", uri=True), and then attempting to write raises OperationalError: “attempt to write a readonly database”. [1] 4) How to explicitly prevent creating a new database file - Python’s docs also say that if you open a database in a way that prevents implicit creation (e.g., a URI that doesn’t allow create, or other explicit settings), “Do not implicitly create a new database file if it does not already exist; will raise OperationalError if unable to create a new file” [1]. Practical takeaway - sqlite3.connect("path/to/db.sqlite") => intended read-write + create-if-missing [1][2]. - If the directory/file isn’t writable, the connection may end up effectively read-only (writes fail) [2]. - Use file:...?mode=ro & uri=True to force read-only [1].

Citations:


🏁 Script executed:

# Check if there are any other callers of cmd_prove or related context
rg -n "cmd_prove" Gradata/src/gradata/cli.py

Repository: Gradata/gradata

Length of output: 117


🏁 Script executed:

# Verify the docstring vs implementation contract mismatch
sed -n '545,560p' Gradata/src/gradata/cli.py

Repository: Gradata/gradata

Length of output: 765


Use sqlite3.connect(..., uri=True, mode='ro') and wrap query execution in a try-except.

sqlite3.connect(db_path) defaults to read-write mode and creates the database file if it does not exist. For a read-only diagnostic command, this causes an unwanted side effect when system.db is missing. Additionally, if the database exists but is not yet initialized (no events table), the SELECT query fails with an unhandled OperationalError, producing a traceback crash instead of a friendly error message.

Use URI mode 'ro' to enforce read-only access and wrap the query in a try-except to catch schema errors.

🤖 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 567 - 587, The code opens the DB
with sqlite3.connect(db_path) and runs a SELECT without protecting against
creation or missing tables; change the connection call to open read-only using
URI mode (use db_path as a URI with uri=True and mode='ro') so the DB is not
created if missing, and wrap the query execution on cur.execute(...)/.fetchall()
in a try-except that catches _sqlite3.OperationalError (or _sqlite3.Error) to
print a friendly "Could not read brain db or missing schema" message and
sys.exit(1); update references to db_path, con, cur, and the SELECT block (the
rows = cur.execute(...)...fetchall()) accordingly.

Comment on lines +12 to +15
try:
from gradata.cli import cmd_prove
except ImportError:
pytest.skip("gradata SDK not importable", allow_module_level=True)
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

Don’t skip the whole unit suite on ImportError.

If gradata.cli stops importing, this turns a real regression into a green build with all prove tests skipped. In-repo unit tests should fail loudly here instead of masking the breakage.

Suggested change
-try:
-    from gradata.cli import cmd_prove
-except ImportError:
-    pytest.skip("gradata SDK not importable", allow_module_level=True)
+from gradata.cli import cmd_prove
As per coding guidelines, "Add unit tests in `tests/test_*.py` for every CI push without LLM calls (deterministic)".
📝 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
try:
from gradata.cli import cmd_prove
except ImportError:
pytest.skip("gradata SDK not importable", allow_module_level=True)
from gradata.cli import cmd_prove
🤖 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_prove_command.py` around lines 12 - 15, Remove the
try/except that swallows ImportError so missing gradata.cli fails the test
suite; replace the guarded import with a direct import of cmd_prove (i.e. remove
the try/except and the pytest.skip call so the statement "from gradata.cli
import cmd_prove" runs normally), ensuring an ImportError surfaces during CI
rather than skipping all tests.

Comment on lines +69 to +79
base_ts = "2026-05-15T"
pattern = [(1, 10), (2, 8), (3, 5), (4, 3), (5, 2)]
for session, n_corr in pattern:
for j in range(n_corr):
_add_event(
brain_dir,
ts=f"{base_ts}{10 + session:02d}:{j:02d}:00+00:00",
session=session,
etype="CORRECTION",
)
out, exit_code = _run_prove(brain_dir, capsys)
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

These test timestamps expire on June 14, 2026.

All of the "2026-05-15..." fixtures rely on that date staying inside the default 30d window. Once CI runs after June 14, 2026, these assertions start failing for calendar reasons rather than behavior. Generate recent timestamps relative to datetime.now(UTC) instead.

As per coding guidelines, "Add unit tests in tests/test_*.py for every CI push without LLM calls (deterministic)".

Also applies to: 90-100, 109-150

🤖 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_prove_command.py` around lines 69 - 79, The test uses a
hardcoded base_ts "2026-05-15T" which will expire; change the fixture generation
to compute recent timestamps from datetime.now(timezone.utc) (or
datetime.utcnow() with tzinfo) and format into the same string pattern used by
_add_event so the timestamps fall inside the default 30d window; update the
block that sets base_ts and the loop that calls _add_event (referencing base_ts,
pattern, _add_event, and _run_prove) to build each event timestamp from the
current UTC date plus desired day/hour/minute offsets so the test is
deterministic for CI without relying on a fixed calendar date.

Comment on lines +105 to +137
def test_prove_shows_top_applied_rules(tmp_path, capsys):
brain_dir = tmp_path / "brain"
_seed_brain(brain_dir)
# Seed corrections so we have some baseline output
for s in range(3):
_add_event(
brain_dir,
ts=f"2026-05-15T10:0{s}:00+00:00",
session=s + 1,
etype="CORRECTION",
)
# Seed rule applications
for _ in range(5):
_add_event(
brain_dir,
ts="2026-05-15T11:00:00+00:00",
session=1,
etype="LESSON_APPLIED",
data={"lesson_description": "Casual tone"},
)
for _ in range(3):
_add_event(
brain_dir,
ts="2026-05-15T11:01:00+00:00",
session=1,
etype="LESSON_APPLIED",
data={"lesson_description": "Use type hints"},
)
out, _ = _run_prove(brain_dir, capsys)
assert "most-applied rules" in out
assert "Casual tone" in out
assert "Use type hints" in out

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Add coverage for the RULE_FAILURE report path.

The command contract includes “top failed rules,” but this suite never seeds a RULE_FAILURE row. That leaves the new failure-reporting branch untested even though it is user-visible output.

As per coding guidelines, "Add unit tests in tests/test_*.py for every CI push without LLM calls (deterministic)".

🤖 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_prove_command.py` around lines 105 - 137, The test
test_prove_shows_top_applied_rules currently only seeds LESSON_APPLIED events,
so the RULE_FAILURE branch of the prove output is never exercised; update this
test to also seed one or more RULE_FAILURE events by calling _add_event with
etype="RULE_FAILURE" (and appropriate ts, session, and data like
{"lesson_description": "..."}), then call _run_prove(brain_dir, capsys) as
before and add assertions that the output contains the failure section header
(e.g., "top failed rules" or the exact string used by the command) and the
failing lesson_description values so the RULE_FAILURE report path is covered.

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).
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.

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.

♻️ Duplicate comments (1)
Gradata/src/gradata/cli.py (1)

728-747: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Wrap queries in try-except and consider read-only connection mode.

The try-except on lines 728-733 only guards the connection, not the queries. If system.db exists but lacks the events table (fresh brain, schema drift), the SELECT at line 739 will raise OperationalError and crash with a traceback instead of a friendly message.

Compare to cmd_status (lines 172-189) which wraps all queries and falls back gracefully. The same pattern should be applied here—either wrap the query block or use a single outer try-except that catches schema errors and prints a user-friendly message before exiting.

🛡️ Suggested fix: wrap queries with error handling
 db_path = str(brain.db_path)
 try:
     con = _sqlite3.connect(db_path)
     cur = con.cursor()
 except _sqlite3.OperationalError as exc:
     print(f"Could not open brain db: {exc}")
     sys.exit(1)

 cutoff = datetime.now(UTC) - timedelta(days=window_days)
 cutoff_iso = cutoff.isoformat()

 # Per-session correction counts in the window
-rows = cur.execute(
-    """
-    SELECT session, ts, type
-      FROM events
-     WHERE type IN ('CORRECTION', 'LESSON_APPLIED', 'RULE_FAILURE')
-       AND ts >= ?
-    """,
-    (cutoff_iso,),
-).fetchall()
+try:
+    rows = cur.execute(
+        """
+        SELECT session, ts, type
+          FROM events
+         WHERE type IN ('CORRECTION', 'LESSON_APPLIED', 'RULE_FAILURE')
+           AND ts >= ?
+        """,
+        (cutoff_iso,),
+    ).fetchall()
+except _sqlite3.OperationalError as exc:
+    print(f"Could not query events (schema missing?): {exc}")
+    con.close()
+    sys.exit(1)
🤖 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 728 - 747, The try/except currently
only guards the sqlite connection but not the query—wrap the query block that
uses con.cursor() and cur.execute(...) (the SELECT from events that produces
rows) in a try/except catching _sqlite3.OperationalError, print a friendly error
(e.g., "Could not read brain db: <exc>") and sys.exit(1), and ensure the
connection is closed in finally; optionally open the DB in read-only mode by
connecting with the URI form (e.g., "file:{db_path}?mode=ro", uri=True) when
creating con to avoid accidental writes.
🤖 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.

Duplicate comments:
In `@Gradata/src/gradata/cli.py`:
- Around line 728-747: The try/except currently only guards the sqlite
connection but not the query—wrap the query block that uses con.cursor() and
cur.execute(...) (the SELECT from events that produces rows) in a try/except
catching _sqlite3.OperationalError, print a friendly error (e.g., "Could not
read brain db: <exc>") and sys.exit(1), and ensure the connection is closed in
finally; optionally open the DB in read-only mode by connecting with the URI
form (e.g., "file:{db_path}?mode=ro", uri=True) when creating con to avoid
accidental writes.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 027418e1-cb3f-4039-bda7-f7ad78e1f9e6

📥 Commits

Reviewing files that changed from the base of the PR and between 3f72430 and 0da2003.

📒 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). (6)
  • GitHub Check: pytest windows-latest / py3.11
  • GitHub Check: pytest macos-latest / py3.11
  • GitHub Check: pytest ubuntu-latest / py3.12
  • GitHub Check: pytest windows-latest / py3.12
  • GitHub Check: pytest macos-latest / py3.12
  • GitHub Check: pytest ubuntu-latest / 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
🔇 Additional comments (7)
Gradata/src/gradata/cli.py (7)

27-27: LGTM!


144-302: LGTM!


872-965: LGTM!


1867-1868: LGTM!


2001-2010: LGTM!


2012-2028: LGTM!


2208-2208: LGTM!

Also applies to: 2222-2222

Gradata added a commit that referenced this pull request May 20, 2026
…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>
@Gradata Gradata merged commit bc4e066 into main May 20, 2026
9 checks passed
@Gradata Gradata deleted the feat/gradata-prove-subcommand branch May 20, 2026 07:12
Gradata added a commit that referenced this pull request May 20, 2026
…-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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant