feat(extensions): add Chat contribution type (SIP-214)#41205
feat(extensions): add Chat contribution type (SIP-214)#41205michael-s-molina wants to merge 18 commits into
Conversation
Bring the chatbot extension feature branch up to date with master. The chatbot work lives in new paths (superset/extensions/*, the core chatbot namespace, ChatbotMount, superset-core namespaces) and merged cleanly with no conflicts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
michael-s-molina
requested review from
eschutho,
geido,
kgabryje,
rusackas and
villebro
as code owners
|
Bito Review Skipped - Source Branch Not Found |
dosubot
Bot
added
change:frontend
github-actions
Bot
added
doc
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
The issue is valid. The To resolve this, you can replace Would you like me to fetch all other comments on this PR to validate and implement fixes for them as well? superset-frontend/src/views/routes.tsx |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #41205 +/- ##
==========================================
+ Coverage 64.33% 64.36% +0.03%
==========================================
Files 2651 2657 +6
Lines 144766 144936 +170
Branches 33401 33435 +34
==========================================
+ Hits 93131 93285 +154
- Misses 49965 49981 +16
Partials 1670 1670
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| | `onDidChangeDisplayMode(listener)` | Subscribe to display mode changes. Returns a `Disposable`. | | ||
| | `onDidRegisterChat(listener)` | Subscribe to registration events. | | ||
| | `onDidUnregisterChat(listener)` | Subscribe to unregistration events. | | ||
|
|
There was a problem hiding this comment.
@michael-s-molina would it make sense to add onDidResizePanel() here?
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #9a27e6
Actionable Suggestions - 1
-
superset-frontend/src/core/navigation/index.ts - 1
- CWE-20: Browser API at module load · Line 60-61
Additional Suggestions - 4
-
superset-frontend/src/views/routes.tsx - 1
-
Duplicate route registration · Line 239-239The `REGISTRATIONS` route is added twice: once unconditionally at line 239 and again conditionally at line 265 when `authRegistrationEnabled` is true. Remove the duplicate at line 239 to prevent redundant route registration.
Code suggestion
--- a/superset-frontend/src/views/routes.tsx +++ b/superset-frontend/src/views/routes.tsx @@ -236,7 +236,6 @@ const routes: Route[] = [ { path: RoutePaths.USER_INFO, Component: UserInfo }, { path: RoutePaths.ACTION_LOG, Component: ActionLogList }, - { path: RoutePaths.REGISTRATIONS, Component: UserRegistrations }, ]; if (isFeatureEnabled(FeatureFlag.TaggingSystem)) {
-
-
superset-frontend/src/core/chat/ChatProvider.test.ts - 2
-
Misleading test name · Line 37-37The test name 'returns the descriptor copy' is misleading — `getChat()` returns the original reference (ChatProvider.ts:131), not a copy. The assertion uses `toEqual` which passes due to object identity. If a true copy were intended, `toEqual` would fail since the original object would be reused.
Code suggestion
--- superset-frontend/src/core/chat/ChatProvider.test.ts (line 37) --- 37:-test('registerChat sets the registration and returns the descriptor copy', () => { +test('registerChat sets the registration and returns the descriptor', () => {
-
Weak message assertions · Line 54-56Warning message assertions use only `toContain` for IDs. The test doesn't verify the 'Using X; discarding Y' message structure, so a malformed message like 'second.chat, first.chat' would pass.
Code suggestion
--- superset-frontend/src/core/chat/ChatProvider.test.ts (lines 54-56) --- 54: expect(warn).toHaveBeenCalledTimes(1); 55: expect(warn.mock.calls[0][0]).toContain('second.chat'); 56: expect(warn.mock.calls[0][0]).toContain('first.chat'); + expect(warn.mock.calls[0][0]).toMatch(/Using ".*"; discarding ".*"/);
-
-
docs/developer_docs/sidebars.js - 1
-
Sidebar naming inconsistency · Line 50-51The 'Extension Points' category in developer_docs/sidebars.js now has 3 items (sqllab, editors, chat), but docs/sidebarTutorials.js only has 2 (sqllab, editors). Verify whether the chat extension point should also be added to the tutorials sidebar for consistency.
-
Filtered by Review Rules
Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
-
superset-frontend/src/core/chat/ChatHost.test.tsx - 1
- Spy cleanup timing issue · Line 70-89
Review Details
-
Files reviewed - 33 · Commit Range:
380e700..0ad3ef2- docs/developer_docs/sidebars.js
- superset-frontend/packages/superset-core/src/chat/index.ts
- superset-frontend/packages/superset-core/src/common/index.ts
- superset-frontend/packages/superset-core/src/contributions/index.ts
- superset-frontend/packages/superset-core/src/index.ts
- superset-frontend/packages/superset-core/src/navigation/index.ts
- superset-frontend/packages/superset-core/src/views/index.ts
- superset-frontend/src/core/chat/ChatHost.test.tsx
- superset-frontend/src/core/chat/ChatHost.tsx
- superset-frontend/src/core/chat/ChatProvider.test.ts
- superset-frontend/src/core/chat/ChatProvider.ts
- superset-frontend/src/core/chat/index.test.ts
- superset-frontend/src/core/chat/index.ts
- superset-frontend/src/core/editors/EditorProviders.test.ts
- superset-frontend/src/core/editors/EditorProviders.ts
- superset-frontend/src/core/editors/index.ts
- superset-frontend/src/core/index.ts
- superset-frontend/src/core/menus/index.ts
- superset-frontend/src/core/navigation/index.test.ts
- superset-frontend/src/core/navigation/index.ts
- superset-frontend/src/core/sqlLab/index.ts
- superset-frontend/src/core/storeUtils.ts
- superset-frontend/src/core/utils.ts
- superset-frontend/src/core/views/index.ts
- superset-frontend/src/extensions/ExtensionsLoader.test.ts
- superset-frontend/src/extensions/ExtensionsStartup.test.tsx
- superset-frontend/src/extensions/ExtensionsStartup.tsx
- superset-frontend/src/extensions/Namespaces.ts
- superset-frontend/src/utils/localStorageHelpers.ts
- superset-frontend/src/views/App.tsx
- superset-frontend/src/views/routePaths.ts
- superset-frontend/src/views/routes.tsx
- superset/extensions/utils.py
-
Files skipped - 4
- docs/developer_docs/extensions/contribution-types.md - Reason: Filter setting
- docs/developer_docs/extensions/extension-points/chat.md - Reason: Filter setting
- superset-frontend/package-lock.json - Reason: Filter setting
- superset-frontend/packages/superset-core/package.json - Reason: Filter setting
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- Eslint (Linter) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
| const pageEmitter = createValueEventEmitter<Page>( | ||
| derivePage(window.location.pathname), |
There was a problem hiding this comment.
Module-level code on line 61 calls window.location.pathname directly at import time, causing a ReferenceError in non-browser environments (SSR, Jest, unit tests). The pageEmitter must be initialized with a static default value ('home'), with the actual page set when useNavigationTracker runs. (See also: CWE-20)
Code Review Run #9a27e6
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
EnxDev
left a comment
EnxDev
left a comment
There was a problem hiding this comment.
LGTM, just a few minor nits.
Thank you for all the work you've put into this.
It was a pleasure collaborating with you, and I really enjoyed working together on it!
| <Suspense fallback={<Fallback />}> | ||
| <ErrorBoundary | ||
| css={css` | ||
| margin: 16px; |
There was a problem hiding this comment.
Wath do you think to use the Antd Token here? Like theme.fontSizeLG
| height: 100vh; | ||
| overflow: hidden; | ||
| `} | ||
| > |
There was a problem hiding this comment.
Do you think it would make sense to replace this with the Ant Design <Flex> component here?
2 participants
SUMMARY
Implements the Chat contribution type described in SIP-214, co-authored with Enzo Martellucci (@EnxDev). This introduces the
chatandnavigationnamespaces in@apache-superset/core, giving extensions a first-class API to contribute a persistent chat interface and react to page navigation events. The host manages layout, open/close state, display mode, and persistence — extensions implement the trigger and panel components and use the API to drive behavior.These are the first two namespaces in a planned series. Surface-specific namespaces (dashboard, explore, SQL Lab, dataset) will follow in subsequent SIPs, allowing extensions to progressively access deeper contextual information. Agentic UI actions — modifying charts, editing queries, triggering workflows — are intentionally out of scope here and deferred to the Client Actions SIP.
Singleton model
Only one chat is active at a time. This is a deliberate design choice: rendering multiple chat providers simultaneously would create competing conversational experiences and introduce ambiguity for the user. Chat is treated as a deployment-level selection rather than a multi-provider composition.
Registering a chat
The trigger is always visible in the bottom-right corner. The panel can be shown as a floating overlay or as a resizable sidebar docked beside the page content. The user's preference is persisted across reloads.
Display modes
'floating'(default) — panel opens as an overlay anchored to the trigger.'panel'— the page layout splits into content + chat sidebar with a draggable divider.Extensions can switch modes programmatically or react to user-initiated changes:
Navigation awareness
The
navigationnamespace is new in@apache-superset/core. Extensions can query the current page or subscribe to page changes, enabling context-aware chat behavior:Documentation
The Contribution Types page and a new Chat reference page (under
docs/developer_docs/extensions/) cover registration, display modes, the full open/close and event API, and the navigation namespace.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Screen.Recording.2026-06-18.at.10.01.17.mov
TESTING INSTRUCTIONS
Manual verification
Enable the
ENABLE_EXTENSIONSfeature flag, then create a minimal extension with the followingindex.tsx:ADDITIONAL INFORMATION
ENABLE_EXTENSIONSflag