Skip to content

feat(ui): move AI-assistant shell + shared routes into OSS app-mode framework - #31911

Merged
karanh37 merged 81 commits into
mainfrom
feature/ai-mode-oss-migration
Aug 29, 2026
Merged

karanh37 merged 81 commits into
mainfrom
feature/ai-mode-oss-migration

Conversation

@karanh37

@karanh37 karanh37 commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Fixes https://github.com/open-metadata/openmetadata-collate/issues/6028

Moves the AI-assistant layout and shared-surface routes from the Collate ai-chat plugin into OpenMetadata OSS, with route/presentation selection driven by app mode. The goal is that only genuinely AI-specific surfaces remain downstream; everything with a classic equivalent (Explore, Observability, Context Center, Marketplace, …) lives in OSS and renders its AI presentation when useIsAiMode().

Stacks on top of #31906 (AppMode switcher) — the first commit here is that switcher, cherry-picked so this branch builds standalone; it will collapse once #31906 merges.

What's in this branch so far

  • App-mode shell + nav-module framework (components/platform/ai-shell/): a mode-agnostic assistant shell (AssistantLayout, Sidebar, KeepAliveRoutes, AppModeRoutes) and an AppModule contract. Downstream plugins contribute their AI-exclusive nav + routes through new app-mode.* extension points on ExtensionPointRegistry — OSS imports no plugin code. Neutral naming (no AI prefixes); sharedAppModules is populated as domains migrate.
  • Explore unified into a single mode-aware page: ExplorePageV1 renders the AI search header (moved to components/discovery/explore/ExploreHeader/) when in AI mode, and the classic page otherwise. Same /explore route in both modes — no AIExplorePage.
  • Domain wave 1 moved into OSS as AppModules: observability, context-center, marketplace, plus the shared helpers/layouts they need (LiveRefreshBoundary, PermissionedLiveRoute, ListPageHeader).

Still to come (tracked, not in this draft yet)

  • Remaining domains: entity, inbox/my-data, connections, chat, AI governance, and the Collate-only pipeline/alert observability pages.
  • Downstream cutover (register AppModeRoutes, contribute the AI-exclusive modules + fallback) lands in the Collate repo.
  • i18n: non-English locale strings for the new keys are English placeholders pending native review.

Verification

  • yarn build (standalone OSS UI) passes.
  • tsc clean for the new/changed files (the repo's pre-existing tsc backlog and the linked-@types/react dual-type noise are unaffected).
  • Jest green for the moved Explore header (37) and observability suites.

🤖 Generated with Claude Code

chirag-madlani and others added 5 commits August 22, 2026 12:18
Popover switcher for Classic/AI app mode with 'Open in {mode} when I
log in' checkbox. Routes are prop-configurable (default '/'); no
Electron/desktop gating (that lives in downstream shells).
…ery domain

The AI Explore presentation (search header + quick filters) moves from the
Collate ai-chat plugin into openmetadata-ui under components/discovery/explore.
ExplorePageV1's default export now selects its presentation from the active app
mode: classic renders the shared page unchanged, AI mode wraps it with the AI
search header. Both modes mount the same /explore route.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Renames ExploreAiHeader/AIExplore* to ExploreHeader/Explore* and neutralizes
testids, class-name constants, i18n keys, and the header asset. Adds the
renamed explore-* i18n keys and syncs locales. Presentation is still selected
by app mode inside ExplorePageV1; the naming no longer carries an AI prefix.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rm/ai-shell)

Adds the mode-aware assistant shell (AssistantLayout, Sidebar tree, KeepAliveRoutes,
AppModeRoutes) and the AppModule/SubNav contract under components/platform/ai-shell,
plus app-mode.* extension points so downstream plugins contribute nav+routes without
OSS importing them. Neutral naming (no AI prefixes); sharedAppModules starts empty and
is populated as domains migrate. Additive — nothing consumes it yet.

Also: add optional isNLPActive to UserPreferences and fix Explore wrapper pageTitle
forwarding surfaced by the moved Explore header.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…p-mode framework

Wave 1 of the domain migration: ports the shared AI-mode helpers (LiveRefreshBoundary,
PermissionedLiveRoute, ListPageHeader) and section layouts, and moves the observability,
context-center, and marketplace surfaces into OSS as AppModule descriptors registered in
sharedAppModules. Presentations live under their handbook domains with neutral naming;
routes are mode-selected by the shell. Pipeline/Alert observability pages remain in Collate
for a later wave (served via the app-mode fallback). Syncs new i18n keys.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added safe to test Add this label to run secure Github workflows on PRs UI UI specific issues labels Aug 22, 2026
…-mode framework

Wave 2: entity-detail and personal-space surfaces become OSS AppModules in
sharedAppModules (both icon-less; reached via deep links / a header entry point).
The entity wrapper resolves the OSS entity-detail component; the inbox/my-data
shell exposes triageContent/myDataContent injection slots (the proprietary feed
bodies stay in Collate and render via those slots, placeholder until wired).
Adds inbox i18n keys. Governance is intentionally NOT moved — it stays Collate.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
karanh37 and others added 9 commits August 22, 2026 22:52
…1Mode

First step of turning the migrated AI-mode shell into a generic ClassicV1
layout: renames the AI_APP_MODE constant to CLASSIC_V1_APP_MODE ('classicV1')
and useIsAiMode to useIsClassicV1Mode across every caller in the OSS UI, so
downstream tasks can build the generic ClassicV1 layout on top of it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…itch)

AppRouter now switches on the stored app mode directly (no route
registry/install-gate consultation), leaving useResolvedAppMode with
zero callers in OSS. Delete it and its test rather than gutting it.

useAppRoutesRegistry itself is NOT deleted: CustomizeUI, AppModeSwitcher,
SettingsAppModePage, and PersonaDetailsPage still read registry.routes to
gate "is a non-default mode installed" UI (unrelated to AppRouter's
routing decision). See task-3-report.md for the full BLOCKED analysis.

Also fixes the now-stale AppModeRoutes doc comment (no more
plugin-registration mechanism; OSS AppRouter renders the shell directly
by mode) and a dangling comment reference to the deleted hook.
ClassicV1 now ships in-tree in OSS (AppRouter switches on the stored mode
directly, no install-gate) so the registry's "is a non-default mode
installed" read is always true. Migrate the 4 real consumers
(AppModeSwitcher, SettingsAppModePage, CustomizeUI, PersonaDetailsPage) to
an unconditional `true` and remove the now-dead import, then delete the
registry and its test. Update the 2 consumer tests that asserted gated
behavior to assert the mode is always available instead. Also fixes the 2
remaining comment-only references in appMode.constants.ts so the module is
what AppRouter actually does now: match the resolved mode string against
CLASSIC_V1_APP_MODE directly, no registry involved.
…-token remember

Rebrands AppModeSwitcher's AI toggle into the ClassicV1 app-mode switch:
renames identifiers/testids/labels (isAiMode->isClassicV1Mode via the
already-committed useIsClassicV1Mode hook, handleAiClick->handleClassicV1Click,
app-mode-option-ai->app-mode-option-classic-v1, label.ai->label.classic-v1),
renames app-mode-ai.svg->app-mode-classic-v1.svg, and removes the dead
isAiInstalled disabled-gate now that ClassicV1 ships in-tree with no
install-gate (Task 3 already hardcoded it to true).

Fixes the pre-existing #31906 wire-contract bug: handleRememberToggle was
writing the runtime mode string ("default"/"classicV1") straight to the
AppModePreference.config.value preference, whose wire enum is actually
["ai","classic","classicV1",null]. Adds a local runtime->wire map
('default'->'classic', 'classicV1'->'classicV1' identity) and routes both
the remember-checkbox write and its "is remembered" read through it.

Also adds the genuinely-missing label.classic key to en-us.json (the
component already called t('label.classic') with no key defined) and
label.classic-v1, translated per-locale (classic-v1 stays untranslated
like label.ai, since both are mode/product names).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…n read

Closes the symmetric read-side gap left by the AppModeSwitcher wire-token
fix: resolveInitialAppMode and AuthProvider.hydrateAndResolveAppMode both
read preferences.appMode and used it directly as a runtime mode string, but
it now holds the AppModePreference wire token ("classic"/"classicV1", plus
legacy "ai") written by the switcher's remember checkbox. A remembered
Classic user stored "classic" and would have read back the literal string
"classic" as the runtime mode instead of DEFAULT_APP_MODE.

Adds RUNTIME_TO_PREFERENCE_WIRE (moved from AppModeSwitcher.tsx, now the
single source of truth) and its inverse PREFERENCE_MODE_TO_RUNTIME plus a
translatePreferenceMode wrapper in useAppMode.ts, co-located with the
analogous CONFIG_MODE_TO_RUNTIME/translateWireMode pair for the tenant-wide
appConfiguration wire vocabulary. Routes both resolveInitialAppMode and
AuthProvider's boot-time preference read through the translation.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… boot

Task 3 deleted useResolvedAppMode, which also removed the only code that
fetched the persona doc and resolved the persona-forced app mode. The boot
path in AuthProvider.hydrateAndResolveAppMode was left calling
resolveEffectiveAppMode(userPref, null, appDefault) with persona hard-coded
null, so a persona with appMode set no longer forced the mode.

Fold persona resolution into the boot path: fetch the persona's
UICustomization doc (personaDocFqn + getDocumentByFQN) when the precedence
chain actually needs to run, translate its appMode via a new pure
resolvePersonaAppMode helper (reusing translatePreferenceMode:
classic->default, classicV1/legacy AI/ai->classicV1), and pass the resolved
mode into resolveEffectiveAppMode. No route-registry install-gate is
reintroduced. Update the stale doc comments that referenced the deleted
async resolver.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread openmetadata-ui/src/main/resources/ui/src/hooks/useAppMode.ts
karanh37 and others added 9 commits August 23, 2026 20:17
Mount the existing AppModeSwitcher in the user profile dropdown
(UserProfileIcon) so users can switch between Classic and ClassicV1
interfaces from the navbar avatar menu, alongside the persona switcher.

Add the three i18n keys the switcher needs to the OSS catalog
(mode-label, switch-interface, open-in-mode-when-login), sync across
all locales, and copy he-he/zh-cn translations from the Collate catalog.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…allback, sidebar styling

Make the ClassicV1 app-mode shell render standalone in OSS (no plugin
required), so it works for every OpenMetadata user, not only when Collate
contributes its surfaces.

- AppModeRoutes: when no plugin contributes a routes fallback, render the
  shared page route table via `applicationRoutesClass.getRouteElements()`
  (OSS `AuthenticatedAppRouter`) as the catch-all — mirroring how the classic
  `AppContainer` renders its content, just wrapped in the ClassicV1 `AppShell`
  chrome instead of the classic sidebar. Every canonical page (Explore,
  Glossary, Settings, entity details) now renders inside the shell, and
  `/` -> `/my-data` comes from that table's own redirect. Replaces the earlier
  interim `/my-data` redirect fallback.
- Add Home, Explore and Govern sidebar modules to `sharedAppModules`. Home and
  Explore are nav-only (content served by the fallback table); Govern carries
  the classic governance sub-nav (Glossary, Ontology Explorer, Classification,
  Metrics, Workflows).
- Sidebar: port the full cosmetic sidebar styling (rail/panel widths, colors,
  spacing, hover/active) that the initial platform/ai-shell port left as a
  structural-only stub — the neutral shell rendered unstyled with no consumer.
- vite: force-prebundle `react-hook-form` in `optimizeDeps.include`. Through a
  symlinked node_modules Vite otherwise serves it raw via `@fs`, pulling a
  second React copy and throwing "Invalid hook call" in every RHF form;
  `dedupe` alone does not cover the dev pre-bundle path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The ClassicV1 sidebar header/footer rendered only plugin-contributed slots, so
standalone OSS had no logo and no user card. Add OSS-native defaults, shown by
MainPanel and Rail whenever no plugin contributes the matching slot (so a
downstream Collate brand/user card still overrides them):

- SidebarBrand (`ask-logo-btn`) — the OpenMetadata monogram via
  `brandClassBase.getMonogram()` (honours white-labeling), navigating home.
- UserProfileCard (`ask-user-card`) — reuses the shared `UserProfileIcon`
  (avatar, name, and the profile dropdown that carries the mode switcher).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… content panel

Address review feedback on the ClassicV1 shell:

- Home and Govern nav icons were off — use the UntitledUI `Home02` and
  `Scales01` line icons (Home02 matches the classic sidebar), replacing the
  mismatched raster/coloured SVGs.
- Collapsed rail logo looked wrong: SidebarBrand hard-coded the MainPanel logo
  class (which sizes the SVG for a wide logo). Add a `variant` so the rail uses
  `ask-rail__logo-btn` and centers the square monogram.
- User card now bottom-aligned (`ask-main-panel__footer { margin-top: auto }`)
  and carries the Classic/ClassicV1 AppModeSwitcher, mirroring Collate. The
  collapsed rail shows a compact avatar-only card (the switcher label can't fit
  the 65px rail).
- AssistantLayout content panel had only a structural stub — give
  `.assistant-content` the white surface, flex column and relative positioning,
  and the layout a page background so the content reads as a framed panel.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…p AppPlugin.getModeModules

ClassicV1 is an app layout, not a plugin layout, so its sidebar modules should
be owned by the app the same way the Classic sidebar is — not contributed by an
installed plugin.

- LeftSidebarClassBase gains `getAppModeModules()` / `setAppModeModules()`,
  the sibling of `getSidebarItems()` for the ClassicV1 layout. It holds the OSS
  app-mode modules (home, explore, entity, observability, govern, context
  center, marketplace, personal space). A downstream build overrides it to
  append its own, exactly like it extends the Classic sidebar.
- `useAllAppModules()` now reads `leftSidebarClassBase.getAppModeModules()` and
  sorts by navOrder — no `ApplicationsProvider`/plugin merge.
- Remove `AppPlugin.getModeModules` and its shell merge; update the docs on
  `ExtensionPointTypes` and `AppModule` and the `sharedAppModules` test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Use the Collate nav-bar Home and Governance icons (default + active) for the
  ClassicV1 Home and Govern sidebar modules, replacing the UntitledUI stand-ins
  so they match the rest of the AI-shell icon set.
- AssistantLayout: give default-variant PageLayoutV1 pages a 16px top inset
  (`.page-layout-v1:not([data-variant='compact'])`) — ClassicV1 has no top
  navbar, so pages otherwise render flush to the panel. Compact pages keep
  their own p-2; PageLayoutV1 owns all other page padding.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nbox01

The ClassicV1 Inbox page header showed the generic @untitledui `Inbox01`,
inconsistent with the sidebar/nav icon set (Marketplace, Observability, etc.
already use their own svg). Use the `ask-collate-nav-bar/inbox-default.svg`
icon so the header matches the rest of the shell.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… into OSS

The ClassicV1 user card was OSS-only chrome around the shared `UserProfileIcon`;
the real AI user menu and the sidebar Inbox launcher lived in Collate. Move them
(and the personal-space store + inbox helpers they need) into OSS so the OSS
ClassicV1 shell has a first-class user card, and Collate reuses it.

- New OSS: `components/discovery/personal-space/AIUserMenu/` (avatar, name,
  profile dropdown; uses OSS `NavbarUtilClassBase.getHelpItems()` — Collate's
  override is picked up automatically by the class-replacement plugin),
  `components/discovery/personal-space/InboxIconButton/` (inbox launcher with
  open-task + unread-activity badge), `hooks/usePersonalSpaceStore.ts`,
  `hooks/useUnreadInboxActivity.ts`, `InboxPage/inbox.utils.ts`, and
  `personal-space/inbox.constants.ts` (the two inbox query keys).
- `UserProfileCard` now composes `AIUserMenu` + `InboxIconButton` +
  `AppModeSwitcher` (expanded) / a compact rail variant, replacing the
  `UserProfileIcon` placeholder.
- All imports repointed to OSS relative paths; 52 moved unit tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ne React

`@openmetadata/ui-core-components` is a yarn `link:` dependency, and worktree
dev setups (e.g. Conductor) symlink `node_modules` itself. With Vite's default
symlink-following, such deps resolve to a real path OUTSIDE the project root and
get served raw via `@fs`, pulling a SECOND React instance — every react-hook-form
`useForm` page then throws "Invalid hook call" / "Cannot read properties of null
(reading 'useRef')" on mount. Preserving symlinks keeps deps on their
node_modules-relative path so their `import 'react'` resolves to the single
optimized React. Benign for plain installs (no symlinks to follow).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Dashboard→Overview rename renamed the route constant to
CONTEXT_CENTER_OVERVIEW and updated the router and sidebar, but the
context-center module still referenced the removed
ROUTES.CONTEXT_CENTER_DASHBOARD and the label.dashboard key. Switch the
navigate target, route path, sub-nav path and label to OVERVIEW /
label.overview so the module matches.

Ports open-metadata/openmetadata-collate#6199 onto the migrated OSS module.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

…ation

App-mode's KeepAliveRoutes keeps every visited static route permanently
mounted in a hidden container to preserve scroll/filter/in-flight state.
A module index route whose element is `<Navigate>` (e.g. `/observability`
→ `/observability/data-quality`) was eligible for this cache, and
react-router runs `Navigate`'s redirect in a dependency-less `useEffect`
that re-fires on every render. Once the bare prefix had been visited, its
hidden `<Navigate>` re-asserted the redirect on each render and yanked the
URL back to the module default the instant the user clicked a sibling
sub-route (Alerts, Incidents, Pipeline, Context Center submenus) — the tab
would flash then snap back.

Exclude redirect routes (element type === Navigate) from the keep-alive
cache so they fall through to the fallback <Routes>, which mounts only the
currently-matched route; the redirect then fires exactly once at the bare
prefix and never again. This generalises the previously-empty
NON_CACHEABLE_ROUTE_PATHS guard to every module index redirect.

Adds KeepAliveRoutes unit tests covering the redirect-exclusion behaviour.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

karanh37 and others added 2 commits August 28, 2026 18:41
…igration

# Conflicts:
#	openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/AuthProvider.tsx
The stricter OSS eslint config surfaced 19 errors in Collate-ported
app-mode files after merging main. Fix them:

- no-non-null-assertion (AIUserMenu): make the menu-row icon optional and
  guard-render it; replace `children!` with `children ?? []`.
- no-array-index-key: derive stable keys from item data where a real list
  (permission sections/rules keyed by their own key/name/type); scope a
  narrow eslint-disable with a reason only for fixed-length loading
  skeletons, index-addressed form-array rows with no stable id for unsaved
  entries, the pathless layout route in KeepAliveRoutes, and generic test
  mocks.
- no-danger: scope a disable on the markdown-preview test mock fixture.

No behavioural change; warnings left untouched.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

`/conversations` is the New-Chat landing. Kept alive, its hidden copy
retained the previous in-progress chat, so navigating back to it ("New
Chat") showed a stale conversation instead of a fresh empty session. Add
it to NON_CACHEABLE_ROUTE_PATHS so it remounts on every visit.
(`/conversations/:id` was already non-cacheable via its dynamic segment.)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

…te churn

Every observability route wraps its own ObservabilityLayout, and the
intent bus holds a single listener per name. Visiting a non-cacheable
sub-route (e.g. a `/observability/data-quality/:tab` tab) mounts a second
layout that clobbers the kept-alive base layout's listener and, on its
unmount, deletes the shared slot — leaving the still-mounted base layout
silently unsubscribed, so the Add Test Case / Add Bundle Suite CTAs went
dead after visiting a DQ tab and returning.

Give useIntent an optional re-register key and have ObservabilityLayout
bump it from a useRouteActivation callback, so the layout re-claims its
intent listeners whenever its route becomes visible again.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

karanh37 and others added 2 commits August 29, 2026 12:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

⚠️ UI Checkstyle passed — lint findings in changed files

🔍 ESLint findings in this PR's files — 0 error(s), 233 warning(s)

Errors block the build. Warnings do not yet — they are rules whose backlog is still
being worked down, listed so this PR does not add to it. See docs/ui-code-quality-gate.md.

0 error(s), 233 warning(s) across 84 changed file(s).

Count Rule
59 sonarjs/no-duplicate-string
36 openmetadata-imports/no-lower-layer-page-imports
34 sonarjs/cyclomatic-complexity
31 react-hooks/exhaustive-deps
29 sonarjs/no-nested-conditional
9 sonarjs/no-nested-functions
9 sonarjs/expression-complexity
8 openmetadata-imports/no-cross-page-imports
4 openmetadata-imports/no-api-calls-in-iteration
3 openmetadata-imports/no-circular-imports
All findings
Location Rule Message
🟡 src/components/AppModeSwitcher/AppModeSwitcher.tsx:48:73 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 12 which is greater than 10 authorized.","cost":2,"secondaryLocations":[{"line":48,"column":72,"endLine":48,"endColumn"
🟡 src/components/AppRouter/SettingsRouter.tsx:413:28 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 4 times.
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:80:1 openmetadata-imports/no-internal-barrel-imports Import the internal module directly instead of its index barrel so unrelated siblings do not enter the bundle graph.
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:309:9 react-hooks/exhaustive-deps The 'onLoginHandler' function makes the dependencies of useMemo Hook (at line 964) change on every render. Move it inside the useMemo callback. Alternatively, w
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:394:6 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'navigate', 'setApplicationLoading', 'setCurrentUser', and 'setIsAuthenticated'. Either include them or remove
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:419:9 react-hooks/exhaustive-deps The 'resetUserDetails' function makes the dependencies of useMemo Hook (at line 964) change on every render. To fix this, wrap the definition of 'resetUserDetai
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:520:45 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 12 which is greater than 10 authorized.","cost":2,"secondaryLocations":[{"line":520,"column":44,"endLine":520,"endColum
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:582:6 react-hooks/exhaustive-deps React Hook useEffect has missing dependencies: 'getLoggedInUserDetails' and 'startTokenExpiryTimer'. Either include them or remove the dependency array.
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:592:9 react-hooks/exhaustive-deps The 'handleFailedLogin' function makes the dependencies of useMemo Hook (at line 964) change on every render. Move it inside the useMemo callback. Alternatively
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:650:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'authConfig?.provider', 'handledVerifiedUser', 'navigate', 'resetUserDetails', and 'startTokenExpiryTimer'. Eit
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:695:9 react-hooks/exhaustive-deps The 'initializeAxiosInterceptors' function makes the dependencies of useMemo Hook (at line 964) change on every render. To fix this, wrap the definition of 'ini
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:763:67 sonarjs/no-nested-functions Refactor this code to not nest functions more than 4 levels deep.
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:770:23 openmetadata-imports/no-api-calls-in-iteration Avoid issuing one API request per item. Fetch at the data owner, use a bulk endpoint, or use useQueries with an intentional concurrency policy.
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:782:37 sonarjs/no-nested-functions Refactor this code to not nest functions more than 4 levels deep.
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:791:27 sonarjs/no-nested-functions Refactor this code to not nest functions more than 4 levels deep.
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:860:30 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 17 which is greater than 10 authorized.","cost":7,"secondaryLocations":[{"line":860,"column":29,"endLine":860,"endColum
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:953:6 react-hooks/exhaustive-deps React Hook useEffect has missing dependencies: 'cleanup', 'fetchAuthConfig', 'initializeAxiosInterceptors', and 'startTokenExpiryTimer'. Either include them or
🟡 src/components/Settings/Applications/plugins/AppPlugin.ts:19:1 openmetadata-imports/no-circular-imports This runtime import participates in a circular dependency. Extract the shared type/constant/utility or invert the dependency.
🟡 src/components/Settings/Persona/CustomizeUI/CustomizeUI.tsx:64:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'navigate'. Either include it or remove the dependency array. Outer scope values like 'history' aren't valid de
🟡 src/components/Settings/Users/UserProfileIcon/UserProfileIcon.component.tsx:114:9 react-hooks/exhaustive-deps The 'handleSelectedPersonaChange' function makes the dependencies of useCallback Hook (at line 190) change on every render. Move it inside the useCallback callb
🟡 src/components/Settings/Users/UserProfileIcon/UserProfileIcon.component.tsx:155:6 react-hooks/exhaustive-deps React Hook useMemo has unnecessary dependencies: 'currentUser?.inheritedPersonas' and 'currentUser?.personas'. Either exclude them or remove the dependency arra
🟡 src/components/Settings/Users/UserProfileIcon/UserProfileIcon.component.tsx:401:5 react-hooks/exhaustive-deps React Hook useMemo has missing dependencies: 'handleCloseDropdown', 'onLogoutHandler', 'personaLabelRenderer', 'readMoreTeamRenderer', and 'teamLabelRenderer'.
🟡 src/components/common/FilterButton/FilterButton.tsx:84:66 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 11 which is greater than 10 authorized.","cost":1,"secondaryLocations":[{"line":84,"column":65,"endLine":84,"endColumn"
🟡 src/components/common/FilterButton/FilterButton.tsx:108:9 sonarjs/no-nested-conditional Extract this nested ternary operation into an independent statement.
🟡 src/components/common/FilterButton/FilterButton.tsx:128:6 react-hooks/exhaustive-deps React Hook useMemo has a missing dependency: 'selectedValues'. Either include it or remove the dependency array.
🟡 src/components/common/FilterButton/FilterButton.tsx:128:23 react-hooks/exhaustive-deps React Hook useMemo has a complex expression in the dependency array. Extract it to a separate variable so it can be statically checked.
🟡 src/components/common/FilterButton/FilterButton.tsx:235:31 sonarjs/no-nested-conditional Extract this nested ternary operation into an independent statement.
🟡 src/components/discovery/explore/ExploreHeader/ExploreSearchCard.tsx:145:11 sonarjs/no-nested-conditional Extract this nested ternary operation into an independent statement.
🟡 src/components/discovery/personal-space/AIUserMenu/AIUserMenu.tsx:219:71 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 13 which is greater than 10 authorized.","cost":3,"secondaryLocations":[{"line":219,"column":70,"endLine":219,"endColum
🟡 src/components/discovery/personal-space/AIUserMenu/AIUserMenu.tsx:375:5 react-hooks/exhaustive-deps React Hook useMemo has an unnecessary dependency: 'openPanel'. Either exclude it or remove the dependency array.
🟡 src/components/discovery/personal-space/InboxIconButton/InboxIconButton.tsx:84:9 sonarjs/no-nested-conditional Extract this nested ternary operation into an independent statement.
🟡 src/components/discovery/personal-space/InboxPage/components/ActivityDetailDrawer.tsx:109:4 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 15 which is greater than 10 authorized.","cost":5,"secondaryLocations":[{"line":109,"column":3,"endLine":109,"endColumn
🟡 src/components/discovery/personal-space/InboxPage/components/ActivityDetailDrawer.tsx:190:10 sonarjs/expression-complexity Reduce the number of conditional operators (4) used in the expression (maximum allowed 3).
🟡 src/components/discovery/personal-space/InboxPage/components/ActivityDetailDrawer.tsx:267:4 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 30 which is greater than 10 authorized.","cost":20,"secondaryLocations":[{"line":267,"column":3,"endLine":267,"endColum
🟡 src/components/discovery/personal-space/InboxPage/components/ActivityDetailDrawer.tsx:352:7 sonarjs/no-nested-conditional Extract this nested ternary operation into an independent statement.
🟡 src/components/discovery/personal-space/InboxPage/components/ActivityFeedItem.tsx:71:4 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 21 which is greater than 10 authorized.","cost":11,"secondaryLocations":[{"line":71,"column":3,"endLine":71,"endColumn"
🟡 src/components/discovery/personal-space/InboxPage/components/ActivityFeedItem.tsx:100:7 sonarjs/no-nested-conditional Extract this nested ternary operation into an independent statement.
🟡 src/components/discovery/personal-space/InboxPage/components/TaskActionCommentModal.tsx:82:4 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 23 which is greater than 10 authorized.","cost":13,"secondaryLocations":[{"line":82,"column":3,"endLine":82,"endColumn"
🟡 src/components/discovery/personal-space/InboxPage/components/TaskActionCommentModal.tsx:190:42 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 3 times.
🟡 src/components/discovery/personal-space/InboxPage/components/TaskDetailPanel.tsx:52:1 openmetadata-imports/no-lower-layer-page-imports Pages are route-level composition modules. Move the shared implementation/type to a lower layer instead of importing a page from here.
🟡 src/components/discovery/personal-space/InboxPage/components/TaskDetailPanel.tsx:158:4 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 11 which is greater than 10 authorized.","cost":1,"secondaryLocations":[{"line":158,"column":3,"endLine":158,"endColumn
🟡 src/components/discovery/personal-space/InboxPage/components/TaskDetailPanel.tsx:221:10 sonarjs/expression-complexity Reduce the number of conditional operators (4) used in the expression (maximum allowed 3).
🟡 src/components/discovery/personal-space/InboxPage/components/TaskDetailPanel.tsx:276:24 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 3 times.
🟡 src/components/discovery/personal-space/InboxPage/components/TaskDetailPanel.tsx:293:4 sonarjs/cognitive-complexity Refactor this function to reduce its Cognitive Complexity from 16 to the 15 allowed.
🟡 src/components/discovery/personal-space/InboxPage/components/TaskDetailPanel.tsx:293:4 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 22 which is greater than 10 authorized.","cost":12,"secondaryLocations":[{"line":293,"column":3,"endLine":293,"endColum
🟡 src/components/discovery/personal-space/InboxPage/components/TaskDetailPanel.tsx:445:41 openmetadata-imports/no-api-calls-in-iteration Avoid issuing one API request per item. Fetch at the data owner, use a bulk endpoint, or use useQueries with an intentional concurrency policy.
🟡 src/components/discovery/personal-space/InboxPage/components/TaskDetailPanel.tsx:621:7 sonarjs/no-nested-conditional Extract this nested ternary operation into an independent statement.
🟡 src/components/discovery/personal-space/InboxPage/components/TaskDetailPanel.tsx:655:9 sonarjs/no-nested-conditional Extract this nested ternary operation into an independent statement.
🟡 src/components/discovery/personal-space/InboxPage/components/TaskDetailPanel.tsx:704:33 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 12 which is greater than 10 authorized.","cost":2,"secondaryLocations":[{"line":704,"column":32,"endLine":704,"endColum
🟡 src/components/discovery/personal-space/InboxPage/components/TaskDetailPanel.tsx:744:23 sonarjs/no-nested-conditional Extract this nested ternary operation into an independent statement.

… and 183 more. Run make ui-checkstyle-changed locally for the full list.


Fix locally (fast - only checks files changed in this branch):

make ui-checkstyle-changed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@gitar-bot

gitar-bot Bot commented Aug 29, 2026

Copy link
Copy Markdown
Code Review ⚠️ Changes requested 6 resolved / 7 findings

Migrates the AI-assistant shell and shared routes into the OSS app-mode framework, but requests changes because the TeamUserSelectInput trigger lacks keyboard accessibility with no role or tabIndex.

⚠️ Quality: Team/user select trigger not keyboard-accessible

📄 openmetadata-ui/src/main/resources/ui/src/components/observability/Alerts/AlertAiDestinationConfigFields.component.tsx:253-267

The TeamUserSelectInput trigger is a plain <div> with onClick/onKeyDown but no role="button" and no tabIndex. Because the div is not in the tab order it can never receive focus, so the onKeyDown (Enter/Space) handler never fires for keyboard or screen-reader users — the dropdown is only openable with a mouse. Add role="button", tabIndex={isDisabled ? -1 : 0}, and aria-expanded/aria-haspopup, or (preferred per the design-system rules) use a real Core UI Button/Select element instead of a click-handled div.

Make the trigger focusable and expose button semantics.
<div
  aria-expanded={isDropdownOpen}
  aria-haspopup="listbox"
  className={classNames(
    ALERT_AI_FORM_CLASS_NAMES.teamUserSelectTrigger,
    isDisabled && ALERT_AI_FORM_CLASS_NAMES.teamUserSelectTriggerDisabled
  )}
  data-testid={`team-user-select-trigger-${destinationNumber}`}
  ref={triggerRef}
  role="button"
  tabIndex={isDisabled ? -1 : 0}
  onClick={isDisabled ? undefined : () => setIsDropdownOpen((p) => !p)}
  onKeyDown={/* unchanged */}>
✅ 6 resolved
Bug: Tenant default "classicV1" is not translated, silently falls to Classic

📄 openmetadata-ui/src/main/resources/ui/src/hooks/useAppMode.ts:407-419 📄 openmetadata-spec/src/main/resources/json/schema/api/configuration/appConfiguration.json:12
The appConfiguration.defaultAppMode enum now permits "classicV1", but CONFIG_MODE_TO_RUNTIME only maps Classic and AI, so translateWireMode('classicV1') returns null and appDefault falls through to DEFAULT_APP_MODE. An admin who sets the tenant-wide default to classicV1 silently gets Classic. Add a classicV1 -> CLASSIC_V1_APP_MODE entry (and regenerate the DefaultAppMode enum) so the new wire value resolves.

Bug: FilterButton uses missing i18n key label.no-options

📄 openmetadata-ui/src/main/resources/ui/src/components/common/FilterButton/FilterButton.tsx:212-214 📄 openmetadata-ui/src/main/resources/ui/src/locale/languages/en-us.json
When the option list is empty and no emptyLabel is passed, FilterButton renders t('label.no-options'), but that key does not exist in en-us.json (nor any other locale). Users will see the raw key string instead of translated empty-state text. Add a no-options entry under label in en-us.json (and the other locales, English placeholders consistent with the other new keys), or reuse an existing key.

Bug: Fresh cross-tab hint never adopted into app-mode store at boot

📄 openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/AuthProvider.tsx:193-196 📄 openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/AuthProvider.tsx:221-232 📄 openmetadata-ui/src/main/resources/ui/src/hooks/useAppMode.ts:226-236
On an already-authenticated tab (e.g. cmd+click opening a new tab) with a fresh omAppModeHint but no session tuple, hydrateAndResolveAppMode returns early at line 194-196 without calling writeAppMode, and the module-init store is seeded only from the session tuple (never the hint). Since useResolvedAppMode — which previously read the hint and adopted it via writeAppMode — was deleted, nothing writes the hint's mode into useAppModeStore, so the new tab stays in DEFAULT_APP_MODE (Classic) even though a sibling tab is in ClassicV1. resolveInitialAppMode only influences the post-login navigate() path, not the returning-tab store. This defeats the cross-tab hint mechanism the code's own comments (line 221-232) claim to honor. Adopt the hint into the store in the fresh-hint branch before returning.

Bug: 'on' drop onto a nested More-child silently drops the item

📄 openmetadata-ui/src/main/resources/ui/src/pages/CustomizeAppModeSidebarPage/CustomizeAppModeSidebarPage.utils.ts:266-280 📄 openmetadata-ui/src/main/resources/ui/src/pages/CustomizeAppModeSidebarPage/CustomizeAppModeSidebarPage.utils.ts:312-326
In insertNode, the position === 'on' branch only searches the top level (nodes.map(... current.key === targetKey)). If the Tree emits dropPosition: 'on' with a targetKey that is a leaf nested inside the "More" node, no top-level node matches, so the detached node is never re-inserted and insertNode returns rest without it. moveSidebarNode then passes isValidSidebarTree(next) (the structure is still valid — the item just vanished) and commits the tree, permanently losing the dragged nav item. The before/after branches handle nested targets, but on does not, and no test covers an on drop onto a More child. Guard against dropping the node by verifying it survived the insert (or reverting when the total node count decreases).

Quality: import statement placed after const declaration

📄 openmetadata-ui/src/main/resources/ui/src/components/platform/ai-shell/AppModeRoutes/AppModeRoutes.tsx:30-33
In AppModeRoutes.tsx the import { useSyncActiveModule } statement now appears after the const PageNotFound = withPageSuspenseFallback(...) declaration. While imports are hoisted so this works at runtime, it violates the import/first convention and is easy to break during refactors. Move the import up with the other imports and declare PageNotFound below all imports.

...and 1 more resolved from earlier reviews

🤖 Prompt for agents
Code Review: Migrates the AI-assistant shell and shared routes into the OSS app-mode framework, but requests changes because the TeamUserSelectInput trigger lacks keyboard accessibility with no role or tabIndex.

1. ⚠️ Quality: Team/user select trigger not keyboard-accessible
   Files: openmetadata-ui/src/main/resources/ui/src/components/observability/Alerts/AlertAiDestinationConfigFields.component.tsx:253-267

   The `TeamUserSelectInput` trigger is a plain `<div>` with `onClick`/`onKeyDown` but no `role="button"` and no `tabIndex`. Because the div is not in the tab order it can never receive focus, so the `onKeyDown` (Enter/Space) handler never fires for keyboard or screen-reader users — the dropdown is only openable with a mouse. Add `role="button"`, `tabIndex={isDisabled ? -1 : 0}`, and `aria-expanded`/`aria-haspopup`, or (preferred per the design-system rules) use a real Core UI `Button`/`Select` element instead of a click-handled div.

   Fix (Make the trigger focusable and expose button semantics.):
   <div
     aria-expanded={isDropdownOpen}
     aria-haspopup="listbox"
     className={classNames(
       ALERT_AI_FORM_CLASS_NAMES.teamUserSelectTrigger,
       isDisabled && ALERT_AI_FORM_CLASS_NAMES.teamUserSelectTriggerDisabled
     )}
     data-testid={`team-user-select-trigger-${destinationNumber}`}
     ref={triggerRef}
     role="button"
     tabIndex={isDisabled ? -1 : 0}
     onClick={isDisabled ? undefined : () => setIsDropdownOpen((p) => !p)}
     onKeyDown={/* unchanged */}>

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source

@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs skip-pr-checks Bypass PR metadata validation check UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants