diff --git a/clients/web/src/components/groups/AppDetailPanel/AppDetailPanel.test.tsx b/clients/web/src/components/groups/AppDetailPanel/AppDetailPanel.test.tsx index 0c40d0ae..effbd5a2 100644 --- a/clients/web/src/components/groups/AppDetailPanel/AppDetailPanel.test.tsx +++ b/clients/web/src/components/groups/AppDetailPanel/AppDetailPanel.test.tsx @@ -149,4 +149,28 @@ describe("AppDetailPanel", () => { await user.click(screen.getByRole("button", { name: /open app/i })); expect(onOpenApp).toHaveBeenCalledTimes(1); }); + + it("drops a previous app's in-progress number text when the app changes", async () => { + const user = userEvent.setup(); + // AppsScreen swaps selectedAppName + formValues in place rather than + // remounting this panel, so two apps exposing a same-named number field + // share the field component. Both values here are undefined, which the + // draft/value re-sync cannot distinguish — only the resetKey identity can. + const numberFieldTool = (name: string): Tool => ({ + name, + title: name, + inputSchema: { + type: "object", + properties: { scale: { type: "number", title: "Scale" } }, + }, + }); + const { rerender } = renderWithMantine( + , + ); + const input = () => screen.getByLabelText(/Scale/) as HTMLInputElement; + await user.type(input(), "-"); + expect(input().value).toBe("-"); + rerender(); + expect(input().value).toBe(""); + }); }); diff --git a/clients/web/src/components/groups/AppDetailPanel/AppDetailPanel.tsx b/clients/web/src/components/groups/AppDetailPanel/AppDetailPanel.tsx index 51bf14c6..8d5a2135 100644 --- a/clients/web/src/components/groups/AppDetailPanel/AppDetailPanel.tsx +++ b/clients/web/src/components/groups/AppDetailPanel/AppDetailPanel.tsx @@ -76,6 +76,12 @@ export function AppDetailPanel({ values={formValues} onChange={onFormChange} disabled={isOpening} + // Like ToolDetailPanel, this panel is reused across app selections + // rather than remounted (AppsScreen.handleSelect swaps + // selectedAppName + formValues in place), so the form needs the app + // tool's name to drop another app's in-progress field text. See + // SchemaFormProps.resetKey. + resetKey={tool.name} /> { expect(onChange).toHaveBeenLastCalledWith({ count: 15 }); }); + it("reports no value for a magnitude JS cannot hold exactly", async () => { + const user = userEvent.setup(); + const onChange = vi.fn(); + renderWithMantine( + , + ); + const input = screen.getByLabelText(/Divisor/) as HTMLInputElement; + // Past Number.MAX_SAFE_INTEGER, Mantine stops emitting a number and hands + // back the raw string to avoid destroying precision. Number() would round + // this to ...904, so parsing it would send a value the user never typed. + await user.type(input, "90071992547409910"); + expect(input.value).toBe("90071992547409910"); + expect(onChange).toHaveBeenLastCalledWith({ divisor: undefined }); + }); + + it("still parses a long decimal, which stays exactly representable", async () => { + const user = userEvent.setup(); + const onChange = vi.fn(); + renderWithMantine( + , + ); + const input = screen.getByLabelText(/Divisor/) as HTMLInputElement; + // Guards the safe-integer check against over-rejecting: the fractional + // digits are not what overflows, so this must not be dropped. + await user.type(input, "3.14159265358979"); + expect(onChange).toHaveBeenLastCalledWith({ divisor: 3.14159265358979 }); + }); + it("drops in-progress text when resetKey moves the form to another entity", async () => { const user = userEvent.setup(); // Both "tools" expose the same-named number field with no default, so the diff --git a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx index efcb32e3..232b5327 100644 --- a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx +++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx @@ -53,10 +53,26 @@ function toEnumData( /** * Interpret whatever Mantine's `NumberInput` reported as the JSON value for the - * field. It emits a `number` once the text parses cleanly, and the **raw string** - * while it does not — `""` when cleared, but also the in-progress `"1."`, `"-"`, - * and `"1e"`. Anything that is not a finite number becomes `undefined`, which is - * how an absent optional argument is represented everywhere else in this form. + * field. Anything that is not a finite number becomes `undefined`, which is how + * an absent optional argument is represented everywhere else in this form. + * + * `NumberInput` emits a `number` only when the text both parses *and* is exactly + * representable; otherwise it hands back the **raw string** (see its + * `isValidNumber` guard). Two quite different situations produce a string, and + * they are treated differently here: + * + * 1. **Mid-entry text** — `""` when cleared, plus `"1."`, `"1.50"`, and a lone + * `"-"`. These are parsed: `"1."` really does mean `1`. (Note that an + * exponent is *not* in this set — `NumberInput` masks input through + * `NumericFormat`, which rejects `e` outright, so `"1e"` can never be typed.) + * 2. **Values JS cannot hold exactly** — anything at or beyond + * `Number.MAX_SAFE_INTEGER`. `Number("90071992547409910")` silently yields + * `90071992547409904`, so parsing here would send the server a number the + * user never entered. An inspector must not misreport what it transmits, so + * these report no value instead — which is also what this field did with such + * input before #1888, making it no regression. Preserving them properly needs + * an exact-serialization path down the whole `tools/call` chain, which is a + * separate concern from being able to type a decimal. */ function toNumericValue(raw: string | number): number | undefined { if (typeof raw === "number") { @@ -66,7 +82,12 @@ function toNumericValue(raw: string | number): number | undefined { return undefined; } const parsed = Number(raw); - return Number.isFinite(parsed) ? parsed : undefined; + if (!Number.isFinite(parsed)) { + return undefined; + } + // Case 2 above. The integer part is what overflows exact representation; the + // fractional digits are bounded by the same guard and stay lossless. + return Number.isSafeInteger(Math.trunc(parsed)) ? parsed : undefined; } interface SchemaNumberInputProps {