Truncate card descriptions in get_cards_by_list_id#67
Truncate card descriptions in get_cards_by_list_id#67louisabraham wants to merge 2 commits intodelorenj:mainfrom
Conversation
When listing cards, descriptions are now capped at 200 characters to prevent oversized responses. If the total response still exceeds 50KB, descriptions are omitted entirely. Both cases append a notice directing users to get_card for full details. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary of ChangesHello @louisabraham, 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 significant improvements 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 useful feature to manage large response sizes from the get_cards_by_list_id tool by truncating or omitting card descriptions. The implementation is solid, introducing constants for limits and updating the tool's description. I've suggested one refactoring to improve the efficiency and readability of the truncation logic by avoiding an unnecessary array iteration and using clearer variable names. Overall, this is a good improvement.
| // Truncate descriptions to keep response size manageable | ||
| const previews = cards.map(card => ({ | ||
| ...card, | ||
| desc: | ||
| card.desc && card.desc.length > descLimit | ||
| ? card.desc.slice(0, descLimit) + '...' | ||
| : card.desc, | ||
| })); | ||
|
|
||
| let result = JSON.stringify(previews, null, 2); | ||
| let truncated = false; | ||
|
|
||
| // If total response is still too large, drop descriptions entirely | ||
| if (result.length > TrelloServer.CARD_LIST_TOTAL_CHAR_LIMIT) { | ||
| const minimal = cards.map(({ desc, ...rest }) => rest); | ||
| result = JSON.stringify(minimal, null, 2); | ||
| truncated = true; | ||
| } | ||
|
|
||
| if (truncated) { | ||
| result += '\n\n(Descriptions omitted due to response size. Use get_card with a specific cardId for full details.)'; | ||
| } else if (cards.some(c => c.desc && c.desc.length > descLimit)) { | ||
| result += '\n\n(Some descriptions were truncated. Use get_card with a specific cardId for full details.)'; | ||
| } |
There was a problem hiding this comment.
The current logic for checking if descriptions were truncated involves an extra iteration over the cards array using cards.some(...). This can be made more efficient by tracking whether truncation occurred during the initial map operation. This refactoring also introduces clearer variable names (descriptionsOmitted, anyTruncated) to improve readability and avoid the ambiguity of the original truncated variable.
// Truncate descriptions to keep response size manageable
let anyTruncated = false;
const previews = cards.map(card => {
if (card.desc && card.desc.length > descLimit) {
anyTruncated = true;
return {
...card,
desc: card.desc.slice(0, descLimit) + '...',
};
}
return card;
});
let result = JSON.stringify(previews, null, 2);
let descriptionsOmitted = false;
// If total response is still too large, drop descriptions entirely
if (result.length > TrelloServer.CARD_LIST_TOTAL_CHAR_LIMIT) {
const minimal = cards.map(({ desc, ...rest }) => rest);
result = JSON.stringify(minimal, null, 2);
descriptionsOmitted = true;
}
if (descriptionsOmitted) {
result += '\n\n(Descriptions omitted due to response size. Use get_card with a specific cardId for full details.)';
} else if (anyTruncated) {
result += '\n\n(Some descriptions were truncated. Use get_card with a specific cardId for full details.)';
}There was a problem hiding this comment.
Pull request overview
This PR updates the get_cards_by_list_id MCP tool response to be more LLM-friendly by limiting how much card description text is returned, reducing oversized responses that waste context window.
Changes:
- Truncate each card’s
descto a short preview inget_cards_by_list_idresponses. - If the overall serialized response is still large, omit descriptions entirely and append a user-facing notice.
- Update the tool description to clarify preview behavior and direct users to
get_cardfor full details.
src/index.ts
Outdated
| const previews = cards.map(card => ({ | ||
| ...card, | ||
| desc: | ||
| card.desc && card.desc.length > descLimit | ||
| ? card.desc.slice(0, descLimit) + '...' | ||
| : card.desc, | ||
| })); | ||
|
|
There was a problem hiding this comment.
The truncation adds '...' after slicing to descLimit, so the resulting desc can exceed the stated 200-character limit (e.g., 203 chars). Consider slicing to descLimit - 3 (or only appending ellipsis when it fits) so the final preview length stays within the configured limit.
| const previews = cards.map(card => ({ | |
| ...card, | |
| desc: | |
| card.desc && card.desc.length > descLimit | |
| ? card.desc.slice(0, descLimit) + '...' | |
| : card.desc, | |
| })); | |
| const previews = cards.map(card => { | |
| const ellipsis = '...'; | |
| return { | |
| ...card, | |
| desc: | |
| card.desc && card.desc.length > descLimit | |
| ? descLimit > ellipsis.length | |
| ? card.desc.slice(0, descLimit - ellipsis.length) + ellipsis | |
| : card.desc.slice(0, descLimit) | |
| : card.desc, | |
| }; | |
| }); |
src/index.ts
Outdated
| if (truncated) { | ||
| result += '\n\n(Descriptions omitted due to response size. Use get_card with a specific cardId for full details.)'; | ||
| } else if (cards.some(c => c.desc && c.desc.length > descLimit)) { | ||
| result += '\n\n(Some descriptions were truncated. Use get_card with a specific cardId for full details.)'; | ||
| } |
There was a problem hiding this comment.
result is JSON, but in the truncated/omitted cases you append a human notice string, making the output no longer valid JSON and changing the output format depending on data. Consider keeping the JSON parseable by returning a structured JSON object (e.g., { cards, notice }) or by returning the notice as a separate content item so the JSON portion remains intact.
| // If total response is still too large, drop descriptions entirely | ||
| if (result.length > TrelloServer.CARD_LIST_TOTAL_CHAR_LIMIT) { | ||
| const minimal = cards.map(({ desc, ...rest }) => rest); | ||
| result = JSON.stringify(minimal, null, 2); | ||
| truncated = true; | ||
| } |
There was a problem hiding this comment.
The 50KB cutoff is implemented using result.length, which counts UTF-16 code units rather than bytes; with non-ASCII descriptions the response can exceed 50KB in bytes without triggering the omit behavior. If the intent is a byte-size limit, consider measuring Buffer.byteLength(result, 'utf8') (or equivalent in Bun) and applying the threshold on that value.
src/index.ts
Outdated
| let truncated = false; | ||
|
|
||
| // If total response is still too large, drop descriptions entirely | ||
| if (result.length > TrelloServer.CARD_LIST_TOTAL_CHAR_LIMIT) { | ||
| const minimal = cards.map(({ desc, ...rest }) => rest); | ||
| result = JSON.stringify(minimal, null, 2); | ||
| truncated = true; | ||
| } | ||
|
|
||
| if (truncated) { |
There was a problem hiding this comment.
The boolean truncated is set only when descriptions are omitted entirely, not when they are truncated to a preview. Renaming it (e.g., descriptionsOmitted / omittedDescriptions) would make the control flow and messages easier to follow.
| let truncated = false; | |
| // If total response is still too large, drop descriptions entirely | |
| if (result.length > TrelloServer.CARD_LIST_TOTAL_CHAR_LIMIT) { | |
| const minimal = cards.map(({ desc, ...rest }) => rest); | |
| result = JSON.stringify(minimal, null, 2); | |
| truncated = true; | |
| } | |
| if (truncated) { | |
| let descriptionsOmitted = false; | |
| // If total response is still too large, drop descriptions entirely | |
| if (result.length > TrelloServer.CARD_LIST_TOTAL_CHAR_LIMIT) { | |
| const minimal = cards.map(({ desc, ...rest }) => rest); | |
| result = JSON.stringify(minimal, null, 2); | |
| descriptionsOmitted = true; | |
| } | |
| if (descriptionsOmitted) { |
|
Thanks for the reviews! Addressed the feedback in https://github.com/louisabraham/mcp-server-trello/tree/anthony: Fixed:
Not changed:
|
- Track truncation flag during the map pass (avoids extra iteration) - Rename truncated -> descriptionsOmitted for clarity - Return notice as a separate content item so JSON remains parseable Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
get_cards_by_list_idcan return very large responses when cards have long descriptions, which wastes context window for LLM consumersget_cardfor full detailsBehavior
Test plan
🤖 Generated with Claude Code