Skip to content

Commit b1ab100

Browse files
singcosingco
authored andcommitted
Merge branch 'core/CSMDict' into refactor/csm-improvements
2 parents c32a522 + 0057860 commit b1ab100

5 files changed

Lines changed: 277 additions & 17 deletions

File tree

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

Lines changed: 0 additions & 2 deletions
This file was deleted.

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

Lines changed: 264 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,12 @@ describe("csm", () => {
6363
// Set mergedIntoStem flags based on merge structure
6464
// Node 8 (parents=[7, 13]) is merged into master at node 2
6565
// Node 11 (parents=[10, 16]) is merged into master at node 3
66-
fakeCommitNodeDict.get("8")!.mergedIntoStem = "master";
67-
fakeCommitNodeDict.get("11")!.mergedIntoStem = "master";
66+
fakeCommitNodeDict.get("8")!.mergedIntoBaseStem = "master";
67+
fakeCommitNodeDict.get("11")!.mergedIntoBaseStem = "master";
6868
// Node 13 is merged into sub1 at node 8
69-
fakeCommitNodeDict.get("13")!.mergedIntoStem = "sub1";
69+
fakeCommitNodeDict.get("13")!.mergedIntoBaseStem = "sub1";
7070
// Node 16 is merged into sub1 at node 11
71-
fakeCommitNodeDict.get("16")!.mergedIntoStem = "sub1";
71+
fakeCommitNodeDict.get("16")!.mergedIntoBaseStem = "sub1";
7272

7373
describe("buildCSM", () => {
7474
let csmDict: CSMDictionary;
@@ -150,12 +150,12 @@ describe("csm", () => {
150150
// Set mergedIntoStem flags for sub1 as base branch
151151
// Node 8 (parents=[7, 13]) is merged into sub1 at itself
152152
// Node 11 (parents=[10, 16]) is merged into sub1 at itself
153-
fakeCommitNodeDictWithSub1.get("8")!.mergedIntoStem = "sub1";
154-
fakeCommitNodeDictWithSub1.get("11")!.mergedIntoStem = "sub1";
153+
fakeCommitNodeDictWithSub1.get("8")!.mergedIntoBaseStem = "sub1";
154+
fakeCommitNodeDictWithSub1.get("11")!.mergedIntoBaseStem = "sub1";
155155
// Node 13 is merged into sub1 at node 8
156-
fakeCommitNodeDictWithSub1.get("13")!.mergedIntoStem = "sub1";
156+
fakeCommitNodeDictWithSub1.get("13")!.mergedIntoBaseStem = "sub1";
157157
// Node 16 is merged into sub1 at node 11
158-
fakeCommitNodeDictWithSub1.get("16")!.mergedIntoStem = "sub1";
158+
fakeCommitNodeDictWithSub1.get("16")!.mergedIntoBaseStem = "sub1";
159159

160160
beforeAll(() => {
161161
csmDict = buildCSMDict(fakeCommitNodeDictWithSub1, fakeStemDictWithSub1, "sub1");
@@ -179,4 +179,260 @@ describe("csm", () => {
179179
});
180180
});
181181
});
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+
});
182438
});

packages/analysis-engine/src/csm.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import {
33
extractNestedMergeParents,
44
findSquashEndIndex,
55
findSquashStartNodeIndex,
6-
getFirstParentCommit,
6+
getParentCommits,
77
} from "./csm.util";
88
import { convertPRCommitsToCommitNodes, convertPRDetailToCommitRaw } from "./pullRequest";
99
import type { CommitDict, CommitNode, CSMDictionary, CSMNode, PullRequest, PullRequestDict, StemDict } from "./types";
@@ -13,19 +13,25 @@ import type { CommitDict, CommitNode, CSMDictionary, CSMNode, PullRequest, PullR
1313
* For merge commits, collects squashed commits using DFS traversal.
1414
*/
1515
const buildCSMNode = (baseCommitNode: CommitNode, commitDict: CommitDict, stemDict: StemDict): CSMNode => {
16+
if (baseCommitNode.commit.parents.length <= 1) {
17+
return {
18+
base: baseCommitNode,
19+
source: [],
20+
};
21+
}
22+
1623
// Return empty source for non-merge commits
17-
const mergeParentCommit = getFirstParentCommit(baseCommitNode, commitDict);
18-
if (!mergeParentCommit) {
24+
const mergeParentCommits = getParentCommits(baseCommitNode, commitDict);
25+
if (mergeParentCommits.length === 0) {
1926
return {
2027
base: baseCommitNode,
2128
source: [],
2229
};
2330
}
2431

2532
const squashCommitNodes: CommitNode[] = [];
26-
const squashTaskQueue: CommitNode[] = [mergeParentCommit];
2733

28-
// Collect commits to be squashed using DFS
34+
const squashTaskQueue: CommitNode[] = [...mergeParentCommits];
2935
while (squashTaskQueue.length > 0) {
3036
const squashStartNode = squashTaskQueue.shift();
3137
if (!squashStartNode?.stemId) {

packages/analysis-engine/src/stem.ts

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

packages/analysis-engine/src/types/CommitNode.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ export interface CommitNode {
44
// stemId does not exist before traversal.
55
stemId?: string;
66
// ID of the stem this node was merged into (for merge parents)
7-
mergedIntoStem?: string;
7+
mergedIntoBaseStem?: string;
88
commit: CommitRaw;
99
}
1010

0 commit comments

Comments
 (0)