scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-09-10 22:41:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 82

fix(chat-ui): address greptile review issues

Browse source 
- Reset responsesSessionId when activeConversationId changes (not just on new conversation)
- Wire onMCPEvent callback into makeOpenAIResponsesRequest; render MCPEventsDisplay below messages
- Clear mcpEvents on each new send
- Explicitly filter history to user/assistant roles only (no tool-role casting)
- Remove duplicate "Chat" menu item from sidebar (pinned button serves same purpose)
- Make Sider a flex column so "Open Chat" button actually pins to bottom
- Fix tests to intercept real HTTP requests and assert previous_response_id in body
This commit is contained in:
Ishaan Jaffer 2026-03-10 16:42:50 -07:00
parent f046b66416
commit 254bc70eee
3 changed files with 147 additions and 57 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

164
tests/test_litellm/test_chat_ui_responses_session.py
View file

@ -1,13 +1,13 @@
"""
Tests for the chat UI responses API session chaining logic.
Tests for responses API session chaining used by the chat UI.
Validates that:
1. previous_response_id is correctly forwarded in responses API calls
2. The parameter is omitted (not sent as None) when starting a new session
Verifies that:
1. previous_response_id is correctly forwarded when provided
2. Absence of previous_response_id does not break the call
3. The aresponses function signature exposes the expected parameters
"""
import os
import sys
from unittest.mock import AsyncMock, MagicMock, patch
sys.path.insert(0, os.path.abspath("../.."))
@ -19,52 +19,118 @@ import litellm
class TestResponsesSessionChaining:
"""Test previous_response_id session chaining for the chat UI."""
@pytest.mark.asyncio
async def test_previous_response_id_forwarded(self):
"""previous_response_id should be passed through to the responses API call."""
captured = {}
async def fake_aresponses(*args, **kwargs):
captured.update(kwargs)
mock_resp = MagicMock()
mock_resp.id = "resp_abc123"
mock_resp.output = []
return mock_resp
with patch("litellm.aresponses", side_effect=fake_aresponses):
await litellm.aresponses(
input="Hello",
model="gpt-4o",
previous_response_id="resp_prev999",
)
assert captured.get("previous_response_id") == "resp_prev999"
@pytest.mark.asyncio
async def test_new_session_has_no_previous_response_id(self):
"""A new conversation should not send previous_response_id."""
captured = {}
async def fake_aresponses(*args, **kwargs):
captured.update(kwargs)
mock_resp = MagicMock()
mock_resp.id = "resp_new001"
mock_resp.output = []
return mock_resp
with patch("litellm.aresponses", side_effect=fake_aresponses):
await litellm.aresponses(
input="Hello",
model="gpt-4o",
# No previous_response_id — new session
)
assert captured.get("previous_response_id") is None
def test_responses_api_signature_accepts_previous_response_id(self):
"""Smoke test: aresponses function signature accepts previous_response_id."""
"""aresponses must accept previous_response_id and onResponseId-like params."""
import inspect
sig = inspect.signature(litellm.aresponses)
assert "previous_response_id" in sig.parameters, (
"aresponses must accept previous_response_id for session chaining"
"aresponses must accept previous_response_id for multi-turn session chaining"
)
assert "input" in sig.parameters, "aresponses must accept input"
assert "model" in sig.parameters, "aresponses must accept model"
@pytest.mark.asyncio
async def test_previous_response_id_included_in_request_body(self):
"""previous_response_id must appear in the outgoing HTTP request body."""
import httpx
captured_body: dict = {}
async def mock_send(self_transport, request: httpx.Request, **kwargs):
import json
try:
captured_body.update(json.loads(request.content))
except Exception:
pass
# Return a minimal valid responses API response
response_json = {
"id": "resp_test123",
"object": "response",
"model": "gpt-4o-mini",
"output": [
{
"type": "message",
"id": "msg_001",
"role": "assistant",
"content": [{"type": "output_text", "text": "hi", "annotations": []}],
"status": "completed",
}
],
"usage": {"input_tokens": 5, "output_tokens": 3, "total_tokens": 8},
"status": "completed",
"created_at": 1700000000,
}
return httpx.Response(
200,
json=response_json,
request=request,
)
import unittest.mock as mock
with mock.patch("httpx.AsyncClient.send", mock_send):
try:
await litellm.aresponses(
input="hello",
model="gpt-4o-mini",
previous_response_id="resp_prev_abc",
api_key="sk-test-fake",
)
except Exception:
pass # response parsing may fail; we only care about the outgoing body
assert captured_body.get("previous_response_id") == "resp_prev_abc", (
f"Expected previous_response_id in request body, got: {captured_body}"
)
@pytest.mark.asyncio
async def test_no_previous_response_id_omitted_from_request(self):
"""When previous_response_id is None, it must not appear in the request body."""
import httpx
captured_body: dict = {}
async def mock_send(self_transport, request: httpx.Request, **kwargs):
import json
try:
captured_body.update(json.loads(request.content))
except Exception:
pass
response_json = {
"id": "resp_new001",
"object": "response",
"model": "gpt-4o-mini",
"output": [
{
"type": "message",
"id": "msg_001",
"role": "assistant",
"content": [{"type": "output_text", "text": "hi", "annotations": []}],
"status": "completed",
}
],
"usage": {"input_tokens": 5, "output_tokens": 3, "total_tokens": 8},
"status": "completed",
"created_at": 1700000000,
}
return httpx.Response(200, json=response_json, request=request)
import unittest.mock as mock
with mock.patch("httpx.AsyncClient.send", mock_send):
try:
await litellm.aresponses(
input="hello",
model="gpt-4o-mini",
previous_response_id=None,
api_key="sk-test-fake",
)
except Exception:
pass
assert "previous_response_id" not in captured_body, (
"previous_response_id must be omitted from the request body when None"
)

