fix: deduplicate configured model badges

[webtecnica]

Summary

• Register configured-model badges only for the canonical picker option.
• Apply the same behavior to both live and static model-catalog builders.
• Add regression coverage for equivalent routing IDs and fallback badges.

Problem

The catalog registered the same configured model under bare, provider/model, and @provider:model forms. The frontend treated these as separate configured entries in some cases, so one model appeared more than once in the picker.

Changes

• static/ui.js: Added _isEquivalentConfiguredModelEntry() to compare model entries by normalized key + provider, replacing the simple _existingConfiguredKeys set deduplication. Updated _modelStateForSelect() to resolve provider from the matched option's authoritative data-provider. Updated _ensureModelOptionInDropdown() for provider-qualified fallback rows.
• tests/test_configured_model_picker_dedup.py: New regression tests for deduplication logic
• tests/test_configured_model_picker_provider_routing.py: New regression tests for provider-qualified fallback rows
• tests/test_issue3691_model_picker_show_all.py: Added _isEquivalentConfiguredModelEntry to extracted functions

Closes #6221

[greptile-apps]

Greptile Summary

This PR fixes duplicate configured-model entries in the picker by replacing a simple normalized-key Set with _isEquivalentConfiguredModelEntry, which also checks provider identity and handles @provider:model routing aliases. It also corrects _modelStateForSelect and _ensureModelOptionInDropdown to read the provider from an option's authoritative data-provider attribute rather than reparsing the value at its last colon — which mis-parsed colon-bearing model IDs like model-a:free.

• static/ui.js: new _isEquivalentConfiguredModelEntry gating the badge-synthesis loop; _modelStateForSelect reads data-provider via _getOptionProviderId for @provider:model values; _ensureModelOptionInDropdown constructs the full @provider:bareModel option value and stores data-model for later retrieval.
• tests/test_configured_model_picker_dedup.py and tests/test_configured_model_picker_provider_routing.py: new Node-driver regression tests covering equivalence logic and provider-qualified fallback rows, including the colon-bearing model-id edge case.
• tests/test_issue3691_model_picker_show_all.py: _isEquivalentConfiguredModelEntry added to four extractFunc blocks so existing show-all tests compile after the new function is introduced.

Confidence Score: 4/5

Safe to merge; the deduplication logic is well-covered by the new Node-driver tests and the core provider-routing fix is exercised end-to-end.

The functional logic looks correct: the equivalence function handles the known edge cases (same model id, different providers; @provider:model aliases; colon-bearing model ids). The main concerns are cosmetic — dead intermediate writes to opt.dataset.provider and a fragile whitespace-sensitive substring assertion that could stop testing its invariant after a reformat, but neither affects runtime behavior.

The whitespace-sensitive assertion in tests/test_configured_model_picker_dedup.py is the only thing worth a second look before merge.

Important Files Changed

Filename	Overview
static/ui.js	Introduces _isEquivalentConfiguredModelEntry for smarter badge deduplication and updates _modelStateForSelect/_ensureModelOptionInDropdown to use authoritative data-provider instead of last-colon reparsing; dead intermediate opt.dataset.provider assignments at lines 3276–3277 are a minor clean-up item.
tests/test_configured_model_picker_dedup.py	New regression tests for _isEquivalentConfiguredModelEntry; Node-driver tests are solid, but one assertion uses a hardcoded newline-plus-indentation substring that will silently lose its invariant on any reformatting of ui.js.
tests/test_configured_model_picker_provider_routing.py	New Node-driver regression tests for provider-qualified fallback row behavior and colon-bearing model IDs; coverage is thorough and drivers are self-contained.
tests/test_issue3691_model_picker_show_all.py	Adds _isEquivalentConfiguredModelEntry to the four extractFunc blocks so existing show-all tests continue to compile after the new function is introduced.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["renderModelDropdown()"] --> B["Build _modelData from dropdown optgroups\n(with providerId)"]
    B --> C["For each badge in _badgeMap"]
    C --> D{"_isEquivalentConfiguredModelEntry\n(modelId, badge, _modelData)"}
    D -- "true (dup)" --> C
    D -- "false (new)" --> E["Push to _modelData\n(no providerId)"]
    E --> C

    F["_ensureModelOptionInDropdown(modelId, sel, preferredProviderId)"] --> G["_applyModelToDropdown(modelId, sel, requestedProvider)"]
    G --> H{"option found AND\nprovider matches?"}
    H -- "yes" --> I["return applied value"]
    H -- "no" --> J["Build value = @provider:bareModel\nCreate option with data-provider + data-model\nAppend to select"]
    J --> I

    K["_modelStateForSelect(sel, modelId)"] --> L{"explicitProvider\nin value?"}
    L -- "yes" --> M["Find matching option by value\nread provider via _getOptionProviderId(selected)\nread model via dataset.model"]
    M --> N["return {model, model_provider}"]
    L -- "no" --> O["Find option by value\nread provider via _getOptionProviderId(opt)"]
    O --> N

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["renderModelDropdown()"] --> B["Build _modelData from dropdown optgroups\n(with providerId)"]
    B --> C["For each badge in _badgeMap"]
    C --> D{"_isEquivalentConfiguredModelEntry\n(modelId, badge, _modelData)"}
    D -- "true (dup)" --> C
    D -- "false (new)" --> E["Push to _modelData\n(no providerId)"]
    E --> C

    F["_ensureModelOptionInDropdown(modelId, sel, preferredProviderId)"] --> G["_applyModelToDropdown(modelId, sel, requestedProvider)"]
    G --> H{"option found AND\nprovider matches?"}
    H -- "yes" --> I["return applied value"]
    H -- "no" --> J["Build value = @provider:bareModel\nCreate option with data-provider + data-model\nAppend to select"]
    J --> I

    K["_modelStateForSelect(sel, modelId)"] --> L{"explicitProvider\nin value?"}
    L -- "yes" --> M["Find matching option by value\nread provider via _getOptionProviderId(selected)\nread model via dataset.model"]
    M --> N["return {model, model_provider}"]
    L -- "no" --> O["Find option by value\nread provider via _getOptionProviderId(opt)"]
    O --> N

Loading

_{Reviews (1): Last reviewed commit: "fix: deduplicate configured model badges" | Re-trigger Greptile}

[greptile-apps]

Dead dataset.provider assignments always overwritten

The two opt.dataset.provider writes on these lines are unconditionally overwritten four lines later by if(provider) opt.dataset.provider=provider;. provider is computed as requestedProvider||(badge&&badge.provider)||(rawBadge&&rawBadge.provider)||..., so it can never be falsy while either badge.provider or rawBadge.provider is truthy — the exact conditions that guard lines 3276–3277. Readers may expect the intermediate assignments to survive in some case, but they never do.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[greptile-apps]

Whitespace-sensitive substring assertion will break on reformatting

assert "providerId,\n modelsEndpointError," in ui encodes a specific newline and exactly 10 spaces of indentation from the overflow-model push in renderModelDropdown. Any auto-formatter, minifier, or even an unrelated whitespace tweak in that block will silently turn this assertion into a false positive (the assert passes at import/read time but stops testing the intended invariant). The structural invariant — that overflow entries carry providerId — could be checked by the same Node driver used in the other tests rather than by substring scan.

[cutter-sh]

🎬 Cutter preview — PR #6275

Open model picker dropdown — Model picker lists each configured model once, with no duplicate provider entries.

[nesquena-hermes]

Closing — this exact fix already shipped in exp-v0.52.110 via #6221 (same approach: register configured-model badges only for the canonical picker option, applied to both the live and static catalog builders, with equivalent-routing-id regression coverage). Current master already contains _isEquivalentConfiguredModelEntry, so the picker no longer shows a configured model more than once.

Thanks @webtecnica — the diagnosis and fix were spot-on; #6221 just landed first. If you spot any remaining picker-dedup edge on the current build, please do open a fresh report.