Skip to content

fix: refine contacts cumulative stats + refine timeout (SQL/backend) - #2883

Open
malek10xdev wants to merge 7 commits into
mainfrom
fix/refine-sql-only
Open

fix: refine contacts cumulative stats + refine timeout (SQL/backend)#2883
malek10xdev wants to merge 7 commits into
mainfrom
fix/refine-sql-only

Conversation

@malek10xdev

@malek10xdev malek10xdev commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

Split from #2881 (now closed): this PR keeps the SQL/backend half — the /contacts realtime INSERT gating moves to #2884, where it belongs with the frontend channel changes.

Fixes two bugs from the client demo against QA:

  1. refine contacts SQL timed outrefine_persons scanned the whole (growing) pointsofcontact table with no (user_id, ...) index, blew Kong's ~60s upstream timeout, and only reflected the last mining cycle.
  2. Redundant writes/events — re-mined folders duplicated POC rows and no-op person updates emitted realtime events for unchanged rows.

Key changes

refine_persons rewrite (SQL)

  • messages becomes a permanent per-user ledger (refined_at marker) — never drained, never re-counted; pointsofcontact becomes transient staging (purged after refine) with idempotent inserts (ON CONFLICT ... DO NOTHING).
  • refinedpersons now accumulates (counts += delta, recency/seniority GREATEST/LEAST, tags distinct-union; temperature computed inline matching the trigger). Stats are cumulative, no more double-counting.
  • Partition-aware indexes on pointsofcontact (user_id, message_id), (user_id, person_id), and messages (user_id, message_id) — the POC join drops the nested-loop seq scan; one-time purge of ~2.5M orphaned POC rows.
  • One-time orphan purge joins on the (id, user_id) PK, not ctid — ctid is only unique per partition on the HASH(user_id)-partitioned table, so cross-partition ctid collisions could have deleted valid rows.

Backend writes

  • Idempotent POC bulk insert (ON CONFLICT (user_id, message_id, person_id) DO NOTHING) — also a prerequisite for feat(mining): incremental passive mining via per-source UID watermark #2882's at-least-once watermark semantics (cancel/uidvalidity re-fetch re-inserts POC rows).
  • IS DISTINCT FROM change-guard on the single-row person upsert (the RETURNING + fallback re-select keeps person-id resolution correct), cutting redundant UPDATE writes/realtime events.
  • 55s refine timeout with clear logging (refine stays awaited — the mining-complete email reads refined stats); Pipeline completion failure logging made structured.

Testing

  • Cross-referenced NEW vs OLD stats on identical generated data (800 people, ~6.5k POC, tag edge cases): 0 diffs across all stat columns; temperature inline == trigger formula (0 mismatches).
  • Load-tested at 60k messages / 10k people / 143k POC: backfill ~13s (under the 55s safety), incumbent re-run ~70ms, incremental +1000 msgs ~2.3s; accumulate exact (+50 → occ 7→57), no double-count, ledger fully marked, POC purged.
  • Current CI baseline: backend tsc build clean, 605/605 unit tests green (after merging latest main, incl. fix(mining): surface real OAuth 401 cause in passive-mining; improve reauth classification + error messages #2880), prettier/eslint clean.

Migration notes for QA/prod

  • Apply 20260904000001_refine_persons_ledger_accumulate.sql, then run one full SELECT private.refine_persons('<user_id>') so residual unrefined messages are accounted for.
  • The migration runs the non-concurrent index builds and the batched orphan purge inside one transaction — expect write locks on pointsofcontact/messages during the deploy window.
  • Concurrent refine_persons runs for the same user could double-count (the backend 55s timeout does not cancel server-side work) — pre-existing race class, not introduced here.
  • The realtime tenant rate-limit raise (100 → 1000 events/s) is handled in leadminer.io (self-hosted Supabase config) — separate PR.

Resolves #2867.

malek10xdev and others added 7 commits September 5, 2026 02:37
refinedpersons stats previously overwrote to the last cycle and pointsofcontact
grew unbounded (every re-mine re-inserted duplicates, full-table scans timed out
past Kong's ~60s upstream). Rework:

- private.messages becomes a permanent per-user ledger with a refined_at marker;
  messages are never drained, and a message is never re-counted.
- private.pointsofcontact becomes transient staging (purged after refine);
  inserts become idempotent via a new unique index.
- refinedpersons accumulates (counts += delta, recency GREATEST, seniority
  LEAST, tags distinct-union, temperature computed inline matching the trigger).
- partition-aware indexes on (user_id, message_id) / (user_id, person_id) and on
  messages (user_id, message_id) so the POC join stops using a nested-loop seq
  scan; orphan POC rows from previous cycles are purged once.

Verified against old semantics on identical data (0 stat diffs) and at 60k msgs /
10k people (backfill ~13s, incumbent no-op ~70ms, incremental ~2.3s).
… timeout

- INSERT_POC_BULK_SQL now ON CONFLICT (user_id, message_id, person_id) DO
  NOTHING so re-mined folders stop duplicating rows.
- single-row person upsert gained the same IS DISTINCT FROM only-if-changed
  guard as the bulk variant, cutting redundant UPDATE writes/realtime events.
- refineContacts races a 55s timeout (under Kong's ~60s) and logs clearly;
  Pipeline.complete keeps refine awaited (mining-complete email reads refined
  stats) and logs a structured completion error instead of a misleading
  'email notification, refine contacts' message.
The contacts channel subscribed to all person changes, so background/passive
mining flooded the client and the realtime tenant (dropping profile/credits
events). The postgres_changes filter set is fixed at subscribe time, so:

- add buildPersonChangeFilters(): UPDATE+DELETE always, INSERT only while a
  foreground mining is active;
- rebuild the channel when activeMiningTask flips so the server-side filter
  stops delivering INSERTs outside foreground mining (keep-last; new contacts
  appear on reload/page entry).
…oid collision

PR #2882 introduces 20260904000000_passive_mining_incremental_config.sql, so
the refine ledger migration must move to 20260904000001 to keep migration
ordering deterministic when both merge to main.
ctid is only unique per partition on a HASH(user_id)-partitioned table, so
joining the batched orphan purge on poc.ctid could match valid rows at the
same physical position in another partition and delete them. Join on the
globally unique (id, user_id) PK instead.

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
The frontend person-INSERT gating (buildPersonChangeFilters + channel
rebuild) is extracted to feat/contacts-realtime-insert-gating so the
refine-persons SQL/backend fixes can land independently.

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
@github-actions

Copy link
Copy Markdown

Coverage Report

Passed

Commit: b3f01a2

Summary

Name Stmts Branch Funcs Lines
🟡 Total 45.7% 43.2% 52% 46.1%
All files
Name Stmts Branch Funcs Lines
🔴 backend/src/db/mail.ts 0% 0% 0% 0%
🔴 backend/src/db/pg/PgContacts.ts 33.7% 6% 40.7% 32.5%
🟢 backend/src/services/tasks-manager-v2/Pipeline.ts 83% 82.4% 82.4% 84.1%
No coverage changes
Name Stmts Branch Funcs Lines
🔴 backend/src/db/mail.ts 0% 0% 0% 0%
🔴 backend/src/db/pg/PgContacts.ts 33.7% 6% 40.7% 32.5%
🟢 backend/src/services/tasks-manager-v2/Pipeline.ts 83% 82.4% 82.4% 84.1%

Generated by Test Coverage Reporter for commit b3f01a2

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.

leadminer — fix release-blocking bugs & minor issues

1 participant