fix CDN build#244
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughRelease v3.4.1 bumps package versions, updates the llama.cpp submodule and propagates the new build version through CDN URLs and generated constants. A new package size validation script enforces a 20 MB unpacked limit before publishing. Build artifact exclusions are updated in .gitignore and .npmignore. ChangesRelease 3.4.1
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
![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 `@scripts/check_package_size.js`:
- Around line 7-8: The script uses a shell-only redirection ('2>/dev/null') in
the execSync call which breaks on Windows; change the execSync call in
scripts/check_package_size.js to run 'npm pack --dry-run --json' without inline
redirection and capture stdout via execSync(..., { encoding: 'utf8', stdio:
['pipe','pipe','ignore'] }) (or at minimum { encoding: 'utf8' }) then JSON.parse
the returned string into result and keep using result[0].unpackedSize for size;
update the execSync usage so result and size are derived from captured stdout
instead of relying on shell redirection.
🪄 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: 7d6ea8bf-8c8b-45ea-b83d-02924a30d041
⛔ Files ignored due to path filters (1)
src/wasm/wllama.wasmis excluded by!**/*.wasm
📒 Files selected for processing (8)
.gitignore.npmignorecompat/package.jsonllama.cpppackage.jsonscripts/check_package_size.jssrc/wasm-from-cdn.tssrc/workers-code/generated.ts
| const result = JSON.parse(execSync('npm pack --dry-run --json 2>/dev/null')); | ||
| const size = result[0].unpackedSize; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ls -la
git ls-files "scripts/check_package_size.js" || true
sed -n '1,200p' scripts/check_package_size.js || true
rg -n "check_package_size\.js" -S . || true
rg -n "npm pack --dry-run --json" -S . || true
rg -n "MAX_SIZE" -S scripts/check_package_size.js || true
rg -n "execSync\\(" scripts/check_package_size.js -n || trueRepository: ngxson/wllama
Length of output: 3035
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ls -la
git ls-files "scripts/check_package_size.js" || true
sed -n '1,200p' scripts/check_package_size.js || true
rg -n "check_package_size\.js" -S . || true
rg -n "npm pack --dry-run --json" -S . || true
rg -n "MAX_SIZE" -S scripts/check_package_size.js || true
rg -n "execSync\\(" scripts/check_package_size.js -n || trueRepository: ngxson/wllama
Length of output: 3035
Make the npm pack size check cross-platform
2>/dev/null is shell-dependent and can break the prepublishOnly size check on Windows. Capture stdout from execSync instead of using inline redirection.
💡 Proposed fix
-const result = JSON.parse(execSync('npm pack --dry-run --json 2>/dev/null'));
+const output = execSync('npm pack --dry-run --json', {
+ encoding: 'utf8',
+ stdio: ['ignore', 'pipe', 'ignore'],
+});
+const result = JSON.parse(output);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const result = JSON.parse(execSync('npm pack --dry-run --json 2>/dev/null')); | |
| const size = result[0].unpackedSize; | |
| const output = execSync('npm pack --dry-run --json', { | |
| encoding: 'utf8', | |
| stdio: ['ignore', 'pipe', 'ignore'], | |
| }); | |
| const result = JSON.parse(output); | |
| const size = result[0].unpackedSize; |
🤖 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 `@scripts/check_package_size.js` around lines 7 - 8, The script uses a
shell-only redirection ('2>/dev/null') in the execSync call which breaks on
Windows; change the execSync call in scripts/check_package_size.js to run 'npm
pack --dry-run --json' without inline redirection and capture stdout via
execSync(..., { encoding: 'utf8', stdio: ['pipe','pipe','ignore'] }) (or at
minimum { encoding: 'utf8' }) then JSON.parse the returned string into result
and keep using result[0].unpackedSize for size; update the execSync usage so
result and size are derived from captured stdout instead of relying on shell
redirection.
1 participant
Summary by CodeRabbit