chore: add new providers for pricing and new models#1126
Merged
Conversation
Contributor
Reviewer's GuideMoves provider metadata from a static TypeScript map into a new ClickHouse ReplacingMergeTree table, rewires the provider registry and tests to be fully DB-driven, adds CRUD APIs and UI for managing providers, extends the import API and docs to support providers, and tightens ID auto-formatting for providers and models. Sequence diagram for ProviderEditorDialog save flowsequenceDiagram
actor User
participant ManageModelsPage
participant ProviderEditorDialog
participant UseFetchWrapper
participant ProvidersRoute as ApiProvidersRoute
participant ClickHouse
User->>ManageModelsPage: click AddProvider or EditProvider
ManageModelsPage->>ProviderEditorDialog: open dialog with ProviderFormData
User->>ProviderEditorDialog: fill form and click Save
ProviderEditorDialog->>UseFetchWrapper: fireRequest(POST or PUT, body ProviderFormData)
UseFetchWrapper->>ApiProvidersRoute: HTTP POST or PUT /api/openground/providers
ApiProvidersRoute->>ApiProvidersRoute: getCurrentUser()
ApiProvidersRoute->>ApiProvidersRoute: getDBConfigByUser(true)
alt creating provider
ApiProvidersRoute->>ClickHouse: insert into openlit_provider_metadata
else updating provider
ApiProvidersRoute->>ClickHouse: ALTER TABLE openlit_provider_metadata UPDATE
end
ClickHouse-->>ApiProvidersRoute: success or error
ApiProvidersRoute-->>UseFetchWrapper: JSON { success, providerId } or error
alt success
UseFetchWrapper-->>ProviderEditorDialog: success callback
ProviderEditorDialog->>ManageModelsPage: onSaved()
ManageModelsPage->>ManageModelsPage: loadProviders() and loadAllCustomModels()
else failure
UseFetchWrapper-->>ProviderEditorDialog: failure callback
end
Sequence diagram for import providers and models APIsequenceDiagram
actor User
participant ManageModelsPage
participant ImportRoute as ApiModelsImportRoute
participant ClickHouse
User->>ManageModelsPage: open Import dialog and submit JSON
ManageModelsPage->>ImportRoute: POST /api/openground/models/import
ImportRoute->>ImportRoute: authenticate and getDBConfigByUser(true)
ImportRoute->>ImportRoute: parse body.providers and body.models
alt providers present
ImportRoute->>ClickHouse: SELECT provider_id FROM openlit_provider_metadata FINAL
ClickHouse-->>ImportRoute: existing provider_ids
ImportRoute->>ImportRoute: build providersToInsert
alt providersToInsert not empty
ImportRoute->>ClickHouse: INSERT INTO openlit_provider_metadata values providersToInsert
ClickHouse-->>ImportRoute: ok
end
end
alt models present
ImportRoute->>ClickHouse: SELECT provider, model_id FROM openlit_provider_models
ClickHouse-->>ImportRoute: existing provider model pairs
ImportRoute->>ImportRoute: build modelsToInsert
alt modelsToInsert not empty
ImportRoute->>ClickHouse: INSERT INTO openlit_provider_models values modelsToInsert
ClickHouse-->>ImportRoute: ok
end
end
ImportRoute-->>ManageModelsPage: JSON { imported, skipped, providersImported, providersSkipped }
ManageModelsPage-->>User: show import result
ER diagram for provider metadata and models tableserDiagram
openlit_provider_metadata {
String provider_id PK
String display_name
String description
Boolean requires_vault
String config_schema
Boolean is_default
DateTime created_at
DateTime updated_at
}
openlit_provider_models {
String provider
String model_id
String displayName
Float inputPricePerMToken
Float outputPricePerMToken
String modelType
String capabilities
}
openlit_providers {
String provider
String api_key
String config
}
openlit_provider_metadata ||--o{ openlit_provider_models : has_models
openlit_provider_metadata ||--o{ openlit_providers : has_api_configs
Class diagram for ProviderRegistry and default provider seed dataclassDiagram
class ProviderMetadata {
string providerId
string displayName
string description
boolean requiresVault
Record configSchema
ModelMetadata[] supportedModels
}
class ModelMetadata {
string provider
string model_id
string displayName
float inputPricePerMToken
float outputPricePerMToken
string modelType
string[] capabilities
}
class ProviderMetadataRow {
string provider_id
string display_name
string description
boolean requires_vault
string config_schema
boolean is_default
}
class ProviderRegistry {
+getAvailableProviders(databaseConfigId string) Promise~ProviderMetadata[]~
+getProviderById(providerId string, databaseConfigId string) Promise~ProviderMetadata~
+searchProviders(query string, databaseConfigId string) Promise~ProviderMetadata[]~
+getModelsForProvider(providerId string, databaseConfigId string) Promise~ModelMetadata[]~
}
class DefaultProviderEntry {
string providerId
string displayName
string description
boolean requiresVault
Record configSchema
}
class DefaultModelEntry {
string provider
string model_id
string displayName
float inputPricePerMToken
float outputPricePerMToken
string modelType
string[] capabilities
}
class DEFAULT_PROVIDERS {
<<array>>
}
class DEFAULT_MODELS_BY_PROVIDER {
<<record>>
}
ProviderRegistry ..> ProviderMetadataRow : uses
ProviderRegistry ..> ProviderMetadata : returns
ProviderRegistry ..> ModelMetadata : returns
ProviderMetadataRow ..> ProviderMetadata : parsed_into
DEFAULT_PROVIDERS ..> DefaultProviderEntry : elements
DEFAULT_MODELS_BY_PROVIDER ..> DefaultModelEntry : elements
class CreateProviderMetadataMigration {
+CreateProviderMetadataMigration(databaseConfigId string) Promise~object~
}
class migrationHelper {
+migrationHelper(config object) Promise~object~
}
class dataCollector {
+dataCollector(params object, mode string, databaseConfigId string) Promise~object~
}
CreateProviderMetadataMigration ..> DEFAULT_PROVIDERS : seeds_from
CreateProviderMetadataMigration ..> dataCollector : executes_queries
CreateProviderMetadataMigration ..> migrationHelper : uses
Class diagram for provider editor UI componentsclassDiagram
class ProviderFormData {
string providerId
string displayName
string description
boolean requiresVault
}
class ProviderEditorDialogProps {
boolean open
function onOpenChange
ProviderFormData provider
function onSaved
}
class ProviderEditorDialog {
+ProviderEditorDialog(props ProviderEditorDialogProps)
-ProviderFormData formData
-boolean isEditing
-handleSave() void
}
class ModelListSidebarProps {
ProviderMetadata[] providers
ModelMetadata[] models
string selectedProviderId
string selectedModelId
boolean selectedIsCustom
function onSelectModel
function onAddNew
function onEditProvider
}
class ModelListSidebar {
+ModelListSidebar(props ModelListSidebarProps)
}
class ManageModelsPageState {
string selectedProviderId
string selectedModelId
boolean isCustomModel
boolean isAddingNew
boolean showImport
boolean showSdkUsage
boolean showProviderEditor
ProviderFormData editingProvider
}
class ManageModelsPage {
+ManageModelsPage()
-handleAddProvider() void
-handleEditProvider(provider ProviderMetadata) void
-handleProviderSaved() void
}
ProviderEditorDialogProps o--> ProviderFormData
ProviderEditorDialog ..> ProviderFormData : manages
ManageModelsPage ..> ProviderEditorDialog : renders
ManageModelsPage ..> ModelListSidebar : renders
ModelListSidebarProps ..> ProviderMetadata : uses
ModelListSidebarProps ..> ModelMetadata : uses
ModelListSidebar ..> ModelListSidebarProps : props
ManageModelsPage ..> ManageModelsPageState : uses_state
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
✅ Jest Coverage — Great (92.08%)
|
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In the models import API, you handle errors from the models insert but ignore the result of the providers insert (
dataCollectorreturn value); consider checkingerrthere as well so provider import failures are surfaced instead of silently continuing. - In the providers DELETE handler you hardcode
openlit_provider_modelsrather than usingOPENLIT_PROVIDER_MODELS_TABLE_NAME; aligning this with the constant used elsewhere will reduce duplication and make future table name changes safer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the models import API, you handle errors from the models insert but ignore the result of the providers insert (`dataCollector` return value); consider checking `err` there as well so provider import failures are surfaced instead of silently continuing.
- In the providers DELETE handler you hardcode `openlit_provider_models` rather than using `OPENLIT_PROVIDER_MODELS_TABLE_NAME`; aligning this with the constant used elsewhere will reduce duplication and make future table name changes safer.
## Individual Comments
### Comment 1
<location path="src/client/src/app/api/openground/providers/route.ts" line_range="222-231" />
<code_context>
+ );
+ }
+
+ const updateFields: string[] = [];
+ if (displayName !== undefined) {
+ updateFields.push(`display_name = '${Sanitizer.sanitizeValue(displayName)}'`);
+ }
+ if (description !== undefined) {
+ updateFields.push(`description = '${Sanitizer.sanitizeValue(description)}'`);
+ }
+ if (requiresVault !== undefined) {
+ updateFields.push(`requires_vault = ${!!requiresVault}`);
+ }
+ if (configSchema !== undefined) {
+ updateFields.push(`config_schema = '${Sanitizer.sanitizeValue(JSON.stringify(configSchema))}'`);
+ }
+ updateFields.push(`updated_at = now()`);
+
+ const query = `
+ ALTER TABLE ${OPENLIT_PROVIDER_METADATA_TABLE_NAME}
+ UPDATE ${updateFields.join(", ")}
</code_context>
<issue_to_address>
**🚨 issue (security):** Building the UPDATE statement via string interpolation is fragile and could allow SQL injection if Sanitizer is bypassed.
Here we interpolate user-controlled values (displayName, description, configSchema) directly into SQL. Even with `Sanitizer.sanitizeValue`, this is fragile: any missed field or future change could reopen SQL injection or quoting bugs. Prefer a parameterized/structured UPDATE via the existing `dataCollector` APIs if possible, or at least a single shared helper that returns safely quoted ClickHouse literals instead of manual string concatenation.
</issue_to_address>
### Comment 2
<location path="src/client/src/app/api/openground/providers/route.ts" line_range="297-298" />
<code_context>
+ const sanitizedId = Sanitizer.sanitizeValue(providerId);
+
+ // Delete provider metadata + all its models
+ const [metaResult, modelsResult] = await Promise.all([
+ dataCollector(
+ {
+ query: `DELETE FROM ${OPENLIT_PROVIDER_METADATA_TABLE_NAME} WHERE provider_id = '${sanitizedId}'`,
</code_context>
<issue_to_address>
**suggestion:** Use the shared table constant instead of hardcoding `openlit_provider_models` in the DELETE query.
This DELETE is the only provider-related query that hardcodes `openlit_provider_models` instead of using the constants from `table-details.ts`. Please switch it to `OPENLIT_PROVIDER_MODELS_TABLE_NAME` to stay consistent and avoid issues if the table name changes later.
</issue_to_address>
### Comment 3
<location path="src/client/src/lib/platform/providers/provider-registry.ts" line_range="40-49" />
<code_context>
+async function loadAllProvidersFromDb(
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Swallowing DB errors and returning an empty provider list can hide operational issues.
If ClickHouse is down or the query fails, this function logs the error but still returns an empty array. Callers like `getAvailableProviders` will treat that as “no providers configured” instead of a backend failure. Please either propagate the error or distinguish between “no rows” and “query failed” so the API can respond with an appropriate 5xx instead of a successful empty result.
Suggested implementation:
```typescript
/**
* Load all provider metadata rows from ClickHouse.
*
* Errors from ClickHouse are intentionally allowed to propagate so that callers
* can distinguish between “no providers configured” and backend failures.
*/
async function loadAllProvidersFromDb(
databaseConfigId: string
): Promise<Omit<ProviderMetadata, "supportedModels">[]> {
const query = `
SELECT
provider_id,
display_name,
description,
requires_vault,
config_schema,
```
The current implementation likely has a `catch` block later in `loadAllProvidersFromDb` that logs the error and returns `[]`. That `catch` block (including any `console.error`/`logger.error` and `return []`) should be removed so that any ClickHouse/query errors bubble up to the caller.
Callers like `getAvailableProviders` that currently assume `loadAllProvidersFromDb` never throws will need to be updated to either:
1. Let the error propagate all the way to the HTTP layer (so it becomes a 5xx), or
2. Wrap `loadAllProvidersFromDb` in a `try/catch` and convert the failure into an appropriate error response, rather than treating it as an empty provider list.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Important
to this Pull Request.
feat: ...orfix: ....Fixes #1125
Change description:
Moves provider definitions from hardcoded TypeScript into a ClickHouse table, making providers fully dynamic — users can add, edit, and delete providers through the UI, API, or import.
New ClickHouse table
openlit_provider_metadata(ReplacingMergeTree) — storesprovider_id,display_name,description,requires_vault,config_schema(JSON),is_defaultcreate-provider-metadata-migration.tscreates the table and seeds 14 built-in providers fromDEFAULT_PROVIDERSindefault-models.tsmigrations/index.tsto run alongside the existing provider/model migrationsProvider registry rewrite
PROVIDER_METADATArecord (~160 lines) fromprovider-registry.tsgetAvailableProviders()now loads provider metadata + models in parallel from ClickHousegetProviderById(),searchProviders()all query the DBFINALkeyword onReplacingMergeTreequeries to deduplicateDEFAULT_PROVIDERSindefault-models.ts(used only by migration seeding)Provider CRUD API
POST /api/openground/providers— create a new providerPUT /api/openground/providers— update provider metadata (displayName, description, requiresVault, configSchema)DELETE /api/openground/providers?provider=<id>— delete provider + all its modelsProvider add/edit UI
PlusCircleicon with tooltip) in the Manage Models headergroup/providerCSSProviderEditorDialogcomponent — form fields: Provider ID (read-only when editing), Display Name, Description, Requires Vault toggleModel ID auto-formatting
model-editor-panel.tsxnow applies the same transformation: lowercase, spaces → hyphens, strips chars outsidea-z 0-9 - / .accounts/fireworks/models/llama-v3p1-70b-instructImport extended
POST /api/openground/models/importnow accepts optionalprovidersarrayprovider_id(no duplicates)providersImportedandprovidersSkippedcountsFiles changed
providers/table-details.ts— addedOPENLIT_PROVIDER_METADATA_TABLE_NAMEproviders/default-models.ts— addedDEFAULT_PROVIDERSarray +DefaultProviderEntrytypecreate-provider-metadata-migration.ts(new),migrations/index.ts(wired in)providers/provider-registry.ts— rewritten, fully DB-drivenapi/openground/providers/route.ts— addedPOST,PUT,DELETEhandlersapi/openground/models/import/route.ts— extended with providers supportprovider-editor-dialog.tsx(new),model-list-sidebar.tsx(addedonEditProvider+ hover pencil),manage-models/page.tsx(Add Provider button + dialog wiring),model-editor-panel.tsx(model ID auto-formatting)en.ts— addedMANAGE_PROVIDERS_*stringsprovider-registry.test.ts— rewritten for DB-driven architecturemanage-models.mdx— added "Managing providers" section, updated import docs, added API reference tableTests
provider-registry.test.tsfully rewritten — mocks both provider metadata and model DB queries. All 114 suites / 1716 tests pass.Checklist
feat: ...orfix: ....update/change?
Acknowledgment
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the project license.
Summary by Sourcery
Make provider definitions fully dynamic by storing metadata in ClickHouse, exposing CRUD APIs and UI for managing providers, and extending import/export flows and docs accordingly.
New Features:
Enhancements:
Build:
Documentation:
Tests: