fix(review): review the PR's own changes: real base, merge-base diff, project scope, renames, model-paths, no raw Jinja

RA
12h ago
+830-76

Issue for this PR

Closes #

Supersedes the base-ref part of #1241 (auto-closed unmerged); the rest of #1241 is CI-comment work and will follow separately.

Type of change

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

What does this PR do?

altimate review could review files the PR never touched. It guessed the base as origin/main, so a PR into any other branch picked up every commit of drift between the two. It also took changed files from the whole repo, ignoring --cwd. On a monorepo PR into a release branch, that meant comments on hundreds of unrelated files. Fixes:

  • Base: taken from --base, else the CI target branch (pull_request.base.ref / GITHUB_BASE_REF, GitLab CI_MERGE_REQUEST_TARGET_BRANCH_NAME), else origin/HEAD, else main/master. If CI names a target branch that isn't fetched, the run fails with a fetch hint rather than guessing.
  • Merge-base diff: the diff and every "old" file come from the merge-base, so working-tree and --head runs agree and commits the base gained later never show up.
  • Project scope: only files under the dbt project at or above --cwd are reviewed; the rest are counted.
  • Renames: the per-file diff names both paths so git pairs them, renamed models get their old side for equivalence/column checks, and pure directory moves that keep the model name are skipped (a name change still goes through, since it breaks ref()).
  • Custom model-paths: files are classified from dbt_project.yml *-paths; without one, the outermost conventional directory wins (models/tests/x.sql is a model).
  • No raw Jinja in engine lanes: without compiled SQL, a Jinja model is marked unverified instead of getting encoding_bypass/multi_statement warnings. Two new {{ config() }} models used to produce REQUEST_CHANGES.
  • The envelope gains an optional signed scope field: base ref/SHA/source, project root, out-of-scope and moved counts, unverified models. Older envelopes still verify.

Behavior change: in CI, an unfetched target branch is now an error (use fetch-depth: 0). --base with no common history also errors.

How did you verify your code works?

  • New test/altimate/review-scope-diff.test.ts (16 tests). Each builds a real git topology in a temp dir:
    • PR into a drifted release branch
    • base moves after branching (working-tree and --head)
    • move + edit and pure move
    • monorepo with two dbt projects
    • custom model-paths
    • two Jinja models with no compiled SQL
    • an end-to-end reviewPullRequest run with GITHUB_BASE_REF
  • All 346 tests in the review-related files pass. Two existing tests that relied on raw Jinja reaching the engine now get compiled SQL.
  • bun typecheck is clean.

Cost side effects

None. Adds a few local git merge-base / symbolic-ref calls per run; no model calls, queries or cloud resources.

Screenshots / recordings

N/A (CLI).

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

Fixes altimate review so it reviews the PR's own changes instead of drifting into unrelated files. The base now comes from --base, the CI target branch, or the remote default branch; diffs use the merge-base so commits the base gained after branching never appear; and only files inside the --cwd dbt project are reviewed.

Behavior changes

  • CI runs now fail when the target branch isn't fetched — use fetch-depth: 0.
  • --base with no common history with the head is an error instead of a silent fallback.
  • Working-tree and --head runs now produce identical diffs.

What changed

  • Renames pair old and new paths, renamed models get their old side for before/after checks, and pure directory moves that keep the model name are skipped.
  • File roles come from dbt_project.yml *-paths (custom model-paths honored), falling back to conventional directories.
  • Jinja models without compiled SQL are marked unverified instead of getting engine warnings about template syntax.
  • The verdict envelope records an optional signed scope field: base ref/SHA/source, project root, out-of-scope and moved counts, and unverified models.

Summary by CodeRabbit

  • New Features
    • Reviews now use the pull request’s target branch in CI, or the remote default branch otherwise, and compare changes from their merge-base.
    • Reviews can identify and scope changes to the relevant dbt project, including projects in monorepos.
    • Review results can report files outside the project, unchanged moves, and models that could not be verified.
  • Bug Fixes
    • Jinja-based SQL without compiled output is no longer sent to engine checks as raw SQL.
    • Renames and other changes with an old-file version are handled in base-side comparisons.
  • Documentation
    • Updated --base guidance to describe the new default behavior.