Conversation
Consolidate AI configuration into a single master-detail view with flat category navigation (General, Providers & Models, Model Aliases, Agents, Prompts & Skills, Variables, Tools, Token Usage, MCP Servers), retiring the legacy widgets and the scattered AI entries in the Settings UI. - Add the AiConfigurationService seam and the AiConfigurationCategory contribution point with an ordered registry (ai-core, ai-core-ui), so the view reads/writes AI preferences workspace-trust-aware and stays insulated from how they are stored. - Provide shared, DI-free page primitives: sections, list rows, settings rows and controls (toggle/select/number/array), a gear context menu (Copy Setting ID / Reset Setting), status badges, and a filterable collapsible list. - Render agents (prompt variant sets, language models, notifications, capabilities), providers & models, model aliases, prompts, skills & slash commands, variables, tools and MCP servers as consistent list/detail pages; complex preferences defer to settings.json. - Hide the AI preferences from the Settings UI and redirect ai-features.* deep-links to the view; show the category > item path as real Theia breadcrumbs. Refs eclipsesource#310, eclipsesource#311, eclipsesource#313, eclipsesource#314, eclipsesource#316. Co-authored-by: Alexandra Muntean <amuntean@eclipsesource.com> Resolves GH-17649
EclipseSourceAI
left a comment
There was a problem hiding this comment.
Note
Autonomous AI review.
This review was done by an AI agent and therefore may contain mistakes. Feel free to ignore any comment you disagree with. Noting why helps, since replies are read and taken into account in follow-up reviews.
Resolving all AI comments does not lead to an automatic approval. A maintainer still needs to review and sign off on the overall architecture and design.
To get an updated review after pushing changes, a maintainer may re-request a review from this account.
Running in Eclipse Enclave, submitted via review-guard-mcp
Consolidates the AI configuration tabs into one master/detail view behind a new public AiConfigurationCategory contribution point in @theia/ai-core-ui, hides ai-features.* from the Settings UI, and reroutes every AI-settings entry point here. Large but well-structured: lint is clean, all 1046 tests in the touched packages pass, and driving all 50 tree nodes in the browser produced no blank pages and no console errors, with edits persisting across reload.
Suggested focus for a maintainer:
ai-configuration-category.tsis the new public API.highlight.subId,ctx.filterandctx.scopeare inert today, which is harder to walk back after adopters see them.HideAiPreferencesContributioncarries the whole "AI prefs are no longer in Settings" cutover on a one-shotsetTimeout(0)pass.initializeLayoutnow auto-opens the view on every fresh layout, so all adopters' first start lands on it even with AI disabled. That looks like a product decision worth confirming.- The
getOwnedPreferenceIds()ownership contract is what keeps General's catch-all from double-rendering, and three categories declare it with string literals instead of the existing constants.
Two confirmed defects on the General page: the hero row lacks the data-ai-config-row-id anchor its own search item deep-links to, and ai-settings-row-id has no CSS rule, so the raw preference id renders next to the title at heading weight. Search-result scroll/flash is also intermittent. Details inline.
EclipseSourceAI
left a comment
There was a problem hiding this comment.
Note
Autonomous AI review.
This review was done by an AI agent and therefore may contain mistakes. Feel free to ignore any comment you disagree with. A thumbs-down reaction on a comment marks it as rejected for follow-up reviews. Noting why in a reply helps, since replies are read too.
Resolving all AI comments does not lead to an automatic approval. A maintainer still needs to review and sign off on the overall architecture and design.
To get an updated review after pushing changes, a maintainer may re-request a review from this account.
Running in Eclipse Enclave, submitted via review-guard-mcp
Went through d5faabaef (the "address review feedback" commit) against every thread from my previous review. All nine fixes described in that commit's message check out in the actual code: the hero row anchor, disabled catch-all rows, the deterministic scroll-and-flash retry with the new centerInBody (avoids the scrollIntoView ancestor-scroll bug), dropping the auto-open-on-fresh-layout initializeLayout, the onDidChangeSchema subscription in HideAiPreferencesContribution (with good new tests), routing the token-usage deep link through AI_SHOW_SETTINGS_COMMAND with argument forwarding on both the ai-core and ai-ide sides, using the existing preference-id constants, dropping the dead ctx.filter/highlight.subId/search-dropdown helpers, and reworking ModelsConfigurationCategory onto CollectionCategoryRenderer. Nice, focused fix commit with tests added where it mattered.
One loose end: a reply on the ai-core-ui/package.json thread claimed a stale JSDoc comment (about which context menu the gear button opens) was fixed as a side effect, but the comment is unchanged, flagged inline.
I wasn't able to get a full local build going in this environment (native module/postinstall issues unrelated to the PR), so this pass is code-review only, no live UI check this time.
These previous comments can be resolved as they are now handled:
- Enable AI features row never got its search deep-link anchor
- raw preference id rendered unstyled next to the hero title
- catch-all rows stayed editable while AI features were off
- tool preference ids hardcoded instead of using existing constants
- duplicate nls key registered with different English text
- AGENT_SETTINGS_PREF/LANGUAGE_MODEL_ALIASES_PREFERENCE not reused for the preference id
- agent/notification descriptions duplicating nls keys with different text
- chat-input-widget hardcoded the ai-ide command id instead of the generic one
- hide-ai-preferences only covered schemas registered in the initial sync pass
- pendingHighlight consumed before the target row was mounted
- view opened as an active tab on every fresh layout
- highlight.subId/ctx.filter dead contribution-point API
- unused search match()/MAX_RESULTS helpers
- unused limitedLabel/limitedDefault props on AiSessionLimitControl
- ModelsConfigurationCategory reimplementing CollectionCategoryRenderer
I can't resolve them myself as I would need write permission on this repository.
- Anchor the General hero row with `data-ai-config-row-id` so the "Enable AI features" search result scrolls to and flashes it, and drop the unstyled raw preference id from its title. - Disable the General catch-all rows while AI features are off, so they match the banner instead of staying editable (`AiSettingsRow` gains a `disabled` prop). - Make search scroll-and-flash deterministic by keeping the highlight pending until the target row is mounted, and center it by scrolling the detail body only: `scrollIntoView` also scrolls the `overflow: hidden` ancestors, which shifted the whole shell and pushed the menu bar out of view. - Stop opening the view automatically on a fresh layout. It stays reachable via the Manage menu and `alt+a`, both ungated. - Keep `ai-features.*` hidden when schemas are registered after startup, by subscribing to `onDidChangeSchema` instead of a one-shot `setTimeout`. - Route the token usage warning through `AI_SHOW_SETTINGS_COMMAND` and forward the preference id, so the deep link also works without `@theia/ai-ide`. - Use the existing preference id constants in `getOwnedPreferenceIds`, and stop registering existing nls keys with different English text. - Drop the inert `ctx.filter`, `CollectionCategoryRenderer.renderFilter` and `highlight.subId` rather than ship contribution-point API nothing can use. - Render Providers & Models through `CollectionCategoryRenderer` like the other collection categories. - Remove the unused search and control helpers, and document the removed exports and the new `@theia/preferences` edge in `Migration.md`.
The existing specs cover the categories' logic but stop at the boundary to the DOM, which is where the defects found in review actually lived. Add tests for the contracts that span that boundary. - Assert every `rowId` the General search index deep-links to is rendered with a matching `data-ai-config-row-id`. The two halves are produced independently, so an indexed-but-unanchored row (as the hero toggle was) silently never scrolls or flashes, and the existing search test cannot see it. - Assert the General catch-all rows are read-only while AI features are off and editable once on, matching the gate note. - Assert the hero does not print its raw preference id. - Assert the categories claim only preference ids a schema declares, that no two claim the same id, and that General claims every row it renders. A typo or rename otherwise leaves the id unclaimed and duplicated by the catch-all. - Pin `centerInBody`'s arithmetic and that it scrolls the detail body only, so a regression to `scrollIntoView` (which also scrolls `overflow: hidden` ancestors and shifts the shell) fails. Each new test was checked to fail against the unfixed behaviour. The render tests re-`set` the `FrontendApplicationConfigProvider` inside `before`, since the config lives on `window` and `enableJSDOM` replaces it.
EclipseSourceAI
left a comment
There was a problem hiding this comment.
Note
Autonomous AI review.
This review was done by an AI agent and therefore may contain mistakes. Feel free to ignore any comment you disagree with. A thumbs-down reaction on a comment marks it as rejected for follow-up reviews. Noting why in a reply helps, since replies are read too.
Resolving all AI comments does not lead to an automatic approval. A maintainer still needs to review and sign off on the overall architecture and design.
To get an updated review after pushing changes, a maintainer may re-request a review from this account.
Running in Eclipse Enclave, submitted via review-guard-mcp
Only one commit landed since my last pass: 277eaa49e ("test(ai): cover the AI configuration contracts the view relies on"). It adds regression tests for exactly the DOM-boundary defects from the earlier review: the search-index-to-data-ai-config-row-id anchor contract, the hero not leaking its raw preference id, catch-all rows being disabled while AI is off, preference-ownership uniqueness/coverage across categories, and centerInBody's scroll arithmetic (including that it never touches an ancestor's scroll position). I checked the math in the centerInBody tests by hand against the implementation and it's correct, and the stub shapes used (AiSettingsRowService, getSectionRefs, getOwnedPreferenceIds) match the real APIs. No issues found in this commit.
I also re-checked the one loose end from my last review: the stale JSDoc on AiSettingRowGear claiming it opens PreferenceMenus.PREFERENCE_EDITOR_CONTEXT_MENU now correctly references AI_CONFIGURATION_SETTING_CONTEXT_MENU. That thread was already marked resolved by the maintainer, so nothing further needed there.
All previously open threads are resolved, and I found nothing new to flag in this round. No inline comments this time.
What it does
Consolidates AI configuration into a single master–detail view with flat category
navigation (General, Providers & Models, Model Aliases, Agents, Prompts & Skills,
Variables, Tools, Token Usage, MCP Servers), retiring the legacy per-tab widgets and the
scattered AI entries in the Settings UI.
AiConfigurationServiceseam and the publicAiConfigurationCategorycontribution point with an ordered, lazily-wired registry (
ai-core,ai-core-ui), sothe view reads/writes AI preferences workspace-trust-aware and stays insulated from how
they are stored.
(toggle/select/number/array), a gear context menu (Copy Setting ID / Reset Setting),
status badges, and a filterable collapsible list.
providers & models, model aliases, prompts, skills & slash commands, variables, tools and
MCP servers as consistent list/detail pages; complex preferences defer to
settings.json.ai-features.*deep-links to theview; shows the category → item path as real Theia breadcrumbs.
aiModelProviderschemamarker (no denylist), so unmarked feature areas fall through to General's catch-all.
Refs eclipsesource#310, eclipsesource#311, eclipsesource#313,
eclipsesource#314, eclipsesource#316.
Resolves GH-17649.
flowchart LR Cat["Config areas, each a category (built-in + contributed by extensions)"] View["AI Configuration view tree on the left, detail page on the right"] Prim["Shared UI building blocks (rows, controls, sections)"] Svc["AiConfigurationService single read/write point"] Store["Settings storage (preferences + per-agent settings)"] Cat -->|registered and listed in| View View -->|detail pages built from| Prim Prim -->|read and write through| Svc Svc --> StoreHow to test
Follow-ups
Breaking changes
Adopter-facing notes are in
doc/Migration.md(AI Configuration View): the per-tabconfig widgets and their
WidgetFactoryregistrations are removed,@theia/ai-ideand@theia/ai-mcpnow depend on@theia/ai-core-ui, and custom AI-config tabs must bere-implemented as
AiConfigurationCategorycontributions. Stable entry points(
aiConfiguration:open,aiConfiguration:openTools, chat toolbar button) are unchanged.Attribution
Review checklist
nlsservice (for details, please see the Internationalization/Localization section in the Coding Guidelines)Reminder for reviewers