From 426e5bd0365734ae1b61ccc511f4ec261560a7c8 Mon Sep 17 00:00:00 2001 From: John Trujillo Date: Fri, 28 Aug 2026 15:26:01 -0500 Subject: [PATCH 1/3] feat(ai-agent-local): load the .gguf in place and flag it when unreachable ADFA-5253: read the model through a held descriptor instead of copying it, and persist the picker's read grant. The settings pane derives its model and engine status from a live readability check, so a deleted file no longer reads as ready. --- ai-agent-local/ai-agent-local.html | 35 +- ai-agent-local/src/main/AndroidManifest.xml | 6 +- .../src/main/assets/docs/index.html | 20 +- .../aiagentlocal/backend/LocalLlmBackend.kt | 481 +++++++++++------- .../backend/ModelResidencyEngine.kt | 39 ++ .../aiagentlocal/model/GgufModelInspector.kt | 18 +- .../aiagentlocal/model/ModelFileSource.kt | 43 ++ .../model/ModelLoadDiagnostics.kt | 67 ++- .../aiagentlocal/model/ModelLoadMessages.kt | 1 + .../aiagentlocal/model/ModelSourceWatcher.kt | 134 +++++ .../aiagentlocal/model/NativeModelSource.kt | 132 +++++ .../aiagentlocal/plugin/LocalLlmPlugin.kt | 8 +- .../settings/LocalLlmSettingsFragment.kt | 76 +-- .../settings/LocalLlmSettingsViewModel.kt | 205 +++++++- .../layout/fragment_local_llm_settings.xml | 2 +- .../src/main/res/values/strings.xml | 5 + .../backend/LocalLlmBackendTest.kt | 270 +++++++++- .../model/ContentModelFileSourceTest.kt | 115 +++++ .../model/ContentNativeModelSourceTest.kt | 114 +++++ .../model/GgufModelInspectorTest.kt | 106 +++- .../aiagentlocal/model/GgufTestFiles.kt | 64 +++ .../model/ModelLoadDiagnosticsTest.kt | 91 ++-- .../model/ModelLoadMessagesTest.kt | 7 + 23 files changed, 1693 insertions(+), 346 deletions(-) create mode 100644 ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/ModelResidencyEngine.kt create mode 100644 ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelSourceWatcher.kt create mode 100644 ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/NativeModelSource.kt create mode 100644 ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ContentModelFileSourceTest.kt create mode 100644 ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ContentNativeModelSourceTest.kt create mode 100644 ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/GgufTestFiles.kt diff --git a/ai-agent-local/ai-agent-local.html b/ai-agent-local/ai-agent-local.html index 0ac3b8fc..299706c1 100644 --- a/ai-agent-local/ai-agent-local.html +++ b/ai-agent-local/ai-agent-local.html @@ -63,14 +63,15 @@

Core functionality

  • Model safety checks — inspects a selected .gguf header and refuses embedding-only models for chat, with a clear error instead of a native crash.
    • -
  • Actionable load failures — a failed load is classified (missing, - empty, not a GGUF, out of memory, unsupported quantization) and reported as - a message that says what to do next.
    • -
  • Storage-picker support — a model chosen as a - content:// document is copied once into private storage so the - native loader can open it, and only the current model is kept.
    • +
  • Actionable load failures — a failed load is classified (no longer + reachable, empty, not a GGUF, out of memory, unsupported quantization) and + reported as a message that says what to do next.
    • +
  • Direct storage access — a model chosen as a content:// + document is read in place, through the read grant the picker persisted. + Nothing is copied into private storage, so a multi-gigabyte model costs no + device space beyond the file you downloaded.
  • Its own settings pane — browse for a .gguf file, - re-load a previously imported model, record the model's published SHA-256, + re-load the model already selected, record the model's published SHA-256, and choose between the short system prompt small models follow reliably and the full tool-calling one. A model too large for the device's free RAM raises a warning first.
    • @@ -82,14 +83,15 @@

    Technical architecture

    LocalLlmPluginPlugin entry point. Registers the backend with AI Core on activation, re-registering if AI Core activates later; frees the native model on dispose. - LocalLlmBackendThe inference engine. Resolves - the selected model to a real file path, manages loading and unloading, and - serializes generations against the shared native context. + LocalLlmBackendThe inference engine. Opens the + selected model in place and hands the native loader that descriptor, manages + loading and unloading, and serializes generations against the shared native + context. GgufModelInspectorMinimal GGUF header reader that classifies a model as chat- or embedding-only. ModelLoadDiagnosticsClassifies a load failure - from the file, free memory and the native error text, as a pure function - that is unit-tested off-device. + from the model's size and readability, free memory and the native error + text, as a pure function that is unit-tested off-device. ModelLoadMessagesRenders a diagnosis as user-facing text, keeping string resources out of the engine. LocalLlmSettingsFragmentThe settings pane AI @@ -107,11 +109,12 @@

    Usage

    Manager, then restart the IDE.
  • Open Preferences → Configuration → Agent and select the local backend. This plugin's own pane appears below it.
    • -
  • Tap Browse and pick a .gguf model file. The file is - copied once into private storage, then loaded; a model larger than the free - RAM asks you to confirm first.
    • +
  • Tap Browse and pick a .gguf model file. It is loaded + from wherever you saved it, with no copy made; a model larger than the free + RAM asks you to confirm first. Leave the file in place — moving or deleting + it breaks the selection.
  • Optionally record the model's published SHA-256, or use Load - from saved to return to a model you already imported.
    • + from saved to return to the model you already selected.
    Model choice drives whether this works at all on a given device. A Q4_K_M diff --git a/ai-agent-local/src/main/AndroidManifest.xml b/ai-agent-local/src/main/AndroidManifest.xml index 3988f4c1..80896e03 100644 --- a/ai-agent-local/src/main/AndroidManifest.xml +++ b/ai-agent-local/src/main/AndroidManifest.xml @@ -36,8 +36,10 @@ android:name="plugin.max_ide_version" android:value="26.99" /> - + diff --git a/ai-agent-local/src/main/assets/docs/index.html b/ai-agent-local/src/main/assets/docs/index.html index c25acac5..232bf4f3 100644 --- a/ai-agent-local/src/main/assets/docs/index.html +++ b/ai-agent-local/src/main/assets/docs/index.html @@ -57,13 +57,14 @@

    The settings pane

    controls:

    • Browse — opens the system file picker to choose a - .gguf model. A model selected as a content:// - document is copied once into the plugin's private storage so the native - loader can open it, and only the current model is kept on disk. If the file - is larger than the device's free RAM, a warning asks you to confirm before - loading.
      • -
    • Load from saved — reloads the model already in private storage - without picking it again. Use this after restarting the IDE, or when a load + .gguf model. The plugin keeps read access to the document you + picked and reads it where it is — on internal storage, an SD card or a USB + volume. Nothing is copied, so a multi-gigabyte model costs no extra device + storage. Keep the file where it is: moving or deleting it breaks the + selection. If the file is larger than the device's free RAM, a warning asks + you to confirm before loading.
      • +
    • Load from saved — reloads the model you already selected without + picking it again. Use this after restarting the IDE, or when a load failed for a transient reason such as low memory.
    • SHA-256 — optional. Paste the checksum published alongside the model download to keep a record of which exact file is configured. It is @@ -93,6 +94,11 @@

      Troubleshooting

      the safest starting point).
    • The local backend never appears — AI Core isn't installed or activated; install it and restart the IDE.
      • +
    • "The selected model can no longer be reached" — the model is read + where you saved it rather than from a copy, so moving, renaming or deleting + the file, or removing the SD card it lives on, breaks the selection. + Clearing the IDE's app data also withdraws the permission to read it. Pick + the model again with Browse.
    diff --git a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/LocalLlmBackend.kt b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/LocalLlmBackend.kt index c54dffb1..30e8ada2 100644 --- a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/LocalLlmBackend.kt +++ b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/LocalLlmBackend.kt @@ -3,14 +3,13 @@ package com.itsaky.androidide.plugins.aiagentlocal.backend import android.app.ActivityManager import android.content.Context import android.llama.cpp.LLamaAndroid -import android.net.Uri -import android.provider.OpenableColumns import com.itsaky.androidide.plugins.PluginContext import com.itsaky.androidide.plugins.aiagentlocal.feedback.IncompatibleModelException import com.itsaky.androidide.plugins.aiagentlocal.feedback.ModelLoadException -import com.itsaky.androidide.plugins.aiagentlocal.feedback.ModelNotConfiguredException import com.itsaky.androidide.plugins.aiagentlocal.feedback.UserActionableLlmException import com.itsaky.androidide.plugins.aiagentlocal.feedback.UserFeedback +import com.itsaky.androidide.plugins.aiagentlocal.format.ByteSize +import com.itsaky.androidide.plugins.aiagentlocal.model.ContentNativeModelSource import com.itsaky.androidide.plugins.aiagentlocal.model.GgufHeader import com.itsaky.androidide.plugins.aiagentlocal.model.GgufHeaderReader import com.itsaky.androidide.plugins.aiagentlocal.model.GgufModelInspector @@ -19,13 +18,17 @@ import com.itsaky.androidide.plugins.aiagentlocal.model.ModelContextResolver import com.itsaky.androidide.plugins.aiagentlocal.model.ModelContextSize import com.itsaky.androidide.plugins.aiagentlocal.model.ModelLoadDiagnostics import com.itsaky.androidide.plugins.aiagentlocal.model.ModelLoadMessages +import com.itsaky.androidide.plugins.aiagentlocal.model.ModelSourceWatcher +import com.itsaky.androidide.plugins.aiagentlocal.model.NativeModelSource +import com.itsaky.androidide.plugins.aiagentlocal.model.OpenModelFile +import com.itsaky.androidide.plugins.aiagentlocal.model.PlatformModelSourceWatcher import com.itsaky.androidide.plugins.aiagentlocal.preferences.LocalLlmPreferences import com.itsaky.androidide.plugins.aiagentlocal.prompt.LocalSystemPrompt import com.itsaky.androidide.plugins.services.LlmInferenceService import com.itsaky.androidide.plugins.services.LlmInferenceService.* import com.itsaky.androidide.plugins.services.SharedServices +import java.io.Closeable import java.io.File -import java.io.FileOutputStream import java.util.concurrent.CompletableFuture import java.util.concurrent.atomic.AtomicBoolean import kotlinx.coroutines.CancellationException @@ -45,7 +48,10 @@ import kotlinx.coroutines.withContext * Wraps llama-impl APIs and implements LlmBackend interface. */ class LocalLlmBackend( - private val context: PluginContext + private val context: PluginContext, + private val modelSourceOverride: NativeModelSource? = null, + private val engineOverride: ModelResidencyEngine? = null, + private val watcherOverride: ModelSourceWatcher? = null, ) : HistoryCapableBackend, CancellableBackend, ConfigurableBackend { companion object { @@ -63,6 +69,12 @@ class LocalLlmBackend( * text, in which case the native stop truncates before the match. */ private val CHAT_STOP = listOf("<|im_end|>") + + /** + * Where models were copied before ADFA-5253. Nothing writes here any more; see + * [deleteLegacyModelCache], which gives the space back. + */ + private const val LEGACY_MODEL_CACHE_DIR = "llm-models" } private val llamaLazy = lazy { LLamaAndroid.instance() } @@ -87,11 +99,85 @@ class LocalLlmBackend( private val loadMessages by lazy { ModelLoadMessages(context.androidContext) } @Volatile private var modelLoaded = false - @Volatile private var currentModelPath: String? = null + + /** + * The configured reference — path or `content://` URI — of the resident model. + * + * Keyed off the *reference*, never off the resolved native path: a document's procfs path is + * a different string on every open, so comparing resolved paths would report "not loaded" for + * a model that is already resident and reload it on every message. + */ + @Volatile private var currentModelRef: String? = null + + /** + * Holds the resident model's descriptor open. Closing it invalidates the procfs path the + * native loader was given, so it lives exactly as long as the loaded model does. + */ + @Volatile private var openModel: OpenModelFile? = null + + /** + * The reference last found unreachable, so the chat is told the backend is unavailable + * instead of being sent to a model that is gone. + * + * Held as the reference rather than a flag so picking a different model clears it by itself; + * a successful load clears it for the same one. + */ + @Volatile private var unreachableModelRef: String? = null + + /** + * Stops the delete watch on the resident model. Follows residency exactly: taken when a model + * is adopted, closed when it is released. + */ + @Volatile private var modelWatch: Closeable? = null + + /** + * Opens the configured model for the native loader. Lazy so construction touches no Android + * services, and overridable so the load path can be tested without a device. + */ + private val modelSource: NativeModelSource by lazy { + modelSourceOverride ?: ContentNativeModelSource(context.androidContext) { message, error -> + context.logger.error("LocalLlmBackend: $message", error) + } + } + + /** + * Drives model residency. Defaults to the shared native engine; overridable so the residency + * rules — evicting a model whose file went away, and releasing its descriptor — can be tested + * without loading real weights. + */ + private val engine: ModelResidencyEngine = engineOverride ?: object : ModelResidencyEngine { + override suspend fun load( + nativePath: String, + contextTokens: Int, + quantizeKv: Boolean, + fallbackContextTokens: Int, + ) = llama.load( + pathToModel = nativePath, + nCtx = contextTokens, + quantizeKv = quantizeKv, + fallbackNCtx = fallbackContextTokens, + ) + override suspend fun unload() = llama.unload() + override suspend fun contextSize() = llama.getContextSize() + } + + /** + * Reports the deletion of the resident model's file, so its gigabytes come back when the user + * deletes it rather than at their next message. Lazy for the same reason as [modelSource]. + */ + private val watcher: ModelSourceWatcher by lazy { + watcherOverride ?: PlatformModelSourceWatcher(context.androidContext) { message, error -> + context.logger.warn("LocalLlmBackend: $message", error) + } + } /** Ensures the background warm-up load is launched at most once. */ private val warmUpStarted = AtomicBoolean(false) + init { + scope.launch { deleteLegacyModelCache() } + } + override fun getId(): String = "local" override fun getName(): String = "Local LLM" @@ -148,8 +234,14 @@ class LocalLlmBackend( context.logger.debug("LocalLlmBackend.isAvailable() - configured path: $configuredPath, modelLoaded: $modelLoaded") // Chat-open hits this; start loading now so the first message isn't gated on a cold load. + // Kept ahead of the check below so a model the user restores is picked up on the next ask. maybeWarmUp(configuredPath) + // A model whose file has gone away is not available, however resident its pages still are. + // Answered from the memo rather than probed here: this runs on the caller's thread, which + // may be the main one, and a document probe is a binder round trip. + if (!configuredPath.isNullOrBlank() && configuredPath == unreachableModelRef) return false + // Available if model is loaded OR if a path is configured return modelLoaded || !configuredPath.isNullOrBlank() } @@ -169,7 +261,7 @@ class LocalLlmBackend( scope.launch { try { // Serialize with real generations so a mid-warm-up send just waits for this load. - generationMutex.withLock { ensureModelLoaded(configuredPath!!) } + generationMutex.withLock { ensureModelLoaded(configuredPath) } context.logger.info("Local model warm-up complete") } catch (e: Exception) { // Stay silent (the real send surfaces config errors); allow a later retry. @@ -180,180 +272,112 @@ class LocalLlmBackend( } /** - * Resolves the user-selected model reference to a real filesystem path the native - * loader can `fopen`. + * Loads [modelRef] unless it is already resident, diagnosing any failure into a + * [ModelLoadException]. Cancellation is rethrown first because [CancellationException] extends + * [IllegalStateException] and would otherwise be diagnosed as a corrupt model. + * + * The model is opened in place — the document the user picked, through the persisted read + * grant — and the native loader is handed the procfs path of that descriptor. Nothing is + * copied. IMPORTANT: this loads *exactly* the file the user selected. It must never fall back + * to "some other .gguf on disk" — doing so silently loads the wrong model (e.g. an embedding + * model), which aborts native inference and takes the IDE down. See ADFA-4388. * - * - A plain path is returned as-is. - * - A `content://` URI (what SAF `OpenDocument` returns, held with persistable read - * permission) is streamed into a private cache file and that path is returned. + * Visible to the module so the failure paths that never reach native code — an unreachable + * model, an embedding model, and the descriptor release that follows both — can be tested off + * a device. * - * IMPORTANT: this loads *exactly* the file the user selected. It must never fall back - * to "some other .gguf on disk" — doing so silently loads the wrong model (e.g. an - * embedding model), which aborts native inference and takes the IDE down. See ADFA-4388. + * @param modelRef the configured model path or content URI */ - private fun resolveContentUriToPath(uriString: String): String? { - if (!uriString.startsWith("content://")) { - return uriString // Already a real file path + internal suspend fun ensureModelLoaded(modelRef: String) { + if (modelLoaded && currentModelRef == modelRef) { + // Residency is not evidence the file still exists. The descriptor this backend holds + // keeps a deleted inode alive, so an unchecked early return keeps answering from a + // model the user threw away — and keeps its gigabytes mapped. Confirm, then serve. + if (modelSource.isReachable(modelRef)) return + context.logger.info("Resident model is no longer reachable; unloading: $modelRef") + evictResidentModel() + throw unopenable(modelRef) } - val uri = Uri.parse(uriString) - context.logger.info("Resolving selected model URI: $uri") + val opened = modelSource.open(modelRef) ?: throw unopenable(modelRef) - val resolver = context.androidContext.contentResolver - - // Read the selected document's display name + size (used to key the cache copy). - var displayName = "model.gguf" - var size = -1L + // Every failure below leaves this handle unadopted; without the finally it would leak a + // file descriptor per failed attempt, and warm-up retries make that a loop. + var adopted = false try { - resolver.query(uri, arrayOf(OpenableColumns.DISPLAY_NAME, OpenableColumns.SIZE), null, null, null) - ?.use { c -> - if (c.moveToFirst()) { - val nameIdx = c.getColumnIndex(OpenableColumns.DISPLAY_NAME) - val sizeIdx = c.getColumnIndex(OpenableColumns.SIZE) - if (nameIdx >= 0 && !c.isNull(nameIdx)) displayName = c.getString(nameIdx) - if (sizeIdx >= 0 && !c.isNull(sizeIdx)) size = c.getLong(sizeIdx) - } - } - } catch (e: Exception) { - context.logger.warn("Could not query model metadata for $uri: ${e.message}") - } - - // Deterministic cache path keyed by URI + size, so the same selection reuses the - // same copy and a different selection can never collide with it. - val modelsDir = File(context.androidContext.filesDir, "llm-models").apply { mkdirs() } - val safeName = displayName.replace(Regex("[^A-Za-z0-9._-]"), "_") - val cacheFile = File(modelsDir, "${kotlin.math.abs(uriString.hashCode())}_${size}_$safeName") - - // Reuse a complete prior copy. - if (cacheFile.exists() && (size < 0 || cacheFile.length() == size)) { - context.logger.info("Using cached model copy: ${cacheFile.absolutePath}") - pruneOtherModels(modelsDir, cacheFile) - return cacheFile.absolutePath - } - - // Materialize the selected URI into the cache. Copy to a temp file then rename, so an - // interrupted copy can't be mistaken for a complete model on the next launch. - return try { - context.logger.info("Copying selected model into app storage: $displayName ($size bytes)") - val tmp = File(modelsDir, cacheFile.name + ".tmp") - val copied = resolver.openInputStream(uri)?.use { input -> - FileOutputStream(tmp).use { output -> input.copyTo(output, 1 shl 20) } - } - if (copied == null) { - context.logger.error("Could not open input stream for selected model $uri") - tmp.delete() - return null - } - if (size >= 0 && tmp.length() != size) { - context.logger.error("Model copy incomplete: expected $size bytes, got ${tmp.length()}") - tmp.delete() - return null + // One parse of the metadata block per load, feeding both the guard below and the + // context sizing after the unload: it sits at the front of a multi-GB file, and a + // model switch used to walk it twice. + val header = withContext(Dispatchers.IO) { GgufHeaderReader.read(opened::openStream) } + // The handle's own size, not File.length(): the native path is a procfs entry, on + // which length() reports 0 and would price the KV cache off a zero-byte model. + val modelSizeBytes = opened.sizeBytes.takeIf { it > 0L } + + // Guard the chat path against encoder-only embedding models. Running causal generation + // on one aborts natively (SIGABRT) and takes the IDE down. Classify BEFORE unloading any + // working chat model, so a wrong selection never tears down a good one. See ADFA-4388. + // The overload rescans for the architecture alone, and only if the parse above gave up. + val kind = withContext(Dispatchers.IO) { + GgufModelInspector.classify(header, opened::openStream) } - if (!tmp.renameTo(cacheFile)) { - tmp.copyTo(cacheFile, overwrite = true) - tmp.delete() + // UNKNOWN means the header could not be read, so the guard let this model through + // unchecked. Logged so a future embedding-model abort can be told apart from one that + // got past a header the inspector did read. + context.logger.debug("Model architecture: ${kind.architecture ?: "unreadable"} (${kind.kind})") + if (kind.isEmbeddingOnly) { + throw IncompatibleModelException( + "The selected model is an embedding model and can't be used for chat. " + + "Choose a chat model in AI Settings." + ) } - pruneOtherModels(modelsDir, cacheFile) - context.logger.info("Model ready at ${cacheFile.absolutePath}") - cacheFile.absolutePath - } catch (e: Exception) { - context.logger.error("Failed to copy selected model into app storage", e) - null - } - } - /** - * Keeps only the active model copy in the cache dir. Model files are large, and we only - * ever need the currently-selected one on disk. Deleting a file that native code has - * already mmap'd is safe on Android — the mapping stays valid until the model is freed. - */ - private fun pruneOtherModels(modelsDir: File, keep: File) { - modelsDir.listFiles()?.forEach { f -> - if (f.absolutePath != keep.absolutePath && f.delete()) { - context.logger.debug("Pruned old model copy: ${f.name}") + // Unload old model if loaded + if (modelLoaded) { + context.logger.info("Unloading previous model: $currentModelRef") + evictResidentModel() } - } - } - /** - * Loads [modelPath] unless it is already resident, diagnosing any native failure into a - * [ModelLoadException]. Cancellation is rethrown first because [CancellationException] extends - * [IllegalStateException] and would otherwise be diagnosed as a corrupt model. - * - * @param modelPath the configured model path or content URI - */ - private suspend fun ensureModelLoaded(modelPath: String) { - // Resolve content URI to actual file path - val resolvedPath = resolveContentUriToPath(modelPath) - if (resolvedPath == null) { - throw ModelNotConfiguredException("Could not read the selected model file. Re-select the .gguf model in AI Settings.") - } - - if (modelLoaded && currentModelPath == resolvedPath) { - return // Already loaded - } - - // One parse of the metadata block per load, feeding both the guard below and the context - // sizing after the unload: it sits at the front of a multi-GB file, and a model switch - // used to walk it twice. - // Every stat is inside the block too: isFile and length() both hit the filesystem, which on - // a removed SD card or a stale SAF mount blocks whoever called us. - val openModel = { File(resolvedPath).takeIf { it.isFile }?.inputStream() } - val (header, modelSizeBytes) = withContext(Dispatchers.IO) { - GgufHeaderReader.read(openModel) to File(resolvedPath).length().takeIf { it > 0L } - } - - // Guard the chat path against encoder-only embedding models. Running causal generation on - // one aborts natively (SIGABRT) and takes the IDE down. Classify BEFORE unloading any - // working chat model, so a wrong selection never tears down a good one. See ADFA-4388. - // The overload rescans for the architecture alone, and only if the parse above gave up. - val modelKind = withContext(Dispatchers.IO) { - GgufModelInspector.classify(header, openModel) - } - if (modelKind.isEmbeddingOnly) { - throw IncompatibleModelException( - "The selected model is an embedding model and can't be used for chat. " + - "Choose a chat model in AI Settings." - ) - } - - // Unload old model if loaded - if (modelLoaded) { - context.logger.info("Unloading previous model: $currentModelPath") - llama.unload() - modelLoaded = false - currentModelPath = null - } - - // Measured after the unload: availMem excludes the context and batch it just released. - val availableBytes = availableMemoryBytes() - ModelLoadDiagnostics.refuseBeforeLoad(availableBytes)?.let { shortfall -> - throw ModelLoadException(loadMessages.describe(shortfall), shortfall) - } + // Measured after the unload: availMem excludes the context and batch it just released. + val availableBytes = availableMemoryBytes() + ModelLoadDiagnostics.refuseBeforeLoad(availableBytes)?.let { shortfall -> + throw ModelLoadException(loadMessages.describe(shortfall), shortfall) + } - val contextSize = resolveContextSize(resolvedPath, availableBytes, header, modelSizeBytes) + val contextSize = resolveContextSize(modelRef, availableBytes, header, modelSizeBytes) - context.logger.info("Loading model: $resolvedPath") - try { - llama.load( - pathToModel = resolvedPath, - nCtx = contextSize.contextTokens, - quantizeKv = contextSize.kvType == KvCacheType.Q8_0, - fallbackNCtx = contextSize.fallbackContextTokens, - ) - } catch (e: CancellationException) { - throw e - } catch (e: Exception) { - if (e is UserActionableLlmException) throw e - // Native load_model() signals failure only with a null handle, so diagnose the likely cause. - context.logger.error("Native model load failed for $resolvedPath", e) - val diagnosis = ModelLoadDiagnostics.diagnose(resolvedPath, availableMemoryBytes(), e.message) - throw ModelLoadException(loadMessages.describe(diagnosis), diagnosis) + context.logger.info("Loading model: $modelRef via ${opened.nativePath}") + try { + engine.load( + nativePath = opened.nativePath, + contextTokens = contextSize.contextTokens, + quantizeKv = contextSize.kvType == KvCacheType.Q8_0, + fallbackContextTokens = contextSize.fallbackContextTokens, + ) + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + if (e is UserActionableLlmException) throw e + // Native load_model() signals failure only with a null handle, so diagnose the likely cause. + context.logger.error("Native model load failed for $modelRef", e) + val diagnosis = ModelLoadDiagnostics.diagnose( + sizeBytes = opened.sizeBytes, + availableMemoryBytes = availableMemoryBytes(), + nativeError = e.message, + openStream = opened::openStream, + ) + throw ModelLoadException(loadMessages.describe(diagnosis), diagnosis) + } + modelLoaded = true + currentModelRef = modelRef + openModel = opened + unreachableModelRef = null + adopted = true + startWatching(modelRef) + context.logger.info("Model loaded successfully") + reportEffectiveContextSize(contextSize.contextTokens) + } finally { + if (!adopted) opened.close() } - modelLoaded = true - currentModelPath = resolvedPath - context.logger.info("Model loaded successfully") - reportEffectiveContextSize(contextSize.contextTokens) } /** @@ -365,7 +389,7 @@ class LocalLlmBackend( */ private suspend fun reportEffectiveContextSize(requestedTokens: Int) { val actual = try { - llama.getContextSize() + engine.contextSize() } catch (e: CancellationException) { throw e } catch (e: Exception) { @@ -385,18 +409,18 @@ class LocalLlmBackend( /** * Sizes the KV cache for this model on this device and picks the type it is stored as. Must run * after any unload, so the freed context is counted as available. Answers rather than applies: - * every part of the shape is an argument to [LLamaAndroid.load], so nothing can drift between - * being chosen here and being used natively. [ModelContextResolver] fails open, so this has no + * every part of the shape is an argument to [ModelResidencyEngine.load], so nothing can drift + * between being chosen here and being used natively. [ModelContextResolver] fails open, so this has no * failure of its own. * - * @param resolvedPath filesystem path to the model, already resolved from any content URI + * @param modelRef the configured model path or content URI, for the log line only * @param availableBytes free RAM as [availableMemoryBytes] reports it, negative if unknown * @param header the model's metadata as read once by [ensureModelLoaded], null if unreadable - * @param modelSizeBytes the model file's size, null if unreadable + * @param modelSizeBytes the model's size, null if unreadable * @return the context size, cache type and f16 fallback size to load the model with */ private fun resolveContextSize( - resolvedPath: String, + modelRef: String, availableBytes: Long, header: GgufHeader?, modelSizeBytes: Long?, @@ -408,7 +432,7 @@ class LocalLlmBackend( ) // Unconditional: a wrongly sized context otherwise just reads as the assistant forgetting. context.logger.info( - "Context size for $resolvedPath: ${resolved.contextTokens} tokens," + + "Context size for $modelRef: ${resolved.contextTokens} tokens," + " ${resolved.kvType} KV cache" + " (model advertises ${resolved.advertisedTokens ?: "unknown"}," + " ${if (availableBytes >= 0L) "$availableBytes bytes free" else "free RAM unknown"})" @@ -416,6 +440,111 @@ class LocalLlmBackend( return resolved } + /** + * Forgets the resident model and releases its descriptor. The native unload is the caller's to + * do first — the mapped pages must be freed before the descriptor behind them goes. + */ + private fun releaseCurrentModel() { + stopWatching() + modelLoaded = false + currentModelRef = null + openModel?.close() + openModel = null + // Re-arm the warm-up: a model that becomes reachable again is loaded without a restart. + warmUpStarted.set(false) + } + + /** + * Gives a resident model back in full — native pages first, then the descriptor holding the + * inode alive. That order is the whole point: closing the descriptor while the loader still + * has its procfs path mapped leaves it reading an entry whose target is gone. + * + * Callers must hold [generationMutex], so a model is never pulled out from under a generation. + */ + private suspend fun evictResidentModel() { + engine.unload() + releaseCurrentModel() + } + + /** + * Records [modelRef] as unreachable and builds the failure to report for it. + * + * @return the exception to throw; never thrown here, so the caller's control flow stays visible + */ + private fun unopenable(modelRef: String): ModelLoadException { + unreachableModelRef = modelRef + val diagnosis = ModelLoadDiagnostics.diagnoseUnopenable(modelRef) + return ModelLoadException(loadMessages.describe(diagnosis), diagnosis) + } + + /** + * Watches the newly resident model's file, so a deletion frees it right away instead of at the + * next message. Best effort — an unwatchable source just leaves the check in + * [ensureModelLoaded] to catch it. + */ + private fun startWatching(modelRef: String) { + modelWatch = try { + watcher.watch(modelRef) { onModelSourceGone(modelRef) } + } catch (e: Exception) { + context.logger.warn("Could not watch the selected model: ${e.message}") + null + } + } + + private fun stopWatching() { + try { + modelWatch?.close() + } catch (e: Exception) { + context.logger.warn("Could not stop watching the selected model: ${e.message}") + } + modelWatch = null + } + + /** + * A watch fired for [modelRef]. Notifications are hints, not verdicts — providers notify for + * edits as well as deletions, and for a whole document tree — so reachability is confirmed + * before anything is torn down. + * + * Runs under [generationMutex] on [cleanupScope]: a generation already in flight finishes on + * the model it started with, and this survives the cancellation of [scope]. + */ + private fun onModelSourceGone(modelRef: String) { + cleanupScope.launch { + generationMutex.withLock { + if (!modelLoaded || currentModelRef != modelRef) return@withLock + if (modelSource.isReachable(modelRef)) return@withLock + context.logger.info("Selected model was deleted; releasing it: $modelRef") + evictResidentModel() + unreachableModelRef = modelRef + } + } + } + + /** + * Deletes the private model copies made before ADFA-5253, which run to gigabytes. The model is + * now read in place through its own grant, so nothing recreates this directory; once it is gone + * this is a single `exists()` call, which is cheaper than storing an "already done" flag. + * + * Walks and deletes gigabytes, so it pins its own dispatcher rather than inheriting whichever + * one a caller happens to launch it on. + * + * Visible to the module so a test can run it deterministically rather than racing [init]. + */ + internal suspend fun deleteLegacyModelCache() = withContext(Dispatchers.IO) { + try { + val legacy = File(context.androidContext.filesDir, LEGACY_MODEL_CACHE_DIR) + if (!legacy.exists()) return@withContext + val freedBytes = legacy.walkBottomUp().filter { it.isFile }.sumOf { it.length() } + if (legacy.deleteRecursively()) { + context.logger.info("Reclaimed ${ByteSize.format(freedBytes)} of copied model files") + } else { + context.logger.warn("Could not fully delete the old model cache at ${legacy.absolutePath}") + } + } catch (e: Exception) { + context.logger.warn("Could not delete the old model cache: ${e.message}") + } + } + /** * @return free RAM the OS reports, or -1 if unreadable (diagnosis then skips the low-memory case) */ @@ -676,9 +805,7 @@ class LocalLlmBackend( /** Suspending model unload — safe to call from any coroutine. */ private suspend fun unloadModelInternal() { if (modelLoaded) { - llama.unload() - modelLoaded = false - currentModelPath = null + evictResidentModel() context.logger.info("Model unloaded") } } @@ -698,6 +825,8 @@ class LocalLlmBackend( fun close() { scope.cancel() val cleanup = cleanupScope.launch { + // Ahead of the native check: a watch outliving the plugin would fire into a dead scope. + stopWatching() if (!llamaLazy.isInitialized()) { return@launch } diff --git a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/ModelResidencyEngine.kt b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/ModelResidencyEngine.kt new file mode 100644 index 00000000..2248a2c0 --- /dev/null +++ b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/ModelResidencyEngine.kt @@ -0,0 +1,39 @@ +package com.itsaky.androidide.plugins.aiagentlocal.backend + +/** + * The slice of the native engine that owns model residency: making a model resident, and giving + * it back. Generation itself still goes straight to `LLamaAndroid`. + * + * A seam rather than a wrapper: it exists so the residency rules that matter most (a model whose + * file went away is unloaded, and its descriptor released) can be exercised off a device, where + * loading real weights is not an option. See ADFA-5253. + */ +interface ModelResidencyEngine { + + /** + * Every part of the load's shape is an argument rather than engine state, so nothing can drift + * between being sized here and being allocated natively. See ADFA-5188. + * + * @param nativePath the path handed to the loader; a procfs entry for a picked document + * @param contextTokens the KV-cache size to create the context with, as sized per model and device + * @param quantizeKv true to store the KV cache as q8_0; the engine may still refuse it, in + * which case it falls back to f16 at [fallbackContextTokens] + * @param fallbackContextTokens the context that f16 fallback gets, sized against f16's own + * per-token cost + */ + suspend fun load( + nativePath: String, + contextTokens: Int, + quantizeKv: Boolean, + fallbackContextTokens: Int, + ) + + /** Frees the model's mapped pages and the buffers around them. */ + suspend fun unload() + + /** + * @return the size of the context actually created, which the loader may clamp below the + * requested one + */ + suspend fun contextSize(): Int +} diff --git a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/GgufModelInspector.kt b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/GgufModelInspector.kt index 8d13d817..c0f193f2 100644 --- a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/GgufModelInspector.kt +++ b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/GgufModelInspector.kt @@ -2,8 +2,6 @@ package com.itsaky.androidide.plugins.aiagentlocal.model import java.io.BufferedInputStream import java.io.DataInputStream -import java.io.File -import java.io.FileInputStream import java.io.InputStream /** @@ -18,6 +16,10 @@ import java.io.InputStream * It deliberately **fails open**: an unreadable header or a missing architecture is reported as * [ModelKind.UNKNOWN] and treated as chat-capable, so a genuine chat model is never wrongly * blocked by a header quirk. + * + * Both entry points take a stream *factory* rather than a path, because since ADFA-5253 the model + * is read in place through a `content://` grant and has no stable filesystem path. Each call opens + * its own stream and closes it, so inspection never disturbs the native loader's file offset. */ object GgufModelInspector { @@ -38,11 +40,15 @@ object GgufModelInspector { /** * Cheap magic-only check that never throws; reads just the first 4 bytes. - * @param modelPath path to the candidate file - * @return true if the file begins with the GGUF magic; false on any read error or mismatch + * + * @param openStream opens a fresh read stream over the candidate model, or returns null when + * it cannot be reached + * @return true if the model begins with the GGUF magic; false on any read error or mismatch */ - fun isGguf(modelPath: String): Boolean = try { - DataInputStream(BufferedInputStream(FileInputStream(File(modelPath)), 16)).use { readU32(it) == GGUF_MAGIC } + fun isGguf(openStream: () -> InputStream?): Boolean = try { + openStream()?.use { stream -> + readU32(DataInputStream(BufferedInputStream(stream, 16))) == GGUF_MAGIC + } ?: false } catch (_: Exception) { false } diff --git a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelFileSource.kt b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelFileSource.kt index fb2ccccf..c783ca4c 100644 --- a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelFileSource.kt +++ b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelFileSource.kt @@ -35,9 +35,26 @@ interface ModelFileSource { /** Opens the model for reading; null when it cannot be opened. Not for the main thread. */ fun openStream(context: Context, uriString: String): InputStream? + /** + * Whether the model can still be opened right now. A configured model can go away underneath + * the settings screen — deleted, unmounted, or its read grant revoked — and the stored path + * says nothing about that, so the screen has to ask. Reports rather than logs: a model that is + * gone is an answer, not a lookup failure. Not for the main thread. + */ + fun isReadable(context: Context, uriString: String): Boolean + /** Decoded last path segment — a cheap name that at least avoids raw `%3A` escapes. */ fun fallbackDisplayName(uriOrPath: String): String + /** + * Turn the picker's one-off read grant for [uriString] into a persistable one, so the model is + * still readable after the IDE is restarted — nothing is copied into private storage, so that + * grant is the only thing keeping it reachable (ADFA-5253). A no-op for a filesystem path. + * + * @return true when the model will still be readable after a restart + */ + fun persistAccess(context: Context, uriString: String): Boolean + /** * Give back the persistable read grant the picker took for [uriString], for a model the user * ended up not keeping — the grant table has a hard per-app limit. A no-op for a filesystem @@ -76,6 +93,17 @@ class ContentModelFileSource( null } + override fun isReadable(context: Context, uriString: String): Boolean = try { + if (uriString.startsWith(CONTENT_SCHEME)) { + context.contentResolver.openInputStream(Uri.parse(uriString))?.use { true } ?: false + } else { + File(uriString).let { it.isFile && it.canRead() } + } + } catch (e: Exception) { + // Deleted, unmounted, or the grant is gone — all of which mean the same thing here. + false + } + override fun fallbackDisplayName(uriOrPath: String): String = (try { Uri.decode(uriOrPath) @@ -83,6 +111,21 @@ class ContentModelFileSource( uriOrPath }).substringAfterLast('/') + override fun persistAccess(context: Context, uriString: String): Boolean { + if (!uriString.startsWith(CONTENT_SCHEME)) return true + return try { + context.contentResolver.takePersistableUriPermission( + Uri.parse(uriString), + Intent.FLAG_GRANT_READ_URI_PERMISSION, + ) + true + } catch (e: Exception) { + // A provider that hands out non-persistable grants, or a grant table that is full. + onError("could not persist the read grant for $uriString", e) + false + } + } + override fun releaseAccess(context: Context, uriString: String) { if (!uriString.startsWith(CONTENT_SCHEME)) return try { diff --git a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadDiagnostics.kt b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadDiagnostics.kt index 2e0c663b..706a058e 100644 --- a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadDiagnostics.kt +++ b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadDiagnostics.kt @@ -1,6 +1,6 @@ package com.itsaky.androidide.plugins.aiagentlocal.model -import java.io.File +import java.io.InputStream /** * Classifies why a native model load failed, as a pure function of the file, free memory, and the @@ -16,9 +16,23 @@ object ModelLoadDiagnostics { /** Floor for [refuseBeforeLoad]; the same allowance the pre-flight estimate budgets for. */ private const val MIN_RUN_BYTES = ModelMemory.RUN_BUFFER_BYTES + /** Marks a reference the document provider owns, rather than a plain filesystem path. */ + private const val CONTENT_SCHEME = "content://" + /** Most likely cause of a load failure; the caller resolves each case to a user-facing string. */ sealed interface Diagnosis { + /** A configured filesystem path with nothing at it — the file was deleted or moved. */ data object FileMissing : Diagnosis + + /** + * A picked document that can no longer be reached: deleted, renamed, on unmounted storage, + * or its persisted read grant was revoked (clearing the IDE's app data does that). + * Distinct from [FileMissing] because the fix is to pick the model again, not to restore a + * path — and distinct from [UnsupportedOrCorrupt], which would send the user chasing a + * corruption that isn't there. See ADFA-5253. + */ + data object SourceUnavailable : Diagnosis + data object FileEmpty : Diagnosis data object NotGguf : Diagnosis /** @@ -38,22 +52,35 @@ object ModelLoadDiagnostics { } /** - * Diagnoses a load that already failed, so it tests a conservative headroom rather than the - * file size: overestimating would blame a corrupt model on memory. [ContextSizePolicy] charges - * the mmap'd weights instead, because it sizes the cache before the load pages them in. + * Why an already-open model failed to load. + * + * Takes the model's size and a stream factory rather than a path: since ADFA-5253 the loader is + * handed the procfs path of a held descriptor, on which `File.length()` reports 0 and + * `File.exists()` says nothing about the underlying document. + * + * Weights are mmap'd, so this tests a conservative headroom rather than the file size: + * overestimating would blame a corrupt model on memory. [ContextSizePolicy] charges the mmap'd + * weights instead, because it sizes the cache before the load pages them in. * - * @param modelPath resolved filesystem path the native loader was handed + * @param sizeBytes the model's size, or negative if the source could not report one * @param availableMemoryBytes free RAM reported by the OS, or negative if unknown * @param nativeError the load failure's message text, or null when unavailable + * @param openStream opens a fresh read stream over the model, or returns null when it is gone * @return the most likely cause of the load failure */ - fun diagnose(modelPath: String, availableMemoryBytes: Long, nativeError: String? = null): Diagnosis { - val file = File(modelPath) - if (!file.exists()) return Diagnosis.FileMissing - - val sizeBytes = file.length() - if (sizeBytes <= 0L) return Diagnosis.FileEmpty - if (!GgufModelInspector.isGguf(modelPath)) return Diagnosis.NotGguf + fun diagnose( + sizeBytes: Long, + availableMemoryBytes: Long, + nativeError: String? = null, + openStream: () -> InputStream?, + ): Diagnosis { + // Only a NEGATIVE size means "unknown"; 0 is a genuine empty file. + if (sizeBytes == 0L) return Diagnosis.FileEmpty + + // Checked before the header read so a source that vanished under us is not mis-reported as + // a malformed one — "pick it again" and "it's corrupt" send the user to different places. + if (!isReadable(openStream)) return Diagnosis.SourceUnavailable + if (!GgufModelInspector.isGguf(openStream)) return Diagnosis.NotGguf // "Already loaded" is a run-loop state problem, not a file or memory one, so report it // before the memory heuristic — otherwise a busy loop is mis-reported as low memory. @@ -80,6 +107,15 @@ object ModelLoadDiagnostics { * @param availableMemoryBytes free RAM reported by the OS, or negative if unknown * @return the shortfall to refuse with, or null to attempt the load */ + /** + * Why a model could not be opened at all, before any load was attempted. + * + * @param modelReference the configured model, as a `content://` URI or a filesystem path + */ + fun diagnoseUnopenable(modelReference: String): Diagnosis = + if (modelReference.startsWith(CONTENT_SCHEME)) Diagnosis.SourceUnavailable + else Diagnosis.FileMissing + fun refuseBeforeLoad(availableMemoryBytes: Long): Diagnosis.LowMemory? = // Only a NEGATIVE reading means "unknown"; 0 is a genuine out-of-memory reading. if (availableMemoryBytes in 0L until MIN_RUN_BYTES) { @@ -88,6 +124,13 @@ object ModelLoadDiagnostics { null } + /** Whether the model can still be opened for reading at all. */ + private fun isReadable(openStream: () -> InputStream?): Boolean = try { + openStream()?.use { true } ?: false + } catch (_: Exception) { + false + } + // The markers below mirror the messages thrown by LLamaAndroid.load(); keep them in sync with // that file. Matching on text is best-effort — an unrecognized message falls back to // UnsupportedOrCorrupt, which is the safe default for a valid-looking file. diff --git a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadMessages.kt b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadMessages.kt index d0e85ade..70629409 100644 --- a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadMessages.kt +++ b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadMessages.kt @@ -22,6 +22,7 @@ internal class ModelLoadMessages(private val context: Context) { */ fun describe(diagnosis: Diagnosis): String = when (diagnosis) { Diagnosis.FileMissing -> context.getString(R.string.llm_load_error_missing) + Diagnosis.SourceUnavailable -> context.getString(R.string.llm_load_error_unavailable) Diagnosis.FileEmpty -> context.getString(R.string.llm_load_error_empty) Diagnosis.NotGguf -> context.getString(R.string.llm_load_error_not_gguf) is Diagnosis.LowMemory -> context.getString( diff --git a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelSourceWatcher.kt b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelSourceWatcher.kt new file mode 100644 index 00000000..c3e5aa13 --- /dev/null +++ b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelSourceWatcher.kt @@ -0,0 +1,134 @@ +package com.itsaky.androidide.plugins.aiagentlocal.model + +import android.content.Context +import android.database.ContentObserver +import android.net.Uri +import android.os.FileObserver +import android.os.Handler +import android.os.HandlerThread +import java.io.Closeable +import java.io.File + +/** + * Watches the file behind a resident model and reports when it goes away, so its gigabytes are + * given back at deletion time rather than at the user's next message. + * + * Best-effort by contract: a provider that does not notify simply never fires, and the + * before-generation reachability check stays the guarantee. Nothing here may be the only thing + * standing between a deleted model and a reply. + */ +interface ModelSourceWatcher { + + /** + * @param modelReference the resident model, as a `content://` URI or a filesystem path + * @param onGone invoked, off the caller's thread, when the file looks gone; may fire more than + * once and may fire spuriously, so the callback must confirm before acting + * @return a handle that stops the watch, or null when this source cannot be watched + */ + fun watch(modelReference: String, onGone: () -> Unit): Closeable? +} + +/** + * [ModelSourceWatcher] over the document provider and the filesystem. + * + * Callbacks arrive on a private [HandlerThread] — never the main thread, and never a thread the + * caller owns — started with the first watch and stopped with the last, so an idle plugin holds + * no thread. See ADFA-5253. + * + * @param onError reports a failed registration, so a silently unwatched model can be explained + */ +class PlatformModelSourceWatcher( + private val context: Context, + private val onError: (String, Throwable) -> Unit = { _, _ -> }, +) : ModelSourceWatcher { + + /** Guards [thread] and [handler]; both are touched from watch and from close. */ + private val lock = Any() + + private var thread: HandlerThread? = null + private var handler: Handler? = null + + /** Live watches, so the last one out stops the thread. */ + private var watchCount = 0 + + override fun watch(modelReference: String, onGone: () -> Unit): Closeable? = try { + if (modelReference.startsWith(CONTENT_SCHEME)) { + watchDocument(modelReference, onGone) + } else { + watchFile(modelReference, onGone) + } + } catch (e: Exception) { + onError("could not watch $modelReference", e) + null + } + + /** + * Providers notify on their own terms — often for the parent tree rather than the document, + * and often for edits rather than deletion — so this registers for descendants too and lets + * the callback decide. `onGone` is a hint, never a verdict. + */ + private fun watchDocument(uriString: String, onGone: () -> Unit): Closeable { + val uri = Uri.parse(uriString) + val observer = object : ContentObserver(acquireHandler()) { + override fun onChange(selfChange: Boolean, uri: Uri?) = onGone() + } + try { + context.contentResolver.registerContentObserver(uri, true, observer) + } catch (e: Exception) { + // The handler is already counted; give it back or the thread outlives every watch. + releaseHandler() + throw e + } + return Closeable { + try { + context.contentResolver.unregisterContentObserver(observer) + } finally { + releaseHandler() + } + } + } + + /** + * `DELETE_SELF` covers the delete; `MOVE_SELF` covers a rename or a move to another volume, + * which breaks a configured path just as thoroughly. + */ + private fun watchFile(path: String, onGone: () -> Unit): Closeable? { + val file = File(path) + if (!file.isFile) return null + val observer = object : FileObserver(file, DELETE_SELF or MOVE_SELF) { + override fun onEvent(event: Int, path: String?) = onGone() + } + // The framework holds FileObserver weakly and stops watching once it is collected, so the + // returned handle keeps the only strong reference alive for as long as the watch is wanted. + observer.startWatching() + return Closeable { observer.stopWatching() } + } + + /** Starts the delivery thread on the first watch. */ + private fun acquireHandler(): Handler = synchronized(lock) { + if (thread == null) { + thread = HandlerThread(THREAD_NAME).also { + it.start() + handler = Handler(it.looper) + } + } + watchCount++ + handler!! + } + + /** Stops the delivery thread with the last watch, so an idle plugin holds no thread. */ + private fun releaseHandler() = synchronized(lock) { + watchCount-- + if (watchCount <= 0) { + watchCount = 0 + thread?.quitSafely() + thread = null + handler = null + } + } + + private companion object { + const val CONTENT_SCHEME = "content://" + const val THREAD_NAME = "LocalLlm-ModelWatch" + } +} diff --git a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/NativeModelSource.kt b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/NativeModelSource.kt new file mode 100644 index 00000000..0738cb25 --- /dev/null +++ b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/NativeModelSource.kt @@ -0,0 +1,132 @@ +package com.itsaky.androidide.plugins.aiagentlocal.model + +import android.content.Context +import android.net.Uri +import java.io.Closeable +import java.io.File +import java.io.FileInputStream +import java.io.InputStream + +/** + * A model file held open for the native loader. + * + * [nativePath] is a path llama.cpp can `fopen` and `mmap`. For a document picked through SAF that is + * `/proc/self/fd/N` for the descriptor this handle owns: opening that procfs entry re-opens the + * underlying inode with an independent file offset, so the native loader behaves exactly as it does + * for a real path — without copying multiple gigabytes into private storage first. See ADFA-5253. + * + * IMPORTANT: the descriptor must stay open for as long as the model is resident. Closing it + * invalidates the procfs entry, and the pages the loader has mapped are the only thing keeping the + * model alive after that. [close] is therefore the unload path's job, not the load path's. + * + * @property nativePath the path to hand the native loader + * @property sizeBytes the model's size, or -1 when the source could not report one + */ +class OpenModelFile( + val nativePath: String, + val sizeBytes: Long, + private val descriptor: Closeable?, +) : Closeable { + + /** + * Opens an independent read stream over the same bytes the native loader sees — header + * inspection must never disturb the loader's own file offset. + * + * @return the stream, or null when the source became unreadable + */ + fun openStream(): InputStream? = try { + FileInputStream(nativePath) + } catch (_: Exception) { + null + } + + override fun close() { + try { + descriptor?.close() + } catch (_: Exception) { + // Already closed, or the provider died with it — there is nothing left to release. + } + } +} + +/** + * Opens the user's selected model for the native loader, in place and without copying it. + * An interface so the backend's load path can be exercised without a device. + */ +interface NativeModelSource { + + /** + * @param modelReference the configured model, as a `content://` URI or a filesystem path + * @return an open handle the caller owns and must [OpenModelFile.close], or null when the + * model cannot be reached at all (deleted, unmounted, or the read grant was revoked) + */ + fun open(modelReference: String): OpenModelFile? + + /** + * Whether [modelReference] still resolves to something readable, reading none of it. + * + * A resident model cannot answer this itself: the descriptor the loader holds keeps the + * deleted inode alive, so the mapped pages outlive the file and the model keeps replying from + * a document the user has thrown away. Only a fresh open off the reference can tell. + * + * @return true when the model is still there; false for deleted, unmounted, or revoked + */ + fun isReachable(modelReference: String): Boolean +} + +/** + * [NativeModelSource] over the document provider and the filesystem. + * + * @param context supplies the resolver holding the picker's persisted read grant + * @param onError reports a failed open, so a bare "model unavailable" can still be explained + */ +class ContentNativeModelSource( + private val context: Context, + private val onError: (String, Throwable) -> Unit = { _, _ -> }, +) : NativeModelSource { + + override fun open(modelReference: String): OpenModelFile? = + if (modelReference.startsWith(CONTENT_SCHEME)) openDocument(modelReference) + else openFile(modelReference) + + /** + * Takes the document's descriptor and hands the native loader its procfs path. `"r"` is the + * only mode asked for, which is all the persisted grant covers. + */ + private fun openDocument(uriString: String): OpenModelFile? = try { + context.contentResolver.openFileDescriptor(Uri.parse(uriString), "r") + ?.let { OpenModelFile("$FD_DIR${it.fd}", it.statSize, it) } + } catch (e: Exception) { + onError("could not open the selected model $uriString", e) + null + } + + /** + * One binder round trip for a document, one stat for a path — nothing is read, so this is + * cheap enough to ask before every generation. A failure here is the routine answer "it is + * gone", not an error worth reporting through [onError]. + */ + override fun isReachable(modelReference: String): Boolean = try { + if (modelReference.startsWith(CONTENT_SCHEME)) { + context.contentResolver + .openFileDescriptor(Uri.parse(modelReference), "r") + ?.use { true } ?: false + } else { + File(modelReference).isFile + } + } catch (_: Exception) { + false + } + + private fun openFile(path: String): OpenModelFile? = try { + File(path).takeIf { it.isFile }?.let { OpenModelFile(it.absolutePath, it.length(), null) } + } catch (e: Exception) { + onError("could not open the model file $path", e) + null + } + + private companion object { + const val CONTENT_SCHEME = "content://" + const val FD_DIR = "/proc/self/fd/" + } +} diff --git a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/plugin/LocalLlmPlugin.kt b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/plugin/LocalLlmPlugin.kt index 6613c057..f7409562 100644 --- a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/plugin/LocalLlmPlugin.kt +++ b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/plugin/LocalLlmPlugin.kt @@ -243,8 +243,12 @@ class LocalLlmPlugin : IPlugin, DocumentationExtension { file is checked before it is stored: a file that isn't a valid .gguf is rejected, and one that looks too large for this device's free memory raises a warning first.

    -

    Load from saved re-selects the model already configured, - which is useful after clearing app data or moving the file.

    +

    The model is read where you saved it and never copied, so leave + the file in place. If it is moved or deleted, if its storage is + disconnected, or if the IDE's app data is cleared, pick it again + with Browse.

    +

    Load from saved reloads the model already configured + without opening the picker — useful after restarting the IDE.

    """.trimIndent(), ), PluginTooltipEntry( diff --git a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsFragment.kt b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsFragment.kt index 879d85b7..49b22e6d 100644 --- a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsFragment.kt +++ b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsFragment.kt @@ -1,6 +1,5 @@ package com.itsaky.androidide.plugins.aiagentlocal.settings -import android.content.Intent import android.net.Uri import android.os.Bundle import android.view.LayoutInflater @@ -40,9 +39,9 @@ class LocalLlmSettingsFragment : Fragment(), MemoryWarningDialogFragment.Host { registerForActivityResult(ActivityResultContracts.OpenDocument()) { uri: Uri? -> uri?.let { try { - requireContext().contentResolver - .takePersistableUriPermission(it, Intent.FLAG_GRANT_READ_URI_PERMISSION) - viewModel.loadModelFromUri(it.toString(), requireContext()) + // The durable read grant is taken by the view model, with the rest of the + // selection's bookkeeping — see LocalLlmSettingsViewModel.loadModelFromUri. + viewModel.loadModelFromUri(it.toString()) Toast.makeText( requireContext(), getString(R.string.model_loading_toast), @@ -131,10 +130,7 @@ class LocalLlmSettingsFragment : Fragment(), MemoryWarningDialogFragment.Host { wireTooltip(browseButton, LocalLlmPlugin.TOOLTIP_TAG_SETTINGS_LOCAL_MODEL) loadSavedButton.setOnClickListener { - val savedPath = viewModel.savedModelPath.value - if (savedPath != null) { - viewModel.loadModelFromUri(savedPath, requireContext()) - } + viewModel.state.value?.savedModelPath?.let(viewModel::loadModelFromUri) } // Same concept as Browse — choosing which local model to run. wireTooltip(loadSavedButton, LocalLlmPlugin.TOOLTIP_TAG_SETTINGS_LOCAL_MODEL) @@ -159,50 +155,56 @@ class LocalLlmSettingsFragment : Fragment(), MemoryWarningDialogFragment.Host { wireTooltip(this, LocalLlmPlugin.TOOLTIP_TAG_SETTINGS_SIMPLE_PROMPT) } - viewModel.engineState.observe(viewLifecycleOwner) { state -> - when (state) { - is EngineState.Initializing, EngineState.Uninitialized -> { - engineStatusTextView.text = getString(R.string.engine_initializing) - browseButton.isEnabled = false - loadSavedButton.isEnabled = false - } - is EngineState.Initialized -> { - engineStatusTextView.text = getString(R.string.engine_ready) - browseButton.isEnabled = true - loadSavedButton.isEnabled = viewModel.savedModelPath.value != null - } - is EngineState.Error -> { - engineStatusTextView.text = state.message - browseButton.isEnabled = false - loadSavedButton.isEnabled = false - } + // All three lines describe the same model, so they are drawn from one state in one pass: + // an unreachable model must not read as ready on one line and missing on another. + viewModel.state.observe(viewLifecycleOwner) { state -> + engineStatusTextView.text = when (val engine = state.engine) { + is EngineState.NoModel -> getString(R.string.engine_no_model) + is EngineState.ModelUnavailable -> getString(R.string.engine_model_unavailable) + is EngineState.Initializing -> getString(R.string.engine_initializing) + is EngineState.Initialized -> getString(R.string.engine_ready) + is EngineState.Error -> engine.message } - } - viewModel.savedModelPath.observe(viewLifecycleOwner) { path -> - loadSavedButton.isEnabled = - path != null && viewModel.engineState.value is EngineState.Initialized + // Enabled off the model status, not off engine readiness: picking a model is exactly + // how the user recovers from an engine that isn't ready, so it must stay reachable. + val busy = state.model is ModelLoadingState.Loading + browseButton.isEnabled = !busy + loadSavedButton.isEnabled = state.savedModelPath != null && !busy - if (path != null) { + val savedName = state.savedModelName + if (savedName != null) { modelPathTextView.visibility = View.VISIBLE - val fileName = viewModel.getSavedModelName() ?: viewModel.fallbackDisplayName(path) - modelPathTextView.text = getString(R.string.model_saved_path, fileName) + modelPathTextView.text = if (state.model is ModelLoadingState.Unavailable) { + getString(R.string.model_saved_path_unavailable, savedName) + } else { + getString(R.string.model_saved_path, savedName) + } } else { modelPathTextView.visibility = View.GONE } - } - viewModel.modelLoadingState.observe(viewLifecycleOwner) { state -> modelStatusTextView.visibility = View.VISIBLE - modelStatusTextView.text = when (state) { + modelStatusTextView.text = when (val model = state.model) { is ModelLoadingState.Idle -> getString(R.string.model_none_loaded) is ModelLoadingState.Loading -> getString(R.string.model_loading_wait) - is ModelLoadingState.Loaded -> getString(R.string.model_loaded, state.modelName) - is ModelLoadingState.Error -> getString(R.string.model_load_error, state.message) + is ModelLoadingState.Loaded -> getString(R.string.model_loaded, model.modelName) + is ModelLoadingState.Unavailable -> + getString(R.string.model_unavailable, model.modelName) + is ModelLoadingState.Error -> getString(R.string.model_load_error, model.message) } } } + /** + * The model file lives outside the IDE and can be deleted or unmounted while this screen is + * away, so its availability is re-checked on every return rather than only at first load. + */ + override fun onResume() { + super.onResume() + viewModel.refreshSavedModelAvailability() + } + /** * Puts a "this model may not fit" question to the user. Collected under STARTED so the dialog is * never shown to a stopped fragment; the event waits in the ViewModel until then. diff --git a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsViewModel.kt b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsViewModel.kt index 4c6b2217..e6582e4d 100644 --- a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsViewModel.kt +++ b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsViewModel.kt @@ -34,19 +34,50 @@ sealed class ModelLoadingState { object Idle : ModelLoadingState() object Loading : ModelLoadingState() data class Loaded(val modelName: String) : ModelLoadingState() + + /** + * A model is configured but its file can no longer be read — deleted, unmounted, or the read + * grant revoked. Distinct from [Error]: nothing failed here, the selection simply went stale, + * and the screen has to say so rather than keep reporting the model as loaded (ADFA-5253). + */ + data class Unavailable(val modelName: String) : ModelLoadingState() + data class Error(val message: String) : ModelLoadingState() } /** - * State for the inference engine initialization. + * Whether this backend can serve a request. The engine itself is loaded lazily on the first + * request, so there is no engine to interrogate here and readiness is a statement about the + * configured model: without one that can actually be loaded there is nothing to be ready for. + * Derived from [ModelLoadingState] — see [LocalLlmSettingsViewModel.engineStateFor]. */ sealed class EngineState { - object Uninitialized : EngineState() + /** No model is configured yet, so the engine has nothing to load. */ + object NoModel : EngineState() + + /** A model is configured but its file cannot be read; the engine cannot load it. */ + object ModelUnavailable : EngineState() + object Initializing : EngineState() object Initialized : EngineState() data class Error(val message: String) : EngineState() } +/** + * Everything this pane draws, as one value: the configured model, how it is doing, and the + * readiness that follows from it. One container rather than three streams, so the three lines are + * published in a single dispatch and can never describe different models mid-update. + * + * @param savedModelPath the configured model, as a `content://` URI or a path; null when unset + * @param savedModelName the display name for [savedModelPath] + */ +data class LocalLlmSettingsState( + val savedModelPath: String? = null, + val savedModelName: String? = null, + val model: ModelLoadingState = ModelLoadingState.Idle, + val engine: EngineState = EngineState.NoModel, +) + /** * A selected model that may not fit in this device's memory, with the figures to show the user. * @@ -98,14 +129,25 @@ class LocalLlmSettingsViewModel( private val KEY_SIMPLE_PROMPT = LocalLlmPreferences.KEY_SIMPLE_PROMPT } - private val _savedModelPath = MutableLiveData(null) - val savedModelPath: LiveData get() = _savedModelPath + /** + * The authoritative state, kept here rather than read back from [_state]: `postValue` publishes + * asynchronously, so a background update that read `_state.value` would compute its copy from + * a version two updates old and silently drop the ones in between. + */ + @Volatile private var current = LocalLlmSettingsState() - private val _modelLoadingState = MutableLiveData(ModelLoadingState.Idle) - val modelLoadingState: LiveData get() = _modelLoadingState + private val _state = MutableLiveData(current) + val state: LiveData get() = _state - private val _engineState = MutableLiveData(EngineState.Initialized) - val engineState: LiveData get() = _engineState + /** + * Applies [transform] to the state and publishes the result. Synchronized because the memory + * pre-flight, the availability re-check and a load can all be in flight at once. + */ + @Synchronized + private fun update(transform: (LocalLlmSettingsState) -> LocalLlmSettingsState) { + current = transform(current) + _state.postValue(current) + } /** The memory pre-flight's consent gate; see [loadModelFromUri]. */ private val memoryConfirmation = UserConfirmation() @@ -129,11 +171,71 @@ class LocalLlmSettingsViewModel( private fun checkInitialState() { val savedPath = prefs()?.getString(KEY_MODEL_PATH, null) - _savedModelPath.value = savedPath + val modelState = modelStateFor(savedPath) + // Optimistic: the file has not been read yet. refreshSavedModelAvailability() corrects it. + update { + LocalLlmSettingsState( + savedModelPath = savedPath, + savedModelName = savedPath?.let { displayNameFor(it) }, + model = modelState, + engine = engineStateFor(modelState) ?: EngineState.Initialized, + ) + } + refreshSavedModelAvailability() + } + + /** + * Re-checks that the configured model is still readable and downgrades the status to + * [ModelLoadingState.Unavailable] when it is not. Call whenever this screen becomes visible: + * the file lives outside the IDE, so it can be deleted or unmounted between two visits, and the + * stored path on its own would keep claiming the model is loaded (ADFA-5253). + */ + fun refreshSavedModelAvailability() { + val savedPath = getLocalModelPath() ?: return + val context = getContext()?.androidContext ?: return + + viewModelScope.launch(ioDispatcher) { + val readable = modelFiles.isReadable(context, savedPath) + + // A selection made while the check ran owns the status now; leave it to that load. + if (getLocalModelPath() != savedPath) return@launch + if (current.model is ModelLoadingState.Loading) return@launch + + if (readable) { + // Only ever clears a stale "unavailable": a live Error is about this same model. + if (current.model is ModelLoadingState.Unavailable) { + publishModelState(modelStateFor(savedPath)) + } + } else { + logger?.warn("$TAG: the configured model can no longer be read: $savedPath") + publishModelState( + ModelLoadingState.Unavailable(displayNameFor(savedPath)) + ) + } + } + } + + /** + * Publishes a model status together with the engine readiness that follows from it, in one + * dispatch, so the screen can never draw a model and a readiness that disagree. + */ + private fun publishModelState(model: ModelLoadingState) { + update { it.copy(model = model, engine = engineStateFor(model) ?: it.engine) } + } - // The engine is loaded lazily by the backend, so from this screen it is always "ready". - _engineState.value = EngineState.Initialized - _modelLoadingState.value = modelStateFor(savedPath) + /** + * Engine readiness implied by a model status, or null to leave the engine's status alone. + * + * @param state the model status just published + */ + private fun engineStateFor(state: ModelLoadingState): EngineState? = when (state) { + is ModelLoadingState.Idle -> EngineState.NoModel + is ModelLoadingState.Loading -> EngineState.Initializing + is ModelLoadingState.Loaded -> EngineState.Initialized + is ModelLoadingState.Unavailable -> EngineState.ModelUnavailable + // A rejected *selection* says nothing about the model that is actually configured, which + // this leaves in place — so it must not restate that model's readiness either way. + is ModelLoadingState.Error -> null } /** @@ -144,7 +246,7 @@ class LocalLlmSettingsViewModel( */ private fun modelStateFor(savedPath: String?): ModelLoadingState = if (savedPath != null) { - ModelLoadingState.Loaded(getSavedModelName() ?: fallbackDisplayName(savedPath)) + ModelLoadingState.Loaded(displayNameFor(savedPath)) } else { ModelLoadingState.Idle } @@ -164,20 +266,25 @@ class LocalLlmSettingsViewModel( get() = getContext()?.logger /** Human-readable name persisted alongside the model path at load time, if any. */ - fun getSavedModelName(): String? = + private fun getSavedModelName(): String? = prefs()?.getString(KEY_MODEL_NAME, null)?.takeIf { it.isNotBlank() } private fun saveLocalModelName(name: String?) { prefs()?.edit()?.putString(KEY_MODEL_NAME, name)?.apply() + update { it.copy(savedModelName = name) } } /** Decoded last path segment — a cheap fallback that at least avoids raw %3A escapes. */ - fun fallbackDisplayName(uriOrPath: String): String = modelFiles.fallbackDisplayName(uriOrPath) + private fun fallbackDisplayName(uriOrPath: String): String = + modelFiles.fallbackDisplayName(uriOrPath) + + /** The name to show for a configured model: the one persisted at load time, else the path's. */ + private fun displayNameFor(uriOrPath: String): String = + getSavedModelName() ?: fallbackDisplayName(uriOrPath) fun saveLocalModelPath(path: String) { prefs()?.edit()?.putString(KEY_MODEL_PATH, path)?.apply() - // Use postValue instead of value since this can be called from background threads - _savedModelPath.postValue(path) + update { it.copy(savedModelPath = path) } } fun getLocalModelPath(): String? = prefs()?.getString(KEY_MODEL_PATH, null) @@ -201,21 +308,47 @@ class LocalLlmSettingsViewModel( * makes it load, so the memory pre-flight gates it: a model the user declines is never stored, * and therefore never loaded (ADFA-1798). * + * The read grant is made persistable first: the model is read in place rather than copied, so + * without a durable grant the stored path would stop resolving at the next restart (ADFA-5253). + * * @param uriString the selected model, as a `content://` URI or a filesystem path - * @param context resolves the model's display name, size and header */ - fun loadModelFromUri(uriString: String, context: Context) { + fun loadModelFromUri(uriString: String) { + // This plugin's own context, not the caller's: a UI Context captured by a coroutine that + // outlives the fragment would hold the Activity, and only this one resolves the plugin's + // own resources for the messages below. + val context = getContext()?.androidContext ?: run { + logger?.error("$TAG: no plugin context; cannot select $uriString") + return + } + viewModelScope.launch(ioDispatcher) { - _modelLoadingState.postValue(ModelLoadingState.Loading) + publishModelState(ModelLoadingState.Loading) try { + // Taken before the first read, so every step below works off the durable grant. + if (!modelFiles.persistAccess(context, uriString)) { + // Readable now through the picker's own grant, but not after a restart. Better + // to load it and say so later than to refuse a model the user just picked. + logger?.warn("$TAG: no persistable read grant for $uriString") + } + // One lookup for both: the real file name to show, and the size to estimate from. val fileInfo = modelFiles.info(context, uriString) val fileName = fileInfo.displayName + // Checked before the GGUF sniff so a model that is simply gone — the "Load from + // saved" case after the file was deleted — is not reported as a corrupt one. + if (!modelFiles.isReadable(context, uriString)) { + releaseUnkeptGrant(context, uriString) + publishModelState(ModelLoadingState.Unavailable(fileName)) + return@launch + } + // Rejected up front, so no bad path is persisted or shown as "Loaded". if (!GgufFileInspector.looksLikeGguf(context.contentResolver, uriString)) { - _modelLoadingState.postValue( + releaseUnkeptGrant(context, uriString) + publishModelState( ModelLoadingState.Error( context.getString(R.string.error_model_not_gguf, fileName) ) @@ -225,27 +358,30 @@ class LocalLlmSettingsViewModel( if (!confirmMemoryHeadroom(uriString, fileInfo, context)) { logger?.info("$TAG: model declined at the memory warning: $fileName") - // Never the configured model: re-checking it and declining must not revoke it. - if (uriString != getLocalModelPath()) { - modelFiles.releaseAccess(context, uriString) - } + releaseUnkeptGrant(context, uriString) restoreSavedModelState() return@launch } + // The model being replaced is no longer read by anything, and grants are capped. + val replaced = getLocalModelPath() + if (replaced != null && replaced != uriString) { + modelFiles.releaseAccess(context, replaced) + } + // Persist the name before the path so the savedModelPath observer can read it. saveLocalModelName(fileName) saveLocalModelPath(uriString) // Nothing is loaded here; the engine reads this path when it needs the model. - _modelLoadingState.postValue(ModelLoadingState.Loaded(fileName)) + publishModelState(ModelLoadingState.Loaded(fileName)) logger?.debug("$TAG: model path saved: $uriString ($fileName)") } catch (e: CancellationException) { throw e } catch (e: Exception) { logger?.error("$TAG: error saving model path", e) - _modelLoadingState.postValue( + publishModelState( ModelLoadingState.Error( context.getString(R.string.error_model_save_failed, e.message.orEmpty()) ) @@ -315,11 +451,24 @@ class LocalLlmSettingsViewModel( } } + /** + * Gives back the grant taken for a selection that was not kept, so an abandoned pick does not + * hold a slot in the capped grant table. + * + * Never touches the configured model: re-checking it and abandoning that check must leave the + * model that is actually in use readable. + */ + private fun releaseUnkeptGrant(context: Context, uriString: String) { + if (uriString != getLocalModelPath()) { + modelFiles.releaseAccess(context, uriString) + } + } + /** * Republishes the model that is actually configured, so abandoning a selection leaves the * screen describing the previous model rather than the one that was never stored. */ private fun restoreSavedModelState() { - _modelLoadingState.postValue(modelStateFor(getLocalModelPath())) + publishModelState(modelStateFor(getLocalModelPath())) } } diff --git a/ai-agent-local/src/main/res/layout/fragment_local_llm_settings.xml b/ai-agent-local/src/main/res/layout/fragment_local_llm_settings.xml index c9ce6b2f..133daa4e 100644 --- a/ai-agent-local/src/main/res/layout/fragment_local_llm_settings.xml +++ b/ai-agent-local/src/main/res/layout/fragment_local_llm_settings.xml @@ -9,7 +9,7 @@ android:id="@+id/engine_status_text" android:layout_width="match_parent" android:layout_height="wrap_content" - android:text="@string/engine_initializing" + android:text="@string/engine_no_model" android:textAppearance="?android:attr/textAppearanceSmall" android:layout_marginTop="8dp" android:textColor="?android:attr/textColorSecondary"/> diff --git a/ai-agent-local/src/main/res/values/strings.xml b/ai-agent-local/src/main/res/values/strings.xml index 2dd066d3..b54fc51a 100644 --- a/ai-agent-local/src/main/res/values/strings.xml +++ b/ai-agent-local/src/main/res/values/strings.xml @@ -3,6 +3,7 @@ The model file could not be found. Re-select the .gguf model in AI Settings. + The selected model can no longer be reached. It may have been moved, deleted, or saved to storage that isn\'t connected right now, or the IDE\'s permission to read it was withdrawn. Select the .gguf model again in AI Settings. The model file is empty — the download may have been interrupted. Re-download the .gguf model and select it again. This file isn\'t a valid .gguf model (it may be corrupt or only partially downloaded). Re-download the model and select it again. Loading this model needs at least %1$s of free memory, but only %2$s is available on this device. Close other apps and try again, or pick a smaller or more heavily quantized model (for example a Q4_K_M build of a 1–3B model). @@ -13,11 +14,15 @@ Error: %s Initializing engine… Engine ready + Engine not ready — no model selected + Engine not ready — the selected model can\'t be reached Saved: %s No model is currently loaded Loading model, please wait… ✅ Model loaded: %s ❌ Error: %s + ⚠️ \"%1$s\" can no longer be reached. It may have been moved, deleted, or saved to storage that isn\'t connected right now. Select the .gguf model again. + Saved: %s (unavailable) Loading model… No model selected Browse for Model File diff --git a/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/LocalLlmBackendTest.kt b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/LocalLlmBackendTest.kt index e2846b40..5e157527 100644 --- a/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/LocalLlmBackendTest.kt +++ b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/LocalLlmBackendTest.kt @@ -1,21 +1,115 @@ package com.itsaky.androidide.plugins.aiagentlocal.backend +import android.content.Context import com.itsaky.androidide.plugins.PluginContext +import com.itsaky.androidide.plugins.aiagentlocal.feedback.IncompatibleModelException +import com.itsaky.androidide.plugins.aiagentlocal.feedback.ModelLoadException +import com.itsaky.androidide.plugins.aiagentlocal.model.GgufTestFiles +import com.itsaky.androidide.plugins.aiagentlocal.model.ModelLoadDiagnostics.Diagnosis +import com.itsaky.androidide.plugins.aiagentlocal.model.ModelSourceWatcher +import com.itsaky.androidide.plugins.aiagentlocal.model.NativeModelSource +import com.itsaky.androidide.plugins.aiagentlocal.model.OpenModelFile import com.itsaky.androidide.plugins.services.LlmInferenceService.* +import io.mockk.every import io.mockk.mockk -import org.junit.Test +import java.io.Closeable +import java.io.File +import kotlinx.coroutines.runBlocking import org.junit.Assert.* import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder class LocalLlmBackendTest { + @get:Rule + val temporaryFolder = TemporaryFolder() + + private lateinit var filesDir: File + private lateinit var pluginContext: PluginContext private lateinit var backend: LocalLlmBackend + /** Records that the model's descriptor was released, which is the fd leak we can catch here. */ + private class RecordingDescriptor : Closeable { + var closed = false + override fun close() { + closed = true + } + } + + /** Serves prepared handles; anything not in [handles] is a model that cannot be reached. */ + private class FakeModelSource(private val handles: Map) : NativeModelSource { + var openCount = 0 + + /** Flipped to simulate the user deleting the file out from under a resident model. */ + var reachable = true + + override fun open(modelReference: String): OpenModelFile? { + openCount++ + return handles[modelReference].takeIf { reachable } + } + + override fun isReachable(modelReference: String): Boolean = + reachable && handles.containsKey(modelReference) + } + + /** Stands in for the native engine, recording residency without loading any weights. */ + private class FakeEngine : ModelResidencyEngine { + var loadCount = 0 + var unloadCount = 0 + + /** The context size the backend sized for the last load, so the sizing is observable. */ + var lastContextTokens = 0 + + /** Whether the last load asked for a quantized KV cache, so that choice is observable. */ + var lastQuantizeKv = false + + override suspend fun load( + nativePath: String, + contextTokens: Int, + quantizeKv: Boolean, + fallbackContextTokens: Int, + ) { + loadCount++ + lastContextTokens = contextTokens + lastQuantizeKv = quantizeKv + } + + override suspend fun unload() { + unloadCount++ + } + + override suspend fun contextSize() = lastContextTokens + } + + /** Captures the delete callback so a test can fire it the way the platform would. */ + private class FakeWatcher : ModelSourceWatcher { + var onGone: (() -> Unit)? = null + var closed = false + override fun watch(modelReference: String, onGone: () -> Unit) = Closeable { + closed = true + }.also { this.onGone = onGone } + } + @Before fun setup() { - backend = LocalLlmBackend(mockk(relaxed = true)) + filesDir = temporaryFolder.newFolder("files") + val androidContext = mockk(relaxed = true) + every { androidContext.filesDir } returns filesDir + pluginContext = mockk(relaxed = true) + every { pluginContext.androidContext } returns androidContext + backend = LocalLlmBackend(pluginContext) } + private fun backendWith(source: NativeModelSource) = LocalLlmBackend(pluginContext, source) + + private fun backendWith( + source: NativeModelSource, + engine: ModelResidencyEngine, + watcher: ModelSourceWatcher = FakeWatcher(), + ) = LocalLlmBackend(pluginContext, source, engine, watcher) + @Test fun testBackendId() { assertEquals("local", backend.getId()) @@ -52,4 +146,176 @@ class LocalLlmBackendTest { // With no model configured, generate() fails fast before any native work. assertTrue(response.error!!.contains("No model configured")) } + + @Test + fun givenAContentUriThatCannotBeOpened_whenLoading_thenFailsAsSourceUnavailable() { + // The model is read in place now, so a revoked grant or a deleted document is the most + // likely failure of all — and must not surface as "your model is corrupt". + val source = FakeModelSource(emptyMap()) + + val error = assertThrows(ModelLoadException::class.java) { + runBlocking { backendWith(source).ensureModelLoaded(CONTENT_URI) } + } + + assertEquals(Diagnosis.SourceUnavailable, error.diagnosis) + assertEquals(1, source.openCount) + } + + @Test + fun givenAConfiguredPathThatIsGone_whenLoading_thenFailsAsFileMissing() { + // A plain path survives from before the picker; "file not found" is still its right answer. + val error = assertThrows(ModelLoadException::class.java) { + runBlocking { backendWith(FakeModelSource(emptyMap())).ensureModelLoaded("/sdcard/model.gguf") } + } + + assertEquals(Diagnosis.FileMissing, error.diagnosis) + } + + @Test + fun givenAnEmbeddingModel_whenLoading_thenRejectedBeforeAnyNativeWork() { + // ADFA-4388: the classify guard must still fire when the header arrives as a stream over a + // document read in place. Reaching native code here would abort the whole IDE. + val source = FakeModelSource(mapOf(CONTENT_URI to handleFor(GgufTestFiles.withArchitecture("bert")))) + + assertThrows(IncompatibleModelException::class.java) { + runBlocking { backendWith(source).ensureModelLoaded(CONTENT_URI) } + } + } + + @Test + fun givenARejectedModel_whenLoading_thenItsDescriptorIsReleased() { + // A held descriptor that is never adopted leaks one fd per attempt, and the warm-up retries. + val descriptor = RecordingDescriptor() + val handle = handleFor(GgufTestFiles.withArchitecture("bert"), descriptor) + val source = FakeModelSource(mapOf(CONTENT_URI to handle)) + + assertThrows(IncompatibleModelException::class.java) { + runBlocking { backendWith(source).ensureModelLoaded(CONTENT_URI) } + } + + assertTrue("the rejected model's descriptor must not leak", descriptor.closed) + } + + @Test + fun givenAContentUriModel_whenLoading_thenNothingIsWrittenToInternalStorage() { + // AC 3, as far as a JVM test can honestly go: resolving a picked model must not copy it. + // The device check is `du -sh .../files/llm-models` before and after a real selection. + val source = FakeModelSource(mapOf(CONTENT_URI to handleFor(GgufTestFiles.withArchitecture("bert")))) + + assertThrows(IncompatibleModelException::class.java) { + runBlocking { backendWith(source).ensureModelLoaded(CONTENT_URI) } + } + + assertEquals(emptyList(), filesDir.walkTopDown().filter { it.isFile }.map { it.name }.toList()) + } + + @Test + fun givenModelCopiesFromAnEarlierRelease_whenCleaningUp_thenTheyAreDeleted() { + // Without this the ticket saves nothing for anyone who already used the plugin. + val legacyDir = File(filesDir, "llm-models").apply { mkdirs() } + File(legacyDir, "1234_5678_model.gguf").writeBytes(ByteArray(4096)) + + runBlocking { backendWith(FakeModelSource(emptyMap())).deleteLegacyModelCache() } + + assertFalse(legacyDir.exists()) + } + + @Test + fun givenNoLegacyModelCache_whenCleaningUp_thenItIsAQuietNoOp() { + // Runs on every activation, so the common case must neither throw nor create the directory. + runBlocking { backendWith(FakeModelSource(emptyMap())).deleteLegacyModelCache() } + + assertFalse(File(filesDir, "llm-models").exists()) + } + + @Test + fun givenAResidentModelWhoseFileWasDeleted_whenGenerating_thenItIsUnloadedAndReported() { + // The descriptor keeps the deleted inode alive, so without the reachability check the + // model answers happily from a file the user threw away. ADFA-5253. + val descriptor = RecordingDescriptor() + val source = FakeModelSource(mapOf(CONTENT_URI to handleFor(chatModel(), descriptor))) + val engine = FakeEngine() + val backend = backendWith(source, engine) + + runBlocking { backend.ensureModelLoaded(CONTENT_URI) } + source.reachable = false + + val error = assertThrows(ModelLoadException::class.java) { + runBlocking { backend.ensureModelLoaded(CONTENT_URI) } + } + + assertEquals(Diagnosis.SourceUnavailable, error.diagnosis) + assertEquals("the model's pages must be freed, not just refused", 1, engine.unloadCount) + assertTrue("the descriptor must be released or the inode stays alive", descriptor.closed) + } + + @Test + fun givenAResidentModelStillOnDisk_whenGenerating_thenItIsServedWithoutReloading() { + // The check must not cost a reload: a document's procfs path differs on every open, and + // reloading gigabytes per message would be far worse than the bug it fixes. + val source = FakeModelSource(mapOf(CONTENT_URI to handleFor(chatModel()))) + val engine = FakeEngine() + val backend = backendWith(source, engine) + + runBlocking { + backend.ensureModelLoaded(CONTENT_URI) + backend.ensureModelLoaded(CONTENT_URI) + backend.ensureModelLoaded(CONTENT_URI) + } + + assertEquals(1, engine.loadCount) + assertEquals(0, engine.unloadCount) + } + + @Test + fun givenAResidentModel_whenItsWatchFires_thenItIsUnloadedWithoutWaitingForAMessage() { + // Checkpoint 3: the gigabytes come back at deletion time, not at the user's next message. + val descriptor = RecordingDescriptor() + val source = FakeModelSource(mapOf(CONTENT_URI to handleFor(chatModel(), descriptor))) + val engine = FakeEngine() + val watcher = FakeWatcher() + val backend = backendWith(source, engine, watcher) + + runBlocking { backend.ensureModelLoaded(CONTENT_URI) } + source.reachable = false + watcher.onGone!!.invoke() + + awaitUnload(engine) + assertTrue("the descriptor must be released or the inode stays alive", descriptor.closed) + } + + @Test + fun givenAResidentModelThatIsStillThere_whenItsWatchFiresForAnEdit_thenItStaysLoaded() { + // Providers notify for edits and for the whole tree, so a notification is a hint. Acting + // on it unconfirmed would unload a working model mid-conversation. + val source = FakeModelSource(mapOf(CONTENT_URI to handleFor(chatModel()))) + val engine = FakeEngine() + val watcher = FakeWatcher() + val backend = backendWith(source, engine, watcher) + + runBlocking { backend.ensureModelLoaded(CONTENT_URI) } + watcher.onGone!!.invoke() + + Thread.sleep(200) + assertEquals("a spurious notification must not unload a reachable model", 0, engine.unloadCount) + } + + /** The eviction runs on the backend's own cleanup scope, so the test waits for it. */ + private fun awaitUnload(engine: FakeEngine) { + val deadline = System.currentTimeMillis() + 2000 + while (engine.unloadCount == 0 && System.currentTimeMillis() < deadline) { + Thread.sleep(10) + } + assertEquals("the deleted model must be unloaded", 1, engine.unloadCount) + } + + /** A minimal GGUF that passes the ADFA-4388 embedding guard, so loads reach the engine. */ + private fun chatModel(): File = GgufTestFiles.withArchitecture("qwen2") + + private fun handleFor(file: File, descriptor: Closeable? = null) = + OpenModelFile(file.absolutePath, file.length(), descriptor) + + private companion object { + const val CONTENT_URI = "content://com.android.externalstorage.documents/document/model.gguf" + } } diff --git a/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ContentModelFileSourceTest.kt b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ContentModelFileSourceTest.kt new file mode 100644 index 00000000..7f1ba276 --- /dev/null +++ b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ContentModelFileSourceTest.kt @@ -0,0 +1,115 @@ +package com.itsaky.androidide.plugins.aiagentlocal.model + +import android.content.ContentResolver +import android.content.Context +import android.content.Intent +import android.net.Uri +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkStatic +import io.mockk.spyk +import io.mockk.unmockkStatic +import io.mockk.verify +import org.junit.After +import org.junit.Assert.* +import org.junit.Before +import org.junit.Test +import java.io.ByteArrayInputStream +import java.io.File +import java.io.FileNotFoundException + +/** + * The read grant is the only thing keeping a picked model reachable — it is read in place, never + * copied — so persisting it is what makes a selection survive a restart (ADFA-5253). + */ +class ContentModelFileSourceTest { + + private lateinit var resolver: ContentResolver + private lateinit var context: Context + private lateinit var uri: Uri + private val errors = mutableListOf() + private val source = ContentModelFileSource { what, _ -> errors += what } + + @Before + fun setup() { + resolver = mockk(relaxed = true) + context = mockk(relaxed = true) + every { context.contentResolver } returns resolver + uri = mockk(relaxed = true) + mockkStatic(Uri::class) + every { Uri.parse(any()) } returns uri + } + + @After + fun tearDown() { + unmockkStatic(Uri::class) + } + + @Test + fun givenContentUri_whenPersistAccess_thenTakesPersistableReadPermission() { + assertTrue(source.persistAccess(context, CONTENT_URI)) + + verify { resolver.takePersistableUriPermission(uri, Intent.FLAG_GRANT_READ_URI_PERMISSION) } + assertTrue(errors.toString(), errors.isEmpty()) + } + + @Test + fun givenFilesystemPath_whenPersistAccess_thenNoGrantIsNeeded() { + assertTrue(source.persistAccess(context, "/sdcard/Download/model.gguf")) + + verify(exactly = 0) { resolver.takePersistableUriPermission(any(), any()) } + } + + @Test + fun givenNonPersistableGrant_whenPersistAccess_thenReportsFailureWithoutThrowing() { + every { resolver.takePersistableUriPermission(any(), any()) } throws + SecurityException("No persistable permission grants found") + + assertFalse(source.persistAccess(context, CONTENT_URI)) + assertEquals(1, errors.size) + } + + @Test + fun givenContentUri_whenReleaseAccess_thenGivesTheReadGrantBack() { + source.releaseAccess(context, CONTENT_URI) + + verify { + resolver.releasePersistableUriPermission(uri, Intent.FLAG_GRANT_READ_URI_PERMISSION) + } + } + + @Test + fun givenDeletedDocument_whenIsReadable_thenFalseWithoutReportingAnError() { + every { resolver.openInputStream(uri) } throws + FileNotFoundException("open failed: ENOENT (No such file or directory)") + + assertFalse(source.isReadable(context, CONTENT_URI)) + // A model that is gone is an answer for the caller, not a lookup failure to log. + assertTrue(errors.toString(), errors.isEmpty()) + } + + @Test + fun givenOpenableDocument_whenIsReadable_thenTrueAndTheStreamIsClosed() { + val stream = spyk(ByteArrayInputStream(ByteArray(4))) + every { resolver.openInputStream(uri) } returns stream + + assertTrue(source.isReadable(context, CONTENT_URI)) + verify { stream.close() } + } + + @Test + fun givenMissingFilesystemPath_whenIsReadable_thenFalse() { + assertFalse(source.isReadable(context, "/sdcard/Download/gone.gguf")) + } + + @Test + fun givenExistingFile_whenIsReadable_thenTrue() { + val file = File.createTempFile("model", ".gguf").apply { deleteOnExit() } + + assertTrue(source.isReadable(context, file.absolutePath)) + } + + private companion object { + const val CONTENT_URI = "content://com.android.providers.downloads/document/42" + } +} diff --git a/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ContentNativeModelSourceTest.kt b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ContentNativeModelSourceTest.kt new file mode 100644 index 00000000..9f377ccd --- /dev/null +++ b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ContentNativeModelSourceTest.kt @@ -0,0 +1,114 @@ +package com.itsaky.androidide.plugins.aiagentlocal.model + +import android.content.ContentResolver +import android.content.Context +import android.net.Uri +import android.os.ParcelFileDescriptor +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkStatic +import io.mockk.unmockkStatic +import java.io.File +import org.junit.After +import org.junit.Assert.* +import org.junit.Before +import org.junit.Test +import org.junit.Rule +import org.junit.rules.TemporaryFolder + +/** + * A resident model cannot notice its own file being deleted — the descriptor the loader holds + * keeps the inode alive — so the reachability probe is the only thing that can (ADFA-5253). + */ +class ContentNativeModelSourceTest { + + @get:Rule + val temporaryFolder = TemporaryFolder() + + private lateinit var resolver: ContentResolver + private lateinit var context: Context + private lateinit var source: ContentNativeModelSource + + @Before + fun setup() { + resolver = mockk(relaxed = true) + context = mockk(relaxed = true) + every { context.contentResolver } returns resolver + mockkStatic(Uri::class) + every { Uri.parse(any()) } returns mockk(relaxed = true) + source = ContentNativeModelSource(context) + } + + @After + fun tearDown() { + unmockkStatic(Uri::class) + } + + @Test + fun givenAPathThatStillExists_whenProbed_thenItIsReachable() { + val model = temporaryFolder.newFile("model.gguf") + + assertTrue(source.isReachable(model.absolutePath)) + } + + @Test + fun givenADeletedPath_whenProbed_thenItIsUnreachable() { + val model = temporaryFolder.newFile("model.gguf") + assertTrue(model.delete()) + + assertFalse(source.isReachable(model.absolutePath)) + } + + @Test + fun givenADirectory_whenProbed_thenItIsUnreachable() { + // A path that resolves but holds no model must not read as a usable one. + val directory = temporaryFolder.newFolder("models") + + assertFalse(source.isReachable(directory.absolutePath)) + } + + @Test + fun givenADocumentTheProviderStillServes_whenProbed_thenItIsReachable() { + every { resolver.openFileDescriptor(any(), "r") } returns mockk(relaxed = true) + + assertTrue(source.isReachable(CONTENT_URI)) + } + + @Test + fun givenADeletedDocument_whenProbed_thenItIsUnreachable() { + // What a deleted document actually does: the provider throws rather than returning null. + every { resolver.openFileDescriptor(any(), "r") } throws java.io.FileNotFoundException() + + assertFalse(source.isReachable(CONTENT_URI)) + } + + @Test + fun givenAProviderThatAnswersWithNothing_whenProbed_thenItIsUnreachable() { + every { resolver.openFileDescriptor(any(), "r") } returns null + + assertFalse(source.isReachable(CONTENT_URI)) + } + + @Test + fun givenAProbedDocument_whenTheProbeEnds_thenItsDescriptorIsClosed() { + // The probe must not leak the fd it opens: one per generation would exhaust the table. + val descriptor = mockk(relaxed = true) + every { resolver.openFileDescriptor(any(), "r") } returns descriptor + + source.isReachable(CONTENT_URI) + + io.mockk.verify { descriptor.close() } + } + + @Test + fun givenADeletedPath_whenOpened_thenNoHandleIsReturned() { + val model = temporaryFolder.newFile("model.gguf") + assertTrue(model.delete()) + + assertNull(source.open(model.absolutePath)) + } + + private companion object { + const val CONTENT_URI = "content://com.android.externalstorage.documents/document/model.gguf" + } +} diff --git a/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/GgufModelInspectorTest.kt b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/GgufModelInspectorTest.kt index 2b1312aa..22dca7b2 100644 --- a/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/GgufModelInspectorTest.kt +++ b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/GgufModelInspectorTest.kt @@ -1,15 +1,20 @@ package com.itsaky.androidide.plugins.aiagentlocal.model +import com.itsaky.androidide.plugins.aiagentlocal.model.GgufModelInspector.ModelKind import java.io.ByteArrayOutputStream +import java.io.File import java.io.InputStream import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue import org.junit.Test /** - * Pins the encoder-only guard against the files a full header parse gives up on. Classifying an - * embedding model as [GgufModelInspector.ModelKind.UNKNOWN] lets it reach a causal `llama_decode`, - * which aborts the whole IDE process — the crash ADFA-4388 added the guard to prevent. + * The ADFA-4388 guard. Running causal generation on an encoder-only model aborts natively and takes + * the IDE down, so misclassifying one is not a wrong message — it is a crash. The parser-give-up + * cases matter most: a file a full header parse rejects still has to be classified, or the guard + * waves it through. Since ADFA-5253 the header arrives as a stream over a document read in place. */ class GgufModelInspectorTest { @@ -18,7 +23,7 @@ class GgufModelInspectorTest { val bytes = gguf(architectureEntry(EMBEDDING_ARCH)) assertEquals(EMBEDDING_ARCH, GgufHeaderReader.read { bytes.inputStream() }?.architecture) - assertEquals(GgufModelInspector.ModelKind.EMBEDDING, classify(bytes).kind) + assertEquals(ModelKind.EMBEDDING, classify(bytes).kind) } @Test @@ -26,7 +31,7 @@ class GgufModelInspectorTest { val bytes = gguf(architectureEntry(EMBEDDING_ARCH), unknownTypeEntry("quirk")) assertNull(GgufHeaderReader.read { bytes.inputStream() }) - assertEquals(GgufModelInspector.ModelKind.EMBEDDING, classify(bytes).kind) + assertEquals(ModelKind.EMBEDDING, classify(bytes).kind) } @Test @@ -34,35 +39,106 @@ class GgufModelInspectorTest { val bytes = gguf(architectureEntry(EMBEDDING_ARCH), declaredEntryCount = 5000L) assertNull(GgufHeaderReader.read { bytes.inputStream() }) - assertEquals(GgufModelInspector.ModelKind.EMBEDDING, classify(bytes).kind) + assertEquals(ModelKind.EMBEDDING, classify(bytes).kind) } @Test fun givenAnUnparseableChatModel_whenClassifying_thenReportsChat() { val bytes = gguf(architectureEntry("llama"), unknownTypeEntry("quirk")) - assertEquals(GgufModelInspector.ModelKind.CHAT, classify(bytes).kind) + assertEquals(ModelKind.CHAT, classify(bytes).kind) } @Test fun givenNoArchitectureAtAll_whenClassifying_thenFailsOpenAsUnknown() { val bytes = gguf(unknownTypeEntry("quirk")) - assertEquals(GgufModelInspector.ModelKind.UNKNOWN, classify(bytes).kind) + assertEquals(ModelKind.UNKNOWN, classify(bytes).kind) } @Test - fun givenAnOpenerThatReturnsNoStream_whenClassifying_thenFailsOpenAsUnknown() { - val result = GgufModelInspector.classify(null) { null } + fun givenABertModel_whenClassified_thenEmbeddingOnly() { + val result = classify(GgufTestFiles.withArchitecture("bert")) - assertEquals(GgufModelInspector.ModelKind.UNKNOWN, result.kind) + assertEquals(ModelKind.EMBEDDING, result.kind) + assertTrue(result.isEmbeddingOnly) } - /** Classifies the way the load path does: one full parse, then the architecture-only retry. */ - private fun classify(bytes: ByteArray): GgufModelInspector.Result { - val openStream: () -> InputStream? = { bytes.inputStream() } - return GgufModelInspector.classify(GgufHeaderReader.read(openStream), openStream) + @Test + fun givenABertFamilyArchitecture_whenClassified_thenStillEmbeddingOnly() { + // The family is matched by substring, so the named variants must not need their own entry. + for (arch in listOf("nomic-bert", "jina-bert-v2", "xlm-roberta")) { + assertTrue(arch, classify(GgufTestFiles.withArchitecture(arch)).isEmbeddingOnly) + } + } + + @Test + fun givenANonBertEmbeddingArchitecture_whenClassified_thenEmbeddingOnly() { + for (arch in listOf("mpnet", "gte", "t5encoder")) { + assertTrue(arch, classify(GgufTestFiles.withArchitecture(arch)).isEmbeddingOnly) + } + } + + @Test + fun givenAChatArchitecture_whenClassified_thenChat() { + val result = classify(GgufTestFiles.withArchitecture("qwen2")) + + assertEquals(ModelKind.CHAT, result.kind) + assertEquals("qwen2", result.architecture) + assertFalse(result.isEmbeddingOnly) } + + @Test + fun givenATruncatedHeader_whenClassified_thenUnknownAndNotBlocked() { + // Fails open: a header quirk must never block a model that would have run fine. + val result = classify(GgufTestFiles.truncated()) + + assertEquals(ModelKind.UNKNOWN, result.kind) + assertFalse(result.isEmbeddingOnly) + } + + @Test + fun givenAnUnreachableSource_whenClassified_thenUnknownRatherThanThrowing() { + // A null stream is what a revoked grant or a deleted file looks like here. + val result = classify { null } + + assertEquals(ModelKind.UNKNOWN, result.kind) + assertNull(result.architecture) + } + + @Test + fun givenAStreamThatThrows_whenClassified_thenUnknownRatherThanPropagating() { + val result = classify { throw java.io.IOException("provider died") } + + assertEquals(ModelKind.UNKNOWN, result.kind) + } + + @Test + fun givenGgufMagic_whenIsGguf_thenTrue() { + assertTrue(GgufModelInspector.isGguf(streamOf(GgufTestFiles.withArchitecture("qwen2")))) + } + + @Test + fun givenNonGgufContent_whenIsGguf_thenFalse() { + assertFalse(GgufModelInspector.isGguf(streamOf(GgufTestFiles.notGguf(64)))) + } + + @Test + fun givenAnUnreachableSource_whenIsGguf_thenFalse() { + assertFalse(GgufModelInspector.isGguf { null }) + } + + private fun streamOf(file: File): () -> InputStream? = + { if (file.isFile) file.inputStream() else null } + + /** Classifies the way the load path does: one full parse, then the architecture-only retry. */ + private fun classify(openStream: () -> InputStream?): GgufModelInspector.Result = + GgufModelInspector.classify(GgufHeaderReader.read(openStream), openStream) + + private fun classify(bytes: ByteArray): GgufModelInspector.Result = + classify { bytes.inputStream() } + + private fun classify(file: File): GgufModelInspector.Result = classify(streamOf(file)) } private const val EMBEDDING_ARCH = "nomic-bert" diff --git a/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/GgufTestFiles.kt b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/GgufTestFiles.kt new file mode 100644 index 00000000..5a045ee9 --- /dev/null +++ b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/GgufTestFiles.kt @@ -0,0 +1,64 @@ +package com.itsaky.androidide.plugins.aiagentlocal.model + +import java.io.ByteArrayOutputStream +import java.io.File + +/** + * Builds the smallest GGUF files [GgufModelInspector] can be asked to classify: magic, version, + * zero tensors and a single metadata entry. Shared by the inspector's own tests and by the + * backend's load-path tests, which need a real file behind an [OpenModelFile]. + */ +internal object GgufTestFiles { + + private const val MAGIC = "GGUF" + + /** GGUF v3: version >= 2 is what makes counts and lengths 64-bit. */ + private const val VERSION = 3 + + private const val ARCHITECTURE_KEY = "general.architecture" + + /** The GGUF metadata value type for a string. */ + private const val TYPE_STRING = 8 + + /** + * @param architecture the value stored under `general.architecture`, e.g. "bert" or "qwen2" + * @return a temp file holding a complete, minimal GGUF header + */ + fun withArchitecture(architecture: String): File { + val out = ByteArrayOutputStream() + out.write(MAGIC.toByteArray(Charsets.US_ASCII)) + out.writeU32(VERSION) + out.writeU64(0) // tensor_count + out.writeU64(1) // metadata_kv_count + out.writeString(ARCHITECTURE_KEY) + out.writeU32(TYPE_STRING) + out.writeString(architecture) + return tempFile(out.toByteArray()) + } + + /** Valid magic, then nothing — the inspector must fail open rather than throw. */ + fun truncated(): File = tempFile(MAGIC.toByteArray(Charsets.US_ASCII)) + + /** A file that is not a GGUF at all. */ + fun notGguf(sizeBytes: Int): File = tempFile(ByteArray(sizeBytes)) + + private fun tempFile(bytes: ByteArray): File = + File.createTempFile("model", ".gguf").apply { + deleteOnExit() + writeBytes(bytes) + } + + private fun ByteArrayOutputStream.writeU32(value: Int) { + for (i in 0 until 4) write((value shr (8 * i)) and 0xFF) + } + + private fun ByteArrayOutputStream.writeU64(value: Long) { + for (i in 0 until 8) write(((value shr (8 * i)) and 0xFF).toInt()) + } + + private fun ByteArrayOutputStream.writeString(value: String) { + val bytes = value.toByteArray(Charsets.UTF_8) + writeU64(bytes.size.toLong()) + write(bytes) + } +} diff --git a/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadDiagnosticsTest.kt b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadDiagnosticsTest.kt index d047f144..1c1a7155 100644 --- a/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadDiagnosticsTest.kt +++ b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadDiagnosticsTest.kt @@ -2,14 +2,24 @@ package com.itsaky.androidide.plugins.aiagentlocal.model import com.itsaky.androidide.plugins.aiagentlocal.model.ModelLoadDiagnostics.Diagnosis import org.junit.Assert.assertEquals -import org.junit.Assert.assertFalse import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Test import java.io.File +import java.io.InputStream class ModelLoadDiagnosticsTest { + /** + * Diagnoses [file] as the backend does: its real size, and a factory that re-opens it. Since + * ADFA-5253 the loader is handed a procfs path, so size and readability arrive separately + * rather than being read off a `File`. + */ + private fun diagnose(file: File, availableMemoryBytes: Long, nativeError: String? = null) = + ModelLoadDiagnostics.diagnose(file.length(), availableMemoryBytes, nativeError) { streamOf(file) } + + private fun streamOf(file: File): InputStream? = if (file.isFile) file.inputStream() else null + private fun tempFile(bytes: Int, magic: Boolean = true): File = File.createTempFile("model", ".gguf").apply { deleteOnExit() @@ -23,28 +33,22 @@ class ModelLoadDiagnosticsTest { } } - @Test - fun givenMissingFile_whenDiagnosed_thenFileMissing() { - val d = ModelLoadDiagnostics.diagnose("/does/not/exist.gguf", availableMemoryBytes = 8L shl 30) - assertEquals(Diagnosis.FileMissing, d) - } - @Test fun givenEmptyFile_whenDiagnosed_thenFileEmpty() { - val d = ModelLoadDiagnostics.diagnose(tempFile(0).absolutePath, availableMemoryBytes = 8L shl 30) + val d = diagnose(tempFile(0), availableMemoryBytes = 8L shl 30) assertEquals(Diagnosis.FileEmpty, d) } @Test fun givenNonGgufContent_whenDiagnosed_thenNotGguf() { - val d = ModelLoadDiagnostics.diagnose(tempFile(2048, magic = false).absolutePath, availableMemoryBytes = 8L shl 30) + val d = diagnose(tempFile(2048, magic = false), availableMemoryBytes = 8L shl 30) assertEquals(Diagnosis.NotGguf, d) } @Test fun givenValidGgufAndLowMemory_whenDiagnosed_thenLowMemory() { // 1 MB "model", only 512 KB free -> below the headroom floor. - val d = ModelLoadDiagnostics.diagnose(tempFile(1 shl 20).absolutePath, availableMemoryBytes = 512L shl 10) + val d = diagnose(tempFile(1 shl 20), availableMemoryBytes = 512L shl 10) assertTrue(d is Diagnosis.LowMemory) // neededBytes reports the headroom that tripped the check; here the 256 MB floor dominates. assertEquals(256L shl 20, (d as Diagnosis.LowMemory).neededBytes) @@ -53,23 +57,20 @@ class ModelLoadDiagnosticsTest { @Test fun givenLargeModelAndHeadroomBelowFileSize_whenDiagnosed_thenNotLowMemory() { // Guards the mmap property: free RAM under the file size is not itself a shortage. - val d = ModelLoadDiagnostics.diagnose( - tempFile(1 shl 20).absolutePath, - availableMemoryBytes = 512L shl 20, // 512 MB free, above the floor - ) + val d = diagnose(tempFile(1 shl 20), availableMemoryBytes = 512L shl 20) // above the floor assertEquals(Diagnosis.UnsupportedOrCorrupt, d) } @Test fun givenValidGgufAndAmpleMemory_whenDiagnosed_thenUnsupportedOrCorrupt() { - val d = ModelLoadDiagnostics.diagnose(tempFile(1 shl 20).absolutePath, availableMemoryBytes = 8L shl 30) + val d = diagnose(tempFile(1 shl 20), availableMemoryBytes = 8L shl 30) assertEquals(Diagnosis.UnsupportedOrCorrupt, d) } @Test fun givenUnknownMemory_whenDiagnosed_thenNotLowMemory() { // availMem < 0 (unreadable) must not be treated as "no memory". - val d = ModelLoadDiagnostics.diagnose(tempFile(1 shl 20).absolutePath, availableMemoryBytes = -1L) + val d = diagnose(tempFile(1 shl 20), availableMemoryBytes = -1L) assertEquals(Diagnosis.UnsupportedOrCorrupt, d) } @@ -77,7 +78,7 @@ class ModelLoadDiagnosticsTest { fun givenZeroFreeMemory_whenDiagnosed_thenLowMemory() { // 0 free bytes is a genuine out-of-memory reading (only a negative value means "unknown"), // so it must classify as low memory rather than falling through to unsupported/corrupt. - val d = ModelLoadDiagnostics.diagnose(tempFile(1 shl 20).absolutePath, availableMemoryBytes = 0L) + val d = diagnose(tempFile(1 shl 20), availableMemoryBytes = 0L) assertTrue(d is Diagnosis.LowMemory) assertEquals(0L, (d as Diagnosis.LowMemory).availableBytes) } @@ -85,22 +86,14 @@ class ModelLoadDiagnosticsTest { @Test fun givenAlreadyLoadedError_whenDiagnosed_thenModelBusy() { // A valid file with ample RAM but the run loop is busy must not be blamed on the file. - val d = ModelLoadDiagnostics.diagnose( - tempFile(1 shl 20).absolutePath, - availableMemoryBytes = 8L shl 30, - nativeError = "Model already loaded", - ) + val d = diagnose(tempFile(1 shl 20), availableMemoryBytes = 8L shl 30, nativeError = "Model already loaded") assertEquals(Diagnosis.ModelBusy, d) } @Test fun givenAlreadyLoadedErrorAndLowMemory_whenDiagnosed_thenModelBusyWinsOverMemory() { // "Already loaded" is a state issue, so it outranks the low-memory heuristic. - val d = ModelLoadDiagnostics.diagnose( - tempFile(1 shl 20).absolutePath, - availableMemoryBytes = 512L shl 10, - nativeError = "Model already loaded", - ) + val d = diagnose(tempFile(1 shl 20), availableMemoryBytes = 512L shl 10, nativeError = "Model already loaded") assertEquals(Diagnosis.ModelBusy, d) } @@ -108,22 +101,14 @@ class ModelLoadDiagnosticsTest { fun givenContextAllocError_whenDiagnosed_thenInitializationFailed() { // A valid file with ample RAM that still fails to allocate its context is memory pressure, // not a corrupt file. - val d = ModelLoadDiagnostics.diagnose( - tempFile(1 shl 20).absolutePath, - availableMemoryBytes = 8L shl 30, - nativeError = "new_context() failed", - ) + val d = diagnose(tempFile(1 shl 20), availableMemoryBytes = 8L shl 30, nativeError = "new_context() failed") assertEquals(Diagnosis.InitializationFailed, d) } @Test fun givenUnrecognizedError_whenDiagnosed_thenUnsupportedOrCorrupt() { // An unknown native message on a valid-looking file falls back to the safe default. - val d = ModelLoadDiagnostics.diagnose( - tempFile(1 shl 20).absolutePath, - availableMemoryBytes = 8L shl 30, - nativeError = "something unexpected", - ) + val d = diagnose(tempFile(1 shl 20), availableMemoryBytes = 8L shl 30, nativeError = "something unexpected") assertEquals(Diagnosis.UnsupportedOrCorrupt, d) } @@ -154,13 +139,35 @@ class ModelLoadDiagnosticsTest { } @Test - fun givenGgufMagic_whenIsGguf_thenTrue() { - assertTrue(GgufModelInspector.isGguf(tempFile(64).absolutePath)) + fun givenUnknownSize_whenDiagnosed_thenNotReportedAsEmpty() { + // A provider that cannot report a size hands back -1. Only 0 is genuinely empty; treating + // a negative as "<= 0" would tell every such user their model is a truncated download. + val file = tempFile(1 shl 20) + val d = ModelLoadDiagnostics.diagnose(-1L, availableMemoryBytes = 8L shl 30) { streamOf(file) } + + assertEquals(Diagnosis.UnsupportedOrCorrupt, d) + } + + @Test + fun givenAnUnreadableSourceAndAValidSize_whenDiagnosed_thenUnavailableRatherThanNotGguf() { + // "Re-download the model" is the wrong instruction for a file that is simply out of reach. + val d = ModelLoadDiagnostics.diagnose(1L shl 20, availableMemoryBytes = 8L shl 30) { null } + + assertEquals(Diagnosis.SourceUnavailable, d) } @Test - fun givenNonGgufContent_whenIsGguf_thenFalse() { - assertFalse(GgufModelInspector.isGguf(tempFile(64, magic = false).absolutePath)) - assertFalse(GgufModelInspector.isGguf("/does/not/exist.gguf")) + fun givenAContentUri_whenDiagnoseUnopenable_thenSourceUnavailable() { + val d = ModelLoadDiagnostics.diagnoseUnopenable("content://com.android.providers/document/1") + + assertEquals(Diagnosis.SourceUnavailable, d) + } + + @Test + fun givenAFilesystemPath_whenDiagnoseUnopenable_thenFileMissing() { + // A configured plain path that is gone really is a missing file, not a withdrawn grant. + val d = ModelLoadDiagnostics.diagnoseUnopenable("/sdcard/Download/model.gguf") + + assertEquals(Diagnosis.FileMissing, d) } } diff --git a/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadMessagesTest.kt b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadMessagesTest.kt index bf4b32c7..095bd417 100644 --- a/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadMessagesTest.kt +++ b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadMessagesTest.kt @@ -24,6 +24,13 @@ class ModelLoadMessagesTest { verify { context.getString(R.string.llm_load_error_missing) } } + @Test + fun givenSourceUnavailable_whenDescribed_thenUnavailableString() { + // Must not share FileMissing's wording: one asks for a path, the other for a fresh pick. + messages.describe(Diagnosis.SourceUnavailable) + verify { context.getString(R.string.llm_load_error_unavailable) } + } + @Test fun givenFileEmpty_whenDescribed_thenEmptyString() { messages.describe(Diagnosis.FileEmpty) From 54c95bd09270bd377c53a8efb33fc6951d6aa184 Mon Sep 17 00:00:00 2001 From: John Trujillo Date: Thu, 3 Sep 2026 13:25:42 -0500 Subject: [PATCH 2/3] fix(ai-agent-local): address review on read-in-place model loading Drop the stale isAvailable() memo, refuse a non-seekable descriptor as SourceNotSeekable, key the pane's unavailable marker off engine status, coalesce watch notifications, and cover openDocument + the grant lifecycle. --- ai-agent-local/ai-agent-local.html | 7 +- ai-agent-local/build.gradle.kts | 3 + .../src/main/assets/docs/index.html | 11 +- .../aiagentlocal/backend/LocalLlmBackend.kt | 51 ++-- .../model/ModelLoadDiagnostics.kt | 23 +- .../aiagentlocal/model/ModelLoadMessages.kt | 1 + .../aiagentlocal/model/ModelSourceWatcher.kt | 6 +- .../aiagentlocal/model/NativeModelSource.kt | 12 +- .../settings/LocalLlmSettingsFragment.kt | 18 +- .../settings/LocalLlmSettingsViewModel.kt | 55 +++- .../src/main/res/values/strings.xml | 1 + .../backend/LocalLlmBackendTest.kt | 47 +++- .../model/ContentNativeModelSourceTest.kt | 43 ++++ .../model/ModelLoadMessagesTest.kt | 7 + .../settings/LocalLlmSettingsViewModelTest.kt | 241 ++++++++++++++++++ 15 files changed, 471 insertions(+), 55 deletions(-) create mode 100644 ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsViewModelTest.kt diff --git a/ai-agent-local/ai-agent-local.html b/ai-agent-local/ai-agent-local.html index 299706c1..6b7412b5 100644 --- a/ai-agent-local/ai-agent-local.html +++ b/ai-agent-local/ai-agent-local.html @@ -64,7 +64,8 @@

    Core functionality

    header and refuses embedding-only models for chat, with a clear error instead of a native crash.
  • Actionable load failures — a failed load is classified (no longer - reachable, empty, not a GGUF, out of memory, unsupported quantization) and + reachable, streamed rather than local, empty, not a GGUF, out of memory, + unsupported quantization) and reported as a message that says what to do next.
  • Direct storage access — a model chosen as a content:// document is read in place, through the read grant the picker persisted. @@ -112,7 +113,9 @@

    Usage

  • Tap Browse and pick a .gguf model file. It is loaded from wherever you saved it, with no copy made; a model larger than the free RAM asks you to confirm first. Leave the file in place — moving or deleting - it breaks the selection.
    • + it breaks the selection. The picker offers device-local documents only: a + model still in a cloud folder can only be read as a stream, which the + in-place loader cannot use.
  • Optionally record the model's published SHA-256, or use Load from saved to return to the model you already selected.
    • diff --git a/ai-agent-local/build.gradle.kts b/ai-agent-local/build.gradle.kts index 0b99ef6b..638a36df 100644 --- a/ai-agent-local/build.gradle.kts +++ b/ai-agent-local/build.gradle.kts @@ -81,6 +81,9 @@ dependencies { testImplementation(files("../libs/plugin-api.jar")) testImplementation("junit:junit:4.13.2") testImplementation("io.mockk:mockk:1.13.8") + // LiveData's postValue needs the arch-core executor swapped for a synchronous one; the + // settings pane publishes its state through it, so its tests cannot run without this. + testImplementation("androidx.arch.core:core-testing:2.2.0") } // The one ABI this plugin ships. Shared by the packaging check and the unit tests. diff --git a/ai-agent-local/src/main/assets/docs/index.html b/ai-agent-local/src/main/assets/docs/index.html index 232bf4f3..5db9e206 100644 --- a/ai-agent-local/src/main/assets/docs/index.html +++ b/ai-agent-local/src/main/assets/docs/index.html @@ -61,8 +61,10 @@

    The settings pane

    picked and reads it where it is — on internal storage, an SD card or a USB volume. Nothing is copied, so a multi-gigabyte model costs no extra device storage. Keep the file where it is: moving or deleting it breaks the - selection. If the file is larger than the device's free RAM, a warning asks - you to confirm before loading. + selection. The picker offers only documents already stored on the device, + because a model still in a cloud folder has to be read as a stream and + cannot be loaded in place. If the file is larger than the device's free RAM, + a warning asks you to confirm before loading.
  • Load from saved — reloads the model you already selected without picking it again. Use this after restarting the IDE, or when a load failed for a transient reason such as low memory.
    • @@ -99,6 +101,11 @@

    Troubleshooting

    the file, or removing the SD card it lives on, breaks the selection. Clearing the IDE's app data also withdraws the permission to read it. Pick the model again with Browse. +
  • "This model is streamed from its storage location" — the file + you picked lives in a cloud folder (Google Drive, OneDrive) rather than on + the device, and can only be read as a stream. Download the + .gguf to the device — Downloads is fine — and pick + it from there.
    • diff --git a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/LocalLlmBackend.kt b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/LocalLlmBackend.kt index 30e8ada2..e29db646 100644 --- a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/LocalLlmBackend.kt +++ b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/LocalLlmBackend.kt @@ -115,21 +115,15 @@ class LocalLlmBackend( */ @Volatile private var openModel: OpenModelFile? = null - /** - * The reference last found unreachable, so the chat is told the backend is unavailable - * instead of being sent to a model that is gone. - * - * Held as the reference rather than a flag so picking a different model clears it by itself; - * a successful load clears it for the same one. - */ - @Volatile private var unreachableModelRef: String? = null - /** * Stops the delete watch on the resident model. Follows residency exactly: taken when a model * is adopted, closed when it is released. */ @Volatile private var modelWatch: Closeable? = null + /** Whether a watch-triggered reachability check is already queued; see [onModelSourceGone]. */ + private val sourceCheckInFlight = AtomicBoolean(false) + /** * Opens the configured model for the native loader. Lazy so construction touches no Android * services, and overridable so the load path can be tested without a device. @@ -237,12 +231,8 @@ class LocalLlmBackend( // Kept ahead of the check below so a model the user restores is picked up on the next ask. maybeWarmUp(configuredPath) - // A model whose file has gone away is not available, however resident its pages still are. - // Answered from the memo rather than probed here: this runs on the caller's thread, which - // may be the main one, and a document probe is a binder round trip. - if (!configuredPath.isNullOrBlank() && configuredPath == unreachableModelRef) return false - - // Available if model is loaded OR if a path is configured + // Unreachability is left to ensureModelLoaded: a memo here goes stale the moment the user + // restores the file, refusing their first message, and that path advises them properly. return modelLoaded || !configuredPath.isNullOrBlank() } @@ -305,6 +295,14 @@ class LocalLlmBackend( // file descriptor per failed attempt, and warm-up retries make that a loop. var adopted = false try { + // Before the first read: the openStream calls below would each eat bytes off a pipe + // llama.cpp never gets to read, leaving a fine model diagnosed as corrupt (ADFA-5253). + if (!opened.isSeekable) { + context.logger.warn("The selected model is not a local file: $modelRef") + val diagnosis = ModelLoadDiagnostics.Diagnosis.SourceNotSeekable + throw ModelLoadException(loadMessages.describe(diagnosis), diagnosis) + } + // One parse of the metadata block per load, feeding both the guard below and the // context sizing after the unload: it sits at the front of a multi-GB file, and a // model switch used to walk it twice. @@ -370,7 +368,6 @@ class LocalLlmBackend( modelLoaded = true currentModelRef = modelRef openModel = opened - unreachableModelRef = null adopted = true startWatching(modelRef) context.logger.info("Model loaded successfully") @@ -467,12 +464,11 @@ class LocalLlmBackend( } /** - * Records [modelRef] as unreachable and builds the failure to report for it. + * Builds the failure to report for a model that could not be opened at all. * * @return the exception to throw; never thrown here, so the caller's control flow stays visible */ private fun unopenable(modelRef: String): ModelLoadException { - unreachableModelRef = modelRef val diagnosis = ModelLoadDiagnostics.diagnoseUnopenable(modelRef) return ModelLoadException(loadMessages.describe(diagnosis), diagnosis) } @@ -507,15 +503,22 @@ class LocalLlmBackend( * * Runs under [generationMutex] on [cleanupScope]: a generation already in flight finishes on * the model it started with, and this survives the cancellation of [scope]. + * + * Coalesced through [sourceCheckInFlight]: a chatty provider would otherwise queue one + * coroutine and one binder probe per notification behind [generationMutex]. */ private fun onModelSourceGone(modelRef: String) { + if (!sourceCheckInFlight.compareAndSet(false, true)) return cleanupScope.launch { - generationMutex.withLock { - if (!modelLoaded || currentModelRef != modelRef) return@withLock - if (modelSource.isReachable(modelRef)) return@withLock - context.logger.info("Selected model was deleted; releasing it: $modelRef") - evictResidentModel() - unreachableModelRef = modelRef + try { + generationMutex.withLock { + if (!modelLoaded || currentModelRef != modelRef) return@withLock + if (modelSource.isReachable(modelRef)) return@withLock + context.logger.info("Selected model was deleted; releasing it: $modelRef") + evictResidentModel() + } + } finally { + sourceCheckInFlight.set(false) } } } diff --git a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadDiagnostics.kt b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadDiagnostics.kt index 706a058e..3e014021 100644 --- a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadDiagnostics.kt +++ b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadDiagnostics.kt @@ -33,6 +33,13 @@ object ModelLoadDiagnostics { */ data object SourceUnavailable : Diagnosis + /** + * The picked document is streamed rather than stored on the device, so its descriptor is a + * pipe the loader cannot `mmap` or re-open. Its own case because the alternative is + * reporting a perfectly good model as corrupt; the fix is to download it (ADFA-5253). + */ + data object SourceNotSeekable : Diagnosis + data object FileEmpty : Diagnosis data object NotGguf : Diagnosis /** @@ -99,14 +106,6 @@ object ModelLoadDiagnostics { else Diagnosis.UnsupportedOrCorrupt } - /** - * Whether to refuse a load outright, before ggml aborts the process trying it. Weighs only the - * compute buffers, so it stays far more permissive than [diagnose]'s attribution headroom: - * the memory-warning dialog lets the user proceed, and a refusal here must not overrule that. - * - * @param availableMemoryBytes free RAM reported by the OS, or negative if unknown - * @return the shortfall to refuse with, or null to attempt the load - */ /** * Why a model could not be opened at all, before any load was attempted. * @@ -116,6 +115,14 @@ object ModelLoadDiagnostics { if (modelReference.startsWith(CONTENT_SCHEME)) Diagnosis.SourceUnavailable else Diagnosis.FileMissing + /** + * Whether to refuse a load outright, before ggml aborts the process trying it. Weighs only the + * compute buffers, so it stays far more permissive than [diagnose]'s attribution headroom: + * the memory-warning dialog lets the user proceed, and a refusal here must not overrule that. + * + * @param availableMemoryBytes free RAM reported by the OS, or negative if unknown + * @return the shortfall to refuse with, or null to attempt the load + */ fun refuseBeforeLoad(availableMemoryBytes: Long): Diagnosis.LowMemory? = // Only a NEGATIVE reading means "unknown"; 0 is a genuine out-of-memory reading. if (availableMemoryBytes in 0L until MIN_RUN_BYTES) { diff --git a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadMessages.kt b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadMessages.kt index 70629409..99634bdd 100644 --- a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadMessages.kt +++ b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadMessages.kt @@ -23,6 +23,7 @@ internal class ModelLoadMessages(private val context: Context) { fun describe(diagnosis: Diagnosis): String = when (diagnosis) { Diagnosis.FileMissing -> context.getString(R.string.llm_load_error_missing) Diagnosis.SourceUnavailable -> context.getString(R.string.llm_load_error_unavailable) + Diagnosis.SourceNotSeekable -> context.getString(R.string.llm_load_error_not_seekable) Diagnosis.FileEmpty -> context.getString(R.string.llm_load_error_empty) Diagnosis.NotGguf -> context.getString(R.string.llm_load_error_not_gguf) is Diagnosis.LowMemory -> context.getString( diff --git a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelSourceWatcher.kt b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelSourceWatcher.kt index c3e5aa13..a76514a4 100644 --- a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelSourceWatcher.kt +++ b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelSourceWatcher.kt @@ -31,9 +31,9 @@ interface ModelSourceWatcher { /** * [ModelSourceWatcher] over the document provider and the filesystem. * - * Callbacks arrive on a private [HandlerThread] — never the main thread, and never a thread the - * caller owns — started with the first watch and stopped with the last, so an idle plugin holds - * no thread. See ADFA-5253. + * Never the main thread, and never a thread the caller owns: a document watch arrives on a private + * [HandlerThread], started with the first such watch and stopped with the last so an idle plugin + * holds no thread, and a filesystem watch on [FileObserver]'s own. See ADFA-5253. * * @param onError reports a failed registration, so a silently unwatched model can be explained */ diff --git a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/NativeModelSource.kt b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/NativeModelSource.kt index 0738cb25..5c5219d6 100644 --- a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/NativeModelSource.kt +++ b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/NativeModelSource.kt @@ -20,7 +20,7 @@ import java.io.InputStream * model alive after that. [close] is therefore the unload path's job, not the load path's. * * @property nativePath the path to hand the native loader - * @property sizeBytes the model's size, or -1 when the source could not report one + * @property sizeBytes the model's size, or -1 when the descriptor names no regular file */ class OpenModelFile( val nativePath: String, @@ -28,6 +28,13 @@ class OpenModelFile( private val descriptor: Closeable?, ) : Closeable { + /** + * Whether [nativePath] can be `mmap`ed and re-opened, which everything above assumes. A + * streaming provider (Drive, OneDrive) hands back a pipe instead, for which `statSize` is -1 + * and each [openStream] eats bytes the loader never sees, so the caller must refuse it. + */ + val isSeekable: Boolean get() = sizeBytes >= 0 + /** * Opens an independent read stream over the same bytes the native loader sees — header * inspection must never disturb the loader's own file offset. @@ -91,7 +98,8 @@ class ContentNativeModelSource( /** * Takes the document's descriptor and hands the native loader its procfs path. `"r"` is the - * only mode asked for, which is all the persisted grant covers. + * only mode asked for, which is all the persisted grant covers. The descriptor need not be a + * file — a pipe is reported through [OpenModelFile.isSeekable] for the caller to refuse. */ private fun openDocument(uriString: String): OpenModelFile? = try { context.contentResolver.openFileDescriptor(Uri.parse(uriString), "r") diff --git a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsFragment.kt b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsFragment.kt index 49b22e6d..4329a4ae 100644 --- a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsFragment.kt +++ b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsFragment.kt @@ -1,5 +1,7 @@ package com.itsaky.androidide.plugins.aiagentlocal.settings +import android.content.Context +import android.content.Intent import android.net.Uri import android.os.Bundle import android.view.LayoutInflater @@ -36,7 +38,7 @@ class LocalLlmSettingsFragment : Fragment(), MemoryWarningDialogFragment.Host { private var tooltipService: IdeTooltipService? = null private val filePickerLauncher = - registerForActivityResult(ActivityResultContracts.OpenDocument()) { uri: Uri? -> + registerForActivityResult(PickLocalDocument) { uri: Uri? -> uri?.let { try { // The durable read grant is taken by the view model, with the rest of the @@ -175,7 +177,9 @@ class LocalLlmSettingsFragment : Fragment(), MemoryWarningDialogFragment.Host { val savedName = state.savedModelName if (savedName != null) { modelPathTextView.visibility = View.VISIBLE - modelPathTextView.text = if (state.model is ModelLoadingState.Unavailable) { + // Off the engine status, which describes the configured model; the model status + // also carries the outcome of a rejected pick, which says nothing about it. + modelPathTextView.text = if (state.engine is EngineState.ModelUnavailable) { getString(R.string.model_saved_path_unavailable, savedName) } else { getString(R.string.model_saved_path, savedName) @@ -253,6 +257,16 @@ class LocalLlmSettingsFragment : Fragment(), MemoryWarningDialogFragment.Host { } } +/** + * The document picker, asked for documents already on the device: a streaming provider hands back + * a pipe the in-place loader cannot `mmap`. Advisory only, so the load path still refuses a + * non-seekable descriptor as `Diagnosis.SourceNotSeekable` (ADFA-5253). + */ +private object PickLocalDocument : ActivityResultContracts.OpenDocument() { + override fun createIntent(context: Context, input: Array): Intent = + super.createIntent(context, input).putExtra(Intent.EXTRA_LOCAL_ONLY, true) +} + /** * Factory for creating [LocalLlmSettingsViewModel] with its PluginContext dependency. */ diff --git a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsViewModel.kt b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsViewModel.kt index e6582e4d..360221c7 100644 --- a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsViewModel.kt +++ b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsViewModel.kt @@ -223,6 +223,27 @@ class LocalLlmSettingsViewModel( update { it.copy(model = model, engine = engineStateFor(model) ?: it.engine) } } + /** + * Publishes a selection that was not kept: the model line says what went wrong with the pick, + * the engine line keeps describing the *configured* model. A pick of another file hands the + * engine back untouched; "Load from saved" re-picks the configured one, so its failure counts. + * + * @param uriString the pick that was abandoned + * @param model what to say about it + * @param engineBefore the engine status from before the selection started + */ + private fun publishAbandonedSelection( + uriString: String, + model: ModelLoadingState, + engineBefore: EngineState, + ) { + val engine = + if (uriString == getLocalModelPath()) engineStateFor(model) ?: engineBefore + else engineBefore + + update { it.copy(model = model, engine = engine) } + } + /** * Engine readiness implied by a model status, or null to leave the engine's status alone. * @@ -322,6 +343,9 @@ class LocalLlmSettingsViewModel( return } + // Taken before the Loading below overwrites it; an abandoned pick puts it back. + val stateBefore = current + viewModelScope.launch(ioDispatcher) { publishModelState(ModelLoadingState.Loading) @@ -341,17 +365,23 @@ class LocalLlmSettingsViewModel( // saved" case after the file was deleted — is not reported as a corrupt one. if (!modelFiles.isReadable(context, uriString)) { releaseUnkeptGrant(context, uriString) - publishModelState(ModelLoadingState.Unavailable(fileName)) + publishAbandonedSelection( + uriString, + ModelLoadingState.Unavailable(fileName), + stateBefore.engine, + ) return@launch } // Rejected up front, so no bad path is persisted or shown as "Loaded". if (!GgufFileInspector.looksLikeGguf(context.contentResolver, uriString)) { releaseUnkeptGrant(context, uriString) - publishModelState( + publishAbandonedSelection( + uriString, ModelLoadingState.Error( context.getString(R.string.error_model_not_gguf, fileName) - ) + ), + stateBefore.engine, ) return@launch } @@ -359,7 +389,7 @@ class LocalLlmSettingsViewModel( if (!confirmMemoryHeadroom(uriString, fileInfo, context)) { logger?.info("$TAG: model declined at the memory warning: $fileName") releaseUnkeptGrant(context, uriString) - restoreSavedModelState() + restoreStateBefore(stateBefore) return@launch } @@ -381,10 +411,12 @@ class LocalLlmSettingsViewModel( throw e } catch (e: Exception) { logger?.error("$TAG: error saving model path", e) - publishModelState( + publishAbandonedSelection( + uriString, ModelLoadingState.Error( context.getString(R.string.error_model_save_failed, e.message.orEmpty()) - ) + ), + stateBefore.engine, ) } } @@ -465,10 +497,13 @@ class LocalLlmSettingsViewModel( } /** - * Republishes the model that is actually configured, so abandoning a selection leaves the - * screen describing the previous model rather than the one that was never stored. + * Puts the screen back as it was before a selection the user declined outright. Restores both + * lines rather than re-deriving them: a configured model that was already unreachable must + * stay reported that way. + * + * @param stateBefore the state captured before the selection started */ - private fun restoreSavedModelState() { - publishModelState(modelStateFor(getLocalModelPath())) + private fun restoreStateBefore(stateBefore: LocalLlmSettingsState) { + update { stateBefore } } } diff --git a/ai-agent-local/src/main/res/values/strings.xml b/ai-agent-local/src/main/res/values/strings.xml index b54fc51a..6098af12 100644 --- a/ai-agent-local/src/main/res/values/strings.xml +++ b/ai-agent-local/src/main/res/values/strings.xml @@ -4,6 +4,7 @@ The model file could not be found. Re-select the .gguf model in AI Settings. The selected model can no longer be reached. It may have been moved, deleted, or saved to storage that isn\'t connected right now, or the IDE\'s permission to read it was withdrawn. Select the .gguf model again in AI Settings. + This model is streamed from its storage location rather than stored on this device, so it can\'t be read in place. Download the .gguf to the device — for example to Downloads — and select it from there. The model file is empty — the download may have been interrupted. Re-download the .gguf model and select it again. This file isn\'t a valid .gguf model (it may be corrupt or only partially downloaded). Re-download the model and select it again. Loading this model needs at least %1$s of free memory, but only %2$s is available on this device. Close other apps and try again, or pick a smaller or more heavily quantized model (for example a Q4_K_M build of a 1–3B model). diff --git a/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/LocalLlmBackendTest.kt b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/LocalLlmBackendTest.kt index 5e157527..45941bb2 100644 --- a/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/LocalLlmBackendTest.kt +++ b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/LocalLlmBackendTest.kt @@ -45,13 +45,18 @@ class LocalLlmBackendTest { /** Flipped to simulate the user deleting the file out from under a resident model. */ var reachable = true + /** Reachability probes served, so a burst of watch notifications can be counted. */ + @Volatile var probeCount = 0 + override fun open(modelReference: String): OpenModelFile? { openCount++ return handles[modelReference].takeIf { reachable } } - override fun isReachable(modelReference: String): Boolean = - reachable && handles.containsKey(modelReference) + override fun isReachable(modelReference: String): Boolean { + probeCount++ + return reachable && handles.containsKey(modelReference) + } } /** Stands in for the native engine, recording residency without loading any weights. */ @@ -171,6 +176,44 @@ class LocalLlmBackendTest { assertEquals(Diagnosis.FileMissing, error.diagnosis) } + @Test + fun givenAStreamingDocument_whenLoading_thenRefusedAsNotSeekableWithoutReadingIt() { + // A cloud provider hands back a pipe, whose bytes the header reads would consume before + // llama.cpp sees any: refuse it with its own advice rather than call it corrupt. + val descriptor = RecordingDescriptor() + val pipe = OpenModelFile("/proc/self/fd/7", -1L, descriptor) + val source = FakeModelSource(mapOf(CONTENT_URI to pipe)) + val engine = FakeEngine() + + val error = assertThrows(ModelLoadException::class.java) { + runBlocking { backendWith(source, engine).ensureModelLoaded(CONTENT_URI) } + } + + assertEquals(Diagnosis.SourceNotSeekable, error.diagnosis) + assertEquals("nothing may reach the engine", 0, engine.loadCount) + assertTrue("the refused descriptor must not leak", descriptor.closed) + } + + @Test + fun givenAResidentModel_whenItsWatchFiresRepeatedly_thenOnlyOneCheckIsQueued() { + // One coroutine per notification would pile up behind generationMutex, each waking to + // issue its own binder probe; the gate collapses a burst to a single check. + val source = FakeModelSource(mapOf(CONTENT_URI to handleFor(chatModel()))) + val engine = FakeEngine() + val watcher = FakeWatcher() + val backend = backendWith(source, engine, watcher) + + runBlocking { backend.ensureModelLoaded(CONTENT_URI) } + val before = source.probeCount + repeat(50) { watcher.onGone!!.invoke() } + + Thread.sleep(300) + val probes = source.probeCount - before + // Not exactly one: a notification arriving just after a check rightly starts another. + assertTrue("a burst of 50 notifications cost $probes probes", probes in 1..5) + assertEquals("a reachable model must stay loaded", 0, engine.unloadCount) + } + @Test fun givenAnEmbeddingModel_whenLoading_thenRejectedBeforeAnyNativeWork() { // ADFA-4388: the classify guard must still fire when the header arrives as a stream over a diff --git a/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ContentNativeModelSourceTest.kt b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ContentNativeModelSourceTest.kt index 9f377ccd..d1538421 100644 --- a/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ContentNativeModelSourceTest.kt +++ b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ContentNativeModelSourceTest.kt @@ -100,6 +100,49 @@ class ContentNativeModelSourceTest { io.mockk.verify { descriptor.close() } } + @Test + fun givenADocument_whenOpened_thenTheLoaderGetsItsProcfsPathAndSize() { + // The contract the read-in-place change rests on: llama.cpp gets the procfs entry for the + // descriptor this handle owns, and the size comes from statSize, not File.length(). + val descriptor = mockk(relaxed = true) + every { descriptor.fd } returns 42 + every { descriptor.statSize } returns 4_294_967_296L + every { resolver.openFileDescriptor(any(), "r") } returns descriptor + + val opened = source.open(CONTENT_URI) + + assertNotNull(opened) + assertEquals("/proc/self/fd/42", opened!!.nativePath) + assertEquals(4_294_967_296L, opened.sizeBytes) + assertTrue(opened.isSeekable) + // The descriptor belongs to the handle now: closing it here would invalidate the path. + io.mockk.verify(exactly = 0) { descriptor.close() } + } + + @Test + fun givenAStreamingProvider_whenOpened_thenTheHandleIsNotSeekable() { + // A cloud provider hands back a pipe, for which statSize is -1: the caller has to be able + // to tell, or a perfectly good model is reported as corrupt. + val descriptor = mockk(relaxed = true) + every { descriptor.fd } returns 7 + every { descriptor.statSize } returns -1L + every { resolver.openFileDescriptor(any(), "r") } returns descriptor + + assertFalse(source.open(CONTENT_URI)!!.isSeekable) + } + + @Test + fun givenAPathThatExists_whenOpened_thenItIsSeekableAtItsOwnPath() { + // A filesystem path has no descriptor to keep, and must not read as a pipe. + val model = temporaryFolder.newFile("model.gguf").apply { writeBytes(ByteArray(64)) } + + val opened = source.open(model.absolutePath) + + assertEquals(model.absolutePath, opened!!.nativePath) + assertEquals(64L, opened.sizeBytes) + assertTrue(opened.isSeekable) + } + @Test fun givenADeletedPath_whenOpened_thenNoHandleIsReturned() { val model = temporaryFolder.newFile("model.gguf") diff --git a/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadMessagesTest.kt b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadMessagesTest.kt index 095bd417..2cb3f6ad 100644 --- a/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadMessagesTest.kt +++ b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadMessagesTest.kt @@ -31,6 +31,13 @@ class ModelLoadMessagesTest { verify { context.getString(R.string.llm_load_error_unavailable) } } + @Test + fun givenSourceNotSeekable_whenDescribed_thenNotSeekableString() { + // Must not share the corrupt-model wording: the model is fine, its location is the problem. + messages.describe(Diagnosis.SourceNotSeekable) + verify { context.getString(R.string.llm_load_error_not_seekable) } + } + @Test fun givenFileEmpty_whenDescribed_thenEmptyString() { messages.describe(Diagnosis.FileEmpty) diff --git a/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsViewModelTest.kt b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsViewModelTest.kt new file mode 100644 index 00000000..99fca1ff --- /dev/null +++ b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsViewModelTest.kt @@ -0,0 +1,241 @@ +package com.itsaky.androidide.plugins.aiagentlocal.settings + +import android.content.ContentResolver +import android.content.Context +import android.content.SharedPreferences +import android.net.Uri +import androidx.arch.core.executor.testing.InstantTaskExecutorRule +import com.itsaky.androidide.plugins.PluginContext +import com.itsaky.androidide.plugins.aiagentlocal.model.DeviceMemory +import com.itsaky.androidide.plugins.aiagentlocal.model.ModelFileInfo +import com.itsaky.androidide.plugins.aiagentlocal.model.ModelFileSource +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkStatic +import io.mockk.unmockkStatic +import java.io.ByteArrayInputStream +import java.io.InputStream +import kotlinx.coroutines.Dispatchers +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Rule +import org.junit.Test + +/** + * The persisted read grant is the only thing keeping a model readable now that nothing is copied, + * so releasing the wrong one strands a model the user is still running. These pin the grant + * lifecycle across the paths that abandon a selection. See ADFA-5253. + */ +class LocalLlmSettingsViewModelTest { + + @get:Rule + val instantTaskExecutorRule = InstantTaskExecutorRule() + + /** Records what was granted and given back, and answers as the file source would. */ + private class FakeModelFiles : ModelFileSource { + val persisted = mutableListOf() + val released = mutableListOf() + + /** References the provider will not serve, standing in for a deleted document. */ + val unreadable = mutableSetOf() + + override fun info(context: Context, uriString: String) = + ModelFileInfo(fallbackDisplayName(uriString), 1_024L) + + override fun openStream(context: Context, uriString: String): InputStream? = null + + override fun isReadable(context: Context, uriString: String) = uriString !in unreadable + + override fun fallbackDisplayName(uriOrPath: String) = uriOrPath.substringAfterLast('/') + + override fun persistAccess(context: Context, uriString: String): Boolean { + persisted += uriString + return true + } + + override fun releaseAccess(context: Context, uriString: String) { + released += uriString + } + } + + private lateinit var stored: MutableMap + private lateinit var resolver: ContentResolver + private lateinit var pluginContext: PluginContext + private lateinit var modelFiles: FakeModelFiles + + @Before + fun setup() { + mockkStatic(Uri::class) + every { Uri.parse(any()) } returns mockk(relaxed = true) + every { Uri.decode(any()) } answers { firstArg() } + + stored = mutableMapOf() + val prefs = mockk(relaxed = true) + val editor = mockk(relaxed = true) + every { prefs.getString(any(), any()) } answers { stored[firstArg()] ?: secondArg() } + every { prefs.edit() } returns editor + every { editor.putString(any(), any()) } answers { + stored[firstArg()] = secondArg() + editor + } + + resolver = mockk(relaxed = true) + // The GGUF sniff fails OPEN, so a pick is accepted unless a test serves other bytes. + every { resolver.openInputStream(any()) } returns null + val androidContext = mockk(relaxed = true) + every { androidContext.contentResolver } returns resolver + + pluginContext = mockk(relaxed = true) + every { pluginContext.androidContext } returns androidContext + every { pluginContext.getPluginSharedPreferences(any()) } returns prefs + + modelFiles = FakeModelFiles() + } + + @After + fun tearDown() { + unmockkStatic(Uri::class) + } + + /** + * Unconfined, so every launch runs inline: nothing here suspends on a real dispatcher, and the + * memory pre-flight fails open on the fake's unreadable header. + */ + private fun viewModel() = LocalLlmSettingsViewModel( + getContext = { pluginContext }, + ioDispatcher = Dispatchers.Unconfined, + deviceMemory = DeviceMemory { null }, + modelFiles = modelFiles, + ) + + @Test + fun givenASelection_whenItIsKept_thenItsGrantIsPersistedAndStored() { + val viewModel = viewModel() + + viewModel.loadModelFromUri(MODEL_A) + + assertEquals(listOf(MODEL_A), modelFiles.persisted) + assertEquals(emptyList(), modelFiles.released) + assertEquals(MODEL_A, viewModel.getLocalModelPath()) + assertEquals(ModelLoadingState.Loaded("a.gguf"), viewModel.state.value?.model) + } + + @Test + fun givenAConfiguredModel_whenAnotherIsSelected_thenOnlyTheReplacedGrantIsReleased() { + // Grants are capped per app, so the model no longer read by anything has to give its back. + val viewModel = viewModel() + viewModel.loadModelFromUri(MODEL_A) + + viewModel.loadModelFromUri(MODEL_B) + + assertEquals(listOf(MODEL_A, MODEL_B), modelFiles.persisted) + assertEquals(listOf(MODEL_A), modelFiles.released) + assertEquals(MODEL_B, viewModel.getLocalModelPath()) + } + + @Test + fun givenAConfiguredModel_whenItIsReSelected_thenItsGrantIsNotReleased() { + // "Load from saved" re-picks the configured model; releasing here would revoke the grant + // on the model the user is still running. + val viewModel = viewModel() + viewModel.loadModelFromUri(MODEL_A) + + viewModel.loadModelFromUri(MODEL_A) + + assertEquals(emptyList(), modelFiles.released) + assertEquals(MODEL_A, viewModel.getLocalModelPath()) + } + + @Test + fun givenAConfiguredModelThatIsGone_whenItIsReSelected_thenItsGrantSurvivesTheFailure() { + // The model may be on storage that is merely unmounted; re-mounting must not need a pick. + val viewModel = viewModel() + viewModel.loadModelFromUri(MODEL_A) + modelFiles.unreadable += MODEL_A + + viewModel.loadModelFromUri(MODEL_A) + + assertEquals(emptyList(), modelFiles.released) + assertEquals(ModelLoadingState.Unavailable("a.gguf"), viewModel.state.value?.model) + assertEquals(EngineState.ModelUnavailable, viewModel.state.value?.engine) + } + + @Test + fun givenANewSelectionThatIsRejected_thenItsOwnGrantIsGivenBackAndTheConfiguredOneKept() { + val viewModel = viewModel() + viewModel.loadModelFromUri(MODEL_A) + modelFiles.unreadable += MODEL_B + + viewModel.loadModelFromUri(MODEL_B) + + assertEquals(listOf(MODEL_B), modelFiles.released) + assertEquals("the configured model must survive a failed pick", MODEL_A, viewModel.getLocalModelPath()) + } + + @Test + fun givenANonGgufSelection_thenItIsRejectedWithoutBeingStoredAndItsGrantIsReleased() { + every { resolver.openInputStream(any()) } answers { ByteArrayInputStream("NOPE".toByteArray()) } + val viewModel = viewModel() + + viewModel.loadModelFromUri(MODEL_B) + + assertEquals(listOf(MODEL_B), modelFiles.released) + assertEquals(null, viewModel.getLocalModelPath()) + assertTrue(viewModel.state.value?.model is ModelLoadingState.Error) + } + + @Test + fun givenARejectedSelection_thenTheConfiguredModelsReadinessIsLeftAlone() { + // The pane keys its "(unavailable)" marker off the engine status, so a rejected pick of + // another file must leave it alone. + every { resolver.openInputStream(any()) } answers { ByteArrayInputStream("NOPE".toByteArray()) } + val viewModel = viewModel() + stored[KEY_MODEL_PATH] = MODEL_A + viewModel.refreshSavedModelAvailability() + modelFiles.unreadable += MODEL_A + viewModel.refreshSavedModelAvailability() + assertEquals(EngineState.ModelUnavailable, viewModel.state.value?.engine) + + viewModel.loadModelFromUri(MODEL_B) + + // Not Initializing: the pick published that on its way in and never got anywhere. + assertEquals(EngineState.ModelUnavailable, viewModel.state.value?.engine) + assertTrue(viewModel.state.value?.model is ModelLoadingState.Error) + } + + @Test + fun givenAConfiguredModelThatWentAway_whenTheScreenReturns_thenItIsReportedUnavailable() { + val viewModel = viewModel() + viewModel.loadModelFromUri(MODEL_A) + modelFiles.unreadable += MODEL_A + + viewModel.refreshSavedModelAvailability() + + assertEquals(ModelLoadingState.Unavailable("a.gguf"), viewModel.state.value?.model) + assertEquals(EngineState.ModelUnavailable, viewModel.state.value?.engine) + assertEquals("a re-check must not touch the grant", emptyList(), modelFiles.released) + } + + @Test + fun givenAModelThatCameBack_whenTheScreenReturns_thenItIsReportedReadyAgain() { + // Unmounted storage comes back; the stale "unavailable" has to clear without a fresh pick. + val viewModel = viewModel() + viewModel.loadModelFromUri(MODEL_A) + modelFiles.unreadable += MODEL_A + viewModel.refreshSavedModelAvailability() + + modelFiles.unreadable -= MODEL_A + viewModel.refreshSavedModelAvailability() + + assertEquals(ModelLoadingState.Loaded("a.gguf"), viewModel.state.value?.model) + assertEquals(EngineState.Initialized, viewModel.state.value?.engine) + } + + private companion object { + const val MODEL_A = "content://com.android.externalstorage.documents/document/a.gguf" + const val MODEL_B = "content://com.android.externalstorage.documents/document/b.gguf" + const val KEY_MODEL_PATH = "local_llm_model_path" + } +} From 86da4412c54dff5a9285a3b032089f5757e2db57 Mon Sep 17 00:00:00 2001 From: John Trujillo Date: Fri, 4 Sep 2026 11:41:26 -0500 Subject: [PATCH 3/3] fix(ai-agent-local): address round-2 review on read-in-place model loading Separate "the provider said no" from "the provider did not answer" so a dead DocumentsProvider no longer evicts a resident multi-GB model, watch the parent's children URI where a delete is actually notified, and refuse a procfs path the native loader cannot re-open with its own diagnosis. --- .../src/main/assets/docs/index.html | 19 ++-- .../aiagentlocal/backend/LocalLlmBackend.kt | 39 +++++--- .../aiagentlocal/model/ModelFileSource.kt | 3 +- .../model/ModelLoadDiagnostics.kt | 7 ++ .../aiagentlocal/model/ModelLoadMessages.kt | 1 + .../aiagentlocal/model/ModelSourceWatcher.kt | 46 ++++++++- .../aiagentlocal/model/NativeModelSource.kt | 75 +++++++++++--- .../settings/LocalLlmSettingsViewModel.kt | 28 +++--- .../src/main/res/values/strings.xml | 1 + .../backend/LocalLlmBackendTest.kt | 99 ++++++++++++++++++- .../model/ContentNativeModelSourceTest.kt | 56 ++++++++--- .../model/ModelSourceWatcherTest.kt | 78 +++++++++++++++ .../settings/LocalLlmSettingsViewModelTest.kt | 87 ++++++++++++++-- 13 files changed, 465 insertions(+), 74 deletions(-) create mode 100644 ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelSourceWatcherTest.kt diff --git a/ai-agent-local/src/main/assets/docs/index.html b/ai-agent-local/src/main/assets/docs/index.html index 5db9e206..d0340c77 100644 --- a/ai-agent-local/src/main/assets/docs/index.html +++ b/ai-agent-local/src/main/assets/docs/index.html @@ -58,12 +58,14 @@

    The settings pane

    • Browse — opens the system file picker to choose a .gguf model. The plugin keeps read access to the document you - picked and reads it where it is — on internal storage, an SD card or a USB - volume. Nothing is copied, so a multi-gigabyte model costs no extra device - storage. Keep the file where it is: moving or deleting it breaks the - selection. The picker offers only documents already stored on the device, - because a model still in a cloud folder has to be read as a stream and - cannot be loaded in place. If the file is larger than the device's free RAM, + picked and reads it where it is, so nothing is copied and a multi-gigabyte + model costs no extra device storage. Internal storage always works; a + removable volume such as an SD card or a USB drive normally does too, and + when one won't allow a direct read the plugin says so and asks you to copy + the model to internal storage. Keep the file where it is: moving or + deleting it breaks the selection. The picker offers only documents already + stored on the device, because a model still in a cloud folder has to be + read as a stream and cannot be loaded in place. If the file is larger than the device's free RAM, a warning asks you to confirm before loading.
    • Load from saved — reloads the model you already selected without picking it again. Use this after restarting the IDE, or when a load @@ -101,6 +103,11 @@

      Troubleshooting

      the file, or removing the SD card it lives on, breaks the selection. Clearing the IDE's app data also withdraws the permission to read it. Pick the model again with Browse.
      • +
    • "This model can't be read from where it is stored" — the + volume the file sits on doesn't let the IDE open it directly, which can + happen on some removable storage. Copy the .gguf to the + device's internal storage — Downloads is fine — and pick it from + there.
    • "This model is streamed from its storage location" — the file you picked lives in a cloud folder (Google Drive, OneDrive) rather than on the device, and can only be read as a stream. Download the diff --git a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/LocalLlmBackend.kt b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/LocalLlmBackend.kt index e29db646..1b9fc4a3 100644 --- a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/LocalLlmBackend.kt +++ b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/LocalLlmBackend.kt @@ -22,6 +22,7 @@ import com.itsaky.androidide.plugins.aiagentlocal.model.ModelSourceWatcher import com.itsaky.androidide.plugins.aiagentlocal.model.NativeModelSource import com.itsaky.androidide.plugins.aiagentlocal.model.OpenModelFile import com.itsaky.androidide.plugins.aiagentlocal.model.PlatformModelSourceWatcher +import com.itsaky.androidide.plugins.aiagentlocal.model.SourceReachability import com.itsaky.androidide.plugins.aiagentlocal.preferences.LocalLlmPreferences import com.itsaky.androidide.plugins.aiagentlocal.prompt.LocalSystemPrompt import com.itsaky.androidide.plugins.services.LlmInferenceService @@ -31,6 +32,7 @@ import java.io.Closeable import java.io.File import java.util.concurrent.CompletableFuture import java.util.concurrent.atomic.AtomicBoolean +import java.util.concurrent.atomic.AtomicReference import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers @@ -165,8 +167,12 @@ class LocalLlmBackend( } } - /** Ensures the background warm-up load is launched at most once. */ - private val warmUpStarted = AtomicBoolean(false) + /** + * The reference the background warm-up has already been launched for. Keyed on the reference + * rather than a flag: a failed warm-up leaves the same path configured, so re-arming it would + * launch one doomed load per [isAvailable] call, which the chat screen makes on open. + */ + private val warmedUpRef = AtomicReference(null) init { scope.launch { deleteLegacyModelCache() } @@ -228,7 +234,7 @@ class LocalLlmBackend( context.logger.debug("LocalLlmBackend.isAvailable() - configured path: $configuredPath, modelLoaded: $modelLoaded") // Chat-open hits this; start loading now so the first message isn't gated on a cold load. - // Kept ahead of the check below so a model the user restores is picked up on the next ask. + // Only ever the first ask for a given selection — a restored file is loaded by the send. maybeWarmUp(configuredPath) // Unreachability is left to ensureModelLoaded: a memo here goes stale the moment the user @@ -237,16 +243,17 @@ class LocalLlmBackend( } /** - * Preloads the configured model in the background, once, so the first generation - * doesn't pay the cold-load cost. No-op unless this backend is the selected one — warming a - * multi-gigabyte model for a user who picked a cloud backend would be pure waste. + * Preloads the configured model in the background, once per selection, so the first generation + * doesn't pay the cold-load cost. No-op unless this backend is the selected one, and never + * retried for a reference that failed — the generation path loads and diagnoses that one. * * @param configuredPath the configured model path/URI, or null/blank if unset. */ private fun maybeWarmUp(configuredPath: String?) { if (configuredPath.isNullOrBlank() || modelLoaded) return if (!isSelectedBackend()) return - if (!warmUpStarted.compareAndSet(false, true)) return + // A different selection re-arms it; the same one, failed or not, does not. + if (warmedUpRef.getAndSet(configuredPath) == configuredPath) return scope.launch { try { @@ -254,9 +261,8 @@ class LocalLlmBackend( generationMutex.withLock { ensureModelLoaded(configuredPath) } context.logger.info("Local model warm-up complete") } catch (e: Exception) { - // Stay silent (the real send surfaces config errors); allow a later retry. + // Stay silent and do not re-arm: the real send surfaces config errors. context.logger.warn("Local model warm-up failed: ${e.message}") - warmUpStarted.set(false) } } } @@ -283,7 +289,8 @@ class LocalLlmBackend( // Residency is not evidence the file still exists. The descriptor this backend holds // keeps a deleted inode alive, so an unchecked early return keeps answering from a // model the user threw away — and keeps its gigabytes mapped. Confirm, then serve. - if (modelSource.isReachable(modelRef)) return + // Anything but GONE is served: a silent provider is no reason to pay a GB reload. + if (modelSource.reachabilityOf(modelRef) != SourceReachability.GONE) return context.logger.info("Resident model is no longer reachable; unloading: $modelRef") evictResidentModel() throw unopenable(modelRef) @@ -303,6 +310,13 @@ class LocalLlmBackend( throw ModelLoadException(loadMessages.describe(diagnosis), diagnosis) } + // Or it arrives as the loader's null handle: "pick it again" for a file that is there. + if (!withContext(Dispatchers.IO) { opened.isReopenable() }) { + context.logger.warn("The selected model cannot be re-opened by path: $modelRef") + val diagnosis = ModelLoadDiagnostics.Diagnosis.SourceNotReopenable + throw ModelLoadException(loadMessages.describe(diagnosis), diagnosis) + } + // One parse of the metadata block per load, feeding both the guard below and the // context sizing after the unload: it sits at the front of a multi-GB file, and a // model switch used to walk it twice. @@ -447,8 +461,6 @@ class LocalLlmBackend( currentModelRef = null openModel?.close() openModel = null - // Re-arm the warm-up: a model that becomes reachable again is loaded without a restart. - warmUpStarted.set(false) } /** @@ -513,7 +525,8 @@ class LocalLlmBackend( try { generationMutex.withLock { if (!modelLoaded || currentModelRef != modelRef) return@withLock - if (modelSource.isReachable(modelRef)) return@withLock + // Only what the provider itself called gone may cost a model its pages. + if (modelSource.reachabilityOf(modelRef) != SourceReachability.GONE) return@withLock context.logger.info("Selected model was deleted; releasing it: $modelRef") evictResidentModel() } diff --git a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelFileSource.kt b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelFileSource.kt index c783ca4c..c9f517c8 100644 --- a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelFileSource.kt +++ b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelFileSource.kt @@ -133,8 +133,9 @@ class ContentModelFileSource( Uri.parse(uriString), Intent.FLAG_GRANT_READ_URI_PERMISSION, ) + } catch (_: SecurityException) { + // Nothing was held, or it was already released: the no-op this documents. } catch (e: Exception) { - // Never held, or already released — nothing is broken either way. onError("could not release the read grant for $uriString", e) } } diff --git a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadDiagnostics.kt b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadDiagnostics.kt index 3e014021..c8378de7 100644 --- a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadDiagnostics.kt +++ b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadDiagnostics.kt @@ -40,6 +40,13 @@ object ModelLoadDiagnostics { */ data object SourceNotSeekable : Diagnosis + /** + * The document opened, but the path standing in for it cannot be opened by name — the only + * way the native loader uses it. Its own case because "pick the model again" is useless + * advice for a file sitting where the user left it; see [OpenModelFile.isReopenable]. + */ + data object SourceNotReopenable : Diagnosis + data object FileEmpty : Diagnosis data object NotGguf : Diagnosis /** diff --git a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadMessages.kt b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadMessages.kt index 99634bdd..45ae9877 100644 --- a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadMessages.kt +++ b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelLoadMessages.kt @@ -24,6 +24,7 @@ internal class ModelLoadMessages(private val context: Context) { Diagnosis.FileMissing -> context.getString(R.string.llm_load_error_missing) Diagnosis.SourceUnavailable -> context.getString(R.string.llm_load_error_unavailable) Diagnosis.SourceNotSeekable -> context.getString(R.string.llm_load_error_not_seekable) + Diagnosis.SourceNotReopenable -> context.getString(R.string.llm_load_error_not_reopenable) Diagnosis.FileEmpty -> context.getString(R.string.llm_load_error_empty) Diagnosis.NotGguf -> context.getString(R.string.llm_load_error_not_gguf) is Diagnosis.LowMemory -> context.getString( diff --git a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelSourceWatcher.kt b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelSourceWatcher.kt index a76514a4..a033c053 100644 --- a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelSourceWatcher.kt +++ b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelSourceWatcher.kt @@ -6,8 +6,10 @@ import android.net.Uri import android.os.FileObserver import android.os.Handler import android.os.HandlerThread +import android.provider.DocumentsContract import java.io.Closeable import java.io.File +import java.util.concurrent.atomic.AtomicBoolean /** * Watches the file behind a resident model and reports when it goes away, so its gigabytes are @@ -63,9 +65,9 @@ class PlatformModelSourceWatcher( } /** - * Providers notify on their own terms — often for the parent tree rather than the document, - * and often for edits rather than deletion — so this registers for descendants too and lets - * the callback decide. `onGone` is a hint, never a verdict. + * Registers on the document URI *and* on [parentChildrenUriOf] it, which is where a provider + * actually notifies a delete and is no descendant of the document URI. Both stay hints, never + * verdicts — the parent's URI fires for every sibling too — so the callback confirms first. */ private fun watchDocument(uriString: String, onGone: () -> Unit): Closeable { val uri = Uri.parse(uriString) @@ -74,12 +76,17 @@ class PlatformModelSourceWatcher( } try { context.contentResolver.registerContentObserver(uri, true, observer) + // Null for a document at the root of its volume; the direct watch then stands alone. + parentChildrenUriOf(uri)?.let { + context.contentResolver.registerContentObserver(it, true, observer) + } } catch (e: Exception) { // The handler is already counted; give it back or the thread outlives every watch. releaseHandler() throw e } - return Closeable { + // One unregister covers both registrations — the resolver keys them by observer. + return closeOnce { try { context.contentResolver.unregisterContentObserver(observer) } finally { @@ -101,7 +108,16 @@ class PlatformModelSourceWatcher( // The framework holds FileObserver weakly and stops watching once it is collected, so the // returned handle keeps the only strong reference alive for as long as the watch is wanted. observer.startWatching() - return Closeable { observer.stopWatching() } + return closeOnce { observer.stopWatching() } + } + + /** + * A handle whose second [Closeable.close] is a no-op. [releaseHandler] counts live watches, so + * a double close would stop the delivery thread out from under the watches still using it. + */ + private fun closeOnce(release: () -> Unit): Closeable { + val closed = AtomicBoolean(false) + return Closeable { if (closed.compareAndSet(false, true)) release() } } /** Starts the delivery thread on the first watch. */ @@ -132,3 +148,23 @@ class PlatformModelSourceWatcher( const val THREAD_NAME = "LocalLlm-ModelWatch" } } + +/** + * The children URI of [uri]'s parent document, which is where a `DocumentsProvider` notifies a + * delete. Drops the last element of the document id — `primary:Download/model.gguf` gives + * `primary:Download`. Top-level so it is testable without a `ContentObserver` and a `HandlerThread`. + * + * @return the parent's children URI, or null when the id names no parent to derive + */ +internal fun parentChildrenUriOf(uri: Uri): Uri? = try { + val documentId = DocumentsContract.getDocumentId(uri) + documentId.substringBeforeLast(DOCUMENT_ID_SEPARATOR, "") + .takeIf { it.isNotEmpty() && it != documentId } + ?.let { DocumentsContract.buildChildDocumentsUri(uri.authority, it) } +} catch (_: Exception) { + // Not a document URI, or an id this provider shapes some other way. + null +} + +/** How every provider that nests documents separates the elements of a document id. */ +private const val DOCUMENT_ID_SEPARATOR = '/' diff --git a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/NativeModelSource.kt b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/NativeModelSource.kt index 5c5219d6..595b2f7f 100644 --- a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/NativeModelSource.kt +++ b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/NativeModelSource.kt @@ -5,6 +5,7 @@ import android.net.Uri import java.io.Closeable import java.io.File import java.io.FileInputStream +import java.io.FileNotFoundException import java.io.InputStream /** @@ -35,6 +36,17 @@ class OpenModelFile( */ val isSeekable: Boolean get() = sizeBytes >= 0 + /** + * Whether [nativePath] can be opened *by name*, which is all the native loader ever does with + * it. That open re-resolves to the real inode against this app's own credentials rather than + * the SAF grant, so removable storage can refuse it where the descriptor was not. ADFA-5253. + */ + fun isReopenable(): Boolean = try { + openStream()?.use { true } ?: false + } catch (_: Exception) { + false + } + /** * Opens an independent read stream over the same bytes the native loader sees — header * inspection must never disturb the loader's own file offset. @@ -56,6 +68,22 @@ class OpenModelFile( } } +/** + * What a reachability probe found. [GONE] and [UNKNOWN] must never be collapsed: a resident + * multi-gigabyte model is the memory pressure that gets a `DocumentsProvider` process killed, and + * reading that silence as a deletion evicts a model that is fine. Only [GONE] evicts. ADFA-5253. + */ +enum class SourceReachability { + /** The source answered, and the model is there. */ + REACHABLE, + + /** The source answered: the model is gone — deleted, unmounted, or the read grant was revoked. */ + GONE, + + /** The source did not answer, which says nothing about the model. */ + UNKNOWN, +} + /** * Opens the user's selected model for the native loader, in place and without copying it. * An interface so the backend's load path can be exercised without a device. @@ -76,9 +104,9 @@ interface NativeModelSource { * deleted inode alive, so the mapped pages outlive the file and the model keeps replying from * a document the user has thrown away. Only a fresh open off the reference can tell. * - * @return true when the model is still there; false for deleted, unmounted, or revoked + * @return what the probe found; [SourceReachability.UNKNOWN] when the source stayed silent */ - fun isReachable(modelReference: String): Boolean + fun reachabilityOf(modelReference: String): SourceReachability } /** @@ -110,20 +138,37 @@ class ContentNativeModelSource( } /** - * One binder round trip for a document, one stat for a path — nothing is read, so this is - * cheap enough to ask before every generation. A failure here is the routine answer "it is - * gone", not an error worth reporting through [onError]. + * One binder round trip for a document, one stat for a path — nothing is read, so this is cheap + * enough to ask before every generation. [SourceReachability.GONE] is only ever what the source + * itself said; a call that failed is [SourceReachability.UNKNOWN], which is not evidence. */ - override fun isReachable(modelReference: String): Boolean = try { - if (modelReference.startsWith(CONTENT_SCHEME)) { - context.contentResolver - .openFileDescriptor(Uri.parse(modelReference), "r") - ?.use { true } ?: false - } else { - File(modelReference).isFile - } - } catch (_: Exception) { - false + override fun reachabilityOf(modelReference: String): SourceReachability = + if (modelReference.startsWith(CONTENT_SCHEME)) documentReachability(modelReference) + else fileReachability(modelReference) + + private fun documentReachability(uriString: String): SourceReachability = try { + context.contentResolver + .openFileDescriptor(Uri.parse(uriString), "r") + ?.use { SourceReachability.REACHABLE } + // No descriptor and no failure is not the provider saying the document is gone. + ?: SourceReachability.UNKNOWN + } catch (_: FileNotFoundException) { + // The routine answer for a deleted or renamed document, and not worth reporting. + SourceReachability.GONE + } catch (_: SecurityException) { + // The persisted grant is gone, which is as final as a deletion from here. + SourceReachability.GONE + } catch (e: Exception) { + // DeadObjectException and friends: the provider died, which is not the routine case. + onError("could not reach the selected model $uriString", e) + SourceReachability.UNKNOWN + } + + private fun fileReachability(path: String): SourceReachability = try { + if (File(path).isFile) SourceReachability.REACHABLE else SourceReachability.GONE + } catch (e: Exception) { + onError("could not stat the model file $path", e) + SourceReachability.UNKNOWN } private fun openFile(path: String): OpenModelFile? = try { diff --git a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsViewModel.kt b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsViewModel.kt index 360221c7..6b1a9f63 100644 --- a/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsViewModel.kt +++ b/ai-agent-local/src/main/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsViewModel.kt @@ -202,8 +202,10 @@ class LocalLlmSettingsViewModel( if (current.model is ModelLoadingState.Loading) return@launch if (readable) { - // Only ever clears a stale "unavailable": a live Error is about this same model. - if (current.model is ModelLoadingState.Unavailable) { + // An Error describes a refused *pick*, so a readable model clears that too. + if (current.model is ModelLoadingState.Unavailable || + current.model is ModelLoadingState.Error + ) { publishModelState(modelStateFor(savedPath)) } } else { @@ -349,6 +351,8 @@ class LocalLlmSettingsViewModel( viewModelScope.launch(ioDispatcher) { publishModelState(ModelLoadingState.Loading) + // Whether this pick became the configured model; anything else gives its grant back. + var stored = false try { // Taken before the first read, so every step below works off the durable grant. if (!modelFiles.persistAccess(context, uriString)) { @@ -364,7 +368,6 @@ class LocalLlmSettingsViewModel( // Checked before the GGUF sniff so a model that is simply gone — the "Load from // saved" case after the file was deleted — is not reported as a corrupt one. if (!modelFiles.isReadable(context, uriString)) { - releaseUnkeptGrant(context, uriString) publishAbandonedSelection( uriString, ModelLoadingState.Unavailable(fileName), @@ -375,7 +378,6 @@ class LocalLlmSettingsViewModel( // Rejected up front, so no bad path is persisted or shown as "Loaded". if (!GgufFileInspector.looksLikeGguf(context.contentResolver, uriString)) { - releaseUnkeptGrant(context, uriString) publishAbandonedSelection( uriString, ModelLoadingState.Error( @@ -388,7 +390,6 @@ class LocalLlmSettingsViewModel( if (!confirmMemoryHeadroom(uriString, fileInfo, context)) { logger?.info("$TAG: model declined at the memory warning: $fileName") - releaseUnkeptGrant(context, uriString) restoreStateBefore(stateBefore) return@launch } @@ -402,6 +403,7 @@ class LocalLlmSettingsViewModel( // Persist the name before the path so the savedModelPath observer can read it. saveLocalModelName(fileName) saveLocalModelPath(uriString) + stored = true // Nothing is loaded here; the engine reads this path when it needs the model. publishModelState(ModelLoadingState.Loaded(fileName)) @@ -418,6 +420,8 @@ class LocalLlmSettingsViewModel( ), stateBefore.engine, ) + } finally { + if (!stored) releaseUnkeptGrant(context, uriString) } } } @@ -485,10 +489,8 @@ class LocalLlmSettingsViewModel( /** * Gives back the grant taken for a selection that was not kept, so an abandoned pick does not - * hold a slot in the capped grant table. - * - * Never touches the configured model: re-checking it and abandoning that check must leave the - * model that is actually in use readable. + * hold a slot in the capped grant table. Called only from [loadModelFromUri]'s `finally`, so no + * abandon path can forget it, and never for the configured model, which stays readable. */ private fun releaseUnkeptGrant(context: Context, uriString: String) { if (uriString != getLocalModelPath()) { @@ -497,13 +499,13 @@ class LocalLlmSettingsViewModel( } /** - * Puts the screen back as it was before a selection the user declined outright. Restores both - * lines rather than re-deriving them: a configured model that was already unreachable must - * stay reported that way. + * Puts the screen back as it was before a selection the user declined outright. Restores the + * two status lines rather than re-deriving them, and only those two: a decline says nothing + * about the configured path or name, so a whole snapshot would revert a concurrent write. * * @param stateBefore the state captured before the selection started */ private fun restoreStateBefore(stateBefore: LocalLlmSettingsState) { - update { stateBefore } + update { it.copy(model = stateBefore.model, engine = stateBefore.engine) } } } diff --git a/ai-agent-local/src/main/res/values/strings.xml b/ai-agent-local/src/main/res/values/strings.xml index 6098af12..56542fd6 100644 --- a/ai-agent-local/src/main/res/values/strings.xml +++ b/ai-agent-local/src/main/res/values/strings.xml @@ -5,6 +5,7 @@ The model file could not be found. Re-select the .gguf model in AI Settings. The selected model can no longer be reached. It may have been moved, deleted, or saved to storage that isn\'t connected right now, or the IDE\'s permission to read it was withdrawn. Select the .gguf model again in AI Settings. This model is streamed from its storage location rather than stored on this device, so it can\'t be read in place. Download the .gguf to the device — for example to Downloads — and select it from there. + This model can\'t be read from where it is stored — the storage volume doesn\'t allow the IDE to open it directly. Copy the .gguf to the device\'s internal storage — for example to Downloads — and select it from there. The model file is empty — the download may have been interrupted. Re-download the .gguf model and select it again. This file isn\'t a valid .gguf model (it may be corrupt or only partially downloaded). Re-download the model and select it again. Loading this model needs at least %1$s of free memory, but only %2$s is available on this device. Close other apps and try again, or pick a smaller or more heavily quantized model (for example a Q4_K_M build of a 1–3B model). diff --git a/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/LocalLlmBackendTest.kt b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/LocalLlmBackendTest.kt index 45941bb2..51dc6bb8 100644 --- a/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/LocalLlmBackendTest.kt +++ b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/backend/LocalLlmBackendTest.kt @@ -9,6 +9,7 @@ import com.itsaky.androidide.plugins.aiagentlocal.model.ModelLoadDiagnostics.Dia import com.itsaky.androidide.plugins.aiagentlocal.model.ModelSourceWatcher import com.itsaky.androidide.plugins.aiagentlocal.model.NativeModelSource import com.itsaky.androidide.plugins.aiagentlocal.model.OpenModelFile +import com.itsaky.androidide.plugins.aiagentlocal.model.SourceReachability import com.itsaky.androidide.plugins.services.LlmInferenceService.* import io.mockk.every import io.mockk.mockk @@ -45,6 +46,9 @@ class LocalLlmBackendTest { /** Flipped to simulate the user deleting the file out from under a resident model. */ var reachable = true + /** What the probe answers when [reachable] is false: a deletion, or a provider that died. */ + var whenUnreachable = SourceReachability.GONE + /** Reachability probes served, so a burst of watch notifications can be counted. */ @Volatile var probeCount = 0 @@ -53,9 +57,10 @@ class LocalLlmBackendTest { return handles[modelReference].takeIf { reachable } } - override fun isReachable(modelReference: String): Boolean { + override fun reachabilityOf(modelReference: String): SourceReachability { probeCount++ - return reachable && handles.containsKey(modelReference) + return if (reachable && handles.containsKey(modelReference)) SourceReachability.REACHABLE + else whenUnreachable } } @@ -292,6 +297,88 @@ class LocalLlmBackendTest { assertTrue("the descriptor must be released or the inode stays alive", descriptor.closed) } + @Test + fun givenAResidentModel_whenTheProviderStopsAnswering_thenItKeepsServing() { + // A dead provider process says nothing about the document; evicting costs a GB reload. + val descriptor = RecordingDescriptor() + val source = FakeModelSource(mapOf(CONTENT_URI to handleFor(chatModel(), descriptor))) + val engine = FakeEngine() + val backend = backendWith(source, engine) + + runBlocking { backend.ensureModelLoaded(CONTENT_URI) } + source.reachable = false + source.whenUnreachable = SourceReachability.UNKNOWN + + runBlocking { backend.ensureModelLoaded(CONTENT_URI) } + + assertEquals("a silent provider must not cost a reload", 1, engine.loadCount) + assertEquals("a silent provider must not evict the model", 0, engine.unloadCount) + assertFalse("the descriptor must stay open", descriptor.closed) + } + + @Test + fun givenAResidentModel_whenItsWatchFiresAndTheProviderIsSilent_thenItStaysLoaded() { + // The same distinction on the watch path, where a burst of notifications arrives. + val source = FakeModelSource(mapOf(CONTENT_URI to handleFor(chatModel()))) + val engine = FakeEngine() + val watcher = FakeWatcher() + val backend = backendWith(source, engine, watcher) + + runBlocking { backend.ensureModelLoaded(CONTENT_URI) } + source.reachable = false + source.whenUnreachable = SourceReachability.UNKNOWN + watcher.onGone!!.invoke() + + Thread.sleep(200) + assertEquals("an unanswered probe must not unload the model", 0, engine.unloadCount) + } + + @Test + fun givenADocumentThatCannotBeReopenedByPath_whenLoading_thenRefusedWithItsOwnAdvice() { + // Refused before native code, or it lands on "pick the model again" for a file that is there. + val descriptor = RecordingDescriptor() + val unreadable = OpenModelFile("/proc/self/fd/99", 4_096L, descriptor) + val source = FakeModelSource(mapOf(CONTENT_URI to unreadable)) + val engine = FakeEngine() + + val error = assertThrows(ModelLoadException::class.java) { + runBlocking { backendWith(source, engine).ensureModelLoaded(CONTENT_URI) } + } + + assertEquals(Diagnosis.SourceNotReopenable, error.diagnosis) + assertEquals("nothing may reach the engine", 0, engine.loadCount) + assertTrue("the refused descriptor must not leak", descriptor.closed) + } + + @Test + fun givenAConfiguredModelThatFailsToLoad_whenAvailabilityIsAskedRepeatedly_thenItIsTriedOnce() { + // A warm-up re-armed on failure launches one doomed load per isAvailable() call. + configureModelPath(CONTENT_URI) + // Not in the fake's handles, so the warm-up fails the way an unreachable model does. + val source = FakeModelSource(emptyMap()) + val backend = backendWith(source, FakeEngine()) + + repeat(5) { backend.isAvailable() } + + Thread.sleep(300) + assertEquals("five asks must cost one warm-up attempt", 1, source.openCount) + } + + @Test + fun givenAFailedWarmUp_whenAnotherModelIsSelected_thenTheWarmUpIsTriedAgain() { + // Keyed on the reference, not disabled outright: a new selection has to be warmed. + configureModelPath(CONTENT_URI) + val source = FakeModelSource(emptyMap()) + val backend = backendWith(source, FakeEngine()) + backend.isAvailable() + + configureModelPath(OTHER_CONTENT_URI) + backend.isAvailable() + + Thread.sleep(300) + assertEquals("a different selection must re-arm the warm-up", 2, source.openCount) + } + @Test fun givenAResidentModelStillOnDisk_whenGenerating_thenItIsServedWithoutReloading() { // The check must not cost a reload: a document's procfs path differs on every open, and @@ -355,10 +442,18 @@ class LocalLlmBackendTest { /** A minimal GGUF that passes the ADFA-4388 embedding guard, so loads reach the engine. */ private fun chatModel(): File = GgufTestFiles.withArchitecture("qwen2") + /** Points the backend's preferences at [modelRef], the way a saved selection does. */ + private fun configureModelPath(modelRef: String) { + val prefs = mockk(relaxed = true) + every { prefs.getString("local_llm_model_path", any()) } returns modelRef + every { pluginContext.getPluginSharedPreferences(any()) } returns prefs + } + private fun handleFor(file: File, descriptor: Closeable? = null) = OpenModelFile(file.absolutePath, file.length(), descriptor) private companion object { const val CONTENT_URI = "content://com.android.externalstorage.documents/document/model.gguf" + const val OTHER_CONTENT_URI = "content://com.android.externalstorage.documents/document/other.gguf" } } diff --git a/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ContentNativeModelSourceTest.kt b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ContentNativeModelSourceTest.kt index d1538421..95e71ffc 100644 --- a/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ContentNativeModelSourceTest.kt +++ b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ContentNativeModelSourceTest.kt @@ -3,6 +3,7 @@ package com.itsaky.androidide.plugins.aiagentlocal.model import android.content.ContentResolver import android.content.Context import android.net.Uri +import android.os.DeadObjectException import android.os.ParcelFileDescriptor import io.mockk.every import io.mockk.mockk @@ -48,45 +49,63 @@ class ContentNativeModelSourceTest { fun givenAPathThatStillExists_whenProbed_thenItIsReachable() { val model = temporaryFolder.newFile("model.gguf") - assertTrue(source.isReachable(model.absolutePath)) + assertEquals(SourceReachability.REACHABLE, source.reachabilityOf(model.absolutePath)) } @Test - fun givenADeletedPath_whenProbed_thenItIsUnreachable() { + fun givenADeletedPath_whenProbed_thenItIsGone() { val model = temporaryFolder.newFile("model.gguf") assertTrue(model.delete()) - assertFalse(source.isReachable(model.absolutePath)) + assertEquals(SourceReachability.GONE, source.reachabilityOf(model.absolutePath)) } @Test - fun givenADirectory_whenProbed_thenItIsUnreachable() { + fun givenADirectory_whenProbed_thenItIsGone() { // A path that resolves but holds no model must not read as a usable one. val directory = temporaryFolder.newFolder("models") - assertFalse(source.isReachable(directory.absolutePath)) + assertEquals(SourceReachability.GONE, source.reachabilityOf(directory.absolutePath)) } @Test fun givenADocumentTheProviderStillServes_whenProbed_thenItIsReachable() { every { resolver.openFileDescriptor(any(), "r") } returns mockk(relaxed = true) - assertTrue(source.isReachable(CONTENT_URI)) + assertEquals(SourceReachability.REACHABLE, source.reachabilityOf(CONTENT_URI)) } @Test - fun givenADeletedDocument_whenProbed_thenItIsUnreachable() { + fun givenADeletedDocument_whenProbed_thenItIsGone() { // What a deleted document actually does: the provider throws rather than returning null. every { resolver.openFileDescriptor(any(), "r") } throws java.io.FileNotFoundException() - assertFalse(source.isReachable(CONTENT_URI)) + assertEquals(SourceReachability.GONE, source.reachabilityOf(CONTENT_URI)) } @Test - fun givenAProviderThatAnswersWithNothing_whenProbed_thenItIsUnreachable() { + fun givenARevokedGrant_whenProbed_thenItIsGone() { + // As final as a deletion from here: only a fresh pick can bring the document back. + every { resolver.openFileDescriptor(any(), "r") } throws SecurityException("no grant") + + assertEquals(SourceReachability.GONE, source.reachabilityOf(CONTENT_URI)) + } + + @Test + fun givenAProviderThatDied_whenProbed_thenTheAnswerIsUnknownRatherThanGone() { + // A resident multi-GB model is the pressure that kills a provider; that is not a delete. + // Instantiated through mockk: the unit-test android.jar stubs its constructor out. + every { resolver.openFileDescriptor(any(), "r") } throws mockk(relaxed = true) + + assertEquals(SourceReachability.UNKNOWN, source.reachabilityOf(CONTENT_URI)) + } + + @Test + fun givenAProviderThatAnswersWithNothing_whenProbed_thenTheAnswerIsUnknown() { + // No descriptor and no failure is not the provider saying the document is gone. every { resolver.openFileDescriptor(any(), "r") } returns null - assertFalse(source.isReachable(CONTENT_URI)) + assertEquals(SourceReachability.UNKNOWN, source.reachabilityOf(CONTENT_URI)) } @Test @@ -95,7 +114,7 @@ class ContentNativeModelSourceTest { val descriptor = mockk(relaxed = true) every { resolver.openFileDescriptor(any(), "r") } returns descriptor - source.isReachable(CONTENT_URI) + source.reachabilityOf(CONTENT_URI) io.mockk.verify { descriptor.close() } } @@ -143,6 +162,21 @@ class ContentNativeModelSourceTest { assertTrue(opened.isSeekable) } + @Test + fun givenAPathTheLoaderCanOpenByName_whenAsked_thenItIsReopenable() { + // The loader opens nativePath by name, so the handle answers for that open. + val model = temporaryFolder.newFile("model.gguf").apply { writeBytes(ByteArray(64)) } + + assertTrue(source.open(model.absolutePath)!!.isReopenable()) + } + + @Test + fun givenAPathNothingCanOpen_whenAsked_thenItIsNotReopenable() { + val opened = OpenModelFile("/proc/self/fd/99999", 64L, null) + + assertFalse(opened.isReopenable()) + } + @Test fun givenADeletedPath_whenOpened_thenNoHandleIsReturned() { val model = temporaryFolder.newFile("model.gguf") diff --git a/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelSourceWatcherTest.kt b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelSourceWatcherTest.kt new file mode 100644 index 00000000..5286794d --- /dev/null +++ b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/model/ModelSourceWatcherTest.kt @@ -0,0 +1,78 @@ +package com.itsaky.androidide.plugins.aiagentlocal.model + +import android.net.Uri +import android.provider.DocumentsContract +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkStatic +import io.mockk.unmockkStatic +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Before +import org.junit.Test + +/** + * A `DocumentsProvider` notifies a delete against the parent's children URI, which is not a + * path-prefix descendant of the document URI — so a document-only watch never sees the deletion it + * exists to catch, and the gigabytes stay mapped until the next message. See ADFA-5253. + */ +class ModelSourceWatcherTest { + + private lateinit var parentUri: Uri + + @Before + fun setup() { + parentUri = mockk(relaxed = true) + mockkStatic(DocumentsContract::class) + every { DocumentsContract.buildChildDocumentsUri(any(), any()) } returns parentUri + } + + @After + fun tearDown() { + unmockkStatic(DocumentsContract::class) + } + + @Test + fun givenANestedDocument_whenDerivingTheWatchTarget_thenItIsTheParentsChildrenUri() { + val uri = documentUri("primary:Download/model.gguf") + + assertEquals(parentUri, parentChildrenUriOf(uri)) + io.mockk.verify { DocumentsContract.buildChildDocumentsUri(AUTHORITY, "primary:Download") } + } + + @Test + fun givenADeeplyNestedDocument_whenDerivingTheWatchTarget_thenOnlyTheLastElementIsDropped() { + val uri = documentUri("primary:Download/models/gguf/model.gguf") + + assertEquals(parentUri, parentChildrenUriOf(uri)) + io.mockk.verify { + DocumentsContract.buildChildDocumentsUri(AUTHORITY, "primary:Download/models/gguf") + } + } + + @Test + fun givenADocumentAtTheRootOfItsVolume_whenDerivingTheWatchTarget_thenThereIsNone() { + // No parent element to drop; the direct watch is all there is. + assertNull(parentChildrenUriOf(documentUri("primary:model.gguf"))) + } + + @Test + fun givenSomethingThatIsNotADocumentUri_whenDerivingTheWatchTarget_thenThereIsNone() { + val uri = mockk(relaxed = true) + every { DocumentsContract.getDocumentId(uri) } throws IllegalArgumentException("not a document") + + assertNull(parentChildrenUriOf(uri)) + } + + private fun documentUri(documentId: String): Uri { + val uri = mockk(relaxed = true) + every { uri.authority } returns AUTHORITY + every { DocumentsContract.getDocumentId(uri) } returns documentId + return uri + } + + private companion object { + const val AUTHORITY = "com.android.externalstorage.documents" + } +} diff --git a/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsViewModelTest.kt b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsViewModelTest.kt index 99fca1ff..2511df86 100644 --- a/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsViewModelTest.kt +++ b/ai-agent-local/src/test/kotlin/com/itsaky/androidide/plugins/aiagentlocal/settings/LocalLlmSettingsViewModelTest.kt @@ -41,8 +41,13 @@ class LocalLlmSettingsViewModelTest { /** References the provider will not serve, standing in for a deleted document. */ val unreadable = mutableSetOf() - override fun info(context: Context, uriString: String) = - ModelFileInfo(fallbackDisplayName(uriString), 1_024L) + /** Makes the lookup blow up, standing in for a provider that fails mid-selection. */ + var failInfo = false + + override fun info(context: Context, uriString: String): ModelFileInfo { + if (failInfo) throw IllegalStateException("provider failed") + return ModelFileInfo(fallbackDisplayName(uriString), 1_024L) + } override fun openStream(context: Context, uriString: String): InputStream? = null @@ -103,12 +108,13 @@ class LocalLlmSettingsViewModelTest { * Unconfined, so every launch runs inline: nothing here suspends on a real dispatcher, and the * memory pre-flight fails open on the fake's unreadable header. */ - private fun viewModel() = LocalLlmSettingsViewModel( - getContext = { pluginContext }, - ioDispatcher = Dispatchers.Unconfined, - deviceMemory = DeviceMemory { null }, - modelFiles = modelFiles, - ) + private fun viewModel(deviceMemory: DeviceMemory = DeviceMemory { null }) = + LocalLlmSettingsViewModel( + getContext = { pluginContext }, + ioDispatcher = Dispatchers.Unconfined, + deviceMemory = deviceMemory, + modelFiles = modelFiles, + ) @Test fun givenASelection_whenItIsKept_thenItsGrantIsPersistedAndStored() { @@ -233,6 +239,71 @@ class LocalLlmSettingsViewModelTest { assertEquals(EngineState.Initialized, viewModel.state.value?.engine) } + @Test + fun givenARejectedPicksError_whenTheScreenReturnsAndTheModelReadsBack_thenItClears() { + // The error described the pick; left standing it shows on every return to the screen. + val viewModel = viewModel() + viewModel.loadModelFromUri(MODEL_A) + every { resolver.openInputStream(any()) } answers { ByteArrayInputStream("NOPE".toByteArray()) } + viewModel.loadModelFromUri(MODEL_B) + assertTrue(viewModel.state.value?.model is ModelLoadingState.Error) + + viewModel.refreshSavedModelAvailability() + + assertEquals(ModelLoadingState.Loaded("a.gguf"), viewModel.state.value?.model) + assertEquals(EngineState.Initialized, viewModel.state.value?.engine) + } + + @Test + fun givenAPickAbandonedBeforeItWasStored_thenItsGrantIsGivenBackByTheFinally() { + // Grants are capped, and only the finally covers every way out of the selection. + val viewModel = viewModel() + + every { resolver.openInputStream(any()) } answers { ByteArrayInputStream("NOPE".toByteArray()) } + viewModel.loadModelFromUri(MODEL_B) + + assertEquals(listOf(MODEL_B), modelFiles.persisted) + assertEquals(listOf(MODEL_B), modelFiles.released) + assertEquals(null, viewModel.getLocalModelPath()) + } + + @Test + fun givenASelectionThatThrows_thenItsGrantIsStillGivenBack() { + // Leaves through code no abandon path runs, as a cancellation at the dialog would. + modelFiles.failInfo = true + val viewModel = viewModel() + + viewModel.loadModelFromUri(MODEL_B) + + assertEquals(listOf(MODEL_B), modelFiles.persisted) + assertEquals(listOf(MODEL_B), modelFiles.released) + assertTrue(viewModel.state.value?.model is ModelLoadingState.Error) + } + + @Test + fun givenAModelDeclinedAtTheMemoryWarning_thenItsGrantIsGivenBackAndNothingIsStored() { + val viewModel = viewModel(deviceMemory = DeviceMemory { 1L }) + viewModel.loadModelFromUri(MODEL_B) + assertTrue("the pre-flight must be waiting on an answer", viewModel.hasPendingMemoryWarning) + + viewModel.onMemoryWarningDecision(false) + + assertEquals(listOf(MODEL_B), modelFiles.released) + assertEquals(null, viewModel.getLocalModelPath()) + } + + @Test + fun givenADeclineAtTheMemoryWarning_whenSomethingWasStoredMeanwhile_thenItIsNotReverted() { + // The decline owns the two status lines and nothing else in the state. + val viewModel = viewModel(deviceMemory = DeviceMemory { 1L }) + viewModel.loadModelFromUri(MODEL_B) + viewModel.saveLocalModelPath(MODEL_A) + + viewModel.onMemoryWarningDecision(false) + + assertEquals(MODEL_A, viewModel.state.value?.savedModelPath) + } + private companion object { const val MODEL_A = "content://com.android.externalstorage.documents/document/a.gguf" const val MODEL_B = "content://com.android.externalstorage.documents/document/b.gguf"