fix(sdk): resolve 10 pyright type errors blocking CI#29
Conversation
- RuleCache: typed as dict[str, list] but stored str (brain.py caches format_rules_for_prompt output). Change to dict[str, str]. - Lesson: add _contradiction_streak field (was set dynamically, failing attribute check in self_improvement + rule_evolution). - behavioral_extractor: sorted(counts, key=counts.get) fails overload resolution; wrap with lambda. - rule_engine: assign example_draft/example_corrected to locals after None guard so pyright narrows the Optional. Pre-existing errors blocking CI on main for multiple sessions. Unblocks PR #28.
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.
📝 Walkthrough
WalkthroughAdded a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 docstrings
Comment |
Auto-fixed 12 (datetime.UTC alias, unused vars, zip strict, comprehensions) plus 5 manual fixes: - _embed.py: zip(a, b, strict=False) for cosine_distance - rule_graph.py: collapse nested ifs with 'and' - rule_tree.py: rename unused loop var path->_path, collapse nested contract check Unblocks sdk-ci.yml workflow on main.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/gradata/enhancements/behavioral_extractor.py (1)
663-670:⚠️ Potential issue | 🔴 CriticalFix CI blocker: unresolved Ruff UP037 on quoted annotations.
Line [663] and Line [670] still use quoted
Lessonannotations despitefrom __future__ import annotations; Ruff flags this and CI fails.Proposed fix
def detect_recurring_patterns( - lessons: list["Lesson"], + lessons: list[Lesson], min_corrections: int = 3, ) -> list[RecurringPattern]: @@ - by_cat: dict[str, list["Lesson"]] = {} + by_cat: dict[str, list[Lesson]] = {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/enhancements/behavioral_extractor.py` around lines 663 - 670, The type annotations use quoted "Lesson" in the function signature and the by_cat variable despite having from __future__ import annotations; remove the string quotes so they are real names (change lessons: list["Lesson"] to lessons: list[Lesson] and by_cat: dict[str, list["Lesson"]] to by_cat: dict[str, list[Lesson]]), leaving other types like RecurringPattern unchanged; update any other nearby quoted Lesson uses in that function to the unquoted form so Ruff UP037 is resolved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/gradata/enhancements/behavioral_extractor.py`:
- Around line 663-670: The type annotations use quoted "Lesson" in the function
signature and the by_cat variable despite having from __future__ import
annotations; remove the string quotes so they are real names (change lessons:
list["Lesson"] to lessons: list[Lesson] and by_cat: dict[str, list["Lesson"]] to
by_cat: dict[str, list[Lesson]]), leaving other types like RecurringPattern
unchanged; update any other nearby quoted Lesson uses in that function to the
unquoted form so Ruff UP037 is resolved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 10e43bc2-66ca-4edc-ae82-153a426631ef
📒 Files selected for processing (4)
src/gradata/_types.pysrc/gradata/enhancements/behavioral_extractor.pysrc/gradata/rules/cache.pysrc/gradata/rules/rule_engine.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Test (Python 3.11)
- GitHub Check: Test (Python 3.12)
- GitHub Check: Test (Python 3.13)
- GitHub Check: test (3.11)
- GitHub Check: test (3.12)
- GitHub Check: test (3.13)
- GitHub Check: Test (Python 3.13)
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (1)
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/enhancements/behavioral_extractor.pysrc/gradata/_types.pysrc/gradata/rules/rule_engine.pysrc/gradata/rules/cache.py
🪛 GitHub Actions: SDK CI
src/gradata/enhancements/behavioral_extractor.py
[error] 663-663: ruff check reported UP037: Remove quotes from type annotation lessons: list["Lesson"],.
[error] 670-670: ruff check reported UP037: Remove quotes from type annotation by_cat: dict[str, list["Lesson"]] = {}.
🔇 Additional comments (4)
src/gradata/enhancements/behavioral_extractor.py (1)
650-650: Nice pyright-safe key function update.The
lambdakey keeps ordering deterministic and avoids the overload issue cleanly.src/gradata/_types.py (1)
177-177: Good fix: formalizing contradiction streak on the dataclass.Adding
_contradiction_streaktoLessoneliminates dynamic-attribute fragility and aligns with current self-improvement usage paths.src/gradata/rules/rule_engine.py (1)
867-870: Optional narrowing is handled correctly here.Moving attribute reads into locals and guarding before slicing fixes the type issue cleanly.
src/gradata/rules/cache.py (1)
12-29: Cache value contract alignment looks correct.The
str-typed cache storage/get/put signatures are consistent with current rule formatting output and caller expectations.
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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/gradata/rules/rule_graph.py (1)
232-245:⚠️ Potential issue | 🟠 MajorEnforce confidence bounds before persistence.
confidenceis written directly to SQLite without constraining/validating range. Add a clamp (or raise) to guarantee valid values on Line 244 before insert.🔧 Proposed fix
def store_relationship( db_path: str | Path, rule_a_id: str, rule_b_id: str, rel_type: RuleRelationType, confidence: float = 0.5, ) -> None: """Store a typed relationship in SQLite.""" + confidence = max(0.0, min(1.0, confidence)) conn = sqlite3.connect(str(db_path)) conn.execute( "INSERT INTO rule_relationships " "(rule_a_id, rule_b_id, relationship, confidence, detected_at) " "VALUES (?, ?, ?, ?, ?)",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/rules/rule_graph.py` around lines 232 - 245, The confidence parameter is written to SQLite without validation; before the INSERT into rule_relationships (the block using rel_type.value and datetime.now(UTC).isoformat()), clamp confidence to the [0.0, 1.0] range (e.g., set confidence = max(0.0, min(1.0, confidence))) or alternatively raise a ValueError if out of bounds, and then use the validated/clamped confidence in the VALUES tuple.src/gradata/enhancements/pubsub_pipeline.py (1)
26-26: 🧹 Nitpick | 🔵 TrivialConsider adding type parameters for improved type precision.
The
Callabletype could be more specific, e.g.,Callable[[dict[str, Any]], Any]to indicate the handler signature. Similarly,list[dict]at lines 34 and 48 could belist[dict[str, Any]].While pyright passes without these (indicating they're not critical), adding them would improve type safety and IDE support.
♻️ Optional type precision improvements
- def __init__(self): - self._subscribers: dict[str, list[Callable]] = {} + def __init__(self): + self._subscribers: dict[str, list[Callable[[dict[str, Any]], Any]]] = {} self._event_log: list[dict] = [] - def subscribe(self, event_type: str, handler: Callable): + def subscribe(self, event_type: str, handler: Callable[[dict[str, Any]], Any]) -> None: if event_type not in self._subscribers: self._subscribers[event_type] = [] self._subscribers[event_type].append(handler) - def emit(self, event_type: str, data: dict[str, Any] | None = None) -> list[dict]: + def emit(self, event_type: str, data: dict[str, Any] | None = None) -> list[dict[str, Any]]: """Emit an event. Returns list of stage results."""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gradata/enhancements/pubsub_pipeline.py` at line 26, Update the vague typing to be more precise: change the declaration of self._subscribers to use a Callable that accepts an event dict (e.g., Callable[[dict[str, Any]], Any]) and update any occurrences of list[dict] (the buffer/queue types referenced in this module) to list[dict[str, Any]]; ensure you import Any from typing and adjust the type annotations on the related methods that append/consume these lists so signatures match the new Callable[[dict[str, Any]], Any] handler shape.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/gradata/enhancements/pubsub_pipeline.py`:
- Line 26: Update the vague typing to be more precise: change the declaration of
self._subscribers to use a Callable that accepts an event dict (e.g.,
Callable[[dict[str, Any]], Any]) and update any occurrences of list[dict] (the
buffer/queue types referenced in this module) to list[dict[str, Any]]; ensure
you import Any from typing and adjust the type annotations on the related
methods that append/consume these lists so signatures match the new
Callable[[dict[str, Any]], Any] handler shape.
In `@src/gradata/rules/rule_graph.py`:
- Around line 232-245: The confidence parameter is written to SQLite without
validation; before the INSERT into rule_relationships (the block using
rel_type.value and datetime.now(UTC).isoformat()), clamp confidence to the [0.0,
1.0] range (e.g., set confidence = max(0.0, min(1.0, confidence))) or
alternatively raise a ValueError if out of bounds, and then use the
validated/clamped confidence in the VALUES tuple.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e8f9d89f-2e7a-4b21-9f1d-2bd45a09b5eb
📒 Files selected for processing (7)
src/gradata/_embed.pysrc/gradata/enhancements/behavioral_extractor.pysrc/gradata/enhancements/meta_rules.pysrc/gradata/enhancements/pubsub_pipeline.pysrc/gradata/enhancements/self_improvement.pysrc/gradata/rules/rule_graph.pysrc/gradata/rules/rule_tree.py
💤 Files with no reviewable changes (1)
- src/gradata/enhancements/meta_rules.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: test (3.12)
- GitHub Check: test (3.11)
- GitHub Check: test (3.13)
- GitHub Check: Python 3.12
- GitHub Check: Python 3.11
- GitHub Check: Python 3.12
- GitHub Check: Python 3.11
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (1)
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/_embed.pysrc/gradata/enhancements/self_improvement.pysrc/gradata/enhancements/pubsub_pipeline.pysrc/gradata/rules/rule_tree.pysrc/gradata/enhancements/behavioral_extractor.pysrc/gradata/rules/rule_graph.py
🔇 Additional comments (10)
src/gradata/rules/rule_tree.py (2)
317-317: Unused loop variable rename is correct.Using
_pathmakes the intent explicit and avoids false-positive lint/type noise without changing behavior.
336-339: Conditional collapse preserves logic and improves readability.The short-circuit
andcheck is safe here and keeps contraction counting behavior intact.src/gradata/enhancements/self_improvement.py (1)
507-507: Type annotation update is correct and pyright-friendly.Using
lesson: Lessonhere is consistent withfrom __future__ import annotationsand keeps runtime behavior unchanged while improving type checking.src/gradata/enhancements/behavioral_extractor.py (2)
650-650: Good fix forsorted()key typing.
key=lambda k: counts[k]preserves behavior and resolves the key-function typing ambiguity cleanly.
663-663: DirectLessongenerics are a solid type-safety improvement.These annotations are consistent with the module’s typing style and improve pyright inference without changing logic.
Also applies to: 670-670
src/gradata/rules/rule_graph.py (3)
17-17: Good modernization of timezone handling.Using
UTCimport directly keeps timestamp code clearer and type-checker friendly.
212-213: Contradiction guard simplification looks correct.The flattened conditional preserves prior semantics and improves readability.
216-217: Reinforcement condition refactor is clean and equivalent.Combining category-match and overlap threshold into one guard is a solid simplification.
src/gradata/_embed.py (1)
222-222: LGTM: explicitstrict=Falseis clear and non-breaking.This keeps the previous truncation behavior explicit and avoids lint noise around implicit
zipstrictness.src/gradata/enhancements/pubsub_pipeline.py (1)
11-12: LGTM! Import modernization follows best practices.Moving
Callablefromtypingtocollections.abcaligns with PEP 585 and modern Python type hinting conventions (Python 3.9+). This is the recommended approach and correctly resolves type checking issues.
Merge from main introduced a duplicate Lesson._contradiction_streak declaration because both main (PR #29) and this branch added the field independently. Collapsed to a single field with a clearer comment. Co-Authored-By: Gradata <noreply@gradata.ai>
…xport (#26) * 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 * style(sdk): fix 15 ruff lint errors blocking PR #26 CI - SIM102 combine nested ifs in rule_graph / rule_tree - SIM105 contextlib.suppress instead of try/except/pass - B007 rename unused loop var path -> _path - B905 zip(strict=False) - 8 other auto-fixable All 91 rule_to_hook + rule_tree + rule_graph tests pass. Co-Authored-By: Gradata <noreply@gradata.ai> * fix(sdk): pyright errors — RuleCache str typing, Lesson._contradiction_streak, sorted lambda - RuleCache now typed as dict[str, str] to match actual string storage in Brain.apply_brain_rules (was dict[str, list]). - Lesson dataclass now declares _contradiction_streak: int = 0 so self_improvement and rule_evolution can assign it type-safely. - behavioral_extractor sorted() uses lambda with default 0 (counts.get can return None per type checker). - rule_engine.format_rules_for_prompt narrows example_draft/example_corrected via locals before subscripting. Pyright now reports 0 errors (was 10). Ruff stays green. All 2055 tests pass. * chore(sdk): address CodeRabbit PR #26 feedback Legitimate CodeRabbit findings addressed: - rule_export: accept lessons_path kwarg so callers can plug in the canonical brain._find_lessons_path() instead of hardcoding brain_root/'lessons.md'. CLI now passes the canonical path. [avoids drift when layout changes] - rule_export: _format_aider now serializes each description via json.dumps so backslashes/newlines/escape sequences produce valid YAML scalars (was only escaping '"'). - _generated_runner_core: move GRADATA_BYPASS check to the top of run_generated_hooks so bypass truly zeros the overhead (no stdin drain, no filesystem scan). - _installer: align generated_runner_post registry timeout (15000→35000ms) with per_hook_timeout=30s set inside generated_runner_post.py — prevents premature termination of slow pytest hooks. - auto_test.js.tmpl: hooks in this directory must fail open. Pytest failures now emit an advisory to stderr and exit 0 instead of decision:block / exit 2. - rule_graph.store_relationship: clamp confidence to [0.0, 1.0] before SQLite persistence per SDK coding guideline. - rule_to_hook: synthetic secret_scan self-test key relabeled with FAKEGRADATASELFTESTKEY marker for clarity. - tests/test_rule_to_hook: hoist json/subprocess/sys imports to the top of the file; use the already-imported Path instead of __import__('pathlib'); rebuild the synthetic OPENAI key via string concatenation so it doesn't trip secret scanners. Declined (with rationale): - cli.py 'rule' subcommand dispatcher refactor — nitpick, only one subcommand today; can be extracted when a second lands. - Moving [hooked] marker from lesson.description to structured metadata — lessons.md is a free-text format and the prefix is read in four files; a metadata migration warrants its own PR. Pyright: 0 errors. Ruff: green. 2055 tests pass. * fix(sdk): remove duplicate _contradiction_streak field (merge artifact) Merge from main introduced a duplicate Lesson._contradiction_streak declaration because both main (PR #29) and this branch added the field independently. Collapsed to a single field with a clearer comment. Co-Authored-By: Gradata <noreply@gradata.ai> * refactor(sdk): simplify pass on rule-to-hook branch - Unify render_hook template substitution: hoist shared {{RULE_TEXT}} / {{SOURCE_HASH}} replacements out of the file_size_check vs. pattern branches (dedup 3 chained replaces). - Collapse _hook_root() helper into install_hook — it was a single-caller private that duplicated the env-var + default pattern already inlined for the PostToolUse branch. Now both branches share one lookup. No behavior change; 2114 tests pass; ruff + pyright clean. Co-Authored-By: Gradata <noreply@gradata.ai> * fix(review): address CodeRabbit feedback round 4 on rule-to-hook * fix(cli): resolve pyright error on rule_cmd dispatch getattr returns Any | None; narrow to str before dict lookup to satisfy pyright's reportArgumentType. Fixes CI pyright failures on Python 3.11 and 3.12 introduced by commit 7c435b5. Co-Authored-By: Gradata <noreply@gradata.ai> --------- Co-authored-by: Oliver Le <oliver@gradata.com> Co-authored-by: Gradata <noreply@gradata.ai>
Summary
mainfor multiple sessions due to pyright (basic mode) errors in SDK — unrelated to recent cloud PRs.Fixes
RuleCachetypeddict[str, list]but storesstr(brain.py cachesformat_rules_for_promptoutput). Changed signatures tostr.Lesson._contradiction_streakset dynamically in self_improvement/rule_evolution but absent on dataclass. Added as field.sorted(counts, key=counts.get, ...)in behavioral_extractor fails overload resolution — wrapped with lambda.Test plan
pyright src/— 0 errors locally (was 10)pytest tests/— 2070 passed, 23 skippedGenerated with Gradata