Skip to content

Refine matcher runtime deps in trigger_event and simplify _simple_run#63

Merged
JohnRichard4096 merged 1 commit intomainfrom
update/perform
May 3, 2026
Merged

Refine matcher runtime deps in trigger_event and simplify _simple_run#63
JohnRichard4096 merged 1 commit intomainfrom
update/perform

Conversation

@JohnRichard4096
Copy link
Copy Markdown
Member

@JohnRichard4096 JohnRichard4096 commented May 3, 2026

Summary by Sourcery

Refine matcher execution to centralize runtime dependency resolution in event triggering and simplify _simple_run arguments, while bumping the package version.

Enhancements:

  • Remove runtime dependency resolution support from _simple_run and enforce prior resolution or use via trigger_event.
  • Move runtime dependency injection handling into trigger_event using DependsFactory-aware argument preprocessing.
  • Simplify _simple_run signature by removing explicit event and config parameters and generalizing extra_args to an iterable.

Build:

  • Bump project version from 0.8.5 to 0.8.6.

Tests:

  • Update matcher tests to reflect the new _simple_run calling convention and removal of runtime dependency resolution behavior.

@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai Bot commented May 3, 2026

Reviewer's Guide

Refactors matcher execution to move runtime dependency resolution from _simple_run into trigger_event, simplifies _simple_run’s API and behavior, and updates tests and versioning to reflect the new dependency injection model and performance focus.

Updated class diagram for matcher dependency injection model

classDiagram
    class MatcherFactory {
        +trigger_event(event, *args, **kwargs, config)
        +_simple_run(matcher_list, exception_ignored, extra_args, extra_kwargs) bool
        +_do_runtime_resolve(runtime_args, runtime_kwargs, args2update, kwargs2update, session_args, session_kwargs, exception_ignored) bool
        +_resolve_dependencies(signature, session_args, session_kwargs) (bool, list, dict, dict)
    }

    class FunctionData {
        +function
        +priority
    }

    class DependsFactory {
        +call()
    }

    class BaseEvent {
    }

    class AmritaConfig {
    }

    MatcherFactory --> FunctionData : uses matcher_list
    MatcherFactory --> DependsFactory : resolves_dependencies
    MatcherFactory --> BaseEvent : consumes_event
    MatcherFactory --> AmritaConfig : optional_config

    FunctionData o--> HandlerFunction

    class HandlerFunction {
        +__call__(...)
    }
Loading

Flow diagram for argument preparation and runtime DI resolution in trigger_event

flowchart TD
    A[trigger_event called
    with event, args, kwargs, config] --> B["Build s_args<br/>[event, args, config_if_present]"]
    B --> C[Deep copy kwargs
    to session_kwargs]
    C --> D[Scan s_args
    for DependsFactory
    instances]
    C --> E[Scan session_kwargs
    for DependsFactory
    instances]
    D --> F{runtime_args or
    runtime_kwargs exist?}
    E --> F

    F -- Yes --> G[Call _do_runtime_resolve
    to fill s_args and
    session_kwargs]
    G --> H{resolved?}
    H -- No --> I[Raise RuntimeError
    Runtime arguments
    cannot be resolved]
    H -- Yes --> J[Proceed with
    resolved s_args and
    session_kwargs]

    F -- No --> J

    J --> K[Loop priorities]
    K --> L[Call _simple_run with
    matcher_list,
    extra_args=s_args,
    extra_kwargs=session_kwargs]

    L --> M[Inside _simple_run:
    reject DependsFactory in
    extra_args or extra_kwargs]
    M --> N[Resolve handler-level
    dependencies and run
    handlers]
    N --> O[Return from
    trigger_event]
Loading

File-Level Changes

