feat(analytics): honour widget dateGranularity, sortBy/sortOrder, and limit in dataset queries (#3588) - #3652
Merged
Conversation
… limit in dataset queries (#3588) Three presentation options were accepted by the metadata layer and then dropped by the analytics query builder — no SQL, no error. The only symptom was the `sql` a dataset response echoes, so a dashboard could declare `dateGranularity: 'month'` and quietly render one bar per record. - `DatasetSelection.dateGranularity` buckets every selected date dimension. Precedence per dimension: an explicit `timeDimensions` granularity, then the selection's, then the dataset dimension's own default — so a widget can bucket a trend by month without the dataset committing every consumer to it. - `order`/`limit`/`offset` are applied to the ASSEMBLED grid: after measure-scoped sub-queries merge, after `compareTo` columns attach, and after derived measures are computed. A derived measure is therefore a valid sort key, and the ObjectQL aggregate path — which has no ordering grammar, and which native SQL hands every date-bucketed query to — orders identically to native SQL. A single-query selection still pushes the window into the statement. An order key naming nothing the selection projects is rejected (400) rather than ignored. - A `limit` without an `order` orders by the selected dimensions first, so it truncates a reproducible window instead of an arbitrary subset. - The four query-affecting widget `options` keys plus `stageOrder` are declared on `DashboardWidgetOptionsSchema`, making a typo an author-time error. The bag stays open, so renderer extras pass through untouched. Two latent bugs surfaced while fixing the above: - `order`/`limit` were forwarded to EVERY sub-query. A measure-scoped supplementary query selects one measure, so an inherited ORDER BY named a column it never selected and an inherited LIMIT truncated it before the merge, dropping rows from the grid. Nothing hit this only because nothing passed `order`. - The `compareTo` pass hand-built its query and skipped granularity resolution, merging a month-bucketed primary grid against raw-timestamp comparison rows. No dimension key matched, so every `<measure>__compare` column came back empty. `ObjectQLStrategy` now also echoes a representative `sql` (date_trunc, WHERE, ORDER BY, LIMIT; filter values parameterized, never inlined). The `sql` field previously vanished whenever a query was date-bucketed, leaving an author unable to tell "not implemented" from "this strategy doesn't report". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SEZLBGyNoJMLvtGPqsMPRC
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 3 package(s): 106 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
…uery options (#3588) `content/docs/references/ui/dashboard.mdx` is generated from the spec and went stale when `DashboardWidgetOptionsSchema` was added — CI's generated-artifact drift check caught it. Also rewrites the hand-written `options` row in the dashboards guide, which described the bag as "Additional display options". That description is what let this bug happen: four of its keys change the compiled query, and an author had no way to know which. The guide now lists the five declared keys with their effects and the behavioural notes that are easy to get wrong (an unprojectable `sortBy` is rejected, a bare `limit` orders by dimensions first, a derived measure is a valid sort key, a funnel without a declared stage order falls back to value-descending). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SEZLBGyNoJMLvtGPqsMPRC
…getOptions (#3588) The api-surface gate reports 0 breaking, 2 added — the new `DashboardWidgetOptionsSchema` const and its inferred `DashboardWidgetOptions` type, both purely additive. Regenerated and committed as the gate asks. Verified the whole spec generated-artifact set locally this time rather than one gate per CI round-trip: check:docs, check:api-surface, check:skill-refs, check:skill-docs, check:spec-changes, check:upgrade-guide, check:react-blocks — all pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SEZLBGyNoJMLvtGPqsMPRC
os-zhuang
marked this pull request as ready for review
July 27, 2026 13:24
os-zhuang
pushed a commit
that referenced
this pull request
Jul 27, 2026
#3652 rewrote `ObjectQLStrategy.generateSql` (date_trunc for bucketed dimensions, COUNT DISTINCT, ORDER BY/LIMIT/OFFSET) and made `execute()` echo the rendered SQL on the result. Kept all of that. Resolved the WHERE clause in favour of this branch's version, which renders the same caller filters plus the two things #3652's inline loop does not: - the READ SCOPE (`compileScopedFilterToSql`), the point of #3602 residual 2 — without it the echoed string still reads as an unscoped table scan while the aggregate is scoped; - `assertJoinedScopesEnforceable`, so it never renders SQL for a query `execute()` would reject. Its filter rendering is also closer to what runs: `contains` binds `%v%` (a bare comparand under LIKE would only match exactly), and values go through `coerceFilterValueForObjectQL`, so the comparand shown is the one this path hands the engine. Scope VALUES stay in `params`, which the `execute()` echo discards — only the predicate shape reaches the browser, same as NativeSQL. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013st2KArjmLQSuVzpDS91jj
This was referenced Jul 27, 2026
os-zhuang
added a commit
that referenced
this pull request
Jul 27, 2026
…#3654) (#3691) The ObjectQL fallback path (date-granularity bucketing, in-memory driver, federated) could not join, so cross-object grouping like `revenue by account.region` was rejected outright (#3664 stopgap). It now serves the common case by FK-expand: 1. group the base aggregate on the lookup FK column (`account`) — which the engine CAN do — scoped to the base object (and, per #3651's second belt, threading the ExecutionContext to the engine too); 2. resolve each FK id to the related attribute (`region`) with a read of the referenced object scoped to THAT object's own RLS; 3. re-bucket by the resolved attribute in memory, recombining measures (sum/count add, min/max take the extremum). A base row whose referenced record the caller cannot read buckets under an explicit `(restricted)` group: the measure still counts (grand totals preserved) but the hidden attribute never appears — no leak (ADR-0021 D-C / #3602). `/analytics/sql` renders the equivalent LEFT JOIN, so preview and execution accept/reject the same set. Bounded — still rejected LOUD (never silently wrong): cross-object in a MEASURE or FILTER, multi-hop dims, and non-recombinable measures (avg/count_distinct) with a cross-object dim. NativeSQLStrategy is unchanged. The pure re-bucketing step is isolated in cross-object-rebucket.ts and exhaustively unit-tested (recombination, restricted-total conservation, null-vs-restricted, multi-dim). Integration tests drive the real AnalyticsService with a two-call aggregate stub, asserting both the base and FK-resolution are scoped and the restricted bucket forms; reverting the strategy turns all three red. Re-applied cleanly on top of #3651/#3652 (executeAggregate carries context; generateSql renders scope) after those landed on main. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3588 (framework side). The companion renderer change is objectstack-ai/objectui#claude/analytics-query-builder-params-vi64vw.
The problem
Three widget options were accepted by the metadata layer and then dropped by the analytics query builder. They reached no SQL and raised no error, so the only way to notice was to read the
sqla dataset response echoes — a dashboard could declaredateGranularity: 'month'and quietly render one bar per record.The issue's framing was right on two of three counts; the third turned out to sit in the renderer, not here:
dateGranularityinto GROUP BYsortBy/sortOrder, makelimitdepend on itAdvancedChartImplre-sorted funnel data by value, so any ordering this layer produced was overridden client-sideChanges
dateGranularitynow buckets.DatasetSelectiongained an optionaldateGranularity, applied to every selecteddatedimension. Precedence per dimension: an explicittimeDimensionsgranularity → the selection's → the dataset dimension's own default. A widget can bucket a trend by month without the dataset committing every other consumer to that granularity.order/limit/offsetnow apply on every path. They are applied to the assembled grid — after measure-scoped sub-queries merge, aftercompareTocolumns attach, and after derived measures are computed. Two consequences that motivated doing it here rather than only in SQL:EngineAggregateOptionshas nowhere to put one), and native SQL declines granularity — so every date-bucketed query is handed to it. Ordering a bucketed trend was unreachable in SQL by construction.A single-query selection still pushes the window down into the statement, so the database does the work and the echoed SQL shows it; the post-pass is then a no-op re-sort.
limitis deterministic. Without anorder, a limit orders by the selected dimensions first, so it truncates a reproducible window rather than an arbitrary subset. Anorderkey naming nothing the selection projects is rejected as a 400 instead of silently ignored.Widget
optionsis a contract again. The four query-affecting keys plusstageOrderare declared on a newDashboardWidgetOptionsSchema, sosortDirectionorgranularityis an author-time error rather than an option that reads as if it works. The bag stays open (passthrough) —icon,columns,striped,densityand friends pass through untouched, and every existing widget still validates.Two latent bugs found on the way
Neither was reachable before, which is why they were quiet:
order/limitwere forwarded to every sub-query. A measure-scoped supplementary query selects one measure, so an inheritedORDER BYnamed a column it never selected, and an inheritedLIMITtruncated it before the merge — dropping rows from the assembled grid. Nothing hit this only because nothing passedorder.compareToskipped granularity resolution. It hand-built its query instead of going throughbuildQuery, so a month-bucketed primary grid was merged against raw-timestamp comparison rows. No dimension key matched and every<measure>__comparecolumn came back empty. Found by a test written for the granularity work.SQL echo
ObjectQLStrategynow attaches a representativesql(withdate_trunc,WHERE,ORDER BY,LIMIT). Without this the fix would have removed thesqlfield from exactly the responses the issue used to diagnose the bug — that path returned none, and bucketing routes queries to it. Filter values are rendered as$nplaceholders and returned inparams, never inlined, since the echoed statement travels to the browser.Testing
dataset-selection-window.test.ts— 20 tests over granularity precedence (including the compareTo merge), ordering on both strategy paths, deterministic limit, derived-measure sort keys, the sub-query regression guard, and the SQL echo.@objectstack/service-analyticssuite: 199 passed (179 pre-existing, unchanged).@objectstack/specdashboard suite green;spec,service-analyticsandrestall build (dts typecheck) clean.Not verified in a running browser — the rendered result depends on the objectui companion PR, so end-to-end confirmation belongs there.
Generated by Claude Code