scriptoriumadmin/GitNexus
1
0
Fork 0
mirror of https://github.com/abhigyanpatwari/GitNexus.git synced 2026-09-30 01:51:20 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 7

fix(web): address three code-review bugs in graph rendering

Browse source 
Bug 1 (useSigma.ts): forces in the tree physics loop were computed once
before the sub-steps loop and reused for every step, causing 2× displacement
on slow frames (>64ms, simulationSteps>1). Fix: move forceX/forceY Maps and
all force accumulation (layer gravity, edge springs, repulsion, spread) inside
the loop so each sub-step integrates from current node positions.

Bug 2 (graph-adapter.ts): all three adapters used `graph.hasEdge(src,tgt)`
as a dedup guard, which silently drops any second edge between the same node
pair. A CALLS relationship between nodes that also have a CONTAINS edge was
always lost. Fix: switch from `new Graph()` to `new MultiGraph()` (allows
multiple edges per pair) and dedup by `rel.id` instead of by node pair.

Bug 3 (graph-adapter.test.ts): the cross-cutting edge styling test never
executed its CALLS branch because Bug 2 dropped the CALLS edge before the
assertion ran. Fix: assert `sigmaGraph.size === 2` and verify both edges
individually after collecting attrs by relationType.

Co-authored-by: Claude <noreply@anthropic.com>
AI-model: claude-sonnet-4-5
This commit is contained in:
Hugo Gu 2026-05-25 23:09:35 +08:00
parent e2f0def372
commit f7aafb02f9
3 changed files with 192 additions and 161 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

283
gitnexus-web/src/hooks/useSigma.ts
View file

@ -835,151 +835,156 @@ export const useSigma = (options: UseSigmaOptions = {}): UseSigmaReturn => {
treeAccumulatorRef.current -= simulationSteps * TREE_TARGET_FRAME_MS;
const dtScale = 0.6;
// --- Accumulate forces ---
const forceX = new Map<string, number>();
const forceY = new Map<string, number>();
// 1. Layer gravity: soft pull toward each node's preferred Y within its band.
// Directional nodes (above-only or below-only connections) are pulled to the
// top or bottom of the band; bidirectional / same-layer-only nodes go to
// the center. This leaves band edges free for nodes that actually use them.
graph.forEachNode((nodeId, attrs) => {
const layer = attrs.treeLayer ?? 0;
const centerY = layerCenterY.get(layer) ?? attrs.y;
const bias = nodeYBias.get(nodeId) ?? 0;
const targetY = centerY + bias * TREE_LAYER_BAND_HALF;
forceX.set(nodeId, 0);
forceY.set(nodeId, (targetY - attrs.y) * TREE_LAYER_GRAVITY * dtScale);
});
// 2. Edge springs — X and Y handled separately.
//
// Root cause of long horizontal edges: the previous 2D spring projected
// force through (dx/distance, dy/distance). When the Y layer gap
// dominates (|dy|≈200, |dx|≈30) the X component shrinks to ~15% of
// the total spring force, too weak to overcome sibling repulsion.
//
// Fix: compute X spring from |dx| alone. This keeps full strength
// regardless of how far apart two nodes are in Y.
graph.forEachEdge((edge, edgeAttrs, source, target, sourceAttrs, targetAttrs) => {
const dx = targetAttrs.x - sourceAttrs.x;
const rawWeight = TREE_EDGE_WEIGHTS[edgeAttrs.relationType] ?? 0.18;
// 2a. Pure X spring.
// Hierarchy edges: zero rest length so children want to sit directly
// under their parent (repulsion then spreads siblings out naturally).
// Cross edges: 60 px rest so far-spanning CALLS/IMPORTS edges only
// pull when really stretched, and their weight is capped so they
// don't override the hierarchy structure.
const xRestLength = edgeAttrs.isHierarchyEdge ? 0 : 60;
const xStretch = Math.abs(dx) - xRestLength;
if (xStretch > 0) {
const xWeight = edgeAttrs.isHierarchyEdge ? rawWeight : Math.min(rawWeight, 0.1);
const fxX = Math.sign(dx) * xStretch * xWeight * 0.3 * dtScale;
forceX.set(source, (forceX.get(source) ?? 0) + fxX);
forceX.set(target, (forceX.get(target) ?? 0) - fxX);
}
// 2b. Weak Y spring — layer gravity handles most vertical placement;
// this just prevents extreme cross-layer stretching.
const dy = targetAttrs.y - sourceAttrs.y;
const distance = Math.sqrt(dx * dx + dy * dy) || 1;
const layerGap = Math.abs((targetAttrs.treeLayer ?? 0) - (sourceAttrs.treeLayer ?? 0));
const yRestLength =
(edgeAttrs.isHierarchyEdge ? 70 : 95) +
layerGap * (edgeAttrs.isHierarchyEdge ? 28 : 36);
const yStretch = distance - yRestLength;
if (yStretch > 0) {
const fy = (dy / distance) * yStretch * rawWeight * 0.008 * dtScale;
forceY.set(source, (forceY.get(source) ?? 0) + fy);
forceY.set(target, (forceY.get(target) ?? 0) - fy);
}
});
// 3. Node repulsion in 2D: all pairs within range (cross-layer included)
// Sort by X for O(n·k) early-exit: once dx > range, all further pairs are too far.
//
// Skipped for large graphs (N > 5 000) — sorting + pair comparisons make each
// frame take hundreds of ms, leaving the canvas apparently frozen. Layer gravity
// and edge springs provide sufficient structure without repulsion.
if (treeUseRepulsion) {
const nodeList = graph.nodes().map((id) => {
const a = graph.getNodeAttributes(id);
return { id, x: a.x, y: a.y, size: a.size ?? 6, layer: a.treeLayer ?? 0 };
});
nodeList.sort((a, b) => a.x - b.x);
for (let i = 0; i < nodeList.length; i++) {
const nodeA = nodeList[i];
for (let j = i + 1; j < nodeList.length; j++) {
const nodeB = nodeList[j];
const dx = nodeB.x - nodeA.x;
if (dx > TREE_REPULSION_RANGE) break; // X-sorted: all further pairs are also too far
const dy = nodeB.y - nodeA.y;
const dist = Math.sqrt(dx * dx + dy * dy) || 1;
if (dist > TREE_REPULSION_RANGE) continue;
const sameLayer = nodeA.layer === nodeB.layer;
// Same-layer repulsion reduced from 160→100 so the stronger X spring
// (0.30) can now overcome collective repulsion from 3-4 nearby nodes.
// Cross-layer kept low (28) so intermediate-layer nodes don't block
// parent-child X alignment.
const repulsionStrength = sameLayer ? 100 : 28;
const minGap = Math.max(28, (nodeA.size + nodeB.size) * 1.8);
let repulsion =
(1 / (dist + 8) - 1 / (TREE_REPULSION_RANGE + 8)) * repulsionStrength * dtScale;
if (dist < minGap && sameLayer) {
repulsion += (minGap - dist) * 0.1 * dtScale;
}
if (repulsion <= 0) continue;
const fx = (dx / dist) * repulsion;
const fy = (dy / dist) * repulsion;
forceX.set(nodeA.id, (forceX.get(nodeA.id) ?? 0) - fx);
forceY.set(nodeA.id, (forceY.get(nodeA.id) ?? 0) - fy);
forceX.set(nodeB.id, (forceX.get(nodeB.id) ?? 0) + fx);
forceY.set(nodeB.id, (forceY.get(nodeB.id) ?? 0) + fy);
}
}
}
// 4. Spread force: equalize node density within each layer.
//
// For each layer, rank nodes by current X, compute where they would sit
// in a perfectly even distribution, then add a weak force toward that
// ideal position. Nodes that are held by strong hierarchy springs
// (force ≈ 1–2 units) resist and stay clustered; nodes without a
// strong spring anchor (isolated or same-layer-only) drift to fill gaps.
// Net effect: dense centre spreads outward, sparse edges fill in.
// Skipped for large graphs — per-layer sort is O(N log N) per frame.
if (treeUseSpread) {
const spreadByLayer = new Map<number, Array<{ id: string; x: number }>>();
graph.forEachNode((nodeId, attrs) => {
const layer = attrs.treeLayer ?? 0;
if (!spreadByLayer.has(layer)) spreadByLayer.set(layer, []);
spreadByLayer.get(layer)!.push({ id: nodeId, x: attrs.x });
});
for (const [, layerNodes] of spreadByLayer) {
if (layerNodes.length < 2) continue;
layerNodes.sort((a, b) => a.x - b.x);
const count = layerNodes.length;
const spacing = (TREE_MAX_X * 2) / count;
for (let i = 0; i < count; i++) {
const { id, x } = layerNodes[i];
const idealX = -TREE_MAX_X + (i + 0.5) * spacing;
forceX.set(id, (forceX.get(id) ?? 0) + (idealX - x) * TREE_SPREAD_STRENGTH * dtScale);
}
}
}
// --- Apply forces: velocity integration with boundary resistance ---
// Forces are recomputed from current node positions each sub-step so that
// slow frames (simulationSteps > 1) integrate correctly and don't double-apply.
let totalVelocity = 0;
let maxVelocity = 0;
let activeNodes = 0;
for (let simulationStep = 0; simulationStep < simulationSteps; simulationStep++) {
// --- Accumulate forces (recomputed each sub-step from current positions) ---
const forceX = new Map<string, number>();
const forceY = new Map<string, number>();
// 1. Layer gravity: soft pull toward each node's preferred Y within its band.
// Directional nodes (above-only or below-only connections) are pulled to the
// top or bottom of the band; bidirectional / same-layer-only nodes go to
// the center. This leaves band edges free for nodes that actually use them.
graph.forEachNode((nodeId, attrs) => {
const layer = attrs.treeLayer ?? 0;
const centerY = layerCenterY.get(layer) ?? attrs.y;
const bias = nodeYBias.get(nodeId) ?? 0;
const targetY = centerY + bias * TREE_LAYER_BAND_HALF;
forceX.set(nodeId, 0);
forceY.set(nodeId, (targetY - attrs.y) * TREE_LAYER_GRAVITY * dtScale);
});
// 2. Edge springs — X and Y handled separately.
//
// Root cause of long horizontal edges: the previous 2D spring projected
// force through (dx/distance, dy/distance). When the Y layer gap
// dominates (|dy|≈200, |dx|≈30) the X component shrinks to ~15% of
// the total spring force, too weak to overcome sibling repulsion.
//
// Fix: compute X spring from |dx| alone. This keeps full strength
// regardless of how far apart two nodes are in Y.
graph.forEachEdge((edge, edgeAttrs, source, target, sourceAttrs, targetAttrs) => {
const dx = targetAttrs.x - sourceAttrs.x;
const rawWeight = TREE_EDGE_WEIGHTS[edgeAttrs.relationType] ?? 0.18;
// 2a. Pure X spring.
// Hierarchy edges: zero rest length so children want to sit directly
// under their parent (repulsion then spreads siblings out naturally).
// Cross edges: 60 px rest so far-spanning CALLS/IMPORTS edges only
// pull when really stretched, and their weight is capped so they
// don't override the hierarchy structure.
const xRestLength = edgeAttrs.isHierarchyEdge ? 0 : 60;
const xStretch = Math.abs(dx) - xRestLength;
if (xStretch > 0) {
const xWeight = edgeAttrs.isHierarchyEdge ? rawWeight : Math.min(rawWeight, 0.1);
const fxX = Math.sign(dx) * xStretch * xWeight * 0.3 * dtScale;
forceX.set(source, (forceX.get(source) ?? 0) + fxX);
forceX.set(target, (forceX.get(target) ?? 0) - fxX);
}
// 2b. Weak Y spring — layer gravity handles most vertical placement;
// this just prevents extreme cross-layer stretching.
const dy = targetAttrs.y - sourceAttrs.y;
const distance = Math.sqrt(dx * dx + dy * dy) || 1;
const layerGap = Math.abs((targetAttrs.treeLayer ?? 0) - (sourceAttrs.treeLayer ?? 0));
const yRestLength =
(edgeAttrs.isHierarchyEdge ? 70 : 95) +
layerGap * (edgeAttrs.isHierarchyEdge ? 28 : 36);
const yStretch = distance - yRestLength;
if (yStretch > 0) {
const fy = (dy / distance) * yStretch * rawWeight * 0.008 * dtScale;
forceY.set(source, (forceY.get(source) ?? 0) + fy);
forceY.set(target, (forceY.get(target) ?? 0) - fy);
}
});
// 3. Node repulsion in 2D: all pairs within range (cross-layer included)
// Sort by X for O(n·k) early-exit: once dx > range, all further pairs are too far.
//
// Skipped for large graphs (N > 5 000) — sorting + pair comparisons make each
// frame take hundreds of ms, leaving the canvas apparently frozen. Layer gravity
// and edge springs provide sufficient structure without repulsion.
if (treeUseRepulsion) {
const nodeList = graph.nodes().map((id) => {
const a = graph.getNodeAttributes(id);
return { id, x: a.x, y: a.y, size: a.size ?? 6, layer: a.treeLayer ?? 0 };
});
nodeList.sort((a, b) => a.x - b.x);
for (let i = 0; i < nodeList.length; i++) {
const nodeA = nodeList[i];
for (let j = i + 1; j < nodeList.length; j++) {
const nodeB = nodeList[j];
const dx = nodeB.x - nodeA.x;
if (dx > TREE_REPULSION_RANGE) break; // X-sorted: all further pairs are also too far
const dy = nodeB.y - nodeA.y;
const dist = Math.sqrt(dx * dx + dy * dy) || 1;
if (dist > TREE_REPULSION_RANGE) continue;
const sameLayer = nodeA.layer === nodeB.layer;
// Same-layer repulsion reduced from 160→100 so the stronger X spring
// (0.30) can now overcome collective repulsion from 3-4 nearby nodes.
// Cross-layer kept low (28) so intermediate-layer nodes don't block
// parent-child X alignment.
const repulsionStrength = sameLayer ? 100 : 28;
const minGap = Math.max(28, (nodeA.size + nodeB.size) * 1.8);
let repulsion =
(1 / (dist + 8) - 1 / (TREE_REPULSION_RANGE + 8)) * repulsionStrength * dtScale;
if (dist < minGap && sameLayer) {
repulsion += (minGap - dist) * 0.1 * dtScale;
}
if (repulsion <= 0) continue;
const fx = (dx / dist) * repulsion;
const fy = (dy / dist) * repulsion;
forceX.set(nodeA.id, (forceX.get(nodeA.id) ?? 0) - fx);
forceY.set(nodeA.id, (forceY.get(nodeA.id) ?? 0) - fy);
forceX.set(nodeB.id, (forceX.get(nodeB.id) ?? 0) + fx);
forceY.set(nodeB.id, (forceY.get(nodeB.id) ?? 0) + fy);
}
}
}
// 4. Spread force: equalize node density within each layer.
//
// For each layer, rank nodes by current X, compute where they would sit
// in a perfectly even distribution, then add a weak force toward that
// ideal position. Nodes that are held by strong hierarchy springs
// (force ≈ 1–2 units) resist and stay clustered; nodes without a
// strong spring anchor (isolated or same-layer-only) drift to fill gaps.
// Net effect: dense centre spreads outward, sparse edges fill in.
// Skipped for large graphs — per-layer sort is O(N log N) per frame.
if (treeUseSpread) {
const spreadByLayer = new Map<number, Array<{ id: string; x: number }>>();
graph.forEachNode((nodeId, attrs) => {
const layer = attrs.treeLayer ?? 0;
if (!spreadByLayer.has(layer)) spreadByLayer.set(layer, []);
spreadByLayer.get(layer)!.push({ id: nodeId, x: attrs.x });
});
for (const [, layerNodes] of spreadByLayer) {
if (layerNodes.length < 2) continue;
layerNodes.sort((a, b) => a.x - b.x);
const count = layerNodes.length;
const spacing = (TREE_MAX_X * 2) / count;
for (let i = 0; i < count; i++) {
const { id, x } = layerNodes[i];
const idealX = -TREE_MAX_X + (i + 0.5) * spacing;
forceX.set(
id,
(forceX.get(id) ?? 0) + (idealX - x) * TREE_SPREAD_STRENGTH * dtScale,
);
}
}
}
totalVelocity = 0;
maxVelocity = 0;
activeNodes = 0;

46
gitnexus-web/src/lib/graph-adapter.test.ts
View file

@ -63,16 +63,23 @@ describe('knowledgeGraphToTreeGraphology', () => {
const sigmaGraph = knowledgeGraphToTreeGraphology(graph);
// Find edges and check their attributes
sigmaGraph.forEachEdge((edge, attrs) => {
if (attrs.relationType === 'CONTAINS') {
expect(attrs.isHierarchyEdge).toBe(true);
expect(attrs.color).toBe(EDGE_INFO.CONTAINS.color);
} else if (attrs.relationType === 'CALLS') {
expect(attrs.isHierarchyEdge).toBe(false);
expect(attrs.color).toBe(EDGE_INFO.CALLS.color);
}
// MultiGraph allows multiple edges per pair — both CONTAINS and CALLS must survive.
expect(sigmaGraph.size).toBe(2);
const attrsByType = new Map<string, { isHierarchyEdge?: boolean; color: string }>();
sigmaGraph.forEachEdge((_edge, attrs) => {
attrsByType.set(attrs.relationType, attrs);
});
const containsAttrs = attrsByType.get('CONTAINS');
expect(containsAttrs).toBeDefined();
expect(containsAttrs!.isHierarchyEdge).toBe(true);
expect(containsAttrs!.color).toBe(EDGE_INFO.CONTAINS.color);
const callsAttrs = attrsByType.get('CALLS');
expect(callsAttrs).toBeDefined();
expect(callsAttrs!.isHierarchyEdge).toBe(false);
expect(callsAttrs!.color).toBe(EDGE_INFO.CALLS.color);
});
it('should treat imports as cross-cutting edges in tree view', () => {
@ -161,16 +168,25 @@ describe('knowledgeGraphToCirclesGraphology', () => {
],
};
// Two relationships between the same pair — graph-adapter deduplicates via
// hasEdge check, so only the first inserted (CONTAINS, hierarchy) is kept.
// MultiGraph allows multiple edges per pair — both CONTAINS and CALLS must survive.
const sigmaGraph = knowledgeGraphToCirclesGraphology(graph);
expect(sigmaGraph.size).toBe(2);
const attrsByType = new Map<string, { isHierarchyEdge?: boolean; color: string }>();
sigmaGraph.forEachEdge((_, attrs) => {
if (attrs.relationType === 'CONTAINS') {
expect(attrs.isHierarchyEdge).toBe(true);
expect(attrs.color).toBe(EDGE_INFO.CONTAINS.color);
}
attrsByType.set(attrs.relationType, attrs);
});
const containsAttrs = attrsByType.get('CONTAINS');
expect(containsAttrs).toBeDefined();
expect(containsAttrs!.isHierarchyEdge).toBe(true);
expect(containsAttrs!.color).toBe(EDGE_INFO.CONTAINS.color);
const callsAttrs = attrsByType.get('CALLS');
expect(callsAttrs).toBeDefined();
expect(callsAttrs!.isHierarchyEdge).toBe(false);
expect(callsAttrs!.color).toBe(EDGE_INFO.CALLS.color);
});
it('should treat CALLS as a cross-cutting edge in circles view', () => {

24
gitnexus-web/src/lib/graph-adapter.ts
View file

@ -1,4 +1,4 @@
import Graph from 'graphology';
import Graph, { MultiGraph } from 'graphology';
import type { NodeLabel } from 'gitnexus-shared';
import type { KnowledgeGraph } from '../core/graph/types';
import { EDGE_INFO, NODE_COLORS, NODE_SIZES, getCommunityColor } from './constants';
@ -95,7 +95,7 @@ export const knowledgeGraphToGraphology = (
knowledgeGraph: KnowledgeGraph,
communityMemberships?: Map<string, number>,
): Graph<SigmaNodeAttributes, SigmaEdgeAttributes> => {
const graph = new Graph<SigmaNodeAttributes, SigmaEdgeAttributes>();
const graph = new MultiGraph<SigmaNodeAttributes, SigmaEdgeAttributes>();
const nodeCount = knowledgeGraph.nodes.length;
// Build parent-child map from hierarchy relationships
@ -314,9 +314,13 @@ export const knowledgeGraphToGraphology = (
// and cross-edges (CALLS, IMPORTS, EXTENDS) are drawn on top.
const BACKGROUND_EDGE_TYPES = new Set(['CONTAINS', 'DEFINES', 'HAS_METHOD', 'HAS_PROPERTY']);
// Dedup by relationship ID, not by node-pair — a node pair can have both a
// CONTAINS edge and a CALLS edge (MultiGraph allows multiple edges per pair).
const addedRelIds = new Set<string>();
const addEdge = (rel: (typeof knowledgeGraph.relationships)[number]) => {
if (!graph.hasNode(rel.sourceId) || !graph.hasNode(rel.targetId)) return;
if (graph.hasEdge(rel.sourceId, rel.targetId)) return;
if (addedRelIds.has(rel.id)) return;
addedRelIds.add(rel.id);
const style = EDGE_STYLES[rel.type] || { color: '#4a4a5a', sizeMultiplier: 0.5 };
const curvature = 0.12 + Math.random() * 0.08;
graph.addEdge(rel.sourceId, rel.targetId, {
@ -343,7 +347,7 @@ export const knowledgeGraphToGraphology = (
export const knowledgeGraphToTreeGraphology = (
knowledgeGraph: KnowledgeGraph,
): Graph<SigmaNodeAttributes, SigmaEdgeAttributes> => {
const graph = new Graph<SigmaNodeAttributes, SigmaEdgeAttributes>();
const graph = new MultiGraph<SigmaNodeAttributes, SigmaEdgeAttributes>();
const nodeCount = knowledgeGraph.nodes.length;
const positions = calculateTreeLayout(knowledgeGraph);
@ -392,9 +396,12 @@ export const knowledgeGraphToTreeGraphology = (
};
// Two-pass insertion: hierarchy edges first (rendered behind), cross-edges on top.
// Dedup by relationship ID so CONTAINS + CALLS between the same pair both survive.
const addedTreeRelIds = new Set<string>();
const addTreeEdge = (rel: (typeof knowledgeGraph.relationships)[number]) => {
if (!graph.hasNode(rel.sourceId) || !graph.hasNode(rel.targetId)) return;
if (graph.hasEdge(rel.sourceId, rel.targetId)) return;
if (addedTreeRelIds.has(rel.id)) return;
addedTreeRelIds.add(rel.id);
const isHierarchy = HIERARCHY_EDGE_STYLES[rel.type] !== undefined;
const style = isHierarchy
? HIERARCHY_EDGE_STYLES[rel.type]
@ -422,7 +429,7 @@ export const knowledgeGraphToTreeGraphology = (
export const knowledgeGraphToCirclesGraphology = (
knowledgeGraph: KnowledgeGraph,
): Graph<SigmaNodeAttributes, SigmaEdgeAttributes> => {
const graph = new Graph<SigmaNodeAttributes, SigmaEdgeAttributes>();
const graph = new MultiGraph<SigmaNodeAttributes, SigmaEdgeAttributes>();
const nodeCount = knowledgeGraph.nodes.length;
const positions = calculateCirclesLayout(knowledgeGraph);
@ -471,9 +478,12 @@ export const knowledgeGraphToCirclesGraphology = (
};
// Two-pass insertion: hierarchy edges first (rendered behind), cross-edges on top.
// Dedup by relationship ID so CONTAINS + CALLS between the same pair both survive.
const addedCirclesRelIds = new Set<string>();
const addCirclesEdge = (rel: (typeof knowledgeGraph.relationships)[number]) => {
if (!graph.hasNode(rel.sourceId) || !graph.hasNode(rel.targetId)) return;
if (graph.hasEdge(rel.sourceId, rel.targetId)) return;
if (addedCirclesRelIds.has(rel.id)) return;
addedCirclesRelIds.add(rel.id);
const isHierarchy = HIERARCHY_EDGE_STYLES[rel.type] !== undefined;
const style = isHierarchy
? HIERARCHY_EDGE_STYLES[rel.type]