refactor: workflows and uv#1
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughMigrates CI from GitLab to GitHub Actions, adds two reusable GitHub workflows, removes Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
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 |
Summary of ChangesHello, 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 focuses on modernizing the project's infrastructure and development workflow. It transitions the project to a new hosting platform and CI/CD system, updates the core dependency management, and ensures that development tools are current, aiming to improve efficiency and maintainability across the board. Highlights
Changelog
Ignored Files
Activity
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
|
There was a problem hiding this comment.
Code Review
This pull request migrates the repository from GitLab to GitHub, switches the dependency manager from pdm to uv, and updates various tools. The changes are generally well-executed and align with the stated goals. I've identified a couple of potential issues: a typo in the Ruff configuration that will prevent a file from being excluded from linting, and the use of GitLab URLs for dependencies which may be outdated due to the migration to GitHub. Addressing these points will help ensure the project's tooling and dependency management are correctly configured.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
.mypy.ini (1)
8-10: Consider addingwarn_unused_configsto catch typos in module patterns.While
ignore_errors = Trueis reasonable for the externally-sourcedpiqe.pyfile (which should remain read-only), thewarn_unused_configsoption in mypy can help prevent silent regressions if the module pattern is ever accidentally misspelled. This is a low-effort safety improvement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.mypy.ini around lines 8 - 10, Add the mypy option warn_unused_configs = True to the [mypy-rationai.qc.blur.piqe] section so mypy will warn if the module pattern is misspelled or unused; update the section named [mypy-rationai.qc.blur.piqe] to include warn_unused_configs = True alongside the existing ignore_errors = True.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/mkdocs-build.yml:
- Around line 3-6: The workflow triggers only list branch "main" under push and
pull_request, so CI won't run for the repo's current "master" branch; update the
trigger lists in the workflow (the push and pull_request branch arrays) to
include "master" as well (or replace with both "main" and "master" or a pattern
like ["main","master"]) so the MkDocs build will run for PRs and pushes
targeting master; locate the branch arrays under the push and pull_request keys
in the mkdocs-build.yml and add "master" to them.
In @.github/workflows/python-lint.yml:
- Around line 3-6: The workflow triggers only target branch name "main" so
pushes and pull requests against the repository's current "master" branch won't
run; update the workflow triggers in the python-lint.yml (the push and
pull_request branch arrays) to include "master" alongside "main" so the lint job
will run for both branches (i.e., add "master" to the branches lists referenced
in the push and pull_request triggers).
In @.pre-commit-config.yaml:
- Around line 17-22: Update the ruff and ruff-format pre-commit hooks to pass
the --force-exclude flag so excluded vendored files (e.g., piqe.py) are not
linted or formatted; specifically, add --force-exclude to the args array for the
hook with id "ruff" (currently args: [--fix]) and add --force-exclude to the
hook with id "ruff-format" so both hooks honor .ruff.toml exclude patterns even
when pre-commit passes staged files explicitly.
In @.ruff.toml:
- Line 5: The extend-exclude entry currently lists "rationai/qc/focus/piqe.py"
which doesn't match the actual vendored file path; update the extend-exclude
value to the correct path "rationai/qc/blur/piqe.py" (or add that path in
addition to the existing one) so Ruff will properly ignore the vendored PIQE
file; target the extend-exclude setting in the .ruff.toml configuration to make
this change.
In `@README.md`:
- Line 8: Remove the literal "TODO: Add link to mkdocs documentation" from
README.md and replace it with a real documentation link or a neutral placeholder
that won't be published (e.g., "Documentation: https://docs.example.com" or
"Documentation: link to be added"); ensure the same README referenced by
pyproject.toml (line 12) no longer contains the TODO string so package metadata
renders cleanly. Locate the README.md line that contains the TODO and update the
markdown to either the final MkDocs URL or a non-TODO placeholder; if the URL is
not yet available, use a clear placeholder like "Documentation: link to be
added" instead of "TODO".
---
Nitpick comments:
In @.mypy.ini:
- Around line 8-10: Add the mypy option warn_unused_configs = True to the
[mypy-rationai.qc.blur.piqe] section so mypy will warn if the module pattern is
misspelled or unused; update the section named [mypy-rationai.qc.blur.piqe] to
include warn_unused_configs = True alongside the existing ignore_errors = True.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e51252a5-5e4f-4b1b-97e2-120b7411afa7
⛔ Files ignored due to path filters (2)
pdm.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
.github/workflows/mkdocs-build.yml.github/workflows/python-lint.yml.gitlab-ci.yml.markdownlint.yaml.mypy.ini.pre-commit-config.yaml.ruff.tomlREADME.mdpyproject.toml
💤 Files with no reviewable changes (1)
- .gitlab-ci.yml
| push: | ||
| branches: ["main"] | ||
| pull_request: | ||
| branches: ["main"] |
There was a problem hiding this comment.
These triggers miss the repo's current master branch.
This PR targets master, but the workflow only listens to main, so linting won't run for the current PR flow or post-merge pushes until the branch rename is complete.
Suggested change
push:
- branches: ["main"]
+ branches: ["main", "master"]
pull_request:
- branches: ["main"]
+ branches: ["main", "master"]📝 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.
| push: | |
| branches: ["main"] | |
| pull_request: | |
| branches: ["main"] | |
| push: | |
| branches: ["main", "master"] | |
| pull_request: | |
| branches: ["main", "master"] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/python-lint.yml around lines 3 - 6, The workflow triggers
only target branch name "main" so pushes and pull requests against the
repository's current "master" branch won't run; update the workflow triggers in
the python-lint.yml (the push and pull_request branch arrays) to include
"master" alongside "main" so the lint job will run for both branches (i.e., add
"master" to the branches lists referenced in the push and pull_request
triggers).
| hooks: | ||
| # Run the linter. | ||
| - id: ruff | ||
| entry: pdm lint --force-exclude | ||
| args: [--fix] | ||
| # Run the formatter. | ||
| - id: ruff-format |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
python -m pip install --quiet "ruff==0.14.10"
cat >"$tmp/.ruff.toml" <<'EOF'
extend-exclude = ["excluded.py"]
EOF
cat >"$tmp/excluded.py" <<'EOF'
import os
EOF
cd "$tmp"
echo "ruff check without --force-exclude"
python -m ruff check --config .ruff.toml excluded.py || true
echo
echo "ruff check with --force-exclude"
python -m ruff check --force-exclude --config .ruff.toml excluded.py || true
echo
echo "ruff format without --force-exclude"
python -m ruff format --check --config .ruff.toml excluded.py || true
echo
echo "ruff format with --force-exclude"
python -m ruff format --check --force-exclude --config .ruff.toml excluded.py || trueRepository: RationAI/quality-control
Length of output: 752
🏁 Script executed:
git ls-files | grep -E "(\.ruff\.toml|\.pre-commit-config\.yaml|piqe\.py)" | head -20Repository: RationAI/quality-control
Length of output: 163
🏁 Script executed:
cat -n .ruff.toml | head -20Repository: RationAI/quality-control
Length of output: 640
🏁 Script executed:
cat -n .pre-commit-config.yaml | sed -n '15,25p'Repository: RationAI/quality-control
Length of output: 321
Add --force-exclude to both Ruff hooks to ensure the excluded vendored file is not linted or formatted.
Pre-commit passes staged files explicitly to Ruff, which bypasses extend-exclude patterns in .ruff.toml without the --force-exclude flag. This can cause the excluded piqe.py file to be checked and formatted unexpectedly.
Suggested change
- id: ruff
args: [--fix, --force-exclude]
# Run the formatter.
- id: ruff-format
+ args: [--force-exclude]📝 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.
| hooks: | |
| # Run the linter. | |
| - id: ruff | |
| entry: pdm lint --force-exclude | |
| args: [--fix] | |
| # Run the formatter. | |
| - id: ruff-format | |
| hooks: | |
| # Run the linter. | |
| - id: ruff | |
| args: [--fix, --force-exclude] | |
| # Run the formatter. | |
| - id: ruff-format | |
| args: [--force-exclude] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.pre-commit-config.yaml around lines 17 - 22, Update the ruff and
ruff-format pre-commit hooks to pass the --force-exclude flag so excluded
vendored files (e.g., piqe.py) are not linted or formatted; specifically, add
--force-exclude to the args array for the hook with id "ruff" (currently args:
[--fix]) and add --force-exclude to the hook with id "ruff-format" so both hooks
honor .ruff.toml exclude patterns even when pre-commit passes staged files
explicitly.
| of the complete QC methods to allow for easier debugging and experimenting. | ||
|
|
||
| TODO: Add link to mkdocs documentation No newline at end of file | ||
| TODO: Add link to mkdocs documentation |
There was a problem hiding this comment.
Don't ship a literal TODO in the published README.
README.md is also the project readme in pyproject.toml, Line 12, so this placeholder will show up anywhere the package metadata is rendered.
If you want, I can draft the final docs-link line once the MkDocs URL is settled.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` at line 8, Remove the literal "TODO: Add link to mkdocs
documentation" from README.md and replace it with a real documentation link or a
neutral placeholder that won't be published (e.g., "Documentation:
https://docs.example.com" or "Documentation: link to be added"); ensure the same
README referenced by pyproject.toml (line 12) no longer contains the TODO string
so package metadata renders cleanly. Locate the README.md line that contains the
TODO and update the markdown to either the final MkDocs URL or a non-TODO
placeholder; if the URL is not yet available, use a clear placeholder like
"Documentation: link to be added" instead of "TODO".
This pull request includes the following changes:
pdmwith theuvdependency manager.Any suggestions for improvement are appreciated. 😄
Summary by CodeRabbit
Chores
CI/CD
Style & Linting
Documentation