fix(card): handle Card footer deserialization errors#479
Merged
Conversation
The `reply` tool was failing silently when the LLM sent malformed JSON arguments for card footers. The footer field could be either a plain string or an object with `text` and optional `icon_url` fields. Changes: - Add `CardFooter` struct with `text` and `icon_url` fields - Add custom deserializer to accept both string and object forms - Update `text_from_cards()` and Discord embed builder - Add WARN-level logging when tool errors occur in hooks This ensures deserialization errors are visible in logs and can be returned to the LLM for self-correction. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds structured card footer support (string or object) with custom deserialization and Display; updates Discord embed footer construction to use footer text and optional icon; logs tool-call failures whose result starts with "Toolset error:"; and adds Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/messaging/discord.rs (1)
1040-1042: Wire up theicon_urlfield fromCardFooterto the Discord embed footer.The
CardFooterstruct includes an optionalicon_urlfield that is currently not used when creating the embed footer. Serenity'sCreateEmbedFootersupports footer icons via the.icon_url()method.♻️ Proposed implementation
if let Some(footer) = &card.footer { - embed = embed.footer(CreateEmbedFooter::new(footer.text.clone())); + let mut embed_footer = CreateEmbedFooter::new(footer.text.clone()); + if let Some(icon_url) = &footer.icon_url { + embed_footer = embed_footer.icon_url(icon_url); + } + embed = embed.footer(embed_footer); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/messaging/discord.rs` around lines 1040 - 1042, The footer's icon_url is not being forwarded to the Discord embed; update the footer construction (where card.footer is handled) to set the icon when present: create a CreateEmbedFooter with footer.text then, if CardFooter.icon_url is Some, call .icon_url(...) with that value before passing it to embed.footer. Use CardFooter, card.footer, CreateEmbedFooter, and the .icon_url() builder method so the embed includes the footer icon when provided.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/messaging/discord.rs`:
- Around line 1040-1042: The footer's icon_url is not being forwarded to the
Discord embed; update the footer construction (where card.footer is handled) to
set the icon when present: create a CreateEmbedFooter with footer.text then, if
CardFooter.icon_url is Some, call .icon_url(...) with that value before passing
it to embed.footer. Use CardFooter, card.footer, CreateEmbedFooter, and the
.icon_url() builder method so the embed includes the footer icon when provided.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 27ceccf0-9379-4edb-9717-1d7a25718d3a
📒 Files selected for processing (4)
.gitignoresrc/hooks/spacebot.rssrc/lib.rssrc/messaging/discord.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/lib.rs`:
- Around line 773-777: The CardFooter struct adds an icon_url field that is
deserialized but never used by the Discord renderer; update the renderer that
builds the Discord footer (the code in src/messaging/discord.rs that currently
constructs the footer from footer.text only) to include footer.icon_url when
present (e.g., set the Discord footer's icon/url property from
CardFooter.icon_url.clone() or equivalent), or if you prefer not to support
icons yet remove icon_url from CardFooter to avoid dead fields—ensure you
reference CardFooter and the footer construction site in the Discord renderer
when making the change.
- Around line 838-851: The deserializer reads "icon_url" as a bare String which
fails on explicit nulls; change the map.next_value call for the "icon_url" key
to deserialize into Option<String> (e.g., use
map.next_value::<Option<String>>()?) so icon_url can be Some(String) or None
when JSON has null, then construct CardFooter { text: t, icon_url } as before;
also add a regression test case that deserializes {"footer":
{"text":"x","icon_url": null}} (alongside the existing footer tests) to ensure
null is accepted.
- Around line 766-768: The schema and deserializer for Card.footer are wrong:
remove or change the #[schemars(with = "Option<String>")] override so the
generated schema reflects Option<CardFooter> (i.e., the object with text and
icon_url) instead of a plain string, and fix the custom deserializer function
deserialize_card_footer to accept a nullable icon by reading icon_url as
Option<String> (use map.next_value::<Option<String>>()? when handling the
"icon_url" key) so {"footer": {"text":"x","icon_url":null}} deserializes
correctly; ensure CardFooter and deserialize_card_footer are the referenced
symbols you edit.
- Remove incorrect schemars override so schema reflects Option<CardFooter> - Fix icon_url deserialization to handle null via Option<String> - Add icon_url support in Discord embed builder - Add regression test for icon_url: null
jamiepine
approved these changes
Mar 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CardFooterstruct to support structured footer data withtextand optionalicon_urlfieldsserde::de::Visitorto accept both string ("footer": "text") and object ("footer": {"text": "..."}) formstext_from_cards()to access footer content via.textpropertyfooter.text.clone()for CreateEmbedFooterSpacebotHook::on_tool_result()when tool errors occurFixes #478
Testing
All footer deserialization tests pass:
card_footer_deserializes_from_string- plain string footercard_footer_deserializes_from_object- object with text and icon_urlcard_footer_deserializes_from_object_text_only- the problematic case from reply tool silently swallows JSON deserialization errors #478card_footer_deserializes_when_missing- optional footer fieldcard_footer_deserializes_when_null- null footer valuecard_footer_display_trait_worksandcard_footer_as_str_works- utility methodsNotes (Optional)
The root cause was the LLM sending footer as
{"text": "Week of March 23, 2026"}when the schema expected a plain string. The custom deserializer now handles both forms so the tool call doesn't fail. Errors are now logged at WARN level in hooks so operators can see when tools fail (including deserialization errors that happen before the tool'scall()method).