Skip to content

fix(store): tolerate duplicate conversation parts#5

Merged
altaywtf merged 1 commit into
mainfrom
fix/intercom-duplicate-parts
May 17, 2026
Merged

fix(store): tolerate duplicate conversation parts#5
altaywtf merged 1 commit into
mainfrom
fix/intercom-duplicate-parts

Conversation

@altaywtf

@altaywtf altaywtf commented May 17, 2026

Copy link
Copy Markdown
Member

Summary

Fix Intercom archive imports when a provider conversation part ID appears more than once in a sync window.

Changed

  • Upsert conversation parts by provider part ID instead of failing on duplicate IDs.
  • Rebuild FTS rows for conversations that lose a moved duplicate part.
  • Normalize blank synthetic fixture part provider IDs before writing.
  • Add synthetic regression tests for duplicate and blank part IDs.

Risks

  • Persistence behavior for duplicate provider parts changes from hard failure to last normalized occurrence wins.

Verification

  • go test ./internal/store -run 'TestSyncConversations(UpsertsDuplicateProviderParts|NormalizesBlankPartProviderIDs)' -count=1\n- ./scripts/verify\n- codex-review --mode local clean after fixing two accepted findings\n- Private scratch 30d encrypted backfill completed with the local source checkout.\n\n## Complexity\n\nNeutral: adds a small helper around the existing store upsert path.

Summary by cubic

Fixes Intercom archive imports that failed when a conversation part provider ID appears more than once. Parts are now deduped and upserted by provider ID, and FTS is rebuilt for any conversation that loses a moved duplicate.

  • Bug Fixes
    • Upsert conversation_parts on (provider, provider_id); last occurrence wins.
    • Dedupe parts before write and normalize blank provider IDs and IDs.
    • Rebuild conversation_fts for the current conversation and any previous owner of a moved part.
    • Add regression tests for duplicate and blank part IDs.

Written for commit 50a48b1. Summary will update on new commits. Review in cubic

Copilot AI review requested due to automatic review settings May 17, 2026 16:56

@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 2 files

Re-trigger cubic

@altaywtf
altaywtf merged commit d156495 into main May 17, 2026
7 checks passed
@altaywtf
altaywtf deleted the fix/intercom-duplicate-parts branch May 17, 2026 16:59

Copilot AI 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.

Pull request overview

This PR updates the store import/sync path to tolerate duplicate provider conversation-part IDs (seen in Intercom archive imports) by switching from “fail on duplicate” behavior to “upsert + last occurrence wins”, and ensures search/FTS stays consistent when parts move between conversations.

Changes:

  • Upsert conversation_parts on (provider, provider_id) conflicts and dedupe duplicate parts within a conversation before writing.
  • Rebuild conversation_fts for the current conversation and any conversation that lost a moved duplicate part.
  • Normalize blank part ProviderID values (and synthesize stable IDs when needed), plus add regression tests covering duplicates and blank IDs.

Reviewed changes

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

File Description
internal/store/fixture.go Implements part dedupe + upsert-by-provider-ID and adds targeted FTS rebuild logic.
internal/store/fixture_test.go Adds synthetic regression tests for duplicate provider part IDs and blank provider IDs.

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

Comment thread internal/store/fixture.go
Comment on lines +424 to +430
movedPartConversationIDs := map[string]struct{}{}
for _, part := range dedupeParts(conversation.ID, conversation.Parts) {
previousConversationID, err := existingPartConversationID(ctx, tx, conversation.Provider, part.ProviderID, conversation.ID)
if err != nil {
return err
}
if previousConversationID != "" {
Comment thread internal/store/fixture.go
Comment on lines +478 to +486
func rebuildConversationFTS(ctx context.Context, tx *sql.Tx, conversationID string) error {
if _, err := tx.ExecContext(ctx, `delete from conversation_fts where conversation_id = ?`, conversationID); err != nil {
return fmt.Errorf("clear fts: %w", err)
}
if _, err := tx.ExecContext(ctx, `insert into conversation_fts(conversation_id, subject, body, tags, participants, assignee, state, rating, fin_status) values(?, ?, ?, ?, ?, ?, ?, ?, ?)`,
conversation.ID, conversation.Subject, strings.Join(bodies, "\n"), strings.Join(conversation.Tags, " "), strings.Join(conversation.Participants, " "),
conversation.Assignee, conversation.State, conversation.Rating, conversation.FinStatus); err != nil {
if _, err := tx.ExecContext(ctx, `insert into conversation_fts(conversation_id, subject, body, tags, participants, assignee, state, rating, fin_status)
select c.id,
c.subject,
coalesce((select group_concat(body, char(10)) from (select p.body as body from conversation_parts p where p.conversation_id = c.id order by p.updated_at, p.provider_id)), ''),
coalesce((select group_concat(name, ' ') from (select t.name as name from conversation_tags ct join tags t on t.id = ct.tag_id where ct.conversation_id = c.id order by t.name)), ''),
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