Claude/mcp codekeeper webapp ldnzsg - #3192
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
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughChangesהעדכון מוסיף ארכוב אוספים עם סינון, UI, גיבוי ותאימות לאוספים ישנים; מעשיר פריטים בתיאורי קבצים; ומעדכן ביטול קאש, ETag ו-Conditional GET. ארכוב אוספים
ביטול קאש קבצים
מטמון HTTP
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant collections_js
participant collections_api
participant CollectionsManager
participant MongoDB
Browser->>collections_js: מעבר לתצוגת ארכיון
collections_js->>collections_api: בקשת archived=1
collections_api->>CollectionsManager: list_collections(archived_only=True)
CollectionsManager->>MongoDB: שאילתת אוספים בארכיון
MongoDB-->>CollectionsManager: תוצאות מסוננות
CollectionsManager-->>collections_api: רשימת אוספים
collections_api-->>collections_js: JSON
collections_js-->>Browser: רינדור ארכיון
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/personal_backup_service.py (1)
924-999: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftשחזור גיבוי לא משמר את סטטוס הארכיון של האוסף.
_export_collectionsכולל אוספים בארכיון (include_archived=True), אבל ב-_restore_collectionsהקריאה ל-mgr.create_collection(...)(שורות 948-956) לא מעבירהis_archived, ו-create_collectionבכלל לא מקבל פרמטר כזה — כל אוסף חדש נוצר עםis_archived=False(הערך הקבוע ב-doc). כתוצאה מכך, שחזור גיבוי שכולל אוספים מאורכבים "משחזר" אותם כפעילים (לא-מאורכבים), בסתירה למטרת התכונה ("תמיכה בגיבוי אוספים מאורכבים").🛠️ הצעת תיקון: שחזור סטטוס is_archived אחרי יצירת האוסף
result = mgr.create_collection( user_id=user_id, name=coll_name, description=coll.get("description", ""), mode=coll.get("mode", "manual"), icon=coll.get("icon"), color=coll.get("color"), is_favorite=coll.get("is_favorite", False), ) if result.get("ok") and result.get("collection"): new_id = result["collection"].get("id") if old_id and new_id: old_to_new_id[old_id] = new_id collections_count += 1 + if coll.get("is_archived") and new_id: + mgr.update_collection(user_id, new_id, is_archived=True)🤖 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 `@services/personal_backup_service.py` around lines 924 - 999, Update _restore_collections around the mgr.create_collection call to preserve each collection’s archived state from the backup. After a new collection is created, read the source collection’s is_archived value and apply it to the created collection using the existing collection-management/archive operation; keep duplicate-name mappings unchanged and ensure collections without the flag remain active.
🤖 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.
Outside diff comments:
In `@services/personal_backup_service.py`:
- Around line 924-999: Update _restore_collections around the
mgr.create_collection call to preserve each collection’s archived state from the
backup. After a new collection is created, read the source collection’s
is_archived value and apply it to the created collection using the existing
collection-management/archive operation; keep duplicate-name mappings unchanged
and ensure collections without the flag remain active.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fd39254a-1936-4d68-acfb-967afa3ccf97
📒 Files selected for processing (10)
cache_manager.pydatabase/collections_manager.pyservices/personal_backup_service.pytests/test_collections_archive.pytests/test_collections_description_enrichment.pywebapp/FEATURE_SUGGESTIONS/MY_COLLECTIONS_IMPLEMENTATION_GUIDE.mdwebapp/app.pywebapp/collections_api.pywebapp/static/css/collections.csswebapp/static/js/collections.js
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
🧯 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 |
📖 Documentation PreviewThe documentation has been built successfully!
To view locally:
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…קט 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
סיכום מה שנעשה (commit
7ea470a)1) הזזת אייקון התיאור בכרטיס אוסף
layoutDescIconsשמנצל את זיהוי ה-is-wrappedהקיים שלautoFitText(סדר reset→autoFit→move כדי למנוע ריצוד).2) ארכיון לאוספים
is_archivedחדש (נפרד מ-is_active).list_collectionsקיבלarchived_onlyו-include_archived; ברירת המחדל מחריגה מאורכבים ($ne:Trueמכסה גם אוספים ישנים בלי השדה).שורש שתיקנתי אגב: הגיבוי האישי (
personal_backup_service) קרא ל-list_collections— עם שינוי ברירת המחדל הוא היה מפספס אוספים בארכיון בגיבוי. הוספתיinclude_archived=Trueבגיבוי ובבדיקת-הכפילות בשחזור, כדי שלא ייווצר אובדן נתונים.בדיקות: 36 טסטי collections (כולל 4 חדשים לארכיון) + 23 MCP + 19 גיבוי — כולם ירוקים. תחביר JS תקין (
node --check), Python מהודר.מה שנשאר לך
Summary by CodeRabbit
תכונות חדשות
שיפורים
תיקוני באגים