Skip to content

Commit 0057860

Browse files
SingTheCodeSingTheCode
authored
Merge pull request #914 from githru/feature/#870_delete-csm-reverse
feat(engine): CSM Squash Merge 로직 개선
2 parents e9f9038 + f672504 commit 0057860

4 files changed

Lines changed: 330 additions & 11 deletions

File tree

‎packages/analysis-engine/src/csm.spec.ts‎

Lines changed: 287 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,16 @@ describe("csm", () => {
6060
return dict;
6161
}, new Map<string, CommitNode>());
6262

63+
// Set mergedIntoStem flags based on merge structure
64+
// Node 8 (parents=[7, 13]) is merged into master at node 2
65+
// Node 11 (parents=[10, 16]) is merged into master at node 3
66+
fakeCommitNodeDict.get("8")!.mergedIntoBaseStem = "master";
67+
fakeCommitNodeDict.get("11")!.mergedIntoBaseStem = "master";
68+
// Node 13 is merged into sub1 at node 8
69+
fakeCommitNodeDict.get("13")!.mergedIntoBaseStem = "sub1";
70+
// Node 16 is merged into sub1 at node 11
71+
fakeCommitNodeDict.get("16")!.mergedIntoBaseStem = "sub1";
72+
6373
describe("buildCSM", () => {
6474
let csmDict: CSMDictionary;
6575

@@ -127,8 +137,28 @@ describe("csm", () => {
127137
makeFakeStemTuple("sub2", [12, 13, 14, 15, 16].reverse().map(String)),
128138
]);
129139

140+
const fakeCommitNodeDictWithSub1: Map<string, CommitNode> = Array.from(fakeStemDictWithSub1.entries()).reduce(
141+
(dict, [, stem]) => {
142+
stem.nodes.forEach((commitNode) => {
143+
dict.set(commitNode.commit.id, commitNode);
144+
});
145+
return dict;
146+
},
147+
new Map<string, CommitNode>()
148+
);
149+
150+
// Set mergedIntoStem flags for sub1 as base branch
151+
// Node 8 (parents=[7, 13]) is merged into sub1 at itself
152+
// Node 11 (parents=[10, 16]) is merged into sub1 at itself
153+
fakeCommitNodeDictWithSub1.get("8")!.mergedIntoBaseStem = "sub1";
154+
fakeCommitNodeDictWithSub1.get("11")!.mergedIntoBaseStem = "sub1";
155+
// Node 13 is merged into sub1 at node 8
156+
fakeCommitNodeDictWithSub1.get("13")!.mergedIntoBaseStem = "sub1";
157+
// Node 16 is merged into sub1 at node 11
158+
fakeCommitNodeDictWithSub1.get("16")!.mergedIntoBaseStem = "sub1";
159+
130160
beforeAll(() => {
131-
csmDict = buildCSMDict(fakeCommitNodeDict, fakeStemDictWithSub1, "sub1");
161+
csmDict = buildCSMDict(fakeCommitNodeDictWithSub1, fakeStemDictWithSub1, "sub1");
132162
});
133163

134164
it("has squash-commits", () => {
@@ -149,4 +179,260 @@ describe("csm", () => {
149179
});
150180
});
151181
});
182+
183+
describe("octopus merge (multiple parents)", () => {
184+
// Test scenario: 3-way merge (octopus merge)
185+
// master = [0, 1, 100]
186+
// branch1 = [2, 3]
187+
// branch2 = [4, 5]
188+
// branch3 = [6, 7]
189+
// 100 = merge(1, 3, 5, 7) - 4 parents octopus merge
190+
191+
const octopusCommitDict: Map<string, CommitRaw> = new Map<
192+
string,
193+
Pick<CommitRaw, "id" | "parents" | "branches" | "sequence">
194+
>([
195+
["0", { id: "0", parents: [], branches: [], sequence: 9 }],
196+
["1", { id: "1", parents: ["0"], branches: [], sequence: 8 }],
197+
["2", { id: "2", parents: ["1"], branches: [], sequence: 7 }],
198+
["3", { id: "3", parents: ["2"], branches: ["branch1"], sequence: 6 }],
199+
["4", { id: "4", parents: ["1"], branches: [], sequence: 5 }],
200+
["5", { id: "5", parents: ["4"], branches: ["branch2"], sequence: 4 }],
201+
["6", { id: "6", parents: ["1"], branches: [], sequence: 3 }],
202+
["7", { id: "7", parents: ["6"], branches: ["branch3"], sequence: 2 }],
203+
["100", { id: "100", parents: ["1", "3", "5", "7"], branches: ["master"], sequence: 0 }],
204+
]) as Map<string, CommitRaw>;
205+
206+
const octopusStemDict: Map<string, Stem> = new Map([
207+
[
208+
"master",
209+
{
210+
nodes: ["100", "1", "0"]
211+
.map((id) => octopusCommitDict.get(id))
212+
.filter((commit): commit is CommitRaw => Boolean(commit))
213+
.map((commit) => ({ stemId: "master", commit })),
214+
},
215+
],
216+
[
217+
"branch1",
218+
{
219+
nodes: ["3", "2"]
220+
.map((id) => octopusCommitDict.get(id))
221+
.filter((commit): commit is CommitRaw => Boolean(commit))
222+
.map((commit) => ({ stemId: "branch1", commit })),
223+
},
224+
],
225+
[
226+
"branch2",
227+
{
228+
nodes: ["5", "4"]
229+
.map((id) => octopusCommitDict.get(id))
230+
.filter((commit): commit is CommitRaw => Boolean(commit))
231+
.map((commit) => ({ stemId: "branch2", commit })),
232+
},
233+
],
234+
[
235+
"branch3",
236+
{
237+
nodes: ["7", "6"]
238+
.map((id) => octopusCommitDict.get(id))
239+
.filter((commit): commit is CommitRaw => Boolean(commit))
240+
.map((commit) => ({ stemId: "branch3", commit })),
241+
},
242+
],
243+
]);
244+
245+
const octopusCommitNodeDict: Map<string, CommitNode> = Array.from(octopusStemDict.entries()).reduce(
246+
(dict, [, stem]) => {
247+
stem.nodes.forEach((commitNode) => {
248+
dict.set(commitNode.commit.id, commitNode);
249+
});
250+
return dict;
251+
},
252+
new Map<string, CommitNode>()
253+
);
254+
255+
// Set mergedIntoStem flags
256+
octopusCommitNodeDict.get("3")!.mergedIntoBaseStem = "master";
257+
octopusCommitNodeDict.get("5")!.mergedIntoBaseStem = "master";
258+
octopusCommitNodeDict.get("7")!.mergedIntoBaseStem = "master";
259+
260+
it("should handle octopus merge with 4 parents", () => {
261+
const csmDict = buildCSMDict(octopusCommitNodeDict, octopusStemDict, "master");
262+
263+
expect(csmDict.master).toBeDefined();
264+
expect(csmDict.master.length).toBe(3); // 100, 1, 0
265+
266+
const octopusMergeNode = csmDict.master.find((node) => node.base.commit.id === "100");
267+
expect(octopusMergeNode).toBeDefined();
268+
expect(octopusMergeNode!.source.length).toBeGreaterThan(0);
269+
270+
// Should include all commits from all 3 branches: 3,2 + 5,4 + 7,6
271+
const sourceIds = octopusMergeNode!.source.map((node) => node.commit.id);
272+
expect(sourceIds).toContain("3");
273+
expect(sourceIds).toContain("2");
274+
expect(sourceIds).toContain("5");
275+
expect(sourceIds).toContain("4");
276+
expect(sourceIds).toContain("7");
277+
expect(sourceIds).toContain("6");
278+
279+
// Should be sorted by sequence
280+
const sequences = octopusMergeNode!.source.map((node) => node.commit.sequence);
281+
const sortedSequences = [...sequences].sort((a, b) => a - b);
282+
expect(sequences).toEqual(sortedSequences);
283+
});
284+
});
285+
286+
describe("single parent commits (early return)", () => {
287+
const singleParentCommitDict: Map<string, CommitRaw> = new Map<
288+
string,
289+
Pick<CommitRaw, "id" | "parents" | "branches" | "sequence">
290+
>([
291+
["initial", { id: "initial", parents: [], branches: [], sequence: 2 }],
292+
["normal", { id: "normal", parents: ["initial"], branches: [], sequence: 1 }],
293+
["latest", { id: "latest", parents: ["normal"], branches: ["master"], sequence: 0 }],
294+
]) as Map<string, CommitRaw>;
295+
296+
const singleParentStemDict: Map<string, Stem> = new Map([
297+
[
298+
"master",
299+
{
300+
nodes: ["latest", "normal", "initial"]
301+
.map((id) => singleParentCommitDict.get(id))
302+
.filter((commit): commit is CommitRaw => Boolean(commit))
303+
.map((commit) => ({ stemId: "master", commit })),
304+
},
305+
],
306+
]);
307+
308+
const singleParentCommitNodeDict: Map<string, CommitNode> = Array.from(singleParentStemDict.entries()).reduce(
309+
(dict, [, stem]) => {
310+
stem.nodes.forEach((commitNode) => {
311+
dict.set(commitNode.commit.id, commitNode);
312+
});
313+
return dict;
314+
},
315+
new Map<string, CommitNode>()
316+
);
317+
318+
it("should return empty source for initial commit (0 parents)", () => {
319+
const csmDict = buildCSMDict(singleParentCommitNodeDict, singleParentStemDict, "master");
320+
321+
const initialNode = csmDict.master.find((node) => node.base.commit.id === "initial");
322+
expect(initialNode).toBeDefined();
323+
expect(initialNode!.source).toEqual([]);
324+
});
325+
326+
it("should return empty source for normal commit (1 parent)", () => {
327+
const csmDict = buildCSMDict(singleParentCommitNodeDict, singleParentStemDict, "master");
328+
329+
const normalNode = csmDict.master.find((node) => node.base.commit.id === "normal");
330+
expect(normalNode).toBeDefined();
331+
expect(normalNode!.source).toEqual([]);
332+
333+
const latestNode = csmDict.master.find((node) => node.base.commit.id === "latest");
334+
expect(latestNode).toBeDefined();
335+
expect(latestNode!.source).toEqual([]);
336+
});
337+
});
338+
339+
describe("edge case: non-existent parent commits", () => {
340+
const edgeCaseCommitDict: Map<string, CommitRaw> = new Map<
341+
string,
342+
Pick<CommitRaw, "id" | "parents" | "branches" | "sequence">
343+
>([
344+
["0", { id: "0", parents: [], branches: [], sequence: 4 }],
345+
["1", { id: "1", parents: ["0"], branches: [], sequence: 3 }],
346+
["2", { id: "2", parents: ["1"], branches: ["branch1"], sequence: 2 }],
347+
// "phantom" is not in commitDict
348+
["merge", { id: "merge", parents: ["1", "2", "phantom"], branches: ["master"], sequence: 0 }],
349+
]) as Map<string, CommitRaw>;
350+
351+
const edgeCaseStemDict: Map<string, Stem> = new Map([
352+
[
353+
"master",
354+
{
355+
nodes: ["merge", "1", "0"]
356+
.map((id) => edgeCaseCommitDict.get(id))
357+
.filter((commit): commit is CommitRaw => Boolean(commit))
358+
.map((commit) => ({ stemId: "master", commit })),
359+
},
360+
],
361+
[
362+
"branch1",
363+
{
364+
nodes: ["2"]
365+
.map((id) => edgeCaseCommitDict.get(id))
366+
.filter((commit): commit is CommitRaw => Boolean(commit))
367+
.map((commit) => ({ stemId: "branch1", commit })),
368+
},
369+
],
370+
]);
371+
372+
const edgeCaseCommitNodeDict: Map<string, CommitNode> = Array.from(edgeCaseStemDict.entries()).reduce(
373+
(dict, [, stem]) => {
374+
stem.nodes.forEach((commitNode) => {
375+
dict.set(commitNode.commit.id, commitNode);
376+
});
377+
return dict;
378+
},
379+
new Map<string, CommitNode>()
380+
);
381+
382+
edgeCaseCommitNodeDict.get("2")!.mergedIntoBaseStem = "master";
383+
384+
it("should filter out non-existent parent commits", () => {
385+
const csmDict = buildCSMDict(edgeCaseCommitNodeDict, edgeCaseStemDict, "master");
386+
387+
const mergeNode = csmDict.master.find((node) => node.base.commit.id === "merge");
388+
expect(mergeNode).toBeDefined();
389+
390+
// Should only include commit "2", not "phantom"
391+
const sourceIds = mergeNode!.source.map((node) => node.commit.id);
392+
expect(sourceIds).toContain("2");
393+
expect(sourceIds).not.toContain("phantom");
394+
});
395+
});
396+
397+
describe("edge case: all parents are invalid", () => {
398+
const allInvalidCommitDict: Map<string, CommitRaw> = new Map<
399+
string,
400+
Pick<CommitRaw, "id" | "parents" | "branches" | "sequence">
401+
>([
402+
["0", { id: "0", parents: [], branches: [], sequence: 2 }],
403+
["1", { id: "1", parents: ["0"], branches: [], sequence: 1 }],
404+
// All merge parents are phantoms
405+
["merge", { id: "merge", parents: ["1", "phantom1", "phantom2"], branches: ["master"], sequence: 0 }],
406+
]) as Map<string, CommitRaw>;
407+
408+
const allInvalidStemDict: Map<string, Stem> = new Map([
409+
[
410+
"master",
411+
{
412+
nodes: ["merge", "1", "0"]
413+
.map((id) => allInvalidCommitDict.get(id))
414+
.filter((commit): commit is CommitRaw => Boolean(commit))
415+
.map((commit) => ({ stemId: "master", commit })),
416+
},
417+
],
418+
]);
419+
420+
const allInvalidCommitNodeDict: Map<string, CommitNode> = Array.from(allInvalidStemDict.entries()).reduce(
421+
(dict, [, stem]) => {
422+
stem.nodes.forEach((commitNode) => {
423+
dict.set(commitNode.commit.id, commitNode);
424+
});
425+
return dict;
426+
},
427+
new Map<string, CommitNode>()
428+
);
429+
430+
it("should return empty source when all merge parents are invalid", () => {
431+
const csmDict = buildCSMDict(allInvalidCommitNodeDict, allInvalidStemDict, "master");
432+
433+
const mergeNode = csmDict.master.find((node) => node.base.commit.id === "merge");
434+
expect(mergeNode).toBeDefined();
435+
expect(mergeNode!.source).toEqual([]);
436+
});
437+
});
152438
});

