test: isolate quickstart smoke brain env#270
Open
Gradata wants to merge 1 commit into
Open
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
WalkthroughThe PR improves the offline quickstart smoke test's determinism and safety by clearing inherited environment variables that might contaminate test execution, validating that the test operates on the expected temporary brain directory, and updating installation documentation to clarify this isolation behavior. ChangesSmoke Test and Documentation Updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 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: 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/examples/offline_quickstart_smoke.py`:
- Around line 64-68: Move the assertion that verifies stats used the correct
brain directory to run immediately after obtaining stats and before calling
run(..., "audit") so the script fails fast; specifically, after the call that
assigns stats = run(["--brain-dir", str(brain), "stats"], env=env) check if
f"Brain: {brain}" is in stats and raise SystemExit(...) if not, and only then
invoke run(["--brain-dir", str(brain), "audit"], env=env) — this ensures the
stats validation gates running audit and prevents contamination.
🪄 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: 54967ac0-7069-4fba-9f9b-790fd5ce88e5
📒 Files selected for processing (2)
Gradata/docs/getting-started/install.mdGradata/examples/offline_quickstart_smoke.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.12)
- GitHub Check: pytest (py3.11)
- GitHub Check: pytest ubuntu-latest / py3.12
- GitHub Check: pytest macos-latest / py3.11
- GitHub Check: pytest macos-latest / py3.12
- GitHub Check: pytest ubuntu-latest / py3.11
- GitHub Check: pytest windows-latest / py3.12
- GitHub Check: pytest windows-latest / py3.11
🧰 Additional context used
🧠 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: 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
📚 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/docs/getting-started/install.mdGradata/examples/offline_quickstart_smoke.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/docs/getting-started/install.mdGradata/examples/offline_quickstart_smoke.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: Use `from gradata import Brain` as the public entry point — `brain.correct()` is THE entry point for the headline product promise
Applied to files:
Gradata/examples/offline_quickstart_smoke.py
🔇 Additional comments (2)
Gradata/examples/offline_quickstart_smoke.py (1)
46-49: LGTM!Gradata/docs/getting-started/install.md (1)
117-119: LGTM!
Comment on lines
+64
to
+68
| stats = run(["--brain-dir", str(brain), "stats"], env=env) | ||
| run(["--brain-dir", str(brain), "audit"], env=env) | ||
|
|
||
| if f"Brain: {brain}" not in stats: | ||
| raise SystemExit(f"stats used the wrong brain directory:\n{stats}") |
There was a problem hiding this comment.
Move the stats brain-directory assertion before audit.
Line 65 runs audit before the guard on Line 67, so contamination is only detected after another command already executed. Reorder to fail fast before audit.
Proposed fix
run(["--brain-dir", str(brain), "recall", "draft a launch email", "--max-tokens", "400"], env=env)
stats = run(["--brain-dir", str(brain), "stats"], env=env)
- run(["--brain-dir", str(brain), "audit"], env=env)
if f"Brain: {brain}" not in stats:
raise SystemExit(f"stats used the wrong brain directory:\n{stats}")
+ run(["--brain-dir", str(brain), "audit"], env=env)
if not (brain / "system.db").exists():
raise SystemExit(f"missing expected brain database: {brain / 'system.db'}")Based on learnings: CLI brain resolution is env-first (GRADATA_BRAIN/BRAIN_DIR before CLI args), so this check should gate subsequent commands.
📝 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
| stats = run(["--brain-dir", str(brain), "stats"], env=env) | |
| run(["--brain-dir", str(brain), "audit"], env=env) | |
| if f"Brain: {brain}" not in stats: | |
| raise SystemExit(f"stats used the wrong brain directory:\n{stats}") | |
| stats = run(["--brain-dir", str(brain), "stats"], env=env) | |
| if f"Brain: {brain}" not in stats: | |
| raise SystemExit(f"stats used the wrong brain directory:\n{stats}") | |
| run(["--brain-dir", str(brain), "audit"], env=env) |
🤖 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/examples/offline_quickstart_smoke.py` around lines 64 - 68, Move the
assertion that verifies stats used the correct brain directory to run
immediately after obtaining stats and before calling run(..., "audit") so the
script fails fast; specifically, after the call that assigns stats =
run(["--brain-dir", str(brain), "stats"], env=env) check if f"Brain: {brain}" is
in stats and raise SystemExit(...) if not, and only then invoke
run(["--brain-dir", str(brain), "audit"], env=env) — this ensures the stats
validation gates running audit and prevents contamination.
Source: Learnings
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
examples/offline_quickstart_smoke.pyagainst inherited live Gradata brain env (BRAIN_DIR,GRADATA_BRAIN)statsis using the temporary smoke brain, so Show HN onboarding smoke catches env contaminationVerification
Prior GRA-1781 docs/smoke artifact already merged in PR #236; this follow-up fixes the environment contamination found while verifying it.