From 6cb490d8e07bcdd428b49fc9a35a29b1d70ebd3b Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Thu, 24 Sep 2026 07:05:37 +0200 Subject: [PATCH 1/2] forms(qml): a plain "number" member is a number on the wire, not a quoted string (fixes #802) `fieldJsonLiteral` had no branch for a schema property declared plain `"number"` -- a bare `double`/`float`, no `x-decimalPlaces`, no `Quantity` wrapper. Such a member fell through to `JSON.stringify(text)`, so the body carried `"3.5"` where the schema asks for `3.5`; and because a `TextField` validates nothing, `{"ratio":"banana"}` was submitted just as readily with the form reporting `ready` for it. Measured, against the renderer before this change: FAIL! : DynamicFormPlainNumber::test_a_decimal_is_submitted_as_a_json_number() FAIL! : DynamicFormPlainNumber::test_text_that_is_not_a_json_number_is_refused() accepted abc ... 9 failed of 315 The fix is an encoding branch, deliberately *not* the readiness mechanism that landed for nested aggregates: a bare number is representable -- a text field is exactly the control for it -- and routing it through the unrepresentable path would leave every form with a `double` unsubmittable, which is worse than a wrong value. The branch normalises the locale's separators as the `Quantity` entry does, requires `-?\d+(\.\d+)?`, gates on the declared range (which for a plain member is the one glaze stamps on the type: a value past a `float`'s 3.4e38 is refused) and on `multipleOf`, and emits the typed digits verbatim rather than round-tripping them through a JS number. The three neighbouring number-ish shapes were surveyed rather than assumed, by printing what `schemaJson()` actually emits: - `x-decimalPlaces` / `units::Quantity` -- one shape, not two: a generated `Quantity` property is `"type": ["object","null"]` with `x-decimalPlaces` beside it, and the exact `{num,den,dp}` branch handles it. Working, now pinned with that measured schema instead of an idealised one. - a bare `math::Rational` member -- **not** working, and not a missing encoding branch either: its schema is an inline object of `num`/`den`/`dp`, which the renderer reports *unrepresentable*, so a form over `SetBudgetLimit`-shaped action cannot be submitted at all. It is encodable -- `x-decimalPlaces` on that property hands it the exact-decimal control whose literal is precisely that shape, which the new suite pins. Recorded on the issue rather than folded in here. `src/qt/forms/tests/tst_DynamicFormPlainNumber.qml` covers the encoding, the wire grammar, the range gate, the branch order against a declared precision, and the two neighbours as regression guards. Mutating the branch to quote its literal while keeping its validation reddens 7 of the 16 new cases, so the number-vs-string assertions are load-bearing and not carried by the validation. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW --- docs/spec/forms/forms.md | 72 +++- src/qt/forms/qml/DynamicForm.qml | 62 +++- .../tests/tst_DynamicFormPlainNumber.qml | 322 ++++++++++++++++++ 3 files changed, 454 insertions(+), 2 deletions(-) create mode 100644 src/qt/forms/tests/tst_DynamicFormPlainNumber.qml diff --git a/docs/spec/forms/forms.md b/docs/spec/forms/forms.md index aeda45efb..e30e7fdd2 100644 --- a/docs/spec/forms/forms.md +++ b/docs/spec/forms/forms.md @@ -918,6 +918,71 @@ unseeded and is omitted from the request body until the user touches it, which is what distinguishes "not answered" from an explicit `false` for a `std::optional` member. +### Plain number fields — `type: "number"` + +glaze emits `{"type": "number"}` for a bare `double` or `float` member, under +`$defs/double` / `$defs/float` with the type's own range on the definition node: + +```json +"$defs": {"float": {"type": "number", + "minimum": -3.4028234663852886e+38, + "maximum": 3.4028234663852886e+38}}, +"properties": {"score": {"$ref": "#/$defs/float", "x-order": 1}} +``` + +Such a member carries no `x-decimalPlaces` and is not a `Quantity`, so none of +the exact-decimal machinery applies to it. The shipped `DynamicForm.qml` +renderer draws the plain `TextField` and encodes its text as a JSON **number**, +which it needs a branch of its own to do: the plain-text fall-through +(`JSON.stringify(text)`) would submit `{"ratio":"3.5"}` where the schema asks +for `{"ratio":3.5}`, and a `TextField` applies no validation of its own, so +`{"ratio":"banana"}` would be submitted just as readily and the form would +report `ready` for it. + +The encoding is: + +- **Locale-normalised first**, exactly as a `Quantity`'s entry is: the decimal + separator and the digit grouping are the display locale's, and neither + belongs in the JSON number (`"1,000.5"` → `1000.5` in a locale that groups on + comma). +- **Grammar `-?\d+(\.\d+)?`** on the normalised text. No exponent, no trailing + separator, no bare fraction: a spelling outside it has no literal, so the + field is not engaged and the form is not ready — it is never encoded as a + string instead. +- **Digits carried through as typed**, not round-tripped through a JS number, + which would re-spell a long entry in exponent form and round it at the + seventeenth digit. Only the leading-zero run is removed, because JSON forbids + it: `"007.50"` → `7.50`. Trailing fraction zeros are kept — re-spelling the + fraction is not the encoder's business. +- **Gated on the declared range** and on `multipleOf`, like an integer field. + For a plain member the range is the one glaze stamps on the type, so a value + a `float` cannot hold is refused by the same check that enforces a + [`FieldMeta` bound](#per-field-scalar-bounds--minimum--maximum--multipleof). + `allFieldBoundsSatisfied` does not check a `double` member — it reads + `Quantity`, bare `math::Rational` and integral members only — but `ready` is + a claim about the payload satisfying **the schema**, and the schema states + the bound. + +Three neighbouring shapes are numeric too and encode differently, which is why +the renderer asks in this order — `Quantity`, then `integer`, then plain +number: + +| Member | Schema | Encoding | +|---|---|---| +| `Quantity` | `"type": ["object","null"]`, `x-decimalPlaces`, `ExtUnits` | exact `{num,den,dp}`, assembled from the typed digits | +| an integral member | `"type": "integer"` (+ `x-exactMinimum`/`x-exactMaximum` past 2^53) | bare integer, gated on the [exact string bounds](#exact-numeric-bounds--x-exactminimum--x-exactmaximum) | +| `double` / `float` | `"type": "number"` | bare JSON number, as above | + +A generated `Quantity` property is therefore not `"number"` at all. The order +still matters, because a *decorated* schema can put `x-decimalPlaces` on a +property whose type is `"number"` — a precise field spelled the plain way. The +declared precision wins there, and the field keeps the exact encoding. + +A bare `math::Rational` member is **not** in this family: its schema is an +inline object of `num`/`den`/`dp` with no `x-decimalPlaces`, so it is +[unrepresentable](#what-ready-claims) — the one object-typed member whose C++ +declaration looks scalar. + ### Nullable fields whose type is a `$ref` — `anyOf` A nullable member whose underlying type is emitted as a definition rather than @@ -1200,7 +1265,12 @@ two shapes: - an **object-typed** member that no typed control claims — a [nested aggregate](#nested-aggregates-recursive-cycle-safe), whose one scalar - control collects text where the schema asks for an object; + control collects text where the schema asks for an object. A bare + `math::Rational` member is this shape too, and is the instance least likely + to be expected: its C++ declaration is a scalar and its schema is an object + of `num`/`den`/`dp`. Wrapping it in a `Quantity` — or giving the field an + `x-decimalPlaces` — is what hands it the exact-decimal control that encodes + that shape; - an **array whose `items` are objects** (or arrays), which takes the [`type: "array"` control](#array-fields--type-array) and encodes each entry as a JSON string. A control *was* drawn and the member is unrepresentable anyway, diff --git a/src/qt/forms/qml/DynamicForm.qml b/src/qt/forms/qml/DynamicForm.qml index 71de74d7b..a820244b2 100644 --- a/src/qt/forms/qml/DynamicForm.qml +++ b/src/qt/forms/qml/DynamicForm.qml @@ -13,6 +13,8 @@ // x-min/x-max/x-step -> slider track bounds + increment (Ranged fields) // type: "array" -> comma-separated-with-validation control; encodes to // a genuine JSON array literal, e.g. "a, b" -> ["a","b"] +// type: "number" -> plain text field encoding a JSON number, for a member +// with no x-decimalPlaces (a bare double/float) // x-submitMode: "explicit" -> suppresses auto-submit-on-validity; renders // an explicit Submit button (enabled only while ready) // instead -- see "Explicit submit mode" below @@ -487,6 +489,9 @@ Frame { // `isArray` is deliberately absent: the array control claims // the property but encodes its items as strings, so an array // of objects is unrepresentable even though a control drew it. + // `isNumber` is absent for the same reason: a member declared + // both "number" and "object" is satisfied by neither reading, + // and the object one is the half no typed text can close. const typedControl = dp !== undefined || optionsAction !== undefined || enumOptionRows.length > 0 || p.format === "date-time" || types.indexOf("integer") !== -1 || types.indexOf("boolean") !== -1 @@ -524,6 +529,16 @@ Frame { isQuantity: dp !== undefined, decimals: opt(dp, 0), isInteger: types.indexOf("integer") !== -1, + // "number" -- a bare `double`/`float`. The plain text + // field draws it, but the JSON *number* encoding is its + // own: the fall-through at the end of fieldJsonLiteral + // would quote the digits. This flag says only what the + // schema's type is, so it is also true for a *precise* + // field spelled "number" with an x-decimalPlaces beside + // it; fieldJsonLiteral asks `isQuantity` first, which is + // what keeps such a field on the exact {num,den,dp} + // encoding. + isNumber: types.indexOf("number") !== -1, // "boolean" -- a CheckBox, not the plain text field's // fall-through (which wrapped the typed text as a JSON // *string*: {"flag":"true"}, or {"flag":"banana"} for @@ -1338,6 +1353,50 @@ Frame { return null return normalised } + if (f.isNumber) { + // A bare `double`/`float` member: `"type": "number"` with no + // x-decimalPlaces and no Quantity wrapper, so none of the encoders + // above claims it. It needs one of its own -- the generic + // fall-through at the end of this function would wrap the typed + // digits as a JSON *string*, and the schema asks for a number. + // Declaring it unrepresentable instead would be worse than the + // wrong value it replaces: a number is exactly what a text field + // collects, so a form carrying one would never be submittable. + // + // Locale-normalised first, like a Quantity's entry: a decimal + // field is typed with the locale's decimal separator and + // grouping, and neither belongs in the JSON number. + const canonicalNumber = normalizeLocaleNumber(text, { + decimalSeparator: qtLocale.decimalPoint, + groupSeparator: qtLocale.groupSeparator, + negativeSign: qtLocale.negativeSign, + positiveSign: qtLocale.positiveSign, + zeroDigit: qtLocale.zeroDigit + }) + // No exponent and no trailing separator: the grammar a text field + // is expected to collect, and every spelling outside it (blank + // fraction, stray sign, letters) is refused rather than encoded. + if (canonicalNumber === null || !/^-?\d+(\.\d+)?$/.test(canonicalNumber)) + return null + const numberValue = parseFloat(canonicalNumber) + // The declared range, which for a plain member is the one glaze + // stamps on the type itself: a `float` field carries ±3.4e38, so + // this gate refuses a value the member cannot hold. `ready` is a + // claim about the payload satisfying the schema, so a bound the + // schema states is checked here whether or not the model's own + // bound check covers this member kind. + if (f.minimum !== undefined && numberValue < f.minimum) + return null + if (f.maximum !== undefined && numberValue > f.maximum) + return null + if (violatesMultipleOf(numberValue, f.multipleOf)) + return null + // Digits carried through as typed, never through parseFloat: a + // round-trip through a JS number re-spells what the user wrote + // ("1e+41" for a long entry) and rounds at the seventeenth digit. + // Only the leading-zero run has to go, because JSON forbids it. + return canonicalNumber.replace(/^(-?)0+(?=\d)/, "$1") + } if (f.isBoolean) { // Emitted bare, never quoted. The CheckBox only ever stores these // two spellings; any other retained value (a prefill from a stale @@ -1796,7 +1855,8 @@ Frame { : (fieldColumn.modelData.isQuantity ? "0." + "0".repeat(Math.max(1, fieldColumn.modelData.decimals)) : (fieldColumn.modelData.isInteger ? "0" : "")) - inputMethodHints: (fieldColumn.modelData.isQuantity || fieldColumn.modelData.isInteger) + inputMethodHints: (fieldColumn.modelData.isQuantity || fieldColumn.modelData.isInteger + || fieldColumn.modelData.isNumber) ? Qt.ImhFormattedNumbersOnly : Qt.ImhNone onTextChanged: form.setFieldValue(fieldColumn.modelData.name, text) // Re-seed from the retained value whenever this delegate is diff --git a/src/qt/forms/tests/tst_DynamicFormPlainNumber.qml b/src/qt/forms/tests/tst_DynamicFormPlainNumber.qml new file mode 100644 index 000000000..58ab66a0a --- /dev/null +++ b/src/qt/forms/tests/tst_DynamicFormPlainNumber.qml @@ -0,0 +1,322 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// A plain `"number"` member -- a bare `double` or `float`, with no +// `x-decimalPlaces` and no `Quantity` wrapper -- is submitted as a JSON +// *number*. +// +// Without an encoding branch of its own such a member reaches +// `fieldJsonLiteral`'s generic fall-through, `JSON.stringify(text)`, and the +// body carries `"3.5"` where the schema asks for `3.5`. The schema side and +// the encoding side each work; nothing spanned them for this shape, which is +// why every case below asserts the *submitted body*, not merely that the form +// reports ready. +// +// The fixtures are the shapes `schemaJson()` really emits, copied from its +// output rather than idealised: a `double` member is a `$ref` into +// `$defs/double`, whose node carries `type: "number"` **and** the type's own +// `minimum`/`maximum` (±DBL_MAX; ±3.4e38 for a `float`). Those bounds are why +// a plain-number field needs a gate as well as an encoder: a value a `float` +// cannot hold is a value the schema declares out of range. +// +// The last two cases are the neighbouring number-ish shapes, here as +// regression guards rather than as new coverage: changing how a plain number +// encodes is exactly the change that could alter them. + +import QtQuick +import QtTest +import MorphForms + +TestCase { + id: testCase + name: "DynamicFormPlainNumber" + visible: true + + QtObject { + id: mockController + signal replyReceived(string actionType, bool ok, string payload) + signal optionsReceived(string optionsAction, bool ok, string payload) + function submitIfValid(actionType, bodyJson) { + replyReceived(actionType, true, JSON.stringify({ok: true})) + } + function fetchOptions(optionsAction) { optionsReceived(optionsAction, true, "[]") } + } + + // `struct { double ratio; float score; }`, exactly as schemaJson emits it. + property var plainSchema: ({ + "$defs": { + "double": { type: "number", minimum: -1.7976931348623157e+308, maximum: 1.7976931348623157e+308 }, + "float": { type: "number", minimum: -3.4028234663852886e+38, maximum: 3.4028234663852886e+38 } + }, + properties: { + ratio: { "$ref": "#/$defs/double", "x-order": 0, title: "Ratio" }, + score: { "$ref": "#/$defs/float", "x-order": 1, title: "Score" } + }, + required: ["ratio"] + }) + + // The same member with declared bounds on it (what FieldMeta's + // minimum/maximum/multipleOf stamp onto the property node). + property var boundedSchema: ({ + properties: { + temperature: { type: "number", minimum: -40.5, maximum: 120, multipleOf: 0.5, "x-order": 0 } + }, + required: ["temperature"] + }) + + // A Quantity member as schemaJson emits it: an object of num/den/dp with + // `x-decimalPlaces` and `ExtUnits` beside it -- its JSON type is + // "object"/"null", not "number". The exact {num,den,dp} encoding applies + // and must keep applying. + property var quantitySchema: ({ + properties: { + mass: { + type: ["object", "null"], + properties: { num: { type: "integer" }, den: { type: "integer" }, dp: { type: "integer" } }, + additionalProperties: false, + ExtUnits: { unitAscii: "kg", unitUnicode: "kg" }, + "x-decimalPlaces": 2, "x-order": 0, title: "Mass" + } + }, + required: ["mass"] + }) + + // A declared precision on a property whose JSON type *is* "number" -- the + // spelling a decorated schema produces. Both kind flags fire, and the + // order in which fieldJsonLiteral asks decides: precision wins, so the + // field encodes as an exact rational rather than as a bare number. + property var precisionNumberSchema: ({ + properties: { reading: { type: "number", "x-decimalPlaces": 1, "x-order": 0 } }, + required: ["reading"] + }) + + // A bare `math::Rational` member, as schemaJson emits it: an inline + // object of num/den/dp. The renderer draws one text field over it and has + // no encoder for the shape, so it is reported *unrepresentable* and the + // form stays unready. What the case below pins is the part that must hold + // under any future treatment of the shape: whatever is typed, the body + // never carries a JSON string for it. + property var rationalSchema: ({ + "$defs": { + "int64_t": { type: "integer", minimum: -9223372036854775808, maximum: 9223372036854775807 }, + "uint32_t": { type: "integer", minimum: 0, maximum: 4294967295 } + }, + properties: { + limit: { + type: "object", + properties: { + num: { "$ref": "#/$defs/int64_t" }, + den: { "$ref": "#/$defs/int64_t" }, + dp: { "$ref": "#/$defs/uint32_t" } + }, + additionalProperties: false, + "x-order": 0, title: "Limit" + } + }, + required: ["limit"] + }) + + // The same bare-Rational member with `x-decimalPlaces` on its property + // node (what a decorated instance schema stamps there). That hands the + // member the exact-decimal control, whose {num,den,dp} literal is exactly + // the shape this schema asks for -- so the shape is encodable, and it is + // the missing precision, not the object type, that leaves the undecorated + // one unrepresentable. + property var decoratedRationalSchema: ({ + properties: { + limit: { + type: "object", + properties: { num: { type: "integer" }, den: { type: "integer" }, dp: { type: "integer" } }, + "x-decimalPlaces": 2, "x-order": 0, title: "Limit" + } + }, + required: ["limit"] + }) + + Component { + id: plainForm + DynamicForm { actionType: "T_Plain"; schema: testCase.plainSchema; controller: mockController } + } + + Component { + id: boundedForm + DynamicForm { actionType: "T_Bounded"; schema: testCase.boundedSchema; controller: mockController } + } + + Component { + id: quantityForm + DynamicForm { actionType: "T_Quantity"; schema: testCase.quantitySchema; controller: mockController } + } + + Component { + id: rationalForm + DynamicForm { actionType: "T_Rational"; schema: testCase.rationalSchema; controller: mockController } + } + + Component { + id: precisionNumberForm + DynamicForm { + actionType: "T_PrecisionNumber" + schema: testCase.precisionNumberSchema + controller: mockController + } + } + + Component { + id: decoratedRationalForm + DynamicForm { + actionType: "T_DecoratedRational" + schema: testCase.decoratedRationalSchema + controller: mockController + } + } + + function typeInto(form, field, text) { + findChild(form, field).text = text + } + + // ── the defect ─────────────────────────────────────────────────────────── + + function test_a_decimal_is_submitted_as_a_json_number() { + var form = createTemporaryObject(plainForm, testCase) + typeInto(form, "field_ratio", "3.5") + compare(form.ready, true) + // The whole point: a number, not the quoted string the generic + // fall-through produced. + verify(form.previewLine.indexOf('"ratio":3.5') !== -1) + verify(form.previewLine.indexOf('"ratio":"') === -1) + } + + function test_a_whole_number_typed_into_a_number_field_stays_a_number() { + var form = createTemporaryObject(plainForm, testCase) + typeInto(form, "field_ratio", "4") + compare(form.ready, true) + verify(form.previewLine.indexOf('"ratio":4') !== -1) + verify(form.previewLine.indexOf('"ratio":"') === -1) + } + + function test_a_float_member_encodes_the_same_way() { + var form = createTemporaryObject(plainForm, testCase) + typeInto(form, "field_ratio", "1") + typeInto(form, "field_score", "1.25") + compare(form.ready, true) + verify(form.previewLine.indexOf('"score":1.25') !== -1) + verify(form.previewLine.indexOf('"score":"') === -1) + } + + function test_zero_and_negative_values_carry_their_sign() { + var form = createTemporaryObject(plainForm, testCase) + typeInto(form, "field_ratio", "0") + compare(form.ready, true) + verify(form.previewLine.indexOf('"ratio":0') !== -1) + typeInto(form, "field_ratio", "-0.25") + compare(form.ready, true) + verify(form.previewLine.indexOf('"ratio":-0.25') !== -1) + } + + // ── the wire grammar ───────────────────────────────────────────────────── + + function test_leading_zeros_are_stripped_because_json_forbids_them() { + var form = createTemporaryObject(plainForm, testCase) + typeInto(form, "field_ratio", "007.50") + compare(form.ready, true) + // Trailing zeros are kept: "7.50" is a well-formed JSON number and + // re-spelling the fraction is not this encoder's business. + verify(form.previewLine.indexOf('"ratio":7.50') !== -1) + } + + function test_text_that_is_not_a_json_number_is_refused() { + var form = createTemporaryObject(plainForm, testCase) + // Each of these would have been submitted as a quoted string, and the + // form reported ready for it. + var malformed = ["abc", "3.", ".5", "1e5", "--1", "1 2", "0x10", "3,5"] + for (var i = 0; i < malformed.length; ++i) { + typeInto(form, "field_ratio", malformed[i]) + compare(form.ready, false, "accepted " + malformed[i]) + compare(form.previewLine, "") + } + } + + function test_a_grouped_entry_is_read_the_way_a_quantity_entry_is() { + var form = createTemporaryObject(plainForm, testCase) + // The typed text is locale-normalised before it is encoded, exactly as + // a Quantity's is -- a decimal field is typed with the locale's + // separators, and the grouping is not part of the JSON number. + typeInto(form, "field_ratio", "1,000.5") + compare(form.ready, true) + verify(form.previewLine.indexOf('"ratio":1000.5') !== -1) + } + + // ── the gate ───────────────────────────────────────────────────────────── + + function test_a_value_the_member_type_cannot_hold_is_refused() { + var form = createTemporaryObject(plainForm, testCase) + typeInto(form, "field_ratio", "1") + // 1e41, past a float's maximum -- which is the bound the schema's own + // $defs/float carries, not a hand-written one. + typeInto(form, "field_score", "100000000000000000000000000000000000000000") + compare(form.ready, false) + // The same magnitude in the double field is inside its range. + typeInto(form, "field_score", "") + typeInto(form, "field_ratio", "100000000000000000000000000000000000000000") + compare(form.ready, true) + } + + function test_declared_bounds_gate_a_plain_number() { + var form = createTemporaryObject(boundedForm, testCase) + typeInto(form, "field_temperature", "-40.5") + compare(form.ready, true) + typeInto(form, "field_temperature", "-41") + compare(form.ready, false) + typeInto(form, "field_temperature", "120") + compare(form.ready, true) + typeInto(form, "field_temperature", "120.5") + compare(form.ready, false) + // multipleOf 0.5: 0.25 is not a multiple, 0.5 is. + typeInto(form, "field_temperature", "0.25") + compare(form.ready, false) + typeInto(form, "field_temperature", "0.5") + compare(form.ready, true) + verify(form.previewLine.indexOf('"temperature":0.5') !== -1) + } + + function test_a_blank_optional_number_is_omitted_not_empty_stringed() { + var form = createTemporaryObject(plainForm, testCase) + typeInto(form, "field_ratio", "2") + compare(form.ready, true) + verify(form.previewLine.indexOf("score") === -1) + } + + // ── the neighbouring number-ish shapes, unchanged ──────────────────────── + + function test_a_quantity_member_still_encodes_as_an_exact_rational() { + var form = createTemporaryObject(quantityForm, testCase) + typeInto(form, "field_mass", "2.50") + compare(form.ready, true) + verify(form.previewLine.indexOf('"mass":{"num":250,"den":100,"dp":2}') !== -1) + } + + function test_a_declared_precision_beats_the_plain_number_encoding() { + var form = createTemporaryObject(precisionNumberForm, testCase) + typeInto(form, "field_reading", "3.5") + compare(form.ready, true) + verify(form.previewLine.indexOf('"reading":{"num":35,"den":10,"dp":1}') !== -1) + } + + function test_a_bare_rational_member_is_never_submitted_as_a_json_string() { + var form = createTemporaryObject(rationalForm, testCase) + typeInto(form, "field_limit", "3.5") + // Either the renderer gains an encoder for the {num,den,dp} shape, or + // it keeps declining to encode it -- but the one outcome that is wrong + // either way is a body carrying a string for it. + verify(form.previewLine.indexOf('"limit":"') === -1) + verify(!form.ready || form.previewLine.indexOf('"limit":{') !== -1) + } + + function test_a_declared_precision_makes_the_rational_shape_encodable() { + var form = createTemporaryObject(decoratedRationalForm, testCase) + typeInto(form, "field_limit", "3.5") + compare(form.ready, true) + // 350/100 is the same value as 7/2; the wire codec canonicalises it. + verify(form.previewLine.indexOf('"limit":{"num":350,"den":100,"dp":2}') !== -1) + } +} From a014cbc2d1b4ba7d59d04843ef95bd22180a128f Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Thu, 24 Sep 2026 07:16:07 +0200 Subject: [PATCH 2/2] core: say why the strand's notify_all is not a herd, and what its drain costs under load (refs #780) Comment and spec only; no behaviour change. The reported superlinear drain was attributed to `notify_all` under contention, which reading the line invites and measuring refutes twice over. **By construction.** The notify sits inside the `--_inFlight == 0` branch, and because the re-arm increments for the next dispatch before the current one decrements, the count does not reach zero across a handoff -- so a handoff signals nothing at all. `~StrandExecutor` is the only waiter on `_cv`, so `notify_all` wakes at most one thread and is equivalent to `notify_one` here. `git log -S notify_all --follow` on the file returns one commit, the initial import: it has never been `notify_one`, so there is no deliberate fix to undo in either direction. **By measurement.** An instrumented copy of the executor (counters around the notify, the drain's wait predicate, each dispatch and each `_mapMtx` acquisition) driving the saturation case's exact shape -- 8 producers x 400 posts on one key, 3 iterations = 9600 tasks -- against synthetic spinner load on a 12-thread host, clang 22.1.8 -O2: spinners wall_s per run us/task handoffs notifies wakeups-that-found-nothing 0 0.019 0.019 0.020 2.0 9600 3 3 12 2.11 3.53 3.60 4.71 220-490 9600 3 3 24 30.6 32.3 36.5 3186-3803 9600 3 3 36 58.9 59.2 60.4 6140-6290 9600 3 3 Every strand-side count is load-invariant -- one handoff per task, one notification per drain, one wakeup per drain -- while per-task wall clock moves by three orders of magnitude. Contended `_mapMtx` acquisitions *fall* as load rises (2477 idle, 0-81 at 36 spinners), since the producers get less CPU to contend with. Scaled to the case's 20 iterations these give 24 s / 214 s / 394 s against the 23.8 / 170.5 / 396.4 on record, so the instrument reproduces the reported curve rather than a different one. What the cost actually is: with the task body's `std::this_thread::yield()` removed and nothing else changed, the same 9600 tasks under 36 spinners run in 0.018-0.035 s -- 2.6 us/task, the idle figure. A yielding thread goes to the back of the run queue with no sleeper credit, so it waits a full queue round per task; the strand's own wakeup path does not, which is why the no-yield figure is flat in load. That yield is the saturation case's way of maximising drain/re-arm interleaving, so it is load-bearing for what the case detects and is not something to remove. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW --- docs/spec/concurrency_and_lifetimes.md | 12 ++++++++++++ include/morph/core/strand.hpp | 8 ++++++++ 2 files changed, 20 insertions(+) diff --git a/docs/spec/concurrency_and_lifetimes.md b/docs/spec/concurrency_and_lifetimes.md index a0fb2a089..8432ddf06 100644 --- a/docs/spec/concurrency_and_lifetimes.md +++ b/docs/spec/concurrency_and_lifetimes.md @@ -128,6 +128,18 @@ turns it into a set of per-key serial queues: - `_inFlight` counts strand lambdas currently dispatched to the base executor. The destructor waits on `_cv` until `_inFlight == 0` before destroying the map, so no pool thread can touch `_strands` after the executor is gone. +- **The drain's own work is one handoff per task and one notification per + quiescence, whatever the host is doing.** `_cv` is signalled only where + `--_inFlight` reaches zero, and `~StrandExecutor` is its only waiter, so the + `notify_all` wakes at most one thread — a handoff between two tasks for one + key signals nothing. What a loaded host adds is therefore *latency between* + those operations, not more of them: a drain of N tasks costs N dispatches + whose wall clock is the base executor's wakeup latency under the run queue of + the moment. Measured on a 12-thread host, the same drain's per-task cost + spans three orders of magnitude with machine load while every count above + stays fixed. A task body that calls `std::this_thread::yield()` pays far more + again, because a yielding thread goes to the back of the run queue with no + sleeper credit; that is a property of the posted task, not of the strand. - **`~StrandExecutor` is a complete-drain barrier, not only a use-after-free guard.** The re-arm in `scheduleNext` increments `_inFlight` for the next dispatch *before* the current dispatch decrements its own, so the count never diff --git a/include/morph/core/strand.hpp b/include/morph/core/strand.hpp index c069352f2..ac73fe409 100644 --- a/include/morph/core/strand.hpp +++ b/include/morph/core/strand.hpp @@ -370,6 +370,14 @@ class StrandExecutor { { std::scoped_lock const lock{_mapMtx}; if (--_inFlight == 0) { + // Inside the `== 0` branch, so a handoff does not signal at + // all: the re-arm above has already incremented for the + // next dispatch, so the count does not reach zero until the + // strand is quiescent. `~StrandExecutor` is the only waiter + // on this variable, so `notify_all` wakes at most one + // thread and is equivalent to `notify_one` here -- there is + // no herd to wake, and no predicate but `_inFlight == 0` + // for a wakeup to land on and be lost. _cv.notify_all(); } }