From 449243f3e95b4bb01f7f45747a10cd9360da07f5 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Tue, 21 Jul 2026 14:54:30 -0400
Subject: [PATCH] Confirm before wiping the index on a Best-Match mode change

Changing the mode -- including disabling -- silently dropped the
stored embeddings and any other downloaded model, costing a full
reindex. Prompt first, since the menu click gives no hint of the cost.
---
 .../preferences/preferences_advanced.js       | 54 +++++++++++++++++--
 .../preferences/preferences_advanced.xhtml    |  3 +-
 chrome/locale/en-US/zotero/preferences.ftl    |  6 +++
 test/tests/preferences_advancedTest.js        | 40 ++++++++++++++
 4 files changed, 97 insertions(+), 6 deletions(-)

diff --git a/chrome/content/zotero/preferences/preferences_advanced.js b/chrome/content/zotero/preferences/preferences_advanced.js
index d41391440f..bec6606527 100644
--- a/chrome/content/zotero/preferences/preferences_advanced.js
+++ b/chrome/content/zotero/preferences/preferences_advanced.js
@@ -63,23 +63,33 @@ Zotero_Preferences.Advanced = {
 
 
 	initSemanticSearch: function () {
-		// Populate the model menu from the model registry. The preferences
-		// code watches bound menulists for added items and selects the one
-		// matching the current pref value.
-		let modelPopup = document.querySelector('#semantic-search-model > menupopup');
+		// Populate the model menu from the model registry. The menu isn't
+		// bound to the pref, since a mode change needs a confirmation prompt
+		// before the pref -- and with it the stored index -- is touched.
+		let modelMenu = document.getElementById('semantic-search-model');
+		let modelPopup = modelMenu.querySelector('menupopup');
 		for (let { name, l10nID } of Zotero.Embeddings.getAvailableModels()) {
 			let menuitem = document.createXULElement('menuitem');
 			menuitem.setAttribute('value', name);
 			document.l10n.setAttributes(menuitem, l10nID);
 			modelPopup.append(menuitem);
 		}
+		modelMenu.value = Zotero.Embeddings.getModelName();
+		modelMenu.addEventListener('command', () => this.handleSemanticSearchModeChange());
 
 		// Live progress updates from the background indexer
 		this._semanticSearchListener = status => this.updateSemanticSearchUI(status);
 		Zotero.Embeddings.Indexing.addProgressListener(this._semanticSearchListener);
+		let modelPrefObserverID = Zotero.Prefs.registerObserver(
+			'embeddings.model',
+			() => {
+				modelMenu.value = Zotero.Embeddings.getModelName();
+			}
+		);
 
 		document.getElementById('zotero-prefpane-advanced').addEventListener('unload', () => {
 			Zotero.Embeddings.Indexing.removeProgressListener(this._semanticSearchListener);
+			Zotero.Prefs.unregisterObserver(modelPrefObserverID);
 		});
 
 		document.getElementById('semantic-search-resume').addEventListener('command', () => {
@@ -96,6 +106,42 @@ Zotero_Preferences.Advanced = {
 	},
 
 
+	handleSemanticSearchModeChange: async function () {
+		let modelMenu = document.getElementById('semantic-search-model');
+		let oldValue = Zotero.Embeddings.getModelName();
+		let newValue = modelMenu.value;
+		if (newValue === oldValue) {
+			return;
+		}
+		// Leaving an enabled mode wipes the stored index (and, when disabling,
+		// the downloaded data), so confirm first. Enabling from Disabled just
+		// starts indexing.
+		if (Zotero.Embeddings.isEnabled()) {
+			let [title, text, button] = await document.l10n.formatValues(
+				(newValue
+					? ['switch-title', 'switch-text', 'switch-button']
+					: ['disable-title', 'disable-text', 'disable-button'])
+					.map(id => ({ id: `preferences-advanced-semantic-search-${id}` }))
+			);
+			let ps = Services.prompt;
+			let index = ps.confirmEx(
+				window,
+				title,
+				text,
+				ps.BUTTON_POS_0 * ps.BUTTON_TITLE_IS_STRING
+					+ ps.BUTTON_POS_1 * ps.BUTTON_TITLE_CANCEL
+					+ ps.BUTTON_POS_1_DEFAULT,
+				button, null, null, null, {}
+			);
+			if (index !== 0) {
+				modelMenu.value = oldValue;
+				return;
+			}
+		}
+		Zotero.Prefs.set('embeddings.model', newValue);
+	},
+
+
 	updateSemanticSearchUI: function (status) {
 		let statusBox = document.getElementById('semantic-search-status');
 		statusBox.hidden = !status.enabled;
diff --git a/chrome/content/zotero/preferences/preferences_advanced.xhtml b/chrome/content/zotero/preferences/preferences_advanced.xhtml
index 7e9a9a7fab..7a66fa47d1 100644
--- a/chrome/content/zotero/preferences/preferences_advanced.xhtml
+++ b/chrome/content/zotero/preferences/preferences_advanced.xhtml
@@ -298,8 +298,7 @@
 			<label><html:h2 data-l10n-id="preferences-advanced-semantic-search-title"/></label>
 			<hbox align="center">
 				<label data-l10n-id="preferences-advanced-semantic-search-model" control="semantic-search-model"/>
-				<menulist id="semantic-search-model"
-					preference="extensions.zotero.embeddings.model" native="true">
+				<menulist id="semantic-search-model" native="true">
 					<menupopup>
 						<menuitem value="" data-l10n-id="preferences-advanced-semantic-search-disabled"/>
 						<!-- Model items are populated from Zotero.Embeddings.getAvailableModels() -->
diff --git a/chrome/locale/en-US/zotero/preferences.ftl b/chrome/locale/en-US/zotero/preferences.ftl
index 16a15368ab..f0f338b83a 100644
--- a/chrome/locale/en-US/zotero/preferences.ftl
+++ b/chrome/locale/en-US/zotero/preferences.ftl
@@ -111,6 +111,12 @@ preferences-advanced-semantic-search-resume =
     .label = Resume
 preferences-advanced-semantic-search-stop =
     .label = Stop
+preferences-advanced-semantic-search-switch-title = Change Mode?
+preferences-advanced-semantic-search-switch-text = Changing the mode will rebuild the search index, which can take a long time for large libraries.
+preferences-advanced-semantic-search-switch-button = Change Mode
+preferences-advanced-semantic-search-disable-title = Disable Best-Match Search?
+preferences-advanced-semantic-search-disable-text = Disabling will delete the search index and downloaded data.
+preferences-advanced-semantic-search-disable-button = Disable
 preferences-advanced-language-and-region-title = Language and Region
 preferences-advanced-enable-bidi-ui =
     .label = Enable bidirectional text editing utilities
diff --git a/test/tests/preferences_advancedTest.js b/test/tests/preferences_advancedTest.js
index 972cbe57b3..7c1464e8dd 100644
--- a/test/tests/preferences_advancedTest.js
+++ b/test/tests/preferences_advancedTest.js
@@ -154,4 +154,44 @@ describe("Advanced Preferences", function () {
 			});
 		})
 	})
+
+	describe("Best-Match Search", function () {
+		it("should confirm mode changes only when leaving an enabled mode", async function () {
+			var stubs = [
+				sinon.stub(Zotero.Embeddings.Indexing, 'startIndexing').resolves(),
+				sinon.stub(Zotero.Embeddings, 'pruneModels').resolves()
+			];
+			var win = await loadPrefPane('advanced');
+			var menu = win.document.getElementById('semantic-search-model');
+			try {
+				// Enabling from Disabled prompts nothing (the test would hang on
+				// an unexpected modal prompt)
+				menu.value = 'bge-small-en-v1.5';
+				await win.Zotero_Preferences.Advanced.handleSemanticSearchModeChange();
+				assert.equal(Zotero.Prefs.get('embeddings.model'), 'bge-small-en-v1.5');
+
+				// Cancelling a switch restores the menu and leaves the pref alone
+				var promise = waitForDialog(null, 'cancel');
+				menu.value = 'multilingual-e5-small';
+				await win.Zotero_Preferences.Advanced.handleSemanticSearchModeChange();
+				await promise;
+				assert.equal(Zotero.Prefs.get('embeddings.model'), 'bge-small-en-v1.5');
+				assert.equal(menu.value, 'bge-small-en-v1.5');
+
+				// Confirming applies the change
+				promise = waitForDialog();
+				menu.value = '';
+				await win.Zotero_Preferences.Advanced.handleSemanticSearchModeChange();
+				await promise;
+				assert.equal(Zotero.Prefs.get('embeddings.model'), '');
+			}
+			finally {
+				win.close();
+				Zotero.Prefs.set('embeddings.model', '');
+				await Zotero.Embeddings.Indexing.waitForPendingModelSwitch();
+				Zotero.Prefs.clear('embeddings.indexingPaused');
+				stubs.forEach(stub => stub.restore());
+			}
+		});
+	})
 })