feat(review): close remaining review gaps: action dbt_prepare, data-test adequacy, parse-only column changes

Draft
RA
2d ago
+542-27

Issue for this PR

Closes #

Stacked on #1725 (← #1724 ← #1723 ← … ← #1719); merge in that order. Draft.

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

This closes the remaining acceptance-criteria gaps in the review stack. Two of the fixes came from replaying the PRs behind known escaped defects.

  • Optional artifact preparation in the action (dbt_prepare). Default none, so nothing changes for existing users.
    • compile runs dbt compile on the head, then compiles the PR's merge-base into target-base/ in a temporary worktree. A shallow checkout is fetched and deepened when needed.
    • full also runs dbt docs generate.
    • A failed base compile is a warning, and those models are marked unverified.
    • Inputs reach the script only through env.
    • The manual docs snippet now compiles the merge-base instead of the base branch's tip.
    • The example workflow gets a generic name.
  • Data tests that can't do their job (yml only, any adapter):
    • a test comparing a model with itself (equality / equal_rowcount against its own ref, or a relationships test to its own column) passes by construction;
    • new tests that inherit severity: warn or an error_if threshold from dbt_project.yml are reported;
    • a dbt_project.yml change that loosens those thresholds for a folder is reported.
  • Column-set rules work in dbt parse-only CI. Without compiled SQL, output columns come from the raw model's final SELECT; a Jinja expression in the list makes it unknown. In a parse-only replay, on_schema_change='fail' with new columns now fires on the two real escapes it targets.
  • Dedup windows split across lines are read whole. "Dedup has no ORDER BY" no longer fires falsely when order by is on its own line, and the load-timestamp rule sees such windows.

How did you verify your code works?

  • New tests:
    • review-action.test.ts: dbt_prepare is opt-in, reads inputs only from env, and builds into target-base/.
    • review-test-adequacy.test.ts: self-comparison, inherited warn/error_if, loosened thresholds, dbt 1.8+ arguments:.
    • Additions to review-incremental-semantics.test.ts: multi-line windows, raw-SQL column delta.
  • Suites: 506 review tests and 1,367 skill and release-gate tests pass, bun typecheck is clean, and the real-world corpus is unchanged (15/15 caught, 0/5 false positives).
  • dbt_prepare run locally against a real dbt 1.10 DuckDB project. The head compiled into target/, the merge-base compiled into target-base/ (the base still has the removed column), and the temporary worktree was cleaned up.
  • Replay of the PRs behind six known escapes in a private dbt monorepo (parse-only, as their CI runs):
    • on_schema_change now catches two of them (two escapes each).
    • One escape lived in Python loader code, outside dbt.
    • One targeted the removed microbatch rule.
    • One needs the opt-in slice var.
    • One is pre-existing code the PR didn't touch, so no rule reports it by design.
  • The earlier 7-PR replay is unchanged: 0 findings on files the PR didn't change.

Cost side effects

dbt_prepare: compile adds one dbt compile per side of the PR in CI, and full adds dbt docs generate (warehouse queries for the catalog). Both are opt-in; the default none adds nothing. No model calls.

Screenshots / recordings

N/A (CLI / action).

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR
  • I checked for cost side effects (model calls, queries or cloud resources that grow with user data) and described them, or wrote "none"

🤖 Generated with Claude Code


Summary by cubic

Closes the remaining acceptance-criteria gaps in the dbt review stack: adds an opt-in dbt_prepare action input for building the artifacts the full review needs, and fixes two classes of missed findings — data tests that can't do their job, and column-set rules that skipped work in parse-only CI.

New dbt_prepare action input

  • Default none; compile runs dbt compile on the head and the PR's merge-base into target-base/, and full also runs dbt docs generate.
  • Inputs reach the script only through env; a failed base compile is a warning that marks affected models unverified.
  • Docs now compile the merge-base instead of the base branch's tip, and the example workflow gets a generic name.

Review gap fixes

  • Reports data tests that pass by construction (self-comparisons with equality/equal_rowcount/relationships) or that inherit severity: warn or an error_if threshold from dbt_project.yml, plus dbt_project.yml changes that loosen those thresholds.
  • Without compiled SQL, output columns come from the raw model's final SELECT, so on_schema_change='fail' fires in parse-only CI; a Jinja expression in the list leaves it unknown.
  • Dedup windows are read from the whole file, so a split ORDER BY no longer triggers a false "no ORDER BY" and the load-timestamp rule sees such windows.