Merge pull request #133 from jonathanstelman/feature/83-resort-data-validation

Add data validation to Resort Pydantic model (#83)

Author: jonathanstelman

backend/models.py

@@ -1,5 +1,84 @@
+import logging
+import re
 from typing import Optional
-from pydantic import BaseModel
+from pydantic import BaseModel, Field, field_validator
+
+logger = logging.getLogger(__name__)
+
+URL_PATTERN = re.compile(r'^https?://')
+
+# (lo, hi) bounds for fields where an out-of-range value most likely means a scraping
+# glitch (e.g. an extra digit) rather than a real resort. Ceilings are set well above the
+# largest real value currently in data/resorts.csv, not at some theoretical world-record max.
+_NONNEG_BOUNDS: dict[str, tuple[float, Optional[float]]] = {
+    'acres': (0, 50_000),
+    'num_trails': (0, 1_000),
+    'num_trails_xc': (0, 500),
+    'num_lifts': (0, 500),
+    'vertical': (0, 15_000),
+    'vertical_meters': (0, 5_000),
+    'vertical_base_ft': (0, 20_000),
+    'vertical_summit_ft': (0, 25_000),
+    'vertical_elevation_ft': (0, 15_000),
+    'snowfall_average_in': (0, 2_000),
+    'snowfall_high_in': (0, 2_500),
+    'trail_length_mi': (0, 200),
+    'trail_length_km': (0, 320),
+}
+
+_DIFFICULTY_FIELDS = (
+    'difficulty_beginner',
+    'difficulty_intermediate',
+    'difficulty_advanced',
+    'difficulty_beginner_xc',
+    'difficulty_intermediate_xc',
+    'difficulty_advanced_xc',
+)
+
+# Peak Rankings has, at least once, scored a resort an 11 (Spinal Tap style) — the ceiling
+# accommodates that rather than treating it as a data error.
+_PR_SUBSCORE_FIELDS = (
+    'pr_snow',
+    'pr_resiliency',
+    'pr_size',
+    'pr_terrain_diversity',
+    'pr_challenge',
+    'pr_lifts',
+    'pr_crowd_flow',
+    'pr_facilities',
+    'pr_navigation',
+    'pr_mountain_aesthetic',
+)
+
+
+def _null_if_out_of_range(value, lo, hi, field_name, resort_id):
+    if value is None:
+        return None
+    if (lo is not None and value < lo) or (hi is not None and value > hi):
+        logger.warning(
+            'resort=%s field=%s value=%r out of range [%s, %s] — nulling',
+            resort_id,
+            field_name,
+            value,
+            lo,
+            hi,
+        )
+        return None
+    return value
+
+
+def _null_if_bad_url(value, field_name, resort_id):
+    if value is None:
+        return None
+    if not URL_PATTERN.match(value):
+        logger.warning(
+            'resort=%s field=%s value=%r is not a valid http(s) URL — nulling',
+            resort_id,
+            field_name,
+            value,
+        )
+        return None
+    return value
 
 
 class RangeField(BaseModel):
@@ -113,7 +192,7 @@ class Resort(BaseModel):
     city: Optional[str] = None
     state: Optional[str] = None
     country: Optional[str] = None
-    indy_page: str
+    indy_page: str = Field(pattern=URL_PATTERN)
     website: Optional[str] = None
     reservation_status: str
     reservation_url: Optional[str] = None
@@ -185,3 +264,34 @@ class Resort(BaseModel):
     pr_nearest_cities: Optional[str] = None
     pr_pass_affiliation: Optional[str] = None
     pr_total_tt: Optional[str] = None
+
+    @field_validator(*_NONNEG_BOUNDS.keys(), mode='after')
+    @classmethod
+    def _validate_nonneg_bounded(cls, v, info):
+        lo, hi = _NONNEG_BOUNDS[info.field_name]
+        return _null_if_out_of_range(v, lo, hi, info.field_name, info.data.get('resort_id'))
+
+    @field_validator(*_DIFFICULTY_FIELDS, mode='after')
+    @classmethod
+    def _validate_difficulty(cls, v, info):
+        return _null_if_out_of_range(v, 0, 100, info.field_name, info.data.get('resort_id'))
+
+    @field_validator(*_PR_SUBSCORE_FIELDS, mode='after')
+    @classmethod
+    def _validate_pr_subscore(cls, v, info):
+        return _null_if_out_of_range(v, 0, 11, info.field_name, info.data.get('resort_id'))
+
+    @field_validator('pr_total', mode='after')
+    @classmethod
+    def _validate_pr_total(cls, v, info):
+        return _null_if_out_of_range(v, 0, None, info.field_name, info.data.get('resort_id'))
+
+    @field_validator('pr_overall_rank', 'pr_regional_rank', mode='after')
+    @classmethod
+    def _validate_pr_rank(cls, v, info):
+        return _null_if_out_of_range(v, 1, None, info.field_name, info.data.get('resort_id'))
+
+    @field_validator('website', 'reservation_url', mode='after')
+    @classmethod
+    def _validate_optional_url(cls, v, info):
+        return _null_if_bad_url(v, info.field_name, info.data.get('resort_id'))

backend/tests/test_meta_endpoint.py

@@ -27,16 +27,16 @@
         num_lifts=10.0,
         trail_length_mi=50.0,
         pr_total=85.0,
-        pr_snow=90.0,
-        pr_resiliency=80.0,
-        pr_size=75.0,
-        pr_terrain_diversity=70.0,
-        pr_challenge=65.0,
-        pr_lifts=60.0,
-        pr_crowd_flow=55.0,
-        pr_facilities=50.0,
-        pr_navigation=45.0,
-        pr_mountain_aesthetic=95.0,
+        pr_snow=9.0,
+        pr_resiliency=8.0,
+        pr_size=7.5,
+        pr_terrain_diversity=7.0,
+        pr_challenge=6.5,
+        pr_lifts=6.0,
+        pr_crowd_flow=5.5,
+        pr_facilities=5.0,
+        pr_navigation=4.5,
+        pr_mountain_aesthetic=9.5,
         blackout_all_dates=json.dumps(['2025-12-25', '2025-12-26']),
         ltt_blackout_all_dates=json.dumps(['2026-01-01']),
     ),
@@ -55,16 +55,16 @@
         num_lifts=5.0,
         trail_length_mi=25.0,
         pr_total=70.0,
-        pr_snow=75.0,
-        pr_resiliency=65.0,
-        pr_size=60.0,
-        pr_terrain_diversity=55.0,
-        pr_challenge=50.0,
-        pr_lifts=45.0,
-        pr_crowd_flow=40.0,
-        pr_facilities=35.0,
-        pr_navigation=30.0,
-        pr_mountain_aesthetic=80.0,
+        pr_snow=7.5,
+        pr_resiliency=6.5,
+        pr_size=6.0,
+        pr_terrain_diversity=5.5,
+        pr_challenge=5.0,
+        pr_lifts=4.5,
+        pr_crowd_flow=4.0,
+        pr_facilities=3.5,
+        pr_navigation=3.0,
+        pr_mountain_aesthetic=8.0,
         blackout_all_dates=json.dumps(['2025-12-24', '2026-01-02']),
         ltt_blackout_all_dates=json.dumps(['2026-01-15']),
     ),
@@ -111,7 +111,7 @@ def test_meta_null_values_excluded_from_ranges():
         response = client.get('/meta')
     data = response.json()
     # id-3 has no numeric fields; ranges should still be computed from id-1 and id-2
-    assert data['pr_mountain_aesthetic'] == {'min': 80.0, 'max': 95.0}
+    assert data['pr_mountain_aesthetic'] == {'min': 8.0, 'max': 9.5}
 
 
 def test_meta_blackout_date_ranges():

backend/tests/test_resort_validation.py

@@ -0,0 +1,90 @@
+import sys
+import os
+
+sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..'))
+
+import pytest
+from pydantic import ValidationError
+
+from data import load_resorts
+from models import Resort
+
+
+BASE_KWARGS = dict(
+    resort_id='r1',
+    name='Test Resort',
+    region='Test Region',
+    indy_page='https://example.com/test',
+    reservation_status='Not Required',
+)
+
+
+def test_negative_acres_is_nulled(caplog):
+    with caplog.at_level('WARNING'):
+        resort = Resort(**BASE_KWARGS, acres=-5)
+    assert resort.acres is None
+    assert any('acres' in r.message for r in caplog.records)
+
+
+def test_acres_within_bounds_is_kept():
+    resort = Resort(**BASE_KWARGS, acres=500)
+    assert resort.acres == 500
+
+
+def test_acres_absurdly_large_is_nulled():
+    # simulates a scraped value with an extra digit tacked on
+    resort = Resort(**BASE_KWARGS, acres=500000)
+    assert resort.acres is None
+
+
+def test_difficulty_over_100_is_nulled():
+    resort = Resort(**BASE_KWARGS, difficulty_beginner=150)
+    assert resort.difficulty_beginner is None
+
+
+def test_difficulty_within_bounds_is_kept():
+    resort = Resort(**BASE_KWARGS, difficulty_beginner=40)
+    assert resort.difficulty_beginner == 40
+
+
+def test_pr_subscore_of_11_is_kept():
+    # Peak Rankings has actually given a resort an 11 (Spinal Tap style)
+    resort = Resort(**BASE_KWARGS, pr_snow=11)
+    assert resort.pr_snow == 11
+
+
+def test_pr_subscore_over_11_is_nulled():
+    resort = Resort(**BASE_KWARGS, pr_snow=15)
+    assert resort.pr_snow is None
+
+
+def test_pr_rank_of_zero_is_nulled():
+    resort = Resort(**BASE_KWARGS, pr_overall_rank=0)
+    assert resort.pr_overall_rank is None
+
+
+def test_pr_total_negative_is_nulled():
+    resort = Resort(**BASE_KWARGS, pr_total=-1)
+    assert resort.pr_total is None
+
+
+def test_malformed_optional_url_is_nulled():
+    resort = Resort(**BASE_KWARGS, website='not-a-url')
+    assert resort.website is None
+
+
+def test_valid_optional_url_is_kept():
+    resort = Resort(**BASE_KWARGS, website='https://example.com')
+    assert resort.website == 'https://example.com'
+
+
+def test_malformed_indy_page_raises():
+    kwargs = dict(BASE_KWARGS)
+    kwargs['indy_page'] = 'not-a-url'
+    with pytest.raises(ValidationError):
+        Resort(**kwargs)
+
+
+def test_load_resorts_still_loads_real_data_without_hard_failures():
+    resorts = load_resorts()
+    assert len(resorts) > 0

docs/decisions.md

@@ -273,4 +273,11 @@ Format for new entries:
 **Issue:** #128
 **Decision:** Discussed design up front and posted the resolution as a comment on #128 before writing code (full rationale there). Landed on: not a locale selector — manual, uniform, explicit toggle only, since locale doesn't reliably predict preference and ski culture already has cross-locale exceptions (vertical drop commonly stays in feet even at metric resorts). Control shape is a single compact button showing the *current* unit as a short text glyph (`ft`/`m`, no flags/globe icons — flags don't render reliably on Windows and are both US-centric and inaccurate as a unit-system metaphor), not a two-option `Segmented`, because mobile screen real estate is too tight for showing both options at once (the search bar already spans full width on mobile). Same shared state (`hooks/useUnits.js` context + `localStorage` persistence, split from `components/common/UnitsProvider.jsx` to satisfy the `react-refresh/only-export-components` lint rule) renders in two places — header (next to "?") and sidebar top row (left-aligned, next to "Reset all filters" which stays right-aligned) — reading/writing the same state directly, no read-only mirror. Implementation is formatter-based (`utils/units.js`: `convert*` for plain numbers, `format*` for numbers with a unit suffix) over the canonical imperial value from the API, not the pre-computed parallel-column pattern (`vertical_meters`, `trail_length_km` in the CSV are now unused by the frontend) — new unit-bearing fields never need a pipeline-side `_meters`/`_km` companion column. `ResortTable.jsx`'s `vertical`/`vertical_meters` and `trail_length_mi`/`trail_length_km` collapsed into single toggle-driven columns (AG Grid `context={{ unit }}` + `headerValueGetter`/`valueFormatter`, refreshed via `api.refreshHeader()`/`refreshCells({force:true})` on toggle) rather than staying as two independent always-optional columns — removes the redundancy the toggle exists to solve. `StatsFilters.jsx`'s Vertical/Trail Length XC sliders convert their displayed labels/values while filtering continues against canonical ft/mi server-side (display-only layer, filter contract unchanged). `HowToUseModal.jsx` documents the toggle; its `Btn` chip helper gained a `variant="primary"` (teal, matching the real button) since the default variant is pink and was misrepresenting the actual toggle color.
 **Rationale:** Two live-review fixes applied after initial implementation: the sidebar toggle was grouped with "Reset all filters" via `marginLeft: auto`, reading as right-aligned in a narrow sidebar — moved to its own left-hand flex group so the row reads left-to-right (toggle+close-button, then Reset). Hectares confirmed correct as the metric acreage unit (not a typo/mix-up with "hectacre") — 1 ha = 10,000 m² = 2.471 ac, the standard metric land-area unit paired with acres.
+
+---
+## 2026-07-13 — Data validation added to `Resort` Pydantic model
+**Issue:** #83
+**Decision:** Constraints added to `backend/models.py`'s `Resort` model only (the CSV-parsing boundary in `backend/data.py`) — `ResortSummary` is always built from an already-validated `Resort.model_dump()` (`main.py:279`), never parsed from raw input, so duplicate constraints there would validate a scenario that can't happen. Two-tier design: **hard** (raise) for missing required fields and a malformed `indy_page` (the one required URL, via `Field(pattern=...)`); **soft** (log a warning, coerce to `None`) for every optional numeric/URL field via `field_validator(mode='after')` — one glitchy scraped value nulls itself out rather than taking down the whole resort or the pipeline run. Bounds were set from an actual audit of `data/resorts.csv` (not guessed): non-negative fields (acres, num_trails(+_xc), num_lifts, vertical variants, snowfall, trail_length) got both a floor *and* a generous ceiling (e.g. acres 0–50,000 against an observed max of 19,136) to catch a scraped value with an accidental extra digit; difficulty percentages 0–100; Peak Rankings subscores 0–**11** (not 10 — Peak Rankings has actually issued a real 11, Spinal-Tap-style, so the ceiling accommodates it instead of nulling a legitimate score); `pr_total` floor only (no confident ceiling); PR ranks `ge=1`.
+**Rationale:** Ceilings are calibrated to current real data with headroom, not a theoretical world-record max — the goal is catching obvious scraping glitches (10x errors), not constraining legitimate future resorts. Considered and rejected: applying the same constraints to `ResortSummary` (redundant, since it's never independently parsed); using `AnyUrl` for URL fields (normalizes/rewrites the string, e.g. adds a trailing slash to bare domains, which could subtly alter what's served — a plain regex `Field(pattern=...)`/soft-null keeps the string byte-for-byte as scraped); hard-failing the whole row on any out-of-range value (rejected per user direction — a single bad field on one resort shouldn't abort load or hide an otherwise-good resort). Verified against production data: all 277 current resorts load with zero fields nulled by the new bounds (bounds are accommodating enough for real data as it exists today).
+**Follow-up:** #77 (scheduled pipeline) still needs its own design pass on how the soft-null warnings surface in the pre-PR review — e.g. a "N fields nulled across M resorts" summary in the PR description — since silent nulling means `load_resorts()` no longer raises on out-of-range values, only on structural failures (missing required fields, malformed `indy_page`). #132 (drop unused `vertical_meters`/`trail_length_km`) will remove some of these fields' constraints entirely once done — not done yet, low priority.
 **Follow-up:** None — settled. Verified live via Playwright at desktop and 390px mobile viewports: toggle stays in sync across header/sidebar/table/sliders/detail modal/map tooltip, and the preference survives a full reload via `localStorage`. Not yet committed.

docs/planning.md

@@ -6,7 +6,9 @@ Current work-in-progress. Update this file at the start and end of every session
 
 ## Current Branch
 
-`feature/128-unit-selector` — Unit selector (imperial/metric). Implemented and verified live via Playwright (desktop + 390px mobile); not yet committed. See `docs/decisions.md` (2026-07-13) for the full design rationale, posted to #128 as a comment before implementation began.
+`feature/83-resort-data-validation` (commit `dde6e90`) — **#83 (data validation on Resort Pydantic model)**: `backend/models.py` gained bounded `field_validator`s (soft log+null) and a hard `indy_page` URL pattern constraint; `backend/tests/test_resort_validation.py` added; `backend/tests/test_meta_endpoint.py` fixture rescaled from placeholder 0–100 PR subscores to the real 0–11 scale. Full backend + pipeline suites pass, Black clean, verified against real `data/resorts.csv` (277 resorts, zero fields nulled). Backend-only change, no frontend surface to verify live. See `docs/decisions.md` (2026-07-13) for full rationale. Committed but not yet pushed/PR'd. Next: push branch, open PR, then #77 (scheduled pipeline, depends on this issue) can proceed.
+
+**#128 (unit selector) merged and deployed (2026-07-13)**, PR #131. Follow-up issue **#132** opened (low priority): drop now-unused `vertical_meters`/`trail_length_km` (and check `vertical_tt`/`acres_tt`) from the pipeline/backend model now that the frontend converts client-side instead of reading precomputed columns.
 
 **#118 merged and deployed (2026-07-12):** AG Grid Theming API migration — see `docs/decisions.md` for the full trail (real prior styling was mostly silently broken; row hover, checkmark contrast fix, header separator investigation).
 
@@ -37,11 +39,12 @@ Target: public launch on Indy Pass Facebook groups ahead of ski season.
 | #109 | UX: Peak Rankings visual encoding | P2 | Done |
 | #108 | UX: "How to use" first-load popover | P2 | Done |
 | #110 | UX: "Help improve this app" feedback section | P2 | Done |
-| #83 | Data validation on Resort Pydantic model | P1 (deferred) | Blocks #77 |
+| #83 | Data validation on Resort Pydantic model | P1 | Implemented, not yet committed |
 | #77 | GitHub Actions scheduled pipeline | P1 (deferred) | Depends on #83 |
 | #11 | Bug: alpine+XC metrics parsing | P1 | Done |
 | #118 | AG Grid Theming API migration | P2 | Done |
-| #128 | Unit selector (imperial/metric) | P2 | Implemented, not yet committed |
+| #128 | Unit selector (imperial/metric) | P2 | Done |
+| #132 | Drop unused vertical_meters/trail_length_km | P3 | Open, not started |
 
 **#113 merged and deployed (2026-05-30):**
 - Map/Table tab switcher, footer hidden, unified attribution ⓘ in tab bar
@@ -106,7 +109,7 @@ Small tasks interspersed with larger feature work. Check off here and in the Git
 - Also fixed `Footer.jsx`'s unrelated pre-existing antd deprecation warning (`overlayInnerStyle` → `styles.container`, same fix pattern as #119's `HowToUseModal`) — console is now fully clean on load, zero warnings or errors.
 - Follow-up caught after the first commit: `BoolCell`'s true-value checkmark used `COLORS.success` — same color as the new chartreuse row hover, so checkmarks vanished on hover (green-on-green). Changed to `COLORS.bgHeader` (near-black), trading the green "yes" cue for guaranteed legibility against both white and hovered rows.
 
-**#83/#77 deferred:** Cosmetic P2 issues take priority over automated pipeline work — the data being a day or two out-of-date isn't consequential right now.
+**#83 done (2026-07-13):** see Current Branch section above and `docs/decisions.md` for full detail.
 
 **#77 revised scope:** Pipeline runs on a schedule and opens a PR against main rather than auto-committing. Human reviews the data diff before merging. Pre-PR sanity check runs `load_resorts()` against the new CSV — any Pydantic validation failures abort the job before a PR is opened. See issue for full acceptance criteria.