chore(deps): migrate @xenova/transformers to @huggingface/transformers v4#1096
chore(deps): migrate @xenova/transformers to @huggingface/transformers v4#1096aqeelat wants to merge 1 commit into
Conversation
|
@aqeelat is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe PR migrates embedding and reranking integrations from ChangesTransformers package migration
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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. 🔧 Checkov (3.3.8)package.jsonTraceback (most recent call last): 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.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/huggingface.d.ts (1)
1-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid broad
any/unknowndeclarations for the optional package.
@huggingface/transformers4.2.0 ships TypeScript declarations, including a typedRawImage.fromBlob(): Promise<RawImage>. This ambient shim widens the APIs toanyandunknown, allowing invalid pipeline options and image handling to compile. Keep only a narrow fallback interface if omitted optional dependencies must still type-check. (npmjs.com)🤖 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/huggingface.d.ts` around lines 1 - 6, Update the ambient module declaration for `@huggingface/transformers` to avoid broad any and unknown types, relying on the package’s shipped declarations when available and retaining only a narrow fallback interface if needed for absent optional dependencies. Preserve typed pipeline options and make RawImage.fromBlob return the declared RawImage type.
🤖 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 `@README.md`:
- Line 1267: Update the README description for EMBEDDING_PROVIDER=local to
identify Xenova/all-MiniLM-L6-v2 as the default on-device model instead of
BGE-small, keeping the existing local-embedding behavior and configuration
guidance unchanged.
In `@test/reranker.test.ts`:
- Around line 3-10: Add a second reranker test case with a successful mock of
the `@huggingface/transformers` pipeline, alongside the existing
unavailable-module test. Exercise rerank and assert the mocked pipeline is
invoked or produces a changed ranking, so the test verifies the intended module
is loaded and used.
---
Nitpick comments:
In `@src/huggingface.d.ts`:
- Around line 1-6: Update the ambient module declaration for
`@huggingface/transformers` to avoid broad any and unknown types, relying on the
package’s shipped declarations when available and retaining only a narrow
fallback interface if needed for absent optional dependencies. Preserve typed
pipeline options and make RawImage.fromBlob return the declared RawImage type.
🪄 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: 15cb4830-5d34-4022-87d3-edd1a031e265
📒 Files selected for processing (24)
README.mdREADMEs/README.de-DE.mdREADMEs/README.es-ES.mdREADMEs/README.fr-FR.mdREADMEs/README.hi-IN.mdREADMEs/README.ja-JP.mdREADMEs/README.ko-KR.mdREADMEs/README.pt-BR.mdREADMEs/README.ru-RU.mdREADMEs/README.tr-TR.mdREADMEs/README.zh-CN.mdREADMEs/README.zh-TW.mdSECURITY.mdbenchmark/LONGMEMEVAL.mdbenchmark/REAL-EMBEDDINGS.mdbenchmark/real-embeddings-eval.tspackage.jsonsrc/huggingface.d.tssrc/providers/embedding/clip.tssrc/providers/embedding/local.tssrc/state/reranker.tssrc/xenova.d.tstest/reranker.test.tstsdown.config.ts
💤 Files with no reviewable changes (1)
- src/xenova.d.ts
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 `@test/reranker.test.ts`:
- Around line 106-107: Make the mock cleanup around the reranker tests
exception-safe by moving vi.doUnmock("`@huggingface/transformers`") into an
afterEach hook or a try/finally block. Keep vi.resetModules() as module-cache
cleanup, and ensure the unmock always runs even when a test assertion 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: 056f358f-378b-4d83-b869-d6f9298d82ea
📒 Files selected for processing (4)
README.mdsrc/providers/embedding/clip.tssrc/state/reranker.tstest/reranker.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/state/reranker.ts
- src/providers/embedding/clip.ts
| vi.doUnmock("@huggingface/transformers"); | ||
| vi.resetModules(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the test file and inspect the relevant section with line numbers.
git ls-files 'test/reranker.test.ts'
wc -l test/reranker.test.ts
sed -n '1,220p' test/reranker.test.tsRepository: rohitg00/agentmemory
Length of output: 1926
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the surrounding test structure and any cleanup hooks.
sed -n '1,220p' test/reranker.test.ts | nl -ba | sed -n '1,220p'Repository: rohitg00/agentmemory
Length of output: 1926
🌐 Web query:
Vitest vi.resetModules does not clear mock registry vi.doUnmock cleanup afterEach finally
💡 Result:
In Vitest, vi.resetModules is designed specifically to clear the module cache, not the mock registry [1][2][3]. By design, these two systems operate independently, and calling vi.resetModules will not remove mocks or reset their state [4][5]. If you are using vi.doMock and require the mock to be cleared or the module to be unmocked between tests, you must use vi.unmock or vi.doUnmock explicitly [1][3]. However, note that if a module is implicitly mocked (e.g., mocked without an explicit factory), there have been historical limitations where vi.unmock/vi.doUnmock might not behave as expected depending on the specific Vitest version [6]. To manage mock state between tests, use the following APIs depending on your goal: 1. To clear mock call history (but keep implementation): Use vi.clearAllMocks [5][7]. 2. To clear call history and reset implementation: Use vi.resetAllMocks [5][7]. 3. To restore original implementations: Use vi.restoreAllMocks [7]. For a clean state between tests, it is common practice to use these in an afterEach hook [8][7], or enable them globally in your configuration (e.g., clearMocks: true, resetMocks: true, or restoreMocks: true) [7]. If you are struggling with module-level state persistence across tests, combining vi.resetModules with vi.doMock and dynamic imports inside your tests is often the required pattern, as vi.mock is hoisted to the top of the file and cannot be dynamically toggled [1][9].
Citations:
- 1: https://vitest.dev/api/vi
- 2: https://github.com/vitest-dev/vitest/blob/v4.1.7/docs/api/vi.md
- 3: https://main.vitest.dev/api/vi
- 4: fix: don't clear mocks with vi.resetModules, cache normalised mock paths vitest-dev/vitest#1726
- 5: https://github.com/vitest-dev/vitest/blob/206e8cff/docs/api/vi.md
- 6:
vi.unmockdoes not unmock imitated module vitest-dev/vitest#2351 - 7: https://main.vitest.dev/guide/learn/mock-functions
- 8: https://vitest.dev/guide/mocking
- 9: mock is cached from test to test vitest-dev/vitest#3027
Make mock cleanup exception-safe. Move the vi.doUnmock("@huggingface/transformers") cleanup into afterEach or try/finally; vi.resetModules() only clears the module cache, so a failing assertion can leave the mock registered for later tests.
🤖 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 `@test/reranker.test.ts` around lines 106 - 107, Make the mock cleanup around
the reranker tests exception-safe by moving
vi.doUnmock("`@huggingface/transformers`") into an afterEach hook or a try/finally
block. Keep vi.resetModules() as module-cache cleanup, and ensure the unmock
always runs even when a test assertion fails.
…s v4 @xenova/transformers@2.x is deprecated and silently broken on Node 22+ (see rohitg00#479). The project was renamed to @huggingface/transformers; same Apache-2.0 license, same code. v4 ships onnxruntime-node/web and sharp as hard deps, so they're dropped from our optionalDependencies. Pipeline / RawImage.fromBlob / tolist / text-classification output shape all unchanged. Three behavior-preserving adjustments needed: - All 4 pipeline call sites pass { dtype: "q8" }. v4's default on Node is fp32 (DEFAULT_DEVICE_DTYPE = "fp32"); v2 defaulted to quantized=true. Without explicit dtype, all 4 sites silently regress (~3.5x larger download, slower inference). dtype "q8" maps to model_quantized.onnx per v4's DEFAULT_DTYPE_SUFFIX_MAPPING; file exists in all 3 Xenova models. This was the regression that prompted the test additions below. - src/providers/embedding/local.ts: split import try/catch from pipeline() call so model-load errors (network, missing q8 variant, disk) propagate with their actual message, not masked as "Install @huggingface/transformers...". - src/providers/embedding/{local,clip}.ts: type module from typeof import("@huggingface/transformers") so PretrainedModelOptions flows through; drop hand-rolled aliases and @ts-ignore. Cast at assignment sites (pipeline return union isn't structurally assignable to our narrow FeatureExtractor / ClipPipeline shapes). Tests added where coverage was zero (would have caught the dtype regression): - test/local-embedding-provider.test.ts (3 tests): unavailable-path install hint; pipeline called with dtype:q8 + extractor options + mapped Float32Array result; embedBatch shape. - test/clip-embedding-provider.test.ts (5 tests): unavailable-path; text pipeline dtype:q8 + result; embedBatch; embedImage with data: URL decode; custom model ID propagation. - test/reranker.test.ts: positive-path using vi.doMock + resetModules. Other: - src/huggingface.d.ts deleted (package ships its own types). - src/xenova.d.ts removed. - src/providers/embedding/clip.ts: inline single-use DIMENSIONS constant. - tsdown.config.ts: trim neverBundle list and comment. - README.md L1267: BGE-small -> Xenova/all-MiniLM-L6-v2 (was always wrong). - 16 docs: install commands + prose mentions across main README, 11 translations, SECURITY.md, 2 benchmark docs, benchmark script. - Model IDs (Xenova/all-MiniLM-L6-v2, Xenova/clip-vit-base-patch32, Xenova/ms-marco-MiniLM-L-6-v2) kept — HF Hub repo names, still valid. Closes rohitg00#1095. Fixes rohitg00#479. Verified: 1424/1424 tests pass, build clean, tsc clean on migrated files.
8207bed to
826b8bf
Compare
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 `@src/providers/embedding/clip.ts`:
- Around line 59-65: Update loadTransformers() to catch errors, replace only
missing-module ERR_MODULE_NOT_FOUND failures with an embedding-agnostic
`@huggingface/transformers` installation hint, and rethrow all other errors
unchanged. Also update the provider.embed("hello") expectation in the CLIP
embedding provider test to avoid asserting the image-specific message.
🪄 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: db31cfcc-2707-47cd-bda0-5af3b848b9fc
📒 Files selected for processing (25)
README.mdREADMEs/README.de-DE.mdREADMEs/README.es-ES.mdREADMEs/README.fr-FR.mdREADMEs/README.hi-IN.mdREADMEs/README.ja-JP.mdREADMEs/README.ko-KR.mdREADMEs/README.pt-BR.mdREADMEs/README.ru-RU.mdREADMEs/README.tr-TR.mdREADMEs/README.zh-CN.mdREADMEs/README.zh-TW.mdSECURITY.mdbenchmark/LONGMEMEVAL.mdbenchmark/REAL-EMBEDDINGS.mdbenchmark/real-embeddings-eval.tspackage.jsonsrc/providers/embedding/clip.tssrc/providers/embedding/local.tssrc/state/reranker.tssrc/xenova.d.tstest/clip-embedding-provider.test.tstest/local-embedding-provider.test.tstest/reranker.test.tstsdown.config.ts
💤 Files with no reviewable changes (1)
- src/xenova.d.ts
🚧 Files skipped from review as they are similar to previous changes (18)
- benchmark/REAL-EMBEDDINGS.md
- READMEs/README.tr-TR.md
- READMEs/README.ru-RU.md
- SECURITY.md
- src/state/reranker.ts
- tsdown.config.ts
- READMEs/README.zh-CN.md
- test/reranker.test.ts
- benchmark/LONGMEMEVAL.md
- READMEs/README.ko-KR.md
- READMEs/README.zh-TW.md
- READMEs/README.es-ES.md
- READMEs/README.hi-IN.md
- READMEs/README.pt-BR.md
- READMEs/README.de-DE.md
- README.md
- src/providers/embedding/local.ts
- READMEs/README.fr-FR.md
| async function loadTransformers(): Promise<TransformersModule> { | ||
| return import("@huggingface/transformers").catch(() => { | ||
| throw new Error( | ||
| "Install @huggingface/transformers for CLIP image embeddings: npm install @huggingface/transformers", | ||
| ); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)clip\.ts$|embedding|CLIP|clip' || true
echo "== target file outline =="
ast-grep outline src/providers/embedding/clip.ts --view expanded || true
echo "== target file relevant lines =="
cat -n src/providers/embedding/clip.ts | sed -n '1,180p'
echo "== loadTransformers usages =="
rg -n "loadTransformers|TextEncoder|image|CLIP|text embeddings|CLIP image embeddings|`@huggingface/transformers`" src/providers/embedding/clip.ts src -g '*.ts' || trueRepository: rohitg00/agentmemory
Length of output: 24333
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== clip test relevant assertions =="
cat -n test/clip-embedding-provider.test.ts | sed -n '1,260p'
echo "== local transformers loadTransformers behavior =="
cat -n src/providers/embedding/local.ts | sed -n '1,80p'
echo "== dynamic import failure behavior probe =="
node - <<'JS'
async function preserve() {
try {
await import('./this-package-should-not-exist-but-should-exist-because-bundling-can-change-runtime-404.mjs');
} catch (error) {
console.log('error_name', error && error.name);
console.log('error_message', error && error.message);
throw error;
}
}
async function rewrite() {
try {
await import('./this-package-should-not-exist-but-should-exist-because-bundling-can-change-runtime-404.mjs');
} catch {
throw new Error('Install `@huggingface/transformers` for CLIP image embeddings: npm install `@huggingface/transformers`');
}
}
(async () => {
try { await preserve() } catch (e) { console.log('preserve_thrown_name', e.name) }
try { await rewrite() } catch (e) { console.log('rewrite_thrown_name', e.name) }
})();
JSRepository: rohitg00/agentmemory
Length of output: 6152
Preserve import errors and use an embedding-agnostic install hint.
loadTransformers() is called from both text and image CLIP paths, but its blanket catch only throws install guidance and hard-codes “CLIP image embeddings,” so text failures are misleading and non-module initialization errors get lost. Only rethrow missing-module ERR_MODULE_NOT_FOUND with a CLIP-transformers install hint; rethrow everything else. Update the provider.embed("hello") expectation in test/clip-embedding-provider.test.ts to avoid depending on the image-specific message.
🤖 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/providers/embedding/clip.ts` around lines 59 - 65, Update
loadTransformers() to catch errors, replace only missing-module
ERR_MODULE_NOT_FOUND failures with an embedding-agnostic
`@huggingface/transformers` installation hint, and rethrow all other errors
unchanged. Also update the provider.embed("hello") expectation in the CLIP
embedding provider test to avoid asserting the image-specific message.
1 participant
Summary
@xenova/transformers@2.17.2is deprecated and silently broken on Node 22+ (see #479). This swaps to@huggingface/transformers@^4.2.0— the same project, renamed, with an actively maintained ONNX runtime backend and Node 22/24 support. Drop-in for our usage:pipeline,RawImage.fromBlob,tolist, andtext-classificationoutput shape are all unchanged.Closes #1095. Fixes #479.
Changes
package.json: replace@xenova/transformers+ directonnxruntime-{node,web}optional deps with a single@huggingface/transformers@^4.2.0. The onnxruntime packages are now bundled as HF v4's hard deps;sharp,@huggingface/jinja, and@huggingface/tokenizerscome along transitively.src/xenova.d.ts→src/huggingface.d.ts: rename ambient module decl and addRawImageexport so the cast inclip.tsis no longer needed.src/providers/embedding/{local,clip}.ts,src/state/reranker.ts: swap import specifier + install error strings. Drop theas unknown as TransformersModulecast inclip.ts.tsdown.config.ts: updateneverBundlelist.test/reranker.test.ts: swapvi.mockspecifier.README.md, 11READMEs/translations,SECURITY.md, 2 benchmark docs, and the benchmark script.CHANGELOG.mdhistorical entry left as-is.Model IDs (
Xenova/all-MiniLM-L6-v2,Xenova/clip-vit-base-patch32,Xenova/ms-marco-MiniLM-L-6-v2) deliberately kept — HF Hub repo names, still valid under v3/v4.Verification
npm install— clean (273 packages added; transitiveonnxruntime-node@1.24.3,sharp@0.34.5,@huggingface/jinja,@huggingface/tokenizers)npm test— 1415/1415 passingnpm run build— clean@huggingface/transformers@4.2.0loads, exposespipeline/RawImage.fromBlob/env.cacheDirtsc --noEmiterrors are baseline (cli.ts, slots.ts, api.ts) — none touch migrated filesLicense check
@huggingface/transformers4.2.0 (new)@xenova/transformers2.17.2 (removed)onnxruntime-node1.14.0 → 1.24.3onnxruntime-web1.14.0 → 1.26.0-devsharp0.34.5 (new transitive)@huggingface/jinja(new transitive)@huggingface/tokenizers(new transitive)All permissive, all compatible with this repo's Apache-2.0. No NOTICE files required.
Follow-ups (deliberately out of scope)
engines.nodebump — acceptance criteria from @xenova/transformers v2.17.2 fails on Node.js 22+ with "blob:nodedata:" worker URL error — semantic embedding silently disabled #479; separate change owned by CI config.local-embeddingsjob.Relates
Summary by CodeRabbit
@huggingface/transformers.Xenova/all-MiniLM-L6-v2.@huggingface/transformersfor improved compatibility.@huggingface/transformersand added coverage for successful pipeline usage.