fix(erd): preserve PostgreSQL identifiers in relationship inference - #774
fix(erd): preserve PostgreSQL identifiers in relationship inference#774seonghobae wants to merge 89 commits into
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughPostgreSQL 식별자를 사용하는 관계 추론 테스트를 스냅샷 기반으로 갱신했다. ChangesPostgreSQL 식별자 관계 추론 테스트
테이블명 정제 테스트
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The PR preserves exact PostgreSQL identifiers during ERD relationship inference and removes the obsolete lossy sanitizer. The change is localized, with no actionable merge-blocking risk remaining beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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. Comment |
seonghobae
changed the title
sanitizeTableName 함수에 대한 테스트 추가
|
@coderabbitai review Please review unchanged exact head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
requested_head="49aa358455e7de72e52ca283c433d986457fc68a"
actual_head="$(git rev-parse HEAD)"
echo "requested_head=$requested_head"
echo "actual_head=$actual_head"
git cat-file -e "${requested_head}^{commit}"
base="$(git merge-base "${requested_head}" "origin/$(git remote show origin | sed -n '/HEAD branch/s/.*: //p')")"
echo "merge_base=$base"
echo '--- changed files ---'
git diff --name-status "$base" "$requested_head"
echo '--- diff ---'
git diff --find-renames --find-copies --unified=80 "$base" "$requested_head"
echo '--- relevant file locations ---'
fd -a 'securityUtils|CHANGELOG|changelog|doctor' frontend . 2>/dev/null || true
echo '--- source/test outlines ---'
for f in $(fd -a -t f 'securityUtils\.(ts|tsx|js|jsx)$|securityUtils\.test\.(ts|tsx|js|jsx)$' frontend 2>/dev/null); do
echo "### $f"
ast-grep outline "$f" --items all || true
doneLength of output: 14136 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- autoInfer implementation ---'
sed -n '1,180p' frontend/src/erd/autoInfer.ts
echo '--- security utility ---'
sed -n '1,120p' frontend/src/erd/securityUtils.ts
echo '--- autoInfer tests and table-node data definitions ---'
fd -a -t f 'autoInfer.*test.*|convert\.ts$' frontend/src/erd | while IFS= read -r f; do
echo "### $f"
ast-grep outline "$f" --items all || true
sed -n '1,260p' "$f"
done
echo '--- sanitizer definitions and call sites ---'
rg -n -C 4 --glob '*.{ts,tsx,js,jsx}' \
'sanitizeTableName\s*\(|function\s+sanitizeTableName\b' frontend
echo '--- identifier/title parsing sites ---'
rg -n -C 3 --glob '*.{ts,tsx,js,jsx}' \
'\.data\.title\.split\("\."\)|split\(["'\'']\.'\''\)' frontend/src
echo '--- frontend scripts and test configuration ---'
for f in package.json frontend/package.json frontend/vitest.config.ts frontend/vite.config.ts; do
if [ -f "$f" ]; then
echo "### $f"
sed -n '1,240p' "$f"
fi
doneLength of output: 24892
|
Acknowledged. |
|
Single-writer lease freeze: another writer moved this branch after exact head |
Pull request was converted to draft
I have reconciled the added tests with the implemented exact-identifier product boundary and restored the previous test-first PostgreSQL identifier fidelity fixes. Re-submitting to trigger required check/review runs. |
Remove the now-unused lossy sanitizer and its behavior-freezing test. Use the map-presence invariant directly so relationship inference retains exact PostgreSQL identifier identity without an unreachable coverage branch.
|
@coderabbitai review Please review exact head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
requested_head="177c3849755a24f54faf4313b22c01195822edb0"
actual_head="$(git rev-parse HEAD)"
echo "requested_head=$requested_head"
echo "actual_head=$actual_head"
git cat-file -e "${requested_head}^{commit}"
default_branch="$(git remote show origin | sed -n '/HEAD branch/s/.*: //p')"
base="$(git merge-base "$requested_head" "origin/$default_branch")"
echo "merge_base=$base"
echo '--- changed files ---'
git diff --name-status --find-renames "$base" "$requested_head"
echo '--- full diff ---'
git diff --find-renames --find-copies --unified=80 "$base" "$requested_head"
echo '--- candidate files ---'
fd -a -t f 'autoInfer|securityUtils|postgresql-identifier-fidelity|CHANGELOG|changelog' . 2>/dev/null || true
echo '--- outlines ---'
for f in $(fd -a -t f 'autoInfer\.(ts|tsx|js|jsx)$|autoInfer\.(test|spec)\.(ts|tsx|js|jsx)$|securityUtils\.(ts|tsx|js|jsx)$|securityUtils\.(test|spec)\.(ts|tsx|js|jsx)$' frontend 2>/dev/null); do
echo "### $f"
ast-grep outline "$f" --items all || true
done
echo '--- identifier-related definitions and call sites ---'
rg -n -C 5 --glob '*.{ts,tsx,js,jsx}' \
'sanitizeTableName\s*\(|inferRelationships\s*\(|snapshotToGraph\s*\(|\.data\.title\.split\(|split\(["'\'']\.'\''\)' \
frontendLength of output: 36581 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- autoInfer.ts ---'
sed -n '1,150p' frontend/src/erd/autoInfer.ts
echo '--- convert.ts snapshot title construction ---'
rg -n -C 12 --glob 'convert.ts' \
'title\s*:|schema_name|relation_name' frontend/src/erd
echo '--- identifier regression and related inference tests ---'
sed -n '1,220p' frontend/src/erd/__tests__/autoInfer.postgresIdentifiers.test.ts
sed -n '1,260p' frontend/src/erd/__tests__/autoInfer.test.ts
echo '--- all exact title parsing sites in ERD source and tests ---'
rg -n -C 4 --glob '*.{ts,tsx}' \
'data\.title\.split\(|title\.split\(|schema_name.*relation_name|relation_name.*schema_name' \
frontend/src/erdLength of output: 25925
Store the relation name separately from the display title, or parse identifier components before extracting the relation name. Add a regression test that passes
|
Acknowledged. |
Reconcile the tested identifier-preservation implementation onto protected main and remove the later branch regression that reintroduced the lossy sanitizer and its behavior-freezing tests. Preserve current protected-main dependencies, workflows, and product state everywhere outside the bounded inference slice.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Queued @opencode-agent for PR #774 at head |
|
Already queued @opencode-agent on this exact request for PR #774 at head |
|
Queued @opencode-agent for PR #774 at head |
|
Queued @opencode-agent for PR #774 at head |
|
Queued @opencode-agent for PR #774 at head |
|
Queued @opencode-agent for PR #774 at head |
|
Already queued @opencode-agent on this exact request for PR #774 at head |
|
Already queued @opencode-agent on this exact request for PR #774 at head |
|
Already queued @opencode-agent on this exact request for PR #774 at head |
|
Already queued @opencode-agent on this exact request for PR #774 at head |
|
Already queued @opencode-agent on this exact request for PR #774 at head |
|
Queued @opencode-agent for PR #774 at head |
|
Queued @opencode-agent for PR #774 at head |
|
Queued @opencode-agent for PR #774 at head |
|
Queued @opencode-agent for PR #774 at head |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
1 participant
Buyer-visible defect
Automatic ERD relationship inference could lose valid PostgreSQL object identity. The previous implementation found an existing relation candidate and then rewrote the exact key through an ASCII-only sanitizer. It also reconstructed relation names by splitting the display title on
., which is ambiguous when a quoted relation name itself contains a dot. Unicode, whitespace, mixed-case and dot-containing relation names could therefore lose inferred edges.Test-first remedy
RED commit
4cd28702107f07e5eb4ff1761f8c8996975e2469adds a product-boundary regression throughsnapshotToGraph+inferRelationshipsfor:public.사용자/사용자_id;public."Order Items"/Order Items_id;Order.Items/Order.Items_id.The production repair then:
relation_nameseparately asTableNodeData.relationName;Current exact tree
main@8dc746920c12988f082e914879d95e13c9693535.5ff276684cbc67a0445b394a34b5b75e018fa984.autoInfer.ts,convert.ts, and deletion of the obsolete sanitizer.Security and authority boundary
The browser inference helper neither renders nor executes SQL. It can only select a node already present in the in-memory graph map. Preserving model identity therefore adds no SQL execution authority; SQL-rendering boundaries remain responsible for context-aware identifier quoting. Character deletion is not a substitute for quoting and can silently select a different model object.
Design-system boundary
This repair changes relationship identity/inference logic, not visual components, design tokens or customer-facing explanatory copy. Fresh Figma metadata for the primary pg-erd-cloud file currently exposes only
Cover, and this exact head has no Storybook script/dependency orfrontend/.storybookconfiguration. No parallel visual authority is introduced in this PR; the separate Figma/Storybook design-system ownership lane remains separate from this bounded ERD logic fix.Traceability
docs/doctoring/postgresql-identifier-fidelity.mdrecords the decision, invariants, monitoring/rollback contract and the PostgreSQL 18 lexical-structure authority in APA 7 style. Both root and frontend Unreleased changelogs record the buyer-visible behavior change.Verification boundary
Historical predecessor-head results are lineage only. Fresh workflows are running for the exact current head and remain non-passing until terminal. The current review finding that reported the pre-repair 0-edge result was revalidated against the current source, answered with exact-head evidence and resolved only after the exact relation metadata, non-sanitizing lookup and dotted-name regression were present.
Merge boundary
Do not merge until the unchanged latest head satisfies every live required CI/security/coverage/SAST/supply-chain/Strix/OpenCode/review-thread context, zero valid current-head findings remain, and a qualifying independent non-author human approval satisfies the live ruleset. Queued, in-progress, skipped-required, stale, predecessor-head, status-only, model-only, synthetic or author-only evidence is non-passing. Do not self-approve or use an admin/ruleset bypass.