Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces direct role-adding in thread notifications with a pre-check and messaging flow. Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/jg/chick/lib/threads.py`:
- Around line 91-95: The current mentions list builds strings like
f"@{member.id}" which Discord won't convert to user pings; update the
construction of mentions (the comprehension referencing role.members and
thread_members_ids) to use the proper Discord mention format by wrapping IDs in
angle brackets like "<@USER_ID>" (i.e., produce "<@{member.id}>" for each
member) and ensure this updated mentions list is used when sending the thread
message so users are actually pinged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 301969b3-5db5-4ca8-8929-14e50115a588
📒 Files selected for processing (2)
src/jg/chick/bot.pysrc/jg/chick/lib/threads.py
There was a problem hiding this comment.
Pull request overview
This PR changes how interest-role members are brought into Discord threads by switching from explicitly adding users to sending a message that mentions the relevant users, and adds a cleanup step to remove recent bot messages before notifying again.
Changes:
- Replace per-user
thread.add_user(...)with a single message containing per-user “mentions” inadd_members_with_role() - Add
clear_bot_messages()helper that purges recent bot-authored messages in a thread - Call
clear_bot_messages()before adding/notifying members in the interest-thread message handler
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
src/jg/chick/lib/threads.py |
Reworks interest-thread member “adding” to mention-based messaging; introduces bot-message purge helper |
src/jg/chick/bot.py |
Hooks bot-message cleanup into interest-thread notification flow |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/jg/chick/lib/threads.py (1)
80-91:⚠️ Potential issue | 🟠 Major
thread.membersis incomplete due to caching and may cause members already in the thread to be re-mentioned.
thread.membersis a cache-only property that Discord doesn't always populate over the gateway. This can result in an incomplete list, causing members who are already in the thread to be incorrectly included inget_missing_members().Use
await thread.fetch_members()to retrieve the complete list from the Discord API. This requires converting the function to async, which is compatible with the existing async calling context.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/jg/chick/lib/threads.py` around lines 80 - 91, get_missing_members currently uses the cache-only property thread.members which can be incomplete; convert get_missing_members to an async function and call await thread.fetch_members() (or use the returned list) to obtain the full member list before computing thread_members_ids, then proceed to compare against role.members; update the signature to async def get_missing_members(thread: discord.Thread, role_id: int) -> list[discord.Member] and ensure callers await it.
🧹 Nitpick comments (2)
src/jg/chick/bot.py (2)
225-227: Bot messages cleared unconditionally before checking if notification is needed.
clear_bot_messagesis called before determining whether there are enough missing members to notify. Iflen(missing_members) <= 1, the bot's previous messages are deleted but no new message is sent, leaving the thread without the bot's context message.Consider moving the clear operation after the member count check, or only clearing when a new message will be sent.
♻️ Proposed reordering
if interests.should_notify(interest, now): - logger.info("Clearing recent bot messages") - await clear_bot_messages(thread) missing_members = get_missing_members(thread, interest["role_id"]) if len(missing_members) <= 1: logger.info(f"Not adding, too few: {len(missing_members)}") else: + logger.info("Clearing recent bot messages") + await clear_bot_messages(thread) logger.info(f"Adding role #{interest['role_id']}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/jg/chick/bot.py` around lines 225 - 227, The code currently calls clear_bot_messages(thread) before checking missing_members, which can remove the bot's context even when no notification will be sent; update the logic in the function handling the thread so that you call get_missing_members(thread, interest["role_id"]) first, check if len(missing_members) > 1 (or your notification threshold), and only then call clear_bot_messages(thread) immediately before sending the new notification; alternatively, guard the existing clear_bot_messages(thread) call so it runs only when you will send a new message (i.e., when missing_members exceeds the threshold).
225-240:last_notified_atupdated even when no message is sent.When
len(missing_members) <= 1, no mention message is sent, butlast_notified_atis still updated at line 240. This resets the cooldown even though no notification occurred, which may not be the intended behavior.If this is intentional (to avoid repeated checks when there's no one to notify), consider adding a comment. Otherwise, move the update inside the
elseblock.💡 Suggested fix if update should only occur when message is sent
if len(missing_members) <= 1: logger.info(f"Not adding, too few: {len(missing_members)}") else: logger.info(f"Adding role #{interest['role_id']}") mentions_text = " ".join([m.mention for m in missing_members]) adding_message = ( f"{mentions_text} přidávám vás, protože jste si " "v <id:customize> vybrali, že vás zajímá tohle téma. " "Pokud vás to tu přestane bavit, spusťte tady příkaz " "`/unfollow` a já vás odeberu." ) await thread.send(adding_message, silent=True) - interest["last_notified_at"] = now + interest["last_notified_at"] = now🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/jg/chick/bot.py` around lines 225 - 240, The code updates interest["last_notified_at"] regardless of whether a notification was sent; change this to only update when a message is actually sent by moving the interest["last_notified_at"] assignment into the else branch after thread.send (or, if the current behavior is intentional, add a clear comment above the assignment explaining why we set last_notified_at even when get_missing_members returns <= 1). Locate the block using get_missing_members, clear_bot_messages, thread.send, and interest["last_notified_at"] and either move the update inside the else or add the explanatory comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/jg/chick/lib/threads.py`:
- Around line 80-91: get_missing_members currently uses the cache-only property
thread.members which can be incomplete; convert get_missing_members to an async
function and call await thread.fetch_members() (or use the returned list) to
obtain the full member list before computing thread_members_ids, then proceed to
compare against role.members; update the signature to async def
get_missing_members(thread: discord.Thread, role_id: int) ->
list[discord.Member] and ensure callers await it.
---
Nitpick comments:
In `@src/jg/chick/bot.py`:
- Around line 225-227: The code currently calls clear_bot_messages(thread)
before checking missing_members, which can remove the bot's context even when no
notification will be sent; update the logic in the function handling the thread
so that you call get_missing_members(thread, interest["role_id"]) first, check
if len(missing_members) > 1 (or your notification threshold), and only then call
clear_bot_messages(thread) immediately before sending the new notification;
alternatively, guard the existing clear_bot_messages(thread) call so it runs
only when you will send a new message (i.e., when missing_members exceeds the
threshold).
- Around line 225-240: The code updates interest["last_notified_at"] regardless
of whether a notification was sent; change this to only update when a message is
actually sent by moving the interest["last_notified_at"] assignment into the
else branch after thread.send (or, if the current behavior is intentional, add a
clear comment above the assignment explaining why we set last_notified_at even
when get_missing_members returns <= 1). Locate the block using
get_missing_members, clear_bot_messages, thread.send, and
interest["last_notified_at"] and either move the update inside the else or add
the explanatory comment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3dfe4d77-3483-4779-b82a-8fcd1acaab60
📒 Files selected for processing (2)
src/jg/chick/bot.pysrc/jg/chick/lib/threads.py
There was a problem hiding this comment.
Pull request overview
Updates interest-thread notification behavior to use explicit user mentions (and cleanup) rather than directly adding role members to the thread.
Changes:
- Replaces
add_members_with_role()withget_missing_members()to compute which role members aren’t in a thread. - Adds
clear_bot_messages()helper to purge recent bot messages from a thread before notifying. - Updates the interest-thread notification flow to mention missing members and send an explanatory message.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/jg/chick/lib/threads.py |
Introduces helpers for missing-member detection and bot-message cleanup. |
src/jg/chick/bot.py |
Switches interest-thread notification from “add users” to “mention users + message” flow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/jg/chick/lib/threads.py`:
- Around line 97-103: clear_bot_messages currently purges any message authored
by the bot; narrow the purge predicate passed to thread.purge so only bot
messages that are clearly bot-generated notifications are removed (e.g., require
m.author == thread.guild.me AND one of: m.reference resolved message was from
the bot, m.content startswith a configurable marker like "[BOT]", or m.embeds
contains a footer/field identifying it as a bot notification). Update the lambda
in clear_bot_messages (and consider adding an optional predicate parameter to
clear_bot_messages) so callers can specify the marker, and ensure thread.purge
uses that stricter check instead of author-only.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4d2040bc-85da-44b6-be64-5996f7718d66
📒 Files selected for processing (2)
src/jg/chick/bot.pysrc/jg/chick/lib/threads.py
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/jg/chick/lib/threads.py (1)
105-109:⚠️ Potential issue | 🟠 MajorNarrow purge predicate to avoid deleting unrelated bot messages.
check=lambda m: m.author == thread.guild.meis still broad and can remove non-notification bot output in the thread.🧹 Proposed narrowing
async def clear_recent_bot_messages( thread: discord.Thread, limit_count: int = 10, limit_days: int = 60, now: datetime | None = None, + marker: str = "přidávám vás, protože", ) -> None: """Clears messages sent by the bot in given thread""" after = (now or datetime.now(UTC)) - timedelta(days=limit_days) await thread.purge( limit=limit_count, after=after, - check=lambda m: m.author == thread.guild.me, + check=lambda m: m.author == thread.guild.me and marker in m.content, reason="Clearing recent bot messages", )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/jg/chick/lib/threads.py` around lines 105 - 109, The purge predicate is too broad (check=lambda m: m.author == thread.guild.me) and may delete unrelated bot messages; replace it with a narrower predicate (e.g., a named function like check_notification(m)) that still requires m.author == thread.guild.me but also checks for something unique to your notifications such as a notification prefix/marker in m.content, a specific embed title/footer, or a custom attribute you set when sending notifications; then pass check_notification to thread.purge so only matching notification messages are deleted.
🧹 Nitpick comments (2)
src/jg/chick/bot.py (2)
232-233: Log message is now misleading.
"Adding role #..."no longer matches behavior (you mention users, not assign roles). Rename this log for clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/jg/chick/bot.py` around lines 232 - 233, The log message currently logged via logger.info("Adding role #{interest['role_id']}") is misleading because the code now constructs mentions_text from missing_members and mentions users instead of assigning roles; update the logger.info call (referencing logger.info, mentions_text, missing_members, and interest['role_id']) to a clear message like "Mentioning users for interest #{interest['role_id']}" or "Notifying missing members for interest #{interest['role_id']}" so the log accurately reflects that users are being mentioned rather than roles being added.
226-231: Only clear old bot notifications when a new one will be sent.Right now, old notifications are cleared even when
len(missing_members) <= 1, so the thread can end up with no bot context at all.♻️ Proposed adjustment
- logger.info("Clearing recent bot messages") - await clear_recent_bot_messages(thread, now=now) missing_members = await get_missing_members(thread, interest["role_id"]) if len(missing_members) <= 1: logger.info(f"Not adding, too few: {len(missing_members)}") else: + logger.info("Clearing recent bot messages") + await clear_recent_bot_messages(thread, now=now) logger.info(f"Adding role #{interest['role_id']}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/jg/chick/bot.py` around lines 226 - 231, The code currently calls await clear_recent_bot_messages(thread, now=now) before computing missing_members, which clears context even when no new notification will be sent; change the flow so clear_recent_bot_messages is only invoked when there will be a new notification (i.e., when len(missing_members) > 1). Concretely, call get_missing_members(thread, interest["role_id"]) first, then if len(missing_members) > 1, log and await clear_recent_bot_messages(thread, now=now) and proceed to add the notification; otherwise keep the current early-return logging path. Ensure you update any surrounding logger.info messages accordingly and reference clear_recent_bot_messages and get_missing_members in the same function where this diff exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/jg/chick/bot.py`:
- Around line 233-240: The code builds a single adding_message from
missing_members (mentions_text) which can exceed Discord's ~2000 char limit;
modify the logic that constructs and sends adding_message to chunk
missing_members into batches so each assembled message (prefix/suffix fixed text
plus joined mentions) stays under the 2000-character limit (subtract the fixed
message length ~166 to compute safe chunk sizes), then loop over those chunks
and call thread.send for each chunk (preserving silent=True); update the code
paths that reference missing_members, mentions_text, adding_message, and
thread.send to perform batching and sequential sends.
---
Duplicate comments:
In `@src/jg/chick/lib/threads.py`:
- Around line 105-109: The purge predicate is too broad (check=lambda m:
m.author == thread.guild.me) and may delete unrelated bot messages; replace it
with a narrower predicate (e.g., a named function like check_notification(m))
that still requires m.author == thread.guild.me but also checks for something
unique to your notifications such as a notification prefix/marker in m.content,
a specific embed title/footer, or a custom attribute you set when sending
notifications; then pass check_notification to thread.purge so only matching
notification messages are deleted.
---
Nitpick comments:
In `@src/jg/chick/bot.py`:
- Around line 232-233: The log message currently logged via logger.info("Adding
role #{interest['role_id']}") is misleading because the code now constructs
mentions_text from missing_members and mentions users instead of assigning
roles; update the logger.info call (referencing logger.info, mentions_text,
missing_members, and interest['role_id']) to a clear message like "Mentioning
users for interest #{interest['role_id']}" or "Notifying missing members for
interest #{interest['role_id']}" so the log accurately reflects that users are
being mentioned rather than roles being added.
- Around line 226-231: The code currently calls await
clear_recent_bot_messages(thread, now=now) before computing missing_members,
which clears context even when no new notification will be sent; change the flow
so clear_recent_bot_messages is only invoked when there will be a new
notification (i.e., when len(missing_members) > 1). Concretely, call
get_missing_members(thread, interest["role_id"]) first, then if
len(missing_members) > 1, log and await clear_recent_bot_messages(thread,
now=now) and proceed to add the notification; otherwise keep the current
early-return logging path. Ensure you update any surrounding logger.info
messages accordingly and reference clear_recent_bot_messages and
get_missing_members in the same function where this diff exists.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a7e625ff-f4a6-46df-8004-f9b638a3812c
📒 Files selected for processing (2)
src/jg/chick/bot.pysrc/jg/chick/lib/threads.py
| mentions_text = " ".join([m.mention for m in missing_members]) | ||
| adding_message = ( | ||
| f"-# {mentions_text} přidávám vás, protože jste si " | ||
| "v <id:customize> vybrali, že vás zajímá tohle téma. " | ||
| "Pokud vás to tu přestane bavit, spusťte tady příkaz " | ||
| "`/unfollow` a já vás odeberu." | ||
| ) | ||
| await thread.send(adding_message, silent=True) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the current maximum Discord message content length for bot messages in threads, and does the 2000-character limit still apply?
💡 Result:
For bot messages sent in threads (including forum/media channel thread “posts”), the maximum length of the content field is still 2,000 characters—threads don’t have a higher content limit. (docs.discord.com)
If you need more text, you must split it across multiple messages or move text into other structures (e.g., embeds have separate limits, such as up to 4096 for an embed description, plus an overall 6000-character combined embed-text cap). (docs.discord.com)
Citations:
- 1: https://docs.discord.com/developers/resources/message
- 2: https://docs.discord.com/developers/resources/message
🏁 Script executed:
# First, check if the file exists and examine the relevant section
if [ -f "src/jg/chick/bot.py" ]; then
echo "File found. Reading lines 233-240:"
sed -n '233,240p' src/jg/chick/bot.py
else
echo "File not found at src/jg/chick/bot.py"
# Try to locate it
find . -name "bot.py" -type f 2>/dev/null | head -10
fiRepository: juniorguru/chick
Length of output: 596
🏁 Script executed:
# Get more context around those lines to understand the code flow
sed -n '220,250p' src/jg/chick/bot.pyRepository: juniorguru/chick
Length of output: 1546
🏁 Script executed:
# Search for where missing_members is defined to understand its source and potential size
rg -B 5 -A 3 "missing_members" src/jg/chick/bot.py | head -50Repository: juniorguru/chick
Length of output: 1173
🏁 Script executed:
# Find and examine the get_missing_members function
fd -t f "threads.py" src/jg/chick/lib/ && rg -A 20 "def get_missing_members" src/jg/chick/lib/threads.pyRepository: juniorguru/chick
Length of output: 832
🏁 Script executed:
# Also check what m.mention returns (it's a Discord member object property)
# Let's also estimate character counts
python3 << 'EOF'
# Fixed message part
fixed_text = "-# přidávám vás, protože jste si v <id:customize> vybrali, že vás zajímá tohle téma. Pokud vás to tu přestane bavit, spusťte tady příkaz `/unfollow` a já vás odeberu."
print(f"Fixed text length: {len(fixed_text)} characters")
# Discord user mentions are typically <@123456789> format
# Smallest mention: <@0> = 4 chars
# Realistic mention: <@123456789> = 12 chars (9 digit ID)
# Large ID mention: <@999999999999> = 14 chars
# Test with different mention counts
for num_mentions in [50, 100, 150, 160]:
# Assume average mention length ~12 chars plus space
mentions_length = num_mentions * 13 # 12 chars + 1 space
total = 4 + mentions_length + len(fixed_text) # "-# " + mentions + fixed
print(f"\n{num_mentions} mentions: {mentions_length} + fixed {len(fixed_text)} + '-# ' (4) = {total} total")
if total > 2000:
print(f" ⚠️ EXCEEDS 2000-char limit by {total - 2000} chars")
else:
print(f" ✓ Under limit, {2000 - total} chars remaining")
EOFRepository: juniorguru/chick
Length of output: 477
Guard against Discord message-length overflow for large mention sets.
Joining mentions into a single message can exceed Discord's 2,000-character content limit in threads. With ~166 characters of fixed text, overflow occurs around 150+ members—realistic for popular interest threads with many opted-in members. Split messages across multiple sends or implement chunking to avoid send failures.
🧩 Suggested robust approach (chunk mentions)
- mentions_text = " ".join([m.mention for m in missing_members])
- adding_message = (
- f"-# {mentions_text} přidávám vás, protože jste si "
- "v <id:customize> vybrali, že vás zajímá tohle téma. "
- "Pokud vás to tu přestane bavit, spusťte tady příkaz "
- "`/unfollow` a já vás odeberu."
- )
- await thread.send(adding_message, silent=True)
+ suffix = (
+ "přidávám vás, protože jste si v <id:customize> vybrali, "
+ "že vás zajímá tohle téma. Pokud vás to tu přestane bavit, "
+ "spusťte tady příkaz `/unfollow` a já vás odeberu."
+ )
+ chunk = "-# "
+ for mention in (m.mention for m in missing_members):
+ candidate = f"{chunk}{mention} "
+ if len(candidate) + len(suffix) > 2000:
+ await thread.send(f"{chunk}{suffix}", silent=True)
+ chunk = f"-# {mention} "
+ else:
+ chunk = candidate
+ await thread.send(f"{chunk}{suffix}", silent=True)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/jg/chick/bot.py` around lines 233 - 240, The code builds a single
adding_message from missing_members (mentions_text) which can exceed Discord's
~2000 char limit; modify the logic that constructs and sends adding_message to
chunk missing_members into batches so each assembled message (prefix/suffix
fixed text plus joined mentions) stays under the 2000-character limit (subtract
the fixed message length ~166 to compute safe chunk sizes), then loop over those
chunks and call thread.send for each chunk (preserving silent=True); update the
code paths that reference missing_members, mentions_text, adding_message, and
thread.send to perform batching and sequential sends.
No description provided.