getModularity silently returns 0 for getLouvainCommunities' bare-id output instead of throwing or scoring correctly #47

Version

Confirmed on @statelyai/graph 2.4.0 and (with a different symptom — see below) the current latest, 2.5.1.

Summary

getLouvainCommunities returns communities as arrays of bare node ids (string[][]), but getModularity's type signature expects Community<N>[], which is GraphNode<N>[][] — arrays of full node objects (declare function getModularity<N>(graph: Graph<N>, communities: Community<N>[]): number;, unchanged in 2.5.1). Passing the direct output of getLouvainCommunities into getModularity does not work, defeating the documented pairing of these two functions: the natural, obvious usage (detect communities, then score them) requires a manual id → node remapping step that isn't mentioned anywhere I could find in the docs or JSDoc.

This got worse between 2.4.0 and 2.5.0, not better. The 2.5.0 changelog lists "getModularity ... tolerates unknown ids in communities" as a fix, but the actual effect for this specific pairing is that the failure mode changed from a loud crash to a silent wrong answer:

• 2.4.0: throws a generic TypeError when given bare ids.
• 2.5.1: does not throw, but silently returns 0 (reads as "no community structure at all") instead of the correct modularity score, because toCommunityIds internally does community.map((node) => node.id) — for a bare string 'a' this reads the (nonexistent) .id property of a string and gets undefined, so every community collapses into a single Set { undefined } and every real edge is skipped.

A 0 modularity score is indistinguishable from "this graph truly has no community structure," so this is arguably a more dangerous defect now than the 2.4.0 crash: it no longer tells the caller that something is wrong.

Minimal repro (2.5.1)

import { createGraph, getLouvainCommunities, getModularity } from '@statelyai/graph';

const graph = createGraph({
  nodes: [{ id: 'a' }, { id: 'b' }, { id: 'c' }, { id: 'd' }],
  edges: [
    { id: 'ab', sourceId: 'a', targetId: 'b' },
    { id: 'cd', sourceId: 'c', targetId: 'd' },
  ],
});

const communities = getLouvainCommunities(graph);
console.log(communities);
// [['a', 'b'], ['c', 'd']]  -- two well-separated communities

console.log(getModularity(graph, communities));
// 0  -- wrong; should be 0.5

// The "correct" usage, mapping ids back onto real node objects first:
const nodesById = new Map(graph.nodes.map((n) => [n.id, n]));
const asNodes = communities.map((ids) => ids.map((id) => nodesById.get(id)));
console.log(getModularity(graph, asNodes));
// 0.5 -- correct

(On 2.4.0, the first getModularity(graph, communities) call throws TypeError: Cannot read properties of undefined (reading 'get') instead of returning 0.)

Expected

Either:

• getModularity accepts string[][] (bare ids) in addition to GraphNode<N>[][], since every id in the input graph is unambiguous — the "tolerates unknown ids" fix seems to have been aiming at exactly this case but didn't quite land it (it tolerates the shape without interpreting it), or
• getLouvainCommunities (and the other community-detection functions returning the same shape, e.g. getGreedyModularityCommunities, getLabelPropagationCommunities) return GraphNode<N>[][] to match Community<N>, or
• at minimum, getModularity throws (rather than silently returning a plausible-looking 0) when none of the ids in communities resolve to a real node, since that is almost certainly a caller passing the wrong shape rather than an intentionally empty community.

Actual

getModularity(graph, getLouvainCommunities(graph)) silently returns 0 instead of the graph's actual modularity, with no indication that the input shape, not the data, is the problem.

Workaround

const nodesById = new Map(graph.nodes.map((n) => [n.id, n]));
const asCommunities = communities.map((ids) => ids.map((id) => nodesById.get(id)!));
getModularity(graph, asCommunities);