5
ui/litellm-dashboard/src/app/(dashboard)/components/Sidebar2.tsx
View file

@ -160,7 +160,6 @@ const toHref = (slugOrPath: string) => {
// ----- Menu config (unchanged labels/icons; same appearance) -----
const menuItems: MenuItemCfg[] = [
{ key: "1", page: "api-keys", label: "Virtual Keys", icon: <KeyOutlined style={{ fontSize: 18 }} /> },
{ key: "29", page: "chat", label: "Chat", icon: <MessageOutlined style={{ fontSize: 18 }} />, newTab: true },
{
key: "3",
page: "llm-playground",
@ -424,6 +423,8 @@ const Sidebar2: React.FC<SidebarProps> = ({ accessToken, userRole, defaultSelect
style={{
transition: "all 0.3s cubic-bezier(0.4, 0, 0.2, 1)",
position: "relative",
display: "flex",
flexDirection: "column",
}}
>
<ConfigProvider
@ -446,6 +447,8 @@ const Sidebar2: React.FC<SidebarProps> = ({ accessToken, userRole, defaultSelect
borderRight: 0,
backgroundColor: "transparent",
fontSize: "14px",
flex: 1,
overflowY: "auto",
}}
items={filteredMenuItems.map((item) => ({
key: item.key,

35
ui/litellm-dashboard/src/components/chat/ChatPage.tsx
View file

@ -27,6 +27,7 @@ import MCPAppsPanel from "./MCPAppsPanel";
import { fetchAvailableModels } from "../playground/llm_calls/fetch_models";
import { makeOpenAIChatCompletionRequest } from "../playground/llm_calls/chat_completion";
import { makeOpenAIResponsesRequest } from "../playground/llm_calls/responses_api";
import MCPEventsDisplay, { MCPEvent } from "../playground/chat_ui/MCPEventsDisplay";
import { getProxyBaseUrl } from "@/components/networking";
import { useUIConfig } from "@/app/(dashboard)/hooks/uiConfig/useUIConfig";
import { getProviderLogoAndName } from "@/components/provider_info_helpers";
@ -137,6 +138,7 @@ const ChatPage: React.FC<ChatPageProps> = ({ accessToken, userRole, userId, user
const [selectedMCPServers, setSelectedMCPServers] = useState<string[]>([]);
const [responsesSessionId, setResponsesSessionId] = useState<string | null>(null);
const [mcpEvents, setMcpEvents] = useState<MCPEvent[]>([]);
const [isStreaming, setIsStreaming] = useState(false);
const [inputText, setInputText] = useState("");
const [mcpPopoverOpen, setMcpPopoverOpen] = useState(false);
@ -205,6 +207,12 @@ const ChatPage: React.FC<ChatPageProps> = ({ accessToken, userRole, userId, user
if (staleId) router.replace(getChatUrl(uiRoot));
}, [staleId, router]);
// Reset the responses session when switching between conversations so that
// previous_response_id from conversation A is never sent for conversation B.
useEffect(() => {
setResponsesSessionId(null);
}, [activeConversationId]);
const toggleModel = useCallback((model: string) => {
setSelectedModels((prev) => {
let next: string[];
@ -240,14 +248,19 @@ const ChatPage: React.FC<ChatPageProps> = ({ accessToken, userRole, userId, user
appendMessage(convId, { role: "user", content: trimmed });
appendMessage(convId, { role: "assistant", content: "" });
setMcpEvents([]); // clear MCP events from previous request
setIsStreaming(true);
abortControllerRef.current = new AbortController();
// Explicitly filter to only user/assistant roles — tool messages lack
// a required tool_call_id and would cause API errors if forwarded.
const history = [
...(historyOverride ?? (activeConversation?.messages ?? [])
.filter((m) => m.role === "user" || m.role === "assistant")
.filter((m): m is typeof m & { role: "user" | "assistant" } =>
m.role === "user" || m.role === "assistant"
)
.map((m) => ({
role: m.role as "user" | "assistant",
role: m.role,
content: m.content,
}))),
{ role: "user" as const, content: trimmed },
@ -275,6 +288,7 @@ const ChatPage: React.FC<ChatPageProps> = ({ accessToken, userRole, userId, user
selectedMCPServers.length > 0 ? selectedMCPServers : undefined,
responsesSessionId,
(id: string) => setResponsesSessionId(id),
(event: MCPEvent) => setMcpEvents((prev) => [...prev, event]),
);
} catch (err: unknown) {
if (err instanceof Error && err.name === "AbortError") {
@ -1120,11 +1134,18 @@ const ChatPage: React.FC<ChatPageProps> = ({ accessToken, userRole, userId, user
})}
</div>
) : (
<ChatMessages
messages={activeConversation!.messages}
isStreaming={isStreaming}
onEditMessage={handleEditAndResend}
/>
<>
<ChatMessages
messages={activeConversation!.messages}
isStreaming={isStreaming}
onEditMessage={handleEditAndResend}
/>
{mcpEvents.length > 0 && (
<div style={{ padding: "0 16px 16px" }}>
<MCPEventsDisplay events={mcpEvents} />
</div>
)}
</>
)}
</div>
{showScrollButton && (