Skip to content

fix: keep placeholder ids and shared meeting accounts from snowflake out of member identities - #5043

Merged
joanagmaia merged 6 commits into
mainfrom
fix/issue-853-meetings-black-hole-merge
Oct 20, 2026
Merged

joanagmaia merged 6 commits into
mainfrom
fix/issue-853-meetings-black-hole-merge

Conversation

@joanagmaia

@joanagmaia joanagmaia commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Stops the Snowflake connectors from turning placeholder values and known shared meeting accounts into member identities. In meetings, these built a black-hole member that grew to 474 identities and that data sink auto-merged about 108 times in 24 hours.

  • Invitees without an LF account arrive with INVITEE_LF_USER_ID = 'NONE'. The lf_sso_lookup CTE grouped on that value, so the COALESCE gave every non-LF invitee the same LF SSO (andres.twenty), and all of them resolved to one member. NONE was also used as the member sourceId, so every non-LF invitee of a meeting shared the activity sourceId <meetingId>-NONE.
  • Shared Zoom host accounts (zoom@hyperledger.org, zoom2@lfai.foundation, meetings@dpdk.org and others) show up as invitees across many meetings and tie unrelated people together. The blob holds them.
  • The other connectors read identity fields the same way. Cvent already has placeholder usernames (na, none, null) and the user id -.

Changes

  • cleanIdentityValue (transformerBase) treats empty values and placeholders (none, null, undefined, na, n/a, -) as missing. Every Snowflake transformer (meetings, Cvent, committees, and TNC enrollments, courses and certificates) uses it for LF username, platform username and user id.

  • cleanPersonalEmail (core/personalEmail.ts) drops a strict list of 9 shared meeting host accounts, such as zoom@hyperledger.org, zoom2@lfai.foundation and meetings@dpdk.org. Every one of them has been found inside the black-hole member; on 2026-10-20 it held 9 of them alongside 23–27 different LF accounts. Every transformer uses it for the email, and a row with a listed email is skipped. Other Zoom accounts, mailing lists, bots and noreply addresses are left alone: they each sit on their own member with 1–2 identities, so nothing shows they cause bad merges.

  • In meetings, the GENERATED_SOURCE_ID fallback applies again, and the lf_sso_lookup CTE leaves out placeholder user ids and SSOs.

  • Migration V1792492400__soft-delete-shared-snowflake-identities.sql soft-deletes identities that are known to be wrong, using the same explicit list:

    • the 9 shared meeting accounts (meetings email and username identities);
    • the Cvent placeholder usernames na, none and null;
    • the LFID undefined.

    It also touches members.updatedAt for each affected member, the same way the data-access layer soft-deletes identities. A read-only dry run against production matches 18 identities on 8 members, including the current black-hole member. It does not split that member.

  • Unit tests cover placeholder cleaning, the email blocklist and the lookup query. The transformers themselves can't be imported under vitest, because @crowd/integrations loads its integrations with dynamic require.

Not in this PR:

  • A guard on data sink's identity-conflict auto-merge for oversized members. It moved to linuxfoundation/lfx-cdp-insights-ops#854.
  • Repairing members already affected: splitting the blob, removing the <meetingId>-NONE activities, and handling the members left with no identities.

Type of change

  • Bug fix
  • New feature
  • Refactor / cleanup
  • Performance improvement
  • Chore / dependency update
  • Documentation

Related issue

Closes linuxfoundation/lfx-cdp-insights-ops#853

Copilot AI balanced review requested due to automatic review settings October 20, 2026 10:08
@cursor

cursor Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes how members are resolved across Snowflake ingest and runs a one-time soft-delete on existing identities; incorrect rules could drop valid people or leave members without identities.

Overview
Stops Snowflake connector transforms from creating member identities from placeholder strings (none, null, undefined, etc.) and from a fixed list of shared Zoom/meeting host emails, so rows with only junk identifiers are skipped instead of collapsing many people into one member.

Connector pipeline: New cleanIdentityValue and cleanPersonalEmail are wired through meetings, Cvent, committees, and TNC transformers for emails, LF usernames, platform usernames, and source IDs. Meeting attendance SQL now excludes those placeholders from the lf_sso_lookup CTE and ignores placeholder INVITEE_LF_SSO when falling back to the lookup.

Data cleanup: Migration V1792492400 soft-deletes matching bad identities (shared meeting emails/usernames, Cvent placeholder usernames, LFID undefined) and bumps members.updatedAt for affected members.

Unit tests cover the cleaners and the meeting source-query placeholder filters.

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

@joanagmaia
joanagmaia force-pushed the fix/issue-853-meetings-black-hole-merge branch from cc99e62 to cfa97ca Compare October 20, 2026 10:12
@joanagmaia joanagmaia changed the title fix: stop meetings placeholder ids and unguarded auto-merges from building black-hole members fix: treat placeholder lf user ids from the meetings source as missing Oct 20, 2026

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.

🔵 Needs a closer look

Production merge-policy changes need human validation, including synchronization of the size guard with concurrent identity writes.

No open issues 🎉

What changed in this PR

Prevents placeholder identities from conflating members during Snowflake ingestion and adds a size guard to identity-conflict auto-merges.

Changes:

  • Cleans placeholder identity values in meetings and Cvent ingestion.
  • Blocks auto-merges involving oversized members and reports refusals.
  • Adds focused tests for cleaning, lookup filtering, and merge-limit decisions.
File Description
services/​apps/​snowflake_connectors/​src/​integrations/​meetings/​meeting-attendance/​transformer.ts Cleans LF identities to restore generated-ID fallback.
services/​apps/​snowflake_connectors/​src/​integrations/​meetings/​meeting-attendance/​buildSourceQuery.ts Excludes placeholders from SSO lookup.
services/​apps/​snowflake_connectors/​src/​integrations/​meetings/​meeting-attendance/​buildSourceQuery.test.ts Tests lookup filters.
services/​apps/​snowflake_connectors/​src/​integrations/​cvent/​event-registrations/​transformer.ts Cleans Cvent identity fields.
services/​apps/​snowflake_connectors/​src/​core/​transformerBase.ts Adds shared identity cleaning.
services/​apps/​snowflake_connectors/​src/​core/​transformerBase.test.ts Tests identity normalization.
services/​apps/​data_sink_worker/​src/​service/​member.service.ts Applies merge guard and explicit outcomes.
services/​apps/​data_sink_worker/​src/​service/​autoMergeGuard.ts Defines identity limit and decision helper.
services/​apps/​data_sink_worker/​src/​service/​autoMergeGuard.test.ts Tests merge-limit boundaries.
services/​apps/​data_sink_worker/​src/​service/​activity.service.ts Reports oversized-member merge refusals.

Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 20, 2026 10:14
@greptile-apps

greptile-apps Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable new defects were established.

Summary

The PR keeps placeholder values and nine known shared meeting addresses out of Snowflake member identities.

  • Snowflake imports drop placeholder identities and shared meeting emails.
  • Meeting exports use only real LF IDs and SSOs for name lookup.
  • A migration deactivates known bad Snowflake identities already stored.

Reviews (3) · Last reviewed commit: "fix: limit the snowflake email blocklist..." · Reviewed by Greptile

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.

🟢 Approval recommended

The localized changes have no identified blocking issues; regression tests were inspected but not executed.

No open issues 🎉

Review effort: Balanced

…as missing

Refs linuxfoundation/lfx-cdp-insights-ops#853

Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
Copilot AI balanced review requested due to automatic review settings October 20, 2026 10:17
@joanagmaia joanagmaia changed the title fix: treat placeholder lf user ids from the meetings source as missing fix: treat placeholder identity values from snowflake connectors as missing Oct 20, 2026

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.

🔵 Needs a closer look

Cross-connector identity-resolution changes require human validation of rollout scope and fallback behavior.

1 open issue
1 previously missed issue

In code that hasn't changed since last review

Medium severity Normalize direct SSO placeholders before COALESCE fallback

services/​apps/​snowflake_connectors/​src/​integrations/​meetings/​meeting-attendance/​buildSourceQuery.ts:33

Normalize the direct SSO before the COALESCE at line 52 as well. If a valid LF user ID has 'NONE' on the current row and a real SSO on another row, the filtered lookup contains the real username, but COALESCE still selects 'NONE'. The transformer then discards it and emits no LFID identity. Convert direct placeholder SSOs to SQL NULL so the lookup fallback can recover the username.

Review effort: Balanced

…ties

Refs linuxfoundation/lfx-cdp-insights-ops#853

Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
Copilot AI balanced review requested due to automatic review settings October 20, 2026 10:41
@joanagmaia joanagmaia changed the title fix: treat placeholder identity values from snowflake connectors as missing fix: keep placeholder and shared emails from snowflake connectors out of member identities Oct 20, 2026
…der sso

Refs linuxfoundation/lfx-cdp-insights-ops#853

Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
…ound in bad merges

Refs linuxfoundation/lfx-cdp-insights-ops#853

Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
@joanagmaia joanagmaia changed the title fix: keep placeholder and shared emails from snowflake connectors out of member identities fix: keep placeholder ids and shared meeting accounts from snowflake out of member identities Oct 20, 2026

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.

🔵 Needs a closer look

Production identity deletions and heuristic email exclusions require human validation, alongside resolution of the retained correctness finding.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Medium severity Normalize placeholder primary SSO before fallback lookup

services/​apps/​snowflake_connectors/​src/​integrations/​meetings/​meeting-attendance/​buildSourceQuery.ts:33

Normalize the primary SSO before applying the fallback. The SELECT still uses COALESCE(t.INVITEE_LF_SSO, sso.LF_SSO), so a row with a real LF user ID and SSO NONE ignores a valid SSO found on another row for that ID. The transformer then cleans NONE to null and omits the LF identity, even though the lookup can supply it. Treat placeholder primary SSOs as NULL before COALESCE, and add a regression case for a placeholder primary SSO with a valid lookup value.

1 resolved since last review

Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 20, 2026 10:46

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.

🔵 Needs a closer look

Production identity cleanup and cross-connector resolution changes warrant human confirmation of affected records and rollout sequencing.

No open issues 🎉

Review effort: Balanced

@joanagmaia
joanagmaia requested a review from mbani01 October 20, 2026 10:49

@mbani01 mbani01 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.

LGTM

@joanagmaia
joanagmaia merged commit 96a3a79 into main Oct 20, 2026
17 checks passed
@joanagmaia
joanagmaia deleted the fix/issue-853-meetings-black-hole-merge branch October 20, 2026 11:11
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.

3 participants