feat(snippets): add member filtering to list snippets command#1205
feat(snippets): add member filtering to list snippets command#1205
Conversation
$snippets/$ls now accepts an optional member argument to list snippets created by a specific user. Uses FlexibleUserConverter to support mentions, user IDs, and usernames, including users who have left the guild. Falls back to search or list-all when no member is provided. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Reviewer's GuideAdds optional member-based filtering to the Sequence diagram for member-filtered snippet listingsequenceDiagram
actor User
participant Discord
participant TuxBot
participant ListSnippets
participant SnippetRepository as SnippetRepository
participant ViewMenu
User->>Discord: send message "$snippets @member"
Discord->>TuxBot: create command Context
TuxBot->>ListSnippets: list_snippets(ctx, member, search_query=None)
alt member argument provided
ListSnippets->>SnippetRepository: get_snippets_by_creator(member.id, ctx.guild.id)
SnippetRepository-->>ListSnippets: list[Snippet]
ListSnippets->>ListSnippets: sort(snippets, key=uses, descending)
else search_query provided
ListSnippets->>SnippetRepository: search_snippets(ctx.guild.id, search_query)
SnippetRepository-->>ListSnippets: list[Snippet]
ListSnippets->>ListSnippets: sort(snippets, key=uses, descending)
else no member or search_query
ListSnippets->>SnippetRepository: get_all_snippets_by_guild_id(ctx.guild.id, order_by=uses_desc)
SnippetRepository-->>ListSnippets: list[Snippet]
end
alt no snippets found
ListSnippets->>TuxBot: send_snippet_error(ctx, message based on member/search)
TuxBot-->>Discord: send error embed
Discord-->>User: display error
else snippets found
ListSnippets->>ViewMenu: create ViewMenu(ctx)
loop pages of SNIPPET_PAGINATION_LIMIT
ListSnippets->>ListSnippets: _create_snippets_list_embed(ctx, page_snippets, total_snippets, search_query, member)
ListSnippets-->>ViewMenu: embed page
end
ViewMenu->>Discord: start pagination
Discord-->>User: display paginated snippet embeds
end
Class diagram for ListSnippets member-filtered listing flowclassDiagram
class ListSnippets {
+Tux bot
+list_snippets(ctx, member, search_query) void
}
class SnippetRepository {
+get_snippets_by_creator(creator_id, guild_id) list~Snippet~
+search_snippets(guild_id, search_query) list~Snippet~
+get_all_snippets_by_guild_id(guild_id, order_by) list~Snippet~
}
class Snippet {
+int id
+int guild_id
+int creator_id
+string name
+string content
+int uses
}
class FlexibleUserConverter {
+convert(ctx, argument) discord.User
}
class SnippetsModule {
+_create_snippets_list_embed(ctx, snippets, total_snippets, search_query, member) discord.Embed
}
class ViewMenu {
+ViewMenu(ctx)
+add_page(embed) void
+start() void
}
ListSnippets --> SnippetRepository : uses
ListSnippets --> Snippet : returns
ListSnippets --> FlexibleUserConverter : member parameter converter
ListSnippets --> SnippetsModule : calls _create_snippets_list_embed
ListSnippets --> ViewMenu : creates and populates
SnippetRepository --> Snippet : manages
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the snippet listing functionality by introducing the ability to filter snippets by their creator. Users can now easily view snippets made by a specific individual using a new optional argument, improving discoverability and organization within the snippet system. The implementation ensures robust user resolution and provides clear feedback through dynamic embed titles and appropriate error messages. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdded an optional member parameter to the snippets list command, enabling users to filter snippets by a specific member using FlexibleUserConverter. The feature supports mentions, user IDs, and usernames, including users who have left the guild. Updated command signature, filtering logic, and embed generation accordingly. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
The implementation correctly adds member filtering to the list snippets command. The code properly uses FlexibleUserConverter for flexible user input handling, maintains consistency with existing patterns, and includes appropriate error messages for different scenarios. The changes are well-integrated and ready for merge.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The nested ternary in the
send_snippet_errorcall makes the description logic hard to follow and behaves oddly whenmemberis set butsearch_queryis not (it falls back to the generic "No snippets found." text); consider rewriting this as a straightforward if/elif/else block so member-specific messages are always used when a member is provided. - When both a member and trailing text are supplied (e.g.
$snippets @user foo), the extra text is parsed intosearch_querybut then ignored because thememberbranch short‑circuits the search logic; consider either supporting combined member+search filtering or validating and rejecting unexpected extra input to avoid confusing behavior. get_snippets_by_creatorresults are sorted in Python byuses, whereas the other paths push ordering into the database; if possible, aligning this to use DB-level ordering as well would keep the behavior consistent and avoid an extra in‑memory sort.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The nested ternary in the `send_snippet_error` call makes the description logic hard to follow and behaves oddly when `member` is set but `search_query` is not (it falls back to the generic "No snippets found." text); consider rewriting this as a straightforward if/elif/else block so member-specific messages are always used when a member is provided.
- When both a member and trailing text are supplied (e.g. `$snippets @user foo`), the extra text is parsed into `search_query` but then ignored because the `member` branch short‑circuits the search logic; consider either supporting combined member+search filtering or validating and rejecting unexpected extra input to avoid confusing behavior.
- `get_snippets_by_creator` results are sorted in Python by `uses`, whereas the other paths push ordering into the database; if possible, aligning this to use DB-level ordering as well would keep the behavior consistent and avoid an extra in‑memory sort.
## Individual Comments
### Comment 1
<location path="src/tux/modules/snippets/list_snippets.py" line_range="65-69" />
<code_context>
- # Fetch snippets with database-level ordering and optional search
- if search_query:
+ if member is not None:
+ filtered_snippets = await self.db.snippet.get_snippets_by_creator(
+ member.id,
+ ctx.guild.id,
+ )
+ filtered_snippets.sort(key=lambda s: s.uses, reverse=True)
+ elif search_query:
filtered_snippets = await self.db.snippet.search_snippets(
</code_context>
<issue_to_address>
**suggestion (performance):** Consider pushing the ordering by usage count into the DB for the member-filtered query as well.
In `get_all_snippets_by_guild_id` ordering is done in the DB via `order_by=desc(...)`, but for the member-specific path you fetch then sort in Python. To keep behavior consistent and avoid in-memory sorting for large result sets, consider adding an `order_by` option (or a variant) to `get_snippets_by_creator` so the DB returns results already ordered by `uses`.
Suggested implementation:
```python
if member is not None:
filtered_snippets = await self.db.snippet.get_snippets_by_creator(
member.id,
ctx.guild.id,
order_by=desc(Snippet.__table__.c.uses), # type: ignore[attr-defined]
)
elif search_query:
```
To make this compile and work correctly, you'll also need to:
1. Update the signature of `get_snippets_by_creator` (likely in your snippet repository/DAO) to accept an optional `order_by` parameter, defaulting to `None`, e.g.:
```python
async def get_snippets_by_creator(
self,
creator_id: int,
guild_id: int,
order_by: Any | None = None,
) -> list[Snippet]:
```
2. Thread the `order_by` argument into the underlying query, similar to how it's done in `get_all_snippets_by_guild_id`, e.g.:
```python
query = select(Snippet).where(...)
if order_by is not None:
query = query.order_by(order_by)
```
3. Ensure any other call sites of `get_snippets_by_creator` are updated if the new parameter is not keyword-only or if its position changes.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| filtered_snippets = await self.db.snippet.get_snippets_by_creator( | ||
| member.id, | ||
| ctx.guild.id, | ||
| ) | ||
| filtered_snippets.sort(key=lambda s: s.uses, reverse=True) |
There was a problem hiding this comment.
suggestion (performance): Consider pushing the ordering by usage count into the DB for the member-filtered query as well.
In get_all_snippets_by_guild_id ordering is done in the DB via order_by=desc(...), but for the member-specific path you fetch then sort in Python. To keep behavior consistent and avoid in-memory sorting for large result sets, consider adding an order_by option (or a variant) to get_snippets_by_creator so the DB returns results already ordered by uses.
Suggested implementation:
if member is not None:
filtered_snippets = await self.db.snippet.get_snippets_by_creator(
member.id,
ctx.guild.id,
order_by=desc(Snippet.__table__.c.uses), # type: ignore[attr-defined]
)
elif search_query:To make this compile and work correctly, you'll also need to:
-
Update the signature of
get_snippets_by_creator(likely in your snippet repository/DAO) to accept an optionalorder_byparameter, defaulting toNone, e.g.:async def get_snippets_by_creator( self, creator_id: int, guild_id: int, order_by: Any | None = None, ) -> list[Snippet]:
-
Thread the
order_byargument into the underlying query, similar to how it's done inget_all_snippets_by_guild_id, e.g.:query = select(Snippet).where(...) if order_by is not None: query = query.order_by(order_by)
-
Ensure any other call sites of
get_snippets_by_creatorare updated if the new parameter is not keyword-only or if its position changes.
📚 Documentation Preview
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches. Re-running this action after a short time may resolve the issue. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
❌ 2 Tests Failed:
View the full list of 2 ❄️ flaky test(s)
To view more test analytics, go to the [Prevent Tests Dashboard](https://All Things Linux.sentry.io/prevent/tests/?preventPeriod=30d&integratedOrgName=allthingslinux&repository=tux&branch=feat%2Fsnippets-list-by-member) |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| async def list_snippets( | ||
| self, | ||
| ctx: commands.Context[Tux], | ||
| member: discord.User | None = _MEMBER_PARAM, | ||
| *, | ||
| search_query: str | None = None, |
There was a problem hiding this comment.
Bug: When list_snippets is called with text that is not a valid user, the text is silently ignored instead of being used as the search_query.
Severity: MEDIUM
Suggested Fix
Modify the command's signature to ensure that text not matching a user is passed to the search_query. This could involve making search_query a regular positional parameter that can consume the remaining arguments, potentially by using a greedy converter or changing the parameter order.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/tux/modules/snippets/list_snippets.py#L41-L46
Potential issue: In the `list_snippets` command, the `member` parameter is a positional
argument with a custom converter, while `search_query` is a keyword-only argument. When
a user provides input that is not a valid user (e.g., `$snippets some text`), the
converter for `member` fails. The optional fallback correctly sets `member` to `None`,
but the remaining text is discarded because the keyword-only `search_query` parameter
cannot accept positional arguments. This results in the search functionality being
silently ignored when the command is invoked with a search term that doesn't parse as a
user.
Did we get this right? 👍 / 👎 to inform future reviews.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 files
Confidence score: 3/5
- There is a concrete regression risk in
src/tux/modules/snippets/list_snippets.py: the new positionalmemberconverter changes parsing so non-user first tokens error instead of falling back to snippet text search. - Given the high confidence (9/10) and meaningful severity (7/10), this is likely user-facing behavior change rather than a minor edge case, so merge risk is moderate.
- Pay close attention to
src/tux/modules/snippets/list_snippets.py- argument parsing now blocks intended search fallback behavior for some queries.
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="src/tux/modules/snippets/list_snippets.py">
<violation number="1" location="src/tux/modules/snippets/list_snippets.py:44">
P1: The new positional `member` converter prevents text queries from falling back to search: non-user first tokens now error out instead of running snippet search.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| async def list_snippets( | ||
| self, | ||
| ctx: commands.Context[Tux], | ||
| member: discord.User | None = _MEMBER_PARAM, |
There was a problem hiding this comment.
P1: The new positional member converter prevents text queries from falling back to search: non-user first tokens now error out instead of running snippet search.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/tux/modules/snippets/list_snippets.py, line 44:
<comment>The new positional `member` converter prevents text queries from falling back to search: non-user first tokens now error out instead of running snippet search.</comment>
<file context>
@@ -37,33 +41,39 @@ def __init__(self, bot: Tux) -> None:
async def list_snippets(
self,
ctx: commands.Context[Tux],
+ member: discord.User | None = _MEMBER_PARAM,
*,
search_query: str | None = None,
</file context>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a useful feature to filter snippets by member. My review focuses on improving the filtering logic to correctly handle cases where both a member and a search query are provided, as the current implementation ignores the search query in this scenario. I've also suggested improvements to the error messaging for better clarity and readability.
1 participant
Summary
$snippets/$lsnow accepts an optional member argument to list snippets created by a specific user (e.g.$ls @user)FlexibleUserConverterto support mentions, user IDs, and usernames — including users who have left the guildTest plan
$snippets— lists all snippets as before$snippets @user— lists only snippets created by that user$snippets some text— search still works when input isn't a valid user$ls @user— alias works🤖 Generated with Claude Code
Summary by Sourcery
Add optional member-based filtering to the snippets listing command and update related presentation and changelog entries.
New Features:
$snippets/$lscommand to accept an optional member argument to list snippets created by a specific user.Documentation: