From 1ea44307aedcb3d980089462d3f006801b08f3f1 Mon Sep 17 00:00:00 2001 From: che cheng Date: Fri, 29 May 2026 19:38:13 +0800 Subject: [PATCH] fix: emit shortest-round-trippable JSON numbers to remove float noise MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tool output rendered non-exact Doubles via JSONSerialization's 17-digit formatter, so coordinates like 25.04 leaked as 25.039999999999999. Rounding the value cannot fix this — 25.04 has no exact IEEE-754 form, so the rounded result is the same bit pattern; the noise lives in the formatter, not the value. Route every serialized object through JSONSanitize.clean, which recursively rewrites each Double to NSDecimalNumber(Double.description) — the shortest string that round-trips bit-exactly. Int/Bool type tags are preserved and inf/nan pass through unchanged (JSONSerialization rejects them as before). Covers all 8 serialization sites (6 jsonResult helpers + 2 inline RailTools), so coordinates inside any payload (incl. rail station matches) are cleaned, not just the bus-position site named in the issue. Refs #1 --- Sources/CheTransportMCP/Tools/AirTools.swift | 2 +- Sources/CheTransportMCP/Tools/BikeTools.swift | 2 +- Sources/CheTransportMCP/Tools/BusTools.swift | 2 +- .../CheTransportMCP/Tools/JSONSanitize.swift | 59 +++++++++++ .../CheTransportMCP/Tools/MaritimeTools.swift | 2 +- .../CheTransportMCP/Tools/ParkingTools.swift | 2 +- Sources/CheTransportMCP/Tools/RailTools.swift | 4 +- .../CheTransportMCP/Tools/TrafficTools.swift | 2 +- .../JSONSanitizeTests.swift | 100 ++++++++++++++++++ .../ModeExecutorTests.swift | 6 ++ .../RailBusExecutorTests.swift | 11 +- 11 files changed, 178 insertions(+), 14 deletions(-) create mode 100644 Sources/CheTransportMCP/Tools/JSONSanitize.swift create mode 100644 Tests/CheTransportMCPTests/JSONSanitizeTests.swift diff --git a/Sources/CheTransportMCP/Tools/AirTools.swift b/Sources/CheTransportMCP/Tools/AirTools.swift index 07a571b..d1953c0 100644 --- a/Sources/CheTransportMCP/Tools/AirTools.swift +++ b/Sources/CheTransportMCP/Tools/AirTools.swift @@ -173,7 +173,7 @@ enum AirTools { } static func jsonResult(_ obj: [String: Any]) -> CallTool.Result { - let data = (try? JSONSerialization.data(withJSONObject: obj)) ?? Data("{}".utf8) + let data = (try? JSONSerialization.data(withJSONObject: JSONSanitize.clean(obj))) ?? Data("{}".utf8) let text = String(data: data, encoding: .utf8) ?? "{}" return CallTool.Result(content: [.text(text: text, annotations: nil, _meta: nil)]) } diff --git a/Sources/CheTransportMCP/Tools/BikeTools.swift b/Sources/CheTransportMCP/Tools/BikeTools.swift index e6a26ba..3b494bc 100644 --- a/Sources/CheTransportMCP/Tools/BikeTools.swift +++ b/Sources/CheTransportMCP/Tools/BikeTools.swift @@ -282,7 +282,7 @@ enum BikeTools { } static func jsonResult(_ obj: [String: Any]) -> CallTool.Result { - let data = (try? JSONSerialization.data(withJSONObject: obj)) ?? Data("{}".utf8) + let data = (try? JSONSerialization.data(withJSONObject: JSONSanitize.clean(obj))) ?? Data("{}".utf8) let text = String(data: data, encoding: .utf8) ?? "{}" return CallTool.Result(content: [.text(text: text, annotations: nil, _meta: nil)]) } diff --git a/Sources/CheTransportMCP/Tools/BusTools.swift b/Sources/CheTransportMCP/Tools/BusTools.swift index cc2f4ab..1d147e2 100644 --- a/Sources/CheTransportMCP/Tools/BusTools.swift +++ b/Sources/CheTransportMCP/Tools/BusTools.swift @@ -328,7 +328,7 @@ enum BusTools { } static func jsonResult(_ obj: [String: Any]) -> CallTool.Result { - let data = (try? JSONSerialization.data(withJSONObject: obj)) ?? Data("{}".utf8) + let data = (try? JSONSerialization.data(withJSONObject: JSONSanitize.clean(obj))) ?? Data("{}".utf8) let text = String(data: data, encoding: .utf8) ?? "{}" return CallTool.Result(content: [.text(text: text, annotations: nil, _meta: nil)]) } diff --git a/Sources/CheTransportMCP/Tools/JSONSanitize.swift b/Sources/CheTransportMCP/Tools/JSONSanitize.swift new file mode 100644 index 0000000..fac36c4 --- /dev/null +++ b/Sources/CheTransportMCP/Tools/JSONSanitize.swift @@ -0,0 +1,59 @@ +// Sources/CheTransportMCP/Tools/JSONSanitize.swift +import Foundation + +/// Rewrites every `Double` in a JSON-serialisable structure to its shortest +/// round-trippable decimal form, so `JSONSerialization` emits `25.04` instead +/// of `25.039999999999999`. +/// +/// ## Why this exists +/// +/// `JSONSerialization` formats `Double` with up to 17 significant digits — enough +/// to round-trip *any* `Double`, but visually noisy for values with no exact binary +/// representation (`25.04`, `88.5`, fares, …). Rounding the value does **not** help: +/// `25.04` has no exact IEEE-754 representation, so `(25.04 * 1e6).rounded() / 1e6` +/// lands back on the identical bit pattern and serialises to the same noisy string. +/// +/// The noise lives in the *formatter*, not the value. Swift's `Double.description` +/// already produces the **shortest** string that round-trips; wrapping that string in +/// `NSDecimalNumber` lets `JSONSerialization` emit the clean form while preserving the +/// exact numeric value. +/// +/// ## Guarantees +/// +/// - **Value-preserving**: the emitted JSON number parses back to a `Double` +/// numerically equal to the input — bit-identical for every finite value +/// except `-0.0`, which renders as `0` (its sign is dropped, but `JSONSerialization` +/// already loses the sign of `-0.0` on round-trip regardless of this code). +/// - **Type-safe**: `Int` stored as `Int` and `Bool` stored as `Bool` are left +/// untouched — `case let d as Double` does not match native `Int`/`Bool` held in +/// `[String: Any]` (confirmed via `NSNumber.objCType`: `Bool` stays `c`, `Int` `q`). +/// - **Non-finite pass-through**: `inf`/`nan` are returned unchanged, so +/// `JSONSerialization` rejects them exactly as it does today (no behaviour change). +/// - **Extreme-magnitude pass-through**: values beyond `NSDecimalNumber`'s exponent +/// range fall back to the raw `Double` (still round-trips, just with the old +/// 17-digit form). Such magnitudes never occur in transport data. +/// +/// The only observable difference for normal values is that integer-valued doubles +/// render as `25` rather than `25.0` — an identical JSON number that round-trips to `25.0`. +enum JSONSanitize { + /// Recursively returns `value` with every `Double` replaced by a + /// shortest-round-trippable `NSDecimalNumber`. Dictionaries and arrays are + /// walked so nested coordinates (e.g. `{"positions":[{"lat":…}]}`) are covered. + static func clean(_ value: Any) -> Any { + switch value { + case let d as Double: + guard d.isFinite else { return d } + // NSDecimalNumber(string:) returns .notANumber for magnitudes beyond + // Decimal's ±128 exponent ceiling (e.g. 1e300); fall back to the raw + // Double so those still serialize and round-trip. + let decimal = NSDecimalNumber(string: d.description) + return decimal == NSDecimalNumber.notANumber ? d : decimal + case let dict as [String: Any]: + return dict.mapValues(clean) + case let array as [Any]: + return array.map(clean) + default: + return value + } + } +} diff --git a/Sources/CheTransportMCP/Tools/MaritimeTools.swift b/Sources/CheTransportMCP/Tools/MaritimeTools.swift index 76ac622..34369e6 100644 --- a/Sources/CheTransportMCP/Tools/MaritimeTools.swift +++ b/Sources/CheTransportMCP/Tools/MaritimeTools.swift @@ -107,7 +107,7 @@ enum MaritimeTools { } static func jsonResult(_ obj: [String: Any]) -> CallTool.Result { - let data = (try? JSONSerialization.data(withJSONObject: obj)) ?? Data("{}".utf8) + let data = (try? JSONSerialization.data(withJSONObject: JSONSanitize.clean(obj))) ?? Data("{}".utf8) let text = String(data: data, encoding: .utf8) ?? "{}" return CallTool.Result(content: [.text(text: text, annotations: nil, _meta: nil)]) } diff --git a/Sources/CheTransportMCP/Tools/ParkingTools.swift b/Sources/CheTransportMCP/Tools/ParkingTools.swift index 873417e..f1f3ef0 100644 --- a/Sources/CheTransportMCP/Tools/ParkingTools.swift +++ b/Sources/CheTransportMCP/Tools/ParkingTools.swift @@ -145,7 +145,7 @@ enum ParkingTools { } static func jsonResult(_ obj: [String: Any]) -> CallTool.Result { - let data = (try? JSONSerialization.data(withJSONObject: obj)) ?? Data("{}".utf8) + let data = (try? JSONSerialization.data(withJSONObject: JSONSanitize.clean(obj))) ?? Data("{}".utf8) let text = String(data: data, encoding: .utf8) ?? "{}" return CallTool.Result(content: [.text(text: text, annotations: nil, _meta: nil)]) } diff --git a/Sources/CheTransportMCP/Tools/RailTools.swift b/Sources/CheTransportMCP/Tools/RailTools.swift index deb59d2..87f3e84 100644 --- a/Sources/CheTransportMCP/Tools/RailTools.swift +++ b/Sources/CheTransportMCP/Tools/RailTools.swift @@ -166,7 +166,7 @@ enum RailTools { private static func executeListSystems() async throws -> CallTool.Result { let systems = listSystems() - let data = try JSONSerialization.data(withJSONObject: ["systems": systems]) + let data = try JSONSerialization.data(withJSONObject: JSONSanitize.clean(["systems": systems])) let json = String(data: data, encoding: .utf8) ?? "{}" return CallTool.Result(content: [.text(text: json, annotations: nil, _meta: nil)]) } @@ -300,7 +300,7 @@ enum RailTools { } } - let json = try JSONSerialization.data(withJSONObject: ["matches": allMatches]) + let json = try JSONSerialization.data(withJSONObject: JSONSanitize.clean(["matches": allMatches])) let text = String(data: json, encoding: .utf8) ?? "{}" return CallTool.Result(content: [.text(text: text, annotations: nil, _meta: nil)]) } diff --git a/Sources/CheTransportMCP/Tools/TrafficTools.swift b/Sources/CheTransportMCP/Tools/TrafficTools.swift index c699d36..5e80326 100644 --- a/Sources/CheTransportMCP/Tools/TrafficTools.swift +++ b/Sources/CheTransportMCP/Tools/TrafficTools.swift @@ -166,7 +166,7 @@ enum TrafficTools { } static func jsonResult(_ obj: [String: Any]) -> CallTool.Result { - let data = (try? JSONSerialization.data(withJSONObject: obj)) ?? Data("{}".utf8) + let data = (try? JSONSerialization.data(withJSONObject: JSONSanitize.clean(obj))) ?? Data("{}".utf8) let text = String(data: data, encoding: .utf8) ?? "{}" return CallTool.Result(content: [.text(text: text, annotations: nil, _meta: nil)]) } diff --git a/Tests/CheTransportMCPTests/JSONSanitizeTests.swift b/Tests/CheTransportMCPTests/JSONSanitizeTests.swift new file mode 100644 index 0000000..e3691e5 --- /dev/null +++ b/Tests/CheTransportMCPTests/JSONSanitizeTests.swift @@ -0,0 +1,100 @@ +import XCTest +@testable import CheTransportMCP + +/// Unit tests for `JSONSanitize.clean(_:)` — the shortest-round-trippable Double +/// sanitizer that removes IEEE-754 formatting noise from tool JSON output (#1). +/// +/// These pin the guarantees documented on `JSONSanitize`: clean rendering, +/// value-preservation (exact round-trip), Int/Bool type-safety, recursion through +/// nested structures, and non-finite pass-through. +final class JSONSanitizeTests: XCTestCase { + + /// Serialize through the sanitizer the same way the executors do. + private func sanitizedJSON(_ obj: Any) throws -> String { + let data = try JSONSerialization.data(withJSONObject: JSONSanitize.clean(obj), + options: [.sortedKeys]) + return String(decoding: data, as: UTF8.self) + } + + func testCleansFloatNoise() throws { + // 25.04 has no exact IEEE-754 representation → raw JSONSerialization emits + // 25.039999999999999. The sanitizer must produce the clean form. + XCTAssertEqual(try sanitizedJSON(["lat": 25.04]), #"{"lat":25.04}"#) + } + + func testPreservesAlreadyCleanDouble() throws { + XCTAssertEqual(try sanitizedJSON(["lon": 121.56]), #"{"lon":121.56}"#) + } + + func testNegativeCoordinate() throws { + XCTAssertEqual(try sanitizedJSON(["lon": -121.5654]), #"{"lon":-121.5654}"#) + } + + func testHighPrecisionCoordinatePreserved() throws { + XCTAssertEqual(try sanitizedJSON(["lat": 25.0478]), #"{"lat":25.0478}"#) + } + + func testIntegerValuedDoubleRendersAsIntegerButRoundTrips() throws { + // The one observable change: 25.0 renders as 25 (identical JSON number). + XCTAssertEqual(try sanitizedJSON(["lat": 25.0]), #"{"lat":25}"#) + + let data = try JSONSerialization.data(withJSONObject: JSONSanitize.clean(["lat": 25.0])) + let parsed = try XCTUnwrap(JSONSerialization.jsonObject(with: data) as? [String: Any]) + XCTAssertEqual((parsed["lat"] as? NSNumber)?.doubleValue, 25.0, "round-trips back to 25.0") + } + + func testRoundTripIsNumericallyExact() throws { + // Includes the adversarial magnitudes the design claims to handle: + // ordinary coords/fares, scientific-notation small/large, the NSDecimal + // out-of-range extremes (which take the raw-Double fallback path), and + // -0.0 (round-trips to 0.0, since -0.0 == 0.0). + let values: [Double] = [ + 25.04, -121.5654, 0.1, 13.456789, 88.5, + 1e-7, 1e20, 1e-300, 1e300, -0.0, 0.1 + 0.2, + Double.greatestFiniteMagnitude, Double.leastNonzeroMagnitude + ] + for value in values { + let data = try JSONSerialization.data(withJSONObject: JSONSanitize.clean(["v": value])) + let parsed = try XCTUnwrap(JSONSerialization.jsonObject(with: data) as? [String: Any]) + XCTAssertEqual((parsed["v"] as? NSNumber)?.doubleValue, value, + "sanitized \(value) must parse back numerically equal") + } + } + + func testRecursesIntoNestedArraysAndDicts() throws { + // Mirror a bus_status_positions payload: array of dicts holding coordinates. + let payload: [String: Any] = [ + "positions": [ + ["lat": 25.04, "lon": 121.56], + ["lat": 24.99999999, "lon": -120.5] + ] + ] + let json = try sanitizedJSON(payload) + XCTAssertTrue(json.contains(#""lat":25.04"#), "nested lat cleaned; got \(json)") + XCTAssertTrue(json.contains(#""lon":121.56"#), "nested lon cleaned; got \(json)") + XCTAssertTrue(json.contains(#""lat":24.99999999"#), "deep coord cleaned; got \(json)") + XCTAssertFalse(json.contains("999999999999"), "no IEEE-754 noise anywhere; got \(json)") + } + + func testPreservesIntAndBool() throws { + // Int must not become a decimal; Bool must stay true, NOT 1. + let json = try sanitizedJSON(["count": 3, "live": true, "lat": 25.04]) + XCTAssertTrue(json.contains(#""count":3"#), "Int preserved; got \(json)") + XCTAssertTrue(json.contains(#""live":true"#), "Bool preserved (not 1); got \(json)") + XCTAssertTrue(json.contains(#""lat":25.04"#), "Double cleaned alongside; got \(json)") + } + + func testStringsAndNullPassThrough() throws { + let json = try sanitizedJSON(["name": "中山", "note": NSNull()]) + XCTAssertTrue(json.contains(#""name":"中山""#)) + XCTAssertTrue(json.contains(#""note":null"#)) + } + + func testNonFiniteDoublePassesThroughUnchanged() { + // inf/nan are returned as-is so JSONSerialization rejects them exactly as + // it does for a raw Double — no behaviour change, no silent corruption. + let cleaned = JSONSanitize.clean(["v": Double.infinity]) + XCTAssertFalse(JSONSerialization.isValidJSONObject(cleaned), + "non-finite still rejected by JSONSerialization, as before") + } +} diff --git a/Tests/CheTransportMCPTests/ModeExecutorTests.swift b/Tests/CheTransportMCPTests/ModeExecutorTests.swift index 24317c5..8925f0b 100644 --- a/Tests/CheTransportMCPTests/ModeExecutorTests.swift +++ b/Tests/CheTransportMCPTests/ModeExecutorTests.swift @@ -69,6 +69,12 @@ final class ModeExecutorTests: XCTestCase { XCTAssertLessThan(nearIdx.lowerBound, farIdx.lowerBound, "nearer station should sort first") } XCTAssertEqual(MockURLProtocol.stub?.calls.count, 3, "token + station list + availability") + // Cross-module clean-coordinate guarantee (#1): station coords + the + // echoed search center must carry no IEEE-754 17-digit noise. This is + // the geo-heaviest executor, so it guards the centralized JSONSanitize + // wiring for every non-bus mode. + XCTAssertFalse(text.contains("999999"), "no float noise in bike output; got \(text)") + XCTAssertTrue(text.contains("\"lat\":25.04"), "search center echoed clean; got \(text)") } func testBikeStatusStationReportsAvailability() async { diff --git a/Tests/CheTransportMCPTests/RailBusExecutorTests.swift b/Tests/CheTransportMCPTests/RailBusExecutorTests.swift index 11ea181..83899f7 100644 --- a/Tests/CheTransportMCPTests/RailBusExecutorTests.swift +++ b/Tests/CheTransportMCPTests/RailBusExecutorTests.swift @@ -223,12 +223,11 @@ final class RailBusExecutorTests: XCTestCase { XCTAssertNotEqual(result.isError, true) let text = TestSupport.textContent(result) XCTAssertTrue(text.contains("\"plate\":\"KKA-1234\"")) - // NOTE: lat/lon are emitted as raw Doubles, so JSONSerialization - // renders 25.04 as "25.039999999999999" (IEEE-754 noise). We assert - // the leading digits rather than the literal to stay robust; the - // precision noise itself is a known minor output-quality issue - // affecting every geo-emitting executor. - XCTAssertTrue(text.contains("\"lat\":25.0"), "bus position lat present; got \(text)") + // lat/lon must render as clean shortest-round-trippable JSON numbers + // (no IEEE-754 17-digit noise). JSONSanitize routes every Double through + // NSDecimalNumber(Double.description) before serialization — see #1. + XCTAssertTrue(text.contains("\"lat\":25.04"), "bus position lat clean; got \(text)") + XCTAssertTrue(text.contains("\"lon\":121.56"), "bus position lon clean; got \(text)") } func testBusRejectsInvalidCity() async {