mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-10 22:41:41 +00:00
fix(chat-ui): switch to responses API, remove dead code, add tests
- Switch handleSend from makeOpenAIChatCompletionRequest to makeOpenAIResponsesRequest with previous_response_id session chaining - Add responsesSessionId state; reset to null when starting a new conversation - Remove unused ChatInputBar.tsx and ModelSelector.tsx (dead code) - Add tests/test_litellm/test_chat_ui_responses_session.py covering previous_response_id forwarding and signature validation
This commit is contained in:
parent
c2c44d993c
commit
f046b66416
4 changed files with 79 additions and 218 deletions
70
tests/test_litellm/test_chat_ui_responses_session.py
Normal file
70
tests/test_litellm/test_chat_ui_responses_session.py
Normal file
|
|
@ -0,0 +1,70 @@
|
|||
"""
|
||||
Tests for the chat UI responses API session chaining logic.
|
||||
|
||||
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
|
||||
"""
|
||||
import os
|
||||
import sys
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
sys.path.insert(0, os.path.abspath("../.."))
|
||||
|
||||
import pytest
|
||||
|
||||
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."""
|
||||
import inspect
|
||||
sig = inspect.signature(litellm.aresponses)
|
||||
assert "previous_response_id" in sig.parameters, (
|
||||
"aresponses must accept previous_response_id for session chaining"
|
||||
)
|
||||
|
|
@ -1,107 +0,0 @@
|
|||
import React, { useState } from "react";
|
||||
import { Button, Input, Popover, Tooltip } from "antd";
|
||||
import { ApiOutlined, BorderOutlined, PaperClipOutlined, SendOutlined } from "@ant-design/icons";
|
||||
|
||||
interface Props {
|
||||
onSend: (text: string) => void;
|
||||
isStreaming: boolean;
|
||||
onStop: () => void;
|
||||
selectedMCPServers: string[];
|
||||
onMCPChange: (servers: string[]) => void;
|
||||
isLoadingModels: boolean;
|
||||
accessToken: string;
|
||||
}
|
||||
|
||||
const ChatInputBar: React.FC<Props> = ({
|
||||
onSend,
|
||||
isStreaming,
|
||||
onStop,
|
||||
selectedMCPServers,
|
||||
onMCPChange,
|
||||
isLoadingModels,
|
||||
accessToken,
|
||||
}) => {
|
||||
const [text, setText] = useState<string>("");
|
||||
const [mcpPopoverOpen, setMcpPopoverOpen] = useState<boolean>(false);
|
||||
|
||||
const handleSend = () => {
|
||||
if (text.trim() === "" || isStreaming || isLoadingModels) return;
|
||||
onSend(text.trim());
|
||||
setText("");
|
||||
};
|
||||
|
||||
const handleKeyDown = (e: React.KeyboardEvent<HTMLTextAreaElement>) => {
|
||||
if (e.key === "Enter" && !e.shiftKey) {
|
||||
e.preventDefault();
|
||||
handleSend();
|
||||
}
|
||||
};
|
||||
|
||||
const mcpButtonLabel =
|
||||
selectedMCPServers.length > 0
|
||||
? `MCP (${selectedMCPServers.length})`
|
||||
: "MCP";
|
||||
|
||||
const mcpPopoverContent = (
|
||||
<div style={{ minWidth: 200 }}>
|
||||
{/* MCPConnectPicker - LIT-2170 */}
|
||||
</div>
|
||||
);
|
||||
|
||||
return (
|
||||
<div
|
||||
style={{
|
||||
display: "flex",
|
||||
alignItems: "flex-end",
|
||||
gap: 8,
|
||||
padding: "12px 16px",
|
||||
borderTop: "1px solid #e5e7eb",
|
||||
backgroundColor: "#ffffff",
|
||||
}}
|
||||
>
|
||||
<Popover
|
||||
content={mcpPopoverContent}
|
||||
title="MCP Servers"
|
||||
trigger="click"
|
||||
open={mcpPopoverOpen}
|
||||
onOpenChange={setMcpPopoverOpen}
|
||||
placement="topLeft"
|
||||
>
|
||||
<Button icon={<ApiOutlined />}>
|
||||
{mcpButtonLabel}
|
||||
</Button>
|
||||
</Popover>
|
||||
|
||||
<Tooltip title="Coming soon">
|
||||
<Button icon={<PaperClipOutlined />} disabled />
|
||||
</Tooltip>
|
||||
|
||||
<Input.TextArea
|
||||
value={text}
|
||||
onChange={(e) => setText(e.target.value)}
|
||||
onKeyDown={handleKeyDown}
|
||||
placeholder="Message..."
|
||||
autoSize={{ minRows: 1, maxRows: 5 }}
|
||||
style={{ flex: 1 }}
|
||||
/>
|
||||
|
||||
{isStreaming ? (
|
||||
<Button
|
||||
icon={<BorderOutlined />}
|
||||
onClick={onStop}
|
||||
type="primary"
|
||||
danger
|
||||
/>
|
||||
) : (
|
||||
<Button
|
||||
icon={<SendOutlined />}
|
||||
onClick={handleSend}
|
||||
type="primary"
|
||||
disabled={isStreaming || isLoadingModels || text.trim() === ""}
|
||||
/>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
};
|
||||
|
||||
export default ChatInputBar;
|
||||
|
|
@ -26,6 +26,7 @@ import MCPConnectPicker from "./MCPConnectPicker";
|
|||
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 { getProxyBaseUrl } from "@/components/networking";
|
||||
import { useUIConfig } from "@/app/(dashboard)/hooks/uiConfig/useUIConfig";
|
||||
import { getProviderLogoAndName } from "@/components/provider_info_helpers";
|
||||
|
|
@ -135,6 +136,7 @@ const ChatPage: React.FC<ChatPageProps> = ({ accessToken, userRole, userId, user
|
|||
const [modelSearchText, setModelSearchText] = useState("");
|
||||
|
||||
const [selectedMCPServers, setSelectedMCPServers] = useState<string[]>([]);
|
||||
const [responsesSessionId, setResponsesSessionId] = useState<string | null>(null);
|
||||
const [isStreaming, setIsStreaming] = useState(false);
|
||||
const [inputText, setInputText] = useState("");
|
||||
const [mcpPopoverOpen, setMcpPopoverOpen] = useState(false);
|
||||
|
|
@ -231,6 +233,7 @@ const ChatPage: React.FC<ChatPageProps> = ({ accessToken, userRole, userId, user
|
|||
let convId = activeConversationId;
|
||||
if (!convId) {
|
||||
convId = createConversation(model);
|
||||
setResponsesSessionId(null); // new conversation starts a fresh session
|
||||
router.push(getChatUrl(uiRoot, convId));
|
||||
}
|
||||
|
||||
|
|
@ -254,15 +257,15 @@ const ChatPage: React.FC<ChatPageProps> = ({ accessToken, userRole, userId, user
|
|||
let accumulatedReasoning = "";
|
||||
|
||||
try {
|
||||
await makeOpenAIChatCompletionRequest(
|
||||
await makeOpenAIResponsesRequest(
|
||||
history,
|
||||
(chunk: string) => {
|
||||
(_role: string, chunk: string) => {
|
||||
accumulatedContent += chunk;
|
||||
updateLastAssistantMessage(convId!, { content: accumulatedContent });
|
||||
},
|
||||
model,
|
||||
accessToken,
|
||||
undefined,
|
||||
undefined, // tags
|
||||
abortControllerRef.current.signal,
|
||||
(rc: string) => {
|
||||
accumulatedReasoning += rc;
|
||||
|
|
@ -270,6 +273,8 @@ const ChatPage: React.FC<ChatPageProps> = ({ accessToken, userRole, userId, user
|
|||
},
|
||||
undefined, undefined, undefined, undefined, undefined, undefined,
|
||||
selectedMCPServers.length > 0 ? selectedMCPServers : undefined,
|
||||
responsesSessionId,
|
||||
(id: string) => setResponsesSessionId(id),
|
||||
);
|
||||
} catch (err: unknown) {
|
||||
if (err instanceof Error && err.name === "AbortError") {
|
||||
|
|
@ -287,7 +292,7 @@ const ChatPage: React.FC<ChatPageProps> = ({ accessToken, userRole, userId, user
|
|||
}
|
||||
},
|
||||
[activeConversationId, activeConversation, selectedModels, selectedMCPServers, accessToken,
|
||||
createConversation, appendMessage, updateLastAssistantMessage, router, isStreaming],
|
||||
createConversation, appendMessage, updateLastAssistantMessage, router, isStreaming, responsesSessionId],
|
||||
);
|
||||
|
||||
const handleSendComparison = useCallback(
|
||||
|
|
|
|||
|
|
@ -1,107 +0,0 @@
|
|||
import React, { useEffect, useState } from "react";
|
||||
import { Select, Skeleton } from "antd";
|
||||
import { fetchAvailableModels, ModelGroup } from "../playground/llm_calls/fetch_models";
|
||||
|
||||
const LOCALSTORAGE_KEY = "litellm_chat_selected_model";
|
||||
const MAX_DISPLAY_LENGTH = 40;
|
||||
|
||||
interface Props {
|
||||
accessToken: string;
|
||||
selectedModel: string;
|
||||
onChange: (model: string) => void;
|
||||
onLoadingChange: (loading: boolean) => void;
|
||||
}
|
||||
|
||||
const ModelSelector: React.FC<Props> = ({
|
||||
accessToken,
|
||||
selectedModel,
|
||||
onChange,
|
||||
onLoadingChange,
|
||||
}) => {
|
||||
const [models, setModels] = useState<ModelGroup[]>([]);
|
||||
const [loading, setLoading] = useState(true);
|
||||
const [fetchFailed, setFetchFailed] = useState(false);
|
||||
|
||||
useEffect(() => {
|
||||
let cancelled = false;
|
||||
|
||||
const load = async () => {
|
||||
setLoading(true);
|
||||
onLoadingChange(true);
|
||||
try {
|
||||
const fetched = await fetchAvailableModels(accessToken);
|
||||
if (cancelled) return;
|
||||
|
||||
setModels(fetched);
|
||||
|
||||
if (fetched.length > 0) {
|
||||
const persisted = localStorage.getItem(LOCALSTORAGE_KEY);
|
||||
const modelNames = fetched.map((m) => m.model_group);
|
||||
|
||||
if (persisted && modelNames.includes(persisted)) {
|
||||
onChange(persisted);
|
||||
} else {
|
||||
// Persisted model not in list — clear stale value and default to first
|
||||
if (persisted) {
|
||||
localStorage.removeItem(LOCALSTORAGE_KEY);
|
||||
}
|
||||
onChange(fetched[0].model_group);
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
if (!cancelled) {
|
||||
setFetchFailed(true);
|
||||
}
|
||||
} finally {
|
||||
if (!cancelled) {
|
||||
setLoading(false);
|
||||
onLoadingChange(false);
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
load();
|
||||
|
||||
return () => {
|
||||
cancelled = true;
|
||||
};
|
||||
}, [accessToken]); // eslint-disable-line react-hooks/exhaustive-deps
|
||||
|
||||
const handleChange = (value: string) => {
|
||||
localStorage.setItem(LOCALSTORAGE_KEY, value);
|
||||
onChange(value);
|
||||
};
|
||||
|
||||
if (loading) {
|
||||
return <Skeleton.Input active style={{ width: 220 }} />;
|
||||
}
|
||||
|
||||
if (fetchFailed || models.length === 0) {
|
||||
return (
|
||||
<span style={{ color: "#8c8c8c", fontSize: 13 }}>
|
||||
No models available — check your proxy config
|
||||
</span>
|
||||
);
|
||||
}
|
||||
|
||||
return (
|
||||
<Select
|
||||
value={selectedModel}
|
||||
onChange={handleChange}
|
||||
style={{ width: 220 }}
|
||||
showSearch
|
||||
filterOption={(input, option) =>
|
||||
(option?.label as string ?? "").toLowerCase().includes(input.toLowerCase())
|
||||
}
|
||||
options={models.map((m) => ({
|
||||
value: m.model_group,
|
||||
label:
|
||||
m.model_group.length > MAX_DISPLAY_LENGTH
|
||||
? `${m.model_group.slice(0, MAX_DISPLAY_LENGTH)}…`
|
||||
: m.model_group,
|
||||
}))}
|
||||
/>
|
||||
);
|
||||
};
|
||||
|
||||
export default ModelSelector;
|
||||
Loading…
Add table
Reference in a new issue