Skip to content

feat(dotAI): Dot AI LangChain4J integration (First provider/OpenAI)#35150

Open
ihoffmann-dot wants to merge 76 commits intomainfrom
dot-ai-langchain-integration
Open

feat(dotAI): Dot AI LangChain4J integration (First provider/OpenAI)#35150
ihoffmann-dot wants to merge 76 commits intomainfrom
dot-ai-langchain-integration

Conversation

@ihoffmann-dot
Copy link
Copy Markdown
Member

@ihoffmann-dot ihoffmann-dot commented Mar 27, 2026

Summary

Replaces the direct OpenAI HTTP client (OpenAIClient) with a LangChain4J abstraction layer, enabling multi-provider AI support in dotCMS. This PR covers Phase 1: OpenAI via LangChain4J.

Changes

  • LangChain4jAIClient.java: New AIProxiedClient implementation that delegates chat, embeddings, and image requests to LangChain4J models.
  • LangChain4jModelFactory.java: Factory that builds ChatModel, EmbeddingModel, and ImageModel instances from a ProviderConfig. Only place with provider-specific builder logic.
  • ProviderConfig.java: Deserializable POJO for the providerConfig JSON secret (per provider section: model, apiKey, endpoint, maxTokens, maxCompletionTokens, etc.).
  • AppConfig.java: Replaced legacy individual-field secrets (apiKey, model, etc.) with a single providerConfig JSON string. isEnabled() now only checks this field.
  • AIAppValidator.java: Removed the OpenAI /v1/models validation call, which is incompatible with the multi-provider architecture.
  • CompletionsResource.java: Updated /api/v1/ai/completions/config to derive model names and config values from AppConfig getters instead of iterating raw AppKeys.
  • dotAI.yml: Removed legacy hidden fields; added providerConfig as the single configuration entry point.
  • Tests: Added unit tests for ProviderConfig, LangChain4jModelFactory, and LangChain4jAIClient; updated AIProxyClientTest integration test to use providerConfig-based setup.

Motivation

The previous implementation was tightly coupled to OpenAI's API contract (hardcoded HTTP calls, OpenAI-specific parameters, model validation via /v1/models). LangChain4J provides a provider-agnostic model interface, allowing future phases to add Azure OpenAI, AWS Bedrock, and Vertex AI without touching the core request/response flow.

The providerConfig JSON secret replaces multiple individual secrets with a single structured configuration, supporting per-section (chat/embeddings/image) provider and model settings.

Related Issue

This PR fixes #35183
EPIC: dotAI Multi-Provider Support #33970


Note

High Risk
High risk because it replaces the core AI client/provider path (OpenAI HTTP + model fallback/validation) with a new LangChain4J-backed implementation and a new providerConfig secret format, impacting chat, embeddings, and image generation behavior and configuration compatibility.

Overview
dotAI now routes chat, embeddings, and image requests through a new LangChain4J-backed client (LangChain4jAIClient) and sets LangChain4J as the default provider, replacing the direct OpenAI HTTP client and removing the model-fallback strategy.

Configuration is migrated from many per-field secrets to a single providerConfig JSON (with hashing + per-host model caching), updating AppConfig.isEnabled(), dotAI.yml, and the /v1/ai/completions/config output (including credential redaction). Several OpenAI-specific model management/validation classes and tests are removed, and integration/unit tests are updated/added for the new providerConfig + LangChain4J flow.

Embeddings/image handling is adjusted: embeddings requests now send raw text (with token-count fallback when encoding is unavailable), the async thread pool key is renamed to AIThreadPool, max-token resolution is made more resilient, and image temp-file creation now supports base64 (b64_json) responses.

Reviewed by Cursor Bugbot for commit 31cb86e. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions github-actions bot added the Area : Backend PR changes Java/Maven backend code label Mar 27, 2026
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Mar 27, 2026

Claude finished @ihoffmann-dot's task in 2m 37s —— View job


