#6040 - Improve LLM infrastructure#6105
Merged
Merged
Conversation
reckart
changed the title
There was a problem hiding this comment.
Pull request overview
This PR refactors the assistant/LLM integration towards a provider-neutral infrastructure by introducing typed, cross-provider chat generation parameters and routing assistant chat/embedding through the LlmChatClient abstraction with configurable provider/URL/auth.
Changes:
- Adds provider-neutral
temperature/topPtoChatOptionsand translates them in the Ollama/OpenAI/Azure adapters while preserving provider-specific overrides via theoptionsbag. - Extends message/adapter plumbing to carry tool calls through conversation history (notably for Ollama) and updates the assistant loop to use the neutral client interfaces.
- Makes the assistant’s chat and embedding endpoints/provider/auth configurable (top-level defaults + per-section overrides), updating wiring and tests accordingly.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| inception/inception-imls-ollama/src/main/java/de/tudarmstadt/ukp/inception/recommendation/imls/llm/ollama/client/OllamaLlmChatClient.java | Maps neutral chat options and tool calls onto Ollama request/response DTOs. |
| inception/inception-imls-llm-support/src/main/java/de/tudarmstadt/ukp/inception/recommendation/imls/llm/client/ChatOptions.java | Introduces typed temperature/topP, defensive copying, and a builder. |
| inception/inception-imls-llm-support/src/main/java/de/tudarmstadt/ukp/inception/recommendation/imls/llm/ChatMessage.java | Adds toolCalls to messages for round-tripping tool invocations in history. |
| inception/inception-imls-llm-support/src/main/java/de/tudarmstadt/ukp/inception/recommendation/imls/llm/ChatBasedLlmRecommenderImplBase.java | Switches to the new ChatOptions builder during exchanges. |
| inception/inception-imls-chatgpt/src/main/java/de/tudarmstadt/ukp/inception/recommendation/imls/llm/chatgpt/client/ChatGptLlmChatClient.java | Translates neutral temperature/topP into OpenAI request options. |
| inception/inception-imls-azureai-openai/src/main/java/de/tudarmstadt/ukp/inception/recommendation/imls/llm/azureaiopenai/client/AzureAiOpenAiLlmChatClient.java | Translates neutral temperature/topP into Azure OpenAI request options. |
| inception/inception-imls-azureai-openai/pom.xml | Updates dependency-analyzer ignore configuration for slf4j false positives. |
| inception/inception-assistant/src/test/java/de/tudarmstadt/ukp/inception/assistant/AgentLoopTest.java | Updates tests to use OllamaLlmChatClient instead of the raw Ollama client. |
| inception/inception-assistant/src/main/java/de/tudarmstadt/ukp/inception/assistant/embedding/EmbeddingServiceImpl.java | Uses configurable embedding provider/URL/auth via the extension point. |
| inception/inception-assistant/src/main/java/de/tudarmstadt/ukp/inception/assistant/config/AssistantPropertiesImpl.java | Adds provider/URL/API-key overrides for chat/embedding and chat options map. |
| inception/inception-assistant/src/main/java/de/tudarmstadt/ukp/inception/assistant/config/AssistantProperties.java | Adds default + effective (resolved) provider/URL/API-key accessors. |
| inception/inception-assistant/src/main/java/de/tudarmstadt/ukp/inception/assistant/config/AssistantEmbeddingProperties.java | Adds embedding provider/URL/API-key override getters. |
| inception/inception-assistant/src/main/java/de/tudarmstadt/ukp/inception/assistant/config/AssistantChatProperties.java | Adds chat provider/URL/API-key override getters and a provider-specific options map. |
| inception/inception-assistant/src/main/java/de/tudarmstadt/ukp/inception/assistant/config/AssistantAutoConfiguration.java | Wires LlmChatClientExtensionPoint into assistant service construction. |
| inception/inception-assistant/src/main/java/de/tudarmstadt/ukp/inception/assistant/AssistantServiceImpl.java | Resolves the chat client by configured provider and updates agent construction. |
| inception/inception-assistant/src/main/java/de/tudarmstadt/ukp/inception/assistant/AgentLoop.java | Migrates the loop to LlmChatClient + neutral message/options types and tool descriptors. |
Comments suppressed due to low confidence (1)
inception/inception-assistant/src/main/java/de/tudarmstadt/ukp/inception/assistant/AgentLoop.java:309
firstTokenTimecan remain0if the streaming callback never emits any chunks (e.g. a tool-call-only assistant turn with no content/thinking deltas). This makes delay negative and duration inflated in the performance metrics.
// Send a final and complete message also including final metrics
var responseMessageBuilder = newMessage(responseId) //
.withContent(response.message().content()) //
.withThinking(response.message().thinking()) //
.withPerformance(MPerformanceMetrics.builder() //
.withDelay(firstTokenTime.get() - startTime) //
.withDuration(endTime - firstTokenTime.get()) //
.withTokens(tokens) //
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
reckart
force-pushed
the
refactoring/6040-Improve-LLM-infrastructure
branch
from
June 22, 2026 20:29
ae2a77a to
207d4e1
Compare
reckart
changed the title
reckart
force-pushed
the
refactoring/6040-Improve-LLM-infrastructure
branch
from
June 24, 2026 18:40
207d4e1 to
3d055d4
Compare
reckart
force-pushed
the
refactoring/6040-Improve-LLM-infrastructure
branch
2 times, most recently
from
July 12, 2026 17:09
c91eebe to
f7f0ede
Compare
- Migrate the assistant's AgentLoop from the Ollama-specific OllamaClient to the provider-neutral LlmChatClient (chat, streaming, structured call, tool calling, usage, endpoint/auth) - Resolve the chat client via LlmChatClientExtensionPoint in AssistantServiceImpl/AssistantAutoConfiguration (provider currently fixed to Ollama) - Extend the shared ChatMessage with a toolCalls field so assistant tool-call turns survive in the conversation history - Translate assistant tool-call turns (and thinking) onto the Ollama wire in OllamaLlmChatClient.toOllamaMessage - Add a provider-specific options map (assistant.chat.options) as an escape hatch for generation knobs that have no provider-neutral equivalent; entries override the typed settings - Update AgentLoopTest to drive the new LlmChatClient-based constructor (wrapping OllamaClientImpl in OllamaLlmChatClient) - Drop the test scope from slf4j-api in inception-imls-azureai-openai so it is available at compile time
- Add provider-neutral temperature and topP fields (nullable Double, unset = use provider default) to ChatOptions - Translate the neutral temperature/topP into each provider's own parameters in the Ollama, ChatGPT and Azure OpenAI adapters, with explicit options-bag entries overriding them - Pass temperature/topP from the assistant as the neutral ChatOptions fields instead of Ollama-keyed entries in the option bag - Add a builder to ChatOptions and use it in AgentLoop and ChatBasedLlmRecommenderImplBase - Defensively copy tools and options into immutable collections in the ChatOptions compact constructor so the record cannot be mutated through a retained caller reference - Configure maven-dependency-plugin in inception-imls-azureai-openai to ignore the analyze-only false positive for slf4j-api (kept at compile scope because it is needed transitively at compile time / by Eclipse)
- Add a top-level assistant.provider default plus per-section provider/url/api-key overrides under assistant.chat and assistant.embedding, so chat and embeddings can target different LLM providers/endpoints - Add effective-setting resolver methods (getChatProvider/Url/ApiKey, getEmbeddingProvider/Url/ApiKey) on AssistantProperties that fall back from the section override to the top-level default - Resolve the chat client by the effective chat provider id in AssistantServiceImpl instead of hardcoding Ollama, and build the chat endpoint from the effective chat url/api-key in AgentLoop - Route the model-capability auto-detection to the effective chat url/api-key - Resolve the embedding client/endpoint by the effective embedding provider/url and build authentication from the effective embedding api-key (previously always unauthenticated)
- Add LLM authentication infrastructure (LlmAuth interface/implementation) for API key management across providers - Introduce ModelCapability enum and supportedCapabilities() API to declare backend features (STREAMING, TOOLS, CHAT, JSON_SCHEMA) - Expand ChatOptions with neutral top-k, repeat-penalty, and context-length fields for provider-agnostic parameter handling - Add ChatOptionsTranslator utility for mapping neutral ChatOptions to provider-specific parameters - Add ModelDetails class for exposing model configuration and capabilities via LlmChatClient - Implement chatStream() API across OpenAI and Azure adapters for streaming-first response handling - Add tool-call support to OpenAI/Azure clients via ChatCompletionFunction, ChatCompletionTool, ChatCompletionToolCall, and ChatCompletionUsage classes - Refactor ChatGptClientImpl and AzureAiOpenAiClientImpl to support both blocking and streaming chat operations with configurable stream callback - Add ChatCompletionStreamOptions for controlling streaming behavior in OpenAI/Azure requests - Enhance AgentLoop to check model capabilities before attempting tool calling or streaming, with graceful fallback - Add LlmAuth apiKeyAuth() static import for standardized API key authentication across adapters - Extend OllamaLlmChatClient to support streaming via chatStream() API - Refactor ChatGptLlmChatClientTest and OllamaLlmChatClientTest to test streaming and non-streaming flows separately - Add AgentLoopCapabilityGatingTest to verify tool calling and streaming are skipped when model capabilities are absent - Reorganize test structure: rename OpenAiClientTest to AzureAiOpenAiClientIntegrationTest, add AzureAiOpenAiClientTest for unit tests - Add ChatGptLlmChatClientIntegrationTest and OpenAiClientIntegrationTest for end-to-end OpenAI client scenarios - Add AzureAiOpenAiLlmChatClientTest for unit testing Azure adapter - Update AssistantAutoConfiguration to wire LlmAuth into the assistant module - Update AssistantPropertiesImpl to support API key authentication configuration - Refactor EmbeddingServiceImpl to use LlmAuth for standardized credential handling - Update settings_assistant.adoc documentation to reflect new authentication and capability-based configuration - Replace direct Option/JacksonOption references in AgentLoop with static imports for consistency - Use a shared SSE parser for OpenAI and Azure providers
- Add reasoning/thinking effort support: new ReasoningEffort enum with model-default semantics, ChatOptions.reasoningEffort field, and adapter-specific mappings to OpenAI reasoning_effort and Ollama think levels
- Add tool_call_id tracking: MToolCall now includes an id field, threaded from wire protocol through AgentLoop to preserve OpenAI/Azure correlation semantics (Ollama uses positional matching)
- Implement JSON response sanitization: JsonResponseSanitizer utility to strip Markdown code fences from JSON mode responses (adapters now apply this conditionally based on ChatOptions.isJsonRequested())
- Fix Ollama thinking mode: change think field from hardcoded false to null=model-default, support level strings ("low"/"medium"/"high"/"max") for depth control alongside boolean true/false
- Add ChatOptions.isJsonRequested() helper to detect JSON-mode responses (jsonSchema supplied or responseFormat=JSON)
- Refactor OpenAI/Azure reasoning_effort handling: map ReasoningEffort.{MODEL_DEFAULT,NONE,MAX} to OpenAI's {null,"high"} space, null field = model default
- Add OllamazureContainer test fixture: testcontainers-based Ollama with Azure OpenAI client for live integration testing
- Split Ollama/Azure integration test scope: testcontainers dependency added to inception-testing as compile-scoped to support LLM test helpers
- Add new test classes: ChatCompletionMessageTest, AzureAiOpenAiClientLiveTest, AzureAiOpenAiLlmChatClientIntegrationTest
- Fix content trimming: non-streaming chat responses now trim surrounding whitespace (streaming path forwards raw deltas unchanged)
- Reorganize AgentLoop timing: set firstTokenTime=0 before non-streaming chat() call so duration attribution is correct
reckart
force-pushed
the
refactoring/6040-Improve-LLM-infrastructure
branch
from
July 12, 2026 18:14
f7f0ede to
53973fa
Compare
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.
What's in the PR
How to test manually
Automatic testing
Documentation