Enhance model validation and tool call management
Browse files- Updated the build_chat_request function to validate model names against available models, raising an HTTPException for unknown models.
- Enhanced UIMessageStreamEncoder to manage open tool call IDs, ensuring proper closure of tool calls that do not complete, and added error handling for unfinished tool calls.
- Modified the GoogleSearchActivity class to accurately track activity completion per cycle, preventing duplicate events and ensuring correct state management.
- Updated tests to validate the new model validation and tool call management functionalities, ensuring robustness in error handling and state transitions.
- app/api.py +19 -1
- app/chat_service.py +6 -1
- app/provider_events.py +14 -3
- frontend/components/chat-shell.tsx +1 -0
- tests/test_api.py +60 -1
- tests/test_chat_service.py +1 -1
- tests/test_provider_events.py +47 -1
app/api.py
CHANGED
|
@@ -148,7 +148,9 @@ def build_chat_request(payload: ApiChatRequest) -> ChatRequest:
|
|
| 148 |
)
|
| 149 |
or DEFAULT_SELECTED_SOURCE_KEYS
|
| 150 |
)
|
| 151 |
-
model_name = (payload.model or DEFAULT_MODEL_NAME).strip()
|
|
|
|
|
|
|
| 152 |
if payload.enabledTools is None:
|
| 153 |
enabled_tools = tuple(
|
| 154 |
tool["key"]
|
|
@@ -185,6 +187,7 @@ class UIMessageStreamEncoder:
|
|
| 185 |
self.text_block_count = 0
|
| 186 |
self.active_reasoning_id = ""
|
| 187 |
self.source_matches_by_call_id: dict[str, list[dict[str, Any]]] = {}
|
|
|
|
| 188 |
self.closed = False
|
| 189 |
|
| 190 |
def close_reasoning_block(self) -> list[dict[str, Any]]:
|
|
@@ -270,6 +273,8 @@ class UIMessageStreamEncoder:
|
|
| 270 |
parts.extend(self.close_text_block())
|
| 271 |
call_id = str(event.data.get("call_id", uuid4().hex))
|
| 272 |
tool_name = str(event.data.get("tool_name", "tool"))
|
|
|
|
|
|
|
| 273 |
parts.append(
|
| 274 |
{
|
| 275 |
"type": "tool-input-start",
|
|
@@ -352,6 +357,8 @@ class UIMessageStreamEncoder:
|
|
| 352 |
"text": str(event.data.get("output_text", "")),
|
| 353 |
"matches": self.source_matches_by_call_id.get(call_id, []),
|
| 354 |
}
|
|
|
|
|
|
|
| 355 |
parts.append(
|
| 356 |
{
|
| 357 |
"type": "tool-output-available",
|
|
@@ -389,6 +396,17 @@ class UIMessageStreamEncoder:
|
|
| 389 |
parts.extend(self.close_reasoning_block())
|
| 390 |
if self.text_block_id:
|
| 391 |
parts.append({"type": "text-end", "id": self.text_block_id})
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 392 |
if self.message_id:
|
| 393 |
parts.append({"type": "finish-step"})
|
| 394 |
parts.append({"type": "finish"})
|
|
|
|
| 148 |
)
|
| 149 |
or DEFAULT_SELECTED_SOURCE_KEYS
|
| 150 |
)
|
| 151 |
+
model_name = (payload.model or DEFAULT_MODEL_NAME).strip() or DEFAULT_MODEL_NAME
|
| 152 |
+
if model_name not in {model["id"] for model in AVAILABLE_MODELS}:
|
| 153 |
+
raise HTTPException(status_code=422, detail="Unknown model")
|
| 154 |
if payload.enabledTools is None:
|
| 155 |
enabled_tools = tuple(
|
| 156 |
tool["key"]
|
|
|
|
| 187 |
self.text_block_count = 0
|
| 188 |
self.active_reasoning_id = ""
|
| 189 |
self.source_matches_by_call_id: dict[str, list[dict[str, Any]]] = {}
|
| 190 |
+
self.open_tool_call_ids: list[str] = []
|
| 191 |
self.closed = False
|
| 192 |
|
| 193 |
def close_reasoning_block(self) -> list[dict[str, Any]]:
|
|
|
|
| 273 |
parts.extend(self.close_text_block())
|
| 274 |
call_id = str(event.data.get("call_id", uuid4().hex))
|
| 275 |
tool_name = str(event.data.get("tool_name", "tool"))
|
| 276 |
+
if call_id not in self.open_tool_call_ids:
|
| 277 |
+
self.open_tool_call_ids.append(call_id)
|
| 278 |
parts.append(
|
| 279 |
{
|
| 280 |
"type": "tool-input-start",
|
|
|
|
| 357 |
"text": str(event.data.get("output_text", "")),
|
| 358 |
"matches": self.source_matches_by_call_id.get(call_id, []),
|
| 359 |
}
|
| 360 |
+
if call_id in self.open_tool_call_ids:
|
| 361 |
+
self.open_tool_call_ids.remove(call_id)
|
| 362 |
parts.append(
|
| 363 |
{
|
| 364 |
"type": "tool-output-available",
|
|
|
|
| 396 |
parts.extend(self.close_reasoning_block())
|
| 397 |
if self.text_block_id:
|
| 398 |
parts.append({"type": "text-end", "id": self.text_block_id})
|
| 399 |
+
# Close tool calls the turn never finished, or the UI shows
|
| 400 |
+
# them as running forever.
|
| 401 |
+
for call_id in self.open_tool_call_ids:
|
| 402 |
+
parts.append(
|
| 403 |
+
{
|
| 404 |
+
"type": "tool-output-error",
|
| 405 |
+
"toolCallId": call_id,
|
| 406 |
+
"errorText": "The response ended before this tool call finished.",
|
| 407 |
+
}
|
| 408 |
+
)
|
| 409 |
+
self.open_tool_call_ids = []
|
| 410 |
if self.message_id:
|
| 411 |
parts.append({"type": "finish-step"})
|
| 412 |
parts.append({"type": "finish"})
|
app/chat_service.py
CHANGED
|
@@ -1051,7 +1051,7 @@ async def stream_chat(request: ChatRequest) -> AsyncIterator[ChatEvent]:
|
|
| 1051 |
if metadata.get("lc_source") == "summarization":
|
| 1052 |
continue
|
| 1053 |
|
| 1054 |
-
step = str(metadata.get("
|
| 1055 |
if include_reasoning:
|
| 1056 |
thought_text = "\n\n".join(extract_thought_summaries(token.content))
|
| 1057 |
if thought_text:
|
|
@@ -1171,6 +1171,11 @@ async def stream_chat(request: ChatRequest) -> AsyncIterator[ChatEvent]:
|
|
| 1171 |
)
|
| 1172 |
if search_started:
|
| 1173 |
yield search_started
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1174 |
|
| 1175 |
if is_anthropic_model(request.model_name):
|
| 1176 |
anthropic_updates, anthropic_tool_uses = (
|
|
|
|
| 1051 |
if metadata.get("lc_source") == "summarization":
|
| 1052 |
continue
|
| 1053 |
|
| 1054 |
+
step = str(metadata.get("langgraph_node", ""))
|
| 1055 |
if include_reasoning:
|
| 1056 |
thought_text = "\n\n".join(extract_thought_summaries(token.content))
|
| 1057 |
if thought_text:
|
|
|
|
| 1171 |
)
|
| 1172 |
if search_started:
|
| 1173 |
yield search_started
|
| 1174 |
+
# This model step is complete, so any search it ran is done:
|
| 1175 |
+
# close the activity now instead of when the whole turn ends.
|
| 1176 |
+
search_step_completed = google_search.completed_event()
|
| 1177 |
+
if search_step_completed:
|
| 1178 |
+
yield search_step_completed
|
| 1179 |
|
| 1180 |
if is_anthropic_model(request.model_name):
|
| 1181 |
anthropic_updates, anthropic_tool_uses = (
|
app/provider_events.py
CHANGED
|
@@ -112,11 +112,17 @@ class GoogleSearchActivity:
|
|
| 112 |
self._message_id = message_id
|
| 113 |
self._web_evidence = web_evidence
|
| 114 |
self._call_id = ""
|
|
|
|
| 115 |
self._queries: list[str] = []
|
| 116 |
self._match_count = 0
|
| 117 |
|
| 118 |
def observe(self, response_metadata: Any) -> ChatEvent | None:
|
| 119 |
-
"""Record metadata; return a tool_call_started event on
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 120 |
new_queries = [
|
| 121 |
q
|
| 122 |
for q in extract_web_search_queries(response_metadata)
|
|
@@ -127,8 +133,11 @@ class GoogleSearchActivity:
|
|
| 127 |
self._web_evidence,
|
| 128 |
)
|
| 129 |
started: ChatEvent | None = None
|
| 130 |
-
if (new_queries or new_grounding) and not self._call_id:
|
| 131 |
self._call_id = uuid4().hex
|
|
|
|
|
|
|
|
|
|
| 132 |
joined = "; ".join(new_queries)
|
| 133 |
started = ChatEvent(
|
| 134 |
"tool_call_started",
|
|
@@ -145,8 +154,10 @@ class GoogleSearchActivity:
|
|
| 145 |
return started
|
| 146 |
|
| 147 |
def completed_event(self) -> ChatEvent | None:
|
| 148 |
-
|
|
|
|
| 149 |
return None
|
|
|
|
| 150 |
joined = "; ".join(self._queries)
|
| 151 |
if self._match_count == 0:
|
| 152 |
output_text = "Google search ran but returned no grounding results."
|
|
|
|
| 112 |
self._message_id = message_id
|
| 113 |
self._web_evidence = web_evidence
|
| 114 |
self._call_id = ""
|
| 115 |
+
self._completed = False
|
| 116 |
self._queries: list[str] = []
|
| 117 |
self._match_count = 0
|
| 118 |
|
| 119 |
def observe(self, response_metadata: Any) -> ChatEvent | None:
|
| 120 |
+
"""Record metadata; return a tool_call_started event on new activity.
|
| 121 |
+
|
| 122 |
+
Activity after a completed cycle starts a fresh synthetic call (a
|
| 123 |
+
later model step ran another search), so completion timing per step
|
| 124 |
+
stays accurate.
|
| 125 |
+
"""
|
| 126 |
new_queries = [
|
| 127 |
q
|
| 128 |
for q in extract_web_search_queries(response_metadata)
|
|
|
|
| 133 |
self._web_evidence,
|
| 134 |
)
|
| 135 |
started: ChatEvent | None = None
|
| 136 |
+
if (new_queries or new_grounding) and (not self._call_id or self._completed):
|
| 137 |
self._call_id = uuid4().hex
|
| 138 |
+
self._completed = False
|
| 139 |
+
self._queries = []
|
| 140 |
+
self._match_count = 0
|
| 141 |
joined = "; ".join(new_queries)
|
| 142 |
started = ChatEvent(
|
| 143 |
"tool_call_started",
|
|
|
|
| 154 |
return started
|
| 155 |
|
| 156 |
def completed_event(self) -> ChatEvent | None:
|
| 157 |
+
"""Close the active cycle; idempotent until new activity arrives."""
|
| 158 |
+
if not self._call_id or self._completed:
|
| 159 |
return None
|
| 160 |
+
self._completed = True
|
| 161 |
joined = "; ".join(self._queries)
|
| 162 |
if self._match_count == 0:
|
| 163 |
output_text = "Google search ran but returned no grounding results."
|
frontend/components/chat-shell.tsx
CHANGED
|
@@ -304,6 +304,7 @@ export function ChatShell() {
|
|
| 304 |
messageId,
|
| 305 |
body: {
|
| 306 |
sourceKeys: selectedSourceKeys,
|
|
|
|
| 307 |
includeReasoning: true,
|
| 308 |
threadId,
|
| 309 |
model: selectedModel,
|
|
|
|
| 304 |
messageId,
|
| 305 |
body: {
|
| 306 |
sourceKeys: selectedSourceKeys,
|
| 307 |
+
enabledTools: enabledToolKeys,
|
| 308 |
includeReasoning: true,
|
| 309 |
threadId,
|
| 310 |
model: selectedModel,
|
tests/test_api.py
CHANGED
|
@@ -17,7 +17,7 @@ import pytest
|
|
| 17 |
import uvicorn
|
| 18 |
from fastapi.testclient import TestClient
|
| 19 |
|
| 20 |
-
from app.api import app
|
| 21 |
from app.chat_types import ChatEvent
|
| 22 |
|
| 23 |
|
|
@@ -273,6 +273,65 @@ class ApiTestCase(unittest.TestCase):
|
|
| 273 |
part_types.index("tool-input-start"), part_types.index("text-start")
|
| 274 |
)
|
| 275 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 276 |
|
| 277 |
LIVE_API_E2E = pytest.mark.skipif(
|
| 278 |
os.getenv("RUN_LIVE_API_E2E") != "1",
|
|
|
|
| 17 |
import uvicorn
|
| 18 |
from fastapi.testclient import TestClient
|
| 19 |
|
| 20 |
+
from app.api import UIMessageStreamEncoder, app
|
| 21 |
from app.chat_types import ChatEvent
|
| 22 |
|
| 23 |
|
|
|
|
| 273 |
part_types.index("tool-input-start"), part_types.index("text-start")
|
| 274 |
)
|
| 275 |
|
| 276 |
+
def test_finish_error_closes_open_tool_calls(self) -> None:
|
| 277 |
+
encoder = UIMessageStreamEncoder()
|
| 278 |
+
encoder.encode(ChatEvent("message_started", {"message_id": "m1"}))
|
| 279 |
+
encoder.encode(
|
| 280 |
+
ChatEvent(
|
| 281 |
+
"tool_call_started",
|
| 282 |
+
{
|
| 283 |
+
"message_id": "m1",
|
| 284 |
+
"call_id": "call_open",
|
| 285 |
+
"tool_name": "google_search",
|
| 286 |
+
"args": {"query": "x"},
|
| 287 |
+
"args_text": "x",
|
| 288 |
+
},
|
| 289 |
+
)
|
| 290 |
+
)
|
| 291 |
+
encoder.encode(
|
| 292 |
+
ChatEvent(
|
| 293 |
+
"tool_call_started",
|
| 294 |
+
{
|
| 295 |
+
"message_id": "m1",
|
| 296 |
+
"call_id": "call_done",
|
| 297 |
+
"tool_name": "run_kb_command",
|
| 298 |
+
"args": {"command": "ls"},
|
| 299 |
+
"args_text": "ls",
|
| 300 |
+
},
|
| 301 |
+
)
|
| 302 |
+
)
|
| 303 |
+
encoder.encode(
|
| 304 |
+
ChatEvent(
|
| 305 |
+
"tool_call_completed",
|
| 306 |
+
{"message_id": "m1", "call_id": "call_done", "output_text": "ok"},
|
| 307 |
+
)
|
| 308 |
+
)
|
| 309 |
+
|
| 310 |
+
parts = encoder.finish_error("boom")
|
| 311 |
+
|
| 312 |
+
tool_errors = [p for p in parts if p["type"] == "tool-output-error"]
|
| 313 |
+
self.assertEqual(len(tool_errors), 1)
|
| 314 |
+
self.assertEqual(tool_errors[0]["toolCallId"], "call_open")
|
| 315 |
+
# Stream still terminates cleanly after closing the tool part.
|
| 316 |
+
part_types = [p["type"] for p in parts]
|
| 317 |
+
self.assertLess(
|
| 318 |
+
part_types.index("tool-output-error"), part_types.index("finish")
|
| 319 |
+
)
|
| 320 |
+
|
| 321 |
+
def test_chat_rejects_unknown_model(self) -> None:
|
| 322 |
+
with TestClient(app) as client:
|
| 323 |
+
response = client.post(
|
| 324 |
+
"/api/chat",
|
| 325 |
+
json={
|
| 326 |
+
"messages": [
|
| 327 |
+
{"role": "user", "parts": [{"type": "text", "text": "hi"}]}
|
| 328 |
+
],
|
| 329 |
+
"model": "anthropic:claude-opus-4-8",
|
| 330 |
+
},
|
| 331 |
+
)
|
| 332 |
+
|
| 333 |
+
self.assertEqual(response.status_code, 422)
|
| 334 |
+
|
| 335 |
|
| 336 |
LIVE_API_E2E = pytest.mark.skipif(
|
| 337 |
os.getenv("RUN_LIVE_API_E2E") != "1",
|
tests/test_chat_service.py
CHANGED
|
@@ -69,7 +69,7 @@ class FakeStreamingAgent(FakeAgent):
|
|
| 69 |
}
|
| 70 |
],
|
| 71 |
),
|
| 72 |
-
{"
|
| 73 |
),
|
| 74 |
}
|
| 75 |
yield {
|
|
|
|
| 69 |
}
|
| 70 |
],
|
| 71 |
),
|
| 72 |
+
{"langgraph_node": "model"},
|
| 73 |
),
|
| 74 |
}
|
| 75 |
yield {
|
tests/test_provider_events.py
CHANGED
|
@@ -1,6 +1,52 @@
|
|
| 1 |
from __future__ import annotations
|
| 2 |
|
| 3 |
-
from app.provider_events import
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 4 |
|
| 5 |
|
| 6 |
def test_web_fetch_result_object_recorded_as_evidence() -> None:
|
|
|
|
| 1 |
from __future__ import annotations
|
| 2 |
|
| 3 |
+
from app.provider_events import (
|
| 4 |
+
GoogleSearchActivity,
|
| 5 |
+
extract_anthropic_source_matches,
|
| 6 |
+
)
|
| 7 |
+
|
| 8 |
+
|
| 9 |
+
def _grounding_metadata(query: str, uri: str, title: str) -> dict:
|
| 10 |
+
return {
|
| 11 |
+
"grounding_metadata": {
|
| 12 |
+
"web_search_queries": [query],
|
| 13 |
+
"grounding_chunks": [{"web": {"uri": uri, "title": title}}],
|
| 14 |
+
"grounding_supports": [],
|
| 15 |
+
}
|
| 16 |
+
}
|
| 17 |
+
|
| 18 |
+
|
| 19 |
+
def test_google_search_activity_completes_once_per_cycle() -> None:
|
| 20 |
+
evidence: dict = {}
|
| 21 |
+
activity = GoogleSearchActivity("msg_1", evidence)
|
| 22 |
+
|
| 23 |
+
started = activity.observe(_grounding_metadata("q1", "https://a.example", "A"))
|
| 24 |
+
assert started is not None
|
| 25 |
+
assert started.data["tool_name"] == "google_search"
|
| 26 |
+
|
| 27 |
+
completed = activity.completed_event()
|
| 28 |
+
assert completed is not None
|
| 29 |
+
assert "1 web result" in completed.data["output_text"]
|
| 30 |
+
assert completed.data["call_id"] == started.data["call_id"]
|
| 31 |
+
# Idempotent: the post-turn fallback must not duplicate the event.
|
| 32 |
+
assert activity.completed_event() is None
|
| 33 |
+
|
| 34 |
+
|
| 35 |
+
def test_google_search_activity_restarts_after_completion() -> None:
|
| 36 |
+
# A later model step running another search gets its own activity cycle
|
| 37 |
+
# instead of being swallowed by the completed one.
|
| 38 |
+
activity = GoogleSearchActivity("msg_1", {})
|
| 39 |
+
first = activity.observe(_grounding_metadata("q1", "https://a.example", "A"))
|
| 40 |
+
assert activity.completed_event() is not None
|
| 41 |
+
|
| 42 |
+
second = activity.observe(_grounding_metadata("q2", "https://b.example", "B"))
|
| 43 |
+
assert second is not None
|
| 44 |
+
assert second.data["call_id"] != first.data["call_id"]
|
| 45 |
+
assert second.data["args_text"] == "q2"
|
| 46 |
+
|
| 47 |
+
completed = activity.completed_event()
|
| 48 |
+
assert completed is not None
|
| 49 |
+
assert "1 web result" in completed.data["output_text"]
|
| 50 |
|
| 51 |
|
| 52 |
def test_web_fetch_result_object_recorded_as_evidence() -> None:
|