Introduce multimodal MessageContent and session-aware chat hooks#7
Merged
JohnRichard4096 merged 2 commits intomainfrom Feb 12, 2026
Merged
Introduce multimodal MessageContent and session-aware chat hooks#7JohnRichard4096 merged 2 commits intomainfrom
JohnRichard4096 merged 2 commits intomainfrom
Conversation
Contributor
Reviewer's GuideIntroduces a more flexible multimodal message content system (including images and metadata), extends session-aware chat handling and matcher hooks, refines MCP tool property casting naming, updates documentation to describe sessions and architecture, and updates dependencies and versioning to support new async I/O and image handling. Sequence diagram for session-aware matcher hook invocation in ChatObject._process_chatsequenceDiagram
participant User
participant SessionsManager
participant ChatObject
participant MatcherManager
participant Hook as MatcherHook
User->>ChatObject: construct(user_input, session_id, auto_create_session, hook_args, hook_kwargs)
ChatObject->>SessionsManager: is_session_registered(session_id)
alt session not registered and auto_create_session
ChatObject->>SessionsManager: init_session(session_id)
end
ChatObject->>SessionsManager: get_session_data(session_id, None)
SessionsManager-->>ChatObject: SessionData or None
ChatObject->>ChatObject: store session and hook_args, hook_kwargs
User->>ChatObject: start _process_chat()
ChatObject->>MatcherManager: trigger_event(ChatEvent, config, ChatObject, preset, *hook_args, session=session, **hook_kwargs)
MatcherManager->>Hook: handle input event(ChatEvent, ChatObject, preset, session, *hook_args, **hook_kwargs)
Hook-->>MatcherManager: possibly modified ChatEvent
MatcherManager-->>ChatObject: return
ChatObject->>ChatObject: update messages from ChatEvent
ChatObject-->>User: stream model response
ChatObject->>MatcherManager: trigger_event(CompletionEvent, config, ChatObject, preset, *hook_args, session=session, **hook_kwargs)
MatcherManager->>Hook: handle completion event(CompletionEvent, ChatObject, preset, session, *hook_args, **hook_kwargs)
Hook-->>MatcherManager: possibly modified CompletionEvent
MatcherManager-->>ChatObject: return
ChatObject-->>User: final response
Class diagram for the new multimodal MessageContent hierarchyclassDiagram
class MessageContent {
+str type
+MessageContent(content_type str)
+str __str__()
+get_content()
}
class RawMessageContent {
+Any raw_data
+RawMessageContent(raw_data Any)
+get_content() Any
}
class StringMessageContent {
+str text
+StringMessageContent(text str)
+get_content() str
}
class MessageMetadata {
+str content
+dict~str, Any~ metadata
}
class MessageWithMetadata {
+str content
+dict~str, Any~ metadata
+MessageWithMetadata(content str, metadata dict~str, Any~)
+get_content() Any
+get_metadata() dict
+get_full_content() MessageMetadata
}
class ImageMessage {
+str|BytesIO|bytes image
+ImageMessage(image str|BytesIO|bytes)
+async get_image(headers dict~str, None~) BytesIO|bytes
+async curl_image(extra_headers dict) bytes
+get_content() str
+async save_to(path Path, headers dict)
}
class COMPLETION_RETURNING {
<<typealias>>
+MessageContent | str | UniResponse~str, None~
}
MessageContent <|-- RawMessageContent
MessageContent <|-- StringMessageContent
MessageContent <|-- MessageWithMetadata
MessageContent <|-- ImageMessage
Class diagram for ChatObject session and matcher hook integrationclassDiagram
class ChatObject {
+bool train
+str session_id
+Any data
+ModelPreset preset
+AmritaConfig config
+SessionData session
+asyncio.Queue _response_queue
+asyncio.Queue _overflow_queue
+bool _is_running
+bool _queue_done
+bool _has_consumer
+tuple~Any~ _hook_args
+dict~str, Any~ _hook_kwargs
+ChatObject(user_input Any, session_id str, train bool, context Any, config AmritaConfig, preset ModelPreset, auto_create_session bool, hook_args tuple~Any~, hook_kwargs dict~str, Any~, queue_size int, overflow_queue_size int)
+async _process_chat()
}
class SessionsManager {
+is_session_registered(session_id str) bool
+init_session(session_id str)
+get_session_data(session_id str, default SessionData) SessionData
}
class SessionData {
+Any memory
+ModelPreset preset
}
class MatcherManager {
+async trigger_event(event Any, config AmritaConfig, chat ChatObject, preset ModelPreset, *hook_args Any, session SessionData, **hook_kwargs Any)
}
ChatObject --> SessionsManager : obtains SessionData
ChatObject --> SessionData : holds session
ChatObject --> MatcherManager : trigger_event(..., session, hook_args, hook_kwargs)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
ChatObject.__init__,hook_kwargsuses a mutable default{}which can lead to state being shared across instances; consider defaulting toNoneand initializing a new dict inside the method. - The
ImageMessage.get_imageandcurl_imagesignatures usedict[str, None] | Noneand untypeddictfor headers, but the actual usage expects standard HTTP header values (typicallystr); tightening these type hints (e.g.,dict[str, str] | None) will make the API clearer and avoid type confusion. - Now that
call_apiandcall_completioncan yieldMessageContentinstead of raw strings, double-check any downstream consumers (e.g., places usingRESPONSE_TYPEor assumingstrchunks) to ensure they correctly handle the newMessageContenttypes rather than relying on implicitstrbehavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `ChatObject.__init__`, `hook_kwargs` uses a mutable default `{}` which can lead to state being shared across instances; consider defaulting to `None` and initializing a new dict inside the method.
- The `ImageMessage.get_image` and `curl_image` signatures use `dict[str, None] | None` and untyped `dict` for headers, but the actual usage expects standard HTTP header values (typically `str`); tightening these type hints (e.g., `dict[str, str] | None`) will make the API clearer and avoid type confusion.
- Now that `call_api` and `call_completion` can yield `MessageContent` instead of raw strings, double-check any downstream consumers (e.g., places using `RESPONSE_TYPE` or assuming `str` chunks) to ensure they correctly handle the new `MessageContent` types rather than relying on implicit `str` behavior.
## Individual Comments
### Comment 1
<location> `src/amrita_core/protocol.py:109-110` </location>
<code_context>
+ super().__init__("image")
+ self.image: str | BytesIO | bytes = image
+
+ async def get_image(
+ self, headers: dict[str, None] | None = None
+ ) -> BytesIO | bytes:
+ if isinstance(self.image, str):
</code_context>
<issue_to_address>
**issue:** The headers type annotation is overly restrictive and inconsistent with `curl_image`.
`headers` is typed as `dict[str, None] | None`, which implies all values are `None`, but `curl_image` does `headers.update(extra_headers or {})` and HTTP headers normally have `str` values. `curl_image` also uses `dict | None` for `extra_headers`, so the two signatures diverge. Please align them on a more accurate type like `Mapping[str, str] | None` (or `dict[str, str] | None`) for both methods.
</issue_to_address>
### Comment 2
<location> `src/amrita_core/protocol.py:70-79` </location>
<code_context>
+ return obj
+ raise ValueError("Image must be a URL to use this method")
+
+ def get_content(self) -> str:
+ if isinstance(self.image, str):
+ return f""
+ elif isinstance(self.image, BytesIO):
+ self.image = self.image.getvalue()
+ image_type = get_image_format(self.image)
+ if not image_type:
+ return "[Unsupported image format]"
+ return f".decode()})"
+
+ async def save_to(self, path: Path, headers: dict | None = None):
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Clarify whether `bytes` represent raw image bytes or already-base64-encoded data to avoid double-encoding.
The docstring says `bytes` are “Base64 encoded image”, but `get_content` treats `self.image` as raw bytes and calls `base64.b64encode(self.image)`, which would double-encode already-base64 input and break the data URL. Please either update the docstring to say the `bytes` are raw image bytes, or adjust `get_content` to skip `b64encode` when `self.image` is documented/expected to be base64-encoded already.
</issue_to_address>
### Comment 3
<location> `src/amrita_core/chatmanager.py:403-404` </location>
<code_context>
config: AmritaConfig | None = None,
preset: ModelPreset | None = None,
auto_create_session: bool = False,
+ hook_args: tuple[Any, ...] = (),
+ hook_kwargs: dict[str, Any] = {},
queue_size: int = 25,
overflow_queue_size: int = 45,
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid using a mutable dict as a default argument for `hook_kwargs`.
Using `{}` as the default means all callers that rely on the default share the same dict, which can cause unintended state leakage between instances. Use `hook_kwargs: dict[str, Any] | None = None` and in `__init__` set `self._hook_kwargs = hook_kwargs or {}` instead.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Deploying amritacore with
|
| Latest commit: |
6dbd97c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9068a8f7.amritacore.pages.dev |
| Branch Preview URL: | https://feat-upd.amritacore.pages.dev |
Member
Author
|
@sourcery-ai title |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Introduce a unified multimodal message content abstraction and strengthen session-awareness across the chat pipeline while updating docs and dependencies accordingly.
New Features:
Enhancements:
Build:
Documentation: