This was just an error when translating the original data from https://github.com/sourcegraph/cody/blob/dc9ea0b18ef4db9141ef204e8a3ea026b21b9bb3/lib/shared/src/models/dotcom.ts#L4 here. The model name is just "gemini-1.5-flash-latest", the "google/" prefix shouldn't be there. (Like above on line 159.)

We didn't catch this sooner because I hadn't (1) loaded the base configuration data and (2) actually tried to use the "google::v1::gemini-1.5-flash-latest" model yet.

TL;DR: Here we conflating the "Model Provider" and "API Provider", but there isn't much we can do without breaking Cody Pro. (Since the allow-list requires the long-ass name "mixtral-8x22b-instruct/accounts/fireworks/models/mixtral-8x22b-instruct".

I actually already added support for this outside of the "base config" scenario, see: https://github.com/sourcegraph/sourcegraph/blob/main/cmd/frontend/internal/httpapi/completions/get_model_test.go#L71.

The issue here is essentially the same as the one above, an error when converting the model from the Cody Gateway "provider/model" naming scheme to the new modelconfig world. (See dotcom.ts)

However, in this case, the model name actually IS that crazy long, weird thing. There's a similar comment in codygateway/codygateway.go, but essentially here the model name -- the thing to identify the model at the LLM API level -- is assumed to be passed via Fireworks.

An alternative way to account for this would be something like:

// Mistral Provider and its Fireworks API Provider configuration.
{
    id: "mistral",
    serverSideConfig: {
        fireworksApiProvider: {
             endpoint: ...
             accessToken: ...
             // ?
             modelAccountNamePrefixThingey: "accounts/fireworks/models"
        }
    }
}

In addition to just moving the code to this separate function, there were no fewer than TWO bugs in the original logic (::sigh::).

// The original code
// ---
				// Status filter.
				if modelFilters.StatusFilter != nil {
					if !slices.Contains(modelFilters.StatusFilter, string(baseConfigModel.Category)) {
						continue
					}

1.) We are filtering base based on the mode's Status and yet comparing against the Category field.
2.) We were filtering OUT the model if the status was defined in the config, rather than what the intention was. (e.g. statusFilter "beta" removed all z"beta"s, not everything but "beta".)

🤦 The originally code has return nil here, which is incorrect. Because that meant that if the first model didn't have the "chat" or "edit" capability, then we just gave up and said nothing was found.

It took me longer to debug this than I would like to admit. But at least things are a step in the right direction, by splitting the logic out into a dedicated maybeFixDefaultModels function.

Previously we were only "looking up an alternative" default model if one wasn't already defined. However, if you add a sourcegraph.modelFilters.deny rule, it's possible that whatever the base config has as the default model are no longer available.

Hence the || !modelFound(...).

It will take a few more PRs to get Cody Pro and the "base config" working correctly, but this is the first step.

Suppose you had the site config:

"modelConfiguration": {
    // Use Sourcegraph-supplied models.
    "sourcegraph": {}
}

That config works great, however, the "base config" (whatever is embedded in the binary or comes from cody gateway) won't have any ServerSideProviderConfig data. (Since it is dependent on the specific Sourcegraph instance and its site license.)

So all this function does is just makes sure that for these cases, it gets routed to Cody Gateway. Which means that we need to be careful about when we call it, so that if a site admin fails to provide the server-side config data, it will fail at runtime. Because we don't want to "default to Cody Gateway", but instead just have this just be implied from the "Sourcegraph supplied model data".

FYI "modelConfiguration": {} should be equal to your configuration snippet above, because it is modelConfiguration that is nullable

FYI

"modelConfiguration": {}

should be equal to

"modelConfiguration": {
    // Use Sourcegraph-supplied models.
    "sourcegraph": {}
}

Help me understand this.

In both cases we are defining the modelConfiguration site config value. So we will use that for determining the LLM models available in the backend, and not the "completions" section.

However, in the first case ("modelConfiguration": {}) the user has not defined the sourcegraph field. So wouldn't that default to null, meaning that no Sourcegraph-defined models will be served?

Or is it the case that somehow modelConfiguration.sourcegraph needs to explicitly be set to = null for this to happen. (And if you don't specify the sourcegraph field at all, it will "default" to {} and will include Sourcegraph-supplied models.)

Just want to make sure I understand how this works.

In both cases we are defining the modelConfiguration site config value. So we will use that for determining the LLM models available in the backend, and not the "completions" section.

Correct

However, in the first case ("modelConfiguration": {}) the user has not defined the sourcegraph field. So wouldn't that default to null, meaning that no Sourcegraph-defined models will be served?

No, modelConfiguration has a default value of null (only so because it is a Go pointer type here), while sourcegraph SHOULD have a default value of {"pollingInterval": "6h"} because that is what we say in the (purely-documentation) default field here

It is possible however that when we look at modelConfiguration.sourcegraph on the Go side, it will be nil - that signals 'we should use the default value' that we indicate in the site.schema.json default field. (go-jsonschema doesn't do any defaults for us, it's our job to detect the zero value in Go and use the appropriate defaults everywhere)

It is possible however that when we look at modelConfiguration.sourcegraph on the Go side, it will be nil - that signals 'we should use the default value' that we indicate in the site.schema.json default field.

But how then would we differentiate the "undefined" case from when the user explicitly set it to null, e.g. "sourcegraph" = null,.

My understanding was that the "default" field in the site config was purely cosmetic, and just used for the autocompletion in the UI. And doesn't have anything to do with how things act at runtime. But I could be wrong about that, but if I am, that seems a little dangerous because of the ambiguity around "undefined" vs. "explicitly null".

The default field in the site config is purely cosmetic, yes, but it also serves as documentation: if I do not specify this field, what is the default?

But how then would we differentiate the "undefined" case from when the user explicitly set it to null, e.g. "sourcegraph" = null,.

Aha! Good question, you got me - I was wrong, the two configurations are NOT equal, I see it now.

What we should do then is update the default field in site.schema.json to indicate the default is null and that it must be specified. (and add an examples field, so it would autocomplete to show that as an example configuration)