fix: keep contacts from succeeded sources when an extractor fails [CM-1341] - #4362
Conversation
Signed-off-by: Mouad BANI <mouad-mb@outlook.com>
PR SummaryMedium Risk Overview Persistence avoids accidental data loss on partial runs. Batch logging now reports partial counts separately from ok and failed. Reviewed by Cursor Bugbot for commit 4ba94fb. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4ba94fb. Configure here.
| 'UPDATE security_contacts SET deleted_at = NOW(), updated_at = NOW() WHERE repo_id = $(repoId) AND deleted_at IS NULL', | ||
| { repoId }, | ||
| ) | ||
| } |
There was a problem hiding this comment.
PVR veto leaves stale github-pvr
Medium Severity
On partial runs, merge mode skips soft-deleting existing security_contacts, but processRepo still drops github-pvr rows from the upsert when PVR reports disabled and updates pvr_enabled on the repo. Previously active github-pvr contacts can remain visible while the repo is marked PVR-disabled.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 4ba94fb. Configure here.
There was a problem hiding this comment.
Pull request overview
Adds partial extraction handling so successful security-contact sources remain usable when another extractor fails.
Changes:
- Introduces the
partialprocessing status. - Uses merge-only persistence for partial results.
- Tracks partial outcomes in batch logging.
Metadata: Rename the title to fix: keep contacts from succeeded sources when an extractor fails (CM-1341) to match repository conventions.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
processBatch.ts |
Classifies and logs partial extraction results. |
types.ts |
Adds the partial result variant. |
writeContacts.ts |
Preserves existing contacts during partial writes. |
ingestSingle.ts |
Enables merge-only writes for partial results. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { repoId: target.repoId, errMsg: failed.reason?.message }, | ||
| 'Extractor failed — preserving existing data', | ||
| { repoId: target.repoId, extractor: EXTRACTORS[i].name, errMsg: r.reason?.message }, | ||
| 'Extractor failed — keeping contacts from remaining sources', |
This pull request improves the reliability of security contact extraction by introducing a new
'partial'status for repositories where some extractors fail but others succeed. The changes ensure that existing contact data is only updated (not deleted) when extraction is partial, preventing accidental data loss. Logging and batch processing are also updated to reflect this new status and handle partial results safely.Handling partial extraction results:
'partial'status toProcessRepoResultand updated return logic inprocessRepoto use it when some, but not all, extractors fail. [1] [2] [3]ingestSecurityContactsForPurlto callwriteContactswith amergeoption when the result is'partial', ensuring existing contacts are not deleted. [1] [2]Safe database updates:
writeContactsandwriteContactsChunkto skip soft-deleting contacts for partial results, so only new/updated contacts are merged and no data is lost if an extractor fails. [1] [2]Batch processing and logging improvements:
These changes make the security contact extraction process more robust to partial failures and reduce the risk of losing valid contact data.