Procházet zdrojové kódy

fix: address code review findings for cache hit rate alert

- Fix costAlertCheckInterval null producing invalid cron in legacy mode
- Cap historicalLookbackDays to 90 to prevent unbounded DB queries
- Strengthen CacheHitRateAlertWindow.mode type from string to union
- Complete default webhook template with all 12 documented placeholders
- Add trailing newline to migration file (POSIX compliance)
- Replace safeNumberOnChange with NumberInput component to fix UX issue
  where clearing a number input via backspace caused value snap-back
ding113 před 1 měsícem
rodič
revize
674813d833

+ 1 - 1
drizzle/0076_mighty_lionheart.sql

@@ -10,4 +10,4 @@ ALTER TABLE "notification_settings" ADD COLUMN "cache_hit_rate_alert_abs_min" nu
 ALTER TABLE "notification_settings" ADD COLUMN "cache_hit_rate_alert_drop_rel" numeric(5, 4) DEFAULT '0.3';--> statement-breakpoint
 ALTER TABLE "notification_settings" ADD COLUMN "cache_hit_rate_alert_drop_abs" numeric(5, 4) DEFAULT '0.1';--> statement-breakpoint
 ALTER TABLE "notification_settings" ADD COLUMN "cache_hit_rate_alert_cooldown_minutes" integer DEFAULT 30;--> statement-breakpoint
-ALTER TABLE "notification_settings" ADD COLUMN "cache_hit_rate_alert_top_n" integer DEFAULT 10;
+ALTER TABLE "notification_settings" ADD COLUMN "cache_hit_rate_alert_top_n" integer DEFAULT 10;

+ 93 - 78
src/app/[locale]/settings/notifications/_components/notification-type-card.tsx

@@ -3,7 +3,7 @@
 import { AlertTriangle, Database, DollarSign, Settings2, TrendingUp } from "lucide-react";
 import { useTranslations } from "next-intl";
 import type { ComponentProps, ReactNode } from "react";
-import { useMemo } from "react";
+import { useEffect, useMemo, useState } from "react";
 import { Badge } from "@/components/ui/badge";
 import { Switch } from "@/components/ui/switch";
 import { cn } from "@/lib/utils";
@@ -124,6 +124,53 @@ function createSettingsPatch<K extends keyof NotificationSettingsState>(
   return { [key]: value } as Pick<NotificationSettingsState, K>;
 }
 
+/**
+ * Controlled number input that allows temporary empty state while editing.
+ *
+ * The standard pattern of `value={state}` + `onChange={guard}` on `<input type="number">`
+ * causes the input to "snap back" when the user clears it (backspace), because `valueAsNumber`
+ * is `NaN` and the guard rejects the update. This component uses local string state to allow
+ * the field to be cleared, then reverts to the last valid value on blur.
+ */
+function NumberInput({
+  value,
+  onValueChange,
+  constraints,
+  ...inputProps
+}: Omit<ComponentProps<"input">, "value" | "onChange" | "type"> & {
+  value: number;
+  onValueChange: (value: number) => void;
+  constraints?: NumberInputConstraints;
+}) {
+  const [localValue, setLocalValue] = useState(String(value));
+
+  useEffect(() => {
+    setLocalValue(String(value));
+  }, [value]);
+
+  return (
+    <input
+      {...inputProps}
+      type="number"
+      value={localValue}
+      onChange={(e) => {
+        const raw = e.currentTarget.value;
+        setLocalValue(raw);
+
+        const num = e.currentTarget.valueAsNumber;
+        if (!Number.isFinite(num)) return;
+        if (constraints?.integer && !Number.isInteger(num)) return;
+        if (constraints?.min !== undefined && num < constraints.min) return;
+        if (constraints?.max !== undefined && num > constraints.max) return;
+        onValueChange(num);
+      }}
+      onBlur={() => {
+        setLocalValue(String(value));
+      }}
+    />
+  );
+}
+
 export function NotificationTypeCard({
   type,
   settings,
@@ -280,17 +327,14 @@ export function NotificationTypeCard({
                 >
                   {t("notifications.dailyLeaderboard.topN")}
                 </label>
-                <input
+                <NumberInput
                   id="dailyLeaderboardTopN"
-                  type="number"
                   min={1}
                   max={20}
                   value={settings.dailyLeaderboardTopN}
                   disabled={!settings.enabled}
-                  onChange={safeNumberOnChange(
-                    (nextValue) => onUpdateSettings({ dailyLeaderboardTopN: nextValue }),
-                    { integer: true, min: 1, max: 20 }
-                  )}
+                  onValueChange={(v) => onUpdateSettings({ dailyLeaderboardTopN: v })}
+                  constraints={{ integer: true, min: 1, max: 20 }}
                   className={cn(
                     "w-full bg-muted/50 border border-border rounded-lg py-2 px-3 text-sm text-foreground",
                     "focus:border-primary focus:ring-1 focus:ring-primary outline-none transition-all",
@@ -333,17 +377,14 @@ export function NotificationTypeCard({
                 >
                   {t("notifications.costAlert.interval")}
                 </label>
-                <input
+                <NumberInput
                   id="costAlertCheckInterval"
-                  type="number"
                   min={10}
                   max={1440}
                   value={settings.costAlertCheckInterval}
                   disabled={!settings.enabled}
-                  onChange={safeNumberOnChange(
-                    (nextValue) => onUpdateSettings({ costAlertCheckInterval: nextValue }),
-                    { integer: true, min: 10, max: 1440 }
-                  )}
+                  onValueChange={(v) => onUpdateSettings({ costAlertCheckInterval: v })}
+                  constraints={{ integer: true, min: 10, max: 1440 }}
                   className={cn(
                     "w-full bg-muted/50 border border-border rounded-lg py-2 px-3 text-sm text-foreground",
                     "focus:border-primary focus:ring-1 focus:ring-primary outline-none transition-all",
@@ -386,18 +427,14 @@ export function NotificationTypeCard({
                   id="cacheHitRateAlertCheckInterval"
                   label={t("notifications.cacheHitRateAlert.checkInterval")}
                 >
-                  <input
+                  <NumberInput
                     id="cacheHitRateAlertCheckInterval"
-                    type="number"
                     min={1}
                     max={1440}
                     value={settings.cacheHitRateAlertCheckInterval}
                     disabled={!settings.enabled}
-                    onChange={safeNumberOnChange(
-                      (nextValue) =>
-                        onUpdateSettings({ cacheHitRateAlertCheckInterval: nextValue }),
-                      { integer: true, min: 1, max: 1440 }
-                    )}
+                    onValueChange={(v) => onUpdateSettings({ cacheHitRateAlertCheckInterval: v })}
+                    constraints={{ integer: true, min: 1, max: 1440 }}
                     className={settingsControlClassName}
                   />
                 </LabeledControl>
@@ -406,20 +443,18 @@ export function NotificationTypeCard({
                   id="cacheHitRateAlertHistoricalLookbackDays"
                   label={t("notifications.cacheHitRateAlert.historicalLookbackDays")}
                 >
-                  <input
+                  <NumberInput
                     id="cacheHitRateAlertHistoricalLookbackDays"
-                    type="number"
                     min={1}
                     max={90}
                     value={settings.cacheHitRateAlertHistoricalLookbackDays}
                     disabled={!settings.enabled}
-                    onChange={safeNumberOnChange(
-                      (nextValue) =>
-                        onUpdateSettings({
-                          cacheHitRateAlertHistoricalLookbackDays: nextValue,
-                        }),
-                      { integer: true, min: 1, max: 90 }
-                    )}
+                    onValueChange={(v) =>
+                      onUpdateSettings({
+                        cacheHitRateAlertHistoricalLookbackDays: v,
+                      })
+                    }
+                    constraints={{ integer: true, min: 1, max: 90 }}
                     className={settingsControlClassName}
                   />
                 </LabeledControl>
@@ -428,18 +463,14 @@ export function NotificationTypeCard({
                   id="cacheHitRateAlertCooldownMinutes"
                   label={t("notifications.cacheHitRateAlert.cooldownMinutes")}
                 >
-                  <input
+                  <NumberInput
                     id="cacheHitRateAlertCooldownMinutes"
-                    type="number"
                     min={0}
                     max={1440}
                     value={settings.cacheHitRateAlertCooldownMinutes}
                     disabled={!settings.enabled}
-                    onChange={safeNumberOnChange(
-                      (nextValue) =>
-                        onUpdateSettings({ cacheHitRateAlertCooldownMinutes: nextValue }),
-                      { integer: true, min: 0, max: 1440 }
-                    )}
+                    onValueChange={(v) => onUpdateSettings({ cacheHitRateAlertCooldownMinutes: v })}
+                    constraints={{ integer: true, min: 0, max: 1440 }}
                     className={settingsControlClassName}
                   />
                 </LabeledControl>
@@ -450,18 +481,15 @@ export function NotificationTypeCard({
                   id="cacheHitRateAlertAbsMin"
                   label={t("notifications.cacheHitRateAlert.absMin")}
                 >
-                  <input
+                  <NumberInput
                     id="cacheHitRateAlertAbsMin"
-                    type="number"
                     min={0}
                     max={1}
                     step={0.01}
                     value={settings.cacheHitRateAlertAbsMin}
                     disabled={!settings.enabled}
-                    onChange={safeNumberOnChange(
-                      (nextValue) => onUpdateSettings({ cacheHitRateAlertAbsMin: nextValue }),
-                      { min: 0, max: 1 }
-                    )}
+                    onValueChange={(v) => onUpdateSettings({ cacheHitRateAlertAbsMin: v })}
+                    constraints={{ min: 0, max: 1 }}
                     className={settingsControlClassName}
                   />
                 </LabeledControl>
@@ -470,18 +498,15 @@ export function NotificationTypeCard({
                   id="cacheHitRateAlertDropAbs"
                   label={t("notifications.cacheHitRateAlert.dropAbs")}
                 >
-                  <input
+                  <NumberInput
                     id="cacheHitRateAlertDropAbs"
-                    type="number"
                     min={0}
                     max={1}
                     step={0.01}
                     value={settings.cacheHitRateAlertDropAbs}
                     disabled={!settings.enabled}
-                    onChange={safeNumberOnChange(
-                      (nextValue) => onUpdateSettings({ cacheHitRateAlertDropAbs: nextValue }),
-                      { min: 0, max: 1 }
-                    )}
+                    onValueChange={(v) => onUpdateSettings({ cacheHitRateAlertDropAbs: v })}
+                    constraints={{ min: 0, max: 1 }}
                     className={settingsControlClassName}
                   />
                 </LabeledControl>
@@ -490,18 +515,15 @@ export function NotificationTypeCard({
                   id="cacheHitRateAlertDropRel"
                   label={t("notifications.cacheHitRateAlert.dropRel")}
                 >
-                  <input
+                  <NumberInput
                     id="cacheHitRateAlertDropRel"
-                    type="number"
                     min={0}
                     max={1}
                     step={0.01}
                     value={settings.cacheHitRateAlertDropRel}
                     disabled={!settings.enabled}
-                    onChange={safeNumberOnChange(
-                      (nextValue) => onUpdateSettings({ cacheHitRateAlertDropRel: nextValue }),
-                      { min: 0, max: 1 }
-                    )}
+                    onValueChange={(v) => onUpdateSettings({ cacheHitRateAlertDropRel: v })}
+                    constraints={{ min: 0, max: 1 }}
                     className={settingsControlClassName}
                   />
                 </LabeledControl>
@@ -512,20 +534,18 @@ export function NotificationTypeCard({
                   id="cacheHitRateAlertMinEligibleRequests"
                   label={t("notifications.cacheHitRateAlert.minEligibleRequests")}
                 >
-                  <input
+                  <NumberInput
                     id="cacheHitRateAlertMinEligibleRequests"
-                    type="number"
                     min={1}
                     max={100000}
                     value={settings.cacheHitRateAlertMinEligibleRequests}
                     disabled={!settings.enabled}
-                    onChange={safeNumberOnChange(
-                      (nextValue) =>
-                        onUpdateSettings({
-                          cacheHitRateAlertMinEligibleRequests: nextValue,
-                        }),
-                      { integer: true, min: 1, max: 100000 }
-                    )}
+                    onValueChange={(v) =>
+                      onUpdateSettings({
+                        cacheHitRateAlertMinEligibleRequests: v,
+                      })
+                    }
+                    constraints={{ integer: true, min: 1, max: 100000 }}
                     className={settingsControlClassName}
                   />
                 </LabeledControl>
@@ -534,19 +554,17 @@ export function NotificationTypeCard({
                   id="cacheHitRateAlertMinEligibleTokens"
                   label={t("notifications.cacheHitRateAlert.minEligibleTokens")}
                 >
-                  <input
+                  <NumberInput
                     id="cacheHitRateAlertMinEligibleTokens"
-                    type="number"
                     min={0}
                     value={settings.cacheHitRateAlertMinEligibleTokens}
                     disabled={!settings.enabled}
-                    onChange={safeNumberOnChange(
-                      (nextValue) =>
-                        onUpdateSettings({
-                          cacheHitRateAlertMinEligibleTokens: nextValue,
-                        }),
-                      { integer: true, min: 0 }
-                    )}
+                    onValueChange={(v) =>
+                      onUpdateSettings({
+                        cacheHitRateAlertMinEligibleTokens: v,
+                      })
+                    }
+                    constraints={{ integer: true, min: 0 }}
                     className={settingsControlClassName}
                   />
                 </LabeledControl>
@@ -555,17 +573,14 @@ export function NotificationTypeCard({
                   id="cacheHitRateAlertTopN"
                   label={t("notifications.cacheHitRateAlert.topN")}
                 >
-                  <input
+                  <NumberInput
                     id="cacheHitRateAlertTopN"
-                    type="number"
                     min={1}
                     max={100}
                     value={settings.cacheHitRateAlertTopN}
                     disabled={!settings.enabled}
-                    onChange={safeNumberOnChange(
-                      (nextValue) => onUpdateSettings({ cacheHitRateAlertTopN: nextValue }),
-                      { integer: true, min: 1, max: 100 }
-                    )}
+                    onValueChange={(v) => onUpdateSettings({ cacheHitRateAlertTopN: v })}
+                    constraints={{ integer: true, min: 1, max: 100 }}
                     className={settingsControlClassName}
                   />
                 </LabeledControl>

+ 1 - 1
src/lib/notification/notification-queue.ts

@@ -764,7 +764,7 @@ export async function scheduleNotifications() {
       }
 
       if (settings.costAlertEnabled && settings.costAlertWebhook) {
-        const interval = settings.costAlertCheckInterval; // 分钟
+        const interval = settings.costAlertCheckInterval ?? 60; // 分钟
         const cron = `*/${interval} * * * *`; // 每 N 分钟
 
         await queue.add(

+ 7 - 2
src/lib/notification/tasks/cache-hit-rate-alert.ts

@@ -13,6 +13,7 @@ import type {
   CacheHitRateAlertSettingsSnapshot,
   CacheHitRateAlertWindow,
 } from "@/lib/webhook";
+import type { CacheHitRateAlertSettingsWindowMode } from "@/lib/webhook/types";
 import { findProviderModelCacheHitRateMetricsForAlert } from "@/repository/cache-hit-rate-alert";
 import { getNotificationSettings } from "@/repository/notifications";
 import { findAllProviders } from "@/repository/provider";
@@ -44,7 +45,7 @@ function resolveWindowMode(
   mode: string | null | undefined,
   intervalMinutes: number
 ): {
-  mode: string;
+  mode: CacheHitRateAlertSettingsWindowMode;
   durationMinutes: number;
 } {
   switch (mode) {
@@ -218,7 +219,11 @@ export async function generateCacheHitRateAlertPayload(
   const dedupMode: CacheHitRateAlertDedupMode = options.dedupMode ?? "global";
 
   const intervalMinutes = Math.max(1, parseIntNumber(settings.cacheHitRateAlertCheckInterval, 5));
-  const lookbackDays = parseIntNumber(settings.cacheHitRateAlertHistoricalLookbackDays, 7);
+  const MAX_LOOKBACK_DAYS = 90;
+  const lookbackDays = Math.min(
+    parseIntNumber(settings.cacheHitRateAlertHistoricalLookbackDays, 7),
+    MAX_LOOKBACK_DAYS
+  );
   const cooldownMinutes = parseIntNumber(settings.cacheHitRateAlertCooldownMinutes, 30);
 
   const decisionSettings: CacheHitRateAlertDecisionSettings = {

+ 7 - 0
src/lib/webhook/templates/defaults.ts

@@ -40,7 +40,14 @@ export const DEFAULT_TEMPLATES = {
     windowStart: "{{window_start}}",
     windowEnd: "{{window_end}}",
     anomalyCount: "{{anomaly_count}}",
+    suppressedCount: "{{suppressed_count}}",
     anomalies: "{{anomalies_json}}",
+    absMin: "{{abs_min}}",
+    dropRel: "{{drop_rel}}",
+    dropAbs: "{{drop_abs}}",
+    cooldownMinutes: "{{cooldown_minutes}}",
+    topN: "{{top_n}}",
+    generatedAt: "{{generated_at}}",
   },
 } as const;
 

+ 1 - 1
src/lib/webhook/types.ts

@@ -125,7 +125,7 @@ export interface CacheHitRateAlertAnomaly {
 }
 
 export interface CacheHitRateAlertWindow {
-  mode: string;
+  mode: CacheHitRateAlertSettingsWindowMode;
   startTime: string;
   endTime: string;
   durationMinutes: number;