Repository navigation
perf(search): האינדקס בזיכרון נבנה רק בחיפושים שקוראים ממנו - #3351
Conversation
`AdvancedSearchEngine.search` בנה את `SearchIndex` לפני שבדק איזה סוג חיפוש התבקש. רק `TEXT` ו-`FUNCTION` קוראים מהאינדקס; `CONTENT`, `REGEX` ו-`FUZZY` סורקים את ה-DB בעצמם ואינם נוגעים בו. מכיוון ש-`CONTENT` הוא ברירת המחדל של החיפוש ב-WebApp, המסלול הרגיל שילם על סריקה מלאה של כל קבצי המשתמש (כולל `code`, במנות של `SEARCH_PAGE_SIZE`, עם קיבוץ גרסאות מלא בכל עמוד) שאיש לא קרא את תוצאתה — ומיד אחריה סריקה מלאה שנייה לחיפוש עצמו. מה השתנה: - `get_index` נקרא רק בענפי `TEXT` ו-`FUNCTION`, דרך `_get_index_for_search` ששומרת על מדידת `search_index_get` הקיימת. - הפרמטר `index` הוסר מ-`_fuzzy_search`; הוא התקבל ומעולם לא נקרא, וזה מה שהסתיר את העובדה שהמסלול אינו נשען על האינדקס. - נוסף `SEARCH_MEMORY_INDEX_ENABLED` (ברירת מחדל `true`) לכיבוי מלא של האינדקס בזיכרון. כבוי ⇒ `TEXT` ו-`FUNCTION` מחזירים רשימה ריקה, ו- `_safe_search` ב-WebApp נופל לחיפוש `$text` במונגו. מה שנעלם: התאמה חלקית (prefix/substring) ו-`function_index`. התיעוד ב-`docs/workflows/search-flow.rst` עודכן בהתאם, כולל היחס בין האינדקס בזיכרון לאינדקס `search_text_idx` של מונגו. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RgnHZBJaH4VhdFFgwYZKLr
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Reviewer's Guideהשינוי משפר ביצועים בכך שמונע סריקת בניית אינדקס מיותרת עבור CONTENT, REGEX ו-FUZZY, ומוסיף מתג סביבה להשבתת האינדקס לחלוטין עם fallback מתועד ל-MongoDB. הבדיקות מכסות את בחירת ענף החיפוש, התנהגות הכיבוי והתוצאה הנצפית בלוגים. Sequence diagram for disabled memory index fallbacksequenceDiagram
participant WebApp
participant SearchEngine
participant MongoDB
WebApp->>SearchEngine: search(user_id, query, TEXT)
SearchEngine->>SearchEngine: _get_index_for_search(user_id)
SearchEngine->>SearchEngine: get_index(user_id)
SearchEngine-->>SearchEngine: SearchIndex()
SearchEngine-->>WebApp: empty result list
WebApp->>MongoDB: $text search
MongoDB-->>WebApp: search results
Flow diagram for conditional search index usageflowchart TD
A["AdvancedSearchEngine.search"] --> B{Search type}
B -->|TEXT| C[_get_index_for_search]
B -->|FUNCTION| C
C --> D[get_index]
D --> E[_text_search or _function_search]
B -->|CONTENT| F[_content_search]
B -->|REGEX| G[_regex_search]
B -->|FUZZY| H[_fuzzy_search]
F --> I["Database scan"]
G --> I
H --> I
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Review limit reachedNext included review available in 1 minute. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (12)
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 |
🧯 Dangerous deletes guard reportPolicy: see .cursorrules — dangerous deletions are blocked unless wrapped safely. Summary:
Flagged findings (file:line:snippet): Excluded matches (by path pattern) |
⏱️ Performance report(No performance test durations collected. Mark tests with |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="search_engine.py" line_range="823-824" />
<code_context>
+ ש-``$text`` אינו תומך בה.
+ """
+
+ if not _memory_index_enabled():
+ return SearchIndex()
+
if user_id not in self.indexes:
</code_context>
<issue_to_address>
**issue (bug_risk):** When `SEARCH_MEMORY_INDEX_ENABLED` is false, `get_index` returns an empty `SearchIndex`, so direct callers of `AdvancedSearchEngine.search` receive no `TEXT` or `FUNCTION` results; only the WebApp-specific `_safe_search` wrapper performs the documented MongoDB fallback. The configuration description therefore promises a fallback that the search-engine API itself does not provide.
**Triggers:** When a bot, internal fallback such as `_fallback_text_search`, or any non-WebApp caller uses `AdvancedSearchEngine.search` while the switch is disabled.
**Suggested fix:** Either implement the MongoDB fallback inside the search engine or narrow the configuration description and make the switch explicitly WebApp-only.
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: search_engine.py:824
| if not _memory_index_enabled(): | ||
| return SearchIndex() |
There was a problem hiding this comment.
issue (bug_risk): When SEARCH_MEMORY_INDEX_ENABLED is false, get_index returns an empty SearchIndex, so direct callers of AdvancedSearchEngine.search receive no TEXT or FUNCTION results; only the WebApp-specific _safe_search wrapper performs the documented MongoDB fallback. The configuration description therefore promises a fallback that the search-engine API itself does not provide.
Triggers: When a bot, internal fallback such as _fallback_text_search, or any non-WebApp caller uses AdvancedSearchEngine.search while the switch is disabled.
Suggested fix: Either implement the MongoDB fallback inside the search engine or narrow the configuration description and make the switch explicitly WebApp-only.
📖 Documentation PreviewThe documentation has been built successfully!
To view locally:
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
המשך ל-#3351, אחרי שהתברר שהסרת בניית האינדקס מחיפוש `content` גררה רגרסיה בהשלמה האוטומטית — ושהתצוגה שלה שבורה ממילא. מתג `SEARCH_MEMORY_INDEX_EAGER_BUILD` (ברירת מחדל `false`) ---------------------------------------------------------- `suggest_completions` קורא את האינדקס דרך `_get_ready_index`, שבמכוון אינו בונה אותו — ולכן ההשלמה נהנתה מאינדקס שחיפוש `content` בנה כתופעת לוואי. הרצת בקרה מול הקוד שלפני #3351, אותו קלט: לפני הצעות `['needle','needlehelper']`, אחרי `[]`. המתג מאפשר להחזיר את החימום מ-ENV, בלי דיפלוי קוד, כדי לשקול את העלות מול התועלת על נתונים אמיתיים. הוא כפוף ל-`SEARCH_MEMORY_INDEX_ENABLED`. תיקון זליגה באינדקס ב-`_text_search` ------------------------------------- `word_index.get(word, set())` מחזיר את הקבוצה השמורה עצמה, וה-`update` אחריו הזליג את התאמות ה-prefix לתוכה לצמיתות. מאותו רגע הן נספרו כהתאמות מדויקות וקיבלו 2.0 במקום 1.0, כלומר הדירוג נשחק בכל חיפוש נוסף על אותו אינדקס. תצוגת ההשלמה האוטומטית ----------------------- שלושה שורשים, כולם נמדדו ב-Chromium על ה-CSS של הריפו: - הוובאפ אינו טוען את ה-CSS של Bootstrap, ולכן `list-group-item` על כל הצעה לא עיצב דבר: ההצעות היו `<a>` עם `display:inline` וללא ריפוד, וחמש מהן נדחסו לשורה אחת בגובה 20 פיקסל. הוחלף במחלקה של הפרויקט, `search-suggestion`. - `overflow-x:hidden` על `.filters-row` גרם לדפדפן לחשב `overflow-y: auto`. השורה הפכה למיכל גלילה אנכי שחתך את הדרופדאון, ולכן הוא נראה רק בגלילה בתוך אזור צר שלא נועד לכך. הוחלף ב-`overflow-x: clip`, עם `hidden` כנפילה-לאחור. - הדרופדאון היה מעוגן לעמודה כולה ולא לתיבת הקלט. הועבר לתוך `.search-box-wrapper`, ועכשיו רוחבו וקצהו זהים לתיבה. בנוסף: `background:#fff` הקשיח הוחלף בטוקני `--solid-surface-*`. הם נמדדו על כל שבע ערכות הנושא — ניגודיות 17.7 בכולן, כולל `custom`, שבו `--bg-secondary` אינו מוגדר כלל והתיבה הייתה יוצאת שקופה. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RgnHZBJaH4VhdFFgwYZKLr
עדכון: מתג לחימום האינדקס, תיקון זליגה, ותיקון תצוגת ההשלמההקומיט 1. רגרסיה שנמצאה בסקירה, ואומתה בהרצת בקרההסרת בניית האינדקס מחיפוש הרצת בקרה מול הקוד שלפני הקומיט הראשון, אותו קלט בדיוק:
מה שנשאר עובד: שמות קבצים, תגיות ושפות — הם מגיעים מ- הפתרון: 2. זליגה לאינדקס ב-
|
| ערכה | --bg-secondary |
--solid-surface-bg |
|---|---|---|
| classic | 3.9 — מתחת ל-AA | 17.74 |
| ocean / rose-pine-dawn / dark / dim / nebula | 6.07–12.44 | 17.74 |
| custom | לא מוגדר — תיבה שקופה | 17.74 |
מדידות אחרי, בארבעה שילובים
בהיר וכהה, 1280 ו-390 פיקסל — זהה בכולם: overflow-y: visible, הדרופדאון נראה מתחת לשורה (נבדק ב-elementFromPoint, לא בגאומטריה), display: block לכל פריט, מפריד 1px, ריפוד 8.8px, גובה תיבה 214 פיקסל במקום 20, יישור מדויק לתיבת הקלט, רקע אטום, ואין גלילה אופקית בעמוד.
שתי מוטציות מוכיחות שהמדידה מסוגלת ליפול: החזרת overflow-x:hidden מחזירה את overflow-y ל-auto ואת visible_below_row ל-false; הסרת מחלקת הפריט מחזירה את ההצעות לשורה אחת. בלי זה המדידה לא הייתה ראיה.
בדיקות
187 עוברים בכל קובצי הטסט הנוגעים ל-search_engine, ל-config ול-Config Inspector. בנוסף 832 עוברים במקטע הטסטים של ה-webapp והתבניות; שלוש הנפילות שם הן ModuleNotFoundError: No module named 'github' בסביבת הפיתוח, והן מופיעות זהות גם על הקוד שלפני השינוי.
ממצאים לדיווח, שלא תוקנו כאן
--solid-surface-*מוגדר פעם אחת ב-:rootואין לו override לערכות כהות. כלומר גם תפריט השפות באותו עמוד הוא לוחית לבנה בערכה כהה, וכך גם ההשלמה עכשיו. הבחירה בטוקנים האלה נעשתה כי הם היחידים שאטומים ועוברים AA בכל שבע הערכות; התיקון הנכון הוא להוסיף להם override כהה ב-base.html, וזה משנה שני רכיבים בבת אחת ולכן לא נכנס ל-PR הזה.overflow: clipנמדד ב-Chromium בלבד — אין כאן Firefox או WebKit. הנפילה-לאחור ל-hiddenשומרת על ההתנהגות הקיימת בדפדפן שלא מכיר אותו.
Generated by Claude Code
* perf(search): האינדקס בזיכרון נבנה רק בחיפושים שקוראים ממנו `AdvancedSearchEngine.search` בנה את `SearchIndex` לפני שבדק איזה סוג חיפוש התבקש. רק `TEXT` ו-`FUNCTION` קוראים מהאינדקס; `CONTENT`, `REGEX` ו-`FUZZY` סורקים את ה-DB בעצמם ואינם נוגעים בו. מכיוון ש-`CONTENT` הוא ברירת המחדל של החיפוש ב-WebApp, המסלול הרגיל שילם על סריקה מלאה של כל קבצי המשתמש (כולל `code`, במנות של `SEARCH_PAGE_SIZE`, עם קיבוץ גרסאות מלא בכל עמוד) שאיש לא קרא את תוצאתה — ומיד אחריה סריקה מלאה שנייה לחיפוש עצמו. מה השתנה: - `get_index` נקרא רק בענפי `TEXT` ו-`FUNCTION`, דרך `_get_index_for_search` ששומרת על מדידת `search_index_get` הקיימת. - הפרמטר `index` הוסר מ-`_fuzzy_search`; הוא התקבל ומעולם לא נקרא, וזה מה שהסתיר את העובדה שהמסלול אינו נשען על האינדקס. - נוסף `SEARCH_MEMORY_INDEX_ENABLED` (ברירת מחדל `true`) לכיבוי מלא של האינדקס בזיכרון. כבוי ⇒ `TEXT` ו-`FUNCTION` מחזירים רשימה ריקה, ו- `_safe_search` ב-WebApp נופל לחיפוש `$text` במונגו. מה שנעלם: התאמה חלקית (prefix/substring) ו-`function_index`. התיעוד ב-`docs/workflows/search-flow.rst` עודכן בהתאם, כולל היחס בין האינדקס בזיכרון לאינדקס `search_text_idx` של מונגו. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RgnHZBJaH4VhdFFgwYZKLr * fix(search): מתג לחימום האינדקס, ותיקון תצוגת ההשלמה האוטומטית המשך ל-#3351, אחרי שהתברר שהסרת בניית האינדקס מחיפוש `content` גררה רגרסיה בהשלמה האוטומטית — ושהתצוגה שלה שבורה ממילא. מתג `SEARCH_MEMORY_INDEX_EAGER_BUILD` (ברירת מחדל `false`) ---------------------------------------------------------- `suggest_completions` קורא את האינדקס דרך `_get_ready_index`, שבמכוון אינו בונה אותו — ולכן ההשלמה נהנתה מאינדקס שחיפוש `content` בנה כתופעת לוואי. הרצת בקרה מול הקוד שלפני #3351, אותו קלט: לפני הצעות `['needle','needlehelper']`, אחרי `[]`. המתג מאפשר להחזיר את החימום מ-ENV, בלי דיפלוי קוד, כדי לשקול את העלות מול התועלת על נתונים אמיתיים. הוא כפוף ל-`SEARCH_MEMORY_INDEX_ENABLED`. תיקון זליגה באינדקס ב-`_text_search` ------------------------------------- `word_index.get(word, set())` מחזיר את הקבוצה השמורה עצמה, וה-`update` אחריו הזליג את התאמות ה-prefix לתוכה לצמיתות. מאותו רגע הן נספרו כהתאמות מדויקות וקיבלו 2.0 במקום 1.0, כלומר הדירוג נשחק בכל חיפוש נוסף על אותו אינדקס. תצוגת ההשלמה האוטומטית ----------------------- שלושה שורשים, כולם נמדדו ב-Chromium על ה-CSS של הריפו: - הוובאפ אינו טוען את ה-CSS של Bootstrap, ולכן `list-group-item` על כל הצעה לא עיצב דבר: ההצעות היו `<a>` עם `display:inline` וללא ריפוד, וחמש מהן נדחסו לשורה אחת בגובה 20 פיקסל. הוחלף במחלקה של הפרויקט, `search-suggestion`. - `overflow-x:hidden` על `.filters-row` גרם לדפדפן לחשב `overflow-y: auto`. השורה הפכה למיכל גלילה אנכי שחתך את הדרופדאון, ולכן הוא נראה רק בגלילה בתוך אזור צר שלא נועד לכך. הוחלף ב-`overflow-x: clip`, עם `hidden` כנפילה-לאחור. - הדרופדאון היה מעוגן לעמודה כולה ולא לתיבת הקלט. הועבר לתוך `.search-box-wrapper`, ועכשיו רוחבו וקצהו זהים לתיבה. בנוסף: `background:#fff` הקשיח הוחלף בטוקני `--solid-surface-*`. הם נמדדו על כל שבע ערכות הנושא — ניגודיות 17.7 בכולן, כולל `custom`, שבו `--bg-secondary` אינו מוגדר כלל והתיבה הייתה יוצאת שקופה. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RgnHZBJaH4VhdFFgwYZKLr * fix(webapp): רשימת ההצעות נכנסה מתחת לכרטיס הבא אחרי שההצעות קיבלו גובה אמיתי, התגלה שהרשימה נחתכת מאחורי הכרטיס הבא בעמוד. נמדד ב-Chromium: מתוך חמש הצעות, שתיים היו מכוסות על ידי תיבת החיפוש שבכרטיס שאחריו. השורש: ל-`.glass-card` יש `backdrop-filter: blur(20px)`, וזה יוצר stacking context. לכן `z-index:1000` של רשימת ההצעות כלוא בגבולות הכרטיס המארח, והכרטיס עצמו הוא `position:static; z-index:auto` — כלומר הכרטיס הבא ב-DOM נצבע מעליו, ושום z-index פנימי אינו יכול לגבור על כך. התיקון מרים את הכרטיס המארח עצמו (`position:relative; z-index:3`), כי זו הרמה שבה ההכרעה באמת מתקבלת. הערך נמוך בכוונה: מודאלים ותפריטים גלובליים יושבים ב-1000 ומעלה וממשיכים לגבור. אימות: 12 הצעות, שתי ערכות נושא, שני רוחבים — אפס פריטים מכוסים בכל השילובים. שתי מוטציות מוכיחות שהמדידה מסוגלת ליפול: הסרת ה-z-index מהכרטיס מחזירה כיסוי של עד ארבעה פריטים, והחזרת `overflow-x:hidden` לשורה מחזירה כיסוי של עד שבעה. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RgnHZBJaH4VhdFFgwYZKLr --------- Co-authored-by: Claude <noreply@anthropic.com>
2 participants
✨ תיאור קצר
AdvancedSearchEngine.searchבנה אתSearchIndexלפני שבדק איזה סוג חיפוש התבקש. רקTEXTו-FUNCTIONקוראים מהאינדקס —CONTENT,REGEXו-FUZZYסורקים את ה-DB בעצמם ואינם נוגעים בו. מכיוון ש-CONTENTהוא ברירת המחדל של החיפוש ב-WebApp, המסלול הרגיל שילם על סריקה מלאה של כל קבצי המשתמש שאיש לא קרא את תוצאתה, ומיד אחריה על סריקה מלאה שנייה לחיפוש עצמו. בנוסף נוסף מתג סביבה לכיבוי מלא של האינדקס בזיכרון.📦 שינויים עיקריים
פירוט:
search_engine.py—get_indexנקרא רק בענפיTEXTו-FUNCTION, דרך_get_index_for_searchששומרת על מדידתsearch_index_getהקיימת.search_engine.py— הפרמטרindexהוסר מ-_fuzzy_search. הוא התקבל ומעולם לא נקרא בגוף הפונקציה, וזה מה שהסתיר את העובדה שהמסלול אינו נשען על האינדקס. הקורא היחיד הוא ה-dispatch עצמו, ואף טסט אינו קורא למתודה ישירות.search_engine.py+config.py— מתגSEARCH_MEMORY_INDEX_ENABLED(ברירת מחדלtrue). כבוי:get_indexמחזיר אינדקס ריק בלי בנייה ובלי לשמור אותו ב-self.indexes,TEXTו-FUNCTIONמחזירים רשימה ריקה, ו-_safe_searchב-WebApp נופל משם לחיפוש$textבמונגו. מה שנעלם בכיבוי:function_index(שמונגו אינו יודע לייצר) וההתאמה החלקית prefix/substring של_text_search(ש-$textאינו תומך בה).services/config_inspector_service.py— הצהרה על המשתנה החדש.docs/—environment-variables.rst,configuration.rst, ו-workflows/search-flow.rst(כולל הבהרת היחס בין האינדקס בזיכרון לאינדקסsearch_text_idxשל מונגו).🧪 בדיקות
הטסטים החדשים הורצו על הקוד הישן ונפלו שם לפני שנכתב התיקון — שלוש נפילות (
CONTENT,REGEX,FUZZY), ושלושה שעברו כצפוי (TEXT,FUNCTION, ו-fallback ל-SEMANTIC).tests/test_search_engine_index_built_only_when_used.py:CONTENT/REGEX/FUZZY, וכן נבנה ב-TEXT/FUNCTIONובנפילה-לאחור של סוג לא מוכר.self.indexes.בונה אינדקס חיפוש עבור משתמש <id>עםrebuild_indexהאמיתי — נעדרת ב-CONTENT, קיימת ב-TEXT. גם הוא הורץ על הקוד הישן ונפל שם.tests/test_config.py— ברירת המחדל של השדה ב-BotConfigוקריאתו מה-ENV.הרצות: 168 טסטים עוברים בכל קובצי הטסט שנוגעים ב-
search_engine, ב-configוב-Config Inspector. בסוויטה המלאה בסביבת הפיתוח יש 33 נפילות — אותן 33 בדיוק גם על הקוד שלפני השינוי (הושווה ריצה מול ריצה). כולן נובעות מתלויות חסרות בקונטיינר (PyGithub, playwright, גופני PIL, posthog) ומהיעדר רשת, ואף אחת מהן אינה בקובץ שהשינוי נוגע בו.דיאגרמת ה-mermaid המעודכנת נבדקה בפריסה מול mermaid 11.12.0 שב-
node_modulesשל הריפו, ולא רק נכתבה.🧪 בדיקות נדרשות ב‑PR
📝 סוג שינוי
📚 עיון בתיעוד
עיינתי ב-CodeBot – Project Docs. הקבצים:
AI-MAP.md,docs/workflows/search-flow.rst,docs/doc-authoring.rst,docs/testing.rst,docs/environment-variables.rst,docs/configuration.rst. בנוסףamir-bug-patterns:claude-md-snippets/testing.md,CRITICAL-PATTERNS.mdK11,bugbot-rules/widened-exception-scope.md,bugbot-rules/line-number-coupling.md.שתי אי-דיוקים קיימים מראש ב-
docs/workflows/search-flow.rst, ששניהם ישבו בתוך הבלוקים שנכתבו מחדש כאן:should_rebuildמגדיר 30 דקות. הניסוח הוחלף בהפניה ל-should_rebuildבמקום לנקוב במספר.fuzz.ratio()._fuzzy_searchמשתמש ב-partial_ratio;ratioשייך ל-_function_search.🔎 ממצאים שלא תוקנו כאן
_regex_searchב-search_engine.pyאכן אינו בודק ReDoS, בעוד_is_regex_safeב-webapp/app.pyכן מריץ היוריסטיקות לפני הקריאה למנוע. לא נגעתי בזה — זה מחוץ לשינוי, ולא בדקתי את מסלול הבוט.AdvancedSearchEngine.get_search_statisticsהוא קוד מת: אין לו אף קורא בריפו. הוא קורא ל-get_index, ולכן עם המתג כבוי יקבל אינדקס ריק. לא נגעתי בו.self.indexesנצבר לפיuser_idואין לו eviction. מחוץ לתחום ה-PR הזה.🤖 Generated with Claude Code
https://claude.ai/code/session_01RgnHZBJaH4VhdFFgwYZKLr
Generated by Claude Code
Summary by Sourcery
Optimize search by building the in-process index only when needed, add configurable index controls, and improve search suggestions and their documentation.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: