Skip to content

Commit 6f460a9

Browse files
authored
refactor(leiden): extract makePartition's inline getters to cut cyclomatic complexity
refactor(leiden): extract makePartition's inline getters to cut cyclomatic complexity makePartition (cyc=10, threshold 10) built its returned Partition object with several getter closures containing their own inline bounds-check ternaries and `|| 0` fallbacks. Extracted fgetOrZero/igetOrZero to typed-array-helpers.ts and a named buildCommunityMembers function, matching the file's existing extraction style. Greptile gave this a 5/5 "safe to merge" review with no actionable comments. The Pre-publish benchmark gate failed once (the routine flaky "1-file rebuild" timing gate seen on prior PRs) and passed clean on rerun with no code changes.
1 parent 68949dc commit 6f460a9

4 files changed

Lines changed: 230 additions & 14 deletions

File tree

src/graph/algorithms/leiden/partition.ts

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99

1010
import type { GraphAdapter } from './adapter.js';
1111
import { accumulateInternalEdgeWeights, accumulateNodeAggregates } from './aggregate-helpers.js';
12-
import { fget, iget, u8get } from './typed-array-helpers.js';
12+
import { fget, fgetOrZero, iget, igetOrZero, u8get } from './typed-array-helpers.js';
1313

1414
export interface CompactOptions {
1515
keepOldOrder?: boolean;
@@ -367,6 +367,22 @@ function compactIds(s: PartitionState, opts: CompactOptions = {}): void {
367367
s.communityTotalInStrength = newTotalInStrength;
368368
}
369369

370+
/* ------------------------------------------------------------------ */
371+
/* Extracted: community membership listing */
372+
/* ------------------------------------------------------------------ */
373+
374+
/** Group node indices [0, n) by their current community id. */
375+
function buildCommunityMembers(
376+
s: PartitionState,
377+
n: number,
378+
nodeCommunity: Int32Array,
379+
): number[][] {
380+
const comms: number[][] = new Array(s.communityCount);
381+
for (let i = 0; i < s.communityCount; i++) comms[i] = [];
382+
for (let i = 0; i < n; i++) comms[iget(nodeCommunity, i)]!.push(i);
383+
return comms;
384+
}
385+
370386
/* ------------------------------------------------------------------ */
371387
/* Factory: thin wrapper that wires state to extracted functions */
372388
/* ------------------------------------------------------------------ */
@@ -435,21 +451,14 @@ export function makePartition(graph: GraphAdapter, options: MakePartitionOptions
435451
accumulateNeighborCommunityEdgeWeights: (v: number) => accumulateNeighborWeights(s, v),
436452
getCandidateCommunityAt: (i: number): number => iget(s.candidateCommunities, i),
437453
getNeighborEdgeWeightToCommunity: (c: number): number =>
438-
fget(s.neighborEdgeWeightToCommunity, c) || 0,
439-
getOutEdgeWeightToCommunity: (c: number): number => fget(s.outEdgeWeightToCommunity, c) || 0,
440-
getInEdgeWeightFromCommunity: (c: number): number => fget(s.inEdgeWeightFromCommunity, c) || 0,
454+
fgetOrZero(s.neighborEdgeWeightToCommunity, c),
455+
getOutEdgeWeightToCommunity: (c: number): number => fgetOrZero(s.outEdgeWeightToCommunity, c),
456+
getInEdgeWeightFromCommunity: (c: number): number => fgetOrZero(s.inEdgeWeightFromCommunity, c),
441457
moveNodeToCommunity: (v: number, newC: number) => moveNode(s, v, newC),
442458
compactCommunityIds: (opts?: CompactOptions) => compactIds(s, opts),
443-
getCommunityMembers(): number[][] {
444-
const comms: number[][] = new Array(s.communityCount);
445-
for (let i = 0; i < s.communityCount; i++) comms[i] = [];
446-
for (let i = 0; i < n; i++) comms[iget(nodeCommunity, i)]!.push(i);
447-
return comms;
448-
},
449-
getCommunityTotalSize: (c: number): number =>
450-
c < s.communityTotalSize.length ? fget(s.communityTotalSize, c) : 0,
451-
getCommunityNodeCount: (c: number): number =>
452-
c < s.communityNodeCount.length ? iget(s.communityNodeCount, c) : 0,
459+
getCommunityMembers: () => buildCommunityMembers(s, n, nodeCommunity),
460+
getCommunityTotalSize: (c: number): number => fgetOrZero(s.communityTotalSize, c),
461+
getCommunityNodeCount: (c: number): number => igetOrZero(s.communityNodeCount, c),
453462
graph: undefined,
454463
};
455464
}

src/graph/algorithms/leiden/typed-array-helpers.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,20 @@ export function u8get(a: Uint8Array, i: number): number {
2626
export function taAdd(a: Float64Array, i: number, v: number): void {
2727
a[i] = fget(a, i) + v;
2828
}
29+
30+
/**
31+
* Bounds-safe zero-fallback read: collapses both an out-of-range index and a
32+
* falsy (0/NaN) in-range value to `0`. Typed arrays already return `undefined`
33+
* for an out-of-range index, and `undefined || 0` lands on the same `0` as a
34+
* falsy in-range read, so this single helper replaces the two hand-rolled
35+
* guards previously duplicated at call sites: `i < a.length ? fget(a, i) : 0`
36+
* and `fget(a, i) || 0`.
37+
*/
38+
export function fgetOrZero(a: Float64Array, i: number): number {
39+
return (a[i] as number) || 0;
40+
}
41+
42+
/** `fgetOrZero`, for `Int32Array`. */
43+
export function igetOrZero(a: Int32Array, i: number): number {
44+
return (a[i] as number) || 0;
45+
}
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
import { describe, expect, it } from 'vitest';
2+
import type { GraphAdapter } from '../../../src/graph/algorithms/leiden/adapter.js';
3+
import { makePartition } from '../../../src/graph/algorithms/leiden/partition.js';
4+
5+
/**
6+
* Minimal 3-node undirected adapter: 0-1 (w=4), 1-2 (w=6), no self-loops.
7+
* Hand-built (not via makeGraphAdapter) to keep this a focused unit test of
8+
* partition.ts's own bookkeeping, independent of adapter.ts.
9+
*/
10+
function makeTestAdapter(): GraphAdapter {
11+
const outEdges: GraphAdapter['outEdges'] = [
12+
[{ to: 1, w: 4 }],
13+
[
14+
{ to: 0, w: 4 },
15+
{ to: 2, w: 6 },
16+
],
17+
[{ to: 1, w: 6 }],
18+
];
19+
const inEdges: GraphAdapter['inEdges'] = [
20+
[{ from: 1, w: 4 }],
21+
[
22+
{ from: 0, w: 4 },
23+
{ from: 2, w: 6 },
24+
],
25+
[{ from: 1, w: 6 }],
26+
];
27+
return {
28+
n: 3,
29+
nodeIds: ['0', '1', '2'],
30+
idToIndex: new Map([
31+
['0', 0],
32+
['1', 1],
33+
['2', 2],
34+
]),
35+
size: new Float64Array([1, 1, 1]),
36+
selfLoop: new Float64Array(3),
37+
strengthOut: new Float64Array([4, 10, 6]),
38+
strengthIn: new Float64Array([4, 10, 6]),
39+
outEdges,
40+
inEdges,
41+
directed: false,
42+
totalWeight: 20,
43+
forEachNeighbor: (i, cb) => {
44+
for (const e of outEdges[i] as { to: number; w: number }[]) cb(e.to, e.w);
45+
},
46+
};
47+
}
48+
49+
describe('makePartition', () => {
50+
it('starts with each node in its own singleton community', () => {
51+
const p = makePartition(makeTestAdapter());
52+
p.initializeAggregates();
53+
expect(p.communityCount).toBe(3);
54+
expect(Array.from(p.nodeCommunity)).toEqual([0, 1, 2]);
55+
expect(p.getCommunityTotalSize(0)).toBe(1);
56+
expect(p.getCommunityTotalSize(1)).toBe(1);
57+
expect(p.getCommunityTotalSize(2)).toBe(1);
58+
expect(p.getCommunityNodeCount(0)).toBe(1);
59+
});
60+
61+
it('getCommunityTotalSize/getCommunityNodeCount return 0 for an out-of-range id', () => {
62+
const p = makePartition(makeTestAdapter());
63+
p.initializeAggregates();
64+
expect(p.getCommunityTotalSize(99)).toBe(0);
65+
expect(p.getCommunityNodeCount(99)).toBe(0);
66+
expect(p.getCommunityTotalSize(-1)).toBe(0);
67+
expect(p.getCommunityNodeCount(-1)).toBe(0);
68+
});
69+
70+
it('moveNodeToCommunity updates size/count for old and new communities', () => {
71+
const p = makePartition(makeTestAdapter());
72+
p.initializeAggregates();
73+
const moved = p.moveNodeToCommunity(0, 1);
74+
expect(moved).toBe(true);
75+
expect(p.getCommunityTotalSize(0)).toBe(0);
76+
expect(p.getCommunityNodeCount(0)).toBe(0);
77+
expect(p.getCommunityTotalSize(1)).toBe(2);
78+
expect(p.getCommunityNodeCount(1)).toBe(2);
79+
expect(Array.from(p.nodeCommunity)).toEqual([1, 1, 2]);
80+
});
81+
82+
it('moveNodeToCommunity is a no-op when the node is already in that community', () => {
83+
const p = makePartition(makeTestAdapter());
84+
p.initializeAggregates();
85+
expect(p.moveNodeToCommunity(0, 0)).toBe(false);
86+
});
87+
88+
it('accumulateNeighborCommunityEdgeWeights/getNeighborEdgeWeightToCommunity report per-community edge weight, and 0 for untouched or out-of-range ids', () => {
89+
const p = makePartition(makeTestAdapter());
90+
p.initializeAggregates();
91+
p.moveNodeToCommunity(0, 1);
92+
93+
const candidateCount = p.accumulateNeighborCommunityEdgeWeights(1);
94+
// node 1's neighbors are 0 (now community 1) and 2 (community 2), plus its
95+
// own community 1 is always touched -- so 2 distinct candidate communities.
96+
expect(candidateCount).toBe(2);
97+
expect(p.getNeighborEdgeWeightToCommunity(1)).toBe(4);
98+
expect(p.getNeighborEdgeWeightToCommunity(2)).toBe(6);
99+
// Community 0 has no members and received no edge weight -- in-range zero.
100+
expect(p.getNeighborEdgeWeightToCommunity(0)).toBe(0);
101+
// Out-of-range id -- must not throw, must fall back to 0.
102+
expect(p.getNeighborEdgeWeightToCommunity(50)).toBe(0);
103+
expect(p.getOutEdgeWeightToCommunity(50)).toBe(0);
104+
expect(p.getInEdgeWeightFromCommunity(50)).toBe(0);
105+
});
106+
107+
it('getCommunityMembers groups node indices by current community', () => {
108+
const p = makePartition(makeTestAdapter());
109+
p.initializeAggregates();
110+
p.moveNodeToCommunity(0, 1);
111+
const members = p.getCommunityMembers();
112+
expect(members[0]).toEqual([]);
113+
expect(members[1]).toEqual([0, 1]);
114+
expect(members[2]).toEqual([2]);
115+
});
116+
117+
it('compactCommunityIds removes empty communities and remaps the rest', () => {
118+
const p = makePartition(makeTestAdapter());
119+
p.initializeAggregates();
120+
p.moveNodeToCommunity(0, 1);
121+
p.compactCommunityIds();
122+
expect(p.communityCount).toBe(2);
123+
// Both remaining communities' sizes should be 2 and 1, in descending-size order.
124+
const sizes = [p.getCommunityTotalSize(0), p.getCommunityTotalSize(1)].sort((a, b) => b - a);
125+
expect(sizes).toEqual([2, 1]);
126+
});
127+
128+
it('respects a custom capacityGrowthFactor option', () => {
129+
const p = makePartition(makeTestAdapter(), { capacityGrowthFactor: 2 });
130+
p.initializeAggregates();
131+
p.resizeCommunities(10);
132+
expect(p.communityCount).toBe(10);
133+
expect(p.communityTotalSize.length).toBeGreaterThanOrEqual(10);
134+
});
135+
});
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import { describe, expect, it } from 'vitest';
2+
import {
3+
fgetOrZero,
4+
igetOrZero,
5+
} from '../../../src/graph/algorithms/leiden/typed-array-helpers.js';
6+
7+
describe('fgetOrZero', () => {
8+
it('returns the in-range value when it is truthy', () => {
9+
const a = new Float64Array([1, 2.5, -3]);
10+
expect(fgetOrZero(a, 0)).toBe(1);
11+
expect(fgetOrZero(a, 1)).toBe(2.5);
12+
expect(fgetOrZero(a, 2)).toBe(-3);
13+
});
14+
15+
it('collapses an in-range zero to 0', () => {
16+
const a = new Float64Array([0, 5]);
17+
expect(fgetOrZero(a, 0)).toBe(0);
18+
});
19+
20+
it('collapses an out-of-range index (positive or negative) to 0', () => {
21+
const a = new Float64Array([1, 2]);
22+
expect(fgetOrZero(a, 2)).toBe(0);
23+
expect(fgetOrZero(a, 99)).toBe(0);
24+
expect(fgetOrZero(a, -1)).toBe(0);
25+
});
26+
27+
it('matches the two guards it replaces: `i < a.length ? fget(a, i) : 0` and `fget(a, i) || 0`', () => {
28+
const a = new Float64Array([0, 7, 0]);
29+
for (let i = -1; i <= a.length; i++) {
30+
const boundsGuard = i < a.length && i >= 0 ? (a[i] as number) : 0;
31+
const orZeroGuard = ((a[i] as number) || 0) as number;
32+
expect(fgetOrZero(a, i)).toBe(boundsGuard);
33+
expect(fgetOrZero(a, i)).toBe(orZeroGuard);
34+
}
35+
});
36+
});
37+
38+
describe('igetOrZero', () => {
39+
it('returns the in-range value when it is truthy', () => {
40+
const a = new Int32Array([4, -6, 0]);
41+
expect(igetOrZero(a, 0)).toBe(4);
42+
expect(igetOrZero(a, 1)).toBe(-6);
43+
});
44+
45+
it('collapses an in-range zero to 0', () => {
46+
const a = new Int32Array([0, 3]);
47+
expect(igetOrZero(a, 0)).toBe(0);
48+
});
49+
50+
it('collapses an out-of-range index to 0', () => {
51+
const a = new Int32Array([1, 2]);
52+
expect(igetOrZero(a, 5)).toBe(0);
53+
expect(igetOrZero(a, -1)).toBe(0);
54+
});
55+
});

0 commit comments

Comments
 (0)