Skip to content

Conversation

@nan-yu
Copy link
Collaborator

@nan-yu nan-yu commented Jan 27, 2026

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.

@nan-yu
Copy link
Collaborator Author

nan-yu commented Jan 27, 2026

No functional changes. Only formatting the ADK samples to reduce diff size for future commits.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
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)

Comment on lines +185 to +193
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)"} } } } ] } }]'
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There's a typo in the method name. It should be programmatically.

Suggested change
async def programmtically_route_user_action_to_subagent(
async def programmatically_route_user_action_to_subagent(

Comment on lines +40 to +42
server_to_client_json["properties"]["surfaceUpdate"]["properties"]["components"][
"items"
]["properties"]["component"]["properties"] = standard_catalog_json
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Comment on lines +42 to +44
restaurant_data_str = restaurant_data_str.replace(
"http://localhost:10002", base_url
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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)

Comment on lines +92 to +94
a2ui_schema_json["properties"]["surfaceUpdate"]["properties"]["components"][
"items"
]["properties"]["component"]["properties"] = catalog_json
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

1 participant