fix: use fetch_thread() fallback when resolving message thread#614
Conversation
Uses _resolve_message_thread() helper that checks the cached thread property first, then falls back to fetch_thread() from the API to handle the case where Discord hasn't populated the cache yet. Applies to all three button handlers (sounding, MD, outlook).
📝 WalkthroughWalkthroughAdded centralized Discord thread resolution with API-fetch fallback and updated three AI summary handlers to use it before posting embeds. ChangesMessage thread resolution
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cogs/ai_summaries.py (1)
421-433: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd focused tests for the new resolution contract.
Cover cached-thread use, API fallback, and lookup failures; the existing outlook test does not configure or assert these paths.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cogs/ai_summaries.py` around lines 421 - 433, Add focused tests for _resolve_message_thread covering a message with a cached thread, a cache miss that successfully returns the result of fetch_thread(), and fetch_thread() raising discord.NotFound or discord.HTTPException. Assert the cached path does not call the API, and lookup failures return None without propagating the exception.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cogs/ai_summaries.py`:
- Around line 430-433: Update the exception handling around the fetch_thread
call to include discord.InvalidData alongside discord.NotFound and
discord.HTTPException, preserving the existing behavior of returning None for
all these fetch failures.
---
Nitpick comments:
In `@cogs/ai_summaries.py`:
- Around line 421-433: Add focused tests for _resolve_message_thread covering a
message with a cached thread, a cache miss that successfully returns the result
of fetch_thread(), and fetch_thread() raising discord.NotFound or
discord.HTTPException. Assert the cached path does not call the API, and lookup
failures return None without propagating the exception.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| try: | ||
| return await message.fetch_thread() | ||
| except (discord.NotFound, discord.HTTPException): | ||
| return None |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
rg -n 'discord\.py|py-cord|nextcord' \
--glob 'pyproject.toml' \
--glob 'requirements*.txt' \
--glob 'setup.py' \
--glob 'poetry.lock'Repository: full-bars/spc-bot
Length of output: 155
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== repo files of interest ==\n'
git ls-files | rg '(^|/)(pyproject\.toml|requirements.*\.txt|setup\.py|poetry\.lock|Pipfile|Pipfile\.lock|uv\.lock|cogs/ai_summaries\.py)$'
printf '\n== outline cogs/ai_summaries.py ==\n'
ast-grep outline cogs/ai_summaries.py --view expanded || true
printf '\n== fetch_thread occurrences ==\n'
rg -n 'fetch_thread|InvalidData|NotFound|HTTPException' cogs/ai_summaries.py
printf '\n== dependency mentions of discord ==\n'
rg -n 'discord' --glob 'pyproject.toml' --glob 'requirements*.txt' --glob 'setup.py' --glob 'poetry.lock' --glob 'uv.lock' --glob 'Pipfile*' || trueRepository: full-bars/spc-bot
Length of output: 2067
🌐 Web query:
discord.py Message.fetch_thread InvalidData NotFound HTTPException documentation
💡 Result:
In discord.py, Message.fetch_thread is designed specifically to retrieve the public thread started from a given message [1][2]. It is important to note that this method uses the message's ID to fetch the thread, which means it will only succeed if the message is the initial message that created the thread [3]. When using Message.fetch_thread, you may encounter the following exceptions [2]: - NotFound: This exception is raised when no thread is found attached to the message [2]. If you attempt to call this method on a message that did not create a thread, or on a subsequent message within a thread (rather than the original starting message), you will likely receive a 404 Not Found error [3][2]. - HTTPException: This is a general exception raised when the HTTP request to the Discord API fails [4][5]. - InvalidData: This is raised if the library receives an unknown channel type or if the guild associated with the thread does not match the one expected by the message object [2]. Because Message.fetch_thread is an API call, the official documentation recommends using the.thread property for general usage, as it accesses the internal cache [1][2]. Note that the.thread property may be None if the thread is not in the cache (e.g., archived threads), in which case fetch_thread is the appropriate fallback if you are targeting the original message [1][2]. If you simply need the channel a message belongs to, you can use the Message.channel property, which will correctly return the thread object if the message was sent within one [3].
Citations:
- 1: https://discordpy.readthedocs.io/en/latest/interactions/api.html
- 2: https://github.com/Rapptz/discord.py/blob/188d32d6/discord/message.py
- 3: Message.fetch_thread throws NotFoundError for subsequent messages Rapptz/discord.py#10251
- 4: https://github.com/Rapptz/discord.py/blob/master/discord/errors.py
- 5: https://discordpy.readthedocs.io/en/latest/api.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '380,470p' cogs/ai_summaries.pyRepository: full-bars/spc-bot
Length of output: 3477
Handle discord.InvalidData here as well.
Message.fetch_thread() can raise discord.InvalidData, so this helper should treat it like NotFound and HTTPException and return None instead of bubbling into the error path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cogs/ai_summaries.py` around lines 430 - 433, Update the exception handling
around the fetch_thread call to include discord.InvalidData alongside
discord.NotFound and discord.HTTPException, preserving the existing behavior of
returning None for all these fetch failures.
Addresses CodeRabbit review on PR #613 —
interaction.message.threadis a lazy property that may not be populated in Discord's cache. Adds_resolve_message_thread()helper that checks the cached property first, then falls back tofetch_thread()from the API. Applies to all three button handlers (sounding, MD, outlook).Summary by CodeRabbit