refactor(erd): avoid intermediate handle-encoding arrays - #1087
refactor(erd): avoid intermediate handle-encoding arrays#1087seonghobae wants to merge 10 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. |
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueNo actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough
Changes핸들 ID 인코딩
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to 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)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation 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)
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 |
Current exact authority
main@8dc746920c12988f082e914879d95e13c9693535aef1be06b45625b8b3a8564b6d30f4e8eae2b2dbfrontend/src/erd/handleUtils.tsonly;.jules/bolt.mdnow points to the exact protected blobValid delta
sanitizeHandleIdstill encodes Unicode code points as lowercase hexadecimal segments separated by-, including the existingc-emptyresult for the empty string. The candidate replacesArray.from(columnName, mapper).join('-')with directfor...ofscalar iteration and incremental string construction, so it no longer constructs that specific intermediate character array.The existing
handleUtils.test.tscontract 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.mdentry also generalized this unproven local trade-off into repository doctrine. Normal descendant68929b4c4fff850420b8f3a70e0271b38eb5e83ffirst removed that branch-local rule,ed45ef1a6bb06bee98d0956ee4c00007424586f7removed the performance-claim comment from production, and final normal childaef1be06b45625b8b3a8564b6d30f4e8eae2b2dbadopts protected.jules/bolt.mdblobf1a8c146608751cd92ce59808a16d4b059eab07fexactly. Fresh PR metadata reports one changed file.Required GREEN
Before Ready/merge, keep one unchanged exact head and obtain:
Current exact-head CI
34013176364, Security Scan34013176344, SAST Semgrep34013176315, and CodeQL PR34013176335are 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.