RHIDP-14178 As a Platform Engineer, I want to configure filters for aggregation scorecard cards by imykhno · Pull Request #4125 · redhat-developer/rhdh-plugins

imykhno · 2026-07-31T10:36:27Z

Hey, I just made a Pull Request!

Implemented logic to support filter by status for scalar aggregation types (sum, average, count, min, max).

This PR for:

https://redhat.atlassian.net/browse/RHIDP-14178

✔️ 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)

How to test

Preparing

Add configuration to the app-config.local.yaml (or app-config.yaml) under scorecard.aggregationKPIs:

scorecard:
  aggregationKPIs:
    totalCriticalPRs:
      title: Total Critical PRs
      description: Sum of open PRs for entities in error status
      type: sum
      metricId: github.openPRs
      filter:
        status: error

Set environment variables for convenience

export BACKSTAGE_URL="http://localhost:7007"
export TOKEN="your-auth-token-here"

Testing

Get filtered by status aggregation result

In this test case expected that:

scorecard.aggregationKPIs.totalCriticalPRs.type should be any of scalar type (sum, average, count, min, max)
scorecard.aggregationKPIs.totalCriticalPRs.filter should be one of configured thresholds.

curl -X GET "${BACKSTAGE_URL}/api/scorecard/aggregations/totalCriticalPRs" \
  -H "Authorization: Bearer ${TOKEN}" | jq

Get error when filter by status is not follow configured thresholds

In this test case expected that:

scorecard.aggregationKPIs.totalCriticalPRs.type should be any of scalar type (sum, average, count, min, max)
scorecard.aggregationKPIs.totalCriticalPRs.filter shouldn't be one of configured thresholds (for example enter status: unreal).

curl -X GET "${BACKSTAGE_URL}/api/scorecard/aggregations/totalCriticalPRs" \
  -H "Authorization: Bearer ${TOKEN}" | jq

Get aggregation result when `statusGrouped` or `weightedStatusScore` aggregation types were used and filters configured

In this test case expected that:

scorecard.aggregationKPIs.totalCriticalPRs.type should be any of scalar type (statusGrouped, weightedStatusScore)
scorecard.aggregationKPIs.totalCriticalPRs.filter should be one of configured thresholds.

curl -X GET "${BACKSTAGE_URL}/api/scorecard/aggregations/totalCriticalPRs" \
  -H "Authorization: Bearer ${TOKEN}" | jq

Result: returned data should not be affected by configured filters

rhdh-gh-app · 2026-07-31T10:36:49Z

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-backend	workspaces/scorecard/plugins/scorecard-backend	minor	`v4.1.0`
@red-hat-developer-hub/backstage-plugin-scorecard-common	workspaces/scorecard/plugins/scorecard-common	minor	`v4.1.0`
@red-hat-developer-hub/backstage-plugin-scorecard	workspaces/scorecard/plugins/scorecard	minor	`v4.1.0`

fullsend-ai-review · 2026-07-31T10:37:21Z

🤖 Finished Review · ✅ Success · Started 10:37 AM UTC · Completed 10:57 AM UTC
Commit: 6ba2b13 · View workflow run →

codecov · 2026-07-31T10:41:41Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.14%. Comparing base (0f32982) to head (8a9dca8).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4125      +/-   ##
==========================================
+ Coverage   58.11%   58.14%   +0.02%     
==========================================
  Files        2422     2426       +4     
  Lines       96484    96536      +52     
  Branches    26885    26890       +5     
==========================================
+ Hits        56075    56127      +52     
  Misses      38914    38914              
  Partials     1495     1495

Flag	Coverage Δ		*Carryforward flag
adoption-insights	`84.55% <ø> (ø)`		Carriedforward from 0f32982
ai-integrations	`69.76% <ø> (ø)`		Carriedforward from 0f32982
app-defaults	`69.79% <ø> (ø)`		Carriedforward from 0f32982
augment	`46.67% <ø> (ø)`		Carriedforward from 0f32982
boost	`76.77% <ø> (ø)`		Carriedforward from 0f32982
bulk-import	`72.56% <ø> (ø)`		Carriedforward from 0f32982
cost-management	`13.55% <ø> (ø)`		Carriedforward from 0f32982
dcm	`60.72% <ø> (ø)`		Carriedforward from 0f32982
extensions	`56.59% <ø> (ø)`		Carriedforward from 0f32982
global-floating-action-button	`71.18% <ø> (ø)`		Carriedforward from 0f32982
global-header	`66.50% <ø> (ø)`		Carriedforward from 0f32982
homepage	`47.59% <ø> (ø)`		Carriedforward from 0f32982
install-dynamic-plugins	`59.95% <ø> (ø)`		Carriedforward from 0f32982
intelligent-assistant	`74.59% <ø> (ø)`		Carriedforward from 0f32982
konflux	`91.98% <ø> (ø)`		Carriedforward from 0f32982
lightspeed	`69.02% <ø> (ø)`		Carriedforward from 0f32982
mcp-integrations	`83.40% <ø> (ø)`		Carriedforward from 0f32982
orchestrator	`66.87% <ø> (ø)`		Carriedforward from 0f32982
quickstart	`63.74% <ø> (ø)`		Carriedforward from 0f32982
sandbox	`79.56% <ø> (ø)`		Carriedforward from 0f32982
scorecard	`86.17% <100.00%> (+0.18%)`	⬆️
theme	`88.77% <ø> (ø)`		Carriedforward from 0f32982
translations	`5.12% <ø> (ø)`		Carriedforward from 0f32982
x2a	`79.20% <ø> (ø)`		Carriedforward from 0f32982

*This pull request uses carry forward flags. Click here to find out more.

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0f32982...8a9dca8. Read the comment docs.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

fullsend-ai-review · 2026-07-31T10:57:45Z

Review

Findings

Low

[edge-case] workspaces/scorecard/plugins/scorecard-backend/src/database/DatabaseMetricValues.ts:498 — The filter condition if (filter?.status) uses JavaScript truthiness. If a caller bypasses config parsing and passes { status: '' }, the filter would be silently skipped. In practice, the Zod schema enforces min(1) and buildAggregationConfigFilter uses getOptionalString, so an empty string cannot reach this code path through any supported flow.
[naming-convention] workspaces/scorecard/plugins/scorecard-backend/src/validation/validateScalarAggregationConfig.ts:49 — Accepts ValidatedAggregationConfig then casts to ScalarAggregationConfig internally. Since the caller already narrows via isScalarAggregationType, the input type could be ScalarAggregationConfig directly, eliminating the cast.
[naming-convention] workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfigFilter.ts:27 — Returns empty object {} and uses Object.keys().length check for absence, while the sister utility buildAggregationConfigThresholds returns undefined. Inconsistent absence-signaling pattern between the two utilities, though both work correctly.
[scope-creep] workspaces/scorecard/plugins/scorecard-backend/src/utils/normalizeTimestamp.ts — The change from new Date() to new Date(0) for undefined/null inputs is a behavioral change that affects all aggregation types, not just filtered scalar KPIs. This is a necessary prerequisite for the filter feature (when filter matches no rows, the timestamp must degrade gracefully to epoch so mergeMaxTimestamp picks the real portfolio timestamp). Edge-case API responses now return 1970-01-01T00:00:00.000Z instead of current time for empty aggregation results — more semantically correct.
[scope-creep] workspaces/scorecard/plugins/scorecard-backend/src/validation/validateScalarAggregationConfig.ts:42 — Boolean metric relaxation for count type is a new behavioral change not mentioned in the PR description. count on boolean metrics is reasonable (COUNT(*) does not depend on numeric values), but the changeset only mentions filter.status.

Previous run

Review

Findings

Low

[scope-creep] workspaces/scorecard/plugins/scorecard-backend/src/utils/normalizeTimestamp.ts — Several changes go beyond the filter.status feature scope: (1) changing normalizeTimestamp to return epoch instead of current time for undefined/null inputs; (2) removing toIsoTimestamp.ts; (3) renaming test fixtures (mockFirstThresholds → mockHigherIsBetterThresholds); (4) renaming MetricValueCreateInput → DbMetricValueCreateInput; (5) caching fallback config in AggregationsService; (6) allowing count on boolean metrics; (7) switching Error to InputError in validation; (8) extracting thresholdsConfigSchema to its own file; (9) replacing ConfigReader with mockServices.rootConfig in tests. Most are reasonable companions to the filter feature, but items (5) and (6) are independent functional changes that could warrant separate commits or acknowledgment in the PR description.
[edge-case] workspaces/scorecard/plugins/scorecard-backend/src/database/DatabaseMetricValues.ts:157 — The filter condition if (filter?.status) uses a truthy check, so an empty string would bypass the filter. The Zod schema (z.string().min(1).max(64)) and startup validation prevent empty strings from reaching this point, so there is no runtime risk. A strict !== undefined check would express intent more clearly.
[conditional-property-inclusion] workspaces/scorecard/plugins/scorecard-backend/src/service/mappers.ts:819 — The filter is conditionally added via if ('filter' in aggregationConfig && aggregationConfig.filter). The 'filter' in check provides runtime narrowing for the discriminated union but is functionally redundant alongside the truthy check.
[type-assertion-pattern] workspaces/scorecard/plugins/scorecard-backend/src/validation/validateScalarAggregationConfig.ts:48 — Uses aggregationConfig as ScalarAggregationConfig type assertion. The caller already guards with isScalarAggregationType, making the cast safe, though the existing isScalarAggregationConfig type guard could narrow the type without a cast.
[internal-API-signature-change] workspaces/scorecard/plugins/scorecard-backend/src/validation/validateAggregationConfig.ts:24 — The function now requires a thresholdResolver parameter. Internal to the backend plugin and not part of the public API, so out-of-tree impact is minimal.
[internal-type-rename] workspaces/scorecard/plugins/scorecard-backend/src/database/utils/mapMetricValueRow.ts:59 — MetricValueCreateInput renamed to DbMetricValueCreateInput, improving consistency with other Db-prefixed database types. Internal to the backend plugin.

Previous run (2)

Review

Findings

High

[stale-reference] workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md — The PR renames the heading ## Aggregation KPIs (homepage and \GET /aggregations`)to## Aggregation KPIsinscorecard-backend/README.md, changing the anchor from #aggregation-kpis-homepage-and-get-aggregationsto#aggregation-kpis. But aggregation.mdstill links to the old anchor in at least two places. These links will be broken after merge. Remediation: Update all references from../README.md#aggregation-kpis-homepage-and-get-aggregationsto../README.md#aggregation-kpis`.
[stale-reference] workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md — thresholds.md links to ../README.md#aggregation-kpis-homepage-and-get-aggregations in the "Further reading" lines for both §4 (weightedStatusScore thresholds) and the new §5 (scalar type thresholds). The target heading was renamed in the same PR, so these anchors will not resolve.
Remediation: Update all references to use the new anchor #aggregation-kpis.
[stale-reference] workspaces/scorecard/plugins/scorecard/README.md — Links to ../scorecard-backend/README.md#aggregation-kpis-homepage-and-get-aggregations. The target heading was renamed in the same PR.
Remediation: Update the link to use the new anchor #aggregation-kpis.

Medium

[breaking-api] workspaces/scorecard/plugins/scorecard-common/src/types/aggregation.ts:148 — The statusScores field on the exported AggregationConfigOptions type changed from required to optional. Downstream consumers typing against this public type will see a compile error. While the Zod schema guards against this for weightedStatusScore configs internally, external consumers are affected.
Remediation: Keep statusScores required in AggregationConfigOptions and define a separate type for scalar config options, or document the breaking change in the changelog.
[consumer-completeness] workspaces/scorecard/plugins/scorecard/src/components/AggregatedMetricCards/AggregatedMetricCard.tsx:44 — The AggregatedMetricCard frontend component only handles statusGrouped and weightedStatusScore. The five new scalar types will fall through to UnsupportedAggregationType, rendering a "not supported" message to users if deployed before the frontend is updated.
Remediation: Ensure the frontend is updated to handle scalar aggregation types before this backend change is deployed, or document the deployment dependency.

Low

[design-consideration] workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregationsService.ts:58 — The aggregationKpisConfigCache never has entries invalidated. Standard for Backstage app-config (no hot-reload support), but worth a comment for maintainability.
[behavioral-change] workspaces/scorecard/plugins/scorecard-backend/src/utils/normalizeTimestamp.ts:27 — The normalizeTimestamp fallback changed from new Date() (current time) to new Date(0) (epoch). Arguably more correct for "no data" scenarios but is a behavior change for the existing status-grouped API response.
[backward-incompatible] workspaces/scorecard/plugins/scorecard-common/src/types/aggregation.ts:124 — AggregationResultByType union extended with ScalarAggregationResult. Consumers with exhaustive switch/if-else chains will need to handle the new case. Expected for a minor version bump.
[breaking-schema] workspaces/scorecard/plugins/scorecard-backend/config.d.ts:19 — Config schema changed from single object type to three-branch discriminated union. Existing YAML remains valid; verify Backstage config resolver handles it.
[naming-convention] workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/types.ts:19 — Uses import { type ... } while the codebase convention is import type { ... }.
[naming-convention] workspaces/scorecard/plugins/scorecard-backend/src/service/mappers.ts:27 — Inconsistent import grouping: DbAggregatedMetric as value import, DbScalarAggregatedMetric as type import, both from the same module.
[naming-convention] workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/parseValidatedAggregationConfig.ts:34 — Redundant template literal: throw new InputError(`${errorMessage}`) — simplify to throw new InputError(errorMessage).
[code-organization] workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfigFilter.ts:28 — Type assertion as AggregationConfigFilter unnecessary for empty object; use type annotation.
[edge-case] workspaces/scorecard/plugins/scorecard-backend/src/database/DatabaseMetricValues.ts:157 — filter?.status is falsy for empty strings; safe due to upstream Zod validation (min(1)) but defense-in-depth relies on upstream.
[architectural-coherence] workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregationsService.ts:43 — MetricProvidersRegistry passed as method parameter rather than constructor dependency, inconsistent with other service dependencies.

Next steps:

/fs-fix — agent addresses review findings automatically
/fs-fix <your instruction> — agent fixes with your specific guidance
Push commits directly — review re-runs automatically on push
/fs-fix-stop — disable automatic fix runs for this PR

Previous run (3)

Review

Findings

Medium

[edge-case] workspaces/scorecard/plugins/scorecard-backend/src/service/mappers.ts:68 — The normalizeTimestamp refactor changes the fallback timestamp from new Date() (current time) to new Date(0) (epoch) when aggregatedMetric is undefined. The old code fell back to new Date().toISOString(). With normalizeTimestamp(undefined) returning new Date(0), the empty-entities path (called from AggregatedMetricLoader when entityRefs.length === 0) now returns timestamp 1970-01-01T00:00:00.000Z instead of the current time. Any downstream consumers or frontend displays that check for a meaningful timestamp will display an epoch date.
Remediation: If epoch is intended as a sentinel for "no data," document this decision. Otherwise, handle the undefined case explicitly to preserve the old new Date() fallback.
[api-contract] workspaces/scorecard/plugins/scorecard/src/components/AggregatedMetricCards/AggregatedMetricCard.tsx:53 — The frontend AggregatedMetricCard component dispatches on aggregationType and only handles statusGrouped and weightedStatusScore. All five new scalar aggregation types (sum, average, max, min, count) will fall through to UnsupportedAggregationType, rendering an "unsupported aggregation type" message. A platform engineer configuring a scalar aggregation KPI will see this error in the UI.
Remediation: Add a scalar card component that renders the ScalarAggregationResult. If frontend work is intentionally deferred, document the limitation.

Low

[edge-case] workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregationsService.ts:78 — The getAggregationConfig method caches validated configs but the default (no-config) path does not cache, meaning it calls metricProviderRegistry.getMetric() and logs a warning on every request for unconfigured aggregation IDs.
[edge-case] workspaces/scorecard/plugins/scorecard-backend/src/database/DatabaseMetricValues.ts:160 — When aggregationFn is count, both the SQL value column and total column resolve to COUNT(*), making them always identical. Semantically correct for count aggregation but may confuse API consumers who expect total to represent all eligible entities.
[scope-creep] workspaces/scorecard/plugins/scorecard-backend/src/database/types.ts — The PR bundles a snake_case-to-camelCase refactoring of DbMetricValue, DbMetricValueCreate, and DbAggregatedMetric types with the filter feature. This cascades through multiple files and broadens the diff. Consider splitting into a separate PR.
[scope-creep] workspaces/scorecard/app-config.yaml:63 — The existing openPrsWeightedKpi title was renamed from "GitHub Open PRs (weighted health)" to "GitHub Open PRs KPI (weighted health)," unrelated to the scalar filter feature.
[design-smell] workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfigThresholds.ts — The threshold rule color field changed from required to optional across all aggregation types, not just scalar. This relaxation affects existing weightedStatusScore configs. Confirm this is intentional for all types; if only scalar types should allow optional color, use separate schemas.
[backward-incompatible] workspaces/scorecard/plugins/scorecard-common/src/types/aggregation.ts:109 — The statusScores property in AggregationConfigOptions changed from required to optional. The parent options field was already optional, so external consumers already needed nil-checks, but this is still a contract change in a shared package.
[naming-convention] workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfig.ts:46 — Unconditional aggregationConfig.options = {} assignment, including for statusGrouped configs where no options are needed. The Zod schema strips the empty object, but this departs from the established pattern.
[import-style-consistency] workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/types.ts:17 — Import changed from import type { ... } to import { type ... } while only types are imported. The codebase consistently uses the former pattern.
[error-handling-idiom] workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/parseValidatedAggregationConfig.ts:34 — Redundant template literal ${errorMessage} in throw new InputError(...). Use throw new InputError(errorMessage) directly.
[naming-convention] workspaces/scorecard/plugins/scorecard-backend/src/database/utils/mapMetricValueRow.ts:48 — DbMetricValueCreateInput naming does not follow the established Db*Create / Db* convention.
[pattern-inconsistency] workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfigFilter.ts:20 — Uses {} as AggregationConfigFilter (type assertion) instead of const filterConfig: AggregationConfigFilter = {} (type annotation).
[naming-convention] workspaces/scorecard/plugins/scorecard-backend/config.d.ts:24 — Imports a runtime value (aggregationTypes) into a .d.ts file. The rest of the file imports only types.
[scope-creep] workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregationsService.ts — File renamed from AggregationService.ts to AggregationsService.ts (pluralization). Minor housekeeping bundled with the feature.
[additive-api-surface] workspaces/scorecard/plugins/scorecard-common/src/types/aggregation.ts:51 — New public types added: ScalarAggregatedMetric, AggregationConfigFilter, ScalarAggregationResult, StatusScoreAggregationOption, scalarAggregationTypes. All purely additive and backward-compatible.
[SQL query construction] workspaces/scorecard/plugins/scorecard-backend/src/database/utils/getAggregateExpression.ts:19 — SQL string interpolation of numericValueExpr and aggregationFn parameters. Both are derived from hardcoded values (database-client-type check and TypeScript union type). No user input reaches either parameter. Safe, but worth noting as a pattern to watch.

Next steps:

/fs-fix — agent addresses review findings automatically
/fs-fix <your instruction> — agent fixes with your specific guidance
Push commits directly — review re-runs automatically on push
/fs-fix-stop — disable automatic fix runs for this PR

Previous run (4)

Review

Findings

Medium

[logic-error] workspaces/scorecard/plugins/scorecard-backend/src/validation/validateScalarAggregationConfig.ts:106 — The validation rejects ALL scalar aggregation types (including count) when the target metric is boolean. However, count uses COUNT(*) (counting rows, not summing values), so it is semantically valid for boolean metrics. A platform engineer might want to count entities with a boolean metric (e.g., "count of entities with a license file").
Remediation: Exclude count from the boolean-metric check, e.g., if (metric.type === 'boolean' && aggregationConfig.type !== aggregationTypes.count).
[scope-creep] workspaces/scorecard/plugins/scorecard-backend/config.d.ts — The config.d.ts schema was refactored from a single union type to a discriminated union of three separate object types (statusGrouped, weightedStatusScore, and scalar). This changes the public config schema shape and makes statusScores required only on the weightedStatusScore variant. Consider documenting this structural change in the changeset.
[broken-internal-link] workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md — The backend README heading was simplified from ## Aggregation KPIs (homepage and \GET /aggregations`)to## Aggregation KPIs, changing the anchor from #aggregation-kpis-homepage-and-get-aggregationsto#aggregation-kpis. Two links in thresholds.md still reference the old anchor. Remediation: Update both links to #aggregation-kpis`.
[broken-internal-link] workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md — Same heading rename: aggregation.md links to the old anchor #aggregation-kpis-homepage-and-get-aggregations in the API Endpoint section.
Remediation: Update the link to #aggregation-kpis.
[broken-internal-link] workspaces/scorecard/plugins/scorecard/README.md — The frontend README links to the old backend README anchor #aggregation-kpis-homepage-and-get-aggregations.
Remediation: Update the link to #aggregation-kpis.
[backward-compatibility] workspaces/scorecard/plugins/scorecard-common/src/types/aggregation.ts:130 — AggregationConfigOptions.statusScores changed from required (Record<string, number>) to optional (StatusScoreAggregationOption | undefined). This is a minor breaking change for external consumers that access .statusScores without an optional check. The minor version bump is appropriate per semver.

Low

[behavioral-change] workspaces/scorecard/plugins/scorecard-backend/src/utils/normalizeTimestamp.ts:29 — The extracted normalizeTimestamp function falls back to new Date(0) (epoch) instead of the original new Date() (current time). This is a subtle behavioral change, though the fallback path is unlikely to execute in practice. Additionally, toIsoTimestamp still falls back to new Date(), creating an inconsistency between the two timestamp utilities.
[edge-case] workspaces/scorecard/plugins/scorecard-backend/src/database/DatabaseMetricValues.ts:161 — When a status filter matches zero rows, SQL aggregate functions return NULL. Number(null) produces 0, so the API returns value: 0, total: 0. For min/max, a result of 0 could mean "no matching rows" or "the actual value is 0". The total: 0 field disambiguates but the API contract should document this.
[naming-consistency] workspaces/scorecard/plugins/scorecard-backend/src/database/utils/mapMetricValueRow.ts:62 — DbMetricValueCreateInput is structurally near-identical to DbMetricValueCreate. Consider using DbMetricValueCreate directly or extending it with only the | null widened fields.
[parameter-ordering] workspaces/scorecard/plugins/scorecard-backend/src/validation/validateScalarAggregationConfig.ts:28 — Takes four positional parameters while sibling validation functions (validateScalarFilterStatus, validateAggregationConfig) use a single destructured options object.
[unnecessary-type-assertion] workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfig.ts:40 — Object literal cast as AggregationConfig then mutated. Other builder functions in this directory return fully-formed objects without intermediate casts.
[scope-creep] workspaces/scorecard/plugins/scorecard-backend/src/database/types.ts — The snake_case-to-camelCase refactoring of DB type interfaces plus mapMetricValueRow.ts mapper layer is independent of the filter feature. Consider splitting into a separate PR to reduce review surface.
[scope-creep] workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregationsService.ts — Introduction of ValidatedAggregationConfig, Zod-based discriminated union parsing, and config cache extends beyond filter support into a type system redesign.
[additive-api-change] workspaces/scorecard/plugins/scorecard-common/src/types/aggregation.ts — New public types (ScalarAggregatedMetric, AggregationConfigFilter, ScalarAggregationResult, StatusScoreAggregationOption) and optional filter property on AggregationMetadata and AggregationConfig are purely additive and backward-compatible.
[additive-api-change] workspaces/scorecard/plugins/scorecard-common/src/constants/aggregations.ts:29 — aggregationTypes extended with five new keys (sum, average, max, min, count) and new scalarAggregationTypes array exported. Additive and backward-compatible.

Previous run (5)

Review

Findings

Low

[edge-case] workspaces/scorecard/plugins/scorecard-backend/src/database/DatabaseMetricValues.ts:176 — Number(aggregateRow.value) || 0 uses logical OR which treats NaN as falsy, silently converting malformed data to 0 without logging. While the result is coincidentally correct for zero values, ?? would be more semantically precise.
[error-handling-gap] workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfigFilter.ts:27 — Returns an empty {} object when the YAML has a filter: block but no status key. This empty filter passes Zod validation and would appear in API responses as metadata.filter: {}.
[edge-case] workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregationsService.ts:57 — aggregationKpisConfigCache caches validated configs permanently; runtime config reloads would serve stale entries.
[naming-convention] workspaces/scorecard/plugins/scorecard-backend/src/database/utils/mapMetricValueRow.ts:28 — MetricValueCreateInput departs from the Db* prefix convention used by other database layer types (DbMetricValue, DbMetricValueCreate, etc.).
[naming-convention] workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockThresholds.ts — mockFirstThresholds and mockSecondThresholds use ordinal prefixes that convey no semantic meaning about the data they contain.
[error-handling-idiom] workspaces/scorecard/plugins/scorecard-backend/src/validation/validateAggregationConfig.ts:53 — Throws plain Error for unregistered metric providers while sibling validation functions throw InputError.

Labels: PR introduces new scalar aggregation KPI types with filtering, documentation, and tests — matches the enhancement label convention used by other scorecard feature PRs.

fullsend-ai-review · 2026-07-31T14:05:10Z

🤖 Review · ❌ Terminated · Started 2:05 PM UTC · Ended 2:47 PM UTC
Commit: be2bce6 · View workflow run →

fullsend-ai-review · 2026-07-31T14:47:53Z

