fix(format): stop sqlmesh format from corrupting dialect-specific types in MODEL/AUDIT/METRIC headers

[mday-io]

Description

#5864 stopped transpiling MODEL/AUDIT/METRIC headers wholesale so that SQLMesh's own scalar properties would survive formatting untouched (e.g. on tsql, allow_partials TRUE was being rewritten to the unparseable (1 = 1)).

That fix rendered the entire header generically, which loses the dialect for the properties whose values are the user's actual warehouse SQL: columns (ts DATETIME2(6)) becomes TIMESTAMP(6), an audit argument such as CAST('2024-01-01' AS DATETIME2) is silently downgraded the same way, and a macro in property position (which wraps user-authored arguments the same way columns/audits do) hit the same generic path.

This isn't just cosmetic — it compounds across repeated formatting. tsql DATETIME2 renders generically as TIMESTAMP, and tsql parses TIMESTAMP back as ROWVERSION, so a second sqlmesh format pass turns it into VARBINARY. For an SCD kind's time_data_type, that's the physical type of the valid_from/valid_to columns — two format runs can silently turn a datetime column into a binary one.

Splits header rendering per-property instead of per-expression. The split is derived from the field declarations themselves: expression-typed fields (columns, audits, signals, partitioned_by, physical_properties, macro properties, and kind's nested expression fields like time_data_type/unique_key/time_column) hold warehouse SQL and render with the model's dialect, while scalar-typed fields (allow_partials, description, kind name, ...) are SQLMesh's own semantics and stay dialect-agnostic. Deriving the policy from field declarations means it stays correct as properties are added, and a field that's missed fails safe — a keyword goes uncanonicalized rather than a user's SQL being corrupted.

The field-declaration-derived policy needed one deliberate boundary: kind's own annotation is a discriminated union of every *Kind class, so naively recursing into it would classify kind itself as "holds an expression" (since some kind, e.g. IncrementalByTimeRangeKind, has an expression field) and route the entire kind (...) block through a dialect-specific renderer — corrupting scalar siblings like forward_only TRUE into tsql's (1 = 1) (which reparses fine but silently evaluates to False). The policy derivation stops at _ModelKind subclasses for this reason: a kind's own nested properties are independently dialect-tagged when the renderer recurses into them, so the outer kind property never needs to be treated as expression-bearing itself.

Covers MODEL, AUDIT and METRIC headers.

Test Plan

• test_format_model_expressions_meta_render_policy (parametrized over tsql/fabric) and test_format_audit_expressions_meta_render_policy, asserting columns, audits, physical_properties, and kind's nested expression properties keep their dialect-specific spelling while SQLMesh's own scalar properties stay dialect-agnostic.
• test_format_model_expressions_is_idempotent, parametrized over columns, audits, kind, physical_properties, SQLMesh-owned scalars, and macro properties, asserting a second sqlmesh format pass is a no-op.
• test_format_model_expressions_time_column_dialect — time_column's identifier quoting (e.g. tsql [end]) survives formatting instead of falling back to ANSI quoting.
• test_format_model_expressions_macro_property_comments_preserved_with_dialect — comments inside a macro header-property's arguments survive formatting on a dialect-bearing model, plus an idempotency check.
• test_format_model_expressions_kind_scalar_sibling_dialect — a kind block combining an expression property (time_column) with a boolean scalar sibling (forward_only) on tsql round-trips both correctly; asserts on the formatted string and round-trips through load_sql_based_model to confirm model.kind.forward_only is True survives (not just the string).
• pytest tests/core/test_dialect.py — 173 passed.
• ruff check on the changed files — clean.

Release Note

Fix: sqlmesh format corrupting warehouse-specific types in MODEL/AUDIT/METRIC headers

sqlmesh format on models with an explicit dialect (tsql, fabric, and others with dialect-specific type spellings) could silently rewrite warehouse-specific types in columns, audits, physical_properties, and kind-nested properties (e.g. time_data_type, time_column) to a generic spelling. For tsql specifically, this was compounding: DATETIME2 → TIMESTAMP → VARBINARY across two format runs — for an SCD Type 2 model's time_data_type, that silently turned the physical type of the valid_from/valid_to columns from a datetime into a binary type.

Header properties that hold SQLMesh's own semantics (allow_partials, description, kind name, boolean kind properties like forward_only, etc.) are unaffected and continue to render dialect-agnostically, so this only changes output for the subset of header properties that carry actual warehouse SQL.

Action for affected users: if you run sqlmesh format on tsql/fabric models with warehouse-specific types in columns, audits, physical_properties, or SCD kind blocks, re-run sqlmesh format after upgrading and review the diff — previously-corrupted types will be restored to their correct dialect-specific spelling.

Checklist

• I have run make style and fixed any issues
• I have added tests for my changes (if applicable)
• All existing tests pass (make fast-test)
• My commits are signed off (git commit -s) per the DCO

[mday-io]

Reviewed the header-rendering split. It correctly fixes the compounding DATETIME2→TIMESTAMP→VARBINARY bug from SQLMesh#5864 and the new idempotency tests genuinely catch that regression class — 170/170 test_dialect.py passes, ruff check clean, DCO trailers verified on all 3 commits.

Found two gaps in the reflection-based policy while testing edge cases beyond what's covered by the new tests, both reproduced and left as line comments:

1. time_column (wraps its exp.Expr inside a plain TimeColumn Pydantic model) isn't detected by _holds_expression, so it keeps losing dialect-specific identifier quoting.
2. Comments inside macro header-properties are silently dropped when the model has a dialect set, due to comments=False disabling the whole subtree's comment rendering rather than just the intended duplicate-suppression.

Neither is a blocker on the level of the original bug (no data corruption), but both undercut the "policy derivation fails safe" claim in the PR description, so flagging before merge.

---

Generated by Claude Code