Skip to content

Commit 59a5f94

Browse files
committed
feat(h07): DimensionsTab Apply dispatches brick params mutation
Closes V5-G19 from the App.tsx side: the DimensionsTab Apply button previously fired a callback that the parent ignored, so the inferred dimension never made it back into the spec — the table just kept showing the same auto-row forever. Now: - Sidebar exposes `onDimensionsApply` and threads it into DimensionsTab. - App's handler writes `entry.value` into the matching brick's `node.data.params[entry.param]`. - The verify debouncer's nodesKey now includes a JSON-serialised params hash, so the spec mutation re-triggers verify and the Dimensions table re-renders the row with source="user". Tests: - vbgui vitest Sidebar.test.tsx: new "H07: Sidebar forwards onDimensionsApply to DimensionsTab" — 27/27 Sidebar tests passing. - vbgui vitest full suite: 201/201 passing. - vbgui e2e 61_dimensions_feedback.spec.ts: load llama3_8b → open Dimensions → click first auto-Apply → assert source badge flips to "user" (or row drops entirely when the inferred value coincides with a user-set default), 1/1 passing. - vbgui e2e V6 suite (55–61) + canvas smoke regression: 15/15 passing — no regression in verify-after-mutation flow.
1 parent 90deea9 commit 59a5f94

4 files changed

Lines changed: 98 additions & 3 deletions

File tree

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// H07: DimensionsTab Apply dispatches a brick params mutation, and the
2+
// next verify cycle re-classifies the affected entry from
3+
// source="auto" → source="user" (closing the feedback loop).
4+
//
5+
// Closes V5-G19 from the UI side: previously the Apply button only
6+
// fired the callback locally; App.tsx ignored it. Now App writes the
7+
// inferred value back into the matching node.data.params, the verify
8+
// debouncer fires, and the row's source badge flips.
9+
10+
import { test, expect } from "@playwright/test";
11+
import { gotoApp, selectPreset } from "../fixtures";
12+
13+
test("H07: Apply on an auto-row flips its source badge to user",
14+
async ({ page }) => {
15+
test.setTimeout(60_000);
16+
await gotoApp(page);
17+
await selectPreset(page, "llama3_8b");
18+
19+
// Open Dimensions tab and wait for at least one auto row to appear
20+
// (mlp bricks have intermediate_size + activation auto-inferred).
21+
await page.getByTestId("sidebar-tab-dimensions").click();
22+
await page.getByTestId("dimensions-tab").waitFor();
23+
24+
// Pick the first auto Apply button on the page.
25+
const applyBtn = page.locator(
26+
"[data-testid^='dim-row-'][data-testid$='-apply']").first();
27+
await applyBtn.waitFor({ timeout: 8_000 });
28+
29+
// The button's testid encodes brick + param: dim-row-<brick>-<param>-apply.
30+
const tid = await applyBtn.getAttribute("data-testid");
31+
expect(tid).toBeTruthy();
32+
const match = tid!.match(/^dim-row-(.+)-(.+)-apply$/);
33+
expect(match).not.toBeNull();
34+
const [, brick, param] = match!;
35+
36+
// Pre-state: source badge for this row is "auto".
37+
const sourceBadge = page.getByTestId(`dim-source-${brick}-${param}`);
38+
expect((await sourceBadge.textContent())?.trim().toLowerCase())
39+
.toBe("auto");
40+
41+
// Click Apply — App writes the value into node.data.params and
42+
// verify-after-mutation re-renders the table.
43+
await applyBtn.click();
44+
45+
// Wait for the badge text to flip to "user" (verify debounce + render).
46+
await expect.poll(
47+
async () => {
48+
const b = page.getByTestId(`dim-source-${brick}-${param}`);
49+
const cnt = await b.count();
50+
if (cnt === 0) return "missing";
51+
const t = await b.textContent();
52+
return (t ?? "").trim().toLowerCase();
53+
},
54+
{ timeout: 8_000 },
55+
).toMatch(/^(user|missing)$/);
56+
});

vbgui/src/App.tsx

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,8 +187,14 @@ export function App(): JSX.Element {
187187
// Trigger verify only on inputs that the user controls. The verify
188188
// response writes back into spec AND edge severity — depending on
189189
// either of those in the effect would loop. Use structural keys.
190-
const nodesKey = nodes.map((n) => `${n.id}:${(n.data as { kind?: string })
191-
?.kind ?? ""}`).join("|");
190+
// H07: include params hash so the verify debouncer re-fires when the
191+
// user mutates brick.data.params via DimensionsTab Apply or
192+
// BrickContextPanel. Without this, the inferred-dimensions feedback
193+
// loop never closed because the table reflected stale verify state.
194+
const nodesKey = nodes.map((n) => {
195+
const d = n.data as { kind?: string; params?: Record<string, unknown> };
196+
return `${n.id}:${d.kind ?? ""}:${JSON.stringify(d.params ?? {})}`;
197+
}).join("|");
192198
const edgesKey = edges.map((e) => `${e.source}>${e.target}`).join("|");
193199
const lossKey = `${spec.loss.kind}::${spec.loss.head_outputs.join(",")}`;
194200
const optimKey = `${spec.optim.kind}::${spec.optim.groups.length}`;
@@ -532,6 +538,23 @@ export function App(): JSX.Element {
532538
onShardingChange={(s) =>
533539
dispatch({ type: "sharding.set", sharding: s })}
534540
onShardingAccept={handleShardingAccept}
541+
onDimensionsApply={(entry) => {
542+
// H07: write the auto-suggested value into the matched
543+
// brick's params. Re-verify will then re-render the
544+
// entry as source="user" (or drop it entirely if the
545+
// inference no longer fires), closing the feedback loop.
546+
setNodes((prev) => prev.map((n) => {
547+
if (n.id !== entry.brick) return n;
548+
const d = (n.data ?? {}) as {
549+
kind?: string; params?: Record<string, unknown>;
550+
};
551+
return { ...n, data: {
552+
...d,
553+
params: { ...(d.params ?? {}),
554+
[entry.param]: entry.value },
555+
} as never };
556+
}));
557+
}}
535558
/>
536559
</>
537560
)}

vbgui/src/components/Sidebar.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@ export interface SidebarProps {
4747
/** E7-2: inferred dimensions populated from verify response. */
4848
inferenceLog?: InferenceEntryClient[];
4949
onHighlightBrick?: (brick: string) => void;
50+
/** H07: parent dispatches the per-brick params mutation when the
51+
* user clicks Apply on an auto-inferred row in DimensionsTab. */
52+
onDimensionsApply?: (entry: InferenceEntryClient) => void;
5053
}
5154

5255
const TAB_LABELS: { key: SidebarTab; label: string }[] = [
@@ -121,7 +124,8 @@ export function Sidebar(p: SidebarProps): JSX.Element {
121124
)}
122125
{active === "dimensions" && (
123126
<DimensionsTab log={p.inferenceLog ?? []}
124-
onHighlight={p.onHighlightBrick} />
127+
onHighlight={p.onHighlightBrick}
128+
onApply={p.onDimensionsApply} />
125129
)}
126130
{active === "ablations" && (
127131
<AblationsTab rpc={p.rpc ?? null}

vbgui/tests/Sidebar.test.tsx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,4 +312,16 @@ describe("Sidebar", () => {
312312
fireEvent.click(screen.getByTestId("sidebar-tab-gotchas"));
313313
expect(screen.getByTestId("gotchas-tab")).toBeTruthy();
314314
});
315+
316+
it("H07: Sidebar forwards onDimensionsApply to DimensionsTab", () => {
317+
const onDimensionsApply = vi.fn();
318+
const entry = { brick: "attn0", param: "head_dim", value: 64,
319+
source: "auto" as const, reason: "inferred from H/nh" };
320+
render(<Sidebar {...stubs}
321+
inferenceLog={[entry]}
322+
onDimensionsApply={onDimensionsApply} />);
323+
fireEvent.click(screen.getByTestId("sidebar-tab-dimensions"));
324+
fireEvent.click(screen.getByTestId("dim-row-attn0-head_dim-apply"));
325+
expect(onDimensionsApply).toHaveBeenCalledWith(entry);
326+
});
315327
});

0 commit comments

Comments
 (0)