From ead6ef8ce16df4416b0d5e9d9b77943b46ad1e1e Mon Sep 17 00:00:00 2001 From: zvx Date: Tue, 19 May 2026 17:34:35 +0000 Subject: [PATCH 1/2] feat(2-G.5): preview_for_settings framework hook + NWIS opt-in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an optional async hook on SourceAdapter so any adapter can surface a settings-driven preview on its /adapters/ edit page. The framework renders the result generically as a table — no adapter-name branches in GUI templates or route code. Framework changes: - src/central/adapter.py: new async preview_for_settings(self, settings) on the base class, default returns None. Adapters opt in by overriding; non-overriding adapters render unchanged. - src/central/gui/routes.py: GET /adapters/{name} instantiates the adapter with a no-op _PreviewConfigStore stub and a /dev/null cursor path (GUI has no live ConfigStore), constructs settings_obj via the schema, and calls preview_for_settings inside a try/except. Result lands in template context as preview_rows / preview_error. - src/central/gui/templates/_adapter_preview.html: new partial. Generic table with columns derived from the first dict's keys; error banner mirrors the existing last_error article style. - src/central/gui/templates/adapters_edit.html: one-line include between the Region fieldset and Save/Cancel. NWIS opt-in: - New NWIS_MONITORING_LOCATIONS_URL constant and _PREVIEW_LIMIT cap of 50. - preview_for_settings returns None when region is None, otherwise one-shot fetches monitoring-locations within the bbox via a fresh aiohttp session. Must work even when adapter is not started -- the GUI process never calls startup(). Returns list[dict] with the contract column order: site_id, name, site_type, state. Errors propagate so the framework can render the operator-visible banner. - HTTP call factored into _fetch_preview_text so tests mock cleanly. Tests (7 new): - tests/test_preview_hook.py: default returns None; partial renders list with correct headers/rows/count; partial renders error banner; partial renders empty when both context values are None. - tests/test_nwis.py adds TestNWISPreview: returns None without region, returns rows with correct column order, propagates HTTP errors. Verification: - 457/457 full suite green (was 450; +7 new tests). - Live /adapters/nwis preview returns 50 rows with the contract keys against the current production Iowa bbox. - /adapters/eonet preview_for_settings returns None via base default -- proves framework is duck-typed, no NWIS-specific code in framework. --- src/central/adapter.py | 9 ++ src/central/adapters/nwis.py | 58 ++++++++++ src/central/gui/routes.py | 39 ++++++- .../gui/templates/_adapter_preview.html | 23 ++++ src/central/gui/templates/adapters_edit.html | 2 + tests/test_nwis.py | 81 +++++++++++++ tests/test_preview_hook.py | 108 ++++++++++++++++++ 7 files changed, 319 insertions(+), 1 deletion(-) create mode 100644 src/central/gui/templates/_adapter_preview.html create mode 100644 tests/test_preview_hook.py diff --git a/src/central/adapter.py b/src/central/adapter.py index 0322d3e..2a408b0 100644 --- a/src/central/adapter.py +++ b/src/central/adapter.py @@ -78,3 +78,12 @@ class SourceAdapter(ABC): async def shutdown(self) -> None: """Optional lifecycle hook called on graceful shutdown.""" pass + + async def preview_for_settings(self, settings: BaseModel) -> list[dict] | None: + """Optional. Override to surface a settings-driven preview on the edit page. + + Return list[dict] (framework renders as a generic table; columns come from + the first dict's keys, in insertion order). Return None to skip preview. + Raise to surface an error banner — framework catches at the route boundary. + """ + return None diff --git a/src/central/adapters/nwis.py b/src/central/adapters/nwis.py index 772262f..e2a922d 100644 --- a/src/central/adapters/nwis.py +++ b/src/central/adapters/nwis.py @@ -28,6 +28,12 @@ logger = logging.getLogger(__name__) NWIS_LATEST_CONTINUOUS_URL = ( "https://api.waterdata.usgs.gov/ogcapi/v0/collections/latest-continuous/items" ) +NWIS_MONITORING_LOCATIONS_URL = ( + "https://api.waterdata.usgs.gov/ogcapi/v0/collections/monitoring-locations/items" +) +# Per-render cap for the settings-driven preview (PR G.5). Keep small so the +# /adapters/ edit page renders quickly. +_PREVIEW_LIMIT = 50 # Single source of truth for the parameter-code default. Operators tune via # NWISSettings.parameter_codes; do NOT duplicate this list elsewhere @@ -375,3 +381,55 @@ class NWISAdapter(SourceAdapter): geo=Geo(centroid=centroid), data=data, ) + + async def _fetch_preview_text(self, url: str) -> str: + """One-shot GET for the preview render. + + Uses a fresh aiohttp session — preview must work even when the adapter + isn't started (the GUI process never calls startup()). Factored out so + tests can mock the HTTP call without touching aiohttp internals. + """ + async with aiohttp.ClientSession( + timeout=aiohttp.ClientTimeout(total=15), + ) as session: + async with session.get( + url, headers={"User-Agent": "Central/0.4"} + ) as resp: + resp.raise_for_status() + return await resp.text() + + async def preview_for_settings(self, settings: NWISSettings) -> list[dict] | None: + """Surface monitoring-locations inside the configured bbox. + + Returns up to _PREVIEW_LIMIT rows from the monitoring-locations + collection. Returns None if region is unset (no useful preview). + Raises on HTTP / JSON / shape failure — framework catches at the route. + """ + if settings.region is None: + return None + + params = { + "bbox": ( + f"{settings.region.west},{settings.region.south}," + f"{settings.region.east},{settings.region.north}" + ), + "limit": str(_PREVIEW_LIMIT), + } + url = f"{NWIS_MONITORING_LOCATIONS_URL}?{urlencode(params)}" + + text = await self._fetch_preview_text(url) + page = json.loads(text) + features = page.get("features") or [] + + rows: list[dict] = [] + for feat in features: + props = feat.get("properties") or {} + rows.append( + { + "site_id": feat.get("id"), + "name": props.get("monitoring_location_name"), + "site_type": props.get("site_type_code"), + "state": props.get("state_name"), + } + ) + return rows diff --git a/src/central/gui/routes.py b/src/central/gui/routes.py index c5baf44..a171c47 100644 --- a/src/central/gui/routes.py +++ b/src/central/gui/routes.py @@ -46,6 +46,9 @@ from central.gui.audit import ( ) from functools import cache +from pathlib import Path + +from central.config_models import AdapterConfig from central.gui.db import get_pool from central.gui.form_descriptors import describe_fields, FieldDescriptor from central.adapter_discovery import discover_adapters @@ -55,13 +58,23 @@ from pydantic import ValidationError @cache def _adapter_classes() -> dict: """Cached adapter class discovery. - + GUI is a separate process from supervisor; walks pkgutil itself. Python's import cache makes subsequent calls free. """ return discover_adapters() +class _PreviewConfigStore: + """No-op stand-in passed to adapter __init__ when calling preview_for_settings. + + preview_for_settings implementations must create their own one-shot HTTP + session and must not depend on config_store / cursor_db state — the GUI + process has no live ConfigStore (the supervisor owns the real one).""" + + pass + + router = APIRouter() # Streams to display on dashboard -- derived from the registry's dashboard flag. @@ -1450,6 +1463,28 @@ async def adapters_edit_form( ) api_key_missing = not has_key + # Generic settings-driven preview. Adapters opt in by overriding + # SourceAdapter.preview_for_settings; the framework is duck-typed on the + # returned list[dict] shape and never branches on adapter name. + preview_rows: list[dict] | None = None + preview_error: str | None = None + if adapter_cls is not None and hasattr(adapter_cls, "settings_schema"): + try: + settings_obj = adapter_cls.settings_schema(**settings) + preview_cfg = AdapterConfig( + name=row["name"], + enabled=row["enabled"], + cadence_s=row["cadence_s"], + settings=settings, + updated_at=row["updated_at"], + ) + preview_adapter = adapter_cls( + preview_cfg, _PreviewConfigStore(), Path("/dev/null") + ) + preview_rows = await preview_adapter.preview_for_settings(settings_obj) + except Exception as exc: + preview_error = f"Preview unavailable: {exc}" + csrf_token = request.state.csrf_token response = templates.TemplateResponse( request=request, @@ -1466,6 +1501,8 @@ async def adapters_edit_form( "tile_attribution": tile_attribution, "api_key_missing": api_key_missing, "requires_api_key_alias": requires_api_key_alias, + "preview_rows": preview_rows, + "preview_error": preview_error, }, ) return response diff --git a/src/central/gui/templates/_adapter_preview.html b/src/central/gui/templates/_adapter_preview.html new file mode 100644 index 0000000..ff9ef0c --- /dev/null +++ b/src/central/gui/templates/_adapter_preview.html @@ -0,0 +1,23 @@ +{% if preview_error %} +
+ {{ preview_error }} +
+{% elif preview_rows %} +
+ Preview ({{ preview_rows|length }} rows) + + + + {% for col in preview_rows[0].keys() %}{% endfor %} + + + + {% for row in preview_rows %} + + {% for col in preview_rows[0].keys() %}{% endfor %} + + {% endfor %} + +
{{ col }}
{{ row[col] }}
+
+{% endif %} diff --git a/src/central/gui/templates/adapters_edit.html b/src/central/gui/templates/adapters_edit.html index 1b5a4b7..6e07f4b 100644 --- a/src/central/gui/templates/adapters_edit.html +++ b/src/central/gui/templates/adapters_edit.html @@ -178,6 +178,8 @@ {% endif %} + {% include "_adapter_preview.html" %} + Cancel diff --git a/tests/test_nwis.py b/tests/test_nwis.py index a68d283..22d7e6b 100644 --- a/tests/test_nwis.py +++ b/tests/test_nwis.py @@ -276,3 +276,84 @@ class TestNWISAdapter: assert first_pass assert second_pass == [] + + +class TestNWISPreview: + """Preview hook (PR G.5) — exercised without starting the adapter.""" + + @pytest.mark.asyncio + async def test_preview_returns_none_without_region(self, tmp_path: Path): + from central.adapters.nwis import NWISAdapter, NWISSettings + + adapter = NWISAdapter(_config({}), MagicMock(), tmp_path / "cursors.db") + result = await adapter.preview_for_settings(NWISSettings()) + assert result is None + + @pytest.mark.asyncio + async def test_preview_returns_rows_with_region(self, tmp_path: Path): + """Given a bbox, preview returns one dict per monitoring-locations feature + with the contract column order: site_id, name, site_type, state.""" + from central.adapters.nwis import NWISAdapter, NWISSettings + + sample_response = json.dumps({ + "type": "FeatureCollection", + "features": [ + { + "type": "Feature", + "id": "USGS-05420500", + "geometry": {"type": "Point", "coordinates": [-90.25, 41.78]}, + "properties": { + "monitoring_location_name": "MISSISSIPPI RIVER AT CLINTON, IA", + "site_type_code": "ST", + "state_name": "Iowa", + }, + }, + { + "type": "Feature", + "id": "USGS-05454500", + "geometry": {"type": "Point", "coordinates": [-91.65, 41.66]}, + "properties": { + "monitoring_location_name": "IOWA RIVER AT IOWA CITY, IA", + "site_type_code": "ST", + "state_name": "Iowa", + }, + }, + ], + }) + + adapter = NWISAdapter( + _config({"region": {"west": -94.0, "south": 40.0, "east": -93.0, "north": 41.0}}), + MagicMock(), + tmp_path / "cursors.db", + ) + adapter._fetch_preview_text = AsyncMock(return_value=sample_response) + settings = NWISSettings(region=adapter.region) + rows = await adapter.preview_for_settings(settings) + + assert rows is not None + assert len(rows) == 2 + # Column order is part of the contract — first row's keys must match exactly. + expected_keys = ["site_id", "name", "site_type", "state"] + assert list(rows[0].keys()) == expected_keys + # Row content reflects fixture data. + assert rows[0]["site_id"] == "USGS-05420500" + assert rows[0]["name"] == "MISSISSIPPI RIVER AT CLINTON, IA" + assert rows[0]["site_type"] == "ST" + assert rows[0]["state"] == "Iowa" + assert rows[1]["site_id"] == "USGS-05454500" + + @pytest.mark.asyncio + async def test_preview_propagates_http_error(self, tmp_path: Path): + """Preview must not swallow upstream errors — the framework needs them + to render the operator-visible 'Preview unavailable: …' banner.""" + from central.adapters.nwis import NWISAdapter, NWISSettings + + adapter = NWISAdapter( + _config({"region": {"west": -94.0, "south": 40.0, "east": -93.0, "north": 41.0}}), + MagicMock(), + tmp_path / "cursors.db", + ) + adapter._fetch_preview_text = AsyncMock(side_effect=RuntimeError("upstream 503")) + settings = NWISSettings(region=adapter.region) + with pytest.raises(RuntimeError, match="upstream 503"): + await adapter.preview_for_settings(settings) diff --git a/tests/test_preview_hook.py b/tests/test_preview_hook.py new file mode 100644 index 0000000..e9fbd30 --- /dev/null +++ b/tests/test_preview_hook.py @@ -0,0 +1,108 @@ +"""Tests for the SourceAdapter.preview_for_settings hook + framework rendering.""" + +from collections.abc import AsyncIterator +from pathlib import Path + +import jinja2 +import pytest +from pydantic import BaseModel + +from central.adapter import SourceAdapter +from central.models import Event + +TEMPLATES_DIR = Path(__file__).resolve().parents[1] / "src" / "central" / "gui" / "templates" + + +class _StubSettings(BaseModel): + pass + + +class _StubAdapter(SourceAdapter): + """Minimal SourceAdapter subclass that does NOT override preview_for_settings.""" + + name = "stub" + display_name = "Stub Adapter" + description = "Test fixture" + settings_schema = _StubSettings + default_cadence_s = 60 + + def __init__(self) -> None: + pass + + async def poll(self) -> AsyncIterator[Event]: # pragma: no cover - never invoked + if False: + yield # type: ignore[unreachable] + return + + async def apply_config(self, new_config) -> None: # pragma: no cover + pass + + def subject_for(self, event: Event) -> str: # pragma: no cover + return "stub" + + +@pytest.mark.asyncio +async def test_default_returns_none(): + """The base SourceAdapter's preview_for_settings default returns None. + + Any adapter that does not override the hook gets a no-op preview, so the + framework renders the page without a preview block (no crash, no opt-out + boilerplate required in each adapter). + """ + adapter = _StubAdapter() + result = await adapter.preview_for_settings(_StubSettings()) + assert result is None + + +def _render_partial(**context) -> str: + env = jinja2.Environment( + loader=jinja2.FileSystemLoader(str(TEMPLATES_DIR)), + autoescape=jinja2.select_autoescape(["html"]), + ) + tmpl = env.get_template("_adapter_preview.html") + return tmpl.render(**context) + + +def test_partial_renders_list(): + """Given list[dict] with insertion-ordered keys, partial renders a table. + + Framework is duck-typed: columns come from the first dict's keys. No + adapter-name branches, no per-adapter Jinja. + """ + rows = [ + {"a": "x1", "b": "y1"}, + {"a": "x2", "b": "y2"}, + ] + out = _render_partial(preview_rows=rows, preview_error=None) + # Header row uses first dict's keys in order. + assert "a" in out + assert "b" in out + # Body rows carry every value. + for cell in ("x1", "y1", "x2", "y2"): + assert cell in out + # Row count surfaces in the legend. + assert "Preview (2 rows)" in out + # No error banner when preview_error is None. + assert "Preview Unavailable" not in out + + +def test_partial_renders_error(): + """When preview_error is set, partial renders the error banner and no table.""" + out = _render_partial(preview_rows=None, preview_error="Preview unavailable: boom") + assert "Preview unavailable: boom" in out + # No table when an error is set. + assert "" not in out + + +def test_partial_renders_nothing_when_both_none(): + """No preview_rows and no preview_error -> partial renders no preview section. + + Lets adapters that don't opt in (e.g. EONET, GDACS) render normally without + any preview-related markup on the page. + """ + out = _render_partial(preview_rows=None, preview_error=None).strip() + assert " Date: Tue, 19 May 2026 17:55:39 +0000 Subject: [PATCH 2/2] fix(2-G.5): preview_for_settings contract in adapter docstring + distinguish [] from None MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixup 1 — Contract section appended to SourceAdapter.preview_for_settings's docstring. Override authors read adapter.py, not routes.py, so the contract (pure function of settings; open your own short-lived aiohttp session; None vs [] semantics) belongs on the base method, not on the GUI stub class. Fixup 2 — _adapter_preview.html distinguishes [] from None. Previously the elif test was truthiness (`elif preview_rows`) which collapsed both into "render nothing". Now uses `elif preview_rows is not none` and special-cases the empty-list case inside: legend "Preview (0 rows)" with no table; None still renders nothing at all. Lets adapters signal "query ran, matched zero" distinctly from "preview not meaningful". Tests +1: - test_partial_renders_empty_list — [] yields "Preview (0 rows)" legend, no table, no headers. Distinct from the existing None case. Acceptance: - 27/27 targeted (preview_hook +1 new, nwis, stream_registry). - 458/458 full suite. - (b) framework GUI dir still has zero adapter-name branches. --- src/central/adapter.py | 11 +++++++++++ src/central/gui/templates/_adapter_preview.html | 4 +++- tests/test_preview_hook.py | 12 ++++++++++++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/central/adapter.py b/src/central/adapter.py index 2a408b0..76ab04e 100644 --- a/src/central/adapter.py +++ b/src/central/adapter.py @@ -85,5 +85,16 @@ class SourceAdapter(ABC): Return list[dict] (framework renders as a generic table; columns come from the first dict's keys, in insertion order). Return None to skip preview. Raise to surface an error banner — framework catches at the route boundary. + + Contract: + - Preview is a pure function of `settings`. Do NOT access + self._config_store or cursor_db state — the framework may instantiate + adapters with a stub config_store solely to call this method. + - Network preview implementations must open their own short-lived + aiohttp session (the adapter's polling session may not exist; the GUI + process never calls startup()). + - Return None when preview is not meaningful (e.g., required settings + like region are unset). Return [] explicitly if the query ran and + matched zero rows — the framework renders that distinctly from None. """ return None diff --git a/src/central/gui/templates/_adapter_preview.html b/src/central/gui/templates/_adapter_preview.html index ff9ef0c..ab8a7f1 100644 --- a/src/central/gui/templates/_adapter_preview.html +++ b/src/central/gui/templates/_adapter_preview.html @@ -2,9 +2,10 @@
{{ preview_error }}
-{% elif preview_rows %} +{% elif preview_rows is not none %}
Preview ({{ preview_rows|length }} rows) + {% if preview_rows %} @@ -19,5 +20,6 @@ {% endfor %}
+ {% endif %}
{% endif %} diff --git a/tests/test_preview_hook.py b/tests/test_preview_hook.py index e9fbd30..88b9b1b 100644 --- a/tests/test_preview_hook.py +++ b/tests/test_preview_hook.py @@ -106,3 +106,15 @@ def test_partial_renders_nothing_when_both_none(): assert "Preview Unavailable" not in out # Either empty or only whitespace/newlines from the template. assert "Preview (" not in out + + +def test_partial_renders_empty_list(): + """Empty list -> legend with (0 rows), no table. + + Distinct from None (which renders nothing at all). Lets adapters signal + 'query ran, matched zero rows' separately from 'preview not meaningful'. + """ + out = _render_partial(preview_rows=[], preview_error=None) + assert "Preview (0 rows)" in out + assert "" not in out