Skip to content

feat: manual model filter for providers and filtered model dropdown#8287

Open
FedeCuci wants to merge 2 commits into
janhq:mainfrom
FedeCuci:feat/manual-model-filter
Open

feat: manual model filter for providers and filtered model dropdown#8287
FedeCuci wants to merge 2 commits into
janhq:mainfrom
FedeCuci:feat/manual-model-filter

Conversation

@FedeCuci

@FedeCuci FedeCuci commented Jun 6, 2026

Copy link
Copy Markdown

Describe Your Changes

Some providers like OpenRouter and Together.ai supply hundreds of models, which makes scrolling through them tedious, especially after you've already picked the few you actually want to use. This PR lets you pin your own selection so that only your chosen models appear in the model dropdown and provider settings list. Auto-fetched models stay in the background and are always one click away via the refresh button. More specifically:

  • Settings → Providers: Add filter button to show only manually-added models ($providerName.tsx)
  • Chat → Model Dropdown: Hide auto-fetched models when user has pinned models (DropdownModelProvider.tsx)
  • Settings → Add Model Dialog: Upgrade existing models instead of rejecting duplicates (AddModel.tsx)
  • Settings → Edit Model Dialog: Mark edited models as manually-added (EditModel.tsx)
  • State Migration: Backfill manuallyAdded flag for existing users with customized models (useModelProvider.ts)
  • Tests: Update icon mock for new filter button ($providerName.test.tsx)

Fixes Issues

N/A — quality of life improvement, no linked issue.

Self Checklist

  • Added relevant comments, esp in complex areas
  • Created issues for follow-up changes or refactoring needed — none needed
  • Updated docs (for bug fixes / features) — N/A, no API or config changes

- Add filter button in provider settings to show only manually-added models
- Auto-fetched models hidden when user has pinned models
- Model dropdown filters to pinned models when available
- Add Model dialog upgrades existing models instead of rejecting duplicates
- Backfill migration for existing users with customized models
- One-time filter resets on refresh
@tokamak-pm

tokamak-pm Bot commented Jun 7, 2026

Copy link
Copy Markdown

PR Review: feat: manual model filter for providers and filtered model dropdown

Summary

This PR adds a "manual model filter" feature so that users who have explicitly added or customized models can filter out the auto-fetched catalog, keeping only their curated selection visible. The changes touch 6 files across the model dropdown, add/edit model dialogs, provider settings page, state migration, and tests.

Code Quality & Correctness

1. Pervasive use of (m as any).manuallyAdded -- missing type definition (fix needed)

The Model type in web-app/src/types/modelProviders.d.ts is never updated to include manuallyAdded. Every access uses (m as any).manuallyAdded, which defeats TypeScript's purpose. The property should be added to the Model type:

type Model = {
  // ...existing fields...
  /** Whether this model was explicitly added or customized by the user. */
  manuallyAdded?: boolean
}

This would eliminate all as any casts across the PR and make the intent discoverable to future contributors.

2. Filter button is one-way -- no toggle off (fix needed)

In $providerName.tsx, the filter button sets showManualOnly to true and is then disabled. The only way back is to click the refresh button, which re-fetches all models from the remote API. This is unintuitive and destructive -- users may just want to toggle the view without re-fetching. The button should toggle the state:

onClick={() => setShowManualOnly((prev) => !prev)}
disabled={false}

Or at least provide a separate "show all" button that does not trigger a network request.

3. Duplicated isManuallyAdded logic (improve needed)

The definition of "manually added" is checked in three separate places with slightly different logic:

  • $providerName.tsx: checks manuallyAdded || imported || displayName || _userConfiguredCapabilities
  • DropdownModelProvider.tsx: checks only manuallyAdded === true
  • useModelProvider.ts migration: backfills based on displayName || _userConfiguredCapabilities

The provider settings page has a broader definition than the dropdown. This means a model with displayName set (but no manuallyAdded flag) will appear in the settings filter but NOT in the chat dropdown. This inconsistency will confuse users. Extract a single isManuallyAdded(model) utility function and use it everywhere.

4. Migration backfill uses displayName as a heuristic -- false positives possible (improve needed)

The migration marks any model with a displayName as manuallyAdded. If the upstream provider catalog sets displayName on fetched models, those will be incorrectly flagged. This is a one-time migration so the blast radius is limited, but worth documenting the assumption.

5. EditModel.tsx -- editing without actual changes still does not flag manuallyAdded (acceptable)

The guard if (nameChanged || capabilitiesChanged) means simply opening and saving the edit dialog without changes won't flag the model. This is correct behavior.

6. Toast message not internationalized (improve needed)

In AddModel.tsx, the new toast uses a hardcoded English string 'Model added to your collection' instead of using the t() translation function. The rest of the file uses i18n. This should be t('providers:addModel.modelPinned') or similar.

7. useMemo dependency arrays are incomplete (fix needed)

In $providerName.tsx:

const displayedChatModels = useMemo(
  () => (showManualOnly ? chatModels.filter(isManuallyAdded) : chatModels),
  [chatModels, showManualOnly]
)

isManuallyAdded is defined as a plain function inside the component body (not wrapped in useCallback), so it's a new reference every render. While this doesn't cause bugs here because the function is pure and captures no state, it should either be moved outside the component or the dependency array should be noted. More importantly, if isManuallyAdded ever closes over component state, this will silently produce stale results.

Risks & Regressions

  • Existing users upgrading: The migration backfill (version 17 -> 18) will suddenly mark models as manuallyAdded based on heuristics. If a provider auto-populates displayName, users could see unexpected filtering behavior in the chat dropdown. This could lead to models "disappearing" from the dropdown.
  • Chat dropdown hiding models: The DropdownModelProvider.tsx change is the riskiest -- if any model for a provider has manuallyAdded === true, all non-manual models are hidden from the chat model selector. A user who adds one model via the dialog could be surprised that all other models from that provider vanish from the chat dropdown with no indication why.
  • No persistence of filter state: The showManualOnly toggle resets on navigation/remount. This is acceptable for a view filter but should be documented.

Missing Tests

  • No tests for the DropdownModelProvider filtering logic (most impactful change).
  • No tests for the migration backfill logic in useModelProvider.ts.
  • No tests for the AddModel dialog's new "upgrade existing model" path.
  • The only test change is adding an IconFilter mock -- this is insufficient coverage for a feature that changes model visibility.

Recommendation: fix needed

The feature concept is solid and the PR description is thorough, but the following should be addressed before merging:

  1. Add manuallyAdded?: boolean to the Model type -- eliminate all as any casts.
  2. Make the filter button a toggle (or add a separate "show all" that doesn't re-fetch) -- current UX is confusing.
  3. Unify the isManuallyAdded logic into a shared utility to prevent the settings/dropdown inconsistency.
  4. Internationalize the toast in AddModel.tsx.
  5. Add tests for the dropdown filtering and migration backfill at minimum.

@FedeCuci

FedeCuci commented Jun 7, 2026

Copy link
Copy Markdown
Author

Review feedback addressed

1. manuallyAdded added to the Model type

manuallyAdded?: boolean and _userConfiguredCapabilities?: boolean are now declared in modelProviders.d.ts. All (m as any).manuallyAdded casts across the PR are removed, and the Partial<Model> & { manuallyAdded?: boolean } intersection in EditModel.tsx is gone.

2. Filter button is now a real toggle

The filter button in the provider settings page now calls setShowManualOnly(prev => !prev) and is no longer disabled when active. You can flip back to "show all" without triggering a network re-fetch. The refresh button still clears the filter as a side effect, which is intentional — after pulling a fresh catalog it makes sense to start from the full list.

3. Unified isManuallyAdded — settings page and chat dropdown now consistent

Extracted a single isManuallyAdded(model: Model): boolean utility in src/lib/models.ts. The definition is narrow and explicit: manuallyAdded === true || imported === true. Both $providerName.tsx and DropdownModelProvider.tsx now import and use this same function, so a model cannot appear manual in one view and non-manual in the other.

4. Migration heuristic tightened — fixes the dropdown regression

The v17→18 migration (useModelProvider.ts) no longer treats displayName alone as a signal for "user curated". It now only backfills manuallyAdded when the display name genuinely differs from both the model's name and id (i.e. the user actually renamed it), or when _userConfiguredCapabilities is set. This prevents remote catalogs that echo displayName === name from incorrectly flagging auto-fetched models, which was the root cause of the "models disappeared from the chat dropdown" regression.

5. Toast and UI strings internationalized

  • AddModel.tsx: toast now uses t('providers:addModel.modelPinned') instead of a hardcoded string.
  • Filter button tooltip and empty-state copy now use providers:manualFilter.tooltip, providers:manualFilter.emptyTitle, and providers:manualFilter.emptyDescription. Keys added to en/providers.json; other locales fall back to English via i18next until translated.

6. Tests

Three areas covered:

src/lib/__tests__/models.test.ts — 5 new cases for isManuallyAdded, pinning down the exact contract: manuallyAdded flag → true, imported flag → true, plain model → false, displayName alone → false, _userConfiguredCapabilities alone → false.

src/hooks/__tests__/useModelProvider.test.ts — new v17 → v18 migration case driving the real migrate function with the full matrix: capability-toggled models flagged, genuinely renamed models flagged, catalog echo (displayName === name / === id) NOT flagged, plain auto-fetched models untouched, already-flagged models preserved, imported models left for read-time.

src/containers/__tests__/DropdownModelProvider.manualFilter.test.tsx — 4 new cases for the dropdown filtering logic: pinned model hides auto-fetched siblings, imported models count as pinned, no-pins shows everything, catalog displayName alone does not activate the filter.

@tokamak-pm tokamak-pm Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Follow-up review (new commits detected since last review)

The second commit (982408b — "fix: address manual model filter PR review feedback") addresses all six concerns from our previous review:

Previously raised issues — now resolved

  1. Model type definition: manuallyAdded?: boolean and _userConfiguredCapabilities?: boolean are now declared in modelProviders.d.ts, eliminating the need for as any casts.

  2. Filter button toggle: The filter button now properly toggles via setShowManualOnly((prev) => !prev) instead of being one-way/disabled. Refresh still resets the filter, which is sensible behavior.

  3. Unified isManuallyAdded logic: A single isManuallyAdded utility in @/lib/models.ts is now shared by both DropdownModelProvider.tsx and $providerName.tsx. The function checks only persisted flags (manuallyAdded === true || imported === true), keeping the heuristic-based logic confined to the one-time v17-to-v18 migration. This is a clean separation.

  4. Internationalized toast: AddModel.tsx now uses t('providers:addModel.modelPinned') with the corresponding key added to providers.json.

  5. Test coverage: Three new test files/suites were added:

    • DropdownModelProvider.manualFilter.test.tsx — 4 test cases covering pinned-model filtering, imported models, no-pinned-models fallback, and catalog displayName false-positive avoidance.
    • useModelProvider.test.ts — migration backfill test (v17 to v18) covering curated, renamed, echo-name, plain, already-flagged, and imported scenarios.
    • models.test.ts — 5 test cases for the isManuallyAdded predicate.
  6. Migration heuristic tightened: The backfill now explicitly skips models where displayName === name or displayName === id, preventing false positives from catalogs that echo the model identifier as the display name.

Remaining observations (non-blocking)

  • useMemo and isManuallyAdded reference: isManuallyAdded is imported as a module-level function (not defined inside the component), so it is a stable reference. The useMemo dependency arrays in $providerName.tsx are correct as-is.

  • Chat dropdown UX: When a provider has any pinned model, all non-pinned models are hidden from the chat dropdown with no in-dropdown indication. This is by design per the PR description ("Auto-fetched models stay in the background and are always one click away via the refresh button"), but it may surprise users who pin one model and see others vanish. Consider adding a brief indicator or count (e.g., "+47 more models") in a future iteration — not a blocker for this PR.

  • Empty-state UX in settings: The empty state when the manual filter is active and no models match is well-handled with a clear message and guidance to use the "Add Model" button or toggle the filter off.

Recommendation: can merge

@tokamak-pm

tokamak-pm Bot commented Jun 9, 2026

Copy link
Copy Markdown

Follow-up Review — All Prior Feedback Addressed

Reviewing commit: 982408b — addressing all 6 issues from the previous review.

Point-by-Point Resolution

Issue from last review Status
manuallyAdded missing from Model type Resolved — properly declared in modelProviders.d.ts with JSDoc
Filter button one-way (no toggle off) Resolved — now a real toggle with visual feedback
Duplicated isManuallyAdded logic Resolved — unified helper exported from lib/models.ts
Migration heuristic false positives Resolved — tightened to require displayName differs from both name and id
Toast not internationalized Resolved — all strings use t() with proper keys
Missing tests Resolved — 3 test suites added (models utility, migration, dropdown filtering)

Code Quality

  • The unified isManuallyAdded predicate eliminates the settings/dropdown inconsistency that was the most dangerous regression risk.
  • The migration test covers the full matrix including the echo cases (displayName === name).
  • Component-level tests for the dropdown verify pinned models hide auto-fetched ones correctly.
  • All (m as any).manuallyAdded casts have been removed.

Minor Observations (Not Blocking)

  • When the chat dropdown silently hides auto-fetched models, there is no visual indicator. Users who are confused about missing models have no obvious path to discover why. Worth tracking as a follow-up issue.
  • The empty-state banner logic handles the edge cases correctly for chat models.

Recommendation: can merge

All six blocking issues from the previous review have been addressed correctly and thoroughly. The feature is ready to land.

@Liamelis

Copy link
Copy Markdown

would be cool to have this!
I think a search bar in the settings where you can search for models would be very handy too. (rn you can only search for models when adding a model but when you have tons of models and just wanna edit one to enable tool calling for example you have to scroll and find it.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants