🎨 Improved analytics chart bucketing for Year to date and All time ranges#29237
Merged
Conversation
…nges fixes https://linear.app/ghost/issue/PLA-236/ytd-range-renders-unaggregated-daily-points-choppy-analytics-charts fixes https://linear.app/ghost/issue/PLA-237/ytd-month-buckets-labeled-inconsistently-web-uses-1st-of-month fixes https://linear.app/ghost/issue/PLA-238/verify-all-time-bucketing-in-post-analytics-web-currently-summarizes Chart bucketing was split across three implementations (a 344-line local copy in src/analytics, a getPeriodText copy in src/posts, and Shade's plainer sanitizeChartData) that disagreed about Year to date and about where bucket labels land. Year to date rendered raw daily points in some views, and monthly buckets were labeled with the 1st of the month on web charts but the last day on newsletters/growth charts. There is now one implementation in apps/admin/src/shared/analytics: - Every range resolves to an effective span in days (Year to date → days since Jan 1, All time → the span of the chart's own data after trimming the API's leading empty padding) and one set of thresholds applies to all of them: under 91 days daily, 91-356 weekly, over 356 monthly. - Buckets are always labeled with their period start regardless of aggregation type, so sum charts (visits) and exact charts (member totals) place points on the same dates, and getEffectiveChartRange maps the chosen bucketing into the matching display band ("Week of ...", "MMM YYYY") for GhAreaChart ticks and tooltips. - Shade's sanitizeChartData is removed: the bucketing rules depend on Ghost's range sentinels, which the design system shouldn't know about. Also removes dead code (bulk-import outlier detection, monthly-exact) and unifies the duplicated KpiMetric type.
ref https://linear.app/ghost/issue/PLA-236/ytd-range-renders-unaggregated-daily-points-choppy-analytics-charts ref https://linear.app/ghost/issue/PLA-238/verify-all-time-bucketing-in-post-analytics-web-currently-summarizes Review feedback on the bucketing unification: - The weekly band ran to 356 days, so an ~11-month "All time" chart rendered ~48 weekly points — dense and jagged next to the ~12 monthly points that "Last 12 months" gets. The weekly band now caps at 270 days (~9 months); Year to date keeps its comfortable ~28 weekly points in mid-summer and flips to monthly from October. Only span-resolved ranges (Year to date, All time, post age) can land between the fixed dropdown options of 91 and 372 days, so no dropdown option changes bands. - Buckets are labeled with their calendar period start, which dated the partial first bucket before the chart's own range — a Year to date chart opened at "Week of 28 Dec" of the previous year. The first bucket's label is now clamped to the range start ("Week of 1 Jan"), or to the first data point for All time. Later buckets keep calendar period starts, and the zero-filling in the paid subscriptions chart clamps its period keys the same way, so cross-chart bucket alignment is unchanged.
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run @tryghost/admin-x-settings:test:acceptance |
✅ Succeeded | 10m 8s | View ↗ |
nx run-many --target=build --projects=tag:publi... |
✅ Succeeded | <1s | View ↗ |
nx run-many -t test:unit -p @tryghost/admin,@tr... |
✅ Succeeded | 5m 40s | View ↗ |
nx run @tryghost/admin:build |
✅ Succeeded | 2m 56s | View ↗ |
nx run ghost-admin:test |
✅ Succeeded | 2m 57s | View ↗ |
nx run ghost-monorepo:lint:boundaries |
✅ Succeeded | <1s | View ↗ |
nx run-many -t lint -p @tryghost/admin,@tryghos... |
✅ Succeeded | 1m 25s | View ↗ |
nx run @tryghost/activitypub:test:acceptance |
✅ Succeeded | 43s | View ↗ |
Additional runs (2) |
✅ Succeeded | ... | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-07-10 16:43:56 UTC
Contributor
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/admin/src/posts/analytics/Overview/overview.tsx (1)
92-92: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winWrap
displayChartRangeinuseMemofor consistency and performance.Unlike all other files in this PR (
overview.tsx,newsletters-kpis.tsx,web-kpis.tsx),displayChartRangeis computed as a bareconstwithoutuseMemo. Since this component holds state (isGiftLinkOpen) that triggers re-renders,getEffectiveChartRange(which runstrimForRange+getAggregationStrategy) will recompute on every render even whenchartDataandchartRangeare unchanged.♻️ Proposed fix
-const displayChartRange = getEffectiveChartRange(chartRange, chartData as KpiDataItem[] || [], {fieldName: currentMetric.dataKey as keyof KpiDataItem}); +const displayChartRange = useMemo(() => { + return getEffectiveChartRange(chartRange, chartData as KpiDataItem[] || [], {fieldName: currentMetric.dataKey as keyof KpiDataItem}); +}, [chartRange, chartData, currentMetric]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/admin/src/posts/analytics/Overview/overview.tsx` at line 92, Wrap the displayChartRange calculation in the Overview component with useMemo, using chartRange, chartData, and currentMetric.dataKey as dependencies. Preserve the existing getEffectiveChartRange arguments and ensure useMemo is imported from React if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/admin/src/shared/analytics/chart-helpers.test.ts`:
- Line 275: Update the test description in the monthly aggregation test to
reference ranges over 270 days, matching the current monthly threshold and the
range used by the test.
---
Nitpick comments:
In `@apps/admin/src/posts/analytics/Overview/overview.tsx`:
- Line 92: Wrap the displayChartRange calculation in the Overview component with
useMemo, using chartRange, chartData, and currentMetric.dataKey as dependencies.
Preserve the existing getEffectiveChartRange arguments and ensure useMemo is
imported from React if needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f2990daf-2631-4abb-b31d-f20abeb21dee
📒 Files selected for processing (30)
apps/admin/src/analytics/utils/chart-helpers.test.tsapps/admin/src/analytics/utils/chart-helpers.tsapps/admin/src/analytics/views/Stats/Growth/components/growth-kpis.tsxapps/admin/src/analytics/views/Stats/Growth/components/growth-sources.tsxapps/admin/src/analytics/views/Stats/Growth/components/new-subscribers-cadence.tsxapps/admin/src/analytics/views/Stats/Growth/components/paid-subscription-change-chart.test.tsxapps/admin/src/analytics/views/Stats/Growth/components/paid-subscription-change-chart.tsxapps/admin/src/analytics/views/Stats/Growth/growth.tsxapps/admin/src/analytics/views/Stats/Locations/components/locations-card.tsxapps/admin/src/analytics/views/Stats/Newsletters/components/newsletters-kpis.tsxapps/admin/src/analytics/views/Stats/Newsletters/newsletters.tsxapps/admin/src/analytics/views/Stats/Overview/components/overview-kpis.tsxapps/admin/src/analytics/views/Stats/Overview/components/top-posts.tsxapps/admin/src/analytics/views/Stats/Overview/overview.tsxapps/admin/src/analytics/views/Stats/Web/components/sources-card.tsxapps/admin/src/analytics/views/Stats/Web/components/top-content.tsxapps/admin/src/analytics/views/Stats/Web/components/web-kpis.tsxapps/admin/src/analytics/views/Stats/Web/web-kpi-metrics.tsapps/admin/src/posts/analytics/Overview/overview.tsxapps/admin/src/posts/analytics/Web/components/kpi-metrics.tsapps/admin/src/posts/analytics/Web/components/kpis.tsxapps/admin/src/posts/analytics/Web/components/sources.tsxapps/admin/src/posts/analytics/Web/web.tsxapps/admin/src/posts/analytics/utils/chart-helpers.test.tsxapps/admin/src/posts/analytics/utils/chart-helpers.tsapps/admin/src/shared/analytics/chart-helpers.test.tsapps/admin/src/shared/analytics/chart-helpers.tsapps/admin/src/shared/analytics/kpi.tsapps/shade/src/app.tsapps/shade/src/lib/app-utils.ts
💤 Files with no reviewable changes (7)
- apps/admin/src/posts/analytics/utils/chart-helpers.ts
- apps/admin/src/posts/analytics/utils/chart-helpers.test.tsx
- apps/admin/src/analytics/views/Stats/Growth/components/paid-subscription-change-chart.test.tsx
- apps/admin/src/analytics/utils/chart-helpers.test.ts
- apps/shade/src/lib/app-utils.ts
- apps/shade/src/app.ts
- apps/admin/src/analytics/utils/chart-helpers.ts
| }); | ||
|
|
||
| describe('monthly aggregation', () => { | ||
| it('aggregates data monthly for ranges over 356 days', () => { |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Stale threshold in test description ("over 356 days").
The weekly band now caps at 270 days (monthly is > 270), per this PR's threshold change. The test uses range 372, so it still passes, but the "356" wording no longer reflects the boundary and is misleading.
📝 Suggested wording fix
- it('aggregates data monthly for ranges over 356 days', () => {
+ it('aggregates data monthly for ranges over 270 days', () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('aggregates data monthly for ranges over 356 days', () => { | |
| it('aggregates data monthly for ranges over 270 days', () => { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/admin/src/shared/analytics/chart-helpers.test.ts` at line 275, Update
the test description in the monthly aggregation test to reference ranges over
270 days, matching the current monthly threshold and the range used by the test.
9larsons
enabled auto-merge (squash)
July 10, 2026 16:47
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
1 participant
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 https://linear.app/ghost/issue/PLA-236/
fixes https://linear.app/ghost/issue/PLA-237/
fixes https://linear.app/ghost/issue/PLA-238/
Chart bucketing/aggregation in Admin existed as three divergent copies: a ~344-line re-implementation in
src/analytics/utils/chart-helpers.ts, agetPeriodTextcopy insrc/posts/analytics/utils/, and Shade's plainersanitizeChartData. Which copy a chart happened to import determined how it bucketed. This PR collapses them into one module,apps/admin/src/shared/analytics/chart-helpers.ts— the rest is cleanup:sanitizeChartDatafrom@tryghost/shade/app(bucketing depends on Ghost's range sentinels, which the design system shouldn't know about)_isOutlierhad no consumers) and the unreachablemonthly-exactpathKpiMetrictype (chartColorvscolor) intosrc/shared/analytics/kpi.tsThe new logic
Every range resolves to an effective span in days — fixed ranges as-is, Year to date → days since Jan 1, All time → the span of the chart's own data — then one threshold set applies: <91 days daily, 91–270 weekly, >270 monthly. Buckets are labeled with their period start for all aggregation types, except the partial first bucket which clamps to the range start. Tick/tooltip formats follow the bucketing via
getEffectiveChartRange().Before → after
api_kpispads the 1000-day window with zeros; trimmed in one view only, after aggregationexactcharts (member totals): last day of month — cross-tab points off by a month's width-1) showed raw dates like "31 Mar" for aggregated buckets; some views force-faked ranges (366/91/30) to work around itgetEffectiveChartRange()maps the actual bucketing to the display band: monthly → "Mar 2026", weekly → "Week of 22 Mar" — the hacks are goneVerification
apps/admin: typecheck, lint, 848 unit tests (incl. a new suite covering thresholds, YTD/All time resolution, label conventions, and first-bucket clamping);apps/shade: build, lint, tests.pnpm dev/pnpm dev:analyticsacross site Analytics (Overview/Growth/Newsletters/Web) and Post Analytics (Web traffic): YTD renders ~28 aligned weekly buckets on every tab, All time on an ~11-month-old post renders ~12 monthly buckets.