ronantakizawa/codereview-bench huggingface dataset:
Repository: All-Hands-AI/OpenHands
File: frontend/src/components/features/diff-viewer/empty-changes-message.tsx (TypeScript)

Code before:
import { useTranslation } from "react-i18next";
import { FaCodeCompare } from "react-icons/fa6";
import { I18nKey } from "#/i18n/declaration";

export function EmptyChangesMessage() {
  const { t } = useTranslation();

  return (
    <div className="flex flex-col items-center justify-center w-full h-full p-10 gap-4">
      <FaCodeCompare size={113} />
      <span className="text-[#8D95A9] text-[19px] font-normal leading-5">
        {t(I18nKey.DIFF_VIEWER$NO_CHANGES)}
      </span>
    </div>
  );
}


Review comment (suggestion):
```suggestion
      <FaCodeCompare size={100} className="text-[#A1A1A1]" />
```

Code after:
import { useTranslation } from "react-i18next";
import { FaCodeCompare } from "react-icons/fa6";
import { I18nKey } from "#/i18n/declaration";

export function EmptyChangesMessage() {
  const { t } = useTranslation();

  return (
    <div className="flex flex-col items-center justify-center w-full h-full p-10 gap-4">
      <FaCodeCompare size={100} className="text-[#A1A1A1]" />
      <span className="text-[#8D95A9] text-[19px] font-normal leading-5">
        {t(I18nKey.DIFF_VIEWER$NO_CHANGES)}
      </span>
    </div>
  );
}
Repository: All-Hands-AI/OpenHands
File: frontend/src/utils/conversation-local-storage.ts (TypeScript)

Code before:
const CONVERSATION_STORAGE_KEYS = [
  "conversation-selected-tab-",
  "conversation-right-panel-shown-",
  "conversation-unpinned-tabs-",
];

export function clearConversationLocalStorage(conversationId: string) {
  try {
    CONVERSATION_STORAGE_KEYS.forEach((prefix) => {
      localStorage.removeItem(`${prefix}${conversationId}`);
    });
  } catch (err) {
    console.warn(
      "Failed to clear conversation localStorage",
      conversationId,
      err,
    );
  }
}

export function cleanupOrphanedConversationLocalStorage(
  existingConversationIds: string[],
) {
  try {
    const validIds = new Set(existingConversationIds);

    Object.keys(localStorage).forEach((key) => {
      const match = key.match(
        /^conversation-(selected-tab|right-panel-shown|unpinned-tabs)-(.+)$/,
      );

Review comment (suggestion):
**suggestion:** It appears these strings are used in multiple places within the codebase. We could consider defining them as shared constants to improve consistency and maintainability. Thank you! 🙏 

```ts
export LOCAL_STORAGE_KEYS={
  CONVERSATION_SELECTED_TAB: "conversation-selected-tab"
  ...
}
```

```ts
const CONVERSATION_STORAGE_KEYS = [
  LOCAL_STORAGE_KEYS.CONVERSATION_SELECTED_TAB
  ...
];
```

```ts
CONVERSATION_STORAGE_KEYS.forEach((prefix) => {
  localStorage.removeItem(`${prefix}-${conversationId}`);
});
```

Code after:
import { useState } from "react";
import type { ConversationTab } from "#/stores/conversation-store";

export const LOCAL_STORAGE_KEYS = {
  CONVERSATION_STATE: "conversation-state",
} as const;

/**
 * Consolidated conversation state stored in a single localStorage key.
 */
export interface ConversationState {
  selectedTab: ConversationTab | null;
  rightPanelShown: boolean;
  unpinnedTabs: string[];
}

const DEFAULT_CONVERSATION_STATE: ConversationState = {
  selectedTab: "editor",
  rightPanelShown: true,
  unpinnedTabs: [],
};

/**
 * Check if a conversation ID is a temporary task ID that should not be persisted.
 * Task IDs have the format "task-{uuid}" and are used during V1 conversation initialization.
 */
export function isTaskConversationId(conversationId: string): boolean {
  return conversationId.startsWith("task-");
}
Repository: All-Hands-AI/OpenHands
File: frontend/src/hooks/query/use-paginated-conversations.ts (TypeScript)

Code before:
      );

      // Optimistically populate individual conversation caches
      result.results.forEach((conversation) => {
        queryClient.setQueryData(
          ["user", "conversation", conversation.conversation_id],
          conversation,
        );
      });

      return result;
    },
    enabled: !!userIsAuthenticated,
    getNextPageParam: (lastPage) => lastPage.next_page_id,
    initialPageParam: undefined as string | undefined,
  });

  useEffect(() => {
    if (!query.data) return;

    const conversationIds = query.data.pages
      .flatMap((page) => page.results)
      .map((conversation) => conversation.conversation_id);

    cleanupOrphanedConversationLocalStorage(conversationIds);
  }, [query.data]);

  return query;
};


Review comment (refactor):
My concern with this is that it will always clear up keys for the second page of the paginated query.

1. Fetch first page
2. Trigger effect
3. Second page keys are not found in first page data
4. Clears second page keys

Let’s remove this effect and just ensure `clearConversationLocalStorage` is called in `useDeleteConversation`. Alternatively we can do a one time clean up, but I’d prefer proposing that in a separate PR

Code after:
import { useInfiniteQuery, useQueryClient } from "@tanstack/react-query";
import ConversationService from "#/api/conversation-service/conversation-service.api";
import { useIsAuthed } from "./use-is-authed";

export const usePaginatedConversations = (limit: number = 20) => {
  const { data: userIsAuthenticated } = useIsAuthed();
  const queryClient = useQueryClient();

  return useInfiniteQuery({
    queryKey: ["user", "conversations", "paginated", limit],
    queryFn: async ({ pageParam }) => {
      const result = await ConversationService.getUserConversations(
        limit,
        pageParam,
      );

      // Optimistically populate individual conversation caches
      result.results.forEach((conversation) => {
        queryClient.setQueryData(
          ["user", "conversation", conversation.conversation_id],
          conversation,
        );
      });

      return result;
    },
    enabled: !!userIsAuthenticated,
    getNextPageParam: (lastPage) => lastPage.next_page_id,
    initialPageParam: undefined as string | undefined,
  });
};
Repository: All-Hands-AI/OpenHands
File: frontend/src/components/features/chat/error-message-banner.tsx (TypeScript)

Code before:

const DEFAULT_MAX_COLLAPSED_CHARS = 220;

export function ErrorMessageBanner({
  message,
  onDismiss,
}: ErrorMessageBannerProps) {
  const { t, i18n } = useTranslation();
  const [isExpanded, setIsExpanded] = React.useState(false);

  const isI18nKey = i18n.exists(message);
  const displayTextForLength = isI18nKey ? String(t(message)) : message;
  const shouldShowToggle =
    displayTextForLength.length > DEFAULT_MAX_COLLAPSED_CHARS;

  const isCollapsed = shouldShowToggle && !isExpanded;

  return (
    <div
      className="w-full rounded-lg p-2 text-black border border-red-800 bg-red-500 flex gap-2 items-start"
      data-testid="error-message-banner"
    >
      <div className="min-w-0 flex-1">
        <div
          className={cn(
            "whitespace-pre-wrap break-words",
            isCollapsed && "line-clamp-3",
          )}
          data-testid="error-message-banner-content"
        >
          {isI18nKey ? (
            <Trans
              i18nKey={message}
              components={{
                a: (
                  <Link
                    className="underline font-bold cursor-pointer"
                    to="/settings/billing"
                  >
                    link
                  </Link>
                ),
              }}
            />
          ) : (
            message
          )}
        </div>

        {shouldShowToggle && (
          <button

Review comment (suggestion):
```suggestion
            "whitespace-pre-wrap wrap-break-words",
```

Code after:

const DEFAULT_MAX_COLLAPSED_CHARS = 220;

export function ErrorMessageBanner({
  message,
  onDismiss,
}: ErrorMessageBannerProps) {
  const { t, i18n } = useTranslation();
  const [isExpanded, setIsExpanded] = React.useState(false);

  const isI18nKey = i18n.exists(message);
  const displayTextForLength = isI18nKey ? String(t(message)) : message;
  const shouldShowToggle =
    displayTextForLength.length > DEFAULT_MAX_COLLAPSED_CHARS;

  const isCollapsed = shouldShowToggle && !isExpanded;

  return (
    <div
      className="w-full rounded-lg p-2 border border-[#FF0006] bg-[#4A0709] flex gap-2 items-start text-white"
      data-testid="error-message-banner"
    >
      <div className="min-w-0 flex-1">
        <div
          className={cn(
            "whitespace-pre-wrap wrap-break-words",
            isCollapsed && "line-clamp-3",
          )}
          data-testid="error-message-banner-content"
        >
          {isI18nKey ? (
            <Trans
              i18nKey={message}
              components={{
                a: (
                  <Link
                    className="underline font-bold cursor-pointer"
                    to="/settings/billing"
                  >
                    link
                  </Link>
                ),
              }}
            />
          ) : (
            message
          )}
        </div>

        {shouldShowToggle && (
          <button
Repository: All-Hands-AI/OpenHands
File: frontend/src/utils/mutate-with-toast.ts (TypeScript)

Code before:
  }

  try {
    const result = await mutation.mutateAsync(variables);

    if (loadingToastId) toast.dismiss(loadingToastId);

    if (success !== false) {
      const message = typeof success === "function" ? success(result) : success;
      if (message) displaySuccessToast(message);
    }

    return result;
  } catch (err) {
    if (loadingToastId) toast.dismiss(loadingToastId);

    if (error !== false) {
      let message: string;
      if (typeof error === "function") {
        message = error(err as Error);
      } else if (error) {
        message = error;
      } else if (err instanceof Error) {
        message = err.message;
      } else {
        message = "An unexpected error occurred";
      }
      displayErrorToast(message);
    }

    throw err;
  }
}


Review comment (suggestion):
**suggestion:** Could we consider supporting multiple languages for this generic error message? Thank you! 🙏 

Code after:
    loadingToastId = toast.loading(loading, TOAST_OPTIONS);
  }

  try {
    const result = await mutation.mutateAsync(variables);

    if (loadingToastId) toast.dismiss(loadingToastId);

    if (success !== false) {
      const message = typeof success === "function" ? success(result) : success;
      if (message) displaySuccessToast(message);
    }

    return result;
  } catch (err) {
    if (loadingToastId) toast.dismiss(loadingToastId);

    if (error !== false) {
      let message: string;
      if (typeof error === "function") {
        message = error(err as Error);
      } else if (error) {
        message = error;
      } else if (err instanceof Error) {
        message = err.message;
      } else {
        message = i18next.t("ERROR$GENERIC");
      }
      displayErrorToast(message);
    }

    throw err;
  }
}
Repository: All-Hands-AI/OpenHands
File: frontend/src/api/event-service/event-service.api.ts (TypeScript)

Code before:

    // Build session headers for authentication
    const headers = buildSessionHeaders(sessionApiKey);

    const { data } = await axios.get<number>(
      `${runtimeUrl}/api/conversations/${conversationId}/events/count`,
      { headers },
    );
    return data;
  }

  // V1 conversations — App Server REST endpoint
  static async searchEventsV1(conversationId: string, limit = 100) {
    const { data } = await openHands.get<{
      items: unknown[];
    }>(`/api/v1/conversation/${conversationId}/events/search`, {
      params: { limit },
    });

    return data.items;
  }

  // V0 conversations — Legacy REST endpoint
  static async searchEventsV0(conversationId: string, limit = 100) {
    const { data } = await openHands.get<{
      events: unknown[];
    }>(`/api/conversations/${conversationId}/events`, {
      params: { limit },
    });

    return data.events;
  }
}
export default EventService;


Review comment (suggestion):
```suggestion
      events: OpenHandsParsedEvent[];
```

Code after:
    const runtimeUrl = buildHttpBaseUrl(conversationUrl);

    // Build session headers for authentication
    const headers = buildSessionHeaders(sessionApiKey);

    const { data } = await axios.get<number>(
      `${runtimeUrl}/api/conversations/${conversationId}/events/count`,
      { headers },
    );
    return data;
  }

  // V1 conversations — App Server REST endpoint
  static async searchEventsV1(conversationId: string, limit = 100) {
    const { data } = await openHands.get<{
      items: OpenHandsEvent[];
    }>(`/api/v1/conversation/${conversationId}/events/search`, {
      params: { limit },
    });

    return data.items;
  }

  // V0 conversations — Legacy REST endpoint
  static async searchEventsV0(conversationId: string, limit = 100) {
    const { data } = await openHands.get<{
      events: OpenHandsEvent[];
    }>(`/api/conversations/${conversationId}/events`, {
      params: { limit },
    });

    return data.events;
  }
}
export default EventService;
Repository: All-Hands-AI/OpenHands
File: frontend/__tests__/hooks/query/use-conversation-history.test.tsx (TypeScript)

Code before:
// --------------------
vi.mock("#/api/event-service/event-service.api");
vi.mock("#/hooks/query/use-user-conversation");

const queryClient = new QueryClient();

function wrapper({ children }: { children: React.ReactNode }) {
  return (
    <QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
  );
}

// --------------------
// Tests
// --------------------
describe("useConversationHistory", () => {
  afterEach(() => {
    vi.clearAllMocks();
  });

  it("calls V1 REST endpoint for V1 conversations", async () => {
    (useUserConversation as any).mockReturnValue({
      data: { conversation_version: "V1" },
    });

    (EventService.searchEventsV1 as any).mockResolvedValue([{ id: "event-1" }]);

    const { result } = renderHook(() => useConversationHistory("conv-123"), {
      wrapper,
    });

    await waitFor(() => {
      expect(result.current.data).toBeDefined();
    });

    expect(EventService.searchEventsV1).toHaveBeenCalledWith("conv-123");
    expect(EventService.searchEventsV0).not.toHaveBeenCalled();
  });

  it("calls V0 REST endpoint for V0 conversations", async () => {
    (useUserConversation as any).mockReturnValue({
      data: { conversation_version: "V0" },
    });

    (EventService.searchEventsV0 as any).mockResolvedValue([{ id: 1 }]);

    const { result } = renderHook(() => useConversationHistory("conv-456"), {
      wrapper,
    });

    await waitFor(() => {

Review comment (suggestion):
```suggestion
    const v1SearchEventsSpy = vi.spyOn(EventService, "searchEventsV1");

    vi.mocked(useUserConversation).mockReturnValue({
      data: { conversation_version: "V1" },
    });

    v1SearchEventsSpy.mockResolvedValue([{ id: "event-1" }]);
```

Code after:
  };
}

function makeEvent(): OpenHandsEvent {
  return {
    id: "evt-1",
  } as OpenHandsEvent;
}

// --------------------
// Mocks
// --------------------
vi.mock("#/api/open-hands-axios", () => ({
  openHands: {
    get: vi.fn(),
  },
}));

vi.mock("#/api/event-service/event-service.api");
vi.mock("#/hooks/query/use-user-conversation");

const queryClient = new QueryClient();

function wrapper({ children }: { children: React.ReactNode }) {
  return (
    <QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
  );
}

// --------------------
// Tests
// --------------------
describe("useConversationHistory", () => {
  afterEach(() => {
    vi.clearAllMocks();
  });

  it("calls V1 REST endpoint for V1 conversations", async () => {
    const v1SearchEventsSpy = vi.spyOn(EventService, "searchEventsV1");

    vi.mocked(useUserConversation).mockReturnValue({
      data: makeConversation("V1"),
      isLoading: false,
      isPending: false,
      isError: false,
      error: null,
      refetch: vi.fn(),
    } as any);

    v1SearchEventsSpy.mockResolvedValue([makeEvent()]);
Repository: All-Hands-AI/OpenHands
File: frontend/__tests__/hooks/query/use-conversation-history.test.tsx (TypeScript)

Code before:

  it("calls V1 REST endpoint for V1 conversations", async () => {
    (useUserConversation as any).mockReturnValue({
      data: { conversation_version: "V1" },
    });

    (EventService.searchEventsV1 as any).mockResolvedValue([{ id: "event-1" }]);

    const { result } = renderHook(() => useConversationHistory("conv-123"), {
      wrapper,
    });

    await waitFor(() => {
      expect(result.current.data).toBeDefined();
    });

    expect(EventService.searchEventsV1).toHaveBeenCalledWith("conv-123");
    expect(EventService.searchEventsV0).not.toHaveBeenCalled();
  });

  it("calls V0 REST endpoint for V0 conversations", async () => {
    (useUserConversation as any).mockReturnValue({
      data: { conversation_version: "V0" },
    });

    (EventService.searchEventsV0 as any).mockResolvedValue([{ id: 1 }]);

    const { result } = renderHook(() => useConversationHistory("conv-456"), {
      wrapper,
    });

    await waitFor(() => {
      expect(result.current.data).toBeDefined();
    });

    expect(EventService.searchEventsV0).toHaveBeenCalledWith("conv-456");
    expect(EventService.searchEventsV1).not.toHaveBeenCalled();
  });
});


Review comment (suggestion):
```suggestion
    const v0SearchEventsSpy = vi.spyOn(EventService, "searchEventsV0");

    vi.mocked(useUserConversation).mockReturnValue({
      data: { conversation_version: "V0" },
    });

    v0SearchEventsSpy.mockResolvedValue([{ id: 1 }]);
```

Code after:
vi.mock("#/hooks/query/use-user-conversation");

const queryClient = new QueryClient();

function wrapper({ children }: { children: React.ReactNode }) {
  return (
    <QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
  );
}

// --------------------
// Tests
// --------------------
describe("useConversationHistory", () => {
  afterEach(() => {
    vi.clearAllMocks();
  });

  it("calls V1 REST endpoint for V1 conversations", async () => {
    const v1SearchEventsSpy = vi.spyOn(EventService, "searchEventsV1");

    vi.mocked(useUserConversation).mockReturnValue({
      data: makeConversation("V1"),
      isLoading: false,
      isPending: false,
      isError: false,
      error: null,
      refetch: vi.fn(),
    } as any);

    v1SearchEventsSpy.mockResolvedValue([makeEvent()]);

    const { result } = renderHook(() => useConversationHistory("conv-123"), {
      wrapper,
    });

    await waitFor(() => {
      expect(result.current.data).toBeDefined();
    });

    expect(EventService.searchEventsV1).toHaveBeenCalledWith("conv-123");
    expect(EventService.searchEventsV0).not.toHaveBeenCalled();
  });

  it("calls V0 REST endpoint for V0 conversations", async () => {
    const v0SearchEventsSpy = vi.spyOn(EventService, "searchEventsV0");

    vi.mocked(useUserConversation).mockReturnValue({
      data: makeConversation("V0"),
      isLoading: false,
      isPending: false,
Repository: All-Hands-AI/OpenHands
File: frontend/src/contexts/conversation-websocket-context.tsx (TypeScript)

Code before:
import React, {
  createContext,
  useContext,
  useEffect,
  useState,
  useCallback,
  useMemo,
  useRef,
} from "react";
import { useTranslation } from "react-i18next";
import { useQueryClient } from "@tanstack/react-query";
import { useWebSocket, WebSocketHookOptions } from "#/hooks/use-websocket";
import { OHEvent, useEventStore } from "#/stores/use-event-store";
import { useErrorMessageStore } from "#/stores/error-message-store";
import { useOptimisticUserMessageStore } from "#/stores/optimistic-user-message-store";
import { useV1ConversationStateStore } from "#/stores/v1-conversation-state-store";
import { useCommandStore } from "#/stores/command-store";
import { useBrowserStore } from "#/stores/browser-store";
import {
  isV1Event,
  isAgentErrorEvent,
  isUserMessageEvent,
  isActionEvent,
  isConversationStateUpdateEvent,
  isFullStateConversationStateUpdateEvent,
  isAgentStatusConversationStateUpdateEvent,
  isStatsConversationStateUpdateEvent,
  isExecuteBashActionEvent,
  isExecuteBashObservationEvent,
  isConversationErrorEvent,
  isPlanningFileEditorObservationEvent,
  isBrowserObservationEvent,
  isBrowserNavigateActionEvent,
} from "#/types/v1/type-guards";
import { ConversationStateUpdateEventStats } from "#/types/v1/core/events/conversation-state-event";
import { handleActionEventCacheInvalidation } from "#/utils/cache-utils";
import { buildWebSocketUrl } from "#/utils/websocket-url";
import type {

Review comment (suggestion):
```suggestion
import { useEventStore } from "#/stores/use-event-store";
```

Code after:
import React, {
  createContext,
  useContext,
  useEffect,
  useState,
  useCallback,
  useMemo,
  useRef,
} from "react";
import { useTranslation } from "react-i18next";
import { useQueryClient } from "@tanstack/react-query";
import { useWebSocket, WebSocketHookOptions } from "#/hooks/use-websocket";
import { useEventStore } from "#/stores/use-event-store";
import { useErrorMessageStore } from "#/stores/error-message-store";
import { useOptimisticUserMessageStore } from "#/stores/optimistic-user-message-store";
import { useV1ConversationStateStore } from "#/stores/v1-conversation-state-store";
import { useCommandStore } from "#/stores/command-store";
import { useBrowserStore } from "#/stores/browser-store";
import {
  isV1Event,
  isAgentErrorEvent,
  isUserMessageEvent,
  isActionEvent,
  isConversationStateUpdateEvent,
  isFullStateConversationStateUpdateEvent,
  isAgentStatusConversationStateUpdateEvent,
  isStatsConversationStateUpdateEvent,
  isExecuteBashActionEvent,
  isExecuteBashObservationEvent,
  isConversationErrorEvent,
  isPlanningFileEditorObservationEvent,
  isBrowserObservationEvent,
  isBrowserNavigateActionEvent,
} from "#/types/v1/type-guards";
import { ConversationStateUpdateEventStats } from "#/types/v1/core/events/conversation-state-event";
import { handleActionEventCacheInvalidation } from "#/utils/cache-utils";
import { buildWebSocketUrl } from "#/utils/websocket-url";
import type {