PR Review: feat(dotAI) LangChain4J Integration

  • Reading key changed files
  • Reviewing LangChain4jAIClient.java
  • Reviewing LangChain4jModelFactory.java
  • Reviewing ProviderConfig.java / AppConfig.java
  • Reviewing EmbeddingsAPIImpl.java / CompletionsAPIImpl.java
  • Reviewing test coverage
  • Final summary

Bugs

1. Streaming chunks are never flushed — real-time output is silently broken

onPartialResponse writes each SSE chunk to output but never flushes it. The bytes sit in the servlet output buffer until the entire stream completes (or the buffer fills), so callers see all tokens arrive at once — identical behavior to non-streaming. This defeats the purpose of streaming.

LangChain4jAIClient.java:208:

output.write(toSseChunk(token).getBytes(StandardCharsets.UTF_8));
// missing: output.flush()

Fix: call output.flush() after every output.write(...) in onPartialResponse. Fix this →


2. buildStreamingChatModel silently drops maxRetries

LangChain4jModelFactory.java:133 passes ignored -> {} as the retries consumer:

// streaming — maxRetries silently ignored
applyCommonConfig(config, builder::baseUrl, ignored -> {}, builder::timeout);

// non-streaming — correctly wired
applyCommonConfig(config, builder::baseUrl, builder::maxRetries, builder::timeout);

Copy-paste mistake. Streaming models will never retry on transient failures regardless of the configured maxRetries. Fix this →


3. Streaming timeout leaves model running — cancelled flag is not a real cancel

LangChain4jAIClient.java:236–239: On timeout, cancelled.set(true) is set, but LangChain4J has no cancellation API. The upstream model keeps calling onPartialResponse; subsequent tokens are silently dropped while the underlying HTTP connection stays open, consuming resources until the provider eventually closes it.

This is a known limitation of LangChain4J's streaming API, but it should be documented or tracked. There is also no interrupt-path for the provider thread.


4. Exception type erasure in cache loader errors

All four model initialization cache loaders re-wrap every failure as IllegalArgumentException:

// LangChain4jAIClient.java lines ~161, ~187, ~282, ~299
throw new IllegalArgumentException("Failed to initialize chat model: " + cause.getMessage(), cause);

Auth errors (401), network unreachable, and bad config all collapse into the same exception type. This will frustrate diagnosis in production logs. Consider preserving the original type or using a more specific dotCMS exception that carries the root cause.


Design / Risk

5. Rollback is one-way (already flagged in prior comment, still unresolved)

After an admin saves the new providerConfig-only secret under N, rolling back to N-1 leaves the AI app silently disabled (isEnabled() returns false because apiKey is blank). The two-phase migration approach (keep legacy fields in dotAI.yml as hidden/deprecated in N, remove in N+1) was proposed but not implemented.

6. Rate limiting removed with no replacement

tokensPerMinute and apiPerMinute are gone from AppKeys and ProviderConfig. High-throughput sites will hit provider 429s without any client-side backpressure. maxRetries handles transient errors but does not prevent rate limit exhaustion. This was flagged in the review; the PR has no response to whether this will be addressed in a follow-up.

7. API_KEY still in AppKeys with a live getter

AppKeys.API_KEY is still present and getApiKey() is @Deprecated but functional. Both AppConfig constructor (line 61) and configValues still read it from secrets. If the new providerConfig format already embeds the key per section, storing it separately in apiKey is dead weight and a credential hygiene risk — a stale apiKey secret that is never used but still present in the secrets store.

8. AtomicReference<Boolean> should be AtomicBoolean

LangChain4jAIClient.java:199:

final AtomicReference<Boolean> cancelled = new AtomicReference<>(false);

AtomicBoolean is the correct type for a boolean flag shared across threads.


Minor

9. toSseChunk omits fields that existing SSE consumers may expect

The delta chunks produced by toSseChunk include only content in the delta, but no id, model, or object field. LineReadingOutputStream and any frontend code parsing the stream against the OpenAI SSE contract may behave unexpectedly on the missing fields.

