feat(sync): retire hook-based cloud sync now that write-through is live (day 4 of #194)#201
Conversation
📝 WalkthroughSummary
WalkthroughThis PR adds a feature gate to control legacy cloud sync behavior in the session close hook. The ChangesWrite-through sync feature gate
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.20.0)OpenGrep fatal error (exit code 2): �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m Comment |
…ve (day 4 of #194) Brain.correct() write-through (PR #200) is now the default cloud sync path. The session_close hook's legacy cloud_sync_tick was double-writing every correction. Default behavior: session_close skips cloud_sync_tick. Lesson graduation, pipeline, tree consolidation, lesson_applications resolution all still run as before — only the cloud sync portion is gated. Opt-out: GRADATA_DISABLE_WRITE_THROUGH=1 restores the legacy hook-based cloud sync path AND disables write-through. The two paths are mutually exclusive: only one ever runs. Untouched: - daemon /sync (dashboard 'Sync Now' button) - /api/v1/sync (bulk endpoint, used by Brain.close() drain) - /home/olive/.gradata/sync_cron.py (safety-net cron, keep for 7-day soak) Follow-up: after 7 days of clean write-through telemetry in cloud, the cron and legacy /api/v1/sync code paths can be removed in a separate PR.
Gradata
force-pushed
the
feat/sync-cleanup-day4-fresh
branch
from
a48eb2d to
8ce46a7
Compare
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Gradata/tests/test_session_close_write_through_gate.py`:
- Around line 21-24: Remove the redundant tuple expression wrapping
mock_tick.assert_not_called() in the test; replace the tuple
(mock_tick.assert_not_called(), ("session_close must NOT call cloud_sync_tick
when write-through is on")) with a direct assertion call and, if a message is
desired, use the testing framework's assertion API (i.e., call
mock_tick.assert_not_called() directly or use an explicit assert with the
message) so the test invokes mock_tick.assert_not_called() rather than creating
a no-op tuple; locate the call near the session_close test referencing mock_tick
and cloud_sync_tick.
🪄 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: fb489615-3956-47bb-b065-284a611a7b5a
📒 Files selected for processing (2)
Gradata/src/gradata/hooks/session_close.pyGradata/tests/test_session_close_write_through_gate.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: pytest macos-latest / py3.11
- GitHub Check: pytest ubuntu-latest / py3.11
- GitHub Check: pytest windows-latest / py3.11
- GitHub Check: pytest windows-latest / py3.12
- GitHub Check: pytest macos-latest / py3.12
- GitHub Check: pytest ubuntu-latest / py3.12
- GitHub Check: pytest (py3.12)
- GitHub Check: pytest (py3.11)
🧰 Additional context used
📓 Path-based instructions (2)
Gradata/tests/**/*.py
📄 CodeRabbit inference engine (Gradata/AGENTS.md)
Gradata/tests/**/*.py: SetBRAIN_DIRenvironment variable viatmp_pathin conftest.py for test isolation — ensure_paths.pymodule cache refreshes when callingBrain.init()directly inside tests
Add unit tests intests/test_*.pyfor every CI push without LLM calls (deterministic); mark integration tests with@pytest.mark.integrationand skip them by default (they hit real LLM APIs)
Files:
Gradata/tests/test_session_close_write_through_gate.py
Gradata/src/**/*.py
📄 CodeRabbit inference engine (Gradata/AGENTS.md)
Gradata/src/**/*.py: Prefersentence-transformersfor local embeddings,google-genaifor Gemini embeddings,cryptographyfor AES-GCM encrypted system.db,bm25sfor BM25 rule ranking, andmem0aifor external memory adapters — guard all optional dependency imports withtry / except ImportErrorat the call site, never at module level
Maintain strict layering: Layer 0 (Primitives: _types.py, _db.py, _events.py, _paths.py, _file_lock.py; Patterns: contrib/patterns/) must never import from Layer 1 (Enhancements: enhancements/, rules/) or Layer 2 (Public API: brain.py, cli.py, daemon.py, mcp_server.py)
Never use bareexcept: pass— use typed exceptions or at minimumlogger.warning(...)withexc_info=Trueto avoid silent failure in a memory product
Never import from out-of-scope sibling directories../Sprites/or../Hausgem/withingradata/*code — that is a layering bug
Never leak private-sibling paths into public docs/code — no references to../Sprites/,../Hausgem/, email addresses, OneDrive paths, or Sprites-specific examples from insidegradata/*
Use atomic-write helper when writing JSON files to prevent corruption from mid-write crashes
Files:
Gradata/src/gradata/hooks/session_close.py
🔇 Additional comments (2)
Gradata/src/gradata/hooks/session_close.py (1)
281-285: LGTM!Also applies to: 289-295
Gradata/tests/test_session_close_write_through_gate.py (1)
1-20: LGTM!Also applies to: 27-43
| ( | ||
| mock_tick.assert_not_called(), | ||
| ("session_close must NOT call cloud_sync_tick when write-through is on"), | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import ast
from pathlib import Path
p = Path("Gradata/tests/test_session_close_write_through_gate.py")
tree = ast.parse(p.read_text(encoding="utf-8"))
for node in ast.walk(tree):
if isinstance(node, ast.Expr) and isinstance(node.value, ast.Tuple):
print(f"{p}:{node.lineno}:{node.col_offset} tuple-expression statement")
PYRepository: Gradata/gradata
Length of output: 146
Remove the no-op tuple wrapper around assert_not_called().
This tuple expression serves no purpose and obfuscates the assertion.
Suggested diff
- (
- mock_tick.assert_not_called(),
- ("session_close must NOT call cloud_sync_tick when write-through is on"),
- )
+ mock_tick.assert_not_called()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Gradata/tests/test_session_close_write_through_gate.py` around lines 21 - 24,
Remove the redundant tuple expression wrapping mock_tick.assert_not_called() in
the test; replace the tuple (mock_tick.assert_not_called(), ("session_close must
NOT call cloud_sync_tick when write-through is on")) with a direct assertion
call and, if a message is desired, use the testing framework's assertion API
(i.e., call mock_tick.assert_not_called() directly or use an explicit assert
with the message) so the test invokes mock_tick.assert_not_called() rather than
creating a no-op tuple; locate the call near the session_close test referencing
mock_tick and cloud_sync_tick.
1 participant
Day 4 of #194 — gate session_close legacy cloud sync behind GRADATA_DISABLE_WRITE_THROUGH=1.
What
session_close._run_cloud_sync now skips cloud_sync_tick by default. With #200 merged, Brain.correct() write-through covers every correction — running the hook tick on top of that was double-writing.
Lesson graduation, pipeline, tree consolidation, lesson_applications resolution still run as before. Only the cloud sync line is gated.
Why
Pre-#194: cloud sync only fired from session_close hook → fragile + missed long-running agent sessions.
Post-#200: Brain.correct() write-through → every correction syncs immediately, no hooks required.
Day 4: stop the hook from also syncing the same data.
Opt-out
GRADATA_DISABLE_WRITE_THROUGH=1 restores legacy behavior. The two paths are mutually exclusive.
Untouched
Tests
tests/test_session_close_write_through_gate.py × 3: default skip, opt-out invoke, no-api-key skip.
Follow-up
After 7 days of clean write-through telemetry, file follow-up PR to remove cron + /api/v1/sync legacy code.
Depends on
#200 (still draft, write-through wire-up). Mark this Ready for Review after #200 merges.