* fix(settings): persist provider-native auxiliary model IDs Co-authored-by: lowlandsheperd <lowlandsheperd@users.noreply.github.com> * docs(changelog): stamp #7261 provider-native aux model IDs --------- Co-authored-by: n <a@n> Co-authored-by: lowlandsheperd <lowlandsheperd@users.noreply.github.com>
This commit is contained in:
5 files changed
+319
-20
No files matched your search
@@ -7,6 +7,8 @@
|
||||
|
||||
- **The workspace switcher now lists workspaces in your configured order instead of re-sorting them alphabetically.** The chat-header workspace dropdown alphabetized its entries client-side, so it disagreed with the drag-and-drop order shown in the Workspaces settings panel (and never kept the default "Home" workspace first). The dropdown now renders the server's stored order verbatim — the same order as the settings view — with no client-side re-sort. Thanks @CharlesMcquade. (#7317)
|
||||
|
||||
- **Auxiliary-model settings now persist provider-native model IDs instead of silently dropping the provider prefix.** When an auxiliary model (title/summary/etc.) was chosen as a provider-qualified ID, settings canonicalization stripped the `@provider:` prefix and could persist a bare name that resolved under the wrong provider group. The picker now preserves the provider-native ID, dedupes qualified/unqualified variants, and fails closed on an ambiguous mismatch rather than persisting a wrong upstream model. Thanks @lowlandsheperd. (#7261)
|
||||
|
||||
- **Hardened a streaming race that could read a stream queue mid-teardown.** Several code paths read the in-memory SSE stream registry with a bare, unlocked `STREAMS.get()`, so a read that raced a concurrent teardown (cancel/finish popping the queue under `STREAMS_LOCK`) could observe-and-use a queue the registry had already released. Registry reads now go through a lock-disciplined `peek_stream()` helper that takes `STREAMS_LOCK`, and a self-enforcing test forbids reintroducing a bare read. Callers keep their existing empty/None-guard fallbacks, so behavior on the common non-race path is unchanged. Thanks @zicochaos. (#7339)
|
||||
|
||||
- **Automatic session titles no longer keep an internal placeholder for workspace-prefixed multimodal first turns, and internal scaffolding no longer leaks into generated titles.** Title eligibility compared persisted content that still carried WebUI workspace/attachment scaffolding against a sanitized visible title, so a workspace-prefixed structured multimodal first turn could never be recognized as provisional and stayed stuck on its placeholder title; the same internal scaffolding could also reach the title provider's input. Title generation now sanitizes title-facing structured text (stripping versioned workspace sentinels and generated attachment suffixes) across eligibility, first-exchange generation, manual prefer-latest generation, and adaptive refresh, while preserving genuine user content and native image parts. Persisted and model-facing message content is unchanged (titles only read it), and a real user-chosen title is never misclassified as provisional. Thanks @starship-s. (#7306)
|
||||
|
||||
+30
-2
@@ -5013,6 +5013,31 @@ def _coerce_optional_positive_int(value, field: str):
|
||||
return number
|
||||
|
||||
|
||||
def _provider_native_auxiliary_model(provider: str, model: str) -> str:
|
||||
"""Return the provider-native model stored by an auxiliary slot.
|
||||
|
||||
``@provider:model`` is a WebUI picker routing token. Auxiliary slots already
|
||||
store the selected provider separately, so only an exact matching prefix is
|
||||
safe to remove. Reject other qualified forms instead of persisting an
|
||||
ambiguous upstream model name.
|
||||
"""
|
||||
provider_id = str(provider or "").strip() or "auto"
|
||||
model_id = str(model or "").strip()
|
||||
if not model_id.startswith("@") or ":" not in model_id:
|
||||
return model_id
|
||||
|
||||
matching_prefix = f"@{provider_id}:"
|
||||
if provider_id != "auto" and model_id.startswith(matching_prefix):
|
||||
native_model = model_id[len(matching_prefix) :]
|
||||
if native_model:
|
||||
return native_model
|
||||
|
||||
raise ValueError(
|
||||
"provider-qualified auxiliary model must match the selected provider "
|
||||
"and include a model name"
|
||||
)
|
||||
|
||||
|
||||
def set_auxiliary_model(task: str, provider: str, model: str, advanced: dict | None = None) -> dict:
|
||||
"""Persist an auxiliary model assignment in config.yaml.
|
||||
|
||||
@@ -5021,6 +5046,8 @@ def set_auxiliary_model(task: str, provider: str, model: str, advanced: dict | N
|
||||
Sensitive api_key values are write-only: get_auxiliary_models() only reports
|
||||
whether one is set.
|
||||
"""
|
||||
provider = str(provider or "").strip() or "auto"
|
||||
model = str(model or "").strip()
|
||||
config_path = _get_config_path()
|
||||
with _cfg_lock:
|
||||
config_data = _load_yaml_config_file(config_path)
|
||||
@@ -5046,11 +5073,12 @@ def set_auxiliary_model(task: str, provider: str, model: str, advanced: dict | N
|
||||
aux_cfg = config_data.get("auxiliary", {})
|
||||
if not isinstance(aux_cfg, dict):
|
||||
aux_cfg = {}
|
||||
model = _provider_native_auxiliary_model(provider, model)
|
||||
slot_cfg = aux_cfg.get(task, {})
|
||||
if not isinstance(slot_cfg, dict):
|
||||
slot_cfg = {}
|
||||
slot_cfg["provider"] = provider or "auto"
|
||||
slot_cfg["model"] = model or ""
|
||||
slot_cfg["provider"] = provider
|
||||
slot_cfg["model"] = model
|
||||
if provider and (provider.startswith("custom:") or provider == "custom"):
|
||||
# Resolve the auxiliary slot's base_url against the SELECTED
|
||||
# provider, not the active main provider. A bare
|
||||
|
||||
+33
-17
@@ -1889,17 +1889,19 @@ function cancelCronForm(){
|
||||
_clearCronDetail();
|
||||
}
|
||||
|
||||
function _cronModelBareName(model, provider) {
|
||||
function _modelBareNameForProvider(model, provider) {
|
||||
// Strip @provider: prefix from a model value when provider is stored separately.
|
||||
// The model dropdown may contain values like "@custom:9router:chat" (from
|
||||
// _apply_provider_prefix) but cron jobs store model and provider separately,
|
||||
// so the model should be just "chat".
|
||||
if (model && provider && model.startsWith('@' + provider + ':')) {
|
||||
return model.slice(('@' + provider + ':').length);
|
||||
}
|
||||
return model;
|
||||
}
|
||||
|
||||
function _cronModelBareName(model, provider) {
|
||||
// Cron jobs store model and provider separately, just like auxiliary slots.
|
||||
return _modelBareNameForProvider(model, provider);
|
||||
}
|
||||
|
||||
async function saveCronForm(){
|
||||
const nameEl=$('cronFormName');
|
||||
const schEl=$('cronFormSchedule');
|
||||
@@ -12326,17 +12328,25 @@ function _buildAuxModelOptions(sel,provider,providers,currentModel){
|
||||
const emptyOpt=document.createElement('option');
|
||||
emptyOpt.value='';emptyOpt.textContent=t('settings_aux_model_auto')||'auto (use provider default)';
|
||||
sel.appendChild(emptyOpt);
|
||||
const canonicalCurrent=_modelBareNameForProvider(currentModel,provider)||'';
|
||||
if(!provider||provider==='auto'){
|
||||
sel.value=currentModel||'';
|
||||
return;
|
||||
sel.value=canonicalCurrent;
|
||||
return canonicalCurrent;
|
||||
}
|
||||
// Find matching provider in cached list
|
||||
const pData=providers.find(p=>p.slug===provider);
|
||||
const modelValues=new Set();
|
||||
if(pData&&pData.models){
|
||||
for(const mId of pData.models){
|
||||
for(const modelEntry of pData.models){
|
||||
const routeId=typeof modelEntry==='string'?modelEntry:String(modelEntry?.id||'');
|
||||
const mId=_modelBareNameForProvider(routeId,provider)||'';
|
||||
if(!mId||modelValues.has(mId)) continue;
|
||||
modelValues.add(mId);
|
||||
const routeLabel=typeof modelEntry==='string'?'':String(modelEntry?.label||'');
|
||||
const modelLabel=_modelBareNameForProvider(routeLabel,provider)||mId;
|
||||
const opt=document.createElement('option');
|
||||
opt.value=mId;opt.textContent=mId;
|
||||
if(mId===currentModel) opt.selected=true;
|
||||
opt.value=mId;opt.textContent=modelLabel;
|
||||
if(mId===canonicalCurrent) opt.selected=true;
|
||||
sel.appendChild(opt);
|
||||
}
|
||||
}
|
||||
@@ -12345,12 +12355,13 @@ function _buildAuxModelOptions(sel,provider,providers,currentModel){
|
||||
customOpt.value='__custom__';customOpt.textContent=t('settings_aux_model_custom')||'Custom model…';
|
||||
sel.appendChild(customOpt);
|
||||
// If currentModel not in list and not empty, add it as a custom option
|
||||
if(currentModel&&!pData?.models?.includes(currentModel)){
|
||||
if(canonicalCurrent&&!modelValues.has(canonicalCurrent)){
|
||||
const existingOpt=document.createElement('option');
|
||||
existingOpt.value=currentModel;existingOpt.textContent=currentModel+' (configured)';
|
||||
existingOpt.value=canonicalCurrent;existingOpt.textContent=canonicalCurrent+' (configured)';
|
||||
existingOpt.selected=true;
|
||||
sel.insertBefore(existingOpt,customOpt);
|
||||
}
|
||||
return canonicalCurrent;
|
||||
}
|
||||
|
||||
function _onAuxProviderChange(taskKey,providers){
|
||||
@@ -12368,13 +12379,16 @@ async function _onAuxModelChange(taskKey){
|
||||
if(modelSel.value==='__custom__'){
|
||||
const customModel=await showPromptDialog({title:t('settings_aux_model_custom')||'Custom model',message:t('settings_aux_model_custom_prompt')||'Enter model ID:',placeholder:'model/provider:model-id',confirmLabel:t('settings_btn_apply_aux_models')||'Apply'});
|
||||
if(customModel&&customModel.trim()){
|
||||
const provider=$('aux-prov-'+taskKey)?.value||'';
|
||||
const enteredModel=customModel.trim();
|
||||
const canonicalModel=_modelBareNameForProvider(enteredModel,provider)||enteredModel;
|
||||
// Insert custom model option before the __custom__ option
|
||||
const opt=document.createElement('option');
|
||||
opt.value=customModel.trim();opt.textContent=customModel.trim();
|
||||
opt.value=canonicalModel;opt.textContent=canonicalModel;
|
||||
// Remove __custom__ selection
|
||||
const customIdx=[...modelSel.options].findIndex(o=>o.value==='__custom__');
|
||||
if(customIdx>=0) modelSel.insertBefore(opt,modelSel.options[customIdx]);
|
||||
modelSel.value=customModel.trim();
|
||||
modelSel.value=canonicalModel;
|
||||
}else{
|
||||
modelSel.value='';
|
||||
}
|
||||
@@ -12581,7 +12595,7 @@ async function _loadAuxiliaryModels(){
|
||||
_auxProviders=groups.filter(g=>g.provider&&((g.models&&g.models.length>0)||(g.extra_models&&g.extra_models.length>0))).map(g=>({
|
||||
slug:g.provider_id||g.provider,
|
||||
name:g.provider,
|
||||
models:[...(g.models||[]),...(g.extra_models||[])].map(m=>m.id),
|
||||
models:[...(g.models||[]),...(g.extra_models||[])].map(m=>({id:m.id,label:m.label||m.id})),
|
||||
}));
|
||||
if(auxData&&Object.prototype.hasOwnProperty.call(auxData,'main')){
|
||||
_mainAdvancedConfig=auxData.main||{};
|
||||
@@ -12596,6 +12610,7 @@ async function _loadAuxiliaryModels(){
|
||||
_auxOriginalConfig=JSON.parse(JSON.stringify(taskMap));
|
||||
|
||||
container.innerHTML='';
|
||||
let needsCanonicalSave=false;
|
||||
for(const task of _auxTasks){
|
||||
const cfg=taskMap[task.task]||{provider:'auto',model:''};
|
||||
const row=document.createElement('div');
|
||||
@@ -12619,7 +12634,8 @@ async function _loadAuxiliaryModels(){
|
||||
const modelSel=document.createElement('select');
|
||||
modelSel.id='aux-model-'+task.task;
|
||||
modelSel.style.cssText=_auxSelectStyle();
|
||||
_buildAuxModelOptions(modelSel,cfg.provider,_auxProviders,cfg.model);
|
||||
const canonicalModel=_buildAuxModelOptions(modelSel,cfg.provider,_auxProviders,cfg.model);
|
||||
if(canonicalModel!==cfg.model) needsCanonicalSave=true;
|
||||
modelSel.addEventListener('change',()=>_onAuxModelChange(task.task));
|
||||
row.appendChild(modelSel);
|
||||
|
||||
@@ -12636,9 +12652,9 @@ async function _loadAuxiliaryModels(){
|
||||
|
||||
container.appendChild(row);
|
||||
}
|
||||
// Hide apply button (no changes yet)
|
||||
// Matching legacy @provider:model values can be repaired with one explicit Apply.
|
||||
const applyBtn=$('btnApplyAuxModels');
|
||||
if(applyBtn) applyBtn.style.display='none';
|
||||
if(applyBtn) applyBtn.style.display=needsCanonicalSave?'':'none';
|
||||
|
||||
// Reset button
|
||||
const resetBtn=$('btnResetAuxModels');
|
||||
|
||||
@@ -0,0 +1,253 @@
|
||||
"""Regression coverage for provider-qualified auxiliary model persistence.
|
||||
|
||||
``GET /api/models`` may expose WebUI-only ``@provider:model`` routing IDs, but
|
||||
auxiliary configuration stores provider and model in separate fields. The
|
||||
provider-native model value must be used by both the settings UI and the shared
|
||||
backend persistence boundary.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import shutil
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
REPO = Path(__file__).resolve().parents[1]
|
||||
PANELS_JS_PATH = REPO / "static" / "panels.js"
|
||||
NODE = shutil.which("node")
|
||||
|
||||
|
||||
@pytest.mark.skipif(NODE is None, reason="node not available")
|
||||
def test_auxiliary_picker_uses_provider_native_model_values():
|
||||
"""A custom-provider catalog must render and select provider-native values."""
|
||||
script = r"""
|
||||
const fs = require('fs');
|
||||
const src = fs.readFileSync(process.argv[1], 'utf8');
|
||||
|
||||
function extract(name){
|
||||
const re = new RegExp('function\\s+' + name + '\\s*\\(');
|
||||
const start = src.search(re);
|
||||
if(start < 0) return '';
|
||||
let i = src.indexOf('{', start);
|
||||
let depth = 0;
|
||||
while(i < src.length){
|
||||
const ch = src[i];
|
||||
if(ch === '{') depth += 1;
|
||||
else if(ch === '}'){
|
||||
depth -= 1;
|
||||
if(depth === 0) return src.slice(start, i + 1);
|
||||
}
|
||||
i += 1;
|
||||
}
|
||||
throw new Error(name + ' parse failed');
|
||||
}
|
||||
|
||||
global.t = () => '';
|
||||
global.document = {
|
||||
createElement: () => ({value:'', textContent:'', selected:false}),
|
||||
};
|
||||
|
||||
function makeSelect(){
|
||||
return {
|
||||
children: [],
|
||||
_value: '',
|
||||
set innerHTML(_value){ this.children = []; this._value = ''; },
|
||||
get innerHTML(){ return ''; },
|
||||
get options(){ return this.children; },
|
||||
appendChild(opt){ this.children.push(opt); },
|
||||
insertBefore(opt, before){
|
||||
const idx = this.children.indexOf(before);
|
||||
if(idx < 0) this.children.push(opt);
|
||||
else this.children.splice(idx, 0, opt);
|
||||
},
|
||||
set value(value){
|
||||
this._value = value;
|
||||
for(const opt of this.children) opt.selected = opt.value === value;
|
||||
},
|
||||
get value(){
|
||||
const selected = this.children.find((opt) => opt.selected);
|
||||
return selected ? selected.value : this._value;
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
const sharedHelper = extract('_modelBareNameForProvider');
|
||||
const buildOptions = extract('_buildAuxModelOptions');
|
||||
if(!buildOptions) throw new Error('_buildAuxModelOptions not found');
|
||||
eval(sharedHelper + '\n' + buildOptions);
|
||||
|
||||
const select = makeSelect();
|
||||
const providers = [{
|
||||
slug: 'my-local-ai-gateway',
|
||||
name: 'My Local AI Gateway',
|
||||
models: [
|
||||
{
|
||||
id: '@my-local-ai-gateway:example-chat-model',
|
||||
label: 'Example Chat Model',
|
||||
},
|
||||
{
|
||||
id: '@my-local-ai-gateway:example-side-model',
|
||||
label: '@my-local-ai-gateway:example-side-model',
|
||||
},
|
||||
{id: 'vendor/example-model', label: 'Vendor Example Model'},
|
||||
{id: 'example-chat-model', label: 'Duplicate Chat Model'},
|
||||
],
|
||||
}];
|
||||
|
||||
const canonicalCurrent = _buildAuxModelOptions(
|
||||
select,
|
||||
'my-local-ai-gateway',
|
||||
providers,
|
||||
'@my-local-ai-gateway:example-side-model',
|
||||
);
|
||||
|
||||
const modelOptions = select.options.filter(
|
||||
(opt) => opt.value && opt.value !== '__custom__',
|
||||
);
|
||||
const helperCases = sharedHelper ? [
|
||||
_modelBareNameForProvider('@custom:router-alias:chat-model', 'custom'),
|
||||
_modelBareNameForProvider('@custom:backup:model:free', 'custom:backup'),
|
||||
_modelBareNameForProvider('vendor/example-model', 'my-local-ai-gateway'),
|
||||
_modelBareNameForProvider('@other-gateway:other-model', 'my-local-ai-gateway'),
|
||||
] : null;
|
||||
|
||||
console.log(JSON.stringify({
|
||||
values: modelOptions.map((opt) => opt.value),
|
||||
labels: modelOptions.map((opt) => opt.textContent),
|
||||
selected: select.value,
|
||||
canonicalCurrent,
|
||||
helperCases,
|
||||
}));
|
||||
"""
|
||||
|
||||
proc = subprocess.run(
|
||||
[NODE, "-e", script, str(PANELS_JS_PATH)],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=20,
|
||||
)
|
||||
assert proc.returncode == 0, f"node probe failed:\n{proc.stderr}"
|
||||
result = json.loads(proc.stdout.strip().splitlines()[-1])
|
||||
|
||||
assert result == {
|
||||
"values": [
|
||||
"example-chat-model",
|
||||
"example-side-model",
|
||||
"vendor/example-model",
|
||||
],
|
||||
"labels": [
|
||||
"Example Chat Model",
|
||||
"example-side-model",
|
||||
"Vendor Example Model",
|
||||
],
|
||||
"selected": "example-side-model",
|
||||
"canonicalCurrent": "example-side-model",
|
||||
"helperCases": [
|
||||
"router-alias:chat-model",
|
||||
"model:free",
|
||||
"vendor/example-model",
|
||||
"@other-gateway:other-model",
|
||||
],
|
||||
}
|
||||
|
||||
|
||||
def test_matching_legacy_value_exposes_explicit_apply_repair():
|
||||
"""Loading a safe legacy prefix should offer, but not force, a config write."""
|
||||
source = PANELS_JS_PATH.read_text(encoding="utf-8")
|
||||
|
||||
assert "if(canonicalModel!==cfg.model) needsCanonicalSave=true;" in source
|
||||
assert "applyBtn.style.display=needsCanonicalSave?'':'none'" in source
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("provider", "requested_model", "persisted_model"),
|
||||
[
|
||||
(
|
||||
"my-local-ai-gateway",
|
||||
"@my-local-ai-gateway:example-side-model",
|
||||
"example-side-model",
|
||||
),
|
||||
(
|
||||
"custom",
|
||||
"@custom:router-alias:chat-model",
|
||||
"router-alias:chat-model",
|
||||
),
|
||||
("custom:backup", "@custom:backup:model:free", "model:free"),
|
||||
(
|
||||
"my-local-ai-gateway",
|
||||
"vendor/example-model",
|
||||
"vendor/example-model",
|
||||
),
|
||||
(
|
||||
"my-local-ai-gateway",
|
||||
"example-side-model",
|
||||
"example-side-model",
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_set_auxiliary_model_persists_provider_native_model(
|
||||
monkeypatch,
|
||||
tmp_path,
|
||||
provider,
|
||||
requested_model,
|
||||
persisted_model,
|
||||
):
|
||||
from api import config
|
||||
|
||||
config_path = tmp_path / "config.yaml"
|
||||
config_path.write_text(
|
||||
"auxiliary:\n vision:\n provider: auto\n model: ''\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
monkeypatch.setattr(config, "_get_config_path", lambda: config_path)
|
||||
monkeypatch.setattr(config, "reload_config", lambda: None)
|
||||
monkeypatch.setattr(
|
||||
config,
|
||||
"resolve_model_provider",
|
||||
lambda model: (model, provider, None),
|
||||
)
|
||||
|
||||
result = config.set_auxiliary_model("vision", provider, requested_model)
|
||||
|
||||
saved = config._load_yaml_config_file(config_path)["auxiliary"]["vision"]
|
||||
assert saved["provider"] == provider
|
||||
assert saved["model"] == persisted_model
|
||||
assert result["provider"] == provider
|
||||
assert result["model"] == persisted_model
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("provider", "model"),
|
||||
[
|
||||
("my-local-ai-gateway", "@other-gateway:other-model"),
|
||||
("my-local-ai-gateway", "@my-local-ai-gateway:"),
|
||||
("auto", "@my-local-ai-gateway:example-side-model"),
|
||||
],
|
||||
)
|
||||
def test_set_auxiliary_model_rejects_invalid_qualified_pair_without_write(
|
||||
monkeypatch,
|
||||
tmp_path,
|
||||
provider,
|
||||
model,
|
||||
):
|
||||
from api import config
|
||||
|
||||
config_path = tmp_path / "config.yaml"
|
||||
original = (
|
||||
"auxiliary:\n"
|
||||
" vision:\n"
|
||||
" provider: openai\n"
|
||||
" model: gpt-5.5\n"
|
||||
)
|
||||
config_path.write_text(original, encoding="utf-8")
|
||||
monkeypatch.setattr(config, "_get_config_path", lambda: config_path)
|
||||
monkeypatch.setattr(config, "reload_config", lambda: None)
|
||||
|
||||
with pytest.raises(ValueError, match="provider-qualified auxiliary model"):
|
||||
config.set_auxiliary_model("vision", provider, model)
|
||||
|
||||
assert config_path.read_text(encoding="utf-8") == original
|
||||
@@ -39,7 +39,7 @@ def _fn_body(name: str) -> str:
|
||||
|
||||
|
||||
def _strip(model, provider):
|
||||
body = _fn_body("_cronModelBareName")
|
||||
body = _fn_body("_modelBareNameForProvider") + "\n" + _fn_body("_cronModelBareName")
|
||||
script = (
|
||||
body
|
||||
+ "\nconst args = "
|
||||
|
||||
Reference in new issue
Block a user