Provider-specific branching is starting to spread

Not a blocker for this PR — the feature works as designed — but I think this is worth capturing before the next provider lands.

ByokModelProvider.isAzure() carries this Javadoc:

"This avoids scattering direct enum displayName comparisons across UI code."

That was the right instinct, but the helper only replaced scattered string comparisons with scattered if branches. After this PR we're at 13 provider-specific branch sites:

ByokService — :334 (isAzure, skip remote fetch), :339 (isOllama, check URL instead of key), :420 (isOllama, auto-register discovered models), :459 (isAzure, filter out of bulk fetch)

ByokPreferencePage — :519, :520, :523, :525, :724, :729, :848, :881, and the onProviderExpanded ternary

All of them are re-deriving answers to the same four orthogonal questions:

1. Does the provider need a provider-level API key? (Azure no — per-model credentials; Ollama no; rest yes)
2. Does it need a provider-level endpoint URL? (Ollama only)
3. Can it discover models remotely? (Azure no)
4. Are discovered models registered by default? (Ollama only)

The part I'd actually change now

requiresApiKey() is the right direction, but it's defined by exclusion:

return !isAzure(providerDisplayName) && !isOllama(providerDisplayName);

The next provider that doesn't take a top-level key — and there's demand for one in #93's comments ("OpenAI protocol compatible local LLM server") — silently gets the wrong answer until someone remembers to edit this line. A positive definition would fail loudly instead of quietly.

Longer-term shape

Push the capabilities onto the enum so each provider declares itself once:

public enum ByokModelProvider {
  AZURE("Azure", Credential.PER_MODEL, /* remoteDiscovery */ false, /* autoRegister */ false),
  OPENAI("OpenAI", Credential.API_KEY, true, false),
  …
  OLLAMA("Ollama", Credential.ENDPOINT_URL, true, true);

  public boolean requiresApiKey() { … }
  public boolean requiresEndpointUrl() { … }
  public boolean supportsRemoteDiscovery() { … }
  public boolean autoRegistersDiscoveredModels() { … }
}

Adding a provider then means one enum line instead of auditing 13 call sites.

I recognise this isn't free: the helpers take String because the UI only has the tree's display name, so instance methods would need a fromDisplayName(String) lookup plus an unknown-name policy. Happy for that to be a follow-up issue rather than scope in here — but the requiresApiKey inversion feels cheap enough to do now.

Provider-specific branching is starting to spread

Not a blocker for this PR — the feature works as designed — but I think this is worth capturing before the next provider lands.

ByokModelProvider.isAzure() carries this Javadoc:

"This avoids scattering direct enum displayName comparisons across UI code."

That was the right instinct, but the helper only replaced scattered string comparisons with scattered if branches. After this PR we're at 13 provider-specific branch sites:

ByokService — :334 (isAzure, skip remote fetch), :339 (isOllama, check URL instead of key), :420 (isOllama, auto-register discovered models), :459 (isAzure, filter out of bulk fetch)

ByokPreferencePage — :519, :520, :523, :525, :724, :729, :848, :881, and the onProviderExpanded ternary

All of them are re-deriving answers to the same four orthogonal questions:

1. Does the provider need a provider-level API key? (Azure no — per-model credentials; Ollama no; rest yes)
2. Does it need a provider-level endpoint URL? (Ollama only)
3. Can it discover models remotely? (Azure no)
4. Are discovered models registered by default? (Ollama only)

The part I'd actually change now

requiresApiKey() is the right direction, but it's defined by exclusion:

return !isAzure(providerDisplayName) && !isOllama(providerDisplayName);

The next provider that doesn't take a top-level key — and there's demand for one in #93's comments ("OpenAI protocol compatible local LLM server") — silently gets the wrong answer until someone remembers to edit this line. A positive definition would fail loudly instead of quietly.

Longer-term shape

Push the capabilities onto the enum so each provider declares itself once:

public enum ByokModelProvider {
  AZURE("Azure", Credential.PER_MODEL, /* remoteDiscovery */ false, /* autoRegister */ false),
  OPENAI("OpenAI", Credential.API_KEY, true, false),
  …
  OLLAMA("Ollama", Credential.ENDPOINT_URL, true, true);

  public boolean requiresApiKey() { … }
  public boolean requiresEndpointUrl() { … }
  public boolean supportsRemoteDiscovery() { … }
  public boolean autoRegistersDiscoveredModels() { … }
}

Adding a provider then means one enum line instead of auditing 13 call sites.

I recognise this isn't free: the helpers take String because the UI only has the tree's display name, so instance methods would need a fromDisplayName(String) lookup plus an unknown-name policy. Happy for that to be a follow-up issue rather than scope in here — but the requiresApiKey inversion feels cheap enough to do now.

Addressed in #388