From 297d0c3d5922f99ac5bc02a3dcf060af76de915d Mon Sep 17 00:00:00 2001 From: NopeCharon <115794254+NopeCharon@users.noreply.github.com> Date: Wed, 4 Nov 2026 13:50:06 +0530 Subject: [PATCH 1/4] fix(work): keep blank context lines in the attempt diff canvas With diff.suppressBlankEmpty=true, git diff writes a blank context line as an empty line instead of a single space. Git apply and GNU patch read that line as context, but parse_unified_diff skipped every empty line. The Work attempt diff canvas therefore dropped blank context lines and numbered every following line of the hunk too low. Read an empty line as blank context while the hunk header still expects old or new lines. Empty lines outside a hunk body, such as the one after the patch's final newline, are still ignored. No command-line option overrides diff.suppressBlankEmpty, so the parser is the layer that has to accept both forms. Tests cover the parser directly and an attempt finalized in a real repository with diff.suppressBlankEmpty set; both fail without the fix. Fixes #285 --- server/unified_diff.py | 8 +++++- tests/test_unified_diff_canvas.py | 40 +++++++++++++++++++++++++++ tests/test_work_artifact_registry.py | 41 ++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 1 deletion(-) diff --git a/server/unified_diff.py b/server/unified_diff.py index a19c6a69..c4fb1046 100644 --- a/server/unified_diff.py +++ b/server/unified_diff.py @@ -128,8 +128,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/tests/test_unified_diff_canvas.py b/tests/test_unified_diff_canvas.py index cc9bd47b..24df0042 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 + + if __name__ == "__main__": test_parse_unified_diff_preserves_dual_line_numbers() test_parse_unified_diff_keeps_untracked_files_without_hunks() @@ -250,4 +289,5 @@ def test_parse_unified_diff_splits_hunk_lines_only_at_newlines() -> 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() print("unified diff canvas tests passed") diff --git a/tests/test_work_artifact_registry.py b/tests/test_work_artifact_registry.py index 8ef84c58..1b6cc31d 100644 --- a/tests/test_work_artifact_registry.py +++ b/tests/test_work_artifact_registry.py @@ -308,6 +308,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: @@ -538,6 +578,7 @@ def _main() -> None: test_non_ascii_entry_revision_is_registered_as_the_real_path() test_non_git_workspace_registers_new_files_without_claiming_preexisting_changes() test_explicit_amend_may_change_predecessor_owned_dirty_artifact() + test_attempt_diff_keeps_blank_context_lines_with_suppress_blank_empty() print("ok: work artifact registry preserves attempt Git boundaries") From 27fc14f1196b9a9a6da94fe464c4d06397909aeb Mon Sep 17 00:00:00 2001 From: NopeCharon <115794254+NopeCharon@users.noreply.github.com> Date: Wed, 4 Nov 2026 13:50:08 +0530 Subject: [PATCH 2/4] fix(work): read copy headers in the attempt diff canvas With diff.renames=copies, git diff reports a new file copied from a changed file with "copy from" / "copy to" extended headers. The parser handled the equivalent rename headers but ignored these, so a copy was shown as "modified". An exact copy has no ---/+++ lines; when its source and target paths contain spaces the diff --git header cannot be split, so the canvas showed a phantom entry plus the real path again as a hunk-less entry from changed_files. Read the copy headers like the rename headers: the entry gets status "copied", oldPath from "copy from", and path/newPath from "copy to". The status is displayed as text by the canvas; no consumer branches on its value. Tests cover exact and edited copies in the parser and a committed copy in an attempt finalized with diff.renames=copies; both fail without the fix. Fixes #286 --- server/unified_diff.py | 9 ++++++ tests/test_unified_diff_canvas.py | 43 +++++++++++++++++++++++++ tests/test_work_artifact_registry.py | 48 ++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+) diff --git a/server/unified_diff.py b/server/unified_diff.py index a19c6a69..8342230a 100644 --- a/server/unified_diff.py +++ b/server/unified_diff.py @@ -109,6 +109,15 @@ def append_file(old_path: str = "", new_path: str = "") -> dict[str, Any] | None current_file["newPath"] = raw_line[len("rename to ") :].strip() current_file["path"] = current_file["newPath"] continue + if raw_line.startswith("copy from "): + current_file["status"] = "copied" + current_file["oldPath"] = raw_line[len("copy from ") :].strip() + continue + if raw_line.startswith("copy to "): + current_file["status"] = "copied" + current_file["newPath"] = raw_line[len("copy to ") :].strip() + current_file["path"] = current_file["newPath"] + continue hunk_match = _HUNK_HEADER.match(raw_line) if hunk_match: diff --git a/tests/test_unified_diff_canvas.py b/tests/test_unified_diff_canvas.py index cc9bd47b..d242d4ca 100644 --- a/tests/test_unified_diff_canvas.py +++ b/tests/test_unified_diff_canvas.py @@ -241,6 +241,48 @@ def test_parse_unified_diff_splits_hunk_lines_only_at_newlines() -> None: ] +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. + parsed = parse_unified_diff( + "\n".join( + [ + "diff --git a/my src.txt b/my dup.txt", + "similarity index 100%", + "copy from my src.txt", + "copy to my dup.txt", + "diff --git a/lib.py b/lib copy.py", + "similarity index 80%", + "copy from lib.py", + "copy to lib copy.py", + "index 01e79c3..94ebaf9 100644", + "--- a/lib.py", + "+++ b/lib copy.py", + "@@ -1 +1,2 @@", + " base", + "+copy", + "diff --git a/my src.txt b/my src.txt", + "index b566061..b13f895 100644", + "--- a/my src.txt", + "+++ b/my src.txt", + "@@ -1 +1,2 @@", + " base", + "+source", + ] + ), + changed_files=["my dup.txt", "lib copy.py", "my src.txt"], + ) + + assert [ + (item["path"], item["oldPath"], item["newPath"], item["status"], item["additions"]) + for item in parsed["files"] + ] == [ + ("my dup.txt", "my src.txt", "my dup.txt", "copied", 0), + ("lib copy.py", "lib.py", "lib copy.py", "copied", 1), + ("my src.txt", "my src.txt", "my src.txt", "modified", 1), + ] + + if __name__ == "__main__": test_parse_unified_diff_preserves_dual_line_numbers() test_parse_unified_diff_keeps_untracked_files_without_hunks() @@ -250,4 +292,5 @@ def test_parse_unified_diff_splits_hunk_lines_only_at_newlines() -> 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_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 8ef84c58..d8764a61 100644 --- a/tests/test_work_artifact_registry.py +++ b/tests/test_work_artifact_registry.py @@ -308,6 +308,53 @@ async def run() -> None: asyncio.run(run()) +def test_attempt_diff_lists_committed_copies_once_with_copy_detection() -> None: + async def run() -> None: + with tempfile.TemporaryDirectory(prefix="work_artifact_copy_") 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") + source = root / "my src.txt" + source.write_text("one\ntwo\nthree\n", encoding="utf-8") + _git(root, "add", ".") + _git(root, "commit", "-m", "baseline") + _git(root, "config", "diff.renames", "copies") + + store = WorkLedgerStore(Path(temp) / "ledger.sqlite3") + project = store.create_or_get_project(root) + item = store.create_work_item(project.project_id, title="Copy notes") + attempt = store.create_attempt(item.work_item_id, provider="codex", task="Copy notes") + registry = WorkArtifactRegistry(store) + await registry.capture_baseline(attempt, item) + (root / "my dup.txt").write_text("one\ntwo\nthree\n", encoding="utf-8") + source.write_text("one\ntwo\nthree\nfour\n", encoding="utf-8") + _git(root, "add", ".") + _git(root, "commit", "-m", "provider commit") + + 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 "copy to my dup.txt" in delta["committed_patch"] + parsed = parse_unified_diff( + delta["patch"], + changed_files=delta["changed_files"], + untracked=delta["untracked"], + ) + assert [ + (entry["path"], entry["oldPath"], entry["status"], entry["additions"]) + for entry in parsed["files"] + ] == [ + ("my dup.txt", "my src.txt", "copied", 0), + ("my src.txt", "my src.txt", "modified", 1), + ] + + 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: @@ -538,6 +585,7 @@ def _main() -> None: test_non_ascii_entry_revision_is_registered_as_the_real_path() 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() print("ok: work artifact registry preserves attempt Git boundaries") From 368bf7a0cf804272f905ded3a3169507fab0ed94 Mon Sep 17 00:00:00 2001 From: Lucas1479 Date: Wed, 4 Nov 2026 22:47:08 +1300 Subject: [PATCH 3/4] fix(work): preserve trailing blank context when joining patches --- server/work_artifact_registry.py | 2 +- tests/test_work_artifact_registry.py | 57 ++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) 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_work_artifact_registry.py b/tests/test_work_artifact_registry.py index 1b6cc31d..b6345f6d 100644 --- a/tests/test_work_artifact_registry.py +++ b/tests/test_work_artifact_registry.py @@ -572,6 +572,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() From 880d7496696122cb467bbe271fd6918e64d74479 Mon Sep 17 00:00:00 2001 From: Lucas1479 Date: Thu, 5 Nov 2026 00:45:28 +1300 Subject: [PATCH 4/4] ci: trigger PR checks after GitHub outage