dbt lab: reviewing a model like a PR

The pull request title is polite: “Add marts_daily_revenue.” Diff stats look modest. CI is green. Someone in Slack already said “LGTM if tests pass.” Then finance opens the board deck on Monday and revenue is 18% higher than the ledger because the model double-counted refunded orders, and the “unique on order_id” test never ran on the grain that actually mattered. Green CI is not a business review. It is a machine saying the files compile.

This is Part 3 of dbt project lab. Part 1 set layout that scales. Part 2 built first models and tests. This part is how you review a model like a production change, not like a homework submit. You still need solid SQL habits from the SQL series, quality language from data quality, and metric discipline from metrics. Map the wider path on Learn.

What you’ll learn

Why a dbt model PR is a contract change, not “just SQL”
A review checklist: grain, joins, filters, tests, docs, and blast radius
How to read a diff for silent metric breakage
What good PR description and reviewer comments look like
A worked revenue mart review with before and after SQL
Common review mistakes and a practice drill you can run this week

A model PR is a product change

In software, a pull request changes behavior for users. In analytics engineering, a model PR changes numbers for decision makers. The users are dashboards, reverse ETL, finance close, and the person who will quote your table in a meeting without opening the SQL.

That means review is not optional politeness. It is how you protect:

Grain: what one row means
Definitions: what “revenue,” “active,” or “churned” include
History: whether past months stay comparable after merge
Downstream trust: who will inherit a wrong join without knowing

dbt makes review better because models, tests, and docs live in the same repo. It does not make review automatic. dbt build can pass while the business logic is still wrong. Tests only catch what someone thought to assert.

Review rule: Approve the change you would defend in a metrics meeting, not the change that merely compiles.

The PR review map

Treat every model PR as a short investigation. You are not hunting style nits first. You are hunting silent wrongness. The map below is the order that catches expensive bugs early.

Filled dbt PR review: scope, grain, joins, tests, blast radius

1. Scope and story

Before the SQL, read the PR body. A useful description answers four questions in plain language:

What business question does this model answer?
What is the grain of the primary model?
What changes for consumers (new columns, renamed fields, different filters)?
How did you validate (sample queries, row counts, reconciliation to a trusted total)?

If the description is “fixes revenue” with no grain and no validation notes, stop and ask for them. Reviewers should not reverse engineer intent from a 200-line SQL file under time pressure.

2. Grain and keys

Grain is the single most important line in a model review. One row per order? Per order line? Per customer-day? If grain is fuzzy, uniqueness tests lie and joins explode.

Look for:

A stated grain in the model description or YAML
unique and not_null tests on the real primary key (or composite key)
GROUP BY that matches that key, not a looser set of dimensions
Window functions that reintroduce duplicates after a careful dedupe

If the author tests uniqueness on order_id but the table is line-item grain, the test is theater. Call that out explicitly.

3. Joins, filters, and time

Most silent metric bugs live here:

Join type: inner join that drops unpaid or unmatched rows when consumers expect left-join coverage
Many-to-many: joining order headers to a non-unique dimension without aggregating first
Status filters: excluding canceled, test, or internal orders inconsistently across models
Timezone and late arrivals: filtering on created_at while the business uses closed_at
Hard-coded dates: magic windows that “fix last week” forever

Read every WHERE as a product decision. “Exclude status = ‘test’” is fine if documented. “Exclude amount < 0” may quietly delete legitimate refunds from a net revenue story.

4. Tests, docs, and contracts

A merge-ready model usually ships with:

Schema tests on keys and critical foreign keys
At least one expression or singular test for a business rule that has burned you before
Column descriptions for fields humans will filter on
Clear naming that matches mart intent (fct_, dim_, or your house convention)

Ask: if this PR only added SQL and no tests, what failure mode is now uncaught? Reviewers can require a test the same way they require a unit test on a risky service change.

5. Blast radius

Use lineage (dbt docs graph, ref search, or warehouse dependents) to list who inherits the change. A staging rename is local. A mart filter change can reprice every dashboard that points at it.

High blast radius PRs need more evidence: side-by-side totals for a few periods, a note on backfill behavior, and an explicit consumer ping. Low blast radius PRs can move faster, but still need grain and keys.

Checklist you can paste into a PR template

Check	What “good” looks like	Red flag
Business intent	One sentence question + owner	“Cleanup” with no consumer
Grain	Named key, matches GROUP BY	Tests on wrong key
Joins	Type justified, fan-out controlled	Unaggregated many-to-many
Filters	Documented inclusion rules	Silent status exclusions
Time logic	Event clock named	Mixed created vs closed
Tests	Key + one business rule	Only default scaffold
Docs	Columns humans filter on described	Empty YAML
Validation	Counts or totals vs trusted source	“Works on my sample”
Downstream	Consumers listed	Unknown mart usage
Rollback	How to revert or pin	No mention of history

Paste this table into your team’s PR template and require authors to fill the left column. Reviewers then argue about substance, not vibes.

How to read the SQL diff

Style comments are fine after correctness. Start with transformations that change totals:

New joins or join type changes
Filter additions or removals
Case statements that rebucket statuses
Currency, tax, discount, or refund handling
Incremental predicates that can skip late data

For incremental models, review the unique key, the lookback window, and what happens on a full refresh. A PR that “speeds up the job” by shrinking the lookback can permanently miss late-arriving facts unless someone notices the gap.

Also scan for copy-paste hazards: a staging model referenced twice under different aliases, a select * that widens a contract without notice, or a hardcoded schema that breaks when the PR runs in another environment.

Worked example: reviewing marts.daily_revenue

Imagine a PR that “fixes daily revenue” after marketing complained the chart was low. Author adds a join to a promotions table “so we can attribute revenue by campaign.” Tests still unique on order_id. CI is green.

Suspicious before (simplified)

-- models/marts/marts_daily_revenue.sql
with orders as (
  select * from {{ ref('stg_orders') }}
  where status not in ('canceled', 'test')
),
promo as (
  select * from {{ ref('stg_promotions') }}
)
select
  o.order_id,
  o.order_date,
  o.customer_id,
  o.amount as revenue,
  p.campaign_id
from orders o
left join promo p
  on o.customer_id = p.customer_id
 -- oops: no date window; one customer, many promos

What a careful reviewer sees in sixty seconds:

Grain claimed as order, but join can fan out orders when a customer has multiple promos
No date predicate on promo eligibility
Revenue will inflate if the mart is later summed without re-deduping
Uniqueness test on order_id will fail once promo fan-out appears, or was never added

Safer after (still simplified)

-- models/marts/marts_daily_revenue.sql
-- Grain: one row per order_id
with orders as (
  select
    order_id,
    order_date,
    customer_id,
    amount as revenue
  from {{ ref('stg_orders') }}
  where status not in ('canceled', 'test')
),
promo_once as (
  select
    customer_id,
    order_date,
    max(campaign_id) as campaign_id
  from {{ ref('stg_promotions') }}
  group by 1, 2
)
select
  o.order_id,
  o.order_date,
  o.customer_id,
  o.revenue,
  p.campaign_id
from orders o
left join promo_once p
  on o.customer_id = p.customer_id
 and o.order_date = p.order_date

Still not perfect (max campaign is a product choice), but grain is preserved and the join key is intentional. Reviewer comments should force that product choice into the PR description: “When multiple campaigns touch a day, we keep the max id for now; open a follow-up for multi-touch attribution.”

YAML and validation the PR should include

models:
  - name: marts_daily_revenue
    description: "One row per order with optional same-day campaign id. Revenue is order amount after excluding canceled and test orders."
    columns:
      - name: order_id
        description: "Primary key. One order per row."
        tests:
          - unique
          - not_null
      - name: revenue
        description: "Order amount in USD. Not net of refunds (see fct_refunds)."
        tests:
          - not_null
      - name: campaign_id
        description: "Nullable. Same-day campaign attribution; max id if multiple."

Validation notes in the PR body might look like this:

-- row count vs stg_orders after same filters
select count(*) from marts_daily_revenue;
select count(*) from stg_orders where status not in ('canceled','test');

-- total revenue vs finance extract for 2026-09-01 to 2026-09-07
select sum(revenue) from marts_daily_revenue
where order_date between '2026-09-01' and '2026-09-07';

Filled PR review result card: grain order_id, fan-out fixed, unique and not_null tests present, revenue total matches…

That result card is what “reviewed” should mean: a human checked the failure modes, not only a bot.

Reviewer comments that help (and ones that waste time)

Helpful:

“This left join can fan out because stg_promotions is not unique on customer_id. Can we aggregate first or change grain?”
“Filter drops refunds; please document whether mart is gross or net.”
“Please add a unique test on the composite key you described.”
“Can you paste week-level sum vs finance in the PR for 2026-09?”

Low value first pass:

Only “nit: trailing comma” while grain is wrong
“Looks fine” with no evidence when the mart is high blast radius
Rewriting style preferences as blocking without a team standard

Authors also have a job: respond with data, not ego. “You’re right, here’s the row count before and after” builds trust faster than “it worked in my CI.”

CI green is necessary, not sufficient

Good pipelines run dbt build (or run + test) on the PR schema, lint SQL, and maybe compare row counts. Use that signal. Do not worship it.

CI will not know that “active customer” now means “logged in last 7 days” instead of “had a paid order last 30.” Only a human who knows the metric contract will. Pair automated checks with the checklist above. If you use AI to draft SQL, treat that the same way you treat any junior author: verify joins and grain yourself. The habits in how to check AI-written SQL transfer cleanly to PR review.

Common mistakes

Approving on green alone. Machines check syntax and declared tests, not business truth.
Reviewing only the new file. Downstream models that ref this mart may inherit a broken filter.
Ignoring incremental quirks. Lookback windows and unique keys change history quietly.
Testing the wrong key. Unique on a non-grain column is false confidence.
Silent definition drift. Renaming “revenue” without updating metric docs or dashboard owners.
Huge PRs. Bundle layout, logic, and rename together so nobody can review carefully.
No validation artifacts. If totals are not in the PR, reviewers invent them in their heads.
Blocking on pure style. Style bots exist; humans should spend attention on grain and joins.

How to practice this week

Pick one recent model PR (or create a small branch that changes a filter) and fill the checklist table honestly.
Write a three-sentence PR description for a model you already own as if a stranger will merge it.
Add one missing uniqueness or business-rule test to a mart that scares you.
Reconcile one metric total to a trusted source for a fixed date range and paste the query into the PR template as a habit.
Review a teammate’s PR with only grain and joins first; save style for a second pass.

If your team is still forming layout habits, revisit Part 1 and Part 2 of this lab so reviews are not fighting folder chaos. For pipeline context outside dbt, the data pipelines series still applies: transforms are only one stage of trust.

Quick recap

A dbt model PR changes numbers people act on. Review it like a product change.
Order of attack: scope, grain, joins and filters, tests and docs, blast radius.
Green CI is necessary, not sufficient. Business rules need human eyes.
Demand a clear grain, intentional joins, and validation totals in the PR body.
Helpful comments name failure modes. Empty “LGTM” on a mart is a risk transfer.
This lab closes the hands-on loop: layout, models and tests, then review discipline.

Sources

dbt Labs, “About dbt projects” and best practices: https://docs.getdbt.com/docs/build/projects
dbt Labs, “Tests”: https://docs.getdbt.com/docs/build/data-tests
dbt Labs, “How we structure our dbt projects”: https://docs.getdbt.com/best-practices/how-we-structure/1-guide-overview
GitHub Docs, “About pull requests”: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests
AMS, Learn map: https://analyticsmadesimple.com/learn/
AMS, How to check AI-written SQL: https://analyticsmadesimple.com/tutorials/how-to-check-ai-written-sql/