Fix 201 response generation - #610
Conversation
|
📚 Docs Preview: https://pr-610.fastapi-code-generator.pages.dev |
|
Warning Review limit reached
More reviews will be available in 36 minutes and 16 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (20)
WalkthroughThe parser now picks a primary successful response from 2xx status codes, sets the route status and response model from that choice, and updates generated fixtures to emit explicit 201 and 202 status codes with matching return annotations. ChangesOpenAPI response selection
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
Merging this PR will not alter performance
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
dfc01ca to
371f3bc
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@fastapi_code_generator/parser.py`:
- Around line 771-783: The _select_route_status_code helper is dropping sole
non-200 success codes when there is no response body, causing the route to fall
back to FastAPI’s default 200. Update this method in parser.py so it preserves a
single unambiguous success code from success_status_codes even when data_types
has no schema for it, while still keeping the existing primary_status_code and
204 handling in place.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 986364f5-93e7-4fa1-844c-49612c54665c
📒 Files selected for processing (4)
fastapi_code_generator/parser.pytests/data/expected/openapi/coverage/callbacks/main.pytests/data/expected/openapi/coverage/callbacks_with_operation_id/main.pytests/data/expected/openapi/coverage/non_200_responses/main.py
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
fastapi_code_generator/parser.py (1)
768-768: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
next(iter(...))over single-element slicing.Ruff (RUF015) flags
list(...)[0]at both locations; it materializes the full list just to take the first element.♻️ Proposed change
- data_type = list(response_data_types.values())[0] + data_type = next(iter(response_data_types.values()))- data_type = list(additional_responses.values())[0] + data_type = next(iter(additional_responses.values()))Also applies to: 814-814
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@fastapi_code_generator/parser.py` at line 768, Replace the eager list indexing in the parser logic with an iterator-based first-element lookup. In the code paths using response_data_types, update the single-element access in the parser function(s) so they use next(iter(...)) instead of materializing list(...)[0], and apply the same change to both flagged locations to satisfy Ruff RUF015 while preserving the existing behavior.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@fastapi_code_generator/parser.py`:
- Around line 749-767: The primary response selection logic in
_select_primary_response_status_code and _get_primary_response_data_type is
using string lookups against data_types even though
parse_responses/_get_success_status_codes work with integer status codes, which
can make primary_status_code resolve to None and fall back to
DataType(type='None') incorrectly. Make the response-status key handling
consistent across these methods by using the same key type everywhere (prefer
integers if that is what parse_responses produces), including the
additional-responses loop, and update the lookups in
_select_primary_response_status_code and _get_primary_response_data_type to
match.
---
Nitpick comments:
In `@fastapi_code_generator/parser.py`:
- Line 768: Replace the eager list indexing in the parser logic with an
iterator-based first-element lookup. In the code paths using
response_data_types, update the single-element access in the parser function(s)
so they use next(iter(...)) instead of materializing list(...)[0], and apply the
same change to both flagged locations to satisfy Ruff RUF015 while preserving
the existing behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 439d32dd-9565-436d-ae89-2f35466b7ae1
📒 Files selected for processing (4)
fastapi_code_generator/parser.pytests/data/expected/openapi/coverage/callbacks/main.pytests/data/expected/openapi/coverage/callbacks_with_operation_id/main.pytests/data/expected/openapi/coverage/non_200_responses/main.py
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/data/expected/openapi/coverage/non_200_responses/main.py
- tests/data/expected/openapi/coverage/callbacks_with_operation_id/main.py
- tests/data/expected/openapi/coverage/callbacks/main.py
42c8ca6 to
3ea19b7
Compare
3ea19b7 to
221e6bf
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
fastapi_code_generator/parser.py (1)
773-790: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winNormalize the primary data lookup for the
200branch.Line 773 can return
'200'fromresponses, butdata_typesis typed to allow either'200'or200. The directdata_types.get(status_code)on Line 789 can therefore miss the parsed schema and generateNoneinstead of the primaryresponse_model.Proposed fix
def _get_primary_response_data_type( self, status_code: ResponseStatusCode | None, data_types: ParsedResponseDataTypes, ) -> DataType: - if status_code is None or not ( - response_data_types := data_types.get(status_code) - ): + parsed_status_code = ( + self._parse_success_status_code(status_code) + if status_code is not None + else None + ) + if parsed_status_code is None or not ( + response_data_types := self._get_response_data_types( + data_types, parsed_status_code + ) + ): return DataType(type='None')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@fastapi_code_generator/parser.py` around lines 773 - 790, The primary response data lookup is inconsistent between the status-code selection and schema lookup paths: `_get_primary_response_status_code_key` can return the string form for 200, while `_get_primary_response_data_type` currently does a direct `data_types.get(status_code)` and may miss the parsed entry. Update the lookup in `_get_primary_response_data_type` (and any related helper logic in `_get_primary_response_status_code_key` / `_find_response_status_code_key`) to normalize the status code key before accessing `data_types`, so both `200` and `'200'` resolve to the same primary response schema and the correct `response_model` is returned.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@fastapi_code_generator/parser.py`:
- Around line 773-790: The primary response data lookup is inconsistent between
the status-code selection and schema lookup paths:
`_get_primary_response_status_code_key` can return the string form for 200,
while `_get_primary_response_data_type` currently does a direct
`data_types.get(status_code)` and may miss the parsed entry. Update the lookup
in `_get_primary_response_data_type` (and any related helper logic in
`_get_primary_response_status_code_key` / `_find_response_status_code_key`) to
normalize the status code key before accessing `data_types`, so both `200` and
`'200'` resolve to the same primary response schema and the correct
`response_model` is returned.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d479ff5c-7d76-4b9d-aa57-ccaeaea8f734
📒 Files selected for processing (19)
fastapi_code_generator/parser.pytests/data/expected/openapi/coverage/callbacks/main.pytests/data/expected/openapi/coverage/callbacks_with_operation_id/main.pytests/data/expected/openapi/coverage/model_options/main.pytests/data/expected/openapi/coverage/non_200_responses/main.pytests/data/expected/openapi/coverage/non_200_status_code/main.pytests/data/expected/openapi/default_template/body_and_parameters/main.pytests/data/expected/openapi/default_template/duplicate_request_param/main.pytests/data/expected/openapi/default_template/same_response_model_for_different_status_codes/main.pytests/data/expected/openapi/default_template/simple/main.pytests/data/expected/openapi/default_template/upload/main.pytests/data/expected/openapi/disable_timestamp/simple/main.pytests/data/expected/openapi/modify_specific_routers/expected/using_routers_example/routers/fat_cats.pytests/data/expected/openapi/modify_specific_routers/expected/using_routers_example/routers/wild_boars.pytests/data/expected/openapi/remote_ref/body_and_parameters/main.pytests/data/expected/openapi/using_routers/using_routers_example/routers/fat_cats.pytests/data/expected/openapi/using_routers/using_routers_example/routers/slim_dogs.pytests/data/expected/openapi/using_routers/using_routers_example/routers/wild_boars.pytests/data/openapi/coverage/non_200_status_code.yaml
✅ Files skipped from review due to trivial changes (6)
- tests/data/expected/openapi/modify_specific_routers/expected/using_routers_example/routers/fat_cats.py
- tests/data/expected/openapi/default_template/same_response_model_for_different_status_codes/main.py
- tests/data/expected/openapi/modify_specific_routers/expected/using_routers_example/routers/wild_boars.py
- tests/data/expected/openapi/default_template/upload/main.py
- tests/data/expected/openapi/default_template/simple/main.py
- tests/data/openapi/coverage/non_200_status_code.yaml
🚧 Files skipped from review as they are similar to previous changes (8)
- tests/data/expected/openapi/using_routers/using_routers_example/routers/wild_boars.py
- tests/data/expected/openapi/using_routers/using_routers_example/routers/slim_dogs.py
- tests/data/expected/openapi/default_template/duplicate_request_param/main.py
- tests/data/expected/openapi/using_routers/using_routers_example/routers/fat_cats.py
- tests/data/expected/openapi/coverage/callbacks/main.py
- tests/data/expected/openapi/coverage/callbacks_with_operation_id/main.py
- tests/data/expected/openapi/coverage/non_200_status_code/main.py
- tests/data/expected/openapi/coverage/non_200_responses/main.py
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #610 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 17 17
Lines 1414 1461 +47
Branches 143 153 +10
=========================================
+ Hits 1414 1461 +47
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
221e6bf to
3a218b5
Compare
github-actions
Bot
added
breaking-change-analyzed
Breaking Change AnalysisResult: Breaking changes detected Reasoning: This PR fundamentally changes how the code generator handles non-200 success responses (201, 202, etc.). Previously, only 200 responses were treated as the "primary" response (setting response_model), and only 204 got an explicit status_code. Now, any non-200 success response can become the primary response with a proper response_model and status_code. This changes the generated FastAPI code output in several ways: (1) new status_code parameters appear in route decorators, (2) response_model changes from None to actual model types for 201 responses, (3) return type annotations change from Optional to non-Optional, and (4) entries move from additional_responses to the primary response_model. These are breaking changes because anyone regenerating their code from the same OpenAPI spec will get different output, and custom templates may behave differently with the changed operation metadata. Content for Release NotesCode Generation Changes
Custom Template Update Required
This analysis was performed by Claude Code Action |
|
🎉 Released in 0.8.0 This PR is now available in the latest release. See the release notes for details. |
2 participants
Fixes: #607
Summary by CodeRabbit
status_codevalues (including201and202) and match handler return annotations to the declared response models, especially for non-200 scenarios.