⚡ Optimize pickTitleKeyWord performance#1097
Conversation
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
|
👋 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. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
Next review available in: 33 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the pickTitleKeyWord function in apps/api/src/utils/citation.ts to manually parse keywords instead of using regex splitting. The reviewer noted that this manual implementation misses Unicode whitespaces and is inefficient due to repeated string concatenation, and provided an optimized alternative using whitespace normalization and string slicing.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const ascii = normalizeAscii(title); | ||
| if (!ascii) return "paper"; | ||
|
|
||
| let firstToken = ""; | ||
| let currentToken = ""; | ||
| const len = ascii.length; | ||
|
|
||
| for (let i = 0; i <= len; i++) { | ||
| const char = i < len ? ascii[i] : ' '; | ||
| // `\s` matches space, tab, newline, return, form feed, vertical tab | ||
| if (char === ' ' || char === '\n' || char === '\t' || char === '\r' || char === '\f' || char === '\v') { | ||
| if (currentToken) { | ||
| if (!firstToken) firstToken = currentToken; | ||
| if (!STOP_WORDS.has(currentToken)) { | ||
| return currentToken; | ||
| } | ||
| currentToken = ""; | ||
| } | ||
| } else { | ||
| currentToken += char; | ||
| } | ||
| } | ||
|
|
||
| return firstToken || "paper"; |
There was a problem hiding this comment.
The current manual string parsing implementation has two main issues:
- Correctness (Unicode Whitespaces):
normalizeAsciipreserves Unicode whitespaces (e.g.,\\u2028,\\u2029) because they match\\sin the regex/[^a-zA-Z0-9\\s]/g. However, the manual character check only covers ASCII whitespaces. This means any Unicode whitespaces will be treated as part of the word token, causing incorrect tokenization. - Efficiency (String Concatenation): Repeatedly doing
currentToken += charinside a loop creates many intermediate string allocations in JavaScript/V8.
We can solve both issues elegantly by:
- Normalizing all whitespace sequences to a single standard space
' 'using.replace(/\\s+/g, " ")right afternormalizeAscii. - Using a
tokenStartindex andascii.slice(tokenStart, i)to extract tokens, which avoids string concatenation entirely.
const ascii = normalizeAscii(title).replace(/\s+/g, " ");
if (!ascii) return "paper";
let firstToken = "";
let tokenStart = 0;
const len = ascii.length;
for (let i = 0; i <= len; i++) {
const char = i < len ? ascii[i] : " ";
if (char === " ") {
if (i > tokenStart) {
const currentToken = ascii.slice(tokenStart, i);
if (!firstToken) firstToken = currentToken;
if (!STOP_WORDS.has(currentToken)) {
return currentToken;
}
}
tokenStart = i + 1;
}
}
return firstToken || "paper";
|
このリポジトリでは staging 先行フローを採用しています。PR のターゲットを |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| function pickTitleKeyWord(title: string): string { | ||
| const tokens = normalizeAscii(title).split(/\s+/).filter(Boolean); | ||
| return tokens.find((token) => !STOP_WORDS.has(token)) || tokens[0] || "paper"; | ||
| const ascii = normalizeAscii(title); | ||
| if (!ascii) return "paper"; | ||
|
|
||
| let firstToken = ""; | ||
| let currentToken = ""; | ||
| const len = ascii.length; | ||
|
|
||
| for (let i = 0; i <= len; i++) { | ||
| const char = i < len ? ascii[i] : ' '; | ||
| // `\s` matches space, tab, newline, return, form feed, vertical tab | ||
| if (char === ' ' || char === '\n' || char === '\t' || char === '\r' || char === '\f' || char === '\v') { | ||
| if (currentToken) { | ||
| if (!firstToken) firstToken = currentToken; | ||
| if (!STOP_WORDS.has(currentToken)) { | ||
| return currentToken; | ||
| } | ||
| currentToken = ""; | ||
| } | ||
| } else { | ||
| currentToken += char; | ||
| } | ||
| } | ||
|
|
||
| return firstToken || "paper"; | ||
| } |
There was a problem hiding this comment.
pickTitleKeyWord のみを手動ループへ書き換えましたが、同じファイルの pickSurname（63行目）と toApaAuthorName（114行目）では引き続き .split(/\s+/).filter(Boolean) を使用しています。パフォーマンスが重要な問題であれば、これらの関数も同様に最適化する必要があります。現状では改善対象の選択に一貫性がなく、コードレビュアーが「なぜここだけ?」と疑問を持ちやすい状態になっています。
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/api/src/utils/citation.ts
Line: 67-92
Comment:
**パフォーマンス改善の対象が不整合**
`pickTitleKeyWord` のみを手動ループへ書き換えましたが、同じファイルの `pickSurname`（63行目）と `toApaAuthorName`（114行目）では引き続き `.split(/\s+/).filter(Boolean)` を使用しています。パフォーマンスが重要な問題であれば、これらの関数も同様に最適化する必要があります。現状では改善対象の選択に一貫性がなく、コードレビュアーが「なぜここだけ?」と疑問を持ちやすい状態になっています。
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| function pickTitleKeyWord(title: string): string { | ||
| const tokens = normalizeAscii(title).split(/\s+/).filter(Boolean); | ||
| return tokens.find((token) => !STOP_WORDS.has(token)) || tokens[0] || "paper"; | ||
| const ascii = normalizeAscii(title); | ||
| if (!ascii) return "paper"; | ||
|
|
||
| let firstToken = ""; | ||
| let currentToken = ""; | ||
| const len = ascii.length; | ||
|
|
||
| for (let i = 0; i <= len; i++) { | ||
| const char = i < len ? ascii[i] : ' '; | ||
| // `\s` matches space, tab, newline, return, form feed, vertical tab | ||
| if (char === ' ' || char === '\n' || char === '\t' || char === '\r' || char === '\f' || char === '\v') { | ||
| if (currentToken) { | ||
| if (!firstToken) firstToken = currentToken; | ||
| if (!STOP_WORDS.has(currentToken)) { | ||
| return currentToken; | ||
| } | ||
| currentToken = ""; | ||
| } | ||
| } else { | ||
| currentToken += char; | ||
| } | ||
| } | ||
|
|
||
| return firstToken || "paper"; | ||
| } |
There was a problem hiding this comment.
元の実装は 2 行で意図が明確でしたが、新しい実装は 20 行超の手動ループになっています。pickTitleKeyWord は buildCitation から引用生成時に 1 回呼ばれるだけの非ホットパスです。ベンチマーク上の 50% 改善は絶対値で見ると 1 呼び出しあたり約 6μs の短縮であり、ユーザー体験には影響しません。現代の JS エンジン（V8）の .split() / .find() は高度に最適化されており、将来の保守者が誤ってこのロジックを変更するリスクの方が高い可能性があります。
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/api/src/utils/citation.ts
Line: 67-92
Comment:
**複雑性の増加に対してトレードオフが見合わない可能性**
元の実装は 2 行で意図が明確でしたが、新しい実装は 20 行超の手動ループになっています。`pickTitleKeyWord` は `buildCitation` から引用生成時に 1 回呼ばれるだけの非ホットパスです。ベンチマーク上の 50% 改善は絶対値で見ると 1 呼び出しあたり約 6μs の短縮であり、ユーザー体験には影響しません。現代の JS エンジン（V8）の `.split()` / `.find()` は高度に最適化されており、将来の保守者が誤ってこのロジックを変更するリスクの方が高い可能性があります。
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
|
Closing because this turns a clear two-line non-hot-path operation into a 20+ line manual parser for roughly 6 microseconds per citation. The maintenance and correctness surface outweighs the user-visible benefit. |
1 participant
💡 What: Replaced the array mapping and searching implementation in
pickTitleKeyWordwith a manual string parsing for-loop that handles all whitespace characters.🎯 Why: The original implementation used
.split(/\s+/)and.filter(Boolean)which allocated new arrays, and then iterated again using.find(). By manually iterating through the characters of the string and evaluating each word token, we avoid unnecessary array allocations and can return early the moment we find the first non-stop-word token.📊 Measured Improvement: Benchmarked against the original array methods across 100,000 iterations:
This shows an approximate 50% improvement in performance for extracting keywords.
PR created automatically by Jules for task 16039237034858135568 started by @is0692vs
Greptile Summary
apps/api/src/utils/citation.tsのpickTitleKeyWord関数を、配列アロケーションを避けた手動文字列パースのforループへ置き換えるパフォーマンス最適化 PR です。ロジック自体は元の実装と等価です。normalizeAscii後の ASCII 文字列を 1 文字ずつ走査しトークンを切り出す実装に変更。NFKD 正規化により Unicode 空白は通常スペースに変換されるため実用上の問題はありません。pickSurname・toApaAuthorNameは.split(/\\s+/).filter(Boolean)のまま残っており、最適化の適用範囲に一貫性がありません。pickTitleKeyWordは引用生成時に 1 回呼ばれる非ホットパスで、ベンチマーク上の短縮は約 6μs/呼び出しにとどまります。Confidence Score: 4/5
実用上の入力に対してロジックは元の実装と等価であり、マージ自体は安全です。
変更されたロジックは NFKD 正規化済み ASCII 文字列を扱うため、Unicode 空白の処理差異による実害は現実的にほぼ存在しません。一方でコードの複雑性が大幅に増加し、同ファイルの類似関数との一貫性が失われている点は将来の保守リスクとして残ります。
apps/api/src/utils/citation.ts — 最適化対象の選択（pickTitleKeyWord のみ）とコード量の増加について確認が必要。
Important Files Changed
pickTitleKeyWordを手動文字列パースのforループへ置き換え。ロジックは元の実装と等価だが、コードが20行超に増加し、同ファイルのpickSurname・toApaAuthorNameとの一貫性が失われている。Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[title: string] --> B[normalizeAscii] B --> C{ascii が空?} C -- Yes --> D[return 'paper'] C -- No --> E[for i=0 to len inclusive] E --> F{char は空白?} F -- No --> G[currentToken += char] G --> E F -- Yes --> H{currentToken が空?} H -- Yes --> E H -- No --> I{firstToken が未設定?} I -- Yes --> J[firstToken = currentToken] J --> K{STOP_WORDSに含まれない?} I -- No --> K K -- Yes --> L[return currentToken] K -- No --> M[currentToken = '' reset] M --> E E -- ループ終了 --> N[return firstToken OR 'paper']%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[title: string] --> B[normalizeAscii] B --> C{ascii が空?} C -- Yes --> D[return 'paper'] C -- No --> E[for i=0 to len inclusive] E --> F{char は空白?} F -- No --> G[currentToken += char] G --> E F -- Yes --> H{currentToken が空?} H -- Yes --> E H -- No --> I{firstToken が未設定?} I -- Yes --> J[firstToken = currentToken] J --> K{STOP_WORDSに含まれない?} I -- No --> K K -- Yes --> L[return currentToken] K -- No --> M[currentToken = '' reset] M --> E E -- ループ終了 --> N[return firstToken OR 'paper']Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "perf: optimize pickTitleKeyWord performa..." | Re-trigger Greptile