refactor(mcp): apply TG-1053 review feedback

Correctness:
- Project.get_summary: exclude is_monitor suites from test_suite_count /
  test_definition_count / test_run_count subqueries (the rule applied per
  CLAUDE.md monitor-exclusion).
- TableGroup._list_with_activity: separate COUNT(*) so total is correct on
  out-of-range pages (window function over an empty page returned 0).
- TestSuite.test_definition_stats: render user-facing test_name_short labels
  via LEFT JOIN to test_types instead of leaking raw test_type codes.

Output shape:
- list_table_groups: expose Columns + Rows + all 3 quality scores (profiling,
  testing, overall), column header is "Quality Score" (overall).
- get_table_group: drop dev-facing first sentence; field labels via the
  _DIFF_LABELS map for parity with create/update tools; Latest activity now
  includes the calculated overall Quality Score via a new
  TableGroup.quality_score property.
- get_connection: rendering consolidated through a new _render_connection_body
  helper shared with _render_created_connection.
- get_project: restructured into UI-mirroring sections (Configuration /
  Observability Integration / Data Retention) with UI-matching labels.

Defense in depth:
- get_test_suite / get_table_group route Connection / TableGroup lookups
  through resolve_connection / resolve_table_group instead of Model.get().
- format_flavor_label helper centralizes the SQL_FLAVOR_CODE_TO_LABEL +
  fallback pattern used across the read tools.

Tests:
- get_connection redaction test uses a real Connection with EncryptedBytea
  fields populated; previously vacuous (mocks bypassed the columns entirely).
- get_test_suite monitor-rejection test now drives an actual is_monitor=True
  suite through resolve_test_suite's filter (was only exercising the missing-
  entity path).
- Updated mocks and assertions for the new label/field set.

Docs:
- mcp-patterns.md followable-IDs table updated with the new producers /
  consumers introduced by this MR.

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

Author: diogodk

testgen/common/models/project.py

@@ -66,19 +66,23 @@ def get_summary(cls, project_code: str) -> ProjectSummary | None:
                 WHERE table_groups.project_code = :project_code
             ) AS profiling_run_count,
             (
-                SELECT COUNT(*) FROM test_suites WHERE test_suites.project_code = :project_code
+                SELECT COUNT(*) FROM test_suites
+                WHERE test_suites.project_code = :project_code
+                  AND test_suites.is_monitor IS NOT TRUE
             ) AS test_suite_count,
             (
                 SELECT COUNT(*)
                 FROM test_definitions
                     LEFT JOIN test_suites ON test_definitions.test_suite_id = test_suites.id
                 WHERE test_suites.project_code = :project_code
+                  AND test_suites.is_monitor IS NOT TRUE
             ) AS test_definition_count,
             (
                 SELECT COUNT(*)
                 FROM test_runs
                     LEFT JOIN test_suites ON test_runs.test_suite_id = test_suites.id
                 WHERE test_suites.project_code = :project_code
+                  AND test_suites.is_monitor IS NOT TRUE
             ) AS test_run_count,
             (
                 SELECT COALESCE(observability_api_key, '') <> ''

testgen/common/models/table_group.py

@@ -55,9 +55,13 @@ class TableGroupListItem(EntityMinimal):
     project_code: str
     connection_name: str | None
     table_count: int
+    column_count: int | None
+    row_count: int | None
     last_profiled_date: datetime | None
     last_tested_date: datetime | None
+    profiling_score: float | None
     testing_score: float | None
+    quality_score: float | None
 
 
 @dataclass
@@ -162,6 +166,20 @@ class TableGroup(Entity):
         dq_score_testing,
     )
 
+    @property
+    def quality_score(self) -> float:
+        """Overall quality score per ``utils.score``: profiling * testing when both
+        exist; the non-null one when only one ran; ``0.0`` when neither has run.
+
+        Mirrors what Project Dashboard, data_catalog, and inventory_service display.
+        Render through ``utils.friendly_score`` to get the user-facing percentage form
+        (``"95.0"`` rather than ``"0.95"``). The list-side equivalent is computed in
+        SQL inside ``_list_with_activity`` — keep both in sync if the formula changes.
+        """
+        from testgen.utils import score
+
+        return score(self.dq_score_profiling, self.dq_score_testing)
+
     @classmethod
     def get_minimal(cls, id_: str | UUID) -> TableGroupMinimal | None:
         result = cls._get_columns(id_, cls._minimal_columns)
@@ -431,10 +449,22 @@ def _list_with_activity(
         page: int,
         limit: int,
     ) -> tuple[list[TableGroupListItem], int]:
+        session = get_current_session()
+
+        # Separate COUNT(*) query — keeps `total` correct on out-of-range pages where
+        # the page rows are empty (a window function over zero rows would return 0).
+        total_query = f"SELECT COUNT(*) FROM table_groups AS groups WHERE {scope_sql};"
+        total = session.execute(text(total_query), scope_params).scalar() or 0
+        if total == 0:
+            return [], 0
+
         params: dict = {**scope_params, "limit": limit, "offset": (page - 1) * limit}
-        query = f"""
+        rows_query = f"""
         WITH stats AS (
-            SELECT table_groups_id, COUNT(*) AS table_count
+            SELECT table_groups_id,
+                COUNT(*) AS table_count,
+                SUM(column_ct) AS column_count,
+                SUM(COALESCE(record_ct, approx_record_ct)) AS row_count
             FROM data_table_chars
             GROUP BY table_groups_id
         ),
@@ -458,10 +488,18 @@ def _list_with_activity(
             groups.project_code,
             connections.connection_name,
             COALESCE(stats.table_count, 0) AS table_count,
+            stats.column_count,
+            stats.row_count,
             latest_profile.started_at AS last_profiled_date,
             latest_test.test_starttime AS last_tested_date,
+            groups.dq_score_profiling AS profiling_score,
             groups.dq_score_testing AS testing_score,
-            COUNT(*) OVER() AS total_count
+            -- Mirrors utils.score: product when both exist, else the non-null one, else NULL.
+            CASE
+                WHEN groups.dq_score_profiling IS NOT NULL AND groups.dq_score_testing IS NOT NULL
+                    THEN groups.dq_score_profiling * groups.dq_score_testing
+                ELSE COALESCE(groups.dq_score_profiling, groups.dq_score_testing)
+            END AS quality_score
         FROM table_groups AS groups
             LEFT JOIN connections ON connections.connection_id = groups.connection_id
             LEFT JOIN stats ON stats.table_groups_id = groups.id
@@ -471,14 +509,8 @@ def _list_with_activity(
         ORDER BY LOWER(groups.table_groups_name)
         LIMIT :limit OFFSET :offset;
         """
-        rows = get_current_session().execute(text(query), params).mappings().all()
-        if not rows:
-            return [], 0
-        total = int(rows[0]["total_count"])
-        items = [
-            TableGroupListItem(**{k: v for k, v in row.items() if k != "total_count"})
-            for row in rows
-        ]
+        rows = session.execute(text(rows_query), params).mappings().all()
+        items = [TableGroupListItem(**row) for row in rows]
         return items, total
 
     @classmethod

testgen/common/models/test_suite.py

@@ -224,24 +224,30 @@ def select_summary(cls, project_code: str, table_group_id: str | UUID | None = N
 
     @classmethod
     def test_definition_stats(cls, test_suite_id: str | UUID) -> "TestDefinitionStats":
-        """Aggregate test-definition counts for a suite: total, locked, and per-test-type."""
+        """Aggregate test-definition counts for a suite: total, locked, and per-test-type.
+
+        The per-type bucket uses the user-facing ``test_name_short`` label from
+        ``test_types``, falling back to the raw ``test_type`` code when the lookup
+        row is missing (defensive — every shipping test type has a short name).
+        """
         query = """
         SELECT
-            test_type,
+            COALESCE(tt.test_name_short, td.test_type) AS type_label,
             COUNT(*) AS total,
-            SUM(CASE WHEN COALESCE(lock_refresh, 'N') = 'Y' THEN 1 ELSE 0 END) AS locked
-        FROM test_definitions
-        WHERE test_suite_id = :test_suite_id
-        GROUP BY test_type
-        ORDER BY test_type;
+            SUM(CASE WHEN COALESCE(td.lock_refresh, 'N') = 'Y' THEN 1 ELSE 0 END) AS locked
+        FROM test_definitions td
+            LEFT JOIN test_types tt ON tt.test_type = td.test_type
+        WHERE td.test_suite_id = :test_suite_id
+        GROUP BY type_label
+        ORDER BY type_label;
         """
         rows = (
             get_current_session()
             .execute(text(query), {"test_suite_id": str(test_suite_id)})
             .mappings()
             .all()
         )
-        counts_by_type = {row["test_type"]: int(row["total"]) for row in rows}
+        counts_by_type = {row["type_label"]: int(row["total"]) for row in rows}
         total = sum(counts_by_type.values())
         locked = sum(int(row["locked"]) for row in rows)
         return TestDefinitionStats(total=total, locked=locked, counts_by_type=counts_by_type)

testgen/mcp/tools/common.py

@@ -874,6 +874,18 @@ def parse_sql_flavor(value: str) -> tuple[SqlFlavorLabel, str, str]:
     return label, code, SQL_FLAVOR_CODE_TO_FAMILY[code]
 
 
+def format_flavor_label(sql_flavor_code: str | None) -> str:
+    """Map a stored ``sql_flavor_code`` to its user-facing display label.
+
+    Returns the raw code as a fallback when the code is not in the registry — defensive
+    against a never-shipping-but-still-stored value rather than letting an LLM see ``None``.
+    """
+    if sql_flavor_code is None:
+        return ""
+    label = SQL_FLAVOR_CODE_TO_LABEL.get(sql_flavor_code)
+    return label.value if label else sql_flavor_code
+
+
 # ===========================================================================
 # Connection-parameter contract (MCP input vocabulary)
 #

testgen/mcp/tools/connections.py

@@ -26,6 +26,7 @@
     apply_connection_params,
     connection_display_fields,
     connection_field_labels,
+    format_flavor_label,
     format_page_footer,
     format_page_info,
     infer_mode,
@@ -69,13 +70,11 @@ def list_connections(project_code: str, page: int = 1, limit: int = 20) -> str:
     doc.text(format_page_info(total, page, limit))
     table_rows: list[list[object]] = []
     for row in rows:
-        label = SQL_FLAVOR_CODE_TO_LABEL.get(row.sql_flavor_code)
-        database_type = label.value if label else row.sql_flavor_code
         table_rows.append(
             [
                 row.connection_id,
                 row.connection_name,
-                database_type,
+                format_flavor_label(row.sql_flavor_code),
                 row.project_host,
                 row.project_db,
                 row.table_group_count,
@@ -100,38 +99,12 @@ def get_connection(connection_id: int) -> str:
     Use this before editing a connection or creating a table group on it.
 
     Args:
-        connection_id: Bigint connection ID returned by `list_connections` or shown on the connections page.
+        connection_id: Bigint connection ID returned by `list_connections`.
     """
     connection = resolve_connection(connection_id)
-
     doc = MdDoc()
     doc.heading(1, f"Connection `{connection.connection_name}`")
-    doc.field("ID", connection.connection_id, code=True)
-    doc.field("Project", connection.project_code, code=True)
-    label = SQL_FLAVOR_CODE_TO_LABEL.get(connection.sql_flavor_code)
-    doc.field("Type", label.value if label else connection.sql_flavor_code)
-
-    if connection.project_host:
-        doc.field("Host", connection.project_host, code=True)
-    if connection.project_port:
-        doc.field("Port", connection.project_port)
-    if connection.project_db:
-        doc.field("Database", connection.project_db, code=True)
-    if connection.project_user:
-        doc.field("User", connection.project_user, code=True)
-    if connection.connect_by_url and connection.url:
-        doc.field("URL", connection.url, code=True)
-    if connection.warehouse:
-        doc.field("Warehouse", connection.warehouse, code=True)
-    if connection.http_path:
-        doc.field("HTTP Path", connection.http_path, code=True)
-    doc.field("Authentication", _authentication_label(connection))
-
-    if connection.max_threads is not None:
-        doc.field("Max Threads", connection.max_threads)
-    if connection.max_query_chars is not None:
-        doc.field("Max Expression Length", connection.max_query_chars)
-
+    _render_connection_body(doc, connection)
     return doc.render()
 
 
@@ -236,12 +209,22 @@ def _raise_validation_error(errors: list[str], header: str) -> None:
 def _render_created_connection(connection: Connection) -> str:
     doc = MdDoc()
     doc.heading(1, f"Connection `{connection.connection_name}` created")
+    _render_connection_body(doc, connection)
+    return doc.render()
+
+
+def _render_connection_body(doc: MdDoc, connection: Connection) -> None:
+    """Render every non-secret connection field below the heading.
+
+    Shared by ``create_connection`` (the response after a successful create) and
+    ``get_connection`` (read tool) so the surfaced field set stays consistent.
+    Encrypted columns are filtered out via ``ConnField.secret``.
+    """
     doc.field("ID", connection.connection_id, code=True)
     doc.field("Project", connection.project_code, code=True)
-    label = SQL_FLAVOR_CODE_TO_LABEL.get(connection.sql_flavor_code)
-    doc.field("Type", label.value if label else connection.sql_flavor_code)
+    doc.field("Type", format_flavor_label(connection.sql_flavor_code))
 
-    # Render each populated, non-secret field under its flavor-specific label
+    # Each populated, non-secret field under its flavor-specific label
     # (e.g. "Catalog" for Databricks, "Login URL" for Salesforce).
     for fld in connection_display_fields(connection):
         if fld.secret:
@@ -257,8 +240,6 @@ def _render_created_connection(connection: Connection) -> str:
     if connection.max_query_chars is not None:
         doc.field("Max Expression Length", connection.max_query_chars)
 
-    return doc.render()
-
 
 def _authentication_label(connection: Connection) -> str:
     """The connection's auth method: the active connection mode for multi-mode

testgen/mcp/tools/discovery.py

@@ -1,16 +1,15 @@
 from testgen.common.mixpanel_service import MixpanelService
 from testgen.common.models import with_database_session
-from testgen.common.models.connection import Connection
 from testgen.common.models.data_table import DataTable
 from testgen.common.models.project import Project
-from testgen.common.models.table_group import TableGroup
 from testgen.common.models.test_run import TestRun
 from testgen.common.models.test_suite import TestSuite
 from testgen.mcp.exceptions import MCPResourceNotAccessible
 from testgen.mcp.permissions import get_project_permissions, mcp_permission
 from testgen.mcp.tools.common import (
-    SQL_FLAVOR_CODE_TO_LABEL,
     DocGroup,
+    format_flavor_label,
+    resolve_connection,
     resolve_table_group,
     resolve_test_suite,
     validate_limit,
@@ -93,13 +92,17 @@ def get_project(project_code: str) -> str:
     doc.field("Test runs", summary.test_run_count)
 
     doc.heading(2, "Configuration")
-    doc.field("Quality-score weighting", project.use_dq_score_weights)
-    doc.field("Data retention", project.data_retention_enabled)
-    if project.data_retention_enabled and project.data_retention_days is not None:
-        doc.field("Retention period (days)", project.data_retention_days)
-    doc.field("Observability export configured", summary.can_export_to_observability)
+    doc.field("Weighted data quality scoring", project.use_dq_score_weights)
+
+    doc.heading(2, "Observability Integration")
+    doc.field("Configured", summary.can_export_to_observability)
     if project.observability_api_url:
-        doc.field("Observability API URL", project.observability_api_url, code=True)
+        doc.field("API URL", project.observability_api_url, code=True)
+
+    doc.heading(2, "Data Retention")
+    doc.field("Automatically delete old profiling and test history", project.data_retention_enabled)
+    if project.data_retention_enabled and project.data_retention_days is not None:
+        doc.field("Delete history older than (days)", project.data_retention_days)
 
     return doc.render()
 
@@ -159,18 +162,21 @@ def list_test_suites(project_code: str) -> str:
 @with_database_session
 @mcp_permission("view")
 def get_test_suite(test_suite_id: str) -> str:
-    """Get a test suite's configuration: default severity, connection, table group, and per-test-type counts.
+    """Get a test suite's configuration: connection, table group, default severity, and per-test-type counts.
 
     Returns the test suite's identity and configuration along with a breakdown of how many test
-    definitions it contains by type and how many are lock-refreshed (excluded from regeneration).
+    definitions it contains by type and how many are locked (excluded from regeneration).
     Use this before changing a suite's tests to understand what will be affected.
 
     Args:
         test_suite_id: The test suite UUID, e.g. from `list_test_suites`.
     """
     suite = resolve_test_suite(test_suite_id)
-    connection = Connection.get(suite.connection_id) if suite.connection_id else None
-    table_group = TableGroup.get(suite.table_groups_id) if suite.table_groups_id else None
+    # Defense in depth: resolve via perm-filtered helpers rather than `Model.get(...)`.
+    # FK constraints guarantee same-project today; the resolvers are the established wrapper
+    # for project-scoped lookups and keep us aligned if those guarantees ever change.
+    connection = resolve_connection(suite.connection_id) if suite.connection_id else None
+    table_group = resolve_table_group(str(suite.table_groups_id)) if suite.table_groups_id else None
     stats = TestSuite.test_definition_stats(suite.id)
 
     doc = MdDoc()
@@ -179,11 +185,9 @@ def get_test_suite(test_suite_id: str) -> str:
     doc.field("Project", suite.project_code, code=True)
 
     if connection is not None:
-        label = SQL_FLAVOR_CODE_TO_LABEL.get(connection.sql_flavor_code)
-        flavor_label = label.value if label else connection.sql_flavor_code
         doc.field(
             "Connection",
-            f"{connection.connection_name} (`{connection.connection_id}`, {flavor_label})",
+            f"{connection.connection_name} (`{connection.connection_id}`, {format_flavor_label(connection.sql_flavor_code)})",
         )
     if table_group is not None:
         doc.field(
@@ -201,8 +205,8 @@ def get_test_suite(test_suite_id: str) -> str:
 
     if stats.counts_by_type:
         doc.heading(2, "Tests by type")
-        rows = [[test_type, count] for test_type, count in stats.counts_by_type.items()]
-        doc.table(["Test type", "Count"], rows, code=[0])
+        rows = [[type_label, count] for type_label, count in stats.counts_by_type.items()]
+        doc.table(["Test", "Count"], rows)
 
     return doc.render()