Skip to content

fix: handle bare dict type annotations - #3376

Closed
he-yufeng wants to merge 2 commits into
openai:mainfrom
he-yufeng:fix/bare-dict-type-handling
Closed

fix: handle bare dict type annotations#3376
he-yufeng wants to merge 2 commits into
openai:mainfrom
he-yufeng:fix/bare-dict-type-handling

Conversation

@he-yufeng

Copy link
Copy Markdown
Contributor

Summary

  • avoid indexing get_args() when request transform sees a bare dict annotation
  • avoid unpacking empty get_args() in construct_type(..., type_=dict)
  • add regressions for sync/async transform and model construction with unparameterized dict

Fixes #3338.
Fixes #3341.

Tested

  • Reproduced both failures locally before the patch with PYTHONPATH=src.
  • python -m pytest tests\test_transform.py::test_bare_dictionary_annotation tests\test_transform.py::test_dictionary_items tests\test_models.py::test_construct_bare_dict_type -q
  • python -m pytest tests\test_transform.py tests\test_models.py -q
  • python -m py_compile src\openai\_utils\_transform.py src\openai\_models.py tests\test_transform.py tests\test_models.py
  • python -m ruff check src\openai\_utils\_transform.py src\openai\_models.py tests\test_transform.py tests\test_models.py
  • python -m ruff format --check src\openai\_utils\_transform.py src\openai\_models.py tests\test_transform.py tests\test_models.py
  • git diff --check

@he-yufeng
he-yufeng requested a review from a team as a code owner June 7, 2026 16:00
@he-yufeng

Copy link
Copy Markdown
Contributor Author

Bump. _models.py has not moved since this was opened, and a bare dict annotation still lands in the generated request types as an opaque mapping instead of an open properties bag, which is what the server accepts. The patch keeps the literal cases untouched and only widens the bare form, with the regression covering both.

@jbeckwith-oai jbeckwith-oai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

tests/test_transform.py:401 causes the repository's strict Pyright check to fail with reportMissingTypeArgument (Expected type arguments for generic class "dict"). Because the bare dict annotation is intentional for this regression test, please add a narrow inline suppression for that diagnostic so the runtime coverage remains intact without breaking CI.

Validation at 5cb7ba509e79ed11de6deb28d760f9f122ae221d: 117 focused tests passed; Ruff lint and format checks passed; mypy passed; git diff --check passed; targeted Pyright failed only on the line above.

Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
@he-yufeng

Copy link
Copy Markdown
Contributor Author

Done — the bare dict now carries a narrow pyright: ignore[reportMissingTypeArgument] with a note on why it is intentional. The regression test itself still runs (2 passed locally); the other pyright noise in my sandbox is the repo's strict unknown-type surface, unrelated to this change.

Copy link
Copy Markdown
Contributor

Thank you for the contribution. The missing type arguments for bare dict annotations are now handled in model construction and sync/async request transformation by #3760, which is merged into main.

Closing this PR because the merged change resolves the same crash. Please use a release containing #3760 when available; the fix is not included in v3.11.0.

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

Labels

None yet

Projects

None yet

4 participants