Repository navigation
Conversation
…tions מיושרת ל-MAX_SECTIONS (#3421, #3420) שני הפארסרים מתועדים כבני-החלפה, ועל קלט שאינו מחרוזת הם התנהגו אחרת: rst_parser.parse_document(None) החזיר מסמך ריק בשקט (מפה ריקה שמתחזה למפה של קובץ בלי כותרות), ו-17 נפל ב-AttributeError גולמי מתוך .replace, בעוד md_parser זרק TypeError שאומר מה התקבל. וברירת המחדל של max_sections הייתה None ב-rst_parser ו-MAX_SECTIONS ב-md_parser, כלומר קורא ששכח להעביר תקרה קיבל הגנה מהפארסר האחד ולא מהשני. - #3421: בדיקת כניסה אחת לשניהם, doc_sections.require_str, שמרימה TypeError עם אותן מילים ("parse_document expects str, got NoneType"). אף קורא לא נשען על הצורה הישנה: המטפל, הסורק והסקריפטים מעבירים תמיד מחרוזת. docs_get_section אינו תופס את החריגה, כמו שלא תפס אותה מ-md_parser: "חוזה נשבר" ולא "הקלט נדחה", ו-content שם תמיד מחרוזת. - #3420, ההכרעה: יישור. MAX_SECTIONS עובר ל-services.doc_sections (המודול המשותף), מיוצא מחדש משני הפארסרים, והוא ברירת המחדל בשניהם. נמדד לפני השינוי על כל 208 קובצי ה-RST ב-docs/: 1,384 סקשנים בסך הכול, הקובץ העשיר ביותר (docs/mcp-server.rst) נושא 50 מול תקרה של 50,000, אחד לאלף, ואף קובץ אינו נחסם. אפס-דיף על docs_get_section: 5,931 רשומות זהות בית-בית לפני ואחרי על אותו קורפוס. - בעקבות היישור docs_get_section אינו מעביר תקרה לאף פארסר (עד עכשיו העביר ל-rst_parser את _ceiling.MAX_SYMBOLS במפורש, כי ברירת המחדל שם הייתה None), ו-"max" בסירוב הוא doc_sections.MAX_SECTIONS, המספר שהפרסר באמת השתמש בו. הייבוא של _ceiling מהמטפל הוסר. - outline_scanners/rst.py ממשיך להעביר את _ceiling.MAX_SYMBOLS במפורש (המספר של המפה, כותרות ותוויות יחד), וטסט חדש מקבע זאת: מוטציה שמוחקת את הארגומנט מפילה אותו. - scripts/measure_md_parse_cost.py: שלוש הצורות רצות עכשיו על ברירת המחדל בשני הפרסרים, כמו הכלי; מתועד בדוקסטרינג, והסקריפט רץ מקצה לקצה. טסטים: 12 טסטים חדשים או שנערכו נופלים על origin/main; הפין של הסורק עובר שם בכוונה. שלוש מוטציות ב-worktree (הסורק בלי הארגומנט, המטפל שמעביר תקרה שוב, require_str שממיר None ל-"") מפילות כל אחת את הטסט שלה. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SfJTSpDAhDr2yhtmpFkwTx
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @amirbiron, you've used your own review budget of 250,000 diff characters for the last 7 days.
You can request another review in 6 hours and 5 minutes by commenting @sourcery-ai review. Upgrade to get a review now.
Contributor
|
Warning Review limit reachedNext included review available in 3 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (13)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
🧯 Dangerous deletes guard reportPolicy: see .cursorrules — dangerous deletions are blocked unless wrapped safely. Summary:
Flagged findings (file:line:snippet): Excluded matches (by path pattern) |
Contributor
Reviewer's GuideThe PR makes RST and Markdown parser contracts consistent by sharing strict string validation and a fail-closed MAX_SECTIONS default, while removing redundant ceiling forwarding from the document handler, preserving the outline scanner’s explicit limit, and updating tests and documentation to verify compatibility. Sequence diagram for shared parser validation and section limitssequenceDiagram
participant Caller
participant DocsHandler as docs_get_section
participant Parser
participant DocSections as doc_sections
Caller->>DocsHandler: docs_get_section(...)
DocsHandler->>Parser: parse_document(content)
Parser->>DocSections: require_str(text)
alt text is not str
DocSections-->>Parser: TypeError
Parser-->>DocsHandler: TypeError
else text is str
Parser->>Parser: apply MAX_SECTIONS default
alt section limit exceeded
Parser-->>DocsHandler: TooManySections
DocsHandler-->>Caller: error: too_many_sections, max=MAX_SECTIONS
else within limit
Parser-->>DocsHandler: Document
DocsHandler-->>Caller: section response
end
end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
⏱️ Performance report(No performance test durations collected. Mark tests with |
Contributor
📖 Documentation PreviewThe documentation has been built successfully!
To view locally:
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This was referenced Sep 20, 2026
Closed
Owner
Author
|
נכנס דרך #3443 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
✨ תיאור קצר
services.rst_parser,services.md_parser) מתועדים כבני-החלפה, ועל קלט שאינו מחרוזת הם התנהגו אחרת:rst_parser.parse_document(None)החזיר מסמך ריק בשקט (מפה ריקה שמתחזה למפה של קובץ בלי כותרות), ו-17נפל ב-AttributeErrorגולמי, בעודmd_parserזרקTypeErrorשאומר מה התקבל (יישור בדיקת הכניסה של rst_parser.parse_document לקלט שאינו מחרוזת #3421). וברירת המחדל שלmax_sectionsהייתהNoneבאחד ו-MAX_SECTIONSבשני, כלומר קורא ששכח להעביר תקרה קיבל הגנה מהפארסר האחד ולא מהשני (refactor: לשקול יישור ברירת המחדל של max_sections ב-rst_parser.parse_document #3420). ה-PR מיישר את שניהם: בדיקת כניסה אחת משותפת, וברירת מחדל אחת משותפת — אחרי מדידה על כל 208 קובצי ה-RST ואפס-דיף מוכח עלcodekeeper_docs_get_section.claude/gracious-einstein-sevk8pנושא את ה-PR הפתוח של docs_get_section: תקרה ל-candidates, ושמירת חוזה Suggestions — לפני שחיבור ה-Markdown נוחת #3426 (fix(mcp): תקרה לרשימת המועמדים של ambiguous_section, וטסט שמקבע את חוזה Suggestions במסלול ה-Markdown (#3426) #3434), ו-PR נפרד דורש ענף נפרד. ה-PR הזה יושב על ענף נגזר,claude/gracious-einstein-sevk8p-3421, שנפתח מ-origin/mainהעדכני (1a45c28c).📦 שינויים עיקריים
פירוט נקודות (רשימת תבליטים):
doc_sections.require_str, שמרימהTypeErrorעם אותן מילים בדיוק (parse_document expects str, got NoneType). לא שתי שורות זהות בשני מודולים — זה מה שנסחף בתיקון הבא (R6). ב-rst_parserהוסר ה-(text or "")שהפךNoneלמסמך ריק בשקט. מי נשען על הצורה הישנה — נבדק, אף אחד:docs_handlersמעבירcontent = res.get("content") or ""(תמיד מחרוזת), הסורק מעביר טקסט מפוענח, והסקריפטים קוראים קבצים. ותשובה לשאלה השנייה באישו:docs_get_sectionאינו ממיר את ה-TypeErrorלתשובתerror— בדיוק כפי שלא המיר אותה מ-md_parserעד היום: זה "חוזה נשבר" ולא "הקלט נדחה", ועטיפה הייתהwidened-exception-scope. ההערה בקוד מעודכנת._declares_write,_ceiling.py), שני הצרכנים בייצור כבר העבירו תקרה ולכן ברירת המחדל משפיעה רק על מי ששכח, והמדידה הראתה שהמחיר על קלט אמיתי הוא אפס.MAX_SECTIONSעבר ל-services.doc_sections(המודול המשותף שכבר מחזיק אתTooManySections), מיוצא מחדש משני הפארסרים (אותו אובייקט), והוא ברירת המחדל בשניהם;Noneמכבה במפורש.docs/, לפני השינוי, נספר כמו שהתקרה סופרת): 1,384 סקשנים בסך הכול; הקובץ העשיר ביותר,docs/mcp-server.rst, נושא 50 מול תקרה של 50,000 — אחד לאלף; ואף קובץ אינו נחסם תחתMAX_SECTIONS. פרסור כולם: 0.05 שניות.docs_get_section:scripts/docs_section_zero_diff.pyעל אותו קורפוס (--corpusמצביע לאותו עץ לשתי ההרצות) — הקוד הישן ב-worktree מנותק עלorigin/mainוהקוד החדש: 5,931 רשומות,sha256זהה (9f12d0ed…5cba9),cmpבייט-בייט זהה. (הרצה ראשונה הראתה 196 רשומות שונות — כי ערכתי אתdocs/mcp-server.rstו-whats-new.rstבין שתי ההרצות, והקורפוס הוא הקבצים החיים; ההרצה החוזרת של הישן על הקורפוס העדכני זהה. מציין את זה כי זה בדיוק מה שהדוקסטרינג של הסקריפט מזהיר מפניו.)mcp_server/docs_handlers.py: הכלי אינו מעביר תקרה לאף פארסר —parse_kwargsוה-parser is rst_parserנעלמו (ההערה שם נימקה את ההעברה המפורשת ב"ברירת המחדל שלrst_parserהיאNone", וזה כבר לא נכון; והנימוק שלה עצמה נגד העברה ל-Markdown — "עותק שני של החלטה שהפארסר כבר הכריע" — חל עכשיו על שניהם)."max"בסירוב הואdoc_sections.MAX_SECTIONS, המספר שהפרסר באמת השתמש בו, ולא_ceiling.MAX_SYMBOLS("נכון ל-Markdown רק מפני שהטסט קושר"). הייבוא של_ceilingמהמטפל הוסר.mcp_server/outline_scanners/rst.pyממשיך להעבירmax_sections=_ceiling.MAX_SYMBOLSבמפורש (המספר של המפה — כותרות ותוויות יחד,Capped), והדוקסטרינג אומר למה גם כשהמספרים שווים. הטסט החדש ב-test_mcp_outline.pyמרגל על הקריאה ומקבע את הארגומנט.scripts/measure_md_parse_cost.py: שלוש הצורות רצות עכשיו על ברירת המחדל בשני הפרסרים — כלומר בדיוק כמו הכלי; הצורה העוינת של RST נעצרת על התקרה (outcome: too_many_sections, 19.9MiB, מול 44.0MiB בלי תקרה). מתועד בדוקסטרינג, והסקריפט הורץ מקצה לקצה (10 שניות)... important::בשניהם שהצהיר "ברירת המחדל כאן הפוכה"), ההערה ב-_ceiling.pyעל העותק השני,docs/mcp-server.rst(הפסקה שלtoo_many_sections), ו-docs/whats-new.rst. ההיסטוריה נשארת כתובה ליד הקבוע — למה זה היהNone, ולמה זה השתנה.🧪 בדיקות
tests/test_rst_parser.py:None/17/b"x"→TypeErrorשנוקב בטיפוס (matchעל שם הטיפוס, כיbytesנפל גם קודם ב-TypeErrorאחר מתוך.replace); שני הפארסרים מרימים את אותן מילים; ברירת המחדל היא אותו אובייקטdoc_sections.MAX_SECTIONSבשניהם (טענת O(1), כמו הטסט המקביל ב-test_md_parser— לא 50,001 סקשנים בכל CI),"MAX_SECTIONS"ב-__all__, ו-Noneמכבה.test_the_two_parsers_export_the_same_names_but_two←..._but_one(ההפרש עכשיו{"InconsistentLineEndings"}בלבד);test_the_default_ceiling_is_the_documented_constantמקבע שזה ייצוא-מחדש ולא עותק; ב-test_mcp_docs_handlers.pyהטסט שקיבע את ההעברה המפורשת ל-RST הפך ל-test_the_rst_reader_is_capped_by_the_parser_default_and_refuses_above_it(הגלגול השלישי שלו — fix: תקרת הסימבולים נוסעת לתוך פרסור ה-RST במקום לסנן את הפלט #3378 ← fix(mcp): מאגר הקריאות נגזר ממכסת הזיכרון של הקונטיינר, לא ממעבדי המארח (#3391) #3429 ← refactor: לשקול יישור ברירת המחדל של max_sections ב-rst_parser.parse_document #3420, מתועד בדוקסטרינג):passed == {}, ברירת המחדל היא הקבוע, סירוב מעל התקרה דרךfunctools.partialעל הפרסר,"max"הוא הקבוע;test_the_two_refusals_are_mapped_whichever_parser_raised_themחזר ל-partial(עכשיו זה עובד, כי הכלי אינו מעביר ארגומנט שדורס);test_the_two_section_ceilings_are_the_same_numberמשווה_ceiling.MAX_SYMBOLSל-doc_sections.MAX_SECTIONSומקבע ששלושת השמות הם אותו אובייקט.tests/test_mcp_outline.py:test_the_rst_scanner_passes_its_own_ceiling_and_does_not_lean_on_the_parser_default— פין, לפי דרישת refactor: לשקול יישור ברירת המחדל של max_sections ב-rst_parser.parse_document #3420 ("בטסט ולא בהיגיון").origin/main=1a45c28c, עם קובצי הטסטים החדשים): 12 נופלים (שלושת סוגי הקלט × שני הטסטים, ברירת המחדל, הייצוא, הקבוע המשותף, שלושת טסטי המטפל), ואחד עובר — הפין של הסורק, בכוונה ומתועד.test_the_rst_reader_is_capped_by_the_parser_default…נופל;require_strשמחזירtext or ""← שלושת טסטי הכניסה נופלים.test_mcp_to_thread,test_mcp_server_build,test_mcp_docs_handlers,test_mcp_outline,test_mcp_repo_backend,test_mcp_primer,test_rst_parser,test_md_parser,test_md_parser_oracle,test_doc_sections,test_docs_headings_carry_no_identifier,test_mcp_logging_visible,test_mcp_analytics_privacy— ירוקים. flake8 מלא (max-line-length=127) על כל הקבצים שנגעתי בהם — נקי. docutils על שני עמודי התיעוד — 0 אזהרות (בלי בנייה מלאה, לפיCLAUDE.md).:data:/:func:roles החדשים בדוקסטרינגים מצביעים ל-services.doc_sections, ובדקתי שאיןnitpickyב-conf.pyואיןautomoduleלשלושת המודולים, כלומר הפניה שלא תיפתר אינה מפילה את הבילד.🧪 בדיקות נדרשות ב‑PR
📝 סוג שינוי
✅ צ'קליסט
docs/whats-new.rstעודכןAI-MAP.md,docs/mcp-server.rst("שני סירובים שמגיעים מהפרסור עצמו"),docs/development/scripts.rst(הסקריפט שמודד את עלות הפרסור),docs/doc-authoring.rst,docs/versioning-stable-anchors.rst| המשפט: "ו-RST דרך התקרה שהכלי מעביר במפורש (_ceiling.MAX_SYMBOLS, מאז fix(mcp): מאגר הקריאות נגזר ממכסת הזיכרון של הקונטיינר, לא ממעבדי המארח (#3391) #3429), כי ברירת המחדל שלrst_parserהיא ללא תקרה" — הפסקה שהשתנתהדפוסי באגים שנקראו ומה שקבעו בקוד:
CORE-PATTERNS.mdU3 +bugbot-rules/external-input-isinstance.md—isinstance(text, str)לפני.replaceעל ערך שמגיע מחוץ לתהליך, וזה כל #3421;bugbot-rules/silent-fallback-to-worse-path.md— ה-(text or "")היה "המחבוא הנפוץ": תוצאה ריקה במקום חריגה, ש"לא נמצא" ו"נשבר" נראים בה זהים — הוסר;RECURRING-PATTERNS.mdR6 —grepעל "expects str" לפני הכתיבה: עותק אחד ב-md_parser← אוחד ל-require_str, ו-MAX_SECTIONSעבר למודול המשותף במקום להיות מוקלד פעם שלישית;bugbot-rules/state-record-without-state-change.md— כל תיאור של ברירת המחדל (שלושה דוקסטרינגים, הערה ב-_ceiling, שני עמודי תיעוד, הערת המטפל) עודכן יחד עם השינוי, כדי שלא תישאר רשומה שמתארת מצב שאינו קיים;bugbot-rules/line-number-coupling.md— בלי מספרי שורות בתיעוד;claude-md-snippets/testing.md§5 +TESTING-PATTERNS.mdT2 — הטסטים הורצו על הקוד הישן ונפלו, והמוטציות מתועדות;bugbot-rules/widened-exception-scope.md— לא הורחב אףexcept, וה-TypeErrorנשאר לא-נתפס במטפל בכוונה.🧩 השפעות/סיכונים
rst_parser.parse_documentשמעביר ערך שאינו מחרוזת מקבלTypeErrorבמקום מסמך ריק; קורא שאינו מעביר תקרה מקבל תקרה של 50,000 סקשנים. בקוד הריפו אין אף קורא משני הסוגים (נבדק: המטפל, הסורק, הסקריפטים והטסטים), ואפס-הדיף מוכיח שהכלי הציבורי מחזיר בדיוק אותן תשובות._ceiling.MAX_SYMBOLSומה שהסורק מעביר,md_parser(אותה בדיקה, אותה ברירת מחדל, רק דרך המודול המשותף), ותשובותdocs_get_sectionעל כל 208 העמודים.🔗 קישורים
docs/mcp-server.rst— "שני סירובים שמגיעים מהפרסור עצמו"🧯 סיכון / החזרה לאחור (Rollback)
🤖 Generated with Claude Code
https://claude.ai/code/session_01SfJTSpDAhDr2yhtmpFkwTx
Generated by Claude Code