You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "EnxDev (via GitHub)" <gi...@apache.org> on 2026/05/26 14:20:59 UTC
[PR] feat(extensions): context sharing namespaces (navigation, explore, dashboard, dataset) [superset]
EnxDev opened a new pull request, #40444:
URL: https://github.com/apache/superset/pull/40444
### SUMMARY
Adds host namespaces that extensions can call to read Superset context: current page type, active dashboard filters, explore chart info, and dataset metadata.
**What this introduces:**
- `navigation` — `onDidChangePage: Event<PageType>` fires on every route change; routing signal only (no entity context)
- `dashboard` — `getContext()` returns active filters from the Redux store
- `explore` — `getContext()` returns chart name, datasource, and current form data
- `dataset` — `getContext()` returns datasource UID and name from explore state
- All namespaces are exposed on `window.superset` for extension use
Extensions compose context themselves from whichever namespaces they need. Surface namespaces (`explore`, `dashboard`, `dataset`) own entity-level context; `navigation` is a routing signal only.
**Stack:**
1. [Contribution point](https://github.com/apache/superset/pull/40439)
2. [Frontend API entry point](https://github.com/apache/superset/pull/40440)
3. [Eager loading](https://github.com/apache/superset/pull/40441)
4. [Admin UI](https://github.com/apache/superset/pull/40442)
5. [Backend settings & permissions](https://github.com/apache/superset/pull/40443) ← merged first
6. **This PR** — context sharing namespaces
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — API surface only, no visible UI change.
### TESTING INSTRUCTIONS
1. On a dashboard page: `window.superset.dashboard.getContext()` → returns `{ filters: [...] }`
2. On an explore page: `window.superset.explore.getContext()` → returns `{ chartName, datasourceUid, formData }`
3. Navigate between pages: verify `navigation.onDidChangePage` fires with the correct `PageType` string
4. On a non-explore page: `window.superset.explore.getContext()` → returns `null`
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [x] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(extensions): context sharing namespaces (navigation, explore, dashboard, dataset) [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40444:
URL: https://github.com/apache/superset/pull/40444#discussion_r3304481740
##########
superset-frontend/src/core/dashboard/index.ts:
##########
@@ -0,0 +1,90 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Host-internal implementation of the `dashboard` namespace.
+ *
+ * Wraps Redux dashboardInfo and dataMask state and normalizes them into the
+ * stable `DashboardContext` contract. Extensions must not depend on the Redux
+ * slice structure directly.
+ */
+
+import type { dashboard as dashboardApi } from '@apache-superset/core';
+import { HYDRATE_DASHBOARD } from 'src/dashboard/actions/hydrate';
+import {
+ UPDATE_DATA_MASK,
+ SET_DATA_MASK_FOR_FILTER_CHANGES_COMPLETE,
+} from 'src/dataMask/actions';
+import { store, RootState } from 'src/views/store';
+import { AnyListenerPredicate } from '@reduxjs/toolkit';
+import { createActionListener } from '../utils';
+
+type DashboardContext = dashboardApi.DashboardContext;
+type FilterValue = dashboardApi.FilterValue;
+
+function buildDashboardContext(): DashboardContext | undefined {
+ const state = store.getState();
+ const info = (state as any).dashboardInfo;
+ if (!info?.id) return undefined;
Review Comment:
**Suggestion:** `getCurrentDashboard()` can return stale dashboard context on non-dashboard pages because it only checks for `dashboardInfo.id`, and dashboard state is kept in the shared store after navigation. This violates the function contract ("undefined when not on Dashboard page") and can make extensions read outdated dashboard identity/filters. Add an explicit page-type/route check before building and returning dashboard context. [stale reference]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Extensions misidentify dashboard when user leaves dashboard page.
- ⚠️ Chatbot context uses outdated dashboard filters across navigation.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. The global Redux store for the SPA is created in
`superset-frontend/src/views/store.ts:132-152,184-185`, where the `dashboardInfo` reducer
is registered and `store` is exported for reuse across all routes.
2. When a dashboard is loaded, the `HYDRATE_DASHBOARD` thunk in
`superset-frontend/src/dashboard/actions/hydrate.ts:103-112` dispatches an action consumed
by `dashboardInfoReducer` in
`superset-frontend/src/dashboard/reducers/dashboardInfo.ts:60-66,122-129,194-27`,
populating `state.dashboardInfo` (including its `id`) in the shared store.
3. Navigate away to a non-dashboard route (e.g. `/sqllab/` or `/superset/welcome/`);
`ExtensionsStartup` in `superset-frontend/src/extensions/ExtensionsStartup.tsx:63-80`
calls `notifyPageChange(location.pathname)`, and `navigation` updates `currentPageType`
via `derivePageType()` in `superset-frontend/src/core/navigation/index.ts:34-52`, but no
reducer clears `state.dashboardInfo`.
4. On that non-dashboard page, an extension or developer calls
`window.superset.dashboard.getCurrentDashboard()`, which resolves to `getCurrentDashboard`
in `superset-frontend/src/core/dashboard/index.ts:73-74`; this calls
`buildDashboardContext()` which only checks `info?.id` at
`superset-frontend/src/core/dashboard/index.ts:43-44`, finds the persisted
`dashboardInfo.id`, and returns a `DashboardContext` instead of `undefined`, violating the
contract documented in
`superset-frontend/packages/superset-core/src/dashboard/index.ts:58-61` ("`undefined` when
the user is not on a Dashboard page").
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f84939dfd6c445869e6a4e70a936360c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=f84939dfd6c445869e6a4e70a936360c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/core/dashboard/index.ts
**Line:** 43:44
**Comment:**
*Stale Reference: `getCurrentDashboard()` can return stale dashboard context on non-dashboard pages because it only checks for `dashboardInfo.id`, and dashboard state is kept in the shared store after navigation. This violates the function contract ("undefined when not on Dashboard page") and can make extensions read outdated dashboard identity/filters. Add an explicit page-type/route check before building and returning dashboard context.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40444&comment_hash=7b1e6ea678ff692618076639b810c7f63d6e175ed2a016c0d59b614901e4dad3&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40444&comment_hash=7b1e6ea678ff692618076639b810c7f63d6e175ed2a016c0d59b614901e4dad3&reaction=dislike'>👎</a>
##########
superset-frontend/src/core/explore/index.ts:
##########
@@ -0,0 +1,84 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Host-internal implementation of the `explore` namespace.
+ *
+ * Wraps Redux explore state and normalizes it into the stable `ChartContext`
+ * contract. Extensions must not depend on the Redux slice structure directly.
+ */
+
+import type { explore as exploreApi } from '@apache-superset/core';
+import { HYDRATE_EXPLORE } from 'src/explore/actions/hydrateExplore';
+import {
+ SET_FORM_DATA,
+ UPDATE_CHART_TITLE,
+} from 'src/explore/actions/exploreActions';
+import { SET_DATASOURCE } from 'src/explore/actions/datasourcesActions';
+import { store, RootState } from 'src/views/store';
+import { AnyListenerPredicate } from '@reduxjs/toolkit';
+import { createActionListener } from '../utils';
+
+type ChartContext = exploreApi.ChartContext;
+
+function buildChartContext(): ChartContext | undefined {
+ const state = store.getState();
+ const exploreState = (state as any).explore;
+ if (!exploreState) return undefined;
Review Comment:
**Suggestion:** `getCurrentChart()` will incorrectly return a chart context even on non-Explore pages because it only checks whether the `explore` slice exists, and that slice is always present in the global store with an initial object. This breaks the API contract (`undefined` when not on Explore) and can expose stale/empty chart context to extensions. Gate this by current route/page type (or an explore-loaded signal) before returning context. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Explore-aware extensions treat empty context as active explore session.
- ⚠️ Misaligned API contract complicates extension logic and error handling.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. The shared SPA Redux store is configured in
`superset-frontend/src/views/store.ts:132-152`, where the `explore` reducer from
`superset-frontend/src/explore/reducers/exploreReducer.ts:38-40` is always registered,
giving `state.explore` an initial object `{ controls: {}, form_data: {} }` even when
Explore has never been visited.
2. On any non-Explore route (e.g. `/superset/welcome/`), the extensions host is
initialized by `ExtensionsStartup` in
`superset-frontend/src/extensions/ExtensionsStartup.tsx:63-80,101-125`, which attaches
`explore` from `src/core` onto `window.superset.explore`.
3. An extension or developer calls `window.superset.explore.getCurrentChart()` on this
non-Explore page; this invokes `getCurrentChart` in
`superset-frontend/src/core/explore/index.ts:67-68`, which calls `buildChartContext()` at
`superset-frontend/src/core/explore/index.ts:40-58`.
4. Inside `buildChartContext()`, `exploreState` is read from `state.explore` at
`superset-frontend/src/core/explore/index.ts:41-42`, and since the reducer's initial state
is an object, the `if (!exploreState) return undefined;` guard at line 43 never fires; a
`ChartContext` with `chartId=null`, `chartName=null`, `vizType=''`, and
`datasourceId=null` is returned even off the Explore page, contradicting the documented
contract in `superset-frontend/packages/superset-core/src/explore/index.ts:48-51` that
`getCurrentChart()` returns `undefined` when the user is not on Explore.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=22a173986f6647378a97385feb1588f4&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=22a173986f6647378a97385feb1588f4&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/core/explore/index.ts
**Line:** 42:43
**Comment:**
*Api Mismatch: `getCurrentChart()` will incorrectly return a chart context even on non-Explore pages because it only checks whether the `explore` slice exists, and that slice is always present in the global store with an initial object. This breaks the API contract (`undefined` when not on Explore) and can expose stale/empty chart context to extensions. Gate this by current route/page type (or an explore-loaded signal) before returning context.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40444&comment_hash=c202b95962114dddeac824cc02210ae3e7b196c34cc9f26de608e79439b257bc&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40444&comment_hash=c202b95962114dddeac824cc02210ae3e7b196c34cc9f26de608e79439b257bc&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(extensions): context sharing namespaces (navigation, explore, dashboard, dataset) [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40444:
URL: https://github.com/apache/superset/pull/40444#issuecomment-4544989736
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(extensions): context sharing namespaces (navigation, explore, dashboard, dataset) [superset]
Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz commented on PR #40444:
URL: https://github.com/apache/superset/pull/40444#issuecomment-4613699933
ci needs fixing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(extensions): context sharing namespaces (navigation, explore, dashboard, dataset) [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #40444:
URL: https://github.com/apache/superset/pull/40444#issuecomment-4545031405
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | d1aa512ec99034a3aefcbe4dfa9425885b67e172 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a15ac4ccd851500097249dc |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-40444--superset-docs-preview.netlify.app](https://deploy-preview-40444--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DctBDoMgEEDRu8xaAjSixRv0GOPMVEwsEJw0Nk3vXnZ_8f4XaiuvqrAADLA2zJR6S75Y3vYpqJYS6lrUUMkql5ozYdvz1jlLPcrnwX2Y0AekkYjvwQfnXJxvY2SC3x8) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-40444--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(extensions): context sharing namespaces (navigation, explore, dashboard, dataset) [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #40444:
URL: https://github.com/apache/superset/pull/40444#issuecomment-4545624158
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/40444?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `0%` with `2 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 67.72%. Comparing base ([`b0d2619`](https://app.codecov.io/gh/apache/superset/commit/b0d26196fc50340cfca4ee27373bf5b81f95e8b4?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`8f03c5a`](https://app.codecov.io/gh/apache/superset/commit/8f03c5a1eaeec3da9697ee3ac91c037713c35578?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/40444?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [superset-frontend/src/views/contributions.ts](https://app.codecov.io/gh/apache/superset/pull/40444?src=pr&el=tree&filepath=superset-frontend%2Fsrc%2Fviews%2Fcontributions.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL2NvbnRyaWJ1dGlvbnMudHM=) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40444?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## enxdev/chat-prototype #40444 +/- ##
=========================================================
+ Coverage 64.18% 67.72% +3.53%
=========================================================
Files 2592 1850 -742
Lines 139036 76990 -62046
Branches 32282 24688 -7594
=========================================================
- Hits 89244 52141 -37103
+ Misses 48260 24849 -23411
+ Partials 1532 0 -1532
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/40444/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/40444/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/40444/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/40444/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/40444/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [python](https://app.codecov.io/gh/apache/superset/pull/40444/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (+39.56%)` | :arrow_up: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/40444/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/40444/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/40444?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(extensions): context sharing namespaces (navigation, explore, dashboard, dataset) [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev merged PR #40444:
URL: https://github.com/apache/superset/pull/40444
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(extensions): context sharing namespaces (navigation, explore, dashboard, dataset) [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40444:
URL: https://github.com/apache/superset/pull/40444#discussion_r3304462053
##########
superset-frontend/src/core/explore/index.ts:
##########
@@ -0,0 +1,84 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Host-internal implementation of the `explore` namespace.
+ *
+ * Wraps Redux explore state and normalizes it into the stable `ChartContext`
+ * contract. Extensions must not depend on the Redux slice structure directly.
+ */
+
+import type { explore as exploreApi } from '@apache-superset/core';
+import { HYDRATE_EXPLORE } from 'src/explore/actions/hydrateExplore';
+import {
+ SET_FORM_DATA,
+ UPDATE_CHART_TITLE,
+} from 'src/explore/actions/exploreActions';
+import { SET_DATASOURCE } from 'src/explore/actions/datasourcesActions';
+import { store, RootState } from 'src/views/store';
+import { AnyListenerPredicate } from '@reduxjs/toolkit';
+import { createActionListener } from '../utils';
+
+type ChartContext = exploreApi.ChartContext;
+
+function buildChartContext(): ChartContext | undefined {
+ const state = store.getState();
+ const exploreState = (state as any).explore;
+ if (!exploreState) return undefined;
Review Comment:
**🟠 Architect Review — HIGH**
getCurrentChart() derives context solely from the global `explore` reducer, which is always mounted, so it returns a ChartContext (often empty or stale) even when the user is not on the Explore page, violating the documented contract that it should return `undefined` off Explore.
**Suggestion:** Gate buildChartContext by the current surface (for example by checking navigation.getPageType() === 'explore') and return undefined when not on Explore, or otherwise clear/disable the explore slice when leaving Explore so off-Explore calls do not return chart context.
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a6c975c5ffca4ae9bef34bd1cc0d0b88&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=a6c975c5ffca4ae9bef34bd1cc0d0b88&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.
**Path:** superset-frontend/src/core/explore/index.ts
**Line:** 40:43
**Comment:**
*HIGH: getCurrentChart() derives context solely from the global `explore` reducer, which is always mounted, so it returns a ChartContext (often empty or stale) even when the user is not on the Explore page, violating the documented contract that it should return `undefined` off Explore.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
##########
superset-frontend/src/core/dashboard/index.ts:
##########
@@ -0,0 +1,90 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Host-internal implementation of the `dashboard` namespace.
+ *
+ * Wraps Redux dashboardInfo and dataMask state and normalizes them into the
+ * stable `DashboardContext` contract. Extensions must not depend on the Redux
+ * slice structure directly.
+ */
+
+import type { dashboard as dashboardApi } from '@apache-superset/core';
+import { HYDRATE_DASHBOARD } from 'src/dashboard/actions/hydrate';
+import {
+ UPDATE_DATA_MASK,
+ SET_DATA_MASK_FOR_FILTER_CHANGES_COMPLETE,
+} from 'src/dataMask/actions';
+import { store, RootState } from 'src/views/store';
+import { AnyListenerPredicate } from '@reduxjs/toolkit';
+import { createActionListener } from '../utils';
+
+type DashboardContext = dashboardApi.DashboardContext;
+type FilterValue = dashboardApi.FilterValue;
+
+function buildDashboardContext(): DashboardContext | undefined {
+ const state = store.getState();
+ const info = (state as any).dashboardInfo;
+ if (!info?.id) return undefined;
+
Review Comment:
**🟠 Architect Review — HIGH**
getCurrentDashboard() only checks whether `dashboardInfo.id` is set in the shared Redux store, so after visiting a dashboard and navigating to a non-dashboard route it continues returning the last dashboard's context instead of `undefined`, contradicting the API contract that it only reflects the dashboard page currently being viewed.
**Suggestion:** Add an explicit surface guard (for example using navigation.getPageType() === 'dashboard') and/or clear `dashboardInfo` when leaving dashboard routes so getCurrentDashboard() returns undefined on non-dashboard pages.
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=02c4b70085c64009b044e53421a22a4c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=02c4b70085c64009b044e53421a22a4c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.
**Path:** superset-frontend/src/core/dashboard/index.ts
**Line:** 41:45
**Comment:**
*HIGH: getCurrentDashboard() only checks whether `dashboardInfo.id` is set in the shared Redux store, so after visiting a dashboard and navigating to a non-dashboard route it continues returning the last dashboard's context instead of `undefined`, contradicting the API contract that it only reflects the dashboard page currently being viewed.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
##########
superset-frontend/src/core/dataset/index.ts:
##########
@@ -0,0 +1,62 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Host-internal implementation of the `dataset` namespace.
+ *
+ * Dataset page components call `setCurrentDataset` to publish context as they
+ * load. Extensions consume the stable `DatasetContext` contract; they are
+ * isolated from the page's internal data-fetching implementation.
+ */
+
+import type { dataset as datasetApi } from '@apache-superset/core';
+import { Disposable } from '../models';
+
+type DatasetContext = datasetApi.DatasetContext;
+
+let currentDataset: DatasetContext | undefined;
+const listeners = new Set<(ctx: DatasetContext) => void>();
+
+/**
+ * Host-internal: called by the Dataset page when its entity loads or changes.
+ * Not part of the public `@apache-superset/core` API.
+ */
+export const setCurrentDataset = (ctx: DatasetContext | undefined): void => {
+ currentDataset = ctx;
+ if (ctx) {
+ listeners.forEach(fn => fn(ctx));
+ }
+};
Review Comment:
**🟠 Architect Review — HIGH**
The dataset namespace relies on setCurrentDataset() being invoked by dataset pages, but there are no call sites in the codebase, so `currentDataset` is never set, getCurrentDataset() always returns `undefined`, and onDidChangeDataset never emits under normal usage.
**Suggestion:** Integrate setCurrentDataset() into the DatasetCreation/dataset detail page lifecycle (and clear it on unmount or when leaving the dataset surface) so dataset context is populated and the dataset namespace behaves as documented.
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8f5bfeeb79574b27a72b2298ebf44c3f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=8f5bfeeb79574b27a72b2298ebf44c3f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.
**Path:** superset-frontend/src/core/dataset/index.ts
**Line:** 40:45
**Comment:**
*HIGH: The dataset namespace relies on setCurrentDataset() being invoked by dataset pages, but there are no call sites in the codebase, so `currentDataset` is never set, getCurrentDataset() always returns `undefined`, and onDidChangeDataset never emits under normal usage.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(extensions): context sharing namespaces (navigation, explore, dashboard, dataset) [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40444:
URL: https://github.com/apache/superset/pull/40444#discussion_r3304468668
##########
superset-frontend/src/core/dataset/index.ts:
##########
@@ -0,0 +1,62 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Host-internal implementation of the `dataset` namespace.
+ *
+ * Dataset page components call `setCurrentDataset` to publish context as they
+ * load. Extensions consume the stable `DatasetContext` contract; they are
Review Comment:
**Suggestion:** `onDidChangeDataset` passes the same mutable object reference stored in `currentDataset` directly to listeners, so any listener mutation can corrupt shared host state and affect later reads. Emit a cloned payload (and preferably store an immutable copy) before notifying subscribers. [stale reference]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ One extension can corrupt dataset context for all others.
- ⚠️ getCurrentDataset returns mutated data, not original snapshot.
- ⚠️ Subtle cross-extension bugs from hidden shared object mutation.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. The host dataset namespace keeps its current state in `currentDataset` and updates it
via `setCurrentDataset` implemented in
`superset-frontend/src/core/dataset/index.ts:21-24`, where `currentDataset = ctx` assigns
the caller-provided `DatasetContext` object directly.
2. After updating, `setCurrentDataset` notifies subscribers with `listeners.forEach(fn =>
fn(ctx))` (`src/core/dataset/index.ts:22-24`), passing the same `ctx` reference that was
just stored in `currentDataset`, without cloning or freezing it.
3. Extensions are expected to subscribe using `dataset.onDidChangeDataset` as documented
in `packages/superset-core/src/dataset/index.ts:62-70` (e.g., `const sub =
dataset.onDidChangeDataset(ds => { ... });`); if any subscriber mutates fields on the
received `ds` object (for example, normalizing strings or attaching metadata properties),
it is mutating the same object instance saved in `currentDataset`.
4. Subsequent calls to `dataset.getCurrentDataset()` in the host implementation
(`superset-frontend/src/core/dataset/index.ts:28-29`) return `{ ...currentDataset }`,
which is a shallow copy of the already-mutated object; thus all future consumers,
including other extensions, observe the corrupted values introduced by one subscriber,
breaking isolation between internal host state and extension listeners and making bugs
dependent on subscription order and mutation patterns.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=05e16aec88c7442db04d0edbf0b617f2&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=05e16aec88c7442db04d0edbf0b617f2&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/core/dataset/index.ts
**Line:** 22:24
**Comment:**
*Stale Reference: `onDidChangeDataset` passes the same mutable object reference stored in `currentDataset` directly to listeners, so any listener mutation can corrupt shared host state and affect later reads. Emit a cloned payload (and preferably store an immutable copy) before notifying subscribers.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40444&comment_hash=d209731eb786a4aed5081db4d80bcac13c8080a20704a83435c8afd0c97e91b7&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40444&comment_hash=d209731eb786a4aed5081db4d80bcac13c8080a20704a83435c8afd0c97e91b7&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org