Skip to content

fix: pass route context to chart/table components for average variation - #134

Merged
darcyclarke merged 1 commit into
mainfrom
darcy/fix-averages
Jul 29, 2026
Merged

fix: pass route context to chart/table components for average variation#134
darcyclarke merged 1 commit into
mainfrom
darcy/fix-averages

Conversation

@vltbaudbot

Copy link
Copy Markdown
Contributor

Problem

When PR #133 added the average variation for registries and task runners, the data was correctly computed and selected per-route in the VariationPage component. However, the child rendering components (VariationChart, VariationTable, HistoryChart, PackageCountTable, ProcessCountTable) independently determined whether they were in a registry or task-runner context by checking only the variation name via isRegistryVariation(currentVariation) and isTaskExecutionVariation(currentVariation) — both of which return false for "average".

This caused the registries and task-runners average pages to render with package-manager formatting:

  • Grouped bar charts instead of horizontal per-fixture bars (the registry/task-runner layout)
  • PM display names (e.g. "npm") instead of registry hostnames (e.g. "registry.npmjs.org")
  • Version strings shown where they should be hidden (registries)
  • No baseline highlighting for npm on registry pages
  • Wrong Y-axis labels

Fix

Add optional isRegistryRoute / isTaskExecutionRoute props to all affected components. VariationPage already correctly derives these flags from the URL route context — now it passes them down so child components use route-aware detection instead of variation-name-only checks. The props use the `?? (nullish coalescing) pattern so existing callers are unaffected — the route override is only applied when explicitly provided.

Files changed

  • app/src/components/variation/index.tsx — pass isRegistryRoute / isTaskExecutionRoute to all child components
  • app/src/components/variation/chart.tsx — accept and use route-aware props for layout, labels, and display names
  • app/src/components/variation/table.tsx — accept and use route-aware isRegistryRoute
  • app/src/components/variation/package-count-table.tsx — accept and use route-aware isRegistryRoute
  • app/src/components/variation/process-count-table.tsx — accept and use route-aware isRegistryRoute
  • app/src/components/history-chart.tsx — accept and use route-aware props for unit labels and formatting

Co-authored-by: Darcy Clarke darcy@darcyclarke.me

When the variation is 'average', child components (VariationChart,
VariationTable, HistoryChart, PackageCountTable, ProcessCountTable)
could not determine whether they were rendering registry, task-runner,
or package-manager data — they only checked the variation name itself
via isRegistryVariation()/isTaskExecutionVariation(), both of which
return false for 'average'.

This caused the registries and task-runners average pages to render
with package-manager formatting:
- Grouped bar charts instead of horizontal per-fixture bars
- PM display names (e.g. 'npm') instead of registry hostnames
  (e.g. 'registry.npmjs.org')
- Version strings shown where they should be hidden (registries)
- No baseline highlighting for npm on registry pages
- Wrong Y-axis labels and data units

Fix by adding optional isRegistryRoute/isTaskExecutionRoute props to
all affected components. VariationPage already correctly derives these
flags from the route context — now it passes them down so child
components use route-aware detection instead of variation-name-only
checks.

Co-authored-by: Darcy Clarke <darcy@darcyclarke.me>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes incorrect rendering for the new "average" variation on registries and task-runners routes by passing route-derived context flags down to child chart/table components, so they no longer rely on variation-name-only detection (which fails for "average").

Changes:

  • Pass isRegistryRoute / isTaskExecutionRoute from VariationPage to affected child components.
  • Update VariationChart, HistoryChart, and relevant tables to use route-aware overrides via ?? fallback.
  • Align registry/task-runner layouts and formatting (labels, versions display, chart layout) for "average" pages.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
app/src/components/variation/index.tsx Passes route-derived isRegistryRoute / isTaskExecutionRoute down to child components.
app/src/components/variation/chart.tsx Uses route-aware flags for chart layout/labels/registry formatting.
app/src/components/variation/table.tsx Uses route-aware isRegistryRoute to control registry-specific formatting.
app/src/components/variation/package-count-table.tsx Uses route-aware isRegistryRoute to control registry-specific formatting.
app/src/components/variation/process-count-table.tsx Uses route-aware isRegistryRoute to control registry-specific formatting.
app/src/components/history-chart.tsx Uses route-aware flags to select correct unit/formatting behavior.
Comments suppressed due to low confidence (1)

app/src/components/variation/chart.tsx:298

  • getYAxisDomain is still keyed off currentVariation (string) and internally uses isTaskExecutionVariation(currentVariation) to bucket variations. On task-runner routes where currentVariation can be "average", the new isTaskExecutionRoute override makes the chart render as task-execution, but Y-axis domain calculations will still treat "average" as non-task-execution, causing the "Consistent Scale" toggle to compute the domain against the wrong variation set.
  const isTaskExecution = isTaskExecutionRoute ?? isTaskExecutionVariation(currentVariation);
  const yAxisLabel = isTaskExecution
    ? "Time (seconds)"
    : isPerPackage
      ? "Time (ms per package)"
      : "Time (seconds)";

  // Calculate Y-axis domain for consistent scaling
  const yAxisDomain = getYAxisDomain(
    variationData,
    filteredPackageManagers,
    chartData,
    isPerPackage,
    currentVariation,
  );

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@darcyclarke
darcyclarke merged commit a7d0c90 into main Jul 29, 2026
10 checks passed
@darcyclarke
darcyclarke deleted the darcy/fix-averages branch July 29, 2026 07:32
darcyclarke added a commit that referenced this pull request Jul 29, 2026
#135)

The Performance Over Time history chart was showing package manager average
data on the registry and task-runner average pages. While PR #134 fixed the
bar charts by passing isRegistryRoute/isTaskExecutionRoute props, the
underlying history data was still using the PM-only synthetic average.

Changes:
- useHistoryData: Extract the average computation into a reusable helper and
  compute three synthetic averages: 'average' (PM variations),
  'registryAverage' (registry-clean, registry-lockfile), and
  'taskRunnerAverage' (build, build-cache, run)
- HistoryChart: When on the average variation, look up the route-specific
  synthetic key (registryAverage or taskRunnerAverage) instead of always
  using the PM average

Co-authored-by: vltbaudbot <264001112+vltbaudbot@users.noreply.github.com>
Co-authored-by: Darcy Clarke <darcy@darcyclarke.me>
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.

4 participants