fix(vectorstore): fresh index builds raced startup validation; id counter survives a missing metadata record

src/vectorstore/HNSWVectorStore.ts

@@ -286,8 +286,47 @@ export class HNSWVectorStore implements VectorStore {
 			this.dimensions = meta.dimensions;
 			this._providerId = meta.providerId;
 			this._modelId = meta.modelId;
-			this.nextHnswId = meta.nextHnswId ?? this.idToNumeric.size;
 		}
+
+		// The id counter must clear every numeric id that is or was in use, and the
+		// metadata record is not a safe sole source for it: a store can hold rows and
+		// a persisted graph without that record at all (a bulk run that never called
+		// `setMetadata` — `updateLastUpdated` then has nothing to write the counter
+		// into). Restoring 0 in that state reassigned live ids: `addPoint` threw
+		// "Node with id N already exists" for every new chunk, and each attempt had
+		// already overwritten the id mapping of the note that owned N. Observed on a
+		// resumed build: 450 graph nodes, no metadata record, 87 rows written over
+		// live ids.
+		//
+		// So take the high-water mark of everything that can hold an id. The
+		// mappings alone are not enough: `remove()` deletes a note's mappings before
+		// its rows, in separate transactions, so an interrupted removal leaves a row
+		// and a persisted graph node with no mapping — and `loadGraph` keeps a node
+		// whose row exists. Each read is one reverse key-cursor step.
+		const [maxMapped, maxGraphNode] = await Promise.all([this.maxKey(ID_MAPPING_STORE), this.maxKey(GRAPH_STORE)]);
+		this.nextHnswId = Math.max(meta?.nextHnswId ?? 0, maxMapped + 1, maxGraphNode + 1);
+	}
+
+	/** Largest numeric primary key in a store, or -1 when it is empty. */
+	private async maxKey(storeName: string): Promise<number> {
+		const db = this.requireDb();
+		const tx = db.transaction(storeName, "readonly");
+		const request = tx.objectStore(storeName).openKeyCursor(null, "prev");
+		return awaitCursor(tx, request, (cursor) => ({
+			done: true,
+			value: cursor && typeof cursor.key === "number" ? cursor.key : -1,
+		}));
+	}
+
+	/** Vector width of the first stored row, or null for an empty store. */
+	private async probeDimensions(): Promise<number | null> {
+		const db = this.requireDb();
+		const tx = db.transaction(DOCUMENTS_STORE, "readonly");
+		const request = tx.objectStore(DOCUMENTS_STORE).openCursor();
+		return awaitCursor(tx, request, (cursor) => ({
+			done: true,
+			value: cursor ? ((cursor as IDBCursorWithValue).value as StoredDocument).vector.length : null,
+		}));
 	}
 
 	/**
@@ -679,6 +718,11 @@ export class HNSWVectorStore implements VectorStore {
 		this._providerId = providerId;
 		this._modelId = modelId;
 
+		// A record written to repair a store that already holds rows must carry
+		// their width, or the next open leaves `dimensions` unset and the graph is
+		// never loaded until something is written. An empty store stays at 0.
+		if (!this.dimensions) this.dimensions = await this.probeDimensions();
+
 		const meta: StoredMetadata = {
 			key: "metadata",
 			version,

src/vectorstore/VectorStoreService.ts

@@ -245,6 +245,8 @@ interface IndexInstance {
 	validationScheduled: boolean;
 	/** A full build (`buildFullIndex`) is in progress. Validation runs set only `progress.isIndexing`. */
 	isIndexing: boolean;
+	/** The full build in flight, so a second caller joins it instead of racing it. */
+	buildPromise: Promise<void> | null;
 	progress: IndexingProgress;
 	/** Timestamp (ms) when the current indexing run started, for ETA estimation */
 	indexingStartedAt: number | null;
