fix: address review feedback for models and routing

2026-03-12 05:51:53 +00:00
parent 5a7531f635
commit 1da3608d42
10 changed files with 444 additions and 44 deletions
@@ -320,4 +320,202 @@ describe('Models marketplace text', () => {
      root?.unmount();
    }
  });
+
+  it('re-sorts models using site-scoped counts after selecting a site filter', async () => {
+    apiMock.getModelsMarketplace.mockResolvedValue({
+      models: [
+        {
+          name: 'gpt-4o',
+          accountCount: 4,
+          tokenCount: 4,
+          avgLatency: 300,
+          successRate: 98,
+          description: null,
+          tags: [],
+          supportedEndpointTypes: [],
+          pricingSources: [],
+          accounts: [
+            {
+              id: 1,
+              site: '站点 A',
+              username: 'user-a-1',
+              latency: 300,
+              balance: 8,
+              tokens: [{ id: 1, name: 'token-a-1', isDefault: true }],
+            },
+            {
+              id: 2,
+              site: '站点 B',
+              username: 'user-b-1',
+              latency: 200,
+              balance: 8,
+              tokens: [{ id: 2, name: 'token-b-1', isDefault: true }],
+            },
+            {
+              id: 3,
+              site: '站点 B',
+              username: 'user-b-2',
+              latency: 250,
+              balance: 8,
+              tokens: [{ id: 3, name: 'token-b-2', isDefault: true }],
+            },
+            {
+              id: 4,
+              site: '站点 B',
+              username: 'user-b-3',
+              latency: 260,
+              balance: 8,
+              tokens: [{ id: 4, name: 'token-b-3', isDefault: true }],
+            },
+          ],
+        },
+        {
+          name: 'claude-3-5-sonnet',
+          accountCount: 2,
+          tokenCount: 2,
+          avgLatency: 420,
+          successRate: 95,
+          description: null,
+          tags: [],
+          supportedEndpointTypes: [],
+          pricingSources: [],
+          accounts: [
+            {
+              id: 5,
+              site: '站点 A',
+              username: 'user-a-2',
+              latency: 410,
+              balance: 9,
+              tokens: [{ id: 5, name: 'token-a-2', isDefault: true }],
+            },
+            {
+              id: 6,
+              site: '站点 A',
+              username: 'user-a-3',
+              latency: 430,
+              balance: 9,
+              tokens: [{ id: 6, name: 'token-a-3', isDefault: true }],
+            },
+          ],
+        },
+      ],
+    });
+
+    let root: ReturnType<typeof create> | null = null;
+
+    try {
+      await act(async () => {
+        root = create(
+          <MemoryRouter initialEntries={['/models']}>
+            <ToastProvider>
+              <Models />
+            </ToastProvider>
+          </MemoryRouter>,
+        );
+      });
+      await flushMicrotasks();
+
+      const siteFilterItem = root!.root.find((node) => (
+        node.type === 'div'
+        && typeof node.props.className === 'string'
+        && node.props.className.includes('filter-item')
+        && typeof node.props.onClick === 'function'
+        && collectText(node).includes('站点 A')
+      ));
+
+      await act(async () => {
+        siteFilterItem.props.onClick();
+      });
+      await flushMicrotasks();
+
+      const cards = root!.root.findAll((node) => (
+        node.type === 'div'
+        && typeof node.props.className === 'string'
+        && node.props.className.split(' ').includes('model-card')
+        && typeof node.props.onClick === 'function'
+      ));
+
+      expect(cards.length).toBe(2);
+      expect(collectText(cards[0]!)).toContain('claude-3-5-sonnet');
+      expect(collectText(cards[1]!)).toContain('gpt-4o');
+    } finally {
+      root?.unmount();
+    }
+  });
+
+  it('renders unknown latency instead of falling back to another site latency', async () => {
+    apiMock.getModelsMarketplace.mockResolvedValue({
+      models: [
+        {
+          name: 'gpt-4o',
+          accountCount: 2,
+          tokenCount: 2,
+          avgLatency: 680,
+          successRate: 93,
+          description: null,
+          tags: [],
+          supportedEndpointTypes: [],
+          pricingSources: [],
+          accounts: [
+            {
+              id: 1,
+              site: '站点 A',
+              username: 'user-a',
+              latency: null,
+              balance: 12,
+              tokens: [{ id: 1, name: 'token-a', isDefault: true }],
+            },
+            {
+              id: 2,
+              site: '站点 B',
+              username: 'user-b',
+              latency: 680,
+              balance: 12,
+              tokens: [{ id: 2, name: 'token-b', isDefault: true }],
+            },
+          ],
+        },
+      ],
+    });
+
+    let root: ReturnType<typeof create> | null = null;
+
+    try {
+      await act(async () => {
+        root = create(
+          <MemoryRouter initialEntries={['/models']}>
+            <ToastProvider>
+              <Models />
+            </ToastProvider>
+          </MemoryRouter>,
+        );
+      });
+      await flushMicrotasks();
+
+      const siteFilterItem = root!.root.find((node) => (
+        node.type === 'div'
+        && typeof node.props.className === 'string'
+        && node.props.className.includes('filter-item')
+        && typeof node.props.onClick === 'function'
+        && collectText(node).includes('站点 A')
+      ));
+
+      await act(async () => {
+        siteFilterItem.props.onClick();
+      });
+      await flushMicrotasks();
+
+      const latencyBadge = root!.root.find((node) => (
+        node.type === 'span'
+        && node.props['data-tooltip'] === '平均延迟'
+      ));
+
+      expect(String(latencyBadge.props.className || '')).toContain('badge-muted');
+      expect(collectText(latencyBadge)).toContain('延迟');
+      expect(collectText(latencyBadge)).toContain('—');
+      expect(collectText(root!.root)).not.toContain('680ms');
+    } finally {
+      root?.unmount();
+    }
+  });
 });
