fix: github cicd support uncategorized snapshots (#1678)

* fix: github cicd skip downstream uncategorized snapshots

* fix: github cicd support uncategorized snapshots

Author: eakmanrq

sqlmesh/core/plan/definition.py

@@ -272,19 +272,32 @@ def restatements(self) -> t.Dict[str, Interval]:
         return self._restatements
 
     @property
-    def loaded_snapshot_intervals(self) -> t.List[LoadedSnapshotIntervals]:
+    def loaded_snapshot_intervals(
+        self,
+    ) -> t.Tuple[t.List[LoadedSnapshotIntervals], t.List[Snapshot]]:
         loaded_snapshots = []
+        unloaded_snapshots = []
         for snapshot in self.directly_modified:
-            if not snapshot.change_category:
-                continue
-            loaded_snapshots.append(LoadedSnapshotIntervals.from_snapshot(snapshot))
+            if snapshot.change_category:
+                loaded_snapshots.append(LoadedSnapshotIntervals.from_snapshot(snapshot))
+            else:
+                logger.debug(f"Got an unloaded snapshot. Snapshot: {snapshot.name}")
+                unloaded_snapshots.append(snapshot)
             for downstream_indirect in self.indirectly_modified.get(snapshot.name, set()):
                 downstream_snapshot = self._snapshot_mapping[downstream_indirect]
                 # We don't want to display indirect non-breaking since to users these are effectively no-op changes
                 if downstream_snapshot.is_indirect_non_breaking:
                     continue
-                loaded_snapshots.append(LoadedSnapshotIntervals.from_snapshot(downstream_snapshot))
-        return loaded_snapshots
+                if downstream_snapshot.change_category:
+                    loaded_snapshots.append(
+                        LoadedSnapshotIntervals.from_snapshot(downstream_snapshot)
+                    )
+                else:
+                    logger.debug(
+                        f"Got an unloaded indirectly-modified snapshot. Snapshot: {downstream_snapshot.name}"
+                    )
+                    unloaded_snapshots.append(downstream_snapshot)
+        return loaded_snapshots, unloaded_snapshots
 
     @property
     def has_changes(self) -> bool:

sqlmesh/integrations/github/cicd/controller.py

@@ -22,6 +22,7 @@
 from sqlmesh.core.context import Context
 from sqlmesh.core.environment import Environment
 from sqlmesh.core.plan import LoadedSnapshotIntervals, Plan
+from sqlmesh.core.snapshot.definition import Snapshot
 from sqlmesh.core.user import User
 from sqlmesh.integrations.github.cicd.config import GithubCICDBotConfig
 from sqlmesh.utils.concurrency import NodeExecutionFailedError
@@ -547,7 +548,9 @@ def try_invalidate_pr_environment(self) -> None:
         if self.bot_config.invalidate_environment_after_deploy:
             self._context.invalidate_environment(self.pr_environment_name)
 
-    def get_loaded_snapshot_intervals(self) -> t.List[LoadedSnapshotIntervals]:
+    def get_loaded_snapshot_intervals(
+        self,
+    ) -> t.Tuple[t.List[LoadedSnapshotIntervals], t.List[Snapshot]]:
         return self.prod_plan_with_gaps.loaded_snapshot_intervals
 
     def _update_check(
@@ -729,8 +732,8 @@ def conclusion_handler(
             conclusion: GithubCheckConclusion, exception: t.Optional[Exception]
         ) -> t.Tuple[GithubCheckConclusion, str, t.Optional[str]]:
             if conclusion.is_success:
-                pr_affected_models = self.get_loaded_snapshot_intervals()
-                if not pr_affected_models:
+                loaded_snapshots, unloaded_snapshots = self.get_loaded_snapshot_intervals()
+                if not loaded_snapshots and not unloaded_snapshots:
                     summary = "No models were modified in this PR.\n"
                 else:
                     header_rows = [
@@ -742,16 +745,24 @@ def conclusion_handler(
                         ],
                     ]
                     body_rows: List[Element | List[Element]] = []
-                    for affected_model in pr_affected_models:
+                    for loaded_snapshot in loaded_snapshots:
                         model_rows = [
-                            h("td", affected_model.node_name),
-                            h("td", affected_model.change_category_str),
+                            h("td", loaded_snapshot.node_name),
+                            h("td", loaded_snapshot.change_category_str),
                         ]
-                        if affected_model.intervals:
-                            model_rows.append(h("td", affected_model.format_intervals()))
+                        if loaded_snapshot.intervals:
+                            model_rows.append(h("td", loaded_snapshot.format_intervals()))
                         else:
                             model_rows.append(h("td", "N/A"))
                         body_rows.append(model_rows)
+                    for unloaded_snapshot in unloaded_snapshots:
+                        body_rows.append(
+                            [
+                                h("td", unloaded_snapshot.name),
+                                h("td", "Uncategorized"),
+                                h("td", "N/A"),
+                            ]
+                        )
                     table_header = h("thead", [h("tr", row) for row in header_rows])
                     table_body = h("tbody", [h("tr", row) for row in body_rows])
                     summary = str(h("table", [table_header, table_body]))

tests/integrations/github/cicd/test_github_controller.py

@@ -1,14 +1,22 @@
 # type: ignore
-from unittest.mock import call
+from unittest.mock import PropertyMock, call
 
 import pytest
 from pytest_mock.plugin import MockerFixture
 
 from sqlmesh.core import constants as c
 from sqlmesh.core.config import CategorizerConfig
+from sqlmesh.core.dialect import parse_one
+from sqlmesh.core.model import SqlModel
+from sqlmesh.core.plan import LoadedSnapshotIntervals
+from sqlmesh.core.snapshot import SnapshotChangeCategory
 from sqlmesh.core.user import User, UserRole
 from sqlmesh.integrations.github.cicd.config import GithubCICDBotConfig, MergeMethod
-from sqlmesh.integrations.github.cicd.controller import BotCommand, MergeStateStatus
+from sqlmesh.integrations.github.cicd.controller import (
+    BotCommand,
+    GithubCheckStatus,
+    MergeStateStatus,
+)
 from tests.integrations.github.cicd.fixtures import MockIssueComment
 
 pytest_plugins = ["tests.integrations.github.cicd.fixtures"]
@@ -503,3 +511,52 @@ def test_bot_command_parsing(
         ),
     )
     assert controller.get_command_from_comment() == command
+
+
+def test_unloaded_snapshots(
+    mocker,
+    github_client,
+    make_controller,
+    make_snapshot,
+    make_mock_check_run,
+    make_mock_issue_comment,
+):
+    snapshot_categrozied = make_snapshot(SqlModel(name="a", query=parse_one("select 1, ds")))
+    snapshot_categrozied.categorize_as(SnapshotChangeCategory.BREAKING)
+    snapshot_uncategorized = make_snapshot(SqlModel(name="b", query=parse_one("select 1, ds")))
+    mocker.patch(
+        "sqlmesh.core.plan.Plan.loaded_snapshot_intervals",
+        PropertyMock(
+            return_value=(
+                [LoadedSnapshotIntervals.from_snapshot(snapshot_categrozied)],
+                [snapshot_uncategorized],
+            )
+        ),
+    )
+    mock_repo = github_client.get_repo()
+    mock_repo.create_check_run = mocker.MagicMock(
+        side_effect=lambda **kwargs: make_mock_check_run(**kwargs)
+    )
+
+    created_comments = []
+    mock_issue = mock_repo.get_issue()
+    mock_issue.create_comment = mocker.MagicMock(
+        side_effect=lambda comment: make_mock_issue_comment(
+            comment=comment, created_comments=created_comments
+        )
+    )
+    mock_issue.get_comments = mocker.MagicMock(side_effect=lambda: created_comments)
+    controller = make_controller(
+        "tests/fixtures/github/pull_request_synchronized.json", github_client
+    )
+    controller.update_pr_environment_check(GithubCheckStatus.COMPLETED)
+
+    assert "SQLMesh - PR Environment Synced" in controller._check_run_mapping
+    pr_environment_check_run = controller._check_run_mapping[
+        "SQLMesh - PR Environment Synced"
+    ].all_kwargs
+    assert len(pr_environment_check_run) == 1
+    assert (
+        pr_environment_check_run[0]["output"]["summary"]
+        == """<table><thead><tr><th colspan="3">PR Environment Summary</th></tr><tr><th>Model</th><th>Change Type</th><th>Dates Loaded</th></tr></thead><tbody><tr><td>a</td><td>Breaking</td><td>N/A</td></tr><tr><td>b</td><td>Uncategorized</td><td>N/A</td></tr></tbody></table>"""
+    )