fix(ol_dbt_cli): bound broken-column-macro collapse to prevent cross-CTE overrun

[blarghmatey]

What are the relevant tickets?

N/A — found while triaging dbt PR CI failures (added in #2430) on two rebased PRs (#2416, #2400).

Description (What does it do?)

Fixes a false-positive bug in ol-dbt validate's SQL parser (src/ol_dbt_cli/ol_dbt_cli/lib/sql_parser.py) that was blocking otherwise-correct PRs from passing the new dbt PR CI gate.

_collapse_broken_column_expressions handles the case where a Jinja macro call like {{ some_macro(...) }} splits a SQL column expression across the }} boundary, leaving orphaned tokens before the real ) as alias. It found the real alias with an unbounded lazy regex search forward through the file.

That's fine when the macro call actually gets an alias nearby. But a macro call can also appear unaliased, e.g. repeated bare in a GROUP BY list:

select
    ...
    , {{ generate_hash_id('...') }}
        as user_hashed_id
    ...
from combined_programs
group by
    ...
    , {{ generate_hash_id('...') }}   -- bare, no alias — this is the case that broke
    , combined_programs.user_id
)

select
    ...
    , case when final_combined_programs.capstone_sum > 0 then 'Y' else 'N' end
        as capstone_indicator
    , date_diff(...) as program_completion_days
from final_combined_programs

The bare GROUP BY occurrence has no real terminal ) as alias nearby, so the unbounded search kept scanning — straight through the rest of the GROUP BY list, the CTE's closing paren, and the outer select keyword — until it landed on the next unrelated as alias several columns and one CTE away (as capstone_indicator). It then collapsed that entire span into a single bogus placeholder, deleting a dozen real columns from the parsed output.

This exact pattern hit marts__combined_program_enrollment_detail.sql in production and produced false yaml_sql_sync / broken_ref_columns errors on two unrelated PRs (#2416, #2400) — the columns user_hashed_id, program_completion_days, capstone_indicator, and unique_courses_taken_in_program are all present in the actual compiled SQL; the validator just couldn't see them because the corrupted intermediate SQL no longer contained them.

Fix:

1. Bound the continuation-line search to 6 lines instead of unbounded. Genuine orphaned-tail cases (the two existing regression tests) resolve within 1–4 lines; past the bound, the regex simply fails to match and the bare placeholder is left untouched — far safer than deleting real SQL.
2. Make the leading ) before as optional in the terminal pattern, so a plain macro call followed by as alias alone on the very next line (no stray tokens — nothing to actually collapse) terminates immediately instead of needing a paren that was never going to appear.

Added a regression test (test_broken_column_regex_stops_at_plain_multiline_alias) reproducing the bare-GROUP BY-placeholder shape.

How can this be tested?

cd src/ol_dbt_cli
uv run pytest tests/test_sql_parser.py -q   # 52 passed, including the new regression test
uv run pytest -q                            # full suite: 284 passed

Also verified end-to-end against the real model that triggered this:

cd src/ol_dbt && uv run --with dbt-duckdb dbt deps && uv run --with dbt-duckdb dbt parse -t dev
uv run ol-dbt validate --skip dimensional_layering --format json

Before the fix: 2 false ERRORs on marts__combined_program_enrollment_detail / program_enrollment_with_user_report. After: those 2 errors are gone, and a full-repo validate diff shows zero newly-introduced findings (3 unrelated pre-existing errors elsewhere are unchanged).

Additional Context

This was found and fixed while rebasing several open PRs onto main to pick up the new lightweight CI (#2430). The identical fix was cherry-picked directly onto the two affected branches (#2416, #2400) to unblock them immediately; once this PR merges, those branches will be rebased onto main to drop the duplicate cherry-picked commit in favor of this one.

[github-actions]

🔎 ol-dbt impact — column-level blast radius

✅ No column-level downstream impact detected for the changed models.

_{Posted by ol-dbt impact (annotate-only — does not block merge).}

[Copilot]

Pull request overview

Fixes a false-positive in ol-dbt validate’s SQL parsing where broken-column macro placeholder collapsing could scan unboundedly forward and incorrectly swallow unrelated columns (even across CTE boundaries). This improves reliability of the new dbt PR CI gate by preventing validator-induced parse corruption.

Changes:

• Bound the broken-column placeholder “continuation line” regex to a small fixed window to prevent cross-CTE overruns.
• Adjusted the terminal alias pattern so plain {{ macro(...) }} followed by as alias on the next line terminates immediately.
• Added a regression test covering the “plain multiline alias” case that previously could trigger runaway collapsing.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/ol_dbt_cli/ol_dbt_cli/lib/sql_parser.py	Bounds the broken-column collapse regex and broadens the terminal alias match to prevent over-consuming SQL.
src/ol_dbt_cli/tests/test_sql_parser.py	Adds regression coverage ensuring plain multiline macro aliases don’t cause later columns to be swallowed.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.