@@ -49,7 +49,7 @@ interface ModelRow {
  name: string;
  accountCount: number;
  tokenCount: number;
-  avgLatency: number;
+  avgLatency: number | null;
  successRate: number | null;
  description: string | null;
  tags: string[];
@@ -68,7 +68,13 @@ interface ModelsMarketplaceResponse {
    refreshJobId?: string | null;
  };
 }
-function getMetricColor(latency: number) {
+
+function isKnownLatency(latency: number | null | undefined): latency is number {
+  return typeof latency === 'number' && Number.isFinite(latency);
+}
+
+function getMetricColor(latency: number | null) {
+  if (!isKnownLatency(latency)) return 'var(--color-text-muted)';
  if (latency >= 3000) return 'var(--color-danger)';
  if (latency >= 2000) return 'color-mix(in srgb, var(--color-warning) 30%, var(--color-danger))';
  if (latency >= 1500) return 'color-mix(in srgb, var(--color-warning) 60%, var(--color-danger))';
@@ -77,12 +83,17 @@ function getMetricColor(latency: number) {
  return 'var(--color-success)';
 }

-function getLatencyBadgeClass(latency: number) {
+function getLatencyBadgeClass(latency: number | null) {
+  if (!isKnownLatency(latency)) return 'badge-muted';
  if (latency >= 3000) return 'badge-error';
  if (latency >= 1000) return 'badge-warning';
  return 'badge-success';
 }

+function formatLatency(latency: number | null): string {
+  return isKnownLatency(latency) ? `${latency}ms` : '—';
+}
+
 function getSuccessBadgeClass(rate: number | null) {
  if (rate == null) return 'badge-muted';
  if (rate >= 90) return 'badge-success';
@@ -113,6 +124,29 @@ function renderGroupPricingValue(pricing: ModelGroupPricing): string {

 const PAGE_SIZES = [10, 20, 50];

+function compareModels(a: ModelRow, b: ModelRow, sortBy: SortColumn, sortDir: 'asc' | 'desc'): number {
+  if (sortBy === 'name') {
+    const cmp = a.name.localeCompare(b.name);
+    return sortDir === 'asc' ? cmp : -cmp;
+  }
+
+  const resolveNumericSortValue = (model: ModelRow) => {
+    if (sortBy === 'successRate') return model.successRate ?? -1;
+    if (sortBy === 'avgLatency') {
+      if (!isKnownLatency(model.avgLatency)) {
+        return sortDir === 'asc' ? Number.POSITIVE_INFINITY : Number.NEGATIVE_INFINITY;
+      }
+      return model.avgLatency;
+    }
+    return model[sortBy] ?? 0;
+  };
+
+  const va = resolveNumericSortValue(a);
+  const vb = resolveNumericSortValue(b);
+  if (va === vb) return a.name.localeCompare(b.name);
+  return sortDir === 'desc' ? vb - va : va - vb;
+}
+
 /* ---- component ---- */
 export default function Models() {
  const toast = useToast();
@@ -259,7 +293,7 @@ export default function Models() {
    return [...m.entries()].sort((a, b) => b[1] - a[1]);
  }, [data.models]);

-  /* ---- filtered + sorted ---- */
+  /* ---- filtered ---- */
  const filteredModels = useMemo(() => {
    let list = data.models;

@@ -280,29 +314,19 @@ export default function Models() {
      list = list.filter(m => m.name.toLowerCase().includes(q));
    }

-    return [...list].sort((a, b) => {
-      if (sortBy === 'name') {
-        const cmp = a.name.localeCompare(b.name);
-        return sortDir === 'asc' ? cmp : -cmp;
-      }
-      const va = sortBy === 'successRate' ? (a.successRate ?? -1) : (a[sortBy] ?? 0);
-      const vb = sortBy === 'successRate' ? (b.successRate ?? -1) : (b[sortBy] ?? 0);
-      if (va === vb) return a.name.localeCompare(b.name);
-      return sortDir === 'desc' ? vb - va : va - vb;
-    });
-  }, [data.models, search, activeSite, activeBrand, sortBy, sortDir]);
+    return list;
+  }, [data.models, search, activeSite, activeBrand]);

  // Keep expanded detail consistent with filters (especially site filter).
  // The list-level filter uses "model has at least one account on this site" semantics;
  // once a model is shown, its detail should honor the active site as well.
  const detailModels = useMemo(() => {
-    if (!activeSite) return filteredModels;
-    return filteredModels.map((model) => {
+    const scopedModels = activeSite ? filteredModels.map((model) => {
      const accounts = model.accounts.filter((account) => account.site === activeSite);
      const pricingSources = model.pricingSources.filter((source) => source.siteName === activeSite);
      const latencyValues = accounts
        .map((account) => account.latency)
-        .filter((latency): latency is number => typeof latency === 'number' && Number.isFinite(latency));
+        .filter(isKnownLatency);
      return {
        ...model,
        accounts,
@@ -311,13 +335,15 @@ export default function Models() {
        tokenCount: accounts.reduce((sum, account) => sum + account.tokens.length, 0),
        avgLatency: latencyValues.length > 0
          ? Math.round(latencyValues.reduce((sum, latency) => sum + latency, 0) / latencyValues.length)
-          : model.avgLatency,
+          : null,
      };
-    });
-  }, [filteredModels, activeSite]);
+    }) : filteredModels;
+
+    return [...scopedModels].sort((a, b) => compareModels(a, b, sortBy, sortDir));
+  }, [filteredModels, activeSite, sortBy, sortDir]);

  /* ---- pagination ---- */
-  const totalPages = Math.max(1, Math.ceil(filteredModels.length / pageSize));
+  const totalPages = Math.max(1, Math.ceil(detailModels.length / pageSize));
  const safePageVal = Math.min(page, totalPages);
  const paged = detailModels.slice((safePageVal - 1) * pageSize, safePageVal * pageSize);

@@ -334,9 +360,12 @@ export default function Models() {
    }
    return ids.size;
  })();
