Skip to content

fix(webapp): מקור אמת אחד לגודל קובץ, והערך נקרא בסדר לטיני גם בעברית - #3345

Merged
amirbiron merged 3 commits into
mainfrom
claude/file-size-no-trailing-zero
Sep 7, 2026
Merged

fix(webapp): מקור אמת אחד לגודל קובץ, והערך נקרא בסדר לטיני גם בעברית#3345
amirbiron merged 3 commits into
mainfrom
claude/file-size-no-trailing-zero

Conversation

@amirbiron

@amirbironamirbiron commented Sep 7, 2026

Copy link
Copy Markdown
Owner

✨ תיאור קצר

כרטיס קובץ הציג 105.0 KB — תו נוסף בלי מידע נוסף. בדרך לתיקון התברר ששני דברים גדולים יותר שבורים מתחתיו: הכלל היה משוכפל בשמונה מקומות שנסחפו זה מזה, והמחרוזת עצמה מתהפכת על המסך בתוך טקסט בעברית.

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

  • קוד (Backend)
  • קוד (Frontend / JS)

1. שישה מימושים בצד הלקוח אוחדו לאחד

compare.js, global_search.js, live-preview.js, json-formatter.js, repo-browser.js ו-file-form-manager.js החזיקו כל אחד מימוש משלו. הם נבדלו בתקרת היחידות, ברווח לפני היחידה ובעיגול — והפיצול הזה ייצר שלושה באגים אמיתיים, שתוקנו בעצם האיחוד:

איפההיוםאחרי
live-preview — חילק ב-1024 פעם אחת בלבד3584KB / 0.6KB3.5 MB / 582 B
global_search — עצר ב-MB2048 MB2 GB
repo-browser — עצר ב-GB3 undefined3 TB

הכלל יושב עכשיו ב-webapp/static/js/utils/size-format.js ונטען משורה אחת ב-base.html, בדיוק כמו utils/animations.js ו-utils/rtl-code.js הקיימים. כל שבע התבניות שטוענות את הצרכנים יורשות מ-base.html, ולכן לא נדרשה עבודת חוטים בתבניות.

2. הערך נקרא בסדר לטיני בכל הקשר

"105 KB" בתוך כרטיס בעברית התהפך על המסך ל-KB 105. אלגוריתם ה-bidi אינו מדביק מספר למילה לטינית דרך רווח ניטרלי: הרווח נופל לכיוון הפסקה, המחרוזת נשברת לשני קטעים, והם מסודרים מימין ← לשמאל.

נמדד ב-Chromium, מיקומי ציור בפועל:

x של 105x של KB
בלי בידוד350325היחידה משמאל למספר
עם בידוד325359תקין

הערך עטוף עכשיו ב-LRI/PDI (U+2066/U+2069) בתוך המחרוזת עצמה, ולא ב-dir="ltr" על האלמנט. הסיבה מדידה ולא סגנונית: לא לכל גודל יש אלמנט משלו.webapp/app.py בונה "הקובץ גדול מדי לתצוגה (105 KB)" כמחרוזת אחת, file-form-manager.js בונה "x גדול מדי (מקסימום 2 MB)", ו-live-preview.js מצרף שלושה שדות ל-textContent יחיד. שם אין על מה לתלות dir, ולכן בידוד בתוך הערך הוא הפתרון היחיד שמכסה את כולם. תווי הבידוד אינם מוסיפים רוחב (נמדד: הפרש 0.02px).

המשמעות לקוראי הערך:"105 KB" in result עדיין עובד; השוואת שוויון מדויקת דורשת את העטיפה.

3. השרת והדפדפן לא הסכימו על 1280 בתים

f"{1.25:.1f}" בפייתון מעגל חצי לזוגי ומחזיר "1.2". (1.25).toFixed(1) ב-JS מעגל חצי כלפי מעלה ומחזיר "1.3". כלומר קובץ של 1280 בתים — גודל אמיתי לגמרי — הוצג 1.2 KB מהשרת ו-1.3 KB מהדפדפן.

זה נתפס בבדיקה שמריצה את שני המימושים ומשווה, לא בקריאת הקוד. שני הקבצים נראים זהים לחלוטין; הם פשוט נשענו כל אחד על ברירת המחדל של שפתו. העיגול כתוב עכשיו במפורש בשתי השפות באותן פעולות על אותם doubles.

מה לא שונה, בכוונה

TextUtils.format_file_size ב-utils.py מזין את הודעות הבוט בטלגרם ולא את הוובאפ, ושני טסטים מתעדים שם את ההתנהגות הקיימת ("1.0 KB"). המשמעות: אותו קובץ יוצג 1.3 KB בוובאפ ו-1.2 KB בבוט. זו החלטת מוצר נפרדת ולא משהו שנכנס בשקט לתוך תיקון בוובאפ — מסומן כהמשך.

🧪 בדיקות

  • tests/size-format.test.js (חדש) — סולם היחידות, העיגול, חיתוך ה-.0 והבידוד. רץ בג'וב JS Tests (node) הקיים. 19 בדיקות.
  • tests/test_file_size_display.py — צד השרת על כל צרכניו, ובדיקה שמריצה את size-format.js האמיתי ב-node ומשווה ערך מול ערך. זו הבדיקה שתפסה את פער העיגול. 19 בדיקות.
  • tests/test_size_format_bidi_browser.py (חדש) — סדר הציור בדפדפן אמיתי, על מה שהשרת שולח ועל מה שהדפדפן מחשב. סדר bidi הוא משהו שהדפדפן אוכף ו-Python אינו רואה, ולכן בדיקת יחידה כאן אינה ראיה.
  • ריצת בקרה בתוך בדיקת הדפדפן — היא דורשת שמחרוזת ללא בידוד כן תתהפך. בלי זה שינוי עתידי היה יכול להפוך את הבדיקה לחסרת משמעות בשקט, והיא הייתה ממשיכה לעבור בלי לבדוק כלום.
  • הבדיקות החדשות הורצו על הקוד שלפני התיקון ונפלו — בדיקת הדפדפן על from-server, ו-16 מ-19 בבדיקת ה-JS. שחזור החזיר את כולן לירוק.
  • אימות אינטגרציה באפליקציה האמיתית — הורמה עם werkzeug והועלה /tools/json בכרומיום: status=200, document.documentElement.dir=rtl, window.SizeFormat נטען, formatFileSize(107520) החזיר את הערך המבודד, והוא צויר בסדר הנכון (105 ב-x=1166, KB ב-x=1197). אפס שגיאות JS בעמוד.
  • סיכון סדר הטעינה נבדק ישירות — הצרכנים compare.js ו-repo-browser.js נטענים בליdefer, והמודול המשותף עםdefer. נמדד: ברגע ה-parse של הצרכן window.SizeFormat באמת עדיין undefined, ובזמן הקריאה הוא כבר קיים והפונקציה עובדת — כי כל שש הקריאות יושבות בתוך פונקציות ולא ברמת המודול.
  • כל 12 קובצי ה-*.test.js של הריפו הורצו כפי ש-CI מריץ אותם — ירוקים.

📝 סוג שינוי

  • fix

✅ צ'קליסט

  • בדיקות רצות ועוברות
  • לא נוספו/שונו משתני סביבה
  • אין סודות/מפתחות בקוד
  • אין מחיקות מסוכנות
  • הודעת הקומיט תואמת Conventional Commits
  • עיינתי במסמכי אתר התיעוד — נתיב: AI-MAP.md | המשפט: grep על format_file_size/formatBytes/humanSize בכל docs/ ו-AI-MAP.md לא החזיר דבר — אין עמוד שמתעד את עיצוב גדלי הקבצים, ולכן לא נדרש עדכון תיעוד. הכלל וההנמקה של הבידוד מתועדים ב-docstring של webapp/size_format.py ובראש size-format.js.

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

  • שינוי נראה לעין בשני מקומות, לבקשת בעל הריפו: היחידה עוברת לצד הלטיני שלה בכרטיס הקבצים, ו-live-preview מקבל רווח ויחידה נכונה (3.5 MB במקום 3584KB).
  • תווי בידוד במחרוזת — בדקתי את כל הבדיקות שמשוות מחרוזות גודל: אין אף השוואת שוויון מדויקת על ערך שמגיע מ-API. הבדיקה הקיימת שמרנדרת את /files משתמשת ב-in, והיא עוברת.
  • אין שינוי בערכים שנשמרים, בשאילתות או בחוזה ה-API — רק במחרוזת שמוצגת.

🔗 קישורים

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

  • git revert לשני הקומיטים. הם עומדים בפני עצמם ואינם נוגעים בשום דבר אחר.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Y251xcBYzeYZUimEMKBQsS

Summary by Sourcery

Standardize webapp file-size display and ensure formatted values remain consistent and readable across Hebrew interfaces.

Bug Fixes:

  • Unify webapp file-size formatting across server and client consumers to correct unit selection, redundant decimals, rounding differences, and malformed large-size output.
  • Preserve left-to-right reading order for file-size values embedded in right-to-left Hebrew text.
  • Fix synchronous page initialization so file-size formatting is available before dependent scripts execute.

Enhancements:

  • Introduce a shared server/client formatting contract with matching rounding, unit limits, invalid-value handling, and bidirectional isolation.

Documentation:

  • Add a What's New entry for the file-size display improvements.

Tests:

  • Add JavaScript, server/client parity, browser bidi-order, and consumer loading-order coverage for file-size formatting.

כרטיס קובץ הציג ‎105.0 KB‎ ו-‎582.0 B‎: תו נוסף בלי מידע נוסף. הסיבה היא
‎:.1f‎ בפורמטרים, שמדפיס ספרה אחרי הנקודה גם כשהיא אפס. שבר אמיתי נשאר —
‎27.9 KB‎ ממשיך להיות ‎27.9 KB‎ — כי שם הספרה כן אומרת משהו.
הכלל היה משוכפל בין webapp/app.py ל-webapp/collections_api.py, ושני
העותקים יכלו להיסחף זה מזה. במקום להוסיף עותק שלישי של החיתוך נוצר
webapp/size_format.py — מודול עלה בלי תלויות, ולכן אפשר לייבא אותו מכל
מקום בלי מעגל ייבוא — ושתי הפונקציות מאצילות אליו. השם format_file_size
נשאר ב-app.py כי הוא נקרא משם בעשרות מקומות ומהתבניות.
בצד הלקוח אותו כלל בשלושה מקומות שהציגו ‎.0‎: compare.js, global_search.js
ו-live-preview.js. הם עברו ל-parseFloat סביב toFixed(1) — האידיום שכבר
היה בשימוש ב-json-formatter.js וב-repo-browser.js, כך שכל חמשת הפורמטרים
בצד הלקוח מתנהגים עכשיו אותו דבר.
TextUtils.format_file_size ב-utils.py **לא** שונה בכוונה: הוא מזין את
הודעות הבוט ולא את הוובאפ, ושני טסטים מתעדים שם את ההתנהגות הקיימת
("1.0 KB"). שינוי שלו הוא החלטה נפרדת.
אימות:
- הפורמטר עצמו: 0 B, 3 B, 582 B, 4 KB, 105 KB, 1 MB, 5 GB, 2 TB, ולצידם
27.9 KB ו-1.5 MB ששומרים את השבר.
- **ברמת הצרכן**: הטסט הקיים שמרנדר את /files דרך Flask ובודק את המחרוזת
בכרטיס עודכן מ-"3.0 B" ל-"3 B" ורץ ירוק.
- שלושת הפורמטרים בצד הלקוח הורצו ב-node מתוך הקבצים האמיתיים שנשלחים
לדפדפן (חילוץ בביטוי רגולרי מהמקור, לא העתק שנכתב ביד): 582 B, 4 KB,
27.9 KB, 105 KB.
- מוטציה: החזרת השורה הישנה הפילה 12 בדיקות, ובהן בדיקת ה-HTML של /files.
שחזור החזיר את כולן לירוק.
- לא פתחתי דפדפן: פיקסצ'ר admin_live_server מרים את הוובאפ בלי מסד נתונים,
ולכן /files אינו מרנדר שם כרטיסי קבצים בכלל. מסלול ה-DOM לא השתנה —
אותה משבצת בתבנית ואותו textContent — ומה שכן השתנה, המחרוזת עצמה, מוכח
ברמת ה-HTML וב-node.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y251xcBYzeYZUimEMKBQsS
@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.

@sourcery-aisourcery-aiBot 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 2 hours and 13 minutes by commenting @sourcery-ai review. Upgrade to get a review now.

@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

Copy link
Copy Markdown
Contributor

Reviewer's Guide

The PR removes redundant .0 suffixes from webapp file-size displays by introducing a shared backend formatter, delegating existing Python consumers to it, and applying the established parseFloat(toFixed(1)) pattern to three JavaScript formatters. Tests cover shared and consumer behavior, rendered HTML, JavaScript outputs, and preserved collection contracts; bot-message formatting remains intentionally unchanged.

Flow diagram for normalized file-size formatting

flowchart TD
Input[File size in bytes] --> Convert[Convert to display units]
Convert --> Round[Round to one decimal place]
Round --> Check{Ends with .0?}
Check -->|Yes| Trim[Remove .0]
Check -->|No| Keep[Keep decimal digit]
Trim --> Output[Display value with unit]
Keep --> Output
Loading

File-Level Changes

ChangeDetailsFiles
Centralize webapp file-size formatting and remove insignificant trailing zeroes.
  • Add a dependency-free formatter with shared unit selection and one-decimal trimming.
  • Delegate the existing app and collection formatters to the shared implementation while preserving public names and None/non-numeric handling.
  • Add unit and consumer-level coverage for integer sizes, meaningful fractions, rounding, unit limits, and legacy contracts.
  • Update the rendered files-page expectation from 3.0 B to 3 B.
webapp/size_format.py
webapp/app.py
webapp/collections_api.py
tests/test_file_size_display.py
tests/test_webapp_files_aggregate_allow_disk_use.py
Normalize client-side file-size displays consistently across all affected formatters.
  • Wrap toFixed(1) results with parseFloat in comparison, global search, and live preview displays.
  • Preserve meaningful fractional values while rendering integral values without .0.
  • Leave the existing formatter implementations that already use this idiom unchanged.
webapp/static/js/compare.js
webapp/static/js/global_search.js
webapp/static/js/live-preview.js

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

amirbiron pushed a commit that referenced this pull request Sep 7, 2026
השינוי עבר ל-PR ממוקד משלו (#3345) מעל main, כדי שיהיה בר-סקירה בפני עצמו
ולא ייבלע בתוך סבב התיעוד של #3297.
זה ביטול על הענף בלבד ולא ויתור על השינוי: הקוד חי ב-#3345 עם אותם
אימותים. שני סדרי המיזוג בטוחים — אם #3345 ייכנס ראשון, ההוספה והביטול
כאן מבטלים זה את זה ולא יגעו בקבצים; ואם הענף הזה ייכנס ראשון, השינוי
פשוט יגיע מאוחר יותר מ-#3345.
This reverts commit d07d83f.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y251xcBYzeYZUimEMKBQsS
@coderabbitai

coderabbitaiBot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

Next included review available in 27 minutes.

Check out review usage here.

View limit details

Limit 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.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: a05c37e7-fda9-4d5d-aff3-aab83f9d4e45

📥 Commits

Reviewing files that changed from the base of the PR and between 92c4079 and a5d94da.

📒 Files selected for processing (5)
  • docs/whats-new.rst
  • tests/test_size_format_bidi_browser.py
  • tests/test_size_format_consumers_browser.py
  • webapp/static/js/utils/size-format.js
  • webapp/templates/base.html

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: cbcac0fa-893a-43eb-929a-085f3092fe43

📥 Commits

Reviewing files that changed from the base of the PR and between 7de6d69 and 92c4079.

📒 Files selected for processing (12)
  • tests/size-format.test.js
  • tests/test_file_size_display.py
  • tests/test_size_format_bidi_browser.py
  • webapp/size_format.py
  • webapp/static/js/compare.js
  • webapp/static/js/file-form-manager.js
  • webapp/static/js/global_search.js
  • webapp/static/js/json-formatter.js
  • webapp/static/js/live-preview.js
  • webapp/static/js/repo-browser.js
  • webapp/static/js/utils/size-format.js
  • webapp/templates/base.html

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


📝 Walkthrough

Walkthrough

השינוי מרכז את עיצוב גדלי הקבצים במימושי Python ו-JavaScript משותפים. הפורמט מטפל ביחידות B עד TB, בעיגול חצאים, בערכים לא-סופיים ובסדר bidi. נוספו בדיקות Node, Python ו-Chromium.

Changes

עיצוב פורמט משותף

Layer / File(s)Summary
מימוש פורמט משותף
webapp/size_format.py, webapp/static/js/utils/size-format.js
המימושים מעצבים גדלים ביחידות B עד TB, מטפלים בערכים לא-סופיים, מעגלים חצאים כלפי מעלה ועוטפים את הפלט ב-LRI/PDI.
חיבור צרכני הפורמט
webapp/app.py, webapp/collections_api.py, webapp/static/js/*.js, webapp/templates/base.html
צרכני Python ו-JavaScript משתמשים בפורמט המשותף. base.html טוען את SizeFormat לפני השימוש בו.
בדיקות תאימות ותצוגה
tests/size-format.test.js, tests/test_file_size_display.py, tests/test_size_format_bidi_browser.py, tests/test_webapp_files_aggregate_allow_disk_use.py
נוספו בדיקות יחידות, parity בין Python ל-JavaScript, בדיקות bidi בדפדפן ועדכון אסרציה ל-"3 B". Claude Code הרחיב את כיסוי הבדיקות באופן עקבי.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk:🟡 Moderate · up to 92c40

The comparison page can fail when a file is selected because the shared formatter may not be loaded yet. Resolve the script ordering before merge.

Poem

גודל עובר בין Python ל-JavaScript
B עד TB נשמרים בפורמט אחיד
bidi נשאר בסדר הנכון
חצאים עולים לפי הכלל
Claude Code כתב בדיקות בקפידה
CodeKeeper forever 💫

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 56.76% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 37 functions across 13 files. (1 skipped:…Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Title check✅ Passedהכותרת קצרה, ברורה ומתארת את השינוי המרכזי: איחוד עיצוב גדלי קבצים ותיקון סדר התצוגה בהקשר עברי.
Description check✅ Passedתיאור ה-PR מקיף את מטרת השינוי, השינויים העיקריים, הבדיקות, ההשפעות, הסיכונים ותוכנית החזרה לאחור. חסר דיווח נפרד על תוצאות Required Checks, אך התיאור עדיין שלם וברור ברובו.
Full details: Docstring Coverage

Explanation

Docstring coverage is 56.76% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 37 functions across 13 files. (1 skipped: 1 unsupported.)

✨ 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/file-size-no-trailing-zero

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

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

@github-actions

github-actionsBot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

⏱️ Performance report

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

@github-actions

github-actionsBot commented Sep 7, 2026

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

@cubic-dev-aicubic-dev-aiBot 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.

All reported issues were addressed across 8 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment threadwebapp/static/js/live-preview.js Outdated
@codecov

codecovBot commented Sep 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

שישה מימושים של עיצוב גודל בצד הלקוח (compare, global_search, live-preview,
json-formatter, repo-browser, file-form-manager) אוחדו ל-utils/size-format.js
שנטען מ-base.html, ותאום שלו בפייתון כבר קיים ב-webapp/size_format.py.
האיחוד תיקן שלושה באגים שנבעו ישירות מהפיצול:
- live-preview חילק ב-1024 פעם אחת בלבד: 3.5 מגה הוצגו "3584KB", 582 בתים "0.6KB"
- global_search עצר ב-MB: 2 ג'יגה הוצגו "2048 MB"
- repo-browser עצר ב-GB: מעל טרה-בייט הוא החזיר "3 undefined"
בנוסף, שני תיקונים שנמצאו במדידה ולא בקריאת קוד:
1. "105 KB" בתוך טקסט בעברית התהפך על המסך ל-"KB 105". אלגוריתם ה-bidi אינו
מדביק מספר למילה לטינית דרך רווח ניטרלי — הרווח נופל לכיוון הפסקה והמחרוזת
נשברת לשני קטעים שמסודרים מימין לשמאל. נמדד ב-Chromium: KB ב-x=325 מול 105
ב-x=350. הערך עטוף עכשיו ב-LRI/PDI בתוך המחרוזת עצמה ולא ב-dir על האלמנט,
כי לא לכל גודל יש אלמנט משלו: app.py בונה "הקובץ גדול מדי לתצוגה (105 KB)"
ו-file-form-manager בונה "x גדול מדי (מקסימום 2 MB)" כמחרוזת אחת.
2. השרת והדפדפן לא הסכימו על 1280 בתים. f"{1.25:.1f}" מעגל חצי לזוגי ומחזיר
"1.2", ו-(1.25).toFixed(1) מעגל חצי כלפי מעלה ומחזיר "1.3". העיגול נכתב
עכשיו במפורש בשתי השפות באותן פעולות על אותם doubles, במקום להישען על
ברירת המחדל של אף אחת מהן.
בדיקות:
- tests/size-format.test.js — סולם היחידות, העיגול והבידוד (רץ בג'וב JS Tests)
- tests/test_file_size_display.py — צד השרת, ובדיקה שמריצה את שני המימושים
על אותו וקטור ומשווה. זו הבדיקה שתפסה את פער העיגול.
- tests/test_size_format_bidi_browser.py — סדר הציור בדפדפן אמיתי, כולל ריצת
בקרה שדורשת שהמחרוזת ללא בידוד כן תתהפך, אחרת הבדיקה מאבדת משמעות בשקט.
שתי הבדיקות החדשות הורצו על הקוד שלפני התיקון ונפלו.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y251xcBYzeYZUimEMKBQsS
@amirbironamirbiron changed the title fix(webapp): גודל קובץ בלי ‎".0"‎ מיותר — 105 KB במקום 105.0 KBfix(webapp): מקור אמת אחד לגודל קובץ, והערך נקרא בסדר לטיני גם בעבריתSep 7, 2026

@cubic-dev-aicubic-dev-aiBot 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.

2 issues found across 12 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="webapp/static/js/live-preview.js">
<violation number="1" location="webapp/static/js/live-preview.js:810">
P2: Custom agent: **Enforce Pragmatic Test Coverage**
The new live-preview delegation is not exercised by the added tests. Both JS test harnesses call `SizeFormat.formatFileSize` directly without loading `live-preview.js`, so they do not validate `updateMeta`'s numeric guard or `window.SizeFormat` integration. Add an integration test that invokes `updateMeta` with representative byte values and missing/non-number metadata.</violation>
</file>
<file name="webapp/static/js/utils/size-format.js">
<violation number="1" location="webapp/static/js/utils/size-format.js:95">
P1: כאשר דף `compare_files` נטען עם קבצים שנבחרו מראש, האתחול הסינכרוני קורא ל-`window.SizeFormat` לפני שהסקריפט הדחוי מגדיר אותו, ולכן האתחול נכשל עם `TypeError` וכרטיסי התצוגה אינם נטענים. חומרה: 7/10. טען את `size-format.js` ללא `defer`, או דחה את אתחול `compare_files` עד לאחר טעינת ה-API.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

}

if (typeof window !== 'undefined') {
window.SizeFormat = {

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.

P1: כאשר דף compare_files נטען עם קבצים שנבחרו מראש, האתחול הסינכרוני קורא ל-window.SizeFormat לפני שהסקריפט הדחוי מגדיר אותו, ולכן האתחול נכשל עם TypeError וכרטיסי התצוגה אינם נטענים. חומרה: 7/10. טען את size-format.js ללא defer, או דחה את אתחול compare_files עד לאחר טעינת ה-API.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At webapp/static/js/utils/size-format.js, line 95:
<comment>כאשר דף `compare_files` נטען עם קבצים שנבחרו מראש, האתחול הסינכרוני קורא ל-`window.SizeFormat` לפני שהסקריפט הדחוי מגדיר אותו, ולכן האתחול נכשל עם `TypeError` וכרטיסי התצוגה אינם נטענים. חומרה: 7/10. טען את `size-format.js` ללא `defer`, או דחה את אתחול `compare_files` עד לאחר טעינת ה-API.</comment>
<file context>
@@ -0,0 +1,103 @@
+ }
+
+ if (typeof window !== 'undefined') {
+ window.SizeFormat = {
+ UNITS: UNITS,
+ LRI: LRI,
</file context>

Comment threadtests/test_size_format_bidi_browser.py Outdated
const size = typeof meta.bytes === 'number' ? `${(meta.bytes / 1024).toFixed(1)}KB` : '';
// המימוש הקודם חילק ב-1024 פעם אחת והציג תמיד KB, ולכן קובץ של 582
// בתים הופיע כ-"0.6KB" ושל 3.5 מגה כ-"3584KB".
const size = typeof meta.bytes === 'number' ? window.SizeFormat.formatFileSize(meta.bytes) : '';

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.

P2: Custom agent: Enforce Pragmatic Test Coverage

The new live-preview delegation is not exercised by the added tests. Both JS test harnesses call SizeFormat.formatFileSize directly without loading live-preview.js, so they do not validate updateMeta's numeric guard or window.SizeFormat integration. Add an integration test that invokes updateMeta with representative byte values and missing/non-number metadata.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At webapp/static/js/live-preview.js, line 810:
<comment>The new live-preview delegation is not exercised by the added tests. Both JS test harnesses call `SizeFormat.formatFileSize` directly without loading `live-preview.js`, so they do not validate `updateMeta`'s numeric guard or `window.SizeFormat` integration. Add an integration test that invokes `updateMeta` with representative byte values and missing/non-number metadata.</comment>
<file context>
@@ -805,7 +805,9 @@
- const size = typeof meta.bytes === 'number' ? `${parseFloat((meta.bytes / 1024).toFixed(1))}KB` : '';
+ // המימוש הקודם חילק ב-1024 פעם אחת והציג תמיד KB, ולכן קובץ של 582
+ // בתים הופיע כ-"0.6KB" ושל 3.5 מגה כ-"3584KB".
+ const size = typeof meta.bytes === 'number' ? window.SizeFormat.formatFileSize(meta.bytes) : '';
this.previewMeta.textContent = [language, size, duration].filter(Boolean).join(' • ');
}
</file context>

Comment threadwebapp/static/js/utils/size-format.js
ארבעה ממצאי ריוויו, כולם אומתו מול הקוד לפני התיקון.
1. עמוד ההשוואה נפל. compare_files.html קורא ל-CompareView.initFilesMode
ב-script אינליין שרץ בזמן ניתוח המסמך, ומשם — כשיש קבצים נבחרים מראש —
bindFilesEvents קורא ל-updateFilePreview ומגיע ל-formatFileSize מיד.
סקריפט דחוי עדיין לא רץ באותו רגע, ולכן:
TypeError: Cannot read properties of undefined (reading 'formatFileSize').
שוחזר בדפדפן לפני התיקון.
השורש: מודול דחוי אינו יכול לשמש תלות של קוד שרץ בזמן הניתוח. התג ב-
base.html איבד את defer — הקובץ שוקל ~5KB, אין לו תלויות, והוא יושב לפני
block extra_js שבו יושבים כל הצרכנים. אותו שיקול חל כבר על utils/rtl-code.js
ב-repo/base_repo.html.
האימות הקודם שלי בדק שכל הקריאות יושבות בתוך פונקציות — והן כן. מה שלא
נבדק היה אם מישהו קורא לפונקציות האלה בזמן הניתוח.
2. UNITS והאובייקט המיוצא מוקפאים. הם היו מערך ואובייקט משותפים, וכל סקריפט
בעמוד יכול היה לשנות את טבלת היחידות או להחליף את הפונקציה ולשנות בכך את
התצוגה בכל ששת הצרכנים. בפייתון המקביל הוא tuple וחסין ממילא.
3. בדיקת ה-bidi דילגה בשקט כש-chromium_executable הוא None. None פירושו "תן
ל-Playwright לחפש בעצמו" ולא "אין דפדפן", ולכן בסביבה שהתקינה playwright
install chromium כרגיל הבדיקה לא הייתה רצה לעולם. עברה לדפוס של שאר קובצי
הדפדפן: נתיב אם יש, אחרת חיפוש עצמי, ודילוג רק אם ההשקה נכשלת.
4. live-preview לא היה מכוסה. הבדיקות קראו ל-SizeFormat ישירות ולא טענו את
live-preview.js, ולכן השומר typeof meta.bytes === 'number' לא נבדק כלל.
תשתית: tests/test_size_format_consumers_browser.py — טוען את compare.js ואת
live-preview.js האמיתיים בסדר הטעינה של האפליקציה, מריץ את האתחול הסינכרוני
של compare_files, ובודק את updateMeta על גודל תקין, חסר, ולא-מספרי.
הבדיקה שולפת את תג ה-script מ-base.html עצמו ולא כותבת אותו מחדש: מה שנשבר
הוא התכונה על התג, ותג שנכתב בקובץ הבדיקה היה ממשיך לעבור אחרי ש-defer יחזור.
אומת: החזרת defer מפילה אותה עם השגיאה המקורית. הסרת ההקפאה מ-UNITS ומהאובייקט
מפילה כל אחת את הבדיקה שלה בנפרד.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y251xcBYzeYZUimEMKBQsS

@cubic-dev-aicubic-dev-aiBot 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.

1 issue found across 5 files (changes from recent commits).

You’re at about 91% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/test_size_format_consumers_browser.py">
<violation number="1" location="tests/test_size_format_consumers_browser.py:156">
P3: The meta evaluate runs `call(...)` before returning the dict, so if `livePreviewController` is undefined the whole `page.evaluate` rejects with a generic TypeError and the `meta["controller"] == "object"` guard never gets to report. Compute and assert the controller guard separately (or return the dict before invoking `call`), so a missing controller produces the targeted message instead of an opaque fixture error.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

return el.textContent;
};
out.controller = typeof window.livePreviewController;
out.small = call({language: 'markdown', bytes: 582});

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.

P3: The meta evaluate runs call(...) before returning the dict, so if livePreviewController is undefined the whole page.evaluate rejects with a generic TypeError and the meta["controller"] == "object" guard never gets to report. Compute and assert the controller guard separately (or return the dict before invoking call), so a missing controller produces the targeted message instead of an opaque fixture error.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_size_format_consumers_browser.py, line 156:
<comment>The meta evaluate runs `call(...)` before returning the dict, so if `livePreviewController` is undefined the whole `page.evaluate` rejects with a generic TypeError and the `meta["controller"] == "object"` guard never gets to report. Compute and assert the controller guard separately (or return the dict before invoking `call`), so a missing controller produces the targeted message instead of an opaque fixture error.</comment>
<file context>
@@ -0,0 +1,247 @@
+ return el.textContent;
+ };
+ out.controller = typeof window.livePreviewController;
+ out.small = call({language: 'markdown', bytes: 582});
+ out.large = call({language: 'markdown', bytes: 3.5 * 1024 * 1024, duration_ms: 12});
+ out.missing = call({language: 'markdown'});
</file context>

@amirbiron
amirbiron merged commit 09aa273 into mainSep 7, 2026
31 checks passed
Sign up for freeto 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

@amirbiron@claude