ADFA-5253 | Persist model URI instead of copying multi-GB models

[jatezzz]

Description

Implemented direct-access behavior for LLM models selected via the Storage Access Framework (SAF). The application now retains persistent read permissions for the chosen URI and queries the .gguf file in-place. This prevents copying multi-gigabyte models into the app's internal sandbox, drastically saving device storage and speeding up the setup process. Additionally, a cleanup routine was added to delete legacy model copies, and the plugin's HTML documentation was updated to reflect this direct-access behavior.

Details

• Updated LocalLlmBackend to hand the native loader a file descriptor path (/proc/self/fd/N) for direct reading.
• Added ModelSourceWatcher and NativeModelSource interfaces to safely monitor, open, and evict models if the underlying source file is deleted or unmounted.
• Adjusted LocalLlmSettingsViewModel and LocalLlmSettingsFragment to properly display and handle "Unavailable" model states when a URI becomes unreachable.
• Refactored GgufModelInspector and ModelLoadDiagnostics to operate on InputStream factories rather than static file paths.

document_4956368585724266828.mp4

Ticket

ADFA-5253

Observation

To facilitate mmap operations in the native code without copying the file, ContentNativeModelSource resolves the content:// URI to a file descriptor and passes the /proc/self/fd/ path to the llama.cpp backend. A legacy cache cleanup (deleteLegacyModelCache) is executed in the background upon initialization to reclaim space from older app versions.

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

_{Tip: disable this comment in your organization's Code Review settings.}

[itsaky-adfa]

Reviewed at 426e5bd. One confirmed defect on a path a user will hit, so requesting changes; the rest are minor. The refactor itself is careful, and the seam design (NativeModelSource / ModelResidencyEngine / ModelSourceWatcher) is what makes the residency rules testable at all - that part is a clear improvement.

Verified locally: ./gradlew testDebugUnitTest in ai-agent-local - 137 tests, 0 failures. git fetch origin first, so the diff is against current origin/main (68abdd6).

Blocking: the isAvailable() memo (LocalLlmBackend.kt:243) makes the first message after a restored model file fail with Backend 'local' is not available. Traced through ai-core's LlmInferenceServiceImpl.generateStreaming, which returns that string verbatim on a false.

Two things only a device can settle, both called out inline:

• A streaming DocumentsProvider hands back a pipe, which the three openStream() calls consume before llama.cpp reads it. Worth picking a .gguf from Drive to see what happens.
• Re-opening /proc/self/fd/N is a fresh path-based open(), checked against path permissions rather than the SAF grant that produced the fd. The attached video covers the happy case; an SD-card and a FUSE-volume load would close it. du -sh .../files/llm-models proves no copy was made, not that the load works everywhere.

Checked and cleared: descriptor lifecycle in ensureModelLoaded (the adopted/finally pairing and the unload-before-close ordering are right, and both header readers .use their streams); double-close() on modelWatch (unreachable - stopWatching nulls it); the close()-vs-onModelSourceGone race on cleanupScope; and no remaining callers anywhere in the repo of the removed or privatised API (engineState, savedModelPath, modelLoadingState, getSavedModelName, fallbackDisplayName, isGguf(String), EngineState.Uninitialized).

Two candidate findings I dropped after checking:

• "The model is permanently unavailable once the memo is set" - false. LlmInferenceServiceImpl.getAvailableBackends() does not filter on isAvailable(), so local stays selectable and the warm-up clears the memo. Only the one-shot stale false survives, which is the blocking finding above.
• "diagnose() losing FileMissing dropped its test" - false. givenMissingFile_whenDiagnosed_thenFileMissing was ported to givenAFilesystemPath_whenDiagnoseUnopenable_thenFileMissing, and diagnose now only runs after a successful open, so the narrowing is deliberate.

No prior review comments on this PR, so there was nothing to re-check from an earlier round. This repo has no written approve/request-changes rule, so the reviewer default applied: any confirmed IMPORTANT blocks. Its CLAUDE.md does require device-level verification over a green build, which is what the two items above are asking for.

Docs are in step with the code - ai-agent-local.html, assets/docs/index.html and the plugin.permissions comment all describe read-in-place rather than the old copy.

[itsaky-adfa]

Round 2, reviewed at e30d9aad. Findings only - the verdict follows separately, once the two unconfirmed items below are settled.

Prior round: all 7 findings fixed

Verified against the code at head, not against the replies. (The reply saying "fixed locally, not pushed yet" was stale by three minutes - 54c95bd landed at 18:25 UTC, the reply at 18:28.)

• isAvailable() memo - unreachableModelRef is gone repo-wide; reachability is answered only on the generation path. It also removes a binder probe from the caller's thread.
• Non-seekable descriptor - refused at LocalLlmBackend.kt:300, before the first openStream(), which is the ordering the finding turned on. Plus EXTRA_LOCAL_ONLY on the picker.
• ModelLoadDiagnostics KDoc - back above refuseBeforeLoad.
• Fragment "(unavailable)" marker - keyed off the engine status, and publishAbandonedSelection covers the stranded Initializing you turned up while testing it.
• Missing tests - openDocument now pins the procfs path and statSize; the 9 LocalLlmSettingsViewModelTest assertions match the grant-lifecycle claims exactly.
• Watch coalescing - sourceCheckInFlight CAS, cleared in a finally outside the lock.
• Watcher KDoc - reworded to name both delivery threads.

Verification

../gradlew testDebugUnitTest: 152 tests, 0 failures. I also stubbed the isSeekable guard to false and confirmed givenAStreamingDocument_whenLoading_thenRefusedAsNotSeekableWithoutReadingIt fails, and only that test - so the new guard is genuinely pinned by its test.

Two findings are NOT confirmed

Both are marked as such inline. Please treat them as questions, not verdicts:

• NativeModelSource.kt:106 - whether /proc/self/fd/N re-opens successfully for a document on removable storage. The code path is confirmed (llama.cpp opens the path by name, never the descriptor); the permission outcome needs a device. Your demo video shows at least one storage location working, so this is scoped to SD/USB.
• ModelSourceWatcher.kt:76 - the URI arithmetic is confirmed against the DocumentsContract contract, but I have not watched a provider fail to notify on a device.

Every other finding is confirmed by reading the code at head. For the eviction-on-transient-failure one, the defect is confirmed and only the trigger's frequency is not.

Checked and dropped

Saying so explicitly, so they are not silently missing: main-thread binder I/O in ensureModelLoaded (every call site is inside a Dispatchers.IO scope); the descriptor being closed while llama.cpp still has pages mapped (engine.unload() drains the native run loop before releaseCurrentModel()); the watch Closeable being double-closed today (stopWatching() nulls the field); and core-testing bypassing a version catalog (this repo has none - every dependency is a hardcoded coordinate).

This repo has no written approve/request-changes rule, so the default one applied.

The seam design continues to be the strength here - NativeModelSource / ModelResidencyEngine / ModelSourceWatcher are what make any of this testable off a device, and the docs were kept in step with the behaviour change.