fix: address message history review feedback

Author: erichare

src/backend/tests/unit/test_messages_endpoints.py

@@ -210,7 +210,6 @@ async def test_get_messages_does_not_return_other_users_messages(
     assert str(cross_user_messages["owned_message"].id) not in other_returned_ids
 
 
-@pytest.mark.api_key_required
 @pytest.mark.usefixtures("timestamped_messages")
 async def test_get_messages_defaults_to_timestamp_ascending(client: AsyncClient, logged_in_headers):
     response = await client.get(
@@ -223,7 +222,6 @@ async def test_get_messages_defaults_to_timestamp_ascending(client: AsyncClient,
     assert [message["text"] for message in response.json()] == ["Message 0", "Message 1", "Message 2"]
 
 
-@pytest.mark.api_key_required
 @pytest.mark.usefixtures("timestamped_messages")
 async def test_get_messages_supports_descending_order_with_pagination(client: AsyncClient, logged_in_headers):
     response = await client.get(
@@ -236,7 +234,6 @@ async def test_get_messages_supports_descending_order_with_pagination(client: As
     assert [message["text"] for message in response.json()] == ["Message 1", "Message 0"]
 
 
-@pytest.mark.api_key_required
 @pytest.mark.usefixtures("timestamped_messages")
 async def test_get_messages_rejects_invalid_order_direction(client: AsyncClient, logged_in_headers):
     response = await client.get(

src/frontend/src/components/core/playgroundComponent/chat-view/chat-messages/hooks/__tests__/use-chat-history.test.ts

@@ -49,7 +49,7 @@ describe("useChatHistory message ordering", () => {
       {
         id: "flow-1",
         mode: "union",
-        params: { limit: 20, order: "DESC" },
+        params: { session_id: "session-1", limit: 20, order: "DESC" },
       },
       { enabled: true },
     );
@@ -74,4 +74,33 @@ describe("useChatHistory message ordering", () => {
       offset: 0,
     });
   });
+
+  it("stops loading when a source repeats the same page", async () => {
+    const queryClient = new QueryClient({
+      defaultOptions: { queries: { retry: false } },
+    });
+    const repeatedPage = Array.from({ length: 20 }, (_, index) => ({
+      id: `message-${index}`,
+    }));
+    const sessionCacheKey = [
+      "useGetMessagesQuery",
+      { id: "flow-1", session_id: "session-1" },
+    ];
+    queryClient.setQueryData(sessionCacheKey, repeatedPage);
+    mockGetMessages
+      .mockResolvedValueOnce({ data: repeatedPage })
+      .mockResolvedValueOnce({ data: repeatedPage });
+
+    const { result } = renderHook(() => useChatHistory("session-1"), {
+      wrapper: createWrapper(queryClient),
+    });
+
+    let prepended = -1;
+    await act(async () => {
+      prepended = await result.current.loadMore();
+    });
+
+    expect(prepended).toBe(0);
+    expect(mockGetMessages).toHaveBeenCalledTimes(2);
+  });
 });

src/frontend/src/components/core/playgroundComponent/chat-view/chat-messages/hooks/use-chat-history.ts

@@ -34,7 +34,11 @@ export const useChatHistory = (visibleSession: string | null) => {
   const messageQueryParams: Parameters<typeof useGetMessagesQuery>[0] = {
     id: currentFlowId,
     mode: "union",
-    params: { limit: 20, order: "DESC" },
+    params: {
+      ...(visibleSession ? { session_id: visibleSession } : {}),
+      limit: 20,
+      order: "DESC",
+    },
   };
   const { data: queryData } = useGetMessagesQuery(messageQueryParams, {
     enabled: isPlaygroundOpen,
@@ -97,6 +101,7 @@ export const useChatHistory = (visibleSession: string | null) => {
       // remount offsetRef restarts at 0 while the cache may already hold the
       // early pages, and dedup would otherwise swallow them and stall.
       let prepended = 0;
+      const seenPageSignatures = new Set<string>();
       while (prepended === 0) {
         const response = await getMessages(currentFlowId, {
           ...(visibleSession ? { session_id: visibleSession } : {}),
@@ -105,6 +110,26 @@ export const useChatHistory = (visibleSession: string | null) => {
           offset: offsetRef.current,
         });
         const olderMessages: Message[] = response.data || [];
+        if (olderMessages.length > 0) {
+          const pageSignature = JSON.stringify(
+            olderMessages.map((message) =>
+              message.id
+                ? message.id
+                : [
+                    message.flow_id,
+                    message.session_id,
+                    message.timestamp,
+                    message.sender,
+                    message.text,
+                  ],
+            ),
+          );
+          if (seenPageSignatures.has(pageSignature)) {
+            setHasMore(false);
+            break;
+          }
+          seenPageSignatures.add(pageSignature);
+        }
         offsetRef.current += olderMessages.length;
 
         const exhausted = olderMessages.length < 20;

src/frontend/src/controllers/API/queries/messages/__tests__/use-get-messages-routing.test.ts

@@ -7,6 +7,8 @@
  * - sessionStorage (anonymous/auto-login shareable playground)
  */
 
+import { renderHook } from "@testing-library/react";
+
 const FLOW_ID = "virtual-flow-id-123";
 const SOURCE_FLOW_ID = "real-flow-id-456";
 const MOCK_MESSAGES = [
@@ -51,6 +53,7 @@ jest.mock("@/modals/IOModal/helpers/playground-auth", () => ({
 }));
 
 const mockApiGet = jest.fn();
+const mockQueryKeys: unknown[] = [];
 jest.mock("@/controllers/API/api", () => ({
   api: { get: (...args: unknown[]) => mockApiGet(...args) },
 }));
@@ -61,12 +64,13 @@ jest.mock("@/controllers/API/helpers/constants", () => ({
 
 jest.mock("@/utils/utils", () => ({
   extractColumnsFromRows: jest.fn(() => []),
-  prepareSessionIdForAPI: (id: string) => id,
+  prepareSessionIdForAPI: (id: string) => encodeURIComponent(id),
 }));
 
 jest.mock("@/controllers/API/services/request-processor", () => ({
   UseRequestProcessor: jest.fn(() => ({
-    query: jest.fn((_key: unknown, fn: () => Promise<unknown>) => {
+    query: jest.fn((key: unknown, fn: () => Promise<unknown>) => {
+      mockQueryKeys.push(key);
       void fn();
       return { data: null, isFetched: false, refetch: jest.fn() };
     }),
@@ -76,7 +80,7 @@ jest.mock("@/controllers/API/services/request-processor", () => ({
 import { isAuthenticatedPlayground } from "@/modals/IOModal/helpers/playground-auth";
 import useFlowStore from "@/stores/flowStore";
 import { useMessagesStore } from "@/stores/messagesStore";
-import { useGetMessagesQuery } from "../use-get-messages";
+import { getMessages, useGetMessagesQuery } from "../use-get-messages";
 
 const mockFlowStore = useFlowStore as unknown as { getState: jest.Mock };
 const mockIsAuth = isAuthenticatedPlayground as jest.MockedFunction<
@@ -89,6 +93,7 @@ describe("useGetMessagesQuery - Routing Logic", () => {
     mockApiGet.mockResolvedValue({ data: MOCK_MESSAGES });
     mockFlowStore.getState.mockReturnValue({ playgroundPage: false });
     mockIsAuth.mockReturnValue(false);
+    mockQueryKeys.length = 0;
     window.sessionStorage.clear();
   });
 
@@ -172,6 +177,86 @@ describe("useGetMessagesQuery - Routing Logic", () => {
     expect(mockApiGet).not.toHaveBeenCalled();
   });
 
+  it("should_filter_order_and_paginate_anonymous_sessionStorage", async () => {
+    mockFlowStore.getState.mockReturnValue({ playgroundPage: true });
+    mockIsAuth.mockReturnValue(false);
+    window.sessionStorage.setItem(
+      FLOW_ID,
+      JSON.stringify([
+        {
+          id: "oldest",
+          session_id: "session one",
+          timestamp: "2026-01-01T00:00:00Z",
+        },
+        {
+          id: "other-session",
+          session_id: "session-two",
+          timestamp: "2026-01-01T00:03:00Z",
+        },
+        {
+          id: "newest",
+          session_id: "session one",
+          timestamp: "2026-01-01T00:02:00Z",
+        },
+        {
+          id: "middle",
+          session_id: "session one",
+          timestamp: "2026-01-01T00:01:00Z",
+        },
+      ]),
+    );
+
+    const response = await getMessages(FLOW_ID, {
+      session_id: "session one",
+      order: "DESC",
+      offset: 1,
+      limit: 1,
+    });
+
+    expect(response.data.map((message) => message.id)).toEqual(["middle"]);
+    expect(mockApiGet).not.toHaveBeenCalled();
+  });
+
+  it("uses a distinct query key when request params change", () => {
+    const { rerender } = renderHook(
+      ({ sessionId }) =>
+        useGetMessagesQuery(
+          {
+            id: FLOW_ID,
+            mode: "union",
+            excludedFields: ["properties"],
+            params: { session_id: sessionId, limit: 20, order: "DESC" },
+          },
+          {},
+        ),
+      { initialProps: { sessionId: "session-1" } },
+    );
+    const firstKey = mockQueryKeys[mockQueryKeys.length - 1];
+
+    rerender({ sessionId: "session-2" });
+    const secondKey = mockQueryKeys[mockQueryKeys.length - 1];
+
+    expect(firstKey).toEqual([
+      "useGetMessagesQuery",
+      {
+        id: FLOW_ID,
+        mode: "union",
+        excludedFields: ["properties"],
+        params: { session_id: "session-1", limit: 20, order: "DESC" },
+      },
+    ]);
+    expect(secondKey).toEqual([
+      "useGetMessagesQuery",
+      {
+        id: FLOW_ID,
+        mode: "union",
+        excludedFields: ["properties"],
+        params: { session_id: "session-2", limit: 20, order: "DESC" },
+      },
+    ]);
+    expect(secondKey).not.toEqual(firstKey);
+  });
+
   it("should_sync_messages_to_zustand_store_when_authenticated_playground", async () => {
     mockFlowStore.getState.mockReturnValue({ playgroundPage: true });
     mockIsAuth.mockReturnValue(true);

src/frontend/src/controllers/API/queries/messages/use-get-messages.ts

@@ -26,6 +26,42 @@ interface MessagesResponse {
   columns: Array<ColDef | ColGroupDef>;
 }
 
+const getStoredMessages = (
+  id: string | undefined,
+  params: Record<string, unknown>,
+): Message[] => {
+  const storedMessages = JSON.parse(
+    window.sessionStorage.getItem(id ?? "") || "[]",
+  ) as Message[];
+  const sessionId =
+    typeof params.session_id === "string" ? params.session_id : undefined;
+  const direction =
+    typeof params.order === "string" && params.order.toUpperCase() === "DESC"
+      ? -1
+      : 1;
+  const offset = typeof params.offset === "number" ? params.offset : 0;
+  const limit = typeof params.limit === "number" ? params.limit : undefined;
+
+  const filteredMessages = sessionId
+    ? storedMessages.filter(
+        (message) =>
+          message.session_id === sessionId ||
+          (sessionId === id && !message.session_id),
+      )
+    : storedMessages;
+  const orderedMessages = [...filteredMessages].sort((a, b) => {
+    const timeA = new Date(a.timestamp).getTime();
+    const timeB = new Date(b.timestamp).getTime();
+    if (Number.isNaN(timeA) || Number.isNaN(timeB)) return 0;
+    return direction * (timeA - timeB);
+  });
+
+  return orderedMessages.slice(
+    offset,
+    limit === undefined ? undefined : offset + limit,
+  );
+};
+
 export const getMessages = async (
   id?: string,
   params: Record<string, unknown> = {},
@@ -61,7 +97,7 @@ export const getMessages = async (
   }
 
   return {
-    data: JSON.parse(window.sessionStorage.getItem(id ?? "") || "[]"),
+    data: getStoredMessages(id, params),
   };
 };
 
@@ -91,10 +127,14 @@ export const useGetMessagesQuery: useQueryFunctionType<
     return { rows: data, columns };
   };
 
-  const queryResult = query(["useGetMessagesQuery", { id }], responseFn, {
-    placeholderData: keepPreviousData,
-    ...options,
-  });
+  const queryResult = query(
+    ["useGetMessagesQuery", { id, mode, excludedFields, params }],
+    responseFn,
+    {
+      placeholderData: keepPreviousData,
+      ...options,
+    },
+  );
 
   return queryResult;
 };