fix(ui): label Decisions playground issues by question key and fix the Response header row (#46145)

* fix(ui): dedupe blank-question issues and label Decisions questions by key, not position

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* fix(ui): make the Response card header a flex row so title and badges share one line

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* test(ui): fail when duplicate blank-name issues stop being deduped

* fix(ui): drop the Decisions issue dedupe, which no reachable endpoint needs

* fix(ui): center the Response header and test the Decisions issue labels end to end

The card's border rule set 24px of bottom padding on the Response header, which put its content 8px above center. The header now keeps 8px on both sides. New tests cover the /v1/decisions position label and the score-level warning label for an all-digit key, which mutations of validatePayload left untested

* test(ui): check the Decisions Response header layout in the browser

* test(ui): register the Decisions header spec's model through the API so it runs in CI

The CI UI stack does not read fixtures/config.yml, so the config-only fake-decisions-model never existed there and the spec could not pick it. The spec now creates a uniquely named typesafe model against the mock server, waits until /model_group/info lists it as an evaluation model and /v1/systemone answers, and deletes it afterwards

---------

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
Mateo Wang 2026-10-14 10:47:05 -07:00 • committed by GitHub
parent b5aa827067
commit 2bb8f7f219
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 275 additions and 48 deletions

View file

@ -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

View file

@ -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<boolean> => {
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<boolean> => {
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<number> => {
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<number> =>
Math.abs((await centerY(inner)) - (await centerY(outer)));
const dismissSearchHint = async (page: PlaywrightPage): Promise<void> => {
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);
}
});
});

View file

@ -223,7 +223,7 @@ export default function ResponseView({
if (!response) {
return (
<Card className="gap-0 py-0">
<CardHeader className="flex min-h-12 flex-row items-center border-b px-4 py-2">
<CardHeader className="flex min-h-12 flex-row items-center border-b px-4 py-2 [.border-b]:pb-2">
<CardTitle className="text-sm font-medium">Response</CardTitle>
</CardHeader>
<CardContent className="flex min-h-[360px] flex-col items-center justify-center gap-3 p-4 text-center">
@ -240,7 +240,7 @@ export default function ResponseView({
return (
<Card className="gap-0 overflow-hidden py-0 wrap-anywhere">
<CardHeader className="min-h-12 flex-row flex-wrap items-center justify-between gap-2 border-b px-4 py-2">
<CardHeader className="flex min-h-12 flex-row flex-wrap items-center justify-between gap-2 border-b px-4 py-2 [.border-b]:pb-2">
<CardTitle className="text-sm font-medium">{view === "curl" ? "Response JSON" : "Response"}</CardTitle>
<div className="flex flex-wrap items-center gap-1.5">
{latencyMs !== undefined && (

View file

@ -13,9 +13,9 @@ describe("ValidationStatus", () => {
render(
<ValidationStatus
validation={validation([
{ path: "state", message: "Required", severity: "error" },
{ path: "model", message: "Required", severity: "error" },
{ path: "questions.a", message: "Long question", severity: "warning" },
{ path: "state", label: "Input", message: "Required", severity: "error" },
{ path: "model", label: "Model", message: "Required", severity: "error" },
{ path: "questions.a", label: "questions.a", message: "Long question", severity: "warning" },
])}
/>,
);
@ -23,7 +23,9 @@ describe("ValidationStatus", () => {
});
it("reports a valid payload when only warnings remain", () => {
render(<ValidationStatus validation={validation([{ path: "q", message: "Long", severity: "warning" }])} />);
render(
<ValidationStatus validation={validation([{ path: "q", label: "q", message: "Long", severity: "warning" }])} />,
);
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(
<IssueList id="issues" validation={validation([{ path: "state", message: "Required", severity: "error" }])} />,
<IssueList
id="issues"
validation={validation([{ path: "state", label: "Input", message: "Required", severity: "error" }])}
/>,
);
expect(screen.getByRole("list", { name: "Payload validation issues" })).toHaveTextContent("InputRequired");
rerender(<IssueList id="issues" validation={validation([])} />);
@ -46,8 +51,8 @@ describe("ValidationSummary", () => {
rerender(
<ValidationSummary
validation={validation([
{ path: "state", message: "Required", severity: "error" },
{ path: "q", message: "Long", severity: "warning" },
{ path: "state", label: "Input", message: "Required", severity: "error" },
{ path: "q", label: "q", message: "Long", severity: "warning" },
])}
/>,
);

View file

@ -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<unknown> }) {
@ -44,7 +43,7 @@ export function IssueList({ id, validation }: { id: string; validation: PayloadV
) : (
<TriangleAlert className="mt-0.5 size-3.5 shrink-0 text-amber-600 dark:text-amber-400" />
)}
<span className="shrink-0 font-medium">{issueLabel(issue.path)}</span>
<span className="shrink-0 font-medium">{issue.label}</span>
<span>{issue.message}</span>
</li>
))}

View file

@ -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');
});
});

View file

@ -15,16 +15,16 @@ const QUESTION_FIELD: Record<string, string> = {
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)}`;
}

View file

@ -11,11 +11,8 @@ const requestWith = (fields: Record<string, unknown> = {}) =>
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?",

View file

@ -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<PlaygroundRequest>;
export const errorCount = (validation: PayloadValidation<unknown>): 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 = <T>(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;