Skip to content

refactor(erd): avoid intermediate handle-encoding arrays - #1087

Draft
seonghobae wants to merge 10 commits into
mainfrom
bolt-optimize-array-from-551622222103629220
Draft

refactor(erd): avoid intermediate handle-encoding arrays#1087
seonghobae wants to merge 10 commits into
mainfrom
bolt-optimize-array-from-551622222103629220

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

Current exact authority

  • protected base: main@8dc746920c12988f082e914879d95e13c9693535
  • exact head: aef1be06b45625b8b3a8564b6d30f4e8eae2b2db
  • lifecycle: Draft / behavior-equivalent refactor candidate / buyer-path performance evidence pending
  • effective delta: frontend/src/erd/handleUtils.ts only; .jules/bolt.md now points to the exact protected blob

Valid delta

sanitizeHandleId still encodes Unicode code points as lowercase hexadecimal segments separated by -, including the existing c-empty result for the empty string. The candidate replaces Array.from(columnName, mapper).join('-') with direct for...of scalar iteration and incremental string construction, so it no longer constructs that specific intermediate character array.

The existing handleUtils.test.ts contract already covers ASCII, empty input, punctuation, Korean text, emoji, and the source/target handle prefixes. The refactor must remain byte-for-byte compatible with those identifiers because they are consumed by ERD graph/export paths.

Evidence correction

The original PR description promoted a one-off 100,000-iteration timing into claims about ~50% speedup, reduced GC pauses, and large-graph render/search improvement. Those conclusions are not established by repository evidence. Removing one explicit intermediate array is observable from the implementation, but whole-path latency, allocation/GC behavior, and buyer-visible rendering impact depend on Node/browser engine, identifier distribution, graph cardinality, call frequency, React Flow/render work, and other surrounding costs.

The branch-local .jules/bolt.md entry also generalized this unproven local trade-off into repository doctrine. Normal descendant 68929b4c4fff850420b8f3a70e0271b38eb5e83f first removed that branch-local rule, ed45ef1a6bb06bee98d0956ee4c00007424586f7 removed the performance-claim comment from production, and final normal child aef1be06b45625b8b3a8564b6d30f4e8eae2b2db adopts protected .jules/bolt.md blob f1a8c146608751cd92ce59808a16d4b059eab07f exactly. Fresh PR metadata reports one changed file.

Required GREEN

Before Ready/merge, keep one unchanged exact head and obtain:

  • focused and full frontend tests proving identifier compatibility;
  • representative/right-cleared ERD workloads with realistic table/column-name distributions and graph cardinalities;
  • protected-vs-candidate measurement under the same supported runtime/browser environment, repeated samples with median/p95, and allocation/GC plus CPU profile evidence;
  • a whole buyer path measurement for the affected graph construction/export path rather than only an isolated 100k loop;
  • terminal exact-head CI/security/SAST/CodeQL evidence and qualifying independent current-head review.

Current exact-head CI 34013176364, Security Scan 34013176344, SAST Semgrep 34013176315, and CodeQL PR 34013176335 are queued; no predecessor result transfers.

If the candidate does not produce a material whole-path gain, prefer the simpler protected implementation rather than retaining an unmeasured micro-optimization. No generated doctrine, self-approval, gate weakening, force push, destructive rebase, source-neutral retrigger, or predecessor GREEN transfer.

@google-labs-jules

Copy link
Copy Markdown

👋 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 @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: c539fc80-1939-41cb-8fb6-f2483c12ff85

📥 Commits

Reviewing files that changed from the base of the PR and between 8dc7469 and d78dc44.

📒 Files selected for processing (2)
  • .jules/bolt.md
  • frontend/src/erd/handleUtils.ts

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

sanitizeHandleId가 중간 배열 없이 문자열을 순회하도록 변경되었습니다. 빈 입력은 c-empty로 처리됩니다. 관련 성능 학습 노트가 추가되었습니다.

Changes

핸들 ID 인코딩

Layer / File(s) Summary
핸들 ID 인코딩 변경
frontend/src/erd/handleUtils.ts, .jules/bolt.md
sanitizeHandleIdfor...of로 코드 포인트를 순회하고, 빈 입력을 즉시 c-empty로 반환합니다. 인코딩 형식은 4자리 16진수와 하이픈 연결을 유지합니다. 해당 구현 원칙을 설명하는 학습 노트가 추가되었습니다.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 1fcbe

Handle-ID generation now avoids intermediate arrays while retaining the existing encoding behavior, including empty and Unicode inputs. The focused coverage supports merge readiness with no identified active risk.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 sanitizeHandleId에서 중간 배열을 제거하는 주요 변경 사항을 정확하고 간결하게 설명합니다.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch bolt-optimize-array-from-551622222103629220

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.

@seonghobae
seonghobae marked this pull request as draft September 6, 2026 05:06
@seonghobae seonghobae changed the title ⚡ Bolt: Replace Array.from with for...of in sanitizeHandleId to reduce GC pressure refactor(erd): avoid intermediate handle-encoding arrays Sep 6, 2026
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.

1 participant