From 60ffc12e8c03357b968524fc459f2e48dd581995 Mon Sep 17 00:00:00 2001 From: "IGNY8 VPS (Salman)" Date: Sun, 16 Nov 2025 09:15:07 +0000 Subject: [PATCH] Add AI framework refactoring plan and orphan code audit - Add REFACTORING-PLAN.md: Plan to remove MODEL_CONFIG and Django settings fallbacks - Add ORPHAN-CODE-AUDIT.md: Audit of unused code and exports - Update CHANGELOG.md: Document API Standard v1.0 completion - Update documentation files --- CHANGELOG.md | 27 + backend/igny8_core/ai/ORPHAN-CODE-AUDIT.md | 262 ++++++++ backend/igny8_core/ai/REFACTORING-PLAN.md | 520 ++++++++++++++++ docs/DOCUMENTATION-SUMMARY.md | 3 + unified-api/API-STANDARD-v1.0.md | 10 +- unified-api/IMPLEMENTATION-PLAN-REMAINING.md | 593 ------------------- 6 files changed, 821 insertions(+), 594 deletions(-) create mode 100644 backend/igny8_core/ai/ORPHAN-CODE-AUDIT.md create mode 100644 backend/igny8_core/ai/REFACTORING-PLAN.md delete mode 100644 unified-api/IMPLEMENTATION-PLAN-REMAINING.md diff --git a/CHANGELOG.md b/CHANGELOG.md index dbbae09e..e4c41f23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,18 +31,38 @@ Each entry follows this format: - API Monitor page for endpoint health monitoring - CRUD operations monitoring for Planner and Writer modules - Sidebar API status indicator for aws-admin accounts +- **Health Check Endpoint**: `GET /api/v1/system/ping/` - Public health check endpoint per API Standard v1.0 requirement + - Returns unified format: `{success: true, data: {status: 'ok'}}` + - Tagged as 'System' in Swagger/ReDoc documentation + - Public endpoint (AllowAny permission) ### Changed - All API endpoints now return unified response format (`{success, data, message, errors}`) - Frontend `fetchAPI` wrapper automatically extracts data from unified format - All error responses follow unified format with `request_id` tracking - Rate limiting configured with scoped throttles per module +- **Integration Views**: All integration endpoints now use unified response format + - Replaced 40+ raw `Response()` calls with `success_response()`/`error_response()` helpers + - All responses include `request_id` for tracking + - Updated frontend components to handle extracted data format +- **API Documentation**: Updated Swagger/ReDoc description to include all public endpoints + - Added `/api/v1/system/ping/` to public endpoints list + - Updated schema extensions to properly tag ping endpoint ### Fixed - Keyword edit form now correctly populates existing values - Auto-cluster function now works correctly with unified API format - ResourceDebugOverlay now correctly extracts data from unified API responses - All frontend pages now correctly handle unified API response format +- **Integration Views**: Fixed all integration endpoints not using unified response format + - `_test_openai()` and `_test_runware()` methods now use unified format + - `generate_image()`, `create()`, `save_settings()` methods now use unified format + - `get_image_generation_settings()` and `task_progress()` methods now use unified format + - All error responses now include `request_id` and follow unified format +- **Frontend Components**: Updated to work with unified format + - `ValidationCard.tsx` - Removed dual-format handling, now works with extracted data + - `Integration.tsx` - Simplified to work with unified format + - `ImageGenerationCard.tsx` - Updated to work with extracted data format --- @@ -82,6 +102,13 @@ Each entry follows this format: ### Documentation - Updated implementation plan to reflect completion of all remaining API Standard v1.0 items - All 8 remaining items from audit completed (100% compliance achieved) +- **API Standard v1.0**: Full compliance achieved + - All 10 audit tasks completed and verified + - All custom @action methods use unified response format + - All ViewSets use proper base classes, pagination, throttles, and permissions + - All error responses include `request_id` tracking + - No raw `Response()` calls remaining (except file downloads) + - All endpoints documented in Swagger/ReDoc with proper tags --- diff --git a/backend/igny8_core/ai/ORPHAN-CODE-AUDIT.md b/backend/igny8_core/ai/ORPHAN-CODE-AUDIT.md new file mode 100644 index 00000000..9eea0fc2 --- /dev/null +++ b/backend/igny8_core/ai/ORPHAN-CODE-AUDIT.md @@ -0,0 +1,262 @@ +# Orphan Code and Unused Files Audit +## AI Framework - Complete Analysis + +**Date:** 2025-01-XX +**Status:** Audit Complete +**Purpose:** Identify orphan AI functions, unused files, and code not part of active processes + +--- + +## Summary + +### Active AI Functions: 5 ✅ +All registered functions are actively used from views. + +### Orphan Functions: 0 ✅ +No registered functions are unused. + +### Unused Files: 0 ✅ +All files in `/ai/` folder are part of active chain. + +### Orphan Code/Exports: 4 ❌ +Functions exported in `__init__.py` but never imported/used. + +### Parallel/Old Code: 1 ⚠️ +Old `utils/ai_processor.py` still used for testing (parallel system). + +--- + +## Active AI Functions (5 Total) + +| Function Name | View/Endpoint | Function File | Status | +|--------------|---------------|---------------|--------| +| `auto_cluster` | `planner/views.py` → `KeywordViewSet.auto_cluster()` | `functions/auto_cluster.py` | ✅ Active | +| `auto_generate_ideas` → `generate_ideas` | `planner/views.py` → `ClusterViewSet.auto_generate_ideas()` | `functions/generate_ideas.py` | ✅ Active | +| `generate_content` | `writer/views.py` → `TaskViewSet.auto_generate_content()` | `functions/generate_content.py` | ✅ Active | +| `generate_image_prompts` | `writer/views.py` → `ContentViewSet.generate_image_prompts()` | `functions/generate_image_prompts.py` | ✅ Active | +| `generate_images` | `writer/views.py` → `ImageViewSet.generate_images()` | `functions/generate_images.py` | ✅ Active | + +**Result:** All 5 registered functions are actively used. No orphan functions. + +--- + +## Files in `/ai/` Folder - Usage Analysis + +### Core Framework Files (All Active ✅) + +| File | Purpose | Used By | Status | +|------|---------|---------|--------| +| `tasks.py` | Celery task entry point | `planner/views.py`, `writer/views.py` | ✅ Active | +| `engine.py` | AI function orchestrator | `tasks.py` | ✅ Active | +| `ai_core.py` | AI request handler | `engine.py`, `functions/*.py` | ✅ Active | +| `base.py` | Base function class | All `functions/*.py` | ✅ Active | +| `registry.py` | Function registry | `tasks.py`, `engine.py` | ✅ Active | +| `prompts.py` | Prompt management | All `functions/*.py` | ✅ Active | +| `tracker.py` | Progress tracking | `engine.py` | ✅ Active | +| `validators.py` | Validation helpers | All `functions/*.py` | ✅ Active | +| `settings.py` | Model configuration | `engine.py`, `ai_core.py` | ✅ Active | +| `constants.py` | Constants (rates, models) | `ai_core.py`, `validators.py`, `utils/ai_processor.py` | ✅ Active | +| `models.py` | AITaskLog model | `engine.py` | ✅ Active | +| `admin.py` | Django admin | Django admin interface | ✅ Active | +| `apps.py` | Django app config | Django framework | ✅ Active | +| `__init__.py` | Package exports | Various imports | ✅ Active | + +**Result:** All files in `/ai/` folder are part of active chain. No unused files. + +--- + +## Function Files (All Active ✅) + +| File | Function Class | Registered | Used From View | Status | +|------|---------------|------------|----------------|--------| +| `functions/auto_cluster.py` | `AutoClusterFunction` | ✅ | `planner/views.py` | ✅ Active | +| `functions/generate_ideas.py` | `GenerateIdeasFunction` | ✅ | `planner/views.py` | ✅ Active | +| `functions/generate_content.py` | `GenerateContentFunction` | ✅ | `writer/views.py` | ✅ Active | +| `functions/generate_image_prompts.py` | `GenerateImagePromptsFunction` | ✅ | `writer/views.py` | ✅ Active | +| `functions/generate_images.py` | `GenerateImagesFunction` | ✅ | `writer/views.py` | ✅ Active | + +**Result:** All 5 function files are registered and actively used. No orphan functions. + +--- + +## Orphan Code - Exported But Never Used + +### 1. `get_model()` Function ❌ + +**Location:** `settings.py` line 106-109 +**Exported in:** `__init__.py` line 69 +**Used:** ❌ Never imported or called anywhere + +```python +def get_model(function_name: str) -> str: + """Get model name for function""" + config = get_model_config(function_name) + return config.get("model", "gpt-4.1") +``` + +**Recommendation:** Remove from `__init__.py` exports and delete function (or keep if planned for future use). + +--- + +### 2. `get_max_tokens()` Function ❌ + +**Location:** `settings.py` line 112-115 +**Exported in:** `__init__.py` line 70 +**Used:** ❌ Never imported or called anywhere + +```python +def get_max_tokens(function_name: str) -> int: + """Get max tokens for function""" + config = get_model_config(function_name) + return config.get("max_tokens", 4000) +``` + +**Recommendation:** Remove from `__init__.py` exports and delete function (or keep if planned for future use). + +--- + +### 3. `get_temperature()` Function ❌ + +**Location:** `settings.py` line 118-121 +**Exported in:** `__init__.py` line 71 +**Used:** ❌ Never imported or called anywhere + +```python +def get_temperature(function_name: str) -> float: + """Get temperature for function""" + config = get_model_config(function_name) + return config.get("temperature", 0.7) +``` + +**Recommendation:** Remove from `__init__.py` exports and delete function (or keep if planned for future use). + +--- + +### 4. `register_function()` Function ❌ + +**Location:** `registry.py` line 15-21 +**Exported in:** `__init__.py` line 44 +**Used:** ❌ Never called anywhere (only `register_lazy_function` is used) + +```python +def register_function(name: str, function_class: Type[BaseAIFunction]): + """Register an AI function""" + if not issubclass(function_class, BaseAIFunction): + raise ValueError(f"{function_class} must inherit from BaseAIFunction") + + _FUNCTION_REGISTRY[name] = function_class + logger.info(f"Registered AI function: {name}") +``` + +**Recommendation:** Keep for potential future use (direct registration), but remove from `__init__.py` exports if not needed. + +--- + +### 5. `list_functions()` Function ❌ + +**Location:** `registry.py` line 50-52 +**Exported in:** `__init__.py` line 46 +**Used:** ❌ Never called anywhere + +```python +def list_functions() -> list: + """List all registered functions""" + return list(_FUNCTION_REGISTRY.keys()) +``` + +**Recommendation:** Keep for debugging/admin purposes, but remove from `__init__.py` exports if not needed. + +--- + +## Internal Helper Functions (Not Orphan ✅) + +### `extract_image_prompts` Config Entry ✅ + +**Location:** `settings.py` line 31-36 (MODEL_CONFIG) +**Used by:** `functions/generate_images.py` line 116, 126, 135 +**Status:** ✅ Active (internal helper, not a registered function) + +This is **NOT** an orphan. It's an internal config entry used by `GenerateImagesFunction` to get model config for extracting image prompts. It's not a registered AI function, just a config key. + +**Recommendation:** Keep - it's part of active chain. + +--- + +## Parallel/Old Code System + +### `utils/ai_processor.py` ⚠️ + +**Location:** `/backend/igny8_core/utils/ai_processor.py` +**Status:** ⚠️ Still used, but parallel to new AI framework +**Used by:** `modules/system/integration_views.py` (image generation testing) + +**Details:** +- Old AI processing system (pre-framework) +- Still used in `integration_views.py` for: + - Image generation testing (`generate_image()` method) + - Model rates display (`MODEL_RATES` import) +- Parallel to new AI framework (not part of it) +- Has deprecated methods: `cluster_keywords()` (line 1070 warns it's deprecated) + +**Recommendation:** +- **Option 1:** Keep for now (used in integration testing) +- **Option 2:** Refactor `integration_views.py` to use new AI framework instead +- **Option 3:** Mark as deprecated and plan migration + +**Note:** This is outside `/ai/` folder, so not part of this audit scope, but worth noting. + +--- + +## Summary Table + +| Category | Count | Status | +|----------|-------|--------| +| **Active AI Functions** | 5 | ✅ All used | +| **Orphan AI Functions** | 0 | ✅ None found | +| **Unused Files in `/ai/`** | 0 | ✅ All active | +| **Orphan Exports** | 4 | ❌ `get_model()`, `get_max_tokens()`, `get_temperature()`, `register_function()`, `list_functions()` | +| **Internal Helpers** | 1 | ✅ `extract_image_prompts` (active) | +| **Parallel Systems** | 1 | ⚠️ `utils/ai_processor.py` (old code) | + +--- + +## Recommendations + +### High Priority + +1. **Remove Orphan Exports from `__init__.py`** + - Remove `get_model`, `get_max_tokens`, `get_temperature` from exports + - These functions are never used and add confusion + +2. **Clean Up `settings.py`** + - After removing MODEL_CONFIG (per refactoring plan), these helper functions become even less useful + - Consider removing them entirely or keeping only if needed for future use + +### Medium Priority + +3. **Review `register_function()` and `list_functions()`** + - Decide if these are needed for future direct registration + - If not needed, remove from exports + - If needed, document their purpose + +### Low Priority + +4. **Consider Migrating `utils/ai_processor.py`** + - Refactor `integration_views.py` to use new AI framework + - Remove old `ai_processor.py` system + - This is a larger refactoring task + +--- + +## Action Items + +- [ ] Remove `get_model`, `get_max_tokens`, `get_temperature` from `__init__.py` exports +- [ ] Delete or comment out unused helper functions in `settings.py` +- [ ] Review and decide on `register_function()` and `list_functions()` exports +- [ ] Document decision on `utils/ai_processor.py` migration (future work) + +--- + +**Last Updated:** 2025-01-XX +**Status:** Audit Complete - Ready for Cleanup + diff --git a/backend/igny8_core/ai/REFACTORING-PLAN.md b/backend/igny8_core/ai/REFACTORING-PLAN.md new file mode 100644 index 00000000..6cd36f9c --- /dev/null +++ b/backend/igny8_core/ai/REFACTORING-PLAN.md @@ -0,0 +1,520 @@ +# AI Framework Refactoring Plan +## Remove Hardcoded Model Defaults - IntegrationSettings Only + +**Date:** 2025-01-XX +**Status:** Planning +**Goal:** +1. Remove Priority 2 (MODEL_CONFIG) and Priority 3 (Django settings) fallbacks. Use only IntegrationSettings (account-specific configuration). +2. Clean up orphan code: Remove unused exports and helper functions. + +--- + +## Overview + +### Current Problem +The AI framework has a 3-tier fallback system: +1. **Priority 1:** IntegrationSettings (account-specific) ✅ Keep +2. **Priority 2:** MODEL_CONFIG hardcoded defaults ❌ Remove +3. **Priority 3:** Django settings DEFAULT_AI_MODEL ❌ Remove + +### Target State +- **Single Source:** IntegrationSettings only (account-specific) +- **No Fallbacks:** If IntegrationSettings not configured → raise clear error +- **Account-Specific:** Each account must configure their own models + +--- + +## Active AI Functions (5 Total) + +These are the only functions that need to work after refactoring: + +| Function Name | View/Endpoint | Function File | Status | +|--------------|---------------|---------------|--------| +| `auto_cluster` | `planner/views.py` → `KeywordViewSet.auto_cluster()` | `functions/auto_cluster.py` | ✅ Active | +| `auto_generate_ideas` → `generate_ideas` | `planner/views.py` → `ClusterViewSet.auto_generate_ideas()` | `functions/generate_ideas.py` | ✅ Active | +| `generate_content` | `writer/views.py` → `TaskViewSet.auto_generate_content()` | `functions/generate_content.py` | ✅ Active | +| `generate_image_prompts` | `writer/views.py` → `ContentViewSet.generate_image_prompts()` | `functions/generate_image_prompts.py` | ✅ Active | +| `generate_images` | `writer/views.py` → `ImageViewSet.generate_images()` | `functions/generate_images.py` | ✅ Active | + +**Note:** `process_image_generation_queue` is a Celery task (not an AI function) - excluded from this refactoring. + +--- + +## Files to Modify + +### 1. `settings.py` - Model Configuration + +**Current Issues:** +- Lines 7-43: `MODEL_CONFIG` dict with hardcoded defaults +- Line 71: Gets config from `MODEL_CONFIG` first +- Lines 96-103: Falls back to `default_config` if IntegrationSettings not found + +**Changes Required:** + +#### Remove MODEL_CONFIG Dict (Lines 7-43) +```python +# REMOVE THIS ENTIRE SECTION: +MODEL_CONFIG = { + "auto_cluster": { + "model": "gpt-4o-mini", + "max_tokens": 4000, + "temperature": 0.7, + }, + "generate_ideas": { + "model": "gpt-4o-mini", + "max_tokens": 4000, + "temperature": 0.7, + }, + # ... rest of dict +} +``` + +#### Update `get_model_config()` Function (Lines 55-103) + +**Current Logic:** +```python +def get_model_config(function_name, account=None): + # 1. Get from MODEL_CONFIG (hardcoded) ❌ + config = MODEL_CONFIG.get(actual_name, {}).copy() + + # 2. Try IntegrationSettings (if account provided) + if account: + # ... get from IntegrationSettings + if model_from_settings: + config['model'] = model_from_settings # Override + + # 3. Fallback to default_config ❌ + if not config.get('model'): + default_config = {"model": "gpt-4.1", ...} + config.update(default_config) + + return config +``` + +**New Logic:** +```python +def get_model_config(function_name, account=None): + """ + Get model configuration from IntegrationSettings only. + No fallbacks - account must have IntegrationSettings configured. + + Args: + function_name: Name of the AI function + account: Account instance (required) + + Returns: + dict: Model configuration with 'model', 'max_tokens', 'temperature' + + Raises: + ValueError: If account not provided or IntegrationSettings not configured + """ + if not account: + raise ValueError("Account is required for model configuration") + + # Resolve function alias + actual_name = FUNCTION_ALIASES.get(function_name, function_name) + + # Get IntegrationSettings for OpenAI + try: + integration_settings = IntegrationSettings.objects.get( + integration_type='openai', + account=account, + is_active=True + ) + except IntegrationSettings.DoesNotExist: + raise ValueError( + f"OpenAI IntegrationSettings not configured for account {account.id}. " + f"Please configure OpenAI settings in the integration page." + ) + + config = integration_settings.config or {} + + # Get model from config + model = config.get('model') + if not model: + raise ValueError( + f"Model not configured in IntegrationSettings for account {account.id}. " + f"Please set 'model' in OpenAI integration settings." + ) + + # Get max_tokens and temperature from config (with reasonable defaults for API) + max_tokens = config.get('max_tokens', 4000) # Reasonable default for API limits + temperature = config.get('temperature', 0.7) # Reasonable default + + return { + 'model': model, + 'max_tokens': max_tokens, + 'temperature': temperature, + } +``` + +**Key Changes:** +- Remove `MODEL_CONFIG` lookup +- Require `account` parameter (no optional) +- Raise `ValueError` if IntegrationSettings not found +- Raise `ValueError` if model not configured +- No fallbacks to hardcoded defaults + +--- + +### 2. `ai_core.py` - Default Model Fallback + +**Current Issues:** +- Lines 84-90: `_default_model` initialized from Django settings +- Line 159: Falls back to `self._default_model` if model not provided + +**Changes Required:** + +#### Remove `_default_model` Initialization (Lines 84-90) +```python +# REMOVE THIS: +try: + from django.conf import settings + self._default_model = getattr(settings, 'DEFAULT_AI_MODEL', 'gpt-4.1') +except: + self._default_model = 'gpt-4.1' +``` + +#### Update `run_ai_request()` Method (Line 159) + +**Current Logic:** +```python +active_model = model or self._default_model # ❌ Fallback +``` + +**New Logic:** +```python +if not model: + raise ValueError("Model is required. Ensure IntegrationSettings is configured for the account.") +active_model = model +``` + +**Key Changes:** +- Remove `_default_model` attribute +- Require `model` parameter (no fallback) +- Raise `ValueError` if model not provided + +--- + +### 3. `engine.py` - Model Configuration Call + +**Current Issues:** +- Line 206: Calls `get_model_config(function_name, account=self.account)` +- May pass `account=None` in some cases + +**Changes Required:** + +#### Ensure Account is Always Passed (Line 206) +```python +# Current: +model_config = get_model_config(function_name, account=self.account) + +# New (same, but ensure account is not None): +if not self.account: + raise ValueError("Account is required for AI function execution") + +model_config = get_model_config(function_name, account=self.account) +``` + +**Key Changes:** +- Validate `self.account` exists before calling `get_model_config()` +- Raise clear error if account missing + +--- + +### 4. `tasks.py` - Task Entry Point + +**Current Issues:** +- May not always have account context + +**Changes Required:** + +#### Ensure Account is Always Available (Line ~50) +```python +# In run_ai_task() function, ensure account is always set: +if not account_id: + raise ValueError("account_id is required for AI task execution") + +account = Account.objects.get(id=account_id) +``` + +**Key Changes:** +- Validate `account_id` is provided +- Ensure account exists before proceeding + +--- + +## Orphan Code Cleanup + +Based on the audit in `ORPHAN-CODE-AUDIT.md`, the following orphan code should be removed: + +### 5. `__init__.py` - Remove Orphan Exports + +**Current Issues:** +- Lines 69-71: Export `get_model`, `get_max_tokens`, `get_temperature` - never imported/used +- Line 44: Export `register_function` - never called (only `register_lazy_function` is used) +- Line 46: Export `list_functions` - never called + +**Changes Required:** + +#### Remove Orphan Exports (Lines 44, 46, 69-71) +```python +# REMOVE FROM __all__: +'register_function', # Line 44 - never called +'list_functions', # Line 46 - never called +'get_model', # Line 69 - never imported +'get_max_tokens', # Line 70 - never imported +'get_temperature', # Line 71 - never imported +``` + +#### Remove Orphan Imports (Lines 29-35) +```python +# REMOVE THESE IMPORTS: +from igny8_core.ai.settings import ( + MODEL_CONFIG, # Will be removed in Step 2 + get_model_config, # Keep - actively used + get_model, # ❌ Remove - never used + get_max_tokens, # ❌ Remove - never used + get_temperature, # ❌ Remove - never used +) +``` + +**Key Changes:** +- Remove unused exports from `__all__` +- Remove unused imports +- Keep `get_model_config` (actively used) +- Keep `register_function` and `list_functions` in registry.py (may be useful for debugging), but don't export them + +--- + +### 6. `settings.py` - Remove Unused Helper Functions + +**Current Issues:** +- Lines 106-109: `get_model()` function - never called +- Lines 112-115: `get_max_tokens()` function - never called +- Lines 118-121: `get_temperature()` function - never called + +**Changes Required:** + +#### Remove Unused Helper Functions (Lines 106-121) +```python +# REMOVE THESE FUNCTIONS: +def get_model(function_name: str) -> str: + """Get model name for function""" + config = get_model_config(function_name) + return config.get("model", "gpt-4.1") + +def get_max_tokens(function_name: str) -> int: + """Get max tokens for function""" + config = get_model_config(function_name) + return config.get("max_tokens", 4000) + +def get_temperature(function_name: str) -> float: + """Get temperature for function""" + config = get_model_config(function_name) + return config.get("temperature", 0.7) +``` + +**Key Changes:** +- These functions are redundant - `get_model_config()` already returns all needed values +- After removing MODEL_CONFIG, these functions become even less useful +- Code should call `get_model_config()` directly and extract values from the returned dict + +--- + +### 7. `registry.py` - Keep Functions, Don't Export + +**Current Issues:** +- `register_function()` is exported but never called (only `register_lazy_function` is used) +- `list_functions()` is exported but never called + +**Changes Required:** + +#### Keep Functions, Remove from Exports +```python +# KEEP THESE FUNCTIONS IN registry.py (may be useful for debugging/admin) +# BUT REMOVE FROM __init__.py exports + +# In registry.py - KEEP: +def register_function(name: str, function_class: Type[BaseAIFunction]): + """Register an AI function""" + # ... keep implementation + +def list_functions() -> list: + """List all registered functions""" + # ... keep implementation + +# In __init__.py - REMOVE from exports: +# 'register_function', # ❌ Remove +# 'list_functions', # ❌ Remove +``` + +**Key Changes:** +- Keep functions in `registry.py` (may be useful for future direct registration or debugging) +- Remove from `__init__.py` exports (not used anywhere) +- If needed in future, can be imported directly from `registry.py` + +--- + +## Error Handling Strategy + +### New Error Messages + +When IntegrationSettings not configured: +```python +ValueError: "OpenAI IntegrationSettings not configured for account {account_id}. Please configure OpenAI settings in the integration page." +``` + +When model not set in IntegrationSettings: +```python +ValueError: "Model not configured in IntegrationSettings for account {account_id}. Please set 'model' in OpenAI integration settings." +``` + +When account not provided: +```python +ValueError: "Account is required for model configuration" +``` + +### Frontend Impact + +The frontend should handle these errors gracefully: +- Show user-friendly error message +- Redirect to integration settings page +- Provide clear instructions on how to configure + +--- + +## Testing Plan + +### Unit Tests + +1. **Test `get_model_config()` with valid IntegrationSettings** + - Should return model from IntegrationSettings + - Should include max_tokens and temperature + +2. **Test `get_model_config()` without IntegrationSettings** + - Should raise ValueError with clear message + +3. **Test `get_model_config()` without model in config** + - Should raise ValueError with clear message + +4. **Test `get_model_config()` without account** + - Should raise ValueError + +5. **Test `ai_core.run_ai_request()` without model** + - Should raise ValueError + +### Integration Tests + +1. **Test each AI function with valid IntegrationSettings** + - `auto_cluster` - should work + - `generate_ideas` - should work + - `generate_content` - should work + - `generate_image_prompts` - should work + - `generate_images` - should work + +2. **Test each AI function without IntegrationSettings** + - Should raise clear error + - Error should be user-friendly + +3. **Test from view endpoints** + - Should return proper error response + - Should include request_id + - Should follow unified API format + +--- + +## Migration Steps + +### Step 1: Backup Current Code +- Create git branch: `refactor/remove-model-fallbacks` +- Commit current state + +### Step 2: Update `settings.py` +- Remove `MODEL_CONFIG` dict +- Update `get_model_config()` function +- Add proper error handling + +### Step 3: Update `ai_core.py` +- Remove `_default_model` initialization +- Update `run_ai_request()` to require model +- Add error handling + +### Step 4: Update `engine.py` +- Validate account before calling `get_model_config()` +- Add error handling + +### Step 5: Update `tasks.py` +- Validate account_id is provided +- Add error handling + +### Step 6: Clean Up Orphan Code +- Remove orphan exports from `__init__.py` +- Remove unused helper functions from `settings.py` +- Remove unused imports from `__init__.py` +- Verify no broken imports + +### Step 7: Update Tests +- Add unit tests for new error cases +- Update integration tests +- Test all 5 active functions + +### Step 8: Test Manually +- Test each AI function with valid IntegrationSettings +- Test each AI function without IntegrationSettings +- Verify error messages are clear + +### Step 9: Update Documentation +- Update AI framework docs +- Document new error handling +- Update API documentation + +### Step 10: Deploy +- Deploy to staging +- Test in staging environment +- Deploy to production + +--- + +## Rollback Plan + +If issues arise: + +1. **Immediate Rollback** + - Revert git branch + - Restore previous version + - Monitor for issues + +2. **Partial Rollback** + - Keep IntegrationSettings changes + - Restore MODEL_CONFIG as fallback temporarily + - Fix issues incrementally + +--- + +## Success Criteria + +- [ ] All 5 active AI functions work with IntegrationSettings only +- [ ] Clear error messages when IntegrationSettings not configured +- [ ] No hardcoded model defaults remain +- [ ] No Django settings fallbacks remain +- [ ] Orphan code removed (orphan exports, unused functions) +- [ ] No broken imports after cleanup +- [ ] All tests pass +- [ ] Documentation updated +- [ ] Frontend handles errors gracefully + +--- + +## Notes + +- **IntegrationSettings Structure:** Each account must have `IntegrationSettings` with `integration_type='openai'` and `config` containing `model`, `max_tokens`, `temperature` +- **Account-Specific:** Each account configures their own models - no global defaults +- **Error Clarity:** All errors must be user-friendly and actionable +- **No Breaking Changes:** This is a refactoring, not a breaking change - existing accounts with IntegrationSettings will continue to work + +--- + +**Last Updated:** 2025-01-XX +**Status:** Ready for Implementation + diff --git a/docs/DOCUMENTATION-SUMMARY.md b/docs/DOCUMENTATION-SUMMARY.md index 30b3e263..ac9a2dcb 100644 --- a/docs/DOCUMENTATION-SUMMARY.md +++ b/docs/DOCUMENTATION-SUMMARY.md @@ -3,8 +3,11 @@ **Section 2: Documentation - COMPLETE** ✅ **Date Completed**: 2025-11-16 +**Last Updated**: 2025-01-XX **Status**: All Documentation Complete and Ready +**API Standard v1.0**: ✅ **100% COMPLIANT** - All remaining items completed + --- ## Implementation Overview diff --git a/unified-api/API-STANDARD-v1.0.md b/unified-api/API-STANDARD-v1.0.md index de307d45..94d390fa 100644 --- a/unified-api/API-STANDARD-v1.0.md +++ b/unified-api/API-STANDARD-v1.0.md @@ -1298,7 +1298,15 @@ from igny8_core.api.viewsets import BaseTenantViewSet --- -**Document Status:** Active Standard +**Document Status:** Active Standard - ✅ **100% IMPLEMENTED** **Last Updated:** 2025-01-XX +**Implementation Status:** All requirements implemented and verified +- ✅ All endpoints return unified response format +- ✅ All errors fully unified with request_id tracking +- ✅ All ViewSets use proper base classes, pagination, throttles, and permissions +- ✅ All custom @action methods use unified format +- ✅ All public endpoints properly configured +- ✅ Health check endpoint `/api/v1/system/ping/` implemented +- ✅ All endpoints documented in Swagger/ReDoc **Next Review:** Quarterly or as needed diff --git a/unified-api/IMPLEMENTATION-PLAN-REMAINING.md b/unified-api/IMPLEMENTATION-PLAN-REMAINING.md deleted file mode 100644 index d3c66324..00000000 --- a/unified-api/IMPLEMENTATION-PLAN-REMAINING.md +++ /dev/null @@ -1,593 +0,0 @@ -# Implementation Plan - Remaining API Standard v1.0 Items - -**Date:** 2025-01-XX -**Status:** Planning -**Purpose:** Complete implementation plan for remaining API Standard v1.0 compliance items - ---- - -## Executive Summary - -**Total Items:** 10 remaining items -**Critical Priority:** 2 items -**High Priority:** 4 items -**Medium Priority:** 4 items - -**Estimated Time:** 12-16 hours -**Risk Level:** Low (mostly adding permissions and refactoring response formats) - ---- - -## Table of Contents - -1. [Critical Priority Items](#critical-priority-items) -2. [High Priority Items](#high-priority-items) -3. [Medium Priority Items](#medium-priority-items) -4. [Implementation Order](#implementation-order) -5. [Testing Strategy](#testing-strategy) -6. [Rollback Plan](#rollback-plan) - ---- - -## Critical Priority Items - -### 1. Fix Auth Endpoints to Use Unified Format - -**Issue:** `RegisterView`, `LoginView`, `ChangePasswordView`, `MeView` in `backend/igny8_core/auth/urls.py` use raw `Response()` instead of unified format helpers. - -**Impact:** These are the most frequently used endpoints and don't comply with the standard. - -**Files to Modify:** -- `backend/igny8_core/auth/urls.py` - -**Implementation Steps:** - -1. **Update RegisterView** (lines 41-58) - ```python - # BEFORE: - return Response({ - 'success': True, - 'message': 'Registration successful', - 'user': user_serializer.data - }, status=status.HTTP_201_CREATED) - - # AFTER: - from igny8_core.api.response import success_response, error_response - - return success_response( - data={'user': user_serializer.data}, - message='Registration successful', - status_code=status.HTTP_201_CREATED, - request=request - ) - ``` - -2. **Update LoginView** (lines 66-134) - - Replace all `Response()` calls with `success_response()` or `error_response()` - - Ensure `request_id` is included - - Fix error response format to use `error` + `errors` structure - -3. **Update ChangePasswordView** (lines 142-167) - - Replace `Response()` with `success_response()` and `error_response()` - - Ensure proper error format - -4. **Update MeView** (lines 175-188) - - Replace `Response()` with `success_response()` - - Ensure `request_id` is included - -5. **Add imports at top of file:** - ```python - from igny8_core.api.response import success_response, error_response - ``` - -**Testing:** -- Test registration with valid/invalid data -- Test login with valid/invalid credentials -- Test change password with valid/invalid old password -- Test /me endpoint -- Verify all responses have `success`, `data`/`error`, `request_id` fields - -**Estimated Time:** 2 hours - ---- - -### 2. Fix AIPromptViewSet Permissions - -**Issue:** `AIPromptViewSet` has `permission_classes = []` which allows unauthenticated access. - -**Impact:** Security vulnerability - anyone can access/modify AI prompts. - -**Files to Modify:** -- `backend/igny8_core/modules/system/views.py` (line 42) - -**Implementation Steps:** - -1. **Update permission_classes:** - ```python - # BEFORE: - permission_classes = [] # Allow any for now (backward compatibility) - - # AFTER: - from igny8_core.api.permissions import IsAuthenticatedAndActive, HasTenantAccess - - permission_classes = [IsAuthenticatedAndActive, HasTenantAccess] - ``` - -2. **Verify custom actions have proper permissions:** - - `get_by_type` - Should allow authenticated users - - `save_prompt` - Should require `IsEditorOrAbove` (per standard: "Saving prompts requires editor/admin") - - `reset_prompt` - Should require `IsEditorOrAbove` - -3. **Update save_prompt action:** - ```python - @action(detail=False, methods=['post'], url_path='save', url_name='save') - def save_prompt(self, request): - """Save or update a prompt - requires editor or above""" - # Add permission check - from igny8_core.api.permissions import IsEditorOrAbove - if not IsEditorOrAbove().has_permission(request, self): - return error_response( - error='Permission denied', - status_code=status.HTTP_403_FORBIDDEN, - request=request - ) - # ... rest of implementation - ``` - -**Testing:** -- Test unauthenticated access (should return 401) -- Test authenticated user access (should work) -- Test save_prompt with viewer role (should return 403) -- Test save_prompt with editor role (should work) - -**Estimated Time:** 1 hour - ---- - -## High Priority Items - -### 3. Update Billing ViewSets to Use Standard Permissions - -**Issue:** All billing ViewSets use `permissions.IsAuthenticated` instead of `IsAuthenticatedAndActive + HasTenantAccess`. - -**Files to Modify:** -- `backend/igny8_core/modules/billing/views.py` - -**Implementation Steps:** - -1. **Update CreditBalanceViewSet** (line 35) - ```python - # BEFORE: - permission_classes = [permissions.IsAuthenticated] - - # AFTER: - from igny8_core.api.permissions import IsAuthenticatedAndActive, HasTenantAccess - - permission_classes = [IsAuthenticatedAndActive, HasTenantAccess] - ``` - -2. **Update CreditUsageViewSet** (line 91) - ```python - # BEFORE: - permission_classes = [permissions.IsAuthenticated] - - # AFTER: - permission_classes = [IsAuthenticatedAndActive, HasTenantAccess] - ``` - -3. **Update CreditTransactionViewSet** (line 466) - ```python - # BEFORE: - permission_classes = [permissions.IsAuthenticated] - - # AFTER: - from igny8_core.api.permissions import IsAuthenticatedAndActive, HasTenantAccess, IsAdminOrOwner - - permission_classes = [IsAuthenticatedAndActive, HasTenantAccess, IsAdminOrOwner] - ``` - Note: Transactions require admin/owner per standard. - -**Testing:** -- Test with unauthenticated user (should return 401) -- Test with authenticated user from different account (should return 403) -- Test with authenticated user from same account (should work) -- Test CreditTransactionViewSet with viewer/editor (should return 403) -- Test CreditTransactionViewSet with admin/owner (should work) - -**Estimated Time:** 1.5 hours - ---- - -### 4. Update System Settings ViewSets to Use Standard Permissions - -**Issue:** All 5 system settings ViewSets use `permissions.IsAuthenticated` instead of standard permissions. - -**Files to Modify:** -- `backend/igny8_core/modules/system/settings_views.py` - -**Implementation Steps:** - -1. **Update SystemSettingsViewSet** (line 37) - ```python - # BEFORE: - permission_classes = [permissions.IsAuthenticated] - - # AFTER: - from igny8_core.api.permissions import IsAuthenticatedAndActive, HasTenantAccess, IsAdminOrOwner - - permission_classes = [IsAuthenticatedAndActive, HasTenantAccess] - - # Update get_permissions() method to use IsAdminOrOwner for write operations - def get_permissions(self): - if self.action in ['create', 'update', 'partial_update', 'destroy']: - return [IsAdminOrOwner()] - return [IsAuthenticatedAndActive(), HasTenantAccess()] - ``` - -2. **Update AccountSettingsViewSet** (line 88) - ```python - permission_classes = [IsAuthenticatedAndActive, HasTenantAccess] - # Write operations should require IsAdminOrOwner (already handled by get_permissions) - ``` - -3. **Update UserSettingsViewSet** (line 148) - ```python - permission_classes = [IsAuthenticatedAndActive, HasTenantAccess] - ``` - -4. **Update ModuleSettingsViewSet** (line 214) - ```python - permission_classes = [IsAuthenticatedAndActive, HasTenantAccess] - ``` - -5. **Update AISettingsViewSet** (line 293) - ```python - permission_classes = [IsAuthenticatedAndActive, HasTenantAccess] - ``` - -**Testing:** -- Test read operations with authenticated user (should work) -- Test write operations with viewer/editor (should return 403) -- Test write operations with admin/owner (should work) -- Test with user from different account (should return 403) - -**Estimated Time:** 2 hours - ---- - -## Medium Priority Items - -### 5. Refactor Billing ViewSets to Use AccountModelViewSet - -**Issue:** Billing ViewSets manually filter by account instead of using base class. - -**Files to Modify:** -- `backend/igny8_core/modules/billing/views.py` - -**Implementation Steps:** - -1. **Refactor CreditBalanceViewSet:** - - Change from `viewsets.ViewSet` to inherit from a base class - - Since it's a custom action-only ViewSet, keep as ViewSet but ensure account filtering - - Verify account is set correctly from request - -2. **Refactor CreditUsageViewSet:** - ```python - # BEFORE: - class CreditUsageViewSet(viewsets.ReadOnlyModelViewSet): - def get_queryset(self): - account = getattr(self.request, 'account', None) - # ... manual filtering - - # AFTER: - from igny8_core.api.base import AccountModelViewSet - - class CreditUsageViewSet(AccountModelViewSet): - # Base class handles account filtering automatically - def get_queryset(self): - queryset = super().get_queryset() - # Only add custom filtering here (operation_type, date range) - # Account filtering is automatic - ``` - -3. **Refactor CreditTransactionViewSet:** - ```python - # BEFORE: - class CreditTransactionViewSet(viewsets.ReadOnlyModelViewSet): - def get_queryset(self): - account = getattr(self.request, 'account', None) - # ... manual filtering - - # AFTER: - class CreditTransactionViewSet(AccountModelViewSet): - # Base class handles account filtering automatically - ``` - -**Testing:** -- Verify account filtering works correctly -- Test with admin/developer (should see all accounts) -- Test with regular user (should only see their account) -- Verify custom filters still work (operation_type, date range) - -**Estimated Time:** 2 hours - ---- - -### 6. Refactor IntegrationSettingsViewSet to Use AccountModelViewSet - -**Issue:** `IntegrationSettingsViewSet` inherits from `viewsets.ViewSet` instead of `AccountModelViewSet`. - -**Files to Modify:** -- `backend/igny8_core/modules/system/integration_views.py` - -**Implementation Steps:** - -1. **Change base class:** - ```python - # BEFORE: - class IntegrationSettingsViewSet(viewsets.ViewSet): - - # AFTER: - from igny8_core.api.base import AccountModelViewSet - - class IntegrationSettingsViewSet(AccountModelViewSet): - ``` - -2. **Update methods to use base class features:** - - `get_queryset()` - Use base class method if model has account field - - `perform_create()` - Use base class to set account - - Verify account scoping works correctly - -3. **Note:** This ViewSet uses custom URL patterns, so ensure base class doesn't conflict. - -**Testing:** -- Test integration settings retrieval (should be account-scoped) -- Test saving integration settings (should set account automatically) -- Test with user from different account (should not see other account's settings) - -**Estimated Time:** 1.5 hours - ---- - -### 7. Add Explicit Standard Permissions to Auth ViewSets - -**Issue:** Auth ViewSets inherit from `AccountModelViewSet` but don't explicitly set standard permissions. - -**Files to Modify:** -- `backend/igny8_core/auth/views.py` - -**Implementation Steps:** - -1. **Update UsersViewSet** (line 135) - ```python - from igny8_core.api.permissions import IsAuthenticatedAndActive, HasTenantAccess, IsAdminOrOwner - - permission_classes = [IsAuthenticatedAndActive, HasTenantAccess, IsAdminOrOwner] - ``` - -2. **Update AccountsViewSet** (line 270) - ```python - permission_classes = [IsAuthenticatedAndActive, HasTenantAccess, IsAdminOrOwner] - ``` - -3. **Update SubscriptionsViewSet** (line 335) - ```python - permission_classes = [IsAuthenticatedAndActive, HasTenantAccess, IsAdminOrOwner] - ``` - -4. **Update SiteViewSet** (line 472) - ```python - from igny8_core.api.permissions import IsAuthenticatedAndActive, HasTenantAccess, IsEditorOrAbove - - permission_classes = [IsAuthenticatedAndActive, HasTenantAccess, IsEditorOrAbove] - ``` - -5. **Update SectorViewSet** (line 715) - ```python - permission_classes = [IsAuthenticatedAndActive, HasTenantAccess, IsEditorOrAbove] - ``` - -6. **Update SiteUserAccessViewSet** (line 396) - ```python - permission_classes = [IsAuthenticatedAndActive, HasTenantAccess, IsAdminOrOwner] - ``` - -**Testing:** -- Test each ViewSet with appropriate roles -- Verify tenant isolation works -- Test with users from different accounts - -**Estimated Time:** 1.5 hours - ---- - -### 8. Add Role-Based Permissions to CreditTransactionViewSet - -**Issue:** `CreditTransactionViewSet` should require `IsAdminOrOwner` per standard but currently only requires `IsAuthenticated`. - -**Files to Modify:** -- `backend/igny8_core/modules/billing/views.py` (line 466) - -**Implementation Steps:** - -1. **Update permissions:** - ```python - # This is already covered in item #3, but ensure it's implemented: - from igny8_core.api.permissions import IsAuthenticatedAndActive, HasTenantAccess, IsAdminOrOwner - - permission_classes = [IsAuthenticatedAndActive, HasTenantAccess, IsAdminOrOwner] - ``` - -2. **Verify in standard:** Per API Standard v1.0, billing/transactions require admin/owner. - -**Testing:** -- Test with viewer role (should return 403) -- Test with editor role (should return 403) -- Test with admin role (should work) -- Test with owner role (should work) - -**Estimated Time:** 0.5 hours (included in item #3) - ---- - -## Implementation Order - -### Phase 1: Critical Security Fixes (3 hours) -1. ✅ Fix AIPromptViewSet permissions (1 hour) - **CRITICAL SECURITY** -2. ✅ Fix auth endpoints unified format (2 hours) - **HIGH VISIBILITY** - -### Phase 2: High Priority Permissions (3.5 hours) -3. ✅ Update billing ViewSets permissions (1.5 hours) -4. ✅ Update system settings ViewSets permissions (2 hours) - -### Phase 3: Base Class Refactoring (3.5 hours) -5. ✅ Refactor billing ViewSets to use AccountModelViewSet (2 hours) -6. ✅ Refactor IntegrationSettingsViewSet (1.5 hours) - -### Phase 4: Auth Module Permissions (1.5 hours) -7. ✅ Add explicit permissions to auth ViewSets (1.5 hours) - -**Total Estimated Time:** 11.5 hours - ---- - -## Testing Strategy - -### Unit Tests - -**File:** `backend/igny8_core/api/tests/test_auth_endpoints.py` (NEW) - -```python -class AuthEndpointsTestCase(TestCase): - def test_register_returns_unified_format(self): - """Test register endpoint returns unified format""" - response = self.client.post('/api/v1/auth/register/', { - 'email': 'test@example.com', - 'password': 'testpass123', - 'password_confirm': 'testpass123' - }) - data = response.json() - self.assertIn('success', data) - self.assertTrue(data['success']) - self.assertIn('data', data) - self.assertIn('request_id', data) - - def test_login_returns_unified_format(self): - """Test login endpoint returns unified format""" - # ... test implementation - - def test_change_password_returns_unified_format(self): - """Test change password endpoint returns unified format""" - # ... test implementation - - def test_me_returns_unified_format(self): - """Test /me endpoint returns unified format""" - # ... test implementation -``` - -**File:** `backend/igny8_core/api/tests/test_permissions_billing.py` (NEW) - -```python -class BillingPermissionsTestCase(TestCase): - def test_credit_balance_requires_authentication(self): - """Test credit balance requires authentication""" - # ... test implementation - - def test_credit_transactions_requires_admin(self): - """Test credit transactions requires admin/owner""" - # ... test implementation -``` - -### Integration Tests - -**Update:** `backend/igny8_core/api/tests/test_integration_auth.py` - -- Add tests for auth endpoints unified format -- Test permission enforcement - -**Update:** `backend/igny8_core/api/tests/test_integration_billing.py` - -- Add tests for permission enforcement -- Test account scoping - -**Update:** `backend/igny8_core/api/tests/test_integration_system.py` - -- Add tests for AIPromptViewSet permissions -- Test system settings permissions - -### Manual Testing Checklist - -- [ ] Test registration with valid/invalid data -- [ ] Test login with valid/invalid credentials -- [ ] Test change password -- [ ] Test /me endpoint -- [ ] Test AIPromptViewSet with unauthenticated user (should fail) -- [ ] Test AIPromptViewSet save_prompt with viewer (should fail) -- [ ] Test billing endpoints with different roles -- [ ] Test system settings with different roles -- [ ] Test account isolation on all endpoints -- [ ] Verify all responses have unified format - ---- - -## Rollback Plan - -### If Issues Arise: - -1. **Immediate Rollback:** - - Revert git commits for problematic changes - - Restore previous version - - Monitor for issues - -2. **Partial Rollback:** - - Revert specific module changes - - Keep other improvements - - Fix issues incrementally - -3. **Feature Flag:** - - Add feature flag for new permissions - - Disable if issues found - - Re-enable after fixes - -### Rollback Order (if needed): - -1. Auth endpoints (most visible) -2. Permissions (security critical) -3. Base class refactoring (less visible) - ---- - -## Success Criteria - -### Definition of Done - -- [ ] All auth endpoints return unified format -- [ ] All ViewSets use standard permissions -- [ ] All ViewSets use base classes where applicable -- [ ] All tests pass -- [ ] Manual testing completed -- [ ] No security vulnerabilities -- [ ] CHANGELOG updated with accurate information - -### Metrics - -- **Coverage:** 100% of remaining items implemented -- **Test Coverage:** >90% for modified code -- **Security:** No endpoints allow unauthenticated access (except public endpoints) -- **Compliance:** 100% compliance with API Standard v1.0 - ---- - -## Notes - -- All changes should maintain backward compatibility where possible -- Permission changes may require frontend updates if error handling changes -- Test thoroughly before deploying to production -- Update CHANGELOG.md after completion to reflect accurate status - ---- - -**Document Status:** Implementation Plan -**Last Updated:** 2025-01-XX -**Next Steps:** Begin Phase 1 implementation -