Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe router template was modified to use Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
📚 Docs Preview: https://pr-554.fastapi-code-generator.pages.dev |
Merging this PR will degrade performance by 16.25%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | test_generate_default_template_benchmark |
43.3 ms | 51.6 ms | -16.25% |
Comparing issue-487-router-path (8537ab0) with main (67bb774)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/main/test_main.py (1)
410-449: LGTM — targeted regression test for the path-parameter-preservation fix.The two assertions cleanly pin down both halves of the fix (preserved URL template and the alias-based binding), and
validate_generated_codeensures the generated module compiles.Optional follow-up (non-blocking): consider also asserting that the
main.pywires up the new router (e.g.,from .routers import items), mirroringtest_generate_router_name_from_hyphenated_tagon lines 403–406. That would catch regressions in router registration alongside the path-rendering behavior in the same test.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/main/test_main.py` around lines 410 - 449, Add an assertion in test_generate_router_preserves_path_parameter_name to also verify the generated main app wires up the new router: after reading router_text, read output_dir.joinpath("main.py") and assert it contains the router import/registration (e.g., "from .routers import items" or the equivalent router inclusion) so the test catches regressions where the router is generated but not registered; keep the existing assertions and call validate_generated_code as before.tests/data/modular_template/routers.jinja2 (1)
13-13: Mirror template kept in sync — LGTM.This is intentionally a duplicate of
fastapi_code_generator/modular_template/routers.jinja2used as a--template-dirin tests. Pre-existing duplication; updating both in lockstep is correct. As a future cleanup, the test could point--template-dirat the package's built-in template directory (BUILTIN_MODULAR_TEMPLATE_DIR) to avoid drift, but that's out of scope here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/data/modular_template/routers.jinja2` at line 13, This duplicate template (tests/data/modular_template/routers.jinja2) intentionally mirrors the built-in template using symbols like operation.type, operation.path, and operation.response; leave it unchanged to keep tests passing and keep it in lockstep with fastapi_code_generator/modular_template/routers.jinja2, or as a future cleanup update the test harness to point at BUILTIN_MODULAR_TEMPLATE_DIR instead of maintaining this duplicate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/data/modular_template/routers.jinja2`:
- Line 13: This duplicate template (tests/data/modular_template/routers.jinja2)
intentionally mirrors the built-in template using symbols like operation.type,
operation.path, and operation.response; leave it unchanged to keep tests passing
and keep it in lockstep with
fastapi_code_generator/modular_template/routers.jinja2, or as a future cleanup
update the test harness to point at BUILTIN_MODULAR_TEMPLATE_DIR instead of
maintaining this duplicate.
In `@tests/main/test_main.py`:
- Around line 410-449: Add an assertion in
test_generate_router_preserves_path_parameter_name to also verify the generated
main app wires up the new router: after reading router_text, read
output_dir.joinpath("main.py") and assert it contains the router
import/registration (e.g., "from .routers import items" or the equivalent router
inclusion) so the test catches regressions where the router is generated but not
registered; keep the existing assertions and call validate_generated_code as
before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d752f732-8398-4a17-b289-8422bc6cc578
📒 Files selected for processing (8)
fastapi_code_generator/modular_template/routers.jinja2tests/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/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/modular_template/routers.jinja2tests/main/test_main.py
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #554 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 16 16
Lines 1064 1074 +10
Branches 110 110
=========================================
+ Hits 1064 1074 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
github-actions
Bot
added
the
breaking-change-analyzed
Breaking Change AnalysisResult: No breaking changes detected Reasoning: The This analysis was performed by repository automation using PR labels and the |
1 participant
Fixes: #487
Summary by CodeRabbit
Bug Fixes
Tests