fix: update scaffolding to make full use of pixi#116
fix: update scaffolding to make full use of pixi#116johanneskoester wants to merge 8 commits intomainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaced Poetry-based dev workflow with Pixi, added pytest-cov and pyrefly, moved coverage config into pyproject.toml, updated CI jobs to run pixi commands (format/typecheck/test), and removed flake8 block and rendering of Changes
Sequence Diagram(s)sequenceDiagram
actor GitHubActions as "GitHub Actions"
participant Repo as "Repository"
participant Pixi as "prefix-dev/setup-pixi"
participant Runner as "Job Runner"
participant Tools as "Tools (pytest, pyrefly, coverage)"
GitHubActions->>Repo: checkout (actions/checkout@v6)
GitHubActions->>Pixi: setup pixi action
GitHubActions->>Runner: start job (format / typecheck / test)
Runner->>Pixi: execute `pixi run <task>`
Pixi->>Tools: invoke corresponding toolchain (format, lint, typecheck, test, coverage-report)
Tools->>Repo: read source & tests
Tools-->>Pixi: return results/artifacts
Pixi-->>GitHubActions: report job status and attach artifacts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
snakedeploy/scaffold_plugins/common.py (1)
118-133:⚠️ Potential issue | 🔴 CriticalTwo bugs in the test task setup: hyphenated
--covargument and an undefinedcoverage-reporttask.Bug 1 (Major) —
--covreceives a hyphenated package name.
package_nameis the raw project name (e.g.snakemake-storage-plugin-s3).pytest-cov's--covflag expects a Python importable path, which uses underscores. Passing a hyphenated name causes pytest-cov to silently measure nothing, resulting in 0% coverage always passing or erroneously failing.Compare with line 169, which correctly uses
.replace("-", "_").Bug 2 (Critical) —
coverage-reportpixi task is never defined but called in the CI template.
ci.yml.j2line 58 runspixi run coverage-report, but no correspondingpixi task add ... coverage-report ...call exists here. The CI will unconditionally fail on that step.🐛 Proposed fix
- "pytest " - f"--cov={package_name} " + "pytest " + f"--cov={package_name.replace('-', '_')} "And add a
coverage-reporttask (or remove the step from the CI template if inline reporting is sufficient):+ sp.run( + [ + "pixi", + "task", + "add", + "--feature", + "dev", + "coverage-report", + "coverage report", + ], + check=True, + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@snakedeploy/scaffold_plugins/common.py` around lines 118 - 133, The pixi test task invocation in the sp.run call passes the raw package_name (e.g., "snakemake-storage-plugin-s3") to pytest-cov causing a hyphenated --cov value; change usage to the importable name by replacing hyphens with underscores (use the same .replace("-", "_") pattern used elsewhere) when building the "--cov=" argument for the pixi "task add" that creates the "test" feature; additionally add a separate pixi "coverage-report" task (or update CI) so the CI step that runs "pixi run coverage-report" has a matching task definition rather than referencing a non-existent task.
🧹 Nitpick comments (2)
snakedeploy/scaffold_plugins/common.py (1)
114-117: Ruff S607: partial executable path"pixi"insp.run.This is consistent with every other
sp.run(["pixi", ...])call in the file, so flagging it only here is an artifact of the diff. If you want to suppress S607 project-wide for these calls, you can resolve the executable path first or add a# noqa: S607comment. No change is strictly required.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@snakedeploy/scaffold_plugins/common.py` around lines 114 - 117, The sp.run call using a partial executable path ("pixi") in the sp.run([...]) invocation (see sp.run in this file) triggers Ruff S607; fix by resolving the full executable path before invoking sp.run (e.g., use shutil.which("pixi") and pass the returned absolute path or raise a clear error if not found), or if you intentionally rely on the PATH, add a localized suppression comment (# noqa: S607) on the sp.run line to silence the linter; apply the same approach consistently for other sp.run(["pixi", ...]) calls such as the one that adds the "dev" feature with "typecheck" and "pyrefly check".snakedeploy/templates/plugins/ci.yml.j2 (1)
17-17: Pinsetup-pixito a specific version rather than the floating@v0tag.Since pixi is not yet stable, the API of this action may change between minor versions. The documentation recommends pinning to a specific version (e.g.,
prefix-dev/setup-pixi@v0.9.4) to avoid breaking changes. This applies to all four occurrences in the file (lines 17, 29, 41, and 52).♻️ Proposed fix (all four occurrences)
- uses: prefix-dev/setup-pixi@v0 + uses: prefix-dev/setup-pixi@v0.9.4🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@snakedeploy/templates/plugins/ci.yml.j2` at line 17, The CI workflow uses the unstable floating tag "prefix-dev/setup-pixi@v0" in multiple places; update all four occurrences of the GitHub Action usage (the lines containing "uses: prefix-dev/setup-pixi@v0") to pin a specific patch version such as "prefix-dev/setup-pixi@v0.9.4" (or whatever tested release you want) so the action API won't change unexpectedly—replace each "prefix-dev/setup-pixi@v0" instance in the template with the chosen exact version.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@snakedeploy/scaffold_plugins/common.py`:
- Around line 68-77: pyproject["tool"] may not exist and will raise KeyError;
before writing into pyproject["tool"]["coverage"] (the block that sets "report"
and "fail_under") ensure pyproject has a "tool" dict by doing a safe default
(e.g., if "tool" not in pyproject: pyproject["tool"] = {}), then set
pyproject["tool"]["coverage"] = { ... }; apply the same defensive check to the
other place in this file that directly accesses pyproject["tool"] so both spots
guard against missing "tool".
In `@snakedeploy/templates/plugins/ci.yml.j2`:
- Line 20: The CI step currently runs "pixi run format --check" (and other pixi
run invocations for lint, typecheck, test) in the default environment which
lacks the dev-feature tasks; update each "pixi run ..." invocation to include
the dev environment flag (e.g. add "-e dev") or alternatively add "environments:
dev" to the corresponding "setup-pixi" steps so the dev environment is
installed; search for the literal "pixi run format --check" and other "pixi run"
commands in the ci template (ci.yml.j2) and make the change consistently across
the steps referenced (lines around the format/lint/typecheck/test steps).
---
Outside diff comments:
In `@snakedeploy/scaffold_plugins/common.py`:
- Around line 118-133: The pixi test task invocation in the sp.run call passes
the raw package_name (e.g., "snakemake-storage-plugin-s3") to pytest-cov causing
a hyphenated --cov value; change usage to the importable name by replacing
hyphens with underscores (use the same .replace("-", "_") pattern used
elsewhere) when building the "--cov=" argument for the pixi "task add" that
creates the "test" feature; additionally add a separate pixi "coverage-report"
task (or update CI) so the CI step that runs "pixi run coverage-report" has a
matching task definition rather than referencing a non-existent task.
---
Duplicate comments:
In `@snakedeploy/templates/plugins/ci.yml.j2`:
- Around line 57-58: The CI step runs a non-existent pixi task
"coverage-report"; either add a pixi task named "coverage-report" in your
scaffolding (e.g. in common.py use pixi task add ... coverage-report ... to run
"coverage report" or "coverage xml") or remove the "pixi run coverage-report"
step from the CI template and rely on the existing "test" pixi task (which
already produces XML and terminal summary via --cov-report); update the template
to reference the actual task name you choose and ensure common.py contains the
corresponding pixi task definition.
---
Nitpick comments:
In `@snakedeploy/scaffold_plugins/common.py`:
- Around line 114-117: The sp.run call using a partial executable path ("pixi")
in the sp.run([...]) invocation (see sp.run in this file) triggers Ruff S607;
fix by resolving the full executable path before invoking sp.run (e.g., use
shutil.which("pixi") and pass the returned absolute path or raise a clear error
if not found), or if you intentionally rely on the PATH, add a localized
suppression comment (# noqa: S607) on the sp.run line to silence the linter;
apply the same approach consistently for other sp.run(["pixi", ...]) calls such
as the one that adds the "dev" feature with "typecheck" and "pyrefly check".
In `@snakedeploy/templates/plugins/ci.yml.j2`:
- Line 17: The CI workflow uses the unstable floating tag
"prefix-dev/setup-pixi@v0" in multiple places; update all four occurrences of
the GitHub Action usage (the lines containing "uses: prefix-dev/setup-pixi@v0")
to pin a specific patch version such as "prefix-dev/setup-pixi@v0.9.4" (or
whatever tested release you want) so the action API won't change
unexpectedly—replace each "prefix-dev/setup-pixi@v0" instance in the template
with the chosen exact version.
| # configure coverage | ||
| pyproject["tool"]["coverage"] = { | ||
| "report": { | ||
| "exclude_lines": [ | ||
| "pass", | ||
| "\\.\\.\\.", | ||
| ], | ||
| "fail_under": 90.0, | ||
| } | ||
| } |
There was a problem hiding this comment.
pyproject["tool"] will raise KeyError if the initial pyproject.toml has no [tool] section.
At this point in execution the file hasn't been touched by any pixi command yet, so the [tool] key must already be present in the user's pyproject.toml. While pixi-generated manifests do include [tool.pixi], user-created files may not. The same fragility exists at line 103, but that executes after pixi has already modified the file.
🛡️ Proposed defensive fix
+ pyproject.setdefault("tool", {})
pyproject["tool"]["coverage"] = {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@snakedeploy/scaffold_plugins/common.py` around lines 68 - 77,
pyproject["tool"] may not exist and will raise KeyError; before writing into
pyproject["tool"]["coverage"] (the block that sets "report" and "fail_under")
ensure pyproject has a "tool" dict by doing a safe default (e.g., if "tool" not
in pyproject: pyproject["tool"] = {}), then set pyproject["tool"]["coverage"] =
{ ... }; apply the same defensive check to the other place in this file that
directly accesses pyproject["tool"] so both spots guard against missing "tool".
| - name: Install poetry | ||
| run: pip install poetry | ||
| - name: Check formatting | ||
| run: pixi run format --check |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | grep -E "(ci\.yml|common\.py|pixi)" | head -20Repository: snakemake/snakedeploy
Length of output: 143
🏁 Script executed:
find . -type f -name "*.py" -o -name "*.yml*" | grep -E "(ci|common|pixi)" | head -30Repository: snakemake/snakedeploy
Length of output: 147
🏁 Script executed:
cat -n snakedeploy/templates/plugins/ci.yml.j2Repository: snakemake/snakedeploy
Length of output: 1533
🏁 Script executed:
cat -n snakedeploy/scaffold_plugins/common.pyRepository: snakemake/snakedeploy
Length of output: 7298
🏁 Script executed:
find . -type f \( -name "pixi.toml" -o -name "pyproject.toml" \) | head -5Repository: snakemake/snakedeploy
Length of output: 81
🏁 Script executed:
cat pyproject.tomlRepository: snakemake/snakedeploy
Length of output: 2054
🏁 Script executed:
# Search for any pixi documentation or examples in the repo
find . -type f \( -name "*.md" -o -name "*.txt" \) | xargs grep -l "pixi" 2>/dev/null | head -5Repository: snakemake/snakedeploy
Length of output: 79
🏁 Script executed:
# Check CHANGELOG for any pixi-related information
cat CHANGELOG.md | grep -i pixi -A3 -B3 | head -50Repository: snakemake/snakedeploy
Length of output: 439
🏁 Script executed:
# Search for how pixi tasks are used elsewhere in the codebase
rg "pixi run" --include="*.md" --include="*.yml" --include="*.yaml" --include="*.sh" -A2 -B2Repository: snakemake/snakedeploy
Length of output: 506
🏁 Script executed:
# Search for how pixi tasks are used elsewhere in the codebase (corrected)
rg "pixi run" -A2 -B2Repository: snakemake/snakedeploy
Length of output: 1658
🏁 Script executed:
# Search for pixi behavior documentation or comments
rg "pixi.*run.*-e|pixi.*environment" -A2 -B2Repository: snakemake/snakedeploy
Length of output: 1068
🏁 Script executed:
# Check the release_please template to see how it handles environments
cat -n snakedeploy/templates/plugins/release_please.yml.j2Repository: snakemake/snakedeploy
Length of output: 1546
🏁 Script executed:
# Search for any setup-pixi configuration with environments
rg "setup-pixi" -A5 -B5Repository: snakemake/snakedeploy
Length of output: 3571
🌐 Web query:
pixi run task feature environment resolution default
💡 Result:
In Pixi, the environment used by pixi run is resolved roughly in this order:
-
Per dependency in
depends-on(highest precedence)
If a task dependency is declared with an explicit environment, that dependency always runs in that environment—even if you pass--environment/-eon the CLI. [1] -
CLI
--environment/-e <ENV>
If you specify an environment on the command line, Pixi runs the task in that environment (unless overridden by rule#1for dependencies). [2] -
Task-level
default-environment
A task can declaredefault-environment = "name"; Pixi uses that environment when you run the task, but it can still be overridden by-e(again, except for explicitly-environment-pinned dependencies). [1] -
Otherwise: the
defaultenvironment
If you don’t specify anything, Pixi runs in thedefaultenvironment (and.pixi/envs/defaultis the conventional on-disk location). [3]
Notes on features ↔ environments:
- An environment is defined as a collection of features in
[environments]. [4] - Unless you opt out, the default feature is implicitly included in environments. (Dependencies in top-level
[dependencies]are part of that default feature.) [4] - If a task is available in multiple environments, Pixi may prompt you to choose (or you can disambiguate with
-e). [3]
Sources: Tasks & default-environment + dependency environment precedence [1]; pixi run CLI --environment option [2]; multi-environment behavior and selection [3]; manifest environment/feature rules [4].
🌐 Web query:
setup-pixi GitHub action environments parameter
💡 Result:
In prefix-dev/setup-pixi, the environments input lets you choose which Pixi environments to install (and cache) during the action run.
- What to pass: one or more environment names (as defined in your
pixi.toml/pyproject.toml[environments]table). - Format: space-separated list (YAML multiline
>-is commonly used). - Default behavior: if you don’t set
environments, the action installs/caches thedefaultenvironment. Even if you later run another env withpixi run -e ..., caching/install done by the action is still fordefaultunless you specify otherwise. [1] [2]
Examples:
# Install one env (e.g., in a matrix job)
- uses: prefix-dev/setup-pixi@v0.9.3
with:
environments: ${{ matrix.environment }}[1]
# Install multiple envs in one job (space-separated)
- uses: prefix-dev/setup-pixi@v0.9.3
with:
environments: >-
py311
py312[1] [2]
Add -e dev to all pixi run commands, or configure setup-pixi to install the dev environment.
Tasks (format, lint, typecheck, test) are registered only in the dev feature via --feature dev in common.py. By default, pixi run <task> executes in the default environment, which does not include the dev feature and therefore cannot find these tasks. Additionally, setup-pixi without explicit environments: dev only installs the default environment, meaning the dev environment won't even be available.
Use pixi run -e dev <task> to explicitly target the environment, or specify environments: dev in each setup-pixi step.
♻️ Suggested pattern for each job step
- - name: Setup pixi
- uses: prefix-dev/setup-pixi@v0
+ - name: Setup pixi
+ uses: prefix-dev/setup-pixi@v0
+ with:
+ environments: dev- run: pixi run format --check
+ run: pixi run -e dev format --checkAlso applies to: 32, 44, 55
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@snakedeploy/templates/plugins/ci.yml.j2` at line 20, The CI step currently
runs "pixi run format --check" (and other pixi run invocations for lint,
typecheck, test) in the default environment which lacks the dev-feature tasks;
update each "pixi run ..." invocation to include the dev environment flag (e.g.
add "-e dev") or alternatively add "environments: dev" to the corresponding
"setup-pixi" steps so the dev environment is installed; search for the literal
"pixi run format --check" and other "pixi run" commands in the ci template
(ci.yml.j2) and make the change consistently across the steps referenced (lines
around the format/lint/typecheck/test steps).
There was a problem hiding this comment.
🧹 Nitpick comments (2)
snakedeploy/scaffold_plugins/common.py (2)
115-118: S607:pixiinvoked via partial executable path (consistent with existing calls).Ruff flags
pixiat line 116 as a partial executable path (S607). The same pattern exists in every othersp.run(["pixi", ...])call in this file. If S607 suppression is desired, a# noqa: S607comment on each call site (or a blanketper-file-ignoresrule) would silence it consistently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@snakedeploy/scaffold_plugins/common.py` around lines 115 - 118, Ruff flags the use of "pixi" as a partial executable path in the sp.run calls (see the sp.run([... "pixi", ...]) invocation that adds the "dev" feature/typecheck task), so either append a per-call suppression comment (# noqa: S607) to each sp.run(["pixi", ...]) invocation in this module or add a per-file/per-project ignore for S607 in the lint configuration; update the sp.run call(s) where "pixi" is used (and any other identical calls in the file) to include the "# noqa: S607" comment if you prefer in-place suppressions, or add S607 to per-file-ignores to silence the warning consistently across the file.
94-95: Note: pyrefly is currently under active development.pyrefly is currently under active development with known issues. Since this scaffolding bakes
pyreflyinto generated plugin projects for all users, it's worth considering whether that maturity level is acceptable for the downstream plugin authors, or whether pinning a minimum version / leaving a comment would help manage breakage as the tool evolves.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@snakedeploy/scaffold_plugins/common.py` around lines 94 - 95, The requirements list currently includes the dependency string "pyrefly" without any version pinning or explanatory note; update the scaffold in snakedeploy/scaffold_plugins/common.py to either pin a safe minimum/maximum version (e.g., "pyrefly>=X.Y.Z,<A.B.C") or add an inline comment next to the "pyrefly" entry warning that it is under active development and may break downstream plugins, so generated projects surface that risk to authors; modify the list where "pyrefly" appears and ensure the change is a simple string/comment adjustment so generated scaffolds include the pin or comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@snakedeploy/scaffold_plugins/common.py`:
- Around line 68-77: The code accesses pyproject["tool"] directly which raises
KeyError when the user's pyproject.toml has no [tool] table; before setting
pyproject["tool"]["coverage"] add a defensive guard such as calling
pyproject.setdefault("tool", {}) (and optionally ensure pyproject["tool"] is a
dict) so the subsequent assignment to pyproject["tool"]["coverage"] in the same
block succeeds without raising; locate the pyproject variable usage in this file
(the block that assigns pyproject["tool"]["coverage"]) and insert the setdefault
call immediately before that assignment.
---
Nitpick comments:
In `@snakedeploy/scaffold_plugins/common.py`:
- Around line 115-118: Ruff flags the use of "pixi" as a partial executable path
in the sp.run calls (see the sp.run([... "pixi", ...]) invocation that adds the
"dev" feature/typecheck task), so either append a per-call suppression comment
(# noqa: S607) to each sp.run(["pixi", ...]) invocation in this module or add a
per-file/per-project ignore for S607 in the lint configuration; update the
sp.run call(s) where "pixi" is used (and any other identical calls in the file)
to include the "# noqa: S607" comment if you prefer in-place suppressions, or
add S607 to per-file-ignores to silence the warning consistently across the
file.
- Around line 94-95: The requirements list currently includes the dependency
string "pyrefly" without any version pinning or explanatory note; update the
scaffold in snakedeploy/scaffold_plugins/common.py to either pin a safe
minimum/maximum version (e.g., "pyrefly>=X.Y.Z,<A.B.C") or add an inline comment
next to the "pyrefly" entry warning that it is under active development and may
break downstream plugins, so generated projects surface that risk to authors;
modify the list where "pyrefly" appears and ensure the change is a simple
string/comment adjustment so generated scaffolds include the pin or comment.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@snakedeploy/scaffold_plugins/common.py`:
- Around line 94-95: The scaffold currently includes "pyrefly" in the test/dev
dependencies list (the string "pyrefly"); update the scaffold to either pin
pyrefly to a known working version (e.g., "pyrefly>=X.Y.Z,<X+1.0.0") or remove
it and instead add a clear note in generated plugin docs/readme that pyrefly is
alpha and optional; modify the dependency list that contains "pytest-cov" and
"pyrefly" accordingly and ensure the generator code that writes README or
templates emits the alpha-stability warning so scaffolded plugins surface the
risk to users.
- Around line 115-118: Ruff S607 is a false positive for the sp.run([...
"pyrefly check"], ...) call; suppress it inline by adding a noqa for S607 to the
sp.run invocation (e.g., append a "# noqa: S607" comment to the sp.run line or
the statement) so the linter ignores this specific warning while keeping the
call unchanged.
- Around line 115-118: CI references a pixi task named "coverage-report" but
common.py doesn't register it, causing the workflow to fail; either remove the
coverage-report step from ci.yml.j2 or register the task in common.py. To fix,
add a pixi task registration in common.py (similar to existing sp.run([...
"pixi", "task", "add", ...]) calls) that defines "coverage-report" and runs the
pytest coverage reporting command (the same flags used in the existing "test"
task) so the CI step "pixi run coverage-report" can execute; alternatively
delete the "pixi run coverage-report" step from ci.yml.j2 if you prefer to keep
coverage only as part of the "test" task.
---
Duplicate comments:
In `@snakedeploy/scaffold_plugins/common.py`:
- Around line 68-77: The code directly indexes pyproject["tool"]["coverage"],
which will raise KeyError if the TOML had no [tool] table; before setting
coverage, ensure the nested dicts exist by calling pyproject.setdefault("tool",
{}) and then either use pyproject["tool"].setdefault("coverage", {}) or assign
into pyproject["tool"]["coverage"] after creating the "tool" dict; update the
block that configures coverage (the pyproject["tool"]["coverage"] assignment) to
use these setdefault guards so it never assumes "tool" exists.
| "pytest-cov", | ||
| "pyrefly", |
There was a problem hiding this comment.
pyrefly is currently in early alpha — confirm stability expectations for scaffolded plugins.
Pyrefly is "currently under active development with known issues", and the announcement explicitly describes it as an alpha version. Every plugin generated by this scaffolding will adopt pyrefly as its type checker. If pyrefly introduces breaking CLI changes or produces noisy false positives in early plugin projects, it could be disruptive. Consider whether pinning a minimum version or noting the alpha status in generated documentation is warranted.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@snakedeploy/scaffold_plugins/common.py` around lines 94 - 95, The scaffold
currently includes "pyrefly" in the test/dev dependencies list (the string
"pyrefly"); update the scaffold to either pin pyrefly to a known working version
(e.g., "pyrefly>=X.Y.Z,<X+1.0.0") or remove it and instead add a clear note in
generated plugin docs/readme that pyrefly is alpha and optional; modify the
dependency list that contains "pytest-cov" and "pyrefly" accordingly and ensure
the generator code that writes README or templates emits the alpha-stability
warning so scaffolded plugins surface the risk to users.
| sp.run( | ||
| ["pixi", "task", "add", "--feature", "dev", "typecheck", "pyrefly check"], | ||
| check=True, | ||
| ) |
There was a problem hiding this comment.
Ruff S607 on line 116 is a false positive — no action required.
"pyrefly check" is a string argument passed to pixi task add, not a subprocess executable path. The process actually launched by sp.run is pixi itself (consistent with every other task-add call in this file). If ruff is run in CI and this causes a lint failure, suppressing it inline is the right fix:
🔇 Suppress false positive if needed
- sp.run(
- ["pixi", "task", "add", "--feature", "dev", "typecheck", "pyrefly check"],
+ sp.run( # noqa: S607
+ ["pixi", "task", "add", "--feature", "dev", "typecheck", "pyrefly check"],
check=True,
)📝 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.
| sp.run( | |
| ["pixi", "task", "add", "--feature", "dev", "typecheck", "pyrefly check"], | |
| check=True, | |
| ) | |
| sp.run( # noqa: S607 | |
| ["pixi", "task", "add", "--feature", "dev", "typecheck", "pyrefly check"], | |
| check=True, | |
| ) |
🧰 Tools
🪛 Ruff (0.15.1)
[error] 116-116: Starting a process with a partial executable path
(S607)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@snakedeploy/scaffold_plugins/common.py` around lines 115 - 118, Ruff S607 is
a false positive for the sp.run([... "pyrefly check"], ...) call; suppress it
inline by adding a noqa for S607 to the sp.run invocation (e.g., append a "#
noqa: S607" comment to the sp.run line or the statement) so the linter ignores
this specific warning while keeping the call unchanged.
🧩 Analysis chain
🏁 Script executed:
# First, find and check ci.yml.j2 for coverage references
fd "ci.yml.j2" --exec grep -n "coverage" {}Repository: snakemake/snakedeploy
Length of output: 105
🏁 Script executed:
# Also search for any task-related references in common.py
fd "common.py" --path "*scaffold_plugins*" --exec wc -l {} \;Repository: snakemake/snakedeploy
Length of output: 280
🏁 Script executed:
# Get a sense of what tasks are registered in common.py
fd "common.py" --path "*scaffold_plugins*" --exec grep -n "pixi.*task.*add" {} \;Repository: snakemake/snakedeploy
Length of output: 280
🏁 Script executed:
# Find common.py in scaffold_plugins
find . -name "common.py" -path "*/scaffold_plugins/*"Repository: snakemake/snakedeploy
Length of output: 105
🏁 Script executed:
# Read common.py to see all pixi task add calls
cat -n snakedeploy/scaffold_plugins/common.py | grep -A 2 "pixi.*task.*add"Repository: snakemake/snakedeploy
Length of output: 626
🏁 Script executed:
# Get the full context around all pixi task add calls
rg "pixi.*task.*add" snakedeploy/scaffold_plugins/common.py -B 2 -A 2Repository: snakemake/snakedeploy
Length of output: 611
🏁 Script executed:
# Also check the full line count and see the complete file structure
wc -l snakedeploy/scaffold_plugins/common.pyRepository: snakemake/snakedeploy
Length of output: 107
🏁 Script executed:
# Search for all task names being added, to get a complete list
rg "task.*add.*--feature" snakedeploy/scaffold_plugins/common.py | sed 's/.*--feature[^"]*"\([^"]*\)".*/\1/'Repository: snakemake/snakedeploy
Length of output: 76
🏁 Script executed:
# Also check for test and check-build tasks
rg "(test|check-build|coverage)" snakedeploy/scaffold_plugins/common.pyRepository: snakemake/snakedeploy
Length of output: 574
🏁 Script executed:
# Read lines around task registrations to see the complete picture
sed -n '100,150p' snakedeploy/scaffold_plugins/common.pyRepository: snakemake/snakedeploy
Length of output: 1596
🏁 Script executed:
# Check if coverage-report task is added anywhere in the file
grep -n "coverage-report" snakedeploy/scaffold_plugins/common.pyRepository: snakemake/snakedeploy
Length of output: 133
🏁 Script executed:
# Let's check what ci.yml.j2 expects and the full context around coverage-report reference
sed -n '50,70p' snakedeploy/scaffold_plugins/templates/ci.yml.j2Repository: snakemake/snakedeploy
Length of output: 156
🏁 Script executed:
# Find ci.yml.j2 file
find . -name "ci.yml.j2"Repository: snakemake/snakedeploy
Length of output: 106
🏁 Script executed:
# Check the context around line 58 where coverage-report is referenced
sed -n '50,70p' ./snakedeploy/templates/plugins/ci.yml.j2Repository: snakemake/snakedeploy
Length of output: 251
🏁 Script executed:
# Also check if there are any other coverage-report references
grep -n "coverage-report" ./snakedeploy/templates/plugins/ci.yml.j2Repository: snakemake/snakedeploy
Length of output: 105
Remove the coverage-report task from ci.yml.j2 or register it in common.py.
The CI template (ci.yml.j2 line 58) references pixi run coverage-report, but common.py never registers this task via pixi task add. Coverage reporting is currently built into the test task (via --cov-report flags), but not exposed as a separate pixi task. This will cause the CI workflow to fail when attempting to run the non-existent coverage-report task.
Either add pixi task add coverage-report in common.py to expose the coverage output as a standalone task, or remove the coverage-report step from ci.yml.j2 if it's redundant with the test task.
🧰 Tools
🪛 Ruff (0.15.1)
[error] 116-116: Starting a process with a partial executable path
(S607)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@snakedeploy/scaffold_plugins/common.py` around lines 115 - 118, CI references
a pixi task named "coverage-report" but common.py doesn't register it, causing
the workflow to fail; either remove the coverage-report step from ci.yml.j2 or
register the task in common.py. To fix, add a pixi task registration in
common.py (similar to existing sp.run([... "pixi", "task", "add", ...]) calls)
that defines "coverage-report" and runs the pytest coverage reporting command
(the same flags used in the existing "test" task) so the CI step "pixi run
coverage-report" can execute; alternatively delete the "pixi run
coverage-report" step from ci.yml.j2 if you prefer to keep coverage only as part
of the "test" task.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@snakedeploy/scaffold_plugins/common.py`:
- Around line 119-122: The sp.run call creating the pixi task is too long for
ruff and triggers an S607 false positive on the "coverage report -m" token;
refactor the call to a multi-line list form so each item fits under the
line-length limit and append a local noqa suppression for S607 to the
problematic element (e.g., add "# noqa: S607" on the list item containing
"coverage report -m") so ruff passes and the false positive is silenced; locate
the sp.run invocation in scaffold_plugins/common.py and update the argument list
to span multiple lines and add the noqa on the offending string.
---
Duplicate comments:
In `@snakedeploy/scaffold_plugins/common.py`:
- Around line 68-77: The code assumes pyproject["tool"] exists and will KeyError
if missing; before assigning pyproject["tool"]["coverage"] ensure the "tool" key
exists and is a dict (e.g., use pyproject.setdefault("tool", {}) or check
isinstance and replace non-dict), then assign the "coverage" entry (refer to the
pyproject variable and the coverage assignment block in common.py).
- Around line 115-118: The Ruff S607 false positive on the sp.run call for the
pixi task is still unaddressed; update the sp.run invocation (the call to sp.run
that runs ["pixi", "task", "add", "--feature", "dev", "typecheck", "pyrefly
check"]) to silence the linter by either splitting the "pyrefly check" entry
into separate arguments ["pyrefly", "check"] or append an inline noqa for Ruff
(e.g., add "# noqa: S607" on the sp.run line) so the warning is explicitly
suppressed while preserving the existing behavior.
- Around line 94-95: The scaffold currently includes the dependency string
"pyrefly" in the default plugin dependency list; because pyrefly is early alpha
you should either remove it from the default list or make it opt-in: update
scaffold_plugins/common.py to stop adding "pyrefly" unconditionally (or move it
to an optional/dev dependency group), add a clear comment/docstring next to the
"pyrefly" entry indicating its alpha status and expected stability, and if you
keep it as an option provide a flag or config in the scaffolding logic to opt
into adding "pyrefly" rather than bundling it by default.
| sp.run( | ||
| ["pixi", "task", "add", "--feature", "dev", "coverage-report", "coverage report -m"], | ||
| check=True, | ||
| ) |
There was a problem hiding this comment.
Pipeline failure: line 120 exceeds ruff's line-length limit and needs reformatting; also carries an S607 false positive.
Line 120 is ~98 characters, which exceeds ruff's default 88-character limit — this is the root cause of the ruff format --check CI failure. Additionally, Ruff flags "coverage report -m" as S607 (partial executable path), the same false-positive pattern already present at line 116.
🐛 Proposed fix: reformat to multi-line style (resolves CI failure) and suppress S607 false positive
- sp.run(
- ["pixi", "task", "add", "--feature", "dev", "coverage-report", "coverage report -m"],
- check=True,
- )
+ sp.run( # noqa: S607
+ [
+ "pixi",
+ "task",
+ "add",
+ "--feature",
+ "dev",
+ "coverage-report",
+ "coverage report -m",
+ ],
+ check=True,
+ )🧰 Tools
🪛 Ruff (0.15.1)
[error] 120-120: Starting a process with a partial executable path
(S607)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@snakedeploy/scaffold_plugins/common.py` around lines 119 - 122, The sp.run
call creating the pixi task is too long for ruff and triggers an S607 false
positive on the "coverage report -m" token; refactor the call to a multi-line
list form so each item fits under the line-length limit and append a local noqa
suppression for S607 to the problematic element (e.g., add "# noqa: S607" on the
list item containing "coverage report -m") so ruff passes and the false positive
is silenced; locate the sp.run invocation in scaffold_plugins/common.py and
update the argument list to span multiple lines and add the noqa on the
offending string.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@snakedeploy/scaffold_plugins/common.py`:
- Around line 107-122: Break the long sp.run(...) calls so they don't exceed
ruff's line-length: either run ruff format (or `pixi run format`) to auto-fix,
or manually rewrite each sp.run invocation in common.py (the calls to sp.run
that add pixi tasks for "lint", "format", "typecheck", and "coverage-report") to
use a multi-line list for the arguments (e.g., put ["pixi", "task", "add",
"--feature", "dev", "lint", "ruff check src tests"] across several lines) so no
line exceeds 88 chars; ensure you preserve the exact argument order and quoting
so the commands remain identical.
---
Duplicate comments:
In `@snakedeploy/scaffold_plugins/common.py`:
- Around line 94-95: The scaffold list that includes the package name "pyrefly"
(adjacent to "pytest-cov") uses an unpinned, early-alpha dependency; change the
entry in the dependencies list in snakedeploy.scaffold_plugins.common to either
pin "pyrefly" to a known-good version (e.g., "pyrefly==X.Y.Z") or replace it
with a safer alternative, and also add a clear inline comment or documentation
note in the same module indicating that "pyrefly" is alpha and should be
reviewed by scaffolded-plugin authors.
- Around line 68-77: The code assumes pyproject["tool"] exists and will KeyError
for pyproject.toml files without a [tool] table; update the logic around the
pyproject dictionary (the pyproject variable in scaffold_plugins/common.py where
pyproject["tool"]["coverage"] is set) to ensure a "tool" mapping exists before
assigning "coverage" (e.g., use setdefault or conditional creation of
pyproject["tool"] as an empty dict), then assign the "coverage" dict to
pyproject["tool"]["coverage"] so the operation is safe for pyproject files that
omit [tool].
| sp.run( | ||
| ["pixi", "task", "add", "--feature", "dev", "lint", "ruff check"], | ||
| ["pixi", "task", "add", "--feature", "dev", "lint", "ruff check src tests"], | ||
| check=True, | ||
| ) | ||
| sp.run( | ||
| ["pixi", "task", "add", "--feature", "dev", "format", "ruff format src tests"], | ||
| check=True, | ||
| ) | ||
| sp.run( | ||
| ["pixi", "task", "add", "--feature", "dev", "typecheck", "pyrefly check src tests"], | ||
| check=True, | ||
| ) | ||
| sp.run( | ||
| ["pixi", "task", "add", "--feature", "dev", "format", "ruff format"], | ||
| ["pixi", "task", "add", "--feature", "dev", "coverage-report", "coverage report -m"], | ||
| check=True, | ||
| ) |
There was a problem hiding this comment.
CI failure: multiple lines exceed ruff's line-length limit.
The pipeline is failing because ruff format --check finds lines that need reformatting. Lines 108, 112, 116, and 120 all appear to exceed the 88-character default. The simplest fix is to run ruff format (or pixi run format) as the CI error message suggests, or break these into multi-line list form:
♻️ Proposed fix (multi-line style for all offending calls)
sp.run(
- ["pixi", "task", "add", "--feature", "dev", "lint", "ruff check src tests"],
+ [
+ "pixi",
+ "task",
+ "add",
+ "--feature",
+ "dev",
+ "lint",
+ "ruff check src tests",
+ ],
check=True,
)
sp.run(
- ["pixi", "task", "add", "--feature", "dev", "format", "ruff format src tests"],
+ [
+ "pixi",
+ "task",
+ "add",
+ "--feature",
+ "dev",
+ "format",
+ "ruff format src tests",
+ ],
check=True,
)
sp.run(
- ["pixi", "task", "add", "--feature", "dev", "typecheck", "pyrefly check src tests"],
+ [
+ "pixi",
+ "task",
+ "add",
+ "--feature",
+ "dev",
+ "typecheck",
+ "pyrefly check src tests",
+ ],
check=True,
)
sp.run(
- ["pixi", "task", "add", "--feature", "dev", "coverage-report", "coverage report -m"],
+ [
+ "pixi",
+ "task",
+ "add",
+ "--feature",
+ "dev",
+ "coverage-report",
+ "coverage report -m",
+ ],
check=True,
)Note: The Ruff S607 warnings on these lines are false positives — the strings like "ruff check src tests" are arguments to pixi task add, not subprocess executable paths.
🧰 Tools
🪛 Ruff (0.15.1)
[error] 108-108: Starting a process with a partial executable path
(S607)
[error] 112-112: Starting a process with a partial executable path
(S607)
[error] 116-116: Starting a process with a partial executable path
(S607)
[error] 120-120: Starting a process with a partial executable path
(S607)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@snakedeploy/scaffold_plugins/common.py` around lines 107 - 122, Break the
long sp.run(...) calls so they don't exceed ruff's line-length: either run ruff
format (or `pixi run format`) to auto-fix, or manually rewrite each sp.run
invocation in common.py (the calls to sp.run that add pixi tasks for "lint",
"format", "typecheck", and "coverage-report") to use a multi-line list for the
arguments (e.g., put ["pixi", "task", "add", "--feature", "dev", "lint", "ruff
check src tests"] across several lines) so no line exceeds 88 chars; ensure you
preserve the exact argument order and quoting so the commands remain identical.
1 participant
Summary by CodeRabbit
New Features
Improvements
Chores