From 97c0cd29052e97a146ab25ebc48765289739447c Mon Sep 17 00:00:00 2001 From: Rio Yu <52408936+rioyu123@users.noreply.github.com> Date: Thu, 27 Aug 2026 11:24:33 +0800 Subject: [PATCH 1/2] fix(hygiene): detect unpinned marked requirements Signed-off-by: Rio Yu <52408936+rioyu123@users.noreply.github.com> --- CHANGELOG.md | 3 ++ src/skillevaluator/validators/hygiene.py | 6 ++- tests/validators/test_hygiene.py | 57 ++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a2ad6515..88728573 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ All notable changes to SkillEvaluator are documented in this file. ### Fixed +- Unpinned-dependency warnings are no longer suppressed by comparison + operators inside PEP 508 environment markers; requirements such as + `pkg; python_version < "3.13"` are now correctly reported. - Tier 3 paired pass@k evidence now respects Python's active integer-string conversion limit, preserves nonzero Wilson interval widths and paired-effect directions at large case counts, and documents exact-rational omission diff --git a/src/skillevaluator/validators/hygiene.py b/src/skillevaluator/validators/hygiene.py index fb243210..d5544728 100644 --- a/src/skillevaluator/validators/hygiene.py +++ b/src/skillevaluator/validators/hygiene.py @@ -166,10 +166,14 @@ def _check_requirements_file(self, req_file: Path) -> ValidationResult: continue pkg_name = match.group(1).lower() + # Marker comparisons do not constrain the package version. Leave + # direct references intact because their URLs may contain ";"; + # markers on direct references therefore keep existing behavior. + constraint_part = line if "@" in line else line.partition(";")[0] if pkg_name in banned_lower: result.add_error(f"{req_file.name}:{line_num} - Banned package: {pkg_name}") - elif not re.search(r"[=<>!]", line): + elif not re.search(r"[=<>!]", constraint_part): result.add_warning(f"{req_file.name}:{line_num} - Unpinned: {line}") if not result.errors and not result.warnings: diff --git a/tests/validators/test_hygiene.py b/tests/validators/test_hygiene.py index ae1b7a07..1100e7da 100644 --- a/tests/validators/test_hygiene.py +++ b/tests/validators/test_hygiene.py @@ -267,6 +267,63 @@ def test_detects_unpinned_dependencies(self, tmp_path: Path): all_messages = result.errors + result.warnings assert any("unpinned" in m.lower() for m in all_messages) + @pytest.mark.parametrize( + "requirement", + [ + 'requests; python_version < "3.13"', + 'requests; python_version != "3.12"', + 'requests;python_version<"3.13"', + 'requests ; python_version < "3.13"', + 'requests; sys_platform == "win32"', + 'requests[security]; python_version >= "3.9"', + ], + ) + def test_environment_marker_comparisons_do_not_hide_unpinned_requirements( + self, tmp_path: Path, requirement: str + ): + """Marker operators must not count as package version constraints.""" + requirements = tmp_path / "requirements.txt" + requirements.write_text(f"{requirement}\n", encoding="utf-8") + + result = HygieneValidator()._check_requirements_file(requirements) + + assert result.errors == [] + assert result.warnings == [f"requirements.txt:1 - Unpinned: {requirement}"] + + @pytest.mark.parametrize( + "requirement", + [ + 'requests>=2; python_version < "3.13"', + "requests>=2.0,<3.0", + "requests==2.31.0", + "requests~=2.31", + "requests!=2.30.0", + "requests @ https://example.invalid/a;v=1/requests.whl", + 'requests @ https://example.invalid/requests.whl ; python_version < "3.13"', + ], + ) + def test_marker_handling_preserves_constrained_and_direct_requirements( + self, tmp_path: Path, requirement: str + ): + """Existing version constraints and direct-reference behavior remain accepted.""" + requirements = tmp_path / "requirements.txt" + requirements.write_text(f"{requirement}\n", encoding="utf-8") + + result = HygieneValidator()._check_requirements_file(requirements) + + assert result.errors == [] + assert result.warnings == [] + + def test_banned_requirement_with_marker_remains_an_error(self, tmp_path: Path): + """Banned-package errors keep precedence over marker-aware warnings.""" + requirements = tmp_path / "requirements.txt" + requirements.write_text('pycrypto; python_version < "3.13"\n', encoding="utf-8") + + result = HygieneValidator()._check_requirements_file(requirements) + + assert result.errors == ["requirements.txt:1 - Banned package: pycrypto"] + assert result.warnings == [] + def test_detects_banned_packages(self, tmp_path: Path): """Test detection of banned/deprecated packages.""" skill_dir = tmp_path / "banned-deps-skill" From 29877b26458c21589ed95fa7fafa4e3b5f629dfd Mon Sep 17 00:00:00 2001 From: Rio Yu <52408936+rioyu123@users.noreply.github.com> Date: Tue, 1 Sep 2026 00:48:53 +0800 Subject: [PATCH 2/2] fix(hygiene): accept plain direct references Signed-off-by: Rio Yu <52408936+rioyu123@users.noreply.github.com> --- src/skillevaluator/validators/hygiene.py | 7 +++---- tests/validators/test_hygiene.py | 1 + 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/skillevaluator/validators/hygiene.py b/src/skillevaluator/validators/hygiene.py index a42a07e6..9d34e9f3 100644 --- a/src/skillevaluator/validators/hygiene.py +++ b/src/skillevaluator/validators/hygiene.py @@ -175,14 +175,13 @@ def _check_requirements_file(self, req_file: Path) -> ValidationResult: pkg_name = match.group(1).lower() # Marker comparisons do not constrain the package version. Detect # direct references before the marker so an "@" inside a marker - # value cannot hide an otherwise unpinned requirement. Keep the - # full direct-reference line because its URL may contain ";". + # value cannot hide an otherwise unpinned requirement. requirement_part = line.partition(";")[0] - constraint_part = line if "@" in requirement_part else requirement_part + is_direct_reference = "@" in requirement_part if pkg_name in banned_lower: result.add_error(f"{req_file.name}:{line_num} - Banned package: {pkg_name}") - elif not re.search(r"[=<>!]", constraint_part): + elif not is_direct_reference and not re.search(r"[=<>!]", requirement_part): result.add_warning(f"{req_file.name}:{line_num} - Unpinned: {line}") if not result.errors and not result.warnings: diff --git a/tests/validators/test_hygiene.py b/tests/validators/test_hygiene.py index 68c776bf..d4e0e40d 100644 --- a/tests/validators/test_hygiene.py +++ b/tests/validators/test_hygiene.py @@ -392,6 +392,7 @@ def test_environment_marker_comparisons_do_not_hide_unpinned_requirements( "requests==2.31.0", "requests~=2.31", "requests!=2.30.0", + "requests @ https://example.invalid/requests.whl", "requests @ https://example.invalid/a;v=1/requests.whl", 'requests @ https://example.invalid/requests.whl ; python_version < "3.13"', ],