Skip to content

Commit c9de0cc

Browse files
aarthy-dkclaude
andcommitted
fix(mcp): align monitor L2 forecasts with the UI and harden the read tools (TG-1092)
Review follow-ups on the monitor L2 read tools: - list_monitor_events: validate the metric monitor_id as a UUID and scope the lookup to the suite + table + Metric_Trend, so a monitor_id belonging to a metric on another table or suite can't be rendered under the wrong heading. - Forecasts now mirror the dashboard exactly. The forecast logic (predicted next-update window, coupled baseline-then-refresh band) moved into the UI-agnostic common.monitor_forecast module shared by the dashboard and the MCP tool. Volume/Metric monitors coupled to a Freshness monitor render the same band the UI plots; Freshness renders its predicted next-update window. No internal coupling terminology is exposed. - Threshold-mode labels are a ThresholdMode StrEnum; DataStructureLog .list_for_table_group takes *clauses like its siblings (the dashboard keeps its exclusive lower-bound schema-change window). - Type the forecast renderer, fix stale docstrings, and make the forecast-pending note accurate (a forecast appears once the Prediction Model has trained on enough history, not after the first run). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 78d1c29 commit c9de0cc

8 files changed

Lines changed: 521 additions & 199 deletions

File tree

testgen/common/models/data_structure_log.py

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
from dataclasses import dataclass
2-
from datetime import date, datetime
2+
from datetime import datetime
33
from uuid import UUID, uuid4
44

55
from sqlalchemy import Column, String, asc, desc, select
@@ -54,19 +54,17 @@ class DataStructureLog(Entity):
5454
def list_for_table_group(
5555
cls,
5656
table_group_id: str | UUID,
57-
*,
58-
table_name: str | None = None,
59-
since: date | datetime | None = None,
60-
until: date | datetime | None = None,
57+
*clauses,
6158
page: int = 1,
6259
limit: int | None = 20,
6360
) -> tuple[list[DataStructureLogEntry], int]:
6461
"""Paginated schema-change audit log for one table group, newest first.
6562
66-
Filters by ``table_name`` (exact match; the audit log stores names
67-
case-sensitively as the source emitted them), ``since`` (lower-bound on
68-
``change_date``), and ``until`` (upper-bound). ``limit=None`` skips
69-
pagination — the caller gets every matching row in one shot.
63+
Caller-supplied ``*clauses`` are WHERE expressions on this model — e.g.
64+
``cls.table_name == name`` (exact match; the audit log stores names
65+
case-sensitively as the source emitted them) or a ``cls.change_date``
66+
bound. ``limit=None`` skips pagination — the caller gets every matching
67+
row in one shot.
7068
"""
7169
query = select(
7270
cls.log_id,
@@ -77,13 +75,7 @@ def list_for_table_group(
7775
cls.change,
7876
cls.old_data_type,
7977
cls.new_data_type,
80-
).where(cls.table_groups_id == table_group_id)
81-
if table_name is not None:
82-
query = query.where(cls.table_name == table_name)
83-
if since is not None:
84-
query = query.where(cls.change_date >= since)
85-
if until is not None:
86-
query = query.where(cls.change_date <= until)
78+
).where(cls.table_groups_id == table_group_id, *clauses)
8779
query = query.order_by(*cls._default_order_by)
8880

8981
if limit is None:

testgen/common/models/test_definition.py

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -271,12 +271,13 @@ class TestDefinitionMinimal(EntityMinimal):
271271
test_name_short: str
272272

273273

274-
# Threshold-mode labels for monitor TestDefinitions — derived from which fields
275-
# on the definition are populated. See ``TestDefinition._derive_threshold_mode``.
276-
THRESHOLD_MODE_PREDICTION = "Prediction Model"
277-
THRESHOLD_MODE_HISTORICAL = "Historical Calculation"
278-
THRESHOLD_MODE_STATIC = "Static"
279-
THRESHOLD_MODE_NONE = "N/A"
274+
class ThresholdMode(StrEnum):
275+
"""How a monitor's bounds are determined — derived from which fields on the
276+
definition are populated. See ``TestDefinition._derive_threshold_mode``."""
277+
PREDICTION = "Prediction Model"
278+
HISTORICAL = "Historical Calculation"
279+
STATIC = "Static"
280+
NONE = "N/A"
280281

281282

282283
@dataclass
@@ -355,7 +356,7 @@ class MonitorConfig(EntityMinimal):
355356
test_type: str
356357
table_name: str
357358
metric_name: str | None
358-
threshold_mode: str
359+
threshold_mode: ThresholdMode
359360
threshold_lower: str | None
360361
threshold_upper: str | None
361362
custom_query: str | None
@@ -706,7 +707,7 @@ def _build_monitor_config(cls, td: "TestDefinition") -> MonitorConfig:
706707
@classmethod
707708
def _derive_threshold_mode(
708709
cls, td: "TestDefinition",
709-
) -> tuple[str, str | None, str | None]:
710+
) -> tuple[ThresholdMode, str | None, str | None]:
710711
"""Pick a mode and the bounds tuple that applies under that mode.
711712
712713
Detection mirrors the UI form (``test_definition_form.js``): a
@@ -728,14 +729,14 @@ def _derive_threshold_mode(
728729
* Static (Volume / Metric): ``(lower_tolerance, upper_tolerance)``.
729730
"""
730731
if td.test_type == MonitorType.SCHEMA.value:
731-
return THRESHOLD_MODE_NONE, None, None
732+
return ThresholdMode.NONE, None, None
732733
if td.history_calculation == "PREDICT":
733-
return THRESHOLD_MODE_PREDICTION, None, None
734+
return ThresholdMode.PREDICTION, None, None
734735
if td.history_calculation and td.test_type != MonitorType.FRESHNESS.value:
735-
return THRESHOLD_MODE_HISTORICAL, td.history_calculation, td.history_calculation_upper
736+
return ThresholdMode.HISTORICAL, td.history_calculation, td.history_calculation_upper
736737
if td.test_type == MonitorType.FRESHNESS.value:
737-
return THRESHOLD_MODE_STATIC, None, td.upper_tolerance
738-
return THRESHOLD_MODE_STATIC, td.lower_tolerance, td.upper_tolerance
738+
return ThresholdMode.STATIC, None, td.upper_tolerance
739+
return ThresholdMode.STATIC, td.lower_tolerance, td.upper_tolerance
739740

740741
@classmethod
741742
def select_page(

testgen/common/models/test_result.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ class MonitorEvent:
149149
Pending rows have no underlying ``test_results`` row — ``monitor_id``,
150150
``test_time``, and result flags are ``None``; ``is_pending`` is True.
151151
Forecast points (future timestamps with predicted bounds) are NOT
152-
events and surface separately via ``TestDefinition.get_forecast_points``.
152+
events and surface separately via ``forecast_points_from_prediction``.
153153
"""
154154
monitor_id: UUID | None
155155
test_type: str
@@ -675,7 +675,7 @@ def list_monitor_events_for_table(
675675
676676
Forecast points for Prediction-Model monitors are NOT included here —
677677
events are only past, observed runs. Read forecasts separately via
678-
``TestDefinition.get_forecast_points(sensitivity)``.
678+
``forecast_points_from_prediction(prediction, sensitivity)``.
679679
"""
680680
monitor_codes = (
681681
[monitor_type] if monitor_type is not None

testgen/common/monitor_forecast.py

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
"""Monitor forecast computation shared by the monitors dashboard and the MCP
2+
monitor tools.
3+
4+
The forecast a monitor displays depends on its threshold mode and type:
5+
6+
* Volume / Metric in Prediction Model mode: a value band extending forward. For
7+
monitors coupled to a Freshness monitor the band holds at the baseline until
8+
the next expected refresh, then steps to the forecast value (``gated_forecast_prediction``);
9+
otherwise it is the raw per-step prediction band.
10+
* Freshness in Prediction Model mode: a predicted next-update *time window*
11+
rather than a value band (``next_update_window``).
12+
13+
These functions are intentionally free of any UI dependency so both surfaces
14+
render the same forecast from the same source of truth.
15+
"""
16+
17+
from datetime import UTC, date, datetime
18+
19+
import pandas as pd
20+
21+
from testgen.common.freshness_service import add_business_minutes, get_schedule_params, resolve_holiday_dates
22+
from testgen.common.models.test_definition import MonitorForecastPoint, TestDefinition, TestDefinitionSummary
23+
from testgen.common.models.test_suite import TestSuite
24+
25+
# A monitor definition carrying the forecast-relevant fields — satisfied by both
26+
# the ORM row and the read-only summary dataclass.
27+
MonitorDefinition = TestDefinition | TestDefinitionSummary
28+
29+
30+
def resolve_suite_holiday_dates(test_suite: TestSuite) -> set[date] | None:
31+
"""Holiday dates excluded from the suite's schedule, over a window spanning
32+
recent history through the forecast horizon. ``None`` when no calendars are set."""
33+
if not test_suite.holiday_codes_list:
34+
return None
35+
now = pd.Timestamp.now("UTC")
36+
idx = pd.DatetimeIndex([now - pd.Timedelta(days=7), now + pd.Timedelta(days=30)])
37+
return resolve_holiday_dates(test_suite.holiday_codes_list, idx)
38+
39+
40+
def next_update_window(
41+
freshness_definition: MonitorDefinition | None,
42+
last_detection_time: datetime | None,
43+
*,
44+
exclude_weekends: bool,
45+
holiday_dates: set[date] | None,
46+
cron_tz: str | None,
47+
) -> dict | None:
48+
"""Predicted next-update window as ``{"start", "end"}`` epoch-ms, or ``None``.
49+
50+
The schedule-derived business-time interval from the last detected update out
51+
to the lower/upper staleness tolerance. ``start`` is ``None`` when only an
52+
upper tolerance is configured (the update is expected by ``end``). Returns
53+
``None`` unless the Freshness monitor is in Prediction Model mode with a
54+
trained schedule and at least one observed update.
55+
"""
56+
if (
57+
freshness_definition is None
58+
or freshness_definition.history_calculation != "PREDICT"
59+
or (freshness_definition.prediction and not freshness_definition.prediction.get("schedule_stage"))
60+
or freshness_definition.upper_tolerance is None
61+
or last_detection_time is None
62+
):
63+
return None
64+
65+
tz = cron_tz or "UTC"
66+
sched = get_schedule_params(freshness_definition.prediction)
67+
68+
window_end = add_business_minutes(
69+
pd.Timestamp(last_detection_time),
70+
float(freshness_definition.upper_tolerance),
71+
exclude_weekends,
72+
holiday_dates, tz,
73+
excluded_days=sched.excluded_days,
74+
)
75+
window_start = None
76+
if lower_minutes := (float(freshness_definition.lower_tolerance) if freshness_definition.lower_tolerance else None):
77+
window_start = add_business_minutes(
78+
pd.Timestamp(last_detection_time),
79+
lower_minutes,
80+
exclude_weekends,
81+
holiday_dates, tz,
82+
excluded_days=sched.excluded_days,
83+
)
84+
85+
return {
86+
"start": int(window_start.timestamp() * 1000) if window_start else None,
87+
"end": int(window_end.timestamp() * 1000),
88+
}
89+
90+
91+
def gated_forecast_prediction(
92+
definition: MonitorDefinition,
93+
freshness_window: dict | None,
94+
last_run_time: datetime | None,
95+
) -> dict | None:
96+
"""Coupled forecast band for a Volume/Metric monitor whose value holds at its
97+
baseline between refreshes.
98+
99+
A flat baseline line from the latest run up to the predicted next-update
100+
window, then a step to the forecast's next-refresh value with the band
101+
opening to its tolerance. The anchor is never earlier than the latest run, so
102+
the forecast extends forward rather than back over history. Returns ``None``
103+
when there is no usable forward window — no predicted window, the window has
104+
already elapsed, or no stored baseline — keyed as ``lower_tolerance`` /
105+
``upper_tolerance`` (no sensitivity suffix); read with ``forecast_band_points``.
106+
"""
107+
baseline = definition.prediction.get("baseline_value") if definition.prediction else None
108+
now_ms = int(pd.Timestamp(last_run_time).timestamp() * 1000) if last_run_time is not None else None
109+
if freshness_window is None or now_ms is None or baseline is None:
110+
return None
111+
window_end = freshness_window.get("end")
112+
if window_end is None or window_end <= now_ms:
113+
return None
114+
115+
forecast_means = (definition.prediction.get("mean") if definition.prediction else None) or {}
116+
next_refresh_mean = forecast_means[min(forecast_means, key=lambda k: int(k))] if forecast_means else baseline
117+
flat_anchor = max(freshness_window.get("start") or now_ms, now_ms)
118+
# lower/upper_tolerance are VARCHAR columns — coerce to float so the band dicts are numerically
119+
# typed throughout (baseline is already a float).
120+
lower_tol = float(definition.lower_tolerance) if definition.lower_tolerance is not None else None
121+
upper_tol = float(definition.upper_tolerance) if definition.upper_tolerance is not None else None
122+
return {
123+
"method": "predict",
124+
"mean": {flat_anchor: baseline, window_end: next_refresh_mean},
125+
"lower_tolerance": {flat_anchor: baseline, window_end: lower_tol},
126+
"upper_tolerance": {flat_anchor: baseline, window_end: upper_tol},
127+
}
128+
129+
130+
def forecast_band_points(prediction: dict | None) -> list[MonitorForecastPoint]:
131+
"""Convert a forecast band dict with plain ``lower_tolerance`` / ``upper_tolerance``
132+
epoch-ms series (as built by ``gated_forecast_prediction``) into time-ordered
133+
forecast points. For the raw per-sensitivity prediction JSONB use
134+
``forecast_points_from_prediction`` instead."""
135+
if not prediction:
136+
return []
137+
lower_series = prediction.get("lower_tolerance") or {}
138+
upper_series = prediction.get("upper_tolerance") or {}
139+
keys = sorted(set(lower_series) | set(upper_series), key=lambda k: int(k))
140+
points: list[MonitorForecastPoint] = []
141+
for k in keys:
142+
ts = datetime.fromtimestamp(int(k) / 1000.0, UTC)
143+
lower = lower_series.get(k)
144+
upper = upper_series.get(k)
145+
points.append(MonitorForecastPoint(
146+
test_time=ts,
147+
lower_bound=float(lower) if lower is not None else None,
148+
upper_bound=float(upper) if upper is not None else None,
149+
))
150+
return points

0 commit comments

Comments
 (0)