From ddc792ff284e58cd29d777c9defb9ceb744bc124 Mon Sep 17 00:00:00 2001 From: Lucio Lelii Date: Thu, 17 Sep 2026 12:27:19 +0200 Subject: [PATCH] Let a provider declare it needs an endpoint, and accept one as a credential Two additions to LLMProvider, both additive defaults so no existing provider or test stub changes behaviour. requiresEndpoint() names the one thing every provider until now has had in common without anyone needing to say so: a base URL it already knows, whether server-configured or a constant of the service it talks to. A provider whose endpoint is not known until a credential names it - coming next - is the first that needs to say otherwise. ProviderCredential carries that endpoint alongside the secret value a provider has always received. The three new generate/generateJson/chat overloads that take one default to unwrapping .value() and calling the String-authorization overload above them, so a provider that only overrides the old ones - which today is every one of them, including every anonymous test stub across the suite - keeps behaving exactly as it did. Only a provider that overrides the new overloads directly gets to read .endpoint() at all. Adding an abstract method instead would have broken every one of those stubs, since none of them implement anything beyond the three methods the interface already requires. One ambiguity fell out of this at the call site InternalOllamaLLMProvider used to have: chat(model, messages, null, null) no longer resolves unambiguously, since a bare null now fits both the String and the ProviderCredential overload equally. Not visible in this diff - that call site was rewritten away in the Ollama extraction - but worth naming since it is the shape of thing this kind of overload addition can trigger elsewhere too. Co-Authored-By: Claude Sonnet 5 --- .../manager/llms/ProviderCredential.java | 24 ++++ .../manager/llms/providers/LLMProvider.java | 36 +++++ .../ProviderCredentialDefaultsTest.java | 130 ++++++++++++++++++ 3 files changed, 190 insertions(+) create mode 100644 src/main/java/it/cnr/isti/workflow/manager/llms/ProviderCredential.java create mode 100644 src/test/java/it/cnr/isti/workflow/manager/llms/providers/ProviderCredentialDefaultsTest.java diff --git a/src/main/java/it/cnr/isti/workflow/manager/llms/ProviderCredential.java b/src/main/java/it/cnr/isti/workflow/manager/llms/ProviderCredential.java new file mode 100644 index 0000000..c75eac0 --- /dev/null +++ b/src/main/java/it/cnr/isti/workflow/manager/llms/ProviderCredential.java @@ -0,0 +1,24 @@ +// SPDX-FileCopyrightText: 2025-2026 Lucio Lelii - ISTI-CNR +// SPDX-License-Identifier: AGPL-3.0-or-later +// Attribution term under AGPL-3.0 section 7(b): see LICENSE-ADDENDUM. + +package it.cnr.isti.workflow.manager.llms; + +/** + * What a resolved vault credential actually carries: the secret value every provider has always + * received, and - only for a provider whose {@code requiresEndpoint()} is true - the base URL that + * came with it. + * + *

{@code endpoint} is null for every provider with a constant or server-configured base URL, + * which today is every provider that exists; it becomes non-null only once a provider whose + * endpoint the user supplies (an OpenAI-compatible gateway, a remote Ollama instance) resolves a + * credential that carries one. See {@code docs/llm-providers-openai-remote-ollama-plan-2026-09-17.md}. + */ +public record ProviderCredential(String value, String endpoint) { + + /** A credential with no endpoint - every provider before this type existed only had this. */ + public static ProviderCredential ofValue(String value) { + return new ProviderCredential(value, null); + } + +} diff --git a/src/main/java/it/cnr/isti/workflow/manager/llms/providers/LLMProvider.java b/src/main/java/it/cnr/isti/workflow/manager/llms/providers/LLMProvider.java index bc2f1d0..c7f314d 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/llms/providers/LLMProvider.java +++ b/src/main/java/it/cnr/isti/workflow/manager/llms/providers/LLMProvider.java @@ -12,6 +12,7 @@ import java.util.stream.Collectors; import it.cnr.isti.workflow.manager.llms.ChatMessage; import it.cnr.isti.workflow.manager.llms.ModelParameter; import it.cnr.isti.workflow.manager.llms.ModelParameters; +import it.cnr.isti.workflow.manager.llms.ProviderCredential; public interface LLMProvider { @@ -64,6 +65,28 @@ public interface LLMProvider { return authorization == null ? chat(model, messages) : chat(model, messages, authorization); } + /** + * The same three calls, taking the credential as a whole rather than the bare secret value. + * + *

Every provider before {@link ProviderCredential} existed only ever had the value, so each + * defaults to unwrapping it and calling the {@code String}-authorization overload above - + * unchanged behaviour for every existing provider, including every test stub, without adding an + * abstract method to this interface. Only a provider whose {@link #requiresEndpoint()} is true + * has a reason to override these instead, to also read {@link ProviderCredential#endpoint()}. + */ + default String generate(String model, String prompt, ProviderCredential credential, ModelParameters parameters) { + return generate(model, prompt, credential == null ? null : credential.value(), parameters); + } + + default String generateJson(String model, String prompt, ProviderCredential credential, ModelParameters parameters) { + return generateJson(model, prompt, credential == null ? null : credential.value(), parameters); + } + + default String chat(String model, List messages, ProviderCredential credential, + ModelParameters parameters) { + return chat(model, messages, credential == null ? null : credential.value(), parameters); + } + /** * Which sampling parameters this provider actually applies. Everything, by default: a provider * that maps none of them still behaves as it always has, and only one that knowingly leaves a @@ -91,6 +114,19 @@ public interface LLMProvider { return false; } + /** + * Whether a call needs an endpoint the user supplies, rather than one this provider already + * knows. False for every provider with a constant or server-configured base URL - which is + * every provider that exists today - true only for one whose base URL is not known until a + * credential names it (an OpenAI-compatible gateway, a remote Ollama instance). + * + *

This is what puts the endpoint field on the "Add credential" dialog for that provider and + * nowhere else: see {@code docs/llm-providers-openai-remote-ollama-plan-2026-09-17.md}. + */ + default boolean requiresEndpoint() { + return false; + } + default String authorizationFieldName() { return "authorization"; } diff --git a/src/test/java/it/cnr/isti/workflow/manager/llms/providers/ProviderCredentialDefaultsTest.java b/src/test/java/it/cnr/isti/workflow/manager/llms/providers/ProviderCredentialDefaultsTest.java new file mode 100644 index 0000000..64672e5 --- /dev/null +++ b/src/test/java/it/cnr/isti/workflow/manager/llms/providers/ProviderCredentialDefaultsTest.java @@ -0,0 +1,130 @@ +// SPDX-FileCopyrightText: 2025-2026 Lucio Lelii - ISTI-CNR +// SPDX-License-Identifier: AGPL-3.0-or-later +// Attribution term under AGPL-3.0 section 7(b): see LICENSE-ADDENDUM. + +package it.cnr.isti.workflow.manager.llms.providers; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +import java.util.List; + +import org.junit.jupiter.api.Test; + +import it.cnr.isti.workflow.manager.llms.ChatMessage; +import it.cnr.isti.workflow.manager.llms.ModelParameters; +import it.cnr.isti.workflow.manager.llms.ProviderCredential; + +/** + * The {@link ProviderCredential}-based overloads {@link LLMProvider} adds alongside its existing + * {@code String}-authorization ones: every provider that predates {@link ProviderCredential} - which + * is every provider today - must keep working unchanged through them, and a provider that does + * override them must actually see the endpoint. + */ +class ProviderCredentialDefaultsTest { + + /** Records what it was called with, on the pre-existing String-authorization overloads only. */ + private static class LegacyProvider implements LLMProvider { + String lastAuthorization = "not called"; + + @Override + public String getName() { + return "Legacy"; + } + + @Override + public List getRegisteredModels() { + return List.of(); + } + + @Override + public String generate(String model, String prompt) { + return "generate:" + prompt; + } + + @Override + public String generate(String model, String prompt, String authorization, ModelParameters parameters) { + lastAuthorization = authorization; + return "generate-authorized:" + prompt; + } + + @Override + public String generateJson(String model, String prompt, String authorization, ModelParameters parameters) { + lastAuthorization = authorization; + return "generateJson-authorized:" + prompt; + } + + @Override + public String chat(String model, List messages, String authorization, ModelParameters parameters) { + lastAuthorization = authorization; + return "chat-authorized"; + } + } + + private final LegacyProvider legacy = new LegacyProvider(); + private final List messages = List.of(new ChatMessage(ChatMessage.Role.USER, "hi")); + + @Test + void unwrapsTheValueForAProviderThatOnlyKnowsTheStringOverload() { + String result = legacy.generate("m", "p", new ProviderCredential("secret", "https://ignored.example.com"), + null); + + assertEquals("generate-authorized:p", result); + assertEquals("secret", legacy.lastAuthorization); + } + + @Test + void generateJsonUnwrapsTheValueTheSameWay() { + legacy.generateJson("m", "p", new ProviderCredential("secret", null), null); + + assertEquals("secret", legacy.lastAuthorization); + } + + @Test + void chatUnwrapsTheValueTheSameWay() { + legacy.chat("m", messages, new ProviderCredential("secret", null), null); + + assertEquals("secret", legacy.lastAuthorization); + } + + @Test + void aNullCredentialBehavesExactlyLikeANullAuthorization() { + legacy.generate("m", "p", (ProviderCredential) null, null); + + assertNull(legacy.lastAuthorization); + } + + @Test + void aProviderThatOverridesTheCredentialOverloadCanReadTheEndpoint() { + // This is the override point OpenAICompatibleProvider and RemoteOllamaProvider use - the + // only reason ProviderCredential exists rather than passing the value string alone. + LLMProvider endpointAware = new LLMProvider() { + @Override + public String getName() { + return "EndpointAware"; + } + + @Override + public List getRegisteredModels() { + return List.of(); + } + + @Override + public String generate(String model, String prompt) { + throw new UnsupportedOperationException("not exercised by this test"); + } + + @Override + public String generate(String model, String prompt, ProviderCredential credential, + ModelParameters parameters) { + return credential.endpoint() + " -> " + prompt; + } + }; + + String result = endpointAware.generate("m", "p", new ProviderCredential("secret", "https://api.example.com"), + null); + + assertEquals("https://api.example.com -> p", result); + } + +}