From c64d71bb6e712ba2e13a95ea1d9818a70cb5fbc0 Mon Sep 17 00:00:00 2001 From: leshem Date: Sat, 8 Aug 2026 22:32:39 -0400 Subject: [PATCH 1/5] Skip the HELM generation-args tests when HELM cannot be imported MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `pytest.importorskip('helm')` passes on a `helm` install whose `helm.common.codec` does not import — the case on Python 3.14, where crfm-helm's spacy dependency ships no wheel. The tests then fail at collection with the converter's own ImportError instead of skipping. Use the same condition as tests/test_helm_adapter.py: the converter's `_HELM_IMPORT_ERROR`, which is the signal the converter itself acts on. --- tests/test_helm_generation_args.py | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/tests/test_helm_generation_args.py b/tests/test_helm_generation_args.py index 666a51249..714c3c905 100644 --- a/tests/test_helm_generation_args.py +++ b/tests/test_helm_generation_args.py @@ -4,18 +4,29 @@ not silently replaced by adapter defaults. """ -import pytest - -pytest.importorskip( - 'helm', reason='crfm-helm not installed; install with: uv sync --extra helm' -) - from types import SimpleNamespace +import pytest + +import every_eval_ever.converters.helm.adapter as helm_adapter_module from every_eval_ever.converters.helm.adapter import HELMAdapter +# `import helm` alone is not enough: on Python 3.14 the top-level package imports +# but `helm.common.codec` does not, so the converter's own import guard is the +# only reliable signal. Same condition as tests/test_helm_adapter.py. +pytestmark = pytest.mark.skipif( + helm_adapter_module._HELM_IMPORT_ERROR is not None, + reason=( + 'HELM converter dependencies are missing: ' + f'{helm_adapter_module._HELM_IMPORT_ERROR!r}. ' + 'Install with: uv sync --extra helm' + ), +) + -def _make_request_state(temperature=None, max_tokens=None, top_p=None, top_k=None): +def _make_request_state( + temperature=None, max_tokens=None, top_p=None, top_k=None +): """Build a minimal mock RequestState with the given request-level values.""" request = SimpleNamespace( temperature=temperature, @@ -29,7 +40,9 @@ def _make_request_state(temperature=None, max_tokens=None, top_p=None, top_k=Non ) -def _make_adapter_spec(temperature=None, max_tokens=None, top_p=None, top_k=None): +def _make_adapter_spec( + temperature=None, max_tokens=None, top_p=None, top_k=None +): """Build a minimal mock AdapterSpec with the given fallback values.""" return SimpleNamespace( temperature=temperature, From e6b17d58257838909af92fbd3fac9cf6cbfa9794 Mon Sep 17 00:00:00 2001 From: leshem Date: Sun, 9 Aug 2026 07:59:24 -0400 Subject: [PATCH 2/5] HELM: read per-instance stats without importing helm.common.codec The converter needs six HELM modules, which pull in cattrs and nothing else. `helm.common.codec` pulls torch, spacy, nltk and regex, for one call in a file that already decodes three other HELM dataclasses with dacite. Decode the fourth the same way. --- every_eval_ever/converters/helm/adapter.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/every_eval_ever/converters/helm/adapter.py b/every_eval_ever/converters/helm/adapter.py index b667628bd..e01016f17 100644 --- a/every_eval_ever/converters/helm/adapter.py +++ b/every_eval_ever/converters/helm/adapter.py @@ -23,7 +23,6 @@ get_model_deployment, ) from helm.benchmark.run_spec import RunSpec - from helm.common.codec import from_json except ( Exception ) as ex: # pragma: no cover - exercised only when optional deps missing @@ -38,7 +37,6 @@ RunSpec = cast(Any, None) get_model_deployment = cast(Any, None) register_builtin_configs_from_helm_package = cast(Any, None) - from_json = cast(Any, None) ModelDeploymentNotFoundError = cast(Any, Exception) from every_eval_ever.converters import SCHEMA_VERSION @@ -224,7 +222,14 @@ def _load_evaluation_run_logfiles(self, dir_path) -> Dict: stats = self._load_file_if_exists(dir_path, self.STATS_FILE) with open(f'{dir_path}/{self.PER_INSTANCE_STATS_FILE}', 'r') as f: - per_instance_stats = from_json(f.read(), List[PerInstanceStats]) + per_instance_stats = [ + from_dict( + data_class=PerInstanceStats, + data=entry, + config=DaciteConfig(cast=[str]), + ) + for entry in json.load(f) + ] return { 'per_instance_stats': per_instance_stats, From d52471818ad47c1677e477d75a18c778c8e234ca Mon Sep 17 00:00:00 2001 From: leshem Date: Sun, 9 Aug 2026 07:59:25 -0400 Subject: [PATCH 3/5] Allow nltk 3.10.2 and later in the helm extra 3.10.1 is the only release carrying the inisec import guard. --- pyproject.toml | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index e252f670f..6022c5d98 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -31,16 +31,14 @@ inspect = ["inspect-ai>=0.3.160,<0.4.0"] helm = [ "crfm-helm>=0.5.14", "typer>=0.12,<1.0", - # crfm-helm pulls nltk transitively. nltk 3.10.1 added an import guard + # crfm-helm pulls nltk transitively. nltk 3.10.1 alone ships an import guard # (nltk/inisec.py, a CWE-427 mitigation) that blocks nltk-initiated imports # of any module resolving *under the current working directory*. uv places # .venv/ inside the project, so site-packages is under the CWD and the guard - # false-positives on nltk's own `regex` dependency -> `import helm` fails at - # import time (breaks the HELM converter in CI's `loose` matrix, and for any - # default venv layout). Cap below the guarded release until nltk fixes the - # false positive (tracking: https://github.com/nltk/nltk/issues/3730); safe - # because crfm-helm is frozen. - "nltk<3.10.1", + # false-positives on nltk's own `regex` dependency -> `import nltk` fails at + # import time (tracking: https://github.com/nltk/nltk/issues/3730). 3.10.2 + # dropped it; its pathsec guard covers corpus paths, not imports. + "nltk!=3.10.1", ] all = [ "every-eval-ever[inspect]", From d66facf1a24e53e9f84172425c7e1368789ea9a2 Mon Sep 17 00:00:00 2001 From: leshem Date: Mon, 10 Aug 2026 11:55:20 -0400 Subject: [PATCH 4/5] Sync the lockfile with the nltk specifier it records `pyproject.toml` relaxed the helm extra's cap to `nltk!=3.10.1` but the lock still recorded `<3.10.1`, so a resolve from the lockfile disagreed with the project it was locked from. --- uv.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/uv.lock b/uv.lock index 4065c6532..e755c1ab8 100644 --- a/uv.lock +++ b/uv.lock @@ -895,7 +895,7 @@ requires-dist = [ { name = "inspect-ai", marker = "extra == 'inspect'", specifier = ">=0.3.160,<0.4.0" }, { name = "jsonschema", specifier = ">=4.26.0,<5.0.0" }, { name = "matplotlib", specifier = ">=3.10.8" }, - { name = "nltk", marker = "extra == 'helm'", specifier = "<3.10.1" }, + { name = "nltk", marker = "extra == 'helm'", specifier = "!=3.10.1" }, { name = "numpy", specifier = ">=2.4.1" }, { name = "pandas", specifier = ">=2.3.3" }, { name = "pydantic", specifier = ">=2.12.5,<3.0.0" }, From 83576f9477eeb7c24140cfb1c82c893c7dc03d2f Mon Sep 17 00:00:00 2001 From: leshem Date: Thu, 13 Aug 2026 16:30:46 -0400 Subject: [PATCH 5/5] Add sentinel test so an installed-but-broken HELM extra fails CI The module-wide skipif on _HELM_IMPORT_ERROR skipped every test in this file whenever the guarded HELM imports failed, including the case where the `helm` package is installed but a submodule import (e.g. helm.common.codec) is broken. A full `uv sync --all-extras` CI row would then report green with HELM silently skipped. Scope the skip to TestExtractGenerationArgsFalsyValues and add one unmarked sentinel that skips only when the top-level `helm` package is absent and otherwise calls _require_helm_dependencies(), so an installed-but-broken extra becomes a hard failure. --- tests/test_helm_generation_args.py | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/tests/test_helm_generation_args.py b/tests/test_helm_generation_args.py index 714c3c905..2c9fff341 100644 --- a/tests/test_helm_generation_args.py +++ b/tests/test_helm_generation_args.py @@ -4,6 +4,7 @@ not silently replaced by adapter defaults. """ +import importlib.util from types import SimpleNamespace import pytest @@ -13,8 +14,10 @@ # `import helm` alone is not enough: on Python 3.14 the top-level package imports # but `helm.common.codec` does not, so the converter's own import guard is the -# only reliable signal. Same condition as tests/test_helm_adapter.py. -pytestmark = pytest.mark.skipif( +# only reliable signal. Same condition as tests/test_helm_adapter.py. Scoped to +# the class rather than the whole module so the sentinel below stays live even +# when the guard has fired. +_requires_helm = pytest.mark.skipif( helm_adapter_module._HELM_IMPORT_ERROR is not None, reason=( 'HELM converter dependencies are missing: ' @@ -24,6 +27,21 @@ ) +def test_helm_extra_is_importable_when_installed(): + """If HELM is installed at all, the converter's guarded imports must work. + + The class-scoped skip above hides a broken guarded import (e.g. a HELM + release whose `helm.common.codec` stops importing) whenever + ``_HELM_IMPORT_ERROR`` is set, so a full ``uv sync --all-extras`` CI row + would report green with HELM silently skipped. This sentinel skips only + when the top-level `helm` package is genuinely absent and otherwise fails, + turning an installed-but-broken extra into a hard failure. + """ + if importlib.util.find_spec('helm') is None: + pytest.skip('HELM is not installed; the extra is optional for core.') + helm_adapter_module._require_helm_dependencies() + + def _make_request_state( temperature=None, max_tokens=None, top_p=None, top_k=None ): @@ -52,6 +70,7 @@ def _make_adapter_spec( ) +@_requires_helm class TestExtractGenerationArgsFalsyValues: """Verify that 0 is treated as a real value, not as missing."""