refactor(mcp): collapse monitor dashboard helpers and close two unit-test gaps

Perf — uncached dashboard render now runs the per-table monitor CTE 3 times
instead of 5 (review feedback #4). ``get_monitor_changes_by_tables`` already
had the total in hand from ``list_monitor_table_summaries`` and was throwing
it away; ``count_monitor_changes_by_tables`` then re-ran the full CTE
(rows + COUNT) just to recover that total. Helper now returns
``(rows, total)`` directly and the count helper is gone. Render code
unpacks the tuple.

Tests:
- ``test_get_monitor_summary_lookback_out_of_range`` is parametrized over
  the lower bound, upper bound, beyond-upper, and a negative — was only
  pinning ``lookback=0`` before, so the 1-365 ceiling was untested.
- New ``test_parse_monitor_table_sort_rejects_legacy_row_count_desc``
  asserts the legacy ``row_count_desc`` name is rejected and the error
  surfaces the renamed ``row_count_change_desc`` — guards against an
  accidental revert of the column-rename signal that the dashboard cell
  shows a delta, not the raw current count.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Author: diogodk

testgen/ui/views/monitors_dashboard.py

@@ -133,7 +133,7 @@ def render(
                     if sort_field and sort_field not in ALLOWED_SORT_FIELDS:
                         sort_field = None
 
-                    monitored_tables_page = get_monitor_changes_by_tables(
+                    monitored_tables_page, all_monitored_tables_count = get_monitor_changes_by_tables(
                         table_group_id,
                         table_name_filter=table_name_filter,
                         anomaly_type_filter=anomaly_type_filter,
@@ -142,11 +142,6 @@ def render(
                         limit=int(items_per_page),
                         offset=page_start,
                     )
-                    all_monitored_tables_count = count_monitor_changes_by_tables(
-                        table_group_id,
-                        table_name_filter=table_name_filter,
-                        anomaly_type_filter=anomaly_type_filter,
-                    )
                     monitor_changes_summary = summarize_monitor_changes(table_group_id)
 
                 monitored_table_names = {table["table_name"] for table in monitored_tables_page}
@@ -358,39 +353,27 @@ def get_monitor_changes_by_tables(
     sort_order: Literal["asc"] | Literal["desc"] | None = None,
     limit: int | None = None,
     offset: int | None = None,
-) -> list[dict]:
+) -> tuple[list[dict], int]:
     """Per-monitored-table summaries shaped for the dashboard's JSON payload.
 
-    Returns plain dicts (rather than ``MonitorTableSummary`` dataclasses) because the
-    monitor-dashboard widget consumes the payload via ``make_json_safe``. Each row is
-    augmented with ``table_group_id`` to match the historical payload shape.
+    Returns ``(rows, total)`` so the dashboard can fill its pager without an extra
+    round-trip to the model (which would re-run the heavy CTE twice — once for the
+    rows, once for the count — just to throw the rows away). Rows are dicts (rather
+    than ``MonitorTableSummary`` dataclasses) because the monitor-dashboard widget
+    consumes the payload via ``make_json_safe``. Each row is augmented with
+    ``table_group_id`` to match the historical payload shape.
     """
     page = 1 + (offset // limit) if limit and offset else 1
-    summaries, _total = TableGroup.list_monitor_table_summaries(
+    summaries, total = TableGroup.list_monitor_table_summaries(
         table_group_id,
         anomaly_types=_dashboard_anomaly_types(anomaly_type_filter),
         sort_by=_dashboard_sort_to_model(sort_field, sort_order),
         table_name_filter=table_name_filter,
         page=page,
         limit=limit or 1000,
     )
-    return [{**dataclasses.asdict(s), "table_group_id": table_group_id} for s in summaries]
-
-
-@st.cache_data(show_spinner=False)
-def count_monitor_changes_by_tables(
-    table_group_id: str,
-    table_name_filter: str | None = None,
-    anomaly_type_filter: list[str] | None = None,
-) -> int:
-    _items, total = TableGroup.list_monitor_table_summaries(
-        table_group_id,
-        anomaly_types=_dashboard_anomaly_types(anomaly_type_filter),
-        table_name_filter=table_name_filter,
-        page=1,
-        limit=1,
-    )
-    return total
+    rows = [{**dataclasses.asdict(s), "table_group_id": table_group_id} for s in summaries]
+    return rows, total
 
 
 @st.cache_data(show_spinner=False)

tests/unit/mcp/test_tools_common.py

@@ -902,6 +902,18 @@ def test_parse_monitor_table_sort_rejects_unknown():
         assert valid in msg
 
 
+def test_parse_monitor_table_sort_rejects_legacy_row_count_desc():
+    """Guard against the pre-review-feedback ``row_count_desc`` name accidentally
+    coming back: the rename to ``row_count_change_desc`` is the canonical signal
+    that the column shows a delta, not the raw current count. Drop this only when
+    introducing a deliberate replacement."""
+    from testgen.mcp.tools.common import parse_monitor_table_sort
+
+    with pytest.raises(MCPUserError, match="Invalid sort_by") as exc:
+        parse_monitor_table_sort("row_count_desc")
+    assert "row_count_change_desc" in str(exc.value)
+
+
 def test_resolve_monitored_table_group_returns_suite():
     from testgen.common.models.table_group import TableGroup
     from testgen.common.models.test_suite import TestSuite

tests/unit/mcp/test_tools_monitors.py

@@ -196,12 +196,15 @@ def test_get_monitor_summary_lookback_override_applied(mock_resolve, mock_tg_cls
     assert "**Lookback:** 14 runs (override)" in out
 
 
-def test_get_monitor_summary_lookback_out_of_range(db_session_mock):
+@pytest.mark.parametrize("bad_lookback", [0, 366, 500, -1])
+def test_get_monitor_summary_lookback_out_of_range(bad_lookback, db_session_mock):
+    """Both bounds (and beyond) are pinned — the model accepts 1..365 inclusive."""
     from testgen.mcp.tools.monitors import get_monitor_summary
 
     with _patch_perms(), pytest.raises(MCPUserError) as exc:
-        get_monitor_summary(str(uuid4()), lookback=0)
+        get_monitor_summary(str(uuid4()), lookback=bad_lookback)
     assert "between 1 and 365" in str(exc.value)
+    assert f"`{bad_lookback}`" in str(exc.value)
 
 
 @patch(f"{MODULE}.next_scheduled_run", return_value=None)