Skip to content

feat(dotAI): Dot AI LangChain4J - Custom UI for provider config#35445

Merged
ihoffmann-dot merged 30 commits into
mainfrom
dot-ai-langchain-custom-ui
Apr 24, 2026
Merged

feat(dotAI): Dot AI LangChain4J - Custom UI for provider config#35445
ihoffmann-dot merged 30 commits into
mainfrom
dot-ai-langchain-custom-ui

Conversation

@ihoffmann-dot
Copy link
Copy Markdown
Member

Summary

Replaces the generic Apps UI textarea for providerConfig with a dedicated Angular screen that shows the current config and an example JSON side-by-side. Also ports backend fixes from #35426 and adds a PUT /api/v1/ai/completions/config endpoint with credential-preserving merge.

  • New DotAiConfigDetailComponent: two-column layout — editable textarea on the left, formatted example JSON on the right
  • DotAiConfigDetailResolver: dedicated resolver that hardcodes dotAI as appKey, fixing the 404 caused by the generic resolver reading null from route params
  • Route dotAI/edit/:id added before the generic :appKey/edit/:id so dotAI navigates to the custom screen
  • ChangeDetectorRef.detectChanges() after async config load to fix textarea not rendering value until user interaction
  • PUT /api/v1/ai/completions/config (admin-only): saves providerConfig JSON; ProviderConfigMerger preserves stored credentials when the payload contains ***** sentinel values
  • dotAI.yml description updated to reference OpenAI/ChatGPT directly
  • Ported from fix(dotAI): Dot AI LangChain4J - ProviderConfig fixes #35426: flush SSE chunks, cancelled flag on IOException, maxRetries warn for streaming, null check in parseSection, deepCopy in injectApiKeyIntoSections, maxTokensmax_completion_tokens routing for OpenAI o-series models, PR review refactors

Configuration

{
  "chat": {
    "provider": "openai",
    "apiKey": "sk-...",
    "model": "gpt-4o",
    "maxTokens": 16384,
    "temperature": 1.0,
    "maxRetries": 3,
    "rolePrompt": "You are dotCMSbot, an AI assistant to help content creators.",
    "textPrompt": "Use Descriptive writing style."
  },
  "embeddings": {
    "provider": "openai",
    "apiKey": "sk-...",
    "model": "text-embedding-ada-002",
    "listenerIndexer": { "default": "blog,news,webPageContent" }
  },
  "image": {
    "provider": "openai",
    "apiKey": "sk-...",
    "model": "dall-e-3",
    "size": "1792x1024",
    "imagePrompt": "Use 16:9 aspect ratio."
  }
}

Notes

  • The custom UI is accessed via #/apps/dotAI/edit/:siteId; the "App screen" link in render.jsp already points to this URL
  • Credential masking: fields with ***** in a PUT payload are replaced with the stored values before saving, so partial edits don't wipe secrets
  • providerConfig is not required — omitting it disables the AI features gracefully

Related Issues

@ihoffmann-dot ihoffmann-dot marked this pull request as draft April 23, 2026 23:42
@github-actions github-actions Bot added Area : Backend PR changes Java/Maven backend code Area : Frontend PR changes Angular/TypeScript frontend code labels Apr 23, 2026
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 23, 2026

Claude finished @ihoffmann-dot's task in 3m 11s —— View job


PR Review

  • Read frontend component files
  • Read backend Java files
  • Read test files
  • Compile findings and post review

Issues Found

1. Model cache not flushed after credential save — stale API keys persist for up to 1 hour

CompletionsResource.saveConfig writes new credentials to AppsAPI but never calls LangChain4jAIClient.get().flushCachesForHost(host.getHostname()). LangChain4jAIClient caches model instances keyed on the config hash with a 1-hour TTL — those instances have the old API key baked in at construction time. After a user rotates their API key via this endpoint, all AI calls continue using the stale key until the cache naturally expires or the server restarts.

// CompletionsResource.java ~line 266 — should follow saveSecret with:
LangChain4jAIClient.get().flushCachesForHost(host.getHostname());

Fix this →


2. Host resolution mismatch between resolver and config operations

dotAiConfigDetailResolver fetches app metadata by site ID (DotAppsService.getConfiguration(DOT_AI_APP_KEY, id)) so the header shows the correct site name. But getConfig() and saveConfig() both derive the host from the HTTP request (WebAPILocator.getHostWebAPI().getCurrentHostNoThrow(request)) — not from the route id. In a multi-host setup, the displayed site name and the actual config being loaded/saved could be for different hosts, making the UI misleading or saving to the wrong host.

The component passes the site ID through the route but never passes it to DotAiService.saveConfig(). Either the backend endpoint should accept a siteId query parameter, or the service should include it.


3. Empty textarea while config loads — user can submit before data arrives

getConfig() is called in ngOnInit after route data resolves. During that HTTP call the textarea is blank and fully editable. If the user clicks Save before the response arrives, onSubmit() will JSON-validate the empty string, fail the JSON.parse, and show an error — but a user who clears the field and saves quickly could trigger a save of "" (blank), which the backend allows (it returns 400 "Request body is required" only for blank). The real risk is an admin who starts typing before the existing config loads, unknowingly discarding it.

A readonly loading = signal(true) with [disabled]="loading()" on the Save button and the textarea would prevent this.


4. saveConfig sends string through Angular HttpClient with Content-Type: application/json — works but fragile

saveConfig(json: string): Observable<DotAiProviderConfig> {
    return this.#http.put<DotAiProviderConfig>(`${API_ENDPOINT}/completions/config`, json, {
        headers  // Content-Type: application/json
    });
}

Angular's HttpClient passes string bodies as-is, so the raw JSON from the textarea is transmitted correctly. However, if json is not valid JSON (which onSubmit() checks before calling), the server receives malformed JSON with Content-Type: application/json. The backend does catch this and returns 400, but the client-side guard in onSubmit() (JSON.parse(this.configJson())) should be sufficient. No immediate bug, but worth noting that the pre-validation and the raw-string transmission are tightly coupled.


5. containsMasked is public with documented false-positive behaviour — invite to misuse

public static boolean containsMasked(final String json) { ... }

The Javadoc explicitly says this method "also matches non-credential fields" and is "not suitable for post-merge validation." It's public, so any future caller not reading the Javadoc carefully will use it for the wrong purpose. Since the only caller is saveConfig as a cheap pre-filter, this should be static package-private or at minimum @VisibleForTesting.


6. DotAiProviderConfig re-exported from the service layer

// dot-ai.service.ts
export { DotAiProviderConfig };

This creates a second import path alongside the canonical @dotcms/dotcms-models. Future callers will import it from the service, which blurs the boundary between data access and models. The component already imports DotApp from @dotcms/dotcms-models directly — DotAiProviderConfig should follow the same pattern.


7. No component spec for DotAiConfigDetailComponent

Per core-web/CLAUDE.md, Jest + Spectator tests are required for all components. No dot-ai-config-detail.component.spec.ts was added. Critical paths to cover: config loads and populates textarea, invalid JSON blocks submit, valid JSON triggers saveConfig, cancel navigates to apps.


8. Streaming onCompleteResponse silently swallows [DONE] write failure

@Override
public void onCompleteResponse(final ChatResponse response) {
    try {
        output.write("data: [DONE]\n\n".getBytes(StandardCharsets.UTF_8));
    } catch (IOException e) {
        Logger.warn(LangChain4jAIClient.class, "Failed to write [DONE] marker: " + e.getMessage());
    } finally {
        latch.countDown();
    }
}

The IOException is caught and logged at WARN, then the latch is released. The error AtomicReference is never set here, so streamWithModel treats this as a successful completion even though the client never received the stream terminator. Consider setting error.set(e) inside the catch block here, consistent with how onError handles failures.


