From bfc5fc8fccb6a9bc9a62422b96376ddf738423c8 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Tue, 15 Sep 2026 17:42:38 -0700 Subject: [PATCH] fix: use total seconds for registry history --- .github/workflows/test.yml | 24 ++++++ README.md | 2 + app/src/hooks/use-history-data.ts | 121 +-------------------------- app/src/lib/history-data.ts | 132 ++++++++++++++++++++++++++++++ scripts/history-data.test.js | 86 +++++++++++++++++++ 5 files changed, 246 insertions(+), 119 deletions(-) create mode 100644 .github/workflows/test.yml create mode 100644 app/src/lib/history-data.ts create mode 100644 scripts/history-data.test.js diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000000..52c14048a0 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,24 @@ +name: Data and App Tests +on: + pull_request: + push: + branches: [main] +permissions: + contents: read +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: actions/setup-node@v6 + with: + node-version: "24" + package-manager-cache: false + - run: npm install --ignore-scripts --no-package-lock + working-directory: app + - run: node --test scripts/*.test.js + - name: Test app data semantics + run: | + if [ -d app/tests ]; then node --test app/tests/*.test.*; fi + - run: npm run build + working-directory: app diff --git a/README.md b/README.md index b4dc5610e0..006c654240 100644 --- a/README.md +++ b/README.md @@ -227,6 +227,8 @@ The generated charts show: - Standard deviation in tooltips - Summary table with total installation times and package counts +Registry history uses total installation time in seconds from `registryChartData`, with legacy registry entries in `chartData` as a fallback. Dates containing only normalized registry data are omitted from the seconds chart because those values are measured in milliseconds per package. + ### View Results Online Results are automatically deployed to GitHub Pages when running on the main branch: diff --git a/app/src/hooks/use-history-data.ts b/app/src/hooks/use-history-data.ts index 7a19274f68..0558a0e7fc 100644 --- a/app/src/hooks/use-history-data.ts +++ b/app/src/hooks/use-history-data.ts @@ -1,5 +1,7 @@ import { useState, useEffect } from "react"; import type { HistoryData, HistoryVariation } from "@/types/history"; +import { PACKAGE_MANAGERS, extractDayData } from "@/lib/history-data"; +import type { ChartDataResponse } from "@/lib/history-data"; /** Max days to attempt fetching (generates date strings, 404s are skipped) */ const MAX_DAYS = 180; @@ -7,27 +9,6 @@ const MAX_DAYS = 180; /** How many fetches to run in parallel */ const CONCURRENCY = 10; -const PACKAGE_MANAGERS = [ - "npm", - "yarn", - "pnpm", - "pacquet", - "berry", - "zpm", - "deno", - "bun", - "vlt", - "aube", - "nx", - "turbo", - "vp", - "node", - "aws", - "cloudsmith", - "github", - "jfrog", -]; - interface UseHistoryDataReturn { historyData: HistoryData | null; loading: boolean; @@ -65,104 +46,6 @@ async function parallelLimit( return results; } -type FixtureDataSet = Record< - string, - Array & { fixture: string }> ->; - -interface ChartDataResponse { - date: string; - chartData: { - variations: string[]; - data: FixtureDataSet; - packageManagers: string[]; - }; - perPackageCountChartData?: { - variations: string[]; - data: FixtureDataSet; - packageManagers: string[]; - }; - registryChartData?: { - variations: string[]; - data: FixtureDataSet; - packageManagers: string[]; - }; - registryPerPackageCountChartData?: { - variations: string[]; - data: FixtureDataSet; - packageManagers: string[]; - }; -} - -/** - * Extract per-PM averages (across fixtures) for each variation from a single - * day's chart-data.json response. - * - * For package-management variations (clean, cache, lockfile, etc.) we use - * perPackageCountChartData when available — these values are already in ms/pkg - * and match what the leaderboard cards display. Falls back to total-time - * chartData for older files that lack per-package data. - * - * Registry and task-runner variations always use total-time data. - */ -function extractDayData( - response: ChartDataResponse, -): Record> { - const result: Record> = {}; - - // Use per-package data for package-management variations when available, - // otherwise fall back to total-time chartData - const pmSource = - response.perPackageCountChartData?.data ?? response.chartData.data; - extractFromDataSet(pmSource, result); - - // Process registry chart data. Use registryPerPackageCountChartData when - // available (normalized ms/pkg values); fall back to total-time - // registryChartData for older files that lack per-package registry data. - const registrySource = - response.registryPerPackageCountChartData?.data ?? - response.registryChartData?.data; - if (registrySource) { - extractFromDataSet(registrySource, result); - } - - return result; -} - -function extractFromDataSet( - data: Record< - string, - Array & { fixture: string }> - >, - result: Record>, -): void { - for (const [variation, fixtures] of Object.entries(data)) { - if (!Array.isArray(fixtures) || fixtures.length === 0) continue; - - const pmTotals: Record = {}; - - for (const fixture of fixtures) { - for (const pm of PACKAGE_MANAGERS) { - const val = fixture[pm]; - if (typeof val === "number" && Number.isFinite(val)) { - if (!pmTotals[pm]) pmTotals[pm] = { sum: 0, count: 0 }; - pmTotals[pm].sum += val; - pmTotals[pm].count++; - } - } - } - - const pmAverages: Record = {}; - for (const [pm, { sum, count }] of Object.entries(pmTotals)) { - pmAverages[pm] = Math.round((sum / count) * 1000) / 1000; - } - - if (Object.keys(pmAverages).length > 0) { - result[variation] = pmAverages; - } - } -} - export const useHistoryData = (): UseHistoryDataReturn => { const [historyData, setHistoryData] = useState(null); const [loading, setLoading] = useState(true); diff --git a/app/src/lib/history-data.ts b/app/src/lib/history-data.ts new file mode 100644 index 0000000000..30e4701ec8 --- /dev/null +++ b/app/src/lib/history-data.ts @@ -0,0 +1,132 @@ +export const PACKAGE_MANAGERS = [ + "npm", + "yarn", + "pnpm", + "pacquet", + "berry", + "zpm", + "deno", + "bun", + "vlt", + "aube", + "nx", + "turbo", + "vp", + "node", + "aws", + "cloudsmith", + "github", + "jfrog", +]; + +type FixtureDataSet = Record< + string, + Array & { fixture: string }> +>; + +export interface ChartDataResponse { + date: string; + chartData: { + variations: string[]; + data: FixtureDataSet; + packageManagers: string[]; + }; + perPackageCountChartData?: { + variations: string[]; + data: FixtureDataSet; + packageManagers: string[]; + }; + registryChartData?: { + variations: string[]; + data: FixtureDataSet; + packageManagers: string[]; + }; + registryPerPackageCountChartData?: { + variations: string[]; + data: FixtureDataSet; + packageManagers: string[]; + }; +} + +/** + * Extract per-PM averages (across fixtures) for each variation from a single + * day's chart-data.json response. + * + * For package-management variations (clean, cache, lockfile, etc.) we use + * perPackageCountChartData when available — these values are already in ms/pkg + * and match what the leaderboard cards display. Falls back to total-time + * chartData for older files that lack per-package data. + * + * Registry variations use total-time data, including legacy totals in chartData. + * Per-package-only registry history is omitted rather than mislabeled as seconds. + */ +export function extractDayData( + response: ChartDataResponse, +): Record> { + const result: Record> = {}; + + // Use per-package data for package-management variations when available, + // otherwise fall back to total-time chartData + const pmSource = + response.perPackageCountChartData?.data ?? response.chartData?.data ?? {}; + extractFromDataSet( + Object.fromEntries( + Object.entries(pmSource).filter( + ([variation]) => !variation.startsWith("registry-"), + ), + ), + result, + ); + + // Registry history is labeled in seconds. Older files may contain registry + // totals in chartData rather than a dedicated registryChartData dataset. + // Normalized ms/package data cannot serve as a seconds fallback. + const registrySource = { + ...response.chartData?.data, + ...response.registryChartData?.data, + }; + extractFromDataSet( + Object.fromEntries( + Object.entries(registrySource).filter(([variation]) => + variation.startsWith("registry-"), + ), + ), + result, + ); + + return result; +} + +function extractFromDataSet( + data: Record< + string, + Array & { fixture: string }> + >, + result: Record>, +): void { + for (const [variation, fixtures] of Object.entries(data)) { + if (!Array.isArray(fixtures) || fixtures.length === 0) continue; + + const pmTotals: Record = {}; + + for (const fixture of fixtures) { + for (const pm of PACKAGE_MANAGERS) { + const val = fixture[pm]; + if (typeof val === "number" && Number.isFinite(val)) { + if (!pmTotals[pm]) pmTotals[pm] = { sum: 0, count: 0 }; + pmTotals[pm].sum += val; + pmTotals[pm].count++; + } + } + } + + const pmAverages: Record = {}; + for (const [pm, { sum, count }] of Object.entries(pmTotals)) { + pmAverages[pm] = Math.round((sum / count) * 1000) / 1000; + } + + if (Object.keys(pmAverages).length > 0) { + result[variation] = pmAverages; + } + } +} diff --git a/scripts/history-data.test.js b/scripts/history-data.test.js new file mode 100644 index 0000000000..062d666596 --- /dev/null +++ b/scripts/history-data.test.js @@ -0,0 +1,86 @@ +const assert = require("node:assert/strict"); +const test = require("node:test"); +const { extractDayData } = require("../app/src/lib/history-data.ts"); + +const dataset = (data) => ({ + data, + variations: Object.keys(data), + packageManagers: ["npm", "vlt"], +}); + +test("registry history prefers total seconds when both sources exist", () => { + const result = extractDayData({ + chartData: dataset({}), + registryChartData: dataset({ + "registry-clean": [{ fixture: "next", npm: 3, vlt: 2 }], + }), + registryPerPackageCountChartData: dataset({ + "registry-clean": [{ fixture: "next", npm: 90.18, vlt: 60.14 }], + }), + }); + assert.deepEqual(result["registry-clean"], { npm: 3, vlt: 2 }); +}); + +test("legacy totals in chartData remain in seconds even with normalized data", () => { + const result = extractDayData({ + chartData: dataset({ + "registry-clean": [{ fixture: "next", npm: 3, vlt: 2 }], + }), + perPackageCountChartData: dataset({ + "registry-clean": [{ fixture: "next", npm: 90.18, vlt: 60.14 }], + }), + registryPerPackageCountChartData: dataset({ + "registry-clean": [{ fixture: "next", npm: 90.18, vlt: 60.14 }], + }), + }); + assert.deepEqual(result["registry-clean"], { npm: 3, vlt: 2 }); +}); + +test("per-package-only registry history is skipped rather than mislabeled", () => { + const result = extractDayData({ + chartData: dataset({}), + perPackageCountChartData: dataset({ + "registry-clean": [{ fixture: "next", npm: 90.18 }], + }), + registryPerPackageCountChartData: dataset({ + "registry-clean": [{ fixture: "next", npm: 90.18 }], + }), + }); + assert.equal(result["registry-clean"], undefined); + assert.deepEqual(extractDayData({}), {}); +}); + +test("dedicated totals override legacy totals per variation", () => { + const result = extractDayData({ + chartData: dataset({ + "registry-clean": [{ fixture: "next", npm: 100 }], + "registry-lockfile": [{ fixture: "next", npm: 4 }], + }), + registryChartData: dataset({ + "registry-clean": [{ fixture: "next", npm: 3 }], + }), + }); + assert.deepEqual(result, { + "registry-clean": { npm: 3 }, + "registry-lockfile": { npm: 4 }, + }); +}); + +test("PM normalization and seconds aggregation across fixtures remain intact", () => { + const result = extractDayData({ + chartData: dataset({ clean: [{ fixture: "next", npm: 1 }] }), + perPackageCountChartData: dataset({ + clean: [{ fixture: "next", npm: 20 }], + }), + registryChartData: dataset({ + "registry-clean": [ + { fixture: "next", npm: 3, vlt: 2 }, + { fixture: "astro", npm: 5, vlt: 4 }, + ], + }), + }); + assert.deepEqual(result, { + clean: { npm: 20 }, + "registry-clean": { npm: 4, vlt: 3 }, + }); +});