Skip to content

fix: correct openedAt epoch corruption in PR merge MV (IN-1190)#4320

Merged
gaspergrom merged 2 commits into
mainfrom
fix/IN-1190-pr-opened-at-epoch
Jul 8, 2026
Merged

fix: correct openedAt epoch corruption in PR merge MV (IN-1190)#4320
gaspergrom merged 2 commits into
mainfrom
fix/IN-1190-pr-opened-at-epoch

Conversation

@gaspergrom

@gaspergrom gaspergrom commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes a data-corruption bug in the Tinybird pull_request_analysis_baseline_merge_MV pipe that caused the Insights "Merge lead time" widget to show absurd values (e.g. "53 years average merge time" for Project Jupiter — IN-1190 / linuxfoundation/insights#1997).

Root cause: openedAt is a non-nullable DateTime64(3) column. In the pipe's ANY LEFT JOIN between newly-aggregated PR events and the existing baseline row, when no baseline row exists yet (a brand-new PR), ClickHouse backfills existing.openedAt with the type's zero-default (1970-01-01, epoch) rather than SQL NULL. The old COALESCE(existing.openedAt, new.openedAt) therefore always picked the non-NULL epoch value over the real timestamp, permanently corrupting openedAt on first merge for any new PR.

Fix: if(existing.openedAt != toDateTime(0), existing.openedAt, new.openedAt) as openedAt — mirrors the exact zero-sentinel guard already used for every other non-nullable field in the same SELECT (id, sourceId, segmentId, channel, memberId, platform).

Confirmed via crowd.dev's production Postgres that the source data (activityRelations, CDC-replicated) is clean — zero epoch-corrupted pull_request-opened/merge_request-opened timestamps for Project Jupiter. The corruption is 100% isolated to this Tinybird MV transformation, not an upstream ingestion issue.

Scope

  • 3-line change, single file (services/libs/tinybird/pipes/pull_request_analysis_baseline_merge_MV.pipe), single SQL node.
  • Pure Tinybird pipe change — no schema change, no new datasource, no API contract change.
  • Prevents new corruption only. Does NOT repair the ~682,595 already-corrupted historical rows platform-wide (including all 4,608 of Project Jupiter's PRs, 100% corrupted). That requires a separate targeted ClickHouse ALTER TABLE ... UPDATE repair, scoped by segmentId, sourced from activityRelations (confirmed 100% match rate against the true historical values) — being coordinated separately since it needs direct ClickHouse/Tinybird admin access outside the standard pipe deploy workflow.

Verification

  • Standalone ClickHouse SQL repro (via Tinybird SQL API): old code → epoch, new code → correct real timestamp, for both the unmatched-baseline case (the bug) and the matched-baseline case (regression guard, confirms no behavior change for already-working PRs).
  • Full cross-repo review completed: fix confirmed correct; historical-backfill gap noted as expected/separate follow-up; one suspected duplicate-bug sibling file investigated and ruled out as dead/unused code.

Deploy order

⚠️ This is a Tinybird MV pipe — merging this PR alone does not fix production. It must be explicitly deployed to Tinybird before the fix takes effect. No insights repo changes are needed or included in this PR — the Insights widgets read this same Tinybird data, so they'll reflect corrected data automatically once the Tinybird deploy lands, with no separate insights deploy required.

Test plan

  • Deploy pipe to Tinybird staging/prod
  • Re-run pull_requests_merge_lead_time for project=project-jupiteropenedToReviewAssignedSeconds should NOT change yet (existing rows are already corrupted; this only affects new merges going forward)
  • Confirm no new epoch-openedAt rows appear in pull_requests_analyzed for newly-ingested PRs after deploy
  • Separately coordinate the historical backfill for already-corrupted rows (Jupiter + platform-wide)

https://claude.ai/code/session_01X6BudzWBdNg4C5A5DKo325


Note

Medium Risk
Touches core PR analytics merge logic in a materialized pipe; behavior is a targeted bugfix but wrong deploy timing could still leave bad metrics until historical backfill.

Overview
Fixes openedAt corruption in the pull_request_analysis_results_merged node of pull_request_analysis_baseline_merge_MV.pipe when a new PR has no baseline row yet.

The merge expression changes from COALESCE(existing.openedAt, new.openedAt) to if(existing.openedAt != toDateTime(0), existing.openedAt, new.openedAt), matching the zero-sentinel pattern already used for other non-nullable string fields. On an ANY LEFT JOIN miss, ClickHouse fills existing.openedAt with 1970-01-01 instead of NULL, so COALESCE kept the epoch and overwrote the real open time—skewing downstream duration fields (e.g. merge lead time). This only stops new bad rows; existing corrupted pull_requests_analyzed data is out of scope for this change.

Reviewed by Cursor Bugbot for commit 792e4f5. Bugbot is set up for automated code reviews on this repo. Configure here.

Signed-off-by: Gašper Grom <gasper.grom@gmail.com>
Copilot AI review requested due to automatic review settings July 8, 2026 13:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a data-corruption bug in the Tinybird materialized-view pipe pull_request_analysis_baseline_merge_MV.pipe, which feeds the Insights "Merge lead time" widget. In the pipe's ANY LEFT JOIN, unmatched baseline rows have their non-nullable openedAt (DateTime64(3)) column backfilled by ClickHouse with the type's zero-default (1970-01-01) rather than SQL NULL. The previous COALESCE(existing.openedAt, new.openedAt) therefore always selected the epoch sentinel over the real timestamp, corrupting openedAt for every new PR on first merge and producing nonsensical lead-time values.

The fix replaces the COALESCE with if(existing.openedAt != toDateTime(0), existing.openedAt, new.openedAt), matching the zero-sentinel guard already used for the other non-nullable fields in the same SELECT.

Changes:

  • Replace COALESCE(existing.openedAt, new.openedAt) with a toDateTime(0) zero-sentinel guard so a brand-new PR's real openedAt is preserved instead of the epoch backfill value.
  • Prevents new corruption only; historical rows are handled by a separate out-of-band backfill (noted in the description).

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

Copilot AI review requested due to automatic review settings July 8, 2026 21:41
@gaspergrom gaspergrom merged commit 485a47b into main Jul 8, 2026
15 of 16 checks passed
@gaspergrom gaspergrom deleted the fix/IN-1190-pr-opened-at-epoch branch July 8, 2026 21:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants