diff --git a/docs/mcp-server.rst b/docs/mcp-server.rst index bf3f42c14..b2e7542f8 100644 --- a/docs/mcp-server.rst +++ b/docs/mcp-server.rst @@ -193,6 +193,85 @@ OAuth 2.1) וגם מול **Claude Code / Claude Desktop** (טוקן אישי). בו — לא ב-mirror ולא בריפו ב-GitHub. כתוב כאן כדי שאיש לא יסיק אחרת משם הכלי. +.. _mcp-limits: + +הקבועים, התקרות ואוצר המילים של התשובה +--------------------------------------- + +המספרים כאן חיים בקוד ונאכפים בו. ההפניה היא לשם המודול ולשם הקבוע ולא +למספר שורה, כדי שהטבלה לא תצביע למקום אחר אחרי הריפקטור הבא. + +.. list-table:: + :header-rows: 1 + :widths: 34 16 50 + + * - הקבוע + - הערך + - מה הוא מגביל + * - ``MAX_FILE_SIZE_FOR_DISPLAY`` + - 512,000 + - קריאה מלאה של קובץ. יושב ב-``services/git_mirror_service.py`` + ומשותף עם הוובאפ + * - ``RANGE_READ_MAX_BYTES`` + - 10,485,760 + - קריאת טווח **ואאוטליין**, ב-``mcp_server/repo_backend.py`` + * - ``OUTPUT_BYTE_BUDGET`` + - 256,000 + - תקציב הבתים של תשובה אחת + * - ``TREE_PER_PAGE_DEFAULT`` · ``TREE_PER_PAGE_MAX`` + - 200 · 1000 + - עמוד בעץ הקבצים + * - ``OUTLINE_PER_PAGE_DEFAULT`` · ``OUTLINE_PER_PAGE_MAX`` + - 100 · 500 + - סימבולים בעמוד באאוטליין. קבועים משלו ולא מיחזור של ``TREE_*``, + כי רשומת סימבול שוקלת יותר מנתיב + * - ``REPOS_LIMIT_DEFAULT`` · ``REPOS_LIMIT_MAX`` + - 50 · 200 + - ריפואים ב-``codekeeper_list_repos`` + * - ``SEARCH_RESULTS_DEFAULT`` · ``SEARCH_RESULTS_MAX`` + - 50 · 100 + - פגיעות ב-``codekeeper_search_repo`` + * - ``CONTEXT_LINES_MAX`` + - 10 + - שורות ההקשר סביב פגיעה בחיפוש + * - ``MAX_SYMBOLS`` + - 50,000 + - סימבולים לקובץ באאוטליין. יושב ב-``mcp_server/outline_scanners/_ceiling.py`` + +כל קבוע שהטבלה אינה נוקבת במודול שלו יושב ב-``mcp_server/repo_handlers.py``, +ליד ה-handler שאוכף אותו. + +.. warning:: + + **ערך מעל התקרה נצמד בשקט, ואינו נדחה.** ``per_page=1000`` באאוטליין + מקבל 500, ואין בתשובה שדה שמצהיר שזה קרה — מה שכן יש הוא ``per_page`` + שחוזר בתשובה, והוא **הערך שנאכף**, ולכן זה מה שכדאי להשוות אליו ולא + למה שנשלח. ``per_page=0`` נצמד ל-1 ולא לברירת המחדל. + + **ומה שאינו מספר שלם אינו מגיע להצמדה בכלל — הוא נדחה בסכימה.** נמדד + מול הסכימה של הכלי: ערך עם שבר (``2.5``), ``null``, ומחרוזת שאינה + מספר נדחים **לפני** שהכלי רץ; מחרוזת מספרית (``"250"``) ומספר עשרוני + שלם (``2.0``) עוברים ומומרים. **ובוליאני עובר ונעשה 1**, בלי שגיאה: + ``per_page=true`` מחזיר עמוד של רשומה אחת. זו אותה המרה ש-``lines`` + חוסם ב-``strict`` — ראו את ההערה שם — וכאן היא אינה חסומה. + + ההצמדה היא מדיניות מוצהרת (``_clamp`` ב-``mcp_server/handlers.py``) + ולא מקריות: דחייה על כל ערך מחוץ לטווח הייתה מכשילה קריאה שלמה בגלל + פרמטר עימוד. התקרה על הגודל דווקא **כן** דוחה, וההבדל מכוון — עמוד + שאינו נכנס בתקציב הבתים חוזר כ-``page_too_large`` ולא נחתך בשקט. + +**אוצר המילים של התשובה, ושתי המשפחות אינן מתערבבות.** ``status`` מתאר +הצלחה, ``error`` מתאר סירוב, וב**תשובת כלי** ``error`` מגיע תמיד עם +``"ok": false``. תשובת HTTP של השרת עצמו אינה תשובת כלי ואינה נושאת את +השדה — ``401`` על טוקן פסול, למשל, הוא ``{"error": "invalid_token"}`` +לבדו, עם הכותרת ``WWW-Authenticate``. ל-``status`` יש בתשובת +כלי רשימה סגורה: ``ok``, ``outline``, ``binary``, ``too_large`` +ו-``no_outline``. ל-``error`` אין רשימה כזאת כאן **בכוונה** — הערכים +נגזרים מהכלי שסירב (``path_denied``, ``outline_and_lines``, +``page_too_large``, ``range_out_of_bounds`` ועוד רבים), ורשימה חלקית +שנראית סגורה גרועה מאין רשימה. ערך ``status`` חדש שיתווסף לקוד מתווסף +גם כאן. + אימות והרשאות -------------- @@ -575,7 +654,8 @@ Claude Desktop **מה שאומר אם יש עוד עמודים הוא ``total`` מול ``page`` ו-``per_page``.** **מגבלת הגודל.** אאוטליין נשפט מול אותה תקרה של קריאת טווח — **10MB**, ולא -500KB — כי הוא קורא את הקובץ ומחזיר פלט זעיר. ראו :ref:`mcp-line-range`. +500KB — כי הוא קורא את הקובץ ומחזיר פלט זעיר. ראו :ref:`mcp-line-range`, והמספרים +עצמם — כולל תקרות העימוד — ב-:ref:`mcp-limits`. **``lines`` ו-``outline`` אינם מצטברים.** קריאה שמעבירה את שניהם מוחזרת כ-``{"ok": false, "error": "outline_and_lines"}``, ולא מתעלמת בשקט מאחד @@ -784,6 +864,11 @@ RST הסעיף שמגדיר אותה, ומפה שאומרת באילו שורות הסעיף יושב מאפשרת להמשיך ישר ל-``lines=``. +**ומה שמעגן את הכללים שלמטה:** זיהוי הכותרות מיושר לפארסר של docutils +0.23, ונבדק מול כל קובצי ה-RST של התיעוד הזה — אותן רמות ואותה ספירה בכל +קובץ — ומול טבלאות של צורות כותרת סינתטיות. זה נאמר כאן כי הכללים נראים +כמו החלטות שלנו, והם אינם: הם מה ש-Sphinx יעשה עם אותו קובץ. + .. important:: **אין משמעות מובנית לתו הפיסוק.** ``===`` אינו "רמה 1". ההיררכיה נקבעת @@ -844,6 +929,15 @@ RST ``start`` מצביע לשורה בקובץ המקור, ושם מנורמל לא היה תואם לשורה שאליה הוא מצביע. +.. warning:: + + **מחלקה אחת מוצהרת שאינה ממודלת: בלוק literal מצוטט.** תקן RST מתיר + בלוק שאינו מוזח ששורותיו נפתחות בתו פיסוק, ו-Sphinx קורא את כולו + כטקסט מילולי. הסורק כאן אינו מזהה את הצורה הזאת, ולכן שורת פיסוק + בתוכה עלולה להיראות לו כקו של כותרת — כלומר שורה במפה שאינה כותרת + בעמוד המרונדר. בקובצי התיעוד האלה אין לצורה אף מופע, והיא כתובה כאן + כדי שלא תיראה כבאג במי שייפגש בה בריפו אחר. + **וקובץ RST בלי אף כותרת אינו כשל** — הוא מחזיר מפה ריקה עם ``status: outline``, כמו קובץ CSS בלי כללים. ``no_outline`` שמור לסיומת שאין לה סורק ולתקרת הסימבולים. @@ -888,6 +982,26 @@ outline``, כמו קובץ CSS בלי כללים. ``no_outline`` שמור לסי המקור, ולא גרסה שהשתנתה בשקט. קיצוץ ה-``\r`` היה שובר את שתי התכונות האלה, **וגם** מחזיר ספירה שסותרת את ``file.lines`` שיושב באותה תשובה. +.. note:: + + **``file.encoding`` אומר פחות ממה שהוא נראה, וכדאי לדעת מה.** הפענוח + מנסה קודם חתימת BOM ואחר כך רשימה, ב-``services/git_mirror_service.py``: + + - ``utf-8`` — הקובץ פוענח כ-UTF-8. זה הרוב המוחלט. + - ``utf-8-sig`` — היה **BOM** בתחילת הקובץ. הוא **מוסר מ-``content``**, + וזה החריג היחיד ל"בית-בית כפי שהוא בקובץ" שלמעלה. ``size`` מגיע + מ-git וסופר את הבלוב, כלומר **כולל** את שלושת הבתים שלא תקבלו, + ו-``lines`` נספר על הטקסט אחרי ההסרה. + - ``latin-1`` — **נפילה-לאחור ולא זיהוי.** ``latin-1`` מפענח כל רצף + בתים ואינו נכשל לעולם, ולכן הוא תופס כל מה ש-UTF-8 דחה. נמדד: קובץ + עברי שנכתב ב-``cp1255`` חוזר עם התווית ``latin-1`` ועם תוכן משובש — + ``שלום`` הופך ל-``ùìåí``. התווית נראית בטוחה, והתוכן אינו. + + **המשמעות המעשית:** ``utf-8`` ו-``utf-8-sig`` הם זיהוי, ו-``latin-1`` + הוא "שום דבר אחר לא עבד". קובץ שחוזר עם התווית הזאת ראוי לחשד לפני + שנשענים על תוכנו — ובעברית זה המקרה השכיח, כי ``cp1255`` אינו נגיש + ברשימה אחרי ``latin-1``. + .. list-table:: :header-rows: 1 :widths: 34 66 @@ -1264,6 +1378,12 @@ outline``, כמו קובץ CSS בלי כללים. ``no_outline`` שמור לסי - שאילתה ארוכה משם פתק אפשרי לא תתפוס דבר. קצרו אותה * - ``sync_in_progress`` - רענון רץ ברגע זה — נסו שוב לפי ``retry_after`` + * - אין שום לוג מהשרת, גם כשהכול תקין + - תהליך ה-MCP אינו מגדיר לוגים: הלוגר השורשי ב-``WARNING`` ובאפס + handlers, ולכן רשומות ``debug``/``info`` שכתובות בקוד אינן נכתבות + לשום מקום. **זה אינו סימן שהכלי לא נקרא** — אבחון דרך הלוגים של + השרת אינו זמין היום, ומדידת השימוש עוברת ב-PostHog. חיבור הלוגים + מתועד באישו #3375 * - ``repo_or_ref_not_found`` - ה-mirror עוד לא שוכפל מקומית (המתינו למעבר autosync) או שאין דיסק/``REPO_MIRROR_PATH`` diff --git a/docs/whats-new.rst b/docs/whats-new.rst index c83c75ba7..224cd60c3 100644 --- a/docs/whats-new.rst +++ b/docs/whats-new.rst @@ -4,6 +4,8 @@ What's New 2026-09-14 ---------- +- **fix: אאוטליין של ``.rst`` פרסר את הקובץ כולו לפני שהתקרה נגעה בו.** התקרה על מספר הסימבולים יושבת בסורקים בכוונה — סינון של פלט אינו חסימה של עבודה — אבל בסורק ה-RST היא נגעה רק ברשימה שנבנתה **אחרי** ש-``parse_document`` סיים. כלומר קובץ שנכנס בנוחות לתקרת ה-10MB של קריאת הטווח יכול לקנות עבודה גדולה: נמדד על 10MB של כותרת בת תו אחד בכל שתי שורות — 2.6 מיליון סקשנים — **18.5 שניות ו-1,132MB**, מול **0.30 שניות ו-83MB** אחרי התיקון. הזיכרון הוא מה שמצדיק את זה: בקשה איטית היא בקשה איטית, אבל ג'יגה-בייט בתהליך משותף מפיל שירות. ``parse_document`` מקבל עכשיו ``max_sections`` שברירת המחדל שלו ללא תקרה, ולכן ``docs_get_section`` — הצרכן השני של הפארסר — זהה בית-בית, והתשובה על קובץ שחוצה את התקרה נשארה בדיוק אותה תשובה: ``no_outline`` עם ``too_many_symbols``, בלי רשימה חלקית. **ומה שלא נחסם, במפורש:** התקרה מגבילה סקשנים ולא שורות, ולכן 10MB של שורות ריקות הוא אפס סקשנים ו-6.3 שניות — זהה עם התקרה ובלעדיה. +- **docs: הקבועים והתקרות של שרת ה-MCP במקום אחד, ואוצר המילים של התשובה.** נוסף סעיף :ref:`mcp-limits` עם כל תקרה שנאכפת בקוד — כולל תקרות העימוד, שלא היו מתועדות כלל — ועם הכלל שערך מעל התקרה **נצמד בשקט** ואינו נדחה, כלומר ``per_page=1000`` באאוטליין מקבל 500 ואין שדה שמצהיר על זה. ובנוסף: הרשימה הסגורה של ערכי ``status`` **בתשובת כלי** (תשובת HTTP של השרת אינה תשובת כלי — ``401`` נושא ``error`` בלי ``ok``), ההסבר למה ל-``error`` אין רשימה כזאת, המחלקה היחידה שסורק ה-RST אינו ממודל (בלוק literal מצוטט), ושורה בפתרון תקלות על כך שתהליך ה-MCP אינו מגדיר לוגים — ולכן היעדר לוג אינו סימן שהכלי לא נקרא. **ובנוסף הוסבר מה ``file.encoding`` באמת אומר:** ``utf-8`` ו-``utf-8-sig`` הם זיהוי, ו-``latin-1`` הוא נפילה-לאחור שאינה נכשלת לעולם — נמדד שקובץ עברי ב-``cp1255`` חוזר עם התווית ``latin-1`` ועם תוכן משובש. וש-BOM **מוסר** מ-``content`` בזמן ש-``size`` סופר אותו, כלומר זה החריג היחיד לכלל שהטקסט חוזר בית-בית. - **fix: שעון חוזר של קורא חיצוני הפך לחד-פעמי בשקט אחרי הערכת regex.** ``ConditionOperators.regex`` דורס את ``SIGALRM`` לשנייה אחת כדי לעצור ביטוי שמתפוצץ, ומשחזר אחר כך את השעון של מי שקרא לו. ``signal.getitimer`` מחזיר **זוג** — מה שנשאר עד הירייה הבאה, ומרווח החזרה — והקוד שמר את האיבר הראשון בלבד והשחזור העביר ארגומנט אחד. כלומר שעון **חוזר** שוחזר כחד-פעמי: הירייה הבאה כן מגיעה, וכל אלה שאחריה אינן, בלי שום שגיאה. נמדד: על שעון של 30 שניות עם חזרה כל 5, הקוד שלפני התיקון החזיר מרווח ``0.0`` אחרי הקריאה והמתוקן מחזיר ``5.0``. **בריפו הזה אין מסלול הגעה ידוע** — ``pytest-timeout`` מצמיד שעון חד-פעמי — ולכן זה פגם נכונות בקוד ייצור ולא תקלה שנצפתה. שלושת הטסטים שכיסו את השחזור הדליקו שעון חד-פעמי בלבד, ולכן אף אחד מהם לא היה יכול לראות את זה, והעזר שבטסטים נשא בעצמו את אותו פגם — גם הוא שמר איבר אחד. - **fix: שתי בדיקות שלא היו מסוגלות לתפוס את מה שהן שומרות עליו.** העד של סורק ה-escape-ים שחזר את הסריקה בתוך עצמו במקום לקרוא למימוש, ולכן מי שישבור את המימוש היה מקבל אפס ממצאים ושתי הבדיקות היו ממשיכות לעבור; הוא עובר עכשיו דרך ``_invalid_escapes`` על קובץ שנשתל ב-``tmp_path``. **ושורת הסיכום שאומרת "כל בדיקות הדפדפן דולגו והכיסוי אפס" לא נכתבה דווקא בתצורה שה-CI מריץ.** הסיבה נרשמה במשתנה ברמת המודול שהפיקסצ'ר ממלא, וה-CI מריץ ``pytest -n auto --dist=loadscope`` — כלומר הפיקסצ'ר רץ ב-worker ו-``pytest_terminal_summary`` רץ ב-controller, ושם המשתנה ריק לעולם. נמדד על קובץ דפדפן אחד בלי דפדפן: בלי ``-n`` השורה נכתבת, ועם ``-n 2 --dist=loadscope`` הריצה מסתיימת ב-``4 skipped`` בלי שום אזכור של הסוויטה. **הפתרון הוא מקור אחד במקום שניים:** הסיבה נקראת מדיווחי הדילוג של pytest, שכן עוברים ל-controller בשתי התצורות — ``when == "collect"`` כשהחבילה חסרה, ו-``when == "setup"`` כשרק הדפדפן חסר, וזו הצורה שה-CI מגיע אליה כי ``requirements/base.txt`` מצמיד את ``playwright`` ואין באף workflow שלב שמתקין דפדפן. אומת בהרצה על ``pytest 8.4.2`` עם ``pytest-xdist 3.8.0`` — הגרסאות שה-CI מתקין, ולא אלה שמותקנות בקונטיינר הפיתוח. **ושתי הבדיקות ההתנהגותיות מריצות עכשיו את תת-התהליך בשתי התצורות**, כי שתיהן הריצו אותו בלי ``-n`` ולכן אף אחת מהן לא הייתה מסוגלת לראות את זה. diff --git a/mcp_server/outline_scanners/rst.py b/mcp_server/outline_scanners/rst.py index 1f8fed594..cdf60c49e 100644 --- a/mcp_server/outline_scanners/rst.py +++ b/mcp_server/outline_scanners/rst.py @@ -60,14 +60,21 @@ .. note:: - **מה שזה עולה על קובץ גדול, נמדד ומדווח.** ``parse_document`` בונה את כל - הסקשנים לפני שהתקרה כאן נוגעת בהם — אותו מסלול בדיוק ש-``_ceiling`` - מתעד לגבי ``ast.parse`` בסורק הפייתון. נמדד על קובץ פתולוגי של 10MB - (שני מיליון כותרות בנות תו אחד): **20.8 שניות ו-933MB**, מול 0.10-0.22 - שניות ל-CSS ול-HTML על קלט דומה. הקורפוס האמיתי רחוק מזה בסדרי גודל — - הגדול בו הוא 2,869 שורות ותשעה סימבולים, והכל ב-2.3ms — אבל התקרה של - ``RANGE_READ_MAX_BYTES`` היא 10MB, ולכן החשיפה קיימת בכל ריפו ממורר. - ההכרעה הזאת מטופלת ב-PR נפרד, עם המדידות. + **מה שזה עולה על קובץ גדול, נמדד — והתקרה נוסעת פנימה בגלל המדידה.** + ``parse_document`` בנה את כל הסקשנים לפני שהתקרה כאן נגעה בהם, ולכן + הוא מקבל עכשיו ``max_sections`` ועוצר בזמן הפרסור. נמדד על 10MB של + כותרת בת תו אחד בכל שתי שורות — 2.6 מיליון סקשנים: **18.5 שניות + ו-1,132MB** בלי התקרה, מול **0.30 שניות ו-83MB** איתה. הזיכרון הוא + הנתון שמצדיק את זה: בקשה איטית היא בקשה איטית, אבל ג'יגה-בייט בתהליך + משותף מפיל שירות. + + **ומה שנשאר פתוח, במפורש.** התקרה מגבילה סקשנים ולא שורות, ולכן קלט + שאינו מייצר סקשנים כלל אינו נחסם: 10MB של שורות ריקות הוא **אפס + סקשנים ו-6.3 שניות**, זהה עם התקרה ובלעדיה. שם החסם היחיד היום הוא + ``RANGE_READ_MAX_BYTES``, ומה שהופך את השארית הזאת לנזק משותף הוא + שהכלי סינכרוני ורץ על לולאת האירועים — העובדה מאומתת ורשומה + ב-``_ceiling``, והטיפול בה הוא בחיווט השרת ולא כאן. **השארית מתועדת + באישו #3379**, עם המדידה ועם שני הכיוונים; הודאה בהערה אינה מעקב. """ from __future__ import annotations @@ -114,11 +121,22 @@ def extract(text: str) -> dict[str, Any]: באג שלנו חייב ליפול בקול ולא להתחפש ל"אין אאוטליין" על קובץ תקין. התקרה נוסעת פנימה כחריגה ויוצאת כאן כערך החזרה — הנימוק ב- - ``_ceiling.TooManySymbols`` — וה-``except`` תופס **אותה בלבד**. + ``_ceiling.TooManySymbols`` — וה-``except`` תופס **את שתי החריגות + בשמן, ולא רחב מזה**. + + **ושתיים ולא אחת, כי יש שני מקומות שבהם אותה תקרה נחצית.** + ``TooManySections`` מורמת בתוך ``parse_document`` כשהכותרות לבדן + ממלאות אותה, ו-``TooManySymbols`` מורמת מהמכל כאן כשהכותרות והתוויות + **יחד** חוצות אותה. אותו מספר, אותה תשובה, ושני שערים כי העבודה + מתחלקת בין שני מודולים. + + **והפרסור יושב בתוך ה-``try`` ולא לפניו** — אחרת החריגה שנוסעת + פנימה עולה בלי מי שיתפוס אותה, והתשובה על קובץ פתולוגי הופכת + מ-``no_outline`` לשגיאת כלי. """ rows: list[dict[str, Any]] = _ceiling.Capped() - document = rst_parser.parse_document(text) try: + document = rst_parser.parse_document(text, max_sections=_ceiling.MAX_SYMBOLS) for section in document.sections: rows.append( { @@ -163,6 +181,6 @@ def extract(text: str) -> dict[str, Any]: "end": number, } ) - except _ceiling.TooManySymbols: + except (rst_parser.TooManySections, _ceiling.TooManySymbols): return _ceiling.too_many_symbols() return {"symbols": rows} diff --git a/services/rst_parser.py b/services/rst_parser.py index 23587f800..1eb163e80 100644 --- a/services/rst_parser.py +++ b/services/rst_parser.py @@ -207,6 +207,23 @@ def _adornment_fits(title: str, adornment: str) -> bool: return True +class TooManySections(Exception): + """הקלט מייצר יותר סקשנים מהתקרה שהמתקשר העביר ב-``max_sections``. + + **חריגה, ולא ערך החזרה ולא ``Document`` חלקי עם דגל "נקטע".** המתקשר + היחיד שמעביר תקרה הוא סורק האאוטליין, ושם הכלל כבר נקבע ומנומק + ב-``mcp_server/outline_scanners/_ceiling.py``: מפה חלקית שמתחזה + למלאה היא בדיוק הכשל שהתקרה קיימת כדי למנוע. דגל דורש בדיקה בכל אתר + קריאה, ובדיקה אחת שנשכחת מחזירה תוכן עניינים שחסרות בו כותרות בלי + שאיש יידע — ואילו חריגה נכשלת בקול. + + **ומוגדרת כאן ולא שם**, כי הכיוון הוא חד-סטרי: הסורק מייבא את המודול + הזה, ו-``services`` אינו מייבא מ-``mcp_server``. ההורשה היא + מ-``Exception`` ישירות ולא מחריגת ספריית תקן, כדי שתפיסה צרה של + החריגה הזאת לא תוכל להתנגש בשגיאה אמיתית. + """ + + def _skip_paragraph_block(lines: List[str], i: int, n: int) -> int: """מדלג על בלוק הפסקה בדיוק כמו ``Text.text``, ומחזיר את השורה שאחריו. @@ -231,7 +248,7 @@ def _skip_paragraph_block(lines: List[str], i: int, n: int) -> int: return i -def parse_document(text: str) -> Document: +def parse_document(text: str, *, max_sections: int | None = None) -> Document: """מפרסר טקסט RST לעץ סקשנים. עמיד ל-literal/code blocks ולדירקטיבות מוזחות. **סיומות השורה מנורמלות כאן, בגבול הקלט, ו-``split("\\n")`` נשאר יחידת @@ -251,6 +268,24 @@ def parse_document(text: str) -> Document: **ומה שהנרמול אינו מכסה, ולכן אינו לבד:** תו whitespace שאין לו נרמול — ``\\v``, ``\\f``, NBSP, ``U+2028`` — אינו נעלם כאן. ההגנה עליו יושבת ב-``_is_title_text``, שם ההגדרה של "שורה ריקה" חייבת להיות אחת. + + **``max_sections`` — תקרה על מספר הסקשנים, וברירת המחדל היא ללא תקרה.** + ``None`` פירושו התנהגות זהה בית-בית לזו שקדמה לפרמטר, ולכן + ``docs_get_section`` — הצרכן השני של המודול הזה — אינו מושפע. כשהתקרה + מועברת ונחצית מורמת ``TooManySections`` **בזמן הפרסור**, ולא אחריו. + + **למה בזמן הפרסור, במספרים שנמדדו על קלט של 10MB:** כותרת בת תו אחד + בכל שתי שורות נותנת 2.6 מיליון סקשנים, ובלי התקרה זה **18.5 שניות + ו-1,132MB**; עם תקרה של 50,000 זה **0.30 שניות ו-83MB**. סינון של + הפלט אחרי שהעץ נבנה אינו חוסם את העבודה, וזו אותה מדידה בדיוק + שמנומקת ב-``_ceiling`` לגבי בדיקת אורך במנתב. + + **ומה שהתקרה הזאת אינה חוסמת, כדי שלא ייקרא כאן יותר ממה שכתוב:** + היא מגבילה סקשנים, לא שורות. 10MB של שורות ריקות הוא **אפס סקשנים + ו-6.3 שניות**, ואותו זמן בדיוק עם התקרה ובלעדיה — כי אין שם מה + לחסום. השארית הזאת נקבעת על ידי מספר השורות, והחסם היחיד עליה היום + הוא תקרת הבתים של הקריאה. **מתועדת באישו #3379** — שם גם הכיוון + שמשלים אותה, ומספר השורות ממילא מחושב כאן בראש הפונקציה. """ lines = (text or "").replace("\r\n", "\n").split("\n") n = len(lines) @@ -281,6 +316,30 @@ def level_of(style: object) -> int: except ValueError: return len(order) + 1 + def add_section(section: Section) -> None: + """מוסיף סקשן, ועוצר את הפרסור כשהתקרה נחצית. + + **הגדרה אחת לשלושת אתרי ההוספה, ולא שלוש בדיקות זהות.** שלוש + בדיקות זהות הן בדיוק מה שאפשר לשכוח באתר הרביעי שייכתב מחר, בלי + שאף טסט יראה — וזה כבר קרה במודול הזה: כלל בליעת הפסקה נכתב + פעמיים, אתר אחד תוקן והשני נשאר מאחור. אותו נימוק בדיוק שכתוב + ב-``_ceiling.Capped`` על שלושה-עשר אתרי ההוספה ב-``html.py``. + + **והגבול זהה לזה של** ``Capped.append``: הסירוב הוא כשהרשימה + **כבר** מלאה, ולכן קובץ עם בדיוק ``max_sections`` סקשנים עובר + במלואו, והסקשן שמעליו הוא זה שעוצר. כך התשובה על הגבול אינה זזה + מזו שהוחזרה לפני שהתקרה נסעה פנימה. + + **והסקשן שחוצה את התקרה כן נבנה, פעם אחת.** ארגומנט מחושב לפני + הקריאה, ולכן ``Section`` שיידחה נוצר ונזרק — אובייקט בודד, ולא + המשך של הפרסור, וזה מה שהטסט על מספר הבנייות מקבע. לבנות את + השדות כאן במקום להעביר אובייקט היה חוסך אותו, במחיר שש מסירות + ארגומנטים בכל אתר — ולא זה מה שנמדד כיקר. + """ + if max_sections is not None and len(sections) >= max_sections: + raise TooManySections + sections.append(section) + i = 0 while i < n: raw = lines[i] @@ -393,7 +452,7 @@ def level_of(style: object) -> int: if lvl > len(order): order.append(style) current_level = lvl - sections.append(Section( + add_section(Section( title=title_line.strip(), level=lvl, title_line=i + 2, heading_line=i + 1, end_line=n, adornment=over, over=True, @@ -425,7 +484,7 @@ def level_of(style: object) -> int: if lvl > len(order): order.append(demoted) current_level = lvl - sections.append(Section( + add_section(Section( title=raw.strip(), level=lvl, title_line=i + 1, heading_line=i + 1, end_line=n, adornment=demoted, over=False, @@ -448,7 +507,7 @@ def level_of(style: object) -> int: if lvl > len(order): order.append(under) current_level = lvl - sections.append(Section( + add_section(Section( title=raw.strip(), level=lvl, title_line=i + 1, heading_line=i + 1, end_line=n, adornment=under, over=False, diff --git a/tests/test_mcp_docs_handlers.py b/tests/test_mcp_docs_handlers.py index 66495878c..a0054e282 100644 --- a/tests/test_mcp_docs_handlers.py +++ b/tests/test_mcp_docs_handlers.py @@ -1,10 +1,12 @@ """טסטים ל-handler של docs_get_section — עם fake backend שקורא RST אמיתי מ-docs/.""" +import inspect from pathlib import Path import pytest from mcp_server import docs_handlers +from services import rst_parser _ROOT = Path(__file__).resolve().parents[1] @@ -220,3 +222,42 @@ def test_an_overlined_heading_is_one_level_deeper_than_the_same_character_underl assert [(t["title"], t["level"]) for t in out["toc"]] == [ ("כותרת א", 1), ("כותרת ב", 2)] assert out["toc"][1]["breadcrumb"] == ["כותרת א", "כותרת ב"] + + +def test_the_docs_reader_parses_without_any_ceiling(monkeypatch): + """הצרכן בייצור אינו מוגבל בתקרה, ונבדק על **התקרה האפקטיבית**. + + ``parse_document`` קיבל ``max_sections`` בשביל סורק האאוטליין, ושני + דברים חייבים להישאר נכונים כדי שהכלי הזה לא ייחסם: שהוא **אינו + מעביר** תקרה, ושברירת המחדל **אינה** תקרה. שניהם נבדקים כאן, כי כל + אחד מהם לבדו מספיק כדי לחסום קובץ תיעוד גדול. + + **וזה נבדק כך במקום להציף את התקרה, בכוונה.** הגרסה הראשונה של + הבדיקה בנתה ``MAX_SYMBOLS + 1`` סקשנים אמיתיים — 819KB, 0.40 שניות + ו-71MB בכל ריצה — כלומר **המחיר שלה היה צמוד לקבוע שהיא מגנה עליו**. + נמדד: בהעלאת התקרה פי עשר, מה שההערה ליד הקבוע מזמינה במפורש, אותה + בדיקה הייתה בונה קלט של 8.9MB ו-500,001 סקשנים — 6.64 שניות ו-482MB + בכל ריצת CI. בדיקה שמתדרדרת בדיוק כשהמערכת גדלה אינה הצורה הנכונה. + + **וליטרל קבוע לא היה מחליף אותה**, כי הוא מפסיק להוכיח משהו ברגע + שהתקרה עולה מעליו. שתי הבדיקות כאן אינן תלויות בגודל התקרה בכלל. + """ + passed: dict[str, object] = {} + real = rst_parser.parse_document + + def spy(text, **kwargs): + passed.update(kwargs) + return real(text, **kwargs) + + monkeypatch.setattr(docs_handlers.rst_parser, "parse_document", spy) + + out = docs_handlers.docs_get_section(_TextBackend("א\n=\n\nב\n=\n\n"), path="x") + + assert out["ok"] and out["mode"] == "toc" + assert out["section_count"] == 2 + assert passed.get("max_sections") is None, ( + f"הכלי העביר תקרה לפרסור: {passed}" + ) + assert inspect.signature(real).parameters["max_sections"].default is None, ( + "ברירת המחדל של max_sections אינה None, ולכן הכלי חסום גם בלי להעביר כלום" + ) diff --git a/tests/test_mcp_outline.py b/tests/test_mcp_outline.py index b13960447..0561e71f9 100644 --- a/tests/test_mcp_outline.py +++ b/tests/test_mcp_outline.py @@ -2776,6 +2776,49 @@ def test_the_ceiling_cannot_fire_without_the_container(monkeypatch): assert result["total"] > _ceiling.MAX_SYMBOLS +def test_the_rst_ceiling_stops_the_parse_and_not_only_the_answer(monkeypatch): + """הטסט היחיד כאן שמקבע **עבודה** ולא תשובה. + + ``rows-rst`` שלמעלה עובר גם על מימוש שמפרסר את הקובץ כולו ורק אחר כך + מסרב — התשובה זהה, והעלות אינה. נמדד על 10MB של כותרת בת תו אחד בכל + שתי שורות: 18.5 שניות ו-1,132MB כשהפרסור מסתיים, מול 0.30 שניות + ו-83MB כשהוא נעצר. כלומר הדבר שצריך הגנה הוא נקודת העצירה, ולא + המילון שחוזר. + + **והמונה הוא בנייה של ``Section`` ולא זמן ולא זיכרון**, כי שני אלה + תלויים במכונה ובעומס והיו הופכים את הטסט להפכפך. ספירת אובייקטים היא + דטרמיניסטית: על 200 כותרות ותקרה של 20, מימוש שנעצר בונה 20 ומימוש + שמסנן פלט בונה 200. + + הייבוא מקומי בכוונה: הקובץ הזה נמנע מייבוא ``services.rst_parser`` + ברמת המודול כדי שהאורקלים שבו לא ייגזרו מהמימוש שהם בודקים, ומונה + אינו אורקל. + """ + from services import rst_parser + + monkeypatch.setattr(_ceiling, "MAX_SYMBOLS", 20) + built = [] + real = rst_parser.Section + + def counting(**fields): + built.append(1) + return real(**fields) + + monkeypatch.setattr(rst_parser, "Section", counting) + + result = extract_outline("a\n=\n\n" * 200, "a.rst") + + assert result == {"status": "no_outline", "reason": "too_many_symbols", "max": 20} + # **חסם ולא מספר מדויק, וזה מכוון.** 20 נכנסו, ועוד אחד נבנה ונדחה — + # הארגומנט מחושב לפני הקריאה ל-``add_section`` — ולכן היום זה 21. + # אבל הריפקטור שה-docstring של ``add_section`` מתאר כחלופה (בניית + # השדות בתוך הפונקציה במקום העברת אובייקט) ייתן 20, ואז השוואה + # מדויקת הייתה נופלת עם ההודעה "הפרסור לא נעצר" — כלומר מאשימה + # בדיוק בהיפוך ממה שקרה. ההבטחה שצריך לשמור היא חסימת העבודה: + # מימוש שמסנן פלט בונה 200. + assert len(built) <= 21, f"נבנו {len(built)} סקשנים — הפרסור לא נעצר על התקרה" + + def test_the_tool_reports_the_ceiling_through_the_real_path(monkeypatch): """דרך ``repo_handlers.get_repo_file`` ולא רק דרך ``extract_outline``. diff --git a/tests/test_rst_parser.py b/tests/test_rst_parser.py index e1a0cbffd..f18a7cabd 100644 --- a/tests/test_rst_parser.py +++ b/tests/test_rst_parser.py @@ -1,9 +1,12 @@ """טסטים לפארסר ה-RST — על קבצי RST אמיתיים מהריפו + מקרי קצה סינתטיים ממוקדים.""" +import ast import json import re from pathlib import Path +import pytest + from services import rst_parser _ENV_RST = Path(__file__).resolve().parents[1] / "docs" / "environment-variables.rst" @@ -1111,3 +1114,141 @@ def test_a_simple_table_with_no_closing_border_swallows_the_rest(): f"ראש\n====\n\n{_TABLE}\na b\n\nכותרת\n------\n\nגוף\n" ) assert [(s.title, s.level) for s in doc.sections] == [("ראש", 1)] + + +# ---- התקרה על מספר הסקשנים ---- +# +# ``max_sections`` קיים כדי שהפרסור **ייעצר**, ולא כדי שהתשובה תיחתך. +# הנימוק והמדידות יושבים ב-docstring של ``parse_document``, והמספר עצמו +# אינו כאן: הוא של סורק האאוטליין, והפארסר אינו מכיר אותו. + +_CEILING_SHAPES = { + "כותרת עם קו תחתון": "a\n=\n\n", + "כותרת עם קו מעליה ומתחתיה": "===\naaa\n===\n\n", + "קו עליון קצר שהודח לכותרת": "~\n=\n\n", +} + + +@pytest.mark.parametrize("shape", sorted(_CEILING_SHAPES)) +def test_each_of_the_three_heading_shapes_honours_the_section_ceiling(shape): + """שלושת אתרי ההוספה, כל אחד בנפרד — וזה הלקח מכלל שנכתב פעמיים. + + ל-``parse_document`` שלוש צורות כותרת ושלושה אתרים שמוסיפים סקשן, + ו-``add_section`` הוא ההגדרה היחידה שכולם עוברים דרכה. טסט על צורה + אחת היה עובר גם על מימוש שבו אתר אחד נשאר בלי הבדיקה — וזה בדיוק מה + שקרה במודול הזה עם כלל בליעת הפסקה, שנכתב בשני אתרים, ואחד מהם תוקן + והשני נשאר מאחור עד שהסבב הבא תפס אותו. + + **והבקרה בשורה הראשונה אינה קוסמטית:** בלעדיה, צורה שהפסיקה להיות + כותרת בכלל הייתה מספקת את הטסט בשקט — אין סקשנים, אין חריגה, הכול + ירוק. ``~`` בודד, למשל, נראה כמו קו עליון קצר אבל ``-`` בודד באותו + מקום הוא פריט רשימה ואינו כותרת. + """ + unit = _CEILING_SHAPES[shape] + + assert len(rst_parser.parse_document(unit).sections) == 1 + + with pytest.raises(rst_parser.TooManySections): + rst_parser.parse_document(unit * 2, max_sections=1) + + +def test_the_section_ceiling_refuses_above_the_line_and_not_on_it(): + """הגבול בשני הכיוונים, כמו ב-``Capped.append``. + + טסט שבודק רק הצפה עובר גם על תקרה שחוסמת קלט תקין, ולכן שני הכיוונים + ביחד הם מה שמקבע את הגבול ותופס off-by-one בין ``>=`` ל-``>``. הגבול + חייב להישאר זהה לזה שהיה לפני שהתקרה נסעה לתוך הפרסור, אחרת התשובה + על קובץ שיושב בדיוק על הגבול משתנה. + """ + three = "a\n=\n\nb\n=\n\nc\n=\n\n" + + assert len(rst_parser.parse_document(three, max_sections=3).sections) == 3 + + with pytest.raises(rst_parser.TooManySections): + rst_parser.parse_document(three + "d\n=\n\n", max_sections=3) + + +def test_no_heading_site_reaches_the_section_list_around_the_funnel(): + """כל אתר כותרת עובר ב-``add_section``, ואין דרך עקיפה — נבדק על המקור. + + **מה זה שומר.** התקרה של ``max_sections`` חיה כולה ב-``add_section``, + ו-``sections`` היא ``list`` רגילה שאינה יכולה לסרב בעצמה. כלומר אתר + כותרת רביעי שייכתב מחר עם ``sections.append(...)`` ישיר פותח מחדש + את החור, ו**כל** הבדיקות הקיימות ימשיכו לעבור: שלוש הפרמטריות + מכסות את שלוש הצורות שקיימות היום, ולא את זו שתיכתב. + + **הבדיקה היא על המקור ולא על התנהגות, וזה אותו נימוק בדיוק** שכתוב + ליד השומר המקביל ב-``tests/test_mcp_outline.py``, זה שחוסם חמש דרכי + עקיפה של המכל בסורקים: התנהגות אפשר לבדוק רק במסלול שכבר קיים, ומה + שצריך לתפוס הוא המסלול שעוד לא נכתב. השומר ההוא מגלוב + ``mcp_server/outline_scanners/*.py`` בלבד, ולכן אינו רואה את המודול + הזה — וזה הפער שהבדיקה הזאת סוגרת. + + **ו-``ast`` ולא רג'קס**, מאותה סיבה שנומקה שם: רג'קס על ``+=`` תופס + גם ``i += 1``, וגם טקסט בתוך docstring שמזכיר את הצורות האסורות. + + נמדד על הקוד של היום: אפס מסלולים. + """ + tree = ast.parse(Path(rst_parser.__file__).read_text(encoding="utf-8")) + + parse_document = next( + node for node in ast.walk(tree) + if isinstance(node, ast.FunctionDef) and node.name == "parse_document" + ) + funnel = next( + node for node in ast.walk(parse_document) + if isinstance(node, ast.FunctionDef) and node.name == "add_section" + ) + inside_funnel = {id(node) for node in ast.walk(funnel)} + + offenders = [] + for node in ast.walk(parse_document): + if id(node) in inside_funnel: + continue + where = f"שורה {getattr(node, 'lineno', 0)}" + + # ``sections.append(...)`` / ``.extend`` / ``.insert`` + if ( + isinstance(node, ast.Call) + and isinstance(node.func, ast.Attribute) + and isinstance(node.func.value, ast.Name) + and node.func.value.id == "sections" + and node.func.attr in {"append", "extend", "insert"} + ): + offenders.append(f"{where}: sections.{node.func.attr}") + + # ``sections += [...]`` + if ( + isinstance(node, ast.AugAssign) + and isinstance(node.target, ast.Name) + and node.target.id == "sections" + ): + offenders.append(f"{where}: += על sections") + + # ``sections[len(sections):] = [...]`` + if isinstance(node, ast.Assign): + for target in node.targets: + if ( + isinstance(target, ast.Subscript) + and isinstance(target.value, ast.Name) + and target.value.id == "sections" + ): + offenders.append(f"{where}: השמה ל-slice של sections") + + # ``list.append(sections, x)`` — קריאה שעוקפת את המשפך + if ( + isinstance(node, ast.Call) + and isinstance(node.func, ast.Attribute) + and isinstance(node.func.value, ast.Name) + and node.func.value.id == "list" + and node.func.attr in {"append", "extend", "insert"} + and node.args + and isinstance(node.args[0], ast.Name) + and node.args[0].id == "sections" + ): + offenders.append(f"{where}: list.{node.func.attr} על sections") + + assert offenders == [], ( + "אתר שמוסיף סקשן בלי לעבור ב-add_section, כלומר בלי התקרה. " + f"אם הוא באמת נחוץ — הוא צריך את הבדיקה לידו במפורש: {offenders}" + )