From e7b783ed85d6fac30878009987e029a12cbcca6e Mon Sep 17 00:00:00 2001 From: Andrew Barnes Date: Wed, 15 Jul 2026 10:58:10 -0400 Subject: [PATCH] fix: validate scorer instance names in config (#407) * fix: validate scorer instance names in config * fix: run scorer config tests in CI * fix: drop redundant UV_PYTHON env and quote unittest pattern * fix: report precise scorer name validation errors --- .github/workflows/ci.yml | 4 +++- src/heretic/config.py | 18 ++++++++++++++ src/heretic/evaluator.py | 12 ---------- tests/test_config.py | 51 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 13 deletions(-) create mode 100644 tests/test_config.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f95bf77..087d9f3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -43,7 +43,9 @@ jobs: - name: Run tests env: PYTHONUNBUFFERED: "1" - run: uv run tests/run_tests.py 2>&1 + run: | + uv run python -m unittest discover -s tests -p 'test_*.py' + uv run tests/run_tests.py 2>&1 - name: Build package run: uv build diff --git a/src/heretic/config.py b/src/heretic/config.py index eef2eb3..a119b24 100644 --- a/src/heretic/config.py +++ b/src/heretic/config.py @@ -9,6 +9,7 @@ from pydantic import ( Field, NonNegativeInt, PositiveInt, + field_validator, ) from pydantic_settings import ( BaseSettings, @@ -123,6 +124,23 @@ class ScorerConfig(BaseModel): ), ) + @field_validator("instance_name") + @classmethod + def validate_instance_name(cls, value: str | None) -> str | None: + if value is None: + return value + + if not value.strip(): + raise ValueError("cannot be empty or whitespace") + + if "." in value: + raise ValueError("'.' is not allowed") + + if any(char.isspace() for char in value): + raise ValueError("whitespace is not allowed") + + return value + class BenchmarkSpecification(BaseModel): task: str = Field( diff --git a/src/heretic/evaluator.py b/src/heretic/evaluator.py index 0e6927a..3c6aba7 100644 --- a/src/heretic/evaluator.py +++ b/src/heretic/evaluator.py @@ -67,18 +67,6 @@ class Evaluator: # Instantiate scorers. instance_name = config.instance_name or None - if instance_name is not None: - if not instance_name.strip(): - raise ValueError( - f"Invalid instance_name {instance_name} for scorer {scorer_cls.__name__}: " - "cannot be empty or whitespace" - ) - if "." in instance_name or " " in instance_name: - raise ValueError( - f"Invalid instance_name {instance_name} for scorer {scorer_cls.__name__}: " - "'.' and whitespace are not allowed" - ) - raw_settings = self._get_scorer_settings_raw( scorer_cls=scorer_cls, instance_name=instance_name ) diff --git a/tests/test_config.py b/tests/test_config.py new file mode 100644 index 0000000..7f3e3e5 --- /dev/null +++ b/tests/test_config.py @@ -0,0 +1,51 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2025-2026 Philipp Emanuel Weidmann + contributors + +import unittest + +from pydantic import ValidationError + +from heretic.config import ScorerConfig + + +class ScorerConfigTests(unittest.TestCase): + def test_accepts_slug_like_instance_name(self) -> None: + config = ScorerConfig( + plugin="heretic.scorers.keyword_rate.KeywordRate", + optimization="minimize", + instance_name="small-1", + ) + + self.assertEqual(config.instance_name, "small-1") + + def test_rejects_empty_instance_name(self) -> None: + with self.assertRaises(ValidationError): + ScorerConfig( + plugin="heretic.scorers.keyword_rate.KeywordRate", + optimization="minimize", + instance_name=" \t", + ) + + def test_rejects_whitespace_in_instance_name(self) -> None: + for instance_name in ["small name", "small\tname", "small\nname"]: + with self.subTest(instance_name=instance_name): + with self.assertRaisesRegex( + ValidationError, "whitespace is not allowed" + ): + ScorerConfig( + plugin="heretic.scorers.keyword_rate.KeywordRate", + optimization="minimize", + instance_name=instance_name, + ) + + def test_rejects_dot_in_instance_name(self) -> None: + with self.assertRaisesRegex(ValidationError, "'\\.' is not allowed"): + ScorerConfig( + plugin="heretic.scorers.keyword_rate.KeywordRate", + optimization="minimize", + instance_name="small.name", + ) + + +if __name__ == "__main__": + unittest.main()