feat(go/ollama): implement DynamicPlugin for dynamic model discovery#4529
feat(go/ollama): implement DynamicPlugin for dynamic model discovery#4529Zereker wants to merge 5 commits intofirebase:mainfrom
Conversation
Implement ListActions and ResolveAction on the Ollama plugin so that locally installed models are discovered automatically via /api/tags, removing the need for users to manually register every model. Fixes firebase#4490
Summary of ChangesHello @Zereker, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Ollama plugin's flexibility by introducing dynamic model discovery. By implementing the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces dynamic model discovery for the Ollama plugin, allowing models not explicitly defined to be used via the /api/tags endpoint. The implementation includes new types for Ollama API responses, a function to list local models, and methods to implement the DynamicPlugin interface (newModel, ListActions, ResolveAction). The changes are generally well-structured, but there are a few areas related to efficiency and correctness that could be improved, particularly around HTTP client reuse and assumptions about dynamically discovered model types.
Add tests for listLocalModels, ListActions, ResolveAction and newModel, covering success paths, embed model filtering, server errors, and non-model action type rejection.
Add TestLiveDynamicDiscovery to verify that models not in hardcoded lists can be discovered via ListActions and used via ResolveAction with a real Ollama server.
…icPlugin Group listLocalModels, ListActions, ResolveAction, and newModel tests into a single TestDynamicPlugin with t.Run subtests.
…test The test appended ":latest" to the model name flag, which broke models with explicit tags like moondream:v2 (producing moondream:v2:latest). Use the flag value directly since /api/tags already returns full names.
|
Thank you @Zereker for the contribution! @hugoaguirre please test this out |
Summary
DynamicPlugininterface (ListActions+ResolveAction) on the Ollama plugin to support dynamic model discovery via/api/tagsmoondream:v2) can now be used without manualDefineModelcallsembed)Fixes #4490
Changes
listLocalModels()to call Ollama'sGET /api/tagsendpointnewModel()helper to create model instances without registry registrationListActions()to enumerate locally installed modelsResolveAction()to create models on demandDefineModeland hardcoded capability lists preservedDesign Notes (referencing JS implementation)
This implementation follows the JS plugin (
js/plugins/ollama/src/index.ts) as closely as possible:Media support:
defaultOllamaSupportssetsMedia: truefor all dynamically discovered models, matching JS'sGENERIC_MODEL_INFO. Note that the existinggeneratefunction restricts image extraction to a hardcodedmediaSupportedModelslist — this is a pre-existing limitation in the Go plugin, not introduced by this PR. The JS implementation does not check model capabilities before sending images (it unconditionally forwards images to Ollama). Fixing the Gogeneratefunction to match JS behavior (removing the hardcoded media check) is a separate concern and should be addressed in a follow-up PR.Timeout for
/api/tags: The JS implementation (listLocalModelsat line 167-188) does not set an explicit timeout on thefetchcall either, relying on default behavior. Our Go implementation useshttp.NewRequestWithContext, so callers can control timeout via context. Adding an explicithttp.Clienttimeout could be done as a follow-up improvement but is not required for parity with the JS version.Test plan
go build ./plugins/ollama/passesgo vet ./plugins/ollama/passesgo test ./plugins/ollama/ -v— all unit tests passgo test ./plugins/ollama/ -v -test-live) —ListActionsdiscovered 2 local models,ResolveActiondynamically resolvedmoondream(not in hardcoded lists), and generated a response successfully