Relax event config requirements and add embedding call helper#58
Merged
JohnRichard4096 merged 5 commits intomainfrom Apr 25, 2026
Merged
Relax event config requirements and add embedding call helper#58JohnRichard4096 merged 5 commits intomainfrom
JohnRichard4096 merged 5 commits intomainfrom
Conversation
Contributor
Reviewer's GuideRefactors event handling and dependency resolution APIs to support optional configuration and improved type clarity, aligns agent strategy abstractions, centralizes adapter reflection usage while adding embedding support, removes unused exceptions/utilities, and bumps the package version to 0.8.4. Sequence diagram for updated trigger_event and matcher executionsequenceDiagram
participant Caller
participant Matcher as Matcher
participant EventRegistry as EventRegistry
participant Handler as HandlerFunction
Caller->>Matcher: trigger_event(event, *args, config=None, exception_ignored, **kwargs)
activate Matcher
Matcher->>Matcher: extract event and optional config from args/kwargs
Matcher->>EventRegistry: get_handlers(event_type)
EventRegistry-->>Matcher: handlers_by_priority
loop priorities
Matcher->>Matcher: _simple_run(matcher_list, event, exception_ignored, extra_args, extra_kwargs, config)
activate Matcher
loop matcher_list
Matcher->>Matcher: build session_args [matcher, event, *extra_args, config?]
Matcher->>Matcher: build session_kwargs from extra_kwargs
alt has runtime_args or runtime_kwargs
Matcher->>Matcher: _do_runtime_resolve(runtime_args, runtime_kwargs, args2update, kwargs2update, session_args, session_kwargs, exception_ignored)
alt resolve failed
Matcher-->>Matcher: raise RuntimeError
end
end
alt has Depends in handler kwargs
Matcher->>Matcher: _do_runtime_resolve(runtime_args={}, runtime_kwargs=d_kw, args2update=[], kwargs2update=f_kwargs, session_args, session_kwargs, exception_ignored)
alt resolve failed
Matcher-->>Matcher: continue to next matcher
end
end
Matcher->>Handler: await handler(*session_args, **session_kwargs)
alt handler raises CancelException
Matcher-->>Caller: return False (stop processing)
else handler raises ChatException or other
Matcher-->>Matcher: log and continue based on exception type
end
end
deactivate Matcher
end
Matcher-->>Caller: return None
Sequence diagram for shared adapter reflection in libchatsequenceDiagram
participant Client
participant Libchat as libchat
participant PresetManager
participant AdapterClass as ModelAdapterClass
participant Adapter as ModelAdapter
Client->>Libchat: tools_caller(messages, tools, tool_choice, preset?, config?)
activate Libchat
Libchat->>PresetManager: get_default_preset() (if preset is None)
PresetManager-->>Libchat: preset
Libchat->>Libchat: _call_with_reflection(preset, _call_tools, config)
activate Libchat
Libchat->>AdapterClass: instantiate(preset, config)
AdapterClass-->>Libchat: Adapter
Libchat->>Adapter: call_tools(messages, tools, tool_choice)
Adapter-->>Libchat: tools_result
Libchat-->>Client: tools_result
deactivate Libchat
Client->>Libchat: call_completion(messages, preset?, config?, **kwargs)
activate Libchat
Libchat->>PresetManager: get_default_preset() (if preset is None)
PresetManager-->>Libchat: preset
Libchat->>Libchat: _call_with_reflection(preset, _call_api, config)
activate Libchat
Libchat->>AdapterClass: instantiate(preset, config)
AdapterClass-->>Libchat: Adapter
Libchat->>Adapter: call_api(messages_model_dump, **kwargs)
Adapter-->>Libchat: async_generator
Libchat-->>Client: async_generator
deactivate Libchat
Client->>Libchat: call_embedding(text_iterable, preset, config?, **kwargs)
activate Libchat
Libchat->>Libchat: _call_with_reflection(preset, _call_embed, config)
activate Libchat
Libchat->>AdapterClass: instantiate(preset, config)
AdapterClass-->>Libchat: Adapter
Libchat->>Adapter: call_embed(text_iterable, **kwargs)
Adapter-->>Libchat: embeddings
Libchat-->>Client: embeddings
deactivate Libchat
Class diagram for matcher, exceptions, and event handling changesclassDiagram
class MatcherException {
}
class CancelException {
}
MatcherException <|-- CancelException
class Matcher {
<<utility>>
+async _do_runtime_resolve(runtime_args: dict~int, DependsFactory~, runtime_kwargs: dict~str, DependsFactory~, args2update: list~Any~, kwargs2update: dict~str, Any~, session_args: list~Any~, session_kwargs: dict~str, Any~, exception_ignored: tuple~type~BaseException~~, ...~) bool
+async _simple_run(matcher_list: list~FunctionData~, event: BaseEvent, /, exception_ignored: tuple~type~BaseException~~, extra_args: tuple, extra_kwargs: dict~str, Any~, config: AmritaConfig|None = None) bool
+async trigger_event(event: BaseEvent, *args: Any, config: AmritaConfig|None = None, exception_ignored: tuple~type~Exception~~ = (), **kwargs) None
}
class FunctionData {
+signature: Signature
+frame: FrameType
+matcher
+function
+d_args
+d_kwargs
}
class BaseEvent {
+get_event_type() EventTypeEnum|str
}
class EventRegistry {
+get_handlers(event_type: EventTypeEnum|str) dict~int, list~FunctionData~~
}
class AmritaConfig {
}
class DependsFactory {
}
class EventTypeEnum {
}
Matcher --> EventRegistry : uses
Matcher --> BaseEvent : handles
Matcher --> FunctionData : executes
Matcher --> AmritaConfig : optional config
Matcher --> DependsFactory : resolves
BaseEvent --> EventTypeEnum : returns
class CancelExceptionHandler {
+handle(e: CancelException)
}
CancelExceptionHandler ..> CancelException : handles
Class diagram for updated agent strategy abstractionsclassDiagram
class BaseReActAgentStrategy {
<<abstract>>
+ctx
+async _build_stop_response_and_append()
+async _handle_error_append()
+async _append_reasoning(tool_call: ToolCall, reasoning_content: UniResponse~str, None~)
+async _append_tool_result_to_context(tool_call: ToolCall, func_response: str, response_msg: UniResponse~None, list~ToolCall~|None~)
}
class ReActAgentStrategy {
}
class HybridReActAgentStrategy {
}
class NoActionAgentStrategy {
}
class AmritaAgentStrategy {
<<alias of ReActAgentStrategy>>
}
class ToolCall {
}
class UniResponse~T, E~ {
}
BaseReActAgentStrategy <|-- ReActAgentStrategy
BaseReActAgentStrategy <|-- HybridReActAgentStrategy
BaseReActAgentStrategy <|-- NoActionAgentStrategy
BaseReActAgentStrategy --> ToolCall : uses
BaseReActAgentStrategy --> UniResponse : uses
AmritaAgentStrategy .. ReActAgentStrategy : alias
Class diagram for shared adapter reflection and new embedding supportclassDiagram
class ModelPreset {
+base_url: str
+model: str
+config
}
class AmritaConfig {
}
class PresetManager {
+get_default_preset() ModelPreset
}
class ModelAdapter {
+ModelAdapter(preset: ModelPreset, config: AmritaConfig)
+call_tools(messages: CONTENT_LIST_TYPE, tools, tool_choice)
+call_api(messages: list~dict~, **kwargs) AsyncGenerator
+call_embed(text: Iterable~str~, **kwargs) Sequence~EmbeddingChunk~
}
class EmbeddingChunk {
}
class LibchatAPI {
<<module functions>>
+async tools_caller(messages: CONTENT_LIST_TYPE, tools, tool_choice, preset: ModelPreset|None = None, config: AmritaConfig|None = None)
+async call_completion(messages: CONTENT_LIST_TYPE, preset: ModelPreset|None = None, config: AmritaConfig|None = None, **kwargs) AsyncGenerator
+async call_embedding(text: Iterable~str~, preset: ModelPreset, config: AmritaConfig|None = None, **kwargs) Sequence~EmbeddingChunk~
+async _call_with_reflection(preset: ModelPreset, call_func: Callable~ModelAdapter, Awaitable~T~~, config: AmritaConfig) T
}
LibchatAPI --> PresetManager : uses
LibchatAPI --> ModelPreset : uses
LibchatAPI --> AmritaConfig : uses
LibchatAPI --> ModelAdapter : instantiates
LibchatAPI --> EmbeddingChunk : returns
ModelAdapter --> ModelPreset : configured by
ModelAdapter --> AmritaConfig : configured by
ModelAdapter --> EmbeddingChunk : produces embeddings
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
_simple_run,session_argsnow appendconfigafter*extra_argsinstead of before, which can break existing handler signatures and dependency index assumptions; consider preserving the original argument order (matcher, event, config, *extra_args) or explicitly updating all dependent call sites/signatures accordingly. - The new
trigger_eventAPI both acceptsconfigas a keyword argument and also infers it from*args, which makes the call contract harder to reason about; consider makingconfigstrictly keyword-only (and not scanning*argsforAmritaConfig) to keep invocation clearer and less error-prone. - With
_call_with_reflectionnow forcingcall_functo accept onlyadapter, more complex uses must create nested wrappers as seen intools_callerandcall_completion; if you expect richer usage patterns, consider allowing*args/**kwargsagain or providing a small helper for constructing these adapter-bound callables to avoid repetitive wrapper boilerplate.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `_simple_run`, `session_args` now append `config` after `*extra_args` instead of before, which can break existing handler signatures and dependency index assumptions; consider preserving the original argument order (matcher, event, config, *extra_args) or explicitly updating all dependent call sites/signatures accordingly.
- The new `trigger_event` API both accepts `config` as a keyword argument and also infers it from `*args`, which makes the call contract harder to reason about; consider making `config` strictly keyword-only (and not scanning `*args` for `AmritaConfig`) to keep invocation clearer and less error-prone.
- With `_call_with_reflection` now forcing `call_func` to accept only `adapter`, more complex uses must create nested wrappers as seen in `tools_caller` and `call_completion`; if you expect richer usage patterns, consider allowing `*args`/`**kwargs` again or providing a small helper for constructing these adapter-bound callables to avoid repetitive wrapper boilerplate.
## Individual Comments
### Comment 1
<location path="src/amrita_core/hook/matcher.py" line_range="417-420" />
<code_context>
)
continue
except Exception as e:
- if isinstance(e, CancelException | BlockException):
+ if isinstance(e, CancelException):
logger.info("Cancelled Matcher processing")
return False
</code_context>
<issue_to_address>
**issue (bug_risk):** Dropping `BlockException` changes control-flow semantics and may break existing code relying on that type.
With `BlockException` removed from the control-flow path and the `isinstance` check restricted to `CancelException`, any external code that raises or catches `BlockException` may now break or behave differently. If the goal is to retire `BlockException`, consider a compatibility alias (e.g., `BlockException = CancelException`) or a deprecation path to avoid breaking downstream users.
</issue_to_address>
### Comment 2
<location path="src/amrita_core/libchat.py" line_range="305-310" />
<code_context>
return resp
+
+
+async def call_embedding(
+ text: Iterable[str],
+ preset: ModelPreset,
+ config: AmritaConfig | None = None,
+ **kwargs,
+) -> Sequence[EmbeddingChunk]:
+ config = config or get_config()
+
</code_context>
<issue_to_address>
**suggestion:** `call_embedding` requires an explicit preset unlike `call_completion` and `tools_caller`, which may be inconsistent for users.
Unlike `tools_caller` and `call_completion`, which accept an optional `preset` and default to `PresetManager().get_default_preset()`, this function requires a `ModelPreset`. Unless there’s a strong reason to force an explicit preset, consider making it optional and using the same defaulting behavior for consistency.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
|
@sourcery-ai title |
sourcery-ai
Bot
changed the 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
1 participant
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
Relax configuration requirements for event triggering and enhance model interaction utilities.
New Features:
Bug Fixes:
Enhancements:
Build:
Tests: