[BUGFIX] StatChart: derive query mode from the spec instead of always using range

[s3onghyun]

Description

Follow-up to #738 (reverted) and perses/perses#4277.

StatChart declares no queryOptions, so it always issues range queries. For a panel that renders one aggregate value that is wasteful, and against backends that split range queries by interval (Loki is the reported case) it fragments the result over wide time ranges.

#738 addressed this with queryOptions: { mode: 'instant' }, and was reverted because the sparkline draws the series itself and breaks without range data. As @Gladorme put it there, "Stat chart need to support both instant and range".

There is a second consumer of the series besides the sparkline: StatChartPanel computes the calculation client-side via calculateValue(calculation, seriesData), so mean / sum / min / max / first / first-number would silently collapse onto the single point an instant query returns. Only last / last-number are safe.

So this derives the mode from the spec rather than hard-coding it:

export function getStatChartQueryMode(spec: StatChartOptions): 'instant' | 'range' {
  if (spec.sparkline !== undefined) return 'range';
  return LATEST_POINT_CALCULATIONS.has(spec.calculation) ? 'instant' : 'range';
}

wired as queryOptions: getStatChartQueryOptions, mirroring how the table plugin already selects its mode through getTablePanelQueryOptions.

spec.sparkline === undefined is the disabled state — the settings editor toggles it with draft.sparkline = checked ? {} : undefined and reads it back as !!value.sparkline.

No change to defaults. createInitialStatChartOptions() sets sparkline: {}, so a freshly created stat panel keeps using range queries. Instant is only requested once a user turns the sparkline off and leaves the calculation on last/last-number — which is exactly the case where the series is unused.

Testing

Added stat-chart-model.test.ts covering the mode matrix, including an explicit regression case for the sparkline (the reason #738 was reverted) and one for each range-dependent calculation.

type-check and lint pass locally. I could not run jest locally — jest.config.ts imports ../jest.shared and that fails to resolve on Node 23 (ERR_MODULE_NOT_FOUND) for untouched plugins too, so it looks environmental rather than related to this change; relying on CI for the test run.

[Nexucis]

please avoid to use the package @perses-dev/core but @perses-dev/spec instead.

[s3onghyun]

Fixed in 3ab3dbe. I used @perses-dev/plugin-system rather than @perses-dev/spec — CalculationType is re-exported there, and it is what every other consumer in the repo uses, including stat-chart-model.ts right next to this test. That test file was the only place reaching into @perses-dev/core directly. Happy to switch to @perses-dev/spec if that is the direction you want these to move.

[Nexucis]

adding @Gladorme for the followup

[celian-garcia]

I am not expert in that part but what is the difference between last and last-number.

If last number means last result of number type or something, could the range query and instance query give different results ?

[s3onghyun]

Great question — and you're right, they are not interchangeable here. last is literally the last value of the series; last-number is the last non-null numeric value ("Last numeric value"), so when the most recent point is null it walks back to an earlier one. An instant query only returns that latest point, so for last-number with a trailing gap it would give a different (or empty) result than the range query — exactly the divergence you spotted. So only last is instant-safe. Fixed in 5ec159a: last-number now uses range like the aggregating calculations, and the set is renamed INSTANT_SAFE_CALCULATIONS = {last} to make the reasoning explicit. Thanks for catching it.

[jgbernalp]

LGTM, @Gladorme WDYT?

[s3onghyun]

Thanks @jgbernalp. Pushed one more fix from @celian-garcia's review (5ec159a): last-number isn't instant-safe either — it's the last non-null value, so it needs range like the aggregations. Only last stays instant. Ready for another look @Gladorme.

[tgitelman]

@s3onghyun So when sparkline is enabled the StatChart value will be incorrect?

[celian-garcia]

A last question from me @s3onghyun if you edit live the sparkline checkbox will it update the options live ?

[s3onghyun]

No — the value stays correct with the sparkline enabled. In that case the panel keeps requesting a range query (the sparkline needs the series to draw), and calculateValue(calculation, seriesData) runs over that full series. For last that's the last point of the range, which is the same value an instant query would return — so the displayed number is identical either way.

The only mode change this PR introduces is: sparkline off and calculation is last → instant. That's exactly the case where the series isn't used for anything (no sparkline to draw, and last == the latest point), so nothing can go wrong. Everything else — sparkline on, or any aggregating calculation, or last-number — stays on range, unchanged from today's behavior.

So this is strictly a narrowing: fewer situations issue a range query, and only in situations where the range data was never consumed. The sparkline path is untouched. Your #738 broke because it forced instant on that path; this keys off the spec so it can't.

[s3onghyun]

@celian-garcia Yes, it updates live — I traced the path rather than assuming:

1. In @perses-dev/dashboards PanelQueriesSharedControls computes the options with useMemo(() => plugin.queryOptions(spec), [panelDefinition.spec.plugin.spec, pluginPreview]). The dependency array includes spec.plugin.spec, so toggling the sparkline checkbox changes the spec and re-runs getStatChartQueryMode, producing the new mode.
2. That pluginQueryOptions (with the new mode) is passed straight into the query hook.
3. In @perses-dev/plugin-system time-series-queries.ts, mode is part of the react-query queryKey ([..., suggestedStepMs, mode]). When the key changes, react-query refetches automatically.

So: check/uncheck sparkline → spec changes → memo recomputes mode → queryKey changes → refetch. Same chain applies to changing the calculation. The GridItemContent path (rendered dashboard) calls queryOptions(spec) the same way, so it is consistent between the editor preview and the final panel.

[tgitelman]

@s3onghyun
The claim that last of a range equals an instant result assumes the range query returns correct data. With Loki's split_queries_by_interval (hardcoded to 30m by the Loki Operator), query_range on wide time windows gets split into sub-queries — each evaluating the range vector with only its chunk's data. The "last point" then reflects a partial 30-minute window, not the full range. We hit this exact issue on our dashboards (perses/perses#4277) — stat panels showed wrong values until we switched to instant.

So when sparkline is ON and the panel stays on range, both the sparkline and the value can be wrong. The real fix is to decouple them: instant query for the value, range query for the sparkline. They have different data needs and shouldn't share a single query mode.

[s3onghyun]

You're right, and thanks for the detail — that's a failure mode I hadn't accounted for. If split_queries_by_interval makes a range query's last point reflect only the final sub-window rather than the true latest sample, then "last of range == instant" doesn't hold, and my premise for the sparkline-on case was wrong. I'll stop claiming the value is safe there.

On the fix, I think it's worth separating what this PR can do from what the real fix needs:

• This PR (queryOptions returns one { mode }) can only pick a single mode for the whole panel. So the honest scope of it is narrow: it makes the value correct in the case where it's unambiguously safe — sparkline off and calculation last → instant — and leaves everything else on range exactly as today. It's a strict improvement over "always range", but it does not fix the sparkline-on value, and I shouldn't have implied otherwise.

• The decoupling you're describing — instant for the value, range for the sparkline — can't be expressed through queryOptions as it stands. PanelPlugin.queryOptions is QueryOptions | ((spec) => QueryOptions), a single mode, and the panel receives one queryResults set. Making the value and the sparkline use different modes means either the panel issuing a second query itself, or extending the plugin-system query API to allow per-consumer modes. That's a bigger change than this PR and probably its own design discussion.

So two honest options:

1. Land this as the narrow, correct-by-construction step (value correct when sparkline off + last; no regression anywhere else), and open a follow-up issue for the decoupled instant-value / range-sparkline design, which is the actual cure for the case you hit.
2. Hold this PR until the decoupling design is settled, if you'd rather not ship a partial improvement.

I'm fine with either — you and @Gladorme know the roadmap here better than I do. If it's (1), I'm happy to write up the follow-up issue with the split_queries_by_interval reproduction so it's captured properly. If it's (2), I'll close this and we design the two-query approach first.

[SoWieMarkus]

Hey, by coincidence I started working on the same topic with a different approach in #752. Would love to hear your opinion on that :D

[s3onghyun]

Update: @SoWieMarkus opened #752, which adds a per-query instant flag to the Prometheus query spec (Grafana-style). That's a better layer than this PR — it lets the stat value use an instant query and the sparkline a range query as two independent queries, which is exactly the decoupling @tgitelman asked for and which queryOptions' single panel-level mode here can't express. It also sidesteps the range-value correctness problem entirely.

So I'd suggest we go with #752 and close this. I'll hold off closing for a moment in case @Gladorme still wants the narrow 'instant when sparkline off + last' improvement as an interim, but my recommendation is to prefer #752. Happy to do the StatChart-side follow-up (issue the instant value query once the flag exists) and move my sparkline regression tests over.

[s3onghyun]

Closing in favour of #752, which the team is already iterating on. Its per-query instant flag is the right layer — it decouples the stat value (instant) from the sparkline (range) as independent queries, which is what @tgitelman's Loki split_queries_by_interval case needs and what this panel-level single-mode approach couldn't express.

Happy to pick up the StatChart-side follow-up once #752 lands (have the stat panel issue an instant query for its value, keep range for the sparkline), and to move my sparkline regression tests over so #738's failure stays covered. Thanks all for the thorough review — the discussion made the right design clearer.