Skip to content

feat: add return types to invoke_hook #707

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jakelorocco wants to merge 1 commit into
base: main
Choose a base branch
from
+14 −5
Open

feat: add return types to invoke_hook #707

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions mellea/plugins/manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from __future__ import annotations

import logging
from typing import TYPE_CHECKING, Any, Literal
from typing import TYPE_CHECKING, Any, Literal, TypeVar, cast

from mellea.plugins.base import MelleaBasePayload, PluginViolationError
from mellea.plugins.context import build_global_context
Expand Down Expand Up @@ -175,13 +175,17 @@ def deregister_session_plugins(session_id: str) -> None:
logger.debug("Plugin %s already unregistered", name, exc_info=True)


# Hooks return the same payload they received. Use this to accurately reflect that typing.
_MelleaBasePayload = TypeVar("_MelleaBasePayload", bound=MelleaBasePayload)


async def invoke_hook(
hook_type: HookType,
payload: MelleaBasePayload,
payload: _MelleaBasePayload,
*,
backend: Backend | None = None,
**context_fields: Any,
) -> tuple[Any | None, MelleaBasePayload]:
) -> tuple[Any | None, _MelleaBasePayload]:
"""Invoke a hook if plugins are configured.

Returns ``(result, possibly-modified-payload)``.
Expand Down Expand Up @@ -241,7 +245,12 @@ async def invoke_hook(
plugin_name=v.plugin_name or "",
)

modified = (
result.modified_payload if result and result.modified_payload else payload
# `result` doesn't type the returned payload correctly.
# If the modified payload exists, cast it as the correct type here,
# else return the original payload.
modified: _MelleaBasePayload = (
cast(_MelleaBasePayload, result.modified_payload)
Copy link
Copy Markdown
Contributor

@planetf1 planetf1 Apr 7, 2026

Choose a reason for hiding this comment

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

the cast works to keep type checking quiet - but it doesn't add any runtime checking. - but would need cpex changes to improve?

Copy link
Copy Markdown
Contributor Author

@jakelorocco jakelorocco Apr 7, 2026

Choose a reason for hiding this comment

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

Yes; the intent here is to just add the type hinting. Stronger assertions would require a lot more code or some sort of mechanism in cpex I think.

We are already relying on the assumption that input type == output type though, so I think this is safe / okay.

if result and result.modified_payload
else payload
)
return result, modified
Loading