mirror of
https://github.com/OpenHands/OpenHands.git
synced 2025-12-26 05:48:36 +08:00
chore(frontend): Add history loading state for V1 conversations (#11536)
This commit is contained in:
parent
6630d5dc4e
commit
704fc6dd69
@ -1,6 +1,7 @@
|
||||
import { describe, it, expect, beforeAll, afterAll, afterEach } from "vitest";
|
||||
import { screen, waitFor, render, cleanup } from "@testing-library/react";
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
import { http, HttpResponse } from "msw";
|
||||
import { useOptimisticUserMessageStore } from "#/stores/optimistic-user-message-store";
|
||||
import {
|
||||
createMockMessageEvent,
|
||||
@ -13,8 +14,12 @@ import {
|
||||
OptimisticUserMessageStoreComponent,
|
||||
ErrorMessageStoreComponent,
|
||||
} from "./helpers/websocket-test-components";
|
||||
import { ConversationWebSocketProvider } from "#/contexts/conversation-websocket-context";
|
||||
import {
|
||||
ConversationWebSocketProvider,
|
||||
useConversationWebSocket,
|
||||
} from "#/contexts/conversation-websocket-context";
|
||||
import { conversationWebSocketTestSetup } from "./helpers/msw-websocket-setup";
|
||||
import { useEventStore } from "#/stores/use-event-store";
|
||||
|
||||
// MSW WebSocket mock setup
|
||||
const { wsLink, server: mswServer } = conversationWebSocketTestSetup();
|
||||
@ -417,7 +422,206 @@ describe("Conversation WebSocket Handler", () => {
|
||||
it.todo("should handle send attempts when disconnected");
|
||||
});
|
||||
|
||||
// 8. Terminal I/O Tests (ExecuteBashAction and ExecuteBashObservation)
|
||||
// 8. History Loading State Tests
|
||||
describe("History Loading State", () => {
|
||||
it("should track history loading state using event count from API", async () => {
|
||||
const conversationId = "test-conversation-with-history";
|
||||
|
||||
// Mock the event count API to return 3 events
|
||||
const expectedEventCount = 3;
|
||||
|
||||
// Create 3 mock events to simulate history
|
||||
const mockHistoryEvents = [
|
||||
createMockUserMessageEvent({ id: "history-event-1" }),
|
||||
createMockMessageEvent({ id: "history-event-2" }),
|
||||
createMockMessageEvent({ id: "history-event-3" }),
|
||||
];
|
||||
|
||||
// Set up MSW to mock both the HTTP API and WebSocket connection
|
||||
mswServer.use(
|
||||
http.get("/api/v1/events/count", ({ request }) => {
|
||||
const url = new URL(request.url);
|
||||
const conversationIdParam = url.searchParams.get(
|
||||
"conversation_id__eq",
|
||||
);
|
||||
|
||||
if (conversationIdParam === conversationId) {
|
||||
return HttpResponse.json(expectedEventCount);
|
||||
}
|
||||
|
||||
return HttpResponse.json(0);
|
||||
}),
|
||||
wsLink.addEventListener("connection", ({ client, server }) => {
|
||||
server.connect();
|
||||
// Send all history events
|
||||
mockHistoryEvents.forEach((event) => {
|
||||
client.send(JSON.stringify(event));
|
||||
});
|
||||
}),
|
||||
);
|
||||
|
||||
// Create a test component that displays loading state
|
||||
const HistoryLoadingComponent = () => {
|
||||
const context = useConversationWebSocket();
|
||||
const { events } = useEventStore();
|
||||
|
||||
return (
|
||||
<div>
|
||||
<div data-testid="is-loading-history">
|
||||
{context?.isLoadingHistory ? "true" : "false"}
|
||||
</div>
|
||||
<div data-testid="events-received">{events.length}</div>
|
||||
<div data-testid="expected-event-count">{expectedEventCount}</div>
|
||||
</div>
|
||||
);
|
||||
};
|
||||
|
||||
// Render with WebSocket context
|
||||
renderWithWebSocketContext(
|
||||
<HistoryLoadingComponent />,
|
||||
conversationId,
|
||||
`http://localhost:3000/api/conversations/${conversationId}`,
|
||||
);
|
||||
|
||||
// Initially should be loading history
|
||||
expect(screen.getByTestId("is-loading-history")).toHaveTextContent("true");
|
||||
|
||||
// Wait for all events to be received
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("events-received")).toHaveTextContent("3");
|
||||
});
|
||||
|
||||
// Once all events are received, loading should be complete
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("is-loading-history")).toHaveTextContent(
|
||||
"false",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
it("should handle empty conversation history", async () => {
|
||||
const conversationId = "test-conversation-empty";
|
||||
|
||||
// Set up MSW to mock both the HTTP API and WebSocket connection
|
||||
mswServer.use(
|
||||
http.get("/api/v1/events/count", ({ request }) => {
|
||||
const url = new URL(request.url);
|
||||
const conversationIdParam = url.searchParams.get(
|
||||
"conversation_id__eq",
|
||||
);
|
||||
|
||||
if (conversationIdParam === conversationId) {
|
||||
return HttpResponse.json(0);
|
||||
}
|
||||
|
||||
return HttpResponse.json(0);
|
||||
}),
|
||||
wsLink.addEventListener("connection", ({ server }) => {
|
||||
server.connect();
|
||||
// No events sent for empty history
|
||||
}),
|
||||
);
|
||||
|
||||
// Create a test component that displays loading state
|
||||
const HistoryLoadingComponent = () => {
|
||||
const context = useConversationWebSocket();
|
||||
|
||||
return (
|
||||
<div>
|
||||
<div data-testid="is-loading-history">
|
||||
{context?.isLoadingHistory ? "true" : "false"}
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
};
|
||||
|
||||
// Render with WebSocket context
|
||||
renderWithWebSocketContext(
|
||||
<HistoryLoadingComponent />,
|
||||
conversationId,
|
||||
`http://localhost:3000/api/conversations/${conversationId}`,
|
||||
);
|
||||
|
||||
// Should quickly transition from loading to not loading when count is 0
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("is-loading-history")).toHaveTextContent(
|
||||
"false",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
it("should handle history loading with large event count", async () => {
|
||||
const conversationId = "test-conversation-large-history";
|
||||
|
||||
// Create 50 mock events to simulate large history
|
||||
const expectedEventCount = 50;
|
||||
const mockHistoryEvents = Array.from({ length: 50 }, (_, i) =>
|
||||
createMockMessageEvent({ id: `history-event-${i + 1}` }),
|
||||
);
|
||||
|
||||
// Set up MSW to mock both the HTTP API and WebSocket connection
|
||||
mswServer.use(
|
||||
http.get("/api/v1/events/count", ({ request }) => {
|
||||
const url = new URL(request.url);
|
||||
const conversationIdParam = url.searchParams.get(
|
||||
"conversation_id__eq",
|
||||
);
|
||||
|
||||
if (conversationIdParam === conversationId) {
|
||||
return HttpResponse.json(expectedEventCount);
|
||||
}
|
||||
|
||||
return HttpResponse.json(0);
|
||||
}),
|
||||
wsLink.addEventListener("connection", ({ client, server }) => {
|
||||
server.connect();
|
||||
// Send all history events
|
||||
mockHistoryEvents.forEach((event) => {
|
||||
client.send(JSON.stringify(event));
|
||||
});
|
||||
}),
|
||||
);
|
||||
|
||||
// Create a test component that displays loading state
|
||||
const HistoryLoadingComponent = () => {
|
||||
const context = useConversationWebSocket();
|
||||
const { events } = useEventStore();
|
||||
|
||||
return (
|
||||
<div>
|
||||
<div data-testid="is-loading-history">
|
||||
{context?.isLoadingHistory ? "true" : "false"}
|
||||
</div>
|
||||
<div data-testid="events-received">{events.length}</div>
|
||||
</div>
|
||||
);
|
||||
};
|
||||
|
||||
// Render with WebSocket context
|
||||
renderWithWebSocketContext(
|
||||
<HistoryLoadingComponent />,
|
||||
conversationId,
|
||||
`http://localhost:3000/api/conversations/${conversationId}`,
|
||||
);
|
||||
|
||||
// Initially should be loading history
|
||||
expect(screen.getByTestId("is-loading-history")).toHaveTextContent("true");
|
||||
|
||||
// Wait for all events to be received
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("events-received")).toHaveTextContent("50");
|
||||
});
|
||||
|
||||
// Once all events are received, loading should be complete
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("is-loading-history")).toHaveTextContent(
|
||||
"false",
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// 9. Terminal I/O Tests (ExecuteBashAction and ExecuteBashObservation)
|
||||
describe("Terminal I/O Integration", () => {
|
||||
it("should append command to store when ExecuteBashAction event is received", async () => {
|
||||
const { createMockExecuteBashActionEvent } = await import(
|
||||
|
||||
@ -38,8 +38,7 @@ export const createWebSocketTestSetup = (
|
||||
/**
|
||||
* Standard WebSocket test setup for conversation WebSocket handler tests
|
||||
* Updated to use the V1 WebSocket URL pattern: /sockets/events/{conversationId}
|
||||
* Uses a wildcard pattern to match any conversation ID
|
||||
*/
|
||||
export const conversationWebSocketTestSetup = () =>
|
||||
createWebSocketTestSetup(
|
||||
"ws://localhost:3000/sockets/events/test-conversation-default",
|
||||
);
|
||||
createWebSocketTestSetup("ws://localhost:3000/sockets/events/*");
|
||||
|
||||
@ -345,6 +345,24 @@ class V1ConversationService {
|
||||
const { data } = await openHands.get<{ runtime_id: string }>(url);
|
||||
return data;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the count of events for a conversation
|
||||
* Uses the V1 API endpoint: GET /api/v1/events/count
|
||||
*
|
||||
* @param conversationId The conversation ID to get event count for
|
||||
* @returns The number of events in the conversation
|
||||
*/
|
||||
static async getEventCount(conversationId: string): Promise<number> {
|
||||
const params = new URLSearchParams();
|
||||
params.append("conversation_id__eq", conversationId);
|
||||
|
||||
const { data } = await openHands.get<number>(
|
||||
`/api/v1/events/count?${params.toString()}`,
|
||||
);
|
||||
|
||||
return data;
|
||||
}
|
||||
}
|
||||
|
||||
export default V1ConversationService;
|
||||
|
||||
@ -48,6 +48,7 @@ import {
|
||||
} from "#/types/v1/type-guards";
|
||||
import { useActiveConversation } from "#/hooks/query/use-active-conversation";
|
||||
import { useTaskPolling } from "#/hooks/query/use-task-polling";
|
||||
import { useConversationWebSocket } from "#/contexts/conversation-websocket-context";
|
||||
|
||||
function getEntryPoint(
|
||||
hasRepository: boolean | null,
|
||||
@ -64,6 +65,7 @@ export function ChatInterface() {
|
||||
const { errorMessage } = useErrorMessageStore();
|
||||
const { isLoadingMessages } = useWsClient();
|
||||
const { isTask } = useTaskPolling();
|
||||
const conversationWebSocket = useConversationWebSocket();
|
||||
const { send } = useSendMessage();
|
||||
const storeEvents = useEventStore((state) => state.events);
|
||||
const { setOptimisticUserMessage, getOptimisticUserMessage } =
|
||||
@ -94,6 +96,25 @@ export function ChatInterface() {
|
||||
|
||||
const isV1Conversation = conversation?.conversation_version === "V1";
|
||||
|
||||
// Instantly scroll to bottom when history loading completes
|
||||
const prevLoadingHistoryRef = React.useRef(
|
||||
conversationWebSocket?.isLoadingHistory,
|
||||
);
|
||||
React.useEffect(() => {
|
||||
const wasLoading = prevLoadingHistoryRef.current;
|
||||
const isLoading = conversationWebSocket?.isLoadingHistory;
|
||||
|
||||
// When history loading transitions from true to false, instantly scroll to bottom
|
||||
if (wasLoading && !isLoading && scrollRef.current) {
|
||||
scrollRef.current.scrollTo({
|
||||
top: scrollRef.current.scrollHeight,
|
||||
behavior: "instant",
|
||||
});
|
||||
}
|
||||
|
||||
prevLoadingHistoryRef.current = isLoading;
|
||||
}, [conversationWebSocket?.isLoadingHistory, scrollRef]);
|
||||
|
||||
// Filter V0 events
|
||||
const v0Events = storeEvents
|
||||
.filter(isV0Event)
|
||||
@ -228,6 +249,14 @@ export function ChatInterface() {
|
||||
</div>
|
||||
)}
|
||||
|
||||
{conversationWebSocket?.isLoadingHistory &&
|
||||
isV1Conversation &&
|
||||
!isTask && (
|
||||
<div className="flex justify-center">
|
||||
<LoadingSpinner size="small" />
|
||||
</div>
|
||||
)}
|
||||
|
||||
{!isLoadingMessages && v0UserEventsExist && (
|
||||
<V0Messages
|
||||
messages={v0Events}
|
||||
@ -237,7 +266,9 @@ export function ChatInterface() {
|
||||
/>
|
||||
)}
|
||||
|
||||
{v1UserEventsExist && <V1Messages messages={v1Events} />}
|
||||
{!conversationWebSocket?.isLoadingHistory && v1UserEventsExist && (
|
||||
<V1Messages messages={v1Events} />
|
||||
)}
|
||||
</div>
|
||||
|
||||
<div className="flex flex-col gap-[6px]">
|
||||
|
||||
@ -5,6 +5,7 @@ import React, {
|
||||
useState,
|
||||
useCallback,
|
||||
useMemo,
|
||||
useRef,
|
||||
} from "react";
|
||||
import { useQueryClient } from "@tanstack/react-query";
|
||||
import { useWebSocket, WebSocketHookOptions } from "#/hooks/use-websocket";
|
||||
@ -27,6 +28,7 @@ import {
|
||||
import { handleActionEventCacheInvalidation } from "#/utils/cache-utils";
|
||||
import { buildWebSocketUrl } from "#/utils/websocket-url";
|
||||
import type { V1SendMessageRequest } from "#/api/conversation-service/v1-conversation-service.types";
|
||||
import V1ConversationService from "#/api/conversation-service/v1-conversation-service.api";
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/naming-convention
|
||||
export type V1_WebSocketConnectionState =
|
||||
@ -38,6 +40,7 @@ export type V1_WebSocketConnectionState =
|
||||
interface ConversationWebSocketContextType {
|
||||
connectionState: V1_WebSocketConnectionState;
|
||||
sendMessage: (message: V1SendMessageRequest) => Promise<void>;
|
||||
isLoadingHistory: boolean;
|
||||
}
|
||||
|
||||
const ConversationWebSocketContext = createContext<
|
||||
@ -67,6 +70,13 @@ export function ConversationWebSocketProvider({
|
||||
const { setAgentStatus } = useV1ConversationStateStore();
|
||||
const { appendInput, appendOutput } = useCommandStore();
|
||||
|
||||
// History loading state
|
||||
const [isLoadingHistory, setIsLoadingHistory] = useState(true);
|
||||
const [expectedEventCount, setExpectedEventCount] = useState<number | null>(
|
||||
null,
|
||||
);
|
||||
const receivedEventCountRef = useRef(0);
|
||||
|
||||
// Build WebSocket URL from props
|
||||
// Only build URL if we have both conversationId and conversationUrl
|
||||
// This prevents connection attempts during task polling phase
|
||||
@ -78,16 +88,43 @@ export function ConversationWebSocketProvider({
|
||||
return buildWebSocketUrl(conversationId, conversationUrl);
|
||||
}, [conversationId, conversationUrl]);
|
||||
|
||||
// Reset hasConnected flag when conversation changes
|
||||
// Reset hasConnected flag and history loading state when conversation changes
|
||||
useEffect(() => {
|
||||
hasConnectedRef.current = false;
|
||||
setIsLoadingHistory(true);
|
||||
setExpectedEventCount(null);
|
||||
receivedEventCountRef.current = 0;
|
||||
}, [conversationId]);
|
||||
|
||||
// Check if we've received all events when expectedEventCount becomes available
|
||||
useEffect(() => {
|
||||
if (
|
||||
expectedEventCount !== null &&
|
||||
receivedEventCountRef.current >= expectedEventCount &&
|
||||
isLoadingHistory
|
||||
) {
|
||||
setIsLoadingHistory(false);
|
||||
}
|
||||
}, [expectedEventCount, isLoadingHistory]);
|
||||
|
||||
const handleMessage = useCallback(
|
||||
(messageEvent: MessageEvent) => {
|
||||
try {
|
||||
const event = JSON.parse(messageEvent.data);
|
||||
|
||||
// Track received events for history loading (count ALL events from WebSocket)
|
||||
// Always count when loading, even if we don't have the expected count yet
|
||||
if (isLoadingHistory) {
|
||||
receivedEventCountRef.current += 1;
|
||||
|
||||
if (
|
||||
expectedEventCount !== null &&
|
||||
receivedEventCountRef.current >= expectedEventCount
|
||||
) {
|
||||
setIsLoadingHistory(false);
|
||||
}
|
||||
}
|
||||
|
||||
// Use type guard to validate v1 event structure
|
||||
if (isV1Event(event)) {
|
||||
addEvent(event);
|
||||
@ -141,6 +178,8 @@ export function ConversationWebSocketProvider({
|
||||
},
|
||||
[
|
||||
addEvent,
|
||||
isLoadingHistory,
|
||||
expectedEventCount,
|
||||
setErrorMessage,
|
||||
removeOptimisticUserMessage,
|
||||
queryClient,
|
||||
@ -164,10 +203,27 @@ export function ConversationWebSocketProvider({
|
||||
return {
|
||||
queryParams,
|
||||
reconnect: { enabled: true },
|
||||
onOpen: () => {
|
||||
onOpen: async () => {
|
||||
setConnectionState("OPEN");
|
||||
hasConnectedRef.current = true; // Mark that we've successfully connected
|
||||
removeErrorMessage(); // Clear any previous error messages on successful connection
|
||||
|
||||
// Fetch expected event count for history loading detection
|
||||
if (conversationId) {
|
||||
try {
|
||||
const count =
|
||||
await V1ConversationService.getEventCount(conversationId);
|
||||
setExpectedEventCount(count);
|
||||
|
||||
// If no events expected, mark as loaded immediately
|
||||
if (count === 0) {
|
||||
setIsLoadingHistory(false);
|
||||
}
|
||||
} catch (error) {
|
||||
// Fall back to marking as loaded to avoid infinite loading state
|
||||
setIsLoadingHistory(false);
|
||||
}
|
||||
}
|
||||
},
|
||||
onClose: (event: CloseEvent) => {
|
||||
setConnectionState("CLOSED");
|
||||
@ -188,7 +244,13 @@ export function ConversationWebSocketProvider({
|
||||
},
|
||||
onMessage: handleMessage,
|
||||
};
|
||||
}, [handleMessage, setErrorMessage, removeErrorMessage, sessionApiKey]);
|
||||
}, [
|
||||
handleMessage,
|
||||
setErrorMessage,
|
||||
removeErrorMessage,
|
||||
sessionApiKey,
|
||||
conversationId,
|
||||
]);
|
||||
|
||||
// Only attempt WebSocket connection when we have a valid URL
|
||||
// This prevents connection attempts during task polling phase
|
||||
@ -246,8 +308,8 @@ export function ConversationWebSocketProvider({
|
||||
}, [socket, wsUrl]);
|
||||
|
||||
const contextValue = useMemo(
|
||||
() => ({ connectionState, sendMessage }),
|
||||
[connectionState, sendMessage],
|
||||
() => ({ connectionState, sendMessage, isLoadingHistory }),
|
||||
[connectionState, sendMessage, isLoadingHistory],
|
||||
);
|
||||
|
||||
return (
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user