fix: detect conflicts in committed tip trees on Review tab - #228
Merged
Conversation
There was a problem hiding this comment.
Rebase conflicts live in committed hunks. Hiding Committed used to wipe those hunks and leave an empty Review pane even when Conflicts still listed the file.
- Always load
getWorkspaceDifffor workspaces - When Committed is off, retain only conflicted committed paths/hunks so the inline card still renders
treq/src/components/changes-diff-viewer/hooks/useFileLoading.ts
Lines 95 to 155 in e05b50f
There was a problem hiding this comment.
Pairwise tree-diffs miss conflicts when tip and target trees are identical (empty WC on a conflicted bookmark tip). Walking MergedTree::conflicts() reports those paths directly.
Lines 3918 to 3922 in e05b50f
- Keeps marker + diff fallbacks for materialization edge cases
Ziinc
commented
Aug 8, 2026
Walk MergedTree::conflicts() so conflicted paths are found even when a pairwise tip/target tree-diff is empty (clean WC on a conflicted tip). Keep conflicted committed files visible in Review when Committed is hidden. Co-authored-by: Ziinc <Ziinc@users.noreply.github.com>
Always load workspace diffs for conflicted paths, and stop clearing those hunks when the Committed toggle is off so Review can still show the card. Co-authored-by: Ziinc <Ziinc@users.noreply.github.com>
Drop pairwise parent/target tree-diff fallbacks and committed marker scans. Empty WC children inherit conflicted tip trees, so jj-lib's tree.conflicts() is sufficient for committed-tip conflicts. Co-authored-by: Ziinc <Ziinc@users.noreply.github.com>
jj-lib MergedTree::conflicts() is the only source of truth for unresolved conflicts. Scanning dirty WC file text for markers can false-positive on literal conflict-marker strings. Co-authored-by: Ziinc <Ziinc@users.noreply.github.com>
Default [] / new array identity each render recreated loadChangedFiles, retrigging the committed-toggle effect in a loop and blanking Review unit tests (empty body / afterEach hookTimeout). Co-authored-by: Ziinc <Ziinc@users.noreply.github.com>
cursor
Bot
force-pushed
the
cursor/review-committed-conflicts-af70
branch
from
August 9, 2026 11:20
2577979 to
cdfe583
Compare
Ziinc
marked this pull request as ready for review
August 9, 2026 11:22
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MergedTree::conflicts()only (WC tip + unsaved snapshot). No file-text marker scans — those false-positive on literal<<<<<<<content.has_conflictswhen either the WC tree is unresolved orconflicted_filesis non-empty.App QA
Before: Conflicts + Committed both visible
After: Committed hidden, conflict card still shown
Test plan
test_detects_conflicts_in_committed_tip_when_tree_diff_is_empty+core_changes_testreview-committed-conflict-hidden.spec.tsxnpm run format && npm run lint && npm run checkTo show artifacts inline, enable in settings.