Skip to content

feat(dag): 6-node flow diagram + relations cycle detection fix #23

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.

Sign up for GitHub

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

Open
villainscode wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

feat(dag): 6-node flow diagram + relations cycle detection fix #23

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
81 changes: 33 additions & 48 deletions apps/web/lib/server/services/relation-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,26 +57,28 @@ export function createRelationService(db: Db) {
}

/**
* Batch-preload cycle detection for prev/next chain.
* Loads all 'next' relations once, then traverses in-memory.
* Starting from `startId`, follows `next` relations and checks if we reach `targetId`.
* Also checks the reverse direction: from `targetId` following `next` relations.
* Cycle detection for the prev/next chain.
*
* Builds the FUTURE 'next' graph that will exist after `setRelations` finishes:
* 1. Load all current 'next' edges and drop the ones adjacent to `docId`.
* Those edges are about to be removed by the deletion step (source=docId
* for any type, or target=docId for type IN ('prev','next')), so they
* must not influence detection — otherwise re-saving an existing prev /
* next value triggers a false positive via the bidirectional pair.
* 2. Add the new 'next' edges introduced by this update:
* - data.next → direct edge docId → data.next
* - data.prev → bidirectional edge data.prev → docId
*
* Then walk the chain starting from `docId`. If it returns to `docId`,
* a cycle exists.
*/
async function detectCycle(
docId: string,
targetId: string,
direction: 'prev' | 'next',
data: SetRelationsInput,
): Promise<boolean> {
const numDocId = Number(docId);
const numTargetId = Number(targetId);
const startNode = direction === 'prev' ? numDocId : numTargetId;
const searchFor = direction === 'prev' ? numTargetId : numDocId;

// Batch preload: load all type='next' relations in a single query.
// Exclude edges that involve the current document — those will be cleared
// before the new relations are inserted, so including them produces false
// positives (e.g. re-saving an existing prev/next value triggers a cycle).
const nextRelations = await db

const surviving = await db
.select({
sourceId: documentRelations.sourceId,
targetId: documentRelations.targetId,
Expand All @@ -90,34 +92,25 @@ export function createRelationService(db: Db) {
),
);

// Build Map<sourceId, targetId> for O(1) lookup
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;
}
Comment on lines 95 to 113
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

현재 사이클 감지 로직은 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);
      }
    }


return false;
}

Expand Down Expand Up @@ -170,19 +163,11 @@ export function createRelationService(db: Db) {
throw badRequest('INVALID_RELATION', 'prev and next cannot point to the same document');
}

// Cycle detection BEFORE clearing existing relations
// (clearing first removes the edges needed for detection)
// Temporarily exclude current doc's outgoing relations to avoid false positives
// from its own existing prev/next, but keep other docs' relations intact.
if (data.prev) {
const hasCycle = await detectCycle(docId, data.prev, 'prev');
if (hasCycle) {
throw badRequest('CIRCULAR_REFERENCE', 'Setting this relation would create a circular reference');
}
}

if (data.next) {
const hasCycle = await detectCycle(docId, data.next, 'next');
// Cycle detection BEFORE clearing existing relations.
// detectCycle simulates the future graph (current edges minus the ones
// adjacent to this doc, plus the new prev/next edges this update introduces).
if (data.prev || data.next) {
const hasCycle = await detectCycle(docId, data);
if (hasCycle) {
throw badRequest('CIRCULAR_REFERENCE', 'Setting this relation would create a circular reference');
}
Expand Down