diff --git a/docs/mcp-server.rst b/docs/mcp-server.rst index 83f59fc72..616881658 100644 --- a/docs/mcp-server.rst +++ b/docs/mcp-server.rst @@ -535,6 +535,112 @@ Claude Desktop זה סימן **לנסות שוב אחרי המתנה קצרה** — לא להסיק שהקובץ או הריפו לא קיימים. +.. _mcp-outline: + +מפת סימבולים (``outline``) +--------------------------- + +``codekeeper_get_repo_file`` עם ``outline=true`` מחזיר את **המפה** של הקובץ +במקום את התוכן: כל פונקציה וכל מחלקה, עם שורת התחלה וסיום. משם ממשיכים +ל-``lines=[start, end]`` וקוראים בדיוק את מה שצריך. + +.. code-block:: json + + {"ok": true, + "status": "outline", + "file": {"path": "webapp/app.py", "ref": "refs/heads/main", + "resolved_commit": "aca7599", "size": 805594, "lines": 20035, + "encoding": "utf-8"}, + "symbols": [{"name": "api_db_collections", "start": 5421, "end": 5481}], + "total": 530, "page": 1, "per_page": 100} + +זו התשובה במלואה, לא קטע ממנה: ``ok`` ו-``file`` תמיד שם, ואין שדה +``truncated``. + +.. note:: + + **אין כאן ``truncated``, ובכוונה.** ב-``codekeeper_list_repo_tree`` המילה + הזו פירושה "תקציב הפלט חתך רשומות **בתוך** העמוד" — לא "יש עמוד נוסף". + באאוטליין אין חיתוך כזה: עמוד שאינו נכנס בתקציב **נדחה במלואו** ולא + מוחזר חלקית, כי חיתוך בתוך עמוד יחד עם עימוד אריתמטי מאבד סימבולים — + העמוד נעצר באמצע והבא מתחיל אחרי ``per_page`` המלא. + + הגודל נמדד על הסריאליזציה האמיתית ב-UTF-8, ולא בספירת תווים. גרסה + קודמת חסמה את אורך השם ב-200 **תווים** והסיקה מכך שהעמוד נכנס — וזה + שגוי פעמיים: תו CJK הוא שלושה בתים, כך שעמוד מקסימלי הגיע ל-323,000 + בתים מול תקציב של 256,000; וקיצוץ השם הרס את הזהות, כך ש-``symbol=`` + עם השם המלא כבר לא מצא את הסימבול. + + שדה שתמיד ``false`` ומסמן משמעות אחרת מזו של הכלי השכן גרוע מהיעדרו. + **מה שאומר אם יש עוד עמודים הוא ``total`` מול ``page`` ו-``per_page``.** + +**מגבלת הגודל.** אאוטליין נשפט מול אותה תקרה של קריאת טווח — **10MB**, ולא +500KB — כי הוא קורא את הקובץ ומחזיר פלט זעיר. ראו :ref:`mcp-line-range`. + +**``lines`` ו-``outline`` אינם מצטברים.** קריאה שמעבירה את שניהם מוחזרת +כ-``{"ok": false, "error": "outline_and_lines"}``, ולא מתעלמת בשקט מאחד +מהם. בקשו מפה, ואז טווח. + +**השם מלא, עם נקודות.** ``ClassName.method``, ``outer.inner``, +``_register_repo_tools.get_repo_file``. הכלל אחד: מרחב שמות בפייתון הוא +פונקציה או מחלקה, ולכן התחילית גדלה רק בהם. ``if``, ``try``, ``with`` +ו-``for`` **אינם** מרחב שמות — פונקציה שהוגדרה בתוך ``except ImportError`` +מופיעה ברמה שלה, בלי תחילית. + +.. note:: + + הכלל הזה הוא כל הפיצ'ר. בלעדיו ``mcp_server/server.py`` החזיר תשעה + סימבולים במקום 38, ו-``build_mcp`` נראתה כבלוק אטום של 513 שורות שכל + כלי ה-MCP חבויים בתוכו. גרסת ביניים שירדה רק לתוך גופי פונקציות עדיין + החמיצה 62 סימבולים ב-``webapp/app.py``, כי מחלקות ה-fallback שם יושבות + בתוך ``except ImportError``. + +**שורת ההתחלה היא של המעטר.** ב-``webapp/app.py`` 203 מתוך 408 הפונקציות +ברמה העליונה מעוטרות; טווח שהיה מתחיל ב-``def`` היה מחמיץ את +``@app.route(...)``, כלומר בדיוק את השורה שמזהה את הנתיב. + +**``symbol=`` מסנן על השם המלא**, ללא תלות ברישיות. לכן +``symbol="build_mcp"`` מחזיר גם את הפונקציה וגם את כל מה שמוגדר בתוכה — +"תן לי הכול תחת המרחב הזה". ``total`` סופר את ההתאמות **אחרי** הסינון, כי +עליו נשען העימוד. על ``webapp/app.py``, ``symbol="api_db"`` מחזיר חמישה +סימבולים במקום 530. + +**שמות אינם ייחודיים.** ``if``/``else`` אינם מרחב שמות, ולכן שתי הגדרות +בשני הענפים חולקות שם מלא ונבדלות רק בשורה. שתיהן מוחזרות — בריפו הזה יש +היום שישה מקרים כאלה. הרשימה ממוינת לפי שורת התחלה, ושובר-השוויון הוא השם, +כדי שגבול העמוד יהיה יציב בין קריאות. + +**השם מוחזר במלואו ולעולם אינו מקוצץ** — מזהה שנחתך אינו מזהה, ו-``symbol=`` +עם השם המלא חייב למצוא אותו. + +**הסיומות הנתמכות** הן ``.py`` ו-``.pyi``, ללא תלות ברישיות. ``symbol=`` +משווה ב-``casefold``, כך ש-``STRASSE`` מוצא את ``straße``. + +.. list-table:: + :header-rows: 1 + :widths: 34 66 + + * - מצב + - התשובה + * - קובץ שאינו פייתון + - ``{"ok": true, "file": {...}, "status": "no_outline", + "reason": "unsupported_language"}`` + * - תחביר שבור, פייתון 2, קידוד פגום + - ``{"ok": true, "file": {...}, "status": "no_outline", + "reason": "parse_error", "error_type": "UnicodeEncodeError", "line": 12}``. + ``error_type`` הוא שם מחלקת החריגה — בלעדיו אי אפשר לאבחן למה קובץ + שנראה תקין לא נותן מפה + * - עמוד גדול מתקציב הפלט + - ``{"ok": false, "error": "page_too_large", "bytes": 292783, + "max": 256000, "per_page": 500}``. בקשו ``per_page`` קטן יותר; אין + כאן חיתוך שקט + +.. important:: + + ``no_outline`` הוא ``status`` ולא ``error``, בדיוק כמו ``binary``: + הקריאה **הצליחה**, פשוט אין תוכן מהסוג שביקשו. מי שבודק רק אם נזרקה + חריגה יראה קובץ שבור כקובץ בלי סימבולים — לכן חובה לבדוק את ``status``. + .. _mcp-line-range: קריאת טווח שורות diff --git a/mcp_server/outline.py b/mcp_server/outline.py new file mode 100644 index 000000000..103b4fe99 --- /dev/null +++ b/mcp_server/outline.py @@ -0,0 +1,144 @@ +"""חילוץ מפת סימבולים מקובץ פייתון, לניווט בקבצים גדולים. + +הכלי מחזיר שם, שורת התחלה ושורת סיום לכל פונקציה ומחלקה, כדי שהקורא +יוכל להמשיך ל-``lines=[start, end]`` במקום לנחש חלון. ``webapp/app.py`` +הוא 20,035 שורות; בלי מפה, סוכן שמחפש בו פונקציה קורא ומנחש. + +**ערוץ הכשל הוא ערך ההחזרה, לא חריגה.** הפונקציה מחזירה תמיד מילון עם +``status``: ``"ok"`` או ``"no_outline"``. קורא שבודק רק אם נזרקה חריגה +יקבל "אין אאוטליין" בשקט ויחשוב שהקובץ ריק — זהו דפוס K11 ב- +``amir-bug-patterns``, ולכן הבחירה מוצהרת כאן ולא משתמעת. +""" + +from __future__ import annotations + +import ast +from typing import Any + +#: מרחב שמות בפייתון הוא פונקציה או מחלקה. ``if``/``try``/``with``/``for`` +#: **אינם** — הם משנים זרימה, לא שיוך. לכן המעבר חוצה אותם בלי להוסיף +#: תחילית, ופונקציה שהוגדרה בתוך ``except ImportError`` נשארת ברמה שלה. +_FUNCTION_NODES = (ast.FunctionDef, ast.AsyncFunctionDef) +_SCOPE_NODES = _FUNCTION_NODES + (ast.ClassDef,) + +#: ``.pyi`` הוא פייתון תקין ש-``ast.parse`` מנתחת, והוא נפוץ הרבה יותר +#: מסיומת באותיות גדולות. הבדיקה מנורמלת, ולא השוואת מחרוזת אחת. +_PYTHON_SUFFIXES = frozenset({".py", ".pyi"}) + + +def extract_outline(text: str, path: str, symbol: str | None = None) -> dict[str, Any]: + """מפת הסימבולים של ``text``, או ``no_outline`` עם הסיבה. + + ``symbol`` מסנן לפי תת-מחרוזת ב**שם המלא**, ללא תלות ברישיות. לכן + ``symbol="build_mcp"`` מחזיר גם את הפונקציה וגם את כל מה שמוגדר + בתוכה — "תן לי הכול תחת המרחב הזה". ``total`` סופר את ההתאמות אחרי + הסינון, כי עליו נשען העימוד. + """ + if not any(path.lower().endswith(suffix) for suffix in _PYTHON_SUFFIXES): + return {"status": "no_outline", "reason": "unsupported_language"} + + # ההרחבה הגורפת עוטפת **רק** את הפרסינג, ובכוונה. הקלט הוא קובץ + # שמישהו אחר כתב, וסוג החריגה משתנה בין גרסאות פייתון: בייט אפס הוא + # ``SyntaxError`` ב-3.11 ו-``ValueError`` במקומות אחרים, ו-surrogate + # לא חוקי הוא ``UnicodeEncodeError``. מניית סוגים כאן היא בדיוק + # הגישה השברירית שהכלל נגד הרחבת except בא למנוע — הרחבה מנומקת + # עדיפה על רשימה שתתיישן בשקט. + # + # מעבר העץ שמתחת רץ **מחוץ** ל-``try``: באג שלנו חייב ליפול בקול ולא + # להתחפש ל"אין אאוטליין" על קובץ תקין לגמרי. + try: + tree = ast.parse(text) + except Exception as error: + return { + "status": "no_outline", + "reason": "parse_error", + "error_type": type(error).__name__, + "line": getattr(error, "lineno", None), + } + + rows = _collect(tree, text.split("\n")) + + # ממוין לפי שורת התחלה, ושובר-שוויון לפי שם. אין כאן מקרה של מעטר + # משותף — מעטר שייך לסימבול אחד — אבל שובר-שוויון קבוע הוא מה שהופך + # את גבול העמוד ליציב בין קריאה לקריאה. + rows.sort(key=lambda row: (row["start"], row["name"])) + + if symbol: + # ``casefold`` ולא ``lower``: זה הפרימיטיב להשוואה חסרת-רישיות. + # ``lower`` מפספסת מיפויים של יותר מתו אחד, למשל ``straße`` מול + # ``STRASSE``. + needle = symbol.casefold() + rows = [row for row in rows if needle in row["name"].casefold()] + + return {"status": "ok", "symbols": rows, "total": len(rows)} + + +def _start_line( + node: ast.FunctionDef | ast.AsyncFunctionDef | ast.ClassDef, lines: list[str] +) -> int: + """שורת ההתחלה: המעטר הראשון, ולא ה-``def``. + + ב-``webapp/app.py`` 203 מתוך 408 הפונקציות ברמה העליונה מעוטרות, וטווח + שהיה מתחיל ב-``def`` היה מחמיץ את ``@app.route(...)``. + + ``decorator_list[i].lineno`` הוא שורת ה**ביטוי**, לא שורת ה-``@``. + בכתיב הרגיל הם זהים, אבל ב-``@(``ואז ירידת שורה הביטוי מתחיל שורה + מאוחר יותר — נמדד. לכן סריקה אחורה עד השורה שמתחילה ב-``@``. מעטר + קודם שייתפס בדרך שייך לאותו סימבול ממילא, ולכן הרחבה כזו נכונה. + """ + # ``decorator_list`` קיים בוודאות בשלושת הטיפוסים שבחתימה, ולכן גישה + # ישירה ולא ``getattr`` עם ברירת מחדל — הגנה שהטיפוס כבר נותן היא + # רעש שמסתיר את מה שבאמת יכול להיכשל. + decorators = node.decorator_list + start = min([node.lineno] + [d.lineno for d in decorators]) + if not decorators: + return start + # סריקה אחורה עד השורה שפותחת ב-``@``. בין ה-``@`` לבין הביטוי יכולות + # לשבת שורות ריקות והערות, ולכן הן מדולגות — אבל **רק** אם בסוף נוחתים + # על ``@`` אמיתי. אם לא, נשארים במקום: עדיף טווח מדויק-חלקית מטווח + # שבלע שורות של סימבול קודם. + index = start - 1 # 0-based + scan = index + while scan > 0: + previous = lines[scan - 1].strip() + if previous.startswith("@"): + scan -= 1 + index = scan + continue + if previous == "" or previous.startswith("#"): + scan -= 1 + continue + break + return index + 1 + + +def _collect(tree: ast.AST, lines: list[str]) -> list[dict[str, Any]]: + """מעבר על העץ עם מחסנית מפורשת, ולא ברקורסיה. + + קינון עמוק לא יכול לייצר ``RecursionError`` בצד שלנו — וזה מה שמאפשר + ל-``except`` שלמעלה להישאר צר סביב הפרסינג בלבד, בלי פיתוי להרחיב + אותו כדי לבלוע נפילה של המעבר. + """ + rows: list[dict[str, Any]] = [] + stack: list[tuple[ast.AST, str]] = [(tree, "")] + while stack: + node, prefix = stack.pop() + for child in ast.iter_child_nodes(node): + if isinstance(child, _SCOPE_NODES): + name = f"{prefix}{child.name}" + rows.append( + { + "name": name, + # שורת ההתחלה היא של המעטר הראשון ולא של ה-``def``. + # ב-``webapp/app.py`` 203 מתוך 408 הפונקציות ברמה + # העליונה מעוטרות, ובלי זה טווח שנקרא לפי האאוטליין + # היה מתחיל **אחרי** ``@app.route(...)`` — כלומר + # מחמיץ את השורה שמזהה את הנתיב. + "start": _start_line(child, lines), + "end": child.end_lineno or child.lineno, + } + ) + stack.append((child, f"{name}.")) + else: + stack.append((child, prefix)) + return rows diff --git a/mcp_server/repo_backend.py b/mcp_server/repo_backend.py index 9a447a926..a6102f73f 100644 --- a/mcp_server/repo_backend.py +++ b/mcp_server/repo_backend.py @@ -16,12 +16,19 @@ from __future__ import annotations +import json import logging from typing import Any from .backend import _json_safe -from .repo_handlers import TREE_PER_PAGE_MAX +from .repo_handlers import ( + OUTLINE_PER_PAGE_DEFAULT, + OUTLINE_PER_PAGE_MAX, + OUTPUT_BYTE_BUDGET, + TREE_PER_PAGE_MAX, +) from .handlers import apply_line_range, normalize_line_range +from .outline import extract_outline from .repo_policy import is_denied #: תקרת גודל נפרדת לקריאת טווח שורות. @@ -66,6 +73,69 @@ def _safe_int(value: Any, default: int) -> int: } +def _outline_response( + file_meta: dict[str, Any], + content: str, + path: str, + symbol: str | None, + page: Any, + per_page: Any, +) -> dict[str, Any]: + """עוטף את ``extract_outline`` בעימוד ובמעטפת התשובה של הכלי. + + **``status`` ולא ``error``.** קובץ שאין לו אאוטליין הוא בדיוק המקרה של + ``binary``: הקריאה הצליחה, פשוט אין תוכן מהסוג שביקשו. ``error`` שמור + ל-``ok: false``, לפי אוצר המילים שכבר קיים בכלים כאן. + """ + result = extract_outline(content, path, symbol=symbol) + if result.get("status") != "ok": + # ערוץ הכשל של ``extract_outline`` הוא ערך ההחזרה ולא חריגה, ולכן + # נבדק כאן במפורש. בדיקה של "לא נזרקה חריגה" בלבד הייתה מציגה + # קובץ שבור כקובץ בלי סימבולים. + return {"ok": True, "file": file_meta, **result} + + # אותה הגנה שהשכן ``list_tree`` עושה בכוונה ועם נימוק כתוב: המטפל כבר + # מהדק, אבל המתודה הזו היא API ציבורי, וקורא ישיר לא יחתוך עם start + # שלילי ולא יקרוס על ערך לא-מספרי. + page_i = max(1, _safe_int(page, 1)) + per_page_i = min(max(1, _safe_int(per_page, OUTLINE_PER_PAGE_DEFAULT)), + OUTLINE_PER_PAGE_MAX) + + symbols = result["symbols"] + window = symbols[(page_i - 1) * per_page_i :][:per_page_i] + + # **מדידה בבתים, על הסריאליזציה האמיתית.** גרסה קודמת ניסתה להוכיח + # שהעמוד נכנס בתקציב על ידי חסימת אורך השם ב-200 **תווים** — וזה נשבר + # בשני מקומות: שם ב-CJK הוא שלושה בתים לתו, כך שעמוד מקסימלי הגיע + # ל-323,000 בתים מול תקציב של 256,000; והקיצוץ הרס את הזהות, כך + # ש-``symbol=`` עם השם המלא לא מצא את הסימבול. חסימת תווים אינה + # בטיחות בתים, וקיצוץ מזהה אינו אופציה. + # + # **וכשהעמוד לא נכנס — דוחים אותו, לא חותכים.** חיתוך בתוך עמוד יחד עם + # עימוד אריתמטי מאבד סימבולים: העמוד נעצר באמצע והבא מתחיל אחרי + # ``per_page`` המלא. דחייה מפורשת עם ``max`` היא חסרת אובדן, דטרמיניסטית, + # ואומרת לקורא בדיוק מה לעשות — במקום להחזיר תשובה שנראית שלמה. + payload_bytes = len(json.dumps(window, ensure_ascii=False).encode("utf-8")) + if payload_bytes > OUTPUT_BYTE_BUDGET: + return { + "ok": False, + "error": "page_too_large", + "bytes": payload_bytes, + "max": OUTPUT_BYTE_BUDGET, + "per_page": per_page_i, + } + + return { + "ok": True, + "status": "outline", + "file": file_meta, + "symbols": window, + "total": result["total"], + "page": page_i, + "per_page": per_page_i, + } + + class RepoBackend: """Duck-typed backend over a pymongo handle + the mirror/search services. @@ -363,6 +433,10 @@ def get_file( path: str, ref: str | None = None, lines: Any = None, + outline: bool = False, + symbol: str | None = None, + page: int = 1, + per_page: int = 100, ) -> dict[str, Any]: if is_denied(path): # policy: block, before touching the mirror return {"ok": False, "error": "path_denied"} @@ -370,6 +444,12 @@ def get_file( # כי קובץ גדול נפסל ממילא; עכשיו טווח פגום כמו ``[9, 2]`` היה גורם # לקריאה ולפענוח של עד 10MB רק כדי להיפסל בסוף. הבדיקה טהורה וזולה, # ואין סיבה שתרוץ אחרי העבודה היקרה. + # שני מצבי קריאה שאינם מצטברים. התעלמות שקטה מאחד מהם היא בדיוק + # הכשל שהפרויקט הזה רודף אחריו: הקורא טרח להעביר פרמטר, קיבל + # תשובה תקינה, ואין שום סימן שמה שביקש לא קרה. + if outline and lines is not None: + return {"ok": False, "error": "outline_and_lines"} + bounds: Any = None if lines is not None: # אותו עוזר משותף שמשרת גם את ``codekeeper_get_file``, כדי @@ -382,7 +462,10 @@ def get_file( # רק לקריאת טווח. בלי ``lines`` לא מועבר ``max_size`` כלל, כך # שברירת המחדל של שירות המראה נשארת מקור האמת היחיד ל-500KB — # ושתי ההתנהגויות לא נפרדות לשני מספרים שצריך לסנכרן. - size_kwargs = {"max_size": RANGE_READ_MAX_BYTES} if lines is not None else {} + # אאוטליין נשפט כמו קריאת טווח: הוא קורא את הקובץ כולו אבל מחזיר + # פלט זעיר, ולכן אין סיבה שתקרת התצוגה של 500KB תחסום אותו. + wants_slice = lines is not None or outline + size_kwargs = {"max_size": RANGE_READ_MAX_BYTES} if wants_slice else {} try: res = self._require_mirror().get_file_at_commit( repo, path, use_ref, **size_kwargs @@ -403,6 +486,9 @@ def get_file( file_meta["lines"] = res.get("lines") file_meta["encoding"] = res.get("encoding") content = res.get("content") + if outline: + return _outline_response(file_meta, content or "", path, + symbol, page, per_page) if bounds is not None: sliced = apply_line_range(content or "", *bounds) if isinstance(sliced, str): diff --git a/mcp_server/repo_handlers.py b/mcp_server/repo_handlers.py index 6b33de88b..afe6e6e30 100644 --- a/mcp_server/repo_handlers.py +++ b/mcp_server/repo_handlers.py @@ -24,6 +24,11 @@ # ומול ``SEARCH_RESULTS_MAX`` ו-``OUTPUT_BYTE_BUDGET`` זו התקרה שמשאירה עמוד # שלם בתקציב במקום להיחתך. CONTEXT_LINES_MAX = 10 +# עימוד האאוטליין. קבועים משלו ולא מיחזור של ``TREE_*``: רשומת סימבול +# נושאת שם מלא בנקודות ושתי שורות, ושוקלת יותר מנתיב. ``TREE_PER_PAGE_MAX`` +# הוא 1000, ועמוד כזה של סימבולים היה חוצה את תקציב הפלט לבדו. +OUTLINE_PER_PAGE_DEFAULT = 100 +OUTLINE_PER_PAGE_MAX = 500 OUTPUT_BYTE_BUDGET = 256_000 @@ -62,6 +67,10 @@ def get_repo_file( path: str, ref: str | None = None, lines: Any = None, + outline: bool = False, + symbol: str | None = None, + page: int = 1, + per_page: int = OUTLINE_PER_PAGE_DEFAULT, ) -> dict[str, Any]: name = (repo or "").strip() file_path = (path or "").strip() @@ -70,7 +79,14 @@ def get_repo_file( if not file_path: return {"ok": False, "error": "missing_path"} return backend.get_file( - repo=name, path=file_path, ref=((ref or "").strip() or None), lines=lines + repo=name, + path=file_path, + ref=((ref or "").strip() or None), + lines=lines, + outline=bool(outline), + symbol=((symbol or "").strip() or None), + page=_clamp(page, 1, 10_000, 1), + per_page=_clamp(per_page, 1, OUTLINE_PER_PAGE_MAX, OUTLINE_PER_PAGE_DEFAULT), ) diff --git a/mcp_server/server.py b/mcp_server/server.py index 1ddfd5d0e..a9c6dcf14 100644 --- a/mcp_server/server.py +++ b/mcp_server/server.py @@ -762,8 +762,13 @@ def list_repo_tree( # עוקף אותה. עכשיו הוא באמת עוקף, אבל **שתי תקרות שונות**, ולכן # שני המספרים חייבים להופיע: סוכן שקיבל ``too_large`` צריך לדעת # אם ``lines`` יעזור לו או שהקובץ מעבר לגבול בכל מקרה. - + " Size limits differ by mode: 500KB for a whole file, 10MB when " - "lines is given. Binary files return metadata only." + + " Set outline=true for a map instead of content: every function and " + "class with its start and end line, so you can follow up with an exact " + "lines= range. Names are fully qualified with dots (Class.method, " + "outer.inner), symbol= filters on that full name, and page/per_page walk " + "long files. Python only; anything else returns status no_outline. Size " + "limits differ by mode: 500KB for a whole file, 10MB with lines or " + "outline. Binary files return metadata only." ), annotations=_READ_ONLY_TOOL, ) @@ -773,10 +778,22 @@ def get_repo_file( path: str, ref: str | None = None, lines: StrictLines | None = None, + outline: bool = False, + symbol: str | None = None, + page: int = 1, + per_page: int = repo_handlers.OUTLINE_PER_PAGE_DEFAULT, ) -> dict: require_admin(ctx) return repo_handlers.get_repo_file( - repo_backend, repo=repo, path=path, ref=ref, lines=lines + repo_backend, + repo=repo, + path=path, + ref=ref, + lines=lines, + outline=outline, + symbol=symbol, + page=page, + per_page=per_page, ) @mcp.tool( diff --git a/tests/test_mcp_outline.py b/tests/test_mcp_outline.py new file mode 100644 index 000000000..bb0d1693a --- /dev/null +++ b/tests/test_mcp_outline.py @@ -0,0 +1,714 @@ +"""מפת סימבולים ב-``codekeeper_get_repo_file``. + +הרקע: ``webapp/app.py`` הוא 20,035 שורות. בלי מפה, סוכן שמחפש בו פונקציה +קורא טווח ומנחש. עם מפה הוא מקבל שורת התחלה וסיום ועובר ל-``lines=``. +""" + +from __future__ import annotations + +import ast +import io +import pathlib +import subprocess +import tokenize +from collections import Counter + +import pytest + +from mcp_server import repo_handlers +from mcp_server.outline import extract_outline + +# --------------------------------------------------------------------------- +# כלל מרחב השמות +# +# מרחב שמות בפייתון הוא פונקציה או מחלקה. ``if``/``try``/``with`` אינם. +# הכלל הזה החליף שלושה מקרים פרטיים — מתודות, פונקציות מקוננות, ומחלקות +# fallback בתוך ``except ImportError`` — והוא מה שהופך את המפה לשלמה. +# --------------------------------------------------------------------------- + + +def _names(text, **kw): + return [row["name"] for row in extract_outline(text, "x.py", **kw)["symbols"]] + + +def test_a_nested_function_is_prefixed_by_the_function_that_holds_it(): + """``build_mcp`` ב-``mcp_server/server.py`` מכילה 24 כלים מקוננים. + + בלי ירידה לעומק, אאוטליין של הקובץ הזה החזיר 9 סימבולים במקום 38, + ו-``build_mcp`` נראתה כבלוק אטום של 513 שורות. סוכן שביקש מפה כדי + למצוא כלי לערוך — קיבל מפה שהכלי לא נמצא בה. + """ + assert _names("def outer():\n def inner():\n pass\n") == [ + "outer", + "outer.inner", + ] + + +def test_a_method_is_the_same_rule_at_depth_two(): + assert _names("class C:\n def m(self):\n pass\n") == ["C", "C.m"] + + +def test_a_control_block_is_not_a_namespace_and_adds_no_prefix(): + """זה הפער השני, ונמצא רק אחרי שהראשון תוקן. + + רקורסיה שיורדת רק לתוך גופי פונקציות ומחלקות עדיין החמיצה 62 + סימבולים ב-``webapp/app.py``, כי ``_Missing``, ``_NoCache`` ו- + ``_NativeThread`` מוגדרות בתוך ``except ImportError`` — דפוס + ה-fallback הרגיל. ``try`` אינו מרחב שמות, ולכן אין תחילית. + """ + text = ( + "try:\n" + " from x import Thing\n" + "except ImportError:\n" + " class Thing:\n" + " def method(self):\n" + " pass\n" + ) + + assert _names(text) == ["Thing", "Thing.method"] + + +@pytest.mark.parametrize( + "block", ["if True:", "with open('f') as f:", "for i in []:", "while True:"] +) +def test_no_control_block_introduces_a_namespace(block): + assert _names(f"{block}\n def f():\n pass\n") == ["f"] + + +def test_an_async_function_is_a_symbol_like_any_other(): + assert _names("async def f():\n async def g():\n pass\n") == [ + "f", + "f.g", + ] + + +# --------------------------------------------------------------------------- +# המונה הבלתי תלוי +# +# ``ast.walk`` הוא אותה ספרייה שהמימוש משתמש בה, ולכן השוואה מולו קרובה +# מדי להשוואת פונקציה לעצמה. ``tokenize`` הוא **לקסר**: הוא לא בונה עץ, +# הוא סופר אסימונים בזרם התווים. זה מונה שנגזר ממקור אחר לגמרי. +# --------------------------------------------------------------------------- + + +def _count_definitions_by_tokenize(text: str) -> int: + return sum( + 1 + for token in tokenize.generate_tokens(io.StringIO(text).readline) + if token.type == tokenize.NAME and token.string in ("def", "class") + ) + + +_REAL_FILES = [ + "mcp_server/server.py", + "mcp_server/repo_backend.py", + "database/manager.py", + "webapp/app.py", + "main.py", +] + + +@pytest.mark.parametrize("relative", _REAL_FILES) +def test_the_outline_finds_every_definition_in_a_real_file(relative): + """הבדיקה שהייתה תופסת את שני הפערים מלכתחילה. + + היא רצה על קבצי הריפו עצמם ולא על דוגמאות, כי שני הפערים התגלו + דווקא במבנים שקיימים כאן ולא במבנים שחשבתי עליהם. + """ + path = pathlib.Path(relative) + if not path.exists(): # pragma: no cover - הריפו תמיד מכיל אותם + pytest.skip(f"{relative} לא קיים") + text = path.read_text(encoding="utf-8") + + found = extract_outline(text, relative) + + assert found["total"] == _count_definitions_by_tokenize(text), relative + + +def test_the_independent_counter_can_actually_fail(): + """מונה שלא מסוגל להפיל מימוש שגוי אינו ראיה. + + הגרסה שירדה רק לתוך גופי פונקציות מוצגת כאן במפורש, ו-``tokenize`` + סופר יותר ממה שהיא מוצאת. + """ + text = ( + "try:\n" + " class Stub:\n" + " def m(self):\n" + " pass\n" + "except ImportError:\n" + " Stub = None\n" + ) + + shallow = [n for n in ast.parse(text).body if isinstance(n, ast.ClassDef)] + + assert shallow == [] # ``class`` יושבת בתוך ``try``, לא ב-``body`` + assert _count_definitions_by_tokenize(text) == 2 + assert extract_outline(text, "x.py")["total"] == 2 + + +# --------------------------------------------------------------------------- +# מעטרים +# --------------------------------------------------------------------------- + + +def test_the_range_starts_at_the_decorator_and_not_at_the_def(): + """ב-``webapp/app.py`` 203 מתוך 408 הפונקציות ברמה העליונה מעוטרות. + + ``node.lineno`` מצביע על ה-``def``, ולכן טווח שנקרא לפי האאוטליין היה + מתחיל שורה אחרי ``@app.route(...)`` — כלומר מחמיץ בדיוק את מה שמזהה + את הנתיב. + """ + text = "@app.route('/x')\n@wraps(f)\ndef view():\n pass\n" + + symbols = extract_outline(text, "x.py")["symbols"] + + assert symbols[0]["start"] == 1 + assert symbols[0]["end"] == 4 + + +# --------------------------------------------------------------------------- +# שמות אינם ייחודיים +# --------------------------------------------------------------------------- + + +def test_two_symbols_may_share_a_full_name_and_both_survive(): + """נגזרת ישירה של כלל מרחב השמות: ``if``/``else`` אינם מרחב. + + זה לא תרחיש מומצא — ב-``webapp/app.py`` וב-``main.py`` יש היום שישה + מקרים כאלה, למשל ``login_required.decorated_function`` בשורות 3448 + ו-3465. כל שלב שיחזיק את הסימבולים ב-dict לפי שם ימחק אחד מהם בשקט. + """ + text = ( + "import sys\n" + "if sys.version_info >= (3, 11):\n" + " def parse():\n" + " pass\n" + "else:\n" + " def parse():\n" + " pass\n" + ) + + symbols = extract_outline(text, "x.py")["symbols"] + + assert [row["name"] for row in symbols] == ["parse", "parse"] + assert [row["start"] for row in symbols] == [3, 6] + + +def test_real_duplicate_names_in_the_repo_are_not_collapsed(): + """כל שם כפול מוחזר במספר המופעים המלא שלו. + + הטסט נגזר מהסימבולים עצמם ולא ננעל על שם ספציפי: ``app.py`` משתנה, + והתכונה שנבדקת היא שאף שלב לא ממפתח לפי שם — לא איזה שם במקרה כפול. + """ + path = pathlib.Path("webapp/app.py") + if not path.exists(): # pragma: no cover + pytest.skip("webapp/app.py לא קיים") + + text = path.read_text(encoding="utf-8") + symbols = extract_outline(text, "webapp/app.py")["symbols"] + + # העוגן הבלתי תלוי. הגרסה הקודמת של הטסט הזה גזרה גם את הציפייה וגם + # את הבדיקה מאותה רשימה, ולכן ``len(rows) == occurrences`` היה + # טאוטולוגיה: קריסה **חלקית** — שם כפול אחד שנעלם ואחר שנשאר — הייתה + # עוברת. הספירה מול ה-לקסר היא מה שתופס כל קריסה, מלאה או חלקית. + assert len(symbols) == _count_definitions_by_tokenize(text) + + repeated = {name: n for name, n in Counter(r["name"] for r in symbols).items() if n > 1} + + assert repeated, "אין שמות כפולים — הטסט מאבד את מה שהוא בודק" + for name in repeated: + starts = {row["start"] for row in symbols if row["name"] == name} + assert len(starts) == repeated[name] + + +# --------------------------------------------------------------------------- +# הסדר, ומה שהעימוד נשען עליו +# --------------------------------------------------------------------------- + + +def test_symbols_are_ordered_by_start_line(): + text = "def b():\n pass\n\n\ndef a():\n pass\n" + + starts = [row["start"] for row in extract_outline(text, "x.py")["symbols"]] + + assert starts == sorted(starts) + + +def test_the_order_is_stable_across_calls(): + """עימוד בלי סדר יציב הוא באג ממתין: סימבול יכול לדלג בין עמודים.""" + path = pathlib.Path("webapp/app.py") + if not path.exists(): # pragma: no cover + pytest.skip("webapp/app.py לא קיים") + text = path.read_text(encoding="utf-8") + + first = extract_outline(text, "webapp/app.py")["symbols"] + second = extract_outline(text, "webapp/app.py")["symbols"] + + assert first == second + + +# --------------------------------------------------------------------------- +# ``symbol=`` — החוזה +# --------------------------------------------------------------------------- + + +def test_the_filter_matches_the_full_name_so_a_namespace_returns_its_children(): + """``symbol="C"`` מחזיר גם את המחלקה וגם את מה שבתוכה. + + זו התנהגות מכוונת — "תן לי הכול תחת המרחב הזה" — ולכן היא מקובעת + ולא נשארת תופעת לוואי של התאמת תת-מחרוזת. + """ + text = "class Outer:\n def inner(self):\n pass\n\n\ndef other():\n pass\n" + + assert _names(text, symbol="Outer") == ["Outer", "Outer.inner"] + + +def test_the_filter_is_case_insensitive(): + assert _names("def ApiHandler():\n pass\n", symbol="apihandler") == ["ApiHandler"] + + +def test_total_counts_matches_and_not_the_whole_file(): + """העימוד נשען על ``total``. אם הוא סופר את הקובץ ולא את ההתאמות, + הקורא יבקש עמוד שני שלא קיים.""" + text = "def alpha():\n pass\n\n\ndef beta():\n pass\n\n\ndef gamma():\n pass\n" + + assert extract_outline(text, "x.py")["total"] == 3 + assert extract_outline(text, "x.py", symbol="alpha")["total"] == 1 + + +# --------------------------------------------------------------------------- +# ערוץ הכשל +# --------------------------------------------------------------------------- + + +def test_a_non_python_file_says_so_instead_of_returning_nothing(): + assert extract_outline("Title\n=====\n", "docs/page.rst") == { + "status": "no_outline", + "reason": "unsupported_language", + } + + +@pytest.mark.parametrize( + "text", + [ + "def f(:\n pass\n", # תחביר שבור + "print 'hello'\n", # פייתון 2 + "def f():\n x = '\ud800'\n", # surrogate — UnicodeEncodeError, לא SyntaxError + ], +) +def test_unparsable_input_is_reported_and_never_raised(text): + """``except SyntaxError`` לבדו היה מפיל את הכלי על השלישי. + + סוג החריגה תלוי-גרסה: בייט אפס הוא ``SyntaxError`` ב-3.11 + ו-``ValueError`` במקומות אחרים. מניית סוגים היא הגישה השברירית, ולכן + ההרחבה גורפת — אבל **רק** סביב הפרסינג. + """ + result = extract_outline(text, "x.py") + + assert result["status"] == "no_outline" + assert result["reason"] == "parse_error" + assert result["error_type"] + + +def test_the_reason_names_the_exception_class(): + """``except`` רחב בלי שם המחלקה הורג את היכולת לאבחן.""" + result = extract_outline("def f():\n x = '\ud800'\n", "x.py") + + assert result["error_type"] == "UnicodeEncodeError" + + +def test_a_bug_in_the_traversal_is_not_swallowed_as_no_outline(monkeypatch): + """ההרחבה עוטפת את הפרסינג בלבד, ובכוונה. + + אילו היא עטפה את כל הפונקציה, ``AttributeError`` על צומת לא צפוי היה + חוזר כ-``no_outline`` וקובץ תקין היה נראה כאילו אין לו סימבולים — + בדיוק הכישלון השקט של K11, שכבה אחת פנימה. + """ + import mcp_server.outline as module + + def _explode(_tree, _lines): + raise AttributeError("boom") + + monkeypatch.setattr(module, "_collect", _explode) + + with pytest.raises(AttributeError): + module.extract_outline("def f():\n pass\n", "x.py") + + +def test_deep_nesting_does_not_raise_from_our_side(): + """מחסנית מפורשת ולא רקורסיה: קינון עמוק אינו ``RecursionError`` שלנו.""" + text = "".join(f"{' ' * (4 * i)}def f{i}():\n" for i in range(60)) + text += " " * (4 * 60) + "pass\n" + + result = extract_outline(text, "x.py") + + assert result["status"] == "ok" + assert result["total"] == 60 + + +# --------------------------------------------------------------------------- +# הצרכן הוא לקוח MCP +# +# ``claude-md-snippets/testing.md`` כלל 1: הבדיקה עוברת דרך אותו ממשק כמו +# הצרכן. הטסטים למעלה בודקים את החילוץ; אלה בודקים את הכלי. +# --------------------------------------------------------------------------- + + +class _Mirror: + def __init__(self, text): + self._text = text + + def get_file_at_commit(self, repo, path, commit, **k): + return { + "success": True, + "file_path": path, + "resolved_commit": "abc123", + "is_binary": False, + "content": self._text, + "encoding": "utf-8", + "size": len(self._text), + "lines": self._text.count("\n") + 1, + } + + def get_default_branch(self, repo): + return "main" + + +def _backend(text): + from mcp_server.repo_backend import RepoBackend + + return RepoBackend(mirror=_Mirror(text)) + + +_SAMPLE = "".join(f"def f{i}():\n pass\n\n\n" for i in range(250)) + + +def test_the_tool_returns_an_outline_status_and_a_page(): + out = repo_handlers.get_repo_file( + _backend(_SAMPLE), repo="r", path="a.py", outline=True + ) + + assert out["status"] == "outline" + assert out["total"] == 250 + assert len(out["symbols"]) == repo_handlers.OUTLINE_PER_PAGE_DEFAULT + + +def test_paging_walks_forward_without_gaps_or_repeats(): + backend = _backend(_SAMPLE) + + first = repo_handlers.get_repo_file( + backend, repo="r", path="a.py", outline=True, page=1, per_page=100 + )["symbols"] + second = repo_handlers.get_repo_file( + backend, repo="r", path="a.py", outline=True, page=2, per_page=100 + )["symbols"] + + assert first[-1]["start"] < second[0]["start"] + assert not ({r["name"] for r in first} & {r["name"] for r in second}) + + +def test_per_page_is_clamped_to_the_outline_ceiling_and_not_the_tree_one(): + """``TREE_PER_PAGE_MAX`` הוא 1000, ורשומת סימבול שוקלת יותר מנתיב.""" + out = repo_handlers.get_repo_file( + _backend(_SAMPLE), repo="r", path="a.py", outline=True, per_page=99_999 + ) + + assert out["per_page"] == repo_handlers.OUTLINE_PER_PAGE_MAX + + +def test_reading_without_outline_is_untouched(): + """תוספתיות: בלי הפרמטר התשובה היא בדיוק זו של היום.""" + out = repo_handlers.get_repo_file(_backend("def f():\n pass\n"), repo="r", path="a.py") + + assert out["status"] == "ok" + assert out["content"] == "def f():\n pass\n" + assert "symbols" not in out + assert "total" not in out + + +def test_the_outline_and_a_range_read_agree_on_where_a_symbol_lives(): + """המסלול השלם: מפה ← טווח. אם הם לא מסכימים, המפה חסרת ערך.""" + text = "def a():\n pass\n\n\n@deco\ndef b():\n return 1\n" + backend = _backend(text) + + found = repo_handlers.get_repo_file( + backend, repo="r", path="a.py", outline=True, symbol="b" + )["symbols"][0] + body = repo_handlers.get_repo_file( + backend, repo="r", path="a.py", lines=[found["start"], found["end"]] + )["content"] + + assert body == "@deco\ndef b():\n return 1" + + +def test_an_unsupported_language_reaches_the_caller_as_a_status(): + out = repo_handlers.get_repo_file( + _backend("Title\n=====\n"), repo="r", path="page.rst", outline=True + ) + + assert out == { + "ok": True, + "file": out["file"], + "status": "no_outline", + "reason": "unsupported_language", + } + + +# --------------------------------------------------------------------------- +# מול git אמיתי +# --------------------------------------------------------------------------- + + +def _git(*args, cwd): + subprocess.run(["git", *args], cwd=cwd, check=True, capture_output=True) + + +def test_a_decorated_tool_is_findable_through_a_real_mirror(tmp_path): + """הטסט שהיה תופס את הפער מלכתחילה, בצורתו הכללית. + + כל הכתיבה תחת ``tmp_path``; אין מחיקה ואין נגיעה בעץ העבודה. + """ + work = tmp_path / "work" + work.mkdir() + (work / "srv.py").write_text( + "def register(mcp):\n" + " @mcp.tool(name='x')\n" + " def get_repo_file(path):\n" + " return path\n", + encoding="utf-8", + ) + _git("init", "-q", "-b", "main", cwd=work) + _git("-c", "user.email=t@t", "-c", "user.name=t", "add", "-A", cwd=work) + _git("-c", "user.email=t@t", "-c", "user.name=t", "commit", "-qm", "init", cwd=work) + + mirrors = tmp_path / "mirrors" + mirrors.mkdir() + subprocess.run( + ["git", "clone", "--quiet", "--mirror", str(work), str(mirrors / "demo.git")], + check=True, + capture_output=True, + ) + + from mcp_server.repo_backend import RepoBackend + from services.git_mirror_service import GitMirrorService + + backend = RepoBackend(mirror=GitMirrorService(base_path=str(mirrors))) + out = backend.get_file(repo="demo", path="srv.py", outline=True) + + names = {row["name"] for row in out["symbols"]} + assert "register.get_repo_file" in names + found = next(r for r in out["symbols"] if r["name"] == "register.get_repo_file") + assert found["start"] == 2 # שורת ה-``@mcp.tool``, לא ה-``def`` + + +# --------------------------------------------------------------------------- +# נרמול הקלט +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize("suffix", [".py", ".PY", ".Py", ".pyi", ".PYI"]) +def test_a_python_file_is_recognised_whatever_the_case_of_its_suffix(suffix): + """``.pyi`` הוא פייתון תקין, והוא נפוץ הרבה יותר מסיומת באותיות גדולות.""" + assert extract_outline("def f():\n pass\n", f"mod{suffix}")["status"] == "ok" + + +def test_a_unicode_name_is_matched_case_insensitively(): + """``lower`` מפספסת מיפויים של יותר מתו אחד; ``casefold`` לא.""" + assert _names("def straße():\n pass\n", symbol="STRASSE") == ["straße"] + + +def test_a_multiline_parenthesised_decorator_still_anchors_at_the_at_sign(): + """``d.lineno`` הוא שורת ה**ביטוי**, לא שורת ה-``@``. + + בכתיב הרגיל הם זהים. ב-``@(`` עם ירידת שורה הביטוי מתחיל שורה מאוחר + יותר, וטווח שנגזר מהאאוטליין היה מחמיץ את השורה הראשונה של הסימבול. + התחביר הזה אינו מופיע בריפו אפילו פעם אחת, אבל התיקון הוא ארבע שורות + בלי תלות חדשה — זול יותר מלתעד מגבלה ידועה. + """ + text = "@(\n deco\n)\ndef f():\n pass\n" + + assert extract_outline(text, "x.py")["symbols"][0]["start"] == 1 + + +def test_an_undecorated_symbol_is_not_dragged_backwards(): + """הסריקה אחורה רצה רק כשיש מעטר, ולא בולעת שורות שכנות.""" + text = "@deco\ndef a():\n pass\n\n\ndef b():\n pass\n" + + symbols = extract_outline(text, "x.py")["symbols"] + + assert symbols[0]["start"] == 1 + assert symbols[1]["start"] == 6 + + +# --------------------------------------------------------------------------- +# העימוד — בטוח בהוכחה, לא בתקווה +# --------------------------------------------------------------------------- + + +#: תו CJK תופס שלושה בתים ב-UTF-8. זה בדיוק המקרה שהוכחה שנשענה על +#: ספירת **תווים** פספסה. +_CJK = "\u5b57" + + +def _wide_symbol(index: int) -> str: + return "def " + _CJK * 181 + f"{index}():\n pass\n" + + +def test_an_oversized_page_is_rejected_and_never_silently_trimmed(): + """הגרסה הקודמת "הוכיחה" שעמוד נכנס בתקציב על ידי חסימת אורך השם + ב-200 **תווים**. זה נשבר בשני מקומות: שם ב-CJK הוא שלושה בתים לתו, + כך שעמוד מקסימלי הגיע ל-323,000 בתים מול תקציב של 256,000; והקיצוץ + הרס את הזהות. עכשיו נמדדים בתים אמיתיים, והעמוד נדחה במקום להיחתך. + """ + text = "".join(_wide_symbol(i) for i in range(500)) + + out = repo_handlers.get_repo_file( + _backend(text), repo="r", path="a.py", outline=True, per_page=500 + ) + + assert out["ok"] is False + assert out["error"] == "page_too_large" + assert out["bytes"] > out["max"] == repo_handlers.OUTPUT_BYTE_BUDGET + + +def test_a_smaller_page_of_the_same_file_succeeds_and_fits(): + """הדחייה אומרת לקורא מה לעשות, וזה באמת עובד.""" + import json + + text = "".join(_wide_symbol(i) for i in range(500)) + + out = repo_handlers.get_repo_file( + _backend(text), repo="r", path="a.py", outline=True, per_page=100 + ) + + assert out["status"] == "outline" + serialized = json.dumps(out["symbols"], ensure_ascii=False).encode("utf-8") + assert len(serialized) <= repo_handlers.OUTPUT_BYTE_BUDGET + + +def test_a_long_name_keeps_its_identity_so_the_filter_still_finds_it(): + """קיצוץ השם שבר את ``symbol=``: חיפוש בשם המלא החזיר אפס תוצאות. + + מזהה שנחתך אינו מזהה, ושני שמות ארוכים ושונים היו יכולים להתנגש בו. + """ + deep = "".join(f"{' ' * (4 * i)}def name{i}():\n" for i in range(60)) + deep += " " * (4 * 60) + "pass\n" + full = ".".join(f"name{i}" for i in range(60)) + + assert len(full) > 200 + assert _names(deep, symbol=full) == [full] + + +def test_no_symbol_is_unreachable_across_pages(): + """המאפיין שהעימוד קיים בשבילו: איחוד כל העמודים הוא כל הסימבולים.""" + backend = _backend(_SAMPLE) + seen = [] + for page in range(1, 6): + seen += repo_handlers.get_repo_file( + backend, repo="r", path="a.py", outline=True, page=page, per_page=50 + )["symbols"] + + every = repo_handlers.get_repo_file( + backend, repo="r", path="a.py", outline=True, per_page=500 + )["symbols"] + + assert seen == every[: len(seen)] + assert len(seen) == 250 + + +@pytest.mark.parametrize("page", [0, -5, "x", None, 1.5]) +def test_a_direct_caller_cannot_slice_with_a_bad_page(page): + """``get_file`` הוא API ציבורי, ולא רק מה שהמטפל קורא לו. + + זו בדיוק ההגנה ש-``list_tree`` עושה בכוונה, עם הערה כתובה, ושמסלול + האאוטליין דילג עליה. + """ + from mcp_server.repo_backend import RepoBackend + + backend = RepoBackend(mirror=_Mirror(_SAMPLE)) + + out = backend.get_file(repo="r", path="a.py", outline=True, page=page) + + assert out["page"] == 1 + assert out["symbols"][0]["name"] == "f0" + + +@pytest.mark.parametrize("per_page", ["x", None, [1]]) +def test_a_non_numeric_per_page_falls_back_to_the_default(per_page): + from mcp_server.repo_backend import RepoBackend + + backend = RepoBackend(mirror=_Mirror(_SAMPLE)) + + out = backend.get_file(repo="r", path="a.py", outline=True, per_page=per_page) + + assert out["per_page"] == repo_handlers.OUTLINE_PER_PAGE_DEFAULT + + +@pytest.mark.parametrize("per_page", [0, -1]) +def test_a_non_positive_per_page_becomes_the_smallest_valid_page(per_page): + """אותה נוסחה בדיוק כמו ב-``list_tree``, ובכוונה. + + אפס אינו "לא מספר" — הוא מספר מחוץ לתחום, ולכן הוא נצבט לגבול הקרוב + ולא נופל לברירת המחדל. יישור למוסכמה של הקובץ עדיף על התנהגות שנייה + שנראית סבירה לבדה. + """ + from mcp_server.repo_backend import RepoBackend + + backend = RepoBackend(mirror=_Mirror(_SAMPLE)) + + out = backend.get_file(repo="r", path="a.py", outline=True, per_page=per_page) + + assert out["per_page"] == 1 + assert len(out["symbols"]) == 1 + + +def test_the_response_does_not_carry_a_truncated_flag(): + """שדה שתמיד ``false`` מסמן משהו אחר ממה שאותה מילה מסמנת בכלי השכן. + + ב-``list_repo_tree`` ``truncated`` פירושו "התקציב חתך בתוך העמוד". + כאן אין חיתוך, ולכן אין שדה — ``total`` הוא מה שאומר אם יש עוד עמודים. + """ + out = repo_handlers.get_repo_file( + _backend(_SAMPLE), repo="r", path="a.py", outline=True + ) + + assert "truncated" not in out + + +# --------------------------------------------------------------------------- +# ``lines`` ו-``outline`` אינם מצטברים +# --------------------------------------------------------------------------- + + +def test_asking_for_both_a_range_and_an_outline_is_rejected(): + """התעלמות שקטה מפרמטר שהקורא העביר היא הכשל השקט מהצד השני. + + הוא היה מקבל תשובה תקינה, בלי שום סימן שהטווח שביקש לא נקרא. + """ + out = repo_handlers.get_repo_file( + _backend(_SAMPLE), repo="r", path="a.py", outline=True, lines=[1, 5] + ) + + assert out == {"ok": False, "error": "outline_and_lines"} + + +@pytest.mark.parametrize( + ("label", "text"), + [ + ("comment", "@(\n # note\n deco\n)\ndef f():\n pass\n"), + ("blank", "@(\n\n deco\n)\ndef f():\n pass\n"), + ], +) +def test_a_decorator_split_by_a_comment_or_blank_line_still_anchors_at_the_at_sign( + label, text +): + """הסריקה אחורה מדלגת על שורות ריקות והערות, אבל רק אם היא נוחתת על ``@``.""" + assert extract_outline(text, "x.py")["symbols"][0]["start"] == 1 + + +def test_a_leading_comment_does_not_drag_an_undecorated_symbol_backwards(): + """הדילוג לא בולע שורות כשאין ``@`` בסוף המסלול.""" + text = "# a note\n\ndef f():\n pass\n" + + assert extract_outline(text, "x.py")["symbols"][0]["start"] == 3 diff --git a/tests/test_mcp_repo_handlers.py b/tests/test_mcp_repo_handlers.py index 766a2ce65..857cef2b0 100644 --- a/tests/test_mcp_repo_handlers.py +++ b/tests/test_mcp_repo_handlers.py @@ -19,8 +19,15 @@ def list_tree(self, *, repo, path, ref, page, per_page, byte_budget, include_sta ) return {"ok": True} - def get_file(self, *, repo, path, ref, lines=None): - self.calls.append(("get", repo, path, ref, lines)) + # כמו ב-``list_tree`` למעלה: הפרמטרים החדשים נוספים **בסוף** החתימה + # ובסוף ה-tuple, כי הטסטים הקיימים משווים לפי אינדקס מיקומי. + def get_file( + self, *, repo, path, ref, lines=None, + outline=False, symbol=None, page=1, per_page=100, + ): + self.calls.append( + ("get", repo, path, ref, lines, outline, symbol, page, per_page) + ) return {"ok": True} def search(self, *, repo, query, file_pattern, max_results, byte_budget, context_lines=0): @@ -61,7 +68,11 @@ def test_get_file_requires_repo_and_path(): assert rh.get_repo_file(be, repo="r", path=" ") == {"ok": False, "error": "missing_path"} assert be.calls == [] rh.get_repo_file(be, repo="r", path=" a.py ", ref=" ") - assert be.calls[0] == ("get", "r", "a.py", None, None) # trimmed; blank ref -> None + # trimmed; blank ref -> None. ברירות המחדל של האאוטליין בסוף ה-tuple, + # והן חייבות להיות "כבוי" — קריאה רגילה לא מבקשת מפה. + assert be.calls[0] == ( + "get", "r", "a.py", None, None, False, None, 1, rh.OUTLINE_PER_PAGE_DEFAULT + ) def test_search_validates_query_and_clamps(): diff --git a/tests/test_mcp_server_build.py b/tests/test_mcp_server_build.py index fe5c8b1d1..d915abd28 100644 --- a/tests/test_mcp_server_build.py +++ b/tests/test_mcp_server_build.py @@ -370,6 +370,17 @@ async def test_the_description_names_both_size_ceilings(): description = mcp._tool_manager.get_tool("codekeeper_get_repo_file").description assert "500KB for a whole file" in description - assert "10MB when lines is given" in description + assert "10MB with lines or outline" in description assert description.index("500KB") > description.index("lines=[start, end]") assert "Binary files return metadata only" in description + + +async def test_the_description_tells_the_agent_the_outline_is_python_only(): + """סוכן שיבקש מפה של ``.rst`` ויקבל ``no_outline`` צריך לדעת שזו + התנהגות מוצהרת ולא תקלה — אחרת הוא ינסה שוב.""" + mcp = build_mcp(_FakeBackend(), repo_backend=_FakeRepoBackend()) + description = mcp._tool_manager.get_tool("codekeeper_get_repo_file").description + + assert "outline=true" in description + assert "Python only" in description + assert "no_outline" in description