diff --git a/frontend/app/api/cities/detail-batch/route.ts b/frontend/app/api/cities/detail-batch/route.ts index a7d8d01d..e795ee68 100644 --- a/frontend/app/api/cities/detail-batch/route.ts +++ b/frontend/app/api/cities/detail-batch/route.ts @@ -42,8 +42,13 @@ export async function GET(req: NextRequest) { try { return await proxyBackendJsonGet(req, { cacheControl: cachePolicy.responseCacheControl, - fetchCache: - cachePolicy.fetchMode === "no-store" ? "no-store" : undefined, + cacheControlForData: (data) => + data && + typeof data === "object" && + (data as { partial?: unknown }).partial === true + ? "no-store, max-age=0" + : cachePolicy.responseCacheControl, + fetchCache: "no-store", publicMessage: "Failed to fetch city detail batch", revalidateSeconds: cachePolicy.revalidateSeconds, signal: controller.signal, diff --git a/frontend/components/dashboard/scan-terminal/__tests__/refreshCadencePolicy.test.ts b/frontend/components/dashboard/scan-terminal/__tests__/refreshCadencePolicy.test.ts index d17b05de..a331a2ce 100644 --- a/frontend/components/dashboard/scan-terminal/__tests__/refreshCadencePolicy.test.ts +++ b/frontend/components/dashboard/scan-terminal/__tests__/refreshCadencePolicy.test.ts @@ -96,6 +96,18 @@ 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("partial?: boolean") && + chartLogicSource.includes("missing?: string[]"), + "frontend city detail batch payload should understand partial responses and missing city markers", + ); + const flushCityDetailBatchBlock = chartLogicSource.match(/async function flushCityDetailBatch[\s\S]*?\r?\n}\r?\n\r?\nfunction fetchCityDetailBatchWithTimeout/)?.[0] || ""; + assert( + flushCityDetailBatchBlock.includes("partialMissingCities") && + flushCityDetailBatchBlock.includes("resolveBatchWaiters(waiters, null)") && + flushCityDetailBatchBlock.includes("payload?.partial === true"), + "partial detail-batch misses should resolve without immediately issuing single-city fallback requests", + ); const fetchHourlyBlock = chartLogicSource.match(/async function fetchHourlyForecastForCity[\s\S]*?\r?\n}\r?\n\r?\nfunction fetchCityDetailWithTimeout/)?.[0] || ""; assert( fetchHourlyBlock.includes("queueCityDetailBatch(city, resParam)") && diff --git a/frontend/components/dashboard/scan-terminal/temperature-chart-logic.ts b/frontend/components/dashboard/scan-terminal/temperature-chart-logic.ts index 39917085..26547366 100644 --- a/frontend/components/dashboard/scan-terminal/temperature-chart-logic.ts +++ b/frontend/components/dashboard/scan-terminal/temperature-chart-logic.ts @@ -966,8 +966,11 @@ type HourlyForecastFetchOptions = { }; type CityDetailBatchPayload = { + cities?: string[]; details?: Record; errors?: Record; + missing?: string[]; + partial?: boolean; }; type CityDetailBatchWaiter = { @@ -1112,6 +1115,10 @@ async function flushCityDetailBatch(resolution: string) { try { const payload = await fetchCityDetailBatchWithTimeout(cities, resolution); const details = payload?.details || {}; + const partialMissingCities = + payload?.partial === true + ? new Set((payload.missing || []).map((city) => normalizeCityKey(city))) + : new Set(); await Promise.all( cities.map(async (city) => { const waiters = queue.waiters.get(city); @@ -1121,6 +1128,10 @@ async function flushCityDetailBatch(resolution: string) { resolveBatchWaiters(waiters, data); return; } + if (partialMissingCities.has(normalizeCityKey(city))) { + resolveBatchWaiters(waiters, null); + return; + } try { resolveBatchWaiters( waiters, diff --git a/frontend/components/ops/__tests__/apiPerformanceTiming.test.ts b/frontend/components/ops/__tests__/apiPerformanceTiming.test.ts index 967f2a45..23ac7683 100644 --- a/frontend/components/ops/__tests__/apiPerformanceTiming.test.ts +++ b/frontend/components/ops/__tests__/apiPerformanceTiming.test.ts @@ -41,6 +41,21 @@ export function runTests() { const detailBatchProxy = readFrontend("app", "api", "cities", "detail-batch", "route.ts"); assert.match(detailBatchProxy, /createProxyTimer\(req,\s*"city_detail_batch"\)/); assert.match(detailBatchProxy, /timing:\s*timer/); + assert.match( + detailBatchProxy, + /fetchCache:\s*"no-store"/, + "city detail batch proxy should avoid caching partial backend fetches in the Next data cache", + ); + assert.match( + detailBatchProxy, + /cacheControlForData/, + "city detail batch proxy should be able to suppress response caching for partial payloads", + ); + assert.match( + apiProxySource, + /cacheControlForData\?:/, + "generic backend JSON proxy should allow response cache policy to depend on parsed data", + ); const scanTerminalProxy = readFrontend("app", "api", "scan", "terminal", "route.ts"); assert.match(scanTerminalProxy, /createProxyTimer\(req,\s*"scan_terminal"\)/); diff --git a/frontend/lib/api-proxy.ts b/frontend/lib/api-proxy.ts index fd3288ce..c0b09ac9 100644 --- a/frontend/lib/api-proxy.ts +++ b/frontend/lib/api-proxy.ts @@ -85,6 +85,7 @@ export async function proxyBackendJsonGet( req: NextRequest, options: { cacheControl?: string; + cacheControlForData?: (data: unknown) => string | undefined; conditionalResponse?: boolean; detailLimit?: number; error?: string; @@ -148,12 +149,14 @@ export async function proxyBackendJsonGet( const data = await (timing ? timing.measure("backend_read", () => res.json()) : res.json()); + const responseCacheControl = + options.cacheControlForData?.(data) ?? options.cacheControl; const response = - options.cacheControl && options.conditionalResponse !== false - ? buildCachedJsonResponse(req, data, options.cacheControl) + responseCacheControl && options.conditionalResponse !== false + ? buildCachedJsonResponse(req, data, responseCacheControl) : NextResponse.json(data, { - headers: options.cacheControl - ? { "Cache-Control": options.cacheControl } + headers: responseCacheControl + ? { "Cache-Control": responseCacheControl } : undefined, }); const withCookies = applyAuthResponseCookies(response, auth.response); diff --git a/tests/test_web_observability.py b/tests/test_web_observability.py index 1753dcf1..af6c682d 100644 --- a/tests/test_web_observability.py +++ b/tests/test_web_observability.py @@ -618,6 +618,44 @@ def test_city_detail_batch_endpoint_limits_backend_concurrency(monkeypatch): assert max_active <= 2 +def test_city_detail_batch_returns_completed_details_when_one_city_is_slow(monkeypatch): + import asyncio + + completed = [] + + async def build_batch_item(city, **kwargs): + if city == "slow": + await asyncio.sleep(0.08) + completed.append(city) + return city, { + "city": city, + "hourly": {"times": ["2026-05-30T00:00:00Z"], "temps": [20.0]}, + "resolution": kwargs.get("resolution"), + } + + monkeypatch.setenv("POLYWEATHER_CITY_DETAIL_BATCH_PARTIAL_TIMEOUT_MS", "20") + monkeypatch.setattr(city_api.legacy_routes, "_assert_entitlement", lambda request: None) + 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) + + payload = asyncio.run( + city_api.get_city_detail_batch_payload( + object(), + cities="fast,slow,other", + resolution="10m", + limit=3, + ) + ) + + assert payload["cities"] == ["fast", "slow", "other"] + assert sorted(payload["details"]) == ["fast", "other"] + assert payload["details"]["fast"]["resolution"] == "10m" + assert payload["partial"] is True + assert payload["missing"] == ["slow"] + assert payload["errors"] == {} + assert "slow" not in completed + + 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 c497cd2e..365f3f4b 100644 --- a/web/services/city_api.py +++ b/web/services/city_api.py @@ -500,6 +500,19 @@ def _city_detail_batch_concurrency() -> int: return max(1, min(6, value)) +def _city_detail_batch_partial_timeout_seconds() -> Optional[float]: + try: + timeout_ms = int( + os.getenv("POLYWEATHER_CITY_DETAIL_BATCH_PARTIAL_TIMEOUT_MS", "8500") + or "8500" + ) + except ValueError: + timeout_ms = 8500 + if timeout_ms <= 0: + return None + return max(0.001, min(60.0, timeout_ms / 1000.0)) + + async def get_city_detail_batch_payload( request: Request, *, @@ -528,7 +541,13 @@ async def get_city_detail_batch_payload( ), ) if not city_names: - return {"cities": [], "details": {}, "errors": {}} + return { + "cities": [], + "details": {}, + "errors": {}, + "missing": [], + "partial": False, + } semaphore = asyncio.Semaphore(_city_detail_batch_concurrency()) @@ -543,27 +562,47 @@ async def get_city_detail_batch_payload( timing_recorder=timer, ) - tasks = [ - _build_with_limit(city) + task_by_city = { + city: asyncio.create_task(_build_with_limit(city)) for city in city_names - ] - results = await timer.measure_async( + } + task_city_lookup = {task: city for city, task in task_by_city.items()} + done, pending = await timer.measure_async( "build_details", - lambda: asyncio.gather(*tasks, return_exceptions=True), + lambda: asyncio.wait( + task_by_city.values(), + timeout=_city_detail_batch_partial_timeout_seconds(), + ), ) details: Dict[str, Any] = {} errors: Dict[str, str] = {} - for city, result in zip(city_names, results): - if isinstance(result, Exception): - errors[city] = str(result) + 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 - result_city, payload = result 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] + partial = bool(missing or errors) + if partial: + outcome = "partial" + return { "cities": city_names, "details": details, "errors": errors, + "missing": missing, + "partial": partial, } except HTTPException as exc: outcome = f"http_{exc.status_code}"