fix: add friendly Gradata auth error#264
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 introduces explicit Cloud API authentication error handling to the ChangesCloud API Authentication Enforcement
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 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/cloud/client.py`:
- Around line 339-343: Add a deterministic unit test that mocks/raises HTTPError
with .code set to 401 and 403 and asserts that the call raises GradataAuthError
(use the same client method that contains the except HTTPError block to trigger
the handler); create separate subtests for 401 and 403, and also include a
negative/other-code case (e.g., 413) to assert the mapping to _TooLargeError
remains unchanged so regressions are caught. Use the real HTTPError,
GradataAuthError, and _TooLargeError symbols from the client code and patch the
network call or method that would raise HTTPError to simulate the conditions.
Ensure the test is isolated, deterministic, and added to the test suite where
other cloud client tests live.
- Around line 27-28: AUTH_ERROR_MESSAGE currently imported in
Gradata/src/gradata/cloud/client.py from gradata.brain creates an unnecessary
coupling; move the constant into gradata.exceptions (or a small shared module)
and update imports. Specifically, add AUTH_ERROR_MESSAGE to gradata.exceptions
(next to GradataAuthError), change the import in cloud/client.py to from
gradata.exceptions import AUTH_ERROR_MESSAGE, GradataAuthError, and update any
other modules (e.g., gradata.brain) to import AUTH_ERROR_MESSAGE from
gradata.exceptions instead of defining or exporting it from gradata.brain.
🪄 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: b53dc92d-c8e7-473f-beb8-b6112823ea04
📒 Files selected for processing (6)
Gradata/src/gradata/__init__.pyGradata/src/gradata/brain.pyGradata/src/gradata/cloud/client.pyGradata/src/gradata/exceptions.pyGradata/src/gradata/onboard.pyGradata/tests/test_auth_error.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 windows-latest / py3.11
- GitHub Check: pytest macos-latest / py3.12
- GitHub Check: pytest ubuntu-latest / py3.11
- GitHub Check: pytest ubuntu-latest / py3.12
- GitHub Check: pytest windows-latest / py3.12
- GitHub Check: pytest (py3.11)
- GitHub Check: pytest (py3.12)
🧰 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_auth_error.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/__init__.pyGradata/src/gradata/onboard.pyGradata/src/gradata/cloud/client.pyGradata/src/gradata/exceptions.pyGradata/src/gradata/brain.py
🧠 Learnings (7)
📓 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.
📚 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_auth_error.py
📚 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_auth_error.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/src/**/*.py : 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)
Applied to files:
Gradata/src/gradata/__init__.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/src/**/*.py : Prefer `sentence-transformers` for local embeddings, `google-genai` for Gemini embeddings, `cryptography` for AES-GCM encrypted system.db, `bm25s` for BM25 rule ranking, and `mem0ai` for external memory adapters — guard all optional dependency imports with `try / except ImportError` at the call site, never at module level
Applied to files:
Gradata/src/gradata/__init__.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/**/pyproject.toml : Maintain `dependencies = []` in pyproject.toml — the base package is pure Python + stdlib with all heavy dependencies gated as optional extras: embeddings, gemini, encrypted, ranking, adapters-mem0
Applied to files:
Gradata/src/gradata/__init__.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/src/gradata/onboard.pyGradata/src/gradata/brain.py
🔇 Additional comments (5)
Gradata/src/gradata/exceptions.py (1)
25-27: LGTM!Gradata/src/gradata/__init__.py (1)
65-65: LGTM!Also applies to: 84-84
Gradata/src/gradata/brain.py (1)
57-60: LGTM!Also applies to: 63-70, 74-75, 101-110, 298-298
Gradata/src/gradata/onboard.py (1)
490-490: LGTM!Gradata/tests/test_auth_error.py (1)
9-35: LGTM!
| from gradata.brain import AUTH_ERROR_MESSAGE | ||
| from gradata.exceptions import GradataAuthError |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Decouple auth message constant from gradata.brain to reduce module coupling.
Importing AUTH_ERROR_MESSAGE from gradata.brain makes the cloud client depend on the full Brain module. Move the shared message constant to gradata.exceptions (or a tiny shared auth constants module) and import it from both callers.
🤖 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/cloud/client.py` around lines 27 - 28, AUTH_ERROR_MESSAGE
currently imported in Gradata/src/gradata/cloud/client.py from gradata.brain
creates an unnecessary coupling; move the constant into gradata.exceptions (or a
small shared module) and update imports. Specifically, add AUTH_ERROR_MESSAGE to
gradata.exceptions (next to GradataAuthError), change the import in
cloud/client.py to from gradata.exceptions import AUTH_ERROR_MESSAGE,
GradataAuthError, and update any other modules (e.g., gradata.brain) to import
AUTH_ERROR_MESSAGE from gradata.exceptions instead of defining or exporting it
from gradata.brain.
| except HTTPError as e: | ||
| if e.code in (401, 403): | ||
| raise GradataAuthError(AUTH_ERROR_MESSAGE) from e | ||
| if e.code == 413: | ||
| raise _TooLargeError() from e |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add a focused unit test for 401/403 → GradataAuthError mapping.
This is a behavior contract change; a deterministic test around mocked HTTPError for 401/403 will prevent regressions.
🤖 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/cloud/client.py` around lines 339 - 343, Add a
deterministic unit test that mocks/raises HTTPError with .code set to 401 and
403 and asserts that the call raises GradataAuthError (use the same client
method that contains the except HTTPError block to trigger the handler); create
separate subtests for 401 and 403, and also include a negative/other-code case
(e.g., 413) to assert the mapping to _TooLargeError remains unchanged so
regressions are caught. Use the real HTTPError, GradataAuthError, and
_TooLargeError symbols from the client code and patch the network call or method
that would raise HTTPError to simulate the conditions. Ensure the test is
isolated, deterministic, and added to the test suite where other cloud client
tests live.
1 participant
Summary
GradataAuthErrorand export it fromgradataBrain(...)with a friendly missing-key message when no explicit/env/keyfile API key is availableBrain.init(...)to keep local-first initialization working, and allowBrain(..., api_key=...)Verification
env -u BRAIN_DIR -u GRADATA_BRAIN python3 -m pytest tests/test_auth_error.py tests/test_brain_write_through.py -qgit diff --check