fix(core): reject fixed-length tuple annotations for **kwargs variadic arguments - #4742
fix(core): reject fixed-length tuple annotations for **kwargs variadic arguments#4742hktitof wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b23c6eccb7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if not (len(args_of_tuple) == 2 and args_of_tuple[1] is Ellipsis) and hasattr( | ||
| ann, "__args__" | ||
| ): |
There was a problem hiding this comment.
Keep fixed tuples valid as individual keyword values
When strict_json_schema=False, **kwargs: tuple[int, str] is already represented correctly as dict[str, tuple[int, str]]: Pydantic emits an additionalProperties array schema with both positional constraints, validates each JSON array into a tuple, and to_call_args() passes those tuples as the keyword values. Likewise, tuple[()] intentionally requires each value to be an empty tuple. This guard therefore rejects valid, fully representable annotations rather than preventing constraint loss, breaking tool construction for callers using fixed-tuple keyword values; remove this rejection instead of requiring them to change their value type.
AGENTS.md reference: AGENTS.md:L194-L194
Useful? React with 👍 / 👎.
|
Good catch via the P2 — you are right. I verified the pre-change behavior: with strict_json_schema=False the **kwargs tuple annotation is already preserved correctly as dict[str, tuple[int, str]] (prefixItems [int,str], validated + reconstructed fine), so this guard rejects a valid, working annotation rather than fixing any loss. My *args analog in #4735 was a real flatten-to-Any bug; the **kwargs path never had that bug. Closing this as incorrect — thanks for the review. |
Summary
A
**kwargsparameter annotated with a fixed-length tuple silently loses every element constraint.function_schema()maps that annotation todict[str, ann]unchanged, sodef f(**kwargs: tuple[int, str])produces anadditionalPropertiesschema whose value type is the bare tupletuple[int, str].tuple[()]is worse:get_args(Tuple[()])reports no args, so the value schema collapses to{type: array, minItems: 0, maxItems: 0}and forces every keyword value to be the empty array.#4735fixed the symmetric*argspath by rejecting fixed-length tuple annotations during tool construction and naming the supported alternatives. This PR applies the same fail-fast guard to**kwargs, which is the mirror direction of the follow-up requested on #4696:tuple[()]is included in the rejection for the same reason as in #4735: it parameterizes an empty tuple but reports no args, so the check tests whether the annotation is parameterized rather than whetherget_args()is non-empty. Unparameterizedtupleis unchanged, since it carries no element type to preserve or reject. This is a behavior change for callers who previously built such a tool: the annotation used to be accepted and silently widened, and now raisesUserErrorat construction rather than producing a schema that cannot describe the call.The supported forms keep building unchanged:
**kwargs: tuple[int, ...],list[T], a scalar, anddict[str, X].Test plan
test_var_keyword_fixed_length_tuple_annotation_is_rejectedis parametrized overtuple[int, str]andtuple[()], and asserts the message names both supported alternatives.test_var_keyword_supported_annotations_still_buildis parametrized overtuple[int, ...],list[T], and a plain scalar so the alternatives named in the error keep working.Verified the rejection test fails without the source change by stashing
src/agents/function_schema.pyand rerunning: the guard is skipped, so the fixed annotation passes through to a late strict-schemaUserErrorinstead of failing at construction..agents/skills/code-change-verification/scripts/run.shpasses end to end: format, lint, typecheck and the targeted suite.pytest tests/test_function_schema.py tests/test_strict_schema.py tests/test_function_tool.pyis 186 passed.Issue number
None. Mirrors the follow-up requested by a maintainer on #4696 and implemented for
*argsin #4735.Checks
.agents/skills/code-change-verification/scripts/run.sh/reviewbefore submitting this PR