feat: daemon + detection layer + plugin tests#18
Merged
Conversation
- daemon.py: 11-endpoint HTTP server (health, apply-rules, correct, detect, end-session, brain-recall, enforce-rules, log-event, tag-delta, checkpoint, maintain) with ThreadingHTTPServer, brain lock, session registry, inactivity shutdown, opt-in telemetry - detection/mode_classifier.py: Signal 5 — rule-based prompt mode detection - detection/addition_pattern.py: Signal 4 — structural fingerprinting of repeated additions (AST for Python, regex for other languages) - detection/correction_conflict.py: Signal 6 — token-level conflict detection with staged demotion (2 conflicts = demote, 3 = kill) Tests: 73 passing (daemon + detection + integration) Co-Authored-By: Gradata <noreply@gradata.ai>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (100)
📝 WalkthroughSummary
WalkthroughAdds a detection package (mode classification, addition-pattern, correction-of-correction conflict) and integrates it into the daemon with six new POST endpoints. Enhances existing endpoints to use classifiers/trackers, adds opt-in daily telemetry, and expands unit and integration tests covering detection and daemon behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant Daemon as GradataDaemon
participant Config as Config File (~/.gradata/config.toml)
participant Clock as System Clock
participant API as api.gradata.com/telemetry
Daemon->>Config: Read config
alt config missing or telemetry=false
Daemon-->>Daemon: Skip telemetry
else telemetry=true
Daemon->>Clock: Get now (UTC)
Daemon->>Config: Read telemetry_last_sent
alt within 24h
Daemon-->>Daemon: Skip send
else
Daemon->>Daemon: Aggregate counts
Daemon->>API: POST telemetry (async)
API-->>Daemon: 200/OK
Daemon->>Config: Update telemetry_last_sent
end
end
sequenceDiagram
participant Client as HTTP Client
participant Daemon as /correct handler
participant Mode as mode_classifier
participant Add as AdditionTracker
participant Conflict as ConflictTracker
participant Response as JSON Response
Client->>Daemon: POST /correct (old,new,rule_id,...)
Daemon->>Mode: classify_mode(prompt)
Mode-->>Daemon: (mode, confidence)
Daemon->>Add: is_addition(old,new)
alt addition detected
Add-->>Daemon: True
Daemon->>Add: classify_addition(old,new,ext)
Add-->>Daemon: (category,type)
Daemon->>Add: record(fingerprint,session_id)
Add-->>Daemon: lesson or None
else not addition
Add-->>Daemon: False
end
Daemon->>Conflict: extract_diff_tokens(old,new)
Conflict-->>Daemon: (added_tokens, removed_tokens)
Daemon->>Conflict: detect_conflict(added,removed,threshold)
alt conflict true
Conflict-->>Daemon: True
Daemon->>Conflict: record_conflict(rule_id)
Conflict-->>Daemon: escalation_action
else
Conflict-->>Daemon: False
end
Daemon->>Response: Build JSON including detection results
Response-->>Client: 200 + payload
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| def _maybe_send_telemetry(self) -> None: | ||
| """Send anonymous daily heartbeat if telemetry is opted in.""" | ||
| import re as _re | ||
| config_path = Path.home() / ".gradata" / "config.toml" |
There was a problem hiding this comment.
Path.home() / ".gradata" / "config.toml" is a hardcoded path. Rule 3 requires using _paths.py utilities or BrainContext for all file paths to ensure cross-platform support. Consider exposing a get_config_path() helper in _paths.py and calling it here.
Rule Used: # Code Review Rules
Rule 1: Never use print() ... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gradata/daemon.py
Line: 726
Comment:
**Hardcoded config path violates Rule 3**
`Path.home() / ".gradata" / "config.toml"` is a hardcoded path. Rule 3 requires using `_paths.py` utilities or `BrainContext` for all file paths to ensure cross-platform support. Consider exposing a `get_config_path()` helper in `_paths.py` and calling it here.
**Rule Used:** # Code Review Rules
## Rule 1: Never use print() ... ([source](https://app.greptile.com/review/custom-context?memory=dee613fe-ca52-4382-b9d7-fad6d0b079ec))
How can I resolve this? If you propose a fix, please make it concise.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 24
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/gradata/daemon.py (1)
1-50:⚠️ Potential issue | 🟡 MinorFile exceeds 500-line limit specified in coding guidelines.
At 882 lines, this file significantly exceeds the 500-line limit. Consider extracting the handler methods into a separate
_handlers.pymodule or splitting detection integration into a dedicated module.As per coding guidelines: "Keep files under 500 lines."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/daemon.py` around lines 1 - 50, This file is too large; extract the HTTP request handling and detection integration into smaller modules: move the BaseHTTPRequestHandler subclass and all endpoint handler methods (the GET /health and POST handlers for /apply-rules, /correct, /detect, /end-session, /brain-recall, /enforce-rules, /log-event, /tag-delta, /checkpoint, /maintain and any helper functions that build responses) into a new gradata.daemon._handlers module, and move detection-related logic that uses AdditionTracker, classify_addition, is_addition, ConflictTracker, extract_diff_tokens, classify_mode and parse_lessons into a dedicated detection integration module; update imports in gradata.daemon to import the handler class (and any exported helper functions) and the detection helpers, preserve usages of RuleScope, LessonState, apply_rules and format_rules_for_prompt, and ensure ThreadingMixIn/HTTPServer setup in daemon.py simply composes the handler rather than containing all handler method bodies.
🤖 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/daemon.py`:
- Around line 744-789: The nested function _send is missing a return type
annotation; update its signature from def _send(): to def _send() -> None: so
the intent (no return value) is explicit—make this change where _send is defined
(it’s the target passed to threading.Thread) and ensure no return statements
return a value.
- Around line 732-733: The current case-insensitive substring check on
config_text (if "telemetry = true" not in config_text.lower()) can match
unintended keys; replace it with a precise regex match against the telemetry
assignment (use re.search with flags re.I|re.M) to look for a line like
^\s*telemetry\s*=\s*true\s*$ so only a top-level telemetry = true setting
matches; update the conditional that references config_text accordingly (the
check around config_text) to use this regex-based test.
- Around line 749-755: The try block silently swallows exceptions and uses an
ambiguous single-letter variable `l`; replace the generator `sum(1 for l in
lessons if l.state.name == "RULE")` with a clearer name like `lesson` and handle
exceptions instead of `pass` by catching `Exception as e` and logging it (e.g.,
via `self._logger.exception(...)` or `self._logger.error(...)`) so failures in
`self._brain._load_lessons()` are recorded; keep the rest of the logic that
computes `lessons_count` and `rules_count` the same but with the renamed
variable.
- Around line 519-528: The try/except around the checkpoint logic is swallowing
all exceptions (in the block using d._brain_lock, d._brain._load_lessons,
LessonState and d._brain.emit), causing the handler to always report
checkpointed: True even when it failed; modify the handler to catch exceptions
explicitly, log the exception (including traceback) and ensure the response
indicates failure (return checkpointed: False) when an error occurs, e.g. wrap
the d._brain calls in a try/except that logs the exception (with session_id and
reason context) and sets/returns checkpointed=False instead of silently passing.
- Around line 403-414: The except block in the /brain-recall handler silently
swallows errors; change the broad "except Exception: pass" to "except Exception
as e:" and log the exception (e.g., call d.logger.exception(...) if the daemon
exposes a logger, or use the module-level logging.exception(...)) so failures in
the with d._brain_lock / d._brain.search(query) block are recorded; keep the
same control flow but ensure the exception message and stacktrace are included
in the log.
- Around line 40-41: The import hashlib is incorrectly separated from the stdlib
imports; move the line importing hashlib into the top stdlib import block
(grouped with other standard library imports near the other imports) so all
stdlib imports are contiguous—update the import section by relocating the
hashlib import into the existing stdlib group rather than leaving it as a
standalone import.
In `@src/gradata/detection/__init__.py`:
- Around line 1-14: Add "from __future__ import annotations" as the first
non-docstring statement in this module so forward-referenced and postponed type
annotations are enabled; update src/gradata/detection/__init__.py by inserting
that import immediately after the module docstring and before the existing
imports (so the import precedes references to classify_mode, MODE_CATEGORY_MAP,
is_addition, classify_addition, AdditionTracker, detect_conflict,
extract_diff_tokens, ConflictTracker).
- Around line 15-24: The __all__ list in src/gradata/detection/__init__.py is
unsorted, triggering RUF022; update the __all__ variable to contain the exported
names sorted alphabetically (e.g., "AdditionTracker", "ConflictTracker",
"MODE_CATEGORY_MAP", "classify_addition", "classify_mode", "detect_conflict",
"extract_diff_tokens", "is_addition") so entries are in lexical order while
preserving the same set of symbols.
In `@src/gradata/detection/addition_pattern.py`:
- Around line 18-29: The functions is_addition and the related logic using
parameters min_added_chars, threshold, and cross_session_threshold lack
validation at their API boundaries; add explicit input checks at the start of
the public functions (e.g., is_addition) to validate that min_added_chars,
threshold, and cross_session_threshold are numeric and within sensible ranges
(e.g., positive integers for min_added_chars and floats between 0 and 1 for
thresholds) and raise a ValueError with a clear message for invalid values;
ensure the same validation is applied where threshold and
cross_session_threshold are accepted (around the code referenced at the other
occurrence) so callers cannot pass degenerate values that would cause instant
lesson creation or other incorrect behavior.
- Around line 1-6: The file addition_pattern.py is missing the required AGPL-3.0
source license header; add the standard AGPL-3.0 comment block at the top of
src/gradata/addition_pattern.py (above the module docstring) so the module
includes the licensing marker used across src/gradata/*.py; ensure the header
references AGPL-3.0 and contains the short copyright/permission statement
consistent with other files in the package.
- Around line 31-36: The current check in is_addition uses "old in new" which
allows replacements to pass if old appears elsewhere; replace this with an
insert-only (subsequence) check: implement a two-pointer subsequence scan in
is_addition that verifies every character of old appears in order inside new
(i.e., old is a subsequence of new), ensure len(new) >= len(old), compute added
= len(new) - len(old) and require added >= min_added_chars, and return False
immediately if the subsequence scan fails; reference the is_addition function
and min_added_chars when making the change.
- Around line 169-199: AdditionTracker.record is not thread-safe: protect
concurrent access to self._counters, the per-fingerprint _FingerprintCounter and
its sessions set by adding a threading.Lock (e.g., self._lock = threading.Lock()
in __init__) and acquiring it around reads/updates in record; specifically, wrap
counter = self._counters[fingerprint], counter.count += 1,
counter.sessions.add(session_id) and the conditional checks and resetting
self._counters[fingerprint] = _FingerprintCounter() in a single locked section,
then (to minimize lock hold) release the lock before calling
_make_lesson(category, stype); remember to import threading and reference
AdditionTracker.__init__, AdditionTracker.record, _counters and
_FingerprintCounter when applying the change.
In `@src/gradata/detection/correction_conflict.py`:
- Around line 1-6: Add an AGPL-3.0 source license header to the top of this new
module (src/gradata/detection/correction_conflict.py): insert the standard
AGPL-3.0 identifier and short header (e.g., SPDX-License-Identifier:
AGPL-3.0-or-later and a one-line copyright/ownership notice) above the existing
module docstring so the file clearly declares AGPL-3.0 licensing; ensure the
header format matches other files under src/gradata (same wording and placement)
so automated checks recognize it.
- Around line 33-42: The detect_conflict function currently accepts an unbounded
threshold which can produce incorrect results; add a validation at the start of
detect_conflict to ensure threshold is a float in the inclusive range [0.0, 1.0]
and raise a ValueError with a clear message if it is out of range (e.g.,
"threshold must be between 0.0 and 1.0"); keep the rest of the logic unchanged
and place this check before any early returns so invalid inputs are rejected at
the system boundary.
- Around line 51-72: ConflictTracker mutates shared _counts without
synchronization; add a threading.Lock instance (e.g., self._lock) in __init__
and use it to protect all accesses to _counts in record_conflict and get_count
(wrap the increment/read and decision logic in a with self._lock: block) so
concurrent threads cannot lose updates; ensure the lock is used consistently in
both methods and that get_count still returns the default 0 for missing
rule_ids.
In `@src/gradata/detection/mode_classifier.py`:
- Around line 71-96: The docstring in classify_mode includes an en-dash
character in the confidence range "0.0–1.0"; replace that en-dash with a
standard hyphen-minus so it reads "0.0-1.0" to avoid encoding/consistency issues
and keep the docstring text updated accordingly.
In `@tests/test_daemon_extended.py`:
- Around line 80-119: The test test_enforce_rules_with_rule duplicates the
daemon startup and brain seeding logic already in the daemon_url fixture;
refactor by creating a reusable fixture (e.g., daemon_with_lessons) that accepts
optional lessons.md content and performs the GradataDaemon setup (instantiate
GradataDaemon, call _try_bind(0), set _server._daemon, set _port from
_server.server_address, call _reset_idle_timer, start serve_forever thread) and
yields the base URL; update test_enforce_rules_with_rule to use this fixture
(pass the lessons.md content as the fixture parameter) and remove the duplicated
setup and shutdown code (replace direct use of GradataDaemon, _try_bind,
_server, and shutdown with the fixture).
- Around line 18-19: The decorator on the fixture function daemon_url uses
unnecessary parentheses: change the fixture decorator from `@pytest.fixture`() to
`@pytest.fixture` in tests/test_daemon_extended.py by editing the decorator above
the daemon_url(tmp_path: Path) function so it has no parentheses, satisfying
Ruff PT001.
In `@tests/test_daemon.py`:
- Around line 104-133: The test test_telemetry_skipped_when_sent_recently is
using an arbitrary time.sleep(0.3) which makes the assertion flaky; remove the
sleep and assert against the patched urllib.request.urlopen immediately (use
mock_urlopen.assert_not_called()) after calling
GradataDaemon._maybe_send_telemetry, or if _maybe_send_telemetry spawns a
background thread, replace the sleep with a synchronization strategy (e.g.,
patch or hook into GradataDaemon._maybe_send_telemetry or its telemetry worker)
so you can deterministically wait for the worker to complete before asserting
that mock_urlopen was not called; reference GradataDaemon and its
_maybe_send_telemetry method and the mock_urlopen patch to locate where to
change the test.
- Around line 86-101: The test currently uses time.sleep to wait for the
background work started by _maybe_send_telemetry which is flaky; instead either
have _maybe_send_telemetry return the Thread (or accept a threading.Event) so
the test can join the thread or wait on the Event, or modify the test to poll
with a short-interval loop and timeout until mock_urlopen.called is True (and
then read config_path), ensuring you fail the test if the timeout elapses;
update the assertions to run only after the explicit join/event/poll confirms
completion.
In `@tests/test_detection_addition.py`:
- Around line 17-51: The test class TestIsAddition duplicates many similar test
methods calling is_addition; replace the multiple methods (e.g.,
test_pure_addition_suffix, test_pure_addition_prefix,
test_replacement_not_addition, test_too_short_addition,
test_major_rewrite_not_addition, test_empty_old_nonempty_new,
test_empty_old_short_new, test_both_empty) with a single parametrized test using
pytest.mark.parametrize that supplies (old, new, expected[, min_added_chars])
cases and asserts is_addition(old, new, ...) == expected; add the pytest import
if missing and include separate parameters for cases that need non-default
min_added_chars.
In `@tests/test_detection_conflict.py`:
- Around line 7-12: The test imports a function named tokenize which shadows the
Python stdlib module; update the import statement from
gradata.detection.correction_conflict to alias the local function (e.g., import
tokenize as cc_tokenize or local_tokenize) and then update any usages in this
test (references to tokenize) to use the new alias; this keeps ConflictTracker,
detect_conflict, and extract_diff_tokens imports unchanged while avoiding stdlib
shadowing.
In `@tests/test_detection_mode.py`:
- Around line 52-64: Tests use direct float equality and an unused unpacked
variable; update test functions that call classify_mode: replace conf == 0.0
assertions with pytest.approx (e.g., assert conf == pytest.approx(0.0)) and mark
the unused returned value by prefixing it with an underscore (change mode, conf
= classify_mode(...) to mode, _conf = classify_mode(...) or only unpack mode).
Also ensure pytest is imported in the test file so pytest.approx is available.
- Around line 77-84: The test test_confidence_capped_at_one currently asserts
conf <= 1.0; replace this with an approximate equality assertion using
pytest.approx to handle floating-point boundary cases (e.g., assert conf ==
pytest.approx(1.0)), and add an import for pytest if it's not already present;
update references to classify_mode and conf in that test only.
---
Outside diff comments:
In `@src/gradata/daemon.py`:
- Around line 1-50: This file is too large; extract the HTTP request handling
and detection integration into smaller modules: move the BaseHTTPRequestHandler
subclass and all endpoint handler methods (the GET /health and POST handlers for
/apply-rules, /correct, /detect, /end-session, /brain-recall, /enforce-rules,
/log-event, /tag-delta, /checkpoint, /maintain and any helper functions that
build responses) into a new gradata.daemon._handlers module, and move
detection-related logic that uses AdditionTracker, classify_addition,
is_addition, ConflictTracker, extract_diff_tokens, classify_mode and
parse_lessons into a dedicated detection integration module; update imports in
gradata.daemon to import the handler class (and any exported helper functions)
and the detection helpers, preserve usages of RuleScope, LessonState,
apply_rules and format_rules_for_prompt, and ensure ThreadingMixIn/HTTPServer
setup in daemon.py simply composes the handler rather than containing all
handler method bodies.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 332c47e4-f762-48c8-ba75-7278a6e1f583
📒 Files selected for processing (10)
src/gradata/daemon.pysrc/gradata/detection/__init__.pysrc/gradata/detection/addition_pattern.pysrc/gradata/detection/correction_conflict.pysrc/gradata/detection/mode_classifier.pytests/test_daemon.pytests/test_daemon_extended.pytests/test_detection_addition.pytests/test_detection_conflict.pytests/test_detection_mode.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Greptile Review
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Always read a file before editing it. Always prefer editing over creating new files.
Files:
tests/test_detection_mode.pytests/test_detection_addition.pysrc/gradata/detection/__init__.pytests/test_daemon.pysrc/gradata/detection/mode_classifier.pytests/test_daemon_extended.pysrc/gradata/detection/correction_conflict.pysrc/gradata/daemon.pysrc/gradata/detection/addition_pattern.pytests/test_detection_conflict.py
**/*.{py,sh,js,ts,json,env*}
📄 CodeRabbit inference engine (CLAUDE.md)
Never hardcode secrets. Never commit .env files. Pre-commit hook blocks both.
Files:
tests/test_detection_mode.pytests/test_detection_addition.pysrc/gradata/detection/__init__.pytests/test_daemon.pysrc/gradata/detection/mode_classifier.pytests/test_daemon_extended.pysrc/gradata/detection/correction_conflict.pysrc/gradata/daemon.pysrc/gradata/detection/addition_pattern.pytests/test_detection_conflict.py
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Keep files under 500 lines. Validate input at system boundaries.
Files:
tests/test_detection_mode.pytests/test_detection_addition.pysrc/gradata/detection/__init__.pytests/test_daemon.pysrc/gradata/detection/mode_classifier.pytests/test_daemon_extended.pysrc/gradata/detection/correction_conflict.pysrc/gradata/daemon.pysrc/gradata/detection/addition_pattern.pytests/test_detection_conflict.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_detection_mode.pytests/test_detection_addition.pytests/test_daemon.pytests/test_daemon_extended.pytests/test_detection_conflict.py
src/gradata/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use AGPL-3.0 licensing. Source code resides in src/gradata/.
Files:
src/gradata/detection/__init__.pysrc/gradata/detection/mode_classifier.pysrc/gradata/detection/correction_conflict.pysrc/gradata/daemon.pysrc/gradata/detection/addition_pattern.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/detection/__init__.pysrc/gradata/detection/mode_classifier.pysrc/gradata/detection/correction_conflict.pysrc/gradata/daemon.pysrc/gradata/detection/addition_pattern.py
🪛 Ruff (0.15.9)
tests/test_detection_mode.py
[warning] 53-53: Unpacked variable conf is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
src/gradata/detection/__init__.py
[warning] 15-24: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
tests/test_daemon.py
[warning] 113-113: Use datetime.UTC alias
Convert to datetime.UTC alias
(UP017)
src/gradata/detection/mode_classifier.py
[warning] 76-76: Docstring contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?
(RUF002)
tests/test_daemon_extended.py
[warning] 18-18: Use @pytest.fixture over @pytest.fixture()
Remove parentheses
(PT001)
[error] 45-50: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 51-51: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
src/gradata/daemon.py
[error] 413-414: try-except-pass detected, consider logging the exception
(S110)
[warning] 413-413: Do not catch blind exception: Exception
(BLE001)
[warning] 465-465: Do not catch blind exception: Exception
(BLE001)
[error] 527-528: try-except-pass detected, consider logging the exception
(S110)
[warning] 527-527: Do not catch blind exception: Exception
(BLE001)
[warning] 561-561: Do not catch blind exception: Exception
(BLE001)
[warning] 739-739: Use datetime.UTC alias
Convert to datetime.UTC alias
(UP017)
[warning] 744-744: Missing return type annotation for private function _send
Add return type annotation: None
(ANN202)
[error] 753-753: Ambiguous variable name: l
(E741)
[error] 754-755: try-except-pass detected, consider logging the exception
(S110)
[warning] 754-754: Do not catch blind exception: Exception
(BLE001)
[error] 770-770: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 771-772: try-except-pass detected, consider logging the exception
(S110)
[warning] 771-771: Do not catch blind exception: Exception
(BLE001)
[warning] 775-775: Use datetime.UTC alias
Convert to datetime.UTC alias
(UP017)
[error] 785-786: try-except-pass detected, consider logging the exception
(S110)
[warning] 785-785: Do not catch blind exception: Exception
(BLE001)
src/gradata/detection/addition_pattern.py
[warning] 71-71: Too many return statements (13 > 6)
(PLR0911)
[warning] 71-71: Too many branches (15 > 12)
(PLR0912)
[warning] 102-103: Use a single if statement instead of nested if statements
(SIM102)
🔇 Additional comments (6)
src/gradata/detection/mode_classifier.py (1)
90-96: LGTM: Confidence calculation correctly clamped to [0, 1].The confidence scoring uses
min(..., 1.0)to cap the upper bound and the formula ensures non-negative values. Theround(confidence, 4)provides consistent decimal precision.tests/test_detection_addition.py (1)
110-161: LGTM: AdditionTracker tests are comprehensive.Good coverage of threshold behavior, cross-session tracking, counter reset after lesson emission, and independent fingerprint tracking.
tests/test_detection_conflict.py (1)
79-84: LGTM: Good boundary condition testing fordetect_conflict.The threshold boundary tests at lines 79-84 properly verify the edge cases where overlap ratio equals or falls below the threshold.
tests/test_daemon_extended.py (1)
42-52: LGTM: Helper function for HTTP POST is clean.The
_posthelper provides a clear abstraction for JSON POST requests. The S310 audit warning is a false positive here since the URL is constructed from a known localhost base.src/gradata/daemon.py (2)
211-218: LGTM: Mode classification integration.The
classify_modeintegration correctly extracts mode and confidence from prompts, replacing hardcoded values. The response structure is clean.
270-306: LGTM: Addition pattern and conflict detection integration.The Signal 4 (addition pattern) and Signal 6 (correction conflict) detection are well-integrated. The response includes
addition_detected,addition_lesson, andcorrection_conflictfields appropriately.
Comment on lines
+40
to
+41
| import hashlib | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Import hashlib should be grouped with stdlib imports.
The hashlib import at line 40 is separated from other stdlib imports by a blank line, which breaks the import grouping convention.
Proposed fix
from socketserver import ThreadingMixIn
-import hashlib
-
import gradata
+import hashlibOr better, move it to the stdlib group above line 26.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/gradata/daemon.py` around lines 40 - 41, The import hashlib is
incorrectly separated from the stdlib imports; move the line importing hashlib
into the top stdlib import block (grouped with other standard library imports
near the other imports) so all stdlib imports are contiguous—update the import
section by relocating the hashlib import into the existing stdlib group rather
than leaving it as a standalone import.
Comment on lines
+732
to
+733
| if "telemetry = true" not in config_text.lower(): | ||
| return |
There was a problem hiding this comment.
Case-insensitive check may match unintended config values.
Line 732 uses .lower() on the entire config text, which could match telemetry = True as well as some_telemetry = true_ish. Consider a more precise regex pattern.
Proposed fix
- if "telemetry = true" not in config_text.lower():
+ import re as _re_check
+ if not _re_check.search(r'(?m)^\s*telemetry\s*=\s*true\s*$', config_text, _re_check.IGNORECASE):
return🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/gradata/daemon.py` around lines 732 - 733, The current case-insensitive
substring check on config_text (if "telemetry = true" not in
config_text.lower()) can match unintended keys; replace it with a precise regex
match against the telemetry assignment (use re.search with flags re.I|re.M) to
look for a line like ^\s*telemetry\s*=\s*true\s*$ so only a top-level telemetry
= true setting matches; update the conditional that references config_text
accordingly (the check around config_text) to use this regex-based test.
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 100 file(s) based on 24 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 100 file(s) based on 24 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Deploying gradata with
|
| Latest commit: |
4495535
|
| Status: | ⚡️ Build in progress... |
Comment on lines
+281
to
+293
|
|
||
| correction_conflict = None | ||
| _, new_removed = extract_diff_tokens(old_string, new_string) | ||
| if new_removed and misfired: | ||
| for rule_id in misfired: | ||
| action = d._conflict_tracker.record_conflict(rule_id) | ||
| if action: | ||
| correction_conflict = { | ||
| "rule_id": rule_id, | ||
| "action": action, | ||
| "conflict_count": d._conflict_tracker.get_count(rule_id), | ||
| } | ||
| break |
There was a problem hiding this comment.
The comment advertises "Correction-of-correction detection (Signal 6)" but the condition if new_removed and misfired only verifies that some tokens were removed in the same session as a fired rule — it never checks whether those removed tokens actually overlap with what the misfired rule injected. The detect_conflict() function exists in correction_conflict.py precisely for this purpose, but is never called here.
As written, every correction that removes any tokens while misfired rules are present will increment those rules' conflict counters, causing legitimate rules to be demoted/killed for completely unrelated edits.
The fix requires wiring in detect_conflict():
# Correction-of-correction detection (Signal 6)
correction_conflict = None
original_added, new_removed = extract_diff_tokens(old_string, new_string)
if new_removed and misfired:
for rule_id in misfired:
if detect_conflict(original_added, new_removed):
action = d._conflict_tracker.record_conflict(rule_id)
if action:
correction_conflict = {
"rule_id": rule_id,
"action": action,
"conflict_count": d._conflict_tracker.get_count(rule_id),
}
breakNote: detect_conflict needs to be added to the import on line 46.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gradata/daemon.py
Line: 281-293
Comment:
**Signal 6 conflict detection skips the token-overlap check**
The comment advertises "Correction-of-correction detection (Signal 6)" but the condition `if new_removed and misfired` only verifies that _some_ tokens were removed in the same session as a fired rule — it never checks whether those removed tokens actually overlap with what the misfired rule injected. The `detect_conflict()` function exists in `correction_conflict.py` precisely for this purpose, but is never called here.
As written, every correction that removes any tokens while misfired rules are present will increment those rules' conflict counters, causing legitimate rules to be demoted/killed for completely unrelated edits.
The fix requires wiring in `detect_conflict()`:
```python
# Correction-of-correction detection (Signal 6)
correction_conflict = None
original_added, new_removed = extract_diff_tokens(old_string, new_string)
if new_removed and misfired:
for rule_id in misfired:
if detect_conflict(original_added, new_removed):
action = d._conflict_tracker.record_conflict(rule_id)
if action:
correction_conflict = {
"rule_id": rule_id,
"action": action,
"conflict_count": d._conflict_tracker.get_count(rule_id),
}
break
```
Note: `detect_conflict` needs to be added to the import on line 46.
How can I resolve this? If you propose a fix, please make it concise.
4 tasks
Gradata
pushed a commit
that referenced
this pull request
Apr 10, 2026
… layer Signal 6 conflict detection now checks token overlap between removed tokens and the fired rule's instruction tokens via detect_conflict(), preventing false-positive conflict escalations when tokens don't actually overlap. Co-Authored-By: Gradata <noreply@gradata.ai>
This was referenced Apr 10, 2026
Merged
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
gradata.daemon) with 11 endpoints for Claude Code pluginNew Files
src/gradata/daemon.pysrc/gradata/detection/mode_classifier.pysrc/gradata/detection/addition_pattern.pysrc/gradata/detection/correction_conflict.pytests/test_daemon.pytests/test_daemon_extended.pytests/test_detection_*.pytests/test_plugin_integration.pyTest plan
Generated with Gradata
Greptile Summary
This PR adds an HTTP daemon (
gradata.daemon) that holds a Brain instance in memory and exposes 11 REST endpoints for IDE plugins (VS Code, JetBrains) to consume, plus a detection layer with three new signal modules (Signal 4: addition pattern, Signal 5: mode classifier, Signal 6: correction-of-correction) and 73 new tests.The architecture is sound — a
ThreadingMixInHTTP server wires plugin calls into the existing Brain/EventBus stack behind athreading.Lock, with idle auto-shutdown and PID-file–based discovery. The detection modules are clean, well-tested, and correctly follow project conventions.Two concrete bugs in
daemon.pyneed attention before merge:_pick_portuses Python's built-inhash(), which is randomized per process (PYTHONHASHSEED). The docstring promises a "deterministic" port derived from the brain directory, but every daemon restart produces a different port. Thehashlib-based fix is a one-liner._handle_correctchecksif new_removed and misfiredto detect correction-of-correction conflicts, but never callsdetect_conflict()to verify token-level overlap. Any correction that removes tokens in a session with fired rules will increment conflict counters for those rules — causing legitimate rules to be demoted or killed due to completely unrelated edits.One additional style note:
test_plugin_integration.pyis missingfrom __future__ import annotations, which all other new files in the PR include.Confidence Score: 3/5
Two targeted fixes needed before merge: deterministic port hashing and wiring detect_conflict() into the conflict handler.
The detection modules and test suite are high quality and the daemon architecture is solid. However, the non-deterministic port breaks the advertised stable-port-per-brain-dir guarantee, and the Signal 6 conflict detection will produce systematic false positives that silently demote or kill valid rules — a correctness regression in the core learning loop. Both are straightforward to fix but material enough to warrant a revision.
src/gradata/daemon.py — _pick_port (line 799) and _handle_correct Signal 6 logic (lines 281–293)
Vulnerabilities
127.0.0.1, limiting exposure to localhost only — good.rules_count,lessons_count),os, andpython_version— no user data or file paths._sendbackground thread writes back toconfig.tomlusing a string captured at call time; a concurrent config write during the thread's network call could be silently overwritten. Low risk given the opt-in nature, but worth noting.Important Files Changed
Sequence Diagram
sequenceDiagram participant Plugin as IDE Plugin participant Daemon as GradataDaemon (HTTP) participant Brain as Brain (in-memory) participant Detection as Detection Layer Plugin->>Daemon: POST /apply-rules {prompt, session_id} Daemon->>Brain: apply_rules(lessons, scope) Daemon->>Detection: classify_mode(prompt) Daemon-->>Plugin: {rules, injection_text, mode_detected} Plugin->>Daemon: POST /correct {old_string, new_string, file_path} Daemon->>Brain: brain.correct(draft, final, category) Daemon->>Detection: is_addition() → classify_addition() Daemon->>Detection: extract_diff_tokens() Daemon->>Detection: conflict_tracker.record_conflict() Daemon-->>Plugin: {captured, addition_detected, correction_conflict} Plugin->>Daemon: POST /detect {user_message} Daemon->>Brain: detect_implicit_feedback(user_message) Daemon->>Detection: classify_mode(user_message) Daemon-->>Plugin: {implicit_feedback, mode, mode_confidence} Plugin->>Daemon: POST /end-session {session_id} Daemon->>Brain: brain.end_session() Daemon->>Brain: brain.convergence() Daemon-->>Plugin: {corrections_captured, lessons_graduated, convergence} Plugin->>Daemon: GET /health Daemon->>Brain: _load_lessons() Daemon-->>Plugin: {status, rules_count, uptime_seconds}Comments Outside Diff (5)
src/gradata/daemon.py, line 799-801 (link)hash()is not deterministic across processesPython randomises
hash()withPYTHONHASHSEEDon every interpreter start (default since Python 3.3), soabs(hash(brain_dir_str))will produce a different value each time the daemon restarts. The docstring says "Deterministic port from brain_dir hash", but this is only deterministic within a single process run. Any client or plugin caching the computed port will break after a restart.Switch to
hashlib(already imported) for a stable hash:Prompt To Fix With AI
src/gradata/daemon.py, line 615-622 (link)_get_session_numreads and writes_sessionsand_session_counterwithout holding any lock. Because the server usesThreadingMixIn, two simultaneous requests can race on these attributes, resulting in duplicate session numbers or a lost_session_counterincrement.The same issue affects every direct access to
d._fired_rulesin_handle_apply_rules(line 209) and_handle_correct(lines 256–268) — both read and mutate that dict outside the_brain_lock.A lightweight fix is to introduce a dedicated
threading.Lockfor_sessions/_fired_rules:Then wrap
_get_session_numand all_fired_rulesaccesses withwith d._session_lock:.Prompt To Fix With AI
src/gradata/daemon.py, line 255-265 (link)misfiredclassification logic is inverted / conceptually inconsistentThe comment says "Acceptance attribution: category-based", and the code marks a rule as
misfiredwhen the rule's category prefix matches the correction's category. But the variable namemisfiredimplies the rule fired incorrectly — yet a rule sharing the correction's category is exactly the rule that should be most relevant. Confusingly, the samemisfiredset is then fed into the conflict-of-correction detector (correction_conflict), which is semantically different from acceptance tracking.More concretely: every rule whose category prefix equals the correction's category is flagged as
misfired, regardless of whether that rule had anything to do with the specific change. This could generate spuriouscorrection_conflictentries for unrelated rules that happen to share the same category.Consider separating acceptance attribution from conflict detection, and either rename or redesign this variable to reflect its actual meaning.
Prompt To Fix With AI
src/gradata/daemon.py, line 127-132 (link)json.JSONDecodeErroron malformed request bodiesIf a client sends a body that is not valid JSON,
json.loads(raw)raisesjson.JSONDecodeError, which propagates up and causes the defaultBaseHTTPRequestHandlererror path (a 500 with a traceback on stderr). Consider wrapping the decode and returning a structured 400 response:Prompt To Fix With AI
src/gradata/daemon.py, line 799-801 (link)_pick_portuses Python's built-inhash(), which is randomized per-process viaPYTHONHASHSEED(default since Python 3.3). This means the daemon will pick a different port on every restart, directly contradicting the docstring claim of "Deterministic port from brain_dir hash." Any plugin that caches this port across daemon restarts will fail to reconnect without reading the PID file each time.Replace with a stable hash from
hashlib:Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (2): Last reviewed commit: "fix: apply CodeRabbit auto-fixes" | Re-trigger Greptile