fix(git): 🐛 Show ours blob as Old side in conflict diff view by jorben · Pull Request #113 · tiylabs/tiycode

jorben · 2026-04-23T04:26:07Z

Summary

Fix conflict diff displaying only New column (all-add lines) with empty Old column
Retrieve ours blob from Git index conflict entry as the Old side, diff against workdir content using similar crate
Graceful fallback to original all-new behavior for edge cases (delete-modify conflicts, binary files)

Test Plan

Open a repo with merge conflicts, click a conflicted file in Source Control
Verify Old column shows ours (pre-merge) content, New column shows workdir content with conflict markers
Verify binary conflict files and delete-modify conflicts fallback to all-new display without errors
cargo test passes (6/6)

🤖 Generated with TiyCode

Replace fallback "all-new content" display for conflicted files with proper side-by-side diff between: - Old side: "ours" blob from index conflict (previously unavailable) - New side: current workdir content (with conflict markers) Improves conflict resolution UX by showing actual changes instead of treating conflicted files as entirely new. Falls back to original behavior for binary files or index access failures. Uses `similar` v2.7.0 for line-based diff algorithm with standard context/remove/add hunks, respecting `MAX_DIFF_LINES` truncation. Includes Cargo.toml and Cargo.lock updates for the new dependency.

github-actions · 2026-04-23T04:29:11Z

AI Code Review Summary

PR: #113 (fix(git): 🐛 Show ours blob as Old side in conflict diff view)
Preferred language: English

Overall Assessment

Detected 3 actionable findings, prioritize CRITICAL/HIGH before merge.

Major Findings by Severity

HIGH (1)
- src-tauri/src/core/git_manager.rs:315 - No integration test for ours-blob extraction path in get_unmerged_diff
MEDIUM (2)
- src-tauri/src/core/git_manager.rs:1492 - Incorrect hunk header line counts when diff is truncated
- src-tauri/src/core/git_manager.rs:354 - No test for fallback path when ours blob extraction fails

Actionable Suggestions

Fix the hunk header computation in build_conflict_diff_hunks to use actual input text line counts rather than iteration counters, ensuring correctness when truncation occurs.
Add a unit test that checks hunk header counts against actual input line counts specifically in the truncation scenario.
Run 'cargo test --manifest-path src-tauri/Cargo.toml' and 'cargo fmt --manifest-path src-tauri/Cargo.toml' before committing, per repository guidelines.
Add an integration test in src-tauri/tests/ that creates a real merge conflict and exercises the full get_unmerged_diff code path, verifying that the ours blob is read and diffed correctly
Add integration tests for each fallback trigger: delete-modify conflict (no ours entry), binary ours blob, non-UTF-8 blob content, and missing workdir file
Add a tracing::warn! or tracing::debug! before the fallback at L354 to log when ours-blob extraction fails and why, improving production debuggability
Consider adding a unit test for mixed CRLF/LF input to build_conflict_diff_hunks to cover cross-platform line-ending scenarios

Potential Risks

Incorrect hunk headers for truncated conflict diffs could cause subtle UI bugs or break downstream consumers that parse the @@ header format.
If the ours-blob extraction silently fails in a common scenario not covered by tests, users will see degraded conflict diffs (all-new-content) without any indication of why
Binary blob check at L325 only checks the ours blob — if the workdir file is binary but the ours blob is not, the diff may produce garbled output for binary content
Large file handling: if a conflicted file is very large, reading both the ours blob and workdir content into memory simultaneously could cause memory pressure, but this is consistent with existing behavior

Test Suggestions

Add an integration test that creates a git repository with a merge conflict, calls get_unmerged_diff through the GitManager, and verifies the diff output contains correct old/new line counts in the hunk header.
Add a unit test for build_conflict_diff_hunks that explicitly verifies hunk header counts match the input text line counts when truncated is true.
Integration test: create repo, commit on branch A, commit different content on branch B, attempt merge, call get_unmerged_diff on conflict file, assert hunks contain Remove+Add lines
Integration test: create delete-modify conflict (delete on ours, modify on theirs), verify fallback produces all-Add hunks
Integration test: create conflict with binary ours blob, verify fallback is triggered and no panic occurs
Unit test: build_conflict_diff_hunks with old='line\n' and new='line\r\n' to verify mixed line-ending handling

File-Level Coverage Notes

src-tauri/src/core/git_manager.rs: partial_coverage (The unit tests for build_conflict_diff_hunks are thorough — covering identical, empty, mixed, line-number tracking, CRLF trimming, truncation, and header format. The gap is entirely in the integration layer that bridges git index operations to the diff function.)
src-tauri/Cargo.toml: ok (Adding the 'similar' crate as a dependency is appropriate for the text diffing functionality. No testing concerns with this change.)

Inline Downgraded Items (processed but not inline)

None

Coverage Status

Target files: 2
Covered files: 2
Uncovered files: 0
No-patch/binary covered as file-level: 0
Findings with unknown confidence (N/A): 0

Uncovered list:

None

No-patch covered list:

None

Runtime/Budget

Rounds used: 1/4
Planned batches: 1
Executed batches: 1
Sub-agent runs: 2
Planner calls: 1
Reviewer calls: 3
Model calls: 4/64
Structured-output summary-only degradation: NO

github-actions

Automated PR review completed.

Findings kept: 2
Findings with unknown confidence: 0
Inline comments attempted: 2
Target files: 2
Covered files: 2
Uncovered files: 0
See the summary comment for detailed analysis and coverage details.

Sign in to view


+/// Compare old (ours blob) text with new (workdir) text using `similar` and
+/// produce standard context/remove/add hunk data, respecting MAX_DIFF_LINES.
+fn build_conflict_diff_hunks(


Sign in to view

+            // Try to get the ours blob from the index conflict to use as Old side.
+            // If that fails (e.g. delete-modify conflict, non-UTF-8 blob), fall
+            // back to the previous all-new-content behaviour.
+            let ours_text = (|| -> Option<String> {


…F trim

github-actions

Automated PR review completed.

Findings kept: 3
Findings with unknown confidence: 0
Inline comments attempted: 3
Target files: 2
Covered files: 2
Uncovered files: 0
See the summary comment for detailed analysis and coverage details.

github-actions · 2026-04-23T05:02:05Z

+            // Try to get the ours blob from the index conflict to use as Old side.
+            // If that fails (e.g. delete-modify conflict, non-UTF-8 blob), fall
+            // back to the previous all-new-content behaviour.
+            let ours_text = (|| -> Option<String> {


[HIGH] No integration test for ours-blob extraction path in get_unmerged_diff

The new ours-blob extraction logic (L315-331) and the diff construction path (L336-352) are the primary feature addition, yet have no integration test. Existing milestone-style integration tests (src-tauri/tests/) should include a scenario that creates a merge conflict and verifies get_unmerged_diff returns proper old-vs-new hunks.

Suggestion: Add an integration test in src-tauri/tests/ that: (1) creates a repo with a conflicting merge, (2) calls get_unmerged_diff on the conflicted file, (3) asserts the returned hunks contain both Remove and Add lines (not all-Add fallback), and (4) verifies additions/deletions counts are correct.

Confidence: 0.92

[From SubAgent: testing]

github-actions · 2026-04-23T05:02:05Z

+        return (Vec::new(), 0, 0, false);
+    }
+
+    let old_total = old_line.saturating_sub(1);


[MEDIUM] Incorrect hunk header line counts when diff is truncated

When MAX_DIFF_LINES is hit and the loop breaks early, old_total and new_total are computed from the partial iteration counters rather than the actual line counts of the input texts, producing an inaccurate hunk header.

Suggestion: Compute old_total and new_total from the actual input texts (e.g., old_text.lines().count() and new_text.lines().count()) or from TextDiff::old_lines()/new_lines() counts, so the header reflects the true file sizes even when truncated.

Risk: Diff renderers or UI components that rely on the hunk header line counts will display incorrect information for large conflicted files that trigger truncation.

Confidence: 0.92

[From SubAgent: general]

github-actions · 2026-04-23T05:02:05Z

+                });
+            }

+            // Fallback: show workdir content as all-new (original behaviour).


[MEDIUM] No test for fallback path when ours blob extraction fails

Multiple conditions cause ours_text to be None, triggering the fallback to all-new-content behavior. None of these branches are tested, meaning regressions in fallback behavior would be invisible.

Suggestion: Add integration tests covering at least: (1) a delete-modify conflict where conflict.our is None, (2) a conflict with a binary ours blob, (3) a conflict where the workdir file is missing. Each should verify the fallback GitDiffDto has hunks from build_added_file_fallback_hunks with all Add lines.

Confidence: 0.88

[From SubAgent: testing]

github-actions Bot reviewed Apr 23, 2026

View reviewed changes

jorben added 2 commits April 23, 2026 12:34

test(git): ✅ add unit tests for build_conflict_diff_hunks and fix CRL…

07b7e07

…F trim

Merge remote-tracking branch 'origin/master' into fix/confict-diff-view

31d7e68

github-actions Bot reviewed Apr 23, 2026

View reviewed changes

jorben merged commit 6a51bc4 into master Apr 23, 2026
4 checks passed

jorben deleted the fix/confict-diff-view branch April 23, 2026 05:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(git): 🐛 Show ours blob as Old side in conflict diff view - #113

fix(git): 🐛 Show ours blob as Old side in conflict diff view#113
jorben merged 3 commits into
masterfrom
fix/confict-diff-view

jorben commented Apr 23, 2026

Uh oh!

github-actions Bot commented Apr 23, 2026 •

edited

Loading

Uh oh!

github-actions Bot left a comment

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

github-actions Bot left a comment

Uh oh!

github-actions Bot Apr 23, 2026

Uh oh!

github-actions Bot Apr 23, 2026

Uh oh!

github-actions Bot Apr 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

jorben commented Apr 23, 2026

Summary

Test Plan

Uh oh!

github-actions Bot commented Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

AI Code Review Summary

Overall Assessment

Major Findings by Severity

Actionable Suggestions

Potential Risks

Test Suggestions

File-Level Coverage Notes

Inline Downgraded Items (processed but not inline)

Coverage Status

Runtime/Budget

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

github-actions Bot commented Apr 23, 2026 •

edited

Loading