postprocess: fix configuration and filtering papercuts

c2rust-postprocess/README.md

@@ -26,8 +26,8 @@ attempts to produce fully safe and idiomatic Rust; not a replacement for these e
 
 `c2rust-postprocess` has a few ways to filter/exclude the function identifiers that are processed.
 
-`--ident-filter` simply takes a regex.
-Anything matching the regex is filtered out of being processed.
+`--ident-filter` takes a regex.
+Only identifiers matching the regex are processed.
 This is very useful for on-the-fly filtering that's easy to change quickly.
 
 `--exclude-file` is for more granular, more permanent filtering/exclusion.
@@ -70,4 +70,3 @@ uv run pytest -v tests/test_utils.py # filter tests to run
 
 - `uv run ruff format` to format
 - `uv run ruff check --fix .` to lint
-

c2rust-postprocess/postprocess/__init__.py

@@ -11,12 +11,15 @@
 
 from postprocess.cache import DirectoryCache, FrozenCache
 from postprocess.exclude_list import IdentifierExcludeList, format_exclude_entries
-from postprocess.models import api_key_from_env, get_model_by_id
+from postprocess.models import (
+    AbstractGenerativeModel,
+    api_key_from_env,
+    get_model_by_id,
+)
 from postprocess.models.gpt import GPTModel
 from postprocess.models.mock import MockGenerativeModel
 from postprocess.transforms import get_transform_by_id
 from postprocess.transforms.base import TransformError, TransformResult
-from postprocess.transforms.comments import AbstractGenerativeModel
 from postprocess.utils import existing_file
 from postprocess.validate import BaselineError, make_validator
 
@@ -132,9 +135,6 @@ def build_arg_parser() -> argparse.ArgumentParser:
         ),
     )
 
-    # TODO: add option to select model
-    # TODO: add option to configure cache
-
     return parser
 
 

c2rust-postprocess/postprocess/exclude_list.py

@@ -40,18 +40,12 @@ def __init__(self, src_path: Path | None) -> None:
             path = check_isinstance(path, str)
             identifiers = check_isinstance(identifiers, list)
             identifiers = [check_isinstance(ident, str) for ident in identifiers]
-            path = Path(path)
-            existing_identifiers = self.paths.get(path)
-            if existing_identifiers is None:
-                self.paths[path] = set(identifiers)
-            else:
-                existing_identifiers.update(*identifiers)
+            path = (src_path.parent / path).resolve()
+            self.paths.setdefault(path, set()).update(identifiers)
 
     def contains(self, path: Path, identifier: str) -> bool:
         # No `src_path` means an empty exclude list.
         if self.src_path is None:
             return False
-        # Consider paths relative to `src_path`, the location of the exclude file.
-        rel_path: Path = path.relative_to(self.src_path.parent)
-        identifiers = self.paths.get(rel_path, set())
+        identifiers = self.paths.get(path.resolve(), set())
         return identifier in identifiers

c2rust-postprocess/postprocess/models/base.py

@@ -15,36 +15,6 @@ def __init__(self, id: str):
     def id(self) -> str:
         return self._id
 
-    # @abstractmethod
-    # async def agenerate_with_tools(
-    #     self,
-    #     messages: list[dict[str, Any]],
-    #     tools: list[Callable] | None = None,
-    #     max_tool_loops: int = 5
-    # ) -> Any:
-    #     """
-    #     Generate a response using native automatic function calling.
-
-    #     Args:
-    #         messages: Chat history.
-    #         tools: List of Python functions.
-    #         max_tool_loops: Maximum number of times the model can call tools
-    #                         consecutively.
-
-    #     Returns:
-    #         The final natural language response from the model.
-    #     """
-    #     pass
-
-    # def generate_with_tools(
-    #     self,
-    #     messages: list[dict[str, Any]],
-    #     tools: Iterable[Callable[..., Any]] = (),
-    #     max_tool_loops: int = 5
-    # ) -> str | None:
-    #     """Synchronous wrapper for agenerate_response."""
-    #     return asyncio.run(self.agenerate_with_tools(messages, tools, max_tool_loops))
-
     @abstractmethod
     def generate_with_tools(
         self,

c2rust-postprocess/postprocess/transforms/base.py

@@ -14,7 +14,8 @@
     update_rust_definition,
 )
 from postprocess.exclude_list import IdentifierExcludeList
-from postprocess.models import AbstractGenerativeModel, api_key_from_env
+from postprocess.models import AbstractGenerativeModel
+from postprocess.models.mock import MockGenerativeModel
 from postprocess.utils import get_highlighted_c
 from postprocess.validate import BatchValidator, Candidate
 
@@ -143,7 +144,7 @@ def generate(
                     f"failed validation: {stale}"
                 )
 
-        if api_key_from_env(self.model.id) is None:
+        if isinstance(self.model, MockGenerativeModel):
             if error is not None:
                 # Can't regenerate the invalid cached response without a key.
                 raise error
@@ -336,13 +337,3 @@ def apply_file(
             )
 
         return result
-
-
-# TODO: We probably want a an interface that generates validators specialized to
-#       each individual prompt so maybe this should take in some transform-
-#       specific parameters and return a callable that only takes the LLM
-#       response as input.
-# class AbstractValidator(ABC):
-#     @abstractmethod
-#     def validate_response(self, response: str) -> str:
-#         pass

c2rust-postprocess/postprocess/utils.py

@@ -11,7 +11,6 @@
 from pygments.lexer import RegexLexer
 from pygments.lexers.c_cpp import CLexer
 from pygments.lexers.rust import RustLexer
-from requests.exceptions import JSONDecodeError
 
 
 def check_isinstance[T](value: object, typ: type[T]) -> T:
@@ -53,7 +52,7 @@ def existing_file(value: str) -> Path:
 def get_compile_commands(compile_commands_path: Path) -> list[dict[str, Any]]:
     try:
         compile_commands = json.loads(compile_commands_path.read_text())
-    except JSONDecodeError as exc:
+    except json.JSONDecodeError as exc:
         raise RuntimeError(
             f"Failed to parse JSON from {compile_commands_path}: {exc}"
         ) from exc

c2rust-postprocess/tests/test_comments_transform.py

@@ -5,6 +5,7 @@
 
 from postprocess.cache import AbstractCache
 from postprocess.definitions import CDefinition
+from postprocess.models import AbstractGenerativeModel
 from postprocess.models.mock import MockGenerativeModel
 from postprocess.transforms import base
 from postprocess.transforms.base import TransformError
@@ -237,11 +238,11 @@ def invalidate(self, *, transform: str, identifier: str) -> None:
         raise AssertionError("no invalidation expected")
 
 
-class QueuedModel(MockGenerativeModel):
+class QueuedModel(AbstractGenerativeModel):
     """Mock model that returns queued responses."""
 
     def __init__(self, responses: list[str]):
-        super().__init__()
+        super().__init__("test-model")
         self.responses = responses
         self.calls = 0
 
@@ -269,7 +270,7 @@ def generate_with_tools(self, messages, tools=(), max_tool_loops=5):
 
 
 def apply_to_body_comment_fn(
-    cache: AbstractCache, model: MockGenerativeModel
+    cache: AbstractCache, model: AbstractGenerativeModel
 ) -> str | None:
     transform = CommentsTransform(cache=cache, model=model)
     return transform.apply_ident(
@@ -281,8 +282,7 @@ def apply_to_body_comment_fn(
     )
 
 
-def test_rejected_response_is_regenerated(monkeypatch) -> None:
-    monkeypatch.setattr(base, "api_key_from_env", lambda model_id: "test-key")
+def test_rejected_response_is_regenerated() -> None:
     cache = RecordingCache(None)
     model = QueuedModel([BAD_RESPONSE, GOOD_RESPONSE])
 
@@ -297,8 +297,7 @@ def test_rejected_response_is_regenerated(monkeypatch) -> None:
     assert response == GOOD_RESPONSE
 
 
-def test_invalid_cached_response_is_regenerated(monkeypatch) -> None:
-    monkeypatch.setattr(base, "api_key_from_env", lambda model_id: "test-key")
+def test_invalid_cached_response_is_regenerated() -> None:
     cache = RecordingCache(BAD_RESPONSE)
     model = QueuedModel([GOOD_RESPONSE])
 
@@ -312,8 +311,7 @@ def test_invalid_cached_response_is_regenerated(monkeypatch) -> None:
     assert response == GOOD_RESPONSE
 
 
-def test_code_changing_cached_response_is_regenerated(monkeypatch) -> None:
-    monkeypatch.setattr(base, "api_key_from_env", lambda model_id: "test-key")
+def test_code_changing_cached_response_is_regenerated() -> None:
     cache = RecordingCache(BAD_CODE_RESPONSE)
     model = QueuedModel([GOOD_RESPONSE])
 

c2rust-postprocess/tests/test_exclude_list.py

@@ -0,0 +1,48 @@
+from pathlib import Path
+
+import pytest
+
+from postprocess.exclude_list import IdentifierExcludeList
+
+
+@pytest.mark.parametrize("absolute_exclude", [False, True])
+@pytest.mark.parametrize("absolute_source", [False, True])
+def test_matches_relative_and_absolute_paths(
+    tmp_path: Path, monkeypatch, absolute_exclude: bool, absolute_source: bool
+) -> None:
+    monkeypatch.chdir(tmp_path)
+    Path("project").mkdir()
+    exclude_path = Path("project/exclude.yml")
+    exclude_path.write_text("src/lib.rs:\n  - excluded\n")
+    source_path = Path("project/src/lib.rs")
+    if absolute_exclude:
+        exclude_path = exclude_path.resolve()
+    if absolute_source:
+        source_path = source_path.resolve()
+
+    exclude_list = IdentifierExcludeList(exclude_path)
+
+    assert exclude_list.contains(source_path, "excluded")
+    assert not exclude_list.contains(source_path, "included")
+
+
+def test_paths_resolve_relative_to_exclude_file(tmp_path: Path) -> None:
+    config_dir = tmp_path / "config"
+    config_dir.mkdir()
+    exclude_path = config_dir / "exclude.yml"
+    exclude_path.write_text("../src/lib.rs:\n  - excluded\n")
+    exclude_list = IdentifierExcludeList(exclude_path)
+
+    assert exclude_list.contains(tmp_path / "src/lib.rs", "excluded")
+    assert not exclude_list.contains(tmp_path / "other/lib.rs", "excluded")
+
+
+def test_combines_identifiers_for_equivalent_paths(tmp_path: Path) -> None:
+    exclude_path = tmp_path / "exclude.yml"
+    exclude_path.write_text("src/lib.rs:\n  - first\n./src/lib.rs:\n  - second\n")
+    exclude_list = IdentifierExcludeList(exclude_path)
+    source_path = tmp_path / "src/lib.rs"
+
+    assert exclude_list.contains(source_path, "first")
+    assert exclude_list.contains(source_path, "second")
+    assert not exclude_list.contains(source_path, "s")