Fix type safety in OpenAIConversationsSession.get_items() #1901
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
**What This PR Fixes** - Resolves type mismatch where `get_items()` returned `list[dict]` but was annotated as `list[TResponseInputItem]` - Removes unnecessary `# type: ignore` comments that masked real type safety issues - Adds explicit non-None assertion for client initialization in `start_openai_conversations_session` **Changes Made** - Added `from typing import cast` import to support explicit type casting - Typed the `all_items` accumulator as `list[TResponseInputItem]` to match method signature - Cast `item.model_dump(exclude_unset=True)` results to `TResponseInputItem` in both iteration branches - Removed `# type: ignore` on `get_items()` return statement since type now matches annotation - Removed `# type: ignore [typeddict-item]` in `pop_item()` since items are now correctly typed - Added explicit `assert _maybe_openai_client is not None` in `start_openai_conversations_session` to document invariant **Why This Matters** - Enables proper static type checking with mypy and other type checkers - Prevents potential runtime errors when downstream code expects proper `TResponseInputItem` objects - Makes type contracts explicit and verifiable - Improves code maintainability without changing runtime behavior **Backward Compatibility** - No changes to public APIs or method signatures - No changes to pagination, ordering, or session management behavior - All existing functionality preserved **Testing** - Existing test suite validates unchanged behavior - Type checking now passes without suppressions
Added type ignore comment for item_id assignment.
|
Thanks for sending this, but I am not sure if having lots of |
…ior unchanged. - Replace multiple per-item casts in get_items by accumulating plain dicts and applying a single cast at the return so the function matches its annotated return type while keeping runtime behavior identical. - Retain a focused type ignore for item_id in pop_item because the TypedDict union does not guarantee an id key even though the API does, avoiding broader casts or schema changes in this small patch. - Preserve ordering, pagination, and session behavior; no public API changes, no control-flow changes, and no added dependencies, making the change safe and easy to review.
|
This PR is stale because it has been open for 10 days with no activity. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What This PR Fixes
get_items()returnedlist[dict]but was annotated aslist[TResponseInputItem]# type: ignorecomments that masked real type safety issuesstart_openai_conversations_sessionChanges Made
from typing import castimport to support explicit type castingall_itemsaccumulator aslist[TResponseInputItem]to match method signatureitem.model_dump(exclude_unset=True)results toTResponseInputItemin both iteration branches# type: ignoreonget_items()return statement since type now matches annotation# type: ignore [typeddict-item]inpop_item()since items are now correctly typedassert _maybe_openai_client is not Noneinstart_openai_conversations_sessionto document invariantWhy This Matters
TResponseInputItemobjectsBackward Compatibility
Testing