-  const avgLatency = detailModels.length
-    ? Math.round(detailModels.reduce((s, m) => s + m.avgLatency, 0) / detailModels.length)
-    : 0;
+  const latencyMetrics = detailModels
+    .map((model) => model.avgLatency)
+    .filter(isKnownLatency);
+  const avgLatency = latencyMetrics.length > 0
+    ? Math.round(latencyMetrics.reduce((sum, latency) => sum + latency, 0) / latencyMetrics.length)
+    : null;

  /* ---- copy ---- */
  const copyName = (name: string) => {
@@ -532,12 +561,12 @@ export default function Models() {
            <span data-tooltip={tr('当前筛选范围内去重后的唯一账号数')}>
              {tr('去重账号')} <b style={{ color: 'var(--color-text-primary)' }}>{uniqueAccountCount}</b>
            </span>
-            <span>{tr('平均延迟')} <b style={{ color: getMetricColor(avgLatency) }}>{avgLatency}ms</b></span>
+            <span>{tr('平均延迟')} <b style={{ color: getMetricColor(avgLatency) }}>{formatLatency(avgLatency)}</b></span>
          </div>
        </div>

        {/* Empty */}
-        {filteredModels.length === 0 ? (
+        {detailModels.length === 0 ? (
          <div className="empty-state">
            <div className="empty-state-icon">
              <svg width="40" height="40" fill="none" viewBox="0 0 24 24" stroke="currentColor">
@@ -572,7 +601,7 @@ export default function Models() {
                        style={{ fontVariantNumeric: 'tabular-nums' }}
                        data-tooltip={tr('平均延迟')}
                      >
-                        {tr('延迟')} {m.avgLatency}ms
+                        {tr('延迟')} {formatLatency(m.avgLatency)}
                      </span>
                      <span
                        className={`badge ${getSuccessBadgeClass(m.successRate)}`}
@@ -617,7 +646,7 @@ export default function Models() {
                  {m.successRate != null && m.successRate < 60 && (
                    <span className="model-tag model-tag-orange">{tr('风险')}</span>
                  )}
-                  {m.avgLatency <= 500 && (
+                  {isKnownLatency(m.avgLatency) && m.avgLatency <= 500 && (
                    <span className="model-tag model-tag-purple">{tr('低延迟')}</span>
                  )}
                </div>
@@ -761,7 +790,7 @@ export default function Models() {
                          className={`badge ${getLatencyBadgeClass(m.avgLatency)}`}
                          style={{ fontSize: 12, fontVariantNumeric: 'tabular-nums' }}
                        >
-                          {m.avgLatency}ms
+                          {formatLatency(m.avgLatency)}
                        </span>
                      </td>
                      <td>
@@ -364,7 +364,6 @@ export default function TokenRoutes() {
    try {
      await api.updateRoute(route.id, { routingStrategy });
      toast.success(routingStrategy === 'round_robin' ? '已切换为轮询策略' : '已切换为权重随机策略');
-      await load();
    } catch (e: any) {
      setRouteSummaries((prev) => prev.map((item) => (
        item.id === route.id
@@ -372,9 +371,16 @@ export default function TokenRoutes() {
          : item
      )));
      toast.error(e.message || '更新路由策略失败');
+      return;
    } finally {
      setUpdatingRoutingStrategyByRoute((prev) => ({ ...prev, [route.id]: false }));
    }
+
+    try {
+      await load();
+    } catch (e: any) {
+      toast.error(e?.message || '路由策略已保存，但刷新列表失败');
+    }
  };

  // Stable derived value: only changes when route patterns change (not on enabled toggle)
@@ -30,6 +30,29 @@ describe('tokenBindingPresentation', () => {
    expect(result.helperText).toContain('不会跟着变');
  });

+  it('describes fixed mode when the selected token is not the account default', () => {
+    const result = describeTokenBinding([
+      { id: 1, name: 'default', isDefault: true },
+      { id: 2, name: 'backup', isDefault: false },
+    ], 2);
+
+    expect(result.bindingModeLabel).toBe('固定令牌');
+    expect(result.effectiveTokenName).toBe('backup');
+    expect(result.helperText).toContain('不会随账号默认变化');
+  });
+
+  it('describes follow-account-default mode when no default token exists yet', () => {
+    const result = describeTokenBinding([
+      { id: 2, name: 'backup', isDefault: false },
+      { id: 3, name: 'spare', isDefault: false },
+    ], 0);
+
+    expect(result.bindingModeLabel).toBe('跟随账号默认');
+    expect(result.effectiveTokenName).toBe('未设置默认令牌');
+    expect(result.helperText).toContain('当前账号还没有默认令牌');
+    expect(result.followOptionDescription).toContain('以后账号默认变化时会自动切换');
+  });
+
  it('formats fixed token options with clearer labels and descriptions', () => {
    const token = {
      id: 3,
@@ -0,0 +1,130 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { act, create, type ReactTestInstance } from 'react-test-renderer';
+import { MemoryRouter } from 'react-router-dom';
+import { ToastProvider } from '../components/Toast.js';
+import TokenRoutes from './TokenRoutes.js';
+
+const { apiMock, getBrandMock } = vi.hoisted(() => ({
+  apiMock: {
+    getRoutesSummary: vi.fn(),
+    getRouteChannels: vi.fn(),
+    getModelTokenCandidates: vi.fn(),
+    getRouteDecisionsBatch: vi.fn(),
+    getRouteWideDecisionsBatch: vi.fn(),
+    updateRoute: vi.fn(),
+  },
+  getBrandMock: vi.fn(),
+}));
+
+vi.mock('../api.js', () => ({
+  api: apiMock,
+}));
+
+vi.mock('../components/BrandIcon.js', () => ({
+  BrandGlyph: ({ brand, icon, model }: { brand?: { name?: string } | null; icon?: string | null; model?: string | null }) => (
+    <span>{brand?.name || icon || model || ''}</span>
+  ),
+  InlineBrandIcon: ({ model }: { model: string }) => model ? <span>{model}</span> : null,
+  getBrand: (...args: unknown[]) => getBrandMock(...args),
+  hashColor: () => 'linear-gradient(135deg,#4f46e5,#818cf8)',
+  normalizeBrandIconKey: (icon: string) => icon,
+}));
+
+function collectText(node: ReactTestInstance): string {
+  return (node.children || []).map((child) => {
+    if (typeof child === 'string') return child;
+    return collectText(child);
+  }).join('');
+}
+
+async function flushMicrotasks() {
+  await act(async () => {
+    await Promise.resolve();
+    await Promise.resolve();
+  });
+}
+
+describe('TokenRoutes routing strategy updates', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    getBrandMock.mockReset();
+    getBrandMock.mockReturnValue(null);
+    apiMock.getRoutesSummary
+      .mockResolvedValueOnce([
+        {
+          id: 1,
+          modelPattern: 'gpt-4o-mini',
+          displayName: 'gpt-4o-mini',
+          displayIcon: null,
+          modelMapping: null,
+          routingStrategy: 'weighted',
+          enabled: true,
+          channelCount: 0,
+          enabledChannelCount: 0,
+          siteNames: [],
+          decisionSnapshot: null,
+          decisionRefreshedAt: null,
+        },
+      ])
+      .mockRejectedValueOnce(new Error('refresh failed'));
+    apiMock.getRouteChannels.mockResolvedValue([]);
+    apiMock.getModelTokenCandidates.mockResolvedValue({ models: {} });
+    apiMock.getRouteDecisionsBatch.mockResolvedValue({ decisions: {} });
+    apiMock.getRouteWideDecisionsBatch.mockResolvedValue({ decisions: {} });
+    apiMock.updateRoute.mockResolvedValue({});
+  });
+
+  afterEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it('keeps the optimistic routing strategy when refresh fails after a successful save', async () => {
+    let root: ReturnType<typeof create> | null = null;
+    try {
+      await act(async () => {
+        root = create(
+          <MemoryRouter initialEntries={['/routes']}>
+            <ToastProvider>
+              <TokenRoutes />
+            </ToastProvider>
+          </MemoryRouter>,
+        );
+      });
+      await flushMicrotasks();
+
+      const expandButton = root.root.find((node) => (
+        node.type === 'div'
+        && String(node.props.className || '').includes('route-card-collapsed')
+      ));
+      await act(async () => {
+        expandButton.props.onClick();
+      });
+      await flushMicrotasks();
+
+      const roundRobinOption = root.root.find((node) => (
+        node.type === 'button'
+        && typeof node.props.className === 'string'
+        && node.props.className.includes('modern-select-option')
+        && collectText(node).includes('轮询')
+      ));
+
+      await act(async () => {
+        roundRobinOption.props.onClick();
+      });
+      await flushMicrotasks();
+
+      expect(apiMock.updateRoute).toHaveBeenCalledWith(1, { routingStrategy: 'round_robin' });
+      expect(apiMock.getRoutesSummary).toHaveBeenCalledTimes(2);
+
+      const strategyTrigger = root.root.find((node) => (
+        node.type === 'button'
+        && typeof node.props.className === 'string'
+        && node.props.className.includes('modern-select-trigger')
+        && collectText(node).includes('轮询')
+      ));
+      expect(collectText(strategyTrigger)).toContain('轮询');
+    } finally {
+      root?.unmount();
+    }
+  });
+});