Conversation
Newtonsoft materializes non-integral JSON numbers as double by default, which silently corrupted any value with more significant digits than a double holds. There were two independent loss points: JObject.Parse on every read, and the ExpandoObject round-trip _toJsonFunc uses to apply camelCase naming on write. decimal.MaxValue was worse than lossy — it was written back as 7.922816251426434E+28, after which the item no longer deserialized at all. JsonParser now parses with FloatParseHandling.Decimal on both paths. Decimal covers a much narrower range than double, so a document holding a value outside it is parsed the old way instead: too large throws and is caught, too small would round to zero without any error, so those documents are recognized from the text before parsing. The dynamic API is unchanged. GetItem normalizes a decimal back to double, because dynamic values have always been doubles and a dynamic decimal cannot even be compared to a double literal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjQuyQX3v7BsMX1uURt8zU
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds a decimal-aware JSON parser and routes datastore parsing through it. High-precision decimal values, including ChangesDecimal precision preservation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant DataStore
participant JsonParser
participant CommitActionHandler
participant JSONFile
DataStore->>JSONFile: read JSON text
DataStore->>JsonParser: parse decimal-aware JSON
DataStore->>CommitActionHandler: apply queued commit actions
CommitActionHandler->>JsonParser: parse updated JSON
CommitActionHandler-->>DataStore: return updated JSON
DataStore->>JSONFile: write preserved JSON
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@JsonFlatFileDataStore/DataStore.cs`:
- Around line 441-455: Update SingleDynamicItemReadConverter and
NormalizeFloatValue so normalization traverses nested JArray/List<object>
elements and JObject/dictionary values recursively before returning. Ensure
every decimal reachable through arrays or object contents is converted to
double, while preserving existing handling for non-decimal values and object
materialization.
In `@JsonFlatFileDataStore/JsonParser.cs`:
- Around line 90-105: Update the negative-exponent detection loop in JsonParser
to stop rejecting literals solely because exponent reaches
UnsafeNegativeExponent; determine underflow using the resulting decimal’s scale
and magnitude so safe values such as e-28 remain accepted. Preserve rejection
for values that cannot roundtrip without precision loss, and add a regression
case covering the e-28 boundary.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e63c7dea-2798-4423-9177-7b5c5f6a0c4b
📒 Files selected for processing (5)
CHANGELOG.mdJsonFlatFileDataStore.Test/NumericEdgeCaseTests.csJsonFlatFileDataStore/CommitActionHandler.csJsonFlatFileDataStore/DataStore.csJsonFlatFileDataStore/JsonParser.cs
| return e.ToObject<List<object>>().Select(NormalizeFloatValue).ToList(); | ||
|
|
||
| case JValue jv when e is JValue: | ||
| return jv.Value; | ||
| return NormalizeFloatValue(jv.Value); | ||
|
|
||
| default: | ||
| return e.ToObject<object>(); | ||
| } | ||
| } | ||
|
|
||
| // The document keeps non-integral numbers as decimal so their precision survives a round trip, | ||
| // but the dynamic API has always handed them out as double — and a dynamic decimal can not even | ||
| // be compared to a double literal. Objects are unaffected: they are read through | ||
| // ExpandoObject, which materializes them as double regardless. | ||
| private static object NormalizeFloatValue(object value) => value is decimal d ? (double)d : value; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate file =="
git ls-files | grep -F 'JsonFlatFileDataStore/DataStore.cs' || true
echo "== outline around JsonFlatFileDataStore/DataStore.cs =="
ast-grep outline JsonFlatFileDataStore/DataStore.cs --view expanded | sed -n '1,220p' || true
echo "== relevant lines =="
sed -n '380,475p' JsonFlatFileDataStore/DataStore.cs | nl -ba -v380
echo "== search NormalizeFloatValue and DynamicTypeProvider/Expando/Object handling =="
rg -n "NormalizeFloatValue|ToObject\\(List<object\\)|ToObject\\(object\\)|ExpandoObject|DynamicTypeProvider|IDynamicObject" JsonFlatFileDataStore/DataStore.cs JsonFlatFileDataStore -S || trueRepository: ttu/json-flatfile-datastore
Length of output: 4066
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== git status/diff stat =="
git diff --stat || true
echo "== if DataStore.cs changed, focus diff context =="
if git diff -- JsonFlatFileDataStore/DataStore.cs | wc -l; then
git diff -- JsonFlatFileDataStore/DataStore.cs | sed -n '1,240p'
fi
echo "== all NormalizeFloatValue occurrences with contexts =="
rg -n -C 5 "NormalizeFloatValue|ToObject\\(List<object\\>|ToObject\\(object\\)" JsonFlatFileDataStore/DataStore.cs JsonFlatFileDataStore -S || trueRepository: ttu/json-flatfile-datastore
Length of output: 470
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant DataStore.cs lines =="
awk '{printf "%d\t%s\n", NR, $0}' JsonFlatFileDataStore/DataStore.cs | sed -n '422,456p'
echo "== NormalizeFloatValue usages/contexts =="
rg -n -C 4 'NormalizeFloatValue|ToObject\([^)]*List<object[^)]*\)|ToObject\(object\)' JsonFlatFileDataStore/DataStore.cs
echo "== implementation shape =="
sed -n '430,456p' JsonFlatFileDataStore/DataStore.cs | awk '{printf "%d\t%s\n", NR+429, $0}'Repository: ttu/json-flatfile-datastore
Length of output: 4580
🌐 Web query:
Newtonsoft JsonConvert DeserializeObject List<object> decimal values JValue type Array
💡 Result:
When using Newtonsoft.Json to deserialize JSON into a List, the library defaults to parsing floating-point numbers as System.Double [1][2][3]. This occurs because the default FloatParseHandling setting in JsonSerializerSettings is FloatParseHandling.Double [1][3]. Consequently, when the deserializer encounters a numeric value, it stores it within a JValue of type JTokenType.Float [1][4] using a double precision value, which can lead to precision loss for decimal numbers [1][5][6]. To ensure decimal values are preserved correctly without loss of precision, you must configure the deserializer to use FloatParseHandling.Decimal [1][7][5]. This can be achieved in the following ways: 1. Global Configuration: Set the default settings for the application [1]: JsonConvert.DefaultSettings = => new JsonSerializerSettings { FloatParseHandling = FloatParseHandling.Decimal }; 2. Specific Serialization: Pass JsonSerializerSettings directly to the DeserializeObject method [8][7]: var settings = new JsonSerializerSettings { FloatParseHandling = FloatParseHandling.Decimal }; var list = JsonConvert.DeserializeObject<List>(jsonString, settings); When FloatParseHandling.Decimal is enabled, Newtonsoft.Json parses floating-point numbers directly into decimal values [7][9][10], avoiding the intermediate double conversion and associated rounding errors [5][10]. Note that if your JSON contains extremely large numbers that exceed the capacity of a System.Decimal, using this setting may cause an overflow exception [7][10]. If you are parsing into a JToken hierarchy, this setting must be applied before or during the initial load, as the conversion happens at the moment the character sequence is read [10].
Citations:
- 1: Deserialize decimal same code results not match JamesNK/Newtonsoft.Json#2928
- 2: https://stackoverflow.com/questions/34585581/using-json-net-to-deserialize-to-any-unkown-type-works-for-objects-but-not-value
- 3: https://www.newtonsoft.com/json/help/html/T_Newtonsoft_Json_JsonSerializerSettings.htm
- 4: https://github.com/JamesNK/Newtonsoft.Json/blob/master/Src/Newtonsoft.Json/Linq/JValue.cs
- 5: JToken.ToObject<decimal> wrongly rounds the value JamesNK/Newtonsoft.Json#2622
- 6: Deserializing decimal -> removes trailing zero JamesNK/Newtonsoft.Json#1884
- 7: Reading decimal.MaxValue results in a System.OverflowException JamesNK/Newtonsoft.Json#1904
- 8: https://www.newtonsoft.com/json/help/html/M_Newtonsoft_Json_JsonConvert_DeserializeObject_4.htm
- 9: https://stackoverflow.com/questions/33421981/newtonsoft-json-deserialize-decimal-numbers-with-more-then-8-decimals
- 10: https://stackoverflow.com/questions/60443818/json-net-not-serializing-decimals-the-same-way-twice
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repository project files mentioning Newtonsoft version =="
rg -n "Newtonsoft.Json|JsonSubTypes|JsonConvert" -g '*.csproj' -g '*.props' -g '*.targets' -g '*.cs' . || true
echo "== project refs =="
find . -maxdepth 4 -type f \( -name '*.csproj' -o -name '*.props' \) -print | sort | xargs -r sed -n '1,220p'Repository: ttu/json-flatfile-datastore
Length of output: 7994
🌐 Web query:
JsonSubTypes dynamic deserialization decimal array nested object Newtonsoft.Json
💡 Result:
To handle dynamic deserialization of nested objects or arrays containing polymorphic types in Newtonsoft.Json, you have several primary approaches depending on your specific requirements and how the type information is provided in your JSON. ### 1. Using the JsonSubTypes Library The JsonSubTypes library is a common extension for Newtonsoft.Json that simplifies polymorphic deserialization by allowing you to specify a property as a type discriminator [1][2]. * Registration: You can register subtypes using a fluent builder, which is particularly useful if you need to configure the serializer at runtime or keep your models clean of attributes [1][3]. * Nested Properties: If your type discriminator is located in a nested object, the library supports JSONPath-style property names (e.g., "data.field") to locate the discriminator value [4]. * Example Usage: csharp var settings = new JsonSerializerSettings(); settings.Converters.Add(JsonSubtypesConverterBuilder .Of<Animal>("Type") // Discriminator property name .RegisterSubtype<Cat>(AnimalType.Cat) .RegisterSubtype<Dog>(AnimalType.Dog) .Build()); ### 2. Custom JsonConverter (For Complex/Dynamic Cases) If your requirements exceed what standard libraries offer (e.g., handling complex decimal arrays or deeply nested structures where type inference is tricky), implementing a custom JsonConverter is the most robust approach [5][6][7]. * Implementation: Override ReadJson to manually load the JSON as a JObject or JArray. You can then inspect properties to determine the concrete type and use serializer.Deserialize<T>(reader) to instantiate the appropriate class [5][6]. * Why this works: This approach provides full control over the deserialization process, allowing you to manually map decimal values or handle arrays that don't follow standard naming conventions [5][6]. ### 3. Built-in TypeNameHandling Newtonsoft.Json supports built-in type handling, which embeds a $type property in the JSON [8]. * Usage: Configure your settings with TypeNameHandling = TypeNameHandling.All or TypeNameHandling.Auto [5][9][8]. * Note: While convenient, this approach is often discouraged for public-facing APIs due to security vulnerabilities related to arbitrary type instantiation. Always ensure you have a SerializationBinder implemented if you use this method to restrict which types can be deserialized. ### Handling Decimals and Arrays * Decimal Precision: When deserializing, Newtonsoft.Json generally preserves numeric types based on the JSON content. If you find numbers are being coerced (e.g., to double), ensure your model properties are explicitly typed as decimal rather than object or float [5]. * Polymorphic Arrays: If you have an array of mixed types, you must use a converter that handles the array iteration and identifies the specific type for each item before deserialization [9][6]. If you are encountering issues where nested objects fail to deserialize into the correct type, ensure that the converter is registered correctly in the JsonSerializerSettings or via [JsonConverter] attributes on the specific property rather than just the base type, to prevent infinite recursion [10].
Citations:
- 1: https://manuc66.github.io/JsonSubTypes/
- 2: https://nolansedley.netlify.app/dotnet/derived-types-deserialization/
- 3: https://github.com/manuc66/JsonSubTypes/blob/master/README.md
- 4: Fallback to JSONPath to allow nested field as a deserialization property. manuc66/JsonSubTypes#89
- 5: https://stackoverflow.com/questions/76072954/can-i-dynamically-deserialize-subtypes-with-newtonsoft-json
- 6: https://stackoverflow.com/questions/47578150/json-array-to-custom-object-mapping-in-c-sharp
- 7: https://stackoverflow.com/questions/19134248/newtonsoft-json-nested-deserialization-doesnt-create-correct-object-type-when-t
- 8: https://stackoverflow.com/questions/29688498/how-to-deserialize-json-to-objects-of-the-correct-type-without-having-to-define
- 9: https://stackoverflow.com/questions/7094955/json-deserialization-with-an-array-of-polymorphic-objects
- 10: https://stackoverflow.com/questions/71406900/polymorphic-json-deserialization-in-nested-scenario
🌐 Web query:
Newtonsoft Json13 FloatParseHandling Decimal JArray ToObject<object> decimal nested object ToObject<object>
💡 Result:
When using Newtonsoft.Json (Json.NET), the default behavior is to parse all floating-point numbers as System.Double [1][2][3]. This causes precision loss for decimal values and can result in the removal of trailing zeros [2][4]. To preserve precision for decimal values, you must configure the parser to use FloatParseHandling.Decimal [5][6]. Key Considerations and Solutions: 1. Configuring FloatParseHandling This setting must be applied during the initial parsing of the JSON text, as it affects how the reader interprets the input [7][6]. - When using JToken.Parse or JArray.Parse: These methods do not natively accept settings [4][8]. Instead, use a JsonTextReader [6][4]: var reader = new JsonTextReader(new StringReader(jsonString)) { FloatParseHandling = FloatParseHandling.Decimal }; JArray jArray = JArray.Load(reader); - When using JsonConvert.DeserializeObject: You can pass JsonSerializerSettings [8]: var settings = new JsonSerializerSettings { FloatParseHandling = FloatParseHandling.Decimal }; var data = JsonConvert.DeserializeObject(jsonString, settings); 2. JArray.ToObject Behavior When you call ToObject on a JArray, Json.NET may not automatically convert it into a standard.NET array or collection (like object[] or List) in a way that recurses through nested objects if you simply pass the generic object type [9]. If you need a deeply converted, conventional.NET structure (e.g., converting all JObject/JArray elements into Dictionaries or Lists), ToObject often leaves JTokens intact [9]. To resolve this, you can: - Use a custom JsonConverter that forces the target type to be a Dictionary<string, object> for JObjects and List (or object[]) for JArrays when the target type is object [10][9]. - Use JsonSerializer with the configured FloatParseHandling setting when calling ToObject [10][11]: var serializer = new JsonSerializer { FloatParseHandling = FloatParseHandling.Decimal }; var result = jArray.ToObject<object[]>(serializer); 3. Precision Note FloatParseHandling.Decimal only affects the parsing of JSON text [7]. It ensures that when the raw numbers are read, they are stored as decimal in the JToken structure [5][12]. When you subsequently call ToObject, those already-parsed decimal values will be preserved as System.Decimal types, provided your JsonSerializer configuration is consistent [5].
Citations:
- 1: https://www.newtonsoft.com/json/help/html/P_Newtonsoft_Json_JsonSerializer_FloatParseHandling.htm
- 2: Deserialize decimal same code results not match JamesNK/Newtonsoft.Json#2928
- 3: https://www.newtonsoft.com/json/help/html/t_newtonsoft_json_floatparsehandling.htm
- 4: Allow configuring JsonTextReader settings via JsonToken.Parse and JArray.Parse JamesNK/Newtonsoft.Json#2167
- 5: JToken.ToObject<decimal> wrongly rounds the value JamesNK/Newtonsoft.Json#2622
- 6: https://stackoverflow.com/questions/66714522/decimals-is-being-truncated-while-jobject-parse-or-jarray-parse-in-c
- 7: FloatParseHandling for Serialising data JamesNK/Newtonsoft.Json#2905
- 8: Trailing zeros are removed after parse & format JamesNK/Newtonsoft.Json#2162
- 9: https://stackoverflow.com/questions/36659560/convert-json-net-objects-to-conventional-net-objects-without-knowing-the-types
- 10: System.InvalidCastException: Unable to cast object of type 'Newtonsoft.Json.Linq.JArray' to type 'System.Object[]'. JamesNK/Newtonsoft.Json#3036
- 11: https://www.newtonsoft.com/json/help/html/M_Newtonsoft_Json_Linq_JToken_ToObject_1.htm
- 12: Feature Request: Improve support for deserializing numbers precisely from JSON text with best effort types. JamesNK/Newtonsoft.Json#3101
Normalize nested dynamic values recursively.
SingleDynamicItemReadConverter only normalizes direct array elements, so nested arrays or object values can still expose decimal under the dynamic API even though the documented behavior exposes floats as double. Apply NormalizeFloatValue recursively for JArray/List<object> and JObject/dictionary contents before returning.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@JsonFlatFileDataStore/DataStore.cs` around lines 441 - 455, Update
SingleDynamicItemReadConverter and NormalizeFloatValue so normalization
traverses nested JArray/List<object> elements and JObject/dictionary values
recursively before returning. Ensure every decimal reachable through arrays or
object contents is converted to double, while preserving existing handling for
non-decimal values and object materialization.
Two gaps from review of the decimal-precision fix: A decimal reached dynamic callers through a nested array or object, because only the top level of a single-item array read was normalized. The array is now read through its JSON text like an object is, so every number inside it is materialized as double. A literal such as 1e-28 pushed the whole document onto the double path even though decimal represents it exactly, which corrupted a high-precision decimal stored next to it. The exponent is now weighed together with the digits the mantissa has after the point, so only values past decimal's 28-digit scale fall back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjQuyQX3v7BsMX1uURt8zU
1 participant
Problem
Newtonsoft materializes non-integral JSON numbers as
doubleby default, which silently corrupted any value with more significant digits than a double can hold. There were two independent loss points, visible by writing the same value in both casing modes:1.2345678901234568E+17— already wrong123456789012345678.5— correct123456789012345680_toJsonFuncround-trips the document through anExpandoObjectto apply camelCase naming, and the value is destroyed before it reaches the file.JObject.Parseloses precision on the way in even when the stored document is exact.decimal.MaxValuewas worse than lossy: the file ended up holding7.922816251426434E+28, after which the item was permanently unreadable (JsonReaderException: Input string '7.922816251426434E+28' is not a valid decimal).Fix
JsonParserparses withFloatParseHandling.Decimalon both paths — the reader used for every parse of the document (DataStore,CommitActionHandler) and the ExpandoObject round-trip in_toJsonFunc.Decimal covers a much narrower range than double, so a document holding a value outside it is parsed the old way instead. Too large throws and is caught; too small rounds to zero without any error, which the first attempt at this turned into a silent
double.Epsilon→0regression — so those documents are now recognized from the text before parsing (a negative exponent of 28 or more, or the same magnitude written out in full).The dynamic API is unchanged:
GetItemnormalizes a decimal back to double, because dynamic values have always been doubles and adynamicdecimal cannot even be compared to a double literal. Objects are unaffected — they are read throughExpandoObject, which materializes them as double regardless.Tests
NumericEdgeCaseTests: both casing modes over the previously failing values (123456789012345678.5,79228162514264337593543950,decimal.MaxValue), a value updated after reload, and the out-of-range doubles (MaxValue,MinValue,Epsilon,1e-30). The oldDecimal_LargeValue_LosesPrecision_BehaviorPinnedtest, which pinned the corruption, is replaced.Full suite passes.
DataStoreDisposeTests.DataStore_Dispose(useDispose: False)times out under full-suite load, but it does so with these library changes stashed as well — it is the pre-existing flakiness the test itself documents, and it passes in isolation.🤖 Generated with Claude Code
https://claude.ai/code/session_01VjQuyQX3v7BsMX1uURt8zU
Summary by CodeRabbit