Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
examples/voice_agents/sms_agent.py
Outdated
| result = await session.run(user_input=ctx.text) | ||
|
|
||
| await ctx.send_result(result) | ||
| await ctx.save_session(session) |
There was a problem hiding this comment.
Q: how do you feel about automatically saving with a context manager?
async with ctx.resume(session=session, session_data=session_data, agent=...):
...
so users don't have to call ctx.save_session explicitly?
examples/voice_agents/sms_agent.py
Outdated
| result = await session.run(user_input=ctx.text) | ||
|
|
||
| await ctx.send_result(result) | ||
| await ctx.save_session(session) |
There was a problem hiding this comment.
So for rtc_session, we have operations at the session level:
- session.generate_reply
- session.say
- session.run
It feels out of sync to me if we have to use ctx to send_result. WDYT?
There was a problem hiding this comment.
Agree it’s a bit odd to have this “out of sync” issue. I think the agent code shouldn’t be sending SMS outside the sms_handler tho. Just for separation of concerns.
| await sms_handler(text_context) | ||
|
|
||
| # serialize the state of the session | ||
| if text_context.session_data: |
There was a problem hiding this comment.
QQ: how about we handle this inside ctx.save_session?
examples/voice_agents/sms_agent.py
Outdated
| self._greet_on_enter = greet_on_enter | ||
|
|
||
| async def on_enter(self): | ||
| if self._greet_on_enter: |
There was a problem hiding this comment.
Do we want to add a function called on_rehydrate?
There was a problem hiding this comment.
This is technically going to be the default Python def __setstate__ method.
| async def aclose(self) -> None: | ||
| await self._aclose_impl(reason=CloseReason.USER_INITIATED) | ||
|
|
||
| def get_state(self) -> dict[str, Any]: |
There was a problem hiding this comment.
Should we standardize this with __setstate__ and __getstate__ functions?
There was a problem hiding this comment.
Fwiw, for the AgentSession, I think this method should stay private
There was a problem hiding this comment.
I didn't use __setstate__ and __getstate__ because we cannot load the AgentSession using pickle.loads(data) directly if it's required to create the AgentSession in user's code.
| tool_ctx = llm.ToolContext(self.tools) | ||
| return { | ||
| "tools": list(tool_ctx.function_tools.keys()), | ||
| "chat_ctx": self._chat_ctx.to_dict( |
There was a problem hiding this comment.
On the AgentSession, we should name it history
| "chat_ctx": self._chat_ctx.to_dict( | |
| "history": self._chat_ctx.to_dict( |
|
|
||
| return self._activity | ||
|
|
||
| def __getstate__(self) -> dict[str, Any]: |
There was a problem hiding this comment.
How should it looks like from the user perspective when they add their own states, should they override this function and call super().__getstate__?
examples/voice_agents/sms_agent.py
Outdated
| async def sms_handler(ctx: TextMessageContext): | ||
| logger.info(f"SMS received: {ctx.text}") | ||
|
|
||
| session = AgentSession(llm="openai/gpt-4.1-mini") |
There was a problem hiding this comment.
Can we let user to setup some config for sms handler here?
Something like that:
- `conversation_id` (required) — unique identifier for the conversation partner (Default to ctx.participant_number?)
- `retention_hours` (optional, default 168 = 7 days) — how long to store session
- `max_tokens` (optional) — limit on loaded context size
- `encryption_key` (optional) — for E2E encryption
There was a problem hiding this comment.
And maybe:
auto_save — almost always True. False only if developer wants conditional saving (e.g., don't save if user asked to "forget" instantly)
examples/voice_agents/sms_agent.py
Outdated
| server = AgentServer() | ||
|
|
||
|
|
||
| @server.sms_handler() |
There was a problem hiding this comment.
let's position this as a text_handler, instead of sms. getting text right would mean this is accessible on LK Cloud: https://myapp.livekit.cloud/agents/name/handler
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
5 participants
to test it