feat: rule-to-hook UX — list, remove, events, stale detection#30
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 0 minutes and 32 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (217)
📝 WalkthroughWalkthroughThis PR introduces a manifest-based hook dispatcher system for consolidating rule-to-hook generation, adds CLI commands for rule management (list/remove), implements stale hook detection, and updates the graduation flow to pass brain context to hook generation for improved logging and context awareness. Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI
participant Manifest as Manifest<br/>(_manifest.json)
participant Dispatcher as Dispatcher<br/>(_dispatcher.js)
participant Legacy as Legacy Hooks<br/>(per-rule .js)
User->>CLI: gradata rule add/list/remove
Note over CLI: New rule subcommands<br/>with brain context
User->>CLI: gradata hooks migrate
CLI->>Legacy: Scan legacy *.js files
Legacy-->>CLI: Extract Template, Rule, Source hash
CLI->>Manifest: Upsert entries (idempotent)
Manifest-->>CLI: Migration summary
User->>CLI: gradata rule list
CLI->>Manifest: Read entries
Manifest-->>CLI: Return slug/rule entries
CLI->>Legacy: Check for *.js files
CLI-->>User: Show [hooked]/[STALE]/ORPHAN status
rect rgba(100, 150, 255, 0.5)
Note over Dispatcher,Legacy: Hook Execution (Two-Phase)
end
participant Hook as Hook Runner<br/>(_generated_runner_core.py)
Hook->>Manifest: Check if _manifest.json exists
alt Manifest + Dispatcher exist
Hook->>Dispatcher: Run single node process
Dispatcher->>Manifest: Load entries
Dispatcher->>Dispatcher: Evaluate rules in sequence
Dispatcher-->>Hook: Exit 0 (pass) or 2 (block)
else Fallback to Legacy
Hook->>Legacy: Scan *.js (skip slugs in manifest)
Hook->>Legacy: Run each legacy hook individually
Legacy-->>Hook: Exit 0 or 2
end
Hook-->>User: Emit decision/relay output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Auto-fixable (9) via ruff --fix: - UP017 datetime.timezone.utc -> datetime.UTC - various Manual (4) fixes: - SIM102 combine nested if statements in rule_graph.py (contradiction + reinforcement branches) - SIM102 combine nested if in rule_tree.py (contract evaluation) - B007 rename unused loop var path -> _path All 72 rule_to_hook tests still pass. Co-Authored-By: Gradata <noreply@gradata.ai>
Deploying gradata-dashboard with
|
| Latest commit: |
e45a4e0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c166a637.gradata-dashboard.pages.dev |
| Branch Preview URL: | https://worktree-rule-to-hook-ux.gradata-dashboard.pages.dev |
There was a problem hiding this comment.
Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
- _types.py: restore Lesson._contradiction_streak field (still used by self_improvement.py and rule_evolution.py for contradiction streak tracking) - rules/cache.py: revert RuleCache value type list -> str (brain.apply_rules caches formatted rule strings, not lists) - rules/rule_engine.py: bind getattr results to locals so pyright narrows None-checks correctly for example_draft/example_corrected - behavioral_extractor.py: revert sorted key to lambda k: counts[k] — dict.get returns int|None which pyright rejects as a sort key Pre: 10 errors, 7 warnings. Post: 0 errors, 7 warnings (pre-existing). Ruff clean. 2069 tests pass, 23 skip.
There was a problem hiding this comment.
Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Too many files changed for review. ( |
There was a problem hiding this comment.
Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
When rule_to_hook graduates a deterministic rule into a generated PreToolUse hook, the soft text reminder becomes noise. Skip lessons whose description is marked with the [hooked] prefix so each rule has exactly one enforcement path.
…check templates, expand phrasing
There was a problem hiding this comment.
Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Rebased on main — conflicts resolved in |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/gradata/cli.py (1)
531-532:⚠️ Potential issue | 🟠 MajorRace condition:
lessons.mdwrite lacks file locking.The
cmd_rule_addfunction openslessons.mdin append mode without acquiring the lessons lock. Per the relevant code snippet fromsrc/gradata/_db.py:62-73, concurrent writes tolessons.mdshould uselessons_lock()orwrite_lessons_safe()to prevent data corruption.🔒 Proposed fix using lessons_lock
+ from gradata._db import lessons_lock + lessons.parent.mkdir(parents=True, exist_ok=True) # ... line construction ... - with lessons.open("a", encoding="utf-8") as f: - f.write(line) + with lessons_lock(lessons): + with lessons.open("a", encoding="utf-8") as f: + f.write(line)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/cli.py` around lines 531 - 532, The append to lessons.md in cmd_rule_add currently uses lessons.open("a", ...) without synchronization, risking concurrent write corruption; modify cmd_rule_add to acquire the lessons lock (use lessons_lock()) or call the provided helper write_lessons_safe() when writing the new line instead of directly opening the file so the write is performed under the lessons_lock; locate the write in cmd_rule_add and replace the direct file append with a call that obtains lessons_lock() (or delegates to write_lessons_safe()) to perform the append safely.
♻️ Duplicate comments (3)
src/gradata/hooks/stale_hook_check.py (1)
151-155:⚠️ Potential issue | 🟠 MajorQuote the suggested
gradata rule addcommand.
current_textis interpolated raw into a shell command. Quotes, backticks, or$(...)in the lesson text make the copy-paste fix unsafe.Suggested fix
+import shlex ... - print(f" fix: gradata rule remove {slug} && gradata rule add \"{current_text}\"") + safe_text = shlex.quote(current_text) + print(f" fix: gradata rule remove {slug} && gradata rule add {safe_text}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/hooks/stale_hook_check.py` around lines 151 - 155, The printed fix command interpolates current_text raw into a shell command, which is unsafe if the lesson contains quotes/backticks/$(...), so change the print to safely quote current_text (e.g., import shlex and use shlex.quote(current_text)) when composing the suggested command inside the loop over slug, path, old_hash, new_hash, current_text; ensure the printed line uses the quoted value (not raw current_text) so the copy-paste command is safe.cloud/app/routes/brains.py (1)
122-131:⚠️ Potential issue | 🟠 MajorFail the request if the owner membership is missing.
If
workspace_memberscomes back empty here, we still create the brain and return201. That leaves a new workspace without a confirmed owner row. Please treat this as a hard failure (ideally in the same transaction as the workspace insert) instead of warning and continuing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cloud/app/routes/brains.py` around lines 122 - 131, The current code logs a warning when db.insert into "workspace_members" returns empty (member_rows) but continues and returns 201; change this to treat missing owner membership as a hard failure by raising/returning an error and rolling back the workspace creation instead of warning. Ensure the workspace and workspace_members inserts occur in the same transaction (use the existing db transaction context used for the workspace insert) and if db.insert("workspace_members", {"workspace_id": workspace_id, "user_id": user_id, "role": "owner"}) returns falsy, abort the transaction and raise an HTTP error (or return a 5xx/4xx response) so the brain/workspace creation does not succeed; update any surrounding create brain logic that uses workspace_id to handle the transaction error path.src/gradata/enhancements/self_improvement.py (1)
858-866: 🛠️ Refactor suggestion | 🟠 MajorType the new
brainkwarg.
graduate()just added a new public keyword parameter, but it is still unannotated. Please use the concrete brain type via a forward reference if needed so this stays statically checkable. As per coding guidelines, "type safety" is required for the core SDK (src/gradata/**/*.py).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/enhancements/self_improvement.py` around lines 858 - 866, The new public kwarg brain on graduate() is untyped; annotate it with the concrete Brain type using a forward reference and avoid runtime imports by guarding the import with typing.TYPE_CHECKING (e.g. add "from typing import TYPE_CHECKING, Optional" and "if TYPE_CHECKING: from gradata.brains import Brain"), then change the signature to accept Optional['Brain'] (or 'Brain | None' if using PEP 604 and from __future__ import annotations is enabled) and keep the default of None so static checkers can validate usages of graduate(brain=...).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/gradata/_lessons.py`:
- Around line 39-49: parse_rule_lesson currently only recognizes the legacy
_HOOKED_MARKER prefix and misses structured metadata like 'Metadata:
{"how_enforced":"hooked"}'; update parse_rule_lesson to also detect and parse
inline structured metadata in raw_desc (e.g., a "Metadata:" JSON object),
extract the "how_enforced" field and set hooked=True when it equals "hooked",
and remove that metadata fragment from desc before returning the RuleLesson
(keep existing _HOOKED_MARKER handling intact so both legacy and structured
formats are supported).
In `@src/gradata/cli.py`:
- Around line 661-682: The read-modify-write on lessons.md in cmd_rule_remove is
not protected by the lessons lock causing TOCTOU races; wrap the entire block
that reads lessons_file, parses lines (using parse_rule_lesson/_slug), builds
out_lines and sets touched_lesson, and writes back the file in a lessons_lock()
context (or replace the final write with write_lessons_safe() that acquires the
same lock) so the read and subsequent write happen atomically and no concurrent
CLI operation can clobber the file.
In `@src/gradata/enhancements/rule_to_hook.py`:
- Around line 294-299: The _hook_root function currently returns a hardcoded
default Path(" .claude/hooks/pre-tool/generated") (and similar hardcoded
defaults around lines 322-328); replace these literal defaults by delegating to
a shared path/config resolver (e.g. a central get_default_path or
resolve_hook_root function in the shared config/paths module) and only fallback
to the environment override if present—i.e., call the shared resolver to obtain
the default hook root instead of embedding the string literal, keep the existing
os.environ lookup but have it override the resolver result, and update any other
functions in this file that use the same hardcoded strings to use the shared
resolver (referencing _hook_root and the other hook-related helpers) so no
hardcoded path remains in core SDK code.
In `@src/gradata/hooks/_generated_runner_core.py`:
- Around line 79-105: The code sets dispatcher_ran = True before attempting to
run the dispatcher, causing manifest_slugs to be read (and legacy hooks skipped)
even when the dispatcher never executed or didn't actually process templates;
fix by only marking the dispatcher as having "ran" after a successful run—move
or change the assignment so dispatcher_ran is set based on the subprocess result
(e.g., set dispatcher_ran = True only if proc is not None and proc.returncode ==
0) in the block around subprocess.run (refer to dispatcher.exists(),
subprocess.run call, proc variable, and the manifest_slugs =
_read_manifest_slugs(manifest) if dispatcher_ran else set() line) so the
fallback loop only skips manifest-backed legacy hooks when the dispatcher truly
ran and covered them.
In `@src/gradata/hooks/stale_hook_check.py`:
- Around line 128-141: When the manifest entry's slug is missing from
lessons_by_slug the code currently skips and therefore misses slug-drift cases;
change the logic in the loop over _mf.read_manifest(kind) so that if
lessons_by_slug.get(slug) is None you compute the _source_hash for every lesson
in lessons_by_slug (or maintain a current_hash -> slug map) and look for a
matching current_hash == recorded to detect a moved/renamed lesson; when found,
treat it as stale (append to stale with the original recorded hash and the found
current_hash/current_text and include both old and new slug info in the
pseudo_path or tuple) instead of continuing, using the existing variables and
functions (_source_hash, lessons_by_slug, stale.append, pseudo_path, d) to keep
drift detection working for bundled-only installs.
In `@tests/test_rule_to_hook.py`:
- Around line 868-870: The tests seed legacy "[hooked]" header fixtures via the
helper _write_lessons which means they only exercise the deprecated header path;
update those fixtures to use the current structured metadata format instead
(e.g. replace lines like "[2026-04-13] [RULE:1.00] FORMATTING: [hooked] ..."
with the new structured metadata entry such as a JSON/YAML-style metadata block
containing "how_enforced": "hooked" or whatever format TestCliRuleAdd expects),
and apply the same change to every occurrence mentioned (around the calls that
write lessons at the spots you noted, including the ones near lines 903-904,
964-965, 1117-1118, 1154-1155) so list/remove/stale tests exercise the current
path rather than the legacy header format; keep using the existing helper
_write_lessons and only change the fixture content strings.
---
Outside diff comments:
In `@src/gradata/cli.py`:
- Around line 531-532: The append to lessons.md in cmd_rule_add currently uses
lessons.open("a", ...) without synchronization, risking concurrent write
corruption; modify cmd_rule_add to acquire the lessons lock (use lessons_lock())
or call the provided helper write_lessons_safe() when writing the new line
instead of directly opening the file so the write is performed under the
lessons_lock; locate the write in cmd_rule_add and replace the direct file
append with a call that obtains lessons_lock() (or delegates to
write_lessons_safe()) to perform the append safely.
---
Duplicate comments:
In `@cloud/app/routes/brains.py`:
- Around line 122-131: The current code logs a warning when db.insert into
"workspace_members" returns empty (member_rows) but continues and returns 201;
change this to treat missing owner membership as a hard failure by
raising/returning an error and rolling back the workspace creation instead of
warning. Ensure the workspace and workspace_members inserts occur in the same
transaction (use the existing db transaction context used for the workspace
insert) and if db.insert("workspace_members", {"workspace_id": workspace_id,
"user_id": user_id, "role": "owner"}) returns falsy, abort the transaction and
raise an HTTP error (or return a 5xx/4xx response) so the brain/workspace
creation does not succeed; update any surrounding create brain logic that uses
workspace_id to handle the transaction error path.
In `@src/gradata/enhancements/self_improvement.py`:
- Around line 858-866: The new public kwarg brain on graduate() is untyped;
annotate it with the concrete Brain type using a forward reference and avoid
runtime imports by guarding the import with typing.TYPE_CHECKING (e.g. add "from
typing import TYPE_CHECKING, Optional" and "if TYPE_CHECKING: from
gradata.brains import Brain"), then change the signature to accept
Optional['Brain'] (or 'Brain | None' if using PEP 604 and from __future__ import
annotations is enabled) and keep the default of None so static checkers can
validate usages of graduate(brain=...).
In `@src/gradata/hooks/stale_hook_check.py`:
- Around line 151-155: The printed fix command interpolates current_text raw
into a shell command, which is unsafe if the lesson contains
quotes/backticks/$(...), so change the print to safely quote current_text (e.g.,
import shlex and use shlex.quote(current_text)) when composing the suggested
command inside the loop over slug, path, old_hash, new_hash, current_text;
ensure the printed line uses the quoted value (not raw current_text) so the
copy-paste command is safe.
🪄 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: 55afb646-1f1e-4151-848d-3aaeeef404fe
📒 Files selected for processing (11)
cloud/app/routes/brains.pysrc/gradata/_lessons.pysrc/gradata/_types.pysrc/gradata/cli.pysrc/gradata/enhancements/rule_to_hook.pysrc/gradata/enhancements/self_improvement.pysrc/gradata/hooks/_generated_runner_core.pysrc/gradata/hooks/_manifest.pysrc/gradata/hooks/stale_hook_check.pysrc/gradata/hooks/templates/_dispatcher.jstests/test_rule_to_hook.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (3)
src/gradata/**/*.py
⚙️ CodeRabbit configuration file
src/gradata/**/*.py: This is the core SDK. Check for: type safety (from future import annotations required), no print()
statements (use logging), all functions accepting BrainContext where DB access occurs, no hardcoded paths. Severity
scoring must clamp to [0,1]. Confidence values must be in [0.0, 1.0].
Files:
src/gradata/_types.pysrc/gradata/enhancements/self_improvement.pysrc/gradata/_lessons.pysrc/gradata/hooks/_generated_runner_core.pysrc/gradata/hooks/stale_hook_check.pysrc/gradata/enhancements/rule_to_hook.pysrc/gradata/hooks/_manifest.pysrc/gradata/cli.py
src/gradata/hooks/**
⚙️ CodeRabbit configuration file
src/gradata/hooks/**: JavaScript hooks for Claude Code integration. Check for: no shell injection (no execSync with user
input), temp files must use per-user subdirectory, HTTP calls must have timeouts, errors must be silent (never block
the tool chain).
Files:
src/gradata/hooks/templates/_dispatcher.jssrc/gradata/hooks/_generated_runner_core.pysrc/gradata/hooks/stale_hook_check.pysrc/gradata/hooks/_manifest.py
tests/**
⚙️ CodeRabbit configuration file
tests/**: Test files. Verify: no hardcoded paths, assertions check specific values not just truthiness,
parametrized tests preferred for boundary conditions, floating point comparisons use pytest.approx.
Files:
tests/test_rule_to_hook.py
🔇 Additional comments (15)
src/gradata/_types.py (1)
177-179: Comment update is accurate and aligned with behavior.The revised
_contradiction_streakcomment matches the current reset and acceleration semantics implemented insrc/gradata/enhancements/self_improvement.py.src/gradata/cli.py (6)
497-506: LGTM! Best-effort brain resolution pattern.The try/except with fallback to
Noneis appropriate here since event logging is optional and shouldn't block the rule-add workflow for users without an initialized brain.
540-546: LGTM! Hook directory resolution with env overrides.The helper correctly honors environment variable overrides and uses sensible relative defaults.
548-626: LGTM! Rule list implementation is well-structured.The function correctly distinguishes between:
- File-based legacy hooks
- Manifest-based bundled hooks
- Stale markers (lesson marked hooked but no hook file)
- Orphan hooks (hook file exists but no matching lesson)
The synthesized pseudo-path for manifest-only entries (Line 586) is a clean approach for uniform display.
696-707: LGTM! Clean subcommand dispatcher.The dispatcher pattern is straightforward and handles unknown subcommands gracefully.
721-740: LGTM! Migration command implementation.The migrate action correctly:
- Processes both "pre" and "post" hook kinds
- Aggregates and reports migration statistics
- Refreshes the dispatcher after migration
861-877: LGTM! Argument parser extensions.The new CLI arguments for
hooks migrate --delete-legacyandrule list/removeare well-structured with appropriate help text.src/gradata/hooks/_manifest.py (8)
1-36: LGTM! Well-documented module with clear entry schema.The docstring clearly documents the manifest entry shape, defensive read/write policy, and purpose. The
from __future__ import annotationsimport is present as required.
38-50: LGTM! Hash and path resolution utilities.The
source_hashfunction matches the convention documented inrule_to_hook._source_hash. The_hook_rootfunction correctly handles env overrides with sensible defaults.
60-86: LGTM! Defensive manifest I/O with atomic writes.The implementation follows best practices:
read_manifestnever raises on malformed data and filters to valid entrieswrite_manifestuses atomic tmp-file +os.replacepattern- Dispatcher is automatically deployed on manifest write
89-106: LGTM! Dispatcher deployment with staleness check.The function correctly:
- Skips rewrite if content matches (avoids unnecessary I/O)
- Handles missing template gracefully (best-effort)
- Uses
contextlib.suppressfor chmod to handle permission errors on some platforms
109-140: LGTM! Idempotent upsert implementation.The function correctly implements upsert semantics: replaces existing entry with matching slug or appends if new. The
source_hashis recomputed fromrule_textensuring consistency.
143-174: LGTM! Clean lifecycle helpers.The
remove_entry,find_entry,clear_dispatcher, anddispatcher_installedfunctions are straightforward and correctly implemented.
181-281: LGTM! Robust migration implementation.The migration logic is well-designed:
- Idempotent: re-running skips already-migrated slugs
- Parses legacy hook headers to reconstruct manifest entries
- Uses
json.loadsto correctly unescape RegExp string literals- Preserves
legacy_source_hashfor audit trail- Optional cleanup with
delete_legacyflag
284-305: LGTM! Clean module exports.The
rollback_dispatcherprovides clear semantic naming for emergency scenarios. The__all__list is comprehensive.
| if dispatcher.exists() and manifest.exists(): | ||
| dispatcher_ran = True | ||
| try: | ||
| proc = subprocess.run( | ||
| ["node", str(dispatcher)], | ||
| input=payload_bytes, | ||
| capture_output=True, | ||
| timeout=per_hook_timeout, | ||
| ) | ||
| except (subprocess.TimeoutExpired, FileNotFoundError): | ||
| proc = None | ||
| except Exception: | ||
| proc = None | ||
| if proc is not None and proc.returncode == 2: | ||
| if proc.stdout: | ||
| with contextlib.suppress(Exception): | ||
| sys.stdout.write(_decode(proc.stdout)) | ||
| if proc.stderr: | ||
| with contextlib.suppress(Exception): | ||
| sys.stderr.write(_decode(proc.stderr)) | ||
| return 2 | ||
|
|
||
| for hook_path in hooks: | ||
| # Fallback / compat path: iterate any legacy per-file hooks not represented | ||
| # in the manifest. Skips internal files (dispatcher + manifest). Only read | ||
| # the manifest slugs here — when the dispatcher blocked we already returned, | ||
| # and when no dispatcher is installed there's nothing to skip. | ||
| manifest_slugs = _read_manifest_slugs(manifest) if dispatcher_ran else set() |
There was a problem hiding this comment.
Don't skip manifest-backed legacy hooks unless the dispatcher actually covered them.
manifest_slugs is populated whenever the dispatcher files exist, even if the dispatcher never ran successfully or returned 0 without evaluating a template. That makes the fallback loop skip the legacy .js file and the rule never executes. auto_test is the concrete case here, because src/gradata/hooks/templates/_dispatcher.js always returns false for it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/gradata/hooks/_generated_runner_core.py` around lines 79 - 105, The code
sets dispatcher_ran = True before attempting to run the dispatcher, causing
manifest_slugs to be read (and legacy hooks skipped) even when the dispatcher
never executed or didn't actually process templates; fix by only marking the
dispatcher as having "ran" after a successful run—move or change the assignment
so dispatcher_ran is set based on the subprocess result (e.g., set
dispatcher_ran = True only if proc is not None and proc.returncode == 0) in the
block around subprocess.run (refer to dispatcher.exists(), subprocess.run call,
proc variable, and the manifest_slugs = _read_manifest_slugs(manifest) if
dispatcher_ran else set() line) so the fallback loop only skips manifest-backed
legacy hooks when the dispatcher truly ran and covered them.
| for entry in _mf.read_manifest(kind): | ||
| slug = entry.get("slug") | ||
| if not slug or slug in seen_slugs: | ||
| continue | ||
| recorded = entry.get("source_hash") | ||
| if not recorded: | ||
| continue | ||
| current_text = lessons_by_slug.get(slug) | ||
| if current_text is None: | ||
| continue | ||
| current_hash = _source_hash(current_text) | ||
| if current_hash != recorded: | ||
| pseudo_path = d / f"{slug} (bundled)" | ||
| stale.append((slug, pseudo_path, recorded, current_hash, current_text)) |
There was a problem hiding this comment.
Manifest-only stale detection misses slug drift.
This branch only checks lessons_by_slug.get(slug) and skips when it misses. After hooks migrate --delete-legacy there is no .js file to hit the earlier orphan-pairing path, so editing a rule text enough to change its slug produces no warning at all in bundled-only installs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/gradata/hooks/stale_hook_check.py` around lines 128 - 141, When the
manifest entry's slug is missing from lessons_by_slug the code currently skips
and therefore misses slug-drift cases; change the logic in the loop over
_mf.read_manifest(kind) so that if lessons_by_slug.get(slug) is None you compute
the _source_hash for every lesson in lessons_by_slug (or maintain a current_hash
-> slug map) and look for a matching current_hash == recorded to detect a
moved/renamed lesson; when found, treat it as stale (append to stale with the
original recorded hash and the found current_hash/current_text and include both
old and new slug info in the pseudo_path or tuple) instead of continuing, using
the existing variables and functions (_source_hash, lessons_by_slug,
stale.append, pseudo_path, d) to keep drift detection working for bundled-only
installs.
| self._write_lessons(brain, [ | ||
| "[2026-04-13] [RULE:1.00] FORMATTING: [hooked] never use em dashes", | ||
| "[2026-04-13] [RULE:0.95] STRUCTURE: lead with the answer", |
There was a problem hiding this comment.
These fixtures only cover the deprecated [hooked] header format.
TestCliRuleAdd in this same file asserts that new installs are written as structured metadata ("how_enforced": "hooked"), but the list/remove/stale tests all seed legacy [hooked] ... headers. They won't catch regressions in the current path.
Also applies to: 903-904, 964-965, 1117-1118, 1154-1155
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_rule_to_hook.py` around lines 868 - 870, The tests seed legacy
"[hooked]" header fixtures via the helper _write_lessons which means they only
exercise the deprecated header path; update those fixtures to use the current
structured metadata format instead (e.g. replace lines like "[2026-04-13]
[RULE:1.00] FORMATTING: [hooked] ..." with the new structured metadata entry
such as a JSON/YAML-style metadata block containing "how_enforced": "hooked" or
whatever format TestCliRuleAdd expects), and apply the same change to every
occurrence mentioned (around the calls that write lessons at the spots you
noted, including the ones near lines 903-904, 964-965, 1117-1118, 1154-1155) so
list/remove/stale tests exercise the current path rather than the legacy header
format; keep using the existing helper _write_lessons and only change the
fixture content strings.
…gnore (#50) Untracks 158 files under graphify-out/ and src/gradata/graphify-out/ (~6.6 MB of regenerable third-party knowledge-graph cache), adds matching .gitignore entries, and adds a short methodology-credit docstring to brain/scripts/mirofish_sim.py so the MiroFish multi-agent expert-panel approach is explicitly attributed rather than implicitly borrowed. Tests: 2070 passed, 23 skipped. Co-authored-by: Gradata <noreply@gradata.ai>
…lineage section (#49)
…tall (#53) Ships .claude-plugin/plugin.json + hooks/hooks.json so users can install Gradata via Claude Code's plugin marketplace. Hooks wire into existing gradata.hooks.{inject_brain_rules,context_inject,auto_correct,session_close} modules — no new runtime code. Plugin assumes pipx install gradata. Co-authored-by: Gradata <noreply@gradata.ai>
* feat(dashboard): add computeTimeSaved with honest + fallback formula * feat(dashboard): add computeWoWDelta with sample-size floor * feat(dashboard): add computeRuleStreak with graduated_at fallback * feat(dashboard): extend Lesson type with recurrence_blocked, last_recurrence_at, graduated_at, correction_count * feat(dashboard): extend KpiMetrics with timeSavedMinutes + WoW deltas * feat(dashboard): KpiStrip 5-card layout with Est. Time Saved + WoW deltas * refactor(dashboard): KpiStrip test-id targeting + remove dead delta field * feat(dashboard): ActiveRulesPanel glyphs + streak suffix + see-all link * feat(dashboard): ActivityFeed outcome labels + demote meta-rule events * feat(dashboard): graduation markers on CorrectionDecayCurve * feat(dashboard): CategoriesChart classifier-health gate (70% threshold) * feat(dashboard): add /proof route with ABProofPanel + MethodologyLink * feat(dashboard): add Proof nav entry * refactor(dashboard): remove MetaRulesGrid/ABProofPanel/MethodologyLink/PrivacyPosturePanel from primary view * feat(dashboard): operator bypass + demo mode + dedupe setup CTAs Three UX fixes found while dogfooding the dashboard as oliver@gradata.ai: A. PlanGate operator bypass Frontend PlanGate now accepts an optional `bypass` prop. Wired to isOperatorEmail(profile.email) at 4 call sites (meta-rules, self-healing, team, team/members). Mirrors the backend OPERATOR_DOMAINS allowlist (cloud/app/auth.py:22) so gradata.ai and sprites.ai domains don't see the blur overlay. UX-only — backend still enforces plan gates on data endpoints. B. /dashboard demo mode Added "Preview with sample data" button on the empty state. Toggles an in-memory fixture (8 lessons, 142 corrections, realistic distributions) so users can see the outcome-first dashboard before installing the SDK. Demo banner explains it's sample data. C. Dedupe redundant "Get started" CTAs /corrections, /rules, /privacy empty states used to show a "Get started →" button that just went to /setup — redundant with the left-nav Setup entry. Replaced with inline text pointer so the CTA isn't duplicated. Tests: 95/95 pass (+11 new: 7 operator + 4 PlanGate). Co-Authored-By: Gradata <noreply@gradata.ai> * fix(dashboard): CR round-1 + promote Preview CTA - operator.ts: reject multi-@ inputs to match backend semantics (prevents "user@evil.com@gradata.ai" bypass drift per CR review) - demo-dashboard.ts: compute Date.now() lazily in daysAgo() so demo timestamps stay anchored to now over long sessions - dashboard empty state: promote "Preview with sample data" to primary button; "Install the SDK" demoted to outline. Was burying the demo affordance behind the SDK pitch. - tests: new security case for multi-@ bypass (96 total, all pass) Co-Authored-By: Gradata <noreply@gradata.ai> * feat(dashboard): marketify pass — plain-language labels Replace analyst jargon with human language throughout the dashboard: KpiStrip (5 cards): - Correction Rate → Mistakes Caught - Est. Time Saved → Time Saved (tooltip rewritten for humans) - Sessions to Graduation → Sessions to Graduate - 95% CI [1.9, 2.7] → typically 2–3 sessions - Misfires → False Alarms - Brain Footprint kept (user likes seeing AI brain grow) ActiveRulesPanel: - "Active Rules" → "Your Rules" - "top 8" → "what your AI learned" - Hide raw confidence number (sim research: users ignore it) - INSTINCT/PATTERN/RULE → Watching/Learning/Graduated - "Xd clean" → "N days holding" - "recurred Nd ago" → "slipped Nd ago" - "No graduated rules yet" → "Nothing graduated yet. Keep correcting — rules emerge after 3+ catches." - "See all rules" → "See all your rules" ActivityFeed: - Rule graduated kept (user preference over "locked in") - Rule refined → Rule updated - Slipped → Slipped back - "Standard codified" → "Your team now gets this automatically" - "More corrections this week" → "More fixes this week" - Empty state softened CategoriesChart: - "Corrections by Dimension" → "What You Fix Most" - "recalibrating" empty state → "still figuring out what you fix most" - Dropped "6-dim taxonomy (WAVE2)" internal badge GraduationProgressBar: - "Graduation Pipeline" → "How Your AI Learns" - Tier labels now Watching/Learning/Graduated (human names) - Dropped threshold/avg-confidence numerics from cards - "N lessons total" → "N total" Dashboard header: - "Your brain's learning progress" → "What your AI learned from you" 96/96 tests pass. Co-Authored-By: Gradata <noreply@gradata.ai> * fix(dashboard): CR round-3 — demo activity, recurrence ordering, category keys - Wire demoActivityEvents fixture into ActivityFeed when demoMode is on so the Activity panel populates in the preview path (was empty/live-only). - Align demoAnalytics.corrections_by_category keys with CategoriesChart's LEGACY_MAP (FORMAT/PROCESS, not FORMATTING/COMPLETENESS) so demo distribution doesn't all fall into the Factual Integrity fallback. - Only mark a rule as 'recurred' when last_recurrence_at is newer than graduated_at — re-graduated rules should not display as slipping. - Replace `as any` casts in ActivityFeed.test.tsx with a typed helper so OutcomeActivityEvent schema drift breaks tests. - Add dashboard-page test for the empty-brain → preview demo → exit flow. Co-Authored-By: Gradata <noreply@gradata.ai> --------- Co-authored-by: Gradata <noreply@gradata.ai>
- lessons.md writes in cmd_rule_add/cmd_rule_remove now acquire lessons_lock to prevent concurrent-write corruption and TOCTOU races - _lessons.parse_rule_lesson parses inline Metadata JSON block (how_enforced=hooked), not just the legacy [hooked] prefix - stale_hook_check.py: shlex.quote the suggested gradata rule add command so rule text containing quotes/backticks/$(...) stays safe - stale_hook_check.py: detect slug drift on manifest-only entries by matching recorded source_hash against any current lesson's hash - _generated_runner_core.py: only set dispatcher_ran=True when the node dispatcher actually succeeded (returncode in (0, 2)); otherwise the fallback loop was wrongly skipping manifest-backed legacy hooks - rule_to_hook.py + cli.py + stale_hook_check.py: delegate hook-root defaults to gradata.hooks._manifest._hook_root for a single source of truth; hardcoded .claude/hooks/... strings live in one place now - self_improvement.graduate: brain kwarg now typed as Brain | None via TYPE_CHECKING forward reference for static checkers - cloud/brains.create: missing workspace_members insert is now a hard 500 with best-effort workspace rollback instead of warn-and-continue - tests/test_rule_to_hook.py: add TestSharedLessonParser covering both legacy [hooked] prefix and structured Metadata JSON parsing paths Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Re-rebased on main (post-session merges: #46, #49-54) + addressed CR round 5. CR fixes applied:
Added TestSharedLessonParser covering both parser formats. 2147/2147 tests pass. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review — rebased on latest main, all prior fixes applied |
|
✅ Actions performedReview triggered.
|
…ne/continue exports (#31) * feat: capture draft_text in CORRECTION events (rule-to-hook groundwork) * feat: add regex_replace.js.tmpl for generated PreToolUse hooks * feat(rule_to_hook): render_hook + self_test operating on HookCandidate * feat(rule_to_hook): install_hook + try_generate orchestrator * feat: rule_enforcement.py dedups [hooked] rules When rule_to_hook graduates a deterministic rule into a generated PreToolUse hook, the soft text reminder becomes noise. Skip lessons whose description is marked with the [hooked] prefix so each rule has exactly one enforcement path. * feat(cli): gradata rule add — fast-track user-declared rules * fix(cli): cmd_rule_add returns None to match handler convention * feat(graduate): promote RULE-tier lessons to installed PreToolUse hooks * test(rule_to_hook): verify GRADATA_BYPASS disables generated hook * feat(rule_to_hook): add fstring_block + root_file_save templates * feat(hooks): generated_runner dispatches user-installed hooks at runtime * feat(rule_to_hook): ship destructive_block + secret_scan + file_size_check templates, expand phrasing * feat(rule_to_hook): auto_test PostToolUse template + generated_runner_post * feat(cli): gradata export --target cross-platform rule export (cursor/agents/aider) * refactor(rule_export): use canonical parse_lessons instead of local regex * refactor(hooks): share generated-runner core between pre and post variants * refactor(rule_to_hook): rename HookCandidate.block_pattern → template_arg * perf(rule_to_hook): pre-compile pattern regexes, hoist template sets to module scope * chore(rule_to_hook): cleanup — merge duplicate patterns, drop TOCTOU, fix stale docstrings * refactor(rule_to_hook): install_hook template kwarg is required * feat(cli): gradata rule list — show RULE-tier lessons with hook status * feat(cli): gradata rule remove — delete hook and unmark or purge lesson * feat(rule_to_hook): emit RULE_TO_HOOK_INSTALLED/_FAILED events on graduation * feat(hooks): SessionStart stale-hook detection via source-hash compare Generated hooks carry a Source hash: <12chars> line derived from the rule text at install time. If the user edits the lesson text in lessons.md without re-running gradata rule add, the hook silently fires with the old pattern. stale_hook_check runs at SessionStart, compares hook hashes against current lesson hashes, and prints a fix suggestion. - New module: src/gradata/hooks/stale_hook_check.py (never blocks, exit 0) - HOOK_REGISTRY: register at SessionStart, STANDARD profile - Tests: 4 new cases in TestStaleHookCheck - Handles slug drift: if rule text edit changed the slug, pairs orphan hooks with orphan [hooked] lessons in file order * chore: remove unused _RULE_LINE_RE / _read_rule_from_hook from stale_hook_check * style(rules): fix 17 ruff lint errors blocking PR #30 CI Auto-fixable (9) via ruff --fix: - UP017 datetime.timezone.utc -> datetime.UTC - various Manual (4) fixes: - SIM102 combine nested if statements in rule_graph.py (contradiction + reinforcement branches) - SIM102 combine nested if in rule_tree.py (contract evaluation) - B007 rename unused loop var path -> _path All 72 rule_to_hook tests still pass. Co-Authored-By: Gradata <noreply@gradata.ai> * feat(sdk): add Brain.add_rule API for programmatic rule creation cmd_rule_add hand-formatted [date] [STATE:conf] CATEGORY: desc lines directly into lessons.md. When the lesson schema evolves (e.g. adding new metadata fields like alpha/beta_param), that hand-formatted writer breaks silently — graduated rules pick up the new schema but user-declared rules don't. Brain.add_rule(description, category, state='RULE', confidence=0.90, data=None) routes through parse_lessons / format_lessons — the same canonical code path the graduation pipeline uses. Schema changes auto-propagate. Features: - Duplicate detection (category + whitespace-normalized description) - Confidence clamping to [0.0, 1.0] - Unknown state rejection with clear reason - Optional data dict for extra Lesson fields (root_cause, agent_type, etc) — protected fields (date/state/conf/category/description) can't be overridden via data - Emits LESSON_ADDED event for audit trail - Graceful handling of missing lessons.md (creates it) cmd_rule_add now uses Brain.add_rule when the resolved brain has a system.db, else falls back to parse/format helpers (still no hand-formatting). Uses _resolve_brain_root (not _get_brain) to avoid writing to CWD when GRADATA_BRAIN points elsewhere. 17 new tests covering happy path, rejection, duplicates, confidence clamp, data dict handling, event emission. * fix(hooks): gate generated_runner on GRADATA_HOOK_PROFILE Every other hook in _installer.HOOK_REGISTRY routes through run_hook() which calls should_run(profile) before executing. generated_runner + generated_runner_post bypass run_hook (they use sys.exit(main()) not run_hook(main, meta)), so they were executing generated rule-hooks even under GRADATA_HOOK_PROFILE=minimal. This breaks the minimal profile contract: users who set minimal expect the core learning loop only, but they were still getting every user-declared rule enforced on every Edit/Write/Bash. Fix: check should_run(Profile.STANDARD) at the top of run_generated_hooks — matches the registry entry. Logs a debug line and exits 0 when skipped. 5 new tests cover minimal skip, standard/strict execute, and unset-profile-defaults-to-standard behavior. * feat(sdk): add codex/cline/continue export targets + DEFAULT_PATHS gradata rule export previously supported cursor/agents/aider only — three common AI coding tools (Codex CLI, Cline, Continue.dev) were left out, so users running those had to hand-roll rule files. New targets + default output paths: - codex -> .codex/AGENTS.md (Codex CLI, AGENTS.md convention) - cline -> .clinerules (Cline single-file rules) - continue -> .continue/rules/gradata-rules.md (Continue.dev rules dir) All three use the same parse_lessons pipeline, filter RULE-tier only, strip the internal [hooked] marker, and group rules by category for readability. Output shape is plain markdown (headings + bullets) — idiomatic for each tool's rules format. Also exports a new DEFAULT_PATHS dict so callers can look up the conventional output path for a target without hard-coding strings. CLI --target now accepts the three new choices. 21 new tests cover registry sanity, path uniqueness, empty-brain placeholder, category grouping, [hooked] marker stripping, and RULE-tier filter. Existing cursor/agents/aider regression tests included. * style: ruff --fix for PR #30 * style: ruff --fix for PR #31 * fix(sdk): resolve 10 pyright errors blocking PR #31 CI - RuleCache: retype _cache from dict[str, list] to dict[str, str]; the sole caller (Brain.apply_brain_rules) stores format_rules_for_prompt output, which is str, not list. - Lesson: declare _contradiction_streak: int = 0 as an explicit field so rule_evolution / self_improvement can read+write it without pyright flagging dynamic attribute assignment. Not persisted (format_lessons is field-explicit). - behavioral_extractor: swap sorted(..., key=counts.get) for a lambda — dict.get is overloaded and pyright can't resolve which overload. - rule_engine.format_rules_for_prompt: bind example_draft / _corrected locally so pyright narrows None away before subscripting. pyright src/gradata/ — 0 errors, 7 pre-existing warnings. ruff + 2112 tests + 59 SPEC tests all green. Co-Authored-By: Gradata <noreply@gradata.ai> * fix(types): resolve 10 pyright errors blocking PR #30 CI - _types.py: restore Lesson._contradiction_streak field (still used by self_improvement.py and rule_evolution.py for contradiction streak tracking) - rules/cache.py: revert RuleCache value type list -> str (brain.apply_rules caches formatted rule strings, not lists) - rules/rule_engine.py: bind getattr results to locals so pyright narrows None-checks correctly for example_draft/example_corrected - behavioral_extractor.py: revert sorted key to lambda k: counts[k] — dict.get returns int|None which pyright rejects as a sort key Pre: 10 errors, 7 warnings. Post: 0 errors, 7 warnings (pre-existing). Ruff clean. 2069 tests pass, 23 skip. * refactor(sdk): simplify pass on rule-api-profile-exports branch - rule_export: collapse 4 near-identical grouped-markdown formatters (_format_agents / _format_codex / _format_cline / _format_continue) into a single _format_grouped_markdown(title, rules) helper driven by a _GROUPED_MARKDOWN_TITLES table. ~90 LOC -> ~25 LOC. - cli.cmd_rule_add: drop the hand-rolled parse_lessons/format_lessons fallback. Brain() works without system.db (run_migrations no-ops on a missing db), so always route through Brain.add_rule. Also drop the redundant lessons_path.write_text("") pre-touch. - _types.Lesson: remove duplicate _contradiction_streak field (merge artifact — declared twice in the dataclass body). No behavior change: 43 PR-scope tests + 2171 SDK tests still green. * fix(rule-api): address round-2 CodeRabbit review on PR #31 - brain.add_rule: canonicalize category casing (upper) pre-dedupe/persist - brain.add_rule: wrap read/check/append/write in lessons_lock to kill TOCTOU - brain.record_correction: apply extras first so explicit fields win - cli.cmd_rule_add: check add_rule return value and exit non-zero on failure - cli.cmd_rule_list/remove: regex accepts legacy "[hooked]" token position - cli.cmd_rule_remove: reuse canonical _slug from rule_to_hook (dedup) - rule_export._format_aider: json.dumps for YAML-safe description escaping - rule_to_hook: tighten overly-broad secret regex with preposition+noun anchors - stale_hook_check: route through parse_lessons, shlex.quote printed fix cmd - auto_test.js.tmpl: handle spawnSync res.error / res.status===null gracefully - test_brain_add_rule: use monkeypatch.setenv for BRAIN_DIR (scoped) - test_rule_to_hook: pytestmark skipif node missing --------- Co-authored-by: Oliver Le <oliver@gradata.com> Co-authored-by: Gradata <noreply@gradata.ai>
Summary
Stacks on PR #26. Makes the rule-to-hook feature actually usable in practice by adding visibility, maintenance, observability, and drift detection.
What's new
CLI:
gradata rule list— shows every RULE-tier lesson with hook status. Flags[hooked](installed and matching),[STALE](marked hooked in lessons but no.js),[ORPHAN](.jswith no matching lesson). Also lists every installed hook file and a summary count.gradata rule remove <slug>— deletes the generated.jsfrom whichever dir (pre/post) holds it, strips[hooked]marker from the matching lesson. Lesson stays as soft injection.gradata rule remove <slug> --purge— same, but deletes the lesson entirely.Observability:
RULE_TO_HOOK_INSTALLEDandRULE_TO_HOOK_FAILEDevents now land inevents.jsonlon every graduation attempt. Event source distinguishes"graduate"(pipeline) vs"user_declared"(CLI).try_generate(candidate, *, brain=None, source="graduate")— brain kwarg is optional; tests without brain still work.graduate()now accepts an optionalbrainkwarg;_core.auto_graduatethreads it through.Drift detection:
src/gradata/hooks/stale_hook_check.py— SessionStart hook. For each generated hook, compares theSource hash:header to sha256[:12] of the current lesson text. On mismatch, prints warning + the exactgradata rule remove ... && gradata rule add "..."command to fix.HOOK_REGISTRYunderSessionStartsogradata hooks installpicks it up automatically.Test plan
pytest tests/test_rule_to_hook.py— 72 tests green (11 new in this PR)pytest tests/ --ignore=tests/test_brain_benchmark.py— 2125 passed, 23 skipped, zero regressionsstale_hook_check→ drift warning with correct fix commandrule listoutput shows[hooked],[STALE],[ORPHAN]correctly on a fixture with all three statesrule removeis idempotent;--purgedeletes lesson line; no-slug-match prints "nothing to remove"slug/rule_text/template/hook_path; FAILED hasrule_text/template/reasonKnown limits (file as follow-ups)
rule listcomputes status by iterating both hook dirs and re-slugifying lesson descriptions; no cache. Fine at small scale.Generated with Gradata