@@ -432,6 +434,7 @@ export class VectorStoreService {
 			hasValidatedThisSession: false,
 			validationScheduled: false,
 			isIndexing: false,
+			buildPromise: null,
 			progress: {
 				isIndexing: false,
 				total: 0,
@@ -845,6 +848,28 @@ export class VectorStoreService {
 			indexedMap.set(note.path, { mtime: note.mtime });
 		}
 
+		// An empty store is not a repair job, it is a first build, and the two paths
+		// differ in what they leave behind: only the full build writes the store's
+		// metadata record (the version, dimensions and the id counter — see
+		// `HNSWVectorStore.open`) and stamps `lastBuiltAt`. This was reachable for
+		// every newly added index: "Add index" runs `ensureIndex`, whose opening of
+		// the instance schedules this validation on a zero-delay timer, and the
+		// timer wins the race against `ensureIndex`'s two worker round-trips.
+		if (indexedMap.size === 0 && vaultFiles.length > 0) {
+			Logger.log(`[VectorStore] ${inst.indexId} is empty — running a full build instead of a repair`);
+			await this.buildFullIndex(inst, embeddings, defaultModel);
+			return;
+		}
+
+		// A store built through the old validation path has rows but no metadata
+		// record, so nothing persists its dimensions or id counter. Repair that
+		// here, whether or not any note needs embedding, so the next open restores
+		// both the normal way.
+		if (indexedMap.size > 0 && (await inst.store.getMetadata()) === null) {
+			Logger.log(`[VectorStore] ${inst.indexId} has rows but no metadata record — writing it`);
+			await inst.store.setMetadata(defaultModel.provider, defaultModel.model, INDEX_VERSION);
+		}
+
 		const missingFiles: TFile[] = [];
 		const staleFiles: TFile[] = [];
 		const vaultPaths = new Set<string>();
@@ -1283,16 +1308,30 @@ export class VectorStoreService {
 	 * Build the full index for a specific instance. The store has been cleared
 	 * (or is empty) when this is called, so chunks are written without purging.
 	 */
-	private async buildFullIndex(
+	private buildFullIndex(
 		inst: IndexInstance,
 		embeddings: EmbeddingsInterface,
 		model: DefaultEmbedModel,
 	): Promise<void> {
+		// One build per instance. The startup validation and `ensureIndex` can both
+		// decide an empty store needs building; the second joins the first rather
+		// than reporting "already in progress" against its own twin.
+		if (inst.buildPromise) return inst.buildPromise;
 		if (inst.isIndexing || inst.progress.isIndexing) {
 			new Notice("Indexing already in progress...");
-			return;
+			return Promise.resolve();
 		}
+		inst.buildPromise = this.runFullBuild(inst, embeddings, model).finally(() => {
+			inst.buildPromise = null;
+		});
+		return inst.buildPromise;
+	}
 
+	private async runFullBuild(
+		inst: IndexInstance,
+		embeddings: EmbeddingsInterface,
+		model: DefaultEmbedModel,
+	): Promise<void> {
 		inst.isIndexing = true;
 		inst.abortController = new AbortController();
 		const { vault } = this.plugin.app;

test/vectorstore/HNSWVectorStore.open.test.ts

@@ -42,6 +42,8 @@ function makeConnection() {
 				objectStore: () => ({
 					getAll: () => makeReq([]),
 					get: () => makeReq(undefined),
+					// `open()` reads each id store's high-water mark; null = empty.
+					openKeyCursor: () => makeReq(null),
 				}),
 				onerror: null as null | (() => void),
 				onabort: null as null | (() => void),

test/vectorstore/HNSWVectorStore.resume.test.ts

@@ -109,6 +109,60 @@ describe("HNSWVectorStore — resuming after an interrupted build", () => {
 		await store.close();
 	});
 
+	it("keeps allocating fresh ids after a reopen when no metadata record was ever written", async () => {
+		// The old validation path embedded into a fresh store without `setMetadata`,
+		// so nothing persisted the id counter. Reopening restored 0 and every new
+		// chunk collided with a live graph node ("Node with id N already exists").
+		const first = await openStore(); // no setMetadata: rows and graph only
+		await first.upsert(doc("a.md", [1, 0, 0]));
+		await first.upsert(doc("b.md", [0, 1, 0]));
+		await first.flush();
+
+		const second = await openStore();
+		await expect(second.upsert(doc("c.md", [0, 0, 1]))).resolves.toBeUndefined();
+		expect(await second.count()).toBe(3);
+		expect(graphNodeCount(second)).toBe(3);
+		const hits = await second.search(new Float32Array([0, 0, 1]), 1);
+		expect(hits.map((h) => h.doc.path)).toEqual(["c.md"]);
+		// a.md still resolves to its own row: no mapping was overwritten.
+		const older = await second.search(new Float32Array([1, 0, 0]), 1);
+		expect(older.map((h) => h.doc.path)).toEqual(["a.md"]);
+		await second.close();
+	});
+
+	it("clears a graph node whose mapping was lost to an interrupted removal", async () => {
+		// `remove()` drops mappings before rows in separate transactions; a kill in
+		// between leaves c's row and graph node with no mapping. Its id must still
+		// be off limits on the next open, or the next upsert collides with it.
+		const first = await openStore();
+		await first.upsert(doc("a.md", [1, 0, 0]));
+		await first.upsert(doc("b.md", [0, 1, 0]));
+		await first.upsert(doc("c.md", [0, 0, 1])); // numeric id 2, the high-water mark
+		await first.flush();
+		const dbName = (first as unknown as { dbName: string }).dbName;
+		await new Promise<void>((resolve, reject) => {
+			const open = indexedDB.open(dbName);
+			open.onerror = () => reject(open.error);
+			open.onsuccess = () => {
+				const db = open.result;
+				const tx = db.transaction("id_mapping", "readwrite");
+				tx.objectStore("id_mapping").delete(2);
+				tx.oncomplete = () => {
+					db.close();
+					resolve();
+				};
+				tx.onerror = () => reject(tx.error);
+			};
+		});
+
+		const second = await openStore();
+		await expect(second.upsert(doc("d.md", [1, 1, 0]))).resolves.toBeUndefined();
+		expect(await second.count()).toBe(4);
+		const hits = await second.search(new Float32Array([1, 1, 0]), 1);
+		expect(hits.map((h) => h.doc.path)).toEqual(["d.md"]);
+		await second.close();
+	});
+
 	it("flush() persists the pending graph immediately instead of on the debounce", async () => {
 		const first = await openForBuild();
 		await first.upsert(doc("a.md", [1, 0]));

test/vectorstore/VectorStoreService.lazyOpen.test.ts

@@ -418,4 +418,58 @@ describe("bulk embed run", () => {
 		await startService();
 		expect(indexStats.documentCount).toBe(1);
 	});
+
+	it("a fresh index gets exactly one full build when startup validation fires before ensureIndex", async () => {
+		platform.isMobile = false;
+		vaultFiles = [file("a.md"), file("b.md"), file("c.md")];
+		const svc = await startService();
+		const store = stores.get(INDEX);
+		if (!store) throw new Error("store not opened");
+		// The real store answers over a worker round-trip, so `ensureIndex`'s reads
+		// land after the zero-delay validation timer registered at open. Model that
+		// by parking the two reads on a (fake) macrotask.
+		for (const method of ["getMetadata", "count"] as const) {
+			const original = store[method].bind(store);
+			(store as unknown as Record<string, unknown>)[method] = async () => {
+				await new Promise((resolve) => setTimeout(resolve, 0));
+				return original();
+			};
+		}
+
+		const run = svc.ensureIndex(INDEX);
+		await vi.advanceTimersByTimeAsync(2_000);
+		expect(await run).toBe(true);
+
+		// Every note embedded once — one build, not a validation run racing a build.
+		const embedded = embedDocuments.mock.calls.flatMap(([texts]) => texts);
+		expect(embedded).toHaveLength(3);
+		expect(await store.countNotes()).toBe(3);
+		// And it was the *full build*: the metadata record and the build date exist.
+		expect(store.meta).not.toBeNull();
+		expect(indexStats.lastBuiltAt).toEqual(expect.any(Number));
+	});
+
+	it("writes the missing metadata record of a store that has rows, even with nothing to embed", async () => {
+		platform.isMobile = false;
+		vaultFiles = [file("a.md", 1_000)];
+		const svc = await startService();
+		const store = stores.get(INDEX);
+		if (!store) throw new Error("store not opened");
+		// Built through the old validation path: a row, no record.
+		await store.upsert({
+			id: "a.md#0",
+			path: "a.md",
+			mtime: 1_000,
+			checksum: "x",
+			chunkIndex: 0,
+			vector: new Float32Array(3),
+		});
+		expect(store.meta).toBeNull();
+
+		expect(await svc.ensureIndex(INDEX)).toBe(true);
+		await vi.advanceTimersByTimeAsync(1_000);
+
+		expect(embedDocuments).not.toHaveBeenCalled();
+		expect(store.meta).toMatchObject({ providerId: "fake", modelId: "embed-model" });
+	});
 });