From 8aa6f0150516198d0255a1175b354f87656b133a Mon Sep 17 00:00:00 2001 From: cliffhall Date: Thu, 27 Aug 2026 06:11:23 -0400 Subject: [PATCH] fix(web): reject a negative-zero literal, which loses its sign when serialized MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot's twelfth pass. The suppressed comment is right and is fixed here; the inline one is real too, but needs a core API change and is filed as #2171. `-0` is represented exactly, so the digit comparison had nothing to say about it — but `JSON.stringify` writes it as `0`, so the loss happens on the way *out* rather than on the way in. The editor showed `-0` and the wire carried `0`, and the viewer rewrote a received `-0` while pretty-printing. `hasImpreciseIntegerLiteral` becomes `findUnsendableNumberLiteral`, checking `Object.is(Number(literal), -0)` for every numeric spelling — `-0`, `-0.0`, `-0e1` — before the integer-only digit check. Returning the offending literal rather than a boolean lets the message name it, which is more use than either of the two rules behind it and matches how the duplicate-key message reads. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UL9retfvAXRgvi6EWk4SY4 Signed-off-by: cliffhall --- .../ContentViewer/contentViewerUtils.ts | 4 +- .../EditReplayModal/EditReplayModal.test.tsx | 4 +- .../EditReplayModal/EditReplayModal.tsx | 9 +-- .../groups/SchemaForm/SchemaForm.test.tsx | 6 +- .../groups/SchemaForm/SchemaForm.tsx | 14 +++-- clients/web/src/utils/jsonObjectDraft.test.ts | 62 ++++++++++++------- clients/web/src/utils/jsonObjectDraft.ts | 44 ++++++++----- 7 files changed, 85 insertions(+), 58 deletions(-) diff --git a/clients/web/src/components/elements/ContentViewer/contentViewerUtils.ts b/clients/web/src/components/elements/ContentViewer/contentViewerUtils.ts index 92c5a44a..670fa3b7 100644 --- a/clients/web/src/components/elements/ContentViewer/contentViewerUtils.ts +++ b/clients/web/src/components/elements/ContentViewer/contentViewerUtils.ts @@ -9,7 +9,7 @@ import { isSerializableJson } from "@inspector/core/json/jsonUtils.js"; import { findDuplicateObjectKey, - hasImpreciseIntegerLiteral, + findUnsendableNumberLiteral, } from "../../../utils/jsonObjectDraft"; /** @@ -169,7 +169,7 @@ export function formatJson(content: string): string { function reformatsFaithfully(text: string, parsed: unknown): boolean { return ( isSerializableJson(parsed) && - !hasImpreciseIntegerLiteral(text) && + findUnsendableNumberLiteral(text) === null && // A duplicate member name is the same class and the starkest instance: // `{"role":"user","role":"admin"}` would be *shown* as only the second, // which is the viewer hiding what the server sent. diff --git a/clients/web/src/components/groups/EditReplayModal/EditReplayModal.test.tsx b/clients/web/src/components/groups/EditReplayModal/EditReplayModal.test.tsx index c041cbae..e2d16826 100644 --- a/clients/web/src/components/groups/EditReplayModal/EditReplayModal.test.tsx +++ b/clients/web/src/components/groups/EditReplayModal/EditReplayModal.test.tsx @@ -218,9 +218,7 @@ describe("EditReplayModal", () => { renderWithMantine(); await setAceText('{"name":"echo","arguments":{"id":9007199254740993}}'); expect(screen.getByRole("button", { name: "Send" })).toBeDisabled(); - expect( - screen.getByText(/cannot be represented exactly/), - ).toBeInTheDocument(); + expect(screen.getByText(/would not arrive as written/)).toBeInTheDocument(); }); it("disables Send for a non-string prompt argument", async () => { diff --git a/clients/web/src/components/groups/EditReplayModal/EditReplayModal.tsx b/clients/web/src/components/groups/EditReplayModal/EditReplayModal.tsx index 17069c38..bcc12cc4 100644 --- a/clients/web/src/components/groups/EditReplayModal/EditReplayModal.tsx +++ b/clients/web/src/components/groups/EditReplayModal/EditReplayModal.tsx @@ -5,9 +5,9 @@ import { JsonEditor } from "../../elements/JsonEditor/JsonEditor"; import { duplicateKeyError, findDuplicateObjectKey, - hasImpreciseIntegerLiteral, + findUnsendableNumberLiteral, isJsonObject, - IMPRECISE_INTEGER_ERROR, + unsendableNumberError, } from "../../../utils/jsonObjectDraft"; import { replayableParams, @@ -113,8 +113,9 @@ function parseParamsDraft( "Numbers must be finite — a value like `1e400` overflows and would be sent as null", }; } - if (hasImpreciseIntegerLiteral(text)) { - return { ok: false, error: IMPRECISE_INTEGER_ERROR }; + const unsendable = findUnsendableNumberLiteral(text); + if (unsendable !== null) { + return { ok: false, error: unsendableNumberError(unsendable) }; } const duplicate = findDuplicateObjectKey(text); if (duplicate !== null) { diff --git a/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx index a2c5e091..fde96a82 100644 --- a/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx +++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx @@ -789,7 +789,7 @@ describe("SchemaForm", () => { await setAceTextByLabel(/Config/, "[9007199254740993]"); expect( - screen.getByText(/^This whole number cannot be represented exactly/), + screen.getByText(/^`9007199254740993` would not arrive as written/), ).toBeInTheDocument(); await setAceTextByLabel(/Config/, '[{"a":1,"a":2}]'); @@ -1976,9 +1976,7 @@ describe("SchemaForm raw JSON (#2151)", () => { await setAceTextByLabel(/Arguments JSON/, '{"id":9007199254740993}'); expect(onValidityChange).toHaveBeenLastCalledWith(true); - expect( - screen.getByText(/cannot be represented exactly/), - ).toBeInTheDocument(); + expect(screen.getByText(/would not arrive as written/)).toBeInTheDocument(); }); // `JSON.parse` keeps only the last occurrence, so the editor would show a diff --git a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx index 748ce554..4ea9fb95 100644 --- a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx +++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx @@ -27,9 +27,9 @@ import { JsonEditor } from "../../elements/JsonEditor/JsonEditor"; import { duplicateKeyError, findDuplicateObjectKey, - hasImpreciseIntegerLiteral, + findUnsendableNumberLiteral, isJsonObject, - IMPRECISE_INTEGER_ERROR, + unsendableNumberError, } from "../../../utils/jsonObjectDraft"; import { isSerializableJson } from "@inspector/core/json/jsonUtils.js"; import { useValueChange } from "../../../hooks/useValueChange"; @@ -331,8 +331,9 @@ function describeJsonDraftProblem(text: string): string | null { if (!isSerializableJson(parsed)) { return "Numbers must be finite — this field will be omitted"; } - if (hasImpreciseIntegerLiteral(text)) { - return `${IMPRECISE_INTEGER_ERROR} — this field will be omitted`; + const unsendable = findUnsendableNumberLiteral(text); + if (unsendable !== null) { + return `${unsendableNumberError(unsendable)} — this field will be omitted`; } const duplicate = findDuplicateObjectKey(text); if (duplicate !== null) { @@ -652,8 +653,9 @@ function parseRawArgumentsDraft( } // The quieter half of the same defect: a whole number past 2^53−1 parses to // the nearest double, so the draft shows digits the wire will not carry. - if (hasImpreciseIntegerLiteral(text)) { - return { ok: false, error: IMPRECISE_INTEGER_ERROR }; + const unsendable = findUnsendableNumberLiteral(text); + if (unsendable !== null) { + return { ok: false, error: unsendableNumberError(unsendable) }; } const duplicate = findDuplicateObjectKey(text); if (duplicate !== null) { diff --git a/clients/web/src/utils/jsonObjectDraft.test.ts b/clients/web/src/utils/jsonObjectDraft.test.ts index 9c3b8a0c..d9fb4462 100644 --- a/clients/web/src/utils/jsonObjectDraft.test.ts +++ b/clients/web/src/utils/jsonObjectDraft.test.ts @@ -1,7 +1,7 @@ import { describe, it, expect } from "vitest"; import { findDuplicateObjectKey, - hasImpreciseIntegerLiteral, + findUnsendableNumberLiteral, parseJsonObjectDraft, } from "./jsonObjectDraft"; @@ -9,65 +9,83 @@ import { // `Infinity` → `null` on the wire). This is the quieter one: a whole number // past 2^53−1 parses to the nearest double, so the draft shows digits the wire // will not carry. -describe("hasImpreciseIntegerLiteral", () => { +describe("findUnsendableNumberLiteral", () => { it("accepts whole numbers inside the safe range", () => { - expect(hasImpreciseIntegerLiteral('{"id":9007199254740991}')).toBe(false); - expect(hasImpreciseIntegerLiteral('{"id":-9007199254740991}')).toBe(false); - expect(hasImpreciseIntegerLiteral('{"id":0}')).toBe(false); + expect(findUnsendableNumberLiteral('{"id":9007199254740991}')).toBeNull(); + expect(findUnsendableNumberLiteral('{"id":-9007199254740991}')).toBeNull(); + expect(findUnsendableNumberLiteral('{"id":0}')).toBeNull(); }); it("rejects a whole number that loses digits when parsed", () => { // Typed as …93, parsed as …92. - expect(hasImpreciseIntegerLiteral('{"id":9007199254740993}')).toBe(true); + expect(findUnsendableNumberLiteral('{"id":9007199254740993}')).toBe( + "9007199254740993", + ); }); // The case the previous, value-based check could not see: at or above 1e21 // JS stringifies in exponent form, so inferring the literal from the parsed // number said "exponent form, nothing lost" about digits that were lost. it("rejects a full-form literal at or above 1e21", () => { - expect(hasImpreciseIntegerLiteral('{"id":1000000000000000000001}')).toBe( - true, + expect(findUnsendableNumberLiteral('{"id":1000000000000000000001}')).toBe( + "1000000000000000000001", ); }); // …and the false positive it also had. Written out in full, past the safe // range, and exactly representable, so it is sent as written. it("accepts a full-form literal that is exactly representable", () => { - expect(hasImpreciseIntegerLiteral('{"id":18014398509481984}')).toBe(false); + expect(findUnsendableNumberLiteral('{"id":18014398509481984}')).toBeNull(); }); // A fractional or exponent-form value is a double by nature and is sent as // the double it parsed to, so nothing is lost between typing and sending. it("accepts fractional and exponent-form values", () => { - expect(hasImpreciseIntegerLiteral('{"n":1.5}')).toBe(false); - expect(hasImpreciseIntegerLiteral('{"n":1e308}')).toBe(false); - expect(hasImpreciseIntegerLiteral('{"n":1e21}')).toBe(false); - expect(hasImpreciseIntegerLiteral('{"n":-2.5e-7}')).toBe(false); + expect(findUnsendableNumberLiteral('{"n":1.5}')).toBeNull(); + expect(findUnsendableNumberLiteral('{"n":1e308}')).toBeNull(); + expect(findUnsendableNumberLiteral('{"n":1e21}')).toBeNull(); + expect(findUnsendableNumberLiteral('{"n":-2.5e-7}')).toBeNull(); }); // The reason this scans rather than running a regex over the text: those // digits are a *string*, sent back exactly as written. it("ignores digits inside a string", () => { - expect(hasImpreciseIntegerLiteral('{"id":"9007199254740993"}')).toBe(false); + expect(findUnsendableNumberLiteral('{"id":"9007199254740993"}')).toBeNull(); }); it("ignores digits inside a key, and an escaped quote inside a string", () => { - expect(hasImpreciseIntegerLiteral('{"9007199254740993":1}')).toBe(false); + expect(findUnsendableNumberLiteral('{"9007199254740993":1}')).toBeNull(); expect( - hasImpreciseIntegerLiteral('{"a":"say \\"9007199254740993\\"","b":1}'), - ).toBe(false); + findUnsendableNumberLiteral('{"a":"say \\"9007199254740993\\"","b":1}'), + ).toBeNull(); }); it("looks through arrays and nesting", () => { - expect(hasImpreciseIntegerLiteral('[{"a":[9007199254740993]}]')).toBe(true); - expect(hasImpreciseIntegerLiteral('{"a":{"b":[1,2,"x",null,true]}}')).toBe( - false, + expect(findUnsendableNumberLiteral('[{"a":[9007199254740993]}]')).toBe( + "9007199254740993", ); + expect( + findUnsendableNumberLiteral('{"a":{"b":[1,2,"x",null,true]}}'), + ).toBeNull(); + }); + + // Represented exactly, so the digit comparison has nothing to say — but + // `JSON.stringify` writes it as `0`, losing the sign on the way *out*. + it.each(["-0", "-0.0", "-0e1", "-0.0e-5"])( + "rejects negative zero written as %s", + (literal) => { + expect(findUnsendableNumberLiteral(`{"n":${literal}}`)).toBe(literal); + }, + ); + + it("accepts a positive zero", () => { + expect(findUnsendableNumberLiteral('{"n":0}')).toBeNull(); + expect(findUnsendableNumberLiteral('{"n":0.0}')).toBeNull(); }); it("says nothing about text that is not JSON", () => { - expect(hasImpreciseIntegerLiteral("")).toBe(false); - expect(hasImpreciseIntegerLiteral("{")).toBe(false); + expect(findUnsendableNumberLiteral("")).toBeNull(); + expect(findUnsendableNumberLiteral("{")).toBeNull(); }); }); diff --git a/clients/web/src/utils/jsonObjectDraft.ts b/clients/web/src/utils/jsonObjectDraft.ts index 98b049ea..e3c722ed 100644 --- a/clients/web/src/utils/jsonObjectDraft.ts +++ b/clients/web/src/utils/jsonObjectDraft.ts @@ -21,8 +21,8 @@ export function isJsonObject(value: unknown): value is StrictJsonObject { } /** - * Whether the draft text writes a whole number out in full that `JSON.parse` - * cannot represent exactly. + * The first numeric literal in the draft that would not arrive as written, or + * `null` when every number survives the trip. * * `isSerializableJson` catches a literal too large to represent at all — * `1e400` parses to `Infinity` and writes back as `null`. This catches the @@ -38,19 +38,25 @@ export function isJsonObject(value: unknown): value is StrictJsonObject { * refused `2^54` (written in full and exactly representable) and accepted every * full-form literal at or above 1e21, where JS switches to exponent form. * - * Only integer literals written out in full are checked. An exponent or a - * fraction is a double by nature and is sent as the double it parsed to, so - * nothing is lost between typing and sending — and `{"n":1e308}` stays - * acceptable, which `parseJsonObjectDraft`'s own tests pin. + * Only integer literals written out in full are checked for lost digits. An + * exponent or a fraction is a double by nature and is sent as the double it + * parsed to, so nothing is lost between typing and sending — and `{"n":1e308}` + * stays acceptable, which `parseJsonObjectDraft`'s own tests pin. Negative zero + * is the exception, and is checked in every spelling: there the loss happens on + * the way *out*, not on the way in. */ -export function hasImpreciseIntegerLiteral(text: string): boolean { +export function findUnsendableNumberLiteral(text: string): string | null { for (const literal of jsonNumberLiterals(text)) { + // Negative zero, in any spelling — `-0`, `-0.0`, `-0e1`. It is represented + // exactly, so the digit comparison below has nothing to say about it, but + // `JSON.stringify` writes it as `0`: the sign is lost on the way out. + if (Object.is(Number(literal), -0)) return literal; if (!/^-?\d+$/.test(literal)) continue; // BigInt on both sides, so the comparison is exact: `Number(literal)` is an // integer-valued double, and converting it back shows what survived. - if (BigInt(literal) !== BigInt(Number(literal))) return true; + if (BigInt(literal) !== BigInt(Number(literal))) return literal; } - return false; + return null; } /** Matches one JSON number token, anchored where the scan currently stands. */ @@ -179,14 +185,17 @@ export function duplicateKeyError(key: string): string { } /** - * The message every draft parser uses for {@link hasImpreciseIntegerLiteral}. + * The message every draft parser uses for + * {@link findUnsendableNumberLiteral}. * - * Phrased as "cannot be represented exactly" rather than naming the safe-integer - * range, because the range is not the rule: a whole number past ±(2^53 − 1) that - * *is* exactly representable — 2^54, say — round-trips and is accepted. + * Names the literal rather than the rule, because the rule is two rules: a + * whole number can lose digits on the way in, and a negative zero loses its + * sign on the way out. Naming what is wrong with *this* document is more use + * than either explanation, and it is how the duplicate-key message reads too. */ -export const IMPRECISE_INTEGER_ERROR = - "This whole number cannot be represented exactly — it is rounded when parsed, so a different value would be sent"; +export function unsendableNumberError(literal: string): string { + return `\`${literal}\` would not arrive as written — it changes when serialized, so a different value would be sent`; +} /** * Read a JSON-object editor's draft text. @@ -228,8 +237,9 @@ export function parseJsonObjectDraft(text: string): JsonObjectDraft { "Numbers must be finite — a value like `1e400` overflows and would be sent as null", }; } - if (hasImpreciseIntegerLiteral(text)) { - return { ok: false, error: IMPRECISE_INTEGER_ERROR }; + const unsendable = findUnsendableNumberLiteral(text); + if (unsendable !== null) { + return { ok: false, error: unsendableNumberError(unsendable) }; } const duplicate = findDuplicateObjectKey(text); if (duplicate !== null) {