feat(models): one canonical HF-token path + surface incomplete cache (#927)
Two Model-management enhancements building on #908 (no re-do of its fixes). Unify the two HF-token entry points. The Model Store toolbar saved the token via /system/set-env (env var + HF-CLI file) while Settings → Credentials saves to the encrypted app store — two stores with an asymmetric clear path, so a toolbar-set token silently outlived the Credentials "Clear" (a support-ticket generator). The toolbar now POSTs the SAME canonical endpoint Credentials uses (/api/settings/hf-token → encrypted store + huggingface_hub.login()), so there is one store with one clear path. In-process parity is preserved (login() populates the HF canonical file, so downloads pick it up immediately). Surface an incomplete/partial cache. A truncated download (config landed, weight shard didn't) occupies disk but used to read as a plain "not installed". The backend already flags it as `incomplete`; the row now shows an "incomplete · N MB" warn badge, relabels the primary action to "Repair" (re-runs snapshot_download to finish the missing shard), and offers a Delete to clear the partial bytes. Tests: modelStoreTokenPath (toolbar hits /api/settings/hf-token, never /system/set-env) + modelStoreIncomplete (badge, Repair→onInstall, Delete, no false positives on normal not-installed/installed rows). Full vitest green; lint + format clean. i18n keys added to en.json (models.incomplete, incomplete_title, repair_btn, repair_title). Co-authored-by: mergetest <test@local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
mergetest
Claude Fable 5
parent
5e135b9655
commit
48a7154810
@@ -96,12 +96,14 @@ export default function ModelStoreTab({ info, modelBadge }) {
|
||||
if (!value) return;
|
||||
setHfSaving(true);
|
||||
try {
|
||||
const { apiFetch } = await import('../../api/client');
|
||||
await apiFetch('/system/set-env', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ key: 'HF_TOKEN', value }),
|
||||
});
|
||||
// One canonical token path: persist to the encrypted app store — the SAME
|
||||
// store Settings → Credentials writes AND clears (/api/settings/hf-token).
|
||||
// This toolbar used to POST /system/set-env (env var + HF-CLI file), a
|
||||
// *second* store the Credentials "Clear" button couldn't reach, so a
|
||||
// toolbar-set token silently outlived a Clear (a support-ticket generator).
|
||||
// Now both entry points share one store with one clear path.
|
||||
const { apiPost } = await import('../../api/client');
|
||||
await apiPost('/api/settings/hf-token', { token: value });
|
||||
toast.success(t('models.hf_token_set_toast'));
|
||||
setHfSaved(true);
|
||||
setHfToken('');
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import React from 'react';
|
||||
import { RefreshCw, Trash2, ExternalLink, Download, X } from 'lucide-react';
|
||||
import { RefreshCw, Trash2, ExternalLink, Download, X, Wrench } from 'lucide-react';
|
||||
import { openExternal } from '../../../api/external';
|
||||
import { Button, Badge, Progress } from '../../../ui';
|
||||
import { fmtBytes, orgColor } from './format';
|
||||
@@ -239,6 +239,17 @@ export function makeModelColumns({
|
||||
<Badge tone="success" size="xs">
|
||||
{t('models.installed')}
|
||||
</Badge>
|
||||
) : m.incomplete ? (
|
||||
// A truncated download (backend `incomplete`): config/tokenizer landed
|
||||
// but the weight shard didn't, so it still occupies disk yet can't be
|
||||
// used. Surface it as its own state — with the partial size — instead
|
||||
// of reading as a plain "not installed", so the user knows to repair it
|
||||
// rather than wonder why bytes are used (#622 UX follow-up).
|
||||
<Badge tone="warn" size="xs" title={t('models.incomplete_title')}>
|
||||
{m.size_on_disk_bytes > 0
|
||||
? `${t('models.incomplete')} · ${fmtBytes(m.size_on_disk_bytes)}`
|
||||
: t('models.incomplete')}
|
||||
</Badge>
|
||||
) : rt.unsupported ? (
|
||||
<Badge tone="neutral" size="xs">
|
||||
{(m.platforms || []).join(', ')}
|
||||
@@ -270,14 +281,34 @@ export function makeModelColumns({
|
||||
>
|
||||
<ExternalLink size={11} />
|
||||
</Button>
|
||||
{!m.installed && !rt.rowBusy && !rt.isInstalling && !rt.unsupported && (
|
||||
{!m.installed &&
|
||||
!rt.rowBusy &&
|
||||
!rt.isInstalling &&
|
||||
!rt.unsupported && (
|
||||
// For an `incomplete` (truncated) cache this is a repair, not a
|
||||
// fresh install: re-running snapshot_download completes the missing
|
||||
// weight shard without re-fetching the already-cached config files.
|
||||
<Button
|
||||
variant="subtle"
|
||||
size="sm"
|
||||
onClick={() => onInstall(m.repo_id)}
|
||||
leading={m.incomplete ? <Wrench size={11} /> : <Download size={11} />}
|
||||
title={m.incomplete ? t('models.repair_title') : undefined}
|
||||
>
|
||||
{m.incomplete ? t('models.repair_btn') : t('models.install_btn')}
|
||||
</Button>
|
||||
)}
|
||||
{/* Let the user clear a truncated download's partial bytes (it's not
|
||||
`installed`, so the delete control below never shows for it). */}
|
||||
{m.incomplete && !rt.rowBusy && !rt.isInstalling && !rt.isDeleting && (
|
||||
<Button
|
||||
variant="subtle"
|
||||
size="sm"
|
||||
onClick={() => onInstall(m.repo_id)}
|
||||
leading={<Download size={11} />}
|
||||
variant="icon"
|
||||
iconSize="sm"
|
||||
onClick={() => onDelete(m.repo_id)}
|
||||
title={t('models.delete_btn')}
|
||||
aria-label={t('models.delete_btn')}
|
||||
>
|
||||
{t('models.install_btn')}
|
||||
<Trash2 size={11} />
|
||||
</Button>
|
||||
)}
|
||||
{/* Cancel an in-flight install (P2-A / FDL-11) — shown for any
|
||||
|
||||
@@ -1931,6 +1931,10 @@
|
||||
"working": "working",
|
||||
"installed": "installed",
|
||||
"not_installed": "not installed",
|
||||
"incomplete": "incomplete",
|
||||
"incomplete_title": "Partial download — the model weights are missing. Click Repair to finish it.",
|
||||
"repair_btn": "Repair",
|
||||
"repair_title": "Finish the interrupted download",
|
||||
"required_tag": "required",
|
||||
"delete_btn": "Delete",
|
||||
"hf_token_btn": "HF Token",
|
||||
|
||||
@@ -0,0 +1,100 @@
|
||||
import React from 'react';
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
import { render, screen, fireEvent } from '@testing-library/react';
|
||||
import i18n from '../i18n';
|
||||
import { makeModelColumns } from '../components/settings/models/columns';
|
||||
|
||||
const t = i18n.t.bind(i18n);
|
||||
const REPO = 'org/model';
|
||||
|
||||
// ── Enhancement: surface an incomplete/partial cache ───────────────────────
|
||||
// The backend already flags a truncated download (config landed, weight shard
|
||||
// didn't) with `incomplete: true` — but the row used to read as a plain "not
|
||||
// installed" even though partial bytes sit on disk. These tests lock in the
|
||||
// dedicated "incomplete — N MB · Repair" surfacing so the state can't silently
|
||||
// regress to "not installed".
|
||||
|
||||
// At-rest runtime (no active download / delete / busy) — the state in which a
|
||||
// row's authoritative installed/incomplete flags drive the badge + actions.
|
||||
const IDLE_RT = {
|
||||
showBar: false,
|
||||
isDeleting: false,
|
||||
isInstalling: false,
|
||||
rowBusy: false,
|
||||
unsupported: false,
|
||||
aggPct: null,
|
||||
totals: { downloaded: 0, total: 0 },
|
||||
hasFiles: false,
|
||||
};
|
||||
|
||||
function renderCell(colId, handlers, mOver = {}) {
|
||||
const cols = makeModelColumns({
|
||||
t,
|
||||
getRowRuntime: () => IDLE_RT,
|
||||
speedRef: { current: {} },
|
||||
MODEL_ROLE_LABEL: {},
|
||||
onInstall: vi.fn(),
|
||||
onDelete: vi.fn(),
|
||||
onReinstall: vi.fn(),
|
||||
onCancel: vi.fn(),
|
||||
onDismissError: vi.fn(),
|
||||
...handlers,
|
||||
});
|
||||
const col = cols.find((c) => c.id === colId);
|
||||
const m = {
|
||||
repo_id: REPO,
|
||||
label: 'My Model',
|
||||
role: 'tts',
|
||||
size_gb: 1.2,
|
||||
installed: false,
|
||||
...mOver,
|
||||
};
|
||||
return render(col.cell({ row: { original: m } }));
|
||||
}
|
||||
|
||||
describe('Model Store row — incomplete cache surfacing', () => {
|
||||
const incomplete = { incomplete: true, size_on_disk_bytes: 12 * 1024 * 1024 };
|
||||
|
||||
it('shows an "incomplete" status badge with the partial size (not "not installed")', () => {
|
||||
renderCell('status', {}, incomplete);
|
||||
// Single badge node: "incomplete · 12.0 MB" — the partial bytes are named.
|
||||
const badge = screen.getByText(/incomplete/i);
|
||||
expect(badge.textContent).toMatch(/12(\.0)? MB/);
|
||||
expect(screen.queryByText(/not installed/i)).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('a plain not-installed row still shows "not installed" (no false positive)', () => {
|
||||
renderCell('status', {}, { installed: false });
|
||||
expect(screen.getByText(/not installed/i)).toBeInTheDocument();
|
||||
expect(screen.queryByText(/incomplete/i)).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('an installed row is unaffected — shows "installed"', () => {
|
||||
renderCell('status', {}, { installed: true, size_on_disk_bytes: 5 });
|
||||
expect(screen.getByText(/^installed$/i)).toBeInTheDocument();
|
||||
expect(screen.queryByText(/incomplete/i)).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('the primary action is "Repair" (not "Install") and fires onInstall(repo_id)', () => {
|
||||
const onInstall = vi.fn();
|
||||
renderCell('actions', { onInstall }, incomplete);
|
||||
const repair = screen.getByRole('button', { name: t('models.repair_btn') });
|
||||
expect(repair).toBeInTheDocument();
|
||||
expect(screen.queryByRole('button', { name: t('models.install_btn') })).not.toBeInTheDocument();
|
||||
fireEvent.click(repair);
|
||||
expect(onInstall).toHaveBeenCalledWith(REPO);
|
||||
});
|
||||
|
||||
it('offers a Delete affordance to clear the partial bytes', () => {
|
||||
const onDelete = vi.fn();
|
||||
renderCell('actions', { onDelete }, incomplete);
|
||||
fireEvent.click(screen.getByRole('button', { name: t('models.delete_btn') }));
|
||||
expect(onDelete).toHaveBeenCalledWith(REPO);
|
||||
});
|
||||
|
||||
it('a normal not-installed row shows "Install" (repair path is incomplete-only)', () => {
|
||||
renderCell('actions', {}, { installed: false });
|
||||
expect(screen.getByRole('button', { name: t('models.install_btn') })).toBeInTheDocument();
|
||||
expect(screen.queryByRole('button', { name: t('models.repair_btn') })).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,88 @@
|
||||
import React from 'react';
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { render, screen, fireEvent, waitFor } from '@testing-library/react';
|
||||
import { I18nextProvider } from 'react-i18next';
|
||||
import i18n from '../i18n';
|
||||
|
||||
// ── Enhancement: unify the two HF-token entry points ───────────────────────
|
||||
// The Models toolbar used to save the token via /system/set-env (env var +
|
||||
// HF-CLI file) while Settings → Credentials saves to the encrypted app store —
|
||||
// two stores with an asymmetric clear path (Credentials' "Clear" couldn't
|
||||
// remove a toolbar-set token). This test locks the toolbar onto the SAME
|
||||
// canonical app-store endpoint Credentials uses (/api/settings/hf-token), and
|
||||
// guards against a regression back to /system/set-env.
|
||||
|
||||
const apiPost = vi.fn(() => Promise.resolve({ active: 'app', sources: [] }));
|
||||
vi.mock('../api/client', () => ({
|
||||
apiPost: (...a) => apiPost(...a),
|
||||
apiFetch: vi.fn(() => Promise.resolve({ ok: true, json: () => Promise.resolve({}) })),
|
||||
}));
|
||||
|
||||
const refetch = vi.fn();
|
||||
vi.mock('../api/hooks', () => ({
|
||||
useModels: () => ({
|
||||
data: {
|
||||
models: [],
|
||||
total_installed_bytes: 0,
|
||||
disk_free_gb: 42.5,
|
||||
hf_cache_dir: '/home/u/.cache/huggingface',
|
||||
},
|
||||
isLoading: false,
|
||||
refetch,
|
||||
}),
|
||||
useRecommendations: () => ({ data: null, refetch }),
|
||||
useInstallModel: () => ({ mutateAsync: vi.fn() }),
|
||||
useDeleteModel: () => ({ mutateAsync: vi.fn() }),
|
||||
}));
|
||||
|
||||
vi.mock('../api/setup', () => ({
|
||||
setupDownloadStreamUrl: () => 'http://localhost/stream',
|
||||
cancelInstallModel: vi.fn(),
|
||||
}));
|
||||
vi.mock('../api/external', () => ({ openExternal: vi.fn() }));
|
||||
// Keep the render light + focused on the toolbar.
|
||||
vi.mock('../components/settings/models/ModelsTable', () => ({ default: () => null }));
|
||||
vi.mock('../components/settings/models/RecoBanner', () => ({ default: () => null }));
|
||||
|
||||
import ModelStoreTab from '../components/settings/ModelStoreTab';
|
||||
|
||||
function withI18n(node) {
|
||||
return <I18nextProvider i18n={i18n}>{node}</I18nextProvider>;
|
||||
}
|
||||
|
||||
describe('Model Store toolbar — HF token saves to the canonical app store', () => {
|
||||
beforeEach(() => {
|
||||
apiPost.mockClear();
|
||||
global.EventSource = class {
|
||||
constructor() {
|
||||
this.onmessage = null;
|
||||
}
|
||||
close() {}
|
||||
};
|
||||
});
|
||||
afterEach(() => vi.restoreAllMocks());
|
||||
|
||||
async function openAndSave(token) {
|
||||
render(withI18n(<ModelStoreTab info={{ has_hf_token: false }} modelBadge={null} />));
|
||||
// The compact toolbar shows a "HF Token" trigger when no token is set.
|
||||
fireEvent.click(screen.getByRole('button', { name: /HF Token/i }));
|
||||
const input = screen.getByPlaceholderText(/hf_/i);
|
||||
fireEvent.change(input, { target: { value: token } });
|
||||
fireEvent.click(screen.getByRole('button', { name: i18n.t('common.save') }));
|
||||
}
|
||||
|
||||
it('POSTs the token to /api/settings/hf-token (same store as Credentials)', async () => {
|
||||
await openAndSave('hf_abc123');
|
||||
await waitFor(() => expect(apiPost).toHaveBeenCalledTimes(1));
|
||||
expect(apiPost).toHaveBeenCalledWith('/api/settings/hf-token', { token: 'hf_abc123' });
|
||||
});
|
||||
|
||||
it('never routes the token through the legacy /system/set-env store', async () => {
|
||||
await openAndSave('hf_xyz');
|
||||
await waitFor(() => expect(apiPost).toHaveBeenCalled());
|
||||
for (const call of apiPost.mock.calls) {
|
||||
expect(call[0]).not.toContain('/system/set-env');
|
||||
}
|
||||
expect(apiPost).toHaveBeenCalledWith('/api/settings/hf-token', { token: 'hf_xyz' });
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user