fix: round-trip non-UTF-8 text files instead of crashing with UnicodeDecodeError

Author: vitali87

pr_split/cli.py

@@ -204,7 +204,7 @@ def _create_single_branch_and_commit(
             if content is not None:
                 p.parent.mkdir(parents=True, exist_ok=True)
                 # newline="" keeps CRLF from the reconstructed content intact.
-                p.write_text(content, encoding="utf-8", newline="")
+                p.write_text(content, encoding="utf-8", errors="surrogateescape", newline="")
             elif p.exists():
                 p.unlink()
 

pr_split/diff_ops/parser.py

@@ -47,7 +47,10 @@ def extract_diff(dev_branch: str, base_branch: str) -> str:
     )
     if result.returncode != 0:
         raise GitOperationError(result.stderr.decode("utf-8", errors="replace").strip())
-    return result.stdout.decode("utf-8")
+    # Git diffs any NUL-free file as text, so legacy latin-1 sources reach us
+    # too; surrogateescape keeps their bytes intact so they round-trip when
+    # the worker writes them back with the same error handler.
+    return result.stdout.decode("utf-8", errors="surrogateescape")
 
 
 _C_ESCAPES = {

pr_split/diff_ops/reconstructor.py

@@ -68,7 +68,7 @@ def _get_base_file_content(file_path: str, ref: str) -> str:
     )
     if result.returncode != 0:
         raise GitOperationError(result.stderr.decode("utf-8", errors="replace").strip())
-    return result.stdout.decode("utf-8")
+    return result.stdout.decode("utf-8", errors="surrogateescape")
 
 
 NO_NEWLINE_MARKER = "\\"

pr_split/plan_store.py

@@ -1,3 +1,4 @@
+import json
 from pathlib import Path
 
 from loguru import logger
@@ -12,15 +13,19 @@ def save_plan(plan_file: PlanFile) -> None:
     path = Path(PLAN_DIR)
     path.mkdir(parents=True, exist_ok=True)
     plan_path = Path(PLAN_FILE)
-    plan_path.write_text(plan_file.model_dump_json(indent=2))
+    # The raw diff may carry surrogate-escaped bytes from non-UTF-8 files;
+    # json.dumps escapes those as \udcXX and loads them back losslessly,
+    # which pydantic's own JSON writer refuses to do.
+    payload = json.dumps(plan_file.model_dump(mode="json"), indent=2, ensure_ascii=True)
+    plan_path.write_text(payload, encoding="utf-8")
     logger.info(logs.SAVING_PLAN.format(path=plan_path))
 
 
 def load_plan() -> PlanFile:
     plan_path = Path(PLAN_FILE)
     if not plan_path.exists():
         raise PRSplitError(ErrorMsg.NO_PLAN())
-    plan_file = PlanFile.model_validate_json(plan_path.read_text())
+    plan_file = PlanFile.model_validate(json.loads(plan_path.read_text(encoding="utf-8")))
     logger.info(logs.PLAN_LOADED.format(count=len(plan_file.plan.groups), path=plan_path))
     return plan_file
 

pr_split/planner/client.py

@@ -107,7 +107,18 @@ def _count_tokens_openai(texts: list[str], *, model: str) -> int:
     return sum(len(enc.encode(t)) for t in texts)
 
 
+def _utf8_safe(text: str) -> str:
+    """Replace surrogate-escaped bytes so the text can be sent as JSON.
+
+    Diff text keeps undecodable bytes as surrogates so files round-trip on
+    disk; the HTTP clients encode request bodies as strict UTF-8, so the
+    prompt copy gets U+FFFD instead.
+    """
+    return text.encode("utf-8", errors="surrogateescape").decode("utf-8", errors="replace")
+
+
 def _count_tokens(system: str, user: str, *, settings: Settings) -> int:
+    system, user = _utf8_safe(system), _utf8_safe(user)
     match settings.provider:
         case Provider.ANTHROPIC:
             return _count_tokens_anthropic(system, user, settings=settings)
@@ -169,6 +180,7 @@ def _call_openai(system: str, user: str, *, settings: Settings) -> RawToolOutput
 
 
 def _call_llm(system: str, user: str, *, settings: Settings) -> RawToolOutput:
+    system, user = _utf8_safe(system), _utf8_safe(user)
     match settings.provider:
         case Provider.ANTHROPIC:
             return _call_anthropic(system, user, settings=settings)

tests/test_client.py

@@ -986,3 +986,24 @@ def test_large_diff_uses_chunking(
         result = _plan_split_with_llm(parsed, settings)
         assert len(result) == 1
         mock_chunked.assert_called_once()
+
+
+class TestSurrogateSafePrompts:
+    @patch("pr_split.planner.client._call_anthropic")
+    def test_call_llm_replaces_surrogates_before_the_request(self, mock_call: MagicMock) -> None:
+        from pr_split.planner.client import _call_llm
+
+        user = b"diff caf\xe9".decode("utf-8", errors="surrogateescape")
+        _call_llm("sys", user, settings=_make_settings())
+        sent_user = mock_call.call_args.args[1]
+        assert "\udce9" not in sent_user
+        assert sent_user == "diff caf\ufffd"
+        sent_user.encode("utf-8")  # must be JSON-serialisable
+
+    @patch("pr_split.planner.client._count_tokens_anthropic", return_value=3)
+    def test_count_tokens_replaces_surrogates(self, mock_count: MagicMock) -> None:
+        from pr_split.planner.client import _count_tokens
+
+        user = b"caf\xe9".decode("utf-8", errors="surrogateescape")
+        assert _count_tokens("sys", user, settings=_make_settings()) == 3
+        mock_count.call_args.args[1].encode("utf-8")

tests/test_diff_parser_extended.py

@@ -355,3 +355,58 @@ def git(*args: str) -> str:
             [(modified, 1), (deleted, 1), ("plain name.md", 1), ('new "file".txt', 1)]
         )
         assert parsed.stats["total_files"] == 4
+
+
+class TestNonUtf8FileContent:
+    def test_latin1_file_round_trips_byte_for_byte(self, tmp_path: Path) -> None:
+        from pr_split.constants import AssignmentType
+        from pr_split.diff_ops.reconstructor import materialize_group_files
+        from pr_split.git_ops.branches import merge_base
+        from pr_split.schemas import Group, GroupAssignment
+
+        def git(*args: str) -> str:
+            return subprocess.run(
+                ["git", "-c", "user.name=t", "-c", "user.email=t@x", *args],
+                cwd=tmp_path,
+                capture_output=True,
+                text=True,
+                check=True,
+            ).stdout
+
+        git("init", "-q", "-b", "main")
+        base_bytes = "caf\xe9 one\nkeep\n".encode("latin-1")
+        dev_bytes = "caf\xe9 two\nkeep\n".encode("latin-1")
+        (tmp_path / "legacy.txt").write_bytes(base_bytes)
+        git("add", "-A")
+        git("commit", "-qm", "base")
+        git("checkout", "-qb", "dev")
+        (tmp_path / "legacy.txt").write_bytes(dev_bytes)
+        git("add", "-A")
+        git("commit", "-qm", "dev")
+
+        cwd = os.getcwd()
+        os.chdir(tmp_path)
+        try:
+            parsed = parse_diff(extract_diff("dev", "main"))
+            group = Group(
+                id="pr-1",
+                title="t",
+                description="d",
+                assignments=[
+                    GroupAssignment(
+                        file_path="legacy.txt",
+                        assignment_type=AssignmentType.WHOLE_FILE,
+                        hunk_indices=[0],
+                    )
+                ],
+            )
+            materialized = materialize_group_files(parsed, group, merge_base("main", "dev"))
+            out = tmp_path / "out.txt"
+            content = materialized["legacy.txt"]
+            assert content is not None
+            out.write_text(content, encoding="utf-8", errors="surrogateescape", newline="")
+        finally:
+            os.chdir(cwd)
+
+        assert [pf.path for pf in parsed.patch_set] == ["legacy.txt"]
+        assert out.read_bytes() == dev_bytes

tests/test_plan_store.py

@@ -106,3 +106,32 @@ def test_saved_file_is_valid_json(
         assert raw["plan"]["priority"] == "logical"
         assert raw["plan"]["min_loc"] == 25
         assert raw["plan"]["strict_loc_bounds"] is True
+
+
+class TestPlanStoreSurrogates:
+    def test_raw_diff_with_undecodable_bytes_round_trips(
+        self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+    ) -> None:
+        from pr_split.constants import Priority
+        from pr_split.plan_store import load_plan, save_plan
+        from pr_split.schemas import PlanFile, SplitPlan
+
+        monkeypatch.chdir(tmp_path)
+        raw = b"--- a/x\n+++ b/x\n@@ -1 +1 @@\n-caf\xe9\n+caf\xe9!\n".decode(
+            "utf-8", errors="surrogateescape"
+        )
+        plan = SplitPlan(
+            dev_branch="dev",
+            base_branch="main",
+            max_loc=400,
+            priority=Priority.ORTHOGONAL,
+            raw_diff=raw,
+        )
+        save_plan(PlanFile(plan=plan))
+
+        loaded = load_plan()
+
+        assert loaded.plan.raw_diff == raw
+        assert loaded.plan.raw_diff.encode("utf-8", errors="surrogateescape").endswith(
+            b"+caf\xe9!\n"
+        )