Claude/mcp codekeeper webapp ldnzsg - #3193
Conversation
שלושה כלים חדשים מעל קולקציית sticky_notes הקיימת של הוובאפ: - codekeeper_list_notes — פתקי הקובץ (קריאה טהורה, בלי ה-backfill של ה-GET בוובאפ) - codekeeper_create_note — פתק חדש על קובץ קיים; line מעגן לשורת מקור, בלעדיו הפתק נוצר צף עם sentinel __floating__ מפורש (אחרת ה-JS מעגן אוטומטית לשורה הקרובה ודורס את הכוונה) - codekeeper_update_note — עדכון חלקי לפי note_id, עם annotation נפרד (destructive+idempotent) כי פתק נדרס במקום ואין לו היסטוריית גרסאות עקרונות: - כותבים בדיוק את סכמת הוובאפ (scope_id מ-sticky_notes_scope.make_scope_id הקנוני, ברירות מחדל בפריטת הקליינט) — פתק מה-MCP מופיע מיד ב-UI - זהות תמיד מהטוקן; יצירה/עדכון מאחורי require_write - תוכן >5000 תווים נדחה בשגיאה (לא קיטום שקט — סוכן לא ישים לב לאובדן) - מגן אנטי-לולאה: עד 200 פתקים לקובץ ביצירה - אינדקס (user_id, scope_id) שחסר היום נוצר lazy/best-effort — משרת גם את שאילתת ה-scope הזהה של הוובאפ - בלי מחיקה ובלי תזכורות (non-goal מתועד); אפס שינויי קוד בוובאפ — דיפלוי לשירות ה-MCP בלבד בדיקות: 19 טסטים הרמטיים חדשים (סניטציה, ולידציות, sentinel, צבעים, scope filter מול make_scope_id האמיתי, סריאליזציה) + עדכון טסט הרישום. 188 טסטי MCP עוברים; black/flake8/doc8 נקיים; Sphinx נבנה עם 0 אזהרות. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
הוספת פריט "תיאור" בתפריט 3-הנקודות בעמוד הצפייה בקובץ (view_file), ראשון ברשימה, שפותח מודאל קטן לעריכת שדה התיאור — חוסך את הכניסה לעריכת קובץ מלאה רק כדי לשנות תיאור. "נעץ לדשבורד" עלה לשני, "שתף" לשלישי. - משתמש ב-endpoint הקיים POST /api/file/<id>/quick-update שמעדכן את התיאור in-place (מטא-דאטה, בלי גרסה חדשה) — אין קוד שרת חדש - מודאל בדפוס מודאל השיתוף הקיים (Escape, לחיצה על הרקע, טוסט הצלחה), עם textarea ומונה תווים (עד 500, תואם למגבלת ה-endpoint) - עדכון חי של התצוגה מתחת לשם ושל תווית התפריט בלי רענון עמוד - זמין לכל הקבצים (התיאור הוא מטא-דאטה אוניברסלי) - אין בעיית מודאל-בתוך-מודאל: התפריט (dropdown) נסגר לפני שהמודאל נפתח, בדיוק כמו "שתף קובץ" הקיים - שינוי template בלבד; דורש דיפלוי לוובאפ בדיקות: Jinja parse תקין, תחביר JS תקין; אין קוד Python שהשתנה. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
באג: אחרי deploy של תבנית, משתמשים שצפו בקובץ לאחרונה המשיכו לראות את
הגרסה הישנה (בלי אלמנטים חדשים) הרבה זמן; אחרים כן ראו חדש. השורש:
ה-ETag של /file/<id> ו-/md/<id> חושב מנתוני הקובץ בלבד (updated_at/תוכן/
version/theme) בלי גרסת ה-deploy — אז קובץ שלא נערך החזיר ETag זהה בין
deploys → הדפדפן קיבל 304 והציג HTML ישן מה-cache. תורם שני: מסלול
If-Modified-Since מבוסס updated_at החזיר 304 בנפרד.
תיקון שורשי (webapp/app.py + cache_manager.py):
- _compute_file_etag כולל עכשיו את _STATIC_VERSION (גרסת deploy) — כל
deploy מבטל ETags ישנים. קורא מרכזי אחד ⇒ מכסה view_file וגם md_preview.
- שלושת מסלולי If-Modified-Since מכבדים RFC 7232 §3.3: מדלגים כשקיים
If-None-Match (אחרת 304 מיושן גם אחרי שה-ETag השתנה).
- מפתח ה-cache צד-שרת של md_preview כולל את גרסת ה-deploy (אחרת HTML
מרונדר ישן מוגש עד 30 דק').
- נלווה: invalidate_file_related מבטל עכשיו גם את המפתח האמיתי
web:md_preview:user:*:{file_id}:* (היה prefix שגוי — עריכת קובץ לא
ביטלה את cache ה-md שלו).
ב-Render גרסת ה-deploy מגיעה מ-RENDER_GIT_COMMIT (משתנה לכל commit).
אימות: py_compile + flake8 + 9 טסטי cache/invalidation עוברים. את לוגיקת
ה-ETag אימתתי בבידוד (הרצת הפונקציה האמיתית: גרסת deploy משנה את ה-ETag,
יציב לאותו קלט, ותוכן עדיין משנה). טסט יחידה שמייבא webapp.app לא ישים —
טסטי ה-webapp מדולגים ב-CI ("צינור הבוט") ו-flask לא זמין בסביבה.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
מתחת לכל קובץ באוסף שיש לו תיאור מופיע אייקון ℹ️; לחיצה פותחת מודאל קטן (קריאה בלבד) עם התיאור — בלי להיכנס לקובץ. - Backend: get_collection_items מצרף עכשיו את ה-description של הקובץ לכל פריט, דרך אותו batch שכבר מחשב is_file_active (הרחבת ה-projection ל-file_name+description) — בלי N+1 ובלי שדות כבדים (Smart Projection נשמר; description ≤500 תווים). קובץ בלי תיאור/לא-פעיל ⇒ "". - Frontend: אייקון ℹ️ ב-.collection-card__meta רק אם יש תיאור; openDescriptionModal בדפוס .collection-modal הקיים (Escape/רקע סוגרים, textContent — בטוח מ-XSS). - מצב workspace לא נכלל בשלב זה. בדיקות: 3 טסטי enrichment חדשים (fakes שתומכים ב-projection) + 49 טסטי collections_manager עוברים; node --check ל-JS; py_compile. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
אייקון התיאור בכרטיס קובץ עבר משורת ה-meta אל צד שם הקובץ (משמאל, RTL) באותה שורה, עם רווח. כשהשם ארוך ונשבר לשתי שורות — האייקון "קופץ" לשורת 4 הכפתורים, משמאל להם (פונקציה layoutDescIcons שמנצלת את זיהוי is-wrapped של autoFitText, בסדר reset→autoFit→move כדי למנוע oscillation). ארכיון לאוספים: שדה is_archived חדש (נפרד מ-is_active), toggle 🗄️ "הצג ארכיון" בסיידבר, וכפתור ארכב/שחזר בכותרת האוסף (מגודר ל-non-workspace). list_collections קיבל archived_only ו-include_archived; ברירת המחדל מחריגה מאורכבים (ne:True מכסה גם אוספים ישנים ללא השדה). הקאש כבר מבחין לפי querystring, וה-PUT מנקה את שתי התצוגות. הגיבוי האישי משתמש ב-include_archived=True כדי לא לפספס אוספים בארכיון. - database/collections_manager.py: doc-build, allow-list, list filter, serializer, index+backfill - webapp/collections_api.py: פרמטר archived ב-GET, דילוג על "שולחן עבודה" בתצוגת ארכיון - webapp/static/js/collections.js + collections.css: אייקון, toggle, כפתורי ארכב/שחזר - services/personal_backup_service.py: include_archived=True בגיבוי ובבדיקת כפילות - tests/test_collections_archive.py: ארכוב/שחזור/include/legacy Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
main קלט את אייקון-התיאור הבסיסי דרך squash (#3191), ולכן נוצר קונפליקט מול העבודה החדשה בענף (הזזת האייקון + ארכיון). מיזגתי את main לענף ופתרתי: - collections.css: נשמרה הגרסה שלי (superset — בסיס .desc-info + flex + Slot 2 + toggle ארכיון). - collections.js: הוחזר לגרסה שלי כדי למנוע שכפול של כפתור התיאור שהמיזוג האוטומטי יצר. תוצאת המיזוג זהה בדיוק לעבודה שכבר נבדקה (36 טסטים ירוקים). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
לפי theming_and_css.rst אסורים צבעים קשיחים בקבצי רכיבים — רק var(--token). שתי חריגות הומרו: - מודאל "ערוך תיאור" (view_file.html): הרקע הכהה הקבוע (#1f2a44, #fff, rgba לבנים, focus בצבע primary קשיח) הוחלף בטוקנים סמנטיים — bg-secondary/tertiary, text-primary/secondary/muted, glass-border, primary; ה-scrim וה-shadow קיבלו טוקן-רכיב עם fallback (var(--modal-backdrop, ...), var(--solid-surface-shadow, ...)). כך המודאל מקבל את צבעי הערכה גם בערכות בהירות (rose-pine-dawn, classic). - כפתור "הצג ארכיון" במצב לחוץ (collections.css): rgba לבנים קשיחים הוחלפו בטוקני glass קיימים (glass-hover/glass-border/glass). מודאל ה-ℹ️ באוספים נבדק ונמצא תקין (יורש var(--collections-modal-*)) — ללא שינוי. מודאל השיתוף הסמוך הוא legacy קיים ולא נכלל (חוב נפרד). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughנוספה תמיכה בארכוב ושחזור אוספים במסד הנתונים, ב־API, בממשק ובגיבויים. נוספו סינון אוספים בארכיון, בדיקות ייעודיות, התאמות לפריסת כרטיסים ושימוש במשתני Theme במודאל התיאור. Changesארכיון אוספים
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CollectionsUI
participant CollectionsAPI
participant CollectionsManager
participant Database
User->>CollectionsUI: בחירת ארכיון או ארכוב אוסף
CollectionsUI->>CollectionsAPI: בקשת GET או PUT
CollectionsAPI->>CollectionsManager: list_collections או update_collection
CollectionsManager->>Database: סינון או עדכון is_archived
Database-->>CollectionsManager: תוצאות אוספים
CollectionsManager-->>CollectionsAPI: רשימת אוספים
CollectionsAPI-->>CollectionsUI: תצוגת אוספים מעודכנת
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
…קט collections.css main קלט את הארכיון והזזת האייקון דרך squash (#3192), והענף ממשיך עם תיקון הטוקנים (991b1ad) שנגע באותה שורה. נשמרה גרסת הטוקנים של #toggleArchivedBtn (var(--glass-*)) — ההבדל היחיד מול main, שהוחלף בכוונה. תוצאת המיזוג זהה לחלוטין לעץ שכבר נבדק. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
🧯 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
database/collections_manager.py (1)
244-252: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winהסרת עדכון גורף בשלב האתחול לשיפור ביצועים.
המיגרציה הזו מתבצעת באופן סינכרוני במהלך האתחול של
CollectionsManager(למשל, בעליית כל תהליך של השרת). במסדי נתונים עם כמות גדולה של אוספים, שאילתת{"$exists": False}תגרום לסריקת טבלה מלאה (COLLSCAN) שעלולה לנעול את מסד הנתונים ולהאט משמעותית את זמן העלייה של המערכת.מכיוון שהלוגיקה ב-
list_collectionsכבר מטפלת נכון במסמכים שחסר בהם השדה (בעזרת{"$ne": True}בשורה 822), כדאי לשקול להסיר את העדכון הגורף הזה כאן. במידת הצורך, ניתן להריץ אותו מאוחר יותר כסקריפט תחזוקה נפרד ברקע.♻️ הצעה להסרת המיגרציה הסינכרונית
- # backfill — אוספים קיימים ללא is_archived יקבלו False (לניקיון וליעילות אינדקס; - # התקינות מובטחת גם בלעדיו בזכות $ne:True בשאילתת הרשימה) - try: - self.collections.update_many( - {"is_archived": {"$exists": False}}, - {"$set": {"is_archived": False}}, - ) - except Exception: - pass🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@database/collections_manager.py` around lines 244 - 252, Remove the synchronous backfill block from CollectionsManager initialization, including the update_many call for documents missing is_archived and its surrounding exception handling. Leave list_collections unchanged so its existing {"$ne": True} filtering continues to handle documents without the field.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@database/collections_manager.py`:
- Around line 244-252: Remove the synchronous backfill block from
CollectionsManager initialization, including the update_many call for documents
missing is_archived and its surrounding exception handling. Leave
list_collections unchanged so its existing {"$ne": True} filtering continues to
handle documents without the field.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f153af46-d65c-4654-b8bb-65e1b2f6462b
📒 Files selected for processing (8)
database/collections_manager.pyservices/personal_backup_service.pytests/test_collections_archive.pywebapp/FEATURE_SUGGESTIONS/MY_COLLECTIONS_IMPLEMENTATION_GUIDE.mdwebapp/collections_api.pywebapp/static/css/collections.csswebapp/static/js/collections.jswebapp/templates/view_file.html
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
תוקן ונדחף (commit
991b1ad). ✅סיכום התשובות והתיקון
מודאל "ערוך תיאור" (עמוד הקובץ) — צדקת, היה הארדקוד ❌. חיקיתי את מודאל השיתוף הסמוך באותו קובץ, שהוא עצמו legacy קשיח (
#1f2a44,#fff,rgbaלבנים). תוקן עכשיו לפיtheming_and_css.rst:var(--bg-secondary), טקסט →var(--text-primary)var(--text-secondary), מונה תווים →var(--text-muted)var(--bg-tertiary)+ מסגרתvar(--glass-border), focus →var(--primary)(במקום ה-rgba שהיה בעצם primary קשיח)var(--modal-backdrop, ...),var(--solid-surface-shadow, ...)) — בדיוק הדפוס המאושר שבדוגמה במסמךעכשיו המודאל יקבל את צבעי הערכה גם ב-rose-pine-dawn/classic במקום להישאר כחול-כהה.
מודאל ה-ℹ️ באוספים — הוגדר לפי טוקנים מלכתחילה ✅: הפאנל הוא
.collection-modal__panelהקיים שצבוע ב-var(--collections-modal-bg)/var(--collections-modal-text)(טוקני-רכיב עם overrides לכל ערכה בראשcollections.css), וה-.desc-modal__textשהוספתי לא מגדיר צבעים בכלל — יורש מהפאנל. לא נדרש שינוי.בונוס מהסריקה: מצאתי חריגה שלישית שלי — מצב "לחוץ" של כפתור הארכיון 🗄️ היה עם
rgbaלבנים קשיחים (הועתק מדפוס.sidebar-item.activeהישן). הומר לטוקני glass קיימים (--glass-hover/--glass-border/--glass).שני דברים שכדאי לדעת:
.share-link-modalב-view_file.html) נשאר קשיח — הוא legacy שלא כתבתי, לא נגעתי בו כדי לא להרחיב סקופ. אם תרצה, אמיר אותו לטוקנים באותה תבנית בסבב נפרד.variables.cssהוא מצב-היעד — בפועל הטוקנים חיים היום בבלוקי:rootבתוךbase.html.הכול בענף
claude/mcp-codekeeper-webapp-ldnzsg(PR #3192). דרוש deploy לוובאפ בלבד, ואחריו כדאי לעבור על המודאל בכמה ערכות (dark / classic / rose-pine-dawn / high-contrast) לוודא שהכול נראה טוב.