docs: improve CLAUDE.md clarity and add project rules#95
Conversation
- Remove boilerplate intro line - Add prominent "Before Every Commit" section with poe check - Standardize all commands to use `poetry run poe` - Add Key Files quick-reference table - Add note about .claude/rules/ auto-loaded guidelines - Clarify generated GraphQL client anti-pattern wording - Move .claude/*.md files into .claude/rules/ for auto-loading
📝 WalkthroughWalkthroughAdds five new Python-specific rule documents under Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CLAUDE.md (1)
78-78:⚠️ Potential issue | 🟡 Minor
poe gen-gqlstill missingpoetry runprefixLine 78 is the only remaining spot in the file that uses the bare
poe gen-gqlcommand. The Anti-Patterns section on Line 88 already usespoetry run poe gen-gqlcorrectly. This inconsistency partially defeats the PR's standardization goal.✏️ Proposed fix
-- **Generated code**: `src/planpilot/core/providers/github/github_gql/` is excluded from mypy and coverage. Regenerate with `poe gen-gql`. +- **Generated code**: `src/planpilot/core/providers/github/github_gql/` is excluded from mypy and coverage. Regenerate with `poetry run poe gen-gql`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLAUDE.md` at line 78, Replace the bare command string "poe gen-gql" with the standardized "poetry run poe gen-gql" in the CLAUDE.md occurrence that references regenerating src/planpilot/core/providers/github/github_gql/ so the file matches the Anti-Patterns section's usage; update the single remaining "poe gen-gql" instance to "poetry run poe gen-gql".
🧹 Nitpick comments (3)
.claude/rules/hooks.md (1)
12-15: Consider providing a concrete~/.claude/settings.jsonsnippetThe guidance says to "configure in
~/.claude/settings.json" but omits a sample. Without an example, developers need to separately research Claude Code's hook configuration format. A minimal JSON snippet would make this immediately actionable.Additionally, Line 14 lists
black/rufffor formatting; since the project standardizes onruff formatalone, preferringruffin the example avoids ambiguity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/rules/hooks.md around lines 12 - 15, Add a concrete example JSON snippet to ~/.claude/settings.json that replaces the ambiguous "black/ruff" text with the project-preferred "ruff" and shows the exact hook entries for Python files: include a "hooks" object with a pattern for "*.py" and commands for formatting using "ruff format" and for type checking using the chosen tool ("mypy" or "pyright"), so reviewers can copy-paste the minimal working configuration rather than researching the hook schema..claude/rules/coding-style.md (1)
19-32:from typing import NamedTupleimport appears mid-blockThe single code block interleaves a second import (
from typing import NamedTuple) after a class definition. While the intent is clear, it reads as non-idiomatic Python where all imports live at the top of a file. Splitting into two labeled code blocks would eliminate any ambiguity.✏️ Suggested fix
-```python -from dataclasses import dataclass - -@dataclass(frozen=True) -class User: - name: str - email: str - -from typing import NamedTuple - -class Point(NamedTuple): - x: float - y: float -``` +**Frozen dataclass:** +```python +from dataclasses import dataclass + +@dataclass(frozen=True) +class User: + name: str + email: str +``` + +**NamedTuple:** +```python +from typing import NamedTuple + +class Point(NamedTuple): + x: float + y: float +```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/rules/coding-style.md around lines 19 - 32, The code block places "from typing import NamedTuple" after the User dataclass, which is non-idiomatic; move all imports to the top or split into two labeled blocks so imports precede definitions. Specifically, ensure "from dataclasses import dataclass" is grouped with the User dataclass declaration (class User) and "from typing import NamedTuple" is placed before the Point definition (class Point), or separate them into two distinct code blocks labeled (e.g., "Frozen dataclass" and "NamedTuple") so each import sits immediately above its related type definition..claude/rules/security.md (1)
23-26:banditis not in dependencies or thepoe checkpipelineThe guide instructs running
bandit -r src/, butbanditis not declared inpyproject.tomland no poe task exists for it. Thepoe checkpipeline currently runs:["lint", "format-check", "typecheck", "test"].Either add a
poe securitytask for bandit and include it in the pipeline, or update the guidance to explicitly note this is an optional manual step that developers must set up separately.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/rules/security.md around lines 23 - 26, The docs instruct running bandit but the project doesn't declare bandit in pyproject.toml nor include it in the poe "check" pipeline; either add a poe task (e.g., name it "security") that runs "bandit -r src/" and include "security" in the poe "check" list, and add bandit to the project's dev dependencies in pyproject.toml, or update .claude/rules/security.md to state that running bandit is an optional manual step and mention that a poe task is not provided by default; reference the bandit command, the poe "check" pipeline, and pyproject.toml when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/rules/coding-style.md:
- Around line 36-38: Replace the misleading tooling list that names "black" and
"isort" alongside "ruff" with a single accurate description stating that the
project uses "ruff format" for formatting and "ruff check" for linting and
import-sorting; specifically update the lines that mention **black** and
**isort** so they no longer appear and instead document **ruff format** and
**ruff check** (or similar phrasing) to reflect the actual setup and avoid
suggesting conflicting formatters.
In @.claude/rules/testing.md:
- Line 8: Update the compound adjective "Python specific content" to the
hyphenated form "Python-specific content" in this file (the line that currently
reads "This file extends [common/testing.md](../common/testing.md) with Python
specific content.") and make the identical change in the other four rule files
that use the same phrasing so all five files consistently use "Python-specific
content".
- Around line 27-34: Register the custom pytest marks "unit" and "integration"
to avoid PytestUnknownMarkWarning by adding them under the
[tool.pytest.ini_options] section in pyproject.toml; specifically add entries
for the markers list that document "unit" and "integration" (so tests that use
`@pytest.mark.unit` and `@pytest.mark.integration` are recognized).
In `@CLAUDE.md`:
- Line 5: Update the short banner description for the poe check command so it
includes "format-check" to match the detailed description; specifically modify
the text "Run `poetry run poe check` (lint + typecheck + tests)" to mention
format-check (e.g., "lint + format-check + typecheck + non-E2E tests") so the
poe check summary and the detailed list are consistent for the `poe check`
entry.
---
Outside diff comments:
In `@CLAUDE.md`:
- Line 78: Replace the bare command string "poe gen-gql" with the standardized
"poetry run poe gen-gql" in the CLAUDE.md occurrence that references
regenerating src/planpilot/core/providers/github/github_gql/ so the file matches
the Anti-Patterns section's usage; update the single remaining "poe gen-gql"
instance to "poetry run poe gen-gql".
---
Nitpick comments:
In @.claude/rules/coding-style.md:
- Around line 19-32: The code block places "from typing import NamedTuple" after
the User dataclass, which is non-idiomatic; move all imports to the top or split
into two labeled blocks so imports precede definitions. Specifically, ensure
"from dataclasses import dataclass" is grouped with the User dataclass
declaration (class User) and "from typing import NamedTuple" is placed before
the Point definition (class Point), or separate them into two distinct code
blocks labeled (e.g., "Frozen dataclass" and "NamedTuple") so each import sits
immediately above its related type definition.
In @.claude/rules/hooks.md:
- Around line 12-15: Add a concrete example JSON snippet to
~/.claude/settings.json that replaces the ambiguous "black/ruff" text with the
project-preferred "ruff" and shows the exact hook entries for Python files:
include a "hooks" object with a pattern for "*.py" and commands for formatting
using "ruff format" and for type checking using the chosen tool ("mypy" or
"pyright"), so reviewers can copy-paste the minimal working configuration rather
than researching the hook schema.
In @.claude/rules/security.md:
- Around line 23-26: The docs instruct running bandit but the project doesn't
declare bandit in pyproject.toml nor include it in the poe "check" pipeline;
either add a poe task (e.g., name it "security") that runs "bandit -r src/" and
include "security" in the poe "check" list, and add bandit to the project's dev
dependencies in pyproject.toml, or update .claude/rules/security.md to state
that running bandit is an optional manual step and mention that a poe task is
not provided by default; reference the bandit command, the poe "check" pipeline,
and pyproject.toml when making the change.
.claude/rules/testing.md
Outdated
| --- | ||
| # Python Testing | ||
|
|
||
| > This file extends [common/testing.md](../common/testing.md) with Python specific content. |
There was a problem hiding this comment.
Minor grammar nit: "Python-specific"
The compound adjective modifying "content" should be hyphenated: "Python-specific content." The same nit applies to the identical phrasing in the other four rule files.
✏️ Suggested fix
-> This file extends [common/testing.md](../common/testing.md) with Python specific content.
+> This file extends [common/testing.md](../common/testing.md) with Python-specific content.📝 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.
| > This file extends [common/testing.md](../common/testing.md) with Python specific content. | |
| > This file extends [common/testing.md](../common/testing.md) with Python-specific content. |
🧰 Tools
🪛 LanguageTool
[grammar] ~8-~8: Use a hyphen to join words.
Context: ...ng.md](../common/testing.md) with Python specific content. ## Framework Use **p...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/rules/testing.md at line 8, Update the compound adjective "Python
specific content" to the hyphenated form "Python-specific content" in this file
(the line that currently reads "This file extends
[common/testing.md](../common/testing.md) with Python specific content.") and
make the identical change in the other four rule files that use the same
phrasing so all five files consistently use "Python-specific content".
.claude/rules/testing.md
Outdated
| @pytest.mark.unit | ||
| def test_calculate_total(): | ||
| ... | ||
|
|
||
| @pytest.mark.integration | ||
| def test_database_connection(): | ||
| ... | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether pytest markers are registered in pyproject.toml
grep -n "markers\|mark\.unit\|mark\.integration" pyproject.toml || echo "No markers registered in pyproject.toml"
# Also check if pytest.ini exists
if [ -f pytest.ini ]; then
echo "---"
echo "Found pytest.ini:"
cat pytest.ini
else
echo "No pytest.ini found"
fi
# Check the full [tool.pytest.ini_options] section
echo "---"
echo "Full pyproject.toml [tool.pytest.ini_options] section:"
grep -A 20 "\[tool.pytest.ini_options\]" pyproject.toml || echo "No [tool.pytest.ini_options] section found"Repository: aryeko/planpilot
Length of output: 693
Register custom pytest marks to suppress PytestUnknownMarkWarning
Custom marks @pytest.mark.unit and @pytest.mark.integration are not registered in pyproject.toml. Add them to the [tool.pytest.ini_options] section:
Suggested fix
[tool.pytest.ini_options]
markers = [
"unit: mark test as a unit test",
"integration: mark test as an integration test",
]Without this registration, pytest emits PytestUnknownMarkWarning for every test using these marks.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/rules/testing.md around lines 27 - 34, Register the custom pytest
marks "unit" and "integration" to avoid PytestUnknownMarkWarning by adding them
under the [tool.pytest.ini_options] section in pyproject.toml; specifically add
entries for the markers list that document "unit" and "integration" (so tests
that use `@pytest.mark.unit` and `@pytest.mark.integration` are recognized).
- Fix missing poetry run prefix on poe gen-gql in Key Conventions - Add format-check to Before Every Commit description - Replace black/isort with ruff format/ruff check in coding-style.md - Split non-idiomatic import+class code block into two labeled blocks - Fix bandit note to clarify it is an optional manual step - Replace black/ruff with ruff in hooks.md PostToolUse guidance - Fix Python-specific hyphenation across all five rule files
Python-specific rules now live in ~/.claude/rules/python/ and apply across all Python projects. Removes redundant project-level .claude/rules/.
1 participant
Summary
poetry run poe check(lint + format-check + typecheck + non-E2E tests)poetry run poeconsistently.claude/rules/*.mdfiles into.claude/rules/so Claude Code auto-loads thempoe gen-gqlprefix, replace black/isort with ruff, clarify bandit is optional, fix Python-specific hyphenation~/.claude/rules/python/— now reusable across all Python projectsTest plan
poetry run poe checkto verify it passes~/.claude/rules/python/files load in Claude Code when working with.pyfilespoetry run poe docs-linksto verify no broken internal links🤖 Generated with Claude Code