Fix aggregate crashing on operations that report null metrics

[serhiy-bzhezytskyy]

Description

calculate_weighted_average reduced min/max with value.get(metric_field, 0). That default only applies when the key is absent, so a key present with value None reached min()/max() and raised TypeError: '<' not supported between instances of 'NoneType' and 'NoneType'. The percentile/median branch just below had the same flaw in a different shape — value * iterations on None.

calculate_rsd is a second, independent site, reached from build_aggregated_results_dict with the per-run mean values (TypeError: can't convert type 'NoneType' to numerator/denominator). Fixing only the first one moves the crash rather than removing it — that is how I found the second, by running against real data after unit tests for the first were already green.

The change:

• extracts the weighted mean into a weighted_mean helper. The divisor now depends on which runs contributed a value, so it can no longer be one total summed up front, and the dict and scalar branches were computing it two different ways;
• leaves nulls out of both the weighted sum and its divisor, so a run that measured nothing does not drag the mean toward zero;
• propagates None when no run contributed a value, rather than substituting 0. 0 would read as "throughput was zero", which is a different claim from "not measured". This matches how aggregate_json_by_key in the same file already treats nulls;
• returns NA from calculate_rsd when no values remain, as it already does for the single-value case.

Issues Resolved

Resolves #1093

Testing

• New functionality includes testing

• Two new tests in tests/aggregator_test.py: all-null metric fields, and partially null — a metric with samples in one run but not another, where the valid values must still aggregate, weighted only by the runs that contributed them.

• Both verified red without the fix, by restoring main's aggregator.py under the new tests rather than by inspection.

• Full suite: 1424 passed, 5 skipped (baseline on main is 1422). pylint clean.

• End-to-end through the real CLI, on test-run files written by OSB's own TestRun.as_dict(): aggregate goes from ❌ FAILURE to ✅ SUCCESS, and the output is correct rather than merely non-crashing — a healthy operation aggregates to overall_min 40000.0, mean 42250.0, median 42150.0, overall_max 44500.0 with mean_rsd 0.8368, while the null operation reports null throughput and mean_rsd "NA".

The calculate_rsd site has no dedicated unit test here; it is covered by the end-to-end run, where it was the crash that surfaced once the first site was fixed. I can add a direct one if you'd prefer it in the suite.

Notes

Found while using Apache solr-orbit, a Python port of OSB whose aggregator.py is byte-identical to this one apart from the import module name, to run a multi-configuration benchmark campaign. The same fix is proposed there as apache/solr-orbit#58. Reported here because the defect is upstream, not port-specific.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit aa93387)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ No major issues detected

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Guard RSD against zero mean division statistics.mean will raise StatisticsError when mean is 0 during the subsequent RSD division, or produce a division-by-zero. After filtering None values, if all remaining values are 0 the RSD calculation std_dev / mean * 100 will fail. Consider guarding against a zero mean by returning "NA" in that case as well. osbenchmark/aggregator.py [256-265] if not values: raise ValueError(f"Cannot calculate RSD for metric '{metric_name}': empty list of values") # operations that produced no valid samples report None, which cannot contribute to a deviation values = [value for value in values if value is not None] if not values: return "NA" # no test run measured this metric if len(values) == 1: return "NA" # RSD is not applicable for a single value mean = statistics.mean(values) +if mean == 0: + return "NA" # RSD is not defined when mean is zero std_dev = statistics.stdev(values) Suggestion importance[1-10]: 5 __ Why: The suggestion identifies a legitimate edge case where a zero mean would cause a division-by-zero in the subsequent RSD calculation. However, this is a pre-existing issue not directly related to the PR's null-handling changes, and zero means may be uncommon in practice.	Low

[OVI3D0]

@serhiy-bzhezytskyy tested and verified this works, LGTM, but looks like there is just a conflict on the test file if you can do a quick rebase

[serhiy-bzhezytskyy]

Rebased onto main — the conflict was in tests/aggregator_test.py, where #1097 and this PR each appended a test after the same anchor. Kept both; the suite is at 1425 passed / 5 skipped locally.

Thanks for reviewing and merging #1097.

[github-actions]

Persistent review updated to latest commit aa93387