feat: support multiple response types in OpenAPI documentation#1373
feat: support multiple response types in OpenAPI documentation#1373
Conversation
Route decorators now accept an `openapi_responses` parameter for
documenting additional response codes in the OpenAPI spec:
@app.get("/users/:id", openapi_responses={
404: {"description": "User not found"},
422: {"description": "Validation error"},
})
Closes #1257
Made-with: Cursor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe changes extend Robyn's route registration and OpenAPI spec generation to support per-route custom response definitions. A new optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
robyn/router.py (1)
345-355:⚠️ Potential issue | 🟡 MinorFreeze
openapi_responsesat registration time.Line 345 and Line 355 store the caller’s dict by reference. If that dict is mutated later, route metadata (and eventual OpenAPI output) can change unexpectedly.
Proposed fix
+import copy ... - if inspect.iscoroutinefunction(handler): + frozen_openapi_responses = copy.deepcopy(openapi_responses) if openapi_responses is not None else None + + if inspect.iscoroutinefunction(handler): function = FunctionInfo( async_inner_handler, True, len(params), params, new_injected_dependencies, ) - self.routes.append(Route(route_type, endpoint, function, is_const, auth_required, openapi_name, openapi_tags, openapi_responses)) + self.routes.append( + Route(route_type, endpoint, function, is_const, auth_required, openapi_name, openapi_tags, frozen_openapi_responses) + ) return async_inner_handler else: function = FunctionInfo( inner_handler, False, len(params), params, new_injected_dependencies, ) - self.routes.append(Route(route_type, endpoint, function, is_const, auth_required, openapi_name, openapi_tags, openapi_responses)) + self.routes.append( + Route(route_type, endpoint, function, is_const, auth_required, openapi_name, openapi_tags, frozen_openapi_responses) + ) return inner_handler🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@robyn/router.py` around lines 345 - 355, The route registration stores the caller's openapi_responses dict by reference (see openapi_responses passed into Route in both branches), so later mutations affect route metadata; fix by freezing/copying it at registration time—replace the direct openapi_responses argument in self.routes.append(Route(..., openapi_responses)) with a copy (e.g., openapi_responses.copy() or copy.deepcopy(openapi_responses) if nested structures) in both the async and non-async branches where Route is constructed (symbols: openapi_responses, Route, self.routes.append, FunctionInfo, inner_handler).
🧹 Nitpick comments (1)
unit_tests/test_openapi_multiple_responses.py (1)
4-30: Add a spec-level assertion test for response merging.These tests validate route storage, but they don’t verify that
openapi_responsesis actually merged into the generated OpenAPI operation (responses). Adding one spec-level test would cover the feature’s user-facing behavior.Proposed test addition
+def test_openapi_responses_are_merged_into_generated_spec(): + app = Robyn(__file__) + + `@app.get`( + "/items/:id", + openapi_name="Get item", + openapi_responses={ + 404: {"description": "Not found"}, + 422: "Validation error", + }, + ) + async def get_item(): + return {"id": 1} + + app.router.prepare_routes_openapi(app.openapi, app.included_routers) + spec = app.openapi.get_openapi_config() + op = spec["paths"]["/items/{id}"]["get"] + + assert "200" in op["responses"] + assert op["responses"]["404"]["description"] == "Not found" + assert op["responses"]["422"]["description"] == "Validation error"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@unit_tests/test_openapi_multiple_responses.py` around lines 4 - 30, Add a spec-level test (e.g., test_openapi_responses_merged_into_spec) that registers a route on Robyn with openapi_responses (reuse the "/items/:id" handler from the diff), generate the OpenAPI spec from the app (call the app's OpenAPI/spec generation method such as app.openapi() or the project-specific get_openapi_spec helper), and assert that the resulting operation for the route's path includes a "responses" mapping that contains the 404 and 422 entries from openapi_responses; ensure you reference the same path and method used in the route registration and verify the merged entries exist in the spec-level operation responses.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@robyn/__init__.py`:
- Around line 792-890: SubRouter is missing an override for connect so CONNECT
routes bypass the prefix; add a connect method on the same class that mirrors
the other HTTP decorators: define def connect(self, endpoint: str,
auth_required: bool = False, openapi_name: str = "", openapi_tags: List[str] =
["connect"], openapi_responses: Optional[dict] = None) and call return
super().connect(endpoint=self.__add_prefix(endpoint),
auth_required=auth_required, openapi_name=openapi_name,
openapi_tags=openapi_tags, openapi_responses=openapi_responses); use the
existing __add_prefix helper and the same parameter names as the other overrides
to keep behavior consistent with BaseRobyn.connect.
---
Outside diff comments:
In `@robyn/router.py`:
- Around line 345-355: The route registration stores the caller's
openapi_responses dict by reference (see openapi_responses passed into Route in
both branches), so later mutations affect route metadata; fix by
freezing/copying it at registration time—replace the direct openapi_responses
argument in self.routes.append(Route(..., openapi_responses)) with a copy (e.g.,
openapi_responses.copy() or copy.deepcopy(openapi_responses) if nested
structures) in both the async and non-async branches where Route is constructed
(symbols: openapi_responses, Route, self.routes.append, FunctionInfo,
inner_handler).
---
Nitpick comments:
In `@unit_tests/test_openapi_multiple_responses.py`:
- Around line 4-30: Add a spec-level test (e.g.,
test_openapi_responses_merged_into_spec) that registers a route on Robyn with
openapi_responses (reuse the "/items/:id" handler from the diff), generate the
OpenAPI spec from the app (call the app's OpenAPI/spec generation method such as
app.openapi() or the project-specific get_openapi_spec helper), and assert that
the resulting operation for the route's path includes a "responses" mapping that
contains the 404 and 422 entries from openapi_responses; ensure you reference
the same path and method used in the route registration and verify the merged
entries exist in the spec-level operation responses.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 66a80765-dc1c-4388-ada3-4d414ce86bf8
📒 Files selected for processing (4)
robyn/__init__.pyrobyn/openapi.pyrobyn/router.pyunit_tests/test_openapi_multiple_responses.py
Summary
openapi_responsesparameter to all route decorators (@app.get,@app.post, etc.) andSubRoutermethods, allowing users to document additional HTTP response codes in the generated OpenAPI spec.openapi_responsesis correctly stored on routes and defaults toNone.Usage
Test plan
test_openapi_responses_registered— verifies responses dict is stored on the routetest_openapi_responses_default_none— verifies routes without the parameter default toNoneCloses #1257
Made with Cursor
Summary by CodeRabbit