Skip to content

refactor(services): חילוץ מודל הסעיפים המשותף ל-services/doc_sections.py - #3394

Merged
amirbiron merged 1 commit into
mainfrom
claude/extract-shared-model-doc-sections-hcjymr
Sep 16, 2026
Merged

amirbiron merged 1 commit into
mainfrom
claude/extract-shared-model-doc-sections-hcjymr

Conversation

@amirbiron

@amirbiron amirbiron commented Sep 16, 2026 •

Copy link
Copy Markdown
Owner

✨ תיאור קצר

PR 2 בתוכנית "שלב 1 — docs_get_section ל-Markdown". כל מה שאינו תלוי בשפת המקור — Section, Document, וכל פונקציות העץ — עובר מ-services/rst_parser.py למודול חדש services/doc_sections.py, ו-rst_parser מייצא אותם מחדש. ריפקטור טהור: אפס שינוי התנהגות, וההוכחה לכך היא אפס-דיף מדוד ולא "הטסטים עוברים".

הסיבה: PR 3 מוסיף פארסר Markdown שבונה לתוך אותו חוזה בדיוק. שני פארסרים שבונים Section משלהם ומחשבים end_line משלהם הם שתי הגדרות לאותו כלל, ומי שיתקן באג בחישוב ההיררכיה יתקן אותו באחד משניהם והשני יסטה בשקט.

📦 שינויים עיקריים

  • קוד (Backend)
  • בוט טלגרם
  • מסד נתונים/מיגרציות
  • תיעוד (docs/)
  • DevOps/CI/CD

פירוט:

  • services/doc_sections.py (חדש) — Section, Document, TooManySections, _finalize, normalize_title, find_sections, section_bounds, section_text, direct_subsections, neighbors, build_toc, suggest. ייבוא קל בלבד: re, dataclasses, difflib, typing.
  • services/rst_parser.py — נשאר הפארסר של RST בלבד (זיהוי כותרות, כללי ה-adornment, parse_document), ומייצא מחדש את השמות שעברו דרך __all__.
  • tests/test_doc_sections.py (חדש) — 19 טסטים.
  • scripts/docs_section_zero_diff.py (חדש) — כלי ההוכחה. ישמש שוב ב-PR 4, שגם הוא דורש אפס-דיף על RST.
  • mcp_server/outline_scanners/__init__.py — תיקון מדידה שהשינוי מייתר (פירוט למטה).

מה שומר על הקוראים הקיימים

הייצוא-מחדש הוא הפניה לאותו אובייקט ולא עותק, ולכן rst_parser.Section is doc_sections.Section. זה מה שמחזיק שני דברים שהיו נשברים בשקט אילו מישהו היה מגדיר מחדש במקום לייבא:

  • mcp_server/outline_scanners/rst.py תופס except rst_parser.TooManySections — שתי מחלקות שונות היו גורמות ל-except לפספס, והחריגה הייתה עולה מהסורק במקום להיהפך ל-too_many_symbols.
  • tests/test_mcp_outline.py מחליף את rst_parser.Section במונה כדי לוודא שהתקרה עוצרת את הפרסור.

mcp_server/docs_handlers.py לא שונה כלל בשלב הזה (הוא משתנה ב-PR 5).

🧪 בדיקות

ההוכחה המרכזית: אפס-דיף מול main

scripts/docs_section_zero_diff.py מריץ סוללה דטרמיניסטית של docs_get_section על כל 208 קובצי ה-RST ב-docs/ ומוציא JSONL קנוני + sha256. הורץ פעמיים — פעם ב-worktree של main ופעם על הענף, על אותה תיקיית קורפוס בדיוק כדי ששינוי בתיעוד לא יתחפש לשינוי התנהגות:

קבצים 208
רשומות 5,840
sha256 (main) 610f831d3b8985bd9101024bec4b0b5aa6efbc1484906f17c09746d4ca7a0f90
sha256 (הענף) 610f831d3b8985bd9101024bec4b0b5aa6efbc1484906f17c09746d4ca7a0f90
diff ריק

פילוח מסלולי התשובה שנכסו: toc 208 · section 5,166 · section_not_found 416 · ambiguous_section 45 · missing_path 3 · repo_not_allowed 1 · not_found 1. בנוסף: 208 תשובות עם suggestions לא ריקות, 239 עם truncated, 685 עם תת-סקשנים, 2,218 עם שכנים.

והסוללה הוכחה כמסוגלת ליפול

שבע מוטציות, כל אחת נותנת digest שונה:

המוטציה digest
ללא מוטציה 610f831d…
_finalize — <= ל-< e4ab446d…
normalize_title בלי casefold 86ef5bd7…
normalize_title בלי כיווץ רווחים 360f10a8…
normalize_title בלי איחוד מקפים 39d045cd…
build_toc סופר תווים ולא בייטים 639b5290…
suggest עם cutoff=0.9 c016b814…
section_bounds off-by-one fda5448e…

ואחת מהן חשפה חור אמיתי בסוללה. בגרסה הראשונה ביטול ה-casefold לא שינה את ה-digest בכלל — כי הסוללה שאלה כל כותרת בדיוק כפי שה-TOC החזיר אותה, ואז שני צדי ההשוואה עוברים את אותו נרמול. כלומר מוטציה אמיתית עברה את ההוכחה בשקט. משם נוספו שלוש וריאציות נרמול לכל כותרת (swapcase, רווחים, מקף ארוך), והמספר עלה מ-3,663 ל-5,840 רשומות.

טסטים

  • tests/test_rst_parser.py — לא שונה כלל. הוא עד הרגרסיה של השינוי הזה.

  • tests/test_doc_sections.py — 19 חדשים. חמישה שומרי מבנה שנבדקים על המקור ולא על ההתנהגות (הגדרה יחידה לכל שם, זהות הייצוא-מחדש, איסור ייבוא פארסר או mcp_server, אפס מודולים כבדים בתת-תהליך נקי, וכיסוי כל השמות ש-docs_handlers משתמש בהם — נגזר מה-AST של הצרכן ולא מרשימה מוקלדת). וארבעה-עשר התנהגותיים שבונים Section ביד ולא דרך פארסר, כי test_rst_parser.py כבר בודק את המודל דרך RST.

  • כל טסט חדש הוכח כנופל תחת מוטציה מתאימה, אחד-אחד ובבידוד. שתי מוטציות נתפסו בתחילה על ידי טסט אחר בגלל -x, ולכן הורצו שוב לבד.

  • 1,114 טסטים ב-tests/test_mcp*.py + test_rst_parser.py + test_doc_sections.py — עוברים.

  • Unit

  • Integration

  • Manual

באג שנמצא בטסט שלי עצמו, תוך כדי

test_doc_sections_imports_no_parser_and_no_mcp בדק בתחילה את node.module בלבד. לכן from services import rst_parser — בדיוק הסגנון הנהוג בריפו, ולכן הצורה הסבירה ביותר שמישהו יכתוב — התחמק ממנו: שם המודול שם הוא services, והפארסר יושב ב-names. נמדד שהמוטציה עברה, ונתפסה רק במקרה על ידי ייבוא מעגלי שהפיל טסט אחר. תוקן לאסוף גם את השמות המיובאים.

🧪 בדיקות נדרשות ב‑PR

  • 🔍 Code Quality & Security
  • Unit Tests (3.11)
  • Unit Tests (3.12)

📝 סוג שינוי

  • refactor: שינוי קוד ללא שינוי התנהגות

✅ צ'קליסט

  • הקוד עוקב אחרי הסגנון — flake8 נקי על כל חמשת הקבצים; isort נקי; mypy ללא שגיאות חדשות (ראו "השפעות/סיכונים")
  • בדיקות רצות ועוברות
  • תיעוד עודכן — לא נדרש: אין שינוי בחוזה החיצוני של אף כלי. התיעוד של הפיצ'ר עצמו נכנס ב-PR 5, לפי כלל "שלושה מקומות שמתעדכנים יחד"
  • אין ג'ובים חדשים
  • אין משתני סביבה חדשים
  • אין טוקנים/ערכת נושא
  • אין סודות/מפתחות בקוד
  • אין מחיקות מסוכנות — הסקריפט כותב רק ל---out שנמסר לו, ואינו מוחק דבר
  • הודעת הקומיט תואמת Conventional Commits
  • עיינתי במסמכי אתר התיעוד — נתיב: docs/mcp-server.rst (סעיפי mcp-outline, mcp-limits) | המשפט: "אל תערבב, ואל תמציא ערך חדש בלי לעדכן את docs/mcp-server.rst" — נבדק שאף ערך status/error/reason לא נוסף ולא השתנה, ולכן אין מה לעדכן

דפוסי באגים — מה נקרא ומה נמצא

  • claude-md-snippets/testing.md — נקרא לפני כתיבת הטסטים. כלל 5 ("הרץ בדיקה חדשה ווּדא שהיא נופלת") הוא מה שהוביל לכל מטריצות המוטציות כאן, ולגילוי החור ב-normalize_title ובטסט הייבוא.
  • BY-STACK/hebrew-source.md H6 — נקרא כי build_toc מודד len(...encode("utf-8")). המסקנה: זה False Positive מפורש לפי המסמך ("מדידה שבאמת עוסקת בגודל אחסון או בגודל תעבורה"), כי approx_bytes משרת את תקציב הבתים של התשובה. אין באג — וזה עדיין קובע בטסט (test_build_toc_measures_bytes_and_not_characters, על עברית דווקא, כי באנגלית שתי היחידות זהות והבדיקה הייתה חסרת ערך). נבדק גם שהחיתוך במסלול עצמו עקבי: docs_handlers חותך בתווים והשדות נקראים max_chars/remaining_chars.
  • CRITICAL-PATTERNS.md K11 — לא רלוונטי ל-PR הזה ואומר זאת במפורש: הכלל נדלק לפני עטיפת קריאה ב-try/except, וה-PR הזה אינו מוסיף אף try/except.
  • line-number-coupling — ההפניות בתיעוד שנכתב כאן הן לשמות סימבולים, לא למספרי שורות.

🧩 השפעות/סיכונים

  • סיכון ייצור: נמוך מאוד. אפס שינוי התנהגות, מוכח בייט-בייט על כל הקורפוס.
  • mypy: שתי שגיאות arg-type ב-mcp_server/docs_handlers.py:130,136 — קיימות בדיוק כך גם ב-main, בקוד שה-PR הזה לא נוגע בו, ו-CI חוסם רק על attr-defined/return-value.
  • black: services/doc_sections.py נשמר זהה לקוד שממנו הועבר ולא עבר פירמוט, כדי שהדיף יישאר "העברה" ולא "העברה + פירמוט". זה עקבי עם הריפו: rst_parser.py עצמו אינו black-clean ב-main, וכך גם 5 מתוך 6 קבצים מקבילים שנבדקו; ב-CI black רץ עם || true.
  • __all__ ב-rst_parser אינו קישוט: בלעדיו שמונה מהשמות אינם נקראים בגוף המודול ו-pyflakes מדווח F401 על כל אחד (נמדד — בדיוק שמונה). # noqa שמונה פעמים היה מסתיר את האזהרה במקום להצהיר על הכוונה. נבדק ששני מצבי הסחיפה מכוסים: הסרה מרשימת הייבוא מפילה טסט, והסרה מ-__all__ בלבד מדליקה F401.

תיקון מדידה שהשינוי הזה מייתר

ה-docstring של mcp_server/outline_scanners/__init__.py קיבע "72 מודולים, שלושה מהריפו" כנימוק להיתר שהסורק מייבא מ-services. השינוי מוסיף מודול לשרשרת, ולכן המספר נמדד מחדש על שני העצים באותה סביבה: התוספת היא מודול אחד בדיוק (services.doc_sections) ואפס מודולים כבדים.

והמספר הכולל הוסר משם, כי הוא תלוי-סביבה ולא תלוי-קוד: services/backoff_state.py מנסה לייבא observability, ולכן אותו ייבוא נמדד 73 מודולים בסביבה שבה זה נכשל ו-330 בסביבה שבה זה מצליח — פי ארבעה וחצי, בלי ששורת קוד אחת השתנתה. מה שמחזיק הוא האינווריאנט (אפס כבדים, ומהריפו רק מה שרשום בשמו), ולא מספר שמתיישן בלי שאיש ישים לב.

🔗 קישורים

🧯 סיכון / החזרה לאחור (Rollback)

git revert של קומיט אחד. אין מיגרציה, אין שינוי סכימה, אין שינוי קונפיג, ואין צרכן חיצוני שהחוזה שלו זז.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JofjowYGDSmyqYvtBxKeJV


Generated by Claude Code

Review in cubic

PR 2 בתוכנית "docs_get_section ל-Markdown". ריפקטור טהור, בלי שום שינוי
התנהגות, שמכין את הקרקע לפארסר Markdown (PR 3).

מה עבר ומה נשאר
---------------
כל מה שאינו תלוי בשפת המקור עבר ל-``services/doc_sections.py``: ``Section``,
``Document``, ``TooManySections``, ``_finalize``, ``normalize_title``,
``find_sections``, ``section_bounds``, ``section_text``,
``direct_subsections``, ``neighbors``, ``build_toc`` ו-``suggest``.

``services/rst_parser.py`` נשאר הפארסר של RST בלבד — זיהוי כותרות, כללי
ה-adornment ו-``parse_document`` — ומייצא מחדש את השמות שעברו, כדי שאף קורא
קיים לא יישבר. הייצוא-מחדש הוא הפניה לאותו אובייקט, ולכן
``rst_parser.Section is doc_sections.Section`` ו-``except
rst_parser.TooManySections`` בסורק ממשיכים לעבוד בדיוק כמו קודם.

למה מודול נפרד: שני פארסרים שבונים ``Section`` משלהם ומחשבים ``end_line``
משלהם הם שתי הגדרות לאותו כלל, ומי שיתקן באג באחת מהן יפספס את השנייה.
התקדים בריפו הוא ``_lines.py`` ו-``_ceiling.py`` ליד סורקי האאוטליין.

ההוכחה: אפס-דיף מדוד
--------------------
``scripts/docs_section_zero_diff.py`` מריץ סוללה דטרמיניסטית של
``docs_get_section`` על כל 208 קובצי ה-RST ב-docs/ ומוציא JSONL + sha256.
הסוללה מכסה TOC, כל כותרת עם תת-סקשנים ובלעדיהם, שלוש וריאציות נרמול לכל
כותרת, ambiguous_section, section_not_found עם הצעות, וחיתוך עם עמוד שני.

5,840 רשומות, sha256 זהה בין main לענף:
610f831d3b8985bd9101024bec4b0b5aa6efbc1484906f17c09746d4ca7a0f90

והסוללה הוכחה כמסוגלת ליפול: שבע מוטציות (``_finalize``, ``build_toc``,
``section_bounds``, ``suggest``, ושלושת כללי ``normalize_title``) נותנות כל
אחת digest שונה. אחת מהן — ביטול ה-``casefold`` — עברה בשקט בגרסה הראשונה,
כי הסוללה שאלה כל כותרת בדיוק כפי שה-TOC החזיר אותה ושני צדי ההשוואה עברו
את אותו נרמול; משם נוספו וריאציות הנרמול.

תיקון מדידה שהשינוי הזה מייתר
-----------------------------
ה-docstring של ``mcp_server/outline_scanners/__init__.py`` קיבע "72 מודולים,
שלושה מהריפו". המספר נמדד מחדש על שני העצים באותה סביבה: התוספת היא מודול
אחד בדיוק (``services.doc_sections``) ואפס מודולים כבדים. המספר הכולל הוסר
משם כי הוא תלוי-סביבה — ``backoff_state`` מנסה לייבא ``observability``, ולכן
אותו ייבוא נמדד 73 בסביבה שבה זה נכשל ו-330 בסביבה שבה זה מצליח.

בדיקות
------
tests/test_doc_sections.py — 19 טסטים חדשים: חמישה שומרי מבנה (הגדרה יחידה,
זהות הייצוא-מחדש, איסור ייבוא פארסר, אפס מודולים כבדים, וכיסוי השמות
שה-handler משתמש בהם) וארבעה-עשר התנהגותיים שבונים ``Section`` ביד ולא דרך
פארסר. כל אחד הוכח כנופל תחת מוטציה מתאימה.

tests/test_rst_parser.py לא שונה כלל — הוא עד הרגרסיה של השינוי הזה.
1,114 טסטים ב-tests/test_mcp*.py + test_rst_parser + test_doc_sections עוברים.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JofjowYGDSmyqYvtBxKeJV

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 5 days and 3 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@github-actions

Copy link
Copy Markdown
Contributor

🧯 Dangerous deletes guard report

Policy: see .cursorrules — dangerous deletions are blocked unless wrapped safely.

Summary:

  • Flagged findings (blocking): 0
    0
  • Excluded matches (not blocking): 15
  • Total matches (all files): 129

Flagged findings (file:line:snippet):
(none)

Excluded matches (by path pattern)
./webapp/static/js/md_preview.bundle.js.map:4:  "sourcesContent": ["// Markdown-it plugin to render GitHub-style task lists; see\n//\n// https://github.com/blog/1375-task-lists-in-gfm-issues-pulls-comments\n// https://github.com/blog/1825-t … [truncated]
./docs/DOCUMENTATION_GUIDE.md:453:rm -rf _build
./docs/Makefile:24:	rm -rf $(BUILDDIR)
./Dockerfile:42:    rm -rf /var/lib/apt/lists/*
./Dockerfile:121:    rm -rf /var/lib/apt/lists/*
./node_modules/katex/package.json:153:    "build": "rimraf dist/ && mkdirp dist && cp README.md dist && rollup -c --failAfterWarnings && webpack && node update-sri.js package dist/README.md",
./node_modules/katex/src/fonts/Makefile:139:	rm -rf pfa ff otf ttf woff woff2
./node_modules/mermaid/dist/mermaid.js.map:4:  "sourcesContent": ["/**\n* Default values for dimensions\n*/\nconst defaultIconDimensions = Object.freeze({\n\tleft: 0,\n\ttop: 0,\n\twidth: 16,\n\theight: 16\n});\n/**\n* Default values for tr … [truncated]
./node_modules/mermaid/dist/chunks/mermaid.esm/chunk-2M32CCKP.mjs.map:4:  "sourcesContent": ["{\n  \"name\": \"mermaid\",\n  \"version\": \"11.12.0\",\n  \"description\": \"Markdown-ish syntax for generating flowcharts, mindmaps, sequence d … [truncated]
./node_modules/mermaid/dist/chunks/mermaid.esm.min/chunk-4HFYJGYH.mjs.map:4:  "sourcesContent": ["{\n  \"name\": \"mermaid\",\n  \"version\": \"11.12.0\",\n  \"description\": \"Markdown-ish syntax for generating flowcharts, mindmaps, sequen … [truncated]
./node_modules/mermaid/dist/chunks/mermaid.esm.min/chunk-4HFYJGYH.mjs:1:var r={name:"mermaid",version:"11.12.0",description:"Markdown-ish syntax for generating flowcharts, mindmaps, sequence diagrams, class diagrams, gantt charts, git graph … [truncated]
./node_modules/mermaid/dist/chunks/mermaid.core/chunk-KS23V3DP.mjs.map:4:  "sourcesContent": ["{\n  \"name\": \"mermaid\",\n  \"version\": \"11.12.0\",\n  \"description\": \"Markdown-ish syntax for generating flowcharts, mindmaps, sequence  … [truncated]
./node_modules/mermaid/dist/mermaid.min.js:1524:`,"getStyles"),c1e=RQe});var h1e={};dr(h1e,{diagram:()=>NQe});var NQe,f1e=N(()=>{"use strict";$ge();a1e();l1e();u1e();NQe={parser:Fge,db:n1e,renderer:o1e,styles:c1e}});var m1e,g1e=N(()=>{"use  … [truncated]
./node_modules/mermaid/dist/mermaid.min.js.map:4:  "sourcesContent": ["/**\n* Default values for dimensions\n*/\nconst defaultIconDimensions = Object.freeze({\n\tleft: 0,\n\ttop: 0,\n\twidth: 16,\n\theight: 16\n});\n/**\n* Default values fo … [truncated]
./README.md:842:find . -name "__pycache__" -exec rm -rf {} +

@sourcery-ai

sourcery-ai Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

הריפקטור מרכז את מודל הסעיפים והלוגיקה המשותפת ב-services/doc_sections.py, בעוד rst_parser נשאר ממוקד בתחביר RST ומייצא את ה-API הישן באמצעות אותן מחלקות ופונקציות. בדיקות מבניות וכלי אפס-דיף מאמתים הן את ארכיטקטורת התלויות והן אי-שינוי ההתנהגות על קורפוס התיעוד.

Sequence diagram for backward-compatible RST parsing

sequenceDiagram
    participant Consumer as Existing consumer
    participant RST as rst_parser
    participant Shared as doc_sections

    Consumer->>RST: parse_document(...)
    RST->>Shared: construct Section and Document
    Shared-->>RST: shared model objects
    RST-->>Consumer: Document
    Consumer->>RST: build_toc / find_sections / section_text
    RST->>Shared: re-exported callable
    Shared-->>Consumer: section results
    Consumer->>RST: catch TooManySections
    RST-->>Consumer: same shared exception class
Loading

File-Level Changes

Change Details Files
חילוץ מודל הסעיפים והלוגיקה הבלתי תלויה בפארסר למודול משותף, תוך שמירת תאימות לייבואי RST קיימים.
  • העברת מבני הנתונים, חישוב ההיררכיה, חיפוש, חיתוך טקסט, TOC, שכנים והצעות למודול עצמאי בעל ייבואי תקן בלבד.
  • החלפת ההגדרות ב-rst_parser בייבוא מחדש מפורש באמצעות all, תוך שמירת זהות האובייקטים והחריגה TooManySections.
  • השארת rst_parser אחראי רק לזיהוי תחביר RST ולבניית Document.
services/doc_sections.py
services/rst_parser.py
הוספת כיסוי בדיקות מבני והתנהגותי לחוזה המשותף ולגבולות התלות.
  • הוספת בדיקות AST לווידוא הגדרה יחידה, זהות ייצוא מחדש, היעדר תלות בפארסרים וב-MCP, ושרידות שמות הנדרשים לצרכן.
  • הוספת בדיקות ישירות לעץ סעיפים שנבנה ידנית, כולל גבולות, היררכיה, טקסט, שכנים, TOC, נרמול, התאמות והצעות.
tests/test_doc_sections.py
הוספת כלי השוואת אפס-דיף למדידת אי-שינוי התנהגותי על קורפוס RST קבוע.
  • הרצת סוללה דטרמיניסטית על כל קובצי ה-RST, כולל TOC, חיפוש סעיפים, שגיאות, הצעות, חיתוך ו-pagination.
  • שמירת השאילתות והתשובות ב-JSONL וחישוב digest להשוואה בין עצי קוד תוך בידוד backend וקורפוס.
scripts/docs_section_zero_diff.py
עדכון תיעוד מדידת שרשרת הייבוא של סורק ה-RST לאחר הוספת המודול המשותף.
  • תיעוד המודול החדש כתוספת יחידה ללא מודולים כבדים.
  • החלפת מספר כולל תלוי-סביבה באינווריאנטים יציבים של מודולים מותרים והיעדר תלויות כבדות.
mcp_server/outline_scanners/__init__.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai

coderabbitai Bot commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 17a830f4-51d8-44f5-82e4-8b19a0b4cb1b

📥 Commits

Reviewing files that changed from the base of the PR and between 06983a8 and 1734a29.

📒 Files selected for processing (5)
  • mcp_server/outline_scanners/__init__.py
  • scripts/docs_section_zero_diff.py
  • services/doc_sections.py
  • services/rst_parser.py
  • tests/test_doc_sections.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

המודול המשותף services.doc_sections מרכז את מודל הסעיפים והפעולות עליו. services.rst_parser מייצא מחדש את הממשק הקיים. נוסף כלי JSONL לבדיקת אפס-דיף, ונוספו בדיקות מבניות והתנהגותיות.

Changes

ריפקטור מודל סעיפי המסמכים

Layer / File(s) Summary
מודל סעיפים משותף
services/doc_sections.py, tests/test_doc_sections.py
נוספו Section, Document, TooManySections ופעולות לחישוב היררכיה, גבולות, ניווט, תוכן עניינים, נרמול וחיפוש כותרות. הבדיקות מאמתות את המבנה ואת ההתנהגות.
תאימות הייצוא של rst_parser
services/rst_parser.py
rst_parser משתמש במימוש המשותף ומייצא מחדש את אותם שמות. parse_document ממשיך להשתמש במודל ובחריגה שהועברו.
סוללת אפס-דיף לקורפוס RST
scripts/docs_section_zero_diff.py, mcp_server/outline_scanners/__init__.py
נוסף סקריפט שמפעיל מסלולי TOC, section, נרמול, שגיאות וטרוקציה. הוא כותב JSONL דטרמיניסטי ומחשב SHA-256. תיעוד ספירת המודולים עודכן כדי לכלול את services.doc_sections ולציין תלות בסביבה.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Refactor

Sequence Diagram(s)

sequenceDiagram
  participant main
  participant _CorpusBackend
  participant docs_handlers
  participant rst_parser
  participant output
  main->>_CorpusBackend: טעינת קובץ RST
  _CorpusBackend->>docs_handlers: get_file
  docs_handlers->>rst_parser: parse_document
  rst_parser-->>docs_handlers: נתוני סעיפים
  docs_handlers-->>main: תשובת section או TOC
  main->>output: כתיבת JSONL וחישוב SHA-256
Loading

Merge Risk: ⚪ Minimal · up to 1734a

The refactor preserves the existing parser interface and has no identified merge-blocking behavior change.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 61.54% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 39 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed כותרת ה-PR קצרה, ברורה ומתארת במדויק את השינוי המרכזי: חילוץ מודל הסעיפים המשותף ל-services/doc_sections.py.
Description check ✅ Passed תיאור ה-PR מקיף את מטרת השינוי, השינויים העיקריים, בדיקות יחידה ואינטגרציה, הוכחת zero-diff, סיכונים, קישורים ותוכנית rollback. רוב סעיפי התבנית מולאו באופן ברור ומספיק להערכת השינוי.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/extract-shared-model-doc-sections-hcjymr

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.

❤️ Share

כותרת עולה, ועץ נבנה,
סעיף מוצא גבול ושכן.
JSONL שומר כל מסלול,
Claude Code טווה קוד צלול.
CodeKeeper forever 💫

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

⏱️ Performance report

(No performance test durations collected. Mark tests with @pytest.mark.performance.)

@github-actions

Copy link
Copy Markdown
Contributor

📖 Documentation Preview

The documentation has been built successfully!

To view locally:

  1. Download the artifacts
  2. Extract the zip file
  3. Open index.html in your browser

@codecov

codecov Bot commented Sep 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.73913% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
services/doc_sections.py 96.62% 2 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@amirbiron
amirbiron merged commit dea1b9e into main Sep 16, 2026
31 checks passed
amirbiron added a commit that referenced this pull request Sep 16, 2026
…figuration, מספר תלוי-סביבה, ו-.pyc בגיט (#3395)

* chore(git): קובצי .pyc יוצאים מניהול גיט

אחד-עשר קבצי ``__pycache__/*.cpython-313.pyc`` נכנסו לגיט ב-8098b67e
(#3239) למרות ש-``.gitignore`` שורה 2 חוסם את התיקייה. ``git rm --cached``
מוציא אותם מהאינדקס ומשאיר אותם על הדיסק, וה-``.gitignore`` הקיים דואג
שהם לא יחזרו כ-untracked.

נבדק שאין תלות: כל עשרת האזכורים של ``__pycache__`` בריפו הם החרגות בלבד
— ``.ruff.toml``, ``bandit.yaml``, ``.yamllint.yaml``,
``services/code_indexer.py``, ``scripts/audit_config_definitions.py``,
``scripts/find_duplicates.py``, ``github_menu_handler.py``,
``repo_analyzer.py`` ושני טסטים. אף אחד מהם אינו קורא מהתיקייה.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P1AYmWhLGtpsr7sSdRZVD3

* test(doc_sections): התת-תהליך רץ עם -B ומחוץ לעץ המקור

``test_doc_sections_pulls_no_heavy_modules`` הריץ
``subprocess.run([sys.executable, "-c", code], cwd=_ROOT)`` — כלומר
תת-תהליך שכותב ``__pycache__`` לתוך ``services/``, וזה טסט שכותב לעץ
המקור. התקדים הנכון כבר קיים בריפו, ב-``tests/test_rst_parser.py``, ושם
ההערה מנמקת את שני הדגלים במפורש.

השורש היה שהייבוא נשען על ה-``cwd``: הוא עבד רק מפני שהתהליך רץ בשורש
הריפו. לכן ``-B`` לבדו לא הספיק — ``sys.path.insert`` עם נתיב מוחלט
מנתק את המדידה מעץ העבודה, ורק אז ``cwd=tmp_path`` אפשרי.

נמדד, ולא הונח. בקרה מבודדת בלי pytest, על ``services/__pycache__`` ריק:
הצורה הישנה כותבת שלושה קבצים (``__init__``, ``backoff_state``,
``doc_sections``), הצורה החדשה כותבת אפס, ותיקיית ה-tmp נשארת ריקה גם היא.

ומה שלא השתנה, כדי שלא ייקרא אחרת: בריצת pytest רגילה הדלתא הייתה אפס
גם קודם, כי האיסוף עצמו מייבא את אותם מודולים וכותב את אותם קבצים.
כלומר זו הפרה של מוסכמה ולא באג נצפה, והערך שלה הוא ביום שבו עץ המקור
יהיה לקריאה בלבד — כלל 8 ב-CLAUDE.md.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P1AYmWhLGtpsr7sSdRZVD3

* docs(configuration): הסרת הבלוק הכפול של Pooling ו-HTTP Clients

שלושת הסעיפים "Databases and Cache (Pooling/Timeouts)", "HTTP Clients"
ו-"שימוש ב-http_sync" הופיעו פעמיים בקובץ. שניהם נכנסו ב**קומיט אחד**,
171d8f6 (#1015, 23.10.2025), שהכניס את אותו בלוק פעם אחרי "Environment
variables" ופעם אחרי "Security" — לפניו הכותרות לא היו בקובץ כלל.

המחיר נמדד: מתוך 45 תשובות ``ambiguous_section`` שהכלי ``docs_get_section``
מחזיר על כל 208 קובצי ה-RST, 25 הגיעו מהקובץ הזה לבדו. הכפילות גם חצתה את
נרמול הכותרות — ``normalize_title`` מאחד מקפים, ולכן ``שימוש ב‑http_sync``
עם U+2011 ו-``שימוש ב-http_sync`` עם מקף ASCII התנגשו זה בזה.

**שני העותקים הושוו שורה-שורה לפני המחיקה, והם אינם זהים** — אבל אף
הבדל אינו הבדל בתוכן: פיסוק ב-``REDIS_URL``, ניסוח ב-``.. note::``
("לא להשתמש" מול "אל תשתמשו", גרשיים עבריים מול מרכאה), והמקף בכותרת.
נשאר העותק הראשון **מילה במילה**, כי הוא במקום הנכון מבנית — עם שאר
סעיפי הקונפיגורציה, והוא היחיד שמלווה ב-"שימוש ב-http_async" שאין לו
כפילות. שבע-עשרה מתוך תשע-עשרה השורות שנמחקו קיימות בו כלשונן, והשתיים
הנותרות הן שתי וריאציות הניסוח.

עוגנים — נבדק ולא הונח: ``autosectionlabel`` אינו מופעל ב-``conf.py``,
ולכן לכותרות אין לייבלים אוטומטיים. העוגן המפורש היחיד בקובץ הוא
``.. _config-error-signatures:``, שיושב הרבה מתחת לבלוק שנמחק, ואליו
מפנה ``docs/observability/log-aggregator.rst``. שאר ההפניות הנכנסות הן
``:doc:`` לעמוד כולו.

אימות: הספירה הורצה מחדש על כל 208 הקבצים — הקובץ הזה ירד מ-25 ל-**0**,
והסך הכול מ-45 ל-20 (הנותרים הם ``development/tools.rst`` ו-
``observability/query-performance-profiler.rst``, שלא נגענו בהם). ובנייה
מלאה של Sphinx עם ``-W --keep-going`` עברה באפס אזהרות.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P1AYmWhLGtpsr7sSdRZVD3

* docs(mcp): משקל הייבוא של הסורק מתואר, ולא נספר

ה-docstring של ``outline_scanners/__init__.py`` קבע ש-``from services
import rst_parser`` מושך "**ארבעה** מודולים מהריפו", ומנה אותם בשמם —
וסגר ב"מהריפו רק מודולים שרשומים כאן בשמם". שתי הטענות אינן נכונות
בסביבה שבה ``structlog`` מותקן.

נמדד בשתי סביבות באותו עץ:

- **בלי** ``structlog``: 73 מודולים, ומהריפו ארבעה — שרשרת ה-``services``.
- **עם** ``structlog``: 330 מודולים, ומהריפו שבעה — אותם ארבעה ועוד
  ``observability``, ``monitoring`` ו-``monitoring.error_signatures``.

השרשרת: ``services/__init__.py`` מייבא ``state`` מ-``backoff_state``,
ששורה 16 בו **מנסה** לייבא ``observability``; ו-``observability.py``
מייבא ``structlog`` בקשיחות בשורה 22 ומנסה ``monitoring.error_signatures``
בשורה 28. כלומר הזנב הזה תלוי במה שמותקן בסביבה, לא בשורת קוד — וזה
בדיוק המבחן שב-``docs/doc-authoring.rst``: מספר שיכול להשתנות בלי שאף
שורת קוד תשתנה אינו ערך שנאכף.

הניסוח החדש מתאר מה נטען ולמה — חלק קבוע וזנב מותנה — ואינו קובע מספר
ואינו סוגר רשימה. האינווריאנט הנושא נשאר, והוא זה שנמדד בשתי הסביבות:
**אפס מודולים כבדים**. ותוקן גם דיוק שני: הטסט שאוכף את האינווריאנט
מודד את ``services.doc_sections``, לא את ``rst_parser``, וזה נכתב עכשיו
כפער מוצהר במקום להשתמע ככיסוי.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P1AYmWhLGtpsr7sSdRZVD3

---------

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants