From bf9f0ed9822c8ee72ca25b5caecb35a2cd4a5881 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Sat, 24 Oct 2026 14:08:19 +0100 Subject: [PATCH] Cap retained chart observations before allocating wire points Share the existing time-nearest selection by original indices, so chart conversion materialises only selected observations. Preserve exact timestamps, values, tie breaks, degenerate spans and oldest-source metadata. Check the frozen former sampler across 1,440 cases and the exported fallback contract. Change-source: pulse-maintainer Contract-Neutral: Exact chart observations, wire schema, sampling policy and guest/storage lifecycle contracts are unchanged; only index selection and allocation order change. --- .../v6/internal/subsystems/api-contracts.md | 19 ++ .../subsystems/performance-and-scalability.md | 19 ++ internal/api/chartapi/charts_test.go | 176 +++++++++++++++ internal/api/chartapi/service.go | 213 ++++++++++-------- internal/api/contract_test.go | 26 +++ 5 files changed, 356 insertions(+), 97 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 89c19de2ad..d699383fa8 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -818,6 +818,25 @@ executes a terminal-free Unix recorder and inspects explicit Windows parameter binding. Browser evidence is UI-only; server/native deregistration and Windows failure retention require their own owner result, not source acceptance. +### Chart caps select before wire conversion (24 October 2026) + +The existing time-nearest chart cap selects indices in the original series +before allocating wire-format points. Its equal-time targets, latest-time tie +break, first/last observations and degenerate-span index fallback are unchanged. +No average, synthetic timestamp or new sampling policy is introduced; zero and +selected extrema retain their exact values. Oldest-data metadata still examines +every eligible source observation, including one omitted from display. Empty +series remain empty and each converted response owns its point slices. The +retained reader, LTTB stage, source/alias choice, scopes and cache budgets stay +unchanged. + +`TestChartPointSelectionMatchesOriginalCap` compares the frozen former algorithm +with both wire and source-point paths over mixed cadence, millisecond ties, +negative epochs, unsorted/degenerate spans and caps from unlimited to one point. +`TestContract_ChartIndexFallbackPreservesObservations` checks the exported fallback +with equal-time, fractional, negative and explicit-zero observations. Source +parity and consuming browser evidence remain separate acceptance obligations. + ## Canonical Files 1a. `docs/API.md` diff --git a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md index 3122fd34a0..a769962262 100644 --- a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md +++ b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md @@ -626,6 +626,25 @@ when the row is clicked: no document listener, timer, signal, metric derivation or History work is added per row. `GuestRow.test.tsx` covers the drag, the in-place click and chevron activation over a selection. +### Chart caps select before wire conversion (24 October 2026) + +The existing time-nearest chart cap selects indices in the original series +before allocating wire-format points. Its equal-time targets, latest-time tie +break, first/last observations and degenerate-span index fallback are unchanged. +No average, synthetic timestamp or new sampling policy is introduced; zero and +selected extrema retain their exact values. Oldest-data metadata still examines +every eligible source observation, including one omitted from display. Empty +series remain empty and each converted response owns its point slices. The +retained reader, LTTB stage, source/alias choice, scopes and cache budgets stay +unchanged. + +`TestChartPointSelectionMatchesOriginalCap` compares the frozen former algorithm +with both wire and source-point paths over mixed cadence, millisecond ties, +negative epochs, unsorted/degenerate spans and caps from unlimited to one point. +`TestContract_ChartIndexFallbackPreservesObservations` checks the exported fallback +with equal-time, fractional, negative and explicit-zero observations. Source +parity and consuming browser evidence remain separate acceptance obligations. + ## Canonical Files - `frontend-modern/src/utils/workloadDiskUsage.ts` — aggregate disk numeric/current-reading admission. diff --git a/internal/api/chartapi/charts_test.go b/internal/api/chartapi/charts_test.go index 67909cea7c..5d17d9c25e 100644 --- a/internal/api/chartapi/charts_test.go +++ b/internal/api/chartapi/charts_test.go @@ -2,6 +2,10 @@ package chartapi import ( "encoding/json" + "fmt" + "math" + "math/rand" + "reflect" "testing" "time" @@ -651,3 +655,175 @@ func TestEmptyDockerData(t *testing.T) { t.Errorf("dockerData should be an empty object, not missing") } } + +// A frozen copy of the former wire-point cap is the oracle: source-point +// selection must preserve its exact timestamps/values, not just a point count. +func referenceChartCapByIndex(points []MetricPoint, maxPoints int) []MetricPoint { + if len(points) <= maxPoints || maxPoints <= 0 { + return points + } + if maxPoints == 1 { + return []MetricPoint{points[len(points)-1]} + } + + result := make([]MetricPoint, 0, maxPoints) + step := float64(len(points)-1) / float64(maxPoints-1) + prevIndex := -1 + + for i := 0; i < maxPoints; i++ { + index := int(float64(i)*step + 0.5) + if index <= prevIndex { + index = prevIndex + 1 + } + if index >= len(points) { + index = len(points) - 1 + } + result = append(result, points[index]) + prevIndex = index + } + + if result[len(result)-1].Timestamp != points[len(points)-1].Timestamp { + result[len(result)-1] = points[len(points)-1] + } + return result +} + +func referenceChartCap(points []MetricPoint, maxPoints int) []MetricPoint { + if len(points) <= maxPoints || maxPoints <= 0 { + return points + } + if maxPoints == 1 { + return []MetricPoint{points[len(points)-1]} + } + + startTimestamp := points[0].Timestamp + endTimestamp := points[len(points)-1].Timestamp + if endTimestamp <= startTimestamp { + return referenceChartCapByIndex(points, maxPoints) + } + + bucketSpan := float64(endTimestamp-startTimestamp) / float64(maxPoints-1) + if bucketSpan < 1 { + return referenceChartCapByIndex(points, maxPoints) + } + + type timeBucketRepresentative struct { + point MetricPoint + distance float64 + ok bool + } + + buckets := make([]timeBucketRepresentative, maxPoints) + for _, point := range points { + index := int(math.Round(float64(point.Timestamp-startTimestamp) / bucketSpan)) + if index < 0 { + index = 0 + } + if index >= maxPoints { + index = maxPoints - 1 + } + + targetTimestamp := float64(startTimestamp) + bucketSpan*float64(index) + distance := math.Abs(float64(point.Timestamp) - targetTimestamp) + current := buckets[index] + if !current.ok || + distance < current.distance || + (distance == current.distance && point.Timestamp > current.point.Timestamp) { + buckets[index] = timeBucketRepresentative{ + point: point, + distance: distance, + ok: true, + } + } + } + + result := make([]MetricPoint, 0, maxPoints) + result = append(result, points[0]) + lastAddedTimestamp := points[0].Timestamp + for index := 1; index < maxPoints-1; index++ { + bucket := buckets[index] + if !bucket.ok { + continue + } + if bucket.point.Timestamp <= lastAddedTimestamp { + continue + } + result = append(result, bucket.point) + lastAddedTimestamp = bucket.point.Timestamp + } + + lastPoint := points[len(points)-1] + if lastPoint.Timestamp <= lastAddedTimestamp { + result[len(result)-1] = lastPoint + return result + } + + result = append(result, lastPoint) + return result +} + +func TestChartPointSelectionMatchesOriginalCap(t *testing.T) { + t.Parallel() + rng := rand.New(rand.NewSource(20261024)) + for scenario := 0; scenario < 160; scenario++ { + n := []int{0, 1, 2, 3, 17, 121, 240, 500, 777}[scenario%9] + points := make([]MetricPoint, n) + stamp := int64(1_800_000_000_123) + if scenario%11 == 0 { + stamp = -100000 + } + for i := range points { + step := int64(rng.Intn(4000)) + if i > n/2 { + step %= 5 // dense recent tail, duplicate/millisecond timestamps + } + stamp += step + points[i] = MetricPoint{Timestamp: stamp, Value: float64(rng.Intn(201)-100) / 7} + } + if n > 2 { + points[n-1].Value = 0 + switch scenario % 5 { + case 0: + for i := range points { + points[i].Timestamp = stamp // no positive span: index fallback + } + case 1: + points[0], points[n-1] = points[n-1], points[0] + case 2: + points[n/2].Timestamp = points[0].Timestamp - 7 // unselected minimum + } + } + for _, cap := range []int{-1, 0, 1, 2, 3, 30, 36, 120, 500} { + t.Run(fmt.Sprintf("%d/cap=%d", scenario, cap), func(t *testing.T) { + want := referenceChartCap(points, cap) + if got := capMetricPointSeries(points, cap); !reflect.DeepEqual(got, want) { + t.Fatal("time cap changed its selected observations") + } + if got, want := capMetricPointSeriesByIndex(points, cap), referenceChartCapByIndex(points, cap); !reflect.DeepEqual(got, want) { + t.Fatal("index fallback changed its selected observations") + } + source := make([]monitoring.MetricPoint, n) + wantOldest := int64(math.MaxInt64) + for i, point := range points { + source[i] = monitoring.MetricPoint{Timestamp: time.UnixMilli(point.Timestamp), Value: point.Value} + if point.Timestamp < wantOldest { + wantOldest = point.Timestamp + } + } + oldest := int64(math.MaxInt64) + got := convertMetricsForChart(map[string][]monitoring.MetricPoint{ + "cpu": source, "not-a-sparkline": {{Timestamp: time.UnixMilli(math.MinInt64), Value: -999}}, + }, &oldest, cap) + if len(got) != 1 || !reflect.DeepEqual(got["cpu"], want) || oldest != wantOldest { + t.Fatalf("source conversion/cap/oldest changed: oldest %d, want %d", oldest, wantOldest) + } + if len(got["cpu"]) > 0 { + got["cpu"][0].Value = 9999 + if source[0].Value != points[0].Value { + t.Fatal("wire output aliases retained source") + } + } + }) + } + } +} diff --git a/internal/api/chartapi/service.go b/internal/api/chartapi/service.go index 9793337aa4..10062b7e8e 100644 --- a/internal/api/chartapi/service.go +++ b/internal/api/chartapi/service.go @@ -623,28 +623,10 @@ func capMetricPointSeriesByIndex(points []MetricPoint, maxPoints int) []MetricPo if len(points) <= maxPoints || maxPoints <= 0 { return points } - if maxPoints == 1 { - return []MetricPoint{points[len(points)-1]} - } - - result := make([]MetricPoint, 0, maxPoints) - step := float64(len(points)-1) / float64(maxPoints-1) - prevIndex := -1 - - for i := 0; i < maxPoints; i++ { - index := int(float64(i)*step + 0.5) - if index <= prevIndex { - index = prevIndex + 1 - } - if index >= len(points) { - index = len(points) - 1 - } - result = append(result, points[index]) - prevIndex = index - } - - if result[len(result)-1].Timestamp != points[len(points)-1].Timestamp { - result[len(result)-1] = points[len(points)-1] + indices := chartPointIndicesByIndex(len(points), maxPoints) + result := make([]MetricPoint, len(indices)) + for i, index := range indices { + result[i] = points[index] } return result } @@ -660,6 +642,89 @@ const ( const InfrastructureSummaryMaxSeriesPoints = infrastructureSummaryMaxSeriesPoints +func chartPointIndicesByIndex(count, maxPoints int) []int { + if maxPoints == 1 { + return []int{count - 1} + } + indices := make([]int, 0, maxPoints) + step := float64(count-1) / float64(maxPoints-1) + prevIndex := -1 + for i := 0; i < maxPoints; i++ { + index := int(float64(i)*step + 0.5) + if index <= prevIndex { + index = prevIndex + 1 + } + if index >= count { + index = count - 1 + } + indices = append(indices, index) + prevIndex = index + } + return indices +} + +// chartPointIndices selects original observations, not averages or synthetic +// timestamps. Call only when count > maxPoints > 0. Keeping indices lets the +// retained reader apply the same cap before allocating wire-format points. +func chartPointIndices(count, maxPoints int, timestampAt func(int) int64) []int { + if maxPoints == 1 { + return []int{count - 1} + } + startTimestamp, endTimestamp := timestampAt(0), timestampAt(count-1) + if endTimestamp <= startTimestamp { + return chartPointIndicesByIndex(count, maxPoints) + } + bucketSpan := float64(endTimestamp-startTimestamp) / float64(maxPoints-1) + if bucketSpan < 1 { + return chartPointIndicesByIndex(count, maxPoints) + } + + type representative struct { + indexPlusOne int // zero means no observation in this time bucket + distance float64 + } + buckets := make([]representative, maxPoints) + for pointIndex := 0; pointIndex < count; pointIndex++ { + timestamp := timestampAt(pointIndex) + index := int(math.Round(float64(timestamp-startTimestamp) / bucketSpan)) + if index < 0 { + index = 0 + } + if index >= maxPoints { + index = maxPoints - 1 + } + targetTimestamp := float64(startTimestamp) + bucketSpan*float64(index) + distance := math.Abs(float64(timestamp) - targetTimestamp) + current := buckets[index] + if current.indexPlusOne == 0 || distance < current.distance || + (distance == current.distance && timestamp > timestampAt(current.indexPlusOne-1)) { + buckets[index] = representative{indexPlusOne: pointIndex + 1, distance: distance} + } + } + + result := make([]int, 0, maxPoints) + result = append(result, 0) + lastAddedTimestamp := startTimestamp + for index := 1; index < maxPoints-1; index++ { + bucket := buckets[index] + if bucket.indexPlusOne == 0 { + continue + } + pointIndex := bucket.indexPlusOne - 1 + timestamp := timestampAt(pointIndex) + if timestamp <= lastAddedTimestamp { + continue + } + result = append(result, pointIndex) + lastAddedTimestamp = timestamp + } + if endTimestamp <= lastAddedTimestamp { + result[len(result)-1] = count - 1 + return result + } + return append(result, count-1) +} + // capMetricPointSeries keeps mixed-cadence series visually proportional across // the selected time window. Index-based capping over-selects recent dense // samples, which bunches the right edge on long ranges. @@ -667,73 +732,11 @@ func capMetricPointSeries(points []MetricPoint, maxPoints int) []MetricPoint { if len(points) <= maxPoints || maxPoints <= 0 { return points } - if maxPoints == 1 { - return []MetricPoint{points[len(points)-1]} + indices := chartPointIndices(len(points), maxPoints, func(i int) int64 { return points[i].Timestamp }) + result := make([]MetricPoint, len(indices)) + for i, index := range indices { + result[i] = points[index] } - - startTimestamp := points[0].Timestamp - endTimestamp := points[len(points)-1].Timestamp - if endTimestamp <= startTimestamp { - return capMetricPointSeriesByIndex(points, maxPoints) - } - - bucketSpan := float64(endTimestamp-startTimestamp) / float64(maxPoints-1) - if bucketSpan < 1 { - return capMetricPointSeriesByIndex(points, maxPoints) - } - - type timeBucketRepresentative struct { - point MetricPoint - distance float64 - ok bool - } - - buckets := make([]timeBucketRepresentative, maxPoints) - for _, point := range points { - index := int(math.Round(float64(point.Timestamp-startTimestamp) / bucketSpan)) - if index < 0 { - index = 0 - } - if index >= maxPoints { - index = maxPoints - 1 - } - - targetTimestamp := float64(startTimestamp) + bucketSpan*float64(index) - distance := math.Abs(float64(point.Timestamp) - targetTimestamp) - current := buckets[index] - if !current.ok || - distance < current.distance || - (distance == current.distance && point.Timestamp > current.point.Timestamp) { - buckets[index] = timeBucketRepresentative{ - point: point, - distance: distance, - ok: true, - } - } - } - - result := make([]MetricPoint, 0, maxPoints) - result = append(result, points[0]) - lastAddedTimestamp := points[0].Timestamp - for index := 1; index < maxPoints-1; index++ { - bucket := buckets[index] - if !bucket.ok { - continue - } - if bucket.point.Timestamp <= lastAddedTimestamp { - continue - } - result = append(result, bucket.point) - lastAddedTimestamp = bucket.point.Timestamp - } - - lastPoint := points[len(points)-1] - if lastPoint.Timestamp <= lastAddedTimestamp { - result[len(result)-1] = lastPoint - return result - } - - result = append(result, lastPoint) return result } @@ -1103,18 +1106,34 @@ func convertMetricsForChart( if !sparklineMetrics[metricType] { continue } - points := make([]MetricPoint, len(metricPoints)) - for i, point := range metricPoints { - ts := point.Timestamp.UnixMilli() - if ts < *oldestTimestamp { + if len(metricPoints) <= maxPoints || maxPoints <= 0 { + points := make([]MetricPoint, len(metricPoints)) + for i, point := range metricPoints { + ts := point.Timestamp.UnixMilli() + if ts < *oldestTimestamp { + *oldestTimestamp = ts + } + points[i] = MetricPoint{Timestamp: ts, Value: point.Value} + } + converted[metricType] = points + continue + } + // Oldest-data metadata describes the source observations, not just + // those selected for display (also for degenerate/unsorted inputs). + for _, point := range metricPoints { + if ts := point.Timestamp.UnixMilli(); ts < *oldestTimestamp { *oldestTimestamp = ts } - points[i] = MetricPoint{ - Timestamp: ts, - Value: point.Value, - } } - converted[metricType] = capMetricPointSeries(points, maxPoints) + indices := chartPointIndices(len(metricPoints), maxPoints, func(i int) int64 { + return metricPoints[i].Timestamp.UnixMilli() + }) + points := make([]MetricPoint, len(indices)) + for i, index := range indices { + point := metricPoints[index] + points[i] = MetricPoint{Timestamp: point.Timestamp.UnixMilli(), Value: point.Value} + } + converted[metricType] = points } return converted } diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index 10874278d5..89492b5a7e 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -27781,3 +27781,29 @@ func TestContractSSOAmbiguousSavePreservesDetail(t *testing.T) { t.Fatal("detail lost its access selection or exposed the saved secret") } } + +// The cap's degenerate-time fallback selects actual observations. Equal +// timestamps cannot authorise averaging values or replacing explicit zero. +func TestContract_ChartIndexFallbackPreservesObservations(t *testing.T) { + points := []chartapi.MetricPoint{ + {Timestamp: 1800000000123, Value: 0.125}, + {Timestamp: 1800000000123, Value: 91.75}, + {Timestamp: 1800000000123, Value: -7.5}, + {Timestamp: 1800000000123, Value: 63.875}, + {Timestamp: 1800000000123, Value: 0}, + } + for _, tc := range []struct { + cap int + indices []int + }{{0, []int{0, 1, 2, 3, 4}}, {1, []int{4}}, {3, []int{0, 2, 4}}, {4, []int{0, 1, 3, 4}}} { + got := chartapi.CapMetricPointSeriesByIndex(points, tc.cap) + if len(got) != len(tc.indices) { + t.Fatalf("cap %d: %d observations, want %d", tc.cap, len(got), len(tc.indices)) + } + for i, index := range tc.indices { + if got[i] != points[index] { + t.Fatalf("cap %d index %d: observation changed: %+v", tc.cap, index, got[i]) + } + } + } +}