Skip to content

fix: P0 council bug fixes (atomic writes, BRAIN_DIR hard-fail, thread-safety lock)#145

Closed
Gradata wants to merge 52 commits intomainfrom
feat/council-phase-b-fixes
Closed

fix: P0 council bug fixes (atomic writes, BRAIN_DIR hard-fail, thread-safety lock)#145
Gradata wants to merge 52 commits intomainfrom
feat/council-phase-b-fixes

Conversation

@Gradata
Copy link
Copy Markdown
Owner

@Gradata Gradata commented Apr 30, 2026

Summary

Council reviewed Gradata overnight (2026-04-30). 4 lenses across 4 vendors (Anthropic/OpenAI/Google/Ollama) found P0 code-quality bugs in the core SDK. This PR lands all P0 fixes in two commits with full test coverage.

3970 tests pass on this branch (4:34 wall, exit 0).

What's in this PR

397f7ae6 — Phase A hygiene

  • Rewrite AGENTS.md to match the actual product (was Sprites/TypeScript boilerplate, now matches the Python SDK)
  • Add .gitignore for committed scratch (.tmp/, .archive/, sessions/handoff-*.md, files literally named 0 and BrainDetail)

016ee286 — P0-2: replace silent except: pass with logged exceptions

  • 57 files in src/gradata/ patched
  • Bare except: pass swapped for logger.warning(..., exc_info=True) — surfaces the failures the council called the "worst possible failure mode for a memory product"
  • New tests/test_no_bare_excepts.py AST regression test (allowlists optional-dep ImportError probes)
  • ImportError probes documented in allowlist

242c408d — P0-4/5/6/7: atomic writes, BRAIN_DIR hard-fail, import integrity, thread-safety lock

  • P0-5 atomic writessrc/gradata/_atomic.py with atomic_write_text() (temp-file + fsync + os.replace + dir fsync). RuleGraph.save() now uses it. Test: simulate crash mid-write, verify prior state preserved.
  • P0-4 BRAIN_DIR hard-fail — new BrainNotConfiguredError in exceptions.py. implicit_feedback hook raises instead of silently logging. gradata doctor reports it as a clear failure. Hook runner no longer suppresses it.
  • P0-7 package import integritytests/test_import_integrity.py runs python -c "import gradata; from gradata import Brain, Lesson, LessonState" in subprocess + smoke-tests Brain.init() in tmp_path.
  • P0-6 thread-safety locksrc/gradata/_brain_lock.py with acquire_brain_lock() (fcntl.flock on POSIX, msvcrt.locking on Windows). daemon.py and mcp_server.py acquire it at startup. Brain class itself does NOT auto-acquire (would break tests).

Test results

3970 passed, 5 skipped, 5 deselected, 4 warnings in 274.91s (0:04:34)
exit=0

5 new tests, all 3970 existing tests still pass. No regressions.

What's NOT in this PR

The lean-out work (council recommended cutting 40% of surface area) is on a separate branch feat/council-phase-c-leanout. That branch contains:

  • LEANOUT_PLAN.md — claude-opus's audit
  • VERIFIED_LEANOUT.md — corrected after grep verification revealed 8/14 deletion candidates were UNSAFE (have callers)

Lean-out work needs more verification before any deletion ships.

Notes for review

  • 41 unpushed commits on local main are NOT in this PR (they're independent; will catch up separately)
  • This PR branches off the unpushed local main, so it includes those 41 commits as ancestors. Reviewer may want to see only the 3 council-phase commits via git log feat/council-phase-b-fixes ^origin/main --oneline
  • Codex (gpt-5.5) generated the patches in a sandbox that couldn't write to .git/. Each commit was manually applied from working-tree changes by the orchestrator.

🤖 Generated with Hermes Agent + Council orchestration

Gradata and others added 30 commits April 20, 2026 15:16
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>
Gradata and others added 10 commits April 24, 2026 06:47
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>
…ignore scratch dirs

P0-1: AGENTS.md previously described 'Sprites Work multi-agent
TypeScript/Claude Flow framework' which is unrelated to this Python SDK.
Council unanimously flagged as credibility-killer for first-time evaluators.
Replaced with accurate guidance for AGENTS.md-aware coding agents.

P0-3 (partial): added .tmp/, .archive/, sessions/handoff-*.md, /0,
/BrainDetail to .gitignore so scratch artifacts stop getting committed.
Existing tracked scratch files left in place — destructive removal
deferred for user review.
P0-2 from council review: silent except-pass blocks in core SDK swallowed
errors during the headline correction-write path.

- Added tests/test_no_bare_excepts.py: AST scan that fails CI if any
  single-statement except-pass returns to src/gradata/, except for
  documented optional-dep ImportError probes (allowlist).
- Converted 57 non-allowlisted handlers to logger.warning(..., exc_info=True).
- Added logger = logging.getLogger(__name__) where files lacked one.
- Codex (gpt-5.5) generated the patch under workspace-write sandbox;
  could not self-commit because .git/ lives outside its writable root.
  This commit lifts the patch into the repo.

Verification:
- python3 exec of test_no_bare_excepts.py: passes
- python3 -m compileall src/gradata: no syntax errors

NOT in this commit (planned next): P0-5 atomic rule_graph writes,
P0-4 BRAIN_DIR hard-fail, P0-7 import integrity, P0-6 thread-safety lock.
…rity, thread-safety lock

P0-5 atomic writes (rule_graph.json):
- New src/gradata/_atomic.py: atomic_write_text() (temp file + fsync + os.replace + dir fsync on POSIX)
- RuleGraph.save() now uses atomic_write_text — crash mid-write preserves prior state
- Test: test_rule_graph_atomic.py::test_rule_graph_save_preserves_prior_state_when_replace_fails

P0-4 BRAIN_DIR hard-fail (no more silent data loss):
- exceptions.py: new GradataError base + BrainNotConfiguredError(GradataError)
- implicit_feedback hook: raises BrainNotConfiguredError instead of swallowing
- hooks/_base.py: hook runner no longer suppresses BrainNotConfiguredError
- _doctor.py: reports missing BRAIN_DIR as clear brain_dir failure
- Test: test_brain_dir_required.py (3 cases)

P0-7 package import integrity:
- Test: test_import_integrity.py — subprocess: import gradata; from gradata import Brain, Lesson, LessonState
- Plus Brain.init() smoke test in tmp_path

P0-6 thread-safety lock (Brain documented NOT thread-safe yet ships daemon + mcp_server):
- New src/gradata/_brain_lock.py: acquire_brain_lock(brain_dir) context mgr
  - POSIX: fcntl.flock LOCK_EX|LOCK_NB on BRAIN_DIR/.brain.lock
  - Windows: msvcrt.locking
  - In-process duplicate-lock guard
- BrainLockedError in exceptions.py
- daemon.py: acquires lock at start, releases in cleanup
- mcp_server.py: acquires for run_server() duration
- Brain itself does NOT auto-acquire (would break tests)
- Test: test_brain_lock.py::test_second_mock_daemon_raises_brain_locked

CHANGES.md and REPORT.md committed for posterity.
Codex (gpt-5.5) generated all four patches; could not self-commit because .git/
is outside its sandbox writable root.
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 30, 2026

Too many files changed for review. (262 files found, 100 file limit)

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 30, 2026

📝 Walkthrough
  • Core P0 fixes

    • Atomic file writes: new atomic_write_text() ensures safe replaces (temp-file + fsync + os.replace + dir fsync) and RuleGraph.save() uses it; tests simulate mid-write crash to verify prior-state preservation.
    • BRAIN_DIR hard-fail: unresolved brain directory now raises BrainNotConfiguredError (implicit_feedback hook raises; hook runner and gradata doctor surface the error).
    • Cross-process brain lock: acquire_brain_lock() provides POSIX/Windows exclusive lock, with BrainLockedError on contention; daemon and mcp_server acquire the lock at startup.
  • Observability / robustness

    • Replaced >50 bare except: pass sites with logger.warning(..., exc_info=True); added AST regression test tests/test_no_bare_excepts.py and documented allowed ImportError probes.
    • Many modules gained module-level loggers and non-fatal warnings where exceptions were previously swallowed.
  • New modules / features

    • src/gradata/_atomic.py (atomic_write_text), src/gradata/_brain_lock.py (acquire_brain_lock), transcript subsystem (log_turn, load_turns, cleanup_ttl), synthesizer and caching for rule synthesis (synthesize_rules_block), cloud sync tick helper (cloud_sync_tick).
    • Import-integrity test + subprocess smoke-tests and migrations for event identity and sync state (002/003).
  • Public API / CLI changes (new or re-exported)

    • New/reauthorized public exceptions: BrainLockedError, BrainNotConfiguredError, GradataError (re-exported).
    • New public functions: atomic_write_text(), acquire_brain_lock(), synthesize_rules_block(), cloud_sync_tick(), log_turn(), load_turns(), cleanup_ttl().
    • CLI: new gradata skill export command; gradata doctor gains --cloud / --no-cloud flags.
  • Learning-loop / architecture changes

    • Meta-rule discovery moved local: discover_meta_rules() and merge_into_meta() run locally (greedy clustering + deterministic principle generation or LLM-backed synthesis).
    • Documentation updated to frame Cloud as optional visualization/backup only; graduation/synthesis executed locally.
  • Data / migrations & DB

    • New migrations: 002_add_event_identity (event_id/device_id/content_hash backfill) and 003_add_sync_state (sync_state table additions).
    • Supabase migrations to add UNIQUE constraints and brains.last_used_at (idempotent checks included).
  • Tests & CI

    • Test run: 3970 passed, 5 skipped, 5 deselected; 5 new tests added (including AST bare-except check and import-integrity smoke tests).
  • Breaking changes / behavior changes to call out

    • implicit_feedback hook and related code now hard-fail when BRAIN_DIR is unset (raises BrainNotConfiguredError) instead of silently logging.
    • implicit_feedback contract standardized to return None and emit IMPLICIT_FEEDBACK events (behavioral/operational semantics changed).
  • Security / privacy adjustments

    • Raw event logging gated by GRADATA_DEBUG_RAW_EVENTS and now emits sha256 prefixes instead of raw task text by default to reduce PII exposure.

Walkthrough

Implements local meta-rule discovery/synthesis, atomic file writes, cross-process brain locking, transcript logging, device/tenant migrations and cloud sync schema logic, expanded hooks/config, CLI skill export, extensive logging (replacing silent excepts), many formatting refactors, and new/updated docs and .gitignore entries.

Changes

Cohort / File(s) Summary
Atomic writes & process locking
Gradata/src/gradata/_atomic.py, Gradata/src/gradata/_brain_lock.py
Add atomic_write_text with fsync and cross-process brain lock (fcntl/msvcrt) to prevent concurrent daemon runs.
Migrations & device/tenant utilities
Gradata/src/gradata/_migrations/002_add_event_identity.py, .../003_add_sync_state.py, .../_ulid.py, .../device_uuid.py, Gradata/migrations/supabase/...
New SQLite migrations (event identity, sync_state), ULID utilities, device-id file handling, and Supabase idempotent SQL migrations (dedupe/unique constraints, last_used_at).
Cloud sync & client
Gradata/src/gradata/_cloud_sync.py, Gradata/src/gradata/cloud/client.py, Gradata/src/gradata/cloud/sync.py
Schema reshaping, payload scrubbing, batch deduplication, error tracking/watermark semantics; client endpoint/path updates and stricter HTTP error logging.
Meta-rules & rule synthesis
Gradata/src/gradata/enhancements/meta_rules.py, Gradata/src/gradata/enhancements/rule_synthesizer.py
Implement local meta-rule discovery/clustering and deterministic principle generation; new LLM-backed rule_synthesizer with per-brain cache and provider routing.
Session transcripts & providers
Gradata/src/gradata/_transcript.py, Gradata/src/gradata/_transcript_providers.py
Session transcript logging (env-gated), TTL cleanup, and transcript source adapters (Claude Code JSONL + local).
Core/daemon/doctor/CLI
Gradata/src/gradata/_core.py, Gradata/src/gradata/daemon.py, Gradata/src/gradata/_doctor.py, Gradata/src/gradata/cli.py
Add cloud_sync_tick; acquire/release brain lock in daemon start/cleanup; doctor gains include_cloud/cloud_only flags and cloud probes; CLI adds doctor flags and skill export subcommand.
Events & PII handling
Gradata/src/gradata/_events.py
Persist redacted event data (email redaction), optional raw side-log when enabled, and log suppressed exceptions instead of silencing.
Hooks & session-start skill
Gradata/hooks/hooks.json, Gradata/skills/core/session-start/SKILL.md
Add ctx_watchdog command hook and extend session_close timeout; new session-start skill doc defining startup orchestration and on-demand loading.
Rule pipeline & promotion
Gradata/src/gradata/enhancements/rule_pipeline.py, Gradata/src/gradata/enhancements/rule_to_hook.py
Phase‑2 now calls canonical graduate promoter; added logging for previously suppressed failures; improved rule→hook emission logging.
Transcript/archived dashboard & docs
Gradata/.archive/dashboard_streamlit_deprecated_2026-04-23.py, Gradata/docs/..., Gradata/CHANGELOG.md, Gradata/CHANGES.md, Gradata/REPORT.md, Gradata/AGENTS.md
Archive deprecated Streamlit dashboard; extensive documentation updates clarifying local-first graduation and cloud as mirror; changelogs and contributor agent guidance.
.gitignore updates
.gitignore, Gradata/.gitignore
Tighten ignore rules for Graphify artifacts, run logs, .archive/, and additional literal patterns.
Transcript synthesis & rule tooling
Gradata/src/gradata/enhancements/* (rule_synthesizer, meta_rules_storage, many enhancement modules)
New synthesizer, meta-rule storage refactors, and many enhancement modules receive logging/formatting updates.
Widespread logging & error visibility
many src/gradata/* modules (see summary)
Replace numerous silent except: pass sites with logger.warning/debug and add module-level loggers to improve observability; majority of remaining edits are formatting-only.
Public exports
Gradata/src/gradata/__init__.py
Re-export GradataError, BrainNotConfiguredError, BrainLockedError to top-level API.

Sequence Diagram(s)

sequenceDiagram
  participant Daemon
  participant BrainDB as "Brain DB (SQLite)"
  participant Synth as "LLM / Synthesizer"
  participant Cloud as "Cloud API (Supabase/PostgREST)"

  Daemon->>BrainDB: read lessons & correction rows
  Daemon->>Synth: request local meta-rule synthesis (discover_meta_rules / synthesize)
  Synth-->>Daemon: return meta-rules / <brain-wisdom> block (or None)
  Daemon->>BrainDB: persist meta-rules / updated lessons
  Daemon->>Cloud: push batched transformed rows (scrub JSONB, remap tables)
  Cloud-->>Daemon: success / constraint error (409)
  alt success for full table
    Daemon->>BrainDB: advance sync watermark
  else partial/failure
    Daemon->>BrainDB: record cloud_push_error.json
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • PR #142 - Implements cloud sync surface, migrations (002/003), device/tenant utilities and sync-state handling; directly overlaps cloud/migration work.
  • PR #97 - Modifies/enhances meta_rules (local synthesis and principle generation); directly related to meta-rule changes.
  • PR #102 - Multi-tenant/cloud alignment (tenant_id handling, sync, Supabase schema) matching migrations and sync design here.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/council-phase-b-fixes

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 39

Caution

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

⚠️ Outside diff range comments (13)
Gradata/src/gradata/contrib/patterns/rag.py (1)

319-449: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

two_pass is dead code here.

cascade_retrieve() returns from the hybrid path whenever all_results is non-empty, so execution never reaches the two-pass expansion block. As written, two_pass=True has no effect. Please move the expansion logic before the hybrid return or gate that return when expansion is enabled.

💡 Suggested fix
-    if all_results:
-        merged = rrf_merge(*all_results, k=cfg.hybrid_rrf_k)
-        graduated = apply_graduation_scoring(merged, cfg)
-        result = RetrievalResult(
-            chunks=graduated[:limit],
-            query=query,
-            mode="hybrid",
-            total_candidates=total,
-        )
-        if _cascade_errors:
-            result.mode = f"hybrid (warnings: {', '.join(_cascade_errors)})"
-        return result
-
-    # Stage 4: Two-pass query expansion (if enabled and we got SOME results)
-    if cfg.two_pass and all_results:
+    # Stage 4: Two-pass query expansion (if enabled and we got SOME results)
+    if cfg.two_pass and all_results:
         flat_chunks = [c for sublist in all_results for c in sublist]
         expansion_terms = extract_expansion_terms(flat_chunks, query, top_k=cfg.two_pass_top_k)
         if expansion_terms:
             expanded_query = query + " " + " ".join(expansion_terms)
             ...
+
+    if all_results:
+        merged = rrf_merge(*all_results, k=cfg.hybrid_rrf_k)
+        graduated = apply_graduation_scoring(merged, cfg)
+        result = RetrievalResult(
+            chunks=graduated[:limit],
+            query=query,
+            mode="hybrid",
+            total_candidates=total,
+        )
+        if _cascade_errors:
+            result.mode = f"hybrid (warnings: {', '.join(_cascade_errors)})"
+        return result
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Gradata/src/gradata/contrib/patterns/rag.py` around lines 319 - 449,
cascade_retrieve currently returns the hybrid result as soon as all_results is
non-empty, making CascadeConfig.two_pass ineffective; to fix, change the control
flow so the two-pass expansion (using extract_expansion_terms, calling
fts_fn/vector_fn for pass2, merging with rrf_merge and rescoring with
apply_graduation_scoring) is executed before returning when cfg.two_pass is true
— either move the entire two_pass block to run immediately after building
all_results (before the current hybrid return) or condition the hybrid return on
not cfg.two_pass (i.e., if all_results and not cfg.two_pass: build merged/result
and return; otherwise run two_pass logic, merge pass1+pass2, then score and
return); ensure you still populate result.mode with any _cascade_errors and
compute total_candidates correctly using total and pass2_total.
Gradata/src/gradata/enhancements/graduation/agent_graduation.py (1)

623-630: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Unreadable scope_json currently widens a scoped lesson into a global one.

Both paths only log parse failures and then keep processing, so an invalid scoped lesson is treated as universal. In get_agent_rules() that injects the wrong guidance, and in get_deterministic_rules() it can enforce a blocking rule on unrelated tasks. Once task_type filtering is active, parse failures should fail closed by skipping that lesson/rule.

Also applies to: 854-860

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

In `@Gradata/src/gradata/enhancements/graduation/agent_graduation.py` around lines
623 - 630, The JSON parse failure for lesson.scope_json currently only logs and
lets the lesson fall through as global; update both
AgentGraduationTracker.get_agent_rules and get_deterministic_rules so that when
json.loads(lesson.scope_json) raises json.JSONDecodeError or TypeError you skip
that lesson/rule (e.g., replace the except block that logs with one that logs
and then continue/skip) to fail-closed when task_type filtering is active;
ensure you still log exc_info=True for diagnostics and reference the same lesson
when skipping.
Gradata/src/gradata/enhancements/rule_context_bridge.py (2)

100-107: ⚠️ Potential issue | 🔴 Critical

Fix sqlite3.Row access on line 103—.get() is not supported and will raise AttributeError.

sqlite3.Row supports index and key access via [] and the in operator, but not .get(). The code at line 103 calls row.get("category", "META"), which will raise AttributeError when processing the first meta-rule row, causing the bootstrap path to fail. The try-except catches (KeyError, IndexError) but not AttributeError, so the error will propagate.

Use explicit key lookup instead:

Proposed fix
-                try:
-                    category = row.get("category", "META")
-                except (KeyError, IndexError):
-                    category = "META"
+                category = row["category"] if "category" in row else "META"

Line 105 (principle = row["principle"] if "principle" in row else str(row)) is already correct.

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

In `@Gradata/src/gradata/enhancements/rule_context_bridge.py` around lines 100 -
107, Replace the unsupported row.get usage by explicitly checking membership on
the sqlite3.Row and avoid relying on .get; change the category extraction to use
row["category"] if "category" in row else "META" (and remove or broaden the
try/except that only caught KeyError/IndexError so AttributeError won't
propagate), leaving the existing principle assignment (principle =
row["principle"] if "principle" in row else str(row)) as-is; update any
surrounding try/except to catch AttributeError if you choose to keep it.

90-95: ⚠️ Potential issue | 🟠 Major

Use a context manager to ensure the WAL checkpoint connection always closes.

If PRAGMA wal_checkpoint(TRUNCATE) raises, c2.close() is skipped and the connection hangs open, potentially leaving a file lock on Windows. Use with to guarantee cleanup on both success and exception paths.

Proposed fix
                 try:
-                    c2 = sqlite3.connect(str(db_path))
-                    c2.execute("PRAGMA wal_checkpoint(TRUNCATE)")
-                    c2.close()
+                    with sqlite3.connect(str(db_path)) as c2:
+                        c2.execute("PRAGMA wal_checkpoint(TRUNCATE)")
                 except Exception:
-                    logger.warning('Suppressed exception in bootstrap_rule_context', exc_info=True)
+                    logger.warning(
+                        "bootstrap_rule_context wal checkpoint failed",
+                        exc_info=True,
+                    )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Gradata/src/gradata/enhancements/rule_context_bridge.py` around lines 90 -
95, The try block in bootstrap_rule_context currently opens a sqlite3 connection
via c2 = sqlite3.connect(str(db_path)) then calls c2.execute(...) and c2.close()
in the success path, which will skip closing on exceptions; replace this pattern
to use a context manager (with sqlite3.connect(str(db_path)) as c2:) so the
connection is always closed even if PRAGMA wal_checkpoint(TRUNCATE) raises, and
keep the existing except logger.warning(..., exc_info=True) behavior around the
with block.
Gradata/src/gradata/contrib/patterns/human_loop.py (1)

494-507: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Thread context through HumanLoopGate.check().

check() currently calls gate(action) and never uses the caller-supplied context, so overrides and target metadata are ignored and the approval decision can 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)
-        if request is None:
+        risk = assess_risk(action, context)
+        if risk.tier == "low":
             return ApprovalResult(approved=True, feedback="auto_approved_low_risk")
+        request = ApprovalRequest(
+            action=action,
+            risk=risk,
+            preview=preview_action(action, context),
+        )
         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 - 507,
The check method in HumanLoopGate ignores the caller-supplied context by calling
gate(action) without it; change the call to pass the context through (e.g., call
gate(action, context) or gate(action, context=context) depending on gate's
signature) so overrides and target metadata are considered, and ensure any
approver invocation (return approver(request)) still works with the request
produced when context is supplied. Update only the gate invocation in
HumanLoopGate.check (and adjust gate's callable type if needed) to thread the
context argument.
Gradata/src/gradata/_context_packet.py (1)

145-163: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Guard the empty prospect-name path.

prospect_lower.split()[0] raises IndexError for empty or whitespace-only input, so a simple lookup miss can now drop the rest of _load_prospect_context().

🛠️ Proposed fix
     try:
         all_events = _events_query(limit=50)
         prospect_lower = prospect_name.lower()
+        prospect_tokens = prospect_lower.split()
         interactions = []
         for e in all_events:
             tags = e.get("tags", [])
             tag_str = " ".join(tags).lower()
-            if prospect_lower.split()[0] in tag_str:
+            if prospect_tokens and prospect_tokens[0] in tag_str:
                 interactions.append(
🤖 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 145 - 163, The code uses
prospect_lower.split()[0] which will raise IndexError for empty or
whitespace-only prospect_name; update the logic in _load_prospect_context (the
block that calls _events_query and builds result["recent_interactions"]) to
first guard prospect_name by checking prospect_name (or prospect_lower) is
non-empty after strip() and only then perform the split/lookup, otherwise skip
matching and set result["recent_interactions"] to an empty list (or leave
interactions empty); ensure this check surrounds the tag-matching loop that uses
prospect_lower.split()[0] and references _events_query, interactions, and result
so no IndexError can propagate.
Gradata/src/gradata/enhancements/router_warmstart.py (1)

75-124: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Close the SQLite connection on every failure path.

conn.close() only runs after fetchall(). If the query raises, the exception is logged and the connection is left open, which can leak descriptors and keep the DB locked. Please wrap the connection in a context manager or move the close into finally.

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

In `@Gradata/src/gradata/enhancements/router_warmstart.py` around lines 75 - 124,
The SQLite connection opened via sqlite3.connect (conn) may remain open if the
query or processing raises; change the logic to always close the connection by
wrapping the connection in a context manager (use "with
sqlite3.connect(str(db_path)) as conn:" and keep conn.row_factory assignment
inside) or move conn.close() into a finally block so conn.close() is executed on
all paths; keep the same query and subsequent router routing/reward/update logic
(router.route, router.reward_from_severity, router.update_reward, router.save)
but ensure the connection is reliably closed even on exceptions.
Gradata/src/gradata/cloud/client.py (1)

29-51: ⚠️ Potential issue | 🟠 Major

Endpoint configuration creates silent routing failures for custom deployments.

CloudClient in client.py bakes /api/v1 into DEFAULT_ENDPOINT, but GRADATA_ENDPOINT overrides the entire base URL. Any deployment setting GRADATA_ENDPOINT to a host root (e.g., https://example.com) will silently fail because routes like /brains/connect and /sync are appended directly, producing https://example.com/brains/connect instead of https://example.com/api/v1/brains/connect. The test at line 301 even demonstrates this gap—it accepts endpoint="https://api.gradata.ai" without /api/v1 and passes, but calling connect() or sync() with that endpoint would hit the wrong routes.

Either enforce that custom endpoints include /api/v1 at the composition point (line 49), or update _post() to normalize the base URL once so that the version prefix is never duplicated or omitted.

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

In `@Gradata/src/gradata/cloud/client.py` around lines 29 - 51, CloudClient
currently stores the endpoint from DEFAULT_ENDPOINT or ENV_ENDPOINT as-is,
causing routes like connect() and sync() to miss the required "/api/v1" prefix;
normalize the base URL in CloudClient.__init__ so self.endpoint always ends with
"/api/v1" (trim trailing slashes first, then append "/api/v1" only if it's not
already present) so that _post(), connect(), sync() and any other methods build
correct routes; reference CloudClient, __init__, DEFAULT_ENDPOINT, ENV_ENDPOINT,
and methods like _post(), connect(), sync() when making the change.
Gradata/src/gradata/cloud/sync.py (1)

170-180: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Align the corpus endpoint with the new /api/v1 routing.

sync_metrics() now targets the /api/v1 prefix, but contribute_corpus() still posts to /v1/corpus/contribute. If the backend has moved under the same prefix, this separate opt-in flow will keep failing with 404s.

Suggested fix
-        result = self._post("/v1/corpus/contribute", {"patterns": anonymized_patterns})
+        result = self._post("/api/v1/corpus/contribute", {"patterns": anonymized_patterns})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Gradata/src/gradata/cloud/sync.py` around lines 170 - 180, contribute_corpus
currently posts to "/v1/corpus/contribute" which is inconsistent with the newer
"/api/v1" routing used elsewhere (e.g. sync_metrics); update the endpoint in
contribute_corpus to use the "/api/v1/corpus/contribute" path and ensure the
call to self._post still sends {"patterns": anonymized_patterns} and returns
result is not None so the behavior remains unchanged besides the corrected URL.
Gradata/docs/cloud/overview.md (1)

3-24: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clarify that synthesis is SDK-local only.

The surrounding prose says Cloud mirrors local results, but the capability table still marks meta-rule synthesis as a Cloud capability. Reword that row so readers don't assume the hosted tier runs the LLM work itself.

Also applies to: 53-56

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

In `@Gradata/docs/cloud/overview.md` around lines 3 - 24, The table incorrectly
implies Cloud performs meta-rule synthesis; update the Meta-rule **synthesis**
row (the line with "Meta-rule **synthesis** (local LLM via your own key or
Claude Code Max OAuth)") so the Open-source SDK column remains "Yes" but the
Gradata Cloud column clearly indicates "No — runs locally only (via the user's
LLM/key); Cloud only visualizes/orchestrates results" and adjust the surrounding
prose sentence that mentions synthesis to state synthesis is SDK-local only;
apply the same edit to the duplicate instance of that row elsewhere in the
document.
Gradata/src/gradata/brain.py (1)

656-675: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Keep the lessons.md write atomic.

lessons_lock only serializes writers; it does not protect against a crash leaving a truncated or half-written file on disk. Please use the same atomic-write path used elsewhere instead of writing the final file directly.

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

In `@Gradata/src/gradata/brain.py` around lines 656 - 675, The code currently
calls lessons_path.write_text(format_lessons(existing), ...) inside lessons_lock
which can leave a truncated file on crash; instead perform an atomic replace
using the same serializer: write format_lessons(existing) to a temporary file
(e.g., lessons_path.with_suffix(".tmp") or tempfile.NamedTemporaryFile in the
same directory), fsync the temp file, then atomically replace the original with
os.replace(temp_path, lessons_path) while still under lessons_lock (or call the
existing project helper for atomic writes if one exists). Keep using
format_lessons and parse_lessons as before and preserve encoding="utf-8".
Gradata/src/gradata/_doctor.py (2)

86-108: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Honor the explicit/resolved brain path when reporting brain_dir.

diagnose(brain_dir=...) resolves a valid brain_path, but _check_brain_dir() still re-reads only BRAIN_DIR. A caller using --brain-dir can therefore pass all filesystem checks and still end up with a failing brain_dir status solely because the env var is unset.

Also applies to: 476-515

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

In `@Gradata/src/gradata/_doctor.py` around lines 86 - 108, The _check_brain_dir
function should honor an explicitly resolved brain path instead of always
reading BRAIN_DIR from the environment; change _check_brain_dir to accept an
optional parameter (e.g., brain_path) and use that when provided (falling back
to os.environ.get("BRAIN_DIR") only if brain_path is None), then update
diagnose(...) to pass the resolved brain_path/brain_dir into _check_brain_dir so
callers using the --brain-dir flag get the correct status; ensure the function
still returns the same dict shape ("name", "status", "detail") and retains the
existing checks (is_dir, exists but not dir, not exists).

464-524: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Optional cloud probes should not turn a healthy local install into broken.

With include_cloud=True by default, _cloud_checks() always runs reachability/auth/data probes, and diagnose() treats any resulting fail as globally broken. That means an offline or local-only user can get BROKEN even when every local check passed. Cloud-not-configured/offline cases need to stay skip/warn, or be gated behind a confirmed cloud credential/config.

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

In `@Gradata/src/gradata/_doctor.py` around lines 464 - 524, The cloud probes
currently run unconditionally and any "fail" from those checks can mark the
whole diagnose() result as "broken"; change the logic so optional cloud checks
don't downgrade a healthy local install: in either _cloud_checks or in
diagnose() gate running
_check_cloud_reachable/_check_cloud_auth/_check_cloud_has_data/_check_cloud_push_error
behind a confirmed cloud configuration by first evaluating _check_cloud_config()
and _check_cloud_env_vars() (or use their combined result) and only include the
deeper probes if config/env indicate cloud is configured; alternatively, after
collecting cloud checks, if _check_cloud_config or _check_cloud_env_vars returns
"skip"/"missing", map any subsequent cloud check statuses to "skip" (or "warn")
before computing overall status so cloud failures cannot mark the whole report
"broken".
🤖 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 1-27: Remove the archived dashboard artifact
Gradata/.archive/dashboard_streamlit_deprecated_2026-04-23.py from the PR by
deleting it from the commit (git rm --cached or remove and recommit) and ensure
similar personal/scratch files under .archive/ or with local absolute paths are
not reintroduced by adding the pattern (.archive/, *.tmp, sessions/handoff-*.md,
files named "0" or "BrainDetail") to .gitignore; if you need to keep the file
for personal use, move it to a private/internal repo instead of tracking it
here.

In `@Gradata/AGENTS.md`:
- Around line 80-85: Several fenced code blocks in AGENTS.md lack a language
identifier; update each triple-backtick fence that precedes the blocks starting
with "Layer 2 — Public API        brain.py, cli.py, daemon.py, mcp_server.py",
"INSTINCT → PATTERN → RULE → META_RULE", and "<type>(<scope>): <imperative
description>" to include a language tag (use "text" as suggested) so the opening
``` becomes ```text, and apply the same change for the other two similar fenced
blocks noted later in the file (the blocks around the same examples) to
eliminate the MD040 markdownlint warnings.

In `@Gradata/CHANGELOG.md`:
- Around line 3-79: The Unreleased changelog publishes unrelated work; edit
Gradata/CHANGELOG.md under the "## [Unreleased] — post-0.6.0 (2026-04-23 →
2026-04-24)" block to only include changes actually introduced by this PR:
remove the "33 commits ahead of public `origin/main`" line and any entries
referencing unrelated production changes such as "Supabase migrations
014/015/016", "Streamlit dashboard deprecated", and other unrelated bullets
(e.g., prod-only schema hardening, dashboard deprecation, or stale test totals).
Keep or add only the Added/Changed/Fixed/Test/Security bullets that correspond
to this PR's diffs (look for items like `_cloud_sync.py`, `003_add_sync_state`,
`ctx_watchdog`, and `implicit_feedback` only if they are part of the PR), and
update counts/headers accordingly so the Unreleased section reflects this PR
alone.

In `@Gradata/docs/concepts/meta-rules.md`:
- Around line 47-50: Replace the indented markdown sample inside the "Local by
default" info callout with a fenced code block to satisfy markdownlint rule
MD046; locate the callout titled "Local by default" in meta-rules.md and change
the indented paragraph sample (the multi-line description under the callout) to
use triple backticks (```) fenced code block syntax so the rendered output stays
the same but avoids the MD046 warning.

In `@Gradata/docs/LEGACY_CLEANUP.md`:
- Line 16: The markdown headings like "Deprecated adapter shims (scheduled
v0.8.0)" (and the other headers at the noted locations) are missing the required
blank line above them and trigger MD022; add a single blank line immediately
before each of those heading lines so each header is separated from the previous
content (ensure the same fix is applied for the other headers referenced: the
ones at lines corresponding to the other headings), which will make the section
headers lint-clean.

In `@Gradata/docs/specs/cloud-sync-and-pricing.md`:
- Line 35: The markdown in cloud-sync-and-pricing.md has markdownlint
violations: ensure there is a blank line immediately before and after every
table (e.g., the table starting with "| Scope | Can read | Can write | Notes |")
and before/after every fenced code block, and add appropriate language
identifiers to fenced code blocks (e.g., ```yaml, ```bash, or ```json as
appropriate) so the linter stops complaining; update the table blocks and the
fenced code blocks referenced around lines with the table header and the code
fences at the other noted locations (60, 81, 95, 109) to include these blank
lines and language tags.

In `@Gradata/migrations/supabase/014_corrections_unique.sql`:
- Around line 23-34: The uniqueness guard currently uses "c.conkey @>
ARRAY[...]" which matches supersets; update the condition so it only treats an
existing constraint as present when the constraint's key matches the exact
three-column key for table "corrections" (columns brain_id, session,
description). Replace the superset check with an exact-array comparison (e.g.
use "c.conkey = ARRAY[ (SELECT attnum ... 'brain_id'), (SELECT attnum ...
'session'), (SELECT attnum ... 'description') ]::smallint[]") or, if
order-independence is required, require both "@>" and "<@" (c.conkey @>
ARRAY[...] AND c.conkey <@ ARRAY[...]) so only an exact 3-column match
short-circuits the migration.

In `@Gradata/migrations/supabase/015_events_unique.sql`:
- Around line 23-34: The IF NOT EXISTS guard is too permissive because it uses
c.conkey @> ARRAY[...] which matches constraints that include these columns plus
others; change that predicate to require exact equality by replacing c.conkey @>
ARRAY[...]::smallint[] with c.conkey = ARRAY[
(SELECT attnum FROM pg_attribute WHERE attrelid = t.oid AND attname =
'brain_id'),
  (SELECT attnum FROM pg_attribute WHERE attrelid = t.oid AND attname = 'type'),
(SELECT attnum FROM pg_attribute WHERE attrelid = t.oid AND attname =
'created_at')
]::smallint[] so the IF NOT EXISTS only skips when a UNIQUE constraint exactly
on (brain_id, type, created_at) already exists.

In `@Gradata/migrations/supabase/README.md`:
- Around line 32-37: The fenced code block showing 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 tag to satisfy
markdownlint; update the opening fence from ``` to ```sql so the block is marked
as SQL and the README lint (MD040) passes.

In `@Gradata/REPORT.md`:
- Around line 5-13: Remove or update the stale status in REPORT.md that states
P0-2 changes were uncommitted (mentions "P0-2", branch
"feat/council-phase-b-fixes" and the git index.lock error) so it no longer
misrepresents the current PR; either delete that paragraph or replace it with a
current summary that reflects the committed fixes, and make the same change for
the duplicate section covering lines 54-80 to ensure the report is accurate
before merge.

In `@Gradata/skills/core/session-start/SKILL.md`:
- Around line 32-36: The fenced code block in SKILL.md is missing a language tag
(markdownlint MD040); update the triple-backtick block that contains the lines
beginning with "[check] S[N] loaded ..." to include a language identifier (e.g.,
```text) so the block becomes ```text ... ```, preserving the exact block
contents and spacing.
- Around line 12-21: SKILL.md embeds user-specific absolute paths (e.g.,
continuation.md, continuation.py archive, lessons.md, loop-state.md,
brain_prompt.md, startup-brief.md) which makes the skill non-portable; replace
each hard-coded absolute path with a brain-relative placeholder (e.g.,
{BRAIN_ROOT} or an env var like BRAIN_ROOT) and update any commands (the
"continuation.py archive" invocation) to use a runtime resolver that joins the
placeholder with the file names (use Path/Pathlib or os.path.join in the
consuming code/snippets), remove the original user-identifying literals from the
file, and add a short note in the skill explaining how BRAIN_ROOT is
set/resolved at runtime.

In `@Gradata/src/gradata/_atomic.py`:
- Around line 13-21: The temp filename used for atomic writes (created via
target.with_name(f".{target.name}.{os.getpid()}.tmp")) is not unique per write
and can collide within the same process; change the temp-name generation in the
atomic write routine (the tmp variable creation near target.with_name and the
subsequent write/replace/_fsync_dir code) to include a per-write unique token
(e.g., uuid.uuid4() or tempfile-generated name in target.parent) so each writer
gets a distinct temp file, then continue to write, fsync, os.replace and
_fsync_dir as before and ensure any temporary file cleanup on error.

In `@Gradata/src/gradata/_brain_manifest.py`:
- Around line 64-77: The DB cross-check currently swallows all exceptions and
only closes the connection on the success path; change the code that calls
get_connection(db) (using ctx/db_path and updating
version_info["sessions_trained"]) to ensure conn is always closed by using a
context manager (with get_connection(db) as conn) or an explicit try/finally
that calls conn.close() in the finally block, and replace the debug-only except
with either a narrower exception type or log at warning level using
_log.warning(..., exc_info=True) so failures are visible and include the
exception info; keep the existing logic that updates
version_info["sessions_trained"] when db_max > current.

In `@Gradata/src/gradata/_cloud_sync.py`:
- Around line 144-149: The current try/except around int(session_raw) in the
cloud event transform silently truncates non-integer floats and loses precision;
change the logic so you only coerce to an int when session_raw is already an int
or an integer-representing string, otherwise preserve the original value (e.g.,
leave session_int as None and keep session_raw in data_blob). Concretely,
replace the blind int(session_raw) conversion with explicit checks: if
isinstance(session_raw, int) set session_int = session_raw; elif
isinstance(session_raw, str) and session_raw.lstrip("+-").isdigit() then set
session_int = int(session_raw); else set session_int = None and ensure
data_blob["session_raw"] = session_raw is set when session_raw exists (so floats
like 4.5 or UUIDs are not truncated). Apply this change where
session_raw/session_int/data_blob are handled in the transform routine.

In `@Gradata/src/gradata/_context_compile.py`:
- Around line 94-96: The fallback exception log in _context_compile.py currently
calls _log.debug("Fallback keyword search failed (non-fatal): %s", e) which only
records the exception string; update the except block that contains this
_log.debug call to include the full traceback by passing exc_info=True (e.g.
_log.debug("Fallback keyword search failed (non-fatal)", exc_info=True)) so the
debug log for the keyword-search fallback contains the full stack trace.

In `@Gradata/src/gradata/_core.py`:
- Around line 1370-1435: cloud_sync_tick currently appends raw events.data_json
dicts (from system.db) into session_corrections and passes them to
_cloud_sync_session, but persisted correction events use draft_text/final_text
while _cloud_sync_session expects a "final" field (it calls c.get("final") to
compute blandness); modify cloud_sync_tick to normalize each parsed correction
dict before appending so it has the same shape _cloud_sync_session expects —
e.g., if a dict has "final_text" or "draft_text", set "final" to final_text if
present else draft_text (and preserve other fields), ensure the normalization
happens inside the loop that builds session_corrections, and keep the symbol
names session_corrections, cloud_sync_tick, and _cloud_sync_session to locate
the change.

In `@Gradata/src/gradata/_doctor.py`:
- Around line 280-306: The cloud-env check is missing the GRADATA_API_URL /
GRADATA_API_KEY vars used by _cloud_sync_session; update _check_cloud_env_vars
(and similarly _check_cloud_auth) to consider GRADATA_API_URL as equivalent to
GRADATA_CLOUD_URL/GRADATA_SUPABASE_URL and GRADATA_API_KEY as equivalent to
GRADATA_CLOUD_KEY/GRADATA_SUPABASE_SERVICE_KEY so an env-only auth path is
recognized; locate and modify the checks that set url_set and key_set and the
missing list strings to include these GRADATA_API_* names and ensure
status/detail logic remains consistent with the existing enabled/url/key logic.

In `@Gradata/src/gradata/_events.py`:
- Around line 203-212: The raw_event dict currently stores the original raw_data
(key "data") which then gets written to events.raw.jsonl and thus persists
unredacted PII; change the code that builds raw_event to store a redacted copy
instead of raw_data (e.g., call the existing redaction routine used for
canonical storage—redact_event/redact_pii/canonical_data—assign to a variable
like redacted_data and set "data": redacted_data) and apply the same change to
the analogous write at lines 224-230 so that no unredacted payload is ever
written to events.raw.jsonl.

In `@Gradata/src/gradata/_file_lock.py`:
- Around line 102-103: The except OSError handler in _lock_win32 currently logs
a warning with traceback on each retry which spams logs; change the
logger.warning('Suppressed exception in _lock_win32', exc_info=True) to a
lower-severity log such as logger.debug(...) to suppress expected contention
noise, or alternatively record the exception but only emit a warning once when
the overall lock timeout branch is reached (i.e., keep per-retry handling quiet
in _lock_win32 and move/wrap a single logger.warning into the timeout/failure
path so only real failures produce warnings); apply the same change to the
corresponding except block used later.

In `@Gradata/src/gradata/_migrations/_ulid.py`:
- Around line 38-51: ulid_from_iso currently returns a random new_ulid() on
parse failure, which makes backfills non-deterministic; change the fallback so
it's deterministic by deriving entropy from the original iso_ts string: on
except (ValueError, TypeError) compute a stable hash of iso_ts (e.g.,
sha1/sha256) and use the required bytes of that hash as the entropy argument to
new_ulid (or set a fixed ts_ms like 0) instead of returning a random ULID;
update ulid_from_iso to call new_ulid with ts_ms and the derived entropy so
reruns produce the same ID for the same malformed input.

In `@Gradata/src/gradata/_migrations/device_uuid.py`:
- Around line 67-70: The code currently only calls os.replace(tmp, fpath) if
fpath doesn't exist, else unlinks tmp which causes a regenerated new_did to
never be persisted when the existing .device_id is present but invalid; update
the logic in the function that handles .device_id (referencing fpath, tmp,
new_did) so that whenever you generate a new_did you atomically replace the
on-disk file (use os.replace(tmp, fpath)) regardless of whether fpath.exists(),
instead of unlinking tmp; ensure tmp is removed only if replace fails or after a
successful replace to avoid leaking temp files.

In `@Gradata/src/gradata/_mine_transcripts.py`:
- Around line 313-317: The fallback timestamp currently uses datetime.now(UTC)
when msg.get("timestamp") is missing, causing non-deterministic ts values on
reruns; replace that dynamic fallback with a deterministic fallback derived from
the transcript file (e.g., use path.stat().st_mtime converted to an ISO
timestamp) so entries appended to events (see session_uuid, msg, events and the
"ts" key) remain stable across reruns and preserve deduplication.

In `@Gradata/src/gradata/_transcript_providers.py`:
- Around line 106-113: Guard against non-string content when building text_parts
so "\n".join won't raise; in the block-processing loop (variables btype, block,
text_parts, has_non_text, turn_type) only append block.get("text", "") if it's
an instance of str (or coerce safe types to str) otherwise set has_non_text=True
and skip adding it; ensure the final "content" value uses "\n".join(text_parts)
only when text_parts contains strings (else None) so transcript parsing does not
crash on non-string blocks.
- Line 61: The current return uses max(all_jsonls, key=lambda p:
p.stat().st_mtime) in _locate(), which will raise if p.stat() fails; instead
iterate all_jsonls, call p.stat() inside a try/except (catch OSError/IOError),
collect only entries with successful stats (e.g., tuples of (p, mtime)), then
select the path with the highest mtime from that filtered list and return it (or
None if empty); reference the all_jsonls variable, the _locate() function, and
the lambda p: p.stat().st_mtime expression when making the change.
- Around line 133-137: The _path method currently uses the raw self._session_id
to build a filesystem path (self._brain_dir / "sessions" / self._session_id /
"transcript.jsonl"), which permits path traversal; sanitize or validate session
ids before using them: ensure session id contains only allowed characters (e.g.
alphanumerics, hyphen/underscore or matches expected UUID regex) or take only
its basename, then construct the Path and verify the resolved path is inside the
sessions directory (compare resolved_path.with_suffix(""), or use
Path(self._brain_dir).resolve() / "sessions" and ensure
resolved_path.resolve().is_relative_to(...) before returning the file if
exists). Update the _path method to perform this validation on self._session_id
and only return the path when validation passes.

In `@Gradata/src/gradata/_transcript.py`:
- Around line 74-78: The code directly calls int(...) on environment values for
truncate limits, which can raise on bad env input; update the logic around limit
(where truncate_at, _TRUNCATE_ENV, DEFAULT_TRUNCATE are used) and the similar
block used by cleanup_ttl() to parse the env string safely: read
os.environ.get(...), attempt int() in a try/except catching
ValueError/TypeError, fall back to DEFAULT_TRUNCATE (or DEFAULT_TTL as
appropriate) on parse failure, and emit a warning/log via the existing logger so
log_turn() and cleanup_ttl() degrade safely instead of crashing. Ensure you
update both places (the limit assignment and the other env-int parsing at lines
referencing cleanup_ttl()) and keep behavior identical when truncate_at is
provided.
- Around line 46-47: The _session_dir function currently concatenates the raw
session_id into a filesystem path which allows path-traversal (e.g., "../") –
update _session_dir to validate and sanitize session_id (reject or normalize
values containing path separators like "/" or "\" or any traversal tokens), then
build the path and call .resolve() on both the base brain_dir Path and the
resulting session path and verify the session path starts with the resolved
brain_dir; if the check fails, raise a ValueError (or similar) to prevent
escaping the intended directory. Ensure references to _session_dir and the Path
operations are used so the fix is applied exactly where the path is composed.

In `@Gradata/src/gradata/adapters/mem0.py`:
- Around line 163-166: The warning logs in Mem0Adapter.pull_memory_for_context
currently call logger.warning without exception context; update both
logger.warning calls inside pull_memory_for_context to pass exc_info=True (e.g.
logger.warning("Mem0Adapter.pull_memory_for_context failed: %s", exc,
exc_info=True)) so the stacktrace is included for debugging, locating these
calls by the Mem0Adapter.pull_memory_for_context function and the two
logger.warning occurrences in that method.

In `@Gradata/src/gradata/brain.py`:
- Around line 895-918: The "rules.injected" event payload currently includes the
raw task string (see the _bus.emit call that sends "rules.injected" with "task":
task), which should not be persisted; modify the emit payload to remove the raw
task field or replace it with a non-sensitive normalized signal (e.g.,
task_category or task_hash) before calling _bus.emit in the same block that
constructs "rules" from applied; ensure the emitted payload only contains safe
fields (rules, scope, and the new non-PII task signal) and update any downstream
references expecting "task" accordingly.

In `@Gradata/src/gradata/cli.py`:
- Around line 1251-1253: The --cloud and --no-cloud flags are currently both
added to p_doctor and can be set together; make them mutually exclusive by
creating an argparse mutually exclusive group (call add_mutually_exclusive_group
on the same parser used for p_doctor) and add the two flags to that group
instead of calling p_doctor.add_argument directly; apply the same change for the
other occurrence that adds these flags so both sets use a mutually exclusive
group.

In `@Gradata/src/gradata/contrib/enhancements/quality_gates.py`:
- Around line 360-370: The SQLite connection in evaluate_success_conditions is
not closed if conn.execute(...) raises because conn.close() is after the call
and exceptions are swallowed; replace the manual connect/close with a context
manager by using with sqlite3.connect(str(db)) as conn: around the
execute/fetchone block (keeping the assignment to max_session and
report.sessions_evaluated inside the with) so the connection is always closed
even on error while preserving the existing exception handling/logger.warning.

In `@Gradata/src/gradata/enhancements/bandits/collaborative_filter.py`:
- Around line 155-159: The current conditional in collaborative_filter.py
matches transfer recommendations by category (rec.category == lesson.category),
which can boost the wrong lesson when multiple lessons share a category; change
the match to compare the rule identity instead (e.g., rec.rule_id or rec.rule)
with the lesson's rule identifier (e.g., lesson.rule_id or lesson.rule) so the
condition becomes something like rec.rule_id == lesson.rule_id and
rec.transfer_boost > 0 and lesson.confidence < 0.90; update any tests or
comments referencing category-based matching to reflect rule-based matching and
ensure you use the actual rule identifier fields present on rec and lesson.

In `@Gradata/src/gradata/enhancements/graduation/agent_graduation.py`:
- Around line 222-225: The "owner_only" regex contains the literal token
EXCLUDED_NAMES_PLACEHOLDER which gets compiled verbatim by
compile_deterministic_rule(), so replace that placeholder with the actual
excluded-owner pattern from configuration before compilation (or have
compile_deterministic_rule() fetch/resolve the config value and substitute it
into the pattern); if the config value is not available, have
compile_deterministic_rule() return None instead of compiling the placeholder.
Update the code paths that define the tuple with "owner_only" and the
compile_deterministic_rule() implementation to perform this
substitution/resolution (or the None-early-return) to ensure the resulting
deterministic rule matches real owner names rather than the placeholder text.
- Around line 319-320: AgentGraduationTracker._load_profile currently suppresses
json.JSONDecodeError/KeyError and returns a fresh AgentProfile, which causes a
corrupt profile to be overwritten on next record_outcome; instead, on
JSONDecodeError/KeyError either re-raise a descriptive exception or quarantine
the unreadable profile file (e.g., move profile.json ->
profile.json.corrupt.{timestamp}) and return None/raise so caller knows not to
persist; update record_outcome to check for a None/exceptional profile and fail
fast (or write to a quarantine-only path) rather than saving a blank
AgentProfile; ensure logs include the filename and exception details so
operators can recover the quarantined file.

In `@Gradata/src/gradata/enhancements/meta_rules.py`:
- Line 301: The fallback "working" is not part of the module's task taxonomy and
causes inconsistent principle prefixes; change the get fallback to use a stable
module-level default (e.g., introduce _DEFAULT_TASK_TYPE set to a taxonomy-valid
label like "other" or whatever label the module uses) and replace task_type =
_CATEGORY_TASK_MAP.get(category, "working") with task_type =
_CATEGORY_TASK_MAP.get(category, _DEFAULT_TASK_TYPE); ensure _DEFAULT_TASK_TYPE
is defined near the other taxonomy defs so it's clearly visible and consistent.

In `@Gradata/src/gradata/enhancements/observation_hooks.py`:
- Line 82: The email regex in the PII redaction patterns uses the character
class "[A-Z|a-z]{2,}" which treats '|' as a literal and weakens matching; update
the pattern expression in the tuple with
_re.compile(r"\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}\b") (or use a
case-insensitive flag) where the pattern tuple is defined (the compiled regex
tuple in observation_hooks.py / the PII redaction patterns list) so the
top-level character class is corrected to "[A-Za-z]{2,}" and retains the
surrounding anchors.

In `@Gradata/src/gradata/enhancements/pipeline_rewriter.py`:
- Line 80: The assignment to desc assumes data["failed_rule_description"] or
data["description"] is a string and calls .strip(), which will raise if the
payload contains non-string types; update the logic around desc (in
pipeline_rewriter.py where desc is assigned) to check the type before calling
.strip()—e.g., use isinstance(value, str) and only strip strings, otherwise
coerce to an empty string or safe str(value) as appropriate so malformed
dicts/ints won't raise.

In `@Gradata/src/gradata/enhancements/rule_synthesizer.py`:
- Around line 107-112: Cached writes via _write_cache currently write directly
and cached reads return content without validation; ensure writes are atomic and
reads validate content before returning to avoid injecting truncated/corrupted
data. Modify _write_cache to write to a temporary file in the same directory
(e.g., brain_dir / CACHE_DIRNAME / f"{cache_key}.tmp") and atomically replace
the final path from _cache_path(brain_dir, cache_key) using Path.replace or an
equivalent atomic rename. In the cache-reading path (the function that calls
_cache_path(...).read_text() / the code around the return at the cached-read
site), attempt to validate/parse the content (use the same parsing/validation
logic the caller expects), and on parse/validation failure delete the bad cache
file and return the fallback (e.g., None or trigger fragmented mode) rather than
returning malformed content.

---

Outside diff comments:
In `@Gradata/docs/cloud/overview.md`:
- Around line 3-24: The table incorrectly implies Cloud performs meta-rule
synthesis; update the Meta-rule **synthesis** row (the line with "Meta-rule
**synthesis** (local LLM via your own key or Claude Code Max OAuth)") so the
Open-source SDK column remains "Yes" but the Gradata Cloud column clearly
indicates "No — runs locally only (via the user's LLM/key); Cloud only
visualizes/orchestrates results" and adjust the surrounding prose sentence that
mentions synthesis to state synthesis is SDK-local only; apply the same edit to
the duplicate instance of that row elsewhere in the document.

In `@Gradata/src/gradata/_context_packet.py`:
- Around line 145-163: The code uses prospect_lower.split()[0] which will raise
IndexError for empty or whitespace-only prospect_name; update the logic in
_load_prospect_context (the block that calls _events_query and builds
result["recent_interactions"]) to first guard prospect_name by checking
prospect_name (or prospect_lower) is non-empty after strip() and only then
perform the split/lookup, otherwise skip matching and set
result["recent_interactions"] to an empty list (or leave interactions empty);
ensure this check surrounds the tag-matching loop that uses
prospect_lower.split()[0] and references _events_query, interactions, and result
so no IndexError can propagate.

In `@Gradata/src/gradata/_doctor.py`:
- Around line 86-108: The _check_brain_dir function should honor an explicitly
resolved brain path instead of always reading BRAIN_DIR from the environment;
change _check_brain_dir to accept an optional parameter (e.g., brain_path) and
use that when provided (falling back to os.environ.get("BRAIN_DIR") only if
brain_path is None), then update diagnose(...) to pass the resolved
brain_path/brain_dir into _check_brain_dir so callers using the --brain-dir flag
get the correct status; ensure the function still returns the same dict shape
("name", "status", "detail") and retains the existing checks (is_dir, exists but
not dir, not exists).
- Around line 464-524: The cloud probes currently run unconditionally and any
"fail" from those checks can mark the whole diagnose() result as "broken";
change the logic so optional cloud checks don't downgrade a healthy local
install: in either _cloud_checks or in diagnose() gate running
_check_cloud_reachable/_check_cloud_auth/_check_cloud_has_data/_check_cloud_push_error
behind a confirmed cloud configuration by first evaluating _check_cloud_config()
and _check_cloud_env_vars() (or use their combined result) and only include the
deeper probes if config/env indicate cloud is configured; alternatively, after
collecting cloud checks, if _check_cloud_config or _check_cloud_env_vars returns
"skip"/"missing", map any subsequent cloud check statuses to "skip" (or "warn")
before computing overall status so cloud failures cannot mark the whole report
"broken".

In `@Gradata/src/gradata/brain.py`:
- Around line 656-675: The code currently calls
lessons_path.write_text(format_lessons(existing), ...) inside lessons_lock which
can leave a truncated file on crash; instead perform an atomic replace using the
same serializer: write format_lessons(existing) to a temporary file (e.g.,
lessons_path.with_suffix(".tmp") or tempfile.NamedTemporaryFile in the same
directory), fsync the temp file, then atomically replace the original with
os.replace(temp_path, lessons_path) while still under lessons_lock (or call the
existing project helper for atomic writes if one exists). Keep using
format_lessons and parse_lessons as before and preserve encoding="utf-8".

In `@Gradata/src/gradata/cloud/client.py`:
- Around line 29-51: CloudClient currently stores the endpoint from
DEFAULT_ENDPOINT or ENV_ENDPOINT as-is, causing routes like connect() and sync()
to miss the required "/api/v1" prefix; normalize the base URL in
CloudClient.__init__ so self.endpoint always ends with "/api/v1" (trim trailing
slashes first, then append "/api/v1" only if it's not already present) so that
_post(), connect(), sync() and any other methods build correct routes; reference
CloudClient, __init__, DEFAULT_ENDPOINT, ENV_ENDPOINT, and methods like _post(),
connect(), sync() when making the change.

In `@Gradata/src/gradata/cloud/sync.py`:
- Around line 170-180: contribute_corpus currently posts to
"/v1/corpus/contribute" which is inconsistent with the newer "/api/v1" routing
used elsewhere (e.g. sync_metrics); update the endpoint in contribute_corpus to
use the "/api/v1/corpus/contribute" path and ensure the call to self._post still
sends {"patterns": anonymized_patterns} and returns result is not None so the
behavior remains unchanged besides the corrected URL.

In `@Gradata/src/gradata/contrib/patterns/human_loop.py`:
- Around line 494-507: The check method in HumanLoopGate ignores the
caller-supplied context by calling gate(action) without it; change the call to
pass the context through (e.g., call gate(action, context) or gate(action,
context=context) depending on gate's signature) so overrides and target metadata
are considered, and ensure any approver invocation (return approver(request))
still works with the request produced when context is supplied. Update only the
gate invocation in HumanLoopGate.check (and adjust gate's callable type if
needed) to thread the context argument.

In `@Gradata/src/gradata/contrib/patterns/rag.py`:
- Around line 319-449: cascade_retrieve currently returns the hybrid result as
soon as all_results is non-empty, making CascadeConfig.two_pass ineffective; to
fix, change the control flow so the two-pass expansion (using
extract_expansion_terms, calling fts_fn/vector_fn for pass2, merging with
rrf_merge and rescoring with apply_graduation_scoring) is executed before
returning when cfg.two_pass is true — either move the entire two_pass block to
run immediately after building all_results (before the current hybrid return) or
condition the hybrid return on not cfg.two_pass (i.e., if all_results and not
cfg.two_pass: build merged/result and return; otherwise run two_pass logic,
merge pass1+pass2, then score and return); ensure you still populate result.mode
with any _cascade_errors and compute total_candidates correctly using total and
pass2_total.

In `@Gradata/src/gradata/enhancements/graduation/agent_graduation.py`:
- Around line 623-630: The JSON parse failure for lesson.scope_json currently
only logs and lets the lesson fall through as global; update both
AgentGraduationTracker.get_agent_rules and get_deterministic_rules so that when
json.loads(lesson.scope_json) raises json.JSONDecodeError or TypeError you skip
that lesson/rule (e.g., replace the except block that logs with one that logs
and then continue/skip) to fail-closed when task_type filtering is active;
ensure you still log exc_info=True for diagnostics and reference the same lesson
when skipping.

In `@Gradata/src/gradata/enhancements/router_warmstart.py`:
- Around line 75-124: The SQLite connection opened via sqlite3.connect (conn)
may remain open if the query or processing raises; change the logic to always
close the connection by wrapping the connection in a context manager (use "with
sqlite3.connect(str(db_path)) as conn:" and keep conn.row_factory assignment
inside) or move conn.close() into a finally block so conn.close() is executed on
all paths; keep the same query and subsequent router routing/reward/update logic
(router.route, router.reward_from_severity, router.update_reward, router.save)
but ensure the connection is reliably closed even on exceptions.

In `@Gradata/src/gradata/enhancements/rule_context_bridge.py`:
- Around line 100-107: Replace the unsupported row.get usage by explicitly
checking membership on the sqlite3.Row and avoid relying on .get; change the
category extraction to use row["category"] if "category" in row else "META" (and
remove or broaden the try/except that only caught KeyError/IndexError so
AttributeError won't propagate), leaving the existing principle assignment
(principle = row["principle"] if "principle" in row else str(row)) as-is; update
any surrounding try/except to catch AttributeError if you choose to keep it.
- Around line 90-95: The try block in bootstrap_rule_context currently opens a
sqlite3 connection via c2 = sqlite3.connect(str(db_path)) then calls
c2.execute(...) and c2.close() in the success path, which will skip closing on
exceptions; replace this pattern to use a context manager (with
sqlite3.connect(str(db_path)) as c2:) so the connection is always closed even if
PRAGMA wal_checkpoint(TRUNCATE) raises, and keep the existing except
logger.warning(..., exc_info=True) behavior around the with block.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

Comment on lines +1 to +27
"""
Gradata Dashboard — Your AI's fitness tracker.
===============================================
Run: streamlit run C:/Users/olive/SpritesWork/brain/scripts/dashboard.py
"""

import json
import re
import sqlite3
from datetime import datetime
from pathlib import Path

import pandas as pd
import plotly.graph_objects as go
import streamlit as st

# ---------------------------------------------------------------------------
# Config
# ---------------------------------------------------------------------------
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")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Remove this archived dashboard artifact from the PR.

This file bakes in local personal paths and user-specific context, and it introduces an archived app script into the SDK repository history. It should stay out of the tracked tree (or be moved to a private/internal repo).

Based on learnings: Never commit scratch files like .tmp/, .archive/, sessions/handoff-*.md, or files literally named 0 or BrainDetail.

🤖 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 1
- 27, Remove the archived dashboard artifact
Gradata/.archive/dashboard_streamlit_deprecated_2026-04-23.py from the PR by
deleting it from the commit (git rm --cached or remove and recommit) and ensure
similar personal/scratch files under .archive/ or with local absolute paths are
not reintroduced by adding the pattern (.archive/, *.tmp, sessions/handoff-*.md,
files named "0" or "BrainDetail") to .gitignore; if you need to keep the file
for personal use, move it to a private/internal repo instead of tracking it
here.

Comment thread Gradata/AGENTS.md
Comment on lines +80 to +85
```
Layer 2 — Public API brain.py, cli.py, daemon.py, mcp_server.py
Layer 1 — Enhancements enhancements/*, rules/
Layer 0 — Primitives _types.py, _db.py, _events.py, _paths.py, _file_lock.py …
Layer 0 — Patterns contrib/patterns/*
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add language identifiers to fenced code blocks (MD040).

Three fenced blocks are missing a language marker, which will keep markdownlint warnings active.

💡 Suggested patch
-```
+```text
 Layer 2 — Public API        brain.py, cli.py, daemon.py, mcp_server.py
 Layer 1 — Enhancements      enhancements/*, rules/
 Layer 0 — Primitives        _types.py, _db.py, _events.py, _paths.py, _file_lock.py …
 Layer 0 — Patterns          contrib/patterns/*

- +text
INSTINCT → PATTERN → RULE → META_RULE
↓ ↓ ↓
KILLED INSTINCT ARCHIVE (contradiction / decay / graduation)


-```
+```text
<type>(<scope>): <imperative description>

[optional body — what + why, not how]

[optional trailers]
</details>
 


Also applies to: 103-107, 175-181

<details>
<summary>🧰 Tools</summary>

<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>

[warning] 80-80: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

</details>

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @Gradata/AGENTS.md around lines 80 - 85, Several fenced code blocks in
AGENTS.md lack a language identifier; update each triple-backtick fence that
precedes the blocks starting with "Layer 2 — Public API brain.py, cli.py,
daemon.py, mcp_server.py", "INSTINCT → PATTERN → RULE → META_RULE", and
"(): " to include a language tag (use
"text" as suggested) so the opening becomestext, and apply the same
change for the other two similar fenced blocks noted later in the file (the
blocks around the same examples) to eliminate the MD040 markdownlint warnings.


</details>

<!-- fingerprinting:phantom:medusa:grasshopper:58aceea9-823f-4495-88d7-9940b13b2dc8 -->

<!-- d98c2f50 -->

<!-- This is an auto-generated comment by CodeRabbit -->

Comment thread Gradata/CHANGELOG.md
Comment on lines +3 to +79
## [Unreleased] — post-0.6.0 (2026-04-23 → 2026-04-24)

33 commits ahead of public `origin/main`. Not yet pushed.

### Added

- **Cloud sync + Supabase schema hardening.** Dual-write path now pushes local
brain events to Supabase with a transform layer (`_cloud_sync.py`) that maps
SQLite rows to cloud schema + scrubs JSONB payloads. Watermark-based
incremental sync via `sync_state` table (migration 003).
- **Local SQLite migrations 002 + 003.** `002_event_id_device_id_content_hash`
adds sync-stable identifiers; `003_add_sync_state` creates the watermark table.
Both idempotent — `CREATE TABLE IF NOT EXISTS` + `add_column_if_missing` + `has_applied()` gating.
- **Supabase migrations 014/015/016 applied to prod.** UNIQUE constraints on
`corrections(brain_id, session, description)` and `events(brain_id, type, created_at)`,
plus `brains.last_used_at` column. Reference SQL tracked under
`Gradata/migrations/supabase/`; README documents application state + governance.
- **Canonical graduation + persistent brain_prompt + two-provider synth**
(`f91d5557`). `<brain-wisdom>` now regenerates on every graduation-triggering
session close, model-agnostic.
- **Context-window watchdog hook** (`ctx_watchdog`, commit `56bac80c`): auto-handoff
when Claude Code context hits threshold. Reduces forced /clear losses.
- **Auto-compact handoff pipeline** (`485cd7b4`): two-phase /clear injection so
session state survives compaction.
- **Code-review-graph activation enforced** before any Glob/Grep call
(`fd956ec4`) — pushes agents to use the structural graph instead of brute-force
file search.
- **Cloud-health probes in `gradata doctor`** (`d5425337`): reachability + auth
token validation + data sanity.
- **`lesson_applications` audit loop** (`d668bab7`): closes the
compound-quality feedback cycle.
- **Implicit feedback: text-speak detection** (`5a6da455`, `1a497e85`):
catches corrections phrased as "r/u/dont/cant".

### Changed

- **Statusline session count** sourced from Anthropic JSONL (`18166663`,
`74af66e6`, `a405447d`) — replaces stale `loop-state.md` counter (367 → 659).
- **Meta-rules `llm_synth` runs locally**, not cloud-side (`0b797b73`).
Removes cloud-dependence for a core graduation primitive.
- **Streamlit dashboard deprecated** (`3ed9438c`). `gradata.ai` web dashboard
now covers all panels (`/rules`, `/corrections`, `/self-healing`,
`/observability`). Legacy CLI archived to
`Gradata/.archive/dashboard_streamlit_deprecated_2026-04-23.py`.
- **`implicit_feedback` hook emit-only contract** (`aace2410`): main() returns
None uniformly; signals emit via `IMPLICIT_FEEDBACK` event instead of as
UserPromptSubmit injection to reduce prompt noise.

### Fixed

- **Bare `except: pass` blocks in core SDK** now log at DEBUG (`812eda9c`).
Removes silent-failure class from Layer 0.
- **MISFIRE_PENALTY sign in `agent_graduation`** (`03ddb6f9`): penalties
were being applied as bonuses.
- **Session-start hook**: correct lessons path + brain_prompt load +
tighten stale-notes detection (`c2cc47b6`).
- **Cluster injection line count** now scopes to `<brain-rules>` block only,
not full prompt (`118122a2`).
- **Public docs truth-pass** on cloud-vs-SDK boundary (`978e4c7f`): removed
stale cloud-graduation claims from Pro tier marketing (`2c65bf2a`).

### Tests

- **3932 pass, 3 skip** (up from 2598 in v0.6.0). No xfail remaining.
- Meta-rules cloud-gated tests unskipped (`509bf927`).
- `pipeline_e2e`: removed "not yet implemented" skips, bumped fixtures (`2a781645`).
- `test_implicit_feedback`: coverage for text-speak + multi-signal inputs.

### Security / Governance

- **Supabase migrations now idempotent.** 014/015 wrapped in `DO $$` blocks
that check `pg_constraint` for existing UNIQUE on same columns before
adding. Prod state: both tables have pre-existing `_key` variants (from
inline `UNIQUE()` in original CREATE TABLE) + the new `_unique` variants —
redundant but harmless. Documented in `migrations/supabase/README.md`.
- `.gitignore` hardened against bash-redirect artifacts (`0`, `BrainDetail`),
graphify cache files (`.graphify_*`), and run.log spray.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Scope the Unreleased changelog to the actual PR contents.

This section currently publishes unrelated local/prod work (33 commits ahead, Supabase prod migrations, dashboard deprecation, local synth changes, stale test totals) that the PR objectives say are not part of #145. If this lands as-is, the release notes will claim behavior users cannot get from this branch.

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

In `@Gradata/CHANGELOG.md` around lines 3 - 79, The Unreleased changelog publishes
unrelated work; edit Gradata/CHANGELOG.md under the "## [Unreleased] —
post-0.6.0 (2026-04-23 → 2026-04-24)" block to only include changes actually
introduced by this PR: remove the "33 commits ahead of public `origin/main`"
line and any entries referencing unrelated production changes such as "Supabase
migrations 014/015/016", "Streamlit dashboard deprecated", and other unrelated
bullets (e.g., prod-only schema hardening, dashboard deprecation, or stale test
totals). Keep or add only the Added/Changed/Fixed/Test/Security bullets that
correspond to this PR's diffs (look for items like `_cloud_sync.py`,
`003_add_sync_state`, `ctx_watchdog`, and `implicit_feedback` only if they are
part of the PR), and update counts/headers accordingly so the Unreleased section
reflects this PR alone.

Comment on lines +47 to +50
!!! info "Local by default"
Meta-rule clustering **and** principle synthesis both run locally. Synthesis uses whichever LLM path you've configured: your own Anthropic API key (set `ANTHROPIC_API_KEY`) or the Claude Code Max OAuth path via `claude -p`. Cloud is not required for any of it — the full `[rule, rule, rule] → "Verify before acting"` pipeline runs in the OSS SDK.

The math, the events, and the storage are all open. Only the LLM-driven synthesis that turns `[rule, rule, rule] → "Verify before acting"` is cloud-gated.
Cloud becomes relevant when you want a hosted dashboard, cross-device sync, team brains, or (future) opt-in corpus donation. It does not re-synthesize or override what graduated locally.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use a fenced code block inside the info callout.

The indented sample here trips MD046, so markdownlint will keep flagging this file. Switching it to a fenced block keeps the rendered output the same.

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 50-50: Code block style
Expected: fenced; Actual: indented

(MD046, code-block-style)

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

In `@Gradata/docs/concepts/meta-rules.md` around lines 47 - 50, Replace the
indented markdown sample inside the "Local by default" info callout with a
fenced code block to satisfy markdownlint rule MD046; locate the callout titled
"Local by default" in meta-rules.md and change the indented paragraph sample
(the multi-line description under the callout) to use triple backticks (```)
fenced code block syntax so the rendered output stays the same but avoids the
MD046 warning.


## Known legacy items to retire

### 1. Deprecated adapter shims (scheduled v0.8.0)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Keep the section headers lint-clean.

These headings are missing the blank lines required by MD022, so this doc will keep tripping markdownlint in CI. Adding the missing spacing is a trivial cleanup.

Also applies to: 22-22, 27-27, 32-32, 37-37, 44-44

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 16-16: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

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

In `@Gradata/docs/LEGACY_CLEANUP.md` at line 16, The markdown headings like
"Deprecated adapter shims (scheduled v0.8.0)" (and the other headers at the
noted locations) are missing the required blank line above them and trigger
MD022; add a single blank line immediately before each of those heading lines so
each header is separated from the previous content (ensure the same fix is
applied for the other headers referenced: the ones at lines corresponding to the
other headings), which will make the section headers lint-clean.

Comment thread Gradata/src/gradata/enhancements/graduation/agent_graduation.py Outdated

def _build_principle(category: str, best_text: str) -> str:
"""Turn a representative correction into a prompt-ready principle."""
task_type = _CATEGORY_TASK_MAP.get(category, "working")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use a stable default task label in principle generation.

Line 301 defaults to "working", which is outside the rest of this module’s task taxonomy and produces inconsistent principle prefixes.

Suggested fix
-    task_type = _CATEGORY_TASK_MAP.get(category, "working")
+    task_type = _CATEGORY_TASK_MAP.get(category, "general")
📝 Committable suggestion

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

Suggested change
task_type = _CATEGORY_TASK_MAP.get(category, "working")
task_type = _CATEGORY_TASK_MAP.get(category, "general")
🤖 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` at line 301, The fallback
"working" is not part of the module's task taxonomy and causes inconsistent
principle prefixes; change the get fallback to use a stable module-level default
(e.g., introduce _DEFAULT_TASK_TYPE set to a taxonomy-valid label like "other"
or whatever label the module uses) and replace task_type =
_CATEGORY_TASK_MAP.get(category, "working") with task_type =
_CATEGORY_TASK_MAP.get(category, _DEFAULT_TASK_TYPE); ensure _DEFAULT_TASK_TYPE
is defined near the other taxonomy defs so it's clearly visible and consistent.

Comment thread Gradata/src/gradata/enhancements/observation_hooks.py Outdated
Comment thread Gradata/src/gradata/enhancements/pipeline_rewriter.py Outdated
Comment thread Gradata/src/gradata/enhancements/rule_synthesizer.py
oliver added 8 commits April 30, 2026 10:25
…Rabbit feedback

events.raw.jsonl now requires GRADATA_DEBUG_RAW_EVENTS=1 (was always-on
and contained unredacted PII). brain.apply_brain_rules emits a sha256
prefix instead of the raw task. Test updated to assert default-off
behavior.
…CodeRabbit feedback

_atomic.atomic_write_text now uses tempfile.mkstemp (was pid-based and
collided on intra-process races). quality_gates.evaluate_success_conditions
wraps sqlite3.connect in a context manager. _file_lock noisy WARNING
on every retry downgraded to DEBUG.
…ddresses PR #145 CodeRabbit feedback

_transcript and _transcript_providers now reject session ids containing
path separators or '..' before joining into a Path. _safe_int_env wraps
os.environ int parses with try/except. _mine_transcripts derives a
deterministic ts hash when msg.timestamp is missing so reruns produce
stable dedup keys.
…145 CodeRabbit feedback

Migrations 014/015 previously used '@>' (superset) when probing for the
unique constraint, so a constraint covering MORE columns would falsely
satisfy the check and skip creation. Switched to a sorted-array equality
test against the exact column set.
…addresses PR #145 CodeRabbit feedback

_ulid.ulid_from_iso now derives a deterministic ULID via sha256 on parse
failure (was new_ulid() which broke migration idempotence). device_uuid
get_or_create_device_id detects malformed-but-existing files and persists
the regenerated id, eliminating the path that re-minted on every call.
… rule — addresses PR #145 CodeRabbit feedback

Removed the DATA_INTEGRITY 'EXCLUDED_NAMES_PLACEHOLDER' regex — a literal
placeholder that could never match real text and silently disabled the
guard. Test updated to assert compile_deterministic_rule returns None
until the excluded-names config source is wired. _load_profile now backs
up corrupt profile files and re-raises once so the failure is surfaced
instead of silently swallowed.
…dresses PR #145 CodeRabbit feedback

rule_synthesizer cache now carries a 'synthesizer-version: vN' header
and rejects stale/invalid payloads (test updated to write the header).
observation_hooks email regex character class fixed ('A-Z|a-z' -> 'A-Za-z',
the pipe was matching a literal '|'). pipeline_rewriter._failures_by_description
now isinstance-checks the description before .strip() so legacy non-string
payloads don't crash.
Lockfile refresh accompanying the CR fix-up commits.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 10

Caution

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

⚠️ Outside diff range comments (3)
Gradata/src/gradata/enhancements/graduation/agent_graduation.py (2)

300-320: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Treat non-object profile.json payloads as corrupt too.

Line 320 only handles syntax errors and missing keys. A file containing [], null, or another valid-but-wrong JSON shape will still blow up with AttributeError/TypeError before the backup path runs, so the new quarantine logic is skipped.

🛠️ Suggested change
                 data = json.loads(profile_path.read_text(encoding="utf-8"))
+                if not isinstance(data, dict):
+                    raise TypeError(f"expected JSON object, got {type(data).__name__}")
                 # Load lessons from lessons.md
                 lessons_path = self._agent_dir(agent_type) / "lessons.md"
@@
-            except (json.JSONDecodeError, KeyError) as exc:
+            except (json.JSONDecodeError, KeyError, TypeError, AttributeError) as exc:
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Gradata/src/gradata/enhancements/graduation/agent_graduation.py` around lines
300 - 320, The code should treat non-object JSON payloads as corrupt: after
json.loads(profile_path.read_text(...)) in the loader inside AgentProfile
creation, validate that the resulting data is a dict (e.g., if not
isinstance(data, dict) raise ValueError("profile.json must be an object")); then
extend the exception handler (the try/except around json.loads and building
AgentProfile in agent_graduation.py) to also catch ValueError (or add ValueError
to the existing except tuple) so that arrays/null/other shapes go to the
quarantine/backup path; reference the variables/profile construction lines
around AgentProfile, profile_path, parse_lessons, and self._profiles to locate
the change.

558-573: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Gate misfire penalties on the matching category as well.

The new category_matches guard only protects fire_count. On Line 573, a single rejected edit still decreases confidence for every lesson in the profile, which lets an unrelated rejection demote mature rules in other categories.

🛠️ Suggested change
-            elif outcome == "rejected":
+            elif outcome == "rejected" and category_matches:
                 lesson.confidence = max(0.0, lesson.confidence + MISFIRE_PENALTY)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Gradata/src/gradata/enhancements/graduation/agent_graduation.py` around lines
558 - 573, The rejection branch currently applies MISFIRE_PENALTY to
lesson.confidence for every lesson regardless of category; add the same
category_matches guard used above so misfire penalties only affect lessons whose
category matches the normalized edited category (respecting the legacy
norm_edit_cat empty behavior). Concretely, modify the elif outcome == "rejected"
branch around lesson.confidence = max(0.0, lesson.confidence + MISFIRE_PENALTY)
to only execute that line when category_matches is true.
Gradata/src/gradata/_events.py (1)

699-724: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Run queued events through the same redaction path before batch writes.

emit() now redacts event payloads before persisting, but RetainOrchestrator.flush() still writes event["data"] verbatim to both events.jsonl and SQLite. Any caller that queues raw events can still persist unredacted emails through the batch path.

🛠️ Suggested change
-            lines = [
-                json.dumps(event, default=str, ensure_ascii=False) + "\n" for event in new_events
-            ]
+            canonical_events: list[dict] = []
+            for event in new_events:
+                canonical = dict(event)
+                canonical["data"] = _redact_payload(canonical.get("data", {}))
+                canonical_events.append(canonical)
+            lines = [
+                json.dumps(event, default=str, ensure_ascii=False) + "\n"
+                for event in canonical_events
+            ]
             _locked_append_many(self.events_path, lines)
-            result["written"] = len(new_events)
+            result["written"] = len(canonical_events)
@@
-                        for event in new_events:
+                        for event in canonical_events:
                             conn.execute(
@@
-                                    json.dumps(event.get("data", {}), default=str),
+                                    json.dumps(event.get("data", {}), default=str),
♻️ Duplicate comments (3)
Gradata/src/gradata/_file_lock.py (1)

149-149: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Downgrade POSIX retry logging to debug (or warn once on timeout).

Line 149 logs a warning + traceback on every retry; lock contention is expected here and this will spam logs. Match the Windows path (debug) or emit a single warning only when timeout is reached.

Suggested change
-        except OSError:
-            logger.warning('Suppressed exception in _lock_posix', exc_info=True)
+        except OSError:
+            logger.debug("lock retry on %s", fh.name, exc_info=True)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Gradata/src/gradata/_file_lock.py` at line 149, The _lock_posix retry path is
logging a full warning+traceback on every retry which spams logs; in the
_lock_posix function replace the per-retry logger.warning call with logger.debug
(preserving exc_info=True) to match the Windows path, or alternatively emit a
single logger.warning only when the overall timeout is exceeded (track the start
time in _lock_posix, log once on timeout and raise). Update the call site
referencing _lock_posix and the existing logger variable so only debug-level
messages appear during normal contention and a single warning is emitted on
timeout.
Gradata/src/gradata/enhancements/rule_synthesizer.py (1)

128-131: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Cache-hit validation should enforce the same minimum block quality as fresh synthesis.

Line 128 validates header/tags, but Line 357 returns cached content without the len(block) >= 50 guard used on fresh responses (Lines 375-376). This allows trivially short <brain-wisdom> cache entries to bypass fail-safe quality checks.

Suggested patch
 def _read_cache(brain_dir: Path, cache_key: str) -> str | None:
@@
-    if not _is_valid_cache_payload(raw):
+    if not _is_valid_cache_payload(raw):
         _log.debug("synth cache invalid (version/empty), ignoring: %s", path)
         return None
-    return _strip_cache_header(raw)
+    cached = _strip_cache_header(raw)
+    block = _extract_wisdom_block(cached)
+    if not block or len(block) < 50:
+        _log.debug("synth cache malformed/too short, ignoring: %s", path)
+        return None
+    return block

Also applies to: 355-357

🤖 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 128 - 131,
The cached payload path currently returns _strip_cache_header(raw) after
_is_valid_cache_payload(raw) but skips the minimum-block-size guard used for
fresh synthesis; change the cache-return branch to strip the header first (call
_strip_cache_header(raw)), extract the synthesized block (the <brain-wisdom>
body or equivalent), and enforce the same len(block) >= 50 check used in the
fresh-synthesis flow—if the block is too short, treat the cache as invalid and
return None; apply the same check to the other cache-return site noted (the
other _strip_cache_header(raw) return).
Gradata/src/gradata/_transcript.py (1)

63-68: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Clamp transcript env integers to non-negative values.

Line 157 currently allows negative GRADATA_TRANSCRIPT_TTL_DAYS; that makes cutoff land in the future and can delete nearly all transcripts in one pass. The same parser also permits negative truncate values at Line 105, which causes unintended slicing behavior.

Suggested fix
-def _safe_int_env(key: str, default: int) -> int:
+def _safe_int_env(key: str, default: int, *, minimum: int = 0) -> int:
     """Read an int from env with try/except fallback."""
     try:
-        return int(os.environ.get(key, str(default)))
+        return max(minimum, int(os.environ.get(key, str(default))))
     except (TypeError, ValueError):
         return default
@@
     limit = (
         truncate_at
         if truncate_at is not None
-        else _safe_int_env(_TRUNCATE_ENV, DEFAULT_TRUNCATE)
+        else _safe_int_env(_TRUNCATE_ENV, DEFAULT_TRUNCATE, minimum=0)
     )
@@
-    days = ttl_days if ttl_days is not None else _safe_int_env(_TTL_ENV, DEFAULT_TTL_DAYS)
+    days = (
+        ttl_days
+        if ttl_days is not None
+        else _safe_int_env(_TTL_ENV, DEFAULT_TTL_DAYS, minimum=0)
+    )
Based on learnings: “_env_int default clamping to minimum”.

Also applies to: 102-106, 155-160

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

In `@Gradata/src/gradata/_transcript.py` around lines 63 - 68, The _safe_int_env
function currently returns parsed ints which may be negative; update it to clamp
results to a non-negative minimum by returning max(int(os.environ.get(key,
str(default))), 0) (still catching TypeError/ValueError and returning
max(default, 0)), so any callers reading environment values like
GRADATA_TRANSCRIPT_TTL_DAYS or truncate counts receive a non-negative integer
and cannot produce future cutoff times or negative slicing 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/migrations/supabase/014_corrections_unique.sql`:
- Around line 14-16: The migration uses unqualified table names (corrections,
events) in the DELETE ... USING ... WHERE statements and in the IF NOT EXISTS
catalog guard, so update those statements to fully qualify the target tables
with the current schema and make the catalog check filter by namespace;
specifically, replace the unqualified "DELETE FROM corrections a USING
corrections b WHERE a.brain_id = b.brain_id" (and the equivalent in events) with
schema-qualified identifiers for the same schema the migration runs in, and
change the IF NOT EXISTS/pg_class check to join pg_namespace (or compare nspname
= current_schema()) so relname is matched within the same namespace as the
DELETE, ensuring idempotency across search_path variations.

In `@Gradata/src/gradata/_migrations/_ulid.py`:
- Around line 56-57: The fallback hashing path can raise AttributeError if
iso_ts is a truthy non-str (e.g., an int); update the fallback to normalize
iso_ts to a string before encoding so hashlib.sha256 always receives bytes.
Specifically, in the block that computes digest and ts_ms (variables digest,
ts_ms) replace the direct .encode() on iso_ts with encoding of str(iso_ts or "")
(or an equivalent defensive normalization) so the migration's deterministic hash
path never depends on iso_ts being a str.

In `@Gradata/src/gradata/_migrations/device_uuid.py`:
- Around line 60-88: The temp-file name tmp (used when creating a PID-scoped
temp) can collide and lead to returning unpersisted new_did; change temp
creation in get_or_create_device_id to use a truly unique temporary file (e.g.,
use tempfile.mkstemp or append a uuid4-based suffix) created atomically (O_EXCL)
in the same directory as fpath, write new_did to that temp, fsync or ensure
close before attempting os.replace(fpath), and always cleanup the temp on error;
keep the existing checks using fpath.read_text, _is_valid, os.replace, and
os.unlink but replace the tmp naming logic so collisions cannot cause falling
back to returning new_did without persistence.

In `@Gradata/src/gradata/_mine_transcripts.py`:
- Around line 414-415: Replace the internal import of Brain with the package
entry point to avoid leaking module internals: change the import statement that
currently reads "from gradata.brain import Brain" to "from gradata import Brain"
in the module where _emit is imported; ensure all references to Brain remain
unchanged so the class is used via the public API.

In `@Gradata/src/gradata/_transcript_providers.py`:
- Around line 24-33: The duplicate session-id validator in
_transcript_providers.py (the _VALID_SESSION_RE and _validate_session_id
function) should be removed and the module should import and reuse the single
canonical validator from gradata._transcript to avoid drift; replace the local
_validate_session_id usage with an import like "from gradata._transcript import
_validate_session_id" (or the agreed public helper name), remove the local
_VALID_SESSION_RE and function definitions, and run tests to ensure all
references in _transcript_providers.py still call the imported
_validate_session_id correctly.
- Around line 56-63: The constructor currently nulled an invalid session_id (via
_validate_session_id) which causes _locate() to fall back to the "latest JSONL"
transcript; instead fail closed by raising an explicit error when a session_id
is provided but invalid (do not set session_id = None). Locate and update the
checks that call _validate_session_id and set session_id = None (e.g., in
ProviderTranscriptSource.__init__, any similar blocks around the second
occurrence and the block at 209-217) to raise ValueError (or a specific
InvalidSessionID error) with a clear message, and ensure _locate() is only
called when a valid session_id exists. Ensure the same behavior is applied
consistently to all three spots that currently null invalid IDs
(_validate_session_id, ProviderTranscriptSource.__init__, and the other similar
methods) so no fallback to unrelated transcripts occurs.

In `@Gradata/src/gradata/_transcript.py`:
- Around line 146-148: The load_turns() function currently appends any JSON
value from each line into turns, which can include scalars or arrays and break
downstream code that expects dicts (e.g., t.get(...)). Change the except/append
logic in load_turns() so that after json.loads(line) you only append the value
if isinstance(value, dict); otherwise skip (and optionally log/debug a warning).
Ensure turns remains typed list[dict] by not appending non-dict JSON values and
continue on JSONDecodeError as currently implemented.
- Around line 115-117: In log_turn, the current content truncation applies to
all roles (entry["content"] = content[:limit] if len(content) > limit else
content); change it so only assistant content is truncated: if role ==
"assistant" then apply the limit (use content[:limit] when longer), otherwise
assign the full content unchanged. Reference the log_turn function and the
variables role, content, limit, and entry["content"] when making this
conditional change.

In `@Gradata/src/gradata/brain.py`:
- Around line 914-920: Replace the unsalted sha256 of the task with a salt-aware
hash using the per-brain secret available as self._brain_salt: when constructing
"task_hash" (the sha256 call that currently hashes (task or "")), include
self._brain_salt in the hash input (e.g. concatenate or use a delimiter) before
encoding and hashing, preserving the hex digest truncation [:16]; update the
expression that builds "task_hash" so it uses self._brain_salt + (task or "")
(or similar) to salt the hash while leaving the rest of the logic unchanged.

In `@Gradata/src/gradata/enhancements/rule_synthesizer.py`:
- Around line 228-249: _call_gemini currently accepts a timeout but never
applies it to the Gemini request; update the GenerateContentConfig creation in
_call_gemini to include http_options using genai_types.HttpOptions with the
timeout converted to milliseconds (int(timeout * 1000)) so the per-call timeout
is enforced by the Gemini client.

---

Outside diff comments:
In `@Gradata/src/gradata/enhancements/graduation/agent_graduation.py`:
- Around line 300-320: The code should treat non-object JSON payloads as
corrupt: after json.loads(profile_path.read_text(...)) in the loader inside
AgentProfile creation, validate that the resulting data is a dict (e.g., if not
isinstance(data, dict) raise ValueError("profile.json must be an object")); then
extend the exception handler (the try/except around json.loads and building
AgentProfile in agent_graduation.py) to also catch ValueError (or add ValueError
to the existing except tuple) so that arrays/null/other shapes go to the
quarantine/backup path; reference the variables/profile construction lines
around AgentProfile, profile_path, parse_lessons, and self._profiles to locate
the change.
- Around line 558-573: The rejection branch currently applies MISFIRE_PENALTY to
lesson.confidence for every lesson regardless of category; add the same
category_matches guard used above so misfire penalties only affect lessons whose
category matches the normalized edited category (respecting the legacy
norm_edit_cat empty behavior). Concretely, modify the elif outcome == "rejected"
branch around lesson.confidence = max(0.0, lesson.confidence + MISFIRE_PENALTY)
to only execute that line when category_matches is true.

---

Duplicate comments:
In `@Gradata/src/gradata/_file_lock.py`:
- Line 149: The _lock_posix retry path is logging a full warning+traceback on
every retry which spams logs; in the _lock_posix function replace the per-retry
logger.warning call with logger.debug (preserving exc_info=True) to match the
Windows path, or alternatively emit a single logger.warning only when the
overall timeout is exceeded (track the start time in _lock_posix, log once on
timeout and raise). Update the call site referencing _lock_posix and the
existing logger variable so only debug-level messages appear during normal
contention and a single warning is emitted on timeout.

In `@Gradata/src/gradata/_transcript.py`:
- Around line 63-68: The _safe_int_env function currently returns parsed ints
which may be negative; update it to clamp results to a non-negative minimum by
returning max(int(os.environ.get(key, str(default))), 0) (still catching
TypeError/ValueError and returning max(default, 0)), so any callers reading
environment values like GRADATA_TRANSCRIPT_TTL_DAYS or truncate counts receive a
non-negative integer and cannot produce future cutoff times or negative slicing
behavior.

In `@Gradata/src/gradata/enhancements/rule_synthesizer.py`:
- Around line 128-131: The cached payload path currently returns
_strip_cache_header(raw) after _is_valid_cache_payload(raw) but skips the
minimum-block-size guard used for fresh synthesis; change the cache-return
branch to strip the header first (call _strip_cache_header(raw)), extract the
synthesized block (the <brain-wisdom> body or equivalent), and enforce the same
len(block) >= 50 check used in the fresh-synthesis flow—if the block is too
short, treat the cache as invalid and return None; apply the same check to the
other cache-return site noted (the other _strip_cache_header(raw) return).
🪄 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: 50696fe5-c0d2-4da8-b1ba-ea6324cef720

📥 Commits

Reviewing files that changed from the base of the PR and between 242c408 and 112e2cf.

⛔ Files ignored due to path filters (1)
  • Gradata/uv.lock is excluded by !**/*.lock
📒 Files selected for processing (19)
  • Gradata/migrations/supabase/014_corrections_unique.sql
  • Gradata/migrations/supabase/015_events_unique.sql
  • Gradata/src/gradata/_atomic.py
  • Gradata/src/gradata/_events.py
  • Gradata/src/gradata/_file_lock.py
  • Gradata/src/gradata/_migrations/_ulid.py
  • Gradata/src/gradata/_migrations/device_uuid.py
  • Gradata/src/gradata/_mine_transcripts.py
  • Gradata/src/gradata/_transcript.py
  • Gradata/src/gradata/_transcript_providers.py
  • Gradata/src/gradata/brain.py
  • Gradata/src/gradata/contrib/enhancements/quality_gates.py
  • Gradata/src/gradata/enhancements/graduation/agent_graduation.py
  • Gradata/src/gradata/enhancements/observation_hooks.py
  • Gradata/src/gradata/enhancements/pipeline_rewriter.py
  • Gradata/src/gradata/enhancements/rule_synthesizer.py
  • Gradata/tests/test_agent_graduation.py
  • Gradata/tests/test_emit_pii_redaction.py
  • Gradata/tests/test_rule_synthesizer.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
Gradata/**/*.py

📄 CodeRabbit inference engine (Gradata/AGENTS.md)

Python project must target Python 3.11+

Use from gradata import Brain as the public entry point

Files:

  • Gradata/src/gradata/enhancements/observation_hooks.py
  • Gradata/src/gradata/_migrations/_ulid.py
  • Gradata/src/gradata/_migrations/device_uuid.py
  • Gradata/src/gradata/_atomic.py
  • Gradata/src/gradata/_file_lock.py
  • Gradata/src/gradata/contrib/enhancements/quality_gates.py
  • Gradata/src/gradata/enhancements/pipeline_rewriter.py
  • Gradata/src/gradata/_transcript.py
  • Gradata/src/gradata/enhancements/rule_synthesizer.py
  • Gradata/src/gradata/_mine_transcripts.py
  • Gradata/src/gradata/_transcript_providers.py
  • Gradata/src/gradata/enhancements/graduation/agent_graduation.py
  • Gradata/src/gradata/_events.py
  • Gradata/src/gradata/brain.py
Gradata/src/**/*.py

📄 CodeRabbit inference engine (Gradata/AGENTS.md)

Guard imports of optional dependencies with try / except ImportError at the call site, never at module level

Lower layers must never import from higher layers (Layer 0 → Layer 2 violations are bugs)

Always write rule_graph.json atomically using the atomic-write helper to prevent corruption on crashes

Never use bare except: pass — use typed exceptions or at minimum logger.warning(..., exc_info=True)

Never assume thread-safety on Brain — document as NOT thread-safe and ensure process-level coordination for concurrent writes

Do not import from or reference out-of-scope sibling directories (../Sprites/, ../Hausgem/) in public gradata/* code

Never leak private-sibling paths, emails, OneDrive paths, or Sprites-specific examples in public docs and code

Files:

  • Gradata/src/gradata/enhancements/observation_hooks.py
  • Gradata/src/gradata/_migrations/_ulid.py
  • Gradata/src/gradata/_migrations/device_uuid.py
  • Gradata/src/gradata/_atomic.py
  • Gradata/src/gradata/_file_lock.py
  • Gradata/src/gradata/contrib/enhancements/quality_gates.py
  • Gradata/src/gradata/enhancements/pipeline_rewriter.py
  • Gradata/src/gradata/_transcript.py
  • Gradata/src/gradata/enhancements/rule_synthesizer.py
  • Gradata/src/gradata/_mine_transcripts.py
  • Gradata/src/gradata/_transcript_providers.py
  • Gradata/src/gradata/enhancements/graduation/agent_graduation.py
  • Gradata/src/gradata/_events.py
  • Gradata/src/gradata/brain.py
🧠 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.
Learnt from: CR
Repo: Gradata/gradata

Timestamp: 2026-04-30T17:34:39.538Z
Learning: Read the target file in full before modifying it
Learnt from: CR
Repo: Gradata/gradata

Timestamp: 2026-04-30T17:34:39.538Z
Learning: Add a failing test that reproduces a bug before fixing it; the test must remain green after the fix
Learnt from: CR
Repo: Gradata/gradata

Timestamp: 2026-04-30T17:34:39.538Z
Learning: Run the smallest relevant test after each change (e.g., `pytest tests/test_brain.py -xvs`) to validate quickly
Learnt from: CR
Repo: Gradata/gradata

Timestamp: 2026-04-30T17:34:39.538Z
Learning: Honor the layering rule and flag Layer 0 → 2 import violations in PR descriptions
Learnt from: CR
Repo: Gradata/gradata

Timestamp: 2026-04-30T17:34:39.538Z
Learning: Enforce single source of truth — pick one when duplicate-purpose modules exist and document the migration; do not add a third
Learnt from: CR
Repo: Gradata/gradata

Timestamp: 2026-04-30T17:34:39.538Z
Learning: Use commit message format: `<type>(<scope>): <imperative description>` with types: feat, fix, docs, refactor, perf, test, chore, revert
Learnt from: CR
Repo: Gradata/gradata

Timestamp: 2026-04-30T17:34:39.538Z
Learning: PR descriptions must include: Summary (1-3 sentences), Test plan, Layering check, and Risk assessment
Learnt from: CR
Repo: Gradata/gradata

Timestamp: 2026-04-30T17:34:39.538Z
Learning: For multi-file changes (3+ files, cross-module refactor, schema changes), use council skill coordination; for single-file edits and 1-2 line fixes, proceed directly
Learnt from: CR
Repo: Gradata/gradata

Timestamp: 2026-04-30T17:34:39.538Z
Learning: Never push to `origin/main` directly — all changes must go through PR review
📚 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/_transcript.py
  • Gradata/src/gradata/brain.py
🪛 Ruff (0.15.12)
Gradata/src/gradata/_atomic.py

[warning] 29-32: Use contextlib.suppress(FileNotFoundError) instead of try-except-pass

Replace try-except-pass with with contextlib.suppress(FileNotFoundError): ...

(SIM105)

Gradata/src/gradata/enhancements/rule_synthesizer.py

[warning] 109-111: Return the negated condition directly

Inline condition

(SIM103)

🔇 Additional comments (10)
Gradata/src/gradata/enhancements/pipeline_rewriter.py (3)

80-83: Good hardening on RULE_FAILURE description normalization.

Line 80–82 now safely handles legacy/non-string payloads before .strip(), which avoids analyzer crashes on malformed events.


128-136: Proposal construction refactor is behavior-preserving and clearer.

The multiline append(ThresholdProposal(...)) reshaping keeps semantics intact while improving readability of rationale/evidence fields.

Also applies to: 151-163, 168-180, 185-197


217-220: ADR intro line reflow looks good.

This is a clean non-functional formatting update and keeps the generated markdown content unchanged.

Gradata/src/gradata/contrib/enhancements/quality_gates.py (5)

58-58: Formatting-only change; no functional concern.


323-343: Whitespace/signature formatting updates only; no issue to raise.


20-25: Good logging initialization for this module.

Adding a module-scoped logger here is a solid foundation for replacing silent suppression paths with actionable diagnostics.


254-257: Score/failure computation refactor preserves behavior.

The condensed expressions are still clear and keep the same evaluation semantics.


360-369: SQLite access path is now safer and debuggable.

Using with sqlite3.connect(...) prevents connection leaks on failure, and logger.warning(..., exc_info=True) removes silent failure behavior while preserving fallback behavior.

Gradata/src/gradata/_migrations/_ulid.py (1)

29-35: ULID core construction looks correct.

Timestamp masking and 10+16 Crockford encoding are implemented cleanly.

Gradata/src/gradata/_atomic.py (1)

10-27: Nice crash-safe write sequence.

mkstemp + file fsync + os.replace + parent-directory fsync is the right pattern here.

Comment on lines +14 to +16
DELETE FROM corrections a
USING corrections b
WHERE a.brain_id = b.brain_id
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

: "${DATABASE_URL:?Set DATABASE_URL to a Postgres connection string}"

echo "Tables named 'corrections' across schemas:"
psql "$DATABASE_URL" -v ON_ERROR_STOP=1 -c "
SELECT n.nspname AS schema_name, c.relname AS table_name
FROM pg_class c
JOIN pg_namespace n ON n.oid = c.relnamespace
WHERE c.relkind = 'r' AND c.relname = 'corrections'
ORDER BY 1,2;
"

echo "Unique constraints on all 'corrections' tables:"
psql "$DATABASE_URL" -v ON_ERROR_STOP=1 -c "
SELECT n.nspname AS schema_name, t.relname AS table_name, con.conname, con.conkey
FROM pg_constraint con
JOIN pg_class t ON t.oid = con.conrelid
JOIN pg_namespace n ON n.oid = t.relnamespace
WHERE con.contype = 'u' AND t.relname = 'corrections'
ORDER BY 1,2,3;
"

Repository: Gradata/gradata

Length of output: 141


🏁 Script executed:

#!/bin/bash
# First, locate the migration files
find . -name "*014_corrections_unique*" -o -name "*015*" | grep -E "migrations.*sql" | head -20

Repository: Gradata/gradata

Length of output: 168


🏁 Script executed:

#!/bin/bash
# Read the full migration file in question
if [ -f "Gradata/migrations/supabase/014_corrections_unique.sql" ]; then
  echo "=== File: 014_corrections_unique.sql ==="
  cat -n "Gradata/migrations/supabase/014_corrections_unique.sql"
else
  # Try to find it
  find . -name "014_corrections_unique.sql" -exec cat -n {} \;
fi

Repository: Gradata/gradata

Length of output: 1830


🏁 Script executed:

#!/bin/bash
# Also find and read migration 015 for comparison
find . -name "*015*" -type f | grep -E "\.sql$" | head -5 | while read f; do
  echo "=== File: $f ==="
  cat -n "$f"
done

Repository: Gradata/gradata

Length of output: 1803


Apply schema-qualification hardening to both corrections and events migrations to ensure idempotency across search_path variations.

Both migrations 014 and 015 share a schema-resolution vulnerability: unqualified table names (corrections, events) resolve based on search_path, and the catalog guard in the IF NOT EXISTS block matches on relname only—without filtering by schema namespace. In multi-schema environments or if search_path is reconfigured, the DELETE may target the wrong table while the constraint check examines a different schema's version, breaking idempotency.

Corrections needed in 014_corrections_unique.sql (lines 14–15, 27, 39) and 015_events_unique.sql (lines 14–15, 27, 39):

Schema-qualified fix pattern
-DELETE FROM corrections a
-USING corrections b
+DELETE FROM public.corrections a
+USING public.corrections b
 WHERE a.brain_id = b.brain_id
@@
     FROM pg_constraint c
     JOIN pg_class t ON t.oid = c.conrelid
+    JOIN pg_namespace n ON n.oid = t.relnamespace
     WHERE t.relname = 'corrections'
+      AND n.nspname = 'public'
       AND c.contype = 'u'
@@
-    ALTER TABLE corrections
+    ALTER TABLE public.corrections
       ADD CONSTRAINT corrections_brain_session_description_unique
📝 Committable suggestion

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

Suggested change
DELETE FROM corrections a
USING corrections b
WHERE a.brain_id = b.brain_id
DELETE FROM public.corrections a
USING public.corrections b
WHERE a.brain_id = b.brain_id
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Gradata/migrations/supabase/014_corrections_unique.sql` around lines 14 - 16,
The migration uses unqualified table names (corrections, events) in the DELETE
... USING ... WHERE statements and in the IF NOT EXISTS catalog guard, so update
those statements to fully qualify the target tables with the current schema and
make the catalog check filter by namespace; specifically, replace the
unqualified "DELETE FROM corrections a USING corrections b WHERE a.brain_id =
b.brain_id" (and the equivalent in events) with schema-qualified identifiers for
the same schema the migration runs in, and change the IF NOT EXISTS/pg_class
check to join pg_namespace (or compare nspname = current_schema()) so relname is
matched within the same namespace as the DELETE, ensuring idempotency across
search_path variations.

Comment on lines +56 to +57
digest = hashlib.sha256((iso_ts or "").encode("utf-8")).digest()
ts_ms = int.from_bytes(digest[:6], "big") & ((1 << 48) - 1)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In Python 3.11, does calling .encode("utf-8")on anintraise an exception, and is normalizing unknown input viastr(value)before.encode() the correct safe pattern?

💡 Result:

Yes, calling .encode("utf-8") on an int raises AttributeError: 'int' object has no attribute 'encode' in Python 3.11. The encode method is defined only on str objects, as per the official documentation. Integers lack this method, leading to the attribute error when attempted. Yes, normalizing unknown input via str(value) before .encode("utf-8") is the correct safe pattern. The built-in str function converts any object (including int, float, None, etc.) to its string representation using str (falling back to repr if needed), producing a valid str that can then be safely encoded to bytes. This handles mixed-type inputs common in data processing, APIs, or logging without type-specific checks. Example: value = 123 bytes_data = str(value).encode('utf-8') # b'123' This pattern is widely recommended in Python communities for robust encoding of heterogeneous data.

Citations:


🏁 Script executed:

cat -n Gradata/src/gradata/_migrations/_ulid.py | head -80

Repository: Gradata/gradata

Length of output: 2696


Normalize iso_ts before encoding in fallback path.

On parse failure, line 56 can still raise AttributeError if iso_ts is a truthy non-str (e.g., 123), because .encode() is called on it directly. This defeats the migration's malformed-input fallback, which is designed to always produce a deterministic hash. Although the function signature declares iso_ts: str, the exception handler should be defensive against type mismatches.

Proposed fix
-        digest = hashlib.sha256((iso_ts or "").encode("utf-8")).digest()
+        normalized = "" if iso_ts is None else str(iso_ts)
+        digest = hashlib.sha256(normalized.encode("utf-8")).digest()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Gradata/src/gradata/_migrations/_ulid.py` around lines 56 - 57, The fallback
hashing path can raise AttributeError if iso_ts is a truthy non-str (e.g., an
int); update the fallback to normalize iso_ts to a string before encoding so
hashlib.sha256 always receives bytes. Specifically, in the block that computes
digest and ts_ms (variables digest, ts_ms) replace the direct .encode() on
iso_ts with encoding of str(iso_ts or "") (or an equivalent defensive
normalization) so the migration's deterministic hash path never depends on
iso_ts being a str.

Comment on lines +60 to +88
tmp = brain / f".device_id.tmp.{os.getpid()}"
flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL
try:
fd = os.open(tmp, flags, 0o644)
except FileExistsError:
# Extremely unlikely PID collision; fall through to disk read.
logger.warning('Suppressed exception in get_or_create_device_id', exc_info=True)
else:
try:
with os.fdopen(fd, "w", encoding="utf-8") as fh:
fh.write(new_did)
# If the existing file is invalid OR missing, replace it.
existing_did = (
fpath.read_text(encoding="utf-8").strip() if fpath.exists() else ""
)
if not existing_did or not _is_valid(existing_did):
os.replace(tmp, fpath)
else:
os.unlink(tmp)
except Exception:
with contextlib.suppress(OSError):
os.unlink(tmp)
raise

if fpath.exists():
did = fpath.read_text(encoding="utf-8").strip()
if _is_valid(did):
return did
return new_did
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make temp-file creation truly unique to avoid returning unpersisted device_id.

At Line 60 and Line 64–88, a PID-scoped temp path can collide (same-process concurrency or stale temp file). In that path, the function can reach Line 88 and return new_did even though it was never persisted, causing unstable identity across calls/restarts.

Proposed fix
+import tempfile
 ...
-    tmp = brain / f".device_id.tmp.{os.getpid()}"
-    flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL
-    try:
-        fd = os.open(tmp, flags, 0o644)
-    except FileExistsError:
-        # Extremely unlikely PID collision; fall through to disk read.
-        logger.warning('Suppressed exception in get_or_create_device_id', exc_info=True)
-    else:
-        try:
-            with os.fdopen(fd, "w", encoding="utf-8") as fh:
-                fh.write(new_did)
+    fd, tmp_name = tempfile.mkstemp(prefix=".device_id.tmp.", dir=brain)
+    tmp = Path(tmp_name)
+    try:
+        with os.fdopen(fd, "w", encoding="utf-8") as fh:
+            fh.write(new_did)
             # If the existing file is invalid OR missing, replace it.
             existing_did = (
                 fpath.read_text(encoding="utf-8").strip() if fpath.exists() else ""
             )
             if not existing_did or not _is_valid(existing_did):
                 os.replace(tmp, fpath)
             else:
                 os.unlink(tmp)
-        except Exception:
-            with contextlib.suppress(OSError):
-                os.unlink(tmp)
-            raise
+    except Exception:
+        with contextlib.suppress(OSError):
+            os.unlink(tmp)
+        raise
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Gradata/src/gradata/_migrations/device_uuid.py` around lines 60 - 88, The
temp-file name tmp (used when creating a PID-scoped temp) can collide and lead
to returning unpersisted new_did; change temp creation in
get_or_create_device_id to use a truly unique temporary file (e.g., use
tempfile.mkstemp or append a uuid4-based suffix) created atomically (O_EXCL) in
the same directory as fpath, write new_did to that temp, fsync or ensure close
before attempting os.replace(fpath), and always cleanup the temp on error; keep
the existing checks using fpath.read_text, _is_valid, os.replace, and os.unlink
but replace the tmp naming logic so collisions cannot cause falling back to
returning new_did without persistence.

Comment on lines 414 to 415
from gradata._events import emit as _emit
from gradata.brain import Brain
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Use the public Brain entry point here.

This module now imports Brain from gradata.brain directly. Please keep SDK consumers on the package entry point so internal module moves do not leak into call sites.

♻️ Suggested change
-    from gradata.brain import Brain
+    from gradata import Brain

As per coding guidelines, "Use from gradata import Brain as the public entry point".

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

In `@Gradata/src/gradata/_mine_transcripts.py` around lines 414 - 415, Replace the
internal import of Brain with the package entry point to avoid leaking module
internals: change the import statement that currently reads "from gradata.brain
import Brain" to "from gradata import Brain" in the module where _emit is
imported; ensure all references to Brain remain unchanged so the class is used
via the public API.

Comment on lines +24 to +33
_VALID_SESSION_RE = re.compile(r"^[A-Za-z0-9_\-]+$")


def _validate_session_id(session_id: str | None) -> bool:
"""Same path-traversal guard as gradata._transcript._validate_session_id."""
if not session_id or not isinstance(session_id, str):
return False
if ".." in session_id or "/" in session_id or "\\" in session_id:
return False
return bool(_VALID_SESSION_RE.match(session_id))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Use one shared session_id validator across transcript modules.

This validator duplicates the one in Gradata/src/gradata/_transcript.py and can drift over time. Extract/import a single helper to keep security checks consistent.

Based on learnings: “Enforce single source of truth — pick one when duplicate-purpose modules exist and document the migration; do not add a third”.

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

In `@Gradata/src/gradata/_transcript_providers.py` around lines 24 - 33, The
duplicate session-id validator in _transcript_providers.py (the
_VALID_SESSION_RE and _validate_session_id function) should be removed and the
module should import and reuse the single canonical validator from
gradata._transcript to avoid drift; replace the local _validate_session_id usage
with an import like "from gradata._transcript import _validate_session_id" (or
the agreed public helper name), remove the local _VALID_SESSION_RE and function
definitions, and run tests to ensure all references in _transcript_providers.py
still call the imported _validate_session_id correctly.

Comment on lines +56 to +63
def __init__(self, session_id: str | None) -> None:
# Reject path-traversal session ids before they get joined into a path.
if session_id is not None and not _validate_session_id(session_id):
_log.debug("ProviderTranscriptSource rejecting invalid session_id")
session_id = None
self._session_id = session_id
self._path: Path | None = self._locate()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fail closed when session_id is invalid (avoid unrelated transcript fallback).

At Line 58, an invalid session_id is nulled, and _locate() then uses “latest JSONL” fallback. That can bind retroactive sweep to the wrong session transcript.

Suggested fix
 class ProviderTranscriptSource:
@@
     def __init__(self, session_id: str | None) -> None:
+        self._invalid_session_id = False
         # Reject path-traversal session ids before they get joined into a path.
         if session_id is not None and not _validate_session_id(session_id):
             _log.debug("ProviderTranscriptSource rejecting invalid session_id")
-            session_id = None
+            self._invalid_session_id = True
+            session_id = None
         self._session_id = session_id
         self._path: Path | None = self._locate()
@@
     def _locate(self) -> Path | None:
+        if self._invalid_session_id:
+            return None
         projects = Path.home() / ".claude" / "projects"

Also applies to: 79-87, 209-217

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

In `@Gradata/src/gradata/_transcript_providers.py` around lines 56 - 63, The
constructor currently nulled an invalid session_id (via _validate_session_id)
which causes _locate() to fall back to the "latest JSONL" transcript; instead
fail closed by raising an explicit error when a session_id is provided but
invalid (do not set session_id = None). Locate and update the checks that call
_validate_session_id and set session_id = None (e.g., in
ProviderTranscriptSource.__init__, any similar blocks around the second
occurrence and the block at 209-217) to raise ValueError (or a specific
InvalidSessionID error) with a clear message, and ensure _locate() is only
called when a valid session_id exists. Ensure the same behavior is applied
consistently to all three spots that currently null invalid IDs
(_validate_session_id, ProviderTranscriptSource.__init__, and the other similar
methods) so no fallback to unrelated transcripts occurs.

Comment on lines +115 to +117
elif content is not None:
entry["content"] = content[:limit] if len(content) > limit else content
else:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Do not truncate user turns in log_turn.

Lines 115-117 truncate all roles, including user. That can drop implicit-feedback matches from long user prompts and conflicts with the policy note that truncation is for assistant content.

Suggested fix
-    elif content is not None:
-        entry["content"] = content[:limit] if len(content) > limit else content
+    elif content is not None:
+        if role == "assistant":
+            entry["content"] = content[:limit] if len(content) > limit else content
+        else:
+            entry["content"] = content
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Gradata/src/gradata/_transcript.py` around lines 115 - 117, In log_turn, the
current content truncation applies to all roles (entry["content"] =
content[:limit] if len(content) > limit else content); change it so only
assistant content is truncated: if role == "assistant" then apply the limit (use
content[:limit] when longer), otherwise assign the full content unchanged.
Reference the log_turn function and the variables role, content, limit, and
entry["content"] when making this conditional change.

Comment on lines +146 to +148
try:
turns.append(json.loads(line))
except json.JSONDecodeError:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Enforce load_turns() return contract (list[dict]).

Line 147 appends any JSON type, not just objects. A valid JSON scalar/array line can leak into turns and break downstream t.get(...) usage.

Suggested fix
                 try:
-                    turns.append(json.loads(line))
+                    entry = json.loads(line)
+                    if isinstance(entry, dict):
+                        turns.append(entry)
                 except json.JSONDecodeError:
                     continue
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Gradata/src/gradata/_transcript.py` around lines 146 - 148, The load_turns()
function currently appends any JSON value from each line into turns, which can
include scalars or arrays and break downstream code that expects dicts (e.g.,
t.get(...)). Change the except/append logic in load_turns() so that after
json.loads(line) you only append the value if isinstance(value, dict); otherwise
skip (and optionally log/debug a warning). Ensure turns remains typed list[dict]
by not appending non-dict JSON values and continue on JSONDecodeError as
currently implemented.

Comment on lines +914 to +920
# SECURITY: don't persist raw task text — it may
# contain PII or proprietary content. Emit a stable
# sha256 prefix so downstream effectiveness tracking
# can correlate without storing the original.
"task_hash": __import__("hashlib")
.sha256((task or "").encode("utf-8"))
.hexdigest()[:16],
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Salt task_hash with the per-brain secret.

This is better than storing raw task text, but an unsalted hash of low-entropy prompts is still guessable and lets different brains correlate the same task. self._brain_salt is already available in this class, so this can stay locally correlatable without being globally reversible.

🔐 Suggested change
-                        "task_hash": __import__("hashlib")
-                        .sha256((task or "").encode("utf-8"))
+                        "task_hash": __import__("hashlib")
+                        .sha256(f"{self._brain_salt}:{task or ''}".encode("utf-8"))
                         .hexdigest()[:16],
📝 Committable suggestion

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

Suggested change
# SECURITY: don't persist raw task text — it may
# contain PII or proprietary content. Emit a stable
# sha256 prefix so downstream effectiveness tracking
# can correlate without storing the original.
"task_hash": __import__("hashlib")
.sha256((task or "").encode("utf-8"))
.hexdigest()[:16],
# SECURITY: don't persist raw task text — it may
# contain PII or proprietary content. Emit a stable
# sha256 prefix so downstream effectiveness tracking
# can correlate without storing the original.
"task_hash": __import__("hashlib")
.sha256(f"{self._brain_salt}:{task or ''}".encode("utf-8"))
.hexdigest()[:16],
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Gradata/src/gradata/brain.py` around lines 914 - 920, Replace the unsalted
sha256 of the task with a salt-aware hash using the per-brain secret available
as self._brain_salt: when constructing "task_hash" (the sha256 call that
currently hashes (task or "")), include self._brain_salt in the hash input (e.g.
concatenate or use a delimiter) before encoding and hashing, preserving the hex
digest truncation [:16]; update the expression that builds "task_hash" so it
uses self._brain_salt + (task or "") (or similar) to salt the hash while leaving
the rest of the logic unchanged.

Comment on lines +228 to +249
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
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

For google-genai Python SDK v1.0.0, what is the documented way to set request timeout for client.models.generate_content(...) (client-level or per-call)?

💡 Result:

The documented way to set request timeout for client.models.generate_content(...) in the google-genai Python SDK v1.0.0 is both at the client level and per-call. Client-level: Pass http_options=types.HttpOptions(timeout=milliseconds) when creating the Client. Example: from google import genai from google.genai import types client = genai.Client( api_key='YOUR_API_KEY', http_options=types.HttpOptions(timeout=30000) # 30 seconds ) Per-call: Pass config=types.GenerateContentConfig(http_options=types.HttpOptions(timeout=milliseconds)) in the generate_content call. Example: response = client.models.generate_content( model='gemini-2.5-flash', contents='Why is the sky blue?', config=types.GenerateContentConfig( http_options=types.HttpOptions(timeout=120000) # 2 minutes ) ) Timeout is specified in milliseconds. This applies to HTTP request timeouts, including for generate_content calls, as confirmed by SDK source code handling of HttpOptions.timeout and documentation examples.

Citations:


Pass timeout to the Gemini API call using http_options in the config.

The timeout parameter (line 228) is accepted but never applied to the API request, allowing slow Gemini calls to block indefinitely instead of failing fast. Use genai_types.HttpOptions(timeout=...) in the GenerateContentConfig to set the per-call timeout in milliseconds (convert the timeout float from seconds):

config = genai_types.GenerateContentConfig(
    system_instruction=system,
    max_output_tokens=max_tokens,
    http_options=genai_types.HttpOptions(timeout=int(timeout * 1000)),
)
🤖 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 228 - 249,
_call_gemini currently accepts a timeout but never applies it to the Gemini
request; update the GenerateContentConfig creation in _call_gemini to include
http_options using genai_types.HttpOptions with the timeout converted to
milliseconds (int(timeout * 1000)) so the per-call timeout is enforced by the
Gemini client.

@Gradata
Copy link
Copy Markdown
Owner Author

Gradata commented May 1, 2026

Superseded by clean rebase. Original branch had 41 stale m1 commits already merged via #144; reopened on rebase/council-phase-b with just the Phase B P0 fixes.

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

Labels

bug Something isn't working refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant