diff --git a/workspaces/scorecard/.changeset/old-kings-care.md b/workspaces/scorecard/.changeset/old-kings-care.md new file mode 100644 index 0000000000..3052bb2cbf --- /dev/null +++ b/workspaces/scorecard/.changeset/old-kings-care.md @@ -0,0 +1,9 @@ +--- +'@red-hat-developer-hub/backstage-plugin-scorecard-backend-module-sonarqube': patch +'@red-hat-developer-hub/backstage-plugin-scorecard-backend-module-jira': patch +'@red-hat-developer-hub/backstage-plugin-scorecard-backend': patch +'@red-hat-developer-hub/backstage-plugin-scorecard-node': patch +'@red-hat-developer-hub/backstage-plugin-scorecard': patch +--- + +Implemented threshold interval validation for Scorecard. joint coverage on the real line, gap detection and error messages, overlaps versus rule order. diff --git a/workspaces/scorecard/app-config.yaml b/workspaces/scorecard/app-config.yaml index 7a38dd3467..a4dfd4a1a3 100644 --- a/workspaces/scorecard/app-config.yaml +++ b/workspaces/scorecard/app-config.yaml @@ -249,7 +249,7 @@ scorecard: expression: '>=80' color: '#6bb300' # green - key: warning - expression: '30-79' + expression: '30-80' color: 'rgb(224, 189, 108)' # light orange - key: error expression: '<30' diff --git a/workspaces/scorecard/packages/app-legacy/e2e-tests/utils/scorecardResponseUtils.ts b/workspaces/scorecard/packages/app-legacy/e2e-tests/utils/scorecardResponseUtils.ts index 272c122fb3..736e3281e8 100644 --- a/workspaces/scorecard/packages/app-legacy/e2e-tests/utils/scorecardResponseUtils.ts +++ b/workspaces/scorecard/packages/app-legacy/e2e-tests/utils/scorecardResponseUtils.ts @@ -232,7 +232,7 @@ export const openPrsWeightedAggregatedResponse = { total: 10, timestamp: '2026-01-24T14:10:32.858Z', thresholds: DEFAULT_NUMBER_THRESHOLDS, - averageScore: 0.5, + averageScore: 50, averageWeightedSum: 500, averageMaxPossible: 1000, aggregationChartDisplayColor: 'rgb(224, 189, 108)', diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-jira/__fixtures__/testUtils.ts b/workspaces/scorecard/plugins/scorecard-backend-module-jira/__fixtures__/testUtils.ts index 2cc8a01d5a..4f9c29db47 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-jira/__fixtures__/testUtils.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-jira/__fixtures__/testUtils.ts @@ -38,9 +38,9 @@ export function newEntityComponent( export function newThresholdsConfig(): ThresholdConfig { return { rules: [ - { key: 'success', expression: '<3' }, - { key: 'warning', expression: '11-32' }, - { key: 'error', expression: '>33' }, + { key: 'success', expression: '<5' }, + { key: 'warning', expression: '5-25' }, + { key: 'error', expression: '>25' }, ], }; } diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/src/metricProviders/SonarQubeNumberMetricProvider.test.ts b/workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/src/metricProviders/SonarQubeNumberMetricProvider.test.ts index aae5173583..f276f5ec66 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/src/metricProviders/SonarQubeNumberMetricProvider.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/src/metricProviders/SonarQubeNumberMetricProvider.test.ts @@ -64,7 +64,10 @@ describe('SonarQubeNumberMetricProvider', () => { it('should return custom thresholds when provided', () => { const custom: ThresholdConfig = { - rules: [{ key: 'ok', expression: '<5', color: '#00ff00', icon: 'ok' }], + rules: [ + { key: 'ok', expression: '<5', color: '#00ff00', icon: 'ok' }, + { key: 'rest', expression: '>=5', color: '#ff0000', icon: 'bad' }, + ], }; const mockConfiWithCustomThresholds = new ConfigReader({ scorecard: { diff --git a/workspaces/scorecard/plugins/scorecard-backend/README.md b/workspaces/scorecard/plugins/scorecard-backend/README.md index 1983b9e714..3f777e0681 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/README.md +++ b/workspaces/scorecard/plugins/scorecard-backend/README.md @@ -116,9 +116,9 @@ Thresholds define conditions to assign metric values to specific visual categori - **App Configuration**: Override defaults through `app-config.yaml` - **Entity Annotations**: Override specific thresholds per entity using catalog annotations -Thresholds are evaluated in order, and the first matching rule determines the category. The plugin supports various operators for number metrics (`>`, `>=`, `<`, `<=`, `==`, `!=`, `-` (range)) and boolean metrics (`==`, `!=`). +Thresholds are evaluated in order, and the first matching rule determines the category. The plugin supports various operators for number metrics (`>`, `>=`, `<`, `<=`, `==`, `!=`, `-` (range)) and boolean metrics (`==`, `!=`). For **number** metrics, configurations loaded through validated paths must cover the **entire real line** when two or more rules are defined (no gaps between intervals); **`average`** KPI **`options.thresholds`** follow the same rule. -For comprehensive threshold configuration guide, examples, best practices, and **aggregation KPI result thresholds** for **`type: average`**, see [thresholds.md](./docs/thresholds.md). +For comprehensive threshold configuration guide, examples, best practices, interval validation, and **aggregation KPI result thresholds** for **`type: average`**, see [thresholds.md](./docs/thresholds.md). ## Aggregation KPIs (homepage and `GET /aggregations`) @@ -154,7 +154,7 @@ scorecard: expression: '>=75' color: success.main - key: warning - expression: '10-74' + expression: '10-75' color: warning.main - key: error expression: '<10' diff --git a/workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md b/workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md index 225f897844..3f10e508e3 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md +++ b/workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md @@ -53,6 +53,8 @@ For **`average`**: **`scorecard.aggregationKPIs`** is validated when the backend plugin starts. Invalid entries (unknown **`type`**, missing **`options`** for **`average`**, empty **`statusScores`**, unknown **`metricId`**, invalid threshold expressions, etc.) cause startup to **fail with an error** so misconfiguration is caught early. Fix app-config and redeploy. +For **`type: average`**, optional **`options.thresholds`** must satisfy the same **number interval / gap** rules as metric thresholds when multiple rules apply (union must cover the full real line with no gaps). Errors mention an approximate **first uncovered region**. See [Joint coverage (number metrics)](./thresholds.md#joint-coverage-number-metrics). + Schema reference for config discovery (IDE / `backstage-cli config:schema`): see **`config.d.ts`** on the backend package (`aggregationKPIs` and nested **`options`**). ## API Endpoint diff --git a/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md b/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md index 6b0f2cb98f..cd162ea6ee 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md +++ b/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md @@ -11,6 +11,42 @@ Thresholds are evaluated in order and the **first matching** threshold rule is a - **`color`** (optional): The color to display for this threshold in the UI (see [Threshold Colors](#threshold-colors)) - **`icon`** (optional): The icon to display for this threshold in the UI (see [Threshold Icons](#threshold-icons)) +## Joint coverage (number metrics) + +For **number** metrics with **two or more** rules, validation requires that the **union** of all rule expressions covers the **entire real line** (from negative infinity to positive infinity). This matches how the backend evaluates values: numeric ranges use **inclusive** endpoints. Comparison operators keep their usual strict or non-strict boundaries. + +**Overlaps:** Overlapping intervals **do not** fail validation—the check only verifies that no **gap** remains. Rule **order** does not affect coverage. It only affects **which key wins** when more than one rule matches ([first-match semantics](#thresholdevaluator)). + +### How validation works + +For each rule, the validator parses the expression and maps it to one or more intervals on the real line: + +- **Range** syntax `min-max` → closed interval `[min, max]`. +- **Comparisons** (`>`, `>=`, `<`, `<=`) → half-lines or bounded rays. +- **`==`** → a single point. +- **`!=`** → **two** intervals (everything except that point), which can help close gaps near a boundary without adding an extra rule. + +All intervals from every rule are merged (overlapping or touching intervals combine). The configuration **passes** only if the merged result is a single interval covering **(-∞, +∞)** with both ends included. + +### Gap detection and error messages + +If the merged union leaves any real number uncovered, validation throws `ThresholdConfigFormatError` with a message similar to: + +`Number threshold rules do not cover the entire real line. First uncovered region (approximately): …` + +The reported interval is an **approximate** description of the **first** gap found (for example `(10, 11)`, `(-∞, 10)`, or `(74, 75)`). Adjust boundaries so adjacent rules meet or overlap—for example use **`10-20`** instead of **`11-20`** next to **`'<10'`**, or **`10-75`** with **`>=75`** instead of **`10-74`** with **`>=75`**. + +**Gap example:** `success: '<10'`, `warning: '11-20'`, `error: '>20'` leaves **`10`** and **`(10, 11)`** uncovered. Prefer **`'<10'`**, **`'10-20'`**, **`'>20'`**, or widen ranges so neighbors touch (**`51-79`** with **`'<51'`** and **`'>=80'`**). + +### When full-line coverage is skipped + +Validation **does not** require full coverage when: + +- The metric type is not **number**, or +- **`rules`** is empty, or +- There is **at most one** rule (for example validating a single expression in isolation), or +- **Every** rule is a numeric **`==`** expression (discrete metrics such as Sonar ratings **`==1`** … **`==5`**). + ## Threshold Configuration Options ### 1. Provider Default Thresholds @@ -45,7 +81,8 @@ Duplicated threshold keys are not allowed (throws `ConfigFormatError`). ```typescript import { MetricProvider, - validateThresholds, + validateThresholdsForMetric, + getThresholdsFromConfig, } from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; export class MyMetricProvider implements MetricProvider<'number'> { @@ -58,12 +95,12 @@ export class MyMetricProvider implements MetricProvider<'number'> { static fromConfig(config: Config): MyMetricProvider { const configPath = 'scorecard.plugins.myDatasource.myMetric.thresholds'; - const configuredThresholds = config.getOptional(configPath); - - if (configuredThresholds) { - // validate threshold configuration is correct, throws ConfigFormatError if not - validateThresholds(configuredThresholds, 'number'); - } + // Validates structure, colors/icons, expressions, and number interval coverage (when applicable) + const configuredThresholds = getThresholdsFromConfig( + config, + configPath, + 'number', + ); return new MyMetricProvider(configuredThresholds); } @@ -74,6 +111,8 @@ export class MyMetricProvider implements MetricProvider<'number'> { } ``` +You can also call **`validateThresholdsForMetric(configuredThresholds, 'number')`** directly if you already have a config object (not reading from `Config`). + **Example App Configuration:** ```yaml @@ -135,7 +174,7 @@ These thresholds are **not** per-entity metric rules. They apply only to homepag **Defaults:** If **`thresholds`** is omitted from app-config under **`options`**, it is not injected at config-parse time. **`AverageAggregationStrategy`** applies **`DEFAULT_AVERAGE_KPI_RESULT_THRESHOLDS`** from [`src/constants/aggregationKPIs.ts`](../src/constants/aggregationKPIs.ts) when serving an aggregation: **`<30`** → error, **`30-79`** → warning, **`>=80`** → success (higher percentage = better). When that default path is used, the strategy logs at **info** that the built-in 0–100% scale is in effect. -**Startup validation:** Invalid rules or expressions are caught when the backend plugin loads, together with the rest of **`scorecard.aggregationKPIs`**. See [aggregation.md — Configuration validation](./aggregation.md#configuration-validation). +**Startup validation:** Invalid rules or expressions are caught when the backend plugin loads, together with the rest of **`scorecard.aggregationKPIs`**. Average KPI **`options.thresholds`** must also satisfy **joint full-line coverage** for number expressions (see [Joint coverage (number metrics)](#joint-coverage-number-metrics)), for example ensure ranges and comparison rules meet at boundaries (**`10-75`** with **`>=75`** and **`<10`**, not **`10-74`** with **`>=75`**, which would leave **`(74, 75)`** uncovered). See [aggregation.md — Configuration validation](./aggregation.md#configuration-validation). **Further reading:** [Entity Aggregation](./aggregation.md) (`average` algorithm, API, drill-down); [Scorecard backend README — Aggregation KPIs](../README.md#aggregation-kpis-homepage-and-get-aggregations) (full **`aggregationKPIs`** example including **`statusScores`**). @@ -204,7 +243,7 @@ finalRules: [ ### Number Metric -Supports operators: `>, >=, <, <=, ==, !=, -`. +Supports operators: `>, >=, <, <=, ==, !=, -`. The **`!=`** operator matches every value except the given number; in coverage validation it contributes **two** intervals, which can help eliminate small gaps at boundaries when combined with other rules. Example: @@ -375,7 +414,7 @@ The `ThresholdEvaluator` service processes threshold rules and determines which 1. **Order-dependent evaluation**: Rules are evaluated in the order they appear. If provider supports overriding defaults through [app configuration](#App-Configuration-Thresholds), you can change the evaluation order by specifying threshold keys in a different order. Entity annotations cannot alter the evaluation order, which is determined by either the [app configuration](#Provider-Default-Thresholds) or, if not specified, the [default provider configuration](#Provider-Default-Thresholds). 2. **First-match wins**: Returns the first threshold rule whose condition the value satisfies 3. **Type-safe**: Validates expressions against metric types -4. **Error handling**: You should validate your expressions loaded from config in your custom providers using `validateThresholds` from `backstage-plugin-scorecard-node`. Invalid expressions will result in evaluation error. +4. **Error handling**: Validate expressions loaded from config in custom providers using **`validateThresholdsForMetric`** or **`getThresholdsFromConfig`** from `@red-hat-developer-hub/backstage-plugin-scorecard-node`. Invalid expressions or gap configurations fail at validation time; unchecked configs may error at evaluation time. ### Best Practices @@ -395,9 +434,9 @@ rules: expression: '>30' ``` -### 2. Avoid Overlapping Ranges +### 2. Overlaps vs gaps -Ensure threshold ranges don't create gaps or unexpected behavior. +**Joint coverage** allows overlapping intervals—only **gaps** cause validation to fail. Overlaps do affect **runtime** behavior: the **first** matching rule in list order wins ([Logical Ordering](#1-logical-ordering)). If overlaps are unintentional, reorder or narrow rules so the intended category matches first. ## Related documentation diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/constants/aggregationKPIs.ts b/workspaces/scorecard/plugins/scorecard-backend/src/constants/aggregationKPIs.ts index bffbc5c06d..65c9fb9eb7 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/constants/aggregationKPIs.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/constants/aggregationKPIs.ts @@ -35,7 +35,7 @@ export const DEFAULT_AVERAGE_KPI_RESULT_THRESHOLDS: ThresholdConfig = { }, { key: 'warning', - expression: '30-79', + expression: '30-80', color: ScorecardThresholdRuleColors.WARNING, }, { diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregationsService.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregationsService.test.ts index af5e31e5d0..38fd55c34f 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregationsService.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregationsService.test.ts @@ -133,7 +133,7 @@ describe('AggregationsService', () => { const aggregationResult = result.result as AggregatedMetricAverageResult; expect(result.metadata?.aggregationType).toBe(aggregationTypes.average); - expect(aggregationResult.averageScore).toBeCloseTo(0.5, 5); + expect(aggregationResult.averageScore).toBe(50); expect(aggregationResult.averageWeightedSum).toBe(150); expect(aggregationResult.averageMaxPossible).toBe(300); }); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/AverageAggregationStrategy.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/AverageAggregationStrategy.ts index 3d2362dd00..13524cb62e 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/AverageAggregationStrategy.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/AverageAggregationStrategy.ts @@ -79,16 +79,14 @@ export class AverageAggregationStrategy implements AggregationStrategy { weightedSum, ); - const scorePercent = averageScore * 100; - const aggregationChartDisplayColor = this.getAggregationChartDisplayColor( - scorePercent, + averageScore, headlineThresholds, ); if (!aggregationChartDisplayColor) { throw new Error( - `The color for percentage '${scorePercent}' metric '${metric.id}' is not configured. Check the 'scorecard.aggregationKPIs.${aggregationConfig.id}.options.thresholds' configuration.`, + `The color for percentage '${averageScore}' metric '${metric.id}' is not configured. Check the 'scorecard.aggregationKPIs.${aggregationConfig.id}.options.thresholds' configuration.`, ); } @@ -162,11 +160,9 @@ export class AverageAggregationStrategy implements AggregationStrategy { const maxPossibleScore = maxScore * numberOfEntities; - const precision = 1000; - const averageScore = numberOfEntities > 0 && maxPossibleScore > 0 - ? Math.round((weightedSum / maxPossibleScore) * precision) / precision + ? Math.round((weightedSum / maxPossibleScore) * 1000) / 10 : 0; return { averageScore, maxPossibleScore }; diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/averageAggregationStrategy.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/averageAggregationStrategy.test.ts index 7dda045de8..72629f1a47 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/averageAggregationStrategy.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/averageAggregationStrategy.test.ts @@ -79,7 +79,7 @@ describe('AverageAggregationStrategy', () => { calculationErrorCount: 2, averageWeightedSum: 150, averageMaxPossible: 300, - averageScore: 0.5, + averageScore: 50, aggregationChartDisplayColor: 'warning.main', }), ); @@ -197,7 +197,7 @@ describe('AverageAggregationStrategy', () => { calculationErrorCount: 1, averageWeightedSum: 100, averageMaxPossible: 300, - averageScore: 0.333, + averageScore: 33.3, }), ); }); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/threshold/numberThresholdCoverageEvaluator.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/threshold/numberThresholdCoverageEvaluator.test.ts new file mode 100644 index 0000000000..0670f2c165 --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard-backend/src/threshold/numberThresholdCoverageEvaluator.test.ts @@ -0,0 +1,56 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { validateThresholdNumberIntervals } from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; +import { ThresholdEvaluator } from './ThresholdEvaluator'; + +describe('numberThresholdCoverage vs ThresholdEvaluator', () => { + const evaluator = new ThresholdEvaluator(); + + it('should allow sampled values to match some rule when coverage passes', () => { + const thresholds = { + rules: [ + { key: 'low', expression: '<10' }, + { key: 'mid', expression: '10-20' }, + { key: 'high', expression: '>20' }, + ], + }; + + expect(() => + validateThresholdNumberIntervals(thresholds.rules, 'number'), + ).not.toThrow(); + + for (const x of [-1e6, -1, 0, 9.5, 10, 15, 20.001, 1e6]) { + expect( + evaluator.getFirstMatchingThreshold(x, 'number', thresholds), + ).toBeDefined(); + } + }); + + it('should throw error when interval validation fails before evaluation', () => { + const thresholds = { + rules: [ + { key: 'low', expression: '<10' }, + { key: 'mid', expression: '11-20' }, + { key: 'high', expression: '>20' }, + ], + }; + + expect(() => + validateThresholdNumberIntervals(thresholds.rules, 'number'), + ).toThrow(/do not cover the entire real line/); + }); +}); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/buildAggregationConfig.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/buildAggregationConfig.test.ts index 094268642c..5b977cb335 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/utils/buildAggregationConfig.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/buildAggregationConfig.test.ts @@ -79,7 +79,7 @@ describe('buildAggregationConfig', () => { thresholds: { rules: [ { key: 'success', expression: '>=75', color: 'success.main' }, - { key: 'warning', expression: '10-74', color: 'warning.main' }, + { key: 'warning', expression: '10-75', color: 'warning.main' }, { key: 'error', expression: '<10', color: 'error.main' }, ], }, @@ -90,7 +90,7 @@ describe('buildAggregationConfig', () => { expect(result.options?.thresholds?.rules).toEqual([ { key: 'success', expression: '>=75', color: 'success.main' }, - { key: 'warning', expression: '10-74', color: 'warning.main' }, + { key: 'warning', expression: '10-75', color: 'warning.main' }, { key: 'error', expression: '<10', color: 'error.main' }, ]); }); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateAggregationConfig.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateAggregationConfig.test.ts index a91fa0265a..951c39d3a1 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateAggregationConfig.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateAggregationConfig.test.ts @@ -219,7 +219,7 @@ describe('validateAggregationConfig', () => { }, { key: 'warning', - expression: '10-74', + expression: '10-75', color: 'warning.main', }, { key: 'error', expression: '<10', color: 'error.main' }, @@ -269,4 +269,48 @@ describe('validateAggregationConfig', () => { /Invalid thresholds configuration|Invalid threshold expression/, ); }); + + it('should throw when average KPI thresholds leave a gap on the number line', () => { + const registry = new MetricProvidersRegistry(); + registry.register(new MockNumberProvider('github.open_prs', 'github')); + + const rootConfig = new ConfigReader({ + scorecard: { + aggregationKPIs: { + avgKpi: { + title: 'Avg KPI', + type: aggregationTypes.average, + description: 'Weighted health', + metricId: 'github.open_prs', + options: { + statusScores: { success: 100, warning: 50, error: 0 }, + thresholds: { + rules: [ + { + key: 'success', + expression: '<10', + color: 'success.main', + }, + { + key: 'warning', + expression: '11-20', + color: 'warning.main', + }, + { + key: 'error', + expression: '>20', + color: 'error.main', + }, + ], + }, + }, + }, + }, + }, + }); + + expect(() => validateAggregationConfig({ rootConfig, registry })).toThrow( + /do not cover the entire real line/, + ); + }); }); diff --git a/workspaces/scorecard/plugins/scorecard-node/report.api.md b/workspaces/scorecard/plugins/scorecard-node/report.api.md index b3aaca8e4e..0e1d830511 100644 --- a/workspaces/scorecard/plugins/scorecard-node/report.api.md +++ b/workspaces/scorecard/plugins/scorecard-node/report.api.md @@ -12,6 +12,7 @@ import { Metric } from '@red-hat-developer-hub/backstage-plugin-scorecard-common import { MetricType } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; import { MetricValue } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; import { ThresholdConfig } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; +import { ThresholdRule } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; // @public export type ComparisonOperator = { @@ -70,6 +71,12 @@ export class ThresholdConfigFormatError extends CustomErrorBase { name: 'ThresholdConfigFormatError'; } +// @public +export function validateThresholdNumberIntervals( + rules: ThresholdRule[], + expectedMetricType: MetricType, +): void; + // @public export function validateThresholdsForAggregation( thresholds: JsonValue, diff --git a/workspaces/scorecard/plugins/scorecard-node/src/utils/index.ts b/workspaces/scorecard/plugins/scorecard-node/src/utils/index.ts index 083fefe490..9ee4193b14 100644 --- a/workspaces/scorecard/plugins/scorecard-node/src/utils/index.ts +++ b/workspaces/scorecard/plugins/scorecard-node/src/utils/index.ts @@ -15,6 +15,7 @@ */ export { parseThresholdExpression } from './thresholds/parseThresholdExpression'; +export { validateThresholdNumberIntervals } from './thresholds/intervals/validateThresholdNumberIntervals'; export { validateThresholdsForMetric, validateThresholdsForAggregation, diff --git a/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/getThresholdsFromConfig.test.ts b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/getThresholdsFromConfig.test.ts index 1f9c6b2100..cb56656f12 100644 --- a/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/getThresholdsFromConfig.test.ts +++ b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/getThresholdsFromConfig.test.ts @@ -102,6 +102,35 @@ describe('getThresholdsFromConfig', () => { ); }); + it('should wrap real-line interval validation errors from metric thresholds', () => { + const { validateThresholdsForMetric: realValidate } = jest.requireActual< + typeof import('./validateThresholds') + >('./validateThresholds'); + mockedValidateThresholdsForMetric.mockImplementation(realValidate); + + const gapConfig = { + rules: [ + { key: 'success', expression: '<10' }, + { key: 'warning', expression: '11-20' }, + { key: 'error', expression: '>20' }, + ], + }; + + mockedConfig = mockServices.rootConfig({ + data: { scorecard: { defaultMetricThresholds: gapConfig } }, + }); + + expect(() => + getThresholdsFromConfig( + mockedConfig, + 'scorecard.defaultMetricThresholds', + 'number', + ), + ).toThrow( + /Invalid thresholds configuration at scorecard\.defaultMetricThresholds: .*do not cover the entire real line/, + ); + }); + it('should return undefined when thresholds config is not present', () => { jest.spyOn(mockedConfig, 'getOptional').mockReturnValue(undefined); const thresholds = getThresholdsFromConfig( diff --git a/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/getThresholdsFromConfig.ts b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/getThresholdsFromConfig.ts index 6a13c2ea79..150e133110 100644 --- a/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/getThresholdsFromConfig.ts +++ b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/getThresholdsFromConfig.ts @@ -34,6 +34,7 @@ export function getThresholdsFromConfig( const thresholdsConfig = config.getOptional(thresholdsPath); if (thresholdsConfig) { validateThresholdsForMetric(thresholdsConfig, expectedMetricType); + return thresholdsConfig; } } catch (error) { diff --git a/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/areAllRulesDiscreteNumberEquals.ts b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/areAllRulesDiscreteNumberEquals.ts new file mode 100644 index 0000000000..63f57f2e33 --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/areAllRulesDiscreteNumberEquals.ts @@ -0,0 +1,30 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { ThresholdRule } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; +import { parseThresholdExpression } from '../parseThresholdExpression'; + +/** + * Checks if all rules are discrete number equals. + */ +export function areAllRulesDiscreteNumberEquals( + rules: ThresholdRule[], +): boolean { + return rules.every( + rule => + parseThresholdExpression(rule.expression, 'number').operator === '==', + ); +} diff --git a/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/describeFirstGap.ts b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/describeFirstGap.ts new file mode 100644 index 0000000000..ffcdb05f15 --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/describeFirstGap.ts @@ -0,0 +1,66 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { NumberInterval } from './types'; + +export function describeFirstGap(merged: NumberInterval[]): string { + if (merged.length === 0) { + return 'the entire real line (no rules)'; + } + + const first = merged[0]!; + if (first.min > -Infinity) { + const rightBound = formatReal(first.min); + return first.minClosed ? `(-∞, ${rightBound})` : `(-∞, ${rightBound}]`; + } + + for (let i = 0; i < merged.length - 1; i++) { + const left = merged[i]!; + const right = merged[i + 1]!; + + const gapLeft = left.max; + const gapRight = right.min; + + const nonEmptyGap = + gapLeft < gapRight || + (gapLeft === gapRight && !(left.maxClosed || right.minClosed)); + + if (nonEmptyGap) { + const l = left.maxClosed ? '(' : '['; + const r = right.minClosed ? ')' : ']'; + return `${l}${formatReal(gapLeft)}, ${formatReal(gapRight)}${r}`; + } + } + + const last = merged[merged.length - 1]!; + + if (last.max < Infinity) { + const l = last.maxClosed ? '(' : '['; + return `${l}${formatReal(last.max)}, ∞)`; + } + + return 'unknown gap'; +} + +function formatReal(x: number): string { + if (x === -Infinity) { + return '-∞'; + } + if (x === Infinity) { + return '∞'; + } + return Number.isInteger(x) ? String(x) : String(x); +} diff --git a/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/getUnionInterval.ts b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/getUnionInterval.ts new file mode 100644 index 0000000000..482cab9ac0 --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/getUnionInterval.ts @@ -0,0 +1,47 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { NumberInterval } from './types'; + +export function getUnionInterval( + left: NumberInterval, + right: NumberInterval, +): NumberInterval { + const lowestInterval = getLowestInterval(left, right); + const highestInterval = getHighestInterval(left, right); + + return { ...lowestInterval, ...highestInterval }; +} + +function getLowestInterval(left: NumberInterval, right: NumberInterval) { + if (left.min < right.min) { + return { min: left.min, minClosed: left.minClosed }; + } + if (right.min < left.min) { + return { min: right.min, minClosed: right.minClosed }; + } + return { min: left.min, minClosed: left.minClosed || right.minClosed }; +} + +function getHighestInterval(left: NumberInterval, right: NumberInterval) { + if (left.max > right.max) { + return { max: left.max, maxClosed: left.maxClosed }; + } + if (right.max > left.max) { + return { max: right.max, maxClosed: right.maxClosed }; + } + return { max: left.max, maxClosed: left.maxClosed || right.maxClosed }; +} diff --git a/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/hasGapBetweenIntervals.ts b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/hasGapBetweenIntervals.ts new file mode 100644 index 0000000000..984710c690 --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/hasGapBetweenIntervals.ts @@ -0,0 +1,30 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { NumberInterval } from './types'; + +export function hasGapBetweenIntervals( + left: NumberInterval, + right: NumberInterval, +): boolean { + if (right.min > left.max) { + return true; + } + if (right.min < left.max) { + return false; + } + return !(left.maxClosed || right.minClosed); +} diff --git a/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/mergeNumberIntervals.ts b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/mergeNumberIntervals.ts new file mode 100644 index 0000000000..330605ba5b --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/mergeNumberIntervals.ts @@ -0,0 +1,60 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { NumberInterval } from './types'; +import { getUnionInterval } from './getUnionInterval'; +import { hasGapBetweenIntervals } from './hasGapBetweenIntervals'; + +export function mergeNumberIntervals( + intervals: NumberInterval[], +): NumberInterval[] { + if (intervals.length === 0) { + return []; + } + + const sorted = [...intervals].sort(compareIntervalSort); + + const merged: NumberInterval[] = [{ ...sorted[0]! }]; + + for (let i = 1; i < sorted.length; i++) { + const current = sorted[i]!; + const previous = merged[merged.length - 1]!; + + if (hasGapBetweenIntervals(previous, current)) { + merged.push({ ...current }); + } else { + merged[merged.length - 1] = getUnionInterval(previous, current); + } + } + + return merged; +} + +function compareIntervalSort(a: NumberInterval, b: NumberInterval): number { + if (a.min !== b.min) { + return a.min < b.min ? -1 : 1; + } + if (a.minClosed !== b.minClosed) { + return a.minClosed ? -1 : 1; + } + if (a.max !== b.max) { + return a.max > b.max ? -1 : 1; + } + if (a.maxClosed !== b.maxClosed) { + return a.maxClosed ? -1 : 1; + } + return 0; +} diff --git a/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/normalizeNumberInterval.ts b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/normalizeNumberInterval.ts new file mode 100644 index 0000000000..1efbe21c56 --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/normalizeNumberInterval.ts @@ -0,0 +1,47 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { ComparisonSign } from '../..'; +import { NumberInterval } from './types'; +import { ThresholdConfigFormatError } from '../../../errors'; + +export function normalizeNumberInterval( + value: number, + operator: ComparisonSign, +): NumberInterval[] { + switch (operator) { + case '>=': + return [{ min: value, max: Infinity, minClosed: true, maxClosed: true }]; + case '>': + return [{ min: value, max: Infinity, minClosed: false, maxClosed: true }]; + case '<=': + return [{ min: -Infinity, max: value, minClosed: true, maxClosed: true }]; + case '<': + return [ + { min: -Infinity, max: value, minClosed: true, maxClosed: false }, + ]; + case '==': + return [{ min: value, max: value, minClosed: true, maxClosed: true }]; + case '!=': + return [ + { min: -Infinity, max: value, minClosed: true, maxClosed: false }, + { min: value, max: Infinity, minClosed: false, maxClosed: true }, + ]; + default: { + throw new ThresholdConfigFormatError(`Invalid operator: ${operator}`); + } + } +} diff --git a/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/normalizeRangeInterval.ts b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/normalizeRangeInterval.ts new file mode 100644 index 0000000000..d30302a3e6 --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/normalizeRangeInterval.ts @@ -0,0 +1,25 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { RangeOperator } from '../..'; +import { NumberInterval } from './types'; + +export function normalizeRangeInterval( + values: RangeOperator['values'], +): NumberInterval[] { + const [min, max] = values; + return [{ min, max, minClosed: true, maxClosed: true }]; +} diff --git a/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/types.ts b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/types.ts new file mode 100644 index 0000000000..3eab5b564b --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/types.ts @@ -0,0 +1,22 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export interface NumberInterval { + min: number; + max: number; + minClosed: boolean; + maxClosed: boolean; +} diff --git a/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/validateThresholdNumberIntervals.test.ts b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/validateThresholdNumberIntervals.test.ts new file mode 100644 index 0000000000..59520755a4 --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/validateThresholdNumberIntervals.test.ts @@ -0,0 +1,176 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import type { ThresholdRule } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; +import { ThresholdConfigFormatError } from '../../../errors'; +import { validateThresholdNumberIntervals } from './validateThresholdNumberIntervals'; + +const coverageError = + /do not cover the entire real line|First uncovered region/; + +describe('validateThresholdNumberIntervals', () => { + describe('skipped when not applicable', () => { + it('should not throw for empty rules', () => { + expect(() => + validateThresholdNumberIntervals([], 'number'), + ).not.toThrow(); + }); + + it('should not throw for non-number metric type', () => { + const rules: ThresholdRule[] = [ + { key: 'success', expression: '==true' }, + { key: 'error', expression: '==false' }, + ]; + + expect(() => + validateThresholdNumberIntervals(rules, 'boolean'), + ).not.toThrow(); + }); + + it('should not throw when all rules are discrete number equals', () => { + const rules: ThresholdRule[] = [ + { key: 'A', expression: '==1', color: '#111', icon: 'a' }, + { key: 'B', expression: '==2', color: '#222', icon: 'b' }, + ]; + + expect(() => + validateThresholdNumberIntervals(rules, 'number'), + ).not.toThrow(); + }); + }); + + describe('validates full coverage', () => { + it('should accept a classic three-band partition', () => { + const rules: ThresholdRule[] = [ + { key: 'success', expression: '<10' }, + { key: 'warning', expression: '10-20' }, + { key: 'error', expression: '>20' }, + ]; + + expect(() => + validateThresholdNumberIntervals(rules, 'number'), + ).not.toThrow(); + }); + + it('should accept aggregation-style ordering with overlapping unions covering the real line', () => { + const rules: ThresholdRule[] = [ + { key: 'success', expression: '>=75', color: 'success.main' }, + { key: 'warning', expression: '10-75', color: 'warning.main' }, + { key: 'error', expression: '<10', color: 'error.main' }, + ]; + + expect(() => + validateThresholdNumberIntervals(rules, 'number'), + ).not.toThrow(); + }); + }); + + describe('accepts overlapping intervals', () => { + it('should accept redundant overlapping rules when the union still covers the real line', () => { + const rules: ThresholdRule[] = [ + { key: 'low', expression: '<50' }, + { key: 'mid', expression: '0-100' }, + { key: 'high', expression: '>40' }, + ]; + + expect(() => + validateThresholdNumberIntervals(rules, 'number'), + ).not.toThrow(); + }); + }); + + describe('rejects gaps', () => { + it('should throw error when an internal gap between rules is detected', () => { + const rules: ThresholdRule[] = [ + { key: 'success', expression: '<10' }, + { key: 'warning', expression: '11-20' }, + { key: 'error', expression: '>20' }, + ]; + + expect(() => validateThresholdNumberIntervals(rules, 'number')).toThrow( + ThresholdConfigFormatError, + ); + expect(() => validateThresholdNumberIntervals(rules, 'number')).toThrow( + coverageError, + ); + }); + + it('should throw error when a single-point gap between strict comparisons is detected', () => { + const rules: ThresholdRule[] = [ + { key: 'low', expression: '<10' }, + { key: 'high', expression: '>10' }, + ]; + + expect(() => validateThresholdNumberIntervals(rules, 'number')).toThrow( + coverageError, + ); + }); + + it('should throw error when unions omit tails outside disjoint bounded bands', () => { + const rules: ThresholdRule[] = [ + { key: 'lowBand', expression: '10-20' }, + { key: 'highBand', expression: '30-40' }, + ]; + + expect(() => validateThresholdNumberIntervals(rules, 'number')).toThrow( + coverageError, + ); + }); + + it('should throw error when an open gap toward +∞ is detected (needs at least two rules to evaluate coverage)', () => { + const rules: ThresholdRule[] = [ + { key: 'left', expression: '<100' }, + { key: 'right', expression: '>=200' }, + ]; + + expect(() => validateThresholdNumberIntervals(rules, 'number')).toThrow( + coverageError, + ); + }); + + it('should throw error when an open gap toward -∞ is detected (needs at least two rules to evaluate coverage)', () => { + const rules: ThresholdRule[] = [ + { key: 'left', expression: '<=100' }, + { key: 'right', expression: '>=200' }, + ]; + + expect(() => validateThresholdNumberIntervals(rules, 'number')).toThrow( + coverageError, + ); + }); + }); + + describe('skips coverage check when applicable', () => { + it('should not throw when there is only one number rule', () => { + const rules: ThresholdRule[] = [{ key: 'success', expression: '<10' }]; + + expect(() => + validateThresholdNumberIntervals(rules, 'number'), + ).not.toThrow(); + }); + + it('should throw error when mixing == with continuous rules without full coverage', () => { + const rules: ThresholdRule[] = [ + { key: 'eq', expression: '==5', color: '#111', icon: 'i' }, + { key: 'rest', expression: '>10' }, + ]; + + expect(() => validateThresholdNumberIntervals(rules, 'number')).toThrow( + coverageError, + ); + }); + }); +}); diff --git a/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/validateThresholdNumberIntervals.ts b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/validateThresholdNumberIntervals.ts new file mode 100644 index 0000000000..3c25ce66bd --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/intervals/validateThresholdNumberIntervals.ts @@ -0,0 +1,93 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + MetricType, + ThresholdRule, +} from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; +import { areAllRulesDiscreteNumberEquals } from './areAllRulesDiscreteNumberEquals'; +import { NumberInterval } from './types'; +import { parseThresholdExpression } from '../parseThresholdExpression'; +import { normalizeRangeInterval } from './normalizeRangeInterval'; +import { normalizeNumberInterval } from './normalizeNumberInterval'; +import { ThresholdConfigFormatError } from '../../../errors'; +import { mergeNumberIntervals } from './mergeNumberIntervals'; +import { describeFirstGap } from './describeFirstGap'; + +/** + * Ensures **number** threshold rules jointly cover the entire real line (union of intervals). + * + * @public + */ +export function validateThresholdNumberIntervals( + rules: ThresholdRule[], + expectedMetricType: MetricType, +): void { + const isNumberMetric = expectedMetricType === 'number'; + + if ( + !isNumberMetric || + rules.length <= 1 || + areAllRulesDiscreteNumberEquals(rules) + ) { + return; + } + + const intervals: NumberInterval[] = []; + for (const rule of rules) { + const parsed = parseThresholdExpression( + rule.expression, + expectedMetricType, + ); + + if (parsed.operator === '-') { + intervals.push(...normalizeRangeInterval(parsed.values)); + } else if ( + parsed.value !== undefined && + typeof parsed.value === expectedMetricType + ) { + intervals.push( + ...normalizeNumberInterval(parsed.value as number, parsed.operator), + ); + } else { + throw new ThresholdConfigFormatError( + `Invalid parsed threshold expression: ${JSON.stringify(parsed)}`, + ); + } + } + + const merged = mergeNumberIntervals(intervals); + + if (coversFullRealLine(merged)) { + return; + } + + const gap = describeFirstGap(merged); + + throw new ThresholdConfigFormatError( + `Number threshold rules do not cover the entire real line. First uncovered region (approximately): ${gap}. Adjust expressions so every real value matches at least one rule (union across rules; order only affects which key wins when overlaps exist).`, + ); +} + +function coversFullRealLine(merged: NumberInterval[]): boolean { + return ( + merged.length === 1 && + merged[0]!.min === -Infinity && + merged[0]!.max === Infinity && + merged[0]!.minClosed && + merged[0]!.maxClosed + ); +} diff --git a/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/validateThresholds.test.ts b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/validateThresholds.test.ts index 6a3d1600a0..f2889de6c6 100644 --- a/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/validateThresholds.test.ts +++ b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/validateThresholds.test.ts @@ -56,9 +56,9 @@ describe('validateThresholds', () => { it('should validate config with range expressions', () => { const validConfig = { rules: [ - { key: 'error', expression: '80-100' }, - { key: 'warning', expression: '50-79' }, - { key: 'success', expression: '0-49' }, + { key: 'error', expression: '>=80' }, + { key: 'warning', expression: '50-80' }, + { key: 'success', expression: '<=50' }, ], }; @@ -86,19 +86,19 @@ describe('validateThresholds', () => { }, { key: 'high', - expression: '60-79', + expression: '60-80', color: '#ff9800', icon: '', }, { key: 'medium', - expression: '40-59', + expression: '40-60', color: '#ffc107', icon: 'kind:component', }, { key: 'low', - expression: '20-39', + expression: '20-40', color: '#4caf50', icon: 'https://raw.githubusercontent.com/redhat-developer/example/main/icons/scorecard-icon.svg', }, @@ -167,7 +167,7 @@ describe('validateThresholds', () => { const validConfig = { rules: [ { key: 'warning', expression: '<5', color: 'rgb(255, 87, 51)' }, - { key: 'error', expression: '<5', color: 'rgba(255, 87, 51, 0.5)' }, + { key: 'error', expression: '>=5', color: 'rgba(255, 87, 51, 0.5)' }, ], }; @@ -175,6 +175,74 @@ describe('validateThresholds', () => { validateThresholdsForMetric(validConfig, 'number'), ).not.toThrow(); }); + + it('should reject number rules that leave a gap on the real line for metric', () => { + const gapConfig = { + rules: [ + { key: 'success', expression: '<10' }, + { key: 'warning', expression: '11-20' }, + { key: 'error', expression: '>20' }, + ], + }; + + expect(() => validateThresholdsForMetric(gapConfig, 'number')).toThrow( + ThresholdConfigFormatError, + ); + expect(() => validateThresholdsForMetric(gapConfig, 'number')).toThrow( + /do not cover the entire real line/, + ); + }); + + it('should throw error when threshold rules do not cover the entire real line for aggregation', () => { + const gapConfig = { + rules: [ + { key: 'success', expression: '<10' }, + { key: 'warning', expression: '11-20' }, + { key: 'error', expression: '>20' }, + ], + }; + expect(() => + validateThresholdsForAggregation(gapConfig, 'number'), + ).toThrow(ThresholdConfigFormatError); + expect(() => + validateThresholdsForAggregation(gapConfig, 'number'), + ).toThrow(/do not cover the entire real line/); + }); + + it('should accept number rules that partition the real line for metrics and aggregation', () => { + const config = { + rules: [ + { key: 'success', expression: '<10' }, + { key: 'warning', expression: '10-20' }, + { key: 'error', expression: '>20' }, + ], + }; + + expect(() => validateThresholdsForMetric(config, 'number')).not.toThrow(); + expect(() => + validateThresholdsForAggregation(config, 'number'), + ).not.toThrow(); + }); + + it('should not run interval coverage when there is only one number rule', () => { + expect(() => + validateThresholdsForMetric( + { rules: [{ key: 'success', expression: '<10' }] }, + 'number', + ), + ).not.toThrow(); + }); + + it('should accept discrete == number rules', () => { + const config = { + rules: [ + { key: 'A', expression: '==1', color: '#111', icon: 'a' }, + { key: 'B', expression: '==2', color: '#222', icon: 'b' }, + ], + }; + + expect(() => validateThresholdsForMetric(config, 'number')).not.toThrow(); + }); }); describe('validateThresholds - invalid configs', () => { @@ -413,7 +481,7 @@ describe('validateThresholds', () => { const aggregationStyleConfig = { rules: [ { key: 'success', expression: '>=75', color: 'success.main' }, - { key: 'warning', expression: '10-74', color: 'warning.main' }, + { key: 'warning', expression: '10-75', color: 'warning.main' }, { key: 'error', expression: '<10', color: 'error.main' }, ], }; diff --git a/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/validateThresholds.ts b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/validateThresholds.ts index 7498a02b86..1fa04bd97f 100644 --- a/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/validateThresholds.ts +++ b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/validateThresholds.ts @@ -23,6 +23,7 @@ import type { import { SCORECARD_THRESHOLD_RULE_COLOR_VALUES } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; import { ThresholdConfigFormatError } from '../../errors'; import { parseThresholdExpression } from './parseThresholdExpression'; +import { validateThresholdNumberIntervals } from './intervals/validateThresholdNumberIntervals'; const THRESHOLD_RULE_KEYS = ['success', 'warning', 'error']; @@ -48,6 +49,8 @@ export function validateThresholdsForMetric( parseThresholdExpression(rule.expression, expectedMetricType); } + + validateThresholdNumberIntervals(thresholds.rules, expectedMetricType); } /** @@ -71,6 +74,8 @@ export function validateThresholdsForAggregation( parseThresholdExpression(rule.expression, expectedMetricType); } + + validateThresholdNumberIntervals(thresholds.rules, expectedMetricType); } /** diff --git a/workspaces/scorecard/plugins/scorecard/src/components/AggregatedMetricCards/AverageCard/AverageCardComponent.tsx b/workspaces/scorecard/plugins/scorecard/src/components/AggregatedMetricCards/AverageCard/AverageCardComponent.tsx index eb36e01289..36eddc27ec 100644 --- a/workspaces/scorecard/plugins/scorecard/src/components/AggregatedMetricCards/AverageCard/AverageCardComponent.tsx +++ b/workspaces/scorecard/plugins/scorecard/src/components/AggregatedMetricCards/AverageCard/AverageCardComponent.tsx @@ -74,7 +74,8 @@ export const AverageCardComponent = ({ }); }; - const rawPercent = scorecard.result.averageScore * 100; + const rawPercent = scorecard.result.averageScore; + const { fill: chartFillPercent, remainder: chartRemainderPercent } = clampPercentForDonut(rawPercent); diff --git a/workspaces/scorecard/plugins/scorecard/src/components/ScorecardHomepageSection/__tests__/ScorecardHomepageCard.test.tsx b/workspaces/scorecard/plugins/scorecard/src/components/ScorecardHomepageSection/__tests__/ScorecardHomepageCard.test.tsx index 7dc804ef7d..91621bbd46 100644 --- a/workspaces/scorecard/plugins/scorecard/src/components/ScorecardHomepageSection/__tests__/ScorecardHomepageCard.test.tsx +++ b/workspaces/scorecard/plugins/scorecard/src/components/ScorecardHomepageSection/__tests__/ScorecardHomepageCard.test.tsx @@ -172,7 +172,7 @@ const mockAverageScorecard: AggregatedMetricResult = { }, result: { ...mockScorecard.result, - averageScore: 0.75, + averageScore: 75, averageWeightedSum: 18, averageMaxPossible: 24, aggregationChartDisplayColor: 'warning.main',