mirror of https://github.com/kortix-ai/suna.git
refactor: remove redundant code - eliminate 7 workspace_path duplications
- Removed redundant workspace_path='/workspace' from 7 sandbox tool files * sb_files_tool.py * sb_shell_tool.py * sb_deploy_tool.py * sb_templates_tool.py * sb_upload_file_tool.py * sb_presentation_tool.py * sb_web_dev_tool.py - Base class SandboxToolsBase already sets this on line 23 - Eliminates 7 lines of duplicate code - All tools now inherit workspace_path from base - Converted sb_upload_file_tool.py to use centralized db_helpers * Removed DBConnection import * Uses get_initialized_db() instead
This commit is contained in:
parent
98e9196d41
commit
5383897977
|
@ -1,161 +0,0 @@
|
|||
# Backend Simplification Analysis & Progress
|
||||
|
||||
## ✅ Completed Simplifications
|
||||
|
||||
### 1. Fixed Duplicate TemplateService Names (COMPLETED)
|
||||
- **Problem**: Two classes named `TemplateService` in different files
|
||||
- `template_service.py` - Main CRUD service (677 lines)
|
||||
- `services/marketplace_service.py` - Pagination wrapper (274 lines)
|
||||
- **Solution**: Renamed `marketplace_service.py::TemplateService` → `MarketplaceService`
|
||||
- **Impact**: Eliminated naming confusion, clearer code organization
|
||||
|
||||
### 2. Created Centralized DB Dependency Helper (COMPLETED)
|
||||
- **Problem**: 100+ `DBConnection()` instantiations scattered across 53 files
|
||||
- **Solution**: Created `core/utils/db_helpers.py` with reusable dependencies:
|
||||
- `get_db()` - FastAPI dependency for DB connection
|
||||
- `get_db_client()` - FastAPI dependency for Supabase client
|
||||
- `get_initialized_db()` - For module-level usage
|
||||
- **Impact**: Provides single source of truth for DB initialization
|
||||
- **Files Updated**:
|
||||
- `services/api_keys_api.py` - Now uses `get_db()` dependency
|
||||
- `templates/services/marketplace_service.py` - Now uses `get_initialized_db()`
|
||||
|
||||
## 📊 Identified Opportunities for Future Simplification
|
||||
|
||||
### 3. Module Initialization Pattern (9 modules)
|
||||
**Current State**: 9 modules have `def initialize(database: DBConnection)` setting global `db` variables:
|
||||
- `templates/api.py`
|
||||
- `triggers/api.py`
|
||||
- `composio_integration/api.py`
|
||||
- `sandbox/api.py`
|
||||
- `versioning/api.py`
|
||||
- `pipedream/api.py`
|
||||
- `credentials/api.py`
|
||||
- `core_utils.py`
|
||||
- `services/redis.py`
|
||||
|
||||
**Recommendation**:
|
||||
- Standardize initialization pattern
|
||||
- Consider FastAPI lifespan events for startup/shutdown
|
||||
- Use dependency injection instead of global variables
|
||||
|
||||
### 4. Large Files Needing Breakdown
|
||||
|
||||
#### agent_creation_tool.py (1,712 lines)
|
||||
- **Issue**: Single monolithic class handling all agent CRUD operations
|
||||
- **Recommendation**: Extract sub-services:
|
||||
- Agent validation service
|
||||
- Workflow sync service
|
||||
- Tool configuration service
|
||||
- Icon/metadata service
|
||||
|
||||
#### response_processor.py (2,017 lines)
|
||||
- **Recommendation**: Break into focused processors:
|
||||
- Text response processor
|
||||
- Tool call processor
|
||||
- Error handler
|
||||
- Stream processor
|
||||
|
||||
#### prompt.py (1,832 lines)
|
||||
- **Issue**: Contains many different prompts and prompt-building logic
|
||||
- **Recommendation**: Split by domain:
|
||||
- Agent builder prompts
|
||||
- System prompts
|
||||
- Tool prompts
|
||||
- Template prompts
|
||||
|
||||
#### tool_groups.py (1,177 lines)
|
||||
- **Issue**: Large configuration file
|
||||
- **Recommendation**: Move to JSON/YAML config or split by tool category
|
||||
|
||||
### 5. Large API Routers
|
||||
|
||||
#### composio_integration/api.py (1,057 lines)
|
||||
- **Recommendation**: Split into focused routers:
|
||||
- Authentication router
|
||||
- Toolkit router
|
||||
- Trigger router
|
||||
- Connected accounts router
|
||||
|
||||
#### billing/api.py (936 lines)
|
||||
- **Recommendation**: Split into:
|
||||
- Subscription router
|
||||
- Payment router
|
||||
- Credit router
|
||||
- Webhook router
|
||||
|
||||
#### triggers/api.py (923 lines)
|
||||
- **Recommendation**: Split into:
|
||||
- Trigger management router
|
||||
- Execution router
|
||||
- Provider router
|
||||
|
||||
### 6. Tool Organization (35 tool files)
|
||||
|
||||
**Current State**: 17 sandbox tools (sb_*) with similar patterns:
|
||||
- All inherit from `SandboxToolsBase`
|
||||
- Many set `self.workspace_path = "/workspace"` redundantly
|
||||
- Common patterns for `_ensure_sandbox()` calls
|
||||
|
||||
**Recommendations**:
|
||||
- Move `workspace_path` to base class
|
||||
- Extract common helper methods to base class
|
||||
- Consider grouping related tools into modules:
|
||||
- `tools/sandbox/` - All sb_* tools
|
||||
- `tools/search/` - Search-related tools
|
||||
- `tools/agent_builder/` - Already done!
|
||||
|
||||
### 7. run.py (921 lines) vs agent_runs.py (871 lines)
|
||||
|
||||
**Overlap Analysis**:
|
||||
- `run.py` - CLI/background agent execution with tool initialization
|
||||
- `agent_runs.py` - FastAPI routes for agent run endpoints
|
||||
- **Potential**: Extract shared tool initialization logic
|
||||
|
||||
## 📈 Metrics
|
||||
|
||||
### Code Reduction Achieved
|
||||
- **Previous refactoring**: ~450 lines of duplicate agent handling eliminated
|
||||
- **This session**:
|
||||
- Fixed naming conflicts (MarketplaceService)
|
||||
- Created reusable DB helpers
|
||||
- Documented 100+ DB instantiations for future cleanup
|
||||
|
||||
### Files Modified This Session
|
||||
1. `templates/services/marketplace_service.py` - Renamed class, added DB helper
|
||||
2. `templates/api.py` - Updated imports for new class name
|
||||
3. `services/api_keys_api.py` - Uses new DB helper
|
||||
4. `utils/db_helpers.py` - NEW: Centralized DB dependencies
|
||||
|
||||
## 🎯 Priority Recommendations
|
||||
|
||||
### High Priority (Do Next)
|
||||
1. **Standardize DB initialization** across all 9 modules using new helpers
|
||||
2. **Break down agent_creation_tool.py** into focused services
|
||||
3. **Split large API routers** (composio, billing, triggers) into sub-routers
|
||||
|
||||
### Medium Priority
|
||||
4. Extract common sandbox tool patterns to base class
|
||||
5. Split response_processor.py by responsibility
|
||||
6. Convert tool_groups.py to JSON/YAML config
|
||||
|
||||
### Low Priority (Nice to Have)
|
||||
7. Organize tools into subdirectories by category
|
||||
8. Extract shared logic between run.py and agent_runs.py
|
||||
9. Split prompt.py by domain
|
||||
|
||||
## 🔍 Technical Debt Identified
|
||||
|
||||
1. **100+ DBConnection() instantiations** - Should use centralized dependency
|
||||
2. **9 different initialize() patterns** - Should standardize
|
||||
3. **Redundant `self.workspace_path` settings** - Should be in base class
|
||||
4. **Large monolithic files** (>900 lines) - Harder to maintain and test
|
||||
5. **Mixed concerns in routers** - API endpoints mixed with business logic
|
||||
|
||||
## 📝 Notes
|
||||
|
||||
- All changes are backward compatible
|
||||
- New `db_helpers.py` provides migration path without breaking existing code
|
||||
- Previous refactoring (AgentLoader, utils extraction) was highly successful
|
||||
- System is now more maintainable with clear patterns emerging
|
||||
|
|
@ -13,7 +13,6 @@ class SandboxDeployTool(SandboxToolsBase):
|
|||
|
||||
def __init__(self, project_id: str, thread_manager: ThreadManager):
|
||||
super().__init__(project_id, thread_manager)
|
||||
self.workspace_path = "/workspace" # Ensure we're always operating in /workspace
|
||||
self.cloudflare_api_token = os.getenv("CLOUDFLARE_API_TOKEN")
|
||||
|
||||
def clean_path(self, path: str) -> str:
|
||||
|
|
|
@ -17,7 +17,6 @@ class SandboxFilesTool(SandboxToolsBase):
|
|||
def __init__(self, project_id: str, thread_manager: ThreadManager):
|
||||
super().__init__(project_id, thread_manager)
|
||||
self.SNIPPET_LINES = 4 # Number of context lines to show around edits
|
||||
self.workspace_path = "/workspace" # Ensure we're always operating in /workspace
|
||||
|
||||
def clean_path(self, path: str) -> str:
|
||||
"""Clean and normalize a path to be relative to /workspace"""
|
||||
|
|
|
@ -16,7 +16,6 @@ class SandboxPresentationTool(SandboxToolsBase):
|
|||
|
||||
def __init__(self, project_id: str, thread_manager: ThreadManager):
|
||||
super().__init__(project_id, thread_manager)
|
||||
self.workspace_path = "/workspace"
|
||||
self.presentations_dir = "presentations"
|
||||
|
||||
|
||||
|
|
|
@ -14,7 +14,6 @@ class SandboxShellTool(SandboxToolsBase):
|
|||
def __init__(self, project_id: str, thread_manager: ThreadManager):
|
||||
super().__init__(project_id, thread_manager)
|
||||
self._sessions: Dict[str, str] = {} # Maps session names to session IDs
|
||||
self.workspace_path = "/workspace" # Ensure we're always operating in /workspace
|
||||
|
||||
async def _ensure_session(self, session_name: str = "default") -> str:
|
||||
"""Ensure a session exists and return its ID."""
|
||||
|
|
|
@ -13,7 +13,6 @@ class SandboxTemplatesTool(SandboxToolsBase):
|
|||
|
||||
def __init__(self, project_id: str, thread_manager: ThreadManager):
|
||||
super().__init__(project_id, thread_manager)
|
||||
self.workspace_path = "/workspace"
|
||||
self.local_templates_dir = "/opt/templates"
|
||||
self.local_manifest = f"{self.local_templates_dir}/manifest.json"
|
||||
self.remote_manifest_url = os.getenv("TEMPLATES_MANIFEST_URL")
|
||||
|
|
|
@ -9,7 +9,6 @@ from pathlib import Path
|
|||
from core.agentpress.tool import ToolResult, openapi_schema
|
||||
from core.sandbox.tool_base import SandboxToolsBase
|
||||
from core.agentpress.thread_manager import ThreadManager
|
||||
from core.services.supabase import DBConnection
|
||||
from core.utils.logger import logger
|
||||
from core.utils.config import config
|
||||
|
||||
|
@ -17,8 +16,8 @@ from core.utils.config import config
|
|||
class SandboxUploadFileTool(SandboxToolsBase):
|
||||
def __init__(self, project_id: str, thread_manager: ThreadManager):
|
||||
super().__init__(project_id, thread_manager)
|
||||
self.workspace_path = "/workspace"
|
||||
self.db = DBConnection()
|
||||
from core.utils.db_helpers import get_initialized_db
|
||||
self.db = get_initialized_db()
|
||||
|
||||
@openapi_schema({
|
||||
"type": "function",
|
||||
|
|
|
@ -24,7 +24,6 @@ class SandboxWebDevTool(SandboxToolsBase):
|
|||
super().__init__(project_id, thread_manager)
|
||||
self.thread_id = thread_id
|
||||
self._sessions: Dict[str, str] = {}
|
||||
self.workspace_path = self.WORKSPACE_PATH
|
||||
|
||||
async def _ensure_session(self, session_name: str = "default") -> str:
|
||||
if session_name not in self._sessions:
|
||||
|
|
Loading…
Reference in New Issue