Skip to content

v0.50.277 — model-picker shared-reference fix (supersedes #1511) - #1515

Merged
nesquena-hermes merged 2 commits into
masterfrom
stage-277
May 3, 2026
Merged

v0.50.277 — model-picker shared-reference fix (supersedes #1511)#1515
nesquena-hermes merged 2 commits into
masterfrom
stage-277

Conversation

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Release v0.50.277 — model-picker shared-reference fix (supersedes PR #1511)

Single self-built fix replacing contributor PR #1511 by @lost9999. Their PR's diagnosis pointed at the wrong layer; I dug into the actual root cause from first principles.

Bug shape

When multiple "auto-detected" providers (Ollama / HuggingFace / custom OpenAI-compatible endpoints / Google Gemini CLI / Xiaomi / etc.) all fell through to the unconfigured-provider branch in api/config.py:get_models_grouped(), every group ended up sharing the SAME auto_detected_models list reference AND the SAME dicts inside.

When _deduplicate_model_ids() then mutated those dicts to add @provider_id: prefixes and provider-name parentheticals, the changes were applied to every group that referenced the same dict.

Visible symptom (vishnu via Discord, relayed to PR #1511):

Deepseek V4 Flash (Xiaomi) (Ollama) (HuggingFace) (Google-Gemini-Cli)

Hidden symptom (worse, never reported as a bug because users couldn't tell): the id field also collapsed to @xiaomi:deepseek-v4-flash (whichever provider_id won the alphabetical-first race) on every group. Selecting the model under any group silently routed the request to the WRONG provider.

Why PR #1511's fix was insufficient

The contributor's PR removed the label-suffix logic in _deduplicate_model_ids(). That would have made labels look clean (Deepseek V4 Flash instead of accumulated parentheticals) — but the IDs would still have been corrupted. The user would see clean labels and silently get their model requests routed to the wrong provider.

The actual fix

api/config.py:2078 — wrap auto_detected_models in copy.deepcopy() when assigning to a group:

                else:
                    if auto_detected_models:
+                       # Per-group deep copy so subsequent mutation by
+                       # _deduplicate_model_ids() (which prefixes ids with
+                       # @provider_id:) does not bleed into other groups
+                       # that also fall through to this branch.
                        groups.append(
                            {
                                "provider": provider_name,
                                "provider_id": pid,
-                               "models": auto_detected_models,
+                               "models": copy.deepcopy(auto_detected_models),
                            }
                        )

The existing _deduplicate_model_ids() logic is unchanged and correct. The bug was in the assignment site (each group sharing the same list/dicts), not the dedup function itself.

The single-parenthetical disambiguation in labels is retained because the composer chip at static/index.html:441 shows the model label without the optgroup header context — Deepseek V4 Flash (Ollama) is more useful there than ambiguous Deepseek V4 Flash.

First-principles investigation (verified empirically)

I read the dedup function and noticed it only ever appends ONE provider name per location it visits — it cannot accumulate four on its own. So either (a) the model dicts are shared by reference, or (b) the function is re-called, or (c) the report is exaggerated.

I wrote a Python repro and tested all three. Scenario (a) — shared references — reproduces vishnu's exact symptom. I then traced the 5 group-build paths in get_models_grouped():

Path Line Status
OpenRouter 2027-2030 new dicts via list-comp ✓
ollama-cloud 2038-2041 new dicts via list-comp ✓
_PROVIDER_MODELS / cfg 2055 explicit copy.deepcopy()
Named custom (custom:slug) 2019 each _slug has its own list ✓
Else (auto-detected fall-through) 2078 shared auto_detected_models

Only the else-branch was broken. Single-line fix.

Tests (4 new in tests/test_issue1511_dedup_shared_reference.py)

  1. test_groups_have_independent_model_lists — structural invariant: no two groups share a list or dict by identity.
  2. test_unconfigured_providers_no_shared_dedup_bleed — end-to-end against the corrected code: 4 colliding providers each get their OWN @provider_id: prefix and exactly ONE parenthetical. Negative: every label has at most ONE (.
  3. test_shared_reference_pre_fix_demonstrates_corruption — direct evidence of the broken state when references ARE shared. Documents reasoning.
  4. test_get_models_grouped_unconfigured_providers_get_independent_dictsproduction-path regression guard added in-release per Opus SHOULD-FIX. Inspects the live source via inspect.getsource() for the literal copy.deepcopy(auto_detected_models) call AND runs an end-to-end smoke of the fixed assignment loop. A future refactor that removes the deepcopy will fail this test immediately.

Full suite: 3929 passed (was 3925 → +4 new). Zero regressions.

Reviews

  • Pre-release Opus advisor: SHIP AS-IS with one SHOULD-FIX (test coverage gap). The SHOULD-FIX was absorbed in-release as test fix(api): resolve model provider from config to prevent misrouting #4 above (the production-path regression guard), per the reviewer-flagged-fix-in-release-not-followup policy (<20 LOC, clearly defensive, narrow scope). Verified all 5 group-build paths and confirmed only the else-branch was the shared-ref site.
  • Self-built: per independent-review policy, self-built PRs need either nesquena APPROVED OR Opus advisor pass. Opus pass + production-path regression test is the gate here.

Behavior change worth noting

For users running multiple unconfigured auto-detected providers (e.g. Ollama + HuggingFace + custom OpenAI-compat endpoints), this release fixes silent model-routing. Pre-fix, selecting a duplicate model under any group routed the request to the alphabetical-first provider regardless of which group the user clicked. Post-fix, requests route to the correct provider. This is a fix, not a regression — but if a user was unknowingly relying on the broken routing, the model they see selected is the model they get.

Diff

api/config.py                                  |   11 ++
tests/test_issue1511_dedup_shared_reference.py |  217 ++++++++++++++++++++
CHANGELOG.md                                   |   12 ++
ROADMAP.md                                     |    2 +-
TESTING.md                                     |    4 +-
5 files changed, 245 insertions(+), 1 deletion(-)

Branches & cleanup

Hermes Bot and others added 2 commits May 3, 2026 06:41
…dup bleed-across (#1511 root cause)

Supersedes contributor PR #1511 (lost9999), which removed the label-suffix
logic in _deduplicate_model_ids() but left the underlying shared-reference
bug intact — IDs would still be silently corrupted across provider groups,
just with cleaner-looking labels.

## Bug shape

When multiple unconfigured providers (Ollama / HuggingFace / custom
endpoints / Google Gemini CLI / Xiaomi / etc.) all fell through to the
'else' branch in api/config.py:get_models_grouped() that ends with:

    groups.append({..., "models": auto_detected_models})

every group ended up sharing the SAME list reference AND the SAME dicts
inside. When _deduplicate_model_ids() then mutated those dicts to add
@provider_id: prefixes and provider-name parentheticals, the changes were
applied to every group that referenced the same dict.

Visible symptom: user 'vishnu' reported the dropdown showing
'Deepseek V4 Flash (Xiaomi) (Ollama) (HuggingFace) (Google-Gemini-Cli)'
on every group. Hidden symptom (worse): the 'id' field collapsed to
'@XiaoMi:deepseek-v4-flash' on every group too, so clicking the entry
under any group routed the request to Xiaomi.

## Fix

api/config.py:2078 — wrap auto_detected_models in copy.deepcopy() at the
groups.append site so each group gets its own independent dicts. The
existing _deduplicate_model_ids() logic is correct and unchanged; the
bug was in the assignment site, not the dedup function.

The single-parenthetical disambiguation in labels is retained because
the composer chip (composer-model-label) shows the model label without
the optgroup header context — 'Deepseek V4 Flash (Ollama)' is more
useful than ambiguous 'Deepseek V4 Flash' there.

## Tests

tests/test_issue1511_dedup_shared_reference.py — 3 new tests:
- test_groups_have_independent_model_lists: structural invariant pin
- test_unconfigured_providers_no_shared_dedup_bleed: end-to-end against
  the corrected code path; verifies each group gets its own @provider_id:
  prefix and exactly ONE provider parenthetical per disambiguated label
- test_shared_reference_pre_fix_demonstrates_corruption: documents the
  broken state that motivated the fix

Full suite: 3925 → 3928 passing (+3 new, 0 regressions).

Co-authored-by: lost9999 <56498264+lost9999@users.noreply.github.com>
…n guard)

CHANGELOG, ROADMAP, TESTING bumped (3925 → 3929 tests collected).

Opus SHOULD-FIX absorbed in-release: tests #1-3 documented the dedup
contract via direct construction but did not invoke get_models_grouped().
Test #4 (test_get_models_grouped_unconfigured_providers_get_independent_dicts)
inspects the live source for the literal copy.deepcopy(auto_detected_models)
call AND runs an end-to-end smoke of the fixed assignment loop.

A future refactor that removes the deepcopy at api/config.py:2078 will
fail this test immediately.
@nesquena-hermes
nesquena-hermes merged commit 7921a47 into master May 3, 2026
3 checks passed
@nesquena-hermes
nesquena-hermes deleted the stage-277 branch May 3, 2026 06:51
SysAdminDoc pushed a commit to SysAdminDoc/hermes-webui that referenced this pull request Jun 26, 2026
v0.50.277 — model-picker shared-reference fix (supersedes nesquena#1511)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant