Repository navigation
refactor(api)!: build on climate-ref's query package with explicit filters - #147
Open
lewisjared wants to merge 27 commits into
Open
lewisjared wants to merge 27 commits into
lewisjared wants to merge 27 commits into
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 50 minutes. View limit details
Walkthrough
Priority: Change: Feature Merge Risk:
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 56.49% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 154 functions across 50 files. (6 skipped… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly identifies the main change: migrating the API to climate-ref's query package with explicit filters. The breaking-change marker is appropriate for the stated API changes. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 56.49% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 154 functions across 50 files. (6 skipped: 4 unsupported, 2 over the file limit.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches: 💡 2
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
lewisjared
added a commit
that referenced
this pull request
Oct 23, 2026
lewisjared
added a commit
that referenced
this pull request
Oct 23, 2026
lewisjared
force-pushed
the
refactor/query-package
branch
from
October 23, 2026 12:08
0db729c to
8c4668c
Compare
Migrates the fixture database for the new migrations. Drops the deprecated MetricValueFilter(promoted_only=True), which is now the default. Handles the reader's total_count now being optional.
Removes the app's own latest-execution subquery. climate-ref now picks the latest execution with the same created_at then id rule.
Uses ResourcesReader.summary for the overall and per-diagnostic resource usage. Passes include_retracted=True, so retracted executions still count as they did before. The roll-ups now keep the promoted version only.
Moves the per-era dataset matching onto climate-ref's InputDatasetFilter. Removes filter_utils, whose only user was the era filter. Adds tests for an unknown era, an unknown facet, a facet only one era carries and a comma list. The no-facet case trips an empty and_() deprecation upstream, which a test pins.
Moves the failure_reason, source_id and mip_era filters into the reader query. Skips the reader's count, because the route counts after its own sort. An unknown failure_reason or mip_era still returns an empty list.
Counts the groups per MIP era with one reader aggregate per era. Adds a test that the CMIP6 count matches the executions list for that era.
Takes the catalog and detail group counts from the reader instead of two hand-written queries each. The latest execution of a group is now the newest by created_at, not the highest id. This matches the executions statistics and the CLI. Adds tests for a backfilled failure and for an unknown era in the catalog.
Takes has_scalar_values and has_series_values from the reader for the list, catalog flags and detail. Hides retracted executions from both values endpoints, so the flags and the values follow one rule. The detail flags now count the promoted version only, like the list. Removes metric_value_type_is, which the reader replaces. The flag test now checks only listed diagnostics, so a local DIAGNOSTIC_EXCLUDE no longer breaks it.
Uses the reader for the dataset list, with slug_contains for name_contains. The list is now ordered by slug rather than by most recently updated, so paging is stable. Adds tests for the order, paging and a case-insensitive name match.
Replaces the legacy Query.get, which SQLAlchemy 2 warns about.
Adds eras_in_scope, which turns a mip_era query value into the eras it selects, in any case. The era filters and the model pages now share it instead of comparing upper-cased text. An empty mip_era on the model routes now means no filter, as it already did elsewhere. Pins the parameter on every route that takes it: an unknown era matches nothing and case is ignored.
Covers an execution mixing CMIP6 and CMIP7 inputs, one with no CMIP input and the mip_id fallback that labels them. These cases are absent from the fixture, so the tests build them in a copy of the database.
Uses InputDatasetFilter(or_no_model_input=True) for the "this era or no model input" rule. This replaces executions_in_mip_era and the hand-built OR in the ensemble comparison. Fixes an unknown mip_era on the diagnostic values and ensemble routes returning observation-only runs. It now matches nothing, like every other route. Moves the writable_client fixture to the shared conftest so the era tests can use it.
Removes the hand-written resource aggregates from the diagnostic catalog. The roll-up keeps the promoted version only and still counts retracted executions. The mip_era and source_id scope now applies to each execution rather than to its group.
climate-ref now reads an empty id list as matching nothing. So `?isolate_ids=,` would have hidden every value instead of isolating none. parse_id_list now returns None for a missing or blank list.
An unknown era on /models/{source_id} now gives the model with no runs rather than a 404.
This matches the empty lists every other route returns for an unknown era.
A missing model, or a known era the model has no runs in, still gives a 404,
because the model page uses it to say there are no runs in that era.
Test modules import test_settings and test_ref_config by name, so pytest ran them as tests. Each one returned a value and raised PytestReturnNotNoneWarning.
Removes execution_count and successful_execution_count from the diagnostic list and detail responses. Nothing in the frontend read them, and the group counts come from the reader.
Nothing in the frontend called it, and it read every value without the version or retraction rules /values applies.
…lters
Every reader call now names its version, retraction and latest-execution scope.
- `mip_era` takes only `CMIP6` or `CMIP7` and rejects anything else with a 422.
It keeps the groups that draw on that era, from their recorded source types.
- Diagnostics report `groups` (total, successful, failed, running, not_started, dirty)
in place of `execution_group_count` and `successful_execution_group_count`.
- `/executions/statistics` reports `groups` and `groups_by_mip_era` in place of the flat counts.
- `/diagnostics/{provider}/{diagnostic}/execution_groups` keeps only the promoted version
and leaves out groups whose latest execution is retracted.
- `/datasets/{id}/executions` lists the groups whose latest execution used the dataset.
- Removes `/diagnostics/{provider}/{diagnostic}/executions`, which the frontend did not call.
- An unknown `failure_reason` on `/executions/` is a 422 rather than an empty list.
lewisjared
force-pushed
the
refactor/query-package
branch
from
October 24, 2026 00:28
89a7eb6 to
475968d
Compare
lewisjared
added a commit
that referenced
this pull request
Oct 24, 2026
lewisjared
force-pushed
the
refactor/query-package
branch
from
October 24, 2026 11:05
475968d to
7988232
Compare
lewisjared
added a commit
that referenced
this pull request
Oct 24, 2026
lewisjared
marked this pull request as ready for review
October 24, 2026 11:12
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
-
-
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
687f1a8c-0022-469b-8771-aabede5bf7ae
-
backend/tests/test-data/tests.integration.test_cmip7_aft/test_solve_cmip7_aft/db/climate_ref.dbis excluded by!**/*.dbbackend/uv.lockis excluded by!**/*.lock
-
backend/pyproject.tomlbackend/src/ref_backend/api/routes/datasets.pybackend/src/ref_backend/api/routes/diagnostics.pybackend/src/ref_backend/api/routes/executions.pybackend/src/ref_backend/api/routes/models.pybackend/src/ref_backend/api/routes/results.pybackend/src/ref_backend/core/execution_groups.pybackend/src/ref_backend/core/filter_utils.pybackend/src/ref_backend/core/metric_values.pybackend/src/ref_backend/core/mip_eras.pybackend/src/ref_backend/core/model_ensemble.pybackend/src/ref_backend/core/model_runs.pybackend/src/ref_backend/core/reader_values.pybackend/src/ref_backend/core/resource_usage.pybackend/src/ref_backend/models/__init__.pybackend/src/ref_backend/models/common.pybackend/src/ref_backend/models/datasets.pybackend/src/ref_backend/models/diagnostics.pybackend/src/ref_backend/models/executions.pybackend/src/ref_backend/models/values.pybackend/src/ref_backend/testing.pybackend/tests/conftest.pybackend/tests/perf/test_endpoints.pybackend/tests/test_api/test_routes/test_datasets.pybackend/tests/test_api/test_routes/test_diagnostics.pybackend/tests/test_api/test_routes/test_executions.pybackend/tests/test_api/test_routes/test_mip_era_params.pybackend/tests/test_core/conftest.pybackend/tests/test_core/test_filter_utils.pybackend/tests/test_core/test_metric_values.pybackend/tests/test_core/test_mip_era_edges.pybackend/tests/test_core/test_model_runs.pybackend/tests/test_core/test_resource_usage.pychangelog/147.breaking.mdchangelog/147.feature.mdchangelog/147.improvement.mdfrontend/src/client/@tanstack/react-query.gen.tsfrontend/src/client/index.tsfrontend/src/client/sdk.gen.tsfrontend/src/client/types.gen.tsfrontend/src/components/dashboard/executionStats.test.tsxfrontend/src/components/dashboard/executionStats.tsxfrontend/src/components/datasets/diagnosticSummaryTable.tsxfrontend/src/components/diagnostics/diagnosticCard.tsxfrontend/src/components/execution/values/valuesPanel.test.tsxfrontend/src/components/execution/values/valuesPanel.tsxfrontend/src/components/explorer/chartValues.test.tsfrontend/src/components/explorer/chartValues.tsfrontend/src/components/explorer/content/filterControls.tsxfrontend/src/components/explorer/content/useExplorerMetricValues.tsfrontend/src/hooks/useDiagnosticMetricValues.tsfrontend/src/hooks/useExecutionMetricValues.tsfrontend/src/hooks/useMetricValues.test.tsfrontend/src/hooks/useMetricValues.tsfrontend/src/lib/failures.test.tsfrontend/src/lib/failures.tsfrontend/src/lib/resourceUsageCsv.test.tsfrontend/src/routes/_app/diagnostics.$providerSlug.$diagnosticSlug/route.tsx
-
- backend/tests/test_core/test_filter_utils.py
- backend/src/ref_backend/core/filter_utils.py
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
-
| columns = [cv_column(value, name) for name in dimensions] | ||
| # A retried group holds a value per attempt, so only the execution that decided it counts. | ||
| latest = latest_executions() | ||
| latest = select_latest_executions().subquery() |
There was a problem hiding this comment.
Exclude retracted executions from ensemble comparisons.
If a group’s latest execution is retracted, select_latest_executions() still selects it. _scalar_rows then includes its scalar values in the model average and ensemble statistics. Filter the selected execution with models.Execution.retracted.is_(False) after choosing the latest execution. Do not filter before selection, as that would admit an older run. (raw.githubusercontent.com)
| institutions still yields one row and is counted once. | ||
| """ | ||
| latest = latest_executions() | ||
| latest = select_latest_executions().subquery() |
There was a problem hiding this comment.
Exclude retracted latest executions from model runs.
If a group’s latest execution is retracted, this selection still passes it to model_run_rows and failed_runs. The model list and detail then count the retracted run, and detail can display it as a failure. Exclude retracted executions after selecting each group’s latest execution. Filtering before selection would incorrectly count an older run instead. (raw.githubusercontent.com)
An era now keeps the groups that draw on no model data in the counts, value flags, execution group lists, statistics and resource usage, as it already did for values.
The diagnostic's value flags only cover its promoted version, so they disabled the tabs of older groups.
lewisjared
force-pushed
the
refactor/query-package
branch
from
October 24, 2026 11:21
7988232 to
c59a4bb
Compare
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.
Moves the backend onto climate-ref's results readers and the
climate_ref.querypackage, so every filter states which version, retraction and latest-execution scope it reads.This builds against climate-ref 0.20.0 from PyPI, which ships the query package (Climate-REF/climate-ref#1011) and the reader fixes in Climate-REF/climate-ref#1018. The fixture database is migrated to match. With 0.20.0, scalar value pages with outlier detection take about 590 ms against 230 ms before at a tenth of production scale, because detection now runs in SQL.
Breaking API changes, with the frontend and generated client updated to match:
mip_eraaccepts onlyCMIP6orCMIP7, so a lowercase, unknown or empty value is now a 422. On the diagnostics, executions and values routes, an era now keeps the groups that draw on that era or on no model data (such as observation-only groups), from the source types climate-ref records per group, instead of the execution's datasets./models/and/models/{source_id}still pick the era from the latest execution's datasets./executions/statisticsreportgroups(total, successful, failed, running, not started, dirty) in place of the separate count fields.groups_by_mip_era, with agroupsobject per era, replacesexecution_groups_by_mip_era.execution_countandsuccessful_execution_count./values, the value flags and/executions/now leave out retracted executions. On main, diagnostic/valuesand the group counts included them./diagnostics/{provider}/{diagnostic}/execution_groupslists only the promoted version, and now fillstotal_count./executions/{group}/valuesnow returns the values of a group from a superseded diagnostic version, instead of nothing. The execution page now enables its value tabs from the group's own values, so these groups can open them.source_idfilters and/datasets/{id}/executionslook at each group's latest execution only, where before any execution in the group matched. The value flags are the exception:with_valuesstill matchessource_idagainst every execution. This only differs when a retry changes a group's model, which its selectors normally prevent./executions/returns a 422 for an unknownfailure_reason(before an empty list) and for a negativelimitoroffset./datasets/is now ordered by slug./diagnostics/facetsand/diagnostics/{provider}/{diagnostic}/executions, which the frontend never called.make perf-compare BASE=origin/feat/query-perf(#145, before it merged) at a tenth of production scale showed every endpoint the same or faster. For examplediagnostics.values.scalarwent from 3283 ms to 256 ms,executions.statisticsfrom 512 ms to 22 ms anddiagnostics.listfrom 221 ms to 44 ms. This can't run againstmainyet, because main's climate-ref 0.19 has no synthetic database generator. A head-only run after the rebase gives similar numbers (295 ms, 19 ms and 34 ms). The group counts read every row ofexecution_group, which the perf suite now allows on the three pages that count every group. That lookup finds each group's latest execution with a sorted subquery per group, which is worth improving upstream.Adds
/diagnostics/{provider}/{diagnostic}/values/facetsand/executions/{group}/values/facets, which return the facets/valuesreturns for the same filters. The values tables now fetch facets once per filter and pages withwith_facets=false, so a series page drops from 63 ms to 14 ms. The explorer filter controls used to fetch facets through/values?limit=1, which ran outlier detection over every value, and now use the facets endpoint (85 ms against 269 ms)./valuesstill returns facets by default.The commits in the middle of the branch were written against unreleased climate-ref builds, so only the tip type-checks and passes the tests against 0.20.0.
Summary by CodeRabbit
New Features
with_facets=false.Improvements
Breaking Changes