-
Notifications
You must be signed in to change notification settings - Fork 615
feat: Enable Rizzcharts sample and enhance client message handling #396
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Update rizzcharts agent to support map images (Google Maps API or placeholder). - Add rizzcharts demo scripts and config to Lit client. - Fix MIME type constant in client (application/json+a2ui). - Improve client robustness: fallback to history for agent messages and handle string-encoded JSON data parts. - Add debug logging to client.
|
suyang/jon/yuzuru do you mind taking a look? thanks! |
sugoi-yuzuru
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All changes look good.
Thank you for noticing the json+a2aui MIME_TYPE issue. It seems it is more prevalent in the codebase that there are more places needed to be adjusted. Would you kindly isolate this small refactor into its own fix/refactor PR?
Changes for the lit looks good too. Can we isolate this change into its own PR to keep the scope of the commit as narrow and targetted as possible?
For Rizzcharts, the change look good too. However, I have some reservations in regards to sourcing image from an external service. Can we simply add a placeholder image within the codebase and use that instead of pointing to a thirdparty service?
| """ | ||
| else: | ||
| map_image_instruction = """ | ||
| **Map Image URL:** When constructing map visualizations, use a placeholder image URL exactly as shown in the example template. Do NOT attempt to use Google Maps Static API or any other external map service. Use exactly: `https://placehold.co/600x400?text=Map+Placeholder` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of depending on an external service, can we serve the image directly from the client app from the public directory? This will reduce concerns for placehold.co availability.
| import { v0_8 } from "@a2ui/lit"; | ||
|
|
||
| const A2AUI_MIME_TYPE = "application/json+a2aui"; | ||
| const A2AUI_MIME_TYPE = "application/json+a2ui"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huge thanks for the fix.
Could we rename the const to A2UI_MIME_TYPE as well?
Also, it would be great if we can decouple this fix into its own PR to limit the scope of the commit for posterity.
Hi @sugoi-yuzuru, Thank you for the detailed review! I've split the changes from this PR into three smaller, more focused PRs to keep the scope narrow and targeted as requested:
I will close this PR now in favor of these individual PRs. Please take a look at them when you have a chance. Thanks again! |
Summary
This PR enables the rizzcharts sample agent with improved map support and significantly hardens the Lit client's message handling capabilities. It addresses issues with MIME type definitions and improves the client's ability to handle various data formats.
Changes
Rizzcharts Agent
Map Support: Updated
agent.pyandmap.jsonto support map images, allowing for either Google Maps API integration or fallback placeholders.Lit Client Improvements
Robust Message Handling:
Configuration:
application/json+a2ui.console.debug) for better traceability.Demo Setup:
samples/client/lit/package.json.Verification