Skip to content

fix: reject unknown kwargs in @remote#288

Open
deanq wants to merge 4 commits intomainfrom
fix/AE-2313-remote-extra-swallows-typos
Open

fix: reject unknown kwargs in @remote#288
deanq wants to merge 4 commits intomainfrom
fix/AE-2313-remote-extra-swallows-typos

Conversation

@deanq
Copy link
Member

@deanq deanq commented Mar 25, 2026

Summary

  • **extra in @remote() silently swallowed typos like depndencies=["torch"], causing missing dependencies at runtime with opaque import errors
  • Added _reject_unknown_kwargs() validation that raises TypeError with "did you mean?" suggestions using difflib.get_close_matches
  • Removed dead extra parameter from the full chain: stub_resource(), all 8 registered stub implementations, create_remote_class(), and RemoteClassWrapper

Test plan

  • 7 new tests in test_remote_unknown_kwargs.py covering: single unknown kwarg, typo suggestions for each known param, multiple unknowns, no-suggestion for unrelated names, valid kwargs still accepted
  • Updated test_p2_gaps.py to verify unknown kwargs now raise TypeError (previously tested they were silently accepted)
  • All 2455 tests pass, 85.83% coverage
  • make quality-check passes (format, lint, tests, coverage)

Closes AE-2313

@deanq deanq requested a review from Copilot March 25, 2026 22:23
@deanq deanq changed the title fix: reject unknown kwargs in @remote (AE-2313) fix: reject unknown kwargs in @remote Mar 25, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens the deprecated @remote() decorator by rejecting unknown keyword arguments (preventing silent typo bugs that lead to missing dependencies at runtime), and removes the now-dead extra plumbing from the stub/remote-class execution chain.

Changes:

  • Add unknown-kwarg validation for remote() with “Did you mean …?” suggestions.
  • Remove the extra parameter from create_remote_class() and stub_resource() call chains.
  • Add/adjust unit tests to assert unknown kwargs now raise TypeError.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/runpod_flash/client.py Adds _reject_unknown_kwargs() and enforces kwarg validation in remote(); removes forwarding of extra to stubs/classes.
src/runpod_flash/execute_class.py Removes extra from create_remote_class() signature and stops passing it into stub_resource().
src/runpod_flash/stubs/registry.py Removes **extra from stub_resource() and registered stub factories; stops using extra.get("sync", ...).
tests/unit/test_remote_unknown_kwargs.py New unit tests covering unknown kwarg rejection + suggestions.
tests/unit/test_p2_gaps.py Updates regression coverage to expect TypeError for unknown kwargs.
tests/unit/test_stub_registry.py Updates stub registry tests to align with new stub_resource() signature.
tests/unit/test_execute_class.py Updates class-execution tests for the updated create_remote_class() signature.
tests/unit/test_class_caching.py Updates cache-related tests for the updated create_remote_class() signature.
tests/unit/test_regressions.py Updates regression tests for the updated create_remote_class() signature.
tests/unit/test_p1_gaps.py Updates gap tests for the updated create_remote_class() signature.
tests/unit/test_p2_remaining_gaps.py Removes extra={} usage in remaining gaps tests.
tests/integration/test_class_execution_integration.py Updates integration tests for the updated create_remote_class() signature and removes timeout kwarg usage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

deanq added 3 commits March 25, 2026 16:04
**extra in @Remote silently swallowed misspelled parameters like
depndencies=["torch"], causing missing deps at runtime with opaque
import errors. Now raises TypeError with "did you mean?" suggestions
using difflib.get_close_matches.

Also removes dead extra parameter from stub_resource chain and
create_remote_class since no downstream consumer used it.
…-2313)

Remove dead **extra parameter from the full chain: client.py remote(),
stubs/registry.py stub_resource() and all registered implementations,
and execute_class.py create_remote_class/RemoteClassWrapper.

Update all test files to remove trailing extra/{} arguments.
- Derive _REMOTE_KNOWN_KWARGS from inspect.signature(remote) so it
  stays in sync automatically when parameters are added/removed
- Move validation after docstring so remote.__doc__ is preserved
- Add comment explaining why **extra is retained in the signature
@deanq deanq force-pushed the fix/AE-2313-remote-extra-swallows-typos branch from 6f92320 to b7ec056 Compare March 25, 2026 23:06
Pass sorted(known) to difflib.get_close_matches so "did you mean?"
hints are deterministic regardless of set iteration order.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@deanq deanq requested review from KAJdev and runpod-Henrik March 25, 2026 23:44
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.

3 participants