fix(review): precision and new rules from a replay of human-caught defects on public dbt PRs
Stacked on #1726. This PR covers precision fixes and new rules for the no-AI review lanes, measured by replaying public dbt PRs that had defects found by human reviewers.
How it was measured
-
Corpus: 391 defects that human reviewers caught on public OSS dbt PRs, replayed with
altimate reviewin compile mode on about 377–423 PRs. -
A judge labelled each defect caught, partial or missed against the findings.
-
Each defect was also tagged by what can reach it:
Can be reached by Defects An existing rule 30 A rule we could write 89 AI judgment only 206 Out of reach 66
Results (no AI lane)
- Detection: 71 of 391 caught (18.2%), or 92 (23.5%) counting partial catches.
- The ceiling for rules is 119 defects (30.4%), and 64 of those are now caught.
- Of the 53 defects this PR targets, 51 are caught. The other 2 can't be caught by a rule.
- Noise:
- The modified-model fan-out finding was removed. It was 22% of all findings and matched none of the defects.
- Findings per PR end at about 15.9 after merging repeats.
- The PR comment shows about 8 grouped summary bullets per PR, down from 9.1, with about 3 warnings.
What changed
False positives and precision
- Replays exposed false positives in these rules:
var_no_default, nesting counts,case-when-no-end,select-star-in-cte-final, partition pruning, test removal on renamed columns, and incremental filter changes. - Precision pass on the highest-volume findings:
- injection rules are skipped in models
- planner errors are skipped
- undecidable equivalence becomes a suggestion to data-diff
- narrower rules:
union-select-star,order-by-in-cte,unqualified-column-multi-tableandjoin-or-condition ref-double-quotesis removed
- The
unique_keyvs declared uniqueness test check is graded by key shape.
Rules that should have fired but didn't
- Monorepo hunk diffs were empty. Pathspecs are now anchored with
:(top). - Join rules now read the whole ON clause.
config(**cfg)is inlined before the config lint.- Changed macros are now reviewed.
New rules (project-rules.ts plus additions to the catalog)
packages.yml: a dependency pinned to a branch.dbt_project.yml: the version going down, a duplicate grantee, or resources newly disabled.- Jinja: unbalanced blocks, macro parameters reordered, a
var()renamed without the old key as a fallback, andutcnow(). - Model config:
unique_keyon a table or view- BigQuery microbatch
batch_sizevs the partition granularity - the partition column missing from the merge key
- an incremental filter that never filters on the partition column
ref()to a name that doesn't exist. YAML snapshots and installed packages count, and a near match is suggested.- A LEFT JOIN onto a model whose declared unique key the join doesn't cover, for example an SCD2 dimension without
is_current. - Portability is judged against every adapter the project's own macros target (
postgres__x,snowflake__x, ...).
PR comment
- Repeats of one rule in one file are merged into a single finding that lists each instance. Critical findings stay separate.
- A rule that fires on 2 or more models is grouped into one bullet.
AI lane input
- Changed macros, singular tests, snapshots, schema/source yml, seeds and project config are now sent to the advisory AI lane, not only model SQL.
- In the AI-only sample, 104 of 206 defects were in non-model files.
Tests
- 493 review tests pass.
tscis clean, apart from existingtui/errors.
🤖 Generated with Claude Code
Summary by cubic
Fixes false positives and adds rules to the no-AI review lanes, measured by replaying 391 defects human reviewers caught on public dbt PRs. The rule set now catches 51 of the 53 defects it targets, and the modified-model fan-out finding (22% of all findings, matched none of the defects) is removed.
Precision and noise
- Corrected false positives in
var_no_default, nesting counts,case-when-no-end,select-star-in-cte-final, partition pruning, test removal on renamed columns, and incremental filter changes. - Injection rules and planner errors are skipped in models; undecidable equivalence becomes a data-diff suggestion;
ref-double-quotesis removed. - Join rules read the whole ON clause,
config(**cfg)is inlined, changed macros are reviewed, and pathspecs are anchored with:(top)so monorepo hunk diffs aren't empty. - Repeats of one rule within a file merge into a single finding, and a rule firing on 2+ models groups into one PR-comment bullet; critical findings stay separate.
New rules
packages.yml: dependency pinned to a branch.dbt_project.yml: version going down, duplicate grantee, newly disabled resources.- Jinja: unbalanced blocks, reordered macro parameters, renamed
var()without a fallback,utcnow(). - Model config:
unique_keyon a table/view, BigQuery microbatchbatch_size, partition column missing from the merge key, an incremental filter that never filters on the partition column. ref()to a nonexistent name; a LEFT JOIN onto a model whose declared unique key the join doesn't cover; portability judged against every adapter the project's own macros target.- The advisory AI lane now also receives changed macros, tests, snapshots, yml, seeds, and project config.