update plan docs with option A impl, and option B assessment

Author: angularsen

review_pr1544.md

@@ -13,15 +13,15 @@ The PR replaces `double` with `QuantityValue` (BigInteger-backed rational) acros
 | | Option A: Add implicit cast | Option B: Keep `double` public |
 |---|---|---|
 | **Approach** | Add `implicit operator double(QuantityValue)` | Revert public API to `double`, use `QuantityValue` internally for conversions |
-| **Rework effort** | Minimal — one operator addition | Significant — revert generated types, adjust CodeGen templates |
+| **Rework effort** | 1 line library code + 101 test fixes (mechanical) | Multi-day: ~10 source files, CodeGen templates, 131 regenerated files, serialization rework |
 | **PR value preserved** | 100% | ~90% |
 | **Consumer breaking changes** | Eliminated for ~60-70% of code | Eliminated for ~60-70% of code |
 | **Precision model** | Consumers store and pass `QuantityValue` (full precision until they convert to `double`) | Consumers always work with `double`; precision only during conversion pipeline |
-| **Risk** | Bidirectional implicit conversion is unusual in C#; may surface edge cases we haven't identified | More conservative; well-understood behavior |
+| **Risk** | Verified: 0 library build errors, 0 new test failures | Not implemented; significant risk of cascading issues in serialization layer |
 | **`var x = len.Meters`** | `x` is `QuantityValue` (but implicitly converts to `double` wherever needed) | `x` is `double` |
 | **Future flexibility** | Consumers who want exact precision can use `QuantityValue` directly today | Would require a later API change to expose `QuantityValue` to consumers |
 
-**Our recommendation: Option A.** It's simpler, preserves the full PR, and our analysis found no concrete compilation failures. Option B is a solid fallback if unforeseen issues arise.
+**Our recommendation: Option A.** Empirically verified: 1 line of library code changed, 0 build errors, 0 new test failures across ~51,900 tests. Option B achieves the same consumer-facing goal but requires multi-day rework with no additional benefit.
 
 ---
 
@@ -81,9 +81,17 @@ With bidirectional implicit conversions (`double ↔ QuantityValue`), C# overloa
 - **Silent precision loss** — converting `QuantityValue` to `double` is lossy, and implicit makes it invisible. This is the *desired* trade-off for most consumers, but purists may object.
 - **lipchev has stated this is a "no-go"** — may need to be verified empirically by building locally with the change
 
-### Verification step
+### Verified empirically
 
-Build the solution locally after changing `explicit` to `implicit` in `QuantityValue.ConvertToNumber.cs`. If it compiles and tests pass, the analysis is confirmed and this is the path forward.
+We implemented Option A on branch `option-a-implicit-cast` and built/tested the full solution.
+
+**Results:**
+- **Library code**: 1 line changed (`explicit` → `implicit` in `QuantityValue.ConvertToNumber.cs`). **Zero library compilation errors.**
+- **Test code**: 101 `Assert.Equal(double, QuantityValue)` calls needed `(double)` casts to resolve xUnit overload ambiguity. This is a mechanical fix — the test semantics are unchanged.
+- **Build**: 0 errors, 0 warnings across all projects.
+- **Tests**: 0 new failures. All ~51,900 tests pass. (4 pre-existing failures on the base PR branch are unrelated: locale-dependent `ConversionExpression.ToString` and custom quantity serialization registration.)
+
+The only issue found was xUnit `Assert.Equal` overload ambiguity when one argument is `double` and the other is `QuantityValue`. This is an xUnit-specific quirk (many overloads including `DateTime`), not a general C# problem. Consumer code using standard method calls, assignments, and operators is unaffected.
 
 ---
 
@@ -139,6 +147,19 @@ The precision benefit is preserved: conversion factors like `1250/381` (feet-to-
 - **Precision only in conversion pipeline** — consumer arithmetic (`length1 + length2`) uses `double`, not rationals
 - **~10% of PR value lost** — the `QuantityValue`-as-public-type feature and its test coverage
 
+### Implementation scope (assessed, not implemented)
+
+Option B was scoped but not implemented due to the scale of changes required:
+
+- **~10 manual source files**: `IQuantity.cs`, `IArithmeticQuantity.cs`, `ILogarithmicQuantity.cs`, `QuantityExtensions.cs`, `QuantityParser.cs`, `Quantity.cs`, `UnitConverter.cs` (public signatures), `Comparison.cs`, plus custom quantity extras
+- **CodeGen templates**: ~15 locations in `QuantityGenerator.cs`, plus `NumberExtensionsGenerator.cs`, `NumberExtensionsCS14Generator.cs`, `QuantityRelationsParser.cs`
+- **Data files**: `UnitRelations.json` type reference
+- **Regeneration**: All 131 quantity files + number extensions + test base classes
+- **Serialization layer rework**: Serializers currently read/write `QuantityValue` from `IQuantity.Value` — changing to `double` fundamentally changes the wire format
+- **Estimated effort**: Multi-day, with significant risk of cascading issues in serialization
+
+Given that Option A achieves the same consumer-facing goal with 1 line of library code changed, Option B is not recommended unless Option A proves unworkable in practice.
+
 ---
 
 ## Other Changes Required Before Merge