Skip to content

IJPL-248963 fix stale parent propagation in VcsLogJoiner - #3559

Closed
Sergej Salnikov (SergejSalnikov) wants to merge 5 commits into
JetBrains:masterfrom
SergejSalnikov:master
Closed

IJPL-248963 fix stale parent propagation in VcsLogJoiner#3559
Sergej Salnikov (SergejSalnikov) wants to merge 5 commits into
JetBrains:masterfrom
SergejSalnikov:master

Conversation

@SergejSalnikov

Copy link
Copy Markdown
Contributor

Root commits (no parents) in the first block were incorrectly kept in the unresolved-hashes set because the removal was guarded by !commit.getParents().isEmpty(). This caused getFirstUnTrackedIndex to scan the saved log looking for these root commit IDs, throwing VcsLogRefreshNotEnoughDataException when they were not found.

Additionally, getAllNewCommits was deduplicated against only the green-index range of the saved log rather than the full unsafe block size. When the red index exceeded the green index, commits in the gap were not checked, potentially producing duplicate entries in the merged result.

Fix both issues by:

  • unconditionally removing all first-block commit IDs from the unresolved set (via a collected firstBlockIds set),
  • computing new commits against savedLog[0..unsafeBlockSize) instead of savedLog[0..greenIndex), ensuring correct deduplication regardless of which index is larger.

https://youtrack.jetbrains.com/issue/IJPL-248963

@SergejSalnikov
Sergej Salnikov (SergejSalnikov) marked this pull request as draft July 7, 2026 14:26
Redesign addCommits to use a single-pass reconstruction with a
firstBlockMap that serves as lookup, replacement source, and
new-commit tracker simultaneously.

The root cause was that RedGreenSorter propagated stale parent lists
from savedLog for commits that also appeared in firstBlock with
updated parents. This caused removed commits to be incorrectly marked
as green and survive refresh, growing the log on every refresh.

Fix: pass a recentCommitParents map to RedGreenSorter so it propagates
updated parents from firstBlock instead of stale ones from savedLog.

Additionally, replace the fragile getSavedGreenIndex (parent-chain
tracing with root-commit special cases) with a direct overlap scan
that finds how deep firstBlock penetrates into savedLog. This handles
disconnected islands, root commits, and same-data refreshes correctly
without special cases.

Removed: getSavedGreenIndex, getFirstUnTrackedIndex, getAllNewCommits.
Net result: -52 lines, simpler logic.

https://youtrack.jetbrains.com/issue/IJPL-248963
Redesign addCommits to replace the fragile green-index parent-chain tracing
with a direct overlap scan. The old approach computed a "green index" by
tracing parent chains from firstBlock into savedLog, which had edge cases
with root commits and could miscalculate the scan depth — leading to
duplicate commits on every refresh.

The new approach uses a firstBlockMap that serves as lookup, replacement
source, and new-commit tracker simultaneously. A single pass over the
unsafe zone handles filtering, replacement, and new-commit detection.

Additionally fix two stale-data bugs in RedGreenSorter:
- Propagate updated parents from firstBlock instead of stale ones from
  savedLog, preventing removed commits from being incorrectly kept alive.
- Mark dropped parents (old parents no longer in the updated parent list)
  as red candidates, so commits that become unreachable due to parent
  changes are properly removed even when no ref changes occur.
@whitec10ud

Copy link
Copy Markdown
Collaborator

Thanks a lot for the contribution! We really appreciate you taking the time to investigate this and submit a fix.

The issue ([IJPL-248963](https://youtrack.jetbrains.com/issue/IJPL-248963/VcsLogJoiner-Stale-Parent-Propagation-Bug)) has been fixed on our side and the fix will be available in the next IDE release.

For now, we decided to go with a smaller, targeted fix for this specific issue rather than incorporating the broader refactorings from this PR. Nevertheless, thank you again for the contribution and for bringing this to our attention!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Fixed Fixed differently VCS Duty label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants