GeezerVison

This comment shows the latest Codex review activity on this pull request.

Argument binding now reports the first missing required positional-or-keyword parameter before converting supplied arguments. A regression test checks missing-argument errors and confirms that conversion still runs when all required arguments are supplied.

Changes: Argument validation

Layer / File(s)	Summary
Scan parameter metadata crates/vm/src/function/argument.rs	A new helper scans parameter metadata, recurses through known flattened parameters, and identifies a missing required positional-or-keyword parameter. It skips defaults and supplied arguments, and stops at unknown flattened or variadic positional parameters.
Check before conversion crates/vm/src/function/argument.rs, extra_tests/snippets/vm_argument_errors.py	bind_for returns the missing-argument error before calling FromArgs::from_args. The regression test checks missing arguments to itertools.compress and confirms that conversion raises when both arguments are supplied.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Possibly related PRs

• RustPython/RustPython#9513: Adds an early missing-argument check in FuncArgs::bind_for and a test for itertools.compress.
• RustPython/RustPython#9563: Changes argument binding validation in argument.rs to detect missing parameters and other binding errors before conversions.

Suggested reviewers: youknowone, shaharnaveh

Merge Risk: Minimal · up to 5311f

The change checks missing required arguments before conversion. Additional flattened-parameter testing would improve regression protection, but no concrete issue currently blocks merging after normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title states the main change: missing required arguments are now reported before any argument is converted. The raw summary confirms FuncArgs::bind_for checks for the first missing required positi…
Linked Issues check		The only direct link is #9506, which is closed and completed. Per the assessment rules, it supplies background only. No active linked-issue coding requirements apply. For context, the change matches …
Out of Scope Changes check		The whole-PR diff against the merge base 151813d touches two files. 1. crates/vm/src/function/argument.rs (+51/-1): adds the ControlFlow import, a missing-argument check in bind_for, and the helper …

Full details: Title check

Explanation

The title states the main change: missing required arguments are now reported before any argument is converted. The raw summary confirms FuncArgs::bind_for checks for the first missing required positional-or-keyword parameter before calling FromArgs::from_args, and the added test checks itertools.compress with a RaisingIter. The title is concise, specific, and matches the PR objectives, so it passes.

Full details: Linked Issues check

Explanation

The only direct link is #9506, which is closed and completed. Per the assessment rules, it supplies background only. No active linked-issue coding requirements apply.

For context, the change matches the behavior described in #9506. In crates/vm/src/function/argument.rs, FuncArgs::bind_for now calls first_missing_required before T::from_args. If a required positional-or-keyword parameter is neither filled by position nor supplied by keyword, the missing-argument TypeError is returned first. Therefore the conversion of an earlier argument (for example, iter in itertools.compress(x)) no longer runs. The check uses Param.default.is_none() as the required marker, which matches count_param and arity in signature.rs. The new test in extra_tests/snippets/vm_argument_errors.py covers compress(RaisingIter()), compress(data=RaisingIter()), and compress(selectors=RaisingIter()), and also checks that a full call still raises RuntimeError.

No test results were re-run in this assessment. The PR description reports test runs that were not independently verified here.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff against the merge base 151813d touches two files.

1. crates/vm/src/function/argument.rs (+51/-1): adds the ControlFlow import, a missing-argument check in bind_for, and the helper first_missing_required. All of these serve the stated objective of reporting a missing required argument before conversion. The helper is a traversal of Param metadata that handles Flatten, VarPositional, and keyword-supplied parameters. It does not alter other binding paths.
2. extra_tests/snippets/vm_argument_errors.py (+20/-0): adds one regression test and one invocation call. This test supports the change.

No unrelated change was found.

One behavior point was noted but not demonstrated as a defect. The check skips positional-only parameters. When a positional-only parameter and a later positional-or-keyword parameter are both missing, the new check reports the positional-or-keyword name. The base code reported the earlier binding error from from_args. The PR description states that positional-only parameters are intentionally left to the type. Whether this mixed-case message matches CPython was not verified in this assessment.

Full details: Docstring Coverage

Explanation

Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 2 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Generate unit tests (beta)

• Create a new PR

• X
• Mastodon
• Reddit
• LinkedIn