Skip to content

feat(web): the serialisation contract — money as strings, values presentation-ready - #544

Merged
eaitbrahim merged 2 commits into
mainfrom
feat-533-serialisation-contract
Aug 24, 2026
Merged

feat(web): the serialisation contract — money as strings, values presentation-ready#544
eaitbrahim merged 2 commits into
mainfrom
feat-533-serialisation-contract

Conversation

@eaitbrahim

Copy link
Copy Markdown
Contributor

Closes #533. Prerequisite for the rest of milestone 22 — seven downstream issues inherit this shape.

Ships keel/web/payload.py and its tests. No endpoints — those are #534.

The three rules

Money crosses the wire as strings, never as JSON numbers. JSON.parse yields IEEE-754 doubles, and keel is Decimal-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.

"pnl": { "value": "-12.34", "display": "▼ −$12.34", "state": "bad" }

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 bool excluded first since bool subclasses int.

That needs no maintained list of money fields, so it cannot rot as fields are added. It also makes the reserved sort field 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:

  1. Notional on an open positionOpenPositionStatus carries qty and entry_price, and my own spec example shows a notional. It is not emitted. A test pins the absence and names gather_status as where the fix belongs.
  2. Drawdowndrawdown_total_pct is named for a percentage but holds a fraction (guards.py:583 compares it against max_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's pct() appends % to that same fraction, so the current browser shows a 20% drawdown ceiling as 0.20%. This PR does not inherit that bug and pins that it does not.
  3. A unit on qty — the base asset is not on the report, and deriving it means parsing product_id, whose audited decoder is in guards.py and out of this layer's reach.
  4. Reusing render._tone_for_rail — it falls through to "good", so "unknown" (which _rail11_status returns deliberately, because "ok" would be a lie) would style as passing.

Thinness pinning

test_console_thinness.py was 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(), no round(), no unscoped float(), no custom JSON encoder.

.normalize() is banned for a measured reason: it renders Decimal("50") as Decimal("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, and money(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

state is present on every field, including neutral and unknown. The spec's abbreviated example shows equity with only value/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 passed
  • mypy — no issues in 355 source files
  • pytest -q4585 passed, 3 skipped

🤖 Generated with Claude Code

https://claude.ai/code/session_01NyeggYtojNXCTHeD3JHxb6

eaitbrahim and others added 2 commits August 23, 2026 21:09
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
@eaitbrahim

Copy link
Copy Markdown
Contributor Author

Review fixes applied — and one that found a deeper hole

All three review findings fixed. The first one turned out to be more interesting than reported.

shown_count — moved upstream, then banned structurally

shown_count is now a @property on JournalReport, following the pattern ActivityCycle.is_quiet/.key already set — derived rather than stored, 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 byte-identical.

The fixture now uses total_count=812 against 2 entries so the two can no longer coincide, and restoring the old shape fails as it should:

AssertionError: journal $.shown_count.value: '2' is on no report field

But that exposed a second problem. Once the property exists, len(report.entries) and report.shown_count are the same number — so reintroducing count(len(...)) passes the runtime guard. The guard compares figures and is structurally blind to that coincidence.

So len() is now banned outright in the serialiser by AST — Rule 6e:

payload:journal_payload: calls len() (line 925) -- a count on the wire
must be one the report already holds; add it to the report builder

That gives the "a second computed field cannot join silently" property by structure rather than by a maintained exception list. An allowance would have to be written entry-scoped, with its reasoning, the way float(ts) is.

stringify — JSON, not Python reprs

Nested values now cross as JSON text, each leaf normalised through stringify first so numbers inside a structure obey the top-level rules. json.dumps is called plainly — every leaf is already a string, which is why Rule 6d's default=/cls= ban costs nothing.

Depth capped at 6 with a (nested) marker: json.loads cannot build a cycle but can build a thousand nested lists, and the depth of a log line keel did not write must not decide how deep this process recurses.

New test feeds 1e50, 1e-30, [1e50] and {"deep": [{"deeper": 1e-30}]} — the scientific-notation path the original test never reached, because it only fed Decimals through report fields.

-0.00 — one reading of the sign

_magnitude now only strips and formats; _is_negative(value) is the single reading from the exact Decimal, feeding both glyph and state. Stripping rather than negating stays deliberate — abs() is arithmetic on money, which Rule 3 forbids.

  • money(Decimal("-0.00"))"$0.00", neutral, value: "-0.00" (exact value untouched)
  • money(Decimal("-0.001"), signed=True)"▼ −$0.00", bad — a loss too small to show keeps its sign and its judgement; suppressing it would report a loss as a break-even

Scope note

keel/commands/insights.py is now touched — a two-line additive property, no behaviour change — so this branch is no longer confined to keel/web/ plus tests. Flagging it because #534 builds directly on this.

Gates

ruff clean · mypy clean, 355 files · 4592 passed, 3 skipped

@eaitbrahim
eaitbrahim merged commit a8b9697 into main Aug 24, 2026
5 checks passed
@eaitbrahim
eaitbrahim deleted the feat-533-serialisation-contract branch August 24, 2026 05:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The serialisation contract: money as strings, values presentation-ready

1 participant