feat: add thread-based event grouping for PRs and issues #75
Conversation
WalkthroughAdds end-to-end event threading: a new SQL migration and Drizzle snapshot adding table Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (5)
🧰 Additional context used📓 Path-based instructions (2)**/*.ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-11-25T03:24:12.463ZApplied to files:
📚 Learning: 2025-11-25T03:24:12.463ZApplied to files:
🧬 Code graph analysis (2)src/services/thread-service.ts (2)
src/github-app/event-processor.ts (2)
🪛 ESLintsrc/services/thread-service.ts[error] 1-1: Unable to resolve path to module 'drizzle-orm'. (import-x/no-unresolved) 🔇 Additional comments (13)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/services/thread-service.ts (1)
111-145: Consider storing interval ID for graceful shutdown.The returned timer ID is not stored or used in
src/index.ts. While this works for long-running processes, storing the interval ID would enable graceful cleanup during testing or shutdown scenarios.// In src/index.ts -threadService.startPeriodicCleanup(); +const threadCleanupInterval = threadService.startPeriodicCleanup(); +// Can be used later: clearInterval(threadCleanupInterval)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
drizzle/0006_certain_micromacro.sql(1 hunks)drizzle/meta/0006_snapshot.json(1 hunks)drizzle/meta/_journal.json(1 hunks)src/db/schema.ts(1 hunks)src/github-app/event-processor.ts(8 hunks)src/index.ts(3 hunks)src/services/subscription-service.ts(3 hunks)src/services/thread-service.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Store context externally - maintain stateless bot architecture with no message history, thread context, or conversation memory
Use<@{userId}>for mentions in messages AND add mentions in sendMessage options - do not use @username format
Implement event handlers for onMessage, onSlashCommand, onReaction, onTip, and onInteractionResponse to respond to Towns Protocol events
Define slash commands in src/commands.ts as a const array with name and description properties, then register handlers using bot.onSlashCommand()
Set ID in interaction requests and match ID in responses to correlate form submissions, button clicks, and transaction/signature responses
Use readContract for reading smart contract state, writeContract for SimpleAccount operations, and execute() for external contract interactions
Fund bot.appAddress (Smart Account) for on-chain operations, not bot.botId (Gas Wallet/EOA)
Use bot.* handler methods directly (outside event handlers) for unprompted messages via webhooks, timers, or tasks - requires channelId, spaceId, or other context stored externally
Always check permissions using handler.hasAdminPermission() before performing admin operations like ban, redact, or pin
User IDs are hex addresses in format 0x..., not usernames - use them consistently throughout event handling and message sending
Slash commands do not trigger onMessage - register slash command handlers using bot.onSlashCommand() instead
Use getSmartAccountFromUserId() to retrieve a user's wallet address from their userId
Include required environment variables: APP_PRIVATE_DATA (bot credentials) and JWT_SECRET (webhook security token)
Files:
src/db/schema.tssrc/index.tssrc/services/subscription-service.tssrc/services/thread-service.tssrc/github-app/event-processor.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Provide alt text for image attachments and use appropriate MIME types for chunked attachments (videos, screenshots)
Files:
src/db/schema.tssrc/index.tssrc/services/subscription-service.tssrc/services/thread-service.tssrc/github-app/event-processor.ts
🧠 Learnings (3)
📚 Learning: 2025-11-25T03:24:12.463Z
Learnt from: CR
Repo: HereNotThere/bot-github PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:24:12.463Z
Learning: All event handlers receive a base payload including userId, spaceId, channelId, eventId, and createdAt - use eventId as threadId/replyId when responding to maintain event threading
Applied to files:
src/db/schema.ts
📚 Learning: 2025-11-25T03:24:12.463Z
Learnt from: CR
Repo: HereNotThere/bot-github PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:24:12.463Z
Learning: Applies to **/*.ts : Use bot.* handler methods directly (outside event handlers) for unprompted messages via webhooks, timers, or tasks - requires channelId, spaceId, or other context stored externally
Applied to files:
src/github-app/event-processor.ts
📚 Learning: 2025-11-18T23:35:49.436Z
Learnt from: shuhuiluo
Repo: HereNotThere/bot-github PR: 26
File: src/db/index.ts:61-69
Timestamp: 2025-11-18T23:35:49.436Z
Learning: In `webhook_deliveries` table (src/db/index.ts), the `installation_id` column should NOT have a FOREIGN KEY constraint because the table serves as an immutable audit log for idempotency tracking. Records must persist independently even after installations are deleted, and a foreign key would create race conditions when webhooks arrive before installation records are created. The field is intentionally nullable to support webhooks without installation context.
Applied to files:
drizzle/meta/0006_snapshot.json
🧬 Code graph analysis (2)
src/index.ts (2)
src/services/thread-service.ts (1)
ThreadService(17-146)src/github-app/event-processor.ts (1)
EventProcessor(52-368)
src/github-app/event-processor.ts (1)
src/services/thread-service.ts (2)
AnchorType(6-6)ThreadService(17-146)
🔇 Additional comments (12)
drizzle/meta/_journal.json (1)
46-53: LGTM!The migration journal entry follows the established pattern with consistent versioning and breakpoints configuration.
src/index.ts (2)
54-62: LGTM!The ThreadService is correctly instantiated and wired into the EventProcessor. The initialization order ensures dependencies are available before use.
240-242: LGTM!Periodic cleanup follows the same pattern as the OAuth cleanup service. The internal error handling in
startPeriodicCleanupensures failures don't crash the application.src/services/subscription-service.ts (1)
614-641: LGTM!The addition of
spaceIdto the return type and query is a clean, additive change that enables thread lookups without breaking existing consumers.src/db/schema.ts (1)
232-272: LGTM!The schema definition is well-documented and correctly mirrors the SQL migration. The check constraint and indexes align with the ThreadService operations. Based on learnings, the
threadEventIdcorrectly stores the Towns eventId for thread/reply correlation.drizzle/meta/0006_snapshot.json (1)
1-963: LGTM!This is an auto-generated Drizzle snapshot that correctly captures the new
event_threadstable schema with its constraints and indexes.src/services/thread-service.ts (2)
17-44: LGTM!The
getThreadIdmethod correctly queries all five composite key columns and safely returns null when no thread exists.
49-90: LGTM!The upsert logic correctly handles both insert and conflict cases. On conflict, only
threadEventIdandexpiresAtare updated, preserving the originalcreatedAttimestamp which is appropriate for tracking when the thread was first created.src/github-app/event-processor.ts (4)
37-44: LGTM!The
ThreadingContextinterface cleanly encapsulates the threading metadata needed for event grouping.
52-65: LGTM!Constructor properly accepts and stores the ThreadService dependency.
280-297: Correct detection of PR vs issue comments.The
pull_requestfield check correctly distinguishes between comments on PRs and issues, ensuring they thread to the appropriate parent.
133-183: Verified: Threading implementation correctly uses Towns Protocol bot SDK.The code properly handles threading:
- Anchor events (opened): sends message with
sendMessage(channelId, message), stores the returnedeventIdasthreadEventIdfor subsequent replies- Follow-up events: retrieves stored
threadEventIdand passes asthreadIdoption tosendMessage(channelId, message, { threadId })- Graceful fallback: if no thread mapping exists, sends as top-level message
This matches the Towns Protocol bot SDK signature where
sendMessagereturns{ eventId }and accepts a{ threadId }option to post within existing threads.
shuhuiluo
force-pushed
the
feat/thread-event-grouping
branch
from
f852948 to
09e156e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/github-app/event-processor.ts (1)
335-344: Missing threading context for PR review comments.
onPullRequestReviewCommentdoesn't includethreadingContext, butonPullRequestReviewdoes (lines 314-328). This inconsistency means review comments won't thread to their parent PR, while reviews will.async onPullRequestReviewComment(event: PullRequestReviewCommentPayload) { const { pull_request, repository } = event; const baseBranch = pull_request.base.ref; await this.processEvent( event, "review_comments", formatPullRequestReviewComment, `PR review comment event: ${event.action} - ${repository.full_name}#${pull_request.number}`, - { branch: baseBranch } + { branch: baseBranch }, + { + anchorType: "pr", + anchorNumber: pull_request.number, + isAnchor: false, // Review comments are never anchors + } ); }
🧹 Nitpick comments (2)
src/index.ts (1)
243-245: Consider storing the timer ID for graceful shutdown.The timer ID returned by
startPeriodicCleanup()is discarded. While this matches the pattern used foroauthService.startOAuthStateCleanup(), storing it would allow stopping the cleanup job during graceful shutdown.+// Store cleanup timer for graceful shutdown +const threadCleanupTimer = threadService.startPeriodicCleanup(); -threadService.startPeriodicCleanup(); console.log("✅ Thread cleanup service started (daily cleanup)");src/services/thread-service.ts (1)
22-44: Consider filtering out expired threads in getThreadId.The query doesn't check
expiresAt, so it could return expired thread IDs if cleanup hasn't run yet. While cleanup runs at startup, there's still a window during the 24h interval where expired entries may persist.const results = await db .select({ threadEventId: eventThreads.threadEventId }) .from(eventThreads) .where( and( eq(eventThreads.spaceId, spaceId), eq(eventThreads.channelId, channelId), eq(eventThreads.repoFullName, repoFullName), eq(eventThreads.anchorType, anchorType), - eq(eventThreads.anchorNumber, anchorNumber) + eq(eventThreads.anchorNumber, anchorNumber), + // Exclude expired threads + lt(new Date(), eventThreads.expiresAt) ) ) .limit(1);Note: You'll need to import or use the appropriate comparison. Since
ltcheckscolumn < value, you'd needgte(greater than or equal) forexpiresAt >= now:-import { and, eq, lt } from "drizzle-orm"; +import { and, eq, gte, lt } from "drizzle-orm";And in the query:
+ gte(eventThreads.expiresAt, new Date())
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
drizzle/0006_certain_micromacro.sql(1 hunks)drizzle/meta/0006_snapshot.json(1 hunks)drizzle/meta/_journal.json(1 hunks)src/db/schema.ts(1 hunks)src/github-app/event-processor.ts(9 hunks)src/index.ts(3 hunks)src/services/subscription-service.ts(3 hunks)src/services/thread-service.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- drizzle/meta/_journal.json
- drizzle/0006_certain_micromacro.sql
- src/db/schema.ts
- drizzle/meta/0006_snapshot.json
- src/services/subscription-service.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Store context externally - maintain stateless bot architecture with no message history, thread context, or conversation memory
Use<@{userId}>for mentions in messages AND add mentions in sendMessage options - do not use @username format
Implement event handlers for onMessage, onSlashCommand, onReaction, onTip, and onInteractionResponse to respond to Towns Protocol events
Define slash commands in src/commands.ts as a const array with name and description properties, then register handlers using bot.onSlashCommand()
Set ID in interaction requests and match ID in responses to correlate form submissions, button clicks, and transaction/signature responses
Use readContract for reading smart contract state, writeContract for SimpleAccount operations, and execute() for external contract interactions
Fund bot.appAddress (Smart Account) for on-chain operations, not bot.botId (Gas Wallet/EOA)
Use bot.* handler methods directly (outside event handlers) for unprompted messages via webhooks, timers, or tasks - requires channelId, spaceId, or other context stored externally
Always check permissions using handler.hasAdminPermission() before performing admin operations like ban, redact, or pin
User IDs are hex addresses in format 0x..., not usernames - use them consistently throughout event handling and message sending
Slash commands do not trigger onMessage - register slash command handlers using bot.onSlashCommand() instead
Use getSmartAccountFromUserId() to retrieve a user's wallet address from their userId
Include required environment variables: APP_PRIVATE_DATA (bot credentials) and JWT_SECRET (webhook security token)
Files:
src/services/thread-service.tssrc/github-app/event-processor.tssrc/index.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Provide alt text for image attachments and use appropriate MIME types for chunked attachments (videos, screenshots)
Files:
src/services/thread-service.tssrc/github-app/event-processor.tssrc/index.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: HereNotThere/bot-github PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:24:12.463Z
Learning: All event handlers receive a base payload including userId, spaceId, channelId, eventId, and createdAt - use eventId as threadId/replyId when responding to maintain event threading
📚 Learning: 2025-11-25T03:24:12.463Z
Learnt from: CR
Repo: HereNotThere/bot-github PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:24:12.463Z
Learning: Applies to **/*.ts : Use bot.* handler methods directly (outside event handlers) for unprompted messages via webhooks, timers, or tasks - requires channelId, spaceId, or other context stored externally
Applied to files:
src/github-app/event-processor.ts
🧬 Code graph analysis (2)
src/services/thread-service.ts (2)
src/db/index.ts (1)
db(57-57)src/db/schema.ts (1)
eventThreads(237-272)
src/index.ts (2)
src/services/thread-service.ts (1)
ThreadService(17-146)src/github-app/event-processor.ts (1)
EventProcessor(54-394)
🔇 Additional comments (8)
src/index.ts (1)
52-60: LGTM! ThreadService integration is well-structured.The ThreadService is correctly instantiated and injected into EventProcessor, following the established dependency injection pattern used for other services.
src/github-app/event-processor.ts (4)
39-46: LGTM! Clean interface for threading context.The
ThreadingContextinterface clearly separates anchor events (that start threads) from follow-up events (that reply to threads).
143-193: Solid threading implementation with proper error isolation.The per-channel try/catch ensures one channel's failure doesn't affect others, and
Promise.allSettledcorrectly awaits all sends. The fallback to top-level messages when no thread is found handles the case where anchor events occurred before the bot joined.
201-215: LGTM! PR threading correctly identifies opened events as anchors.The
action === "opened"check properly distinguishes anchor events from follow-ups like reviews, comments, and state changes.
290-306: Good detection of PR vs issue comments.The check for
"pull_request" in issue && issue.pull_request != nullcorrectly handles GitHub's behavior of firingissue_commentfor both issues and PRs, ensuring comments thread to the appropriate anchor type.src/services/thread-service.ts (3)
64-90: LGTM! Upsert logic handles re-opened PRs/issues correctly.The
onConflictDoUpdateensures that if a PR or issue is closed and reopened, the thread mapping is refreshed with a newthreadEventIdand extendedexpiresAt.
96-103: LGTM! Efficient cleanup with indexed query.The cleanup uses the
expiresIndexdefined in the schema for efficient deletion, andreturning()provides the count without a separate query.
118-129: Initial cleanup is fire-and-forget by design.The initial cleanup runs asynchronously without blocking startup, which is reasonable for a non-critical operation. Errors are properly caught and logged.
shuhuiluo
force-pushed
the
feat/thread-event-grouping
branch
from
679224e to
93c483d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/github-app/event-processor.ts (1)
335-345: Missing threading context for PR review comments.
onPullRequestReviewCommentshould thread to the parent PR, similar toonPullRequestReview. This appears to be an oversight since review comments are logically part of the PR discussion.Apply this diff to add threading support:
async onPullRequestReviewComment(event: PullRequestReviewCommentPayload) { const { pull_request, repository } = event; const baseBranch = pull_request.base.ref; await this.processEvent( event, "review_comments", formatPullRequestReviewComment, `PR review comment event: ${event.action} - ${repository.full_name}#${pull_request.number}`, - { branch: baseBranch } + { branch: baseBranch }, + { + anchorType: "pr", + anchorNumber: pull_request.number, + isAnchor: false, // Review comments are never anchors + } ); }
🧹 Nitpick comments (2)
src/db/schema.ts (1)
263-268: Consider removing the redundantrepoAnchorIndex.The
repoAnchorIndexon(repoFullName, anchorType, anchorNumber)appears to be redundant. The primary query inThreadService.getThreadIduses all five columns(spaceId, channelId, repoFullName, anchorType, anchorNumber), which is already covered byuniqueThread. There doesn't appear to be a query pattern that would benefit from this partial index.If there's a planned use case for looking up threads across all spaces/channels for a given repo anchor, this index would be useful. Otherwise, it adds write overhead without query benefit.
src/services/thread-service.ts (1)
112-146: Consider structured logging for observability.The cleanup logic is correct with proper error handling. For production observability, consider using structured logging with key-value pairs rather than template strings, which would make it easier to query and aggregate in logging systems.
-console.log( - `[Thread Cleanup] Starting periodic cleanup (every ${intervalMs / 1000 / 60 / 60} hours)` -); +console.log("[Thread Cleanup] Starting periodic cleanup", { + intervalHours: intervalMs / 1000 / 60 / 60, +});
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
drizzle/0006_certain_micromacro.sql(1 hunks)drizzle/meta/0006_snapshot.json(1 hunks)drizzle/meta/_journal.json(1 hunks)src/db/schema.ts(1 hunks)src/github-app/event-processor.ts(9 hunks)src/index.ts(3 hunks)src/services/subscription-service.ts(3 hunks)src/services/thread-service.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/services/subscription-service.ts
- drizzle/meta/0006_snapshot.json
- drizzle/0006_certain_micromacro.sql
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Store context externally - maintain stateless bot architecture with no message history, thread context, or conversation memory
Use<@{userId}>for mentions in messages AND add mentions in sendMessage options - do not use @username format
Implement event handlers for onMessage, onSlashCommand, onReaction, onTip, and onInteractionResponse to respond to Towns Protocol events
Define slash commands in src/commands.ts as a const array with name and description properties, then register handlers using bot.onSlashCommand()
Set ID in interaction requests and match ID in responses to correlate form submissions, button clicks, and transaction/signature responses
Use readContract for reading smart contract state, writeContract for SimpleAccount operations, and execute() for external contract interactions
Fund bot.appAddress (Smart Account) for on-chain operations, not bot.botId (Gas Wallet/EOA)
Use bot.* handler methods directly (outside event handlers) for unprompted messages via webhooks, timers, or tasks - requires channelId, spaceId, or other context stored externally
Always check permissions using handler.hasAdminPermission() before performing admin operations like ban, redact, or pin
User IDs are hex addresses in format 0x..., not usernames - use them consistently throughout event handling and message sending
Slash commands do not trigger onMessage - register slash command handlers using bot.onSlashCommand() instead
Use getSmartAccountFromUserId() to retrieve a user's wallet address from their userId
Include required environment variables: APP_PRIVATE_DATA (bot credentials) and JWT_SECRET (webhook security token)
Files:
src/index.tssrc/db/schema.tssrc/github-app/event-processor.tssrc/services/thread-service.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Provide alt text for image attachments and use appropriate MIME types for chunked attachments (videos, screenshots)
Files:
src/index.tssrc/db/schema.tssrc/github-app/event-processor.tssrc/services/thread-service.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: HereNotThere/bot-github PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:24:12.463Z
Learning: All event handlers receive a base payload including userId, spaceId, channelId, eventId, and createdAt - use eventId as threadId/replyId when responding to maintain event threading
📚 Learning: 2025-11-25T03:24:12.463Z
Learnt from: CR
Repo: HereNotThere/bot-github PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:24:12.463Z
Learning: All event handlers receive a base payload including userId, spaceId, channelId, eventId, and createdAt - use eventId as threadId/replyId when responding to maintain event threading
Applied to files:
src/db/schema.ts
📚 Learning: 2025-11-25T03:24:12.463Z
Learnt from: CR
Repo: HereNotThere/bot-github PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:24:12.463Z
Learning: Applies to **/*.ts : Use bot.* handler methods directly (outside event handlers) for unprompted messages via webhooks, timers, or tasks - requires channelId, spaceId, or other context stored externally
Applied to files:
src/github-app/event-processor.ts
🧬 Code graph analysis (3)
src/index.ts (2)
src/services/thread-service.ts (1)
ThreadService(17-147)src/github-app/event-processor.ts (1)
EventProcessor(54-394)
src/github-app/event-processor.ts (1)
src/services/thread-service.ts (2)
AnchorType(6-6)ThreadService(17-147)
src/services/thread-service.ts (2)
src/db/index.ts (1)
db(57-57)src/db/schema.ts (1)
eventThreads(237-272)
🪛 ESLint
src/services/thread-service.ts
[error] 1-1: Unable to resolve path to module 'drizzle-orm'.
(import-x/no-unresolved)
🔇 Additional comments (11)
drizzle/meta/_journal.json (1)
46-53: LGTM!The new migration journal entry follows the established pattern and correctly appends the
0006_certain_micromacromigration for theevent_threadstable.src/index.ts (2)
52-60: LGTM!The ThreadService integration follows the established patterns in the codebase. The service is properly instantiated and injected into the EventProcessor, consistent with how other services are wired.
243-245: LGTM!The periodic cleanup startup mirrors the OAuth cleanup pattern and provides appropriate logging. The daily interval aligns with the 30-day expiry strategy.
src/db/schema.ts (1)
232-272: LGTM - well-designed schema for thread mappings.The table structure correctly models the threading requirement with appropriate constraints. The composite unique index on
(spaceId, channelId, repoFullName, anchorType, anchorNumber)aligns with theThreadService.getThreadIdquery pattern. Based on learnings, usingthreadEventIdto store the TownseventIdis the correct approach for thread replies.src/services/thread-service.ts (3)
1-9: LGTM!Clean imports and well-named constants. The
AnchorTypeexport provides type safety for threading context. The static analysis hint aboutdrizzle-ormis a false positive—the module is a valid project dependency.
22-45: LGTM - correct query implementation.The
getThreadIdmethod properly filters by all composite key columns and excludes expired threads with thegte(expiresAt, now)check. Usinglimit(1)is appropriate since the unique index guarantees at most one result.
78-90: Verify upsert behavior preservescreatedAt.The
onConflictDoUpdateonly updatesthreadEventIdandexpiresAt, which meanscreatedAtis preserved on re-opened PRs/issues. This appears intentional (keeps the original thread creation time), but worth confirming this is the desired behavior.Additionally, the expiry extension on conflict means a re-opened PR/issue refreshes the 30-day window—this seems reasonable for active threads.
src/github-app/event-processor.ts (4)
39-46: LGTM - clean interface definition.The
ThreadingContextinterface clearly models the threading behavior withisAnchordistinguishing between thread-starting and follow-up events.
143-193: LGTM - well-implemented threading logic.The per-channel send flow correctly:
- Sends anchor events as top-level messages and stores thread mappings
- Looks up existing threads for follow-up events
- Falls back to top-level messages gracefully when no thread exists
- Uses
Promise.allSettledto ensure one channel failure doesn't block othersThe error handling is appropriate for production use.
201-216: LGTM - correct anchor detection for PRs.Using
action === "opened"correctly identifies when to start a new thread. Follow-up actions (merged, closed, synchronize, etc.) will reply to the existing thread.
290-307: LGTM - smart detection of PR vs issue comments.Correctly uses the presence of
issue.pull_requestto determine the anchor type, since GitHub firesissue_commentevents for both issues and PRs.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/github-app/event-processor.ts (1)
324-333: Consider threading PR review comments to the parent PR as well
onPullRequestReviewCommentcurrently callsprocessEventwithout athreadingContext, so review comment notifications will always appear as top‑level messages, even when a thread exists for the parent PR.If you want all PR‑related follow‑ups grouped (as with reviews and issue comments), consider passing a threading context here too:
async onPullRequestReviewComment(event: PullRequestReviewCommentPayload) { const { pull_request, repository } = event; const baseBranch = pull_request.base.ref; await this.processEvent( event, "review_comments", formatPullRequestReviewComment, `PR review comment event: ${event.action} - ${repository.full_name}#${pull_request.number}`, - { branch: baseBranch } + { branch: baseBranch }, + { + anchorType: "pr", + anchorNumber: pull_request.number, + isAnchor: false, // review comments are follow-ups + } ); }This would keep review comments in the same Towns thread as the PR and its other follow‑up events.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/github-app/event-processor.ts(9 hunks)src/services/thread-service.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Store context externally - maintain stateless bot architecture with no message history, thread context, or conversation memory
Use<@{userId}>for mentions in messages AND add mentions in sendMessage options - do not use @username format
Implement event handlers for onMessage, onSlashCommand, onReaction, onTip, and onInteractionResponse to respond to Towns Protocol events
Define slash commands in src/commands.ts as a const array with name and description properties, then register handlers using bot.onSlashCommand()
Set ID in interaction requests and match ID in responses to correlate form submissions, button clicks, and transaction/signature responses
Use readContract for reading smart contract state, writeContract for SimpleAccount operations, and execute() for external contract interactions
Fund bot.appAddress (Smart Account) for on-chain operations, not bot.botId (Gas Wallet/EOA)
Use bot.* handler methods directly (outside event handlers) for unprompted messages via webhooks, timers, or tasks - requires channelId, spaceId, or other context stored externally
Always check permissions using handler.hasAdminPermission() before performing admin operations like ban, redact, or pin
User IDs are hex addresses in format 0x..., not usernames - use them consistently throughout event handling and message sending
Slash commands do not trigger onMessage - register slash command handlers using bot.onSlashCommand() instead
Use getSmartAccountFromUserId() to retrieve a user's wallet address from their userId
Include required environment variables: APP_PRIVATE_DATA (bot credentials) and JWT_SECRET (webhook security token)
Files:
src/github-app/event-processor.tssrc/services/thread-service.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Provide alt text for image attachments and use appropriate MIME types for chunked attachments (videos, screenshots)
Files:
src/github-app/event-processor.tssrc/services/thread-service.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: HereNotThere/bot-github PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:24:12.463Z
Learning: All event handlers receive a base payload including userId, spaceId, channelId, eventId, and createdAt - use eventId as threadId/replyId when responding to maintain event threading
📚 Learning: 2025-11-25T03:24:12.463Z
Learnt from: CR
Repo: HereNotThere/bot-github PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:24:12.463Z
Learning: Applies to **/*.ts : Use bot.* handler methods directly (outside event handlers) for unprompted messages via webhooks, timers, or tasks - requires channelId, spaceId, or other context stored externally
Applied to files:
src/github-app/event-processor.ts
🧬 Code graph analysis (2)
src/github-app/event-processor.ts (2)
src/services/thread-service.ts (2)
AnchorType(6-6)ThreadService(17-142)src/types/webhooks.ts (3)
PullRequestPayload(14-14)IssuesPayload(15-15)IssueCommentPayload(19-19)
src/services/thread-service.ts (2)
src/db/index.ts (1)
db(57-57)src/db/schema.ts (1)
eventThreads(237-272)
🪛 ESLint
src/services/thread-service.ts
[error] 1-1: Unable to resolve path to module 'drizzle-orm'.
(import-x/no-unresolved)
🔇 Additional comments (5)
src/services/thread-service.ts (3)
1-1: Resolvedrizzle-ormimport lint errorESLint is flagging
import { and, eq, gte, lt } from "drizzle-orm";as unresolved. Please confirm that:
drizzle-ormis present independencies(not onlydevDependenciesif this code runs in production), and- your TS/eslint/module resolver config is set up so this bare import can be resolved.
Otherwise CI/lint may fail even though the runtime dependency is installed.
22-86: Thread mapping queries and upsert look correctThe
getThreadIdfilter (includingexpiresAt >= now) andstoreThreadupsert keyed on(spaceId, channelId, repoFullName, anchorType, anchorNumber)line up with theevent_threadsschema and unique index. This should give deterministic per-space/channel threading and safe overwrites when an anchor is resent, without leaving expired mappings in use.
92-141: Periodic cleanup implementation is safe and non‑blocking
cleanupExpired()withexpiresAt < nowcombined withstartPeriodicCleanup()’s immediate run andsetIntervalloop looks solid: errors are contained to logging, and deletions are bounded by the indexedexpiresAtcolumn. No changes needed here.src/github-app/event-processor.ts (2)
39-46: Threading integration inprocessEventis coherent and robustThe
ThreadingContext+ per‑channelthreadService.getThreadId/storeThreadflow is well factored:
- Anchors (
isAnchor=true) always post top‑level and persist the returnedeventIdas the thread key per(spaceId, channelId, repo, anchorType, anchorNumber).- Follow‑ups attempt thread lookup and gracefully fall back to top‑level when no mapping exists or it has expired.
- Branch filtering and subscription filtering remain unchanged, and
Promise.allSettledsemantics are preserved via thesendPromisesarray and per‑channeltry/catch.This aligns with the “store context externally and use event IDs as thread IDs” design described in the repo docs.
Based on learnings, this keeps the bot stateless while using externally stored thread context.
Also applies to: 80-89, 102-183
190-205: Anchor/follow‑up threading for PRs/issues/comments is consistentDeriving the threading context from the webhook payloads:
- PRs:
anchorType: "pr",anchorNumber: pull_request.number,isAnchor: action === "opened".- Issues:
anchorType: "issue",anchorNumber: issue.number,isAnchor: action === "opened".- Issue comments: detect PR vs issue via
issue.pull_requestand always treat asisAnchor: false.This should ensure all PR/issue lifecycle events, comments, and reviews (via
onPullRequestReview) correctly attach to the same thread started by theopenedevent when present, while still working sensibly when a channel only subscribes to a subset of event types.Also applies to: 228-241, 279-295, 303-317
Group PR and issue events into threads: - PR/issue opened creates anchor thread - Follow-up events (comments, reviews, closed) reply to thread - 30-day thread expiration with daily cleanup - Filter expired threads in lookups Changes: - Add event_threads table for thread mappings - Add ThreadService for storage/lookup - Update EventProcessor with threading context - Include PR review comments in threading 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
shuhuiluo
force-pushed
the
feat/thread-event-grouping
branch
from
0d5aec5 to
a421e99
Compare
Group related GitHub events into Towns threads to reduce channel noise:
Implementation:
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.