sync with latest upstream llama.cpp (v3.5.0)#252
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 (1)
📝 WalkthroughWalkthroughBumps the Changeswllama 3.5.0 Release with llama.cpp Update
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/bisect_test.sh`:
- Around line 21-24: The sed commands modifying CONTEXT_H at lines 21 and 24 use
overly broad regex patterns that match any standalone `#if` 0 or `#if` 1 line in the
file, potentially rewriting unrelated preprocessor blocks and invalidating the
bisect test. To fix this, scope the sed replacements to target only the
common_get_device_memory_data compatibility block by including surrounding
context lines in the sed command. This could be done by matching a few lines of
context before or after the target `#if` statement, or by anchoring the pattern to
a nearby function name or comment that uniquely identifies the compatibility
block you intend to modify. This ensures only the specific block gets rewritten
while leaving other `#if` blocks in the file untouched.
In `@scripts/docker-compose.yml`:
- Around line 53-67: The `SKIP_COMPAT` conditional block (lines 54-67) skips
creation of the `build-compat` directory when the flag is set, but a subsequent
`ls` command at line 74 unconditionally attempts to access this directory. With
`set -e`, this causes the script to fail even when the main build succeeds. Wrap
the `ls build-compat` command at line 74 with the same `if [ -z "${SKIP_COMPAT}"
]` conditional guard used for the build block so it only executes when the
directory actually exists.
🪄 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: ca935937-3f17-4ed4-b2c6-16547340efe8
⛔ Files ignored due to path filters (1)
src/wasm/wllama.wasmis excluded by!**/*.wasm
📒 Files selected for processing (13)
CMakeLists.txtcompat/package.jsoncompat/wasm/wllama.jscpp/wllama-context.hllama.cpppackage.jsonscripts/bisect_test.shscripts/docker-compose.ymlsrc/wasm-from-cdn.tssrc/wasm/source-map.tssrc/wasm/wllama.jssrc/workers-code/generated.tsvitest.config.ts
Summary by CodeRabbit