Skip to content

Commit 971886e

Browse files
Mokuroh54claude
andcommitted
fix(wandb): working endpoints — dead install gate removed, resume can't smuggle wandb on, key checked before side effects (MT40)
Fixes main's wandb surface per the MT40 audit, re-derived for main's code (not a port of rig's restore — rig's panel/lineage interactions arrive with the later carve train): - Install gate deleted: /system/wandb-extra*, the wandb InstallManager, and WandbInstallDialog. It was dead code — wandb is a hard transitive dep of the pinned lerobot training extra, so the probe could never fail — and its fetch-error fallthrough silently enabled wandb. Replaced by GET /system/wandb-credentials on the existing resolve_wandb_api_key (env -> ~/.netrc; boolean only, never the key). - Resume emits an explicit --wandb.enable, so a resumed checkpoint's train_config.json can never silently re-enable wandb (the MT40 core); enable/project/entity inherit server-side from the parent record (the wandb run identity rides the checkpoint; verified against lerobot v0.6.0's resume path). - Credential preflight: a wandb-enabled run with no resolvable key is refused 400 BEFORE any side effect — before the dataset Hub push and before the deferred resume-upload thread — instead of dying inside a billed GPU container 20 minutes in. - Defaults and copy: wandb defaults on when a key exists (form-level, once, never clobbering a user's choice); artifact upload defaults off; Entity/Project labeled in W&B's own vocabulary with the blank-value semantics spelled out. - URL scrape: comment corrected (lerobot's 'Track this run -->' line; wandb's own banner is WANDB_SILENT-suppressed) and run-id class widened. Known divergence kept deliberately: wandb_run_id stays on main's TrainingRequest (inert; rig removed it — reconciled by the carve train). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent d83a091 commit 971886e

13 files changed

Lines changed: 910 additions & 377 deletions

File tree

frontend/src/components/training/TrainingConfigurator.tsx

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,9 +239,14 @@ const TrainingConfigurator: React.FC<TrainingConfiguratorProps> = ({
239239
resume_from_step: resumeSeed?.step ?? undefined,
240240
finetune_from_job_id: finetuneSeed?.jobId,
241241
finetune_from_step: finetuneSeed?.step ?? undefined,
242+
// Off here, and switched on by the credentials probe below once a key is
243+
// confirmed — an initial value can't wait on an async answer. A RESUME is
244+
// never defaulted: its W&B state is inherited server-side.
242245
wandb_enable: false,
243246
wandb_mode: "online",
244-
wandb_disable_artifact: false,
247+
// TRUE = artifacts off. Per-checkpoint model uploads to W&B are opt-in,
248+
// not a side effect of turning logging on.
249+
wandb_disable_artifact: true,
245250
policy_device: resumeSeed?.policyDevice ?? "auto",
246251
policy_use_amp: resumeSeed?.policyUseAmp ?? false,
247252
optimizer_type: resumeSeed?.optimizerType ?? "adam",
@@ -260,6 +265,12 @@ const TrainingConfigurator: React.FC<TrainingConfiguratorProps> = ({
260265
hf_job_timeout: resumeSeed?.hfJobTimeout,
261266
});
262267

268+
// Whether the user has touched the W&B toggle, and whether the credentials
269+
// probe has already had its one say. Refs, not state: nothing renders from
270+
// them, and the default-on effect below must not re-run when they change.
271+
const wandbEnableTouched = useRef(false);
272+
const wandbDefaultApplied = useRef(false);
273+
263274
// The config the form actually reads: internal state overlaid with the
264275
// controlled policy type + dataset. Keeps EssentialsCard's frozen dataset
265276
// display and policy dropdown bound to the caller's selection.
@@ -332,6 +343,46 @@ const TrainingConfigurator: React.FC<TrainingConfiguratorProps> = ({
332343
.finally(() => setHardwareLoading(false));
333344
}, [baseUrl, fetchWithHeaders, auth.status]);
334345

346+
// Default W&B logging ON once the backend confirms it can resolve an API key.
347+
// A UI-level default only: the backend's `TrainingRequest.wandb_enable` still
348+
// defaults false, so non-UI callers keep opt-in semantics and the submit-time
349+
// preflight still protects them.
350+
//
351+
// Three guards, each earning its place:
352+
// * `resumeSeed` — a continuation's W&B state is inherited from its parent
353+
// server-side, never defaulted; enabling it here could only contradict
354+
// what JobRegistry.start is about to write.
355+
// * `wandbDefaultApplied` — fires at most once per mounted form, so a
356+
// re-render or a re-answered probe can't re-assert the default after the
357+
// user has moved on.
358+
// * `wandbEnableTouched` — an explicit decision always wins, including one
359+
// made while the probe was still in flight. The probe answers
360+
// asynchronously, so without this a late "yes, there's a key" would flip
361+
// the toggle back on under someone who had just switched it off.
362+
//
363+
// A failed probe leaves `available` false and changes nothing: not evidence a
364+
// key exists, so it must not turn logging on.
365+
useEffect(() => {
366+
if (resumeSeed || wandbDefaultApplied.current) return;
367+
let cancelled = false;
368+
fetchWithHeaders(`${baseUrl}/system/wandb-credentials`)
369+
.then((r) => r.json())
370+
.then((data: { available: boolean }) => {
371+
if (cancelled || !data.available) return;
372+
wandbDefaultApplied.current = true;
373+
if (wandbEnableTouched.current) return;
374+
setTrainingConfig((prev) =>
375+
prev.wandb_enable ? prev : { ...prev, wandb_enable: true },
376+
);
377+
})
378+
.catch(() => {
379+
/* older backend / transport blip — leave the default off */
380+
});
381+
return () => {
382+
cancelled = true;
383+
};
384+
}, [baseUrl, fetchWithHeaders, resumeSeed]);
385+
335386
const updateConfig = <T extends keyof TrainingConfig>(
336387
key: T,
337388
value: TrainingConfig[T],
@@ -344,6 +395,7 @@ const TrainingConfigurator: React.FC<TrainingConfiguratorProps> = ({
344395
return;
345396
}
346397
if (key === "dataset_repo_id") return;
398+
if (key === "wandb_enable") wandbEnableTouched.current = true;
347399
setTrainingConfig((prev) => ({ ...prev, [key]: value }));
348400
};
349401

frontend/src/components/training/WandbInstallDialog.tsx

Lines changed: 0 additions & 68 deletions
This file was deleted.

frontend/src/components/training/config/EssentialsCard.tsx

Lines changed: 27 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, { useState } from "react";
1+
import React from "react";
22
import { Input } from "@/components/ui/input";
33
import { NumberInput } from "@/components/ui/number-input";
44
import { Label } from "@/components/ui/label";
@@ -16,8 +16,6 @@ import {
1616
RESUME_INHERITED_NOTE,
1717
RESUME_INHERITED_SHORT,
1818
} from "../types";
19-
import WandbInstallDialog from "../WandbInstallDialog";
20-
import { useApi } from "@/contexts/ApiContext";
2119

2220
/** The run's headline settings — steps, batch size, name, and W&B logging.
2321
* Flat: each control carries its own <Label> and the section has no eyebrow
@@ -27,46 +25,20 @@ import { useApi } from "@/contexts/ApiContext";
2725
* On a resume, `steps` stays editable (the resume branch passes --steps, and
2826
* raising it is the whole point of a continuation) while `batch_size` does not
2927
* — lerobot takes it from the checkpoint's train_config.json. The whole W&B
30-
* group is locked for the same reason: the resume branch emits no --wandb.*, so
31-
* lerobot logs (or doesn't) exactly as the parent run's config said. The one
32-
* live effect the toggle keeps is a bad one — HfCloudJobRunner reads
33-
* `wandb_enable` when assembling job secrets and 400s on a missing
34-
* WANDB_API_KEY, so leaving it enabled could only ever block a launch without
35-
* turning any logging on. */
28+
* group is locked for a stronger reason: lerobot re-opens the PARENT's W&B run
29+
* (`wandb.init(resume="must")` with the run id stored in the checkpoint), so a
30+
* continuation cannot log anywhere else. JobRegistry.start inherits
31+
* enable/project/entity from the parent record and ignores what the form sends;
32+
* the values shown are the parent's, which is what the run really uses.
33+
*
34+
* On a FRESH run the toggle defaults ON once the backend reports a resolvable
35+
* W&B API key (see TrainingConfigurator); with no key it stays off, and a run
36+
* that enables it anyway is refused at submit time with the reason. */
3637
const EssentialsCard: React.FC<ConfigComponentProps> = ({
3738
config,
3839
updateConfig,
3940
resumeLocked,
4041
}) => {
41-
const { baseUrl, fetchWithHeaders } = useApi();
42-
const [wandbDialogOpen, setWandbDialogOpen] = useState(false);
43-
const [wandbInstallHint, setWandbInstallHint] = useState("pip install wandb");
44-
45-
const handleWandbToggle = async (checked: boolean) => {
46-
if (!checked) {
47-
updateConfig("wandb_enable", false);
48-
return;
49-
}
50-
// Check availability before flipping the switch on. If wandb isn't
51-
// importable in this MakerMods Lab process, surface the same install flow used
52-
// for the training extra (accelerate) instead of letting the user start
53-
// a run that will fail.
54-
try {
55-
const r = await fetchWithHeaders(`${baseUrl}/system/wandb-extra`);
56-
const data: { available: boolean; install_hint: string } = await r.json();
57-
if (data.available) {
58-
updateConfig("wandb_enable", true);
59-
} else {
60-
setWandbInstallHint(data.install_hint);
61-
setWandbDialogOpen(true);
62-
}
63-
} catch {
64-
// Backend unreachable — let the user proceed; training start will
65-
// surface the real error if wandb is genuinely missing.
66-
updateConfig("wandb_enable", true);
67-
}
68-
};
69-
7042
return (
7143
<section className="space-y-4">
7244
<div className="grid grid-cols-2 gap-4">
@@ -131,44 +103,47 @@ const EssentialsCard: React.FC<ConfigComponentProps> = ({
131103
<Switch
132104
id="wandb_enable"
133105
checked={config.wandb_enable}
134-
onCheckedChange={handleWandbToggle}
106+
onCheckedChange={(checked) => updateConfig("wandb_enable", checked)}
135107
disabled={resumeLocked}
136108
className="data-[state=checked]:bg-primary"
137109
/>
138110
<Label htmlFor="wandb_enable">Log to Weights &amp; Biases</Label>
139111
</div>
140112

141-
<WandbInstallDialog
142-
open={wandbDialogOpen}
143-
onOpenChange={setWandbDialogOpen}
144-
installHint={wandbInstallHint}
145-
/>
146-
147113
{config.wandb_enable && (
148114
<div className="space-y-4 border-l-2 border-border pl-4">
149115
<div className="space-y-2">
150-
<Label htmlFor="wandb_project">W&amp;B project name</Label>
116+
<Label htmlFor="wandb_project">Project</Label>
151117
<Input
152118
id="wandb_project"
153119
value={config.wandb_project || ""}
154120
onChange={(e) =>
155121
updateConfig("wandb_project", e.target.value || undefined)
156122
}
157-
placeholder="my-robotics-project"
123+
placeholder="lerobot (default)"
158124
disabled={resumeLocked}
159125
/>
160126
</div>
161127
<div className="space-y-2">
162-
<Label htmlFor="wandb_entity">W&amp;B entity (optional)</Label>
128+
<Label htmlFor="wandb_entity">Entity</Label>
163129
<Input
164130
id="wandb_entity"
165131
value={config.wandb_entity || ""}
166132
onChange={(e) =>
167133
updateConfig("wandb_entity", e.target.value || undefined)
168134
}
169-
placeholder="your-username"
135+
placeholder="your-username or team"
170136
disabled={resumeLocked}
171137
/>
138+
{/* The 403 trap, stated as what the field IS rather than as a
139+
warning: W&B rejects a run aimed at an entity you aren't a
140+
member of, and it rejects it at run start, long after Start
141+
was clicked. Naming "a team you belong to" is what stops
142+
someone typing a placeholder word into it. */}
143+
<p className="text-xs text-muted-foreground">
144+
Your W&amp;B username or a team you belong to. Blank = your
145+
personal account.
146+
</p>
172147
</div>
173148
<div className="space-y-2">
174149
<Label htmlFor="wandb_notes">W&amp;B notes (optional)</Label>
@@ -209,7 +184,9 @@ const EssentialsCard: React.FC<ConfigComponentProps> = ({
209184
disabled={resumeLocked}
210185
className="data-[state=checked]:bg-primary"
211186
/>
212-
<Label htmlFor="wandb_disable_artifact">Disable artifacts</Label>
187+
<Label htmlFor="wandb_disable_artifact">
188+
Don't upload checkpoints to W&amp;B
189+
</Label>
213190
</div>
214191
</div>
215192
)}

frontend/src/hooks/useInstallExtra.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export interface UseInstallExtraResult {
2626
}
2727

2828
/**
29-
* Drives the backend extra-install flow (`accelerate`, `wandb`, …). Seeds state
29+
* Drives the backend extra-install flow (`accelerate`, policy extras, …). Seeds state
3030
* from `${endpointPrefix}/install-status`, polls while installing, and exposes
3131
* install/retry handlers. Pass `enabled=false` to gate seeding on dialog open.
3232
*/

0 commit comments

Comments
 (0)