feat(review): close remaining review gaps: action dbt_prepare, data-test adequacy, parse-only column changes
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). Defaultnone, so nothing changes for existing users.compilerunsdbt compileon the head, then compiles the PR's merge-base intotarget-base/in a temporary worktree. A shallow checkout is fetched and deepened when needed.fullalso runsdbt 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_rowcountagainst its ownref, or arelationshipstest to its own column) passes by construction; - new tests that inherit
severity: warnor anerror_ifthreshold fromdbt_project.ymlare reported; - a
dbt_project.ymlchange that loosens those thresholds for a folder is reported.
- a test comparing a model with itself (
- 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 byis 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_prepareis opt-in, reads inputs only from env, and builds intotarget-base/.review-test-adequacy.test.ts: self-comparison, inheritedwarn/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 typecheckis clean, and the real-world corpus is unchanged (15/15 caught, 0/5 false positives). dbt_preparerun locally against a real dbt 1.10 DuckDB project. The head compiled intotarget/, the merge-base compiled intotarget-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_changenow 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;compilerunsdbt compileon the head and the PR's merge-base intotarget-base/, andfullalso runsdbt 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 inheritseverity: warnor anerror_ifthreshold fromdbt_project.yml, plusdbt_project.ymlchanges 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 BYno longer triggers a false "no ORDER BY" and the load-timestamp rule sees such windows.