Skip to content

Commit bbfc898

Browse files
committed
feat(v3-8+v3-9): Train button gated on gotcha severity=error
Closes V3-8 / cppmega-mlx-cwz and V3-9 / cppmega-mlx-7ut. Combined because both reduce to "disable Train when any gotcha is severity=error": - V3-8 case: check_gotchas stage returns severity='error' (critical config issue). Examples: loss kind incompatible with brick output. - V3-9 case: verify_build_spec rejects spec; verify RPC surfaces the rejection as a severity='error' gotcha in r.gotchas. App.tsx computes trainDisabled prop from spec.gotchas; TopBar renders the Train button as disabled with title + visible reason child element (data-testid=top-bar-train-disabled-reason). 3 new e2e scenarios in 15_gating.spec.ts use page.route() to inject synthetic error-severity gotchas into the verify response — Playwright asserts toBeDisabled() + reason text. Negative control verifies Train stays enabled with no error gotchas. 3/3 green, 8/8 TopBar vitest unchanged.
1 parent 7526375 commit bbfc898

3 files changed

Lines changed: 122 additions & 1 deletion

File tree

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
// V3-8 + V3-9: Train button gating.
2+
//
3+
// Train must be disabled when verify or check_gotchas produces a
4+
// severity='error' gotcha. We exercise this by intercepting the
5+
// /rpc verify response and injecting a synthetic error gotcha. The
6+
// UI's gating logic (App.tsx → TopBar.trainDisabled) must surface
7+
// the reason via data-testid='top-bar-train-disabled-reason'.
8+
9+
import { test, expect } from "@playwright/test";
10+
import { gotoApp, selectPreset } from "../fixtures";
11+
12+
async function injectVerifyGotcha(
13+
page: import("@playwright/test").Page,
14+
gotcha: { id: string; severity: "info" | "warning" | "error";
15+
message: string },
16+
): Promise<void> {
17+
await page.route("**/rpc", async (route, request) => {
18+
const body = JSON.parse(request.postData() ?? "{}");
19+
if (body.method !== "verify") {
20+
await route.continue();
21+
return;
22+
}
23+
const upstream = await page.request.fetch(request);
24+
const json = await upstream.json();
25+
json.result.gotchas = [...(json.result.gotchas ?? []), gotcha];
26+
await route.fulfill({
27+
status: 200,
28+
contentType: "application/json",
29+
body: JSON.stringify(json),
30+
});
31+
});
32+
}
33+
34+
// ---------------------------------------------------------------------------
35+
// V3-8: critical gotcha (check_gotchas) disables Train
36+
// ---------------------------------------------------------------------------
37+
38+
test("V3-8: critical gotcha disables Train + surfaces reason", async ({
39+
page,
40+
}) => {
41+
await injectVerifyGotcha(page, {
42+
id: "fake_critical",
43+
severity: "error",
44+
message: "synthetic critical: incompatible loss for brick",
45+
});
46+
await gotoApp(page);
47+
await selectPreset(page, "llama3_8b");
48+
49+
// Wait for verify to complete; the gotcha route fires per verify.
50+
await page.waitForTimeout(500);
51+
await page.getByTestId("run-pipeline-toggle").click();
52+
const trainBtn = page.getByTestId("run-pipeline-train");
53+
await expect(trainBtn).toBeDisabled();
54+
await expect(page.getByTestId("top-bar-train-disabled-reason"))
55+
.toContainText("synthetic critical");
56+
});
57+
58+
// ---------------------------------------------------------------------------
59+
// V3-9: verify=error severity gotcha disables Train
60+
// ---------------------------------------------------------------------------
61+
62+
test("V3-9: verify error severity disables Train", async ({ page }) => {
63+
await injectVerifyGotcha(page, {
64+
id: "fake_verify_error",
65+
severity: "error",
66+
message: "synthetic verify error: parallel-block requires pre_norm",
67+
});
68+
await gotoApp(page);
69+
await selectPreset(page, "llama3_8b");
70+
71+
await page.waitForTimeout(500);
72+
await page.getByTestId("run-pipeline-toggle").click();
73+
await expect(page.getByTestId("run-pipeline-train")).toBeDisabled();
74+
await expect(page.getByTestId("top-bar-train-disabled-reason"))
75+
.toContainText("parallel-block requires pre_norm");
76+
});
77+
78+
// ---------------------------------------------------------------------------
79+
// Negative control: no error gotcha → Train remains enabled
80+
// ---------------------------------------------------------------------------
81+
82+
test("Train stays enabled when no error-severity gotchas present",
83+
async ({ page }) => {
84+
await gotoApp(page);
85+
await selectPreset(page, "llama3_8b");
86+
await page.waitForTimeout(500);
87+
await page.getByTestId("run-pipeline-toggle").click();
88+
await expect(page.getByTestId("run-pipeline-train")).toBeEnabled();
89+
await expect(page.getByTestId("top-bar-train-disabled-reason"))
90+
.toHaveCount(0);
91+
});

vbgui/src/App.tsx

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,19 @@ export function App(): JSX.Element {
317317
onCompileModeChange={(m) => dispatch({ type: "sharding.set",
318318
sharding: { ...spec.sharding, compile_mode: m } })}
319319
onRunPipeline={handleRunPipeline}
320+
trainDisabled={
321+
(() => {
322+
// V3-8/V3-9: gate Train on gotcha severity. The verify RPC
323+
// returns gotchas with severity "error" for both validator
324+
// failures (verify=error, V3-9) and check_gotchas critical
325+
// findings (V3-8). Surfacing them via the disabled-reason
326+
// attribute lets Playwright assert on the exact gating cause.
327+
const errs = spec.gotchas.filter(g => g.severity === "error");
328+
if (errs.length === 0) return null;
329+
return { reason: `${errs.length} critical issue${
330+
errs.length > 1 ? "s" : ""}: ${errs[0].message}` };
331+
})()
332+
}
320333
/>
321334
<AppTabs active={activeTab} onChange={setActiveTab} />
322335
<div style={{ flex: 1, display: "flex", minHeight: 0 }}>

vbgui/src/components/TopBar.tsx

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ export interface TopBarProps {
1414
onTopologyChange: (t: TopologyFactory) => void;
1515
onCompileModeChange: (m: SpecState["sharding"]["compile_mode"]) => void;
1616
onRunPipeline: (mode: RunMode, opts?: { num_steps?: number }) => void;
17+
/** V3-8/V3-9: when present, Train button is rendered disabled with
18+
* reason exposed via data-testid='top-bar-train-disabled-reason'. */
19+
trainDisabled?: { reason: string } | null;
1720
}
1821

1922
export function TopBar(p: TopBarProps): JSX.Element {
@@ -88,7 +91,21 @@ export function TopBar(p: TopBarProps): JSX.Element {
8891
onClick={() => { setOpen(false);
8992
p.onRunPipeline("train",
9093
{ num_steps: trainNumSteps }); }}
91-
style={menuItem}>Train</button>
94+
disabled={!!p.trainDisabled}
95+
title={p.trainDisabled?.reason ?? ""}
96+
style={{ ...menuItem,
97+
opacity: p.trainDisabled ? 0.5 : 1,
98+
cursor: p.trainDisabled ? "not-allowed"
99+
: "pointer" }}>
100+
Train
101+
{p.trainDisabled && (
102+
<span data-testid="top-bar-train-disabled-reason"
103+
style={{ display: "block", fontSize: 10,
104+
color: "#dc2626" }}>
105+
{p.trainDisabled.reason}
106+
</span>
107+
)}
108+
</button>
92109
</div>
93110
)}
94111
</div>

0 commit comments

Comments
 (0)