Skip to content

fix: weekly chart prediction and data pipeline extraction#2014

Open
jycouet wants to merge 4 commits intonpmx-dev:mainfrom
jycouet:fix/weekly-end-anchored
Open

fix: weekly chart prediction and data pipeline extraction#2014
jycouet wants to merge 4 commits intonpmx-dev:mainfrom
jycouet:fix/weekly-end-anchored

Conversation

@jycouet
Copy link
Contributor

@jycouet jycouet commented Mar 9, 2026

🔗 Linked issue

Closes: #2013

🧭 Context

Weekly chart displayed partial last-week buckets with very low values (1–2 days instead of 7), and prediction/estimation was only available for monthly & yearly granularity.

📚 Description

  • Extend estimation support to weekly (and daily) granularity
  • Add linear regression prediction based on the last N complete data points (configurable, default 4)
  • Extract prediction & bucket logic from TrendsChart.vue into a new chart-data-prediction.ts utility
  • Introduce applyDataPipeline that orchestrates: prediction → data correction (smoothing/averaging)
  • Add a prediction points slider (0–30) in the chart filter UI
  • Apply prediction before smoothing so partial periods don't skew nearby curves

Before

image

After

image

- extend estimation support to weekly (and daily) granularity
- add linear regression prediction based on recent data points
- extract prediction logic into chart-data-prediction utility
- add configurable prediction points slider (0–30, default 4)
- apply prediction before smoothing so partial periods don't skew curves
@vercel
Copy link

vercel bot commented Mar 9, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
npmx.dev Building Building Preview, Comment Mar 10, 2026 5:27am
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview Mar 10, 2026 5:27am
npmx-lunaria Ignored Ignored Mar 10, 2026 5:27am

Request Review

@github-actions
Copy link

github-actions bot commented Mar 9, 2026

Lunaria Status Overview

🌕 This pull request will trigger status changes.

Learn more

By default, every PR changing files present in the Lunaria configuration's files property will be considered and trigger status changes accordingly.

You can change this by adding one of the keywords present in the ignoreKeywords property in your Lunaria configuration file in the PR's title (ignoring all files) or by including a tracker directive in the merged commit's description.

Tracked Files

File Note
i18n/locales/en.json Source changed, localizations will be marked as outdated.
Warnings reference
Icon Description
🔄️ The source for this localization has been updated since the creation of this pull request, make sure all changes in the source have been applied.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 9, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7a590f3f-1016-4850-81d3-1f9676942292

📥 Commits

Reviewing files that changed from the base of the PR and between 626d233 and 1dc9041.

📒 Files selected for processing (3)
  • app/components/Package/TrendsChart.vue
  • app/composables/useSettings.ts
  • app/utils/chart-data-prediction.ts

📝 Walkthrough

Walkthrough

Replaces the legacy data-correction path in TrendsChart.vue with a new data-prediction pipeline (applyDataPipeline), adds bucket-aware utilities and extrapolation logic for partial periods (daily/weekly/monthly/yearly) in a new module app/utils/chart-data-prediction.ts, threads predictionPoints through settings and UI (persisted default 4, control 0–30), updates multi-series and single-series processing to use the pipeline, removes old extrapolation helpers, and adds an i18n key for "prediction".

Possibly related PRs

Suggested reviewers

  • danielroe
  • graphieros
  • serhalp
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The PR description clearly addresses issue #2013, explaining the problem (partial weekly buckets) and detailing the solution (linear regression prediction, data pipeline extraction, UI controls).
Linked Issues check ✅ Passed The PR implements Option B from issue #2013: extends estimation to weekly/daily, adds configurable linear regression prediction (default 4 points), extracts logic into chart-data-prediction.ts, introduces applyDataPipeline, and adds UI slider.
Out of Scope Changes check ✅ Passed All changes are directly scoped to issue #2013: TrendsChart modifications for prediction integration, new prediction utility, settings schema updates, i18n translations, and UI controls—no extraneous modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8518dbe0-f937-49f5-955e-3349033f19e0

📥 Commits

Reviewing files that changed from the base of the PR and between 8cea27f and 626d233.

📒 Files selected for processing (5)
  • app/components/Package/TrendsChart.vue
  • app/composables/useSettings.ts
  • app/utils/chart-data-prediction.ts
  • i18n/locales/en.json
  • i18n/schema.json

@codecov
Copy link

codecov bot commented Mar 9, 2026

Codecov Report

❌ Patch coverage is 84.52381% with 13 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
app/components/Package/TrendsChart.vue 55.00% 3 Missing and 6 partials ⚠️
app/utils/chart-data-prediction.ts 93.75% 2 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

@graphieros
Copy link
Contributor

graphieros commented Mar 10, 2026

Setting the start date to the previous day yields a prediction of 0.
Setting previous start date to a mod != 7 yields predictions that do not seem correct.

Example with vue, keeping the default 14-day prediction window:

image
day - 1 day - 2 day -3 day -4 day-5 day -6
image image image image image image

@jycouet jycouet force-pushed the fix/weekly-end-anchored branch from 1dc9041 to 626d233 Compare March 10, 2026 05:02
@jycouet
Copy link
Contributor Author

jycouet commented Mar 10, 2026

Sorry @graphieros when trying to address the rabbit, it destroyed the feature!

image

Could you check it again? :/

Note: could you add the link to the package with modal & range, it goes faster to find it. Thx

Copy link
Contributor

@graphieros graphieros left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes appear to fix the weekly estimations when start date is =! mod 7, but you will need to check if it's ok ^^

Also I feel this file should deserve a test too

// ---------------------------------------------------------------------------
// Bucket boundaries (UTC)
// ---------------------------------------------------------------------------

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const DAY_MS = 86_400_000

function getUtcDayStart(ts: number): number {
  const d = new Date(ts)
  return Date.UTC(d.getUTCFullYear(), d.getUTCMonth(), d.getUTCDate())
}

// Monday-based week start in UTC
function getWeeklyBucketStartUtc(ts: number): number {
  const dayStart = getUtcDayStart(ts)
  const day = new Date(dayStart).getUTCDay()
  const diffFromMonday = (day + 6) % 7
  return dayStart - diffFromMonday * DAY_MS
}

export function getBucketStartUtc(ts: number, g: ChartTimeGranularity): number {
const d = new Date(ts)
if (g === 'yearly') return Date.UTC(d.getUTCFullYear(), 0, 1)
if (g === 'monthly') return Date.UTC(d.getUTCFullYear(), d.getUTCMonth(), 1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now with

if (g === 'weekly') return getWeeklyBucketStartUtc(ts)

const d = new Date(ts)
if (g === 'yearly') return Date.UTC(d.getUTCFullYear() + 1, 0, 1)
if (g === 'monthly') return Date.UTC(d.getUTCFullYear(), d.getUTCMonth() + 1, 1)
if (g === 'weekly') return getBucketStartUtc(ts, 'daily') + 7 * 86_400_000
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced with

if (g === 'weekly') return getWeeklyBucketStartUtc(ts) + 7 * DAY_MS

const last = series.at(-1) ?? 0

// Weekly dates are stored as timestampEnd; bucket start is 6 days earlier
const bucketTs = granularity === 'weekly' ? lastDateMs - 6 * 86_400_000 : lastDateMs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const bucketTs = lastDateMs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chart weekly display improvement

2 participants