Change Details Files
Simplify MatcherFactory._simple_run API and remove runtime dependency resolution from it.
  • Remove event and config parameters from _simple_run and treat extra_args as a generic iterable of arguments.
  • Disallow DependsFactory instances in extra_args and extra_kwargs within _simple_run, raising ValueError if present instead of resolving them.
  • Build session_args from matcher plus extra_args only, and pass extra_kwargs directly into dependency resolution.
  • Adjust calls to _do_default_resolve to use extra_kwargs as the session kwargs context.
src/amrita_core/hook/matcher.py
Move runtime dependency injection handling into trigger_event before delegating to _simple_run.
  • In trigger_event, construct a unified argument list s_args including event, additional args, and config (if present).
  • Scan s_args and kwargs for DependsFactory instances and resolve them via _do_runtime_resolve before running matchers.
  • Raise RuntimeError from trigger_event when runtime arguments cannot be resolved, keeping _simple_run free of runtime DI concerns.
  • Invoke _simple_run with pre-resolved s_args and session_kwargs for each priority without passing event or config explicitly.
src/amrita_core/hook/matcher.py
Align tests with the new _simple_run signature and runtime DI behavior.
  • Update tests to call _simple_run with exception_ignored, extra_args, and extra_kwargs instead of separate event and config parameters.
  • Remove tests that exercised runtime dependency resolution inside _simple_run, since this responsibility moved to trigger_event.
  • Keep and adapt tests that validate default dependency resolution and failure behavior to the new API shape.
tests/test_matcher.py
Bump package version to reflect behavior changes.
  • Increment project version from 0.8.5 to 0.8.6 in pyproject.toml.
  • Update lockfile metadata accordingly.
pyproject.toml
uv.lock

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@JohnRichard4096
Copy link
Copy Markdown
Member Author

@sourcery-ai title

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • In _simple_run, extra_args is typed as Iterable[Any] but is iterated multiple times and then combined into a list; consider tightening this to Sequence[Any] (or converting once at the top) to avoid surprises with one-shot iterables.
  • The runtime DI resolution logic is now duplicated between trigger_event and the previous _simple_run implementation; consider extracting the common resolution into a shared helper to keep behavior consistent and reduce maintenance overhead.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `_simple_run`, `extra_args` is typed as `Iterable[Any]` but is iterated multiple times and then combined into a list; consider tightening this to `Sequence[Any]` (or converting once at the top) to avoid surprises with one-shot iterables.
- The runtime DI resolution logic is now duplicated between `trigger_event` and the previous `_simple_run` implementation; consider extracting the common resolution into a shared helper to keep behavior consistent and reduce maintenance overhead.

## Individual Comments

### Comment 1
<location path="src/amrita_core/hook/matcher.py" line_range="369-378" />
<code_context>
         /,
         exception_ignored: tuple[type[BaseException], ...],
-        extra_args: tuple,
+        extra_args: Iterable[Any],
         extra_kwargs: dict[str, Any],
-        config: AmritaConfig | None = None,
</code_context>
<issue_to_address>
**issue (bug_risk):** Using a generic Iterable for extra_args risks consuming single-use iterables before they are expanded.

Because extra_args is now Iterable[Any], it’s iterated once in the any(...) check and then expanded with *extra_args for session_args. For single-use iterables (e.g. generators), the first pass will exhaust them and session_args will miss those values.

To prevent this while keeping the general type, normalize extra_args at the start of _simple_run, for example:

```python
def _simple_run(..., extra_args: Iterable[Any], ...):
    extra_args = tuple(extra_args)
    ...
```

Alternatively, change the type back to a multi-pass container like Sequence/tuple to make this usage explicit.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src/amrita_core/hook/matcher.py
@sourcery-ai sourcery-ai Bot changed the title Performance: Event DI Refine matcher runtime deps in trigger_event and simplify _simple_run May 3, 2026
@JohnRichard4096 JohnRichard4096 merged commit 97cbc4b into main May 3, 2026
7 checks passed
@JohnRichard4096 JohnRichard4096 deleted the update/perform branch May 3, 2026 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant