Conversation
|
No functional changes. Only formatting the ADK samples to reduce diff size for future commits. |
![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 applies pyink formatting across the Python sample files, which is a great step for code consistency. The changes are mostly stylistic, involving indentation and quote style.
I've added a few comments on areas that could be improved for better maintainability and correctness:
- Replacing magic strings (hardcoded URLs) with constants.
- Refactoring a large, hardcoded JSON string into a more manageable form.
- Improving the robustness of deep dictionary access.
- Correcting a typo in a method name.
Overall, the formatting changes look good. Addressing the suggested improvements will further enhance the code quality.
| with open(file_path) as f: | ||
| contact_data_str = f.read() | ||
| if base_url := tool_context.state.get("base_url"): | ||
| contact_data_str = contact_data_str.replace("http://localhost:10002", base_url) |
There was a problem hiding this comment.
The hardcoded URL "http://localhost:10002" is a magic string. It's better to define it as a constant at the module level (e.g., DEFAULT_DATA_BASE_URL = "http://localhost:10002") to improve readability and maintainability. This makes it easier to find and update if the default URL changes.
| contact_data_str = contact_data_str.replace("http://localhost:10002", base_url) | |
| contact_data_str = contact_data_str.replace(DEFAULT_DATA_BASE_URL, base_url) |
| json_content = ( | ||
| '[{ "beginRendering": { "surfaceId": "action-modal", "root":' | ||
| ' "modal-wrapper" } }, { "surfaceUpdate": { "surfaceId": "action-modal",' | ||
| ' "components": [ { "id": "modal-wrapper", "component": { "Modal": {' | ||
| ' "entryPointChild": "hidden", "contentChild": "msg", "open": true } } },' | ||
| ' { "id": "hidden", "component": { "Text": { "text": {"literalString": "' | ||
| ' "} } } }, { "id": "msg", "component": { "Text": { "text":' | ||
| ' {"literalString": "Message Sent (Fallback)"} } } } ] } }]' | ||
| ) |
There was a problem hiding this comment.
This hardcoded fallback JSON string is very long and difficult to read and maintain. Consider moving it to a separate constant or loading it from a file, similar to how other examples are handled. This would improve readability and make it easier to update the fallback UI.
| SUPPORTED_CONTENT_TYPES = ["text", "text/plain"] | ||
|
|
||
| @classmethod | ||
| async def programmtically_route_user_action_to_subagent( |
There was a problem hiding this comment.
There's a typo in the method name. It should be programmatically.
| async def programmtically_route_user_action_to_subagent( | |
| async def programmatically_route_user_action_to_subagent( |
| server_to_client_json["properties"]["surfaceUpdate"]["properties"]["components"][ | ||
| "items" | ||
| ]["properties"]["component"]["properties"] = standard_catalog_json |
There was a problem hiding this comment.
This deep dictionary access is brittle and can easily break if the schema structure changes. Consider creating a helper function to navigate and update the schema. This would encapsulate the logic, make it more robust against future schema changes, and improve readability.
| restaurant_data_str = restaurant_data_str.replace( | ||
| "http://localhost:10002", base_url | ||
| ) |
There was a problem hiding this comment.
The hardcoded URL "http://localhost:10002" is a magic string. It's better to define it as a constant at the module level (e.g., DEFAULT_DATA_BASE_URL = "http://localhost:10002") to improve readability and maintainability. This makes it easier to find and update if the default URL changes.
restaurant_data_str = restaurant_data_str.replace(DEFAULT_DATA_BASE_URL, base_url)| a2ui_schema_json["properties"]["surfaceUpdate"]["properties"]["components"][ | ||
| "items" | ||
| ]["properties"]["component"]["properties"] = catalog_json |
There was a problem hiding this comment.
This deep dictionary access is brittle and can easily break if the schema structure changes. Consider creating a helper function to navigate and update the schema. This would encapsulate the logic, make it more robust against future schema changes, and improve readability.
1 participant
Description
Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.
List which issues are fixed by this PR. For larger changes, raising an issue first helps reduce redundant work.
Pre-launch Checklist
If you need help, consider asking for advice on the discussion board.