Python: [BREAKING] Require committed finish reason for tool calls

docs/decisions/0041-authorize-function-calls-with-finish-reason.md

@@ -0,0 +1,78 @@
+---
+status: proposed
+contact: eavanvalkenburg
+date: 2026-09-11
+deciders: eavanvalkenburg
+---
+
+# Authorize function calls with the finish reason
+
+## Context and Problem Statement
+
+Streaming providers expose function-call content before they commit the complete model turn. Agent Framework already
+waits for the provider stream to finish before local invocation, but iterator completion alone does not prove that the
+provider emitted its terminal event or committed every call. A syntactically complete call fragment can therefore be
+mistaken for an executable request after an interrupted or failed response.
+
+## Decision Drivers
+
+- Execute local side effects only after an authoritative provider completion signal.
+- Keep one provider-neutral authorization rule for streaming and non-streaming responses.
+- Preserve incremental function-call updates for callers without adding buffering or another stream drain.
+- Avoid adding a second response lifecycle field or changing the meaning of `informational_only`.
+- Keep provider-specific protocol evidence in the provider adapter.
+
+## Considered Options
+
+### Add a response completion field
+
+- Good: separates lifecycle state from generation finish reason.
+- Bad: adds a public concept that overlaps existing finish-reason behavior.
+- Bad: requires every response and update mapping to carry another field.
+
+### Mark speculative calls as informational
+
+- Good: reuses the existing core execution filter.
+- Bad: `informational_only` describes calls the framework does not own, not uncommitted local calls.
+- Bad: changes the meaning of caller-visible function-call content.
+
+### Infer commitment from content or stream exhaustion
+
+- Good: requires no provider changes.
+- Bad: complete JSON and clean iterator exhaustion do not prove that the provider committed the turn.
+- Bad: different providers expose different authoritative terminal events.
+
+### Authorize with `finish_reason == "tool_calls"`
+
+- Good: uses the existing provider-neutral response field.
+- Good: gives core one fail-closed rule for streaming and non-streaming responses.
+- Good: lets providers continue yielding speculative content immediately and resolve the reason on their existing
+  terminal event.
+- Bad: custom clients that return local function calls without `finish_reason="tool_calls"` must be updated.
+- Bad: provider adapters must assign `"tool_calls"` only after their own protocol commitment evidence is satisfied.
+
+## Decision Outcome
+
+Chosen option: "Authorize with `finish_reason == "tool_calls"`", because it establishes one explicit execution
+boundary without another public lifecycle model.
+
+Core approves or executes model-issued local calls only when the finalized response has
+`finish_reason == "tool_calls"`. Missing and all other finish reasons are non-authorizing.
+
+Each provider adapter owns its commitment evidence. Adapters that require correction use the same small local
+`_resolve_finish_reason(...)` pattern: committed calls override the ordinary reason to `"tool_calls"`; calls without
+commitment cannot retain or manufacture `"tool_calls"`; responses without calls preserve their ordinary reason. The
+function remains local to each provider module because the evidence is protocol-specific and does not justify a shared
+base class, mixin, registry, or cross-provider helper.
+
+Streaming adapters resolve the finish reason on the provider's existing terminal event. They do not add another drain,
+buffer speculative content, or wait after iterator exhaustion. Calls returned without authorization remain visible to
+the caller, while every assistant message from that uncommitted provider turn is excluded from later model-bound replay.
+
+### Consequences
+
+- Local tool bodies, approval requests, function middleware, function results, and follow-up model calls require an
+  authoritatively committed `"tool_calls"` response.
+- Custom clients must set `finish_reason="tool_calls"` for completed local function-call turns.
+- Provider adapters must not infer commitment from parseable arguments, observed call content, or iterator exhaustion.
+- Interrupted call-bearing responses remain inspectable without becoming executable or replayable model history.

docs/specs/004-python-function-calling-loop.md

@@ -128,14 +128,42 @@ Code-reading landmarks:
 - `_get_response_with_function_invocation(...)` owns non-streaming aggregation.
 - `_stream_response_with_function_invocation(...)` owns streamed emission/finalization.
 - `_resolve_approval_responses(...)` handles only inbound approval decisions.
-- `_process_model_function_calls(...)` handles only calls from a completed model response.
+- `_process_model_function_calls(...)` handles only calls from a provider-committed model response whose
+  `finish_reason` is `"tool_calls"`.
 - `_try_execute_function_calls(...)` decides approval/declaration/execution behavior for a batch.
 - `_replace_approval_contents_with_results(...)` is the occurrence-aware approval transcript normalizer.
 - `FunctionInvocationLayer._update_function_invocation_continuation_state(...)` updates continuation state after
   every service response. Provider layers may override it to carry provider-specific continuation metadata into
   the next service call, but must delegate to the base implementation so generic conversation continuation remains
   synchronized with the active `AgentSession`.
 
+### Function-call commitment boundary
+
+`finish_reason == "tool_calls"` is the provider-neutral authorization signal for local function invocation. A
+call-bearing response with a missing or different finish reason is caller-visible but must not create a local approval
+request, enter function middleware, execute a tool body, create a function result, or trigger another model call.
+
+Provider adapters may set `"tool_calls"` only after the provider's authoritative terminal evidence commits every
+actionable call in that response. For non-streaming APIs, the final SDK response must prove completion. For streaming
+APIs, adapters resolve the reason on the existing terminal event; parseable arguments and iterator exhaustion are not
+commitment evidence. Providers must continue yielding incremental call updates without adding a second drain,
+buffering the complete stream, or waiting after EOF.
+
+Adapters that need to combine an ordinary provider reason with protocol-specific call commitment use a small local
+`_resolve_finish_reason(provider_finish_reason, *, has_function_calls, function_calls_committed)` function:
+
+- committed calls return `"tool_calls"` regardless of the ordinary provider reason;
+- calls without commitment never return `"tool_calls"`;
+- responses without calls preserve their ordinary reason.
+
+The helper is intentionally repeated in affected provider modules. The commitment evidence differs by protocol and
+does not justify a shared provider abstraction. Custom clients that return completed local function calls must set
+`finish_reason="tool_calls"` explicitly.
+
+Uncommitted call-bearing assistant turns remain in the caller-visible response but are marked as excluded from future
+model input. All assistant messages in that provider turn are omitted together, including reasoning that arrived in a
+separate streamed message, so later stateless replay cannot create a dangling provider call.
+
 ### Approval pause and resume
 
 ```mermaid
@@ -489,6 +517,8 @@ that manually replay messages own the equivalent rule: do not resend an approval
 | String input | Flexible string input follows the same loop behavior. | `test_base_client_with_function_calling_string_input` |
 | Multiple sequential rounds | Each round retains one call/result pair. | `test_base_client_with_function_calling_resets` |
 | Streaming call | Call chunks, one result update, and final text are emitted in order. | `test_base_client_with_streaming_function_calling` |
+| Missing or non-authorizing finish reason | Call content remains caller-visible, but no approval, middleware, tool body, result, or follow-up model call occurs; the uncommitted turn is excluded from later model input. | `test_function_calls_require_tool_calls_finish_reason`, `test_missing_tool_calls_finish_reason_does_not_request_approval` |
+| Provider terminal commitment | Streaming providers emit incremental calls immediately and assign `"tool_calls"` only on their authoritative terminal event; incomplete, failed, malformed, or prematurely exhausted streams remain non-authorizing. | Provider-specific completion tests in OpenAI Responses, Anthropic, Gemini, and AG-UI |
 | Function-call occurrence identity | Actionable calls gain one stable `Content.id`; a safe local empty-`call_id` fallback uses that id with a migration warning, and streaming aggregation preserves provider-assigned occurrence ids across interleaved fragments. OpenAI Chat Completions scopes fragment correlation to each request and `(choice.index, tool.index)`. | `test_actionable_function_call_gets_stable_occurrence_identity`, `test_actionable_function_call_uses_occurrence_identity_for_empty_call_id`, `test_streaming_empty_call_id_keeps_occurrence_identity_through_approval`, `test_streaming_empty_call_id_delta_reuses_opening_call_identity`, `test_streaming_interleaved_indexed_call_fragments_coalesce_by_occurrence`, `packages/core/tests/core/test_types.py::test_function_call_occurrence_id_roundtrips_without_regeneration`, `packages/openai/tests/openai/test_openai_chat_completion_client.py::test_streaming_tool_call_identity_is_request_local_and_scoped_by_choice_index` |
 | Reasoning-bound call | Finalized output retains reasoning, function call, function result, and final text. | `test_streaming_function_calling_response_includes_reasoning_and_tool_results` |
 | Calls across response messages | Every actionable call is executed once. | `test_base_client_executes_function_calls_across_multiple_response_messages` |

python/packages/ag-ui/agent_framework_ag_ui/_client.py

@@ -21,6 +21,7 @@
     ChatResponse,
     ChatResponseUpdate,
     Content,
+    FinishReason,
     FunctionTool,
     Message,
     ResponseStream,
@@ -58,6 +59,22 @@
 logger: logging.Logger = logging.getLogger("agent_framework.ag_ui")
 
 
+def _resolve_finish_reason(
+    provider_finish_reason: FinishReason | None,
+    *,
+    has_function_calls: bool,
+    function_calls_committed: bool,
+) -> FinishReason | None:
+    """Resolve the provider finish reason after function-call commitment."""
+    if not has_function_calls:
+        return provider_finish_reason
+    if function_calls_committed:
+        return FinishReason("tool_calls")
+    if provider_finish_reason == "tool_calls":
+        return None
+    return provider_finish_reason
+
+
 def _unwrap_server_function_call_contents(contents: MutableSequence[Content | dict[str, Any]]) -> None:
     """Replace server_function_call instances with their underlying call content."""
     for idx, content in enumerate(contents):
@@ -495,6 +512,8 @@ async def _streaming_impl(
         logger.debug(f"[AGUIChatClient] Client tool set: {client_tool_set}")
 
         converter = AGUIEventConverter()
+        has_function_calls = False
+        open_client_tool_call_ids: set[str] = set()
 
         available_interrupts = options.get("available_interrupts", options.get("availableInterrupts"))
 
@@ -508,6 +527,14 @@ async def _streaming_impl(
             resume=_serialize_resume(options.get("resume")),
         ):
             logger.debug(f"[AGUIChatClient] Raw AG-UI event: {event}")
+            event_type = str(event.get("type", "")).upper()
+            event_tool_call_id = str(event.get("toolCallId") or event.get("tool_call_id") or "")
+            if event_type == "TOOL_CALL_START":
+                event_tool_name = event.get("toolName") or event.get("toolCallName") or event.get("tool_call_name")
+                if event_tool_name in client_tool_set:
+                    open_client_tool_call_ids.add(event_tool_call_id)
+            elif event_type == "TOOL_CALL_END":
+                open_client_tool_call_ids.discard(event_tool_call_id)
             update = converter.convert_event(event)
             if update is not None:
                 logger.debug(
@@ -522,6 +549,7 @@ async def _streaming_impl(
                         )
                         if content.name in client_tool_set:
                             # Client tool - let function invocation execute it
+                            has_function_calls = True
                             if not content.additional_properties:
                                 content.additional_properties = {}
                             content.additional_properties["agui_thread_id"] = thread_id
@@ -531,4 +559,9 @@ async def _streaming_impl(
                             self._register_server_tool_placeholder(content.name)  # type: ignore[arg-type]
                             update.contents[i] = Content(type="server_function_call", function_call=content)  # type: ignore
 
+                update.finish_reason = _resolve_finish_reason(
+                    cast(FinishReason | None, update.finish_reason),
+                    has_function_calls=has_function_calls,
+                    function_calls_committed=event_type == "RUN_FINISHED" and not open_client_tool_call_ids,
+                )
                 yield update

python/packages/ag-ui/tests/ag_ui/conftest.py

@@ -38,6 +38,16 @@
 ResponseFn = Callable[..., Awaitable[ChatResponse]]
 
 
+def _finish_committed_test_calls(response: ChatResponse) -> ChatResponse:
+    if response.finish_reason is None and any(
+        content.type == "function_call" and not content.informational_only
+        for message in response.messages
+        for content in message.contents
+    ):
+        response.finish_reason = "tool_calls"
+    return response
+
+
 def pytest_configure() -> None:
     """Ensure this test directory is on sys.path so helper modules can be imported by name."""
     test_dir = str(Path(__file__).resolve().parent)
@@ -130,10 +140,28 @@ def _inner_get_response(
     ) -> Awaitable[ChatResponse] | ResponseStream[ChatResponseUpdate, ChatResponse]:
         if stream:
 
+            async def _stream() -> AsyncIterator[ChatResponseUpdate]:
+                has_function_calls = False
+                has_finish_reason = False
+                pending_update: ChatResponseUpdate | None = None
+                async for update in self._stream_fn(messages, options, **kwargs):
+                    has_function_calls = has_function_calls or any(
+                        content.type == "function_call" and not content.informational_only
+                        for content in update.contents
+                    )
+                    has_finish_reason = has_finish_reason or update.finish_reason is not None
+                    if pending_update is not None:
+                        yield pending_update
+                    pending_update = update
+                if pending_update is not None:
+                    if has_function_calls and not has_finish_reason:
+                        pending_update.finish_reason = "tool_calls"
+                    yield pending_update
+
             def _finalize(updates: Sequence[ChatResponseUpdate]) -> ChatResponse:
-                return ChatResponse.from_updates(updates)
+                return _finish_committed_test_calls(ChatResponse.from_updates(updates))
 
-            return ResponseStream(self._stream_fn(messages, options, **kwargs), finalizer=_finalize)
+            return ResponseStream(_stream(), finalizer=_finalize)
 
         return self._get_response_impl(messages, options, **kwargs)
 
@@ -142,15 +170,17 @@ async def _get_response_impl(
     ) -> ChatResponse:
         """Non-streaming implementation."""
         if self._response_fn is not None:
-            return await self._response_fn(messages, options, **kwargs)
+            return _finish_committed_test_calls(await self._response_fn(messages, options, **kwargs))
 
         contents: list[Any] = []
         async for update in self._stream_fn(list(messages), dict(options), **kwargs):
             contents.extend(update.contents)
 
-        return ChatResponse(
-            messages=[Message(role="assistant", contents=contents)],
-            response_id="stub-response",
+        return _finish_committed_test_calls(
+            ChatResponse(
+                messages=[Message(role="assistant", contents=contents)],
+                response_id="stub-response",
+            )
         )