fix(order_fact): compute revenue from line items (DATA-123)

[aliciatoshima]

Summary

Fixes the Q1 revenue reconciliation gap reported in DATA-123. order_fact.revenue was under-reporting by ~$2.47M versus the Stripe captures total.

Root cause was two interacting bugs in models/orders/dw/order_fact.sql:

• INNER JOIN to stg_line_items inside the shipment CTE meant orders that hadn't shipped yet contributed NULL revenue. ~$1.55M lost across 1,374 orders.
• QUALIFY row_number() … = 1 kept only the first shipment per order, silently dropping revenue from later shipments. ~$0.92M lost.

Per models/orders/dw/orders_dw.md (the order_fact_revenue doc), revenue is sum(quantity * unit_price) across an order's line items, gross of refunds. Restructured the model to compute revenue / line_count / total_quantity from stg_line_items at the order grain, and shipment_count / shipped_at from stg_shipments separately. The QUALIFY dedup is no longer needed.

Also bumps the existing reconciliation test from severity='warn' to error, per CONTRIBUTING.md: "severity: warn is for noisy upstream conditions, not for 'we know this is broken.'"

Reconciliation

	Before	After
order_fact non-test revenue	$10,522,258.04	$12,989,886.01
Stripe captures (target)	$12,989,886.01	$12,989,886.01
Gap	$2,467,627.97	$0.00

Test plan

• make full — clean rebuild succeeds (17/17)
• make test — all 12 tests pass, including the now-error reconciliation test
• sqlfluff lint models/orders/dw/order_fact.sql — clean
• Manual: confirmed sum(revenue) where !is_test in order_fact matches sum(quantity * unit_price) from stg_line_items for non-test orders

Follow-ups (not in this PR)

• daily_revenue.sql (and any other consumers of order_fact) auto-corrects since it selects from order_fact.
• Incremental watermark still uses updated_at_dwh = current_timestamp, which means the watermark advances by wall-clock rather than by data time. Worth a separate ticket — out of scope here.
• Pre-existing sqlfluff issues in tests/order_fact_revenue_reconciliation.sql (lowercase keywords) left alone to keep PR scope tight.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Updated order metrics calculation methodology for revenue, quantity, and line count reporting.

• Tests

  • Revenue reconciliation test now enforces stricter validation with error-level reporting.

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4c1c034d-d27c-4184-8cbe-6224ea5f1b8f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

Walkthrough

This PR refactors the order_fact model to compute order-level metrics directly from staging tables at order grain, replacing a multi-CTE shipment-centric pipeline. Two new aggregations—order_revenue from line items and order_shipments from shipments—feed into the enriched CTE, eliminating per-shipment deduplication logic. The test configuration for revenue reconciliation is simultaneously escalated from warning to error severity to enforce data integrity after the restructuring.

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)

• DATA-123: Request failed with status code 401

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

models/orders/dw/order_fact.sql (1)

71-72: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Incremental filtering still misses late changes on existing orders.

Line 72 only reloads rows whose ordered_at is newer than the last model run. If a shipment is added later, or line items are corrected for an older order, that order will never be recomputed, so shipment_count, shipped_at, and revenue can drift again after the initial load.

This should key incremental processing off changed order_ids from the contributing sources instead of ordered_at. At minimum, this rollout needs a full refresh or targeted backfill so historical rows pick up the new logic.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@models/orders/dw/order_fact.sql` around lines 71 - 72, The incremental WHERE
currently uses ordered_at (get_incremental_value('updated_at_dwh')) which misses
later changes to existing orders; change the incremental logic in the
is_incremental() branch so rows are selected by changed order_id from upstream
sources (e.g., compare orders.updated_at, shipments.updated_at,
line_items.updated_at) instead of ordered_at — for example, replace the single
ordered_at filter with a predicate that reloads order_id values whose latest
source updated_at >= get_incremental_value('updated_at_dwh') (or perform a
full_refresh/targeted backfill if you cannot derive changed order_ids), so
computed fields like shipment_count, shipped_at, and revenue are recomputed when
any contributing source changes.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@models/orders/dw/order_fact.sql`:
- Around line 71-72: The incremental WHERE currently uses ordered_at
(get_incremental_value('updated_at_dwh')) which misses later changes to existing
orders; change the incremental logic in the is_incremental() branch so rows are
selected by changed order_id from upstream sources (e.g., compare
orders.updated_at, shipments.updated_at, line_items.updated_at) instead of
ordered_at — for example, replace the single ordered_at filter with a predicate
that reloads order_id values whose latest source updated_at >=
get_incremental_value('updated_at_dwh') (or perform a full_refresh/targeted
backfill if you cannot derive changed order_ids), so computed fields like
shipment_count, shipped_at, and revenue are recomputed when any contributing
source changes.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 97bdd27c-eeb8-4369-bf7b-e6bb3dadc206

📥 Commits

Reviewing files that changed from the base of the PR and between 1b51f4e and da4ee7c.

📒 Files selected for processing (2)

• models/orders/dw/order_fact.sql
• tests/order_fact_revenue_reconciliation.sql