-
Notifications
You must be signed in to change notification settings - Fork 38
feat: unify apple and llama models on API (prepare) and storage methods #175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note: Tests will be failing on purpose, the example app is broken and will soon be fixed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR unifies the prepare() API pattern across Apple and Llama providers and simplifies model management by removing redundant model-level methods in favor of centralized storage functions. The changes improve API consistency and make the provider APIs more similar to each other.
Changes:
- Added
prepare()method to all model classes with auto-prepare fallback and warning pattern for better performance optimization - Changed Llama models to accept file paths instead of model IDs, with storage functions (
downloadModel,getModelPath,isModelDownloaded) exported for model management - Moved Apple provider language configuration from
providerOptionsto constructor options for better type safety and consistency
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/llama/src/storage.ts | Made storage path constant and removed setter/getter functions |
| packages/llama/src/index.ts | Removed LlamaEngine and LlamaProviderOptions exports, added storage function exports |
| packages/llama/src/ai-sdk.ts | Removed LlamaEngine, changed constructors to accept paths, added prepare() with auto-prepare pattern, removed download/remove methods from models |
| packages/apple-llm/src/ai-sdk.ts | Added option interfaces, moved language to constructor, added prepare() method with auto-prepare pattern |
| website/src/docs/llama/model-management.md | Updated to reflect new storage API pattern and removed LlamaEngine references |
| website/src/docs/llama/getting-started.md | Updated examples to show new downloadModel/path pattern |
| website/src/docs/llama/generating.md | Updated all examples to use new API |
| website/src/docs/llama/embeddings.md | Updated examples and removed model deletion section |
| website/src/docs/apple/transcription.md | Updated to show language in constructor and new prepare() API |
| website/src/docs/apple/embeddings.md | Updated to show language in constructor and new prepare() API |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Some of those comments are fixed in subsequent PRs, so marking them as unrelated. |
Summary
Unifies the
prepare()API across Apple and Llama providers, simplifies model management, and removes redundant APIs. This is part of the work I'm doing in the example application as well, wanted to bring both providers as closely together as possible.Apple LLM:
prepare()method to all model classes with auto-prepare + warning patternproviderOptionsto constructor optionsLlama:
downloadModel()orgetModelPath()to get the pathdownload(),isDownloaded(),remove()LlamaEngine- use storage functions directlyprepare()not called ahead of timeStorage API:
${DocumentDir}/llama-models/)setStoragePath(),getStoragePath(), andstoragePathparametergetModelPath()for getting path without downloadingThis will allow us to share Storage API with other runtimes and use it for other purposes in the future as well.
New Usage Pattern
Breaking changes
language setting must be set for Apple provider in constructor, instead of per-call (which is more correct)
removed string argument for apple provider since only one value was accepted (I doubt this API had any real coverage, so decided to drop it to avoid introducing unnecessary legacy code at this point)