diff --git a/e2e/follow-up-steering.spec.ts b/e2e/follow-up-steering.spec.ts new file mode 100644 index 0000000..6034823 --- /dev/null +++ b/e2e/follow-up-steering.spec.ts @@ -0,0 +1,53 @@ +import { expect, test } from "@playwright/test"; + +test("a follow-up steers the active agent run before it completes", async ({ page }) => { + await page.goto("/"); + const [newChatResponse] = await Promise.all([ + page.waitForResponse((response) => + response.request().method() === "POST" && new URL(response.url()).pathname === "/api/conversations", + ), + page.getByTestId("sidebar").getByRole("button", { name: "New chat", exact: true }).first().click(), + ]); + expect(newChatResponse.ok()).toBe(true); + const { id: conversationId } = await newChatResponse.json() as { id: string }; + + const composer = page.getByTestId("composer-input"); + await expect(composer).toBeEnabled({ timeout: 60_000 }); + await composer.fill("follow-up-steering-hold: start a box design"); + await page.getByTestId("composer-send").click(); + + await expect.poll(async () => { + const status = await ( + await page.request.get(`/api/test/fake-model-holds?conversationId=${conversationId}`) + ).json() as { held: boolean }; + return status.held; + }).toBe(true); + + await composer.fill("change the width to 40 mm"); + await page.getByTestId("composer-send").click(); + await expect(page.getByTestId("queued-message")).toContainText("change the width to 40 mm"); + await expect.poll(async () => { + const result = await ( + await page.request.post(`/api/test/fake-model-holds/release?conversationId=${conversationId}`) + ).json() as { released: boolean }; + return result.released; + }).toBe(true); + + await expect(page.getByText("Correction consumed by the active run before it completed.")).toBeVisible(); + await expect(page.getByTestId("queued-message")).toHaveCount(0); + + const diagnostics = await ( + await page.request.get(`/api/test/fake-model-requests?conversationId=${conversationId}`) + ).json() as { requests: Array<{ sequence: number; messageCount: number }> }; + expect(diagnostics.requests).toEqual([ + expect.objectContaining({ sequence: 1, messageCount: 1 }), + expect.objectContaining({ sequence: 2, messageCount: 3 }), + ]); + + const rows = await ( + await page.request.get(`/api/conversations/${conversationId}/messages`) + ).json() as Array<{ seq: number; role: string; contentJson: string }>; + const corrections = rows.filter((row) => row.contentJson.includes("change the width to 40 mm")); + expect(corrections).toHaveLength(1); + expect(corrections[0]).toMatchObject({ seq: 2, role: "user" }); +}); diff --git a/e2e/params.spec.ts b/e2e/params.spec.ts index 04fbd9f..c45a0c2 100644 --- a/e2e/params.spec.ts +++ b/e2e/params.spec.ts @@ -1,4 +1,39 @@ -import { expect, test } from "@playwright/test"; +import { expect, test, type Page } from "@playwright/test"; +import sharp from "sharp"; + +async function maxChannelDeviation(png: Buffer): Promise { + const stats = await sharp(png).removeAlpha().stats(); + return Math.max(...stats.channels.map((channel) => channel.stdev)); +} + +async function meanPixelDifference(before: Buffer, after: Buffer): Promise { + const stats = await sharp(before) + .composite([{ input: after, blend: "difference" }]) + .removeAlpha() + .stats(); + return Math.max(...stats.channels.map((channel) => channel.mean)); +} + +async function waitForViewerPaint(page: Page): Promise { + await page.evaluate(async () => { + await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(() => resolve()))); + }); +} + +async function dragSlider(page: Page, name: string, fraction: number): Promise { + const thumb = page.getByTestId(`param-${name}`).getByRole("slider"); + const track = thumb.locator("xpath=.."); + const box = await track.boundingBox(); + if (!box) throw new Error(`Slider track for ${name} has no bounding box`); + await thumb.hover(); + await page.mouse.down(); + await page.mouse.move( + box.x + Math.max(1, Math.min(box.width - 1, box.width * fraction)), + box.y + box.height / 2, + { steps: 8 }, + ); + await page.mouse.up(); +} // Fake-LLM mode (CHAMFER_FAKE_LLM=1): the scripted agent turn produces a // parametric 10x20x30 box whose dimensions are params. Committing a new width @@ -56,3 +91,94 @@ test("param slider re-runs locally without a new chat message", async ({ page }) await expect(rightPanel.getByTestId("params-panel")).toHaveCount(0); await expect(rightPanel.getByTestId("export-step")).toBeDisabled(); }); + +test("pointer dragging accepts responsive geometry and rejects an ineffective parameter", async ({ page }, testInfo) => { + test.setTimeout(600_000); + await page.setViewportSize({ width: 1440, height: 900 }); + await page.goto("/"); + + const created = page.waitForResponse( + (response) => response.url().includes("/api/conversations") && response.request().method() === "POST", + ); + await page.getByTestId("sidebar").getByRole("button", { name: "New chat", exact: true }).first().click(); + const conversation = (await (await created).json()) as { id: string }; + + const rightPanel = page.getByTestId("right-panel"); + await rightPanel.getByTestId("script-panel-toggle").click(); + const scriptInput = rightPanel.getByTestId("script-input"); + const measurements = rightPanel.getByTestId("measurements"); + const responsiveCode = `# --- params --- +width = 10 # [10, 100] Overall width in mm +# --- end params --- +from build123d import * +result = Box(width, 20, 30)`; + + await scriptInput.fill(responsiveCode); + await rightPanel.getByTestId("script-run").click(); + await expect(measurements).toContainText("10 x 20 x 30", { timeout: 600_000 }); + await rightPanel.getByTestId("params-panel-toggle").click(); + + const canvas = rightPanel.locator("canvas"); + await waitForViewerPaint(page); + const beforePixels = await canvas.screenshot({ path: testInfo.outputPath("responsive-before.png") }); + expect(await maxChannelDeviation(beforePixels)).toBeGreaterThan(5); + + await dragSlider(page, "width", 1); + const widthSlider = rightPanel.getByTestId("param-width").getByRole("slider"); + await expect(widthSlider).not.toHaveAttribute("aria-valuenow", "10"); + const responsiveWidth = Number(await widthSlider.getAttribute("aria-valuenow")); + expect(responsiveWidth).toBeGreaterThan(10); + await expect(measurements).toContainText(`${responsiveWidth} x 20 x 30`, { timeout: 120_000 }); + await expect(measurements).toContainText(String(responsiveWidth * 600)); + await expect(rightPanel.getByTestId("viewer")).toHaveAttribute("data-has-geometry", "true"); + await waitForViewerPaint(page); + const afterPixels = await canvas.screenshot({ path: testInfo.outputPath("responsive-after.png") }); + expect(await maxChannelDeviation(afterPixels)).toBeGreaterThan(5); + expect(await meanPixelDifference(beforePixels, afterPixels)).toBeGreaterThan(1); + + await expect.poll(async () => { + const response = await page.request.get(`/api/conversations/${conversation.id}/artifacts`); + return ((await response.json()) as unknown[]).length; + }).toBe(1); + const artifactsAfterValidDrag = await page.request.get(`/api/conversations/${conversation.id}/artifacts`); + const validArtifacts = (await artifactsAfterValidDrag.json()) as Array<{ pySource: string }>; + expect(validArtifacts).toHaveLength(1); + expect(validArtifacts[0]?.pySource).toContain(`width = ${responsiveWidth}`); + + const ineffectiveCode = `# --- params --- +width = 10 # [10, 100] Overall width in mm +# --- end params --- +from build123d import * +result = Box(10, 20, 30)`; + await scriptInput.fill(ineffectiveCode); + await rightPanel.getByTestId("script-run").click(); + await expect(measurements).toContainText("10 x 20 x 30", { timeout: 120_000 }); + await expect(rightPanel.getByTestId("param-width").getByRole("slider")).toHaveAttribute( + "aria-valuenow", + "10", + ); + + await dragSlider(page, "width", 1); + const error = rightPanel.getByTestId("param-error"); + await expect(error).toContainText("Parameter `width` does not change the executed geometry", { + timeout: 120_000, + }); + await expect(measurements).toContainText("10 x 20 x 30"); + + const artifactsAfterRejectedDrag = await page.request.get( + `/api/conversations/${conversation.id}/artifacts`, + ); + expect((await artifactsAfterRejectedDrag.json()) as unknown[]).toHaveLength(1); + + await rightPanel.getByTestId("param-input-width").fill("10"); + await rightPanel.getByTestId("param-input-width").press("Enter"); + await expect(error).toHaveCount(0); + await expect(rightPanel.getByTestId("param-width").getByRole("slider")).toHaveAttribute("aria-valuenow", "10"); + + await page.reload(); + await rightPanel.getByTestId("script-panel-toggle").click(); + await expect(rightPanel.getByTestId("measurements")).toContainText(`${responsiveWidth} x 20 x 30`, { + timeout: 600_000, + }); + await expect(rightPanel.getByTestId("export-step")).toBeEnabled(); +}); diff --git a/packages/client/public/py/harness.py b/packages/client/public/py/harness.py index 100469c..5e85d32 100644 --- a/packages/client/public/py/harness.py +++ b/packages/client/public/py/harness.py @@ -5,6 +5,7 @@ import ast import contextlib import io +import math import re import traceback @@ -16,6 +17,8 @@ _EXPECT_REQUIRED_KEYS = ("bodies", "bbox_mm") _EXPECT_ALLOWED_KEYS = frozenset({"bodies", "bbox_mm", "bbox_tol", "volume_mm3"}) DEFAULT_BBOX_TOL = 0.5 +PARAMETER_GEOMETRY_REL_TOL = 1e-7 +PARAMETER_GEOMETRY_ABS_TOL = 1e-7 CHECKS_START = "# --- checks ---" CHECKS_END = "# --- end checks ---" @@ -177,6 +180,139 @@ def set_params(source: str, values: dict[str, float]) -> str: return "\n".join(lines) +def _parameter_probe_values(value, lo, hi): + """Return deterministic interior probe values for one visible parameter.""" + if not all(_is_number(number) and math.isfinite(number) for number in (value, lo, hi)): + return [] + if lo >= hi or value < lo or value > hi: + return [] + + integral = all(isinstance(number, int) for number in (value, lo, hi)) + span = hi - lo + raw_candidates = (lo + span * 0.25, lo + span * 0.75) + if integral: + candidates = [int(round(candidate)) for candidate in raw_candidates] + candidates.extend((lo, hi)) + else: + candidates = list(raw_candidates) + + unique = [] + for candidate in candidates: + if candidate != value and candidate not in unique and lo <= candidate <= hi: + unique.append(candidate) + return sorted(unique, key=lambda candidate: (-abs(candidate - value), candidate)) + + +def _geometry_signature(shape): + """Executed geometry evidence used to compare a parameter probe with its baseline.""" + bb = shape.bounding_box() + center = shape.center() + vertices, _triangles = shape.tessellate(tolerance=0.1) + points = sorted((float(v.X), float(v.Y), float(v.Z)) for v in vertices) + return { + "bbox": ( + float(bb.min.X), float(bb.min.Y), float(bb.min.Z), + float(bb.max.X), float(bb.max.Y), float(bb.max.Z), + ), + "center": (float(center.X), float(center.Y), float(center.Z)), + "volume": float(shape.volume), + "area": float(shape.area), + "points": points, + } + + +def _numbers_close(left, right, scale=1.0): + return math.isclose( + left, + right, + rel_tol=PARAMETER_GEOMETRY_REL_TOL, + abs_tol=max(PARAMETER_GEOMETRY_ABS_TOL, abs(scale) * PARAMETER_GEOMETRY_REL_TOL), + ) + + +def _same_executed_geometry(left, right): + scale = max( + (abs(value) for value in left["bbox"] + right["bbox"]), + default=1.0, + ) + if len(left["points"]) != len(right["points"]): + return False + if not all( + _numbers_close(a, b, scale) + for a, b in zip(left["bbox"] + left["center"], right["bbox"] + right["center"]) + ): + return False + if not _numbers_close(left["volume"], right["volume"], max(abs(left["volume"]), abs(right["volume"]), 1.0)): + return False + if not _numbers_close(left["area"], right["area"], max(abs(left["area"]), abs(right["area"]), 1.0)): + return False + return all( + _numbers_close(a, b, scale) + for left_point, right_point in zip(left["points"], right["points"]) + for a, b in zip(left_point, right_point) + ) + + +def _parameter_responsiveness_checks(source, shape): + baseline = _geometry_signature(shape) + checks = [] + for spec in parse_params(source): + name = spec["name"] + probes = _parameter_probe_values(spec["value"], spec["min"], spec["max"]) + if not probes: + checks.append( + _gate_check( + f"parameter_{name}", + False, + f"Parameter `{name}` needs a valid adjustable range containing its current value; " + f"found value {spec['value']} with range [{spec['min']}, {spec['max']}].", + ) + ) + continue + + probe_errors = [] + responsive_at = None + for probe in probes: + try: + probe_source = set_params(source, {name: probe}) + probe_result, _stdout = _execute(probe_source) + probe_shape = _to_shape(probe_result) + if not _same_executed_geometry(baseline, _geometry_signature(probe_shape)): + responsive_at = probe + break + except Exception as error: + probe_errors.append(f"{probe}: {error}") + + if responsive_at is not None: + checks.append( + _gate_check( + f"parameter_{name}", + True, + f"Parameter `{name}` changes the executed geometry at probe value {responsive_at}.", + ) + ) + elif probe_errors and len(probe_errors) == len(probes): + checks.append( + _gate_check( + f"parameter_{name}", + False, + f"Parameter `{name}` could not be verified because every in-range probe failed: " + + "; ".join(probe_errors), + ) + ) + else: + checks.append( + _gate_check( + f"parameter_{name}", + False, + f"Parameter `{name}` does not change the executed geometry at deterministic " + f"in-range probes {probes}. Use `{name}` to derive a dimension, feature, or placement " + "of `result`, or remove it from the params block.", + ) + ) + return checks + + def _is_number(value): return isinstance(value, (int, float)) and not isinstance(value, bool) @@ -909,6 +1045,7 @@ def _run_gate_checks(source, shape): _gate_check("valid", shape.is_valid, "B-rep validity (is_valid)"), _gate_check("nondegenerate", volume > 0, f"total volume {volume:.6g} mm^3 must be > 0"), ] + checks.extend(_parameter_responsiveness_checks(source, shape)) try: parse_component(source) except ValueError as e: diff --git a/packages/client/src/agent/session.test.ts b/packages/client/src/agent/session.test.ts index 821f3b0..aff79bf 100644 --- a/packages/client/src/agent/session.test.ts +++ b/packages/client/src/agent/session.test.ts @@ -192,6 +192,383 @@ describe("createSession", () => { expect(finalState?.messages).toHaveLength(2); }); + it("consumes a user correction in the active pi run before the original send settles", async () => { + const turnContexts: Array<{ messages: unknown[] }> = []; + let finishFirstTurn: (() => void) | undefined; + const streamFn = vi.fn((_model: unknown, context: { messages: unknown[] }) => { + turnContexts.push(context); + const stream = createAssistantMessageEventStream(); + const response: AssistantMessage = { + role: "assistant", + content: [{ type: "text", text: turnContexts.length === 1 ? "Working on the first request." : "Applied the correction." }], + api: FAKE_MODEL.api, + provider: FAKE_MODEL.provider, + model: FAKE_MODEL.id, + usage: ZERO_USAGE, + stopReason: "stop", + timestamp: Date.now(), + }; + const finish = () => { + stream.push({ type: "start", partial: response }); + stream.push({ type: "done", reason: "stop", message: response }); + stream.end(response); + }; + if (turnContexts.length === 1) finishFirstTurn = finish; + else queueMicrotask(finish); + return stream; + }); + const session = createSession({ + conversationId: "conv-1", + modelJson: JSON.stringify(FAKE_MODEL), + systemPrompt, + priorMessages: [], + __streamFn: streamFn, + } as unknown as Parameters[0]); + + let originalSettled = false; + const originalSend = session.send("Make a 10 mm wide box").then(() => { + originalSettled = true; + }); + await vi.waitUntil(() => finishFirstTurn !== undefined); + + const correction = session.steer("correction-1", "Change the width to 40 mm", []); + expect(originalSettled).toBe(false); + finishFirstTurn?.(); + await Promise.all([originalSend, correction]); + + expect(turnContexts).toHaveLength(2); + expect(JSON.stringify(turnContexts[1]?.messages)).toContain("Change the width to 40 mm"); + const persistedUsers = vi.mocked(rest.postMessage).mock.calls + .map((call) => JSON.parse(call[1].contentJson) as { role?: string; content?: unknown }) + .filter((message) => message.role === "user"); + expect(persistedUsers).toHaveLength(2); + expect(JSON.stringify(persistedUsers[1])).toContain("Change the width to 40 mm"); + }); + + it("consumes multiple corrections one at a time in send order", async () => { + const turnContexts: Array<{ messages: unknown[] }> = []; + let finishFirstTurn: (() => void) | undefined; + const streamFn = vi.fn((_model: unknown, context: { messages: unknown[] }) => { + turnContexts.push(context); + const stream = createAssistantMessageEventStream(); + const response: AssistantMessage = { + role: "assistant", + content: [{ type: "text", text: `response ${turnContexts.length}` }], + api: FAKE_MODEL.api, + provider: FAKE_MODEL.provider, + model: FAKE_MODEL.id, + usage: ZERO_USAGE, + stopReason: "stop", + timestamp: Date.now(), + }; + const finish = () => { + stream.push({ type: "start", partial: response }); + stream.push({ type: "done", reason: "stop", message: response }); + stream.end(response); + }; + if (turnContexts.length === 1) finishFirstTurn = finish; + else queueMicrotask(finish); + return stream; + }); + const session = createSession({ + conversationId: "conv-1", + modelJson: JSON.stringify(FAKE_MODEL), + systemPrompt, + priorMessages: [], + __streamFn: streamFn, + } as unknown as Parameters[0]); + + const original = session.send("Start"); + await vi.waitUntil(() => finishFirstTurn !== undefined); + const first = session.steer("correction-1", "First correction", []); + const second = session.steer("correction-2", "Second correction", []); + finishFirstTurn?.(); + await Promise.all([original, first, second]); + + expect(turnContexts).toHaveLength(3); + expect(JSON.stringify(turnContexts[1]?.messages)).toContain("First correction"); + expect(JSON.stringify(turnContexts[1]?.messages)).not.toContain("Second correction"); + expect(JSON.stringify(turnContexts[2]?.messages)).toContain("Second correction"); + const persistedUserText = vi.mocked(rest.postMessage).mock.calls + .map((call) => JSON.stringify(JSON.parse(call[1].contentJson))) + .filter((serialized) => serialized.includes('"role":"user"')); + expect(persistedUserText).toEqual([ + expect.stringContaining("Start"), + expect.stringContaining("First correction"), + expect.stringContaining("Second correction"), + ]); + }); + + it("keeps a delayed image correction ahead of a later text correction at the turn boundary", async () => { + let releaseImage: (() => void) | undefined; + const readSpy = vi.spyOn(FileReader.prototype, "readAsDataURL").mockImplementation(function (this: FileReader, file) { + releaseImage = () => { + Object.defineProperty(this, "result", { configurable: true, value: "data:image/png;base64,AQID" }); + this.onload?.(new ProgressEvent("load") as ProgressEvent); + }; + void file; + }); + try { + const turnContexts: Array<{ messages: unknown[] }> = []; + let finishFirstTurn: (() => void) | undefined; + const streamFn = vi.fn((_model: unknown, context: { messages: unknown[] }) => { + turnContexts.push(context); + const stream = createAssistantMessageEventStream(); + const response: AssistantMessage = { + role: "assistant", + content: [{ type: "text", text: `response ${turnContexts.length}` }], + api: FAKE_MODEL.api, + provider: FAKE_MODEL.provider, + model: FAKE_MODEL.id, + usage: ZERO_USAGE, + stopReason: "stop", + timestamp: Date.now(), + }; + const finish = () => { + stream.push({ type: "start", partial: response }); + stream.push({ type: "done", reason: "stop", message: response }); + stream.end(response); + }; + if (turnContexts.length === 1) finishFirstTurn = finish; + else queueMicrotask(finish); + return stream; + }); + const session = createSession({ + conversationId: "conv-1", + modelJson: JSON.stringify(FAKE_MODEL), + systemPrompt, + priorMessages: [], + __streamFn: streamFn, + } as unknown as Parameters[0]); + const image = new File([new Uint8Array([1, 2, 3])], "slow.png", { type: "image/png" }); + + const original = session.send("Start"); + await vi.waitUntil(() => finishFirstTurn !== undefined); + const imageCorrection = session.steer("slow-image", "Use this image first", [image]); + const textCorrection = session.steer("later-text", "Then make it wider", []); + await vi.waitUntil(() => releaseImage !== undefined); + finishFirstTurn?.(); + await new Promise((resolve) => setTimeout(resolve, 0)); + + expect(turnContexts).toHaveLength(1); + releaseImage?.(); + await Promise.all([original, imageCorrection, textCorrection]); + + expect(turnContexts).toHaveLength(3); + expect(JSON.stringify(turnContexts[1]?.messages.at(-1))).toContain("Use this image first"); + expect(JSON.stringify(turnContexts[2]?.messages.at(-1))).toContain("Then make it wider"); + const persistedUsers = [ + ...vi.mocked(rest.postMessage).mock.calls.map((call) => call[1]), + ...vi.mocked(rest.postMessageWithAttachments).mock.calls.map((call) => call[1]), + ] + .filter((row) => row.role === "user") + .sort((left, right) => left.seq - right.seq) + .map((row) => row.contentJson); + expect(persistedUsers).toEqual([ + expect.stringContaining("Start"), + expect.stringContaining("Use this image first"), + expect.stringContaining("Then make it wider"), + ]); + } finally { + readSpy.mockRestore(); + } + }); + + it("activates each queued image reference only when pi consumes that steering message", async () => { + const turnContexts: Array<{ messages: unknown[] }> = []; + let finishFirstTurn: (() => void) | undefined; + const streamFn = vi.fn((_model: unknown, context: { messages: unknown[] }) => { + turnContexts.push(context); + const stream = createAssistantMessageEventStream(); + const response: AssistantMessage = { + role: "assistant", + content: [{ type: "text", text: `response ${turnContexts.length}` }], + api: FAKE_MODEL.api, + provider: FAKE_MODEL.provider, + model: FAKE_MODEL.id, + usage: ZERO_USAGE, + stopReason: "stop", + timestamp: Date.now(), + }; + const finish = () => { + stream.push({ type: "start", partial: response }); + stream.push({ type: "done", reason: "stop", message: response }); + stream.end(response); + }; + if (turnContexts.length === 1) finishFirstTurn = finish; + else queueMicrotask(finish); + return stream; + }); + const session = createSession({ + conversationId: "conv-1", + modelJson: JSON.stringify(FAKE_MODEL), + systemPrompt, + priorMessages: [], + __streamFn: streamFn, + } as unknown as Parameters[0]); + const firstImage = new File([new Uint8Array([1])], "first.png", { type: "image/png" }); + const secondImage = new File([new Uint8Array([2])], "second.png", { type: "image/png" }); + const waitForRead = (file: File) => new Promise((resolve, reject) => { + const reader = new FileReader(); + reader.onload = () => resolve(); + reader.onerror = () => reject(reader.error); + reader.readAsDataURL(file); + }); + + const original = session.send("Start"); + await vi.waitUntil(() => finishFirstTurn !== undefined); + const first = session.steer("image-1", "First image", [firstImage]); + const second = session.steer("image-2", "Second image", [secondImage]); + await Promise.all([waitForRead(firstImage), waitForRead(secondImage)]); + await Promise.resolve(); + finishFirstTurn?.(); + await Promise.all([original, first, second]); + + const pendingIds = (context: { messages: unknown[] }) => { + const ids = new Set(); + for (const match of JSON.stringify(context.messages).matchAll(/Pending reference images: ([^.\]]+)/g)) { + for (const id of (match[1] ?? "").split(",")) ids.add(id.trim()); + } + return ids; + }; + const secondRequestIds = pendingIds(turnContexts[1]!); + const thirdRequestIds = pendingIds(turnContexts[2]!); + expect(secondRequestIds.size).toBe(1); + expect(thirdRequestIds.size).toBe(2); + expect([...secondRequestIds].every((id) => thirdRequestIds.has(id))).toBe(true); + expect(vi.mocked(rest.postMessageWithAttachments).mock.calls.map((call) => call[1].seq)).toEqual([2, 4]); + }); + + it("cancelling a prepared image correction leaves no reference gate in the next run", async () => { + const turnContexts: Array<{ messages: unknown[] }> = []; + let finishFirstTurn: (() => void) | undefined; + const execute = vi.fn(async () => ({ content: [{ type: "text" as const, text: "ran" }], details: {} })); + const tool = { + name: "run_build123d", + label: "Run build123d", + description: "fake run tool", + parameters: Type.Object({ code: Type.String() }), + execute, + }; + const streamFn = vi.fn((_model: unknown, context: { messages: unknown[] }) => { + turnContexts.push(context); + const stream = createAssistantMessageEventStream(); + const response: AssistantMessage = turnContexts.length === 2 + ? { + role: "assistant", + content: [{ type: "toolCall", id: "clean-run", name: "run_build123d", arguments: { code: "result = Box(1, 1, 1)" } }], + api: FAKE_MODEL.api, + provider: FAKE_MODEL.provider, + model: FAKE_MODEL.id, + usage: ZERO_USAGE, + stopReason: "toolUse", + timestamp: Date.now(), + } + : { + role: "assistant", + content: [{ type: "text", text: "done" }], + api: FAKE_MODEL.api, + provider: FAKE_MODEL.provider, + model: FAKE_MODEL.id, + usage: ZERO_USAGE, + stopReason: "stop", + timestamp: Date.now(), + }; + const finish = () => { + stream.push({ type: "start", partial: response }); + stream.push({ type: "done", reason: response.stopReason === "toolUse" ? "toolUse" : "stop", message: response }); + stream.end(response); + }; + if (turnContexts.length === 1) finishFirstTurn = finish; + else queueMicrotask(finish); + return stream; + }); + const session = createSession({ + conversationId: "conv-1", + modelJson: JSON.stringify(FAKE_MODEL), + systemPrompt, + tools: [tool], + priorMessages: [], + __streamFn: streamFn, + } as unknown as Parameters[0]); + const image = new File([new Uint8Array([1, 2, 3])], "cancel.png", { type: "image/png" }); + + const original = session.send("Start"); + await vi.waitUntil(() => finishFirstTurn !== undefined); + const correction = session.steer("cancel-image", "Ignore this image", [image]); + await new Promise((resolve, reject) => { + const reader = new FileReader(); + reader.onload = () => resolve(); + reader.onerror = () => reject(reader.error); + reader.readAsDataURL(image); + }); + await Promise.resolve(); + session.cancelSteering("cancel-image"); + await expect(correction).resolves.toBe("cancelled"); + finishFirstTurn?.(); + await original; + + await session.send("Start a clean run"); + expect(execute).toHaveBeenCalledTimes(1); + }); + + it("preserves image content and atomic attachment persistence for a steered correction", async () => { + const turnContexts: Array<{ messages: Array<{ content?: Array<{ type?: string; data?: string }> }> }> = []; + let finishFirstTurn: (() => void) | undefined; + const streamFn = vi.fn((_model: unknown, context: { messages: Array<{ content?: Array<{ type?: string; data?: string }> }> }) => { + turnContexts.push(context); + const stream = createAssistantMessageEventStream(); + const response: AssistantMessage = { + role: "assistant", + content: [{ type: "text", text: "done" }], + api: FAKE_MODEL.api, + provider: FAKE_MODEL.provider, + model: FAKE_MODEL.id, + usage: ZERO_USAGE, + stopReason: "stop", + timestamp: Date.now(), + }; + const finish = () => { + stream.push({ type: "start", partial: response }); + stream.push({ type: "done", reason: "stop", message: response }); + stream.end(response); + }; + if (turnContexts.length === 1) finishFirstTurn = finish; + else queueMicrotask(finish); + return stream; + }); + const session = createSession({ + conversationId: "conv-1", + modelJson: JSON.stringify(FAKE_MODEL), + systemPrompt, + priorMessages: [], + __streamFn: streamFn as never, + } as unknown as Parameters[0]); + const image = new File([new Uint8Array([1, 2, 3])], "correction.png", { type: "image/png" }); + + const original = session.send("Start"); + await vi.waitUntil(() => finishFirstTurn !== undefined); + const correction = session.steer("image-correction", "Use this reference instead", [image]); + await new Promise((resolve, reject) => { + const reader = new FileReader(); + reader.onload = () => resolve(); + reader.onerror = () => reject(reader.error); + reader.readAsDataURL(image); + }); + await Promise.resolve(); + finishFirstTurn?.(); + await Promise.all([original, correction]); + + const secondRequestImages = turnContexts[1]?.messages.flatMap((message) => message.content ?? []) + .filter((block) => block.type === "image"); + expect(secondRequestImages).toEqual([expect.objectContaining({ type: "image", data: "AQID" })]); + const atomicCalls = vi.mocked(rest.postMessageWithAttachments).mock.calls; + expect(atomicCalls).toHaveLength(1); + const durable = JSON.parse(atomicCalls[0]![1].contentJson); + expect(durable.content.map((block: { type: string }) => block.type)).toEqual(["text", "attachment-reference"]); + expect(atomicCalls[0]![2]).toEqual([expect.objectContaining({ kind: "user-image", data: "AQID" })]); + }); + it("recovers after a mid-stream network error without persisting the interrupted assistant response", async () => { pinResolvingPostMessage(); const successfulAttempt = makeFakeStreamFn() as StreamFn; @@ -358,6 +735,144 @@ describe("createSession", () => { expect(latest?.error).toBeUndefined(); }); + it("abort cancels pending steering so it cannot leak into a later run", async () => { + const laterStream = makeFakeStreamFn() as StreamFn; + const turnContexts: Array<{ messages: unknown[] }> = []; + let firstRunStarted = false; + const streamFn = vi.fn( + (_model: unknown, context: { messages: unknown[] }, options?: { signal?: AbortSignal }) => { + turnContexts.push(context); + if (turnContexts.length > 1) return laterStream(_model as never, context as never, options as never); + firstRunStarted = true; + return new Promise((_resolve, reject) => { + const fail = () => reject(options?.signal?.reason ?? new Error("aborted")); + if (options?.signal?.aborted) fail(); + else options?.signal?.addEventListener("abort", fail); + }); + }, + ); + const session = createSession({ + conversationId: "conv-1", + modelJson: JSON.stringify(FAKE_MODEL), + systemPrompt, + priorMessages: [], + __streamFn: streamFn as never, + } as unknown as Parameters[0]); + + const activeSend = session.send("Start the first run"); + await vi.waitUntil(() => firstRunStarted); + const steering = session.steer("stopped-correction", "This correction belongs only to the stopped run", []); + session.abort(); + await activeSend; + + expect(await Promise.race([steering, Promise.resolve("still-pending")])).toBe("cancelled"); + await session.send("Start a clean run"); + expect(JSON.stringify(turnContexts.at(-1)?.messages)).not.toContain("This correction belongs only to the stopped run"); + }); + + it("provider termination cancels unconsumed steering so a later run cannot inherit it", async () => { + const laterStream = makeFakeStreamFn() as StreamFn; + const turnContexts: Array<{ messages: unknown[] }> = []; + let rejectFirst: ((error: Error) => void) | undefined; + const streamFn = vi.fn( + (_model: unknown, context: { messages: unknown[] }, options?: { signal?: AbortSignal }) => { + turnContexts.push(context); + if (turnContexts.length > 1) return laterStream(_model as never, context as never, options as never); + return new Promise((_resolve, reject) => { + rejectFirst = reject; + }); + }, + ); + const session = createSession({ + conversationId: "conv-1", + modelJson: JSON.stringify(FAKE_MODEL), + systemPrompt, + priorMessages: [], + __streamFn: streamFn as never, + } as unknown as Parameters[0]); + + const failedSend = session.send("Start the failing run"); + await vi.waitUntil(() => rejectFirst !== undefined); + const steering = session.steer("failed-correction", "Do not leak this correction", []); + rejectFirst?.(new Error("invalid request shape")); + + await failedSend; + await expect(steering).resolves.toBe("cancelled"); + await session.send("Start a clean run"); + expect(JSON.stringify(turnContexts.at(-1)?.messages)).not.toContain("Do not leak this correction"); + }); + + it("gives user steering the next model turn before an autonomous self-check", async () => { + const turnContexts: Array<{ messages: unknown[] }> = []; + let resolveTool: (() => void) | undefined; + const execute = vi.fn( + () => + new Promise<{ content: [{ type: "text"; text: string }]; details: { gate: { status: "passed"; checks: [] } } }>((resolve) => { + resolveTool = () => resolve({ + content: [{ type: "text", text: "ran" }], + details: { gate: { status: "passed", checks: [] } }, + }); + }), + ); + const tool = { + name: "run_build123d", + label: "Run build123d", + description: "fake run tool", + parameters: Type.Object({ code: Type.String() }), + execute, + }; + const streamFn = vi.fn((_model: unknown, context: { messages: unknown[] }) => { + turnContexts.push(context); + const stream = createAssistantMessageEventStream(); + const response: AssistantMessage = turnContexts.length === 1 + ? { + role: "assistant", + content: [{ type: "toolCall", id: "run-1", name: "run_build123d", arguments: { code: "result = Box(1, 1, 1)" } }], + api: FAKE_MODEL.api, + provider: FAKE_MODEL.provider, + model: FAKE_MODEL.id, + usage: ZERO_USAGE, + stopReason: "toolUse", + timestamp: Date.now(), + } + : { + role: "assistant", + content: [{ type: "text", text: "done" }], + api: FAKE_MODEL.api, + provider: FAKE_MODEL.provider, + model: FAKE_MODEL.id, + usage: ZERO_USAGE, + stopReason: "stop", + timestamp: Date.now(), + }; + queueMicrotask(() => { + stream.push({ type: "start", partial: response }); + stream.push({ type: "done", reason: response.stopReason === "toolUse" ? "toolUse" : "stop", message: response }); + stream.end(response); + }); + return stream; + }); + const session = createSession({ + conversationId: "conv-1", + modelJson: JSON.stringify(FAKE_MODEL), + systemPrompt, + tools: [tool], + priorMessages: [], + __streamFn: streamFn, + } as unknown as Parameters[0]); + + const original = session.send("Build a box"); + await vi.waitUntil(() => resolveTool !== undefined); + const steering = session.steer("priority-correction", "Make the box 40 mm wide", []); + resolveTool?.(); + await Promise.all([original, steering]); + + expect(turnContexts).toHaveLength(3); + expect(JSON.stringify(turnContexts[1]?.messages.at(-1))).toContain("Make the box 40 mm wide"); + expect(JSON.stringify(turnContexts[1]?.messages)).not.toContain(SELF_CHECK_MARKER); + expect(JSON.stringify(turnContexts[2]?.messages.at(-1))).toContain(SELF_CHECK_MARKER); + }); + it("recovers from a transient postMessage failure: retries once, persists both messages in order with contiguous seq, and never persists a synthetic error message", async () => { const streamFn = makeFakeStreamFn(); const postMessage = rest.postMessage as unknown as ReturnType; diff --git a/packages/client/src/agent/session.ts b/packages/client/src/agent/session.ts index 9b2904a..18ec754 100644 --- a/packages/client/src/agent/session.ts +++ b/packages/client/src/agent/session.ts @@ -44,6 +44,13 @@ export interface ChatSession { * tied to the persisted user message. */ send(text: string, images?: File[]): Promise; + /** + * Queues a user correction into the active pi run. The promise settles after pi + * consumes and persists that exact message at the next turn boundary. + */ + steer(id: string, text: string, images?: File[]): Promise<"consumed" | "cancelled">; + cancelSteering(id: string): void; + prioritizeSteering(id: string): void; abort(): void; subscribe(listener: (state: SessionState) => void): () => void; } @@ -329,6 +336,17 @@ export function createSession(opts: CreateSessionOptions): ChatSession { let projectedVisualBatchPlan: VisualVerificationBatchPlan | undefined; const pendingThisTurn = new Set(); const imageReferenceIds = new WeakMap(); + type SteeringEntry = { + message?: AgentMessage; + status: "preparing" | "ready" | "offered" | "consuming"; + attachmentIds: string[]; + ready: Promise; + resolveReady: () => void; + resolve: (outcome: "consumed" | "cancelled") => void; + promise: Promise<"consumed" | "cancelled">; + }; + const steeringEntries = new Map(); + const steeringIds = new WeakMap(); let persistQueue: Promise = Promise.resolve(); const terminalPriorMessage = priorMessages.at(-1) as @@ -359,6 +377,8 @@ export function createSession(opts: CreateSessionOptions): ChatSession { let imagePlanRequiredThisTurn = false; let imagePlanAcceptedThisTurn = false; + let imagePlanRequiredAtSend = false; + let consumedSteeringImagePlanRequired = false; // update_plan and load_skill validate against the live transcript (latest plan + // gate evidence; already-loaded skill payloads), so they are session-owned: the @@ -544,6 +564,52 @@ export function createSession(opts: CreateSessionOptions): ChatSession { for (const listener of listeners) listener(state); } + function rebuildOfferedSteeringQueue(): void { + agent.clearSteeringQueue(); + for (const entry of steeringEntries.values()) { + if (entry.status === "offered" && entry.message) agent.steer(entry.message); + } + } + + function recomputeImagePlanRequirement(): void { + imagePlanRequiredThisTurn = imagePlanRequiredAtSend || consumedSteeringImagePlanRequired || + [...steeringEntries.values()].some((entry) => + entry.status === "consuming" && entry.attachmentIds.length > 0, + ); + } + + function removeSteeringGateState(entry: SteeringEntry): void { + for (const attachmentId of entry.attachmentIds) pendingThisTurn.delete(attachmentId); + recomputeImagePlanRequirement(); + } + + function cancelPendingSteering(): void { + agent.clearSteeringQueue(); + const pending = [...steeringEntries.values()]; + steeringEntries.clear(); + for (const entry of pending) { + removeSteeringGateState(entry); + entry.resolveReady(); + entry.resolve("cancelled"); + } + } + + async function offerReadySteeringAtTurnBoundary(): Promise { + while (true) { + const preparing = [...steeringEntries.values()] + .filter((entry) => entry.status === "preparing") + .map((entry) => entry.ready); + if (preparing.length === 0) break; + await Promise.all(preparing); + } + for (const entry of steeringEntries.values()) { + if (entry.status === "preparing") break; + if (entry.status !== "ready" || !entry.message) continue; + entry.status = "offered"; + agent.steer(entry.message); + } + } + // Persistence must never throw back into the agent loop: a rejection escaping this // listener is caught by pi's runWithLifecycle and routed into handleRunFailure, which // fabricates a synthetic error assistant message and persists that instead of (or after) @@ -661,6 +727,26 @@ export function createSession(opts: CreateSessionOptions): ChatSession { let visualNudgedWithoutProgress = false; agent.subscribe(async (event: AgentEvent) => { + if (event.type === "message_start") { + const steeringId = steeringIds.get(event.message); + const entry = steeringId ? steeringEntries.get(steeringId) : undefined; + if (entry) { + entry.status = "consuming"; + if (entry.attachmentIds.length > 0) { + for (const attachmentId of entry.attachmentIds) pendingThisTurn.add(attachmentId); + consumedSteeringImagePlanRequired = true; + imagePlanRequiredThisTurn = true; + } + } + } + if ( + event.type === "turn_end" && + event.message.role === "assistant" && + event.message.stopReason !== "error" && + event.message.stopReason !== "aborted" + ) { + await offerReadySteeringAtTurnBoundary(); + } if ( event.type === "tool_execution_end" && event.toolName === "run_build123d" && @@ -749,6 +835,13 @@ export function createSession(opts: CreateSessionOptions): ChatSession { const seq = nextSeq; nextSeq += 1; queuePersist(seq, event.message); + const steeringId = steeringIds.get(event.message); + const steeringEntry = steeringId ? steeringEntries.get(steeringId) : undefined; + if (steeringId && steeringEntry) { + await persistQueue; + steeringEntries.delete(steeringId); + steeringEntry.resolve("consumed"); + } } if ( event.type === "turn_end" && @@ -766,6 +859,12 @@ export function createSession(opts: CreateSessionOptions): ChatSession { if (event.type === "agent_start") { lastError = undefined; } + if (event.type === "agent_end") { + // A provider error or a final steering-poll race can end the run with pi + // queue entries still present. They belong to this run and must never leak + // into a later prompt; callers retain cancelled entries for explicit replay. + cancelPendingSteering(); + } if (event.type === "tool_execution_start" && event.toolName === "run_build123d") { cadRunsThisTurn += 1; planNudgedWithoutRun = false; @@ -816,6 +915,8 @@ export function createSession(opts: CreateSessionOptions): ChatSession { planNudgedWithoutRun = false; visualNudgedWithoutProgress = false; imagePlanRequiredThisTurn = Boolean(images?.length); + imagePlanRequiredAtSend = imagePlanRequiredThisTurn; + consumedSteeringImagePlanRequired = false; imagePlanAcceptedThisTurn = false; // Compaction runs between turns, before the prompt: when the LLM-visible context // is near the window, older history is summarized into a persisted compaction @@ -873,7 +974,84 @@ export function createSession(opts: CreateSessionOptions): ChatSession { notify(); } }, + steer(id: string, text: string, images?: File[]): Promise<"consumed" | "cancelled"> { + const existing = steeringEntries.get(id); + if (existing) return existing.promise; + let resolveEntry!: (outcome: "consumed" | "cancelled") => void; + let resolveReady!: () => void; + const promise = new Promise<"consumed" | "cancelled">((resolve) => { + resolveEntry = resolve; + }); + const ready = new Promise((resolve) => { + resolveReady = resolve; + }); + const entry: SteeringEntry = { + status: "preparing", + attachmentIds: [], + ready, + resolveReady, + resolve: resolveEntry, + promise, + }; + steeringEntries.set(id, entry); + void (async () => { + try { + const imageBlocks = images && images.length > 0 + ? await Promise.all(images.map(fileToImageContent)) + : []; + if (steeringEntries.get(id) !== entry || !agent.state.isStreaming) { + if (steeringEntries.delete(id)) { + entry.resolveReady(); + entry.resolve("cancelled"); + } + return; + } + entry.attachmentIds = imageBlocks.map((block) => { + const attachmentId = crypto.randomUUID(); + imageReferenceIds.set(block, attachmentId); + return attachmentId; + }); + entry.message = { + role: "user", + content: [{ type: "text", text }, ...imageBlocks], + timestamp: Date.now(), + } as AgentMessage; + steeringIds.set(entry.message, id); + entry.status = "ready"; + entry.resolveReady(); + } catch { + if (steeringEntries.delete(id)) { + entry.resolveReady(); + entry.resolve("cancelled"); + } + } + })(); + return promise; + }, + cancelSteering(id: string): void { + const entry = steeringEntries.get(id); + if (!entry || entry.status === "consuming") return; + steeringEntries.delete(id); + removeSteeringGateState(entry); + entry.resolveReady(); + entry.resolve("cancelled"); + if (entry.status === "offered") rebuildOfferedSteeringQueue(); + }, + prioritizeSteering(id: string): void { + const chosen = steeringEntries.get(id); + if (!chosen || chosen.status === "consuming") return; + steeringEntries.delete(id); + const ordered = [[id, chosen] as const, ...steeringEntries.entries()]; + steeringEntries.clear(); + for (const [entryId, entry] of ordered) steeringEntries.set(entryId, entry); + if (chosen.status === "offered") rebuildOfferedSteeringQueue(); + }, abort(): void { + // Stop invalidates both user steering and autonomous plan/visual/self-check + // continuations associated with this run. ChatState retains cancelled user + // entries in its explicit paused queue. + agent.clearAllQueues(); + cancelPendingSteering(); agent.abort(); }, subscribe(listener: (state: SessionState) => void): () => void { diff --git a/packages/client/src/components/ChatPanel.test.tsx b/packages/client/src/components/ChatPanel.test.tsx index cf0687e..fdae6cc 100644 --- a/packages/client/src/components/ChatPanel.test.tsx +++ b/packages/client/src/components/ChatPanel.test.tsx @@ -47,6 +47,9 @@ function makeFakeSession(conversationId = "conv-1") { for (const listener of listeners) listener(state); } }), + steer: vi.fn(async () => "consumed" as const), + cancelSteering: vi.fn(), + prioritizeSteering: vi.fn(), abort: vi.fn(), subscribe: vi.fn((listener: (s: SessionState) => void) => { listeners.add(listener); diff --git a/packages/client/src/components/ChatPanel.tsx b/packages/client/src/components/ChatPanel.tsx index bb42f76..61ba8f8 100644 --- a/packages/client/src/components/ChatPanel.tsx +++ b/packages/client/src/components/ChatPanel.tsx @@ -179,7 +179,7 @@ export function ChatPanel({ onOpenSettings }: ChatPanelProps) { ); } - // Streaming no longer disables the composer: sends during a turn join the queue. + // Streaming no longer disables the composer: sends during a turn steer the active run. const disabled = !settingsPresent || !session; const disabledHint = !settingsPresent ? SETTINGS_HINT : undefined; diff --git a/packages/client/src/components/ParamsPanel.test.tsx b/packages/client/src/components/ParamsPanel.test.tsx index 6073a87..caa4368 100644 --- a/packages/client/src/components/ParamsPanel.test.tsx +++ b/packages/client/src/components/ParamsPanel.test.tsx @@ -114,10 +114,10 @@ describe("ParamsPanel", () => { expect(screen.queryByTestId("param-overall_width")).toBeNull(); }); - it("shows a Python error inline and keeps the controls usable", async () => { + it("shows an artifact persistence error inline and keeps the controls usable", async () => { const onChange = vi .fn<(values: Record) => Promise>() - .mockRejectedValueOnce(new Error("NameError: name 'Bax' is not defined")) + .mockRejectedValueOnce(new Error("artifact store unavailable")) .mockResolvedValue(undefined); renderExpanded(); @@ -127,7 +127,7 @@ describe("ParamsPanel", () => { await flushDebounce(); const error = screen.getByTestId("param-error"); - expect(error.textContent).toContain("NameError"); + expect(error.textContent).toContain("artifact store unavailable"); // The controls stay usable: a follow-up commit goes through and clears // the inline error. diff --git a/packages/client/src/components/ParamsPanel.tsx b/packages/client/src/components/ParamsPanel.tsx index 08ebc2f..514760b 100644 --- a/packages/client/src/components/ParamsPanel.tsx +++ b/packages/client/src/components/ParamsPanel.tsx @@ -66,6 +66,7 @@ export function ParamsPanel({ params, onChange }: ParamsPanelProps) { valuesRef.current = next; committedRef.current = next; setValues(next); + setError(null); }, [params]); useEffect( @@ -86,7 +87,10 @@ export function ParamsPanel({ params, onChange }: ParamsPanelProps) { timerRef.current = setTimeout(() => { timerRef.current = null; const snapshot = { ...valuesRef.current }; - if (sameValues(snapshot, committedRef.current)) return; + if (sameValues(snapshot, committedRef.current)) { + setError(null); + return; + } setError(null); onChangeRef.current(snapshot).then( () => { @@ -213,13 +217,17 @@ export function ConnectedParamsPanel() { const { activeConversationId } = useChatState(); async function handleChange(values: Record): Promise { - const code = await applyParams(values); - if (activeConversationId) { - await rest.postArtifact(activeConversationId, { - pySource: code, - paramsJson: JSON.stringify(values), - }); - } + await applyParams( + values, + activeConversationId + ? async (code) => { + await rest.postArtifact(activeConversationId, { + pySource: code, + paramsJson: JSON.stringify(values), + }); + } + : undefined, + ); } return ; diff --git a/packages/client/src/state/appState.test.tsx b/packages/client/src/state/appState.test.tsx new file mode 100644 index 0000000..61de91b --- /dev/null +++ b/packages/client/src/state/appState.test.tsx @@ -0,0 +1,155 @@ +import { act, render, waitFor } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { CadRequest, CadResponse, Measurements, MeshPayload } from "@chamfer/shared"; +import { AppStateProvider, type AppState, useAppState } from "./appState"; + +class FakeWorker { + onmessage: ((event: MessageEvent) => void) | null = null; + posted: CadRequest[] = []; + + postMessage(message: CadRequest): void { + this.posted.push(message); + } + + terminate(): void {} + + emit(data: unknown): void { + this.onmessage?.({ data } as MessageEvent); + } +} + +const OLD_MEASUREMENTS: Measurements = { + bboxMm: [10, 20, 30], + volumeMm3: 6000, + areaMm2: 2200, + children: [], +}; +const NEW_MEASUREMENTS: Measurements = { + bboxMm: [20, 20, 30], + volumeMm3: 12000, + areaMm2: 3200, + children: [], +}; +const OLD_MESH: MeshPayload = { + positions: new Float32Array([1, 2, 3]), + indices: new Uint32Array([0]), +}; +const NEW_MESH: MeshPayload = { + positions: new Float32Array([4, 5, 6]), + indices: new Uint32Array([0]), +}; + +function latestRequest(worker: FakeWorker, cmd: T): Extract { + for (let index = worker.posted.length - 1; index >= 0; index -= 1) { + const candidate = worker.posted[index]; + if (candidate?.cmd === cmd) return candidate as Extract; + } + throw new Error(`No ${cmd} request was posted`); +} + +describe("AppState parameter transaction", () => { + let workers: FakeWorker[]; + let state: AppState; + + function StateProbe() { + state = useAppState(); + return null; + } + + beforeEach(() => { + workers = []; + vi.stubGlobal( + "Worker", + class { + constructor() { + const worker = new FakeWorker(); + workers.push(worker); + return worker as unknown as Worker; + } + }, + ); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + async function renderReadyState(): Promise { + render( + + + , + ); + await waitFor(() => expect(workers).toHaveLength(1)); + const worker = workers[0]!; + await act(async () => worker.emit({ phase: "ready" })); + await act(async () => { + state.publishCadResult({ mesh: OLD_MESH, measurements: OLD_MEASUREMENTS }); + state.restoreScript("width = 10"); + }); + await waitFor(() => expect(worker.posted.some((request) => request.cmd === "parseParams")).toBe(true)); + const parseRequest = latestRequest(worker, "parseParams"); + await act(async () => { + worker.emit({ id: parseRequest.id, ok: true, cmd: "parseParams", params: [] } satisfies CadResponse); + }); + return worker; + } + + async function finishCandidateRun(worker: FakeWorker): Promise { + await waitFor(() => expect(worker.posted.some((request) => request.cmd === "setParams")).toBe(true)); + const setRequest = latestRequest(worker, "setParams"); + await act(async () => { + worker.emit({ id: setRequest.id, ok: true, cmd: "setParams", code: "width = 20" } satisfies CadResponse); + }); + await waitFor(() => expect(worker.posted.some((request) => request.cmd === "run")).toBe(true)); + const runRequest = latestRequest(worker, "run"); + await act(async () => { + worker.emit({ + id: runRequest.id, + ok: true, + cmd: "run", + stdout: "", + measurements: NEW_MEASUREMENTS, + mesh: NEW_MESH, + gate: { + status: "passed", + checks: [{ name: "parameter_width", passed: true, detail: "responsive" }], + }, + } satisfies CadResponse); + }); + } + + it("keeps the last valid local and durable artifact when persistence rejects", async () => { + const worker = await renderReadyState(); + const durableArtifacts = ["width = 10"]; + const persist = vi.fn(async (_code: string) => { + throw new Error("artifact store unavailable"); + }); + + const edit = state.applyParams({ width: 20 }, persist); + const rejection = edit.then( + () => undefined, + (error: unknown) => error, + ); + await finishCandidateRun(worker); + + await expect(rejection).resolves.toEqual(new Error("artifact store unavailable")); + expect(persist).toHaveBeenCalledWith("width = 20"); + expect(durableArtifacts).toEqual(["width = 10"]); + expect(state.currentScript).toBe("width = 10"); + expect(state.measurements).toEqual(OLD_MEASUREMENTS); + expect(state.mesh).toEqual(OLD_MESH); + }); + + it("publishes a verified edit without a persistence hook", async () => { + const worker = await renderReadyState(); + + const edit = state.applyParams({ width: 20 }); + await finishCandidateRun(worker); + + await expect(edit).resolves.toBe("width = 20"); + expect(state.currentScript).toBe("width = 20"); + expect(state.measurements).toEqual(NEW_MEASUREMENTS); + expect(state.mesh).toEqual(NEW_MESH); + }); +}); diff --git a/packages/client/src/state/appState.tsx b/packages/client/src/state/appState.tsx index aad3789..2f64083 100644 --- a/packages/client/src/state/appState.tsx +++ b/packages/client/src/state/appState.tsx @@ -1,5 +1,12 @@ import { createContext, useCallback, useContext, useEffect, useRef, useState, type ReactNode } from "react"; -import type { CadBootStatus, ExportFormat, Measurements, MeshPayload, ParamSpec } from "@chamfer/shared"; +import type { + CadBootStatus, + ExportFormat, + Gate, + Measurements, + MeshPayload, + ParamSpec, +} from "@chamfer/shared"; import { CadClient } from "@/cad/cadClient"; // First script run can include installing build123d + OCP.wasm wheels into @@ -7,6 +14,12 @@ import { CadClient } from "@/cad/cadClient"; // headroom so the run does not time out mid-install. const RUN_TIMEOUT_MS = 600_000; +function parameterResponsivenessFailure(gate: Gate | undefined): string | undefined { + return gate?.checks.find( + (check) => check.name.startsWith("parameter_") && !check.passed, + )?.detail; +} + export interface AppState { bootStatus: CadBootStatus; cad: CadClient | null; @@ -30,12 +43,15 @@ export interface AppState { /** Params parsed from currentScript; empty when there is no params block. */ params: ParamSpec[]; /** - * Splices new param values into currentScript, runs the result, and (on - * success) publishes mesh/measurements and promotes the new code to - * currentScript. Returns the new code so callers can persist it. Throws on - * Python errors without touching the current state. + * Splices new param values into currentScript and verifies the result. The + * optional persistence hook runs before mesh, measurements, and code are + * published, so a durable-write failure leaves the last valid version intact. + * Without a hook, verified edits publish directly (the no-conversation flow). */ - applyParams: (values: Record) => Promise; + applyParams: ( + values: Record, + persistBeforePublish?: (code: string) => Promise, + ) => Promise; exportModel: (format: ExportFormat) => Promise<{ data: Uint8Array; filename: string }>; } @@ -118,11 +134,17 @@ export function AppStateProvider({ children }: { children: ReactNode }) { } } - async function applyParams(values: Record): Promise { + async function applyParams( + values: Record, + persistBeforePublish?: (code: string) => Promise, + ): Promise { if (!client) throw new Error("CAD worker is not initialized yet"); if (currentScript === null) throw new Error("No script has been run yet"); const newCode = await client.setParams(currentScript, values); const result = await client.run(newCode, RUN_TIMEOUT_MS); + const responsivenessFailure = parameterResponsivenessFailure(result.gate); + if (responsivenessFailure) throw new Error(responsivenessFailure); + await persistBeforePublish?.(newCode); setMesh(result.mesh); setMeasurements(result.measurements); setCurrentScript(newCode); diff --git a/packages/client/src/state/chatState.test.tsx b/packages/client/src/state/chatState.test.tsx index 80bcc79..00a71cb 100644 --- a/packages/client/src/state/chatState.test.tsx +++ b/packages/client/src/state/chatState.test.tsx @@ -25,6 +25,9 @@ function makeFakeSession(conversationId: string): ChatSession { return { conversationId, send: vi.fn(async () => {}), + steer: vi.fn(async () => "consumed" as const), + cancelSteering: vi.fn(), + prioritizeSteering: vi.fn(), abort: vi.fn(), subscribe: vi.fn((listener: (state: SessionState) => void) => { listeners.add(listener); @@ -252,6 +255,7 @@ describe("ChatProvider message queue", () => { function makeQueueSession(conversationId: string) { const listeners = new Set<(state: SessionState) => void>(); const pendingResolvers: Array<() => void> = []; + const steeringResolvers = new Map void>(); const session: ChatSession = { conversationId, send: vi.fn( @@ -260,7 +264,21 @@ describe("ChatProvider message queue", () => { pendingResolvers.push(resolve); }), ), - abort: vi.fn(), + steer: vi.fn( + (id: string) => + new Promise<"consumed" | "cancelled">((resolve) => { + steeringResolvers.set(id, resolve); + }), + ), + cancelSteering: vi.fn((id: string) => { + steeringResolvers.get(id)?.("cancelled"); + steeringResolvers.delete(id); + }), + prioritizeSteering: vi.fn(), + abort: vi.fn(() => { + for (const resolve of steeringResolvers.values()) resolve("cancelled"); + steeringResolvers.clear(); + }), subscribe: (listener: (state: SessionState) => void) => { listeners.add(listener); listener({ messages: [], streaming: false }); @@ -273,8 +291,14 @@ describe("ChatProvider message queue", () => { for (const listener of listeners) listener(state); }, finishTurn: () => { + for (const resolve of steeringResolvers.values()) resolve("cancelled"); + steeringResolvers.clear(); pendingResolvers.shift()?.(); }, + finishSteering: (id: string, outcome: "consumed" | "cancelled" = "consumed") => { + steeringResolvers.get(id)?.(outcome); + steeringResolvers.delete(id); + }, }; } @@ -328,6 +352,31 @@ describe("ChatProvider message queue", () => { expect(getLatest()?.queuedMessages.map((m) => m.text)).toEqual(["third"]); }); + it("steers a busy session and keeps the correction pending until pi consumes it", async () => { + const { fakeA, getLatest } = await renderQueueHarness(); + + act(() => getLatest()?.sendMessage("first", [])); + act(() => fakeA.emit({ messages: [], streaming: true })); + act(() => { + getLatest()?.sendMessage("make it 40 mm wide", []); + expect(fakeA.session.steer).toHaveBeenCalledTimes(1); + }); + + await waitFor(() => expect(fakeA.session.steer).toHaveBeenCalledTimes(1)); + expect(fakeA.session.send).toHaveBeenCalledTimes(1); + const pending = getLatest()?.queuedMessages[0]; + expect(pending?.text).toBe("make it 40 mm wide"); + expect(fakeA.session.steer).toHaveBeenCalledWith(pending?.id, "make it 40 mm wide", []); + + await act(async () => { + fakeA.finishSteering(pending!.id); + await Promise.resolve(); + }); + + expect(getLatest()?.queuedMessages).toHaveLength(0); + expect(fakeA.session.send).toHaveBeenCalledTimes(1); + }); + it("stopAgent aborts the session and pauses draining, keeping queued items", async () => { const { fakeA, getLatest } = await renderQueueHarness(); @@ -438,6 +487,9 @@ describe("ChatProvider auto-titling", () => { session: { conversationId, send: vi.fn(async () => {}), + steer: vi.fn(async () => "consumed" as const), + cancelSteering: vi.fn(), + prioritizeSteering: vi.fn(), abort: vi.fn(), subscribe: (listener: (state: SessionState) => void) => { listeners.add(listener); diff --git a/packages/client/src/state/chatState.tsx b/packages/client/src/state/chatState.tsx index 6c20de5..300acfc 100644 --- a/packages/client/src/state/chatState.tsx +++ b/packages/client/src/state/chatState.tsx @@ -57,12 +57,12 @@ export interface ChatContextValue { /** Re-fetches /api/settings (e.g. after saving in SettingsModal) so settings-gated UI * such as the preset prompt cards enables without a reload. */ refreshSettings: () => Promise; - /** Messages waiting for the agent to become idle, in send order. */ + /** Messages pending pi consumption or waiting for an idle recovery send, in send order. */ queuedMessages: QueuedMessage[]; /** True after stopAgent(): queued messages stay put until the user resumes * (by sending anything, or sendQueuedNow on a specific item). */ queuePaused: boolean; - /** Sends now when the agent is idle; enqueues when a turn is streaming. */ + /** Sends a new run when idle and steers the active pi run when streaming. */ sendMessage: (text: string, images: File[]) => void; /** Aborts the in-flight turn and pauses queue draining. */ stopAgent: () => void; @@ -144,6 +144,7 @@ export function ChatProvider({ children, __createSession }: ChatProviderProps) { // only when the whole agent turn is done). Guards the drain effect against firing a // second send into a turn whose streaming flag has not propagated yet. const sendInFlightRef = useRef(false); + const steeringInFlightRef = useRef(new Set()); // Bumped when a send settles, so the drain effect re-runs even though a promise // resolution alone triggers no render. const [drainTick, setDrainTick] = useState(0); @@ -211,6 +212,7 @@ export function ChatProvider({ children, __createSession }: ChatProviderProps) { // previous visit gets another attempt now that the user opened it again. if (id) titleAttemptedRef.current.delete(id); sessionRef.current = null; + steeringInFlightRef.current.clear(); setSession(null); setSessionState(EMPTY_SESSION_STATE); // The queue is conversation-scoped: messages typed for one conversation must @@ -398,6 +400,22 @@ export function ChatProvider({ children, __createSession }: ChatProviderProps) { }); }, []); + const dispatchSteering = useCallback((message: QueuedMessage) => { + const live = sessionRef.current; + if (!live || steeringInFlightRef.current.has(message.id)) return; + steeringInFlightRef.current.add(message.id); + void live.steer(message.id, message.text, message.images).then((outcome) => { + steeringInFlightRef.current.delete(message.id); + if (outcome === "consumed") { + setQueuedMessages((prev) => prev.filter((candidate) => candidate.id !== message.id)); + } + setDrainTick((tick) => tick + 1); + }).catch(() => { + steeringInFlightRef.current.delete(message.id); + setDrainTick((tick) => tick + 1); + }); + }, []); + const sendMessage = useCallback( (text: string, images: File[]) => { if (!sessionRef.current) return; @@ -405,12 +423,14 @@ export function ChatProvider({ children, __createSession }: ChatProviderProps) { // Stop-induced pause ends here. setQueuePaused(false); if (sessionState.streaming || sendInFlightRef.current) { - setQueuedMessages((prev) => [...prev, { id: crypto.randomUUID(), text, images }]); + const message = { id: crypto.randomUUID(), text, images }; + setQueuedMessages((prev) => [...prev, message]); + if (sessionState.streaming) dispatchSteering(message); return; } dispatchSend(text, images); }, - [dispatchSend, sessionState.streaming], + [dispatchSend, dispatchSteering, sessionState.streaming], ); const stopAgent = useCallback(() => { @@ -421,6 +441,8 @@ export function ChatProvider({ children, __createSession }: ChatProviderProps) { }, []); const removeQueued = useCallback((id: string) => { + sessionRef.current?.cancelSteering(id); + steeringInFlightRef.current.delete(id); setQueuedMessages((prev) => prev.filter((message) => message.id !== id)); }, []); @@ -432,9 +454,17 @@ export function ChatProvider({ children, __createSession }: ChatProviderProps) { if (!chosen) return prev; return [chosen, ...prev.filter((message) => message.id !== id)]; }); + sessionRef.current?.prioritizeSteering(id); setQueuePaused(false); }, []); + // Busy-session delivery uses pi steering. Messages remain in queuedMessages so + // pending controls stay visible until the session reports consumption. + useEffect(() => { + if (!sessionState.streaming || queuePaused || sessionState.error) return; + for (const message of queuedMessages) dispatchSteering(message); + }, [dispatchSteering, queuePaused, queuedMessages, sessionState.error, sessionState.streaming]); + // Queue drain: whenever the agent is idle, nothing is paused or errored, and a // message is waiting, send exactly one. Each drained turn re-triggers this effect // via drainTick when it settles, delivering the rest FIFO. @@ -444,6 +474,7 @@ export function ChatProvider({ children, __createSession }: ChatProviderProps) { if (queuePaused || sessionState.error) return; const next = queuedMessages[0]; if (!next || !sessionRef.current) return; + if (steeringInFlightRef.current.has(next.id)) return; setQueuedMessages((prev) => prev.slice(1)); dispatchSend(next.text, next.images); }, [dispatchSend, drainTick, queuePaused, queuedMessages, sessionState]); diff --git a/packages/server/src/app.ts b/packages/server/src/app.ts index 0e3a97c..5e412ad 100644 --- a/packages/server/src/app.ts +++ b/packages/server/src/app.ts @@ -14,6 +14,7 @@ import { AttachmentStore, type AttachmentStoreOptions } from "./attachmentStore" import { inspectionLeaseRoutes } from "./routes/inspectionLeases"; import { visualVerificationRoutes } from "./routes/visualVerifications"; import { imageDiagnosticsRoutes } from "./routes/imageDiagnostics"; +import { fakeLlmTestControlRoutes } from "./routes/fakeLlmTestControls"; export interface AppOptions extends AttachmentStoreOptions { dataDir?: string; @@ -31,7 +32,8 @@ export function createApp(db: DatabaseSync, llm: LlmStreamer = realLlm(), option app.route("/", referenceRoutes(db, attachmentStore)); app.route("/", inspectionLeaseRoutes(db, attachmentStore)); app.route("/", visualVerificationRoutes(db)); - app.route("/", imageDiagnosticsRoutes(db, llm, attachmentStore)); + app.route("/", imageDiagnosticsRoutes(db, attachmentStore)); + app.route("/", fakeLlmTestControlRoutes(db, llm)); app.route("/", artifactsRoutes(db)); return app; } diff --git a/packages/server/src/fakeLlm.ts b/packages/server/src/fakeLlm.ts index dc3443a..2e40d7c 100644 --- a/packages/server/src/fakeLlm.ts +++ b/packages/server/src/fakeLlm.ts @@ -3,8 +3,10 @@ import type { LlmStreamer } from "./llm"; import { TITLE_SYSTEM_PROMPT } from "./titles"; import { sanitizeModelRequest, type ModelRequestDiagnostic } from "./imageContextDiagnostics"; -export interface FakeLlmRequestDiagnostics extends LlmStreamer { +export interface FakeLlmTestController extends LlmStreamer { getRequestDiagnostics(conversationId: string): ModelRequestDiagnostic[]; + isRequestHeld(conversationId: string): boolean; + releaseHeldRequest(conversationId: string): boolean; } export const FAKE_MODEL = { @@ -754,12 +756,23 @@ function* retrievableEvidenceWorkflowStep( yield* streamText("CAD revision rendered; attempting to finish before visual verification."); } -export function fakeLlm(): FakeLlmRequestDiagnostics { +export function fakeLlm(): FakeLlmTestController { const diagnostics = new Map(); + const heldRequests = new Map void>(); return { getRequestDiagnostics(conversationId) { return [...(diagnostics.get(conversationId) ?? [])]; }, + isRequestHeld(conversationId) { + return heldRequests.has(conversationId); + }, + releaseHeldRequest(conversationId) { + const release = heldRequests.get(conversationId); + if (!release) return false; + heldRequests.delete(conversationId); + release(); + return true; + }, async *stream(_model, context, options): AsyncIterable { const { messages = [], systemPrompt } = context as { messages?: Array<{ role?: string }>; @@ -784,6 +797,20 @@ export function fakeLlm(): FakeLlmRequestDiagnostics { if (!Array.isArray(content)) return count; return count + content.filter((block) => (block as { type?: unknown })?.type === "image").length; }, 0); + if (transcript.includes("follow-up-steering-hold")) { + const lastMessage = JSON.stringify(messages.at(-1)); + if (lastMessage.includes("change the width to 40 mm")) { + yield* streamText("Correction consumed by the active run before it completed."); + return; + } + // The browser test releases this request only after it has observed the + // correction in the pending UI, making the active-run boundary deterministic. + await new Promise((resolve) => { + heldRequests.set(conversationId, resolve); + }); + yield* streamText("Initial response finished; checking for steering."); + return; + } if (transcript.includes("attachment-replay")) { yield* streamText(`Received ${imageCount} native image block${imageCount === 1 ? "" : "s"}.`); return; diff --git a/packages/server/src/routes/fakeLlmTestControls.test.ts b/packages/server/src/routes/fakeLlmTestControls.test.ts new file mode 100644 index 0000000..87c7a14 --- /dev/null +++ b/packages/server/src/routes/fakeLlmTestControls.test.ts @@ -0,0 +1,35 @@ +import { Hono } from "hono"; +import { describe, expect, it, vi } from "vitest"; +import { openDb } from "../db"; +import { createConversation } from "../conversationStore"; +import { fakeLlm } from "../fakeLlm"; +import { fakeLlmTestControlRoutes } from "./fakeLlmTestControls"; + +describe("fake LLM test controls", () => { + it("holds a scripted request until its conversation is explicitly released", async () => { + const db = openDb(":memory:"); + const conversation = createConversation(db, "held request"); + const llm = fakeLlm(); + const app = new Hono().route("/", fakeLlmTestControlRoutes(db, llm)); + const drain = (async () => { + for await (const _event of llm.stream({}, { + messages: [{ role: "user", content: [{ type: "text", text: "follow-up-steering-hold" }] }], + }, { sessionId: conversation.id })) { + // The request must remain suspended until the release route resolves its latch. + } + })(); + + await vi.waitUntil(() => llm.isRequestHeld(conversation.id)); + const held = await app.request(`/api/test/fake-model-holds?conversationId=${conversation.id}`); + expect(await held.json()).toEqual({ held: true }); + + const released = await app.request(`/api/test/fake-model-holds/release?conversationId=${conversation.id}`, { + method: "POST", + }); + expect(await released.json()).toEqual({ released: true }); + await drain; + + const requests = await app.request(`/api/test/fake-model-requests?conversationId=${conversation.id}`); + expect(await requests.json()).toMatchObject({ requests: [{ sequence: 1, messageCount: 1 }] }); + }); +}); diff --git a/packages/server/src/routes/fakeLlmTestControls.ts b/packages/server/src/routes/fakeLlmTestControls.ts new file mode 100644 index 0000000..0c38548 --- /dev/null +++ b/packages/server/src/routes/fakeLlmTestControls.ts @@ -0,0 +1,35 @@ +import { Hono } from "hono"; +import type { DatabaseSync } from "node:sqlite"; +import type { LlmStreamer } from "../llm"; +import type { FakeLlmTestController } from "../fakeLlm"; +import { conversationExists } from "../conversationStore"; +import { summarizeImageExposure } from "../imageContextDiagnostics"; + +function hasFakeLlmTestControls(llm: LlmStreamer): llm is FakeLlmTestController { + return "getRequestDiagnostics" in llm && typeof llm.getRequestDiagnostics === "function" && + "isRequestHeld" in llm && typeof llm.isRequestHeld === "function" && + "releaseHeldRequest" in llm && typeof llm.releaseHeldRequest === "function"; +} + +export function fakeLlmTestControlRoutes(db: DatabaseSync, llm: LlmStreamer): Hono { + const app = new Hono(); + if (!hasFakeLlmTestControls(llm)) return app; + + app.get("/api/test/fake-model-requests", (c) => { + const conversationId = c.req.query("conversationId") ?? ""; + if (!conversationExists(db, conversationId)) return c.json({ error: "not found" }, 404); + const requests = llm.getRequestDiagnostics(conversationId); + return c.json({ requests, exposure: summarizeImageExposure(requests) }); + }); + app.get("/api/test/fake-model-holds", (c) => { + const conversationId = c.req.query("conversationId") ?? ""; + if (!conversationExists(db, conversationId)) return c.json({ error: "not found" }, 404); + return c.json({ held: llm.isRequestHeld(conversationId) }); + }); + app.post("/api/test/fake-model-holds/release", (c) => { + const conversationId = c.req.query("conversationId") ?? ""; + if (!conversationExists(db, conversationId)) return c.json({ error: "not found" }, 404); + return c.json({ released: llm.releaseHeldRequest(conversationId) }); + }); + return app; +} diff --git a/packages/server/src/routes/imageDiagnostics.ts b/packages/server/src/routes/imageDiagnostics.ts index 3815dc4..63dd313 100644 --- a/packages/server/src/routes/imageDiagnostics.ts +++ b/packages/server/src/routes/imageDiagnostics.ts @@ -1,32 +1,15 @@ import { Hono } from "hono"; import type { DatabaseSync } from "node:sqlite"; -import type { LlmStreamer } from "../llm"; -import type { FakeLlmRequestDiagnostics } from "../fakeLlm"; -import { - verifyConversationImageDiagnostics, - summarizeImageExposure, -} from "../imageContextDiagnostics"; +import { verifyConversationImageDiagnostics } from "../imageContextDiagnostics"; import { conversationExists } from "../conversationStore"; import type { AttachmentStore } from "../attachmentStore"; -function hasRequestDiagnostics(llm: LlmStreamer): llm is FakeLlmRequestDiagnostics { - return "getRequestDiagnostics" in llm && typeof llm.getRequestDiagnostics === "function"; -} - -export function imageDiagnosticsRoutes(db: DatabaseSync, llm: LlmStreamer, store: AttachmentStore): Hono { +export function imageDiagnosticsRoutes(db: DatabaseSync, store: AttachmentStore): Hono { const app = new Hono(); app.get("/api/conversations/:id/image-diagnostics", async (c) => { const conversationId = c.req.param("id"); if (!conversationExists(db, conversationId)) return c.json({ error: "not found" }, 404); return c.json(await verifyConversationImageDiagnostics(db, conversationId, store)); }); - if (hasRequestDiagnostics(llm)) { - app.get("/api/test/fake-model-requests", (c) => { - const conversationId = c.req.query("conversationId") ?? ""; - if (!conversationExists(db, conversationId)) return c.json({ error: "not found" }, 404); - const requests = llm.getRequestDiagnostics(conversationId); - return c.json({ requests, exposure: summarizeImageExposure(requests) }); - }); - } return app; } diff --git a/py-tests/test_params.py b/py-tests/test_params.py index e22015b..ebaa72a 100644 --- a/py-tests/test_params.py +++ b/py-tests/test_params.py @@ -54,3 +54,92 @@ def test_set_params_roundtrip_executes(): out = harness.set_params(SRC, {"overall_width": 120}) m = harness.run_script(out)["measurements"] assert abs(m["bboxMm"][0] - 120) < 1e-6 + + +def test_run_script_rejects_a_visible_param_that_does_not_change_geometry(): + source = """# --- params --- +width = 10 # [5, 100] Overall width in mm +# --- end params --- +from build123d import * +result = Box(10, 20, 30) +""" + + out = harness.run_script(source) + + failed = [check for check in out["gate"]["checks"] if not check["passed"]] + assert any( + check["name"] == "parameter_width" + and "does not change the executed geometry" in check["detail"] + and "Use `width`" in check["detail"] + for check in failed + ) + + +def parameter_check(source, name): + checks = harness.run_script(source)["gate"]["checks"] + return next(check for check in checks if check["name"] == f"parameter_{name}") + + +def test_run_script_accepts_a_parameter_that_changes_executed_geometry(): + check = parameter_check(SRC, "overall_width") + + assert check["passed"] is True + assert "changes the executed geometry" in check["detail"] + + +def test_run_script_rejects_a_param_without_an_adjustable_range(): + source = """# --- params --- +width = 10 # [10, 10] Overall width in mm +# --- end params --- +from build123d import * +result = Box(width, 20, 30) +""" + + check = parameter_check(source, "width") + + assert check["passed"] is False + assert "valid adjustable range" in check["detail"] + + +def test_run_script_probes_a_one_step_integer_range(): + source = """# --- params --- +count = 1 # [1, 2] Number of adjacent boxes +# --- end params --- +from build123d import * +result = Box(count * 10, 20, 30) +""" + + check = parameter_check(source, "count") + + assert check["passed"] is True + assert "probe value 2" in check["detail"] + + +def test_run_script_tries_another_probe_when_one_in_range_value_errors(): + source = """# --- params --- +width = 10 # [5, 15] Overall width in mm +# --- end params --- +from build123d import * +if width <= 8: + raise ValueError("width is too small for this feature") +result = Box(width, 20, 30) +""" + + check = parameter_check(source, "width") + + assert check["passed"] is True + assert "probe value 15" in check["detail"] + + +def test_sub_tolerance_numeric_noise_does_not_make_a_parameter_responsive(): + source = """# --- params --- +jitter = 0 # [0, 1] Numerical jitter +# --- end params --- +from build123d import * +result = Box(10 + jitter * 1e-10, 20, 30) +""" + + check = parameter_check(source, "jitter") + + assert check["passed"] is False + assert "does not change the executed geometry" in check["detail"]