Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions src/embeddings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export function enforceDimensions(provider: EmbeddingProvider, dimensions: numbe
if (embedding && embedding.length !== dimensions) {
if (!warned) {
warned = true;
console.warn(
console.info(
`openbrain: the embedding model returned ${embedding.length} dimensions but ` +
`config.embeddings.dimensions is ${dimensions}; ignoring these embeddings. ` +
`Update embeddings.dimensions to match the model, then run "openbrain index rebuild".`
Expand Down Expand Up @@ -155,10 +155,9 @@ class TransformersEmbeddingProvider implements EmbeddingProvider {
private async loadExtractor() {
const transformers = await import("@huggingface/transformers");
transformers.env.cacheDir = modelCacheDir(this.options);
return (await transformers.pipeline(
"feature-extraction",
this.config.embeddings.model
)) as unknown as FeatureExtractor;
return (await transformers.pipeline("feature-extraction", this.config.embeddings.model, {
dtype: "auto"
})) as unknown as FeatureExtractor;
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export async function searchMemories(query: string, options: SearchMemoriesOptio
if (!queryEmbedding && !provider.disabled) {
// Degrading to FTS-only used to be silent, which made semantic search
// look enabled while it never actually ran.
console.warn(
console.info(
"openbrain: embedding the query failed or timed out; results are FTS-only. " +
"A first search may still be downloading the local embedding model."
);
Expand Down Expand Up @@ -78,7 +78,7 @@ export async function searchMemories(query: string, options: SearchMemoriesOptio
.map((result) => result.row);

if (dimensionMismatches > 0) {
console.warn(
console.info(
`openbrain: skipped ${dimensionMismatches} memor${dimensionMismatches === 1 ? "y" : "ies"} ` +
`with embeddings that no longer match the current model (${queryEmbedding.length} dims). ` +
`Run "openbrain index rebuild" to re-embed them.`
Expand Down
2 changes: 1 addition & 1 deletion src/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ interface ApplyUpdateOptions {
export async function maybePrintUpdateNotice(options: UpdateOptions = {}) {
const notice = await getUpdateNotice(options);
if (notice) {
console.error(notice);
console.info(notice);
}
}

Expand Down
43 changes: 37 additions & 6 deletions tests/embeddings.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,39 @@
import { describe, expect, test, vi } from "vitest";
import { embedWithTimeout, enforceDimensions, serialiseEmbeds } from "../src/embeddings.js";
import { DEFAULT_CONFIG } from "../src/config.js";
import {
createEmbeddingProvider,
embedWithTimeout,
enforceDimensions,
serialiseEmbeds
} from "../src/embeddings.js";
import type { EmbeddingProvider } from "../src/types.js";

const transformers = vi.hoisted(() => ({ env: {}, pipeline: vi.fn() }));
vi.mock("@huggingface/transformers", () => transformers);

describe("embedWithTimeout", () => {
test("loads Transformers without writing warnings to stderr", async () => {
vi.stubEnv("OPENBRAIN_REAL_EMBEDDINGS", "1");
transformers.pipeline.mockResolvedValueOnce(async () => ({ data: new Float32Array(384) }));
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
const error = vi.spyOn(console, "error").mockImplementation(() => {});
const provider = createEmbeddingProvider(DEFAULT_CONFIG);
vi.unstubAllEnvs();
try {
expect(await embedWithTimeout(provider, "query", 1000)).toHaveLength(384);
expect(transformers.pipeline).toHaveBeenCalledWith(
"feature-extraction",
DEFAULT_CONFIG.embeddings.model,
{ dtype: "auto" }
);
expect(warn).not.toHaveBeenCalled();
expect(error).not.toHaveBeenCalled();
} finally {
warn.mockRestore();
error.mockRestore();
}
});

test("returns the embedding when it resolves before the timeout", async () => {
const provider: EmbeddingProvider = {
async embed() {
Expand Down Expand Up @@ -124,22 +155,22 @@ describe("enforceDimensions", () => {
expect(await enforceDimensions(inner, 3).embed("query")).toEqual([1, 2, 3]);
});

test("rejects mismatched embeddings and warns once", async () => {
test("rejects mismatched embeddings and reports once", async () => {
const inner: EmbeddingProvider = {
async embed() {
return [1, 2, 3, 4];
}
};
const provider = enforceDimensions(inner, 3);

const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
const info = vi.spyOn(console, "info").mockImplementation(() => {});
try {
expect(await provider.embed("first")).toBeNull();
expect(await provider.embed("second")).toBeNull();
expect(warn).toHaveBeenCalledTimes(1);
expect(String(warn.mock.calls[0]?.[0])).toContain("embeddings.dimensions");
expect(info).toHaveBeenCalledTimes(1);
expect(String(info.mock.calls[0]?.[0])).toContain("embeddings.dimensions");
} finally {
warn.mockRestore();
info.mockRestore();
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
});
});
25 changes: 17 additions & 8 deletions tests/openbrain.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,7 @@ describe("OpenBrain local storage", () => {
expect(results[0]?.excerpt.startsWith("12345678 alpha")).toBe(true);
});

test("warns when query embedding fails so FTS-only degradation is visible", async () => {
test("reports when query embedding fails so FTS-only degradation is visible", async () => {
const home = await tempHome();
await addMemory({ type: "workflow", text: "Deploy with the release checklist." }, options(home));
const failingEmbedder: EmbeddingProvider = {
Expand All @@ -794,13 +794,19 @@ describe("OpenBrain local storage", () => {
}
};

const info = vi.spyOn(console, "info").mockImplementation(() => {});
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
const error = vi.spyOn(console, "error").mockImplementation(() => {});
try {
const results = await searchMemories("release checklist", options(home, failingEmbedder));
expect(results).toHaveLength(1);
expect(warn.mock.calls.map((call) => String(call[0])).join("\n")).toContain("FTS-only");
expect(info.mock.calls.map((call) => String(call[0])).join("\n")).toContain("FTS-only");
expect(warn).not.toHaveBeenCalled();
expect(error).not.toHaveBeenCalled();
} finally {
info.mockRestore();
warn.mockRestore();
error.mockRestore();
}
});

Expand Down Expand Up @@ -998,16 +1004,16 @@ describe("OpenBrain local storage", () => {
expect(await listPendingReviews(options(home))).toHaveLength(0);
});

test("does not warn about FTS-only results when embeddings are disabled", async () => {
test("does not report FTS-only results when embeddings are disabled", async () => {
const home = await tempHome();
await addMemory({ type: "workflow", text: "Deploy with the release checklist." }, options(home));

const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
const info = vi.spyOn(console, "info").mockImplementation(() => {});
try {
await searchMemories("release checklist", options(home));
expect(warn.mock.calls.map((call) => String(call[0])).join("\n")).not.toContain("FTS-only");
expect(info.mock.calls.map((call) => String(call[0])).join("\n")).not.toContain("FTS-only");
} finally {
warn.mockRestore();
info.mockRestore();
}
});

Expand Down Expand Up @@ -1226,15 +1232,18 @@ describe("OpenBrain local storage", () => {
return [1, 0];
}
};
const info = vi.spyOn(console, "info").mockImplementation(() => {});
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
try {
const results = await searchMemories("pnpm TypeScript", options(home, narrowEmbedder));

expect(results[0]?.id).toBe(added.id);
expect(results[0]?.match).toBe("fts");
expect(warn).toHaveBeenCalledTimes(1);
expect(String(warn.mock.calls[0]?.[0])).toContain("index rebuild");
expect(info).toHaveBeenCalledTimes(1);
expect(String(info.mock.calls[0]?.[0])).toContain("index rebuild");
expect(warn).not.toHaveBeenCalled();
} finally {
info.mockRestore();
warn.mockRestore();
}
});
Expand Down
29 changes: 27 additions & 2 deletions tests/update.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises";
import { tmpdir } from "node:os";
import path from "node:path";
import { afterEach, describe, expect, test } from "vitest";
import { applyUpdate, getUpdateNotice, isNewerVersion, planUpdate } from "../src/update.js";
import { afterEach, describe, expect, test, vi } from "vitest";
import {
applyUpdate,
getUpdateNotice,
isNewerVersion,
maybePrintUpdateNotice,
planUpdate
} from "../src/update.js";

const tempRoots: string[] = [];

Expand Down Expand Up @@ -44,6 +50,25 @@ describe("update notice", () => {
expect(notice).toContain("openbrain update");
});

test("prints update notices without writing to stderr", async () => {
const home = await tempHome();
const info = vi.spyOn(console, "info").mockImplementation(() => {});
const error = vi.spyOn(console, "error").mockImplementation(() => {});
try {
await maybePrintUpdateNotice({
home,
currentVersion: "0.1.0",
fetch: async () => new Response(JSON.stringify({ version: "0.1.1" }), { status: 200 })
});

expect(info).toHaveBeenCalledWith(expect.stringContaining("update available"));
expect(error).not.toHaveBeenCalled();
} finally {
info.mockRestore();
error.mockRestore();
}
});

test("checks at most once per day", async () => {
const home = await tempHome();
let calls = 0;
Expand Down
Loading