Add DI matcher system, preset fallbacks, and Anthropic adapter#24
Closed
JohnRichard4096 wants to merge 9 commits intomainfrom
Closed
Add DI matcher system, preset fallbacks, and Anthropic adapter#24JohnRichard4096 wants to merge 9 commits intomainfrom
JohnRichard4096 wants to merge 9 commits intomainfrom
Conversation
Member
Author
|
@sourcery-ai title |
Contributor
Reviewer's GuideIntroduces a new event matcher and dependency injection system with support for preset fallback handling, Anthropic adapter integration, and CoT/think-tag filtering; refactors matcher internals around a priority-based EventRegistry and MatcherFactory, adds FallbackContext-based retry flows in ChatObject, extends protocol and config types, and updates docs/tests and project dependencies accordingly. Class diagram for new matcher and dependency injection systemclassDiagram
class EventRegistry {
<<singleton>>
-_instance: EventRegistry
-_event_handlers: defaultdict~str, defaultdict~int, list~FunctionData~~~~
+__new__() EventRegistry
+register_handler(event_type: str, data: FunctionData) void
+get_handlers(event_type: str) defaultdict~int, list~FunctionData~~~~
+_all() defaultdict~str, defaultdict~int, list~FunctionData~~~~
+get_all() defaultdict~str, defaultdict~int, list~FunctionData~~~~
}
class FunctionData {
+function: Callable
+signature: inspect.Signature
+frame: FrameType
+priority: int
+matcher: Matcher
}
class Matcher {
+event_type: EventTypeEnum~str|
+priority: int
+block: bool
+append_handler(func: Callable[..., Awaitable~Any~]]) void
+wrap_function(func: Callable[..., Awaitable~Any~]]) Callable
+set_block(block: bool) void
+stop_process() void
+cancel_matcher() void
+pass_event() void
}
class MatcherFactory {
<<utility>>
+trigger_event(event: BaseEvent, config: AmritaConfig, *args: Any, exception_ignored: tuple~type[Exception],...~, **kwargs: Any) void
+_simple_run(matcher_list: list~FunctionData~, event: BaseEvent, config: AmritaConfig, exception_ignored: tuple~type[BaseException],...~, extra_args: tuple, extra_kwargs: dict~str, Any~) bool
+_resolve_dependencies(signature: inspect.Signature, session_args: Iterable~Any~, session_kwargs: dict~str, Any~) tuple~bool, tuple, dict~str, Any~, dict~str, DependsFactory~~
+_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
}
class DependsFactory~T~ {
-_depency_func: Callable[..., T|Awaitable~T~]
+__init__(depency: Callable[..., T|Awaitable~T~])
+resolve(*args, **kwargs) T|None
}
class DependsFn {
<<function>>
+Depends(dependency: Callable[..., T|Awaitable~T~]) Any
}
class BaseEvent {
<<abstract>>
+get_event_type() EventTypeEnum~str|
+event_type: EventTypeEnum~str|
}
class EventTypeEnum {
<<enum>>
PRE_COMPLETION
COMPLETION
PRESET_FALLBACK
Nil
+validate(name: str) bool
}
class MatcherException {
}
class BlockException {
}
class CancelException {
}
class PassException {
}
class AmritaConfig {
}
%% Relationships
EventRegistry --> "*" FunctionData : stores
FunctionData --> Matcher : matcher
MatcherFactory ..> EventRegistry : uses
MatcherFactory ..> FunctionData : runs
MatcherFactory ..> DependsFactory : resolves
Matcher ..> EventRegistry : register_handler
Matcher ..> BaseEvent : handles
DependsFactory ..> MatcherFactory : uses _resolve_dependencies
DependsFn ..> DependsFactory : creates
BaseEvent <|-- FallbackContext
BaseEvent <|-- Event
MatcherException <|-- BlockException
MatcherException <|-- CancelException
MatcherException <|-- PassException
MatcherFactory ..> BaseEvent
MatcherFactory ..> AmritaConfig
MatcherFactory ..> MatcherException
MatcherFactory ..> EventTypeEnum
class FallbackContext {
+preset: ModelPreset
+exc_info: BaseException
+config: AmritaConfig
+context: SendMessageWrap
+term: int
+event_type: EventTypeEnum
+get_event_type() EventTypeEnum
+fail(reason: Any|None) Never
}
class Event {
+user_input: USER_INPUT
+config: AmritaConfig
+chat_object: ChatObject
+event_type: EventTypeEnum
+get_event_type() EventTypeEnum
}
class ChatObject {
+preset: ModelPreset
+config: AmritaConfig
+context_wrap: SendMessageWrap
+_process_chat() None
}
ChatObject ..> MatcherFactory : trigger_event
FallbackContext ..> FallbackFailed
class FallbackFailed {
}
Class diagram for adapters, protocol changes, and CoT filteringclassDiagram
class LLMConfig {
+auto_retry: bool
+max_retries: int
+max_fallbacks: int
+enable_memory_abstract: bool
}
class ModelConfig {
+temperature: float
+top_p: float
+stream: bool
+multimodal: bool
+cot_model: bool
}
class ModelPreset {
+name: str
+model: str
+api_key: str
+base_url: str
+config: ModelConfig
}
class AmritaConfig {
+llm: LLMConfig
}
class UniResponseUsage~T~ {
+prompt_tokens: T
+completion_tokens: T
+total_tokens: T
}
class UniResponse {
+role: str
+content: str
+usage: UniResponseUsage~int~
+tool_calls: Any
}
class MessageContent {
}
class CompletionChunk {
+content: str
+metadata: dict~str, Any~
+get_content() str
+get_metadata() dict
}
class ModelAdapter {
<<abstract>>
-preset: ModelPreset
-config: AmritaConfig
+call_api(messages: Iterable, **kwargs) AsyncGenerator~COMPLETION_RETURNING, None~
+call_tools(...) AsyncGenerator
+get_adapter_protocol() str|tuple~str,...~
}
class OpenAIAdapter {
+call_api(messages: Iterable, *args, **kwargs) AsyncGenerator~COMPLETION_RETURNING, None~
+call_tools(...)
}
class AnthropicAdapter {
+call_api(messages: Iterable, *args, **kwargs) AsyncGenerator~COMPLETION_RETURNING, None~
+get_adapter_protocol() str|tuple~str,...~
}
class LibChatInner {
<<function>>
+inner(preset: ModelPreset, config: AmritaConfig, messages: Iterable) AsyncGenerator
}
ModelAdapter <|-- OpenAIAdapter
ModelAdapter <|-- AnthropicAdapter
ModelAdapter --> ModelPreset
ModelAdapter --> AmritaConfig
OpenAIAdapter ..> UniResponse
OpenAIAdapter ..> UniResponseUsage
AnthropicAdapter ..> UniResponse
AnthropicAdapter ..> UniResponseUsage
CompletionChunk --> MessageContent : content type
LibChatInner ..> ModelAdapter : uses call_api via reflection
LibChatInner ..> ModelPreset
LibChatInner ..> AmritaConfig
note for LibChatInner "If preset.config.cot_model is True, inner() filters out <think>...</think> chunks before yielding to callers"
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
sourcery-ai
Bot
changed the title
![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 3 issues, and left some high level feedback:
- In
MatcherFactory._simple_run,isinstance(e, CancelException | BlockException)will raise aTypeErroron Python 3.10+ because|produces atypes.UnionType; useisinstance(e, (CancelException, BlockException))instead. - In
ChatObject._process_chat, the fallback logging and trigger call look inconsistent with the newmax_fallbacksbehavior: the warning still usesself.config.llm.max_retriesin the message, andMatcherManager.trigger_event(ctx, ctx.config, (FallbackFailed,))passesFallbackFailedas a positional arg rather than viaexception_ignored=(FallbackFailed,), so fallback exceptions won’t be treated as intended. - The helper
model_dumpinbuiltins/adapter.pyshadows itsobjparameter name inside the list comprehension (for obj in obj), which is confusing and easy to misuse; consider renaming the loop variable (e.g.for item in obj) to avoid shadowing and clarify intent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `MatcherFactory._simple_run`, `isinstance(e, CancelException | BlockException)` will raise a `TypeError` on Python 3.10+ because `|` produces a `types.UnionType`; use `isinstance(e, (CancelException, BlockException))` instead.
- In `ChatObject._process_chat`, the fallback logging and trigger call look inconsistent with the new `max_fallbacks` behavior: the warning still uses `self.config.llm.max_retries` in the message, and `MatcherManager.trigger_event(ctx, ctx.config, (FallbackFailed,))` passes `FallbackFailed` as a positional arg rather than via `exception_ignored=(FallbackFailed,)`, so fallback exceptions won’t be treated as intended.
- The helper `model_dump` in `builtins/adapter.py` shadows its `obj` parameter name inside the list comprehension (`for obj in obj`), which is confusing and easy to misuse; consider renaming the loop variable (e.g. `for item in obj`) to avoid shadowing and clarify intent.
## Individual Comments
### Comment 1
<location path="src/amrita_core/hook/matcher.py" line_range="448-449" />
<code_context>
+ elif isinstance(chunk, MessageContent | str):
+ await self.yield_response(chunk)
+ break
+ except Exception as e:
+ logger.warning(
+ f"Because of `{e!s}`, LLM request failed, retrying ({i}/{self.config.llm.max_retries})..."
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `CancelException | BlockException` inside `isinstance` will raise a `TypeError` at runtime.
This will raise `TypeError: isinstance() argument 2 cannot be a union` at runtime, since `isinstance` expects a type or tuple of types, not a PEP 604 union. Use a tuple instead, e.g.:
```python
if isinstance(e, (CancelException, BlockException)):
logger.info("Cancelled Matcher processing")
return False
```
As written, the `TypeError` will mask the original exception and break the intended control flow.
</issue_to_address>
### Comment 2
<location path="docs/guide/api-reference/classes/DependsFactory.md" line_range="8" />
<code_context>
+## Constructor
+
+```python
+def __init__(self, depency: Callable[..., T | Awaitable[T]])
+```
+
</code_context>
<issue_to_address>
**issue (typo):** Typo in parameter name `depency` – should be `dependency`.
Please rename the parameter to `dependency` in the constructor (and corresponding docs) to keep the public API clear and consistent.
</issue_to_address>
### Comment 3
<location path="docs/zh/guide/api-reference/classes/DependsFactory.md" line_range="8" />
<code_context>
+## Constructor
+
+```python
+def __init__(self, depency: Callable[..., T | Awaitable[T]])
+```
+
</code_context>
<issue_to_address>
**issue (typo):** 参数名 `depency` 存在拼写错误，建议改为 `dependency`。
该拼写在构造函数签名及后续参数说明中也一致使用了 `depency`，请统一更正为 `dependency`。
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+448
to
+449
| except Exception as e: | ||
| if isinstance(e, CancelException | BlockException): |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Using CancelException | BlockException inside isinstance will raise a TypeError at runtime.
This will raise TypeError: isinstance() argument 2 cannot be a union at runtime, since isinstance expects a type or tuple of types, not a PEP 604 union. Use a tuple instead, e.g.:
if isinstance(e, (CancelException, BlockException)):
logger.info("Cancelled Matcher processing")
return FalseAs written, the TypeError will mask the original exception and break the intended control flow.
| ## Constructor | ||
|
|
||
| ```python | ||
| def __init__(self, depency: Callable[..., T | Awaitable[T]]) |
Contributor
There was a problem hiding this comment.
issue (typo): Typo in parameter name depency – should be dependency.
Please rename the parameter to dependency in the constructor (and corresponding docs) to keep the public API clear and consistent.
| ## Constructor | ||
|
|
||
| ```python | ||
| def __init__(self, depency: Callable[..., T | Awaitable[T]]) |
Contributor
There was a problem hiding this comment.
issue (typo): 参数名 depency 存在拼写错误，建议改为 dependency。
该拼写在构造函数签名及后续参数说明中也一致使用了 depency，请统一更正为 dependency。
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.
close #22
Summary by Sourcery
Add a new dependency-injection-based matcher system with preset fallback handling and Anthropic adapter support, and extend documentation and tests accordingly.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests: