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
This commit is contained in:
27
CHANGELOG.md
27
CHANGELOG.md
@@ -31,18 +31,38 @@ Each entry follows this format:
|
|||||||
- API Monitor page for endpoint health monitoring
|
- API Monitor page for endpoint health monitoring
|
||||||
- CRUD operations monitoring for Planner and Writer modules
|
- CRUD operations monitoring for Planner and Writer modules
|
||||||
- Sidebar API status indicator for aws-admin accounts
|
- 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
|
### Changed
|
||||||
- All API endpoints now return unified response format (`{success, data, message, errors}`)
|
- All API endpoints now return unified response format (`{success, data, message, errors}`)
|
||||||
- Frontend `fetchAPI` wrapper automatically extracts data from unified format
|
- Frontend `fetchAPI` wrapper automatically extracts data from unified format
|
||||||
- All error responses follow unified format with `request_id` tracking
|
- All error responses follow unified format with `request_id` tracking
|
||||||
- Rate limiting configured with scoped throttles per module
|
- 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
|
### Fixed
|
||||||
- Keyword edit form now correctly populates existing values
|
- Keyword edit form now correctly populates existing values
|
||||||
- Auto-cluster function now works correctly with unified API format
|
- Auto-cluster function now works correctly with unified API format
|
||||||
- ResourceDebugOverlay now correctly extracts data from unified API responses
|
- ResourceDebugOverlay now correctly extracts data from unified API responses
|
||||||
- All frontend pages now correctly handle unified API response format
|
- 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
|
### Documentation
|
||||||
- Updated implementation plan to reflect completion of all remaining API Standard v1.0 items
|
- Updated implementation plan to reflect completion of all remaining API Standard v1.0 items
|
||||||
- All 8 remaining items from audit completed (100% compliance achieved)
|
- 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
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|||||||
262
backend/igny8_core/ai/ORPHAN-CODE-AUDIT.md
Normal file
262
backend/igny8_core/ai/ORPHAN-CODE-AUDIT.md
Normal file
@@ -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
|
||||||
|
|
||||||
520
backend/igny8_core/ai/REFACTORING-PLAN.md
Normal file
520
backend/igny8_core/ai/REFACTORING-PLAN.md
Normal file
@@ -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
|
||||||
|
|
||||||
@@ -3,8 +3,11 @@
|
|||||||
**Section 2: Documentation - COMPLETE** ✅
|
**Section 2: Documentation - COMPLETE** ✅
|
||||||
|
|
||||||
**Date Completed**: 2025-11-16
|
**Date Completed**: 2025-11-16
|
||||||
|
**Last Updated**: 2025-01-XX
|
||||||
**Status**: All Documentation Complete and Ready
|
**Status**: All Documentation Complete and Ready
|
||||||
|
|
||||||
|
**API Standard v1.0**: ✅ **100% COMPLIANT** - All remaining items completed
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Implementation Overview
|
## Implementation Overview
|
||||||
|
|||||||
@@ -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
|
**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
|
**Next Review:** Quarterly or as needed
|
||||||
|
|
||||||
|
|||||||
@@ -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
|
|
||||||
|
|
||||||
Reference in New Issue
Block a user