fix(dashboard): refresh semantic metadata across edits

[mikebridge]

SUMMARY

Follow-up to #44455. Dashboard charts on a semantic view decide whether a saved time grain is still valid by consulting the dashboard's semantic metadata. That metadata was captured once at page load, so after editing the dashboard it could be stale, and a saved grain could be kept or dropped based on outdated information.

• Page load, adding a semantic chart, and saving the dashboard now all publish to one dashboard-scoped Redux snapshot of semantic metadata.
• Adding a semantic chart fetches fresh source metadata even when the datasource cache already has a row for it. The provider identifier is normalized only after its numeric ID and type match the chart's source key.
• Saving fetches the dashboard's datasets independently of json_metadata and invalidates the previous snapshot until the response arrives.
• Metadata that is unknown, failed to load, omitted, or mismatched is treated as unproven, so the saved grain is retained rather than cleared. A failed metadata fetch while adding a semantic chart is logged and no longer aborts adding the chart.
• Request IDs and generations guard against stale responses, save/add races, independent adds, same-dashboard remounts, and overlap between page load and add. Late page-load data can still supply sources that are unaffected.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A (frontend state handling, no visual change).

TESTING INSTRUCTIONS

Run the focused Jest suites under superset-frontend/src/dashboard: actions/dashboardState.test.ts, actions/datasources.test.ts, containers/DashboardPage.test.tsx, and reducers/dashboardInfo.test.ts (131 tests pass, 1 skipped). They cover provider identifier rejection, independent adds, navigation and remounts, save/add and failed-fetch interleavings, stale versus fresh page responses, and page-load/add overlap. Changed-file pre-commit hooks, including type checking, pass.

These are mocked frontend tests. No live dashboard or semantic provider was exercised.

ADDITIONAL INFORMATION

• Has associated issue
• Required feature flags
• Changes UI
• Includes DB Migration
• Introduces new feature or API
• Removes existing feature or API

🤖 Generated with Claude Code

[netlify]

✅ Deploy Preview for superset-docs-preview ready!

Name	Link
🔨 Latest commit	d4c95a8
🔍 Latest deploy log	https://app.netlify.com/projects/superset-docs-preview/deploys/6ac66d7238698a0007524d90
😎 Deploy Preview	https://deploy-preview-45052--superset-docs-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[codecov]

Codecov Report

❌ Patch coverage is 96.66667% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.54%. Comparing base (5a2f4c8) to head (79f1551).
⚠️ Report is 40 commits behind head on master.

Files with missing lines	Patch %	Lines
...rontend/src/dashboard/containers/DashboardPage.tsx	84.21%	3 Missing ⚠️
...et-frontend/src/dashboard/actions/dashboardInfo.ts	97.29%	1 Missing ⚠️
...t-frontend/src/dashboard/actions/dashboardState.ts	92.30%	1 Missing ⚠️
...t-frontend/src/dashboard/reducers/dashboardInfo.ts	98.92%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #45052      +/-   ##
==========================================
+ Coverage   82.52%   82.54%   +0.02%     
==========================================
  Files        3005     3005              
  Lines      186595   186761     +166     
  Branches    43234    43338     +104     
==========================================
+ Hits       153979   154168     +189     
+ Misses      29854    29831      -23     
  Partials     2762     2762              

