-
Notifications
You must be signed in to change notification settings - Fork 1
⚡ Bolt: [O(1) Map lookups in NetworkGraph] #1377
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
31da352
6b9c138
b4ea176
e75ce01
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| import { readFileSync } from "node:fs"; | ||
| import { fileURLToPath } from "node:url"; | ||
|
|
||
| import { describe, expect, it } from "vitest"; | ||
|
|
||
| const networkGraphSource = readFileSync( | ||
| fileURLToPath(new URL("./NetworkGraph.tsx", import.meta.url)), | ||
| "utf8", | ||
| ); | ||
|
|
||
| function sourceBetween(startMarker: string, endMarker: string): string { | ||
| const startIndex = networkGraphSource.indexOf(startMarker); | ||
| const endIndex = networkGraphSource.indexOf(endMarker, startIndex); | ||
|
|
||
| expect(startIndex).toBeGreaterThanOrEqual(0); | ||
| expect(endIndex).toBeGreaterThan(startIndex); | ||
|
|
||
| return networkGraphSource.slice(startIndex, endIndex); | ||
| } | ||
|
|
||
| describe("NetworkGraph constant-time selection lookup contract", () => { | ||
| it("keeps graph event selection on memoized maps without linear fallback scans", () => { | ||
| const edgeSelection = sourceBetween("const selectEdge =", "const selectNode ="); | ||
| const nodeSelection = sourceBetween("const selectNode =", "const handleEdgeSelection ="); | ||
|
|
||
| expect(edgeSelection).toContain("edgeMap.get(String(edgeId))"); | ||
| expect(edgeSelection).not.toContain(".find("); | ||
|
|
||
| expect(nodeSelection).toContain("nodeMap.get(String(nodeId))"); | ||
| expect(nodeSelection).toContain("?? String(nodeId)"); | ||
| expect(nodeSelection).not.toContain("findNodeLabel("); | ||
| expect(nodeSelection).not.toContain(".find("); | ||
| }); | ||
|
|
||
| it("keeps select controls on memoized maps without rescanning nodes or edges", () => { | ||
| const graphNodeSelection = sourceBetween( | ||
| "const selectGraphNode =", | ||
| "const handleSelectFirstRelationship =", | ||
| ); | ||
| const relationshipControl = sourceBetween( | ||
| "const handleRelationshipOptionChange =", | ||
| "const handleNodeOptionChange =", | ||
| ); | ||
| const nodeControl = sourceBetween( | ||
| "const handleNodeOptionChange =", | ||
| "const handleZoomGraph =", | ||
| ); | ||
|
|
||
| expect(graphNodeSelection).toContain("nodeMap.get(String(node.id))"); | ||
| expect(graphNodeSelection).toContain("?? String(node.id)"); | ||
| expect(graphNodeSelection).not.toContain("findNodeLabel("); | ||
| expect(graphNodeSelection).not.toContain(".find("); | ||
|
|
||
| expect(relationshipControl).toContain("edgeMap.get(value)"); | ||
| expect(relationshipControl).not.toContain(".find("); | ||
|
|
||
| expect(nodeControl).toContain("nodeInstanceMap.get(value)"); | ||
| expect(nodeControl).not.toContain(".find("); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -159,6 +159,8 @@ export default function NetworkGraph() { | |
| const [graphActionStatus, setGraphActionStatus] = useState('그래프 준비 완료'); | ||
| const [relationshipOptionId, setRelationshipOptionId] = useState(''); | ||
| const [nodeOptionId, setNodeOptionId] = useState(''); | ||
| const edgeMap = useMemo(() => new Map(edges.map((e) => [String(e.id), e])), [edges]); | ||
| const nodeInstanceMap = useMemo(() => new Map(nodes.map((n) => [String(n.id), n])), [nodes]); | ||
|
Comment on lines
+162
to
+163
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1 — last-wins Map construction. A repeated relationship id therefore opens the later edge while labels still describe the first node pair. Build both maps with first-set-wins (see |
||
| const nodeMap = useMemo(() => { | ||
| const map = new Map<string, string>(); | ||
| for (const node of nodes) { | ||
|
|
@@ -202,7 +204,7 @@ export default function NetworkGraph() { | |
| }; | ||
|
|
||
| const selectEdge = (edgeId: number | string) => { | ||
| const edge = edges.find((candidate) => graphIdEquals(candidate.id, edgeId)); | ||
| const edge = edgeMap.get(String(edgeId)); | ||
| if (!edge) return; | ||
| setRelationshipOptionId(String(edge.id)); | ||
| setNodeOptionId(''); | ||
|
|
@@ -213,7 +215,7 @@ export default function NetworkGraph() { | |
| const selectNode = (nodeId: number | string) => { | ||
| setRelationshipOptionId(''); | ||
| setNodeOptionId(String(nodeId)); | ||
| setSelectedGraphDetail(`선택된 노드: ${findNodeLabel(nodes, nodeId)}`); | ||
| setSelectedGraphDetail(`선택된 노드: ${nodeMap.get(String(nodeId)) ?? String(nodeId)}`); | ||
| setGraphActionStatus('그래프에서 노드를 선택했습니다.'); | ||
| }; | ||
|
|
||
|
|
@@ -262,7 +264,7 @@ export default function NetworkGraph() { | |
| network.destroy(); | ||
| }; | ||
| } | ||
| }, [nodes, edges, nodeMap]); | ||
| }, [nodes, edges, nodeMap, edgeMap, nodeInstanceMap]); | ||
|
|
||
| const nodeLabels = useMemo(() => { | ||
| return nodes | ||
|
|
@@ -303,7 +305,7 @@ export default function NetworkGraph() { | |
| if (!isGraphId(node.id)) return; | ||
| setRelationshipOptionId(''); | ||
| setNodeOptionId(String(node.id)); | ||
| setSelectedGraphDetail(`선택된 노드: ${findNodeLabel(nodes, node.id)}`); | ||
| setSelectedGraphDetail(`선택된 노드: ${nodeMap.get(String(node.id)) ?? String(node.id)}`); | ||
| setGraphActionStatus(status); | ||
| networkRef.current?.selectNodes?.([node.id]); | ||
| networkRef.current?.fit?.({ nodes: [node.id], animation: false }); | ||
|
|
@@ -315,13 +317,13 @@ export default function NetworkGraph() { | |
| }; | ||
|
|
||
| const handleRelationshipOptionChange = (value: string) => { | ||
| const edge = edges.find((candidate) => String(candidate.id) === value); | ||
| const edge = edgeMap.get(value); | ||
| if (!edge) return; | ||
| selectRelationship(edge, '선택한 관계를 열었습니다.'); | ||
| }; | ||
|
|
||
| const handleNodeOptionChange = (value: string) => { | ||
| const node = nodes.find((candidate) => String(candidate.id) === value); | ||
| const node = nodeInstanceMap.get(value); | ||
| if (!node) return; | ||
| selectGraphNode(node, '선택한 노드를 열었습니다.'); | ||
| }; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| # NetworkGraph constant-time lookup plan | ||
|
|
||
| 1. Pre-compute `edgeMap` and `nodeInstanceMap` with `useMemo`, and keep the existing `nodeMap` as the authoritative node-label lookup for rendered selections. | ||
| - `selectEdge` uses `edgeMap.get(String(edgeId))`. | ||
| - `selectNode` uses `nodeMap.get(String(nodeId))` with the node identifier as the no-entry fallback. | ||
| - `selectGraphNode` uses `nodeMap.get(String(node.id))` with the node identifier as the no-entry fallback. | ||
| - `handleRelationshipOptionChange` uses `edgeMap.get(value)`. | ||
| - `handleNodeOptionChange` uses `nodeInstanceMap.get(value)`. | ||
| - Selection handlers must not fall back to `Array.prototype.find()` or `findNodeLabel()` scans. | ||
|
|
||
| 2. Verify the exact branch head from `frontend/` with these commands: | ||
|
|
||
| ```bash | ||
| pnpm test -- src/components/NetworkGraph.test.tsx src/components/NetworkGraph.map-lookup.test.ts | ||
| pnpm exec eslint src/components/NetworkGraph.tsx src/components/NetworkGraph.test.tsx src/components/NetworkGraph.map-lookup.test.ts | ||
| pnpm typecheck | ||
| pnpm build | ||
| ``` | ||
|
|
||
| 3. Keep the pull request open until the unchanged exact head has terminal-success required checks, all addressed review threads are resolved, and protected-branch review requirements are satisfied without bypass. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1 — this contract never mounts the graph.
readFileSync+.toContain('edgeMap.get')will stay green ifselectNode/selectEdgestop updating the Korean detail or the<select>value.Keep the source scan if useful as a lint, but add a jsdom case that registers the vis-network handlers and fires mixed numeric/string ids (
101,"recipient-1",7). #1382 adds that coverage.