Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fixed model not loading, even after baseUrl set in .env file #816

Merged
merged 8 commits into from
Dec 18, 2024

Conversation

thecodacus
Copy link
Collaborator

Fix Model List Loading from Environment Variables

Overview

This PR fixes a critical issue where model lists were not properly loading when configured via environment variables. Previously, the application would fail to recognize models configured in .env files, leading to incomplete model lists and potential connectivity issues.

Key Changes

1. Environment Variable Resolution

  • Added proper environment variable cascade for model providers
  • Fixed base URL resolution order: Settings → Process Env → Server Env → Import Meta
  • Unified environment variable handling across different providers

2. API Key and Provider Settings

  • Improved API key handling during initialization
  • Added proper type definitions for server environment
  • Fixed race conditions in provider settings initialization

3. Model List Initialization

  • Restructured initializeModelList to accept comprehensive options
  • Added proper environment variable forwarding to model providers
  • Fixed server-side model list initialization

Technical Details

Environment Variable Resolution

The core fix involves proper cascade of environment variables. Example from Together provider:

const baseUrl =
  settingsBaseUrl ||
  serverEnv?.TOGETHER_API_BASE_URL ||
  process.env.TOGETHER_API_BASE_URL ||
  import.meta.env.TOGETHER_API_BASE_URL ||
  '';

Model List Initialization Changes

Key changes to the initialization function:

async function initializeModelList(options: {
  env?: Record<string, string>;
  providerSettings?: Record<string, IProviderSetting>;
  apiKeys?: Record<string, string>;
}): Promise<ModelInfo[]>

Provider Updates

Updated provider type definition to support server environment:

getDynamicModels?: (
  apiKeys?: Record<string, string>,
  providerSettings?: IProviderSetting,
  serverEnv?: Record<string, string>
) => Promise<ModelInfo[]>;

Testing

  • Verified model loading with environment variables
  • Tested model list initialization on server startup
  • Validated provider settings override behavior
  • Confirmed proper cascade of environment variables

Migration Impact

  • No breaking changes
  • No migration steps required
  • Existing environment variables will work as expected

Future Improvements

  • Add environment variable validation
  • Implement provider-specific environment variable schemas
  • Add better error handling for misconfigured environments

@thecodacus thecodacus changed the title fix: fixed model not loading, BaseUrl set in .env file fix: fixed model not loading, even after baseUrl set in .env file Dec 18, 2024
Copy link
Collaborator

@dustinwloring1988 dustinwloring1988 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is recognizing the baseurl set in the .env.local however is not displaying in the provider tab it is showing as blank, but debug tab shows the correct baseURL, This problem existed before this PR though and I can address it in a future on.

@thecodacus
Copy link
Collaborator Author

I was not aiming to sync the env urls with settings tab. as it can have weird consequences like
changing the url in env should it change the url in settings ?
if so then we cannot override the env with ui settings, it will already be in sync

and if it should not sync then people who does not know about setting tab and only using env file, if they change the url in env file and it was previously added in the settings tab then the new url in env will not work anymore they have to clear the cache.

@thecodacus
Copy link
Collaborator Author

what we can do is add an indicator in the provider tab that shows that the baseUrl is already set from env file, and if you want to override that you can add a new one in the text box

@thecodacus
Copy link
Collaborator Author

@dustinwloring1988 check now

@dustinwloring1988
Copy link
Collaborator

@thecodacus Thanks, love it

@thecodacus thecodacus merged commit 50e6778 into stackblitz-labs:main Dec 18, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants