Couple Diagnostic.version into the regression CI coupling gate#770
Merged
Conversation
The coupling gate previously read only test_case_version, so an execution-affecting change (recipe, ESMValTool commit, update_recipe) could slip through as a green replay against a stale committed bundle because replay never re-runs execution. Couple the per-diagnostic, author-declared Diagnostic.version into the gate as an authoritative staleness signal: - Manifest schema 1 -> 2 with a required diagnostic_version field. - decide_coupling FAILs a case when the in-code Diagnostic.version exceeds the manifest's recorded value (stale baseline) or falls below it (illegal downgrade); a cross-PR decrease FAILs only when the committed bundle is unchanged, so a legitimate revert-plus-remint has a green path. - mint (incl. --from-replay) stamps diag.version; run/build preserve the recorded value, so a bump without a re-mint FAILs the gate. - New 'ref test-cases migrate-manifests' verb backfills diagnostic_version across all existing baselines (idempotent, iterator-driven). test_case_version remains a separate, independently-monotonic per-case axis.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds Changesdiagnostic_version schema, gate, and migration
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
…on-gate * origin/main: feat(example): extract model/reference/bias time series as CMEC series refactor(example): rename diagnostic to global-sst-bias and drop version guard test: close NetCDF datasets in surface-temperature end-to-end tests feat(example): use ocean SST (tos) and add comparison figures test(example): mint regression baselines for surface-temperature-bias fix: address review feedback on surface temperature diagnostic test: update solver regression snapshots for new example diagnostic feat(example): add model-vs-obs surface temperature diagnostic and 5-minute quickstart
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f6cf6b14-e599-4f73-bd13-4d925e564837
📒 Files selected for processing (98)
changelog/770.feature.mddocs/background/regression-baselines.mdpackages/climate-ref-core/src/climate_ref_core/regression/gate.pypackages/climate-ref-core/src/climate_ref_core/regression/manifest.pypackages/climate-ref-core/tests/unit/regression/test_gate.pypackages/climate-ref-core/tests/unit/regression/test_manifest.pypackages/climate-ref-esmvaltool/tests/test-data/climate-at-global-warming-levels/cmip6/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/climate-drivers-for-fire/cmip6/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/climate-drivers-for-fire/cmip7/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/cloud-radiative-effects/cmip6/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/cloud-scatterplots-cli-ta/cmip6/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/cloud-scatterplots-clivi-lwcre/cmip6/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/cloud-scatterplots-clivi-lwcre/cmip7/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/cloud-scatterplots-clt-swcre/cmip6/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/cloud-scatterplots-clt-swcre/cmip7/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/cloud-scatterplots-clwvi-pr/cmip6/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/cloud-scatterplots-clwvi-pr/cmip7/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/enso-basic-climatology/cmip6/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/enso-characteristics/cmip6/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/equilibrium-climate-sensitivity/cmip6/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/global-mean-timeseries/cmip6/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/global-mean-timeseries/cmip7/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/ozone-annual-cycle/cmip6/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/ozone-annual-cycle/cmip7/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/ozone-lat-time/cmip6/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/ozone-lat-time/cmip7/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/ozone-nh-mar/cmip6/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/ozone-nh-mar/cmip7/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/ozone-sh-oct/cmip6/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/ozone-sh-oct/cmip7/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/ozone-zonal/cmip6/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/regional-historical-annual-cycle/cmip6/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/regional-historical-annual-cycle/cmip7/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/regional-historical-timeseries/cmip6/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/regional-historical-timeseries/cmip7/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/regional-historical-trend/cmip6/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/regional-historical-trend/cmip7/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/sea-ice-area-basic/cmip6/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/sea-ice-area-basic/cmip7/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/sea-ice-sensitivity/cmip6/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/sea-ice-sensitivity/cmip7/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/transient-climate-response-emissions/cmip6/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/transient-climate-response-emissions/cmip7/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/transient-climate-response/cmip6/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/transient-climate-response/cmip7/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/zero-emission-commitment/cmip6-1pctCO2-parent/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/zero-emission-commitment/cmip6-esm-1pctCO2-parent/manifest.jsonpackages/climate-ref-example/tests/test-data/global-mean-timeseries/cmip7/manifest.jsonpackages/climate-ref-example/tests/test-data/global-mean-timeseries/default/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/burntfractionall-gfed/cmip6/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/burntfractionall-gfed/cmip7/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/csoil-hwsd2/cmip6/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/csoil-hwsd2/cmip7/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/emp-gleamgpcp2.3/cmip6/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/gpp-fluxnet2015/cmip6/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/gpp-wecann/cmip6/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/lai-avh15c1/cmip6/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/lai-avh15c1/cmip7/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/mrro-lora/cmip6/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/mrro-lora/cmip7/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/mrsos-wangmao/cmip6/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/nbp-hoffman/cmip6/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/nbp-hoffman/cmip7/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/snc-esacci/cmip6/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/snc-esacci/cmip7/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/so-woa2023-surface/cmip6/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/so-woa2023-surface/cmip7/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/thetao-woa2023-surface/cmip6/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/thetao-woa2023-surface/cmip7/manifest.jsonpackages/climate-ref-pmp/tests/test-data/annual-cycle/cmip6-pr/manifest.jsonpackages/climate-ref-pmp/tests/test-data/annual-cycle/cmip6-ts/manifest.jsonpackages/climate-ref-pmp/tests/test-data/annual-cycle/cmip7-ts/manifest.jsonpackages/climate-ref-pmp/tests/test-data/enso_proc/cmip6/manifest.jsonpackages/climate-ref-pmp/tests/test-data/enso_proc/cmip7/manifest.jsonpackages/climate-ref-pmp/tests/test-data/enso_tel/cmip6/manifest.jsonpackages/climate-ref-pmp/tests/test-data/enso_tel/cmip7/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-nam/cmip6/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-nam/cmip7/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-nao/cmip6/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-nao/cmip7/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npgo/cmip6/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npgo/cmip7/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npo/cmip6/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-npo/cmip7/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pdo/cmip6/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pdo/cmip7/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pna/cmip6/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-pna/cmip7/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-sam/cmip6/manifest.jsonpackages/climate-ref-pmp/tests/test-data/extratropical-modes-of-variability-sam/cmip7/manifest.jsonpackages/climate-ref/src/climate_ref/cli/test_cases/__init__.pypackages/climate-ref/src/climate_ref/cli/test_cases/_common.pypackages/climate-ref/src/climate_ref/cli/test_cases/baselines.pypackages/climate-ref/src/climate_ref/cli/test_cases/ci_gate.pypackages/climate-ref/src/climate_ref/cli/test_cases/migrate.pypackages/climate-ref/src/climate_ref/cli/test_cases/run.pypackages/climate-ref/tests/integration/test_native_roundtrip.pypackages/climate-ref/tests/unit/cli/test_test_cases.py
Only backfill diagnostic_version when absent so re-running after an authorised version bump cannot re-stamp a stale bundle as current and subvert the gate's staleness check. Add a regression test for the preserve path and a ci-gate test that drives the real schema-1 base-manifest seeding fallback.
Tighten the new decide_coupling comments and the regression-baselines prose, reflow to semantic line breaks, and drop em dashes from the added comments, docstrings, and FAIL messages. No behaviour change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Couples the per-diagnostic, author-declared
Diagnostic.versioninto the regression CI coupling gate as an authoritative "results changed" staleness signal, alongside the existing per-casetest_case_version.Why. A
replayre-runs only the extraction code (build_execution_result) against the cached native baseline, so it is structurally blind to execution-affecting changes — the recipe, the pinned ESMValTool commit,update_recipe, or data requirements. Path-basedextraction_changeddetection cannot tell an execution change from an extraction change within a provider package, so such a change currently routes to a green replay that validates nothing about the new computation.Diagnostic.versionis the author's existing, explicit declaration that a diagnostic's results changed (it already drives live-solver recomputation), so the gate now treats it as authoritative.What changed.
1->2with a new requireddiagnostic_versionfield, recorded at mint time.decide_couplingFAILs a case when the in-codeDiagnostic.versionexceeds the manifest's recorded value (stale baseline — re-mint required) or falls below it (illegal downgrade). A cross-PRdiagnostic_versiondecrease FAILs only when the committed bundle is unchanged, so a legitimate revert that lowers the version and re-mints the baseline keeps a green path.mint(including--from-replay) stampsdiag.version;run/buildpreserve the recorded value. A version bump without a re-mint therefore FAILs the gate, forcing the baseline to be regenerated.ref test-cases migrate-manifestsverb backfillsdiagnostic_versionacross all existing baselines (idempotent, iterates the same_iter_test_casesthe gate uses). Applied here to all 84 committed manifests (23 carry version2: every ilamb case and the pmpannual_cyclecases).extraction_changedreplay backstop is retained — it and the author-declared signal are complementary.test_case_versionremains a separate, independently-monotonic per-case axis; the two are not collapsed here.This change was produced through a Planner/Architect/Critic consensus review; the design rationale (including the downgrade carve-out and the rejected
--allow-downgradealternative) is captured in the commit body and the updateddocs/background/regression-baselines.md.Verification.
make mypyclean (141 files); 87 gate/manifest unit tests, 149 CLI/integration tests pass;ref test-cases ci-gatereports no spurious failures (native-bearing cases route to replay as expected for a core-code change).Checklist
Please confirm that this pull request has done the following:
changelog/Summary by CodeRabbit
diagnostic_versionsupport to regression test-case manifests, with manifest schema v2.ref test-cases migrate-manifestscommand to backfill and update existing manifests.