Skip to content

Commit 10a8d69

Browse files
chore: pin the lint/type-check toolchain instead of floating it (#533)
requirements.txt pinned all 16 runtime deps with ==, but every entry in requirements-test.txt used >=, so ruff, mypy and bandit resolved to whatever was newest on PyPI at the moment CI ran. Those three decide the Lint and Security Scan verdicts, and unlike a test runner they change their answer on unchanged code: a new ruff rule or a widened mypy check reddens a commit nobody touched, and re-running an old green build stops reproducing it. That is the worst failure mode for an external contributor, whose first PR goes red for a reason not visible in the diff. The drift was already real, not hypothetical. The file read ruff>=0.15.21 while CI had been installing 0.16.0 — a minor-version jump past the 0.15.22 that #522 proposed, which is exactly why Dependabot closed that PR as redundant ("updatable in another way"). The declared floor and the executed version had silently diverged. cache: 'pip' never mitigated this: it caches downloaded wheels, but pip still resolves against the index and still picks the newest match. test.yml:336 already guards the coverage job against this class of problem by reinstalling from the same requirements file, which buys consistency WITHIN a run. This buys reproducibility ACROSS runs. Pins ruff==0.16.0, mypy==2.3.0, bandit==1.9.4 — the versions CI already resolves, so no behaviour changes. The pytest stack deliberately stays on >=: it changes what runs, not what counts as a violation, and pinning it would multiply Dependabot volume for less benefit. Dependabot still manages these files, so upgrades keep arriving — as a reviewable PR with CI attached rather than silently mid-week. A parametrized test in tests/unit/shared/test_pinned_dependencies.py keeps the three from silently loosening again. Closes #532 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 7ba44c0 commit 10a8d69

3 files changed

Lines changed: 54 additions & 3 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ This project uses [Conventional Commits](https://conventionalcommits.org/) and [
66

77
## [Unreleased]
88

9+
### Changed
10+
- **The lint/type-check toolchain is pinned exactly instead of floating** ([#532](https://github.com/bitwize-music-studio/claude-ai-music-skills/issues/532)) — `requirements.txt` pinned all 16 runtime deps with `==`, but every entry in `requirements-test.txt` used `>=`, so `ruff`, `mypy` and `bandit` resolved to whatever was newest on PyPI at the moment CI ran. Those three decide the Lint and Security Scan verdicts, and unlike a test runner they change their answer on unchanged code — a new rule or a widened check reddens a commit nobody touched, and re-running an old green build no longer reproduces it. The drift was already visible: the file read `ruff>=0.15.21` while CI had been installing `0.16.0`, which is why Dependabot closed #522 as redundant. `ruff`, `mypy` and `bandit` are now `==` pins (`cache: 'pip'` never mitigated this — it caches wheels, but pip still resolves to newest). The `pytest` stack stays on `>=`: it changes what runs, not what counts as a violation. A parametrized test in `tests/unit/shared/test_pinned_dependencies.py` keeps the three from silently loosening again.
11+
912
### Fixed
1013
- **`tracks_completed` no longer flips between two different numbers depending on which code path last ran** ([#523](https://github.com/bitwize-music-studio/claude-ai-music-skills/issues/523)) — `reference/state-schema.md` defines the field as "Number of tracks with completed status", but only the incremental track-change path counted the track files. A full rebuild (`scan_albums`) and the incremental README-changed branch both counted the album README's `## Tracklist` table instead. Since `update_track_field` rewrites a track file without touching that table, the two drifted the moment a track's status changed: `rebuild_state()` — the documented remedy for a stale cache — discarded the correct count and reinstated the README's stale one, and editing only the README silently reset it. This also made `list_albums` and `get_album_progress` disagree about the same album, and made the CLI print a README-derived numerator over an actual-file denominator (a finished album could render as `[0/12 tracks]`). All three sites now derive the count from the track files through a single `_count_completed_tracks` helper. `parse_album_readme` still reports what the README table claims, but nothing feeds it into state.
1114
- **A corrupt `ideas` or `skills` section in state.json no longer crashes `python -m tools.state update`** ([#525](https://github.com/bitwize-music-studio/claude-ai-music-skills/issues/525)) — `incremental_update` type-guards its top-level state sections so a wrong-typed one returns `None` and the caller falls back to a full rebuild (the `#393` contract), but the guard only covered `config` and `albums`. The function also does `state['ideas'].get('file_mtime')` and `state['skills'].get('skills_root')`, so a non-mapping value in either raised `AttributeError` straight past `cmd_update`'s `is None` fallback and aborted the CLI with a traceback — the exact failure the guard exists to prevent. All four sections are now guarded; both are re-derived from disk on a rebuild, so falling back loses nothing.

requirements-test.txt

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,15 @@ pytest-xdist>=3.8.0
99
pyyaml>=6.0
1010

1111
# Linting
12-
ruff>=0.16.0
13-
mypy>=2.3.0
12+
# ruff and mypy are pinned exactly, not floored: their output IS the Lint job's
13+
# verdict, and a new rule or widened check reddens a commit nobody touched while
14+
# an old green build stops reproducing. Dependabot still bumps these — the
15+
# difference is the upgrade arrives as a reviewable PR with CI attached (#532).
16+
ruff==0.16.0
17+
mypy==2.3.0
1418
types-PyYAML>=6.0.12.20260724
1519

1620
# Security
17-
bandit>=1.9.4
21+
# Pinned exactly for the same reason as ruff/mypy above — bandit decides the
22+
# Security Scan gate.
23+
bandit==1.9.4

tests/unit/shared/test_pinned_dependencies.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
from __future__ import annotations
2222

2323
import importlib
24+
import re
25+
from pathlib import Path
2426

2527
import pytest
2628

@@ -50,3 +52,43 @@ def test_pinned_dependency_imports(module_name: str, dist_name: str) -> None:
5052
"combination. Do not silence it with importorskip — that would hide "
5153
"the regression this test exists to surface."
5254
)
55+
56+
57+
# Tools whose output IS the pass/fail verdict of a CI gate. Unlike the test
58+
# runner, these change their verdict on unchanged code — a new ruff rule or a
59+
# widened mypy check reddens a build nobody touched, and a re-run of an old
60+
# green commit no longer reproduces. Exact pins move those upgrades into a
61+
# reviewable Dependabot PR with CI attached instead of landing silently
62+
# mid-week (#532).
63+
#
64+
# Deliberately NOT covering pytest/pytest-cov/pytest-xdist/pyyaml/types-PyYAML:
65+
# they change *what runs*, not *what counts as a violation*, so they stay on
66+
# `>=` to keep Dependabot volume proportionate.
67+
GATE_TOOLS = ["ruff", "mypy", "bandit"]
68+
69+
70+
def _requirement_line(requirements_text: str, package: str) -> str | None:
71+
"""Return the requirement line for *package*, ignoring comments."""
72+
pattern = re.compile(rf"^{re.escape(package)}\s*([<>=!~].*)$", re.MULTILINE)
73+
match = pattern.search(requirements_text)
74+
return match.group(1).strip() if match else None
75+
76+
77+
@pytest.mark.unit
78+
@pytest.mark.parametrize("package", GATE_TOOLS)
79+
def test_gate_tool_is_exactly_pinned(package: str, project_root: Path) -> None:
80+
"""ruff/mypy/bandit must use `==`, so a CI verdict is reproducible."""
81+
text = (project_root / "requirements-test.txt").read_text(encoding="utf-8")
82+
spec = _requirement_line(text, package)
83+
84+
assert spec is not None, (
85+
f"{package} not found in requirements-test.txt — if it was removed, drop "
86+
f"it from GATE_TOOLS too."
87+
)
88+
assert spec.startswith("=="), (
89+
f"{package} is specified as `{package}{spec}` in requirements-test.txt. "
90+
f"Gate-deciding tools must be pinned exactly (`{package}==X.Y.Z`): with "
91+
f"`>=`, CI resolves whatever is newest at that moment, so a release by "
92+
f"{package} can fail a commit nobody touched and an old green build "
93+
f"stops reproducing. See #532."
94+
)

0 commit comments

Comments
 (0)