🤖 Finished Review · ✅ Success · Started 2:05 PM UTC · Completed 2:47 PM UTC
Commit: be2bce6 · View workflow run →

fullsend-ai-review · 2026-08-03T14:16:45Z

🤖 Review · ❌ Terminated · Started 2:16 PM UTC · Ended 2:59 PM UTC
Commit: ea955a2 · View workflow run →

fullsend-ai-review · 2026-08-03T14:59:16Z

🤖 Finished Review · ✅ Success · Started 2:16 PM UTC · Completed 2:59 PM UTC
Commit: ea955a2 · View workflow run →

fullsend-ai-review · 2026-08-03T16:19:23Z

🤖 Review · ❌ Terminated · Started 4:19 PM UTC · Ended 4:41 PM UTC
Commit: 647246b · View workflow run →

fullsend-ai-review · 2026-08-03T16:41:05Z

🤖 Finished Review · ✅ Success · Started 4:19 PM UTC · Completed 4:41 PM UTC
Commit: 647246b · View workflow run →

Signed-off-by: Ihor Mykhno <imykhno@redhat.com>

fullsend-ai-review · 2026-08-05T09:06:25Z

🤖 Review · ❌ Terminated · Started 9:06 AM UTC · Ended 9:23 AM UTC
Commit: 8a9dca8 · View workflow run →

sonarqubecloud · 2026-08-05T09:07:39Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Superseded by updated review

fullsend-ai-review · 2026-08-05T09:23:55Z

🤖 Finished Review · ✅ Success · Started 9:06 AM UTC · Completed 9:23 AM UTC
Commit: 8a9dca8 · View workflow run →

fullsend-ai-review · 2026-08-05T09:31:15Z

🤖 Review · ❌ Terminated · Started 9:31 AM UTC · Ended 9:47 AM UTC
Commit: 8a9dca8 · View workflow run →

rhdh-qodo-merge · 2026-08-05T09:32:22Z

PR Summary by Qodo

Add filter.status for scalar Scorecard aggregations with startup validation

✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

AI Description

• Add optional filter.status to scalar aggregation KPIs to limit contributed rows.
• Validate filter.status against metric threshold keys at backend startup.
• Document behavior and add tests across router/service/DB layers.

Diagram

graph TD
  Cfg["app-config.yaml / scorecard.aggregationKPIs"] --> Svc["AggregationsService"] --> Strat["ScalarAggregationStrategy"] --> Loader["AggregatedMetricLoader"] --> DB[("metric_values DB")]
  Cfg --> Val{"validateAggregationConfig"} --> TR["ThresholdResolver"]
  Val --> Svc
  Svc --> Api["HTTP Router /aggregations"] --> Client["UI / API client"]
  Common["scorecard-common types"] --> Svc
  Common --> Api
  subgraph Legend
    direction LR
    _cfg["Config"] ~~~ _svc["Service"] ~~~ _dec{"Validation"} ~~~ _db[("Database")]
  end

High-Level Assessment

The following are alternative approaches to this PR:

1. Application-layer filtering (fetch latest rows then filter in JS)

➕ Avoids DB-specific SQL differences and NULL aggregate handling
➕ Easier to extend to multi-field filters later
➖ More data transferred from DB; slower for large portfolios
➖ Harder to keep timestamp/total semantics consistent with DB aggregation

2. Generalized filter DSL for all aggregation types

➕ Consistent filtering across scalar/statusGrouped/weightedStatusScore
➕ Future-proofs for additional filter dimensions (owner, kind, tag)
➖ Larger surface area and validation complexity for this ticket
➖ Requires defining semantics for distribution/weighted types (currently explicitly not filtered)

Recommendation: Current approach (DB-level status filtering for scalar types only, validated at startup) is the best fit: it keeps aggregation efficient, limits scope to the requested KPI types, and provides deterministic semantics (total: 0 + epoch timestamp preservation) when a filter matches nothing. Consider a broader filter DSL only if upcoming requirements extend beyond scalar filter.status.

Files changed (42) +1817 / -323

Enhancement (17) +244 / -39

green-cats-care.mdAdd changeset for scalar KPI status filtering feature +7/-0

Add changeset for scalar KPI status filtering feature
• Introduces a changeset bumping scorecard backend/common/frontend packages. Notes support for 'filter.status' on scalar aggregations.
workspaces/scorecard/.changeset/green-cats-care.md

config.d.tsAdd filter.status to scalar KPI config typing +5/-0

Add filter.status to scalar KPI config typing
• Extends the backend config schema typings to allow an optional 'filter.status' for scalar aggregation KPI entries.
workspaces/scorecard/plugins/scorecard-backend/config.d.ts

DatabaseMetricValues.tsApply filter.status in scalar aggregation SQL and harden empty-set handling +23/-5

Apply filter.status in scalar aggregation SQL and harden empty-set handling
• Threads an optional filter into scalar aggregation queries and adds a status WHERE clause when provided. Normalizes NULL/NaN aggregates to '0' and preserves epoch timestamps when no rows contribute (e.g., filter matches nothing).
workspaces/scorecard/plugins/scorecard-backend/src/database/DatabaseMetricValues.ts

plugin.tsPass ThresholdResolver into aggregation config validation +1/-0

Pass ThresholdResolver into aggregation config validation
• Updates plugin initialization to provide 'thresholdResolver' to 'validateAggregationConfig', enabling validation of 'filter.status' against resolved threshold keys.
workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts

AggregatedMetricLoader.tsAdd optional filter parameter to scalar loader API +3/-0

Add optional filter parameter to scalar loader API
• Adds 'AggregationConfigFilter' parameter plumbing so scalar aggregation strategies can pass filters down to the database layer.
workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregatedMetricLoader.ts

AggregationsService.tsCache metric-id fallback aggregation configs +4/-1

Cache metric-id fallback aggregation configs
• Stores the computed fallback config (metric-id defaulting to statusGrouped) in the service cache, reducing repeated provider lookups and duplicate warnings.
workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregationsService.ts

ScalarAggregationStrategy.tsForward KPI filter to scalar aggregation loader +1/-0

Forward KPI filter to scalar aggregation loader
• Passes 'aggregationConfig.filter' into the scalar loader so DB aggregation can apply 'filter.status'.
workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/ScalarAggregationStrategy.ts

mappers.tsSwitch timestamp mapping to normalizeTimestamp and include metadata.filter +14/-4

Switch timestamp mapping to normalizeTimestamp and include metadata.filter
• Replaces 'toIsoTimestamp' usage with 'normalizeTimestamp(...).toISOString()' for consistent epoch-on-missing behavior. Adds conditional inclusion of 'filter' in aggregation metadata when present in validated scalar configs.
workspaces/scorecard/plugins/scorecard-backend/src/service/mappers.ts

buildAggregationConfig.tsMap scalar filter block from config into AggregationConfig +6/-0

Map scalar filter block from config into AggregationConfig
• Introduces filter parsing for scalar types via 'buildAggregationConfigFilter', attaching 'aggregationConfig.filter' only when non-empty.
workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfig.ts

buildAggregationConfigFilter.tsParse filter.status from KPI config +35/-0

Parse filter.status from KPI config
• Adds a helper that reads the optional 'filter' config block and returns a typed 'AggregationConfigFilter' object containing 'status' when present.
workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfigFilter.ts

aggregationConfigSchemas.tsAdd scalar filter schema and refactor thresholds schema into shared module +5/-12

Add scalar filter schema and refactor thresholds schema into shared module
• Extracts thresholds schema into 'aggregationThresholdsConfigSchema' and introduces optional 'aggregationFilterSchema' for scalar KPI config validation.
workspaces/scorecard/plugins/scorecard-backend/src/validation/schemas/aggregationConfigSchemas.ts

aggregationFilterSchema.tsDefine zod schema for scalar aggregation filter config +5/-5

Define zod schema for scalar aggregation filter config
• Repurposes the file to define 'aggregationFilterSchema' with 'status' length constraints for scalar KPI filter validation.
workspaces/scorecard/plugins/scorecard-backend/src/validation/schemas/aggregationFilterSchema.ts

validateAggregationConfig.tsDelegate scalar-specific validation and validate filter.status via ThresholdResolver +14/-12

Delegate scalar-specific validation and validate filter.status via ThresholdResolver
• Adds ThresholdResolver dependency, uses InputError for misconfiguration, and delegates scalar checks to 'validateScalarAggregationConfig' (including filter.status validation).
workspaces/scorecard/plugins/scorecard-backend/src/validation/validateAggregationConfig.ts

validateScalarAggregationConfig.tsCentralize scalar-only validation rules +56/-0

Centralize scalar-only validation rules
• Introduces scalar validation enforcing numeric-metric requirement (except count) and calling filter.status validation against resolved threshold keys.
workspaces/scorecard/plugins/scorecard-backend/src/validation/validateScalarAggregationConfig.ts

validateScalarFilterStatus.tsValidate filter.status against resolved metric threshold rule keys +48/-0

Validate filter.status against resolved metric threshold rule keys
• Implements startup validation ensuring configured scalar 'filter.status' matches a threshold rule key for the KPI’s metric (including app-config overrides).
workspaces/scorecard/plugins/scorecard-backend/src/validation/validateScalarFilterStatus.ts

report.api.mdExpose AggregationConfigFilter and metadata.filter in API report +7/-0

Expose AggregationConfigFilter and metadata.filter in API report
• Updates the generated API report to include 'AggregationConfigFilter' and adds 'filter?: AggregationConfigFilter' to AggregationConfig and AggregationMetadata.
workspaces/scorecard/plugins/scorecard-common/report.api.md

aggregation.tsAdd shared AggregationConfigFilter type and metadata support +10/-0

Add shared AggregationConfigFilter type and metadata support
• Introduces 'AggregationConfigFilter' (currently 'status?: string') and threads it into AggregationConfig and AggregationMetadata types shared across frontend/backend.
workspaces/scorecard/plugins/scorecard-common/src/types/aggregation.ts

Bug fix (1) +4 / -1

normalizeTimestamp.tsNormalize missing timestamps to epoch (Date(0)) +4/-1

Normalize missing timestamps to epoch (Date(0))
• Changes default behavior to return epoch for unsupported/missing inputs, avoiding misleading "now" timestamps when no data contributes.
workspaces/scorecard/plugins/scorecard-backend/src/utils/normalizeTimestamp.ts

Refactor (2) +29 / -2

mapMetricValueRow.tsRename DB metric value create input type for clarity +2/-2

Rename DB metric value create input type for clarity
• Renames 'MetricValueCreateInput' to 'DbMetricValueCreateInput' and updates 'toMetricValueRow' signature accordingly.
workspaces/scorecard/plugins/scorecard-backend/src/database/utils/mapMetricValueRow.ts

aggregationThresholdsConfigSchema.tsIntroduce shared thresholds zod schema +27/-0

Introduce shared thresholds zod schema
• Adds a standalone 'aggregationThresholdsConfigSchema' used by both scalar and weighted KPI schemas for consistent validation.
workspaces/scorecard/plugins/scorecard-backend/src/validation/schemas/aggregationThresholdsConfigSchema.ts

Tests (16) +1414 / -230

mockMetricProvidersRegistry.tsExtend mock metric provider registry with listProviders +4/-0

Extend mock metric provider registry with listProviders
• Adds 'listProviders()' to the mock registry to support ThresholdResolver-based validation tests.
workspaces/scorecard/plugins/scorecard-backend/fixtures/mockMetricProvidersRegistry.ts

mockThresholds.tsRename and clarify threshold fixtures +4/-2

Rename and clarify threshold fixtures
• Renames fixtures to 'mockHigherIsBetterThresholds' / 'mockLowerIsBetterThresholds' and adds comments describing intended semantics.
workspaces/scorecard/plugins/scorecard-backend/fixtures/mockThresholds.ts

DatabaseMetricValues.test.tsAdd DB-level tests for scalar filter.status behavior +93/-0

Add DB-level tests for scalar filter.status behavior
• Adds cross-database tests verifying scalar aggregates only include rows matching 'status', preserve portfolio counts, and return 'value: 0'/'total: 0' when no rows match.
workspaces/scorecard/plugins/scorecard-backend/src/database/DatabaseMetricValues.test.ts

CatalogMetricService.test.tsUpdate expected timestamp baseline for empty portfolio +1/-1

Update expected timestamp baseline for empty portfolio
• Adjusts test expectations to use epoch timestamps when no maxTimestamp is available (after normalizeTimestamp semantic change).
workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.test.ts

AggregatedMetricLoader.test.tsVerify scalar filter is forwarded to DB read +17/-0

Verify scalar filter is forwarded to DB read
• Extends loader tests to ensure an optional 'filter' argument is passed through to 'readScalarAggregatedMetricByEntityRefs'.
workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregatedMetricLoader.test.ts

AggregationsService.test.tsTest parsing of scalar filter.status and adjust caching expectations +49/-11

Test parsing of scalar filter.status and adjust caching expectations
• Adds a test that scalar KPI configs parse and retain 'filter.status'. Updates behavior to cache fallback configs and adjusts warning/registry call expectations.
workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregationsService.test.ts

scalarAggregationStrategy.test.tsAdd strategy-level tests for filter forwarding +34/-8

Add strategy-level tests for filter forwarding
• Updates fixtures and adds coverage to ensure scalar strategies call the loader with the expected filter argument when configured.
workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/scalarAggregationStrategy.test.ts

statusGroupedAggregationStrategy.test.tsUpdate threshold fixture names in statusGrouped tests +6/-6

Update threshold fixture names in statusGrouped tests
• Renames fixture imports to the new threshold helper names without changing behavior.
workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/statusGroupedAggregationStrategy.test.ts

weightedStatusScoreAggregationStrategy.test.tsUpdate threshold fixture names in weighted tests +8/-8

Update threshold fixture names in weighted tests
• Renames fixture imports to the new threshold helper names without changing behavior.
workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/weightedStatusScoreAggregationStrategy.test.ts

mappers.test.tsEnsure metadata.filter is present only when configured +54/-18

Ensure metadata.filter is present only when configured
• Adds tests asserting scalar KPIs include 'metadata.filter' when 'filter.status' is configured, and omit it otherwise for both metadata and full result wrappers.
workspaces/scorecard/plugins/scorecard-backend/src/service/mappers.test.ts

router.test.tsAdd router tests for scalar filter.status and metadata exposure +231/-39

Add router tests for scalar filter.status and metadata exposure
• Refactors config setup to use 'mockServices.rootConfig', adds GET /aggregations tests verifying DB calls receive filters, and adds metadata endpoint tests verifying 'filter' is included/omitted appropriately.
workspaces/scorecard/plugins/scorecard-backend/src/service/router.test.ts

buildAggregationConfig.test.tsAdd buildAggregationConfig coverage for filter mapping +98/-7

Add buildAggregationConfig coverage for filter mapping
• Updates threshold fixture names and adds tests ensuring scalar KPI configs map 'filter.status' and omit empty filter blocks; confirms non-scalar types ignore filters.
workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfig.test.ts

buildAggregationConfigFilter.test.tsAdd unit tests for parsing filter.status from config +92/-0

Add unit tests for parsing filter.status from config
• Covers absent filter, empty filter block, and mapping of 'filter.status' into an 'AggregationConfigFilter'.
workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfigFilter.test.ts

normalizeTimestamp.test.tsChange normalizeTimestamp contract to return epoch for missing timestamps +10/-2

Change normalizeTimestamp contract to return epoch for missing timestamps
• Updates tests to reflect that undefined/null/empty timestamp inputs now normalize to epoch rather than the current time.
workspaces/scorecard/plugins/scorecard-backend/src/utils/normalizeTimestamp.test.ts

validateAggregationConfig.test.tsAdd validation coverage for filter.status and boolean metric scalar rules +523/-128

Add validation coverage for filter.status and boolean metric scalar rules
• Refactors tests to use ThresholdResolver and adds cases for: boolean metrics (count allowed, others rejected), scalar filter.status validity against default and overridden threshold keys, and length constraints.
workspaces/scorecard/plugins/scorecard-backend/src/validation/validateAggregationConfig.test.ts

validateScalarFilterStatus.test.tsAdd unit tests for filter.status threshold-key validation +190/-0

Add unit tests for filter.status threshold-key validation
• Tests that filter.status is optional, accepts default keys, accepts app-config override keys, and rejects unknown keys with a helpful error message.
workspaces/scorecard/plugins/scorecard-backend/src/validation/validateScalarFilterStatus.test.ts

Documentation (5) +114 / -44

README.mdUpdate workspace docs index to reference filter.status +7/-7

Update workspace docs index to reference filter.status
• Refreshes documentation table entries to explicitly mention 'filter.status' support for scalar KPIs and related thresholds documentation.
workspaces/scorecard/README.md

README.mdDocument filter.status for scalar aggregation KPIs +50/-10

Document filter.status for scalar aggregation KPIs
• Expands aggregation KPI configuration docs with a 'filter' field description and startup validation notes. Updates API docs to indicate 'metadata.filter' is returned for filtered scalar KPIs.
workspaces/scorecard/plugins/scorecard-backend/README.md

aggregation.mdDocument scalar status filtering semantics and edge cases +45/-23

Document scalar status filtering semantics and edge cases
• Adds a dedicated section for 'filter.status' on scalar KPIs, including startup validation behavior. Clarifies 'count' support for boolean metrics and documents 'total: 0' / 'value: 0' semantics when filters match nothing.
workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md

thresholds.mdLink filter.status keys to metric threshold rule keys +4/-2

Link filter.status keys to metric threshold rule keys
• Documents that scalar 'filter.status' values must match metric threshold rule 'key's (provider defaults or app-config overrides). Expands defaults explanation for scalar result thresholds.
workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md

README.mdClarify scalar KPI support and document filter.status capabilities +8/-2

Clarify scalar KPI support and document filter.status capabilities
• Explains that bundled homepage card renders only statusGrouped/weightedStatusScore, while scalar KPIs (including filter.status) are API-supported but require custom UI. Adds terminology note distinguishing scalar 'average' from 'weightedStatusScore'.
workspaces/scorecard/plugins/scorecard/README.md

Other (1) +12 / -7

app-config.yamlAdd example scalar KPI with filter.status and updated thresholds +12/-7

Add example scalar KPI with filter.status and updated thresholds
• Replaces the sample scalar KPI with 'totalCriticalPRs' and configures 'filter.status: error'. Updates example threshold rules and adds an explicit 'error' bucket.
workspaces/scorecard/app-config.yaml

rhdh-qodo-merge · 2026-08-05T09:37:56Z

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Wrong threshold override path 🐞 Bug ⚙ Maintainability

Description

The docs for filter.status validation instruct users to configure threshold overrides under
scorecard.plugins.<...>, but the backend actually resolves overrides under
scorecard.metricProviders.<datasource>.<providerName>.... Following the docs will make valid
overrides (and thus valid filter.status keys) appear unsupported or fail startup validation
unexpectedly.

Code

workspaces/scorecard/plugins/scorecard-backend/README.md[227]

+| `filter`      | Optional for **scalar types** only. **`filter.status`**: threshold status key (case-sensitive). Only latest rows with that status contribute to **`value`** and **`total`**. Must match a threshold rule **`key`** for **`metricId`** from provider defaults or **`scorecard.plugins.<datasource>.<metricName>.thresholds`** (validated at startup). **`entitiesConsidered`** and **`calculationErrorCount`** stay full-portfolio. Returned in API **`metadata.filter`** when set. |

Relevance

●●● Strong
Team recently accepted fixing docs/config paths (plugins→metricProviders); wrong override path is
actionable doc bug.
PR-#4022

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new docs explicitly reference scorecard.plugins... for threshold overrides, but the backend’s
resolver constructs and checks only scorecard.metricProviders... paths; the existing thresholds
documentation section also lists scorecard.metricProviders... as the override location.
workspaces/scorecard/plugins/scorecard-backend/README.md[221-233]
workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md[216-221]
workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md[194-197]
workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md[87-90]
workspaces/scorecard/plugins/scorecard-backend/src/utils/metricProviderConfigKeys.ts[46-70]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Documentation for scalar `filter.status` validation references `scorecard.plugins.<datasource>.<metricName>.thresholds`, but the implementation reads thresholds from `scorecard.metricProviders.<datasource>.<providerName>.metrics.<metricName>.thresholds` (or provider-level thresholds). This misleads users configuring custom threshold keys and can cause confusing startup validation failures.

## Issue Context
`ThresholdResolver` uses `resolveThresholdsConfigPath(...)`, which constructs `scorecard.metricProviders...` paths and never reads `scorecard.plugins...`.

## Fix Focus Areas
- workspaces/scorecard/plugins/scorecard-backend/README.md[221-233]
- workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md[216-221]
- workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md[186-197]
- workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md[83-90]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. Non-scalar filter doc mismatch 🐞 Bug ⚙ Maintainability

Description

Docs claim filter is rejected at startup for non-scalar KPI types, but the implementation ignores
filter unless the aggregation type is scalar. This creates silent misconfiguration risk: operators
may believe a filter is active (or expect a startup failure) when it is actually discarded.

Code

workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md[R218-220]

+Scalar KPIs may include an optional top-level **`filter.status`** to restrict aggregation to latest rows whose threshold evaluation matches the given status key (for example `success`, `warning`, `error`, or any custom threshold key). **`filter`** is rejected on non-scalar types at startup.
+
+**Startup validation:** When **`filter.status`** is set, the backend validates at plugin load that the value is a **threshold rule key** for the KPI’s **`metricId`** (provider defaults plus **`scorecard.plugins.<datasource>.<metricName>.thresholds`** in app-config — for example `scorecard.plugins.jira.openIssues.thresholds` when **`metricId`** is `jira.openIssues`). Keys are **case-sensitive** (`error` ≠ `Error`) and must be 1–64 characters. Invalid keys cause startup to fail with an error listing valid keys. Per-entity annotation threshold overrides are **not** considered (they apply at metric sync time only). See [thresholds.md — Scalar status filter](./thresholds.md#5-aggregation-kpi-result-thresholds-scalar-types).

Relevance

●●● Strong
Doc/behavior mismatch risks misconfig; similar aggregation docs clarifications have been accepted
previously.
PR-#2020

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The docs assert non-scalar filters are rejected, but the config builder only attaches filter for
scalar types, and tests verify non-scalar KPIs with filter are accepted (not rejected).
workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md[216-221]
workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfig.ts[48-60]
workspaces/scorecard/plugins/scorecard-backend/src/validation/validateAggregationConfig.test.ts[703-760]
workspaces/scorecard/plugins/scorecard-backend/README.md[231-233]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Documentation states `filter` is rejected on non-scalar aggregation types at startup, but config building/validation currently does not reject it (it is effectively ignored because only scalar configs map `filter`). This mismatch can hide configuration errors.

## Issue Context
`buildAggregationConfig(...)` only maps `filter` for scalar aggregation types, and validation tests explicitly assert that `statusGrouped`/`weightedStatusScore` KPIs with `filter` do not throw.

## Fix Focus Areas
- workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md[216-221]
- workspaces/scorecard/plugins/scorecard-backend/README.md[227-233]
- workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfig.ts[48-60]
- workspaces/scorecard/plugins/scorecard-backend/src/validation/validateAggregationConfig.test.ts[703-760]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Context used

⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials

✅ Compliance rules (platform): 11 rules

✅ Cross-repo context

Explored: repo: redhat-developer/rhdh (sha: e3b3f0bc)

Explored: repo: redhat-developer/rhdh-local (sha: a1776caa)

Not relevant to this PR: redhat-developer/rhdh-chart

Not relevant to this PR: redhat-developer/rhdh-operator

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

fullsend-ai-review

Note: The following inline comments could not be posted on the diff (GitHub returned 422) and are included here instead:

workspaces/scorecard/plugins/scorecard-backend/src/database/DatabaseMetricValues.ts (file-level): Line 498 · [low] edge-case

The filter condition if (filter?.status) uses JavaScript truthiness. If a caller bypasses config parsing and passes { status: '' }, the filter would be silently skipped. In practice, the Zod schema enforces min(1) and buildAggregationConfigFilter uses getOptionalString, so an empty string cannot reach this code path through any supported flow.

workspaces/scorecard/plugins/scorecard-backend/src/validation/validateScalarAggregationConfig.ts:49: [low] naming-convention

Accepts ValidatedAggregationConfig then casts to ScalarAggregationConfig internally. Since the caller already narrows via isScalarAggregationType, the input type could be ScalarAggregationConfig directly, eliminating the cast and improving type safety.

Suggested fix: Change the aggregationConfig field in ValidateScalarAggregationConfigOptions from ValidatedAggregationConfig to ScalarAggregationConfig, then remove the as ScalarAggregationConfig cast.

workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfigFilter.ts:27: [low] naming-convention

Returns empty object {} and uses Object.keys().length check for absence, while sister utility buildAggregationConfigThresholds returns undefined. Inconsistent absence-signaling pattern between the two utilities.

Suggested fix: Consider returning AggregationConfigFilter | undefined and returning undefined when no filter properties are set, matching buildAggregationConfigThresholds.

workspaces/scorecard/plugins/scorecard-backend/src/validation/validateScalarAggregationConfig.ts:42: [low] scope-creep

Boolean metric relaxation for count type is not mentioned in the PR description. Expands feature surface beyond filter.status.

fullsend-ai-review · 2026-08-05T09:47:54Z

🤖 Finished Review · ✅ Success · Started 9:31 AM UTC · Completed 9:47 AM UTC
Commit: 8a9dca8 · View workflow run →

openshift-ci Bot added the do-not-merge/work-in-progress label Jul 31, 2026

github-actions Bot added the workspace/scorecard label Jul 31, 2026