Skip to content

feat(scorecard): add entity-page sparkline charts for time-series metrics - #4573

Open
Eswaraiahsapram wants to merge 3 commits into
redhat-developer:mainfrom
Eswaraiahsapram:feat/scorecard-entity-sparkline-cards-ui
Open

feat(scorecard): add entity-page sparkline charts for time-series metrics#4573
Eswaraiahsapram wants to merge 3 commits into
redhat-developer:mainfrom
Eswaraiahsapram:feat/scorecard-entity-sparkline-cards-ui

Conversation

@Eswaraiahsapram

@Eswaraiahsapram Eswaraiahsapram commented Sep 3, 2026

Copy link
Copy Markdown
Member

Hey, I just made a Pull Request!

What

Adds sparkline (area chart) visualization support for entity-page scorecard metrics whose defaultVisualization is sparkline. This is the foundation PR — shared chart components and utilities are included here and will be reused by the homepage sparkline PR that follows.

What changed

New components

  • SparklineChart — Recharts-based area chart with gradient fill, error-dot markers, hover tooltip, and threshold legend
  • SparklineTooltip / SparklineLegend — supporting chart sub-components
  • EntitySparklineCard — entity-page card that fetches time-series data and renders a sparkline with a "View data sources" dialog for collector metadata
  • EntityMetricCard — routing component that renders EntitySparklineCard or the existing Scorecard card based on defaultVisualization

New API methods

  • getMetricTimeSeriesGET /metrics/catalog/:kind/:namespace/:name/time-series
  • getMetricCollectorsGET /metrics/:metricId/collectors

New hooks

  • useMetricTimeSeriesuseQuery-based hook for 30-day entity metric time series
  • useMetricCollectorsuseQuery-based hook for collector metadata (fetched only when the data-sources dialog is open)

New utilities

  • timeSeriesChartData — maps API points to chart-ready data with interpolation for error gaps
  • sparklineLegend — builds threshold legend items with color + line-style pairing
  • sparklineChartModel — shared view-model factory used by both entity and homepage cards
  • metricVisualizationisSparklineVisualization() helper
  • timeSeriesRange — computes the default 30-day ISO-8601 range

Refactors

  • DataSourcesDialog now accepts generic SourceRow[] instead of building rows internally
  • Extracted collectorSourceRows.ts (for sparkline metrics) and metricSourceRows.ts (for existing donut metrics) as separate row builders
  • All collector labels (GitHub, Jira, empty value --, unavailable status N/A) are now translated via i18n keys instead of hardcoded strings

Translations

  • Added 6 new dataSourcesDialog.* keys to ref.ts and all locale files (de, es, fr, it, ja)

How to test

  1. Configure a catalog entity with DORA metric providers (or any metric with defaultVisualization: sparkline)
  2. Navigate to the entity's Scorecard tab
  3. Verify the sparkline chart renders with a 30-day trend line
  4. Click the menu → "View data sources" and verify collectors are listed
  5. Error days should show red dots on the chart with tooltip messages

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or Updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)

@rhdh-gh-app

rhdh-gh-app Bot commented Sep 3, 2026

Copy link
Copy Markdown

Important

This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior.

Changed Packages

Package Name Package Path Changeset Bump Current Version
@red-hat-developer-hub/backstage-plugin-scorecard workspaces/scorecard/plugins/scorecard minor v4.2.0

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:36 PM UTC · Completed 7:44 PM UTC

Commit: b56fa91 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Cost: $2.16

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 3, 2026

Copy link
Copy Markdown

Review

Findings

Medium

  • [error-handling-idiom] workspaces/scorecard/plugins/scorecard/src/hooks/useMetricCollectors.tsx — Every existing hook wraps queryFn in a try-catch that converts non-Error throwables to a translated message (e.g., t('errors.fetchError', { error: String(err) })). useMetricCollectors passes a bare queryFn with no error wrapping, breaking the established error-handling pattern. useMetricTimeSeries in the same PR correctly follows this pattern.

  • [Hook parameter convention] workspaces/scorecard/plugins/scorecard/src/hooks/useMetricCollectors.tsx — All existing hooks that accept an enabled flag use a destructured options-object pattern (e.g., useAggregatedScorecard({ aggregationId, enabled })). useMetricCollectors uses positional parameters (metricId, enabled), diverging from the established convention.

  • [stale-api-report] workspaces/scorecard/plugins/scorecard/report.api.md — The main report.api.md is missing 5 new translation keys (collectorStatusTooltip, collectorEmptyValue, collectorUnavailableStatus, pluginGithub, pluginJira) that were added to report-alpha.api.md and report-legacy.api.md. The report jumps directly from dataSourcesDialog.statusTooltip to dataSourcesDialog.columns.plugin.
    Remediation: Regenerate report.api.md by running the API Extractor.

  • [missing-doc] workspaces/scorecard/plugins/scorecard/README.md — The README Features section lists four features but does not mention sparkline chart visualization. This is a user-visible feature that administrators and integrators should know about.
    Remediation: Add a fifth bullet to the Features list describing sparkline chart support for time-series metrics.

Low

  • [edge-case] workspaces/scorecard/plugins/scorecard/src/utils/timeSeriesChartData.ts:136toAggregationSparklinePoints determines error labels based solely on point.status === 'error'. If a point has status: 'success' but value: null, no error label is set, causing the tooltip to display null. By contrast, toMetricSparklinePoints defensively checks point.value === null as a fallback.

  • [api-surface] workspaces/scorecard/plugins/scorecard/report-alpha.api.mdpluginGithub and pluginJira translation keys are hardcoded to two specific collector providers. Future providers would need new keys, though a fallback to extractPluginName exists for unknown prefixes.

  • [pattern-inconsistency] workspaces/scorecard/plugins/scorecard/src/api/index.tsgetMetricCollectors encodes metricId with encodeURIComponent, but getAggregationTimeSeries and getMetricTimeSeries do not encode their path segments. This is consistent with the pre-existing codebase pattern (most methods do not encode), making getMetricCollectors the outlier.

  • [naming-convention] workspaces/scorecard/plugins/scorecard/dev/mocks.ts — Type-only symbols (ScorecardApi, ScorecardOptions, etc.) imported with value import instead of import type. Sibling file legacy.tsx uses import type for the same symbols.

  • [interface-extension] workspaces/scorecard/plugins/scorecard/src/api/types.ts — Three new mandatory methods added to the ScorecardApi interface. Not part of the declared public API surface; any downstream consumer that deep-imports this internal interface will get clear compile errors at the missing methods. Minor version bump correctly signals additive changes.

  • [behavioral-change] workspaces/scorecard/plugins/scorecard/src/utils/translationUtils.tsextractPluginName regex changed from split('.') to split(/[.:]/) to handle colon-separated collector IDs. Internal function, backward-compatible for dot-separated IDs.

  • [internal-component-contract] workspaces/scorecard/plugins/scorecard/src/components/MetricGroupCard/DataSourcesDialog.tsxDataSourcesDialogProps interface refactored: metrics replaced with rows, isLoading, error, buckets. Internal component, not part of public API. Row computation now happens in callers (MetricGroupCard and EntitySparklineCard).


Labels: Feature PR adding new sparkline chart capability to the scorecard workspace.

Previous run

Review

Verdict: comment

This PR adds sparkline (area chart) visualization support for entity-page scorecard metrics, including new chart components, API methods, hooks, utilities, and i18n strings. The architecture is well-structured: components follow the existing project patterns, hooks use the established useQuery + UseResponseData<T> pattern, and the code includes comprehensive test coverage across all new modules. The security posture is clean — all data rendering goes through React's auto-escaping, API calls use Backstage's authenticated fetch wrapper, and input validation is present on all new API methods.

Two medium-severity findings require attention before merge. Several low-severity items are noted for consideration.


Medium

1. Stale API report — report.api.md not regenerated

File: workspaces/scorecard/plugins/scorecard/report.api.md

The PR updates report-alpha.api.md and report-legacy.api.md with 5 new dataSourcesDialog.* translation keys (collectorStatusTooltip, collectorEmptyValue, collectorUnavailableStatus, pluginGithub, pluginJira), but report.api.md is not updated. This file exports the same scorecardTranslationRef and is now inconsistent with the other two reports.

Remediation: Run the API report generation command (e.g., yarn backstage-repo-tools api-reports) to regenerate report.api.md.

2. Breaking ScorecardApi interface — dev mocks will not compile

File: workspaces/scorecard/plugins/scorecard/src/api/types.ts (line ~105)

Three new required methods are added to the ScorecardApi interface (getAggregationTimeSeries, getMetricTimeSeries, getMetricCollectors). The MockScorecardApi classes in dev/mocks.ts (line 57) and dev/legacy.tsx (line 82) both implements ScorecardApi but are not updated with these methods, causing TypeScript compilation errors in the dev environment.

Remediation: Add stub implementations for the three new methods to both MockScorecardApi classes. Alternatively, consider making the new methods optional on the interface if downstream consumers implement ScorecardApi directly.


Low

3. URL construction inconsistency in getMetricCollectors

getMetricCollectors uses a template literal with explicit encodeURIComponent() while every other method in the class (including the two new time-series methods) uses new URL(). This creates a split pattern for URL construction within the same class.

4. Hardcoded 'DORA' in collectorStatusTooltip translation

The collectorStatusTooltip string hardcodes "DORA" but the tooltip displays for any metric with isCollector: true, not only DORA metrics. Consider using a generic term or a translation interpolation variable.

5. useMetricCollectors hook missing error-wrapping pattern

Unlike the sibling hooks (useMetricTimeSeries, useAggregatedScorecard), useMetricCollectors does not wrap its queryFn in a try/catch with translated error messages. The practical risk is low since the API client handles errors internally, but it breaks the convention.

6. getAggregationTimeSeries has no caller in this PR

The method is defined, tested, and added to the interface, but no hook or component in this PR calls it. The PR body notes a follow-up homepage sparkline PR will use it. Consider whether this uncalled method belongs in this PR or the follow-up.

7. Test gap — no test for collector fetch error

EntitySparklineCard.test.tsx covers loading state, time-series fetch errors, empty data, and successful rendering, but does not test the path where useMetricCollectors returns an error.

8. README not updated with sparkline feature

The README Features section does not mention sparkline time-series charts. The changeset describes the feature, but a README update would help users discover it.

9. extractPluginName regex change

The split regex changed from '.' to /[.:]/ to support colon-delimited collector IDs (e.g., github:deploymentWorkflowRuns). This is functionally correct and tested, but the behavioral change to an existing utility is not called out in the PR description.

10. Hardcoded pluginGithub/pluginJira translation keys

The pluginLabels map must be manually extended for each new collector integration. The fallback (extractPluginName) already capitalizes the first segment, so these keys only improve brand-name casing.

Previous run (2)

Review

Verdict: comment — medium-severity findings worth noting but none that should block.

Summary

This PR adds sparkline chart support to the scorecard entity page, enabling time-series visualization for DORA and other metrics alongside the existing score donut cards. The change spans 52 files (+4101/−315) across API client extensions, new React components, custom hooks, utility functions, translations, and comprehensive tests.

Architecture is clean: EntityMetricCard acts as a visualization router (sparkline vs. donut), EntitySparklineCard wires up data fetching and chart rendering, and SparklineChart is a reusable Recharts wrapper. The DataSourcesDialog refactoring from raw MetricResult[] to pre-built SourceRow[] improves separation of concerns and enables collector-based data source rows.

Test coverage is strong — ~20 new test files cover API client methods, hooks, utility functions, chart components, and the integration between EntitySparklineCard and data source dialogs.

Findings

1. Missing changeset [medium · process]

File: (repository root — no .changeset/*.md file present)

The PR adds a user-visible feature (feat prefix) but includes no changeset. Per CONTRIBUTING.md and .fullsend/AGENTS.md, a changeset with minor bump level is expected for new features. The PR checklist also shows all items unchecked.

Remediation: Add a changeset via npx changeset selecting the @red-hat-developer-hub/backstage-plugin-scorecard package with a minor bump.

2. Entity path segments not URL-encoded in getMetricTimeSeries [low · defense-in-depth]

File: workspaces/scorecard/plugins/scorecard/src/api/index.ts (new method getMetricTimeSeries)

The URL is built with entity kind, namespace, and name interpolated directly into the path:

const url = new URL(
  `${baseUrl}/metrics/catalog/${entity.kind}/${entity.metadata.namespace}/${entity.metadata.name}/time-series`,
);

While entity names from the Backstage catalog are typically safe, this is inconsistent with getMetricCollectors which properly uses encodeURIComponent(metricId). If an entity name contained / or other URL-special characters, the URL would be malformed.

Note: This pattern is consistent with existing methods in the same file (e.g., getScorecards), so it is pre-existing rather than introduced by this PR.

Remediation: Consider wrapping path segments with encodeURIComponent() for defense-in-depth:

`${baseUrl}/metrics/catalog/${encodeURIComponent(entity.kind)}/${encodeURIComponent(entity.metadata.namespace)}/${encodeURIComponent(entity.metadata.name)}/time-series`

3. Hardcoded plugin labels may not scale [low · maintainability]

File: workspaces/scorecard/plugins/scorecard/src/translations/ref.ts and collectorSourceRows.ts

Plugin labels for collectors (pluginGithub, pluginJira) are hardcoded as translation keys and passed via a pluginLabels map. When a new collector provider is added (e.g., PagerDuty, GitLab), a new translation key and mapping would need to be added manually. The fallback to extractPluginName handles unknown providers by capitalizing the prefix from the collector ID, which is reasonable, but the hardcoded map adds ongoing maintenance.

Remediation: Consider whether the extractPluginName fallback alone is sufficient, or document the pattern for adding new collector providers.

What looks good

  • Clean component extraction: EntityMetricCard centralizes the visualization-type routing, eliminating duplicated status/translation logic from both EntityScorecardContent and ScorecardEntityContentGridView.
  • Smart data fetching: useMetricCollectors is gated by enabled so collector data is only fetched when the data-sources dialog opens and the metric has collector IDs — no wasted requests.
  • Robust chart data handling: The interpolatePlotValue function linearly interpolates null/error points to keep the sparkline continuous, with proper edge-case handling (no prev, no next, both missing).
  • Comprehensive i18n: All 5 new translation keys are added across all 7 supported languages (en, de, es, fr, it, ja, ref).
  • Thorough test coverage: API client, hooks, utility functions, and component integration are all well-tested with edge cases.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Sep 3, 2026
@Eswaraiahsapram
Eswaraiahsapram force-pushed the feat/scorecard-entity-sparkline-cards-ui branch from b56fa91 to 3dd8c8b Compare September 4, 2026 08:51
@fullsend-ai-review

fullsend-ai-review Bot commented Sep 4, 2026

Copy link
Copy Markdown

🤖 Review · ❌ Terminated · Started 8:53 AM UTC · Ended 9:09 AM UTC

Commit: 3dd8c8b · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels Sep 4, 2026
@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:53 AM UTC · Completed 9:09 AM UTC

Commit: 3dd8c8b · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Cost: $9.51

@sonarqubecloud

sonarqubecloud Bot commented Sep 4, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
3.9% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 4, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:11 AM UTC · Completed 9:53 AM UTC

Commit: 46c6489 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Cost: $13.56

@fullsend-ai-review fullsend-ai-review Bot 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.

See the review comment for full details.

/**
* Maps API time-series points into chart rows. Error / null values keep their
* x position and get an interpolated Y so the sparkline stays continuous.
*/

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[low] edge-case

toAggregationSparklinePoints determines error labels based solely on point.status === 'error'. If a point has status 'success' but value null, no error label is set. toMetricSparklinePoints defensively checks point.value === null as a fallback.

Suggested fix: Add fallback in toAggregationSparklinePoints to handle null values when status is not 'error'.

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment enhancement New feature or request and removed requires-manual-review Review requires human judgment labels Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request requires-manual-review Review requires human judgment workspace/scorecard

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant