test: cover session close graduation export#261
Conversation
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.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent 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)
🧰 Additional context used📓 Path-based instructions (1)Gradata/tests/**/*.py📄 CodeRabbit inference engine (Gradata/AGENTS.md)
Files:
🧠 Learnings (4)📓 Common learnings📚 Learning: 2026-04-17T17:18:07.439ZApplied to files:
📚 Learning: 2026-05-01T15:50:32.772ZApplied to files:
📚 Learning: 2026-05-01T15:50:32.772ZApplied to files:
🔇 Additional comments (1)
📝 Walkthrough
WalkthroughAdds one integration pytest that seeds a deterministic ChangesSession Close Graduation Integration Test
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 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 |
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_hooks_learning.py`:
- Around line 416-417: Replace the ad-hoc os.environ patch that sets
GRADATA_BRAIN_DIR around the close_main({"session_number": 7}) call with the
shared brain-dir test fixture provided in conftest.py: remove the
patch.dict(...) block and add the conftest fixture parameter (the brain-dir
fixture that wires BRAIN_DIR and companion env from tmp_path) to the test
signature so the test runs with the fixture-provided tmp_path/BRAIN_DIR, keeping
the call to close_main({"session_number": 7}) unchanged; ensure any references
to GRADATA_BRAIN_DIR in the test are removed or rely on the fixture's
environment setup instead.
🪄 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: d3e2ea75-0a0d-4da8-9fe9-23dd5848fff3
📒 Files selected for processing (1)
Gradata/tests/test_hooks_learning.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 windows-latest / py3.11
- GitHub Check: pytest ubuntu-latest / py3.11
- GitHub Check: pytest ubuntu-latest / py3.12
- GitHub Check: pytest macos-latest / py3.12
- GitHub Check: pytest macos-latest / py3.11
- GitHub Check: pytest windows-latest / py3.12
- GitHub Check: pytest (py3.11)
- GitHub Check: pytest (py3.12)
🧰 Additional context used
📓 Path-based instructions (1)
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_hooks_learning.py
🧠 Learnings (1)
📓 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
🔇 Additional comments (1)
Gradata/tests/test_hooks_learning.py (1)
417-417: Ensureclose_maininGradata/tests/test_hooks_learning.pystays CI-deterministic (no real LLM/network).
close_main({"session_number": 7})delegates toGradata/src/gradata/hooks/session_close.py:_run_pipeline, which imports and runsgradata.enhancements.rule_pipeline.run_rule_pipeline(i.e., it’s not stubbed in this test). A scan for common LLM/network libraries (openai,anthropic,requests,httpx,aiohttp, etc.) didn’t find direct matches inGradata/src/Gradata/tests, but determinism still can’t be guaranteed because the call path goes intorun_rule_pipeline. Mock that step (or force the non-LLM branch) or mark/skip as@pytest.mark.integrationper policy.
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.
|
Superseded by merged PR #263 for GRA-1102. |
Summary
Paperclip issue: d100fb3c-a477-449c-84d5-84de5a044879
Verification
python3 -m pytest tests/test_hooks_learning.py::test_session_close_graduates_fixture_to_agents_md -qpython3 -m pytest tests/test_hooks_learning.py -q