feat: M1 Skills export + cloud sync fixes (batch of 41 commits)#143
feat: M1 Skills export + cloud sync fixes (batch of 41 commits)#143
Conversation
Local SQLite and cloud Supabase schemas diverged (wide `tenant_id` + `data_json` vs narrow `brain_id` + `data` jsonb, plus table rename `correction_patterns` -> `corrections`). Added `_transform_row` per-table mapper with deterministic uuid5 ids so repeat pushes upsert cleanly. `_scrub` strips NUL bytes and lone UTF-16 surrogates that Postgres JSONB rejects. `_post` dedupes within each batch, honors `_TABLE_REMAP`, and chunks large pushes to avoid PostgREST's opaque "Empty or invalid json" body-limit errors. `GRADATA_SUPABASE_URL` / `GRADATA_SUPABASE_SERVICE_KEY` now work as aliases so one .env serves both backend and SDK. Co-Authored-By: Gradata <noreply@gradata.ai>
…provider synth Phase 1 of the learning-pipeline revamp. Rule graduation now flows through the canonical _graduation.graduate() path (strict > for INSTINCT->PATTERN, >= for PATTERN->RULE) instead of the inline duplicate in rule_pipeline. Injection hook reads a persistent brain_prompt.md gated by an AUTO-GENERATED header, regenerated only at session_close after the pipeline fires. LLM synthesis gets a two-provider path: anthropic SDK (ANTHROPIC_API_KEY) with claude CLI fallback (Max-plan OAuth) so users without an exportable key still get synthesis. Meta-rule deterministic fallback now warns loudly instead of silently discarding. Drops five env-flag gates in favour of file-based signals. Co-Authored-By: Gradata <noreply@gradata.ai>
Adds --cloud / --no-cloud flags to the doctor CLI command and the underlying diagnose() function. Flips the default cloud endpoint to api.gradata.ai/api/v1. Covers new behaviour with test_doctor_cloud.py (all passing). Co-Authored-By: Gradata <noreply@gradata.ai>
Regex coverage was brittle to shorthand: real corrections like
"Why r you not asking" and "Why flag.. we dont skip" slipped the
\bwhy (did|would|are) you\b pattern and never became IMPLICIT_FEEDBACK
events. That silently breaks Gradata's core promise ("learn from any
correction").
Adds:
- negation: dont/cant/shouldnt (no-apostrophe variants), never
- reminder: "again" marker, "dont forget"
- challenge: "why r u", "why not/r/are/is/does", "why word..",
"how come", "you missed/forgot/failed/didnt"
All 8 target phrases now detect. 25 existing implicit-feedback tests
remain green.
Co-Authored-By: Gradata <noreply@gradata.ai>
14 new tests pinning the regex expansion from 5a6da45. Covers real corrections observed this session ("Why r you not asking council", "Why flag.. we don't skip we do work") plus shorthand cases (dont / cant / again / you missed / how come). Dual-signal cases assert both types detect. Full suite: 37 passed, 1 pre-existing skip. Co-Authored-By: Gradata <noreply@gradata.ai>
Five post-launch metrics with precise definitions (activation, D7 retention, time-to-first-graduation, free->Pro conversion, correction-rate decay). Numeric triggers: pivot <20% activation + flat decay at D30; kill <100 installs at D60; scale >1K installs + >=5% conversion at D90. Monday 30-min retro agenda. Source: Card 8 of the pre-launch gap analysis. Co-Authored-By: Gradata <noreply@gradata.ai>
The source-provenance docstring referenced "cloud-side LLM synthesis" which is stale since the graduation-cloud-gate was removed. Synthesis runs on the user's machine via rule_synthesizer.py's two-provider path (Anthropic SDK with user's key, or Claude Code Max CLI OAuth). Co-Authored-By: Gradata <noreply@gradata.ai>
Graduation and meta-rule LLM synthesis run entirely locally as of a few sessions ago (rule_synthesizer.py uses user's own Anthropic key or Claude Code Max CLI OAuth). The Pro-tier inclusion list incorrectly still claimed "cloud runs better graduation engine" and implied a cloud-enhanced sqlite-vec path. Rewrite the inclusion list + philosophy paragraph to match reality: free is functionally complete; Pro is visualization, history, export, and the future community corpus. NOTE: this file is listed in .gitignore per the earlier "untrack private files" cleanup. Force-added at request. Co-Authored-By: Gradata <noreply@gradata.ai>
Test was checking the pre-transform local key name. _cloud_sync._transform_row correctly emits brain_id (cloud schema) from tenant_id (local schema); the assertion was stale. Co-Authored-By: Gradata <noreply@gradata.ai>
Previously nothing wrote to lesson_applications — the table existed
(onboard.py), was size-checked (_validator.py), and synced to cloud
(_cloud_sync.py), but no code ever inserted a row. The compound-quality
story had no evidence: rules claimed to fire with no receipt.
Now:
- inject_brain_rules writes one PENDING row per injected rule (cluster
members included), storing {category, description, task} in context so
session_close can attribute outcomes back to specific rules.
- session_close resolves PENDING rows at end-of-waterfall:
REJECTED if any CORRECTION/IMPLICIT_FEEDBACK/RULE_FAILURE in the
session shares the lesson's category (or description substring).
CONFIRMED otherwise (rule survived the session).
Both paths are best-effort — DB missing, schema drift, or IO errors
degrade silently rather than blocking injection or session close.
Unblocks the Card 6 MVP day-14 metric: "did a graduated rule actually
fire and survive?" — the answer now has a row-level audit trail.
Co-Authored-By: Gradata <noreply@gradata.ai>
Sweeps the remaining docs that still claimed cloud gated any part of the learning loop. Actual architecture (as of the graduation-local pivot): Local SDK owns: correction capture, graduation, meta-rule clustering AND LLM-synthesis (via user's Anthropic key or Claude Code Max OAuth), rule-to-hook promotion, manifest computation. Cloud owns: dashboard/visualization, cross-device sync, team brains, managed backups, future opt-in corpus donation. Files touched: - docs/cloud/overview.md — capability matrix, architecture diagram, use-when guidance. - docs/architecture/cloud-monolith-v2.md — cloud-side workload framing. - docs/architecture/multi-tenant-future-proofing.md — proprietary boundary, verification flow. - docs/concepts/meta-rules.md — synthesis is local, not cloud-gated. - docs/cloud/dashboard.md — dashboard visualizes local output, does not re-synthesize. README.md was already accurate; no changes there. Co-Authored-By: Gradata <noreply@gradata.ai>
Silent-failure-hunter CRITICAL-1:
- inject_brain_rules: wrap lesson_applications connection in try/finally
and escalate OperationalError to warning (missing-table surfaces).
Silent-failure-hunter CRITICAL-2:
- _cloud_sync.push: per-row try/except on _transform_row so one bad row
no longer propagates and kills the whole push batch.
Leak scan blockers:
- Delete docs/pre-launch-plan.md and docs/gradata-marketing-strategy.md
from the public repo; add both to .gitignore. These contain kill
triggers, pricing, and PII that belong in the private brain vault only.
Code-reviewer BLOCKER-3:
- _doctor._check_vector_store returns status="ok" with FTS5 detail in
the detail field, restoring the documented status vocabulary
({ok, warn, fail, skip, missing, error}).
Test-coverage gaps:
- Add tests/test_rule_synthesizer.py — both providers absent, empty
input, cache hit, CLI fallback on SDK raise, malformed output.
- Add IMPLICIT_FEEDBACK → REJECTED integration test to
test_lesson_applications.py.
Verification: full suite 3802 pass, 22 skip, 2 xfailed.
Gradata is fully local-first now. Cloud-gate stubs and "requires cloud" skip markers were legacy artifacts from an earlier architecture where discovery/synthesis lived server-side. This commit finishes the port: - meta_rules.discover_meta_rules + merge_into_meta run locally: category grouping + greedy semantic-similarity clustering, zombie filter on RULE-state lessons below 0.90, decay after 20 sessions, count/(count+3) confidence smoothing. - Drop @_requires_cloud markers from test_bug_fixes, test_llm_synthesizer, test_meta_rule_generalization, test_multi_brain_simulation, test_pipeline_e2e. These tests now exercise the local impl directly. - Retire the api_key-kwarg-on-merge_into_meta path (session-close rule_synthesizer drives LLM distillation now). - Update fixtures to realistic prose so they survive the noise filter that rejects "cut:/added:" edit-distance summaries. - Bump test_meta_rules confidence assertion to the smoothed formula. - Add docs/LEGACY_CLEANUP.md tracking the remaining cloud-gate vestiges (deprecated adapter shims, cloud docs, stale module docstrings). Suite: 3809 passed, 14 skipped, 2 xfailed. Co-Authored-By: Gradata <noreply@gradata.ai>
…xtures
discover_meta_rules is implemented now (local-first). The
if not metas: pytest.skip('discover_meta_rules not yet implemented')
guards were vestiges from the cloud-only era — convert to real asserts.
Also bump 0.88-confidence RULE-state fixtures to 0.90 so they survive
the zombie filter (RULE at <0.90 is treated as a decayed rule).
Suite: 3813 passed, 10 skipped, 2 xfailed.
Remaining skips are all legit:
- test_file_lock.py (2): Windows vs POSIX platform gates
- test_integration_workflow.py (5): require ANTHROPIC/OPENAI keys, cost money
- test_mem0_adapter.py::test_real_mem0_roundtrip: requires MEM0_API_KEY
- test_meta_rules.py::test_with_real_data: requires GRADATA_LESSONS_PATH env
xfails (2) are tracked for v0.7 reconciliation in test docstring.
Co-Authored-By: Gradata <noreply@gradata.ai>
Found while clearing remaining skipped/xfailed tests: Bug: agent_graduation._update_lesson_confidence had confidence = max(0.0, confidence - MISFIRE_PENALTY) but MISFIRE_PENALTY = -0.15 (negative). Subtracting a negative added confidence on rejection. Test test_rejection_decreases_confidence was xfail'd with 'API drift, reconcile in v0.7' — it was a real bug. Fix: align with canonical _confidence.py usage (confidence + MISFIRE_PENALTY). Other cleanups in the same pass: - test_agent_graduation: drop both xfail markers. test_lesson_graduates_to_pattern was also wrong on its own terms — with ACCEPTANCE_BONUS=0.20 the lesson graduates straight to RULE (stronger than PATTERN). Accept either state. - test_integration_workflow: delete stale module-level skipif guarding 5 tests behind ANTHROPIC/OPENAI keys they never actually use. They only exercise local brain.correct/convergence/efficiency — no network. - test_mem0_adapter: delete test_real_mem0_roundtrip (live-API smoke test already covered by the 20+ fake-client tests in the same file). - test_meta_rules: delete test_with_real_data — dev-time exploration script with zero asserts, requiring GRADATA_LESSONS_PATH env var. Suite: 3820 passed, 3 skipped, 0 xfailed, 0 failed. Remaining 3 skips are test_file_lock.py POSIX paths that require fcntl, which does not exist on Windows. Complementary Windows paths skip on Linux — running on each platform covers all 4. Cannot be eliminated. From 22 skipped + 2 xfailed to 3 skipped + 0 xfailed. Co-Authored-By: Gradata <noreply@gradata.ai>
…ten stale notes Co-Authored-By: Gradata <noreply@gradata.ai>
…ate refresh - agent_graduation: add _extract_output() to handle all Claude Code PostToolUse payload key variants (tool_response/tool_output/tool_result/output/response) so plan-mode agents no longer silently drop output - session_close: add _load_soul_mandatories() (VOICE rules from soul.md injected into brain_prompt.md) and _refresh_loop_state() (regenerates loop-state.md on session close with live DB + lesson counts); raise Stop hook timeout to 90 s - _events: add _redact_payload() (recursive email PII redaction) wired into emit() before any write; raw side-log to events.raw.jsonl (best-effort); redactor failure aborts write (fail closed) Co-Authored-By: Gradata <noreply@gradata.ai>
…e watermarks - _ulid.py: minimal stdlib ULID generator (no external dep); ulid_from_iso() preserves timestamp sort order during historical backfill - device_uuid.py: atomic read-or-create of per-brain dev_<hex> device id; race-safe via O_EXCL temp file + os.replace - 002_add_event_identity: adds event_id/device_id/content_hash/correction_chain_id/ origin_agent columns + indexes to events table; chunked 10k-row backfill that is idempotent and resumes on restart - 003_add_sync_state: creates sync_state table if missing and adds device_id/ last_push_event_id/last_pull_cursor/tenant_id watermark columns + composite indexes - tests: 44 tests covering all migration paths, chunked backfill, idempotency, PII redaction (email), loop-state generation, and session_close functions Co-Authored-By: Gradata <noreply@gradata.ai>
…ts DB Reads ~/.claude/projects/<project-hash>/*.jsonl count as the session number — the actual Anthropic session log — rather than MAX(session) from the Gradata events table. The two diverged (314 vs 367). Falls back to the events DB if the project dir can't be located. Co-Authored-By: Gradata <noreply@gradata.ai>
Previous fix only counted the active project dir (314). Global sum across all project dirs gives 659, matching the actual Anthropic session log total. Falls back to events DB if projects dir missing. Co-Authored-By: Gradata <noreply@gradata.ai>
…oop-state.md (367) Session number was read from loop-state.md (Gradata events DB counter). Now counts .jsonl files across all ~/.claude/projects/ dirs — the real Claude Code session total, same logic as status_line.py. Co-Authored-By: Gradata <noreply@gradata.ai>
Every silent except Exception: pass in the core library layers now emits a _log.debug() so failures surface under GRADATA_LOG=debug without breaking the best-effort semantics. Files touched: brain.py (telemetry guard), context_wrapper.py (apply_brain_rules / context_for fallbacks), _brain_manifest.py + _context_compile.py (added module loggers), _context_packet.py (12 data-loading guards), _manifest_metrics.py (7 DB query guards), _doctor.py (HTTP body read guard + contextlib import), _mine_transcripts.py (SIM108 ternary), hooks/session_close.py (4 x SIM105 OSError guards converted to contextlib.suppress). Co-Authored-By: Gradata <noreply@gradata.ai>
ruff check src/ --fix resolved 8 auto-fixable violations (E, F, I rules). ruff format src/ reformatted 163 files to enforce consistent style. Zero errors remain; 13 pre-existing warnings (optional cloud/framework imports, lazy __all__ patterns) are unchanged. Co-Authored-By: Gradata <noreply@gradata.ai>
Two tests expected s0/s42 but got s659 because _claude_session_count() was walking the real ~/.claude/projects/. Add fake_home fixture so the function returns None and falls back to the events DB as intended. Co-Authored-By: Gradata <noreply@gradata.ai>
…eshold
New Stop hook writes a structured handoff to brain/sessions/handoff-{ts}.md
when context usage exceeds GRADATA_CTX_THRESHOLD (default 65%). inject_brain_rules
surfaces a <watchdog-alert> block at next session start so the LLM knows to
review the handoff and run /compact or /clear.
Also: bracket_confidence() in session_close for cache-key stability; remove
MAX_RULES render cap from inject_brain_rules (overshoot logic was masking gaps);
13 new tests in test_ctx_watchdog, tests in test_rule_synthesizer updated.
Co-Authored-By: Gradata <noreply@gradata.ai>
…ript store + retroactive sweep P1: call_provider() dispatch in rule_synthesizer.py routes by model prefix (claude-* → Anthropic, gpt-*/o1/o3 → OpenAI, gemini-* → Google, http → generic). session_close._refresh_brain_prompt now uses call_provider instead of inline SDK. P2: _bracket_confidence() buckets FSRS floats into 3 stable bands (low/mid/high) so per-tick confidence changes no longer bust the synthesis cache. P3: New _transcript.py (log_turn, load_turns, cleanup_ttl) and _transcript_providers.py (ProviderTranscriptSource + GradataTranscriptSource) form the transcript store layer. _retroactive_sweep() in the waterfall runs implicit_feedback patterns across all session turns (gated on GRADATA_TRANSCRIPT=1). OpenAI, LangChain, CrewAI middleware adapters gain session_id + log_turn() calls. 21 new tests in test_transcript.py. Co-Authored-By: Gradata <noreply@gradata.ai>
…only The global Path.is_file patch in _run_main() caused inject_brain_rules to also read a fake pending_handoff.txt and append a <watchdog-alert> block. Test now extracts content between <brain-rules>...</brain-rules> before counting lines, making it immune to any outer blocks appended to the result. Co-Authored-By: Gradata <noreply@gradata.ai>
- pre_compact.py rewritten: when auto-compact fires with a pending handoff, replaces the compact summary verbatim with handoff content so no lossy LLM summarization occurs. Manual compact falls back to snapshot. Corrects field name from "type" → "trigger" (keeps legacy fallback). - inject_brain_rules._build_watchdog_block() extracted from inline main(): Phase 1 (pre-/clear): consumes pending_handoff.txt, stages content to post_clear_handoff.txt, injects <watchdog-alert> with run-/clear prompt. Phase 2 (post-/clear): consumes post_clear_handoff.txt, injects <session-handoff> into fresh session. Phase 2 takes priority if both exist. - implicit_feedback: return None instead of signal name string to reduce UserPromptSubmit noise. - tests/test_pre_compact.py: 9 tests covering both trigger paths. - tests/test_inject_watchdog_phases.py: 8 tests covering both phases. Co-Authored-By: Gradata <noreply@gradata.ai>
graph_first_check.py (PreToolUse, Glob|Grep): blocks exploratory code searches until the session flag is set. Returns a block decision with the exact ToolSearch call needed to unblock. graph_session_track.py (PostToolUse, ToolSearch): writes a per-session flag file when a ToolSearch query contains "code-review-graph", clearing the block for the rest of the session. inject_brain_rules.py: appends <code-graph-tools> directive to every SessionStart injection, with the mandatory ToolSearch query string. Both hooks registered in ~/.claude/settings.json. Bypass via GRADATA_GRAPH_CHECK=0. 18 tests, smoke-tested end-to-end. Co-Authored-By: Gradata <noreply@gradata.ai>
…tignore cleanup - test_hooks_intelligence.py: implicit_feedback tests now assert result is None and verify IMPLICIT_FEEDBACK event via mock_emit (hook emits, doesn't return) - session_close.py: reorder imports alphabetically (isort) - .gitignore: add graphify temp files, run.log patterns, and /.archive/ personal Claude Code config backups so they never accidentally land in commits Co-Authored-By: Gradata <noreply@gradata.ai>
… migration reference - Gradata/.archive/dashboard_streamlit_deprecated_2026-04-23.py: move legacy Streamlit dashboard per Phase 4 deprecation plan (gradata.ai web dashboard now covers all panels — /rules, /corrections, /self-healing, /observability) - Gradata/migrations/supabase/: reference copies of cloud migrations 014-016 applied to prod 2026-04-24 (corrections unique, events unique, brains.last_used_at) - Gradata/docs/specs/cloud-sync-and-pricing.md: DRAFT v1 sync architecture + pricing tier spec Co-Authored-By: Gradata <noreply@gradata.ai>
Stale file created by a subagent Bash redirect. Grouped with the existing Windows cmd.exe stdout misparse artifact entries. Co-Authored-By: Gradata <noreply@gradata.ai>
Co-Authored-By: Gradata <noreply@gradata.ai>
- CHANGELOG.md: add [Unreleased] section covering 18 commits since 2026-04-23 (cloud sync, hooks hardening, Supabase migrations, Streamlit archival, statusline session-count source, implicit_feedback emit-only contract) - migrations/supabase/014,015: wrap constraint adds in DO blocks that check pg_constraint first, making re-runs safe on any DB (prod already had inline UNIQUE _key variants from CREATE TABLE; these migrations added redundant _unique variants, now documented as no-op on existing systems) - migrations/supabase/README.md: document prod constraint state (both _key and _unique present on corrections + events) and drift-cleanup deferred Co-Authored-By: Gradata <noreply@gradata.ai>
Critic audit flagged a silent-drop path: when resolve_brain_dir() returns None (fresh install, CI env, unconfigured brain) the hook detected signals but skipped emit() with no log — every correction became invisible. - hooks/implicit_feedback.py: add debug log in the else branch recording how many signals were detected and of which types, so operators running `GRADATA_LOG_LEVEL=DEBUG` see the breadcrumb. - tests/test_implicit_feedback.py: add TestMainNoBrainDir covering the main() path (previously only _detect_signals was tested) — verifies the debug log fires on detected signals, stays quiet on no-signal input, and short messages don't crash. Co-Authored-By: Gradata <noreply@gradata.ai>
Watermark stalls from 23505 unique-violations were invisible unless a
caller grepped logs: _post() logged everything at WARNING. Now HTTP 409
and any "23505" body are logged at ERROR with a body snippet, and the
last error is persisted to brain_dir/cloud_push_error.json so
'gradata doctor' can surface it ('fail' for constraint violations,
'warn' for other non-2xx). Successful pushes clear the file.
_post() signature is now (accepted, error_info|None); call sites and
the three existing tests patching _post are updated. A _coerce_post_result
shim tolerates legacy int returns from any external patches.
Closes T17 from the overnight backlog (critic finding cycle-2 #1).
Addresses three cycle-3 council findings on commit 492c3dd: 1. Non-atomic write (critic #1, high-severity race). `_record_push_error` now writes to `<name>.tmp` then `os.replace`s into the target. Concurrent readers (doctor + daemon + MCP server) can no longer observe a truncated file that would mask a constraint violation as "error file unreadable". 2. PII leak in persisted error (critic #2). PostgREST 23505 bodies echo conflicting row values in `details`/`hint` fields, and `gradata doctor` prints the file verbatim. New `_scrub_error_body` parses the body as JSON and keeps only `code` + the first 120 chars of `message` (enough for the constraint name). Non-JSON bodies reduce to a length marker. Log messages use the scrubbed form too. 3. Removed the `_coerce_post_result` shim (verifier + critic). Zero tests exercised the bare-int branch it guarded; callers now destructure `_post` returns directly. Tests: +2 (`test_post_error_body_scrubs_row_values`, `test_scrub_error_body_handles_non_json`), 28/28 in the cloud test files pass, 3944 passed / 3 skipped full suite. Ruff + pyright clean. Co-Authored-By: Gradata <noreply@gradata.ai>
When doctor reports on cloud_push_error.json, the detail string now names the brain directory it checked. In multi-brain deployments, push() and doctor() can resolve different brain_dirs silently — surfacing the path lets users spot the divergence instead of chasing phantom "ok" reports. Cycle-3 critic finding #3. Co-Authored-By: Gradata <noreply@gradata.ai>
Co-Authored-By: Gradata <noreply@gradata.ai>
…metry Three bugs kept last_sync_at frozen: - cloud/client.py POSTed /brains/sync (path doesn't exist) -> /sync - cloud/sync.py POSTed /v1/telemetry/metrics -> /api/v1/telemetry/metrics - Stop hook never fired cloud sync because Claude Code doesn't call brain.end_session(). Added cloud_sync_tick() helper in _core.py and new _run_cloud_sync step in session_close.py waterfall. Also elevated silent DEBUG failures to WARNING with HTTP status + exc_info so the next failure mode surfaces in run.log. 3945 tests pass. Co-Authored-By: Gradata <noreply@gradata.ai>
New CLI: gradata skill export <name> [--output-dir DIR] [--description STR]
[--category CAT] [--no-meta]
The bet: Claude Skills' "gotchas" section is exactly what graduated
RULE-tier lessons are -- but generated from real corrections instead of
hand-written. This turns a brain into a portable, shippable Skill folder
with valid YAML frontmatter, category-grouped gotchas, and (when
available) injectable meta-principles.
- new module enhancements/skill_export.py reuses _parse_rules from
rule_export so the RULE-only filter and [hooked] marker stripping
stay consistent across exporters
- auto-generated frontmatter description lists rule categories with
defensive 900-char clip (Anthropic 1024 ceiling)
- name slugified for safe folder name + frontmatter alignment
- description quote-escapes preserve YAML validity
- meta-rule loader degrades gracefully on missing system.db / table
24 new tests; full suite 3969 pass (+24, 0 regressions).
Unblocks M4 items 7 and 9 (self-dev Skill, composition Skill) per
plans/swift-toasting-origami.md.
Co-Authored-By: Gradata <noreply@gradata.ai>
|
Too many files changed for review. ( |
📝 Walkthrough
WalkthroughMoves meta-rule synthesis and graduation to a local-first flow, implements robust Supabase-aware cloud sync and migrations (ULID/device id/sync_state), adds transcript persistence and device utilities, hardens event emit redaction, introduces rule synthesis and skill-export features, updates hooks/CLI, and applies widespread formatting and debug-logging improvements. Changes
Sequence Diagram(s)sequenceDiagram
participant SDK as SDK (Local)
participant DB as SQLite
participant Cloud as Gradata Cloud
participant Supabase as Supabase
Note over SDK,DB: Local-first meta-rule synthesis
SDK->>DB: Load lessons + events
SDK->>SDK: Filter eligible lessons, cluster by category
SDK->>SDK: Build deterministic principles
alt LLM creds available
SDK->>SDK: Call rule_synthesizer (LLM provider)
SDK->>SDK: Cache synthesis result
else Fallback
SDK->>SDK: Emit deterministic principles (no LLM)
end
SDK->>DB: Store meta-rules locally
Note over SDK,Cloud: Read-only outbound sync
SDK->>Cloud: Push transformed rows (deterministic UUIDs, remapped tables)
Cloud->>Supabase: Persist to Supabase (may return 409 on constraint violation)
Cloud-->>SDK: ACK or partial error
alt Partial/failed push
Cloud->>SDK: Write `cloud_push_error.json`
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
|
There was a problem hiding this comment.
Actionable comments posted: 40
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
Gradata/src/gradata/contrib/patterns/human_loop.py (1)
494-503:⚠️ Potential issue | 🟠 MajorHonor
contextinHumanLoopGate.check()risk evaluation.
check()acceptscontextbut callsgate(action)without using it, sorisk_override(and context-driven classification) can be ignored and approvals may be wrong.🔧 Proposed fix
def check( self, action: str, context: dict | None = None, approver: Callable | None = None, ) -> ApprovalResult: """Full gate check: assess risk, request approval if needed.""" - request = gate(action) + risk = assess_risk(action, context) + request = gate(action, risk=risk) if request is None: return ApprovalResult(approved=True, feedback="auto_approved_low_risk") if approver is not None: return approver(request) return ApprovalResult(approved=False, feedback="requires_human_review")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Gradata/src/gradata/contrib/patterns/human_loop.py` around lines 494 - 503, The check method in HumanLoopGate ignores the provided context by calling gate(action) without passing context, which prevents context-driven risk overrides from being considered; update HumanLoopGate.check to pass the context through to the gate invocation (e.g., call gate with the context argument) so risk_override and any context-based classification are honored when building the ApprovalResult.Gradata/src/gradata/_context_packet.py (1)
231-240:⚠️ Potential issue | 🟠 MajorClose SQLite connections on failure paths in
_load_audit_context.
conn.close()is only reached on the success path. Ifsqlite3.connect,execute, orfetchoneraises, the connection may remain open.♻️ Proposed fix
def _load_audit_context(session: int, ctx: "BrainContext | None" = None) -> dict: result = {"metrics": {}, "outputs": [], "gates": [], "correction_rate": {}} try: db = ctx.db_path if ctx else _p.DB_PATH - conn = sqlite3.connect(str(db)) - conn.row_factory = sqlite3.Row - row = conn.execute("SELECT * FROM session_metrics WHERE session = ?", (session,)).fetchone() - if row: - result["metrics"] = dict(row) - conn.close() + with sqlite3.connect(str(db)) as conn: + conn.row_factory = sqlite3.Row + row = conn.execute( + "SELECT * FROM session_metrics WHERE session = ?", + (session,), + ).fetchone() + if row: + result["metrics"] = dict(row) except Exception as e: _log.debug("audit metrics query failed (non-fatal): %s", e)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Gradata/src/gradata/_context_packet.py` around lines 231 - 240, The SQLite connection opened in _load_audit_context (via sqlite3.connect) is only closed on the success path; change the logic to always close the connection on error by using a context manager or a try/finally: either wrap sqlite3.connect(...) in a with sqlite3.connect(...) as conn block (ensuring conn.row_factory and the execute/fetchone run inside it) or keep the existing try but add a finally that checks if conn is defined and then calls conn.close(); update references to conn.row_factory and execute accordingly.Gradata/src/gradata/_manifest_metrics.py (1)
54-102:⚠️ Potential issue | 🟠 MajorEnsure SQLite connections are always closed on exceptions.
Several paths open
connand only close on success; exceptions can leave handles open and cause intermittent SQLite lock issues.♻️ Suggested pattern (apply consistently)
+from contextlib import closing + def _correction_rate_trend(ctx: "BrainContext | None" = None, window: int = 10) -> dict | None: try: db = ctx.db_path if ctx else _p.DB_PATH - conn = get_connection(db) - max_session, _ = _session_window(conn, window) + with closing(get_connection(db)) as conn: + max_session, _ = _session_window(conn, window) - if max_session < window * 2: - conn.close() - return None + if max_session < window * 2: + return None - def _cro(min_s, max_s): + def _cro(min_s, max_s): ... - current = _cro(max_session - window + 1, max_session) - baseline = _cro(max_session - window * 2 + 1, max_session - window) - conn.close() + current = _cro(max_session - window + 1, max_session) + baseline = _cro(max_session - window * 2 + 1, max_session - window)Also applies to: 132-219, 305-390, 491-499
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Gradata/src/gradata/_manifest_metrics.py` around lines 54 - 102, The function _correction_rate_trend opens a SQLite connection via get_connection and may return early or raise, leaving conn unclosed; change the code to ensure conn is always closed by acquiring conn then wrapping all usage in a try/finally (or using a context manager if supported) and call conn.close() in the finally block; specifically ensure conn is closed even when _session_window, _cro, or any early return occurs (apply the same try/finally pattern around get_connection/conn usage for _correction_rate_trend, its inner helper _cro, and the other similar metric functions mentioned so they cannot leak SQLite handles).Gradata/src/gradata/context_wrapper.py (1)
141-148:⚠️ Potential issue | 🟠 MajorFall back to the preloaded rules when task-specific lookup fails.
After this change the failure is logged, but the prompt still drops all rules even when
self._rules_textwas already loaded in__init__. A transientapply_brain_rules()error should degrade to generic guidance, not to an unadapted prompt.Suggested fix
if self._brain and task: try: rules = self._brain.apply_brain_rules(task, context) if rules: parts.append(rules) except Exception as e: logger.debug("apply_brain_rules failed (non-fatal): %s", e) + if self._rules_text: + parts.append(self._rules_text) elif self._rules_text: parts.append(self._rules_text)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Gradata/src/gradata/context_wrapper.py` around lines 141 - 148, When apply_brain_rules(task, context) raises an exception in the ContextWrapper logic, the except block should append the preloaded self._rules_text (if present) so the prompt falls back to generic guidance instead of dropping all rules; update the except handler around the call to self._brain.apply_brain_rules in the method using self._brain, parts, and self._rules_text to log the error (keep logger.debug) and then if self._rules_text is truthy do parts.append(self._rules_text) so behavior matches the original __init__ preload; ensure existing branches for the case when self._brain is falsy remain unchanged.Gradata/src/gradata/enhancements/meta_rules.py (1)
326-374:⚠️ Potential issue | 🟠 MajorThis new discovery path is not wired into the session refresh flow yet.
brain_end_session()still callsrefresh_meta_rules(), and that function only revalidatesexisting_metas. As a result, the newdiscover_meta_rules()/merge_into_meta()logic here never runs for the main session-close path, someta_rules_discoveredwill stay at zero for fresh brains unless another caller invokes discovery explicitly.Also applies to: 377-429
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Gradata/src/gradata/enhancements/meta_rules.py` around lines 326 - 374, The session-close flow never invokes the new discovery path, so discover_meta_rules() and merge_into_meta() are not run and meta_rules_discovered stays zero; update the session refresh/close path by modifying brain_end_session() (and/or refresh_meta_rules()) to call discover_meta_rules() for eligible lessons (or call merge_into_meta() on new clusters) before or as part of the existing refresh_meta_rules() revalidation step, merge returned metas into the existing_metas set (preserving existing revalidation/decay logic such as _apply_decay and session=current_session), and ensure meta_rules_discovered is incremented or updated from the results of discover_meta_rules() so fresh brains register discovered rules.Gradata/src/gradata/_core.py (1)
1743-1791: 🛠️ Refactor suggestion | 🟠 MajorUnify the brain-level skill export API with
gradata.enhancements.skill_export.
brain_export_skill()/brain_export_skills()are still rendering their own skill format while this PR introduces a new canonical Anthropic skill exporter. Keeping both renderers will let CLI and Python exports drift on frontmatter, filtering, and meta-principle behavior almost immediately. Please delegate these brain helpers to the new module instead of maintaining a second formatter.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Gradata/src/gradata/_core.py` around lines 1743 - 1791, The brain-level exporters brain_export_skill and brain_export_skills duplicate formatting logic and should delegate to the canonical exporter in gradata.enhancements.skill_export; update brain_export_skill to call the appropriate function(s) from gradata.enhancements.skill_export (passing through brain, output_dir, min_state, skill_name) and return the same Path result, and change brain_export_skills to call the corresponding multi-skill exporter there (returning list[str]) instead of rendering its own skill format; ensure any provenance/manifest handling or defaults are passed through or removed so the single exporter in gradata.enhancements.skill_export controls frontmatter, filtering, and meta-principle behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Gradata/.archive/dashboard_streamlit_deprecated_2026-04-23.py`:
- Around line 178-183: brief_age_hours uses naive datetimes which can yield
incorrect differences across timezones; update it to use timezone-aware UTC
datetimes by calling datetime.now(UTC) and
datetime.fromtimestamp(BRIEF_PATH.stat().st_mtime, UTC) (or equivalent
datetime.timezone.utc) so the subtraction is between two aware datetimes and
then compute .total_seconds() / 3600 as before; ensure you import or reference
the same UTC tz object used elsewhere (e.g., UTC) to keep consistency with
_transcript.py.
- Around line 20-26: The constants BRAIN_DIR, DB_PATH, EVENTS_PATH,
LESSONS_PATH, PROSPECTS_DIR, BRIEF_PATH and TASKS_DIR use hardcoded absolute
Windows paths which break portability; replace them with a configurable base
directory (e.g., read from an environment variable or default to Path.home() /
".gradata" or a relative project path) and construct the derived paths (DB_PATH,
EVENTS_PATH, etc.) from that base using Path.joinpath to ensure cross-platform
behavior and easier extraction for future use.
- Around line 966-973: The loop over tables builds a SQL string with the dynamic
variable name and calls q(f"SELECT COUNT(*) as c FROM [{name}]"), which risks
SQL injection even if current names come from sqlite_master; update the logic in
the loop that iterates over tables and the q(...) invocation to avoid
interpolating raw table names by either (a) using a parameterized API that
supports identifiers or (b, preferred) validate/whitelist table names before use
(e.g., only allow exact names matching an explicit allowlist or a strict regex)
and then use the validated name when calling q; ensure the change touches the
for t in tables loop, the name variable, and the q(...) call so dynamic
identifiers are never passed unchecked.
In `@Gradata/CHANGELOG.md`:
- Line 66: The changelog entry currently reads "3932 pass, 3 skip" but the PR
objectives state "3969 passed, 3 skipped"; open CHANGELOG.md and update the test
summary string to match the verified test results (replace "3932 pass, 3 skip"
with the correct "3969 passed, 3 skipped" or the accurate numbers you confirm),
and ensure the surrounding sentence punctuation and tense match the rest of the
file (e.g., "passed"/"skipped" vs "pass"/"skip") so formatting stays consistent
with other entries.
In `@Gradata/docs/cloud/dashboard.md`:
- Line 3: The doc currently contradicts itself: line 3 says meta-rule synthesis
runs locally in the SDK, but the "Meta-rules" bullet in the Brain detail section
still calls them “cloud-synthesized principles.” Update the Brain detail
"Meta-rules" bullet (and any other references in this page using the phrase
“cloud-synthesized” or “cloud-synthesized principles”) to say that
meta-rules/principles are synthesized locally in the SDK (e.g., “synthesized
locally in the SDK” or “local meta-rule synthesis”), ensuring the page
consistently treats the SDK as the model-of-record for synthesis.
In `@Gradata/docs/concepts/meta-rules.md`:
- Around line 47-50: The admonition titled "Local by default" is being parsed as
ending early causing MD046 because the blank line between paragraphs isn’t
indented as part of the admonition body; make the admonition content contiguous
by either removing the blank line or by indenting that blank continuation line
(and any subsequent paragraph lines) to match the existing indented content
under the "!!! info \"Local by default\"" admonition so the entire block is
treated as one continuous admonition body.
In `@Gradata/docs/specs/cloud-sync-and-pricing.md`:
- Around line 35-40: The spec's markdown tables and code blocks are missing the
required blank lines and fence language identifiers which trigger markdownlint
rules MD058/MD031/MD040; update the examples around the table that contains
entries like `brain:sync`, `brain:read`, `team:admin`, and `marketplace:publish`
to ensure there is a blank line before and after the table and any fenced code
blocks, and add explicit fence language identifiers (e.g., ```yaml or ```json as
appropriate) for all fenced blocks in the range that includes the table (and
similarly for the examples in lines 60-112) so the linter stops flagging
MD058/MD031/MD040.
In `@Gradata/hooks/hooks.json`:
- Around line 50-70: Clarify that the Stop-chain hooks run continue-on-error
(ctx_watchdog timeout does not block session_close) and add a safety net: update
the Stop hook descriptions in hooks.json to explicitly state that
gradata.hooks.ctx_watchdog must complete before gradata.hooks.session_close for
a valid handoff, and modify gradata.hooks.session_close to detect a
missing/stale handoff (created by gradata.hooks.ctx_watchdog), log a clear
warning/error, and apply a safe fallback behavior (e.g., skip graduation or use
conservative defaults) so graduation logic does not proceed silently without
context.
In `@Gradata/migrations/supabase/015_events_unique.sql`:
- Around line 23-33: The current predicate uses containment (c.conkey @>
ARRAY[...]) which matches any unique constraint that is a superset; change it to
require exact match by replacing the containment operator with equality so the
constraint only matches when c.conkey equals the exact ARRAY[(SELECT attnum ...
'brain_id'), (SELECT attnum ... 'type'), (SELECT attnum ...
'created_at')]::smallint[]; target the check on table "events" and the c.conkey
expression in the migration so the migration will create UNIQUE (brain_id, type,
created_at) when no exact unique constraint exists.
In `@Gradata/migrations/supabase/README.md`:
- Around line 32-37: The fenced code block containing the constraint lines
(e.g., corrections_brain_session_desc_key,
corrections_brain_session_description_unique, events_brain_type_created_at_key,
events_brain_type_created_at_unique) needs a language specifier; update the
opening fence to use ```sql (or ```text) so the block renders correctly and the
linter stops flagging it, keeping the existing content and the closing ```
unchanged.
In `@Gradata/skills/core/session-start/SKILL.md`:
- Line 12: Replace the hardcoded absolute path
"C:/Users/olive/SpritesWork/brain/..." in SKILL.md with a portable
brain-relative reference: refer to the repository/workspace brain root (e.g.,
BRAIN_ROOT or workspace-relative path) and update the references to
continuation.md and scripts/continuation.py archive to use that relative root;
search SKILL.md for the exact string "C:/Users/olive/SpritesWork/brain" and
replace all occurrences (including the lines mentioning continuation.md and
scripts/continuation.py) with a pattern like "<BRAIN_ROOT>/continuation.md" and
"<BRAIN_ROOT>/scripts/continuation.py archive" or documentation text instructing
use of an env var or workspace variable so the doc works across machines.
- Around line 32-36: The fenced code block in SKILL.md (the triple-backtick
block containing the lines starting with “[check] S[N] loaded…”) lacks a
language identifier and triggers markdownlint MD040; update that block by adding
a language tag (e.g., ```text) immediately after the opening backticks so the
block becomes a labeled fenced code block and MD040 is resolved.
In `@Gradata/src/gradata/_cloud_sync.py`:
- Around line 514-533: When _transform_row(table, r, tenant_id) raises we
currently set all_ok=False but never record last_error, so the watermark isn't
advanced and no cloud_push_error.json is written; update the loop in the cloud
sync routine to capture transform exceptions and populate last_error (or build
an error object with table, row identifier and exception info) when catching in
the for r in rows loop, so that after the loop the existing logic will call
_record_push_error(brain, last_error) instead of stalling; alternatively,
quarantine the bad row (e.g., mark it in DB or push to a quarantine handler) and
set last_error accordingly so _mark_push(conn, tenant_id, started) and
_clear_push_error(brain) behavior remains correct for entirely successful pushes
while transform failures are surfaced.
- Around line 46-57: The code currently accepts a backend service-role secret
via ENV_KEY_ALIAS which exposes tenant-wide write authority; change the alias to
a client-scoped/anon key and stop consuming the service key. Update the
ENV_KEY_ALIAS constant value from "GRADATA_SUPABASE_SERVICE_KEY" to a
client-safe name such as "GRADATA_SUPABASE_ANON_KEY" (or remove the alias
entirely) and update the _env_key() function to only check ENV_KEY and the new
client-scoped alias (do not read or fallback to any service-role env var).
Ensure references to ENV_KEY_ALIAS and _env_key() are adjusted accordingly so
the client SDK never accepts the service-role key.
In `@Gradata/src/gradata/_core.py`:
- Around line 1399-1433: The CORRECTION rows read into session_corrections are
the persisted event payload shape (they contain final_text) but
_cloud_sync_session() expects the session-correction schema (it reads final), so
normalize each parsed dict before appending: inside the loop that builds
session_corrections (where rows are iterated and parsed via _json.loads),
map/rename the payload's final_text -> final (and copy or transform any other
fields the session schema expects) so each entry in session_corrections matches
the shape _cloud_sync_session(stub, session_number, all_lessons,
session_corrections, {}) expects; keep using the existing variables
session_corrections, db_path, CORRECTION, and _json/_cloud_sync_session to find
the code to change.
In `@Gradata/src/gradata/_doctor.py`:
- Around line 461-470: The cloud checks currently always perform network probes;
change _cloud_checks so network-dependent checks ( _check_cloud_reachable,
_check_cloud_auth, _check_cloud_has_data, _check_cloud_push_error ) are only
executed when configuration or environment indicates cloud usage (use the result
of _check_cloud_config() and/or _check_cloud_env_vars() as the gate), otherwise
yield/return skip results; likewise update the individual check functions
(_check_cloud_reachable, _check_cloud_auth, _check_cloud_has_data,
_check_cloud_push_error) to short-circuit and return a skip state if cloud is
not configured so local/offline users are not probed.
- Around line 345-383: _checks in _check_cloud_auth and _check_cloud_has_data
currently call _read_cloud_config() which only reads the file and ignores
env-driven credentials; replace that call with the shared cloud-config resolver
used by the sync code (the same function/import that other modules use to
resolve API_KEY, API_URL and BRAIN_ID from environment or file), then read
bearer = resolved["api_key"] (or empty), api_url = resolved["api_url"] (with the
same default and rstrip("/")), and brain_id = resolved["brain_id"] so both
functions honor env-based setup and probe the correct endpoint.
In `@Gradata/src/gradata/_events.py`:
- Around line 196-228: The raw side-log currently writes unredacted raw_event
(using raw_data) via _locked_append to events_jsonl.parent / "events.raw.jsonl",
violating the new redaction guarantee and risking leftover PII if emit() later
fails; update this block to either (preferably) write the redacted payload (use
redacted_data in raw_event/data) or gate the unredacted write behind an explicit
debug opt-in flag (e.g., check a new/existing config like
enable_raw_side_log_debug before writing), and ensure any fallback logic still
uses _redact_payload and the same redacted_data so events.raw.jsonl never
contains plain PII; keep function references emit(), _redact_payload,
_locked_append and the raw_event construction in mind when making the change.
In `@Gradata/src/gradata/_export_brain.py`:
- Around line 17-47: The export paths helpers (_VAULT_DIR, _LESSONS_ACTIVE,
_LESSONS_ARCHIVE, _QUALITY_RUBRICS, _DOMAIN_CONFIG, _DOMAIN_SOUL, _CARL_LOOP,
_CARL_GLOBAL) currently read global _p.* values instead of using the context
passed into export_brain(ctx), causing exports to resolve to the wrong brain;
update export_brain to derive brain_dir and working_dir from ctx and thread
those derived paths into all collectors and metadata readers (rather than
leaving them to read _p.*), and change any call sites that call these helper
functions or rely on _p to accept and use the contextual brain_dir/working_dir
so every exported asset is scoped to ctx.
In `@Gradata/src/gradata/_migrations/002_add_event_identity.py`:
- Around line 181-187: The fallback to Path.cwd() in _brain_dir_for can point to
the wrong directory for in-memory or unnamed DBs; change _brain_dir_for to treat
missing/":memory:" paths specially (check the result of PRAGMA database_list and
row[2] for empty or ":memory:") and instead return a safer default such as
Path.home().resolve() (or raise a clear exception) so .device_id isn’t created
in the current working directory; update the function _brain_dir_for to
implement this check and use Path.home().resolve() (or raise) as the fallback.
In `@Gradata/src/gradata/_migrations/003_add_sync_state.py`:
- Around line 127-128: Remove the redundant sys.path.insert call that re-inserts
the migration parent directory into sys.path; locate the duplicated statement
sys.path.insert(0, str(Path(__file__).resolve().parent)) near the import of
tenant_uuid/get_or_create_tenant_id and delete that second insertion so only the
initial sys.path modification (earlier in the file) remains, leaving the from
tenant_uuid import get_or_create_tenant_id import intact.
In `@Gradata/src/gradata/_migrations/device_uuid.py`:
- Around line 73-77: The code can return new_did without persisting it if the
temp-file creation raises FileExistsError and fpath still doesn't exist; update
the temp-file creation/exception handling so that on FileExistsError you check
fpath.exists() and if it now exists read-and-validate (using _is_valid) and
return that value, otherwise remove the stale temp artifact (or generate a new
unique temp name) and retry writing new_did atomically to fpath (or open fpath
with exclusive-create) until either a valid on-disk ID is found or new_did is
persisted; adjust the logic around fpath, new_did, and the temp-file creation
block to ensure new_did is always written before being returned.
In `@Gradata/src/gradata/_transcript_providers.py`:
- Around line 54-61: The fallback that picks the most-recent JSONL can crash if
a file is removed between iterdir() and stat() because max(all_jsonls,
key=lambda p: p.stat().st_mtime) does not handle stat errors; modify the
selection to compute mtimes defensively (e.g. replace the direct key call with a
small helper or loop that calls p.stat().st_mtime inside a try/except catching
FileNotFoundError/OSError and skipping files that fail) so all_jsonls only
contains files with successful stats before calling max(), and then return the
max (or None if none remain).
In `@Gradata/src/gradata/_transcript.py`:
- Around line 138-148: The cleanup currently unlinks old transcript files but
leaves their parent session directories orphaned; after calling
transcript.unlink(missing_ok=True) in the loop (and incrementing deleted), check
whether session_dir is now empty and if so remove it (e.g., call
session_dir.rmdir() or equivalent) wrapped in a try/except to ignore OSError if
the dir is not empty or removal fails; use the existing session_dir and
transcript variables and perform this removal only after a successful unlink to
avoid deleting dirs that still hold other artifacts.
In `@Gradata/src/gradata/cli.py`:
- Around line 225-231: Reject contradictory flag combinations by checking args
before calling diagnose: detect when both getattr(args, "cloud", False) and
getattr(args, "no_cloud", False) are true and fail fast (raise a
UsageError/ValueError or print a clear error and exit) instead of constructing
cloud_only/include_cloud; update the logic around cloud_only, include_cloud and
the call to diagnose(brain_dir=brain_dir, include_cloud=include_cloud,
cloud_only=cloud_only) to perform this guard first and return/exit with a clear
message referencing the conflicting --cloud and --no-cloud flags.
- Around line 1110-1115: cmd_skill_export currently resolves brain_root with
_resolve_brain_root(args) but then calls _get_brain(args) separately, which can
pick a different root; instead resolve the brain once and use that instance for
lessons and metadata. Change the code so you call _resolve_brain_root(args) to
get brain_root, then obtain the Brain instance from that root (e.g., call
_get_brain with the explicit root or add a helper that loads a Brain from
brain_root) and use that single Brain instance for brain._find_lessons_path()
and all subsequent operations, removing the separate _get_brain(args) call to
avoid mixed-brain exports.
In `@Gradata/src/gradata/cloud/sync.py`:
- Around line 160-163: The module docstring's wire-protocol path is out of sync
with the actual call in sync.py — update the header/docs to reference the new
metrics endpoint /api/v1/telemetry/metrics (so it matches the POST call via
self._post in the sync module). Locate the module-level docstring or
wire-protocol section in Gradata/src/gradata/cloud/sync.py and replace any
occurrences of /v1/telemetry/metrics with /api/v1/telemetry/metrics (ensure any
examples or notes reference the same path used by the
_post("/api/v1/telemetry/metrics", ...) call).
In `@Gradata/src/gradata/contrib/enhancements/eval_benchmark.py`:
- Line 243: The single-line ternary calculating high_value_accuracy (sum(1 for
cr in hv_cases if cr.high_value_correct) / len(hv_cases) if hv_cases else 1.0)
should be reformatted to match the multi-line style used earlier (226-230,
234-238): break the expression across lines with the sum(...) on its own line,
the division on the next line, include the if hv_cases else 1.0 on separate
lines wrapped in parentheses for readability, and keep the same variable names
(hv_cases, cr, high_value_correct) and logic in the function/method where this
expression appears.
In `@Gradata/src/gradata/daemon.py`:
- Around line 879-881: Replace the use of Python's randomized built-in hash in
_pick_port with a stable cryptographic hash: compute a digest (e.g.,
hashlib.sha256) of the brain_dir_str (encoded to bytes), convert the digest to
an integer (e.g., int.from_bytes or int(hex, 16)), then take that integer modulo
16383 and add 49152 to return the port; update _pick_port to use this
deterministic approach so the same brain_dir_str produces the same port across
processes.
In `@Gradata/src/gradata/enhancements/bandits/contextual_bandit.py`:
- Around line 52-55: The diff shows whitespace/comment alignment changes around
the contextual bandit dataclass fields (rule_id, alpha, beta) that are
formatting-only; revert manual spacing and run an automated formatter (e.g.,
ruff format and/or black) on gradata/enhancements/bandits/contextual_bandit.py
(and the other affected ranges like the block at lines 72-75) to enforce
consistent formatting, then split formatting-only changes into a separate
commit/PR so future reviews separate style fixes from functional changes.
In `@Gradata/src/gradata/enhancements/edit_classifier.py`:
- Around line 56-58: _TONE_WORDS contains "I think", "I believe", "I feel" with
capital I but matching is done against old_lower/new_lower, causing those never
to match; update the entries in _TONE_WORDS to their lowercase forms ("i think",
"i believe", "i feel") or normalize the whole list by lowercasing it where
_TONE_WORDS is constructed so that comparisons against old_lower/new_lower
succeed (refer to the _TONE_WORDS symbol in edit_classifier.py).
In `@Gradata/src/gradata/enhancements/git_backfill.py`:
- Around line 183-187: The list comprehension for filtered_files redundantly
calls f.strip() twice; change it to compute a single stripped variable per
element and reuse it for both the output and the Path match (i.e., iterate over
files_result.completed.stdout.strip().splitlines(), set stripped = f.strip(),
then include stripped in the list and call Path(stripped).match(p) against
patterns). Update the expression that builds filtered_files to use this cached
stripped value.
In `@Gradata/src/gradata/enhancements/observation_hooks.py`:
- Around line 82-87: The private-key regex currently only matches the BEGIN
header, leaving PEM body unredacted; replace the tuple containing
_re.compile(r"-----BEGIN [A-Z ]+-----") with a regex that matches entire PEM
blocks (BEGIN header through END footer and their base64/body), e.g. use
_re.compile(r"-----BEGIN [A-Z ]+-----[\\s\\S]+?-----END [A-Z ]+-----", _re.I)
(or equivalent non-greedy/single-line pattern) so the whole key block is
replaced with "[PRIVATE_KEY]" wherever the list of redaction patterns (the
tuples in observation_hooks.py) is defined.
In `@Gradata/src/gradata/enhancements/reporting.py`:
- Around line 335-340: The SQLite connection is opened into conn and manually
closed, which can leak if an exception occurs; refactor to use a context
manager: replace sqlite3.connect(str(db)); ... conn.close() with a with
sqlite3.connect(str(db)) as conn: block and move the two queries that set
report.sessions_total and report.events_total inside that block so the
connection is always closed even if execute(...).fetchone() raises; update
references to conn, report.sessions_total and report.events_total accordingly.
In `@Gradata/src/gradata/enhancements/rule_pipeline.py`:
- Around line 434-438: The direct import and call of _graduate (symbols:
_graduate, pre_states, all_lessons) can raise and bypass the pipeline's error
contract; wrap both the import and the _graduate(all_lessons) invocation in a
try/except that catches Exception, and on error append a structured error entry
to the pipeline result's error collection (e.g., PipelineResult.errors or
pipeline_result.errors) instead of letting the exception propagate; keep
existing pre_states computation as-is and only mutate lesson state when the call
succeeds, and ensure the exception handler preserves control flow by returning
or continuing the pipeline per existing error-handling conventions.
In `@Gradata/src/gradata/enhancements/rule_synthesizer.py`:
- Around line 74-94: The cache key omits the `context` that affects prompt
synthesis, causing stale cache reuse; modify the signature of _compute_cache_key
to accept an additional context: str parameter and include a normalized context
(e.g., context.strip()) as a new part (e.g., "CONTEXT:" + context.strip())
before hashing, keeping the same hashing/truncation behavior; then update every
caller of _compute_cache_key to pass the current context value so the cache
differentiates sessions.
- Around line 326-329: The cached value returned from _read_cache(cache_key)
must be validated before returning to avoid sticky corrupt entries: after
retrieving cached in the rule_synthesizer logic (the block using cached,
_read_cache, _log.debug, and cache_key), run the same _extract_brain_wisdom and
length/format checks that the non-cached path uses; only return cached if those
validations pass, otherwise log a cache-miss/validation-failure and proceed with
the normal extraction/generation flow (and consider deleting or refreshing the
invalid cache entry).
- Around line 200-225: The _call_gemini function currently ignores the timeout
parameter; update the GenerateContentConfig call inside _call_gemini to pass
http_options=genai_types.HttpOptions(timeout=int(timeout * 1000)) so the Gemini
SDK receives a millisecond timeout, ensuring the same 20s fail-safe behavior as
other branches; keep using genai_types for HttpOptions and convert seconds to
milliseconds with int(timeout * 1000) when constructing the config.
In `@Gradata/src/gradata/enhancements/scoring/failure_detectors.py`:
- Around line 190-192: The alert message string that was collapsed to a single
line ("Rule misfire rate increased. New rules may be overfitting to specific
sessions.") should be converted back to the same multi-line string formatting
used by the other detectors; locate the detector with that message (the
rule-misfire detector) and change its message value to the multi-line style
consistent with detect_being_ignored, detect_playing_safe, and
detect_regression_to_mean so all four detectors use the same formatting pattern.
In `@Gradata/src/gradata/enhancements/skill_export.py`:
- Around line 101-104: The current code in skill_export.py builds frontmatter by
hand-escaping the description (variable description -> safe_desc) and can
produce invalid YAML for newlines or trailing backslashes; replace the manual
escaping and f-string lines.append(f'description: "{safe_desc}"') with a real
scalar serializer (e.g. use json.dumps(description) or yaml.safe_dump of the
single string) so the description is correctly serialized as a YAML/quoted
string; update the code around lines.append(f"name: {name}") / safe_desc to call
the serializer and append its result (refer to the variables description,
safe_desc and the list lines).
---
Outside diff comments:
In `@Gradata/src/gradata/_context_packet.py`:
- Around line 231-240: The SQLite connection opened in _load_audit_context (via
sqlite3.connect) is only closed on the success path; change the logic to always
close the connection on error by using a context manager or a try/finally:
either wrap sqlite3.connect(...) in a with sqlite3.connect(...) as conn block
(ensuring conn.row_factory and the execute/fetchone run inside it) or keep the
existing try but add a finally that checks if conn is defined and then calls
conn.close(); update references to conn.row_factory and execute accordingly.
In `@Gradata/src/gradata/_core.py`:
- Around line 1743-1791: The brain-level exporters brain_export_skill and
brain_export_skills duplicate formatting logic and should delegate to the
canonical exporter in gradata.enhancements.skill_export; update
brain_export_skill to call the appropriate function(s) from
gradata.enhancements.skill_export (passing through brain, output_dir, min_state,
skill_name) and return the same Path result, and change brain_export_skills to
call the corresponding multi-skill exporter there (returning list[str]) instead
of rendering its own skill format; ensure any provenance/manifest handling or
defaults are passed through or removed so the single exporter in
gradata.enhancements.skill_export controls frontmatter, filtering, and
meta-principle behavior.
In `@Gradata/src/gradata/_manifest_metrics.py`:
- Around line 54-102: The function _correction_rate_trend opens a SQLite
connection via get_connection and may return early or raise, leaving conn
unclosed; change the code to ensure conn is always closed by acquiring conn then
wrapping all usage in a try/finally (or using a context manager if supported)
and call conn.close() in the finally block; specifically ensure conn is closed
even when _session_window, _cro, or any early return occurs (apply the same
try/finally pattern around get_connection/conn usage for _correction_rate_trend,
its inner helper _cro, and the other similar metric functions mentioned so they
cannot leak SQLite handles).
In `@Gradata/src/gradata/context_wrapper.py`:
- Around line 141-148: When apply_brain_rules(task, context) raises an exception
in the ContextWrapper logic, the except block should append the preloaded
self._rules_text (if present) so the prompt falls back to generic guidance
instead of dropping all rules; update the except handler around the call to
self._brain.apply_brain_rules in the method using self._brain, parts, and
self._rules_text to log the error (keep logger.debug) and then if
self._rules_text is truthy do parts.append(self._rules_text) so behavior matches
the original __init__ preload; ensure existing branches for the case when
self._brain is falsy remain unchanged.
In `@Gradata/src/gradata/contrib/patterns/human_loop.py`:
- Around line 494-503: The check method in HumanLoopGate ignores the provided
context by calling gate(action) without passing context, which prevents
context-driven risk overrides from being considered; update HumanLoopGate.check
to pass the context through to the gate invocation (e.g., call gate with the
context argument) so risk_override and any context-based classification are
honored when building the ApprovalResult.
In `@Gradata/src/gradata/enhancements/meta_rules.py`:
- Around line 326-374: The session-close flow never invokes the new discovery
path, so discover_meta_rules() and merge_into_meta() are not run and
meta_rules_discovered stays zero; update the session refresh/close path by
modifying brain_end_session() (and/or refresh_meta_rules()) to call
discover_meta_rules() for eligible lessons (or call merge_into_meta() on new
clusters) before or as part of the existing refresh_meta_rules() revalidation
step, merge returned metas into the existing_metas set (preserving existing
revalidation/decay logic such as _apply_decay and session=current_session), and
ensure meta_rules_discovered is incremented or updated from the results of
discover_meta_rules() so fresh brains register discovered rules.
🪄 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: 877d56b3-791b-415c-921e-c18efabdb357
⛔ Files ignored due to path filters (1)
.claude/hooks/statusline/sprites-statusline.jsis excluded by!.claude/**
📒 Files selected for processing (240)
.gitignoreGradata/.archive/dashboard_streamlit_deprecated_2026-04-23.pyGradata/CHANGELOG.mdGradata/docs/LEGACY_CLEANUP.mdGradata/docs/architecture/cloud-monolith-v2.mdGradata/docs/architecture/multi-tenant-future-proofing.mdGradata/docs/cloud/dashboard.mdGradata/docs/cloud/overview.mdGradata/docs/concepts/meta-rules.mdGradata/docs/specs/cloud-sync-and-pricing.mdGradata/hooks/hooks.jsonGradata/migrations/supabase/014_corrections_unique.sqlGradata/migrations/supabase/015_events_unique.sqlGradata/migrations/supabase/016_brains_last_used_at.sqlGradata/migrations/supabase/README.mdGradata/skills/core/session-start/SKILL.mdGradata/src/gradata/__init__.pyGradata/src/gradata/_brain_manifest.pyGradata/src/gradata/_cloud_sync.pyGradata/src/gradata/_config.pyGradata/src/gradata/_config_paths.pyGradata/src/gradata/_context_compile.pyGradata/src/gradata/_context_packet.pyGradata/src/gradata/_core.pyGradata/src/gradata/_data_flow_audit.pyGradata/src/gradata/_db.pyGradata/src/gradata/_doctor.pyGradata/src/gradata/_events.pyGradata/src/gradata/_export_brain.pyGradata/src/gradata/_fact_extractor.pyGradata/src/gradata/_file_lock.pyGradata/src/gradata/_http.pyGradata/src/gradata/_installer.pyGradata/src/gradata/_manifest_helpers.pyGradata/src/gradata/_manifest_metrics.pyGradata/src/gradata/_migrations/001_add_tenant_id.pyGradata/src/gradata/_migrations/002_add_event_identity.pyGradata/src/gradata/_migrations/003_add_sync_state.pyGradata/src/gradata/_migrations/_runner.pyGradata/src/gradata/_migrations/_ulid.pyGradata/src/gradata/_migrations/device_uuid.pyGradata/src/gradata/_migrations/fill_null_tenant.pyGradata/src/gradata/_migrations/tenant_uuid.pyGradata/src/gradata/_mine_transcripts.pyGradata/src/gradata/_paths.pyGradata/src/gradata/_query.pyGradata/src/gradata/_stats.pyGradata/src/gradata/_telemetry.pyGradata/src/gradata/_tenant.pyGradata/src/gradata/_text_utils.pyGradata/src/gradata/_transcript.pyGradata/src/gradata/_transcript_providers.pyGradata/src/gradata/_types.pyGradata/src/gradata/_validator.pyGradata/src/gradata/_workers.pyGradata/src/gradata/adapters/mem0.pyGradata/src/gradata/audit.pyGradata/src/gradata/brain.pyGradata/src/gradata/brain_inspection.pyGradata/src/gradata/cli.pyGradata/src/gradata/cloud/client.pyGradata/src/gradata/cloud/sync.pyGradata/src/gradata/context_wrapper.pyGradata/src/gradata/contrib/enhancements/eval_benchmark.pyGradata/src/gradata/contrib/enhancements/install_manifest.pyGradata/src/gradata/contrib/enhancements/quality_gates.pyGradata/src/gradata/contrib/enhancements/truth_protocol.pyGradata/src/gradata/contrib/patterns/__init__.pyGradata/src/gradata/contrib/patterns/agent_modes.pyGradata/src/gradata/contrib/patterns/context_brackets.pyGradata/src/gradata/contrib/patterns/evaluator.pyGradata/src/gradata/contrib/patterns/execute_qualify.pyGradata/src/gradata/contrib/patterns/guardrails.pyGradata/src/gradata/contrib/patterns/human_loop.pyGradata/src/gradata/contrib/patterns/loop_detection.pyGradata/src/gradata/contrib/patterns/mcp.pyGradata/src/gradata/contrib/patterns/memory.pyGradata/src/gradata/contrib/patterns/middleware.pyGradata/src/gradata/contrib/patterns/orchestrator.pyGradata/src/gradata/contrib/patterns/parallel.pyGradata/src/gradata/contrib/patterns/pipeline.pyGradata/src/gradata/contrib/patterns/q_learning_router.pyGradata/src/gradata/contrib/patterns/rag.pyGradata/src/gradata/contrib/patterns/reconciliation.pyGradata/src/gradata/contrib/patterns/reflection.pyGradata/src/gradata/contrib/patterns/sub_agents.pyGradata/src/gradata/contrib/patterns/task_escalation.pyGradata/src/gradata/contrib/patterns/tools.pyGradata/src/gradata/contrib/patterns/tree_of_thoughts.pyGradata/src/gradata/correction_detector.pyGradata/src/gradata/daemon.pyGradata/src/gradata/detection/addition_pattern.pyGradata/src/gradata/enhancements/_sanitize.pyGradata/src/gradata/enhancements/bandits/collaborative_filter.pyGradata/src/gradata/enhancements/bandits/contextual_bandit.pyGradata/src/gradata/enhancements/behavioral_engine.pyGradata/src/gradata/enhancements/causal_chains.pyGradata/src/gradata/enhancements/cluster_manager.pyGradata/src/gradata/enhancements/clustering.pyGradata/src/gradata/enhancements/contradiction_detector.pyGradata/src/gradata/enhancements/dedup.pyGradata/src/gradata/enhancements/diff_engine.pyGradata/src/gradata/enhancements/edit_classifier.pyGradata/src/gradata/enhancements/freshness.pyGradata/src/gradata/enhancements/git_backfill.pyGradata/src/gradata/enhancements/graduation/agent_graduation.pyGradata/src/gradata/enhancements/graduation/judgment_decay.pyGradata/src/gradata/enhancements/graduation/rules_distillation.pyGradata/src/gradata/enhancements/graduation/scoring.pyGradata/src/gradata/enhancements/instruction_cache.pyGradata/src/gradata/enhancements/learning_pipeline.pyGradata/src/gradata/enhancements/lesson_discriminator.pyGradata/src/gradata/enhancements/llm_provider.pyGradata/src/gradata/enhancements/llm_synthesizer.pyGradata/src/gradata/enhancements/memory_taxonomy.pyGradata/src/gradata/enhancements/meta_rules.pyGradata/src/gradata/enhancements/meta_rules_storage.pyGradata/src/gradata/enhancements/metrics.pyGradata/src/gradata/enhancements/observation_hooks.pyGradata/src/gradata/enhancements/pattern_extractor.pyGradata/src/gradata/enhancements/pattern_integration.pyGradata/src/gradata/enhancements/pipeline_rewriter.pyGradata/src/gradata/enhancements/profiling/tone_profile.pyGradata/src/gradata/enhancements/prompt_synthesizer.pyGradata/src/gradata/enhancements/reporting.pyGradata/src/gradata/enhancements/retrieval_fusion.pyGradata/src/gradata/enhancements/router_warmstart.pyGradata/src/gradata/enhancements/rule_canary.pyGradata/src/gradata/enhancements/rule_context_bridge.pyGradata/src/gradata/enhancements/rule_export.pyGradata/src/gradata/enhancements/rule_integrity.pyGradata/src/gradata/enhancements/rule_pipeline.pyGradata/src/gradata/enhancements/rule_synthesizer.pyGradata/src/gradata/enhancements/rule_to_hook.pyGradata/src/gradata/enhancements/rule_verifier.pyGradata/src/gradata/enhancements/scoring/brain_scores.pyGradata/src/gradata/enhancements/scoring/calibration.pyGradata/src/gradata/enhancements/scoring/correction_tracking.pyGradata/src/gradata/enhancements/scoring/failure_detectors.pyGradata/src/gradata/enhancements/scoring/gate_calibration.pyGradata/src/gradata/enhancements/scoring/loop_intelligence.pyGradata/src/gradata/enhancements/scoring/memory_extraction.pyGradata/src/gradata/enhancements/scoring/reports.pyGradata/src/gradata/enhancements/scoring/success_conditions.pyGradata/src/gradata/enhancements/self_improvement/__init__.pyGradata/src/gradata/enhancements/self_improvement/_confidence.pyGradata/src/gradata/enhancements/self_improvement/_graduation.pyGradata/src/gradata/enhancements/similarity.pyGradata/src/gradata/enhancements/skill_export.pyGradata/src/gradata/events_bus.pyGradata/src/gradata/graph.pyGradata/src/gradata/hooks/_base.pyGradata/src/gradata/hooks/_generated_runner_core.pyGradata/src/gradata/hooks/_installer.pyGradata/src/gradata/hooks/_profiles.pyGradata/src/gradata/hooks/agent_graduation.pyGradata/src/gradata/hooks/agent_precontext.pyGradata/src/gradata/hooks/auto_correct.pyGradata/src/gradata/hooks/brain_maintain.pyGradata/src/gradata/hooks/claude_code.pyGradata/src/gradata/hooks/client.pyGradata/src/gradata/hooks/config_protection.pyGradata/src/gradata/hooks/config_validate.pyGradata/src/gradata/hooks/context_inject.pyGradata/src/gradata/hooks/ctx_watchdog.pyGradata/src/gradata/hooks/daemon.pyGradata/src/gradata/hooks/dispatch_post.pyGradata/src/gradata/hooks/duplicate_guard.pyGradata/src/gradata/hooks/generated_runner.pyGradata/src/gradata/hooks/generated_runner_post.pyGradata/src/gradata/hooks/graph_first_check.pyGradata/src/gradata/hooks/graph_session_track.pyGradata/src/gradata/hooks/implicit_feedback.pyGradata/src/gradata/hooks/inject_brain_rules.pyGradata/src/gradata/hooks/jit_inject.pyGradata/src/gradata/hooks/pre_compact.pyGradata/src/gradata/hooks/rule_enforcement.pyGradata/src/gradata/hooks/secret_scan.pyGradata/src/gradata/hooks/self_review.pyGradata/src/gradata/hooks/session_boot.pyGradata/src/gradata/hooks/session_close.pyGradata/src/gradata/hooks/session_persist.pyGradata/src/gradata/hooks/stale_hook_check.pyGradata/src/gradata/hooks/status_line.pyGradata/src/gradata/hooks/telemetry_summary.pyGradata/src/gradata/hooks/tool_failure_emit.pyGradata/src/gradata/hooks/tool_finding_capture.pyGradata/src/gradata/inspection.pyGradata/src/gradata/integrations/anthropic_adapter.pyGradata/src/gradata/integrations/openai_adapter.pyGradata/src/gradata/mcp_server.pyGradata/src/gradata/mcp_tools.pyGradata/src/gradata/middleware/__init__.pyGradata/src/gradata/middleware/_core.pyGradata/src/gradata/middleware/anthropic_adapter.pyGradata/src/gradata/middleware/crewai_adapter.pyGradata/src/gradata/middleware/langchain_adapter.pyGradata/src/gradata/middleware/openai_adapter.pyGradata/src/gradata/notifications.pyGradata/src/gradata/onboard.pyGradata/src/gradata/rules/rule_context.pyGradata/src/gradata/rules/rule_engine/__init__.pyGradata/src/gradata/rules/rule_engine/_formatting.pyGradata/src/gradata/rules/rule_ranker.pyGradata/src/gradata/rules/scope.pyGradata/src/gradata/safety.pyGradata/src/gradata/security/correction_hash.pyGradata/src/gradata/security/correction_provenance.pyGradata/src/gradata/security/manifest_signing.pyGradata/src/gradata/sidecar/watcher.pyGradata/tests/conftest.pyGradata/tests/test_agent_graduation.pyGradata/tests/test_bug_fixes.pyGradata/tests/test_cloud_row_push.pyGradata/tests/test_cloud_sync.pyGradata/tests/test_cluster_injection.pyGradata/tests/test_ctx_watchdog.pyGradata/tests/test_doctor_cloud.pyGradata/tests/test_emit_pii_redaction.pyGradata/tests/test_graph_enforcement.pyGradata/tests/test_hooks_intelligence.pyGradata/tests/test_hooks_learning.pyGradata/tests/test_implicit_feedback.pyGradata/tests/test_inject_watchdog_phases.pyGradata/tests/test_integration_workflow.pyGradata/tests/test_lesson_applications.pyGradata/tests/test_llm_synthesizer.pyGradata/tests/test_mem0_adapter.pyGradata/tests/test_meta_rule_generalization.pyGradata/tests/test_meta_rules.pyGradata/tests/test_migration_002_event_identity.pyGradata/tests/test_migration_003_sync_state.pyGradata/tests/test_multi_brain_simulation.pyGradata/tests/test_pipeline_e2e.pyGradata/tests/test_pre_compact.pyGradata/tests/test_rule_pipeline.pyGradata/tests/test_rule_synthesizer.pyGradata/tests/test_session_close_loop_state.pyGradata/tests/test_skill_export.pyGradata/tests/test_transcript.py
💤 Files with no reviewable changes (1)
- Gradata/src/gradata/enhancements/self_improvement/_graduation.py
📜 Review details
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Gradata
Repo: Gradata/gradata PR: 0
File: :0-0
Timestamp: 2026-04-17T17:18:07.439Z
Learning: In PR `#102` (gradata/gradata), Round 2 addressed: cli.py env-first brain resolution (GRADATA_BRAIN > --brain-dir > cwd), _tenant.py corrupt .tenant_id overwrite, _env_int default clamping to minimum, and _events.py tenant-scoped fallback SELECT for dedup. All ruff and 99 tests green after these fixes.
📚 Learning: 2026-04-17T17:18:07.439Z
Learnt from: Gradata
Repo: Gradata/gradata PR: 0
File: :0-0
Timestamp: 2026-04-17T17:18:07.439Z
Learning: In PR `#102` (gradata/gradata), Round 2 addressed: cli.py env-first brain resolution (GRADATA_BRAIN > --brain-dir > cwd), _tenant.py corrupt .tenant_id overwrite, _env_int default clamping to minimum, and _events.py tenant-scoped fallback SELECT for dedup. All ruff and 99 tests green after these fixes.
Applied to files:
Gradata/src/gradata/_migrations/tenant_uuid.pyGradata/src/gradata/_migrations/fill_null_tenant.pyGradata/src/gradata/_migrations/001_add_tenant_id.pyGradata/docs/architecture/multi-tenant-future-proofing.md.gitignoreGradata/CHANGELOG.mdGradata/src/gradata/brain.pyGradata/src/gradata/brain_inspection.pyGradata/src/gradata/_export_brain.pyGradata/src/gradata/_core.py
🪛 LanguageTool
Gradata/docs/specs/cloud-sync-and-pricing.md
[grammar] ~102-~102: Ensure spelling is correct
Context: ...vior: - Triggered on Stop hook or every 5min when events accumulated. - Pushes since...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~269-~269: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...vent push logged with content_hash. - Every ACL change emits an acl_changed event...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[grammar] ~290-~290: Ensure spelling is correct
Context: ... cadence:** hourly for Personal+, every 15min for Teams+, continuous WAL for Enterpri...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
Gradata/docs/cloud/overview.md
[style] ~3-~3: ‘on top of that’ might be wordy. Consider a shorter alternative.
Context: ...uity, team sharing, and managed backups on top of that local loop. ## What's in the SDK vs th...
(EN_WORDINESS_PREMIUM_ON_TOP_OF_THAT)
🪛 markdownlint-cli2 (0.22.1)
Gradata/skills/core/session-start/SKILL.md
[warning] 32-32: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
Gradata/migrations/supabase/README.md
[warning] 32-32: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
Gradata/docs/concepts/meta-rules.md
[warning] 50-50: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
Gradata/docs/specs/cloud-sync-and-pricing.md
[warning] 35-35: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 60-60: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 81-81: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 95-95: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 109-109: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
| BRAIN_DIR = Path("C:/Users/olive/SpritesWork/brain") | ||
| DB_PATH = BRAIN_DIR / "system.db" | ||
| EVENTS_PATH = BRAIN_DIR / "events.jsonl" | ||
| LESSONS_PATH = BRAIN_DIR / "lessons.md" | ||
| PROSPECTS_DIR = BRAIN_DIR / "prospects" | ||
| BRIEF_PATH = BRAIN_DIR / "morning-brief.md" | ||
| TASKS_DIR = Path("C:/Users/olive/.claude/scheduled-tasks") |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Hardcoded absolute paths limit portability (archived file).
The hardcoded Windows paths like C:/Users/olive/SpritesWork/brain will fail on other machines. Since this is archived, no action is required, but note this pattern if extracting any code for future use.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Gradata/.archive/dashboard_streamlit_deprecated_2026-04-23.py` around lines
20 - 26, The constants BRAIN_DIR, DB_PATH, EVENTS_PATH, LESSONS_PATH,
PROSPECTS_DIR, BRIEF_PATH and TASKS_DIR use hardcoded absolute Windows paths
which break portability; replace them with a configurable base directory (e.g.,
read from an environment variable or default to Path.home() / ".gradata" or a
relative project path) and construct the derived paths (DB_PATH, EVENTS_PATH,
etc.) from that base using Path.joinpath to ensure cross-platform behavior and
easier extraction for future use.
| def brief_age_hours(): | ||
| if not BRIEF_PATH.exists(): | ||
| return 9999 | ||
| return ( | ||
| datetime.now() - datetime.fromtimestamp(BRIEF_PATH.stat().st_mtime) | ||
| ).total_seconds() / 3600 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Naive datetime comparison may cause timezone inconsistencies (archived file).
datetime.now() and datetime.fromtimestamp() return naive (timezone-unaware) datetimes. If the system timezone changes or differs from UTC, this calculation can be off. The newer _transcript.py correctly uses datetime.now(UTC) — a pattern worth following if this code is ever revived.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Gradata/.archive/dashboard_streamlit_deprecated_2026-04-23.py` around lines
178 - 183, brief_age_hours uses naive datetimes which can yield incorrect
differences across timezones; update it to use timezone-aware UTC datetimes by
calling datetime.now(UTC) and datetime.fromtimestamp(BRIEF_PATH.stat().st_mtime,
UTC) (or equivalent datetime.timezone.utc) so the subtraction is between two
aware datetimes and then compute .total_seconds() / 3600 as before; ensure you
import or reference the same UTC tz object used elsewhere (e.g., UTC) to keep
consistency with _transcript.py.
| for t in tables: | ||
| name = t["name"] | ||
| if name.startswith("brain_fts") or name == "sqlite_sequence": | ||
| continue | ||
| cnt = q(f"SELECT COUNT(*) as c FROM [{name}]") | ||
| rows = cnt[0]["c"] if cnt else 0 | ||
| total_rows += rows | ||
| stats.append({"Table": name, "Records": rows}) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Dynamic table name in SQL query.
The table name is interpolated directly into the SQL string. While name originates from sqlite_master (trusted), this pattern is generally risky. Since this is archived, no immediate fix needed, but prefer parameterized queries or explicit allowlists in active code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Gradata/.archive/dashboard_streamlit_deprecated_2026-04-23.py` around lines
966 - 973, The loop over tables builds a SQL string with the dynamic variable
name and calls q(f"SELECT COUNT(*) as c FROM [{name}]"), which risks SQL
injection even if current names come from sqlite_master; update the logic in the
loop that iterates over tables and the q(...) invocation to avoid interpolating
raw table names by either (a) using a parameterized API that supports
identifiers or (b, preferred) validate/whitelist table names before use (e.g.,
only allow exact names matching an explicit allowlist or a strict regex) and
then use the validated name when calling q; ensure the change touches the for t
in tables loop, the name variable, and the q(...) call so dynamic identifiers
are never passed unchecked.
|
|
||
| ### Tests | ||
|
|
||
| - **3932 pass, 3 skip** (up from 2598 in v0.6.0). No xfail remaining. |
There was a problem hiding this comment.
Verify the test count.
The changelog states "3932 pass, 3 skip" but the PR objectives mention "3969 passed, 3 skipped". Please verify and update the correct count.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Gradata/CHANGELOG.md` at line 66, The changelog entry currently reads "3932
pass, 3 skip" but the PR objectives state "3969 passed, 3 skipped"; open
CHANGELOG.md and update the test summary string to match the verified test
results (replace "3932 pass, 3 skip" with the correct "3969 passed, 3 skipped"
or the accurate numbers you confirm), and ensure the surrounding sentence
punctuation and tense match the rest of the file (e.g., "passed"/"skipped" vs
"pass"/"skip") so formatting stays consistent with other entries.
| # Dashboard | ||
|
|
||
| The Gradata Cloud dashboard is a Next.js app at [app.gradata.ai](https://app.gradata.ai). It wraps the same data the local `brain.manifest.json` exposes, plus Cloud-only views for meta-rule synthesis, team management, and the operator console. | ||
| The Gradata Cloud dashboard is a Next.js app at [app.gradata.ai](https://app.gradata.ai). It visualizes the same data the local `brain.manifest.json` exposes, plus Cloud-only views for team management and the operator console. Meta-rule synthesis runs locally in the SDK — the dashboard renders the results, it does not re-run them. |
There was a problem hiding this comment.
This now contradicts the later “Meta-rules” bullet.
Line 3 says synthesis runs locally, but the Brain detail section still describes meta-rules as “cloud-synthesized principles.” Please update that later bullet too so the page has one model of record.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Gradata/docs/cloud/dashboard.md` at line 3, The doc currently contradicts
itself: line 3 says meta-rule synthesis runs locally in the SDK, but the
"Meta-rules" bullet in the Brain detail section still calls them
“cloud-synthesized principles.” Update the Brain detail "Meta-rules" bullet (and
any other references in this page using the phrase “cloud-synthesized” or
“cloud-synthesized principles”) to say that meta-rules/principles are
synthesized locally in the SDK (e.g., “synthesized locally in the SDK” or “local
meta-rule synthesis”), ensuring the page consistently treats the SDK as the
model-of-record for synthesis.
| def _compute_cache_key( | ||
| mandatory_lines: list[str], | ||
| cluster_lines: list[str], | ||
| individual_lines: list[str], | ||
| meta_block: str, | ||
| disposition_block: str, | ||
| task_type: str, | ||
| model: str, | ||
| ) -> str: | ||
| # Signature stable under wording tweaks: sort + normalize whitespace. | ||
| parts = [ | ||
| "MANDATORY:" + "|".join(sorted(mandatory_lines)), | ||
| "CLUSTER:" + "|".join(sorted(cluster_lines)), | ||
| "RULE:" + "|".join(sorted(individual_lines)), | ||
| "META:" + meta_block.strip(), | ||
| "DISP:" + disposition_block.strip(), | ||
| "TASK:" + task_type, | ||
| "MODEL:" + model, | ||
| ] | ||
| joined = "\n".join(parts).encode("utf-8") | ||
| return hashlib.sha256(joined).hexdigest()[:16] |
There was a problem hiding this comment.
Include context in the cache key.
context changes the synthesized prompt, but it is not part of _compute_cache_key(). After the first write, later calls with the same rules/task/model but different context will reuse stale <brain-wisdom> from the wrong session.
Suggested fix
def _compute_cache_key(
mandatory_lines: list[str],
cluster_lines: list[str],
individual_lines: list[str],
meta_block: str,
disposition_block: str,
task_type: str,
+ context: str,
model: str,
) -> str:
@@
parts = [
"MANDATORY:" + "|".join(sorted(mandatory_lines)),
"CLUSTER:" + "|".join(sorted(cluster_lines)),
"RULE:" + "|".join(sorted(individual_lines)),
"META:" + meta_block.strip(),
"DISP:" + disposition_block.strip(),
"TASK:" + task_type,
+ "CTX:" + context.strip(),
"MODEL:" + model,
]
@@
cache_key = _compute_cache_key(
mandatory_lines,
cluster_lines,
individual_lines,
meta_block,
disposition_block,
task_type,
+ context,
model,
)Also applies to: 317-325
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Gradata/src/gradata/enhancements/rule_synthesizer.py` around lines 74 - 94,
The cache key omits the `context` that affects prompt synthesis, causing stale
cache reuse; modify the signature of _compute_cache_key to accept an additional
context: str parameter and include a normalized context (e.g., context.strip())
as a new part (e.g., "CONTEXT:" + context.strip()) before hashing, keeping the
same hashing/truncation behavior; then update every caller of _compute_cache_key
to pass the current context value so the cache differentiates sessions.
| def _call_gemini( | ||
| model: str, system: str, user_prompt: str, max_tokens: int, timeout: float | ||
| ) -> str | None: | ||
| key = os.environ.get("GOOGLE_API_KEY") or os.environ.get("GEMINI_API_KEY") | ||
| if not key: | ||
| _log.debug("synth: GOOGLE_API_KEY / GEMINI_API_KEY not set") | ||
| return None | ||
| try: | ||
| from google import genai | ||
| from google.genai import types as genai_types | ||
|
|
||
| client = genai.Client(api_key=key) | ||
| config = genai_types.GenerateContentConfig( | ||
| system_instruction=system, | ||
| max_output_tokens=max_tokens, | ||
| ) | ||
| resp = client.models.generate_content( | ||
| model=model, | ||
| contents=user_prompt, | ||
| config=config, | ||
| ) | ||
| text = resp.text | ||
| return text.strip() if text else None | ||
| except Exception as exc: | ||
| _log.debug("synth: gemini SDK failed: %s", exc) | ||
| return None |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In the latest google-genai Python SDK, how do you configure a per-request timeout for Client.models.generate_content(), and does genai.Client(api_key=...) apply any default timeout if none is set?
💡 Result:
To set a per-request timeout for client.models.generate_content, pass HttpOptions(timeout=...) via the GenerateContentConfig for that specific call (timeout is in milliseconds): from google import genai from google.genai import types client = genai.Client(api_key='GEMINI_API_KEY') response = client.models.generate_content( model='gemini-2.0-flash', contents='Tell me a story', config=types.GenerateContentConfig( http_options=types.HttpOptions(timeout=120000) # 2 minutes ), ) Regarding genai.Client(api_key=...): there is no explicit default timeout documented/advertised in the SDK call surface. In the implementation, if a per-request timeout is not configured, the client falls back to the client-level http_options.timeout; if that is also not set, the effective timeout remains None (i.e., no request timeout is configured at the HTTP layer). So: genai.Client(api_key=...) does not apply a default timeout when none is set; you must provide http_options=types.HttpOptions(timeout=...) either on the client or per request.
Citations:
- 1: https://mintlify.com/googleapis/python-genai/guides/http-options
- 2: https://github.com/googleapis/python-genai/blob/main/google/genai/_api_client.py
- 3: https://github.com/googleapis/python-genai/blob/main/google/genai/client.py
- 4: https://googleapis-python-genai-70.mintlify.app/guides/generate-content
Gemini path does not wire the timeout parameter.
The timeout argument is accepted but never passed to the Gemini SDK. The 20s fail-safe contract only applies to the Anthropic/OpenAI branches. Wire the timeout using http_options=genai_types.HttpOptions(timeout=int(timeout * 1000)) in the GenerateContentConfig call (note: google-genai expects milliseconds, not seconds).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Gradata/src/gradata/enhancements/rule_synthesizer.py` around lines 200 - 225,
The _call_gemini function currently ignores the timeout parameter; update the
GenerateContentConfig call inside _call_gemini to pass
http_options=genai_types.HttpOptions(timeout=int(timeout * 1000)) so the Gemini
SDK receives a millisecond timeout, ensuring the same 20s fail-safe behavior as
other branches; keep using genai_types for HttpOptions and convert seconds to
milliseconds with int(timeout * 1000) when constructing the config.
| cached = _read_cache(brain_dir, cache_key) | ||
| if cached: | ||
| _log.debug("synth cache hit: %s", cache_key) | ||
| return cached |
There was a problem hiding this comment.
Validate cache hits before returning them.
A corrupt or partially-written cache file is returned as-is here, bypassing the <brain-wisdom> extraction and length checks below. That makes one bad cache entry sticky across every subsequent session.
Suggested fix
cached = _read_cache(brain_dir, cache_key)
if cached:
- _log.debug("synth cache hit: %s", cache_key)
- return cached
+ block = _extract_wisdom_block(cached)
+ if block and len(block) >= 50:
+ _log.debug("synth cache hit: %s", cache_key)
+ return block
+ _log.debug("synth cache invalid, regenerating: %s", cache_key)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Gradata/src/gradata/enhancements/rule_synthesizer.py` around lines 326 - 329,
The cached value returned from _read_cache(cache_key) must be validated before
returning to avoid sticky corrupt entries: after retrieving cached in the
rule_synthesizer logic (the block using cached, _read_cache, _log.debug, and
cache_key), run the same _extract_brain_wisdom and length/format checks that the
non-cached path uses; only return cached if those validations pass, otherwise
log a cache-miss/validation-failure and proceed with the normal
extraction/generation flow (and consider deleting or refreshing the invalid
cache entry).
| message=( | ||
| "Rule misfire rate increased. " | ||
| "New rules may be overfitting to specific sessions." | ||
| "Rule misfire rate increased. New rules may be overfitting to specific sessions." | ||
| ), |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Inconsistent message formatting across detectors.
The message at line 191 was reformatted to a single line, but all other detectors in this file (detect_being_ignored, detect_playing_safe, detect_regression_to_mean) use multi-line strings for their Alert messages. For consistency and maintainability, all four detectors should use the same formatting pattern.
♻️ Revert to multi-line format for consistency
message=(
- "Rule misfire rate increased. New rules may be overfitting to specific sessions."
+ "Rule misfire rate increased. "
+ "New rules may be overfitting to specific sessions."
),📝 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.
| message=( | |
| "Rule misfire rate increased. " | |
| "New rules may be overfitting to specific sessions." | |
| "Rule misfire rate increased. New rules may be overfitting to specific sessions." | |
| ), | |
| message=( | |
| "Rule misfire rate increased. " | |
| "New rules may be overfitting to specific sessions." | |
| ), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Gradata/src/gradata/enhancements/scoring/failure_detectors.py` around lines
190 - 192, The alert message string that was collapsed to a single line ("Rule
misfire rate increased. New rules may be overfitting to specific sessions.")
should be converted back to the same multi-line string formatting used by the
other detectors; locate the detector with that message (the rule-misfire
detector) and change its message value to the multi-line style consistent with
detect_being_ignored, detect_playing_safe, and detect_regression_to_mean so all
four detectors use the same formatting pattern.
| lines.append(f"name: {name}") | ||
| # Quote the description so colons / hashes inside don't break YAML. | ||
| safe_desc = description.replace('"', '\\"') | ||
| lines.append(f'description: "{safe_desc}"') |
There was a problem hiding this comment.
Serialize the frontmatter description instead of hand-escaping it.
Replacing only " is not enough here. A description containing a newline or a trailing backslash still produces invalid YAML frontmatter, which makes the exported SKILL.md unreadable to downstream tooling. Use a real scalar serializer here (for example, json.dumps(...), which is valid YAML for a quoted string) instead of manual escaping.
Suggested fix
+import json
...
- safe_desc = description.replace('"', '\\"')
- lines.append(f'description: "{safe_desc}"')
+ lines.append(f"description: {json.dumps(description, ensure_ascii=False)}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Gradata/src/gradata/enhancements/skill_export.py` around lines 101 - 104, The
current code in skill_export.py builds frontmatter by hand-escaping the
description (variable description -> safe_desc) and can produce invalid YAML for
newlines or trailing backslashes; replace the manual escaping and f-string
lines.append(f'description: "{safe_desc}"') with a real scalar serializer (e.g.
use json.dumps(description) or yaml.safe_dump of the single string) so the
description is correctly serialized as a YAML/quoted string; update the code
around lines.append(f"name: {name}") / safe_desc to call the serializer and
append its result (refer to the variables description, safe_desc and the list
lines).
- cloud/sync: contribute_corpus posts to /api/v1/corpus/contribute (was /v1/corpus/contribute, would 404 since backend mounts router under /api/v1) - _core: clarifying comment on _BrainStub explaining db_path may not exist for fresh brains and that downstream compute_metrics tolerates that - skill_export: hoist `import re` to module-level (deferred-import pattern is reserved for heavy optional extras per CLAUDE.md) - test_cloud_sync: update assertion to match corrected corpus path Co-Authored-By: Gradata <noreply@gradata.ai>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Gradata/src/gradata/enhancements/skill_export.py`:
- Around line 196-198: Remove redundant Path(...) wrapping where variables are
already Path-typed: change calls like _parse_rules(Path(brain_root),
lessons_path=...) and _load_meta_principles(Path(brain_root)) and any use of
Path(output_dir) inside write_skill to pass the existing Path objects directly
(e.g., _parse_rules(brain_root, ...), _load_meta_principles(brain_root), and use
output_dir as-is). Update occurrences around _parse_rules, _filter_rules,
_load_meta_principles and write_skill to eliminate the extra Path()
construction.
🪄 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: 9e9d3005-191d-43cf-a8de-cd0de3be4878
📒 Files selected for processing (4)
Gradata/src/gradata/_core.pyGradata/src/gradata/cloud/sync.pyGradata/src/gradata/enhancements/skill_export.pyGradata/tests/test_cloud_sync.py
📜 Review details
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Gradata
Repo: Gradata/gradata PR: 0
File: :0-0
Timestamp: 2026-04-17T17:18:07.439Z
Learning: In PR `#102` (gradata/gradata), Round 2 addressed: cli.py env-first brain resolution (GRADATA_BRAIN > --brain-dir > cwd), _tenant.py corrupt .tenant_id overwrite, _env_int default clamping to minimum, and _events.py tenant-scoped fallback SELECT for dedup. All ruff and 99 tests green after these fixes.
📚 Learning: 2026-04-17T17:18:07.439Z
Learnt from: Gradata
Repo: Gradata/gradata PR: 0
File: :0-0
Timestamp: 2026-04-17T17:18:07.439Z
Learning: In PR `#102` (gradata/gradata), Round 2 addressed: cli.py env-first brain resolution (GRADATA_BRAIN > --brain-dir > cwd), _tenant.py corrupt .tenant_id overwrite, _env_int default clamping to minimum, and _events.py tenant-scoped fallback SELECT for dedup. All ruff and 99 tests green after these fixes.
Applied to files:
Gradata/src/gradata/_core.py
🔇 Additional comments (9)
Gradata/src/gradata/enhancements/skill_export.py (2)
100-104: YAML escaping remains incomplete — descriptions with newlines or backslashes will produce invalid frontmatter.The previous review flagged this: only replacing
"is insufficient. A description containing\nor a trailing\will break YAML parsing for downstream tooling.Suggested fix using json.dumps
+import json ... - safe_desc = description.replace('"', '\\"') - lines.append(f'description: "{safe_desc}"') + lines.append(f"description: {json.dumps(description, ensure_ascii=False)}")
151-175: LGTM!The defensive exception handling with empty-list fallbacks is appropriate here — a fresh brain or missing DB should still produce a usable Skill without crashing. The
INJECTABLE_META_SOURCESfilter correctly excludes deterministic meta-rules per the dataclass documentation.Gradata/src/gradata/_core.py (4)
1399-1416: CORRECTION rows are not normalized —blandness_scorewill be zero on the Stop-hook path.Previous review flagged this:
data_jsoncontainsfinal_text(see line 268), but_cloud_sync_sessionat line 1294 readsc.get("final", ""). These rows need field mapping before being passed to_cloud_sync_session.Normalize the parsed dict
parsed = _json.loads(raw) if isinstance(raw, str) else raw if isinstance(parsed, dict): + # Normalize event payload shape to session-correction shape + if "final_text" in parsed and "final" not in parsed: + parsed["final"] = parsed["final_text"] session_corrections.append(parsed)
1368-1377: LGTM — lightweight stub approach avoids Brain initialization overhead.The
_BrainStubpattern is a pragmatic choice for the Stop hook. The comment clearly documents that_cloud_sync_sessiononly reads.dirand.db_path, and thetype: ignorecorrectly signals the intentional duck-typing.Also applies to: 1425-1439
1364-1366: Good improvement — surfacing cloud sync failures at WARNING level.Changing from
debugtowarningwithexc_info=Truehelps diagnose sync issues that were previously silent. This aligns with the same fix insync.pyfor HTTP errors.
34-42: Formatting changes look fine.The multi-line reformatting of dictionary literals and function arguments improves readability with no semantic changes.
Gradata/src/gradata/cloud/sync.py (3)
1-15: Module docstring still documents the old endpoint path.Previous review flagged this: line 3 says
/v1/telemetry/metricsbut the actual endpoint at line 163 is/api/v1/telemetry/metrics. This will mislead operators checking the wire protocol.Update the docstring
-Wire protocol: POST https://api.gradata.ai/v1/telemetry/metrics +Wire protocol: POST https://api.gradata.ai/api/v1/telemetry/metrics
141-151: Good improvement — distinguishing HTTP errors from network failures.Separating
HTTPError(with code/reason) fromURLError/OSErrorand logging both at WARNING level will help diagnose sync issues. The inline comment about the "last_sync never updates" bug provides useful context.
160-163: Endpoint paths correctly updated to include/api/v1prefix.The paths align with the backend router structure documented in the inline comments and are validated by the test assertions in
test_cloud_sync.py(context snippets 1 and 2).Also applies to: 179-181
| rules = _parse_rules(Path(brain_root), lessons_path=lessons_path) | ||
| rules = _filter_rules(rules, category) | ||
| metas = _load_meta_principles(Path(brain_root)) if include_meta else [] |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Redundant Path() wrapping — parameters are already typed as Path.
brain_root is typed Path in the function signature, so Path(brain_root) is unnecessary. Same for output_dir in write_skill. While harmless (Path is idempotent), it adds noise.
Remove redundant wrapping
- rules = _parse_rules(Path(brain_root), lessons_path=lessons_path)
+ rules = _parse_rules(brain_root, lessons_path=lessons_path)
rules = _filter_rules(rules, category)
- metas = _load_meta_principles(Path(brain_root)) if include_meta else []
+ metas = _load_meta_principles(brain_root) if include_meta else []
...
text = export_skill(
- Path(brain_root),
+ brain_root,
name=slug,
...
- skill_dir = Path(output_dir) / slug
+ skill_dir = output_dir / slugAlso applies to: 223-223, 230-230
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Gradata/src/gradata/enhancements/skill_export.py` around lines 196 - 198,
Remove redundant Path(...) wrapping where variables are already Path-typed:
change calls like _parse_rules(Path(brain_root), lessons_path=...) and
_load_meta_principles(Path(brain_root)) and any use of Path(output_dir) inside
write_skill to pass the existing Path objects directly (e.g.,
_parse_rules(brain_root, ...), _load_meta_principles(brain_root), and use
output_dir as-is). Update occurrences around _parse_rules, _filter_rules,
_load_meta_principles and write_skill to eliminate the extra Path()
construction.
|
Superseded by clean rebase against current main. The original branch had 38 stale commits that conflicted with #142 (cloud Phase 2). New PR contains only the 3 M1 commits cherry-picked onto current main. |
* feat(skill-export): export brain as Anthropic Claude Skill folder
New CLI: gradata skill export <name> [--output-dir DIR] [--description STR]
[--category CAT] [--no-meta]
The bet: Claude Skills' "gotchas" section is exactly what graduated
RULE-tier lessons are -- but generated from real corrections instead of
hand-written. This turns a brain into a portable, shippable Skill folder
with valid YAML frontmatter, category-grouped gotchas, and (when
available) injectable meta-principles.
- new module enhancements/skill_export.py reuses _parse_rules from
rule_export so the RULE-only filter and [hooked] marker stripping
stay consistent across exporters
- auto-generated frontmatter description lists rule categories with
defensive 900-char clip (Anthropic 1024 ceiling)
- name slugified for safe folder name + frontmatter alignment
- description quote-escapes preserve YAML validity
- meta-rule loader degrades gracefully on missing system.db / table
24 new tests; full suite 3969 pass (+24, 0 regressions).
Unblocks M4 items 7 and 9 (self-dev Skill, composition Skill) per
plans/swift-toasting-origami.md.
Co-Authored-By: Gradata <noreply@gradata.ai>
* fix(cloud-sync): correct endpoint paths + wire Stop hook to fire telemetry
Three bugs kept last_sync_at frozen:
- cloud/client.py POSTed /brains/sync (path doesn't exist) -> /sync
- cloud/sync.py POSTed /v1/telemetry/metrics -> /api/v1/telemetry/metrics
- Stop hook never fired cloud sync because Claude Code doesn't call
brain.end_session(). Added cloud_sync_tick() helper in _core.py and
new _run_cloud_sync step in session_close.py waterfall.
Also elevated silent DEBUG failures to WARNING with HTTP status +
exc_info so the next failure mode surfaces in run.log.
3945 tests pass.
Co-Authored-By: Gradata <noreply@gradata.ai>
* fix(review): address PR #143 review findings
- cloud/sync: contribute_corpus posts to /api/v1/corpus/contribute (was
/v1/corpus/contribute, would 404 since backend mounts router under /api/v1)
- _core: clarifying comment on _BrainStub explaining db_path may not exist
for fresh brains and that downstream compute_metrics tolerates that
- skill_export: hoist `import re` to module-level (deferred-import pattern
is reserved for heavy optional extras per CLAUDE.md)
- test_cloud_sync: update assertion to match corrected corpus path
Co-Authored-By: Gradata <noreply@gradata.ai>
---------
Co-authored-by: Gradata <noreply@gradata.ai>
Summary
Large batch PR bringing origin/main up to local main. Two in-scope commits on top of 39 prior commits that accumulated locally across recent sessions without being pushed.
In-scope this session (top 2 commits)
0ed95ee5feat(skill-export) — M1 Item 2 of the 9-item Skills Integration Plan. Newgradata skill exportCLI +enhancements/skill_export.pymodule. Exports the brain as an Anthropic Claude Skill folder (YAML frontmatter + SKILL.md). 24 new tests. Unblocks M4 Items 7 and 9.d2e70c80fix(cloud-sync) — Corrects endpoint paths (/brains/sync→/sync,/v1/telemetry/metrics→/api/v1/telemetry/metrics), wires Stop hook to fire telemetry via newcloud_sync_tick()helper. Elevates silent debug-swallowed exceptions to warnings.Prior commits batched in (39)
Range
5635a665..470da2f7. Themes:inject_brain_rulescluster collapse,agent_precontextmeta-rules,ctx_watchdogauto-handoff, graduation hooks, secret_scan.doctor, constraint violation logging.brain_prompt, two-provider LLM synth, P1-P3 model-agnostic transcript store, implicit_feedback text-speak support.See
git log 5635a665..470da2f7 --onelinefor full list.Test plan
gradata skill export sample-skillstdout smoke testgradata skill export "My Sales Kit" --output-dir <dir>directory smoke test (UTF-8 em-dash verified)Generated with Gradata
Co-Authored-By: Gradata noreply@gradata.ai