Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughCLI tag-filtering was implemented to select which routers are generated or regenerated based on a comma-separated Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (CLI)
participant CLI as fastapi_code_generator/cli.py
participant FS as File System (output_dir)
participant Templ as Template Renderer
User->>CLI: invoke with --generate-routers --specify-tags "TagA,TagB"
CLI->>FS: read OpenAPI spec (operation tags)
CLI->>CLI: build deduplicated (router, tag) pairs
CLI->>FS: read output_dir/main.py (check for "app.include_router")
CLI->>CLI: parse specified_tags set
alt main.py lacks includes and specified_tags present
CLI->>CLI: filter (router, tag) pairs by specified_tags
alt no matches
CLI-->>User: raise ClickException (exit 1) with requested vs available tags
end
end
CLI->>Templ: render main.py with selected routers/tags
Templ->>FS: write main.py
loop for each selected (router, tag)
CLI->>Templ: render router module for tag
Templ->>FS: write router module file
end
CLI-->>User: exit 0 (success)
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 |
|
📚 Docs Preview: https://pr-558.fastapi-code-generator.pages.dev |
Merging this PR will not alter performance
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #558 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 17 17
Lines 1204 1231 +27
Branches 126 128 +2
=========================================
+ Hits 1204 1231 +27
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:
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
fastapi_code_generator/cli.py (1)
278-283:⚠️ Potential issue | 🟠 MajorRouter/tag pairing can become incorrect due to independent sorting.
At Line 279 and Line 282, router names and tags are sorted separately and then zipped. If normalization changes ordering, routers can be paired with the wrong tags, causing wrong files to be regenerated for selected tags.
Suggested fix
- sorted_tags = sorted(set(all_tags), key=lambda x: x.lower()) - routers = sorted( - [re.sub(TITLE_PATTERN, '_', tag.strip()).lower() for tag in sorted_tags] - ) - router_tag_pairs = list(zip(routers, sorted_tags)) + sorted_tags = sorted(set(all_tags), key=lambda x: x.lower()) + router_tag_pairs: list[tuple[str, str]] = [] + seen_router_to_tag: dict[str, str] = {} + for tag in sorted_tags: + router = re.sub(TITLE_PATTERN, "_", tag.strip()).lower() + prev = seen_router_to_tag.get(router) + if prev is not None and prev != tag: + raise ClickException( + f"Tag name collision after router normalization: {prev!r} and {tag!r} -> {router!r}" + ) + seen_router_to_tag[router] = tag + router_tag_pairs.append((router, tag))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fastapi_code_generator/cli.py` around lines 278 - 283, The current code sorts tags into sorted_tags then independently computes and sorts routers, which can change order and break the router_tag_pairs mapping; instead derive routers directly from sorted_tags in the same order (use the normalized form via TITLE_PATTERN on each tag in sorted_tags) and then zip those routers with sorted_tags to build router_tag_pairs, removing the separate sorting of the router list so the pairing remains correct.
🧹 Nitpick comments (1)
tests/main/test_main.py (1)
684-690: Addapp.include_router(...)assertions in this scenario too.This test currently verifies imports and files but not router inclusion calls. A missing
include_routerregression inmain.pycould slip through here.Suggested assertion additions
main_text = output_dir.joinpath("main.py").read_text(encoding="utf-8") assert "from .routers import fat_cats, wild_boars" in main_text + assert "app.include_router(fat_cats.router)" in main_text + assert "app.include_router(wild_boars.router)" in main_text assert "slim_dogs" not in main_text🤖 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 684 - 690, The test reads main.py into main_text and checks imports and existence of router files but not that those routers are actually mounted; add assertions that main_text includes the include_router calls (e.g., check for "app.include_router(fat_cats.router" and "app.include_router(wild_boars.router" and that it does NOT include "slim_dogs" include_router) so that main.py both imports and calls app.include_router for fat_cats and wild_boars; place these assertions near the existing import/file assertions that reference main_text and the routers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@fastapi_code_generator/cli.py`:
- Around line 294-303: If specified_tags is provided but filtering yields an
empty main_router_tag_pairs, fail fast with a clear error instead of proceeding
to set routers/tags to empty and rendering invalid main.py; detect this after
computing main_router_tag_pairs (the tuple list produced from router_tag_pairs
when specified_tags and not existing_main_has_router_includes) and raise/exit
with a descriptive message referencing the provided specified_tags and that no
routers matched, so the caller can correct the tag list before template
rendering that uses template_vars["routers"] and template_vars["tags"].
---
Outside diff comments:
In `@fastapi_code_generator/cli.py`:
- Around line 278-283: The current code sorts tags into sorted_tags then
independently computes and sorts routers, which can change order and break the
router_tag_pairs mapping; instead derive routers directly from sorted_tags in
the same order (use the normalized form via TITLE_PATTERN on each tag in
sorted_tags) and then zip those routers with sorted_tags to build
router_tag_pairs, removing the separate sorting of the router list so the
pairing remains correct.
---
Nitpick comments:
In `@tests/main/test_main.py`:
- Around line 684-690: The test reads main.py into main_text and checks imports
and existence of router files but not that those routers are actually mounted;
add assertions that main_text includes the include_router calls (e.g., check for
"app.include_router(fat_cats.router" and "app.include_router(wild_boars.router"
and that it does NOT include "slim_dogs" include_router) so that main.py both
imports and calls app.include_router for fat_cats and wild_boars; place these
assertions near the existing import/file assertions that reference main_text and
the routers.
🪄 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: cee315bd-06b3-4f7a-b74a-d57a80ba8187
📒 Files selected for processing (5)
docs/cli-reference.mddocs/llms-full.txtfastapi_code_generator/cli.pyfastapi_code_generator/prompt_data.pytests/main/test_main.py
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/main/test_main.py (1)
643-651: Consider extracting repeated router-assertion checks into a helper.These two blocks are effectively identical; factoring them into a shared assertion helper would reduce maintenance overhead for future message/import/order changes.
Also applies to: 684-692
🤖 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 643 - 651, The test repeats assertions checking router imports/usage and file existence (main_text, app.include_router, routers/fat_cats.py, routers/wild_boars.py, routers/slim_dogs.py, validate_generated_code); extract those checks into a small helper (e.g., assert_routers_present(output_dir, main_text, expected_present, expected_absent)) that asserts the import lines, include_router calls, existence/non-existence of files, and calls validate_generated_code, then replace both identical blocks with calls to this helper to reduce duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@fastapi_code_generator/cli.py`:
- Line 280: The zip call that creates router_tag_pairs uses zip(routers,
sorted_tags) without the strict argument, which Ruff B905 flags; update the call
in the CLI code where router_tag_pairs is created (variable router_tag_pairs,
using inputs routers and sorted_tags) to use zip(..., strict=True) so mismatched
lengths would raise immediately.
---
Nitpick comments:
In `@tests/main/test_main.py`:
- Around line 643-651: The test repeats assertions checking router imports/usage
and file existence (main_text, app.include_router, routers/fat_cats.py,
routers/wild_boars.py, routers/slim_dogs.py, validate_generated_code); extract
those checks into a small helper (e.g., assert_routers_present(output_dir,
main_text, expected_present, expected_absent)) that asserts the import lines,
include_router calls, existence/non-existence of files, and calls
validate_generated_code, then replace both identical blocks with calls to this
helper to reduce duplication.
🪄 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: 9aa00851-6678-4304-95b6-ae00f8fc4925
📒 Files selected for processing (2)
fastapi_code_generator/cli.pytests/main/test_main.py
Breaking Change AnalysisResult: No breaking changes detected Reasoning: The This analysis was performed by repository automation using PR labels and the |
github-actions
Bot
added
the
breaking-change-analyzed
1 participant
Fixes: #488
Summary
--specify-tagsto modular router imports and generated router files for fresh output directories.main.pyalready has router includes.Checks
tox -e py3.14tox -e cli-docs -- --checktox -e llms-txt -- --checktox -e readmetox -e fixtox -e typetox -e config-typestox -e schema-docs -- --checkSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests