fix: use platform-appropriate model cache defaults#248
Merged
Conversation
Contributor
|
Chat app preview removed for |
There was a problem hiding this comment.
Pull request overview
Adjusts DefaultModelDownloadManager defaults to be platform-appropriate (shared per-user cache on desktop/server, app-private cache/temp fallback on mobile), adds optional platform-specific mobile directory parameters to auto(...), and updates docs + tests to match the new behavior.
Changes:
- Updated native IO
DefaultModelDownloadManager()to prefer the platform’s implicit cache root (shared cache where supported; temp/cache fallback otherwise). - Extended
DefaultModelDownloadManager.auto(...)withandroidAppPrivateCacheDirectoryandiosAppPrivateCacheDirectory, plus new mobile fallback behavior when no app-private directory is provided. - Refreshed README / website docs / migration notes and updated unit tests to cover the new defaults and namespace handling.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
lib/src/platform/io/model_download_manager_io.dart |
Implements new platform-appropriate default cache directory selection and expands auto(...) with platform-specific mobile directory parameters + fallback behavior. |
lib/src/core/models/download/model_download_manager_stub.dart |
Keeps non-IO stub API surface in sync by adding the new optional named parameters. |
test/unit/platform/io/model_download_manager_io_test.dart |
Adds/updates tests for constructor defaults, mobile fallbacks, namespace validation, and platform-specific mobile directory selection. |
test/unit/core/models/download/model_download_manager_stub_test.dart |
Verifies new auto(...) parameters compile on non-IO platforms and remain unsupported at runtime as expected. |
README.md |
Documents updated cache defaults and recommended usage patterns for cross-platform apps (including new mobile directory parameters). |
website/docs/guides/model-lifecycle.md |
Updates model cache guidance to reflect new auto(...) mobile behavior and constructor defaults. |
MIGRATION.md |
Adds migration guidance describing the runtime default changes and how to preserve previous temp-cache behavior. |
CHANGELOG.md |
Updates release notes text to describe the new default/cache behavior (currently placed under 0.8.8). |
website/docs/changelog/recent-releases.md |
Mirrors release note updates for the website’s “Recent Releases” page (currently placed under 0.8.8). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0161f98 to
d0dcdbe
Compare
d0dcdbe to
4922f1c
Compare
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DefaultModelDownloadManager()default match platform-appropriate cache behavior: desktop/server shared cache, mobile app-private temp/cache fallback, with temp fallback preserved for unusual desktop/server embedders that cannot expose a cache environment.DefaultModelDownloadManager.auto(...)with optionalandroidAppPrivateCacheDirectoryandiosAppPrivateCacheDirectoryso apps can provide platform-specific mobile paths without constructor-levelPlatform.isAndroid/Platform.isIOSbranching.MIGRATION.md.Platform defaults
$XDG_CACHE_HOME/llamadart/models, or$HOME/.cache/llamadart/modelswhenXDG_CACHE_HOMEis unset.$HOME/Library/Caches/llamadart/models.%LOCALAPPDATA%\llamadart\models, then%APPDATA%\llamadart\models, then%USERPROFILE%\AppData\Local\llamadart\models.androidAppPrivateCacheDirectory, thenappPrivateCacheDirectory, thenDirectory.systemTemp/llamadart/models.iosAppPrivateCacheDirectory, thenappPrivateCacheDirectory, thenDirectory.systemTemp/llamadart/models.Compatibility / migration
androidAppPrivateCacheDirectoryandiosAppPrivateCacheDirectoryare optional named parameters.DefaultModelDownloadManager()now prefers the shared cache instead of always using process temp.DefaultModelDownloadManager.auto()without an explicit app-private path now falls back to best-effort temp/cache instead of throwing.MIGRATION.mddocuments how to preserve the old temp-cache behavior and how to update tests that expected the previous mobile throw.Existing issue check
DefaultModelDownloadManager cache path shared model cache appPrivateCacheDirectory; found prior related PR Add cross-platform model cache strategies #242, but no dedicated open issue to close.Test Plan
Latest reviewed head:
4922f1c4febed3d13e230727a14d7de18595386c.dart format --output=none --set-exit-if-changed ...on changed Dart filesgit diff --checkdart analyze— exits 0 with three pre-existing unrelated info lintsdart test test/unit/platform/io/model_download_manager_io_test.dart— 61 tests passeddart test test/unit/core/models/download/model_download_manager_stub_test.dart— 4 tests passeddart test test/unit/core/models— 278 tests passed earlier in this PR loopdart testinexample/basic_appflutter analyze && flutter testinexample/chat_app/private/tmp/llamadart_api_compat_review:dart pub get && dart analyze && dart run bin/compat.dartnpm run buildinwebsite/source.path), no credential-bearing URL exposureReview notes
namespace; website docs no longer describe mobileauto(...)as throwing when it falls back.~/Library/Application Support/llamadart/modelsis a durable physical store, not automatically a package-managed cache root unless metadata entries are seeded. This is documented as a migration/local-store concern, not a blocker for the package default behavior.