-
-
Notifications
You must be signed in to change notification settings - Fork 181
Fix/logs provider badge overflow #581
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
59a06cb
753a5dc
46eacda
132f37d
edf2376
d8d0a89
0be62a5
179db42
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -536,16 +536,20 @@ export function ErrorDetailsDialog({ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| )} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {costMultiplier && parseFloat(String(costMultiplier)) !== 1.0 && ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div className="flex justify-between"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <span className="text-muted-foreground"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {t("logs.billingDetails.multiplier")}: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </span> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <span className="font-mono"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {parseFloat(String(costMultiplier)).toFixed(2)}x | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </span> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| )} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (costMultiplier === "" || costMultiplier == null) return null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const multiplier = Number(costMultiplier); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!Number.isFinite(multiplier) || multiplier === 1) return null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div className="flex justify-between"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <span className="text-muted-foreground"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {t("logs.billingDetails.multiplier")}: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </span> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <span className="font-mono">{multiplier.toFixed(2)}x</span> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| })()} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+539
to
+552
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 空白字符串会被当作 0 渲染为 当前仅排除了 建议修改(仅限本段)- {(() => {
- if (costMultiplier === "" || costMultiplier == null) return null;
- const multiplier = Number(costMultiplier);
+ {(() => {
+ if (costMultiplier == null) return null;
+ const raw = costMultiplier.trim();
+ if (raw === "") return null;
+ const multiplier = Number(raw);
if (!Number.isFinite(multiplier) || multiplier === 1) return null;
return (
<div className="flex justify-between">
<span className="text-muted-foreground">
{t("logs.billingDetails.multiplier")}:
</span>
<span className="font-mono">{multiplier.toFixed(2)}x</span>
</div>
);
})()}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div className="mt-3 pt-3 border-t flex justify-between items-center"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <span className="font-medium">{t("logs.billingDetails.totalCost")}:</span> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,149 @@ | ||
| import type { ReactNode } from "react"; | ||
| import { renderToStaticMarkup } from "react-dom/server"; | ||
| import { NextIntlClientProvider } from "next-intl"; | ||
| import { Window } from "happy-dom"; | ||
| import { describe, expect, test, vi } from "vitest"; | ||
|
|
||
| vi.mock("@/lib/utils/provider-chain-formatter", () => ({ | ||
| formatProviderDescription: () => "provider description", | ||
| })); | ||
|
|
||
| vi.mock("@/components/ui/tooltip", () => { | ||
| type PropsWithChildren = { children?: ReactNode }; | ||
|
|
||
| function TooltipProvider({ children }: PropsWithChildren) { | ||
| return <div data-slot="tooltip-provider">{children}</div>; | ||
| } | ||
|
|
||
| function Tooltip({ children }: PropsWithChildren) { | ||
| return <div data-slot="tooltip-root">{children}</div>; | ||
| } | ||
|
|
||
| function TooltipTrigger({ children }: PropsWithChildren) { | ||
| return <div data-slot="tooltip-trigger">{children}</div>; | ||
| } | ||
|
|
||
| function TooltipContent({ children }: PropsWithChildren) { | ||
| return <div data-slot="tooltip-content">{children}</div>; | ||
| } | ||
|
|
||
| return { TooltipProvider, Tooltip, TooltipTrigger, TooltipContent }; | ||
| }); | ||
|
|
||
| vi.mock("@/components/ui/popover", () => { | ||
| type PropsWithChildren = { children?: ReactNode }; | ||
|
|
||
| function Popover({ children }: PropsWithChildren) { | ||
| return <div data-slot="popover-root">{children}</div>; | ||
| } | ||
|
|
||
| function PopoverTrigger({ children }: PropsWithChildren) { | ||
| return <div data-slot="popover-trigger">{children}</div>; | ||
| } | ||
|
|
||
| function PopoverContent({ children }: PropsWithChildren) { | ||
| return <div data-slot="popover-content">{children}</div>; | ||
| } | ||
|
|
||
| return { Popover, PopoverTrigger, PopoverContent }; | ||
| }); | ||
|
|
||
| vi.mock("@/components/ui/button", () => ({ | ||
| Button: ({ | ||
| children, | ||
| className, | ||
| ...props | ||
| }: React.ComponentProps<"button"> & { variant?: string }) => ( | ||
| <button className={className} {...props}> | ||
| {children} | ||
| </button> | ||
| ), | ||
| })); | ||
|
|
||
| vi.mock("@/components/ui/badge", () => ({ | ||
| Badge: ({ children, className }: React.ComponentProps<"span"> & { variant?: string }) => ( | ||
| <span data-slot="badge" className={className}> | ||
| {children} | ||
| </span> | ||
| ), | ||
| })); | ||
|
|
||
| import { ProviderChainPopover } from "./provider-chain-popover"; | ||
|
|
||
| const messages = { | ||
| dashboard: { | ||
| logs: { | ||
| table: { | ||
| times: "times", | ||
| }, | ||
| providerChain: { | ||
| decisionChain: "Decision chain", | ||
| }, | ||
| details: { | ||
| clickStatusCode: "Click status code", | ||
| }, | ||
| }, | ||
| }, | ||
| "provider-chain": {}, | ||
| }; | ||
|
|
||
| function renderWithIntl(node: ReactNode) { | ||
| return renderToStaticMarkup( | ||
| <NextIntlClientProvider locale="en" messages={messages} timeZone="UTC"> | ||
| <div id="root">{node}</div> | ||
| </NextIntlClientProvider> | ||
| ); | ||
| } | ||
|
|
||
| function parseHtml(html: string) { | ||
| const window = new Window(); | ||
| window.document.body.innerHTML = html; | ||
| return window.document; | ||
| } | ||
|
|
||
| describe("provider-chain-popover layout", () => { | ||
| test("requestCount<=1 branch keeps truncation container shrinkable", () => { | ||
| const html = renderWithIntl( | ||
| <ProviderChainPopover | ||
| chain={[{ id: 1, name: "p1", reason: "request_success", statusCode: 200 }]} | ||
| finalProvider={"Very long provider name that should truncate"} | ||
| /> | ||
| ); | ||
| const document = parseHtml(html); | ||
|
|
||
| const container = document.querySelector("#root > div"); | ||
| const containerClass = container?.getAttribute("class") ?? ""; | ||
| expect(containerClass).toContain("min-w-0"); | ||
| expect(containerClass).toContain("w-full"); | ||
|
|
||
| const truncateNode = document.querySelector("#root span.truncate"); | ||
| expect(truncateNode).not.toBeNull(); | ||
| }); | ||
|
|
||
| test("requestCount>1 branch uses w-full/min-w-0 button and flex-1 name container", () => { | ||
| const html = renderWithIntl( | ||
| <ProviderChainPopover | ||
| chain={[ | ||
| { id: 1, name: "p1", reason: "retry_failed" }, | ||
| { id: 2, name: "p2", reason: "request_success", statusCode: 200 }, | ||
| ]} | ||
| finalProvider={"Very long provider name that should truncate"} | ||
| /> | ||
| ); | ||
| const document = parseHtml(html); | ||
|
|
||
| const button = document.querySelector("#root button"); | ||
| expect(button).not.toBeNull(); | ||
| const buttonClass = button?.getAttribute("class") ?? ""; | ||
| expect(buttonClass).toContain("w-full"); | ||
| expect(buttonClass).toContain("min-w-0"); | ||
|
|
||
| const nameContainer = document.querySelector("#root button .flex-1.min-w-0"); | ||
| expect(nameContainer).not.toBeNull(); | ||
|
|
||
| const countBadge = Array.from(document.querySelectorAll('#root [data-slot="badge"]')).find( | ||
| (node) => (node.getAttribute("class") ?? "").includes("ml-1") | ||
| ); | ||
| expect(countBadge).not.toBeUndefined(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,184 @@ | ||
| import { renderToStaticMarkup } from "react-dom/server"; | ||
| import type { ReactNode } from "react"; | ||
| import { createRoot } from "react-dom/client"; | ||
| import { act } from "react"; | ||
| import { describe, expect, test, vi } from "vitest"; | ||
|
|
||
| import type { UsageLogRow } from "@/repository/usage-logs"; | ||
|
|
||
| vi.mock("next-intl", () => ({ | ||
| useTranslations: () => (key: string) => key, | ||
| })); | ||
|
|
||
| vi.mock("@/components/ui/tooltip", () => ({ | ||
| TooltipProvider: ({ children }: { children?: ReactNode }) => <div>{children}</div>, | ||
| Tooltip: ({ children }: { children?: ReactNode }) => <div>{children}</div>, | ||
| TooltipTrigger: ({ children }: { children?: ReactNode }) => <div>{children}</div>, | ||
| TooltipContent: ({ children }: { children?: ReactNode }) => <div>{children}</div>, | ||
| })); | ||
|
|
||
| vi.mock("@/components/ui/relative-time", () => ({ | ||
| RelativeTime: ({ fallback }: { fallback: string }) => <span>{fallback}</span>, | ||
| })); | ||
|
|
||
| vi.mock("./model-display-with-redirect", () => ({ | ||
| ModelDisplayWithRedirect: ({ | ||
| currentModel, | ||
| onRedirectClick, | ||
| }: { | ||
| currentModel: string | null; | ||
| onRedirectClick?: () => void; | ||
| }) => ( | ||
| <button type="button" data-slot="model-redirect" onClick={onRedirectClick}> | ||
| {currentModel ?? "-"} | ||
| </button> | ||
| ), | ||
| })); | ||
|
|
||
| vi.mock("./error-details-dialog", () => ({ | ||
| ErrorDetailsDialog: () => <div data-slot="error-details-dialog" />, | ||
| })); | ||
|
|
||
| import { UsageLogsTable } from "./usage-logs-table"; | ||
|
|
||
| function makeLog(overrides: Partial<UsageLogRow>): UsageLogRow { | ||
| return { | ||
| id: 1, | ||
| createdAt: new Date(), | ||
| sessionId: null, | ||
| requestSequence: null, | ||
| userName: "u", | ||
| keyName: "k", | ||
| providerName: "p", | ||
| model: "m", | ||
| originalModel: null, | ||
| endpoint: "/v1/messages", | ||
| statusCode: 200, | ||
| inputTokens: 1, | ||
| outputTokens: 1, | ||
| cacheCreationInputTokens: 0, | ||
| cacheReadInputTokens: 0, | ||
| cacheCreation5mInputTokens: 0, | ||
| cacheCreation1hInputTokens: 0, | ||
| cacheTtlApplied: null, | ||
| totalTokens: 2, | ||
| costUsd: "0.01", | ||
| costMultiplier: null, | ||
| durationMs: 100, | ||
| ttfbMs: 50, | ||
| errorMessage: null, | ||
| providerChain: null, | ||
| blockedBy: null, | ||
| blockedReason: null, | ||
| userAgent: null, | ||
| messagesCount: null, | ||
| context1mApplied: null, | ||
| specialSettings: null, | ||
| ...overrides, | ||
| }; | ||
| } | ||
|
|
||
| describe("usage-logs-table multiplier badge", () => { | ||
| test("does not render multiplier badge for null/undefined/empty/NaN/Infinity", () => { | ||
| for (const costMultiplier of [null, undefined, "", "NaN", "Infinity"] as const) { | ||
| const html = renderToStaticMarkup( | ||
| <UsageLogsTable | ||
| logs={[makeLog({ id: 1, costMultiplier })]} | ||
| total={1} | ||
| page={1} | ||
| pageSize={50} | ||
| onPageChange={() => {}} | ||
| isPending={false} | ||
| /> | ||
| ); | ||
|
|
||
| expect(html).not.toContain("×0.00"); | ||
| expect(html).not.toContain("×NaN"); | ||
| expect(html).not.toContain("×Infinity"); | ||
| } | ||
| }); | ||
|
|
||
| test("renders multiplier badge when finite and != 1", () => { | ||
| const html = renderToStaticMarkup( | ||
| <UsageLogsTable | ||
| logs={[makeLog({ id: 1, costMultiplier: "0.2" })]} | ||
| total={1} | ||
| page={1} | ||
| pageSize={50} | ||
| onPageChange={() => {}} | ||
| isPending={false} | ||
| /> | ||
| ); | ||
|
|
||
| expect(html).toContain("×0.20"); | ||
| expect(html).toContain("0.20x"); | ||
| }); | ||
|
|
||
| test("renders warmup skipped and blocked labels", () => { | ||
| const htmlWarmup = renderToStaticMarkup( | ||
| <UsageLogsTable | ||
| logs={[makeLog({ id: 1, blockedBy: "warmup" })]} | ||
| total={1} | ||
| page={1} | ||
| pageSize={50} | ||
| onPageChange={() => {}} | ||
| isPending={false} | ||
| /> | ||
| ); | ||
| expect(htmlWarmup).toContain("logs.table.skipped"); | ||
|
|
||
| const htmlBlocked = renderToStaticMarkup( | ||
| <UsageLogsTable | ||
| logs={[makeLog({ id: 1, blockedBy: "sensitive_word" })]} | ||
| total={1} | ||
| page={1} | ||
| pageSize={50} | ||
| onPageChange={() => {}} | ||
| isPending={false} | ||
| /> | ||
| ); | ||
| expect(htmlBlocked).toContain("logs.table.blocked"); | ||
| }); | ||
|
|
||
| test("invokes model redirect and pagination callbacks", async () => { | ||
| const onPageChange = vi.fn(); | ||
| const container = document.createElement("div"); | ||
| document.body.appendChild(container); | ||
|
|
||
| const root = createRoot(container); | ||
| await act(async () => { | ||
| root.render( | ||
| <UsageLogsTable | ||
| logs={[makeLog({ id: 1, costMultiplier: "0.2" })]} | ||
| total={100} | ||
| page={1} | ||
| pageSize={50} | ||
| onPageChange={onPageChange} | ||
| isPending={false} | ||
| /> | ||
| ); | ||
| }); | ||
|
|
||
| // Trigger model redirect click (covers onRedirectClick handler) | ||
| const redirectButton = container.querySelector('button[data-slot="model-redirect"]'); | ||
| expect(redirectButton).not.toBeNull(); | ||
| await act(async () => { | ||
| redirectButton?.dispatchEvent(new MouseEvent("click", { bubbles: true })); | ||
| }); | ||
|
|
||
| // Trigger pagination (covers onClick handlers) | ||
| const nextButton = Array.from(container.querySelectorAll("button")).find((b) => | ||
| (b.textContent ?? "").includes("logs.table.nextPage") | ||
| ); | ||
| expect(nextButton).not.toBeUndefined(); | ||
| await act(async () => { | ||
| nextButton?.dispatchEvent(new MouseEvent("click", { bubbles: true })); | ||
| }); | ||
| expect(onPageChange).toHaveBeenCalledWith(2); | ||
|
|
||
| await act(async () => { | ||
| root.unmount(); | ||
| }); | ||
| container.remove(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
您好,这部分对
costMultiplier的校验逻辑非常棒,很严谨。为了与本次 PR 中其他文件(如
usage-logs-table.tsx)的风格保持一致,并提高代码可读性,建议将这部分逻辑从 IIFE (立即调用函数表达式) 中提取出来。例如,可以在组件函数体顶部(返回 JSX 之前)定义
multiplier和showMultiplier变量,然后在 JSX 中直接使用它们进行条件渲染。这样可以让 JSX 结构更简洁清晰,也更便于维护。