Skip to content

dbt PR review: CI experience hides the engine's proofs (undocumented artifacts, misleading lint-only banner, silent AI lane, guessed base ref) #1240

Description

@anandgupta42

Summary

Running the dbt PR review end to end (altimate-code 0.10.0 on jaffle_shop_duckdb with five injected changes) and mining the 30 most recent reviews in our own dogfood repo surfaced a cluster of CI-experience problems. None change what the engine proves; all change whether users see it.

Problems

  1. Artifacts that decide fidelity are undocumented. Equivalence and lineage-breakage proofs need target/catalog.json (dbt docs generate) and compiled SQL for the base ref in target-base/compiled. Neither appears in the usage doc quick-start, github/review/action.yml, or github/review/examples/altimate-ingestion.yml; target-base is mentioned only in prose. 57% of our dogfood reviews ran lint-only.
  2. "Lint-only run — no dbt manifest/warehouse was available" is shown when a manifest was present. summary.degraded is runDegraded || findings.some(f => f.degraded), so a single undecidable finding flips the whole-run banner (orchestrate.ts, format.ts).
  3. The AI lane fails silently. Every failure path in runAiReview returns [] with only a log; telemetry shows 18% of CI runs call review_ai_prompt and never review_ai_parse. A default run with no credentials attempted a stale local endpoint every time; the docs say the lane is "skipped".
  4. Base ref is guessed. defaultBaseRef walks origin/main → origin/master → main → master and ignores the event's pull_request.base.ref. A PR against deployment was diffed against main.
  5. One rule is half of all findings. "new model has no uniqueness/grain test" was 427 of 853 findings across 30 dogfood PRs, one bullet per model, never acted on.
  6. Headless review telemetry has no project_id. Telemetry.setContext is only called from the interactive session loop, so altimate review events cannot be grouped by repo.
  7. PR title/body never reach the AI lane in CI, so its intent check cannot run.
  8. Docs describe PII classification as "columns that newly expose sensitive data"; the lane flags any PII column present in a touched model.

Fix (this PR)

Separate lintOnly from undecidableFindings in the envelope and render each honestly; add artifact hints with the exact commands; AI lane returns ok | skipped | timeout | error with a reason, rendered in the summary and recorded in review_run (ai_status, ai_findings, undecidable_findings); scale the AI timeout with prompt size; read base ref and PR title/body from GITHUB_EVENT_PATH; group repeated grain-test findings in the summary while keeping atomic findings; set project_id on the headless path; update the docs, action and example workflow to compile head and base and generate the catalog.

Out of scope, tracked in docs/internal/2026-09-03-dbt-pr-review-deep-dive.md

Verdict provenance (catalog rules can currently block, contrary to the README), loading .altimate/review.yml from the base rather than the PR head, gate lifecycle / override wiring, inline-comment dedupe, positional equivalence false positive (core), feedback loop.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions