Skip to content

Fix mypy attr-defined errors in JobRegistry - #2395

Merged
amirbiron merged 3 commits into
cursor/github-context-response-a963from
copilot/sub-pr-2394
Dec 26, 2025
Merged

Fix mypy attr-defined errors in JobRegistry#2395
amirbiron merged 3 commits into
cursor/github-context-response-a963from
copilot/sub-pr-2394

Conversation

Copilot AI commented Dec 26, 2025

Copy link
Copy Markdown
Contributor

✨ תיאור קצר

תיקון שגיאות mypy attr-defined ב-JobRegistry שנגרמו מאתחול דינמי של _jobs ב-__new__. mypy לא זיהה את האטריביוט כאשר הוא הוגדר רק במתודת הסינגלטון.

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

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

פירוט נקודות (רשימת תבליטים):

  • services/job_registry.py: נוספה מתודת __init__ ל-JobRegistry המאתחלת את _jobs: Dict[str, JobDefinition] עם type annotation מפורש. הדבר שומר על דפוס הסינגלטון תוך פתרון 7 שגיאות mypy attr-defined.
def __init__(self) -> None:
    """Initialize the instance (only once)"""
    if not hasattr(self, '_jobs'):
        self._jobs: Dict[str, JobDefinition] = {}

🧪 בדיקות

  • mypy רץ על הקובץ המעודכן ואישר אפס שגיאות
  • Unit
  • Integration
  • Manual

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

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

📝 סוג שינוי

  • feat: פיצ'ר חדש
  • fix: תיקון באג
  • docs: שינוי תיעוד בלבד
  • refactor: שינוי קוד ללא שינוי התנהגות
  • perf: שיפור ביצועים
  • chore/ci: תשתית/CI
  • breaking change: שינוי שובר תאימות

✅ צ'קליסט

  • הקוד עוקב אחרי הסגנון (Black/isort/flake8/mypy)
  • בדיקות רצות ועוברות
  • תיעוד עודכן (README/Docs)
  • אם נוספו/שונו משתני סביבה – עודכן docs/environment-variables.rst וגם services/config_inspector_service.py
  • אם נוספו/השתנו טוקנים – עודכן גם docs/webapp/theming_and_css.rst + FEATURE_SUGGESTIONS/theme_matrix.md
  • אין סודות/מפתחות בקוד
  • אין מחיקות מסוכנות/פעולות על root (ראו .cursorrules)
  • הודעת הקומיט תואמת Conventional Commits (ע"פ הטבלה)
  • CHANGELOG עודכן אם נדרש
  • כל ה‑Required Checks לעיל ירוקים
  • צילום/וידאו UI מצורף אם רלוונטי

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

  • אין שינוי בלוגיקה או התנהגות, רק type annotation למען mypy. הסינגלטון ממשיך לעבוד כמצופה.

🔗 קישורים

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

  • סיכון אפסי. שינוי קוסמטי למען type checking בלבד.

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.


Note

Resolves mypy attr-defined errors by ensuring _jobs is defined with a clear type on instance initialization.

  • In services/job_registry.py, adds __init__ that sets self._jobs: Dict[str, JobDefinition] = {} if missing
  • Updates __new__ to assign new_instance._jobs = {} (type annotation moved to __init__) to keep thread-safe singleton setup

Written by Cursor Bugbot for commit 88c3e88. This will update automatically on new commits. Configure here.

…te for mypy

Co-authored-by: amirbiron <215461772+amirbiron@users.noreply.github.com>
Copilot AI changed the title [WIP] Add background jobs monitoring system Fix mypy attr-defined errors in JobRegistry Dec 26, 2025
Copilot AI requested a review from amirbiron December 26, 2025 17:51
@amirbiron

Copy link
Copy Markdown
Owner

@cursoragent

This branch has conflicts that must be resolved
Use the web editor or the command line to resolve conflicts before continuing.

services/job_registry.py

@cursor

cursor Bot commented Dec 26, 2025

Copy link
Copy Markdown
Contributor

פתרתי את הקונפליקט ב-services/job_registry.py והדחפתי לענף ה-PR.

הפתרון משלב את שני השינויים:

  1. מהענף הבסיסי (cursor/github-context-response-a963): שמרתי את הדפוס הבטוח יותר ב-__new__ שמאתחל את _jobs לפני שחושף את ה-instance - מונע race condition.

  2. מהענף הנוכחי (PR זה): שמרתי את מתודת __init__ עם ה-type annotation עבור mypy.

הסרתי את ה-type annotation מתוך __new__ כי mypy לא תומך בהצהרת טיפוסים על non-self attributes (זו הייתה שגיאה חדשה שהופיעה).

הקוד הסופי:

  • ✅ עובר mypy בהצלחה (0 שגיאות)
  • ✅ עובר את כל הטסטים (23 passed)
  • ✅ ה-PR עכשיו MERGEABLE (הקונפליקט נפתר)

Open in Cursor Open in Web

Learn more about Cursor Agents

Merged changes from base branch (cursor/github-context-response-a963):
- Keep the race-condition-safe pattern in __new__ (initialize _jobs before exposing instance)
- Keep the __init__ method with type annotation for mypy compatibility
- Remove inline type annotation from __new__ (mypy doesn't allow type declarations on non-self attributes)

Both the race condition fix and mypy compatibility are preserved.
@github-actions

github-actions Bot commented Dec 26, 2025

Copy link
Copy Markdown

🧯 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): 125

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

Excluded matches (by path pattern)
./Dockerfile:42:    rm -rf /var/lib/apt/lists/*
./Dockerfile:121:    rm -rf /var/lib/apt/lists/*
./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]
./README.md:842:find . -name "__pycache__" -exec rm -rf {} +
./docs/DOCUMENTATION_GUIDE.md:453:rm -rf _build
./docs/Makefile:24:	rm -rf $(BUILDDIR)
./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/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.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/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.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/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]
./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

@amirbiron
amirbiron marked this pull request as ready for review December 26, 2025 17:59
@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.

@cursor

cursor Bot commented Dec 26, 2025

Copy link
Copy Markdown
Contributor

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
Learn more about Cursor Agents

@github-actions

github-actions Bot commented Dec 26, 2025

Copy link
Copy Markdown

⏱️ Performance report

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

@sentry

sentry Bot commented Dec 26, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
services/job_registry.py 50.00% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@amirbiron
amirbiron merged commit b4d5579 into cursor/github-context-response-a963 Dec 26, 2025
33 of 34 checks passed
amirbiron added a commit that referenced this pull request Dec 26, 2025
* feat: Add background jobs monitor API and UI

Co-authored-by: amirbiron <amirbiron@gmail.com>

* feat: Add background jobs monitor to settings

Co-authored-by: amirbiron <amirbiron@gmail.com>

* Fix: Implement job trigger endpoint and improve error handling

Co-authored-by: amirbiron <amirbiron@gmail.com>

* Fix mypy attr-defined errors in JobRegistry (#2395)

* Initial plan

* fix: Add __init__ to JobRegistry to properly initialize _jobs attribute for mypy

Co-authored-by: amirbiron <215461772+amirbiron@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: amirbiron <215461772+amirbiron@users.noreply.github.com>
Co-authored-by: Cursor Agent <cursoragent@cursor.com>

* Make get_job_tracker thread-safe

Co-authored-by: amirbiron <amirbiron@gmail.com>

* Refactor job tracking and registry, improve error handling

Co-authored-by: amirbiron <amirbiron@gmail.com>

* Refactor: Improve logging levels in JobTracker

Co-authored-by: amirbiron <amirbiron@gmail.com>

* Refactor: Improve job registry and command handling

Co-authored-by: amirbiron <amirbiron@gmail.com>

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: amirbiron <215461772+amirbiron@users.noreply.github.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.

3 participants