From 432a3b46a6127c2a6fbc6a6d2b57dea3bb01b92c Mon Sep 17 00:00:00 2001 From: Per Larsen Date: Sun, 6 Sep 2026 20:39:02 -0700 Subject: [PATCH 1/6] postprocess: resolve exclude entries relative to their file Mixed absolute and relative source/exclude arguments could raise ValueError before processing any functions. Resolve both sides before matching, and union whole identifiers for equivalent YAML paths. Cover path combinations, entries outside the exclude directory, and equivalent entries without splitting identifiers into characters. --- .../postprocess/exclude_list.py | 12 ++--- c2rust-postprocess/tests/test_exclude_list.py | 48 +++++++++++++++++++ 2 files changed, 51 insertions(+), 9 deletions(-) create mode 100644 c2rust-postprocess/tests/test_exclude_list.py diff --git a/c2rust-postprocess/postprocess/exclude_list.py b/c2rust-postprocess/postprocess/exclude_list.py index eec947892c..9ae829e849 100644 --- a/c2rust-postprocess/postprocess/exclude_list.py +++ b/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 diff --git a/c2rust-postprocess/tests/test_exclude_list.py b/c2rust-postprocess/tests/test_exclude_list.py new file mode 100644 index 0000000000..380b84da08 --- /dev/null +++ b/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") From 2889d97b33580e96b339c536a7d69f60a205358a Mon Sep 17 00:00:00 2001 From: Per Larsen Date: Sun, 6 Sep 2026 20:39:02 -0700 Subject: [PATCH 2/6] postprocess: honor explicitly configured model clients Generation rechecked provider environment variables after model selection, silently skipping CRISP endpoints and clients constructed with an explicit API key. Use the selected cache-only mock to decide whether generation is available. Exercise retries with a configured fake model that needs no provider environment variable. --- .../postprocess/transforms/base.py | 5 +++-- .../tests/test_comments_transform.py | 16 +++++++--------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/c2rust-postprocess/postprocess/transforms/base.py b/c2rust-postprocess/postprocess/transforms/base.py index f446d2efeb..deaa21f218 100644 --- a/c2rust-postprocess/postprocess/transforms/base.py +++ b/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 diff --git a/c2rust-postprocess/tests/test_comments_transform.py b/c2rust-postprocess/tests/test_comments_transform.py index 0cf697ccab..ee594a0e71 100644 --- a/c2rust-postprocess/tests/test_comments_transform.py +++ b/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]) From 2141c7eae44b0833ea0a8fd5d4bccea2d487a1c0 Mon Sep 17 00:00:00 2001 From: Per Larsen Date: Sun, 6 Sep 2026 20:39:02 -0700 Subject: [PATCH 3/6] postprocess: catch the standard-library JSON decoding error json.loads raises json.JSONDecodeError, not the requests exception. Catch the correct exception so malformed compile_commands.json files receive the intended path-specific error. This also removes an undeclared requests import. --- c2rust-postprocess/postprocess/utils.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/c2rust-postprocess/postprocess/utils.py b/c2rust-postprocess/postprocess/utils.py index 2627b5c69a..b577fb187d 100644 --- a/c2rust-postprocess/postprocess/utils.py +++ b/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 From 599bc5aa0eaa3a51cd6ea01dc8c0e9f1c5384fbc Mon Sep 17 00:00:00 2001 From: Per Larsen Date: Sun, 6 Sep 2026 20:39:02 -0700 Subject: [PATCH 4/6] postprocess: import the model interface directly The CLI uses the model interface independently of comment transfer. Import it from the models package instead of relying on an incidental import in the comments transform. --- c2rust-postprocess/postprocess/__init__.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/c2rust-postprocess/postprocess/__init__.py b/c2rust-postprocess/postprocess/__init__.py index 6cd6a24222..f4ba3d3c7f 100644 --- a/c2rust-postprocess/postprocess/__init__.py +++ b/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 From 19c10a3399cab664056d43917d9b1d121c767a20 Mon Sep 17 00:00:00 2001 From: Per Larsen Date: Sun, 6 Sep 2026 20:39:02 -0700 Subject: [PATCH 5/6] postprocess: remove obsolete interface sketches and TODOs Delete commented-out model and validator interface sketches, along with TODOs for model and cache options that already exist. Active interfaces and behavior are unchanged. --- c2rust-postprocess/postprocess/__init__.py | 3 -- c2rust-postprocess/postprocess/models/base.py | 30 ------------------- .../postprocess/transforms/base.py | 10 ------- 3 files changed, 43 deletions(-) diff --git a/c2rust-postprocess/postprocess/__init__.py b/c2rust-postprocess/postprocess/__init__.py index f4ba3d3c7f..1f6753af5b 100644 --- a/c2rust-postprocess/postprocess/__init__.py +++ b/c2rust-postprocess/postprocess/__init__.py @@ -135,9 +135,6 @@ def build_arg_parser() -> argparse.ArgumentParser: ), ) - # TODO: add option to select model - # TODO: add option to configure cache - return parser diff --git a/c2rust-postprocess/postprocess/models/base.py b/c2rust-postprocess/postprocess/models/base.py index 3739b876be..4473476b24 100644 --- a/c2rust-postprocess/postprocess/models/base.py +++ b/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, diff --git a/c2rust-postprocess/postprocess/transforms/base.py b/c2rust-postprocess/postprocess/transforms/base.py index deaa21f218..b6e21ecd8a 100644 --- a/c2rust-postprocess/postprocess/transforms/base.py +++ b/c2rust-postprocess/postprocess/transforms/base.py @@ -337,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 From f7cbb5ed0948b76eb5c59fd0fa8fbe2b729728f8 Mon Sep 17 00:00:00 2001 From: Per Larsen Date: Sun, 6 Sep 2026 20:39:02 -0700 Subject: [PATCH 6/6] postprocess: correct the identifier filter documentation The filter selects matching identifiers for processing; the README incorrectly described matches as excluded. --- c2rust-postprocess/README.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/c2rust-postprocess/README.md b/c2rust-postprocess/README.md index 3a0de3726f..1b90e42e08 100644 --- a/c2rust-postprocess/README.md +++ b/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 -