Fix transcript and memory handling in openai realtime integration#376
Fix transcript and memory handling in openai realtime integration#376
Conversation
|
""" WalkthroughThis set of changes includes both code and configuration updates across several files. The Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Complexity label: Complex The review involves significant changes to core session and event handling logic, including asynchronous event processing, metadata tracking, and context management, particularly in Possibly related PRs
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
pyproject.toml(1 hunks)src/draive/openai/lmm_session.py(11 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.py: Use absolute imports fromdraivepackage
Follow Ruff import ordering (standard library, third party, local)
Use Python 3.12+ type features (type unions with|, generic syntax)
Use base and abstract types likeSequenceorIterableinstead of concrete types
Use custom exceptions for specific errors
Use NumPy docstring convention for all functions, classes, and methods
Skip module-level docstrings unless explicitly requested
Include sections: Parameters, Returns, Raises, Notes (if needed) in docstrings
ALWAYS useSequence[T]instead oflist[T],Mapping[K,V]instead ofdict[K,V], andSet[T]instead ofset[T]for collections in State, Config and DataModel classes
Immutable updates to State, Config, and DataModel classes should be performed through the.updated()method
Files:
src/draive/openai/lmm_session.py
🧠 Learnings (1)
pyproject.toml (3)
Learnt from: KaQuMiQ
PR: #338
File: src/draive/lmm/init.py:1-2
Timestamp: 2025-06-16T10:28:07.434Z
Learning: The draive project requires Python 3.12+ as specified in pyproject.toml with "requires-python = ">=3.12"" and uses Python 3.12+ specific features like PEP 695 type aliases and generic syntax extensively throughout the codebase.
Learnt from: CR
PR: miquido/draive#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T11:09:57.256Z
Learning: Applies to **/*.py : Use absolute imports from draive package
Learnt from: KaQuMiQ
PR: #327
File: src/draive/helpers/instruction_preparation.py:28-34
Timestamp: 2025-05-28T17:41:57.460Z
Learning: The draive project uses and requires Python 3.12+, so PEP-695 generic syntax with square brackets (e.g., def func[T: Type]()) is valid and should be used instead of the older TypeVar approach.
🔇 Additional comments (7)
pyproject.toml (1)
8-8: LGTM: Appropriate version bump for bug fixes.The version increment from "0.80.0" to "0.80.1" follows semantic versioning conventions for a patch release, which aligns with the PR's focus on fixing transcript and memory handling issues.
src/draive/openai/lmm_session.py (6)
197-208: LGTM: Consistent transcript completion handling.The audio transcript completion event handling follows the established pattern and correctly signals transcript completion.
267-267: LGTM: Good defensive programming for None content.The
or ()addition properly handles cases whereevent.item.contentmight be None, preventing potential errors.
278-288: LGTM: Consistent EOD signaling.Always sending an empty EOD chunk ensures reliable response completion signaling, which is important for proper session management.
322-322: LGTM: Improved comment clarity.The updated comment better explains why non-user events are skipped in this context.
337-354: LGTM: Proper transcription completion handling.The new event handler correctly processes completed transcriptions and maintains consistency with the transcript streaming pattern. The assertion ensures transcription is properly configured when this event occurs.
233-233: Behavioral change verified: EOD now sent inresponse.output_item.doneThe
response.donecase (line 233) no longer emits an EOD chunk—it only records usage and continues. The end-of-data marker is instead emitted in theresponse.output_item.done→messagehandler:
- src/draive/openai/lmm_session.py lines 235–262:
- Under
case "response.output_item.done"→case "message", the final
return LMMStreamChunk.of(..., eod=True, ...)sends the empty EOD chunk.This matches the pattern in other LLM adapters and preserves correct completion signaling. No further changes required.
KaQuMiQ
force-pushed
the
feature/realtime_memory
branch
6 times, most recently
from
8f0329c to
48555b4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/draive/openai/lmm_session.py (2)
162-162: Fix typo in comment.Apply this diff to fix the typo:
- # send the text chuink + # send the text chunk
186-186: Fix typo in comment.The same typo appears here as well.
Apply this diff to fix the typo:
- # send the text chuink + # send the text chunk
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
pyproject.toml(1 hunks)src/draive/conversation/realtime/default.py(1 hunks)src/draive/openai/lmm_session.py(10 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.py: Use absolute imports fromdraivepackage
Follow Ruff import ordering (standard library, third party, local)
Use Python 3.12+ type features (type unions with|, generic syntax)
Use base and abstract types likeSequenceorIterableinstead of concrete types
Use custom exceptions for specific errors
Use NumPy docstring convention for all functions, classes, and methods
Skip module-level docstrings unless explicitly requested
Include sections: Parameters, Returns, Raises, Notes (if needed) in docstrings
ALWAYS useSequence[T]instead oflist[T],Mapping[K,V]instead ofdict[K,V], andSet[T]instead ofset[T]for collections in State, Config and DataModel classes
Immutable updates to State, Config, and DataModel classes should be performed through the.updated()method
Files:
src/draive/conversation/realtime/default.pysrc/draive/openai/lmm_session.py
🧠 Learnings (1)
pyproject.toml (2)
Learnt from: KaQuMiQ
PR: #338
File: src/draive/lmm/init.py:1-2
Timestamp: 2025-06-16T10:28:07.434Z
Learning: The draive project requires Python 3.12+ as specified in pyproject.toml with "requires-python = ">=3.12"" and uses Python 3.12+ specific features like PEP 695 type aliases and generic syntax extensively throughout the codebase.
Learnt from: CR
PR: miquido/draive#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T11:09:57.256Z
Learning: Applies to **/*.py : Use absolute imports from draive package
🔇 Additional comments (7)
pyproject.toml (1)
8-8: LGTM! Version bump is appropriate.The patch version increment from 0.80.0 to 0.80.1 correctly reflects the bug fixes for transcript and memory handling.
src/draive/conversation/realtime/default.py (2)
117-122: Event handling simplification looks good.The generalized event handling approach with category-based logging and metadata preservation provides better extensibility for handling new event types introduced in the LMM session layer.
124-124: Good addition of debug logging for tool requests.The debug log helps trace which tools are being requested, improving observability.
src/draive/openai/lmm_session.py (4)
87-89: Good fix for websocket message size limits.Explicitly setting
max_size: Noneremoves any size restrictions on websocket messages, which is important for handling large audio transcripts and multimodal content.
185-211: Good implementation of audio transcript streaming support.The new event handlers for
response.audio_transcript.deltaandresponse.audio_transcript.doneproperly handle streaming transcripts using MetaContent with appropriate metadata.
262-283: Improved message completion handling.The changes ensure that full message content is retrieved before signaling end-of-data, which prevents incomplete messages in the conversation memory.
320-363: Excellent unification of memory handling.The new
conversation.item.retrievedhandler with retry logic ensures complete items are stored in memory. The separation of user and assistant roles with proper LMMInput/LMMCompletion creation is well implemented.
KaQuMiQ
force-pushed
the
feature/realtime_memory
branch
8 times, most recently
from
e037611 to
c071160
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/draive/openai/lmm_session.py (2)
185-185: Fix typo in comment.Change "chuink" to "chunk" in the comment.
345-345: Consider more defensive assertion handling.Replace the assertion with explicit error handling to avoid potential crashes in production.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
pyproject.toml(2 hunks)src/draive/conversation/completion/default.py(2 hunks)src/draive/conversation/realtime/default.py(2 hunks)src/draive/conversation/types.py(10 hunks)src/draive/mistral/lmm_generation.py(2 hunks)src/draive/openai/lmm_session.py(13 hunks)src/draive/utils/processing.py(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.py: Use absolute imports fromdraivepackage
Follow Ruff import ordering (standard library, third party, local)
Use Python 3.12+ type features (type unions with|, generic syntax)
Use base and abstract types likeSequenceorIterableinstead of concrete types
Use custom exceptions for specific errors
Use NumPy docstring convention for all functions, classes, and methods
Skip module-level docstrings unless explicitly requested
Include sections: Parameters, Returns, Raises, Notes (if needed) in docstrings
ALWAYS useSequence[T]instead oflist[T],Mapping[K,V]instead ofdict[K,V], andSet[T]instead ofset[T]for collections in State, Config and DataModel classes
Immutable updates to State, Config, and DataModel classes should be performed through the.updated()method
Files:
src/draive/mistral/lmm_generation.pysrc/draive/conversation/completion/default.pysrc/draive/conversation/realtime/default.pysrc/draive/utils/processing.pysrc/draive/conversation/types.pysrc/draive/openai/lmm_session.py
🧬 Code Graph Analysis (1)
src/draive/utils/processing.py (3)
src/draive/agents/node.py (1)
identifier(52-53)src/draive/multimodal/content.py (2)
meta(151-164)MultimodalContent(24-236)src/draive/conversation/types.py (1)
of(232-249)
🔇 Additional comments (18)
src/draive/mistral/lmm_generation.py (2)
213-213: Good defensive programming for token usage metrics.The
or 0fallback ensures that metric recording receives a numeric value even if the Mistral API returnsNonefor token counts. This pattern is applied consistently to bothprompt_tokensandcompletion_tokens.Also applies to: 221-221
335-335: Consistent token usage handling in streaming mode.The fallback pattern matches the non-streaming implementation, ensuring consistent behavior across both code paths. This maintains robustness in metric recording for streaming completions.
Also applies to: 343-343
pyproject.toml (1)
8-8: Standard version updates for minor release.The project version increment to 0.81.0 and haiway dependency update to 0.27.3 follow proper semantic versioning practices.
Also applies to: 24-24
src/draive/conversation/completion/default.py (2)
6-6: Clean import update removes unused UUID type.Good practice to remove the unused
UUIDimport while retaininguuid4for identifier generation.
177-177: Identifier type migration from UUID to string.The change from
UUIDtostrwith explicitstr(uuid4())conversion aligns with the systematic identifier type unification across the codebase. String identifiers provide better serialization consistency.src/draive/conversation/realtime/default.py (4)
112-119: Improved identifier extraction using metadata.The shift to metadata-based identifier extraction with
get_str()and string UUID defaults provides more flexible identifier management while maintaining consistency with the codebase-wide string identifier migration.
124-128: Simplified event handling with generic category-based processing.The generic event handling approach reduces complexity while maintaining essential event information through category and metadata. This aligns with the streamlined event processing design.
131-131: Enhanced debug logging for tool requests.Good addition for debugging visibility. The specific tool name in the log message helps track tool request processing.
141-159: Explicit input type handling with proper conversions.The separation of
ConversationMessageChunkandConversationEventhandling with appropriate conversions to LMM types is well-structured and maintains data integrity. The assertion-based type checking is functional but could benefit from more robust type guards in the future.src/draive/utils/processing.py (4)
3-3: Clean import update removes unused UUID type.Consistent with the pattern across other files, removing the unused
UUIDimport while retaininguuid4for generation.
33-33: Parameter type migration maintains consistency.The change from
UUID | Nonetostr | Nonewith explicitstr(uuid4())conversion follows the systematic identifier type unification pattern seen across the codebase.Also applies to: 38-38
44-44: Field type migration with consistent string UUID generation.The field type change to
strwithlambda: str(uuid4())factory maintains lazy generation while ensuring type consistency with the parameter changes.
176-176: Method signature updates maintain type consistency.The parameter type changes to
str | Nonein theProcessing.eventmethod overloads ensure consistency with theProcessingEvent.ofmethod signature updates.Also applies to: 188-188
src/draive/conversation/types.py (2)
23-77: LGTM! Consistent UUID to string migration.The changes correctly replace UUID types with string representations throughout the ConversationMessageChunk class, maintaining consistency with the PR's objective.
230-261: LGTM! UUID to string migration completed successfully.The ConversationEvent class changes are consistent with the overall migration from UUID to string identifiers.
src/draive/openai/lmm_session.py (3)
83-93: Good addition of explicit websocket configuration.Setting
max_size: Noneexplicitly removes size limits on websocket messages, which is important for handling potentially large audio/text streams.
472-551: Well-implemented metadata tracking in helper functions.The updates to
_send_contextand_reset_contextfunctions properly handle the new metadata mapping system, ensuring consistent tracking of conversation items.
707-714: Good use of conditional configuration.Wrapping the turn detection dictionary with
without_missingensures that the configuration is only included whenvad_typeis properly set, preventing invalid configurations.
KaQuMiQ
force-pushed
the
feature/realtime_memory
branch
10 times, most recently
from
08a3494 to
e42f4ce
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/draive/conversation/types.py (1)
195-195: Fix typo in metadata key name.There's a typo in the metadata key:
indentifiershould beidentifierto maintain consistency with the retrieval logic infrom_lmm_contextmethod.Apply this diff to fix the typo:
- indentifier=str(self.identifier), + identifier=str(self.identifier),Also applies to: 206-206
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.gitignore(1 hunks)pyproject.toml(2 hunks)src/draive/conversation/realtime/default.py(2 hunks)src/draive/conversation/types.py(4 hunks)src/draive/lmm/types.py(1 hunks)src/draive/mistral/lmm_generation.py(2 hunks)src/draive/openai/lmm_session.py(15 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.py: Use absolute imports fromdraivepackage
Follow Ruff import ordering (standard library, third party, local)
Use Python 3.12+ type features (type unions with|, generic syntax)
Use base and abstract types likeSequenceorIterableinstead of concrete types
Use custom exceptions for specific errors
Use NumPy docstring convention for all functions, classes, and methods
Skip module-level docstrings unless explicitly requested
Include sections: Parameters, Returns, Raises, Notes (if needed) in docstrings
ALWAYS useSequence[T]instead oflist[T],Mapping[K,V]instead ofdict[K,V], andSet[T]instead ofset[T]for collections in State, Config and DataModel classes
Immutable updates to State, Config, and DataModel classes should be performed through the.updated()method
Files:
src/draive/lmm/types.pysrc/draive/conversation/realtime/default.pysrc/draive/mistral/lmm_generation.pysrc/draive/conversation/types.pysrc/draive/openai/lmm_session.py
🔇 Additional comments (27)
.gitignore (1)
84-84: LGTM!Adding
.zed/to the gitignore is appropriate for excluding Zed editor configuration files, consistent with other editor directories already ignored.pyproject.toml (2)
8-8: Version bump looks appropriate.Minor version increment from 0.80.0 to 0.81.0 is consistent with the fixes and improvements mentioned in the PR title.
24-24: Dependency update is reasonable.The haiway dependency update from ~=0.27.2 to ~=0.27.4 represents a patch-level increment, which is appropriate for bug fixes and minor improvements.
src/draive/lmm/types.py (1)
179-179: Literal reordering has no functional impact.Swapping "result" and "error" in the
LMMToolResponseHandlingtype alias is purely cosmetic since Literal types are unordered. The change may reflect preferred ordering but doesn't affect runtime behavior.src/draive/mistral/lmm_generation.py (4)
213-213: Good defensive programming practice.Adding the
or 0fallback ensures the metric value is always numeric, preventing potential issues ifusage.prompt_tokensis None or falsy.
221-221: Consistent defensive handling.The
or 0fallback forusage.completion_tokensmatches the pattern applied to prompt tokens, ensuring robust metric recording.
335-335: Defensive pattern applied consistently in streaming.The
or 0fallback forusage.prompt_tokensin the streaming path maintains consistency with the non-streaming implementation.
343-343: Consistent fallback handling in streaming.Adding
or 0forusage.completion_tokensin the streaming context ensures metrics are always numeric, completing the defensive pattern.src/draive/conversation/realtime/default.py (3)
121-125: Generic event handling improves maintainability.The shift from specific event category pattern matching to generic event handling simplifies the code and makes it more maintainable. The debug logging provides good visibility into event processing.
128-128: Good addition of debug logging.Adding debug logging for tool requests improves observability and debugging capabilities.
138-156: Improved event handling with explicit input type processing.The write function now properly handles both
ConversationMessageChunkandConversationEventinputs instead of ignoring events. This enhances the conversation system's event processing capabilities and follows the coding guidelines for explicit type handling.src/draive/conversation/types.py (2)
132-132: LGTM: Identifier fallback logic is correctly implemented.The fallback to
uuid4()whenelement.meta.identifierisNoneensures robust identifier assignment for conversation messages created from LMM context elements.Also applies to: 140-140
237-237: No positionalConversationEvent.ofcalls detected—safe to reorderA project-wide grep for
ConversationEvent.of(only returns two call sites, both using keyword arguments for all parameters:
- src/draive/conversation/completion/default.py
- src/draive/conversation/realtime/default.py
Since neither caller relies on positional args for
content(or any other parameter), the reordering introduces no breaking changes.src/draive/openai/lmm_session.py (14)
2-2: LGTM: Import additions support the enhanced functionality.The new imports (TaskGroup, MutableMapping, META_EMPTY, without_missing) are properly added to support the improved metadata tracking and async task management.
Also applies to: 4-4, 11-11, 23-23
85-89: LGTM: Enhanced scope configuration with local task group.The explicit task group configuration and scope setup properly manages the session lifecycle with better control over concurrent operations.
93-96: LGTM: Explicit websocket configuration removes size limits.Setting
"max_size": Noneexplicitly removes message size limits, which is appropriate for real-time audio streaming scenarios.
123-123: LGTM: Improved metadata tracking with mapping structure.The change from
set[str]toMutableMapping[str, Meta]enables richer metadata tracking for conversation items, supporting the enhanced identifier and metadata management described in the PR objectives.
183-204: LGTM: Audio transcription event handling implementation.The new event handlers for
response.audio_transcript.deltaandresponse.audio_transcript.doneproperly support streaming audio transcription with appropriate metadata association and content wrapping usingMetaContent.
230-230: LGTM: Improved response.done event handling.Changing from sending an empty end-of-data chunk to continuing reading is more appropriate, as the actual completion should be signaled by specific item completion events.
254-268: LGTM: Enhanced message completion handling with proper validation.The updated logic properly validates item ID presence and sends appropriate end-of-data chunks for assistant messages, improving the completion signaling mechanism.
294-326: LGTM: Comprehensive conversation item creation handling.The enhanced event handling properly:
- Validates item ID presence
- Skips system messages and already tracked items
- Manages metadata for user and assistant items
- Handles transcription model workflow appropriately
328-366: LGTM: Robust conversation item retrieval and memory management.The updated retrieval logic properly:
- Validates item completeness and retries if needed
- Handles different content types with appropriate metadata
- Uses stored metadata for memory operations
- Converts items to appropriate LMM types
401-401: LGTM: Proper cleanup of current_items mapping.Resetting
current_itemsto an empty dictionary after context reset ensures clean state management.
480-497: LGTM: Enhanced context sending with metadata tracking.The updated context sending logic properly:
- Generates consistent identifiers using
.hexformat- Stores metadata in the current_items mapping
- Uses the input_content flag for proper content type differentiation
560-560: LGTM: Enhanced content processing with input/output differentiation.The
input_contentparameter and corresponding type field logic properly distinguishes between input and output content types, and the transcript handling for audio content is well implemented.Also applies to: 568-568, 576-591, 612-612
723-730: LGTM: Improved turn detection configuration with conditional wrapping.Using
without_missingfor the turn detection dictionary ensures clean configuration object creation with proper handling of missing values.
766-766: LGTM: Enhanced audio metadata handling with transcript support.The conditional transcript metadata inclusion properly supports the enhanced audio transcription functionality while maintaining backward compatibility.
KaQuMiQ
force-pushed
the
feature/realtime_memory
branch
from
e42f4ce to
d14c66e
Compare
KaQuMiQ
force-pushed
the
feature/realtime_memory
branch
from
d14c66e to
b81d0d2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/draive/conversation/realtime/default.py (1)
112-116: Past review comment issue remains unresolved.The
message_identifier=chunk.meta.get_uuid("message_identifier", default=uuid4())will always fall back to a fresh UUID because upstream code insrc/draive/openai/lmm_session.pynever sets the"message_identifier"metadata field. This breaks chunk-to-message grouping as identified in the previous review.The identifier extraction from
chunk.meta.identifieralso needs verification that the upstream session layer consistently provides this field.src/draive/conversation/types.py (1)
195-195: Fix typo in metadata key name.There's a typo in the metadata key:
indentifiershould beidentifierto maintain consistency with the retrieval logic infrom_lmm_contextmethod.Apply this diff to fix the typo:
- indentifier=str(self.identifier), + identifier=str(self.identifier),Also applies to: 206-206
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.gitignore(1 hunks)pyproject.toml(2 hunks)src/draive/conversation/realtime/default.py(2 hunks)src/draive/conversation/types.py(4 hunks)src/draive/lmm/types.py(1 hunks)src/draive/mistral/lmm_generation.py(2 hunks)src/draive/openai/lmm_session.py(15 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.py: Use absolute imports fromdraivepackage
Follow Ruff import ordering (standard library, third party, local)
Use Python 3.12+ type features (type unions with|, generic syntax)
Use base and abstract types likeSequenceorIterableinstead of concrete types
Use custom exceptions for specific errors
Use NumPy docstring convention for all functions, classes, and methods
Skip module-level docstrings unless explicitly requested
Include sections: Parameters, Returns, Raises, Notes (if needed) in docstrings
ALWAYS useSequence[T]instead oflist[T],Mapping[K,V]instead ofdict[K,V], andSet[T]instead ofset[T]for collections in State, Config and DataModel classes
Immutable updates to State, Config, and DataModel classes should be performed through the.updated()method
Files:
src/draive/lmm/types.pysrc/draive/mistral/lmm_generation.pysrc/draive/conversation/realtime/default.pysrc/draive/conversation/types.pysrc/draive/openai/lmm_session.py
🧠 Learnings (3)
pyproject.toml (2)
Learnt from: KaQuMiQ
PR: #338
File: src/draive/lmm/init.py:1-2
Timestamp: 2025-06-16T10:28:07.434Z
Learning: The draive project requires Python 3.12+ as specified in pyproject.toml with "requires-python = ">=3.12"" and uses Python 3.12+ specific features like PEP 695 type aliases and generic syntax extensively throughout the codebase.
Learnt from: CR
PR: miquido/draive#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T11:09:57.256Z
Learning: Applies to **/*.py : Use absolute imports from draive package
src/draive/lmm/types.py (1)
Learnt from: KaQuMiQ
PR: #338
File: src/draive/bedrock/lmm_generation.py:102-107
Timestamp: 2025-06-16T09:44:08.688Z
Learning: LMMInstruction is a type alias for str in the draive codebase, so LMMInstruction and str are equivalent types and can be used interchangeably.
src/draive/conversation/types.py (1)
Learnt from: KaQuMiQ
PR: #338
File: src/draive/conversation/types.py:123-144
Timestamp: 2025-06-16T10:09:51.508Z
Learning: In the draive.commons.metadata.Meta class, properties like identifier, origin_identifier, predecessor_identifier, and successor_identifier automatically parse stored hex strings into UUID objects when accessed, returning None if the string is missing or invalid. This means accessing meta.identifier returns a UUID object, not a string, even though it's stored internally as a hex string.
🧬 Code Graph Analysis (3)
src/draive/mistral/lmm_generation.py (3)
src/draive/lmm/types.py (1)
completion(225-250)src/draive/stages/stage.py (1)
completion(326-410)src/draive/lmm/state.py (3)
completion(30-39)completion(43-52)completion(55-82)
src/draive/conversation/realtime/default.py (2)
src/draive/lmm/types.py (12)
LMMSessionEvent(373-388)of(84-107)of(133-142)of(153-162)of(184-200)of(211-220)of(260-274)of(284-295)of(309-320)of(375-385)LMMToolRequest(258-279)LMMStreamChunk(307-327)src/draive/conversation/types.py (3)
ConversationEvent(230-255)of(232-249)ConversationMessageChunk(23-76)
src/draive/conversation/types.py (4)
src/draive/agents/node.py (1)
identifier(52-53)src/draive/tools/types.py (1)
meta(74-74)src/draive/multimodal/content.py (1)
meta(151-164)src/draive/lmm/types.py (1)
LMMCompletion(151-168)
🔇 Additional comments (22)
.gitignore (1)
84-84: LGTM! Appropriate editor exclusion.Adding
.zed/to gitignore is a sensible change to exclude Zed editor configuration and cache files from version control.pyproject.toml (2)
8-8: LGTM! Version bump aligns with PR changes.The version increment from 0.80.0 to 0.81.0 appropriately reflects the fixes and improvements in this PR.
24-24: LGTM! Dependency update is appropriate.The haiway dependency update from ~=0.27.2 to ~=0.27.4 likely includes relevant bug fixes or improvements for the realtime integration fixes.
src/draive/mistral/lmm_generation.py (4)
213-213: LGTM! Defensive programming improvement.Adding the
or 0fallback ensures robust handling whenusage.prompt_tokensis None or falsy, preventing potential issues in metric recording.
221-221: LGTM! Consistent defensive programming.The
or 0fallback forusage.completion_tokensmaintains consistency with the input tokens handling and ensures reliable metric recording.
335-335: LGTM! Stream consistency maintained.The defensive
or 0fallback is appropriately applied in the streaming context as well, ensuring consistent behavior between completion and streaming modes.
343-343: LGTM! Complete defensive coverage.The completion tokens fallback in streaming mode completes the defensive programming pattern, ensuring all token usage metrics are reliable.
src/draive/lmm/types.py (1)
179-179: LGTM! Minor reordering aligns with processing priorities.Reordering the literals to prioritize "result" before "error" may reflect updated event handling priorities, though functionally this doesn't change the type behavior.
src/draive/conversation/realtime/default.py (2)
128-128: LGTM! Improved debugging visibility.Adding the debug log for tool requests improves observability of the request handling flow.
138-156: Improved input handling with explicit type discrimination.The refactored write function now explicitly handles different input types (
ConversationMessageChunkvsConversationEvent) instead of ignoring events. This is a significant improvement that ensures all input types are properly processed and forwarded to the session layer.src/draive/conversation/types.py (2)
132-132: LGTM! Improved identifier preservation logic.The change to use
element.meta.identifier or uuid4()is a good improvement that preserves existing identifiers when available while providing a fallback. This aligns well with the metadata tracking enhancements in the OpenAI realtime integration.Also applies to: 140-140
232-240: No breaking changes: parameter reorder is safeAll existing
ConversationEvent.ofcalls use only the first (category) argument positionally and supplyidentifier,content, andmetaas keyword-only. Reordering those keyword-only parameters does not affect any call sites. Approving the change.src/draive/openai/lmm_session.py (10)
85-96: LGTM! Good improvements to connection handling.The addition of a local TaskGroup for concurrency management and explicit websocket configuration with no size limit are solid improvements that enhance reliability for streaming large content.
123-124: Excellent improvement to metadata tracking.Changing
current_itemsfrom a set to a mapping enables proper metadata tracking for each conversation item, which is essential for maintaining context throughout the conversation flow.
182-204: Well-implemented audio transcript streaming support.The new event handlers for audio transcripts correctly emit transcript chunks as MetaContent and maintain proper metadata tracking. This enables real-time transcription streaming, which is a valuable feature.
206-231: Correct handling of response completion event.The change to continue processing instead of sending an EOD chunk is appropriate. The
response.doneevent indicates server-side completion but doesn't necessarily mean content delivery is complete. EOD signaling is properly handled byresponse.output_item.done.
293-327: Excellent conversation item lifecycle management.The enhanced handling properly tracks items with metadata, respects transcription configuration, and avoids duplicates. The role-based logic and ID validation ensure robust conversation flow management.
328-366: Well-designed item retrieval with retry logic.The implementation correctly validates items before processing and retries incomplete items. The memory update logic properly stores user inputs and assistant completions with their associated metadata.
470-509: Robust context management with metadata tracking.The updated
_send_contextfunction properly tracks metadata for each context item and generates consistent identifiers. Using hex representation for IDs aligns with OpenAI's format requirements.
559-593: Proper content type handling with transcript support.The distinction between input and output content types is correctly implemented, and the conditional inclusion of transcripts in audio parts is well-designed. This aligns with OpenAI's API requirements.
723-732: Clean configuration handling.Wrapping the turn detection configuration with
without_missingensures proper conditional inclusion and prevents sending undefined values to the API.
751-768: Improved transcript metadata attachment.Attaching transcripts directly to the audio MediaData's metadata is cleaner than using separate MetaContent parts. This maintains the proper relationship between audio and its transcript.
KaQuMiQ
force-pushed
the
feature/realtime_memory
branch
3 times, most recently
from
43c0055 to
3d3cbfd
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
src/draive/conversation/realtime/default.py (2)
112-116: Action Required: Alignmessage_identifiermetadata between session and realtime layersThe new
message_identifier=chunk.meta.get_uuid("message_identifier", default=uuid4())will always fall back to a fresh UUID because no upstream code ever sets"message_identifier"in the metadata. This breaks chunk‐to‐message grouping.Please address the following:
- In src/draive/openai/lmm_session.py, when creating conversation items for messages and completions, include a
"message_identifier"metadata field (e.g. set it to the same value as"id"/identifier) so that the realtime reader can retrieve it instead of defaulting.- Fix the typo in src/draive/conversation/types.py where
"indentifier"is used—rename it to"identifier"in themeta.updated(...)calls to ensure metadata keys are correct.
121-125: Event handling simplified but may lose specificity.The previous detailed pattern matching for specific event categories (
"output.completed","input.started", etc.) has been replaced with generic handling. While this is more maintainable, ensure that downstream consumers don't rely on the specific event processing that was removed.src/draive/conversation/types.py (1)
195-196: Fix typo in metadata key name.There's a typo in the metadata key:
indentifiershould beidentifierto maintain consistency with the retrieval logic infrom_lmm_contextmethod.Apply this diff to fix the typo:
- indentifier=str(self.identifier), + identifier=str(self.identifier),Also applies to: 206-207
src/draive/openai/lmm_session.py (1)
340-340: Consider more defensive assertion handling.While the assertion is reasonable given the status check, consider handling this more gracefully in production code.
Consider replacing the assertion with explicit error handling:
- assert event.item.content # nosec: B101 + if not event.item.content: + ctx.log_warning( + f"Completed item {event.item.id} has no content, skipping memory update" + ) + continue
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
.gitignore(1 hunks)pyproject.toml(2 hunks)src/draive/anthropic/lmm.py(1 hunks)src/draive/bedrock/lmm.py(2 hunks)src/draive/conversation/realtime/default.py(2 hunks)src/draive/conversation/types.py(4 hunks)src/draive/gemini/lmm.py(2 hunks)src/draive/lmm/types.py(1 hunks)src/draive/mistral/lmm.py(2 hunks)src/draive/mistral/lmm_generation.py(2 hunks)src/draive/openai/lmm.py(2 hunks)src/draive/openai/lmm_session.py(15 hunks)src/draive/stages/stage.py(1 hunks)src/draive/vllm/lmm.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.py: Use absolute imports fromdraivepackage
Follow Ruff import ordering (standard library, third party, local)
Use Python 3.12+ type features (type unions with|, generic syntax)
Use base and abstract types likeSequenceorIterableinstead of concrete types
Use custom exceptions for specific errors
Use NumPy docstring convention for all functions, classes, and methods
Skip module-level docstrings unless explicitly requested
Include sections: Parameters, Returns, Raises, Notes (if needed) in docstrings
ALWAYS useSequence[T]instead oflist[T],Mapping[K,V]instead ofdict[K,V], andSet[T]instead ofset[T]for collections in State, Config and DataModel classes
Immutable updates to State, Config, and DataModel classes should be performed through the.updated()method
Files:
src/draive/lmm/types.pysrc/draive/mistral/lmm_generation.pysrc/draive/openai/lmm.pysrc/draive/vllm/lmm.pysrc/draive/mistral/lmm.pysrc/draive/conversation/realtime/default.pysrc/draive/conversation/types.pysrc/draive/gemini/lmm.pysrc/draive/anthropic/lmm.pysrc/draive/bedrock/lmm.pysrc/draive/stages/stage.pysrc/draive/openai/lmm_session.py
🧠 Learnings (3)
pyproject.toml (2)
Learnt from: KaQuMiQ
PR: #338
File: src/draive/lmm/init.py:1-2
Timestamp: 2025-06-16T10:28:07.434Z
Learning: The draive project requires Python 3.12+ as specified in pyproject.toml with "requires-python = ">=3.12"" and uses Python 3.12+ specific features like PEP 695 type aliases and generic syntax extensively throughout the codebase.
Learnt from: CR
PR: miquido/draive#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T11:09:57.256Z
Learning: Applies to **/*.py : Use absolute imports from draive package
src/draive/lmm/types.py (1)
Learnt from: KaQuMiQ
PR: #338
File: src/draive/bedrock/lmm_generation.py:102-107
Timestamp: 2025-06-16T09:44:08.688Z
Learning: LMMInstruction is a type alias for str in the draive codebase, so LMMInstruction and str are equivalent types and can be used interchangeably.
src/draive/conversation/types.py (2)
Learnt from: KaQuMiQ
PR: #338
File: src/draive/conversation/types.py:123-144
Timestamp: 2025-06-16T10:09:51.508Z
Learning: In the draive.commons.metadata.Meta class, properties like identifier, origin_identifier, predecessor_identifier, and successor_identifier automatically parse stored hex strings into UUID objects when accessed, returning None if the string is missing or invalid. This means accessing meta.identifier returns a UUID object, not a string, even though it's stored internally as a hex string.
Learnt from: KaQuMiQ
PR: #338
File: src/draive/conversation/completion/state.py:61-68
Timestamp: 2025-06-16T10:01:14.014Z
Learning: ConversationMessage is a DataModel with built-in runtime validation. When calling methods like ConversationMessage.user(multimodal), the DataModel validates types at runtime and raises appropriate exceptions for invalid inputs, eliminating the need for explicit type checking in pattern matching.
🧬 Code Graph Analysis (7)
src/draive/openai/lmm.py (4)
src/draive/multimodal/meta.py (1)
MetaContent(12-30)src/draive/multimodal/content.py (2)
meta(151-164)to_str(176-187)src/draive/multimodal/media.py (2)
to_str(70-71)to_str(102-111)src/draive/parameters/model.py (1)
to_str(529-530)
src/draive/vllm/lmm.py (4)
src/draive/multimodal/meta.py (1)
MetaContent(12-30)src/draive/multimodal/content.py (2)
meta(151-164)to_str(176-187)src/draive/multimodal/media.py (2)
to_str(70-71)to_str(102-111)src/draive/parameters/model.py (1)
to_str(529-530)
src/draive/mistral/lmm.py (4)
src/draive/multimodal/meta.py (1)
MetaContent(12-30)src/draive/multimodal/content.py (2)
meta(151-164)to_str(176-187)src/draive/multimodal/media.py (2)
to_str(70-71)to_str(102-111)src/draive/parameters/model.py (1)
to_str(529-530)
src/draive/conversation/realtime/default.py (2)
src/draive/lmm/types.py (12)
LMMSessionEvent(373-388)of(84-107)of(133-142)of(153-162)of(184-200)of(211-220)of(260-274)of(284-295)of(309-320)of(375-385)LMMToolRequest(258-279)LMMStreamChunk(307-327)src/draive/conversation/types.py (3)
ConversationEvent(230-255)of(232-249)ConversationMessageChunk(23-76)
src/draive/conversation/types.py (1)
src/draive/lmm/types.py (1)
LMMCompletion(151-168)
src/draive/anthropic/lmm.py (2)
src/draive/multimodal/meta.py (1)
MetaContent(12-30)src/draive/parameters/model.py (1)
to_str(529-530)
src/draive/bedrock/lmm.py (3)
src/draive/multimodal/meta.py (1)
MetaContent(12-30)src/draive/multimodal/content.py (3)
MultimodalContent(24-236)meta(151-164)to_str(176-187)src/draive/parameters/model.py (1)
to_str(529-530)
🔇 Additional comments (19)
.gitignore (1)
84-84: LGTM! Standard editor directory exclusion.Adding
.zed/to gitignore follows best practices for excluding editor-specific configuration and cache files from version control.pyproject.toml (2)
8-8: LGTM! Version increment for new release.The version bump to 0.81.0 appropriately reflects the transcript and memory handling improvements in this PR.
24-24: LGTM! Dependency update.The haiway dependency update from ~=0.27.2 to ~=0.27.5 likely includes bug fixes or improvements that support the transcript handling enhancements.
src/draive/gemini/lmm.py (2)
22-22: LGTM! Import alignment with coding guidelines.The import change from
draive.multimodal.metatodraive.multimodalfollows the coding guidelines for absolute imports from the draive package.
152-155: LGTM! Transcript metadata support added.The new pattern matching case correctly handles
MetaContentwith category "transcript" by converting it to text format usingto_str(). The positioning before theDataModelcase ensures proper precedence.src/draive/anthropic/lmm.py (1)
226-230: LGTM! Transcript metadata support for Anthropic.The new pattern matching case correctly handles transcript
MetaContentby converting it to Anthropic's text block format. The implementation is consistent with the established patterns and properly positioned before theDataModelcase.src/draive/vllm/lmm.py (2)
29-29: LGTM! Required import addition.The
MetaContentimport is correctly added to support the new transcript handling functionality.
83-87: LGTM! Transcript metadata support for VLLM.The new pattern matching case correctly handles transcript
MetaContentby converting it to OpenAI-compatible text format. The implementation is consistent with other LMM backends and properly positioned before theDataModelcase.src/draive/mistral/lmm.py (2)
32-32: LGTM! Import update aligns with module structure.The import change from
draive.multimodal.metatodraive.multimodalis correct and follows the expected module organization.
139-143: Good implementation of transcript metadata handling.The new pattern match case correctly handles
MetaContentinstances with category "transcript", converting them to text content chunks. This implementation is consistent with similar changes across other LMM backends and properly checks for both the category match and content presence before processing.src/draive/lmm/types.py (1)
179-179: LGTM! Literal reordering has no functional impact.The reordering of literals in the
LMMToolResponseHandlingtype alias doesn't affect functionality sinceLiteraltypes are unordered unions. This change may reflect prioritization of "result" over "error" handling, which aligns with the PR's focus on improved response handling.src/draive/mistral/lmm_generation.py (2)
213-213: Good defensive programming for token usage metrics.Adding
or 0fallback ensures thatNoneor falsy token counts are recorded as zero instead of potentially causing metric recording issues. This improves robustness of the observability system.Also applies to: 221-221
335-335: Consistent token usage handling in streaming path.The same defensive fallback logic is correctly applied in the streaming completion path, ensuring consistent metric recording behavior across both streaming and non-streaming code paths.
Also applies to: 343-343
src/draive/openai/lmm.py (2)
29-29: LGTM! Import addition supports transcript handling.Adding
MetaContentto the imports correctly supports the new transcript metadata handling functionality.
82-86: Consistent transcript metadata implementation.The new pattern match case correctly handles
MetaContentinstances with category "transcript", following the same pattern as other LMM backends. The implementation properly validates both the category and content presence before processing.src/draive/bedrock/lmm.py (2)
17-18: LGTM! Import updates support transcript functionality.Adding
MetaContentandMultimodalContentimports correctly supports the new transcript metadata handling. The import organization follows the established pattern.
66-69: Consistent transcript handling across backends.The implementation mirrors the pattern established in other LMM backends (Mistral, OpenAI), providing consistent transcript metadata handling. The logic correctly validates category and content presence before converting to text format.
src/draive/conversation/realtime/default.py (1)
138-157: Improved input handling in write coroutine.The explicit handling for both
ConversationMessageChunkandConversationEventinputs is a good improvement. The code properly converts these to their LMM equivalents (LMMStreamChunkandLMMSessionEvent) before passing to the session.src/draive/openai/lmm_session.py (1)
559-619: Good improvement for content type distinction.The addition of the
input_contentparameter properly distinguishes between input and output content types, ensuring the correcttypefield values are used for the OpenAI realtime API.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
src/draive/conversation/realtime/default.py (2)
112-116: Action Required: Alignmessage_identifiermetadata between session and realtime layersThe new
message_identifier=chunk.meta.get_uuid("message_identifier", default=uuid4())will always fall back to a fresh UUID because no upstream code sets"message_identifier"in the metadata. This breaks chunk-to-message grouping.Please address:
- In src/draive/openai/lmm_session.py, when creating conversation items, include a
"message_identifier"metadata field- Fix the typo in src/draive/conversation/types.py where
"indentifier"is used instead of"identifier"
121-125: Event handling simplified but may lose specificity.The previous detailed pattern matching for specific event categories has been replaced with generic handling. While this is more maintainable, ensure that downstream consumers don't rely on the specific event processing that was removed.
src/draive/conversation/types.py (1)
195-195: Fix typo in metadata key name.There's a typo in the metadata key:
indentifiershould beidentifierto maintain consistency with the retrieval logic infrom_lmm_contextmethod.- indentifier=str(self.identifier), + identifier=str(self.identifier),Also applies to: 206-206
src/draive/openai/lmm_session.py (1)
340-340: Consider more defensive assertion handling.While the assertion is reasonable given the status check, consider handling this more gracefully in production code.
- assert event.item.content # nosec: B101 + if not event.item.content: + ctx.log_warning( + f"Completed item {event.item.id} has no content, skipping memory update" + ) + continue
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
.gitignore(1 hunks)pyproject.toml(2 hunks)src/draive/anthropic/lmm.py(1 hunks)src/draive/bedrock/lmm.py(2 hunks)src/draive/conversation/realtime/default.py(2 hunks)src/draive/conversation/types.py(4 hunks)src/draive/gemini/lmm.py(2 hunks)src/draive/lmm/types.py(1 hunks)src/draive/mistral/lmm.py(2 hunks)src/draive/mistral/lmm_generation.py(2 hunks)src/draive/openai/lmm.py(2 hunks)src/draive/openai/lmm_session.py(15 hunks)src/draive/stages/stage.py(1 hunks)src/draive/vllm/lmm.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.py: Use absolute imports fromdraivepackage
Follow Ruff import ordering (standard library, third party, local)
Use Python 3.12+ type features (type unions with|, generic syntax)
Use base and abstract types likeSequenceorIterableinstead of concrete types
Use custom exceptions for specific errors
Use NumPy docstring convention for all functions, classes, and methods
Skip module-level docstrings unless explicitly requested
Include sections: Parameters, Returns, Raises, Notes (if needed) in docstrings
ALWAYS useSequence[T]instead oflist[T],Mapping[K,V]instead ofdict[K,V], andSet[T]instead ofset[T]for collections in State, Config and DataModel classes
Immutable updates to State, Config, and DataModel classes should be performed through the.updated()method
Files:
src/draive/mistral/lmm.pysrc/draive/anthropic/lmm.pysrc/draive/gemini/lmm.pysrc/draive/vllm/lmm.pysrc/draive/openai/lmm.pysrc/draive/bedrock/lmm.pysrc/draive/mistral/lmm_generation.pysrc/draive/lmm/types.pysrc/draive/conversation/realtime/default.pysrc/draive/stages/stage.pysrc/draive/conversation/types.pysrc/draive/openai/lmm_session.py
🧠 Learnings (3)
pyproject.toml (2)
Learnt from: KaQuMiQ
PR: #338
File: src/draive/lmm/init.py:1-2
Timestamp: 2025-06-16T10:28:07.434Z
Learning: The draive project requires Python 3.12+ as specified in pyproject.toml with "requires-python = ">=3.12"" and uses Python 3.12+ specific features like PEP 695 type aliases and generic syntax extensively throughout the codebase.
Learnt from: CR
PR: miquido/draive#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T11:09:57.256Z
Learning: Applies to **/*.py : Use absolute imports from draive package
src/draive/lmm/types.py (1)
Learnt from: KaQuMiQ
PR: #338
File: src/draive/bedrock/lmm_generation.py:102-107
Timestamp: 2025-06-16T09:44:08.688Z
Learning: LMMInstruction is a type alias for str in the draive codebase, so LMMInstruction and str are equivalent types and can be used interchangeably.
src/draive/conversation/types.py (1)
Learnt from: KaQuMiQ
PR: #338
File: src/draive/conversation/types.py:123-144
Timestamp: 2025-06-16T10:09:51.508Z
Learning: In the draive.commons.metadata.Meta class, properties like identifier, origin_identifier, predecessor_identifier, and successor_identifier automatically parse stored hex strings into UUID objects when accessed, returning None if the string is missing or invalid. This means accessing meta.identifier returns a UUID object, not a string, even though it's stored internally as a hex string.
🧬 Code Graph Analysis (8)
src/draive/mistral/lmm.py (4)
src/draive/multimodal/meta.py (1)
MetaContent(12-30)src/draive/multimodal/content.py (2)
meta(151-164)to_str(176-187)src/draive/multimodal/media.py (2)
to_str(70-71)to_str(102-111)src/draive/parameters/model.py (1)
to_str(529-530)
src/draive/anthropic/lmm.py (3)
src/draive/multimodal/meta.py (1)
MetaContent(12-30)src/draive/multimodal/content.py (2)
meta(151-164)to_str(176-187)src/draive/parameters/model.py (1)
to_str(529-530)
src/draive/gemini/lmm.py (3)
src/draive/multimodal/meta.py (1)
MetaContent(12-30)src/draive/multimodal/content.py (2)
meta(151-164)to_str(176-187)src/draive/parameters/model.py (1)
to_str(529-530)
src/draive/vllm/lmm.py (4)
src/draive/multimodal/meta.py (1)
MetaContent(12-30)src/draive/multimodal/content.py (2)
meta(151-164)to_str(176-187)src/draive/multimodal/media.py (2)
to_str(70-71)to_str(102-111)src/draive/parameters/model.py (1)
to_str(529-530)
src/draive/openai/lmm.py (4)
src/draive/multimodal/meta.py (1)
MetaContent(12-30)src/draive/multimodal/content.py (2)
meta(151-164)to_str(176-187)src/draive/multimodal/media.py (2)
to_str(70-71)to_str(102-111)src/draive/parameters/model.py (1)
to_str(529-530)
src/draive/bedrock/lmm.py (3)
src/draive/multimodal/meta.py (1)
MetaContent(12-30)src/draive/multimodal/content.py (3)
MultimodalContent(24-236)meta(151-164)to_str(176-187)src/draive/parameters/model.py (1)
to_str(529-530)
src/draive/conversation/realtime/default.py (2)
src/draive/lmm/types.py (12)
LMMSessionEvent(373-388)of(84-107)of(133-142)of(153-162)of(184-200)of(211-220)of(260-274)of(284-295)of(309-320)of(375-385)LMMToolRequest(258-279)LMMStreamChunk(307-327)src/draive/conversation/types.py (3)
ConversationEvent(230-255)of(232-249)ConversationMessageChunk(23-76)
src/draive/conversation/types.py (4)
src/draive/agents/node.py (1)
identifier(52-53)src/draive/tools/types.py (1)
meta(74-74)src/draive/multimodal/content.py (1)
meta(151-164)src/draive/lmm/types.py (1)
LMMCompletion(151-168)
🔇 Additional comments (19)
.gitignore (1)
84-84: LGTM: Standard editor exclusion added.The addition of
.zed/follows the established pattern for excluding editor-specific directories from version control.pyproject.toml (1)
8-8: LGTM: Version increment and dependency update.The version bump to 0.81.0 and haiway dependency update to ~=0.27.5 are appropriate for this release containing transcript and memory handling improvements.
Also applies to: 24-24
src/draive/mistral/lmm_generation.py (2)
213-213: LGTM: Defensive programming for token usage metrics.Adding
or 0fallbacks ensures that token usage metrics always record numeric values instead of potentially None values, which could cause issues in observability systems.Also applies to: 221-221
335-335: LGTM: Consistent defensive programming in streaming method.The same
or 0fallback pattern is correctly applied in the streaming method for consistency with the non-streaming implementation.Also applies to: 343-343
src/draive/mistral/lmm.py (2)
32-32: LGTM: Import follows coding guidelines.The import change to use absolute imports from the
draivepackage follows the established coding guidelines.
139-143: LGTM: Transcript metadata support added.The new pattern matching case correctly handles
MetaContentwith category "transcript" by converting it to text content usingmeta.content.to_str(). This aligns with the PR objective to improve transcript handling.src/draive/lmm/types.py (1)
179-179: LGTM: Reordered literals for better organization.The reordering of
LMMToolResponseHandlingliterals (moving "result" before "error") improves logical organization by prioritizing positive outcomes. This doesn't affect functionality since Literal type order is not significant for type checking.src/draive/gemini/lmm.py (2)
22-22: Import refactoring looks good.The import of
MetaContentfromdraive.multimodalfollows the coding guidelines for absolute imports from thedraivepackage.
152-155: Transcript handling implementation is correct.The pattern match case properly handles
MetaContentwith category "transcript" by:
- Validating both category and content existence
- Converting content to string representation via
to_str()- Returning the appropriate dictionary structure for Gemini API
The placement before the
DataModelcase ensures transcript metadata is handled specifically rather than falling through to generic data handling.src/draive/anthropic/lmm.py (1)
226-230: Transcript handling implementation is correct for Anthropic API.The pattern match case properly handles transcript
MetaContentby:
- Validating category equals "transcript" and content exists
- Converting content to string using
to_str()- Returning the correct format for Anthropic API with
"type": "text"The implementation is consistent with the other LMM backends while respecting Anthropic's specific message format.
src/draive/vllm/lmm.py (2)
29-29: Import addition is correct.The
MetaContentimport follows the established pattern and coding guidelines for absolute imports from thedraivepackage.
83-87: Transcript handling implementation is correct for VLLM.The pattern match case properly handles transcript
MetaContentwith:
- Correct validation of category and content existence
- Appropriate string conversion via
to_str()- OpenAI-compatible format with
"type": "text"for VLLM clientThe implementation maintains consistency with other LMM backends while using the appropriate API format.
src/draive/openai/lmm.py (2)
29-29: Import addition follows coding guidelines.The
MetaContentimport is correctly added using absolute imports from thedraivepackage as specified in the coding guidelines.
82-86: Transcript handling implementation is correct for OpenAI API.The pattern match case properly handles transcript
MetaContentby:
- Validating both category ("transcript") and content presence
- Converting content to string representation using
to_str()- Returning the correct OpenAI chat completion format with
"type": "text"The implementation is consistent with other LMM backends and follows the established pattern.
src/draive/bedrock/lmm.py (2)
17-18: Import reorganization is correct.The imports have been properly organized by adding
MetaContentandMultimodalContentfromdraive.multimodal, following the coding guidelines for absolute imports from thedraivepackage.
66-69: Transcript handling implementation is correct for Bedrock API.The pattern match case properly handles transcript
MetaContentwith:
- Appropriate validation of category and content existence
- String conversion using
to_str()method- Bedrock-compatible format returning
{"text": ...}The implementation maintains consistency with other LMM backends while using the correct format for AWS Bedrock's API.
src/draive/conversation/realtime/default.py (1)
138-156: Well-structured input handling implementation!The explicit handling of both
ConversationMessageChunkandConversationEventinputs is a significant improvement. The conversion to corresponding LMM types preserves all metadata and properly routes the events to the session.src/draive/openai/lmm_session.py (2)
183-204: Well-implemented audio transcript streaming support!The handling of transcript delta and completion events properly streams transcript data as
MetaContentwith category "transcript", which aligns with the transcript handling pattern used across LMM modules.
559-619: Proper input/output content type distinction!The addition of the
input_contentparameter and corresponding type field changes correctly distinguish between input and output content types as required by OpenAI's realtime API. The transcript handling also properly converts MetaContent to text parts.
KaQuMiQ
force-pushed
the
feature/realtime_memory
branch
2 times, most recently
from
f2a9252 to
aacb370
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
src/draive/conversation/realtime/default.py (2)
112-116: LGTM! Improved identifier extraction with proper fallback.The change to use
chunk.meta.identifierdirectly andget_uuid("message_identifier", default=uuid4())provides better fallback handling and aligns with the updated metadata structure.
121-125: Event handling appropriately simplified.The generic event handling approach is cleaner and more maintainable than the previous detailed pattern matching. The logging provides adequate debugging information while the
ConversationEvent.of()factory method preserves all necessary metadata.src/draive/stages/stage.py (1)
582-647: Fix typo in docstring.There's a typo in the docstring at line 595: "nex input" should be "new input".
The implementation is correct and follows the established pattern of other completion methods.
Apply this diff to fix the typo:
- This Stage takes the current stage result, appends it as a nex input, and executes + This Stage takes the current stage result, appends it as a new input, and executessrc/draive/conversation/types.py (1)
195-195: Fix typo in metadata key name.There's a typo in the metadata key:
indentifiershould beidentifierto maintain consistency with the retrieval logic infrom_lmm_contextmethod.Apply this diff to fix the typo:
- indentifier=str(self.identifier), + identifier=str(self.identifier),Also applies to: 206-206
src/draive/openai/lmm_session.py (3)
184-184: Fix typo in comment.- # send the text chuink + # send the text chunk
304-309: Addmessage_identifierto conversation item metadata.The metadata is missing the
message_identifierfield that the conversation realtime layer expects for chunk-to-message grouping.Apply this diff to add the missing field:
current_items[event.item.id] = Meta.of( { # using predefined meta keys "identifier": str(uuid4()), + "message_identifier": str(uuid4()), "created": datetime.now().isoformat(), } )Also applies to: 315-320
340-340: Replace assertion with defensive error handling.Consider replacing the assertion with explicit error handling:
- assert event.item.content # nosec: B101 + if not event.item.content: + ctx.log_warning( + f"Completed item {event.item.id} has no content, skipping memory update" + ) + continue
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
.gitignore(1 hunks)pyproject.toml(2 hunks)src/draive/anthropic/lmm.py(6 hunks)src/draive/anthropic/lmm_generation.py(2 hunks)src/draive/bedrock/lmm.py(5 hunks)src/draive/conversation/realtime/default.py(2 hunks)src/draive/conversation/types.py(4 hunks)src/draive/gemini/lmm.py(3 hunks)src/draive/gemini/lmm_generation.py(1 hunks)src/draive/lmm/types.py(1 hunks)src/draive/mistral/lmm.py(4 hunks)src/draive/mistral/lmm_generation.py(4 hunks)src/draive/openai/lmm.py(4 hunks)src/draive/openai/lmm_session.py(15 hunks)src/draive/stages/stage.py(1 hunks)src/draive/vllm/lmm.py(3 hunks)src/draive/vllm/lmm_generation.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.py: Use absolute imports fromdraivepackage
Follow Ruff import ordering (standard library, third party, local)
Use Python 3.12+ type features (type unions with|, generic syntax)
Use base and abstract types likeSequenceorIterableinstead of concrete types
Use custom exceptions for specific errors
Use NumPy docstring convention for all functions, classes, and methods
Skip module-level docstrings unless explicitly requested
Include sections: Parameters, Returns, Raises, Notes (if needed) in docstrings
ALWAYS useSequence[T]instead oflist[T],Mapping[K,V]instead ofdict[K,V], andSet[T]instead ofset[T]for collections in State, Config and DataModel classes
Immutable updates to State, Config, and DataModel classes should be performed through the.updated()method
Files:
src/draive/lmm/types.pysrc/draive/anthropic/lmm_generation.pysrc/draive/vllm/lmm_generation.pysrc/draive/mistral/lmm_generation.pysrc/draive/gemini/lmm_generation.pysrc/draive/bedrock/lmm.pysrc/draive/conversation/realtime/default.pysrc/draive/anthropic/lmm.pysrc/draive/vllm/lmm.pysrc/draive/mistral/lmm.pysrc/draive/gemini/lmm.pysrc/draive/stages/stage.pysrc/draive/conversation/types.pysrc/draive/openai/lmm.pysrc/draive/openai/lmm_session.py
🧠 Learnings (6)
pyproject.toml (2)
Learnt from: KaQuMiQ
PR: #338
File: src/draive/lmm/init.py:1-2
Timestamp: 2025-06-16T10:28:07.434Z
Learning: The draive project requires Python 3.12+ as specified in pyproject.toml with "requires-python = ">=3.12"" and uses Python 3.12+ specific features like PEP 695 type aliases and generic syntax extensively throughout the codebase.
Learnt from: CR
PR: miquido/draive#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T11:09:57.256Z
Learning: Applies to **/*.py : Use absolute imports from draive package
src/draive/lmm/types.py (1)
Learnt from: KaQuMiQ
PR: #338
File: src/draive/bedrock/lmm_generation.py:102-107
Timestamp: 2025-06-16T09:44:08.688Z
Learning: LMMInstruction is a type alias for str in the draive codebase, so LMMInstruction and str are equivalent types and can be used interchangeably.
src/draive/bedrock/lmm.py (1)
Learnt from: CR
PR: miquido/draive#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T11:09:57.256Z
Learning: Applies to **/*.py : Use base and abstract types like Sequence or Iterable instead of concrete types
src/draive/anthropic/lmm.py (1)
Learnt from: CR
PR: miquido/draive#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T11:09:57.256Z
Learning: Applies to **/*.py : Use base and abstract types like Sequence or Iterable instead of concrete types
src/draive/gemini/lmm.py (1)
Learnt from: CR
PR: miquido/draive#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T11:09:57.256Z
Learning: Applies to **/*.py : Use base and abstract types like Sequence or Iterable instead of concrete types
src/draive/conversation/types.py (2)
Learnt from: KaQuMiQ
PR: #338
File: src/draive/conversation/completion/state.py:61-68
Timestamp: 2025-06-16T10:01:14.014Z
Learning: ConversationMessage is a DataModel with built-in runtime validation. When calling methods like ConversationMessage.user(multimodal), the DataModel validates types at runtime and raises appropriate exceptions for invalid inputs, eliminating the need for explicit type checking in pattern matching.
Learnt from: KaQuMiQ
PR: #338
File: src/draive/conversation/types.py:123-144
Timestamp: 2025-06-16T10:09:51.508Z
Learning: In the draive.commons.metadata.Meta class, properties like identifier, origin_identifier, predecessor_identifier, and successor_identifier automatically parse stored hex strings into UUID objects when accessed, returning None if the string is missing or invalid. This means accessing meta.identifier returns a UUID object, not a string, even though it's stored internally as a hex string.
🧬 Code Graph Analysis (5)
src/draive/anthropic/lmm_generation.py (1)
src/draive/anthropic/lmm.py (1)
content_elements(141-203)
src/draive/gemini/lmm_generation.py (1)
src/draive/gemini/lmm.py (1)
context_element_as_content(33-83)
src/draive/conversation/realtime/default.py (2)
src/draive/lmm/types.py (12)
LMMSessionEvent(373-388)of(84-107)of(133-142)of(153-162)of(184-200)of(211-220)of(260-274)of(284-295)of(309-320)of(375-385)LMMToolRequest(258-279)LMMStreamChunk(307-327)src/draive/conversation/types.py (3)
ConversationEvent(230-255)of(232-249)ConversationMessageChunk(23-76)
src/draive/vllm/lmm.py (7)
src/draive/multimodal/meta.py (1)
MetaContent(12-30)src/draive/multimodal/content.py (3)
MultimodalContent(24-236)text(171-174)to_str(176-187)src/draive/multimodal/text.py (1)
TextContent(10-27)src/draive/openai/lmm.py (4)
content_parts(45-51)content_parts(55-61)content_parts(64-133)context_element_as_messages(136-202)src/draive/multimodal/media.py (7)
MediaData(77-128)kind(67-68)kind(99-100)to_str(70-71)to_str(102-111)to_data_uri(113-125)MediaReference(46-74)src/draive/lmm/types.py (3)
LMMInput(131-148)LMMCompletion(151-168)completion(225-250)src/draive/stages/stage.py (1)
completion(326-410)
src/draive/conversation/types.py (1)
src/draive/lmm/types.py (1)
LMMCompletion(151-168)
🔇 Additional comments (42)
.gitignore (1)
84-84: Ignoring.zed/directory is appropriate and consistent with existing IDE excludes
No further action needed.pyproject.toml (1)
8-8: LGTM: Version and dependency updates look good.The version increment to 0.81.0 and haiway dependency update to ~=0.27.5 align with the coordinated refactoring changes across the codebase.
Also applies to: 24-24
src/draive/lmm/types.py (1)
179-179: LGTM: Reordering of literal values in type alias.The reordering of
LMMToolResponseHandlingliterals to prioritize "result" before "error" appears intentional and doesn't affect functionality.src/draive/anthropic/lmm_generation.py (2)
20-20: LGTM: Updated import for refactored content conversion.The import change from
convert_content_elementtocontent_elementsaligns with the generator-based content conversion refactoring.
125-125: LGTM: Updated to use generator-based content conversion.The change from a list comprehension using
convert_content_elementto directly callingcontent_elementsis consistent with the refactoring to generator-based multi-element content processing. This improves transcript handling as mentioned in the PR objectives.src/draive/vllm/lmm_generation.py (1)
115-115: LGTM: Updated parameter passing to be more specific.The change from passing the full
configobject to onlyvision_details=completion_config.vision_detailsimproves the interface by being more explicit about dependencies and reducing coupling.src/draive/mistral/lmm_generation.py (4)
43-43: LGTM: Updated import for refactored content conversion.The import change from
content_element_as_content_chunktocontent_chunksaligns with the generator-based content conversion refactoring across LMM modules.
127-127: LGTM: Updated to use generator-based content conversion.The change to use
list(content_chunks(...))instead of a list comprehension is consistent with the refactoring to generator-based multi-element content processing.
210-210: LGTM: Added defensive fallbacks for token usage metrics.The
or 0fallbacks ensure that token usage metrics are recorded as zero when values are missing or falsy, preventing potential issues with None values.Also applies to: 218-218
332-332: LGTM: Added defensive fallbacks for streaming token usage metrics.The
or 0fallbacks in the streaming path ensure consistent metric recording even when token counts are missing or falsy.Also applies to: 340-340
src/draive/gemini/lmm_generation.py (1)
117-119: LGTM! Fixed incorrect flattening of content dictionaries.The change correctly removes the unnecessary
chain.from_iterableoperation. Sincecontext_element_as_contentreturns a singleContentDict(not an iterable of content elements), the previous flattening was incorrect and could cause issues. The new direct list comprehension is the correct approach.src/draive/conversation/realtime/default.py (2)
128-128: Good addition of debug logging for tool requests.The debug log provides useful information for tracking tool request processing flow.
138-156: Improved input handling with explicit type processing.The change from ignoring
ConversationEventinputs to explicitly handling bothConversationMessageChunkandConversationEventtypes improves functionality. The conversion logic correctly maps conversation types to their LMM equivalents while preserving metadata and content.src/draive/mistral/lmm.py (4)
2-2: LGTM: Import additions support the refactoring.The addition of
GeneratorandSequenceimports properly supports the new generator-based content conversion function.
32-32: LGTM: Import follows project conventions.The import change from
draive.multimodal.metatodraive.multimodalfollows the project's absolute import guidelines.
100-146: LGTM: Well-implemented refactoring to generator-based content processing.The refactoring from single-element to sequence-based processing improves flexibility and aligns with similar changes across other LMM modules. The implementation correctly:
- Handles all content element types appropriately
- Adds proper MetaContent transcript support (aligning with PR objectives)
- Maintains error handling for unsupported media types
- Uses proper type annotations
58-58: LGTM: Function call updates are consistent and correct.All usages of the refactored function are properly updated to collect the generator results using
list(), maintaining consistent behavior across the module.Also applies to: 66-66, 94-94
src/draive/bedrock/lmm.py (4)
1-1: LGTM: Import additions support the refactoring.The addition of
GeneratorandSequenceimports properly supports the new generator-based content conversion approach.
17-19: LGTM: Import additions follow coding guidelines.The import additions for
MetaContentandMultimodalContentsupport the enhanced content handling and follow the project's absolute import guidelines.
30-76: LGTM: Consistent refactoring with proper error handling.The refactoring to generator-based sequence processing is well-implemented and consistent with other LMM modules. The implementation correctly:
- Handles all supported content element types
- Properly rejects unsupported MediaReference (appropriate for Bedrock)
- Adds MetaContent transcript support aligning with PR objectives
- Maintains clear error messages for unsupported content
85-85: LGTM: Function call updates are correct and consistent.All usages properly updated to collect generator results using
list(), maintaining consistent behavior while leveraging the new sequence-based processing.Also applies to: 91-91, 116-116
src/draive/conversation/types.py (3)
132-134: LGTM: Improved identifier handling and metadata key naming.The changes improve the robustness of identifier assignment with the fallback pattern and use more descriptive metadata key naming with "created" instead of "creation".
Also applies to: 140-142
196-197: LGTM: Consistent metadata key naming and explicit string conversion.The change from "creation" to "created" maintains consistency with other changes, and using
str()for identifier conversion is more explicit than.hex.Also applies to: 207-208
237-237: LGTM: Parameter order adjustment.The parameter order change moving
contentafteridentifiermay improve consistency with other method signatures in the codebase.src/draive/anthropic/lmm.py (3)
2-2: Imports follow best practices.The addition of
GeneratorandSequencefromcollections.abcfollows the coding guidelines for using abstract types and maintains proper import ordering.Also applies to: 42-48
56-58: Consistent generator usage pattern.The conversion from generator to list using
list(content_elements(...))is appropriate for creating message content parameters. The explicit type annotations improve code clarity.Also applies to: 74-77, 96-96, 131-134
141-204: Well-structured content generator implementation.The
content_elementsfunction efficiently processes multimodal content using a generator pattern with clear type handling:
- Proper use of
Sequencetype annotation following coding guidelines- Clear handling of each content type with appropriate transformations
- Special handling for MetaContent categories (thinking, redacted_thinking, transcript)
- Appropriate error handling for unsupported media types
src/draive/gemini/lmm.py (4)
1-1: Import changes align with refactoring pattern.The addition of
GeneratorandSequenceimports supports the new generator-based approach, maintaining proper import ordering.Also applies to: 22-22
36-84: Clean refactoring to if-elif structure.The change from pattern matching to explicit type checks improves readability while maintaining the same logic. The return type correctly matches the signature.
86-131: Generator implementation follows consistent pattern.The
content_partsfunction correctly:
- Uses
Sequencetype annotation per guidelines- Handles Gemini-specific "thought" content with boolean flag
- Processes transcript metadata as plain text
- Maintains consistency with other LMM implementations
133-167: Efficient generator conversion.The refactoring from list building to generator yielding improves memory efficiency while preserving the same logic flow.
src/draive/vllm/lmm.py (5)
2-5: Imports properly support new functionality.The added imports for
Generator,Sequence,overload, and type annotations follow proper ordering and enable the new overloaded generator pattern.Also applies to: 9-9, 23-23, 30-30
38-38: Type-safe overloads enhance API clarity.The overloaded signatures clearly distinguish between text-only and multimodal content generation, providing better type safety for callers.
Also applies to: 44-62
64-134: Robust content generation with text-only support.The implementation effectively handles the dual-mode operation:
- Clear separation of text-only vs multimodal output
- Proper error handling for unsupported media types
- Consistent transcript metadata processing
- Appropriate use of vision_details parameter
136-203: Context messages correctly use content generation modes.The function appropriately uses:
text_only=Falsefor user input (supports images)text_only=Truefor assistant and tool responses (text-only output)- Consistent vision_details parameter passing
224-229: Good cleanup of unreachable code.Removing the redundant return statement after the "json" case improves code clarity.
src/draive/openai/lmm_session.py (6)
2-4: Improved concurrency and connection handling.Good improvements:
- Local
TaskGroupfor better concurrency scope management- Explicit
max_size: Nonefor websocket to handle large messagesMutableMappingtype accurately represents the new metadata tracking structureAlso applies to: 11-11, 23-23, 85-96
206-230: Cleaner response completion handling.Good change to remove the empty EOD chunk from
response.doneevent, avoiding duplicate completion signals sinceresponse.output_item.donehandles the actual completion.
462-465: Helper functions properly support input/output distinction.The updates correctly:
- Distinguish between input and output content types
- Handle transcript metadata as text content
- Maintain consistent error logging for unsupported content
Also applies to: 569-570, 579-580, 596-610
401-401: Proper state management in context reset.The context reset correctly:
- Iterates over mapping keys for deletion
- Handles deletion errors gracefully
- Resets
current_itemsto empty dict for clean stateAlso applies to: 537-544
724-731: Clean conditional configuration handling.Using
without_missingto wrap the turn_detection config is a clean approach for conditional inclusion.
770-777: Consistent transcript metadata handling.The change to wrap transcript content in
MetaContentwith "transcript" category aligns with the unified transcript handling approach across the codebase.
KaQuMiQ
force-pushed
the
feature/realtime_memory
branch
from
aacb370 to
b185d9c
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (7)
src/draive/conversation/realtime/default.py (2)
112-116: Action Required: Address metadata field consistency issue.As noted in previous reviews, the
message_identifier=chunk.meta.get_uuid("message_identifier", default=uuid4())will always fall back to a fresh UUID because upstream code never sets"message_identifier"in the metadata, breaking chunk-to-message grouping functionality.
121-125: Event handling simplified but may lose specificity.The generic event handling approach improves maintainability but may lose specific event processing that downstream consumers could depend on. As noted in previous reviews, verify that no downstream code relies on the removed specific event handling logic.
src/draive/stages/stage.py (1)
595-595: Fix typo in docstring.- This Stage takes the current stage result, appends it as a new input, and executes + This Stage takes the current stage result, appends it as a new input, and executessrc/draive/conversation/types.py (1)
195-195: Fix typo in metadata key name.There's a typo in the metadata key:
message_indentifiershould bemessage_identifierto maintain consistency with the retrieval logic infrom_lmm_contextmethod.Also applies to: 206-206
src/draive/openai/lmm.py (1)
44-134: Implementation consistent with VLLM module.The
content_partsimplementation follows the same pattern as the VLLM module, which is good for consistency. The provider-specific error messages are appropriate.Consider extracting the common content processing logic into a shared utility module to reduce code duplication between OpenAI and VLLM implementations, as they are nearly identical except for error messages.
src/draive/openai/lmm_session.py (2)
312-328: Addmessage_identifierto conversation item metadata.The metadata tracking has been improved, but it's missing the
message_identifierfield that the conversation realtime layer expects (see line 113-115 insrc/draive/conversation/realtime/default.py).Consider adding
message_identifierto the metadata:current_items[event.item.id] = Meta.of( { # using predefined meta keys "identifier": event.item.id, + "message_identifier": event.item.id, "created": datetime.now().isoformat(), } )
348-348: Consider more defensive assertion handling.While the assertion is reasonable given the status check, consider handling this more gracefully in production code.
Consider replacing the assertion with explicit error handling:
- assert event.item.content # nosec: B101 + if not event.item.content: + ctx.log_warning( + f"Completed item {event.item.id} has no content, skipping memory update" + ) + continue
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
.gitignore(1 hunks)pyproject.toml(2 hunks)src/draive/anthropic/lmm.py(6 hunks)src/draive/anthropic/lmm_generation.py(2 hunks)src/draive/bedrock/lmm.py(5 hunks)src/draive/conversation/realtime/default.py(2 hunks)src/draive/conversation/types.py(4 hunks)src/draive/gemini/lmm.py(3 hunks)src/draive/gemini/lmm_generation.py(1 hunks)src/draive/lmm/types.py(1 hunks)src/draive/mistral/lmm.py(4 hunks)src/draive/mistral/lmm_generation.py(4 hunks)src/draive/openai/lmm.py(4 hunks)src/draive/openai/lmm_session.py(15 hunks)src/draive/stages/stage.py(1 hunks)src/draive/vllm/lmm.py(3 hunks)src/draive/vllm/lmm_generation.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.py: Use absolute imports fromdraivepackage
Follow Ruff import ordering (standard library, third party, local)
Use Python 3.12+ type features (type unions with|, generic syntax)
Use base and abstract types likeSequenceorIterableinstead of concrete types
Use custom exceptions for specific errors
Use NumPy docstring convention for all functions, classes, and methods
Skip module-level docstrings unless explicitly requested
Include sections: Parameters, Returns, Raises, Notes (if needed) in docstrings
ALWAYS useSequence[T]instead oflist[T],Mapping[K,V]instead ofdict[K,V], andSet[T]instead ofset[T]for collections in State, Config and DataModel classes
Immutable updates to State, Config, and DataModel classes should be performed through the.updated()method
Files:
src/draive/vllm/lmm_generation.pysrc/draive/anthropic/lmm_generation.pysrc/draive/mistral/lmm_generation.pysrc/draive/conversation/realtime/default.pysrc/draive/mistral/lmm.pysrc/draive/gemini/lmm_generation.pysrc/draive/stages/stage.pysrc/draive/bedrock/lmm.pysrc/draive/conversation/types.pysrc/draive/lmm/types.pysrc/draive/vllm/lmm.pysrc/draive/anthropic/lmm.pysrc/draive/gemini/lmm.pysrc/draive/openai/lmm.pysrc/draive/openai/lmm_session.py
🧠 Learnings (6)
pyproject.toml (2)
Learnt from: KaQuMiQ
PR: #338
File: src/draive/lmm/init.py:1-2
Timestamp: 2025-06-16T10:28:07.434Z
Learning: The draive project requires Python 3.12+ as specified in pyproject.toml with "requires-python = ">=3.12"" and uses Python 3.12+ specific features like PEP 695 type aliases and generic syntax extensively throughout the codebase.
Learnt from: CR
PR: miquido/draive#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T11:09:57.256Z
Learning: Applies to **/*.py : Use absolute imports from draive package
src/draive/bedrock/lmm.py (1)
Learnt from: CR
PR: miquido/draive#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T11:09:57.256Z
Learning: Applies to **/*.py : Use base and abstract types like Sequence or Iterable instead of concrete types
src/draive/conversation/types.py (1)
Learnt from: KaQuMiQ
PR: #338
File: src/draive/conversation/types.py:123-144
Timestamp: 2025-06-16T10:09:51.508Z
Learning: In the draive.commons.metadata.Meta class, properties like identifier, origin_identifier, predecessor_identifier, and successor_identifier automatically parse stored hex strings into UUID objects when accessed, returning None if the string is missing or invalid. This means accessing meta.identifier returns a UUID object, not a string, even though it's stored internally as a hex string.
src/draive/lmm/types.py (1)
Learnt from: KaQuMiQ
PR: #338
File: src/draive/bedrock/lmm_generation.py:102-107
Timestamp: 2025-06-16T09:44:08.688Z
Learning: LMMInstruction is a type alias for str in the draive codebase, so LMMInstruction and str are equivalent types and can be used interchangeably.
src/draive/anthropic/lmm.py (1)
Learnt from: CR
PR: miquido/draive#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T11:09:57.256Z
Learning: Applies to **/*.py : Use base and abstract types like Sequence or Iterable instead of concrete types
src/draive/gemini/lmm.py (1)
Learnt from: CR
PR: miquido/draive#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T11:09:57.256Z
Learning: Applies to **/*.py : Use base and abstract types like Sequence or Iterable instead of concrete types
🧬 Code Graph Analysis (5)
src/draive/anthropic/lmm_generation.py (1)
src/draive/anthropic/lmm.py (1)
content_elements(141-203)
src/draive/conversation/realtime/default.py (2)
src/draive/lmm/types.py (12)
LMMSessionEvent(373-388)of(84-107)of(133-142)of(153-162)of(184-200)of(211-220)of(260-274)of(284-295)of(309-320)of(375-385)LMMToolRequest(258-279)LMMStreamChunk(307-327)src/draive/conversation/types.py (3)
ConversationEvent(229-254)of(231-248)ConversationMessageChunk(23-75)
src/draive/gemini/lmm_generation.py (1)
src/draive/gemini/lmm.py (1)
context_element_as_content(33-83)
src/draive/bedrock/lmm.py (7)
src/draive/multimodal/meta.py (1)
MetaContent(12-30)src/draive/multimodal/content.py (4)
MultimodalContent(24-236)text(171-174)media(58-68)to_str(176-187)src/draive/multimodal/text.py (1)
TextContent(10-27)src/draive/parameters/model.py (3)
DataModel(386-756)to_str(529-530)to_json(740-756)src/draive/multimodal/media.py (6)
MediaData(77-128)kind(67-68)kind(99-100)MediaReference(46-74)to_str(70-71)to_str(102-111)src/draive/lmm/types.py (1)
LMMCompletion(151-168)src/draive/bedrock/models.py (1)
ChatMessage(63-65)
src/draive/anthropic/lmm.py (7)
src/draive/lmm/types.py (1)
completion(225-250)src/draive/stages/stage.py (1)
completion(326-410)src/draive/multimodal/text.py (1)
TextContent(10-27)src/draive/multimodal/content.py (4)
text(171-174)media(58-68)to_str(176-187)meta(151-164)src/draive/multimodal/media.py (6)
MediaData(77-128)kind(67-68)kind(99-100)MediaReference(46-74)to_str(70-71)to_str(102-111)src/draive/multimodal/meta.py (1)
MetaContent(12-30)src/draive/parameters/model.py (3)
to_str(529-530)default(760-774)to_json(740-756)
🔇 Additional comments (34)
pyproject.toml (1)
24-24: Validate thehaiwayupgrade pathThe core dependency jumps from
~0.27.2to~0.27.5. Although this is a patch-level change, haiway occasionally introduces tracing/OTel tweaks that can surface at runtime. Recommend verifying the release notes and running the full suite against0.27.5to catch any subtle regressions before publishing..gitignore (1)
84-84: LGTM! Appropriate editor directory exclusion.Adding
.zed/to the gitignore is consistent with existing editor exclusions and follows standard practices for excluding editor-specific configuration directories.src/draive/bedrock/lmm.py (3)
1-1: LGTM! Proper import additions for the refactoring.The import additions (Generator, Sequence, MetaContent, MultimodalContent) correctly support the function signature changes and new content handling capabilities.
30-76: LGTM! Well-structured content conversion refactoring.The refactoring from single-element to multi-element generator-based processing is well-implemented:
- Proper use of
Sequence[MultimodalContentElement]follows coding guidelines- Generator return type appropriately handles multiple content items
- MetaContent transcript handling correctly converts content to text
- Error handling for unsupported MediaReference and media types is maintained
- The function correctly handles all expected content element types
The complexity is justified given the multiple content types being handled.
85-85: LGTM! Proper generator consumption.The
list(convert_content(...))calls correctly collect the generator output into lists as required by the ChatMessage content structure.Also applies to: 91-91, 116-116
src/draive/lmm/types.py (1)
179-179: LGTM! Harmless literal reordering.The reordering of
"result"and"error"in theLMMToolResponseHandlingtype alias doesn't affect functionality since Literal types are unordered unions. This may reflect a preferred priority order for tool response handling.src/draive/anthropic/lmm_generation.py (2)
20-20: LGTM! Import updated for refactored content conversion.The import change correctly reflects the shift from single-element
convert_content_elementto the generator-basedcontent_elementsfunction.
125-125: LGTM! Consistent content conversion refactoring.The change from list comprehension with
convert_content_elementto direct use ofcontent_elementsis consistent with the broader refactoring across LMM backends. The generator-based approach provides better flexibility for multi-element content processing.src/draive/vllm/lmm_generation.py (1)
115-115: LGTM! Improved parameter specificity.The change from passing the entire
completion_configto justcompletion_config.vision_detailsimproves modularity by providing only the specific data needed bycontext_element_as_messages. This reduces coupling and aligns with the broader refactoring effort across LMM backends.src/draive/mistral/lmm_generation.py (4)
43-43: LGTM! Import change aligns with content handling refactoring.The replacement of
content_element_as_content_chunkwithcontent_chunksis consistent with the broader refactoring pattern across LMM modules to use generator-based content conversion functions.
127-127: LGTM! Prefill content construction updated correctly.The change from list comprehension with
content_element_as_content_chunkto directly usinglist(content_chunks(...))correctly implements the new generator-based content conversion approach.
210-210: LGTM! Token usage fallbacks improve robustness.Adding
or 0fallbacks for token usage metrics ensures consistent metric recording when the API returnsNoneor falsy values for token counts. This is a good defensive programming practice.Also applies to: 218-218
332-332: LGTM! Streaming token usage fallbacks maintain consistency.The
or 0fallbacks for token usage metrics in the streaming path maintain consistency with the non-streaming implementation and ensure robust metric recording.Also applies to: 340-340
src/draive/gemini/lmm_generation.py (1)
117-119: LGTM! Request content construction aligns with updated function signature.The removal of
chain.from_iterableis correct sincecontext_element_as_contentnow returns a singleContentDictinstead of an iterable of content elements. The change creates the properContentListUnionDictstructure expected by the Gemini API.src/draive/conversation/realtime/default.py (1)
138-156: LGTM! Explicit input handling improves interface completeness.The explicit handling of both
ConversationMessageChunkandConversationEventinputs is an improvement over the previous approach that ignored events. The conversion logic correctly maps:
ConversationMessageChunk→LMMStreamChunkwith proper metadata transferConversationEvent→LMMSessionEventwith proper category and metadata transfersrc/draive/mistral/lmm.py (4)
2-2: LGTM!Import changes correctly add
GeneratorandSequencetypes needed for the new generator function, following coding guidelines to use abstract types.
100-146: LGTM!The refactored
content_chunksgenerator function correctly processes sequences of multimodal content elements and adds proper handling for transcript meta content while maintaining backward compatibility.
58-58: LGTM!Usage updates correctly replace list comprehensions with the new
content_chunksgenerator, maintaining functionality while improving consistency across the codebase.Also applies to: 66-66, 94-94
41-41: LGTM!Export list correctly updated to reflect the new
content_chunksfunction replacing the previous single-element converter.src/draive/anthropic/lmm.py (4)
2-2: LGTM!Import changes correctly add required abstract types for the new generator function, following coding guidelines.
141-204: LGTM!The
content_elementsgenerator function is well-implemented with comprehensive handling of Anthropic-specific meta content types (thinking, redacted_thinking) and general transcript content, while maintaining proper error handling and type safety.
58-58: LGTM!Usage updates correctly implement the new generator-based approach with proper type annotations and casting where needed, maintaining functionality while improving code consistency.
Also applies to: 76-76, 96-96, 133-133
44-44: LGTM!Export list correctly updated to reflect the new
content_elementsfunction, maintaining API consistency.src/draive/conversation/types.py (2)
131-131: Good defensive programming with identifier fallback.The addition of
or uuid4()ensures that every conversation message has a valid identifier, even when the metadata doesn't provide one. This is consistent with the default factory pattern used in the class definition.Also applies to: 139-139
132-133: Consistent metadata key naming improvement.The change from
"creation"to"created"is applied consistently across both reading and writing operations. This follows common naming conventions for timestamp fields.Also applies to: 140-141, 195-195, 206-206
src/draive/gemini/lmm.py (3)
33-84: Clean refactoring from pattern matching to explicit type checks.The change from pattern matching to if-elif checks improves clarity and the conversion to using the new
content_partsgenerator is consistent with the broader refactoring across LMM modules.
86-131: Well-implemented content parts generator with proper transcript handling.The generator function efficiently processes multimodal content elements with special handling for MetaContent categories. The transcript content is correctly converted to plain text (lines 119-123), which aligns with the PR's objective of fixing transcript handling.
133-167: Efficient generator pattern implementation.The conversion from list building to a generator improves memory efficiency while maintaining the same logic. This is consistent with the generator-based approach adopted across the LMM modules.
src/draive/openai/lmm.py (1)
120-128: Correct implementation of transcript metadata handling.The handling of MetaContent with category "transcript" properly converts the content to text format, which aligns with the PR's objective of fixing transcript handling in the OpenAI realtime integration.
src/draive/openai/lmm_session.py (5)
87-88: Good use of local TaskGroup for concurrency scope.Using a local TaskGroup provides better control over the concurrent tasks within the session scope.
92-94: Important websocket configuration for large messages.Explicitly setting
max_size: Noneremoves message size limits, which is essential for handling large audio/video content in realtime sessions.
186-209: Well-implemented audio transcript streaming support.The new event handlers for
response.audio_transcript.deltaandresponse.audio_transcript.donecorrectly wrap transcript content in MetaContent with category "transcript", maintaining consistency with the transcript handling across the LMM modules.
567-627: Improved content type distinction and transcript handling.The addition of the
input_contentflag properly distinguishes between input and output content types, and the MetaContent transcript handling (lines 604-612) is consistent with the transcript processing in other LMM modules.
779-785: Consistent transcript wrapping in MetaContent.The change to wrap transcripts in MetaContent with category "transcript" is consistent with the transcript handling pattern established across all LMM modules in this PR.
1 participant
No description provided.