10. generateEmbeddings private method name

EmbeddingsAPIImpl.generateEmbeddings (line 439) is the method that was previously sendTokensToOpenAI. The rename to generateEmbeddings was done, but the method's Javadoc still says "Posts the specified list of tokens to the OpenAI Embeddings Endpoint" and references tokens in the @param block even though the signature changed to content. The stale doc is misleading.


@ihoffmann-dot ihoffmann-dot marked this pull request as ready for review March 30, 2026 23:15
host = new SiteDataGen().nextPersisted();
AiTest.aiAppSecretsWithProviderConfig(
host,
AiTest.providerConfigJson(AiTest.PORT, "gpt-4o-mini"));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will this aiAppSecretsWithProviderConfig work when you implement new providers? will you have to call it for each provider?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. This is just a test with an OpenAI model, but aiAppSecretsWithProviderConfig just saves a JSON string and doesn’t know about the provider. It will work with any model. To test other providers we only need to pass a different JSON in the providerConfigJson param.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, this test was implemented to work with an OpenAI model. When you implement the other providers and want to include new tests for them, you will have to refactor it to make it scalable; otherwise, your test class will contain duplicate logic, become huge, and /or difficult to follow

Comment thread dotcms-integration/src/test/java/com/dotcms/ai/AiTest.java Outdated
Comment thread dotCMS/src/main/java/com/dotcms/ai/api/EmbeddingsAPIImpl.java
Comment thread dotCMS/src/main/java/com/dotcms/ai/api/EmbeddingsAPIImpl.java Outdated
Comment thread dotCMS/src/main/java/com/dotcms/ai/client/langchain4j/ProviderConfig.java Outdated
Comment thread dotCMS/src/main/java/com/dotcms/ai/client/langchain4j/ProviderConfig.java Outdated
@dotCMS dotCMS deleted a comment from claude bot Apr 16, 2026
@dotCMS dotCMS deleted a comment from claude bot Apr 16, 2026
@dotCMS dotCMS deleted a comment from claude bot Apr 17, 2026
Comment thread dotcms-integration/src/test/java/com/dotcms/ai/AiTest.java
Copy link
Copy Markdown
Member

@fabrizzio-dotCMS fabrizzio-dotCMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review — LangChain4J Integration

Reviewed the full diff. Overall the architecture direction is solid (single providerConfig JSON, LangChain4J abstraction, hash-keyed model cache). Issues below are grouped by severity.


Bugs

1. buildStreamingChatModel silently drops maxRetriesLangChain4jModelFactory.java

// current — maxRetries is silently ignored for streaming
applyCommonConfig(config, builder::baseUrl, ignored -> {}, builder::timeout);

// non-streaming correctly passes it:
applyCommonConfig(config, builder::baseUrl, builder::maxRetries, builder::timeout);

Streaming models will never retry on transient failures. Looks like a copy-paste mistake — the second argument should be builder::maxRetries, not ignored -> {}.


2. temperature=0 is suppressed — CompletionsAPIImpl.java

if (temperature > 0) {
    json.put(AiKeys.TEMPERATURE, temperature);
}

Temperature 0.0 is valid and meaningful (fully deterministic output). If 0 was being used as a "not set" sentinel, a comment or a distinct sentinel (e.g., -1) is needed. As written, deterministic use cases silently get the model's default temperature instead.


3. generateEmbeddings receives tokens but never uses them — EmbeddingsAPIImpl.java

The method signature still has @NotNull final List<Integer> tokens but the body only logs it and then sends content as the input. The parameter is dead weight and should be removed from the signature to avoid confusion.


Missing Behavior / Regressions

4. Rate-limiting removed entirely

tokensPerMinute and apiPerMinute per model have been removed from AppKeys and ProviderConfig. High-volume installations will hit API 429s without any client-side backpressure. maxRetries partially compensates, but it is not equivalent to proactive rate limiting.


Design / Minor

5. sha256Hex reimplemented from scratch — AppConfig.java

