From 820c2f82e7b4f8567141fed48bbe6b2ecfb75821 Mon Sep 17 00:00:00 2001 From: Vadim Sofin Date: Tue, 7 Jul 2026 11:43:40 +0400 Subject: [PATCH 1/3] doc: add design doc on conversation mode --- docs/designs/subagents_conversation_mode.md | 217 ++++++++++++++++++++ 1 file changed, 217 insertions(+) create mode 100644 docs/designs/subagents_conversation_mode.md diff --git a/docs/designs/subagents_conversation_mode.md b/docs/designs/subagents_conversation_mode.md new file mode 100644 index 00000000..629fe564 --- /dev/null +++ b/docs/designs/subagents_conversation_mode.md @@ -0,0 +1,217 @@ +# Design: Subagent Conversation Mode and Awareness + +- **Status:** Implemented + +## Problem Statement + +When the orchestrator LLM calls a deployment tool (subagent), it has no way to know whether the subagent is stateless (each call is independent) or maintains conversation history. The operator may configure `propagate_history=True` or `conversation_mode=stateful`, which causes the backend to reconstruct and prepend prior `[user, assistant]` exchanges on each call — but the LLM is unaware of this. This creates two distinct problems. + +### 1. Redundant context injection + +The LLM's universal fallback assumption is that every tool call is stateless. So when it calls a history-aware subagent for the second time — say, to provide an employee ID that the subagent asked for — it re-summarizes the entire prior exchange into `query`: + +> "Previously you asked me to assign badge 'Happy 5th Anniversary' to John Smith and requested an EmployeeID. The ID is 123456, please proceed." + +But because `conversation_mode=stateful`, the backend has already reconstructed and prepended the prior `[user, assistant]` exchange. The subagent now receives the prior context twice: once in the prepended history, once in the new `query`. This causes confusion, wasted tokens, and potentially incorrect behavior. + +The LLM should instead send only the **delta** — the new information that wasn't in the prior exchange: + +> "123456" + +### 2. No conversation thread isolation across multiple subagent instances + +`_extract_tool_history` currently identifies prior exchanges by **tool name only**. When the LLM calls the same tool multiple times in parallel — for example, spinning up two independent research subagents — all prior calls to that tool name pool into a single history. The LLM receives back clarifying questions from both subagents mixed together in tool results, with no way to route its follow-up answers to the correct instance. + +A `session_id` argument lets the LLM tag each call with a thread identifier, enabling `_extract_tool_history` to filter by `(tool_name, session_id)` rather than `tool_name` alone. + +--- + +## Design Goals + +- The LLM must be aware, per tool, whether the subagent has conversation history — so it sends only the delta on follow-up calls. +- `conversation_mode` (operator config) is the authoritative switch for whether the backend propagates history. The LLM does not set this; it is fixed at app-authoring time. +- `session_id` isolates conversation threads when the same tool is invoked multiple times in a session. +- Preserve full backward-compatibility: existing `propagate_history=True` manifests and `stateless` (default) tools are unchanged. + +--- + +## Use Cases + +### UC-1: Single stateful subagent — multi-step task + +**Trigger:** User asks the orchestrator: "Assign badge 'Happy 5th Anniversary' to John Smith." + +**Behavior:** Orchestrator calls `assign_badge` with `query: "Assign badge … to John Smith"` and `session_id: "badge-task-1"`. Subagent responds: "Please provide EmployeeID." Orchestrator calls `get_employee_id` → gets `123456`. Now the LLM knows `assign_badge` is history-aware (from the tool description), so it calls `assign_badge` again with `query: "123456"` and `session_id: "badge-task-1"`. The backend prepends the prior exchange for that session, and the subagent receives the full thread and completes the task. + +**Outcome:** The second call's `query` contains only the delta. The subagent receives the correct full context without duplication. + +### UC-2: Multiple parallel research subagents + +**Trigger:** User asks the orchestrator to research two separate topics simultaneously. The orchestrator calls `research_tool` twice in parallel: first with `session_id: "research-climate"`, second with `session_id: "research-energy"`. + +**Behavior:** Both subagents return clarifying questions. On follow-up, the orchestrator calls each with the same `session_id` used to start that thread. `_extract_tool_history` isolates each session's history by `(tool_name, session_id)`. + +**Outcome:** Each subagent receives only its own prior exchanges, not the other's. The orchestrator can correctly route answers. + +### UC-3: Stateless tool — no change + +**Trigger:** An operator configures a tool without `conversation_mode` (default) or with `conversation_mode: "stateless"`. + +**Behavior:** No `session_id` parameter is injected into the schema. No description suffix is added. `_extract_tool_history` is never called. + +**Outcome:** Identical to today's behavior. No migration required. + +--- + +## Proposed Design + +### `conversation_mode` — two values + +``` +stateless (default) — each call is independent; no history or state is propagated +stateful — backend reconstructs [user, assistant] pairs from the parent's messages and + prepends them on each call via _extract_tool_history; subagent's + TOOL_EXECUTION_HISTORY state is threaded back automatically +``` + +The admin configuring the manifest picks the mode based on what they know about the target DIAL app: `stateful` if it needs prior history sent to it; `stateless` if it handles continuity itself or has no memory. + +**Location:** `ConversationMode` enum in `src/quickapp/config/tools/deployment.py`, field on `ContentPropagation`. + +`propagate_history=True` remains and continues to trigger the same extraction path — it is not deprecated. + +--- + +### LLM awareness via `enrich_openai_tool_schema` + +**What:** For tools with `conversation_mode=stateful`, append a suffix to `function.description` informing the LLM of stateful semantics and delta-only expectations, and inject a `session_id` parameter into the tool schema. + +**Where:** `StagedBaseTool.enrich_openai_tool_schema(open_ai_tool)` — no-op base method, overridden in `BaseDeploymentTool`. Wired into `AgentModule.provide_openai_tools` after `_append_default_props`, before serialization. The ordering is intentional: `_append_default_props` only touches `properties` (adds `query` and `attachment_urls`), never `description`, so `enrich_openai_tool_schema` always appends to the original description rather than to something `_append_default_props` may have written. + +**Description suffix (informative draft — exact wording TBD):** + +> "This tool is a stateful subagent: it maintains conversation history across calls within the same session. On each follow-up call, the backend automatically prepends prior exchanges for that session. **Send only the new information (delta)** in `query` — do not re-summarize prior context. Assign a unique `session_id` at the start of each conversation thread and use the same value for all follow-up calls in that thread." + +--- + +### `session_id` parameter + +**What:** An optional string parameter injected into the tool's JSON schema for all tools where `conversation_mode=stateful`. + +**Where:** Injected by `BaseDeploymentTool.enrich_openai_tool_schema()` alongside the description suffix. + +**Schema shape:** +```json +"session_id": { + "type": "string", + "description": "Identifier for this conversation thread. Assign a unique value when starting a new multi-turn exchange with this subagent (e.g. 'research-climate'). Use the same session_id on all follow-up calls for that thread. Use a different session_id to start an independent conversation thread." +} +``` + +**Who generates `session_id`:** The LLM. The tool description explicitly instructs it to create a unique identifier per thread and use it consistently. The LLM does not need to generate a UUID — any stable string it chooses (e.g., `"badge-task-1"`, `"research-climate"`) is sufficient. Modern instruction-following models reliably maintain such identifiers when clearly instructed. + +**`session_id` is NOT sent to the subagent** — it is popped from kwargs in `_run_in_stage_async` before `DialCompletionService.complete_request_async` is called, the same way `attachment_urls` is consumed at the tool layer. + +**Fallback when `session_id` is omitted:** If the LLM omits `session_id` on a stateful tool call (e.g., an older model that ignores injected parameters), `_extract_tool_history` falls back to `tool_name`-only filtering — all prior calls to that tool are included regardless of session, which is the same behaviour as `propagate_history=True`. + +--- + +### `_extract_tool_history` changes + +Current signature: `_extract_tool_history(tool_name: str)` + +New signature: `_extract_tool_history(tool_name: str, session_id: str | None = None)` + +**First pass** (collecting TOOL results by `tool_call_id`) — unchanged. + +**Second pass** (walking ASSISTANT messages): when `session_id` is provided, only include a tool call if the parsed arguments for that call contain `"session_id": `. Tool calls without a matching `session_id` are excluded when filtering is active. + +When `session_id` is `None`, falls back to filtering by `tool_name` only — preserving current behavior for `propagate_history=True` callers that do not use `session_id`. + +--- + +### `_run_in_stage_async` changes + +At the start of the method, `session_id` is popped from kwargs (consumed here, not forwarded to the subagent). It is then passed as a keyword argument to `_extract_tool_history` when history extraction is triggered by `propagate_history=True` or `conversation_mode=stateful`. + +--- + +## Secondary Fixes + +### Empty content with state silently dropped + +`_extract_tool_history` previously dropped TOOL messages with empty string content even when `custom_content.state` was present, breaking state threading for subagents that return empty text but carry internal tool-execution history. Fixed: + +- First pass: removed `and msg.content` guard; content normalized to `""` for `None`. +- Second pass: guard changed from `if tool_content:` to `if tool_content or tool_custom_content:`. + +--- + +## Out of Scope + +**LLM choosing `conversation_mode`.** Issue #88 proposed `conversation_mode` as a runtime tool argument. This design keeps it as operator config: the app author knows at wiring time whether the subagent is stateful. Making the LLM choose adds prompt-engineering fragility — the LLM must correctly infer statefulness from a description and reliably pick the right mode on every call. + +**`conversation_summary` mode.** Mentioned in issue #88 as a future extension. Deferred until there is a concrete use case and a summary provider is defined. + +**Deprecation of `propagate_history`.** Triggers the same extraction path as `stateful`. Removing it requires a deprecation notice and grace period; deferred. + +--- + +## Configuration / Usage Examples + +### Stateful subagent — multi-step task with session isolation + +```json +{ + "type": "deployment-tool", + "deployment": { "endpoint": "assign-badge-app" }, + "content_propagation": { + "conversation_mode": "stateful" + } +} +``` + +The LLM will see `session_id` in the tool schema and call like: +```json +{ "query": "Assign badge 'Happy 5th Anniversary' to John Smith", "session_id": "badge-task-1" } +``` + +Follow-up (delta only): +```json +{ "query": "123456", "session_id": "badge-task-1" } +``` + +### Stateless (default) + +```json +{ + "type": "deployment-tool", + "deployment": { "endpoint": "search-tool" } +} +``` + +No `session_id` in schema. No description suffix. No history extraction. + +--- + +## Migration + +### Breaking changes + +None. Default is `stateless`; existing manifests without `conversation_mode` behave identically to today. + +### Non-breaking additions + +Default is `stateless`; existing manifests without `conversation_mode` and `propagate_history=True` callers behave identically to before. + +--- + +## Summary of Changes + +| Component | Change | +|---|---| +| `config/tools/deployment.py` | `ConversationMode` enum with `STATELESS` and `STATEFUL`. `ContentPropagation.conversation_mode` documented. | +| `common/staged_base_tool.py` | Add `enrich_openai_tool_schema(open_ai_tool)` extension hook (no-op in base, overridden in `BaseDeploymentTool`). | +| `core/agent/agent_module.py` | `provide_openai_tools`: call `tool.enrich_openai_tool_schema(open_ai_tool)` for every tool after `_append_default_props`. | +| `dial_deployment_tooling/base_deployment_tool.py` | Override `enrich_openai_tool_schema`: inject description suffix and `session_id` parameter when `conversation_mode=STATEFUL`. `_run_in_stage_async`: pop `session_id` from kwargs, pass to `_extract_tool_history`. `_extract_tool_history`: accept `session_id`, filter by `(tool_name, session_id)` when provided. Fix empty-content-with-state edge case. | +| `docs/generated-app-schema.json` | Regenerated after config model changes. | From 4a6caab182bac1c5855713671e2300acb0ed1185 Mon Sep 17 00:00:00 2001 From: Vadim Sofin Date: Tue, 7 Jul 2026 11:44:54 +0400 Subject: [PATCH 2/3] feat: implement stateful subagent conversation --- docs/generated-app-schema.json | 13 + src/quickapp/common/staged_base_tool.py | 4 + src/quickapp/config/tools/deployment.py | 15 + src/quickapp/core/agent/agent_module.py | 1 + .../base_deployment_tool.py | 70 ++++- .../test_base_deployment_tool.py | 293 +++++++++++++++++- 6 files changed, 388 insertions(+), 8 deletions(-) diff --git a/docs/generated-app-schema.json b/docs/generated-app-schema.json index 57bfc526..397f0d43 100644 --- a/docs/generated-app-schema.json +++ b/docs/generated-app-schema.json @@ -451,6 +451,11 @@ }, "ContentPropagation": { "properties": { + "conversation_mode": { + "$ref": "#/$defs/ConversationMode", + "default": "stateless", + "description": "Controls how prior interactions with this deployment are replayed. 'stateless': each call is independent (default). 'stateful': prior [user, assistant] pairs including the subagent's internal tool-execution state are sent, enabling stateful multi-turn subagents." + }, "propagate_history": { "default": false, "description": "Flag to propagate messages to deployment. If True, the messages will be propagated in such way: [assistant, tool] -> [user, assistant].", @@ -515,6 +520,14 @@ "title": "ContinueStrategyModel", "type": "object" }, + "ConversationMode": { + "enum": [ + "stateless", + "stateful" + ], + "title": "ConversationMode", + "type": "string" + }, "ConversationStarter": { "properties": { "title": { diff --git a/src/quickapp/common/staged_base_tool.py b/src/quickapp/common/staged_base_tool.py index ff01c429..cec4ceef 100644 --- a/src/quickapp/common/staged_base_tool.py +++ b/src/quickapp/common/staged_base_tool.py @@ -14,6 +14,7 @@ ) from quickapp.config.application import StageDisplayLevel from quickapp.config.tools.base import BaseTool as _BaseToolConfig +from quickapp.config.tools.base import OpenAiToolConfig from quickapp.config.tools.tool_fallback import RetryStrategyModel from .exceptions import InvalidToolCallParameterException, ToolTimeoutError @@ -149,6 +150,9 @@ async def __run_tool_body( ) return FallbackProcessor.process_fallback(fallback.strategies, tool_call_id, e) + def enrich_openai_tool_schema(self, open_ai_tool: OpenAiToolConfig) -> OpenAiToolConfig: + return open_ai_tool + async def _pre_process_params(self, **kwargs: Any) -> dict[str, Any]: for transformer in self.__argument_transformers: kwargs = await transformer.transform(kwargs) diff --git a/src/quickapp/config/tools/deployment.py b/src/quickapp/config/tools/deployment.py index 9a9856f9..b566ccfc 100644 --- a/src/quickapp/config/tools/deployment.py +++ b/src/quickapp/config/tools/deployment.py @@ -1,3 +1,4 @@ +from enum import Enum from typing import Literal from pydantic import BaseModel, Field @@ -7,7 +8,21 @@ from quickapp.config.tools.base import BaseOpenAITool +class ConversationMode(str, Enum): + STATELESS = "stateless" + STATEFUL = "stateful" + + class ContentPropagation(BaseModel): + conversation_mode: ConversationMode = Field( + default=ConversationMode.STATELESS, + description=( + "Controls how prior interactions with this deployment are replayed. " + "'stateless': each call is independent (default). " + "'stateful': prior [user, assistant] pairs including the subagent's " + "internal tool-execution state are sent, enabling stateful multi-turn subagents." + ), + ) propagate_history: bool = Field( default=False, description="Flag to propagate messages to deployment. If True, the messages will be propagated in such way: [assistant, tool] -> [user, assistant].", diff --git a/src/quickapp/core/agent/agent_module.py b/src/quickapp/core/agent/agent_module.py index a7f69177..1dfad3d9 100644 --- a/src/quickapp/core/agent/agent_module.py +++ b/src/quickapp/core/agent/agent_module.py @@ -171,6 +171,7 @@ def provide_openai_tools( open_ai_tool = self._remove_const_params(open_ai_tool) if isinstance(tool.tool_config, DialDeploymentTool): open_ai_tool = self._append_default_props(open_ai_tool) + open_ai_tool = tool.enrich_openai_tool_schema(open_ai_tool) openai_functions.append(open_ai_tool.model_dump(mode="json", exclude_none=True)) for default_tool in static_tools: openai_functions.append(default_tool.model_dump(mode="json", exclude_none=True)) diff --git a/src/quickapp/dial_deployment_tooling/base_deployment_tool.py b/src/quickapp/dial_deployment_tooling/base_deployment_tool.py index 3fd336c1..e17de2fc 100644 --- a/src/quickapp/dial_deployment_tooling/base_deployment_tool.py +++ b/src/quickapp/dial_deployment_tooling/base_deployment_tool.py @@ -19,7 +19,12 @@ from quickapp.common.perf_timer.perf_timer import PerformanceTimer from quickapp.common.utils import to_plain_dict from quickapp.config.dial_deployment import DialDeploymentParameters -from quickapp.config.tools.deployment import ContentPropagation, DialDeploymentTool +from quickapp.config.tools.base import ConfigurableSchemaSimpleType, JsonTypeEnum, OpenAiToolConfig +from quickapp.config.tools.deployment import ( + ContentPropagation, + ConversationMode, + DialDeploymentTool, +) from quickapp.dial_deployment_tooling._attachment_resolver import AttachmentResolver from quickapp.dial_deployment_tooling.constants import ( ATTACHMENT_PARAM, @@ -71,9 +76,15 @@ async def _run_in_stage_async( **kwargs, ) -> ToolCallResult: tool_config = cast(DialDeploymentTool, self.tool_config) + session_id: str | None = kwargs.pop("session_id", None) history = None - if self.__content_propagation and self.__content_propagation.propagate_history: - history = await self._extract_tool_history(tool_config.open_ai_tool.function.name) + if self.__content_propagation and ( + self.__content_propagation.propagate_history + or self.__content_propagation.conversation_mode == ConversationMode.STATEFUL + ): + history = await self._extract_tool_history( + tool_config.open_ai_tool.function.name, session_id=session_id + ) return await self.__dial_completion_service.complete_request_async( kwargs, self.__application_id, @@ -84,6 +95,37 @@ async def _run_in_stage_async( supports_url_attachments=tool_config.supports_url_attachments, ) + _STATEFUL_DESCRIPTION_SUFFIX = ( + "\n\nThis tool is a stateful subagent: it maintains conversation history across calls " + "within the same session. On each follow-up call, the backend automatically prepends " + "prior exchanges for that session. Send only the new information (delta) in `query` — " + "do not re-summarize prior context. Assign a unique `session_id` at the start of each " + "conversation thread and use the same value for all follow-up calls in that thread." + ) + + _SESSION_ID_PARAM = ConfigurableSchemaSimpleType( + type=JsonTypeEnum.string, + description=( + "Identifier for this conversation thread. Assign a unique value when starting a " + "new multi-turn exchange with this subagent (e.g. 'research-climate'). Use the " + "same session_id on all follow-up calls for that thread. Use a different " + "session_id to start an independent conversation thread." + ), + ) + + def enrich_openai_tool_schema(self, open_ai_tool: OpenAiToolConfig) -> OpenAiToolConfig: + if not ( + self.__content_propagation + and self.__content_propagation.conversation_mode == ConversationMode.STATEFUL + ): + return open_ai_tool + open_ai_tool.function.description = ( + open_ai_tool.function.description or "" + ) + self._STATEFUL_DESCRIPTION_SUFFIX + if "session_id" not in open_ai_tool.function.parameters.properties: + open_ai_tool.function.parameters.properties["session_id"] = self._SESSION_ID_PARAM + return open_ai_tool + @staticmethod def _sdk_attachment_to_param(attachment: SdkAttachment) -> AttachmentParam: return AttachmentParam( @@ -96,6 +138,7 @@ def _sdk_attachment_to_param(attachment: SdkAttachment) -> AttachmentParam: async def _extract_tool_history( self, tool_name: str, + session_id: str | None = None, ) -> list[UserMessageParam | AssistantMessageParam]: if not tool_name: return [] @@ -105,8 +148,9 @@ async def _extract_tool_history( # Build map: tool_call_id -> (content, custom_content) tool_result_by_id: dict[str, tuple[str, CustomContent | None]] = {} for msg in messages: - if msg.role == Role.TOOL and msg.tool_call_id and msg.content: - content = str(msg.content) if not isinstance(msg.content, str) else msg.content + if msg.role == Role.TOOL and msg.tool_call_id: + raw = msg.content + content = "" if raw is None else (str(raw) if not isinstance(raw, str) else raw) tool_result_by_id[msg.tool_call_id] = (content, msg.custom_content) # Walk ASSISTANT messages, find completed tool_calls matching tool_name @@ -119,6 +163,16 @@ async def _extract_tool_history( if tc.function.name != tool_name: continue + # When session_id is provided, only include calls from the same thread. + # Absent session_id falls back to tool_name-only filtering (propagate_history behaviour). + if session_id is not None: + try: + call_args = json.loads(tc.function.arguments) + if call_args.get("session_id") != session_id: + continue + except (json.JSONDecodeError, AttributeError): + continue + result_entry = tool_result_by_id.get(tc.id) if result_entry is None: # Current call — its TOOL result hasn't been appended yet @@ -130,8 +184,10 @@ async def _extract_tool_history( if user_msg is not None: history.append(user_msg) - if tool_content: - assistant_msg = self._build_assistant_message(tool_content, tool_custom_content) + if tool_content or tool_custom_content: + assistant_msg = self._build_assistant_message( + tool_content or "", tool_custom_content + ) history.append(assistant_msg) return history diff --git a/src/tests/unit_tests/dial_deployment_tooling_tests/test_base_deployment_tool.py b/src/tests/unit_tests/dial_deployment_tooling_tests/test_base_deployment_tool.py index acab68b4..9af7a2df 100644 --- a/src/tests/unit_tests/dial_deployment_tooling_tests/test_base_deployment_tool.py +++ b/src/tests/unit_tests/dial_deployment_tooling_tests/test_base_deployment_tool.py @@ -1,3 +1,4 @@ +import copy import json from typing import Any from unittest.mock import AsyncMock, MagicMock @@ -9,6 +10,7 @@ from pydantic import StrictStr from quickapp.common.messages_mixin import MessagesMixin +from quickapp.common.tool_call_result import ToolCallResult from quickapp.config.application import StageDisplayLevel from quickapp.config.dial_deployment import ( CustomFieldsConfig, @@ -22,7 +24,11 @@ OpenAiToolFunction, OpenAiToolFunctionParameters, ) -from quickapp.config.tools.deployment import DialDeploymentTool +from quickapp.config.tools.deployment import ( + ContentPropagation, + ConversationMode, + DialDeploymentTool, +) from quickapp.dial_deployment_tooling.base_deployment_tool import BaseDeploymentTool @@ -37,10 +43,13 @@ def _make_tool_call( name: str, query: str, attachment_urls: list[str] | None = None, + session_id: str | None = None, ) -> ToolCall: args: dict[str, Any] = {"query": query} if attachment_urls is not None: args["attachment_urls"] = attachment_urls + if session_id is not None: + args["session_id"] = session_id return ToolCall( id=tool_call_id, type="function", @@ -455,3 +464,285 @@ async def test_pre_process_params_standard_params_stay_flat(): assert result["temperature"] == 0.7 assert "temperature" not in result.get("custom_fields", {}).get("configuration", {}) assert result["custom_fields"]["configuration"]["size"] == "1024x1024" + + +# --------------------------------------------------------------------------- +# _extract_tool_history: empty content with state +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_extract_preserves_state_with_empty_content(): + """TOOL message with state but empty content still produces an AssistantMessageParam with state.""" + state_data = {"tool_execution_history": [{"role": "assistant", "content": "step 1"}]} + messages: list[Message] = [ + Message(role=Role.USER, content=StrictStr("Hello")), + _make_assistant_with_tool_calls([_make_tool_call("tc1", "my_tool", "go")]), + _make_tool_result("tc1", "", custom_content=CustomContent(state=state_data)), + ] + + tool = _build_tool(messages) + history = await tool._extract_tool_history("my_tool") + + assert len(history) == 2 + assistant_msg = history[1] + assert assistant_msg["role"] == "assistant" + assert assistant_msg["content"] == "" + assert assistant_msg["custom_content"]["state"] == state_data + + +# --------------------------------------------------------------------------- +# conversation_mode: _run_in_stage_async history triggering +# --------------------------------------------------------------------------- + + +def _build_tool_with_propagation( + messages: list[Message], + content_propagation: ContentPropagation | None, + dial_completion_service: Any = None, +) -> BaseDeploymentTool: + """Build a BaseDeploymentTool with a real tool_config and configurable content_propagation.""" + if dial_completion_service is None: + dial_completion_service = AsyncMock() + return BaseDeploymentTool( + application_id="test-app", + application_name="Test App", + tool_config=_make_tool_config(), + content_propagation=content_propagation, + dial_completion_service=dial_completion_service, + attachment_resolver=MagicMock(), + messages_mixin=_make_messages_mixin(messages), + perf_timer=MagicMock(), + stage_wrapper_builder=MagicMock(), + stage_display_level=StageDisplayLevel.INFO, + ) + + +def _make_mock_completion_service() -> Any: + svc = AsyncMock() + svc.complete_request_async.return_value = ToolCallResult( + content="ok", content_type="text/plain" + ) + return svc + + +@pytest.mark.asyncio +async def test_conversation_mode_stateful_triggers_extract(): + """conversation_mode=stateful causes history to be built and passed to the service.""" + svc = _make_mock_completion_service() + tool = _build_tool_with_propagation( + messages=[], + content_propagation=ContentPropagation(conversation_mode=ConversationMode.STATEFUL), + dial_completion_service=svc, + ) + + await tool._run_in_stage_async(stage_wrapper=None, query="hello") + + _, kwargs = svc.complete_request_async.call_args + assert kwargs["history"] is not None + + +@pytest.mark.asyncio +async def test_conversation_mode_stateless_skips_extract(): + """conversation_mode=stateless with propagate_history=False passes history=None.""" + svc = _make_mock_completion_service() + tool = _build_tool_with_propagation( + messages=[], + content_propagation=ContentPropagation( + conversation_mode=ConversationMode.STATELESS, + propagate_history=False, + ), + dial_completion_service=svc, + ) + + await tool._run_in_stage_async(stage_wrapper=None, query="hello") + + _, kwargs = svc.complete_request_async.call_args + assert kwargs["history"] is None + + +@pytest.mark.asyncio +async def test_propagate_history_and_stateful_are_independent(): + """propagate_history=True and conversation_mode=stateful each independently trigger extraction.""" + for propagation in [ + ContentPropagation(propagate_history=True, conversation_mode=ConversationMode.STATELESS), + ContentPropagation(propagate_history=False, conversation_mode=ConversationMode.STATEFUL), + ]: + svc = _make_mock_completion_service() + tool = _build_tool_with_propagation( + messages=[], + content_propagation=propagation, + dial_completion_service=svc, + ) + + await tool._run_in_stage_async(stage_wrapper=None, query="hello") + + _, kwargs = svc.complete_request_async.call_args + assert kwargs["history"] is not None, f"Expected history for {propagation}" + + +# --------------------------------------------------------------------------- +# _extract_tool_history: session_id filtering +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_extract_session_id_filters_to_matching_thread(): + """When session_id is provided, only calls with that session_id are included.""" + messages: list[Message] = [ + Message(role=Role.USER, content=StrictStr("Start")), + _make_assistant_with_tool_calls( + [ + _make_tool_call("tc1", "my_tool", "thread-A step 1", session_id="session-A"), + _make_tool_call("tc2", "my_tool", "thread-B step 1", session_id="session-B"), + ] + ), + _make_tool_result("tc1", "A answer 1"), + _make_tool_result("tc2", "B answer 1"), + _make_assistant_with_tool_calls( + [ + _make_tool_call("tc3", "my_tool", "thread-A step 2", session_id="session-A"), + ] + ), + _make_tool_result("tc3", "A answer 2"), + ] + + tool = _build_tool(messages) + history = await tool._extract_tool_history("my_tool", session_id="session-A") + + assert len(history) == 4 + assert history[0]["content"] == "thread-A step 1" + assert history[1]["content"] == "A answer 1" + assert history[2]["content"] == "thread-A step 2" + assert history[3]["content"] == "A answer 2" + + +@pytest.mark.asyncio +async def test_extract_session_id_excludes_other_threads(): + """Calls with a different session_id are not included.""" + messages: list[Message] = [ + Message(role=Role.USER, content=StrictStr("Start")), + _make_assistant_with_tool_calls( + [ + _make_tool_call("tc1", "my_tool", "thread-A step", session_id="session-A"), + ] + ), + _make_tool_result("tc1", "A answer"), + ] + + tool = _build_tool(messages) + history = await tool._extract_tool_history("my_tool", session_id="session-B") + + assert history == [] + + +@pytest.mark.asyncio +async def test_extract_session_id_none_falls_back_to_tool_name_only(): + """When session_id is None, falls back to tool_name-only filtering (includes all sessions).""" + messages: list[Message] = [ + Message(role=Role.USER, content=StrictStr("Start")), + _make_assistant_with_tool_calls( + [ + _make_tool_call("tc1", "my_tool", "thread-A", session_id="session-A"), + _make_tool_call("tc2", "my_tool", "thread-B", session_id="session-B"), + ] + ), + _make_tool_result("tc1", "A answer"), + _make_tool_result("tc2", "B answer"), + ] + + tool = _build_tool(messages) + history = await tool._extract_tool_history("my_tool", session_id=None) + + assert len(history) == 4 + assert history[0]["content"] == "thread-A" + assert history[1]["content"] == "A answer" + assert history[2]["content"] == "thread-B" + assert history[3]["content"] == "B answer" + + +# --------------------------------------------------------------------------- +# _run_in_stage_async: session_id is consumed, not forwarded to subagent +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_run_in_stage_pops_session_id_before_forwarding(): + """session_id is consumed by _run_in_stage_async and not forwarded to the completion service.""" + svc = _make_mock_completion_service() + tool = _build_tool_with_propagation( + messages=[], + content_propagation=ContentPropagation(conversation_mode=ConversationMode.STATEFUL), + dial_completion_service=svc, + ) + + await tool._run_in_stage_async(stage_wrapper=None, query="hello", session_id="my-thread") + + forwarded_kwargs, _ = svc.complete_request_async.call_args + payload = forwarded_kwargs[0] + assert "session_id" not in payload + assert payload["query"] == "hello" + + +# --------------------------------------------------------------------------- +# enrich_openai_tool_schema +# --------------------------------------------------------------------------- + + +def _build_tool_with_content_propagation( + content_propagation: ContentPropagation | None, +) -> BaseDeploymentTool: + return BaseDeploymentTool( + application_id="test-app", + application_name="Test App", + tool_config=_make_tool_config(), + content_propagation=content_propagation, + dial_completion_service=MagicMock(), + attachment_resolver=MagicMock(), + messages_mixin=_make_messages_mixin([]), + perf_timer=MagicMock(), + stage_wrapper_builder=MagicMock(), + stage_display_level=StageDisplayLevel.INFO, + ) + + +def test_enrich_stateful_injects_session_id_and_description_suffix(): + """For conversation_mode=stateful, enrich_openai_tool_schema injects session_id and appends description.""" + tool = _build_tool_with_content_propagation( + ContentPropagation(conversation_mode=ConversationMode.STATEFUL) + ) + tool_config = _make_tool_config() + open_ai_tool = copy.deepcopy(tool_config.open_ai_tool) + enriched = tool.enrich_openai_tool_schema(open_ai_tool) + + assert "session_id" in enriched.function.parameters.properties + assert enriched.function.description is not None + assert "stateful subagent" in enriched.function.description + + +def test_enrich_stateless_is_noop(): + """For conversation_mode=stateless (default), enrich_openai_tool_schema returns the tool unchanged.""" + tool = _build_tool_with_content_propagation( + ContentPropagation(conversation_mode=ConversationMode.STATELESS) + ) + tool_config = _make_tool_config() + open_ai_tool = copy.deepcopy(tool_config.open_ai_tool) + original_description = open_ai_tool.function.description + original_props = set(open_ai_tool.function.parameters.properties.keys()) + + enriched = tool.enrich_openai_tool_schema(open_ai_tool) + + assert enriched.function.description == original_description + assert set(enriched.function.parameters.properties.keys()) == original_props + + +def test_enrich_no_propagation_is_noop(): + """With no content_propagation, enrich_openai_tool_schema is a no-op.""" + tool = _build_tool_with_content_propagation(None) + tool_config = _make_tool_config() + open_ai_tool = copy.deepcopy(tool_config.open_ai_tool) + original_props = set(open_ai_tool.function.parameters.properties.keys()) + + enriched = tool.enrich_openai_tool_schema(open_ai_tool) + + assert set(enriched.function.parameters.properties.keys()) == original_props From fbe19609920e3689fc101e785f00cb3a454e23c2 Mon Sep 17 00:00:00 2001 From: Vadim Sofin Date: Tue, 14 Jul 2026 13:22:15 +0400 Subject: [PATCH 3/3] fix: handle session_id generation on deployment_tool side --- docs/designs/subagents_conversation_mode.md | 42 +++++---- .../base_deployment_tool.py | 49 ++++++----- .../test_base_deployment_tool.py | 86 +++++++++++++++++-- 3 files changed, 124 insertions(+), 53 deletions(-) diff --git a/docs/designs/subagents_conversation_mode.md b/docs/designs/subagents_conversation_mode.md index 629fe564..8850a2a6 100644 --- a/docs/designs/subagents_conversation_mode.md +++ b/docs/designs/subagents_conversation_mode.md @@ -41,7 +41,7 @@ A `session_id` argument lets the LLM tag each call with a thread identifier, ena **Trigger:** User asks the orchestrator: "Assign badge 'Happy 5th Anniversary' to John Smith." -**Behavior:** Orchestrator calls `assign_badge` with `query: "Assign badge … to John Smith"` and `session_id: "badge-task-1"`. Subagent responds: "Please provide EmployeeID." Orchestrator calls `get_employee_id` → gets `123456`. Now the LLM knows `assign_badge` is history-aware (from the tool description), so it calls `assign_badge` again with `query: "123456"` and `session_id: "badge-task-1"`. The backend prepends the prior exchange for that session, and the subagent receives the full thread and completes the task. +**Behavior:** Orchestrator calls `assign_badge` with `query: "Assign badge … to John Smith"` (no `session_id` — first call). The backend assigns `session_id = tool_call_id` and appends `[session_id: call-abc]` to the response. Subagent responds: "Please provide EmployeeID." Orchestrator calls `get_employee_id` → gets `123456`. Now the LLM echoes the session_id, calling `assign_badge` again with `query: "123456"` and `session_id: "call-abc"`. The backend prepends the prior exchange for that session, and the subagent receives the full thread and completes the task. **Outcome:** The second call's `query` contains only the delta. The subagent receives the correct full context without duplication. @@ -57,7 +57,7 @@ A `session_id` argument lets the LLM tag each call with a thread identifier, ena **Trigger:** An operator configures a tool without `conversation_mode` (default) or with `conversation_mode: "stateless"`. -**Behavior:** No `session_id` parameter is injected into the schema. No description suffix is added. `_extract_tool_history` is never called. +**Behavior:** No `session_id` parameter is injected into the schema. `_extract_tool_history` is never called. **Outcome:** Identical to today's behavior. No migration required. @@ -84,13 +84,9 @@ The admin configuring the manifest picks the mode based on what they know about ### LLM awareness via `enrich_openai_tool_schema` -**What:** For tools with `conversation_mode=stateful`, append a suffix to `function.description` informing the LLM of stateful semantics and delta-only expectations, and inject a `session_id` parameter into the tool schema. +**What:** For tools with `conversation_mode=stateful`, inject a `session_id` parameter into the tool schema. Its description alone is sufficient for the LLM to use it correctly — no change to `function.description` is needed. -**Where:** `StagedBaseTool.enrich_openai_tool_schema(open_ai_tool)` — no-op base method, overridden in `BaseDeploymentTool`. Wired into `AgentModule.provide_openai_tools` after `_append_default_props`, before serialization. The ordering is intentional: `_append_default_props` only touches `properties` (adds `query` and `attachment_urls`), never `description`, so `enrich_openai_tool_schema` always appends to the original description rather than to something `_append_default_props` may have written. - -**Description suffix (informative draft — exact wording TBD):** - -> "This tool is a stateful subagent: it maintains conversation history across calls within the same session. On each follow-up call, the backend automatically prepends prior exchanges for that session. **Send only the new information (delta)** in `query` — do not re-summarize prior context. Assign a unique `session_id` at the start of each conversation thread and use the same value for all follow-up calls in that thread." +**Where:** `StagedBaseTool.enrich_openai_tool_schema(open_ai_tool)` — no-op base method, overridden in `BaseDeploymentTool`. Wired into `AgentModule.provide_openai_tools` after `_append_default_props`, before serialization. --- @@ -98,21 +94,21 @@ The admin configuring the manifest picks the mode based on what they know about **What:** An optional string parameter injected into the tool's JSON schema for all tools where `conversation_mode=stateful`. -**Where:** Injected by `BaseDeploymentTool.enrich_openai_tool_schema()` alongside the description suffix. +**Where:** Injected by `BaseDeploymentTool.enrich_openai_tool_schema()`. **Schema shape:** ```json "session_id": { "type": "string", - "description": "Identifier for this conversation thread. Assign a unique value when starting a new multi-turn exchange with this subagent (e.g. 'research-climate'). Use the same session_id on all follow-up calls for that thread. Use a different session_id to start an independent conversation thread." + "description": "The session identifier returned at the end of a previous response from this tool. Pass it back unchanged to continue that conversation thread. Omit to start a new independent thread." } ``` -**Who generates `session_id`:** The LLM. The tool description explicitly instructs it to create a unique identifier per thread and use it consistently. The LLM does not need to generate a UUID — any stable string it chooses (e.g., `"badge-task-1"`, `"research-climate"`) is sufficient. Modern instruction-following models reliably maintain such identifiers when clearly instructed. +**Who generates `session_id`:** The **backend**. On the first call to a stateful tool (no `session_id` in kwargs), `_run_in_stage_async` assigns `session_id = tool_call_id` — the unique call identifier already provided by the LLM framework. The backend then appends `[session_id: {id}]` to the result content so the LLM can read and echo it on follow-up calls. This is more reliable than asking the LLM to invent an identifier: echoing a value you received is a much simpler task than generating a unique, stable, thread-consistent string. **`session_id` is NOT sent to the subagent** — it is popped from kwargs in `_run_in_stage_async` before `DialCompletionService.complete_request_async` is called, the same way `attachment_urls` is consumed at the tool layer. -**Fallback when `session_id` is omitted:** If the LLM omits `session_id` on a stateful tool call (e.g., an older model that ignores injected parameters), `_extract_tool_history` falls back to `tool_name`-only filtering — all prior calls to that tool are included regardless of session, which is the same behaviour as `propagate_history=True`. +**Fallback when `session_id` is omitted:** If the LLM omits `session_id` on a stateful follow-up call, `_extract_tool_history` falls back to `tool_name`-only filtering — all prior calls to that tool are included regardless of session, which is the same behaviour as `propagate_history=True`. --- @@ -124,15 +120,18 @@ New signature: `_extract_tool_history(tool_name: str, session_id: str | None = N **First pass** (collecting TOOL results by `tool_call_id`) — unchanged. -**Second pass** (walking ASSISTANT messages): when `session_id` is provided, only include a tool call if the parsed arguments for that call contain `"session_id": `. Tool calls without a matching `session_id` are excluded when filtering is active. +**Second pass** (walking ASSISTANT messages): when `session_id` is provided, include a tool call if it matches either condition: -When `session_id` is `None`, falls back to filtering by `tool_name` only — preserving current behavior for `propagate_history=True` callers that do not use `session_id`. +- **Anchor** (`tc.id == session_id`): the first call in the thread, identified by the `tool_call_id` the backend returned as the session identifier. It has no `session_id` in its own arguments. +- **Follow-up** (`args["session_id"] == session_id`): a subsequent call where the LLM echoed the session identifier back. + +When `session_id` is `None`, falls back to filtering by `tool_name` only — preserving current behavior for `propagate_history=True` callers. --- ### `_run_in_stage_async` changes -At the start of the method, `session_id` is popped from kwargs (consumed here, not forwarded to the subagent). It is then passed as a keyword argument to `_extract_tool_history` when history extraction is triggered by `propagate_history=True` or `conversation_mode=stateful`. +`session_id` is popped from kwargs (consumed here, not forwarded to the subagent). If absent and `conversation_mode=stateful` (first call), the backend assigns `session_id = tool_call_id` and appends `[session_id: {id}]` to the result content. The `session_id` is then passed to `_extract_tool_history` when history extraction is triggered by `propagate_history=True` or `conversation_mode=stateful`. --- @@ -171,14 +170,13 @@ At the start of the method, `session_id` is popped from kwargs (consumed here, n } ``` -The LLM will see `session_id` in the tool schema and call like: +First call (no `session_id`): ```json -{ "query": "Assign badge 'Happy 5th Anniversary' to John Smith", "session_id": "badge-task-1" } +{ "query": "Assign badge 'Happy 5th Anniversary' to John Smith" } ``` - -Follow-up (delta only): +Backend appends `[session_id: call-abc123]` to the response. Follow-up (delta only, LLM echoes session_id): ```json -{ "query": "123456", "session_id": "badge-task-1" } +{ "query": "123456", "session_id": "call-abc123" } ``` ### Stateless (default) @@ -190,7 +188,7 @@ Follow-up (delta only): } ``` -No `session_id` in schema. No description suffix. No history extraction. +No `session_id` in schema. No history extraction. --- @@ -213,5 +211,5 @@ Default is `stateless`; existing manifests without `conversation_mode` and `prop | `config/tools/deployment.py` | `ConversationMode` enum with `STATELESS` and `STATEFUL`. `ContentPropagation.conversation_mode` documented. | | `common/staged_base_tool.py` | Add `enrich_openai_tool_schema(open_ai_tool)` extension hook (no-op in base, overridden in `BaseDeploymentTool`). | | `core/agent/agent_module.py` | `provide_openai_tools`: call `tool.enrich_openai_tool_schema(open_ai_tool)` for every tool after `_append_default_props`. | -| `dial_deployment_tooling/base_deployment_tool.py` | Override `enrich_openai_tool_schema`: inject description suffix and `session_id` parameter when `conversation_mode=STATEFUL`. `_run_in_stage_async`: pop `session_id` from kwargs, pass to `_extract_tool_history`. `_extract_tool_history`: accept `session_id`, filter by `(tool_name, session_id)` when provided. Fix empty-content-with-state edge case. | +| `dial_deployment_tooling/base_deployment_tool.py` | Override `enrich_openai_tool_schema`: inject `session_id` parameter when `conversation_mode=STATEFUL`. `_run_in_stage_async`: pop `session_id` from kwargs, generate from `tool_call_id` on first call, inject into result content, pass to `_extract_tool_history`. `_extract_tool_history`: accept `session_id`, filter by anchor (`tc.id`) or follow-up (`args["session_id"]`) when provided. Fix empty-content-with-state edge case. | | `docs/generated-app-schema.json` | Regenerated after config model changes. | diff --git a/src/quickapp/dial_deployment_tooling/base_deployment_tool.py b/src/quickapp/dial_deployment_tooling/base_deployment_tool.py index e17de2fc..7cf01642 100644 --- a/src/quickapp/dial_deployment_tooling/base_deployment_tool.py +++ b/src/quickapp/dial_deployment_tooling/base_deployment_tool.py @@ -77,15 +77,24 @@ async def _run_in_stage_async( ) -> ToolCallResult: tool_config = cast(DialDeploymentTool, self.tool_config) session_id: str | None = kwargs.pop("session_id", None) + is_stateful = ( + self.__content_propagation is not None + and self.__content_propagation.conversation_mode == ConversationMode.STATEFUL + ) + is_first_call = is_stateful and session_id is None + if is_first_call: + session_id = tool_call_id + logger.info("Stateful tool first call — assigned session_id=%s", session_id) + elif is_stateful and session_id: + logger.info("Stateful tool follow-up — session_id=%s", session_id) history = None if self.__content_propagation and ( - self.__content_propagation.propagate_history - or self.__content_propagation.conversation_mode == ConversationMode.STATEFUL + self.__content_propagation.propagate_history or is_stateful ): history = await self._extract_tool_history( tool_config.open_ai_tool.function.name, session_id=session_id ) - return await self.__dial_completion_service.complete_request_async( + result = await self.__dial_completion_service.complete_request_async( kwargs, self.__application_id, self.__application_name, @@ -94,22 +103,16 @@ async def _run_in_stage_async( history=history, supports_url_attachments=tool_config.supports_url_attachments, ) - - _STATEFUL_DESCRIPTION_SUFFIX = ( - "\n\nThis tool is a stateful subagent: it maintains conversation history across calls " - "within the same session. On each follow-up call, the backend automatically prepends " - "prior exchanges for that session. Send only the new information (delta) in `query` — " - "do not re-summarize prior context. Assign a unique `session_id` at the start of each " - "conversation thread and use the same value for all follow-up calls in that thread." - ) + if is_first_call and session_id: + result.content = result.content + f"\n\n[session_id: {session_id}]" + return result _SESSION_ID_PARAM = ConfigurableSchemaSimpleType( type=JsonTypeEnum.string, description=( - "Identifier for this conversation thread. Assign a unique value when starting a " - "new multi-turn exchange with this subagent (e.g. 'research-climate'). Use the " - "same session_id on all follow-up calls for that thread. Use a different " - "session_id to start an independent conversation thread." + "The session identifier returned at the end of a previous response from this tool. " + "Pass it back unchanged to continue that conversation thread. " + "Omit to start a new independent thread." ), ) @@ -119,9 +122,6 @@ def enrich_openai_tool_schema(self, open_ai_tool: OpenAiToolConfig) -> OpenAiToo and self.__content_propagation.conversation_mode == ConversationMode.STATEFUL ): return open_ai_tool - open_ai_tool.function.description = ( - open_ai_tool.function.description or "" - ) + self._STATEFUL_DESCRIPTION_SUFFIX if "session_id" not in open_ai_tool.function.parameters.properties: open_ai_tool.function.parameters.properties["session_id"] = self._SESSION_ID_PARAM return open_ai_tool @@ -163,15 +163,20 @@ async def _extract_tool_history( if tc.function.name != tool_name: continue - # When session_id is provided, only include calls from the same thread. - # Absent session_id falls back to tool_name-only filtering (propagate_history behaviour). + # When session_id is provided, match calls that are either: + # - the thread anchor (tc.id == session_id): the first call, identified by + # the tool_call_id the backend returned to the LLM as the session identifier + # - a follow-up (args["session_id"] == session_id): the LLM echoed it back if session_id is not None: try: call_args = json.loads(tc.function.arguments) - if call_args.get("session_id") != session_id: + is_anchor = tc.id == session_id + is_followup = call_args.get("session_id") == session_id + if not (is_anchor or is_followup): continue except (json.JSONDecodeError, AttributeError): - continue + if tc.id != session_id: + continue result_entry = tool_result_by_id.get(tc.id) if result_entry is None: diff --git a/src/tests/unit_tests/dial_deployment_tooling_tests/test_base_deployment_tool.py b/src/tests/unit_tests/dial_deployment_tooling_tests/test_base_deployment_tool.py index 9af7a2df..ca1ae06b 100644 --- a/src/tests/unit_tests/dial_deployment_tooling_tests/test_base_deployment_tool.py +++ b/src/tests/unit_tests/dial_deployment_tooling_tests/test_base_deployment_tool.py @@ -588,27 +588,29 @@ async def test_propagate_history_and_stateful_are_independent(): @pytest.mark.asyncio async def test_extract_session_id_filters_to_matching_thread(): - """When session_id is provided, only calls with that session_id are included.""" + """Anchor call (tc.id == session_id) and follow-ups (args session_id == session_id) are included; other threads excluded.""" messages: list[Message] = [ Message(role=Role.USER, content=StrictStr("Start")), _make_assistant_with_tool_calls( [ - _make_tool_call("tc1", "my_tool", "thread-A step 1", session_id="session-A"), - _make_tool_call("tc2", "my_tool", "thread-B step 1", session_id="session-B"), + _make_tool_call("tc1", "my_tool", "thread-A step 1"), # anchor for session tc1 + _make_tool_call("tc2", "my_tool", "thread-B step 1"), # anchor for session tc2 ] ), _make_tool_result("tc1", "A answer 1"), _make_tool_result("tc2", "B answer 1"), _make_assistant_with_tool_calls( [ - _make_tool_call("tc3", "my_tool", "thread-A step 2", session_id="session-A"), + _make_tool_call( + "tc3", "my_tool", "thread-A step 2", session_id="tc1" + ), # follow-up to A ] ), _make_tool_result("tc3", "A answer 2"), ] tool = _build_tool(messages) - history = await tool._extract_tool_history("my_tool", session_id="session-A") + history = await tool._extract_tool_history("my_tool", session_id="tc1") assert len(history) == 4 assert history[0]["content"] == "thread-A step 1" @@ -617,6 +619,23 @@ async def test_extract_session_id_filters_to_matching_thread(): assert history[3]["content"] == "A answer 2" +@pytest.mark.asyncio +async def test_extract_anchor_matches_by_tool_call_id(): + """First call matched by tc.id == session_id (the anchor), even without session_id in its args.""" + messages: list[Message] = [ + Message(role=Role.USER, content=StrictStr("Start")), + _make_assistant_with_tool_calls([_make_tool_call("call-abc", "my_tool", "first question")]), + _make_tool_result("call-abc", "first answer"), + ] + + tool = _build_tool(messages) + history = await tool._extract_tool_history("my_tool", session_id="call-abc") + + assert len(history) == 2 + assert history[0]["content"] == "first question" + assert history[1]["content"] == "first answer" + + @pytest.mark.asyncio async def test_extract_session_id_excludes_other_threads(): """Calls with a different session_id are not included.""" @@ -684,6 +703,57 @@ async def test_run_in_stage_pops_session_id_before_forwarding(): assert payload["query"] == "hello" +@pytest.mark.asyncio +async def test_run_in_stage_injects_session_id_into_first_result(): + """First stateful call (no session_id) appends session_id from tool_call_id to result content.""" + svc = _make_mock_completion_service() + tool = _build_tool_with_propagation( + messages=[], + content_propagation=ContentPropagation(conversation_mode=ConversationMode.STATEFUL), + dial_completion_service=svc, + ) + + result = await tool._run_in_stage_async( + stage_wrapper=None, tool_call_id="call-xyz", query="hello" + ) + + assert "[session_id: call-xyz]" in result.content + + +@pytest.mark.asyncio +async def test_run_in_stage_no_injection_on_follow_up(): + """Follow-up call (session_id already provided) does not modify result content.""" + svc = _make_mock_completion_service() + tool = _build_tool_with_propagation( + messages=[], + content_propagation=ContentPropagation(conversation_mode=ConversationMode.STATEFUL), + dial_completion_service=svc, + ) + + result = await tool._run_in_stage_async( + stage_wrapper=None, tool_call_id="call-xyz2", query="hello", session_id="call-xyz" + ) + + assert result.content == "ok" + + +@pytest.mark.asyncio +async def test_run_in_stage_no_injection_for_stateless(): + """Stateless tool calls never inject session_id, even when tool_call_id is present.""" + svc = _make_mock_completion_service() + tool = _build_tool_with_propagation( + messages=[], + content_propagation=ContentPropagation(conversation_mode=ConversationMode.STATELESS), + dial_completion_service=svc, + ) + + result = await tool._run_in_stage_async( + stage_wrapper=None, tool_call_id="call-xyz", query="hello" + ) + + assert result.content == "ok" + + # --------------------------------------------------------------------------- # enrich_openai_tool_schema # --------------------------------------------------------------------------- @@ -706,8 +776,8 @@ def _build_tool_with_content_propagation( ) -def test_enrich_stateful_injects_session_id_and_description_suffix(): - """For conversation_mode=stateful, enrich_openai_tool_schema injects session_id and appends description.""" +def test_enrich_stateful_injects_session_id(): + """For conversation_mode=stateful, enrich_openai_tool_schema injects session_id into the schema.""" tool = _build_tool_with_content_propagation( ContentPropagation(conversation_mode=ConversationMode.STATEFUL) ) @@ -716,8 +786,6 @@ def test_enrich_stateful_injects_session_id_and_description_suffix(): enriched = tool.enrich_openai_tool_schema(open_ai_tool) assert "session_id" in enriched.function.parameters.properties - assert enriched.function.description is not None - assert "stateful subagent" in enriched.function.description def test_enrich_stateless_is_noop():