Fix OpenAPI marker and model schema generation - #179
Conversation
![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: 9c8d33cea9
ℹ️ 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".
| if multipart_body_items: | ||
| content["multipart/form-data"] = { | ||
| "schema": _build_object_body_schema(multipart_body_items) | ||
| } |
There was a problem hiding this comment.
When an endpoint mixes File() and Form() parameters, the runtime accepts the form fields and file parts in the same multipart request, but this schema generation splits the form fields into application/x-www-form-urlencoded and builds multipart/form-data from only multipart_body_items. For routes such as def upload(file: UploadFile = File(), description: str = Form()), clients generated from the OpenAPI document will omit description from the multipart payload and the docs also fail to mark that multipart field as required.
Useful? React with 👍 / 👎.
| for item in metadata: | ||
| if isinstance(item, _MARKER_TYPES): | ||
| return item |
There was a problem hiding this comment.
Returning markers from Annotated metadata makes the OpenAPI document advertise Annotated[..., Form()]/Annotated[..., File()] parameters as form or multipart fields, but the request parsing paths inspected in create_enhanced_handler and the test client only enable form/file extraction from param.default. In those Annotated-only cases the runtime will not populate the handler argument, so generated docs and clients point users at requests that fail with missing required arguments.
Useful? React with 👍 / 👎.
| name = annotation.__name__ | ||
| if name in components: | ||
| return name |
There was a problem hiding this comment.
If two routes use different model classes with the same __name__ (for example Request or Response models from separate modules), the second registration returns the existing component and its operation $ref points at the first model's schema. That produces incorrect request/response documentation for one of the routes; the component key needs to account for the model identity or otherwise generate a unique name.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2359ddca4c
ℹ️ 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".
| else: | ||
| schema = {"title": name, "type": "object"} | ||
|
|
||
| components[name] = _json_safe_schema(schema) |
There was a problem hiding this comment.
When a DHI/Pydantic model contains nested models, model_json_schema() commonly emits local definitions and refs such as #/$defs/Child. Storing that raw schema under components.schemas.<Name> leaves those refs pointing at the OpenAPI document root, where this generator never creates $defs, so nested request or response models get dangling references even though the top-level component exists. Use a components ref template or hoist nested definitions when registering the schema.
Useful? React with 👍 / 👎.
| parameters.append( | ||
| _build_parameter(name, "header", annotation, components, param, marker) | ||
| ) | ||
| elif isinstance(marker, Cookie): |
There was a problem hiding this comment.
For a handler like def read(session: str = Cookie()), this now advertises a real OpenAPI cookie parameter, but the request path inspected in request_handler.py never parses the Cookie header into Cookie() parameters; HeaderParser only matches headers by parameter/header name, and cookie parsing is limited to APIKeyCookie/security helpers. Clients generated from this schema will send a cookie while the handler still receives the Cookie() marker/default rather than the cookie value.
Useful? React with 👍 / 👎.
| if marker is not None: | ||
| return getattr(marker, "default", ...) is ... |
There was a problem hiding this comment.
For def list_items(limit: int = Query(default=100)), this marks the query parameter optional, but the runtime query parser only fills values that are present in the query string and never unwraps Query.default when the client omits the parameter. A client following the new schema can omit limit, leaving the handler's Python default as the Query(...) marker object instead of 100, so the documented request shape does not match runtime behavior.
Useful? React with 👍 / 👎.
2 participants
Summary
Validation
Note: full pytest collection is currently blocked by an unrelated auth-dependent vendored frontend test under frontend/zig-pkg/.../tools/test_rag.py returning HTTP 401.