Minor / Nits

  • dot-ai-config-detail-resolver.service.ts file name says "service" but it exports a function resolver (ResolveFn), not a class. Naming is misleading.
  • In dot-apps.routes.ts, the dotAI/edit/:id route doesn't include providers: [DotAppsService, DotAppsConfigurationDetailResolver] — intentional since it uses its own resolver, but DotAppsService is still needed. It's included, which is correct.
  • EXAMPLE_CONFIG constant at the top of the component file hardcodes sk-... as example API keys; fine for a UI example but worth a comment that these are illustrative only (they match what's in the PR description).
  • The o[0-9].* regex in LangChain4jModelFactory.applyOpenAiTokenLimit correctly routes o-series OpenAI models to maxCompletionTokens. It would also match a hypothetical non-OpenAI model named o1-something from another provider — low risk now, but as more providers are added this routing logic should move into the provider-specific builder.

@ihoffmann-dot ihoffmann-dot marked this pull request as ready for review April 24, 2026 01:35
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 24, 2026

Pull Request Unsafe to Rollback!!!

  • Category: M-3 — REST / Headless API Contract Change

  • Risk Level: 🟡 MEDIUM

  • Why it's unsafe: The PR introduces a new PUT /api/v1/ai/completions/config endpoint and simultaneously updates the Angular admin frontend to call it when saving AI provider configuration (saveConfig() in DotAiService). If the backend is rolled back to N-1, this endpoint does not exist. Any browser session or CDN-cached copy of the N-era Angular code will receive a 404 when the user tries to save AI configuration via the new custom screen — the save operation is completely non-functional until browser caches clear or a CDN purge completes. This matches the M-3 scenario exactly: "N's frontend (cached by CDN or browser) calls an endpoint that only exists in N."

    Additionally, the GET /api/v1/ai/completions/config response drops five fields — rolePrompt, textPrompt, imagePrompt, imageSize, listenerIndexer — without a deprecation period. Any external API consumer parsing those fields from the config endpoint will find them absent immediately after N is deployed. Rollback restores them (the removal is in code, not data), so this is a forward-breaking change rather than a pure rollback-unsafe one, but it accompanies the M-3 signal.

  • Code that makes it unsafe:

    • core-web/libs/data-access/src/lib/dot-ai/dot-ai.service.ts — new saveConfig() method issues PUT ${API_ENDPOINT}/completions/config (lines ~119–122); if N is rolled back this endpoint vanishes and any cached Angular page gives a 404 on save
    • dotCMS/src/main/java/com/dotcms/ai/rest/CompletionsResource.java@PUT @Path("/config") handler only exists in N; the five map.put(AppKeys.ROLE_PROMPT…) etc. lines are removed from the GET handler (lines ~185–189), dropping those fields from the response
    • dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml — adds the PUT operation, confirming it has no prior existence in N-1
  • Alternative (if possible): Use a two-phase deployment: add the PUT /api/v1/ai/completions/config endpoint stub to N-1 first (returning 501 Not Implemented, or the full implementation), then ship the Angular saveConfig() call in N. This means rolling back N still leaves a valid endpoint for any cached N-era Angular code. Alternatively, include a CDN/browser-cache purge as a required step in the rollback runbook so clients immediately reload N-1 Angular code.

@dotCMS dotCMS deleted a comment from claude Bot Apr 24, 2026
@ihoffmann-dot ihoffmann-dot added this pull request to the merge queue Apr 24, 2026
Merged via the queue into main with commit 4c55a47 Apr 24, 2026
56 checks passed
@ihoffmann-dot ihoffmann-dot deleted the dot-ai-langchain-custom-ui branch April 24, 2026 19:17
ihoffmann-dot added a commit that referenced this pull request Apr 28, 2026
ihoffmann-dot added a commit that referenced this pull request Apr 28, 2026
…35456) (#35494)

## Summary
Reverts the LangChain4J integration and related changes merged in:
- #35150 — LangChain4J integration (Phase 1 / OpenAI)
- #35445 — Custom UI for provider config
- #35456 — Per-site config support via siteId

Restores dotAI to the state at tag `v26.04.28-01` prior to those merges.

## Test plan
- Verify dotAI app configuration UI works as before
- Verify AI completions, embeddings, and image generation function with
the original OpenAI client
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Not Safe To Rollback AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code Area : Frontend PR changes Angular/TypeScript frontend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants