feat: Chart query last run timestamp #36934
Conversation
luizotavio32
force-pushed
the
chart-query-last-run-timestamp
branch
from
e5f1654 to
2592b1c
Compare
luizotavio32
changed the title
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #36934 +/- ##
===========================================
+ Coverage 0 67.93% +67.93%
===========================================
Files 0 642 +642
Lines 0 47898 +47898
Branches 0 5227 +5227
===========================================
+ Hits 0 32538 +32538
- Misses 0 14079 +14079
- Partials 0 1281 +1281
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #7e0279
Actionable Suggestions - 2
-
superset-frontend/src/components/LastQueriedLabel/index.tsx - 1
- Missing date validation · Line 31-38
-
superset/common/utils/query_cache_manager.py - 1
- Incorrect cache persistence for queried_dttm · Line 114-114
Additional Suggestions - 3
-
superset/charts/schemas.py - 1
-
Required schema field may lack implementation · Line 1467-1473The added queried_dttm field is marked as required=True in the marshmallow schema, meaning it must be present in the response dict during serialization. Since the frontend TypeScript interface already expects this field and test data includes it, the backend code probably sets it—but please confirm the response-building logic includes it to avoid ValidationError. If it's not always available, update the schema to required=False for consistency.
-
-
superset/common/query_context_processor.py - 1
-
Inconsistent queried_dttm semantics · Line 184-184The addition of queried_dttm to the response payload aligns with the schema and frontend expectations, but there's an inconsistency: for fresh queries, it correctly shows the execution time, while for cached results, it shows the cache timestamp instead. This could mislead users about when the query was actually run. Consider updating the caching logic to store and retrieve the original queried_dttm.
-
-
superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.jsx - 1
-
Hardcoded height constant risk · Line 92-92The QUERIED_LABEL_HEIGHT constant is hardcoded to 24, but the LastQueriedLabel component's height depends on theme-based styling (fontSizeSM and sizeUnit), which may not always result in exactly 24px. This could lead to slight height miscalculations in getChartHeight, though Math.max ensures a minimum chart height of 20px.
-
Review Details
-
Files reviewed - 8 · Commit Range:
0232349..2592b1c- superset-frontend/packages/superset-ui-core/src/query/types/QueryResponse.ts
- superset-frontend/plugins/plugin-chart-table/test/testData.ts
- superset-frontend/src/components/LastQueriedLabel/index.tsx
- superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.jsx
- superset-frontend/src/explore/components/ChartPills.tsx
- superset/charts/schemas.py
- superset/common/query_context_processor.py
- superset/common/utils/query_cache_manager.py
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- Eslint (Linter) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at [email protected].
Documentation & Help
AI Code Review powered by
superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.jsx
Outdated
Show resolved
Hide resolved
michael-s-molina
requested review from
geido,
kasiazjc,
kgabryje,
msyavuz,
rusackas and
yousoph
Code Review Agent Run #830853Actionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Thanks @michael-s-molina for feedback 🙏 @luizotavio32 My first thought was to actually add it to dashboard properties, but I think it makes much more sense to add it in the dashboard "more" dropdown, since this is already where users manage auto-refresh, force refresh etc, so it's kept in context. I think it could look like this: This could also have a tooltip, but if you think the label is enough I am good with that. If we decide on including tooltip though, we could do something like Displays the In addition, I’ve been thinking about a couple of lightweight enhancements that build on existing patterns:
Wdyt @luizotavio32 @michael-s-molina ? |
|
@kasiazjc I love the idea of the improved tooltips 😍 Regarding the location of the option, there's a fundamental difference with Refresh/Full Screen/Auto interval: these are individual settings for users that are persistent only to the current browser session. I think the timestamp is more of a persistent and dashboard configuration (with permission) setting like the color scheme. My suggestion would be to put this setting under Styling or General Information. 
|
That makes sense to me! For some reason thought auto interval is dashboard level setting, not user one. In that case I think I would put it under styling - above CSS. Side note - I didn't realize this section was being called Styling and to me it doesn't sound right. I think this is because it relates more to purely visual/aesthetic changes if that makes sense (sounds shallow and CSS/timestamp/sound more deep). I would change the title to Customize (which I had in designs, but must have missed the discussion), especially because we use it in explore too in the tabs for visual changes. What do you think? (might be me, so not pushing :D) |
|
Seems to me there're 2 clear next steps:
About:
If you're ok with it, I'd leave it for another PR since it's a change more related to dashboards instead of charts. @kasiazjc @michael-s-molina what do you think? |
… Force Refresh on Chart's header controls
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
My question here is if people need to see the full timestamp all the time, of if we should make the chart display a tooltip that appears on an ℹ icon in the chart header. There, we could have the tooltip show the update time, but potentially add other details too (chart certification, author/owner, datasets used... ) |
Code Review Agent Run #7d69f6Actionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Great question @rusackas. I asked AI to analyze both options and also compare with what other BI tools do and the recommendation was:
It's worth noting that many BI tools display the timestamp just once in the dashboard header, but since we allow individual chart refreshes, that approach may not suit our needs. Additionally, with plans to fully virtualize dashboards—where chart queries are triggered only when charts become visible—each chart could have a different timestamp, making per-chart display even more relevant. |
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Code Review Agent Run #57b113Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Code Review Agent Run #203720Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
| "annotation_data": self.annotation_data, | ||
| "sql_rowcount": self.sql_rowcount, | ||
| "queried_dttm": self.queried_dttm, | ||
| "dttm": self.queried_dttm, # Backwards compatibility |
There was a problem hiding this comment.
It was a codent suggestion #36934 (comment)
michael-s-molina
left a comment
michael-s-molina
left a comment
There was a problem hiding this comment.
Thank you for the PR @luizotavio32 and thanks @kasiazjc for the inputs!
|
Bito Automatic Review Skipped – PR Already Merged |
(cherry picked from commit 1e8d648)
(cherry picked from commit 1e8d648)
4 participants
SUMMARY
Adds a "Last queried at" timestamp to charts, showing users when the underlying query was last executed. This provides visibility into data freshness, helping users understand whether they're viewing cached results and when the data was last fetched from the database.
Dashboard-level configuration: The timestamp visibility can be toggled on/off per dashboard via
Dashboard Properties → Styling → "Show chart query timestamps". This setting is off by default and only affects charts in the dashboard view, chart explore view and Force Refresh tooltip will still be shown.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Dashboard View -
Show chart query timestampsmust be ON:Chart Explore View:
Chart's
Force Refreshtooltip:Setting up dashboard configuration:
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION