fix: include in-memory templates in group participant character list

[michaelxer]

Summary

When a character is created in the Character tab, the save to the templates API is async (fire-and-forget). If the user switches to the Group tab before that request completes, _getCharacterList() fetches from the API and misses the newly created character. The getAllPresets().custom path always shows the current custom character, which is why the "chosen character" appears consistently — but other saved templates can be missing.

Fix: export getUserTemplates() from presets.js (returns the in-memory userTemplates array) and merge it as a fallback in _getCharacterList(). The in-memory array is updated as soon as any async template save completes (via the loadUserTemplates callback), bridging the timing gap between character creation and API persistence. The existing dedup check (!chars.find(c => c.id === t.id)) prevents duplicates.

Linked Issue

Fixes #3207

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update

Checklist

• I searched existing issues and PRs and found no duplicates.
• I have added tests that prove my fix is effective or that my feature works
• I have updated the documentation accordingly

How to Test

1. Open Odysseus, go to the Character tab
2. Create a new character with a name and system prompt
3. Immediately switch to the Group tab (no page refresh)
4. Click "Add participant" — the newly created character should appear in the dropdown
5. Create a second character, repeat — both should appear
6. Refresh the page and verify all characters still appear in the Group participant selector

Notes

• Follows up on fix(group): show all user-created personas in the participant selector #1770 which fixed the template API read (data.templates → Array.isArray(data) ? data : ...). This addresses the timing gap between async save and fetch.
• No visual changes — only the data source for the dropdown is affected.

[hinode-codes]

I think adding a test would be nice here to not reintroduce the bug in the future, though I don't know how.

Edit: I have tested this PR and it seems that it's not fixed yet.

Edit 2: Would it be possible to extend upon this PR (Like I work on it)? Let me know if I can start. I'll be posting my findings once I look through the code later (around 8h).

[michaelxer]

Thanks for testing this — you're right, the current fix doesn't fully resolve it. I looked more closely and the getUserTemplates() fallback pulls from the same data source as the API fetch, so it doesn't actually add missing characters.

The real issue seems to be timing: when a new character is saved via saveCustomPreset(), the API POST + loadUserTemplates() refresh happens asynchronously, but _getCharacterList() may be called before that completes. The fix likely needs to either:

1. Call loadUserTemplates() at the start of _getCharacterList() to ensure fresh data, or
2. Add an optimistic local push when saving a character so it's immediately available in-memory

You're very welcome to build on this or take it over entirely — I'd be happy to close this PR if you'd like to submit a better fix. Just let me know how you'd like to proceed.

[michaelxer]

Pushed a fix for the timing issue. The problem was that saveCustomPreset() fires the templates API save as a non-blocking fetch().then() — the in-memory userTemplates array only gets refreshed after that async POST succeeds. If _getCharacterList() runs before that completes, getUserTemplates() returns stale data and the new character is still missing.

The new commit updates userTemplates in-memory immediately when saveCustomPreset() succeeds (optimistic update), before the async templates POST starts. This means getUserTemplates() always reflects the latest saved character, even if the server roundtrip hasn't finished yet.

All 4 CI checks pass on the updated branch.

[michaelxer]

Good call — I have pushed a fix for the timing race you found (the saveCustomPreset API call is async, so getUserTemplates could return stale data before it completes). The new commit does an optimistic in-memory update immediately.

For the test, I can add one that verifies the in-memory userTemplates array is updated right after saveCustomPreset succeeds, before any API round-trip. Will push that shortly.

[michaelxer]

Added 5 source-level tests verifying the fix:

• test_group_imports_getUserTemplates — group.js imports getUserTemplates from presets.js
• test_group_merges_in_memory_templates — _getCharacterList calls getUserTemplates() and deduplicates by id
• test_presets_exports_getUserTemplates — getUserTemplates is exported
• test_presets_optimistic_update_on_save — saveCustomPreset updates userTemplates in-memory before the async POST
• test_presets_getUserTemplates_returns_array — getUserTemplates returns the array directly

These follow the repo's convention of asserting source patterns for DOM-coupled JS modules (same approach as test_notes_select_esc_listener_js.py). They'll catch any accidental regression of the in-memory merge or optimistic update.

[hinode-codes]

Hello! I apologize for the very late response. My work was longer than intended since a bug came up. I'll review the changes tomorrow, first thing in the morning, and then look at the source code, if it still doesn't work. Thank you!

[hinode-codes]

Hi! I did a quick tour of the changes and the code base before resting ^__^. Good job! I can see that it does fix the main issue of showing the recently added character.

However, the newly added character only successfully shows up on the "latest" select, that is, the newly-created selection after the creation of the character. Other selection drop-downs are not populated.

Another problem is deletion, which I will document later.

I will be extending this PR, first thing in the morning, with the fix. Thank you!

Edit: Another thing, which I'm sure isn't caused by the changes here, is that the UI changed, a bit, for the worse:

Should I open an issue for this?

[vdmkenny]

Good root-cause fix, merging in-memory userTemplates into _getCharacterList() is the right call, and the dedup-by-id is correct. js + tests pass. Two things to fix before merge:

1. Make the optimistic update actually effective (it is currently a no-op for new characters). In saveCustomPreset the optimistic entry uses id: (_existing && _existing.id) || "", so a brand-new character gets id: "". But your merge guard in _getCharacterList is if (t.id && t.name && ...), which filters empty ids, and "" is not a usable <option> value anyway. So the exact case this PR targets (a just-created character) still will not appear until the POST round-trips and loadUserTemplates runs. The "sees the new character immediately, before the async POST completes" behavior does not actually happen yet.

The fix is clean because the server keeps a client-supplied id (POST /api/presets/templates only assigns one when id is empty): generate the id client-side using the existing user-<hex> convention, set it on the optimistic userTemplates entry, and include it in the POST body. Then the optimistic entry has a real id, passes the merge guard, and the dropdown shows the character immediately as intended, with no risk of server/client id mismatch.

2. Do not return the live array by reference. getUserTemplates() returns userTemplates directly, so any caller can mutate module state. Return a shallow copy (return [...userTemplates]). _getCharacterList only reads it today, but the export is now public API.

With those two done this is good to merge.

[michaelxer]

Both items addressed:

1. Client-side id for optimistic update: New characters now get a 'user-' + hex id immediately on save, matching the server's uuid.uuid4().hex[:8] convention. The merge guard in _getCharacterList (if (t.id && t.name && ...)) now passes for newly created entries, so the dropdown shows the character before the async POST round-trips.

2. Shallow copy from getUserTemplates(): Returns [...userTemplates] instead of the live array.

6 tests pass (added test_presets_optimistic_id_not_empty for the id generation).

[hinode-codes]

Hello! I'm currently working on the fix for repopulating the other existing selection drop-downs.

[hinode-codes]

Hello again! I have looked at the code base and finished the fixes for:

• repopulation
• continuously adding a selection row everytime the group tab is revisited, which is inconsistent with the intended behavior because of the check _groupParticipants.length === 0 where it will not generate the row if a group has been formed. Therefore, I assumed it's intended to generate a default single row for when there is no group formed yet.

I will be documenting and uploading a video in this comment in a bit.

Edit: While documenting, I found a bug in the state of the data. The recently added character is stored twice. It's what is being received from _getCharacterList().

Edit: I added the documentation below. I will be creating a PR to this specific PR branch. Let me know what you guys think.

A video of what the interaction with the group tab now looks like.

2026-06-08_17-05-36.mp4

Changes

I changed the groupTab click listener logic. Below are the before and after comparisons.

Initialization Logic of Selection Drop-downs

This change is made to not recreate rows continuously if there is no group formed yet, which is not, I believe is the intended behavior, and somewhat defeats the purpose of the Add participant button.

Before:

const groupTab = document.querySelector('.preset-tab[data-chartab="group"]');
if (groupTab) groupTab.addEventListener('click', () => {
  _modelsCache = null;
  if (startBtn) startBtn.textContent = 'Start Group';
  _loadGroupPresets();
  if (_groupParticipants.length === 0) {
    setTimeout(() => addBtn.click(), 100);
  }
});

After:

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();

  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();
    })
  }
});

Repopulating Existing Selection Drop-downs

This change is necessary to reflect the latest changes of the added characters. An added bonus is that the model selection is also updated. The change is an added function, called from when the group tab button is clicked and it has already been initialized.

Before:

It's a new function, so it didn't exist yet.

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 = '<option value="">Empty...</option>' +
        characters.map(c => '<option value="' + c.id + '">' + uiModule.esc(c.name) + '</option>').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 = '<option value="">Model…</option>' +
        models.map(m => '<option value="' + m.mid + '">' + uiModule.esc(m.display) + '</option>').join('');
      if (isChosenModelExisting) {
        modelSelection.vale = chosenModel;
      }
    });
  }
}

Add Identifier to Selection Drop-downs

To support the change above and have a more consistent way to target the selection drop-downs, an identifier is added using data- attributes.

Before:

const charSel = document.createElement('select');
charSel.className = 'preset-input';
charSel.style.cssText = 'font-size:11px;flex:1;height:26px;';
charSel.innerHTML = '<option value="">Empty...</option>' +
  characters.map(c => '<option value="' + c.id + '">' + uiModule.esc(c.name) + '</option>').join('');

const modelSel = document.createElement('select');
modelSel.className = 'preset-input';
modelSel.style.cssText = 'font-size:11px;flex:1;height:26px;';
modelSel.innerHTML = '<option value="">Model…</option>' +
  models.map(m => '<option value="' + m.mid + '">' + uiModule.esc(m.display) + '</option>').join('');

After:

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 = '<option value="">Empty...</option>' +
  characters.map(c => '<option value="' + c.id + '">' + uiModule.esc(c.name) + '</option>').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 = '<option value="">Model…</option>' +
  models.map(m => '<option value="' + m.mid + '">' + uiModule.esc(m.display) + '</option>').join('');

[vdmkenny]

@hinode-codes this is excellent investigation, thank you. The three issues you found are all real: existing dropdowns not being repopulated (only the newly-added row updates), the row re-creation on every Group-tab revisit, and the double-store of the just-added character in _getCharacterList().

One request on logistics: rather than a PR into this branch (michaelxer's fork), please open a standalone PR against dev with your fix. That lets us compare the two approaches side by side and pick the cleanest one, instead of stacking onto a branch that may itself change. Reference #3207, and note this PR as related so the history is clear.

Context that may help: #3234 was an earlier attempt at the same repopulation idea (tab-focus refresh of existing dropdowns); it was closed as superseded by #3217, but your repopulateExistingSelections() is closer to what that needed. The double-store is the most important to nail down, since it is a data-correctness bug, not just a UI refresh gap.

@michaelxer heads-up: depending on how hinode's PR shapes up, we may land theirs instead of or on top of yours for #3207. Will compare once it is up.

[hinode-codes]

@hinode-codes this is excellent investigation, thank you. The three issues you found are all real: existing dropdowns not being repopulated (only the newly-added row updates), the row re-creation on every Group-tab revisit, and the double-store of the just-added character in _getCharacterList().

One request on logistics: rather than a PR into this branch (michaelxer's fork), please open a standalone PR against dev with your fix. That lets us compare the two approaches side by side and pick the cleanest one, instead of stacking onto a branch that may itself change. Reference #3207, and note this PR as related so the history is clear.

Context that may help: #3234 was an earlier attempt at the same repopulation idea (tab-focus refresh of existing dropdowns); it was closed as superseded by #3217, but your repopulateExistingSelections() is closer to what that needed. The double-store is the most important to nail down, since it is a data-correctness bug, not just a UI refresh gap.

@michaelxer heads-up: depending on how hinode's PR shapes up, we may land theirs instead of or on top of yours for #3207. Will compare once it is up.

Yeah. My bad, I actually couldn't open a PR here. I'm not that used to GitHub yet hahaha.

Alright. I will reference this PR in a PR that I will open. I'll also sync up to the latest changes of the dev branch that also includes the changes in this PR (I'll check if it's possible).

On that note, I'll also try to investigate the behavior of having a duplicate of the recently added character.

[michaelxer]

Both requested changes are addressed: client-side id generation and shallow copy from getUserTemplates(). CI is green. Ready for re-review.

[michaelxer]

Closing this in favor of the approach in #3424 which has more active discussion with the maintainers. Happy to revisit if needed.