feat: add openai responses provider#8157
Conversation
…gement tools - Add SubAgentInstance DB model with versioned CRUD and cleanup - Add SubAgentRuntimeManager for preset normalization, scope resolution, instance CRUD, locks, begin-dialog injection, history pruning - Split orchestrator: runtime_mode=handoff keeps HandoffTool, runtime_mode=persistent uses new runtime manager - Wire lifecycle initialization, Context, and cleanup callbacks - Add 7 builtin runtime management tools (list/create/list-instances/ run/update/reset/delete) injected only into main agent - Implement persistent run_subagent execution via tool-loop helper - Enforce image URL normalization, execution error handling, token-based pruning, invalid-field gating on update - Add dashboard Vue form fields for runtime_mode and skills with active-only skills API query param and i18n - Add comprehensive unit and dashboard tests
feat: persistent subagent runtime and dedicated skill tool
…cing in PersonaForm
There was a problem hiding this comment.
Sorry @mapleluvr, your pull request is larger than the review limit of 150000 diff characters
There was a problem hiding this comment.
Code Review
This pull request introduces a persistent subagent runtime and an agent group collaboration system, enabling long-lived conversation contexts and multi-agent workflows. Key additions include a dedicated skill tool for runtime-aware instruction loading, management tools for agent groups, and support for the OpenAI Responses API. Feedback focuses on performance and robustness: specifically, implementing chunked reading for file hashing to prevent OOM errors, adding null checks in API response parsing, replacing expensive deep copies with shallow copies in payload preparation, and improving the efficiency of the token-based history pruning logic to avoid quadratic complexity.
| "mtime_ns": stat.st_mtime_ns, | ||
| "size": stat.st_size, | ||
| } | ||
| digest = hashlib.sha256(path.read_bytes()).hexdigest() |
There was a problem hiding this comment.
Reading the entire file into memory with read_bytes() to calculate a hash can lead to high memory consumption or Out-Of-Memory (OOM) errors if the workspace contains large files. It is safer to read the file in chunks. Additionally, ensure that this functionality is accompanied by corresponding unit tests.
sha256_hash = hashlib.sha256()
with path.open("rb") as f:
for byte_block in iter(lambda: f.read(8192), b""):
sha256_hash.update(byte_block)
digest = sha256_hash.hexdigest()References
- New functionality, such as handling attachments, should be accompanied by corresponding unit tests.
| def _get_response_field(item: Any, field: str, default: Any = None) -> Any: | ||
| if isinstance(item, dict): | ||
| return item.get(field, default) | ||
| return getattr(item, field, default) |
There was a problem hiding this comment.
The _get_response_field method does not check if item is None before calling getattr(). This will cause an AttributeError if the input item is None, which is a likely scenario when parsing nested optional fields from API responses (e.g., usage.input_tokens_details might be missing in some responses).
| def _get_response_field(item: Any, field: str, default: Any = None) -> Any: | |
| if isinstance(item, dict): | |
| return item.get(field, default) | |
| return getattr(item, field, default) | |
| @staticmethod | |
| def _get_response_field(item: Any, field: str, default: Any = None) -> Any: | |
| if item is None: | |
| return default | |
| if isinstance(item, dict): | |
| return item.get(field, default) | |
| return getattr(item, field, default) |
| return responses_tool | ||
|
|
||
| def _convert_chat_payload_to_responses_payload(self, payloads: dict) -> dict: | ||
| payloads = copy.deepcopy(payloads) |
There was a problem hiding this comment.
Using copy.deepcopy(payloads) on every request can be very expensive in terms of CPU and memory, especially when the payload contains large base64-encoded images for vision tasks. Since _prepare_chat_payload typically returns a fresh dictionary, a shallow copy (payloads.copy()) should be sufficient and much more efficient for the subsequent pop operations. Additionally, ensure that this new attachment-related functionality is accompanied by corresponding unit tests.
| payloads = copy.deepcopy(payloads) | |
| payloads = payloads.copy() |
References
- New functionality, such as handling attachments, should be accompanied by corresponding unit tests.
| > max_persisted_tokens | ||
| ): | ||
| non_system_messages = non_system_messages[2:] | ||
| first_user_index = next( | ||
| ( |
There was a problem hiding this comment.
This loop performs token counting on the entire message history in every iteration while pruning. This results in max_persisted_turns is configured to a large value or if messages are very long. Consider estimating the number of tokens to remove or using a more efficient pruning strategy.
Summary
Test Plan
uv run pytest tests/test_openai_source.py -q -k "openai_responses or responses"uv run pytest tests/test_dashboard.py::test_provider_templates_include_openai_responses_type -qnode --test tests/providerUtils.test.mjsfromdashboard/uv run ruff check astrbot/core/provider/manager.py astrbot/core/provider/sources/openai_source.py tests/test_openai_source.py