Flag	Coverage Δ
javascript	78.31% <96.66%> (+0.06%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[codeant-ai-for-open-source]

CodeAnt PR Risk: Low Risk

• The PR appears safe to merge; the changed paths refresh semantic datasource metadata and guard against stale save, page-load, and source-fetch responses.
• Added tests cover metadata refreshes, omitted sources, failed requests, dashboard navigation, and competing updates.

Assessed commit: 79f15510bd6d

[aminghadersohi]

No findings; ran the four suites (82 pass) plus guard mutants. Not approving only because codeant's open thread on dashboardState.ts:966 is unaddressed; that non-semantic reject path is unchanged from master, so a reply/resolve is enough.

[mikebridge]

@aminghadersohi I replied to codeant's thread on dashboardState.ts:966 (that reject path is unchanged from master, as you noted), so I think that was the last thing holding this. Would you mind taking another look when you have a moment?

[fitzee]

Verdict: approve. Nothing blocking. Reviewed at d4c95a896d.

I found no correctness bugs. Every path where metadata is missing or uncertain (fetch failure, save in flight, a narrowed 403 payload with columns popped by DASHBOARD_DATASET_INACCESSIBLE_FIELDS, a provider-uid mismatch, a dashboard switch) leaves temporal_columns_lookup undefined. omitDormantGrain then keeps the saved grain, which is how things behaved before #44455. So any ordering mistake in this bookkeeping keeps a grain that could have been dropped. It cannot drop a grain that is still valid.

What I checked and found OK

• No effect loop. The DashboardPage.tsx:390-422 effect depends on semanticDatasetsGeneration. The page-path REPLACE never bumps the generation (reducers/dashboardInfo.ts:205, + (requestId ? 1 : 0)), and useTransformedResource memoizes datasets. So the effect runs once per real change.
• Stale responses cannot win. A late page-load response after a save is rejected by pageGeneration < saveGeneration (reducers/dashboardInfo.ts:164). A late add response after a save started or after a dashboard switch is rejected by the per-key request-id check. A cross-dashboard response is rejected by dashboardId !== state.id. I mutated four guards (datasources.ts:73 cache bypass, reducers/dashboardInfo.ts:164, reducers/dashboardInfo.ts:201, the save-start invalidation at dashboardState.ts:602), and each mutation fails at least one test. The tests exercise the guards and are not just snapshots.
• uid normalization is correct. /api/v1/dashboard/<id>/datasets already rewrites semantic uid to <id>__semantic_view (dashboards/api.py:871). /fetch_datasource_metadata returns the provider uid plus the numeric id, which provenSemanticDataset checks before rewriting (actions/dashboardInfo.ts:79-87).
• Non-semantic datasources are unchanged. fetchDatasourceMetadata without a dashboardId keeps the cached short-circuit and rethrows. Chart.tsx:506-511 already filters to SemanticView. The save-time /datasets GET already existed; it is only moved out of the json_metadata branch.
• Request volume. One extra fetch_datasource_metadata call per semantic chart added. Save still makes one /datasets GET. Nothing scales with chart count. A changed context value re-creates runQuery, but src/components/Chart/Chart.tsx:281-285 only fires when triggerQuery is set, so existing charts do not re-query.

Nits (non-blocking)

1. reducers/dashboardInfo.ts:357-386: the HYDRATE_DASHBOARD branch repeats state.id === action.data.dashboardInfo.id || state.semanticDatasets?.dashboardId === action.data.dashboardInfo.id six times. Compute const sameDashboard = … once. Combined with the four-level guard at :158-167, these seven new dashboardInfo fields are hard to follow. A short comment block listing the invariants (what each generation and request id protects against) would help the next person who edits it.
2. actions/dashboardInfo.ts:87: { ...candidate, uid: sourceKey } stores the whole dataset payload (metrics, verbose_map, …) in dashboardInfo, but the type says Pick<Datasource, 'uid' | 'type' | 'columns'>. Picking { uid, type, columns } would match the type and avoid duplicating state.datasources.
3. DashboardPage.tsx:390-422: each accepted page REPLACE allocates a new semanticDatasets object even when nothing changed. One semantic add causes up to four context identity changes (add start, page re-merge, add end, page re-merge), and each one recomputes formData for every chart through Chart.tsx:540. That is extra render work but no extra queries. A shallow-equality bail-out in the reducer would remove it.
4. datasources.ts:85-100: if a semantic metadata fetch fails or the response fails the provenance check, and the source was not cached, setDatasource is skipped. The chart is still added (an improvement over the old partial add), but it renders against PLACEHOLDER_DATASOURCE (Chart.tsx:251-252) until reload. That is fine, but worth knowing.
5. The tests cover the reducer, the thunks and the page. Nothing asserts the end result, that Chart.tsx sends a different temporal_columns_lookup after a save. The PR body also says no live semantic provider was exercised.

Earlier hold / bot thread: resolved. aminghadersohi's review said a reply or resolve on codeant's dashboardState.ts:966 thread was enough. That thread now has a reply and is marked resolved. The reply is accurate: the Promise.all partial add on an uncached ordinary datasource failure is unchanged from master, because datasources.ts:95 still rethrows when there is no dashboardId. One side effect: semantic and ordinary sources now behave differently on a metadata failure (semantic finishes the add, ordinary leaves it half-added). That supports the follow-up the author proposed, but it should not block this PR.

The four focused suites plus Chart/ and ChartHolder/ pass locally (145 passed, 1 skipped). CI is green.

[gabotorresruiz]

Hi @mikebridge, thanks for the follow-up. Approving: I could not find a path that lands metadata that is present but wrong, and omitDormantGrain retains the saved grain in every unproven case I could reach, so the fail-open direction holds.

I ran the four suites plus the rest of src/dashboard at the head and at the merge base and the failed-test sets are identical, and 20 of the 21 new tests fail without the fix. I also checked your reply on the dashboardState.ts thread and it holds up: the Promise.all sequencing is unchanged from the merge base and the ordinary datasource fetch still rethrows at datasources.ts:93.

CI is fully green (66 passing, the rest skipped by path filters). Three non-blocking notes inline.

[gabotorresruiz]

HYDRATE_DASHBOARD keeps semanticDatasetRequests when the dashboard id is unchanged, which leaves one case the generations do not cover: a same-dashboard remount with an add's /fetch_datasource_metadata still in flight. That sourceKey is then in pageModifiedKeys, so the freshly loaded page metadata for it is discarded, and the pre-remount response still matches at line 222 and becomes the published snapshot. Checking it against the reducer, the published is_dttm is the pre-remount one, which would drop the grain if the view's schema changed in between.

Not a blocker: the window is one request long, and master froze the snapshot at load time anyway. But would clearing this even for a matching id be safe? A remount's page load is strictly newer than anything issued before it, and dropping the request just leaves the source unproven, which retains the grain. Or is there a case where the pre-remount response is the one you want?

[mikebridge]

Addressed in 79f15510bd: hydration clears the per-source add request map even when the dashboard id matches, so a pre-remount response arriving afterward is rejected. Red-first regressions cover both page/add completion orders after hydration and confirm that fresh temporal metadata wins. There is still a nonblocking edge where page metadata arrives before hydration and stays unproven until another refresh; that retains the saved grain, and I have recorded it for a focused lifecycle follow-up.

[gabotorresruiz]

Small thing: the source-key pattern hardcodes semantic_view while the next line uses DatasourceType.SemanticView. If that enum value ever moves, this stops matching silently and every grain becomes unproven rather than failing loudly. Worth hoisting it so there is one source of truth?

const SEMANTIC_SOURCE_KEY_RE = new RegExp(
  `^([1-9]\\d*)__${DatasourceType.SemanticView}$`,
);

[mikebridge]

Addressed in 79f15510bd: the hoisted source-key regexp uses DatasourceType.SemanticView, keeping one source of truth and preserving the existing positive-integer key format. The focused metadata-fetch regressions pass.

[gabotorresruiz]

REPLACE tolerates state.id === undefined here, while UPDATE requires an exact match at line 214. Any reason for the asymmetry? Let's align them unless the looser form is load-bearing for some early dispatch.

[mikebridge]

Addressed in 79f15510bd: REPLACE requires the same exact dashboard-id match as UPDATE. The page effect already waits for the hydrated dashboard id, so the looser guard was not needed for initial loading. A red-first test pins rejection before hydration, and the existing first-load/navigation page tests pass.