Skip to content

feat(frontend): add relationship-aware ERD layout - #856

Open
seonghobae wants to merge 2 commits into
mainfrom
codex/relationship-aware-layout
Open

feat(frontend): add relationship-aware ERD layout#856
seonghobae wants to merge 2 commits into
mainfrom
codex/relationship-aware-layout

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Outcome

Imported snapshots and the explicit toolbar action now use one deterministic relationship-aware Dagre layout instead of the alphabetical fixed grid.

Changes

  • pin @dagrejs/dagre exactly at 3.1.0 in the canonical npm manifest and lockfile;
  • add a pure computeDagreLayout contract with LR/TB directions, stable insertion order, measured rectangle support, top-left coordinate conversion, and no input mutation;
  • ignore dangling endpoints and support cycles and disconnected components;
  • preserve prior coordinates when the layout engine throws or returns missing/non-finite geometry;
  • apply the same contract during snapshot conversion and toolbar auto-layout;
  • preserve the existing exact one-step undo behavior, including nodes added after layout;
  • document status, recovery, limitations, research traceability, and changelog impact.

No backend, API, database, workflow, DSN, TLS, redirect, or deployment behavior changes.

Test-first evidence

RED was observed at the intended module boundary because ./dagreLayout did not exist. On the exact published tree, using Node 26.0.0 and npm 11.19.0:

  • lifecycle-disabled clean install: passed (npm ci --ignore-scripts);
  • typecheck: passed;
  • complete frontend suite: 28 files / 207 tests passed;
  • owned layout module: 100% statements, branches, functions, and lines (54 statements, 43 branches);
  • repository aggregate coverage remains its existing baseline: 99.8% statements / 99.48% branches;
  • production build: passed;
  • production dependency audit at Moderate and above: 0 vulnerabilities;
  • git diff --check: passed.

The repository-required CodeGraph executable was unavailable in this runner; rg, call-site inspection, focused tests, and the complete suite were used instead. Exact-head CI, Security Scan, Semgrep, zero valid unresolved findings, and qualifying independent current-head approval remain mandatory before merge.

Closes #755.

Summary by CodeRabbit

  • 새로운 기능

    • 관계를 반영한 ERD 자동 정렬을 지원합니다. 순환·분리 그래프와 누락된 연결도 안전하게 처리합니다.
    • 관계 자동 추론, 전체 노드 삭제, 테이블 복제 기능을 추가했습니다.
    • DBML 및 Data Dictionary 형식으로 내보낼 수 있습니다.
    • 공유 내보내기 시 민감한 정보가 자동으로 제거됩니다.
  • 버그 수정

    • 함수 인덱스의 중복 오탐을 수정했습니다.
  • 문서 및 테스트

    • ERD 자동 정렬 동작과 제한 사항을 문서화하고 관련 테스트를 보강했습니다.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@seonghobae, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 11 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1ff43bfa-7923-4dee-b886-a4ca230742d5

📥 Commits

Reviewing files that changed from the base of the PR and between e7110d9 and a0f8cad.

📒 Files selected for processing (2)
  • frontend/src/erd/dagreLayout.test.ts
  • frontend/src/erd/dagreLayout.ts
📝 Walkthrough

Walkthrough

관계 인식 Dagre 레이아웃을 추가했습니다. 노드 크기와 외래 키 관계를 사용해 LR 또는 TB 좌표를 계산합니다. 순환 그래프, 분리 그래프, dangling edge와 실패 상황을 처리합니다. 스냅샷 변환과 툴바 자동 정렬에 새 레이아웃을 적용했습니다.

Changes

ERD 레이아웃

Layer / File(s) Summary
Dagre 레이아웃 계산
frontend/src/erd/dagreLayout.ts, frontend/src/erd/dagreLayout.test.ts
computeDagreLayout이 노드 크기와 유효한 외래 키 edge를 사용해 결정적인 LR/TB 레이아웃을 계산합니다. 순환·분리 그래프와 dangling edge를 처리합니다. 유효하지 않은 결과나 예외가 발생하면 기존 좌표와 입력 객체를 보존합니다.
ERD 자동 정렬 연동
frontend/package.json, frontend/src/erd/convert.ts, frontend/src/App.tsx, frontend/src/App.coverage.test.tsx, frontend/src/erd/convert.test.ts
스냅샷 변환과 툴바 자동 정렬이 computeDagreLayout(nodes, edges)를 사용합니다. 테스트는 외래 키 방향, 위치 변경, 기존 좌표의 1단계 되돌리기를 검증합니다.
레이아웃 계약과 릴리스 기록
docs/doctoring/relationship-aware-erd-layout.md, CHANGELOG.md, frontend/CHANGELOG.md
레이아웃의 입력, 크기 계산, 좌표 변환, 결정성, 실패 처리, 테스트 범위와 Dagre 의존성을 문서화합니다.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Toolbar
  participant App
  participant computeDagreLayout
  participant ReactFlow
  Toolbar->>App: 자동 정렬 실행
  App->>computeDagreLayout: nodes와 edges 전달
  computeDagreLayout-->>App: 관계 기반 좌표 반환
  App->>ReactFlow: 정렬된 노드 표시
  Toolbar->>App: 되돌리기 실행
  App->>ReactFlow: 이전 좌표 복원
Loading
🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (1 warning, 2 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Linked Issues check ❓ Inconclusive 구현과 테스트는 대부분의 요구사항을 충족하지만, frontend/package-lock.json은 경로 필터로 제외되어 잠금 상태를 검증할 수 없습니다. frontend/package-lock.json을 검토하여 @dagrejs/dagre 3.1.0 잠금과 관련 설치 검증 결과를 확인하십시오.
Out of Scope Changes check ❓ Inconclusive 검토된 변경은 레이아웃 구현, 테스트, 문서, 변경 로그, 의존성 추가 범위에 있습니다. 단, 제외된 잠금 파일의 범위는 확인할 수 없습니다. frontend/package-lock.json을 검토하여 무관한 의존성 변경이나 금지된 버전 변경이 없는지 확인하십시오.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 고정 격자를 관계 인식 ERD 레이아웃으로 대체하는 PR의 주요 변경 사항을 정확하고 간결하게 설명합니다.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/relationship-aware-layout

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

Please review unchanged exact head e7110d9064f8f3f6f32ae0e22d1bf6a52ed7ecc0. This draft is the bounded issue #755 relationship-aware layout slice: exact Dagre 3.1.0 pin, one pure LR/TB coordinate contract, snapshot and toolbar integration, failure preservation, exact one-step undo, focused 100% owned module coverage, and transparent aggregate coverage evidence. Please treat predecessor or other layout branches as non-transferable evidence.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

@seonghobae I will review the requested exact head e7110d9064f8f3f6f32ae0e22d1bf6a52ed7ecc0. I will evaluate this bounded #755 slice independently. I will not use predecessor or other layout branches as evidence.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@seonghobae
seonghobae marked this pull request as ready for review August 11, 2026 12:38

Copy link
Copy Markdown
Collaborator Author

@opencode-agent review

Please independently review unchanged exact head e7110d9064f8f3f6f32ae0e22d1bf6a52ed7ecc0 against issue #755. CI, Security Scan, and SAST Semgrep are terminal-success on this head; there are currently no review threads. Focus on deterministic Dagre semantics, React Flow coordinate/dimension compatibility, failure preservation, snapshot and toolbar parity, exact undo, lockfile scope, tests, and documentation. Do not transfer evidence from predecessor branches.

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

Retry after the bot-reported review window elapsed. Review only unchanged exact head e7110d9064f8f3f6f32ae0e22d1bf6a52ed7ecc0 against issue #755. Protected main remains 72afe6db712b145baaba084f64a1ff4fb36d9fd0; CI, Security Scan, and SAST Semgrep are terminal-success; there are zero review threads. Do not treat the CodeRabbit commit status or the earlier rate-limited attempt as review evidence.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

@seonghobae I will review pull request #856 at exact head e7110d9064f8f3f6f32ae0e22d1bf6a52ed7ecc0 against protected main 72afe6db712b145baaba084f64a1ff4fb36d9fd0.

I will review the #755 scope independently. I will not use the earlier rate-limited attempt, CodeRabbit status, or predecessor branches as evidence.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

The bot-reported countdown has now elapsed. Review only unchanged exact head e7110d9064f8f3f6f32ae0e22d1bf6a52ed7ecc0 against protected main 72afe6db712b145baaba084f64a1ff4fb36d9fd0 and issue #755. Exact-head CI, Security Scan, and SAST Semgrep remain terminal-success with zero review threads. If capacity is still unavailable, report that state; no further retry will be made in this run.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

@seonghobae I will review pull request #856 at exact head e7110d9064f8f3f6f32ae0e22d1bf6a52ed7ecc0 against 72afe6db712b145baaba084f64a1ff4fb36d9fd0 and issue #755. I will assess this head independently and will not use predecessor branches or the earlier rate-limited attempts as evidence.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
docs/doctoring/relationship-aware-erd-layout.md (1)

48-52: 🔒 Security & Privacy | 🔵 Trivial

필수 Security Scan 게이트 결과를 병합 기준으로 확인하세요.

제공된 컨텍스트에는 osv-scan, Medium 이상 diff-scoped dependency-review, repo-wide trivy-fs의 성공 로그나 SARIF가 없습니다. 병합 전에 세 결과가 merge ref 기준으로 통과했는지 확인하세요. 취약점 결과가 있으면 Medium 이상 항목을 수정하고 게이트를 완화하지 마세요.

As per coding guidelines, 모든 pull request는 osv-scan, Medium 이상 diff-scoped dependency-review, CRITICAL/HIGH/MEDIUM 대상 repo-wide trivy-fs 게이트를 통과해야 합니다.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/doctoring/relationship-aware-erd-layout.md` around lines 48 - 52, Before
merging, verify that the merge ref passes the required osv-scan,
Medium-or-higher diff-scoped dependency-review, and repo-wide trivy-fs scans,
with successful logs or SARIF recorded. Address any Medium-or-higher findings
without weakening or bypassing these gates.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@docs/doctoring/relationship-aware-erd-layout.md`:
- Around line 48-52: Before merging, verify that the merge ref passes the
required osv-scan, Medium-or-higher diff-scoped dependency-review, and repo-wide
trivy-fs scans, with successful logs or SARIF recorded. Address any
Medium-or-higher findings without weakening or bypassing these gates.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e8ac1d9a-af94-4122-ae87-f906c73f4487

📥 Commits

Reviewing files that changed from the base of the PR and between 72afe6d and e7110d9.

⛔ Files ignored due to path filters (1)
  • frontend/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (10)
  • CHANGELOG.md
  • docs/doctoring/relationship-aware-erd-layout.md
  • frontend/CHANGELOG.md
  • frontend/package.json
  • frontend/src/App.coverage.test.tsx
  • frontend/src/App.tsx
  • frontend/src/erd/convert.test.ts
  • frontend/src/erd/convert.ts
  • frontend/src/erd/dagreLayout.test.ts
  • frontend/src/erd/dagreLayout.ts

Copy link
Copy Markdown
Collaborator Author

Verified the sole CodeRabbit nit against unchanged exact head e7110d9064f8f3f6f32ae0e22d1bf6a52ed7ecc0 and protected main 72afe6db712b145baaba084f64a1ff4fb36d9fd0.

Security Scan run 31491964574 is terminal-success, with each required job independently terminal-success:

  • trivy-fs job 93780353250: filesystem scan, required SARIF output, finding report, and SARIF upload all succeeded;
  • dependency-review job 93780353253: support check and dependency review succeeded;
  • osv-scan job 93780353325: base/head scans, required output, PR-introduced finding comparison, comprehensive clean-SARIF marking, SARIF upload, and debug-artifact upload all succeeded.

No Medium-or-higher finding was reported, no gate was weakened, and no product or documentation change is warranted for this evidence-only finding. CI and SAST Semgrep also remain terminal-success; zero review threads exist. Qualifying independent non-author approval remains required before merge.

Copy link
Copy Markdown
Collaborator Author

Exact-head fail-closed layout remediation is published at a0f8cadf2fc818b8acd0c666a192e64fcfaf1ce1.

The current implementation violated the documented/issue contract when Dagre returned valid geometry for one node and invalid geometry for another: the valid node moved while the invalid node stayed put, creating a partial layout. RED reproduced the mixed state with the valid node moving from (12,34) to (310,240). The fix now validates all computed top-left coordinates before applying any, and preserves every prior coordinate on missing, non-positive, non-finite, overflowed, or thrown geometry.

Verification on the published tree:

  • focused Dagre suite: 12 passed;
  • dagreLayout.ts: 100% statements, branches, functions, and lines;
  • complete frontend: 28 files / 209 tests passed;
  • complete coverage: 99.80% statements, 99.48% branches, 99.69% functions, 99.92% lines;
  • TypeScript typecheck and production build: passed;
  • git diff --check: clean.

Local runtime was Node 24.14 while the repository requires Node 26, so exact-head CI remains authoritative.

@coderabbitai review
@opencode-agent @cwl-noema-review Please review this unchanged exact head and provide a qualifying formal non-author verdict. Predecessor and status-only evidence do not transfer.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='a0f8cadf2fc818b8acd0c666a192e64fcfaf1ce1'
base='72afe6db712b145baaba084f64a1ff4fb36d9fd0'

git cat-file -e "${head}^{commit}"
git cat-file -e "${base}^{commit}"

echo '=== Exact head metadata ==='
git show -s --format='%H%n%P%n%s' "$head"

echo '=== Changed paths from protected main ==='
git diff --name-status "$base" "$head"

echo '=== Remediation commit diff ==='
git diff --check "${head}^" "$head"
git diff --unified=80 "${head}^" "$head" -- frontend/src/erd/dagreLayout.ts frontend/src/erd/dagreLayout.test.ts

echo '=== Layout module outline ==='
ast-grep outline frontend/src/erd/dagreLayout.ts --items all --view expanded

echo '=== Layout test outline ==='
ast-grep outline frontend/src/erd/dagreLayout.test.ts --items all --view expanded

echo '=== Integration module outlines ==='
ast-grep outline frontend/src/erd/convert.ts --items all --view expanded
ast-grep outline frontend/src/App.tsx --items all --view expanded

echo '=== Dagre dependency declarations ==='
rg -n -C2 '"`@dagrejs/dagre`"|`@dagrejs/dagre`@' frontend/package.json frontend/package-lock.json 2>/dev/null || true

echo '=== Layout call sites ==='
rg -n -C3 '\bcomputeDagreLayout\s*\(' frontend/src

echo '=== Full changed-file diff statistics ==='
git diff --stat "$base" "$head"

Length of output: 22707


🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='a0f8cadf2fc818b8acd0c666a192e64fcfaf1ce1'
base='72afe6db712b145baaba084f64a1ff4fb36d9fd0'

echo '=== Complete layout module ==='
git show "${head}:frontend/src/erd/dagreLayout.ts" | nl -ba

echo '=== Complete layout test ==='
git show "${head}:frontend/src/erd/dagreLayout.test.ts" | nl -ba

echo '=== Snapshot conversion context ==='
git show "${head}:frontend/src/erd/convert.ts" | nl -ba | sed -n '25,180p'

echo '=== Toolbar layout and undo context ==='
git show "${head}:frontend/src/App.tsx" | nl -ba | sed -n '410,505p'
git show "${head}:frontend/src/App.tsx" | nl -ba | sed -n '1060,1160p'
rg -n -C4 'autoLayout|Auto layout|layout|undo|Undo|history|setNodes\(next\)' frontend/src/App.tsx

echo '=== Application coverage test context ==='
git show "${head}:frontend/src/App.coverage.test.tsx" | nl -ba | sed -n '1,260p'

echo '=== Conversion test context ==='
git show "${head}:frontend/src/erd/convert.test.ts" | nl -ba | sed -n '1,240p'

echo '=== Package lock Dagre package entries ==='
git show "${head}:frontend/package-lock.json" | nl -ba | sed -n '1,100p'
git show "${head}:frontend/package-lock.json" | rg -n -C4 '"node_modules/@dagrejs/dagre"|"`@dagrejs/dagre`"|`@dagrejs/graphlib`'

echo '=== Documentation ==='
git show "${head}:docs/doctoring/relationship-aware-erd-layout.md" | nl -ba
git show "${head}:CHANGELOG.md" | nl -ba | sed -n '1,80p'
git show "${head}:frontend/CHANGELOG.md" | nl -ba | sed -n '1,80p'

echo '=== Full patch against protected main ==='
git diff --unified=12 "$base" "$head" -- \
  CHANGELOG.md docs/doctoring/relationship-aware-erd-layout.md \
  frontend/CHANGELOG.md frontend/package.json frontend/package-lock.json \
  frontend/src/App.tsx frontend/src/App.coverage.test.tsx \
  frontend/src/erd/convert.ts frontend/src/erd/convert.test.ts \
  frontend/src/erd/dagreLayout.ts frontend/src/erd/dagreLayout.test.ts

Length of output: 2268


🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='a0f8cadf2fc818b8acd0c666a192e64fcfaf1ce1'
base='72afe6db712b145baaba084f64a1ff4fb36d9fd0'

echo '=== Complete layout module ==='
git show "${head}:frontend/src/erd/dagreLayout.ts"

echo '=== Complete layout test ==='
git show "${head}:frontend/src/erd/dagreLayout.test.ts"

echo '=== Snapshot conversion context ==='
git show "${head}:frontend/src/erd/convert.ts" | sed -n '25,180p'

echo '=== Toolbar layout and undo context ==='
git show "${head}:frontend/src/App.tsx" | sed -n '410,505p'
git show "${head}:frontend/src/App.tsx" | sed -n '1060,1160p'
rg -n -C4 'autoLayout|Auto layout|layout|undo|Undo|history|setNodes\(next\)' frontend/src/App.tsx

echo '=== Application coverage test context ==='
git show "${head}:frontend/src/App.coverage.test.tsx" | sed -n '1,260p'

echo '=== Conversion test context ==='
git show "${head}:frontend/src/erd/convert.test.ts" | sed -n '1,240p'

echo '=== Package lock Dagre package entries ==='
git show "${head}:frontend/package-lock.json" | sed -n '1,100p'
git show "${head}:frontend/package-lock.json" | rg -n -C4 '"node_modules/@dagrejs/dagre"|"`@dagrejs/dagre`"|`@dagrejs/graphlib`'

echo '=== Documentation ==='
git show "${head}:docs/doctoring/relationship-aware-erd-layout.md"
git show "${head}:CHANGELOG.md" | sed -n '1,80p'
git show "${head}:frontend/CHANGELOG.md" | sed -n '1,80p'

echo '=== Full patch against protected main ==='
git diff --unified=12 "$base" "$head" -- \
  CHANGELOG.md docs/doctoring/relationship-aware-erd-layout.md \
  frontend/CHANGELOG.md frontend/package.json frontend/package-lock.json \
  frontend/src/App.tsx frontend/src/App.coverage.test.tsx \
  frontend/src/erd/convert.ts frontend/src/erd/convert.test.ts \
  frontend/src/erd/dagreLayout.ts frontend/src/erd/dagreLayout.test.ts

Length of output: 89729


🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo='ContextualWisdomLab/pg-erd-cloud'
head='a0f8cadf2fc818b8acd0c666a192e64fcfaf1ce1'

echo '=== Pull request exact head and review state ==='
gh pr view 856 --repo "$repo" \
  --json number,headRefOid,baseRefOid,isDraft,mergeStateStatus,reviewDecision,statusCheckRollup \
  --jq '{number, headRefOid, baseRefOid, isDraft, mergeStateStatus, reviewDecision,
         checks: [.statusCheckRollup[] | {name, status, conclusion, workflowName}]}'

echo '=== Checks associated with the requested head ==='
gh api "repos/${repo}/commits/${head}/check-runs?per_page=100" --paginate \
  --jq '.check_runs[] | {name, status, conclusion, details_url}'

echo '=== Focused toolbar regression test ==='
git show "${head}:frontend/src/App.coverage.test.tsx" | sed -n '350,430p'

echo '=== Direct layout imports and call sites at the exact head ==='
git grep -n -E "import .*computeDagreLayout|computeDagreLayout\\(" "$head" -- frontend/src

echo '=== Exact-head worktree state and diff validation ==='
test "$(git rev-parse HEAD)" = "$head" && echo 'worktree-at-requested-head=true' || \
  printf 'worktree-at-requested-head=false actual=%s\n' "$(git rev-parse HEAD)"
git diff --check 72afe6db712b145baaba084f64a1ff4fb36d9fd0 "$head"

Length of output: 16848


@seonghobae Code review verdict: no blocking findings.

I reviewed exact head a0f8cadf2fc818b8acd0c666a192e64fcfaf1ce1 against base 72afe6db712b145baaba084f64a1ff4fb36d9fd0.

The fail-closed remediation is correct. computeDagreLayout validates all Dagre results and all converted top-left coordinates before it returns changed nodes. If any node has missing, non-positive, non-finite, or overflowed geometry, it preserves the prior position of every node. The new mixed-geometry regression test verifies this behavior.

The implementation meets the reviewed #755 requirements:

  • The manifest and lockfile pin @dagrejs/dagre to exact version 3.1.0.
  • The layout helper is pure and does not mutate input nodes or edges.
  • Stable node and edge insertion supports deterministic layout.
  • LR and TB directions, measured dimensions, cycles, disconnected nodes, parallel edges, and dangling edges have focused coverage.
  • Dagre center coordinates convert to React Flow top-left coordinates.
  • Snapshot import and toolbar layout use the same computeDagreLayout(nodes, edges) contract.
  • Toolbar layout captures one position snapshot and restores the exact positions on one undo.
  • The diff is limited to the layout slice, tests, dependency lockfile, changelogs, and documentation.
  • git diff --check is clean.

I did not execute repository code in this sandbox. The exact-head frontend, backend, CodeQL, Semgrep, dependency-review, Trivy, and review checks are successful.

I cannot give an unconditional merge-ready verdict yet. GitHub reports osv-scan and strix as IN_PROGRESS, and PR #856 is BLOCKED. Re-check those exact-head jobs after they complete.


✏️ Learnings added
Learnt from: seonghobae
URL: https://github.com/ContextualWisdomLab/pg-erd-cloud/pull/856

Timestamp: 2026-08-11T16:19:55.284Z
Learning: For the TypeScript module `frontend/src/erd/dagreLayout.ts`, `computeDagreLayout` must fail closed: if Dagre throws or any node result has missing, non-positive, non-finite, or overflowed geometry after conversion from center to React Flow top-left coordinates, it must preserve the prior coordinates of every input node. Partial layout application is invalid.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

You are interacting with an AI system.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Product Gap] Rebuild relationship-aware ERD layout as a bounded, canonical-lock PR

1 participant