Skip to content

fix(#1106): iterate custom_providers[].models dict keys for dropdown population#1111

Closed
bergeouss wants to merge 1 commit intonesquena:masterfrom
bergeouss:fix/1106-custom-providers-models-dict
Closed

fix(#1106): iterate custom_providers[].models dict keys for dropdown population#1111
bergeouss wants to merge 1 commit intonesquena:masterfrom
bergeouss:fix/1106-custom-providers-models-dict

Conversation

@bergeouss
Copy link
Copy Markdown
Contributor

Thinking Path

  • custom_providers[].models dict (YAML key-value) is parsed but never iterated for dropdown population
  • Only the singular model field was read → users with multiple models in models dict see only one
  • This compounds with SSRF false positives (SSRF check blocks legitimate local-network custom providers from model auto-detection #1105): when auto-detect is blocked, custom_providers is the only fallback but only picks up one model
  • Fix: after reading model field, also iterate models dict keys and add each as a selectable model

What Changed

  • api/config.py: _build_available_models_uncached() now collects model IDs from both model (singular) and models (dict) fields in custom_providers entries. Dict keys are deduplicated against the singular model field. Non-string keys are silently skipped.
  • tests/test_issue1106_custom_providers_models.py: 7 regression tests covering dict iteration, no duplicates, overlap handling, empty dict, non-string keys, unnamed providers, and multiple named providers.

Why It Matters

Users with local inference servers (llama.cpp, llama-swap, vLLM, TabbyAPI) list their models in custom_providers[].models in config.yaml. Currently only one model appears in the dropdown. After this fix, all models from the dict are available for selection.

Verification

  • pytest tests/test_issue1106_custom_providers_models.py -v — 7/7 pass
  • pytest tests/test_custom_provider_display_name.py tests/test_model_resolver.py -v — 26/27 pass (1 pre-existing failure)
  • Syntax check: py_compile api/config.py — OK

Risks / Follow-ups

Model Used

  • Provider: zai
  • Model: glm-5-turbo
  • Tools: Hermes Agent

Closes #1106

…ropdown population

- After reading singular 'model' field, also iterate 'models' dict keys
- Deduplicate: model field value not repeated if also in models dict
- Skip non-string keys gracefully
- Works for both named and unnamed custom_providers entries
- Add 7 regression tests
@nesquena-hermes
Copy link
Copy Markdown
Collaborator

Thanks for this fix! The root cause analysis is accurate — custom_providers[].models dict was silently ignored, leaving users with only the singular model field populating the dropdown.

A few notes on the implementation:

Logic looks correct. Iterating dict keys and deduplicating against the singular model field is the right approach. Silently skipping non-string keys is a safe choice.

Test coverage is solid — 7 regression tests covering the key edge cases (overlap, empty dict, non-string keys, unnamed providers). The 1 pre-existing test failure in test_model_resolver.py is worth noting in the PR description so reviewers don't flag it as a regression — good that it's already called out in Verification.

One thing to verify: Does the fix handle the case where models is present but model is absent (i.e., only the dict, no singular field)? The description says "also iterates models dict keys" after reading model, but it's worth confirming this works when model is unset entirely.

Relates to #1105 (SSRF blocking auto-detect) — once both are fixed, local inference servers (llama-swap, vLLM, llama.cpp) should work end-to-end from custom_providers config.

@bergeouss
Copy link
Copy Markdown
Contributor Author

Yes — when only the models dict is present (no singular model field), it works correctly:

_cp_model = _cp.get("model", "")       # returns "" if absent → not added
_cp_models_dict = _cp.get("models")     # iterates dict keys

The singular field is optional — if it's missing, _cp_model_ids starts empty and only gets populated from the models dict keys.

Already covered by test test_models_dict_without_model_field_still_works which configures only "models": {"llama-3-8b": {}, "mistral-7b": {}} with no "model" field — both models appear in the dropdown. ✅

@nesquena-hermes
Copy link
Copy Markdown
Collaborator

Merged in v0.50.221 via PR #1117. Thank you @bergeouss — great contribution (custom providers models dict)! 🎉

1 similar comment
@nesquena-hermes
Copy link
Copy Markdown
Collaborator

Merged in v0.50.221 via PR #1117. Thank you @bergeouss — great contribution (custom providers models dict)! 🎉

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

Labels

merge soon Reviewed and queued for next release batch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

custom_providers[].models dict ignored — only singular model field populates dropdown

2 participants