Repository navigation
feat(mcp): ניקוי הודעת השגיאה, כיווניות, מודאל לכוונה, ובאנר "מאז הביקור האחרון" ב-/admin/mcp - #3335
Merged
Merged
Conversation
…קור האחרון"
ארבעה תיקוני קריאוּת במסך /admin/mcp, אחרי שהטבלאות התמלאו בנתונים אמיתיים
והתברר מה קשה לקרוא בהן.
ניקוי ההודעה
הודעת ValidationError של Pydantic היא ארבע שורות, ושלוש מהן רעש בהקשר של
טבלה: כותרת שנושאת את שם הכלי — שכבר יש לו עמודה משלו; הזחות; ושורת "For
further information visit ..." שזהה בכל שגיאה מאותו סוג. summarize_validation_message
משאיר שורה אחת לכל שדה שנדחה:
lines.1 · Input should be a valid integer [input_value='9', input_type=str]
הפירסור **נכשל בטוח**: מה שאינו נראה כמו הודעת Pydantic מוחזר כפי שהוא, בלי
לגעת. הכלל הזה חשוב יותר מהניקוי עצמו — מאז שהשער נפתח לכל סוגי השגיאות
ההודעה בתא יכולה להיות של כל ספרייה, וקיצור שמנחש פורמט היה מוחק דווקא את מה
שאי אפשר לאבחן בלעדיו. המבנה שהפירסור נשען עליו נמדד על pydantic 2.12.3 ולא
נלקח מהתיעוד: הרינדור עצמו הוא ב-Rust ב-pydantic_core, ואין קוד פייתון לקרוא.
כיווניות
ההודעה והכוונה הן טקסט טכני באנגלית בתוך עמוד RTL. בלי כפיית כיווניות
האלגוריתם הדו-כיווני מזיז סוגריים וסימני שוויון לקצה הלא נכון,
ו-[input_value='9'] מוצג שבור. .mcp-ltr יושב על התא ולא על הטבלה — סדר
העמודות נשאר RTL — ומשתמש ב-unicode-bidi: isolate כדי לא לדלוף לשכנים.
מודאל לכוונה המלאה
הטקסט מקוצץ בתא, והתא עצמו הוא <button> אמיתי ולא span עם tabindex: כך Enter
ו-Space עובדים בלי JavaScript משלנו, וקורא מסך מכריז "כפתור" במקום להשמיע
טקסט שלא ברור שאפשר ללחוץ עליו. Escape סוגר ומחזיר את הפוקוס לכפתור שפתח.
הטקסט נכתב ב-textContent — זהו מסלול שני לאותו טקסט שסוכן חיצוני כתב, וה-escape
של Jinja אינו מגן עליו.
באנר "X שגיאות חדשות מאז הביקור האחרון"
הטבלה מציגה חלון נע של 30 יום, ולכן המספר הכולל יורד מעצמו כששגיאות ישנות
נושרות — גם כשנוספו חדשות — ואינו יכול לשמש איתות. מה שכן יציב הוא חותמת הזמן
של החדשה ביותר שכבר נראתה: סופרים כמה שורות חדשות ממנה, ורק אז שומרים את
הנוכחית. ההשוואה היא על data-at הגולמי ולא על המחרוזת המוצגת, שמעוגלת לדקה.
בביקור ראשון הבאנר שותק — אז הכול "חדש". localStorage חסום פשוט לא מציג אותו.
בדיקות
12 מוטציות, כל אחת מאמתת שהעוגן שלה קיים לפני שהיא רצה, וכל אחת מפילה בדיוק
את הטסט שאמור לתפוס אותה: ויתור על הפירסור, הצגת ההודעה הגולמית, הסרת ה-LTR,
חזרה ל-span, מודאל פתוח כברירת מחדל, innerHTML במקום textContent (בשני
מסלולים), הסרת data-at, ניתוק הלחיצה, רישום הביקור לפני הספירה, ספירה שמתעלמת
מחותמת הזמן, וביקור ראשון שנספר כאילו הכול חדש.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018YKyee3UAYEbTKFjxdmBjs
|
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 12 hours and 9 minutes by commenting @sourcery-ai review. Upgrade to get a review now.
Contributor
|
Warning Review limit reachedNext included review available in 15 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: Team Run ID: 📒 Files selected for processing (8)
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 refines /admin/mcp entirely at the presentation layer: it safely summarizes recognizable Pydantic errors, isolates technical text in the RTL UI, adds an accessible full-intent modal, and signals failures newer than the last browser visit via localStorage, with documentation and unit/browser coverage. Sequence diagram for the admin MCP failure presentationsequenceDiagram
participant Admin
participant Browser
participant AdminMCP as Admin MCP page
participant Jinja
participant Service as MCP analytics service
participant Storage as localStorage
Admin->>Browser: Open /admin/mcp
Browser->>AdminMCP: Request page
AdminMCP->>Service: Load failure rows
Service-->>AdminMCP: Rows with raw error messages and timestamps
AdminMCP->>Jinja: Apply validation_summary
Jinja->>Service: summarize_validation_message(message)
Service-->>Jinja: Cleaned Pydantic text or original message
Jinja-->>Browser: Render RTL table with LTR cells and data-at
Browser->>Storage: readLastSeen()
Storage-->>Browser: Previous newest timestamp
Browser->>Browser: Count rows newer than previous timestamp
Browser-->>Admin: Show fresh-failures banner when count > 0
Browser->>Storage: writeLastSeen(newest timestamp)
Sequence diagram for opening the full MCP intent modalsequenceDiagram
actor Admin
participant Browser
participant IntentButton as Intent button
participant Modal as Intent modal
Admin->>IntentButton: Click, Enter, or Space
IntentButton->>Browser: click event
Browser->>Modal: openModal(button)
Modal->>Modal: textContent = button title or text
Modal-->>Admin: Show full intent safely as text
Admin->>Browser: Press Escape or click close
Browser->>Modal: closeModal()
Modal-->>IntentButton: Restore focus
File-Level Changes
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! |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 8 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…י ההודעה שישה ממצאי ריוויו, כולם אמיתיים. שלושה מהם אותו שורש: פעולה גורפת על טקסט שמכיל גם רעש וגם ערך, או הצהרה שרחבה יותר מהנתונים שמאחוריה. ניקוי ההודעה — שני באגים ``\r\n`` השאיר ``\r`` בסוף כל שורה, ואז התבנית של שורת התיעוד — שנגמרת ב-``$`` — כבר לא תפסה, והקישור לתיעוד של Pydantic שרד בתא. הפיצול עובר עכשיו דרך ``\r\n|\r|\n``, ולא דרך ``splitlines()`` שמפצל גם על ``\x0b`` ו-`` `` שיכולים לשבת בתוך ``input_value``. והשני: ניקוי הסוגריים הריקות שנשארו מהסרת ``type=`` מחק גם ``[]`` שהיה **בתוך** הערך, ו-``input_value=[]`` הפך ל-``input_value=`` — כלומר נמחק בדיוק מה שמפריד בין "הסוכן שלח רשימה ריקה" ל"הסוכן לא שלח כלום". במקום שתי פעולות יש עכשיו ביטוי אחד עם קבוצה אופציונלית, ומה שמגן הוא שהתאמה נעשית שמאלית-ראשונה: קבוצת הסוגריים האמיתית היא הראשונה בשורה, ולכן ``[type=...]`` שיושב בתוך הערך לעולם אינו נתפס במקומה. בדרך תוקן גם ניסיון ביניים שלי: עיגנתי את התבנית ל-``$`` והסברתי שזה מה שמגן. מוטציה הראתה שאף טסט לא מבחין — ובצדק, כי ההגנה מגיעה מסדר ההתאמה ולא מהעיגון, ובמקרה קצה העיגון היה בוחר דווקא קבוצה מאוחרת יותר. הוסר. המודאל — ``<dialog>`` נייטיב הגרסה הידנית הייתה ``div`` שמוצג ומוסתר, ולכן ``Shift+Tab`` מכפתור הסגירה יצא אל התוכן שמאחור בזמן שהוא חסום ויזואלית. הפתרון אינו להוסיף לכידת ``Tab`` משלנו אלא להשתמש במה שהדפדפן כבר נותן: ``showModal()`` נועל את הפוקוס, סוגר ב-``Escape``, מחזיר את הפוקוס לכפתור שפתח, ומצייר את הרקע ב-``::backdrop``. המעבר **מחק** קוד — מטפל ``Escape`` ידני, ניהול ``lastFocused``, ושכבת רקע — במקום להוסיף. נמדד בכרומיום אמיתי לפני שנבחר: ``Shift+Tab`` מתחלף בין הכפתור לדיאלוג ואינו מגיע החוצה, והפוקוס חוזר לבד. תמיכה: Baseline "widely available" מאז מרץ 2022, 96% גלובלית, iOS Safari 15.4+, Samsung Internet 4+. הבאנר — "לפחות" כשהטבלה חתוכה הבאנר סופר שורות ב-DOM, והשאילתה מוגבלת ל-25 שורות. כשכל השורות שהוצגו חדשות ויש עוד מעבר לתקרה, המספר הוא **רצפה** ולא ספירה — והצהרה מדויקת עליו היא המצאת נתון. ``has_more`` עובר לדפדפן ב-``data-has-more``, והניסוח משתנה ל"לפחות X". הספירה לא השתנתה, כי אי אפשר לספור מה שהשאילתה לא החזירה: ספירה אמיתית הייתה דורשת לשלוח לשרת את חותמת הזמן ששמורה בדפדפן, בקשה נוספת עבור שורת טקסט. הטסטים — מרים דפדפן במקום אחד ``_fresh_page`` היה עותק שלישי של אותן שלושים שורות הרמה שכבר חיו בפיקסצ'ר, ובנוסף הניקוי שלו ישב אצל הקורא — ולכן נפילה של ``goto`` דילגה עליו לגמרי והשאירה דפדפן וסשן Playwright תלויים. שניהם נפתרו באותו צעד: מנהל הקשר יחיד ``_browser_page`` שהפיקסצ'ר עצמו מאציל אליו, עם ה-``finally`` בפנים. יש טסט שמפנה את ``goto`` לפורט סגור ומוודא שהסגירה בכל זאת רצה. מוטציות: תשע, כל אחת מפילה בדיוק את הטסט שאמור לתפוס אותה. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018YKyee3UAYEbTKFjxdmBjs
Contributor
There was a problem hiding this comment.
4 issues found across 7 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/templates/admin_mcp.html">
<violation number="1" location="webapp/templates/admin_mcp.html:799">
P3: The backdrop-click close uses an axis-aligned bounding box with inclusive bounds, but the dialog has `border-radius: 16px`. A click in the small rounded-corner band is inside the rect yet visually on the dimmed backdrop, so it won't close the dialog. Minor; optionally use the rect plus a check against the dialogs padding/content before treating it as an inside click.</violation>
</file>
<file name="services/mcp_analytics_service.py">
<violation number="1" location="services/mcp_analytics_service.py:177">
P2: חומרה: 5/10. כשמסר Pydantic מכיל `[type=...]` בתוך טקסט השגיאה לפני המטא־דאטה, התבנית מסירה את המופע המוקדם ומשאירה את קבוצת `[type=...]` האמיתית. עגן את התבנית לסוף שורת הפירוט כדי להסיר רק את קבוצת המטא־דאטה.</violation>
</file>
<file name="tests/test_admin_mcp_tabs_browser.py">
<violation number="1" location="tests/test_admin_mcp_tabs_browser.py:228">
P3: The new `_browser_page` docstring claims it is "המקום היחיד שמרים דפדפן בקובץ הזה" (the only place in this file that launches a browser), but `test_an_intent_that_looks_like_markup_is_never_a_live_element` still launches its own browser directly at line 413 with `with sync_playwright() as pw: pw.chromium.launch(...)`. That test also keeps its cleanup in a caller-side `finally` that begins only *after* `goto`/`evaluate`, so a navigation failure there still leaks the browser/context — the exact leak this refactor was built to eliminate. Correct the docstring or convert that test to `_browser_page` so the claim and the safety guarantee are true. חומרת הממצא: 3 (ניקיון/תחזוקה).</violation>
<violation number="2" location="tests/test_admin_mcp_tabs_browser.py:704">
P3: `test_a_failed_page_load_still_shuts_the_browser_down` forces the navigation failure by pointing at hardcoded port 9 and expects `goto` to raise, truncating the check with a broad `with pytest.raises(Exception):`. If a discard/other service answers on port 9 or the OS blackholes it, `goto` may succeed (test fails spuriously) or hang up to the 30s default timeout instead of failing fast with ERR_CONNECTION_REFUSED. The broad exception also passes if an unrelated setup step fails, so the test does not prove a page-load failure actually happened. Prefer binding a socket to an ephemeral port, closing it, and pointing `goto` there (least likely to be grabbed), and asserting the specific navigation failure. חומרת הממצא: 3 (יציבות הטסט).</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| #: לסוגריים שנשארו ריקות — ושתיהן החטיאו: הראשונה דילגה על הקבוצה האמיתית | ||
| #: (שאחרי ``type`` יש בה פסיק) ונחתה על זו שבתוך הערך, והשנייה מחקה גם | ||
| #: ``input_value=[]`` והפכה אותו ל-``input_value=``. | ||
| _PYDANTIC_BRACKETS = re.compile(r"\s*\[type=[^,\]]+(?:,\s*(?P<rest>.*))?\]") |
Contributor
There was a problem hiding this comment.
P2: חומרה: 5/10. כשמסר Pydantic מכיל [type=...] בתוך טקסט השגיאה לפני המטא־דאטה, התבנית מסירה את המופע המוקדם ומשאירה את קבוצת [type=...] האמיתית. עגן את התבנית לסוף שורת הפירוט כדי להסיר רק את קבוצת המטא־דאטה.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At services/mcp_analytics_service.py, line 177:
<comment>חומרה: 5/10. כשמסר Pydantic מכיל `[type=...]` בתוך טקסט השגיאה לפני המטא־דאטה, התבנית מסירה את המופע המוקדם ומשאירה את קבוצת `[type=...]` האמיתית. עגן את התבנית לסוף שורת הפירוט כדי להסיר רק את קבוצת המטא־דאטה.</comment>
<file context>
@@ -157,10 +157,34 @@
+#: לסוגריים שנשארו ריקות — ושתיהן החטיאו: הראשונה דילגה על הקבוצה האמיתית
+#: (שאחרי ``type`` יש בה פסיק) ונחתה על זו שבתוך הערך, והשנייה מחקה גם
+#: ``input_value=[]`` והפכה אותו ל-``input_value=``.
+_PYDANTIC_BRACKETS = re.compile(r"\s*\[type=[^,\]]+(?:,\s*(?P<rest>.*))?\]")
+
+
</file context>
Suggested change
| _PYDANTIC_BRACKETS = re.compile(r"\s*\[type=[^,\]]+(?:,\s*(?P<rest>.*))?\]") | |
| _PYDANTIC_BRACKETS = re.compile(r"\s*\[type=[^,\]]+(?:,\s*(?P<rest>.*))?\]\s*$") |
| modal.addEventListener('click', function (event) { | ||
| if (event.target !== modal) { return; } | ||
| var box = modal.getBoundingClientRect(); | ||
| var inside = event.clientX >= box.left && event.clientX <= box.right |
Contributor
There was a problem hiding this comment.
P3: The backdrop-click close uses an axis-aligned bounding box with inclusive bounds, but the dialog has border-radius: 16px. A click in the small rounded-corner band is inside the rect yet visually on the dimmed backdrop, so it won't close the dialog. Minor; optionally use the rect plus a check against the dialogs padding/content before treating it as an inside click.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At webapp/templates/admin_mcp.html, line 799:
<comment>The backdrop-click close uses an axis-aligned bounding box with inclusive bounds, but the dialog has `border-radius: 16px`. A click in the small rounded-corner band is inside the rect yet visually on the dimmed backdrop, so it won't close the dialog. Minor; optionally use the rect plus a check against the dialogs padding/content before treating it as an inside click.</comment>
<file context>
@@ -767,34 +770,35 @@ <h2 id="mcpIntentModalTitle">מה הסוכן ניסה לעשות</h2>
+ modal.addEventListener('click', function (event) {
+ if (event.target !== modal) { return; }
+ var box = modal.getBoundingClientRect();
+ var inside = event.clientX >= box.left && event.clientX <= box.right
+ && event.clientY >= box.top && event.clientY <= box.bottom;
+ if (!inside) { modal.close(); }
</file context>
| try: | ||
| with pytest.raises(Exception): | ||
| # פורט 9 סגור, ולכן ``goto`` נכשל אחרי ``launch``. | ||
| with _browser_page(("http://127.0.0.1:9", session_cookie)): |
Contributor
There was a problem hiding this comment.
P3: test_a_failed_page_load_still_shuts_the_browser_down forces the navigation failure by pointing at hardcoded port 9 and expects goto to raise, truncating the check with a broad with pytest.raises(Exception):. If a discard/other service answers on port 9 or the OS blackholes it, goto may succeed (test fails spuriously) or hang up to the 30s default timeout instead of failing fast with ERR_CONNECTION_REFUSED. The broad exception also passes if an unrelated setup step fails, so the test does not prove a page-load failure actually happened. Prefer binding a socket to an ephemeral port, closing it, and pointing goto there (least likely to be grabbed), and asserting the specific navigation failure. חומרת הממצא: 3 (יציבות הטסט).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_admin_mcp_tabs_browser.py, line 704:
<comment>`test_a_failed_page_load_still_shuts_the_browser_down` forces the navigation failure by pointing at hardcoded port 9 and expects `goto` to raise, truncating the check with a broad `with pytest.raises(Exception):`. If a discard/other service answers on port 9 or the OS blackholes it, `goto` may succeed (test fails spuriously) or hang up to the 30s default timeout instead of failing fast with ERR_CONNECTION_REFUSED. The broad exception also passes if an unrelated setup step fails, so the test does not prove a page-load failure actually happened. Prefer binding a socket to an ephemeral port, closing it, and pointing `goto` there (least likely to be grabbed), and asserting the specific navigation failure. חומרת הממצא: 3 (יציבות הטסט).</comment>
<file context>
@@ -631,13 +621,89 @@ def test_the_banner_counts_only_failures_newer_than_the_last_visit(live_server):
+ try:
+ with pytest.raises(Exception):
+ # פורט 9 סגור, ולכן ``goto`` נכשל אחרי ``launch``.
+ with _browser_page(("http://127.0.0.1:9", session_cookie)):
+ pass # pragma: no cover
+ finally:
</file context>
|
|
||
| @contextlib.contextmanager | ||
| def _browser_page(live_server, local_storage=None, viewport=None): | ||
| """פותח את העמוד בהקשר דפדפן נקי — **המקום היחיד** שמרים דפדפן בקובץ הזה. |
Contributor
There was a problem hiding this comment.
P3: The new _browser_page docstring claims it is "המקום היחיד שמרים דפדפן בקובץ הזה" (the only place in this file that launches a browser), but test_an_intent_that_looks_like_markup_is_never_a_live_element still launches its own browser directly at line 413 with with sync_playwright() as pw: pw.chromium.launch(...). That test also keeps its cleanup in a caller-side finally that begins only after goto/evaluate, so a navigation failure there still leaks the browser/context — the exact leak this refactor was built to eliminate. Correct the docstring or convert that test to _browser_page so the claim and the safety guarantee are true. חומרת הממצא: 3 (ניקיון/תחזוקה).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_admin_mcp_tabs_browser.py, line 228:
<comment>The new `_browser_page` docstring claims it is "המקום היחיד שמרים דפדפן בקובץ הזה" (the only place in this file that launches a browser), but `test_an_intent_that_looks_like_markup_is_never_a_live_element` still launches its own browser directly at line 413 with `with sync_playwright() as pw: pw.chromium.launch(...)`. That test also keeps its cleanup in a caller-side `finally` that begins only *after* `goto`/`evaluate`, so a navigation failure there still leaks the browser/context — the exact leak this refactor was built to eliminate. Correct the docstring or convert that test to `_browser_page` so the claim and the safety guarantee are true. חומרת הממצא: 3 (ניקיון/תחזוקה).</comment>
<file context>
@@ -195,11 +217,28 @@ def live_server():
+
+@contextlib.contextmanager
+def _browser_page(live_server, local_storage=None, viewport=None):
+ """פותח את העמוד בהקשר דפדפן נקי — **המקום היחיד** שמרים דפדפן בקובץ הזה.
+
+ ``local_storage`` הוא הערך של ``mcpFailuresLastSeenAt``; ``None`` מוחק
</file context>
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.
✨ תיאור קצר
המשך ל-#3328, אחרי שהטבלאות התמלאו בנתונים אמיתיים והתברר מה קשה לקרוא בהן: הודעת השגיאה ארוכה ורובה רעש, טקסט טכני באנגלית מוצג שבור בתוך עמוד RTL, הכוונה נחתכת ואין דרך לראות את המשך המשפט, ומספר השגיאות הכולל לא יכול לשמש איתות כי החלון נע.
📦 שינויים עיקריים
פירוט:
ValidationErrorשל Pydantic היא ארבע שורות, ושלוש מהן רעש בהקשר של טבלה: כותרת שנושאת את שם הכלי — שכבר יש לו עמודה משלו; הזחות; ושורתFor further information visit https://errors.pydantic.dev/...שזהה בכל שגיאה מאותו סוג.summarize_validation_messageמשאיר שורה אחת לכל שדה שנדחה:lines.1 · Input should be a valid integer [input_value='9', input_type=str]. הפירסור נכשל בטוח — מה שאינו נראה כמו הודעת Pydantic מוחזר כפי שהוא, בלי לגעת. הכלל הזה חשוב יותר מהניקוי עצמו: מאז שהשער נפתח לכל סוגי השגיאות, ההודעה בתא יכולה להיות של כל ספרייה, וקיצור שמנחש פורמט היה מוחק דווקא את מה שאי אפשר לאבחן בלעדיו.[input_value='9']מוצג שבור..mcp-ltrיושב על התא ולא על הטבלה — סדר העמודות נשאר RTL — ומשתמש ב-unicode-bidi: isolateכדי לא לדלוף לשכנים.<button>אמיתי ולאspanעםtabindex: כך Enter ו-Space עובדים בלי JavaScript משלנו, וקורא מסך מכריז "כפתור" במקום להשמיע טקסט שלא ברור שאפשר ללחוץ עליו.Escapeסוגר ומחזיר את הפוקוס לכפתור שפתח.data-atהגולמי ולא על המחרוזת המוצגת, שמעוגלת לדקה.ב-PostHog: אין שינוי. לא נוספו ולא שונו אנדפוינטים; כל השינוי כאן הוא בתצוגה.
🧪 בדיקות
Unit
Integration
Manual
tests/test_admin_mcp_page.py,tests/test_mcp_analytics_service.py,tests/test_mcp_analytics_privacy.py,tests/test_mcp_server_build.py,tests/test_doc_summary_style.py— 228 עוברים על הבסיס הנוכחי.tests/test_admin_mcp_tabs_browser.py— 15 עוברים ב-Chromium אמיתי, כולל חמישה חדשים: לחיצה על כוונה מקוצצת פותחת מודאל עם הטקסט המלא, כוונה עוינת נשארת טקסט גם בתוך המודאל, הבאנר שותק בביקור ראשון, הבאנר סופר רק שורות חדשות מהביקור האחרון, וכלום אינו חדש כשהביקור מאוחר מכל השגיאות.מוטציות — 12, כל אחת מפילה בדיוק את הטסט שאמור לתפוס אותה. כל מוטציה מאמתת שהעוגן שלה קיים לפני שהיא רצה, כי מוטציה שלא הוחלה מציגה "מעבר" מזויף:
test_a_validation_message_becomes_one_readable_linetest_the_error_message_cell_shows_the_cleaned_linetest_technical_text_is_forced_ltr_inside_the_rtl_pagespanעםtabindextest_a_clipped_intent_is_a_real_button_and_not_a_faked_onetest_the_intent_modal_exists_and_starts_closedinnerHTMLבמקוםtextContent(סריקת מקור)test_the_mcp_script_never_assigns_to_innerhtmlinnerHTMLבמקוםtextContent(דפדפן אמיתי)test_a_hostile_intent_stays_text_inside_the_modal_toodata-atמהשורותtest_failure_rows_carry_a_raw_timestamp_for_the_new_since_last_visit_counttest_clicking_a_clipped_intent_opens_a_modal_with_the_full_texttest_the_banner_counts_only_failures_newer_than_the_last_visittest_nothing_is_new_when_the_last_visit_is_after_every_failuretest_the_banner_stays_quiet_on_a_first_visitמדידה, לא הנחה. המבנה שהפירסור נשען עליו נמדד אמפירית על
pydantic 2.12.3בארבעה סוגי שגיאה, ולא נלקח מהתיעוד: הרינדור עצמו הוא ב-Rust ב-pydantic_core, ואין קוד פייתון לקרוא. במקביל נבדק גם מה לא קורה: הערך שיוצא מהשרת נושא ירידות שורה אמיתיות ולא\nספרותי — הצורה הבורחת שנראתה בפלט הייתה ארטיפקט של תצוגת JSON, ולכן "ניקוי" כאן פירושו הסרת הרעש של Pydantic ולא טיפול בתווים.📝 סוג שינוי
✅ צ'קליסט
flake8 --select=E9,F63,F7,F82נקי, ו-ruffנקי על השורות שנוספו (הממצאים שנשארו ב-webapp/app.pyקיימים מלפני ה-PR)--card-bg,--card-border,--glass,--glass-border,--glass-hover,--primary,--text-primary), עם fallback, לפיtheming_and_cssdocs/whats-new.rstעודכןdocs/webapp/mcp-analytics.rst,docs/webapp/theming_and_css.rst,docs/doc-authoring.rst,docs/versioning-stable-anchors.rst,AI-MAP.md| המשפט: "התיעוד הוא התמצאות, לא סמכות. כל טענה עובדתית בו — התנהגות, נתיב, פרמטר, "נתמך" — קודם תאמת מול הקוד לפני שתסתמך עליה."🧩 השפעות/סיכונים
1. הפירסור נוגע בטקסט שמוצג לאדמין. הסיכון האמיתי אינו קיצור גרוע אלא מחיקה של מידע מאבחן. לכן הכלל אינו "קצר מה שאפשר" אלא "אל תיגע במה שלא זוהה בוודאות": כל הודעה שאינה תואמת את כותרת Pydantic (
N validation errors for X) מוחזרת בשלמותה, וכך גם מבנה שנראה כמו Pydantic אך אינו ניתן לפירוק. יש טסט שמוודא שהתוצאה אינה מכילה אף מילה שלא הייתה בקלט.2. מסלול שני לטקסט שסוכן חיצוני כתב. ה-escape של Jinja מגן על התא בטבלה, אבל המודאל נבנה ב-JavaScript ולכן צריך הגנה משלו. הוא נכתב ב-
textContentבלבד, ויש שתי בדיקות נפרדות: אחת סורקת את מקור הסקריפט ואוסרתinnerHTML, והשנייה מזריקה כוונה עוינת ובודקת בדפדפן אמיתי שהיא נשארה צומת טקסט. "רק אדמינים רואים" אינו הגנה — אדמינים הם היעד.3.
localStorageהוא לכל דפדפן, לא לכל משתמש. מי שנכנס ממכשיר אחר יראה ספירה שמתחילה מחדש. זו החלטה מודעת: הבאנר הוא עזר תצוגה, והוא לא שווה אוסף חדש במסד או מסלול כתיבה נוסף. קריאה וכתיבה עטופות, כיlocalStorageזורק בגלישה פרטית — ושם הבאנר פשוט לא מופיע, בלי לגעת בשאר העמוד.4. אין שינוי במה שנשלח ל-PostHog. השער, הכלים והאנדפוינטים לא נגעו. כל מה שכאן קורה אחרי שהנתון כבר הגיע.
🔗 קישורים
docs/webapp/mcp-analytics.rst→mcp-analytics-reading,mcp-analytics-fresh🧯 סיכון / החזרה לאחור (Rollback)
הכל הפיך ובלי מיגרציה.
revertשל הקומיט מחזיר את ההודעה הגולמית, את הכיווניות הקודמת, את ה-spanבמקום הכפתור ומסיר את הבאנר. שום נתון לא נאסף ולא נמחק, ואין שינוי ב-PostHog. הערך היחיד שנשאר הוא מפתח ב-localStorageשל הדפדפן, שפשוט מפסיק להיקרא.🤖 Generated with Claude Code
https://claude.ai/code/session_018YKyee3UAYEbTKFjxdmBjs
Generated by Claude Code
Summary by Sourcery
Improve the MCP analytics admin page so technical failures and intents are easier and safer to read while providing a reliable signal for errors added since the previous visit.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: