fix(dbt): resolve 3 P0 identity/fan-out/sign bugs from warehouse audit

[blarghmatey]

What are the relevant tickets?

Closes #2376
Related to #1957 (same fan-out defect family; that issue investigates conditional uniqueness in a sibling model and isn't fully resolved by this PR)
Related to #2083 (that issue tracks migrating this model off int__/stg__ refs onto the dimensional layer — a different problem than the sign bug fixed here, so not closed by this PR)

Description (What does it do?)

Fixes three P0 correctness defects surfaced by a dimensional/marts/reporting semantic-accuracy audit of src/ol_dbt:

1. dim_user NULL-email identity collapse (Fix dim_user NULL-email identity collapse #2376) — dim_user.sql keys user_pk on hash(lower(email)). The users_with_global_id (MITx/mitlearn), mitxpro_user_view, and mitxresidential_user_view branches did not filter out NULL emails before this, unlike the emeritus/global_alumni/bootcamps branches which already did. Every NULL-email row across those branches collapsed onto a single surrogate key, and agg_view's max()-merge silently fused unrelated people's platform IDs together. Fix: added the same null-email filter to the three unguarded branches, plus a not_null test on dim_user.email as a regression guard.

2. Un-deduped user joins / missing platform key fanning out engagement marts — marts__combined_course_engagements.sql already deduped int__combined__users with row_number() over (partition by user_username, platform) before joining it — proving duplicates exist in that model — but marts__combined_problem_submissions.sql and marts__combined_video_engagements.sql left-joined the same model with no dedup, duplicating event rows for any duplicated user. Separately, marts__combined_course_engagements.sql joined its three daily-activity CTEs on (date, courserun_readable_id, user_username) without platform, double-counting engagement metrics whenever a username collided across platforms. Fix: moved the dedup into int__combined__users.sql itself (the root cause), so all three downstream marts get deduped data for free, and added platform to the three join predicates in marts__combined_course_engagements.sql. The now-redundant local dedup in marts__combined_course_engagements.sql was removed rather than duplicated.

   Update after review: the initial dedup partitioned by (user_username, platform), but emeritus and global_alumni rows always have user_username = NULL — since PARTITION BY groups all NULLs together, this collapsed every emeritus user (and every global_alumni user) into a single row, a data-loss regression caught by review (thanks Copilot/Sentry bots). Fixed by partitioning on coalesce(user_username, user_id, user_email, user_full_name) instead, so platforms without a username still dedupe on a stable per-user identifier instead of collapsing.

3. Inverted sign of program_completion_days — marts__combined_program_enrollment_detail.sql computed date_diff('day', programcertificate_created_on_date, first_courserun_start_on_date). Trino's date_diff(unit, x, y) returns y - x, so this produced negative day counts for completions, contradicting the documented "days between the first course start date and the program completion certificate date" semantics. Fix: swapped the argument order. Confirmed no downstream consumer was already negating the value to compensate.

How can this be tested?

• dbt compile --select dim_user int__combined__users marts__combined_course_engagements marts__combined_program_enrollment_detail marts__combined_problem_submissions marts__combined_video_engagements against the dev_local DuckDB/Iceberg target compiles cleanly (verified).
• Pre-commit (sqlfluff-fix/lint, yaml checks) passes on the changed files.
• Once merged, run/refresh the affected models in a QA environment and spot-check: no more NULL-email dim_user rows with fused platform IDs; distinct emeritus/global_alumni users still appear as separate rows in int__combined__users (not collapsed to one); marts__combined_problem_submissions/video_engagements row counts drop for any user with a previously-duplicated int__combined__users row; program_completion_days is positive for completed programs.

Additional Context

• reporting/program_enrollment_with_user_report.sql independently re-derives program_completion_days with the correct sign already from a raw string — that competing definition is tracked separately (unblocked by this PR) and not addressed here.
• Part of a broader dbt warehouse semantic-accuracy audit; these were the only P0-severity findings.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR addresses multiple correctness issues in the src/ol_dbt dbt project that can cause identity collisions, fan-out duplication, and incorrect metric signs in downstream marts.

Changes:

• Filters out NULL emails in additional dim_user branches and adds a not_null test on dim_user.email to prevent identity collapse.
• Adds platform to daily-activity join predicates in marts__combined_course_engagements to prevent cross-platform fan-out/double counting.
• Fixes the sign of program_completion_days by correcting date_diff argument order, and moves user dedup logic into int__combined__users.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/ol_dbt/models/marts/combined/marts__combined_program_enrollment_detail.sql	Swaps date_diff arguments to make program_completion_days positive per intended semantics.
src/ol_dbt/models/marts/combined/marts__combined_course_engagements.sql	Adds platform to join predicates for daily activity rollups to avoid cross-platform collisions.
src/ol_dbt/models/intermediate/combined/int__combined__users.sql	Introduces upstream dedup logic so downstream marts don’t need to defensively dedup user rows.
src/ol_dbt/models/dimensional/dim_user.sql	Adds NULL-email filters in additional union branches to prevent NULL-email identity collapse.
src/ol_dbt/models/dimensional/_dim__models.yml	Adds a not_null test on dim_user.email as a regression guard.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.