fix(types): allow find_in_page actions without url - #3792
fix(types): allow find_in_page actions without url#3792sylvesterkaczmarek wants to merge 8 commits into
Conversation
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e600208635
ℹ️ 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".
|
|
||
|
|
||
| def test_find_in_page_action_allows_missing_url() -> None: | ||
| item = ResponseFunctionWebSearch.model_validate(_find_payload()) |
There was a problem hiding this comment.
When ./scripts/test reaches its unconditional Pydantic 1.10.26 pass, this call raises AttributeError because Pydantic v1 models do not provide model_validate; the beta test repeats the same call on line 25. Use the repository's openai._compat.model_parse helper (or another v1-compatible parsing path) so the new tests pass in both supported Pydantic environments.
Useful? React with 👍 / 👎.
|
|
||
| assert item.action.type == "find_in_page" | ||
| assert item.action.url is None | ||
| assert "url" not in ActionFindParam.__required_keys__ |
There was a problem hiding this comment.
__required_keys__
This assertion does not verify the intended TypedDict change: both parameter modules use postponed annotations, so typing_extensions records Required[str] as a forward reference and __required_keys__ omits url even with the old broken url: Required[str] declaration. The beta assertion has the same false-positive behavior; add a statically checked assignment that omits url, or resolve and inspect the annotation, so reverting the parameter fix would actually fail validation.
Useful? React with 👍 / 👎.
|
|
||
| url: Required[str] | ||
| """The URL of the page searched for the pattern.""" | ||
| url: Optional[str] |
There was a problem hiding this comment.
The reported API discrepancy is that url may be omitted, not that a present url may be null; the checked-in contract still defines it as a non-null string URI in api_reference/openapi.transformed.yml:60837-60841. Because this TypedDict already has total=False, declaring url: str makes the key optional while continuing to reject None; using Optional[str] instead exposes null as a valid request value to type-checked callers even though the API may reject it. The beta parameter type has the same problem.
Useful? React with 👍 / 👎.
1 participant
Summary
Fixes #3788.
The live Responses API can omit
urlfrom completedweb_search_callactions of typefind_in_page, but the generated stable and betafind_in_pageaction types still require it. Re-validating an API-produced item therefore fails even though the payload is valid.This aligns both stable and beta action models and parameter types with the live response shape by making
urloptional, matching the existing optional treatment foropen_page.url.Tests
Adds regression coverage that validates completed stable and beta
find_in_pageactions with nourland verifies the parameter TypedDicts no longer require the key.