-
Notifications
You must be signed in to change notification settings - Fork 2
[LiveKit] Adds feature to add custom session name to LiveKit sessions #92
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.
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
base: main
Are you sure you want to change the base?
[LiveKit] Adds feature to add custom session name to LiveKit sessions #92
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughSummary by CodeRabbitRelease Notes – Version 3.12.1
WalkthroughAdds a MaximWrappedAgentSession wrapper to accept custom session_name and tags, applies those parameters when starting LiveKit sessions, implements chunked conversation attachments (>10 MB) in realtime_session, updates tests to use the wrapper, and bumps version to 3.12.1. Changes
Sequence Diagram(s)sequenceDiagram
actor TestRunner
participant MaximWrappedAgentSession
participant AgentSessionModule as agent_session.py
participant LiveKit
TestRunner->>MaximWrappedAgentSession: instantiate(maxim_params={session_name,tags}, ...)
MaximWrappedAgentSession->>MaximWrappedAgentSession: store _maxim_params
MaximWrappedAgentSession->>LiveKit: super().__init__(...)
TestRunner->>AgentSessionModule: on_session_started()
AgentSessionModule->>MaximWrappedAgentSession: get_all_maxim_params()
MaximWrappedAgentSession-->>AgentSessionModule: return maxim_params
rect rgb(220,240,255)
AgentSessionModule->>AgentSessionModule: set custom_session_name & custom_tags
AgentSessionModule->>LiveKit: create session(name=custom_session_name, tags=custom_tags)
end
LiveKit-->>TestRunner: session started event
sequenceDiagram
actor User
participant RealtimeSession as realtime_session.py
participant ConversationBuffer
User->>RealtimeSession: handle_off()
RealtimeSession->>ConversationBuffer: read buffer (BytesIO)
ConversationBuffer-->>RealtimeSession: bytes, size
alt size > 10 MB
rect rgb(255,245,230)
RealtimeSession->>RealtimeSession: split into 10MB chunks
loop per chunk
RealtimeSession->>RealtimeSession: create FileDataAttachment("Conversation part {i}")
RealtimeSession->>RealtimeSession: increment conversation_buffer_index
end
end
else size <= 10 MB
rect rgb(235,255,240)
RealtimeSession->>RealtimeSession: create single FileDataAttachment("Conversation")
RealtimeSession->>RealtimeSession: increment conversation_buffer_index
end
end
RealtimeSession->>ConversationBuffer: reset to empty BytesIO
RealtimeSession->>RealtimeSession: persist session
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds the ability to specify custom session names and tags for LiveKit sessions through a new MaximWrappedAgentSession wrapper class, replacing the previous callback-based approach for session naming. It also fixes an issue where conversation attachments were being added multiple times.
Key changes:
- Introduced
MaximWrappedAgentSessionwrapper class to accept custom session metadata (name and tags) - Updated session instrumentation to extract and use custom parameters from the wrapper
- Fixed conversation buffer handling to prevent duplicate attachments
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Version bump to 3.12.1 |
| maxim/logger/livekit/wrapped_agent_session.py | New wrapper class for AgentSession with custom Maxim parameters |
| maxim/logger/livekit/agent_session.py | Updated to extract and use custom session name and tags from wrapped sessions |
| maxim/logger/livekit/realtime_session.py | Fixed conversation buffer handling to prevent duplicate attachments and handle large buffers |
| maxim/tests/test_livekit_realtime.py | Updated to use MaximWrappedAgentSession with custom parameters |
| maxim/tests/test_livekit_ptt.py | Updated to use MaximWrappedAgentSession with custom parameters |
| maxim/tests/test_livekit_interview_agent.py | Updated to use MaximWrappedAgentSession and switched from Google to OpenAI model |
| README.md | Added changelog entry for version 3.12.1 |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (4)
README.md (1)
63-66: Clarify wording and point to the new wrapper.Suggest minor edits for clarity and discoverability.
-### 3.12.1 -- feat: Added the ability to add custom session name and tags to LiveKit session -- fix: Fixes conversation attachment being added multiple times +### 3.12.1 +- feat: Added custom session name and tags for LiveKit sessions via `MaximWrappedAgentSession` +- fix: Fixed conversation attachment being added multiple timesmaxim/logger/livekit/wrapped_agent_session.py (1)
63-77: Defensive copy of incoming params.Avoid caller mutating
maxim_paramsafter init.- # Store the additional parameters - self._maxim_params: MaximParams = maxim_params or {} + # Store a shallow copy to avoid external mutation + self._maxim_params: MaximParams = dict(maxim_params) if maxim_params else {}maxim/logger/livekit/agent_session.py (1)
116-121: Optional: add explicitagent_session_idtag to avoid ambiguity.Current
session_idtag usesid(self)(agent-session id), which can be confused with Maxim session id.- tags["session_id"] = str(id(self)) + tags["session_id"] = str(id(self)) + tags["agent_session_id"] = str(id(self))maxim/tests/test_livekit_interview_agent.py (1)
131-133: Consider addingmaxim_paramsto demonstrate the new feature.While this correctly uses
MaximWrappedAgentSession, it doesn't pass the optionalmaxim_paramsargument. Since the PR's purpose is to add custom session naming and tagging, consider demonstrating this feature in the test:session = MaximWrappedAgentSession( llm=openai.realtime.RealtimeModel(voice="alloy"), + maxim_params={ + "session_name": "interview-session", + "tags": {"interview_type": "audio_engineer", "jd_provided": "true"} + } )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
README.md(1 hunks)maxim/logger/livekit/agent_session.py(4 hunks)maxim/logger/livekit/realtime_session.py(2 hunks)maxim/logger/livekit/wrapped_agent_session.py(1 hunks)maxim/tests/test_livekit_interview_agent.py(2 hunks)maxim/tests/test_livekit_ptt.py(2 hunks)maxim/tests/test_livekit_realtime.py(2 hunks)pyproject.toml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
maxim/logger/livekit/realtime_session.py (4)
maxim/logger/livekit/store.py (3)
get_maxim_logger(80-84)get_session_store(221-223)set_session(129-140)maxim/logger/logger.py (1)
session_add_attachment(257-265)maxim/logger/components/attachment.py (1)
FileDataAttachment(41-74)maxim/logger/utils.py (1)
pcm16_to_wav_bytes(51-77)
maxim/logger/livekit/agent_session.py (3)
maxim/logger/livekit/wrapped_agent_session.py (1)
get_all_maxim_params(91-98)maxim/scribe.py (2)
scribe(127-145)debug(42-52)maxim/logger/components/base.py (1)
add_tag(268-284)
maxim/tests/test_livekit_realtime.py (2)
maxim/logger/livekit/wrapped_agent_session.py (1)
MaximWrappedAgentSession(40-98)maxim/logger/logger.py (1)
session(185-195)
maxim/tests/test_livekit_ptt.py (2)
maxim/logger/livekit/instrumenter.py (1)
instrument_livekit(16-123)maxim/logger/livekit/wrapped_agent_session.py (1)
MaximWrappedAgentSession(40-98)
maxim/tests/test_livekit_interview_agent.py (2)
maxim/logger/livekit/instrumenter.py (1)
instrument_livekit(16-123)maxim/logger/livekit/wrapped_agent_session.py (1)
MaximWrappedAgentSession(40-98)
🪛 Ruff (0.14.1)
maxim/logger/livekit/realtime_session.py
173-173: f-string without any placeholders
Remove extraneous f prefix
(F541)
🔇 Additional comments (8)
pyproject.toml (1)
7-7: Version bump aligns with changelog.3.12.1 matches README. No further action.
maxim/logger/livekit/wrapped_agent_session.py (1)
78-99: API surface looks good.Getter methods are minimal and sufficient; returning a copy prevents accidental mutation.
maxim/logger/livekit/agent_session.py (1)
81-86: Session creation with custom name looks good.Name flows into Maxim session as intended.
maxim/tests/test_livekit_interview_agent.py (1)
9-9: LGTM on import updates.The switch from Google plugin to OpenAI plugin and the addition of
MaximWrappedAgentSessionimport are appropriate for the new session wrapper implementation.Also applies to: 14-14
maxim/tests/test_livekit_ptt.py (2)
7-7: LGTM on import updates.The import changes correctly adopt
MaximWrappedAgentSessionfrom the new wrapper module.Also applies to: 11-11
43-43: LGTM! Proper usage of the new feature.The session correctly demonstrates the custom session naming feature with both
session_nameandtagsparameters. This is a good test case for the new functionality.maxim/tests/test_livekit_realtime.py (2)
14-14: LGTM on import addition.The import of
MaximWrappedAgentSessionis correctly added to support the new session wrapper.
83-86: LGTM! Proper usage of the new feature.The session correctly demonstrates the custom session naming feature with both
session_nameandtagsparameters. This properly showcases the new functionality being added in this PR.
6ac8b6e to
bfb075c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
maxim/logger/livekit/realtime_session.py (1)
138-183: Address the buffer handling issues raised in previous review.The chunked attachment implementation still has the issues previously identified:
- OOM risk:
getvalue()loads the entire buffer into memory- Audio artifacts: Chunk boundaries are not aligned to 16-bit samples
- Missing validation: No check for empty buffers
- Missing metadata:
mime_typenot set to "audio/wav"- Static analysis: Line 173 uses an unnecessary f-string for a constant
The previous review comment provides a comprehensive streaming solution that addresses all of these concerns. Please implement the suggested fix.
🧹 Nitpick comments (1)
maxim/logger/livekit/__init__.py (1)
2-4: Export looks correct.The
MaximWrappedAgentSessionimport and export enable the documented usage pattern. The static analysis suggestion to alphabetically sort__all__is a minor style preference that you may optionally apply.If you'd like to address the sorting suggestion:
-__all__ = ["instrument_livekit", "MaximWrappedAgentSession"] +__all__ = ["MaximWrappedAgentSession", "instrument_livekit"]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
README.md(1 hunks)maxim/logger/livekit/__init__.py(1 hunks)maxim/logger/livekit/agent_session.py(4 hunks)maxim/logger/livekit/realtime_session.py(2 hunks)maxim/logger/livekit/wrapped_agent_session.py(1 hunks)maxim/tests/test_livekit_interview_agent.py(2 hunks)maxim/tests/test_livekit_ptt.py(2 hunks)maxim/tests/test_livekit_realtime.py(2 hunks)pyproject.toml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- README.md
- maxim/tests/test_livekit_interview_agent.py
- maxim/logger/livekit/agent_session.py
- maxim/tests/test_livekit_ptt.py
🧰 Additional context used
🧬 Code graph analysis (3)
maxim/logger/livekit/realtime_session.py (4)
maxim/logger/livekit/store.py (3)
get_maxim_logger(80-84)get_session_store(221-223)set_session(129-140)maxim/logger/logger.py (1)
session_add_attachment(257-265)maxim/logger/components/attachment.py (1)
FileDataAttachment(41-74)maxim/logger/utils.py (1)
pcm16_to_wav_bytes(51-77)
maxim/tests/test_livekit_realtime.py (1)
maxim/logger/livekit/wrapped_agent_session.py (1)
MaximWrappedAgentSession(40-98)
maxim/logger/livekit/__init__.py (1)
maxim/logger/livekit/wrapped_agent_session.py (1)
MaximWrappedAgentSession(40-98)
🪛 Ruff (0.14.1)
maxim/logger/livekit/realtime_session.py
173-173: f-string without any placeholders
Remove extraneous f prefix
(F541)
maxim/logger/livekit/__init__.py
4-4: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
🔇 Additional comments (5)
pyproject.toml (1)
7-7: Version bump looks appropriate.The patch version increment (3.12.0 → 3.12.1) aligns with the addition of the new
MaximWrappedAgentSessionpublic API.maxim/tests/test_livekit_realtime.py (1)
83-86: Test implementation looks good.The test correctly demonstrates the new
MaximWrappedAgentSessionfeature, passing customsession_nameandtagsviamaxim_params. This provides a clear usage example for consumers of the API.maxim/logger/livekit/wrapped_agent_session.py (3)
29-37:MaximParamstype definition is well-designed.The
TypedDictwithtotal=Falsecorrectly allows optionalsession_nameandtagsfields, providing type safety while maintaining flexibility.
63-76: Constructor implementation is solid.The wrapper cleanly forwards arguments to the parent
AgentSessionwhile storing the additionalmaxim_params. The default empty dictionary handling is appropriate.
78-98: Helper methods are well-implemented.Both
get_maxim_paramandget_all_maxim_paramsfollow best practices:
- Safe dictionary access with defaults
- Defensive copying to prevent unintended mutations
- Clear documentation

Uh oh!
There was an error while loading. Please reload this page.