Skip to content

[Bug]: replay() leaks its own kwargs into strategy causing TypeError #948

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.

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
ai-yann opened this issue Apr 17, 2025 · 0 comments
Open
Labels
bug Something isn't working

Comments

@ai-yann
Copy link

ai-yann commented Apr 17, 2025

Describe the bug

openadapt.replay.replay() forwards every keyword argument it receives to the chosen strategy’s constructor.
Kwargs that belong only to replay() itself—e.g. recording_id, timestamp, capture, status_pipe—end up in the strategy __init__, which usually does not accept them, raising:

TypeError: VisualReplayStrategy.__init__() got an unexpected keyword argument 'recording_id'

Expected behaviour

replay() should strip its own kwargs and pass only the ones intended for the strategy constructor.


Actual behaviour

Internal kwargs are forwarded wholesale, triggering the TypeError above whenever the strategy doesn’t accept them.


Root cause (code)

openadapt/replay.py, inside replay() :

strategy = strategy_class(recording, **kwargs)  # kwargs still includes recording_id, etc.

Proposed fix

A deny‑list works, but an allow‑list derived from the strategy signature is maintenance‑free:

import inspect

init_params = set(inspect.signature(strategy_class.init).parameters) - {"self"}
strategy_kwargs = {k: v for k, v in kwargs.items() if k in init_params}
strategy = strategy_class(recording, **strategy_kwargs)
logger.debug("Passing to %s: %s", strategy_class.name, strategy_kwargs)


Environment

Item Value (replace)
OpenAdapt main @ 1899393
Python 3.11.9
OS macOS 15.3.2

Workaround

Subclass the strategy so it absorbs unexpected kwargs:

from openadapt.strategies import VisualReplayStrategy

class PatchedVisualReplayStrategy(VisualReplayStrategy):
def init(self, recording, instructions, **_):
super().init(recording, instructions)

Pass "PatchedVisualReplayStrategy" to replay() until the fix lands.


Thanks for maintaining OpenAdapt—happy to open a PR with the patch!

To Reproduce

Minimal repro

from openadapt.replay import replay

replay(
    "VisualReplayStrategy",          # strategy (positional OK)
    recording_id=1,                   # belongs to replay(), not the strategy
    instructions="Do the task"        # legitimate strategy kwarg
)

Any strategy whose constructor lacks recording_id (or **kwargs) will fail identically.

@ai-yann ai-yann added the bug Something isn't working label Apr 17, 2025
@ai-yann ai-yann changed the title [Bug]: Bug: replay() leaks its own kwargs into strategy causing TypeError [Bug]: replay() leaks its own kwargs into strategy causing TypeError Apr 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant