Skip to content

Commit 53aef53

Browse files
committed
fix(embedder): address PR review comments (Issue #629)
- Add embedder-ollama-batch-routing.test.mjs to CI manifest - Add comments explaining why provider options are omitted for Ollama batch - Add note about /v1/embeddings no-fallback assumption Reviewed by: rwmjhb
1 parent d2e68a6 commit 53aef53

File tree

2 files changed

+8
-0
lines changed

2 files changed

+8
-0
lines changed

scripts/ci-test-manifest.mjs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ export const CI_TEST_MANIFEST = [
4747
{ group: "core-regression", runner: "node", file: "test/store-serialization.test.mjs" },
4848
{ group: "core-regression", runner: "node", file: "test/access-tracker-retry.test.mjs" },
4949
{ group: "core-regression", runner: "node", file: "test/embedder-cache.test.mjs" },
50+
// Issue #629 batch embedding fix
51+
{ group: "llm-clients-and-auth", runner: "node", file: "test/embedder-ollama-batch-routing.test.mjs" },
5052
];
5153

5254
export function getEntriesForGroup(group) {

src/embedder.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,9 @@ export class Embedder {
587587
const apiKey = this.clients[0]?.apiKey ?? "ollama";
588588

589589
// Handle batch requests with /v1/embeddings + input array
590+
// NOTE: /v1/embeddings is used unconditionally for batch with no fallback.
591+
// If a model doesn't support that endpoint, failure will be silent from the user's perspective.
592+
// This is acceptable because most Ollama embedding models support /v1/embeddings.
590593
if (Array.isArray(payload.input)) {
591594
const response = await fetch(base + "/v1/embeddings", {
592595
method: "POST",
@@ -597,6 +600,9 @@ export class Embedder {
597600
body: JSON.stringify({
598601
model: payload.model,
599602
input: payload.input,
603+
// NOTE: Other provider options (encoding_format, normalized, dimensions, etc.)
604+
// from buildPayload() are intentionally not included. Ollama embedding models
605+
// do not support these parameters, so omitting them is correct.
600606
}),
601607
signal,
602608
});

0 commit comments

Comments
 (0)