Add backfill service for slack#34
Conversation
There was a problem hiding this comment.
Pull Request Overview
Adds Slack historical backfill and related automation, plus analysis versioning improvements.
- Implements backfill of Slack channel history and queues events for processing
- Adds auto-join/leave channel helpers, bot join/leave outbox events, and auto-approval of high-confidence intents
- Introduces force analysis mode (versioning) and optional dedup skip; exposes new GraphQL mutations and queries
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/services-api/index.ts | Extends IVectorService with deleteIntentVectors for cleanup in force analysis mode |
| packages/external-services/src/slack/slack-webhook-handler.ts | Adds handling for member_joined/left_channel and propagates skipped flags |
| packages/external-services/src/slack/slack-types.ts | Adds types for Slack member join/leave events |
| packages/external-services/src/slack/slack-service.ts | Adds joinChannel, leaveChannel, and postMessage helpers |
| packages/external-services/src/slack/tests/slack-webhook-handler.test.ts | Adds tests for signature verification and event processing |
| packages/external-services/src/slack/tests/slack-message-adapter.test.ts | Adds tests for message adaptation logic |
| packages/database/src/services/vector.ts | Adds deleteIntentVectors implementation (currently deletes all vectors in namespace) |
| packages/database/src/services/slack-message-processor.ts | Implements channel backfill and updates processing/analysis flow; emits outbox events |
| packages/database/src/services/outbox.ts | Adds outbox events for Slack bot join/leave |
| packages/database/src/services/intent.ts | Adds restoreIntentsFromAnalysis to rollback to a prior analysis |
| packages/ai/src/context/utils.ts | Adds skipDeduplication flag to enhancedDeduplication |
| packages/ai/src/context/analyze/analyzeConversation.ts | Wires forceRefresh into deduplication logic |
| packages/ai/src/context/SpecGeneration.ts | Marks existing intents inactive when forceAnalyze is true |
| apps/api/src/plugins/slack-webhooks.ts | Emits outbox events for bot join/leave and enhances logging |
| apps/api/src/modules/slack/type.ts | Adds BackfillSlackChannelResponse GraphQL type |
| apps/api/src/modules/slack/mutation.ts | Adds configure/remove/backfill Slack channel mutations and cleanup on disconnect |
| apps/api/src/modules/intent/query.ts | Adds filters (analysisId, confidence ranges) and Slack-specific intents query |
| apps/api/src/modules/intent/mutation.ts | Adds bulkExecuteIntents and updateIntentReviewState mutations |
| apps/api/src/modules/conversation/query.ts | Adds queries for conversation analyses and latest analysis |
| apps/api/src/modules/conversation/mutation.ts | Minor adjustments; adds analysis versioning docs in comments |
| apps/docs/* | Adds detailed docs for Phase 4, force analysis behavior, and troubleshooting |
Comments suppressed due to low confidence (1)
apps/docs/FORCE_ANALYZE_BEHAVIOR.md:1
- Docs state intent vectors are deleted during force analysis, but code does not currently call deleteIntentVectors anywhere in the force path. Either (a) implement the deletion (e.g., call vectorSvc.deleteIntentVectors before dedup when forceRefresh is true) or (b) update docs to reflect the current behavior (skip dedup + mark old intents inactive).
# Force Analysis Behavior
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // Then, deduplicate against prior intents in the vector store | ||
| // Skip deduplication if forceRefresh is true (fresh analysis) | ||
| const { deduplicatedIntents, deduplicationStats } = | ||
| await enhancedDeduplication( | ||
| (analysis as any).intents, | ||
| options.conversationId || '', | ||
| vectorSvc, | ||
| embeddingService | ||
| embeddingService, | ||
| options.forceRefresh || false | ||
| ); |
There was a problem hiding this comment.
Force analysis docs state that intent vectors should be deleted before re-analysis, but no deletion is performed here. To avoid old vectors influencing future dedup runs, invoke vectorSvc.deleteIntentVectors when options.forceRefresh is true and a conversationId is present, prior to enhancedDeduplication.
| embeddings: number[][] | ||
| ): Promise<void>; | ||
|
|
||
| deleteIntentVectors(conversationId: string): Promise<void>; |
There was a problem hiding this comment.
The new API suggests deletion of 'intent' vectors only, but the current VectorService implementation deletes the entire namespace (all vectors). Either ensure callers only intend to wipe everything or rename this method to deleteConversationVectors to match behavior and avoid confusion.
| deleteIntentVectors(conversationId: string): Promise<void>; | |
| deleteConversationVectors(conversationId: string): Promise<void>; |
| export interface BackfillSlackChannelResponse { | ||
| success: boolean; | ||
| messagesFetched: number; | ||
| eventsCreated: number; | ||
| errors: string[]; | ||
| } |
There was a problem hiding this comment.
errors as string[] loses structured detail produced by backfill (message + error). Consider exposing a structured error type, e.g., { message: String!, error: String! }, and returning that in the GraphQL schema to preserve actionable diagnostics.
| intentId: t.arg.string({ required: true }), | ||
| reviewState: t.arg.string({ required: true }), // PROPOSED | APPROVED | REJECTED | ||
| rejectionReason: t.arg.string({ required: false }), |
There was a problem hiding this comment.
Use the existing GraphQL enum (IntentReviewStateEnum) for reviewState instead of a string to enforce schema-level validation and better DX. This also removes the need for manual runtime validation.
| // Auto-join channel if this is a new configuration or reactivation | ||
| if (isNewOrReactivated && (input.isActive ?? true)) { | ||
| console.log(`🔄 Attempting to auto-join channel ${input.channelId}...`); |
There was a problem hiding this comment.
Replace console.log with the standard logger for consistency and to avoid noisy stdout; consider request.log.info or a shared logger injected in context.
|
@coderabbitai please review |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.