# De Pr Reviewer

> Review a data engineering pull request covering SQL, Dataform, Airflow DAGs, or Terraform, and return blocking findings separated from suggestions. Use when the user asks for a code review, pastes a diff or PR link for pipeline code, asks whether a change is safe to merge, or asks what would break if they ship something.

- Skill: `rk-chavali/de-pr-reviewer` (Agent Skill, multi-file: 2 files)
- Install (CLI): `npx skillmds@latest add rk-chavali/de-pr-reviewer`
- Raw SKILL.md: https://api.skillmd.com/api/skills/rk-chavali/de-pr-reviewer/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: DevOps & Infra
- Author: rk-chavali (https://skillmd.com/u/rk-chavali)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/rk-chavali/de-pr-reviewer

---


# Data engineering PR reviewer

Read `references/conventions.md`. If the repo has its own conventions file, that
one wins and you say so in the review.

## Separate blocking from non-blocking, always

A review that mixes "this will corrupt production" with "prefer trailing commas"
gets skimmed. Two sections, blocking first, and if there is nothing blocking, say
so in the first line.

## Blocking findings

Anything on this list blocks a merge.

**Correctness**
- The grain changed and no downstream aggregate was updated.
- A join that can fan out with no dedupe and no uniqueness assertion.
- `LEFT JOIN` where an inner join is intended, changing the row count silently.
- Timezone handling: `CURRENT_DATE()` with no timezone in a scheduled job.
- A filter that drops rows in staging, hiding data from every consumer.
- Non-idempotent writes. Re-running the task produces different output.

**Safety**
- A schema change that breaks a contract with no version bump or notice.
- Dropping or renaming a column that appears in another model or a dashboard.
- `WRITE_TRUNCATE` on a whole table where a partition write was intended.
- Terraform that destroys and re-creates a stateful resource. Read the plan
  output, not just the diff.
- A new IAM binding at project scope, or any primitive role.
- A service account key added anywhere.
- Secrets, project ids, or emails hardcoded in the diff.

**Operability**
- A new production table with no partition, no assertion, or no description.
- A DAG with `catchup=True` and an old start date.
- A sensor with no timeout, or a sensor in poke mode.
- A new scheduled model with no tag, meaning it runs in no schedule.
- A query whose cost is materially higher and unexplained. Ask for the dry run.

## Non-blocking suggestions

Naming, CTE ordering, comment quality, minor readability, opportunities to
consolidate. Cap this section at five items. Beyond five, the author stops
reading and the blocking section loses its force.

## Verify the diff against the warehouse

Read `references/execution-model.md`. Several blocking findings can be confirmed
rather than suspected when a connection is available, and a confirmed finding
survives an argument in the PR thread:

- Changed or dropped column: `get_table_info` on the table, then a job history
  scan for queries referencing that column in the last 30 days. Name the
  consumers in the review.
- New table with no partition: `get_table_info` confirms it.
- Fan-out risk on a join: run the uniqueness check on the join key through
  `execute_sql_readonly` and report the actual duplicate count.

`terraform plan` is a read and worth running for the destroy list. `apply` is
not. A review never mutates anything.

## What to ask for when the diff is not enough

- The dry run byte estimate for any new or changed scheduled query.
- The `terraform plan` output for any infra change, specifically the destroy list.
- The downstream references for any changed column:

```bash
grep -rn "column_name" definitions/ dags/ --include=*.sqlx --include=*.py
```

- Who consumes this table, if the grain or schema moved.

## Output format

```
VERDICT: block | approve-with-comments | approve

BLOCKING
1. <file:line> <what breaks, and what it breaks>
   Fix: <specific change>

SUGGESTIONS
- <file:line> <short>

QUESTIONS
- <the one thing you cannot tell from the diff>
```

## Rules

- Review what changed, plus what the change touches. A one-line SQL edit to a
  model with nine downstream dependents is not a one-line review.
- Do not invent findings to fill the sections. "Nothing blocking, three small
  suggestions" is a complete and useful review.
- If you are unsure whether something breaks, say so and name the check that
  would settle it. Do not soften a real risk into a suggestion.
- No em dashes in review comments.