From 9bba72edabd7bf93fb2bcb00cf168912454f0665 Mon Sep 17 00:00:00 2001 From: Mac Lee Date: Sun, 5 Jul 2026 15:42:02 +0000 Subject: [PATCH] [ci]: tolerate legacy performance result artifacts (#1531) --- docs/contributing/performance_benchmarks.md | 9 +- .../tests/performance/compare_baseline.py | 85 +++++++++++-------- .../test_compare_baseline_policy.py | 43 ++++++++++ 3 files changed, 101 insertions(+), 36 deletions(-) diff --git a/docs/contributing/performance_benchmarks.md b/docs/contributing/performance_benchmarks.md index 237b19df9..b36a89a43 100644 --- a/docs/contributing/performance_benchmarks.md +++ b/docs/contributing/performance_benchmarks.md @@ -234,8 +234,10 @@ Recipe fingerprinting, hardware/software profile IDs, exact-identity comparison, and dashboard cohort grouping land with this change: v2 records compare only within their identity cohort, and a record that opens a NEW cohort is marked `baseline_status: "initialized_new_cohort"` (regression -gating starts once that cohort accumulates history). Legacy v1 configs keep -the `(model_id, gpu_type)` rolling-baseline comparison. Metric-specific +gating starts once that cohort accumulates history). Legacy v1 configs still +run and are normalized for reporting, but their records skip rolling-baseline +comparison entirely (`baseline_status: "skipped_missing_identity"`, never +baseline eligible); only static thresholds gate them. Metric-specific threshold policies and promoted baselines remain separate follow-ups. ### Raw record (`results/perf_*.json`) @@ -378,6 +380,9 @@ component timing fields, or v2 identity/profile fields. Records without for metric series that have no non-null values. Records missing both `run_source` and `baseline_eligible` are treated as legacy successful main/full-suite uploads and remain eligible for rolling baselines. +Current `perf_*.json` artifacts that lack the v2 comparison identity are +normalized for reporting but skip rolling-baseline comparison and are not marked +baseline eligible. New records compare only against the same `model_id`, `gpu_type`, `workload_id`, `variant_id`, `benchmark_version`, `recipe_fingerprint`, diff --git a/fastvideo/tests/performance/compare_baseline.py b/fastvideo/tests/performance/compare_baseline.py index 4e8165f72..a8c4bf7e2 100644 --- a/fastvideo/tests/performance/compare_baseline.py +++ b/fastvideo/tests/performance/compare_baseline.py @@ -185,11 +185,6 @@ def _comparison_identity_filters(record: dict[str, Any]) -> dict[str, str]: key for key in COMPARISON_IDENTITY_KEYS if key not in record or record[key] is None or (isinstance(record[key], str) and not record[key].strip()) ] - if len(missing) == len(COMPARISON_IDENTITY_KEYS): - # Legacy v1 record: no identity fields at all. Keep the pre-v2 - # (model_id, gpu_type) rolling-baseline comparison instead of - # crashing — v1 configs are documented as loadable. - return {} if missing: raise ValueError("Performance record missing required comparison identity fields: " + ", ".join(missing)) return { @@ -198,6 +193,18 @@ def _comparison_identity_filters(record: dict[str, Any]) -> dict[str, str]: } +def _comparison_identity_filters_or_none(record: dict[str, Any]) -> dict[str, str] | None: + try: + return _comparison_identity_filters(record) + except ValueError as exc: + print("Skipping rolling baseline comparison for " + f"{record.get('model_id', 'unknown')} on " + f"{record.get('gpu_type', 'unknown')}: {exc}. " + "The normalized artifact will still be written, but this record " + "will not be baseline eligible.") + return None + + def _format_identity_filters(filters: dict[str, str]) -> str: if not filters: return "" @@ -488,43 +495,53 @@ def main() -> int: for raw in current_results: record = _normalize_record(raw) metric_policies = resolve_metric_policies(record.get("regression_thresholds")) + identity_filters = _comparison_identity_filters_or_none(record) - baseline_records = load_records_for_model( - TRACKING_ROOT, - record["model_id"], - record["gpu_type"], - **_comparison_identity_filters(record), - last_n=5, - successful_only=True, - baseline_eligible_only=True, - ) - - if not baseline_records: - # A brand-new cohort has NO regression gating until history - # accumulates — make that loud and machine-readable instead of - # an indistinguishable pass, so a cohort shift (intended or - # accidental, e.g. an identity-field change) never silently - # blinds the comparison. - record["baseline_status"] = "initialized_new_cohort" - print("=" * 72) - print(f"WARNING: NO BASELINE — initializing a NEW cohort for " - f"{record['model_id']} on {record['gpu_type']}" - f"{_format_identity_filters(_comparison_identity_filters(record))}") - print("Regression gating is INACTIVE for this cohort until " - "baseline history accumulates. If this cohort shift is " - "unexpected, check the identity fields above.") - print("=" * 72) + baseline_records: list[dict[str, Any]] = [] + if identity_filters is None: + # Records without the full v2 identity block skip rolling-baseline + # comparison entirely: only the static thresholds gate them and + # they never become baseline eligible. + record["baseline_status"] = "skipped_missing_identity" failures: list[str] = [] - record["success"] = True else: - record["baseline_status"] = "compared" - failures = _check_regressions(record, baseline_records, metric_policies) + baseline_records = load_records_for_model( + TRACKING_ROOT, + record["model_id"], + record["gpu_type"], + **identity_filters, + last_n=5, + successful_only=True, + baseline_eligible_only=True, + ) + + if not baseline_records: + # A brand-new cohort has NO regression gating until history + # accumulates — make that loud and machine-readable instead of + # an indistinguishable pass, so a cohort shift (intended or + # accidental, e.g. an identity-field change) never silently + # blinds the comparison. + record["baseline_status"] = "initialized_new_cohort" + print("=" * 72) + print(f"WARNING: NO BASELINE — initializing a NEW cohort for " + f"{record['model_id']} on {record['gpu_type']}" + f"{_format_identity_filters(identity_filters)}") + print("Regression gating is INACTIVE for this cohort until " + "baseline history accumulates. If this cohort shift is " + "unexpected, check the identity fields above.") + print("=" * 72) + failures = [] + else: + record["baseline_status"] = "compared" + failures = _check_regressions(record, baseline_records, metric_policies) if static_threshold_failed: failures.append(f"{record['model_id']} fixed-threshold phase failed " f"(PERF_PYTEST_RC={os.environ.get('PERF_PYTEST_RC')})") record["success"] = not failures - record["baseline_eligible"] = _is_baseline_eligible(record["run_source"], record["success"]) + record["baseline_eligible"] = ( + identity_filters is not None and _is_baseline_eligible(record["run_source"], record["success"]) + ) all_failures.extend(failures) _write_normalized_artifact(record) diff --git a/fastvideo/tests/performance/test_compare_baseline_policy.py b/fastvideo/tests/performance/test_compare_baseline_policy.py index 41f6c9fc7..f8a7cd5b4 100644 --- a/fastvideo/tests/performance/test_compare_baseline_policy.py +++ b/fastvideo/tests/performance/test_compare_baseline_policy.py @@ -1,5 +1,7 @@ # SPDX-License-Identifier: Apache-2.0 +import json + import pytest from fastvideo.tests.performance import compare_baseline @@ -208,6 +210,47 @@ def test_v1_normalized_record_has_no_result_schema_version(monkeypatch): assert "result_schema_version" not in record +def test_main_writes_v1_current_artifact_without_comparison_identity(monkeypatch, tmp_path, capsys): + results_dir = tmp_path / "results" + reports_dir = tmp_path / "reports" + tracking_root = tmp_path / "tracking" + results_dir.mkdir() + (results_dir / "perf_legacy.json").write_text(json.dumps(_raw_result()), encoding="utf-8") + uploaded_records = [] + + monkeypatch.setenv("PERF_RUN_SOURCE", "scheduled_main") + monkeypatch.delenv("PERF_PYTEST_RC", raising=False) + monkeypatch.setattr(compare_baseline, "RESULTS_DIR", str(results_dir)) + monkeypatch.setattr(compare_baseline, "PERF_REPORTS_DIR", str(reports_dir)) + monkeypatch.setattr(compare_baseline, "TRACKING_ROOT", str(tracking_root)) + monkeypatch.setattr(compare_baseline, "UPLOAD_POLICY", "always") + monkeypatch.setattr(compare_baseline, "sync_from_hf", lambda local_dir, strict=False: local_dir) + + def fail_load_records_for_model(*_args, **_kwargs): + raise AssertionError("legacy current records should skip v2 baseline lookup") + + def fake_upload_record(_path, record, *, strict=False): + uploaded_records.append(record.copy()) + + monkeypatch.setattr(compare_baseline, "load_records_for_model", fail_load_records_for_model) + monkeypatch.setattr(compare_baseline, "upload_record", fake_upload_record) + + assert compare_baseline.main() == 0 + + output = capsys.readouterr().out + normalized_files = list((reports_dir / "results").glob("normalized_perf_*.json")) + assert "Skipping rolling baseline comparison" in output + assert "workload_id" in output + assert len(normalized_files) == 1 + + normalized = json.loads(normalized_files[0].read_text(encoding="utf-8")) + assert "result_schema_version" not in normalized + assert normalized["success"] is True + assert normalized["baseline_eligible"] is False + assert len(uploaded_records) == 1 + assert uploaded_records[0]["baseline_eligible"] is False + + def test_normalized_record_reads_identity_labels_from_recipe(monkeypatch): monkeypatch.setenv("PERF_RUN_SOURCE", "pr") raw = _raw_result()