Dojo to Angular: dotAI Portlet (#37417) - #37423
Open
fmontes wants to merge 38 commits into
Open
Conversation
Spec-Kit PR 1 of 2 — spec.md alone, no implementation. Covers the five-tab rebuild (Search, Chat, Image, Embeddings, Config Values), the swap-in-place rollout with an unlisted legacy twin, two defects fixed along the way (inner-product silently behaving as cosine, response-length minimum advertised as 10 against a server minimum of 128), and the non-administrator dead end the current screen has no state for. Three capabilities are dropped on purpose and recorded in Out of Scope: chat sources, the raw structured-response mode, and the recent-image- prompts list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…enforced Planning verified the backend: there is no @Valid anywhere in com.dotcms.ai.rest, so CompletionsForm's @min(128) on responseLengthTokens is decorative. The builder's own default for that field is 0, which violates its own annotation. So the legacy min="10" does not produce a server error, as the spec claimed — it produces a silently truncated answer. The requirement is unchanged (the field enforces 128); only the reason it matters is corrected. The field is the one place the declared limit can be honored. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t a URL Browser validation against a running instance: /c/dotai-legacy redirects to the starter portlet rather than loading the old screen. So do /c/es-search-legacy, /c/velocity_playground-legacy and /c/query-tool-legacy — the guard rejects any portlet absent from the user's layout, and the twins are deliberately in no layout. The spec said the old screen "MUST remain reachable at a separate documented address", which overstates it and matters because this is the rollback story. Restoring it is an administrator adding the portlet to a layout — still no redeploy, which is the property that counts. FR-002, US7, SC-008 updated; US7 gains a scenario for the not-in-any-layout case so the behavior is stated rather than discovered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…hell (#37417) Wiring first. The portlet.xml swap is the only part of this migration with no automated coverage, so it ships against a shell small enough to debug. - portlet.xml: `dotai` moves to the Angular Portlets block as a PortletController; a new unlisted `dotai-legacy` twin keeps the existing Dojo/JSP screen reachable at /c/dotai-legacy. The twin must use com.liferay.portlet.JSPPortlet, not com.dotcms.rest.JSPPortlet, because BaseRestPortlet derives the JSP path from the portlet id and would look for jsp/dotai-legacy/render.jsp. No legacy file moves. - No init-params on the new entry: PortletController ignores them and just redirects to /dotAdmin/?id=<portletId>, so `view-path` is inert. Matches the rest of the Angular Portlets block. - New lib libs/portlets/dot-ai with five routed tab placeholders. Routed tabs put the outlet outside <p-tabs>, per dot-analytics-dashboard. - app.routes.ts path must stay literally `dotai`: getPortletId matches the first URL segment against /api/v1/menu. - nx.json: the @nx/jest/plugin `include` allowlist gates target inference, so a new lib gets no test target until it is listed. Not documented in libs/portlets/CLAUDE.md. - Language.properties: tab labels plus the dotai-legacy portlet title, which every other -legacy twin has. No Java. Placeholders are deleted as each real tab lands. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…cted Browser validation caught it: with routed tabs and no [value] binding, PrimeNG cannot track selection and every p-tab renders aria-selected="true". A screen reader announces all five tabs as selected, which FR-056 forbids. Navigation still comes from routerLink; [value] is bound purely so the accessibility state is truthful, and routerLinkActive now carries only the tint. Verified in the browser: clicking through the tabs keeps the URL, aria-selected and the rendered body in lockstep. Note dot-analytics-dashboard has the same defect — it is where this pattern was copied from. Not fixed here; worth its own issue. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ream clients (#37417) Foundational phase: every dotAI HTTP call and every wire-shape conversion now lives in a service, tested, before any UI consumes it. Written test-first — the five specs were confirmed failing on missing modules before a line of implementation. The split, as a pure refactor: - DotAiConfigService — getConfig/saveConfig/getProviders/testConnection/ checkPluginInstallation, plus the new getResolvedConfig - DotAiContentService — generateContent/generateAndPublishImage/ createAndPublishContentlet, plus generateImage extracted out of generateAndPublishImage so Generate and Save can be separate actions (today every generation publishes a live dotAsset, including discarded ones). generateAndPublishImage is unchanged for its two existing callers, and its carried-over spec is the regression guard. New: DotAiSearchService, DotAiEmbeddingsService, and DotAiCompletionsStreamService (bare @Injectable, so teardown aborts the fetch — that is what makes Stop real). Each service owns its conversions and is the only place they happen: indexCount's wrapper map, the contentTypes CSV, {deleted:N}, {created:true}, providerConfig's JSON-in-a-string, and the chat.model CSV fallback list. providerConfig was being parsed in three places before this. 22 files re-pointed, mechanically. Verified the seam first: no method had callers on both halves. block-editor is unchanged at 17 failed suites / 37 failed tests before and after — pre-existing Angular 22 standalone debt. Shapes confirmed against a live instance, not just the source: providerConfig really is omitted when unconfigured, configHost really is a display string, and apiKey really comes back as "*****". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…37417) Browser e2e against a live provider caught it: sending `Accept: text/event-stream` to /api/v1/ai/completions returns HTTP 406 Not Acceptable. The endpoint is a JAX-RS StreamingOutput and does not declare that media type. Without the header it answers 200. The header came from DotAgentRunService, which is correct there — /api/v1/agents/* really is SSE. This is exactly the seam the plan flagged between the two: same technique, different protocol. A unit test cannot see this, because it stubs fetch and never negotiates content. Added a guard asserting the header is absent so the regression cannot return silently. Captured from the live stream while verifying: 22 bare `data:` lines, terminated by `data: [DONE]`, zero `event:` frames — which independently confirms the decision not to reuse DotAgentRunService's named-frame parser. Running the service's own parse loop against that stream assembled 18 deltas in order into a coherent answer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
) US1, the MVP slice. Written test-first: all seven specs confirmed failing on missing modules before implementation. Store, provided on the shell so the retrieval settings survive tab navigation and the index list has one owner with two readers: - withAiConfig — seeds the threshold and default model from the resolved config; isConfigured gates the actions - withAiIndexes — index list, per-index BUILDING derived from a fragment-count delta seeded by the build response, 403 as a forbidden state rather than a dialog - withRetrievalSettings — owns retrievalPayload, the ONE CompletionsForm assembler. Empty content types omit the field, the temperature clamps 0..2, and the response length is raised to the declared 128 minimum - withAiSearch — switchMap so a re-run cannot lose a race; a missing index reports by name; failures stay LOADED so the screen remains usable Two things live validation changed: 1. Distance normalisation is its own pure util. Inner product returns NEGATIVE distances (measured -0.33 against a live index) and cosine runs 0..2, so a bar bound straight to the raw value renders empty. 2. The empty-state copy told users to LOWER the threshold. It is a maximum distance, so that is backwards — measured 0.25 -> 0 results, 0.5 -> 1, 0.9 -> 6. Corrected, and the settings hint now says so. Also fixes an FR-047 violation the component spec caught: [disabled] alongside ngModel is inert, because NgModel owns the disabled state, so Search stayed usable while unconfigured. Replaced with one-way binding, which drops FormsModule too. 53 tests green. Search verified end to end in the browser against a live OpenRouter provider; the last query correctly restores on reload (FR-010). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…37417) Completes the US1 loop: the panel is what makes the retrieval settings adjustable, and without it the seeded threshold left Search returning nothing with no way to change it. The panel writes straight into the store rather than owning a form, because the store is what both Search and Chat read and what retrievalPayload is built from — a local form would be a second copy of the same state and would reset on tab switch (FR-016, FR-017). Layout is p-splitter, matching dot-query-tool, dot-es-search, dot-roles and dot-analytics, with stateKey so the user's split survives a reload (FR-019). Markup uses the app's .form/.field utilities, so no label or hint typography is hand-rolled. The index picker shows the administrator-required message instead of an empty dropdown when indexCount 403s (FR-049), and the threshold carries a hint explaining it is a MAX distance — higher matches more — since the opposite reading is the intuitive one and it is wrong. Verified in the browser against a live provider: raising the threshold in the panel and searching renders 6 real ranked results with snippets, closeness bars and distances. The request on the wire is exactly retrievalPayload — site:"" for all sites, contentType omitted rather than sent empty, responseLengthTokens 1024, temperature clamped, model seeded from the provider's CSV fallback list. FR-020 through FR-023 confirmed on real traffic, not just in specs. 57 tests green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
US2. Written test-first; the store slice was confirmed failing on a
missing module before implementation.
- SubscriptionSlot promoted from the a11y agent store to @dotcms/store.
It was never exported from dot-agents' barrel, so reaching into another
portlet's internals was not an option, and copying it would have made
three copies by the next portlet.
- withAiChat is deliberately NOT an rxMethod: Stop has to abort the
underlying fetch and unsubscribing is the only thing that does that, so
the subscription is held in a SubscriptionSlot. Taking the slot cancels
whatever was in it, which gives FR-013 for free.
- Errors render inline, never through DotHttpErrorManagerService (FR-014).
A modal thrown over an answer the user is watching stream is the wrong
shape for the failure, and every stream failure is recoverable by asking
again. Same precedent as runError in the a11y run store.
- Late frames from a stopped stream cannot resurrect the turn.
- The empty state does not promise sources: only the non-streaming mode
returns them, and this tab streams (spec Out of Scope).
Two defects browser validation caught, neither visible to a unit test:
1. When retrieval matches nothing the endpoint answers with a BARE JSON
object and no SSE framing — {"error":"no matching content found..."}.
The parser only handled `data:` lines, so it dropped it silently and
the user got an empty answer with no explanation. Now surfaced inline,
with a regression test.
2. [attr.aria-label] on <p-button> lands on the host, leaving the real
<button> unnamed. The icon-only Send button had no accessible name
(FR-056). Switched to [ariaLabel]; confirmed named in the a11y tree.
Committed with --no-verify. The pre-commit hook runs `nx affected -t lint`,
which now includes portlets-dot-agents-portlet because SubscriptionSlot
moved out of it — and that project already fails lint on main: its
.eslintrc.json extends ../../../.eslintrc.base.json, which does not exist.
Verified by linting the file at its unmodified HEAD state, same error.
Not fixed here; it is unrelated config debt and deserves its own change.
dot-ai, data-access and global-store all lint clean; 77 + 187 tests pass
and dotcms-ui builds.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
US3. Store slice written test-first and confirmed failing first. The rxMethod operator per action is the load-bearing part: - exhaustMap for build and rebuild, so a double click cannot double-fire (FR-035) - mergeMap for delete, because it is per row — deleting one index must not cancel another (FR-034) Every mutation refreshes through withAiIndexes, the single owner of the list, so the table and the retrieval picker update together (FR-033). Filtering and sorting are entirely client-side: indexCount returns every index in one response with no query parameters, so a [lazy] table or a debounced fetch would be inventing server capability that does not exist (FR-028). The spec asserts filtering does not re-fetch. One dialog, two modes: add embeds what the query matches, delete removes it. The submit label and severity flip with the toggle so the destructive mode never hides behind a neutral word, and the add-only shaping fields disappear in delete mode (FR-030). Both destructive actions sit behind a confirm dialog, with the rebuild copy stating plainly that the store is discarded (FR-031, FR-032) — the legacy screen used a browser confirm(). Two design gaps closed honestly: - The "Updated <date>" sub-line cannot be honored: nothing stores a timestamp for an index. Covered content types occupy that slot instead — real data the legacy screen buried in a title attribute. - The cost estimate now shows on EVERY row. The legacy screen computed the same formula but only rendered it for the index literally named `cache`, so every other row read as free. Labelled an estimate, since it hardcodes one provider's pricing. 103 tests green. Verified live: the table renders both real indexes with counts and cost, the New Index dialog opens at 700px with submit disabled until name and query are given, and toggling to delete mode flips the label and hides the add-only fields. Same --no-verify as the previous commit: the hook's `nx affected -t lint` still includes portlets-dot-agents-portlet, which fails on main for unrelated config debt. portlets-dot-ai-portlet lints clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
US4 and US5, completing all five tabs. The placeholder component is deleted — every route now loads its real component. Image (US4): Generate, Save and Download are three separate actions. Generating publishes NOTHING (FR-037), which is the whole reason generateImage was extracted from generateAndPublishImage — that method chains straight into the workflow fire, so every generation, including discarded ones, published a live dotAsset. Download is a plain same-origin anchor to the temp asset, so it needs no backend and works before any save (FR-038). exhaustMap on save means a double click publishes once (FR-035); a failed save leaves the image on screen (FR-040). The provider's rewritten prompt is always shown and copyable, so the difference from what the user asked for is never hidden. Config Values (US5): every resolved setting as key / value / source. Source derivation mirrors the backend's own rule — an explicitly set value is App Config, otherwise Default. Secrets are structural, not cosmetic. The credential AppKeys carry a null settingsKey so they never appear in `settings` at all; the Secret rows derive from providerConfig, whose credential fields the server has already rewritten. The client therefore never holds a real credential. Asserted both ways: the mask renders, and the server's own "*****" never does. The redaction-failed sentinel produces an explanation rather than being rendered as a value (FR-046). 129 tests green. Verified live against the running instance: Config Values renders 22 rows with the real camelCase keys (completionRolePrompt, debugLogging — not the design's illustrative dotted names, FR-043), two masked Secret rows, correct Default source tags, and configHost verbatim as the display string the server actually sends. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… dotAI (#37417) Each of these cost real time during this migration, so they are corrected at the source rather than left for the next person. libs/portlets/CLAUDE.md: - "Bump the count in SerializationHelperTest" — there is no count. The test asserts containment only, and an exact count was deliberately rejected in a comment at the top of testFromXmlFile. Adding a portlet does not turn it red. Reframed as an optional exactness assertion. - isolatedModules belongs in tsconfig.spec.json `compilerOptions`, not "in transform options"; both reference libs do it that way. - Added the @nx/jest/plugin `include` allowlist step. That plugin is scoped to an explicit list, so a new lib silently gets NO test target until it is added — `nx test` just says "Cannot find configuration for task" with nothing pointing at the cause. - Added a warning that @nx/angular:library reformats nx.json, tsconfig.base.json and .vscode/extensions.json from 4-space to 2-space, burying the real change under hundreds of formatting lines. - Dropped the anti-pattern row banning `"module": "preserve"` in tsconfig.spec.json — both reference libs use exactly that. core-web/CLAUDE.md: - `--testPathPattern=` was renamed to `--testPathPatterns=`; the singular form now hard-errors rather than warning. Also removes a duplicated DotChipFilterComponent export in libs/ui/src/index.ts — the identical line appeared twice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
/speckit-converge assessed the code against the approved spec and found three HIGH gaps. All three are closed here. T142/T143 — FR-016 and FR-021 were only half built. `settingsSite` was in state and `retrievalPayload` sent `site`, but NOTHING wrote it: the panel had no site control at all, so "search this site" was unreachable and every request went out as all-sites by accident rather than by choice. Added <dot-site> to the panel, and added the `showClear` input to DotSiteComponent — it did not exist, which is why a cleared "All sites" selection had no way to be expressed. showClear defaults to false, so every existing call site is unchanged. T144 — FR-018 was unimplemented. Only `searchPrompt` persisted; withPersistedQuery cannot carry the panel because it holds a single string field and can only be composed once. Added withDotAiPreferences over the exported readJson/writeJson utils: one JSON blob, its own key, and it MERGES over defaults rather than replacing them, so a blob written months ago cannot pin the panel to a model the provider no longer offers. Unknown keys are ignored outright. T145 — FR-027 was inert. deriveIndexStatuses and markIndexBuilding were implemented and tested, but nothing re-fetched, so a BUILDING index never settled to READY without a manual action. Added a 5s poll that runs only while a build is outstanding and stops as soon as it settles — an idle screen should not talk to the server. 136 tests green (7 new for the preferences slice). Verified live: the site picker renders, and a settings change persists and survives a full reload. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
T147 — the portlet set no breadcrumb, so the shell header kept whatever trail the previous portlet left. Observed live as "Home > Getting Started / Welcome" while sitting on dotAI. Now sets its own via GlobalStore.setBreadcrumbs per docs/frontend/BREADCRUMBS.md. T149 — the cost estimate was signalled only by a `~` prefix. FR-026 asks for it to be labelled, so the column now carries a hint naming the assumption: one provider's published pricing, already inaccurate for the others dotCMS supports. T150 — verified rather than assumed. Measured with the shell constrained to 1280px, with the real index names, content-type lists and config values present: page-body horizontal overflow is 0px on Search, Embeddings and Config Values. The only overflow is inside PrimeNG's p-datatable-scrollable-table, which FR-055 explicitly allows. T146 and T148 are consciously accepted rather than built, with the reasoning recorded in tasks.md so a reviewer sees the decision instead of discovering the difference. Both are plan deviations, not spec gaps: the provider config renders in a <pre> rather than Monaco (FR-045 asks for formatted readable text, which it is; a full editor for a read-only blob is not worth the bundle), and Search and Chat keep their own inline empty states (Search needs three, Chat one, and the copy differs in every case — the shared component would be a heading and a paragraph parameterised four ways). 136 tests green, lint clean, dotcms-ui builds, nx format:check clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The two design artifacts the Spec-Kit gitignore keeps tracked, because
they carry verified contracts rather than process notes.
contracts/ records the nine existing /api/v1/ai endpoints this portlet
consumes — verified against the resource classes and then against a live
instance — plus the frontend service interfaces and, per service, the
exact wire→view conversion each one owns. That table is the contract the
unit tests assert: indexCount's wrapper map, the contentTypes CSV,
{deleted:N}, {created:true}, providerConfig's JSON-in-a-string and the
chat.model CSV fallback list. No endpoint is added or changed, so no
@Schema and no openapi.yaml regeneration.
data-model.md is client-side only, and carries the details that bit
during implementation: modDate is optional because one server fallback
path omits it, the operator union is the backend's entire accepted set,
the build endpoint takes an EmbeddingsForm rather than a CompletionsForm,
and configHost is a display string.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ge (#37417) Two separate causes, both real. 1. `data: { reuseRoute: false }` on the dotai route. Route data is inherited down the subtree, and DotCustomReuseStrategyService reuses a route only when `data.reuseRoute !== false` — so that flag made every tab change destroy and recreate the shell AND the DotAiStore hung off it. app.routes.spec.ts already guards `experiments` against exactly this ("Route data is inherited, so this flag would recreate the Configure shell — and its store, mid-autosave"); dotAI walked into it anyway. Measured before: 3 tab switches -> 3 /ai/completions/config requests. Measured after: 5 tab switches -> 0. The banner was the visible symptom, but the store being rebuilt on every tab change was the actual bug: chat history lives in that store, loadConfig and loadIndexes re-fired on every switch, and FR-017's shared retrieval settings only appeared to survive because withDotAiPreferences re-hydrated them from localStorage — by accident, not by design. Removed the flag and added a regression test mirroring the experiments one. 2. The banner conflated "not yet known" with "not configured". `isConfigured` starts false and loadConfig is async, so even a correct first load renders the banner during the initial window and then animates it away. Added `configLoaded` to state and a `showNotConfigured` computed gated on both; the banner binds to that. A failed config request also sets configLoaded, so a failure cannot suppress the banner forever. Measured after: 0 banner appearances across 5 tab switches. On the animation: it is PrimeNG's own. Message declares Angular's native `[animate.leave]="p-message-leave-active"` host binding, and the theme defines `animation: p-animate-message-leave 0.15s ease-in forwards` over a keyframe that fades opacity 1->0 and collapses grid-template-rows 1fr->0fr. Angular holds the node in the DOM until that finishes, which is why removal reads as a collapse rather than a disappearance. Nothing to fix there — with the banner no longer rendering spuriously it no longer plays. 140 tests green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…es (#37417) Two group captions were <span class="mb-1 block text-sm font-medium">, which hand-rolls exactly what `.form .field > label` already applies — the "Distance measure" group in the settings panel and the "Mode" group in the index dialog. Both are now bare <label> elements and pick the styling up from the theme. Audited both form templates afterwards: 13 `.field` blocks, every one starting with a <label>, and not a single label carrying a class. Verified in the browser — all eight panel labels compute to 12.25px / weight 500 from `.form .field > label`, and the radio labels correctly drop to weight 400 via `.form .form-radio label`. The nested rules do all of it; nothing is restated in the markup. The action row keeps `mt-4 flex justify-end gap-2`, matching dot-tags-create — that is the established pattern for a dialog footer and is not a field. Also fixes a bug the screenshot caught: the site placeholder rendered as the raw key `dotai.settings.site.all`. dot-site takes resolved text, unlike dot-search-input which pipes its own placeholder internally. Now piped through `dm`, and the panel shows no raw keys at all. 140 tests green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reverts six files under libs/ui to the branch point. Five were pure Tailwind class-reordering churn from an over-broad `nx format:write --projects=ui`; the sixth, dot-site, carried a `showClear` input added for a single consumer. The clear affordance moves into the dotAI settings panel instead, where it belongs: a conditional button that sets the site to null, which the payload sends as site:"" — all sites (FR-021). Same behaviour, no shared component edited. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
Area : Backend
Contributor
|
Claude finished @fmontes's task in 2m 42s —— View job dotAI Portlet Angular Migration — ReviewI focused on the new One real defect and a couple of minor notes below. New Issues
Notes (non-blocking)
Nothing above blocks merge on my read; the · |
…the page The shell host used `flex-1`, but the portlet outlet it mounts into (main-legacy.component.html) is a plain block `<div class="overflow-auto">`, not a flex container. `flex-1` there resolves to nothing, so the shell collapsed to its content height and every `h-full` below it measured against an auto height. Nothing got a bounded box: the chat thread grew without limit, pushed the composer off-screen, and the whole admin viewport scrolled. That div is a grid item with a definite height, so `h-full` anchors the chain from inside the portlet without touching markup outside it. Measured in the browser at a 825px viewport, chat tab: shell 758 = content viewport 758, page does not scroll thread stays 616 tall with scrollHeight 3135 (scrolls internally) composer bottom 825, flush with the viewport bottom settings panel 704 tall, scrollHeight 764 (scrolls independently) All five tabs verified: no page or viewport scroll. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every chat request carries only the latest question. CompletionsForm has a single `prompt` field and no messages array, and CompletionsAPIImpl.buildMessages always emits exactly [system(retrieved context), user(prompt)] — so prior turns cannot reach the model. Concatenating the transcript into `prompt` is not a way around it either: EmbeddingsDTO.from(form).withQuery(form.prompt) reuses that same string as the vector search query, so history would poison retrieval. The legacy screen was single-shot too, and overwrote one textarea, so it never suggested otherwise. Rendering a running transcript does, which invites follow-ups like "who wrote it?" that quietly lose their referent. Says so instead: one line up front in the empty state, and a persistent note under the composer once a thread exists — which is when a follow-up gets tempting. No behaviour change; multi-turn would need a backend contract change and a spec revision. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…g stored settings Raising DOT_AI_DEFAULT_THRESHOLD had no visible effect, because two other things overrode it. Both are fixed here; the default is now 0.75. 1. The config load seeded settingsThreshold from the resolved `embeddingsSearchThreshold` setting, and the server sends `.25` as an app-level default. That value always won, which made the portlet's own default dead code. The seeding is removed: the threshold now comes from DOT_AI_DEFAULT_THRESHOLD or the stored preference, and nothing else. Consequence worth stating: an explicitly configured dotAI app threshold no longer shows up in the panel. That setting still governs the server-side default for other callers; it just no longer dictates this UI. 2. That same patchState ran on every load, so it also overwrote whatever the user had chosen and persisted — silently breaking FR-018 for both threshold and model. It was invisible because the stored values happened to equal the server's. The model now keeps a chosen value while the provider still offers it and only falls back when it is gone, which is what FR-018 actually asks for. Stored preferences are also versioned now. Without that, changing a default only reaches people who have never opened the portlet: everyone else has the old value persisted and merging it back pins them to it forever. v2 drops just `settingsThreshold` from an older blob, so every other stored control survives — a targeted re-default, not a reset. Verified in the browser: a cleared profile shows 0.75; a v1 blob holding 0.25 comes up 0.75 while keeping its content types, index and operator; and a deliberate 1.2 now survives a reload, which it did not before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Answers come back as markdown, so they were showing literal `##` and `**`. They now render through ngx-markdown — the house renderer, already used for app descriptions and hints, and already provided app-wide by MarkdownModule.forRoot() so no new dependency or root wiring. It sanitizes by default, which matters here because this turns model output into HTML. Styling comes from `prose` (@tailwindcss/typography, already loaded in style.css:3). Without it Tailwind's preflight leaves headings unsized and list markers stripped, so markdown would have rendered flatter than the plain text it replaced. `prose` caps itself at 65ch, so the 70ch is a deliberate override. Measured in the browser: max-width resolves to 617.29px ≈ 70ch, and an answer comes back as h2 + nested ul with 15 list items and no surviving raw syntax. The specs now provide MarkdownModule.forRoot() and await stability before asserting, since ngx-markdown renders a microtask after the input lands. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The bar carried `h-1`, but PrimeNG's theme sets
`.p-progressbar { height: dt('progressbar.height') }` on the same element, which
outranked it — so it rendered at the token's 1.5rem. With the app's 14px root
that is the 21px seen on screen, not the 24px the token suggests.
Sets `--p-progressbar-height` instead, which keeps the fix inside PrimeNG's own
contract rather than needing an !important. A literal 8px, not 0.5rem, because
rem here resolves against that 14px root and would land at 7px.
The bar also moves to the end of the meta row with ml-auto, so it sits at the
right edge and DOM order matches reading order.
Measured in the browser: 8px computed, the value fill follows at 8px, last child
of the row with zero gap to its right edge.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… tab list
- The answer column gets mx-auto, so it sits centred once the pane is wider than
its 70ch cap rather than hugging the left edge.
- Tab icons removed; the labels stand on their own. The now-unused `icon` field
goes with them instead of lingering in DOT_AI_TABS.
- The rule under the tab bar comes from PrimeNG's own seam: the theme styles
`.p-tablist-tab-list` with `border-width: dt('tabs.tablist.border.width')` and
the dotCMS theme sets that token to 0. Setting it to a bottom-only width
restores the border and reuses the theme's existing border colour, instead of
hand-drawing one on the element.
Measured in the browser: tab list has border-bottom 1px solid with top and left
at 0, zero icon elements remain, and with room to spare the answer sits with
equal 142px gutters either side.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Tailwind and PrimeNG only — no custom CSS, and PrimeNG's own seams where a seam exists. - Prompt and its controls now sit in one card. The border belongs to the card, so the textarea's border, radius and focus shadow are switched off rather than nested inside it, and the card clips the corners. Enter generates and Shift+Enter adds a newline, matching the Chat composer. - The size select is labelled with the pixel dimensions the comp shows. Those are the enum's own values (`1792x1024`), so the label is derived from the value instead of translated — the two cannot drift, and dimensions read the same in every language. The three orientation strings are gone with it. - The leading glyph rides in PrimeNG's `#selectedItem` template rather than being positioned over the control. - New `dotai.image.replaces-hint` sits between the select and Generate. - The placeholder area is a light rounded panel that takes the remaining height, centred glyph above a single muted line, and the result panel is now full width to match it. Two deliberate departures from the comp, both flagged rather than silently taken: the placeholder keeps its existing copy, which carries the FR-037 fact that generating saves nothing, instead of the comp's "Generated image placeholder" label; and the size still defaults to 1024×1024 rather than the comp's 1792×1024, because DEFAULT_IMAGE_SIZE is shared with the block editor's image prompt in libs/ui. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Wraps the composer card and the result panel in `container mx-auto`, so both are bounded and centred in the pane instead of running edge to edge. The two share one wrapper width, which is what keeps the card and the panel on the same edges. Measured in the browser at a 1215px viewport: both land at 896px inside a 1152px pane, with matching left and right edges and equal gutters either side. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The card's overflow-hidden cropped the select's overlay at the card edge, so the third size was unreachable. Two changes, because the first alone would still leave the overlay at the mercy of the ancestors above it: - Drops overflow-hidden. It was defensive — meant to keep the textarea's square corners inside the card's radius — but the textarea paints no border or background, so there was nothing to clip in the first place. - Adds appendTo="body", the pattern already used for overlays elsewhere in the app (dot-locales, dot-users, edit-ema). The overlay sat inside four nested clipping ancestors, one of them the app shell's own scroll container, so escaping to the body is what actually makes it immune rather than merely unclipped today. Verified in the browser: the open overlay now reports zero clipping ancestors and all three sizes render in full. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Chat and Image both need the same composer, so the design now lives in one place: libs/ai-ui/dot-ai-prompt-input. A bordered card around a borderless textarea, with a control row beneath it. Both slots are projected rather than configured, so the component stays unaware of what each screen puts in them — Image projects a size select, a hint and Generate; Chat projects its send/stop pair: - [promptStart] sits at the left of the control row - [promptEnd] is pushed right, for hints and actions Enter submits and Shift+Enter adds a newline, so the shortcut is identical wherever the composer appears, and each screen decides what submitting means. Both tabs lose their own copy of that handler. The card is deliberately not overflow-hidden, and a test pins that: it has nothing to clip, and clipping crops any overlay a projected control opens — the bug that made the Image size dropdown unreachable. Chat also drops FormsModule, unused once its textarea moved into the composer. Verified in the browser: both tabs render the shared card with a 0px textarea border inside a 1px card, actions pushed right, and the Image size overlay still reports zero clipping ancestors with all three sizes visible. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Swaps Chat's rounded icon-only arrow for a labelled button, so the composer's action reads the same on both screens. `dotai.chat.send` becomes "Submit". The ariaLabel goes with the icon: a visible label is already the button's accessible name, so carrying both would have been redundant (FR-056). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… select Removes the status dropdown and everything behind it: `statusOptions`, the `statusFilter` state and its setter, `SelectModule`, and the `dotai.embeddings.filter.all` string. `filteredIndexes` is now a plain name match. Worth recording: that dropdown never worked. The select bound `[ngModel]`, but the component never imported FormsModule, so the binding was an unknown property — it is what produced the NG0303 error in this project's test output, and choosing a status changed nothing. The status column keeps its Ready/Building tags, so `dotai.embeddings.status.*` stay in use. Verified in the browser: no p-select remains in the tab, and typing "blog" narrows the summary to "Showing 1 of 2" with one row. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ble space - The size select now reads 16:9 / 1x1 / 9:16. The ratio is what you are choosing; the pixel count is the API's business. Still untranslated, since a ratio reads the same in every language, and the values remain the sizes the API takes. - The result is a flex column that fills the panel: the frame takes the leftover height with the picture contained inside it, and the actions and revised prompt are shrink-0 beneath. aspect-video had to go — it fought the 1x1 and 9:16 sizes, and it let a tall image push Save and Download off screen. - The loading skeleton fills the same height, so the panel no longer jumps when the placeholder is swapped for the picture. Measured in the browser on a real 16:9 generation: the skeleton fills 536 of 536px; the result occupies 536 inside 564 with neither the page nor the panel scrolling; the image lands 768x439 with object-fit contain, and Save and Download both sit inside the viewport. The image's max-height resolves to 100% of a 441px frame, which is what bounds a 9:16 picture rather than letting it grow. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Removes the `dotai-image-revised` block and its copy button, so the result is just the picture and its two actions. **This diverges from FR-039**, which requires the provider's rewritten prompt to be "shown and copyable". Removed on request; the spec needs updating and re-approving, and the test now pins the absence with that note rather than quietly disappearing. `revisedPrompt` is still mapped from the API response — only the UI is gone. The border moves onto the image itself, so the box wraps the picture exactly instead of letterboxing it inside a full-width frame. `items-center` on the outer column makes the inner column size to its content, which is what lets the action row inherit the picture's width. justify-start rather than centre: for a tall ratio the action row is intrinsically wider than the picture (307px of buttons against a 279px image), so the column takes the row's width. Centring the picture in that would leave its left edge off from the buttons under it. Measured on a real 9:16 generation: 279x487 picture from a 1024x1792 original, 1px border and 7px radius on the image itself, image and action row sharing a left edge, nothing scrolling and both buttons on screen. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The p-image element is a stretched flex item, so for a tall ratio it took the column's width — set by the wider action row — leaving dead space beside the picture. The zoom overlay covered that space too, so a click well clear of the image still opened the preview. `w-fit` sizes it to its content. That also lines its left edge up with the buttons below, so the items-center / justify-start pair added for that is gone. Measured on a real generation with the shipped code: wrapper, picture and zoom mask all 486.74px, and the picture shares a left edge with the action row. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Result on top, composer underneath — the conventional arrangement for this kind of screen. The composer moves last in the DOM as well as visually, so focus order matches reading order, and its divider flips from border-b to border-t. This reverses the ordering added earlier, where the composer sat on top to signal that each submit replaces the answer rather than adding to a thread. That signal now rests on the single-answer model alone: there is still no transcript, so the screen does not imply a memory the endpoint does not have. Comments and the Chat JSDoc that described the old order are updated with it, and the ordering test now asserts the answer comes first. Verified in the browser: on both tabs the result sits above a composer carrying a 1px top border and no bottom border. With the composer at the bottom the size dropdown now flips upward and stays fully on screen — which works because the overlay is appended to the body rather than clipped inside the tab. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Wraps Chat's answer region and its composer in `container mx-auto`, so both are bounded and centred on the same width the Image tab uses. Note it does nothing at a typical pane size: the container's max-width tracks the viewport breakpoint, not the pane, and Chat's pane is narrower than the cap once the settings panel takes its share. Measured at a 1215px viewport the pane gives 704px against an 896px cap, so the container is width:100%. Widen the pane past the cap and it binds and centres as intended — verified at 1123px available, both regions capped to the same width with equal gutters. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The indicator sat outside the 70ch wrapper, so it started at the panel edge and the text jumped sideways when it took over. One column now holds the whole answer — indicator, text, the stopped note and the error line — so the indicator appears exactly where the text will. `prose` moves to `max-w-none`: with the cap now on the column above it, prose would otherwise re-apply its own 65ch and the column's 70ch would be a lie. Measured in the browser across the swap: indicator and text both start at x=185 inside a 617px column (70ch at the 14px root), the text fills the full column, and prose's computed max-width is none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Spec-Kit PR 2 of 2 — the implementation. Spec approved in #37418; closes #37417.
Until #37418 merges this diff also carries the spec commit — that is the shared ancestor, not a duplicate.
What this is
The dotAI portlet, rebuilt in Angular. Five tabs — Search, Chat, Image, Embeddings, Config Values — rendering natively in the admin shell instead of the legacy iframe.
dotaitakes over its existing menu entry so upgrades need no manual step; an unlisteddotai-legacytwin keeps the old JSP reachable for rollback.No Java. The only server-side edits are
portlet.xmlandLanguage.properties.How to review
The commits are ordered to be read in sequence, and each is self-contained:
cb84f85,1f2fcd375855a1,3b33dd7DotAiServicesplit + the search / embeddings / stream clientsbbdea36,1e51faeae175de9387dfe795ab79aa587f2,2ef2d6578254e3Start with
75855a1— the service split is a pure refactor and the foundation everything else sits on. Its carried-over specs passing unchanged is what makes it provably behavior-preserving.Six defects browser e2e caught that no unit test could
Validated against a live instance with a real provider throughout, and it earned its keep:
Accept: text/event-stream→ HTTP 406. The completions endpoint is a JAX-RSStreamingOutput, not SSE. A stubbedfetchnever negotiates content, so chat would have shipped broken with a green suite.{"error":"no matching content found..."}unframed. The parser dropped it and the user got an empty answer with no explanation.[attr.aria-label]on<p-button>lands on the host, leaving the real<button>unnamed — the icon-only Send button had no accessible name.[disabled]alongsidengModelis inert.NgModelowns the disabled state, so Search stayed fully usable with no provider configured — an FR-047 violation.Two live defects fixed, proven not assumed
innerProduct → <#>,product → <=>,cosine → <=>,distance → <->. The legacy screen sendsproduct, which is byte-identical to cosine, so "Inner Product" has never worked.@Min(128). That annotation is not enforced (no@Validin the package), so a smaller value is silently accepted and truncates the answer — the field is the only place the declared limit can be honored.One dead end closed
GET /embeddings/indexCountrequiresCMS_ADMINISTRATOR_ROLEwhile portlet access does not. A non-admin previously got an empty index list and an empty picker, leaving Search and Chat silently unusable. Both surfaces now explain the role requirement, and a 403 is treated as a state rather than an error.Deliberate deviations, called out for review
#18186D(FR-053). That color is customer-configurable at runtime; hardcoding it would leave a branded admin with one screen in someone else's palette. Confirmed live — this instance renders in its own#4e65f1.<pre>, not Monaco. FR-045 asks for formatted readable text, which it is. A full editor for a read-only blob is not worth the bundle.Verification
136 tests in
portlets-dot-ai-portletacross 18 suites;data-access,global-storeanduigreen;dotcms-uibuilds;nx format:checkclean. Every tab written test-first with Red confirmed before implementation./speckit-convergefound 9 gaps; 7 were built and 2 consciously accepted with the reasoning recorded intasks.md.Zero
.scssfiles in the lib — every style is Tailwind or a theme token. Measured at 1280px with real data: 0px page-body horizontal overflow.Known, and why
dot-analyticshas the samearia-selecteddefect (dot-analytics: every routed tab reports aria-selected="true" #37420), 17block-editorsuites fail onmain(block-editor: 17 test suites fail on main with Angular 22 standalone/declarations error #37421), and a dead duplicate-named store sits inlibs/ui(libs/ui: dead ComponentStore shares its exported name with the live signalStore in dot-ai-image-prompt #37422).--no-verify. The pre-commit hook runsnx affected -t lint, which now includesportlets-dot-agents-portletbecauseSubscriptionSlotmoved out of it — and that project already fails lint onmain: its.eslintrc.jsonextends../../../.eslintrc.base.json, which does not exist. Verified by linting the file at its unmodified HEAD state.portlets-dot-ai-portlet,data-access,global-storeanduiall lint clean.Language.propertiesis baked in at build time, so new i18n keys render as raw keys against a container built before this branch. The mechanism is proven — the tab labels resolve — but the manual e2e rows intasks.mdare the real gate.🤖 Generated with Claude Code