-
Notifications
You must be signed in to change notification settings - Fork 2.6k
refactor: align HuggingFace provider with established pattern #6205
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
Conversation
- Move huggingface-models.ts from src/services/ to src/api/providers/fetchers/huggingface.ts - Update fetcher to return ModelInfo records instead of raw HuggingFace models - Add HuggingFace to RouterName type and integrate with modelCache.ts - Update HuggingFace provider to extend RouterProvider base class - Remove unnecessary src/api/huggingface-models.ts wrapper - Update webviewMessageHandler to use the new pattern with getModels() - Maintain backward compatibility with webview by transforming ModelInfo to expected format
|
@roomote-agent Can we get rid of the |
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
|
Hi @cte! I see your suggestion about removing |
… instead - Removed requestHuggingFaceModels message type from WebviewMessage.ts - Removed requestHuggingFaceModels handler from webviewMessageHandler.ts - Removed huggingFaceModels from ExtensionMessage.ts - Updated HuggingFace.tsx component to use requestRouterModels and extract HuggingFace models from the routerModels response This simplifies the codebase by using a single unified message type for all router-based model providers.
| setModels(message.huggingFaceModels || []) | ||
| case "routerModels": | ||
| // Extract HuggingFace models from routerModels | ||
| if (message.routerModels?.huggingface) { |
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.
Consider extracting the conversion logic (lines 75–108) into a separate helper for improved readability and maintainability.
This comment was generated because it violated a code review rule: irule_tTqpIuNs8DV0QFGj.
|
Done! I've successfully removed Here's what I changed:
This simplifies the codebase by using a single unified message type for all router-based model providers. All tests are passing and CI checks have completed successfully. |
|
Done by #6228 |
Summary
This PR refactors the HuggingFace provider implementation to match the established pattern used by other providers that fetch models via network calls (e.g., OpenRouter, Glama, Ollama).
Changes
huggingface-models.tsfromsrc/services/tosrc/api/providers/fetchers/huggingface.tsRecord<string, ModelInfo>instead of raw HuggingFace model data, consistent with other providersRouterNametype and integrated it with themodelCache.tssystem for memory and file cachingRouterProviderbase class and uses thefetchModel()patternsrc/api/huggingface-models.tsas it's no longer neededwebviewMessageHandler.tsto use the new pattern withgetModels()while maintaining backward compatibilityBenefits
Testing
Important
Refactor HuggingFace provider to align with other providers, improving consistency, caching, and maintainability.
huggingface-models.tsmoved tosrc/api/providers/fetchers/huggingface.ts.Record<string, ModelInfo>instead of raw data.modelCache.tsfor caching.webviewMessageHandler.tsto usegetModels()for HuggingFace.HuggingFaceHandlernow extendsRouterProviderand usesfetchModel().src/api/huggingface-models.tsas it's redundant.RouterNametype inapi.ts.ExtensionMessage.tsandWebviewMessage.tsto reflect changes.HuggingFace.tsxto handle new model fetching logic.This description was created by
for 86e05ab. You can customize this summary. It will automatically update as commits are pushed.