feat(web): the serialisation contract — money as strings, values presentation-ready - #544
Conversation
The prerequisite the rest of the web-UI milestone inherits: one shared serialiser turning the frozen report dataclasses into browser-ready JSON, plus the tests that make its three rules checkable rather than merely intended. Endpoints are #534; this ships the contract and nothing that serves it. `keel/web/payload.py` is a FOURTH renderer over the same reports -- `keel/commands/*` renders them to terminal lines, `keel/web/render.py` to HTML, this to JSON -- and it re-gathers, re-derives and re-measures nothing. It consumes `gather_status` (status.py:336), `build_insights_report` (insights.py:251), `build_journal_report` (insights.py:370) and `build_activity_feed`. Rule 1, money crosses the wire as a string. `JSON.parse` yields IEEE-754 doubles and keel is `Decimal`-only precisely because binary floats corrupt money. The failure is silent -- nothing raises, nothing warns, and it only shows past the seventeenth digit or in the last cent of a large notional -- so it is pinned mechanically: a recursive walk over the real payload fails the build on ANY JSON number anywhere in it. That is stronger than "no monetary field is a number" on purpose; the weaker form needs a maintained list of which fields are money, and that list is what rots. The `5E+1` hazard shaped the file. `Decimal.normalize()` renders `Decimal("50")` as `Decimal("5E+1")`, a form that has reached the wire here before and broken real orders, and `str(Decimal(...))` does the same for any positive exponent. So exactly one place turns a Decimal into a string -- `_plain`, using `format(value, "f")`, the only rendering that cannot emit an exponent -- and `normalize()` appears nowhere. Trailing-zero trimming for display is string work on already-formatted text for the same reason. Rule 2, values arrive presentation-ready, with the formatting/computing line drawn where a figure has to already exist on the report. Two places that line was live: - An open position gets NO `notional` and NO `pnl`. `OpenPositionStatus` carries neither, so `qty * entry_price` would have put a figure on the wire the rails never saw -- and the spec's own illustrative payload shows one. Not emitted; the fix, if wanted, is `gather_status`. The spec's `pnl` example lands instead on `JournalEntry.pnl_net`, which `build_journal_report` genuinely computes. - A drawdown is NOT rescaled into a percentage. `drawdown_total_pct` is named for a percentage but holds a fraction (`_rail11_status` compares it against `max_total_dd_pct=Decimal("0.20")`). Multiplying by 100 to make the name true would be arithmetic in the serialiser, so it crosses unchanged through `ratio` and the display carries no `%`. `render.py`'s `pct()` appends one to this same raw fraction and therefore prints "0.05%" for a 5% drawdown; the contract does not inherit that, and the HTML fix is a separate surface. - `qty` carries no unit for the same reason at lower stakes: the base asset is not on the report and parsing it out of `product_id` would be a second place decoding product ids, which is what `guards.py::_asset` exists to be the only one of. Rule 3, semantic state is a field. `state` comes from a closed vocabulary and the `display` glyph carries the same distinction without colour -- today's `--good` and `--bad` differ by hue alone (1.01:1 luminance), which fails WCAG 1.4.1 in an application whose central signal is gain versus loss. `unknown` is kept distinct from `neutral`, so the classifier is deliberately not `render._tone_for_rail`: that helper falls through to "good" and would style a rail nobody has measured as if it passed. Rejected alternatives, recorded: - An integer companion field scaled to cents (the spec's own rejection, restated in the module because this is where it would be added). Precision is per-product -- `base_increment` varies by instrument, which is what #514 and #517 were about -- so a fixed 100x scale truncates anything finer than a cent and a 1e8 scale caps a USD notional near `Number.MAX_SAFE_INTEGER`. No `sort` field ships here; the guard test names itself as where its allowance would have to be written down. - Epoch seconds for a `moment`'s `value`. Reading them means `new Date(Number(value) * 1000)`, and that multiplication is client arithmetic -- small, but exactly the category Rule 2 exists to keep out. ISO-8601 with `Z` instead. - `state` only where interesting, as the spec's abbreviated example shows. Rejected: a client testing whether `state` is present is branching on payload shape, which is inference by another route, and #532's styling needs a word for every field. It is present on every field, always. This is the one deliberate departure from the spec's worked payload. - Bare JSON numbers for counts, which JSON would carry intact. Rejected: "it is only a count" is how the first double gets in, and a displayed count needs a `display` that a bare number cannot carry. The pin extends rather than forks. `keel/web/payload.py` is already inside `test_console_thinness.py`'s scanned set, so Rules 1-5 covered it on arrival; Rule 6 adds what is specific to a module whose output is a money contract rather than a screen -- no `normalize()`, no `float()` except on a timestamp (entry-scoped to `_gmt`), no `round()`, no `json.dumps(..., default=float)`, which is one keyword and looks like a helpful fix for the `TypeError` a `Decimal` raises. It carries a positive control, as Rule 5 does, and `SERIALISER_STEMS` is asserted inside the scanned set so a rename cannot silently drop the pin while Rules 1-5 keep passing. Both halves were watched red. Three mutations -- a leaked raw int, `normalize()` in `_plain`, and `money(qty * entry_price)` -- each failed the test that claims to guard it, including "the serialiser computes nothing", which named the invented notional by path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NyeggYtojNXCTHeD3JHxb6
… once
Three review findings on the serialisation contract, each a place the first
draft's own stated rule did not actually hold.
1. `shown_count` computed, and its guard passed by coincidence.
`journal_payload` shipped `count(len(report.entries))` -- a figure `JournalReport`
does not hold, which is precisely the criterion "the serialiser computes nothing"
claims to prove. It is numerically harmless (a list length is exact) and that is
what made it dangerous: the runtime guard compares FIGURES, and the fixture set
`total_count=2` with exactly 2 entries, so `Decimal(2)` was already in the report's
figure set. One fixture edit from red.
`JournalReport.shown_count` is now a property on the report, the way
`ActivityCycle.is_quiet`/`.key` already are -- a derived reading rather than a
stored field, because a second field holding `len(self.entries)` is state that can
drift from the list it describes. `asdict()` skips properties, so `keel insights
journal --json` is unchanged. The serialiser reads it.
The fixture now sets `total_count=812` against 2 entries, so the two can no longer
coincide; with the old code that guard fires by path
(`journal $.shown_count.value: '2' is on no report field`).
That is still not enough on its own. Once the property exists, `len(entries)` and
`report.shown_count` are the same number, so the runtime guard cannot tell a
measured length from a read one -- confirmed, it passes on that mutation. So Rule 6
of `test_console_thinness.py` gains 6e: `len()` is banned in the serialiser
outright. An AST rule does not care that the coincidence is numerically benign, and
it fails by file, function and line. The two guards fail for different reasons,
which is the point of having both.
2. `stringify` rendered Python reprs.
`ActivityEvent.fields` is parsed out of the engine's own log lines, so a value can
be a list or an object, and the `str(value)` fallback produced
`"{'a': 1, 'b': None}"` -- single quotes, `None`, `True`: text no client can parse
and none should display. No JSON number leaked out of it, so the number-walker
stayed green over a clear Rule 2 breach.
Worse, `str([1e+50])` is `"[1e+50]"`. That is scientific notation reaching the wire
through a path no money field touches, so every `Decimal` precaution in the module
was irrelevant to it -- and the existing notation test could not see it, because it
only feeds Decimals through named report fields.
Nested values now cross as JSON text, normalised leaf by leaf through `stringify`
itself first, so the numbers inside a structure are rendered by exactly the rules
the top-level ones are. Flattened to one string rather than kept as a nested object
because Rule 2 says an open-ended structure handed to the client is a structure the
client must decide how to format. `json.dumps` is called plainly -- every leaf is
already a string, so no encoder is needed, which is why Rule 6d's ban on
`default=`/`cls=` costs nothing. Depth is capped at 6 with a `(nested)` marker:
`json.loads` cannot build a cycle but it can build a thousand nested lists, and the
depth of a line keel did not write must not decide how deep this process recurses.
The notation test now has a sibling covering the open-ended path, including
`[1e+50]` and a nested `1e-30`.
3. `-0.00` made display and state disagree.
`money(Decimal("-0.00"))` displayed `−$0.00` with `state: "neutral"`: the sign was
read off the FORMATTED text (`format(Decimal("-0.00"), ",.2f")` is `"-0.00"`) while
the state was read off the exact value (`Decimal("-0.00") == 0`). A client styling
by `state` and showing `display` printed a minus sign in a neutral colour. Rule 3's
premise is that the two never disagree, and a premise with one exception is not a
premise.
There is now one reading, `_is_negative`, from the exact `Decimal`, feeding both the
glyph and the state; `_magnitude` only strips and formats. Stripping rather than
negating stays deliberate -- `abs()`/`-value` is arithmetic on money, which Rule 3
of the thinness pin forbids outright. A value that IS negative but rounds to zero at
the display precision keeps its minus and keeps `bad` beside it: that is a small
loss honestly labelled, not a disagreement, and suppressing the sign there would
report a loss as a break-even.
Pinned by a case-walk asserting the equivalence over negative zero, true zero,
`0E-8`, a sub-precision loss and ordinary gains and losses, in `money`, `percent`,
`ratio` and `quantity`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NyeggYtojNXCTHeD3JHxb6
Review fixes applied — and one that found a deeper holeAll three review findings fixed. The first one turned out to be more interesting than reported.
|
Closes #533. Prerequisite for the rest of milestone 22 — seven downstream issues inherit this shape.
Ships
keel/web/payload.pyand its tests. No endpoints — those are #534.The three rules
Money crosses the wire as strings, never as JSON numbers.
JSON.parseyields IEEE-754 doubles, and keel isDecimal-only precisely because binary floats corrupt money.Values arrive presentation-ready. The client places them; it never derives them. Every figure a user sees was computed by the Python that holds the rails — which is what makes the invariant checkable rather than merely likely, and why the client needs no decimal library at all.
Semantic state is a field, not an inference.
A client must never decide "this is bad" from a minus sign — that is arithmetic by another route, and it relocates a judgement into the browser.
The guard is stronger than briefed
I asked for "no monetary field serialises as a number". What shipped is no JSON number anywhere — a recursive walk over the round-tripped payload, with
boolexcluded first sinceboolsubclassesint.That needs no maintained list of money fields, so it cannot rot as fields are added. It also makes the reserved
sortfield coherent: it would be the single, named exception.There is a positive control (
test_the_number_walker_is_proven_false_capable) so the walker cannot silently stop walking.Four places it declined to compute
This is the invariant doing its job, and worth reading before review:
OpenPositionStatuscarriesqtyandentry_price, and my own spec example shows anotional. It is not emitted. A test pins the absence and namesgather_statusas where the fix belongs.drawdown_total_pctis named for a percentage but holds a fraction (guards.py:583compares it againstmax_total_dd_pct: 0.20). It crosses unchanged, with no%in the display. This surfaced The browser shows the drawdown breaker 100x too small: pct() appends % to a fraction #542:render.py'spct()appends%to that same fraction, so the current browser shows a 20% drawdown ceiling as0.20%. This PR does not inherit that bug and pins that it does not.qty— the base asset is not on the report, and deriving it means parsingproduct_id, whose audited decoder is inguards.pyand out of this layer's reach.render._tone_for_rail— it falls through to"good", so"unknown"(which_rail11_statusreturns deliberately, because "ok" would be a lie) would style as passing.Thinness pinning
test_console_thinness.pywas extended, not forked — the module was already in its scanned set, so Rules 1–5 applied on arrival. Added Rule 6, serialiser-scoped: no.normalize(), noround(), no unscopedfloat(), no custom JSON encoder..normalize()is banned for a measured reason: it rendersDecimal("50")asDecimal("5E+1"), and that exact form has previously reached the wire in this codebase.Watched red, for the right reasons. Three deliberate mutations — a leaked raw
int,normalize()in_plain, andmoney(qty * entry_price)— each failed the test claiming to guard it, and the computes-nothing test named the invented notional by its JSON path.One deliberate departure from the spec
stateis present on every field, includingneutralandunknown. The spec's abbreviated example showsequitywith onlyvalue/display.Reason: a client testing for
state's presence is branching on payload shape, which is inference by another route — the thing rule 3 exists to prevent. Recorded in the module docstring and pinned by a test.Gates
ruff check keel tests packages— All checks passedmypy— no issues in 355 source filespytest -q— 4585 passed, 3 skipped🤖 Generated with Claude Code
https://claude.ai/code/session_01NyeggYtojNXCTHeD3JHxb6