fix: resolve mutable defaults, narrow exception handling, and sys.modules leak (#33)

Author: zurfjereluhmie

src/evalwire/evaluators/contains.py

@@ -8,9 +8,17 @@
 def make_contains_evaluator() -> Callable[[str, dict], bool]:
     """Return a substring-containment evaluator.
 
-    Checks whether the first value in ``expected["expected_output"]`` appears
-    as a substring of ``output``.  Useful for free-text generation tasks where
-    the answer must include a specific phrase or keyword.
+    Checks whether the **first** value in ``expected["expected_output"]``
+    appears as a substring of ``output``.  Useful for free-text generation
+    tasks where the answer must include a specific phrase or keyword.
+
+    .. note::
+
+       Only the first item of ``expected_output`` is checked.  If the
+       expected value contains pipe-delimited alternatives (e.g.
+       ``"phrase1|phrase2"``), only ``"phrase1"`` is used after splitting.
+       Use the ``membership`` evaluator if you need to check whether the
+       output matches any one of several accepted values.
 
     To test the reverse (output is a substring of the expected string), wrap
     the result with ``not``::

src/evalwire/runner.py

@@ -247,6 +247,7 @@ def _load_attribute(self, path: Path, attribute: str) -> Any:
             spec.loader.exec_module(module)  # type: ignore[union-attr]
             return getattr(module, attribute, None)
         except Exception as exc:
+            sys.modules.pop(module_name, None)
             logger.error("Failed to load %s: %s", path, exc, exc_info=True)
             return None
 

src/evalwire/uploader.py

@@ -41,15 +41,17 @@ def __init__(
         self,
         csv_path: Path | str,
         phoenix_client: Client,
-        input_keys: list[str] = ["user_query"],  # noqa: B006
-        output_keys: list[str] = ["expected_output"],  # noqa: B006
+        input_keys: list[str] | None = None,
+        output_keys: list[str] | None = None,
         tag_column: str = "tags",
         delimiter: str = "|",
     ) -> None:
         self.csv_path = Path(csv_path)
         self.client = phoenix_client
-        self.input_keys = list(input_keys)
-        self.output_keys = list(output_keys)
+        self.input_keys = list(input_keys) if input_keys is not None else ["user_query"]
+        self.output_keys = (
+            list(output_keys) if output_keys is not None else ["expected_output"]
+        )
         self.tag_column = tag_column
         self.delimiter = delimiter
 
@@ -130,12 +132,10 @@ def _upload_one(
                 existing = self.client.datasets.get_dataset(dataset=name)
                 logger.info("Dataset %r already exists, skipping.", name)
                 return existing
-            except Exception:
-                logger.warning(
-                    "Could not fetch dataset %r; creating it instead (if this is "
-                    "not a 'not found' error, check your Phoenix endpoint and credentials).",
+            except ValueError:
+                logger.debug(
+                    "Dataset %r not found; creating it.",
                     name,
-                    exc_info=True,
                 )
             return self.client.datasets.create_dataset(
                 dataframe=df,
@@ -165,13 +165,10 @@ def _upload_one(
                 )
                 logger.debug("Appended %d examples to dataset %r.", len(df), name)
                 return dataset
-            except Exception:
-                logger.warning(
-                    "Could not append to dataset %r; creating it instead "
-                    "(if this is not a 'not found' error, check your Phoenix "
-                    "endpoint and credentials).",
+            except ValueError:
+                logger.debug(
+                    "Dataset %r not found; creating it.",
                     name,
-                    exc_info=True,
                 )
             return self.client.datasets.create_dataset(
                 dataframe=df,

tests/test_cli.py

@@ -75,7 +75,7 @@ def test_upload_custom_input_output_keys(self, tmp_path: Path):
         csv_file = tmp_path / "custom.csv"
         csv_file.write_text("q,ans,grp\nq1,a1,g1\n")
         client = _mock_client()
-        client.datasets.get_dataset.side_effect = Exception("not found")
+        client.datasets.get_dataset.side_effect = ValueError("not found")
         with patch("evalwire.cli._make_client", return_value=client):
             result = _runner().invoke(
                 main,

tests/test_runner.py

@@ -143,6 +143,21 @@ def test_returns_none_on_import_error(
         result = runner._load_attribute(py_file, "anything")
         assert result is None
 
+    def test_cleans_sys_modules_on_import_error(
+        self, tmp_path: Path, mock_phoenix_client: MagicMock
+    ):
+        """A broken module must not leak into sys.modules."""
+        import sys
+
+        py_file = tmp_path / "leaky.py"
+        py_file.write_text("raise RuntimeError('kaboom')\n")
+        runner = _make_runner(tmp_path, mock_phoenix_client)
+        module_name = f"_evalwire_exp_{py_file.parent.name}_{py_file.stem}"
+
+        result = runner._load_attribute(py_file, "anything")
+        assert result is None
+        assert module_name not in sys.modules
+
 
 class TestRun:
     def test_run_calls_run_experiment_for_each_discovered(

tests/test_uploader.py

@@ -3,6 +3,8 @@
 from pathlib import Path
 from unittest.mock import MagicMock
 
+import pytest
+
 from evalwire.uploader import DatasetUploader
 
 
@@ -87,12 +89,22 @@ def test_skips_when_dataset_already_exists(
     def test_uploads_when_dataset_does_not_exist(
         self, sample_csv: Path, mock_phoenix_client: MagicMock
     ):
-        mock_phoenix_client.datasets.get_dataset.side_effect = Exception("not found")
+        mock_phoenix_client.datasets.get_dataset.side_effect = ValueError("not found")
         uploader = _make_uploader(sample_csv, mock_phoenix_client)
         result = uploader.upload(on_exist="skip")
         assert mock_phoenix_client.datasets.create_dataset.called
         assert len(result) == 2
 
+    def test_non_not_found_error_propagates(
+        self, sample_csv: Path, mock_phoenix_client: MagicMock
+    ):
+        mock_phoenix_client.datasets.get_dataset.side_effect = RuntimeError(
+            "connection refused"
+        )
+        uploader = _make_uploader(sample_csv, mock_phoenix_client)
+        with pytest.raises(RuntimeError, match="connection refused"):
+            uploader.upload(on_exist="skip")
+
 
 class TestUploadOverwrite:
     def test_always_calls_upload_dataset(
@@ -105,7 +117,7 @@ def test_always_calls_upload_dataset(
     def test_overwrite_when_dataset_missing_still_creates(
         self, sample_csv: Path, mock_phoenix_client: MagicMock
     ):
-        mock_phoenix_client.datasets.get_dataset.side_effect = Exception("not found")
+        mock_phoenix_client.datasets.get_dataset.side_effect = ValueError("not found")
         uploader = _make_uploader(sample_csv, mock_phoenix_client)
         result = uploader.upload(on_exist="overwrite")
         assert mock_phoenix_client.datasets.create_dataset.called
@@ -123,7 +135,7 @@ def test_calls_append_to_dataset_when_dataset_exists(
     def test_falls_back_to_create_when_dataset_missing(
         self, sample_csv: Path, mock_phoenix_client: MagicMock
     ):
-        mock_phoenix_client.datasets.add_examples_to_dataset.side_effect = Exception(
+        mock_phoenix_client.datasets.add_examples_to_dataset.side_effect = ValueError(
             "not found"
         )
         uploader = _make_uploader(sample_csv, mock_phoenix_client)
@@ -144,7 +156,7 @@ def test_custom_delimiter(self, tmp_path: Path, mock_phoenix_client: MagicMock):
             tag_column="group",
             delimiter=";",
         )
-        mock_phoenix_client.datasets.get_dataset.side_effect = Exception("not found")
+        mock_phoenix_client.datasets.get_dataset.side_effect = ValueError("not found")
         result = uploader.upload(on_exist="skip")
         assert "g1" in result
         assert "g2" in result
@@ -157,6 +169,6 @@ def test_custom_tag_column(self, tmp_path: Path, mock_phoenix_client: MagicMock)
             phoenix_client=mock_phoenix_client,
             tag_column="category",
         )
-        mock_phoenix_client.datasets.get_dataset.side_effect = Exception("not found")
+        mock_phoenix_client.datasets.get_dataset.side_effect = ValueError("not found")
         result = uploader.upload(on_exist="skip")
         assert "cat1" in result