From 37e749b8a57dcad813e8084ddc7a773b1dd1b730 Mon Sep 17 00:00:00 2001 From: jgjesdal Date: Thu, 17 Sep 2026 09:41:36 +0200 Subject: [PATCH 1/2] fix: binary ingest builds against ResponseError::content_type #123 was branched before #128 gave ResponseError a content_type field and merged after it, so main stopped compiling at the three places the binary path builds one. All three are raised by the SDK with no server response behind them (a 204 that fails to deserialize, a value refused locally, a series /byids cannot find), so each carries None. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: jgjesdal --- src/generic.rs | 1 + src/timeseries/binary.rs | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/generic.rs b/src/generic.rs index 56ed379..feddc72 100644 --- a/src/generic.rs +++ b/src/generic.rs @@ -865,6 +865,7 @@ pub trait ApiServiceProvider { ResponseError { status: response.status(), message: err.to_string(), + content_type: None, } }); } diff --git a/src/timeseries/binary.rs b/src/timeseries/binary.rs index 0ebea83..fee9834 100644 --- a/src/timeseries/binary.rs +++ b/src/timeseries/binary.rs @@ -737,6 +737,7 @@ fn unprocessable(message: String) -> ResponseError { ResponseError { status: StatusCode::UNPROCESSABLE_ENTITY, message, + content_type: None, } } @@ -931,6 +932,7 @@ impl TimeSeriesService { return Err(ResponseError { status: StatusCode::NOT_FOUND, message: format!("Could not find following timeseries: {}", missing.join(", ")), + content_type: None, }); } Ok(resolved) From e01437e22ef3763f823c7f4023d3235738ecba8b Mon Sep 17 00:00:00 2001 From: jgjesdal Date: Thu, 17 Sep 2026 09:41:51 +0200 Subject: [PATCH 2/2] fix(tests): compare the two ingest paths' daily avg within tolerance test_datapoints_binary truncated avg/min/max to ten decimals and required the binary and JSON series to match exactly. It failed twice in a row on one bucket of 60 (2025-02-28): avg 179.9759117537 against ...538, min and max identical, both series walked to all 5,184,000 points. A dropped or duplicated point moves a daily avg by ~1e-3; a 1e-10 difference is ClickHouse summing the two series' parts in a different order, landing either side of the truncation boundary. min and max select a stored value, so they stay exact and still catch the two paths storing different values. avg is compared within AVG_TOLERANCE, the bound validate_daily_avg already uses for the same reason. truncate_10 goes with its last caller, and an aggregate the read path leaves out now fails by name instead of as a NaN comparison. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: jgjesdal --- src/timeseries/test.rs | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/src/timeseries/test.rs b/src/timeseries/test.rs index 4512a75..d1b0caf 100644 --- a/src/timeseries/test.rs +++ b/src/timeseries/test.rs @@ -676,10 +676,23 @@ mod tests { let binary_aggs = daily_aggregates(&api_service, &new_ts_ext_id).await; let json_aggs = daily_aggregates(&api_service, &json_ext_id).await; assert!(!binary_aggs.is_empty(), "no daily buckets came back"); - assert_eq!( - binary_aggs, json_aggs, - "the binary path aggregates differently from the JSON path for identical input" - ); + assert_eq!(binary_aggs.len(), json_aggs.len(), "the two paths answered a different number of buckets"); + // min and max select a stored value, so they must match exactly. avg is computed, and + // ClickHouse sums the two series' parts in different orders. + for ((bucket, binary_avg, binary_min, binary_max), (json_bucket, json_avg, json_min, json_max)) in + binary_aggs.into_iter().zip(json_aggs) + { + assert_eq!(bucket, json_bucket, "the two paths answered different buckets"); + assert_eq!( + (binary_min, binary_max), + (json_min, json_max), + "bucket {bucket}: min/max differ, so the two paths stored different values" + ); + assert!( + (binary_avg - json_avg).abs() < AVG_TOLERANCE, + "bucket {bucket}: avg {binary_avg} from the binary path against {json_avg} from JSON" + ); + } println!("Validate raw datapoints with a cursor walk..."); // Not validate_raw_datapoints_with_cursor: that one asserts a fixed final page size @@ -878,15 +891,7 @@ mod tests { } } - fn truncate_10(x: f64) -> f64 { - // Clickhouse will have rounding errors using for example avg(), so we truncate the returned - // values to mitigate this - let multiplier = 10f64.powf(10.0); - (x * multiplier).floor() / multiplier - } - - /// Daily avg/min/max for the whole window, truncated so two paths that stored the same - /// values compare equal without depending on how many digits the read path prints. + /// `(bucket start millis, avg, min, max)` per day for the whole window. async fn daily_aggregates( api_service: &Arc, external_id: &str, @@ -913,9 +918,9 @@ mod tests { .map(|dp| { ( dp.timestamp().timestamp_millis(), - truncate_10(dp.average().unwrap_or(f64::NAN)), - truncate_10(dp.min().unwrap_or(f64::NAN)), - truncate_10(dp.max().unwrap_or(f64::NAN)), + dp.average().expect("a daily bucket without avg"), + dp.min().expect("a daily bucket without min"), + dp.max().expect("a daily bucket without max"), ) }) .collect()