Generate LLMs text during site build - #29
Conversation
📝 WalkthroughWalkthroughThis pull request adds a PHP script that aggregates ChangesContent generation and build automation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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 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 |
Code Review — PR #29: Generate LLMs text during site buildOverviewThis PR moves Positives
Issues1. Unchecked file_put_contents($outputFile, $content);If the write fails (disk full, permission error, etc.) the script exits if (file_put_contents($outputFile, $content) === false) {
fwrite(STDERR, "Failed to write {$outputFile}\n");
exit(1);
}2. "Included files" count is pre-filter (
$included = 0;
foreach ($files as $file) {
// ...
if ($markdown === '') { continue; }
$content .= ...;
$included++;
}
echo 'Included files: ' . $included . "\n";3. If set -e # add at top, after the shebangMinor / Worth NotingSupply-chain pinning in the workflow — Local build not verified — noted in the PR description. The critical path (CI workflow generating files before SummaryThe overall direction is correct and the implementation is clean. The two functional issues (#1 and #2) are worth fixing before merge: a silent write failure could silently ship an incomplete |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bin/serve_local.sh (1)
1-8:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail fast if preprocessing commands fail.
Without strict shell mode, failed generators can still fall through to
jekyll serve.Proposed fix
#!/bin/bash +set -euo pipefail + # This script is used to serve the Jekyll site locally with automatic rebuilding. # 'bundle exec' ensures we're using the correct versions of each gem according to our Gemfile.lock. # 'jekyll serve' starts a Jekyll development server.🤖 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 `@bin/serve_local.sh` around lines 1 - 8, Enable strict-failure behavior in the serve_local.sh script so preprocessing failures stop execution: ensure the script sets strict shell options (e.g., exit on error, undefined var, and failing pipes) before running the preprocessing commands, and/or explicitly check the exit status of the calls to "ruby bin/merge_md_files.rb" and "php bin/generate_llms_full.php" and exit with a non-zero status if either fails so that "bundle exec jekyll serve --watch --trace" does not run when preprocessing fails.
🤖 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 @.github/workflows/jekyll.yml:
- Around line 8-12: The workflow currently grants pages: write and id-token:
write at the top-level permissions which apply to all jobs; move those two
permissions into only the deploy job's permissions block so only the deploy job
gets pages: write and id-token: write while keeping contents: read at top-level.
Locate the top-level permissions block and the deploy job definition (the deploy
job name) and remove pages and id-token from the global permissions, then add
them under the deploy job's permissions section (ensuring pages: write and
id-token: write appear only there).
- Around line 21-23: The checkout step using actions/checkout@v4 currently
leaves default token credentials available; update the "Checkout" step
(actions/checkout@v4) to add persist-credentials: false so the GITHUB_TOKEN is
not persisted to subsequent steps—modify the checkout step configuration to
include the persist-credentials: false property under the uses entry.
In `@bin/generate_llms_full.php`:
- Around line 49-53: The script silently ignores failures from file_get_contents
and file_put_contents which can lead to partial output and false success
messages; update the logic around the $content assembly and the final write so
that each file_get_contents($llmsFile) and file_get_contents($file) call in the
foreach is checked for a false return and handled (log/processLogger->error or
fwrite to STDERR and exit with non-zero) with the filename included, and
likewise check the return value of file_put_contents when writing the final
output (lines referenced around where $content is written) and abort/report on
failure instead of proceeding to print “Generated … successfully.” Ensure the
checks reference the existing variables ($llmsFile, $file, $content) and exit
with a non-zero status on error.
---
Outside diff comments:
In `@bin/serve_local.sh`:
- Around line 1-8: Enable strict-failure behavior in the serve_local.sh script
so preprocessing failures stop execution: ensure the script sets strict shell
options (e.g., exit on error, undefined var, and failing pipes) before running
the preprocessing commands, and/or explicitly check the exit status of the calls
to "ruby bin/merge_md_files.rb" and "php bin/generate_llms_full.php" and exit
with a non-zero status if either fails so that "bundle exec jekyll serve --watch
--trace" does not run when preprocessing fails.
🪄 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: e5efde77-4dc7-4197-8bbc-8810d07a0a5f
📒 Files selected for processing (5)
.github/workflows/jekyll.yml.gitignorebin/generate_llms_full.phpbin/serve_local.shllms-full.txt
💤 Files with no reviewable changes (1)
- llms-full.txt
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/jekyll.yml (1)
21-60:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPin GitHub Actions to full-length commit SHAs.
This workflow references third-party actions via mutable tags (
@v1/@v2/@v3/@v4/@v5) instead of 40-char commit SHAs (e.g.,actions/checkout@v4,ruby/setup-ruby@v1,shivammathur/setup-php@v2,actions/configure-pages@v5,actions/upload-pages-artifact@v3,actions/deploy-pages@v4). Replace eachuses:value with the corresponding commit SHA to reduce supply-chain risk.🤖 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 @.github/workflows/jekyll.yml around lines 21 - 60, Replace mutable action tags with fixed 40-character commit SHAs for every third-party action used in the workflow: actions/checkout@v4, ruby/setup-ruby@v1, shivammathur/setup-php@v2, actions/configure-pages@v5, actions/upload-pages-artifact@v3, and actions/deploy-pages@v4. Locate the steps named "Checkout", "Setup Ruby", "Setup PHP", "Setup Pages", "Upload artifact", and "Deploy to GitHub Pages" and change each uses: entry to the corresponding action@<full-commit-sha> (obtain the exact SHA from the action's official GitHub repo tags/releases) so the workflow references immutable commit SHAs instead of version tags.
🤖 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.
Outside diff comments:
In @.github/workflows/jekyll.yml:
- Around line 21-60: Replace mutable action tags with fixed 40-character commit
SHAs for every third-party action used in the workflow: actions/checkout@v4,
ruby/setup-ruby@v1, shivammathur/setup-php@v2, actions/configure-pages@v5,
actions/upload-pages-artifact@v3, and actions/deploy-pages@v4. Locate the steps
named "Checkout", "Setup Ruby", "Setup PHP", "Setup Pages", "Upload artifact",
and "Deploy to GitHub Pages" and change each uses: entry to the corresponding
action@<full-commit-sha> (obtain the exact SHA from the action's official GitHub
repo tags/releases) so the workflow references immutable commit SHAs instead of
version tags.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ed6d3772-f60a-47fb-9945-42f2dad1f6c1
📒 Files selected for processing (3)
.github/workflows/jekyll.ymlbin/generate_llms_full.phpbin/serve_local.sh
1 participant
Summary
Verification
Note: local Jekyll build was not run successfully because required bundle gems are not installed in this local environment.
Summary by CodeRabbit
Chores
Documentation