diff --git a/tests/e2e/ui/fixtures/mock_llm_server/server.py b/tests/e2e/ui/fixtures/mock_llm_server/server.py index 82c90a9dd64..1e3dcecaff3 100644 --- a/tests/e2e/ui/fixtures/mock_llm_server/server.py +++ b/tests/e2e/ui/fixtures/mock_llm_server/server.py @@ -117,6 +117,17 @@ async def embeddings(request: Request): } +@app.post("/v1/systemone") +async def systemone(request: Request): + body = await request.json() + noul_keys = [key for key, question in body.get("questions", {}).items() if question.get("type") == "noul"] + return { + "model": body.get("model", "mock-decisions"), + "answers": {key: {"type": "noul", "noul": 0.9} for key in noul_keys}, + "usage": {"input_tokens": 12, "output_tokens": 3}, + } + + if __name__ == "__main__": # The port is overridable so two checkouts can run the harness at the same # time; the default keeps every existing caller (run_e2e.sh, the CircleCI diff --git a/tests/e2e/ui/tests/playground/decisionsResponseHeader.spec.ts b/tests/e2e/ui/tests/playground/decisionsResponseHeader.spec.ts new file mode 100644 index 00000000000..b4dba6765a1 --- /dev/null +++ b/tests/e2e/ui/tests/playground/decisionsResponseHeader.spec.ts @@ -0,0 +1,162 @@ +import { + test as base, + expect, + type APIRequestContext, + type Locator, + type Page as PlaywrightPage, +} from "@playwright/test"; +import { ADMIN_STORAGE_PATH } from "../../constants"; +import { Page } from "../../fixtures/pages"; +import { navigateToPage } from "../../helpers/navigation"; +import { masterKey, rootPath, uniqueSuffix } from "../../helpers/traffic"; + +const MOCK_LLM_BASE = `http://127.0.0.1:${process.env.MOCK_LLM_PORT ?? "8090"}/v1`; + +const adminHeaders = () => ({ + Authorization: `Bearer ${masterKey()}`, + "Content-Type": "application/json", +}); + +const isListedAsDecisionsModel = async ( + request: APIRequestContext, + name: string, +): Promise => { + const listed = await request.get("/model_group/info", { + headers: adminHeaders(), + }); + if (!listed.ok()) return false; + const body: { data?: { model_group?: string; mode?: string }[] } = + await listed.json(); + return (body.data ?? []).some( + (group) => group.model_group === name && group.mode === "evaluation", + ); +}; + +const isRoutable = async ( + request: APIRequestContext, + name: string, +): Promise => { + const ping = await request.post(`${rootPath()}/v1/systemone`, { + headers: adminHeaders(), + data: { + model: name, + state: "readiness ping", + questions: { + ready: { + type: "noul", + instructions: "Is this a ping?", + criteria: { true: "yes", false: "no" }, + }, + }, + }, + }); + return ping.ok(); +}; + +const test = base.extend<{ decisionsModel: string }>({ + decisionsModel: async ({ request }, use) => { + const name = `e2e-decisions-header-${uniqueSuffix()}`; + const created = await request.post("/model/new", { + headers: adminHeaders(), + data: { + model_name: name, + litellm_params: { + model: "typesafe/jev-latest", + api_base: MOCK_LLM_BASE, + api_key: "fake-key", + }, + }, + }); + expect( + created.ok(), + `POST /model/new failed (${created.status()}): ${await created.text()}`, + ).toBe(true); + const modelId: string = (await created.json()).model_id; + try { + await expect + .poll( + async () => + (await isListedAsDecisionsModel(request, name)) && + (await isRoutable(request, name)), + { + message: `${name} never became listed and routable`, + timeout: 60_000, + }, + ) + .toBe(true); + await use(name); + } finally { + await request.post("/model/delete", { + headers: adminHeaders(), + data: { id: modelId }, + }); + } + }, +}); + +const centerY = async (locator: Locator): Promise => { + const box = await locator.boundingBox(); + expect(box, "element has no layout box").not.toBeNull(); + return box!.y + box!.height / 2; +}; + +const offCenterBy = async (inner: Locator, outer: Locator): Promise => + Math.abs((await centerY(inner)) - (await centerY(outer))); + +const dismissSearchHint = async (page: PlaywrightPage): Promise => { + const dismiss = page.getByRole("button", { name: "Dismiss search hint" }); + if (await dismiss.isVisible()) { + await dismiss.click(); + } +}; + +test.describe("Decisions playground Response header", () => { + test.use({ + storageState: ADMIN_STORAGE_PATH, + viewport: { width: 1440, height: 900 }, + }); + + test("keeps the title and its badges on one row centered in the header", async ({ + page, + decisionsModel, + }) => { + await navigateToPage(page, Page.LlmPlayground); + await page.getByRole("tab", { name: /Decisions/ }).click(); + + const header = page + .getByRole("region", { name: "Decisions results" }) + .locator('[data-slot="card-header"]'); + const title = header.getByText("Response", { exact: true }); + await expect(title).toBeVisible({ timeout: 20_000 }); + expect( + await offCenterBy(title, header), + "empty-state title is off center", + ).toBeLessThanOrEqual(1); + + await page.getByRole("button", { name: "Load example" }).click(); + await page + .getByRole("menuitem", { name: "Moderate a forum comment" }) + .click(); + const modelPicker = page.getByRole("combobox", { name: "Decision model" }); + await modelPicker.click(); + await modelPicker.fill(decisionsModel); + await page + .getByRole("option", { name: decisionsModel, exact: true }) + .click(); + await dismissSearchHint(page); + await page.getByRole("button", { name: "Send", exact: true }).click(); + + const usage = header.getByText("12 in / 3 out"); + await expect(usage).toBeVisible({ timeout: 30_000 }); + expect( + await offCenterBy(title, header), + "title is off center after Send", + ).toBeLessThanOrEqual(1); + for (const badge of [header.getByText(/^\d+ ms$/), usage]) { + expect( + await offCenterBy(badge, title), + "badge wrapped off the title's row", + ).toBeLessThanOrEqual(1); + } + }); +}); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/ResponseView.tsx b/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/ResponseView.tsx index cc73b44265c..572bf765ac1 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/ResponseView.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/ResponseView.tsx @@ -223,7 +223,7 @@ export default function ResponseView({ if (!response) { return ( - + Response @@ -240,7 +240,7 @@ export default function ResponseView({ return ( - + {view === "curl" ? "Response JSON" : "Response"}
{latencyMs !== undefined && ( diff --git a/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/ValidationFeedback.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/ValidationFeedback.test.tsx index a2f6c24781a..5089179ef13 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/ValidationFeedback.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/ValidationFeedback.test.tsx @@ -13,9 +13,9 @@ describe("ValidationStatus", () => { render( , ); @@ -23,7 +23,9 @@ describe("ValidationStatus", () => { }); it("reports a valid payload when only warnings remain", () => { - render(); + render( + , + ); expect(screen.getByText("Valid payload")).toBeInTheDocument(); }); }); @@ -31,7 +33,10 @@ describe("ValidationStatus", () => { describe("IssueList", () => { it("lists every issue under its field label and renders nothing when there are none", () => { const { rerender } = render( - , + , ); expect(screen.getByRole("list", { name: "Payload validation issues" })).toHaveTextContent("InputRequired"); rerender(); @@ -46,8 +51,8 @@ describe("ValidationSummary", () => { rerender( , ); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/ValidationFeedback.tsx b/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/ValidationFeedback.tsx index e8152456d15..554e7da3770 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/ValidationFeedback.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/ValidationFeedback.tsx @@ -1,6 +1,5 @@ import { Badge } from "@/components/ui/badge"; import { CircleAlert, CircleCheck, TriangleAlert } from "lucide-react"; -import { issueLabel } from "./lib/issueLabel"; import { errorCount as countErrors, type PayloadValidation } from "./lib/validatePayload"; export function ValidationStatus({ validation }: { validation: PayloadValidation }) { @@ -44,7 +43,7 @@ export function IssueList({ id, validation }: { id: string; validation: PayloadV ) : ( )} - {issueLabel(issue.path)} + {issue.label} {issue.message} ))} diff --git a/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/lib/issueLabel.test.ts b/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/lib/issueLabel.test.ts index 29ae0f60ae4..85d2a576194 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/lib/issueLabel.test.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/lib/issueLabel.test.ts @@ -3,29 +3,34 @@ import { issueLabel } from "./issueLabel"; describe("issueLabel", () => { it("names the Input field for both endpoints instead of the JSON key", () => { - expect(issueLabel("state")).toBe("Input"); - expect(issueLabel("input")).toBe("Input"); + expect(issueLabel(["state"])).toBe("Input"); + expect(issueLabel(["input"])).toBe("Input"); }); it("names top-level fields and the request itself", () => { - expect(issueLabel("model")).toBe("Model"); - expect(issueLabel("questions")).toBe("Questions"); - expect(issueLabel("safety_identifier")).toBe("Safety identifier"); - expect(issueLabel("root")).toBe("Request"); - expect(issueLabel("syntax")).toBe("Request"); + expect(issueLabel(["model"])).toBe("Model"); + expect(issueLabel(["questions"])).toBe("Questions"); + expect(issueLabel(["safety_identifier"])).toBe("Safety identifier"); + expect(issueLabel([])).toBe("Request"); + expect(issueLabel(["syntax"])).toBe("Request"); }); - it("names a question by its key or its 1-based position and the field inside it", () => { - expect(issueLabel("questions.urgency")).toBe('Question "urgency"'); - expect(issueLabel("questions.urgency.criteria")).toBe('Question "urgency" options'); - expect(issueLabel("questions.urgency.criteria.1")).toBe('Question "urgency" options'); - expect(issueLabel("questions.category.instructions")).toBe('Question "category" instructions'); - expect(issueLabel("questions.0.name")).toBe("Question 1 name"); - expect(issueLabel("questions.2.type")).toBe("Question 3 answer type"); + it("names a keyed question by its key, even an all-digit or dotted one, and the field inside it", () => { + expect(issueLabel(["questions", "urgency"])).toBe('Question "urgency"'); + expect(issueLabel(["questions", "urgency", "criteria"])).toBe('Question "urgency" options'); + expect(issueLabel(["questions", "urgency", "criteria", 1])).toBe('Question "urgency" options'); + expect(issueLabel(["questions", "category", "instructions"])).toBe('Question "category" instructions'); + expect(issueLabel(["questions", "2", "criteria"])).toBe('Question "2" options'); + expect(issueLabel(["questions", "v1.2", "type"])).toBe('Question "v1.2" answer type'); + }); + + it("names a listed question by its 1-based position", () => { + expect(issueLabel(["questions", 0, "name"])).toBe("Question 1 name"); + expect(issueLabel(["questions", 2, "type"])).toBe("Question 3 answer type"); }); it("falls back to the humanized key for anything unknown", () => { - expect(issueLabel("max_tokens")).toBe("max tokens"); - expect(issueLabel("questions.a.max_items")).toBe('Question "a" max items'); + expect(issueLabel(["max_tokens"])).toBe("max tokens"); + expect(issueLabel(["questions", "a", "max_items"])).toBe('Question "a" max items'); }); }); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/lib/issueLabel.ts b/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/lib/issueLabel.ts index 76acc266117..60ca47ead2a 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/lib/issueLabel.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/lib/issueLabel.ts @@ -15,16 +15,16 @@ const QUESTION_FIELD: Record = { criteria: "options", }; -const humanize = (segment: string): string => segment.replace(/_/g, " "); +const humanize = (segment: PropertyKey): string => String(segment).replace(/_/g, " "); -export function issueLabel(path: string): string { - const [head, question, field] = path.split("."); +export function issueLabel(path: readonly PropertyKey[]): string { + const [head = "root", question, field] = path; if (head !== "questions" || question === undefined) { - return TOP_LEVEL[head] ?? humanize(head); + return TOP_LEVEL[String(head)] ?? humanize(head); } - const title = /^\d+$/.test(question) ? `Question ${Number(question) + 1}` : `Question "${question}"`; + const title = typeof question === "number" ? `Question ${question + 1}` : `Question "${String(question)}"`; if (field === undefined) { return title; } - return `${title} ${QUESTION_FIELD[field] ?? humanize(field)}`; + return `${title} ${QUESTION_FIELD[String(field)] ?? humanize(field)}`; } diff --git a/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/lib/validatePayload.test.ts b/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/lib/validatePayload.test.ts index 6e91f09f8bd..e28d7da8e3f 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/lib/validatePayload.test.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/lib/validatePayload.test.ts @@ -11,11 +11,8 @@ const requestWith = (fields: Record = {}) => describe("validateSystemOnePayload", () => { it("rejects empty input", () => { - expect(validateSystemOnePayload(" ").issues).toContainEqual({ - path: "root", - message: "Payload cannot be empty.", - severity: "error", - }); + const emptyIssue = { path: "root", label: "Request", message: "Payload cannot be empty.", severity: "error" }; + expect(validateSystemOnePayload(" ").issues).toContainEqual(emptyIssue); }); it("reports JSON syntax errors", () => { @@ -249,6 +246,17 @@ describe("validateSystemOnePayload", () => { ); }); + it("labels a score-level warning by its question key even when the key is all digits", () => { + const result = validateSystemOnePayload( + requestWith({ + questions: { + "2": { type: "score", instructions: "Rate urgency", criteria: Array.from({ length: 11 }, () => "Level") }, + }, + }), + ); + expect(result.issues.map((issue) => issue.label)).toEqual(['Question "2" options']); + }); + it("accepts a score question with two levels", () => { expect( validateSystemOnePayload( @@ -346,11 +354,19 @@ describe("blank context", () => { expect(validateSystemOnePayload(raw, "/v1/systemone").issues.map((issue) => issue.path)).toEqual(["state"]); const filled = validateSystemOnePayload(raw.replace('"state":""', '"state":"A ticket"'), "/v1/systemone"); expect(filled.isValid).toBe(false); - expect(filled.issues).toContainEqual({ - path: "questions", - message: "Give every question a name.", - severity: "error", + expect(filled.issues).toEqual([ + { path: "questions", label: "Questions", message: "Give every question a name.", severity: "error" }, + ]); + }); + + it("labels a /v1/systemone question by its key even when the key is all digits", () => { + const raw = JSON.stringify({ + state: "A ticket", + questions: { "2": { type: "choice", instructions: "", criteria: {} } }, }); + const labels = validateSystemOnePayload(raw, "/v1/systemone").issues.map((issue) => issue.label); + expect(labels).toContain('Question "2" options'); + expect(labels.some((label) => label.startsWith("Question 3"))).toBe(false); }); it("accepts /v1/systemone once the state carries text", () => { @@ -365,6 +381,25 @@ describe("blank context", () => { expect(result.issues.map((issue) => issue.path)).toEqual(["input"]); }); + it("labels a /v1/decisions issue by the question's position", () => { + const duplicateChoices = [{ value: "a" }, { value: "a" }]; + const raw = JSON.stringify({ + input: "Ship it?", + questions: [ + { type: "predicate", name: "q", instructions: "Is it?" }, + { type: "choice", name: "pick", instructions: "Pick one", choices: duplicateChoices }, + ], + }); + expect(validateOpenAIDecisionsPayload(raw).issues).toEqual([ + { + path: "questions.1.choices", + label: "Question 2 choices", + message: "Choice values must be unique, including their string representations.", + severity: "error", + }, + ]); + }); + it("accepts /v1/decisions once the input carries text", () => { const raw = JSON.stringify({ input: "Ship it?", diff --git a/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/lib/validatePayload.ts b/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/lib/validatePayload.ts index 1fb32efd158..05ed0f14511 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/lib/validatePayload.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/playground/components/systemOneUI/lib/validatePayload.ts @@ -1,4 +1,5 @@ import { z } from "zod"; +import { issueLabel } from "./issueLabel"; import { openAIDecisionsRequestSchema, type OpenAIDecisionsRequest } from "./openAIDecisions"; import { decisionsRequestSchema, @@ -11,6 +12,7 @@ const RECOMMENDED_MAX_SCORE_LEVELS = 10; export interface SystemOnePayloadIssue { path: string; + label: string; message: string; severity: "error" | "warning"; } @@ -26,21 +28,28 @@ export type SystemOnePayloadValidation = PayloadValidation; export const errorCount = (validation: PayloadValidation): number => validation.issues.filter((issue) => issue.severity === "error").length; +const error = (path: readonly PropertyKey[], message: string): SystemOnePayloadIssue => ({ + path: path.length > 0 ? path.join(".") : "root", + label: issueLabel(path), + message, + severity: "error", +}); + const toIssue = (path: PropertyKey[], message: string): SystemOnePayloadIssue => path[0] === "questions" && path[1] === "" - ? { path: "questions", message: "Give every question a name.", severity: "error" } - : { path: path.length > 0 ? path.join(".") : "root", message, severity: "error" }; + ? error(["questions"], "Give every question a name.") + : error(path, message); const invalid = (path: string, message: string) => ({ isValid: false, - issues: [{ path, message, severity: "error" as const }], + issues: [error([path], message)], }); export function parseJson(raw: string): { ok: true; value: unknown } | { ok: false; message: string } { try { return { ok: true, value: JSON.parse(raw) }; - } catch (error: unknown) { - return { ok: false, message: error instanceof Error ? error.message : String(error) }; + } catch (cause: unknown) { + return { ok: false, message: cause instanceof Error ? cause.message : String(cause) }; } } @@ -49,6 +58,7 @@ const scoreLevelWarnings = (payload: PlaygroundRequest): SystemOnePayloadIssue[] .filter(([, question]) => question.type === "score" && question.criteria.length > RECOMMENDED_MAX_SCORE_LEVELS) .map(([id]) => ({ path: `questions.${id}.criteria`, + label: issueLabel(["questions", id, "criteria"]), message: `More than ${RECOMMENDED_MAX_SCORE_LEVELS} score levels may reduce result quality.`, severity: "warning", })); @@ -90,7 +100,7 @@ const requireContext = (raw: string, path: "state" | "input", result: Payload ? { ...result, isValid: false, - issues: [{ path, message: "Add the text the model should decide about.", severity: "error" }], + issues: [error([path], "Add the text the model should decide about.")], } : result;