Repository navigation
Add nameFilter to get_cards_by_list_id - #76
Conversation
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 introduces a significant enhancement to the 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
|
There was a problem hiding this comment.
Code Review
This pull request introduces a nameFilter to the get_cards_by_list_id tool, which is a valuable addition for improving efficiency by filtering cards client-side. The implementation is well-tested. My review includes two suggestions for improvement: one to enhance the filtering logic by handling whitespace and another to remove an unused parameter for better code clarity.
There was a problem hiding this comment.
Pull request overview
Adds an optional nameFilter parameter to the get_cards_by_list_id MCP tool and implements client-side filtering of cards by case-insensitive substring match to reduce token usage for name-based lookups.
Changes:
- Extend
get_cards_by_list_idtool input schema with optionalnameFilter, and pass it through to the client. - Update
TrelloClient.getCardsByListto optionally filter returned cards by name (case-insensitive substring). - Add Bun unit tests covering validation and filtering behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/index.ts | Adds nameFilter to the MCP tool schema and forwards it to the Trello client call. |
| src/trello-client.ts | Implements optional nameFilter trimming + case-insensitive substring filtering of cards. |
| tests/get-cards-name-filter.test.ts | Adds unit tests for schema validation and filtering behavior/backward compatibility. |
| async getCardsByList(boardId: string | undefined, listId: string, nameFilter?: string): Promise<TrelloCard[]> { | ||
| return this.handleRequest(async () => { | ||
| const response = await this.axiosInstance.get(`/lists/${listId}/cards`); | ||
| return response.data; | ||
| let cards: TrelloCard[] = response.data; | ||
| const trimmed = nameFilter?.trim(); | ||
| if (trimmed) { | ||
| const searchTerm = trimmed.toLowerCase(); | ||
| cards = cards.filter((card) => card.name.toLowerCase().includes(searchTerm)); | ||
| } |
There was a problem hiding this comment.
This block doesn’t match the repo’s Prettier settings (printWidth: 100, arrowParens: "avoid"): the method signature is overlong and the filter callback uses parentheses around a single parameter. Running Prettier (or manually wrapping + changing to card => ...) will align formatting with the project standards.
| const MOCK_CARDS = [ | ||
| { id: '1', name: 'FEAT: Add Slippage to Position History', desc: '', due: null, idList: 'list1', idLabels: [], closed: false, url: '', dateLastActivity: '' }, | ||
| { id: '2', name: 'FEAT: Execute trade slippage config', desc: '', due: null, idList: 'list1', idLabels: [], closed: false, url: '', dateLastActivity: '' }, | ||
| { id: '3', name: 'BUG: Fix login page crash', desc: '', due: null, idList: 'list1', idLabels: [], closed: false, url: '', dateLastActivity: '' }, | ||
| { id: '4', name: 'Add Slippage to Position History', desc: '', due: null, idList: 'list1', idLabels: [], closed: false, url: '', dateLastActivity: '' }, | ||
| ]; |
There was a problem hiding this comment.
The MOCK_CARDS entries are formatted as very long single lines (well over the repo’s Prettier printWidth: 100). Please run Prettier on this test file so the fixtures and object literals are wrapped consistently.
| // Mirror the schema from index.ts for validation tests | ||
| const nameFilterSchema = z.string().trim().min(1, 'nameFilter must not be empty').optional(); | ||
|
|
There was a problem hiding this comment.
This test duplicates the nameFilter Zod schema inline (“Mirror the schema from index.ts”). That can drift from the actual tool schema over time. Consider defining/exporting a shared nameFilter validator (e.g., in src/validators.ts) and reusing it both in src/index.ts and in tests.
| } | ||
|
|
||
| async getCardsByList(boardId: string | undefined, listId: string): Promise<TrelloCard[]> { | ||
| async getCardsByList(boardId: string | undefined, listId: string, nameFilter?: string): Promise<TrelloCard[]> { |
There was a problem hiding this comment.
boardId is now an unused parameter in getCardsByList. With the current ESLint config (@typescript-eslint/no-unused-vars), this will fail linting unless the arg is used or renamed to start with _ (e.g., _boardId) or removed entirely from the signature/callers.
| async getCardsByList(boardId: string | undefined, listId: string, nameFilter?: string): Promise<TrelloCard[]> { | |
| async getCardsByList( | |
| _boardId: string | undefined, | |
| listId: string, | |
| nameFilter?: string | |
| ): Promise<TrelloCard[]> { |
|
@copilot apply changes based on the comments in this thread Then resolve conflicts in prep for merge |
|
@copilot apply changes based on the comments in this thread |
|
Thanks for this — client-side name filtering is a genuinely useful token-saver. A few notes before merging:
Once rebased, I'll review the diff and merge. Appreciate the work. |
Without this, the only way to find a specific card by name is to fetch all cards in the list and filter them in the LLM context, which burns tokens unnecessarily. This adds an optional client-side nameFilter parameter so the MCP server returns only matching cards. When nameFilter is provided, cards are filtered by case-insensitive substring match against the card name, using the same toLowerCase().includes() pattern as findChecklistItemsByDescription. The parameter is optional and backward compatible — omitting it returns all cards as before.
Address PR feedback: empty string and whitespace-only nameFilter values are now rejected via zod schema validation (.trim().min(1)) rather than silently returning all cards. The client method also trims as defence in depth. Added schema validation tests.
6487333 to
806cdb6
Compare
Summary
nameFilterparameter to theget_cards_by_list_idtooltoLowerCase().includes()pattern asfindChecklistItemsByDescriptionnameFilterreturns all cards as beforeChanges
src/index.ts— addednameFilteroptional string toinputSchemaand passed through to clientsrc/trello-client.ts— addednameFilterparameter togetCardsByListwith client-side filteringtests/get-cards-name-filter.test.ts— 8 unit tests covering exact match, substring, case-insensitivity, no match, and backward compatibilityTest plan
bun run buildcompiles successfullybun test— all 8 tests passnameFilterreturns all cards (backward compatible)nameFilterreturns only matching cards