geolocator: Rev 6A agri_yield_pipeline physical-state overlay - #18
geolocator: Rev 6A agri_yield_pipeline physical-state overlay#18aurascoper wants to merge 2 commits into
Conversation
New agri_overlay TrackedSource, REFERENCE class, registered in the same pattern as power/worldgrid. Reads a hash-verified JSON export from a separate repo (agri_yield_pipeline/data/overlay/latest.json, a symlink that repo's export script retargets each run) -- never imports that repo's code, only its frozen export. _load_agri_overlay() verifies the export's own embedded cells_sha256 before accepting it -- same corruption-detection discipline recently added to live_trading's energy_market_bridge_probe*.py snapshot loaders. Verified live: a deliberately corrupted export is rejected (status unavailable, clear hash-mismatch error) before being served, and touching the file reloads it (reloaded_at advances) without changing the reported vintage. Also fixed the /api/layers color_field ternary before it grew a third, wrong case -- the existing comment already documents that a two-way ternary once told the legend something false for one layer; converted to an explicit per-layer dict.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 177226aece
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "agri_overlay": TrackedSource( | ||
| # Rev 6A: county-level NDVI/weather/yield-sensitivity physical-state overlay from | ||
| # agri_yield_pipeline. No trading claim; see that repo's plan for scope. REFERENCE, not | ||
| # LIVE -- the vintage is the export's generated_at, never today's clock. | ||
| id="agri_overlay", path=AGRI_OVERLAY_JSON, layer_class=REFERENCE, |
There was a problem hiding this comment.
Adding this source makes agri_overlay available through the API, but the globe UI still hardcodes only the original six IDs in geolocator/static/app/layers.js:21-28; all group creation, toggles, and /api/plants fetches iterate that list. Consequently, even when AGRI_OVERLAY_JSON contains valid data, users have no way to enable or view the new physical-state overlay in the application.
Useful? React with 👍 / 👎.
| # a float (see _f()); /api/stats and /api/plants both compare/sum it | ||
| # unconditionally, so a missing z becomes 0.0, not None -- caught by the | ||
| # public test suite before this ever shipped. | ||
| "capacity_mw": abs(z) if z is not None else 0.0, |
There was a problem hiding this comment.
When this layer is queried, storing abs(z) as capacity_mw exposes a dimensionless NDVI z-score as megawatts: /api/stats sums it into total_capacity_mw, while the globe HUD and tooltip label the same value as Capacity ... MW and apply the Min MW filter. This produces physically false statistics and labels for every overlay cell with NDVI data; use a separate marker-magnitude property or make the shared consumers layer-aware instead.
Useful? React with 👍 / 👎.
Second review pass, before the first cherry-pick's fix (None -> 0.0) merged anywhere: - _load_agri_overlay reverts capacity_mw to None when no NDVI baseline exists. 0.0 made "unknown" indistinguishable from "measured zero" in any sum or average -- the fix belonged in the two generic endpoints that assumed always-float, not in this loader picking a number to make them stop crashing. - /api/stats excludes None from total_capacity_mw and capacity_by_fuel, and always reports unknown_capacity_count (not just implied by a smaller-than-count total). - /api/plants: an explicit min_capacity/max_capacity filter can't be verified against an unknown value, so it's excluded -- but the *default* min_capacity=0.0 must not exclude unknowns just because 0.0 is a technically-passable floor. excluded_unknown_capacity is always present (0 by default), matching /api/stats's convention, so a consumer can tell "no filter active" apart from "filter active, nothing excluded" instead of the two endpoints answering the same question in different shapes. - _load_agri_overlay verifies payload_sha256 (schema_version>=2) over the whole payload minus that key, not cells_sha256 over cells alone -- a cells-only hash left generated_at/source_git_sha/provenance unprotected. Falls back to the old cells_sha256 check for schema_version 1 exports. New tests/test_agri_overlay.py: valid-payload parsing, a positive control (corrupt cells -> rejected) and the negative control that actually proves the hash widened (corrupt generated_at specifically -> rejected; would have passed silently under the old cells-only scheme), a schema-version-1 backward-compatibility check, and endpoint-level coverage for both the default and active-filter paths through /api/stats and /api/plants.
|
Second review pass, pushed as
107 → 115 passed. |
1 participant
Summary
agri_overlayTrackedSource,REFERENCEclass, registered in the same pattern aspower/worldgrid. Reads a hash-verified JSON export from a separate repo (agri_yield_pipeline,data/overlay/latest.json— a symlink that repo's export script retargets each run). Never imports that repo's code, only its frozen export — same two-product boundary already established for the trading-lattice proxy._load_agri_overlay()verifies the export's own embeddedcells_sha256before accepting it — same corruption-detection discipline as the snapshot loaders recently added tolive_trading's energy-market-bridge research harness./api/layerscolor_fieldternary before it grew a third, wrong case — the existing comment already documents a two-way ternary once telling the legend something false for one layer; converted to an explicit per-layer dict.capacity_mwwasNonefor counties with no NDVI baseline, but two existing endpoints (/api/stats,/api/plants) assume it's always a float (the repo-wide_f()convention). Now defaults to0.0, consistent with every other layer.What this is not
No trading claim. County-level NDVI z-score, a July Daymet weather anomaly (the exporting repo's data doesn't extend past 2023-12-31), the static county tmax-yield sensitivity table, and Sentinel-1 SAR reported as an explicitly-stale reference value (the exporting repo's weekly refresh automation has been broken for 4 months — documented in its own export, not silently smoothed over here).
Test plan
python3.13 -m pytest -q geolocator/tests— 107 passed (was 105 passed / 2 failed before thecapacity_mwfix)/api/healthreportsclass: reference, a real vintage (the export's owngenerated_at, never live)reloaded_atadvances) without changing the reported vintagestatus: unavailable, clear hash-mismatch error) before being served/api/stats?layer=agri_overlayand/api/plants?layer=agri_overlaynow return 200 on the live service🤖 Generated with Claude Code
https://claude.ai/code/session_019ULbfnMy6CKDWWD9aQdYY2