mirror of
https://github.com/lukaszraczylo/traefik-llmgateway.git
synced 2026-10-15 05:29:28 +00:00
fix(webui): accessible warn token, day-window model badges, review minors
Review round (v0.22) — MUST-1, SHOULD-2's frontend half, and every folded minor that touched webui/. MUST-1 (contrast): the degraded badge's amber previously reused --color-chart-tokens-out, tuned for a chart mark, not body text — measured ~2.22:1 against --card in light mode, a WCAG 1.4.3 failure on exactly the "something's wrong" tier. A new semantic --status-warn token pair (main.css, with a @theme inline mapping) replaces it: #a35c00 light (~5.06:1), #f0ac1f dark (~8.97:1, already passing, redefined anyway so every semantic-color token's dark value lives in one place). ProviderRateBadge.vue's degraded tier now uses border-status-warn/40 text-status-warn. SHOULD-2 (frontend half — see the backend commit for admin.go/limits.go): AdminModelRateView drops attemptsMinute/failuresMinute (the backend no longer sends them at all). ProviderRateBadge.vue's attempts-minute/ failures-minute props become optional; their absence (not a passed- through 0) triggers a new dayRateTitle fallback (lib/provider-rate.ts) — a real "0 attempts this minute" and "no minute data fetched at all" read identically as 0 otherwise, which would misreport a model badge as having live-minute data it never measured. Folded minors: - formatRatePercent floors instead of rounds — only a true 100% rate (zero failures) ever renders "100%"; 99.6% now shows "99%", not a round-tripped "100%" that would overstate it. - providerSuccessRate NaN-guards both inputs (Number.isFinite). - ProvidersView's modelRateFor uses `p.modelRates?.[model]` — a stale client against an older server build could otherwise throw rather than fall back, matching its own doc comment's stated intent. - ProviderRateBadge's title is now also its aria-label — title alone is mouse-only; a keyboard/screen-reader user previously got none of the minute/day detail. - The accordion's "Last refresh" DETAIL row (not just the trigger's compact text) now shares refreshLabel's three-state logic via a new refreshDetailLabel (lib/format.ts) — it previously called formatTimestamp directly, so a discovery-off provider's expanded row still said "never" beside a trigger that correctly said "discovery off". - providerModelScopeID's Go doc comment (limits.go, backend commit) cross-references configNamePattern for why the "/" join is collision- safe even though a model id may itself contain "/". - useTabHash.ts documents its two intentional URL-normalization quirks: case-sensitive hash matching, and an invalid/absent initial hash normalizing only in memory, not in the address bar, until the next tab change.
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { refreshLabel } from './format'
|
||||
import { refreshDetailLabel, refreshLabel } from './format'
|
||||
|
||||
const ZERO_TIME = '0001-01-01T00:00:00Z'
|
||||
|
||||
@@ -24,3 +24,28 @@ describe('refreshLabel', () => {
|
||||
expect(refreshLabel(true, oneMinuteAgo)).toMatch(/^refreshed \(\d+s ago\)$/)
|
||||
})
|
||||
})
|
||||
|
||||
// TestAdminOverview_ProviderLastErrAfterFailedRefresh's webui counterpart:
|
||||
// the accordion DETAIL row (folded review minor, v0.22 review round) must
|
||||
// share refreshLabel's own three-state logic instead of calling
|
||||
// formatTimestamp directly — the bug this closes was a discovery-off
|
||||
// provider's expanded row still reading the raw "never" right next to a
|
||||
// trigger row that correctly said "discovery off".
|
||||
describe('refreshDetailLabel', () => {
|
||||
it('reads "discovery off" when discovery is disabled, regardless of lastRefresh', () => {
|
||||
expect(refreshDetailLabel(false, ZERO_TIME)).toBe('discovery off')
|
||||
expect(refreshDetailLabel(false, undefined)).toBe('discovery off')
|
||||
})
|
||||
|
||||
it('reads "pending" when discovery is enabled but lastRefresh is still the unset sentinel', () => {
|
||||
expect(refreshDetailLabel(true, ZERO_TIME)).toBe('pending')
|
||||
})
|
||||
|
||||
it('reads the full formatTimestamp date/time once discovery has completed a refresh, not a relative "(Ns ago)"', () => {
|
||||
const iso = new Date(2026, 7, 21, 12, 0, 0).toISOString()
|
||||
const got = refreshDetailLabel(true, iso)
|
||||
expect(got).not.toMatch(/ago/)
|
||||
expect(got).not.toBe('pending')
|
||||
expect(got).not.toBe('discovery off')
|
||||
})
|
||||
})
|
||||
|
||||
+49
-14
@@ -40,22 +40,57 @@ export function formatTimestamp(iso: string | undefined): string {
|
||||
}
|
||||
|
||||
/**
|
||||
* refreshLabel renders a provider's discovery/refresh status as the
|
||||
* Providers tab's own three-state label (Feature B, v0.22 — last-refresh
|
||||
* label honesty): discovery disabled reads "discovery off" regardless of
|
||||
* lastRefresh (a disabled provider's lastRefresh is always the unset
|
||||
* sentinel, registry.go's own maybeRefresh/warmFill never call
|
||||
* finishRefresh for it — but this checks discoveryEnabled first anyway,
|
||||
* not lastRefresh's value, so it stays correct even if that invariant
|
||||
* ever changes); discovery enabled but never yet refreshed reads
|
||||
* "pending"; otherwise it is the existing "refreshed (Ns ago)" relative-
|
||||
* time text, unchanged.
|
||||
* refreshState is the Providers tab's three-state refresh classification
|
||||
* (Feature B, v0.22 — last-refresh label honesty), shared by refreshLabel
|
||||
* (the accordion trigger's compact text) and refreshDetailLabel (the
|
||||
* accordion content's "Last refresh" detail row — folded review minor,
|
||||
* v0.22 review round: that row still read raw formatTimestamp before this
|
||||
* existed, so a discovery-off provider's expanded detail said "never"
|
||||
* right next to a trigger that correctly said "discovery off"):
|
||||
* discovery disabled reads 'off' regardless of lastRefresh (a disabled
|
||||
* provider's lastRefresh is always the unset sentinel, registry.go's own
|
||||
* maybeRefresh/warmFill never call finishRefresh for it — but this checks
|
||||
* discoveryEnabled first anyway, not lastRefresh's value, so it stays
|
||||
* correct even if that invariant ever changes); discovery enabled but
|
||||
* never yet refreshed reads 'pending'; otherwise 'refreshed'.
|
||||
*/
|
||||
function refreshState(discoveryEnabled: boolean, lastRefresh: string | undefined): 'off' | 'pending' | 'refreshed' {
|
||||
if (!discoveryEnabled) return 'off'
|
||||
return formatAgo(lastRefresh) ? 'refreshed' : 'pending'
|
||||
}
|
||||
|
||||
/**
|
||||
* refreshLabel renders refreshState as the Providers tab's compact
|
||||
* trigger-row text: "discovery off", "pending", or the existing
|
||||
* "refreshed (Ns ago)" relative-time text.
|
||||
*/
|
||||
export function refreshLabel(discoveryEnabled: boolean, lastRefresh: string | undefined): string {
|
||||
if (!discoveryEnabled) return 'discovery off'
|
||||
const ago = formatAgo(lastRefresh)
|
||||
if (!ago) return 'pending'
|
||||
return `refreshed${ago}`
|
||||
switch (refreshState(discoveryEnabled, lastRefresh)) {
|
||||
case 'off':
|
||||
return 'discovery off'
|
||||
case 'pending':
|
||||
return 'pending'
|
||||
case 'refreshed':
|
||||
return `refreshed${formatAgo(lastRefresh)}`
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* refreshDetailLabel renders refreshState as the Providers tab's
|
||||
* accordion-content "Last refresh" detail row: "discovery off", "pending",
|
||||
* or the full formatTimestamp date/time (this row's own pre-existing
|
||||
* convention — refreshLabel's relative "(Ns ago)" belongs on the compact
|
||||
* trigger row only).
|
||||
*/
|
||||
export function refreshDetailLabel(discoveryEnabled: boolean, lastRefresh: string | undefined): string {
|
||||
switch (refreshState(discoveryEnabled, lastRefresh)) {
|
||||
case 'off':
|
||||
return 'discovery off'
|
||||
case 'pending':
|
||||
return 'pending'
|
||||
case 'refreshed':
|
||||
return formatTimestamp(lastRefresh)
|
||||
}
|
||||
}
|
||||
|
||||
/** formatLimits renders a LimitsConfig as a short comma-joined summary, or "none" when unset. */
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import {
|
||||
dayRateTitle,
|
||||
formatRatePercent,
|
||||
isModelDegraded,
|
||||
minuteRateTitle,
|
||||
@@ -30,6 +31,15 @@ describe('providerSuccessRate', () => {
|
||||
it('returns null for a negative attempts count (defensive edge case)', () => {
|
||||
expect(providerSuccessRate(-1, 0)).toBeNull()
|
||||
})
|
||||
|
||||
it('returns null for a non-finite attempts count (folded review minor: NaN-guard the badge props)', () => {
|
||||
expect(providerSuccessRate(Number.NaN, 0)).toBeNull()
|
||||
expect(providerSuccessRate(Number.POSITIVE_INFINITY, 0)).toBeNull()
|
||||
})
|
||||
|
||||
it('treats a non-finite failures count as 0 rather than propagating NaN', () => {
|
||||
expect(providerSuccessRate(10, Number.NaN)).toBe(1)
|
||||
})
|
||||
})
|
||||
|
||||
describe('providerRateStatus', () => {
|
||||
@@ -81,10 +91,19 @@ describe('isModelDegraded', () => {
|
||||
})
|
||||
|
||||
describe('formatRatePercent', () => {
|
||||
it('rounds to the nearest whole percent', () => {
|
||||
it('floors rather than rounds, so only a true 100% rate ever shows 100%', () => {
|
||||
expect(formatRatePercent(0.994)).toBe('99%')
|
||||
expect(formatRatePercent(0.995)).toBe('100%')
|
||||
// Round would have bumped this to "100%" (99.5 rounds up) — floor
|
||||
// correctly keeps it "99%": the rate is not actually a perfect 100%.
|
||||
expect(formatRatePercent(0.995)).toBe('99%')
|
||||
expect(formatRatePercent(0.999)).toBe('99%')
|
||||
})
|
||||
|
||||
it('shows 100% only for an exact rate of 1', () => {
|
||||
expect(formatRatePercent(1)).toBe('100%')
|
||||
})
|
||||
|
||||
it('floors 0 to "0%"', () => {
|
||||
expect(formatRatePercent(0)).toBe('0%')
|
||||
})
|
||||
})
|
||||
@@ -98,11 +117,29 @@ describe('minuteRateTitle', () => {
|
||||
expect(minuteRateTitle(1, 1)).toBe('1 attempt, 1 failure in the last minute (0%)')
|
||||
})
|
||||
|
||||
it('pluralizes attempts independently of a singular failure count, with the rounded percent', () => {
|
||||
expect(minuteRateTitle(12, 1)).toBe('12 attempts, 1 failure in the last minute (92%)')
|
||||
it('pluralizes attempts independently of a singular failure count, with the floored percent', () => {
|
||||
expect(minuteRateTitle(12, 1)).toBe('12 attempts, 1 failure in the last minute (91%)')
|
||||
})
|
||||
|
||||
it('omits failures from wording only via the count itself (0 failures still says "0 failures")', () => {
|
||||
expect(minuteRateTitle(5, 0)).toBe('5 attempts, 0 failures in the last minute (100%)')
|
||||
})
|
||||
})
|
||||
|
||||
describe('dayRateTitle', () => {
|
||||
it('reports no traffic distinctly when attemptsDay is 0 (per-model badge fallback, SHOULD-2)', () => {
|
||||
expect(dayRateTitle(0, 0)).toBe('no traffic today')
|
||||
})
|
||||
|
||||
it('uses singular attempt/failure wording for a count of exactly 1', () => {
|
||||
expect(dayRateTitle(1, 1)).toBe('1 attempt, 1 failure today (0%)')
|
||||
})
|
||||
|
||||
it('pluralizes independently, with the floored percent', () => {
|
||||
expect(dayRateTitle(12, 1)).toBe('12 attempts, 1 failure today (91%)')
|
||||
})
|
||||
|
||||
it('says "today" rather than "in the last minute" — the only wording difference from minuteRateTitle', () => {
|
||||
expect(dayRateTitle(5, 0)).toBe('5 attempts, 0 failures today (100%)')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -34,14 +34,19 @@ export type ProviderRateStatus = 'no-traffic' | 'healthy' | 'degraded' | 'severe
|
||||
|
||||
/**
|
||||
* providerSuccessRate computes the (attempts-failures)/attempts success
|
||||
* rate as a 0-1 fraction, or null when attempts is 0 or negative — the
|
||||
* "no traffic yet" case a caller must render as its own state, never as a
|
||||
* fraction (a 0/0 division would otherwise read as NaN, or worse, as a
|
||||
* misleading 100%).
|
||||
* rate as a 0-1 fraction, or null when attempts is 0, negative, or not a
|
||||
* finite number — the "no traffic yet" case a caller must render as its
|
||||
* own state, never as a fraction (a 0/0 division would otherwise read as
|
||||
* NaN, or worse, as a misleading 100%). failures is defensively treated
|
||||
* as 0 when it is not a finite number (folded review minor, v0.22 review
|
||||
* round): every real caller sources both counters from the same
|
||||
* admin.go response, so this only guards a version-skew or malformed-
|
||||
* response case, not a path normal operation reaches.
|
||||
*/
|
||||
export function providerSuccessRate(attempts: number, failures: number): number | null {
|
||||
if (attempts <= 0) return null
|
||||
return (attempts - failures) / attempts
|
||||
if (!Number.isFinite(attempts) || attempts <= 0) return null
|
||||
const safeFailures = Number.isFinite(failures) ? failures : 0
|
||||
return (attempts - safeFailures) / attempts
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -69,24 +74,56 @@ export function isModelDegraded(attemptsDay: number, failuresDay: number): boole
|
||||
return rate !== null && rate < PROVIDER_RATE_AMBER_THRESHOLD
|
||||
}
|
||||
|
||||
/** formatRatePercent renders a 0-1 fraction as a rounded whole-percent string, e.g. 0.994 -> "99%", 1 -> "100%". */
|
||||
/**
|
||||
* formatRatePercent renders a 0-1 fraction as a whole-percent string,
|
||||
* FLOORED rather than rounded (folded review minor, v0.22 review round):
|
||||
* only a true 100% rate — zero failures — ever renders "100%"; a rate
|
||||
* like 0.996 (unrounded, sub-1% failure) renders "99%", not a
|
||||
* round-tripped "100%" that would read as flawless when it is not quite.
|
||||
* A rate at or above 1 (the only way to reach exactly 100%, given
|
||||
* providerSuccessRate's own (attempts-failures)/attempts shape) is
|
||||
* special-cased rather than left to Math.floor's own rounding, purely for
|
||||
* clarity at the boundary.
|
||||
*/
|
||||
export function formatRatePercent(rate: number): string {
|
||||
return `${Math.round(rate * 100)}%`
|
||||
if (rate >= 1) return '100%'
|
||||
return `${Math.floor(rate * 100)}%`
|
||||
}
|
||||
|
||||
/** countWord returns singular when n is exactly 1, plural otherwise — shared by minuteRateTitle and dayRateTitle so their wording never drifts apart. */
|
||||
function countWord(n: number, singular: string, plural: string): string {
|
||||
return n === 1 ? singular : plural
|
||||
}
|
||||
|
||||
/**
|
||||
* minuteRateTitle builds the badge's title attribute — the "right now"
|
||||
* minute-window detail the spec asks for beside the day-window badge text
|
||||
* itself, e.g. "12 attempts, 1 failure in the last minute (92%)". Singular/
|
||||
* plural wording follows the actual count, and a zero-attempt minute reads
|
||||
* as its own sentence rather than a "0%" that would misread as a total
|
||||
* outage.
|
||||
* minuteRateTitle builds the provider-level badge's title/aria-label —
|
||||
* the "right now" minute-window detail the spec asks for beside the
|
||||
* day-window badge text itself, e.g. "12 attempts, 1 failure in the last
|
||||
* minute (92%)". Singular/plural wording follows the actual count, and a
|
||||
* zero-attempt minute reads as its own sentence rather than a "0%" that
|
||||
* would misread as a total outage. Provider-level only — a per-model
|
||||
* badge no longer has minute-window counters to build this from at all
|
||||
* (SHOULD-2, v0.22 review round: see dayRateTitle, its own fallback).
|
||||
*/
|
||||
export function minuteRateTitle(attemptsMinute: number, failuresMinute: number): string {
|
||||
if (attemptsMinute <= 0) return 'no traffic in the last minute'
|
||||
const rate = providerSuccessRate(attemptsMinute, failuresMinute)
|
||||
const pct = rate === null ? '' : ` (${formatRatePercent(rate)})`
|
||||
const attemptWord = attemptsMinute === 1 ? 'attempt' : 'attempts'
|
||||
const failureWord = failuresMinute === 1 ? 'failure' : 'failures'
|
||||
return `${attemptsMinute} ${attemptWord}, ${failuresMinute} ${failureWord} in the last minute${pct}`
|
||||
return `${attemptsMinute} ${countWord(attemptsMinute, 'attempt', 'attempts')}, ${failuresMinute} ${countWord(failuresMinute, 'failure', 'failures')} in the last minute${pct}`
|
||||
}
|
||||
|
||||
/**
|
||||
* dayRateTitle is minuteRateTitle's day-window equivalent — the per-model
|
||||
* badge's title/aria-label (SHOULD-2, v0.22 review round): a model
|
||||
* scope's minute-window counters are no longer fetched anywhere
|
||||
* (admin.go's buildAdminOverview, limits.go's providerCounterKeys), so a
|
||||
* per-model badge cannot build minuteRateTitle's "in the last minute"
|
||||
* sentence — this is the "today" equivalent, built from the same day
|
||||
* counters the badge's own visible percent already uses.
|
||||
*/
|
||||
export function dayRateTitle(attemptsDay: number, failuresDay: number): string {
|
||||
if (attemptsDay <= 0) return 'no traffic today'
|
||||
const rate = providerSuccessRate(attemptsDay, failuresDay)
|
||||
const pct = rate === null ? '' : ` (${formatRatePercent(rate)})`
|
||||
return `${attemptsDay} ${countWord(attemptsDay, 'attempt', 'attempts')}, ${failuresDay} ${countWord(failuresDay, 'failure', 'failures')} today${pct}`
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user