Refactor OKC system data fetching into reusable hook#24
Conversation
Graceful demo fallback for agency APIs - /api/usgs and /api/usace now fall back to realistic mock data when the upstream feed is unreachable, empty, or 403-blocked. Responses carry canonical X-Data-Source headers (usgs-live | usace-live | mock-demo) so clients can tell live from demo at a glance. - Realigned MOCK_WATER_DATA keys with the site IDs actually referenced by SETTLEMENT_WATER_BODIES and OKC_RESERVOIR_SYSTEM, and added a USACE-ID -> USGS-ID map so SHEF sites get the right fallback. Shared live OKC-system snapshot - Introduced lib/useOKCSystem.ts which fetches all six city reservoirs once and derives combined storage, drought condition, and the Sardis release floor. OKCSystemStatus and DroughtMeter both accept an optional snapshot prop; the dashboard now threads a single live snapshot through both cards and the lawn-watering widget, replacing the previous hardcoded 78.5 / 82.3 / 79.1 placeholders. LakeCard source transparency - Cards now read X-Data-Source and display a dot + "Live" / "Demo" badge in the header, plus a matching footer label. Links flip to USACE vs USGS based on which source actually returned data. Data-status banner - Samples USGS and USACE endpoints, matches canonical live headers, and explains what "Demo Data Mode" means without blaming only USGS. Unblocked the production build - Added minimal local shims at components/ui/card.tsx, components/ui/button.tsx, and components/ui/icons.tsx so the water manager game can compile without pulling shadcn/ui or lucide-react. - Fixed a TS error in PixelLakeScene (drawPixelRect accepting gradients) and removed duplicate imageRendering keys. Verified with `npm run build` and smoke-tested all four routes (/, /dashboard, /settlement, /game) on the dev server.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis PR introduces a mock data fallback system for reservoir and water flow APIs, centralizes OKC system state into a custom hook, updates dashboard components to consume that hook, and adds a basic UI component library (Button, Card, Icons) to support the game component. ChangesUnified Data Layer & Component Refactor
Sequence DiagramsequenceDiagram
participant Dashboard
participant useOKCSystem
participant API Routes
participant Mock Data
participant Components
Dashboard->>useOKCSystem: Call hook on mount
activate useOKCSystem
useOKCSystem->>API Routes: Fetch USACE elevations (all reservoirs)
activate API Routes
alt Live data available
API Routes->>API Routes: Return with X-Data-Source: usace-live
else Live data unavailable
API Routes->>Mock Data: Fallback to mock
Mock Data-->>API Routes: Generator produces mock time-series
API Routes->>API Routes: Return with X-Data-Source: mock-demo
end
API Routes-->>useOKCSystem: Elevation values
deactivate API Routes
useOKCSystem->>useOKCSystem: Calculate storage %,<br/>derive drought condition,<br/>compute sardis rule
useOKCSystem-->>Dashboard: OKCSystemSnapshot
deactivate useOKCSystem
Dashboard->>Components: Pass snapshot to<br/>OKCSystemStatus,<br/>DroughtMeter,<br/>LawnWateringWidget
activate Components
Components->>Components: Render from snapshot data
Components-->>Dashboard: UI with live/mock badges
deactivate Components
Dashboard->>DataStatusBanner: Sample endpoints
activate DataStatusBanner
DataStatusBanner->>API Routes: Fetch from SAMPLE_REQUESTS
API Routes-->>DataStatusBanner: X-Data-Source headers
DataStatusBanner->>DataStatusBanner: Calculate live/mock/mixed ratio
DataStatusBanner-->>Dashboard: Status display
deactivate DataStatusBanner
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request centralizes the Oklahoma City reservoir system status logic into a new useOKCSystem hook and implements a robust mock data fallback for USACE and USGS API routes. UI components like DroughtMeter and OKCSystemStatus have been refactored to consume shared state, and a new DataStatusBanner provides transparency regarding data sources. Feedback identifies a safety concern in the drought trigger logic where missing data defaults to a 'Normal' status, potentially masking critical conditions. Other recommendations include validating USACE API responses to prevent NaN errors, optimizing the useOKCSystem hook to avoid redundant fetches, and addressing potential latency issues in the USACE route's sequential pattern matching.
| const hefnerPct = details.find((d) => d.id === 'hefner')?.percentFull ?? 100 | ||
| const draperPct = details.find((d) => d.id === 'draper')?.percentFull ?? 100 |
There was a problem hiding this comment.
Defaulting missing reservoir data to 100 is unsafe. The WSA drought logic requires all three indicators (System, Hefner, Draper) to be below a threshold. If data for one of these critical reservoirs is missing, the system will fail to trigger a drought condition even if the others are critical. Consider using 0 or handling the missing state explicitly to avoid false "Normal" status reports.
| const hefnerPct = details.find((d) => d.id === 'hefner')?.percentFull ?? 100 | |
| const draperPct = details.find((d) => d.id === 'draper')?.percentFull ?? 100 | |
| const hefnerPct = details.find((d) => d.id === 'hefner')?.percentFull ?? 0 | |
| const draperPct = details.find((d) => d.id === 'draper')?.percentFull ?? 0 |
| const d = await r.json() | ||
| const values: USACEValue[] | undefined = d.values | ||
| if (values && values.length) { | ||
| return Number(values[values.length - 1].value) |
There was a problem hiding this comment.
The USACE elevation value should be validated before being returned. If the API returns a non-numeric value or null, it could cause NaN issues in downstream calculations.
const latest = Number(values[values.length - 1].value)
return Number.isFinite(latest) ? latest : null| export function useOKCSystem(): OKCSystemSnapshot { | ||
| const [elevations, setElevations] = useState<Map<string, number>>(new Map()) | ||
| const [loading, setLoading] = useState(true) | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
The hook should support an enabled option to prevent redundant fetching when a snapshot is already provided by a parent component. This is especially important as this hook is used in multiple components on the same page.
| export function useOKCSystem(): OKCSystemSnapshot { | |
| const [elevations, setElevations] = useState<Map<string, number>>(new Map()) | |
| const [loading, setLoading] = useState(true) | |
| useEffect(() => { | |
| export function useOKCSystem(options: { enabled?: boolean } = {}): OKCSystemSnapshot { | |
| const { enabled = true } = options | |
| const [elevations, setElevations] = useState<Map<string, number>>(new Map()) | |
| const [loading, setLoading] = useState(enabled) | |
| useEffect(() => { | |
| if (!enabled) return |
| for (const pattern of ID_PATTERNS) { | ||
| const tsId = pattern(site, param) | ||
| const url = `${BASE_URL}?office=SWT&name=${encodeURIComponent(tsId)}&begin=${start.toISOString()}&end=${end.toISOString()}&page-size=500` | ||
|
|
||
| try { | ||
| const res = await fetchWithRetry(url, { | ||
| headers: { 'Accept': 'application/json;version=2' }, | ||
| next: { revalidate: 300 } // Cache for 5 minutes | ||
| headers: { Accept: 'application/json;version=2' }, | ||
| next: { revalidate: 300 } | ||
| }) | ||
|
|
||
| if (!res.ok) continue | ||
|
|
||
| const data = await res.json() | ||
| if (data.values && data.values.length > 0) { | ||
| // Filter out null values and format | ||
| interface USACETimeSeriesValue { | ||
| 0: number; // timestamp | ||
| 1: number | null; // value | ||
| 0: number | ||
| 1: number | null | ||
| } | ||
| const cleanValues = data.values | ||
| .filter((v: USACETimeSeriesValue) => v[1] !== null) | ||
| .map((v: USACETimeSeriesValue) => ({ | ||
| const cleanValues = (data.values as USACETimeSeriesValue[]) | ||
| .filter((v) => v[1] !== null) | ||
| .map((v) => ({ | ||
| dateTime: new Date(v[0]).toISOString(), | ||
| value: v[1] | ||
| value: v[1] as number | ||
| })) | ||
|
|
||
| if (cleanValues.length > 0) { | ||
| return NextResponse.json({ | ||
| source: 'usace', | ||
| tsId, | ||
| site, | ||
| param, | ||
| values: cleanValues | ||
| }, { | ||
| headers: { | ||
| 'Cache-Control': 'public, s-maxage=300, stale-while-revalidate=600', | ||
| 'X-Data-Source': 'usace-cwms', | ||
| 'X-Site-ID': site | ||
| return NextResponse.json( | ||
| { source: 'usace', tsId, site, param, values: cleanValues }, | ||
| { | ||
| headers: { | ||
| 'Cache-Control': 'public, s-maxage=300, stale-while-revalidate=600', | ||
| 'X-Data-Source': 'usace-live', | ||
| 'X-Site-ID': site | ||
| } | ||
| } | ||
| }) | ||
| ) | ||
| } | ||
| } | ||
| } catch (e) { | ||
| console.error(`USACE fetch failed for ${tsId}:`, e) | ||
| } | ||
| } |
There was a problem hiding this comment.
The sequential loop over ID_PATTERNS with an 8-second timeout per request can lead to extremely slow API responses, potentially exceeding serverless execution limits (e.g., Vercel's 10s limit) if multiple patterns are unresponsive. Total latency could reach 32+ seconds. Consider reducing the timeout further or attempting pattern fetches in parallel using Promise.any.
| const ownSnapshot = useOKCSystem() | ||
| const data = snapshot ?? ownSnapshot |
There was a problem hiding this comment.
The useOKCSystem hook is called even when a snapshot is provided via props, triggering redundant network requests. The hook should be updated to support an enabled option to skip fetching when data is already available.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
components/DroughtMeter.tsx (1)
14-20:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftSame unconditional
useOKCSystem()call — duplicate fetches whensnapshotis supplied.This mirrors
OKCSystemStatus: even when the dashboard passessnapshot, the hook still mounts its own effect and refetches the entire reservoir set. Fix at the hook/provider level (see comment inlib/useOKCSystem.ts), or makesnapshotrequired for the dashboard usage and dropownSnapshot.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/DroughtMeter.tsx` around lines 14 - 20, DroughtMeter currently calls useOKCSystem() unconditionally causing duplicate fetches when a snapshot prop is passed; change the component to avoid invoking useOKCSystem when snapshot is supplied (e.g., only call useOKCSystem when snapshot is undefined) or alternatively make snapshot required for dashboard usage and remove ownSnapshot logic; update the data selection so system/hefner/draper values derive from snapshot when present and only fall back to the hook (useOKCSystem) otherwise, and/or move the conditional fetch fix into the hook/provider (lib/useOKCSystem.ts) per the other comment so useOKCSystem does not start effects when its data is provided externally.
🧹 Nitpick comments (5)
components/ui/button.tsx (1)
23-25: ⚡ Quick winExtract the
cnhelper into a shared utility — it's already duplicated incard.tsx.Both
button.tsxandcard.tsxdefine an identical one-liner. Any future change (e.g. switching toclsx/tailwind-merge) must be applied in two places.♻️ Proposed refactor
Create
components/ui/utils.ts(orlib/cn.ts):export function cn(...parts: Array<string | undefined>) { return parts.filter(Boolean).join(' ') }Then in both
card.tsxandbutton.tsx:-function cn(...parts: Array<string | undefined>) { - return parts.filter(Boolean).join(' ') -} +import { cn } from './utils'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/ui/button.tsx` around lines 23 - 25, There are duplicate one-line helpers named cn in the button and card components; extract a single exported helper (e.g., export function cn(...parts: Array<string|undefined>) { return parts.filter(Boolean).join(' ') }) into a new shared module (e.g., utils or lib), replace the local cn definitions in both components with an import of the shared cn, and remove the duplicated function declarations so both components use the same implementation.components/LakeCard.tsx (1)
96-118: 💤 Low valueOptional: short-circuit on non-OK USGS response.
The USACE branch checks
res.okbefore parsing, but the USGS branch parses JSON unconditionally. It happens to work becausemockResponse(..., 'unavailable')returns valid JSON withoutvalue.timeSeries, so the empty-points guard takes over. Adding anif (!res.ok)early-return would make the two branches symmetric and make the intent (live vs fallback) more explicit, without changing observable behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/LakeCard.tsx` around lines 96 - 118, In LakeCard.tsx inside the USGS fetch branch, add a check for non-OK responses before calling res.json() to mirror the USACE branch: after awaiting fetch(`/api/usgs?...`) inspect if (!res.ok) then handle/return early (e.g., skip parsing and let the fallback logic proceed) so you don't parse error responses; update references in this block around the variables res, json, values, pts, setSeries, setLatest and the try/catch to ensure behavior remains unchanged for valid responses.lib/useOKCSystem.ts (2)
88-104: 💤 Low valueOptional: memoize the derived snapshot to stabilize references.
calculateCombinedStorage,determineWSADroughtCondition,getSardisRestriction, and the returned object literal all run on every render and produce new references even whenelevationsis unchanged. Wrapping the derivation and the returned object inuseMemo([elevations, loading])would let downstreamReact.memo/useMemoconsumers skip work and is straightforward.♻️ Sketch
- const { totalStorage, percentage, details } = calculateCombinedStorage(elevations) - const hefnerPct = details.find((d) => d.id === 'hefner')?.percentFull ?? 100 - const draperPct = details.find((d) => d.id === 'draper')?.percentFull ?? 100 - const drought = determineWSADroughtCondition(percentage, hefnerPct, draperPct) - const sardisRule = getSardisRestriction(drought.condition) - - return { - loading, - elevations, - totalStorage, - percentage, - details, - hefnerPct, - draperPct, - drought, - sardisRule - } + return useMemo<OKCSystemSnapshot>(() => { + const { totalStorage, percentage, details } = calculateCombinedStorage(elevations) + const hefnerPct = details.find((d) => d.id === 'hefner')?.percentFull ?? 100 + const draperPct = details.find((d) => d.id === 'draper')?.percentFull ?? 100 + const drought = determineWSADroughtCondition(percentage, hefnerPct, draperPct) + const sardisRule = getSardisRestriction(drought.condition) + return { loading, elevations, totalStorage, percentage, details, hefnerPct, draperPct, drought, sardisRule } + }, [elevations, loading])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/useOKCSystem.ts` around lines 88 - 104, Wrap the computation and returned snapshot in a useMemo so derived values (calculateCombinedStorage, determineWSADroughtCondition, getSardisRestriction and the returned object) only re-compute when inputs change; e.g., memoize the block that calls calculateCombinedStorage(elevations), extracts hefnerPct/draperPct, computes drought and sardisRule, and returns the object using React.useMemo with [elevations, loading] (or the minimal dependency list) so stable references are returned for downstream React.memo/useMemo consumers.
14-43: 💤 Low valueDefaulting
hefnerPct/draperPctto 100 when reservoirs are missing can mask drought signals.If every USACE/USGS fetch fails (e.g., total outage with no
MOCK_WATER_DATAmatch),detailsis empty sohefnerPct = draperPct = 100andpercentage = 0.determineWSADroughtCondition(0, 100, 100)returnscondition: 'none'because the rule requires all three to be below the threshold — i.e., the meter would show 0% combined storage and "No Restrictions" simultaneously. Mock fallbacks make this unlikely in practice, but consider either gating the UI onloading || details.length < expectedor returningnull/undefinedpercentages to make this state explicit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/useOKCSystem.ts` around lines 14 - 43, The code currently defaults hefnerPct and draperPct to 100 when reservoir fetches fail (details is empty), which can hide drought signals; update the logic in the useOKCSystem hook so that when fetches fail or details.length is less than the expected reservoir count you either (A) set hefnerPct and draperPct to null/undefined (not 100) and propagate that through percentage and UI, or (B) add a loading/missing-data gate (e.g., loading || details.length < expected) to prevent rendering/deciding drought state; ensure you update any callers of determineWSADroughtCondition to handle nullable percentages and reference the variables/details used in the hook (hefnerPct, draperPct, percentage, details, and determineWSADroughtCondition) so the UI shows an explicit missing-data state instead of "No Restrictions."app/api/usgs/route.ts (1)
7-9: 💤 Low valueMinor: retry/timeout constants drift from
app/api/usace/route.ts.USGS uses
MAX_RETRIES = 2while USACE usesMAX_RETRIES = 1. If this is intentional (e.g., USGS is more transient and worth one extra try), no action needed; otherwise consider extracting both routes' constants into a shared module so they stay aligned.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/usgs/route.ts` around lines 7 - 9, The MAX_RETRIES/RETRY_DELAY_MS/FETCH_TIMEOUT_MS values in app/api/usgs/route.ts differ from the ones in app/api/usace/route.ts; to keep them aligned extract these constants into a shared module (e.g., export const MAX_RETRIES, RETRY_DELAY_MS, FETCH_TIMEOUT_MS from a new shared file) and replace the literal declarations in both route.ts files with imports from that module, or if the higher MAX_RETRIES for USGS is intentional, add a comment near MAX_RETRIES in app/api/usgs/route.ts documenting the reason so divergence is explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/OKCSystemStatus.tsx`:
- Around line 12-17: OKCSystemStatus currently calls the hook useOKCSystem()
unconditionally which violates the snapshot optimization; either update
lib/useOKCSystem.ts to avoid performing a fresh fetch when a prepopulated
snapshot is available (e.g., add a context/shared in-flight cache or a parameter
like useOKCSystem({preferCache: true, initialSnapshot}) so the hook returns the
provided snapshot synchronously without refetching), or change OKCSystemStatus
signature to make snapshot required and remove the useOKCSystem() call so no
redundant fetch happens; reference the OKCSystemStatus component and
useOKCSystem hook when applying the change.
In `@components/ui/button.tsx`:
- Line 36: Update the Button component's class string: replace the Tailwind
utility 'focus:outline-none' with 'focus:outline-hidden' inside the class list
used by the Button (the inline-flex...rounded-md...transition-colors string) so
high-contrast/forced-colors users retain an accessible focus outline while
keeping the existing ring utilities.
In `@components/ui/icons.tsx`:
- Around line 1-5: The Icon component uses React.ReactNode without importing
React, causing TS2304; fix by importing the React types and updating the props:
add an import of the ReactNode type (e.g., import type { SVGProps, ReactNode }
from 'react') and change the component signature to use ReactNode instead of
React.ReactNode (ensure IconProps and the Icon function parameter keep their
existing names).
In `@lib/useOKCSystem.ts`:
- Around line 57-105: The hook useOKCSystem currently runs its own fetch effect
on every mount causing duplicate requests when multiple components
(OKCSystemStatus, DroughtMeter, Dashboard) call it; fix by moving the
fetch/elevation state into a context provider (e.g., create an OKCSystemProvider
that runs the existing load effect which iterates OKC_RESERVOIR_SYSTEM and calls
fetchLatestElevation, stores elevations/loading/derived values via
calculateCombinedStorage, determineWSADroughtCondition, getSardisRestriction)
and change useOKCSystem to read from that context (and make consumer components
accept a required snapshot prop instead of falling back to their own hook), so
all consumers share a single underlying effect and in-flight work.
---
Duplicate comments:
In `@components/DroughtMeter.tsx`:
- Around line 14-20: DroughtMeter currently calls useOKCSystem() unconditionally
causing duplicate fetches when a snapshot prop is passed; change the component
to avoid invoking useOKCSystem when snapshot is supplied (e.g., only call
useOKCSystem when snapshot is undefined) or alternatively make snapshot required
for dashboard usage and remove ownSnapshot logic; update the data selection so
system/hefner/draper values derive from snapshot when present and only fall back
to the hook (useOKCSystem) otherwise, and/or move the conditional fetch fix into
the hook/provider (lib/useOKCSystem.ts) per the other comment so useOKCSystem
does not start effects when its data is provided externally.
---
Nitpick comments:
In `@app/api/usgs/route.ts`:
- Around line 7-9: The MAX_RETRIES/RETRY_DELAY_MS/FETCH_TIMEOUT_MS values in
app/api/usgs/route.ts differ from the ones in app/api/usace/route.ts; to keep
them aligned extract these constants into a shared module (e.g., export const
MAX_RETRIES, RETRY_DELAY_MS, FETCH_TIMEOUT_MS from a new shared file) and
replace the literal declarations in both route.ts files with imports from that
module, or if the higher MAX_RETRIES for USGS is intentional, add a comment near
MAX_RETRIES in app/api/usgs/route.ts documenting the reason so divergence is
explicit.
In `@components/LakeCard.tsx`:
- Around line 96-118: In LakeCard.tsx inside the USGS fetch branch, add a check
for non-OK responses before calling res.json() to mirror the USACE branch: after
awaiting fetch(`/api/usgs?...`) inspect if (!res.ok) then handle/return early
(e.g., skip parsing and let the fallback logic proceed) so you don't parse error
responses; update references in this block around the variables res, json,
values, pts, setSeries, setLatest and the try/catch to ensure behavior remains
unchanged for valid responses.
In `@components/ui/button.tsx`:
- Around line 23-25: There are duplicate one-line helpers named cn in the button
and card components; extract a single exported helper (e.g., export function
cn(...parts: Array<string|undefined>) { return parts.filter(Boolean).join(' ')
}) into a new shared module (e.g., utils or lib), replace the local cn
definitions in both components with an import of the shared cn, and remove the
duplicated function declarations so both components use the same implementation.
In `@lib/useOKCSystem.ts`:
- Around line 88-104: Wrap the computation and returned snapshot in a useMemo so
derived values (calculateCombinedStorage, determineWSADroughtCondition,
getSardisRestriction and the returned object) only re-compute when inputs
change; e.g., memoize the block that calls calculateCombinedStorage(elevations),
extracts hefnerPct/draperPct, computes drought and sardisRule, and returns the
object using React.useMemo with [elevations, loading] (or the minimal dependency
list) so stable references are returned for downstream React.memo/useMemo
consumers.
- Around line 14-43: The code currently defaults hefnerPct and draperPct to 100
when reservoir fetches fail (details is empty), which can hide drought signals;
update the logic in the useOKCSystem hook so that when fetches fail or
details.length is less than the expected reservoir count you either (A) set
hefnerPct and draperPct to null/undefined (not 100) and propagate that through
percentage and UI, or (B) add a loading/missing-data gate (e.g., loading ||
details.length < expected) to prevent rendering/deciding drought state; ensure
you update any callers of determineWSADroughtCondition to handle nullable
percentages and reference the variables/details used in the hook (hefnerPct,
draperPct, percentage, details, and determineWSADroughtCondition) so the UI
shows an explicit missing-data state instead of "No Restrictions."
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 05d21a57-5247-4410-ab0e-ebab81826fbc
📒 Files selected for processing (14)
app/api/usace/route.tsapp/api/usgs/route.tsapp/dashboard/page.tsxcomponents/DataStatusBanner.tsxcomponents/DroughtMeter.tsxcomponents/LakeCard.tsxcomponents/OKCSystemStatus.tsxcomponents/PixelLakeScene.tsxcomponents/WaterManagerGame.tsxcomponents/ui/button.tsxcomponents/ui/card.tsxcomponents/ui/icons.tsxlib/mockData.tslib/useOKCSystem.ts
| export function useOKCSystem(): OKCSystemSnapshot { | ||
| const [elevations, setElevations] = useState<Map<string, number>>(new Map()) | ||
| const [loading, setLoading] = useState(true) | ||
|
|
||
| useEffect(() => { | ||
| let cancelled = false | ||
|
|
||
| const load = async () => { | ||
| const entries = await Promise.all( | ||
| OKC_RESERVOIR_SYSTEM.map(async (res) => { | ||
| const val = await fetchLatestElevation(res.usgsId, res.usaceId) | ||
| return [res.id, val] as const | ||
| }) | ||
| ) | ||
|
|
||
| if (cancelled) return | ||
|
|
||
| const next = new Map<string, number>() | ||
| for (const [id, val] of entries) { | ||
| if (val !== null) next.set(id, val) | ||
| } | ||
| setElevations(next) | ||
| setLoading(false) | ||
| } | ||
|
|
||
| void load() | ||
| return () => { | ||
| cancelled = true | ||
| } | ||
| }, []) | ||
|
|
||
| const { totalStorage, percentage, details } = calculateCombinedStorage(elevations) | ||
| const hefnerPct = details.find((d) => d.id === 'hefner')?.percentFull ?? 100 | ||
| const draperPct = details.find((d) => d.id === 'draper')?.percentFull ?? 100 | ||
| const drought = determineWSADroughtCondition(percentage, hefnerPct, draperPct) | ||
| const sardisRule = getSardisRestriction(drought.condition) | ||
|
|
||
| return { | ||
| loading, | ||
| elevations, | ||
| totalStorage, | ||
| percentage, | ||
| details, | ||
| hefnerPct, | ||
| draperPct, | ||
| drought, | ||
| sardisRule | ||
| } | ||
| } |
There was a problem hiding this comment.
Hook produces redundant fetches when used alongside the optional snapshot prop pattern.
OKCSystemStatus and DroughtMeter both call useOKCSystem() unconditionally (Rules of Hooks) and only then choose snapshot ?? ownSnapshot. On the dashboard — which already calls useOKCSystem() and passes the snapshot down — this means three independent hook instances each fire OKC_RESERVOIR_SYSTEM.length × 2 fetches on mount, defeating the "share system state across components" goal stated in the PR description.
Consider one of these to actually share state:
- Provide a
OKCSystemProvidercontext wrapping the dashboard; letuseOKCSystem()read from the context (with a single underlying effect at the provider level), and have consumers read from the context directly. - Or, deduplicate the underlying fetches at module scope (e.g., a singleton
Promise<Map>keyed by mount tick / TTL) so concurrent hook instances share the same in-flight work.
A lighter alternative is to drop the snapshot ?? ownSnapshot fallback in the consumers and make snapshot required, leaving useOKCSystem only for code paths that genuinely render standalone.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/useOKCSystem.ts` around lines 57 - 105, The hook useOKCSystem currently
runs its own fetch effect on every mount causing duplicate requests when
multiple components (OKCSystemStatus, DroughtMeter, Dashboard) call it; fix by
moving the fetch/elevation state into a context provider (e.g., create an
OKCSystemProvider that runs the existing load effect which iterates
OKC_RESERVOIR_SYSTEM and calls fetchLatestElevation, stores
elevations/loading/derived values via calculateCombinedStorage,
determineWSADroughtCondition, getSardisRestriction) and change useOKCSystem to
read from that context (and make consumer components accept a required snapshot
prop instead of falling back to their own hook), so all consumers share a single
underlying effect and in-flight work.
Drought trigger safety (gemini-code-assist HIGH) - Default missing Hefner/Draper percentages to 0 (fail-safe), not 100. Under WSA Section 6 the drought condition only fires when all three indicators are below threshold, so defaulting absent readings high could mask a real drought; defaulting to 0 errs toward declaration. Hook & data-fetch hygiene - Add Number.isFinite validation to the USACE elevation path so a malformed value can't poison combined-storage math. - Add `enabled` option to useOKCSystem; OKCSystemStatus and DroughtMeter now pass `enabled: !snapshot` so they don't refetch when the dashboard has already supplied a snapshot. - Wrap the derived snapshot in useMemo so consumers get a stable reference between renders. USACE route latency (gemini-code-assist MEDIUM) - Race the four timeseries-ID patterns in parallel via Promise.any instead of awaiting them sequentially. Bounds total latency to a single fetch's worth instead of 4x, fitting comfortably under Vercel's serverless timeout. UI shims polish (CodeRabbit) - icons.tsx: import ReactNode directly instead of relying on a missing React namespace. - button.tsx: focus:outline-none -> focus:outline-hidden to preserve the accessible outline in Tailwind v4 forced-colors / High Contrast. - Extract `cn` helper into components/ui/utils.ts and reuse it from card.tsx and button.tsx. LakeCard symmetry - Skip JSON parsing on non-OK USGS responses so the USGS branch matches the USACE branch's early-return shape. Build verified with `npm run build` and routes smoke-tested locally.
2 participants
Summary
Extracted the OKC reservoir system data fetching and calculation logic from
OKCSystemStatuscomponent into a newuseOKCSystemhook, enabling better code reuse and component composition across the dashboard.Key Changes
New
useOKCSystemhook (lib/useOKCSystem.ts): Centralizes all OKC reservoir elevation fetching, storage calculations, and drought condition determination. Returns aOKCSystemSnapshotwith loading state and computed metrics.Refactored
OKCSystemStatus: Now accepts optional pre-fetched snapshot via props, falling back touseOKCSystem()if not provided. Simplified from 75 to 27 lines by removing fetch/calculation logic.Updated
DroughtMeter: Now accepts optional snapshot and supports override percentages for "what if" scenarios. Includes loading state handling and cleaner prop interface.Enhanced
LakeCard: Improved data source tracking with newDataSourcetype andSOURCE_LABELmapping. Better error handling and type safety for USACE/USGS responses.Improved API routes (
/api/usace,/api/usgs): Added mock data fallback withX-Data-Sourceheaders for better observability. Reduced retry counts and timeouts for faster failure detection.Mock data refactor (
lib/mockData.ts): Changed from static data to generator functions, allowing fresh data on each request. AddedgetMockUsaceValues()export for API fallbacks.New UI components: Added
ButtonandCardcomponents tocomponents/ui/for consistent styling.Dashboard integration (
app/dashboard/page.tsx): Now usesuseOKCSystemhook to share system state across multiple components, enabling coordinated drought stage display.Data status detection: Enhanced
DataStatusBannerto sample both USGS and USACE endpoints for more accurate live/mock/mixed state detection.Implementation Details
useEffectwith cancellation flag to prevent state updates after unmountX-Data-Sourceheader for client-side source trackinghttps://claude.ai/code/session_01VBRrwqDSFmiHPBVZjfFybQ
Summary by CodeRabbit
New Features
Improvements