From 9ecda5b2666979eba81f1c0ec12ce24b49783afc Mon Sep 17 00:00:00 2001 From: Jared Lewis Date: Sun, 25 Oct 2026 10:15:29 +1100 Subject: [PATCH 1/2] perf(diagnostics): load providers and group ids with the listing The listing and catalog lazy loaded each diagnostic's execution groups, one query per diagnostic. --- .../src/ref_backend/api/routes/diagnostics.py | 22 ++++++++++++++----- .../test_api/test_routes/test_diagnostics.py | 22 ++++++++++++++++++- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/backend/src/ref_backend/api/routes/diagnostics.py b/backend/src/ref_backend/api/routes/diagnostics.py index 82d2c576..2d268524 100644 --- a/backend/src/ref_backend/api/routes/diagnostics.py +++ b/backend/src/ref_backend/api/routes/diagnostics.py @@ -1,6 +1,7 @@ from typing import Literal, NamedTuple from fastapi import APIRouter, HTTPException, Query, Request +from sqlalchemy.orm import joinedload, selectinload from starlette.responses import StreamingResponse from climate_ref import models @@ -59,8 +60,15 @@ async def _get_diagnostic( return diagnostic -def _listed_diagnostics(app_context: AppContextDep) -> list[models.Diagnostic]: - query = app_context.session.query(models.Diagnostic) +def _listed_diagnostics( + app_context: AppContextDep, *, with_group_ids: bool = False +) -> list[models.Diagnostic]: + query = app_context.session.query(models.Diagnostic).options(joinedload(models.Diagnostic.provider)) + if with_group_ids: + # One query for every diagnostic's group ids, rather than one per diagnostic. + query = query.options( + selectinload(models.Diagnostic.execution_groups).load_only(models.ExecutionGroup.id) + ) if app_context.settings.DIAGNOSTIC_PROVIDERS: query = query.join(models.Provider).filter( models.Provider.slug.in_(app_context.settings.DIAGNOSTIC_PROVIDERS) @@ -77,10 +85,12 @@ class _Scope(NamedTuple): filters: DiagnosticFilter -def _scoped_diagnostics(app_context: AppContextDep, mip_era: MipEra | None, source_id: str | None) -> _Scope: +def _scoped_diagnostics( + app_context: AppContextDep, mip_era: MipEra | None, source_id: str | None, *, with_group_ids: bool = False +) -> _Scope: """Get the listed diagnostics, counting only the groups that draw on `mip_era` and ran `source_id`""" return _Scope( - _listed_diagnostics(app_context), + _listed_diagnostics(app_context, with_group_ids=with_group_ids), DiagnosticFilter( include_superseded=False, include_retracted=False, @@ -146,7 +156,7 @@ async def _list( Counts cover the promoted version of each diagnostic and leave out retracted executions. """ - scope = _scoped_diagnostics(app_context, mip_era, source_id) + scope = _scoped_diagnostics(app_context, mip_era, source_id, with_group_ids=True) value_types = _value_types(app_context, scope) return Collection( data=[ @@ -172,7 +182,7 @@ async def catalog( Checking for metric values is most of the cost of the full listing. Fetch those from `/diagnostics/value-flags`. """ - scope = _scoped_diagnostics(app_context, mip_era, source_id) + scope = _scoped_diagnostics(app_context, mip_era, source_id, with_group_ids=True) return Collection(data=_catalog_entries(app_context, scope)) diff --git a/backend/tests/test_api/test_routes/test_diagnostics.py b/backend/tests/test_api/test_routes/test_diagnostics.py index 768de038..bb30d756 100644 --- a/backend/tests/test_api/test_routes/test_diagnostics.py +++ b/backend/tests/test_api/test_routes/test_diagnostics.py @@ -2,7 +2,7 @@ import pytest from fastapi.testclient import TestClient -from sqlalchemy import select +from sqlalchemy import event, select from climate_ref import models from climate_ref.database import Database @@ -992,3 +992,23 @@ def test_value_facets_of_an_unknown_diagnostic(client: TestClient) -> None: r = client.get("/api/v1/diagnostics/nope/nope/values/facets", params={"value_type": "scalar"}) assert r.status_code == 404 + + +@pytest.mark.parametrize("path", ["/diagnostics/", "/diagnostics/catalog", "/diagnostics/value-flags"]) +def test_listing_does_not_query_per_diagnostic(client: TestClient, settings, path: str): + engine = deps._get_database_dependency(settings, test_ref_config())._engine + statements: list[str] = [] + + def record(conn, cursor, statement, *_): + statements.append(statement) + + event.listen(engine, "before_cursor_execute", record) + try: + r = client.get(f"{settings.API_V1_STR}{path}") + finally: + event.remove(engine, "before_cursor_execute", record) + + assert r.status_code == 200 + assert len(r.json()["data"]) > 10 + # One query per diagnostic would scale with the catalogue, so stay under a small fixed number. + assert len(statements) <= 10, statements From 83a2e95260cfcb522253d3eab1caed62fb4a0107 Mon Sep 17 00:00:00 2001 From: Jared Lewis Date: Sun, 25 Oct 2026 10:15:41 +1100 Subject: [PATCH 2/2] docs: add changelog for #157 --- changelog/157.improvement.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/157.improvement.md diff --git a/changelog/157.improvement.md b/changelog/157.improvement.md new file mode 100644 index 00000000..57fff2a4 --- /dev/null +++ b/changelog/157.improvement.md @@ -0,0 +1 @@ +Sped up the diagnostics listing by loading every diagnostic's provider and execution groups in a fixed number of queries.