Skip to content

x-decimalPlaces enforcement retags without re-rounding: the spec says the stored value is reduced, the code only changes the tag #159

Description

@Yaraslaut

Summary

morph::forms::reconcileDeclaredPrecision is documented as making x-decimalPlaces an enforced contract, re-rounding each submitted Quantity so the stored value matches the advertised precision. It does not re-round. It changes only the precision tag, leaving the exact value untouched — so a value stored at full submitted precision is displayed at the declared one.

Display and storage disagree, silently, on every dispatch path.

The contradiction

docs/spec/forms/forms.md, "Advertised precision is enforced on dispatch":

morph::forms::reconcileDeclaredPrecision retags every Quantity member of the action to declaredPrecision() (an exact Rational re-rounding — an empty Quantity stays empty), so the value the handler stores is at the precision the schema advertised, not at the client's submitted dp. This makes x-decimalPlaces an enforced contract on that path rather than an advisory hint.

include/morph/util/quantity.hpp, what the code actually does:

/// @brief Retags the value's *runtime* precision (the exact value itself is
///        unchanged — precision only affects rounding and formatting).
[[nodiscard]] Quantity withDecimalPlaces(math::DecimalPlaces newPrecision) const {
    auto adjusted = *payload;
    adjusted.decimalPlaces = math::DecimalPlaces{...};   // only the tag
    ...
}
[[nodiscard]] Quantity atDeclaredPrecision() const { return withDecimalPlaces(declaredPrecision()); }

Quantity's own doc comment states the truth — "the exact value itself is unchanged" — and directly contradicts the forms spec.

Reproduction

A field declaring dp = 1, given a hand-built payload of 1.23456 at dp = 5:

using Conc = morph::units::Quantity<lims::LimsUnit::mg_per_L, 1>;   // declared dp = 1
struct CaptureResult { Conc value; };

CaptureResult action{Conc{Rational{Numerator{123456}, Denominator{100000}, DecimalPlaces{5}}}};
morph::forms::reconcileDeclaredPrecision(action);

Output:

before: num=3858 den=3125 dp=5
after : num=3858 den=3125 dp=1

3858/3125 is exactly 1.23456. After "enforcement" the stored value is still exactly 1.23456; only the tag moved to dp=1, so it renders as 1.2. Nothing rounded.

Why this matters more than a doc bug

For a lab or financial record, display ≠ stored is disqualifying. The report says 1.2, the database says 1.23456, and the audit trail cannot show which number the analyst actually saw when they verified it. A regulator asking "what value was released?" gets two different answers depending on which layer answers.

It also defeats the stated purpose. The spec's argument for enforcing rather than advising is that a client "may set dp to anything" — but a hostile or buggy client can still get arbitrary precision stored; it just cannot get it shown. Hiding the excess precision is arguably worse than ignoring it, because the value looks compliant.

Which is wrong, the code or the spec?

That is the decision this needs, and it is genuinely open:

  1. Make the code match the spec — re-round the value, not just the tag. Rational can do this exactly. Storage then matches display, and x-decimalPlaces means what the spec says. This loses information the client sent, which is the point.
  2. Make the spec match the code — document x-decimalPlaces as a display/entry-granularity hint that never alters stored values, and drop the "enforced contract" language. Then a caller who needs storage precision enforced must do it themselves, and reconcileDeclaredPrecision's name is misleading.
  3. Reject rather than adjust — a submitted dp finer than declared is a protocol violation; fail the dispatch. Consistent with how morph treats other malformed wire input, and loses nothing silently.

I would take (1) or (3) over (2) for a framework that advertises "exact values for financial/lab data", but this is a public-contract decision.

Context

Found while implementing rung 6 (lims), whose README predicts exactly this ("retag-vs-round … spec text and code disagree; display ≠ stored is disqualifying in a LIMS — this rung owns the decision test, review D1"). Filing it as its own issue since the defect is in morph::forms/morph::units, not in the rung.

Affects every dispatch path: the spec notes ActionDispatcher::registerAction's runner performs the same reconciliation server-side, so local, client-bridge and remote wire paths all behave this way.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area: formsSubsystem: formstriage: validWell-framed; implement as written

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions