Skip to content

Fix analytics CDC recovery follow-up - #1320

Merged
Asherlc merged 19 commits into
mainfrom
Asherlc/missing-from-activities
Jun 19, 2026
Merged

Asherlc merged 19 commits into
mainfrom
Asherlc/missing-from-activities

Conversation

@Asherlc

@Asherlc Asherlc commented Jun 19, 2026

Copy link
Copy Markdown
Owner

Summary

  • preserve the ClickHouse client query method binding in CDC setup
  • truncate destination tables before recreating missing raw analytics mirrors with initial copy
  • align deduped_activities incremental SELECT column order with the existing production ClickHouse table
  • record production validation in the incident baseline

Validation

  • pnpm exec vitest run src/db/clickhouse-cdc.test.ts analytics/models/read_models/read_model_microbatch.sql.test.ts
  • pnpm lint
  • pnpm tsc --noEmit
  • cd packages/server && pnpm tsc --noEmit
  • cd packages/web && pnpm tsc --noEmit
  • git diff --check

Production verification

  • branch deploy run 27807103397 completed successfully
  • services rolled to ghcr.io/asherlc/dofek:sha-4db22d8
  • analytics worker completed dbt with PASS=14 WARN=0 ERROR=0 SKIP=0 NO-OP=0 TOTAL=14
  • ClickHouse raw mirror counts matched expected restored data after dofek_fitness_raw_analytics initial copy

Summary by cubic

Fixes analytics CDC recovery by preserving ClickHouse client method binding and truncating the correct ClickHouse destinations before recreating missing mirrors. Normalizes analytics.deduped_activities column order and adds migration and test coverage to prevent regressions.

  • Bug Fixes

    • Preserve clickHouseClient.query binding to avoid @clickhouse/client-common TypeError; add a regression test that asserts this is bound with a context-aware client.
    • Truncate raw analytics destination tables when recreating absent mirrors with do_initial_copy = true; add tests that verify table-specific truncation in both absent and existing mirror cases.
    • Keep dbt’s deduped_activities incremental SELECT in canonical order; strengthen tests to assert emitted column order fragments.
    • Document incident details and production validation in the baseline.
  • Migration

    • Add ClickHouse migration 0033_recreate_deduped_activities_column_order (using exported canonical SQL) to drop and recreate analytics.deduped_activities; register it and update tests to expect 33 migrations.

Written for commit 42bf911. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes

    • Fixed column order mismatch in analytics deduped activities data import that caused incorrect data destination.
    • Resolved ClickHouse client binding issue affecting raw fitness mirror synchronization.
    • Added automatic table truncation for raw analytics mirror recreation.
  • Tests

    • Updated test coverage for ClickHouse table operations and client binding behavior.
  • Documentation

    • Added production incident records and resolution details.

Copilot AI review requested due to automatic review settings June 19, 2026 05:42
@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@sourcery-ai sourcery-ai Bot 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.

Sorry @Asherlc, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Two production incident fixes from 2026-06-19: (1) readClickHouseDestinationRowCount now calls clickHouseClient.query directly to preserve this-binding, and setupClickHouseCdc truncates missing initial-copy raw analytics destination tables before creating mirrors; (2) deduped_activities.sql reorders absent_source_external_ids to the end of both UNION ALL SELECT projections to match the ClickHouse incremental table's column order.

Changes

ClickHouse CDC this-binding fix and destination truncation

Layer / File(s) Summary
this-binding fix in readClickHouseDestinationRowCount
src/db/clickhouse-cdc.ts, src/db/clickhouse-cdc.test.ts
Removes the local query variable assignment; calls clickHouseClient.query(...) directly. New BoundClickHouseClient test asserts queryContextIsBound is true during setupClickHouseCdc and verifies do_initial_copy = false is set from destination row counts.
Truncation helpers and setupClickHouseCdc wiring
src/db/clickhouse-cdc.ts, src/db/clickhouse-cdc.test.ts, docs/production-incident-baseline.md
Adds truncateRawAnalyticsDestinationTables (TRUNCATE TABLE IF EXISTS per mapped table) and truncateMissingInitialCopyRawAnalyticsDestinations (filters to mirrors absent from existingMirrorNames and enabled in rawAnalyticsInitialCopyValues). setupClickHouseCdc calls the helper before splitPeerDbSqlStatements. Tests assert exact TRUNCATE command lists for all three test scenarios. Docs document the 2026-06-19 CDC incident and fix.

deduped_activities SELECT column reorder

Layer / File(s) Summary
absent_source_external_ids reorder in both UNION ALL branches
analytics/models/read_models/deduped_activities.sql, analytics/models/read_models/read_model_microbatch.sql.test.ts, docs/production-incident-baseline.md
Moves absent_source_external_ids after refreshed_at in both the current_deduped_activities and stale_deduped_activities SELECT lists. Test adds two normalizedSql assertions covering the is_deleted=0 and is_deleted=1 variants with the corrected column ordering. Docs describe the incremental INSERT column mismatch incident, the fix, and remaining risk guidance.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • Asherlc/dofek#1098: Modifies setupClickHouseCdc bootstrap flow for PeerDB mirror initialization SQL execution, the same function extended here with truncation logic.
  • Asherlc/dofek#1172: Introduces TRUNCATE TABLE IF EXISTS for ClickHouse raw analytics destinations during mirror drop reconciliation — directly related to the new truncation path added here for missing initial-copy mirrors.
  • Asherlc/dofek#1218: Refactors the activity-dedupe pipeline that feeds analytics.deduped_activities, the same model whose final SELECT projection is reordered here.

Suggested labels

area/db, type/bug

Suggested reviewers

  • cubic-dev-ai
🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title 'Fix analytics CDC recovery follow-up' is in imperative mood, 36 characters (under 70), lacks a trailing period, but omits the area prefix required by the guidelines. Add an area prefix such as [analytics] or [deploy] to comply with the requirement: 'prefixed by area when relevant'. Example: '[analytics] Fix CDC recovery follow-up'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Storybook previews for 8d38306d are ready:

This comment updates automatically on each PR push.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@analytics/models/read_models/read_model_microbatch.sql.test.ts`:
- Around line 137-142: The two expect(normalizedSql).toContain() assertions in
the test are validating an incorrect column order that does not match the actual
production ClickHouse schema. According to the schema,
absent_source_external_ids should appear at position 16 (before
member_activity_ids), not after refreshed_at. Update both expect statements to
verify the correct column sequence where source_external_ids comes first,
followed by absent_source_external_ids, then member_activity_ids, and the
remaining columns in their proper order.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: dcdef84a-dca5-4cc5-9f72-67f9cda53356

📥 Commits

Reviewing files that changed from the base of the PR and between 35c26ee and d946fd5.

📒 Files selected for processing (5)
  • analytics/models/read_models/deduped_activities.sql
  • analytics/models/read_models/read_model_microbatch.sql.test.ts
  • docs/production-incident-baseline.md
  • src/db/clickhouse-cdc.test.ts
  • src/db/clickhouse-cdc.ts

Comment thread analytics/models/read_models/read_model_microbatch.sql.test.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No issues found across 5 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

You’re at about 95% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Requires human review: This PR modifies change data capture (CDC) and analytics read-model logic, including ClickHouse client binding fixes, table truncation for mirror recreation, and SQL column reordering. These changes touch production data infrastructure and could cause data loss or corruption if incorrect.

Re-trigger cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

0 issues found across 7 files (changes from recent commits).

You’re at about 96% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Requires human review: Changes include database schema migration (dropping and recreating tables), core CDC data pipeline logic modifications, and production infrastructure recovery. These are high-risk operations affecting data integrity and production systems.

Re-trigger cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

0 issues found across 2 files (changes from recent commits).

You’re at about 96% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Requires human review: Contains a ClickHouse migration that drops and recreates analytics.deduped_activities, plus new truncation logic in CDC setup. Both are medium-risk changes requiring human review for correctness and production impact.

Re-trigger cubic

@Asherlc
Asherlc merged commit 1738aab into main Jun 19, 2026
72 checks passed
@Asherlc
Asherlc deleted the Asherlc/missing-from-activities branch June 19, 2026 13:19
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