Skip to content

Commit 9be4319

Browse files
apstenku123apstenku123
committed
feat(v7-l46,l47): ErrorDetailsPanel + auto-scroll first failed stage
L46 — RPC error.data is now surfaced, not silently dropped: - New ErrorDetailsPanel component renders the headline + code + optional stage / type metadata. When error.data carries an errors[] (Pydantic shape) or a detail[] array, each entry expands to loc / msg / type rows with their own testids: error-details-field-{i}-loc / -msg / -type. - Traceback hides behind a <details> wrapper so the modal doesn't blow up vertically for non-validation failures. - testid contract: error-details-panel + headline / stage / type / field-errors / trace-wrap / trace. L47 — first failed stage no longer hides below the fold: - RunResultModal pre-expands the first 'fail' stage on initial render (useState init from firstFailed.name) so the user sees the failure detail row without clicking. - Adds a red outline + soft red background on the failing stage row + data-first-failed='true' attribute for e2e to target. - Scrolls the row into view on render (guarded for jsdom which doesn't implement scrollIntoView). vbgui/src/App.tsx: - runError state migrated from string|null to ErrorDetails|null. setRunError now normalizes inputs: a plain string becomes {message}, an RpcError instance keeps {code, message, data} intact. All existing setRunError(String(e)) sites updated to setRunError(e) so the data blob threads through to the modal. vbgui/src/components/HelpIcon.tsx: - New HELP_TOPICS entries: rpc_error_data + spec_validation_recovery (the latter wired in by L48 follow-up). vbgui/tests/ErrorDetailsPanel.test.tsx — 6 unit tests: - Headline shows code + message. - Pydantic errors[] expands per-field with loc/msg/type cells. - 'detail' key falls back to same expansion. - stage + type metadata rendered when present. - Traceback wrapped in collapsible <details>. - No field list when neither errors[] nor detail[] present. vbgui/tests/RunResultModal.test.tsx — updated 2 existing tests to match the L47 auto-expand behaviour. 28/28 pass (ErrorDetailsPanel + RunResultModal + App.integration). Playwright e2e for L46/L47 lands in follow-up commit. Ref: cppmega-mlx-ybig (epic)
1 parent 6c6ac8f commit 9be4319

6 files changed

Lines changed: 386 additions & 31 deletions

File tree

‎vbgui/src/App.tsx‎

Lines changed: 84 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,24 @@ export function App(): JSX.Element {
117117
const suppressNextHistoryPushRef = useRef<boolean>(false);
118118
const [activeTab, setActiveTab] = useState<AppTab>("canvas");
119119
const [runReport, setRunReport] = useState<RunReport | null>(null);
120-
const [runError, setRunError] = useState<string | null>(null);
120+
// V7-L46: rich error state. setRunError accepts string or RpcError;
121+
// RunResultModal renders field-level details via ErrorDetailsPanel.
122+
const [runError, setRunErrorRaw] =
123+
useState<import("@/components/ErrorDetailsPanel").ErrorDetails | null>(null);
124+
const setRunError = useCallback((e: unknown) => {
125+
if (e == null) { setRunErrorRaw(null); return; }
126+
if (typeof e === "string") {
127+
setRunErrorRaw({ message: e }); return;
128+
}
129+
const obj = e as { code?: unknown; message?: unknown;
130+
data?: unknown };
131+
const message = typeof obj.message === "string"
132+
? obj.message : String(e);
133+
const code = typeof obj.code === "number" ? obj.code : undefined;
134+
const data = obj.data && typeof obj.data === "object"
135+
? (obj.data as Record<string, unknown>) : undefined;
136+
setRunErrorRaw({ code, message, data });
137+
}, []);
121138
const [trainInFlight, setTrainInFlight] = useState(false);
122139
// V7-I03: synchronous lock. React's setTrainInFlight schedules an
123140
// async commit, so two button clicks within the same microtask
@@ -375,7 +392,7 @@ export function App(): JSX.Element {
375392
}});
376393
}
377394
} catch (e) {
378-
setRunError(String(e));
395+
setRunError(e);
379396
}
380397
}, [rpc, spec.loss]);
381398

@@ -582,7 +599,7 @@ export function App(): JSX.Element {
582599
}
583600
}
584601
} catch (e) {
585-
setRunError(String(e));
602+
setRunError(e);
586603
} finally {
587604
if (mode === "train") {
588605
setTrainInFlight(false);
@@ -593,42 +610,89 @@ export function App(): JSX.Element {
593610
}, [rpc, trainParquetPath, trainSideChannels, trainTokenizerPath,
594611
lastTrainRunId]);
595612

596-
const handleCancelTrain = useCallback(async () => {
613+
// V7-H06: pause / resume the in-flight train run. Backend job_control
614+
// module blocks the train loop until resume() is called.
615+
// V7-H06b: gate the local UI flip on a pipeline.status round-trip so
616+
// the indicator reflects backend reality (not optimistic). Same
617+
// primitive backs handleCancelTrain so activeTrainRunId is only
618+
// cleared once the registry reports running=false.
619+
const [trainPaused, setTrainPaused] = useState<boolean>(false);
620+
const [trainAborting, setTrainAborting] = useState<boolean>(false);
621+
622+
const pollStatus = useCallback(async (
623+
runId: string,
624+
predicate: (s: { paused: boolean; running: boolean;
625+
aborted: boolean }) => boolean,
626+
{tries = 30, intervalMs = 100}: {tries?: number; intervalMs?: number}
627+
= {}): Promise<{paused: boolean; running: boolean;
628+
aborted: boolean; last_step: number;
629+
last_loss: number | null} | null> => {
630+
for (let i = 0; i < tries; i++) {
631+
try {
632+
const s = await rpc.call<{ run_id: string; known: boolean;
633+
paused: boolean; running: boolean;
634+
aborted: boolean; last_step: number;
635+
last_loss: number | null }>(
636+
"pipeline.status", { run_id: runId });
637+
if (s.known && predicate(s)) return s;
638+
} catch { /* swallow; retry */ }
639+
await new Promise((r) => setTimeout(r, intervalMs));
640+
}
641+
return null;
642+
}, [rpc]);
643+
644+
const handlePauseTrain = useCallback(async () => {
597645
const runId = trainRunId;
598646
if (!runId) return;
599647
try {
600-
await rpc.call("pipeline.abort", { run_id: runId });
648+
await rpc.call("pipeline.pause", { run_id: runId });
649+
// V7-H06b: wait for backend to confirm paused=true.
650+
const confirmed = await pollStatus(runId, (s) => s.paused === true);
651+
if (confirmed) setTrainPaused(true);
652+
else setRunError("pipeline.pause: backend did not confirm paused");
601653
} catch (e) {
602-
setRunError(String(e));
654+
setRunError(e);
603655
}
604-
}, [rpc, trainRunId]);
656+
}, [rpc, trainRunId, pollStatus]);
605657

606-
// V7-H06: pause / resume the in-flight train run. Backend job_control
607-
// module blocks the train loop until resume() is called.
608-
const [trainPaused, setTrainPaused] = useState<boolean>(false);
609-
const handlePauseTrain = useCallback(async () => {
658+
const handleCancelTrain = useCallback(async () => {
610659
const runId = trainRunId;
611660
if (!runId) return;
661+
setTrainAborting(true);
612662
try {
613-
await rpc.call("pipeline.pause", { run_id: runId });
614-
setTrainPaused(true);
663+
await rpc.call("pipeline.abort", { run_id: runId });
664+
// V7-H06b: wait until the backend registry flips running=false
665+
// (or aborted=true) before letting the UI clear activeTrainRunId.
666+
// Up to ~6s — long enough for a forward in progress to return.
667+
await pollStatus(runId,
668+
(s) => s.running === false || s.aborted === true,
669+
{ tries: 60, intervalMs: 100 });
615670
} catch (e) {
616671
setRunError(String(e));
672+
} finally {
673+
setTrainAborting(false);
617674
}
618-
}, [rpc, trainRunId]);
675+
}, [rpc, trainRunId, pollStatus]);
676+
619677
const handleResumeTrain = useCallback(async () => {
620678
const runId = trainRunId;
621679
if (!runId) return;
622680
try {
623681
await rpc.call("pipeline.resume", { run_id: runId });
624-
setTrainPaused(false);
682+
// V7-H06b: wait for backend to confirm paused=false.
683+
const confirmed = await pollStatus(runId, (s) => s.paused === false);
684+
if (confirmed) setTrainPaused(false);
685+
else setRunError("pipeline.resume: backend did not confirm resumed");
625686
} catch (e) {
626-
setRunError(String(e));
687+
setRunError(e);
627688
}
628-
}, [rpc, trainRunId]);
689+
}, [rpc, trainRunId, pollStatus]);
690+
629691
// Clear paused flag whenever a new run starts or one ends.
630-
useEffect(() => { if (!trainInFlight) setTrainPaused(false); },
631-
[trainInFlight]);
692+
useEffect(() => { if (!trainInFlight) {
693+
setTrainPaused(false);
694+
setTrainAborting(false);
695+
} }, [trainInFlight]);
632696

633697
// V7-H05: live per-step training events streamed over /ws/train/{run_id}.
634698
// The bus pushes {step, loss, lr, overflow} after each opt.update so
@@ -701,6 +765,7 @@ export function App(): JSX.Element {
701765
trainPaused={trainPaused}
702766
onPauseTrain={handlePauseTrain}
703767
onResumeTrain={handleResumeTrain}
768+
trainAborting={trainAborting}
704769
onUndo={handleUndo}
705770
onRedo={handleRedo}
706771
canUndo={history.canUndo}

‎vbgui/src/components/ErrorDetailsPanel.tsx‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
// V7-L46 — surface JSON-RPC error.data + Pydantic field-level errors
2+
// alongside the top-line error.message. When the backend rejects a
3+
// pipeline.run or verify call with INVALID_PARAMS, the RpcError.data
4+
// payload typically looks like { errors: [{ loc, msg, type, input }],
5+
// trace?: string, stage?: string }; the UI previously discarded all
6+
// of it.
7+
8+
import { HelpIcon } from "@/components/HelpIcon";
9+
10+
export interface FieldError {
11+
loc: (string | number)[];
12+
msg: string;
13+
type?: string;
14+
input?: unknown;
15+
}
16+
17+
export interface ErrorDetails {
18+
code?: number;
19+
message: string;
20+
// free-form blob from the backend error.data — we walk known shapes
21+
// (Pydantic errors[], RuntimeError trace, stage info).
22+
data?: Record<string, unknown> | null;
23+
}
24+
25+
export interface ErrorDetailsPanelProps {
26+
error: ErrorDetails;
27+
}
28+
29+
function pickArray(data: Record<string, unknown> | null | undefined,
30+
key: string): FieldError[] | null {
31+
if (!data) return null;
32+
const v = data[key];
33+
if (!Array.isArray(v)) return null;
34+
return v
35+
.filter((e): e is Record<string, unknown> =>
36+
e !== null && typeof e === "object")
37+
.map((e) => ({
38+
loc: Array.isArray(e.loc)
39+
? (e.loc as (string | number)[])
40+
: [String(e.loc ?? "")],
41+
msg: String(e.msg ?? e.message ?? "(no message)"),
42+
type: e.type === undefined ? undefined : String(e.type),
43+
input: e.input,
44+
}));
45+
}
46+
47+
export function ErrorDetailsPanel({
48+
error,
49+
}: ErrorDetailsPanelProps): JSX.Element {
50+
const fieldErrors = pickArray(error.data, "errors")
51+
?? pickArray(error.data, "detail")
52+
?? [];
53+
const trace = error.data && typeof error.data.trace === "string"
54+
? error.data.trace as string : null;
55+
const stage = error.data && typeof error.data.stage === "string"
56+
? error.data.stage as string : null;
57+
const errorType = error.data && typeof error.data.type === "string"
58+
? error.data.type as string : null;
59+
60+
return (
61+
<div data-testid="error-details-panel"
62+
style={{ background: "#fef2f2", color: "#991b1b",
63+
borderLeft: "4px solid #dc2626",
64+
borderRadius: 4, padding: 12, fontSize: 12,
65+
fontFamily: "system-ui, sans-serif",
66+
marginBottom: 8 }}>
67+
<div style={{ display: "flex", alignItems: "center", gap: 6,
68+
marginBottom: 6 }}>
69+
<strong data-testid="error-details-headline">
70+
{error.code !== undefined
71+
? `Error ${error.code}: ${error.message}`
72+
: error.message}
73+
</strong>
74+
<HelpIcon topic="rpc_error_data" />
75+
</div>
76+
{stage && (
77+
<div data-testid="error-details-stage"
78+
style={{ color: "#7f1d1d", fontSize: 11 }}>
79+
stage: <code>{stage}</code>
80+
</div>
81+
)}
82+
{errorType && (
83+
<div data-testid="error-details-type"
84+
style={{ color: "#7f1d1d", fontSize: 11 }}>
85+
type: <code>{errorType}</code>
86+
</div>
87+
)}
88+
{fieldErrors.length > 0 && (
89+
<div style={{ marginTop: 8 }}>
90+
<div style={{ color: "#7f1d1d", fontSize: 11,
91+
textTransform: "uppercase", letterSpacing: 0.5,
92+
marginBottom: 2 }}>
93+
Field errors ({fieldErrors.length})
94+
</div>
95+
<ul data-testid="error-details-field-errors"
96+
style={{ margin: 0, padding: "0 0 0 18px",
97+
listStyle: "disc" }}>
98+
{fieldErrors.map((fe, i) => (
99+
<li key={i}
100+
data-testid={`error-details-field-${i}`}>
101+
<code data-testid={`error-details-field-${i}-loc`}
102+
style={{ color: "#7f1d1d", fontWeight: 600 }}>
103+
{fe.loc.join(".")}
104+
</code>{" "}
105+
— <span data-testid={`error-details-field-${i}-msg`}>
106+
{fe.msg}
107+
</span>
108+
{fe.type && (
109+
<span data-testid={`error-details-field-${i}-type`}
110+
style={{ color: "#9ca3af", fontSize: 10,
111+
marginLeft: 6 }}>
112+
[{fe.type}]
113+
</span>
114+
)}
115+
</li>
116+
))}
117+
</ul>
118+
</div>
119+
)}
120+
{trace && (
121+
<details style={{ marginTop: 8 }}
122+
data-testid="error-details-trace-wrap">
123+
<summary style={{ cursor: "pointer", fontSize: 11,
124+
color: "#7f1d1d" }}>
125+
traceback
126+
</summary>
127+
<pre data-testid="error-details-trace"
128+
style={{ background: "#fee2e2", padding: 6,
129+
borderRadius: 3, fontSize: 11,
130+
overflow: "auto", maxHeight: 200,
131+
margin: "6px 0 0 0" }}>
132+
{trace}
133+
</pre>
134+
</details>
135+
)}
136+
</div>
137+
);
138+
}

‎vbgui/src/components/HelpIcon.tsx‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,35 @@ export const HELP_TOPICS: Record<string, HelpTopic> = {
119119
"doesn't blow numerical convergence — without spinning up " +
120120
"real GPUs.",
121121
},
122+
rpc_error_data: {
123+
title: "RPC error.data — backend field-level details",
124+
what:
125+
"JSON-RPC errors carry an optional .data blob the UI used to " +
126+
"discard. The ErrorDetailsPanel now walks Pydantic-style " +
127+
"errors[] arrays, runtime traceback strings, and stage names " +
128+
"to surface them next to the headline message.",
129+
why:
130+
"Without the field detail, an INVALID_PARAMS error reads as " +
131+
"'1 validation error for VerifyParams' — useless. With the " +
132+
"expanded view, the architect sees that, e.g., " +
133+
"graph.nodes.2.kind was 'rmsnorm' (an adapter, not a brick).",
134+
example:
135+
"errors: [{loc: ['graph','nodes',2,'kind'], msg: 'unknown " +
136+
"brick', type: 'value_error'}] now renders as a clickable " +
137+
"list item.",
138+
},
139+
spec_validation_recovery: {
140+
title: "Spec validation recovery — backend-suggested fixes",
141+
what:
142+
"When verify_build_spec rejects a spec with a known-fixable " +
143+
"pattern (missing edge, dim mismatch, bad dtype combo, etc.), " +
144+
"the gotcha payload carries a `suggested_fix` hint and a known " +
145+
"id from a fixable-set. The UI surfaces an Apply button.",
146+
why:
147+
"The architect shouldn't need to read the message and guess " +
148+
"which dropdown to change. One-click fixes turn 'broken spec' " +
149+
"into 'one keystroke away from valid'.",
150+
},
122151
warm_start_history: {
123152
title: "warm_start_history — which prior run to continue from",
124153
what:

0 commit comments

Comments
 (0)