Add Python project standards to setuplab skill and docs#2945
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a new Python standards document for velocity stack projects and updates automaker's setuplab guidance to detect Python projects and apply those standards based on presence of Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 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)
📝 Coding Plan
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 the current code and only fix it if needed.
Inline comments:
In `@docs/internal/dev/python-standards.md`:
- Around line 84-92: The fenced code block showing the test directory tree (the
block starting with "tests/" and containing unit/ integration/ and conftest.py)
is missing a language tag which triggers MD040; update the opening fence from
``` to ```text to mark it as plain text so the linter recognizes it (i.e., add
the "text" language identifier to the code fence surrounding that tree snippet).
In `@packages/mcp-server/plugins/automaker/commands/setuplab.md`:
- Around line 78-80: The current classifier logic in the "Python project" rule
(the bullet line that checks for `.py` files, `Dockerfile`, and absence of
`package.json`) wrongly requires a Dockerfile to identify a Python repo; change
the detection to treat presence of `.py` files or `pyproject.toml` as sufficient
to classify a layer as Python (i.e., evaluate `.py`/`pyproject.toml` first), and
then separately add a check that emits a "missing Dockerfile" gap if no
Dockerfile is present rather than preventing Python classification; keep the
existing TypeScript/Node.js rule (presence of `package.json`) and the Mixed rule
behavior unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cb5ca40e-67f6-4c0d-ad8d-184e76200276
📒 Files selected for processing (2)
docs/internal/dev/python-standards.mdpackages/mcp-server/plugins/automaker/commands/setuplab.md
Comment on lines
+84
to
+92
| ``` | ||
| tests/ | ||
| unit/ | ||
| test_classifier.py | ||
| test_parser.py | ||
| integration/ | ||
| test_pipeline.py | ||
| conftest.py | ||
| ``` |
There was a problem hiding this comment.
Add language identifier to fenced code block
The tree snippet fence is missing a language tag, which triggers MD040.
Proposed fix
-```
+```text
tests/
unit/
test_classifier.py
test_parser.py
integration/
test_pipeline.py
conftest.py</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 84-84: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/internal/dev/python-standards.md` around lines 84 - 92, The fenced code
block showing the test directory tree (the block starting with "tests/" and
containing unit/ integration/ and conftest.py) is missing a language tag which
triggers MD040; update the opening fence from ``` to ```text to mark it as plain
text so the linter recognizes it (i.e., add the "text" language identifier to
the code fence surrounding that tree snippet).
Comment on lines
+78
to
+80
| - **Python project** — presence of `.py` files, `Dockerfile`, and no `package.json` → apply Python standard | ||
| - **TypeScript/Node.js project** — presence of `package.json` → apply TypeScript/Node.js standard | ||
| - **Mixed** — both present → apply TypeScript standard for JS/TS layers, Python standard for Python layers; flag for human review |
There was a problem hiding this comment.
Python detection currently misses valid Python repos without Dockerfile
Requiring Dockerfile in the classifier means Python repos that should be flagged as missing containerization won’t be treated as Python at all. Consider classifying Python by .py (and/or pyproject.toml) first, then reporting missing Dockerfile as a gap.
Proposed adjustment
-- **Python project** — presence of `.py` files, `Dockerfile`, and no `package.json` → apply Python standard
+- **Python project** — presence of `.py` files (or `pyproject.toml`) and no `package.json` → apply Python standard📝 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
| - **Python project** — presence of `.py` files, `Dockerfile`, and no `package.json` → apply Python standard | |
| - **TypeScript/Node.js project** — presence of `package.json` → apply TypeScript/Node.js standard | |
| - **Mixed** — both present → apply TypeScript standard for JS/TS layers, Python standard for Python layers; flag for human review | |
| - **Python project** — presence of `.py` files (or `pyproject.toml`) and no `package.json` → apply Python standard | |
| - **TypeScript/Node.js project** — presence of `package.json` → apply TypeScript/Node.js standard | |
| - **Mixed** — both present → apply TypeScript standard for JS/TS layers, Python standard for Python layers; flag for human review |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/mcp-server/plugins/automaker/commands/setuplab.md` around lines 78 -
80, The current classifier logic in the "Python project" rule (the bullet line
that checks for `.py` files, `Dockerfile`, and absence of `package.json`)
wrongly requires a Dockerfile to identify a Python repo; change the detection to
treat presence of `.py` files or `pyproject.toml` as sufficient to classify a
layer as Python (i.e., evaluate `.py`/`pyproject.toml` first), and then
separately add a check that emits a "missing Dockerfile" gap if no Dockerfile is
present rather than preventing Python classification; keep the existing
TypeScript/Node.js rule (presence of `package.json`) and the Mixed rule behavior
unchanged.
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
setuplab currently only defines a TypeScript/Node.js gold standard. Add Python project support.
setuplab skill update (packages/mcp-server/plugins/automaker/commands/setuplab.md)
Add a Python Gold Standard table alongside the existing one:
Recovered automatically by Automaker post-agent hook
Summary by CodeRabbit