Fix for setting LLM model, frontend settings refactor

[rbren]

Fixes https://github.com/OpenDevin/OpenDevin/issues/1167

This ended up being a much bigger change than I hoped. Fortunately most of it is deleted code.

• One really important change is that we always recreate the agent controller when we get an initialize event. This gives the frontend the ability to change settings during the same session
• There's a lot of cleanup on frontend localStorage mgmt. We were saving things there that don't really need to be saved (e.g. supportedModels).
• We were also downloading a configuration from the server, and setting that as the initial configuration, which isn't a great pattern--the server was overriding choices the user made in the FE.
• There was a bug where the session wasn't starting properly for brand new sessions, and you needed to refresh the page (you can see this by running localStorage.clear()

There's probably room to fix up some of my frontend work. Definitely open to feedback here

[lowlyocean]

We were also downloading a configuration from the server, and setting that as the initial configuration.

Is config.toml still needed to get Local LLM running? I think it behaved this way to accomodate the inability to enter everything into FE. Otherwise, entry in the UI would make it so you'd have to clear local storage to re-ingest config.toml

[rbren]

Is config.toml still needed to get Local LLM running? I think it behaved this way to accomodate the inability to enter everything into FE. Otherwise, entry in the UI would make it so you'd have to clear local storage to re-ingest config.toml

Now that we're inside docker, most of this stuff is controlled via env vars passed to the container with -e. Those are indeed still required. But setting MODEL or AGENT via the UI won't undo those env vars--they should work together fine.

For example, you can pass -e LLM_API_KEY=sk-..., and then change MODEL in the UI without undoing the LLM_API_KEY variable

[lowlyocean]

Think the concern is the opposite situation

If you've already selected a model manually in the UI, will the UI ignore subsequent environment variable changes sent with -e?

If we make sure that the api key and the Embedding and Inference server's URL can be set in the UI, then this becomes less of a concern. But I imagine people expect environment variables to take precedence over what's set in the UI

[rbren]

But I imagine people expect environment variables to take precedence over what's set in the UI

Ah no we've made the decision that the UI takes precedence--that's how it's supposed to work today too (though it seems to just not work at all at the moment).

The one thing we're losing here is that if you set the LLM_MODEL or AGENT env var, the UI will still default to gpt-3.5. But we're also no longer encouraging folks to set that env var unless they're running headless.

Once https://github.com/OpenDevin/OpenDevin/issues/1146 is in, most users will no longer set env vars, and instead will control everything via the UI. Would be great to add LLM_BASE_URL and LLM_EMBEDDING_MODEL to settings too...

[lowlyocean]

Got it, so the current recommendation is to omit LLM_MODEL and AGENT from the environment variables. Those should be set directly from the UI. However, environemnt variables are still necessary for LLM_BASE_URL and LLM_EMBEDDING_MODEL if you're trying to use Local LLM. Yes , I agree those two should be included in https://github.com/OpenDevin/OpenDevin/issues/1146 .

So far it's been a bit of a situation where one PR has undone the changes a prior PR introduced, to allow Local LLM.

Has the project considered tagging releases? It's a bit of a crapshoot under the above circumstances to know if ollama will work after a particular merge or not.

Edit: I see just starting yesterday there are some Tags created. Is making sure Local LLM works, one of the checks that go into allowing a tag pipeline to succeed? If so, folks looking for a stable experience could be encouraged to use a tagged commit

[rbren]

Yup! We just tagged our first release yesterday!

Currently the README still recommends main since things are still pretty fluid, and we don't want to have to constantly update the README. But I'm hoping we can release a stable 1.0 soon.

Would love to have a way to validate an end-to-end local LLM installation in CI/CD...

[lowlyocean]

Absolutely. I think Cypress.io could be handy here. Since there's a backed & a frontend to run separately on the CI pipeline host, this would probably be a relevant Github Action setup (with a 3rd npm command to launch ollama)

To make sure the test runner sees environment variables, you might have cypress/plugins/index.js that looks like:

const dotenvPlugin = require('cypress-dotenv');

/**
 * @type {Cypress.PluginConfig}
 */
module.exports = (on, config) => {
  // `on` is used to hook into various events Cypress emits
  // `config` is the resolved Cypress config

  // CI doesn't use dotenv to pass these for e2e
  config.env.LLM_BASE_URL = process.env.LLM_BASE_URL;
  config.env.LLM_EMBEDDING_MODEL = process.env.LLM_EMBEDDING_MODEL;
  config.env.LLM_MODEL = process.env.LLM_MODEL;
  config.env.AGENT= process.env.AGENT;
  config.env.SANDBOX_TYPE = process.env.SANDBOX_TYPE;
  config.env.WORKSPACE_DIR = process.env.WORKSPACE_DIR;

  config = dotenvPlugin(config, undefined, true)  

  return config
}

and then an actual E2E test defined in a new file cypress/integration/local_llm.spec.js that would look something like:

describe('Confirm settings work for Local LLM', function () {
  before( function() {
    cy.visit('/')
  })
  
  it('Make sure the model was picked up from the config', function () {
    cy.location('pathname').should('eq','/')
    cy.get('[data-cy="settingsButton"]').click()
    cy.get('[data-cy="settingModal"]').get([data-cy="modelDropdown"]).should('eq', Cypress.env('LLM_MODEL'));
  })

  it('Send a simple prompt and confirm the Chat window doesn't show message corresponding to Local LLM issue', function () {
    cy.location('pathname').should('eq','/')
    cy.get('[data-cy="chatPromptEntryBox"]').type('Write a file named HelloWorld.txt')
    cy.get('[data-cy="buttonToSubmitThePrompt"]').click()
    cy.get('[data-cy="chatBox"]', {timeout: 10000} /* wait for ollama to respond with action */ ).should('not.have.string','OpenAIException')
  })
})

Of course the UI elements should be instrumented with data-cy attributes to prevent the test spec from being brittle

[rbren]

Confirmed make setup-config works fine still!