feat(sessions): Add session title management functionality #3703
Conversation
Summary of ChangesHello @zhangp365, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a crucial feature for user experience by enabling the management of titles for user sessions. Previously, chat histories lacked readable titles, making it difficult for users to distinguish between different sessions. This change addresses that by integrating a Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
adk-bot
added
the
services
![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 introduces a valuable feature for managing session titles, which will improve the user experience by providing readable titles in the chat history. The changes are well-implemented across different session services, including in-memory, database, SQLite, and Vertex AI. The implementation is solid and includes corresponding unit tests to ensure correctness. I've provided a few suggestions to improve API consistency, code clarity, and efficiency.
| if ( | ||
| app_name not in self.sessions | ||
| or user_id not in self.sessions[app_name] | ||
| or session_id not in self.sessions[app_name][user_id] | ||
| ): | ||
| raise ValueError( | ||
| f'Session not found: app_name={app_name}, user_id={user_id},' | ||
| f' session_id={session_id}' | ||
| ) | ||
| self.sessions[app_name][user_id][session_id].title = title |
There was a problem hiding this comment.
The check for session existence is a bit verbose. It can be written more concisely and safely using .get() with default values. This makes the code more readable and less prone to errors.
session = self.sessions.get(app_name, {}).get(user_id, {}).get(session_id)
if session is None:
raise ValueError(
f'Session not found: app_name={app_name}, user_id={user_id},'
f' session_id={session_id}'
)
session.title = title| async def update_session_title( | ||
| self, | ||
| *, | ||
| app_name: str, | ||
| user_id: str, | ||
| session_id: str, | ||
| title: Optional[str], | ||
| ) -> None: | ||
| async with self._get_db_connection() as db: | ||
| async with db.execute( | ||
| "SELECT 1 FROM sessions WHERE app_name=? AND user_id=? AND id=?", | ||
| (app_name, user_id, session_id), | ||
| ) as cursor: | ||
| if not await cursor.fetchone(): | ||
| raise ValueError( | ||
| f"Session not found: app_name={app_name}, user_id={user_id}," | ||
| f" session_id={session_id}" | ||
| ) | ||
| await db.execute( | ||
| "UPDATE sessions SET title=? WHERE app_name=? AND user_id=? AND id=?", | ||
| (title, app_name, user_id, session_id), | ||
| ) | ||
| await db.commit() |
There was a problem hiding this comment.
This implementation uses two separate database queries (SELECT then UPDATE) to update the session title. This can be optimized into a single, more efficient, and atomic UPDATE operation. You can check the cursor.rowcount after the UPDATE to see if any rows were affected. If rowcount is 0, it means the session was not found, and you can raise the ValueError.
async def update_session_title(
self,
*,
app_name: str,
user_id: str,
session_id: str,
title: Optional[str],
) -> None:
async with self._get_db_connection() as db:
cursor = await db.execute(
"UPDATE sessions SET title=? WHERE app_name=? AND user_id=? AND id=?",
(title, app_name, user_id, session_id),
)
if cursor.rowcount == 0:
raise ValueError(
f"Session not found: app_name={app_name}, user_id={user_id},"
f" session_id={session_id}"
)
await db.commit()| except Exception as e: | ||
| logger.error('Error updating session title %s: %s', session_id, e) | ||
| raise |
There was a problem hiding this comment.
Catching a broad Exception can hide other unexpected errors. It's better to catch a more specific exception, such as api_core_exceptions.GoogleAPICallError, which is the base class for errors from the Google API client. This makes the error handling more robust.
You'll need to add from google.api_core import exceptions as api_core_exceptions at the top of the file.
| except Exception as e: | |
| logger.error('Error updating session title %s: %s', session_id, e) | |
| raise | |
| except api_core_exceptions.GoogleAPICallError as e: | |
| logger.error('Error updating session title %s: %s', session_id, e) | |
| raise |
There was a problem hiding this comment.
No, this exception should be kept as other methods.
… optional title parameter
|
Hi @zhangp365 , Thank you for your work on this pull request. We appreciate the effort you've invested. |
ryanaiagent
added
the
request clarification
have fixed it. |
3 participants
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
title#1340 feat: Add title attribute to Session model #1372Problem:
There is no title field in the sessions table, so when the frontend tries to display the chat history to users, there are no readable titles in the history list. In contrast, when users interact with other agents, the history list always contains readable titles.
Solution:
This update introduces the ability to set, update, and clear titles for user sessions across various session services. The following changes were made:
titleattribute to theSessionmodel.update_session_titlemethod inBaseSessionServiceand its concrete implementations (e.g.,DatabaseSessionService,InMemorySessionService,SqliteSessionService,VertexAiSessionService).adk_web_server.pyto include a new endpoint for updating session titles.Unit Tests
Manual End-to-End (E2E) Tests
I have installed the updated code on both my local machine and my server, started adk web, and run the agent. Everything works as expected with no issues.
Checklist