Skip to content

fix(smrt-core): prefer huggingface transformers for embeddings#1109

Merged
willgriffin merged 2 commits intomainfrom
codex/fix-core-transformers-runtime
Apr 9, 2026
Merged

fix(smrt-core): prefer huggingface transformers for embeddings#1109
willgriffin merged 2 commits intomainfrom
codex/fix-core-transformers-runtime

Conversation

@willgriffin
Copy link
Copy Markdown
Contributor

Summary

  • prefer @huggingface/transformers for local embeddings when available, with fallback to @xenova/transformers
  • preserve non-module runtime failures and improve the missing-package error message
  • add regression coverage for the module resolution behavior

Verification

  • pnpm --filter @happyvertical/smrt-core exec vitest run src/tests/embedding-provider-transformers-resolution.test.ts
  • pnpm --filter @happyvertical/smrt-core exec tsc --noEmit
  • pnpm --filter @happyvertical/smrt-core build

Copilot AI review requested due to automatic review settings April 9, 2026 19:53
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates @happyvertical/smrt-core local embedding initialization to prefer @huggingface/transformers (when installed) and fall back to @xenova/transformers, while improving missing-package errors and adding regression tests for module resolution behavior.

Changes:

  • Add runtime resolution logic to prefer @huggingface/transformers for local embeddings with fallback to @xenova/transformers.
  • Improve the “missing optional dependency” error message and preserve non-module runtime failures.
  • Add Vitest coverage for the package resolution order and error behavior.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pnpm-lock.yaml Adds @huggingface/transformers to core dev deps (lockfile needs to be consistent with package.json).
packages/core/src/embeddings/provider.ts Implements optional transformers package resolution and uses it for local pipeline initialization and availability checks.
packages/core/src/tests/embedding-provider-transformers-resolution.test.ts Adds regression tests for preference order, fallback, and error propagation.
packages/core/package.json Declares @huggingface/transformers as an optional peer dependency and adds it to devDependencies.
.changeset/fair-squids-tickle.md Patch changeset documenting the behavior change and Node 24/sharp motivation.
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +705 to 710
'@huggingface/transformers':
specifier: ^3.8.1
version: 3.8.1
'@types/node':
specifier: 24.10.9
version: 24.10.9
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the lockfile, packages/core still pins @types/node to 24.10.9, but packages/core/package.json specifies @types/node: "25.0.9". Please regenerate/update pnpm-lock.yaml so the recorded specifier/version matches the package.json dependency set (usually by running pnpm install from the repo root).

Copilot uses AI. Check for mistakes.
Comment on lines +69 to +73
if (lastError) {
throw formatTransformersResolutionError(LOCAL_TRANSFORMERS_PACKAGES);
}

throw formatTransformersResolutionError(LOCAL_TRANSFORMERS_PACKAGES);
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The final if (lastError) { throw ... } followed by an unconditional throw ... is redundant here, since both branches throw the same error. Consider removing the conditional (and potentially the unused lastError tracking) to simplify the control flow.

Copilot uses AI. Check for mistakes.
@willgriffin willgriffin merged commit 60084ad into main Apr 9, 2026
12 checks passed
@willgriffin willgriffin deleted the codex/fix-core-transformers-runtime branch April 9, 2026 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants