Skip to content

Commit 53310c5

Browse files
aarthy-dkclaude
andcommitted
fix(mcp): correct monitor forecast pagination, coupling parity, and notes (TG-1092)
Addresses review findings on the forecast work: - Render the forecast only on the first page — it is forward-looking and page-independent, but its anchor (the latest event) was read from the current page, producing a stale forecast on page >= 2. - Match the dashboard's coupled-forecast precondition: a freshness-coupled Volume/Metric monitor with no configured tolerance has no band on either surface (it was entering the coupled path on the coupling flag alone). - Resolve schedule holidays only after the next-update-window guards pass, restoring the dashboard's short-circuit (no calendar work for non-Prediction monitors); next_update_window now takes the test suite. - Exclude Error-status freshness events from the next-update anchor, matching the dashboard. - Use an accurate note when a forecast is absent for a non-training reason (elapsed window / no baseline) instead of telling a trained monitor to keep training. - Fix the dashboard unit test that imported the relocated forecast helper (this broke the Python Tests CI job), and add coverage for the coupled-without-tolerance case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent c9de0cc commit 53310c5

5 files changed

Lines changed: 66 additions & 32 deletions

File tree

testgen/common/monitor_forecast.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,7 @@ def next_update_window(
4141
freshness_definition: MonitorDefinition | None,
4242
last_detection_time: datetime | None,
4343
*,
44-
exclude_weekends: bool,
45-
holiday_dates: set[date] | None,
44+
test_suite: TestSuite,
4645
cron_tz: str | None,
4746
) -> dict | None:
4847
"""Predicted next-update window as ``{"start", "end"}`` epoch-ms, or ``None``.
@@ -63,6 +62,8 @@ def next_update_window(
6362
return None
6463

6564
tz = cron_tz or "UTC"
65+
exclude_weekends = test_suite.predict_exclude_weekends
66+
holiday_dates = resolve_suite_holiday_dates(test_suite)
6667
sched = get_schedule_params(freshness_definition.prediction)
6768

6869
window_end = add_business_minutes(

testgen/mcp/tools/monitors.py

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
forecast_band_points,
3232
gated_forecast_prediction,
3333
next_update_window,
34-
resolve_suite_holiday_dates,
3534
)
3635
from testgen.common.monitor_service import disable_monitoring, enable_monitoring, update_monitoring
3736
from testgen.mcp.exceptions import MCPUserError
@@ -61,6 +60,11 @@
6160
"have trained on enough history._"
6261
)
6362

63+
# Used where a forecast can be absent for reasons other than training (e.g. the
64+
# predicted next-update window has already passed, or no baseline is stored), so
65+
# the message must not claim the model still needs to train.
66+
_FORECAST_UNAVAILABLE_NOTE = "_No forecast available for this monitor right now._"
67+
6468
_MONITOR_LABEL: dict[MonitorType, str] = {
6569
MonitorType.FRESHNESS: "Freshness",
6670
MonitorType.VOLUME: "Volume",
@@ -623,7 +627,7 @@ def list_monitor_events(
623627
table_name: Table name exactly as stored in TestGen (case-sensitive).
624628
monitor_type: One of ``freshness`` / ``volume`` / ``schema`` / ``metric``.
625629
monitor_id: Required for ``monitor_type="metric"``, rejected for other types. Get it from ``list_monitors``.
626-
include_predictions: When True, append the monitor's forecast after the historical events, if available.
630+
include_predictions: When True, append the monitor's forecast, if available (shown on the first page only).
627631
limit: Page size (default 20, max 100).
628632
page: Page number starting at 1 (default 1).
629633
"""
@@ -716,7 +720,7 @@ def list_monitor_events(
716720
doc.text(f"No events on page {page} (total: {total}).")
717721
else:
718722
doc.text("_No monitor events in the active lookback window._")
719-
if include_predictions:
723+
if include_predictions and page == 1:
720724
_render_forecast_section(doc, _compute_forecast(suite, table_name, parsed_type, monitor_def, events))
721725
return doc.render()
722726

@@ -827,8 +831,11 @@ def _compute_forecast(
827831
# A monitor coupled to a Freshness monitor holds at its baseline until the
828832
# next expected refresh, so its band is the coupled baseline-then-refresh
829833
# shape keyed off the freshness next-update window — not the raw per-step
830-
# prediction series.
834+
# prediction series. The tolerance precondition mirrors the dashboard: a
835+
# coupled monitor with no configured tolerance has no band on either surface.
831836
if monitor_def.prediction and monitor_def.prediction.get("freshness_gated"):
837+
if monitor_def.lower_tolerance is None and monitor_def.upper_tolerance is None:
838+
return _Forecast(note=_FORECAST_UNAVAILABLE_NOTE)
832839
freshness_def = TestDefinition.get_singleton_monitor(
833840
suite.id, table_name, MonitorType.FRESHNESS.value
834841
)
@@ -838,7 +845,7 @@ def _compute_forecast(
838845
window = _next_update_window_for_table(suite, freshness_def, freshness_events)
839846
points = forecast_band_points(gated_forecast_prediction(monitor_def, window, last_run_time))
840847
if not points:
841-
return _Forecast(note=_FORECAST_PENDING_NOTE)
848+
return _Forecast(note=_FORECAST_UNAVAILABLE_NOTE)
842849
return _Forecast(points=points)
843850

844851
sensitivity = suite.predict_sensitivity.value if suite.predict_sensitivity is not None else "medium"
@@ -858,7 +865,7 @@ def _next_update_window_for_table(
858865
last_detection = max(
859866
(
860867
e.test_time for e in freshness_events
861-
if e.test_time is not None and not e.is_training and not e.is_pending
868+
if e.test_time is not None and not e.is_training and not e.is_pending and not e.is_error
862869
and _parse_freshness_message(e.message)[0] == "Yes"
863870
),
864871
default=None,
@@ -867,8 +874,7 @@ def _next_update_window_for_table(
867874
return next_update_window(
868875
freshness_def,
869876
last_detection,
870-
exclude_weekends=suite.predict_exclude_weekends,
871-
holiday_dates=resolve_suite_holiday_dates(suite),
877+
test_suite=suite,
872878
cron_tz=schedule.cron_tz if schedule else None,
873879
)
874880

testgen/ui/views/monitors_dashboard.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
from testgen.common.monitor_forecast import (
2222
gated_forecast_prediction,
2323
next_update_window,
24-
resolve_suite_holiday_dates,
2524
)
2625
from testgen.common.monitor_service import disable_monitoring, enable_monitoring, update_monitoring
2726
from testgen.ui.components import widgets as testgen
@@ -530,8 +529,7 @@ def _freshness_next_update_window(
530529
return next_update_window(
531530
freshness_definition,
532531
last_detection_time,
533-
exclude_weekends=test_suite.predict_exclude_weekends,
534-
holiday_dates=resolve_suite_holiday_dates(test_suite),
532+
test_suite=test_suite,
535533
cron_tz=monitor_schedule.cron_tz if monitor_schedule else None,
536534
)
537535

tests/unit/mcp/test_tools_monitors.py

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1011,13 +1011,12 @@ def test_list_monitor_events_predictions_not_applicable_for_schema(
10111011

10121012

10131013
@patch(f"{MODULE}.next_update_window")
1014-
@patch(f"{MODULE}.resolve_suite_holiday_dates")
10151014
@patch(f"{MODULE}.JobSchedule")
10161015
@patch(f"{MODULE}.TestDefinition")
10171016
@patch(f"{MODULE}.TestResult")
10181017
@patch(f"{MODULE}.resolve_monitored_table_group")
10191018
def test_list_monitor_events_freshness_prediction_shows_window(
1020-
mock_resolve, mock_tr_cls, mock_td_cls, mock_sched, mock_holidays, mock_window, db_session_mock,
1019+
mock_resolve, mock_tr_cls, mock_td_cls, mock_sched, mock_window, db_session_mock,
10211020
):
10221021
"""A Freshness monitor in Prediction Model mode forecasts a next-update time
10231022
window (the same one the dashboard computes), not a value band — so the
@@ -1031,7 +1030,6 @@ def test_list_monitor_events_freshness_prediction_shows_window(
10311030
monitor_def = MagicMock()
10321031
monitor_def.history_calculation = "PREDICT"
10331032
mock_td_cls.get_singleton_monitor.return_value = monitor_def
1034-
mock_holidays.return_value = None
10351033
mock_sched.get_for_monitor_suite.return_value = SimpleNamespace(cron_tz="UTC")
10361034
start_ms = int(datetime(2026, 7, 1, 9, 0, tzinfo=UTC).timestamp() * 1000)
10371035
end_ms = int(datetime(2026, 7, 1, 17, 0, tzinfo=UTC).timestamp() * 1000)
@@ -1050,13 +1048,12 @@ def test_list_monitor_events_freshness_prediction_shows_window(
10501048

10511049

10521050
@patch(f"{MODULE}.next_update_window")
1053-
@patch(f"{MODULE}.resolve_suite_holiday_dates")
10541051
@patch(f"{MODULE}.JobSchedule")
10551052
@patch(f"{MODULE}.TestDefinition")
10561053
@patch(f"{MODULE}.TestResult")
10571054
@patch(f"{MODULE}.resolve_monitored_table_group")
10581055
def test_list_monitor_events_freshness_coupled_volume_shows_gated_band(
1059-
mock_resolve, mock_tr_cls, mock_td_cls, mock_sched, mock_holidays, mock_window, db_session_mock,
1056+
mock_resolve, mock_tr_cls, mock_td_cls, mock_sched, mock_window, db_session_mock,
10601057
):
10611058
"""A Volume/Metric monitor coupled to a Freshness monitor holds at its
10621059
baseline until the next expected refresh — the forecast renders that coupled
@@ -1079,7 +1076,6 @@ def test_list_monitor_events_freshness_coupled_volume_shows_gated_band(
10791076
# list_monitor_events looks up the volume singleton; _compute_forecast then
10801077
# looks up the table's freshness definition for the window.
10811078
mock_td_cls.get_singleton_monitor.side_effect = [volume_def, freshness_def]
1082-
mock_holidays.return_value = None
10831079
mock_sched.get_for_monitor_suite.return_value = SimpleNamespace(cron_tz="UTC")
10841080
start_ms = int(datetime(2026, 6, 1, 18, 0, tzinfo=UTC).timestamp() * 1000)
10851081
mock_window.return_value = {"start": start_ms, "end": end_ms}
@@ -1097,6 +1093,39 @@ def test_list_monitor_events_freshness_coupled_volume_shows_gated_band(
10971093
assert "gated" not in out.lower()
10981094

10991095

1096+
@patch(f"{MODULE}.TestDefinition")
1097+
@patch(f"{MODULE}.TestResult")
1098+
@patch(f"{MODULE}.resolve_monitored_table_group")
1099+
def test_list_monitor_events_freshness_coupled_volume_without_tolerances_shows_note(
1100+
mock_resolve, mock_tr_cls, mock_td_cls, db_session_mock,
1101+
):
1102+
"""A coupled Volume/Metric monitor with no configured tolerance has no band on
1103+
the dashboard either, so the MCP shows a note (and skips the window queries),
1104+
rather than diverging by computing a band the UI never plots."""
1105+
tg = _mock_table_group()
1106+
mock_resolve.return_value = (tg, _mock_monitor_suite())
1107+
mock_tr_cls.list_monitor_events_for_table.return_value = ([_monitor_event()], 1)
1108+
volume_def = MagicMock()
1109+
volume_def.history_calculation = "PREDICT"
1110+
volume_def.prediction = {"freshness_gated": True, "baseline_value": 500.0}
1111+
volume_def.lower_tolerance = None
1112+
volume_def.upper_tolerance = None
1113+
mock_td_cls.get_singleton_monitor.return_value = volume_def
1114+
1115+
from testgen.mcp.tools.monitors import list_monitor_events
1116+
1117+
with _patch_perms():
1118+
out = list_monitor_events(str(tg.id), "orders", "volume", include_predictions=True)
1119+
1120+
assert "## Forecast" in out
1121+
assert "No forecast available for this monitor right now" in out
1122+
# No band, and no terminology leak.
1123+
assert "| Time | Predicted lower | Predicted upper |" not in out
1124+
assert "gated" not in out.lower()
1125+
# The freshness window queries are skipped when there's no tolerance to band.
1126+
assert mock_td_cls.get_singleton_monitor.call_count == 1 # only the volume singleton lookup
1127+
1128+
11001129
@patch(f"{MODULE}.resolve_monitored_table_group")
11011130
def test_list_monitor_events_not_monitored(mock_resolve, db_session_mock):
11021131
tg = _mock_table_group(monitor_test_suite_id=None)

tests/unit/ui/test_monitors_dashboard.py

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,14 @@
44
import pandas as pd
55
import pytest
66

7-
from testgen.ui.views.monitors_dashboard import (
8-
_build_gated_forecast_prediction,
9-
_freshness_next_update_window,
10-
)
7+
from testgen.common.monitor_forecast import gated_forecast_prediction
8+
from testgen.ui.views.monitors_dashboard import _freshness_next_update_window
119

1210
pytestmark = pytest.mark.unit
1311

14-
MODULE = "testgen.ui.views.monitors_dashboard"
12+
# The business-time helpers live in (and are called from) the shared forecast
13+
# module, so patch them there even when exercising the dashboard wrapper.
14+
MODULE = "testgen.common.monitor_forecast"
1515

1616

1717
def _freshness_def(history_calculation="PREDICT", prediction=None, upper="1000", lower="500"):
@@ -93,7 +93,7 @@ def test_window_start_is_none_when_no_lower_tolerance(mock_abm, _mock_sched):
9393
assert window["end"] == int(pd.Timestamp("2026-06-23 19:00").timestamp() * 1000)
9494

9595

96-
# --- _build_gated_forecast_prediction ---
96+
# --- gated_forecast_prediction ---
9797

9898
LAST_RUN = datetime(2026, 6, 23, 16, 0)
9999
NOW_MS = int(pd.Timestamp(LAST_RUN).timestamp() * 1000)
@@ -111,29 +111,29 @@ def _gated_def(baseline=1000.0, mean=None, lower="950", upper="1400"):
111111

112112

113113
def test_gated_prediction_none_without_window():
114-
assert _build_gated_forecast_prediction(_gated_def(), None, LAST_RUN) is None
114+
assert gated_forecast_prediction(_gated_def(), None, LAST_RUN) is None
115115

116116

117117
def test_gated_prediction_none_when_window_elapsed():
118118
window = {"start": NOW_MS - 12 * HOUR, "end": NOW_MS - HOUR} # window_end already in the past
119-
assert _build_gated_forecast_prediction(_gated_def(), window, LAST_RUN) is None
119+
assert gated_forecast_prediction(_gated_def(), window, LAST_RUN) is None
120120

121121

122122
def test_gated_prediction_none_without_baseline():
123123
window = {"start": NOW_MS - HOUR, "end": NOW_MS + 3 * HOUR}
124-
assert _build_gated_forecast_prediction(_gated_def(baseline=None), window, LAST_RUN) is None
124+
assert gated_forecast_prediction(_gated_def(baseline=None), window, LAST_RUN) is None
125125

126126

127127
def test_gated_prediction_none_without_last_run():
128128
window = {"start": NOW_MS - HOUR, "end": NOW_MS + 3 * HOUR}
129-
assert _build_gated_forecast_prediction(_gated_def(), window, None) is None
129+
assert gated_forecast_prediction(_gated_def(), window, None) is None
130130

131131

132132
def test_gated_prediction_anchors_at_now_when_window_started():
133133
# window_start is before the latest run → anchor clamps to now (forecast never draws backward)
134134
window = {"start": NOW_MS - 12 * HOUR, "end": NOW_MS + 3 * HOUR}
135135
mean = {str(NOW_MS + 3 * HOUR): 1200.0, str(NOW_MS + 27 * HOUR): 1300.0}
136-
result = _build_gated_forecast_prediction(_gated_def(mean=mean), window, LAST_RUN)
136+
result = gated_forecast_prediction(_gated_def(mean=mean), window, LAST_RUN)
137137
assert result["method"] == "predict"
138138
assert result["mean"] == {NOW_MS: 1000.0, NOW_MS + 3 * HOUR: 1200.0}
139139
# tolerances coerced from VARCHAR to float
@@ -145,11 +145,11 @@ def test_gated_prediction_anchors_at_window_start_when_future():
145145
# window opens after the latest run → flat segment runs out to window_start
146146
window = {"start": NOW_MS + HOUR, "end": NOW_MS + 3 * HOUR}
147147
mean = {str(NOW_MS + 3 * HOUR): 1200.0}
148-
result = _build_gated_forecast_prediction(_gated_def(mean=mean), window, LAST_RUN)
148+
result = gated_forecast_prediction(_gated_def(mean=mean), window, LAST_RUN)
149149
assert set(result["mean"].keys()) == {NOW_MS + HOUR, NOW_MS + 3 * HOUR}
150150

151151

152152
def test_gated_prediction_next_mean_falls_back_to_baseline_without_forecast():
153153
window = {"start": NOW_MS - HOUR, "end": NOW_MS + 3 * HOUR}
154-
result = _build_gated_forecast_prediction(_gated_def(mean=None), window, LAST_RUN)
154+
result = gated_forecast_prediction(_gated_def(mean=None), window, LAST_RUN)
155155
assert result["mean"][NOW_MS + 3 * HOUR] == 1000.0 # baseline used as the step value

0 commit comments

Comments
 (0)