diff --git a/keel/commands/insights.py b/keel/commands/insights.py index 52b8e8e..94ec8b3 100644 --- a/keel/commands/insights.py +++ b/keel/commands/insights.py @@ -90,6 +90,23 @@ class JournalReport: """The full filtered (closed) entry count BEFORE `--limit` truncates for display.""" filters: dict[str, Any] + @property + def shown_count(self) -> int: + """How many entries this report actually carries, after `--limit` truncated. + + A derived reading rather than a stored field, for the reason `ActivityCycle.key` and + `.is_quiet` are: a second field holding `len(self.entries)` is state that can drift out of + agreement with the list it describes, and there is nothing to gain by letting it. + + It lives HERE rather than in a front-end because `total_count` alone does not answer "am I + looking at a page"; the pair does, and the pair is the report's statement about itself. + Every renderer -- the CLI's lines, the HTML, `keel/web/payload.py`'s JSON -- reads it + instead of measuring the list again, so none of them can disagree about how many rows the + same report held. `asdict()` does not include properties, so `--json`'s shape is + unchanged. + """ + return len(self.entries) + @dataclass(frozen=True) class GateDistance: diff --git a/keel/web/payload.py b/keel/web/payload.py new file mode 100644 index 0000000..72889c0 --- /dev/null +++ b/keel/web/payload.py @@ -0,0 +1,1027 @@ +"""The serialisation contract (#533): frozen report dataclasses in, browser-ready JSON out. + +This module is a FOURTH renderer over the same reports, never a second place that computes them. +`keel/commands/*` builds the frozen report dataclasses and renders them to terminal lines +(`gather_status`/`render_human`, `build_insights_report`/`render_summary`); +`keel/web/render.py` renders the same dataclasses to HTML; this renders them to JSON. Nothing +here re-gathers, re-derives or re-measures anything -- which is what keeps +`tests/commands/test_console_thinness.py` able to pin this layer with the rules it already +applies to the console and the HTML renderer. + +Every endpoint in the web-UI milestone (#534 onwards) serialises through here, so the three +rules below are stated once, in one file a reviewer can read end to end. + +RULE 1 -- MONEY CROSSES THE WIRE AS A STRING, NEVER AS A JSON NUMBER. +`JSON.parse` in a browser yields IEEE-754 doubles. keel is `Decimal`-only precisely because +binary floats corrupt money, and JSON's number type would silently undo that at the boundary -- +silently being the operative word: nothing raises, nothing warns, and the error only shows up +past the seventeenth significant digit or in the last cent of a large notional. +`tests/web/test_payload.py::test_no_wire_value_is_ever_a_json_number` walks the parsed payload +and fails the build on ANY JSON number anywhere in it, which is deliberately stronger than "no +monetary field is a number": the weaker form needs a maintained list of which fields are money, +and that list is exactly the thing that rots. + +**The one measured hazard this file is shaped around.** `Decimal.normalize()` renders +`Decimal("50")` as `Decimal("5E+1")`, and `"5E+1"` has previously reached the wire in this +codebase and broken real orders. `str(Decimal(...))` does the same for any Decimal with a +positive exponent. So there is exactly ONE place a `Decimal` becomes a string here -- `_plain`, +which uses `format(value, "f")`, the only formatting path that cannot emit an exponent -- and +`normalize()` appears nowhere. `tests/commands/test_console_thinness.py`'s Rule 6 fails the build +if `normalize`, `float()` or `round()` is ever added to this module; `test_payload.py` checks the +absence at the level of the rendered payload as well, because an AST rule and a string check fail +for different reasons and neither subsumes the other. + +*An integer companion field scaled to cents was considered and rejected* (spec, § The data +contract). Precision here is PER-PRODUCT -- `base_increment` varies by instrument, which is what +#514 and #517 were about -- so a fixed 100x scale silently truncates anything finer than a cent, +and a 1e8 scale caps a USD notional near `Number.MAX_SAFE_INTEGER`. Should instant client-side +re-sorting ever be wanted, that field is named `sort`, is a plain JSON number, and is documented +as *ordering only -- never displayed, never summed*. It is not in this issue, and the guard test +above is where its allowance would have to be written down. + +RULE 2 -- VALUES ARRIVE PRESENTATION-READY; THE CLIENT PLACES THEM, NEVER DERIVES THEM. +Not `{"qty": "0.01", "price": "50000"}` for a client to multiply. Every figure a user will see is +formatted here, by the process that holds the trading rails, so the client needs no decimal +library at all and a reviewer can confirm the absence of client-side money arithmetic by reading +one file. + +The line between FORMATTING (allowed, and the whole job of this module) and COMPUTING +(forbidden) is: a figure on the wire must already exist on the report. `format`, grouping +separators, trailing-zero trimming, currency symbols and glyphs are formatting. Multiplication, +addition, rescaling and unit conversion are computing, and they belong upstream in the report +builder where the rails can see them. Two places where that line was live while writing this: + +* **An open position has no notional here.** `OpenPositionStatus` carries `qty` and + `entry_price` and nothing else numeric, so `qty * entry_price` was one multiplication away -- + and the spec's own illustrative payload shows a `notional` on a position. It is not emitted. + If a notional is wanted, it is added to `gather_status`. +* **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")`, so `0.05` means five percent. 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 a `%` to this same raw + fraction and therefore prints "0.05%" for a 5% drawdown; the contract does not inherit that.) + +RULE 3 -- SEMANTIC STATE IS AN EXPLICIT FIELD, NEVER AN INFERENCE. +A client must never decide "this is bad" by inspecting a minus sign: that is arithmetic by +another name, and it relocates a trading judgement into a browser. Every field therefore carries +`state`, drawn from the closed vocabulary `STATES`, and the `display` string carries a glyph so +the same distinction survives without colour -- #532's palette work rests on this, and +`--good`/`--bad` in today's stylesheet are separated by hue alone (luminance ratio 1.01:1), which +fails WCAG 1.4.1 in an application whose central signal is gain versus loss. + +**`state` is present on every field, including ones with nothing interesting to say +(`"neutral"`) and ones with no value at all (`"unknown"`).** This is a deliberate departure from +the spec's abbreviated example, which shows `equity` with only `value` and `display`: a client +that must test whether `state` is present is branching on payload SHAPE, which is inference by +another route, and #532's styling table needs a word for every field it touches. Reversing it +would mean accepting that branch on the client, which is the thing this contract exists to +remove. + +WHAT IS *NOT* A FIELD. Identifiers and enum words -- `product_id`, `rule_name`, `mode`, +`as_of` -- cross as bare JSON strings. They carry no precision hazard, no rounding decision and +no judgement, and wrapping them would be ceremony rather than contract. Bare JSON *numbers* +never appear at all: counts, timestamps and open-ended log fields all cross as strings, because +"it is only a count" is how the first double gets in. +""" + +from __future__ import annotations + +import json +import time +from collections.abc import Mapping +from decimal import Decimal, InvalidOperation +from typing import TYPE_CHECKING, Any, TypedDict + +# The CLI's own words for "how long ago" and "how much longer". Imported rather than +# re-implemented so the browser and the terminal can never disagree about the age of the same +# candle -- `keel/commands/insights.py` reaches into `status.py` for `_human_age` the same way, +# for the same reason. `keel.commands.*` is the SERVICE layer, which this layer is allowed to +# read; the compute trees (`keel.strategy`, `keel.execution.*`, `keel.analysis`) are not. +from keel.commands.status import _human_age, _human_remaining + +if TYPE_CHECKING: # pragma: no cover - typing only + from keel.commands.activity import ActivityCycle, ActivityEvent, ActivityFeed + from keel.commands.insights import ( + AccountSummary, + GateDistance, + InsightsReport, + JournalEntry, + JournalReport, + RuleTrackRecord, + ) + from keel.commands.status import ( + AutonomyStatus, + MarketSessionStatus, + OpenPositionStatus, + ProductFreshness, + RuleSummary, + StatusReport, + SubscriptionStatusRow, + WithdrawalAttestationStatus, + ) + + +class Field(TypedDict): + """One value, ready to place. `value` is machine input (exact, ungrouped, `Decimal`-parseable + for figures and ISO-8601 for instants); `display` is the human's, already formatted; `state` + is the judgement, from `STATES`.""" + + value: str + display: str + state: str + + +#: The closed `state` vocabulary. Closed on purpose: #532's palette and glyph table key off +#: exactly these words, and a free-text state would push the client back into guessing. +#: +#: `"unknown"` is NOT a synonym for `"neutral"`. `_rail11_status` returns `"unknown"` when a +#: drawdown scalar was never written, explicitly because reporting an unwritten value as a +#: confident "ok" would be a lie -- there may be a real breach the agent has not computed yet. +#: Collapsing that into a calm-looking neutral would put the lie back. +STATES: frozenset[str] = frozenset({"good", "warn", "bad", "neutral", "unknown"}) + +GOOD = "good" +WARN = "warn" +BAD = "bad" +NEUTRAL = "neutral" +UNKNOWN = "unknown" + +#: An em dash, for a value that was never recorded. `None` means "not recorded" and `0` means +#: "recorded as zero"; collapsing the first into the second is the shape of the always-passing +#: fee rail (#198) -- a missing number that reads as a real one. +ABSENT = "—" + +#: U+25B2 / U+25BC, and U+2212 MINUS SIGN rather than the hyphen-minus a keyboard produces. +#: The glyphs are the non-colour half of #532's gain/loss signal; the true minus is typography. +UP = "▲" +DOWN = "▼" +MINUS = "−" + + +def absent() -> Field: + """A value that was never recorded. + + A fresh dict each call, never a shared constant: every builder returns a `Field` a caller + may put straight into a payload, and one shared mutable default would let an edit in one + endpoint reach every other.""" + return {"value": "", "display": ABSENT, "state": UNKNOWN} + + +# -- the primitives ------------------------------------------------------------------------------ + + +def _plain(value: Decimal) -> str: + """THE one place a `Decimal` becomes a string. `format(value, "f")` is the only rendering + that cannot emit an exponent: `str()` and `normalize()` both turn `Decimal("50")` into + `"5E+1"`, which is the bug that has already reached the wire here. It preserves the full + coefficient, so the result re-parses to a `Decimal` equal to the input, for any input -- + including quantities finer than a cent and notionals past a double's 17 digits.""" + return format(value, "f") + + +def _decimalise(value: Decimal | float | None) -> Decimal | None: + """A finite `Decimal`, or `None` for anything that cannot honestly become one. + + A `float` arrives only from a non-monetary statistic (`RuleTrackRecord.win_rate`); money is + `Decimal` end to end and never takes this path. `repr()` gives the shortest string that + round-trips the float, so the figure on the wire is the figure the report held -- re-encoded, + not recomputed. NaN/Inf (from a corrupt log timestamp, or a `Decimal("NaN")` sentinel) become + `None` and render as absent: a payload is a worse place to raise than a cell is to be empty. + """ + if value is None: + return None + if isinstance(value, Decimal): + return value if value.is_finite() else None + try: + candidate = Decimal(repr(value)) + except (InvalidOperation, ValueError, TypeError): + return None + return candidate if candidate.is_finite() else None + + +def _magnitude(value: Decimal, places: int) -> str: + """The unsigned, grouped, human-readable text for `value`. + + The sign is stripped here and decided elsewhere -- by `_is_negative` below, from the exact + `Decimal` -- so that ONE reading of the sign feeds both the displayed glyph and the `state`. + + The first draft read the sign off this formatted string instead, and the two readings could + disagree: `format(Decimal("-0.00"), ",.2f")` is `"-0.00"`, so the display said `−$0.00` while + `state` said `neutral`, because `Decimal("-0.00") == 0`. A client styling by `state` and + showing `display` would then have printed a minus sign in a neutral colour -- a small thing, + but Rule 3's whole premise is that the two never disagree, and a premise with one exception + is not a premise. + + Stripping rather than negating is still deliberate: `abs()`/`-value` would be arithmetic on + money in this layer, which Rule 3 of the thinness pin forbids outright. + """ + return format(value, f",.{places}f").lstrip("-") + + +def _is_negative(value: Decimal) -> bool: + """THE reading of a money value's sign, for both the glyph and the state. + + A comparison, not arithmetic. Negative zero is NOT negative here -- `Decimal("-0.00") == 0` -- + which is the whole point of having one reading: a value that is not negative never displays a + minus. A value that IS negative but rounds to `0.00` at the requested precision still shows + its minus, and (when the field is a gain/loss) still reads `bad`; the two agree, which is what + matters. + """ + return value < 0 + + +def _trim(text: str) -> str: + """Drop a formatted decimal's trailing zeros -- `"0.01000000"` -> `"0.01"`, `"50.00000000"` -> + `"50"`. String work on an already-formatted number, deliberately, because the obvious + alternative is `Decimal.normalize()` and that is the `5E+1` bug.""" + if "." not in text: + return text + return text.rstrip("0").rstrip(".") + + +def _sign_state(value: Decimal) -> str: + """Rule 3's judgement, made HERE. A comparison, not arithmetic: the browser is told the + answer so it never has to look at the sign itself.""" + if value < 0: + return BAD + if value > 0: + return GOOD + return NEUTRAL + + +#: How deep `stringify` will follow a nested structure before giving up. +#: +#: Log fields are shallow by construction -- each one is a `log_event` kwarg -- so six levels is +#: already generous. The cap exists so that the depth of a value parsed from a line keel did not +#: write cannot decide how deep this process recurses: `json.loads` cannot build a cycle, but it +#: can build a thousand nested lists, and a `RecursionError` while rendering a log line would take +#: down the whole response for a row nobody needed. +_MAX_NESTING = 6 + +#: What a value too deep to render shows as. A marker, not the raw structure: this layer's job is +#: to hand the client something it can display, and "there is more here than we will render" is a +#: displayable fact where a truncated Python repr is not. +_TOO_DEEP = "(nested)" + + +def _normalise(value: Any, depth: int = 0) -> Any: + """A JSON-safe mirror of `value` with every SCALAR already rendered as a string. + + Structure is kept (a list stays a list, an object stays an object) so the shape a log line + recorded survives; only the leaves change. Because every leaf is already a string by the time + `json.dumps` sees it, no encoder is needed -- which matters more than it looks: the encoder a + hurried author reaches for is `default=float`, and that single keyword is the whole contract + dying quietly. Rule 6d of `test_console_thinness.py` fails the build on it. + """ + if depth >= _MAX_NESTING: + return _TOO_DEEP + if isinstance(value, Mapping): + return {str(key): _normalise(item, depth + 1) for key, item in value.items()} + if isinstance(value, (list, tuple)): + return [_normalise(item, depth + 1) for item in value] + return stringify(value, _depth=depth) + + +def stringify(value: Any, *, _depth: int = 0) -> str: + """Any open-ended value, as ONE display string. + + Used for the two places keel's own reports carry values of unconstrained type: + `ActivityEvent.fields` (every `log_event` call site invents its own kwargs, and the parser + keeps them whole rather than projecting onto a fixed schema) and `JournalReport.filters` (the + query echoed back). Rule 1 applies to those exactly as it does to a named money field -- an + echoed `limit` is a JSON number like any other, and "it is only a count" is how the first + double gets in. + + Booleans render as `"true"`/`"false"` rather than Python's `"True"`/`"False"`: the reader is + JavaScript. + + **A nested value crosses as JSON text, never as a Python repr.** `ActivityEvent.fields` is + parsed out of the engine's own log lines, so a value can be a list or an object, and the + obvious `str(value)` produced `"{'a': 1, 'b': None}"` -- single quotes, `None`, `True`: text + no client can parse and none should display. Worse, `str([1e+50])` is `"[1e+50]"`, which puts + scientific notation on the wire through a path no money field touches, so none of the Decimal + care taken elsewhere in this file would have caught it. The structure is normalised leaf by + leaf through this same function first, so the numbers inside a nested value are rendered by + exactly the rules the top-level ones are, and only then dumped as JSON. + + Flattened to a string rather than kept as a nested object because of Rule 2: an open-ended + structure handed to the client is a structure the CLIENT has to decide how to format, and + deciding how to format is what this layer exists to do instead. The activity view places one + string per field. + """ + if value is None: + return "" + if isinstance(value, bool): + return "true" if value else "false" + if isinstance(value, Decimal): + return _plain(value) if value.is_finite() else str(value) + if isinstance(value, float): + decimalised = _decimalise(value) + # A non-finite float has no decimal form at all; `str` gives "nan"/"inf", which is at + # least a word rather than a number a client would try to read as one. + return _plain(decimalised) if decimalised is not None else str(value) + if isinstance(value, (Mapping, list, tuple)): + if _depth >= _MAX_NESTING: + return _TOO_DEEP + # Plain `json.dumps`: `_normalise` has already turned every leaf into a string, so there + # is nothing left for an encoder to convert. `ensure_ascii=False` because the payload is + # UTF-8 and a product id or rule name has no business becoming an escape sequence. + return json.dumps(_normalise(value, _depth), ensure_ascii=False) + return str(value) + + +def _gmt(ts: float | int | None, fmt: str) -> str: + """`ts` in UTC under `fmt`, or `""` for anything unrenderable. + + UTC always, and labelled as such wherever it is shown: keel's day boundaries are UTC + everywhere -- gates, scoping, the activity feed -- and rendering in local time is what made + the activity feed show a stale date (#381), a "today" view that could be permanently empty. + + Total by construction. Log timestamps come from outside this process, and a NaN or an + out-of-range float must cost one cell, not the whole response. `float()` here is the one + conversion this module makes and it is on a TIMESTAMP, which has no cent to lose -- + `time.gmtime` accepts nothing else. `test_console_thinness.py`'s Rule 6b allows it by name + and by argument, so `float(price)` at the same site would still fail the build. + """ + if ts is None: + return "" + try: + return time.strftime(fmt, time.gmtime(float(ts))) + except (OverflowError, OSError, ValueError): + return "" + + +def iso(ts: float | int | None) -> str: + """An instant as ISO-8601 UTC with an explicit `Z`, or `""`. + + `Z` rather than a bare naive string so `new Date(value)` in a browser cannot read it as local. + This is what the top-level `as_of` on every payload carries. + """ + return _gmt(ts, "%Y-%m-%dT%H:%M:%SZ") + + +# -- the field builders -------------------------------------------------------------------------- + + +def money( + value: Decimal | None, + *, + places: int = 2, + signed: bool = False, + state: str | None = None, + symbol: str = "$", +) -> Field: + """A monetary figure. `value` is the exact source `Decimal`; `display` is the human's. + + `signed=True` marks a GAIN-OR-LOSS figure (a net P&L, an R-multiple): it gets the glyph, an + explicit sign, and a `state` derived from the exact source value -- which is Rule 3's whole + point, the judgement made in Python so the client never reads a minus sign. `signed=False` is + a magnitude (a balance, a high-water mark, a fee): no glyph, `neutral` unless the caller knows + better and passes `state`. + + Zero is deliberately glyph-less even when `signed=True`: `▲ +$0.00` claims a direction that + the number does not have. + """ + figure = _decimalise(value) + if figure is None: + return absent() + negative, magnitude = _is_negative(figure), _magnitude(figure, places) + if signed and figure != 0: + glyph = DOWN if negative else UP + sign = MINUS if negative else "+" + display = f"{glyph} {sign}{symbol}{magnitude}" + elif negative: + display = f"{MINUS}{symbol}{magnitude}" + else: + display = f"{symbol}{magnitude}" + resolved = state if state is not None else (_sign_state(figure) if signed else NEUTRAL) + return {"value": _plain(figure), "display": display, "state": resolved} + + +def quantity(value: Decimal | None, *, unit: str = "", places: int = 8) -> Field: + """An instrument quantity. `value` keeps the ledger's own precision -- `0.01000000`, because + `base_increment` is per-product and truncating it is what #514 and #517 were about -- while + `display` trims the padding a human does not read: `0.01 BTC`. + + `places=8` is Coinbase's finest base increment, not a universal truth; it is the ceiling on + what is shown, never a rescaling of what is sent. + """ + figure = _decimalise(value) + if figure is None: + return absent() + negative, magnitude = _is_negative(figure), _magnitude(figure, places) + trimmed = _trim(magnitude) + display = f"{MINUS}{trimmed}" if negative else trimmed + if unit: + display = f"{display} {unit}" + return {"value": _plain(figure), "display": display, "state": NEUTRAL} + + +def percent( + value: Decimal | float | None, + *, + places: int = 2, + signed: bool = False, + state: str | None = None, +) -> Field: + """A figure ALREADY IN PERCENT UNITS (`41.5` meaning 41.5%), suffixed with `%`. + + Never use this for a fraction: `ratio` exists for those, and the conversion between them is + arithmetic this layer does not do. See `ratio`'s note on `drawdown_total_pct`. + """ + figure = _decimalise(value) + if figure is None: + return absent() + negative, magnitude = _is_negative(figure), _magnitude(figure, places) + if signed and figure != 0: + glyph = DOWN if negative else UP + sign = MINUS if negative else "+" + display = f"{glyph} {sign}{magnitude}%" + elif negative: + display = f"{MINUS}{magnitude}%" + else: + display = f"{magnitude}%" + resolved = state if state is not None else (_sign_state(figure) if signed else NEUTRAL) + return {"value": _plain(figure), "display": display, "state": resolved} + + +def ratio( + value: Decimal | float | None, + *, + places: int = 2, + signed: bool = False, + state: str | None = None, +) -> Field: + """A bare dimensionless figure -- a drawdown FRACTION, an R-multiple, a profit factor. + + Kept separate from `percent` because of a real trap: `StatusReport.drawdown_total_pct` is + named for a percentage but holds a fraction (`_rail11_status` compares it against + `max_total_dd_pct=Decimal("0.20")`). Rescaling it by 100 here to make the name true would be + the serialiser computing a figure the report never held -- the exact thing Rule 2 forbids -- + so it crosses unchanged and the display carries no `%`. Making the units honest is a change + to `gather_status`, not to this file. + """ + figure = _decimalise(value) + if figure is None: + return absent() + negative, magnitude = _is_negative(figure), _magnitude(figure, places) + if signed and figure != 0: + glyph = DOWN if negative else UP + sign = MINUS if negative else "+" + display = f"{glyph} {sign}{magnitude}" + elif negative: + display = f"{MINUS}{magnitude}" + else: + display = magnitude + resolved = state if state is not None else (_sign_state(figure) if signed else NEUTRAL) + return {"value": _plain(figure), "display": display, "state": resolved} + + +def count(value: int | None, *, state: str = NEUTRAL) -> Field: + """A whole number of things -- trades, cycles, errors, lines read. + + A count would survive JSON's number type intact, and it still crosses as a string. Two + reasons: a payload with "only a few" numbers in it needs a per-field rule about which ones, + and that rule is what rots; and a count is displayed, so under Rule 2 it needs a `display` + (grouped for reading) that a bare number cannot carry. + """ + if value is None: + return absent() + return {"value": str(value), "display": f"{value:,}", "state": state} + + +def flag( + value: bool | None, + *, + on: str, + off: str, + on_state: str = NEUTRAL, + off_state: str = NEUTRAL, +) -> Field: + """A boolean an operator has to act on -- the kill switch, autonomy, a bracket. + + It arrives with the WORD to show and the state to style it by, rather than as a bare `true` + for a client to translate. Translating is a judgement ("engaged" is bad, "autonomous" is a + warning), and Rule 3 keeps judgements in Python. + """ + if value is None: + return absent() + return { + "value": "true" if value else "false", + "display": on if value else off, + "state": on_state if value else off_state, + } + + +def label(value: str | None, *, display: str | None = None, state: str = NEUTRAL) -> Field: + """An enum word that carries a judgement -- a rail status, an attestation state, an outcome. + + Distinct from a bare JSON string (a `product_id`, a `rule_name`) precisely because of the + judgement: `state` is why this is a field. + """ + if value is None: + return absent() + return {"value": value, "display": display if display is not None else value, "state": state} + + +def moment(ts: float | int | None, *, state: str = NEUTRAL) -> Field: + """An instant. `value` is ISO-8601 UTC so a client can hand it straight to `new Date(...)` + with no arithmetic; `display` is the same instant already written out, so no client ever + formats a date to put one in a table. + + Epoch seconds were the obvious alternative for `value` and were rejected: 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. + + A broken timestamp (NaN, an out-of-range float, a log line written by something else) + degrades to absent, the same choice `render.utc` already makes: a dash in one cell beats a + 500 on the page. + """ + text = _gmt(ts, "%Y-%m-%dT%H:%M:%SZ") + human = _gmt(ts, "%Y-%m-%d %H:%M:%S UTC") + if not text or not human: + return absent() + return {"value": text, "display": human, "state": state} + + +def duration(seconds: int | None, *, elapsed: bool = True, state: str = NEUTRAL) -> Field: + """A span of seconds, in the CLI's own words. + + `_human_age` ("5m ago") and `_human_remaining` ("6d") come from `keel/commands/status.py` -- + the functions `keel status` itself prints -- rather than being re-derived here, so the browser + and the terminal can never disagree about how old the same candle is or how long an + attestation has left. + """ + if seconds is None: + return absent() + human = _human_age(seconds) if elapsed else _human_remaining(seconds) + return {"value": str(seconds), "display": human, "state": state} + + +# -- state classifiers --------------------------------------------------------------------------- + + +def _rail_state(status: str | None) -> str: + """Rail 11's status word, judged. + + Deliberately NOT `render._tone_for_rail`: that helper matches on "breach"/"halt"/"warn" and + falls THROUGH to `"good"`, so `"unknown"` -- the word `_rail11_status` returns when a drawdown + scalar was never written -- is styled as if the rail had passed. `_rail11_status`'s own + docstring says that would be a lie, so the contract keeps `unknown` as its own state. (The + HTML renderer's version is a separate surface and a separate fix.) + """ + lowered = (status or "").lower() + if not lowered or "unknown" in lowered: + return UNKNOWN + if "halt" in lowered or "breach" in lowered or "trip" in lowered: + return BAD + if "warn" in lowered or "near" in lowered: + return WARN + return GOOD + + +#: Rail 17's four states, judged. `expired` and `suspended` are the two that halt live entries; +#: `unattested` has never been answered at all, which is a gap rather than a breach. +_ATTESTATION_STATES: Mapping[str, str] = { + "attested": GOOD, + "suspended": BAD, + "expired": BAD, + "unattested": WARN, + "unknown": UNKNOWN, +} + +#: The venue clock, judged. `clock_unavailable` is a WARNING and not an error: the agent +#: fails closed on it (cycles skip), so the deployment is safe but not trading, and an operator +#: needs to know which of those two it is looking at. +_SESSION_STATES: Mapping[str, str] = { + "open": GOOD, + "closed": NEUTRAL, + "clock_unavailable": WARN, +} + +#: `ActivityFeed.status`, judged. `missing` is the commonest state on a fresh install and is NOT +#: an error -- it also happens when keel is run from a directory that is not the deployment +#: folder -- so it is a warning, not a failure. +_FEED_STATES: Mapping[str, str] = { + "ok": GOOD, + "missing": WARN, + "empty": NEUTRAL, + "unparseable": WARN, + "oversized": WARN, + "unreadable": BAD, +} + + +# -- status --------------------------------------------------------------------------------------- + + +def _autonomy_payload(autonomy: AutonomyStatus) -> dict[str, Any]: + return { + "live": flag( + autonomy.live, + on="ON — orders placed without asking", + off="off", + on_state=WARN, + off_state=NEUTRAL, + ), + "configured": flag(autonomy.autonomous, on="on", off="off"), + "lapses_at": moment(autonomy.autonomous_until), + "updated_at": moment(autonomy.updated_ts), + # An unreadable profile row is reported as autonomy OFF (the safe reading) by + # `_autonomy_status`; the browser is told the reading was degraded rather than being shown + # a confident "off" it cannot distinguish from a measured one. + "profile_readable": flag( + autonomy.profile_readable, + on="yes", + off="unreadable — autonomy reported OFF", + on_state=NEUTRAL, + off_state=WARN, + ), + } + + +def _attestation_payload(attestation: WithdrawalAttestationStatus) -> dict[str, Any]: + return { + "state": label( + attestation.state, + state=_ATTESTATION_STATES.get(attestation.state or "", UNKNOWN), + ), + "enabled": flag(attestation.enabled, on="enabled", off="disabled"), + "attested_at": moment(attestation.attested_at), + "expires_in": duration(attestation.expires_in_sec, elapsed=False), + "expired_for": duration(attestation.expired_for_sec), + } + + +def _session_payload(session: MarketSessionStatus) -> dict[str, Any]: + return { + "state": label( + session.state, state=_SESSION_STATES.get(session.state or "", UNKNOWN) + ), + "recorded_at": moment(session.recorded_ts), + # `defused` is whether a recorded CLOSED still vouches for the quiet -- the reason a + # weekend's stale data must not alert. It is a fact the report already carries, and the + # freshness rows below key off the same word, so the two can never disagree. + "defused": flag(session.defused, on="staleness explained", off="staleness alerts"), + } + + +def _position_payload(position: OpenPositionStatus) -> dict[str, Any]: + """One open position. + + NO `notional` and NO `pnl`. `OpenPositionStatus` carries neither, so emitting one would mean + this layer multiplied `qty` by `entry_price` and put a figure on the wire that the trading + rails never saw. See the module docstring; the fix, if it is wanted, is upstream. + + NO unit on `qty` either, for a quieter version of the same reason. The spec's example shows + `"0.01 BTC"`, but the base asset is not on the report -- it would have to be parsed out of + `product_id`, and the audited place that decodes a product id into an asset is + `keel/execution/guards.py::_asset`, which this layer may not reach (Rules 1 and 2) precisely + so that a malformed id is decoded in ONE place with one set of consequences. `quantity` takes + a `unit` for the day `OpenPositionStatus` carries a `base_asset`; until then the exact + quantity sits beside the `product_id` and nothing is guessed. + """ + return { + "id": str(position.id), + "product_id": position.product_id, + "rule_name": position.rule_name, + "qty": quantity(position.qty), + "entry_price": money(position.entry_price), + "opened_at": moment(position.opened_at), + "bracket": flag( + position.has_bracket, + on="bracketed", + off="NO bracket", + on_state=GOOD, + off_state=WARN, + ), + } + + +def _rule_payload(rule: RuleSummary) -> dict[str, Any]: + return { + "id": str(rule.id), + "kind": rule.kind, + "status": label(rule.status), + "product_id": rule.product_id or "", + # Rule params are operator-supplied and open-ended (any rule kind may invent its own), so + # they cross as strings for the same reason `ActivityEvent.fields` does. + "params": {str(key): stringify(value) for key, value in sorted(rule.params.items())}, + } + + +def _freshness_payload(row: ProductFreshness, session_defused: bool) -> dict[str, Any]: + """One product's data freshness. + + The staleness judgement is NOT made from the age here -- there is no threshold in this file. + It reads `MarketSessionStatus.defused`, which the report already computed: closed AND inside + its trust window is the state under which staleness does not alert, and the TUI's freshness + styling keys off the same field so the cells and the session line can never disagree about + the same weekend. + """ + state = NEUTRAL if (row.last_ts is not None or session_defused) else WARN + return { + "product_id": row.product_id, + "granularity": row.granularity or "", + "last_candle_at": moment(row.last_ts), + "age": duration(row.age_sec, state=state), + } + + +def _subscription_payload(row: SubscriptionStatusRow) -> dict[str, Any]: + return { + "venue": row.venue, + "tier_name": row.tier_name, + "pacing": row.pacing, + "stored_status": label(row.stored_status), + "effective_status": label( + row.effective_status, + state=GOOD if row.effective_status == "active" else WARN, + ), + # `None` means UNLIMITED here, not "not recorded" -- the one place in this file where an + # absent Decimal is a fact rather than a gap, so it is spelled out instead of dashed. + "effective_cap": ( + label("unlimited", state=NEUTRAL) + if row.effective_cap is None + else money(row.effective_cap) + ), + } + + +def status_payload(report: StatusReport) -> dict[str, Any]: + """`gather_status`'s `StatusReport`, as JSON. Serialises; never re-gathers.""" + session_defused = bool(report.market_session.defused) + return { + "as_of": iso(report.now_ts), + "generated_at": moment(report.now_ts), + "mode": report.mode, + "kill_switch": flag( + report.kill_switch_engaged, + on="ENGAGED", + off="clear", + on_state=BAD, + off_state=GOOD, + ), + "autonomy": _autonomy_payload(report.autonomy), + "market_session": _session_payload(report.market_session), + "equity": { + "state_mode": label(report.equity_state_mode), + "high_water_mark": money(report.high_water_mark), + "paper_cash": money(report.paper_cash_usdc), + }, + "drawdown": { + "total": ratio(report.drawdown_total_pct), + "weekly": ratio(report.drawdown_weekly_pct), + "max_total": ratio(report.max_total_dd_pct), + "max_weekly": ratio(report.max_weekly_dd_pct), + "rail11": label(report.rail11_status, state=_rail_state(report.rail11_status)), + }, + "withdrawal_attestation": _attestation_payload(report.withdrawal_attestation), + "open_positions": [_position_payload(p) for p in report.open_positions], + # A LIST of pairs rather than an object, because the order is a presentation decision and + # Rule 2 says presentation decisions are made here. `sorted` matches `render_human`'s own + # ordering, so the two front-ends list the statuses the same way. + "rule_counts": [ + {"status": status, "count": count(number)} + for status, number in sorted(report.rule_counts.items()) + ], + "live_rules": [_rule_payload(r) for r in report.live_rules], + "data_freshness": [_freshness_payload(f, session_defused) for f in report.data_freshness], + "subscriptions": [_subscription_payload(s) for s in report.subscriptions], + } + + +# -- insights -------------------------------------------------------------------------------------- + + +def _account_payload(account: AccountSummary) -> dict[str, Any]: + return { + "mode": account.mode, + "state_mode": label(account.equity_state_mode), + "high_water_mark": money(account.high_water_mark), + "paper_cash": money(account.paper_cash_usdc), + "drawdown_total": ratio(account.drawdown_total_pct), + "drawdown_weekly": ratio(account.drawdown_weekly_pct), + "max_total_dd": ratio(account.max_total_dd_pct), + "max_weekly_dd": ratio(account.max_weekly_dd_pct), + "rail11": label(account.rail11_status, state=_rail_state(account.rail11_status)), + } + + +def _gate_payload(gate: GateDistance) -> dict[str, Any]: + """The promotion gate's distance. `passing` is the engine's verdict, copied; the floors are + the config's own values, copied. Nothing here re-runs `check_floors`.""" + return { + "rule_name": gate.rule_name, + "promotion_class": gate.promotion_class, + "n_trades": count(gate.n_trades), + "min_trades": count(gate.min_trades), + "trades_remaining": count( + gate.trades_remaining, state=GOOD if gate.trades_remaining == 0 else WARN + ), + "win_rate": percent(gate.win_rate, places=1), + "min_win_rate": percent(gate.min_win_rate, places=1), + "realized_rr": ratio(gate.realized_rr), + "min_rr": ratio(gate.min_rr), + "expectancy": money(gate.expectancy, places=4, signed=True), + "min_expectancy": money(gate.min_expectancy, places=4), + "passing": flag(gate.passing, on="passing", off="blocked", on_state=GOOD, off_state=WARN), + "blocking_reasons": list(gate.blocking_reasons), + } + + +def _track_record_payload(record: RuleTrackRecord) -> dict[str, Any]: + return { + "rule_name": record.rule_name, + "status": label(record.status), + "promotion_class": record.promotion_class, + "n_trades": count(record.n_trades), + "win_rate": percent(record.win_rate, places=1), + "avg_win": money(record.avg_win), + "avg_loss": money(record.avg_loss), + "realized_rr": ratio(record.realized_rr), + "expectancy": money(record.expectancy, places=4, signed=True), + "profit_factor": ratio(record.profit_factor), + "max_drawdown": money(record.max_drawdown), + # `significant` is `n_trades >= 30`, computed by the report. Below that floor a win rate + # is not distinguishable from random entry, so the payload SAYS so rather than leaving the + # number to speak for itself -- the same choice `render_insights` makes in HTML. + "significant": flag( + record.significant, + on="n≥30", + off="below the n=30 floor", + on_state=NEUTRAL, + off_state=WARN, + ), + "gate": _gate_payload(record.gate) if record.gate is not None else None, + } + + +def insights_payload(report: InsightsReport) -> dict[str, Any]: + """`build_insights_report`'s `InsightsReport`, as JSON.""" + return { + "as_of": iso(report.now_ts), + "generated_at": moment(report.now_ts), + "account": _account_payload(report.account), + "closed_trade_count": count(report.closed_trade_count), + "rules": [_track_record_payload(r) for r in report.rules], + } + + +# -- journal --------------------------------------------------------------------------------------- + + +def _journal_entry_payload(entry: JournalEntry) -> dict[str, Any]: + """One closed (or, with `--include-open`, one open) trade. + + This is where the spec's worked `pnl` example actually lands: `pnl_net` is a figure + `build_journal_report` computed off the fee-honest `trade_outcomes` ledger, so it can be + serialised without this layer deriving anything. `None` stays absent rather than becoming + `0.00` -- a trade with no recorded net is not a break-even trade, which is the same + distinction `render_insights` protects in HTML. + """ + return { + "closed_at": moment(entry.closed_at), + "opened_at": moment(entry.opened_at), + "rule_name": entry.rule_name or "", + "product_id": entry.product_id, + "qty": quantity(entry.qty), + "entry_fill": money(entry.entry_fill), + "exit_fill": money(entry.exit_fill), + "pnl": money(entry.pnl_net, signed=True), + "fees": money(entry.fees, places=4), + "r_multiple": ratio(entry.r_multiple, signed=True), + "is_dca": flag(entry.is_dca, on="DCA", off="rule"), + "outcome": label(entry.outcome, state=_OUTCOME_STATES.get(entry.outcome, NEUTRAL)), + } + + +#: `JournalEntry.outcome`'s vocabulary, judged. `dca` is neutral by design: a DCA row has no stop +#: to measure against, so `build_journal_report` labels it `"dca"` regardless of the P&L sign and +#: calling it a win or a loss here would invent a verdict the ledger declined to make. +_OUTCOME_STATES: Mapping[str, str] = {"win": GOOD, "loss": BAD, "dca": NEUTRAL, "open": NEUTRAL} + + +def journal_payload(report: JournalReport) -> dict[str, Any]: + """`build_journal_report`'s `JournalReport`, as JSON. + + `total_count` is the full filtered count BEFORE `--limit` truncated `entries`, and + `shown_count` is how many survived; both cross, so a client showing "50 of 812" needs no + subtraction to know it is looking at a page. + + `shown_count` is READ from the report, never measured here. It shipped in the first draft as + `count(len(report.entries))`, which is a figure `JournalReport` did not hold -- numerically + harmless, since a list length is exact, but a breach of the rule this module claims to keep, + and one whose guard passed only by coincidence (the fixture happened to set + `total_count == len(entries)`). It is now a property of the report, and Rule 6e of + `test_console_thinness.py` bans `len()` here so the shortcut cannot be taken again. + """ + return { + "as_of": iso(report.now_ts), + "generated_at": moment(report.now_ts), + "mode": report.mode, + "total_count": count(report.total_count), + "shown_count": count(report.shown_count), + # The query echoed back. Open-ended by shape (`limit` and `since_ts` are ints), so it + # crosses as strings -- see `stringify`. + "filters": {str(key): stringify(value) for key, value in sorted(report.filters.items())}, + "entries": [_journal_entry_payload(e) for e in report.entries], + } + + +# -- activity ------------------------------------------------------------------------------------ + + +def _event_payload(event: ActivityEvent) -> dict[str, Any]: + return { + "at": moment(event.ts), + "level": event.level, + "event": event.event, + "cycle_id": event.cycle_id or "", + # Kept whole rather than projected onto a fixed schema: the event vocabulary grows with + # every `log_event` call site, and an overlay that silently dropped a field it had not + # been taught about would be worse than one that renders it as key=value. + "fields": {str(key): stringify(value) for key, value in event.fields.items()}, + } + + +def _cycle_payload(cycle: ActivityCycle) -> dict[str, Any]: + """One engine cycle. + + `quiet` is `neutral`, never `warn`. A cycle in which the agent looked at the market and + nothing happened is a POSITIVE observation -- a long run of them is how this feed answers "is + it alive" -- so it is rendered muted but never omitted and never flagged. + """ + return { + "key": cycle.key, + "cycle_id": cycle.cycle_id or "", + "started_at": moment(cycle.started_ts), + "ended_at": moment(cycle.ended_ts), + "mode": cycle.mode or "", + "products": list(cycle.products), + "rules": list(cycle.rules), + "signals": count(cycle.signals), + "blocked": count(cycle.blocked), + "entered": count(cycle.entered), + "exited": count(cycle.exited), + "errors": count(cycle.errors, state=BAD if cycle.errors else NEUTRAL), + "highlights": list(cycle.highlights), + "quiet": flag(cycle.is_quiet, on="quiet", off="active"), + "uncorrelated": flag(cycle.is_uncorrelated, on="uncorrelated", off="correlated"), + "events": [_event_payload(e) for e in cycle.events], + "events_dropped": count( + cycle.events_dropped, state=WARN if cycle.events_dropped else NEUTRAL + ), + } + + +def activity_payload(feed: ActivityFeed) -> dict[str, Any]: + """`build_activity_feed`'s `ActivityFeed`, as JSON. + + Every non-`ok` status crosses as a judged state rather than being suppressed: `missing` is the + commonest state on a fresh install and is not an error, and hiding it would leave a user + staring at a blank panel with nothing to act on. + + `scope_fully_covered` is the one field a client must not ignore. False means the bounded tail + read could not prove it reached back past the scope boundary, so an empty feed cannot be read + as "the scope was quiet" -- which is why it arrives already judged as a warning. + """ + return { + "as_of": iso(feed.now_ts), + "generated_at": moment(feed.now_ts), + "status": label(feed.status, state=_FEED_STATES.get(feed.status, UNKNOWN)), + "source": feed.source, + "detail": feed.detail or "", + "scope": feed.scope, + "scope_start_at": moment(feed.scope_start_ts), + "scope_fully_covered": flag( + feed.scope_fully_covered, + on="complete", + off="window may not cover the whole scope", + on_state=NEUTRAL, + off_state=WARN, + ), + "lines_read": count(feed.lines_read), + # Lines read but unusable -- a crash's half-written JSON, a record with no timestamp. + # Surfaced rather than swallowed: silently discarding input is how a feed comes to + # under-report reality while looking healthy. + "lines_skipped": count(feed.lines_skipped, state=WARN if feed.lines_skipped else NEUTRAL), + "window_truncated": flag( + feed.window_truncated, + on="window truncated", + off="whole window read", + on_state=WARN, + off_state=NEUTRAL, + ), + "cycles_dropped": count( + feed.cycles_dropped, state=WARN if feed.cycles_dropped else NEUTRAL + ), + "cycles_out_of_scope": count(feed.cycles_out_of_scope), + "last_cycle_before_scope": ( + _cycle_payload(feed.last_cycle_before_scope) + if feed.last_cycle_before_scope is not None + else None + ), + "cycles": [_cycle_payload(c) for c in feed.cycles], + } diff --git a/tests/commands/test_console_thinness.py b/tests/commands/test_console_thinness.py index 2133aaf..d510f8d 100644 --- a/tests/commands/test_console_thinness.py +++ b/tests/commands/test_console_thinness.py @@ -5,7 +5,8 @@ (`keel/commands/console.py`), the live loop (`keel/commands/tui.py`), every console sub-menu module (`keel/commands/*console*.py`), and -- since #435 -- every module of the local web UI (`keel/web/*.py`). Those files render and dispatch; all behavior must come from the services -the CLI calls. The pin is an AST scan enforcing five rules: +the CLI calls. The pin is an AST scan enforcing six rules -- five that hold across the whole +layer, and one (#533) scoped to the JSON serialiser: * **Rule 1 -- no compute-module imports.** Nothing may be imported from the compute trees -- `keel.strategy.*` (sizing/backtest/promotion math), `keel.execution.guards` / @@ -42,6 +43,20 @@ (`keel.commands.update`), which this pin does not scan -- so the rule needs no allowance at all, and a console module that inlines any of that orchestration fails here rather than shipping a second, unpinned place that replaces the running binary. +* **Rule 6 -- the serialisation contract** (issue #533), scoped to the module that turns the + frozen reports into the browser's JSON (`SERIALISER_STEMS`). That module is the one place in + this layer whose output is a machine-readable MONEY contract rather than a screen, so it + carries one extra pin over the five ways the `Decimal`-only guarantee dies at that boundary: + `Decimal.normalize()` (which renders `Decimal("50")` as `Decimal("5E+1")` -- a form that has + reached the wire in this codebase before and broken real orders), `float()` on anything but a + timestamp, `round()`, `json.dumps(..., default=float)` (one keyword, and it looks like a + helpful fix for the `TypeError` a `Decimal` raises), and `len()` -- the quiet way a serialiser + starts producing counts of its own instead of reading ones the report holds. Rules 1-5 already + cover the serialiser as a member of `keel/web/`; this covers what is specific to it. The + runtime half of the same contract -- a recursive walk over the real payload asserting that no + JSON number appears anywhere in it -- lives in `tests/web/test_payload.py`. An AST rule and a + walk over the output fail for different reasons and neither subsumes the other, which is why + both exist. The allowlists are deliberately entry-scoped (module + enclosing function + callee), so an allowance cannot leak to a new call site: the same callee at a different place, or a @@ -63,7 +78,7 @@ #: The PRESENTATION layer: the console shell, the live loop, every sub-menu module -- and, since #: #435, every module of the local web UI (`keel/web/`). #: -#: The web UI is scanned by the same five rules rather than a parallel pin of its own, because it +#: The web UI is scanned by the same rules rather than a parallel pin of its own, because it #: is the same kind of thing: a second front-end over `keel/commands/*`. A separate pin would have #: drifted -- two files stating the same architecture, diverging one allowance at a time -- and #: the failure it is guarding against is identical in both. `keel/commands/serve.py` is NOT @@ -90,6 +105,29 @@ def _console_module_paths() -> list[str]: RULE5_IMPORT_ALLOWLIST: frozenset[tuple[str, str]] = frozenset({("server", "urllib.parse")}) +#: The SERIALISER: the module turning the frozen reports into the browser's JSON (#533). It is +#: already inside the scanned set (it lives under `keel/web/`), so Rules 1-5 apply to it as they +#: do to every other module of this layer. Rule 6 below is the extra, narrower pin it needs, +#: because it is the one module in the layer whose OUTPUT is a machine-readable money contract +#: rather than a screen. +SERIALISER_STEMS: frozenset[str] = frozenset({"payload"}) + + +#: Rule 6b's entry-scoped allowance, in the same (module, enclosing function, argument) shape as +#: every other allowance here. +#: +#: `float()` is banned in the serialiser because it is the one call that turns a `Decimal` into +#: the IEEE-754 double the whole contract exists to keep off the wire. It is allowed on a +#: TIMESTAMP, which is not money: `time.gmtime` takes a float and nothing else, and a UTC instant +#: has no cent to lose. Scoped to the argument NAME, so `float(price)` at the same call site +#: still fails. +RULE6_FLOAT_ALLOWLIST: frozenset[tuple[str, str, str]] = frozenset( + { + ("payload", "_gmt", "ts"), + } +) + + #: The compute trees -- where sizing/backtest/gate/screening/reporting math lives. The #: console layer reaches these ONLY through the `keel.commands.*` service layer (or the #: audited allowances below). @@ -430,6 +468,102 @@ def _inside_service_lambda(node: ast.AST) -> bool: "console layer never shells out, opens a socket or replaces " "its own process; that is keel.commands.update's job", ) + + # Rule 6 (#533): the serialisation contract, in the ONE module whose output is a + # machine-readable money contract rather than a screen. + if stem in SERIALISER_STEMS: + for key, messages in _rule6_findings(stem, tree, aliases, owners).items(): + for message in messages: + found.add(key, message) + return found + + +def _argument_identifier(node: ast.expr) -> str: + """The last identifier of a call argument -- `ts` for `ts`, `x.ts` and `self.x.ts`. + + Used only to scope Rule 6b's allowance to the ARGUMENT, so `float(ts)` can be allowed at a + site where `float(price)` still fails.""" + if isinstance(node, ast.Name): + return node.id + if isinstance(node, ast.Attribute): + return node.attr + return "" + + +def _rule6_findings( + stem: str, tree: ast.Module, aliases: dict[str, str], owners: dict[ast.AST, str] +) -> dict[str, list[str]]: + """The four ways the `Decimal`-only guarantee dies at the JSON boundary, as an AST scan. + + Written as its own function rather than inline in the fixture so the positive control below + can run it over a synthetic module and prove it is capable of finding anything at all.""" + found = _Findings() + for node in ast.walk(tree): + if not isinstance(node, ast.Call): + continue + owner = owners.get(node, "") + name = _dotted_call_name(node.func, aliases) + short = name.rsplit(".", 1)[-1] if name else None + + # 6a -- normalize(). THE hazard: `Decimal("50").normalize()` is `Decimal("5E+1")`, and + # "5E+1" on the wire is fifty read as five, or as nothing. Matched on the ATTRIBUTE, not + # on a resolved dotted name, because the receiver is a value and never resolves. + if isinstance(node.func, ast.Attribute) and node.func.attr == "normalize": + found.add( + "rule6_serialisation", + f"{stem}:{owner}: calls .normalize() (line {node.lineno}) -- it renders " + 'Decimal("50") as Decimal("5E+1"); format(value, "f") is the only rendering ' + "that cannot emit an exponent", + ) + + # 6b -- float(), the one call that turns a Decimal into the IEEE-754 double this whole + # contract exists to keep off the wire. Allowed on a timestamp, entry-scoped. + if short == "float" and name == "float": + argument = _argument_identifier(node.args[0]) if node.args else "" + if (stem, owner, argument) not in RULE6_FLOAT_ALLOWLIST: + found.add( + "rule6_serialisation", + f"{stem}:{owner}: calls float({argument}) (line {node.lineno}) -- money " + "crosses the wire as a string; only a timestamp may become a float here", + ) + + # 6c -- round(), which both computes and (on a float) reintroduces binary rounding. + # Rounding for DISPLAY is done with a format spec, which leaves the wire value exact. + if name == "round": + found.add( + "rule6_serialisation", + f"{stem}:{owner}: calls round() (line {node.lineno}) -- display rounding " + "belongs in a format spec, which leaves the wire value exact", + ) + + # 6d -- json.dumps(..., default=float). `json.dumps` raises a TypeError on a Decimal, + # and `default=float` is the one-keyword fix that silently converts every money value in + # the payload to a double. A PLAIN `json.dumps` is fine and is used: the serialiser + # normalises every leaf to a string first, so it needs no encoder. + if name in ("json.dumps", "json.dump"): + for keyword in node.keywords: + if keyword.arg in ("default", "cls"): + found.add( + "rule6_serialisation", + f"{stem}:{owner}: json.dumps(..., {keyword.arg}=...) (line " + f"{node.lineno}) -- a custom encoder is how every Decimal in the " + "payload becomes a double in one keyword", + ) + + # 6e -- len(). A count on the wire must be one the REPORT holds, and `len()` is how a + # serialiser quietly starts producing its own. The first draft of `journal_payload` + # shipped `count(len(report.entries))`; it is numerically exact, which is precisely why + # the runtime "computes nothing" guard could not be relied on to catch it -- that guard + # compares figures, and a list length can coincide with one the report already holds. + # An AST ban does not care about the coincidence. `JournalReport.shown_count` is where + # that figure belongs, and any future count needs the same treatment upstream (or an + # entry-scoped allowance here, in RULE6_FLOAT_ALLOWLIST's shape, with its reasoning). + if name == "len": + found.add( + "rule6_serialisation", + f"{stem}:{owner}: calls len() (line {node.lineno}) -- a count on the wire " + "must be one the report already holds; add it to the report builder", + ) return found @@ -476,6 +610,59 @@ def test_rule_5_no_process_or_network_orchestration_in_the_console_layer( assert not findings.get("rule5_orchestration"), findings["rule5_orchestration"] +def test_rule_6_the_serialisation_contract_holds_in_the_serialiser( + findings: dict[str, list[str]], +) -> None: + """Issue #533's rule: the module that writes keel's JSON never converts a money value to a + binary float, never rounds one, never renders one through `normalize()`, and never installs a + `json` encoder that would do any of those for it. + + All four are the same failure with different spellings -- a `Decimal` arriving in a browser as + an IEEE-754 double -- and all four are SILENT, which is why they are pinned mechanically + rather than left to review. `tests/web/test_payload.py` pins the same contract from the other + end, by walking the rendered payload. + """ + assert not findings.get("rule6_serialisation"), findings["rule6_serialisation"] + + +def test_rule_6_is_proven_false_capable() -> None: + """Rule 6's own positive control, run over a synthetic module that commits all four sins. + + Without this, a typo in an attribute name or a resolver that never matches would leave Rule 6 + green over a serialiser doing exactly what it forbids -- and Rule 6 guards a contract seven + downstream issues inherit, so a vacuously-green version of it is worse than none. + + The allowed shape is included too: `float(ts)` inside `_gmt` must NOT be flagged, or the rule + is a blanket ban wearing an allowlist. + """ + snippet = ( + "import json\n" + "def _gmt(ts, fmt):\n" + " return float(ts)\n" + "def bad(price, payload, rows):\n" + " a = price.normalize()\n" + " b = float(price)\n" + " c = round(price, 2)\n" + " d = json.dumps(payload, default=float)\n" + " e = len(rows)\n" + " return a, b, c, d, e\n" + ) + tree = ast.parse(snippet) + owners = _enclosing_functions(tree) + + messages = _rule6_findings("payload", tree, _collect_aliases(tree), owners).get( + "rule6_serialisation", [] + ) + + assert len(messages) == 5, messages + assert any(".normalize()" in m for m in messages) + assert any("float(price)" in m for m in messages) + assert any("round()" in m for m in messages) + assert any("default=" in m for m in messages) + assert any("len()" in m for m in messages) + assert not any(":_gmt:" in m for m in messages), messages + + def test_rule_5_is_proven_false_capable() -> None: """The detector's own positive control: a synthetic module that shells out and execv's is flagged (an AST scan that silently matched nothing would make Rule 5 @@ -505,6 +692,10 @@ def test_the_scan_actually_scanned_the_console_layer() -> None: # Named explicitly so that deleting or renaming a web module fails HERE, loudly, rather than # quietly shrinking the scanned set and leaving the rules green over less code. assert {"render", "security", "server"} <= stems + # #533: the JSON serialiser joins them. Named here for the same reason and with an extra one: + # Rule 6 is scoped BY STEM, so a serialiser renamed out of `SERIALISER_STEMS` would keep + # passing Rules 1-5 while silently losing the money-contract pin entirely. + assert SERIALISER_STEMS <= stems assert any(os.path.join("keel", "web") in path for path in paths) assert { "compliance_console", diff --git a/tests/web/test_payload.py b/tests/web/test_payload.py new file mode 100644 index 0000000..ec6be35 --- /dev/null +++ b/tests/web/test_payload.py @@ -0,0 +1,928 @@ +"""The serialisation contract (#533) -- the rule every endpoint in the web-UI milestone inherits. + +These tests exist because the failure they guard against is SILENT. A `Decimal` that becomes a +JSON number does not raise, does not warn, and looks right for every value a developer happens to +try by hand: `0.1 + 0.2` only misbehaves at the seventeenth digit, and a notional only loses a +cent once it is large enough that nobody is checking. The contract is therefore pinned +mechanically -- a recursive walk over the real payload -- rather than by review. + +Three properties are pinned, in the order the issue states them: + +* **Rule 1** -- money crosses the wire as a STRING. `test_no_wire_value_is_ever_a_json_number` + walks the parsed payload and fails on any `int`/`float` anywhere in it, which is a strictly + stronger statement than "no monetary field is a number" and needs no per-field judgement about + which fields are monetary. `test_the_number_walker_is_proven_false_capable` is its positive + control: a walker that silently matched nothing would make the whole milestone vacuously safe. +* **Rule 2** -- values arrive presentation-ready. + `test_the_serialiser_computes_nothing_every_wire_figure_came_from_the_report` proves the + serialiser reads rather than derives: every `Decimal`-parseable value on the wire must equal a + `Decimal`/`int` the report builder already put on the report. +* **Rule 3** -- semantic state is an explicit field. The `state` tests pin that a loss is + labelled `"bad"` in Python and that the glyph in `display` carries the same distinction without + colour (#532's non-colour signal). + +Plus the hazard that has already cost this codebase real orders: `Decimal.normalize()` renders +`Decimal("50")` as `Decimal("5E+1")`, and `"5E+1"` on the wire is a number no `Decimal(...)` +constructor on the far side will read as fifty in a form a human recognises. +`test_scientific_notation_never_reaches_the_wire` feeds the serialiser the exact shapes that +produce it. +""" + +from __future__ import annotations + +import json +from decimal import Decimal +from typing import Any + +import pytest + +from keel.commands.activity import ActivityCycle, ActivityEvent, ActivityFeed +from keel.commands.insights import ( + AccountSummary, + GateDistance, + InsightsReport, + JournalEntry, + JournalReport, + RuleTrackRecord, +) +from keel.commands.status import ( + AutonomyStatus, + MarketSessionStatus, + OpenPositionStatus, + ProductFreshness, + RuleSummary, + StatusReport, + SubscriptionStatusRow, + WithdrawalAttestationStatus, +) +from keel.web import payload + +NOW_TS = 1_756_000_000 + + +# -- fixtures: real report dataclasses, populated ------------------------------------------------ + + +def _status_report(**overrides: Any) -> StatusReport: + """A fully populated `StatusReport` -- every list non-empty, so the recursive walkers below + actually reach every branch of the payload instead of passing over empty collections.""" + base: dict[str, Any] = dict( + now_ts=NOW_TS, + mode="paper", + kill_switch_engaged=False, + autonomy=AutonomyStatus( + live=True, + autonomous=True, + autonomous_until=NOW_TS + 3600, + updated_ts=NOW_TS - 60, + profile_readable=True, + ), + equity_state_mode="paper", + high_water_mark=Decimal("12345.67"), + drawdown_total_pct=Decimal("0.05"), + drawdown_weekly_pct=Decimal("0.01"), + max_total_dd_pct=Decimal("0.20"), + max_weekly_dd_pct=Decimal("0.08"), + rail11_status="ok", + withdrawal_attestation=WithdrawalAttestationStatus( + state="attested", + enabled=True, + attested_at=NOW_TS - 86400, + expires_in_sec=6 * 86400, + expired_for_sec=None, + ), + paper_cash_usdc=Decimal("955.25"), + open_positions=[ + OpenPositionStatus( + id=7, + product_id="BTC-USD", + rule_name="turtle_breakout", + qty=Decimal("0.01000000"), + entry_price=Decimal("50000.00"), + opened_at=NOW_TS - 7200, + has_bracket=True, + ) + ], + rule_counts={"live": 1, "paper": 2}, + live_rules=[ + RuleSummary( + id=3, kind="dca", status="live", product_id="ETH-USD", params={"budget_usd": 50} + ) + ], + data_freshness=[ + ProductFreshness( + product_id="BTC-USD", granularity="ONE_HOUR", last_ts=NOW_TS - 300, age_sec=300 + ) + ], + subscriptions=[ + SubscriptionStatusRow( + venue="coinbase", + tier_name="advanced", + pacing="steady", + stored_status="active", + effective_status="active", + effective_cap=Decimal("2500.00"), + ) + ], + market_session=MarketSessionStatus(state="open", recorded_ts=NOW_TS - 120, defused=False), + ) + base.update(overrides) + return StatusReport(**base) + + +def _insights_report(**overrides: Any) -> InsightsReport: + base: dict[str, Any] = dict( + now_ts=NOW_TS, + account=AccountSummary( + mode="paper", + equity_state_mode="paper", + high_water_mark=Decimal("12345.67"), + drawdown_total_pct=Decimal("0.05"), + drawdown_weekly_pct=Decimal("0.01"), + max_total_dd_pct=Decimal("0.20"), + max_weekly_dd_pct=Decimal("0.08"), + rail11_status="ok", + paper_cash_usdc=Decimal("955.25"), + ), + rules=[ + RuleTrackRecord( + rule_name="turtle_breakout", + status="paper", + promotion_class="trend", + n_trades=12, + win_rate=41.5, + avg_win=Decimal("31.20"), + avg_loss=Decimal("-14.05"), + realized_rr=Decimal("2.22"), + expectancy=Decimal("0.8410"), + profit_factor=Decimal("1.57"), + max_drawdown=Decimal("-88.40"), + significant=False, + gate=GateDistance( + rule_name="turtle_breakout", + promotion_class="trend", + n_trades=12, + min_trades=30, + trades_remaining=18, + win_rate=41.5, + min_win_rate=40.0, + realized_rr=Decimal("2.22"), + min_rr=Decimal("2.0"), + expectancy=Decimal("0.8410"), + min_expectancy=Decimal("0.5"), + passing=False, + blocking_reasons=["n_trades 12 < 30"], + ), + ) + ], + closed_trade_count=12, + ) + base.update(overrides) + return InsightsReport(**base) + + +def _journal_report(**overrides: Any) -> JournalReport: + base: dict[str, Any] = dict( + now_ts=NOW_TS, + mode="paper", + entries=[ + JournalEntry( + closed_at=NOW_TS - 3600, + opened_at=NOW_TS - 86400, + rule_name="turtle_breakout", + product_id="BTC-USD", + qty=Decimal("0.01000000"), + entry_fill=Decimal("50000.00"), + exit_fill=Decimal("48766.00"), + pnl_net=Decimal("-12.34"), + fees=Decimal("0.6150"), + r_multiple=Decimal("-0.82"), + is_dca=False, + outcome="loss", + ), + JournalEntry( + closed_at=NOW_TS - 1800, + opened_at=NOW_TS - 90000, + rule_name="dca", + product_id="ETH-USD", + qty=Decimal("0.50000000"), + entry_fill=Decimal("2000.00"), + exit_fill=None, + pnl_net=Decimal("41.00"), + fees=None, + r_multiple=None, + is_dca=True, + outcome="dca", + ), + ], + # Deliberately NOT `len(entries)`. `total_count` is the pre-`--limit` count, so a real + # report almost always has more of them than it carries entries -- and while these two + # were equal, `test_the_serialiser_computes_nothing...` passed over a `shown_count` the + # serialiser had measured with `len()` rather than read, because `Decimal(2)` happened to + # be on the report already. A guard that passes on a coincidence is not a guard. + total_count=812, + filters={ + "rule": None, + "asset": None, + "since_ts": NOW_TS - 604800, + "until_ts": None, + "limit": 50, + "include_open": False, + }, + ) + base.update(overrides) + return JournalReport(**base) + + +def _activity_feed(**overrides: Any) -> ActivityFeed: + cycle = ActivityCycle( + cycle_id="c-1", + started_ts=float(NOW_TS - 600), + ended_ts=float(NOW_TS - 598), + mode="paper", + products=("BTC-USD",), + rules=("turtle_breakout",), + signals=1, + blocked=1, + entered=0, + exited=0, + errors=2, + highlights=("rail 11 blocked an entry",), + events=( + ActivityEvent( + ts=float(NOW_TS - 599), + level="INFO", + event="cycle_start", + cycle_id="c-1", + fields={ + "products": 1, + "budget_usd": Decimal("50.00"), + "dry_run": True, + # Structured values: a log line is parsed JSON, so a field can be a list or + # an object. `str()` would render these as Python reprs. + "symbols": ["BTC-USD", "ETH-USD"], + "limits": {"max": 1e50, "min": None, "on": False}, + }, + ), + ), + events_dropped=3, + ) + base: dict[str, Any] = dict( + status="ok", + source="/tmp/keel.log", + cycles=(cycle,), + detail=None, + lines_read=120, + lines_skipped=2, + window_truncated=True, + cycles_dropped=1, + scope="today", + scope_start_ts=float(NOW_TS - 50000), + now_ts=float(NOW_TS), + cycles_out_of_scope=4, + last_cycle_before_scope=cycle, + scope_fully_covered=False, + ) + base.update(overrides) + return ActivityFeed(**base) + + +def _every_payload() -> dict[str, Any]: + """All four payload builders at once. The guards below run over the whole surface, because a + contract that holds for `status` and leaks on `journal` is not a contract.""" + return { + "status": payload.status_payload(_status_report()), + "insights": payload.insights_payload(_insights_report()), + "journal": payload.journal_payload(_journal_report()), + "activity": payload.activity_payload(_activity_feed()), + } + + +# -- recursive walkers --------------------------------------------------------------------------- + + +def _walk(node: Any, path: str = "$") -> list[tuple[str, Any]]: + """Every leaf in a parsed JSON document, with the path that reaches it. Written here rather + than imported so the guard cannot be weakened by a change to production code.""" + if isinstance(node, dict): + out: list[tuple[str, Any]] = [] + for key, value in node.items(): + out.extend(_walk(value, f"{path}.{key}")) + return out + if isinstance(node, list): + out = [] + for index, value in enumerate(node): + out.extend(_walk(value, f"{path}[{index}]")) + return out + return [(path, node)] + + +def _json_numbers(document: Any) -> list[str]: + """Paths whose leaf is a JSON number. `bool` is excluded FIRST and deliberately: Python's + `bool` is a subclass of `int`, so an `isinstance(x, int)` test alone would flag every JSON + `true` as a number and make this guard fail for the wrong reason.""" + return [ + path + for path, leaf in _walk(document) + if not isinstance(leaf, bool) and isinstance(leaf, (int, float)) + ] + + +# -- Rule 1: money crosses the wire as a string --------------------------------------------------- + + +def test_no_wire_value_is_ever_a_json_number() -> None: + """THE guard the whole milestone rests on. + + Stated as "no JSON numbers ANYWHERE" rather than "no monetary field is a number", because the + second form needs a list of which fields are monetary -- and that list is exactly the thing + that rots when a new field lands. The strong form needs no list and cannot rot. + + The spec's one reserved exception is the `sort` field (ordering only, never displayed, never + summed), which #533 explicitly does not ship. If it is ever added, THIS is where its allowance + goes, named and scoped, so that adding it is a decision rather than an accident. + """ + document = json.loads(json.dumps(_every_payload())) + + assert _json_numbers(document) == [] + + +def test_the_number_walker_is_proven_false_capable() -> None: + """The guard's own positive control. An `_json_numbers` that matched nothing -- a typo in the + isinstance, a walker that stops at the first list -- would make the test above green over a + payload full of doubles. So: plant one at depth, under a list, and require it to be found; + and require a JSON `true` NOT to be found, since `bool` is an `int` in Python.""" + planted = {"a": [{"b": {"c": 0.1}}], "flag": True, "text": "0.1"} + + assert _json_numbers(planted) == ["$.a[0].b.c"] + + +def test_a_decimal_round_trips_through_the_wire_string_without_loss() -> None: + """Rule 1's actual point: the string form is not merely "not a float", it is EXACT. + + Every value below is one `float` would corrupt or one that has bitten this codebase before -- + a quantity at Coinbase's eight-decimal base increment, a notional past the 17 significant + digits a double carries, and the `Decimal("50")` whose `normalize()` form is `5E+1`. + """ + hazards = [ + Decimal("0.1"), + Decimal("0.01000000"), + Decimal("50"), + Decimal("50.00"), + Decimal("12345.67"), + Decimal("-12.34"), + Decimal("0.00000001"), + Decimal("123456789012345678901234567890.123456789"), + Decimal("5E+1"), + Decimal("1E-9"), + Decimal("-0.000000000000000001"), + ] + + for source in hazards: + wire = payload.money(source)["value"] + assert Decimal(wire) == source, (source, wire) + + +def test_the_wire_value_is_decimal_parseable_and_the_display_string_is_not() -> None: + """The two strings do different jobs and must not be conflated. `value` is machine input -- + no grouping separators, no currency symbol, no glyph, so `Decimal(value)` just works. + `display` is the human's, and carries all of that.""" + field = payload.money(Decimal("12345.67")) + + assert field["value"] == "12345.67" + assert Decimal(field["value"]) == Decimal("12345.67") + assert field["display"] == "$12,345.67" + + +def test_scientific_notation_never_reaches_the_wire() -> None: + """The `5E+1` hazard, at the level of the whole payload rather than one helper. + + `Decimal.normalize()` turns `Decimal("50")` into `Decimal("5E+1")`, and `str()` on any Decimal + with a positive exponent does the same. A client reading `"5E+1"` as a price has read fifty as + five, or as nothing at all. Every string leaf of every payload is checked, not just the ones + the author remembered were money. + """ + report = _status_report( + high_water_mark=Decimal("5E+1"), + paper_cash_usdc=Decimal("1E-9"), + drawdown_total_pct=Decimal("2E-3"), + ) + document = json.loads(json.dumps(payload.status_payload(report))) + + offenders = [ + (path, leaf) + for path, leaf in _walk(document) + if isinstance(leaf, str) and ("E+" in leaf or "E-" in leaf or "e+" in leaf) + ] + + assert offenders == [] + assert payload.money(Decimal("5E+1"))["value"] == "50" + assert payload.money(Decimal("1E-9"), places=9)["display"] == "$0.000000001" + + +def test_scientific_notation_cannot_reach_the_wire_through_an_open_ended_field_either() -> None: + """The same hazard by the other road. + + The test above only feeds Decimals through NAMED report fields, so it can say nothing about + `ActivityEvent.fields` and `JournalReport.filters` -- the two places a value of unconstrained + type crosses. Those took a `str(value)` fallback in the first draft, and `str(1e+50)` is + `"1e+50"`: an exponent on the wire that every `Decimal` precaution in the module was + irrelevant to, because no `Decimal` was involved. + """ + hazards = [1e50, 1e-30, Decimal("5E+1"), [1e50], {"deep": [{"deeper": 1e-30}]}] + + for value in hazards: + rendered = payload.stringify(value) + assert "e+" not in rendered and "e-" not in rendered, (value, rendered) + assert "E+" not in rendered and "E-" not in rendered, (value, rendered) + + +# -- Rule 2: the serialiser computes nothing ------------------------------------------------------ + + +def _report_figures(node: Any, seen: set[int] | None = None) -> set[Decimal]: + """Every `Decimal`/`int` the report builder already put on the report, as `Decimal`. + + Walks dataclasses, mappings and sequences generically rather than naming fields, so a field + added upstream is covered the day it appears instead of the day someone remembers to add it + here.""" + seen = set() if seen is None else seen + if id(node) in seen: + return set() + seen.add(id(node)) + if isinstance(node, bool): + return set() + if isinstance(node, Decimal): + return {node} + if isinstance(node, int): + return {Decimal(node)} + if isinstance(node, str): + return set() + if isinstance(node, dict): + out: set[Decimal] = set() + for key, value in node.items(): + out |= _report_figures(key, seen) | _report_figures(value, seen) + return out + if isinstance(node, (list, tuple, set, frozenset)): + out = set() + for item in node: + out |= _report_figures(item, seen) + return out + fields = getattr(node, "__dataclass_fields__", None) + if fields is not None: + out = set() + for name in fields: + out |= _report_figures(getattr(node, name), seen) + # PROPERTIES count as figures the report returns, exactly as fields do. A frozen report + # here derives some of its readings from its own data rather than storing them -- + # `ActivityCycle.is_quiet`/`.key` and `JournalReport.shown_count` are the established + # pattern -- and a stored duplicate of a derived reading is state that can drift out of + # agreement with what it describes. What matters for this guard is that the REPORT vouches + # for the figure, not which side of a `@property` it was written on. + for name, attribute in vars(type(node)).items(): + if not isinstance(attribute, property): + continue + try: + out |= _report_figures(getattr(node, name), seen) + except Exception: # pragma: no cover - a property that raises is not a figure + continue + return out + return set() + + +def test_the_serialiser_computes_nothing_every_wire_figure_came_from_the_report() -> None: + """Rule 2, enforced rather than asserted in prose. + + Every `Decimal`-parseable `value` on the wire must equal a figure the report ALREADY held. A + serialiser that multiplied `qty` by `entry_price` to offer the client a notional, or scaled a + drawdown fraction by 100 to make it read as a percentage, would produce a number that appears + nowhere on the report -- and would fail here, by name and path. + + `StatusReport` and `JournalReport` are used because every figure they carry is a `Decimal` or + an `int`; `InsightsReport` carries `float` win rates, which have no exact decimal form and are + covered by their own test below. + """ + for name, report, built in ( + ("status", _status_report(), payload.status_payload(_status_report())), + ("journal", _journal_report(), payload.journal_payload(_journal_report())), + ): + available = _report_figures(report) + for path, leaf in _walk(json.loads(json.dumps(built))): + if not path.endswith(".value") or not isinstance(leaf, str) or not leaf: + continue + try: + figure = Decimal(leaf) + except ArithmeticError: + continue # an ISO-8601 instant or an enum word, not a figure + except ValueError: + continue + assert figure in available, f"{name} {path}: {leaf!r} is on no report field" + + +def test_an_open_position_carries_no_notional_because_the_report_holds_none() -> None: + """The single most tempting place to break Rule 2, pinned so the temptation fails loudly. + + `qty * entry_price` is one multiplication away, and the spec's illustrative payload even shows + a `notional` on a position. But `OpenPositionStatus` (`keel/commands/status.py`) carries + `qty`, `entry_price`, `opened_at`, `rule_name` and `has_bracket` -- and NOT a notional, and + not a P&L. Serialising one would mean this layer computed a figure the trading rails never + saw, which is precisely the invariant `test_console_thinness.py` exists to keep checkable. + + If a notional is wanted, it is added to `gather_status`, where the rails can see it. Then this + test changes -- deliberately, in a commit that says so. + """ + position = payload.status_payload(_status_report())["open_positions"][0] + + assert "notional" not in position + assert "pnl" not in position + assert position["qty"]["value"] == "0.01000000" + assert position["entry_price"]["value"] == "50000.00" + + +def test_a_drawdown_fraction_is_not_rescaled_into_a_percentage() -> None: + """`drawdown_total_pct` is named for a percentage but is a FRACTION: `_rail11_status` compares + it against `max_total_dd_pct=Decimal("0.20")`, so `0.05` means five percent, not five basis + points. Multiplying by 100 to make the label true would be arithmetic in the serialiser -- so + the value crosses unchanged and the display says what it is. + + (`keel/web/render.py:150` renders this same field through `pct()`, which appends a `%` to the + raw fraction and therefore prints "0.05%" for a 5% drawdown. The contract does not inherit + that; fixing the HTML renderer is a separate change against a separate surface.) + """ + built = payload.status_payload(_status_report()) + + assert built["drawdown"]["total"]["value"] == "0.05" + assert "%" not in built["drawdown"]["total"]["display"] + assert built["drawdown"]["max_total"]["value"] == "0.20" + + +def test_a_win_rate_float_is_re_encoded_not_recomputed() -> None: + """`RuleTrackRecord.win_rate` is a `float` upstream -- a statistic, never money. It reaches the + wire through its own shortest round-trip repr, so the figure on the wire is the figure the + report held and nothing was recomputed on the way.""" + built = payload.insights_payload(_insights_report()) + + assert built["rules"][0]["win_rate"]["value"] == "41.5" + assert built["rules"][0]["win_rate"]["display"] == "41.5%" + + +# -- Rule 3: semantic state is a field, never an inference ---------------------------------------- + + +def test_a_loss_is_labelled_bad_in_python_not_by_the_sign_on_the_wire() -> None: + """Rule 3. The client must never read a minus sign and conclude "bad" -- that is arithmetic by + another name, and it relocates a trading judgement into a browser.""" + entries = payload.journal_payload(_journal_report())["entries"] + loss, gain = entries[0], entries[1] + + assert loss["pnl"]["state"] == "bad" + assert loss["pnl"]["value"] == "-12.34" + assert gain["pnl"]["state"] == "good" + + +def test_state_survives_without_colour() -> None: + """#532: colour must never be the only signal. `display` carries a glyph and an explicit sign, + so profit and loss stay distinguishable in greyscale, on e-ink, in sunlight, and to the + roughly one man in twelve who cannot separate the palette's red from its green.""" + entries = payload.journal_payload(_journal_report())["entries"] + + assert entries[0]["pnl"]["display"] == "▼ −$12.34" + assert entries[1]["pnl"]["display"] == "▲ +$41.00" + + +def test_the_displayed_sign_and_the_state_never_disagree() -> None: + """Rule 3's premise, checked rather than assumed: if `display` shows a minus, `state` must not + be calling the same value neutral. + + It failed on negative zero. `format(Decimal("-0.00"), ",.2f")` is `"-0.00"`, so a sign read + off the formatted TEXT said negative while `state`, read off the exact value, said neutral -- + `Decimal("-0.00") == 0`. A client styling by `state` and showing `display` printed a minus + sign in a neutral colour. There is now one reading of the sign, `_is_negative`, feeding both. + + A value that is genuinely negative but rounds to zero at the display precision keeps its + minus, and keeps `bad` alongside it. That is not a disagreement: it is a small loss, honestly + labelled and honestly styled. + """ + cases = [ + Decimal("-0.00"), + Decimal("0.00"), + Decimal("-0.001"), + Decimal("-12.34"), + Decimal("41.00"), + Decimal("0E-8"), + ] + + for value in cases: + field = payload.money(value, signed=True) + shows_negative = payload.MINUS in field["display"] or payload.DOWN in field["display"] + assert shows_negative == (field["state"] == "bad"), (value, field) + + unsigned = payload.money(value) + assert (payload.MINUS in unsigned["display"]) == (value < 0), (value, unsigned) + + +def test_negative_zero_is_not_negative_anywhere_it_is_rendered() -> None: + """`Decimal("-0.00")` is zero, and every builder that renders a sign must agree.""" + assert payload.money(Decimal("-0.00"))["display"] == "$0.00" + assert payload.money(Decimal("-0.00"), signed=True)["state"] == "neutral" + assert payload.percent(Decimal("-0.00"))["display"] == "0.00%" + assert payload.ratio(Decimal("-0.00"))["display"] == "0.00" + assert payload.quantity(Decimal("-0.00"))["display"] == "0" + # ...and the exact value still crosses untouched: nothing was normalised away to achieve it. + assert payload.money(Decimal("-0.00"))["value"] == "-0.00" + + +def test_a_loss_too_small_to_show_keeps_its_sign_and_its_judgement() -> None: + """The other side of the same rule. `-0.001` at two places renders `0.00`, and suppressing the + minus there would report a loss as a break-even. It shows, and `state` agrees.""" + field = payload.money(Decimal("-0.001"), signed=True) + + assert field["display"] == "▼ −$0.00" + assert field["state"] == "bad" + assert field["value"] == "-0.001" + + +def test_shown_count_is_read_from_the_report_not_measured_here() -> None: + """The count of rows a journal page carries is `JournalReport.shown_count`, a reading the + report makes about itself -- not `len(entries)` measured by the serialiser. + + The distinction is invisible in the number (a list length is exact) and that is exactly the + problem: the runtime "computes nothing" guard compares FIGURES, so it cannot tell a measured + length from a read one when they coincide. Rule 6e of `test_console_thinness.py` bans `len()` + in the serialiser for that reason, and this pins the pair a client needs to say "2 of 812" + without subtracting anything. + """ + built = payload.journal_payload(_journal_report()) + + assert built["total_count"]["value"] == "812" + assert built["total_count"]["display"] == "812" + assert built["shown_count"]["value"] == "2" + + +def test_every_field_carries_all_three_keys_always() -> None: + """Schema uniformity is itself part of the contract: a client that has to test whether `state` + is present is branching on payload SHAPE, which is inference by another route. Every field + object has `value`, `display` and `state`, in every payload, for every value including + absent ones.""" + for name, built in _every_payload().items(): + leaves = _walk(built) + by_parent: dict[str, set[str]] = {} + for path, _leaf in leaves: + parent, _, key = path.rpartition(".") + by_parent.setdefault(parent, set()).add(key) + for parent, keys in by_parent.items(): + if "value" in keys: + assert keys == {"value", "display", "state"}, f"{name} {parent}: {sorted(keys)}" + + +def test_every_state_word_is_one_the_contract_declares() -> None: + """A free-text `state` would push the client back into guessing. The vocabulary is closed, and + #532's palette and glyph table key off exactly these words.""" + for name, built in _every_payload().items(): + for path, leaf in _walk(built): + if path.endswith(".state"): + assert leaf in payload.STATES, f"{name} {path}: {leaf!r}" + + +def test_absence_is_unknown_and_never_a_zero() -> None: + """`None` means "not recorded"; `0` means "recorded as zero". Collapsing the first into the + second is the shape of the always-passing fee rail (#198) -- a missing number that reads as a + real one. On the wire, absence is an empty `value`, a dash, and `state: "unknown"`.""" + absent = payload.money(None) + + assert absent == {"value": "", "display": "—", "state": "unknown"} + assert payload.money(Decimal("0.00"))["value"] == "0.00" + assert payload.money(Decimal("0.00"))["state"] == "neutral" + + +def test_a_kill_switch_carries_its_own_judgement() -> None: + """A boolean the operator must act on is not a bare `true`: it arrives with the word to show + and the state to style it by.""" + engaged = payload.status_payload(_status_report(kill_switch_engaged=True))["kill_switch"] + clear = payload.status_payload(_status_report(kill_switch_engaged=False))["kill_switch"] + + assert engaged["display"] == "ENGAGED" + assert engaged["state"] == "bad" + assert clear["state"] == "good" + + +def test_an_unknown_rail_state_is_unknown_and_not_quietly_good() -> None: + """`_rail11_status` returns `"unknown"` when a drawdown scalar was never written -- explicitly, + because reporting an unwritten value as a confident "ok" would be a lie. The contract keeps + that distinction: `unknown` is its own state, not a green one. + + This is why the classifier here is NOT `render._tone_for_rail`, which matches on the words + "breach"/"halt"/"warn" and falls through to `"good"` -- so it styles a rail nobody has measured + as if it had passed. + """ + halted = payload.status_payload(_status_report(rail11_status="HALTED")) + unknown = payload.status_payload(_status_report(rail11_status="unknown")) + ok = payload.status_payload(_status_report(rail11_status="ok")) + + assert halted["drawdown"]["rail11"]["state"] == "bad" + assert unknown["drawdown"]["rail11"]["state"] == "unknown" + assert ok["drawdown"]["rail11"]["state"] == "good" + + +def test_an_expired_withdrawal_attestation_reads_as_bad() -> None: + """Rail 17's state is the one an operator must act on within a TTL; it arrives judged.""" + expired = payload.status_payload( + _status_report( + withdrawal_attestation=WithdrawalAttestationStatus( + state="expired", + enabled=True, + attested_at=NOW_TS - 12 * 86400, + expires_in_sec=None, + expired_for_sec=5 * 86400, + ) + ) + )["withdrawal_attestation"] + + assert expired["state"]["state"] == "bad" + # `_human_age`, exactly as `_rail17_line` prints it -- "EXPIRED 5d ago", not "5d". + assert expired["expired_for"]["display"] == "5d ago" + + +# -- presentation-ready: displays are built here, never derivable there --------------------------- + + +def test_a_quantity_shows_its_unit_and_drops_its_padding_without_normalize() -> None: + """`0.01000000 BTC` is how the ledger stores it and `0.01 BTC` is how a human reads it. The + trim is string work on an already-formatted decimal, NOT `Decimal.normalize()` -- normalize is + what turns `50` into `5E+1`.""" + assert payload.quantity(Decimal("0.01000000"), unit="BTC")["display"] == "0.01 BTC" + assert payload.quantity(Decimal("0.01000000"), unit="BTC")["value"] == "0.01000000" + assert payload.quantity(Decimal("50"), unit="BTC")["display"] == "50 BTC" + assert payload.quantity(Decimal("1234.5"), unit="ETH")["display"] == "1,234.5 ETH" + + +def test_counts_are_grouped_for_reading_and_exact_on_the_wire() -> None: + assert payload.count(1234567)["value"] == "1234567" + assert payload.count(1234567)["display"] == "1,234,567" + assert payload.count(None)["state"] == "unknown" + + +def test_an_instant_arrives_as_utc_in_both_forms() -> None: + """Every day boundary in keel is UTC. `value` is ISO-8601 with an explicit `Z` so a client can + hand it straight to `new Date(...)` without arithmetic; `display` is the same instant already + written out, so no client ever formats a date to show it in a table.""" + field = payload.moment(0) + + assert field["value"] == "1970-01-01T00:00:00Z" + assert field["display"] == "1970-01-01 00:00:00 UTC" + + +def test_a_broken_instant_degrades_instead_of_taking_the_payload_down() -> None: + """Log timestamps come from outside this process. A corrupt one renders as absent -- the same + choice `render.utc` already makes -- because a 500 on the whole page is a worse answer than a + dash in one cell.""" + for broken in (None, float("nan"), 1e30, float("inf")): + assert payload.moment(broken)["state"] == "unknown" + + +def test_a_duration_uses_the_cli_s_own_words() -> None: + """`_human_age`/`_human_remaining` live in `keel/commands/status.py` and are what `keel status` + prints. The web payload calls them rather than re-deriving the ladder, so the browser and the + terminal can never disagree about how old the same candle is.""" + assert payload.duration(300)["display"] == "5m ago" + assert payload.duration(6 * 86400, elapsed=False)["display"] == "6d" + assert payload.duration(300)["value"] == "300" + + +def test_the_activity_feed_stringifies_the_open_ended_event_fields() -> None: + """`ActivityEvent.fields` is deliberately open -- every `log_event` call site invents its own + kwargs, and the parser keeps them whole rather than projecting onto a fixed schema. That means + arbitrary ints and Decimals arrive here, and Rule 1 applies to them exactly as it does to a + known field: they cross as strings.""" + event = payload.activity_payload(_activity_feed())["cycles"][0]["events"][0] + + assert event["fields"]["products"] == "1" + assert event["fields"]["budget_usd"] == "50.00" + assert event["fields"]["dry_run"] == "true" + + +def test_a_nested_log_field_crosses_as_json_never_as_a_python_repr() -> None: + """A log line is parsed JSON, so a field's value can be a list or an object. + + The first draft fell through to `str(value)` for those, which produced `"{'a': 1, 'b': None}"` + -- single quotes, `None`, `True`. No JSON number leaks out of that (it is one string), so the + recursive number-walker stayed green while the payload carried text no client can parse and + none should display. Rule 2 says values arrive presentation-ready, and a Python repr is not. + + The numbers INSIDE a nested value go through the same rendering as a top-level one, which is + the half that actually bites: `str([1e+50])` is `"[1e+50]"` -- scientific notation reaching + the wire through a path no money field touches, so every `Decimal` precaution in the module + would have been irrelevant to it. + """ + fields = payload.activity_payload(_activity_feed())["cycles"][0]["events"][0]["fields"] + + assert fields["symbols"] == '["BTC-USD", "ETH-USD"]' + assert fields["limits"] == ( + '{"max": "100000000000000000000000000000000000000000000000000",' + ' "min": "", "on": "false"}' + ) + assert "1e+50" not in fields["limits"] + assert "'" not in fields["limits"] and "None" not in fields["limits"] + + +def test_a_pathologically_nested_log_field_cannot_recurse_the_response_to_death() -> None: + """Depth in a log line is attacker-adjacent input: `json.loads` cannot build a cycle, but it + can build a thousand nested lists, and a `RecursionError` while rendering one field would take + down a whole response for a row nobody needed. Log fields are shallow by construction -- each + is a `log_event` kwarg -- so the cap costs nothing real and the depth of the input never + decides how deep this process goes.""" + deep: Any = "bottom" + for _ in range(200): + deep = [deep] + + rendered = payload.stringify(deep) + + assert "(nested)" in rendered + assert "bottom" not in rendered + + +def test_the_journal_filters_cross_as_strings_too() -> None: + """`JournalReport.filters` holds the query that produced the report -- `limit`, `since_ts` and + friends, which are ints. They are echoed back for the client to show, and an echoed int is a + JSON number like any other.""" + filters = payload.journal_payload(_journal_report())["filters"] + + assert filters["limit"] == "50" + assert filters["since_ts"] == str(NOW_TS - 604800) + assert filters["rule"] == "" + + +def test_an_error_count_is_bad_and_a_quiet_cycle_is_neutral() -> None: + """A cycle in which the agent looked and nothing happened is a POSITIVE observation, not an + absence of one -- it is how the feed answers "is it alive". So quiet is `neutral`, never + `warn`; errors are what turn a row `bad`.""" + cycle = payload.activity_payload(_activity_feed())["cycles"][0] + + assert cycle["errors"]["state"] == "bad" + assert cycle["quiet"]["state"] == "neutral" + + +# -- totality ------------------------------------------------------------------------------------ + + +def test_every_builder_survives_a_completely_empty_report() -> None: + """A fresh install is the commonest state there is, and it is not an error. Every list empty, + every optional `None`, no exception, and the guards still hold.""" + empty_status = _status_report( + autonomy=AutonomyStatus( + live=False, + autonomous=False, + autonomous_until=None, + updated_ts=None, + profile_readable=False, + ), + equity_state_mode=None, + high_water_mark=None, + drawdown_total_pct=None, + drawdown_weekly_pct=None, + rail11_status="unknown", + withdrawal_attestation=WithdrawalAttestationStatus( + state="unattested", + enabled=None, + attested_at=None, + expires_in_sec=None, + expired_for_sec=None, + ), + paper_cash_usdc=None, + open_positions=[], + rule_counts={}, + live_rules=[], + data_freshness=[], + subscriptions=[], + market_session=MarketSessionStatus(state=None, recorded_ts=None), + ) + documents = { + "status": payload.status_payload(empty_status), + "insights": payload.insights_payload(_insights_report(rules=[], closed_trade_count=0)), + "journal": payload.journal_payload(_journal_report(entries=[], total_count=0, filters={})), + "activity": payload.activity_payload( + ActivityFeed(status="missing", source="/tmp/nope.log") + ), + } + + parsed = json.loads(json.dumps(documents)) + assert _json_numbers(parsed) == [] + + +@pytest.mark.parametrize( + "builder", + ["status_payload", "insights_payload", "journal_payload", "activity_payload"], +) +def test_every_payload_is_json_serialisable_without_a_custom_encoder(builder: str) -> None: + """The builders return plain `dict`/`list`/`str` -- no `Decimal` reaches `json.dumps`. That + matters beyond tidiness: `json.dumps(Decimal(...))` raises, and the natural fix a hurried + author reaches for is `default=float`, which is the contract's exact failure mode installed as + a convenience.""" + report = { + "status_payload": _status_report(), + "insights_payload": _insights_report(), + "journal_payload": _journal_report(), + "activity_payload": _activity_feed(), + }[builder] + + json.dumps(getattr(payload, builder)(report)) # no cls=, no default=