feat(dag): 6-node flow diagram + relations cycle detection fix#23
feat(dag): 6-node flow diagram + relations cycle detection fix#23villainscode wants to merge 1 commit intodevelopfrom
Conversation
기존 detectCycle은 현재 그래프만 보고 새 엣지를 반영하지 않아 두 가지 결함이 있었다. 1. False positive: 같은 prev/next 값을 재저장할 때 기존 양방향 짝 (B→A type='next') 때문에 CIRCULAR_REFERENCE(400)가 발생했다. 증상: 문서 상세에서 연관문서만 추가/삭제 후 저장 시 400 에러. 2. False negative: prev와 next를 동시에 설정해 미래 그래프에 진짜 사이클이 만들어지는 케이스(예: A.next=C, A.prev=B + 기존 C→…→B) 를 감지하지 못했다. setRelations가 적용되기 직전 상태(현재 엣지 - docId 인접 엣지 + 새 prev/next 엣지) 그래프를 메모리에 구성한 뒤, docId에서 'next' 체인을 따라가 다시 docId로 돌아오는지 확인하는 단일 traversal로 재작성했다. 호출부도 두 번에서 한 번으로 단순화했다.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the cycle detection logic in the relation service to simulate the future state of the graph before applying updates. The implementation simplifies the process by combining forward and reverse checks into a single traversal starting from the modified document. However, a high-severity issue was identified: the current logic uses a Map<number, number>, which assumes a linear chain and cannot handle branching. If multiple documents reference the same document as their 'prev', the cycle detection will fail to explore all paths. The reviewer recommends using a Map<number, number[]> and a depth-first search (DFS) to ensure all possible paths are checked for circular references.
| const nextMap = new Map<number, number>(); | ||
| for (const rel of nextRelations) { | ||
| for (const rel of surviving) { | ||
| nextMap.set(rel.sourceId, rel.targetId); | ||
| } | ||
| if (data.next) { | ||
| nextMap.set(numDocId, Number(data.next)); | ||
| } | ||
| if (data.prev) { | ||
| nextMap.set(Number(data.prev), numDocId); | ||
| } | ||
|
|
||
| // Forward traversal: startNode -> ... -> searchFor? | ||
| let current: number | null = nextMap.get(numDocId) ?? null; | ||
| const visited = new Set<number>(); | ||
| let current: number | null = startNode; | ||
| while (current !== null) { | ||
| if (visited.has(current)) break; | ||
| if (current === numDocId) return true; | ||
| if (visited.has(current)) return false; | ||
| visited.add(current); | ||
| const next: number | null = nextMap.get(current) ?? null; | ||
| if (next === searchFor) return true; | ||
| current = next; | ||
| } | ||
|
|
||
| // Reverse traversal: searchFor -> ... -> startNode? | ||
| const visited2 = new Set<number>(); | ||
| let current2: number | null = searchFor; | ||
| while (current2 !== null) { | ||
| if (visited2.has(current2)) break; | ||
| visited2.add(current2); | ||
| const next: number | null = nextMap.get(current2) ?? null; | ||
| if (next === startNode) return true; | ||
| current2 = next; | ||
| current = nextMap.get(current) ?? null; | ||
| } |
There was a problem hiding this comment.
현재 사이클 감지 로직은 Map<number, number>를 사용하여 각 문서가 최대 하나의 'next' 관계만 가진다고 가정하고 있습니다. 하지만 데이터베이스 스키마와 setRelations 로직상 여러 문서가 동일한 문서를 'prev'로 지정할 수 있으며, 이 경우 해당 문서는 여러 개의 'next' 관계를 가지게 됩니다(분기 발생). 이러한 경우 현재 로직은 일부 경로를 누락하여 사이클을 감지하지 못할 수 있습니다. Map<number, number[]>와 DFS(깊이 우선 탐색)를 사용하여 모든 가능한 경로를 탐색하도록 수정해야 합니다.
const nextMap = new Map<number, number[]>();
const addEdge = (s: number, t: number) => {
const edges = nextMap.get(s) ?? [];
edges.push(t);
nextMap.set(s, edges);
};
for (const rel of surviving) addEdge(rel.sourceId, rel.targetId);
if (data.next) addEdge(numDocId, Number(data.next));
if (data.prev) addEdge(Number(data.prev), numDocId);
const stack = [...(nextMap.get(numDocId) ?? [])];
const visited = new Set<number>();
while (stack.length > 0) {
const curr = stack.pop()!;
if (curr === numDocId) return true;
if (!visited.has(curr)) {
visited.add(curr);
const targets = nextMap.get(curr);
if (targets) stack.push(...targets);
}
}1 participant
Summary
f4bccb4)bbd81df)be92840) + 사이클 감지 로직 재작성 (a9f4e5c)eab2906)175ccb5)사이클 감지 수정 (a9f4e5c)
문제
PUT /api/v1/workspaces/:wsId/documents/:docId/relations저장 시 400 (CIRCULAR_REFERENCE)detectCycle이 (1) 현재 그래프만 보고 새 엣지를 시뮬레이션하지 않아 prev+next 동시 설정 시 진짜 미래 사이클을 놓침, (2) 양방향 짝 엣지 때문에 동일 prev/next 재저장 시 false positive 발생해결
detectCycle을 미래 그래프 시뮬레이션 방식으로 재작성:setRelations가 삭제할 엣지(source=docId / target=docId 의 next)를 SQL 단계에서 제외하고, 새 prev/next 엣지를 in-memory에 주입한 뒤 docId에서 'next' 체인을 따라 docId로 돌아오는지 단일 traversal로 검사apps/web/lib/server/services/relation-service.ts)Test plan
pnpm --filter @markflow/web test(96/96 통과)pnpm --filter @markflow/web exec tsc --noEmit통과🤖 Generated with Claude Code