Conversation
Merging this PR will improve performance by 12.67%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | test_generate_default_template_benchmark |
54.4 ms | 48.3 ms | +12.67% |
Comparing chore/harden-ci-cd (a5728a4) with main (bde5216)
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds Changes
Sequence Diagram(s)sequenceDiagram
participant User as PR / Release Event
participant GH as GitHub Actions (caller)
participant Repo as Repository (checkout)
participant Deploy as Docs Deploy (reusable)
participant CF as Cloudflare Pages
User->>GH: trigger docs workflow (PR / release / dispatch)
GH->>Repo: checkout specific ref / PR head
GH->>Deploy: call reusable docs-deploy with inputs (checkout_ref, deploy_branch)
Deploy->>Repo: checkout inputs.checkout_ref
Deploy->>Repo: install deps and run docs build (zensical build)
Deploy->>CF: if deploy_branch set -> authenticate & deploy site to Pages
Deploy-->>GH: return deploy result
GH-->>User: post PR comment / status update
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)
✏️ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/CODEOWNERS (1)
2-4: Optional: also protect repository-hosted GitHub Actions code.If this repo uses custom actions, consider adding
/.github/actions/**@koxudaxi`` so action logic changes require the same owner review.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/CODEOWNERS around lines 2 - 4, Update the CODEOWNERS entries to also protect repository-hosted GitHub Actions by adding a rule for the actions folder; specifically, add an entry for "/.github/actions/** `@koxudaxi`" in the CODEOWNERS file so changes to custom actions require review by the same owner listed for the other .github resources.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/CODEOWNERS:
- Around line 2-4: Update the CODEOWNERS entries to also protect
repository-hosted GitHub Actions by adding a rule for the actions folder;
specifically, add an entry for "/.github/actions/** `@koxudaxi`" in the CODEOWNERS
file so changes to custom actions require review by the same owner listed for
the other .github resources.
|
📚 Docs Preview: https://pr-538.fastapi-code-generator.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #538 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 16 16
Lines 984 1022 +38
Branches 110 110
=========================================
+ Hits 984 1022 +38
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:
|
There was a problem hiding this comment.
🧹 Nitpick comments (3)
.github/workflows/generated-docs-sync.yml (1)
81-92: DRY the generated-file list.The same seven-path list is hard-coded in both
git diff --quiet --andgit diff --binary --. Drift between the two is an easy foot-gun (one gets updated, the other doesn't). Extract to a shell variable.Proposed diff
- name: Create generated docs patch id: patch env: PATCH_PATH: ${{ runner.temp }}/generated-docs.patch run: | set -euo pipefail - if git diff --quiet -- README.md docs/index.md docs/cli-reference.md docs/llms.txt docs/llms-full.txt docs/supported_formats.md fastapi_code_generator/prompt_data.py; then + paths=( + README.md + docs/index.md + docs/cli-reference.md + docs/llms.txt + docs/llms-full.txt + docs/supported_formats.md + fastapi_code_generator/prompt_data.py + ) + if git diff --quiet -- "${paths[@]}"; then echo "has_changes=false" >> "$GITHUB_OUTPUT" exit 0 fi - git diff --binary -- README.md docs/index.md docs/cli-reference.md docs/llms.txt docs/llms-full.txt docs/supported_formats.md fastapi_code_generator/prompt_data.py > "$PATCH_PATH" + git diff --binary -- "${paths[@]}" > "$PATCH_PATH" echo "has_changes=true" >> "$GITHUB_OUTPUT"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/generated-docs-sync.yml around lines 81 - 92, DRY the repeated file list by defining a shell variable (e.g., FILES) containing the seven paths and use that variable in both git diff commands instead of duplicating the list; update the step that currently references git diff --quiet -- README.md ... and git diff --binary -- README.md ... to use the variable (take care to quote or use an array so paths with spaces are handled), keeping PATCH_PATH and the rest of the logic unchanged and replacing both occurrences so they cannot drift..github/workflows/docs.yml (1)
47-49:issues: writeis redundant alongsidepull-requests: write.The script only calls
issues.listComments/createComment/updateCommentagainst PR numbers. For pull requests, those endpoints are authorized bypull-requests: write;issues: writedoesn't grant additional capability here and slightly widens the token scope.Proposed diff
permissions: - issues: write pull-requests: write🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docs.yml around lines 47 - 49, Remove the redundant "issues: write" permission from the workflow permissions block since the actions only call pull request comment endpoints and are already covered by "pull-requests: write"; update the permissions section to only include "pull-requests: write" (remove the "issues: write" entry) so the token scope is minimal while still allowing create/update/list comments on PRs..github/workflows/changelog.yml (1)
108-118: Optional: patch apply may fail ifmainadvances between jobs.Because
commit-changelogre-checks out a freshmain(line 92) and applies a patch generated in the earlierbuild-changelogjob, any concurrent commit that touchedCHANGELOG.mdwould causegit apply --indexto fail with no fallback. On arelease: publishedtrigger this is extremely rare, but considergit apply --index --3way --whitespace=nowarn(or equivalent) to make this resilient.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/changelog.yml around lines 108 - 118, The git apply step can fail if main has advanced; update the git apply invocation (the `git apply --index --whitespace=nowarn "$PATCH_PATH"` call) to use three-way fallback so it can auto-resolve when main changed (e.g., add the --3way flag), keeping --whitespace=nowarn and --index to preserve the staged patch behavior; ensure the command still errors out on unrecoverable conflicts so the workflow fails rather than silently producing a bad changelog.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/changelog.yml:
- Around line 108-118: The git apply step can fail if main has advanced; update
the git apply invocation (the `git apply --index --whitespace=nowarn
"$PATCH_PATH"` call) to use three-way fallback so it can auto-resolve when main
changed (e.g., add the --3way flag), keeping --whitespace=nowarn and --index to
preserve the staged patch behavior; ensure the command still errors out on
unrecoverable conflicts so the workflow fails rather than silently producing a
bad changelog.
In @.github/workflows/docs.yml:
- Around line 47-49: Remove the redundant "issues: write" permission from the
workflow permissions block since the actions only call pull request comment
endpoints and are already covered by "pull-requests: write"; update the
permissions section to only include "pull-requests: write" (remove the "issues:
write" entry) so the token scope is minimal while still allowing
create/update/list comments on PRs.
In @.github/workflows/generated-docs-sync.yml:
- Around line 81-92: DRY the repeated file list by defining a shell variable
(e.g., FILES) containing the seven paths and use that variable in both git diff
commands instead of duplicating the list; update the step that currently
references git diff --quiet -- README.md ... and git diff --binary -- README.md
... to use the variable (take care to quote or use an array so paths with spaces
are handled), keeping PATCH_PATH and the rest of the logic unchanged and
replacing both occurrences so they cannot drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 37a32846-240d-4707-8882-44f779e4bc8c
📒 Files selected for processing (8)
.github/workflows/changelog.yml.github/workflows/cli-docs.yml.github/workflows/docs-deploy.yml.github/workflows/docs.yml.github/workflows/generated-docs-sync.yml.github/workflows/llms-txt.yml.github/workflows/readme.yml.github/workflows/schema-docs.yml
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)
.github/workflows/cli-docs.yml (1)
43-48:⚠️ Potential issue | 🟠 MajorFail the PR when generated docs are stale.
Lines 43-48 rebuild
docs/cli-reference.mdandfastapi_code_generator/prompt_data.py, but nothing checks whether those generated files changed. Since the write-back job was removed, this workflow can now pass even when the PR forgot to commit regenerated output.Suggested validation step
- name: Build prompt data run: .tox/cli-docs/bin/python scripts/build_prompt_data.py --output fastapi_code_generator/prompt_data.py + - name: Verify generated docs are up to date + run: | + git diff --exit-code -- docs/cli-reference.md fastapi_code_generator/prompt_data.py🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/cli-docs.yml around lines 43 - 48, The workflow currently rebuilds the CLI docs and prompt data in the "Collect CLI doc metadata", "Build CLI docs", and "Build prompt data" steps but does not fail the job if those generated files (docs/cli-reference.md and fastapi_code_generator/prompt_data.py) changed; add a new step (e.g., "Validate generated docs are committed") immediately after the "Build prompt data" step that runs a git check like `git diff --exit-code -- docs/cli-reference.md fastapi_code_generator/prompt_data.py` (or `git status --porcelain` and fail if non-empty) to cause the job to fail when generated outputs are stale so the PR must include regenerated files. Ensure the checkout step fetches sufficient history (fetch-depth: 0) if needed for the git diff to work.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/build_prompt_data.py`:
- Line 11: The import "from black import FileMode, format_str" in
scripts/build_prompt_data.py will raise ImportError in the tox "test"
environment because black is only in the lint group; add black==24.10.0 to the
test dependency group in pyproject.toml (duplicate the exact version currently
used in the lint group) so the CLI/docs tox env can import FileMode and
format_str successfully.
---
Outside diff comments:
In @.github/workflows/cli-docs.yml:
- Around line 43-48: The workflow currently rebuilds the CLI docs and prompt
data in the "Collect CLI doc metadata", "Build CLI docs", and "Build prompt
data" steps but does not fail the job if those generated files
(docs/cli-reference.md and fastapi_code_generator/prompt_data.py) changed; add a
new step (e.g., "Validate generated docs are committed") immediately after the
"Build prompt data" step that runs a git check like `git diff --exit-code --
docs/cli-reference.md fastapi_code_generator/prompt_data.py` (or `git status
--porcelain` and fail if non-empty) to cause the job to fail when generated
outputs are stale so the PR must include regenerated files. Ensure the checkout
step fetches sufficient history (fetch-depth: 0) if needed for the git diff to
work.
🪄 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: a9e7c337-71ee-4ccf-aa9a-64f2dfdf158e
📒 Files selected for processing (6)
.github/CODEOWNERS.github/workflows/cli-docs.yml.github/workflows/generated-docs-sync.yml.github/workflows/readme.ymlscripts/build_prompt_data.pytests/test_prompt_data.py
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/readme.yml
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/docs-deploy.yml:
- Around line 45-50: Add a preflight validation step before the "Deploy" step
that checks inputs.deploy_branch and the Cloudflare secrets and fails fast with
a clear error if deployment was requested but either
secrets.CLOUDFLARE_API_TOKEN or secrets.CLOUDFLARE_ACCOUNT_ID is empty;
specifically, insert a step named like "Validate Cloudflare credentials" that
uses the same if: inputs.deploy_branch != '' condition and runs a short shell
script (or actions/github-script) to test the environment variables ${{
secrets.CLOUDFLARE_API_TOKEN }} and ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} and
exit 1 with a descriptive message when missing, ensuring this validation step
runs before the cloudflare/wrangler-action "Deploy" step.
🪄 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: d3431f5c-6571-45f5-a1a9-853163c71e5f
📒 Files selected for processing (2)
.github/workflows/docs-deploy.ymltests/test_prompt_data.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_prompt_data.py
1 participant
Summary by CodeRabbit