Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,13 @@ in this repo.
topic components, or label evidence by a bare document, model, topic, rank,
label, or display value.
- When reviews find public/private identifier leaks, stale API fixture shapes, or recurring bug patterns, update tests, frontend mocks, E2E mocks, README examples, architecture docs, and explicitly record the anti-pattern in `AGENTS.md` so the same bug pattern does not reappear in copied examples.
- Memoized id-to-record Maps must be first-wins (`if (!map.has(key)) map.set(...)`).
`new Map(items.map((item) => [String(item.id), item]))` is last-wins and
desynchronizes first-wins label maps from the selected node or edge when
ids collide. Keep a rendered selection test that repeats an id and asserts
the first instance is the one opened. Do not treat a source-substring scan
as the only selection-path contract; fire the vis-network `selectNode` /
`selectEdge` callbacks with mixed numeric and string ids.
- When reviews find missing browser security headers or tabnabbing hardening,
update both backend header tests and frontend link tests. Global backend
responses must include `Referrer-Policy`, and `target="_blank"` links must
Expand Down
66 changes: 66 additions & 0 deletions frontend/src/components/NetworkGraph.map-lookup.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
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(");
});

it("builds edge and node instance maps as first-wins lookups", () => {
expect(networkGraphSource).toContain("firstGraphEntryById(edges");
expect(networkGraphSource).toContain("firstGraphEntryById(nodes");
expect(networkGraphSource).not.toMatch(/new Map\((edges|nodes)\.map\(/);
});
});
110 changes: 110 additions & 0 deletions frontend/src/components/NetworkGraph.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,116 @@ describe("NetworkGraph", () => {
expect(mountedContainer.textContent).toContain("그래프 맞춤 완료");
});

function registeredGraphHandler(eventName: string) {
const handler = onMock.mock.calls.find((call) => call[0] === eventName)?.[1];
if (typeof handler !== "function") {
throw new Error(`${eventName} handler was not registered.`);
}
return handler as (event: {
nodes?: Array<number | string>;
edges?: Array<number | string>;
}) => void;
}

it("resolves vis-network selection events for mixed numeric and string ids", async () => {
const fetchMock = vi.fn(() =>
Promise.resolve(
jsonResponse({
nodes: [
{ id: 101, label: "발신자", title: "PM" },
{ id: "recipient-1", label: "수신자", title: "Owner" },
],
edges: [
{ id: 7, from: 101, to: "recipient-1", title: "메일 1건" },
],
}),
),
);
vi.stubGlobal("fetch", fetchMock);

await renderGraph();
await flushAsyncWork();

const mountedContainer = getMountedContainer();
const selectNode = registeredGraphHandler("selectNode");
const selectEdge = registeredGraphHandler("selectEdge");

await act(async () => {
selectNode({ nodes: [101] });
});

const nodeSelect = mountedContainer.querySelector('select[aria-label="노드 선택"]');
expect(nodeSelect).toBeInstanceOf(HTMLSelectElement);
expect((nodeSelect as HTMLSelectElement).value).toBe("101");
expect(mountedContainer.textContent).toContain("선택된 노드: 발신자");
expect(mountedContainer.textContent).toContain("그래프에서 노드를 선택했습니다.");

await act(async () => {
selectEdge({ edges: [7] });
});

const relationshipSelect = mountedContainer.querySelector('select[aria-label="관계 선택"]');
expect(relationshipSelect).toBeInstanceOf(HTMLSelectElement);
expect((relationshipSelect as HTMLSelectElement).value).toBe("7");
expect(mountedContainer.textContent).toContain("선택된 관계: 발신자 -> 수신자 (메일 1건)");
expect(mountedContainer.textContent).toContain("그래프에서 관계를 선택했습니다.");
});

it("keeps the first edge instance when duplicate relationship ids collide", async () => {
const fetchMock = vi.fn(() =>
Promise.resolve(
jsonResponse({
nodes: [
{ id: "sender-1", label: "김지현", title: "PM" },
{ id: "recipient-1", label: "사용자", title: "Owner" },
{ id: "calendar-1", label: "일정", title: "Schedule" },
],
edges: [
{ id: "rel-shared", from: "sender-1", to: "recipient-1", title: "메일 2건" },
{ id: "rel-shared", from: "sender-1", to: "calendar-1", title: "일정 후보 1건" },
],
}),
),
);
vi.stubGlobal("fetch", fetchMock);

await renderGraph();
await flushAsyncWork();

const mountedContainer = getMountedContainer();
const selectEdge = registeredGraphHandler("selectEdge");

await act(async () => {
selectEdge({ edges: ["rel-shared"] });
});

expect(mountedContainer.textContent).toContain("선택된 관계: 김지현 -> 사용자 (메일 2건)");
expect(mountedContainer.textContent).not.toContain("선택된 관계: 김지현 -> 일정 (일정 후보 1건)");
expect(selectEdgesMock).not.toHaveBeenCalled();

const relationshipSelect = mountedContainer.querySelector('select[aria-label="관계 선택"]');
expect(relationshipSelect).toBeInstanceOf(HTMLSelectElement);

await act(async () => {
if (relationshipSelect instanceof HTMLSelectElement) {
relationshipSelect.value = "rel-shared";
relationshipSelect.dispatchEvent(new Event("change", { bubbles: true }));
}
});

expect(selectEdgesMock).toHaveBeenCalledWith(["rel-shared"]);
expect(fitMock).toHaveBeenCalledWith({
nodes: ["sender-1", "recipient-1"],
animation: false,
});
expect(fitMock).not.toHaveBeenCalledWith({
nodes: ["sender-1", "calendar-1"],
animation: false,
});
expect(mountedContainer.textContent).toContain("선택된 관계: 김지현 -> 사용자 (메일 2건)");
expect(mountedContainer.textContent).toContain("선택한 관계를 열었습니다.");
});

it("normalizes backend source target edges before rendering the graph", async () => {
const fetchMock = vi.fn(() =>
Promise.resolve(
Expand Down
47 changes: 36 additions & 11 deletions frontend/src/components/NetworkGraph.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,31 @@ function findNodeLabel(nodes: Node[], id: number | string) {
return String(node?.label ?? id);
}

/**
* Index graph records by public id, keeping the first instance.
*
* `new Map(items.map((item) => [String(item.id), item]))` is last-wins and
* desynchronizes first-wins label maps from the selected node or edge when
* the API repeats an id. The previous `.find()` selection path was first-wins.
*/
function firstGraphEntryById<T>(
items: readonly T[],
readId: (item: T) => unknown,
): Map<string, T> {
const map = new Map<string, T>();
for (const item of items) {
const rawId = readId(item);
if (!isGraphId(rawId)) {
continue;
}
const key = String(rawId);
if (!map.has(key)) {
map.set(key, item);
}
}
return map;
}

function describeEdge(edge: Edge, nodes: Node[], nodeMap?: Map<string | number, string>) {
let fromLabel, toLabel;
if (nodeMap) {
Expand Down Expand Up @@ -159,6 +184,8 @@ export default function NetworkGraph() {
const [graphActionStatus, setGraphActionStatus] = useState('그래프 준비 완료');
const [relationshipOptionId, setRelationshipOptionId] = useState('');
const [nodeOptionId, setNodeOptionId] = useState('');
const edgeMap = useMemo(() => firstGraphEntryById(edges, (edge) => edge.id), [edges]);
const nodeInstanceMap = useMemo(() => firstGraphEntryById(nodes, (node) => node.id), [nodes]);
const nodeMap = useMemo(() => {
const map = new Map<string, string>();
for (const node of nodes) {
Expand Down Expand Up @@ -202,7 +229,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('');
Expand All @@ -213,7 +240,6 @@ export default function NetworkGraph() {
const selectNode = (nodeId: number | string) => {
setRelationshipOptionId('');
setNodeOptionId(String(nodeId));
// ⚡ Bolt: Replace O(N) findNodeLabel array lookup with O(1) nodeMap lookup
setSelectedGraphDetail(`선택된 노드: ${nodeMap.get(String(nodeId)) ?? String(nodeId)}`);
setGraphActionStatus('그래프에서 노드를 선택했습니다.');
};
Expand Down Expand Up @@ -263,7 +289,7 @@ export default function NetworkGraph() {
network.destroy();
};
}
}, [nodes, edges, nodeMap]);
}, [nodes, edges, nodeMap, edgeMap]);

const nodeLabels = useMemo(() => {
return nodes
Expand All @@ -274,20 +300,20 @@ export default function NetworkGraph() {

const firstEdge = edges[0] ?? null;
const relationshipOptions = useMemo(() => {
return edges.slice(0, 5).map((edge, index) => ({
return Array.from(edgeMap.values()).slice(0, 5).map((edge, index) => ({
edge,
id: String(edge.id),
label: `관계 ${index + 1}: ${describeEdge(edge, nodes, nodeMap)}`,
}));
}, [edges, nodes, nodeMap]);
}, [edgeMap, nodes, nodeMap]);

const nodeOptions = useMemo(() => {
return nodes.slice(0, 8).map((node) => ({
return Array.from(nodeInstanceMap.values()).slice(0, 8).map((node) => ({
id: String(node.id),
label: `노드: ${String(node.label ?? node.id)}`,
node,
}));
}, [nodes]);
}, [nodeInstanceMap]);

const selectRelationship = (edge: Edge, status: string) => {
setRelationshipOptionId(String(edge.id));
Expand All @@ -304,8 +330,7 @@ export default function NetworkGraph() {
if (!isGraphId(node.id)) return;
setRelationshipOptionId('');
setNodeOptionId(String(node.id));
// ⚡ Bolt: Use direct property access instead of O(N) findNodeLabel array lookup
setSelectedGraphDetail(`선택된 노드: ${String(node.label ?? 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 });
Expand All @@ -317,13 +342,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, '선택한 노드를 열었습니다.');
};
Expand Down
21 changes: 21 additions & 0 deletions plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# 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.
- `edgeMap` and `nodeInstanceMap` are first-wins, matching `nodeMap` and the previous `.find()` path. Last-wins `new Map(items.map(...))` construction is rejected.

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.
Loading