From 9d4d56d3be1fc6b5ee22cfa1f3fd9253c732dfb6 Mon Sep 17 00:00:00 2001 From: Graham Neubig Date: Thu, 8 May 2025 20:23:10 -0400 Subject: [PATCH] Add more extensive typing to server/routes files (#8336) Co-authored-by: openhands --- openhands/server/routes/files.py | 62 +++++++++++++++++++++++------ openhands/server/routes/settings.py | 32 +++++++++++++-- 2 files changed, 79 insertions(+), 15 deletions(-) diff --git a/openhands/server/routes/files.py b/openhands/server/routes/files.py index bf0e5ec324..9dd546cd5c 100644 --- a/openhands/server/routes/files.py +++ b/openhands/server/routes/files.py @@ -42,8 +42,17 @@ from openhands.utils.async_utils import call_sync_from_async app = APIRouter(prefix='/api/conversations/{conversation_id}') -@app.get('/list-files') -async def list_files(request: Request, path: str | None = None) -> dict[str, str]: +@app.get( + '/list-files', + response_model=list[str], + responses={ + 404: {'description': 'Runtime not initialized', 'model': dict}, + 500: {'description': 'Error listing or filtering files', 'model': dict}, + }, +) +async def list_files( + request: Request, path: str | None = None +) -> list[str] | JSONResponse: """List files in the specified path. This function retrieves a list of files from the agent's runtime file store, @@ -84,9 +93,7 @@ async def list_files(request: Request, path: str | None = None) -> dict[str, str file_list = [f for f in file_list if f not in FILES_TO_IGNORE] - async def filter_for_gitignore( - file_list: list[dict[str, Any]], base_path: str - ) -> list[dict[str, str]]: + async def filter_for_gitignore(file_list: list[str], base_path: str) -> list[str]: gitignore_path = os.path.join(base_path, '.gitignore') try: read_action = FileReadAction(gitignore_path) @@ -112,7 +119,20 @@ async def list_files(request: Request, path: str | None = None) -> dict[str, str return file_list -@app.get('/select-file', response_model=None) +# NOTE: We use response_model=None for endpoints that can return multiple response types +# (like FileResponse | JSONResponse). This is because FastAPI's response_model expects a +# Pydantic model, but Starlette response classes like FileResponse are not Pydantic models. +# Instead, we document the possible responses using the 'responses' parameter and maintain +# proper type annotations for mypy. +@app.get( + '/select-file', + response_model=None, + responses={ + 200: {'description': 'File content returned as JSON', 'model': dict[str, str]}, + 500: {'description': 'Error opening file', 'model': dict}, + 415: {'description': 'Unsupported media type', 'model': dict}, + }, +) async def select_file(file: str, request: Request) -> FileResponse | JSONResponse: """Retrieve the content of a specified file. @@ -169,8 +189,15 @@ async def select_file(file: str, request: Request) -> FileResponse | JSONRespons ) -@app.get('/zip-directory') -def zip_current_workspace(request: Request) -> FileResponse: +@app.get( + '/zip-directory', + response_model=None, + responses={ + 200: {'description': 'Zipped workspace returned as FileResponse'}, + 500: {'description': 'Error zipping workspace', 'model': dict}, + }, +) +def zip_current_workspace(request: Request) -> FileResponse | JSONResponse: try: logger.debug('Zipping workspace') runtime: Runtime = request.state.conversation.runtime @@ -197,12 +224,19 @@ def zip_current_workspace(request: Request) -> FileResponse: ) -@app.get('/git/changes') +@app.get( + '/git/changes', + response_model=dict[str, Any], + responses={ + 404: {'description': 'Not a git repository', 'model': dict}, + 500: {'description': 'Error getting changes', 'model': dict}, + }, +) async def git_changes( request: Request, conversation_id: str, user_id: str = Depends(get_user_id), -) -> dict[str, Any]: +) -> dict[str, Any] | JSONResponse: runtime: Runtime = request.state.conversation.runtime conversation_store = await ConversationStoreImpl.get_instance( config, @@ -238,13 +272,17 @@ async def git_changes( ) -@app.get('/git/diff') +@app.get( + '/git/diff', + response_model=dict[str, Any], + responses={500: {'description': 'Error getting diff', 'model': dict}}, +) async def git_diff( request: Request, path: str, conversation_id: str, conversation_store: Any = Depends(get_conversation_store), -) -> dict[str, Any]: +) -> dict[str, Any] | JSONResponse: runtime: Runtime = request.state.conversation.runtime cwd = await get_cwd( diff --git a/openhands/server/routes/settings.py b/openhands/server/routes/settings.py index e9a5e270fc..27020c5cb5 100644 --- a/openhands/server/routes/settings.py +++ b/openhands/server/routes/settings.py @@ -25,7 +25,14 @@ from openhands.server.shared import server_config app = APIRouter(prefix='/api') -@app.get('/settings', response_model=GETSettingsModel) +@app.get( + '/settings', + response_model=GETSettingsModel, + responses={ + 404: {'description': 'Settings not found', 'model': dict}, + 401: {'description': 'Invalid token', 'model': dict}, + }, +) async def load_settings( provider_tokens: PROVIDER_TOKEN_TYPE | None = Depends(get_provider_tokens), settings_store: SettingsStore = Depends(get_user_settings_store), @@ -75,7 +82,15 @@ async def load_settings( ) -@app.post('/reset-settings', response_model=dict[str, str]) +@app.post( + '/reset-settings', + responses={ + 410: { + 'description': 'Reset settings functionality has been removed', + 'model': dict, + } + }, +) async def reset_settings() -> JSONResponse: """ Resets user settings. (Deprecated) @@ -105,7 +120,18 @@ async def store_llm_settings( return settings -@app.post('/settings', response_model=dict[str, str]) +# NOTE: We use response_model=None for endpoints that return JSONResponse directly. +# This is because FastAPI's response_model expects a Pydantic model, but we're returning +# a response object directly. We document the possible responses using the 'responses' +# parameter and maintain proper type annotations for mypy. +@app.post( + '/settings', + response_model=None, + responses={ + 200: {'description': 'Settings stored successfully', 'model': dict}, + 500: {'description': 'Error storing settings', 'model': dict}, + }, +) async def store_settings( settings: Settings, settings_store: SettingsStore = Depends(get_user_settings_store),