Code review — это не только формальность для merge. Это главный механизм обмена знаниями в команде: junior учится у senior, senior ловит баги до прода, все вместе вырабатывают стандарты.

В dbt-проектах review отличается от типичного бэкенд-review. Логика декларативна, нет «алгоритмов», но есть тонкие dba-проблемы вроде «оператор join поменял grain таблицы», «удалили колонку, на которую опираются 5 моделей вниз по DAG». Этот урок — чеклист, что искать.

Большая картина: что review-ит ревьюер

1. CI зелёный

Это сначала. Если CI красный — review не имеет смысла. PR не готов.

В GitHub/GitLab/Cloud видна checkbox: «CI Passed» или «CI Failed». Если красный — ревьюер пишет «CI красный, посмотри логи перед review» и ждёт.

Что обычно красное:

• Compilation Error — Jinja развернулось не так. Открывают компилированный SQL.
• Database Error — модель ref-нула несуществующую колонку, или JOIN с неподходящим типом.
• Test failure — тест провалился на проваленных строках.
• Source freshness fail — источник устарел, не связано с PR’ом, но блокирует.
• Linter (sqlfluff) fail — стиль не соответствует.

Если CI красный по причине, не связанной с PR (например, freshness на source) — ревьюер пишет «CI красный из-за source freshness, не блок» и продолжает.

2. Тесты добавлены?

Главное правило: новая модель = минимум один тест.

Минимум:

• Primary key: not_null + unique на ключевой колонке.

Желательно для marts:

• Relationships к upstream-таблицам через FK.
• Accepted values для enum-полей (status, segment, currency).
• Business rules через singular tests (например, «sum revenue per day = sum revenue from order_items»).

Что искать ревьюеру:

# models/marts/_models.yml
models:
  - name: customer_segments
    description: "..."
    columns:
      - name: customer_id
        data_tests:
          - not_null          # ← должно быть
          - unique            # ← должно быть для PK
      - name: segment
        data_tests:
          - accepted_values:  # ← хорошо
              values: ['bronze', 'silver', 'gold']

Если этих тестов нет — comment в PR: «Добавь не null + unique на customer_id, accepted_values на segment».

Для staging часто допустимо меньше тестов (главное not_null + unique на PK), для marts — больше (по бизнес-инвариантам).

Анти-pattern: тесты только на staging, marts без тестов. Junior часто думает «marts — это конечная точка, проверять не нужно». На самом деле наоборот: marts читают BI, баги там видны менеджменту, нужно тестировать ОСОБЕННО marts.

3. Documentation?

Минимум для новой модели:

models:
  - name: customer_segments
    description: |
      Сегментирует клиентов по lifetime value:
      - Bronze: <$100 общего spend
      - Silver: $100-$500
      - Gold: >$500
      
      Обновляется ежедневно из fct_orders.

Желательно:

    columns:
      - name: customer_id
        description: "FK на customers.customer_id"
      - name: segment
        description: "Тир клиента: bronze | silver | gold"
      - name: lifetime_value_cents
        description: "Total spend за всю историю в копейках"

Что искать ревьюеру:

• Хотя бы description модели — что эта модель и зачем.
• Description для нетривиальных колонок — особенно тех, где имя не очевидно (например, lifetime_value_cents — почему центы? потому что бизнес-логика без float).
• Doc-blocks для повторяющихся описаний — если у тебя в 5 моделях есть колонка customer_id, опиши её в doc-block:

  # docs.md
  `{% docs customer_id %}`
  Unique identifier для клиента. Используется как PK в customers и FK везде.
  `{% enddocs %}`

  И ссылайся: description: "{{ doc('customer_id') }}".

Junior часто пропускает description: «и так понятно». Через 6 месяцев придёт другой аналитик, посмотрит lineage в Explorer, увидит модель без описания — и пойдёт спрашивать в Slack. Хорошее description — это инвестиция в будущее команды.

4. Lineage не сломан неожиданно

Это самая важная часть review в больших проектах.

Когда меняешь модель, в её downstream могут быть неожиданные эффекты. Например:

• Поменял тип колонки customer_id с INT на STRING — downstream JOIN’ы могут сломаться.
• Убрал колонку — downstream-модели, использующие её, упадут на следующем билде.
• Изменил grain таблицы (был один row per customer, стало один row per customer per month) — downstream-агрегации сломаются.

Что искать ревьюеру:

Шаг 1: Открыть lineage в Explorer (или dbt docs). Найти изменённую модель, посмотреть downstream — какие модели зависят?

Шаг 2: Прочитать diff. Изменились ли:

• Имена колонок? (рефакторинг рассматривается как breaking change)
• Типы колонок? (cast issues в downstream)
• Grain таблицы? (новый JOIN, новый GROUP BY)
• Семантика фильтра? (например, WHERE deleted_at IS NULL добавил/убрал — меняет количество строк)

Шаг 3: Если что-то менялось — проверь, что downstream обновлён или unaffected.

Например, PR переименовывает customer_email в email_address. Ревьюер открывает Explorer, видит 3 downstream-модели, использующие customer_email. Comment: «Переименование без update downstream. Какие модели нужно обновить?».

В Cloud Explorer column-level lineage (Fusion) помогает: точно видно, какие колонки куда идут. Без Fusion — на глаз через model-level lineage.

5. Naming и слой

Стандартная структура staging / intermediate / marts (см. модуль 14). Каждый слой имеет свои naming conventions:

• Staging: stg_<source>__<table>.sql (двойной underscore). Это renamed/typed копия source без бизнес-логики.
• Intermediate: int_<purpose>.sql. Композиты, pivot, обогащения.
• Marts: <entity> (customers, orders, revenue_daily). Бизнес-сущности.

Что искать ревьюеру:

• Новая модель в правильной папке? Не staging-модель не в models/staging/.
• Имя в правильном формате? stg_jaffle_customers (без двойного underscore) — неконсистентно с stg_jaffle__orders.
• Не смешана логика разных слоёв? staging-модель с join’ами — это intermediate, не staging.

Анти-pattern: «модель в marts, которая делает все: фильтрует raw, агрегирует, считает бизнес-метрики, плюс маленький JOIN». Должно быть разбито на staging + intermediate + mart.

Ревьюер: «Эта логика по grain — это intermediate, а агрегация в mart. Раздели на две модели».

6. SQL качество

Наконец, сам SQL. Что искать:

Hardcoded table names

Анти-pattern: from stg_orders (прямое имя) Правильно: from {{ ref('stg_orders') }}

В compiled SQL (target/compiled/) видно сразу — если ссылка не развёрнута в полное имя, значит, прямое hardcode.

Hardcoded source names

Анти-pattern: from raw.jaffle.customers (прямое имя в БД) Правильно: from {{ source('jaffle', 'customers') }}

Антипаттерны JOIN

• LEFT JOIN без ON clause — декартово произведение.
• JOIN, который меняет grain без GROUP BY после.
• Несколько JOIN’ов на ту же таблицу с разными условиями — обычно ошибка, должно быть один JOIN с CASE.

Magic numbers

Анти-pattern:

case when total_spend > 100 then 'silver' when total_spend > 500 then 'gold' else 'bronze' end

Лучше:

case
  when total_spend > {{ var('gold_threshold') }} then 'gold'
  when total_spend > {{ var('silver_threshold') }} then 'silver'
  else 'bronze'
end

С var() в dbt_project.yml пороги становятся документированы и легко меняются.

Ещё лучше — seed-таблица segment_thresholds.csv с порогами.

Большие CTE без комментариев

Анти-pattern: 100-строчная модель с 8 CTE без объяснений. Лучше: каждый CTE — комментарий что он делает и зачем.

-- 1. Filter orders to last 90 days (active customers definition)
with active_orders as (
  select * from {{ ref('fct_orders') }}
  where order_date >= current_date - interval '90 days'
),

-- 2. Aggregate per customer
spend_per_customer as (
  select customer_id, sum(amount_cents) as total_spend_cents
  from active_orders
  group by customer_id
),

-- 3. Classify into tiers
final as (
  select 
    customer_id,
    case 
      when total_spend_cents > {{ var('gold_threshold_cents') }} then 'gold'
      ...
    end as tier
  from spend_per_customer
)

select * from final

Перформанс anti-patterns

• Дорогие функции (regex, JSON parse) в SELECT без previous filter.
• SELECT * в marts вместо явных колонок.
• Несколько pass на одну таблицу — обычно можно объединить в один.

Реадабельность

• Снейк-кейс везде (customer_id, не customerId).
• Алиасы JOIN-таблиц короткие но осмысленные (c для customers, o для orders).
• Каждая колонка — на новой строке для длинных SELECT’ов.
• Trailing commas — стиль команды (sqlfluff настроит).

Бонус: что ревьюер пишет в comments

Хорошие comments в PR:

• Suggestion (Github feature): «Замени hardcoded threshold на var(). Вот код [diff]».
• Question: «Почему ты выбрал LEFT JOIN, а не INNER? customer может не иметь orders?»
• Concern: «Этот SELECT * в marts вернёт 50 колонок. BI потом не понимает, какие нужны. Лучше explicit».
• Praise: «Хорошее description в YAML, я бы тоже так писал».
• Nit (мелочь): «nit: trailing comma после последней колонки — это команда обычно без, но не блок».

Плохие comments:

• Subjective без обоснования: «Не нравится».
• Personal: «Junior бы сделал лучше».
• Vague: «Что-то не так».
• Blocking без альтернативы: «Перепиши» — без указания, что переписать.

Junior как reviewer — будет неуверенным, и это нормально. Лучше начать с «Question:» чем «Concern:». Если что-то непонятно — задавай вопрос, не указывай.

Чек-лист для PR review

Когда открываешь PR на review, идёшь по списку:

[ ] CI зелёный
[ ] Новые модели имеют not_null + unique на PK
[ ] Marts имеют business-rule тесты (relationships, accepted_values)
[ ] Description у новой модели в YAML
[ ] Description у нетривиальных колонок
[ ] Lineage — open Explorer, посмотри downstream. Если изменилась схема — downstream обновлён?
[ ] Naming: правильный prefix (stg_/int_/), правильная папка
[ ] SQL: ref() и source() везде, не hardcoded table names
[ ] SQL: магические числа вынесены в var() или seeds
[ ] SQL: длинная модель имеет CTE с комментариями
[ ] SQL: нет SELECT * в marts (или есть веская причина)
[ ] PR описание понятное: что делает, какой impact, что проверить ревьюеру

Это занимает 15-30 минут для маленького PR, час для большого. Время инвестировано — потом ловишь меньше багов в проде.

Попробуй сам

1. Открой свой dbt-проект в GitHub. Создай PR с новой моделью (можно простую).

2. Попроси кого-то review-ить (или сам пройдись по чеклисту).

3. Найди реальный open PR в популярном open-source dbt-проекте (например, dbt-labs/jaffle_shop) и пройдись по чеклисту — что бы ты comment-нул.

4. Если ты на работе — посмотри merged PR на ваш dbt-проект за последний месяц. Какие чеки прошли, какие нет?

Чек-лист

• 6 областей review: CI, tests, docs, lineage, naming, SQL quality.
• CI зелёный — обязательное условие для review.
• Тесты: minimum not_null + unique на PK. Marts должны иметь business-rule тесты.
• Documentation: description минимум на модели, желательно на нетривиальных колонках.
• Lineage: открой Explorer, проверь downstream impact для изменений схемы.
• Naming: stg_/int_/<entity>, правильная папка.
• SQL: ref()/source() везде, var() для magic numbers, CTE с комментариями.
• Comments в style of Suggestion/Question/Concern/Praise — не subjective.
• Junior как reviewer — стартуй с Question, не Concern.