From 1b0f236cc654499c3d731ae0fa2d94206c8d2ec1 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Sat, 11 Jul 2026 12:10:28 -0600 Subject: [PATCH] fix(status): require total, not errors, for the error-rate panel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The error-rate derived-metric override declared ['count', 'errors'], but recomputeErrorRate reads count and total — never errors. On a Harper whose success records lack an errors column, MetricPanel showed a false "field unavailable" empty state for a chart that would render fine, while a genuinely missing total (which silently zeroes the numerator and renders a flat 100% error rate) was never flagged. Change the override to ['count', 'total'] and pin it behaviorally: the new tests feed recomputeErrorRate records with each declared required field omitted and assert the output changes (load-bearing), and assert that omitting errors leaves the output untouched — so the test fails if either the override or the recompute drifts. Fixes #1441 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UZEaYdKFXuQw2Hf6XpPdEV --- .../analytics/lib/specRequiredFields.test.ts | 33 +++++++++++++++++++ .../analytics/lib/specRequiredFields.ts | 2 +- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/features/instance/status/analytics/lib/specRequiredFields.test.ts b/src/features/instance/status/analytics/lib/specRequiredFields.test.ts index a50464cdb..4b78ea5a4 100644 --- a/src/features/instance/status/analytics/lib/specRequiredFields.test.ts +++ b/src/features/instance/status/analytics/lib/specRequiredFields.test.ts @@ -1,4 +1,6 @@ import { describe, expect, it } from 'vitest'; +import { recomputeErrorRate } from '../pipeline/derived/error-rate.tsx'; +import type { AnalyticsDataPoint, TimeRange } from '../types/analytics.ts'; import { getSpecRequiredFields } from './specRequiredFields.ts'; describe('getSpecRequiredFields', () => { @@ -29,6 +31,37 @@ describe('getSpecRequiredFields', () => { const errRate = getSpecRequiredFields('error-rate'); expect(errRate).toContain('count'); + expect(errRate).toContain('total'); + }); + + // Pin the error-rate override to what `recomputeErrorRate` actually reads. + // The override once listed `errors` (which the recompute never touches), + // blanking a working chart on Harpers whose success records lack that + // column — while a genuinely missing `total` went unflagged (#1441). + describe('error-rate override vs recomputeErrorRate behavior', () => { + const window: TimeRange = { startTime: 0, endTime: 60_000 }; + // Σ-arithmetic fixture from the error-rate spec: baseline ≈ 0.0188. + const records: AnalyticsDataPoint[] = [ + { time: 1_000, node: 'n1', path: '/api', count: 1000, total: 990, errors: 10 }, + { time: 1_000, node: 'n1', path: '/api', count: 10, total: 1, errors: 9 }, + ]; + const firstY = (recs: AnalyticsDataPoint[]) => + recomputeErrorRate(recs, window, ['n1']).series[0]?.points[0]?.y ?? null; + const omit = (field: string) => records.map(({ [field]: _omitted, ...rest }) => rest as AnalyticsDataPoint); + + it('every declared required field is load-bearing for the recompute', () => { + const baseline = firstY(records); + expect(baseline).toBeCloseTo(1 - 991 / 1010, 6); + for (const field of getSpecRequiredFields('error-rate')) { + expect(firstY(omit(field)), `dropping required field "${field}" should change the output`) + .not.toEqual(baseline); + } + }); + + it('does NOT require `errors` — the recompute never reads it', () => { + expect(getSpecRequiredFields('error-rate')).not.toContain('errors'); + expect(firstY(omit('errors'))).toEqual(firstY(records)); + }); }); it('returns [] for a derived metric without an explicit override', () => { diff --git a/src/features/instance/status/analytics/lib/specRequiredFields.ts b/src/features/instance/status/analytics/lib/specRequiredFields.ts index d727aa7c4..ea47c1be1 100644 --- a/src/features/instance/status/analytics/lib/specRequiredFields.ts +++ b/src/features/instance/status/analytics/lib/specRequiredFields.ts @@ -9,7 +9,7 @@ import type { FieldExpr, FieldSpec, MetricSpec, Transform } from '../types/analy * the source-record fields the recompute reads. */ const DERIVED_REQUIRED_FIELDS: Record = { 'request-rate': ['count', 'period'], - 'error-rate': ['count', 'errors'], + 'error-rate': ['count', 'total'], // mqtt-traffic-* delegate to runPipeline against bytes-{sent,received} // inner specs, so they are covered transitively through the source spec // and the renderer fetches the source metric directly.