Skip to content

fix: Windows build — decouple tsc from optional transformers dep#274

Merged
efenocchi merged 2 commits into
mainfrom
fix/windows-tsc-optional-transformers
Jun 17, 2026
Merged

fix: Windows build — decouple tsc from optional transformers dep#274
efenocchi merged 2 commits into
mainfrom
fix/windows-tsc-optional-transformers

Conversation

@efenocchi

@efenocchi efenocchi commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

Problem

The Windows smoke CI job has been red since cdc4ed83 moved @huggingface/transformers to optionalDependencies. On Windows the optional native package doesn't install, and npm install fires the prepare hook (husky && npm run build) → tsc, which fails:

src/embeddings/nomic.ts(21,41): error TS2307: Cannot find module '@huggingface/transformers' ...
src/embeddings/nomic.ts(54,28): error TS2307 ...
tests/claude-code/embeddings-nomic.test.ts(44,51): error TS2307 ...

Root cause: the typecheck hard-depends on an optional, on-demand package. type TransformersModule = typeof import("@huggingface/transformers") and literal import("@huggingface/transformers") calls force tsc to resolve the module at compile time, even though it's only installed by hivemind embeddings install and is absent on some platforms.

Fix (at the source, not the CI)

  • nomic.ts: replace typeof import(...) with a local interface TransformersModule describing only what the wrapper uses (env, pipeline). The runtime bare-specifier import uses a non-literal string specifier so tsc treats it as a dynamic any import and doesn't resolve the package.
  • test: same non-literal-specifier trick for the import(...) calls; vi.mock("@huggingface/transformers", ...) still intercepts them at runtime by resolved module id.

No runtime behavior change — the daemon still resolves and loads the real package via the canonical shared-deps path and bare fallback exactly as before.

Verification (real, local)

Reproduced the Windows condition on Linux by removing node_modules/@huggingface/transformers:

  • Before: tsc --noEmit → the TS2307 errors above.
  • After: tsc --noEmitpasses with the package absent.
  • With the package restored: embeddings-nomic.test.ts24/24 pass (mock interception via the dynamic import still works). Full npm run build green.

Summary by CodeRabbit

  • Refactor

    • Optimized module import handling for improved compatibility and stability.
  • Tests

    • Updated test suite to align with refactored import logic.

@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 4fdd8efc-4f3c-4878-ad28-f7e5584789f9

📥 Commits

Reviewing files that changed from the base of the PR and between 1ee2ab8 and 736be92.

📒 Files selected for processing (3)
  • embeddings/embed-daemon.js
  • src/embeddings/nomic.ts
  • tests/claude-code/embeddings-nomic.test.ts

📝 Walkthrough

Walkthrough

Three files replace inline "@huggingface/transformers" string literals in dynamic import() calls with local non-literal string variables (spec, TRANSFORMERS_PKG). nomic.ts also replaces a typeof import(...) type with a locally declared minimal TransformersModule interface. No runtime behavior or public APIs change.

Changes

Avoid tsc compile-time resolution of @huggingface/transformers

Layer / File(s) Summary
Non-literal import in daemon and Nomic embeddings module
embeddings/embed-daemon.js, src/embeddings/nomic.ts
_importFromBareSpecifier in the daemon and the loader in nomic.ts now pass a local string variable to import() instead of a literal. nomic.ts also replaces the typeof import("@huggingface/transformers") type with a locally declared minimal TransformersModule interface covering env and pipeline.
Test suite updated to use TRANSFORMERS_PKG constant
tests/claude-code/embeddings-nomic.test.ts
Introduces TRANSFORMERS_PKG: string constant and replaces all inline literal specifier imports across six test assertions (pipeline reuse, query-prefix, zero-norm, concurrent load, embedBatch) with imports via that constant.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related PRs

  • activeloopai/hivemind#248: Moved @huggingface/transformers to optionalDependencies, which is the companion change that motivated making the dynamic import non-literal to avoid compile-time resolution of the now-optional package.

Suggested reviewers

  • kaghni

🐇 A string in a variable, not a literal raw,
So tsc won't stumble on a package it never saw.
The daemon, the nomic, the tests all agree—
A spec and TRANSFORMERS_PKG set the imports free!
Compile in peace, little bunny. 🌿

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main fix: decoupling TypeScript compilation from an optional dependency to resolve Windows build failures.
Description check ✅ Passed The PR description is comprehensive, clearly explains the problem, root cause, solution, and verification steps. However, the Version Bump section from the template is not filled out.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/windows-tsc-optional-transformers

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report

Scope: files changed in this PR. Enforced threshold: 90% per metric (per file via vitest.config.ts).

Status Category Percentage Covered / Total
🟢 Lines 100.00% (🎯 90%) 63 / 63
🟢 Statements 97.33% (🎯 90%) 73 / 75
🟢 Functions 100.00% (🎯 90%) 14 / 14
🟢 Branches 94.59% (🎯 90%) 35 / 37
File Coverage — 1 file changed
File Stmts Branches Functions Lines
src/embeddings/nomic.ts 🟢 97.3% 🟢 94.6% 🟢 100.0% 🟢 100.0%

Generated for commit 3017504.

@efenocchi efenocchi merged commit 83280d3 into main Jun 17, 2026
11 checks passed
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.

1 participant