Skip to content

feat: record where each merge suggestion came from - #5073

Merged
skwowet merged 6 commits into
mainfrom
feat/merge-suggestion-sources
Oct 21, 2026
Merged

skwowet merged 6 commits into
mainfrom
feat/merge-suggestion-sources

Conversation

@skwowet

@skwowet skwowet commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Merge suggestions come from a few different flows, but nothing records which flow created a pair. Suggestions are deleted once they're merged or rejected, so there's no way to tell how each flow performs without heuristics.

This records the source of every suggestion when it's created, in a new mergeSuggestionSources table that outlives the suggestion. Merges and rejections per source come from joining it with mergeActions and the noMerge tables. The reasoning and the meaning of each source are in ADR-0035.

Changes

  • Add the mergeSuggestionSources table, keyed by type, pair and source. Pairs are stored sorted so direction doesn't matter, and there are no foreign keys, so merges and deletes don't remove the history.
  • Add MergeSuggestionSource with four values: global-similarity, subproject-similarity, identity-conflict and enrichment-conflict.
  • upsertMemberMergeSuggestions and the organization addToMerge now take a required source, so every producer has to pass one. The source is recorded in the same transaction as the suggestions it belongs to, and only for pairs that were actually stored.
  • Producers pass their source:
    • global-similarity: member and organization similarity workflows
    • subproject-similarity: subproject member workflow
    • identity-conflict: public API identity create, verify and resolve
    • enrichment-conflict: identity conflicts found during enrichment
  • A pair keeps its first source when it's rescored. A different flow suggesting the same pair adds its own row.
  • Add ADR-0035.

Temporal activity tasks scheduled before the deploy carry the old arguments, so a few in-flight ones may fail or record a wrong source during the rollout.

Type of change

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

Related issue

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

Copilot AI balanced review requested due to automatic review settings October 21, 2026 18:30
@skwowet
skwowet force-pushed the feat/merge-suggestion-sources branch from a670f63 to 581849d Compare October 21, 2026 18:34

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.

🟡 Changes recommended

Member source history can include pairs excluded from suggestion storage, making per-source counts inaccurate.

1 open issue
What changed in this PR

Adds durable source tracking for merge suggestions so CDP can measure outcomes by the flow that generated them.

Changes:

  • Adds a source-history table, source enum, and shared insertion helper.
  • Threads source attribution through suggestion producers.
  • Documents source meanings and retention in ADR-0035.
File Description
services/​libs/​types/​src/​enums/​suggestions.ts Defines suggestion sources.
services/​libs/​data-access-layer/​src/​old/​apps/​merge_suggestions_worker/​organizationMergeSuggestions.repo.ts Records organization suggestion sources.
services/​libs/​data-access-layer/​src/​merge-suggestion-sources/​index.ts Inserts normalized pair history.
services/​libs/​data-access-layer/​src/​member_merge/​suggestions.ts Records member suggestion sources.
services/​libs/​data-access-layer/​src/​index.ts Exports the source helper.
services/​apps/​merge_suggestions_worker/​src/​workflows/​generateSubprojectMemberMergeSuggestions.ts Labels subproject suggestions.
services/​apps/​merge_suggestions_worker/​src/​workflows/​generateOrganizationMergeSuggestions.ts Labels global organization suggestions.
services/​apps/​merge_suggestions_worker/​src/​workflows/​generateMemberMergeSuggestions.ts Labels global member suggestions.
services/​apps/​merge_suggestions_worker/​src/​activities/​organizationMergeSuggestions.ts Forwards organization sources.
services/​apps/​merge_suggestions_worker/​src/​activities/​memberMergeSuggestions.ts Forwards member sources.
services/​apps/​members_enrichment_worker/​src/​activities/​enrichment.ts Labels enrichment identity conflicts.
docs/​adr/​README.md Indexes ADR-0035.
docs/​adr/​0035-merge-suggestion-sources.md Explains attribution and retention.
backend/​src/​database/​migrations/​V1792510516__merge-suggestion-sources.sql Creates durable source storage.
backend/​src/​api/​public/​v1/​members/​resolveMember.ts Labels resolution conflicts.
backend/​src/​api/​public/​v1/​members/​identities/​verifyMemberIdentity.ts Labels verification conflicts.
backend/​src/​api/​public/​v1/​members/​identities/​createMemberIdentity.ts Labels identity-creation conflicts.

Review effort: Balanced

Comment thread services/libs/data-access-layer/src/member_merge/suggestions.ts Outdated
Copilot AI balanced review requested due to automatic review settings October 21, 2026 18:35
@skwowet
skwowet force-pushed the feat/merge-suggestion-sources branch from 581849d to 317f903 Compare October 21, 2026 18:37

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.

🟡 Changes recommended

The attribution design must clarify that deletable rejection records cannot support complete historical rejection counts.

2 open issues

Review effort: Balanced

Comment thread docs/adr/0035-merge-suggestion-sources.md
Copilot AI balanced review requested due to automatic review settings October 21, 2026 18: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.

🔵 Needs a closer look

Source history still records member pairs skipped by suggestion persistence, which can distort attribution metrics.

2 open issues

Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 21, 2026 19:26

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.

🟡 Changes recommended

Concurrent member inserts and non-atomic organization writes can permanently lose source attribution.

2 open issues
1 previously missed issue

In code that hasn't changed since last review

Medium severity Use a shared transaction for suggestions and source attribution

services/​apps/​merge_suggestions_worker/​src/​activities/​organizationMergeSuggestions.ts:405

This activity passes a bare connection, so the suggestion writes commit before the source insert. A worker interruption or database error between them leaves suggestions without attribution. If an organization is merged or deleted before retry, findNonExistingIds filters out the pair and its source is permanently lost. Run this repository call in a shared transaction so suggestions and their sources commit together.

1 resolved since last review

Review effort: Balanced

Comment thread services/libs/data-access-layer/src/member_merge/suggestions.ts
Copilot AI balanced review requested due to automatic review settings October 21, 2026 19:56
Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
@skwowet skwowet self-assigned this Oct 21, 2026
@skwowet
skwowet force-pushed the feat/merge-suggestion-sources branch from 7dba64d to 8a5ed77 Compare October 21, 2026 19:59

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

Persisted Temporal activity arguments require human rollout review, and organization source writes still lack atomic persistence.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Medium severity Wrap organization activity upserts and source insert in one transaction

services/​libs/​data-access-layer/​src/​old/​apps/​merge_suggestions_worker/​organizationMergeSuggestions.repo.ts:246

The global organization activity passes a plain connection, so the suggestion writes commit before this source insert. If the insert fails or the worker stops between writes, suggestions remain without source history. A retry cannot recover that history if an organization has since been deleted: findNonExistingIds filters out the pair. Run each chunk's raw and filtered upserts, cleanup, and source insert in one transaction, passing that transaction into the upsert helper and pgpQx, as the member path does.

2 resolved since last review

Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 21, 2026 20:01

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.

🟡 Changes recommended

Organization suggestions can commit without source history, allowing permanent provenance loss after interrupted writes.

1 open issue

Review effort: Balanced

Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 21, 2026 20:16

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

The acknowledged incompatibility with in-flight Temporal activity arguments requires human review of the rollout plan.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 21, 2026 20:22

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

The acknowledged incompatibility with in-flight Temporal activity arguments requires human confirmation of rollout handling.

No open issues 🎉

Review effort: Balanced

@skwowet
skwowet merged commit d86d7db into main Oct 21, 2026
14 checks passed
@skwowet
skwowet deleted the feat/merge-suggestion-sources branch October 21, 2026 20:27
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