sadpandajoe commented on code in PR #43135:
URL: https://github.com/apache/superset/pull/43135#discussion_r3872116722


##########
superset/ai/runtime/messages.py:
##########
@@ -0,0 +1,577 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+The default runtime: a plain tool-use loop over the provider's message API.
+
+Chosen as the default because it needs nothing beyond an HTTP call — no agent
+engine subprocess, no working directory, no bundled binary — so it works with
+whatever provider a deployment configures.
+"""
+
+from __future__ import annotations
+
+import logging
+import time
+from collections.abc import AsyncIterator
+from typing import Any
+
+from superset.ai.events import (
+    assistant_delta_event,
+    checkpoint_event,
+    error_event,
+    final_event,
+    GENERIC_ERROR_MESSAGE,
+    StreamEvent,
+    thinking_event,
+    thoughts_event,
+)
+from superset.ai.llm.base import (
+    CompletionRequest,
+    LLMError,
+    LLMResponse,
+    Message,
+    StreamEventKind,
+    ToolCall,
+    ToolResult,
+)
+from superset.ai.runtime.base import BaseAgentRuntime, RunRequest, RunResult
+from superset.ai.telemetry import (
+    current_run,
+    POLICY_DENIED,
+    RunRecorder,
+    TOOL_UNAVAILABLE,
+)
+from superset.ai.types import MessageRole, ProgressStage, TokenUsage
+
+logger = logging.getLogger(__name__)
+
+#: How much of a tool's output is kept on the persisted message. The model
+#: still sees the whole thing; this is the audit copy.
+_RECORDED_OUTPUT_LIMIT = 2_000
+
+#: Size of the chunks the finished answer is delivered in.
+_DELIVERY_CHUNK_SIZE = 512
+
+#: How much reasoning is kept on the result. Reasoning can run several times
+#: longer than the answer, and this is persisted next to it.
+_RECORDED_THOUGHTS_LIMIT = 8_000
+
+_NO_ANSWER = (
+    "I wasn't able to reach an answer for that. Try narrowing the question, "
+    "or naming the dataset you have in mind."
+)
+
+
+class MessagesApiRuntime(BaseAgentRuntime):
+    """
+    Alternates model calls and tool calls until the model stops asking.
+
+    Two behaviours are worth understanding before changing this class.
+
+    First, prose the model emits *before* a tool call is treated as reasoning,
+    not answer: it becomes a ``thoughts`` event and is dropped from the answer.
+    A model narrating "the orders table looks right, let me check" is stating a
+    hypothesis it may abandon, and appending that to the answer produces a
+    reply that contradicts itself.
+
+    Second, the loop always terminates and never raises for an operational
+    failure. By the time it runs, response headers have been flushed and an
+    exception can no longer become an HTTP status, so every failure is an 
event.
+    """
+
+    def __init__(self, provider: Any) -> None:
+        super().__init__(provider)
+        self._result = RunResult()
+        #: Set when the model signals it has finished answering.
+        self._finished = False
+        #: The most recent round trip's response, or ``None`` if it failed. The
+        #: turn methods are generators and cannot return a value.
+        self._last_response: LLMResponse | None = None
+        #: Whether any answer text has already been sent as it was generated. 
The
+        #: finished answer is only replayed in chunks when it has not.
+        self._streamed_text = False
+
+    @property
+    def result(self) -> RunResult:
+        return self._result
+
+    async def run(self, request: RunRequest) -> AsyncIterator[StreamEvent]:
+        self._result = RunResult()
+        self._finished = False
+        self._last_response = None
+        self._streamed_text = False
+        answer_parts: list[str] = []
+
+        yield thinking_event(ProgressStage.START, "Working on your question")
+
+        # The provider's connection pool belongs to the loop this run is driven
+        # on, and the caller closes that loop as soon as the run ends. Closing
+        # here — inside the loop, however the run finishes, including when the
+        # generator is abandoned mid-way by a user pressing stop — is what 
keeps
+        # a client from being finalised against a dead loop.
+        try:
+            async for event in self._turn_loop(request, answer_parts):
+                yield event
+
+            # A run that failed or was abandoned has already said so; emitting 
an
+            # answer as well would contradict it.
+            if self._result.error is not None or self._result.cancelled:
+                return
+
+            answer = "\n\n".join(part for part in answer_parts if part).strip()
+            self._result.answer = answer or _NO_ANSWER
+
+            # Only replayed when nothing was streamed — a provider without
+            # streaming support still gets to deliver its answer progressively.
+            # Replaying after live text would show the answer twice.
+            if not self._streamed_text:
+                for chunk in _chunk(self._result.answer):
+                    yield assistant_delta_event(chunk)
+            yield final_event(self._result.answer)
+        finally:
+            await self.provider.aclose()
+
+    async def _turn_loop(
+        self,
+        request: RunRequest,
+        answer_parts: list[str],
+    ) -> AsyncIterator[StreamEvent]:
+        """
+        Alternate model and tool calls until the model stops or a budget runs 
out.
+
+        Appends to ``answer_parts`` rather than returning the answer, because 
an
+        async generator cannot both yield events and return a value.
+        """
+        deadline = time.monotonic() + request.timeout_seconds
+        conversation = list(request.messages)
+
+        for turn in range(1, request.max_turns + 1):
+            self._result.turns = turn
+
+            if self._should_stop(request, deadline):
+                if self._result.timed_out:
+                    yield thinking_event(
+                        ProgressStage.FALLBACK,
+                        "Taking longer than expected — answering with what I 
have",
+                    )
+                return
+
+            async for event in self._safe_turn(request, conversation, turn):
+                yield event
+            response = self._last_response
+            if response is None:
+                yield error_event()
+                return
+
+            async for event in self._consume(
+                request, response, conversation, answer_parts
+            ):
+                yield event
+
+            if self._finished or self._result.cancelled:
+                return
+
+        # Budget exhausted without the model choosing to stop.

Review Comment:
   The worker transport still returns as soon as it reads `error`, so it never 
delivers the following `final` replacement. A turn that streamed narration 
before the step limit still leaves that narration visible; can the worker 
stream defer its terminal error until after the replacement?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to