chore: fix 66 ruff violations (unblock SDK CI)#100
Conversation
Mix of auto-fixes (ruff --fix) and manual touch-ups. No behavior change. Auto-fixes (57 lines across 20 files): - UP017: datetime.timezone.utc → datetime.UTC - I001: unsorted imports - F401: unused imports - UP037: quoted annotations unwrapped - UP045: Optional[X] → X | None - RUF100: unused noqa - F541: f-strings missing placeholders - TC005: empty type-checking blocks Manual (9): - cli.py: collapse nested HTTPS-guard if; 2× chmod try/except → contextlib.suppress - correction_detector.py: remove unused correction_text local - collaborative_filter.py: zip(strict=True) for vec_a/vec_b cosine - clustering.py: for _category (unused loop key in .items()) - rule_pipeline.py: collapse graduation nested-if pairs into single conditions - loop_intelligence.py: 2× try/except/pass → contextlib.suppress Unblocks SDK CI so PR #98 (RetainOrchestrator) and PR #99 (hook daemon) can land through normal CI rather than admin bypass. Co-Authored-By: Gradata <noreply@gradata.ai>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
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.
📝 WalkthroughSummary
WalkthroughThis PR contains extensive refactoring across 25+ files, primarily modernizing type annotations (removing forward references), cleaning unused imports, standardizing datetime handling to use Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/gradata/enhancements/scoring/loop_intelligence.py (3)
391-403:⚠️ Potential issue | 🟡 MinorSession filter skips valid
session=0values.Line 401 uses truthiness (
if session:), so0is treated as “no filter.” Use an explicitNonecheck.Proposed fix
- if session: + if session is not None: query += " AND session = ?" params.append(session)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/enhancements/scoring/loop_intelligence.py` around lines 391 - 403, The session filter currently uses truthiness (if session) and therefore skips valid session=0; update the conditional in the function that builds the events query (the block using the local variables query and params and the parameter session) to check explicitly for None (if session is not None) before appending " AND session = ?" and params.append(session) so zero-valued sessions are included.
437-462:⚠️ Potential issue | 🟠 Major
confidenceis not constrained to a numeric value in[0.0, 1.0].Line 461 returns label strings (e.g.,
[EMERGING]) under theconfidencekey. This violates the numeric confidence contract.Proposed fix
for val, s in stats.items(): rate = round(s["replies"] / s["sent"] * 100, 1) if s["sent"] > 0 else 0 + confidence = max(0.0, min(1.0, (s["replies"] / s["sent"]) if s["sent"] > 0 else 0.0)) result[val] = { "sent": s["sent"], "replies": s["replies"], "rate": rate, - "confidence": confidence_label(s["sent"]), + "confidence": round(confidence, 4), + "confidence_label": confidence_label(s["sent"]), }As per coding guidelines,
Confidence values must be in [0.0, 1.0].🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/enhancements/scoring/loop_intelligence.py` around lines 437 - 462, The aggregate_by_key function currently sets "confidence" to the string returned by confidence_label(s["sent"]) which violates the numeric [0.0, 1.0] contract; change this so "confidence" is a float in [0.0,1.0] by either (A) calling or adding a function that returns a numeric confidence (e.g., confidence_score(sent_count) -> float) and using that instead of confidence_label, or (B) map the existing label from confidence_label to a numeric value before assigning it to result[val]["confidence"]; ensure the chosen function name (confidence_score or mapping of confidence_label) is used consistently and that the value is clamped to the [0.0,1.0] range.
133-143:⚠️ Potential issue | 🟠 MajorDB-accessing APIs still bypass
BrainContext.The updated signatures continue to pass raw
db_pathand perform direct DB access (e.g., Line 133, Line 189, Line 220, Line 277, Line 389, Line 533). This keeps the module outside the SDK’s required context boundary.As per coding guidelines,
all functions accepting BrainContext where DB access occurs.Also applies to: 189-196, 220-227, 277-283, 389-393, 533-537
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/enhancements/scoring/loop_intelligence.py` around lines 133 - 143, Several DB-accessing functions (starting with log_activity) still accept a raw db_path and perform direct DB access; update these to accept a BrainContext and use its database access methods instead. Specifically, modify the signature of log_activity to take context: BrainContext (replace db_path: str | Path) and update its internals to call context.db or context.get_db()/context.execute_* methods rather than opening the DB directly; apply the same pattern to the other DB-using functions referenced around lines 189-196, 220-227, 277-283, 389-393, and 533-537 so none open the database directly and all use the BrainContext boundary. Ensure optional parameters and emit_event behavior remain unchanged and update any callers to pass the BrainContext instance.
🤖 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/enhancements/carl.py`:
- Around line 1-3: The module is missing the required future import for
postponed evaluation of annotations; insert the line "from __future__ import
annotations" immediately below the module docstring at the top of the file
(above the existing import from gradata.enhancements.behavioral_engine) so that
type annotations in this shim (and any references in imports like
gradata.enhancements.behavioral_engine) use postponed evaluation for type
safety.
In `@src/gradata/enhancements/scoring/reports.py`:
- Line 146: The missing-DB branch in generate_health_report returns a naive
timestamp while the normal path uses a UTC-aware ISO string, causing
inconsistent HealthReport.timestamp values; update the missing-DB return path to
produce the same UTC-aware ISO timestamp (e.g., use
datetime.now(UTC).isoformat() or datetime.now(tz=UTC).isoformat()) so every
return path of generate_health_report assigns HealthReport.timestamp
consistently.
---
Outside diff comments:
In `@src/gradata/enhancements/scoring/loop_intelligence.py`:
- Around line 391-403: The session filter currently uses truthiness (if session)
and therefore skips valid session=0; update the conditional in the function that
builds the events query (the block using the local variables query and params
and the parameter session) to check explicitly for None (if session is not None)
before appending " AND session = ?" and params.append(session) so zero-valued
sessions are included.
- Around line 437-462: The aggregate_by_key function currently sets "confidence"
to the string returned by confidence_label(s["sent"]) which violates the numeric
[0.0, 1.0] contract; change this so "confidence" is a float in [0.0,1.0] by
either (A) calling or adding a function that returns a numeric confidence (e.g.,
confidence_score(sent_count) -> float) and using that instead of
confidence_label, or (B) map the existing label from confidence_label to a
numeric value before assigning it to result[val]["confidence"]; ensure the
chosen function name (confidence_score or mapping of confidence_label) is used
consistently and that the value is clamped to the [0.0,1.0] range.
- Around line 133-143: Several DB-accessing functions (starting with
log_activity) still accept a raw db_path and perform direct DB access; update
these to accept a BrainContext and use its database access methods instead.
Specifically, modify the signature of log_activity to take context: BrainContext
(replace db_path: str | Path) and update its internals to call context.db or
context.get_db()/context.execute_* methods rather than opening the DB directly;
apply the same pattern to the other DB-using functions referenced around lines
189-196, 220-227, 277-283, 389-393, and 533-537 so none open the database
directly and all use the BrainContext boundary. Ensure optional parameters and
emit_event behavior remain unchanged and update any callers to pass the
BrainContext instance.
🪄 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: cc96b752-9c5c-40e5-8459-2efc11458886
📒 Files selected for processing (21)
src/gradata/_core.pysrc/gradata/_events.pysrc/gradata/cli.pysrc/gradata/correction_detector.pysrc/gradata/enhancements/bandits/collaborative_filter.pysrc/gradata/enhancements/carl.pysrc/gradata/enhancements/clustering.pysrc/gradata/enhancements/graduation/agent_graduation.pysrc/gradata/enhancements/graduation/judgment_decay.pysrc/gradata/enhancements/meta_rules.pysrc/gradata/enhancements/profiling/tone_profile.pysrc/gradata/enhancements/rule_pipeline.pysrc/gradata/enhancements/scoring/brain_scores.pysrc/gradata/enhancements/scoring/calibration.pysrc/gradata/enhancements/scoring/correction_tracking.pysrc/gradata/enhancements/scoring/gate_calibration.pysrc/gradata/enhancements/scoring/loop_intelligence.pysrc/gradata/enhancements/scoring/memory_extraction.pysrc/gradata/enhancements/scoring/reports.pysrc/gradata/enhancements/self_improvement.pysrc/gradata/hooks/self_review.py
💤 Files with no reviewable changes (3)
- src/gradata/enhancements/scoring/correction_tracking.py
- src/gradata/enhancements/self_improvement.py
- src/gradata/_core.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Python 3.11
- GitHub Check: Python 3.12
- GitHub Check: Test (Python 3.13)
- GitHub Check: test (3.12)
- GitHub Check: test (3.11)
- GitHub Check: test (3.13)
🧰 Additional context used
📓 Path-based instructions (2)
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/hooks/self_review.pysrc/gradata/enhancements/clustering.pysrc/gradata/enhancements/scoring/calibration.pysrc/gradata/enhancements/scoring/gate_calibration.pysrc/gradata/enhancements/scoring/reports.pysrc/gradata/enhancements/carl.pysrc/gradata/enhancements/graduation/judgment_decay.pysrc/gradata/enhancements/meta_rules.pysrc/gradata/enhancements/scoring/memory_extraction.pysrc/gradata/enhancements/scoring/brain_scores.pysrc/gradata/_events.pysrc/gradata/enhancements/profiling/tone_profile.pysrc/gradata/enhancements/bandits/collaborative_filter.pysrc/gradata/correction_detector.pysrc/gradata/enhancements/graduation/agent_graduation.pysrc/gradata/cli.pysrc/gradata/enhancements/rule_pipeline.pysrc/gradata/enhancements/scoring/loop_intelligence.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/self_review.py
🔇 Additional comments (27)
src/gradata/enhancements/clustering.py (1)
159-159: LGTM! Correct fix for unused loop variable.Renaming
categoryto_categoryappropriately signals that the variable is intentionally unused, following Python conventions for unused loop variables.src/gradata/enhancements/scoring/gate_calibration.py (1)
34-34: Import cleanup looks good.This change is safe and keeps the module focused on used symbols only.
src/gradata/enhancements/scoring/calibration.py (1)
31-31: Dataclass import simplification is correct.No behavior impact; this is a clean lint-aligned update.
src/gradata/_events.py (1)
417-417: Type-hint update is clean.This is a straightforward annotation modernization with no behavior change.
src/gradata/enhancements/scoring/brain_scores.py (1)
231-231: Dynamic import cleanup is good.Removing the stale suppression here preserves the same control flow and fallback behavior.
src/gradata/correction_detector.py (2)
27-27: Import cleanup is correct.This is a safe lint-driven simplification.
152-152:from_dictannotation modernization looks good.Clean type-hint update with no runtime impact.
src/gradata/enhancements/profiling/tone_profile.py (2)
120-120:ToneFeatures.from_dicttype update is clean.No behavioral change, and the annotation is clearer.
252-252:ToneProfile.from_dicttype update is clean.This is a safe modernization-only change.
src/gradata/enhancements/bandits/collaborative_filter.py (3)
37-37: Dataclass import cleanup is good.Safe removal of unused symbol usage.
64-64:from_lessonsannotation update is correct.This keeps typing modernized without changing behavior.
125-125: Cosine dot-product strict zip update looks good.This keeps the current logic intact while enforcing the intended vector-length invariant.
src/gradata/hooks/self_review.py (1)
39-39: Import reorder is safe and behavior-preserving.This is a mechanical lint cleanup with no functional impact.
src/gradata/enhancements/meta_rules.py (1)
684-685: Stale suppression removal looks good.The fallback behavior remains intact while removing unnecessary lint suppression.
src/gradata/enhancements/graduation/judgment_decay.py (1)
39-40: Import reordering is clean.No correctness or behavioral concerns in this segment.
src/gradata/enhancements/scoring/memory_extraction.py (1)
38-39: UTC migration and string cleanup are correct.These edits are mechanical and preserve existing behavior.
Also applies to: 113-114, 214-214
src/gradata/enhancements/scoring/reports.py (1)
20-20: UTC standardization in the main return path is good.This aligns with the PR-wide datetime modernization and keeps timestamps timezone-aware.
Also applies to: 146-146
src/gradata/enhancements/rule_pipeline.py (3)
190-190: Type annotation modernization is correct.Removing the quoted annotation here is safe under
from __future__ import annotations.
408-421: Graduation condition collapse preserves behavior.The combined
andpredicates maintain the same INSTINCT→PATTERN and PATTERN→RULE gates.
374-376: Import cleanup is safe.These are mechanical formatting/reordering updates with no runtime impact.
Also applies to: 425-427, 482-484, 627-629
src/gradata/enhancements/graduation/agent_graduation.py (2)
51-68: Datetime UTC migration looks correct.The timestamp/date modernization is consistent and behavior-preserving.
Also applies to: 252-252, 452-452
775-775: Return annotation cleanup is correct.Unquoting these annotations is safe and improves type clarity.
Also applies to: 822-822
src/gradata/cli.py (4)
23-23: Type/import hygiene updates are good.These are safe lint-oriented cleanups with no behavior change.
Also applies to: 520-520, 542-542
548-553: Collapsed HTTPS guard keeps the same security intent.The merged conditional still rejects non-HTTPS URLs except localhost/127.0.0.1 dev cases.
620-621:contextlib.suppressreplacement is a good simplification.This preserves best-effort permission hardening without changing failure behavior.
Also applies to: 639-640
646-646: Literal success message cleanup is correct.Removing the placeholderless f-string is a safe mechanical fix.
src/gradata/enhancements/scoring/loop_intelligence.py (1)
23-23: UTC normalization and modernized union annotations are clean.The
datetime.now(UTC)migration andT | Noneconversions are consistent and align with the stated mechanical cleanup intent.Also applies to: 146-147, 198-199, 229-229, 266-266, 285-285, 328-328
| """Backward-compatibility shim — all code moved to behavioral_engine.py.""" | ||
|
|
||
| from gradata.enhancements.behavioral_engine import ( # noqa: F401 | ||
| from gradata.enhancements.behavioral_engine import ( |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether this file (and optionally other SDK Python files) includes the required future import.
set -euo pipefail
echo "Checking src/gradata/enhancements/carl.py..."
sed -n '1,20p' src/gradata/enhancements/carl.py
echo
echo "Scanning all SDK Python files missing 'from __future__ import annotations'..."
fd -e py . src/gradata | while read -r f; do
if ! rg -n "^from __future__ import annotations$" "$f" >/dev/null; then
echo "$f"
fi
doneRepository: Gradata/gradata
Length of output: 1686
Add required from __future__ import annotations at module top.
This SDK file is missing the required future-annotations import for type safety. Add it directly below the module docstring.
Proposed fix
"""Backward-compatibility shim — all code moved to behavioral_engine.py."""
+
+from __future__ import annotations
from gradata.enhancements.behavioral_engine import (
BehavioralContract,Per coding guidelines for src/gradata/**/*.py: type safety requires from __future__ import annotations.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| """Backward-compatibility shim — all code moved to behavioral_engine.py.""" | |
| from gradata.enhancements.behavioral_engine import ( # noqa: F401 | |
| from gradata.enhancements.behavioral_engine import ( | |
| """Backward-compatibility shim — all code moved to behavioral_engine.py.""" | |
| from __future__ import annotations | |
| from gradata.enhancements.behavioral_engine import ( |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/gradata/enhancements/carl.py` around lines 1 - 3, The module is missing
the required future import for postponed evaluation of annotations; insert the
line "from __future__ import annotations" immediately below the module docstring
at the top of the file (above the existing import from
gradata.enhancements.behavioral_engine) so that type annotations in this shim
(and any references in imports like gradata.enhancements.behavioral_engine) use
postponed evaluation for type safety.
| rules_active=rules_count, | ||
| lessons_active=lessons_count, | ||
| timestamp=datetime.now(timezone.utc).isoformat(), | ||
| timestamp=datetime.now(UTC).isoformat(), |
There was a problem hiding this comment.
Keep timestamp format consistent across all return paths.
generate_health_report now returns UTC-aware timestamps on the normal path, but the missing-DB branch (Line 63) still returns a naive timestamp. This makes HealthReport.timestamp format branch-dependent.
Proposed fix
- timestamp=datetime.now().isoformat(),
+ timestamp=datetime.now(UTC).isoformat(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/gradata/enhancements/scoring/reports.py` at line 146, The missing-DB
branch in generate_health_report returns a naive timestamp while the normal path
uses a UTC-aware ISO string, causing inconsistent HealthReport.timestamp values;
update the missing-DB return path to produce the same UTC-aware ISO timestamp
(e.g., use datetime.now(UTC).isoformat() or datetime.now(tz=UTC).isoformat()) so
every return path of generate_health_report assigns HealthReport.timestamp
consistently.
* docs: v0.6.1 changelog entry covering post-v0.6.0 work through PR #114 Covers: gradata.patterns deprecation (remove in v0.8.0), Alert dedup (#109), Meta-Harness A-D, multi-tenant SDK (#102), BM25 JIT ranking (#101), gradata seed/mine CLI, rule_verifier wiring, security hardening, and 67+66 ruff-violation fixes (#100, #103). Co-Authored-By: Gradata <noreply@gradata.ai> * docs(changelog): qualify env-centralization claim CodeRabbit flagged that GRADATA_RULE_VERIFIER is still read directly in rule_pipeline.py. Weaken the claim rather than block on the migration. Co-Authored-By: Gradata <noreply@gradata.ai> --------- Co-authored-by: Gradata <noreply@gradata.ai>
1 participant
Summary
Clears 66 pre-existing ruff violations that had been failing
SDK CIon every push tomain. No behavior change — auto-fixes + mechanical rewrites only.Breakdown
ruff check --fix: UP017 datetime.UTC modernization, I001 import sorting, F401 unused imports, UP037 quoted annotations, UP045 PEP-604 Optional, RUF100 stale noqa, F541 placeholderless f-strings, TC005 empty TC blocks.cli.py: collapse nested HTTPS-URL guard into single condition; 2× chmodtry/except→contextlib.suppress(OSError, AttributeError).correction_detector.py: drop unusedcorrection_textlocal.collaborative_filter.py:zip(..., strict=True)for cosine sim vectors.clustering.py: rename loop var to_categorywhere onlymembersis used.rule_pipeline.py: collapse INSTINCT→PATTERN and PATTERN→RULE nested-if pairs into singleandchains.loop_intelligence.py: 2×try/except/pass→contextlib.suppress.Verification
ruff check src/→ All checks passed.pytest tests/→ 2875 pass, 19 skipped, 2 xfailed. The 2 failures intest_scoped_brains.pyexist onmainwithout this PR and are brain-data bleed into unit tests (test fixture pollution), unrelated.pyright src/→ clean.Why
Unblocks normal-CI merge for PR #98 (RetainOrchestrator dedup) and PR #99 (SDK hook daemon) — those landed behind a red
SDK CIthat had to be admin-bypassed for #97.Generated with Gradata