feat: Integrate PyCrucible#370
Conversation
|
Review these changes at https://app.gitnotebooks.com/AlphaSphereDotAI/visualizr/pull/370 |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR integrates the pycrucible tool into the project by adding it as a development dependency and introducing a pycrucible.toml configuration file to specify entrypoint, packaging patterns, and source update strategy. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds PyCrucible config and a CI action step, registers Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as GitHub Actions
participant Repo as Repository
participant Job as setup_and_build
participant PyC as PyCrucible Action
GH->>Repo: push / workflow trigger
GH->>Job: start setup_and_build
Job->>Repo: checkout code
Job->>Job: build source & wheel
Job->>PyC: run "Build Python App with PyCrucible" (razorblade23/pycrucible-action@v2)
PyC->>Job: read `pycrucible.toml` and run PyCrucible build
Note right of Job: no artifact upload step present
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
|
Here's the code health analysis summary for commits Analysis Summary
|
Summary of ChangesHello @MH0386, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request integrates Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Double-check that the entrypoint path in pycrucible.toml correctly matches your package layout—consider using the module path (e.g.
visualizr:main) instead of a direct file reference. - Your include/exclude patterns might miss common build artifacts (e.g.
dist/,build/,.pytest_cache/), so consider adding those to the exclude list. - Review whether it’s necessary to explicitly include
pyproject.tomlandREADME.mdin patterns or if those can be handled by default by the repository source control.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Double-check that the entrypoint path in pycrucible.toml correctly matches your package layout—consider using the module path (e.g. `visualizr:main`) instead of a direct file reference.
- Your include/exclude patterns might miss common build artifacts (e.g. `dist/`, `build/`, `.pytest_cache/`), so consider adding those to the exclude list.
- Review whether it’s necessary to explicitly include `pyproject.toml` and `README.md` in patterns or if those can be handled by default by the repository source control.
## Individual Comments
### Comment 1
<location> `pycrucible.toml:5-13` </location>
<code_context>
+entrypoint = "./src/visualizr/__main__.py"
+
+[package.patterns]
+patterns.include = ["**/*.py", "**/README.md", "**/pyproject.toml"]
+patterns.exclude = [
+ "**/*.pyc",
</code_context>
<issue_to_address>
**suggestion:** Review exclude patterns for completeness and potential edge cases.
Consider adding patterns for other generated files or directories such as build/, dist/, or .mypy_cache/ if relevant to your workflow.
```suggestion
patterns.exclude = [
"**/*.pyc",
"**/*.pyd",
"**/*.pyo",
"**/.trunk/**/*",
"**/__pycache__/**",
".git/**/*",
".venv/**/*",
"build/**/*",
"dist/**/*",
".mypy_cache/**/*",
".pytest_cache/**/*",
".coverage",
]
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| patterns.exclude = [ | ||
| "**/*.pyc", | ||
| "**/*.pyd", | ||
| "**/*.pyo", | ||
| "**/.trunk/**/*", | ||
| "**/__pycache__/**", | ||
| ".git/**/*", | ||
| ".venv/**/*", | ||
| ] |
There was a problem hiding this comment.
suggestion: Review exclude patterns for completeness and potential edge cases.
Consider adding patterns for other generated files or directories such as build/, dist/, or .mypy_cache/ if relevant to your workflow.
| patterns.exclude = [ | |
| "**/*.pyc", | |
| "**/*.pyd", | |
| "**/*.pyo", | |
| "**/.trunk/**/*", | |
| "**/__pycache__/**", | |
| ".git/**/*", | |
| ".venv/**/*", | |
| ] | |
| patterns.exclude = [ | |
| "**/*.pyc", | |
| "**/*.pyd", | |
| "**/*.pyo", | |
| "**/.trunk/**/*", | |
| "**/__pycache__/**", | |
| ".git/**/*", | |
| ".venv/**/*", | |
| "build/**/*", | |
| "dist/**/*", | |
| ".mypy_cache/**/*", | |
| ".pytest_cache/**/*", | |
| ".coverage", | |
| ] |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully adds pycrucible as a development dependency and introduces its configuration file. The changes to pyproject.toml and the lock file are correct. However, the new pycrucible.toml configuration file contains a syntax issue in how the package patterns are defined, which will likely prevent the tool from functioning as intended. I've provided a specific comment with a suggested fix for this.
| patterns.include = ["**/*.py", "**/README.md", "**/pyproject.toml"] | ||
| patterns.exclude = [ | ||
| "**/*.pyc", | ||
| "**/*.pyd", | ||
| "**/*.pyo", | ||
| "**/.trunk/**/*", | ||
| "**/__pycache__/**", | ||
| ".git/**/*", | ||
| ".venv/**/*", | ||
| ] |
There was a problem hiding this comment.
The keys patterns.include and patterns.exclude appear to be incorrect. Within the [package.patterns] table, using dotted keys like this creates a nested table [package.patterns.patterns]. It's very likely that the intended keys are include and exclude directly under [package.patterns] for the configuration to be parsed correctly by pycrucible.
| patterns.include = ["**/*.py", "**/README.md", "**/pyproject.toml"] | |
| patterns.exclude = [ | |
| "**/*.pyc", | |
| "**/*.pyd", | |
| "**/*.pyo", | |
| "**/.trunk/**/*", | |
| "**/__pycache__/**", | |
| ".git/**/*", | |
| ".venv/**/*", | |
| ] | |
| include = ["**/*.py", "**/README.md", "**/pyproject.toml"] | |
| exclude = [ | |
| "**/*.pyc", | |
| "**/*.pyd", | |
| "**/*.pyo", | |
| "**/.trunk/**/*", | |
| "**/__pycache__/**", | |
| ".git/**/*", | |
| ".venv/**/*", | |
| ] |
There was a problem hiding this comment.
Pull Request Overview
This PR adds the pycrucible tool as a development dependency and creates its configuration file. The purpose is to integrate pycrucible for managing code sharing and synchronization with the project repository.
Key changes:
- Added
pycrucible>=0.3.0as a dev dependency - Created
pycrucible.tomlconfiguration file with entrypoint, package patterns, and source repository settings - Updated
.gitignoreto exclude pycrucible-related files and various IDE-specific rules files
Reviewed Changes
Copilot reviewed 2 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pyproject.toml | Added pycrucible as a development dependency |
| uv.lock | Lock file updated with pycrucible package metadata and wheels |
| pycrucible.toml | New configuration file defining entrypoint, included/excluded patterns, and source repository |
| .gitignore | Added exclusions for pycrucible payload directory and IDE-specific rules files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| patterns.include = ["**/*.py", "**/README.md", "**/pyproject.toml"] | ||
| patterns.exclude = [ |
There was a problem hiding this comment.
The TOML structure '[package.patterns]' followed by 'patterns.include' creates a redundant nested structure. This should either be '[package.patterns.include]' and '[package.patterns.exclude]' as separate sections, or the 'patterns.' prefix should be removed from the keys. The current structure creates 'package.patterns.patterns.include' which is likely not the intended configuration path.
| patterns.include = ["**/*.py", "**/README.md", "**/pyproject.toml"] | |
| patterns.exclude = [ | |
| include = ["**/*.py", "**/README.md", "**/pyproject.toml"] | |
| exclude = [ |
🧪 CI InsightsHere's what we observed from your CI run for 3cae4ea. 🟢 All jobs passed!But CI Insights is watching 👀 |
![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
♻️ Duplicate comments (1)
pycrucible.toml (1)
4-5: Fix TOML syntax: incorrect nested key structure.Lines 4–5 use dotted keys (
patterns.includeandpatterns.exclude), which in TOML create a nested table[package.patterns.patterns]instead of direct keys under[package.patterns]. This breaks pycrucible's configuration parsing. This issue was previously flagged and remains unresolved.Apply this diff to fix the syntax:
[package.patterns] -patterns.include = ["**/*.py", "**/README.md", "**/pyproject.toml"] -patterns.exclude = [ +include = ["**/*.py", "**/README.md", "**/pyproject.toml"] +exclude = [ "**/*.pyc", "**/*.pyd", "**/*.pyo", "**/.trunk/**/*", "**/__pycache__/**", - ".git/**/*", + ".git/**/*", ".venv/**/*", ]
🧹 Nitpick comments (1)
pycrucible.toml (1)
5-13: Consider expanding exclude patterns for completeness.The exclude list covers common Python build artifacts but omits directories found in
.gitignorelikebuild/,dist/,.mypy_cache/, and.pytest_cache/. Including these would prevent pycrucible from processing generated or cached files that should not be part of the package analysis.Consider expanding the exclude patterns:
exclude = [ "**/*.pyc", "**/*.pyd", "**/*.pyo", "**/.trunk/**/*", "**/__pycache__/**", ".git/**/*", ".venv/**/*", + "build/**/*", + "dist/**/*", + ".mypy_cache/**/*", + ".pytest_cache/**/*", + ".coverage", ]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
.gitignore(1 hunks)pycrucible.toml(1 hunks)pyproject.toml(1 hunks)
⏰ 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). (4)
- GitHub Check: Check Dependency Compatibility
- GitHub Check: Sourcery review
- GitHub Check: Mergify Merge Protections
- GitHub Check: Summary
🔇 Additional comments (3)
pyproject.toml (1)
64-64: Dependency addition looks good.The pycrucible>=0.3.0 dependency is properly placed in the dev group and uses a reasonable version constraint.
.gitignore (1)
15-24: Repository metadata updates are sensible.Moving test.http to tracked status, ignoring AI tool configurations, and excluding the pycrucible payload directory align well with the PR objectives and standard development practices.
pycrucible.toml (1)
1-1: Entrypoint path verified as valid.The file
./src/visualizr/__main__.pyexists and contains a properly structured entry point with amain()function that launches the Gradio application viaif __name__ == "__main__":. The configuration is correct.
|
Hi @MH0386, Your PR is in conflict and cannot be merged. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build.yaml(1 hunks)
⏰ 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). (3)
- GitHub Check: Sourcery review
- GitHub Check: Mergify Merge Protections
- GitHub Check: Summary
🔇 Additional comments (1)
.github/workflows/build.yaml (1)
49-66: Clarify job dependency and artifact handling.The
build_binaryjob depends onsetup_and_buildbut doesn't download thedist/artifacts produced by that job. Additionally, it repeats the checkout and uv setup steps. Please clarify:
- Is the dependency on
setup_and_buildnecessary, or should this job run independently?- What artifacts or outputs should the PyCrucible build produce, and how should they be handled (uploaded, committed, etc.)?
Without artifact handling, it's unclear how the binary distribution integrates with the release workflow.
…ant binary distribution job
MH0386
changed the title
|
Summary by Sourcery
Integrate PyCrucible by adding it as a development dependency and providing a default configuration file for its usage.
New Features: