Skip to content

feat: wire LLM meta-rule synthesis (Gemma native)#97

Merged
Gradata merged 4 commits intomainfrom
feat/wire-llm-meta-synthesis
Apr 17, 2026
Merged

feat: wire LLM meta-rule synthesis (Gemma native)#97
Gradata merged 4 commits intomainfrom
feat/wire-llm-meta-synthesis

Conversation

@Gradata
Copy link
Copy Markdown
Owner

@Gradata Gradata commented Apr 17, 2026

Summary

  • Wires LLM principle synthesis into synthesize_meta_rules_agentic. When LLM succeeds the MetaRule gets source="llm_synth" (passes INJECTABLE_META_SOURCES); on failure/no-creds falls back to the existing deterministic path — never blocks.
  • Adds native Gemma API caller (x-goog-api-key header) because Google's OpenAI-compat endpoint rejects AQ.-prefixed Google AI Studio keys with "Multiple authentication credentials received". Now GRADATA_GEMMA_API_KEY from .env actually works.
  • _resolve_llm_credentials keeps GRADATA_LLM_KEY+GRADATA_LLM_BASE as first priority; Gemma is a fallback.

Closes the S113-era blocker: the deterministic filter meant zero meta-rules ever reached the AI. Ablation (2026-04-14, 432 trials) showed deterministic principles regress correctness — LLM-synthesized ones are what should inject.

Verification

  • pytest tests/test_agentic_synthesis.py — 23 passed (20 existing + 3 new covering LLM-success, None fallback, no-creds paths).
  • Live Gemma call against 3 getattr-related RULE-state lessons produced: "When accessing potentially missing object attributes, utilize a function call with a default value instead of explicit error handling or preliminary existence checks." — stored with source="llm_synth", passes the injection filter.

Test plan

  • Unit tests: LLM-success sets source="llm_synth"; None return falls back to deterministic
  • No-credentials case returns None (doesn't raise)
  • Live end-to-end: Gemma → agentic synth → save_meta_rules → load_meta_rules → format_meta_rules_for_prompt emits <brain-meta-rules> block
  • Existing test_successful_synthesis_produces_meta_rule still asserts source="deterministic" when no LLM creds

Generated with Gradata

Gradata and others added 2 commits April 16, 2026 18:16
Meta-rules were synthesized deterministically and hardcoded source="deterministic"
which the injection filter rejects (INJECTABLE_META_SOURCES = {llm_synth, human_curated}),
so zero meta-rules ever reached the AI. Now tries LLM principle synthesis first via
the existing synthesise_principle_llm, setting source="llm_synth" on success.
LLM failure (no creds, network error, bad response) degrades silently to the
existing deterministic path — non-blocking by design.

_resolve_llm_credentials gains a GRADATA_GEMMA_API_KEY fallback pointing at Google
AI Studio's OpenAI-compat endpoint so users with the Gemma free tier can wire it
without also setting GRADATA_LLM_BASE.

Test coverage: LLM-success path, None fallback path, and no-credentials path.
Existing 20 tests pass; 3 new tests added.

Co-Authored-By: Gradata <noreply@gradata.ai>
The OpenAI-compat Gemma endpoint (generativelanguage.googleapis.com/v1beta/openai)
rejects AQ.-prefixed Google AI Studio keys with "Multiple authentication credentials
received" when passed as Bearer. Google's native generateContent endpoint accepts
them via the x-goog-api-key header.

_try_llm_principle now routes GRADATA_GEMMA_API_KEY through a native Gemma caller
so the free-tier Gemma key in Oliver's .env actually produces llm_synth meta-rules
instead of silently falling back to deterministic. Explicit GRADATA_LLM_KEY + BASE
still takes precedence for other OpenAI-compat providers.

Verified live: one Gemma synthesis run against 3 getattr-related RULE-state lessons
produces: "When accessing potentially missing object attributes, utilize a function
call with a default value instead of explicit error handling or preliminary
existence checks." source=llm_synth, passes INJECTABLE_META_SOURCES filter.

Co-Authored-By: Gradata <noreply@gradata.ai>
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

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

Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 17, 2026

📝 Walkthrough
  • LLM-first synthesis: synthesize_meta_rules_agentic() tries LLM-based principle synthesis first; on success MetaRule.principle is set to the LLM text and MetaRule.source="llm_synth" (and saved). On LLM failure, missing creds, or errors it falls back to the existing deterministic principle and never raises.
  • Gemma-native integration: added _call_gemma_native() to call Google AI Studio’s OpenAI-compatible generateContent endpoint using the x-goog-api-key header (works around AQ.-prefixed key rejection when sent as Bearer). Live Gemma call verified an example meta-rule saved with source="llm_synth".
  • Credential resolution: _resolve_llm_credentials() prefers GRADATA_LLM_KEY + GRADATA_LLM_BASE first; falls back to GRADATA_GEMMA_API_KEY with GRADATA_GEMMA_BASE/GRADATA_GEMMA_MODEL defaults if present; otherwise returns empty/previous defaults.
  • MetaRule injection policy: MetaRule.source is now set dynamically ("llm_synth" or "deterministic"); deterministic heuristics remain a non-blocking fallback and are not injected when INJECTABLE_META_SOURCES restricts to {"llm_synth","human_curated"}.
  • Rule pipeline enhancements: added _patterns_to_graduated_lessons() which normalizes descriptions (strips noise prefixes), deduplicates, and synthesizes RULE-state Lesson objects; run_rule_pipeline adds Phase 1.6 to lift qualifying correction_patterns and increments a new PipelineResult.patterns_lifted counter.
  • Tests: added 3 agentic-synthesis tests (LLM success, deterministic fallback, no-creds silent degradation) and multiple rule_pipeline tests (lifting, noise stripping, empty-db). Reported verification: 23 pytest tests passed (20 existing + 3 new).
  • Typing/CI fixes: adjusted type annotations (Lesson import and list[Lesson] return), added type: ignore[attr-defined] at Brain._causal_chain runtime-injection sites to unblock pyright/CI.
  • Public/API notes and potential impact: PipelineResult gained a new public field patterns_lifted (breaking for callers that assume strict shape); _generate_skill_file narrowed its parameter type from a generic object to Lesson (type-level change). No other runtime-breaking API changes; behavior changes are internal to synthesis/pipeline logic.
  • No security fixes introduced beyond the key-handling change to support Gemma keys; LLM failures are handled gracefully (no secrets exposure).

Walkthrough

Added Gemma-native support and dual-path LLM credential resolution; implemented best-effort behavioral principle synthesis (OpenAI-compatible or Gemma native) with deterministic fallback and source tagging. Added pattern-to-lesson lifting in the rule pipeline (normalization, dedupe, Phase 1.6) and corresponding tests.

Changes

Cohort / File(s) Summary
LLM Principle Synthesis
src/gradata/enhancements/meta_rules.py
Add dual-path credential resolution (OpenAI-compatible env vars or Gemma-native env vars); new helpers _build_principle_prompt(), _call_gemma_native(), _try_llm_principle(); attempt LLM/Gemma synthesis per category and set MetaRule.source to "llm_synth" or fall back to "deterministic".
Rule Pipeline — Pattern Lifting
src/gradata/enhancements/rule_pipeline.py
Add PipelineResult.patterns_lifted counter; add _normalize_pattern_description() and _patterns_to_graduated_lessons() (query graduation candidates, normalize/strip noise, dedupe, synthesize Lesson objects); integrate as Phase 1.6 in run_rule_pipeline, increment counter and record errors.
Core Type Annotation
src/gradata/_core.py
Add # type: ignore[attr-defined] to causal-chain initialization and add_link() calls to suppress static type checks; no runtime behavior change.
Tests — Agentic Synthesis
tests/test_agentic_synthesis.py
Add tests asserting LLM principle synthesis success sets meta.principle and source == "llm_synth", failure falls back to deterministic principle with source == "deterministic", and _try_llm_principle() returns None when env vars absent.
Tests — Rule Pipeline
tests/test_rule_pipeline.py
Add helpers to seed correction_patterns and tests for _patterns_to_graduated_lessons: lifts qualifying clusters, strips noise prefixes, and returns empty when DB missing.

Sequence Diagram(s)

sequenceDiagram
    participant Caller as Caller
    participant Synth as synthesize_meta_rules_agentic()
    participant TryLLM as _try_llm_principle()
    participant Resolve as _resolve_llm_credentials()
    participant OpenAI as OpenAI-compatible API
    participant Gemma as Gemma native API

    Caller->>Synth: request meta-rule synthesis (grouped rules)
    Synth->>TryLLM: attempt principle synthesis for group
    TryLLM->>Resolve: request credentials
    Resolve-->>TryLLM: credentials (OpenAI or Gemma or none)
    alt OpenAI credentials present
        TryLLM->>OpenAI: synthesise_principle_llm()
        OpenAI-->>TryLLM: principle text
    else Gemma credentials present
        TryLLM->>Gemma: _call_gemma_native(generateContent)
        Gemma-->>TryLLM: principle text
    else No credentials
        TryLLM-->>TryLLM: return None
    end
    alt principle text returned
        TryLLM-->>Synth: principle text
        Synth->>Synth: set meta.principle and meta.source = "llm_synth"
    else None returned
        TryLLM-->>Synth: None
        Synth->>Synth: keep deterministic principle and set meta.source = "deterministic"
    end
    Synth-->>Caller: MetaRules with principle and source
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: wiring LLM meta-rule synthesis with Gemma native support, which is the primary objective across multiple files in the changeset.
Description check ✅ Passed The description clearly explains the key changes: LLM principle synthesis integration, Gemma API caller implementation, credential resolution strategy, and the problem it solves. All content is directly related to the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 85.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/wire-llm-meta-synthesis

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot added the feature label Apr 17, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/gradata/enhancements/meta_rules.py`:
- Around line 671-691: The code in _try_llm_principle duplicates environment
checks (_GRADATA_LLM_KEY/_GRADATA_LLM_BASE and _GRADATA_GEMMA_API_KEY) already
handled by _resolve_llm_credentials; extract provider-detection into a shared
helper (e.g., _detect_llm_provider) or change _resolve_llm_credentials to return
a provider tag plus credential bundle, then update _try_llm_principle to call
that helper and branch on the provider value (OpenAI-compatible vs Gemma) and
only use corresponding creds to call synthesise_principle_llm or
_call_gemma_native, removing the duplicate os.environ.get checks and
centralizing credential resolution logic.
- Around line 684-686: Remove the unnecessary "noqa: BLE001" directive on the
except clause: locate the try/except block that currently reads "except
Exception as exc:  # noqa: BLE001 -- degrade to deterministic" (the block that
logs "_log.debug('OpenAI-compat synthesis failed for %s: %s', category, exc)"
and returns None) and delete the " # noqa: BLE001 -- degrade to deterministic"
suffix so the line becomes simply "except Exception as exc:". Ensure no other
changes to the logging or return behavior.
- Around line 605-616: The code silently skips partial OpenAI-compatible
credentials (when one of GRADATA_LLM_KEY or GRADATA_LLM_BASE is set but the
other is empty), so update the function that evaluates key, base, model
(variables key, base, model) to detect this partial-credential case and emit a
debug (or warning) log indicating which env var is missing before falling
through to the Gemma check; specifically, after reading key and base from env
(GRADATA_LLM_KEY, GRADATA_LLM_BASE) and before the existing `if key and base:
return ...` block add a branch that logs a clear message referencing the missing
piece (e.g., "GRADATA_LLM_KEY set but GRADATA_LLM_BASE missing") so users aren’t
silently misrouted to Gemma, then continue with the existing fallthrough
behavior.

In `@tests/test_agentic_synthesis.py`:
- Around line 352-362: Add a new test that verifies credential precedence when
both OpenAI-compatible and Gemma credentials are present: use monkeypatch.setenv
to set GRADATA_LLM_KEY and GRADATA_LLM_BASE as well as GRADATA_GEMMA_API_KEY and
GRADATA_GEMMA_BASE, call mr._try_llm_principle(rules, "DRAFTING") (same rules
from _make_rule_group) and assert the returned principle matches the result when
only GRADATA_LLM_* is set (i.e., precedence is OpenAI-compatible); then clear
GRADATA_LLM_* and assert the result changes to the Gemma-backed principle to
prove precedence behavior.
🪄 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: f03bdd6e-6fdc-41e6-8dfb-65080cdf02b9

📥 Commits

Reviewing files that changed from the base of the PR and between ae423a7 and 296cbed.

📒 Files selected for processing (2)
  • src/gradata/enhancements/meta_rules.py
  • tests/test_agentic_synthesis.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). (7)
  • GitHub Check: Test (Python 3.13)
  • GitHub Check: Test (Python 3.12)
  • GitHub Check: test (3.13)
  • GitHub Check: Test (Python 3.11)
  • GitHub Check: test (3.12)
  • GitHub Check: test (3.11)
  • GitHub Check: Test (Python 3.12)
🧰 Additional context used
📓 Path-based instructions (2)
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_agentic_synthesis.py
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/meta_rules.py
🪛 GitHub Actions: SDK CI
src/gradata/enhancements/meta_rules.py

[error] 684-684: ruff RUF100: Unused noqa directive (non-enabled: BLE001). help: Remove unused noqa directive

🔇 Additional comments (6)
src/gradata/enhancements/meta_rules.py (4)

589-591: LGTM!

The Gemma default constants are appropriately defined for the Google AI Studio OpenAI-compatible endpoint.


619-628: LGTM!

The prompt builder correctly caps input to 10 rules and constructs a well-structured synthesis prompt consistent with the existing llm_synthesizer.py pattern.


631-653: LGTM!

The Gemma native API caller correctly uses the x-goog-api-key header (required for Google AI Studio), validates response length, and handles errors gracefully by returning None.


1022-1044: LGTM!

The LLM-first synthesis with deterministic fallback is well-implemented. The source field correctly tracks provenance (llm_synth vs deterministic), and the fallback logic preserves the original deterministic behavior.

tests/test_agentic_synthesis.py (2)

323-336: LGTM!

The test correctly mocks _try_llm_principle to return a specific string and verifies both the source field and exact principle text.


339-349: LGTM!

The test properly validates the deterministic fallback path by mocking _try_llm_principle to return None and asserting the expected source and principle substring.

Comment on lines +605 to +616
if key and base:
return key, base, model

gemma_key = os.environ.get("GRADATA_GEMMA_API_KEY", "")
if gemma_key:
return (
gemma_key,
os.environ.get("GRADATA_GEMMA_BASE", _GEMMA_DEFAULT_BASE),
os.environ.get("GRADATA_GEMMA_MODEL", _GEMMA_DEFAULT_MODEL),
)

return "", "", model
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Edge case: partial OpenAI-compat credentials fall through silently.

If GRADATA_LLM_KEY is set but GRADATA_LLM_BASE is empty, the function silently falls through to the Gemma check. This could surprise users who set only the key and expect an error. Consider logging a debug message when one is set without the other.

💡 Proposed logging for partial credentials
     key = os.environ.get("GRADATA_LLM_KEY", "")
     base = os.environ.get("GRADATA_LLM_BASE", "")
     model = os.environ.get("GRADATA_LLM_MODEL", "gpt-4o-mini")
     if key and base:
         return key, base, model
+    if key and not base:
+        _log.debug("GRADATA_LLM_KEY set but GRADATA_LLM_BASE missing; checking Gemma fallback")
+    elif base and not key:
+        _log.debug("GRADATA_LLM_BASE set but GRADATA_LLM_KEY missing; checking Gemma fallback")

     gemma_key = os.environ.get("GRADATA_GEMMA_API_KEY", "")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/gradata/enhancements/meta_rules.py` around lines 605 - 616, The code
silently skips partial OpenAI-compatible credentials (when one of
GRADATA_LLM_KEY or GRADATA_LLM_BASE is set but the other is empty), so update
the function that evaluates key, base, model (variables key, base, model) to
detect this partial-credential case and emit a debug (or warning) log indicating
which env var is missing before falling through to the Gemma check;
specifically, after reading key and base from env (GRADATA_LLM_KEY,
GRADATA_LLM_BASE) and before the existing `if key and base: return ...` block
add a branch that logs a clear message referencing the missing piece (e.g.,
"GRADATA_LLM_KEY set but GRADATA_LLM_BASE missing") so users aren’t silently
misrouted to Gemma, then continue with the existing fallthrough behavior.

Comment on lines +671 to +691
k = os.environ.get("GRADATA_LLM_KEY", "")
b = os.environ.get("GRADATA_LLM_BASE", "")
if k and b:
try:
from gradata.enhancements.llm_synthesizer import synthesise_principle_llm

return synthesise_principle_llm(
lessons=rules,
theme=category,
api_key=k,
api_base=b,
model=os.environ.get("GRADATA_LLM_MODEL", "gpt-4o-mini"),
)
except Exception as exc: # noqa: BLE001 -- degrade to deterministic
_log.debug("OpenAI-compat synthesis failed for %s: %s", category, exc)
return None

g = os.environ.get("GRADATA_GEMMA_API_KEY", "")
if g:
model = os.environ.get("GRADATA_GEMMA_MODEL", _GEMMA_DEFAULT_MODEL)
return _call_gemma_native(_build_principle_prompt(rules, category), g, model)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider extracting provider detection to reduce credential-resolution duplication.

_try_llm_principle re-reads the same environment variables that _resolve_llm_credentials handles. While the branching logic differs (provider selection vs. credential retrieval), extracting a shared helper or returning a provider indicator from _resolve_llm_credentials could improve maintainability.

🧰 Tools
🪛 GitHub Actions: SDK CI

[error] 684-684: ruff RUF100: Unused noqa directive (non-enabled: BLE001). help: Remove unused noqa directive

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/gradata/enhancements/meta_rules.py` around lines 671 - 691, The code in
_try_llm_principle duplicates environment checks
(_GRADATA_LLM_KEY/_GRADATA_LLM_BASE and _GRADATA_GEMMA_API_KEY) already handled
by _resolve_llm_credentials; extract provider-detection into a shared helper
(e.g., _detect_llm_provider) or change _resolve_llm_credentials to return a
provider tag plus credential bundle, then update _try_llm_principle to call that
helper and branch on the provider value (OpenAI-compatible vs Gemma) and only
use corresponding creds to call synthesise_principle_llm or _call_gemma_native,
removing the duplicate os.environ.get checks and centralizing credential
resolution logic.

Comment on lines +684 to +686
except Exception as exc: # noqa: BLE001 -- degrade to deterministic
_log.debug("OpenAI-compat synthesis failed for %s: %s", category, exc)
return None
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix pipeline failure: remove unused noqa directive.

The ruff RUF100 error indicates BLE001 is not enabled, making the noqa comment unnecessary.

🔧 Proposed fix
-        except Exception as exc:  # noqa: BLE001 -- degrade to deterministic
+        except Exception as exc:  # degrade to deterministic
🧰 Tools
🪛 GitHub Actions: SDK CI

[error] 684-684: ruff RUF100: Unused noqa directive (non-enabled: BLE001). help: Remove unused noqa directive

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/gradata/enhancements/meta_rules.py` around lines 684 - 686, Remove the
unnecessary "noqa: BLE001" directive on the except clause: locate the try/except
block that currently reads "except Exception as exc:  # noqa: BLE001 -- degrade
to deterministic" (the block that logs "_log.debug('OpenAI-compat synthesis
failed for %s: %s', category, exc)" and returns None) and delete the " # noqa:
BLE001 -- degrade to deterministic" suffix so the line becomes simply "except
Exception as exc:". Ensure no other changes to the logging or return behavior.

Comment on lines +352 to +362
def test_try_llm_principle_returns_none_without_creds(monkeypatch):
"""_try_llm_principle degrades silently when no credentials configured."""
import gradata.enhancements.meta_rules as mr

monkeypatch.delenv("GRADATA_LLM_KEY", raising=False)
monkeypatch.delenv("GRADATA_LLM_BASE", raising=False)
monkeypatch.delenv("GRADATA_GEMMA_API_KEY", raising=False)
monkeypatch.delenv("GRADATA_GEMMA_BASE", raising=False)

rules = _make_rule_group("DRAFTING", n=3)
assert mr._try_llm_principle(rules, "DRAFTING") is None
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

LGTM, but consider adding a precedence test.

This test correctly validates silent degradation when no credentials are configured. However, there's no test coverage for the precedence behavior when both GRADATA_LLM_KEY/GRADATA_LLM_BASE and GRADATA_GEMMA_API_KEY are set. A test confirming that OpenAI-compat credentials take precedence would prevent future regressions in the resolution order.

Would you like me to draft a test case for credential precedence verification?

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_agentic_synthesis.py` around lines 352 - 362, Add a new test that
verifies credential precedence when both OpenAI-compatible and Gemma credentials
are present: use monkeypatch.setenv to set GRADATA_LLM_KEY and GRADATA_LLM_BASE
as well as GRADATA_GEMMA_API_KEY and GRADATA_GEMMA_BASE, call
mr._try_llm_principle(rules, "DRAFTING") (same rules from _make_rule_group) and
assert the returned principle matches the result when only GRADATA_LLM_* is set
(i.e., precedence is OpenAI-compatible); then clear GRADATA_LLM_* and assert the
result changes to the Gemma-backed principle to prove precedence behavior.

Closes the orphaned nervous-system link: correction_patterns had 437 entries
but query_graduation_candidates() had no production caller, so meta-rule
synthesis starved whenever lessons.md was empty (as it is on fresh brains).

run_rule_pipeline now pulls graduation-ready clusters (sessions >= 2, severity
weight >= 3.0) and lifts them into synthetic RULE-state lessons with confidence
0.92 -- above the 0.90 min for synthesis. [AUTO] evaluator noise is dropped;
"User corrected: X" and "X" are normalized so dedup actually catches duplicates.

Verified live against Oliver's brain: 18 patterns lifted, 3 llm_synth meta-rules
synthesized by Gemma across LEADS/DEMO_PREP/GENERAL categories from real user
corrections (e.g. "When engaging with potential customers, prioritize qualifying
and progressing interest... instead of offering easy outs").

Co-Authored-By: Gradata <noreply@gradata.ai>
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

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

Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🤖 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/rule_pipeline.py`:
- Around line 397-401: existing_keys is built once before the loop and never
updated, so duplicates within pattern_lessons can be appended multiple times;
update the dedup set inside the loop after appending each new lesson by adding
the tuple (pl.category, pl.description) to existing_keys so the check
(pl.category, pl.description) not in existing_keys correctly prevents
duplicates; modify the loop that iterates over pattern_lessons (which currently
appends to all_lessons and increments result.patterns_lifted) to also update
existing_keys after a successful append.
- Around line 95-99: The code is using a hardcoded fallback "2026-01-01" for
first_seen which can produce misleading metadata; update the logic around the
first_seen variable and the Lesson(...) creation to validate/parse
row.get("first_seen") (e.g., try parsing ISO date or check regex), and if
invalid or missing either set date to a safe dynamic fallback like
datetime.date.today().isoformat() or omit adding the Lesson entirely; change the
assignment that builds first_seen and the lessons.append(...) call (the
Lesson(...) instantiation and LessonState.RULE usage) to use the
validated/parsing result instead of the hardcoded "2026-01-01".
- Around line 49-53: The function signature in rule_pipeline.py currently uses
quoted forward references for the db_path and return types ("Path" and "list")
which triggers UP037 — remove the quotes and use the actual types (e.g., Path
and list or typing.List as appropriate) in the signature (identify the function
by the parameters db_path, current_session, min_sessions, min_score). Also fix
the import block formatting (the unsorted/misformatted imports referenced in the
review) by sorting and reformatting the imports (run isort/ruff or manually
reorder/format the import statements) so they comply with I001; after changes,
run the linter to confirm UP037 and I001 are resolved.

In `@tests/test_rule_pipeline.py`:
- Around line 543-586: Convert the repetitive boundary tests in
tests/test_rule_pipeline.py into pytest parametrized cases: replace the separate
test_patterns_to_graduated_lessons_lifts_qualifying_clusters and
test_patterns_to_graduated_lessons_strips_noise with a single parametrized test
that calls _patterns_to_graduated_lessons (from
gradata.enhancements.rule_pipeline) for multiple inputs, where each param row
supplies the seeded pattern list, current_session, expected_count,
expected_categories (or expected_description) and any min_session/min_score
variations you want to assert; ensure you include a case for the absent DB (or
keep test_patterns_to_graduated_lessons_empty_when_no_db) and name parameters to
document the lift thresholds and noise-shaping boundary values so the
min_session/min_score behavior is explicit.
- Around line 543-561: The test
test_patterns_to_graduated_lessons_lifts_qualifying_clusters currently asserts
lesson.confidence >= 0.90 which is too loose; update the assertion to check the
exact synthesized confidence using pytest.approx (e.g. assert l.confidence ==
pytest.approx(<expected_value>, rel=1e-3)) for each lesson returned by
_patterns_to_graduated_lessons and keep the other checks (state ==
LessonState.RULE and category sorting) the same so regressions in the confidence
contract are caught.
🪄 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: df3267c9-73e1-41d0-bedf-3a674342660e

📥 Commits

Reviewing files that changed from the base of the PR and between 296cbed and 9604d26.

📒 Files selected for processing (2)
  • src/gradata/enhancements/rule_pipeline.py
  • tests/test_rule_pipeline.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). (5)
  • GitHub Check: Test (Python 3.12)
  • GitHub Check: test (3.12)
  • GitHub Check: test (3.13)
  • GitHub Check: test (3.11)
  • GitHub Check: Test (Python 3.11)
🧰 Additional context used
📓 Path-based instructions (2)
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_pipeline.py
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/rule_pipeline.py
🪛 GitHub Actions: SDK CI
src/gradata/enhancements/rule_pipeline.py

[error] 49-49: ruff check (UP037): Remove quotes from type annotation. --> src/gradata/enhancements/rule_pipeline.py:49:14


[error] 53-53: ruff check (UP037): Remove quotes from type annotation. --> src/gradata/enhancements/rule_pipeline.py:53:6


[error] 63-66: ruff check (I001): Import block is un-sorted or un-formatted. Organize imports. --> src/gradata/enhancements/rule_pipeline.py:63:9


[error] 108-110: ruff check (UP037): Remove quotes from type annotation. --> src/gradata/enhancements/rule_pipeline.py:108:13


[error] 110-110: ruff check (UP037): Remove quotes from type annotation. --> src/gradata/enhancements/rule_pipeline.py:110:6


[error] 191-191: ruff check (UP037): Remove quotes from type annotation. --> src/gradata/enhancements/rule_pipeline.py:191:40


[error] 374-375: ruff check (I001): Import block is un-sorted or un-formatted. Organize imports. --> src/gradata/enhancements/rule_pipeline.py:374:17


[error] 412-415: ruff check (SIM102): Use a single if statement instead of nested if statements. --> src/gradata/enhancements/rule_pipeline.py:412:9


[error] 419-423: ruff check (I001): Import block is un-sorted or un-formatted. Organize imports. --> src/gradata/enhancements/rule_pipeline.py:419:9


[error] 474-474: ruff check (I001): Import block is un-sorted or un-formatted. Organize imports. --> src/gradata/enhancements/rule_pipeline.py:474:9


[error] 617-617: ruff check (I001): Import block is un-sorted or un-formatted. Organize imports. --> src/gradata/enhancements/rule_pipeline.py:617:9

Comment thread src/gradata/enhancements/rule_pipeline.py Outdated
Comment on lines +95 to +99
first_seen = str(row.get("first_seen") or "")[:10] or "2026-01-01"
lessons.append(Lesson(
date=first_seen,
state=LessonState.RULE,
confidence=0.92,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Avoid hardcoded fallback lesson date.

Using "2026-01-01" as fallback can write misleading metadata if first_seen is missing/malformed.

Safer fallback
-        first_seen = str(row.get("first_seen") or "")[:10] or "2026-01-01"
+        from datetime import date as _date
+        first_seen = str(row.get("first_seen") or "")[:10] or _date.today().isoformat()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
first_seen = str(row.get("first_seen") or "")[:10] or "2026-01-01"
lessons.append(Lesson(
date=first_seen,
state=LessonState.RULE,
confidence=0.92,
from datetime import date as _date
first_seen = str(row.get("first_seen") or "")[:10] or _date.today().isoformat()
lessons.append(Lesson(
date=first_seen,
state=LessonState.RULE,
confidence=0.92,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/gradata/enhancements/rule_pipeline.py` around lines 95 - 99, The code is
using a hardcoded fallback "2026-01-01" for first_seen which can produce
misleading metadata; update the logic around the first_seen variable and the
Lesson(...) creation to validate/parse row.get("first_seen") (e.g., try parsing
ISO date or check regex), and if invalid or missing either set date to a safe
dynamic fallback like datetime.date.today().isoformat() or omit adding the
Lesson entirely; change the assignment that builds first_seen and the
lessons.append(...) call (the Lesson(...) instantiation and LessonState.RULE
usage) to use the validated/parsing result instead of the hardcoded
"2026-01-01".

Comment on lines +397 to +401
existing_keys = {(l.category, l.description) for l in all_lessons}
for pl in pattern_lessons:
if (pl.category, pl.description) not in existing_keys:
all_lessons.append(pl)
result.patterns_lifted += 1
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Update dedup set after append in Phase 1.6.

existing_keys is initialized once and never updated, so in-loop dedup only reflects preexisting lessons.

Dedup fix
             for pl in pattern_lessons:
                 if (pl.category, pl.description) not in existing_keys:
                     all_lessons.append(pl)
+                    existing_keys.add((pl.category, pl.description))
                     result.patterns_lifted += 1
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
existing_keys = {(l.category, l.description) for l in all_lessons}
for pl in pattern_lessons:
if (pl.category, pl.description) not in existing_keys:
all_lessons.append(pl)
result.patterns_lifted += 1
existing_keys = {(l.category, l.description) for l in all_lessons}
for pl in pattern_lessons:
if (pl.category, pl.description) not in existing_keys:
all_lessons.append(pl)
existing_keys.add((pl.category, pl.description))
result.patterns_lifted += 1
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/gradata/enhancements/rule_pipeline.py` around lines 397 - 401,
existing_keys is built once before the loop and never updated, so duplicates
within pattern_lessons can be appended multiple times; update the dedup set
inside the loop after appending each new lesson by adding the tuple
(pl.category, pl.description) to existing_keys so the check (pl.category,
pl.description) not in existing_keys correctly prevents duplicates; modify the
loop that iterates over pattern_lessons (which currently appends to all_lessons
and increments result.patterns_lifted) to also update existing_keys after a
successful append.

Comment on lines +543 to +586
def test_patterns_to_graduated_lessons_lifts_qualifying_clusters(tmp_path):
from gradata.enhancements.rule_pipeline import _patterns_to_graduated_lessons

db_path = tmp_path / "system.db"
_seed_correction_patterns(db_path, [
("h1", "LEADS", "Don't give prospects a way out when interest is stated", 10, "major", 2.0, "2026-04-01"),
("h1", "LEADS", "Don't give prospects a way out when interest is stated", 11, "major", 2.0, "2026-04-02"),
("h2", "DEMO_PREP", "Always trigger post-demo workflow", 10, "major", 2.0, "2026-04-01"),
("h2", "DEMO_PREP", "Always trigger post-demo workflow", 11, "major", 2.0, "2026-04-02"),
])

lessons = _patterns_to_graduated_lessons(db_path, current_session=12)
assert len(lessons) == 2
cats = sorted(l.category for l in lessons)
assert cats == ["DEMO_PREP", "LEADS"]
for l in lessons:
assert l.state == LessonState.RULE
assert l.confidence >= 0.90


def test_patterns_to_graduated_lessons_strips_noise(tmp_path):
"""[AUTO] evaluator noise and 'User corrected:' prefixes must be normalized."""
from gradata.enhancements.rule_pipeline import _patterns_to_graduated_lessons

db_path = tmp_path / "system.db"
_seed_correction_patterns(db_path, [
("h1", "ACCURACY", "[AUTO] heuristic evaluator output", 10, "minor", 2.0, "2026-04-01"),
("h1", "ACCURACY", "[AUTO] heuristic evaluator output", 11, "minor", 2.0, "2026-04-02"),
("h2", "LEADS", "User corrected: Use reply CTAs not booking links", 10, "major", 2.0, "2026-04-01"),
("h2", "LEADS", "User corrected: Use reply CTAs not booking links", 11, "major", 2.0, "2026-04-02"),
("h3", "LEADS", "Use reply CTAs not booking links", 12, "major", 2.0, "2026-04-03"),
("h3", "LEADS", "Use reply CTAs not booking links", 13, "major", 2.0, "2026-04-04"),
])

lessons = _patterns_to_graduated_lessons(db_path, current_session=14)
assert len(lessons) == 1
assert lessons[0].category == "LEADS"
assert lessons[0].description == "Use reply CTAs not booking links"


def test_patterns_to_graduated_lessons_empty_when_no_db(tmp_path):
from gradata.enhancements.rule_pipeline import _patterns_to_graduated_lessons

assert _patterns_to_graduated_lessons(tmp_path / "absent.db", current_session=1) == []
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider parametrizing lift thresholds and noise-shaping boundary cases.

These new cases are strong; converting them to pytest.mark.parametrize would reduce duplication and make min-session/min-score boundaries clearer.

As per coding guidelines, tests/**: parametrized tests are preferred for boundary conditions.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_rule_pipeline.py` around lines 543 - 586, Convert the repetitive
boundary tests in tests/test_rule_pipeline.py into pytest parametrized cases:
replace the separate
test_patterns_to_graduated_lessons_lifts_qualifying_clusters and
test_patterns_to_graduated_lessons_strips_noise with a single parametrized test
that calls _patterns_to_graduated_lessons (from
gradata.enhancements.rule_pipeline) for multiple inputs, where each param row
supplies the seeded pattern list, current_session, expected_count,
expected_categories (or expected_description) and any min_session/min_score
variations you want to assert; ensure you include a case for the absent DB (or
keep test_patterns_to_graduated_lessons_empty_when_no_db) and name parameters to
document the lift thresholds and noise-shaping boundary values so the
min_session/min_score behavior is explicit.

Comment on lines +543 to +561
def test_patterns_to_graduated_lessons_lifts_qualifying_clusters(tmp_path):
from gradata.enhancements.rule_pipeline import _patterns_to_graduated_lessons

db_path = tmp_path / "system.db"
_seed_correction_patterns(db_path, [
("h1", "LEADS", "Don't give prospects a way out when interest is stated", 10, "major", 2.0, "2026-04-01"),
("h1", "LEADS", "Don't give prospects a way out when interest is stated", 11, "major", 2.0, "2026-04-02"),
("h2", "DEMO_PREP", "Always trigger post-demo workflow", 10, "major", 2.0, "2026-04-01"),
("h2", "DEMO_PREP", "Always trigger post-demo workflow", 11, "major", 2.0, "2026-04-02"),
])

lessons = _patterns_to_graduated_lessons(db_path, current_session=12)
assert len(lessons) == 2
cats = sorted(l.category for l in lessons)
assert cats == ["DEMO_PREP", "LEADS"]
for l in lessons:
assert l.state == LessonState.RULE
assert l.confidence >= 0.90

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Assert the synthesized confidence precisely, not via threshold.

The check at Line 560 (>= 0.90) is too broad and can hide regressions in the synthesized confidence contract.

Proposed test tightening
     for l in lessons:
         assert l.state == LessonState.RULE
-        assert l.confidence >= 0.90
+        assert l.confidence == pytest.approx(0.92)

As per coding guidelines, tests/**: assertions should check specific values (not broad truthy/threshold checks), and floating-point comparisons should use pytest.approx.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_rule_pipeline.py` around lines 543 - 561, The test
test_patterns_to_graduated_lessons_lifts_qualifying_clusters currently asserts
lesson.confidence >= 0.90 which is too loose; update the assertion to check the
exact synthesized confidence using pytest.approx (e.g. assert l.confidence ==
pytest.approx(<expected_value>, rel=1e-3)) for each lesson returned by
_patterns_to_graduated_lessons and keep the other checks (state ==
LessonState.RULE and category sorting) the same so regressions in the confidence
contract are caught.

- rule_pipeline.py: use Lesson import + list[Lesson] return type so pyright
  can narrow attribute access in _generate_skill_file and the pattern-lift
  caller loop. String-literal "object" forward-ref blocked all attr inference.
- _core.py: type: ignore[attr-defined] on Brain._causal_chain runtime-injection
  sites (pre-existing, surfaced by CI). Not adding the attr to Brain's class
  because it's optional + initialized lazily inside a try/except.

Unblocks CI on PR #97.

Co-Authored-By: Gradata <noreply@gradata.ai>
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

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

Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/gradata/_core.py (1)

990-993: ⚠️ Potential issue | 🟡 Minor

Fix import ordering to unblock CI (ruff I001).

The CI failure points to Line 990-993 for unsorted/unformatted imports. Please reorder/format that import block to satisfy ruff, otherwise the PR remains red.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/gradata/_core.py` around lines 990 - 993, Reorder the import block so
standard-library imports are sorted and grouped correctly to satisfy ruff I001:
replace the three-line block with the standard-library imports in alphabetical
order (import hashlib, import os) followed by the stdlib from-import (from
pathlib import Path) with no extra blank lines between them; then run ruff/isort
or the project's formatter to verify the CI warning is resolved.
src/gradata/enhancements/rule_pipeline.py (1)

411-414: ⚠️ Potential issue | 🟠 Major

Collapse the nested PATTERN → RULE guard to satisfy Ruff SIM102.

Lines 411–414 have a nested if statement where the inner condition can be combined with the elif. Apply the suggested fix:

Suggested fix
-        elif lesson.state.name == "PATTERN" and lesson.confidence >= RULE_THRESHOLD:
-            if lesson.fire_count >= MIN_APPLICATIONS_FOR_RULE:
-                lesson.state = LessonState.RULE
-                result.graduated.append(f"{lesson.category}:{lesson.description[:30]}")
+        elif (
+            lesson.state.name == "PATTERN"
+            and lesson.confidence >= RULE_THRESHOLD
+            and lesson.fire_count >= MIN_APPLICATIONS_FOR_RULE
+        ):
+            lesson.state = LessonState.RULE
+            result.graduated.append(f"{lesson.category}:{lesson.description[:30]}")

Note: The INSTINCT → PATTERN block above (lines 407–410) has the same nested structure and should also be refactored for consistency.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/gradata/enhancements/rule_pipeline.py` around lines 411 - 414, Combine
the nested guards into a single compound condition in the PATTERN->RULE
transition: replace the current "elif lesson.state.name == 'PATTERN' and
lesson.confidence >= RULE_THRESHOLD: if lesson.fire_count >=
MIN_APPLICATIONS_FOR_RULE:" pattern with a single elif that checks
lesson.state.name == "PATTERN" and lesson.confidence >= RULE_THRESHOLD and
lesson.fire_count >= MIN_APPLICATIONS_FOR_RULE, then set lesson.state =
LessonState.RULE and append to result.graduated as before; apply the same
refactor to the INSTINCT->PATTERN block that uses lesson.state.name, confidence
threshold, and lesson.fire_count to keep consistency.
♻️ Duplicate comments (3)
src/gradata/enhancements/rule_pipeline.py (3)

190-190: ⚠️ Potential issue | 🟠 Major

Remove the quoted Path annotation here too.

This still trips Ruff UP037, so SDK CI will remain red until it is unquoted.

Suggested fix
-def review_generated_skill(skill_path: "Path") -> dict:
+def review_generated_skill(skill_path: Path) -> dict:
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/gradata/enhancements/rule_pipeline.py` at line 190, The function
signature review_generated_skill currently uses a quoted type annotation "Path"
which triggers Ruff UP037; update the annotation to use the actual Path type
(change "Path" to Path) and ensure Path is imported (from pathlib import Path)
at the top of the module if it isn't already so the annotation is valid and
unquoted.

94-101: ⚠️ Potential issue | 🟡 Minor

Validate first_seen instead of writing a fixed date.

str(... )[:10] or "2026-01-01" can persist invalid or misleading metadata when the source value is missing or malformed. Please parse first_seen and use a safe dynamic fallback instead.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/gradata/enhancements/rule_pipeline.py` around lines 94 - 101, Replace the
brittle slice-based assignment for first_seen with proper parsing and
validation: in the code that builds the Lesson (the first_seen variable before
lessons.append(Lesson(...)) in rule_pipeline.py), try to parse
row.get("first_seen") as a date/time (e.g., using datetime.fromisoformat or a
parsing utility), on success set first_seen to the parsed date's ISO 10-char
(YYYY-MM-DD) string, and on failure (None/malformed) set first_seen to a safe
dynamic fallback such as datetime.date.today().isoformat(); ensure you catch
parsing exceptions and avoid hard-coded "2026-01-01".

373-375: ⚠️ Potential issue | 🟠 Major

Normalize these local import blocks to clear I001.

Ruff is still flagging these import groups, so the file will keep failing CI until they are regrouped/reordered consistently.

Also applies to: 418-422, 473-475, 616-617

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/gradata/enhancements/rule_pipeline.py` around lines 373 - 375, Several
small local import blocks (e.g., "from gradata._types import Lesson as _Lesson"
and "from datetime import date as _date" near candidate = _Lesson, and the
similar groups at the reported locations) are breaking Ruff I001; consolidate
and reorder them into a single normalized import group (standard library,
third-party, first-party) instead of scattered local imports. Locate the import
occurrences referenced around candidate/_Lesson (and the blocks at the other
reported locations) and merge them into the file's top-level import section or a
single, consistently ordered local-import block, keeping the same aliases
(_Lesson, _date) so the rest of the code continues to work.
🤖 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/rule_pipeline.py`:
- Around line 48-74: Modify _patterns_to_graduated_lessons to accept a
BrainContext parameter and use it for DB access instead of taking/using a raw
Path; specifically change the signature to include ctx: BrainContext (or replace
db_path with ctx), derive the Path from ctx (or pass ctx directly) and call
query_graduation_candidates through the SDK/ctx (i.e., pass ctx or ctx.db rather
than db_path) so the DB read goes through the BrainContext boundary; update any
imports/typing (BrainContext) and adjust callers to pass the context.
- Around line 82-86: The code currently checks raw.startswith("[AUTO]") before
trimming, so strings with leading whitespace (e.g., "  [AUTO]...") bypass the
filter; update the logic in the rule pipeline so you strip/trim the raw string
first (use the existing variable raw) and then perform the startswith("[AUTO]")
check, and then pass the trimmed value into _normalize_pattern_description
instead of the untrimmed one to ensure evaluator-generated noise is properly
dropped.

---

Outside diff comments:
In `@src/gradata/_core.py`:
- Around line 990-993: Reorder the import block so standard-library imports are
sorted and grouped correctly to satisfy ruff I001: replace the three-line block
with the standard-library imports in alphabetical order (import hashlib, import
os) followed by the stdlib from-import (from pathlib import Path) with no extra
blank lines between them; then run ruff/isort or the project's formatter to
verify the CI warning is resolved.

In `@src/gradata/enhancements/rule_pipeline.py`:
- Around line 411-414: Combine the nested guards into a single compound
condition in the PATTERN->RULE transition: replace the current "elif
lesson.state.name == 'PATTERN' and lesson.confidence >= RULE_THRESHOLD: if
lesson.fire_count >= MIN_APPLICATIONS_FOR_RULE:" pattern with a single elif that
checks lesson.state.name == "PATTERN" and lesson.confidence >= RULE_THRESHOLD
and lesson.fire_count >= MIN_APPLICATIONS_FOR_RULE, then set lesson.state =
LessonState.RULE and append to result.graduated as before; apply the same
refactor to the INSTINCT->PATTERN block that uses lesson.state.name, confidence
threshold, and lesson.fire_count to keep consistency.

---

Duplicate comments:
In `@src/gradata/enhancements/rule_pipeline.py`:
- Line 190: The function signature review_generated_skill currently uses a
quoted type annotation "Path" which triggers Ruff UP037; update the annotation
to use the actual Path type (change "Path" to Path) and ensure Path is imported
(from pathlib import Path) at the top of the module if it isn't already so the
annotation is valid and unquoted.
- Around line 94-101: Replace the brittle slice-based assignment for first_seen
with proper parsing and validation: in the code that builds the Lesson (the
first_seen variable before lessons.append(Lesson(...)) in rule_pipeline.py), try
to parse row.get("first_seen") as a date/time (e.g., using
datetime.fromisoformat or a parsing utility), on success set first_seen to the
parsed date's ISO 10-char (YYYY-MM-DD) string, and on failure (None/malformed)
set first_seen to a safe dynamic fallback such as
datetime.date.today().isoformat(); ensure you catch parsing exceptions and avoid
hard-coded "2026-01-01".
- Around line 373-375: Several small local import blocks (e.g., "from
gradata._types import Lesson as _Lesson" and "from datetime import date as
_date" near candidate = _Lesson, and the similar groups at the reported
locations) are breaking Ruff I001; consolidate and reorder them into a single
normalized import group (standard library, third-party, first-party) instead of
scattered local imports. Locate the import occurrences referenced around
candidate/_Lesson (and the blocks at the other reported locations) and merge
them into the file's top-level import section or a single, consistently ordered
local-import block, keeping the same aliases (_Lesson, _date) so the rest of the
code continues to work.
🪄 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: ea2807ed-62f4-4653-a911-d05f821bef25

📥 Commits

Reviewing files that changed from the base of the PR and between 9604d26 and 762682d.

📒 Files selected for processing (2)
  • src/gradata/_core.py
  • src/gradata/enhancements/rule_pipeline.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: test (3.11)
  • GitHub Check: test (3.12)
🧰 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/_core.py
  • src/gradata/enhancements/rule_pipeline.py
🪛 GitHub Actions: SDK CI
src/gradata/_core.py

[error] 990-993: ruff I001: Import block is un-sorted or un-formatted. Organize imports.

src/gradata/enhancements/rule_pipeline.py

[error] 190-190: ruff UP037: Remove quotes from type annotation ("Path").


[error] 373-374: ruff I001: Import block is un-sorted or un-formatted. Organize imports.


[error] 411-412: ruff SIM102: Use a single if statement instead of nested if statements.


[error] 418-422: ruff I001: Import block is un-sorted or un-formatted. Organize imports.


[error] 473-475: ruff I001: Import block is un-sorted or un-formatted. Organize imports.


[error] 616-617: ruff I001: Import block is un-sorted or un-formatted. Organize imports.

🔇 Additional comments (2)
src/gradata/_core.py (1)

397-400: _causal_chain typing suppression is acceptable here.

Line 397/400 and Line 449/452 use hasattr(...) guards before dynamic assignment/calls, so this attr-defined suppression is a reasonable, targeted fix for static typing noise without behavior changes.

Also applies to: 449-452

src/gradata/enhancements/rule_pipeline.py (1)

106-109: Good type narrowing here.

Switching _generate_skill_file() from object to Lesson gives downstream attribute access a real type to narrow against.

Comment on lines +48 to +74
def _patterns_to_graduated_lessons(
db_path: Path,
current_session: int,
min_sessions: int = 2,
min_score: float = 3.0,
) -> list[Lesson]:
"""Lift graduated correction_patterns into synthetic RULE-state lessons.

Before this wiring the 437-row correction_patterns table was orphaned --
query_graduation_candidates had no production caller, so meta-rule
synthesis never saw the real user corrections. This bridges the gap:
clusters that already hit (sessions >= min_sessions, weight >= min_score)
are lifted directly to RULE state for synthesis.
"""
try:
from gradata.enhancements.meta_rules_storage import ( # type: ignore[import]
query_graduation_candidates,
)
except ImportError:
return []
if not db_path.is_file():
return []

try:
candidates = query_graduation_candidates(
db_path, min_sessions=min_sessions, min_score=min_score,
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Pass BrainContext through this new DB read path.

_patterns_to_graduated_lessons() adds another direct database entry point by calling query_graduation_candidates() from a raw Path. Please thread BrainContext through here instead of bypassing the SDK DB boundary.

As per coding guidelines, src/gradata/**/*.py requires "all functions accepting BrainContext where DB access occurs".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/gradata/enhancements/rule_pipeline.py` around lines 48 - 74, Modify
_patterns_to_graduated_lessons to accept a BrainContext parameter and use it for
DB access instead of taking/using a raw Path; specifically change the signature
to include ctx: BrainContext (or replace db_path with ctx), derive the Path from
ctx (or pass ctx directly) and call query_graduation_candidates through the
SDK/ctx (i.e., pass ctx or ctx.db rather than db_path) so the DB read goes
through the BrainContext boundary; update any imports/typing (BrainContext) and
adjust callers to pass the context.

Comment on lines +82 to +86
raw = row.get("representative_text") or ""
# Drop evaluator-generated noise -- not real user corrections
if raw.startswith("[AUTO]"):
continue
desc = _normalize_pattern_description(raw)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Trim before filtering [AUTO] rows.

raw.startswith("[AUTO]") runs before whitespace is stripped, so values like " [AUTO] ..." will currently be lifted into synthesis as real lessons.

Suggested fix
-        raw = row.get("representative_text") or ""
+        raw = str(row.get("representative_text") or "").strip()
         # Drop evaluator-generated noise -- not real user corrections
         if raw.startswith("[AUTO]"):
             continue
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/gradata/enhancements/rule_pipeline.py` around lines 82 - 86, The code
currently checks raw.startswith("[AUTO]") before trimming, so strings with
leading whitespace (e.g., "  [AUTO]...") bypass the filter; update the logic in
the rule pipeline so you strip/trim the raw string first (use the existing
variable raw) and then perform the startswith("[AUTO]") check, and then pass the
trimmed value into _normalize_pattern_description instead of the untrimmed one
to ensure evaluator-generated noise is properly dropped.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant