Repository navigation
Conversation
…G-004, SUGG-010 (#3433) - SUGG-004: שורת הקיבולת קוראת את ThreadPoolExecutor._max_workers הפרטי דרך _installed_width, וכשהמאפיין ייעלם היא מדפיסה את הרוחב שהתבקש עם "(requested; installed width unreadable)" ו-WARNING שאומר למה — במקום שהשירות לא יעלה בגלל שורת לוג, ובמקום להדהד את הבקשה כאילו היא המצב (state-record-without-state-change). אותו כלל גם ב-WARNING של ה-fallback. - SUGG-010: md_parser.token_count — פונקציה ציבורית ומתועדת שסופרת טוקנים על _MD, המופע שהכלי מריץ — במקום שהסקריפט יקרא ל-_build_parser הפרטית ויבנה פרסר חדש לכל קובץ. מחוץ ל-__all__ בכוונה: הרשימה שם היא החוזה של "שני פארסרים בני-החלפה", וטסט מקבע את ההפרש בינה לבין זו של rst_parser. - SUGG-014: שני טסטים לקובץ memory.max שקיים אבל ריק (IndexError) או לא-מספרי (ValueError) — שניהם נופלים ל-cgroup v1 ומחזירים את הערך שלו. כיסוי השורות היה מלא; התרחיש חסר. מוטציה שמסירה כל אחת מהחריגות מה-except מפילה את הטסט שלה. - SUGG-001: האסרשן שלא יכול היה ליפול הוחלף: os.cpu_count מוצמד ל-16 כדי ש"cpu_count + 4" יהיה 20, מספר שהרצפה לעולם אינה — ומוטציה שמחזירה את ה-fallback ל-min(32, cpu_count + 4) מפילה אותו. על origin/main: שלושה טסטים נופלים (הרוחב הלא-קריא, token_count, הסקריפט דרך הפונקציה הציבורית); שלושת האחרים עוברים שם ומוכחים במוטציות. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SfJTSpDAhDr2yhtmpFkwTx
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
There was a problem hiding this comment.
Sorry @amirbiron, you've used your own review budget of 250,000 diff characters for the last 7 days.
You can request another review in 13 minutes by commenting @sourcery-ai review. Upgrade to get a review now.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reachedNext included review available in 52 minutes. 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: Advanced Run ID: 📒 Files selected for processing (7)
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) |
Reviewer's GuideThis maintenance PR hardens MCP startup diagnostics, exposes shared Markdown token counting for the measurement script, adds missing cgroup parsing coverage, and fixes an ineffective fallback assertion, with corresponding documentation and tests. Sequence diagram for resilient MCP capacity loggingsequenceDiagram
participant Startup
participant Logger
participant Pool
participant Sizing
Startup->>Logger: _log_dispatch_capacity(read_pool, memory_display, sizing)
Logger->>Pool: _installed_width(read_pool)
alt _max_workers available
Pool-->>Logger: installed width
Logger->>Sizing: _width_label(read_pool, sizing)
Sizing-->>Logger: installed width
else _max_workers unavailable
Pool-->>Logger: None
Logger-->>Logger: warning(unreadable installed width)
Logger->>Sizing: _width_label(read_pool, sizing)
Sizing-->>Logger: requested width (unreadable)
end
Logger-->>Startup: capacity line, startup continues
Sequence diagram for shared Markdown token countingsequenceDiagram
participant Script as measure_md_parse_cost.py
participant Parser as md_parser
participant MD as _MD
Script->>Parser: token_count(text)
Parser->>MD: parse(text)
MD-->>Parser: block tokens
Parser-->>Script: token count
Script-->>Script: compute density per KB
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
⏱️ Performance report(No performance test durations collected. Mark tests with |
📖 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! |
|
נכנס דרך #3443 |
✨ תיאור קצר
md_parser), SUGG-014 (אין טסט לקובץmemory.maxשקיים אבל לא ניתן לפענוח), ו-SUGG-001 (אסרשן שלא יכול ליפול). SUGG-006 נשאר דחוי עם הטריגר שכתוב באישו, ו-SUGG-008 (העברת לוגיקת התקציב למודול משלו) הוא PR ריפקטור נפרד.claude/gracious-einstein-sevk8pנושא את ה-PR הפתוח של docs_get_section: תקרה ל-candidates, ושמירת חוזה Suggestions — לפני שחיבור ה-Markdown נוחת #3426 (fix(mcp): תקרה לרשימת המועמדים של ambiguous_section, וטסט שמקבע את חוזה Suggestions במסלול ה-Markdown (#3426) #3434), ו-PR נפרד דורש ענף נפרד. ה-PR הזה על ענף נגזר,claude/gracious-einstein-sevk8p-3433b, מ-origin/mainהעדכני (1a45c28c), עצמאי מהמנה הראשונה (fix(mirror): get_file_at_commit בודק את גודל האובייקט לפני git show, וקובץ מעל התקרה אינו נטען כלל (#3433, פריט 9) #3437).📦 שינויים עיקריים
פירוט נקודות (רשימת תבליטים):
_installed_widthו-_width_labelב-mcp_server/server.py. האישו הציע שתי דרכים:getattr(read_pool, "_max_workers", sizing.workers)או להצהיר שכשל עלייה כאן מכוון. נבחרה דרך שלישית שמכבדת את שני העקרונות בבת אחת: המאפיין הפרטי עדיין נקרא (state-record-without-state-change— השורה מתארת את המאגר שקיים, לא את זה שהתבקש), אבל כשהוא ייעלם התשובה היאNone— לא הרוחב שהתבקש, שהיה בדיוק ההד שהשורה קיימת כדי למנוע — והשורה מדפיסהread pool 10 (requested; installed width unreadable) threadsעם WARNING שאומר למה. שורת לוג לעולם אינה מפילה את העלייה, וההבטחה בדוקסטרינג נשארת נכונה. אותו כלל גם ב-WARNING של ה-fallback, דרך אותה פונקציה. הפורמט במצב הרגיל לא השתנה (read pool 3 threads, הטסט הקיים).md_parser.token_count(text). פונקציה ציבורית ומתועדת שסופרת טוקנים על_MD, המופע שהכלי מריץ, בליenvולכן בלי תקרה ובלי טוקני inline — כלומר בדיוק מה שהפרסור האמיתי סופר. הסקריפט קרא ל-_build_parserהפרטית ובנה פרסר חדש לכל קובץ; עכשיו התלות גלויה ואין בנייה. מחוץ ל-__all__בכוונה: הרשימה שם היא החוזה של "שני פארסרים בני-החלפה", וטסט מקבע את ההפרש בינה לבין זו שלrst_parser(וגם כדי לא להתנגש עם fix(parsers): rst_parser בודק את הקלט בכניסה, וברירת המחדל של max_sections מיושרת ל-MAX_SECTIONS (#3421, #3420) #3436 שעורך את אותו טסט). הנימוק בדוקסטרינג.tests/test_mcp_to_thread.py:memory.maxשקיים אבל ריק (split()ריק ←IndexError) ו-memory.maxעם מילה שאינהmax(ValueError) — שניהם נופלים ל-cgroup v1 ומחזירים את הערך שלו, דרך_fake_cgroupהקיים. כיסוי השורות היה מלא; התרחיש חסר.workers != min(32, cpu_count + 4) or workers == 2לא יכול היה ליפול אחרי שהשורה שלפניו קיבעהworkers == 2. הוחלף:os.cpu_countמוצמד ל-16 כדי ש"cpu_count + 4" יהיה 20 — מספר שהרצפה לעולם אינה — ו-fallback שהיה מתייעץ בליבות המארח היה עונה 20 ונופל.docs/whats-new.rst: רשומה אחת לארבעת הפריטים.🧪 בדיקות
origin/main=1a45c28c, עם שלושת קובצי הטסטים): שלושה נופלים — הרוחב הלא-קריא (AttributeErrorעל_max_workers),token_count(אינו קיים), והסקריפט דרך הפונקציה הציבורית (קורא ל-_build_parser); שלושת האחרים (שני טסטי SUGG-014 ו-SUGG-001) עוברים שם, כי הם מקבעים התנהגות קיימת — ולכן הוכחו במוטציות.min(32, cpu_count + 4)← טסט SUGG-001 נופל;IndexErrorמוסר מה-exceptשל קורא ה-v2 ← טסט הקובץ הריק נופל;ValueErrorמוסר ← טסט הקובץ הלא-מספרי נופל.test_mcp_to_thread,test_md_parser,test_measure_md_parse_cost_script,test_mcp_logging_visible(שמריץ uvicorn אמיתי ובודק את שורת הקיבולת בפלט), ועוד 654 ב-test_mcp_server_build,test_mcp_docs_handlers,test_mcp_outline,test_mcp_analytics_privacy,test_mcp_primer,test_md_parser_oracle,test_doc_sections— ירוקים. flake8 עם ה-selection של CI (E9,F63,F7,F82) וגםF401,F841,E303,W291,W293,E501על הקבצים שנגעתי בהם — נקי (נשארE302קיים ב-server.py:206מ-feat(mcp): docs_get_section קורא גם Markdown, לפי מדיניות נתיבים לכל ריפו #3428, שכבר מתוקן ב-fix(mcp): תקרה לרשימת המועמדים של ambiguous_section, וטסט שמקבע את חוזה Suggestions במסלול ה-Markdown (#3426) #3434 וב-fix(mcp): תקרת גודל לגוף בקשה והגבלת קצב לפי זהות, עם סירוב שנוקב בסיבתו (#3431) #3435). docutils עלwhats-new.rst— 0 אזהרות._max_workersבאמת חסר — אין כזה; המקרה מדומה באובייקט בלי המאפיין, שזה בדיוק מה שהקוד היה רואה.🧪 בדיקות נדרשות ב‑PR
📝 סוג שינוי
✅ צ'קליסט
docs/whats-new.rstעודכןAI-MAP.md,docs/mcp-server.rst("מודל הריצה של הכלים" — תיאור שורת הקיבולת),docs/development/scripts.rst(הסקריפט שמודד את עלות הפרסור) | המשפט: "שורת הקיבולת ... קוראת את_max_workersשל המאגר שהותקן בפועל" — הכלל שהמימוש כאן שומר גם כשהמאפיין חסרדפוסי באגים שנקראו ומה שקבעו בקוד:
bugbot-rules/state-record-without-state-change.md— הבחירה ב-SUGG-004: כשהמצב אינו קריא, השורה אומרת זאת ואינה מהדהדת את הבקשה;bugbot-rules/silent-fallback-to-worse-path.md— הרוחב הלא-קריא מדווח ב-WARNING, לא נופל בשקט;RECURRING-PATTERNS.mdR6 —token_countהוא פונקציה אחת במקום בנייה חוזרת של פרסר;claude-md-snippets/testing.md§5 +TESTING-PATTERNS.mdT2 — ריצה על הקוד הישן, ומוטציות לשלושת הטסטים שעוברים שם;bugbot-rules/widened-exception-scope.md— לא הורחב אףexcept(המוטציות דווקא מצרות אותו כדי להוכיח את הטסטים).🧩 השפעות/סיכונים
token_countעל_MDהוא אותו פרסר באותה תצורה).🔗 קישורים
🧯 סיכון / החזרה לאחור (Rollback)
🤖 Generated with Claude Code
https://claude.ai/code/session_01SfJTSpDAhDr2yhtmpFkwTx
Generated by Claude Code
Summary by Sourcery
Address four small review follow-ups across MCP capacity reporting, cgroup limit handling, Markdown measurement, and test reliability.
Bug Fixes:
Enhancements:
Documentation:
Tests: