diff --git a/server/unified_diff.py b/server/unified_diff.py index 8342230a..9e21f1e0 100644 --- a/server/unified_diff.py +++ b/server/unified_diff.py @@ -137,8 +137,14 @@ def append_file(old_path: str = "", new_path: str = "") -> dict[str, Any] | None current_file["hunks"].append(current_hunk) continue - if current_hunk is None or not raw_line: + if current_hunk is None: continue + if not raw_line: + if not in_hunk_body: + continue + # diff.suppressBlankEmpty (and GNU diff) write a blank context + # line without its leading space. Git apply reads it as context. + raw_line = " " prefix = raw_line[0] if prefix not in {" ", "+", "-", "\\"}: continue diff --git a/server/work_artifact_registry.py b/server/work_artifact_registry.py index e44746d3..3f5c1273 100644 --- a/server/work_artifact_registry.py +++ b/server/work_artifact_registry.py @@ -472,7 +472,7 @@ async def collect_git_delta( "conflicts": conflicts, "truncated_paths": truncated_paths, "patch": "\n".join( - part.rstrip("\n") + part.removesuffix("\n") for part in (committed_patch, working_patch, untracked_patch) if part ), diff --git a/tests/test_unified_diff_canvas.py b/tests/test_unified_diff_canvas.py index d242d4ca..2a1f5e23 100644 --- a/tests/test_unified_diff_canvas.py +++ b/tests/test_unified_diff_canvas.py @@ -241,6 +241,45 @@ def test_parse_unified_diff_splits_hunk_lines_only_at_newlines() -> None: ] +def test_parse_unified_diff_reads_empty_lines_inside_hunks_as_blank_context() -> None: + # Output of `git -c diff.suppressBlankEmpty=true diff`: blank context + # lines lose their leading space. + parsed = parse_unified_diff( + "diff --git a/notes.txt b/notes.txt\n" + "--- a/notes.txt\n" + "+++ b/notes.txt\n" + "@@ -1,4 +1,4 @@\n" + " a\n" + "\n" + "-b\n" + "+B\n" + "\n" + "diff --git a/next.txt b/next.txt\n" + "--- a/next.txt\n" + "+++ b/next.txt\n" + "@@ -1 +1 @@\n" + "-old\n" + "+new\n" + "\n" + ) + + first, second = parsed["files"] + assert first["additions"] == 1 + assert first["deletions"] == 1 + assert first["hunks"][0]["lines"] == [ + {"kind": "context", "oldLine": 1, "newLine": 1, "text": "a"}, + {"kind": "context", "oldLine": 2, "newLine": 2, "text": ""}, + {"kind": "remove", "oldLine": 3, "text": "b"}, + {"kind": "add", "newLine": 3, "text": "B"}, + {"kind": "context", "oldLine": 4, "newLine": 4, "text": ""}, + ] + assert second["hunks"][0]["lines"] == [ + {"kind": "remove", "oldLine": 1, "text": "old"}, + {"kind": "add", "newLine": 1, "text": "new"}, + ] + assert parsed["lineCount"] == 7 + + def test_parse_unified_diff_reads_copy_headers() -> None: # Output of `git -c diff.renames=copies diff HEAD` after copying a # modified file: an exact copy has no marker lines or hunks. @@ -292,5 +331,6 @@ def test_parse_unified_diff_reads_copy_headers() -> None: test_diff_canvas_payload_keeps_report_for_local_mode_switch() test_parse_unified_diff_caps_files_without_merging_hunks() test_parse_unified_diff_splits_hunk_lines_only_at_newlines() + test_parse_unified_diff_reads_empty_lines_inside_hunks_as_blank_context() test_parse_unified_diff_reads_copy_headers() print("unified diff canvas tests passed") diff --git a/tests/test_work_artifact_registry.py b/tests/test_work_artifact_registry.py index d4ea541a..e1d171b0 100644 --- a/tests/test_work_artifact_registry.py +++ b/tests/test_work_artifact_registry.py @@ -427,6 +427,46 @@ async def run() -> None: asyncio.run(run()) +def test_attempt_diff_keeps_blank_context_lines_with_suppress_blank_empty() -> None: + async def run() -> None: + with tempfile.TemporaryDirectory(prefix="work_artifact_blank_context_") as temp: + root = Path(temp) / "project" + root.mkdir() + _git(root, "init") + _git(root, "config", "user.email", "amadeus-test@example.invalid") + _git(root, "config", "user.name", "Amadeus Test") + (root / "notes.txt").write_text("a\n\nb\nc\n", encoding="utf-8") + _git(root, "add", ".") + _git(root, "commit", "-m", "baseline") + _git(root, "config", "diff.suppressBlankEmpty", "true") + + store = WorkLedgerStore(Path(temp) / "ledger.sqlite3") + project = store.create_or_get_project(root) + item = store.create_work_item(project.project_id, title="Edit notes") + attempt = store.create_attempt(item.work_item_id, provider="codex", task="Edit notes") + registry = WorkArtifactRegistry(store) + await registry.capture_baseline(attempt, item) + (root / "notes.txt").write_text("a\n\nB\nc\n", encoding="utf-8") + + current_attempt = store.get_attempt(attempt.attempt_id) + assert current_attempt is not None + delta = await registry.finalize_attempt(current_attempt, item) + store.close() + + assert "\n a\n\n-b\n" in delta["patch"] + parsed = parse_unified_diff(delta["patch"], changed_files=delta["changed_files"]) + assert [entry["path"] for entry in parsed["files"]] == ["notes.txt"] + assert parsed["files"][0]["hunks"][0]["lines"] == [ + {"kind": "context", "oldLine": 1, "newLine": 1, "text": "a"}, + {"kind": "context", "oldLine": 2, "newLine": 2, "text": ""}, + {"kind": "remove", "oldLine": 3, "text": "b"}, + {"kind": "add", "newLine": 3, "text": "B"}, + {"kind": "context", "oldLine": 4, "newLine": 4, "text": "c"}, + ] + + asyncio.run(run()) + + def test_non_git_workspace_registers_new_files_without_claiming_preexisting_changes() -> None: async def run() -> None: with tempfile.TemporaryDirectory(prefix="work_artifact_filesystem_") as temp: @@ -651,6 +691,63 @@ async def run() -> None: asyncio.run(run()) +@pytest.mark.parametrize("patch_sources", ["working", "committed", "both"]) +@pytest.mark.parametrize("blank_lines", [1, 2]) +def test_attempt_diff_preserves_trailing_blank_context( + tmp_path: Path, patch_sources: str, blank_lines: int, +) -> None: + async def run() -> None: + root = tmp_path / "workspace" + root.mkdir() + _git(root, "init") + _git(root, "config", "user.email", "amadeus-test@example.invalid") + _git(root, "config", "user.name", "Amadeus Test") + _git(root, "config", "core.autocrlf", "false") + _git(root, "config", "diff.suppressBlankEmpty", "true") + before = ("x\ny\n" + "\n" * blank_lines).encode("utf-8") + after = ("X\ny\n" + "\n" * blank_lines).encode("utf-8") + paths = ( + ["committed.txt", "working.txt"] if patch_sources == "both" + else [f"{patch_sources}.txt"] + ) + for path in paths: + (root / path).write_bytes(before) + _git(root, "add", ".") + _git(root, "commit", "-m", "baseline") + with WorkLedgerStore(tmp_path / "ledger.sqlite3") as store: + project = store.create_or_get_project(root) + item = store.create_work_item(project.project_id, title="Edit files") + attempt = store.create_attempt(item.work_item_id, provider="codex", task="Edit files") + registry = WorkArtifactRegistry(store) + await registry.capture_baseline(attempt, item) + if "committed.txt" in paths: + (root / "committed.txt").write_bytes(after) + _git(root, "commit", "-am", "provider commit") + if "working.txt" in paths: + (root / "working.txt").write_bytes(after) + current_attempt = store.get_attempt(attempt.attempt_id) + assert current_attempt is not None + delta = await registry.finalize_attempt(current_attempt, item) + + parsed = parse_unified_diff(delta["patch"], changed_files=delta["changed_files"]) + assert sorted(file["path"] for file in parsed["files"]) == sorted(paths) + assert parsed["additions"] == parsed["deletions"] == len(paths) + for file in parsed["files"]: + hunk, = file["hunks"] + assert hunk["oldCount"] == hunk["newCount"] == 2 + blank_lines + assert hunk["lines"] == [ + {"kind": "remove", "oldLine": 1, "text": "x"}, + {"kind": "add", "newLine": 1, "text": "X"}, + {"kind": "context", "oldLine": 2, "newLine": 2, "text": "y"}, + *[ + {"kind": "context", "oldLine": number, "newLine": number, "text": ""} + for number in range(3, 3 + blank_lines) + ], + ] + + asyncio.run(run()) + + def _main() -> None: test_committed_delta_survives_and_baseline_dirty_is_not_stolen() test_changed_preexisting_dirty_path_is_ambiguous_not_claimed_exact() @@ -658,6 +755,7 @@ def _main() -> None: test_non_git_workspace_registers_new_files_without_claiming_preexisting_changes() test_explicit_amend_may_change_predecessor_owned_dirty_artifact() test_attempt_diff_lists_committed_copies_once_with_copy_detection() + test_attempt_diff_keeps_blank_context_lines_with_suppress_blank_empty() print("ok: work artifact registry preserves attempt Git boundaries")