fix: show same model from different custom providers instead of deduplicating#1947
Conversation
SummaryPulled this branch and read the full diff: What this fixes (and where it overlaps with #1874)The backend half is correct and necessary. On master, _dedup_key = f"{_slug}:{_cp_model}" if _slug else _cp_model
if _cp_model and _dedup_key not in _seen_custom_ids:
_cp_label = _get_label_for_model(_cp_model, [])
_seen_custom_ids.add(_dedup_key)That said — this overlaps significantly with PR #1874 ("Fix duplicate models across named custom providers"), which already includes a per-group seen set ( If maintainers are leaning toward #1874 landing first, this PR's backend change is largely subsumed. If #1874 is stuck on review, the narrower change here is a clean partial fix and reasonable to land standalone — but rebasing #1874 on top would then cost work. Frontend half — the dedup loosening is risky in isolationThe if(existingNormProvider.has(normKey)) continue; // dedup same provider+normId only
if(existingNorm.has(_normId(mid)) && !_providerOf(mid)) continue; // dedup non-prefixed cross-prefix onlyis the bit I'd ask you to walk through carefully. The original The right invariant, I think, is: dedup if Worth pulling up Recommendation
The backend half is solid and this is a real bug. I'd just want to see #907 coverage before signing off on the frontend change. |
…licating When multiple custom providers expose the same model ID (e.g. baidu, huoshan, and liantong all offering glm-5.1), only the first provider's entry was shown in the model dropdown. Root cause (backend): used the bare model ID as the dedup key, so the second and subsequent providers with the same model were silently skipped. Root cause (frontend): stripped the @Provider: prefix before comparing, so @Custom:baidu:glm-5.1 and @Custom:huoshan:glm-5.1 were treated as duplicates. Fix: - Backend: change _seen_custom_ids key to '{slug}:{model_id}' so each provider's models are tracked independently. - Frontend: add _providerOf() helper and deduplicate on the composite (normId, provider) key instead of normId alone. Bare model IDs (without @Provider: prefix) still deduplicate on normId for backward compatibility.
b358b0e to
a6599cd
Compare
Updated FixThanks for the detailed review! We've simplified the fix based on your feedback: Only the backend change is kept — the frontend Why the backend-only approach works:
The backend fix:- _seen_custom_ids = {m["id"] for m in auto_detected_models}
+ _seen_custom_ids = set()
- if _cp_model and _cp_model not in _seen_custom_ids:
+ _dedup_key = f"{_slug}:{_cp_model}" if _slug else _cp_model
+ if _cp_model and _dedup_key not in _seen_custom_ids:
- _seen_custom_ids.add(_cp_model)
+ _seen_custom_ids.add(_dedup_key)Using Verified on our deployment:
This aligns with your recommendation to avoid the risky frontend loosening. Let us know if you'd like any adjustments! |
Backend-only fix LGTM, but please add a regression testRe-read the diff at PR #1947:1 (now 4/3 lines, only The simplified fix is correct. Removing the _cp_option_id = _cp_model
if active_provider != _slug and not _cp_option_id.startswith("@"):
_cp_option_id = f"@{_slug}:{_cp_option_id}"so the active provider keeps the bare id and the non-active siblings get One blocking ask: add a testThe fix is minimal enough that the absence of a test makes this hard to merge confidently. There's a parallel PR (#1874) that addresses the same issue and does ship a test — I'd ask either:
Without it, future refactors of Notes on coexistence with #1874These two PRs are now nearly disjoint in scope:
Either one resolves the user-reported symptom. If maintainers prefer the surgical path, this PR + the test from #1874 would land cleanly. If they want the broader refactor, #1874 supersedes this. Worth a Verification stepThe reporter's |
CHANGELOG, ROADMAP, TESTING refresh for v0.51.31 stage release covering 12 contributor PRs: Added (2 PRs): - #1956 JKJameson — persistent composer draft (server-side, cross-client) - #1957 hermes-gimmethebeans — configurable session TTL via env + settings Fixed (10 PRs): - #1939 ai-ag2026 — theme-color + sw cache regression coverage - #1941 ai-ag2026 — preserve chat scroll across final render - #1945 franksong2702 — localize session jump controls (#1938) - #1947 happy5318 — show same model from different custom providers (Co-authored-by hacker1e7 for #1874 close) - #1949 Sanjays2402 — close #1937 endless-scroll vs Start-jump race with generation-token + mutex (Co-authored-by franksong2702 + Michaelyklam) - #1950 franksong2702 — mute stale stopped gateway heartbeat (#1944) - #1951 amlyczz — gate goal hook on goal-related turns (#1932) (Co-authored-by franksong2702 for #1946 close) - #1953 lucky-yonug — skip provider peel for custom host:port slugs - #1960 Michaelyklam — translate hidden-files workspace label (#1841) - #1961 sbe27 — respect image_input_mode (#1959) Closed in favor of canonical: #1942, #1962, #1946, #1874, #1311. Stage-326 hotfixes (per Opus advisor): - CRITICAL #1951 PENDING_GOAL_CONTINUATION race fix (removed finally discard that race-erased the marker before consumer could read it) - #1956 composer-draft input validation (50 KB text / 50 file clamp + type coercion to prevent unbounded session-JSON bloat) - #1957 SESSION_TTL constant preserved as named fallback (existing regression tests pin it; #1957 originally deleted it) Tests: 5006 → 5028 (+51 net new) — 0 regressions, 142.61s runtime.
8a653ba
…d-custom-providers Adds tests/test_pr1947_same_model_multiple_custom_providers.py covering: 1. Two named custom providers exposing the same model id — both must surface in the rendered groups (one bare, one @Custom:slug:model) 2. Three named providers all exposing the same model — none dropped 3. Distinct-model-per-provider sanity check (still grouped correctly) Verified the regression-detecting tests (1 + 2) FAIL against master's api/config.py (where _seen_custom_ids was seeded from auto_detected_models and used as a global bare-id bucket — the second provider's entry was silently dropped) and PASS against the contributor fix on this branch. Test 3 (distinct-models sanity) passes either way as expected. Co-authored-by: happy5318 <happy5318@users.noreply.github.com> Co-authored-by: hacker1e7 <hacker1e7@users.noreply.github.com>
…tom providers instead of deduplicating by @happy5318
Problem
When multiple custom providers expose the same model ID (e.g.
baidu,huoshan, andliantongall offeringglm-5.1), only the first provider's entry appears in the model dropdown. The others are silently dropped.Root Cause
Backend (
api/config.py):_seen_custom_idsis initialized with bare model IDs fromauto_detected_modelsand used as a global dedup set when iteratingcustom_providers. Since the dedup key is the bare model ID (e.g.glm-5.1), the second and subsequent providers offering the same model are skipped.Frontend (
static/ui.js):_normId()strips the@provider:prefix and normalizes separators, so@custom:baidu:glm-5.1and@custom:huoshan:glm-5.1both normalize toglm.5.1and are treated as duplicates.Fix
Backend: Change the
_seen_custom_idsdedup key to{slug}:{model_id}so each provider's models are tracked independently.Frontend: Add
_providerOf()helper and deduplicate on the composite(normId, provider)key. Bare model IDs without@provider:prefix still deduplicate onnormIdalone for backward compatibility.Testing
Verified with a config containing 5 custom providers, where
glm-5.1appears underbaidu,huoshan, andliantong. After the fix, all three entries appear in the dropdown with their@custom:provider:prefix.GET /api/modelsnow returns: