fix(core): preserve variadic Annotated constraints - #4799
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
sylvesterkaczmarek
left a comment
There was a problem hiding this comment.
I traced this through the full function_schema() path against the released v0.22.0 compatibility boundary.
The important part is that the change keeps the existing stripped annotation as the source of truth for parameter classification, including the current fixed-length tuple rejection, but uses the include_extras=True annotation only when constructing the collected list[...] / dict[str, ...] value type. That lets Pydantic apply Annotated/Field constraints to each variadic positional item or keyword value without creating a second interpretation of the signature.
I also checked reconstruction: to_call_args() still only extends the validated *args container and updates from the validated **kwargs mapping, so the schema fix does not alter Python call binding or the existing keyword-collision boundary. The new tests exercise both generated JSON-schema constraints and runtime Pydantic rejection, rather than only inspecting annotations.
I don't see a correctness or compatibility blocker in the current head. GitHub's test workflow is still action_required for this fork head, so this is a source/contract verification rather than an independent CI confirmation.
|
Thanks for tracing the full function_schema path and checking the compatibility boundary. The current head and regression tests cover the behavior you described, so no additional code change is needed from this review. I will leave the PR open for maintainer review. |
|
Follow-up validation: with uv 0.12.9, uv run --locked pytest tests/test_function_schema.py -q passes all 54 tests. The worktree remains clean and no source changes were needed for the review. |
Summary
This pull request fixes
function_schemadropping PydanticAnnotatedmetadata from variadic item types. Constraints on*argsitems and**kwargsvalues now remain in the generated JSON schema and runtime validation, while the existing stripped annotation still controls tuple-shape classification.The implementation reuses Pydantic's nested annotation handling and leaves ordinary parameters, variadic call reconstruction, and strict-schema behavior unchanged.
Test plan
.agents/skills/code-change-verification/scripts/run.shwith the repository CI-pinneduv 0.11.14src/agents/function_schema.pyThe repository wrapper passed format, lint, typecheck, and the full local test suite. Native macOS sandbox-marked tests were skipped locally under the repository's Codex policy and remain covered by the dedicated GitHub macOS job.
Issue number
N/A
Checks
.agents/skills/code-change-verification/scripts/run.sh/reviewbefore submitting this PR