mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-08-28 05:25:25 +00:00
* fix(embeddings): use system-matched onnxruntime-node CUDA build so CUDA 13 hosts use the GPU
transformers.js exact-pins a CUDA-12 onnxruntime-node while gitnexus' own dep floats to a CUDA-13 build. npm/pnpm cannot dedupe an exact pin against a range, so npm i -g installs two copies and the gitnexus overrides block (root-only) is inert. On a CUDA-13-only host the nested CUDA-12 provider cannot load libcublasLt.so.12, the CUDA EP fails, and embeddings silently fall back to CPU (isCudaAvailable() also only probed .so.12).
Add onnxruntime-node-resolver.ts (module.registerHooks redirect to the host-matching build, no-op elsewhere) mirroring onnxruntime-common-resolver.ts; probe libcublasLt .so.12 OR .so.13 against the copy that actually loads; unit test with 12 cases.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(embeddings): wire CUDA-13 build-match resolver into MCP query embedder
The MCP query-time embedder (src/mcp/core/embedder.ts) has its own,
separate initEmbedder() used for semantic search — it only called
ensureOnnxRuntimeCommonResolvable() before importing transformers.js, so
the CUDA-13 build-matching redirect added for the analyze/CLI embedder
never applied here. A CUDA-13 host running MCP search with
--embedding-device cuda still loaded the mismatched default onnxruntime-node
build.
Wire ensureOnnxRuntimeNodeMatchesSystem() into the same call site, mirroring
the core embedder's ordering (registered after the common-resolver fallback,
before the dynamic transformers import).
* fix(embeddings): gate CUDA redirect decision on registerHooks availability
decide() computed the CUDA-major redirect independent of whether Node's
module.registerHooks API actually exists — only ensureOnnxRuntimeNodeMatchesSystem()
checked that. On Node 22.0-22.14 (allowed by this package's engines
floor; registerHooks needs >=22.15), isCudaAvailable() could therefore
report a redirect target that ensureOnnxRuntimeNodeMatchesSystem() then
silently failed to install, so transformers.js loaded the mismatched
default onnxruntime-node build while the embedder still requested
device:'cuda' against it — reintroducing the uncatchable native crash
this probe exists to prevent.
Move the registerHooks check to the top of decide() so the probe and the
loader can never disagree, and skip CUDA-major probing entirely in that
case (a redirect could never install anyway).
Also fixes a related test-helper bug found while writing this unit's
tests: loadResolver's destructuring default (`registerHooks = vi.fn()`)
silently substituted a real mock function even when a test passed
`registerHooks: undefined` to simulate Node < 22.15 — meaning the
existing 'no-ops... when registerHooks is unavailable' test never
actually exercised that path. Distinguish 'omitted' from 'explicitly
undefined' via an 'in' check.
* test(embeddings): drive decide() -> redirect:true and assert the resolve() closure
The PR's actual shipped behavior — the installed registerHooks resolve()
closure, and the full redirect-active decision path — had zero executed
test coverage. All 4 prior ensureOnnxRuntimeNodeMatchesSystem tests avoided
driving decide() into redirect:true because require.resolve/createRequire
were never mocked, so the two-distinct-directory comparison decide()
depends on always resolved against whatever's actually installed in this
test's real node_modules (a single real copy, not the PR's two-copy
scenario).
Extend loadResolver()'s existing node:module mock to also fake createRequire,
keyed by call origin, so resolveOurOrtNodeDir/resolveDefaultOrtNodeDir can be
driven to two distinct fake directories with distinct CUDA majors — reaching
redirect:true without adding any injection points to production code. Then
capture the installed resolve() closure (mirroring the sibling
onnxruntime-common-resolver.test.ts's captureResolve() pattern) and assert
its three branches directly: onnxruntime-node redirect, onnxruntime-common
redirect, and passthrough for any other specifier.
* fix(embeddings): distinguish ldd detection-failure from no-CUDA-provider
ortCudaMajor treated any execFileSync('ldd', ...) failure with no usable
stdout (missing ldd binary, permission-denied .so, sandboxed exec)
identically to 'CUDA provider genuinely absent'. The pre-PR detection
(hasOrtCudaProvider) only used existsSync, never ldd, so this is a
regression: a CUDA-12 host that worked fine before this PR can now
silently fall back to CPU if ldd itself can't run, even though the
provider .so and system CUDA libs are both genuinely present.
readSoNeeded now reports whether ldd produced any usable output at all,
distinct from 'ldd ran and just found no matching NEEDED entry' (the
existing, already-handled '=> not found' case). When detection genuinely
fails, log a warning so an operator can tell 'CPU fallback because
detection itself failed' apart from 'CPU fallback because no CUDA build
shipped' — the return value stays null either way (the type can't
distinguish a third state), but the two cases are now observably
different via the log.
* fix(embeddings): check ourDir independently of whether defaultDir resolved
decide()'s ourDir fallback lookup was nested inside
'if (systemMajor != null && defaultDir)', so a null defaultDir (transformers'
own onnxruntime-node resolution failing outright, e.g. a partial/broken
install) skipped checking ourDir entirely — getEffectiveOnnxRuntimeNodeDir()
returned null even when gitnexus' own matching CUDA-13 copy would have
resolved fine and worked.
defaultDir resolving is not a precondition for the comparison: an
unresolvable default already counts as 'the default doesn't match', so the
ourDir check now runs whenever systemMajor is known, regardless of whether
defaultDir resolved.
* fix(embeddings): prefer CUDA 13 globally across the env-var directory scan
detectSystemCudaMajor's CUDA_PATH/LD_LIBRARY_PATH scan returned on the
first CUDA-major match within a single dir/sub pair, so a stale .so.12
found early (e.g. a leftover CUDA_PATH entry from a prior install) shadowed
a genuine .so.13 found later in the search path, even though the scan's
own ordering (checking 13 before 12 within each pair) was clearly intended
to prefer 13 wherever possible.
Keep scanning the full search space once a 12 is found, only returning
early once a 13 is found (the best possible answer) or the space is
exhausted.
* fix(embeddings): have onnxruntime-common-resolver defer to the effective onnxruntime-node dir
onnxruntime-common-resolver.ts independently re-derived transformers'
default onnxruntime-node dir (its own copy of the 'resolve transformers'
main entry, then onnxruntime-node' walk) to compute which onnxruntime-common
to pair with — duplicating onnxruntime-node-resolver.ts's own walk, and
capable of disagreeing with it: when the CUDA-major redirect is active,
this hook would still pair onnxruntime-common with transformers' default
(unredirected) onnxruntime-node, not the redirected copy the other hook
just switched onnxruntime-node itself to.
Have it call the already-exported getEffectiveOnnxRuntimeNodeDir() instead
— the same decision the CUDA-major redirect hook uses — so both hooks
always agree on which onnxruntime-node they're pairing onnxruntime-common
against, and the duplicated resolve-walk is removed entirely rather than
merely factored out.
* fix(embeddings): cache the effective CUDA major to remove redundant subprocess spawns
isCudaAvailable() in embedder.ts re-invoked ortCudaMajor/detectSystemCudaMajor
directly even though decide() (via getEffectiveOnnxRuntimeNodeDir) had
already computed both to make its redirect decision — a second, wasted
ldconfig + up to 2 ldd spawns on every initEmbedder() call.
Add effectiveMajor to the memoized Decision, computed once inside decide()
alongside effectiveDir/systemMajor, and export a single
isEffectiveCudaAvailable() that reads straight from the cached decision.
embedder.ts's local isCudaAvailable() wrapper (and its now-unused
getEffectiveOnnxRuntimeNodeDir/ortCudaMajor/detectSystemCudaMajor imports)
is replaced by this one exported function.
* fix(embeddings): surface CUDA redirect state at info level and in doctor
A successful CUDA-build redirect logged only at logger.debug (filtered
by the default 'info' level), and gitnexus doctor's embeddings section
never mentioned the redirect at all — leaving no diagnostic path for
'why is my CUDA-13 host still on CPU' after this PR ships.
Log the successful-redirect line at info (no-redirect/failure paths stay
at debug, since those are the common, expected case). Add
cudaRedirectDoctorStatus(), a pure summary of decide()'s already-computed
decision mirroring doctor.ts's existing localEmbeddingDoctorStatus shape,
and print it as a new literal (non-i18n) 'CUDA:' line in doctor's
embeddings section alongside the existing 'Support:' line, matching that
line's established convention.
* test(embeddings): register onnxruntime-node-resolver.test.ts in the cross-platform subset
The new test file guards on process.platform (linux/darwin cases) but was
absent from cross-platform-tests.ts's PLATFORM_LOGIC list, which
TESTING.md says platform-sensitive tests should be added to — so it never
ran on the Windows/macOS CI matrix, only Ubuntu.
Note: the sibling onnxruntime-common-resolver.test.ts has the identical,
pre-existing gap (it predates this PR) — left as-is here, since fixing
unrelated pre-existing test-registration debt is out of scope for this
PR's own follow-up fixes.
* test(embeddings): strengthen weak assertions, add garbled-output and CUDA_PATH coverage
Three of the four ensureOnnxRuntimeNodeMatchesSystem tests only asserted
'doesn't throw' rather than a concrete outcome — including one literally
named 'idempotent' that never asserted a call count on its own spy.
Strengthen each to assert real outcomes (module stays functional after a
no-op; spy call counts; return-value shape), while keeping the true
install-once idempotency proof in the redirect-active test added earlier
(this file's no-redirect scenario can't exercise it, since registerHooks
is never called either way).
Add the missing edge cases flagged in review: a CUDA_PATH-only fallback
scan test (mirroring the existing LD_LIBRARY_PATH one), and garbled/
unrecognized ldconfig and ldd output cases for both detectSystemCudaMajor
and ortCudaMajor, confirming neither falsely matches a CUDA major on
unparseable input. Also parameterize the non-linux platform test across
both darwin and win32 rather than darwin alone.
Not changed: the process.env reassignment vs. Object.defineProperty
'inconsistency' flagged in review — process.env, unlike process.platform,
has no getter-only restriction, so plain reassignment is already correct
and switching it to Object.defineProperty would be unnecessary ceremony.
* docs(embeddings): note the npm link/symlinked dev-checkout resolution caveat
resolveOurOrtNodeDir/resolveDefaultOrtNodeDir anchor to this module's own
real (post-symlink) location via import.meta.url, so a linked local dev
checkout may resolve against its own node_modules rather than the
consuming app's. Narrow, dev-only blast radius (regular npm/pnpm installs
are unaffected) — document-only, no structural fix warranted.
* fix(test): point the windowsHide spawn-family registry at the file that actually spawns
hooks.test.ts's windowsHide regression check still listed
gitnexus/src/core/embeddings/embedder.ts as a child_process-spawning
file, but this PR itself already moved all execFileSync usage out of
embedder.ts and into the new onnxruntime-node-resolver.ts — without
updating this registry. The check was silently failing at the PR's own
head commit (confirmed: 0 spawn-family calls found in embedder.ts,
'expected 0 to be greater than 0'), a pre-existing gap this fix-pass
surfaced via a full-suite run rather than something introduced by any of
the preceding follow-up commits.
Swap the registry entry to onnxruntime-node-resolver.ts, which does
import execFileSync (ldd + ldconfig, both already correctly passing
windowsHide: true).
* fix(test): make onnxruntime-node-resolver.test.ts path comparisons OS-agnostic
Registering this file in cross-platform-tests.ts's PLATFORM_LOGIC (a
prior commit in this series) means it now runs on the Windows CI matrix,
not just Ubuntu — and several of the fakeDirs-based tests (redirect:true,
ourDir-independent, subprocess-count, doctor-status) compared the
resolver's real join()/dirname() output against hardcoded forward-slash
fixture strings via exact-match or .startsWith().
Node's module is bound to path.win32 (or path.posix) based on the
REAL host OS at process start — stubbing process.platform later, as these
tests already do for the resolver's own platform branching, has no effect
on it. So on a genuine Windows runner, join(effectiveDir, 'package.json')
backslash-normalizes even under a faked platform:'linux', silently
breaking every forward-slash comparison in this file: the createRequire
dispatch would route to the wrong fake require, throw MODULE_NOT_FOUND,
get swallowed by ensureOnnxRuntimeNodeMatchesSystem's outer try/catch, and
registerHooks would never fire — the redirect-active tests would fail
outright on Windows CI.
Normalize every comparison point (the createRequire dispatcher, and the
shared execFileSync/existsSync mocks) with a single toPosix() helper.
Added a forceWin32Path test option (using path.win32's real join/dirname
behavior) to prove this holds without needing an actual Windows runner —
confirmed by temporarily reverting the fix and observing the new test
fail with the exact predicted mismatch before restoring it.
* chore(autofix): apply prettier + eslint fixes via /autofix command
* fix(embeddings): keep CUDA auto-detect working on Node < 22.15 when the default build already matches
The registerHooks guard in decide() returned effectiveMajor: null
unconditionally, so on Node 22.0-22.14 / 23.0-23.4 (engines floor is
>=22.0.0) isEffectiveCudaAvailable() was always false and a CUDA-12 host
whose default onnxruntime-node build already matched — which needs no
hook at all to use the GPU — silently regressed from CUDA to CPU on the
auto device path (pre-PR isCudaAvailable() behavior).
Probe the system and the default copy regardless of registerHooks
availability; only the ourDir redirect branch stays gated on it, so the
probe still never reports a redirect target that cannot be installed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
166 lines
6.6 KiB
TypeScript
166 lines
6.6 KiB
TypeScript
import { describe, it, expect, vi, afterEach } from 'vitest';
|
|
|
|
/**
|
|
* Tests for the #307 pnpm-strict / `pnpm dlx` fix: a synchronous in-thread ESM
|
|
* resolution hook (`module.registerHooks`) that redirects @huggingface/
|
|
* transformers' phantom `onnxruntime-common` import to gitnexus' own copy.
|
|
*
|
|
* Each test mocks `node:module` with a chosen `registerHooks` (a spy, or
|
|
* `undefined` to simulate Node < 22.15) so we can assert one-shot installation,
|
|
* graceful degradation, and the redirect/passthrough/rethrow logic of the
|
|
* resolve closure — without mutating the real process loader.
|
|
*/
|
|
|
|
const RESOLVER = '../../src/core/embeddings/onnxruntime-common-resolver.js';
|
|
|
|
/**
|
|
* (Re)load the resolver with a chosen `registerHooks` mocked into node:module.
|
|
* `vi.resetModules()` + the fresh `import()` re-initialises the module-level
|
|
* one-shot guard, so each test gets a pristine resolver with no shared state.
|
|
*
|
|
* When `getEffectiveOnnxRuntimeNodeDir` is supplied, the sibling
|
|
* onnxruntime-node-resolver.js is also mocked with it — letting a test drive
|
|
* (or spy on) whichever onnxruntime-node dir this hook's own onnxruntime-common
|
|
* lookup defers to, instead of independently re-deriving transformers'
|
|
* default (#2341 follow-up).
|
|
*/
|
|
async function loadResolver(
|
|
registerHooks: unknown,
|
|
getEffectiveOnnxRuntimeNodeDir?: () => string | null,
|
|
) {
|
|
vi.resetModules();
|
|
vi.doMock('node:module', async (importOriginal) => {
|
|
const orig = await importOriginal<typeof import('node:module')>();
|
|
return { ...orig, registerHooks };
|
|
});
|
|
if (getEffectiveOnnxRuntimeNodeDir) {
|
|
vi.doMock('../../src/core/embeddings/onnxruntime-node-resolver.js', () => ({
|
|
getEffectiveOnnxRuntimeNodeDir,
|
|
}));
|
|
}
|
|
return import(RESOLVER);
|
|
}
|
|
|
|
const ctx = { conditions: [], importAttributes: {} } as never;
|
|
const moduleNotFound = (): Error => {
|
|
const e = new Error("Cannot find package 'onnxruntime-common'") as Error & { code: string };
|
|
e.code = 'ERR_MODULE_NOT_FOUND';
|
|
return e;
|
|
};
|
|
|
|
afterEach(() => {
|
|
vi.doUnmock('node:module');
|
|
vi.doUnmock('../../src/core/embeddings/onnxruntime-node-resolver.js');
|
|
});
|
|
|
|
describe('ensureOnnxRuntimeCommonResolvable — installation', () => {
|
|
it('installs the resolve hook exactly once (idempotent)', async () => {
|
|
const spy = vi.fn();
|
|
const mod = await loadResolver(spy);
|
|
|
|
mod.ensureOnnxRuntimeCommonResolvable();
|
|
mod.ensureOnnxRuntimeCommonResolvable(); // second call is a no-op
|
|
|
|
expect(spy).toHaveBeenCalledTimes(1);
|
|
expect(typeof spy.mock.calls[0][0].resolve).toBe('function');
|
|
});
|
|
|
|
it('no-ops gracefully when registerHooks is unavailable (Node < 22.15)', async () => {
|
|
const mod = await loadResolver(undefined);
|
|
// Must not throw even though there is no synchronous-hooks API to call.
|
|
expect(() => mod.ensureOnnxRuntimeCommonResolvable()).not.toThrow();
|
|
});
|
|
|
|
it('is best-effort: swallows a registerHooks() failure instead of throwing into the embedder', async () => {
|
|
const mod = await loadResolver(
|
|
vi.fn(() => {
|
|
throw new Error('hook-install-failed');
|
|
}),
|
|
);
|
|
// The call site (initEmbedder) does not guard the return; a throw here would
|
|
// break `analyze --embeddings`. The outer try/catch must absorb it.
|
|
expect(() => mod.ensureOnnxRuntimeCommonResolvable()).not.toThrow();
|
|
});
|
|
});
|
|
|
|
describe('ensureOnnxRuntimeCommonResolvable — resolve hook behaviour', () => {
|
|
/** Install the fallback and return the resolve closure handed to registerHooks. */
|
|
async function captureResolve(getEffectiveOnnxRuntimeNodeDir?: () => string | null) {
|
|
const spy = vi.fn();
|
|
const mod = await loadResolver(spy, getEffectiveOnnxRuntimeNodeDir);
|
|
mod.ensureOnnxRuntimeCommonResolvable();
|
|
return spy.mock.calls[0][0].resolve as (
|
|
s: string,
|
|
c: never,
|
|
n: (s: string, c: never) => unknown,
|
|
) => unknown;
|
|
}
|
|
|
|
it('passes a successful default resolution through unchanged (no-op on hoisted layouts)', async () => {
|
|
const resolve = await captureResolve();
|
|
const real = { url: 'file:///real/onnxruntime-common/index.js', shortCircuit: true };
|
|
const next = vi.fn(() => real);
|
|
|
|
const res = resolve('onnxruntime-common', ctx, next);
|
|
|
|
expect(next).toHaveBeenCalledTimes(1);
|
|
expect(res).toBe(real); // the real resolution, NOT a redirect
|
|
});
|
|
|
|
it('redirects onnxruntime-common to the gitnexus copy when default resolution fails', async () => {
|
|
const resolve = await captureResolve();
|
|
const next = vi.fn(() => {
|
|
throw moduleNotFound();
|
|
});
|
|
|
|
const res = resolve('onnxruntime-common', ctx, next) as { url: string; shortCircuit: boolean };
|
|
|
|
expect(res.shortCircuit).toBe(true);
|
|
// The real resolved onnxruntime-common in node_modules (require.resolve runs
|
|
// for real here) — not just any path containing the substring.
|
|
expect(res.url).toMatch(/^file:\/\/.*\/node_modules\/onnxruntime-common\/.*\.js$/);
|
|
});
|
|
|
|
it('never masks an unrelated resolution failure (other specifiers rethrow)', async () => {
|
|
const resolve = await captureResolve();
|
|
const err = moduleNotFound();
|
|
const next = vi.fn(() => {
|
|
throw err;
|
|
});
|
|
|
|
expect(() => resolve('some-other-package', ctx, next)).toThrow(err);
|
|
});
|
|
|
|
it('rethrows when onnxruntime-common fails for a non-absence reason', async () => {
|
|
const resolve = await captureResolve();
|
|
// A present-but-otherwise-broken resolution (not a missing package) must
|
|
// surface, not be silently papered over with gitnexus' copy.
|
|
const err = Object.assign(new Error('bad specifier'), {
|
|
code: 'ERR_INVALID_MODULE_SPECIFIER',
|
|
});
|
|
const next = vi.fn(() => {
|
|
throw err;
|
|
});
|
|
|
|
expect(() => resolve('onnxruntime-common', ctx, next)).toThrow(err);
|
|
});
|
|
|
|
it("defers to getEffectiveOnnxRuntimeNodeDir() instead of independently re-deriving transformers' default (#2341 follow-up)", async () => {
|
|
const effectiveDirSpy = vi.fn(() => null as string | null);
|
|
const resolve = await captureResolve(effectiveDirSpy);
|
|
const next = vi.fn(() => {
|
|
throw moduleNotFound();
|
|
});
|
|
|
|
const res = resolve('onnxruntime-common', ctx, next) as { url: string; shortCircuit: boolean };
|
|
|
|
// The sibling module's decision is consulted (not bypassed)...
|
|
expect(effectiveDirSpy).toHaveBeenCalled();
|
|
// ...and since it reported no effective dir here, the code falls back to
|
|
// gitnexus' own direct dependency (the same fallback as "default
|
|
// resolution fails") rather than independently re-deriving a different
|
|
// path from @huggingface/transformers on its own.
|
|
expect(res.shortCircuit).toBe(true);
|
|
expect(res.url).toMatch(/^file:\/\/.*\/node_modules\/onnxruntime-common\/.*\.js$/);
|
|
});
|
|
});
|