From d911af12256cfdacb69aae05b9f92626fb48540d Mon Sep 17 00:00:00 2001 From: "2569718930@qq.com" <2569718930@qq.com> Date: Sun, 31 May 2026 22:14:17 +0800 Subject: [PATCH] Reduce duplicate terminal detail loads --- .../LiveTemperatureThresholdChart.tsx | 17 +- .../__tests__/refreshCadencePolicy.test.ts | 4 + .../__tests__/ssePatchArchitecture.test.ts | 3 +- .../scan-terminal/temperature-chart-logic.ts | 2 +- tests/test_web_observability.py | 103 ++++++++++ web/services/city_api.py | 184 +++++++++++++----- 6 files changed, 262 insertions(+), 51 deletions(-) diff --git a/frontend/components/dashboard/scan-terminal/LiveTemperatureThresholdChart.tsx b/frontend/components/dashboard/scan-terminal/LiveTemperatureThresholdChart.tsx index 6faed4bb..22c4ae84 100644 --- a/frontend/components/dashboard/scan-terminal/LiveTemperatureThresholdChart.tsx +++ b/frontend/components/dashboard/scan-terminal/LiveTemperatureThresholdChart.tsx @@ -56,6 +56,7 @@ const PEAK_GLOW_BADGE_CLASS = { } as const; const PROBABILITY_REFRESH_AFTER_PATCH_MS = 60_000; +const FOREGROUND_FULL_DETAIL_REFRESH_DEDUP_MS = 90_000; const DETAIL_LOAD_BATCH_DELAY_MS = 0; const TemperatureChartCanvas = dynamic( @@ -165,6 +166,7 @@ export function LiveTemperatureThresholdChart({ const lastPatchAtRef = useRef(Date.now()); const lastAppliedPatchRevisionRef = useRef(0); const lastProbabilityRefreshAtRef = useRef(0); + const lastForegroundRefreshAtRef = useRef(0); const localDayRolloverFetchDateRef = useRef(""); const [isChartVisible, setIsChartVisible] = useState( () => typeof IntersectionObserver === "undefined", @@ -197,6 +199,7 @@ export function LiveTemperatureThresholdChart({ lastPatchAtRef.current = Date.now(); lastAppliedPatchRevisionRef.current = 0; lastProbabilityRefreshAtRef.current = 0; + lastForegroundRefreshAtRef.current = 0; localDayRolloverFetchDateRef.current = ""; setCurrentCityLocalDate(formatCityLocalDate(row?.tz_offset_seconds)); }, [city]); @@ -395,7 +398,19 @@ export function LiveTemperatureThresholdChart({ let cancelled = false; const refreshForegroundFullDetail = () => { - lastPatchAtRef.current = Date.now(); + const now = Date.now(); + const cacheKey = `${city}:${targetResolution}`; + const cached = _hourlyCache.get(cacheKey); + const cacheAge = cached ? now - Number(cached.ts || 0) : Number.POSITIVE_INFINITY; + if ( + now - lastForegroundRefreshAtRef.current < FOREGROUND_FULL_DETAIL_REFRESH_DEDUP_MS || + (cacheAge >= 0 && cacheAge < FOREGROUND_FULL_DETAIL_REFRESH_DEDUP_MS) + ) { + return; + } + + lastForegroundRefreshAtRef.current = now; + lastPatchAtRef.current = now; fetchHourlyForecastForCity(city, { ignoreCache: true, resolution: targetResolution }) .then((data) => { diff --git a/frontend/components/dashboard/scan-terminal/__tests__/refreshCadencePolicy.test.ts b/frontend/components/dashboard/scan-terminal/__tests__/refreshCadencePolicy.test.ts index 231f612d..8bb15b76 100644 --- a/frontend/components/dashboard/scan-terminal/__tests__/refreshCadencePolicy.test.ts +++ b/frontend/components/dashboard/scan-terminal/__tests__/refreshCadencePolicy.test.ts @@ -102,6 +102,10 @@ export async function runTests() { chartLogicSource.includes("primeCityDetailCache"), "visible terminal chart detail fetches should be coalesced into one batch request and prime the shared chart cache", ); + assert( + chartLogicSource.includes("CITY_DETAIL_BATCH_WINDOW_MS = 100"), + "visible terminal chart detail fetches should use a wide enough batch window to coalesce cards mounted across adjacent frames", + ); assert( chartLogicSource.includes("partial?: boolean") && chartLogicSource.includes("missing?: string[]"), diff --git a/frontend/components/dashboard/scan-terminal/__tests__/ssePatchArchitecture.test.ts b/frontend/components/dashboard/scan-terminal/__tests__/ssePatchArchitecture.test.ts index dabf6fc5..1350457b 100644 --- a/frontend/components/dashboard/scan-terminal/__tests__/ssePatchArchitecture.test.ts +++ b/frontend/components/dashboard/scan-terminal/__tests__/ssePatchArchitecture.test.ts @@ -198,8 +198,9 @@ export function runTests() { assert( foregroundRefreshBlock.includes("ignoreCache: true") && foregroundRefreshBlock.includes("fetchHourlyForecastForCity") && + foregroundRefreshBlock.includes("FOREGROUND_FULL_DETAIL_REFRESH_DEDUP_MS") && !foregroundRefreshBlock.includes("setIsHourlyLoading(true)"), - "foreground resume refresh should update full detail immediately in the background without showing the loading overlay", + "foreground resume refresh should update full detail in the background without showing the loading overlay or refetching fresh detail", ); assert( !chart.includes("/api/city/${encodeURIComponent(city)}/summary"), diff --git a/frontend/components/dashboard/scan-terminal/temperature-chart-logic.ts b/frontend/components/dashboard/scan-terminal/temperature-chart-logic.ts index a22d4df2..edd81b27 100644 --- a/frontend/components/dashboard/scan-terminal/temperature-chart-logic.ts +++ b/frontend/components/dashboard/scan-terminal/temperature-chart-logic.ts @@ -988,7 +988,7 @@ type CityDetailBatchQueue = { timer: ReturnType | null; }; -const CITY_DETAIL_BATCH_WINDOW_MS = 25; +const CITY_DETAIL_BATCH_WINDOW_MS = 100; const CITY_DETAIL_BATCH_MAX_CITIES = 12; const _cityDetailBatchQueues = new Map(); diff --git a/tests/test_web_observability.py b/tests/test_web_observability.py index 9e236694..417555c0 100644 --- a/tests/test_web_observability.py +++ b/tests/test_web_observability.py @@ -767,6 +767,109 @@ def test_city_detail_batch_returns_completed_details_when_one_city_is_slow(monke assert "slow" not in completed +def test_city_detail_batch_response_cache_keeps_entitlement_check(monkeypatch): + import asyncio + + entitlement_calls = 0 + build_calls = 0 + + async def build_batch_item(city, **kwargs): + nonlocal build_calls + build_calls += 1 + return city, { + "city": city, + "hourly": {"times": ["2026-05-30T00:00:00Z"], "temps": [20.0]}, + "resolution": kwargs.get("resolution"), + } + + def assert_entitlement(request): + nonlocal entitlement_calls + entitlement_calls += 1 + + city_api._CITY_DETAIL_BATCH_RESPONSE_CACHE.clear() + city_api._CITY_DETAIL_BATCH_RESPONSE_CACHE_TS.clear() + city_api._CITY_DETAIL_BATCH_RESPONSE_INFLIGHT.clear() + + monkeypatch.setenv("POLYWEATHER_CITY_DETAIL_BATCH_RESPONSE_CACHE_TTL_SEC", "20") + monkeypatch.setattr(city_api.legacy_routes, "_assert_entitlement", assert_entitlement) + monkeypatch.setattr(city_api.legacy_routes, "_normalize_city_or_404", lambda name: name.strip().lower()) + monkeypatch.setattr(city_api, "_build_city_detail_batch_item_async", build_batch_item) + + first = asyncio.run( + city_api.get_city_detail_batch_payload( + object(), + cities="Paris", + resolution="10m", + limit=12, + ) + ) + second = asyncio.run( + city_api.get_city_detail_batch_payload( + object(), + cities="Paris", + resolution="10m", + limit=12, + ) + ) + + assert first == second + assert first["details"]["paris"]["resolution"] == "10m" + assert entitlement_calls == 2 + assert build_calls == 1 + + +def test_concurrent_city_detail_batch_requests_share_inflight_response(monkeypatch): + import asyncio + + entitlement_calls = 0 + build_calls = 0 + + async def build_batch_item(city, **kwargs): + nonlocal build_calls + build_calls += 1 + await asyncio.sleep(0.02) + return city, { + "city": city, + "hourly": {"times": ["2026-05-30T00:00:00Z"], "temps": [20.0]}, + "resolution": kwargs.get("resolution"), + } + + def assert_entitlement(request): + nonlocal entitlement_calls + entitlement_calls += 1 + + city_api._CITY_DETAIL_BATCH_RESPONSE_CACHE.clear() + city_api._CITY_DETAIL_BATCH_RESPONSE_CACHE_TS.clear() + city_api._CITY_DETAIL_BATCH_RESPONSE_INFLIGHT.clear() + + monkeypatch.setenv("POLYWEATHER_CITY_DETAIL_BATCH_RESPONSE_CACHE_TTL_SEC", "20") + monkeypatch.setattr(city_api.legacy_routes, "_assert_entitlement", assert_entitlement) + monkeypatch.setattr(city_api.legacy_routes, "_normalize_city_or_404", lambda name: name.strip().lower()) + monkeypatch.setattr(city_api, "_build_city_detail_batch_item_async", build_batch_item) + + async def run_requests(): + return await asyncio.gather( + city_api.get_city_detail_batch_payload( + object(), + cities="Paris", + resolution="10m", + limit=12, + ), + city_api.get_city_detail_batch_payload( + object(), + cities="Paris", + resolution="10m", + limit=12, + ), + ) + + first, second = asyncio.run(run_requests()) + + assert first == second + assert entitlement_calls == 2 + assert build_calls == 1 + + def test_concurrent_city_detail_requests_share_same_full_cache_refresh(monkeypatch): import asyncio diff --git a/web/services/city_api.py b/web/services/city_api.py index 365f3f4b..57324faa 100644 --- a/web/services/city_api.py +++ b/web/services/city_api.py @@ -27,11 +27,16 @@ _CITY_FULL_REFRESH_INFLIGHT: Dict[str, "asyncio.Task[Dict[str, Any]]"] = {} _CITY_FULL_STALE_REFRESH_TASKS: Dict[str, "asyncio.Task[Dict[str, Any]]"] = {} _CITY_FULL_REFRESH_LOCK = asyncio.Lock() CityDetailPayloadCacheKey = Tuple[str, str, str, str, str, int] +CityDetailBatchResponseCacheKey = Tuple[Tuple[str, ...], bool, str, str, str] _CITY_DETAIL_PAYLOAD_CACHE: Dict[CityDetailPayloadCacheKey, Dict[str, Any]] = {} _CITY_DETAIL_PAYLOAD_CACHE_TS: Dict[CityDetailPayloadCacheKey, float] = {} _CITY_DETAIL_PAYLOAD_INFLIGHT: Dict[CityDetailPayloadCacheKey, "asyncio.Task[Dict[str, Any]]"] = {} _CITY_DETAIL_PAYLOAD_EPOCH: Dict[str, int] = {} _CITY_DETAIL_PAYLOAD_LOCK = asyncio.Lock() +_CITY_DETAIL_BATCH_RESPONSE_CACHE: Dict[CityDetailBatchResponseCacheKey, Dict[str, Any]] = {} +_CITY_DETAIL_BATCH_RESPONSE_CACHE_TS: Dict[CityDetailBatchResponseCacheKey, float] = {} +_CITY_DETAIL_BATCH_RESPONSE_INFLIGHT: Dict[CityDetailBatchResponseCacheKey, "asyncio.Task[Dict[str, Any]]"] = {} +_CITY_DETAIL_BATCH_RESPONSE_LOCK = asyncio.Lock() def _city_detail_payload_cache_ttl() -> float: @@ -42,6 +47,17 @@ def _city_detail_payload_cache_ttl() -> float: return max(0.0, min(30.0, value)) +def _city_detail_batch_response_cache_ttl() -> float: + try: + value = float( + os.getenv("POLYWEATHER_CITY_DETAIL_BATCH_RESPONSE_CACHE_TTL_SEC", "12") + or "12" + ) + except ValueError: + value = 12.0 + return max(0.0, min(30.0, value)) + + async def _overlay_cached_wunderground(city: str, payload: Dict[str, Any]) -> Dict[str, Any]: return await run_in_threadpool( legacy_routes._overlay_latest_wunderground_current, @@ -458,6 +474,23 @@ def _parse_batch_city_names(raw_cities: str, *, limit: int) -> List[str]: return out +def _city_detail_batch_response_cache_key( + city_names: List[str], + *, + force_refresh: bool, + market_slug: Optional[str], + target_date: Optional[str], + resolution: Optional[str], +) -> CityDetailBatchResponseCacheKey: + return ( + tuple(city_names), + bool(force_refresh), + str(market_slug or ""), + str(target_date or ""), + str(resolution or "10m"), + ) + + async def _build_city_detail_batch_item_async( city: str, *, @@ -549,61 +582,116 @@ async def get_city_detail_batch_payload( "partial": False, } - semaphore = asyncio.Semaphore(_city_detail_batch_concurrency()) + async def _build_uncached_payload() -> Dict[str, Any]: + semaphore = asyncio.Semaphore(_city_detail_batch_concurrency()) - async def _build_with_limit(city: str) -> Tuple[str, Dict[str, Any]]: - async with semaphore: - return await _build_city_detail_batch_item_async( - city, - force_refresh=force_refresh, - market_slug=market_slug, - target_date=target_date, - resolution=resolution, - timing_recorder=timer, - ) + async def _build_with_limit(city: str) -> Tuple[str, Dict[str, Any]]: + async with semaphore: + return await _build_city_detail_batch_item_async( + city, + force_refresh=force_refresh, + market_slug=market_slug, + target_date=target_date, + resolution=resolution, + timing_recorder=timer, + ) - task_by_city = { - city: asyncio.create_task(_build_with_limit(city)) - for city in city_names - } - task_city_lookup = {task: city for city, task in task_by_city.items()} - done, pending = await timer.measure_async( - "build_details", - lambda: asyncio.wait( - task_by_city.values(), - timeout=_city_detail_batch_partial_timeout_seconds(), - ), + task_by_city = { + city: asyncio.create_task(_build_with_limit(city)) + for city in city_names + } + task_city_lookup = {task: city for city, task in task_by_city.items()} + done, pending = await timer.measure_async( + "build_details", + lambda: asyncio.wait( + task_by_city.values(), + timeout=_city_detail_batch_partial_timeout_seconds(), + ), + ) + details: Dict[str, Any] = {} + errors: Dict[str, str] = {} + missing: List[str] = [] + for task in done: + city = task_city_lookup[task] + try: + result_city, payload = task.result() + except Exception as exc: + errors[city] = str(exc) + continue + details[result_city] = payload + + for task in pending: + city = task_city_lookup[task] + missing.append(city) + task.cancel() + + missing_set = set(missing) + missing = [city for city in city_names if city in missing_set] + return { + "cities": city_names, + "details": details, + "errors": errors, + "missing": missing, + "partial": bool(missing or errors), + } + + cache_ttl = _city_detail_batch_response_cache_ttl() + cache_key = _city_detail_batch_response_cache_key( + city_names, + force_refresh=force_refresh, + market_slug=market_slug, + target_date=target_date, + resolution=resolution, ) - details: Dict[str, Any] = {} - errors: Dict[str, str] = {} - missing: List[str] = [] - for task in done: - city = task_city_lookup[task] + if cache_ttl > 0 and not force_refresh: + now_ts = time.time() + async with _CITY_DETAIL_BATCH_RESPONSE_LOCK: + cached = _CITY_DETAIL_BATCH_RESPONSE_CACHE.get(cache_key) + cached_ts = _CITY_DETAIL_BATCH_RESPONSE_CACHE_TS.get(cache_key, 0.0) + if cached is not None and now_ts - cached_ts < cache_ttl: + outcome = "cache_hit" + return cached + task = _CITY_DETAIL_BATCH_RESPONSE_INFLIGHT.get(cache_key) + owner = False + if task is None: + owner = True + task = asyncio.create_task(_build_uncached_payload()) + _CITY_DETAIL_BATCH_RESPONSE_INFLIGHT[cache_key] = task + try: - result_city, payload = task.result() - except Exception as exc: - errors[city] = str(exc) - continue - details[result_city] = payload + payload = await timer.measure_async( + "build_or_wait_cached_batch", + lambda: task, + ) + finally: + if owner and task.done(): + async with _CITY_DETAIL_BATCH_RESPONSE_LOCK: + if _CITY_DETAIL_BATCH_RESPONSE_INFLIGHT.get(cache_key) is task: + _CITY_DETAIL_BATCH_RESPONSE_INFLIGHT.pop(cache_key, None) + if payload.get("partial"): + outcome = "partial" + elif not owner: + outcome = "shared_inflight" - for task in pending: - city = task_city_lookup[task] - missing.append(city) - task.cancel() + if owner: + async with _CITY_DETAIL_BATCH_RESPONSE_LOCK: + if not payload.get("partial"): + _CITY_DETAIL_BATCH_RESPONSE_CACHE[cache_key] = payload + _CITY_DETAIL_BATCH_RESPONSE_CACHE_TS[cache_key] = time.time() + if len(_CITY_DETAIL_BATCH_RESPONSE_CACHE) > 128: + oldest_keys = sorted( + _CITY_DETAIL_BATCH_RESPONSE_CACHE_TS, + key=lambda item: _CITY_DETAIL_BATCH_RESPONSE_CACHE_TS.get(item, 0.0), + )[:32] + for old_key in oldest_keys: + _CITY_DETAIL_BATCH_RESPONSE_CACHE.pop(old_key, None) + _CITY_DETAIL_BATCH_RESPONSE_CACHE_TS.pop(old_key, None) + return payload - missing_set = set(missing) - missing = [city for city in city_names if city in missing_set] - partial = bool(missing or errors) - if partial: + payload = await _build_uncached_payload() + if payload.get("partial"): outcome = "partial" - - return { - "cities": city_names, - "details": details, - "errors": errors, - "missing": missing, - "partial": partial, - } + return payload except HTTPException as exc: outcome = f"http_{exc.status_code}" status_code = exc.status_code