mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-23 05:34:41 +00:00
fix(chat): retain MCP results when replaying conversation history
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
90766d0655
commit
8cc40dfc5e
4 changed files with 170 additions and 22 deletions
|
|
@ -5,6 +5,8 @@ import ChatConversationPage from "./page";
|
|||
import { renderWithProviders } from "@/../tests/test-utils";
|
||||
import type { OnUrlUpdateFunction } from "nuqs/adapters/testing";
|
||||
import { fetchAvailableModels } from "@/components/llm_calls/fetch_models";
|
||||
import type { makeOpenAIResponsesRequest } from "@/components/llm_calls/responses_api";
|
||||
import type { MCPEvent } from "@/components/chat/types";
|
||||
|
||||
const { mockMakeOpenAIResponsesRequest, shellState } = vi.hoisted(() => ({
|
||||
mockMakeOpenAIResponsesRequest: vi.fn(),
|
||||
|
|
@ -139,6 +141,68 @@ describe("/ui/chat request metrics", () => {
|
|||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ providers: ["openrouter"], stateless: true },
|
||||
{ providers: ["openai", "openrouter"], stateless: true },
|
||||
{ providers: ["openai"], stateless: false },
|
||||
])("preserves MCP context with providers=$providers", async ({ providers, stateless }) => {
|
||||
const event: MCPEvent = {
|
||||
type: "response.output_item.done",
|
||||
item: {
|
||||
type: "mcp_call",
|
||||
id: "mcp-inventory",
|
||||
name: "lookup_inventory",
|
||||
arguments: "{}",
|
||||
output: '{"quantity":17}',
|
||||
},
|
||||
};
|
||||
vi.mocked(fetchAvailableModels).mockResolvedValue([{ model_group: "gpt-5.4-mini", providers }]);
|
||||
mockMakeOpenAIResponsesRequest.mockImplementationOnce(
|
||||
async (...args: Parameters<typeof makeOpenAIResponsesRequest>) => {
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
args[1]("assistant", "Lookup completed");
|
||||
args[15]?.("response-first");
|
||||
args[16]?.(event);
|
||||
},
|
||||
);
|
||||
mockMakeOpenAIResponsesRequest.mockResolvedValue(undefined);
|
||||
|
||||
await sendOneMessage();
|
||||
expect(await screen.findByText("Lookup completed")).toBeVisible();
|
||||
fireEvent.change(screen.getByRole("textbox"), { target: { value: "What was the quantity?" } });
|
||||
await waitFor(() => expect(screen.getByRole("button", { name: "Send" })).toBeEnabled());
|
||||
fireEvent.click(screen.getByRole("button", { name: "Send" }));
|
||||
await waitFor(() => expect(mockMakeOpenAIResponsesRequest).toHaveBeenCalledTimes(2));
|
||||
|
||||
const second = mockMakeOpenAIResponsesRequest.mock.calls[1];
|
||||
expect(second[14]).toBe(stateless ? null : "response-first");
|
||||
expect(second[0]).toEqual(
|
||||
stateless
|
||||
? [
|
||||
{ role: "user", content: "How much did this cost?" },
|
||||
{ role: "assistant", content: "Lookup completed", mcpEvents: [event] },
|
||||
{ role: "user", content: "What was the quantity?" },
|
||||
]
|
||||
: [{ role: "user", content: "What was the quantity?" }],
|
||||
);
|
||||
|
||||
fireEvent.mouseOver(screen.getByText("What was the quantity?"));
|
||||
fireEvent.click(await screen.findByRole("button", { name: "Edit message" }));
|
||||
fireEvent.change(screen.getByRole("textbox", { name: "Edit message" }), {
|
||||
target: { value: "Repeat the inventory quantity" },
|
||||
});
|
||||
fireEvent.click(screen.getByRole("button", { name: "Save & Send" }));
|
||||
await waitFor(() => expect(mockMakeOpenAIResponsesRequest).toHaveBeenCalledTimes(3));
|
||||
|
||||
const edited = mockMakeOpenAIResponsesRequest.mock.calls[2];
|
||||
expect(edited[14]).toBeNull();
|
||||
expect(edited[0]).toEqual([
|
||||
{ role: "user", content: "How much did this cost?" },
|
||||
{ role: "assistant", content: "Lookup completed", mcpEvents: [event] },
|
||||
{ role: "user", content: "Repeat the inventory quantity" },
|
||||
]);
|
||||
});
|
||||
|
||||
it("resends history instead of reusing another model's response ID after changing models", async () => {
|
||||
vi.mocked(fetchAvailableModels).mockResolvedValue([
|
||||
{ model_group: "gpt-5.4-mini", providers: ["openai"] },
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@ import { getChatRoutes } from "@/components/chat/ChatShell";
|
|||
import ChatMessages from "@/components/chat/ChatMessages";
|
||||
import MCPConnectPicker from "@/components/chat/MCPConnectPicker";
|
||||
import { fetchAvailableModels } from "@/components/llm_calls/fetch_models";
|
||||
import { makeOpenAIResponsesRequest } from "@/components/llm_calls/responses_api";
|
||||
import { makeOpenAIResponsesRequest, type ResponsesMessage } from "@/components/llm_calls/responses_api";
|
||||
import type { TokenUsage } from "@/components/chat_ui/ResponseMetrics";
|
||||
import type { MCPEvent } from "@/components/chat/types";
|
||||
import { getProviderLogoAndName } from "@/components/provider_info_helpers";
|
||||
|
|
@ -150,7 +150,7 @@ export default function ChatConversationPage() {
|
|||
}, []);
|
||||
|
||||
const handleSend = useCallback(
|
||||
async (text: string, historyOverride?: Array<{ role: "user" | "assistant"; content: string }>) => {
|
||||
async (text: string, historyOverride?: ResponsesMessage[]) => {
|
||||
const trimmed = text.trim();
|
||||
if (!trimmed || !selectedModel || isStreaming) return;
|
||||
const model = selectedModel;
|
||||
|
|
@ -181,7 +181,7 @@ export default function ChatConversationPage() {
|
|||
|
||||
const previousResponseId = historyOverride || statelessModelGroups.has(model) ? null : responsesSessionId;
|
||||
|
||||
const history: Array<{ role: "user" | "assistant"; content: string }> = historyOverride
|
||||
const history: ResponsesMessage[] = historyOverride
|
||||
? [...historyOverride, { role: "user" as const, content: trimmed }]
|
||||
: previousResponseId
|
||||
? [{ role: "user" as const, content: trimmed }]
|
||||
|
|
@ -192,7 +192,7 @@ export default function ChatConversationPage() {
|
|||
.filter(
|
||||
(m): m is typeof m & { role: "user" | "assistant" } => m.role === "user" || m.role === "assistant",
|
||||
)
|
||||
.map((m) => ({ role: m.role, content: m.content })),
|
||||
.map((m) => ({ role: m.role, content: m.content, mcpEvents: m.mcpEvents })),
|
||||
{ role: "user" as const, content: trimmed },
|
||||
];
|
||||
|
||||
|
|
@ -296,7 +296,7 @@ export default function ChatConversationPage() {
|
|||
const idx = msgs.findIndex((m) => m.id === messageId);
|
||||
const priorMessages = (idx === -1 ? msgs : msgs.slice(0, idx))
|
||||
.filter((m) => m.role === "user" || m.role === "assistant")
|
||||
.map((m) => ({ role: m.role as "user" | "assistant", content: m.content }));
|
||||
.map((m) => ({ role: m.role, content: m.content, mcpEvents: m.mcpEvents }));
|
||||
truncateFromMessage(activeConversationId, messageId);
|
||||
handleSend(newContent, priorMessages);
|
||||
},
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ import openai from "openai";
|
|||
import { makeOpenAIResponsesRequest } from "./responses_api";
|
||||
import { MessageType } from "../chat_ui/types";
|
||||
import type { TokenUsage } from "../chat_ui/ResponseMetrics";
|
||||
import type { MCPEvent } from "../mcp_tools/types";
|
||||
|
||||
vi.mock("@/components/networking", () => ({
|
||||
getProxyBaseUrl: vi.fn(() => "https://backend.example.com"),
|
||||
|
|
@ -79,6 +80,74 @@ describe("responses_api", () => {
|
|||
expect(mockUpdateTextUI).toHaveBeenCalledWith("assistant", "Hi", "gpt-4");
|
||||
});
|
||||
|
||||
it("replays completed MCP calls and results in conversation order without partial or discovery events", async () => {
|
||||
const item = {
|
||||
type: "mcp_call",
|
||||
id: "mcp-inventory",
|
||||
name: "lookup_inventory",
|
||||
arguments: '{"location":"warehouse"}',
|
||||
output: '{"quantity":17}',
|
||||
};
|
||||
const events: MCPEvent[] = [
|
||||
{ type: "response.output_item.done", item: { type: "mcp_list_tools", tools: [] } },
|
||||
{ type: "response.output_item.added", item },
|
||||
{ type: "response.mcp_call.in_progress", item_id: item.id },
|
||||
{ type: "response.output_item.done", item },
|
||||
{
|
||||
type: "response.output_item.done",
|
||||
item_id: "mcp-empty",
|
||||
item: { ...item, id: undefined, output: "" },
|
||||
},
|
||||
{ type: "response.output_item.done", item: { ...item, id: undefined } },
|
||||
{ type: "response.output_item.done", item: { ...item, name: undefined } },
|
||||
{ type: "response.output_item.done", item: { ...item, arguments: undefined } },
|
||||
{ type: "response.output_item.done", item: { ...item, output: undefined } },
|
||||
];
|
||||
const originalEvents = structuredClone(events);
|
||||
const history = [
|
||||
{ role: "user", content: "Check inventory", mcpEvents: events },
|
||||
{ role: "assistant", content: "Lookup completed", mcpEvents: events },
|
||||
{ role: "user", content: "What was the quantity?" },
|
||||
];
|
||||
|
||||
await makeOpenAIResponsesRequest(history, mockUpdateTextUI, "model", "test-token");
|
||||
|
||||
expect(mockResponsesCreate.mock.calls[0][0].input).toEqual([
|
||||
{ role: "user", content: "Check inventory", type: "message" },
|
||||
{ type: "function_call", call_id: item.id, name: item.name, arguments: item.arguments },
|
||||
{ type: "function_call_output", call_id: item.id, output: item.output },
|
||||
{ type: "function_call", call_id: "mcp-empty", name: item.name, arguments: item.arguments },
|
||||
{ type: "function_call_output", call_id: "mcp-empty", output: "" },
|
||||
{ role: "assistant", content: "Lookup completed", type: "message" },
|
||||
{ role: "user", content: "What was the quantity?", type: "message" },
|
||||
]);
|
||||
expect(events).toEqual(originalEvents);
|
||||
});
|
||||
|
||||
it("replays an MCP-only assistant turn without an empty message", async () => {
|
||||
const history = [
|
||||
{
|
||||
role: "assistant",
|
||||
content: "",
|
||||
mcpEvents: [
|
||||
{
|
||||
type: "response.output_item.done",
|
||||
item: { type: "mcp_call", id: "mcp-only", name: "lookup", arguments: "{}", output: "17" },
|
||||
},
|
||||
],
|
||||
},
|
||||
{ role: "user", content: "What was the quantity?" },
|
||||
];
|
||||
|
||||
await makeOpenAIResponsesRequest(history, mockUpdateTextUI, "model", "test-token");
|
||||
|
||||
expect(mockResponsesCreate.mock.calls[0][0].input).toEqual([
|
||||
{ type: "function_call", call_id: "mcp-only", name: "lookup", arguments: "{}" },
|
||||
{ type: "function_call_output", call_id: "mcp-only", output: "17" },
|
||||
{ role: "user", content: "What was the quantity?", type: "message" },
|
||||
]);
|
||||
});
|
||||
|
||||
it("should send a non-streaming request and render the whole output at once when streaming is disabled", async () => {
|
||||
mockResponsesCreate.mockReturnValueOnce(
|
||||
nonStreamingResponse({
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import openai from "openai";
|
||||
import type { ResponseFunctionToolCall, ResponseInputItem } from "openai/resources/responses/responses";
|
||||
import { MessageType } from "../chat_ui/types";
|
||||
import { TokenUsage } from "../chat_ui/ResponseMetrics";
|
||||
import { getGatewayBaseUrl, getProxyBaseUrl } from "@/components/networking";
|
||||
|
|
@ -17,6 +18,10 @@ import {
|
|||
|
||||
export type { CodeInterpreterResult } from "./code_interpreter_handler";
|
||||
|
||||
export interface ResponsesMessage extends MessageType {
|
||||
mcpEvents?: MCPEvent[];
|
||||
}
|
||||
|
||||
interface ResponseOutputPart {
|
||||
type?: string;
|
||||
text?: string;
|
||||
|
|
@ -61,7 +66,7 @@ const responseAsEvents = (response: NonStreamedResponse): SynthesizedResponseEve
|
|||
};
|
||||
|
||||
export async function makeOpenAIResponsesRequest(
|
||||
messages: MessageType[],
|
||||
messages: ResponsesMessage[],
|
||||
updateTextUI: (role: string, delta: string, model?: string) => void,
|
||||
selectedModel: string,
|
||||
accessToken: string | null,
|
||||
|
|
@ -118,22 +123,32 @@ export async function makeOpenAIResponsesRequest(
|
|||
let firstTokenReceived = false;
|
||||
let servedFromResponseCache = false;
|
||||
|
||||
// Format messages for the API
|
||||
const formattedInput = messages.map((message) => {
|
||||
// If content is already an array (multimodal), use it directly
|
||||
if (Array.isArray(message.content)) {
|
||||
return {
|
||||
role: message.role,
|
||||
content: message.content,
|
||||
type: "message",
|
||||
};
|
||||
}
|
||||
// Otherwise, wrap text content in the expected format
|
||||
return {
|
||||
role: message.role,
|
||||
content: message.content,
|
||||
type: "message",
|
||||
};
|
||||
const formattedInput = messages.flatMap((message) => {
|
||||
const mcpInput =
|
||||
message.role === "assistant"
|
||||
? (message.mcpEvents ?? []).flatMap<ResponseFunctionToolCall | ResponseInputItem.FunctionCallOutput>(
|
||||
(event) => {
|
||||
const item = event.item;
|
||||
const callId = item?.id || event.item_id;
|
||||
if (event.type !== "response.output_item.done" || item?.type !== "mcp_call" || !callId) {
|
||||
return [];
|
||||
}
|
||||
if (!item.name || typeof item.arguments !== "string" || typeof item.output !== "string") {
|
||||
return [];
|
||||
}
|
||||
return [
|
||||
{ type: "function_call", call_id: callId, name: item.name, arguments: item.arguments },
|
||||
{ type: "function_call_output", call_id: callId, output: item.output },
|
||||
];
|
||||
},
|
||||
)
|
||||
: [];
|
||||
return [
|
||||
...mcpInput,
|
||||
...(mcpInput.length > 0 && message.content === ""
|
||||
? []
|
||||
: [{ role: message.role, content: message.content, type: "message" }]),
|
||||
];
|
||||
});
|
||||
|
||||
// Build tools array
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue