diff --git a/dashboard/routes/batch.py b/dashboard/routes/batch.py index 1452e9e..72fc94d 100644 --- a/dashboard/routes/batch.py +++ b/dashboard/routes/batch.py @@ -101,20 +101,27 @@ def get(batch_name: str, chart_index: int): Returns: Card: The chart. """ - df = app.state.df_cache[batch_name] - metric_stats = app.state.stats_cache[batch_name] + # Read each cache once into a local. A concurrent toggle or refresh can + # clear these shared dicts mid-request, so rebuild on a miss instead of + # indexing the dict a second time. + df = app.state.df_cache.get(batch_name) + metric_stats = app.state.stats_cache.get(batch_name) + if df is None or metric_stats is None: + df = get_batch_data(batch_name) + app.state.df_cache[batch_name] = df + metric_stats = app.state.calculate_metric_stats(batch_name, df=df) + metric_name = metric_stats[chart_index]["metric_name"] anomaly_rate = metric_stats[chart_index]["anomaly_rate"] avg_score = metric_stats[chart_index]["avg_score"] - if batch_name not in app.state.chart_cache: - app.state.chart_cache[batch_name] = {} - - if chart_index not in app.state.chart_cache[batch_name]: + batch_charts = app.state.chart_cache.setdefault(batch_name, {}) + fig = batch_charts.get(chart_index) + if fig is None: df_metric = df[df["metric_name"] == metric_name] df_metric = extract_metadata(df_metric, "anomaly_explanation") fig = ChartManager.create_chart(df_metric, chart_index) - app.state.chart_cache[batch_name][chart_index] = fig + batch_charts[chart_index] = fig modal_id = f"modal-{batch_name}-{chart_index}" @@ -170,7 +177,7 @@ def get(batch_name: str, chart_index: int): } """ ), - Safe(app.state.chart_cache[batch_name][chart_index]), + Safe(fig), # Expand button overlay positioned on entire card Button( UkIcon("expand", height=16, width=16), diff --git a/dashboard/state.py b/dashboard/state.py index 3c1b4c1..02f1661 100644 --- a/dashboard/state.py +++ b/dashboard/state.py @@ -171,11 +171,16 @@ def clear_batch_cache(self, batch_name): self.chart_cache.pop(batch_name, None) self.stats_cache.pop(batch_name, None) - def calculate_metric_stats(self, batch_name): + def calculate_metric_stats(self, batch_name, df=None): """ Calculate the metric stats for a given batch name. + + Reads the batch dataframe from the cache when ``df`` is not supplied. + Returns the metric stats so a caller can keep a local reference that a + concurrent cache clear cannot remove. """ - df = self.df_cache[batch_name] + if df is None: + df = self.df_cache[batch_name] metric_stats = [] for metric_name in df["metric_name"].unique(): df_metric = df[df["metric_name"] == metric_name] @@ -198,3 +203,4 @@ def calculate_metric_stats(self, batch_name): ) metric_stats.sort(key=lambda x: (-x["anomaly_rate"], -x["avg_score"])) self.stats_cache[batch_name] = metric_stats + return metric_stats diff --git a/tests/test_dashboard.py b/tests/test_dashboard.py index 33dda9e..95275fa 100644 --- a/tests/test_dashboard.py +++ b/tests/test_dashboard.py @@ -129,3 +129,63 @@ def test_search_empty_and_invalid_patterns(): assert "Invalid search pattern" in to_xml(metric_results(state, "demo")) state.search_term["demo"] = "" assert "/chart/0" in to_xml(metric_results(state, "demo")) + + +def test_calculate_metric_stats_accepts_df_and_returns_stats(): + state = AppState() + df = pd.DataFrame( + { + "metric_name": ["m1", "m1"], + "metric_alert": [1, 0], + "metric_score": [0.5, 0.7], + "thumbsup_sum": [1, 2], + "thumbsdown_sum": [0, 1], + } + ) + # No df_cache entry: an explicit df must still produce and return the stats. + stats = state.calculate_metric_stats("b1", df=df) + assert stats == state.stats_cache["b1"] + assert stats[0]["metric_name"] == "m1" + + +def test_chart_route_survives_cache_clear_during_render(monkeypatch): + """A toggle or refresh clearing the caches mid-request must not 500.""" + import logging + + from fasthtml.common import to_xml + + # The batch route imports ``log`` and ``rt`` from dashboard.app; the stubbed + # module only supplies ``app``, so provide passthrough versions here. + stub_app.log = logging.getLogger("test") + stub_app.rt = lambda *args, **kwargs: lambda func: func + + from dashboard.routes import batch as batch_route + + state = AppState() + batch = "demo" + df = pd.DataFrame( + { + "metric_name": ["m1", "m1"], + "metric_alert": [1, 0], + "metric_score": [0.5, 0.7], + "thumbsup_sum": [1, 2], + "thumbsdown_sum": [0, 1], + } + ) + state.df_cache[batch] = df + state.calculate_metric_stats(batch) + monkeypatch.setattr(batch_route.app, "state", state, raising=False) + monkeypatch.setattr(batch_route, "extract_metadata", lambda frame, column: frame) + + def clear_everything_then_render(df_metric, chart_index): + # Emulate a concurrent toggle/refresh wiping the shared caches while + # this request is still building its chart. + state.df_cache.clear() + state.stats_cache.clear() + state.chart_cache.clear() + return "