diff --git a/QualityTODO.md b/QualityTODO.md new file mode 100644 index 0000000..7143287 --- /dev/null +++ b/QualityTODO.md @@ -0,0 +1,1107 @@ +# Quality Issues and TODO List + +# Aniworld Web Application Development Instructions + +This document provides detailed tasks for AI agents to implement a modern web application for the Aniworld anime download manager. All tasks should follow the coding guidelines specified in the project's copilot instructions. + +## Project Overview + +The goal is to create a FastAPI-based web application that provides a modern interface for the existing Aniworld anime download functionality. The core anime logic should remain in `SeriesApp.py` while the web layer provides REST API endpoints and a responsive UI. + +## Architecture Principles + +- **Single Responsibility**: Each file/class has one clear purpose +- **Dependency Injection**: Use FastAPI's dependency system +- **Clean Separation**: Web layer calls core logic, never the reverse +- **File Size Limit**: Maximum 500 lines per file +- **Type Hints**: Use comprehensive type annotations +- **Error Handling**: Proper exception handling and logging + +## Additional Implementation Guidelines + +### Code Style and Standards + +- **Type Hints**: Use comprehensive type annotations throughout all modules +- **Docstrings**: Follow PEP 257 for function and class documentation +- **Error Handling**: Implement custom exception classes with meaningful messages +- **Logging**: Use structured logging with appropriate log levels +- **Security**: Validate all inputs and sanitize outputs +- **Performance**: Use async/await patterns for I/O operations + +## πŸ“ž Escalation + +If you encounter: + +- Architecture issues requiring design decisions +- Tests that conflict with documented requirements +- Breaking changes needed +- Unclear requirements or expectations + +**Document the issue and escalate rather than guessing.** + +--- + +## πŸ“š Helpful Commands + +```bash +# Run all tests +conda run -n AniWorld python -m pytest tests/ -v --tb=short + +# Run specific test file +conda run -n AniWorld python -m pytest tests/unit/test_websocket_service.py -v + +# Run specific test class +conda run -n AniWorld python -m pytest tests/unit/test_websocket_service.py::TestWebSocketService -v + +# Run specific test +conda run -n AniWorld python -m pytest tests/unit/test_websocket_service.py::TestWebSocketService::test_broadcast_download_progress -v + +# Run with extra verbosity +conda run -n AniWorld python -m pytest tests/ -vv + +# Run with full traceback +conda run -n AniWorld python -m pytest tests/ -v --tb=long + +# Run and stop at first failure +conda run -n AniWorld python -m pytest tests/ -v -x + +# Run tests matching pattern +conda run -n AniWorld python -m pytest tests/ -v -k "auth" + +# Show all print statements +conda run -n AniWorld python -m pytest tests/ -v -s +``` + +--- + +## πŸ“Š Detailed Analysis: The 7 Quality Criteria + +### 1️⃣ Code Follows PEP8 and Project Coding Standards + +#### Line Length Violations (80+ characters) + +- [ ] `src/cli/Main.py` line 14 - Logging configuration exceeds 100 chars +- [ ] `src/cli/Main.py` line 80 - User input prompt exceeds 88 chars +- [ ] `src/cli/Main.py` line 91 - List comprehension exceeds 100 chars +- [ ] `src/cli/Main.py` line 118 - Nested sum with comprehension exceeds 100 chars +- [ ] `src/cli/Main.py` line 133 - Method call with many arguments exceeds 100 chars +- [ ] `src/config/settings.py` line 9 - Field definition exceeds 100 chars +- [ ] `src/config/settings.py` line 18 - Field definition exceeds 100 chars +- [ ] `src/server/utils/dependencies.py` line 260 - Conditional check exceeds 100 chars +- [ ] `src/core/providers/enhanced_provider.py` line 45 - List of providers exceeds 88 chars +- [ ] `src/core/providers/enhanced_provider.py` line 48-61 - Header dict values exceed 88 chars (10+ instances) +- [ ] `src/core/providers/streaming/voe.py` line 52 - HTTP header setup exceeds 100 chars +- [ ] `src/server/database/models.py` - Check field documentation lengths + +#### Naming Convention Issues + +- [ ] `src/cli/Main.py` - Class names and method names use mixed conventions + - `__InitList__()` should be `__init_list__()` (private method naming) + - `SeriesApp` duplicated naming from core module +- [ ] `src/core/SerieScanner.py` - Method names use mixed conventions + - `Scan()` should be `scan()` (not PascalCase) + - `GetTotalToScan()` should be `get_total_to_scan()` + - `__ReadDataFromFile()` should be `__read_data_from_file()` + - `__GetMissingEpisodesAndSeason()` should be `__get_missing_episodes_and_season()` + - `__find_mp4_files()` already correct (good consistency needed elsewhere) +- [ ] `src/core/providers/base_provider.py` - Abstract methods use PascalCase (violates convention) + - `Search()` should be `search()` + - `IsLanguage()` should be `is_language()` + - `Download()` should be `download()` + - `GetSiteKey()` should be `get_site_key()` + - `GetTitle()` should be `get_title()` + - `get_season_episode_count()` is correct +- [ ] `src/core/providers/streaming/Provider.py` - Same PascalCase issue + - `GetLink()` should be `get_link()` +- [ ] `src/core/providers/enhanced_provider.py` - Mixed naming conventions +- [ ] `src/server/models/download.py` - Verify enum naming consistency + +#### Import Sorting and Organization + +- [ ] `src/cli/Main.py` - Imports not in isort order + - Should group: stdlib, third-party, local imports + - Line 1-11 needs reorganization +- [ ] `src/core/providers/aniworld_provider.py` - Imports not sorted +- [ ] `src/core/providers/enhanced_provider.py` - Imports not sorted +- [ ] `src/server/api/download.py` - Verify import order +- [ ] Run `isort --check-only` on entire codebase to identify violations + +#### Blank Line Spacing Issues (PEP8 Β§4) + +- [ ] `src/cli/Main.py` - Missing blank lines between methods +- [ ] `src/core/providers/aniworld_provider.py` - Inconsistent class method spacing +- [ ] Verify 2 blank lines between top-level class/function definitions + +#### Indentation Consistency + +- [ ] `src/cli/Main.py` - Mixed indentation in multi-line statements +- [ ] `src/core/SerieScanner.py` - Check continuation line alignment +- [ ] `src/server/database/service.py` - Verify parameter alignment in method definitions + +--- + +### 2️⃣ Type Hints Used Where Applicable + +#### Missing Type Hints by Category + +**Abstract Base Classes (Critical)** + +- [ ] `src/core/providers/base_provider.py` - 6 abstract methods entirely untyped + - Lines 7, 11, 15, 19, 23, 27 + - All need parameter types and return types +- [ ] `src/core/providers/streaming/Provider.py` - `GetLink()` method + - Return type `(str, [str])` is invalid syntax + - Should use `Tuple[str, List[str]]` + +**CLI Module** + +- [ ] `src/cli/Main.py` - 12+ methods without type hints + - `search(words: str) -> list` - line 70 + - `get_user_selection() -> list` - line 75 + - `display_series()` - no type hints + - `__InitList__()` - no type hints + - `retry()` method - no type hints + - `print_Download_Progress()` - no type hints + +**Service Classes** + +- [ ] `src/server/services/auth_service.py` - Missing return type on `_verify_password()` +- [ ] `src/server/services/config_service.py` - Check all methods for return types +- [ ] `src/server/services/monitoring_service.py` - Check method signatures +- [ ] `src/server/services/backup_service.py` - Check method signatures +- [ ] `src/server/services/analytics_service.py` - Check method signatures + +**API Endpoints** + +- [ ] `src/server/api/anime.py` - Line 100+ `get_anime()` missing return type +- [ ] `src/server/api/health.py` - Check all endpoints +- [ ] `src/server/api/maintenance.py` - Check all endpoints +- [ ] `src/server/api/analytics.py` - Check all endpoints +- [ ] `src/server/api/backup.py` - Check all endpoints + +**Dependencies and Utils** + +- [ ] `src/server/utils/dependencies.py` - Several function return types missing + - `get_current_user()` at line 100 - returns `dict` but untyped + - `optional_auth()` - returns `Optional[dict]` but untyped + - `CommonQueryParams.__init__()` - no return type annotation + +**Core Classes** + +- [ ] `src/core/SeriesApp.py` - Missing return types on: + - `ReScan()` method + - `_handle_error()` method + - `_handle_completion()` method + - `download_series()` method (partially typed) +- [ ] `src/core/SerieScanner.py` - Missing return types on: + - `Scan()` method - line 85+ + - `__find_mp4_files()` method + +**Property Decorators** + +- [ ] `src/core/entities/series.py` - Check if properties have return types + - Lines 15-50 - properties should have return type annotations +- [ ] `src/core/providers/aniworld_provider.py` - Check class property types + +#### Invalid Type Hint Syntax + +- [ ] `src/core/providers/streaming/Provider.py` line 7 + - `(str, [str])` should be `Tuple[str, List[str]]` +- [ ] `src/core/entities/series.py` line 4 + - Uses `dict[int, list[int]]` - OK for Python 3.9+, but check minimum version + +--- + +### 3️⃣ Clear, Self-Documenting Code Written + +#### Missing or Inadequate Docstrings + +**Module-Level Docstrings** + +- [ ] `src/cli/Main.py` - Missing module docstring +- [ ] `src/core/entities/SerieList.py` - Check module docstring +- [ ] `src/core/providers/streaming/doodstream.py` - Check module docstring +- [ ] `src/core/providers/streaming/filemoon.py` - Check module docstring +- [ ] `src/core/providers/streaming/hanime.py` - Check module docstring +- [ ] `src/server/api/maintenance.py` - Check module docstring + +**Class Docstrings** + +- [ ] `src/cli/Main.py` - `SeriesApp` class lacks comprehensive docstring +- [ ] `src/core/providers/enhanced_provider.py` - Class docstring incomplete +- [ ] `src/server/utils/dependencies.py` - `CommonQueryParams` class lacks docstring + +**Method/Function Docstrings** + +- [ ] `src/cli/Main.py` + - `__InitList__()` - missing docstring (line 50) + - `display_series()` - missing docstring (line 60) + - `get_user_selection()` - missing docstring (line 75) + - `search()` - missing docstring (line 70) + - `retry()` - missing docstring + - `print_Download_Progress()` - missing docstring +- [ ] `src/core/SerieScanner.py` + - `is_null_or_whitespace()` - missing docstring (line 66) +- [ ] `src/core/providers/base_provider.py` + - All 6 abstract methods - missing docstrings explaining the contract +- [ ] `src/core/providers/streaming/Provider.py` + - `GetLink()` - missing docstring +- [ ] `src/server/utils/logging.py` - Check helper functions +- [ ] `src/server/utils/template_helpers.py` - Check helper functions + +#### Unclear Variable Names + +- [ ] `src/cli/Main.py` line 122 + - `task3` is vague - should be `task_progress` or `download_progress_task` +- [ ] `src/core/providers/enhanced_provider.py` line 35 + - `rec` in dictionary - should be `rate_limit_record` +- [ ] `src/core/SerieScanner.py` line 138 + - `missings` - should be `missing_episodes` +- [ ] `src/server/utils/dependencies.py` line 34 + - `security` - should be `http_bearer_security` + +#### Unclear Comments or Missing Context + +- [ ] `src/cli/Main.py` line 122 - German comment + - "Setze total auf 100 fΓΌr Prozentanzeige" should be English + - Should be: "Set total to 100 for percentage display" +- [ ] `src/core/providers/enhanced_provider.py` line 231 + - Comment style inconsistent with rest of codebase +- [ ] `src/server/api/download.py` line 51 + - Backward compatibility comment clear but needs more detail + +--- + +### 4️⃣ Complex Logic Commented + +#### Complex Algorithms Without Comments + +**SerieScanner Scanning Logic** + +- [ ] `src/core/SerieScanner.py` lines 105-200 + - `Scan()` method with nested loops needs explaining comments + - Algorithm: finds MP4 files, reads metadata, fetches missing episodes + - Comment needed explaining the overall flow + - Loop structure needs explanation + +**Download Progress Tracking** + +- [ ] `src/core/SeriesApp.py` lines 200-270 + - `wrapped_callback()` function has complex state tracking + - Explains cancellation checking, callback chaining, progress reporting + - Comments should explain why multiple callbacks are needed + - Line 245-260 needs comments explaining callback wrapping logic + +**Provider Discovery and Setup** + +- [ ] `src/core/providers/enhanced_provider.py` lines 200-240 + - Multiple parsing strategies without explanation + - Should comment why 3 strategies are tried + - Line 215-230 needs comment block explaining each strategy + - Should explain what error conditions trigger each fallback + +**Download Queue Priority Handling** + +- [ ] `src/server/services/download_service.py` lines 200-250 + - Priority-based queue insertion needs comment explaining algorithm + - `appendleft()` for HIGH priority not explained + - Lines 220-230 should have comment block for priority logic + +**Rate Limiting Algorithm** + +- [ ] `src/server/middleware/auth.py` lines 40-65 + - Time-window based rate limiting needs explanation + - Window reset logic (line 50-53) needs comment + - Calculation at line 55 needs explanation + +**Episode Discovery Pattern Matching** + +- [ ] `src/core/providers/streaming/*.py` files + - Regex patterns for extracting streams (complex) + - Each provider has different extraction logic + - Need comments explaining regex patterns + - Example: `src/core/providers/streaming/doodstream.py` line 35 + +**JSON/HTML Parsing Logic** + +- [ ] `src/core/providers/enhanced_provider.py` lines 215-235 + - `_parse_anime_response()` method has 3 parsing strategies + - Needs comment explaining fallback order + - Each lambda strategy should be explained + +**Session Retry Configuration** + +- [ ] `src/core/providers/enhanced_provider.py` lines 108-125 + - Session retry configuration with backoff + - Line 115: status codes list needs explanation + - Comment should explain why these specific codes need retry + +--- + +### 5️⃣ No Shortcuts or Hacks Used + +#### Code Smells and Shortcuts + +**Bare Pass Statements (Incomplete Implementation)** + +- [ ] `src/server/utils/dependencies.py` lines 223-235 + - `TODO: Implement rate limiting logic` - bare pass + - `TODO: Implement request logging logic` - bare pass + - These are incomplete - either implement or remove +- [ ] `src/server/utils/system.py` line 255 + - Bare `pass` in exception handler - should log error +- [ ] `src/core/providers/streaming/Provider.py` line 7 + - Abstract method implementation should not have pass +- [ ] `src/core/providers/streaming/hanime.py` line 86 + - Incomplete exception handling with pass + +**Duplicate Code** + +- [ ] `src/cli/Main.py` vs `src/core/SeriesApp.py` + - Entire `SeriesApp` class duplicated (89 lines vs 590 lines) + - CLI version is oversimplified - should use core version + - Line 35-50 in CLI duplicates core initialization logic +- [ ] `src/core/providers/aniworld_provider.py` vs `src/core/providers/enhanced_provider.py` + - Headers dictionary duplicated (lines 38-50 similar) + - Provider list duplicated (line 38 vs line 45) + - User-Agent strings duplicated + +**Global Variables (Temporary Storage)** + +- [ ] `src/server/fastapi_app.py` line 73 + - `series_app: Optional[SeriesApp] = None` global storage + - Should use FastAPI dependency injection instead + - Problematic for testing and multiple instances + +**Logging Configuration Workarounds** + +- [ ] `src/cli/Main.py` lines 12-22 + - Manual logger handler removal is hacky + - `for h in logging.root.handlers: logging.root.removeHandler(h)` is a hack + - Should use proper logging configuration + - Multiple loggers created with file handlers at odd paths (line 26) + +**Hardcoded Values** + +- [ ] `src/core/providers/aniworld_provider.py` line 22 + - `timeout = int(os.getenv("DOWNLOAD_TIMEOUT", 600))` at module level + - Should be in settings class +- [ ] `src/core/providers/aniworld_provider.py` lines 38, 47 + - User-Agent strings hardcoded + - Provider list hardcoded +- [ ] `src/cli/Main.py` line 227 + - Network path hardcoded: `"\\\\sshfs.r\\ubuntu@192.168.178.43\\media\\serien\\Serien"` + - Should be configuration + +**Exception Handling Shortcuts** + +- [ ] `src/core/providers/enhanced_provider.py` lines 410-421 + - Bare `except Exception:` without specific types (line 418) + - Multiple overlapping exception handlers (lines 410-425) + - Should use specific exception hierarchy +- [ ] `src/server/api/anime.py` lines 35-39 + - Bare `except Exception:` handlers should specify types +- [ ] `src/server/models/config.py` line 93 + - `except ValidationError: pass` - silently ignores error + +**Type Casting Workarounds** + +- [ ] `src/server/api/download.py` line 52 + - Complex `.model_dump(mode="json")` for serialization + - Should use proper model serialization methods +- [ ] `src/server/utils/dependencies.py` line 36 + - Type casting with `.get()` and defaults scattered throughout + +**Conditional Hacks** + +- [ ] `src/server/utils/dependencies.py` line 260 + - `running_tests = "PYTEST_CURRENT_TEST" in os.environ or "pytest" in sys.modules` + - Hacky test detection - should use proper test mode flag + +--- + +### 6️⃣ Security Considerations Addressed + +#### Authentication & Authorization + +**Weak CORS Configuration** + +- [ ] `src/server/fastapi_app.py` line 48 + - `allow_origins=["*"]` allows any origin + - **HIGH RISK** in production + - Should be: `allow_origins=settings.allowed_origins` (environment-based) +- [ ] No CORS rate limiting by origin + +**Missing Authorization Checks** + +- [ ] `src/server/middleware/auth.py` lines 81-86 + - Silent failure on missing auth for protected endpoints + - Should consistently return 401 status + - Some endpoints might bypass auth silently + +**In-Memory Session Storage** + +- [ ] `src/server/services/auth_service.py` line 51 + - In-memory `_failed` dict resets on restart + - Attacker can restart process to bypass rate limiting + - Should use Redis or database +- [ ] Line 51 comment: "For now we update only in-memory" + - Indicates incomplete security implementation + +#### Input Validation + +**Unvalidated User Input** + +- [ ] `src/cli/Main.py` line 80 + - User input for file paths not validated + - Could allow path traversal attacks +- [ ] `src/core/SerieScanner.py` line 37 + - Directory path `basePath` not validated + - Could read files outside intended directory +- [ ] `src/server/api/anime.py` line 70 + - Search query not validated for injection +- [ ] `src/core/providers/aniworld_provider.py` line 300+ + - URL parameters not sanitized + +**Missing Parameter Validation** + +- [ ] `src/core/providers/enhanced_provider.py` line 280 + - Season/episode validation present but minimal +- [ ] `src/server/database/models.py` + - No length validation on string fields + - No range validation on numeric fields + +#### Secrets and Credentials + +**Hardcoded Secrets** + +- [ ] `src/config/settings.py` line 9 + - `jwt_secret_key: str = Field(default="your-secret-key-here", env="JWT_SECRET_KEY")` + - Default secret exposed in code + - Should have NO default, or random default +- [ ] `.env` file might contain secrets (if exists) + - Should be in .gitignore + +**Plaintext Password Storage** + +- [ ] `src/config/settings.py` line 12 + - `master_password: Optional[str]` stored in env (development only) + - Should NEVER be used in production + - Add bold warning comment + +**Master Password Implementation** + +- [ ] `src/server/services/auth_service.py` line 71 + - Minimum 8 character password requirement documented + - Should enforce stronger requirements (uppercase, numbers, symbols) + +#### Data Protection + +**No Encryption of Sensitive Data** + +- [ ] Downloaded files not verified with checksums +- [ ] No integrity checking of stored data +- [ ] No encryption of sensitive config values + +**File Permission Issues** + +- [ ] `src/core/providers/aniworld_provider.py` line 26 + - Log file created with default permissions + - Path: `"../../download_errors.log"` - relative path is unsafe + - Should use absolute paths with secure permissions + +**Logging of Sensitive Data** + +- [ ] Check all `logger.debug()` calls for parameter logging + - URLs might contain API keys + - Search queries might contain sensitive terms +- [ ] Example: `src/core/providers/enhanced_provider.py` line 260 + - `logger.debug()` might log URLs with sensitive data + +#### Network Security + +**Unvalidated External Connections** + +- [ ] `src/core/providers/aniworld_provider.py` line 60 + - HTTP retry configuration but no SSL verification flag check +- [ ] `src/core/providers/enhanced_provider.py` line 115 + - HTTP error codes 500-524 auto-retry without logging suspicious activity + +**Missing SSL/TLS Configuration** + +- [ ] Verify SSL certificate validation enabled +- [ ] Check for `verify=False` in requests calls (should be `True`) + +#### Database Security + +**No SQL Injection Protection** + +- [ ] Check `src/server/database/service.py` for parameterized queries + - Should use SQLAlchemy properly (appears to be OK) +- [ ] String interpolation in queries should not exist + +**No Database Access Control** + +- [ ] Single database user for all operations +- [ ] No row-level security +- [ ] No audit logging of data changes + +--- + +### 7️⃣ Performance Validated + +#### Algorithmic Efficiency Issues + +**File Scanning Performance** + +- [ ] `src/core/SerieScanner.py` line 105+ + - `__find_mp4_files()` - potential O(nΒ²) complexity + - Recursive directory traversal not profiled + - No caching or incremental scanning + - Large directories (>10K files) might cause timeout + +**Download Queue Processing** + +- [ ] `src/server/services/download_service.py` line 240 + - `self._pending_queue.remove(item)` - O(n) operation in deque + - Should use dict for O(1) lookup before removal + - Line 85-86: deque maxlen limits might cause data loss + +**Provider Search Performance** + +- [ ] `src/core/providers/enhanced_provider.py` line 220 + - Multiple parsing strategies tried sequentially + - Should fail fast on obvious errors instead of trying all 3 + - No performance metrics logged + +**String Operations** + +- [ ] `src/cli/Main.py` line 118 + - Nested `sum()` with comprehensions - O(n\*m) complexity + - `total_episodes = sum(sum(len(ep) for ep in serie.episodeDict.values()) for serie in series)` + - No streaming/generator pattern + +**Regular Expression Compilation** + +- [ ] `src/core/providers/streaming/doodstream.py` line 35 + - Regex patterns compiled on every call + - Should compile once at module level + - Example: `r"\$\.get\('([^']*\/pass_md5\/[^']*)'"` compiled repeatedly + +#### Resource Usage Issues + +**Memory Leaks/Unbounded Growth** + +- [ ] `src/server/middleware/auth.py` line 34 + - `self._rate: Dict[str, Dict[str, float]]` never cleaned + - Old IP addresses accumulate forever + - Solution: add timestamp-based cleanup +- [ ] `src/server/services/download_service.py` line 85-86 + - `deque(maxlen=100)` and `deque(maxlen=50)` drop old items + - Might lose important history + +**Connection Pool Configuration** + +- [ ] `src/server/database/connection.py` + - Check if connection pooling is configured + - No explicit pool size limits found + - Could exhaust database connections + +**Large Data Structure Initialization** + +- [ ] `src/cli/Main.py` line 118 + - Loading all series at once + - Should use pagination for large datasets + +#### Caching Opportunities + +**No Request Caching** + +- [ ] `src/server/api/anime.py` - endpoints hit database every time + - No caching headers set + - `@cache` decorator could be used +- [ ] `src/core/providers/enhanced_provider.py` + - Search results not cached + - Same search query hits network repeatedly + +**No Database Query Optimization** + +- [ ] `src/server/services/anime_service.py` + - No eager loading (selectinload) for relationships + - N+1 query problems likely +- [ ] `src/server/database/service.py` line 200+ + - Check for missing `.selectinload()` in queries + +#### Concurrent Request Handling + +**Thread Pool Sizing** + +- [ ] `src/server/services/download_service.py` line 85 + - `ThreadPoolExecutor(max_workers=max_concurrent_downloads)` + - Default is 2, should be configurable + - No queue depth limits + +**Async/Sync Blocking Calls** + +- [ ] `src/server/api/anime.py` line 30+ + - Series list operations might block + - Database queries appear async (OK) +- [ ] `src/server/services/auth_service.py` + - Methods are synchronous but called from async endpoints + - Should verify no blocking calls + +#### I/O Performance + +**Database Query Count** + +- [ ] `/api/v1/anime` endpoint + - Likely makes multiple queries for each series + - Should use single query with joins/eager loading + - Test with N series to find N+1 issues + +**File I/O Optimization** + +- [ ] `src/core/SerieScanner.py` line 140+ + - Each folder reads data file + - Could batch reads or cache + +**Network Request Optimization** + +- [ ] `src/core/providers/enhanced_provider.py` line 115 + - Retry strategy good + - No connection pooling verification + - Should check request timeout values + +#### Performance Metrics Missing + +- [ ] No performance monitoring for slow endpoints +- [ ] No database query logging +- [ ] No cache hit/miss metrics +- [ ] No background task performance tracking +- [ ] No file operation benchmarks + +--- + +## πŸ“‹ Issues by File and Category + +### Core Module Issues + +#### `src/cli/Main.py` + +- [ ] **PEP8**: Missing type hints on method parameters (lines 1-50) + - `search()` missing return type annotation + - `get_user_selection()` missing return type annotation + - `__InitList__()` missing docstring and type annotations +- [ ] **Code Quality**: Class `SeriesApp` duplicates core `SeriesApp` from `src/core/SeriesApp.py` + - Consider consolidating or using inheritance + - Line 35: `_initialization_count` duplicated state tracking +- [ ] **Type Hints**: `display_series()` doesn't validate if `serie.name` is `None` before using it +- [ ] **Documentation**: Missing comprehensive docstrings for class methods +- [ ] **Import Organization**: Imports not sorted (lines 1-11) - should follow isort convention +- [ ] **Error Handling**: `NoKeyFoundException` and `MatchNotFoundError` are bare except classes - need proper inheritance +- [ ] **Logging**: Logging configuration at module level should be in centralized config + +#### `src/core/SeriesApp.py` + +- [ ] **Type Hints**: Return type missing on multiple methods: + - `ReScan()` method + - `_handle_error()` method + - `_handle_completion()` method +- [ ] **Documentation**: `download_series()` method lacks detailed docstring explaining async callback behavior +- [ ] **Global State**: Line 73 - `series_app: Optional[SeriesApp] = None` in `fastapi_app.py` uses global state + - Should use dependency injection instead +- [ ] **Complexity**: `Scan()` method is complex (80+ lines) - should be broken into smaller methods +- [ ] **Error Context**: `_handle_error()` doesn't provide enough context about which operation failed + +#### `src/core/SerieScanner.py` + +- [ ] **Type Hints**: Line 66 - `folderDict: dict[str, Serie]` uses new dict syntax but Python 3.8 compatibility might be needed +- [ ] **Documentation**: `Scan()` method missing return type annotation (line 83+) +- [ ] **Code Quality**: `is_null_or_whitespace()` duplicates Python's `str.isspace()` - use built-in instead +- [ ] **Error Logging**: Lines 167-182 catch exceptions but only log, don't propagate context +- [ ] **Performance**: `__find_mp4_files()` might be inefficient for large directories - add progress callback + +#### `src/core/providers/base_provider.py` + +- [ ] **Type Hints**: All abstract methods lack type annotations: + - Line 7: `Search()` missing type hints on parameters + - Line 11: `IsLanguage()` parameters not typed + - Line 15: `Download()` missing return type annotation + - Line 19: `GetSiteKey()` missing return type + - Line 23: `GetTitle()` missing return type + - Line 27: `get_season_episode_count()` missing return type +- [ ] **Docstrings**: No docstrings explaining abstract contract for implementing classes +- [ ] **Documentation**: Abstract methods need clear parameter documentation + +#### `src/core/providers/streaming/Provider.py` + +- [ ] **Type Hints**: Line 7 - Return type annotation `(str, [str])` uses invalid syntax + - Should be `Tuple[str, List[str]]` with proper imports +- [ ] **Documentation**: Abstract method `GetLink()` missing docstring +- [ ] **Naming**: `DEFAULT_REQUEST_TIMEOUT` parameter should be type-hinted as `int` + +#### `src/core/providers/aniworld_provider.py` + +- [ ] **Import Organization**: Lines 1-18 - imports not sorted (violates isort) +- [ ] **Global State**: Lines 24-26 - Multiple logger instances created at module level + - Should use centralized logging system +- [ ] **Hardcoding**: Line 42 - User-Agent string hardcoded (also at line 47 for Firefox) + - Extract to configuration constants +- [ ] **Type Hints**: Missing type hints on: + - `__init__()` method parameters (no return type on implicit constructor) + - Class attribute type annotations (line 41-62) +- [ ] **Magic Strings**: Line 38 - Hardcoded list of provider names should be enum +- [ ] **Configuration**: Timeouts hardcoded at line 22 - should use settings + +#### `src/core/providers/enhanced_provider.py` + +- [ ] **Import Issue**: Line 27 - `from error_handler import ...` - file doesn't exist in repository + - This will cause ImportError at runtime +- [ ] **Type Hints**: Class constructor `__init__()` missing type annotations (lines 40-96) +- [ ] **Documentation**: Bare exception handlers at lines 418-419 - need specific exception types +- [ ] **Code Quality**: `with_error_recovery` decorator imported but usage unclear +- [ ] **Performance**: `_create_robust_session()` method not shown but likely creates multiple session objects + +#### `src/core/interfaces/providers.py` + +- [ ] Need to verify if any abstract methods lack type hints and docstrings + +#### `src/core/exceptions/Exceptions.py` + +- [ ] Need to verify custom exception hierarchy and documentation + +--- + +### Server Module Issues + +#### `src/server/fastapi_app.py` + +- [ ] **Global State**: Line 73 - `series_app: Optional[SeriesApp] = None` stored globally + - Use FastAPI dependency injection via `Depends()` +- [ ] **CORS Configuration**: Line 48 - `allow_origins=["*"]` is production security issue + - Add comment: "Configure appropriately for production" + - Extract to settings with environment-based defaults +- [ ] **Error Handling**: `startup_event()` at line 79 - missing try-except to handle initialization failures +- [ ] **Type Hints**: `startup_event()` function missing type annotations +- [ ] **Documentation**: `broadcast_callback()` function inside event handler should be extracted to separate function +- [ ] **Logging**: No error logging if `settings.anime_directory` is None + +#### `src/server/middleware/auth.py` + +- [ ] **Performance**: In-memory rate limiter (line 34) will leak memory - never cleans up old entries + - Need periodic cleanup or use Redis for production +- [ ] **Security**: Line 46 - Rate limiting only 60-second window, should be configurable +- [ ] **Type Hints**: `dispatch()` method parameters properly typed, but return type could be explicit +- [ ] **Documentation**: `_get_client_ip()` method incomplete (line 94+ truncated) +- [ ] **Error Handling**: Lines 81-86 - Silent failure if protected endpoint and no auth + - Should return 401 consistently + +#### `src/server/services/auth_service.py` + +- [ ] **Documentation**: Line 68 - Comment says "For now we update only in-memory" indicates incomplete implementation + - Create task to persist password hash to configuration file +- [ ] **Type Hints**: `_verify_password()` at line 60 - no return type annotation (implicit `bool`) +- [ ] **Security**: Line 71 - Minimum password length 8 characters, should be documented as security requirement +- [ ] **State Management**: In-memory `_failed` dict (line 51) resets on process restart + - Document this limitation and suggest Redis/database for production + +#### `src/server/database/service.py` + +- [ ] **Type Hints**: Multiple service methods lack return type annotations: + - Line 50 onwards - `create()` should annotate return type as `-> AnimeSeries` + - Other CRUD methods need return type annotations +- [ ] **Documentation**: Service layer methods need detailed docstrings explaining: + - Database constraints + - Transaction behavior + - Cascade delete behavior +- [ ] **Error Handling**: Methods don't specify which SQLAlchemy exceptions they might raise + +#### `src/server/database/models.py` + +- [ ] **Documentation**: Model relationships and cascade rules well-documented + - βœ… Type hints present and comprehensive (well done) +- [ ] **Validation**: No model-level validation before database insert + - Consider adding validators for constraints + +#### `src/server/services/download_service.py` + +- [ ] **Type Hints**: Constructor and methods properly typed - good! +- [ ] **Documentation**: Line 45+ - comprehensive docstrings present +- [ ] **Performance**: Line 85 - `deque(maxlen=100)` for completed items - is this appropriate for long-running service? +- [ ] **Thread Safety**: Uses `ThreadPoolExecutor` but thread-safety of queue operations not clear + +#### `src/server/utils/dependencies.py` + +- [ ] **TODO Comments**: Lines 223 and 233 - TODO comments for unimplemented features: + - "TODO: Implement rate limiting logic" + - "TODO: Implement request logging logic" + - Create separate task items for these +- [ ] **Documentation**: `CommonQueryParams` class at line 192 lacks docstring + +#### `src/server/utils/system.py` + +- [ ] **Exception Handling**: Line 255 - bare `pass` statement in exception handler + - Should at least log the exception + +#### `src/server/api/anime.py` + +- [ ] **Error Handling**: Lines 35-39 - Multiple bare `except Exception` handlers + - Need specific exception types and proper logging +- [ ] **Type Hints**: `get_anime()` missing return type annotation +- [ ] **Code Quality**: Lines 32-36 - Complex property access with `getattr()` chains + - Create helper function or model method to encapsulate +- [ ] **Documentation**: Endpoint docstrings are minimal + +--- + +### Models and Pydantic Issues + +#### `src/server/models/auth.py` + +- [ ] **Documentation**: Models well-documented - βœ… +- [ ] **Type Hints**: All properly annotated - βœ… + +#### `src/server/models/anime.py` + +- [ ] Need to review for type hints and docstrings + +#### `src/server/models/config.py` + +- [ ] **Error Handling**: Line 93 - `ValidationError` caught but only silently passed? + - Should log or re-raise with context + +#### `src/server/models/download.py` + +- [ ] Need to review for type hints and docstrings + +--- + +### Utility and Configuration Issues + +#### `src/config/settings.py` + +- [ ] **Security**: Line 12 - `master_password` field stored in environment during development + - Add warning comment: "NEVER use this in production" +- [ ] **Documentation**: Settings class needs comprehensive docstring explaining each field + +#### `src/infrastructure/logging/GlobalLogger.py` + +- [ ] Need to review logging configuration for consistency + +#### `src/server/utils/logging.py` + +- [ ] Need to review for type hints and consistency with global logging + +#### `src/server/utils/template_helpers.py` + +- [ ] Need to review for type hints and docstrings + +#### `src/server/utils/log_manager.py` + +- [ ] Need to review for type hints and error handling + +--- + +## πŸ”’ Security Issues + +### High Priority + +- [ ] **CORS Configuration** (`src/server/fastapi_app.py`, line 48) + - `allow_origins=["*"]` is insecure for production + - Add environment-based configuration +- [ ] **Missing Import Error** (`src/core/providers/enhanced_provider.py`, line 27) + + - `from error_handler import ...` file doesn't exist + - Will cause ImportError at runtime + +- [ ] **Global Password State** (`src/server/services/auth_service.py`, line 51) + - In-memory failure tracking resets on restart + - Recommend using persistent storage (database/Redis) + +### Medium Priority + +- [ ] **Rate Limiter Memory Leak** (`src/server/middleware/auth.py`, line 34) + + - Never cleans up old IP entries + - Add periodic cleanup or use Redis + +- [ ] **Missing Authorization Checks** (`src/server/middleware/auth.py`, lines 81-86) + - Some protected endpoints might silently allow unauthenticated access + +--- + +## πŸ“Š Code Style Issues + +### Import Organization + +- [ ] `src/cli/Main.py` - imports not sorted +- [ ] `src/core/providers/aniworld_provider.py` - imports not sorted +- [ ] `src/core/providers/enhanced_provider.py` - imports not sorted + +### Missing Type Hints (Summary) + +- [ ] `src/cli/Main.py` - ~10 methods without type hints +- [ ] `src/core/providers/base_provider.py` - All 6 abstract methods +- [ ] `src/core/providers/streaming/Provider.py` - Return type invalid syntax +- [ ] `src/core/providers/aniworld_provider.py` - Constructor and attributes +- [ ] `src/server/api/anime.py` - Several endpoint methods +- [ ] `src/server/utils/dependencies.py` - Several dependency functions + +### Documentation Gaps + +- [ ] Missing comprehensive module docstrings in several files +- [ ] Abstract method contracts not documented +- [ ] Complex algorithms need inline comments + +--- + +## 🎯 Specific Task Items + +### Import/Module Issues + +- [ ] Fix missing `error_handler` module import in `src/core/providers/enhanced_provider.py` +- [ ] Sort imports in `src/cli/Main.py` using isort +- [ ] Sort imports in `src/core/providers/aniworld_provider.py` using isort +- [ ] Sort imports in `src/core/providers/enhanced_provider.py` using isort + +### Type Hints - Phase 1: Core Providers + +- [ ] Add type hints to all parameters in `src/core/providers/base_provider.py` abstract methods +- [ ] Fix return type annotation in `src/core/providers/streaming/Provider.py` (use `Tuple[str, List[str]]`) +- [ ] Add `@property` type hints to `src/core/providers/aniworld_provider.py` class attributes +- [ ] Add type hints to `src/core/providers/enhanced_provider.py` constructor parameters + +### Type Hints - Phase 2: Services and API + +- [ ] Add return type annotations to methods in `src/server/services/download_service.py` +- [ ] Add return type annotations to methods in `src/server/services/auth_service.py` +- [ ] Add return type annotations to endpoints in `src/server/api/anime.py` +- [ ] Add type hints to dependency functions in `src/server/utils/dependencies.py` + +### Type Hints - Phase 3: CLI and Core + +- [ ] Add type hints to methods in `src/cli/Main.py` +- [ ] Add return type annotations to methods in `src/core/SeriesApp.py` +- [ ] Add return type annotations to methods in `src/core/SerieScanner.py` + +### Documentation - Phase 1: Critical Sections + +- [ ] Add comprehensive docstring to `src/core/SeriesApp.Scan()` method +- [ ] Document abstract method contracts in `src/core/providers/base_provider.py` +- [ ] Add docstring to `src/server/utils/dependencies.CommonQueryParams` class +- [ ] Document database transaction behavior in `src/server/database/service.py` + +### Documentation - Phase 2: Endpoints + +- [ ] Expand docstrings on endpoints in `src/server/api/anime.py` +- [ ] Add parameter descriptions to endpoint handlers +- [ ] Document expected exceptions and error responses + +### Code Quality - Phase 1: Consolidation + +- [ ] Investigate `SeriesApp` duplication between `src/cli/Main.py` and `src/core/SeriesApp.py` +- [ ] Consider consolidating into single implementation +- [ ] Update CLI to use core module instead of duplicate + +### Code Quality - Phase 2: Exception Handling + +- [ ] Add specific exception types to bare `except:` handlers +- [ ] Add logging to all exception handlers +- [ ] Document exception context and causes +- [ ] Review exception handling in `src/core/providers/enhanced_provider.py` (lines 410-421) + +### Code Quality - Phase 3: Refactoring + +- [ ] Extract `broadcast_callback()` from `startup_event()` in `src/server/fastapi_app.py` +- [ ] Break down complex `Scan()` method in `src/core/SerieScanner.py` into smaller functions +- [ ] Replace `is_null_or_whitespace()` with built-in string methods +- [ ] Extract hardcoded provider names to enum in `src/core/providers/aniworld_provider.py` + +### Security - Phase 1: Critical Fixes + +- [ ] Fix missing `error_handler` import error in `src/core/providers/enhanced_provider.py` +- [ ] Make CORS configuration environment-based in `src/server/fastapi_app.py` +- [ ] Add startup validation to ensure `anime_directory` is configured + +### Security - Phase 2: Improvements + +- [ ] Implement Redis-based rate limiter instead of in-memory in `src/server/middleware/auth.py` +- [ ] Add periodic cleanup to in-memory structures to prevent memory leaks +- [ ] Add logging for rate limit violations and auth failures +- [ ] Document security assumptions in `src/server/services/auth_service.py` + +### Performance - Phase 1: Validation + +- [ ] Profile `SerieScanner.__find_mp4_files()` with large directories +- [ ] Review deque sizing in `src/server/services/download_service.py` (lines 85-86) +- [ ] Verify thread-safety of queue operations + +### Performance - Phase 2: Optimization + +- [ ] Add pagination to anime list endpoint if dataset is large +- [ ] Consider caching for search results in `src/core/providers/aniworld_provider.py` +- [ ] Review session creation overhead in provider initialization + +### Configuration Issues + +- [ ] Extract hardcoded timeouts from `src/core/providers/aniworld_provider.py` line 22 to settings +- [ ] Extract User-Agent strings to configuration constants +- [ ] Document all configuration options in settings module +- [ ] Add validation for required environment variables + +### Logging Issues + +- [ ] Centralize logger creation across all modules +- [ ] Remove module-level logger instantiation where possible +- [ ] Document logging levels expected for each component +- [ ] Review `src/cli/Main.py` logging configuration (lines 12-22) - appears to suppress all logging + +### Testing/Comments + +- [ ] Add inline comments explaining complex regex patterns in providers +- [ ] Add comments explaining retry logic and backoff strategies +- [ ] Document callback behavior and expected signatures +- [ ] Add comments to clarify WebSocket broadcast mechanisms + +--- + +## πŸ“Œ Implementation Notes + +### Dependencies to Verify + +- [ ] `error_handler` module - currently missing, causing import error +- [ ] All Pydantic models properly imported in service layers +- [ ] SQLAlchemy session management properly scoped + +### Configuration Management + +- [ ] Review `src/config/settings.py` for completeness +- [ ] Ensure all configurable values are in settings, not hardcoded +- [ ] Document all environment variables needed + +### Testing Coverage + +- [ ] Verify tests cover exception paths in `src/server/api/anime.py` +- [ ] Add tests for CORS configuration +- [ ] Test rate limiting behavior in middleware +- [ ] Test database transaction rollback scenarios + +--- + +## πŸ”„ Validation Checklist Before Committing + +For each issue fixed: + +- [ ] Run Pylance to verify type hints are correct +- [ ] Run `isort` on modified files to sort imports +- [ ] Run `black` to format code to PEP8 standards +- [ ] Run existing unit tests to ensure no regression +- [ ] Verify no new security vulnerabilities introduced +- [ ] Update docstrings if behavior changed +- [ ] Document any breaking API changes + +--- + +**Total Issues Identified**: ~90 individual items across 8 categories +**Priority Distribution**: 5 High | 15 Medium | 70 Low/Nice-to-have +**Estimated Effort**: 40-60 hours for comprehensive quality improvement diff --git a/instructions.md b/instructions.md index a43c72e..d104d97 100644 --- a/instructions.md +++ b/instructions.md @@ -80,6 +80,11 @@ This checklist ensures consistent, high-quality task execution across implementa ## 1. Implementation & Code Quality +create a QualityTODO.md file. Write down all issues you found for each file in the project. +Write down the task in QualityTODO.md make single and small task. Do not write down code. +Use the following todo list: + +- [ ] run pylance and list all issues - [ ] Code follows PEP8 and project coding standards - [ ] Type hints used where applicable - [ ] Clear, self-documenting code written