org.apache.commons.codec.digest.DigestUtils.sha256Hex() is already on the classpath (commons-codec is a transitive dependency). The 14-line custom implementation is unnecessary.

Comment thread dotCMS/src/main/java/com/dotcms/ai/api/CompletionsAPIImpl.java
Comment thread dotCMS/src/main/java/com/dotcms/ai/app/AppConfig.java Outdated
@dotCMS dotCMS deleted a comment from claude bot Apr 17, 2026
@dotCMS dotCMS deleted a comment from claude bot Apr 17, 2026
Comment thread dotCMS/src/main/java/com/dotcms/ai/api/CompletionsAPIImpl.java Outdated
@dotCMS dotCMS deleted a comment from claude bot Apr 17, 2026
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 17, 2026

Pull Request Unsafe to Rollback!!!

  • Category: M-3 — REST / GraphQL / Headless API Contract Change
  • Risk Level: 🟡 MEDIUM
  • Why it's unsafe: The /api/v1/ai/completions/config endpoint response has been restructured. N removes ~14 response fields (textModelNames, imageModelNames, embeddingsModelNames, textModelMaxTokens, imageModelMaxTokens, embeddingsModelMaxTokens, all *PerMinute fields, availableModels, and apiKey) and replaces them with a single providerConfig JSON field plus a handful of prompt fields. Any consumer of this endpoint — including the dotCMS Angular admin if it is updated separately — built against N's contract will break on rollback to N-1, which restores the old per-field format.
  • Code that makes it unsafe: dotCMS/src/main/java/com/dotcms/ai/rest/CompletionsResource.java lines 177–207 — the config endpoint now builds a map with AppKeys.PROVIDER_CONFIG.key, AppKeys.ROLE_PROMPT.key, etc. instead of iterating over all AppKeys.values() and appending availableModels.
  • Alternative (if possible): Keep the old per-field keys in the response alongside providerConfig (derived from it) so N-1 and N consumers both receive what they expect. Remove the legacy keys in a subsequent release once N-1 is outside the rollback window.

  • Category: H-1 (analogous) — One-Way Application Configuration Migration
  • Risk Level: 🟠 HIGH
  • Why it's unsafe: dotAI.yml removes all individual secret fields (apiKey, textModelNames, imageModelNames, embeddingsModelNames, and all per-model rate/token fields) and replaces them with a single providerConfig JSON secret. App secrets are persisted in the dotcms_apps_secrets database table. Once an administrator saves the dotAI configuration under N (which writes only providerConfig to the DB), rolling back to N-1 is effectively a one-way migration: N-1's AppConfig.isEnabled() evaluates Stream.of(apiUrl, apiImageUrl, apiEmbeddingsUrl, apiKey).allMatch(StringUtils::isNotBlank) and returns false because apiKey is now blank in the secrets store. All AI features (chat completions, embeddings, image generation) are silently disabled after rollback — with no error thrown and no automatic recovery path.
  • Code that makes it unsafe:
    • dotCMS/src/main/resources/apps/dotAI.yml — removes apiKey, textModelNames, imageModelNames, embeddingsModelNames, and 12 other individual fields; adds providerConfig as the sole required entry.
    • dotCMS/src/main/java/com/dotcms/ai/app/AppConfig.java lines 882–892 — N-1's isEnabled() requires apiKey + URL fields to be non-blank; after N's config save, apiKey no longer exists in the secret store.
  • Alternative (if possible): Follow the two-phase migration pattern: in N, keep apiKey (and the other legacy fields) in dotAI.yml as hidden/deprecated but still present, so N-1 can still read them after rollback. Populate providerConfig from those fields as a convenience in N. Remove the legacy fields only in N+1, once N-1 is outside the rollback window.

@dotCMS dotCMS deleted a comment from claude bot Apr 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Not Safe To Rollback Area : Backend PR changes Java/Maven backend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

[FEATURE] dotAI: LangChain4J integration — Phase 1 (OpenAI)

4 participants