diff --git a/static/js/group.js b/static/js/group.js index 64f1859c7..2d0ab5b3e 100644 --- a/static/js/group.js +++ b/static/js/group.js @@ -6,7 +6,7 @@ import markdownModule from './markdown.js'; import chatRenderer from './chatRenderer.js'; import spinnerModule from './spinner.js'; import { providerLogo } from './providers.js'; -import { PROMPT_TEMPLATES, getAllPresets } from './presets.js'; +import { PROMPT_TEMPLATES, getUserTemplates } from './presets.js'; import { sortModelObjects } from './modelSort.js'; import Storage from './storage.js'; @@ -89,12 +89,16 @@ function _initGroupTab() { const charSel = document.createElement('select'); charSel.className = 'preset-input'; + // add an identifier that this is a character selection + charSel.dataset.selectionType = "character" charSel.style.cssText = 'font-size:11px;flex:1;height:26px;'; charSel.innerHTML = '' + characters.map(c => '').join(''); const modelSel = document.createElement('select'); modelSel.className = 'preset-input'; + // add an identifier that this is a model selection + modelSel.dataset.selectionType = "model" modelSel.style.cssText = 'font-size:11px;flex:1;height:26px;'; modelSel.innerHTML = '' + models.map(m => '').join(''); @@ -196,15 +200,67 @@ function _initGroupTab() { }); const groupTab = document.querySelector('.preset-tab[data-chartab="group"]'); + // whenever a user navigates to the Group tab if (groupTab) groupTab.addEventListener('click', () => { _modelsCache = null; if (startBtn) startBtn.textContent = 'Start Group'; _loadGroupPresets(); - if (_groupParticipants.length === 0) { + + const isGroupTabUnInitialized = + _groupParticipants.length === 0 && participantsEl.children.length === 0; + + if (isGroupTabUnInitialized) { setTimeout(() => addBtn.click(), 100); + } else { + // queue this asynchronously since repopulating the selection drop-downs + // do not need to be visible right away; it can be safely delayed before + // the next event loop + queueMicrotask(() => { + repopulateExistingSelections(); + }) } }); + async function repopulateExistingSelections() { + const EMPTY = ""; + + const characterSelections = participantsEl.querySelectorAll("select.preset-input[data-selection-type=character]"); + const modelSelections = participantsEl.querySelectorAll("select.preset-input[data-selection-type=model]"); + + if (characterSelections.length !== 0) { + const characters = await _getCharacterList(); + + characterSelections.forEach((characterSelection) => { + + const chosenCharacter = characterSelection.value; + const isChosenCharacterExisting = chosenCharacter !== EMPTY + && characters.findIndex((char) => char.id === chosenCharacter) !== -1; + + characterSelection.innerHTML = '' + + characters.map(c => '').join(''); + if (isChosenCharacterExisting) { + characterSelection.value = chosenCharacter; + } + }); + } + + if (modelSelections.length !== 0) { + const models = await _getModels(); + + modelSelections.forEach((modelSelection) => { + const chosenModel = modelSelection.value; + const isChosenModelExisting = chosenModel !== EMPTY + && models.findIndex((model) => model.mid === chosenModel) !== -1; + + modelSelection.innerHTML = '' + + models.map(m => '').join(''); + if (isChosenModelExisting) { + modelSelection.value = chosenModel; + } + }); + } + } + // Load and render saved group presets async function _loadGroupPresets() { try { @@ -288,17 +344,6 @@ async function _getCharacterList() { const chars = PROMPT_TEMPLATES.filter(t => t.isCharacter).map(t => ({ id: t.id, name: t.name, prompt: t.prompt, })); - // User-created characters from presets - try { - const allPresets = getAllPresets(); - if (allPresets && allPresets.custom && allPresets.custom.character_name) { - chars.push({ - id: 'custom', - name: allPresets.custom.character_name, - prompt: allPresets.custom.system_prompt || allPresets.custom.prompt || '', - }); - } - } catch (e) {} // Load user templates and wait for them before returning. // The endpoint returns a JSON array directly (not {templates:[...]}). // All user templates are personas by definition — no isCharacter filter needed. @@ -306,12 +351,26 @@ async function _getCharacterList() { const r = await fetch(API_BASE + '/api/presets/templates', { credentials: 'same-origin' }); const data = await r.json(); const templates = Array.isArray(data) ? data : (data.templates || []); + templates.forEach(t => { if (t.id && t.name && !chars.find(c => c.id === t.id)) { chars.push({ id: t.id, name: t.name, prompt: t.system_prompt || t.prompt || '' }); } }); } catch (e) {} + + // Also merge in-memory templates from presets.js — these may include + // newly created characters whose async save-to-API hasn't completed yet. + const memTemplates = getUserTemplates(); + + if (Array.isArray(memTemplates)) { + memTemplates.forEach(t => { + if (t.id && t.name && !chars.find(c => c.id === t.id)) { + chars.push({ id: t.id, name: t.name, prompt: t.system_prompt || t.prompt || '' }); + } + }); + } + return chars; } diff --git a/static/js/presets.js b/static/js/presets.js index 4922000af..bd72b7184 100644 --- a/static/js/presets.js +++ b/static/js/presets.js @@ -830,15 +830,48 @@ export async function saveCustomPreset(showToast, showError) { const _selVal = document.getElementById('char-template-select')?.value || ''; const isBuiltinPreset = PROMPT_TEMPLATES.some(t => t.isPreset && (t.name === name || t.name === _selVal)); const saveName = isBuiltinPreset ? null : (name || null); + if (saveName) { - fetch(`${API_BASE}/api/presets/templates`, { - method: 'POST', + const _existing = userTemplates.find(t => t.name === saveName); + let clone; + const _entry = { + id: _existing && _existing.id + || 'user-' + Math.random().toString(16).slice(2, 10), + name: saveName, + // use ?? since it's more semantic for null-coalescing + system_prompt: system_prompt ?? '', + temperature: config.temperature, + max_tokens: config.max_tokens, + } + const ENDPOINT = `${API_BASE}/api/presets/templates`; + + // Optimistically update the in-memory templates list by @michaelxer + if (_existing) { + // slow but works for now + clone = JSON.parse(JSON.stringify(_existing)); + + Object.assign(_existing, _entry); + } else { + userTemplates.push(_entry); + } + + fetch(ENDPOINT, { + method: "POST", headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ - id: (userTemplates.find(t => t.name === saveName) || {}).id || '', - name: saveName, system_prompt, temperature: config.temperature, max_tokens: config.max_tokens, - }), - }).then(r => { if (r.ok) loadUserTemplates(); }).catch(() => {}); + body: JSON.stringify(_entry) + }).then((r) => { + if (r.ok) { + loadUserTemplates(); + } + }).catch(() => { + if (clone) { + Object.assign(_existing, clone); + } + + if (showError) { + showError(_isInjectStart ? "Something went wrong. Saved prompt has been undone." : "Something went wrong. Saved persona has been undone."); + } + }); } if (showToast) { @@ -883,6 +916,13 @@ export function getAllPresets() { return presets; } +/** + * Get the in-memory user templates list (may be stale; call loadUserTemplates first if freshness matters). + */ +export function getUserTemplates() { + return [...userTemplates]; +} + /** * Get the character name (if set) */ @@ -1099,6 +1139,7 @@ const presetsModule = { getSelectedPreset, getPreset, getAllPresets, + getUserTemplates, getCharacterName, onSessionSwitch, isPersistentChat, diff --git a/tests/test_group_character_dropdown.py b/tests/test_group_character_dropdown.py new file mode 100644 index 000000000..df306cca5 --- /dev/null +++ b/tests/test_group_character_dropdown.py @@ -0,0 +1,84 @@ +"""Issue #3207 — newly created characters missing from Group participant dropdown. + +The fix has two parts: +1. group.js _getCharacterList() merges in-memory userTemplates from presets.js + as a fallback (covers the gap while the async templates API save is in-flight). +2. presets.js saveCustomPreset() does an optimistic in-memory update of + userTemplates immediately on success (bridges the timing race where + loadUserTemplates hasn't been triggered yet). + +These tests assert the source patterns exist so they can't be silently removed. +""" +from pathlib import Path + +GROUP_JS = Path("static/js/group.js").read_text(encoding="utf-8") +PRESETS_JS = Path("static/js/presets.js").read_text(encoding="utf-8") + + +# --- group.js: in-memory template merge in _getCharacterList --- + +def test_group_imports_getUserTemplates(): + """group.js must import getUserTemplates from presets.js.""" + assert "getUserTemplates" in GROUP_JS + assert "from './presets.js'" in GROUP_JS or 'from "./presets.js"' in GROUP_JS + + +def test_group_merges_in_memory_templates(): + """_getCharacterList must call getUserTemplates() and merge results.""" + assert "getUserTemplates()" in GROUP_JS + # The merge loop should check for duplicates by id + assert "!chars.find(c => c.id === t.id)" in GROUP_JS + + +# --- presets.js: optimistic in-memory update on save --- + +def test_presets_exports_getUserTemplates(): + """getUserTemplates must be exported from presets.js.""" + assert "export function getUserTemplates()" in PRESETS_JS + + +def test_presets_optimistic_update_on_save(): + """saveCustomPreset must update userTemplates in-memory before the async POST.""" + # Find the optimistic update block + assert "Optimistically update the in-memory templates list" in PRESETS_JS + # Must push to userTemplates for new entries + assert "userTemplates.push(_entry)" in PRESETS_JS + # Must Object.assign for existing entries + assert "Object.assign(_existing, _entry)" in PRESETS_JS + + +def test_presets_getUserTemplates_returns_array(): + """getUserTemplates should return a shallow copy of userTemplates.""" + assert "return [...userTemplates]" in PRESETS_JS + + +def test_presets_optimistic_id_not_empty(): + """Optimistic update must generate a client-side id for new characters (not empty string).""" + # The id generation uses 'user-' prefix matching server's uuid convention + assert "user-' + Math.random" in PRESETS_JS + # Must NOT use empty string as fallback (that was the bug) + assert "(_existing && _existing.id) || ''" not in PRESETS_JS + +def test_presets_clone_happens_before_mutation(): + """Rollback snapshot must be taken before Object.assign mutates _existing.""" + clone_idx = PRESETS_JS.find("clone = JSON.parse(JSON.stringify(_existing))") + assign_idx = PRESETS_JS.find("Object.assign(_existing, _entry)") + + assert clone_idx != -1 + assert assign_idx != -1 + assert clone_idx < assign_idx + +def test_presets_rollbak_restores_from_clone(): + """Failed save must restore the original object from the pre-mutation clone.""" + assert "if (clone)" in PRESETS_JS + assert "Object.assign(_existing, clone)" in PRESETS_JS + +def test_presets_clone_is_deep_copy(): + """Rollback snapshot must be a deep clone, not an alias.""" + assert "clone = JSON.parse(JSON.stringify(_existing))" in PRESETS_JS + +def test_presets_no_alias_clone(): + """Prevent accidental rollback breakage via reference assignment.""" + assert "clone = _existing" not in PRESETS_JS + assert "const clone = _existing" not in PRESETS_JS + assert "let clone = _existing" not in PRESETS_JS \ No newline at end of file