From ce289dc11820afe2b9659e77d783050c3f34af9a Mon Sep 17 00:00:00 2001 From: Vamsi-klu Date: Sat, 17 Oct 2026 11:06:58 +0000 Subject: [PATCH] fix(console): preserve literal SQL in GitHub plan diffs Signed-off-by: Vamsi-klu --- sqlmesh/core/console.py | 25 +- tests/core/test_console.py | 225 ++++++++++++++++++ .../github/cicd/test_github_controller.py | 56 +++++ 3 files changed, 298 insertions(+), 8 deletions(-) diff --git a/sqlmesh/core/console.py b/sqlmesh/core/console.py index ca11b6e607..8ab6ab0c0f 100644 --- a/sqlmesh/core/console.py +++ b/sqlmesh/core/console.py @@ -6,6 +6,7 @@ import unittest import uuid import logging +import re import textwrap from humanize import metric, naturalsize from itertools import zip_longest @@ -3354,10 +3355,13 @@ def show_environment_difference_summary( if context_diff.has_environment_statements_changes and not no_diff: self._print("\nEnvironment statements:\n") - for _, diff in context_diff.environment_statements_diff( + for language, diff in context_diff.environment_statements_diff( include_python_env=not context_diff.is_new_environment ): - self._print(diff) + if language == "sql": + self._print_diff(diff) + else: + self._print(diff) def show_model_difference_summary( self, @@ -3474,12 +3478,8 @@ def _print_modified_models( diff_text = context_diff.text_diff(snapshot.name) # sometimes there is no text_diff, like on a seed model where the data has been updated if diff_text: - diff_text = f"\n```diff\n{diff_text}\n```" # these are part of a Markdown list, so indent them by 2 spaces to relate them to the current list item - diff_text_indented = "\n".join( - [f" {line}" for line in diff_text.splitlines()] - ) - self._print(diff_text_indented) + self._print_diff(diff_text, indent=" ") else: if indirectly_modified_children: self._print("\n") @@ -3573,7 +3573,7 @@ def _show_categorized_snapshots(self, plan: Plan, default_catalog: t.Optional[st else: continue - self._print(f"```diff\n{context_diff.text_diff(snapshot.name)}\n```\n") + self._print_diff(context_diff.text_diff(snapshot.name)) self._print("```\n") self._print(tree) self._print("\n```") @@ -3697,6 +3697,15 @@ def _print(self, value: t.Any, **kwargs: t.Any) -> None: self.console.print(value, **kwargs) self._captured_outputs.append(capture.get()) + def _print_diff(self, diff: str, indent: str = "") -> None: + longest_backticks = max((len(run) for run in re.findall(r"`+", diff)), default=0) + fence = "`" * max(3, longest_backticks + 1) + indented_diff = diff.replace("\n", f"\n{indent}") + block = f"{indent}\n{indent}{fence}diff\n{indent}{indented_diff}\n{indent}{fence}\n" + # Capture the Markdown source, since even literal Rich output expands tabs. + self._captured_outputs.append(block) + self.console.print(block, markup=False, emoji=False, soft_wrap=True, end="") + class DatabricksMagicConsole(CaptureTerminalConsole): """ diff --git a/tests/core/test_console.py b/tests/core/test_console.py index f899713235..12e68b990e 100644 --- a/tests/core/test_console.py +++ b/tests/core/test_console.py @@ -1,4 +1,229 @@ +import pytest + from sqlmesh.core.console import MarkdownConsole +from sqlmesh.core.context_diff import ContextDiff +from sqlmesh.core.dialect import parse +from sqlmesh.core.environment import EnvironmentNamingInfo, EnvironmentStatements +from sqlmesh.core.model import load_sql_based_model +from sqlmesh.core.snapshot import SnapshotChangeCategory +from sqlmesh.utils.metaprogramming import Executable + + +@pytest.fixture +def markdown_context_diff(make_snapshot): + def make_diff(old_query, new_query, dialect="tsql"): + old, new = [ + make_snapshot( + load_sql_based_model(parse(f"MODEL (name test.model, dialect {dialect}); {query}")) + ) + for query in (old_query, new_query) + ] + new.categorize_as(SnapshotChangeCategory.NON_BREAKING) + return ContextDiff( + environment="prod", + is_new_environment=False, + is_unfinalized_environment=False, + normalize_environment_name=True, + create_from="prod", + create_from_env_exists=True, + added=set(), + removed_snapshots={}, + modified_snapshots={new.name: (new, old)}, + snapshots={new.snapshot_id: new}, + new_snapshots={new.snapshot_id: new}, + previous_plan_id=None, + previously_promoted_snapshot_ids=set(), + previous_finalized_snapshots=None, + previous_gateway_managed_virtual_layer=False, + gateway_managed_virtual_layer=False, + environment_statements=[], + ) + + return make_diff + + +@pytest.mark.parametrize( + "dialect,old_column,new_column,table", + [ + ("tsql", "[old_col]", "[new_col]", "[schema].[table]"), + ("tsql", "[old column]", "[new]]column]", "[my schema].[my]]table]"), + ("duckdb", "old_col", "new_col", "schema_name.table_name"), + ], +) +def test_markdown_console_model_diff_identifiers( + markdown_context_diff, dialect, old_column, new_column, table +): + context_diff = markdown_context_diff( + f"SELECT {old_column} FROM {table}", + f"SELECT {new_column} FROM {table}", + dialect, + ) + snapshot = next(iter(context_diff.snapshots.values())) + diff = context_diff.text_diff(snapshot.name) + assert old_column in diff and new_column in diff and table in diff + + console = MarkdownConsole() + console.show_model_difference_summary( + context_diff, EnvironmentNamingInfo(), None, no_diff=False + ) + + output = console.consume_captured_output() + assert ( + " ```diff\n" + "\n".join(f" {line}" for line in diff.split("\n")) + "\n ```\n" in output + ) + assert "**Directly Modified:**" in output + assert "(Non-breaking)" in output + assert console.consume_captured_output() == "" + + +@pytest.mark.parametrize("categorized", [False, True]) +@pytest.mark.parametrize("backticks", ["", "```", "`````"]) +def test_markdown_console_literal_model_diffs( + markdown_context_diff, mocker, capsys, categorized, backticks +): + literal = f"[bold] [/invalid] :smile: \\path\t{'long text ' * 20}\n{backticks}\nend" + context_diff = markdown_context_diff("SELECT 1 AS [old]", f"SELECT '{literal}' AS [new]") + snapshot = next(iter(context_diff.snapshots.values())) + diff = context_diff.text_diff(snapshot.name) + assert all(line in diff for line in literal.split("\n")) + console = MarkdownConsole(width=30) + console.log_success("before diff") + if categorized: + plan = mocker.Mock( + context_diff=context_diff, + categorized=[snapshot], + indirectly_modified={}, + environment_naming_info=EnvironmentNamingInfo(), + ) + console._show_categorized_snapshots(plan, None) + else: + console.show_model_difference_summary( + context_diff, EnvironmentNamingInfo(), None, no_diff=False + ) + console.log_success("after diff") + + output = console.consume_captured_output() + fence = "`" * max(3, len(backticks) + 1) + indent = "" if categorized else " " + block = ( + f"{indent}{fence}diff\n" + + "\n".join(f"{indent}{line}" for line in diff.split("\n")) + + f"\n{indent}{fence}\n" + ) + assert block in output + assert output.count(block) == 1 + assert output.index("before diff") < output.index(block) < output.index("after diff") + assert console.consume_captured_output() == "" + visible = capsys.readouterr().out + assert visible.count(f"{fence}diff") == 1 + assert "[bold] [/invalid] :smile:" in visible + if categorized: + assert "Directly Modified:" in output + assert "[bold]" not in output.split(block, 1)[1] + + +@pytest.mark.parametrize("no_diff,empty_diff", [(True, False), (False, True)]) +def test_markdown_console_omitted_model_diffs(markdown_context_diff, mocker, no_diff, empty_diff): + context_diff = markdown_context_diff("SELECT 1 AS [old]", "SELECT 2 AS [new]") + text_diff = mocker.patch.object(ContextDiff, "text_diff", return_value="") + console = MarkdownConsole() + console.show_model_difference_summary( + context_diff, EnvironmentNamingInfo(), None, no_diff=no_diff + ) + output = console.consume_captured_output() + assert "```" not in output + assert "**Directly Modified:**" in output + if empty_diff: + text_diff.assert_called_once() + else: + text_diff.assert_not_called() + + +@pytest.mark.parametrize("no_diff", [False, True]) +def test_markdown_console_environment_sql_diffs(markdown_context_diff, no_diff): + context_diff = markdown_context_diff("SELECT 1", "SELECT 2") + context_diff.previous_environment_statements = [ + EnvironmentStatements( + before_all=["SELECT [old] FROM [schema].[table]"], + after_all=["SELECT [old]"], + python_env={"version": Executable.value(1)}, + ) + ] + context_diff.environment_statements = [ + EnvironmentStatements( + before_all=["SELECT [new], ':smile: [/invalid]\\path\t' FROM [schema].[table]"], + after_all=[f"SELECT '```\n{'long text ' * 20}\n```' AS [new]"], + python_env={"version": Executable.value(2)}, + ) + ] + console = MarkdownConsole(width=35) + console.show_environment_difference_summary(context_diff, no_diff=no_diff) + output = console.consume_captured_output() + assert "**Summary of differences from" in output and "`prod`:**" in output + if no_diff: + assert "Environment statements:" not in output + assert "```" not in output + return + + assert "Environment statements:" in output + for language, diff in context_diff.environment_statements_diff(include_python_env=True): + if language == "sql": + fence = "````" if "```" in diff else "```" + assert f"{fence}diff\n{diff}\n{fence}\n" in output + else: + assert diff in output + assert output.index("before_all:") < output.index("after_all:") < output.index("dependencies:") + + +@pytest.mark.parametrize("indirect_only", [False, True]) +def test_markdown_console_model_change_categories( + markdown_context_diff, make_snapshot, indirect_only +): + context_diff = markdown_context_diff("SELECT 1 AS [old]", "SELECT 2 AS [new]") + current, previous = next(iter(context_diff.modified_snapshots.values())) + child_model = load_sql_based_model( + parse("MODEL (name test.child, dialect tsql); SELECT * FROM test.model") + ) + old_child, new_child = [ + make_snapshot(child_model, nodes={parent.name: parent.model}) + for parent in (previous, current) + ] + new_child.categorize_as(SnapshotChangeCategory.INDIRECT_NON_BREAKING) + old_metadata, new_metadata = [ + make_snapshot( + load_sql_based_model( + parse(f"MODEL (name test.metadata, owner {owner}); SELECT 1 AS value") + ) + ) + for owner in ("old_owner", "new_owner") + ] + context_diff.modified_snapshots.update( + { + new_child.name: (new_child, old_child), + new_metadata.name: (new_metadata, old_metadata), + } + ) + context_diff.snapshots.update( + {new_child.snapshot_id: new_child, new_metadata.snapshot_id: new_metadata} + ) + assert context_diff.indirectly_modified(new_child.name) + assert context_diff.metadata_updated(new_metadata.name) + if indirect_only: + del context_diff.modified_snapshots[current.name] + + console = MarkdownConsole() + console.show_model_difference_summary( + context_diff, EnvironmentNamingInfo(), None, no_diff=False + ) + output = console.consume_captured_output() + assert "**Indirectly Modified:**\n- `test.child` (Indirect Non-breaking)" in output + assert "**Metadata Updated:**\n- `test.metadata`" in output + if indirect_only: + assert "**Directly Modified:**" not in output + assert "```" not in output + else: + assert " Indirectly Modified Children:\n - `test.child`" in output + assert output.index(" ```diff") < output.index(" Indirectly Modified Children:") def test_markdown_console_warning_block(): diff --git a/tests/integrations/github/cicd/test_github_controller.py b/tests/integrations/github/cicd/test_github_controller.py index dbb09f4336..66c5999850 100644 --- a/tests/integrations/github/cicd/test_github_controller.py +++ b/tests/integrations/github/cicd/test_github_controller.py @@ -12,7 +12,9 @@ from sqlmesh.core import constants as c from sqlmesh.core.config import CategorizerConfig from sqlmesh.core.dialect import parse_one +from sqlmesh.core.environment import EnvironmentNamingInfo, EnvironmentStatements from sqlmesh.core.model import SqlModel +from sqlmesh.core.snapshot import SnapshotChangeCategory from sqlmesh.core.user import User, UserRole from sqlmesh.core.plan.definition import Plan from sqlmesh.core.linter.rule import RuleViolation @@ -800,6 +802,60 @@ def test_get_plan_summary_includes_warnings_and_errors( ) in summary +def test_get_plan_summary_preserves_literal_sql( + github_client, make_controller, make_snapshot, mocker +): + controller = make_controller( + "tests/fixtures/github/pull_request_synchronized.json", github_client + ) + context_diff = controller.prod_plan.context_diff + old, new = [ + make_snapshot( + SqlModel( + name="test.model", + dialect="tsql", + query=parse_one(f"SELECT [{column}] FROM [schema].[table]", dialect="tsql"), + ) + ) + for column in ("old_col", "new_col") + ] + new.categorize_as(SnapshotChangeCategory.NON_BREAKING) + context_diff.is_new_environment = False + context_diff.added = set() + context_diff.snapshots = {new.snapshot_id: new} + context_diff.modified_snapshots = {new.name: (new, old)} + context_diff.environment_statements = [ + EnvironmentStatements( + before_all=["SELECT [before] FROM [schema].[table]"], + after_all=["SELECT ':smile: [/invalid] ```' AS [after]"], + python_env={}, + ) + ] + plan = mocker.Mock( + spec=Plan, + context_diff=context_diff, + restatements={}, + missing_intervals=[], + user_provided_flags={}, + environment_naming_info=EnvironmentNamingInfo(), + ) + controller._console.log_success("stale output") + github_client.reset_mock() + + summary = controller.get_plan_summary(plan) + + diff = context_diff.text_diff(new.name) + assert ( + " ```diff\n" + "\n".join(f" {line}" for line in diff.split("\n")) + "\n ```\n" in summary + ) + for _, diff in context_diff.environment_statements_diff(): + fence = "````" if "```" in diff else "```" + assert f"{fence}diff\n{diff}\n{fence}\n" in summary + assert "stale output" not in summary + assert controller._console.consume_captured_output() == "" + assert not github_client.mock_calls + + def test_get_pr_environment_summary_includes_warnings_and_errors( github_client, make_controller: t.Callable[..., GithubController] ):