fix(explorer): address edge label review findings

This commit is contained in:
Sameer6305
2026-08-18 20:34:54 +05:30
parent 75bc6255d4
commit 3fbe3cfd2d
4 changed files with 90 additions and 19 deletions
@@ -1312,6 +1312,9 @@ export const GraphCanvas = forwardRef<GraphCanvasHandle, GraphCanvasProps>(
const onEdgeClickRef = useRef(onEdgeClick);
const onSceneRuntimeChangeRef = useRef(onSceneRuntimeChange);
const onCameraStateChangeRef = useRef(onCameraStateChange);
// #1009: tracked as a ref so the Sigma creation effect always reads the
// current value without needing effectsState in its dependency array.
const effectsStateRef = useRef(effectsState);
const [hoveredNodeId, setHoveredNodeId] = useState<string | null>(null);
const [zoomTier, setZoomTier] = useState<GraphZoomTier>("overview");
const [analyticsSnapshot, setAnalyticsSnapshot] = useState<GraphAnalyticsSnapshot | null>(null);
@@ -1340,6 +1343,7 @@ export const GraphCanvas = forwardRef<GraphCanvasHandle, GraphCanvasProps>(
onEdgeClickRef.current = onEdgeClick;
onSceneRuntimeChangeRef.current = onSceneRuntimeChange;
onCameraStateChangeRef.current = onCameraStateChange;
effectsStateRef.current = effectsState;
const behaviors = useMemo<GraphBehavior[]>(
() => [
@@ -1852,7 +1856,13 @@ export const GraphCanvas = forwardRef<GraphCanvasHandle, GraphCanvasProps>(
return;
}
const sigma = new Sigma(displayGraphRef.current, containerRef.current, SIGMA_SETTINGS);
const sigma = new Sigma(displayGraphRef.current, containerRef.current, {
...SIGMA_SETTINGS,
// #1009: initialize with the current toggle value rather than the
// static default so that a user who disabled Edge Labels before
// graph/Sigma initialization sees the correct state after mount.
renderEdgeLabels: effectsStateRef.current.edgeLabelsEnabled,
});
sigmaRef.current = sigma;
appliedGraphVersionRef.current = graphVersionRef.current;
@@ -2099,6 +2099,15 @@ function createCollapsedNeighborhoodGraph(
return collapsedGraph;
}
// Normalize an edge relationship type: empty string, null, and undefined all
// fall back to the project-wide default used consistently across every
// aggregation path. Keep this local — it exists only to guarantee that the
// three code paths (single-entry, multi-entry, community-grouped) produce the
// same semantics and do not diverge again.
function normalizeEdgeType(value: string | null | undefined): string {
return value || "related_to";
}
function aggregateDisplayGraph(graphRef: GraphRef): Graph<NodeAttributes, EdgeAttributes> {
const aggregated = new Graph<NodeAttributes, EdgeAttributes>({
type: "directed",
@@ -2124,10 +2133,13 @@ function aggregateDisplayGraph(graphRef: GraphRef): Graph<NodeAttributes, EdgeAt
const [{ edgeId, attrs }] = entries;
aggregated.mergeDirectedEdgeWithKey(edgeId, sourceId, targetId, {
...attrs,
// #1009: normalize empty/null/undefined edgeType so Sigma's label
// renderer never receives a blank string on the single-entry path.
edgeType: normalizeEdgeType(attrs.edgeType),
dominantEdgeType: normalizeEdgeType(attrs.dominantEdgeType ?? attrs.edgeType),
rawEdgeIds: collectRawEdgeIds(attrs, edgeId),
isAggregated: isAggregatedEdgeAttributes(attrs),
aggregateCount: attrs.aggregateCount ?? collectRawEdgeIds(attrs, edgeId).length,
dominantEdgeType: attrs.dominantEdgeType ?? attrs.edgeType,
representativeWeight: attrs.representativeWeight ?? Number(attrs.weight ?? 1),
});
return;
@@ -2150,10 +2162,11 @@ function aggregateDisplayGraph(graphRef: GraphRef): Graph<NodeAttributes, EdgeAt
const rawEdgeIds = entries.flatMap(({ edgeId, attrs }) => collectRawEdgeIds(attrs, edgeId));
const typeCounts = new Map<string, number>();
entries.forEach(({ attrs }) => {
const edgeType = String(attrs.edgeType || "related_to");
const edgeType = normalizeEdgeType(attrs.edgeType);
typeCounts.set(edgeType, (typeCounts.get(edgeType) ?? 0) + 1);
});
const dominantEdgeType = [...typeCounts.entries()].sort((left, right) => right[1] - left[1])[0]?.[0] ?? representative.attrs.edgeType ?? "related_to";
const dominantEdgeType = [...typeCounts.entries()].sort((left, right) => right[1] - left[1])[0]?.[0]
?? normalizeEdgeType(representative.attrs.edgeType);
const reverseKey = `${targetId}${sourceId}`;
const isBidirectionalBundle = groupedEdges.has(reverseKey);
const syntheticEdgeId = `${AGGREGATED_EDGE_PREFIX}${sourceId}::${targetId}`;
@@ -2167,10 +2180,10 @@ function aggregateDisplayGraph(graphRef: GraphRef): Graph<NodeAttributes, EdgeAt
rawEdgeIds,
isAggregated: true,
aggregateCount: rawEdgeIds.length,
dominantEdgeType: String(dominantEdgeType),
dominantEdgeType: dominantEdgeType,
representativeWeight: Number(representative.attrs.weight ?? 1),
weight: Number(representative.attrs.weight ?? 1),
edgeType: String(representative.attrs.edgeType || dominantEdgeType || "related_to"),
edgeType: representative.attrs.edgeType || dominantEdgeType,
parallelCount: rawEdgeIds.length,
familySize: rawEdgeIds.length,
bundleKind: isBidirectionalBundle ? "bidirectional" : "parallel",
@@ -2280,7 +2293,7 @@ function buildCommunityGroupedGraph(): GraphDisplayResult {
};
bucket.rawEdgeIds.push(String(edgeId));
bucket.weight = Math.max(bucket.weight, Number((attrs as EdgeAttributes).weight ?? 1));
const edgeType = String((attrs as EdgeAttributes).edgeType || "related_to");
const edgeType = normalizeEdgeType((attrs as EdgeAttributes).edgeType);
bucket.typeCounts.set(edgeType, (bucket.typeCounts.get(edgeType) ?? 0) + 1);
groupedEdges.set(key, bucket);
});
@@ -2396,7 +2409,8 @@ function buildCommunityGroupedGraph(): GraphDisplayResult {
if (!visibleGroupedEdgeKeys.has(key)) {
return;
}
const dominantEdgeType = [...bundle.typeCounts.entries()].sort((left, right) => right[1] - left[1])[0]?.[0] ?? "related_to";
const dominantEdgeType = [...bundle.typeCounts.entries()].sort((left, right) => right[1] - left[1])[0]?.[0]
?? "related_to";
const reverseKey = `${bundle.targetId}${bundle.sourceId}`;
const syntheticEdgeId = `${AGGREGATED_EDGE_PREFIX}${key}`;
const aggregateCount = bundle.rawEdgeIds.length;
@@ -1,6 +1,7 @@
import type { CSSProperties } from "react";
import type {
GraphDiagnosticsSnapshot,
GraphEffectAvailability,
GraphEffectToggle,
} from "../types";
@@ -42,6 +43,17 @@ const EFFECT_ROWS: EffectRowConfig[] = [
},
];
// Maps the effect toggle keys rendered by this plugin to their corresponding
// availability keys in GraphDiagnosticsSnapshot["effectAvailability"]. Kept
// local because this plugin only renders a subset of all effects.
const EFFECT_AVAILABILITY_KEYS: Partial<Record<GraphEffectToggle, keyof GraphDiagnosticsSnapshot["effectAvailability"]>> = {
pathPulseEnabled: "pathPulse",
pathFlowEnabled: "pathFlow",
lensEnabled: "lens",
edgeLabelsEnabled: "edgeLabels",
legendEnabled: "legend",
};
function renderAvailabilityText(availability: GraphEffectAvailability) {
if (availability.available) {
if (typeof availability.visibleSegments === "number" && typeof availability.segmentCap === "number") {
@@ -144,17 +156,9 @@ export const explorationEffectsPlugin: GraphPlugin = {
description={row.description}
checked={effectsState[row.key]}
availability={
availability?.[
row.key === "pathPulseEnabled"
? "pathPulse"
: row.key === "pathFlowEnabled"
? "pathFlow"
: row.key === "lensEnabled"
? "lens"
: row.key === "edgeLabelsEnabled"
? "edgeLabels"
: "legend"
] ?? {
(EFFECT_AVAILABILITY_KEYS[row.key] !== undefined
? availability?.[EFFECT_AVAILABILITY_KEYS[row.key]!]
: undefined) ?? {
enabled: effectsState[row.key],
available: false,
reason: "Waiting for graph runtime",
@@ -1213,3 +1213,46 @@ test("resolveEdgeElementStyle hidden class produces hidden:true for suppressed e
);
assert.equal(style.hidden, true);
});
// ── #1009 maintainer-blocking regression: single-edge empty edgeType ─────────
test("resolveDisplayGraph single-edge normalizes empty-string edgeType to related_to", () => {
addNode("a");
addNode("b");
// One edge only — exercises the entries.length === 1 path in aggregateDisplayGraph.
batchMergeEdges([{
id: "e-single-empty",
source: "a",
target: "b",
attributes: { edgeType: "", weight: 1, properties: {} },
}]);
const { graph: displayGraph } = resolveDisplayGraph("", [], [], "full", { aggregationEnabled: true });
assert.equal(displayGraph.size, 1);
const edgeId = displayGraph.edges()[0];
const attrs = displayGraph.getEdgeAttributes(edgeId) as { edgeType?: string; dominantEdgeType?: string };
assert.equal(attrs.edgeType, "related_to",
"single-edge path must normalize empty edgeType to the canonical fallback");
assert.equal(attrs.dominantEdgeType, "related_to",
"single-edge dominantEdgeType must also be normalized");
});
test("resolveDisplayGraph single-edge preserves a valid non-empty edgeType unchanged", () => {
addNode("a");
addNode("b");
batchMergeEdges([{
id: "e-single-valid",
source: "a",
target: "b",
attributes: { edgeType: "works_for", weight: 1, properties: {} },
}]);
const { graph: displayGraph } = resolveDisplayGraph("", [], [], "full", { aggregationEnabled: true });
assert.equal(displayGraph.size, 1);
const edgeId = displayGraph.edges()[0];
const attrs = displayGraph.getEdgeAttributes(edgeId) as { edgeType?: string };
assert.equal(attrs.edgeType, "works_for",
"single-edge path must not alter a valid relationship type");
});