docs: add offline quickstart smoke proof#235
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.
|
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/src/**/*.py📄 CodeRabbit inference engine (Gradata/AGENTS.md)
Files:
🔇 Additional comments (2)
📝 Walkthrough
WalkthroughThis PR adds an offline smoke test script plus helper and pytest, documents the optional offline quickstart step and ships the script via .gitignore, and changes the CLI install verification to inspect recent CORRECTION events instead of searching rule text. ChangesOffline quickstart smoke test
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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: 3
🤖 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/scripts/smoke_quickstart.py`:
- Line 35: The hard-coded timeout=30 in smoke_quickstart.py makes the smoke run
flaky on slow CI; make the timeout configurable by replacing the literal with a
variable (e.g., cmd_timeout) that is populated from a config source such as an
environment variable (SMOKE_CMD_TIMEOUT) or a CLI argument with a default of 30,
parse it to int, and use that variable wherever timeout=30 is currently passed
(search for the exact token "timeout=30" in smoke_quickstart.py and update the
surrounding function that runs subprocesses or commands to accept/consume the
configurable timeout).
- Line 57: The env construction currently overwrites PYTHONPATH by setting
"PYTHONPATH": str(SRC); instead prepend SRC to any existing PYTHONPATH instead
of replacing it: read the current value via os.environ.get("PYTHONPATH", ""),
join SRC and the existing value with os.pathsep (handling empty existing value
so you don't add a trailing separator), and set that combined string as the
"PYTHONPATH" entry in the env dict where SRC is referenced.
In `@Gradata/tests/test_quickstart_smoke.py`:
- Line 14: The assertion for result["sessions_trained"] only checks for not None
and would accept 0 (a no-op run); change the check on the test that contains
assert result["sessions_trained"] is not None to assert that
result["sessions_trained"] is a positive integer (e.g., assert
isinstance(result["sessions_trained"], int) and result["sessions_trained"] > 0
or simply assert result["sessions_trained"] >= 1) so the test fails when no
training sessions were actually run.
🪄 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: 484650c1-c1f6-4c47-b7f2-42a56603c65a
📒 Files selected for processing (5)
.gitignoreGradata/README.mdGradata/docs/getting-started/quickstart.mdGradata/scripts/smoke_quickstart.pyGradata/tests/test_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 windows-latest / py3.12
- GitHub Check: pytest ubuntu-latest / py3.12
- GitHub Check: pytest macos-latest / py3.11
- GitHub Check: pytest windows-latest / py3.11
- GitHub Check: pytest macos-latest / py3.12
- GitHub Check: pytest ubuntu-latest / py3.11
- GitHub Check: pytest (py3.12)
- GitHub Check: pytest (py3.11)
🧰 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_quickstart_smoke.py
🔇 Additional comments (3)
.gitignore (1)
178-178: LGTM!Gradata/README.md (1)
109-116: LGTM!Gradata/docs/getting-started/quickstart.md (1)
5-14: LGTM!
| env=env, | ||
| text=True, | ||
| capture_output=True, | ||
| timeout=30, |
There was a problem hiding this comment.
Make command timeout configurable to reduce flaky failures.
A fixed 30s timeout is brittle for slower CI/dev hosts; one slow command will fail the full smoke run.
Proposed fix
- timeout=30,
+ timeout=int(env.get("GRADATA_SMOKE_TIMEOUT_SEC", "60")),🤖 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/scripts/smoke_quickstart.py` at line 35, The hard-coded timeout=30 in
smoke_quickstart.py makes the smoke run flaky on slow CI; make the timeout
configurable by replacing the literal with a variable (e.g., cmd_timeout) that
is populated from a config source such as an environment variable
(SMOKE_CMD_TIMEOUT) or a CLI argument with a default of 30, parse it to int, and
use that variable wherever timeout=30 is currently passed (search for the exact
token "timeout=30" in smoke_quickstart.py and update the surrounding function
that runs subprocesses or commands to accept/consume the configurable timeout).
| env.update( | ||
| { | ||
| "HOME": str(home_dir), | ||
| "PYTHONPATH": str(SRC), |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Preserve existing PYTHONPATH instead of overwriting it.
Overwriting can break environments that already depend on PYTHONPATH. Prepend SRC and keep existing entries.
Proposed fix
- "PYTHONPATH": str(SRC),
+ "PYTHONPATH": (
+ f"{SRC}{os.pathsep}{env['PYTHONPATH']}"
+ if env.get("PYTHONPATH")
+ else str(SRC)
+ ),📝 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.
| "PYTHONPATH": str(SRC), | |
| "PYTHONPATH": ( | |
| f"{SRC}{os.pathsep}{env['PYTHONPATH']}" | |
| if env.get("PYTHONPATH") | |
| else str(SRC) | |
| ), |
🤖 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/scripts/smoke_quickstart.py` at line 57, The env construction
currently overwrites PYTHONPATH by setting "PYTHONPATH": str(SRC); instead
prepend SRC to any existing PYTHONPATH instead of replacing it: read the current
value via os.environ.get("PYTHONPATH", ""), join SRC and the existing value with
os.pathsep (handling empty existing value so you don't add a trailing
separator), and set that combined string as the "PYTHONPATH" entry in the env
dict where SRC is referenced.
| commands = cast("list[str]", result["commands"]) | ||
|
|
||
| assert result["database_created"] is True | ||
| assert result["sessions_trained"] is not None |
There was a problem hiding this comment.
Strengthen sessions_trained assertion to catch no-op runs.
is not None still passes for 0; this can miss regressions where training doesn’t happen.
Proposed fix
- assert result["sessions_trained"] is not None
+ assert isinstance(result["sessions_trained"], int)
+ assert result["sessions_trained"] >= 1📝 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.
| assert result["sessions_trained"] is not None | |
| assert isinstance(result["sessions_trained"], int) | |
| assert result["sessions_trained"] >= 1 |
🤖 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_quickstart_smoke.py` at line 14, The assertion for
result["sessions_trained"] only checks for not None and would accept 0 (a no-op
run); change the check on the test that contains assert
result["sessions_trained"] is not None to assert that result["sessions_trained"]
is a positive integer (e.g., assert isinstance(result["sessions_trained"], int)
and result["sessions_trained"] > 0 or simply assert result["sessions_trained"]
>= 1) so the test fails when no training sessions were actually run.
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.
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.
1 participant
Summary
scripts/smoke_quickstart.py, an offline SDK/CLI smoke proof for Show HN readerstests/test_quickstart_smoke.pyVerification
python3 scripts/smoke_quickstart.py→ PASS; created temp local brain, recorded correction, recalled rules, rendered manifest withsessions_trained: 1python3 -m pytest tests/test_quickstart_smoke.py→ 1 passeduv run --extra dev ruff check scripts/smoke_quickstart.py tests/test_quickstart_smoke.py→ All checks passedPaperclip: GRA-1783