Skip to content

feat: Add user_id property to ReadonlyContext #2875

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: Add user_id property to ReadonlyContext #2875

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/google/adk/agents/readonly_context.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,8 @@ def agent_name(self) -> str:
def state(self) -> MappingProxyType[str, Any]:
"""The state of the current session. READONLY field."""
return MappingProxyType(self._invocation_context.session.state)

@property
def user_id(self) -> str:
Copy link

Choose a reason for hiding this comment

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

high

The user_id property is typed to return a str. While the underlying InvocationContext also types it as str, it's common for user identifiers to be optional (e.g., for anonymous sessions). If _invocation_context.user_id were to return None, this property would violate its type hint, potentially leading to runtime errors in client code.

To make the API more robust, I suggest changing the return type to Optional[str]. This makes it explicit that user_id can be missing and requires consumers to handle this case.

Suggested change
def user_id(self) -> str:
def user_id(self) -> Optional[str]:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the feedback. I understand your concern about the robustness of the API. However, I believe the current implementation with str return type is correct based on the codebase design.

Looking at the Session base class and its implementations (specifically InMemorySessionService), the user_id is a required field that cannot be None:

  1. The Session base class defines user_id as a non-optional str field
  2. In InMemorySessionService, sessions are created with a required user_id parameter (line 106 in src/google/adk/sessions/in_memory_session_service.py)
  3. The InvocationContext that wraps the session also treats user_id as a required str

This architectural decision means that anonymous sessions are not supported in the current design - every session must have an associated user_id. Making it Optional[str] would imply that anonymous sessions are possible, which would be misleading given the current implementation constraints.

If anonymous sessions become a requirement in the future, it would need to be a broader architectural change affecting the Session base class and all its implementations, not just this readonly accessor.

Therefore, keeping the return type as str accurately reflects the current API contract and ensures type consistency throughout the codebase.

"""The id of the user. READONLY field."""
return self._invocation_context.user_id
6 changes: 6 additions & 0 deletions tests/unittests/agents/test_readonly_context.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ def mock_invocation_context():
mock_context.invocation_id = "test-invocation-id"
mock_context.agent.name = "test-agent-name"
mock_context.session.state = {"key1": "value1", "key2": "value2"}
mock_context.user_id = "test-user-id"
return mock_context


Expand All @@ -31,3 +32,8 @@ def test_state_content(mock_invocation_context):
assert isinstance(state, MappingProxyType)
assert state["key1"] == "value1"
assert state["key2"] == "value2"


def test_user_id(mock_invocation_context):
readonly_context = ReadonlyContext(mock_invocation_context)
assert readonly_context.user_id == "test-user-id"