feat: count videos/tags within thread replies#409
feat: count videos/tags within thread replies#409recoup-coding-agent wants to merge 1 commit intotestfrom
Conversation
Previously, fetchBotMentions only scanned conversations.history (top-level messages) for bot mentions. This missed tags and videos from users who mentioned the bot inside thread replies. Now the function also fetches conversations.replies for threaded messages and captures mentions there, using the parent thread_ts for video link extraction. Co-Authored-By: Paperclip <noreply@paperclip.ing>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 17 minutes and 32 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 2 files
Confidence score: 3/5
- There is a concrete regression risk in
lib/admins/slack/fetchBotMentions.ts:topLevelMentionThreadsis written but never read, which suggests the deduplication flow is incomplete and could allow duplicate mention handling. - The
userCacheaccess pattern inside concurrentPromise.allSettledwork inlib/admins/slack/fetchBotMentions.tscan trigger duplicategetSlackUserInfocalls for the same uncached user; this is likely non-fatal but adds correctness/performance uncertainty. - Given one high-severity, high-confidence logic issue plus a smaller concurrency concern, this looks like moderate merge risk rather than a hard block.
- Pay close attention to
lib/admins/slack/fetchBotMentions.ts- deduplication and cache behavior may cause duplicate mentions or redundant Slack user lookups.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/admins/slack/fetchBotMentions.ts">
<violation number="1" location="lib/admins/slack/fetchBotMentions.ts:137">
P1: Custom agent: **Flag AI Slop and Fabricated Changes**
`topLevelMentionThreads` is populated but never read — it's dead code. This appears to be an incomplete deduplication mechanism: mentions that appear both at the top level and inside thread replies will be counted twice. Either implement the deduplication check (e.g., skip thread-reply mentions whose `channelId:ts` is in this set) or remove the variable.</violation>
<violation number="2" location="lib/admins/slack/fetchBotMentions.ts:171">
P2: Race condition on `userCache` within the concurrent `Promise.allSettled` batch. If two threads in the same batch contain replies from the same uncached user, both will call `getSlackUserInfo` before either populates the cache. Consider pre-populating the cache or serializing the user lookups to avoid redundant Slack API calls that count against rate limits.</violation>
</file>
<file name="lib/admins/slack/__tests__/fetchBotMentions.test.ts">
<violation number="1" location="lib/admins/slack/__tests__/fetchBotMentions.test.ts:32">
P3: `vi.resetAllMocks()` deviates from the project convention (`vi.clearAllMocks()`) documented in CLAUDE.md and used across all other test files. If stronger isolation is intentionally desired here, consider updating the project convention; otherwise, revert to `vi.clearAllMocks()` for consistency.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Admin as Admin /content Page
participant Service as fetchBotMentions
participant Slack as Slack API
participant Extractor as fetchThreadResponses
Note over Admin,Extractor: Runtime flow for fetching bot usage metrics
Admin->>Service: fetchBotMentions(period)
loop For each channel
Service->>Slack: conversations.history
Slack-->>Service: List of messages
Service->>Service: Filter top-level bot mentions
Service->>Service: NEW: Identify messages with reply_count > 0
end
Note over Service,Slack: NEW: Scan Thread Replies (Rate Limited)
loop In batches of 5 (1.1s delay)
Service->>Slack: NEW: conversations.replies (channel, thread_ts)
Slack-->>Service: Thread message list
Service->>Service: NEW: Find bot mentions inside thread
opt Mentioning user not in cache
Service->>Slack: users.info
Slack-->>Service: User profile
end
end
Note over Service,Extractor: Extract Bot Content (Videos/Tags)
Service->>Extractor: CHANGED: fetchThreadResponses(token, targetTs)
Note right of Service: targetTs is now parent thread_ts<br/>for mentions inside threads
loop For each mention
Extractor->>Slack: conversations.replies (targetTs)
Slack-->>Extractor: Bot responses in thread
Extractor->>Extractor: Parse video URLs and tags
end
Extractor-->>Service: content data (links/tags)
Service-->>Admin: Array of BotTag objects (sorted newest first)
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| channelName: channel.name, | ||
| }); | ||
|
|
||
| topLevelMentionThreads.add(`${channel.id}:${msg.ts}`); |
There was a problem hiding this comment.
P1: Custom agent: Flag AI Slop and Fabricated Changes
topLevelMentionThreads is populated but never read — it's dead code. This appears to be an incomplete deduplication mechanism: mentions that appear both at the top level and inside thread replies will be counted twice. Either implement the deduplication check (e.g., skip thread-reply mentions whose channelId:ts is in this set) or remove the variable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/admins/slack/fetchBotMentions.ts, line 137:
<comment>`topLevelMentionThreads` is populated but never read — it's dead code. This appears to be an incomplete deduplication mechanism: mentions that appear both at the top level and inside thread replies will be counted twice. Either implement the deduplication check (e.g., skip thread-reply mentions whose `channelId:ts` is in this set) or remove the variable.</comment>
<file context>
@@ -96,18 +129,72 @@ export async function fetchBotMentions(options: FetchBotMentionsOptions): Promis
channelName: channel.name,
});
+
+ topLevelMentionThreads.add(`${channel.id}:${msg.ts}`);
}
</file context>
| // Skip the parent message (already captured as top-level) | ||
| if (msg.ts === tp.ts) continue; | ||
|
|
||
| if (!userCache[msg.user]) { |
There was a problem hiding this comment.
P2: Race condition on userCache within the concurrent Promise.allSettled batch. If two threads in the same batch contain replies from the same uncached user, both will call getSlackUserInfo before either populates the cache. Consider pre-populating the cache or serializing the user lookups to avoid redundant Slack API calls that count against rate limits.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/admins/slack/fetchBotMentions.ts, line 171:
<comment>Race condition on `userCache` within the concurrent `Promise.allSettled` batch. If two threads in the same batch contain replies from the same uncached user, both will call `getSlackUserInfo` before either populates the cache. Consider pre-populating the cache or serializing the user lookups to avoid redundant Slack API calls that count against rate limits.</comment>
<file context>
@@ -96,18 +129,72 @@ export async function fetchBotMentions(options: FetchBotMentionsOptions): Promis
+ // Skip the parent message (already captured as top-level)
+ if (msg.ts === tp.ts) continue;
+
+ if (!userCache[msg.user]) {
+ userCache[msg.user] = await getSlackUserInfo(token, msg.user);
+ }
</file context>
| describe("fetchBotMentions", () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| vi.resetAllMocks(); |
There was a problem hiding this comment.
P3: vi.resetAllMocks() deviates from the project convention (vi.clearAllMocks()) documented in CLAUDE.md and used across all other test files. If stronger isolation is intentionally desired here, consider updating the project convention; otherwise, revert to vi.clearAllMocks() for consistency.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/admins/slack/__tests__/fetchBotMentions.test.ts, line 32:
<comment>`vi.resetAllMocks()` deviates from the project convention (`vi.clearAllMocks()`) documented in CLAUDE.md and used across all other test files. If stronger isolation is intentionally desired here, consider updating the project convention; otherwise, revert to `vi.clearAllMocks()` for consistency.</comment>
<file context>
@@ -29,7 +29,7 @@ vi.mock("@/lib/admins/slack/getCutoffTs", () => ({
describe("fetchBotMentions", () => {
beforeEach(() => {
- vi.clearAllMocks();
+ vi.resetAllMocks();
});
</file context>
| vi.resetAllMocks(); | |
| vi.clearAllMocks(); |
1 participant
Summary
/contentpage missing videos and tags from Slack thread repliesfetchBotMentionsnow scansconversations.repliesfor threaded messages (those withreply_count > 0) to find bot mentions inside threadsthread_tsfor video link extraction, ensuring bot responses in the same thread are capturedTest plan
/contentpage tag/video counts before and after deployment🤖 Generated with Claude Code
Summary by cubic
Include bot mentions inside Slack thread replies in
/contentcounts, fixing missing videos and tags. Addresses Linear REC-54 by scanning thread replies and using the parent thread timestamp for response extraction.fetchBotMentionsto scanconversations.repliesfor parents withreply_count > 0.thread_tswhen extracting video links so bot responses in the same thread are captured.Written for commit 980a71a. Summary will update on new commits.