‎packages/analysis-engine/src/csm.ts‎

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,19 @@ import { convertPRCommitsToCommitNodes, convertPRDetailToCommitRaw } from "./pul
22
import type { CommitDict, CommitNode, CSMDictionary, CSMNode, PullRequest, PullRequestDict, StemDict } from "./types";
33

44
const buildCSMNode = (baseCommitNode: CommitNode, commitDict: CommitDict, stemDict: StemDict): CSMNode => {
5-
const mergeParentCommit = commitDict.get(baseCommitNode.commit.parents[1]);
6-
if (!mergeParentCommit) {
5+
if (baseCommitNode.commit.parents.length <= 1) {
6+
return {
7+
base: baseCommitNode,
8+
source: [],
9+
};
10+
}
11+
12+
const mergeParentCommits = baseCommitNode.commit.parents
13+
.slice(1)
14+
.map((parentId) => commitDict.get(parentId))
15+
.filter((commit): commit is CommitNode => !!commit);
16+
17+
if (mergeParentCommits.length === 0) {
718
return {
819
base: baseCommitNode,
920
source: [],
@@ -12,7 +23,7 @@ const buildCSMNode = (baseCommitNode: CommitNode, commitDict: CommitDict, stemDi
1223

1324
const squashCommitNodes: CommitNode[] = [];
1425

15-
const squashTaskQueue: CommitNode[] = [mergeParentCommit];
26+
const squashTaskQueue: CommitNode[] = [...mergeParentCommits];
1627
while (squashTaskQueue.length > 0) {
1728
// get target
1829
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
@@ -26,13 +37,32 @@ const buildCSMNode = (baseCommitNode: CommitNode, commitDict: CommitDict, stemDi
2637
continue;
2738
}
2839

29-
// prepare squash
30-
const squashStemLastIndex = squashStem.nodes.length - 1;
40+
// find squashStartNode index in stem
3141
const squashStartNodeIndex = squashStem.nodes.findIndex(({ commit: { id } }) => id === squashStartNode.commit.id);
32-
const spliceCount = squashStemLastIndex - squashStartNodeIndex + 1;
42+
if (squashStartNodeIndex === -1) {
43+
continue;
44+
}
45+
46+
// collect nodes from squashStartNode to end of stem
47+
// (or until the next mergedIntoStem node for future merges)
48+
const spliceCommitNodes: CommitNode[] = [];
49+
50+
// First, find the end index (before next mergedIntoStem or end of stem)
51+
let endIndex = squashStem.nodes.length - 1;
52+
for (let i = squashStartNodeIndex + 1; i < squashStem.nodes.length; i++) {
53+
if (squashStem.nodes[i].mergedIntoBaseStem) {
54+
endIndex = i - 1;
55+
break;
56+
}
57+
}
58+
59+
// Collect nodes from start to end
60+
for (let i = squashStartNodeIndex; i <= endIndex; i++) {
61+
spliceCommitNodes.push(squashStem.nodes[i]);
62+
}
3363

34-
// squash
35-
const spliceCommitNodes = squashStem.nodes.splice(squashStartNodeIndex, spliceCount);
64+
// remove collected nodes from stem
65+
squashStem.nodes.splice(squashStartNodeIndex, spliceCommitNodes.length);
3666
squashCommitNodes.push(...spliceCommitNodes);
3767

3868
// check nested-merge
@@ -48,7 +78,7 @@ const buildCSMNode = (baseCommitNode: CommitNode, commitDict: CommitDict, stemDi
4878
squashTaskQueue.push(...nestedMergeParentCommits);
4979
}
5080

51-
squashCommitNodes.sort((a, b) => b.commit.sequence - a.commit.sequence);
81+
squashCommitNodes.sort((a, b) => a.commit.sequence - b.commit.sequence);
5282

5383
return {
5484
base: baseCommitNode,

‎packages/analysis-engine/src/stem.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ export function getStemNodes(
1919
if (idx === 0) return;
2020
const parentNode = commitDict.get(parent);
2121
if (parentNode) {
22+
parentNode.mergedIntoBaseStem = stemId;
2223
q.push(parentNode);
2324
}
2425
}, q);

0 commit comments

Comments
 (0)