You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "mikebridge (via GitHub)" <gi...@apache.org> on 2026/09/12 18:11:28 UTC
[PR] feat(semantic-layer): show provider queries from chart results [superset]
mikebridge opened a new pull request, #44206:
URL: https://github.com/apache/superset/pull/44206
### SUMMARY
Show semantic-view provider requests in **View query** using the chart's existing run report, in both Explore and the dashboard chart menu.
- Display and copy the complete provider request text verbatim, including host-authored kind headers and multiple request blocks.
- Use SQL/JSON highlighting as a presentation hint, with plain text for unknown kinds.
- Show distinct messages when the chart has not run or no request text is available for the run, including redacted reports.
- Do not fetch or execute anything when opening View query for a semantic view. Regular dataset behavior is unchanged.
- Replace the semantic-view compile-only placeholder with a translated validation error through the existing chart-data response envelope.
- Add tests, translation catalog entries, and user documentation. No new endpoints, provider interfaces, migrations, or dependencies.
Pre-execution preview is out of scope: providers report these requests with chart results. This PR reads that existing report rather than introducing a compile-only provider API.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before: semantic-view View query displays `Not implemented for semantic layers`.
After: it displays copyable provider request text from the chart's run, or the appropriate empty-state message.
Screenshots and live-provider verification are pending. This PR is intentionally draft for manual review.
### TESTING INSTRUCTIONS
Automated checks completed locally:
- `npm run test -- ViewQueryModal.test.tsx SemanticRequestView.test.tsx --maxWorkers=2`: 27 tests passed, including the five unchanged existing dataset cases.
- `pytest tests/unit_tests/semantic_layers/models_test.py -q`: 101 tests passed.
- `pre-commit run --files <branch-changed files>`: all applicable hooks passed, including MyPy and frontend type checking; no repository-wide run.
- Source-placeholder absence is checked by an automated regression test.
Manual checks still required:
1. With `SEMANTIC_LAYERS` enabled, run a chart backed by a SQL-producing provider and open **View query** from Explore and a dashboard chart menu.
2. Compare displayed/copied text with the provider's reported request. Check complete headers, whitespace, multiple requests, and long-text scrolling.
3. Rerun with different filters and confirm the request updates with the chart's results.
4. Repeat for a non-SQL provider; unknown kinds must remain readable and copyable.
5. Check the distinct not-run and no-request empty states, and confirm opening the modal adds no chart-data requests.
6. Confirm regular dataset query fetching, formatting, and SQL Lab affordances remain unchanged.
No live-provider or OS-clipboard pass is claimed. CI results will be evaluated separately.
### ADDITIONAL INFORMATION
- [x] Has associated issue: [SC-104912](https://app.shortcut.com/preset/story/104912)
- [x] Required feature flags: `SEMANTIC_LAYERS` (existing flag)
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [x] Introduces new feature or API: provider-request display; no new 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(semantic-layer): show provider queries from chart results [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #44206:
URL: https://github.com/apache/superset/pull/44206#issuecomment-5647756661
### <span aria-hidden="true">โ
</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">๐จ</span> Latest commit | 073f1fd8eea8e4cc216b9faa75b7f7983a2dcadb |
|<span aria-hidden="true">๐</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6aa595d07345c300089529f7 |
|<span aria-hidden="true">๐</span> Deploy Preview | [https://deploy-preview-44206--superset-docs-preview.netlify.app](https://deploy-preview-44206--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#DcpLCoAgFEDRvbxxwkszsx20DFMjwV8aRUR7z8nlDs4LuaSQT5gBOliLinpvXzXpcZA9JdUGFU-nyeXsTeqeWg7frLHZp2cxTY9KcckNCjZwzRBxkpzKTcD3Aw) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-44206--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(semantic-layer): show provider queries from chart results [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44206:
URL: https://github.com/apache/superset/pull/44206#issuecomment-5741223395
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers an incremental AI Review.
- `/review full` - 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 <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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(semantic-layer): show provider queries from chart results [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #44206:
URL: https://github.com/apache/superset/pull/44206#discussion_r4053075947
##########
superset-frontend/src/explore/components/controls/ViewQueryModal.tsx:
##########
@@ -76,22 +103,51 @@ const ViewQueryModal: FC<Props> = ({ latestQueryFormData, ownState }) => {
setError(null);
})
.catch(response => {
- getClientErrorObject(response).then(({ error, message }) => {
- setError(
- error ||
- message ||
- response.statusText ||
- t('Sorry, An error occurred'),
- );
- setIsLoading(false);
- });
+ getClientErrorObject(response).then(
+ ({ error, message }: ClientErrorObject) => {
+ setError(
+ error ||
+ message ||
+ response.statusText ||
+ t('Sorry, An error occurred'),
+ );
+ setIsLoading(false);
+ },
+ );
});
},
[latestQueryFormData, ownState],
);
useEffect(() => {
- loadChartData('query');
- }, [loadChartData]);
+ if (!isSemanticView) {
+ loadChartData('query');
+ }
+ }, [isSemanticView, loadChartData]);
+
+ if (isSemanticView) {
+ const reportState = getSemanticReportState(queriesResponse);
+ const noRequestMessage = t('No provider query is available for this run.');
+ return (
+ <ViewQueryModalContainer>
+ {reportState === 'not-run' ? (
+ <Alert
+ type="info"
+ message={t(
+ 'The provider query will be available after the chart runs.',
+ )}
+ />
+ ) : (
+ queriesResponse?.map((entry, index) =>
+ entry.query ? (
+ <SemanticRequestView key={index} requestText={entry.query} />
+ ) : (
+ <Alert key={index} type="info" message={noRequestMessage} />
+ ),
+ )
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Chart errors masked as info</b></div>
<div id="fix">
For semantic views, `queriesResponse` comes from redux `state.charts[...].queriesResponse`, which `chartUpdateFailed` (src/components/Chart/chartAction.ts) populates with error objects that have no `query` field. Those entries fall into the `entry.query ? ... : <Alert type="info" noRequestMessage>` branch, so a failed chart shows "No provider query is available for this run." and hides the real backend error. Render `entry.error` as an error Alert when present.
</div>
</div>
<small><i>Code Review Run #4cac80</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/semantic_layers/models.py:
##########
@@ -378,7 +378,13 @@ def get_query_result(self, query_object: QueryObject) -> QueryResult:
return result
def get_query_str(self, query_obj: QueryObjectDict) -> str:
- return "Not implemented for semantic layers"
+ """Reject previews because provider queries are returned with chart data."""
+ raise QueryObjectValidationError(
+ _(
+ "A semantic view's provider query is produced when the chart runs "
+ "and returned with its data."
+ )
+ )
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Untranslated error message</b></div>
<div id="fix">
This new user-facing error message has msgstr "" (untranslated) in all 30 superset/translations/*/LC_MESSAGES/messages.po files, so non-English users hit by the new `QueryObjectValidationError` from `SemanticView.get_query_str` will see English text. BITO rules 13343/12743 require complete translations before merge; populate the msgstr entries (e.g. via backfill_po.py) before release.
</div>
</div>
<small><i>Code Review Run #4cac80</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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(semantic-layer): show provider queries from chart results [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi merged PR #44206:
URL: https://github.com/apache/superset/pull/44206
--
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(semantic-layer): show provider queries from chart results [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #44206:
URL: https://github.com/apache/superset/pull/44206#issuecomment-6026123707
## CodeAnt PR Risk: Medium Risk
- The PR needs attention before merging because the semantic request panel may read a different chart-state key than the chart that produced the report.
- Entries with an error but no query are rendered as โNo provider query,โ which could conceal a chart validation or execution failure.
**Assessed commit:** `0ada91d42c7f`
--
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(semantic-layer): show provider queries from chart results [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44206:
URL: https://github.com/apache/superset/pull/44206#issuecomment-5647746262
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(semantic-layer): show provider queries from chart results [superset]
Posted by "mikebridge (via GitHub)" <gi...@apache.org>.
mikebridge commented on code in PR #44206:
URL: https://github.com/apache/superset/pull/44206#discussion_r4200912249
##########
superset-frontend/src/explore/components/controls/ViewQueryModal.tsx:
##########
@@ -44,14 +51,34 @@ type Result = {
error?: string;
};
+export function getSemanticReportState(
+ queriesResponse: readonly { query?: string }[] | null | undefined,
+): 'has-requests' | 'none-reported' | 'not-run' {
+ if (!queriesResponse?.length) {
+ return 'not-run';
+ }
+ return queriesResponse.some(entry => entry.query)
+ ? 'has-requests'
+ : 'none-reported';
+}
+
const ViewQueryModalContainer = styled.div`
height: 100%;
display: flex;
flex-direction: column;
- gap: ${({ theme }) => theme.sizeUnit * 4}px;
+ gap: ${({ theme }: { theme: SupersetTheme }) => theme.sizeUnit * 4}px;
`;
const ViewQueryModal: FC<Props> = ({ latestQueryFormData, ownState }) => {
+ const isSemanticView =
+ new DatasourceKey(latestQueryFormData.datasource).type ===
+ DatasourceType.SemanticView;
+ const queriesResponse = useSelector<
+ RootState,
+ ChartState['queriesResponse'] | undefined
+ >(
+ state => state.charts?.[latestQueryFormData.slice_id ?? 0]?.queriesResponse,
Review Comment:
Fixed in 3e2d2de2e4. Explore and Dashboard pass the owning chart ID explicitly, so View query no longer depends on `latestQueryFormData` carrying `slice_id`. The regressions open both real menus with `slice_id` absent and a different report under key zero; they show the owning chart's request instead.
##########
superset-frontend/src/explore/components/controls/ViewQueryModal.tsx:
##########
@@ -76,22 +103,51 @@ const ViewQueryModal: FC<Props> = ({ latestQueryFormData, ownState }) => {
setError(null);
})
.catch(response => {
- getClientErrorObject(response).then(({ error, message }) => {
- setError(
- error ||
- message ||
- response.statusText ||
- t('Sorry, An error occurred'),
- );
- setIsLoading(false);
- });
+ getClientErrorObject(response).then(
+ ({ error, message }: ClientErrorObject) => {
+ setError(
+ error ||
+ message ||
+ response.statusText ||
+ t('Sorry, An error occurred'),
+ );
+ setIsLoading(false);
+ },
+ );
});
},
[latestQueryFormData, ownState],
);
useEffect(() => {
- loadChartData('query');
- }, [loadChartData]);
+ if (!isSemanticView) {
+ loadChartData('query');
+ }
+ }, [isSemanticView, loadChartData]);
+
+ if (isSemanticView) {
+ const reportState = getSemanticReportState(queriesResponse);
+ const noRequestMessage = t('No provider query is available for this run.');
+ return (
+ <ViewQueryModalContainer>
+ {reportState === 'not-run' ? (
+ <Alert
+ type="info"
+ message={t(
+ 'The provider query will be available after the chart runs.',
+ )}
+ />
+ ) : (
+ queriesResponse?.map((entry, index) =>
+ entry.query ? (
+ <SemanticRequestView key={index} requestText={entry.query} />
+ ) : (
+ <Alert key={index} type="info" message={noRequestMessage} />
+ ),
Review Comment:
Fixed in 3e2d2de2e4. Error-bearing entries render the actual error, including alongside reported request text. A failed run without a response uses the chart's existing error message. Reducer-driven tests cover provider and network failures without displaying the previous successful request or the generic no-query message.
##########
superset-frontend/src/explore/components/controls/ViewQueryModal.tsx:
##########
@@ -76,22 +103,51 @@ const ViewQueryModal: FC<Props> = ({ latestQueryFormData, ownState }) => {
setError(null);
})
.catch(response => {
- getClientErrorObject(response).then(({ error, message }) => {
- setError(
- error ||
- message ||
- response.statusText ||
- t('Sorry, An error occurred'),
- );
- setIsLoading(false);
- });
+ getClientErrorObject(response).then(
+ ({ error, message }: ClientErrorObject) => {
+ setError(
+ error ||
+ message ||
+ response.statusText ||
+ t('Sorry, An error occurred'),
+ );
+ setIsLoading(false);
+ },
+ );
});
},
[latestQueryFormData, ownState],
);
useEffect(() => {
- loadChartData('query');
- }, [loadChartData]);
+ if (!isSemanticView) {
+ loadChartData('query');
+ }
+ }, [isSemanticView, loadChartData]);
+
+ if (isSemanticView) {
+ const reportState = getSemanticReportState(queriesResponse);
+ const noRequestMessage = t('No provider query is available for this run.');
+ return (
+ <ViewQueryModalContainer>
+ {reportState === 'not-run' ? (
+ <Alert
+ type="info"
+ message={t(
+ 'The provider query will be available after the chart runs.',
+ )}
+ />
+ ) : (
+ queriesResponse?.map((entry, index) =>
+ entry.query ? (
+ <SemanticRequestView key={index} requestText={entry.query} />
+ ) : (
+ <Alert key={index} type="info" message={noRequestMessage} />
+ ),
+ )
Review Comment:
Fixed in 3e2d2de2e4: semantic response errors render as error alerts. The regressions dispatch `CHART_UPDATE_FAILED` through the chart reducer and verify the actual error appears; mixed request/error responses preserve both in entry order.
##########
superset/semantic_layers/models.py:
##########
@@ -378,7 +378,13 @@ def get_query_result(self, query_object: QueryObject) -> QueryResult:
return result
def get_query_str(self, query_obj: QueryObjectDict) -> str:
- return "Not implemented for semantic layers"
+ """Reject previews because provider queries are returned with chart data."""
+ raise QueryObjectValidationError(
+ _(
+ "A semantic view's provider query is produced when the chart runs "
+ "and returned with its data."
+ )
+ )
Review Comment:
Leaving translation backfill unchanged in this round, as agreed with rusackas; the existing messages remain marked for translation.
--
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(semantic-layer): show provider queries from chart results [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 #44206:
URL: https://github.com/apache/superset/pull/44206#discussion_r4200913953
##########
superset-frontend/src/explore/components/controls/ViewQueryModal.tsx:
##########
@@ -44,14 +51,34 @@ type Result = {
error?: string;
};
+export function getSemanticReportState(
+ queriesResponse: readonly { query?: string }[] | null | undefined,
+): 'has-requests' | 'none-reported' | 'not-run' {
+ if (!queriesResponse?.length) {
+ return 'not-run';
+ }
+ return queriesResponse.some(entry => entry.query)
+ ? 'has-requests'
+ : 'none-reported';
+}
+
const ViewQueryModalContainer = styled.div`
height: 100%;
display: flex;
flex-direction: column;
- gap: ${({ theme }) => theme.sizeUnit * 4}px;
+ gap: ${({ theme }: { theme: SupersetTheme }) => theme.sizeUnit * 4}px;
`;
const ViewQueryModal: FC<Props> = ({ latestQueryFormData, ownState }) => {
+ const isSemanticView =
+ new DatasourceKey(latestQueryFormData.datasource).type ===
+ DatasourceType.SemanticView;
+ const queriesResponse = useSelector<
+ RootState,
+ ChartState['queriesResponse'] | undefined
+ >(
+ state => state.charts?.[latestQueryFormData.slice_id ?? 0]?.queriesResponse,
Review Comment:
โ
**CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved** as of `3e2d2de`.
The chart-state selector now prefers the supplied `chartId` before falling back to `latestQueryFormData.slice_id` or `0`.
<sub>If that's not right, unresolve this thread and CodeAnt will leave it open.</sub>
<!-- codeant-auto-resolve-reply -->
##########
superset-frontend/src/explore/components/controls/ViewQueryModal.tsx:
##########
@@ -76,22 +103,51 @@ const ViewQueryModal: FC<Props> = ({ latestQueryFormData, ownState }) => {
setError(null);
})
.catch(response => {
- getClientErrorObject(response).then(({ error, message }) => {
- setError(
- error ||
- message ||
- response.statusText ||
- t('Sorry, An error occurred'),
- );
- setIsLoading(false);
- });
+ getClientErrorObject(response).then(
+ ({ error, message }: ClientErrorObject) => {
+ setError(
+ error ||
+ message ||
+ response.statusText ||
+ t('Sorry, An error occurred'),
+ );
+ setIsLoading(false);
+ },
+ );
});
},
[latestQueryFormData, ownState],
);
useEffect(() => {
- loadChartData('query');
- }, [loadChartData]);
+ if (!isSemanticView) {
+ loadChartData('query');
+ }
+ }, [isSemanticView, loadChartData]);
+
+ if (isSemanticView) {
+ const reportState = getSemanticReportState(queriesResponse);
+ const noRequestMessage = t('No provider query is available for this run.');
+ return (
+ <ViewQueryModalContainer>
+ {reportState === 'not-run' ? (
+ <Alert
+ type="info"
+ message={t(
+ 'The provider query will be available after the chart runs.',
+ )}
+ />
+ ) : (
+ queriesResponse?.map((entry, index) =>
+ entry.query ? (
+ <SemanticRequestView key={index} requestText={entry.query} />
+ ) : (
+ <Alert key={index} type="info" message={noRequestMessage} />
+ ),
Review Comment:
โ
**CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved** as of `3e2d2de`.
Entries with an `error` now render an error alert, and the no-provider-query fallback is shown only when an entry has neither an error nor a query.
<sub>If that's not right, unresolve this thread and CodeAnt will leave it open.</sub>
<!-- codeant-auto-resolve-reply -->
--
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(semantic-layer): show provider queries from chart results [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 #44206:
URL: https://github.com/apache/superset/pull/44206#discussion_r4052606718
##########
superset-frontend/src/explore/components/controls/ViewQueryModal.tsx:
##########
@@ -44,14 +51,34 @@ type Result = {
error?: string;
};
+export function getSemanticReportState(
+ queriesResponse: readonly { query?: string }[] | null | undefined,
+): 'has-requests' | 'none-reported' | 'not-run' {
+ if (!queriesResponse?.length) {
+ return 'not-run';
+ }
+ return queriesResponse.some(entry => entry.query)
+ ? 'has-requests'
+ : 'none-reported';
+}
+
const ViewQueryModalContainer = styled.div`
height: 100%;
display: flex;
flex-direction: column;
- gap: ${({ theme }) => theme.sizeUnit * 4}px;
+ gap: ${({ theme }: { theme: SupersetTheme }) => theme.sizeUnit * 4}px;
`;
const ViewQueryModal: FC<Props> = ({ latestQueryFormData, ownState }) => {
+ const isSemanticView =
+ new DatasourceKey(latestQueryFormData.datasource).type ===
+ DatasourceType.SemanticView;
+ const queriesResponse = useSelector<
+ RootState,
+ ChartState['queriesResponse'] | undefined
+ >(
+ state => state.charts?.[latestQueryFormData.slice_id ?? 0]?.queriesResponse,
Review Comment:
**Suggestion:** Explore's chart state is keyed by `getChartKey`, but `latestQueryFormData` usually lacks `slice_id`; saved charts therefore read key `0` and show no provider request.
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes` ยท ๐ท๏ธ `Stale reference`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=24657b4c19c14744a22e2ba3fcb4833d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=24657b4c19c14744a22e2ba3fcb4833d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/explore/components/controls/ViewQueryModal.tsx
**Line:** 78:80
**Comment:**
*Stale Reference: Explore's chart state is keyed by `getChartKey`, but `latestQueryFormData` usually lacks `slice_id`; saved charts therefore read key `0` and show no provider request.
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%2F44206&comment_hash=c42457e3d70c8c00960ca39a149a0c434c4d0bed81ceda7ecc4c1722027ae766&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44206&comment_hash=c42457e3d70c8c00960ca39a149a0c434c4d0bed81ceda7ecc4c1722027ae766&reaction=dislike'>๐</a>
##########
superset-frontend/src/explore/components/controls/ViewQueryModal.tsx:
##########
@@ -76,22 +103,51 @@ const ViewQueryModal: FC<Props> = ({ latestQueryFormData, ownState }) => {
setError(null);
})
.catch(response => {
- getClientErrorObject(response).then(({ error, message }) => {
- setError(
- error ||
- message ||
- response.statusText ||
- t('Sorry, An error occurred'),
- );
- setIsLoading(false);
- });
+ getClientErrorObject(response).then(
+ ({ error, message }: ClientErrorObject) => {
+ setError(
+ error ||
+ message ||
+ response.statusText ||
+ t('Sorry, An error occurred'),
+ );
+ setIsLoading(false);
+ },
+ );
});
},
[latestQueryFormData, ownState],
);
useEffect(() => {
- loadChartData('query');
- }, [loadChartData]);
+ if (!isSemanticView) {
+ loadChartData('query');
+ }
+ }, [isSemanticView, loadChartData]);
+
+ if (isSemanticView) {
+ const reportState = getSemanticReportState(queriesResponse);
+ const noRequestMessage = t('No provider query is available for this run.');
+ return (
+ <ViewQueryModalContainer>
+ {reportState === 'not-run' ? (
+ <Alert
+ type="info"
+ message={t(
+ 'The provider query will be available after the chart runs.',
+ )}
+ />
+ ) : (
+ queriesResponse?.map((entry, index) =>
+ entry.query ? (
+ <SemanticRequestView key={index} requestText={entry.query} />
+ ) : (
+ <Alert key={index} type="info" message={noRequestMessage} />
+ ),
Review Comment:
**Suggestion:** Entries containing only an `error` are rendered as โNo provider query,โ hiding chart validation or execution errors from the View query dialog.
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes` ยท ๐ท๏ธ `Logic error`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=fca1b1e3beb84a1dadbbb5e36fce552f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=fca1b1e3beb84a1dadbbb5e36fce552f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/explore/components/controls/ViewQueryModal.tsx
**Line:** 140:145
**Comment:**
*Logic Error: Entries containing only an `error` are rendered as โNo provider query,โ hiding chart validation or execution errors from the View query dialog.
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%2F44206&comment_hash=0e5f8bc03d42222adc9fc776b15a945439c9384d740b31ccfab9e88edf2b8acc&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44206&comment_hash=0e5f8bc03d42222adc9fc776b15a945439c9384d740b31ccfab9e88edf2b8acc&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(semantic-layer): show provider queries from chart results [superset]
Posted by "mikebridge (via GitHub)" <gi...@apache.org>.
mikebridge commented on PR #44206:
URL: https://github.com/apache/superset/pull/44206#issuecomment-6026361625
@rusackas thanks for the pass. Both points are fixed in 3e2d2de2e4 (merged with current master first):
- **Saved charts:** Explore and Dashboard pass the owning chart ID explicitly, so View query no longer depends on `latestQueryFormData` carrying `slice_id`.
- **Failed charts:** a failed run now shows the chart's actual error instead of "No provider query is available for this run", including when a provider returns both a request and an error.
The translation thread is left as you suggested.
--
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(semantic-layer): show provider queries from chart results [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #44206:
URL: https://github.com/apache/superset/pull/44206#issuecomment-5647776147
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/44206?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 `1 line` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 57.44%. Comparing base ([`3e7bdec`](https://app.codecov.io/gh/apache/superset/commit/3e7bdec53bb7830b524a4b2b53478332b00d9c72?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`073f1fd`](https://app.codecov.io/gh/apache/superset/commit/073f1fd8eea8e4cc216b9faa75b7f7983a2dcadb?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 9 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/44206?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/semantic\_layers/models.py](https://app.codecov.io/gh/apache/superset/pull/44206?src=pr&el=tree&filepath=superset%2Fsemantic_layers%2Fmodels.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvc2VtYW50aWNfbGF5ZXJzL21vZGVscy5weQ==) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/44206?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (3e7bdec) and HEAD (073f1fd). Click for more details.
>
> <details><summary>HEAD has 124 uploads less than BASE</summary>
>
>| Flag | BASE (3e7bdec) | HEAD (073f1fd) |
>|------|------|------|
>|python|96|2|
>|presto|16|1|
>|hive|16|1|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #44206 +/- ##
===========================================
- Coverage 80.16% 57.44% -22.73%
===========================================
Files 2925 2925
Lines 172563 172644 +81
Branches 40058 40077 +19
===========================================
- Hits 138339 99167 -39172
- Misses 31627 72537 +40910
+ Partials 2597 940 -1657
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/44206/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/44206/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `37.33% <0.00%> (-0.01%)` | :arrow_down: |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/44206/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/44206/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/44206/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.22% <0.00%> (-0.01%)` | :arrow_down: |
| [python](https://app.codecov.io/gh/apache/superset/pull/44206/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.27% <0.00%> (-45.29%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/44206/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/44206/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
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 Harness](https://app.codecov.io/gh/apache/superset/pull/44206?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(semantic-layer): show provider queries from chart results [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #44206:
URL: https://github.com/apache/superset/pull/44206#issuecomment-5943407483
Thanks @mikebridge, this is looking good, and it's nice that it's all driven off the existing run report with no new endpoints.
I'd love another pass on a couple of the open bot threads before this gets merged. Codeant's point that `getChartKey` needs a `slice_id` that `latestQueryFormData` usually doesn't carry seems valid, since saved charts would read key `0` and show nothing. Bito and Codeant are also both right that a failed chart ends up with "No provider query is available for this run" instead of its actual error. Could you take a look and resolve those? The translation thread is fine to ignore, empty `msgstr` is expected for new strings.
--
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