-
Notifications
You must be signed in to change notification settings - Fork 0
refactor(erd): decode canonical column handles during export #1062
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
b0743da
126f632
ca2a63a
ff20016
c8b8ba1
e3c6cf0
7f07fc0
ebe36a7
ac7d754
cbe424b
ed8e52f
cae10c3
30ce2ed
92190ab
e200b8f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -77,3 +77,6 @@ Optimized metric route processing to O(N) by creating a mapping of routes direct | |
| ## 2024-07-13 - [Optimize Export Dictionary FK lookups] | ||
| **Learning:** Found O(N * C * E) performance bottleneck in ERD export dictionaries due to repeated array searching with `edges.some()` inside a nested loop over nodes and columns. | ||
| **Action:** Replace repeated linear array scans for edges by precomputing O(1) Set lookups of foreign key column handles per node before looping. | ||
| ## 2025-03-02 - Optimize column parsing in ERD export | ||
| **Learning:** Identifying and reducing repeated iterations over large arrays of node columns (like checking `column.column_name` by encoding it as a handle ID iteratively in `export.ts`) is crucial. A simple helper `parseColumnNameFromHandle` effectively achieves O(1) string decodes on the edge handles themselves without O(N) generation of handles for each element of columns array, saving allocation and iteration regressions when resolving edge handles. However, directly performing a `.some()` search for membership inside edge processing still leads to an $O(E \times C)$ iteration bottleneck, as warned by reviewer constraints. | ||
| **Action:** Add a column handle parsing utility directly, to pre-parse handles during export functionality, avoiding array scanning and multiple DOM ID generation. Pre-compute node columns into Map/Sets to guarantee $O(1)$ amortized membership verification, making the final edge processing complexity genuinely $O(L)$ where $L$ is handle string length. | ||
|
Comment on lines
+80
to
+82
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🤖 get_repo_knowledge executed:
Length of output: 9423 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- tracked file ---'
git ls-files .jules/bolt.md
printf '%s\n' '--- file excerpt ---'
cat -n .jules/bolt.md | sed -n '1,130p'
printf '%s\n' '--- repository guidance candidates ---'
git ls-files | rg '(^|/)(CONTRIBUTING|CODE_OF_CONDUCT|README|AGENTS|CLAUDE|review|guideline|policy|bolt)' | head -80
printf '%s\n' '--- academic-literature wording ---'
rg -n -i --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' 'academic|literature|paper|citation|research|substantive feature|process pull request' .Repository: ContextualWisdomLab/pg-erd-cloud Length of output: 13470 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- research-grounding rule ---'
cat -n AGENTS.md | sed -n '84,106p'
printf '%s\n' '--- papers guidance ---'
cat -n docs/papers/README.md | sed -n '1,80p'Repository: ContextualWisdomLab/pg-erd-cloud Length of output: 2077 학술 근거를 PR에 추가하세요. 실질적인 기능 또는 프로세스 변경에는 관련 논문의 PDF와 전체 인용 정보를 PR에 포함해야 합니다. 재배포가 허용되지 않으면 논문을 인용하고 링크와 요약을 제공하세요. 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| import { describe, it, expect } from 'vitest'; | ||
| import { exportDictionaryCsv } from '../exportDataDictionary'; | ||
| import { sourceColumnHandleId, targetColumnHandleId } from '../handleUtils'; | ||
| import type { Node, Edge } from '@xyflow/react'; | ||
| import type { TableNodeData } from '../convert'; | ||
|
|
||
| describe('Export Dictionary Benchmark', () => { | ||
| it('should efficiently export dictionaries for large graphs without N^2 scaling', () => { | ||
| const nodes: Node<TableNodeData>[] = []; | ||
| const edges: Edge[] = []; | ||
|
|
||
| // Generate 500 tables, each with 20 columns | ||
| const numTables = 500; | ||
| const numCols = 20; | ||
|
|
||
| for (let i = 0; i < numTables; i++) { | ||
| nodes.push({ | ||
| id: `t${i}`, | ||
| data: { | ||
| title: `table_${i}`, | ||
| columns: Array.from({ length: numCols }, (_, c) => ({ | ||
| column_name: `col_${c}`, | ||
| data_type: 'text', | ||
| is_not_null: false, | ||
| is_pk: c === 0, | ||
| })), | ||
| badges: { pk: true, fk: i > 0 } | ||
| }, | ||
| position: { x: 0, y: 0 } | ||
| }); | ||
|
|
||
| // Connect each table to the previous one | ||
| if (i > 0) { | ||
| edges.push({ | ||
| id: `e${i}`, | ||
| source: `t${i}`, | ||
| target: `t${i-1}`, | ||
| sourceHandle: sourceColumnHandleId('col_1'), // FK column | ||
| targetHandle: targetColumnHandleId('col_0'), // PK column | ||
| data: {} | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| const start = performance.now(); | ||
| const csv = exportDictionaryCsv(nodes, edges); | ||
| const elapsed = performance.now() - start; | ||
|
|
||
| expect(csv).toContain('table_0'); | ||
| expect(csv).toContain('table_499'); | ||
|
|
||
| // This previously took hundreds of milliseconds due to O(N * C * E) | ||
| // and is now expected to be well under 50ms. | ||
| console.log(`Large CSV export took: ${elapsed.toFixed(2)}ms`); | ||
| expect(elapsed).toBeLessThan(100); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| import { describe, it, expect } from 'vitest'; | ||
| import fc from 'fast-check'; | ||
| import { sanitizeHandleId, sourceColumnHandleId, targetColumnHandleId, parseColumnNameFromHandle } from '../handleUtils'; | ||
|
|
||
| describe('Handle encoding/decoding properties', () => { | ||
| it('should round-trip correctly for arbitrary valid column names (including ASCII, CJK, emoji, punctuation)', () => { | ||
| fc.assert( | ||
| fc.property(fc.string({ minLength: 0 }), (str) => { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🌐 Web query:
💡 Result: In fast-check 4.8.0, the default value for the Citations:
🏁 Script executed: # 변경된 테스트와 fast-check 버전, 관련 속성 테스트의 입력 생성을 확인합니다.
printf '%s\n' '--- test ---'
cat -n frontend/src/erd/__tests__/handleUtils.property.test.ts | sed -n '1,100p'
printf '%s\n' '--- fast-check declarations ---'
rg -n --glob 'package.json' --glob 'pnpm-lock.yaml' --glob 'yarn.lock' --glob 'package-lock.json' '"fast-check"|fast-check@' .
printf '%s\n' '--- related tests and implementation ---'
rg -n -C 3 'Unicode|unicode|fc\.string|handleUtils|parse' frontend/src/erd frontend/src | head -240Repository: ContextualWisdomLab/pg-erd-cloud Length of output: 18114 🤖 get_repo_knowledge executed:
Length of output: 7648 속성 테스트가 유니코드 입력을 생성하도록 수정하세요.
🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| const sourceHandle = sourceColumnHandleId(str); | ||
| const parsedSource = parseColumnNameFromHandle(sourceHandle, 'src'); | ||
| expect(parsedSource).toBe(str); | ||
|
|
||
| const targetHandle = targetColumnHandleId(str); | ||
| const parsedTarget = parseColumnNameFromHandle(targetHandle, 'tgt'); | ||
| expect(parsedTarget).toBe(str); | ||
| }) | ||
| ); | ||
| }); | ||
|
|
||
| it('should reject malformed handles, noncanonical padded hex, missing digits, and direction mismatches', () => { | ||
| // Malformed/empty | ||
| expect(parseColumnNameFromHandle(null)).toBeNull(); | ||
| expect(parseColumnNameFromHandle(undefined)).toBeNull(); | ||
| expect(parseColumnNameFromHandle('')).toBeNull(); | ||
| expect(parseColumnNameFromHandle('invalid-format')).toBeNull(); | ||
|
|
||
| // Direction swaps | ||
| expect(parseColumnNameFromHandle('tgt-c-0069-0064', 'src')).toBeNull(); | ||
| expect(parseColumnNameFromHandle('src-c-0069-0064', 'tgt')).toBeNull(); | ||
|
|
||
| // Invalid hex / bad scalars | ||
| expect(parseColumnNameFromHandle('src-c-nothex')).toBeNull(); | ||
| expect(parseColumnNameFromHandle('src-c-g000')).toBeNull(); | ||
| expect(parseColumnNameFromHandle('src-c-1000000')).toBeNull(); // Out of range (> 0x10FFFF) | ||
|
|
||
| // Non-canonical padding (e.g. 00069 instead of 0069) | ||
| // The letter 'i' (0069 in hex) | ||
| expect(parseColumnNameFromHandle('src-c-00069')).toBeNull(); | ||
| // Uppercase letters in hex (should be lowercase according to toString(16)) | ||
| // '006A' will round trip back to '006a'. Thus '006A' fails the round trip check. | ||
| expect(parseColumnNameFromHandle('src-c-006A')).toBeNull(); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
핸들 디코딩 복잡도를 O(L)로 수정하세요.
parseColumnNameFromHandle는 모든 hex segment를 분할하고 순회합니다. 따라서 디코딩 시간과 추가 메모리는 handle 길이L에 비례합니다. 평균 O(1)인 작업은 사전 계산된Setmembership 검사입니다.🤖 Prompt for AI Agents