Fix floating-point precision error in calculator#4555
Conversation
📝 WalkthroughWalkthroughThe Calculator plugin now converts engine results through a double-based path before decimal rounding, with special handling for non-finite values. The test suite adds precision cases for decimal subtraction and large-number expressions. ChangesCalculator precision fix
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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: 1
🧹 Nitpick comments (1)
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs (1)
131-141: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
catch (FormatException)doesn't actually cover the case it seems intended for.For
double.PositiveInfinity/NegativeInfinity,rawValue.ToString("G14")yields"Infinity", which failsdecimal.ParsewithFormatException— but the fallback(decimal)rawValuethen throwsOverflowException, which isn't caught here and propagates to the outerQuerytry/catch. Functionally this matches the oldConvert.ToDecimalbehavior (also uncaught locally), so there's no regression, but the local catch is misleading as written since it can't actually complete for the inputs it appears designed to handle.🤖 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 `@Plugins/Flow.Launcher.Plugin.Calculator/Main.cs` around lines 131 - 141, The local decimal conversion fallback in Main.cs is misleading because the catch around decimal.Parse in the result formatting path does not actually handle double.PositiveInfinity/NegativeInfinity safely. Update the conversion logic around rawValue, decValue, and roundedResult so the fallback only handles parse-format issues that can truly recover, or remove the misleading catch if the OverflowException is meant to be handled by the outer Query try/catch; keep the behavior aligned with the existing Convert.ToDecimal path and preserve the intent in this result-normalization block.
🤖 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 `@Plugins/Flow.Launcher.Plugin.Calculator/Main.cs`:
- Around line 131-141: The conversion in the Calculator result handling is
losing precision because `rawValue.ToString("G14",
CultureInfo.InvariantCulture)` truncates exact values before `decimal.Parse` in
the `Main` flow. Update this path to preserve full precision for integral or
exact results, either by avoiding the 14-digit format limit or by using a
round-trip-safe conversion before `decValue` is rounded. Keep the existing
`rawValue`, `decValue`, and `roundedResult` logic intact, but ensure the
pre-rounding conversion does not change exact 15+ digit outputs.
---
Nitpick comments:
In `@Plugins/Flow.Launcher.Plugin.Calculator/Main.cs`:
- Around line 131-141: The local decimal conversion fallback in Main.cs is
misleading because the catch around decimal.Parse in the result formatting path
does not actually handle double.PositiveInfinity/NegativeInfinity safely. Update
the conversion logic around rawValue, decValue, and roundedResult so the
fallback only handles parse-format issues that can truly recover, or remove the
misleading catch if the OverflowException is meant to be handled by the outer
Query try/catch; keep the behavior aligned with the existing Convert.ToDecimal
path and preserve the intent in this result-normalization block.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c59fbf30-5f8e-49ff-ac2d-a5b9d80443f7
📒 Files selected for processing (2)
Flow.Launcher.Test/Plugins/CalculatorTest.csPlugins/Flow.Launcher.Plugin.Calculator/Main.cs
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs (1)
133-136: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueNaN/Infinity "special case" always throws — doesn't actually special-case anything.
Casting
double.NaNordouble.PositiveInfinity/NegativeInfinitytodecimalunconditionally throwsOverflowException. Additionally,NaNis already intercepted earlier (line 119-122, replaced with a localized string), soConvert.ToDouble(result)on that string would throw aFormatExceptionbefore this code is even reached — making thedouble.IsNaNbranch here unreachable. OnlyInfinitycan realistically hit this path, and it will always throw, falling into the outercatch (Exception)exactly as it would without this special case at all. As written, this branch adds complexity without changing any behavior.Consider either removing this branch (since it provides no functional benefit over letting the fallthrough path throw) or genuinely handling these values (e.g., mapping to a defined error message) if that was the actual intent.
🔍 Verification
Confirmed via Microsoft's numeric-conversions docs: casting NaN, infinity, or an out-of-range double to decimal always throws OverflowException.
🤖 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 `@Plugins/Flow.Launcher.Plugin.Calculator/Main.cs` around lines 133 - 136, The NaN/Infinity special-case in Main.TryParseResult is unreachable or still throws when casting to decimal, so it adds no behavior beyond the existing exception path. Remove the double.IsNaN/double.IsInfinity branch entirely, or replace it with a real handling path in the calculator result flow that returns a defined message instead of casting; use the surrounding result conversion logic in Main to keep the behavior consistent with the earlier NaN localization branch.
🤖 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.
Nitpick comments:
In `@Plugins/Flow.Launcher.Plugin.Calculator/Main.cs`:
- Around line 133-136: The NaN/Infinity special-case in Main.TryParseResult is
unreachable or still throws when casting to decimal, so it adds no behavior
beyond the existing exception path. Remove the double.IsNaN/double.IsInfinity
branch entirely, or replace it with a real handling path in the calculator
result flow that returns a defined message instead of casting; use the
surrounding result conversion logic in Main to keep the behavior consistent with
the earlier NaN localization branch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 76a16d6c-2f06-4b21-9ef0-3a93e5a69329
📒 Files selected for processing (2)
Flow.Launcher.Test/Plugins/CalculatorTest.csPlugins/Flow.Launcher.Plugin.Calculator/Main.cs
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Plugins/Flow.Launcher.Plugin.Calculator/Main.cs">
<violation number="1" location="Plugins/Flow.Launcher.Plugin.Calculator/Main.cs:133">
P2: The explicit NaN/Infinity branch casts special double values to `decimal`, which throws an `OverflowException` instead of handling them. `decimal` has no representation for `NaN` or `Infinity`, so this check gives a false impression of handling while guaranteeing a throw.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| if (double.IsNaN(rawValue) || double.IsInfinity(rawValue)) | ||
| { | ||
| decValue = (decimal)rawValue; | ||
| } |
There was a problem hiding this comment.
P2: The explicit NaN/Infinity branch casts special double values to decimal, which throws an OverflowException instead of handling them. decimal has no representation for NaN or Infinity, so this check gives a false impression of handling while guaranteeing a throw.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Plugins/Flow.Launcher.Plugin.Calculator/Main.cs, line 133:
<comment>The explicit NaN/Infinity branch casts special double values to `decimal`, which throws an `OverflowException` instead of handling them. `decimal` has no representation for `NaN` or `Infinity`, so this check gives a false impression of handling while guaranteeing a throw.</comment>
<file context>
@@ -130,13 +130,14 @@ public List<Result> Query(Query query)
double rawValue = Convert.ToDouble(result);
decimal decValue;
- try
+ if (double.IsNaN(rawValue) || double.IsInfinity(rawValue))
{
- decValue = decimal.Parse(rawValue.ToString("G14", CultureInfo.InvariantCulture), NumberStyles.Any, CultureInfo.InvariantCulture);
</file context>
| if (double.IsNaN(rawValue) || double.IsInfinity(rawValue)) | |
| { | |
| decValue = (decimal)rawValue; | |
| } | |
| if (double.IsNaN(rawValue) || double.IsInfinity(rawValue)) | |
| { | |
| return new List<Result> | |
| { | |
| new Result | |
| { | |
| Title = rawValue.ToString(CultureInfo.InvariantCulture), | |
| IcoPath = IcoPath, | |
| Score = 300 | |
| } | |
| }; | |
| } |
1 participant
Fixes #4547
Fixes floating-point precision error in the calculator by rounding the double result to 14 significant digits.
Summary by cubic
Fixes floating‑point rounding errors and preserves full precision for large integers by formatting results with culture‑invariant significant‑digit strings before rounding to user settings.
Summary of changes
double, formats with"G17"for integers and"G14"for non‑integers usingInvariantCulture, parses todecimal, then rounds to_settings.MaxDecimalPlaceswithMidpointRounding.AwayFromZero. Replaces directConvert.ToDecimal(result)rounding that leaked FP noise.NaN/Infinity paths before parsing.SubTitleassignment (no behavior change).3000000-2911111.82→88888.18) and adds two large‑integer cases (123456789012345,9007199254740991) to verify integer precision.Release Note
Calculator now shows accurate decimals and preserves exact large integers.
Written for commit 330b93a. Summary will update on new commits.