adds instructions for Claude #530
Conversation
WalkthroughAdds Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Suggested reviewers
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Pre-merge checks (3 passed)✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 0
🧹 Nitpick comments (7)
CLAUDE.md (7)
16-16: Fix MD040: add a language to the fenced code block (Project Structure).Specify a language to satisfy markdownlint.
-``` +```text
74-74: Fix MD040: add a language to the fenced code block (Test Structure).Specify a language to satisfy markdownlint.
-``` +```text
35-35: Make the import example actually “relative.”The example currently shows an absolute import; use a relative import per the guideline.
-- Use relative imports within modules: `from auth import get_auth_dependency` +- Use relative imports within modules: `from .auth import get_auth_dependency`
66-70: Prefer typing.Self on Python ≥3.12; fall back to typing_extensions only for older versions.Keeps guidance aligned with the stated Python versions.
-- Use `typing_extensions.Self` for model validators +- Use `typing.Self` (Python ≥3.12). Use `typing_extensions.Self` only if supporting ≤3.11. - Union types: `str | int` (modern syntax) - - Optional: `Optional[Type]` or `Type | None` + - Optional: prefer `Type | None` for consistency
41-45: Clarify Pydantic v2 configuration pattern.If
ConfigurationBaseis project-specific, call that out; otherwise show the v2-native pattern with ConfigDict.-- All config uses Pydantic models extending `ConfigurationBase` -- Base class sets `extra="forbid"` to reject unknown fields +- All config uses Pydantic v2 `BaseModel` (or project `ConfigurationBase`) with + `model_config = ConfigDict(extra="forbid")` to reject unknown fields +- Use `@model_validator` for custom validationWould you like me to update this section with a short code snippet showing
ConfigDict?
131-137: Avoid pinning versions in docs; source-of-truth should be pyproject.toml.Prevents drift; keep this section stable.
-- **FastAPI**: Web framework (`>=0.115.12`) -- **Llama Stack**: AI integration (`==0.2.19`) - - **Pydantic**: Data validation/serialization -- **SQLAlchemy**: Database ORM (`>=2.0.42`) -- **Kubernetes**: K8s auth integration (`>=30.1.0`) +- Refer to `pyproject.toml` for authoritative versions/constraints. +- **FastAPI**: Web framework +- **Llama Stack**: AI integration +- **Pydantic**: Data validation/serialization +- **SQLAlchemy**: Database ORM +- **Kubernetes**: K8s auth integration
126-130: Optional: add secret scanning to QA.Recommend pre-commit hooks for gitleaks or detect-secrets to enforce “never commit secrets.”
I can propose a minimal pre-commit config if you want it included.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CLAUDE.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
CLAUDE.md
16-16: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
74-74: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (2)
- GitHub Check: build-pr
- GitHub Check: e2e_tests
🔇 Additional comments (5)
CLAUDE.md (5)
1-144: Solid addition—clear, actionable guidance.Overall looks good; the above are minor doc nits and consistency checks.
60-64: Use APIConnectionError from llama_stack_client (explicit import)Update CLAUDE.md (lines 60–64) to reference llama_stack_client.APIConnectionError and show the import example:
from llama_stack_client import APIConnectionError.
139-140: dev and llslibdev groups exist in pyproject.toml — no change required.
Both groups are defined (pyproject.toml: dev at line 92; llslibdev at line 114).
118-125: Update CLAUDE.md to match Makefile 'verify' target
Makefile's verify target invokes these targets: black (uv run black --check .), pylint (uv run pylint src tests), pyright (uv run pyright src), ruff (uv run ruff check . --per-file-ignores=tests/:S101 --per-file-ignores=scripts/:S101), docstyle (uv run pydocstyle -v src), check-types → mypy (uv run mypy --explicit-package-bases --disallow-untyped-calls --disallow-untyped-defs --disallow-incomplete-defs --ignore-missing-imports --disable-error-code attr-defined src/).
Replace the ambiguous "docstyle"/"check-types" wording in CLAUDE.md (lines 118–125) with the exact target names above and remove the claim that pyright excludes src/auth/k8s.py (no such exclusion in Makefile). Optionally mention that the format target runs uv run black . and uv run ruff check . --fix.
7-12: No change required — CLAUDE.md matches repo configurationpyproject.toml has requires-python = ">=3.12,<3.14" and GitHub Actions run 3.12 / 3.13 matrices, so the CLAUDE.md claim (3.12–3.13; 3.14 not supported) is consistent.
CLAUDE.md
Outdated
| ### Coding Standards | ||
|
|
||
| #### Imports & Dependencies | ||
| - Use relative imports within modules: `from auth import get_auth_dependency` |
There was a problem hiding this comment.
relative imports starts with . or ..
CLAUDE.md
Outdated
| #### Configuration | ||
| - All config uses Pydantic models extending `ConfigurationBase` | ||
| - Base class sets `extra="forbid"` to reject unknown fields | ||
| - Use `@model_validator` for custom validation |
There was a problem hiding this comment.
@field_validator can also be used
CLAUDE.md
Outdated
| #### Error Handling | ||
| - Use FastAPI `HTTPException` with appropriate status codes | ||
| - Handle `APIConnectionError` from Llama Stack | ||
| - Logging: `import logging` and use module logger |
There was a problem hiding this comment.
prob. also mention which loglevel to use in the new code
There was a problem hiding this comment.
added a logging section specifically with d4af8e0
| - Use `typing_extensions.Self` for model validators | ||
| - Union types: `str | int` (modern syntax) | ||
| - Optional: `Optional[Type]` or `Type | None` | ||
|
|
There was a problem hiding this comment.
instructions for modules, functions, classes, and methods docstrings...
There was a problem hiding this comment.
Is this the exhaustive list, or are there other items? CLAUDE.md is automatically read by CLAUDE, so if you intend to fully document coding practices for "everything" you'll need a list of "everything".
I'll try to extract specifics for what you listed, but if there's anything else, can you list it?
| return mock_client, mock_agent | ||
| ``` | ||
| - **Auth Mock**: `MOCK_AUTH = ("mock_user_id", "mock_username", False, "mock_token")` | ||
| - **Coverage**: Unit tests require 60% coverage, integration 10% |
There was a problem hiding this comment.
- instruction to use
pytesteverywhere, forbid to useunittest
CLAUDE.md
Outdated
| - Use environment variables for sensitive data | ||
|
|
||
| ## Key Dependencies | ||
| - **FastAPI**: Web framework (`>=0.115.12`) |
There was a problem hiding this comment.
dunno about versions there, prob. instruct to read from pyproject.toml?
There was a problem hiding this comment.
ya, they might change, and unless we auto update the claude.md all the time, this may get stale.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
CLAUDE.md (7)
16-30: Add language to fenced code block (markdownlint MD040).Mark the project tree as text for proper highlighting.
-``` +```text src/ ├── app/ # FastAPI application │ ├── endpoints/ # REST API endpoints │ └── main.py # Application entry point ├── auth/ # Authentication modules (k8s, jwk, noop) ├── authorization/ # Authorization middleware & resolvers ├── models/ # Pydantic models │ ├── config.py # Configuration classes │ ├── requests.py # Request models │ └── responses.py # Response models ├── utils/ # Utility functions ├── client.py # Llama Stack client wrapper └── configuration.py # Config management -``` +```
111-117: Add language to fenced code block (markdownlint MD040).Mark the tests tree as text.
-``` +```text tests/ ├── unit/ # Unit tests (pytest) ├── integration/ # Integration tests └── e2e/ # End-to-end tests (behave) └── features/ # Gherkin feature files -``` +```
99-106: Replace bare URL with descriptive link (markdownlint MD034).Improves readability and satisfies linters.
-- Follow Google Python docstring conventions: https://google.github.io/styleguide/pyguide.html +- Follow Google Python docstring conventions: [Google Python Style Guide](https://google.github.io/styleguide/pyguide.html)
56-59: Clarify Self typing guidance for Python versions.Prefer typing.Self on 3.11+, fall back to typing_extensions on older versions.
- - Use `typing_extensions.Self` for model validators + - Prefer `typing.Self` (Python 3.11+); otherwise use `typing_extensions.Self` for model validators
10-12: Align “verify” tool list with Linting Tools; choose one primary type checker.“verify” mentions “docstyle, check-types” while Linting Tools list pydocstyle + both pyright and mypy. Recommend naming the exact tools consistently and documenting which type checker is authoritative to avoid conflicting results.
161-167: Document the canonical type checker and where it’s configured.Running both pyright and mypy can be redundant; if both are required, state their scopes (e.g., pyright for editors, mypy in CI) and link to the config sections.
152-159: Consider adding pre-commit hook instructions.Eases consistent formatting/linting locally: pre-commit with black, ruff, pydocstyle, bandit.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CLAUDE.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
CLAUDE.md
16-16: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
99-99: Bare URL used
(MD034, no-bare-urls)
111-111: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (4)
CLAUDE.md (4)
145-150: Makefile targets verified — no action needed.
Makefile defines test-unit, test-integration, test-e2e, format, and verify targets; docs match.
182-183: uv groups 'dev' and 'llslibdev' are present in pyproject.toml.
Founddev = [at line 92 andllslibdev = [at line 114 in pyproject.toml.
75-77: Use the llama_stack_client concrete exception classesReplace "APIConnectionError" in CLAUDE.md (lines 75–77) with the actual client exceptions — e.g. llama_stack_client.APIConnectionError (connection issues); also mention llama_stack_client.APIStatusError and llama_stack_client.RateLimitError (and the status-specific errors like BadRequestError/AuthenticationError/NotFoundError) as appropriate. (github.com)
48-53: ConfigurationBase exists and enforces extra="forbid"
ConfigurationBase is defined in src/models/config.py and sets model_config = ConfigDict(extra="forbid").
|
@tisnik I made all the requested changes here. |
2 participants
Description
Adds a CLAUDE.md (read by default) so that Claude understands the coding, testing, and other practices for working with this project.
Type of change
Related Tickets & Documents
N/A
Checklist before requesting a review
N/A
Testing
N/A
Summary by CodeRabbit