fix: action bot properly output all changes (#2513)

Author: eakmanrq

sqlmesh/core/console.py

@@ -1406,16 +1406,16 @@ def show_model_difference_summary(
         ignored_snapshot_ids = ignored_snapshot_ids or set()
         if context_diff.is_new_environment:
             self._print(
-                f"**New environment `{context_diff.environment}` will be created from `{context_diff.create_from}`**\n\n"
+                f"**New environment `{context_diff.environment}` will be created from `{context_diff.create_from}`**\n"
             )
             if not context_diff.has_snapshot_changes:
                 return
 
         if not context_diff.has_changes:
-            self._print(f"**No differences when compared to `{context_diff.environment}`**\n\n")
+            self._print(f"**No differences when compared to `{context_diff.environment}`**\n")
             return
 
-        self._print(f"**Summary of differences against `{context_diff.environment}`:**\n\n")
+        self._print(f"**Summary of differences against `{context_diff.environment}`:**\n")
 
         added_snapshots = {
             context_diff.snapshots[s_id]
@@ -1424,21 +1424,19 @@ def show_model_difference_summary(
         }
         added_snapshot_models = {s for s in added_snapshots if s.is_model}
         if added_snapshot_models:
-            self._print(f"**Added Models:**\n")
-            for snapshot in added_snapshot_models:
+            self._print(f"\n**Added Models:**")
+            for snapshot in sorted(added_snapshot_models):
                 self._print(
-                    f"- {snapshot.display_name(environment_naming_info, default_catalog)}\n"
+                    f"- `{snapshot.display_name(environment_naming_info, default_catalog)}`"
                 )
-            self._print("\n")
 
         added_snapshot_audits = {s for s in added_snapshots if s.is_audit}
         if added_snapshot_audits:
-            self._print(f"**Added Standalone Audits:**\n")
-            for snapshot in added_snapshot_audits:
+            self._print(f"\n**Added Standalone Audits:**")
+            for snapshot in sorted(added_snapshot_audits):
                 self._print(
-                    f"- {snapshot.display_name(environment_naming_info, default_catalog)}\n"
+                    f"- `{snapshot.display_name(environment_naming_info, default_catalog)}`"
                 )
-            self._print("\n")
 
         removed_snapshot_table_infos = {
             snapshot_table_info
@@ -1447,21 +1445,19 @@ def show_model_difference_summary(
         }
         removed_model_snapshot_table_infos = {s for s in removed_snapshot_table_infos if s.is_model}
         if removed_model_snapshot_table_infos:
-            self._print(f"**Removed Models:**\n")
-            for snapshot_table_info in removed_model_snapshot_table_infos:
+            self._print(f"\n**Removed Models:**")
+            for snapshot_table_info in sorted(removed_model_snapshot_table_infos):
                 self._print(
-                    f"- {snapshot_table_info.display_name(environment_naming_info, default_catalog)}\n"
+                    f"- `{snapshot_table_info.display_name(environment_naming_info, default_catalog)}`"
                 )
-            self._print("\n")
 
         removed_audit_snapshot_table_infos = {s for s in removed_snapshot_table_infos if s.is_audit}
         if removed_audit_snapshot_table_infos:
-            self._print(f"**Removed Standalone Audits:**\n")
-            for snapshot_table_info in removed_audit_snapshot_table_infos:
+            self._print(f"\n**Removed Standalone Audits:**")
+            for snapshot_table_info in sorted(removed_audit_snapshot_table_infos):
                 self._print(
-                    f"- {snapshot_table_info.display_name(environment_naming_info, default_catalog)}\n"
+                    f"- `{snapshot_table_info.display_name(environment_naming_info, default_catalog)}`"
                 )
-            self._print("\n")
 
         modified_snapshots = {
             current_snapshot
@@ -1480,43 +1476,39 @@ def show_model_difference_summary(
                 elif context_diff.metadata_updated(snapshot.name):
                     metadata_modified.append(snapshot)
             if directly_modified:
-                self._print(f"**Directly Modified:**\n")
-                for snapshot in directly_modified:
+                self._print(f"\n**Directly Modified:**")
+                for snapshot in sorted(directly_modified):
                     self._print(
-                        f"- `{snapshot.display_name(environment_naming_info, default_catalog)}`\n"
+                        f"- `{snapshot.display_name(environment_naming_info, default_catalog)}`"
                     )
                     if not no_diff:
-                        self._print(f"```diff\n{context_diff.text_diff(snapshot.name)}\n```\n")
-                self._print("\n")
+                        self._print(f"```diff\n{context_diff.text_diff(snapshot.name)}\n```")
             if indirectly_modified:
-                self._print(f"**Indirectly Modified:**\n")
-                for snapshot in indirectly_modified:
+                self._print(f"\n**Indirectly Modified:**")
+                for snapshot in sorted(indirectly_modified):
                     self._print(
-                        f"- `{snapshot.display_name(environment_naming_info, default_catalog)}`\n"
+                        f"- `{snapshot.display_name(environment_naming_info, default_catalog)}`"
                     )
-                self._print("\n")
             if metadata_modified:
-                self._print(f"**Metadata Updated:**\n")
-                for snapshot in metadata_modified:
+                self._print(f"\n**Metadata Updated:**")
+                for snapshot in sorted(metadata_modified):
                     self._print(
-                        f"- `{snapshot.display_name(environment_naming_info, default_catalog)}`\n"
+                        f"- `{snapshot.display_name(environment_naming_info, default_catalog)}`"
                     )
-                self._print("\n")
         if ignored_snapshot_ids:
-            self._print(f"**Ignored Models (Expected Plan Start):**\n")
-            for s_id in ignored_snapshot_ids:
+            self._print(f"\n**Ignored Models (Expected Plan Start):**")
+            for s_id in sorted(ignored_snapshot_ids):
                 snapshot = context_diff.snapshots[s_id]
                 self._print(
-                    f"- `{snapshot.display_name(environment_naming_info, default_catalog)}` ({snapshot.get_latest(start_date(snapshot, context_diff.snapshots.values()))})\n"
+                    f"- `{snapshot.display_name(environment_naming_info, default_catalog)}` ({snapshot.get_latest(start_date(snapshot, context_diff.snapshots.values()))})"
                 )
-            self._print("\n")
 
     def _show_missing_dates(self, plan: Plan, default_catalog: t.Optional[str]) -> None:
         """Displays the models with missing dates."""
         missing_intervals = plan.missing_intervals
         if not missing_intervals:
             return
-        self._print("**Models needing backfill (missing dates):**\n\n")
+        self._print("\n**Models needing backfill (missing dates):**")
         for missing in missing_intervals:
             snapshot = plan.context_diff.snapshots[missing.snapshot_id]
             if not snapshot.is_model:
@@ -1527,9 +1519,8 @@ def _show_missing_dates(self, plan: Plan, default_catalog: t.Optional[str]) -> N
                 preview_modifier = " (**preview**)"
 
             self._print(
-                f"* `{snapshot.display_name(plan.environment_naming_info, default_catalog)}`: {missing.format_intervals(snapshot.node.interval_unit)}{preview_modifier}\n"
+                f"* `{snapshot.display_name(plan.environment_naming_info, default_catalog)}`: {missing.format_intervals(snapshot.node.interval_unit)}{preview_modifier}"
             )
-        self._print("\n")
 
     def _show_categorized_snapshots(self, plan: Plan, default_catalog: t.Optional[str]) -> None:
         context_diff = plan.context_diff

sqlmesh/core/plan/definition.py

@@ -17,7 +17,12 @@
     merge_intervals,
     missing_intervals,
 )
-from sqlmesh.core.snapshot.definition import Interval, SnapshotId, format_intervals
+from sqlmesh.core.snapshot.definition import (
+    Interval,
+    SnapshotId,
+    SnapshotTableInfo,
+    format_intervals,
+)
 from sqlmesh.utils.date import TimeLike, now, to_datetime, to_timestamp
 from sqlmesh.utils.pydantic import PydanticModel
 
@@ -123,7 +128,7 @@ def snapshots(self) -> t.Dict[SnapshotId, Snapshot]:
         }
 
     @cached_property
-    def modified_snapshots(self) -> t.Dict[SnapshotId, Snapshot]:
+    def modified_snapshots(self) -> t.Dict[SnapshotId, t.Union[Snapshot, SnapshotTableInfo]]:
         """Returns the modified (either directly or indirectly) snapshots."""
         return {
             **{s_id: self.context_diff.snapshots[s_id] for s_id in sorted(self.directly_modified)},
@@ -132,6 +137,7 @@ def modified_snapshots(self) -> t.Dict[SnapshotId, Snapshot]:
                 for downstream_s_ids in self.indirectly_modified.values()
                 for s_id in sorted(downstream_s_ids)
             },
+            **self.context_diff.removed_snapshots,
         }
 
     @property

sqlmesh/integrations/github/cicd/controller.py

@@ -22,7 +22,13 @@
 from sqlmesh.core.context import Context
 from sqlmesh.core.environment import Environment
 from sqlmesh.core.plan import Plan, PlanBuilder
-from sqlmesh.core.snapshot.definition import Snapshot, SnapshotId, format_intervals
+from sqlmesh.core.snapshot.definition import (
+    Snapshot,
+    SnapshotChangeCategory,
+    SnapshotId,
+    SnapshotTableInfo,
+    format_intervals,
+)
 from sqlmesh.core.user import User
 from sqlmesh.integrations.github.cicd.config import GithubCICDBotConfig
 from sqlmesh.utils import word_characters_only
@@ -426,9 +432,13 @@ def bot_config(self) -> GithubCICDBotConfig:
         return bot_config
 
     @property
-    def modified_snapshots(self) -> t.Dict[SnapshotId, Snapshot]:
+    def modified_snapshots(self) -> t.Dict[SnapshotId, t.Union[Snapshot, SnapshotTableInfo]]:
         return self.prod_plan_with_gaps.modified_snapshots
 
+    @property
+    def removed_snapshots(self) -> t.Set[SnapshotId]:
+        return set(self.prod_plan_with_gaps.context_diff.removed_snapshots)
+
     @classmethod
     def _append_output(cls, key: str, value: str) -> None:
         """
@@ -442,13 +452,19 @@ def get_plan_summary(self, plan: Plan) -> str:
         try:
             # Clear out any output that might exist from prior steps
             self._console.clear_captured_outputs()
-            self._console._show_categorized_snapshots(plan, self._context.default_catalog)
-            catagorized_snapshots = self._console.consume_captured_output()
+            self._console.show_model_difference_summary(
+                context_diff=plan.context_diff,
+                environment_naming_info=plan.environment_naming_info,
+                default_catalog=self._context.default_catalog,
+                no_diff=False,
+                ignored_snapshot_ids=plan.ignored,
+            )
+            difference_summary = self._console.consume_captured_output()
             self._console._show_missing_dates(plan, self._context.default_catalog)
             missing_dates = self._console.consume_captured_output()
-            if not catagorized_snapshots and not missing_dates:
+            if not difference_summary and not missing_dates:
                 return "No changes to apply."
-            return f"{catagorized_snapshots}\n{missing_dates}"
+            return f"{difference_summary}\n{missing_dates}"
         except PlanError as e:
             return f"Plan failed to generate. Check for pending or unresolved changes. Error: {e}"
 
@@ -759,22 +775,32 @@ def conclusion_handler(
                         # We don't want to display indirect non-breaking since to users these are effectively no-op changes
                         if modified_snapshot.is_indirect_non_breaking:
                             continue
-                        model_name = modified_snapshot.node.name
-                        change_category = (
-                            "Uncategorized"
-                            if not modified_snapshot.change_category
-                            else SNAPSHOT_CHANGE_CATEGORY_STR[modified_snapshot.change_category]
-                        )
-                        intervals = (
-                            modified_snapshot.dev_intervals
-                            if modified_snapshot.is_forward_only
-                            else modified_snapshot.intervals
-                        )
-                        interval_output = (
-                            format_intervals(intervals, modified_snapshot.node.interval_unit)
-                            if intervals
-                            else "N/A"
-                        )
+                        if modified_snapshot.snapshot_id in self.removed_snapshots:
+                            # This will be an FQN since we don't have access to node name from a snapshot table info
+                            # which is what a removed snapshot is
+                            model_name = modified_snapshot.name
+                            change_category = SNAPSHOT_CHANGE_CATEGORY_STR[
+                                SnapshotChangeCategory.BREAKING
+                            ]
+                            interval_output = "REMOVED"
+                        else:
+                            assert isinstance(modified_snapshot, Snapshot)
+                            model_name = modified_snapshot.node.name
+                            change_category = (
+                                "Uncategorized"
+                                if not modified_snapshot.change_category
+                                else SNAPSHOT_CHANGE_CATEGORY_STR[modified_snapshot.change_category]
+                            )
+                            intervals = (
+                                modified_snapshot.dev_intervals
+                                if modified_snapshot.is_forward_only
+                                else modified_snapshot.intervals
+                            )
+                            interval_output = (
+                                format_intervals(intervals, modified_snapshot.node.interval_unit)
+                                if intervals
+                                else "N/A"
+                            )
                         body_rows.append(
                             [
                                 h("td", model_name),
@@ -865,8 +891,6 @@ def conclusion_handler(
             title = conclusion_to_title.get(
                 conclusion, f"Got an unexpected conclusion: {conclusion.value}"
             )
-            if conclusion.is_success and summary:
-                summary = "**Preview of Prod Plan**\n" + summary
             return conclusion, title, summary
 
         self._update_check_handler(

tests/integrations/github/cicd/test_github_commands.py

@@ -141,8 +141,7 @@ def test_run_all_success_with_approvers_approved(
 <details>
   <summary>Prod Plan Being Applied</summary>
 
-
-**Models needing backfill (missing dates):**"""
+**New environment `prod` will be created from `prod`**"""
     )
     with open(github_output_file, "r", encoding="utf-8") as f:
         output = f.read()
@@ -268,8 +267,7 @@ def test_run_all_success_with_approvers_approved_merge_delete(
 <details>
   <summary>Prod Plan Being Applied</summary>
 
-
-**Models needing backfill (missing dates):**"""
+**New environment `prod` will be created from `prod`**"""
     )
     with open(github_output_file, "r", encoding="utf-8") as f:
         output = f.read()
@@ -907,8 +905,7 @@ def raise_on_prod_plan(plan: Plan):
 <details>
   <summary>Prod Plan Being Applied</summary>
 
-
-**Models needing backfill (missing dates):**"""
+**New environment `prod` will be created from `prod`**"""
     )
 
     with open(github_output_file, "r", encoding="utf-8") as f:
@@ -1101,8 +1098,7 @@ def test_comment_command_deploy_prod(
 <details>
   <summary>Prod Plan Being Applied</summary>
 
-
-**Models needing backfill (missing dates):**"""
+**New environment `prod` will be created from `prod`**"""
     )
 
     with open(github_output_file, "r", encoding="utf-8") as f: