From 523a62e4dbbaf4611e2d651c25f2092a7d783be5 Mon Sep 17 00:00:00 2001 From: Naputt1 Date: Sat, 29 Aug 2026 14:36:24 +0700 Subject: [PATCH] fix(git-graph): keep passing for reused merge parent to close lane gap (#3222) Thanks for the crisp fix and regression test! Merging. --- .../src/components/views/git/gitGraph.test.ts | 64 +++++++++++++++++++ .../ui/src/components/views/git/gitGraph.ts | 22 +++++-- 2 files changed, 80 insertions(+), 6 deletions(-) diff --git a/packages/ui/src/components/views/git/gitGraph.test.ts b/packages/ui/src/components/views/git/gitGraph.test.ts index ccffe4a5..10b6b371 100644 --- a/packages/ui/src/components/views/git/gitGraph.test.ts +++ b/packages/ui/src/components/views/git/gitGraph.test.ts @@ -166,4 +166,68 @@ describe('assignLanes', () => { const bottomStub = cResult.connectors.find((c) => c.type === 'bottom-stub'); expect(bottomStub).toBeTruthy(); }); + + test('handles double merge of same branch with single commit between merges (screenshot case)', () => { + // Repro for screenshot: admin branch forked from base, 3 commits (48f6,c55f,2949), + // merged into main at 594c, then one more admin commit 3257 whose parent is + // the same 2949 as the merge's second parent (criss-cross), then merged again at a37. + // Order is topo-order as returned by `git log --all --topo-order` for that DAG. + const commits = [ + makeCommit('a37', ['594c', '3257']), + makeCommit('3257', ['2949']), + makeCommit('594c', ['base', '2949']), + makeCommit('2949', ['c55f']), + makeCommit('c55f', ['48f6']), + makeCommit('48f6', ['base']), + makeCommit('base', []), + ]; + const result = assignLanes(commits); + + // Should use only 2 lanes (main=0, admin=1) throughout – no lane jump to 2 + const maxLane = Math.max(...result.map((r) => r.lane)); + expect(maxLane).toBe(1); + + // The intermediate admin commit 3257 should be on admin lane + const c3257 = result.find((r) => r.commit.hash === '3257')!; + expect(c3257.lane).toBe(1); + + // Second merge (594c) must reuse admin lane rather than opening a new one, + // so its extra parent lane is 1 (reused) not a fresh lane. + const m1 = result.find((r) => r.commit.hash === '594c')!; + const m1BranchOut = m1.connectors.find((c) => c.type === 'branch-out')!; + expect(m1BranchOut.toLane).toBe(1); + + // Crucial: at the merge row, the reused admin lane must keep its vertical + // passing segment for continuity between 3257 above and 2949 below. + // Without this, a gap appears between those rows (the screenshot bug). + const m1Passing = m1.connectors.filter((c) => c.type === 'passing'); + expect(m1Passing.some((c) => c.fromLane === 1)).toBe(true); + + // Top merge also branch-out to admin lane + const m2 = result.find((r) => r.commit.hash === 'a37')!; + const m2BranchOut = m2.connectors.find((c) => c.type === 'branch-out')!; + expect(m2BranchOut.toLane).toBe(1); + // Top merge's admin lane is new, so no passing at that row (branch starts there) + expect(m2.connectors.some((c) => c.type === 'passing' && c.fromLane === 1)).toBe(false); + + // Base should merge both lanes cleanly + const base = result.find((r) => r.commit.hash === 'base')!; + const mergeIns = base.connectors.filter((c) => c.type === 'merge-in'); + expect(mergeIns.length).toBe(1); + }); + + test('reuses lane when merge second parent already active (no extra lane)', () => { + const commits = [ + makeCommit('m2', ['m1', 'a3']), + makeCommit('a3', ['common']), + makeCommit('m1', ['base', 'common']), + makeCommit('common', ['base']), + makeCommit('base', []), + ]; + const result = assignLanes(commits); + // m1 should reuse lane 1 (where a3 lives) rather than opening lane 2 + const m1 = result.find((r) => r.commit.hash === 'm1')!; + expect(m1.connectors.find((c) => c.type === 'branch-out')!.toLane).toBe(1); + expect(Math.max(...result.map((r) => r.lane))).toBe(1); + }); }); diff --git a/packages/ui/src/components/views/git/gitGraph.ts b/packages/ui/src/components/views/git/gitGraph.ts index 066ba868..a63aa330 100644 --- a/packages/ui/src/components/views/git/gitGraph.ts +++ b/packages/ui/src/components/views/git/gitGraph.ts @@ -101,6 +101,7 @@ export function assignLanes(commits: GitLogEntry[]): LanedCommit[] { // Open new lanes for additional parents (merge commits) const extraParentLanes: number[] = []; + const extraParentIsNew = new Set(); for (let p = 1; p < commit.parents.length; p++) { const parentHash = commit.parents[p]; // Check if another lane is already waiting for this parent @@ -109,10 +110,16 @@ export function assignLanes(commits: GitLogEntry[]): LanedCommit[] { extraParentLanes.push(existingLane); } else { const freeLane = activeLanes.indexOf(null); - const newLane = freeLane !== -1 ? freeLane : activeLanes.length; - activeLanes[newLane] = parentHash; - if (newLane === activeLanes.length) activeLanes.push(parentHash); - extraParentLanes.push(newLane); + if (freeLane !== -1) { + activeLanes[freeLane] = parentHash; + extraParentLanes.push(freeLane); + extraParentIsNew.add(freeLane); + } else { + const newLane = activeLanes.length; + activeLanes.push(parentHash); + extraParentLanes.push(newLane); + extraParentIsNew.add(newLane); + } } } @@ -151,11 +158,14 @@ export function assignLanes(commits: GitLogEntry[]): LanedCommit[] { }); } - // Passing-through lanes (active but not this commit's lane or extra parent lanes) + // Passing-through lanes (active but not this commit's lane or newly-opened extra parent lanes) + // Reused extra parents already have an active lane above the merge, so they must keep their + // vertical passing segment for continuity (otherwise a gap appears between + // the commit above and the merge row, as in the double-merge-of-same-branch case). for (let lane = 0; lane < activeLanes.length; lane++) { if (activeLanes[lane] === null) continue; if (lane === assignedLane) continue; - if (extraParentLanes.includes(lane)) continue; + if (extraParentIsNew.has(lane)) continue; connectors.push({ fromLane: lane, toLane: lane,