From 538389797709b34a45a90e8afe5a11a737cd245c Mon Sep 17 00:00:00 2001 From: marko-kraemer Date: Sat, 4 Oct 2025 22:43:46 +0200 Subject: [PATCH] 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 --- backend/SIMPLIFICATION_ANALYSIS.md | 161 --------------------- backend/core/tools/sb_deploy_tool.py | 1 - backend/core/tools/sb_files_tool.py | 1 - backend/core/tools/sb_presentation_tool.py | 1 - backend/core/tools/sb_shell_tool.py | 1 - backend/core/tools/sb_templates_tool.py | 1 - backend/core/tools/sb_upload_file_tool.py | 5 +- backend/core/tools/sb_web_dev_tool.py | 1 - 8 files changed, 2 insertions(+), 170 deletions(-) delete mode 100644 backend/SIMPLIFICATION_ANALYSIS.md diff --git a/backend/SIMPLIFICATION_ANALYSIS.md b/backend/SIMPLIFICATION_ANALYSIS.md deleted file mode 100644 index 3b57575b..00000000 --- a/backend/SIMPLIFICATION_ANALYSIS.md +++ /dev/null @@ -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 - diff --git a/backend/core/tools/sb_deploy_tool.py b/backend/core/tools/sb_deploy_tool.py index a5ff1e81..448f78ac 100644 --- a/backend/core/tools/sb_deploy_tool.py +++ b/backend/core/tools/sb_deploy_tool.py @@ -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: diff --git a/backend/core/tools/sb_files_tool.py b/backend/core/tools/sb_files_tool.py index 90c42e2e..830f6647 100644 --- a/backend/core/tools/sb_files_tool.py +++ b/backend/core/tools/sb_files_tool.py @@ -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""" diff --git a/backend/core/tools/sb_presentation_tool.py b/backend/core/tools/sb_presentation_tool.py index c0ca333c..4c67a8c6 100644 --- a/backend/core/tools/sb_presentation_tool.py +++ b/backend/core/tools/sb_presentation_tool.py @@ -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" diff --git a/backend/core/tools/sb_shell_tool.py b/backend/core/tools/sb_shell_tool.py index d05abc26..1b23722c 100644 --- a/backend/core/tools/sb_shell_tool.py +++ b/backend/core/tools/sb_shell_tool.py @@ -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.""" diff --git a/backend/core/tools/sb_templates_tool.py b/backend/core/tools/sb_templates_tool.py index 7f63679f..2797395a 100644 --- a/backend/core/tools/sb_templates_tool.py +++ b/backend/core/tools/sb_templates_tool.py @@ -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") diff --git a/backend/core/tools/sb_upload_file_tool.py b/backend/core/tools/sb_upload_file_tool.py index da9e861c..e17552ca 100644 --- a/backend/core/tools/sb_upload_file_tool.py +++ b/backend/core/tools/sb_upload_file_tool.py @@ -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", diff --git a/backend/core/tools/sb_web_dev_tool.py b/backend/core/tools/sb_web_dev_tool.py index 5297154c..e162e8d0 100644 --- a/backend/core/tools/sb_web_dev_tool.py +++ b/backend/core/tools/sb_web_dev_tool.py @@ -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: