Conversation
📝 WalkthroughWalkthroughThis PR performs a comprehensive version bump and configuration update across the project infrastructure. It upgrades development tooling versions, GitHub Actions, devcontainer features, CI/CD dependencies, and pre-commit hooks in both root and template directories, with minor CI workflow logic adjustments to validate job outcomes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 7
♻️ Duplicate comments (1)
.github/pull_request_template.md (1)
1-1: Same MD041markdownlintwarning astemplate/.github/pull_request_template.md.Both files need the same fix — see the comment in the template file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/pull_request_template.md at line 1, The file triggers MD041 because it lacks a top-level heading; prepend a single H1 heading before the existing "## Link to Issue or Message thread" line (or change that line to a top-level heading) so the document begins with a top-level heading; ensure no content appears before the new H1 to satisfy markdownlint.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.devcontainer/Dockerfile:
- Around line 9-10: The Dockerfile RUN line uses curl without --fail which can
mask HTTP errors; update the RUN command that removes yarn.list and fetches the
Yarn GPG key so the curl invocation includes --fail (i.e., change "curl -sS" to
"curl -sS --fail") to ensure non-2xx responses cause the build to fail and
surface download errors when executing that RUN layer.
In @.github/workflows/ci.yaml:
- Around line 155-161: The current required-check accepts skipped results and
only inspects needs.lint-matrix.result; change the logic so success_pattern is
only "^success$" and verify both needs.get-values.result and
needs.lint-matrix.result equal "success" (do not treat "skipped" as acceptable).
Update the conditional that currently tests "${{ needs.lint-matrix.result }}" to
instead check both "${{ needs.get-values.result }}" and "${{
needs.lint-matrix.result }}" against the new success_pattern (or test equality
to "success") and make the step fail (exit 1) if either is not "success".
In `@copier.yml`:
- Line 79: Update the Python version pin in copier.yml by replacing the literal
"3.13.9" with "3.13.10" (i.e., change the version string in copier.yml), then
run your CI/tests to ensure no regressions; if the intention was to stay on
3.13.9, add a short comment explaining why the older patch is required instead
of upgrading.
- Around line 146-155: The inline sed invocation in the copier _tasks block (the
line using sed -i -E to edit ruff.toml) is not cross-platform and will fail on
macOS; change the task to use a portable approach such as either: detect Darwin
via uname and use sed -i '' -E on macOS and sed -i -E elsewhere, or replace the
sed command entirely with a cross-platform single-line Python or Perl in-place
edit that uses the same regex to update target-version (operating on ruff.toml
and using the same py_major_minor/py_tag variables).
In `@pyproject.toml`:
- Line 16: The dependency constraint for ty in pyproject.toml is too loose
("ty>=0.0.17"); change it to an exact pin (e.g., "ty==0.0.17") so the package is
version-fixed like copier (==9.11.3), preventing accidental pulls of
incompatible 0.0.x releases; update the ty entry in pyproject.toml accordingly.
In `@template/.devcontainer/Dockerfile`:
- Around line 9-10: The RUN line that pipes the Yarn GPG key URL into gpg (the
command invoking curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | gpg
--dearmor | tee /etc/apt/keyrings/yarn-archive-keyring.gpg) should add curl's
--fail flag (and optionally --show-error) so HTTP 4xx/5xx responses cause curl
to exit non-zero instead of passing an HTML error page into gpg; update that RUN
pipeline to use curl --fail -sS (and keep the existing gpg --dearmor | tee ...)
so the build fails with a clear HTTP error when the key URL is unavailable or
returns an error.
In `@template/.github/pull_request_template.md`:
- Line 1: The file currently starts with "## Link to Issue or Message thread"
which triggers markdownlint MD041 because the first line is an H2; fix by making
the first line a top-level heading (change to "# Pull Request" or insert a
leading H1 title above "## Link to Issue or Message thread") or, if you prefer
to suppress the rule globally for PR templates, add MD041: false (or scope it to
templates) in .markdownlint.yaml; update whichever approach you choose so the
first line is an H1 or the rule is disabled for this file.
---
Duplicate comments:
In @.github/pull_request_template.md:
- Line 1: The file triggers MD041 because it lacks a top-level heading; prepend
a single H1 heading before the existing "## Link to Issue or Message thread"
line (or change that line to a top-level heading) so the document begins with a
top-level heading; ensure no content appears before the new H1 to satisfy
markdownlint.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (27)
.copier-answers.yml.devcontainer/Dockerfile.devcontainer/devcontainer.json.devcontainer/install-ci-tooling.py.github/actions/update-devcontainer-hash/action.yml.github/pull_request_template.md.github/reusable_workflows/build-docker-image.yaml.github/workflows/ci.yaml.github/workflows/get-values.yaml.github/workflows/pre-commit.yaml.gitignore.pre-commit-config.yamlcopier.ymlextensions/context.pypyproject.tomlruff.tomltemplate/.devcontainer/Dockerfiletemplate/.devcontainer/devcontainer.json.jinjatemplate/.github/actions/update-devcontainer-hash/action.ymltemplate/.github/pull_request_template.mdtemplate/.github/workflows/ci.yaml.jinjatemplate/.github/workflows/get-values.yamltemplate/.github/workflows/pre-commit.yamltemplate/.gitignoretemplate/.pre-commit-config.yamltemplate/ruff.tomltests/copier_data/data2.yaml
| RUN rm /etc/apt/sources.list.d/yarn.list || true && \ | ||
| curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | gpg --dearmor | tee /etc/apt/keyrings/yarn-archive-keyring.gpg > /dev/null |
There was a problem hiding this comment.
Same curl -sS without --fail issue as in template/.devcontainer/Dockerfile.
Apply the same fix here for consistency.
🛠️ Proposed fix
- curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | gpg --dearmor | tee /etc/apt/keyrings/yarn-archive-keyring.gpg > /dev/null
+ curl -fsSL https://dl.yarnpkg.com/debian/pubkey.gpg | gpg --dearmor | tee /etc/apt/keyrings/yarn-archive-keyring.gpg > /dev/null📝 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.
| RUN rm /etc/apt/sources.list.d/yarn.list || true && \ | |
| curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | gpg --dearmor | tee /etc/apt/keyrings/yarn-archive-keyring.gpg > /dev/null | |
| RUN rm /etc/apt/sources.list.d/yarn.list || true && \ | |
| curl -fsSL https://dl.yarnpkg.com/debian/pubkey.gpg | gpg --dearmor | tee /etc/apt/keyrings/yarn-archive-keyring.gpg > /dev/null |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.devcontainer/Dockerfile around lines 9 - 10, The Dockerfile RUN line uses
curl without --fail which can mask HTTP errors; update the RUN command that
removes yarn.list and fetches the Yarn GPG key so the curl invocation includes
--fail (i.e., change "curl -sS" to "curl -sS --fail") to ensure non-2xx
responses cause the build to fail and surface download errors when executing
that RUN layer.
| success_pattern="^(skipped|success)$" # these are the possibilities: https://docs.github.com/en/actions/reference/workflows-and-actions/contexts#needs-context | ||
|
|
||
| if [[ ! "${{ needs.lint-matrix.result }}" =~ $success_pattern ]]; then | ||
| echo "❌ One or more jobs did not finish with skipped or success" | ||
| exit 1 | ||
| fi | ||
| echo "✅ All jobs finished with skipped or success" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's find and examine the ci.yaml file
find . -name "ci.yaml" -o -name "ci.yml" | head -20Repository: LabAutomationAndScreening/copier-python-package-template
Length of output: 127
🌐 Web query:
GitHub Actions needs context job result values skipped failure success
💡 Result:
In GitHub Actions, when job B has needs: A, the needs context exposes A’s result as:
needs.A.result→ one of:success,failure,cancelled,skipped. [1]
Common patterns:
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: exit 0
deploy:
needs: build
# Default behavior is effectively success() gating unless you override it
if: ${{ always() && needs.build.result != 'cancelled' }}
runs-on: ubuntu-latest
steps:
- run: echo "build result was ${{ needs.build.result }}"Notes:
- A job skipped via
jobs.<job_id>.if: <false>is marked “skipped”, and the UI may show it as “Success” (it won’t block required checks), but itsneeds.<job_id>.resultis stillskipped. [2] - In
if:conditionals, a default status check ofsuccess()is applied unless you include a status-check function likealways(),failure(),cancelled(), orsuccess(). [3]
Sources: GitHub Actions contexts reference (needs context) [1], job conditions docs (skipped job behavior) [2], expressions/status-check functions docs [3].
🏁 Script executed:
#!/bin/bash
# Test the regex pattern behavior
success_pattern='^(skipped|success)$'
echo "=== Testing regex pattern: $success_pattern ==="
for result in success skipped failure cancelled skipped_by_conditional; do
if [[ ! "$result" =~ $success_pattern ]]; then
outcome="FAILS"
else
outcome="PASSES"
fi
printf '%-25s -> %s\n' "$result" "$outcome"
doneRepository: LabAutomationAndScreening/copier-python-package-template
Length of output: 327
🏁 Script executed:
#!/bin/bash
# Read the ci.yaml file to understand the workflow structure
wc -l ./.github/workflows/ci.yamlRepository: LabAutomationAndScreening/copier-python-package-template
Length of output: 131
🏁 Script executed:
#!/bin/bash
# Read the section around the review lines (155-161) plus context
sed -n '1,180p' ./.github/workflows/ci.yaml | cat -nRepository: LabAutomationAndScreening/copier-python-package-template
Length of output: 8482
Required-check incorrectly allows upstream failures to be reported as success.
When pre-commit fails, lint-matrix is automatically skipped (due to its needs: pre-commit dependency without an explicit if: always() condition). The current regex pattern ^(skipped|success)$ treats skipped jobs as acceptable, allowing the required-check to pass despite upstream failures. This masks real failures.
The required-check must only accept success and should verify both get-values and lint-matrix completed successfully:
Fix required
- success_pattern="^(skipped|success)$" # these are the possibilities: https://docs.github.com/en/actions/reference/workflows-and-actions/contexts#needs-context
-
- if [[ ! "${{ needs.lint-matrix.result }}" =~ $success_pattern ]]; then
- echo "❌ One or more jobs did not finish with skipped or success"
+ required_pattern="^success$"
+ if [[ ! "${{ needs.get-values.result }}" =~ $required_pattern ]] || [[ ! "${{ needs.lint-matrix.result }}" =~ $required_pattern ]]; then
+ echo "❌ One or more required jobs did not finish with success"
exit 1
fi
- echo "✅ All jobs finished with skipped or success"
+ echo "✅ All required jobs finished with success"📝 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.
| success_pattern="^(skipped|success)$" # these are the possibilities: https://docs.github.com/en/actions/reference/workflows-and-actions/contexts#needs-context | |
| if [[ ! "${{ needs.lint-matrix.result }}" =~ $success_pattern ]]; then | |
| echo "❌ One or more jobs did not finish with skipped or success" | |
| exit 1 | |
| fi | |
| echo "✅ All jobs finished with skipped or success" | |
| required_pattern="^success$" | |
| if [[ ! "${{ needs.get-values.result }}" =~ $required_pattern ]] || [[ ! "${{ needs.lint-matrix.result }}" =~ $required_pattern ]]; then | |
| echo "❌ One or more required jobs did not finish with success" | |
| exit 1 | |
| fi | |
| echo "✅ All required jobs finished with success" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yaml around lines 155 - 161, The current required-check
accepts skipped results and only inspects needs.lint-matrix.result; change the
logic so success_pattern is only "^success$" and verify both
needs.get-values.result and needs.lint-matrix.result equal "success" (do not
treat "skipped" as acceptable). Update the conditional that currently tests "${{
needs.lint-matrix.result }}" to instead check both "${{ needs.get-values.result
}}" and "${{ needs.lint-matrix.result }}" against the new success_pattern (or
test equality to "success") and make the step fail (exit 1) if either is not
"success".
| choices: | ||
| - "3.12.7" | ||
| - "3.13.2" | ||
| - "3.13.9" |
There was a problem hiding this comment.
Python 3.13.9 is not the latest 3.13.x patch.
Python 3.13.10 was released on December 2, 2025, meaning 3.13.9 is one patch behind. Consider whether this should be bumped to "3.13.10" for the latest security/bug fixes, or confirm this is intentional.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@copier.yml` at line 79, Update the Python version pin in copier.yml by
replacing the literal "3.13.9" with "3.13.10" (i.e., change the version string
in copier.yml), then run your CI/tests to ensure no regressions; if the
intention was to stay on 3.13.9, add a short comment explaining why the older
patch is required instead of upgrading.
| _tasks: | ||
| - command: | | ||
| if [ -f ruff.toml ]; then | ||
| echo "Updating ruff target-version from python_version..." | ||
| py_major_minor="$(printf '%s' '{{ python_version }}' | cut -d. -f1,2)" | ||
| py_tag="py$(printf '%s' "$py_major_minor" | tr -d '.')" | ||
| sed -i -E 's/^target-version = "py[0-9]+"/target-version = "'"$py_tag"'"/' ruff.toml | ||
| else | ||
| echo "ruff.toml not found; skipping Ruff target-version update." | ||
| fi |
There was a problem hiding this comment.
sed -i -E will fail on macOS (BSD sed).
GNU sed (Linux) accepts sed -i -E for in-place editing, but BSD sed (macOS) requires an explicit backup extension argument: sed -i '' -E. As written, running this task locally on a macOS developer machine will fail. Use a cross-platform approach instead:
🐛 Proposed cross-platform fix
- sed -i -E 's/^target-version = "py[0-9]+"/target-version = "'"$py_tag"'"/' ruff.toml
+ if sed --version 2>/dev/null | grep -q GNU; then
+ sed -i -E 's/^target-version = "py[0-9]+"/target-version = "'"$py_tag"'"/' ruff.toml
+ else
+ sed -i '' -E 's/^target-version = "py[0-9]+"/target-version = "'"$py_tag"'"/' ruff.toml
+ fiAlternatively, use python -c or perl -i -pe which are available and behave consistently on both platforms.
📝 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.
| _tasks: | |
| - command: | | |
| if [ -f ruff.toml ]; then | |
| echo "Updating ruff target-version from python_version..." | |
| py_major_minor="$(printf '%s' '{{ python_version }}' | cut -d. -f1,2)" | |
| py_tag="py$(printf '%s' "$py_major_minor" | tr -d '.')" | |
| sed -i -E 's/^target-version = "py[0-9]+"/target-version = "'"$py_tag"'"/' ruff.toml | |
| else | |
| echo "ruff.toml not found; skipping Ruff target-version update." | |
| fi | |
| _tasks: | |
| - command: | | |
| if [ -f ruff.toml ]; then | |
| echo "Updating ruff target-version from python_version..." | |
| py_major_minor="$(printf '%s' '{{ python_version }}' | cut -d. -f1,2)" | |
| py_tag="py$(printf '%s' "$py_major_minor" | tr -d '.')" | |
| if sed --version 2>/dev/null | grep -q GNU; then | |
| sed -i -E 's/^target-version = "py[0-9]+"/target-version = "'"$py_tag"'"/' ruff.toml | |
| else | |
| sed -i '' -E 's/^target-version = "py[0-9]+"/target-version = "'"$py_tag"'"/' ruff.toml | |
| fi | |
| else | |
| echo "ruff.toml not found; skipping Ruff target-version update." | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@copier.yml` around lines 146 - 155, The inline sed invocation in the copier
_tasks block (the line using sed -i -E to edit ruff.toml) is not cross-platform
and will fail on macOS; change the task to use a portable approach such as
either: detect Darwin via uname and use sed -i '' -E on macOS and sed -i -E
elsewhere, or replace the sed command entirely with a cross-platform single-line
Python or Perl in-place edit that uses the same regex to update target-version
(operating on ruff.toml and using the same py_major_minor/py_tag variables).
| "pyright[nodejs]>=1.1.408", | ||
| "ty>=0.0.13", | ||
| "copier==9.11.2", | ||
| "ty>=0.0.17", |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider pinning ty to an exact version, consistent with copier.
ty uses 0.0.x versioning and does not yet have a stable API; breaking changes, including changes to diagnostics, may occur between any two versions. The loose >=0.0.17 constraint allows pip to silently pull in a breaking 0.0.x release, which could unexpectedly break CI. Since copier is pinned exactly (==9.11.3), the same strategy should apply here.
📌 Proposed fix: pin ty to an exact version
- "ty>=0.0.17",
+ "ty==0.0.17",📝 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.
| "ty>=0.0.17", | |
| "ty==0.0.17", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pyproject.toml` at line 16, The dependency constraint for ty in
pyproject.toml is too loose ("ty>=0.0.17"); change it to an exact pin (e.g.,
"ty==0.0.17") so the package is version-fixed like copier (==9.11.3), preventing
accidental pulls of incompatible 0.0.x releases; update the ty entry in
pyproject.toml accordingly.
| RUN rm /etc/apt/sources.list.d/yarn.list || true && \ | ||
| curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | gpg --dearmor | tee /etc/apt/keyrings/yarn-archive-keyring.gpg > /dev/null |
There was a problem hiding this comment.
curl -sS without --fail silently swallows HTTP error responses.
If the Yarn GPG URL returns an HTTP 4xx/5xx, curl -sS exits 0 and passes the error page body to gpg --dearmor. While gpg will then exit non-zero (saving the build from silently installing a corrupt keyring), the failure message will be an opaque GPG parse error rather than a clear HTTP error. Adding --fail makes the failure point explicit and the error message actionable.
🛠️ Proposed fix
- curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | gpg --dearmor | tee /etc/apt/keyrings/yarn-archive-keyring.gpg > /dev/null
+ curl -fsSL https://dl.yarnpkg.com/debian/pubkey.gpg | gpg --dearmor | tee /etc/apt/keyrings/yarn-archive-keyring.gpg > /dev/null📝 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.
| RUN rm /etc/apt/sources.list.d/yarn.list || true && \ | |
| curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | gpg --dearmor | tee /etc/apt/keyrings/yarn-archive-keyring.gpg > /dev/null | |
| RUN rm /etc/apt/sources.list.d/yarn.list || true && \ | |
| curl -fsSL https://dl.yarnpkg.com/debian/pubkey.gpg | gpg --dearmor | tee /etc/apt/keyrings/yarn-archive-keyring.gpg > /dev/null |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@template/.devcontainer/Dockerfile` around lines 9 - 10, The RUN line that
pipes the Yarn GPG key URL into gpg (the command invoking curl -sS
https://dl.yarnpkg.com/debian/pubkey.gpg | gpg --dearmor | tee
/etc/apt/keyrings/yarn-archive-keyring.gpg) should add curl's --fail flag (and
optionally --show-error) so HTTP 4xx/5xx responses cause curl to exit non-zero
instead of passing an HTML error page into gpg; update that RUN pipeline to use
curl --fail -sS (and keep the existing gpg --dearmor | tee ...) so the build
fails with a clear HTTP error when the key URL is unavailable or returns an
error.
| @@ -1,21 +1,21 @@ | |||
| ## Link to Issue or Message thread | |||
| ## Link to Issue or Message thread | |||
There was a problem hiding this comment.
markdownlint MD041 warning: first line should be a top-level heading.
Both this file and .github/pull_request_template.md trigger MD041 because the first line is ## (H2) rather than # (H1). If the markdownlint rule is enforced in CI, this will fail. Consider either adding an H1 title before the first H2, or suppressing the rule for PR templates via a .markdownlint.yaml config.
📝 Option 1: Add an H1 title
+# Pull Request
+
## Link to Issue or Message thread📝 Option 2: Suppress MD041 for this file in markdownlint config
# .markdownlint.yaml
MD041: falseOr scope it to the template paths only.
📝 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.
| ## Link to Issue or Message thread | |
| # Pull Request | |
| ## Link to Issue or Message thread |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@template/.github/pull_request_template.md` at line 1, The file currently
starts with "## Link to Issue or Message thread" which triggers markdownlint
MD041 because the first line is an H2; fix by making the first line a top-level
heading (change to "# Pull Request" or insert a leading H1 title above "## Link
to Issue or Message thread") or, if you prefer to suppress the rule globally for
PR templates, add MD041: false (or scope it to templates) in .markdownlint.yaml;
update whichever approach you choose so the first line is an H1 or the rule is
disabled for this file.
1 participant
Pull in upstream template changes
Tested in downstream repo
Summary by CodeRabbit