From 17b2cd2456eb0671d1efe7eb1ebaff0891156fc1 Mon Sep 17 00:00:00 2001 From: danyalxahid-askui Date: Fri, 5 Sep 2025 10:10:53 +0200 Subject: [PATCH 01/19] feat(executions): add execution management API with CRUD operations - Introduced `ExecutionService` for managing execution resources with filesystem persistence. - Implemented API routes for listing, creating, retrieving, and modifying executions. - Added models for execution parameters and status handling. - Integrated execution service into the main application router. --- src/askui/chat/api/app.py | 2 + src/askui/chat/api/executions/__init__.py | 1 + src/askui/chat/api/executions/dependencies.py | 13 ++ src/askui/chat/api/executions/models.py | 89 +++++++++++++ src/askui/chat/api/executions/router.py | 72 ++++++++++ src/askui/chat/api/executions/service.py | 126 ++++++++++++++++++ src/askui/chat/api/models.py | 1 + 7 files changed, 304 insertions(+) create mode 100644 src/askui/chat/api/executions/__init__.py create mode 100644 src/askui/chat/api/executions/dependencies.py create mode 100644 src/askui/chat/api/executions/models.py create mode 100644 src/askui/chat/api/executions/router.py create mode 100644 src/askui/chat/api/executions/service.py diff --git a/src/askui/chat/api/app.py b/src/askui/chat/api/app.py index 439ef61c..a4cf4b18 100644 --- a/src/askui/chat/api/app.py +++ b/src/askui/chat/api/app.py @@ -9,6 +9,7 @@ from askui.chat.api.assistants.dependencies import get_assistant_service from askui.chat.api.assistants.router import router as assistants_router from askui.chat.api.dependencies import SetEnvFromHeadersDep, get_settings +from askui.chat.api.executions.router import router as executions_router from askui.chat.api.files.router import router as files_router from askui.chat.api.health.router import router as health_router from askui.chat.api.mcp_configs.dependencies import get_mcp_config_service @@ -48,6 +49,7 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: # noqa: ARG001 # Include routers v1_router = APIRouter(prefix="/v1") v1_router.include_router(assistants_router) +v1_router.include_router(executions_router) v1_router.include_router(threads_router) v1_router.include_router(messages_router) v1_router.include_router(runs_router) diff --git a/src/askui/chat/api/executions/__init__.py b/src/askui/chat/api/executions/__init__.py new file mode 100644 index 00000000..a1b7b448 --- /dev/null +++ b/src/askui/chat/api/executions/__init__.py @@ -0,0 +1 @@ +"""Execution models and services for the chat API.""" diff --git a/src/askui/chat/api/executions/dependencies.py b/src/askui/chat/api/executions/dependencies.py new file mode 100644 index 00000000..4adcf04b --- /dev/null +++ b/src/askui/chat/api/executions/dependencies.py @@ -0,0 +1,13 @@ +from fastapi import Depends + +from askui.chat.api.dependencies import SettingsDep +from askui.chat.api.executions.service import ExecutionService +from askui.chat.api.settings import Settings + + +def get_execution_service(settings: Settings = SettingsDep) -> ExecutionService: + """Get ExecutionService instance.""" + return ExecutionService(settings.data_dir) + + +ExecutionServiceDep = Depends(get_execution_service) diff --git a/src/askui/chat/api/executions/models.py b/src/askui/chat/api/executions/models.py new file mode 100644 index 00000000..6d093d71 --- /dev/null +++ b/src/askui/chat/api/executions/models.py @@ -0,0 +1,89 @@ +import datetime +from enum import Enum +from typing import Literal + +from pydantic import BaseModel, Field + +from askui.chat.api.models import ExecutionId, ThreadId, WorkspaceId, WorkspaceResource +from askui.chat.api.workflows.models import WorkflowId +from askui.utils.datetime_utils import now +from askui.utils.id_utils import generate_time_ordered_id +from askui.utils.not_given import NOT_GIVEN, BaseModelWithNotGiven, NotGiven + + +class ExecutionStatus(str, Enum): + """The current status of the workflow execution.""" + + PASSED = "passed" + FAILED = "failed" + PENDING = "pending" + ERROR = "error" + INCOMPLETE = "incomplete" + SKIPPED = "skipped" + INDETERMINATE = "indeterminate" + + +class ExecutionCreateParams(BaseModel): + """ + Parameters for creating an execution via API. + """ + + workflow: WorkflowId + thread: ThreadId + status: ExecutionStatus = ExecutionStatus.PENDING + + +class ExecutionModifyParams(BaseModelWithNotGiven): + """ + Parameters for modifying an execution via API. + Only status can be updated. + """ + + status: ExecutionStatus | NotGiven = NOT_GIVEN + + +class Execution(WorkspaceResource): + """ + A workflow execution resource in the chat API. + + Args: + id (ExecutionId): The id of the execution. Must start with the 'exec_' prefix and be + followed by one or more alphanumerical characters. + object (Literal['execution']): The object type, always 'execution'. + created_at (datetime.datetime): The creation time as a datetime. + workflow (WorkflowId): The id of the workflow being executed. Must start with the 'wf_' prefix. + thread (ThreadId): The id of the thread this execution is associated with. Must start with the 'thread_' prefix. + status (ExecutionStatus): The current status of the workflow execution. + workspace_id (WorkspaceId | None, optional): The workspace this execution belongs to. + """ + + id: ExecutionId + object: Literal["execution"] = "execution" + created_at: datetime.datetime + workflow: WorkflowId + thread: ThreadId + status: ExecutionStatus = Field( + ..., description="The current status of the workflow execution." + ) + + @classmethod + def create( + cls, workspace_id: WorkspaceId | None, params: ExecutionCreateParams + ) -> "Execution": + return cls( + id=generate_time_ordered_id("exec"), + created_at=now(), + workspace_id=workspace_id, + **params.model_dump(), + ) + + def modify(self, params: ExecutionModifyParams) -> "Execution": + update_data = { + k: v for k, v in params.model_dump().items() if v is not NOT_GIVEN + } + return Execution.model_validate( + { + **self.model_dump(), + **update_data, + } + ) diff --git a/src/askui/chat/api/executions/router.py b/src/askui/chat/api/executions/router.py new file mode 100644 index 00000000..60e1ae7f --- /dev/null +++ b/src/askui/chat/api/executions/router.py @@ -0,0 +1,72 @@ +from typing import Annotated + +from fastapi import APIRouter, Header, Query + +from askui.chat.api.dependencies import ListQueryDep +from askui.chat.api.executions.dependencies import ExecutionServiceDep +from askui.chat.api.executions.models import ( + Execution, + ExecutionCreateParams, + ExecutionId, + ExecutionModifyParams, +) +from askui.chat.api.executions.service import ExecutionService +from askui.chat.api.models import ThreadId, WorkspaceId +from askui.chat.api.workflows.models import WorkflowId +from askui.utils.api_utils import ListQuery, ListResponse + +router = APIRouter(prefix="/executions", tags=["executions"]) + + +@router.get("/") +def list_executions( + askui_workspace: Annotated[WorkspaceId, Header()], + query: ListQuery = ListQueryDep, + workflow_id: Annotated[WorkflowId | None, Query()] = None, + thread_id: Annotated[ThreadId | None, Query()] = None, + execution_service: ExecutionService = ExecutionServiceDep, +) -> ListResponse[Execution]: + """List executions with optional filtering by workflow and/or thread.""" + return execution_service.list_( + workspace_id=askui_workspace, + query=query, + workflow_id=workflow_id, + thread_id=thread_id, + ) + + +@router.post("/") +def create_execution( + askui_workspace: Annotated[WorkspaceId, Header()], + params: ExecutionCreateParams, + execution_service: ExecutionService = ExecutionServiceDep, +) -> Execution: + """Create a new workflow execution.""" + return execution_service.create(workspace_id=askui_workspace, params=params) + + +@router.get("/{execution_id}") +def retrieve_execution( + askui_workspace: Annotated[WorkspaceId, Header()], + execution_id: ExecutionId, + execution_service: ExecutionService = ExecutionServiceDep, +) -> Execution: + """Retrieve a specific execution by ID.""" + return execution_service.retrieve( + workspace_id=askui_workspace, execution_id=execution_id + ) + + +@router.patch("/{execution_id}") +def modify_execution( + askui_workspace: Annotated[WorkspaceId, Header()], + execution_id: ExecutionId, + params: ExecutionModifyParams, + execution_service: ExecutionService = ExecutionServiceDep, +) -> Execution: + """Update an existing execution (only status can be modified).""" + return execution_service.modify( + workspace_id=askui_workspace, + execution_id=execution_id, + params=params, + ) diff --git a/src/askui/chat/api/executions/service.py b/src/askui/chat/api/executions/service.py new file mode 100644 index 00000000..d828fd83 --- /dev/null +++ b/src/askui/chat/api/executions/service.py @@ -0,0 +1,126 @@ +from pathlib import Path +from typing import Callable + +from askui.chat.api.executions.models import ( + Execution, + ExecutionCreateParams, + ExecutionId, + ExecutionModifyParams, +) +from askui.chat.api.models import ThreadId, WorkspaceId +from askui.chat.api.utils import build_workspace_filter_fn +from askui.chat.api.workflows.models import WorkflowId +from askui.utils.api_utils import ( + ConflictError, + ListQuery, + ListResponse, + NotFoundError, + list_resources, +) + + +def _build_execution_filter_fn( + workspace_id: WorkspaceId | None, + workflow_id: WorkflowId | None = None, + thread_id: ThreadId | None = None, +) -> Callable[[Execution], bool]: + """Build filter function for executions with optional workflow and thread filters.""" + workspace_filter: Callable[[Execution], bool] = build_workspace_filter_fn( + workspace_id, Execution + ) + + def filter_fn(execution: Execution) -> bool: + if not workspace_filter(execution): + return False + if workflow_id is not None and execution.workflow != workflow_id: + return False + if thread_id is not None and execution.thread != thread_id: + return False + return True + + return filter_fn + + +class ExecutionService: + """Service for managing Execution resources with filesystem persistence.""" + + def __init__(self, base_dir: Path) -> None: + self._base_dir = base_dir + self._executions_dir = base_dir / "executions" + + def _get_execution_path(self, execution_id: ExecutionId, new: bool = False) -> Path: + """Get the file path for an execution.""" + execution_path = self._executions_dir / f"{execution_id}.json" + exists = execution_path.exists() + if new and exists: + error_msg = f"Execution {execution_id} already exists" + raise ConflictError(error_msg) + if not new and not exists: + error_msg = f"Execution {execution_id} not found" + raise NotFoundError(error_msg) + return execution_path + + def list_( + self, + workspace_id: WorkspaceId | None, + query: ListQuery, + workflow_id: WorkflowId | None = None, + thread_id: ThreadId | None = None, + ) -> ListResponse[Execution]: + """List executions with optional filtering by workflow and/or thread.""" + return list_resources( + base_dir=self._executions_dir, + query=query, + resource_type=Execution, + filter_fn=_build_execution_filter_fn(workspace_id, workflow_id, thread_id), + ) + + def retrieve( + self, workspace_id: WorkspaceId | None, execution_id: ExecutionId + ) -> Execution: + """Retrieve a specific execution by ID.""" + try: + execution_path = self._get_execution_path(execution_id) + execution = Execution.model_validate_json(execution_path.read_text()) + + # Check workspace access - allow if workspace_id is None (global access) + # or if execution workspace matches or execution has no workspace + if ( + workspace_id is not None + and execution.workspace_id is not None + and execution.workspace_id != workspace_id + ): + error_msg = f"Execution {execution_id} not found" + raise NotFoundError(error_msg) + + except FileNotFoundError as e: + error_msg = f"Execution {execution_id} not found" + raise NotFoundError(error_msg) from e + else: + return execution + + def create( + self, workspace_id: WorkspaceId | None, params: ExecutionCreateParams + ) -> Execution: + """Create a new execution.""" + execution = Execution.create(workspace_id, params) + self._save(execution, new=True) + return execution + + def modify( + self, + workspace_id: WorkspaceId | None, + execution_id: ExecutionId, + params: ExecutionModifyParams, + ) -> Execution: + """Update an existing execution (only status can be modified).""" + execution = self.retrieve(workspace_id, execution_id) + modified = execution.modify(params) + self._save(modified) + return modified + + def _save(self, execution: Execution, new: bool = False) -> None: + """Save execution to filesystem.""" + self._executions_dir.mkdir(parents=True, exist_ok=True) + execution_file = self._get_execution_path(execution.id, new=new) + execution_file.write_text(execution.model_dump_json(), encoding="utf-8") diff --git a/src/askui/chat/api/models.py b/src/askui/chat/api/models.py index faf1f622..b18bc221 100644 --- a/src/askui/chat/api/models.py +++ b/src/askui/chat/api/models.py @@ -6,6 +6,7 @@ from askui.utils.id_utils import IdField AssistantId = Annotated[str, IdField("asst")] +ExecutionId = Annotated[str, IdField("exec")] McpConfigId = Annotated[str, IdField("mcpcnf")] FileId = Annotated[str, IdField("file")] MessageId = Annotated[str, IdField("msg")] From 90550d41ef371aafb014ac2addffaea6ab48952d Mon Sep 17 00:00:00 2001 From: danyalxahid-askui Date: Mon, 8 Sep 2025 08:58:43 +0200 Subject: [PATCH 02/19] feat(executions): implement status transition validation - Added `InvalidStatusTransitionError` for handling invalid status transitions. - Introduced `_validate_status_transition` function to validate allowed transitions between `ExecutionStatus`. - Updated `Execution.modify` method to validate status changes before applying updates. - Defined a `_STATUS_TRANSITIONS` mapping to specify valid transitions for execution statuses. --- src/askui/chat/api/executions/models.py | 77 ++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 2 deletions(-) diff --git a/src/askui/chat/api/executions/models.py b/src/askui/chat/api/executions/models.py index 6d093d71..dee1620e 100644 --- a/src/askui/chat/api/executions/models.py +++ b/src/askui/chat/api/executions/models.py @@ -17,10 +17,65 @@ class ExecutionStatus(str, Enum): PASSED = "passed" FAILED = "failed" PENDING = "pending" - ERROR = "error" INCOMPLETE = "incomplete" SKIPPED = "skipped" - INDETERMINATE = "indeterminate" + + +class InvalidStatusTransitionError(ValueError): + """Raised when attempting an invalid status transition.""" + + def __init__( + self, from_status: ExecutionStatus, to_status: ExecutionStatus + ) -> None: + error_msg = f"Invalid status transition from '{from_status}' to '{to_status}'" + super().__init__(error_msg) + self.from_status = from_status + self.to_status = to_status + + +# Status transition map: defines which status transitions are allowed +_STATUS_TRANSITIONS: dict[ExecutionStatus, set[ExecutionStatus]] = { + # PENDING can transition to any status except PENDING (to avoid no-op updates) + ExecutionStatus.PENDING: { + ExecutionStatus.INCOMPLETE, + ExecutionStatus.PASSED, + ExecutionStatus.FAILED, + ExecutionStatus.SKIPPED, + }, + # INCOMPLETE can transition to final states or back to PENDING + ExecutionStatus.INCOMPLETE: { + ExecutionStatus.PENDING, + ExecutionStatus.PASSED, + ExecutionStatus.FAILED, + ExecutionStatus.SKIPPED, + }, + # Final states (PASSED, FAILED, SKIPPED) cannot transition to other states + ExecutionStatus.PASSED: set(), + ExecutionStatus.FAILED: set(), + ExecutionStatus.SKIPPED: set(), +} + + +def _validate_status_transition( + from_status: ExecutionStatus, to_status: ExecutionStatus +) -> None: + """ + Validate that a status transition is allowed. + + Args: + from_status (ExecutionStatus): The current status. + to_status (ExecutionStatus): The target status. + + Raises: + InvalidStatusTransitionError: If the transition is not allowed. + """ + if from_status == to_status: + # Allow same-status updates (no-op) + return + + allowed_transitions = _STATUS_TRANSITIONS.get(from_status, set()) + if to_status not in allowed_transitions: + raise InvalidStatusTransitionError(from_status, to_status) class ExecutionCreateParams(BaseModel): @@ -78,9 +133,27 @@ def create( ) def modify(self, params: ExecutionModifyParams) -> "Execution": + """ + Modify the execution with the provided parameters. + + Args: + params (ExecutionModifyParams): The parameters to update. + + Returns: + Execution: A new execution instance with the updated values. + + Raises: + InvalidStatusTransitionError: If attempting an invalid status transition. + """ update_data = { k: v for k, v in params.model_dump().items() if v is not NOT_GIVEN } + + # Validate status transition if status is being updated + if "status" in update_data: + new_status = update_data["status"] + _validate_status_transition(self.status, new_status) + return Execution.model_validate( { **self.model_dump(), From a11880f545efe5c12deb5bf4c1b25070852e33a5 Mon Sep 17 00:00:00 2001 From: danyalxahid-askui Date: Mon, 8 Sep 2025 08:59:57 +0200 Subject: [PATCH 03/19] feat(tests): add integration and unit tests for execution status transitions - Introduced integration tests for execution router and service to validate status transitions. - Added unit tests for the `_validate_status_transition` function to ensure correct transition logic. - Covered scenarios for valid and invalid status transitions, including edge cases for final states. - Ensured persistence of execution modifications across service instances. --- .../chat/api/test_executions_router.py | 321 +++++++++++++++ .../chat/api/test_executions_service.py | 382 ++++++++++++++++++ .../unit/test_execution_status_transitions.py | 167 ++++++++ 3 files changed, 870 insertions(+) create mode 100644 tests/integration/chat/api/test_executions_router.py create mode 100644 tests/integration/chat/api/test_executions_service.py create mode 100644 tests/unit/test_execution_status_transitions.py diff --git a/tests/integration/chat/api/test_executions_router.py b/tests/integration/chat/api/test_executions_router.py new file mode 100644 index 00000000..ffb2a20b --- /dev/null +++ b/tests/integration/chat/api/test_executions_router.py @@ -0,0 +1,321 @@ +"""Integration tests for execution router endpoints with status transition validation.""" + +import uuid + +import pytest +from fastapi.testclient import TestClient + +from askui.chat.api.app import app +from askui.chat.api.executions.models import ExecutionStatus +from askui.chat.api.models import WorkspaceId + + +class TestExecutionRouter: + """Test execution router endpoints with status transition validation.""" + + @pytest.fixture + def client(self) -> TestClient: + """Create a test client for the FastAPI app.""" + return TestClient(app) + + @pytest.fixture + def workspace_id(self) -> WorkspaceId: + """Create a test workspace ID.""" + return uuid.uuid4() + + @pytest.fixture + def execution_data(self) -> dict[str, str]: + """Create sample execution data.""" + return { + "workflow": "wf_test123", + "thread": "thread_test123", + "status": ExecutionStatus.PENDING.value, + } + + def test_create_execution_success( + self, + client: TestClient, + workspace_id: WorkspaceId, + execution_data: dict[str, str], + ) -> None: + """Test successful execution creation.""" + response = client.post( + "/executions/", + json=execution_data, + headers={"askui-workspace": str(workspace_id)}, + ) + + assert response.status_code == 200 + data = response.json() + assert data["workflow"] == execution_data["workflow"] + assert data["thread"] == execution_data["thread"] + assert data["status"] == ExecutionStatus.PENDING.value + assert data["object"] == "execution" + assert "id" in data + assert "created_at" in data + + def test_modify_execution_valid_transition( + self, + client: TestClient, + workspace_id: WorkspaceId, + execution_data: dict[str, str], + ) -> None: + """Test successful execution modification with valid status transition.""" + # First create an execution + create_response = client.post( + "/executions/", + json=execution_data, + headers={"askui-workspace": str(workspace_id)}, + ) + assert create_response.status_code == 200 + execution = create_response.json() + execution_id = execution["id"] + + # Then modify it with a valid transition (PENDING -> PASSED) + modify_data = {"status": ExecutionStatus.PASSED.value} + modify_response = client.patch( + f"/executions/{execution_id}", + json=modify_data, + headers={"askui-workspace": str(workspace_id)}, + ) + + assert modify_response.status_code == 200 + modified_execution = modify_response.json() + assert modified_execution["status"] == ExecutionStatus.PASSED.value + assert modified_execution["id"] == execution_id + + def test_modify_execution_invalid_transition( + self, + client: TestClient, + workspace_id: WorkspaceId, + execution_data: dict[str, str], + ) -> None: + """Test execution modification with invalid status transition returns 422.""" + # First create an execution and transition it to a final state + create_response = client.post( + "/executions/", + json=execution_data, + headers={"askui-workspace": str(workspace_id)}, + ) + assert create_response.status_code == 200 + execution = create_response.json() + execution_id = execution["id"] + + # Transition to final state (PENDING -> PASSED) + modify_data = {"status": ExecutionStatus.PASSED.value} + modify_response = client.patch( + f"/executions/{execution_id}", + json=modify_data, + headers={"askui-workspace": str(workspace_id)}, + ) + assert modify_response.status_code == 200 + + # Try to transition from final state to non-final state (PASSED -> PENDING) + invalid_modify_data = {"status": ExecutionStatus.PENDING.value} + invalid_response = client.patch( + f"/executions/{execution_id}", + json=invalid_modify_data, + headers={"askui-workspace": str(workspace_id)}, + ) + + # FastAPI should convert ValueError to 422 Unprocessable Entity + assert invalid_response.status_code == 422 + error_data = invalid_response.json() + assert "detail" in error_data + # The error should contain information about the invalid transition + error_detail = str(error_data["detail"]) + assert "Invalid status transition" in error_detail + assert "passed" in error_detail + assert "pending" in error_detail + + def test_modify_execution_same_status_allowed( + self, + client: TestClient, + workspace_id: WorkspaceId, + execution_data: dict[str, str], + ) -> None: + """Test that modifying to the same status is allowed (no-op).""" + # Create an execution + create_response = client.post( + "/executions/", + json=execution_data, + headers={"askui-workspace": str(workspace_id)}, + ) + assert create_response.status_code == 200 + execution = create_response.json() + execution_id = execution["id"] + + # Modify to the same status (PENDING -> PENDING) + modify_data = {"status": ExecutionStatus.PENDING.value} + modify_response = client.patch( + f"/executions/{execution_id}", + json=modify_data, + headers={"askui-workspace": str(workspace_id)}, + ) + + assert modify_response.status_code == 200 + modified_execution = modify_response.json() + assert modified_execution["status"] == ExecutionStatus.PENDING.value + + def test_modify_execution_multiple_valid_transitions( + self, + client: TestClient, + workspace_id: WorkspaceId, + execution_data: dict[str, str], + ) -> None: + """Test multiple valid status transitions in sequence.""" + # Create an execution + create_response = client.post( + "/executions/", + json=execution_data, + headers={"askui-workspace": str(workspace_id)}, + ) + assert create_response.status_code == 200 + execution = create_response.json() + execution_id = execution["id"] + + # Valid transition sequence: PENDING -> INCOMPLETE -> PASSED + transitions = [ + (ExecutionStatus.INCOMPLETE.value, 200), + (ExecutionStatus.PASSED.value, 200), + ] + + for status, expected_code in transitions: + modify_data = {"status": status} + modify_response = client.patch( + f"/executions/{execution_id}", + json=modify_data, + headers={"askui-workspace": str(workspace_id)}, + ) + + assert modify_response.status_code == expected_code + if expected_code == 200: + modified_execution = modify_response.json() + assert modified_execution["status"] == status + + def test_modify_execution_not_found( + self, client: TestClient, workspace_id: WorkspaceId + ) -> None: + """Test modifying non-existent execution returns 404.""" + modify_data = {"status": ExecutionStatus.PASSED.value} + response = client.patch( + "/executions/exec_nonexistent123", + json=modify_data, + headers={"askui-workspace": str(workspace_id)}, + ) + + assert response.status_code == 404 + + def test_retrieve_execution_success( + self, + client: TestClient, + workspace_id: WorkspaceId, + execution_data: dict[str, str], + ) -> None: + """Test successful execution retrieval.""" + # Create an execution + create_response = client.post( + "/executions/", + json=execution_data, + headers={"askui-workspace": str(workspace_id)}, + ) + assert create_response.status_code == 200 + execution = create_response.json() + execution_id = execution["id"] + + # Retrieve the execution + retrieve_response = client.get( + f"/executions/{execution_id}", + headers={"askui-workspace": str(workspace_id)}, + ) + + assert retrieve_response.status_code == 200 + retrieved_execution = retrieve_response.json() + assert retrieved_execution["id"] == execution_id + assert retrieved_execution["status"] == ExecutionStatus.PENDING.value + + def test_list_executions_success( + self, + client: TestClient, + workspace_id: WorkspaceId, + execution_data: dict[str, str], + ) -> None: + """Test successful execution listing.""" + # Create an execution + create_response = client.post( + "/executions/", + json=execution_data, + headers={"askui-workspace": str(workspace_id)}, + ) + assert create_response.status_code == 200 + + # List executions + list_response = client.get( + "/executions/", + headers={"askui-workspace": str(workspace_id)}, + ) + + assert list_response.status_code == 200 + data = list_response.json() + assert "data" in data + assert "object" in data + assert data["object"] == "list" + assert len(data["data"]) >= 1 + + @pytest.mark.parametrize( + "final_status", + [ + ExecutionStatus.PASSED, + ExecutionStatus.FAILED, + ExecutionStatus.SKIPPED, + ], + ) + @pytest.mark.parametrize( + "invalid_target", + [ + ExecutionStatus.PENDING, + ExecutionStatus.INCOMPLETE, + ], + ) + def test_final_states_cannot_transition_parametrized( + self, + client: TestClient, + workspace_id: WorkspaceId, + execution_data: dict[str, str], + final_status: ExecutionStatus, + invalid_target: ExecutionStatus, + ) -> None: + """Test that final states cannot transition to non-final states (parametrized).""" + # Create an execution + create_response = client.post( + "/executions/", + json=execution_data, + headers={"askui-workspace": str(workspace_id)}, + ) + assert create_response.status_code == 200 + execution = create_response.json() + execution_id = execution["id"] + + # Transition to final state + modify_data = {"status": final_status.value} + modify_response = client.patch( + f"/executions/{execution_id}", + json=modify_data, + headers={"askui-workspace": str(workspace_id)}, + ) + assert modify_response.status_code == 200 + + # Try to transition to invalid target + invalid_modify_data = {"status": invalid_target.value} + invalid_response = client.patch( + f"/executions/{execution_id}", + json=invalid_modify_data, + headers={"askui-workspace": str(workspace_id)}, + ) + + assert invalid_response.status_code == 422 + error_data = invalid_response.json() + error_detail = str(error_data["detail"]) + assert "Invalid status transition" in error_detail + assert final_status.value in error_detail + assert invalid_target.value in error_detail diff --git a/tests/integration/chat/api/test_executions_service.py b/tests/integration/chat/api/test_executions_service.py new file mode 100644 index 00000000..029b8f95 --- /dev/null +++ b/tests/integration/chat/api/test_executions_service.py @@ -0,0 +1,382 @@ +"""Integration tests for execution service with status transition validation.""" + +import uuid +from pathlib import Path + +import pytest + +from askui.chat.api.executions.models import ( + Execution, + ExecutionCreateParams, + ExecutionModifyParams, + ExecutionStatus, + InvalidStatusTransitionError, +) +from askui.chat.api.executions.service import ExecutionService +from askui.chat.api.models import WorkspaceId +from askui.utils.api_utils import ListQuery, NotFoundError + + +class TestExecutionService: + """Test execution service with status transition validation.""" + + @pytest.fixture + def execution_service(self, tmp_path: Path) -> ExecutionService: + """Create an execution service for testing.""" + return ExecutionService(tmp_path) + + @pytest.fixture + def workspace_id(self) -> WorkspaceId: + """Create a test workspace ID.""" + return uuid.uuid4() + + @pytest.fixture + def create_params(self) -> ExecutionCreateParams: + """Create sample execution creation parameters.""" + return ExecutionCreateParams( + workflow="wf_test123", + thread="thread_test123", + status=ExecutionStatus.PENDING, + ) + + @pytest.fixture + def sample_execution( + self, + execution_service: ExecutionService, + workspace_id: WorkspaceId, + create_params: ExecutionCreateParams, + ) -> Execution: + """Create a sample execution for testing.""" + return execution_service.create(workspace_id=workspace_id, params=create_params) + + def test_create_execution_success( + self, + execution_service: ExecutionService, + workspace_id: WorkspaceId, + create_params: ExecutionCreateParams, + ) -> None: + """Test successful execution creation.""" + execution = execution_service.create( + workspace_id=workspace_id, params=create_params + ) + + assert execution.workflow == create_params.workflow + assert execution.thread == create_params.thread + assert execution.status == create_params.status + assert execution.workspace_id == workspace_id + assert execution.object == "execution" + assert execution.id.startswith("exec_") + assert execution.created_at is not None + + def test_retrieve_execution_success( + self, + execution_service: ExecutionService, + workspace_id: WorkspaceId, + sample_execution: Execution, + ) -> None: + """Test successful execution retrieval.""" + retrieved = execution_service.retrieve( + workspace_id=workspace_id, execution_id=sample_execution.id + ) + + assert retrieved.id == sample_execution.id + assert retrieved.workflow == sample_execution.workflow + assert retrieved.thread == sample_execution.thread + assert retrieved.status == sample_execution.status + assert retrieved.workspace_id == sample_execution.workspace_id + + def test_retrieve_execution_not_found( + self, execution_service: ExecutionService, workspace_id: WorkspaceId + ) -> None: + """Test retrieving non-existent execution raises NotFoundError.""" + with pytest.raises(NotFoundError) as exc_info: + execution_service.retrieve( + workspace_id=workspace_id, execution_id="exec_nonexistent123" + ) + + assert "not found" in str(exc_info.value) + + def test_modify_execution_valid_transition( + self, + execution_service: ExecutionService, + workspace_id: WorkspaceId, + sample_execution: Execution, + ) -> None: + """Test successful execution modification with valid status transition.""" + modify_params = ExecutionModifyParams(status=ExecutionStatus.PASSED) + modified = execution_service.modify( + workspace_id=workspace_id, + execution_id=sample_execution.id, + params=modify_params, + ) + + assert modified.id == sample_execution.id + assert modified.status == ExecutionStatus.PASSED + assert modified.workflow == sample_execution.workflow + assert modified.thread == sample_execution.thread + + def test_modify_execution_invalid_transition_raises_error( + self, + execution_service: ExecutionService, + workspace_id: WorkspaceId, + sample_execution: Execution, + ) -> None: + """Test that invalid status transition raises InvalidStatusTransitionError.""" + # First transition to a final state + modify_params = ExecutionModifyParams(status=ExecutionStatus.PASSED) + execution_service.modify( + workspace_id=workspace_id, + execution_id=sample_execution.id, + params=modify_params, + ) + + # Try to transition from final state to non-final state + invalid_params = ExecutionModifyParams(status=ExecutionStatus.PENDING) + with pytest.raises(InvalidStatusTransitionError) as exc_info: + execution_service.modify( + workspace_id=workspace_id, + execution_id=sample_execution.id, + params=invalid_params, + ) + + assert exc_info.value.from_status == ExecutionStatus.PASSED + assert exc_info.value.to_status == ExecutionStatus.PENDING + assert "Invalid status transition from 'passed' to 'pending'" in str( + exc_info.value + ) + + def test_modify_execution_same_status_allowed( + self, + execution_service: ExecutionService, + workspace_id: WorkspaceId, + sample_execution: Execution, + ) -> None: + """Test that modifying to the same status is allowed (no-op).""" + modify_params = ExecutionModifyParams(status=ExecutionStatus.PENDING) + modified = execution_service.modify( + workspace_id=workspace_id, + execution_id=sample_execution.id, + params=modify_params, + ) + + assert modified.status == ExecutionStatus.PENDING + assert modified.id == sample_execution.id + + def test_modify_execution_not_found( + self, execution_service: ExecutionService, workspace_id: WorkspaceId + ) -> None: + """Test modifying non-existent execution raises NotFoundError.""" + modify_params = ExecutionModifyParams(status=ExecutionStatus.PASSED) + + with pytest.raises(NotFoundError) as exc_info: + execution_service.modify( + workspace_id=workspace_id, + execution_id="exec_nonexistent123", + params=modify_params, + ) + + assert "not found" in str(exc_info.value) + + def test_list_executions_success( + self, + execution_service: ExecutionService, + workspace_id: WorkspaceId, + sample_execution: Execution, + ) -> None: + """Test successful execution listing.""" + query = ListQuery(limit=10, order="desc") + result = execution_service.list_(workspace_id=workspace_id, query=query) + + assert result.object == "list" + assert len(result.data) >= 1 + assert any(execution.id == sample_execution.id for execution in result.data) + + def test_list_executions_with_filters( + self, + execution_service: ExecutionService, + workspace_id: WorkspaceId, + create_params: ExecutionCreateParams, + ) -> None: + """Test execution listing with workflow and thread filters.""" + # Create multiple executions with different workflows and threads + execution_service.create(workspace_id=workspace_id, params=create_params) + + different_params = ExecutionCreateParams( + workflow="wf_different456", + thread="thread_different456", + status=ExecutionStatus.PENDING, + ) + execution_service.create(workspace_id=workspace_id, params=different_params) + + query = ListQuery(limit=10, order="desc") + + # Test workflow filter + workflow_filtered = execution_service.list_( + workspace_id=workspace_id, + query=query, + workflow_id=create_params.workflow, + ) + assert len(workflow_filtered.data) >= 1 + assert all( + exec.workflow == create_params.workflow for exec in workflow_filtered.data + ) + + # Test thread filter + thread_filtered = execution_service.list_( + workspace_id=workspace_id, + query=query, + thread_id=create_params.thread, + ) + assert len(thread_filtered.data) >= 1 + assert all(exec.thread == create_params.thread for exec in thread_filtered.data) + + # Test combined filters + combined_filtered = execution_service.list_( + workspace_id=workspace_id, + query=query, + workflow_id=create_params.workflow, + thread_id=create_params.thread, + ) + assert len(combined_filtered.data) >= 1 + assert all( + exec.workflow == create_params.workflow + and exec.thread == create_params.thread + for exec in combined_filtered.data + ) + + def test_persistence_across_service_instances( + self, + tmp_path: Path, + workspace_id: WorkspaceId, + create_params: ExecutionCreateParams, + ) -> None: + """Test that executions persist across service instances.""" + # Create execution with first service instance + service1 = ExecutionService(tmp_path) + execution = service1.create(workspace_id=workspace_id, params=create_params) + + # Retrieve with second service instance + service2 = ExecutionService(tmp_path) + retrieved = service2.retrieve( + workspace_id=workspace_id, execution_id=execution.id + ) + + assert retrieved.id == execution.id + assert retrieved.status == execution.status + assert retrieved.workflow == execution.workflow + + def test_modify_execution_persists_changes( + self, + tmp_path: Path, + workspace_id: WorkspaceId, + create_params: ExecutionCreateParams, + ) -> None: + """Test that execution modifications are persisted to filesystem.""" + # Create execution + service1 = ExecutionService(tmp_path) + execution = service1.create(workspace_id=workspace_id, params=create_params) + + # Modify execution + modify_params = ExecutionModifyParams(status=ExecutionStatus.PASSED) + service1.modify( + workspace_id=workspace_id, + execution_id=execution.id, + params=modify_params, + ) + + # Verify changes persist with new service instance + service2 = ExecutionService(tmp_path) + retrieved = service2.retrieve( + workspace_id=workspace_id, execution_id=execution.id + ) + assert retrieved.status == ExecutionStatus.PASSED + + @pytest.mark.parametrize( + "valid_transition", + [ + (ExecutionStatus.PENDING, ExecutionStatus.INCOMPLETE), + (ExecutionStatus.PENDING, ExecutionStatus.PASSED), + (ExecutionStatus.PENDING, ExecutionStatus.FAILED), + (ExecutionStatus.PENDING, ExecutionStatus.SKIPPED), + (ExecutionStatus.INCOMPLETE, ExecutionStatus.PENDING), + (ExecutionStatus.INCOMPLETE, ExecutionStatus.PASSED), + (ExecutionStatus.INCOMPLETE, ExecutionStatus.FAILED), + (ExecutionStatus.INCOMPLETE, ExecutionStatus.SKIPPED), + ], + ) + def test_valid_transitions_parametrized( + self, + execution_service: ExecutionService, + workspace_id: WorkspaceId, + valid_transition: tuple[ExecutionStatus, ExecutionStatus], + ) -> None: + """Test all valid status transitions (parametrized).""" + from_status, to_status = valid_transition + + # Create execution with initial status + create_params = ExecutionCreateParams( + workflow="wf_test123", + thread="thread_test123", + status=from_status, + ) + execution = execution_service.create( + workspace_id=workspace_id, params=create_params + ) + + # Modify to target status + modify_params = ExecutionModifyParams(status=to_status) + modified = execution_service.modify( + workspace_id=workspace_id, + execution_id=execution.id, + params=modify_params, + ) + + assert modified.status == to_status + + @pytest.mark.parametrize( + "invalid_transition", + [ + (ExecutionStatus.PASSED, ExecutionStatus.PENDING), + (ExecutionStatus.PASSED, ExecutionStatus.INCOMPLETE), + (ExecutionStatus.PASSED, ExecutionStatus.FAILED), + (ExecutionStatus.PASSED, ExecutionStatus.SKIPPED), + (ExecutionStatus.FAILED, ExecutionStatus.PENDING), + (ExecutionStatus.FAILED, ExecutionStatus.INCOMPLETE), + (ExecutionStatus.FAILED, ExecutionStatus.PASSED), + (ExecutionStatus.FAILED, ExecutionStatus.SKIPPED), + (ExecutionStatus.SKIPPED, ExecutionStatus.PENDING), + (ExecutionStatus.SKIPPED, ExecutionStatus.INCOMPLETE), + (ExecutionStatus.SKIPPED, ExecutionStatus.PASSED), + (ExecutionStatus.SKIPPED, ExecutionStatus.FAILED), + ], + ) + def test_invalid_transitions_parametrized( + self, + execution_service: ExecutionService, + workspace_id: WorkspaceId, + invalid_transition: tuple[ExecutionStatus, ExecutionStatus], + ) -> None: + """Test all invalid status transitions (parametrized).""" + from_status, to_status = invalid_transition + + # Create execution with initial status + create_params = ExecutionCreateParams( + workflow="wf_test123", + thread="thread_test123", + status=from_status, + ) + execution = execution_service.create( + workspace_id=workspace_id, params=create_params + ) + + # Try to modify to invalid target status + modify_params = ExecutionModifyParams(status=to_status) + with pytest.raises(InvalidStatusTransitionError) as exc_info: + execution_service.modify( + workspace_id=workspace_id, + execution_id=execution.id, + params=modify_params, + ) + + assert exc_info.value.from_status == from_status + assert exc_info.value.to_status == to_status diff --git a/tests/unit/test_execution_status_transitions.py b/tests/unit/test_execution_status_transitions.py new file mode 100644 index 00000000..74a8bb8a --- /dev/null +++ b/tests/unit/test_execution_status_transitions.py @@ -0,0 +1,167 @@ +"""Unit tests for execution status transition validation logic.""" + +import pytest + +from askui.chat.api.executions.models import ( + ExecutionStatus, + InvalidStatusTransitionError, + _validate_status_transition, +) + + +class TestStatusTransitionValidation: + """Test the low-level status transition validation function.""" + + def test_same_status_transition_allowed(self) -> None: + """Test that transitioning to the same status is always allowed.""" + for status in ExecutionStatus: + # Should not raise an exception + _validate_status_transition(status, status) + + def test_valid_transitions_from_pending(self) -> None: + """Test valid transitions from PENDING status.""" + valid_targets = [ + ExecutionStatus.INCOMPLETE, + ExecutionStatus.PASSED, + ExecutionStatus.FAILED, + ExecutionStatus.SKIPPED, + ] + + for target in valid_targets: + _validate_status_transition(ExecutionStatus.PENDING, target) + + def test_invalid_transitions_from_pending(self) -> None: + """Test that PENDING cannot transition back to PENDING (via validation logic).""" + # Note: same-status transitions are allowed at the validation level, + # but this tests the transition map logic + # PENDING -> PENDING is actually allowed as a no-op + return + + def test_valid_transitions_from_incomplete(self) -> None: + """Test valid transitions from INCOMPLETE status.""" + valid_targets = [ + ExecutionStatus.PENDING, + ExecutionStatus.PASSED, + ExecutionStatus.FAILED, + ExecutionStatus.SKIPPED, + ] + + for target in valid_targets: + _validate_status_transition(ExecutionStatus.INCOMPLETE, target) + + def test_no_transitions_from_final_states(self) -> None: + """Test that final states cannot transition to any other status.""" + final_states = [ + ExecutionStatus.PASSED, + ExecutionStatus.FAILED, + ExecutionStatus.SKIPPED, + ] + + all_other_statuses = [ + ExecutionStatus.PENDING, + ExecutionStatus.INCOMPLETE, + ] + + for final_state in final_states: + for target in all_other_statuses: + with pytest.raises(InvalidStatusTransitionError) as exc_info: + _validate_status_transition(final_state, target) + + assert exc_info.value.from_status == final_state + assert exc_info.value.to_status == target + + def test_final_states_cannot_transition_to_each_other(self) -> None: + """Test that final states cannot transition to other final states.""" + final_states = [ + ExecutionStatus.PASSED, + ExecutionStatus.FAILED, + ExecutionStatus.SKIPPED, + ] + + for from_state in final_states: + for to_state in final_states: + if from_state == to_state: + # Same status is allowed + continue + + with pytest.raises(InvalidStatusTransitionError) as exc_info: + _validate_status_transition(from_state, to_state) + + assert exc_info.value.from_status == from_state + assert exc_info.value.to_status == to_state + + def test_error_message_format(self) -> None: + """Test that the error message has the expected format.""" + with pytest.raises(InvalidStatusTransitionError) as exc_info: + _validate_status_transition(ExecutionStatus.PASSED, ExecutionStatus.PENDING) + + error = exc_info.value + expected_msg = "Invalid status transition from 'passed' to 'pending'" + assert str(error) == expected_msg + + def test_error_attributes(self) -> None: + """Test that the error has the correct attributes.""" + from_status = ExecutionStatus.FAILED + to_status = ExecutionStatus.INCOMPLETE + + with pytest.raises(InvalidStatusTransitionError) as exc_info: + _validate_status_transition(from_status, to_status) + + error = exc_info.value + assert error.from_status == from_status + assert error.to_status == to_status + assert isinstance(error, ValueError) + + @pytest.mark.parametrize( + "final_state", + [ + ExecutionStatus.PASSED, + ExecutionStatus.FAILED, + ExecutionStatus.SKIPPED, + ], + ) + @pytest.mark.parametrize( + "target_state", + [ + ExecutionStatus.PENDING, + ExecutionStatus.INCOMPLETE, + ], + ) + def test_specific_invalid_transitions_parametrized( + self, final_state: ExecutionStatus, target_state: ExecutionStatus + ) -> None: + """Test specific invalid transitions mentioned in requirements using parametrization.""" + with pytest.raises(InvalidStatusTransitionError): + _validate_status_transition(final_state, target_state) + + def test_transition_map_structure(self) -> None: + """Test the structure of the transition map.""" + from askui.chat.api.executions.models import _STATUS_TRANSITIONS + + # All statuses should be keys in the transition map + all_statuses = set(ExecutionStatus) + map_keys = set(_STATUS_TRANSITIONS.keys()) + assert all_statuses == map_keys + + # Final states should have empty transition sets + final_states = [ + ExecutionStatus.PASSED, + ExecutionStatus.FAILED, + ExecutionStatus.SKIPPED, + ] + + for final_state in final_states: + assert _STATUS_TRANSITIONS[final_state] == set(), ( + f"{final_state} should have no allowed transitions" + ) + + # Non-final states should have non-empty transition sets + non_final_states = [ + ExecutionStatus.PENDING, + ExecutionStatus.INCOMPLETE, + ] + + for non_final_state in non_final_states: + assert len(_STATUS_TRANSITIONS[non_final_state]) > 0, ( + f"{non_final_state} should have allowed transitions" + ) From 74a7ca9d2ac7e4b55acaec3841f909ff5a0a7972 Mon Sep 17 00:00:00 2001 From: danyalxahid-askui Date: Mon, 8 Sep 2025 09:23:19 +0200 Subject: [PATCH 04/19] refactor(executions): update execution status handling and tests - Revised `_STATUS_TRANSITIONS` to clarify that `INCOMPLETE` can only transition to final states. - Removed redundant `status` field from `ExecutionCreateParams` and set default status to `PENDING` in `Execution` initialization. - Updated integration tests to reflect changes in status handling, ensuring correct transitions from `PENDING` and `INCOMPLETE`. - Added unit test to validate that `INCOMPLETE` cannot transition back to `PENDING`. --- src/askui/chat/api/executions/models.py | 5 +- .../chat/api/test_executions_router.py | 1 - .../chat/api/test_executions_service.py | 177 +++++++++++++----- .../unit/test_execution_status_transitions.py | 11 +- 4 files changed, 145 insertions(+), 49 deletions(-) diff --git a/src/askui/chat/api/executions/models.py b/src/askui/chat/api/executions/models.py index dee1620e..0e72fd38 100644 --- a/src/askui/chat/api/executions/models.py +++ b/src/askui/chat/api/executions/models.py @@ -42,9 +42,8 @@ def __init__( ExecutionStatus.FAILED, ExecutionStatus.SKIPPED, }, - # INCOMPLETE can transition to final states or back to PENDING + # INCOMPLETE can only transition to final states (no going backwards) ExecutionStatus.INCOMPLETE: { - ExecutionStatus.PENDING, ExecutionStatus.PASSED, ExecutionStatus.FAILED, ExecutionStatus.SKIPPED, @@ -85,7 +84,6 @@ class ExecutionCreateParams(BaseModel): workflow: WorkflowId thread: ThreadId - status: ExecutionStatus = ExecutionStatus.PENDING class ExecutionModifyParams(BaseModelWithNotGiven): @@ -129,6 +127,7 @@ def create( id=generate_time_ordered_id("exec"), created_at=now(), workspace_id=workspace_id, + status=ExecutionStatus.PENDING, **params.model_dump(), ) diff --git a/tests/integration/chat/api/test_executions_router.py b/tests/integration/chat/api/test_executions_router.py index ffb2a20b..8977a343 100644 --- a/tests/integration/chat/api/test_executions_router.py +++ b/tests/integration/chat/api/test_executions_router.py @@ -29,7 +29,6 @@ def execution_data(self) -> dict[str, str]: return { "workflow": "wf_test123", "thread": "thread_test123", - "status": ExecutionStatus.PENDING.value, } def test_create_execution_success( diff --git a/tests/integration/chat/api/test_executions_service.py b/tests/integration/chat/api/test_executions_service.py index 029b8f95..aa0761bd 100644 --- a/tests/integration/chat/api/test_executions_service.py +++ b/tests/integration/chat/api/test_executions_service.py @@ -36,7 +36,6 @@ def create_params(self) -> ExecutionCreateParams: return ExecutionCreateParams( workflow="wf_test123", thread="thread_test123", - status=ExecutionStatus.PENDING, ) @pytest.fixture @@ -62,7 +61,7 @@ def test_create_execution_success( assert execution.workflow == create_params.workflow assert execution.thread == create_params.thread - assert execution.status == create_params.status + assert execution.status == ExecutionStatus.PENDING assert execution.workspace_id == workspace_id assert execution.object == "execution" assert execution.id.startswith("exec_") @@ -204,7 +203,6 @@ def test_list_executions_with_filters( different_params = ExecutionCreateParams( workflow="wf_different456", thread="thread_different456", - status=ExecutionStatus.PENDING, ) execution_service.create(workspace_id=workspace_id, params=different_params) @@ -292,85 +290,176 @@ def test_modify_execution_persists_changes( assert retrieved.status == ExecutionStatus.PASSED @pytest.mark.parametrize( - "valid_transition", + "target_status", [ - (ExecutionStatus.PENDING, ExecutionStatus.INCOMPLETE), - (ExecutionStatus.PENDING, ExecutionStatus.PASSED), - (ExecutionStatus.PENDING, ExecutionStatus.FAILED), - (ExecutionStatus.PENDING, ExecutionStatus.SKIPPED), - (ExecutionStatus.INCOMPLETE, ExecutionStatus.PENDING), - (ExecutionStatus.INCOMPLETE, ExecutionStatus.PASSED), - (ExecutionStatus.INCOMPLETE, ExecutionStatus.FAILED), - (ExecutionStatus.INCOMPLETE, ExecutionStatus.SKIPPED), + ExecutionStatus.INCOMPLETE, + ExecutionStatus.PASSED, + ExecutionStatus.FAILED, + ExecutionStatus.SKIPPED, ], ) - def test_valid_transitions_parametrized( + def test_valid_transitions_from_pending( self, execution_service: ExecutionService, workspace_id: WorkspaceId, - valid_transition: tuple[ExecutionStatus, ExecutionStatus], + target_status: ExecutionStatus, ) -> None: - """Test all valid status transitions (parametrized).""" - from_status, to_status = valid_transition + """Test all valid transitions from PENDING status (parametrized).""" + # Create execution (always starts as PENDING) + create_params = ExecutionCreateParams( + workflow="wf_test123", + thread="thread_test123", + ) + execution = execution_service.create( + workspace_id=workspace_id, params=create_params + ) + + # Test transition from PENDING to target status + modify_params = ExecutionModifyParams(status=target_status) + modified = execution_service.modify( + workspace_id=workspace_id, + execution_id=execution.id, + params=modify_params, + ) + + assert modified.status == target_status - # Create execution with initial status + @pytest.mark.parametrize( + "target_status", + [ + ExecutionStatus.PASSED, + ExecutionStatus.FAILED, + ExecutionStatus.SKIPPED, + ], + ) + def test_valid_transitions_from_incomplete( + self, + execution_service: ExecutionService, + workspace_id: WorkspaceId, + target_status: ExecutionStatus, + ) -> None: + """Test all valid transitions from INCOMPLETE status (parametrized).""" + # Create execution and move to INCOMPLETE create_params = ExecutionCreateParams( workflow="wf_test123", thread="thread_test123", - status=from_status, ) execution = execution_service.create( workspace_id=workspace_id, params=create_params ) - # Modify to target status - modify_params = ExecutionModifyParams(status=to_status) + # First move to INCOMPLETE + incomplete_params = ExecutionModifyParams(status=ExecutionStatus.INCOMPLETE) + execution = execution_service.modify( + workspace_id=workspace_id, + execution_id=execution.id, + params=incomplete_params, + ) + + # Test transition from INCOMPLETE to target status + modify_params = ExecutionModifyParams(status=target_status) modified = execution_service.modify( workspace_id=workspace_id, execution_id=execution.id, params=modify_params, ) - assert modified.status == to_status + assert modified.status == target_status + + def test_incomplete_cannot_go_back_to_pending( + self, + execution_service: ExecutionService, + workspace_id: WorkspaceId, + ) -> None: + """Test that INCOMPLETE cannot transition back to PENDING.""" + # Create execution and move to INCOMPLETE + create_params = ExecutionCreateParams( + workflow="wf_test123", + thread="thread_test123", + ) + execution = execution_service.create( + workspace_id=workspace_id, params=create_params + ) + + # Move to INCOMPLETE + incomplete_params = ExecutionModifyParams(status=ExecutionStatus.INCOMPLETE) + execution = execution_service.modify( + workspace_id=workspace_id, + execution_id=execution.id, + params=incomplete_params, + ) + # Try to go back to PENDING (should fail) + pending_params = ExecutionModifyParams(status=ExecutionStatus.PENDING) + with pytest.raises(InvalidStatusTransitionError) as exc_info: + execution_service.modify( + workspace_id=workspace_id, + execution_id=execution.id, + params=pending_params, + ) + + assert exc_info.value.from_status == ExecutionStatus.INCOMPLETE + assert exc_info.value.to_status == ExecutionStatus.PENDING + + @pytest.mark.parametrize( + "final_status", + [ + ExecutionStatus.PASSED, + ExecutionStatus.FAILED, + ExecutionStatus.SKIPPED, + ], + ) @pytest.mark.parametrize( - "invalid_transition", + "target_status", [ - (ExecutionStatus.PASSED, ExecutionStatus.PENDING), - (ExecutionStatus.PASSED, ExecutionStatus.INCOMPLETE), - (ExecutionStatus.PASSED, ExecutionStatus.FAILED), - (ExecutionStatus.PASSED, ExecutionStatus.SKIPPED), - (ExecutionStatus.FAILED, ExecutionStatus.PENDING), - (ExecutionStatus.FAILED, ExecutionStatus.INCOMPLETE), - (ExecutionStatus.FAILED, ExecutionStatus.PASSED), - (ExecutionStatus.FAILED, ExecutionStatus.SKIPPED), - (ExecutionStatus.SKIPPED, ExecutionStatus.PENDING), - (ExecutionStatus.SKIPPED, ExecutionStatus.INCOMPLETE), - (ExecutionStatus.SKIPPED, ExecutionStatus.PASSED), - (ExecutionStatus.SKIPPED, ExecutionStatus.FAILED), + ExecutionStatus.PENDING, + ExecutionStatus.INCOMPLETE, + ExecutionStatus.PASSED, + ExecutionStatus.FAILED, + ExecutionStatus.SKIPPED, ], ) - def test_invalid_transitions_parametrized( + def test_final_states_cannot_transition( self, execution_service: ExecutionService, workspace_id: WorkspaceId, - invalid_transition: tuple[ExecutionStatus, ExecutionStatus], + final_status: ExecutionStatus, + target_status: ExecutionStatus, ) -> None: - """Test all invalid status transitions (parametrized).""" - from_status, to_status = invalid_transition + """Test that final states cannot transition to any other status (parametrized).""" + # Skip same-status transitions (they're allowed as no-ops) + if final_status == target_status: + pytest.skip("Same-status transitions are allowed as no-ops") - # Create execution with initial status + # Create execution and move to final state create_params = ExecutionCreateParams( workflow="wf_test123", thread="thread_test123", - status=from_status, ) execution = execution_service.create( workspace_id=workspace_id, params=create_params ) - # Try to modify to invalid target status - modify_params = ExecutionModifyParams(status=to_status) + # Move to final status (via INCOMPLETE if needed for realistic flow) + if final_status in [ExecutionStatus.PASSED, ExecutionStatus.FAILED]: + # Realistic flow: PENDING → INCOMPLETE → PASSED/FAILED + incomplete_params = ExecutionModifyParams(status=ExecutionStatus.INCOMPLETE) + execution = execution_service.modify( + workspace_id=workspace_id, + execution_id=execution.id, + params=incomplete_params, + ) + + # Move to final status + final_params = ExecutionModifyParams(status=final_status) + execution = execution_service.modify( + workspace_id=workspace_id, + execution_id=execution.id, + params=final_params, + ) + + # Try to transition from final state to target status (should fail) + modify_params = ExecutionModifyParams(status=target_status) with pytest.raises(InvalidStatusTransitionError) as exc_info: execution_service.modify( workspace_id=workspace_id, @@ -378,5 +467,5 @@ def test_invalid_transitions_parametrized( params=modify_params, ) - assert exc_info.value.from_status == from_status - assert exc_info.value.to_status == to_status + assert exc_info.value.from_status == final_status + assert exc_info.value.to_status == target_status diff --git a/tests/unit/test_execution_status_transitions.py b/tests/unit/test_execution_status_transitions.py index 74a8bb8a..c69c7a60 100644 --- a/tests/unit/test_execution_status_transitions.py +++ b/tests/unit/test_execution_status_transitions.py @@ -40,7 +40,6 @@ def test_invalid_transitions_from_pending(self) -> None: def test_valid_transitions_from_incomplete(self) -> None: """Test valid transitions from INCOMPLETE status.""" valid_targets = [ - ExecutionStatus.PENDING, ExecutionStatus.PASSED, ExecutionStatus.FAILED, ExecutionStatus.SKIPPED, @@ -49,6 +48,16 @@ def test_valid_transitions_from_incomplete(self) -> None: for target in valid_targets: _validate_status_transition(ExecutionStatus.INCOMPLETE, target) + def test_incomplete_cannot_go_back_to_pending(self) -> None: + """Test that INCOMPLETE cannot transition back to PENDING.""" + with pytest.raises(InvalidStatusTransitionError) as exc_info: + _validate_status_transition( + ExecutionStatus.INCOMPLETE, ExecutionStatus.PENDING + ) + + assert exc_info.value.from_status == ExecutionStatus.INCOMPLETE + assert exc_info.value.to_status == ExecutionStatus.PENDING + def test_no_transitions_from_final_states(self) -> None: """Test that final states cannot transition to any other status.""" final_states = [ From 230df82b3459d711ef53571a393c13629cc9b6ed Mon Sep 17 00:00:00 2001 From: danyalxahid-askui Date: Mon, 8 Sep 2025 09:34:09 +0200 Subject: [PATCH 05/19] refactor(tests): enhance docstrings in execution tests for clarity - Updated docstrings in integration and unit tests for execution status transitions to improve clarity and consistency. - Adjusted formatting for better readability in test cases. --- .../chat/api/test_executions_router.py | 8 ++++++-- .../chat/api/test_executions_service.py | 18 ++++++++++++------ .../unit/test_execution_status_transitions.py | 9 +++++++-- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/tests/integration/chat/api/test_executions_router.py b/tests/integration/chat/api/test_executions_router.py index 8977a343..2787f236 100644 --- a/tests/integration/chat/api/test_executions_router.py +++ b/tests/integration/chat/api/test_executions_router.py @@ -1,4 +1,6 @@ -"""Integration tests for execution router endpoints with status transition validation.""" +""" +Integration tests for execution router endpoints with status transition validation. +""" import uuid @@ -284,7 +286,9 @@ def test_final_states_cannot_transition_parametrized( final_status: ExecutionStatus, invalid_target: ExecutionStatus, ) -> None: - """Test that final states cannot transition to non-final states (parametrized).""" + """ + Test that final states cannot transition to non-final states (parametrized). + """ # Create an execution create_response = client.post( "/executions/", diff --git a/tests/integration/chat/api/test_executions_service.py b/tests/integration/chat/api/test_executions_service.py index aa0761bd..eaf4af10 100644 --- a/tests/integration/chat/api/test_executions_service.py +++ b/tests/integration/chat/api/test_executions_service.py @@ -216,7 +216,8 @@ def test_list_executions_with_filters( ) assert len(workflow_filtered.data) >= 1 assert all( - exec.workflow == create_params.workflow for exec in workflow_filtered.data + execution.workflow == create_params.workflow + for execution in workflow_filtered.data ) # Test thread filter @@ -226,7 +227,10 @@ def test_list_executions_with_filters( thread_id=create_params.thread, ) assert len(thread_filtered.data) >= 1 - assert all(exec.thread == create_params.thread for exec in thread_filtered.data) + assert all( + execution.thread == create_params.thread + for execution in thread_filtered.data + ) # Test combined filters combined_filtered = execution_service.list_( @@ -237,9 +241,9 @@ def test_list_executions_with_filters( ) assert len(combined_filtered.data) >= 1 assert all( - exec.workflow == create_params.workflow - and exec.thread == create_params.thread - for exec in combined_filtered.data + execution.workflow == create_params.workflow + and execution.thread == create_params.thread + for execution in combined_filtered.data ) def test_persistence_across_service_instances( @@ -426,7 +430,9 @@ def test_final_states_cannot_transition( final_status: ExecutionStatus, target_status: ExecutionStatus, ) -> None: - """Test that final states cannot transition to any other status (parametrized).""" + """ + Test that final states cannot transition to any other status (parametrized). + """ # Skip same-status transitions (they're allowed as no-ops) if final_status == target_status: pytest.skip("Same-status transitions are allowed as no-ops") diff --git a/tests/unit/test_execution_status_transitions.py b/tests/unit/test_execution_status_transitions.py index c69c7a60..4f13f918 100644 --- a/tests/unit/test_execution_status_transitions.py +++ b/tests/unit/test_execution_status_transitions.py @@ -31,7 +31,9 @@ def test_valid_transitions_from_pending(self) -> None: _validate_status_transition(ExecutionStatus.PENDING, target) def test_invalid_transitions_from_pending(self) -> None: - """Test that PENDING cannot transition back to PENDING (via validation logic).""" + """ + Test that PENDING cannot transition back to PENDING (via validation logic). + """ # Note: same-status transitions are allowed at the validation level, # but this tests the transition map logic # PENDING -> PENDING is actually allowed as a no-op @@ -139,7 +141,10 @@ def test_error_attributes(self) -> None: def test_specific_invalid_transitions_parametrized( self, final_state: ExecutionStatus, target_state: ExecutionStatus ) -> None: - """Test specific invalid transitions mentioned in requirements using parametrization.""" + """ + Test specific invalid transitions mentioned in requirements using + parametrization. + """ with pytest.raises(InvalidStatusTransitionError): _validate_status_transition(final_state, target_state) From 48650c7a587603751682d81536e9c54c9cdff5d5 Mon Sep 17 00:00:00 2001 From: danyalxahid-askui Date: Mon, 8 Sep 2025 09:54:23 +0200 Subject: [PATCH 06/19] fix(executions): improve error message in InvalidStatusTransitionError - Updated the error message in `InvalidStatusTransitionError` to include the string values of `from_status` and `to_status` instead of the enum instances, enhancing clarity for debugging invalid status transitions. --- src/askui/chat/api/executions/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/askui/chat/api/executions/models.py b/src/askui/chat/api/executions/models.py index 0e72fd38..891699f1 100644 --- a/src/askui/chat/api/executions/models.py +++ b/src/askui/chat/api/executions/models.py @@ -27,7 +27,7 @@ class InvalidStatusTransitionError(ValueError): def __init__( self, from_status: ExecutionStatus, to_status: ExecutionStatus ) -> None: - error_msg = f"Invalid status transition from '{from_status}' to '{to_status}'" + error_msg = f"Invalid status transition from '{from_status.value}' to '{to_status.value}'" super().__init__(error_msg) self.from_status = from_status self.to_status = to_status From 09466d2297699be734c9b50dbb7a6583488e2cb1 Mon Sep 17 00:00:00 2001 From: danyalxahid-askui Date: Mon, 8 Sep 2025 10:22:49 +0200 Subject: [PATCH 07/19] feat(executions): integrate WorkflowService into ExecutionService - Updated `get_execution_service` to include `workflow_service` as a dependency. - Modified `ExecutionService` to accept `workflow_service` in its constructor and store it as a private variable. - Enhanced the `create` method in `ExecutionService` to validate the existence of the workflow in the same workspace before creating an execution. - Added tests to ensure that creating an execution with a non-existent workflow or a workflow from a different workspace raises appropriate errors. --- src/askui/chat/api/executions/dependencies.py | 9 +- src/askui/chat/api/executions/service.py | 7 +- .../chat/api/test_executions_service.py | 96 +++++++++++++++---- 3 files changed, 92 insertions(+), 20 deletions(-) diff --git a/src/askui/chat/api/executions/dependencies.py b/src/askui/chat/api/executions/dependencies.py index 4adcf04b..aeaa6c21 100644 --- a/src/askui/chat/api/executions/dependencies.py +++ b/src/askui/chat/api/executions/dependencies.py @@ -3,11 +3,16 @@ from askui.chat.api.dependencies import SettingsDep from askui.chat.api.executions.service import ExecutionService from askui.chat.api.settings import Settings +from askui.chat.api.workflows.dependencies import WorkflowServiceDep +from askui.chat.api.workflows.service import WorkflowService -def get_execution_service(settings: Settings = SettingsDep) -> ExecutionService: +def get_execution_service( + settings: Settings = SettingsDep, + workflow_service: WorkflowService = WorkflowServiceDep, +) -> ExecutionService: """Get ExecutionService instance.""" - return ExecutionService(settings.data_dir) + return ExecutionService(settings.data_dir, workflow_service) ExecutionServiceDep = Depends(get_execution_service) diff --git a/src/askui/chat/api/executions/service.py b/src/askui/chat/api/executions/service.py index d828fd83..001f9dbd 100644 --- a/src/askui/chat/api/executions/service.py +++ b/src/askui/chat/api/executions/service.py @@ -10,6 +10,7 @@ from askui.chat.api.models import ThreadId, WorkspaceId from askui.chat.api.utils import build_workspace_filter_fn from askui.chat.api.workflows.models import WorkflowId +from askui.chat.api.workflows.service import WorkflowService from askui.utils.api_utils import ( ConflictError, ListQuery, @@ -44,9 +45,10 @@ def filter_fn(execution: Execution) -> bool: class ExecutionService: """Service for managing Execution resources with filesystem persistence.""" - def __init__(self, base_dir: Path) -> None: + def __init__(self, base_dir: Path, workflow_service: WorkflowService) -> None: self._base_dir = base_dir self._executions_dir = base_dir / "executions" + self._workflow_service = workflow_service def _get_execution_path(self, execution_id: ExecutionId, new: bool = False) -> Path: """Get the file path for an execution.""" @@ -103,6 +105,9 @@ def create( self, workspace_id: WorkspaceId | None, params: ExecutionCreateParams ) -> Execution: """Create a new execution.""" + # Validate that the workflow exists in the same workspace + self._workflow_service.retrieve(workspace_id, params.workflow) + execution = Execution.create(workspace_id, params) self._save(execution, new=True) return execution diff --git a/tests/integration/chat/api/test_executions_service.py b/tests/integration/chat/api/test_executions_service.py index eaf4af10..8e62546b 100644 --- a/tests/integration/chat/api/test_executions_service.py +++ b/tests/integration/chat/api/test_executions_service.py @@ -14,6 +14,8 @@ ) from askui.chat.api.executions.service import ExecutionService from askui.chat.api.models import WorkspaceId +from askui.chat.api.workflows.models import WorkflowCreateParams +from askui.chat.api.workflows.service import WorkflowService from askui.utils.api_utils import ListQuery, NotFoundError @@ -21,9 +23,16 @@ class TestExecutionService: """Test execution service with status transition validation.""" @pytest.fixture - def execution_service(self, tmp_path: Path) -> ExecutionService: + def workflow_service(self, tmp_path: Path) -> WorkflowService: + """Create a workflow service for testing.""" + return WorkflowService(tmp_path) + + @pytest.fixture + def execution_service( + self, tmp_path: Path, workflow_service: WorkflowService + ) -> ExecutionService: """Create an execution service for testing.""" - return ExecutionService(tmp_path) + return ExecutionService(tmp_path, workflow_service) @pytest.fixture def workspace_id(self) -> WorkspaceId: @@ -31,10 +40,24 @@ def workspace_id(self) -> WorkspaceId: return uuid.uuid4() @pytest.fixture - def create_params(self) -> ExecutionCreateParams: + def test_workflow_id( + self, workflow_service: WorkflowService, workspace_id: WorkspaceId + ) -> str: + """Create a test workflow and return its ID.""" + workflow_params = WorkflowCreateParams( + name="Test Workflow", + description="A test workflow for execution testing", + ) + workflow = workflow_service.create( + workspace_id=workspace_id, params=workflow_params + ) + return workflow.id + + @pytest.fixture + def create_params(self, test_workflow_id: str) -> ExecutionCreateParams: """Create sample execution creation parameters.""" return ExecutionCreateParams( - workflow="wf_test123", + workflow=test_workflow_id, thread="thread_test123", ) @@ -67,6 +90,45 @@ def test_create_execution_success( assert execution.id.startswith("exec_") assert execution.created_at is not None + def test_create_execution_with_nonexistent_workflow_fails( + self, + execution_service: ExecutionService, + workspace_id: WorkspaceId, + ) -> None: + """Test that creating execution with non-existent workflow raises NotFoundError.""" + invalid_params = ExecutionCreateParams( + workflow="wf_nonexistent123", + thread="thread_test123", + ) + + with pytest.raises(NotFoundError) as exc_info: + execution_service.create(workspace_id=workspace_id, params=invalid_params) + + assert "Workflow wf_nonexistent123 not found" in str(exc_info.value) + + def test_create_execution_with_workflow_from_different_workspace_fails( + self, + tmp_path: Path, + test_workflow_id: str, + ) -> None: + """Test that creating execution with workflow from different workspace fails.""" + # Create execution service with different workspace + different_workspace_id = uuid.uuid4() + workflow_service = WorkflowService(tmp_path) + execution_service = ExecutionService(tmp_path, workflow_service) + + invalid_params = ExecutionCreateParams( + workflow=test_workflow_id, # This workflow belongs to a different workspace + thread="thread_test123", + ) + + with pytest.raises(NotFoundError) as exc_info: + execution_service.create( + workspace_id=different_workspace_id, params=invalid_params + ) + + assert "not found" in str(exc_info.value) + def test_retrieve_execution_success( self, execution_service: ExecutionService, @@ -250,16 +312,16 @@ def test_persistence_across_service_instances( self, tmp_path: Path, workspace_id: WorkspaceId, + execution_service: ExecutionService, create_params: ExecutionCreateParams, ) -> None: """Test that executions persist across service instances.""" - # Create execution with first service instance - service1 = ExecutionService(tmp_path) - execution = service1.create(workspace_id=workspace_id, params=create_params) + # Create execution with existing service instance + execution = execution_service.create( + workspace_id=workspace_id, params=create_params + ) - # Retrieve with second service instance - service2 = ExecutionService(tmp_path) - retrieved = service2.retrieve( + retrieved = execution_service.retrieve( workspace_id=workspace_id, execution_id=execution.id ) @@ -269,26 +331,26 @@ def test_persistence_across_service_instances( def test_modify_execution_persists_changes( self, - tmp_path: Path, workspace_id: WorkspaceId, + execution_service: ExecutionService, create_params: ExecutionCreateParams, ) -> None: """Test that execution modifications are persisted to filesystem.""" - # Create execution - service1 = ExecutionService(tmp_path) - execution = service1.create(workspace_id=workspace_id, params=create_params) + # Create execution using existing service + execution = execution_service.create( + workspace_id=workspace_id, params=create_params + ) # Modify execution modify_params = ExecutionModifyParams(status=ExecutionStatus.PASSED) - service1.modify( + execution_service.modify( workspace_id=workspace_id, execution_id=execution.id, params=modify_params, ) # Verify changes persist with new service instance - service2 = ExecutionService(tmp_path) - retrieved = service2.retrieve( + retrieved = execution_service.retrieve( workspace_id=workspace_id, execution_id=execution.id ) assert retrieved.status == ExecutionStatus.PASSED From 9fc035b7dac195bde95c07569cd8cc97adedc212 Mon Sep 17 00:00:00 2001 From: danyalxahid-askui Date: Mon, 8 Sep 2025 10:59:49 +0200 Subject: [PATCH 08/19] feat(executions): add exception handler for InvalidStatusTransitionError - Implemented a custom exception handler for `InvalidStatusTransitionError` to return a 422 Unprocessable Entity response with a detailed error message. - Updated integration tests to utilize the new `workflow_service` fixture for better dependency management and to ensure accurate execution data during tests. - Refactored execution routes to include versioning in the endpoint paths for consistency and clarity. --- src/askui/chat/api/app.py | 12 +++ .../chat/api/test_executions_router.py | 99 ++++++++++++++----- .../chat/api/test_executions_service.py | 24 ++++- 3 files changed, 107 insertions(+), 28 deletions(-) diff --git a/src/askui/chat/api/app.py b/src/askui/chat/api/app.py index 8c89d831..fc7eb658 100644 --- a/src/askui/chat/api/app.py +++ b/src/askui/chat/api/app.py @@ -9,6 +9,7 @@ from askui.chat.api.assistants.dependencies import get_assistant_service from askui.chat.api.assistants.router import router as assistants_router from askui.chat.api.dependencies import SetEnvFromHeadersDep, get_settings +from askui.chat.api.executions.models import InvalidStatusTransitionError from askui.chat.api.executions.router import router as executions_router from askui.chat.api.files.router import router as files_router from askui.chat.api.health.router import router as health_router @@ -132,6 +133,17 @@ def forbidden_error_handler( ) +@app.exception_handler(InvalidStatusTransitionError) +def invalid_status_transition_error_handler( + request: Request, # noqa: ARG001 + exc: InvalidStatusTransitionError, +) -> JSONResponse: + return JSONResponse( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + content={"detail": str(exc)}, + ) + + @app.exception_handler(Exception) def catch_all_exception_handler( request: Request, # noqa: ARG001 diff --git a/tests/integration/chat/api/test_executions_router.py b/tests/integration/chat/api/test_executions_router.py index 2787f236..703548ec 100644 --- a/tests/integration/chat/api/test_executions_router.py +++ b/tests/integration/chat/api/test_executions_router.py @@ -2,23 +2,62 @@ Integration tests for execution router endpoints with status transition validation. """ +import tempfile import uuid +from pathlib import Path import pytest from fastapi.testclient import TestClient from askui.chat.api.app import app from askui.chat.api.executions.models import ExecutionStatus +from askui.chat.api.executions.service import ExecutionService from askui.chat.api.models import WorkspaceId +from askui.chat.api.workflows.models import WorkflowCreateParams +from askui.chat.api.workflows.service import WorkflowService class TestExecutionRouter: """Test execution router endpoints with status transition validation.""" @pytest.fixture - def client(self) -> TestClient: - """Create a test client for the FastAPI app.""" - return TestClient(app) + def temp_workspace_dir(self) -> Path: + """Create a temporary workspace directory for testing.""" + temp_dir = tempfile.mkdtemp() + return Path(temp_dir) + + @pytest.fixture + def workflow_service(self, temp_workspace_dir: Path) -> WorkflowService: + """Create a workflow service for testing.""" + return WorkflowService(temp_workspace_dir) + + @pytest.fixture + def execution_service( + self, temp_workspace_dir: Path, workflow_service: WorkflowService + ) -> ExecutionService: + """Create an execution service for testing.""" + return ExecutionService(temp_workspace_dir, workflow_service) + + @pytest.fixture + def client( + self, + temp_workspace_dir: Path, + workflow_service: WorkflowService, + execution_service: ExecutionService, + ) -> TestClient: + """Create a test client for the FastAPI app with overridden dependencies.""" + from askui.chat.api.executions.dependencies import get_execution_service + from askui.chat.api.workflows.dependencies import get_workflow_service + + # Override service dependencies directly + app.dependency_overrides[get_workflow_service] = lambda: workflow_service + app.dependency_overrides[get_execution_service] = lambda: execution_service + + client = TestClient(app) + + # Clean up overrides after test (this will be called when fixture is torn down) + yield client + app.dependency_overrides.clear() @pytest.fixture def workspace_id(self) -> WorkspaceId: @@ -26,10 +65,24 @@ def workspace_id(self) -> WorkspaceId: return uuid.uuid4() @pytest.fixture - def execution_data(self) -> dict[str, str]: + def test_workflow_id( + self, workflow_service: WorkflowService, workspace_id: WorkspaceId + ) -> str: + """Create a test workflow and return its ID.""" + workflow_params = WorkflowCreateParams( + name="Test Workflow", + description="A test workflow for execution testing", + ) + workflow = workflow_service.create( + workspace_id=workspace_id, params=workflow_params + ) + return workflow.id + + @pytest.fixture + def execution_data(self, test_workflow_id: str) -> dict[str, str]: """Create sample execution data.""" return { - "workflow": "wf_test123", + "workflow": test_workflow_id, "thread": "thread_test123", } @@ -41,7 +94,7 @@ def test_create_execution_success( ) -> None: """Test successful execution creation.""" response = client.post( - "/executions/", + "/v1/executions/", json=execution_data, headers={"askui-workspace": str(workspace_id)}, ) @@ -64,7 +117,7 @@ def test_modify_execution_valid_transition( """Test successful execution modification with valid status transition.""" # First create an execution create_response = client.post( - "/executions/", + "/v1/executions/", json=execution_data, headers={"askui-workspace": str(workspace_id)}, ) @@ -75,7 +128,7 @@ def test_modify_execution_valid_transition( # Then modify it with a valid transition (PENDING -> PASSED) modify_data = {"status": ExecutionStatus.PASSED.value} modify_response = client.patch( - f"/executions/{execution_id}", + f"/v1/executions/{execution_id}", json=modify_data, headers={"askui-workspace": str(workspace_id)}, ) @@ -94,7 +147,7 @@ def test_modify_execution_invalid_transition( """Test execution modification with invalid status transition returns 422.""" # First create an execution and transition it to a final state create_response = client.post( - "/executions/", + "/v1/executions/", json=execution_data, headers={"askui-workspace": str(workspace_id)}, ) @@ -105,7 +158,7 @@ def test_modify_execution_invalid_transition( # Transition to final state (PENDING -> PASSED) modify_data = {"status": ExecutionStatus.PASSED.value} modify_response = client.patch( - f"/executions/{execution_id}", + f"/v1/executions/{execution_id}", json=modify_data, headers={"askui-workspace": str(workspace_id)}, ) @@ -114,7 +167,7 @@ def test_modify_execution_invalid_transition( # Try to transition from final state to non-final state (PASSED -> PENDING) invalid_modify_data = {"status": ExecutionStatus.PENDING.value} invalid_response = client.patch( - f"/executions/{execution_id}", + f"/v1/executions/{execution_id}", json=invalid_modify_data, headers={"askui-workspace": str(workspace_id)}, ) @@ -138,7 +191,7 @@ def test_modify_execution_same_status_allowed( """Test that modifying to the same status is allowed (no-op).""" # Create an execution create_response = client.post( - "/executions/", + "/v1/executions/", json=execution_data, headers={"askui-workspace": str(workspace_id)}, ) @@ -149,7 +202,7 @@ def test_modify_execution_same_status_allowed( # Modify to the same status (PENDING -> PENDING) modify_data = {"status": ExecutionStatus.PENDING.value} modify_response = client.patch( - f"/executions/{execution_id}", + f"/v1/executions/{execution_id}", json=modify_data, headers={"askui-workspace": str(workspace_id)}, ) @@ -167,7 +220,7 @@ def test_modify_execution_multiple_valid_transitions( """Test multiple valid status transitions in sequence.""" # Create an execution create_response = client.post( - "/executions/", + "/v1/executions/", json=execution_data, headers={"askui-workspace": str(workspace_id)}, ) @@ -184,7 +237,7 @@ def test_modify_execution_multiple_valid_transitions( for status, expected_code in transitions: modify_data = {"status": status} modify_response = client.patch( - f"/executions/{execution_id}", + f"/v1/executions/{execution_id}", json=modify_data, headers={"askui-workspace": str(workspace_id)}, ) @@ -200,7 +253,7 @@ def test_modify_execution_not_found( """Test modifying non-existent execution returns 404.""" modify_data = {"status": ExecutionStatus.PASSED.value} response = client.patch( - "/executions/exec_nonexistent123", + "/v1/executions/exec_nonexistent123", json=modify_data, headers={"askui-workspace": str(workspace_id)}, ) @@ -216,7 +269,7 @@ def test_retrieve_execution_success( """Test successful execution retrieval.""" # Create an execution create_response = client.post( - "/executions/", + "/v1/executions/", json=execution_data, headers={"askui-workspace": str(workspace_id)}, ) @@ -226,7 +279,7 @@ def test_retrieve_execution_success( # Retrieve the execution retrieve_response = client.get( - f"/executions/{execution_id}", + f"/v1/executions/{execution_id}", headers={"askui-workspace": str(workspace_id)}, ) @@ -244,7 +297,7 @@ def test_list_executions_success( """Test successful execution listing.""" # Create an execution create_response = client.post( - "/executions/", + "/v1/executions/", json=execution_data, headers={"askui-workspace": str(workspace_id)}, ) @@ -252,7 +305,7 @@ def test_list_executions_success( # List executions list_response = client.get( - "/executions/", + "/v1/executions/", headers={"askui-workspace": str(workspace_id)}, ) @@ -291,7 +344,7 @@ def test_final_states_cannot_transition_parametrized( """ # Create an execution create_response = client.post( - "/executions/", + "/v1/executions/", json=execution_data, headers={"askui-workspace": str(workspace_id)}, ) @@ -302,7 +355,7 @@ def test_final_states_cannot_transition_parametrized( # Transition to final state modify_data = {"status": final_status.value} modify_response = client.patch( - f"/executions/{execution_id}", + f"/v1/executions/{execution_id}", json=modify_data, headers={"askui-workspace": str(workspace_id)}, ) @@ -311,7 +364,7 @@ def test_final_states_cannot_transition_parametrized( # Try to transition to invalid target invalid_modify_data = {"status": invalid_target.value} invalid_response = client.patch( - f"/executions/{execution_id}", + f"/v1/executions/{execution_id}", json=invalid_modify_data, headers={"askui-workspace": str(workspace_id)}, ) diff --git a/tests/integration/chat/api/test_executions_service.py b/tests/integration/chat/api/test_executions_service.py index 8e62546b..e609e3f0 100644 --- a/tests/integration/chat/api/test_executions_service.py +++ b/tests/integration/chat/api/test_executions_service.py @@ -255,6 +255,7 @@ def test_list_executions_success( def test_list_executions_with_filters( self, execution_service: ExecutionService, + workflow_service: WorkflowService, workspace_id: WorkspaceId, create_params: ExecutionCreateParams, ) -> None: @@ -262,8 +263,17 @@ def test_list_executions_with_filters( # Create multiple executions with different workflows and threads execution_service.create(workspace_id=workspace_id, params=create_params) + # Create a second workflow for the different execution + different_workflow_params = WorkflowCreateParams( + name="Different Test Workflow", + description="A different test workflow for execution filtering testing", + ) + different_workflow = workflow_service.create( + workspace_id=workspace_id, params=different_workflow_params + ) + different_params = ExecutionCreateParams( - workflow="wf_different456", + workflow=different_workflow.id, thread="thread_different456", ) execution_service.create(workspace_id=workspace_id, params=different_params) @@ -368,12 +378,13 @@ def test_valid_transitions_from_pending( self, execution_service: ExecutionService, workspace_id: WorkspaceId, + test_workflow_id: str, target_status: ExecutionStatus, ) -> None: """Test all valid transitions from PENDING status (parametrized).""" # Create execution (always starts as PENDING) create_params = ExecutionCreateParams( - workflow="wf_test123", + workflow=test_workflow_id, thread="thread_test123", ) execution = execution_service.create( @@ -402,12 +413,13 @@ def test_valid_transitions_from_incomplete( self, execution_service: ExecutionService, workspace_id: WorkspaceId, + test_workflow_id: str, target_status: ExecutionStatus, ) -> None: """Test all valid transitions from INCOMPLETE status (parametrized).""" # Create execution and move to INCOMPLETE create_params = ExecutionCreateParams( - workflow="wf_test123", + workflow=test_workflow_id, thread="thread_test123", ) execution = execution_service.create( @@ -436,11 +448,12 @@ def test_incomplete_cannot_go_back_to_pending( self, execution_service: ExecutionService, workspace_id: WorkspaceId, + test_workflow_id: str, ) -> None: """Test that INCOMPLETE cannot transition back to PENDING.""" # Create execution and move to INCOMPLETE create_params = ExecutionCreateParams( - workflow="wf_test123", + workflow=test_workflow_id, thread="thread_test123", ) execution = execution_service.create( @@ -489,6 +502,7 @@ def test_final_states_cannot_transition( self, execution_service: ExecutionService, workspace_id: WorkspaceId, + test_workflow_id: str, final_status: ExecutionStatus, target_status: ExecutionStatus, ) -> None: @@ -501,7 +515,7 @@ def test_final_states_cannot_transition( # Create execution and move to final state create_params = ExecutionCreateParams( - workflow="wf_test123", + workflow=test_workflow_id, thread="thread_test123", ) execution = execution_service.create( From 361f257ab79c532ca5428a14862e40fe37a144af Mon Sep 17 00:00:00 2001 From: danyalxahid-askui Date: Mon, 8 Sep 2025 11:01:27 +0200 Subject: [PATCH 09/19] test(mcp_configs): refactor test for listing MCP configs with dependency injection - Updated the `test_list_mcp_configs_empty` method to utilize FastAPI's dependency injection for `McpConfigService`. - Created a temporary workspace directory for the service during the test. - Ensured proper cleanup of dependency overrides after the test execution. - Enhanced assertions to verify the response structure when no MCP configs exist. --- .../integration/chat/api/test_mcp_configs.py | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/tests/integration/chat/api/test_mcp_configs.py b/tests/integration/chat/api/test_mcp_configs.py index f29c5a2c..6956fef4 100644 --- a/tests/integration/chat/api/test_mcp_configs.py +++ b/tests/integration/chat/api/test_mcp_configs.py @@ -13,17 +13,30 @@ class TestMcpConfigsAPI: """Test suite for the MCP configs API endpoints.""" - def test_list_mcp_configs_empty( - self, test_client: TestClient, test_headers: dict[str, str] - ) -> None: + def test_list_mcp_configs_empty(self, test_headers: dict[str, str]) -> None: """Test listing MCP configs when no configs exist.""" - response = test_client.get("/v1/mcp-configs", headers=test_headers) + temp_dir = tempfile.mkdtemp() + workspace_path = Path(temp_dir) - assert response.status_code == status.HTTP_200_OK - data = response.json() - assert data["object"] == "list" - assert data["data"] == [] - assert data["has_more"] is False + from askui.chat.api.app import app + from askui.chat.api.mcp_configs.dependencies import get_mcp_config_service + + def override_mcp_config_service() -> McpConfigService: + return McpConfigService(workspace_path) + + app.dependency_overrides[get_mcp_config_service] = override_mcp_config_service + + try: + with TestClient(app) as client: + response = client.get("/v1/mcp-configs", headers=test_headers) + + assert response.status_code == status.HTTP_200_OK + data = response.json() + assert data["object"] == "list" + assert data["data"] == [] + assert data["has_more"] is False + finally: + app.dependency_overrides.clear() def test_list_mcp_configs_with_configs(self, test_headers: dict[str, str]) -> None: """Test listing MCP configs when configs exist.""" From 237e2c09625864fc5c4a435deb75c0a6eafdbbe6 Mon Sep 17 00:00:00 2001 From: danyalxahid-askui Date: Thu, 11 Sep 2025 10:01:18 +0200 Subject: [PATCH 10/19] feat(executions): enhance execution model and service with workflow and thread integration - Updated `ExecutionService` to accept `thread_facade` as a dependency and modified the `create` method to handle thread and run creation. - Refactored the `create_execution` route to support asynchronous execution creation with background tasks. - Adjusted integration tests to reflect changes in parameter names and ensure proper validation of workflow and thread associations. --- src/askui/chat/api/executions/dependencies.py | 5 +- src/askui/chat/api/executions/models.py | 26 ++++++--- src/askui/chat/api/executions/router.py | 19 +++++-- src/askui/chat/api/executions/service.py | 54 ++++++++++++++----- src/askui/chat/api/models.py | 2 + src/askui/chat/api/workflows/models.py | 15 ++++-- src/askui/chat/api/workflows/service.py | 12 ++--- .../chat/api/test_executions_service.py | 40 +++++++------- 8 files changed, 117 insertions(+), 56 deletions(-) diff --git a/src/askui/chat/api/executions/dependencies.py b/src/askui/chat/api/executions/dependencies.py index aeaa6c21..c3b8e4ca 100644 --- a/src/askui/chat/api/executions/dependencies.py +++ b/src/askui/chat/api/executions/dependencies.py @@ -3,6 +3,8 @@ from askui.chat.api.dependencies import SettingsDep from askui.chat.api.executions.service import ExecutionService from askui.chat.api.settings import Settings +from askui.chat.api.threads.dependencies import ThreadFacadeDep +from askui.chat.api.threads.facade import ThreadFacade from askui.chat.api.workflows.dependencies import WorkflowServiceDep from askui.chat.api.workflows.service import WorkflowService @@ -10,9 +12,10 @@ def get_execution_service( settings: Settings = SettingsDep, workflow_service: WorkflowService = WorkflowServiceDep, + thread_facade: ThreadFacade = ThreadFacadeDep, ) -> ExecutionService: """Get ExecutionService instance.""" - return ExecutionService(settings.data_dir, workflow_service) + return ExecutionService(settings.data_dir, workflow_service, thread_facade) ExecutionServiceDep = Depends(get_execution_service) diff --git a/src/askui/chat/api/executions/models.py b/src/askui/chat/api/executions/models.py index 891699f1..99ab97b4 100644 --- a/src/askui/chat/api/executions/models.py +++ b/src/askui/chat/api/executions/models.py @@ -4,7 +4,13 @@ from pydantic import BaseModel, Field -from askui.chat.api.models import ExecutionId, ThreadId, WorkspaceId, WorkspaceResource +from askui.chat.api.models import ( + ExecutionId, + RunId, + ThreadId, + WorkspaceId, + WorkspaceResource, +) from askui.chat.api.workflows.models import WorkflowId from askui.utils.datetime_utils import now from askui.utils.id_utils import generate_time_ordered_id @@ -82,8 +88,7 @@ class ExecutionCreateParams(BaseModel): Parameters for creating an execution via API. """ - workflow: WorkflowId - thread: ThreadId + workflow_id: WorkflowId class ExecutionModifyParams(BaseModelWithNotGiven): @@ -113,22 +118,29 @@ class Execution(WorkspaceResource): id: ExecutionId object: Literal["execution"] = "execution" created_at: datetime.datetime - workflow: WorkflowId - thread: ThreadId + workflow_id: WorkflowId + thread_id: ThreadId + run_id: RunId status: ExecutionStatus = Field( ..., description="The current status of the workflow execution." ) @classmethod def create( - cls, workspace_id: WorkspaceId | None, params: ExecutionCreateParams + cls, + workspace_id: WorkspaceId, + workflow_id: WorkflowId, + run_id: RunId, + thread_id: ThreadId, ) -> "Execution": return cls( id=generate_time_ordered_id("exec"), created_at=now(), workspace_id=workspace_id, status=ExecutionStatus.PENDING, - **params.model_dump(), + run_id=run_id, + thread_id=thread_id, + workflow_id=workflow_id, ) def modify(self, params: ExecutionModifyParams) -> "Execution": diff --git a/src/askui/chat/api/executions/router.py b/src/askui/chat/api/executions/router.py index 60e1ae7f..870a0a92 100644 --- a/src/askui/chat/api/executions/router.py +++ b/src/askui/chat/api/executions/router.py @@ -1,6 +1,9 @@ +from collections.abc import AsyncGenerator from typing import Annotated -from fastapi import APIRouter, Header, Query +from fastapi import APIRouter, BackgroundTasks, Header, Query, Response, status +from fastapi.responses import JSONResponse, StreamingResponse +from pydantic import BaseModel from askui.chat.api.dependencies import ListQueryDep from askui.chat.api.executions.dependencies import ExecutionServiceDep @@ -36,13 +39,23 @@ def list_executions( @router.post("/") -def create_execution( +async def create_execution( askui_workspace: Annotated[WorkspaceId, Header()], params: ExecutionCreateParams, + background_tasks: BackgroundTasks, execution_service: ExecutionService = ExecutionServiceDep, ) -> Execution: """Create a new workflow execution.""" - return execution_service.create(workspace_id=askui_workspace, params=params) + execution, async_generator = await execution_service.create( + workspace_id=askui_workspace, params=params + ) + + async def _run_async_generator() -> None: + async for _ in async_generator: + pass + + background_tasks.add_task(_run_async_generator) + return execution @router.get("/{execution_id}") diff --git a/src/askui/chat/api/executions/service.py b/src/askui/chat/api/executions/service.py index 001f9dbd..308ac1e9 100644 --- a/src/askui/chat/api/executions/service.py +++ b/src/askui/chat/api/executions/service.py @@ -1,3 +1,4 @@ +from collections.abc import AsyncGenerator from pathlib import Path from typing import Callable @@ -7,7 +8,12 @@ ExecutionId, ExecutionModifyParams, ) +from askui.chat.api.messages.models import MessageCreateParams from askui.chat.api.models import ThreadId, WorkspaceId +from askui.chat.api.runs.models import ThreadAndRunCreateParams +from askui.chat.api.runs.runner.events.events import Events +from askui.chat.api.threads.facade import ThreadFacade +from askui.chat.api.threads.models import ThreadCreateParams from askui.chat.api.utils import build_workspace_filter_fn from askui.chat.api.workflows.models import WorkflowId from askui.chat.api.workflows.service import WorkflowService @@ -21,7 +27,7 @@ def _build_execution_filter_fn( - workspace_id: WorkspaceId | None, + workspace_id: WorkspaceId, workflow_id: WorkflowId | None = None, thread_id: ThreadId | None = None, ) -> Callable[[Execution], bool]: @@ -33,9 +39,9 @@ def _build_execution_filter_fn( def filter_fn(execution: Execution) -> bool: if not workspace_filter(execution): return False - if workflow_id is not None and execution.workflow != workflow_id: + if workflow_id is not None and execution.workflow_id != workflow_id: return False - if thread_id is not None and execution.thread != thread_id: + if thread_id is not None and execution.thread_id != thread_id: return False return True @@ -45,10 +51,16 @@ def filter_fn(execution: Execution) -> bool: class ExecutionService: """Service for managing Execution resources with filesystem persistence.""" - def __init__(self, base_dir: Path, workflow_service: WorkflowService) -> None: + def __init__( + self, + base_dir: Path, + workflow_service: WorkflowService, + thread_facade: ThreadFacade, + ) -> None: self._base_dir = base_dir self._executions_dir = base_dir / "executions" self._workflow_service = workflow_service + self._thread_facade = thread_facade def _get_execution_path(self, execution_id: ExecutionId, new: bool = False) -> Path: """Get the file path for an execution.""" @@ -64,7 +76,7 @@ def _get_execution_path(self, execution_id: ExecutionId, new: bool = False) -> P def list_( self, - workspace_id: WorkspaceId | None, + workspace_id: WorkspaceId, query: ListQuery, workflow_id: WorkflowId | None = None, thread_id: ThreadId | None = None, @@ -78,7 +90,7 @@ def list_( ) def retrieve( - self, workspace_id: WorkspaceId | None, execution_id: ExecutionId + self, workspace_id: WorkspaceId, execution_id: ExecutionId ) -> Execution: """Retrieve a specific execution by ID.""" try: @@ -101,20 +113,36 @@ def retrieve( else: return execution - def create( - self, workspace_id: WorkspaceId | None, params: ExecutionCreateParams - ) -> Execution: + async def create( + self, workspace_id: WorkspaceId, params: ExecutionCreateParams + ) -> tuple[Execution, AsyncGenerator[Events, None]]: """Create a new execution.""" # Validate that the workflow exists in the same workspace - self._workflow_service.retrieve(workspace_id, params.workflow) + workflow = self._workflow_service.retrieve(workspace_id, params.workflow_id) + + # Create a thread and a run + run, async_generator = await self._thread_facade.create_thread_and_run( + workspace_id, + ThreadAndRunCreateParams( + assistant_id=workflow.assistant_id, + thread=ThreadCreateParams( + name=workflow.name, + messages=[ + MessageCreateParams(role="user", content=workflow.description) + ], + ), + ), + ) - execution = Execution.create(workspace_id, params) + execution = Execution.create( + workspace_id, params.workflow_id, run.id, run.thread_id + ) self._save(execution, new=True) - return execution + return execution, async_generator def modify( self, - workspace_id: WorkspaceId | None, + workspace_id: WorkspaceId, execution_id: ExecutionId, params: ExecutionModifyParams, ) -> Execution: diff --git a/src/askui/chat/api/models.py b/src/askui/chat/api/models.py index b18bc221..2d6ae342 100644 --- a/src/askui/chat/api/models.py +++ b/src/askui/chat/api/models.py @@ -12,6 +12,8 @@ MessageId = Annotated[str, IdField("msg")] RunId = Annotated[str, IdField("run")] ThreadId = Annotated[str, IdField("thread")] +WorkflowId = Annotated[str, IdField("wf")] + WorkspaceId = UUID4 diff --git a/src/askui/chat/api/workflows/models.py b/src/askui/chat/api/workflows/models.py index daf984bb..4b00c146 100644 --- a/src/askui/chat/api/workflows/models.py +++ b/src/askui/chat/api/workflows/models.py @@ -2,12 +2,15 @@ from pydantic import BaseModel, Field -from askui.chat.api.models import WorkspaceId, WorkspaceResource +from askui.chat.api.models import ( + AssistantId, + WorkflowId, + WorkspaceId, + WorkspaceResource, +) from askui.utils.datetime_utils import UnixDatetime, now from askui.utils.id_utils import IdField, generate_time_ordered_id -WorkflowId = Annotated[str, IdField("wf")] - class WorkflowCreateParams(BaseModel): """ @@ -16,6 +19,7 @@ class WorkflowCreateParams(BaseModel): name: str description: str + assistant_id: AssistantId tags: list[str] = Field(default_factory=list) @@ -42,11 +46,12 @@ class Workflow(WorkspaceResource): description (str): A detailed description of the workflow's purpose and steps. tags (list[str], optional): Tags associated with the workflow for filtering or categorization. Default is an empty list. - workspace_id (WorkspaceId | None, optional): The workspace this workflow belongs to. + workspace_id (WorkspaceId, optional): The workspace this workflow belongs to. """ id: WorkflowId object: Literal["workflow"] = "workflow" + assistant_id: AssistantId created_at: UnixDatetime name: str description: str @@ -54,7 +59,7 @@ class Workflow(WorkspaceResource): @classmethod def create( - cls, workspace_id: WorkspaceId | None, params: WorkflowCreateParams + cls, workspace_id: WorkspaceId, params: WorkflowCreateParams ) -> "Workflow": return cls( id=generate_time_ordered_id("wf"), diff --git a/src/askui/chat/api/workflows/service.py b/src/askui/chat/api/workflows/service.py index 29aaa1aa..d95b1f69 100644 --- a/src/askui/chat/api/workflows/service.py +++ b/src/askui/chat/api/workflows/service.py @@ -19,7 +19,7 @@ def _build_workflow_filter_fn( - workspace_id: WorkspaceId | None, + workspace_id: WorkspaceId, tags: list[str] | None = None, ) -> Callable[[Workflow], bool]: workspace_filter: Callable[[Workflow], bool] = build_workspace_filter_fn( @@ -54,7 +54,7 @@ def _get_workflow_path(self, workflow_id: WorkflowId, new: bool = False) -> Path def list_( self, - workspace_id: WorkspaceId | None, + workspace_id: WorkspaceId, query: ListQuery, tags: list[str] | None = None, ) -> ListResponse[Workflow]: @@ -65,9 +65,7 @@ def list_( filter_fn=_build_workflow_filter_fn(workspace_id, tags=tags), ) - def retrieve( - self, workspace_id: WorkspaceId | None, workflow_id: WorkflowId - ) -> Workflow: + def retrieve(self, workspace_id: WorkspaceId, workflow_id: WorkflowId) -> Workflow: try: workflow_path = self._get_workflow_path(workflow_id) workflow = Workflow.model_validate_json(workflow_path.read_text()) @@ -84,7 +82,7 @@ def retrieve( return workflow def create( - self, workspace_id: WorkspaceId | None, params: WorkflowCreateParams + self, workspace_id: WorkspaceId, params: WorkflowCreateParams ) -> Workflow: workflow = Workflow.create(workspace_id, params) self._save(workflow, new=True) @@ -92,7 +90,7 @@ def create( def modify( self, - workspace_id: WorkspaceId | None, + workspace_id: WorkspaceId, workflow_id: WorkflowId, params: WorkflowModifyParams, ) -> Workflow: diff --git a/tests/integration/chat/api/test_executions_service.py b/tests/integration/chat/api/test_executions_service.py index e609e3f0..dca545c8 100644 --- a/tests/integration/chat/api/test_executions_service.py +++ b/tests/integration/chat/api/test_executions_service.py @@ -57,7 +57,7 @@ def test_workflow_id( def create_params(self, test_workflow_id: str) -> ExecutionCreateParams: """Create sample execution creation parameters.""" return ExecutionCreateParams( - workflow=test_workflow_id, + workflow_id=test_workflow_id, thread="thread_test123", ) @@ -82,7 +82,7 @@ def test_create_execution_success( workspace_id=workspace_id, params=create_params ) - assert execution.workflow == create_params.workflow + assert execution.workflow == create_params.workflow_id assert execution.thread == create_params.thread assert execution.status == ExecutionStatus.PENDING assert execution.workspace_id == workspace_id @@ -97,7 +97,7 @@ def test_create_execution_with_nonexistent_workflow_fails( ) -> None: """Test that creating execution with non-existent workflow raises NotFoundError.""" invalid_params = ExecutionCreateParams( - workflow="wf_nonexistent123", + workflow_id="wf_nonexistent123", thread="thread_test123", ) @@ -118,7 +118,7 @@ def test_create_execution_with_workflow_from_different_workspace_fails( execution_service = ExecutionService(tmp_path, workflow_service) invalid_params = ExecutionCreateParams( - workflow=test_workflow_id, # This workflow belongs to a different workspace + workflow_id=test_workflow_id, # This workflow belongs to a different workspace thread="thread_test123", ) @@ -141,8 +141,8 @@ def test_retrieve_execution_success( ) assert retrieved.id == sample_execution.id - assert retrieved.workflow == sample_execution.workflow - assert retrieved.thread == sample_execution.thread + assert retrieved.workflow_id == sample_execution.workflow_id + assert retrieved.thread_id == sample_execution.thread_id assert retrieved.status == sample_execution.status assert retrieved.workspace_id == sample_execution.workspace_id @@ -173,8 +173,8 @@ def test_modify_execution_valid_transition( assert modified.id == sample_execution.id assert modified.status == ExecutionStatus.PASSED - assert modified.workflow == sample_execution.workflow - assert modified.thread == sample_execution.thread + assert modified.workflow_id == sample_execution.workflow_id + assert modified.thread_id == sample_execution.thread_id def test_modify_execution_invalid_transition_raises_error( self, @@ -273,7 +273,7 @@ def test_list_executions_with_filters( ) different_params = ExecutionCreateParams( - workflow=different_workflow.id, + workflow_id=different_workflow.id, thread="thread_different456", ) execution_service.create(workspace_id=workspace_id, params=different_params) @@ -284,11 +284,11 @@ def test_list_executions_with_filters( workflow_filtered = execution_service.list_( workspace_id=workspace_id, query=query, - workflow_id=create_params.workflow, + workflow_id=create_params.workflow_id, ) assert len(workflow_filtered.data) >= 1 assert all( - execution.workflow == create_params.workflow + execution.workflow_id == create_params.workflow_id for execution in workflow_filtered.data ) @@ -300,7 +300,7 @@ def test_list_executions_with_filters( ) assert len(thread_filtered.data) >= 1 assert all( - execution.thread == create_params.thread + execution.thread_id == create_params.thread for execution in thread_filtered.data ) @@ -308,13 +308,13 @@ def test_list_executions_with_filters( combined_filtered = execution_service.list_( workspace_id=workspace_id, query=query, - workflow_id=create_params.workflow, + workflow_id=create_params.workflow_id, thread_id=create_params.thread, ) assert len(combined_filtered.data) >= 1 assert all( - execution.workflow == create_params.workflow - and execution.thread == create_params.thread + execution.workflow_id == create_params.workflow_id + and execution.thread_id == create_params.thread for execution in combined_filtered.data ) @@ -337,7 +337,7 @@ def test_persistence_across_service_instances( assert retrieved.id == execution.id assert retrieved.status == execution.status - assert retrieved.workflow == execution.workflow + assert retrieved.workflow_id == execution.workflow def test_modify_execution_persists_changes( self, @@ -384,7 +384,7 @@ def test_valid_transitions_from_pending( """Test all valid transitions from PENDING status (parametrized).""" # Create execution (always starts as PENDING) create_params = ExecutionCreateParams( - workflow=test_workflow_id, + workflow_id=test_workflow_id, thread="thread_test123", ) execution = execution_service.create( @@ -419,7 +419,7 @@ def test_valid_transitions_from_incomplete( """Test all valid transitions from INCOMPLETE status (parametrized).""" # Create execution and move to INCOMPLETE create_params = ExecutionCreateParams( - workflow=test_workflow_id, + workflow_id=test_workflow_id, thread="thread_test123", ) execution = execution_service.create( @@ -453,7 +453,7 @@ def test_incomplete_cannot_go_back_to_pending( """Test that INCOMPLETE cannot transition back to PENDING.""" # Create execution and move to INCOMPLETE create_params = ExecutionCreateParams( - workflow=test_workflow_id, + workflow_id=test_workflow_id, thread="thread_test123", ) execution = execution_service.create( @@ -515,7 +515,7 @@ def test_final_states_cannot_transition( # Create execution and move to final state create_params = ExecutionCreateParams( - workflow=test_workflow_id, + workflow_id=test_workflow_id, thread="thread_test123", ) execution = execution_service.create( From dccd7d1cb18c7e73c19a58fe16feeb9941c71668 Mon Sep 17 00:00:00 2001 From: danyalxahid-askui Date: Thu, 11 Sep 2025 10:33:29 +0200 Subject: [PATCH 11/19] test(executions): enhance integration tests with additional fixtures and async support - Added new fixtures for `AssistantService`, `MessageService`, `RunService`, `ThreadService`, and `ThreadFacade` to improve test setup. - Updated existing tests to utilize the new fixtures and support asynchronous execution. - Refactored parameter names in tests to align with recent changes in the execution model. - Ensured proper validation of workflow and thread associations in test cases. --- .../chat/api/test_executions_router.py | 101 ++++++++- .../chat/api/test_executions_service.py | 213 ++++++++++++++---- 2 files changed, 258 insertions(+), 56 deletions(-) diff --git a/tests/integration/chat/api/test_executions_router.py b/tests/integration/chat/api/test_executions_router.py index 703548ec..f32f1313 100644 --- a/tests/integration/chat/api/test_executions_router.py +++ b/tests/integration/chat/api/test_executions_router.py @@ -5,14 +5,21 @@ import tempfile import uuid from pathlib import Path +from unittest.mock import Mock import pytest from fastapi.testclient import TestClient from askui.chat.api.app import app +from askui.chat.api.assistants.models import AssistantCreateParams +from askui.chat.api.assistants.service import AssistantService from askui.chat.api.executions.models import ExecutionStatus from askui.chat.api.executions.service import ExecutionService +from askui.chat.api.messages.service import MessageService from askui.chat.api.models import WorkspaceId +from askui.chat.api.runs.service import RunService +from askui.chat.api.threads.facade import ThreadFacade +from askui.chat.api.threads.service import ThreadService from askui.chat.api.workflows.models import WorkflowCreateParams from askui.chat.api.workflows.service import WorkflowService @@ -31,12 +38,75 @@ def workflow_service(self, temp_workspace_dir: Path) -> WorkflowService: """Create a workflow service for testing.""" return WorkflowService(temp_workspace_dir) + @pytest.fixture + def assistant_service(self, temp_workspace_dir: Path) -> AssistantService: + """Create an assistant service for testing.""" + return AssistantService(temp_workspace_dir) + + @pytest.fixture + def message_service(self, temp_workspace_dir: Path) -> MessageService: + """Create a message service for testing.""" + return MessageService(temp_workspace_dir) + + @pytest.fixture + def mock_mcp_config_service(self) -> Mock: + """Create a mock MCP config service for testing.""" + mock_service = Mock() + mock_service.list_.return_value.data = [] + return mock_service + + @pytest.fixture + def mock_message_translator(self) -> Mock: + """Create a mock message translator for testing.""" + return Mock() + + @pytest.fixture + def run_service( + self, + temp_workspace_dir: Path, + assistant_service: AssistantService, + mock_mcp_config_service: Mock, + message_service: MessageService, + mock_message_translator: Mock, + ) -> RunService: + """Create a run service for testing.""" + return RunService( + temp_workspace_dir, + assistant_service, + mock_mcp_config_service, + message_service, + mock_message_translator, + ) + + @pytest.fixture + def thread_service( + self, + temp_workspace_dir: Path, + message_service: MessageService, + run_service: RunService, + ) -> ThreadService: + """Create a thread service for testing.""" + return ThreadService(temp_workspace_dir, message_service, run_service) + + @pytest.fixture + def thread_facade( + self, + thread_service: ThreadService, + message_service: MessageService, + run_service: RunService, + ) -> ThreadFacade: + """Create a thread facade for testing.""" + return ThreadFacade(thread_service, message_service, run_service) + @pytest.fixture def execution_service( - self, temp_workspace_dir: Path, workflow_service: WorkflowService + self, + temp_workspace_dir: Path, + workflow_service: WorkflowService, + thread_facade: ThreadFacade, ) -> ExecutionService: """Create an execution service for testing.""" - return ExecutionService(temp_workspace_dir, workflow_service) + return ExecutionService(temp_workspace_dir, workflow_service, thread_facade) @pytest.fixture def client( @@ -64,14 +134,32 @@ def workspace_id(self) -> WorkspaceId: """Create a test workspace ID.""" return uuid.uuid4() + @pytest.fixture + def test_assistant_id( + self, assistant_service: AssistantService, workspace_id: WorkspaceId + ) -> str: + """Create a test assistant and return its ID.""" + assistant_params = AssistantCreateParams( + name="Test Assistant", + description="A test assistant for execution testing", + ) + assistant = assistant_service.create( + workspace_id=workspace_id, params=assistant_params + ) + return assistant.id + @pytest.fixture def test_workflow_id( - self, workflow_service: WorkflowService, workspace_id: WorkspaceId + self, + workflow_service: WorkflowService, + workspace_id: WorkspaceId, + test_assistant_id: str, ) -> str: """Create a test workflow and return its ID.""" workflow_params = WorkflowCreateParams( name="Test Workflow", description="A test workflow for execution testing", + assistant_id=test_assistant_id, ) workflow = workflow_service.create( workspace_id=workspace_id, params=workflow_params @@ -82,8 +170,7 @@ def test_workflow_id( def execution_data(self, test_workflow_id: str) -> dict[str, str]: """Create sample execution data.""" return { - "workflow": test_workflow_id, - "thread": "thread_test123", + "workflow_id": test_workflow_id, } def test_create_execution_success( @@ -101,8 +188,8 @@ def test_create_execution_success( assert response.status_code == 200 data = response.json() - assert data["workflow"] == execution_data["workflow"] - assert data["thread"] == execution_data["thread"] + assert data["workflow_id"] == execution_data["workflow_id"] + assert data["thread_id"] is not None # Thread is created automatically assert data["status"] == ExecutionStatus.PENDING.value assert data["object"] == "execution" assert "id" in data diff --git a/tests/integration/chat/api/test_executions_service.py b/tests/integration/chat/api/test_executions_service.py index dca545c8..2d6404bb 100644 --- a/tests/integration/chat/api/test_executions_service.py +++ b/tests/integration/chat/api/test_executions_service.py @@ -2,9 +2,12 @@ import uuid from pathlib import Path +from unittest.mock import Mock import pytest +import pytest_asyncio +from askui.chat.api.assistants.service import AssistantService from askui.chat.api.executions.models import ( Execution, ExecutionCreateParams, @@ -13,7 +16,11 @@ InvalidStatusTransitionError, ) from askui.chat.api.executions.service import ExecutionService +from askui.chat.api.messages.service import MessageService from askui.chat.api.models import WorkspaceId +from askui.chat.api.runs.service import RunService +from askui.chat.api.threads.facade import ThreadFacade +from askui.chat.api.threads.service import ThreadService from askui.chat.api.workflows.models import WorkflowCreateParams from askui.chat.api.workflows.service import WorkflowService from askui.utils.api_utils import ListQuery, NotFoundError @@ -27,26 +34,109 @@ def workflow_service(self, tmp_path: Path) -> WorkflowService: """Create a workflow service for testing.""" return WorkflowService(tmp_path) + @pytest.fixture + def assistant_service(self, tmp_path: Path) -> AssistantService: + """Create an assistant service for testing.""" + return AssistantService(tmp_path) + + @pytest.fixture + def mock_mcp_config_service(self) -> Mock: + """Create a mock MCP config service for testing.""" + mock_service = Mock() + mock_service.list_.return_value.data = [] + return mock_service + + @pytest.fixture + def mock_message_translator(self) -> Mock: + """Create a mock message translator for testing.""" + return Mock() + + @pytest.fixture + def thread_service( + self, + tmp_path: Path, + message_service: MessageService, + run_service: RunService, + ) -> ThreadService: + """Create a thread service for testing.""" + return ThreadService(tmp_path, message_service, run_service) + + @pytest.fixture + def message_service(self, tmp_path: Path) -> MessageService: + """Create a message service for testing.""" + return MessageService(tmp_path) + + @pytest.fixture + def run_service( + self, + tmp_path: Path, + assistant_service: AssistantService, + mock_mcp_config_service: Mock, + message_service: MessageService, + mock_message_translator: Mock, + ) -> RunService: + """Create a run service for testing.""" + return RunService( + tmp_path, + assistant_service, + mock_mcp_config_service, + message_service, + mock_message_translator, + ) + + @pytest.fixture + def thread_facade( + self, + thread_service: ThreadService, + message_service: MessageService, + run_service: RunService, + ) -> ThreadFacade: + """Create a thread facade for testing.""" + return ThreadFacade(thread_service, message_service, run_service) + @pytest.fixture def execution_service( - self, tmp_path: Path, workflow_service: WorkflowService + self, + tmp_path: Path, + workflow_service: WorkflowService, + thread_facade: ThreadFacade, ) -> ExecutionService: """Create an execution service for testing.""" - return ExecutionService(tmp_path, workflow_service) + return ExecutionService(tmp_path, workflow_service, thread_facade) @pytest.fixture def workspace_id(self) -> WorkspaceId: """Create a test workspace ID.""" return uuid.uuid4() + @pytest.fixture + def test_assistant_id( + self, assistant_service: AssistantService, workspace_id: WorkspaceId + ) -> str: + """Create a test assistant and return its ID.""" + from askui.chat.api.assistants.models import AssistantCreateParams + + assistant_params = AssistantCreateParams( + name="Test Assistant", + description="A test assistant for execution testing", + ) + assistant = assistant_service.create( + workspace_id=workspace_id, params=assistant_params + ) + return assistant.id + @pytest.fixture def test_workflow_id( - self, workflow_service: WorkflowService, workspace_id: WorkspaceId + self, + workflow_service: WorkflowService, + workspace_id: WorkspaceId, + test_assistant_id: str, ) -> str: """Create a test workflow and return its ID.""" workflow_params = WorkflowCreateParams( name="Test Workflow", description="A test workflow for execution testing", + assistant_id=test_assistant_id, ) workflow = workflow_service.create( workspace_id=workspace_id, params=workflow_params @@ -58,39 +148,43 @@ def create_params(self, test_workflow_id: str) -> ExecutionCreateParams: """Create sample execution creation parameters.""" return ExecutionCreateParams( workflow_id=test_workflow_id, - thread="thread_test123", ) - @pytest.fixture - def sample_execution( + @pytest_asyncio.fixture + async def sample_execution( self, execution_service: ExecutionService, workspace_id: WorkspaceId, create_params: ExecutionCreateParams, ) -> Execution: """Create a sample execution for testing.""" - return execution_service.create(workspace_id=workspace_id, params=create_params) + execution, _ = await execution_service.create( + workspace_id=workspace_id, params=create_params + ) + return execution - def test_create_execution_success( + @pytest.mark.asyncio + async def test_create_execution_success( self, execution_service: ExecutionService, workspace_id: WorkspaceId, create_params: ExecutionCreateParams, ) -> None: """Test successful execution creation.""" - execution = execution_service.create( + execution, _ = await execution_service.create( workspace_id=workspace_id, params=create_params ) - assert execution.workflow == create_params.workflow_id - assert execution.thread == create_params.thread + assert execution.workflow_id == create_params.workflow_id + assert execution.thread_id is not None # Thread is created automatically assert execution.status == ExecutionStatus.PENDING assert execution.workspace_id == workspace_id assert execution.object == "execution" assert execution.id.startswith("exec_") assert execution.created_at is not None - def test_create_execution_with_nonexistent_workflow_fails( + @pytest.mark.asyncio + async def test_create_execution_with_nonexistent_workflow_fails( self, execution_service: ExecutionService, workspace_id: WorkspaceId, @@ -98,15 +192,17 @@ def test_create_execution_with_nonexistent_workflow_fails( """Test that creating execution with non-existent workflow raises NotFoundError.""" invalid_params = ExecutionCreateParams( workflow_id="wf_nonexistent123", - thread="thread_test123", ) with pytest.raises(NotFoundError) as exc_info: - execution_service.create(workspace_id=workspace_id, params=invalid_params) + await execution_service.create( + workspace_id=workspace_id, params=invalid_params + ) assert "Workflow wf_nonexistent123 not found" in str(exc_info.value) - def test_create_execution_with_workflow_from_different_workspace_fails( + @pytest.mark.asyncio + async def test_create_execution_with_workflow_from_different_workspace_fails( self, tmp_path: Path, test_workflow_id: str, @@ -115,21 +211,28 @@ def test_create_execution_with_workflow_from_different_workspace_fails( # Create execution service with different workspace different_workspace_id = uuid.uuid4() workflow_service = WorkflowService(tmp_path) - execution_service = ExecutionService(tmp_path, workflow_service) + # Create minimal thread facade for this test + mock_thread_service = Mock() + mock_message_service = Mock() + mock_run_service = Mock() + thread_facade = ThreadFacade( + mock_thread_service, mock_message_service, mock_run_service + ) + execution_service = ExecutionService(tmp_path, workflow_service, thread_facade) invalid_params = ExecutionCreateParams( workflow_id=test_workflow_id, # This workflow belongs to a different workspace - thread="thread_test123", ) with pytest.raises(NotFoundError) as exc_info: - execution_service.create( + await execution_service.create( workspace_id=different_workspace_id, params=invalid_params ) assert "not found" in str(exc_info.value) - def test_retrieve_execution_success( + @pytest.mark.asyncio + async def test_retrieve_execution_success( self, execution_service: ExecutionService, workspace_id: WorkspaceId, @@ -157,7 +260,8 @@ def test_retrieve_execution_not_found( assert "not found" in str(exc_info.value) - def test_modify_execution_valid_transition( + @pytest.mark.asyncio + async def test_modify_execution_valid_transition( self, execution_service: ExecutionService, workspace_id: WorkspaceId, @@ -176,7 +280,8 @@ def test_modify_execution_valid_transition( assert modified.workflow_id == sample_execution.workflow_id assert modified.thread_id == sample_execution.thread_id - def test_modify_execution_invalid_transition_raises_error( + @pytest.mark.asyncio + async def test_modify_execution_invalid_transition_raises_error( self, execution_service: ExecutionService, workspace_id: WorkspaceId, @@ -206,7 +311,8 @@ def test_modify_execution_invalid_transition_raises_error( exc_info.value ) - def test_modify_execution_same_status_allowed( + @pytest.mark.asyncio + async def test_modify_execution_same_status_allowed( self, execution_service: ExecutionService, workspace_id: WorkspaceId, @@ -238,7 +344,8 @@ def test_modify_execution_not_found( assert "not found" in str(exc_info.value) - def test_list_executions_success( + @pytest.mark.asyncio + async def test_list_executions_success( self, execution_service: ExecutionService, workspace_id: WorkspaceId, @@ -252,21 +359,26 @@ def test_list_executions_success( assert len(result.data) >= 1 assert any(execution.id == sample_execution.id for execution in result.data) - def test_list_executions_with_filters( + @pytest.mark.asyncio + async def test_list_executions_with_filters( self, execution_service: ExecutionService, workflow_service: WorkflowService, workspace_id: WorkspaceId, create_params: ExecutionCreateParams, + test_assistant_id: str, ) -> None: """Test execution listing with workflow and thread filters.""" # Create multiple executions with different workflows and threads - execution_service.create(workspace_id=workspace_id, params=create_params) + first_execution, _ = await execution_service.create( + workspace_id=workspace_id, params=create_params + ) # Create a second workflow for the different execution different_workflow_params = WorkflowCreateParams( name="Different Test Workflow", description="A different test workflow for execution filtering testing", + assistant_id=test_assistant_id, ) different_workflow = workflow_service.create( workspace_id=workspace_id, params=different_workflow_params @@ -274,9 +386,10 @@ def test_list_executions_with_filters( different_params = ExecutionCreateParams( workflow_id=different_workflow.id, - thread="thread_different456", ) - execution_service.create(workspace_id=workspace_id, params=different_params) + second_execution, _ = await execution_service.create( + workspace_id=workspace_id, params=different_params + ) query = ListQuery(limit=10, order="desc") @@ -292,15 +405,15 @@ def test_list_executions_with_filters( for execution in workflow_filtered.data ) - # Test thread filter + # Test thread filter - use the actual thread_id from the first execution thread_filtered = execution_service.list_( workspace_id=workspace_id, query=query, - thread_id=create_params.thread, + thread_id=first_execution.thread_id, ) assert len(thread_filtered.data) >= 1 assert all( - execution.thread_id == create_params.thread + execution.thread_id == first_execution.thread_id for execution in thread_filtered.data ) @@ -309,16 +422,17 @@ def test_list_executions_with_filters( workspace_id=workspace_id, query=query, workflow_id=create_params.workflow_id, - thread_id=create_params.thread, + thread_id=first_execution.thread_id, ) assert len(combined_filtered.data) >= 1 assert all( execution.workflow_id == create_params.workflow_id - and execution.thread_id == create_params.thread + and execution.thread_id == first_execution.thread_id for execution in combined_filtered.data ) - def test_persistence_across_service_instances( + @pytest.mark.asyncio + async def test_persistence_across_service_instances( self, tmp_path: Path, workspace_id: WorkspaceId, @@ -327,7 +441,7 @@ def test_persistence_across_service_instances( ) -> None: """Test that executions persist across service instances.""" # Create execution with existing service instance - execution = execution_service.create( + execution, _ = await execution_service.create( workspace_id=workspace_id, params=create_params ) @@ -337,9 +451,10 @@ def test_persistence_across_service_instances( assert retrieved.id == execution.id assert retrieved.status == execution.status - assert retrieved.workflow_id == execution.workflow + assert retrieved.workflow_id == execution.workflow_id - def test_modify_execution_persists_changes( + @pytest.mark.asyncio + async def test_modify_execution_persists_changes( self, workspace_id: WorkspaceId, execution_service: ExecutionService, @@ -347,7 +462,7 @@ def test_modify_execution_persists_changes( ) -> None: """Test that execution modifications are persisted to filesystem.""" # Create execution using existing service - execution = execution_service.create( + execution, _ = await execution_service.create( workspace_id=workspace_id, params=create_params ) @@ -374,7 +489,8 @@ def test_modify_execution_persists_changes( ExecutionStatus.SKIPPED, ], ) - def test_valid_transitions_from_pending( + @pytest.mark.asyncio + async def test_valid_transitions_from_pending( self, execution_service: ExecutionService, workspace_id: WorkspaceId, @@ -385,9 +501,8 @@ def test_valid_transitions_from_pending( # Create execution (always starts as PENDING) create_params = ExecutionCreateParams( workflow_id=test_workflow_id, - thread="thread_test123", ) - execution = execution_service.create( + execution, _ = await execution_service.create( workspace_id=workspace_id, params=create_params ) @@ -409,7 +524,8 @@ def test_valid_transitions_from_pending( ExecutionStatus.SKIPPED, ], ) - def test_valid_transitions_from_incomplete( + @pytest.mark.asyncio + async def test_valid_transitions_from_incomplete( self, execution_service: ExecutionService, workspace_id: WorkspaceId, @@ -420,9 +536,8 @@ def test_valid_transitions_from_incomplete( # Create execution and move to INCOMPLETE create_params = ExecutionCreateParams( workflow_id=test_workflow_id, - thread="thread_test123", ) - execution = execution_service.create( + execution, _ = await execution_service.create( workspace_id=workspace_id, params=create_params ) @@ -444,7 +559,8 @@ def test_valid_transitions_from_incomplete( assert modified.status == target_status - def test_incomplete_cannot_go_back_to_pending( + @pytest.mark.asyncio + async def test_incomplete_cannot_go_back_to_pending( self, execution_service: ExecutionService, workspace_id: WorkspaceId, @@ -454,9 +570,8 @@ def test_incomplete_cannot_go_back_to_pending( # Create execution and move to INCOMPLETE create_params = ExecutionCreateParams( workflow_id=test_workflow_id, - thread="thread_test123", ) - execution = execution_service.create( + execution, _ = await execution_service.create( workspace_id=workspace_id, params=create_params ) @@ -498,7 +613,8 @@ def test_incomplete_cannot_go_back_to_pending( ExecutionStatus.SKIPPED, ], ) - def test_final_states_cannot_transition( + @pytest.mark.asyncio + async def test_final_states_cannot_transition( self, execution_service: ExecutionService, workspace_id: WorkspaceId, @@ -516,9 +632,8 @@ def test_final_states_cannot_transition( # Create execution and move to final state create_params = ExecutionCreateParams( workflow_id=test_workflow_id, - thread="thread_test123", ) - execution = execution_service.create( + execution, _ = await execution_service.create( workspace_id=workspace_id, params=create_params ) From 345cbb2c6fce6383a07310f171e16c34b59fc036 Mon Sep 17 00:00:00 2001 From: danyalxahid-askui Date: Thu, 11 Sep 2025 10:55:41 +0200 Subject: [PATCH 12/19] refactor(tests): remove unused parameters from test fixtures - Removed `temp_workspace_dir` and `tmp_path` parameters from the `client` and `test_persistence_across_service_instances` methods in the integration tests, as they were not utilized. - Updated docstrings for clarity and consistency. --- tests/integration/chat/api/test_executions_router.py | 1 - tests/integration/chat/api/test_executions_service.py | 7 ++++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/integration/chat/api/test_executions_router.py b/tests/integration/chat/api/test_executions_router.py index f32f1313..094ceac7 100644 --- a/tests/integration/chat/api/test_executions_router.py +++ b/tests/integration/chat/api/test_executions_router.py @@ -111,7 +111,6 @@ def execution_service( @pytest.fixture def client( self, - temp_workspace_dir: Path, workflow_service: WorkflowService, execution_service: ExecutionService, ) -> TestClient: diff --git a/tests/integration/chat/api/test_executions_service.py b/tests/integration/chat/api/test_executions_service.py index 2d6404bb..726c409e 100644 --- a/tests/integration/chat/api/test_executions_service.py +++ b/tests/integration/chat/api/test_executions_service.py @@ -189,7 +189,9 @@ async def test_create_execution_with_nonexistent_workflow_fails( execution_service: ExecutionService, workspace_id: WorkspaceId, ) -> None: - """Test that creating execution with non-existent workflow raises NotFoundError.""" + """ + Test that creating execution with non-existent workflow raises NotFoundError. + """ invalid_params = ExecutionCreateParams( workflow_id="wf_nonexistent123", ) @@ -221,7 +223,7 @@ async def test_create_execution_with_workflow_from_different_workspace_fails( execution_service = ExecutionService(tmp_path, workflow_service, thread_facade) invalid_params = ExecutionCreateParams( - workflow_id=test_workflow_id, # This workflow belongs to a different workspace + workflow_id=test_workflow_id, ) with pytest.raises(NotFoundError) as exc_info: @@ -434,7 +436,6 @@ async def test_list_executions_with_filters( @pytest.mark.asyncio async def test_persistence_across_service_instances( self, - tmp_path: Path, workspace_id: WorkspaceId, execution_service: ExecutionService, create_params: ExecutionCreateParams, From ebbe43bb85c8eb337222085ec094b7454f954573 Mon Sep 17 00:00:00 2001 From: danyalxahid-askui Date: Thu, 11 Sep 2025 23:04:40 +0200 Subject: [PATCH 13/19] feat(workflow-executions): introduce execution models and service for workflow management - moved resource to workflow_execution for less contradicting with execution tools --- src/askui/chat/api/app.py | 4 ++-- .../__init__.py | 0 .../dependencies.py | 2 +- .../models.py | 0 .../router.py | 18 +++++++++--------- .../service.py | 12 ++++++------ .../chat/api/test_executions_router.py | 8 +++++--- .../chat/api/test_executions_service.py | 14 +++++++------- .../unit/test_execution_status_transitions.py | 4 ++-- 9 files changed, 32 insertions(+), 30 deletions(-) rename src/askui/chat/api/{executions => workflow_executions}/__init__.py (100%) rename src/askui/chat/api/{executions => workflow_executions}/dependencies.py (91%) rename src/askui/chat/api/{executions => workflow_executions}/models.py (100%) rename src/askui/chat/api/{executions => workflow_executions}/router.py (85%) rename src/askui/chat/api/{executions => workflow_executions}/service.py (99%) diff --git a/src/askui/chat/api/app.py b/src/askui/chat/api/app.py index 7d4559be..cde4b5c5 100644 --- a/src/askui/chat/api/app.py +++ b/src/askui/chat/api/app.py @@ -9,8 +9,6 @@ from askui.chat.api.assistants.dependencies import get_assistant_service from askui.chat.api.assistants.router import router as assistants_router from askui.chat.api.dependencies import SetEnvFromHeadersDep, get_settings -from askui.chat.api.executions.models import InvalidStatusTransitionError -from askui.chat.api.executions.router import router as executions_router from askui.chat.api.files.router import router as files_router from askui.chat.api.health.router import router as health_router from askui.chat.api.mcp_clients.dependencies import get_mcp_client_manager_manager @@ -23,6 +21,8 @@ from askui.chat.api.messages.router import router as messages_router from askui.chat.api.runs.router import router as runs_router from askui.chat.api.threads.router import router as threads_router +from askui.chat.api.workflow_executions.models import InvalidStatusTransitionError +from askui.chat.api.workflow_executions.router import router as executions_router from askui.chat.api.workflows.router import router as workflows_router from askui.utils.api_utils import ( ConflictError, diff --git a/src/askui/chat/api/executions/__init__.py b/src/askui/chat/api/workflow_executions/__init__.py similarity index 100% rename from src/askui/chat/api/executions/__init__.py rename to src/askui/chat/api/workflow_executions/__init__.py diff --git a/src/askui/chat/api/executions/dependencies.py b/src/askui/chat/api/workflow_executions/dependencies.py similarity index 91% rename from src/askui/chat/api/executions/dependencies.py rename to src/askui/chat/api/workflow_executions/dependencies.py index c3b8e4ca..68e5abc2 100644 --- a/src/askui/chat/api/executions/dependencies.py +++ b/src/askui/chat/api/workflow_executions/dependencies.py @@ -1,10 +1,10 @@ from fastapi import Depends from askui.chat.api.dependencies import SettingsDep -from askui.chat.api.executions.service import ExecutionService from askui.chat.api.settings import Settings from askui.chat.api.threads.dependencies import ThreadFacadeDep from askui.chat.api.threads.facade import ThreadFacade +from askui.chat.api.workflow_executions.service import ExecutionService from askui.chat.api.workflows.dependencies import WorkflowServiceDep from askui.chat.api.workflows.service import WorkflowService diff --git a/src/askui/chat/api/executions/models.py b/src/askui/chat/api/workflow_executions/models.py similarity index 100% rename from src/askui/chat/api/executions/models.py rename to src/askui/chat/api/workflow_executions/models.py diff --git a/src/askui/chat/api/executions/router.py b/src/askui/chat/api/workflow_executions/router.py similarity index 85% rename from src/askui/chat/api/executions/router.py rename to src/askui/chat/api/workflow_executions/router.py index 870a0a92..143ed7a9 100644 --- a/src/askui/chat/api/executions/router.py +++ b/src/askui/chat/api/workflow_executions/router.py @@ -6,23 +6,23 @@ from pydantic import BaseModel from askui.chat.api.dependencies import ListQueryDep -from askui.chat.api.executions.dependencies import ExecutionServiceDep -from askui.chat.api.executions.models import ( +from askui.chat.api.models import ThreadId, WorkspaceId +from askui.chat.api.workflow_executions.dependencies import ExecutionServiceDep +from askui.chat.api.workflow_executions.models import ( Execution, ExecutionCreateParams, ExecutionId, ExecutionModifyParams, ) -from askui.chat.api.executions.service import ExecutionService -from askui.chat.api.models import ThreadId, WorkspaceId +from askui.chat.api.workflow_executions.service import ExecutionService from askui.chat.api.workflows.models import WorkflowId from askui.utils.api_utils import ListQuery, ListResponse -router = APIRouter(prefix="/executions", tags=["executions"]) +router = APIRouter(prefix="/workflow-executions", tags=["workflow-executions"]) @router.get("/") -def list_executions( +def list_workflow_executions( askui_workspace: Annotated[WorkspaceId, Header()], query: ListQuery = ListQueryDep, workflow_id: Annotated[WorkflowId | None, Query()] = None, @@ -39,7 +39,7 @@ def list_executions( @router.post("/") -async def create_execution( +async def create_workflow_execution( askui_workspace: Annotated[WorkspaceId, Header()], params: ExecutionCreateParams, background_tasks: BackgroundTasks, @@ -59,7 +59,7 @@ async def _run_async_generator() -> None: @router.get("/{execution_id}") -def retrieve_execution( +def retrieve_workflow_execution( askui_workspace: Annotated[WorkspaceId, Header()], execution_id: ExecutionId, execution_service: ExecutionService = ExecutionServiceDep, @@ -71,7 +71,7 @@ def retrieve_execution( @router.patch("/{execution_id}") -def modify_execution( +def modify_workflow_execution( askui_workspace: Annotated[WorkspaceId, Header()], execution_id: ExecutionId, params: ExecutionModifyParams, diff --git a/src/askui/chat/api/executions/service.py b/src/askui/chat/api/workflow_executions/service.py similarity index 99% rename from src/askui/chat/api/executions/service.py rename to src/askui/chat/api/workflow_executions/service.py index 308ac1e9..b6f46a72 100644 --- a/src/askui/chat/api/executions/service.py +++ b/src/askui/chat/api/workflow_executions/service.py @@ -2,12 +2,6 @@ from pathlib import Path from typing import Callable -from askui.chat.api.executions.models import ( - Execution, - ExecutionCreateParams, - ExecutionId, - ExecutionModifyParams, -) from askui.chat.api.messages.models import MessageCreateParams from askui.chat.api.models import ThreadId, WorkspaceId from askui.chat.api.runs.models import ThreadAndRunCreateParams @@ -15,6 +9,12 @@ from askui.chat.api.threads.facade import ThreadFacade from askui.chat.api.threads.models import ThreadCreateParams from askui.chat.api.utils import build_workspace_filter_fn +from askui.chat.api.workflow_executions.models import ( + Execution, + ExecutionCreateParams, + ExecutionId, + ExecutionModifyParams, +) from askui.chat.api.workflows.models import WorkflowId from askui.chat.api.workflows.service import WorkflowService from askui.utils.api_utils import ( diff --git a/tests/integration/chat/api/test_executions_router.py b/tests/integration/chat/api/test_executions_router.py index 094ceac7..a23ea98d 100644 --- a/tests/integration/chat/api/test_executions_router.py +++ b/tests/integration/chat/api/test_executions_router.py @@ -13,13 +13,13 @@ from askui.chat.api.app import app from askui.chat.api.assistants.models import AssistantCreateParams from askui.chat.api.assistants.service import AssistantService -from askui.chat.api.executions.models import ExecutionStatus -from askui.chat.api.executions.service import ExecutionService from askui.chat.api.messages.service import MessageService from askui.chat.api.models import WorkspaceId from askui.chat.api.runs.service import RunService from askui.chat.api.threads.facade import ThreadFacade from askui.chat.api.threads.service import ThreadService +from askui.chat.api.workflow_executions.models import ExecutionStatus +from askui.chat.api.workflow_executions.service import ExecutionService from askui.chat.api.workflows.models import WorkflowCreateParams from askui.chat.api.workflows.service import WorkflowService @@ -115,7 +115,9 @@ def client( execution_service: ExecutionService, ) -> TestClient: """Create a test client for the FastAPI app with overridden dependencies.""" - from askui.chat.api.executions.dependencies import get_execution_service + from askui.chat.api.workflow_executions.dependencies import ( + get_execution_service, + ) from askui.chat.api.workflows.dependencies import get_workflow_service # Override service dependencies directly diff --git a/tests/integration/chat/api/test_executions_service.py b/tests/integration/chat/api/test_executions_service.py index 726c409e..9f74f2ea 100644 --- a/tests/integration/chat/api/test_executions_service.py +++ b/tests/integration/chat/api/test_executions_service.py @@ -8,19 +8,19 @@ import pytest_asyncio from askui.chat.api.assistants.service import AssistantService -from askui.chat.api.executions.models import ( +from askui.chat.api.messages.service import MessageService +from askui.chat.api.models import WorkspaceId +from askui.chat.api.runs.service import RunService +from askui.chat.api.threads.facade import ThreadFacade +from askui.chat.api.threads.service import ThreadService +from askui.chat.api.workflow_executions.models import ( Execution, ExecutionCreateParams, ExecutionModifyParams, ExecutionStatus, InvalidStatusTransitionError, ) -from askui.chat.api.executions.service import ExecutionService -from askui.chat.api.messages.service import MessageService -from askui.chat.api.models import WorkspaceId -from askui.chat.api.runs.service import RunService -from askui.chat.api.threads.facade import ThreadFacade -from askui.chat.api.threads.service import ThreadService +from askui.chat.api.workflow_executions.service import ExecutionService from askui.chat.api.workflows.models import WorkflowCreateParams from askui.chat.api.workflows.service import WorkflowService from askui.utils.api_utils import ListQuery, NotFoundError diff --git a/tests/unit/test_execution_status_transitions.py b/tests/unit/test_execution_status_transitions.py index 4f13f918..78ef0a3c 100644 --- a/tests/unit/test_execution_status_transitions.py +++ b/tests/unit/test_execution_status_transitions.py @@ -2,7 +2,7 @@ import pytest -from askui.chat.api.executions.models import ( +from askui.chat.api.workflow_executions.models import ( ExecutionStatus, InvalidStatusTransitionError, _validate_status_transition, @@ -150,7 +150,7 @@ def test_specific_invalid_transitions_parametrized( def test_transition_map_structure(self) -> None: """Test the structure of the transition map.""" - from askui.chat.api.executions.models import _STATUS_TRANSITIONS + from askui.chat.api.workflow_executions.models import _STATUS_TRANSITIONS # All statuses should be keys in the transition map all_statuses = set(ExecutionStatus) From c51ac932b5bef7d1b420db8d9f1c27ef3e0aaf29 Mon Sep 17 00:00:00 2001 From: danyalxahid-askui Date: Thu, 11 Sep 2025 23:20:30 +0200 Subject: [PATCH 14/19] refactor(workflow-executions): rename execution identifiers and remove unused error handler - Renamed `ExecutionId` to `WorkflowExecutionId` for clarity. - Updated related models and service methods to reflect the new naming convention. - Removed the unused `invalid_status_transition_error_handler` to streamline the codebase. --- src/askui/chat/api/app.py | 11 -- src/askui/chat/api/models.py | 2 +- .../chat/api/workflow_executions/models.py | 126 +----------- .../chat/api/workflow_executions/router.py | 32 +--- .../chat/api/workflow_executions/service.py | 57 +++--- .../chat/api/test_executions_service.py | 50 +++-- .../unit/test_execution_status_transitions.py | 181 ------------------ 7 files changed, 61 insertions(+), 398 deletions(-) delete mode 100644 tests/unit/test_execution_status_transitions.py diff --git a/src/askui/chat/api/app.py b/src/askui/chat/api/app.py index cde4b5c5..bf633d2c 100644 --- a/src/askui/chat/api/app.py +++ b/src/askui/chat/api/app.py @@ -141,17 +141,6 @@ def forbidden_error_handler( ) -@app.exception_handler(InvalidStatusTransitionError) -def invalid_status_transition_error_handler( - request: Request, # noqa: ARG001 - exc: InvalidStatusTransitionError, -) -> JSONResponse: - return JSONResponse( - status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, - content={"detail": str(exc)}, - ) - - @app.exception_handler(Exception) def catch_all_exception_handler( request: Request, # noqa: ARG001 diff --git a/src/askui/chat/api/models.py b/src/askui/chat/api/models.py index 2d6ae342..d566c86b 100644 --- a/src/askui/chat/api/models.py +++ b/src/askui/chat/api/models.py @@ -6,7 +6,7 @@ from askui.utils.id_utils import IdField AssistantId = Annotated[str, IdField("asst")] -ExecutionId = Annotated[str, IdField("exec")] +WorkflowExecutionId = Annotated[str, IdField("wfexec")] McpConfigId = Annotated[str, IdField("mcpcnf")] FileId = Annotated[str, IdField("file")] MessageId = Annotated[str, IdField("msg")] diff --git a/src/askui/chat/api/workflow_executions/models.py b/src/askui/chat/api/workflow_executions/models.py index 99ab97b4..3d91bc49 100644 --- a/src/askui/chat/api/workflow_executions/models.py +++ b/src/askui/chat/api/workflow_executions/models.py @@ -1,111 +1,34 @@ import datetime -from enum import Enum from typing import Literal -from pydantic import BaseModel, Field +from pydantic import BaseModel from askui.chat.api.models import ( - ExecutionId, RunId, ThreadId, + WorkflowExecutionId, WorkspaceId, WorkspaceResource, ) from askui.chat.api.workflows.models import WorkflowId from askui.utils.datetime_utils import now from askui.utils.id_utils import generate_time_ordered_id -from askui.utils.not_given import NOT_GIVEN, BaseModelWithNotGiven, NotGiven -class ExecutionStatus(str, Enum): - """The current status of the workflow execution.""" - - PASSED = "passed" - FAILED = "failed" - PENDING = "pending" - INCOMPLETE = "incomplete" - SKIPPED = "skipped" - - -class InvalidStatusTransitionError(ValueError): - """Raised when attempting an invalid status transition.""" - - def __init__( - self, from_status: ExecutionStatus, to_status: ExecutionStatus - ) -> None: - error_msg = f"Invalid status transition from '{from_status.value}' to '{to_status.value}'" - super().__init__(error_msg) - self.from_status = from_status - self.to_status = to_status - - -# Status transition map: defines which status transitions are allowed -_STATUS_TRANSITIONS: dict[ExecutionStatus, set[ExecutionStatus]] = { - # PENDING can transition to any status except PENDING (to avoid no-op updates) - ExecutionStatus.PENDING: { - ExecutionStatus.INCOMPLETE, - ExecutionStatus.PASSED, - ExecutionStatus.FAILED, - ExecutionStatus.SKIPPED, - }, - # INCOMPLETE can only transition to final states (no going backwards) - ExecutionStatus.INCOMPLETE: { - ExecutionStatus.PASSED, - ExecutionStatus.FAILED, - ExecutionStatus.SKIPPED, - }, - # Final states (PASSED, FAILED, SKIPPED) cannot transition to other states - ExecutionStatus.PASSED: set(), - ExecutionStatus.FAILED: set(), - ExecutionStatus.SKIPPED: set(), -} - - -def _validate_status_transition( - from_status: ExecutionStatus, to_status: ExecutionStatus -) -> None: - """ - Validate that a status transition is allowed. - - Args: - from_status (ExecutionStatus): The current status. - to_status (ExecutionStatus): The target status. - - Raises: - InvalidStatusTransitionError: If the transition is not allowed. - """ - if from_status == to_status: - # Allow same-status updates (no-op) - return - - allowed_transitions = _STATUS_TRANSITIONS.get(from_status, set()) - if to_status not in allowed_transitions: - raise InvalidStatusTransitionError(from_status, to_status) - - -class ExecutionCreateParams(BaseModel): +class WorkflowExecutionCreateParams(BaseModel): """ - Parameters for creating an execution via API. + Parameters for creating a workflow execution via API. """ workflow_id: WorkflowId -class ExecutionModifyParams(BaseModelWithNotGiven): - """ - Parameters for modifying an execution via API. - Only status can be updated. - """ - - status: ExecutionStatus | NotGiven = NOT_GIVEN - - -class Execution(WorkspaceResource): +class WorkflowExecution(WorkspaceResource): """ A workflow execution resource in the chat API. Args: - id (ExecutionId): The id of the execution. Must start with the 'exec_' prefix and be + id (WorkflowExecutionId): The id of the execution. Must start with the 'exec_' prefix and be followed by one or more alphanumerical characters. object (Literal['execution']): The object type, always 'execution'. created_at (datetime.datetime): The creation time as a datetime. @@ -115,15 +38,12 @@ class Execution(WorkspaceResource): workspace_id (WorkspaceId | None, optional): The workspace this execution belongs to. """ - id: ExecutionId + id: WorkflowExecutionId object: Literal["execution"] = "execution" created_at: datetime.datetime workflow_id: WorkflowId thread_id: ThreadId run_id: RunId - status: ExecutionStatus = Field( - ..., description="The current status of the workflow execution." - ) @classmethod def create( @@ -132,42 +52,12 @@ def create( workflow_id: WorkflowId, run_id: RunId, thread_id: ThreadId, - ) -> "Execution": + ) -> "WorkflowExecution": return cls( id=generate_time_ordered_id("exec"), created_at=now(), workspace_id=workspace_id, - status=ExecutionStatus.PENDING, run_id=run_id, thread_id=thread_id, workflow_id=workflow_id, ) - - def modify(self, params: ExecutionModifyParams) -> "Execution": - """ - Modify the execution with the provided parameters. - - Args: - params (ExecutionModifyParams): The parameters to update. - - Returns: - Execution: A new execution instance with the updated values. - - Raises: - InvalidStatusTransitionError: If attempting an invalid status transition. - """ - update_data = { - k: v for k, v in params.model_dump().items() if v is not NOT_GIVEN - } - - # Validate status transition if status is being updated - if "status" in update_data: - new_status = update_data["status"] - _validate_status_transition(self.status, new_status) - - return Execution.model_validate( - { - **self.model_dump(), - **update_data, - } - ) diff --git a/src/askui/chat/api/workflow_executions/router.py b/src/askui/chat/api/workflow_executions/router.py index 143ed7a9..81133c02 100644 --- a/src/askui/chat/api/workflow_executions/router.py +++ b/src/askui/chat/api/workflow_executions/router.py @@ -9,10 +9,9 @@ from askui.chat.api.models import ThreadId, WorkspaceId from askui.chat.api.workflow_executions.dependencies import ExecutionServiceDep from askui.chat.api.workflow_executions.models import ( - Execution, - ExecutionCreateParams, - ExecutionId, - ExecutionModifyParams, + WorkflowExecution, + WorkflowExecutionCreateParams, + WorkflowExecutionId, ) from askui.chat.api.workflow_executions.service import ExecutionService from askui.chat.api.workflows.models import WorkflowId @@ -28,7 +27,7 @@ def list_workflow_executions( workflow_id: Annotated[WorkflowId | None, Query()] = None, thread_id: Annotated[ThreadId | None, Query()] = None, execution_service: ExecutionService = ExecutionServiceDep, -) -> ListResponse[Execution]: +) -> ListResponse[WorkflowExecution]: """List executions with optional filtering by workflow and/or thread.""" return execution_service.list_( workspace_id=askui_workspace, @@ -41,10 +40,10 @@ def list_workflow_executions( @router.post("/") async def create_workflow_execution( askui_workspace: Annotated[WorkspaceId, Header()], - params: ExecutionCreateParams, + params: WorkflowExecutionCreateParams, background_tasks: BackgroundTasks, execution_service: ExecutionService = ExecutionServiceDep, -) -> Execution: +) -> WorkflowExecution: """Create a new workflow execution.""" execution, async_generator = await execution_service.create( workspace_id=askui_workspace, params=params @@ -61,25 +60,10 @@ async def _run_async_generator() -> None: @router.get("/{execution_id}") def retrieve_workflow_execution( askui_workspace: Annotated[WorkspaceId, Header()], - execution_id: ExecutionId, + execution_id: WorkflowExecutionId, execution_service: ExecutionService = ExecutionServiceDep, -) -> Execution: +) -> WorkflowExecution: """Retrieve a specific execution by ID.""" return execution_service.retrieve( workspace_id=askui_workspace, execution_id=execution_id ) - - -@router.patch("/{execution_id}") -def modify_workflow_execution( - askui_workspace: Annotated[WorkspaceId, Header()], - execution_id: ExecutionId, - params: ExecutionModifyParams, - execution_service: ExecutionService = ExecutionServiceDep, -) -> Execution: - """Update an existing execution (only status can be modified).""" - return execution_service.modify( - workspace_id=askui_workspace, - execution_id=execution_id, - params=params, - ) diff --git a/src/askui/chat/api/workflow_executions/service.py b/src/askui/chat/api/workflow_executions/service.py index b6f46a72..aa7390da 100644 --- a/src/askui/chat/api/workflow_executions/service.py +++ b/src/askui/chat/api/workflow_executions/service.py @@ -10,10 +10,9 @@ from askui.chat.api.threads.models import ThreadCreateParams from askui.chat.api.utils import build_workspace_filter_fn from askui.chat.api.workflow_executions.models import ( - Execution, - ExecutionCreateParams, - ExecutionId, - ExecutionModifyParams, + WorkflowExecution, + WorkflowExecutionCreateParams, + WorkflowExecutionId, ) from askui.chat.api.workflows.models import WorkflowId from askui.chat.api.workflows.service import WorkflowService @@ -30,13 +29,13 @@ def _build_execution_filter_fn( workspace_id: WorkspaceId, workflow_id: WorkflowId | None = None, thread_id: ThreadId | None = None, -) -> Callable[[Execution], bool]: +) -> Callable[[WorkflowExecution], bool]: """Build filter function for executions with optional workflow and thread filters.""" - workspace_filter: Callable[[Execution], bool] = build_workspace_filter_fn( - workspace_id, Execution + workspace_filter: Callable[[WorkflowExecution], bool] = build_workspace_filter_fn( + workspace_id, WorkflowExecution ) - def filter_fn(execution: Execution) -> bool: + def filter_fn(execution: WorkflowExecution) -> bool: if not workspace_filter(execution): return False if workflow_id is not None and execution.workflow_id != workflow_id: @@ -62,7 +61,9 @@ def __init__( self._workflow_service = workflow_service self._thread_facade = thread_facade - def _get_execution_path(self, execution_id: ExecutionId, new: bool = False) -> Path: + def _get_execution_path( + self, execution_id: WorkflowExecutionId, new: bool = False + ) -> Path: """Get the file path for an execution.""" execution_path = self._executions_dir / f"{execution_id}.json" exists = execution_path.exists() @@ -80,30 +81,28 @@ def list_( query: ListQuery, workflow_id: WorkflowId | None = None, thread_id: ThreadId | None = None, - ) -> ListResponse[Execution]: + ) -> ListResponse[WorkflowExecution]: """List executions with optional filtering by workflow and/or thread.""" return list_resources( base_dir=self._executions_dir, query=query, - resource_type=Execution, + resource_type=WorkflowExecution, filter_fn=_build_execution_filter_fn(workspace_id, workflow_id, thread_id), ) def retrieve( - self, workspace_id: WorkspaceId, execution_id: ExecutionId - ) -> Execution: + self, workspace_id: WorkspaceId, execution_id: WorkflowExecutionId + ) -> WorkflowExecution: """Retrieve a specific execution by ID.""" try: execution_path = self._get_execution_path(execution_id) - execution = Execution.model_validate_json(execution_path.read_text()) + execution = WorkflowExecution.model_validate_json( + execution_path.read_text() + ) # Check workspace access - allow if workspace_id is None (global access) # or if execution workspace matches or execution has no workspace - if ( - workspace_id is not None - and execution.workspace_id is not None - and execution.workspace_id != workspace_id - ): + if execution.workspace_id != workspace_id: error_msg = f"Execution {execution_id} not found" raise NotFoundError(error_msg) @@ -114,8 +113,8 @@ def retrieve( return execution async def create( - self, workspace_id: WorkspaceId, params: ExecutionCreateParams - ) -> tuple[Execution, AsyncGenerator[Events, None]]: + self, workspace_id: WorkspaceId, params: WorkflowExecutionCreateParams + ) -> tuple[WorkflowExecution, AsyncGenerator[Events, None]]: """Create a new execution.""" # Validate that the workflow exists in the same workspace workflow = self._workflow_service.retrieve(workspace_id, params.workflow_id) @@ -134,25 +133,13 @@ async def create( ), ) - execution = Execution.create( + execution = WorkflowExecution.create( workspace_id, params.workflow_id, run.id, run.thread_id ) self._save(execution, new=True) return execution, async_generator - def modify( - self, - workspace_id: WorkspaceId, - execution_id: ExecutionId, - params: ExecutionModifyParams, - ) -> Execution: - """Update an existing execution (only status can be modified).""" - execution = self.retrieve(workspace_id, execution_id) - modified = execution.modify(params) - self._save(modified) - return modified - - def _save(self, execution: Execution, new: bool = False) -> None: + def _save(self, execution: WorkflowExecution, new: bool = False) -> None: """Save execution to filesystem.""" self._executions_dir.mkdir(parents=True, exist_ok=True) execution_file = self._get_execution_path(execution.id, new=new) diff --git a/tests/integration/chat/api/test_executions_service.py b/tests/integration/chat/api/test_executions_service.py index 9f74f2ea..cd09c35c 100644 --- a/tests/integration/chat/api/test_executions_service.py +++ b/tests/integration/chat/api/test_executions_service.py @@ -14,11 +14,11 @@ from askui.chat.api.threads.facade import ThreadFacade from askui.chat.api.threads.service import ThreadService from askui.chat.api.workflow_executions.models import ( - Execution, - ExecutionCreateParams, ExecutionModifyParams, ExecutionStatus, InvalidStatusTransitionError, + WorkflowExecution, + WorkflowExecutionCreateParams, ) from askui.chat.api.workflow_executions.service import ExecutionService from askui.chat.api.workflows.models import WorkflowCreateParams @@ -144,9 +144,9 @@ def test_workflow_id( return workflow.id @pytest.fixture - def create_params(self, test_workflow_id: str) -> ExecutionCreateParams: + def create_params(self, test_workflow_id: str) -> WorkflowExecutionCreateParams: """Create sample execution creation parameters.""" - return ExecutionCreateParams( + return WorkflowExecutionCreateParams( workflow_id=test_workflow_id, ) @@ -155,8 +155,8 @@ async def sample_execution( self, execution_service: ExecutionService, workspace_id: WorkspaceId, - create_params: ExecutionCreateParams, - ) -> Execution: + create_params: WorkflowExecutionCreateParams, + ) -> WorkflowExecution: """Create a sample execution for testing.""" execution, _ = await execution_service.create( workspace_id=workspace_id, params=create_params @@ -168,7 +168,7 @@ async def test_create_execution_success( self, execution_service: ExecutionService, workspace_id: WorkspaceId, - create_params: ExecutionCreateParams, + create_params: WorkflowExecutionCreateParams, ) -> None: """Test successful execution creation.""" execution, _ = await execution_service.create( @@ -177,7 +177,6 @@ async def test_create_execution_success( assert execution.workflow_id == create_params.workflow_id assert execution.thread_id is not None # Thread is created automatically - assert execution.status == ExecutionStatus.PENDING assert execution.workspace_id == workspace_id assert execution.object == "execution" assert execution.id.startswith("exec_") @@ -192,7 +191,7 @@ async def test_create_execution_with_nonexistent_workflow_fails( """ Test that creating execution with non-existent workflow raises NotFoundError. """ - invalid_params = ExecutionCreateParams( + invalid_params = WorkflowExecutionCreateParams( workflow_id="wf_nonexistent123", ) @@ -222,7 +221,7 @@ async def test_create_execution_with_workflow_from_different_workspace_fails( ) execution_service = ExecutionService(tmp_path, workflow_service, thread_facade) - invalid_params = ExecutionCreateParams( + invalid_params = WorkflowExecutionCreateParams( workflow_id=test_workflow_id, ) @@ -238,7 +237,7 @@ async def test_retrieve_execution_success( self, execution_service: ExecutionService, workspace_id: WorkspaceId, - sample_execution: Execution, + sample_execution: WorkflowExecution, ) -> None: """Test successful execution retrieval.""" retrieved = execution_service.retrieve( @@ -248,7 +247,6 @@ async def test_retrieve_execution_success( assert retrieved.id == sample_execution.id assert retrieved.workflow_id == sample_execution.workflow_id assert retrieved.thread_id == sample_execution.thread_id - assert retrieved.status == sample_execution.status assert retrieved.workspace_id == sample_execution.workspace_id def test_retrieve_execution_not_found( @@ -267,7 +265,7 @@ async def test_modify_execution_valid_transition( self, execution_service: ExecutionService, workspace_id: WorkspaceId, - sample_execution: Execution, + sample_execution: WorkflowExecution, ) -> None: """Test successful execution modification with valid status transition.""" modify_params = ExecutionModifyParams(status=ExecutionStatus.PASSED) @@ -278,7 +276,6 @@ async def test_modify_execution_valid_transition( ) assert modified.id == sample_execution.id - assert modified.status == ExecutionStatus.PASSED assert modified.workflow_id == sample_execution.workflow_id assert modified.thread_id == sample_execution.thread_id @@ -287,7 +284,7 @@ async def test_modify_execution_invalid_transition_raises_error( self, execution_service: ExecutionService, workspace_id: WorkspaceId, - sample_execution: Execution, + sample_execution: WorkflowExecution, ) -> None: """Test that invalid status transition raises InvalidStatusTransitionError.""" # First transition to a final state @@ -318,7 +315,7 @@ async def test_modify_execution_same_status_allowed( self, execution_service: ExecutionService, workspace_id: WorkspaceId, - sample_execution: Execution, + sample_execution: WorkflowExecution, ) -> None: """Test that modifying to the same status is allowed (no-op).""" modify_params = ExecutionModifyParams(status=ExecutionStatus.PENDING) @@ -328,7 +325,6 @@ async def test_modify_execution_same_status_allowed( params=modify_params, ) - assert modified.status == ExecutionStatus.PENDING assert modified.id == sample_execution.id def test_modify_execution_not_found( @@ -351,7 +347,7 @@ async def test_list_executions_success( self, execution_service: ExecutionService, workspace_id: WorkspaceId, - sample_execution: Execution, + sample_execution: WorkflowExecution, ) -> None: """Test successful execution listing.""" query = ListQuery(limit=10, order="desc") @@ -367,7 +363,7 @@ async def test_list_executions_with_filters( execution_service: ExecutionService, workflow_service: WorkflowService, workspace_id: WorkspaceId, - create_params: ExecutionCreateParams, + create_params: WorkflowExecutionCreateParams, test_assistant_id: str, ) -> None: """Test execution listing with workflow and thread filters.""" @@ -386,7 +382,7 @@ async def test_list_executions_with_filters( workspace_id=workspace_id, params=different_workflow_params ) - different_params = ExecutionCreateParams( + different_params = WorkflowExecutionCreateParams( workflow_id=different_workflow.id, ) second_execution, _ = await execution_service.create( @@ -438,7 +434,7 @@ async def test_persistence_across_service_instances( self, workspace_id: WorkspaceId, execution_service: ExecutionService, - create_params: ExecutionCreateParams, + create_params: WorkflowExecutionCreateParams, ) -> None: """Test that executions persist across service instances.""" # Create execution with existing service instance @@ -451,7 +447,6 @@ async def test_persistence_across_service_instances( ) assert retrieved.id == execution.id - assert retrieved.status == execution.status assert retrieved.workflow_id == execution.workflow_id @pytest.mark.asyncio @@ -459,7 +454,7 @@ async def test_modify_execution_persists_changes( self, workspace_id: WorkspaceId, execution_service: ExecutionService, - create_params: ExecutionCreateParams, + create_params: WorkflowExecutionCreateParams, ) -> None: """Test that execution modifications are persisted to filesystem.""" # Create execution using existing service @@ -479,7 +474,6 @@ async def test_modify_execution_persists_changes( retrieved = execution_service.retrieve( workspace_id=workspace_id, execution_id=execution.id ) - assert retrieved.status == ExecutionStatus.PASSED @pytest.mark.parametrize( "target_status", @@ -500,7 +494,7 @@ async def test_valid_transitions_from_pending( ) -> None: """Test all valid transitions from PENDING status (parametrized).""" # Create execution (always starts as PENDING) - create_params = ExecutionCreateParams( + create_params = WorkflowExecutionCreateParams( workflow_id=test_workflow_id, ) execution, _ = await execution_service.create( @@ -535,7 +529,7 @@ async def test_valid_transitions_from_incomplete( ) -> None: """Test all valid transitions from INCOMPLETE status (parametrized).""" # Create execution and move to INCOMPLETE - create_params = ExecutionCreateParams( + create_params = WorkflowExecutionCreateParams( workflow_id=test_workflow_id, ) execution, _ = await execution_service.create( @@ -569,7 +563,7 @@ async def test_incomplete_cannot_go_back_to_pending( ) -> None: """Test that INCOMPLETE cannot transition back to PENDING.""" # Create execution and move to INCOMPLETE - create_params = ExecutionCreateParams( + create_params = WorkflowExecutionCreateParams( workflow_id=test_workflow_id, ) execution, _ = await execution_service.create( @@ -631,7 +625,7 @@ async def test_final_states_cannot_transition( pytest.skip("Same-status transitions are allowed as no-ops") # Create execution and move to final state - create_params = ExecutionCreateParams( + create_params = WorkflowExecutionCreateParams( workflow_id=test_workflow_id, ) execution, _ = await execution_service.create( diff --git a/tests/unit/test_execution_status_transitions.py b/tests/unit/test_execution_status_transitions.py deleted file mode 100644 index 78ef0a3c..00000000 --- a/tests/unit/test_execution_status_transitions.py +++ /dev/null @@ -1,181 +0,0 @@ -"""Unit tests for execution status transition validation logic.""" - -import pytest - -from askui.chat.api.workflow_executions.models import ( - ExecutionStatus, - InvalidStatusTransitionError, - _validate_status_transition, -) - - -class TestStatusTransitionValidation: - """Test the low-level status transition validation function.""" - - def test_same_status_transition_allowed(self) -> None: - """Test that transitioning to the same status is always allowed.""" - for status in ExecutionStatus: - # Should not raise an exception - _validate_status_transition(status, status) - - def test_valid_transitions_from_pending(self) -> None: - """Test valid transitions from PENDING status.""" - valid_targets = [ - ExecutionStatus.INCOMPLETE, - ExecutionStatus.PASSED, - ExecutionStatus.FAILED, - ExecutionStatus.SKIPPED, - ] - - for target in valid_targets: - _validate_status_transition(ExecutionStatus.PENDING, target) - - def test_invalid_transitions_from_pending(self) -> None: - """ - Test that PENDING cannot transition back to PENDING (via validation logic). - """ - # Note: same-status transitions are allowed at the validation level, - # but this tests the transition map logic - # PENDING -> PENDING is actually allowed as a no-op - return - - def test_valid_transitions_from_incomplete(self) -> None: - """Test valid transitions from INCOMPLETE status.""" - valid_targets = [ - ExecutionStatus.PASSED, - ExecutionStatus.FAILED, - ExecutionStatus.SKIPPED, - ] - - for target in valid_targets: - _validate_status_transition(ExecutionStatus.INCOMPLETE, target) - - def test_incomplete_cannot_go_back_to_pending(self) -> None: - """Test that INCOMPLETE cannot transition back to PENDING.""" - with pytest.raises(InvalidStatusTransitionError) as exc_info: - _validate_status_transition( - ExecutionStatus.INCOMPLETE, ExecutionStatus.PENDING - ) - - assert exc_info.value.from_status == ExecutionStatus.INCOMPLETE - assert exc_info.value.to_status == ExecutionStatus.PENDING - - def test_no_transitions_from_final_states(self) -> None: - """Test that final states cannot transition to any other status.""" - final_states = [ - ExecutionStatus.PASSED, - ExecutionStatus.FAILED, - ExecutionStatus.SKIPPED, - ] - - all_other_statuses = [ - ExecutionStatus.PENDING, - ExecutionStatus.INCOMPLETE, - ] - - for final_state in final_states: - for target in all_other_statuses: - with pytest.raises(InvalidStatusTransitionError) as exc_info: - _validate_status_transition(final_state, target) - - assert exc_info.value.from_status == final_state - assert exc_info.value.to_status == target - - def test_final_states_cannot_transition_to_each_other(self) -> None: - """Test that final states cannot transition to other final states.""" - final_states = [ - ExecutionStatus.PASSED, - ExecutionStatus.FAILED, - ExecutionStatus.SKIPPED, - ] - - for from_state in final_states: - for to_state in final_states: - if from_state == to_state: - # Same status is allowed - continue - - with pytest.raises(InvalidStatusTransitionError) as exc_info: - _validate_status_transition(from_state, to_state) - - assert exc_info.value.from_status == from_state - assert exc_info.value.to_status == to_state - - def test_error_message_format(self) -> None: - """Test that the error message has the expected format.""" - with pytest.raises(InvalidStatusTransitionError) as exc_info: - _validate_status_transition(ExecutionStatus.PASSED, ExecutionStatus.PENDING) - - error = exc_info.value - expected_msg = "Invalid status transition from 'passed' to 'pending'" - assert str(error) == expected_msg - - def test_error_attributes(self) -> None: - """Test that the error has the correct attributes.""" - from_status = ExecutionStatus.FAILED - to_status = ExecutionStatus.INCOMPLETE - - with pytest.raises(InvalidStatusTransitionError) as exc_info: - _validate_status_transition(from_status, to_status) - - error = exc_info.value - assert error.from_status == from_status - assert error.to_status == to_status - assert isinstance(error, ValueError) - - @pytest.mark.parametrize( - "final_state", - [ - ExecutionStatus.PASSED, - ExecutionStatus.FAILED, - ExecutionStatus.SKIPPED, - ], - ) - @pytest.mark.parametrize( - "target_state", - [ - ExecutionStatus.PENDING, - ExecutionStatus.INCOMPLETE, - ], - ) - def test_specific_invalid_transitions_parametrized( - self, final_state: ExecutionStatus, target_state: ExecutionStatus - ) -> None: - """ - Test specific invalid transitions mentioned in requirements using - parametrization. - """ - with pytest.raises(InvalidStatusTransitionError): - _validate_status_transition(final_state, target_state) - - def test_transition_map_structure(self) -> None: - """Test the structure of the transition map.""" - from askui.chat.api.workflow_executions.models import _STATUS_TRANSITIONS - - # All statuses should be keys in the transition map - all_statuses = set(ExecutionStatus) - map_keys = set(_STATUS_TRANSITIONS.keys()) - assert all_statuses == map_keys - - # Final states should have empty transition sets - final_states = [ - ExecutionStatus.PASSED, - ExecutionStatus.FAILED, - ExecutionStatus.SKIPPED, - ] - - for final_state in final_states: - assert _STATUS_TRANSITIONS[final_state] == set(), ( - f"{final_state} should have no allowed transitions" - ) - - # Non-final states should have non-empty transition sets - non_final_states = [ - ExecutionStatus.PENDING, - ExecutionStatus.INCOMPLETE, - ] - - for non_final_state in non_final_states: - assert len(_STATUS_TRANSITIONS[non_final_state]) > 0, ( - f"{non_final_state} should have allowed transitions" - ) From 06f1ffa59e5dd38b9cda1270275f72d483c0bf7a Mon Sep 17 00:00:00 2001 From: danyalxahid-askui Date: Thu, 11 Sep 2025 23:57:52 +0200 Subject: [PATCH 15/19] refactor(workflow-executions): update endpoint and model references for workflow executions - Updated model attribute `object` from `"execution"` to `"workflow_execution"` to reflect the new naming convention. - Adjusted related tests to align with the updated endpoint and model changes. - Removed unused imports and test cases related to execution status transitions. --- src/askui/chat/api/app.py | 1 - .../chat/api/workflow_executions/models.py | 4 +- .../chat/api/test_executions_router.py | 227 +------------ .../chat/api/test_executions_service.py | 304 +----------------- 4 files changed, 11 insertions(+), 525 deletions(-) diff --git a/src/askui/chat/api/app.py b/src/askui/chat/api/app.py index bf633d2c..491ca1dc 100644 --- a/src/askui/chat/api/app.py +++ b/src/askui/chat/api/app.py @@ -21,7 +21,6 @@ from askui.chat.api.messages.router import router as messages_router from askui.chat.api.runs.router import router as runs_router from askui.chat.api.threads.router import router as threads_router -from askui.chat.api.workflow_executions.models import InvalidStatusTransitionError from askui.chat.api.workflow_executions.router import router as executions_router from askui.chat.api.workflows.router import router as workflows_router from askui.utils.api_utils import ( diff --git a/src/askui/chat/api/workflow_executions/models.py b/src/askui/chat/api/workflow_executions/models.py index 3d91bc49..3e2c9529 100644 --- a/src/askui/chat/api/workflow_executions/models.py +++ b/src/askui/chat/api/workflow_executions/models.py @@ -39,7 +39,7 @@ class WorkflowExecution(WorkspaceResource): """ id: WorkflowExecutionId - object: Literal["execution"] = "execution" + object: Literal["workflow_execution"] = "workflow_execution" created_at: datetime.datetime workflow_id: WorkflowId thread_id: ThreadId @@ -54,7 +54,7 @@ def create( thread_id: ThreadId, ) -> "WorkflowExecution": return cls( - id=generate_time_ordered_id("exec"), + id=generate_time_ordered_id("wfexec"), created_at=now(), workspace_id=workspace_id, run_id=run_id, diff --git a/tests/integration/chat/api/test_executions_router.py b/tests/integration/chat/api/test_executions_router.py index a23ea98d..dc8ff8cc 100644 --- a/tests/integration/chat/api/test_executions_router.py +++ b/tests/integration/chat/api/test_executions_router.py @@ -18,7 +18,6 @@ from askui.chat.api.runs.service import RunService from askui.chat.api.threads.facade import ThreadFacade from askui.chat.api.threads.service import ThreadService -from askui.chat.api.workflow_executions.models import ExecutionStatus from askui.chat.api.workflow_executions.service import ExecutionService from askui.chat.api.workflows.models import WorkflowCreateParams from askui.chat.api.workflows.service import WorkflowService @@ -182,7 +181,7 @@ def test_create_execution_success( ) -> None: """Test successful execution creation.""" response = client.post( - "/v1/executions/", + "/v1/workflow-executions/", json=execution_data, headers={"askui-workspace": str(workspace_id)}, ) @@ -191,163 +190,10 @@ def test_create_execution_success( data = response.json() assert data["workflow_id"] == execution_data["workflow_id"] assert data["thread_id"] is not None # Thread is created automatically - assert data["status"] == ExecutionStatus.PENDING.value - assert data["object"] == "execution" + assert data["object"] == "workflow_execution" assert "id" in data assert "created_at" in data - def test_modify_execution_valid_transition( - self, - client: TestClient, - workspace_id: WorkspaceId, - execution_data: dict[str, str], - ) -> None: - """Test successful execution modification with valid status transition.""" - # First create an execution - create_response = client.post( - "/v1/executions/", - json=execution_data, - headers={"askui-workspace": str(workspace_id)}, - ) - assert create_response.status_code == 200 - execution = create_response.json() - execution_id = execution["id"] - - # Then modify it with a valid transition (PENDING -> PASSED) - modify_data = {"status": ExecutionStatus.PASSED.value} - modify_response = client.patch( - f"/v1/executions/{execution_id}", - json=modify_data, - headers={"askui-workspace": str(workspace_id)}, - ) - - assert modify_response.status_code == 200 - modified_execution = modify_response.json() - assert modified_execution["status"] == ExecutionStatus.PASSED.value - assert modified_execution["id"] == execution_id - - def test_modify_execution_invalid_transition( - self, - client: TestClient, - workspace_id: WorkspaceId, - execution_data: dict[str, str], - ) -> None: - """Test execution modification with invalid status transition returns 422.""" - # First create an execution and transition it to a final state - create_response = client.post( - "/v1/executions/", - json=execution_data, - headers={"askui-workspace": str(workspace_id)}, - ) - assert create_response.status_code == 200 - execution = create_response.json() - execution_id = execution["id"] - - # Transition to final state (PENDING -> PASSED) - modify_data = {"status": ExecutionStatus.PASSED.value} - modify_response = client.patch( - f"/v1/executions/{execution_id}", - json=modify_data, - headers={"askui-workspace": str(workspace_id)}, - ) - assert modify_response.status_code == 200 - - # Try to transition from final state to non-final state (PASSED -> PENDING) - invalid_modify_data = {"status": ExecutionStatus.PENDING.value} - invalid_response = client.patch( - f"/v1/executions/{execution_id}", - json=invalid_modify_data, - headers={"askui-workspace": str(workspace_id)}, - ) - - # FastAPI should convert ValueError to 422 Unprocessable Entity - assert invalid_response.status_code == 422 - error_data = invalid_response.json() - assert "detail" in error_data - # The error should contain information about the invalid transition - error_detail = str(error_data["detail"]) - assert "Invalid status transition" in error_detail - assert "passed" in error_detail - assert "pending" in error_detail - - def test_modify_execution_same_status_allowed( - self, - client: TestClient, - workspace_id: WorkspaceId, - execution_data: dict[str, str], - ) -> None: - """Test that modifying to the same status is allowed (no-op).""" - # Create an execution - create_response = client.post( - "/v1/executions/", - json=execution_data, - headers={"askui-workspace": str(workspace_id)}, - ) - assert create_response.status_code == 200 - execution = create_response.json() - execution_id = execution["id"] - - # Modify to the same status (PENDING -> PENDING) - modify_data = {"status": ExecutionStatus.PENDING.value} - modify_response = client.patch( - f"/v1/executions/{execution_id}", - json=modify_data, - headers={"askui-workspace": str(workspace_id)}, - ) - - assert modify_response.status_code == 200 - modified_execution = modify_response.json() - assert modified_execution["status"] == ExecutionStatus.PENDING.value - - def test_modify_execution_multiple_valid_transitions( - self, - client: TestClient, - workspace_id: WorkspaceId, - execution_data: dict[str, str], - ) -> None: - """Test multiple valid status transitions in sequence.""" - # Create an execution - create_response = client.post( - "/v1/executions/", - json=execution_data, - headers={"askui-workspace": str(workspace_id)}, - ) - assert create_response.status_code == 200 - execution = create_response.json() - execution_id = execution["id"] - - # Valid transition sequence: PENDING -> INCOMPLETE -> PASSED - transitions = [ - (ExecutionStatus.INCOMPLETE.value, 200), - (ExecutionStatus.PASSED.value, 200), - ] - - for status, expected_code in transitions: - modify_data = {"status": status} - modify_response = client.patch( - f"/v1/executions/{execution_id}", - json=modify_data, - headers={"askui-workspace": str(workspace_id)}, - ) - - assert modify_response.status_code == expected_code - if expected_code == 200: - modified_execution = modify_response.json() - assert modified_execution["status"] == status - - def test_modify_execution_not_found( - self, client: TestClient, workspace_id: WorkspaceId - ) -> None: - """Test modifying non-existent execution returns 404.""" - modify_data = {"status": ExecutionStatus.PASSED.value} - response = client.patch( - "/v1/executions/exec_nonexistent123", - json=modify_data, - headers={"askui-workspace": str(workspace_id)}, - ) - - assert response.status_code == 404 - def test_retrieve_execution_success( self, client: TestClient, @@ -357,7 +203,7 @@ def test_retrieve_execution_success( """Test successful execution retrieval.""" # Create an execution create_response = client.post( - "/v1/executions/", + "/v1/workflow-executions/", json=execution_data, headers={"askui-workspace": str(workspace_id)}, ) @@ -367,14 +213,13 @@ def test_retrieve_execution_success( # Retrieve the execution retrieve_response = client.get( - f"/v1/executions/{execution_id}", + f"/v1/workflow-executions/{execution_id}", headers={"askui-workspace": str(workspace_id)}, ) assert retrieve_response.status_code == 200 retrieved_execution = retrieve_response.json() assert retrieved_execution["id"] == execution_id - assert retrieved_execution["status"] == ExecutionStatus.PENDING.value def test_list_executions_success( self, @@ -385,7 +230,7 @@ def test_list_executions_success( """Test successful execution listing.""" # Create an execution create_response = client.post( - "/v1/executions/", + "/v1/workflow-executions/", json=execution_data, headers={"askui-workspace": str(workspace_id)}, ) @@ -393,7 +238,7 @@ def test_list_executions_success( # List executions list_response = client.get( - "/v1/executions/", + "/v1/workflow-executions/", headers={"askui-workspace": str(workspace_id)}, ) @@ -403,63 +248,3 @@ def test_list_executions_success( assert "object" in data assert data["object"] == "list" assert len(data["data"]) >= 1 - - @pytest.mark.parametrize( - "final_status", - [ - ExecutionStatus.PASSED, - ExecutionStatus.FAILED, - ExecutionStatus.SKIPPED, - ], - ) - @pytest.mark.parametrize( - "invalid_target", - [ - ExecutionStatus.PENDING, - ExecutionStatus.INCOMPLETE, - ], - ) - def test_final_states_cannot_transition_parametrized( - self, - client: TestClient, - workspace_id: WorkspaceId, - execution_data: dict[str, str], - final_status: ExecutionStatus, - invalid_target: ExecutionStatus, - ) -> None: - """ - Test that final states cannot transition to non-final states (parametrized). - """ - # Create an execution - create_response = client.post( - "/v1/executions/", - json=execution_data, - headers={"askui-workspace": str(workspace_id)}, - ) - assert create_response.status_code == 200 - execution = create_response.json() - execution_id = execution["id"] - - # Transition to final state - modify_data = {"status": final_status.value} - modify_response = client.patch( - f"/v1/executions/{execution_id}", - json=modify_data, - headers={"askui-workspace": str(workspace_id)}, - ) - assert modify_response.status_code == 200 - - # Try to transition to invalid target - invalid_modify_data = {"status": invalid_target.value} - invalid_response = client.patch( - f"/v1/executions/{execution_id}", - json=invalid_modify_data, - headers={"askui-workspace": str(workspace_id)}, - ) - - assert invalid_response.status_code == 422 - error_data = invalid_response.json() - error_detail = str(error_data["detail"]) - assert "Invalid status transition" in error_detail - assert final_status.value in error_detail - assert invalid_target.value in error_detail diff --git a/tests/integration/chat/api/test_executions_service.py b/tests/integration/chat/api/test_executions_service.py index cd09c35c..34d36090 100644 --- a/tests/integration/chat/api/test_executions_service.py +++ b/tests/integration/chat/api/test_executions_service.py @@ -14,9 +14,6 @@ from askui.chat.api.threads.facade import ThreadFacade from askui.chat.api.threads.service import ThreadService from askui.chat.api.workflow_executions.models import ( - ExecutionModifyParams, - ExecutionStatus, - InvalidStatusTransitionError, WorkflowExecution, WorkflowExecutionCreateParams, ) @@ -178,8 +175,8 @@ async def test_create_execution_success( assert execution.workflow_id == create_params.workflow_id assert execution.thread_id is not None # Thread is created automatically assert execution.workspace_id == workspace_id - assert execution.object == "execution" - assert execution.id.startswith("exec_") + assert execution.object == "workflow_execution" + assert execution.id.startswith("wfexec_") assert execution.created_at is not None @pytest.mark.asyncio @@ -260,88 +257,6 @@ def test_retrieve_execution_not_found( assert "not found" in str(exc_info.value) - @pytest.mark.asyncio - async def test_modify_execution_valid_transition( - self, - execution_service: ExecutionService, - workspace_id: WorkspaceId, - sample_execution: WorkflowExecution, - ) -> None: - """Test successful execution modification with valid status transition.""" - modify_params = ExecutionModifyParams(status=ExecutionStatus.PASSED) - modified = execution_service.modify( - workspace_id=workspace_id, - execution_id=sample_execution.id, - params=modify_params, - ) - - assert modified.id == sample_execution.id - assert modified.workflow_id == sample_execution.workflow_id - assert modified.thread_id == sample_execution.thread_id - - @pytest.mark.asyncio - async def test_modify_execution_invalid_transition_raises_error( - self, - execution_service: ExecutionService, - workspace_id: WorkspaceId, - sample_execution: WorkflowExecution, - ) -> None: - """Test that invalid status transition raises InvalidStatusTransitionError.""" - # First transition to a final state - modify_params = ExecutionModifyParams(status=ExecutionStatus.PASSED) - execution_service.modify( - workspace_id=workspace_id, - execution_id=sample_execution.id, - params=modify_params, - ) - - # Try to transition from final state to non-final state - invalid_params = ExecutionModifyParams(status=ExecutionStatus.PENDING) - with pytest.raises(InvalidStatusTransitionError) as exc_info: - execution_service.modify( - workspace_id=workspace_id, - execution_id=sample_execution.id, - params=invalid_params, - ) - - assert exc_info.value.from_status == ExecutionStatus.PASSED - assert exc_info.value.to_status == ExecutionStatus.PENDING - assert "Invalid status transition from 'passed' to 'pending'" in str( - exc_info.value - ) - - @pytest.mark.asyncio - async def test_modify_execution_same_status_allowed( - self, - execution_service: ExecutionService, - workspace_id: WorkspaceId, - sample_execution: WorkflowExecution, - ) -> None: - """Test that modifying to the same status is allowed (no-op).""" - modify_params = ExecutionModifyParams(status=ExecutionStatus.PENDING) - modified = execution_service.modify( - workspace_id=workspace_id, - execution_id=sample_execution.id, - params=modify_params, - ) - - assert modified.id == sample_execution.id - - def test_modify_execution_not_found( - self, execution_service: ExecutionService, workspace_id: WorkspaceId - ) -> None: - """Test modifying non-existent execution raises NotFoundError.""" - modify_params = ExecutionModifyParams(status=ExecutionStatus.PASSED) - - with pytest.raises(NotFoundError) as exc_info: - execution_service.modify( - workspace_id=workspace_id, - execution_id="exec_nonexistent123", - params=modify_params, - ) - - assert "not found" in str(exc_info.value) - @pytest.mark.asyncio async def test_list_executions_success( self, @@ -385,7 +300,7 @@ async def test_list_executions_with_filters( different_params = WorkflowExecutionCreateParams( workflow_id=different_workflow.id, ) - second_execution, _ = await execution_service.create( + await execution_service.create( workspace_id=workspace_id, params=different_params ) @@ -448,216 +363,3 @@ async def test_persistence_across_service_instances( assert retrieved.id == execution.id assert retrieved.workflow_id == execution.workflow_id - - @pytest.mark.asyncio - async def test_modify_execution_persists_changes( - self, - workspace_id: WorkspaceId, - execution_service: ExecutionService, - create_params: WorkflowExecutionCreateParams, - ) -> None: - """Test that execution modifications are persisted to filesystem.""" - # Create execution using existing service - execution, _ = await execution_service.create( - workspace_id=workspace_id, params=create_params - ) - - # Modify execution - modify_params = ExecutionModifyParams(status=ExecutionStatus.PASSED) - execution_service.modify( - workspace_id=workspace_id, - execution_id=execution.id, - params=modify_params, - ) - - # Verify changes persist with new service instance - retrieved = execution_service.retrieve( - workspace_id=workspace_id, execution_id=execution.id - ) - - @pytest.mark.parametrize( - "target_status", - [ - ExecutionStatus.INCOMPLETE, - ExecutionStatus.PASSED, - ExecutionStatus.FAILED, - ExecutionStatus.SKIPPED, - ], - ) - @pytest.mark.asyncio - async def test_valid_transitions_from_pending( - self, - execution_service: ExecutionService, - workspace_id: WorkspaceId, - test_workflow_id: str, - target_status: ExecutionStatus, - ) -> None: - """Test all valid transitions from PENDING status (parametrized).""" - # Create execution (always starts as PENDING) - create_params = WorkflowExecutionCreateParams( - workflow_id=test_workflow_id, - ) - execution, _ = await execution_service.create( - workspace_id=workspace_id, params=create_params - ) - - # Test transition from PENDING to target status - modify_params = ExecutionModifyParams(status=target_status) - modified = execution_service.modify( - workspace_id=workspace_id, - execution_id=execution.id, - params=modify_params, - ) - - assert modified.status == target_status - - @pytest.mark.parametrize( - "target_status", - [ - ExecutionStatus.PASSED, - ExecutionStatus.FAILED, - ExecutionStatus.SKIPPED, - ], - ) - @pytest.mark.asyncio - async def test_valid_transitions_from_incomplete( - self, - execution_service: ExecutionService, - workspace_id: WorkspaceId, - test_workflow_id: str, - target_status: ExecutionStatus, - ) -> None: - """Test all valid transitions from INCOMPLETE status (parametrized).""" - # Create execution and move to INCOMPLETE - create_params = WorkflowExecutionCreateParams( - workflow_id=test_workflow_id, - ) - execution, _ = await execution_service.create( - workspace_id=workspace_id, params=create_params - ) - - # First move to INCOMPLETE - incomplete_params = ExecutionModifyParams(status=ExecutionStatus.INCOMPLETE) - execution = execution_service.modify( - workspace_id=workspace_id, - execution_id=execution.id, - params=incomplete_params, - ) - - # Test transition from INCOMPLETE to target status - modify_params = ExecutionModifyParams(status=target_status) - modified = execution_service.modify( - workspace_id=workspace_id, - execution_id=execution.id, - params=modify_params, - ) - - assert modified.status == target_status - - @pytest.mark.asyncio - async def test_incomplete_cannot_go_back_to_pending( - self, - execution_service: ExecutionService, - workspace_id: WorkspaceId, - test_workflow_id: str, - ) -> None: - """Test that INCOMPLETE cannot transition back to PENDING.""" - # Create execution and move to INCOMPLETE - create_params = WorkflowExecutionCreateParams( - workflow_id=test_workflow_id, - ) - execution, _ = await execution_service.create( - workspace_id=workspace_id, params=create_params - ) - - # Move to INCOMPLETE - incomplete_params = ExecutionModifyParams(status=ExecutionStatus.INCOMPLETE) - execution = execution_service.modify( - workspace_id=workspace_id, - execution_id=execution.id, - params=incomplete_params, - ) - - # Try to go back to PENDING (should fail) - pending_params = ExecutionModifyParams(status=ExecutionStatus.PENDING) - with pytest.raises(InvalidStatusTransitionError) as exc_info: - execution_service.modify( - workspace_id=workspace_id, - execution_id=execution.id, - params=pending_params, - ) - - assert exc_info.value.from_status == ExecutionStatus.INCOMPLETE - assert exc_info.value.to_status == ExecutionStatus.PENDING - - @pytest.mark.parametrize( - "final_status", - [ - ExecutionStatus.PASSED, - ExecutionStatus.FAILED, - ExecutionStatus.SKIPPED, - ], - ) - @pytest.mark.parametrize( - "target_status", - [ - ExecutionStatus.PENDING, - ExecutionStatus.INCOMPLETE, - ExecutionStatus.PASSED, - ExecutionStatus.FAILED, - ExecutionStatus.SKIPPED, - ], - ) - @pytest.mark.asyncio - async def test_final_states_cannot_transition( - self, - execution_service: ExecutionService, - workspace_id: WorkspaceId, - test_workflow_id: str, - final_status: ExecutionStatus, - target_status: ExecutionStatus, - ) -> None: - """ - Test that final states cannot transition to any other status (parametrized). - """ - # Skip same-status transitions (they're allowed as no-ops) - if final_status == target_status: - pytest.skip("Same-status transitions are allowed as no-ops") - - # Create execution and move to final state - create_params = WorkflowExecutionCreateParams( - workflow_id=test_workflow_id, - ) - execution, _ = await execution_service.create( - workspace_id=workspace_id, params=create_params - ) - - # Move to final status (via INCOMPLETE if needed for realistic flow) - if final_status in [ExecutionStatus.PASSED, ExecutionStatus.FAILED]: - # Realistic flow: PENDING → INCOMPLETE → PASSED/FAILED - incomplete_params = ExecutionModifyParams(status=ExecutionStatus.INCOMPLETE) - execution = execution_service.modify( - workspace_id=workspace_id, - execution_id=execution.id, - params=incomplete_params, - ) - - # Move to final status - final_params = ExecutionModifyParams(status=final_status) - execution = execution_service.modify( - workspace_id=workspace_id, - execution_id=execution.id, - params=final_params, - ) - - # Try to transition from final state to target status (should fail) - modify_params = ExecutionModifyParams(status=target_status) - with pytest.raises(InvalidStatusTransitionError) as exc_info: - execution_service.modify( - workspace_id=workspace_id, - execution_id=execution.id, - params=modify_params, - ) - - assert exc_info.value.from_status == final_status - assert exc_info.value.to_status == target_status From 97371de44a771bf277d1d4bf83757f1b6eeb06de Mon Sep 17 00:00:00 2001 From: danyalxahid-askui Date: Fri, 12 Sep 2025 01:49:32 +0200 Subject: [PATCH 16/19] feat(seeds): add new workflow-related endpoints to COMPUTER_AGENT - Included new endpoints for workflow management: `create_workflow_v1_workflows_post`, `list_workflows_v1_workflows_get`, `modify_workflow_v1_workflows`, and `create_workflow_execution_v1_workflow_executions` to enhance the capabilities of the COMPUTER_AGENT. --- src/askui/chat/api/assistants/seeds.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/askui/chat/api/assistants/seeds.py b/src/askui/chat/api/assistants/seeds.py index 2ad29e5a..c8785ab7 100644 --- a/src/askui/chat/api/assistants/seeds.py +++ b/src/askui/chat/api/assistants/seeds.py @@ -19,6 +19,10 @@ "list_displays", "set_active_display", "retrieve_active_display", + "create_workflow_v1_workflows_post", + "list_workflows_v1_workflows_get", + "modify_workflow_v1_workflows", + "create_workflow_execution_v1_workflow_executions", ], ) From 62145058c4cae21919b8c2a3a649c12fb4468959 Mon Sep 17 00:00:00 2001 From: danyalxahid-askui Date: Fri, 12 Sep 2025 02:19:08 +0200 Subject: [PATCH 17/19] fix(router): require `askui_workspace` header in workflow endpoints - Updated the `askui_workspace` parameter in the `list_workflows`, `create_workflow`, `retrieve_workflow`, and `modify_workflow` endpoints to be a required header instead of optional. This change ensures that the workspace context is always provided for workflow operations. - Refactored tests to utilize the `TestClient` context manager for better resource management and cleanup. --- src/askui/chat/api/workflows/router.py | 8 +- .../chat/api/test_executions_router.py | 136 ++++++++---------- 2 files changed, 66 insertions(+), 78 deletions(-) diff --git a/src/askui/chat/api/workflows/router.py b/src/askui/chat/api/workflows/router.py index c0546db7..34c7d883 100644 --- a/src/askui/chat/api/workflows/router.py +++ b/src/askui/chat/api/workflows/router.py @@ -19,7 +19,7 @@ @router.get("") def list_workflows( - askui_workspace: Annotated[WorkspaceId | None, Header()], + askui_workspace: Annotated[WorkspaceId, Header()], tags: Annotated[list[str] | None, Query()] = None, query: ListQuery = ListQueryDep, workflow_service: WorkflowService = WorkflowServiceDep, @@ -41,7 +41,7 @@ def list_workflows( @router.post("", status_code=status.HTTP_201_CREATED) def create_workflow( - askui_workspace: Annotated[WorkspaceId | None, Header()], + askui_workspace: Annotated[WorkspaceId, Header()], params: WorkflowCreateParams, workflow_service: WorkflowService = WorkflowServiceDep, ) -> Workflow: @@ -61,7 +61,7 @@ def create_workflow( @router.get("/{workflow_id}") def retrieve_workflow( - askui_workspace: Annotated[WorkspaceId | None, Header()], + askui_workspace: Annotated[WorkspaceId, Header()], workflow_id: Annotated[WorkflowId, Path(...)], workflow_service: WorkflowService = WorkflowServiceDep, ) -> Workflow: @@ -86,7 +86,7 @@ def retrieve_workflow( @router.patch("/{workflow_id}") def modify_workflow( - askui_workspace: Annotated[WorkspaceId | None, Header()], + askui_workspace: Annotated[WorkspaceId, Header()], workflow_id: Annotated[WorkflowId, Path(...)], params: WorkflowModifyParams, workflow_service: WorkflowService = WorkflowServiceDep, diff --git a/tests/integration/chat/api/test_executions_router.py b/tests/integration/chat/api/test_executions_router.py index dc8ff8cc..ee7956f7 100644 --- a/tests/integration/chat/api/test_executions_router.py +++ b/tests/integration/chat/api/test_executions_router.py @@ -107,28 +107,6 @@ def execution_service( """Create an execution service for testing.""" return ExecutionService(temp_workspace_dir, workflow_service, thread_facade) - @pytest.fixture - def client( - self, - workflow_service: WorkflowService, - execution_service: ExecutionService, - ) -> TestClient: - """Create a test client for the FastAPI app with overridden dependencies.""" - from askui.chat.api.workflow_executions.dependencies import ( - get_execution_service, - ) - from askui.chat.api.workflows.dependencies import get_workflow_service - - # Override service dependencies directly - app.dependency_overrides[get_workflow_service] = lambda: workflow_service - app.dependency_overrides[get_execution_service] = lambda: execution_service - - client = TestClient(app) - - # Clean up overrides after test (this will be called when fixture is torn down) - yield client - app.dependency_overrides.clear() - @pytest.fixture def workspace_id(self) -> WorkspaceId: """Create a test workspace ID.""" @@ -175,76 +153,86 @@ def execution_data(self, test_workflow_id: str) -> dict[str, str]: def test_create_execution_success( self, - client: TestClient, workspace_id: WorkspaceId, execution_data: dict[str, str], ) -> None: """Test successful execution creation.""" - response = client.post( - "/v1/workflow-executions/", - json=execution_data, - headers={"askui-workspace": str(workspace_id)}, - ) - - assert response.status_code == 200 - data = response.json() - assert data["workflow_id"] == execution_data["workflow_id"] - assert data["thread_id"] is not None # Thread is created automatically - assert data["object"] == "workflow_execution" - assert "id" in data - assert "created_at" in data + try: + with TestClient(app) as client: + response = client.post( + "/v1/workflow-executions/", + json=execution_data, + headers={"askui-workspace": str(workspace_id)}, + ) + + assert response.status_code == 200 + data = response.json() + assert data["workflow_id"] == execution_data["workflow_id"] + assert data["thread_id"] is not None # Thread is created automatically + assert data["object"] == "workflow_execution" + assert "id" in data + assert "created_at" in data + finally: + app.dependency_overrides.clear() def test_retrieve_execution_success( self, - client: TestClient, workspace_id: WorkspaceId, execution_data: dict[str, str], ) -> None: """Test successful execution retrieval.""" # Create an execution - create_response = client.post( - "/v1/workflow-executions/", - json=execution_data, - headers={"askui-workspace": str(workspace_id)}, - ) - assert create_response.status_code == 200 - execution = create_response.json() - execution_id = execution["id"] - - # Retrieve the execution - retrieve_response = client.get( - f"/v1/workflow-executions/{execution_id}", - headers={"askui-workspace": str(workspace_id)}, - ) - assert retrieve_response.status_code == 200 - retrieved_execution = retrieve_response.json() - assert retrieved_execution["id"] == execution_id + try: + with TestClient(app) as client: + create_response = client.post( + "/v1/workflow-executions/", + json=execution_data, + headers={"askui-workspace": str(workspace_id)}, + ) + assert create_response.status_code == 200 + execution = create_response.json() + execution_id = execution["id"] + + # Retrieve the execution + retrieve_response = client.get( + f"/v1/workflow-executions/{execution_id}", + headers={"askui-workspace": str(workspace_id)}, + ) + + assert retrieve_response.status_code == 200 + retrieved_execution = retrieve_response.json() + assert retrieved_execution["id"] == execution_id + finally: + app.dependency_overrides.clear() def test_list_executions_success( self, - client: TestClient, workspace_id: WorkspaceId, execution_data: dict[str, str], ) -> None: """Test successful execution listing.""" # Create an execution - create_response = client.post( - "/v1/workflow-executions/", - json=execution_data, - headers={"askui-workspace": str(workspace_id)}, - ) - assert create_response.status_code == 200 - - # List executions - list_response = client.get( - "/v1/workflow-executions/", - headers={"askui-workspace": str(workspace_id)}, - ) - - assert list_response.status_code == 200 - data = list_response.json() - assert "data" in data - assert "object" in data - assert data["object"] == "list" - assert len(data["data"]) >= 1 + try: + with TestClient(app) as client: + create_response = client.post( + "/v1/workflow-executions/", + json=execution_data, + headers={"askui-workspace": str(workspace_id)}, + ) + assert create_response.status_code == 200 + + # List executions + list_response = client.get( + "/v1/workflow-executions/", + headers={"askui-workspace": str(workspace_id)}, + ) + + assert list_response.status_code == 200 + data = list_response.json() + assert "data" in data + assert "object" in data + assert data["object"] == "list" + assert len(data["data"]) >= 1 + finally: + app.dependency_overrides.clear() From 4725024bd3064b3a6893ae3a8b635610133a5b60 Mon Sep 17 00:00:00 2001 From: danyalxahid-askui Date: Fri, 12 Sep 2025 02:41:29 +0200 Subject: [PATCH 18/19] refactor(tests): enhance dependency injection in execution router tests - Updated the `TestExecutionRouter` tests to include dependency overrides for `ExecutionService`, `WorkflowService`, `AssistantService`, and `ThreadFacade`. This change improves the test setup by allowing for more controlled and isolated testing of execution-related functionalities. - Refactored the test methods to ensure proper dependency management using the FastAPI dependency injection system. --- .../chat/api/test_executions_router.py | 49 ++++++++++++++++++- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/tests/integration/chat/api/test_executions_router.py b/tests/integration/chat/api/test_executions_router.py index ee7956f7..2a089ce3 100644 --- a/tests/integration/chat/api/test_executions_router.py +++ b/tests/integration/chat/api/test_executions_router.py @@ -155,8 +155,24 @@ def test_create_execution_success( self, workspace_id: WorkspaceId, execution_data: dict[str, str], + execution_service: ExecutionService, + workflow_service: WorkflowService, + assistant_service: AssistantService, + thread_facade: ThreadFacade, ) -> None: """Test successful execution creation.""" + from askui.chat.api.assistants.dependencies import get_assistant_service + from askui.chat.api.threads.dependencies import get_thread_facade + from askui.chat.api.workflow_executions.dependencies import ( + get_execution_service, + ) + from askui.chat.api.workflows.dependencies import get_workflow_service + + app.dependency_overrides[get_execution_service] = lambda: execution_service + app.dependency_overrides[get_workflow_service] = lambda: workflow_service + app.dependency_overrides[get_assistant_service] = lambda: assistant_service + app.dependency_overrides[get_thread_facade] = lambda: thread_facade + try: with TestClient(app) as client: response = client.post( @@ -179,9 +195,23 @@ def test_retrieve_execution_success( self, workspace_id: WorkspaceId, execution_data: dict[str, str], + execution_service: ExecutionService, + workflow_service: WorkflowService, + assistant_service: AssistantService, + thread_facade: ThreadFacade, ) -> None: """Test successful execution retrieval.""" - # Create an execution + from askui.chat.api.assistants.dependencies import get_assistant_service + from askui.chat.api.threads.dependencies import get_thread_facade + from askui.chat.api.workflow_executions.dependencies import ( + get_execution_service, + ) + from askui.chat.api.workflows.dependencies import get_workflow_service + + app.dependency_overrides[get_execution_service] = lambda: execution_service + app.dependency_overrides[get_workflow_service] = lambda: workflow_service + app.dependency_overrides[get_assistant_service] = lambda: assistant_service + app.dependency_overrides[get_thread_facade] = lambda: thread_facade try: with TestClient(app) as client: @@ -210,9 +240,24 @@ def test_list_executions_success( self, workspace_id: WorkspaceId, execution_data: dict[str, str], + execution_service: ExecutionService, + workflow_service: WorkflowService, + assistant_service: AssistantService, + thread_facade: ThreadFacade, ) -> None: """Test successful execution listing.""" - # Create an execution + from askui.chat.api.assistants.dependencies import get_assistant_service + from askui.chat.api.threads.dependencies import get_thread_facade + from askui.chat.api.workflow_executions.dependencies import ( + get_execution_service, + ) + from askui.chat.api.workflows.dependencies import get_workflow_service + + app.dependency_overrides[get_execution_service] = lambda: execution_service + app.dependency_overrides[get_workflow_service] = lambda: workflow_service + app.dependency_overrides[get_assistant_service] = lambda: assistant_service + app.dependency_overrides[get_thread_facade] = lambda: thread_facade + try: with TestClient(app) as client: create_response = client.post( From 0c579d1f62a832029e8fdecb320e571cccc87713 Mon Sep 17 00:00:00 2001 From: danyalxahid-askui Date: Fri, 12 Sep 2025 03:49:29 +0200 Subject: [PATCH 19/19] docs(router): enhance `create_workflow` docstring for clarity - Updated the docstring for the `create_workflow` function to provide clearer instructions on how to structure the workflow description. The new guidance emphasizes starting with the end goal followed by a set of sequential instructions. --- src/askui/chat/api/workflows/router.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/askui/chat/api/workflows/router.py b/src/askui/chat/api/workflows/router.py index 34c7d883..441df503 100644 --- a/src/askui/chat/api/workflows/router.py +++ b/src/askui/chat/api/workflows/router.py @@ -46,7 +46,9 @@ def create_workflow( workflow_service: WorkflowService = WorkflowServiceDep, ) -> Workflow: """ - Create a new workflow. + Create a new workflow. The description should start off with the end goal and then + to achieve that goal have a set of clear instructions to be executed in order e.g. + \n1.) open a new tab in the browser \n2.) navigate to a specific URL. Args: askui_workspace: The workspace ID from header