ci: align shared security and release tooling#121
Conversation
- Run CI through pinned uv and cached Cargo tooling across all supported platforms. - Add repository Semgrep rules, action allowlist coverage, CodeQL, zizmor, and SARIF workflows. - Raise the Rust and Python tooling floors to Rust 1.96 and Python 3.12. - Add archive-aware changelog generation, post-processing, and tag-release tooling. - Preserve exact arithmetic overflow reporting without non-finite sentinel defaults. Closes #117
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
📝 WalkthroughWalkthroughModernizes CI/security workflows and tooling, pins and caches cargo-installed tools, adds Semgrep/CodeQL/zizmor runs and SECURITY.md, implements changelog postprocessing + archival CLI with tests, tightens exact→f64 overflow handling, and updates docs/metadata and benchmarks. ChangesCI/Tooling Modernization & Changelog Automation
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
|
@coderabbitai full review |
✅ Action performedFull review finished. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #121 +/- ##
==========================================
- Coverage 99.81% 99.62% -0.19%
==========================================
Files 5 5
Lines 2146 2150 +4
==========================================
Hits 2142 2142
- Misses 4 8 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
scripts/tag_release.py (1)
1-1:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSwitch
scripts/tag_release.pyentrypoint frompython3touv run
File: scripts/tag_release.pystill uses#!/usr/bin/env python3, which violates the**/*.pyguideline to use#!/usr/bin/env -S uv run(to avoid drifting from the pinned CI/type-check environment).scripts/subprocess_utils.pyhas the same shebang issue.Suggested fix
-#!/usr/bin/env python3 +#!/usr/bin/env -S uv run🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/tag_release.py` at line 1, Update the shebang lines in the scripts to use the pinned CI/type-check runtime: replace the current "#!/usr/bin/env python3" in scripts/tag_release.py (and the same occurrence in scripts/subprocess_utils.py) with "#!/usr/bin/env -S uv run" so the scripts execute under the pinned uv environment instead of the system python3; ensure only the shebang line is changed and nothing else in the files (no function or behavior modifications).scripts/subprocess_utils.py (1)
1-20:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSwitch
scripts/subprocess_utils.pyshebang touv runto avoid Python 3.12 syntax failures
scripts/subprocess_utils.pyuses Python 3.12-onlytypesyntax (type RunKwargs = ...) and the repo declaresrequires-python = ">=3.12", but its shebang is#!/usr/bin/env python3, which can still resolve to an older 3.x on PATH—leading to import/runtime failure. Useuv runso the correct interpreter/environment is selected.Suggested fix
-#!/usr/bin/env python3 +#!/usr/bin/env -S uv run🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/subprocess_utils.py` around lines 1 - 20, The shebang at the top of scripts/subprocess_utils.py can invoke an older python3 on PATH causing the newer 3.12-only syntax (the type alias RunKwargs) to fail; update the first line to use the environment launcher "uv run" (e.g., replace "#!/usr/bin/env python3" with a shebang that invokes "uv run" so the repo's declared Python >=3.12 is used), ensure the file remains executable, and verify the module imports/type alias RunKwargs load correctly under the new interpreter.
🧹 Nitpick comments (2)
justfile (1)
750-755: 💤 Low valueClarify the
yaml-lintalias purpose.The
yaml-lintrecipe now aliasesyaml-check(dprint-based YAML formatting check), whilezizmoris a separate recipe for GitHub Actions security analysis. This is semantically correct but may surprise users expectingyaml-lintto perform structural validation rather than formatting checks.Consider adding a brief comment above
yaml-lintnoting it's now an alias for format checking.📝 Suggested clarification
+# yaml-lint now delegates to yaml-check (dprint format verification). +# For GitHub Actions security scanning, use: just zizmor yaml-lint: yaml-check🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@justfile` around lines 750 - 755, Add a short comment above the yaml-lint alias clarifying it is an alias for the formatting/format-checking recipe yaml-check (dprint-based), e.g., "yaml-lint is a formatting check alias for yaml-check (dprint) — not a structural/validation linter"; leave the alias definition (yaml-lint: yaml-check) unchanged and ensure the zendizmor/zizmor recipe reference remains separate.scripts/tests/test_tag_release.py (1)
119-152: ⚡ Quick winAdd one archive-fallback test for
extract_changelog_section().The changed behavior here is archive-aware extraction, but these cases still only cover sections found in the root
CHANGELOG.md. Please add a tmp-path case that writesdocs/archive/changelog/0.1.md, extracts an archived version, and asserts the returnedsourcepoints at the archive file.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/tests/test_tag_release.py` around lines 119 - 152, Add a new test method in TestExtractChangelogSection that creates an archived changelog file under tmp_path / "docs" / "archive" / "changelog" / "0.1.md", writes a changelog entry for the target version (e.g., header and some unique text), calls extract_changelog_section(...) with that version string, and asserts the returned section contains the unique text and that the returned source equals the archive file path; reference extract_changelog_section and the TestExtractChangelogSection class when adding the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cliff.toml`:
- Around line 95-98: The current generic message regex "(?i)^bump " in the TOML
entry will misclassify human-authored "Bump ..." commits; update that message
pattern to be more specific so only dependency/version bump commits match (for
example require "bump" to be followed by explicit dependency indicators such as
"deps", "dependencies", "packages", "versions", or a dependency
identifier/version token like an @, /, or a semantic version number), replacing
the existing "(?i)^bump " pattern in the message field so ordinary prose "Bump
MSRV..." subjects no longer land in the Dependencies group.
In `@scripts/tests/test_subprocess_utils.py`:
- Around line 134-141: The test test_git_convenience_helpers is brittle because
it calls real git and can fail in shallow/stripped checkouts; update the test to
mock the underlying git interactions instead of calling the real repo: patch the
helper used by subprocess_utils (e.g., monkeypatch subprocess_utils.git or
subprocess.run used by get_git_commit_hash, get_git_remote_url, check_git_repo,
check_git_history) to return deterministic stdout/returncodes that simulate a
normal repo, then assert the helpers behave as expected; alternatively move
these assertions into an integration test; ensure you reference the functions
get_git_commit_hash, get_git_remote_url, check_git_repo, and check_git_history
when locating where to apply the mocks.
In `@semgrep.yaml`:
- Around line 104-117: The Semgrep rule
la-stack.github-actions.external-action-version-comment currently only matches
when the SHA is the last token on the line (pattern-regex) and therefore allows
lines like "uses: owner/action@<sha> # pinned" to bypass the check; tighten the
rule by changing the pattern-regex (or replacing it with a structured Semgrep
pattern) so it requires a trailing comment that looks like a version (e.g.,
match "#\s*v?\d+(\.\d+)*" or similar after the 40‑char SHA) and treat lines that
lack such a version-shaped comment as violations; update the rule's
pattern-regex reference to include the SHA followed by optional whitespace and
then a version-style comment and ensure
la-stack.github-actions.external-action-version-comment flags missing or
non-version comments.
---
Outside diff comments:
In `@scripts/subprocess_utils.py`:
- Around line 1-20: The shebang at the top of scripts/subprocess_utils.py can
invoke an older python3 on PATH causing the newer 3.12-only syntax (the type
alias RunKwargs) to fail; update the first line to use the environment launcher
"uv run" (e.g., replace "#!/usr/bin/env python3" with a shebang that invokes "uv
run" so the repo's declared Python >=3.12 is used), ensure the file remains
executable, and verify the module imports/type alias RunKwargs load correctly
under the new interpreter.
In `@scripts/tag_release.py`:
- Line 1: Update the shebang lines in the scripts to use the pinned
CI/type-check runtime: replace the current "#!/usr/bin/env python3" in
scripts/tag_release.py (and the same occurrence in scripts/subprocess_utils.py)
with "#!/usr/bin/env -S uv run" so the scripts execute under the pinned uv
environment instead of the system python3; ensure only the shebang line is
changed and nothing else in the files (no function or behavior modifications).
---
Nitpick comments:
In `@justfile`:
- Around line 750-755: Add a short comment above the yaml-lint alias clarifying
it is an alias for the formatting/format-checking recipe yaml-check
(dprint-based), e.g., "yaml-lint is a formatting check alias for yaml-check
(dprint) — not a structural/validation linter"; leave the alias definition
(yaml-lint: yaml-check) unchanged and ensure the zendizmor/zizmor recipe
reference remains separate.
In `@scripts/tests/test_tag_release.py`:
- Around line 119-152: Add a new test method in TestExtractChangelogSection that
creates an archived changelog file under tmp_path / "docs" / "archive" /
"changelog" / "0.1.md", writes a changelog entry for the target version (e.g.,
header and some unique text), calls extract_changelog_section(...) with that
version string, and asserts the returned section contains the unique text and
that the returned source equals the archive file path; reference
extract_changelog_section and the TestExtractChangelogSection class when adding
the test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 00997127-53f5-43c0-aaed-e8a373693a7c
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (42)
.github/dependabot.yml.github/workflows/audit.yml.github/workflows/benchmarks.yml.github/workflows/ci.yml.github/workflows/codacy.yml.github/workflows/codecov.yml.github/workflows/codeql.yml.github/workflows/rust-clippy.yml.github/workflows/semgrep-sarif.yml.github/workflows/zizmor.yml.gitignore.python-versionAGENTS.mdCHANGELOG.mdCargo.tomlREADME.mdREFERENCES.mdSECURITY.mdbenches/vs_linalg.rscliff.tomldocs/archive/changelog/0.1.mddocs/archive/changelog/0.2.mddocs/archive/changelog/0.3.mddprint.jsonjustfilepyproject.tomlrust-toolchain.tomlscripts/archive_changelog.pyscripts/bench_compare.pyscripts/criterion_dim_plot.pyscripts/postprocess_changelog.pyscripts/subprocess_utils.pyscripts/tag_release.pyscripts/tests/test_archive_changelog.pyscripts/tests/test_criterion_dim_plot.pyscripts/tests/test_postprocess_changelog.pyscripts/tests/test_subprocess_utils.pyscripts/tests/test_tag_release.pysemgrep.yamlsrc/exact.rstests/proptest_exact.rsty.toml
💤 Files with no reviewable changes (2)
- .gitignore
- scripts/tests/test_criterion_dim_plot.py
| # Dependency bumps: keep all bump commits so archived release history | ||
| # preserves CI/action/tooling dependency updates as well as Rust crates. | ||
| { message = "^chore\\(deps\\): bump ", group = "Dependencies" }, | ||
| { message = "(?i)^bump ", group = "Dependencies" }, |
There was a problem hiding this comment.
Narrow the generic bump parser before it swallows non-dependency commits.
(?i)^bump will also catch human-authored subjects like Bump MSRV to Rust 1.95..., so those entries will start landing under Dependencies instead of Maintenance or Changed.
Suggested adjustment
- { message = "(?i)^bump ", group = "Dependencies" },
+ { message = '(?i)^bump (?:the dependencies group\b|[A-Za-z0-9_.-]+(?:/[A-Za-z0-9_.-]+)? from )', group = "Dependencies" },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cliff.toml` around lines 95 - 98, The current generic message regex
"(?i)^bump " in the TOML entry will misclassify human-authored "Bump ..."
commits; update that message pattern to be more specific so only
dependency/version bump commits match (for example require "bump" to be followed
by explicit dependency indicators such as "deps", "dependencies", "packages",
"versions", or a dependency identifier/version token like an @, /, or a semantic
version number), replacing the existing "(?i)^bump " pattern in the message
field so ordinary prose "Bump MSRV..." subjects no longer land in the
Dependencies group.
| def test_git_convenience_helpers(self) -> None: | ||
| assert get_git_commit_hash() | ||
| assert get_git_remote_url() | ||
| assert check_git_repo() is True | ||
| assert check_git_history() is True | ||
|
|
||
| def test_find_project_root(self) -> None: | ||
| assert (find_project_root() / "Cargo.toml").is_file() |
There was a problem hiding this comment.
Make the git-helper test independent of the checkout state.
This test currently depends on the sandbox having a configured remote and enough git history. get_git_remote_url() and check_git_history() will fail in shallow clones, source archives, or stripped CI checkouts even if subprocess_utils is correct. Please mock the underlying git calls here the same way Lines 118-132 already do, or move this to an explicit integration test.
Suggested direction
- def test_git_convenience_helpers(self) -> None:
- assert get_git_commit_hash()
- assert get_git_remote_url()
- assert check_git_repo() is True
- assert check_git_history() is True
+ `@patch`("subprocess_utils.run_git_command")
+ def test_git_convenience_helpers(self, mock_run: MagicMock) -> None:
+ mock_run.side_effect = [
+ MagicMock(stdout="abc1234\n"),
+ MagicMock(stdout="https://example.invalid/acgetchell/la-stack.git\n"),
+ MagicMock(returncode=0),
+ MagicMock(returncode=0),
+ ]
+
+ assert get_git_commit_hash() == "abc1234"
+ assert get_git_remote_url() == "https://example.invalid/acgetchell/la-stack.git"
+ assert check_git_repo() is True
+ assert check_git_history() is True🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/tests/test_subprocess_utils.py` around lines 134 - 141, The test
test_git_convenience_helpers is brittle because it calls real git and can fail
in shallow/stripped checkouts; update the test to mock the underlying git
interactions instead of calling the real repo: patch the helper used by
subprocess_utils (e.g., monkeypatch subprocess_utils.git or subprocess.run used
by get_git_commit_hash, get_git_remote_url, check_git_repo, check_git_history)
to return deterministic stdout/returncodes that simulate a normal repo, then
assert the helpers behave as expected; alternatively move these assertions into
an integration test; ensure you reference the functions get_git_commit_hash,
get_git_remote_url, check_git_repo, and check_git_history when locating where to
apply the mocks.
| - id: la-stack.github-actions.external-action-version-comment | ||
| languages: | ||
| - regex | ||
| severity: WARNING | ||
| message: "Keep a readable version comment next to external GitHub Action SHA pins." | ||
| metadata: | ||
| category: maintainability | ||
| rationale: "Version comments make Dependabot updates and human review manageable." | ||
| paths: | ||
| include: | ||
| - "/.github/workflows/**/*.yml" | ||
| - "/.github/workflows/**/*.yaml" | ||
| patterns: | ||
| - pattern-regex: '(?m)^\s*uses:\s*(?!\./)(?!docker://)[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+(?:/[A-Za-z0-9_.-]+)?@[a-fA-F0-9]{40}\s*$' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rule='^\s*uses:\s*(?!\./)(?!docker://)[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+(?:/[A-Za-z0-9_.-]+)?@[a-fA-F0-9]{40}\s*$'
bad='uses: codacy/codacy-analysis-cli-action@562ee3e92b8e92df8b67e0a5ff8aa8e261919c08 # pinned'
good='uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2'
printf '%s\n' "$bad" | grep -P "$rule" >/dev/null && echo "bad matched" || echo "bad not matched"
printf '%s\n' "$good" | grep -P "$rule" >/dev/null && echo "good matched" || echo "good not matched"Repository: acgetchell/la-stack
Length of output: 96
🏁 Script executed:
#!/bin/bash
set -euo pipefail
nl -ba semgrep.yaml | sed -n '80,150p'Repository: acgetchell/la-stack
Length of output: 106
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the referenced rule area with an available line-numbering tool
cat -n semgrep.yaml | sed -n '90,140p'Repository: acgetchell/la-stack
Length of output: 1726
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rule='^\s*uses:\s*(?!\./)(?!docker://)[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+(?:/[A-Za-z0-9_.-]+)?@[a-fA-F0-9]{40}\s*$'
no_comment='uses: codacy/codacy-analysis-cli-action@562ee3e92b8e92df8b67e0a5ff8aa8e261919c08'
printf '%s\n' "$no_comment" | grep -P "$rule" >/dev/null && echo "no_comment matched" || echo "no_comment not matched"Repository: acgetchell/la-stack
Length of output: 82
Tighten Semgrep rule to require a version-shaped comment (not just “no trailing text”).
la-stack.github-actions.external-action-version-comment only matches when the SHA is the last token on the line (...@[a-fA-F0-9]{40}\s*$). So uses: owner/action@<sha> # pinned (or # v6.0.2) avoids the finding entirely—meaning the rule enforces “missing trailing comment” rather than “readable version comment exists,” weakening the intended supply-chain policy.
Update the regex (or use a structured Semgrep pattern) to require a # ... version suffix after the SHA (e.g., # v1.2.3 / # 1.2.3) and flag when it’s missing or doesn’t look like a version.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@semgrep.yaml` around lines 104 - 117, The Semgrep rule
la-stack.github-actions.external-action-version-comment currently only matches
when the SHA is the last token on the line (pattern-regex) and therefore allows
lines like "uses: owner/action@<sha> # pinned" to bypass the check; tighten the
rule by changing the pattern-regex (or replacing it with a structured Semgrep
pattern) so it requires a trailing comment that looks like a version (e.g.,
match "#\s*v?\d+(\.\d+)*" or similar after the 40‑char SHA) and treat lines that
lack such a version-shaped comment as violations; update the rule's
pattern-regex reference to include the SHA followed by optional whitespace and
then a version-style comment and ensure
la-stack.github-actions.external-action-version-comment flags missing or
non-version comments.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
scripts/tag_release.py (1)
1-1:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate shebang to use
uv runper repository guidelines.As per coding guidelines, all Python scripts under
scripts/**/*.pyshould useuv run; never invokepython3orpythondirectly. The other scripts in this PR (postprocess_changelog.py,archive_changelog.py) use the correct shebang.Proposed fix
-#!/usr/bin/env python3 +#!/usr/bin/env -S uv runBased on learnings: Repository Python scripts should use
uv runfor execution. As per coding guidelines: Useuv runfor all Python scripts; never invokepython3orpythondirectly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/tag_release.py` at line 1, Replace the shebang in scripts/tag_release.py that currently invokes python3 with the repository-standard uv run shebang used by the other scripts (i.e., change the "#!/usr/bin/env python3" line to the equivalent that runs via uv run), and ensure the file remains executable; locate the shebang at the top of the file to make this one-line change so the script uses uv run instead of calling python directly.justfile (1)
539-554:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
setup-toolsverifiesjqbut never installs or guides it.The required-commands loop includes
jq, yet (unlike the cargo/uv tools above) nothing insetup-toolsinstalls or points to ajqinstall. On a host withoutjq, this fails with✗ jqand the generic "Fix the installs above" message, even though there is nojqstep above to fix. Consider adding a guidance line (or install path) forjq, consistent with_ensure-jq's hint.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@justfile` around lines 539 - 554, The required-commands check in setup-tools currently lists "jq" but setup-tools never installs or guides how to install it; update setup-tools to either invoke the existing _ensure-jq helper or add a clear guidance/install step for jq so the "✗ jq" case can be resolved. Specifically, modify the setup-tools recipe to call _ensure-jq (or echo the same installation hint used by _ensure-jq) before the verification loop so jq is installed or a direct instruction is printed when missing, ensuring the required-commands loop's jq entry is actionable.
♻️ Duplicate comments (2)
cliff.toml (1)
98-98:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNarrow the generic
bumpparser before it swallows non-dependency commits.The past review comment on lines 95-98 flagged this same issue:
(?i)^bumpwill also catch human-authored subjects likeBump MSRV to Rust 1.95..., causing those entries to land under Dependencies instead of Maintenance or Changed.Suggested adjustment
- { message = "(?i)^bump ", group = "Dependencies" }, + { message = '(?i)^bump (?:the dependencies group\b|[A-Za-z0-9_.-]+(?:/[A-Za-z0-9_.-]+)? from )', group = "Dependencies" },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cliff.toml` at line 98, The current generic parser message pattern message = "(?i)^bump " catches human-authored subjects; tighten it by replacing that pattern with a regex that requires either an explicit "deps"/"dependencies" keyword or a package identifier after "bump" (for example using a word boundary and requiring bump\s+(deps?|dependencies|[A-Za-z0-9_\-\/]+)), so update the message value used for grouping "Dependencies" accordingly to avoid matching generic "Bump MSRV..." subjects.scripts/tests/test_subprocess_utils.py (1)
134-138:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMock the git helpers to avoid CI brittleness.
This test still calls real git commands (
get_git_commit_hash(),get_git_remote_url(),check_git_repo(),check_git_history()) and will fail in shallow clones, source archives, or stripped CI checkouts. Mock the underlyingrun_git_commandcalls to return deterministic results, as already done in lines 118-132.🔒 Mock pattern for git helpers
+ `@patch`("subprocess_utils.run_git_command") - def test_git_convenience_helpers(self) -> None: + def test_git_convenience_helpers(self, mock_run: MagicMock) -> None: + mock_run.side_effect = [ + MagicMock(stdout="abc1234\n"), + MagicMock(stdout="https://example.invalid/acgetchell/la-stack.git\n"), + MagicMock(returncode=0), + MagicMock(returncode=0), + ] + - assert get_git_commit_hash() - assert get_git_remote_url() + assert get_git_commit_hash() == "abc1234" + assert get_git_remote_url() == "https://example.invalid/acgetchell/la-stack.git" assert check_git_repo() is True assert check_git_history() is True🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/tests/test_subprocess_utils.py` around lines 134 - 138, The test test_git_convenience_helpers currently invokes real git and is brittle; change it to mock the underlying run_git_command calls used by get_git_commit_hash, get_git_remote_url, check_git_repo, and check_git_history so they return deterministic values (e.g. a fake commit hash, a fake remote URL, and success outputs) instead of running git. Use the same mocking pattern used earlier in the file (patch/monkeypatch run_git_command) and ensure each helper is asserted against the mocked return values rather than relying on the real environment.
🧹 Nitpick comments (3)
.github/workflows/ci.yml (1)
73-76: 💤 Low valueConsider sourcing the Python version from
.python-version.
.python-versionalready pins3.12; hardcoding"3.12"here creates a second source of truth that can silently drift.actions/setup-pythonsupportspython-version-file.♻️ Single source of truth
- name: Set up Python uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 with: - python-version: "3.12" + python-version-file: ".python-version"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 73 - 76, Update the "Set up Python" GH Actions step that uses actions/setup-python (the step with name "Set up Python" and input key python-version) to read the interpreter version from the repository .python-version file instead of hardcoding "3.12": replace the python-version input with python-version-file and point it to ".python-version" so the workflow uses that single source of truth; keep the same action/commit reference (actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405)..github/workflows/codacy.yml (1)
86-86: 💤 Low valueInclude the resolved version in the pin comment.
Every other action in these workflows uses a readable
# vX.Y.Zannotation alongside the SHA. The bare# pinnedcomment here gives no hint of whichcodacy-analysis-cli-actionrelease562ee3ecorresponds to, which hurts auditability and Dependabot review.♻️ Suggested annotation
- uses: codacy/codacy-analysis-cli-action@562ee3e92b8e92df8b67e0a5ff8aa8e261919c08 # pinned + uses: codacy/codacy-analysis-cli-action@562ee3e92b8e92df8b67e0a5ff8aa8e261919c08 # v4.4.5(replace with the actual release that SHA maps to)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/codacy.yml at line 86, The pinned GitHub Action reference uses a SHA-only comment on the uses line ("codacy/codacy-analysis-cli-action@562ee3e92b8e92df8b67e0a5ff8aa8e261919c08") which lacks a readable release tag; update the trailing comment on that uses entry to include the resolved release (e.g. "# vX.Y.Z") alongside or replacing "# pinned" so the line reads like "uses: codacy/codacy-analysis-cli-action@562ee3e... # vX.Y.Z" — look for the exact uses entry for codacy/codacy-analysis-cli-action and add the human-readable version string that the SHA corresponds to.justfile (1)
28-36: ⚡ Quick win
grep … | head -1in a bare assignment can abort the recipe underset -e/pipefail.These
_ensure-*helpers runinstalled_version="$(… | grep -oE … | head -1)"as a standalone assignment withset -euo pipefail. If--versionoutput contains no semver (grepexits 1) orgrepis killed bySIGPIPEwhenheadcloses early, the pipeline returns non-zero andset -eaborts the recipe immediately — so the intended❌ … not found. Install with: …message never prints. This pattern repeats across all_ensure-*helpers and the inline checks insetup-tools.♻️ Make the version capture non-fatal (apply to each occurrence)
- installed_version="$(cargo llvm-cov --version 2>/dev/null | grep -oE '[0-9]+\.[0-9]+\.[0-9]+' | head -1)" + installed_version="$(cargo llvm-cov --version 2>/dev/null | grep -oE '[0-9]+\.[0-9]+\.[0-9]+' | head -1 || true)"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@justfile` around lines 28 - 36, The pipeline used to extract versions (e.g., installed_version="$(cargo llvm-cov --version 2>/dev/null | grep -oE '[0-9]+\.[0-9]+\.[0-9]+' | head -1)") can fail under set -euo pipefail and abort the recipe; make the capture non-fatal by allowing the pipeline to return zero even if grep/head fail — for example, append "|| true" to the command substitution or wrap only the pipeline in a subshell with "|| true" so installed_version becomes empty instead of causing exit; apply this change to every occurrence in the _ensure-* helpers and the inline checks in setup-tools (references: installed_version assignment lines, _ensure-* helper functions, and setup-tools check blocks).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pyproject.toml`:
- Around line 140-148: The dev dependencies list currently uses "pytest>=9.0.3"
which should be tightened to the released PyPI version to avoid uv sync
failures; update the dependency in the [dependency-groups] dev array from
"pytest>=9.0.3" to "pytest==9.0.3" and then sanity-check that the other pinned
dev tools ("ruff>=0.15.14", "semgrep==1.164.0", "actionlint-py==1.7.12.24",
"ty>=0.0.40") exist on PyPI (or pin them to exact released versions if you want
the same stability) before running uv sync --group dev.
---
Outside diff comments:
In `@justfile`:
- Around line 539-554: The required-commands check in setup-tools currently
lists "jq" but setup-tools never installs or guides how to install it; update
setup-tools to either invoke the existing _ensure-jq helper or add a clear
guidance/install step for jq so the "✗ jq" case can be resolved. Specifically,
modify the setup-tools recipe to call _ensure-jq (or echo the same installation
hint used by _ensure-jq) before the verification loop so jq is installed or a
direct instruction is printed when missing, ensuring the required-commands
loop's jq entry is actionable.
In `@scripts/tag_release.py`:
- Line 1: Replace the shebang in scripts/tag_release.py that currently invokes
python3 with the repository-standard uv run shebang used by the other scripts
(i.e., change the "#!/usr/bin/env python3" line to the equivalent that runs via
uv run), and ensure the file remains executable; locate the shebang at the top
of the file to make this one-line change so the script uses uv run instead of
calling python directly.
---
Duplicate comments:
In `@cliff.toml`:
- Line 98: The current generic parser message pattern message = "(?i)^bump "
catches human-authored subjects; tighten it by replacing that pattern with a
regex that requires either an explicit "deps"/"dependencies" keyword or a
package identifier after "bump" (for example using a word boundary and requiring
bump\s+(deps?|dependencies|[A-Za-z0-9_\-\/]+)), so update the message value used
for grouping "Dependencies" accordingly to avoid matching generic "Bump MSRV..."
subjects.
In `@scripts/tests/test_subprocess_utils.py`:
- Around line 134-138: The test test_git_convenience_helpers currently invokes
real git and is brittle; change it to mock the underlying run_git_command calls
used by get_git_commit_hash, get_git_remote_url, check_git_repo, and
check_git_history so they return deterministic values (e.g. a fake commit hash,
a fake remote URL, and success outputs) instead of running git. Use the same
mocking pattern used earlier in the file (patch/monkeypatch run_git_command) and
ensure each helper is asserted against the mocked return values rather than
relying on the real environment.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 73-76: Update the "Set up Python" GH Actions step that uses
actions/setup-python (the step with name "Set up Python" and input key
python-version) to read the interpreter version from the repository
.python-version file instead of hardcoding "3.12": replace the python-version
input with python-version-file and point it to ".python-version" so the workflow
uses that single source of truth; keep the same action/commit reference
(actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405).
In @.github/workflows/codacy.yml:
- Line 86: The pinned GitHub Action reference uses a SHA-only comment on the
uses line
("codacy/codacy-analysis-cli-action@562ee3e92b8e92df8b67e0a5ff8aa8e261919c08")
which lacks a readable release tag; update the trailing comment on that uses
entry to include the resolved release (e.g. "# vX.Y.Z") alongside or replacing
"# pinned" so the line reads like "uses:
codacy/codacy-analysis-cli-action@562ee3e... # vX.Y.Z" — look for the exact uses
entry for codacy/codacy-analysis-cli-action and add the human-readable version
string that the SHA corresponds to.
In `@justfile`:
- Around line 28-36: The pipeline used to extract versions (e.g.,
installed_version="$(cargo llvm-cov --version 2>/dev/null | grep -oE
'[0-9]+\.[0-9]+\.[0-9]+' | head -1)") can fail under set -euo pipefail and abort
the recipe; make the capture non-fatal by allowing the pipeline to return zero
even if grep/head fail — for example, append "|| true" to the command
substitution or wrap only the pipeline in a subshell with "|| true" so
installed_version becomes empty instead of causing exit; apply this change to
every occurrence in the _ensure-* helpers and the inline checks in setup-tools
(references: installed_version assignment lines, _ensure-* helper functions, and
setup-tools check blocks).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9751823a-419b-40b6-bf03-6cc44d392212
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (42)
.github/dependabot.yml.github/workflows/audit.yml.github/workflows/benchmarks.yml.github/workflows/ci.yml.github/workflows/codacy.yml.github/workflows/codecov.yml.github/workflows/codeql.yml.github/workflows/rust-clippy.yml.github/workflows/semgrep-sarif.yml.github/workflows/zizmor.yml.gitignore.python-versionAGENTS.mdCHANGELOG.mdCargo.tomlREADME.mdREFERENCES.mdSECURITY.mdbenches/vs_linalg.rscliff.tomldocs/archive/changelog/0.1.mddocs/archive/changelog/0.2.mddocs/archive/changelog/0.3.mddprint.jsonjustfilepyproject.tomlrust-toolchain.tomlscripts/archive_changelog.pyscripts/bench_compare.pyscripts/criterion_dim_plot.pyscripts/postprocess_changelog.pyscripts/subprocess_utils.pyscripts/tag_release.pyscripts/tests/test_archive_changelog.pyscripts/tests/test_criterion_dim_plot.pyscripts/tests/test_postprocess_changelog.pyscripts/tests/test_subprocess_utils.pyscripts/tests/test_tag_release.pysemgrep.yamlsrc/exact.rstests/proptest_exact.rsty.toml
💤 Files with no reviewable changes (2)
- scripts/tests/test_criterion_dim_plot.py
- .gitignore
| [dependency-groups] | ||
| dev = [ | ||
| "actionlint-py==1.7.12.24", | ||
| "mypy>=1.19.0", | ||
| "pytest>=8.0.0", | ||
| "ruff>=0.12.11", | ||
| "ty>=0.0.8", | ||
| "pytest>=9.0.3", | ||
| "ruff>=0.15.14", | ||
| "semgrep==1.164.0", | ||
| "ty>=0.0.40", | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the latest released version of pytest on PyPI?
💡 Result:
The latest released version of pytest on PyPI as of June 3, 2026, is 9.0.3, which was released on April 7, 2026 [1][2][3].
Citations:
- 1: https://pypi.org/project/pytest/
- 2: https://pypi.org/project/pytest/9.0.3/
- 3: https://pypistats.org/packages/pytest
Update the concern: pytest>=9.0.3 is a released PyPI version.
pytest==9.0.3 is the latest released version on PyPI (released Apr 7, 2026), so the “9.0.x not published → uv sync --group dev fails” risk for pytest specifically is unlikely. Still worth sanity-checking the other pinned dev tools (ruff, semgrep, actionlint-py, ty) for availability to fully guarantee uv sync --group dev resolves.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyproject.toml` around lines 140 - 148, The dev dependencies list currently
uses "pytest>=9.0.3" which should be tightened to the released PyPI version to
avoid uv sync failures; update the dependency in the [dependency-groups] dev
array from "pytest>=9.0.3" to "pytest==9.0.3" and then sanity-check that the
other pinned dev tools ("ruff>=0.15.14", "semgrep==1.164.0",
"actionlint-py==1.7.12.24", "ty>=0.0.40") exist on PyPI (or pin them to exact
released versions if you want the same stability) before running uv sync --group
dev.
2 participants
Closes #117
Summary by CodeRabbit
New Features
Documentation
Chores