Add @wllama/wllama-compat for Safari compatibility#223
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds a compat build and runtime mode: a new ChangesCompat mode for JSPI/Memory64-less browsers
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 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)
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 |
Owner
Author
|
CC @reeselevine , we are now distributing an optional package This is to avoid bloating user's |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/workers-code/llama-cpp.js (1)
252-275:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize Memory64 read offsets before crossing into DOM APIs.
Line 261 forwards raw
offset/req_size, and Line 273 uses rawout_ptr. In the non-compat Memory64 build those can beBigInt, butBlob.slice()insrc/worker.tsand typed-array offsets only acceptnumber, so async file reads can throw during model load. Convert them once at this JS boundary. The in-file 2GB cap makes that cast safe here.Proposed fix
const _wllama_js_file_read = async (path, offset, req_size, out_ptr) => { const name = _stripModelsPrefix(path); + const offsetNum = Number(offset); + const reqSizeNum = Number(req_size); + const outPtrNum = Number(out_ptr); pendingReadPromise = new Promise((res, rej) => { pendingReadResolve = res; pendingReadReject = rej; }); isAwaitReading = true; - postMessage({ verb: 'fs.read_req', args: [name, offset, req_size] }); + postMessage({ verb: 'fs.read_req', args: [name, offsetNum, reqSizeNum] }); let data; try { data = await pendingReadPromise; } finally { @@ const bytes = new Uint8Array(data); - getHeapU8().set(bytes, out_ptr); + getHeapU8().set(bytes, outPtrNum); return toSizeT(bytes.length); };🤖 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 `@src/workers-code/llama-cpp.js` around lines 252 - 275, _wllama_js_file_read forwards Memory64 BigInt offsets into DOM APIs and typed-array ops; normalize offset, req_size and out_ptr to JS Numbers before use. Inside _wllama_js_file_read (and before postMessage/getHeapU8().set), coerce each of offset, req_size, out_ptr from BigInt to Number (e.g. const off = typeof offset === 'bigint' ? Number(offset) : offset), then apply the existing in-file 2GB cap (e.g. cappedOff = Math.min(off, 0x7fffffff)) for safety, use these capped numeric values in postMessage({... args: [name, cappedOff, cappedReqSize] }) and in getHeapU8().set(bytes, cappedOutPtr); leave pendingReadPromise/pendingReadResolve/pendingReadReject, _stripModelsPrefix, toSizeT unchanged.
🧹 Nitpick comments (1)
examples/embeddings/index.html (1)
39-43: 💤 Low valueInconsistent compat configuration compared to other examples.
Other examples (basic, multimodal, tools) include the
'exclude_firefox'argument when callingsetCompat()for localhost, but this example omits it. For consistency across all examples, consider adding it.♻️ Proposed change for consistency
wllama.setCompat({ wasm: '../../compat/wasm/wllama.wasm', worker: '../../compat/wasm/wllama.js', - }); + }, 'exclude_firefox');🤖 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 `@examples/embeddings/index.html` around lines 39 - 43, The localhost compat block for wllama is missing the exclude_firefox option; update the call to wllama.setCompat(...) in the examples/embeddings/index.html file to include the same 'exclude_firefox' argument used in other examples so the call to wllama.setCompat({ wasm: ..., worker: ..., exclude_firefox: true }) matches the basic/multimodal/tools examples and keeps behavior consistent across examples.
🤖 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 `@compat/package.json`:
- Around line 5-6: The package sets "main": "index.js" but no compat/index.js
exists so importing the package root fails; fix by either adding a minimal
compat/index.js (and optional compat/index.d.ts) that re-exports the wasm
entrypoints, or by updating compat/package.json to remove or change "main" and
add appropriate "exports" entries that point only to the existing wasm files
(e.g., export "./wasm/*" and/or "./package.json" as needed); edit the
package.json "main" value or the "exports" map and/or create the minimal
compat/index.js file and ensure the release scripts (scripts/post_build.sh,
scripts/docker-compose.yml) will include it so root imports like import
'`@wllama/wllama-compat`' resolve.
In `@compat/README.md`:
- Around line 73-77: Summary: The README text incorrectly says "import it as
`?raw`" which conflicts with the example that uses `?url` for the .wasm file and
`?raw` for the worker JS. Fix: update the sentence to explicitly state that the
.wasm asset should be imported with `?url` and the worker JS with `?raw`
(referencing compatWasm and compatWorker in the example), or reword to: ".wasm
-> ?url; worker JS -> ?raw" so the guidance matches the import lines for
compatWasm and compatWorker.
In `@examples/basic/index.html`:
- Around line 91-97: The snippet only sets compat for localhost; add an else
branch to handle non-localhost by calling wllama.setCompat('default') so the app
uses the default compatibility on deployments—locate the window.location.href
check and the existing wllama.setCompat({...}, 'exclude_firefox') block and
append an else that invokes wllama.setCompat('default') for all other hosts.
In `@examples/main/src/config.ts`:
- Around line 3-4: The import in examples/main/src/config.ts references a
missing asset wllama.wasm (import lines compatWasm and compatWorker); fix by
either adding the missing file compat/wasm/wllama.wasm into the compat package
so `@wllama/wllama-compat/wasm/wllama.wasm`?url resolves, or update the example to
point compatWasm to a valid source (e.g., a CDN or the actual generated wasm
path used in your build) and keep the compatWorker import as-is.
In `@src/wllama.ts`:
- Line 153: The field this.compat is being initialized to WasmCompatFromCDN
which lets needCompat() trigger compat behavior by default; change
initialization so this.compat starts as null (or an explicit opt-out) and only
assign WasmCompatFromCDN inside setCompat()/when mode: 'firefox_safari' is
selected so compat becomes opt-in; update any places that rely on this.compat
(references around lines with WasmCompatFromCDN and methods needCompat(),
setCompat()) to handle a null compat value safely and ensure Firefox/Safari
compat is only enabled when setCompat(mode: 'firefox_safari') is called.
- Around line 493-501: The Firefox branch currently assumes missing JSPI when
workerResources.compat is false and emits an incorrect message (and the text has
a typo "compat more"); update the logic in the isFirefox() handling to
explicitly detect JSPI support (e.g., check an existing flag like
workerResources.jspiAvailable or add one) instead of inferring from
workerResources.compat, and emit one warning when JSPI is actually unavailable
and a different, accurate guidance when compat is simply off; also fix the typo
by replacing "compat more" with clearer wording such as "enable compat mode" in
the logger().warn calls.
In `@src/worker.ts`:
- Around line 78-83: The WllamaWorkerResources interface and init logic must
prevent using the default WLLAMA_EMSCRIPTEN_CODE JS when compat is true: update
the code that currently falls back to WLLAMA_EMSCRIPTEN_CODE to instead throw an
error (or require a compat-specific js payload) when resources.compat === true
and resources.jsPath is missing; specifically, change the logic that reads
jsPath/uses WLLAMA_EMSCRIPTEN_CODE (referencing WllamaWorkerResources, jsPath,
compat and the WLLAMA_EMSCRIPTEN_CODE constant) so it validates if (compat &&
!jsPath) { throw new Error("compat mode requires an explicit jsPath/compat JS
bundle"); } and remove the implicit fallback in that branch or provide a
dedicated compat JS constant.
---
Outside diff comments:
In `@src/workers-code/llama-cpp.js`:
- Around line 252-275: _wllama_js_file_read forwards Memory64 BigInt offsets
into DOM APIs and typed-array ops; normalize offset, req_size and out_ptr to JS
Numbers before use. Inside _wllama_js_file_read (and before
postMessage/getHeapU8().set), coerce each of offset, req_size, out_ptr from
BigInt to Number (e.g. const off = typeof offset === 'bigint' ? Number(offset) :
offset), then apply the existing in-file 2GB cap (e.g. cappedOff = Math.min(off,
0x7fffffff)) for safety, use these capped numeric values in postMessage({...
args: [name, cappedOff, cappedReqSize] }) and in getHeapU8().set(bytes,
cappedOutPtr); leave pendingReadPromise/pendingReadResolve/pendingReadReject,
_stripModelsPrefix, toSizeT unchanged.
---
Nitpick comments:
In `@examples/embeddings/index.html`:
- Around line 39-43: The localhost compat block for wllama is missing the
exclude_firefox option; update the call to wllama.setCompat(...) in the
examples/embeddings/index.html file to include the same 'exclude_firefox'
argument used in other examples so the call to wllama.setCompat({ wasm: ...,
worker: ..., exclude_firefox: true }) matches the basic/multimodal/tools
examples and keeps behavior consistent across examples.
🪄 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: 07bcd36d-2e32-4ee4-88fd-685653b59353
⛔ Files ignored due to path filters (2)
examples/main/package-lock.jsonis excluded by!**/package-lock.jsonsrc/wasm/wllama.wasmis excluded by!**/*.wasm
📒 Files selected for processing (27)
.gitignore.prettierignoreCMakeLists.txtREADME.mdcompat/README.mdcompat/index.tscompat/package.jsoncompat/wasm/wllama.jsexamples/basic/index.htmlexamples/embeddings/index.htmlexamples/main/package.jsonexamples/main/src/config.tsexamples/main/src/utils/wllama.context.tsxexamples/multimodal/index.htmlexamples/tools/index.htmlllama.cpppackage.jsonscripts/docker-compose.ymlscripts/generate_wasm_from_cdn.jsscripts/post_build.shsrc/utils.tssrc/wasm-from-cdn.tssrc/wasm/wllama.jssrc/wllama.tssrc/worker.tssrc/workers-code/generated.tssrc/workers-code/llama-cpp.js
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/wllama.ts (1)
499-502:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix inverted JSPI check and typo in Firefox warning.
Line 499 currently warns about missing JSPI only when
isSupportJSPI()is true, so guidance is inverted. Line 501 also still says “compat more”.Proposed fix
- } else if (isSupportJSPI()) { + } else if (!isSupportJSPI()) { this.logger().warn( - 'WebGPU is disabled on Firefox due to missing JSPI support. Please consider enabling compat more, or enabling "javascript.options.wasm_js_promise_integration" in "about:config".' + 'WebGPU is disabled on Firefox due to missing JSPI support. Please consider enabling compat mode, or enabling "javascript.options.wasm_js_promise_integration" in "about:config".' ); }🤖 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 `@src/wllama.ts` around lines 499 - 502, The condition and message are inverted: change the branch that logs the Firefox JSPI warning to trigger when JSPI is NOT supported (use !isSupportJSPI()), and update the logger().warn text in the block to fix the typo and clarify guidance (e.g., "WebGPU is disabled on Firefox due to missing JSPI support. Please consider enabling compatibility mode, or enable \"javascript.options.wasm_js_promise_integration\" in about:config."). Ensure you modify the isSupportJSPI() check and the string passed to logger().warn accordingly.
🤖 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 `@examples/main/vite.config.ts`:
- Around line 3-4: The build fails because vite.config.ts imports Node built-ins
(import { existsSync } from 'fs'; import { resolve } from 'path';) but
examples/main lacks Node type coverage; add "`@types/node`" to
examples/main/package.json devDependencies and update
examples/main/tsconfig.node.json to include Node types (e.g., add "types":
["node"] under "compilerOptions") so tsc can resolve fs/path typings for
vite.config.ts.
---
Duplicate comments:
In `@src/wllama.ts`:
- Around line 499-502: The condition and message are inverted: change the branch
that logs the Firefox JSPI warning to trigger when JSPI is NOT supported (use
!isSupportJSPI()), and update the logger().warn text in the block to fix the
typo and clarify guidance (e.g., "WebGPU is disabled on Firefox due to missing
JSPI support. Please consider enabling compatibility mode, or enable
\"javascript.options.wasm_js_promise_integration\" in about:config."). Ensure
you modify the isSupportJSPI() check and the string passed to logger().warn
accordingly.
🪄 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: 3c9068f8-e28c-42b1-a6f5-df502651f7e5
⛔ Files ignored due to path filters (1)
src/wasm/wllama.wasmis excluded by!**/*.wasm
📒 Files selected for processing (15)
CMakeLists.txtcompat/README.mdcompat/index.jscompat/wasm/wllama.jsexamples/basic/index.htmlexamples/main/src/config.tsexamples/main/src/vite-env.d.tsexamples/main/vite.config.tssrc/mjs.test.tssrc/wasm/wllama.jssrc/wllama.test.tssrc/wllama.tssrc/wllama.wgpu.test.tssrc/worker.tssrc/workers-code/generated.ts
💤 Files with no reviewable changes (1)
- compat/index.js
✅ Files skipped from review due to trivial changes (1)
- compat/README.md
Contributor
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)
examples/main/vite.config.ts (1)
10-18:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize + JSON-encode Windows paths when embedding them into the virtual module import specifiers.
COMPAT_WASM/COMPAT_JSare built with Noderesolve()and injected into the generated code as'${COMPAT_WASM}?url'/'${COMPAT_JS}?raw'. On Windows this can yield backslash-containing specifiers (including escape sequences like\n) and break the virtual module; usenormalizePathandJSON.stringifywhen generating the import strings. (https://vite.dev/guide/api-plugin)🪟 Suggested change
- import { defineConfig } from 'vite'; + import { defineConfig, normalizePath } from 'vite'; @@ -const COMPAT_WASM = resolve( +const COMPAT_WASM = normalizePath(resolve( __dirname, 'node_modules/@wllama/wllama-compat/wasm/wllama.wasm' -); -const COMPAT_JS = resolve( +)); +const COMPAT_JS = normalizePath(resolve( __dirname, 'node_modules/@wllama/wllama-compat/wasm/wllama.js' -); +)); @@ - return ` -import wasm from '${COMPAT_WASM}?url'; -import worker from '${COMPAT_JS}?raw'; + return ` +import wasm from ${JSON.stringify(`${COMPAT_WASM}?url`)}; +import worker from ${JSON.stringify(`${COMPAT_JS}?raw`)}; export default { wasm, worker: { code: worker } }; `;🤖 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 `@examples/main/vite.config.ts` around lines 10 - 18, When embedding COMPAT_WASM and COMPAT_JS into the virtual module import specifiers, normalize their paths and JSON-encode the resulting strings to avoid backslashes/escape sequences on Windows; specifically, call normalizePath(resolve(...)) for COMPAT_WASM and COMPAT_JS and then wrap the normalized string with JSON.stringify before injecting as "'...'?url" / "'...'?raw" in the generated code so the import specifiers are safe across platforms.
🤖 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 `@package.json`:
- Line 23: The "upload" npm script currently publishes the main package before
the companion "compat" package, risking the main release going live if the
compat publish fails; update the "upload" script so it runs the compat publish
first (ensure the "(cd compat && npm publish --access public)" step executes
before publishing the main package), keep the format and build steps intact, and
use && chaining so the main package publish only runs if the compat publish
succeeds; modify the "upload" script entry in package.json accordingly.
---
Outside diff comments:
In `@examples/main/vite.config.ts`:
- Around line 10-18: When embedding COMPAT_WASM and COMPAT_JS into the virtual
module import specifiers, normalize their paths and JSON-encode the resulting
strings to avoid backslashes/escape sequences on Windows; specifically, call
normalizePath(resolve(...)) for COMPAT_WASM and COMPAT_JS and then wrap the
normalized string with JSON.stringify before injecting as "'...'?url" /
"'...'?raw" in the generated code so the import specifiers are safe across
platforms.
🪄 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: a7afe29b-5a60-4d6a-8d9a-a94bbfdc9fd0
⛔ Files ignored due to path filters (2)
examples/main/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
examples/main/package.jsonexamples/main/vite.config.tspackage.jsonsrc/wllama.ts
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
package.json (1)
23-23:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPublishing order issue remains unaddressed.
The main package is still published before the compat package. If the compat publish fails,
@wllama/wllama@3.2.2will already be live on npm with CDN URLs pointing to a non-existent@wllama/wllama-compat@3.2.2, breaking Safari compatibility for all users who install this version.🔁 Apply the previously suggested fix
- "upload": "npm run format && npm run build && npm publish --access public && (cd compat && npm publish --access public)", + "upload": "npm run format && npm run build && (cd compat && npm publish --access public) && npm publish --access public",🤖 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 `@package.json` at line 23, The "upload" npm script currently publishes the main package before the compat package; change the script so the compat package is published first (run the compat publish step before publishing the main package) and ensure the build/format steps still run before both publishes; specifically update the "upload" script entry so it runs format and build, then publishes the compat package (cd compat && npm publish --access public) and only after that publishes the main package (npm publish --access public) to prevent releasing the main package that references a missing compat release.
🧹 Nitpick comments (1)
scripts/generate_wasm_from_cdn.js (1)
7-19: 🏗️ Heavy liftVersion-sync concern is not a current blocker; consider guarding against future drift.
The generator’s use of the main
packageJson.versionfor@wllama/wllama-compatURLs is consistent in this repo (compat and main versions both equal3.2.2), so the generated CDN URLs should exist. Add a build-time check to fail fast ifcompat/package.jsonever diverges from the main version.🤖 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/generate_wasm_from_cdn.js` around lines 7 - 19, Add a build-time guard in scripts/generate_wasm_from_cdn.js that reads and compares the main packageJson.version (variable "version") with the compat package version (read from compat/package.json, call it "compatVersion"); if they differ, fail fast (throw or process.exit with an error) and include both versions in the error message, and then use "compatVersion" when constructing the WasmCompatFromCDN URLs rather than reusing "version" so the generated CDN links reflect the actual compat package version; place this check before building the outputContent constant.
🤖 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.
Duplicate comments:
In `@package.json`:
- Line 23: The "upload" npm script currently publishes the main package before
the compat package; change the script so the compat package is published first
(run the compat publish step before publishing the main package) and ensure the
build/format steps still run before both publishes; specifically update the
"upload" script entry so it runs format and build, then publishes the compat
package (cd compat && npm publish --access public) and only after that publishes
the main package (npm publish --access public) to prevent releasing the main
package that references a missing compat release.
---
Nitpick comments:
In `@scripts/generate_wasm_from_cdn.js`:
- Around line 7-19: Add a build-time guard in scripts/generate_wasm_from_cdn.js
that reads and compares the main packageJson.version (variable "version") with
the compat package version (read from compat/package.json, call it
"compatVersion"); if they differ, fail fast (throw or process.exit with an
error) and include both versions in the error message, and then use
"compatVersion" when constructing the WasmCompatFromCDN URLs rather than reusing
"version" so the generated CDN links reflect the actual compat package version;
place this check before building the outputContent constant.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 634e7485-5c94-4559-a521-738063bf31a3
📒 Files selected for processing (4)
compat/package.jsonpackage.jsonscripts/generate_wasm_from_cdn.jssrc/wasm-from-cdn.ts
✅ Files skipped from review due to trivial changes (2)
- src/wasm-from-cdn.ts
- compat/package.json
This was referenced May 27, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Chores
Examples
Tests