ronantakizawa/codereview-bench huggingface dataset:
Repository: All-Hands-AI/OpenHands
File: openhands-cli/build.py (Python)

Code before:

    # Test the executable
    if not args.no_test:
        dummy_agent = get_default_cli_agent(
            llm=LLM(
                model='dummy-model',
                api_key='dummy-key',
                metadata=get_llm_metadata(model_name='dummy-model', llm_type='openhands'),
            )
        )
        if not test_executable(dummy_agent):
            print('❌ Executable test failed, build process failed')
            return 1

    print('\n🎉 Build process completed!')
    print("📁 Check the 'dist/' directory for your executable")

    return 0


if __name__ == '__main__':
    try:
        sys.exit(main())
    except Exception as e:
        print(e)
        print('❌ Executable test failed, build process failed')
        sys.exit(1)



Review comment (bug):
```suggestion
        print('❌ Executable test failed')
```

Code after:

    # Test the executable
    if not args.no_test:
        dummy_agent = get_default_cli_agent(
            llm=LLM(
                model='dummy-model',
                api_key='dummy-key',
                metadata=get_llm_metadata(model_name='dummy-model', llm_type='openhands'),
            )
        )
        if not test_executable(dummy_agent):
            print('❌ Executable test failed, build process failed')
            return 1

    print('\n🎉 Build process completed!')
    print("📁 Check the 'dist/' directory for your executable")

    return 0


if __name__ == '__main__':
    try:
        sys.exit(main())
    except Exception as e:
        print(e)
        print('❌ Executable test failed')
        sys.exit(1)
Repository: All-Hands-AI/OpenHands
File: openhands-cli/tests/commands/test_new_command.py (Python)

Code before:
    mock_agent = MagicMock()
    mock_verify_agent.return_value = mock_agent

    # Mock conversation - only one is created when /new is called
    conv1 = MagicMock()
    conv1.id = UUID("aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa")
    mock_setup_conversation.return_value = conv1

    # One runner instance for the conversation
    runner1 = MagicMock()
    runner1.is_confirmation_mode_active = True
    mock_runner_cls.return_value = runner1

    # Real session fed by a pipe (no interactive confirmation now)
    from openhands_cli.user_actions.utils import (
        get_session_prompter as real_get_session_prompter,
    )

    with create_pipe_input() as pipe:
        output = DummyOutput()
        session = real_get_session_prompter(input=pipe, output=output)
        mock_get_session_prompter.return_value = session

        from openhands_cli.agent_chat import run_cli_entry

        # Trigger /new, then send a message to trigger runner creation, then /exit (exit will be auto-accepted)
        for ch in "/new\rhello\r/exit\r":
            pipe.send_text(ch)

        run_cli_entry(None)

    # Assert we created one runner for the conversation when a message was processed after /new
    assert mock_runner_cls.call_count == 1
    assert mock_runner_cls.call_args_list[0].args[0] is conv1


Review comment (suggestion):
```suggestion
        # Trigger /new
        # First user message should trigger runner creation
        # Then /exit (exit will be auto-accepted)
```

Code after:
    mock_agent = MagicMock()
    mock_verify_agent.return_value = mock_agent

    # Mock conversation - only one is created when /new is called
    conv1 = MagicMock()
    conv1.id = UUID("aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa")
    mock_setup_conversation.return_value = conv1

    # One runner instance for the conversation
    runner1 = MagicMock()
    runner1.is_confirmation_mode_active = True
    mock_runner_cls.return_value = runner1

    # Real session fed by a pipe (no interactive confirmation now)
    from openhands_cli.user_actions.utils import (
        get_session_prompter as real_get_session_prompter,
    )

    with create_pipe_input() as pipe:
        output = DummyOutput()
        session = real_get_session_prompter(input=pipe, output=output)
        mock_get_session_prompter.return_value = session

        from openhands_cli.agent_chat import run_cli_entry

        # Trigger /new
        # First user message should trigger runner creation
        # Then /exit (exit will be auto-accepted)
        for ch in "/new\rhello\r/exit\r":
            pipe.send_text(ch)

        run_cli_entry(None)

    # Assert we created one runner for the conversation when a message was processed after /new
    assert mock_runner_cls.call_count == 1
    assert mock_runner_cls.call_args_list[0].args[0] is conv1
Repository: All-Hands-AI/OpenHands
File: openhands-cli/openhands_cli/agent_chat.py (Python)

Code before:
#!/usr/bin/env python3
"""
Agent chat functionality for OpenHands CLI.
Provides a conversation interface with an AI agent using OpenHands patterns.
"""

import sys
from datetime import datetime
import uuid

from openhands.sdk import (
    Message,
    TextContent,
)
from openhands.sdk.conversation.state import AgentExecutionStatus
from prompt_toolkit import print_formatted_text
from prompt_toolkit.formatted_text import HTML

from openhands_cli.runner import ConversationRunner
from openhands_cli.setup import (
    MissingAgentSpec,
    setup_conversation,
    verify_agent_exists_or_setup_agent
)
from openhands_cli.tui.settings.mcp_screen import MCPScreen
from openhands_cli.tui.settings.settings_screen import SettingsScreen
from openhands_cli.tui.status import display_status
from openhands_cli.tui.tui import (
    display_help,
    display_welcome,
)
from openhands_cli.user_actions import UserConfirmation, exit_session_confirmation
from openhands_cli.user_actions.utils import get_session_prompter


def _restore_tty() -> None:
    """
    Ensure terminal modes are reset in case prompt_toolkit cleanup didn't run.
    - Turn off application cursor keys (DECCKM): ESC[?1l
    - Turn off bracketed paste: ESC[?2004l

Review comment (suggestion):
@openhands 
Read how ConversationExecutionStatus is defined in the repository https://github.com/OpenHands/software-agent-sdk
and how it is used in the current repository at openhands-cli package.
There seems to be a dependency issue: the openhands CLI is dependent on what looks like internal  classes from the software-agent-sdk package. When someone modifies the software-agent-sdk it unexpectedly breaks code from the OpenHands CLI.
What would be a good pattern to reinforce that dependency?
Don't make any edits. Think hard and suggest possible solutions 

Code after:
#!/usr/bin/env python3
"""
Agent chat functionality for OpenHands CLI.
Provides a conversation interface with an AI agent using OpenHands patterns.
"""

import sys
from datetime import datetime
import uuid

from openhands.sdk import (
    Message,
    TextContent,
)
from openhands.sdk.conversation.state import ConversationExecutionStatus
from prompt_toolkit import print_formatted_text
from prompt_toolkit.formatted_text import HTML

from openhands_cli.runner import ConversationRunner
from openhands_cli.setup import (
    MissingAgentSpec,
    setup_conversation,
    verify_agent_exists_or_setup_agent
)
from openhands_cli.tui.settings.mcp_screen import MCPScreen
from openhands_cli.tui.settings.settings_screen import SettingsScreen
from openhands_cli.tui.status import display_status
from openhands_cli.tui.tui import (
    display_help,
    display_welcome,
)
from openhands_cli.user_actions import UserConfirmation, exit_session_confirmation
from openhands_cli.user_actions.utils import get_session_prompter


def _restore_tty() -> None:
    """
    Ensure terminal modes are reset in case prompt_toolkit cleanup didn't run.
    - Turn off application cursor keys (DECCKM): ESC[?1l
    - Turn off bracketed paste: ESC[?2004l
Repository: All-Hands-AI/OpenHands
File: frontend/src/context/ws-client-provider.tsx (TypeScript)

Code before:
    useActiveConversation();

  function flushPendingEvents(socket: Socket | null = sioRef.current) {
    if (!socket || pendingEventsRef.current.length === 0) {
      return;
    }

    pendingEventsRef.current.forEach((queuedEvent) => {
      socket.emit("oh_user_action", queuedEvent);
    });
    pendingEventsRef.current = [];
  }

  function send(event: Record<string, unknown>) {
    const socket = sioRef.current;

    if (!socket) {
      pendingEventsRef.current.push(event);
      return;
    }

    if (pendingEventsRef.current.length > 0) {
      flushPendingEvents(socket);
    }

    socket.emit("oh_user_action", event);
  }

  function handleConnect() {
    setWebSocketStatus("CONNECTED");
    removeErrorMessage();
    flushPendingEvents();
  }

  function handleMessage(event: Record<string, unknown>) {
    handleAssistantMessage(event);

    if (isOpenHandsEvent(event)) {
      const isStatusUpdateError =
        isStatusUpdate(event) && event.type === "error";

      const isAgentStateChangeError =
        isAgentStateChangeObservation(event) &&
        event.extras.agent_state === "error";

      if (isStatusUpdateError || isAgentStateChangeError) {
        const errorMessage = isStatusUpdate(event)
          ? event.message
          : event.extras.reason || "Unknown error";

        trackError({

Review comment (suggestion):
```suggestion
      EventLogger.error("WebSocket is not connected, queuing message...");
      pendingEventsRef.current.push(event);
```

Code after:
  const [webSocketStatus, setWebSocketStatus] =
    React.useState<V0_WebSocketStatus>("DISCONNECTED");
  const lastEventRef = React.useRef<Record<string, unknown> | null>(null);
  const { providers } = useUserProviders();

  const messageRateHandler = useRate({ threshold: 250 });
  const { data: conversation, refetch: refetchConversation } =
    useActiveConversation();

  function flushPendingEvents(socket: Socket | null = sioRef.current) {
    if (!socket || pendingEventsRef.current.length === 0) {
      return;
    }

    pendingEventsRef.current.forEach((queuedEvent) => {
      socket.emit("oh_user_action", queuedEvent);
    });
    pendingEventsRef.current = [];
  }

  function send(event: Record<string, unknown>) {
    const socket = sioRef.current;

    if (!socket) {
      EventLogger.error("WebSocket is not connected, queuing message...");
      pendingEventsRef.current.push(event);
      return;
    }

    if (pendingEventsRef.current.length > 0) {
      flushPendingEvents(socket);
    }

    socket.emit("oh_user_action", event);
  }

  function handleConnect() {
    setWebSocketStatus("CONNECTED");
    removeErrorMessage();
    flushPendingEvents();
  }

  function handleMessage(event: Record<string, unknown>) {
    handleAssistantMessage(event);

    if (isOpenHandsEvent(event)) {
      const isStatusUpdateError =
        isStatusUpdate(event) && event.type === "error";

      const isAgentStateChangeError =
        isAgentStateChangeObservation(event) &&
Repository: All-Hands-AI/OpenHands
File: frontend/src/entry.client.tsx (TypeScript)

Code before:
import OptionService from "./api/option-service/option-service.api";
import { displayErrorToast } from "./utils/custom-toast-handlers";
import { queryClient } from "./query-client-config";

function PostHogWrapper({ children }: { children: React.ReactNode }) {
  const [posthogClientKey, setPosthogClientKey] = React.useState<string | null>(
    null,
  );
  const [isLoading, setIsLoading] = React.useState(true);

  React.useEffect(() => {
    (async () => {
      try {
        const config = await OptionService.getConfig();
        setPosthogClientKey(config.POSTHOG_CLIENT_KEY);
      } catch {
        displayErrorToast("Error fetching PostHog client key");
      } finally {
        setIsLoading(false);
      }
    })();
  }, []);

  if (isLoading) {
    return children;
  }

  if (!posthogClientKey) {
    return children;
  }

  return (
    <PostHogProvider
      apiKey={posthogClientKey}
      options={{
        api_host: "https://us.i.posthog.com",
        person_profiles: "identified_only",
      }}
    >
      {children}
    </PostHogProvider>
  );
}

async function prepareApp() {
  if (
    process.env.NODE_ENV === "development" &&
    import.meta.env.VITE_MOCK_API === "true"
  ) {
    const { worker } = await import("./mocks/browser");


Review comment (refactor):
**suggestion (non-blocking):** The two if statements return the same result, so they can be combined to simplify the code.

```js
if (isLoading || !posthogClientKey) {
  return children;
}
```

Code after:
import OptionService from "./api/option-service/option-service.api";
import { displayErrorToast } from "./utils/custom-toast-handlers";
import { queryClient } from "./query-client-config";

function PostHogWrapper({ children }: { children: React.ReactNode }) {
  const [posthogClientKey, setPosthogClientKey] = React.useState<string | null>(
    null,
  );
  const [isLoading, setIsLoading] = React.useState(true);

  React.useEffect(() => {
    (async () => {
      try {
        const config = await OptionService.getConfig();
        setPosthogClientKey(config.POSTHOG_CLIENT_KEY);
      } catch {
        displayErrorToast("Error fetching PostHog client key");
      } finally {
        setIsLoading(false);
      }
    })();
  }, []);

  if (isLoading || !posthogClientKey) {
    return children;
  }

  return (
    <PostHogProvider
      apiKey={posthogClientKey}
      options={{
        api_host: "https://us.i.posthog.com",
        person_profiles: "identified_only",
      }}
    >
      {children}
    </PostHogProvider>
  );
}

async function prepareApp() {
  if (
    process.env.NODE_ENV === "development" &&
    import.meta.env.VITE_MOCK_API === "true"
  ) {
    const { worker } = await import("./mocks/browser");

    await worker.start({
      onUnhandledRequest: "bypass",
    });
  }
Repository: All-Hands-AI/OpenHands
File: frontend/src/entry.client.tsx (TypeScript)

Code before:
function PostHogWrapper({ children }: { children: React.ReactNode }) {
  const [posthogClientKey, setPosthogClientKey] = React.useState<string | null>(
    null,
  );
  const [isLoading, setIsLoading] = React.useState(true);

  React.useEffect(() => {
    (async () => {
      try {
        const config = await OptionService.getConfig();
        setPosthogClientKey(config.POSTHOG_CLIENT_KEY);
      } catch {
        displayErrorToast("Error fetching PostHog client key");
      } finally {
        setIsLoading(false);
      }
    })();
  }, []);

  if (isLoading) {
    return children;
  }

  if (!posthogClientKey) {
    return children;
  }

  return (
    <PostHogProvider
      apiKey={posthogClientKey}
      options={{
        api_host: "https://us.i.posthog.com",
        person_profiles: "identified_only",
      }}
    >
      {children}
    </PostHogProvider>
  );
}

async function prepareApp() {
  if (
    process.env.NODE_ENV === "development" &&
    import.meta.env.VITE_MOCK_API === "true"
  ) {
    const { worker } = await import("./mocks/browser");

    await worker.start({
      onUnhandledRequest: "bypass",
    });
  }

Review comment (suggestion):
```suggestion
  if (isLoading || !posthogClientKey) {
    return children;
  }
```

Code after:
function PostHogWrapper({ children }: { children: React.ReactNode }) {
  const [posthogClientKey, setPosthogClientKey] = React.useState<string | null>(
    null,
  );
  const [isLoading, setIsLoading] = React.useState(true);

  React.useEffect(() => {
    (async () => {
      try {
        const config = await OptionService.getConfig();
        setPosthogClientKey(config.POSTHOG_CLIENT_KEY);
      } catch {
        displayErrorToast("Error fetching PostHog client key");
      } finally {
        setIsLoading(false);
      }
    })();
  }, []);

  if (isLoading || !posthogClientKey) {
    return children;
  }

  return (
    <PostHogProvider
      apiKey={posthogClientKey}
      options={{
        api_host: "https://us.i.posthog.com",
        person_profiles: "identified_only",
      }}
    >
      {children}
    </PostHogProvider>
  );
}

async function prepareApp() {
  if (
    process.env.NODE_ENV === "development" &&
    import.meta.env.VITE_MOCK_API === "true"
  ) {
    const { worker } = await import("./mocks/browser");

    await worker.start({
      onUnhandledRequest: "bypass",
    });
  }
}

prepareApp().then(() =>
  startTransition(() => {
Repository: All-Hands-AI/OpenHands
File: frontend/src/components/features/chat/change-agent-button.tsx (TypeScript)

Code before:
      {
        parentConversationId: conversation.conversation_id,
        agentType: "plan",
      },
      {
        onSuccess: (data) => {
          displaySuccessToast(
            t(I18nKey.PLANNING_AGENTT$PLANNING_AGENT_INITIALIZED),
          );
          // Track the task ID to poll for sub-conversation creation
          if (data.v1_task_id) {
            setSubConversationTaskId(data.v1_task_id);
          }
        },
      },
    );
  };

  // Trigger handlePlanClick when isCreateAPlanClicked becomes true
  useEffect(() => {
    if (isCreateAPlanClicked) {
      handlePlanClick();
      // Reset the flag after triggering
      setIsCreateAPlanClicked(false);
    }
  }, [isCreateAPlanClicked, setIsCreateAPlanClicked, handlePlanClick]);

  const isButtonDisabled =
    isAgentRunning ||
    isCreatingConversation ||
    !isWebSocketConnected ||
    !shouldUsePlanningAgent;

  // Handle Shift + Tab keyboard shortcut to cycle through modes
  useEffect(() => {
    if (isButtonDisabled) {
      return undefined;
    }

    const handleKeyDown = (event: KeyboardEvent) => {
      // Check for Shift + Tab combination
      if (event.shiftKey && event.key === "Tab") {
        // Prevent default tab navigation behavior
        event.preventDefault();
        event.stopPropagation();

        // Cycle between modes: code -> plan -> code
        const nextMode = conversationMode === "code" ? "plan" : "code";
        if (nextMode === "plan") {
          handlePlanClick(event);
        } else {

Review comment (suggestion):
Maybe we can define `handlePlanClick` within the conversation store? That way we can avoid implicitly calling it via a use effect and just have it called directly.


Code after:
    };

    document.addEventListener("keydown", handleKeyDown);

    return () => {
      document.removeEventListener("keydown", handleKeyDown);
    };
  }, [
    isButtonDisabled,
    conversationMode,
    setConversationMode,
    handlePlanClick,
  ]);

  const handleButtonClick = (event: React.MouseEvent<HTMLButtonElement>) => {
    event.preventDefault();
    event.stopPropagation();
    setContextMenuOpen(!contextMenuOpen);
  };

  const handleCodeClick = (event: React.MouseEvent<HTMLButtonElement>) => {
    event.preventDefault();
    event.stopPropagation();
    setConversationMode("code");
  };

  const isExecutionAgent = conversationMode === "code";

  const buttonLabel = useMemo(() => {
    if (isExecutionAgent) {
      return t(I18nKey.COMMON$CODE);
    }
    return t(I18nKey.COMMON$PLAN);
  }, [isExecutionAgent, t]);

  const buttonIcon = useMemo(() => {
    if (isExecutionAgent) {
      return <CodeTagIcon width={18} height={18} color="#737373" />;
    }
    return <LessonPlanIcon width={18} height={18} color="#ffffff" />;
  }, [isExecutionAgent]);

  if (!shouldUsePlanningAgent) {
    return null;
  }

  return (
    <div className="relative">
      <button
        type="button"
        onClick={handleButtonClick}
Repository: All-Hands-AI/OpenHands
File: frontend/src/components/features/settings/settings-navigation.tsx (TypeScript)

Code before:
import { useTranslation } from "react-i18next";
import { NavLink } from "react-router";
import { cn } from "#/utils/utils";
import { Typography } from "#/ui/typography";
import { I18nKey } from "#/i18n/declaration";
import SettingsIcon from "#/icons/settings-gear.svg?react";
import CloseIcon from "#/icons/close.svg?react";
import { SettingsNavItem } from "#/constants/settings-nav";

interface SettingsNavigationProps {
  isMobileMenuOpen: boolean;
  onCloseMobileMenu: () => void;
  navigationItems: SettingsNavItem[]; // <-- FIXED
}

export function SettingsNavigation({
  isMobileMenuOpen,
  onCloseMobileMenu,
  navigationItems,
}: SettingsNavigationProps) {
  const { t } = useTranslation();

  return (
    <>
      {/* Mobila backdrop */}
      {isMobileMenuOpen && (
        <div
          className="fixed inset-0 bg-black bg-opacity-50 z-40 md:hidden"
          onClick={onCloseMobileMenu}
        />
      )}
      {/* Navigation sidebar */}
      <nav
        data-testid="settings-navbar"
        className={cn(
          "flex flex-col gap-6 transition-transform duration-300 ease-in-out",
          // Mobile: full screen overlay
          "fixed inset-0 z-50 w-full bg-base-secondary p-4 transform md:transform-none",

Review comment (suggestion):
```suggestion
  navigationItems: SettingsNavItem[];
```

Code after:
import { useTranslation } from "react-i18next";
import { NavLink } from "react-router";
import { cn } from "#/utils/utils";
import { Typography } from "#/ui/typography";
import { I18nKey } from "#/i18n/declaration";
import SettingsIcon from "#/icons/settings-gear.svg?react";
import CloseIcon from "#/icons/close.svg?react";
import { SettingsNavItem } from "#/constants/settings-nav";

interface SettingsNavigationProps {
  isMobileMenuOpen: boolean;
  onCloseMobileMenu: () => void;
  navigationItems: SettingsNavItem[];
}

export function SettingsNavigation({
  isMobileMenuOpen,
  onCloseMobileMenu,
  navigationItems,
}: SettingsNavigationProps) {
  const { t } = useTranslation();

  return (
    <>
      {/* Mobile backdrop */}
      {isMobileMenuOpen && (
        <div
          className="fixed inset-0 bg-black bg-opacity-50 z-40 md:hidden"
          onClick={onCloseMobileMenu}
        />
      )}
      {/* Navigation sidebar */}
      <nav
        data-testid="settings-navbar"
        className={cn(
          "flex flex-col gap-6 transition-transform duration-300 ease-in-out",
          // Mobile: full screen overlay
          "fixed inset-0 z-50 w-full bg-base-secondary p-4 transform md:transform-none",