Skip to content

Conversation

@daniel-lxs
Copy link
Member

@daniel-lxs daniel-lxs commented Aug 25, 2025

Description

This PR adds logic to hide static providers from the provider dropdown list when they have no models available, while preserving backward compatibility for existing user configurations.

Changes

  • Filter static providers without models: Static providers (those with predefined models in MODELS_BY_PROVIDER) are now hidden from the provider list if they have no models after organization filtering
  • Preserve existing configurations: The currently selected provider is always shown in the list, even if it has no models, to avoid breaking existing user configurations
  • Dynamic providers always visible: Dynamic providers (OpenRouter, Ollama, LiteLLM, etc.) that fetch models asynchronously are always shown to avoid race conditions

Implementation Details

The filtering logic in ApiOptions.tsx:

  1. First applies organization allow list filtering
  2. Checks if each provider is static (has models in MODELS_BY_PROVIDER)
  3. For static providers, verifies they have at least one model after filtering
  4. Always includes the currently selected provider
  5. Always includes dynamic providers

Testing

Added comprehensive test suite in ApiOptions.provider-filtering.spec.tsx that verifies:

  • All providers are shown when no organization allow list is provided
  • Static providers with empty models are hidden
  • Dynamic providers are always shown (even if they have no models yet)
  • Static providers are filtered based on organization allow list
  • Static providers with allowAll: true are always shown
  • Currently selected provider is always shown even if it has no models

All tests pass successfully ✅

Impact

This improves the user experience by:

  • Reducing clutter in the provider dropdown
  • Only showing providers that have available models
  • Maintaining backward compatibility for users with existing configurations

Fixes the issue where providers with no models would appear in the list but be unusable.


Important

Hides static providers without models from the provider list in ApiOptions.tsx, ensuring dynamic providers and selected providers are always visible, with comprehensive tests added.

  • Behavior:
    • In ApiOptions.tsx, static providers without models are hidden from the dropdown list unless they are currently selected.
    • Dynamic providers are always shown to prevent race conditions with async model fetching.
    • Organization allow list filtering is applied to static providers.
  • Testing:
    • Added ApiOptions.provider-filtering.spec.tsx to test provider visibility based on model availability and organization allow list.
    • Tests ensure static providers with no models are hidden, dynamic providers are always shown, and selected providers are always visible.
  • Misc:
    • Ensures backward compatibility by always showing the currently selected provider even if it has no models.

This description was created by Ellipsis for eec762b. You can customize this summary. It will automatically update as commits are pushed.

- Add logic to filter out static providers that have no models after organization filtering
- Always show currently selected provider to preserve existing configurations
- Only apply filtering to static providers to avoid race conditions with dynamic providers
- Add comprehensive test suite to verify behavior

This improves the user experience by hiding providers that have no available models while maintaining backward compatibility for existing configurations.
@daniel-lxs daniel-lxs requested review from cte, jr and mrubens as code owners August 25, 2025 16:08
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Aug 25, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Review] in Roo Code Roadmap Aug 25, 2025
@dosubot dosubot bot added the enhancement New feature or request label Aug 25, 2025
})

it("should hide static providers with empty models", () => {
// Mock MODELS_BY_PROVIDER to have an empty provider
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests directly mutate global constants (MODELS_BY_PROVIDER and PROVIDERS) for setup/cleanup. Consider using beforeEach/afterEach or deep clones to avoid side effects and test order dependencies.

Copy link
Contributor

@roomote roomote bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution! I've reviewed the changes and this is a well-implemented feature that successfully hides static providers with no models from the provider dropdown list.

Positive Aspects:
✅ Excellent backward compatibility - always shows the currently selected provider even if it has no models
✅ Smart distinction between static and dynamic providers
✅ Comprehensive test coverage with 6 well-structured test cases
✅ Clear code comments explaining the logic
✅ Proper handling of organization allow lists

I've left a few minor suggestions inline that could improve code organization and maintainability, but overall this implementation is solid and ready to merge. Great work on maintaining backward compatibility and providing comprehensive test coverage!

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Aug 25, 2025
@mrubens mrubens merged commit c75f184 into main Aug 25, 2025
22 checks passed
@mrubens mrubens deleted the feature/hide-empty-providers branch August 25, 2025 21:17
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Aug 25, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Aug 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request lgtm This PR has been approved by a maintainer PR - Needs Review size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants