OpenHands icon indicating copy to clipboard operation
OpenHands copied to clipboard

The default model from the UI is not applied

Open enyst opened this issue 1 year ago • 4 comments

Describe the bug The UI has now the ability to choose model, and it defaults to GPT-3.5. But GPT-3.5 is not the model in use, if you had another in your config and you didn't select anything in the UI.

Steps to Reproduce

  1. Have GPT-4 or another model in config or .env.
  2. Start opendevin, the UI loads models and it shows a GPT-3.5 as the default choice.
  3. Start tasks.

Expected behavior GPT-3.5 is used.

Actual behavior GPT-4 from .env was used.

Additional context I was keeping an eye on openai billing, I used only 3.5 according the UI for 1 1/2 days, and openai showed clearly that GPT-4 was used in that time. GPT-4 was set in .env.

Related: the UI should default to the model in the config. (I think that is discussed elsewhere too.) In any case, the user should be able to count on the UI to tell the truth about the model in actual use, so to speak. It wasn't a problem yet, but it can be! To be clear, if you change the UI choice, it will be applied. But if you don't, it displays the wrong one.

enyst avatar Apr 01 '24 13:04 enyst

This happens because setting the model is done via the initialize event, which is only sent when the model is changed.

@rbren Can we remove creating the controller from the Session initializer and initializer via the initialize event only?

yimothysu avatar Apr 01 '24 16:04 yimothysu

We can extract settings from SettingModal into Redux state and send the settings once the WebSocket connection is established.

yimothysu avatar Apr 01 '24 16:04 yimothysu

I believe that was done because the socket is not available when the UI is initially loaded. We could queue up the messages so that they can be fired once the socket is established.

jagadish-k avatar Apr 01 '24 17:04 jagadish-k

This could work...

// frontend/src/socket/socket.ts
const socket = new WebSocket(WS_URL);

// Define a queue to hold messages that are to be sent when the WebSocket connects
const messageQueue: SocketMessage[] = [];

export function sendMessageOnSocket(socketMessage: SocketMessage) {
  if ("action" in socketMessage) {
    handleActionMessage(socketMessage);
  } else {
    handleObservationMessage(socketMessage);
  }
  if (socket.readyState === WebSocket.OPEN) {
    socket.send(JSON.stringify(socketMessage));
  } else {
    messageQueue.push(socketMessage);
  }
}

socket.addEventListener("open", () => {
  // Send any messages that were queued while the WebSocket was connecting
  while (messageQueue.length > 0) {
    const message = messageQueue.shift();
    socket.send(JSON.stringify(message));
  }
});

socket.addEventListener("message", (event) => {
  const socketMessage = JSON.parse(event.data) as SocketMessage;
  sendMessageOnSocket(socketMessage);
});

And from the changeSetting file use this utility method instead of sending on the socket directly.

// frontend/src/services/settingsService.ts
function changeSetting(setting: string, value: string): void {
  const event = { action: "initialize", args: { [setting]: value } };

  sendMessageOnSocket(event);

jagadish-k avatar Apr 02 '24 01:04 jagadish-k

I think it's OK to wait for an initialize event to start the controller, so long as we change the frontend to work with that behavior. Probably better practice TBH

rbren avatar Apr 02 '24 03:04 rbren