Skip to content

fix(core): preserve fixed tuple annotations for variadic tool arguments - #4696

Closed
rioyu123 wants to merge 2 commits into
openai:mainfrom
rioyu123:codex/fix-fixed-tuple-varargs
Closed

fix(core): preserve fixed tuple annotations for variadic tool arguments#4696
rioyu123 wants to merge 2 commits into
openai:mainfrom
rioyu123:codex/fix-fixed-tuple-varargs

Conversation

@rioyu123

@rioyu123 rioyu123 commented Aug 26, 2026

Copy link
Copy Markdown

Summary

This pull request fixes schema generation for fixed-length tuple annotations used on variadic tool parameters.

A signature such as *args: tuple[int, str] currently degrades to list[Any], so the generated JSON Schema accepts invalid arity and element types. This change preserves any parameterized tuple as the type of each positional argument while retaining the existing permissive behavior for bare and empty tuple annotations, including the Python 3.10 typing.Tuple[()] representation.

Fixed tuples are represented with prefixItems. Strict schema conversion now traverses those entries, closing nested object schemas and rejecting free-form mappings during tool construction instead of sending a non-strict schema to the provider.

Regression coverage verifies schema constraints, nested strictness, invalid open mappings, validation failures, and reconstruction of validated positional arguments into tuples.

Test plan

  • uv run pytest -q tests/test_function_schema.py tests/test_strict_schema.py (113 passed)
  • Repository verification script on Linux / Python 3.12: format, Ruff, Mypy, Pyright, and the full test target all passed

Issue number

N/A

Checks

  • I've added new tests, if relevant
  • I've run .agents/skills/code-change-verification/scripts/run.sh
  • I've confirmed all verification steps pass

@rioyu123

Copy link
Copy Markdown
Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ecd123b581

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/agents/function_schema.py
@chatgpt-codex-connector

Copy link
Copy Markdown

Security review completed. No security issues were found in this pull request.

Reviewed commit: ecd123b581

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@rioyu123
rioyu123 force-pushed the codex/fix-fixed-tuple-varargs branch from ecd123b to 2771088 Compare August 26, 2026 16:58
@rioyu123

Copy link
Copy Markdown
Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Security review completed. No security issues were found in this pull request.

Reviewed commit: 2771088610

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@rioyu123

Copy link
Copy Markdown
Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Security review completed. No security issues were found in this pull request.

Reviewed commit: 82f0a64084

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@seratch seratch left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The Python annotation analysis is correct, but the proposed provider schema relies on prefixItems. OpenAI Structured Outputs supports only a documented JSON Schema subset, and prefixItems is not listed among the supported array properties. Unsupported strict schemas are rejected by the API.

Because function tools normally use strict schemas, we should not make fixed-length variadic tuples generate prefixItems without authoritative evidence that this is accepted across the supported OpenAI tool paths and models.

Please narrow this to an actionable construction-time error for fixed-length tuple annotations, while preserving the existing homogeneous tuple[T, ...] behavior. That keeps the unreleased #4655 correction without adding a provider contract we cannot currently substantiate.

@seratch

seratch commented Aug 27, 2026

Copy link
Copy Markdown
Member

Checked this PR again. I am closing this because emitting prefixItems is not a supported direction for OpenAI Structured Outputs. The documented supported array constraints do not include prefixItems, so this change would turn a Python annotation into a provider-invalid strict schema.

The narrower SDK behavior should be to reject fixed-length tuple annotations during tool construction and identify tuple[T, ...] or list[T] as the supported alternative. A focused follow-up PR implementing that fail-fast behavior would be welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants