Serve dashboard specs in the official UI#264
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d79247788
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| dimensions: dimensions.length ? dimensions : model.dimensions, | ||
| selectedMetric, | ||
| grain: timeRef ? grainForRef(timeRef, fallbackGrain) : fallbackGrain, | ||
| filters: chart.query.filters ?? [], |
There was a problem hiding this comment.
For dashboard specs that declare query.segments, the validator/backend already accept and apply those segment refs, but this config only carries query.filters into the official UI. Since the Explorer queries are built from configured.filters, any dashboard served with segment-scoped charts will silently drop those constraints and show unsegmented totals/leaderboards.
Useful? React with 👍 / 👎.
| const encodedMetric = chart.encoding?.y; | ||
| const selectedMetric = | ||
| (encodedMetric && metrics.some((metric) => metric.ref === encodedMetric) ? encodedMetric : undefined) ?? |
There was a problem hiding this comment.
Valid dashboard specs can use encoding.y as an array of metrics, and the existing dashboard builder treats that array as the ordered y-metric list. In that case encodedMetric is an array here, so comparing it directly to metric.ref always fails and the UI falls back to metrics[0]; dashboards whose y encoding intentionally selects a different first metric open/rank by the wrong metric unless query.metrics happens to be in the same order.
Useful? React with 👍 / 👎.
2f1c858 to
aec6a5b
Compare
2c82864 to
a701a2d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a3a71d75e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| view: "home", | ||
| // A configured dashboard opens directly; otherwise land on the model index. | ||
| view: configured ? "explore" : "home", | ||
| dashboardTab: configured?.id, |
There was a problem hiding this comment.
When a dashboard has multiple tabs, initial is always derived from dashboardTabConfig(catalog, dashboard) without a tab id, so it stores the first tab here. The toolbar Reset button dispatches reset with this fixed initial; after a user selects tab B and applies a date/filter, Reset clears controls by jumping back to tab A instead of staying on tab B, so the selected dashboard context is lost.
Useful? React with 👍 / 👎.
| } from "../data/types"; | ||
| import { graphMetricsForModel } from "./catalog"; | ||
|
|
||
| const GRAINS = new Set<Grain>(["hour", "day", "week", "month", "quarter", "year"]); |
There was a problem hiding this comment.
The dashboard schema/type generator accepts time dimensions at second and minute granularity, but this set makes grainForRef() reject those refs and fall back to the model default. For a served dashboard with encoding.x: orders.created_at__minute or __second, the canonical UI will query the wrong bucket size (for example month/day), even though dashboard validate accepts the spec.
Useful? React with 👍 / 👎.
6a3a71d to
3d249f3
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d249f33a6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| selectedMetric: configured.selectedMetric, | ||
| grain: configured.grain, |
There was a problem hiding this comment.
When a dashboard user selects a different KPI card or time grain and then uses Copy link or refreshes, decodeState() has already parsed ?metric= and ?grain=, but these assignments overwrite both values with the tab's configured defaults. The URL is still written with the user's selections, so shared dashboard links reopen on the original encoded metric/grain instead of the view the user copied.
Useful? React with 👍 / 👎.
| const baseFilters = useMemo( | ||
| () => composeFilters(state.filters, { timeRef, range: state.dateRange, types }), | ||
| [state.filters, timeRef, state.dateRange, types], | ||
| () => [...(configured?.filters ?? []), ...composeFilters(state.filters, { timeRef, range: state.dateRange, types })], |
There was a problem hiding this comment.
This only injects configured.filters into the Explore totals/series path; dashboard mode still exposes the filter editor and row-preview drawer, and those components build their queries from composeFilters(state.filters, ...) without the dashboard spec filters. For a dashboard scoped with query.filters such as a tenant or region predicate, opening Rows preview or the Add filter value list can show unscoped rows/values outside the dashboard's configured slice.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 529ab8c3a1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| metrics: metrics.length ? metrics : model.metrics, | ||
| dimensions: dimensions.length ? dimensions : model.dimensions, | ||
| selectedMetric, | ||
| grain: timeRef ? grainForRef(timeRef, fallbackGrain) : fallbackGrain, |
There was a problem hiding this comment.
When a valid dashboard encodes a non-default time dimension, such as orders.shipped_at__month on a model whose default timeDimension is orders.created_at, this resolves that ref only to extract the grain and then discards it. ExplorerView still builds the series and date-range filters from model.timeDimension, so the served dashboard queries the wrong timestamp while appearing to honor the spec's encoding.x.
Useful? React with 👍 / 👎.
| filters: chart.query.filters ?? [], | ||
| segments: chart.query.segments ?? [], |
There was a problem hiding this comment.
Dashboard specs can declare query.order_by/orderBy and query.limit, and the validator compiles those fields, but the official UI config only carries filters and segments out of chart.query. In any dashboard where those fields define the intended ranking or row window, the served UI silently falls back to Explore defaults like metric-desc leaderboards and fixed limits instead of the validated spec.
Useful? React with 👍 / 👎.
529ab8c to
ca0b56b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca0b56bc7a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const modelName = | ||
| dimensionRefs.find((ref) => ref.includes("."))?.split(".")[0] ?? | ||
| metricRefs.find((ref) => ref.includes("."))?.split(".")[0]; | ||
| const model = catalog.models.find((candidate) => candidate.name === modelName) ?? catalog.models[0]; |
There was a problem hiding this comment.
When a valid dashboard chart contains only a graph-level metric (a bare ref such as gross_margin_rate) and no model-qualified dimensions/metrics, modelName is undefined and this falls back to catalog.models[0]. Because graphMetricsForModel() filters graph metrics by ownerModel, if the first model is not the metric's owner or joinable model the requested metric is dropped and selectedMetric falls through to the first model metric, so the served dashboard renders the wrong KPI. Consider resolving the model from a matching graph metric's ownerModel before using the first catalog model.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f235e6068
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| filters: chart.query.filters ?? [], | ||
| segments: chart.query.segments ?? [], |
There was a problem hiding this comment.
When a dashboard chart sets query.use_preaggregations: false / usePreaggregations: false, the validator and legacy chart builder honor that flag, but the canonical UI config produced here only carries filters and segments. The Explore query builders therefore omit any per-dashboard override, and HttpBackend injects its adapter default usePreaggregations: true into every totals/series/leaderboard request, so dashboards that deliberately opt out because rollups are stale or not appropriate can render rollup-backed results instead of the validated base-table query.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea8fbe35e2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| resolved_connection = _resolve_connection(connection=connection, database=db, models=directory) | ||
| connection_str = resolved_connection.connection if resolved_connection else None | ||
| init_sql = resolved_connection.init_sql if resolved_connection else None |
There was a problem hiding this comment.
When --demo is used from a project that has only conventional data/*.duckdb discovery and no explicit DB option, this still resolves the project connection and picks that warehouse even though directory has been switched to the packaged demo models. The later demo population writes customers, products, and orders into the user's project database instead of an isolated in-memory demo DB, or fails if those tables already exist. Skip project connection resolution in demo mode unless --db/--connection is explicitly passed; the same pattern also affects the postgres demo server path.
Useful? React with 👍 / 👎.
| const availableMetrics = [...model.metrics, ...graphMetricsForModel(catalog, model.name)]; | ||
| const metrics = metricRefs | ||
| .map((ref) => availableMetrics.find((metric) => metric.ref === ref)) | ||
| .filter((metric): metric is CatalogMetric => Boolean(metric)); |
There was a problem hiding this comment.
When a validated dashboard groups a metric from one model by a dimension from a related model, such as orders.revenue by customers.country, modelName is chosen from the first dimension and this list then contains only that model's metrics plus graph metrics. The requested model metric is filtered out and the UI falls back to another metric or an empty state, so dashboard serve can render a different KPI than the spec even though the backend compile path supports the cross-model query. Include model metrics from all query refs or avoid discarding joinable metric refs when deriving the tab config.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Line 132 in d244ced
Preserve fallback for auto engine configs
When a config contains runtime.engine: auto but omits runtime.fallback, RuntimeConfig supplies its default False, and this assignment treats that default as an explicit no-fallback choice. _configure_engine_environment then sets SIDEMANTIC_RS_NO_FALLBACK=1, so commands that should try Rust and fall back to Python instead fail in environments without sidemantic-rs or for Rust-unsupported queries. Keep the auto fallback default enabled unless the user explicitly opts out.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if db_files: | ||
| connection_str = f"duckdb:///{db_files[0].absolute()}" | ||
| models = _models_path(models) | ||
| resolved_connection = _resolve_connection(connection=connection, database=db, models=models) |
There was a problem hiding this comment.
Because _load_query_layer is now used by compile-only commands such as rewrite and explain, this unconditional connection discovery makes those commands fail when a project has multiple conventional data/*.db or data/*.duckdb files. For example, sidemantic rewrite 'SELECT order_count FROM orders' exits with “Multiple databases found” even though rewriting does not execute a query and can safely use the default DuckDB dialect when no DB was explicitly selected.
Useful? React with 👍 / 👎.
d244ced to
6f1185d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f1185d04c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!model) return <div className="p-4"><EmptyState message="No model available in this semantic layer." /></div>; | ||
|
|
||
| const leaderboardDims = model.dimensions.filter((dim) => dim.type !== "time"); | ||
| const leaderboardDims = (configured?.dimensions ?? model.dimensions).filter((dim) => dim.type !== "time"); |
There was a problem hiding this comment.
When a dashboard has multiple tabs with different categorical dimensions, expanding a leaderboard on tab A and then switching to tab B leaves expandedLeaderboard set to tab A's dimension. This per-tab dimension list then changes, but the filter below still requires the stale ref, so tab B can render the empty “No categorical dimensions” state with no visible way to collapse back to its own leaderboards. Clear or remount the expanded state when state.dashboardTab/model changes.
Useful? React with 👍 / 👎.
6f1185d to
207faef
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 207faefcbb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .map((ref) => availableMetrics.find((metric) => metric.ref === ref)) | ||
| .filter((metric): metric is CatalogMetric => Boolean(metric)); | ||
| const dimensions = dimensionRefs | ||
| .map((ref) => dimensionForRef(model, ref)) |
There was a problem hiding this comment.
For dashboards that compile across related models, this resolves every query.dimensions ref only against the single model chosen above, so any valid dimension from another joined model is dropped from configured.dimensions. For example, a chart with metrics: [orders.revenue] and dimensions: [orders.created_at__month, customers.country] can validate/compile through the semantic graph, but the served dashboard will omit customers.country from its leaderboards/filter surface and no longer reflects the spec's grouping.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 328d6700f7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| grain: timeRef ? grainForRef(timeRef, fallbackGrain) : fallbackGrain, | ||
| filters: chart.query.filters ?? [], | ||
| segments: chart.query.segments ?? [], | ||
| usePreaggregations: chart.query.use_preaggregations ?? chart.query.usePreaggregations, |
There was a problem hiding this comment.
When a dashboard puts use_preaggregations: false / usePreaggregations: false under defaults.query instead of repeating it on each chart, this leaves configured.usePreaggregations undefined. The HTTP adapter then applies its default usePreaggregations: true to the UI's totals/series/leaderboard requests, so dashboards that validate and intend to stay on base tables can still render rollup-backed results unless every chart repeats the opt-out.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4783ddcdc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const isHome = state.view === "home"; | ||
| const { state, dispatch, catalog, initial, dashboard } = useExplorer(); | ||
| const isDashboard = Boolean(dashboard); | ||
| const isHome = state.view === "home" && !isDashboard; |
There was a problem hiding this comment.
When a dashboard is loaded, the brand button still dispatches setView to home, but this new guard keeps isHome false for dashboards while the content branch still renders ExploreIndexView. If a dashboard user clicks the title and then opens any model card, setModel does not clear dashboardTab, so ExplorerView combines the dashboard tab's configured metrics/dimensions with the arbitrary model's time/filter metadata and can issue invalid or mis-scoped queries. In dashboard mode, either disable that home navigation or clear the dashboard context when leaving the configured tab flow.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21593ddc11
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const dimensionRefs = chart.query.dimensions ?? []; | ||
| const metricRefs = chart.query.metrics ?? []; | ||
| const graphMetricOwner = metricRefs |
There was a problem hiding this comment.
For dashboard YAML that uses scalar query fields, such as metrics: orders.revenue, the Python dashboard validator/legacy builder normalizes them with _as_list, so the spec can validate and compile. Here the raw /dashboard JSON is cast to DashboardSpec and metricRefs/dimensionRefs are assumed to be arrays; a scalar metric reaches metricRefs.map and crashes the official UI before the dashboard renders. Normalize these query fields before deriving the tab config.
Useful? React with 👍 / 👎.
| if (!model) return <div className="p-4"><EmptyState message="No model available in this semantic layer." /></div>; | ||
|
|
||
| const leaderboardDims = model.dimensions.filter((dim) => dim.type !== "time"); | ||
| const leaderboardDims = (configured?.dimensions ?? model.dimensions).filter((dim) => dim.type !== "time"); |
There was a problem hiding this comment.
When a dashboard includes a dimension from a related model, this line now surfaces it as a clickable leaderboard, but the filter type maps used by composeFilters are still built only from the active model.dimensions. If that related dimension is numeric or boolean, clicking a row emits quoted string filters like customers.age = '42' instead of typed literals, which can fail or miscompare on type-strict warehouses. Build the type map from the configured/catalog dimensions whenever dashboard leaderboards can span models.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71a12ae9b6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| usePreaggregations: | ||
| chart.query.use_preaggregations ?? chart.query.usePreaggregations ?? defaultPreaggregations(dashboard), |
There was a problem hiding this comment.
When a dashboard relies on the documented interaction_preaggregations / interactionPreaggregations setting, this leaves usePreaggregations undefined because it only checks use_preaggregations. The HTTP adapter then applies its default usePreaggregations: true, so dashboards with interaction_preaggregations: false still route the official UI's interactive totals/series/leaderboards through pre-aggregations instead of the raw-table path that the validated dashboard spec requests.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b05ac3919
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <title>Sidemantic</title> | ||
| <script type="module" crossorigin src="/assets/index-D6RBWkgb.js"></script> | ||
| <link rel="stylesheet" crossorigin href="/assets/index-CxchelG9.css"> | ||
| <script type="module" crossorigin src="/assets/index-D_EIRKyZ.js"></script> |
There was a problem hiding this comment.
On this commit, I checked the bundle gates that CI runs: python scripts/build_webapp.py --check rebuilds the app as assets/index-CBoCPZLy.js while this committed HTML still points at index-D_EIRKyZ.js, and python scripts/build_ui_distribution.py --check also reports the static UI distribution out of sync. The webapp workflow's "Check committed bundles" step runs these checks, so the PR will fail CI until the generated sidemantic/ui/static, sidemantic-rs/ui, and static distribution artifacts are regenerated and committed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0616a12f8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const grains = grainOptions(model?.timeDimension?.supportedGranularities); | ||
| const activeTimeDimension = configured?.timeDimension ?? model?.timeDimension; | ||
| const hasTime = Boolean(activeTimeDimension); | ||
| const grains = grainOptions(activeTimeDimension?.supportedGranularities); |
There was a problem hiding this comment.
For a validated dashboard whose x encoding is ...__minute or ...__second and whose catalog dimension does not declare supported_granularities, dashboardTabConfig sets state.grain to that fine grain, but this call passes undefined to grainOptions, which falls back to only day/week/month/quarter/year. The <select> then has no option matching the active dashboard grain, and once a user changes away they cannot choose the configured grain again; although the earlier grain parsing was fixed, this toolbar option source is fresh evidence that minute/second dashboards are still not fully honored.
Useful? React with 👍 / 👎.
| second: 86_400, // one day secondly | ||
| minute: 525_600, // one year minutely |
There was a problem hiding this comment.
When a minute-grain dashboard opens without a date range, which is the default initial state, this cap allows the first series query to return up to 525,600 buckets; TimeSeriesChart then maps every row into an SVG path, and the strip query can include multiple metrics, so a valid dashboard can download and render hundreds of thousands of points before the user has any chance to narrow the range. Use a render-safe cap, decimation, or a default fine-grain date window for second/minute grains.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 456326273b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const modelName = | ||
| dimensionRefs.find((ref) => ref.includes("."))?.split(".")[0] ?? | ||
| metricRefs.find((ref) => ref.includes("."))?.split(".")[0] ?? | ||
| graphMetricOwner; | ||
| const model = catalog.models.find((candidate) => candidate.name === modelName) ?? catalog.models[0]; |
There was a problem hiding this comment.
When a valid cross-model dashboard groups a fact metric only by a related dimension, such as orders.revenue by customers.country, this picks the related dimension's model before considering the metric. If that related model has its own default time dimension, the later fallback to model.timeDimension makes the dashboard date controls and series query bucket/filter orders.revenue by the customer's timestamp instead of the fact metric's clock. Fresh evidence beyond the earlier metric-availability comment is that the metric is now preserved, but the dimension-first model selection still drives the time fallback.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3a86c63d1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| orderBy: [`${timeDim} ASC`], | ||
| limit: GRAIN_BUCKET_CAP[grain] ?? 2000, |
There was a problem hiding this comment.
When a second/minute dashboard opens with the default all-time range, the new 2,000-bucket cap is applied while metricSeries() still orders ascending, so the backend returns the earliest 2,000 seconds/minutes in the table rather than the latest window or a representative all-time series. The KPI totals are still all-time, which makes the initial dashboard stale and misleading on histories longer than the cap; consider applying a default fine-grain date window, fetching the latest buckets and reversing them, or decimating instead of ASC LIMIT.
Useful? React with 👍 / 👎.
| const query = open && model ? previewRows({ dimensions: selectedDimensions, metrics: [] }, filters, 50) : null; | ||
| const query = open && model | ||
| ? previewRows( | ||
| { dimensions: selectedDimensions, metrics: [] }, |
There was a problem hiding this comment.
When a dashboard tab is configured around dimensions outside the active model, such as orders.revenue by customers.country, dashboard mode renders those configured dimensions in leaderboards but hides the normal field picker. This preview query still uses selectedDimensions, which is derived only from model.dimensions, so opening Rows preview omits the dashboard's actual dimension and shows arbitrary fact-model columns instead of the rows behind the displayed breakdown; use configured.dimensions (and their labels) when a dashboard config is active.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b23ef8b678
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case "second": | ||
| case "minute": |
There was a problem hiding this comment.
When a user brushes a second or minute dashboard chart, these new cases collapse the bucket timestamp to dateOnly(start); ExplorerView.onBrush also stores dateOnly(range.from), so timeFilters() later emits a full-day predicate instead of the selected time interval. On second-grain data this can even combine with the DESC/2000 cap to reload the last ~33 minutes of that day rather than the minutes the user brushed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5cf08bec2e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| forced = bool(os.environ.get("FORCE_COLOR", "")) | ||
| resolved = forced or (enabled and is_terminal(sys.stdout)) |
There was a problem hiding this comment.
When FORCE_COLOR is set and the invocation also uses --no-color or NO_COLOR, color_enabled() has already resolved enabled to false, but this recomputes forced and turns Click/Rich coloring back on. In CI or scripted contexts that set FORCE_COLOR globally, sidemantic --no-color ... can still emit ANSI escapes, defeating the explicit opt-out and potentially breaking captured output consumers.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6d3bd5b04
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| orderBy: [`${timeDim} ${cappedFineGrain ? "DESC" : "ASC"}`], | ||
| limit: GRAIN_BUCKET_CAP[grain] ?? 2000, |
There was a problem hiding this comment.
For second/minute grains this limit is applied to every series query, not only the initial all-time dashboard load. If a user explicitly selects a bounded range longer than the cap (for example Last 7 days on a minute-grain dashboard), the filters still cover the full range but the backend returns only the latest 2,000 buckets, while totals and the range label reflect the full selection; the chart and sparklines therefore silently omit most of the selected interval. Consider applying the cap only to unbounded loads, surfacing truncation, or decimating the bounded range.
Useful? React with 👍 / 👎.
💡 Codex Reviewsidemantic/webapp/src/lib/dashboard.ts Line 91 in dd3998a Preserve aliases for cross-model dashboard metrics
When a dashboard tab lists model-scoped metrics from different models that share the same metric name, e.g. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 similar comment
💡 Codex Reviewsidemantic/webapp/src/lib/dashboard.ts Line 91 in dd3998a Preserve aliases for cross-model dashboard metrics
When a dashboard tab lists model-scoped metrics from different models that share the same metric name, e.g. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Connects dashboard specs to the official React explorer, restores the compact leaderboard treatment, and rebuilds the Python and Rust UI distributions from the same source.