From 63565982aa1f94a5cd1e468a705777848634357c Mon Sep 17 00:00:00 2001 From: Cheng Yang <93481273+young010101@users.noreply.github.com> Date: Sat, 15 Feb 2025 09:51:59 +0800 Subject: [PATCH] docs: improve docstrings for CLI and config utils (#5398) Co-authored-by: Engel Nyst --- docs/modules/usage/architecture/runtime.md | 6 +-- openhands/README.md | 3 ++ openhands/core/cli.py | 2 +- openhands/core/config/utils.py | 18 +++++---- openhands/core/main.py | 21 ++++++++++ openhands/core/message.py | 4 +- openhands/llm/llm.py | 4 +- .../runtime/impl/docker/docker_runtime.py | 13 +++--- .../plugins/agent_skills/file_ops/file_ops.py | 40 +++++++++++++------ openhands/runtime/utils/runtime_build.py | 4 +- openhands/server/routes/public.py | 3 +- tests/unit/test_micro_agents.py | 2 +- 12 files changed, 82 insertions(+), 38 deletions(-) diff --git a/docs/modules/usage/architecture/runtime.md b/docs/modules/usage/architecture/runtime.md index b08a1ed99b..f682f249ac 100644 --- a/docs/modules/usage/architecture/runtime.md +++ b/docs/modules/usage/architecture/runtime.md @@ -54,14 +54,13 @@ graph TD 6. Action Execution: The runtime client receives actions from the backend, executes them in the sandboxed environment, and sends back observations 7. Observation Return: The action execution server sends execution results back to the OpenHands backend as observations - The role of the client: + - It acts as an intermediary between the OpenHands backend and the sandboxed environment - It executes various types of actions (shell commands, file operations, Python code, etc.) safely within the container - It manages the state of the sandboxed environment, including the current working directory and loaded plugins - It formats and returns observations to the backend, ensuring a consistent interface for processing results - ## How OpenHands builds and maintains OH Runtime images OpenHands' approach to building and managing runtime images ensures efficiency, consistency, and flexibility in creating and maintaining Docker images for both production and development environments. @@ -78,16 +77,15 @@ Tags may be in one of 2 formats: - **Source Tag**: `oh_v{openhands_version}_{16_digit_lock_hash}_{16_digit_source_hash}` (e.g.: `oh_v0.9.9_1234567890abcdef_1234567890abcdef`) - #### Source Tag - Most Specific This is the first 16 digits of the MD5 of the directory hash for the source directory. This gives a hash for only the openhands source - #### Lock Tag This hash is built from the first 16 digits of the MD5 of: + - The name of the base image upon which the image was built (e.g.: `nikolaik/python-nodejs:python3.12-nodejs22`) - The content of the `pyproject.toml` included in the image. - The content of the `poetry.lock` included in the image. diff --git a/openhands/README.md b/openhands/README.md index 4c6a67f097..f43f9ae379 100644 --- a/openhands/README.md +++ b/openhands/README.md @@ -6,6 +6,7 @@ This diagram provides an overview of the roles of each component and how they co ![OpenHands System Architecture Diagram (July 4, 2024)](../docs/static/img/system_architecture_overview.png) ## Classes + The key classes in OpenHands are: * LLM: brokers all interactions with large language models. Works with any underlying completion model, thanks to LiteLLM. @@ -23,7 +24,9 @@ The key classes in OpenHands are: * ConversationManager: keeps a list of active sessions, and ensures requests are routed to the correct Session ## Control Flow + Here's the basic loop (in pseudocode) that drives agents. + ```python while True: prompt = agent.generate_prompt(state) diff --git a/openhands/core/cli.py b/openhands/core/cli.py index f1f687fed5..1e31537155 100644 --- a/openhands/core/cli.py +++ b/openhands/core/cli.py @@ -99,7 +99,7 @@ def read_input(config: AppConfig) -> str: async def main(loop: asyncio.AbstractEventLoop): - """Runs the agent in CLI mode""" + """Runs the agent in CLI mode.""" args = parse_arguments() diff --git a/openhands/core/config/utils.py b/openhands/core/config/utils.py index e0b1ee71ad..f057eb6ad2 100644 --- a/openhands/core/config/utils.py +++ b/openhands/core/config/utils.py @@ -29,9 +29,14 @@ JWT_SECRET = '.jwt_secret' load_dotenv() -def load_from_env(cfg: AppConfig, env_or_toml_dict: dict | MutableMapping[str, str]): - """Reads the env-style vars and sets config attributes based on env vars or a config.toml dict. - Compatibility with vars like LLM_BASE_URL, AGENT_MEMORY_ENABLED, SANDBOX_TIMEOUT and others. +def load_from_env( + cfg: AppConfig, env_or_toml_dict: dict | MutableMapping[str, str] +) -> None: + """Sets config attributes from environment variables or TOML dictionary. + + Reads environment-style variables and updates the config attributes accordingly. + Supports configuration of LLM settings (e.g., LLM_BASE_URL), agent settings + (e.g., AGENT_MEMORY_ENABLED), sandbox settings (e.g., SANDBOX_TIMEOUT), and more. Args: cfg: The AppConfig object to set attributes on. @@ -44,7 +49,7 @@ def load_from_env(cfg: AppConfig, env_or_toml_dict: dict | MutableMapping[str, s return next((t for t in types if t is not type(None)), None) # helper function to set attributes based on env vars - def set_attr_from_env(sub_config: BaseModel, prefix=''): + def set_attr_from_env(sub_config: BaseModel, prefix='') -> None: """Set attributes of a config model based on environment variables.""" for field_name, field_info in sub_config.model_fields.items(): field_value = getattr(sub_config, field_name) @@ -95,7 +100,7 @@ def load_from_env(cfg: AppConfig, env_or_toml_dict: dict | MutableMapping[str, s set_attr_from_env(default_agent_config, 'AGENT_') -def load_from_toml(cfg: AppConfig, toml_file: str = 'config.toml'): +def load_from_toml(cfg: AppConfig, toml_file: str = 'config.toml') -> None: """Load the config from the toml file. Supports both styles of config vars. Args: @@ -103,8 +108,7 @@ def load_from_toml(cfg: AppConfig, toml_file: str = 'config.toml'): toml_file: The path to the toml file. Defaults to 'config.toml'. See Also: - - `config.template.toml` for the full list of config options. - - `SandboxConfig` for the sandbox-specific config options. + - config.template.toml for the full list of config options. """ # try to read the config.toml file into the config object try: diff --git a/openhands/core/main.py b/openhands/core/main.py index 474757d9c7..2652931cce 100644 --- a/openhands/core/main.py +++ b/openhands/core/main.py @@ -78,6 +78,7 @@ async def run_controller( headless_mode: bool = True, ) -> State | None: """Main coroutine to run the agent controller with task input flexibility. + It's only used when you launch openhands backend directly via cmdline. Args: @@ -91,6 +92,26 @@ async def run_controller( fake_user_response_fn: An optional function that receives the current state (could be None) and returns a fake user response. headless_mode: Whether the agent is run in headless mode. + + Returns: + The final state of the agent, or None if an error occurred. + + Raises: + AssertionError: If initial_user_action is not an Action instance. + Exception: Various exceptions may be raised during execution and will be logged. + + Notes: + - State persistence: If config.file_store is set, the agent's state will be + saved between sessions. + - Trajectories: If config.trajectories_path is set, execution history will be + saved as JSON for analysis. + - Budget control: Execution is limited by config.max_iterations and + config.max_budget_per_task. + + Example: + >>> config = load_app_config() + >>> action = MessageAction(content="Write a hello world program") + >>> state = await run_controller(config=config, initial_user_action=action) """ sid = sid or generate_sid(config) diff --git a/openhands/core/message.py b/openhands/core/message.py index ea4f0106ab..b508142242 100644 --- a/openhands/core/message.py +++ b/openhands/core/message.py @@ -122,8 +122,8 @@ class Message(BaseModel): def _add_tool_call_keys(self, message_dict: dict) -> dict: """Add tool call keys if we have a tool call or response. - NOTE: this is necessary for both native and non-native tool calling.""" - + NOTE: this is necessary for both native and non-native tool calling + """ # an assistant message calling a tool if self.tool_calls is not None: message_dict['tool_calls'] = [ diff --git a/openhands/llm/llm.py b/openhands/llm/llm.py index e8dd2f6f1e..b5fe679434 100644 --- a/openhands/llm/llm.py +++ b/openhands/llm/llm.py @@ -605,7 +605,9 @@ class LLM(RetryMixin, DebugMixin): return False def _completion_cost(self, response) -> float: - """Calculate the cost of a completion response based on the model. Local models are treated as free. + """Calculate completion cost and update metrics with running total. + + Calculate the cost of a completion response based on the model. Local models are treated as free. Add the current cost into total cost in metrics. Args: diff --git a/openhands/runtime/impl/docker/docker_runtime.py b/openhands/runtime/impl/docker/docker_runtime.py index 4312f3b6e6..b2c5e98022 100644 --- a/openhands/runtime/impl/docker/docker_runtime.py +++ b/openhands/runtime/impl/docker/docker_runtime.py @@ -39,6 +39,7 @@ APP_PORT_RANGE_2 = (55000, 59999) class DockerRuntime(ActionExecutionClient): """This runtime will subscribe the event stream. + When receive an event, it will send the event to runtime-client which run inside the docker environment. Args: @@ -405,11 +406,11 @@ class DockerRuntime(ActionExecutionClient): """Pause the runtime by stopping the container. This is different from container.stop() as it ensures environment variables are properly preserved.""" if not self.container: - raise RuntimeError("Container not initialized") - + raise RuntimeError('Container not initialized') + # First, ensure all environment variables are properly persisted in .bashrc # This is already handled by add_env_vars in base.py - + # Stop the container self.container.stop() self.log('debug', f'Container {self.container_name} paused') @@ -418,12 +419,12 @@ class DockerRuntime(ActionExecutionClient): """Resume the runtime by starting the container. This is different from container.start() as it ensures environment variables are properly restored.""" if not self.container: - raise RuntimeError("Container not initialized") - + raise RuntimeError('Container not initialized') + # Start the container self.container.start() self.log('debug', f'Container {self.container_name} resumed') - + # Wait for the container to be ready self._wait_until_alive() diff --git a/openhands/runtime/plugins/agent_skills/file_ops/file_ops.py b/openhands/runtime/plugins/agent_skills/file_ops/file_ops.py index b2e1b4c8aa..47451c2985 100644 --- a/openhands/runtime/plugins/agent_skills/file_ops/file_ops.py +++ b/openhands/runtime/plugins/agent_skills/file_ops/file_ops.py @@ -1,6 +1,8 @@ -"""file_ops.py +"""File operations module for OpenHands agent. -This module provides various file manipulation skills for the OpenHands agent. +This module provides a collection of file manipulation skills that enable the OpenHands +agent to perform various file operations such as opening, searching, and navigating +through files and directories. Functions: - open_file(path: str, line_number: int | None = 1, context_lines: int = 100): Opens a file and optionally moves to a specific line. @@ -10,6 +12,9 @@ Functions: - search_dir(search_term: str, dir_path: str = './'): Searches for a term in all files in the specified directory. - search_file(search_term: str, file_path: str | None = None): Searches for a term in the specified file or the currently open file. - find_file(file_name: str, dir_path: str = './'): Finds all files with the given name in the specified directory. + +Note: + All functions return string representations of their results. """ import os @@ -81,11 +86,18 @@ def _clamp(value, min_value, max_value): def _lint_file(file_path: str) -> tuple[str | None, int | None]: - """Lint the file at the given path and return a tuple with a boolean indicating if there are errors, + """Perform linting on a file and identify the first error location. + + Lint the file at the given path and return a tuple with a boolean indicating if there are errors, and the line number of the first error, if any. + Args: + file_path: str: The path to the file to lint. + Returns: - tuple[str | None, int | None]: (lint_error, first_error_line_number) + A tuple containing: + - The lint error message if found, None otherwise + - The line number of the first error, None if no errors """ linter = DefaultLinter() lint_error: list[LintResult] = linter.lint(file_path) @@ -165,14 +177,18 @@ def _cur_file_header(current_file, total_lines) -> str: def open_file( path: str, line_number: int | None = 1, context_lines: int | None = WINDOW ) -> None: - """Opens the file at the given path in the editor. IF the file is to be edited, first use `scroll_down` repeatedly to read the full file! - If line_number is provided, the window will be moved to include that line. - It only shows the first 100 lines by default! `context_lines` is the max number of lines to be displayed, up to 100. Use `scroll_up` and `scroll_down` to view more content up or down. + """Opens a file in the editor and optionally positions at a specific line. + + The function displays a limited window of content, centered around the specified line + number if provided. To view the complete file content, the agent should use scroll_down and scroll_up + commands iteratively. Args: - path: str: The path to the file to open, preferred absolute path. - line_number: int | None = 1: The line number to move to. Defaults to 1. - context_lines: int | None = 100: Only shows this number of lines in the context window (usually from line 1), with line_number as the center (if possible). Defaults to 100. + path: The path to the file to open. Absolute path is recommended. + line_number: The target line number to center the view on (if possible). + Defaults to 1. + context_lines: Maximum number of lines to display in the view window. + Limited to 100 lines. Defaults to 100. """ global CURRENT_FILE, CURRENT_LINE, WINDOW @@ -316,8 +332,8 @@ def search_file(search_term: str, file_path: str | None = None) -> None: """Searches for search_term in file. If file is not provided, searches in the current open file. Args: - search_term: str: The term to search for. - file_path: str | None: The path to the file to search. + search_term: The term to search for. + file_path: The path to the file to search. """ global CURRENT_FILE if file_path is None: diff --git a/openhands/runtime/utils/runtime_build.py b/openhands/runtime/utils/runtime_build.py index bbb83ac7f9..862ce04d7a 100644 --- a/openhands/runtime/utils/runtime_build.py +++ b/openhands/runtime/utils/runtime_build.py @@ -69,7 +69,6 @@ def get_runtime_image_repo_and_tag(base_image: str) -> tuple[str, str]: Returns: - tuple[str, str]: The Docker repo and tag of the Docker image """ - if get_runtime_image_repo() in base_image: logger.debug( f'The provided image [{base_image}] is already a valid runtime image.\n' @@ -115,6 +114,7 @@ def build_runtime_image( extra_build_args: List[str] | None = None, ) -> str: """Prepares the final docker build folder. + If dry_run is False, it will also build the OpenHands runtime Docker image using the docker build folder. Parameters: @@ -349,7 +349,7 @@ def _build_sandbox_image( platform: str | None = None, extra_build_args: List[str] | None = None, ): - """Build and tag the sandbox image. The image will be tagged with all tags that do not yet exist""" + """Build and tag the sandbox image. The image will be tagged with all tags that do not yet exist.""" names = [ f'{runtime_image_repo}:{source_tag}', f'{runtime_image_repo}:{lock_tag}', diff --git a/openhands/server/routes/public.py b/openhands/server/routes/public.py index a5c861a62e..59e5c4e4ef 100644 --- a/openhands/server/routes/public.py +++ b/openhands/server/routes/public.py @@ -23,8 +23,7 @@ app = APIRouter(prefix='/api/options') @app.get('/models') async def get_litellm_models() -> list[str]: - """ - Get all models supported by LiteLLM. + """Get all models supported by LiteLLM. This function combines models from litellm and Bedrock, removing any error-prone Bedrock models. diff --git a/tests/unit/test_micro_agents.py b/tests/unit/test_micro_agents.py index c7461bbda2..7f78df16b1 100644 --- a/tests/unit/test_micro_agents.py +++ b/tests/unit/test_micro_agents.py @@ -53,7 +53,7 @@ def test_all_agents_are_loaded(): def test_coder_agent_with_summary(event_stream: EventStream, agent_configs: dict): - """Coder agent should render code summary as part of prompt""" + """Coder agent should render code summary as part of prompt.""" mock_llm = MagicMock() content = json.dumps({'action': 'finish', 'args': {}}) mock_llm.completion.return_value = {'choices': [{'message': {'content': content}}]}