refactor: unify ThreadingContext and EntityContext

[shuhuiluo]

• Migrate eventThreads table data into messageMappings and drop it
• Replace ThreadingContext with unified EntityContext (isAnchor, parentType, parentNumber)
• Consolidate getAnchorMessageId into getMessageId
• Move cleanup logging into cleanupExpired and make it private
• Remove duplicate code in startPeriodicCleanup

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

• Chores
  • Refactored the message mapping and threading system to use a unified GitHub entity schema, consolidating how messages are tracked for GitHub pull requests, issues, and comments.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The changes implement a migration from an event-threading model to a unified message-mapping model. The event_threads table is migrated to message_mappings using a SQL migration with upsert logic, then dropped. The codebase is refactored to remove ThreadingContext and related threading infrastructure, replacing anchor-based identifiers with GitHub entity types and IDs, while using parent type/number to express resource context. This affects the event processor, message delivery service, and database schema.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

• Migration correctness: The SQL upsert logic on composite keys (space_id, channel_id, repo_full_name, github_entity_type, github_entity_id) must be verified to avoid data loss or duplication during the event_threads → message_mappings migration.
• Type system changes: Removal of ThreadingContext and ParentType, plus updates to EntityContext and DeliveryParams interfaces represent breaking API changes that require tracing all callsites.
• Message delivery flow refactoring: The thread ID calculation and message creation logic now derive threading context solely from EntityContext fields (isAnchor, parentType); this logic density and scope change requires careful verification.
• Data persistence logic: Unified storeMapping call replaces prior separate storeThread plus storeMapping calls; the new signature and onConflict behavior changes need validation.
• Cleanup behavior: cleanupExpired is now private and targets only messageMappings; periodic scheduling behavior changes (no await, no return counts) should be verified against intended cleanup semantics.

Possibly related PRs

• PR #75: Directly related—introduced the event_threads table and ThreadingContext that this PR removes and replaces.
• PR #86: Directly related—implemented message_mappings table and delivery logic that this PR now unifies with the removed threading model.
• PR #70: Shares code-level changes to src/github-app/event-processor.ts event handler signatures and payload structures.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title 'refactor: unify ThreadingContext and EntityContext' accurately summarizes the main objective—consolidating two context types into one unified EntityContext.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch refactor/unify-entity-context

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f141837 and c92812f.

📒 Files selected for processing (6)

• drizzle/0008_bouncy_jack_power.sql (1 hunks)
• drizzle/meta/0008_snapshot.json (1 hunks)
• drizzle/meta/_journal.json (1 hunks)
• src/db/schema.ts (0 hunks)
• src/github-app/event-processor.ts (5 hunks)
• src/services/message-delivery-service.ts (8 hunks)

💤 Files with no reviewable changes (1)

• src/db/schema.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/github-app/event-processor.ts
• src/services/message-delivery-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.ts
• src/services/message-delivery-service.ts

🧠 Learnings (3)

📚 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/0008_snapshot.json

📚 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/github-app/event-processor.ts
• src/services/message-delivery-service.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/services/message-delivery-service.ts

🧬 Code graph analysis (1)

src/services/message-delivery-service.ts (2)

src/db/index.ts (1)

• db (57-57)

src/db/schema.ts (1)

• messageMappings (237-282)

🔇 Additional comments (13)

drizzle/meta/_journal.json (1)

60-67: LGTM!

The journal entry correctly registers the new migration with proper index sequencing, version, and breakpoints configuration.

src/github-app/event-processor.ts (5)

142-149: LGTM!

The PR event correctly sets githubEntityType to "pr" and uses the PR number as githubEntityId. Setting parentType/parentNumber to the PR's own values is appropriate since anchors serve as thread roots that child entities (comments, reviews) will reference.

---

183-190: LGTM!

The issue event handler correctly mirrors the PR handler pattern with appropriate entity type and parent context.

---

245-253: LGTM!

Issue comments correctly set isAnchor: false and determine parentType based on whether the comment is on a PR or issue. The githubUpdatedAt timestamp enables stale update detection.

---

277-285: LGTM!

PR review handler correctly threads to the parent PR with appropriate entity context.

---

305-313: LGTM!

PR review comment handler follows the established pattern for child entities.

drizzle/meta/0008_snapshot.json (1)

424-538: LGTM!

The message_mappings table snapshot correctly reflects the unified schema with:

• Composite primary key on (space_id, channel_id, repo_full_name, github_entity_type, github_entity_id)
• Nullable parent_type/parent_number for threading
• Appropriate check constraints for entity_type and parent_type
• Expiry index for cleanup queries

The event_threads table is correctly absent from the snapshot.

src/services/message-delivery-service.ts (5)

17-28: Clean interface design.

The unified EntityContext interface elegantly captures both anchor entities (PRs/issues that start threads) and child entities (comments/reviews that reply to threads) through the isAnchor flag, replacing the previous dual-context approach.

---

66-79: LGTM!

The thread lookup logic correctly resolves the parent anchor's message ID for non-anchor entities. The parentType and parentNumber are used to find the PR/issue message that child entities should thread under.

---

236-249: LGTM!

The simplified handleCreate correctly stores entity mappings for all contextual messages, enabling both edit/delete operations and thread parent lookups.

---

382-400: LGTM!

Making cleanupExpired private is appropriate since it's only invoked internally. The added logging for removed records improves observability without verbose output when nothing is cleaned.

---

405-416: LGTM!

The startPeriodicCleanup correctly uses fire-and-forget (void) for both initial and periodic cleanup, preventing the caller from blocking on cleanup operations.

drizzle/0008_bouncy_jack_power.sql (1)

1-15: Migration logic is correct and fully compatible.

The migration properly transfers thread data by mapping anchor_type → github_entity_type and anchor_number::text → github_entity_id. The ON CONFLICT DO NOTHING guard prevents overwriting existing mappings, and CASCADE ensures dependent objects are handled.

The anchor_type column in event_threads is constrained to only ('pr', 'issue') values, which are a valid subset of the entity_type_check constraint in message_mappings ('pr', 'issue', 'comment', 'review', 'review_comment'). Therefore, no constraint violations are possible.

Note that parent_type and parent_number columns will be NULL for migrated rows, which is acceptable since these were anchor threads (PRs/issues) that don't have parents.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}