fix(missing_documentation): Def parse in src/openai/resources/chat/completions/complet - #3636
Conversation
…completions/complet Gap ID: openai#249 Co-authored-by: Hermes Agent <hermes-agent@nousresearch.com>
|
Automated contribution summary (Hermes Agent / altitude-upstream)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a5910404c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| [model guide](https://platform.openai.com/docs/models) to browse available | ||
| models. | ||
|
|
||
| response_format: A Pydantic model class or other type that the response content should be |
There was a problem hiding this comment.
Restrict response-format guidance to supported types
When a caller interprets “other type” as an ordinary class or TypedDict, _type_to_response_format() raises TypeError before sending the request; it accepts only pydantic.BaseModel subclasses and Pydantic dataclass-like types, with the latter requiring Pydantic v2. Name those supported alternatives instead of implying that an arbitrary type can be parsed.
Useful? React with 👍 / 👎.
| deserialises the returned JSON into an instance of that type. Omit this | ||
| argument (or pass `openai.NOT_GIVEN`) to receive a plain | ||
| `ParsedChatCompletion` without automatic deserialisation. |
There was a problem hiding this comment.
Recommend a sentinel accepted by the public signature
When a typed caller follows this advice, response_format=openai.NOT_GIVEN is rejected because NOT_GIVEN is a NotGiven, while the public parameter accepts only type[ResponseFormatT] | Omit. Recommend omitting the argument or passing openai.omit, unless NotGiven is deliberately added to the public signature.
Useful? React with 👍 / 👎.
| reasoning_effort: Constrains effort on reasoning for reasoning models. Supported values are | ||
| `low`, `medium`, and `high`. |
There was a problem hiding this comment.
Document all supported reasoning-effort values
When users need the newer reasoning modes, this incorrectly limits the supported values to low, medium, and high. The ReasoningEffort type used by this method and the canonical create() documentation also support none, minimal, xhigh, and max subject to model-specific availability, so this documentation hides valid options.
Useful? React with 👍 / 👎.
|
|
||
| service_tier: Specifies the processing type used for serving the request. | ||
|
|
||
| stop: Up to 4 sequences where the API will stop generating further tokens. |
There was a problem hiding this comment.
Qualify stop-sequence support for reasoning models
When parse() is used with o3 or o4-mini, this unqualified statement suggests that stop is supported, while the same endpoint's generated create() documentation explicitly says it is unsupported for those models. Carry that qualification into the helper documentation so callers do not construct unsupported requests.
Useful? React with 👍 / 👎.
| tools: A list of tools the model may call. Pass tool schemas created with | ||
| `openai.pydantic_function_tool()` to enable automatic parsing of tool-call | ||
| arguments into the corresponding Pydantic model instances. |
There was a problem hiding this comment.
State that parse rejects unsupported tool schemas
When a caller supplies a custom tool or an ordinary non-strict function tool, _validate_input_tools() raises ValueError before the request; these are not merely left unparsed. This generic description should state that parse() accepts only strict function tools, including those produced by pydantic_function_tool().
Useful? React with 👍 / 👎.
| top_logprobs: An integer between 0 and 20 specifying the number of most likely tokens to | ||
| return at each token position. `logprobs` must be `true` when used. |
There was a problem hiding this comment.
Describe top_logprobs as an upper bound
When consumers use this value to size or index the returned alternatives, the API may return fewer entries than requested. The canonical endpoint documentation defines top_logprobs as the maximum number of likely tokens and explicitly allows fewer results, so saying it specifies “the number” incorrectly implies an exact count.
Useful? React with 👍 / 👎.
| A `ParsedChatCompletion[ResponseFormatT]` object. When `response_format` is | ||
| provided the `.choices[i].message.parsed` attribute contains the deserialised | ||
| response as an instance of `ResponseFormatT`. Tool calls made with |
There was a problem hiding this comment.
Preserve the optionality of message.parsed
When the model refuses the request or returns no content, maybe_parse_content() leaves message.parsed as None even though response_format was provided. This return documentation promises an instance unconditionally, which can lead callers to dereference parsed without the guard already shown in the example; qualify the statement for non-refusal responses with parseable content.
Useful? React with 👍 / 👎.
| max_tokens: The maximum number of tokens that can be generated in the chat completion. | ||
| Deprecated in favour of `max_completion_tokens`. |
There was a problem hiding this comment.
Warn that max_tokens is incompatible with o-series models
When a caller uses an o-series model such as the documented o3 example, this description does not disclose that max_tokens is incompatible with those models. The canonical create() documentation states that restriction explicitly, so direct these callers to max_completion_tokens rather than merely labeling the old parameter deprecated.
Useful? React with 👍 / 👎.
|
|
||
| top_p: Nucleus sampling probability mass. | ||
|
|
||
| user: A stable identifier for your end-users. |
There was a problem hiding this comment.
Direct user callers to its replacement fields
When a new caller uses user for cache bucketing or abuse detection based on this description, they miss that the field is being replaced. The canonical endpoint documentation directs cache use to prompt_cache_key and abuse-detection identity to safety_identifier; include that transition here so the new helper documentation does not steer integrations toward the legacy field.
Useful? React with 👍 / 👎.
TLDR
Gap: Def
parsein src/openai/resources/chat/completions/completions.py has no docstring — add parameter docs, return type, and exampleWedge type:
missing_documentationIssue: https://github.com/openai/openai-python/blob/main/src/openai/resources/chat/completions/completions.py#L91
Changes
src/openai/resources/chat/completions/completions.pyDiff size: 170 lines across 1 file(s)
Pre-submission checklist
AI Assistance Disclosure
This contribution was AI-assisted using Hermes Agent (Nous Research).
Co-authored-by: Hermes Agent hermes-agent@nousresearch.com