mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
fix(custom-model): merge-time fixes for the promoted-model picker (#459)
The maintainer's promised follow-ups to opticon454's picker promotion, applied on the landing branch after the merge (ecb95b5d): - session-ui.js: the promotion tag ("Currently loaded" / "Last used") and the "Default" pill are two separate spans, so a promoted row that is also the endpoint's defaultModelId shows both instead of silently losing its Default marking; two tests pin it (both fail on the old exclusive-slot rendering). - styles.css: a dedicated #customModelPickModal .set-scope rule, since the pill was only styled inside the three settings modals and rendered as plain body text here; same skin tokens, modal layout untouched. - docs/wiki/Custom-Model-Endpoints.md: describe the promotion (currently loaded, else last used per device), the separate Default pill, and that nothing is ever auto-chosen. - CLAUDE.md + docs/custom-model-endpoints.md: credit the real "Last used" writers (_runCustomModelEntryViaRestart and _quickStartWithCustomModelConfirm; runCustomModelEntry only dispatches since88e5b7b2) and drop the now-wrong "both defer to Default" sentence. - Not done: moving the one-shot "last used" write into _runCustomModelEntryOneShot, because the existing one-shot tests assert that _quickStartWithCustomModelConfirm writes the key itself. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 0cb0f911adc14a852ba5c2951768a4aa87c25657)
This commit is contained in:
@@ -196,14 +196,20 @@ under your thumb:
|
|||||||
- **"Last used"** — shown only when nothing is currently loaded: the model
|
- **"Last used"** — shown only when nothing is currently loaded: the model
|
||||||
actually launched last for this exact (harness, endpoint) pair, read
|
actually launched last for this exact (harness, endpoint) pair, read
|
||||||
from the per-device `codeman:customModelLastUsed:<mode>:<endpointId>`
|
from the per-device `codeman:customModelLastUsed:<mode>:<endpointId>`
|
||||||
localStorage key. Written by `runCustomModelEntry` /
|
localStorage key. Written by `_runCustomModelEntryViaRestart` (claude)
|
||||||
`_quickStartWithCustomModelConfirm` only once the model is actually
|
and `_quickStartWithCustomModelConfirm` (every one-shot launch; the
|
||||||
applied, never on the mere click — declining the context-window warning
|
`runCustomModelEntry` entry point itself only dispatches between the
|
||||||
means this exact model cannot work with this CLI at all, so promoting it
|
two) only once the model is actually applied, never on the mere click —
|
||||||
next time would be actively wrong, not just premature.
|
declining the context-window warning means this exact model cannot work
|
||||||
|
with this CLI at all, so promoting it next time would be actively wrong,
|
||||||
|
not just premature.
|
||||||
|
|
||||||
Neither tag reorders anything past that one promoted row, and both defer
|
Neither tag reorders anything past that one promoted row. The "Default"
|
||||||
to "Default" when neither applies.
|
pill is a separate span, not a third value of the same slot: a promoted
|
||||||
|
row that is also the endpoint's `defaultModelId` shows both tags (on a
|
||||||
|
single-purpose GPU box that is the common case, and an exclusive slot
|
||||||
|
silently dropped the Default marking for exactly that row), and a row
|
||||||
|
with neither promotion nor default shows no tag at all.
|
||||||
|
|
||||||
**How the launch itself applies the endpoint depends on the harness.** For
|
**How the launch itself applies the endpoint depends on the harness.** For
|
||||||
opencode, Codex, Gemini, Pi, Grok, DeepSeek and OMP (`runCustomModelEntry` →
|
opencode, Codex, Gemini, Pi, Grok, DeepSeek and OMP (`runCustomModelEntry` →
|
||||||
|
|||||||
@@ -50,7 +50,13 @@ pointed at.
|
|||||||
the session launches straight away on that model — nothing to choose. With two or more, a
|
the session launches straight away on that model — nothing to choose. With two or more, a
|
||||||
small dialog asks which one to use for this launch before starting the session; the
|
small dialog asks which one to use for this launch before starting the session; the
|
||||||
endpoint's default model, if set, is marked but not auto-picked, so a launch can deliberately
|
endpoint's default model, if set, is marked but not auto-picked, so a launch can deliberately
|
||||||
use a different one without changing the saved default.
|
use a different one without changing the saved default. The list is not raw discovery order
|
||||||
|
either: the model llama-swap reports loaded and ready is moved to the top and tagged
|
||||||
|
**Currently loaded**, and when nothing is loaded, the model you last launched on this harness
|
||||||
|
and endpoint pair is moved up instead and tagged **Last used** (a per-device browser value, so
|
||||||
|
another device starts from its own history). The default model keeps its own **Default** pill
|
||||||
|
in both cases, and nothing is ever auto-chosen: the promoted row is simply the one under your
|
||||||
|
thumb.
|
||||||
|
|
||||||
**For opencode, Codex, Gemini, Pi, Grok, DeepSeek and OMP, picking an entry launches
|
**For opencode, Codex, Gemini, Pi, Grok, DeepSeek and OMP, picking an entry launches
|
||||||
straight onto the endpoint** — no restart, because the endpoint is applied before the
|
straight onto the endpoint** — no restart, because the endpoint is applied before the
|
||||||
|
|||||||
@@ -825,12 +825,17 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
`${cliLabel} → ${host.label} — ${(host.models || []).length} models discovered.`;
|
`${cliLabel} → ${host.label} — ${(host.models || []).length} models discovered.`;
|
||||||
list.innerHTML = models
|
list.innerHTML = models
|
||||||
.map((m) => {
|
.map((m) => {
|
||||||
const isDefault = m === host.defaultModelId;
|
// Two independent tags, never one exclusive slot: the promotion tag says what
|
||||||
const tag = m === currentlyLoaded ? 'Currently loaded' : m === lastUsed ? 'Last used' : isDefault ? 'Default' : null;
|
// llama-swap (or this device's history) knows about the model, the Default pill
|
||||||
|
// says what the saved endpoint says about it, and on a single-purpose GPU box the
|
||||||
|
// promoted model IS the default more often than not. One slot holding whichever
|
||||||
|
// applied first silently dropped the Default marking for exactly that row.
|
||||||
|
const promotion = m === currentlyLoaded ? 'Currently loaded' : m === lastUsed ? 'Last used' : null;
|
||||||
|
const tags = [promotion, m === host.defaultModelId ? 'Default' : null].filter(Boolean);
|
||||||
const arg = escapeHtml(JSON.stringify(m));
|
const arg = escapeHtml(JSON.stringify(m));
|
||||||
return `
|
return `
|
||||||
<button class="run-mode-option" onclick="app.chooseCustomModelAndRun(${arg})">
|
<button class="run-mode-option" onclick="app.chooseCustomModelAndRun(${arg})">
|
||||||
<span class="run-mode-dot ${escapeHtml(mode)}"></span>${escapeHtml(m)}${tag ? ` <span class="set-scope">${escapeHtml(tag)}</span>` : ''}
|
<span class="run-mode-dot ${escapeHtml(mode)}"></span>${escapeHtml(m)}${tags.map((t) => ` <span class="set-scope">${escapeHtml(t)}</span>`).join('')}
|
||||||
</button>`;
|
</button>`;
|
||||||
})
|
})
|
||||||
.join('');
|
.join('');
|
||||||
|
|||||||
@@ -6869,6 +6869,27 @@ body.touch-device .terminal-container .xterm .xterm-helper-textarea {
|
|||||||
min-height: 0;
|
min-height: 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* The picker's row tags ("Currently loaded", "Last used", "Default") reuse the
|
||||||
|
settings surface's .set-scope pill, but that pill is styled only inside
|
||||||
|
:is(#appSettingsModal, #sessionOptionsModal, #createCaseModal) (see the
|
||||||
|
settings-surface block), so in here it rendered as plain body text and
|
||||||
|
"qwen3 Currently loaded" read as one model name. Same pill, same skin
|
||||||
|
tokens (never a hardcoded colour: --text-muted / --border are what each
|
||||||
|
html[data-skin] block redefines), and nothing about the modal's layout or
|
||||||
|
z-index. The row is a flex container, so the pills sit after the name
|
||||||
|
with the row's own gap and the trailing whitespace collapses. */
|
||||||
|
#customModelPickModal .set-scope {
|
||||||
|
font-size: 0.52rem;
|
||||||
|
letter-spacing: 0.06em;
|
||||||
|
text-transform: uppercase;
|
||||||
|
border-radius: 4px;
|
||||||
|
padding: 1px 4px;
|
||||||
|
white-space: nowrap;
|
||||||
|
color: var(--text-muted);
|
||||||
|
border: 1px solid var(--border);
|
||||||
|
opacity: 0.8;
|
||||||
|
}
|
||||||
|
|
||||||
/* Custom Model Endpoint Profiles: llama-swap model-swap confirmation — replaces a native
|
/* Custom Model Endpoint Profiles: llama-swap model-swap confirmation — replaces a native
|
||||||
confirm() popup (docs/custom-model-endpoints-plan.md) so it looks and feels like the
|
confirm() popup (docs/custom-model-endpoints-plan.md) so it looks and feels like the
|
||||||
rest of the app instead of a browser chrome dialog. Shares the context-window-too-small
|
rest of the app instead of a browser chrome dialog. Shares the context-window-too-small
|
||||||
|
|||||||
@@ -335,6 +335,67 @@ describe('Custom Model Endpoint Profiles: the "which model" picker', () => {
|
|||||||
expect(buttons[0].textContent).toContain('Currently loaded');
|
expect(buttons[0].textContent).toContain('Currently loaded');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('a currently-loaded row that is also the endpoint default shows BOTH tags, as two separate spans', async () => {
|
||||||
|
// The common case on a single-purpose GPU box: the one model that is loaded is the
|
||||||
|
// saved default too. An exclusive tag slot (promotion, else Default) silently dropped
|
||||||
|
// the Default marking for exactly that row.
|
||||||
|
const { win, app } = bootApp({
|
||||||
|
hosts: [
|
||||||
|
{
|
||||||
|
id: 'llama-box',
|
||||||
|
label: 'llama.cpp',
|
||||||
|
baseUrl: 'http://x',
|
||||||
|
models: ['qwen3', 'llama3', 'phi4'],
|
||||||
|
defaultModelId: 'phi4',
|
||||||
|
},
|
||||||
|
],
|
||||||
|
});
|
||||||
|
const origApiJson = app._apiJson;
|
||||||
|
app._apiJson = async (path: string) => {
|
||||||
|
if (path === '/api/model-endpoints/llama-box/running-status') {
|
||||||
|
return { isLlamaSwap: true, running: [{ model: 'phi4', state: 'ready' }] };
|
||||||
|
}
|
||||||
|
return origApiJson(path);
|
||||||
|
};
|
||||||
|
|
||||||
|
await app.selectCustomModelEntry('claude', 'llama-box');
|
||||||
|
|
||||||
|
const buttons = [...win.document.getElementById('customModelPickList')!.querySelectorAll('button')];
|
||||||
|
expect(buttons[0].textContent).toContain('phi4');
|
||||||
|
const tags = [...buttons[0].querySelectorAll('.set-scope')].map((el) => el.textContent);
|
||||||
|
expect(tags).toEqual(['Currently loaded', 'Default']);
|
||||||
|
// Rows with neither a promotion nor the default carry no tag at all.
|
||||||
|
expect(buttons[1].querySelectorAll('.set-scope')).toHaveLength(0);
|
||||||
|
expect(buttons[2].querySelectorAll('.set-scope')).toHaveLength(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('a "last used" row that is also the endpoint default shows both tags too', async () => {
|
||||||
|
const { win, app } = bootApp({
|
||||||
|
hosts: [
|
||||||
|
{
|
||||||
|
id: 'llama-box',
|
||||||
|
label: 'llama.cpp',
|
||||||
|
baseUrl: 'http://x',
|
||||||
|
models: ['qwen3', 'llama3', 'phi4'],
|
||||||
|
defaultModelId: 'llama3',
|
||||||
|
},
|
||||||
|
],
|
||||||
|
});
|
||||||
|
const origApiJson = app._apiJson;
|
||||||
|
app._apiJson = async (path: string) => {
|
||||||
|
if (path === '/api/model-endpoints/llama-box/running-status') return { isLlamaSwap: false, running: [] };
|
||||||
|
return origApiJson(path);
|
||||||
|
};
|
||||||
|
win.localStorage.setItem('codeman:customModelLastUsed:claude:llama-box', 'llama3');
|
||||||
|
|
||||||
|
await app.selectCustomModelEntry('claude', 'llama-box');
|
||||||
|
|
||||||
|
const buttons = [...win.document.getElementById('customModelPickList')!.querySelectorAll('button')];
|
||||||
|
expect(buttons[0].textContent).toContain('llama3');
|
||||||
|
const tags = [...buttons[0].querySelectorAll('.set-scope')].map((el) => el.textContent);
|
||||||
|
expect(tags).toEqual(['Last used', 'Default']);
|
||||||
|
});
|
||||||
|
|
||||||
it('is not fooled by a model llama-swap reports loaded but not yet ready, or one this host no longer lists', async () => {
|
it('is not fooled by a model llama-swap reports loaded but not yet ready, or one this host no longer lists', async () => {
|
||||||
const { win, app } = bootApp({
|
const { win, app } = bootApp({
|
||||||
hosts: [{ id: 'llama-box', label: 'llama.cpp', baseUrl: 'http://x', models: ['qwen3', 'llama3'] }],
|
hosts: [{ id: 'llama-box', label: 'llama.cpp', baseUrl: 'http://x', models: ['qwen3', 'llama3'] }],
|
||||||
|
|||||||
Reference in New Issue
Block a user