cleanup: refactor after fallow audit

.vscode/settings.json

@@ -4,6 +4,7 @@
     "makefile.configureOnOpen": false,
     "biome.configurationPath": "frontend/biome.json",
     "biome.lsp.bin": "frontend/node_modules/.bin/biome",
+    "fallow.configPath": "frontend/.fallowrc.jsonc",
     "editor.codeActionsOnSave": {
         "source.fixAll.biome": "explicit",
         "source.organizeImports.biome": "explicit"

Makefile

@@ -50,6 +50,10 @@ setup: ## Setup the backend and frontend
 generate-metadata: ## Generate diagnostic metadata YAML from provider registry
 	$(MAKE) -C backend generate-metadata
 
+.PHONY: fallow-audit
+fallow-audit: ## run tests with coverage, then audit frontend changes against main with fallow
+	cd frontend && npm run test:coverage && npm run fallow:audit
+
 .PHONY: changelog-draft
 changelog-draft: ## compile a draft of the next changelog
 	cd backend && uv run towncrier build --draft --config ../towncrier.toml

backend/ruff.toml

@@ -13,6 +13,8 @@ select = [
     "RUF",
     "UP",
     "S",
+    "C90",
+    "C4",
 ]
 unfixable = [
     "PD002",
@@ -26,8 +28,13 @@ ignore = [
     "D200",
     "D400",
     "UP007",
+    "C408", # dict(key=value) calls read as well as literals
 ]
 
+[lint.mccabe]
+# Flags functions with more independent paths than a reader can comfortably follow
+max-complexity = 10
+
 [lint.per-file-ignores]
 "test*.py" = [
     "D",  # Documentation not needed in tests

backend/src/ref_backend/api/routes/diagnostics.py

@@ -1,4 +1,4 @@
-from typing import Literal
+from typing import Literal, NamedTuple
 
 from fastapi import APIRouter, HTTPException, Query, Request
 from sqlalchemy import ColumnElement, Integer, func, select
@@ -92,6 +92,18 @@ def _group_scope(
     return group_scope
 
 
+def _scoped_diagnostics(
+    app_context: AppContextDep, mip_era: str | None, source_id: str | None
+) -> tuple[list[models.Diagnostic], list[ColumnElement[bool]]]:
+    """
+    Get the listed diagnostics, with the scope that counts their groups for `mip_era` and `source_id`
+
+    Without any listed diagnostic the scope matches no group, so the counts come out empty.
+    """
+    diagnostics = _listed_diagnostics(app_context)
+    return diagnostics, _group_scope([d.id for d in diagnostics], mip_era, source_id)
+
+
 def _diagnostics_with_values(
     app_context: AppContextDep, value_class: type[models.MetricValue], group_scope: list[ColumnElement[bool]]
 ) -> set[int]:
@@ -105,6 +117,20 @@ def _diagnostics_with_values(
     return {row[0] for row in rows}
 
 
+class _ValueTypes(NamedTuple):
+    """The ids of the diagnostics with scalar values and of those with series values"""
+
+    scalar: set[int]
+    series: set[int]
+
+
+def _value_types(app_context: AppContextDep, group_scope: list[ColumnElement[bool]]) -> _ValueTypes:
+    return _ValueTypes(
+        scalar=_diagnostics_with_values(app_context, models.ScalarMetricValue, group_scope),
+        series=_diagnostics_with_values(app_context, models.SeriesMetricValue, group_scope),
+    )
+
+
 def _catalog_entries(
     app_context: AppContextDep, diagnostics: list[models.Diagnostic], group_scope: list[ColumnElement[bool]]
 ) -> list[DiagnosticCatalogEntry]:
@@ -188,19 +214,14 @@ async def _list(
     Pass `mip_era` to count only the execution groups that ran against that era,
     and `source_id` to count only those that ran against that model.
     """
-    diagnostics = _listed_diagnostics(app_context)
-    if not diagnostics:
-        return Collection(data=[])
-
-    group_scope = _group_scope([d.id for d in diagnostics], mip_era, source_id)
-    scalar_ids = _diagnostics_with_values(app_context, models.ScalarMetricValue, group_scope)
-    series_ids = _diagnostics_with_values(app_context, models.SeriesMetricValue, group_scope)
+    diagnostics, group_scope = _scoped_diagnostics(app_context, mip_era, source_id)
+    value_types = _value_types(app_context, group_scope)
     return Collection(
         data=[
             DiagnosticSummary.from_catalog_entry(
                 entry,
-                has_scalar_values=entry.id in scalar_ids,
-                has_series_values=entry.id in series_ids,
+                has_scalar_values=entry.id in value_types.scalar,
+                has_series_values=entry.id in value_types.series,
             )
             for entry in _catalog_entries(app_context, diagnostics, group_scope)
         ]
@@ -219,11 +240,7 @@ async def catalog(
 
     Pass `mip_era` and `source_id` to count only the execution groups that ran against that era or model.
     """
-    diagnostics = _listed_diagnostics(app_context)
-    if not diagnostics:
-        return Collection(data=[])
-
-    group_scope = _group_scope([d.id for d in diagnostics], mip_era, source_id)
+    diagnostics, group_scope = _scoped_diagnostics(app_context, mip_era, source_id)
     return Collection(data=_catalog_entries(app_context, diagnostics, group_scope))
 
 
@@ -236,20 +253,15 @@ async def value_flags(
 
     These are the flags `/diagnostics/catalog` leaves out.
     """
-    diagnostics = _listed_diagnostics(app_context)
-    if not diagnostics:
-        return Collection(data=[])
-
-    group_scope = _group_scope([d.id for d in diagnostics], mip_era, source_id)
-    scalar_ids = _diagnostics_with_values(app_context, models.ScalarMetricValue, group_scope)
-    series_ids = _diagnostics_with_values(app_context, models.SeriesMetricValue, group_scope)
+    diagnostics, group_scope = _scoped_diagnostics(app_context, mip_era, source_id)
+    value_types = _value_types(app_context, group_scope)
     return Collection(
         data=[
             DiagnosticValueFlags(
                 id=d.id,
-                has_metric_values=d.id in scalar_ids or d.id in series_ids,
-                has_scalar_values=d.id in scalar_ids,
-                has_series_values=d.id in series_ids,
+                has_metric_values=d.id in value_types.scalar or d.id in value_types.series,
+                has_scalar_values=d.id in value_types.scalar,
+                has_series_values=d.id in value_types.series,
             )
             for d in diagnostics
         ]

backend/src/ref_backend/core/filter_utils.py

@@ -14,21 +14,16 @@ def _normalize_list_from_value(value: Any) -> list[str] | None:
 
     Returns None if the input should be treated as a single value (no list semantics).
     """
-    if value is None:
-        return None
-
-    # If already a list (e.g. from parsed JSON), coerce & clean
     if isinstance(value, list):
-        vals = [str(v).strip() for v in value if v is not None and str(v).strip()]
-        return vals if vals else None
-
-    # If a comma-separated string, split and clean
-    if isinstance(value, str) and "," in value:
-        vals = [v.strip() for v in value.split(",") if v.strip()]
-        return vals if vals else None
+        # e.g. from parsed JSON
+        items = [str(v) for v in value if v is not None]
+    elif isinstance(value, str) and "," in value:
+        items = value.split(",")
+    else:
+        return None
 
-    # Otherwise, treat as a single scalar value
-    return None
+    vals = [item.strip() for item in items if item.strip()]
+    return vals or None
 
 
 def build_filter_clause(column: Any, value: Any) -> BinaryExpression[Any]:
@@ -40,27 +35,15 @@ def build_filter_clause(column: Any, value: Any) -> BinaryExpression[Any]:
       an `IN` clause is returned.
     - If `value` contains exactly one normalized item, a simple equality
       (`column == item`) is returned.
-    - Otherwise, fall back to equality with the original `value`.
+    - Otherwise, fall back to equality with the original `value`,
+      including for an empty list.
 
     This function is intended to be used in route handlers where query values
     may be single values, comma-separated strings, or parsed JSON lists.
     """
-    # Handle lists explicitly
-    if isinstance(value, list):
-        vals = _normalize_list_from_value(value)
-        if vals is None:
-            # Empty list -> fallback equality (keeps prior behavior)
-            return column == value
-        if len(vals) == 1:
-            return column == vals[0]
-        return column.in_(vals)
-
-    # Handle comma-separated strings or other scalars
     vals = _normalize_list_from_value(value)
-    if vals:
-        if len(vals) == 1:
-            return column == vals[0]
-        return column.in_(vals)
-
-    # Default: equality
-    return column == value
+    if vals is None:
+        return column == value
+    if len(vals) == 1:
+        return column == vals[0]
+    return column.in_(vals)

backend/src/ref_backend/core/reader_values.py

@@ -2,8 +2,8 @@
 
 import csv
 import io
-from collections.abc import Generator, Mapping
-from typing import TYPE_CHECKING, Literal
+from collections.abc import Generator, Iterable, Mapping, Sequence
+from typing import TYPE_CHECKING, Any, Literal
 
 from fastapi import HTTPException
 from starlette.responses import StreamingResponse
@@ -31,6 +31,46 @@ def parse_dimension_filters(query_params: Mapping[str, str]) -> dict[str, str]:
     return {key: value for key, value in query_params.items() if key in cv_dimensions}
 
 
+def _csv_response(
+    rows: Iterable[Sequence[Any]], filename: str, headers: Mapping[str, str] | None = None
+) -> StreamingResponse:
+    """Stream `rows` as a CSV download, writing each row as it is produced."""
+
+    def generate_csv() -> Generator[str]:
+        output = io.StringIO()
+        writer = csv.writer(output)
+        for row in rows:
+            writer.writerow(row)
+            yield output.getvalue()
+            output.seek(0)
+            output.truncate()
+
+    return StreamingResponse(
+        generate_csv(),
+        media_type="text/csv",
+        headers={"Content-Disposition": f"attachment; filename={filename}", **(headers or {})},
+    )
+
+
+def _scalar_csv_rows(collection: ScalarValueCollection, detection_ran: bool) -> Generator[list[Any]]:
+    items = collection.items
+    # Dimension columns come from the values, so an empty collection only has the fixed ones
+    dimensions = sorted(items[0].dimensions.keys()) if items else []
+    header = [*dimensions, "value", "type"]
+    if detection_ran:
+        header.extend(["is_outlier", "verification_status"])
+    yield header
+
+    for item in items:
+        row = [item.dimensions.get(d) for d in dimensions] + [
+            sanitize_float_value(item.value),
+            "scalar",
+        ]
+        if detection_ran:
+            row.extend([item.is_outlier, item.verification_status])
+        yield row
+
+
 def generate_csv_response_scalar(
     collection: ScalarValueCollection,
     detection_ran: bool,
@@ -41,45 +81,35 @@ def generate_csv_response_scalar(
 
     Preserves the historical column layout: sorted dimension columns, then ``value`` and
     ``type``, and (when detection ran) ``is_outlier`` and ``verification_status``.
+    An empty collection gets a header of the columns that do not depend on the values.
     """
+    headers = {}
+    if detection_ran:
+        headers["X-REF-Had-Outliers"] = "true" if collection.had_outliers else "false"
+        headers["X-REF-Outlier-Count"] = str(collection.outlier_count)
+    return _csv_response(_scalar_csv_rows(collection, detection_ran), filename, headers)
 
-    def generate_csv() -> Generator[str]:
-        output = io.StringIO()
-        writer = csv.writer(output)
-
-        items = collection.items
-        if not items:
-            yield ""
-            return
 
-        dimensions = sorted(items[0].dimensions.keys())
-        header = [*dimensions, "value", "type"]
-        if detection_ran:
-            header.extend(["is_outlier", "verification_status"])
-        writer.writerow(header)
+_SERIES_COLUMNS = ["value", "index", "index_name", "type"]
 
-        for item in items:
-            row = [item.dimensions.get(d) for d in dimensions] + [
-                sanitize_float_value(item.value),
-                "scalar",
-            ]
-            if detection_ran:
-                row.extend([item.is_outlier, item.verification_status])
-            writer.writerow(row)
 
-        output.seek(0)
-        yield output.read()
+def _series_csv_rows(collection: SeriesValueCollection) -> Generator[list[Any]]:
+    if not collection.items:
+        yield list(_SERIES_COLUMNS)
+        return
 
-    headers = {"Content-Disposition": f"attachment; filename={filename}"}
-    if detection_ran:
-        headers["X-REF-Had-Outliers"] = "true" if collection.had_outliers else "false"
-        headers["X-REF-Outlier-Count"] = str(collection.outlier_count)
+    for sv in collection.items:
+        dimensions = sorted(sv.dimensions.keys())
+        yield [*dimensions, *_SERIES_COLUMNS]
 
-    return StreamingResponse(
-        generate_csv(),
-        media_type="text/csv",
-        headers=headers,
-    )
+        for i, value in enumerate(sv.values):
+            index_value = sv.index[i] if sv.index and i < len(sv.index) else i
+            yield [sv.dimensions.get(d) for d in dimensions] + [
+                sanitize_float_value(value),
+                index_value,
+                sv.index_name or "index",
+                "series",
+            ]
 
 
 def generate_csv_response_series(
@@ -90,41 +120,10 @@ def generate_csv_response_series(
     Generate a CSV streaming response from a reader series collection.
 
     Preserves the historical flattened layout: one header/data block per series, with a
-    row per index point.
+    row per index point. An empty collection gets a header of the columns that do not
+    depend on the values.
     """
-
-    def generate_csv() -> Generator[str]:
-        output = io.StringIO()
-        writer = csv.writer(output)
-
-        items = collection.items
-        if not items:
-            yield ""
-            return
-
-        for sv in items:
-            dimensions = sorted(sv.dimensions.keys())
-            header = [*dimensions, "value", "index", "index_name", "type"]
-            writer.writerow(header)
-
-            for i, value in enumerate(sv.values):
-                index_value = sv.index[i] if sv.index and i < len(sv.index) else i
-                row = [sv.dimensions.get(d) for d in dimensions] + [
-                    sanitize_float_value(value),
-                    index_value,
-                    sv.index_name or "index",
-                    "series",
-                ]
-                writer.writerow(row)
-
-        output.seek(0)
-        yield output.read()
-
-    return StreamingResponse(
-        generate_csv(),
-        media_type="text/csv",
-        headers={"Content-Disposition": f"attachment; filename={filename}"},
-    )
+    return _csv_response(_series_csv_rows(collection), filename)
 
 
 def annotate_mip_eras(app_context: "AppContext", collection: MetricValueCollection) -> MetricValueCollection: