test: cover Stop session graduation sweep#263
Conversation
![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.
📝 Walkthrough
WalkthroughThis PR refactors AGENTS.md export logic into a reusable helper function and extends post-processing to export agent rules when patterns are lifted, not just when lessons graduate. A new integration test validates the end-to-end behavior by seeding correction patterns and asserting the expected promoted and lifted rule entries. ChangesPattern-lifted AGENTS.md export
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.22.0)OpenGrep fatal error (exit code 2): �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/src/gradata/hooks/session_close.py`:
- Around line 186-190: Replace the broad "except Exception as e" that wraps the
call to _export_agents_rules(brain_dir, lessons_path) with typed exception
handlers: catch expected failures like ImportError and OSError (e.g., "except
(ImportError, OSError) as e:") and call _log.warning("AGENTS.md pipeline export
skipped: %s", e, exc_info=True); optionally keep a narrower fallback that also
logs as warning (not silently) for any other Exception to avoid swallowing
errors. Ensure you modify the block around the _export_agents_rules invocation
and update _log calls accordingly.
In `@Gradata/tests/test_session_close_write_through_gate.py`:
- Around line 83-86: Replace the direct monkeypatch of
session_close.resolve_brain_dir with the shared BRAIN_DIR test-isolation
pattern: set the BRAIN_DIR environment variable to the conftest-provided
tmp_path / BRAIN_DIR (via monkeypatch.setenv("BRAIN_DIR", str(tmp_path /
"BRAIN_DIR"))) so `_paths.py` is refreshed when tests call Brain.init(); remove
the monkeypatch.setattr(session_close, "resolve_brain_dir", ...) and ensure
tests use Brain.init() after setting BRAIN_DIR to pick up the proper cached path
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c13d5203-4ec9-41c4-882d-cbc32e2dce13
📒 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 (py3.11)
- GitHub Check: pytest (py3.12)
- GitHub Check: pytest ubuntu-latest / py3.11
- GitHub Check: pytest macos-latest / py3.12
- GitHub Check: pytest macos-latest / py3.11
- GitHub Check: pytest ubuntu-latest / py3.12
- GitHub Check: pytest windows-latest / py3.12
- GitHub Check: pytest windows-latest / py3.11
🧰 Additional context used
📓 Path-based instructions (2)
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
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
🧠 Learnings (4)
📓 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 PR: 0
File: Gradata/AGENTS.md:0-0
Timestamp: 2026-05-01T15:50:32.772Z
Learning: Ensure the 4 deterministic guarantees have tests: (1) Correction in → rule extracted out, (2) Rule retrieved/applied in subsequent session, (3) Contradicting evidence lowers confidence, (4) Stale rules decay below threshold
📚 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/tests/test_session_close_write_through_gate.py
📚 Learning: 2026-05-01T15:50:32.772Z
Learnt from: CR
Repo: Gradata/gradata PR: 0
File: Gradata/AGENTS.md:0-0
Timestamp: 2026-05-01T15:50:32.772Z
Learning: Applies to Gradata/tests/**/*.py : Set `BRAIN_DIR` environment variable via `tmp_path` in conftest.py for test isolation — ensure `_paths.py` module cache refreshes when calling `Brain.init()` directly inside tests
Applied to files:
Gradata/tests/test_session_close_write_through_gate.py
📚 Learning: 2026-05-01T15:50:32.772Z
Learnt from: CR
Repo: Gradata/gradata PR: 0
File: Gradata/AGENTS.md:0-0
Timestamp: 2026-05-01T15:50:32.772Z
Learning: Applies to Gradata/tests/**/*.py : Add unit tests in `tests/test_*.py` for every CI push without LLM calls (deterministic); mark integration tests with `pytest.mark.integration` and skip them by default (they hit real LLM APIs)
Applied to files:
Gradata/tests/test_session_close_write_through_gate.py
🔇 Additional comments (1)
Gradata/src/gradata/hooks/session_close.py (1)
160-168: LGTM!
| try: | ||
| _export_agents_rules(brain_dir, lessons_path) | ||
| except Exception as e: | ||
| _log.debug("AGENTS.md pipeline export skipped: %s", e, exc_info=True) | ||
| if ( |
There was a problem hiding this comment.
Raise pipeline export failures to warning level with typed exceptions.
Line 188 currently swallows all failures under except Exception and only logs at debug, which makes AGENTS.md write-through failures too easy to miss. Catch expected failure classes (ImportError, OSError, etc.) and log at warning level with exc_info=True.
Proposed fix
if result.graduated or result.patterns_lifted:
try:
_export_agents_rules(brain_dir, lessons_path)
- except Exception as e:
- _log.debug("AGENTS.md pipeline export skipped: %s", e, exc_info=True)
+ except ImportError as e:
+ _log.warning(
+ "AGENTS.md pipeline export unavailable (dependency/import): %s",
+ e,
+ exc_info=True,
+ )
+ except OSError as e:
+ _log.warning("AGENTS.md pipeline export failed: %s", e, exc_info=True)As per coding guidelines: "Never use bare except: pass — use typed exceptions or at minimum logger.warning(...) with exc_info=True to avoid silent failure in a memory product".
🤖 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/src/gradata/hooks/session_close.py` around lines 186 - 190, Replace
the broad "except Exception as e" that wraps the call to
_export_agents_rules(brain_dir, lessons_path) with typed exception handlers:
catch expected failures like ImportError and OSError (e.g., "except
(ImportError, OSError) as e:") and call _log.warning("AGENTS.md pipeline export
skipped: %s", e, exc_info=True); optionally keep a narrower fallback that also
logs as warning (not silently) for any other Exception to avoid swallowing
errors. Ensure you modify the block around the _export_agents_rules invocation
and update _log calls accordingly.
Source: Coding guidelines
| monkeypatch.setenv("GRADATA_BETA_LB_GATE", "0") | ||
| monkeypatch.delenv("GRADATA_API_KEY", raising=False) | ||
| monkeypatch.setattr(session_close, "resolve_brain_dir", lambda: str(tmp_path)) | ||
|
|
There was a problem hiding this comment.
Use the shared BRAIN_DIR test-isolation path instead of patching resolve_brain_dir.
Line 85 bypasses the test isolation contract and can drift from the _paths.py cache-refresh behavior expected by the suite. Route this through the conftest tmp_path + BRAIN_DIR setup pattern used for isolated brain state.
As per coding guidelines: "Set BRAIN_DIR environment variable via tmp_path in conftest.py for test isolation — ensure _paths.py module cache refreshes when calling Brain.init() directly inside tests". Based on learnings: "Applies to Gradata/tests/**/*.py : Set BRAIN_DIR environment variable via tmp_path in conftest.py for test isolation — ensure _paths.py module cache refreshes when calling Brain.init() directly inside tests".
🤖 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 83 - 86,
Replace the direct monkeypatch of session_close.resolve_brain_dir with the
shared BRAIN_DIR test-isolation pattern: set the BRAIN_DIR environment variable
to the conftest-provided tmp_path / BRAIN_DIR (via
monkeypatch.setenv("BRAIN_DIR", str(tmp_path / "BRAIN_DIR"))) so `_paths.py` is
refreshed when tests call Brain.init(); remove the
monkeypatch.setattr(session_close, "resolve_brain_dir", ...) and ensure tests
use Brain.init() after setting BRAIN_DIR to pick up the proper cached path
behavior.
Sources: Coding guidelines, Learnings
1 participant
Summary
Paperclip
Issue: GRA-1102
Issue UUID: d100fb3c-a477-449c-84d5-84de5a044879
Verification
python3 -m pytest tests/test_session_close_write_through_gate.py -q→ 5 passedpython3 -m pytest tests/test_session_close_write_through_gate.py tests/test_rule_pipeline.py tests/test_rule_export.py -q→ 53 passed