From 5e846866784e3c7f98c610b095c8788a574352c2 Mon Sep 17 00:00:00 2001 From: Frank Tovar Date: Sat, 7 Feb 2026 01:47:36 +0100 Subject: [PATCH] Serialize refresh scheduling and remove overlap polling --- REVIEW.md | 27 +++++------------ wwwroot/app.js | 79 +++++++++++++++++++++++++++++++++++++------------- 2 files changed, 66 insertions(+), 40 deletions(-) diff --git a/REVIEW.md b/REVIEW.md index 4775b1d..eb3f095 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -11,19 +11,15 @@ Active maintainability risks (priority order): - Cross-feature coupling still exists via shared mutable state usage across UI/data modules (`wwwroot/js/ui.js:180`, `wwwroot/js/ui.js:401`, `wwwroot/js/ui.js:622`, `wwwroot/js/data.js:82`). - Impact: high regression surface and expensive refactors even after removing global `window` bridges. -2. Frontend refresh scheduling can overlap async work (High) -- Refresh loop is still interval-driven in `wwwroot/app.js:290` with async work rooted in `wwwroot/js/data.js:82`. -- Impact: overlapping requests can race state updates and produce noisy UI transitions. - -3. Legacy phase path remains in active code (Medium) +2. Legacy phase path remains in active code (Medium) - `Reveal` is still present in `Domain/Phase.cs:6` and compatibility branches remain in `Endpoints/StateEndpoints.cs:102` and `Endpoints/StateEndpoints.cs:111`. - Impact: extra cognitive load and more branches to reason about during phase-flow changes. -4. Unauthenticated 401 response shape is still framework-driven (Medium) +3. Unauthenticated 401 response shape is still framework-driven (Medium) - Endpoint and filter unauthorized responses are standardized when app logic executes (`Infrastructure/AdminOnlyFilter.cs:15`, `Infrastructure/PhaseRequirementFilter.cs:15`, `Endpoints/SuggestEndpoints.cs:18`), but anonymous challenge responses remain middleware-controlled (`GameList.Tests/StateTests.cs:214`). - Impact: clients must tolerate both app-produced problem payloads and framework challenge responses. -5. Static analysis and frontend lint guardrails are still missing (Medium) +4. Static analysis and frontend lint guardrails are still missing (Medium) - CI currently gates restore/build/test only (`.github/workflows/ci.yml:23`-`.github/workflows/ci.yml:29`). - Impact: style drift and low-signal warnings can enter the codebase undetected. @@ -37,14 +33,6 @@ Active maintainability risks (priority order): - Effort / Risk: `L / Med`. - Dependencies (if any): none. -[P1] Replace uncontrolled polling with serialized refresh scheduling -- Problem: Severity `Medium`, Category `Reliability/Complexity`. Fixed 4-second polling can overlap when requests take longer than interval. -- Evidence: `wwwroot/app.js:290`, `wwwroot/js/data.js:82`. -- Recommendation: introduce a single-flight scheduler with backpressure, visibility pause/resume, and explicit trigger support. -- Acceptance criteria (testable): at most one in-flight refresh at a time; no duplicate refresh overlap during induced latency. -- Effort / Risk: `M / Low`. -- Dependencies (if any): none. - [P1] Remove legacy `Reveal` phase compatibility branches - Problem: Severity `Medium`, Category `Complexity`. Legacy phase compatibility logic is still present in runtime paths. - Evidence: `Domain/Phase.cs:6`, `Endpoints/StateEndpoints.cs:102`, `Endpoints/StateEndpoints.cs:111`, `wwwroot/js/data.js:30`. @@ -80,11 +68,10 @@ Active maintainability risks (priority order): ## C) Suggested execution order 1. Decompose `ui.js` by feature and keep orchestration thin. -2. Introduce serialized refresh scheduler. -3. Remove `Reveal` phase compatibility branches. -4. Normalize/declare unauthenticated 401 contract behavior. -5. Add analyzers + JS lint gates in CI. -6. Externalize i18n/FAQ assets. +2. Remove `Reveal` phase compatibility branches. +3. Normalize/declare unauthenticated 401 contract behavior. +4. Add analyzers + JS lint gates in CI. +5. Externalize i18n/FAQ assets. ## D) Guardrails diff --git a/wwwroot/app.js b/wwwroot/app.js index 8fd73f3..65684df 100644 --- a/wwwroot/app.js +++ b/wwwroot/app.js @@ -30,8 +30,55 @@ import { refreshPhaseData, } from "./js/data.js"; initI18n(); + +const REFRESH_INTERVAL_MS = 4000; +let refreshInFlight = null; +let refreshTimerId = null; +let refreshSchedulerStarted = false; + +async function runSerializedRefresh() { + if (refreshInFlight) return refreshInFlight; + refreshInFlight = refreshPhaseData().finally(() => { + refreshInFlight = null; + }); + return refreshInFlight; +} + +async function refreshWithUiErrorHandling() { + try { + await runSerializedRefresh(); + } catch (err) { + if (!handleAuthError(err, clearUserState)) toast(err.message, true); + } +} + +function scheduleNextRefresh() { + refreshTimerId = window.setTimeout(async () => { + if (!document.hidden) { + await refreshWithUiErrorHandling(); + } + scheduleNextRefresh(); + }, REFRESH_INTERVAL_MS); +} + +function startRefreshScheduler() { + if (refreshSchedulerStarted) return; + refreshSchedulerStarted = true; + + document.addEventListener("visibilitychange", () => { + if (!document.hidden) { + refreshWithUiErrorHandling(); + } + }); + + if (refreshTimerId !== null) { + window.clearTimeout(refreshTimerId); + } + scheduleNextRefresh(); +} + configureUiRuntime({ - refreshPhaseData, + refreshPhaseData: runSerializedRefresh, loadSuggestData, loadVoteData, handleAuthError: (err) => handleAuthError(err, clearUserState), @@ -105,7 +152,7 @@ function setupHandlers() { setSavedUsername(username); state.isAuthenticated = true; setAuthUI(true); - await refreshPhaseData(); + await runSerializedRefresh(); toast(t("toast.loggedIn")); } catch (err) { if (err?.status === 401) return toast(t("auth.invalidCredentials"), true); @@ -132,7 +179,7 @@ function setupHandlers() { setSavedUsername(username); state.isAuthenticated = true; setAuthUI(true); - await refreshPhaseData(); + await runSerializedRefresh(); toast(t("toast.registered")); } catch (err) { if (handleAuthError(err, clearUserState)) return; @@ -214,7 +261,7 @@ function setupHandlers() { } renderPhasePill(); toast(t("admin.resultsUpdated")); - await refreshPhaseData(); + await runSerializedRefresh(); } catch (err) { e.target.checked = !desired; toast(err.message, true); @@ -233,7 +280,7 @@ function setupHandlers() { try { await adminApi.linkSuggestions(source, target); toast(t("admin.linkDone")); - await refreshPhaseData(); + await runSerializedRefresh(); } catch (err) { toast(err.message, true); } @@ -250,7 +297,7 @@ function setupHandlers() { try { await adminApi.grantJoker(playerId); toast(t("admin.jokerGranted")); - await refreshPhaseData(); + await runSerializedRefresh(); } catch (err) { toast(err.message, true); } @@ -266,7 +313,7 @@ function setupHandlers() { await adminApi.deletePlayer(playerId); toast(t("admin.deleteDone")); close(); - await refreshPhaseData(); + await runSerializedRefresh(); } catch (err) { toast(err.message, true); } @@ -281,7 +328,7 @@ async function adminAction(fn, successMessage) { try { await fn(); toast(successMessage); - await refreshPhaseData(); + await runSerializedRefresh(); } catch (err) { toast(err.message, true); } @@ -289,16 +336,8 @@ async function adminAction(fn, successMessage) { async function main() { setupHandlers(); - try { - await refreshPhaseData(); - } catch (err) { - toast(err.message, true); - } - setInterval(() => { - refreshPhaseData().catch((err) => { - if (!handleAuthError(err, clearUserState)) toast(err.message, true); - }); - }, 4000); + await refreshWithUiErrorHandling(); + startRefreshScheduler(); } main(); @@ -358,7 +397,7 @@ function bindNavButtons() { state.resultsOpen = resp.resultsOpen ?? state.resultsOpen; state.votesRendered = false; renderPhasePill(); - await refreshPhaseData(); + await runSerializedRefresh(); } catch (err) { toast(err.message, true); } @@ -376,7 +415,7 @@ function bindNavButtons() { state.resultsOpen = resp.resultsOpen ?? state.resultsOpen; state.votesRendered = false; renderPhasePill(); - await refreshPhaseData(); + await runSerializedRefresh(); } catch (err) { toast(err.message, true); }