From 28ff8969e49432fc9b2128a8a5edd3bf1369194c Mon Sep 17 00:00:00 2001 From: John Trujillo Date: Mon, 17 Aug 2026 16:13:30 -0500 Subject: [PATCH] fix(ai): never substitute another LLM backend for the selected one AUTO routing and the chat's availability check both stepped past an unready backend to whichever other one happened to be configured, sending the prompt and attached source to a provider the user did not choose. Both now resolve the selected backend only and fail naming it. --- .../plugins/aicore/backends/AiBackend.kt | 22 +-- .../aicore/backends/BackendRegistry.kt | 35 ++++ .../services/LlmInferenceServiceImpl.kt | 76 ++++++--- .../plugins/aicore/viewmodel/ChatViewModel.kt | 153 +++++++++++------- ai-core/src/main/res/values/strings.xml | 4 +- .../plugins/aicore/backends/AiBackendTest.kt | 8 +- .../aicore/backends/BackendRegistryTest.kt | 92 +++++++++++ .../services/LlmInferenceServiceImplTest.kt | 104 +++++++++++- 8 files changed, 392 insertions(+), 102 deletions(-) create mode 100644 ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/backends/BackendRegistryTest.kt diff --git a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/backends/AiBackend.kt b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/backends/AiBackend.kt index 832727c4..6d65465b 100644 --- a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/backends/AiBackend.kt +++ b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/backends/AiBackend.kt @@ -56,16 +56,20 @@ object AiBackend { * The backend to act on given what is installed, for every caller that has to answer that * question: the settings selector, the chat's status line, and its availability check. * - * They must agree, or one launch can route to one backend while the label names another. The - * order is deliberate — the stored choice first, then [DEFAULT_ID], and only then whatever came - * first, which is registration or sort order and so effectively arbitrary. + * They must agree, or one launch can route to one backend while the label names another. + * + * A stored selection is honoured or nothing is: answering "some other installed one" would send + * the prompt, and the attached source with it, to a provider the user did not choose — silently, + * for the consumers that route by [AUTO]. Only when *nothing* is stored is there a choice to + * make, and then it is [DEFAULT_ID] first, then whatever came first; pass [installedIds] in the + * selector's own order, or that last resort answers differently per caller. * * @param storedId the persisted selection, or null when nothing has been chosen - * @param installedIds ids of the backends currently registered - * @return the id to act on, or null when nothing is installed + * @param installedIds ids of the backends currently registered, in the selector's order + * @return the id to act on; null when nothing is installed, or when the stored selection is */ - fun preferredId(storedId: String?, installedIds: Collection): String? = - storedId?.takeIf { it in installedIds } - ?: DEFAULT_ID.takeIf { it in installedIds } - ?: installedIds.firstOrNull() + fun preferredId(storedId: String?, installedIds: Collection): String? { + if (storedId != null) return storedId.takeIf { it in installedIds } + return DEFAULT_ID.takeIf { it in installedIds } ?: installedIds.firstOrNull() + } } diff --git a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/backends/BackendRegistry.kt b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/backends/BackendRegistry.kt index 7ffefcd1..36099e28 100644 --- a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/backends/BackendRegistry.kt +++ b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/backends/BackendRegistry.kt @@ -21,6 +21,25 @@ data class BackendOption( val classLoader: ClassLoader?, ) +/** + * What the stored backend selection resolves to against what is installed right now. + * + * The three cases are three different fixes for the user, so callers that report the selection have + * to tell them apart: configure the chosen backend, install the plugin it came from, or install any + * backend at all. + */ +sealed interface SelectedBackend { + + /** The selected backend, installed and registered. */ + data class Installed(val option: BackendOption) : SelectedBackend + + /** A selection is stored, but no installed backend answers to it. */ + data object Missing : SelectedBackend + + /** No backend is installed to select. */ + data object None : SelectedBackend +} + /** * AI Core's own view of the backends currently installed: the registry the settings selector, the * chat's status line and its availability check all read. @@ -68,6 +87,22 @@ object BackendRegistry { return options.firstOrNull { it.id == id } } + /** + * The selection resolved against what is installed, as the one question a caller has to ask. + * + * Resolved from a single snapshot: asking [preferred] and [selectedId] as two separate + * questions reads the registry and the preference twice, and a backend registering in between + * let the two answers disagree — reporting "not installed" about a backend that just appeared. + * + * @param storedId the persisted selection; defaults to the stored one + * @return which of the three [SelectedBackend] cases holds now + */ + @JvmOverloads + fun selected(storedId: String? = selectedId()): SelectedBackend { + preferred(options(), storedId)?.let { return SelectedBackend.Installed(it) } + return if (storedId != null) SelectedBackend.Missing else SelectedBackend.None + } + /** * The loader that can instantiate [fragmentClassName], found by asking each backend plugin's * loader whether it can see the class. Used by [BackendFragmentFactory] to rebuild a restored diff --git a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/services/LlmInferenceServiceImpl.kt b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/services/LlmInferenceServiceImpl.kt index 2a559252..d9dd0c0c 100644 --- a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/services/LlmInferenceServiceImpl.kt +++ b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/services/LlmInferenceServiceImpl.kt @@ -2,7 +2,7 @@ package com.itsaky.androidide.plugins.aicore.services import com.itsaky.androidide.plugins.PluginLogger import com.itsaky.androidide.plugins.aicore.backends.AiBackend -import com.itsaky.androidide.plugins.aicore.plugin.AiCorePlugin +import com.itsaky.androidide.plugins.aicore.backends.BackendRegistry import com.itsaky.androidide.plugins.services.LlmInferenceService import com.itsaky.androidide.plugins.services.LlmInferenceService.* import java.util.concurrent.CompletableFuture @@ -31,9 +31,19 @@ class LlmInferenceServiceImpl(private val logger: PluginLogger? = null) : LlmInf backends.remove(backendId) } - override fun getAvailableBackends(): List { - return backends.values.toList() - } + /** + * Every registered backend, in the order the settings selector lists them. + * + * Sorted here rather than left in this map's hash order, so a caller that lists them or takes + * the first one gets the same answer on every launch instead of an arbitrary backend. + * + * @return the registered backends, sorted by display name, tolerating a backend that throws + * from its own accessor by sorting it under its id + */ + override fun getAvailableBackends(): List = + backends.entries + .sortedBy { (id, backend) -> runCatching { backend.name }.getOrNull() ?: id } + .map { (_, backend) -> backend } override fun getBackend(backendId: String): LlmBackend? { return backends[backendId] @@ -162,43 +172,57 @@ class LlmInferenceServiceImpl(private val logger: PluginLogger? = null) : LlmInf } /** - * Resolves the backend id a request should run on. An explicit id is returned unchanged - * (so the caller keeps its "not found"/"not available" errors); [AiBackend.AUTO] is - * resolved to the user-selected backend, then to any available backend, so callers can - * defer backend choice to AI Core instead of hardcoding one. + * Resolves the backend id a request should run on. An explicit id is returned unchanged; + * [AiBackend.AUTO] resolves to the selected backend the same way the settings screen and the + * chat's status line resolve it, so all three name one backend. + * + * Availability is deliberately not consulted: stepping past an unready backend to another that + * happens to answer would send the prompt to a provider the user did not choose, so an unready + * selection has to fail here instead. * * @param requestedId the id from [LlmConfig.backendId] - * @return the id to route to; for AUTO with nothing available, the selected backend's id - * so the downstream "not available" error stays meaningful + * @return the id to route to, unavailable or not, so the caller's "not found"/"not available" + * error names the backend the user actually selected */ private fun effectiveBackendId(requestedId: String): String { if (requestedId != AiBackend.AUTO) return requestedId - val preferredId = AiBackend.idFromPreference(readSelectedBackendPreference()) - return backends[preferredId]?.takeIf { it.isAvailable() }?.getId() - ?: backends.values.firstOrNull { it.isAvailable() }?.getId() - ?: preferredId + val storedId = getPreferredBackendId() + // Falling back to the stored id keeps the failure naming the backend the user selected, + // rather than the default one, when its plugin is no longer installed. + return AiBackend.preferredId(storedId, installedIdsInSelectorOrder()) + ?: storedId + ?: AiBackend.DEFAULT_ID } /** - * The backend the user selected on the Agent settings screen. + * Installed ids in the order the settings selector lists them, which is by display name. * - * Published so a backend can find out whether it is the active one without reading this - * plugin's preferences — see [LlmInferenceService.getPreferredBackendId]. + * With nothing stored yet, [AiBackend.preferredId] falls back to the first id offered, so the + * order decides the answer. Handing it this map's own keys would hand it a hash order, and AUTO + * would route to a backend other than the one the selector and the chat's status line name. * - * @return the selected backend id, or null when nothing has been chosen yet + * @return every registered id, sorted by display name, tolerating a backend that throws from + * its own accessor by sorting it under its id */ - override fun getPreferredBackendId(): String? = - readSelectedBackendPreference()?.let(AiBackend::idFromPreference) + private fun installedIdsInSelectorOrder(): List = + backends.entries + .map { (id, backend) -> id to (runCatching { backend.name }.getOrNull() ?: id) } + .sortedBy { (_, displayName) -> displayName } + .map { (id, _) -> id } /** - * Reads the raw stored selection from this plugin's own preferences. + * The backend the user selected on the Agent settings screen. + * + * Published so a backend can find out whether it is the active one without reading this + * plugin's preferences — see [LlmInferenceService.getPreferredBackendId]. * - * @return the stored preference value, or null when unset or before initialization + * Read through the registry rather than from the preference file directly, so "nothing has been + * chosen" means the same here as it does on the settings screen; resolution turns on that + * distinction now, and reading the file twice let the two answer differently on a blank value. + * + * @return the selected backend id, or null when nothing has been chosen yet */ - private fun readSelectedBackendPreference(): String? = - AiCorePlugin.getContext() - ?.getPluginSharedPreferences(AiBackend.PREFERENCE_FILE) - ?.getString(AiBackend.PREFERENCE_KEY, null) + override fun getPreferredBackendId(): String? = BackendRegistry.selectedId() override fun cancelGeneration() { currentGeneration?.cancel(true) diff --git a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/ChatViewModel.kt b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/ChatViewModel.kt index 3e5764f6..6d374964 100644 --- a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/ChatViewModel.kt +++ b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/ChatViewModel.kt @@ -6,6 +6,7 @@ import com.itsaky.androidide.plugins.PluginContext import com.itsaky.androidide.plugins.aicore.R import com.itsaky.androidide.plugins.aicore.backends.AiBackend import com.itsaky.androidide.plugins.aicore.backends.BackendRegistry +import com.itsaky.androidide.plugins.aicore.backends.SelectedBackend import com.itsaky.androidide.plugins.aicore.logging.AgentTrace import com.itsaky.androidide.plugins.aicore.logging.LOG_PREFIX import com.itsaky.androidide.plugins.aicore.managers.ChatStorageManager @@ -51,6 +52,7 @@ import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.isActive import kotlinx.coroutines.launch @@ -141,8 +143,10 @@ class ChatViewModel( private val _agentState = MutableStateFlow(AgentState.Idle) val agentState: StateFlow = _agentState.asStateFlow() - private val _isBackendAvailable = MutableStateFlow(false) - val isBackendAvailable: StateFlow = _isBackendAvailable.asStateFlow() + private val _backendStatus = MutableStateFlow(BackendStatus(AiBackend.DEFAULT_ID, false)) + val isBackendAvailable: StateFlow = _backendStatus + .map { it.isAvailable } + .stateIn(viewModelScope, SharingStarted.Eagerly, false) private val _sessions = MutableStateFlow>(emptyList()) val sessions: StateFlow> = _sessions.asStateFlow() @@ -159,12 +163,20 @@ class ChatViewModel( }.stateIn(viewModelScope, SharingStarted.Lazily, null) /** - * Backend the last availability check resolved. Volatile because that check runs on IO and - * [sendMessage] reads it on Main; a stale read would build the request for a different backend - * than the one just found. + * What the last availability check resolved: which backend to send to, and whether it is ready. + * + * One value, not two fields. The check runs on IO and [sendMessage] reads it on Main, and a + * superseded run writing one field after a newer run wrote both would leave the id naming one + * backend while the verdict describes another. + * + * @param id the backend the request must be built for + * @param isAvailable whether that backend is configured and ready */ - @Volatile - private var currentBackendId: String = AiBackend.DEFAULT_ID + private data class BackendStatus(val id: String, val isAvailable: Boolean) + + /** Backend the last availability check resolved; see [BackendStatus]. */ + private val currentBackendId: String + get() = _backendStatus.value.id /** * Label for the backend the user *selected* in settings, shown under the chat input. Tracks the @@ -174,14 +186,16 @@ class ChatViewModel( private val _activeBackendLabel = MutableStateFlow(selectedBackendLabel()) val activeBackendLabel: StateFlow = _activeBackendLabel.asStateFlow() - private fun selectedBackendLabel(): String { + private fun selectedBackendLabel(): String = // Resolved exactly as the settings screen and the availability check resolve it, so the // three cannot name different backends on the same launch. - val selected = BackendRegistry.preferred(BackendRegistry.options()) - return selected?.displayName - ?: getContext()?.androidContext?.getString(R.string.backend_none_installed_short) - ?: "" - } + when (val selected = BackendRegistry.selected()) { + is SelectedBackend.Installed -> selected.option.displayName + // A stored selection resolving to nothing means its plugin is gone. Saying "no backend" + // there would read as "install one" when one is installed — just not the chosen one. + SelectedBackend.Missing -> str(R.string.backend_selected_missing_short) + SelectedBackend.None -> str(R.string.backend_none_installed_short) + } /** Re-read the selected backend and update [activeBackendLabel]; call when returning to chat. */ fun refreshBackendLabel() { @@ -200,6 +214,15 @@ class ChatViewModel( /** The in-flight backend availability check, so a resume can supersede the previous one. */ private var backendCheckJob: Job? = null + + /** + * Sequence number of the newest availability check, identifying which run may publish. + * + * Main-thread only: handed out in [checkBackendAvailability] and tested in + * [publishBackendStatus], both on Main, so a superseded run cannot slip a write past the test. + */ + private var backendCheckSequence = 0 + private val generationEpoch = java.util.concurrent.atomic.AtomicInteger(0) /** True while a generation is admitted and its coroutine has not yet unwound; gates re-entry. */ @@ -575,9 +598,13 @@ class ChatViewModel( } /** - * Check if any LLM backend is available. - * Should be called when the fragment becomes visible. - * Retries with delays to handle plugin loading timing. + * Check whether the backend the user *selected* is available. The chat sends to that backend or + * to none: substituting whichever other backend happened to be configured would hand the + * prompt, and the source files with it, to a provider the user did not choose. + * + * Should be called when the fragment becomes visible. Retries with delays while the selection + * is not registered yet, to absorb plugin loading order, but settles at once once it is + * registered — "registered but unconfigured" is an answer, not a race. * * The previous run is cancelled first: every resume starts one, and a run still inside its * retry loop would otherwise write its stale verdict over a newer one — leaving the chat @@ -586,6 +613,7 @@ class ChatViewModel( fun checkBackendAvailability() { android.util.Log.d(TAG, "checkBackendAvailability: Starting check") backendCheckJob?.cancel() + val sequence = ++backendCheckSequence backendCheckJob = viewModelScope.launch(Dispatchers.IO) { // Retry up to 5 times with 500ms delays to handle plugin loading order repeat(5) { attempt -> @@ -598,42 +626,20 @@ class ChatViewModel( // status line resolve it. Going to the service's own list instead would // hand AiBackend.preferredId a hash-ordered collection, and with nothing // stored the two would answer differently on the same launch. - val options = BackendRegistry.options() - android.util.Log.d(TAG, "checkBackendAvailability: Found ${options.size} backends") - - val preferredBackendId = BackendRegistry.preferred(options)?.id - android.util.Log.d(TAG, "checkBackendAvailability: Preferred backend = $preferredBackendId") - - // Same order as the selector, so the fallback below is reproducible too. - val backends = options.mapNotNull { llmService.getBackend(it.id) } - - // First try to use the preferred backend - var foundAvailable = false - val preferredBackend = backends.find { it.id == preferredBackendId } - android.util.Log.d(TAG, "checkBackendAvailability: Preferred backend (${preferredBackendId}) found=${preferredBackend != null}, available=${preferredBackend?.isAvailable}") - if (preferredBackend != null && preferredBackend.isAvailable) { - if (!isActive) return@launch - _isBackendAvailable.value = true - currentBackendId = preferredBackend.id - android.util.Log.d(TAG, "checkBackendAvailability: Using preferred backend ${preferredBackend.id}") - return@launch // Success, exit retry loop - } - - // If preferred backend not available, try any available backend as fallback - for (backend in backends) { - android.util.Log.d(TAG, "checkBackendAvailability: Checking backend ${backend.id}, available=${backend.isAvailable}") - if (backend.isAvailable) { - if (!isActive) return@launch - _isBackendAvailable.value = true - currentBackendId = backend.id - foundAvailable = true - android.util.Log.d(TAG, "checkBackendAvailability: Using fallback backend ${backend.id}") - break + val selected = BackendRegistry.selected() + android.util.Log.d(TAG, "checkBackendAvailability: Selection resolved to $selected") + + val selectedId = (selected as? SelectedBackend.Installed)?.option?.id + val backend = selectedId?.let { llmService.getBackend(it) } + android.util.Log.d(TAG, "checkBackendAvailability: Preferred backend ($selectedId) found=${backend != null}, available=${backend?.isAvailable}") + if (backend != null) { + // Id set even when unavailable: a stale id from an earlier check would + // otherwise build the next request for a backend since moved off. + val published = publishBackendStatus(sequence) { + BackendStatus(backend.id, backend.isAvailable) } - } - - if (foundAvailable) { - return@launch // Success, exit retry loop + android.util.Log.d(TAG, "checkBackendAvailability: Selected backend ${backend.id}, available=${backend.isAvailable}, published=$published") + return@launch // Answered — available or not, there is no substitute } } catch (e: Exception) { android.util.Log.e(TAG, "Error checking backends on attempt ${attempt + 1}: ${e.message}", e) @@ -647,12 +653,35 @@ class ChatViewModel( } // All retries failed - android.util.Log.d(TAG, "checkBackendAvailability: All retries failed, no backend available") - if (!isActive) return@launch - _isBackendAvailable.value = false + android.util.Log.d(TAG, "checkBackendAvailability: No backend registered for the selection") + // Keeps whichever id is on record: nothing was resolved to replace it with. + publishBackendStatus(sequence) { it.copy(isAvailable = false) } } } + /** + * Write this check's verdict, unless a newer check has started. + * + * Confined to the main thread, where [checkBackendAvailability] hands out sequence numbers: the + * test and the write then sit in one non-suspending block, so nothing can land between them. + * Cancelling the previous job is not enough on its own — cancellation is cooperative, so a run + * already past an `isActive` test still runs to its next suspension point and would write its + * stale verdict over the newer one, leaving the chat refusing to send with a backend that was + * configured in between. + * + * @param sequence the sequence number this run was started with + * @param transform builds the new status from the current one + * @return true if the verdict was written, false if a newer check had superseded this one + */ + private suspend fun publishBackendStatus( + sequence: Int, + transform: (BackendStatus) -> BackendStatus, + ): Boolean = withContext(Dispatchers.Main.immediate) { + if (sequence != backendCheckSequence) return@withContext false + _backendStatus.value = transform(_backendStatus.value) + true + } + /** * Send a user message and get agent response. */ @@ -661,15 +690,23 @@ class ChatViewModel( val llmService = getLlmService() if (llmService == null) { android.util.Log.d(TAG, "sendMessage: LLM service not available") - emitSystemError("LLM service not available. Install the AI Core plugin.") + emitSystemError(str(R.string.error_llm_service_not_available)) return } - if (!_isBackendAvailable.value) { + if (!_backendStatus.value.isAvailable) { android.util.Log.d(TAG, "sendMessage: Backend not available") + // Names the selected backend: the point of stopping here is that the user learns which + // backend is not ready, instead of the request quietly going somewhere else. emitSystemError( - "No LLM backend is set up yet. Open Settings to choose an installed " + - "backend and finish configuring it." + when (val selected = BackendRegistry.selected()) { + is SelectedBackend.Installed -> + str(R.string.error_backend_not_ready, selected.option.displayName) + // Resolved to nothing with a selection stored: the chosen backend's plugin is + // gone, which is a different fix from having installed no backend at all. + SelectedBackend.Missing -> str(R.string.backend_selected_not_installed) + SelectedBackend.None -> str(R.string.backend_none_installed) + } ) return } diff --git a/ai-core/src/main/res/values/strings.xml b/ai-core/src/main/res/values/strings.xml index c1e21bb1..923074b3 100644 --- a/ai-core/src/main/res/values/strings.xml +++ b/ai-core/src/main/res/values/strings.xml @@ -55,7 +55,7 @@ LLM service not available. Install AI Core plugin. - No LLM backend available. Please configure one in AI Core plugin. + %1$s is not ready, so nothing was sent. Open Settings to finish configuring it, or choose a different backend. Failed to generate response Error reading file: %s Error writing file: %s @@ -156,6 +156,8 @@ No AI backend is installed. Install a backend plugin from the Plugin Manager, then return to this screen to choose it. No backend + The AI backend you selected is not installed, so nothing was sent. No other backend stands in for it — install its plugin again, or choose a different backend in Settings. + Backend not installed %1$s has no settings to configure. This backend\'s settings are unavailable. It may have been uninstalled or disabled. Error: %s diff --git a/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/backends/AiBackendTest.kt b/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/backends/AiBackendTest.kt index 445c2285..f60ece90 100644 --- a/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/backends/AiBackendTest.kt +++ b/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/backends/AiBackendTest.kt @@ -32,13 +32,13 @@ class AiBackendTest { } @Test - fun givenAStoredSelectionWhoseBackendIsGone_whenResolving_thenTheDefaultWins() { - assertEquals("local", AiBackend.preferredId("uninstalled", installed)) + fun givenAStoredSelectionWhoseBackendIsGone_whenResolving_thenThereIsNoBackend() { + assertNull(AiBackend.preferredId("uninstalled", installed)) } @Test - fun givenNeitherTheSelectionNorTheDefaultIsInstalled_whenResolving_thenTheFirstOfferedWins() { - assertEquals("gemini", AiBackend.preferredId("uninstalled", listOf("gemini", "openai"))) + fun givenNothingStoredAndNoDefaultInstalled_whenResolving_thenTheFirstOfferedWins() { + assertEquals("gemini", AiBackend.preferredId(null, listOf("gemini", "openai"))) } @Test diff --git a/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/backends/BackendRegistryTest.kt b/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/backends/BackendRegistryTest.kt new file mode 100644 index 00000000..3c915563 --- /dev/null +++ b/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/backends/BackendRegistryTest.kt @@ -0,0 +1,92 @@ +package com.itsaky.androidide.plugins.aicore.backends + +import com.itsaky.androidide.plugins.services.LlmInferenceService +import com.itsaky.androidide.plugins.services.LlmInferenceService.LlmBackend +import com.itsaky.androidide.plugins.services.SharedServices +import com.itsaky.androidide.plugins.aicore.services.LlmInferenceServiceImpl +import io.mockk.every +import io.mockk.mockk +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test + +/** + * Tests [BackendRegistry.selected], the one place the selection is resolved against what is + * installed. Every surface that reports the selection — the chat's status line, its send-blocked + * error, its availability check — branches on this, so the three cases have to stay distinct. + * + * The stored id is passed explicitly: no PluginContext is registered in a JVM test, so the + * preference file cannot be written. + */ +class BackendRegistryTest { + + @Before + fun setUp() { + SharedServices.clear() + } + + @After + fun tearDown() { + SharedServices.clear() + } + + @Test + fun givenNothingInstalled_whenNothingStored_thenNone() { + installBackends() + + assertEquals(SelectedBackend.None, BackendRegistry.selected(storedId = null)) + } + + @Test + fun givenNothingInstalled_whenSelectionStored_thenMissing() { + installBackends() + + // Not None: the user has to reinstall the backend they chose, not pick one for the first + // time, and those are two different instructions. + assertEquals(SelectedBackend.Missing, BackendRegistry.selected(storedId = "gemini")) + } + + @Test + fun givenBackendInstalled_whenSelectionNamesAnother_thenMissing() { + installBackends(backend("local", "Local LLM")) + + // Never Installed("local"): substituting the one that happens to be installed would hand + // the prompt to a provider the user did not choose. + assertEquals(SelectedBackend.Missing, BackendRegistry.selected(storedId = "gemini")) + } + + @Test + fun givenSelectionInstalled_whenResolved_thenInstalledNamesIt() { + installBackends(backend("local", "Local LLM"), backend("gemini", "Gemini API")) + + val selected = BackendRegistry.selected(storedId = "gemini") + + assertTrue("expected Installed, got $selected", selected is SelectedBackend.Installed) + assertEquals("gemini", (selected as SelectedBackend.Installed).option.id) + assertEquals("Gemini API", selected.option.displayName) + } + + @Test + fun givenNothingStored_whenBackendsInstalled_thenInstalledFallsBackToDefault() { + installBackends(backend("gemini", "Gemini API"), backend(AiBackend.DEFAULT_ID, "Local LLM")) + + val selected = BackendRegistry.selected(storedId = null) + + assertTrue("expected Installed, got $selected", selected is SelectedBackend.Installed) + assertEquals(AiBackend.DEFAULT_ID, (selected as SelectedBackend.Installed).option.id) + } + + private fun installBackends(vararg backends: LlmBackend) { + val service = LlmInferenceServiceImpl() + backends.forEach(service::registerBackend) + SharedServices.register(LlmInferenceService::class.java, service) + } + + private fun backend(backendId: String, displayName: String) = mockk { + every { getId() } returns backendId + every { getName() } returns displayName + every { isAvailable() } returns true + } +} diff --git a/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/services/LlmInferenceServiceImplTest.kt b/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/services/LlmInferenceServiceImplTest.kt index cb110a91..0b384cde 100644 --- a/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/services/LlmInferenceServiceImplTest.kt +++ b/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/services/LlmInferenceServiceImplTest.kt @@ -6,6 +6,7 @@ import com.itsaky.androidide.plugins.services.LlmInferenceService.* import com.itsaky.androidide.plugins.services.SharedServices import io.mockk.every import io.mockk.mockk +import io.mockk.verify import java.util.concurrent.CompletableFuture import org.junit.Assert.* import org.junit.Before @@ -18,7 +19,7 @@ class LlmInferenceServiceImplTest { @Before fun setup() { // No AI Assistant PluginContext registered → no stored preference, so AUTO resolves - // via the LOCAL default + availability fallback rather than a value left by another test. + // via the LOCAL default and what is installed, not a value left by another test. SharedServices.clear() service = LlmInferenceServiceImpl() } @@ -27,9 +28,10 @@ class LlmInferenceServiceImplTest { backendId: String, available: Boolean = true, text: String = "generated", + displayName: String = backendId, ) = mockk { every { getId() } returns backendId - every { getName() } returns backendId + every { getName() } returns displayName every { isAvailable() } returns available every { generate(any(), any()) } returns CompletableFuture.completedFuture( LlmResponse.success(text, 1, 1) @@ -51,6 +53,18 @@ class LlmInferenceServiceImplTest { assertEquals("test-backend", backends[0].getId()) } + @Test + fun givenSeveralBackends_whenListingThem_thenTheSelectorsOrderIsReturned() { + // Listed by display name, not by the registry map's hash order, or a caller reading this + // list disagrees with the settings selector about which backend comes first. + service.registerBackend(mockBackend("zulu", displayName = "Alpha")) + service.registerBackend(mockBackend("alpha", displayName = "Zulu")) + + val ids = service.getAvailableBackends().map { it.id } + + assertEquals(listOf("zulu", "alpha"), ids) + } + @Test fun testGetBackend() { val mockBackend = mockk { @@ -128,8 +142,10 @@ class LlmInferenceServiceImplTest { } @Test - fun testAutoFallsBackToAvailableBackendWhenSelectedMissing() { - // Selection defaults to LOCAL, but only Gemini is registered/available. + fun givenNothingStoredAndNoDefaultInstalled_whenAutoRouting_thenTheInstalledOneIsUsed() { + // Nothing has been chosen and the default is not installed, so there is a choice to make. + // Resolving by what is *installed* is what the settings screen and the chat label do, so + // AUTO must agree — otherwise the AUTO consumers resolve to an unregistered id. service.registerBackend(mockBackend("gemini", text = "from gemini")) val config = LlmConfig(AiBackend.AUTO) @@ -140,6 +156,86 @@ class LlmInferenceServiceImplTest { assertEquals("gemini", config.backendId) } + @Test + fun givenSelectedBackendUnavailable_whenAutoRouting_thenNoOtherBackendIsCalled() { + // ADFA-5132: an unconfigured local model used to hand the prompt to whichever remote + // provider had a key, sending the user's code to a third party they never chose. + val local = mockBackend("local", available = false) + val gemini = mockBackend("gemini", text = "from gemini") + service.registerBackend(local) + service.registerBackend(gemini) + + val config = LlmConfig(AiBackend.AUTO) + val response = service.generateCompletion("prompt", config).get() + + assertFalse(response.success) + // The verdict alone would pass even if the hop still happened somewhere downstream. + verify(exactly = 0) { gemini.generate(any(), any()) } + } + + @Test + fun givenAnExplicitlyRequestedBackendThatIsUnavailable_whenGenerating_thenNoOtherBackendIsCalled() { + // The AUTO path is not the only way in: a caller naming an unready backend outright must + // fail on it too, or the hard gate would depend on which entry point the caller picked. + val local = mockBackend("local", available = false) + val gemini = mockBackend("gemini", text = "from gemini") + service.registerBackend(local) + service.registerBackend(gemini) + + val response = service.generateCompletion("prompt", LlmConfig("local")).get() + + assertFalse(response.success) + verify(exactly = 0) { gemini.generate(any(), any()) } + verify(exactly = 0) { local.generate(any(), any()) } + } + + @Test + fun givenAnExplicitlyRequestedBackendThatIsUnavailable_whenStreamingWithTools_thenItFailsAtOnce() { + // The chat's own path: it stamps the resolved id into the config, so the request arrives + // explicit rather than as AUTO. + val local = object : RecordingBackend("local") { + override fun isAvailable(): Boolean = false + } + val gemini = RecordingBackend("gemini") + service.registerBackend(local) + service.registerBackend(gemini) + + val errors = mutableListOf() + service.generateStreamingWithTools( + "prompt", + emptyList(), + LlmConfig("local"), + emptyList(), + object : ToolStreamCallback { + override fun onToken(token: String) = Unit + override fun onToolCall(toolCall: ToolCallRequest) = Unit + override fun onComplete(response: LlmResponse) = Unit + override fun onError(error: String) { + errors.add(error) + } + }, + ) + + assertEquals(1, errors.size) + assertTrue("the error must name the backend the user selected", errors[0].contains("local")) + assertTrue("no substitute may be streamed to", gemini.streamedPrompts.isEmpty()) + } + + @Test + fun givenNothingStoredAndNoDefaultInstalled_whenAutoRouting_thenTheSelectorsOrderDecides() { + // The tie-break is "first offered", so this has to be offered the selector's order — by + // display name — and not the registry map's hash order, or AUTO routes to one backend + // while the settings screen and the chat label name another. + service.registerBackend(mockBackend("zulu", displayName = "Alpha", text = "from zulu")) + service.registerBackend(mockBackend("alpha", displayName = "Zulu", text = "from alpha")) + + val config = LlmConfig(AiBackend.AUTO) + val response = service.generateCompletion("prompt", config).get() + + assertEquals("from zulu", response.text) + assertEquals("zulu", config.backendId) + } + @Test fun testAutoFailsWhenNoBackendAvailable() { val config = LlmConfig(AiBackend.AUTO)