diff --git a/keel/agent.py b/keel/agent.py index 1aaf6f2..e7aa66e 100644 --- a/keel/agent.py +++ b/keel/agent.py @@ -415,6 +415,22 @@ def _position_state(repo: Repository, product_id: str) -> dict[str, Any] | None: return raw +@dataclass(frozen=True) +class _EquityParts: + """One read of the live account, split into the pieces `equity_points` records (#698). + + `equity` is the whole reading; `cash` and `unrealized` are two of its legs (the third, the + cost basis of open positions, is implied: `cash + cost + unrealized == equity`). They come + out of ONE pass over ONE set of balances and marks, which is the point -- reading the total + and the split separately would let a price tick between them and file a row that does not + reconcile. + """ + + equity: Decimal + cash: Decimal + unrealized: Decimal + + def _mark_to_market_equity( repo: Repository, broker: Any, @@ -425,6 +441,9 @@ def _mark_to_market_equity( """Quote balance + mark-to-market value of every open position, or `None` if the quote balance could not be read. + The scalar face of `_mark_to_market_parts`, kept because most callers want only the total. + Every word below describes that function's single pass; nothing is computed twice. + Unrealized P&L is included on purpose: a breaker that saw only realized P&L would read 0% while a position bled and would notice only after the loss was booked -- backwards for a circuit breaker, which has to fire WHILE you are losing. @@ -452,6 +471,27 @@ def _mark_to_market_equity( simply unknowable then, and a wrong one corrupts the high-water mark PERMANENTLY (an HWM never falls, so an under-read arms the breaker on a phantom drawdown from then on). """ + parts = _mark_to_market_parts(repo, broker, products, price_by_product, quote_currency) + return None if parts is None else parts.equity + + +def _mark_to_market_parts( + repo: Repository, + broker: Any, + products: list[str], + price_by_product: dict[str, Decimal], + quote_currency: str, +) -> _EquityParts | None: + """`_mark_to_market_equity`'s single pass, returning the split as well as the total (#698). + + See that function's docstring for every rule this implements -- the FX bound, the + cost-basis fallback, the product union, and why an unreadable balance is `None` rather than + a partial total. This is the same computation; it just keeps the legs it was already + computing instead of discarding them. + + `unrealized` follows `equity.unrealized_on_marks`: a position valued at cost (no fresh + price) contributes ZERO, because the equity in the same record valued it at cost too. + """ currencies: list[str] = [] scanned = (*products, *repo.held_products()) for candidate in (quote_currency, *(quote_currency_of(p) for p in scanned)): @@ -477,6 +517,7 @@ def _mark_to_market_equity( # is still open would otherwise drop that holding out of equity in a single step. valued: set[str] = set() total = quote + unrealized = Decimal("0") for product_id in (*products, *repo.held_products()): if product_id in valued: continue @@ -489,13 +530,17 @@ def _mark_to_market_equity( # A held product with no fresh price is valued at its cost basis rather than dropped: # dropping it understates equity and would trip rail 11 on a DATA GAP rather than on a # loss. `avg_cost` comes from the same audit log as `qty`, so the two always agree. - mark = price_by_product.get(product_id) - if mark is None: - mark = avg_cost + fresh = price_by_product.get(product_id) + mark = avg_cost if fresh is None else fresh if mark <= 0: continue total += qty * mark - return total + # Only a FRESH price books a gain or loss. On the fallback the mark IS the basis, so + # `qty * (mark - avg_cost)` would be zero anyway -- but stating it as a branch keeps the + # meaning ("nothing was observed") rather than leaning on the arithmetic. + if fresh is not None and fresh > 0: + unrealized += qty * (fresh - avg_cost) + return _EquityParts(equity=total, cash=quote, unrealized=unrealized) def _seed_paper_account_if_needed( @@ -1707,15 +1752,28 @@ def run_once( equity_mod.record_external_flow(repo, amount=contribution) repo.set_state("paper_last_contribution_month", month_start) equity_now = paper_trader.equity(latest_price_by_product) + # The same reading, split for the `equity_points` row (#698). `None` when the paper + # account is unseeded -- there is nothing to split -- and never derived from + # `equity_now`, which would invent the split rather than record it. + equity_cash = paper_trader.get_cash() + equity_unrealized = ( + None if equity_cash is None else paper_trader.unrealized(latest_price_by_product) + ) else: # Mirrors `_seed_paper_account_if_needed`'s clear: unconditional, at the top of the # branch, BEFORE the broker-equity read, so an unreadable broker on the first live # cycle after a paper->live flip can't skip the clear (see # `_clear_live_mode_if_needed`'s docstring). _clear_live_mode_if_needed(repo) - equity_now = _mark_to_market_equity( + # One pass, three numbers (#698): the total rail 11 reads, and the cash/unrealized + # legs the series records. Read together so a price cannot tick between them and + # file a row whose parts contradict its own total. + live_parts = _mark_to_market_parts( repo, broker, products, latest_price_by_product, config.quote_currency ) + equity_now = None if live_parts is None else live_parts.equity + equity_cash = None if live_parts is None else live_parts.cash + equity_unrealized = None if live_parts is None else live_parts.unrealized # Task 9: paper-forward observability -- the synthetic equity + drawdown scalars this # cycle advanced, surfaced on `LoopResult` (rendered by # `keel.commands.trading.render_loop_result` + this log line) instead @@ -1735,7 +1793,13 @@ def run_once( # The live-side mode clear now happens up-front in the live branch above (see # `_clear_live_mode_if_needed`), symmetrically with the paper-side clear in # `_seed_paper_account_if_needed`. Nothing left to stamp/clear here. - equity_mod.update_drawdown(repo, equity=equity_now, now_ts=now_ts) + equity_mod.update_drawdown( + repo, + equity=equity_now, + now_ts=now_ts, + cash=equity_cash, + unrealized=equity_unrealized, + ) if paper_trader is not None: result_paper_equity = equity_now diff --git a/keel/commands/insights.py b/keel/commands/insights.py index 1afc308..e6630a6 100644 --- a/keel/commands/insights.py +++ b/keel/commands/insights.py @@ -47,6 +47,7 @@ from typing import Any import click +from keel_core.types import EquityReading from keel import agent as agent_mod from keel.commands._common import DISCLAIMER, _load_cfg, _open_repo @@ -151,6 +152,16 @@ def shown_count(self) -> int: #: readable in view-source, which is the whole argument for this interface. _COORD = Decimal("0.01") +#: A plain arithmetic zero, deliberately NOT `_BASELINE`. +#: +#: `_BASELINE` is a claim about an axis -- "zero is always on this canvas" -- and it is +#: true of cumulative net P&L, where zero separates a rule that has made money from one +#: that has lost it. `build_equity_series` plots ACCOUNT EQUITY, which has no such line: +#: an account is not up or down against nothing. Forcing zero into that range would +#: squash a $10,000 account's real moves into a sliver at the top of a box that is +#: mostly empty space, so the two zeros are kept apart by name. +_ZERO = Decimal("0") + @dataclass(frozen=True) class EquityPoint: @@ -206,12 +217,245 @@ def point_count(self) -> int: return len(self.points) +@dataclass(frozen=True) +class EquitySeriesPoint: + """One cycle's account equity, and where it is drawn (#698). + + `equity`, `hwm` and `dd_floor` are the exact figures; `x` and `y` are plot coordinates in + the same `PLOT_WIDTH` x `PLOT_HEIGHT` box `EquityPoint` uses, with `y` growing DOWNWARD for + the reason documented there. + + `hwm_y` and `dd_floor_y` are the overlay coordinates for the same instant, so the ceiling + lines are drawn from the same pass and cannot drift from the point they belong to. + `dd_floor` is `None` when no rail setting was supplied -- an unknown ceiling, which is not + the same as a ceiling of zero. + """ + + ts: int + mode: str + equity: Decimal + cash: Decimal | None + unrealized: Decimal | None + hwm: Decimal + dd_floor: Decimal | None + x: Decimal + y: Decimal + hwm_y: Decimal + dd_floor_y: Decimal | None + + +@dataclass(frozen=True) +class EquitySeriesSegment: + """A run of consecutive readings in ONE mode -- one unbroken polyline. + + Segments exist because paper and live are different accounts sharing a database, and a line + drawn across the flip states a continuity that does not exist. A mode that resumes after a + flip is a NEW segment rather than a continuation of its earlier one: grouping by mode alone + would draw a line across the stretch the account spent somewhere else. + """ + + mode: str + points: list[EquitySeriesPoint] + + +@dataclass(frozen=True) +class EquitySeries: + """Account equity over TIME, as the agent marked it each cycle (#698). + + A different chart from `EquityCurve`, not a replacement for it. That one plots cumulative + net P&L over closed TRADES and argues -- correctly, for that quantity -- that its axis + should be trade order. This plots what the account was worth whether or not it traded, and + for that the gaps between cycles are information: a week the agent did not run is a week + with no readings, and it has to look like one. + + `low`/`high` are the axis bounds actually used and include the OVERLAYS, so a drawdown floor + is guaranteed to fit inside the box. A floor drawn off the bottom edge reads as absent, and + an absent rail ceiling is the one thing this chart must never imply. + + Empty is a real answer: a deployment that has not completed a cycle since the v19 upgrade + has no series, and `segments == []` says exactly that. There is no synthetic flat line, + because a flat line is what an account that did not move looks like. + """ + + segments: list[EquitySeriesSegment] + low: Decimal + high: Decimal + width: Decimal + height: Decimal + #: How many readings the TABLE holds, when the caller bounded its read and knows. `None` + #: means the caller did not say -- which is deliberately not the same as "nothing was left + #: out", because a series that asserted completeness on behalf of a caller that never + #: counted would be the chart lying about its own span. + total_recorded: int | None = None + + @property + def point_count(self) -> int: + """How many readings the series holds, across every segment. + + Derived rather than stored, for the reason `EquityCurve.point_count` is: a stored count + can drift from the list it describes, and `keel/web/payload.py` may not call `len()`. + """ + return sum(len(segment.points) for segment in self.segments) + + @property + def is_truncated(self) -> bool: + """Whether this series is a WINDOW onto a longer record. + + What the text equivalent turns on. A bounded read is honest only if it says so: a chart + that quietly begins wherever a row cap fell would misstate the span of the record while + every individual point remained true, which is the harder kind of wrong to notice. + + `False` when the total is unknown -- an unstated total is not a claim in either + direction, and inventing "complete" from silence is the failure this guards. + """ + return self.total_recorded is not None and self.total_recorded > self.point_count + + @property + def modes(self) -> list[str]: + """The distinct accounts this series spans, in the order they first appear. + + NOT the segment list: paper, live, paper is three segments and two modes. The segments + are what the chart draws; this is what a sentence about the chart names, and conflating + them would have a spoken summary announce the same account twice. + + It lives on the report for the reason `point_count` does -- `keel/web/payload.py` may + not call `len()` (Rule 6e), and the serialiser deriving its own mode list would be a + second answer to a question the report already answers. + """ + seen: list[str] = [] + for segment in self.segments: + if segment.mode not in seen: + seen.append(segment.mode) + return seen + + @property + def is_partitioned(self) -> bool: + """Whether this series spans more than one account. + + The thing the chart's text equivalent turns on: the sentence explaining that each mode + is drawn as its own line must be said only when there IS a split. Told to a deployment + that has only ever run paper, it sends a reader looking for a second line that is not + there -- the same false continuity the segments prevent, pointing the other way. + """ + return len(self.modes) > 1 + + def _plot_y(value: Decimal, *, low: Decimal, span: Decimal) -> Decimal: """`value` mapped into `0..PLOT_HEIGHT`, with the top of the box being `low + span`.""" fraction = (value - low) / span return (PLOT_HEIGHT - PLOT_HEIGHT * fraction).quantize(_COORD) +def build_equity_series( + readings: Sequence[EquityReading], + *, + max_total_dd_pct: Decimal | None = None, + total_recorded: int | None = None, +) -> EquitySeries: + """The account-equity series over `readings`, oldest first, ready to draw (#698). + + **The horizontal axis is TIME**, unlike `build_equity_curve`'s. The quantity is different: + that curve plots closed trades, where a quiet week is not an event, while this plots what + the account was worth every cycle -- and there, a week with no readings IS the event. Even + spacing would draw a gap in the record as an ordinary step between two cycles. + + **Readings are segmented by mode**, and by RUNS of it rather than by the mode set: paper, + live, paper is three segments. Paper and live are unrelated accounts that share a database + (they flip within one, which is why `agent._clear_live_mode_if_needed` wipes the shared + high-water mark), so a polyline crossing a flip asserts a continuity that does not exist -- + a $10k paper account followed by a $250 live one would draw a 97.5% collapse that never + happened. + + **`hwm` is read off each row, never recomputed.** It is not the running maximum of the + equity: `execution.equity.record_external_flow` REBASES it on a declared deposit so the + drawdown keeps measuring trading performance. The overlay's whole purpose is to show the + ceiling rail 11 actually had in force, so a recomputed maximum would be a different line + wearing its name. + + `max_total_dd_pct` is the rail's own setting (`config.money_mgmt.max_total_dd_pct`). Given + it, each point carries the equity at which rail 11 would start vetoing entries -- derived + from THAT point's high-water mark, so the floor moves with the rebase. Omitted, `dd_floor` + is `None`: the ceiling is unknown, which is not a ceiling of zero. + + Callers pass the readings they are displaying, so the chart and any table beside it can + never disagree about which cycles they describe. + """ + if not readings: + return EquitySeries( + segments=[], + low=_ZERO, + high=_ZERO, + width=PLOT_WIDTH, + height=PLOT_HEIGHT, + total_recorded=total_recorded, + ) + + floors = [ + None if max_total_dd_pct is None else reading.hwm * (Decimal("1") - max_total_dd_pct) + for reading in readings + ] + + # The bounds span the OVERLAYS as well as the equity: the floor and the high-water mark are + # drawn in this box, and a line outside it is a line a reader cannot see. + values = [reading.equity for reading in readings] + values += [reading.hwm for reading in readings] + values += [floor for floor in floors if floor is not None] + low, high = min(values), max(values) + span = high - low + + first_ts, last_ts = readings[0].ts, readings[-1].ts + time_span = last_ts - first_ts + + def _y(value: Decimal) -> Decimal: + # Zero span has no range to normalise against, so a flat account is drawn mid-box: + # pinning it to an edge would imply it sat at an extreme of something. + if span == _ZERO: + return (PLOT_HEIGHT / 2).quantize(_COORD) + return _plot_y(value, low=low, span=span) + + def _x(ts: int) -> Decimal: + # A single reading (or several within one second) has no span to place points along. The + # left edge, because one cycle is the BEGINNING of the record, not the whole of it. + if time_span <= 0: + return _ZERO + return (PLOT_WIDTH * (Decimal(ts - first_ts) / Decimal(time_span))).quantize(_COORD) + + segments: list[EquitySeriesSegment] = [] + current: list[EquitySeriesPoint] = [] + current_mode: str | None = None + for reading, floor in zip(readings, floors): + if reading.mode != current_mode: + if current: + segments.append(EquitySeriesSegment(mode=str(current_mode), points=current)) + current, current_mode = [], reading.mode + current.append( + EquitySeriesPoint( + ts=reading.ts, + mode=reading.mode, + equity=reading.equity, + cash=reading.cash, + unrealized=reading.unrealized, + hwm=reading.hwm, + dd_floor=floor, + x=_x(reading.ts), + y=_y(reading.equity), + hwm_y=_y(reading.hwm), + dd_floor_y=None if floor is None else _y(floor), + ) + ) + if current: + segments.append(EquitySeriesSegment(mode=str(current_mode), points=current)) + + return EquitySeries( + segments=segments, + low=low, + high=high, + width=PLOT_WIDTH, + height=PLOT_HEIGHT, + total_recorded=total_recorded, + ) + + def build_equity_curve(entries: Sequence[JournalEntry]) -> EquityCurve: """The cumulative net-P&L curve over `entries`, oldest first. diff --git a/keel/data/db.py b/keel/data/db.py index eab962d..0774a45 100644 --- a/keel/data/db.py +++ b/keel/data/db.py @@ -20,7 +20,7 @@ from pathlib import Path from typing import Any -SCHEMA_VERSION = 18 +SCHEMA_VERSION = 19 # Creation order matters for readability (and for backends that validate FK targets eagerly); # SQLite itself only checks FK targets at DML time, but we still declare referenced tables first. @@ -369,6 +369,30 @@ updated_ts INTEGER NOT NULL ) """, + """ + CREATE TABLE IF NOT EXISTS equity_points ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + -- Epoch seconds, INTEGER, like every other timestamp in this schema (`positions. + -- opened_at`, `asset_attestations.attested_at`) and like the `now_ts` the agent passes + -- down its whole cycle. TEXT would order an epoch LEXICALLY, so a `ts >= ?` window -- + -- which is how the chart reads this table -- would silently return the wrong rows. + ts INTEGER NOT NULL, + -- 'paper' | 'live'. NO `profile` column: the database is already one-per-profile + -- (ADR 0002), while paper and live flip WITHIN one database -- so mode is the + -- partition that actually needs storing, and the one a reader must never blend. + mode TEXT NOT NULL, + equity TEXT NOT NULL, + -- NULL is "not recorded", never zero: a cycle can know its total while the split is + -- unavailable (`orders.filled_quantity`'s convention, and for the same reason). + cash TEXT, + unrealized TEXT, + -- The high-water mark AFTER this reading, so the row carries the rail-11 ceiling that + -- was in force rather than one a reader recomputes -- `record_external_flow` rebases + -- the HWM on a declared deposit, and a recomputed maximum would miss that. + hwm TEXT NOT NULL + ) + """, + "CREATE INDEX IF NOT EXISTS idx_equity_points_mode_ts ON equity_points(mode, ts)", ) @@ -816,6 +840,23 @@ def _migrate_v18_venue_cash_postures(conn: sqlite3.Connection) -> None: """ +def _migrate_v19_equity_points(conn: sqlite3.Connection) -> None: + """v19 adds `equity_points`. Table creation is handled by `_SCHEMA_STATEMENTS`; there is + deliberately NO backfill, and what could be backfilled is exactly what must not be. + + Two sources look like history. `agent_state["equity_history"]` holds at most 7 days, and it + is a RAIL's working set, not a record: `record_external_flow` shifts every point in it by a + declared deposit so the weekly drawdown keeps measuring trading performance. Replaying those + shifted numbers as observations would publish equities the account never had. The `orders` + ledger could reconstruct a curve, but only for closed trades, only at trade resolution, and + with no cash leg -- a different quantity wearing this table's name. + + An empty table means "not observed before v19", which is true: nothing wrote it down. The + chart starts at the first cycle after the upgrade and says so, rather than opening on a + fabricated past. + """ + + _MIGRATIONS: dict[int, Callable[[sqlite3.Connection], None]] = { 2: _migrate_v2_broker_subscriptions, 3: _migrate_v3_trade_outcomes, @@ -834,6 +875,7 @@ def _migrate_v18_venue_cash_postures(conn: sqlite3.Connection) -> None: 16: _migrate_v16_orders_submit_book, 17: _migrate_v17_candle_series_feed, 18: _migrate_v18_venue_cash_postures, + 19: _migrate_v19_equity_points, } diff --git a/keel/data/repository.py b/keel/data/repository.py index 63ba591..5a2bb67 100644 --- a/keel/data/repository.py +++ b/keel/data/repository.py @@ -18,7 +18,7 @@ from keel_core.subscription import BrokerSubscription, SubscriptionStatus from keel_core.trade_scope import TradeScopeState, VenueTradeScope -from keel.types import Candle, Granularity, Profile +from keel.types import Candle, EquityReading, Granularity, Profile _TRANSACTION_COLUMNS = ( "coinbase_id", @@ -152,6 +152,24 @@ def _cash_posture_from_row(row: Any) -> VenueCashPosture: credential_fingerprint=row["credential_fingerprint"], ) + +def _equity_point_from_row(row: Any) -> EquityReading: + """Map an `equity_points` row to the domain record (#698). + + `cash` and `unrealized` stay `None` when the column is NULL rather than becoming + `Decimal("0")`: the column means "not recorded", and a reader must be able to tell an + unobserved split from an observed flat one. + """ + return EquityReading( + ts=int(row["ts"]), + mode=row["mode"], + equity=Decimal(row["equity"]), + cash=_text_to_dec(row["cash"]), + unrealized=_text_to_dec(row["unrealized"]), + hwm=Decimal(row["hwm"]), + ) + + def _json_default(obj: Any) -> Any: if isinstance(obj, Decimal): return {"__decimal__": str(obj)} @@ -696,6 +714,98 @@ def list_venue_cash_postures(self) -> list[VenueCashPosture]: rows = self._conn.execute("SELECT * FROM venue_cash_postures ORDER BY venue").fetchall() return [_cash_posture_from_row(row) for row in rows] + # -- equity points (the mark-to-market series; #698) -------------------- + + def record_equity_point(self, point: EquityReading) -> None: + """Append one cycle's mark-to-market reading. Append-only: never updated, never deleted. + + There is no uniqueness constraint on `(ts, mode)` and deliberately so. This is an + observation log, not a keyed record: two readings that genuinely happened at the same + epoch second are two observations, and silently collapsing them with an upsert would + hide a double-run of the cycle -- exactly the operational fact an operator would want + the series to show. `update_drawdown` calls this once per cycle; anything more is a + symptom worth seeing. + """ + self._conn.execute( + """ + INSERT INTO equity_points (ts, mode, equity, cash, unrealized, hwm) + VALUES (?, ?, ?, ?, ?, ?) + """, + ( + point.ts, + point.mode, + _dec_to_text(point.equity), + _dec_to_text(point.cash), + _dec_to_text(point.unrealized), + _dec_to_text(point.hwm), + ), + ) + self._conn.commit() + + def get_equity_points( + self, + mode: str | None = None, + since_ts: int | None = None, + limit: int | None = None, + ) -> list[EquityReading]: + """The series, oldest first, optionally narrowed to one mode, a time window, or a count. + + `mode=None` returns paper AND live rows interleaved by time. That is the honest raw + read, but it is NOT a curve: a caller drawing it as one line joins two unrelated + accounts across the flip. Readers that plot must group by `mode` (`insights` does). + + `limit` keeps the MOST RECENT `limit` readings and still returns them oldest first. Two + halves of one decision: + + * **Most recent, not first.** This table is append-only and grows one row per cycle + forever -- at the default `auto_trade.interval_sec` of 900 that is ~35,000 rows a + year. A cap that kept the OLDEST rows would answer "where is this account now?" with + the readings furthest from the answer, and would freeze the chart the day the cap was + reached. + * **Still oldest first.** The ordering is the caller's contract, not an artefact of how + the rows were selected. `ORDER BY ts DESC LIMIT n` in a subquery, re-ordered outside + it, so a bounded read and an unbounded one differ only in how much they return. + + A caller that bounds a read is showing a WINDOW of the record and must say so: + `count_equity_points` is how it learns what it is leaving out. + """ + clauses: list[str] = [] + params: list[object] = [] + if mode is not None: + clauses.append("mode = ?") + params.append(mode) + if since_ts is not None: + clauses.append("ts >= ?") + params.append(since_ts) + where = (" WHERE " + " AND ".join(clauses)) if clauses else "" + # `id` breaks the tie so two readings at the same epoch second keep insertion order -- + # `ts` alone leaves that to SQLite, and a chart would draw them in an arbitrary one. It + # is applied in BOTH directions below so the newest-N and the oldest-first re-order + # agree about which of two same-second readings is the newer. + if limit is None: + query = f"SELECT * FROM equity_points{where} ORDER BY ts, id" + else: + query = ( + f"SELECT * FROM (SELECT * FROM equity_points{where} " + "ORDER BY ts DESC, id DESC LIMIT ?) ORDER BY ts, id" + ) + params.append(limit) + return [_equity_point_from_row(row) for row in self._conn.execute(query, params)] + + def count_equity_points(self, mode: str | None = None) -> int: + """How many readings the table holds -- the total a bounded read is a window ONTO. + + A `COUNT(*)`, never `len(get_equity_points())`: the entire point is to learn the size + without paying to materialise the rows, which is what the caller just declined to do. + """ + query = "SELECT COUNT(*) AS n FROM equity_points" + params: list[object] = [] + if mode is not None: + query += " WHERE mode = ?" + params.append(mode) + row = self._conn.execute(query, params).fetchone() + return int(row["n"]) + # -- trade outcomes (closed round-trips; rails 11 and 16) --------------- def insert_trade_outcome(self, outcome: dict[str, Any]) -> int: diff --git a/keel/execution/equity.py b/keel/execution/equity.py index 421d683..16680bf 100644 --- a/keel/execution/equity.py +++ b/keel/execution/equity.py @@ -16,6 +16,7 @@ from decimal import Decimal from keel_core.telemetry import log_event +from keel_core.types import EquityReading from keel.compliance.purification import build_report from keel.data.repository import Repository @@ -56,6 +57,40 @@ def mark_positions( return total +def unrealized_on_marks( + positions: list[tuple[Decimal, Decimal]], + price_by_product: dict[str, Decimal], + product_ids: list[str], +) -> Decimal: + """Unrealized P&L = Σ qty·(mark − cost_basis), on the SAME marks `mark_positions` used (#698). + + Deliberately a sibling of `mark_positions` with an identical shape and identical guards: the + two are written into one `equity_points` row for one cycle, so a position the equity valued + at cost must contribute ZERO here. A helper that skipped the unpriced position instead, or + marked it differently, would file a `cash`/`unrealized`/`equity` triple that does not add up + -- and the row is only worth keeping if it reconciles (see + `test_cash_plus_cost_basis_plus_unrealized_reconstructs_the_equity`). + + Signed: negative while a position is under water, which is the direction that matters, since + the whole reason equity is marked to market is to see a loss WHILE it is happening. + """ + total = Decimal("0") + for (qty, cost_basis), product_id in zip(positions, product_ids): + if qty <= 0: + continue + mark = price_by_product.get(product_id) + if mark is None or mark <= 0: + # Valued at cost by `mark_positions`, so there is no observed gain or loss to book. + # (When the basis is non-positive too, `mark_positions` drops the position outright + # and this `continue` has already matched it -- the two stay in step either way.) + continue + # No guard on a non-positive `cost_basis`: `mark_positions` values a fresh-priced + # position at `qty * mark` whatever it cost, so a zero-basis holding is ALL unrealized + # gain. Skipping it here would leave the row's parts short of its own equity. + total += qty * (mark - cost_basis) + return total + + def pending_purification_usd(repo: Repository) -> Decimal: """Accrued-but-unpurified non-compliant income, in USD, from the repo's own ledger (#490). @@ -140,8 +175,27 @@ def record_external_flow(repo: Repository, *, amount: Decimal) -> None: ) -def update_drawdown(repo: Repository, *, equity: Decimal, now_ts: int) -> None: - """Record `equity` and refresh the drawdown scalars rail 11 consumes.""" +def update_drawdown( + repo: Repository, + *, + equity: Decimal, + now_ts: int, + cash: Decimal | None = None, + unrealized: Decimal | None = None, +) -> None: + """Record `equity` and refresh the drawdown scalars rail 11 consumes. + + Also appends one row to the durable `equity_points` series (#698). The two records are NOT + redundant: `equity_history` below is a 7-day window that `record_external_flow` REWRITES on + a declared deposit, because the weekly rail must keep measuring trading performance. That + makes it a working set, not a record -- so the series is written alongside it rather than + derived from it, and neither is reconstructable from the other. + + `cash` and `unrealized` are the optional split of `equity`. They are passed, never derived + here: this function has a total and no positions, and inventing the split from the total is + exactly the fabrication `None` exists to avoid. Callers that know it (the agent's paper and + live branches both do) pass it; callers that do not leave it unrecorded. + """ _warn_on_unexplained_jump(repo, equity=equity) hwm = repo.get_state("equity_high_water_mark") @@ -170,6 +224,51 @@ def update_drawdown(repo: Repository, *, equity: Decimal, now_ts: int) -> None: else max((weekly_peak - equity) / weekly_peak, Decimal("0")), ) + _append_equity_point( + repo, equity=equity, now_ts=now_ts, hwm=hwm, cash=cash, unrealized=unrealized + ) + + +def _append_equity_point( + repo: Repository, + *, + equity: Decimal, + now_ts: int, + hwm: Decimal, + cash: Decimal | None, + unrealized: Decimal | None, +) -> None: + """Append this cycle's reading to the durable series, stamped with the mode that produced it. + + The mode comes from `equity_state_mode` -- the SAME stamp `agent._clear_live_mode_if_needed` + and `agent._seed_paper_account_if_needed` read before wiping the shared HWM on a flip. + Deriving it a second way here (from a passed flag, say) would let two answers to one question + drift apart, and the failure would be silent: a row filed under the wrong mode does not go + missing, it lands in the other account's curve. + + An UNSTAMPED mode writes nothing, and does not raise. Every agent path stamps it + unconditionally before this runs, so the unstamped case is not a real cycle (a direct call in + a test, a caller yet to be written). Two things it must not do: guess a mode -- that is the + mislabelling above, manufactured -- or fail the call, which would take rail 11's scalars down + with the chart. The drawdown scalars are already written by the time this is reached, so a + skipped point costs a gap in a chart and nothing else. + + LAST in `update_drawdown` for that reason: the rail is served before the record is kept. + """ + mode = repo.get_state("equity_state_mode") + if mode is None: + return + repo.record_equity_point( + EquityReading( + ts=now_ts, + mode=str(mode), + equity=equity, + cash=cash, + unrealized=unrealized, + hwm=hwm, + ) + ) + def _warn_on_unexplained_jump(repo: Repository, *, equity: Decimal) -> None: """Log a WARNING when equity moves more than `UNEXPLAINED_JUMP_PCT` between cycles. diff --git a/keel/strategy/paper.py b/keel/strategy/paper.py index 72115e3..aacfad0 100644 --- a/keel/strategy/paper.py +++ b/keel/strategy/paper.py @@ -242,6 +242,21 @@ def equity(self, price_by_product: dict[str, Decimal]) -> Decimal | None: positions = [(self._open[p].qty, self._open[p].entry_fill) for p in product_ids] return mark_positions(self._cash, positions, price_by_product, product_ids) + def unrealized(self, price_by_product: dict[str, Decimal]) -> Decimal | None: + """The unrealized leg of the SAME reading `equity()` reports (#698). + + `None` under exactly `equity()`'s condition -- unseeded cash -- so the two are recorded + together or not at all, and the same `costed=False` exclusion applies: a position + nothing paid for is not in the equity, so its gain must not be in the P&L either. + """ + if self._cash is None: + return None + from keel.execution.equity import unrealized_on_marks + + product_ids = [p for p in self._open if self._open[p].costed] + positions = [(self._open[p].qty, self._open[p].entry_fill) for p in product_ids] + return unrealized_on_marks(positions, price_by_product, product_ids) + def on_signal( self, signal: Signal, candle: Candle | None = None, qty: Decimal = _QTY ) -> int | None: diff --git a/keel/web/api.py b/keel/web/api.py index f336594..6a07fd6 100644 --- a/keel/web/api.py +++ b/keel/web/api.py @@ -54,6 +54,23 @@ #: journal` is, and it runs in the operator's own process against their own machine's limits. MAX_JOURNAL_LIMIT = 1000 +#: How many equity readings `/api/insights` draws, newest first (#698). +#: +#: A fixed cap rather than a `?limit=`: this route carries no collection a caller sorts or pages, +#: and a chart is not a bulk export -- the whole series is on disk for anyone who wants it. +#: +#: 1000 because the plot box is `PLOT_WIDTH` = 1000 units wide, so past one reading per unit the +#: extra rows land on coordinates the chart has already drawn: cost with nothing on screen to +#: show for it. Measured at 580 bytes per point in the rendered payload, this caps the response +#: near 580 KB; the page re-polls every 15 seconds (`main.js`'s `POLL_MS`), which is what makes +#: an unbounded read here a recurring cost rather than a one-off one. At the default +#: `auto_trade.interval_sec` of 900 the table passes this cap in about ten days. +#: +#: What it would take to change: a chart the operator can pan or zoom over a range they choose. +#: That makes the range a client concern and this builder a service the client re-asks with new +#: bounds -- the same condition `insights.py`'s own module note already names. +EQUITY_POINT_LIMIT = 1000 + #: The two directions, and no third spelling. `desc`/`descending`/`down` would all have to be #: accepted forever once accepted once, and a client reading `sort.direction` back off the #: response needs one word to compare against. @@ -279,21 +296,44 @@ def read_orders(cfg: ServeConfig, query: Query, _state: Any, _now_ts: int) -> di def read_insights(cfg: ServeConfig, _query: Query, _state: Any, now_ts: int) -> dict[str, Any]: - """The per-rule track records and the promotion-gate distances. + """The per-rule track records, the promotion-gate distances, and the account-equity series. The journal the HTML `/insights` page renders below these is `/api/journal` instead of a second table here. One sortable collection per endpoint keeps `?sort=` unambiguous without a `?table=` beside it, and it gives the journal somewhere to carry its own `?limit=` -- the cap - the HTML page apologises for in a comment ("a cap, not a paginator").""" - from keel.commands.insights import build_insights_report + the HTML page apologises for in a comment ("a cap, not a paginator"). + + The equity series (#698) is read here rather than on `/api/journal`, next to the curve it is + drawn above, because it describes the ACCOUNT and not the closed trades. The journal's curve + narrows with that endpoint's `?limit=`; this does not narrow with anything, and two charts + that answer a query differently must not share one payload where a reader would assume they + agree. + + `max_total_dd_pct` comes off the SAME loaded config `build_insights_report` reads, so the + drawdown floor drawn under the curve and the ceiling quoted in the account card beside it are + one setting rather than two reads that can disagree. + + The series read is BOUNDED (`EQUITY_POINT_LIMIT`) and says so. `equity_points` is + append-only and grows one row per cycle forever, so an unbounded read here would be this + route's memory cost rising without limit for the life of the deployment -- the exact hazard + `MAX_JOURNAL_LIMIT`'s note names, on a route with no `?limit=` for an operator to moderate + it with. `count_equity_points` is passed alongside so the chart can state what it is not + showing rather than quietly beginning wherever the cap fell. + """ + from keel.commands.insights import build_equity_series, build_insights_report repo = open_repo(cfg.db_path) try: config = load_config(cfg.config_path) report = build_insights_report(repo, config, _status_report(cfg, now_ts), now_ts) + series = build_equity_series( + repo.get_equity_points(limit=EQUITY_POINT_LIMIT), + max_total_dd_pct=config.money_mgmt.max_total_dd_pct, + total_recorded=repo.count_equity_points(), + ) finally: close_repo(repo) - return payload.insights_payload(report) + return payload.insights_payload(report, series=series) def read_journal(cfg: ServeConfig, query: Query, _state: Any, now_ts: int) -> dict[str, Any]: diff --git a/keel/web/payload.py b/keel/web/payload.py index 82fcddf..eacc623 100644 --- a/keel/web/payload.py +++ b/keel/web/payload.py @@ -124,6 +124,8 @@ AccountSummary, EquityCurve, EquityPoint, + EquitySeries, + EquitySeriesPoint, GateDistance, InsightsReport, JournalEntry, @@ -907,13 +909,25 @@ def _track_record_payload(record: RuleTrackRecord) -> dict[str, Any]: } -def insights_payload(report: InsightsReport) -> dict[str, Any]: - """`build_insights_report`'s `InsightsReport`, as JSON.""" +def insights_payload(report: InsightsReport, *, series: EquitySeries) -> dict[str, Any]: + """`build_insights_report`'s `InsightsReport`, as JSON, with the equity series beside it. + + `series` is a REQUIRED keyword for the reason `journal_payload`'s `curve` is: a default would + quietly serve an insights view with no chart, and the caller is the one place that knows + which readings and which rail setting the chart is drawn from. + + It rides the insights payload rather than the journal's because it describes the ACCOUNT, not + the closed trades. The distinction is load-bearing at the other end: the journal's curve is + built from `report.entries` and therefore narrows with a `?limit=`, while this series does + not narrow with anything the journal does. Two charts that respond differently to the same + query belong to two different payloads. + """ 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), + "equity_series": equity_series_payload(series), "rules": [_track_record_payload(r) for r in report.rules], } @@ -1026,6 +1040,106 @@ def equity_curve_payload(curve: EquityCurve) -> dict[str, Any]: } +def _equity_series_point_payload(point: EquitySeriesPoint) -> dict[str, Any]: + """One cycle's reading: where to draw it, and what it says (#698). + + `x`, `y`, `hwm_y` and `dd_floor_y` are BARE STRINGS for the reason `_equity_point_payload` + gives -- they are positions inside a viewBox, not figures a human reads. `dd_floor_y` is + `null`, not `"0"`, when the rail setting is unknown: a zero coordinate is the TOP of an SVG + box, so it would draw a ceiling line in force above every reading rather than none at all. + + `unrealized` is the one leg that carries a verdict, because it is the one that is a + gain-or-loss figure. `equity`, `cash` and `hwm` are magnitudes -- an account balance is not + good or bad on its own, and `money(signed=True)` would put a ▲ on a number that has no + direction. `dd_floor` is a magnitude too: it is the rail's ceiling, not a judgement about + how close this reading sits to it. + """ + return { + "x": _plain(point.x), + "y": _plain(point.y), + "hwm_y": _plain(point.hwm_y), + "dd_floor_y": None if point.dd_floor_y is None else _plain(point.dd_floor_y), + "at": moment(point.ts), + "mode": point.mode, + "equity": money(point.equity), + "cash": money(point.cash), + "unrealized": money(point.unrealized, signed=True), + "hwm": money(point.hwm), + "dd_floor": money(point.dd_floor), + } + + +def equity_series_payload(series: EquitySeries) -> dict[str, Any]: + """`build_equity_series`'s `EquitySeries`, as JSON (#698). + + **Segments cross as segments, never as one flat list of points.** The mode partition is the + whole reason this shape exists: paper and live are unrelated accounts that share a database, + and a client handed one list would join them into a polyline showing a collapse that never + happened. Flattening is the one operation the wire must not make easy. + + `reading` is the chart's text equivalent, written here for the reason + `equity_curve_payload`'s is: summarising a chart is a judgement, and a reader who cannot see + it must be told the same thing in words. It NAMES THE MODES, because the mode split is the + part a spoken summary would otherwise flatten -- a sentence saying an account went from ten + thousand dollars to two hundred and fifty is exactly the false continuity the segments exist + to prevent, and it would be no less false for being spoken. + + `state` on that sentence is `neutral`, unlike the curve's. The curve closes on a cumulative + net P&L, which is a verdict; a series closes on an account balance, which is not one. Taking + the state from the last reading's sign would call an account "good" for holding money. + """ + low, high = money(series.low), money(series.high) + cycles = count(series.point_count) + truncated_from = count(series.total_recorded) + if series.segments: + # `series.modes` and `series.is_partitioned`, never a mode list assembled here: Rule 6e + # bans `len()` in this module precisely so a count on the wire is one the report already + # holds, and a serialiser deriving its own answer is how two answers start to differ. + span = " and ".join(series.modes) + # "the most recent N", never a bare N, when the read was bounded. The sentence is the + # whole of what a reader who cannot see the chart is told about its span, so a window + # described as a history is a lie by omission that every individual point survives. + scope = "the most recent " if series.is_truncated else "" + reading = ( + f"Account equity over time across {scope}{cycles['display']} cycle(s) in {span}, " + f"ranging from {low['display']} to {high['display']}." + ) + if series.is_truncated: + reading += f" {truncated_from['display']} readings are recorded in total." + # Said only when there IS a partition. A deployment that has only ever run paper would + # otherwise be told about a second line that is not on the chart -- the same false + # continuity the segments exist to prevent, pointing the other way. + if series.is_partitioned: + reading += " Each mode is drawn as its own line; they are separate accounts." + else: + # Not "the account was flat", and not an empty string. A deployment that has not run a + # cycle since the series began recording has no readings at all, and telling those two + # apart is the whole difference between an empty chart and a broken one. + reading = "No equity readings recorded yet, so there is no series to draw." + return { + "width": _plain(series.width), + "height": _plain(series.height), + "point_count": cycles, + "total_recorded": truncated_from, + # A `flag`, not a bare boolean: a client must not have to compare `point_count` against + # `total_recorded` to learn this. That comparison is arithmetic, and the answer is a + # statement about how much of the record the chart is showing -- a judgement, made here. + "is_truncated": flag( + series.is_truncated, on="a window onto a longer record", off="the whole record" + ), + "low": low, + "high": high, + "reading": label("series", display=reading, state=NEUTRAL), + "segments": [ + { + "mode": segment.mode, + "points": [_equity_series_point_payload(p) for p in segment.points], + } + for segment in series.segments + ], + } + + def journal_payload(report: JournalReport, *, curve: EquityCurve) -> dict[str, Any]: """`build_journal_report`'s `JournalReport`, as JSON. diff --git a/keel/web/static/css/keel.css b/keel/web/static/css/keel.css index d942c4f..8b36427 100644 --- a/keel/web/static/css/keel.css +++ b/keel/web/static/css/keel.css @@ -712,6 +712,45 @@ pre.joblines { .chart .dot { fill: var(--accent); opacity: 0.55; } .chart figcaption { margin: 0.6rem 0 0; } +/* ── the account-equity series (#698) ───────────────────────────────────────────────────────── + * + * The chart stacked ABOVE the closed-trade curve: what the account was worth each cycle, on a + * time axis. No `cursor: crosshair` and no `touch-action: none`, because `main.js`'s #602 + * gestures deliberately bind to `svg.curve` and not to this canvas. + * + * PAPER IS DASHED, LIVE IS SOLID, and that is not decoration. The two modes are unrelated + * accounts drawn on one canvas, so the distinction has to survive greyscale, e-ink and + * red-green colour deficiency exactly as the ▲/▼ glyphs do for the figures -- the same rule + * `chart.js::outcomeOf` follows for a losing segment. Solid reads as the real one. + * + * Both rails are drawn beneath the account line in `--muted`, the token the baseline already + * uses: they are the context the equity is read against, never the subject. The floor is dashed + * more finely than the high-water mark so the pair are told apart without colour either. + */ +.chart svg.series { + display: block; + width: 100%; + height: 14rem; + overflow: visible; +} +.chart svg.series .line.paper { stroke-dasharray: 7 4; } +.chart svg.series .hwm { + fill: none; + stroke: var(--muted); + stroke-width: 1; + stroke-dasharray: 5 3; + vector-effect: non-scaling-stroke; +} +.chart svg.series .floor { + fill: none; + stroke: var(--muted); + stroke-width: 1; + stroke-dasharray: 2 3; + opacity: 0.75; + vector-effect: non-scaling-stroke; +} +.chart svg.series .dot { r: 3; } + /* ── the cursor legend and the trade highlight (#602) ──────────────────────────────────────── * * Every colour below is one of the tokens `tests/web/test_palette_contrast.py` already measures diff --git a/keel/web/static/js/chart.js b/keel/web/static/js/chart.js index 1ae8b5d..f242645 100644 --- a/keel/web/static/js/chart.js +++ b/keel/web/static/js/chart.js @@ -110,7 +110,23 @@ function svg(tag, attributes) { * @returns {string} */ function polylinePoints(points) { - return points.map(/** @param {any} point */ (point) => [point.x, point.y].join(",")).join(" "); + return polylineOn(points, "y"); +} + +/** + * The `points` attribute of a polyline over one named vertical coordinate. + * + * `key` picks which line a point contributes to -- its equity (`"y"`), the high-water mark in + * force at that instant (`"hwm_y"`), or the drawdown floor beneath it (`"dd_floor_y"`). All + * three arrive finished from `build_equity_series`; choosing between them is a lookup, not the + * arithmetic this file is not allowed to do. + * + * @param {any[]} points + * @param {string} key + * @returns {string} + */ +function polylineOn(points, key) { + return points.map(/** @param {any} point */ (point) => [point.x, point[key]].join(",")).join(" "); } /** @@ -214,6 +230,124 @@ function outcomeOf(point) { * @param {string} id the id given to the caption, which names the chart for a screen reader. * @returns {HTMLElement|null} */ +/** + * The account-equity series over time (#698), drawn ABOVE the closed-trade curve. + * + * ── WHY IT IS A SECOND CHART AND NOT A REPLACEMENT ─────────────────────────────────────────── + * + * `equityChart` plots cumulative net P&L over closed TRADES, on a trade-order axis, and + * `build_equity_curve` argues for that axis on its own terms: a quiet week must not carry the + * visual weight of fifty trades when the subject is statistical expectancy. This plots what the + * ACCOUNT was worth every cycle, traded or not, where the quiet week is the information. Two + * questions, two charts, stacked -- portfolio reality above, expectancy below. + * + * ── ONE POLYLINE PER SEGMENT, NEVER ONE PER SERIES ─────────────────────────────────────────── + * + * Paper and live are unrelated accounts that share a database and flip within it. The payload + * hands this file `segments`, already split by `build_equity_series`, precisely so that joining + * them is not something this file can do by accident: a single line from a $10,000 paper account + * to a $250 live one would draw a collapse that never happened. The modes are told apart by + * DASH as well as by colour, the same rule the losing-trade segment below follows -- paper is + * dashed because it is the synthetic one. + * + * ── THE TWO OVERLAYS ARE READ, NOT DERIVED ─────────────────────────────────────────────────── + * + * Each point carries `hwm_y` (the rail-11 high-water mark in force when the agent acted) and + * `dd_floor_y` (the equity at which rail 11 starts vetoing entries). Both are computed in + * Python from the recorded row, never here -- and `dd_floor_y` is `null` rather than `"0"` when + * the rail setting is unknown, because a zero coordinate is the TOP of the box and would draw a + * ceiling that is in force above every reading. + * + * ── NO ZOOM, NO PAN, NO CURSOR LEGEND ──────────────────────────────────────────────────────── + * + * `main.js`'s #602 gestures bind to `svg.curve`, and this canvas is `svg.series` on purpose: the + * two would otherwise fight over `contentNode.querySelector("svg.curve")`, which takes the FIRST + * match and would find this chart instead of the one the gestures were written for. Adding them + * here is a separate piece of work with its own arithmetic to place in `main.js`. + * + * @param {any} series `/api/insights`'s `data.equity_series`. + * @param {string} id the `
` id this chart is named by. + * @returns {HTMLElement|null} + */ +export function equitySeriesChart(series, id) { + if (!series || !Array.isArray(series.segments) || series.segments.length === 0) return null; + + const canvas = svg("svg", { + viewBox: ["0", "0", series.width, series.height].join(" "), + preserveAspectRatio: "none", + class: "series", + role: "img", + "aria-labelledby": id, + }); + + for (const segment of series.segments) { + // The floor first, then the ceiling, then the account: the equity line is the subject and + // sits over both rails rather than under them. + const floored = segment.points.filter( + /** @param {any} point */ (point) => point.dd_floor_y !== null, + ); + // `!== 0`, never `> 0`: `test_render_never_judges_a_value_itself` bans relational operators + // in this file, and an emptiness check has no business needing an ordering anyway. + if (floored.length !== 0) { + canvas.append( + svg("polyline", { + class: ["floor", segment.mode].join(" "), + points: polylineOn(floored, "dd_floor_y"), + }), + ); + } + canvas.append( + svg("polyline", { + class: ["hwm", segment.mode].join(" "), + points: polylineOn(segment.points, "hwm_y"), + }), + ); + const line = svg("polyline", { + class: ["line", segment.mode].join(" "), + points: polylinePoints(segment.points), + }); + // Names the account this line belongs to on hover. Redundant for a screen reader -- the + // `role="img"` above flattened this subtree and `aria-labelledby` names the modes in a + // sentence -- which is why it is a convenience and not the accessible name. + const lineTip = svg("title", {}); + lineTip.textContent = [segment.mode, "equity"].join(" "); + line.append(lineTip); + canvas.append(line); + + // A marker per reading. Same reason `equityChart` draws them: a segment of one point is a + // polyline with nothing to draw between, so the first cycle after an upgrade -- or the + // first live cycle after a flip -- would otherwise render as empty space. + for (const point of segment.points) { + const dot = svg("circle", { + class: ["dot", segment.mode].join(" "), + cx: point.x, + cy: point.y, + r: "3", + }); + const tip = svg("title", {}); + tip.textContent = [point.at.display, point.equity.display, point.mode].join(" · "); + dot.append(tip); + canvas.append(dot); + } + } + + const figure = document.createElement("figure"); + // `"chart series"`, never a bare `"chart"`. `svg.series` keeps the #602 POINTER gestures off + // this canvas, but `main.js` also reaches for the WRAPPER twice -- `highlightJournalRow` and + // the chart-action handler -- with `querySelector`, which takes the first match in document + // order, and this figure is appended ABOVE the curve's. Left as a bare `figure.chart` it + // intercepts both: `highlightTrade` finds no `.highlight` group here, returns early, and + // hovering a journal row silently stops highlighting anything. The second half of the fix is + // the `:not(.series)` in those two selectors. + figure.className = "chart series"; + const caption = document.createElement("figcaption"); + caption.className = "note"; + caption.id = id; + caption.textContent = series.reading.display; + figure.append(canvas, caption); + return figure; +} + export function equityChart(curve, id) { if (!curve || !Array.isArray(curve.points) || curve.points.length === 0) return null; diff --git a/keel/web/static/js/main.js b/keel/web/static/js/main.js index acc97c0..9d89f5f 100644 --- a/keel/web/static/js/main.js +++ b/keel/web/static/js/main.js @@ -977,7 +977,11 @@ function reapplyChartView() { * @param {HTMLElement|null} row */ function highlightJournalRow(row) { - const figure = contentNode.querySelector("figure.chart"); + // `:not(.series)` because /insights renders TWO `figure.chart`s and the account-equity series + // (#698) is the first in document order. Without it this lookup returns that figure, which + // carries no `.highlight` group, and `highlightTrade` returns early -- the highlight silently + // stops working, with nothing in the console to say why. + const figure = contentNode.querySelector("figure.chart:not(.series)"); if (!(figure instanceof HTMLElement)) return; if (!row || !activeCurve) { @@ -1111,7 +1115,11 @@ contentNode.addEventListener("click", (event) => { if (!button || !activeCurve) return; const canvas = contentNode.querySelector("svg.curve"); - const figure = contentNode.querySelector("figure.chart"); + // Paired with the canvas above, so both must name the SAME chart: `svg.curve` already + // excludes the #698 series canvas, and `:not(.series)` is what keeps the wrapper lookup in + // step with it. Mismatched, "Save as image" would read its background colour off one figure + // and its geometry off another. + const figure = contentNode.querySelector("figure.chart:not(.series)"); if (!(canvas instanceof SVGSVGElement) || !(figure instanceof HTMLElement)) return; const action = button.getAttribute("data-chart-action"); diff --git a/keel/web/static/js/render.js b/keel/web/static/js/render.js index 4223ded..a4dd719 100644 --- a/keel/web/static/js/render.js +++ b/keel/web/static/js/render.js @@ -55,7 +55,7 @@ * the fix is in `keel/web/payload.py`. There is nowhere in this file to put it. */ -import { equityChart } from "./chart.js"; +import { equityChart, equitySeriesChart } from "./chart.js"; import { termLink } from "./docs.js"; import { instant } from "./format.js"; @@ -1676,6 +1676,38 @@ export function insightsView(insights, journal, sort, onSort, journalSort, onJou shown.append(field(journal.shown_count), " of ", field(journal.total_count), " closed trade(s)"); fragment.append(shown); + // #698: portfolio reality above statistical expectancy. The account-equity series is what the + // account was WORTH each cycle; the curve below it is what the closed trades DID. Stacked + // rather than merged because they are different quantities on different axes -- see + // `chart.js::equitySeriesChart`. + // + // It comes off `/api/insights`, not `/api/journal`, so it is NOT narrowed by the journal's + // `?limit=`. The note below says so: a reader looking at two stacked charts would otherwise + // reasonably assume the row cap above applies to both. + const series = equitySeriesChart(insights.equity_series, "h-series"); + if (series) { + // What the top chart's span actually is, in words, next to the chart. `is_truncated` is a + // `flag` whose `display` already says which case this is -- the client never compares + // `point_count` against `total_recorded` to find out, because that comparison is arithmetic + // and the answer is a claim about how much of the record is on screen. + const scope = el("p", "note"); + scope.append( + field(insights.equity_series.point_count), + " of ", + field(insights.equity_series.total_recorded), + " recorded cycle(s) — ", + field(insights.equity_series.is_truncated), + ". Not narrowed by the journal row cap below.", + ); + fragment.append(series, scope); + } else if (insights.equity_series) { + fragment.append(el("p", "empty", plain(insights.equity_series.reading.display))); + } + // No `else` for a payload with no `equity_series` at all. That is not a deployment with no + // readings -- it is a RESPONSE FROM BEFORE THIS FIELD EXISTED, which the service worker can + // still be holding after an upgrade. Reading `.reading.display` off it would throw and blank + // the whole insights view over a stale cache entry the next refresh fixes on its own. + const chart = equityChart(journal.curve, "h-curve"); if (chart) { fragment.append(chart); diff --git a/packages/keel-core/keel_core/types.py b/packages/keel-core/keel_core/types.py index fb5e95a..6d9667a 100644 --- a/packages/keel-core/keel_core/types.py +++ b/packages/keel-core/keel_core/types.py @@ -41,6 +41,34 @@ class Candle: volume: Decimal +@dataclass(frozen=True) +class EquityReading: + """One cycle's mark-to-market equity reading, as the agent computed it (#698). + + `mode` is `"paper"` or `"live"` -- the same partition `agent._clear_live_mode_if_needed` + enforces on the shared high-water mark. Two modes share one database (ADR 0002 gives each + profile its own), and their equities are unrelated accounts: a reader that blends them draws + a cliff at the flip and calls it a drawdown. + + `cash` and `unrealized` are `None` for "not recorded", never zero -- the `orders.filled_ + quantity` convention. A cycle can know its total equity while the split is unavailable (an + unseeded paper account, a broker that answered for the total but not per-currency), and + writing a zero there would state a flat position that was never observed. + + `hwm` is the high-water mark AFTER this reading was folded in, so a row carries the rail-11 + ceiling that was actually in force when the agent acted on it -- the chart's drawdown + overlay reads it rather than recomputing a monotonic maximum the engine may have rebased + (`execution.equity.record_external_flow` shifts the HWM on a declared deposit). + """ + + ts: int + mode: str + equity: Decimal + cash: Decimal | None + unrealized: Decimal | None + hwm: Decimal + + @dataclass(frozen=True) class Profile: """The user's own settings, as opposed to operational state or file configuration. @@ -70,4 +98,4 @@ def is_autonomous(self, now_ts: int) -> bool: return now_ts < self.autonomous_until -__all__ = ["Granularity", "Side", "Candle", "Profile"] +__all__ = ["Granularity", "Side", "Candle", "EquityReading", "Profile"] diff --git a/tests/commands/test_equity_series.py b/tests/commands/test_equity_series.py new file mode 100644 index 0000000..95917db --- /dev/null +++ b/tests/commands/test_equity_series.py @@ -0,0 +1,248 @@ +"""The time-axised account-equity chart over `equity_points` -- issue #698. + +This is NOT a replacement for `build_equity_curve`. That one plots cumulative net P&L over +CLOSED TRADES on a trade-order axis, and its docstring argues for that axis on its own terms: a +quiet week must not get the visual weight of fifty trades when the subject is a track record. +This builder plots a different quantity -- what the ACCOUNT was worth, every cycle, whether it +traded or not -- and for that quantity time is the only honest axis, because the gaps are the +information. + +The properties pinned here are the ones that would otherwise let the chart tell a lie the data +does not support: a flip between two accounts drawn as one line, a gap drawn as a straight +segment, and a rail ceiling recomputed rather than read. +""" + +from __future__ import annotations + +from decimal import Decimal + +from keel_core.types import EquityReading + +from keel.commands.insights import PLOT_HEIGHT, PLOT_WIDTH, build_equity_series + +NOW = 1_800_000_000 +DAY = 86_400 + + +def _reading( + ts: int, + equity: str, + *, + mode: str = "live", + hwm: str | None = None, + cash: str | None = None, + unrealized: str | None = None, +) -> EquityReading: + return EquityReading( + ts=ts, + mode=mode, + equity=Decimal(equity), + cash=None if cash is None else Decimal(cash), + unrealized=None if unrealized is None else Decimal(unrealized), + hwm=Decimal(hwm if hwm is not None else equity), + ) + + +def test_a_window_onto_a_longer_record_knows_it_is_one() -> None: + """A bounded read must not draw as though it were the whole history. The chart's own claim + is that these are the numbers the engine acted on; starting the line wherever a row cap + happened to fall, silently, would make the span of the record a lie by omission.""" + series = build_equity_series( + [_reading(NOW, "10000"), _reading(NOW + DAY, "10100")], total_recorded=900 + ) + + assert series.is_truncated is True + assert series.total_recorded == 900 + assert series.point_count == 2 + + +def test_a_series_holding_every_recorded_reading_is_not_truncated() -> None: + series = build_equity_series([_reading(NOW, "10000")], total_recorded=1) + + assert series.is_truncated is False + + +def test_an_unstated_total_is_not_a_claim_that_nothing_was_left_out() -> None: + """`total_recorded=None` means the caller did not say. Defaulting it to the point count + would have the series assert it is complete on behalf of a caller that never checked -- + so the honest answer to "is this truncated?" is not-known, which reads as no claim.""" + series = build_equity_series([_reading(NOW, "10000")]) + + assert series.total_recorded is None + assert series.is_truncated is False + + +def test_the_series_names_the_modes_it_spans_in_the_order_they_appear() -> None: + """The report holds this, not the serialiser. `keel/web/payload.py` may not call `len()` + (Rule 6e of `tests/commands/test_console_thinness.py`), and the sentence it writes for a + reader who cannot see the chart has to name the accounts -- so the list and the count are + the report's own statement about itself, exactly like `point_count`.""" + series = build_equity_series( + [ + _reading(NOW, "10000", mode="paper"), + _reading(NOW + DAY, "250", mode="live"), + _reading(NOW + 2 * DAY, "10100", mode="paper"), + ] + ) + # Three SEGMENTS, two MODES: the run order is the chart's, the mode set is the sentence's. + assert [segment.mode for segment in series.segments] == ["paper", "live", "paper"] + assert series.modes == ["paper", "live"] + assert series.is_partitioned is True + + +def test_a_single_mode_series_is_not_partitioned() -> None: + """What the sentence turns on: with one account there is no split to explain, and telling a + reader the lines are separate accounts would send them looking for a line that is not there.""" + series = build_equity_series([_reading(NOW, "10000", mode="paper")]) + + assert series.modes == ["paper"] + assert series.is_partitioned is False + + +def test_an_empty_series_spans_no_modes() -> None: + series = build_equity_series([]) + + assert series.modes == [] + assert series.is_partitioned is False + + +def test_no_readings_is_a_real_answer_not_an_empty_chart() -> None: + """A deployment that has not run a cycle since v19 has no series. Distinct from a flat + line, which is what an account that did not move looks like.""" + series = build_equity_series([]) + assert series.segments == [] + assert series.point_count == 0 + + +def test_the_x_axis_is_time_so_a_quiet_stretch_leaves_a_gap() -> None: + """The whole reason this chart exists next to the trade-order one: equity is a property of + the calendar. Three cycles, the last of them a week after the second, must NOT be evenly + spaced -- even spacing would draw a week of silence as one ordinary step.""" + series = build_equity_series( + [ + _reading(NOW, "10000"), + _reading(NOW + DAY, "10100"), + _reading(NOW + 8 * DAY, "10200"), + ] + ) + xs = [point.x for point in series.segments[0].points] + assert xs[0] == Decimal("0") + assert xs[-1] == PLOT_WIDTH + # 1 day into an 8-day span is one eighth across, not one half. + assert xs[1] == (PLOT_WIDTH / 8).quantize(Decimal("0.01")) + + +def test_a_mode_flip_produces_two_segments_not_one_blended_curve() -> None: + """The acceptance criterion this chart is judged on. $10k of paper and $250 of live are two + unrelated accounts; joined, the flip draws a 97.5% collapse that never happened.""" + series = build_equity_series( + [ + _reading(NOW, "10000", mode="paper"), + _reading(NOW + DAY, "10100", mode="paper"), + _reading(NOW + 2 * DAY, "250", mode="live"), + _reading(NOW + 3 * DAY, "260", mode="live"), + ] + ) + assert [segment.mode for segment in series.segments] == ["paper", "live"] + assert [len(segment.points) for segment in series.segments] == [2, 2] + + +def test_a_mode_that_resumes_after_a_flip_is_a_third_segment() -> None: + """Segments follow the ORDER of the readings, not the set of modes: paper, live, paper is + three runs. Grouping by mode alone would join the two paper stretches across the live one + and draw a line through time the account did not spend in paper.""" + series = build_equity_series( + [ + _reading(NOW, "10000", mode="paper"), + _reading(NOW + DAY, "250", mode="live"), + _reading(NOW + 2 * DAY, "10100", mode="paper"), + ] + ) + assert [segment.mode for segment in series.segments] == ["paper", "live", "paper"] + + +def test_the_high_water_mark_overlay_is_read_from_the_rows_not_recomputed() -> None: + """`record_external_flow` REBASES the HWM on a declared deposit, so it is not the running + maximum of the equity series. Recomputing it here would draw a ceiling rail 11 never used -- + and the point of the overlay is to show the ceiling that was actually in force.""" + readings = [ + _reading(NOW, "10000", hwm="10000"), + _reading(NOW + DAY, "9000", hwm="10000"), + # A $5k deposit: equity jumps and the operator declares the flow, so the HWM is rebased + # UP rather than the deposit reading as a recovery. A running maximum would say 14000. + _reading(NOW + 2 * DAY, "14000", hwm="15000"), + ] + series = build_equity_series(readings) + assert [point.hwm for point in series.segments[0].points] == [ + Decimal("10000"), + Decimal("10000"), + Decimal("15000"), + ] + + +def test_the_drawdown_ceiling_is_drawn_beneath_each_points_own_high_water_mark() -> None: + """Rail 11 vetoes entries at `drawdown >= max_total_dd_pct` measured from the HWM, so the + ceiling is a function of the HWM in force at that instant -- it MOVES with the rebase, and a + single horizontal line would be wrong from the first deposit onwards.""" + series = build_equity_series( + [_reading(NOW, "10000", hwm="10000"), _reading(NOW + DAY, "14000", hwm="15000")], + max_total_dd_pct=Decimal("0.20"), + ) + assert [point.dd_floor for point in series.segments[0].points] == [ + Decimal("8000"), + Decimal("12000"), + ] + + +def test_without_a_ceiling_there_is_no_floor_line_rather_than_a_zero_one() -> None: + """A caller that did not supply the rail's setting has not told us the ceiling is zero.""" + series = build_equity_series([_reading(NOW, "10000")]) + assert series.segments[0].points[0].dd_floor is None + + +def test_the_axis_bounds_contain_the_overlays_not_only_the_equity() -> None: + """The ceiling is drawn on this canvas, so it has to fit on it. Bounds taken from the equity + alone would push a floor line off the bottom of the box, where it reads as absent -- the one + thing a rail's ceiling must never look like.""" + series = build_equity_series( + [_reading(NOW, "10000", hwm="12000"), _reading(NOW + DAY, "10500", hwm="12000")], + max_total_dd_pct=Decimal("0.20"), + ) + assert series.low == Decimal("9600") # the floor, below every equity reading + assert series.high == Decimal("12000") # the HWM, above every equity reading + + +def test_zero_is_not_forced_into_the_equity_axis() -> None: + """`build_equity_curve` puts zero on its canvas unconditionally, because it plots net P&L + and zero is the line between making and losing money. Account equity has no such line: an + account is not "up" or "down" against nothing. Forcing zero in would squash every real move + into the top sliver of a box that is mostly empty space below the account.""" + series = build_equity_series([_reading(NOW, "10000"), _reading(NOW + DAY, "10500")]) + assert series.low == Decimal("10000") + assert series.high == Decimal("10500") + + +def test_a_flat_account_is_drawn_on_a_line_not_at_the_edge_of_the_box() -> None: + """Zero span has no range to normalise against. Drawn mid-box: the account did not move, + and pinning it to the top or the bottom would imply it sat at an extreme of something.""" + series = build_equity_series([_reading(NOW, "10000"), _reading(NOW + DAY, "10000")]) + ys = [point.y for point in series.segments[0].points] + assert ys == [(PLOT_HEIGHT / 2).quantize(Decimal("0.01"))] * 2 + + +def test_a_single_reading_sits_at_the_start_of_the_time_axis() -> None: + """One cycle is a real state (the first after upgrading). A zero-width time span must not + divide by zero, and the point belongs at the left edge: it is the beginning of the record, + not the whole of it.""" + series = build_equity_series([_reading(NOW, "10000")]) + point = series.segments[0].points[0] + assert point.x == Decimal("0") + assert series.point_count == 1 + + +def test_y_grows_downward_like_svgs_does() -> None: + """The same convention `EquityPoint` documents: the browser is not allowed to do the one + subtraction that would flip it.""" + series = build_equity_series([_reading(NOW, "10000"), _reading(NOW + DAY, "20000")]) + low_point, high_point = series.segments[0].points + assert low_point.y > high_point.y diff --git a/tests/data/test_db.py b/tests/data/test_db.py index 62f2467..48a8f01 100644 --- a/tests/data/test_db.py +++ b/tests/data/test_db.py @@ -103,11 +103,11 @@ def test_agent_state_table_has_key_primary_key(): assert pk_columns == {"key"} -def test_schema_version_is_18(): +def test_schema_version_is_19(): """Deliberate tripwire: bump this literal consciously on every schema change.""" from keel.data.db import SCHEMA_VERSION - assert SCHEMA_VERSION == 18 + assert SCHEMA_VERSION == 19 def test_a_v6_database_migrates_up_and_gains_the_profile_table(tmp_path): diff --git a/tests/data/test_equity_points.py b/tests/data/test_equity_points.py new file mode 100644 index 0000000..06725c4 --- /dev/null +++ b/tests/data/test_equity_points.py @@ -0,0 +1,177 @@ +"""Storage for the mark-to-market equity series -- issue #698. + +The agent has always computed mark-to-market equity every cycle and always thrown it away: +`agent_state["equity_history"]` is a 7-day rolling window kept for the weekly drawdown rail, and +nothing else persisted. This table is the long-term record, and the tests below pin the two +properties that make it usable as one. + +`mode` is the load-bearing partition. The DB is already one-per-profile (ADR 0002), but paper and +live flip WITHIN one database, and `_clear_live_mode_if_needed` wipes the shared HWM on every +flip precisely because one mode's equity is not the other's. A series that blended them would +draw a cliff between two unrelated accounts and call it a drawdown. + +Money is TEXT holding `str(Decimal(...))`, the standing convention (`db.py` module docstring): +these rows feed a chart whose whole claim is that the numbers are the ones the engine acted on. +""" + +from __future__ import annotations + +from decimal import Decimal + +import pytest +from keel_core.types import EquityReading + +from keel.data.db import connect, migrate +from keel.data.repository import Repository + +NOW = 1_800_000_000 +DAY = 86_400 + + +@pytest.fixture() +def repo() -> Repository: + conn = connect(":memory:") + migrate(conn) + return Repository(conn) + + +def _point( + ts: int = NOW, + mode: str = "paper", + equity: str = "10000.55", + cash: str | None = "9000.25", + unrealized: str | None = "-12.30", + hwm: str = "10500.00", +) -> EquityReading: + return EquityReading( + ts=ts, + mode=mode, + equity=Decimal(equity), + cash=None if cash is None else Decimal(cash), + unrealized=None if unrealized is None else Decimal(unrealized), + hwm=Decimal(hwm), + ) + + +def test_the_schema_carries_the_table() -> None: + conn = connect(":memory:") + migrate(conn) + names = {r["name"] for r in conn.execute("SELECT name FROM sqlite_master WHERE type='table'")} + assert "equity_points" in names + + +def test_a_fresh_database_carries_no_points(repo: Repository) -> None: + """No backfill, by design (`_migrate_v19_equity_points`): the 7-day window is a rail's + working set that `record_external_flow` rewrites, and replaying it would publish equities + the account never had.""" + assert repo.get_equity_points() == [] + + +def test_a_point_round_trips_exactly(repo: Repository) -> None: + repo.record_equity_point(_point()) + assert repo.get_equity_points() == [_point()] + + +def test_money_is_stored_as_exact_decimal_strings(repo: Repository) -> None: + """The standing TEXT convention, checked at the storage layer rather than trusted: a float + column would round 10000.55 and the chart's claim is that these are the numbers the engine + acted on.""" + repo.record_equity_point(_point(equity="10000.55", cash="9000.25", unrealized="-12.30")) + row = repo._conn.execute("SELECT equity, cash, unrealized, hwm FROM equity_points").fetchone() + assert (row["equity"], row["cash"], row["unrealized"], row["hwm"]) == ( + "10000.55", + "9000.25", + "-12.30", + "10500.00", + ) + + +def test_an_unrecorded_split_round_trips_as_none_not_zero(repo: Repository) -> None: + """`None` means the cycle knew its total but not the split. Zero would state a flat cash + position and a flat unrealized P&L, neither of which was observed.""" + repo.record_equity_point(_point(cash=None, unrealized=None)) + got = repo.get_equity_points()[0] + assert got.cash is None + assert got.unrealized is None + assert got.equity == Decimal("10000.55") + + +def test_points_read_back_oldest_first(repo: Repository) -> None: + """A chart draws left to right; ordering at the read keeps every caller from re-sorting.""" + for offset in (2 * DAY, 0, DAY): + repo.record_equity_point(_point(ts=NOW + offset)) + assert [p.ts for p in repo.get_equity_points()] == [NOW, NOW + DAY, NOW + 2 * DAY] + + +def test_a_mode_reads_back_only_its_own_points(repo: Repository) -> None: + """The partition that matters: paper and live share one database and flip within it, and + `_clear_live_mode_if_needed` wipes the shared HWM on every flip for this reason.""" + repo.record_equity_point(_point(ts=NOW, mode="paper", equity="10000")) + repo.record_equity_point(_point(ts=NOW + DAY, mode="live", equity="250")) + assert [p.equity for p in repo.get_equity_points(mode="paper")] == [Decimal("10000")] + assert [p.equity for p in repo.get_equity_points(mode="live")] == [Decimal("250")] + assert len(repo.get_equity_points()) == 2 + + +def test_a_since_window_is_bounded_by_epoch_order_not_string_order(repo: Repository) -> None: + """Guards the `ts INTEGER` choice. As TEXT, "1800086400" < "1800000000" is false but + "999999999" > "1800000000" is true -- a window straddling a digit-count change would drop + or admit the wrong rows, silently.""" + repo.record_equity_point(_point(ts=999_999_999)) + repo.record_equity_point(_point(ts=NOW)) + assert [p.ts for p in repo.get_equity_points(since_ts=NOW)] == [NOW] + + +def test_a_limit_takes_the_MOST_RECENT_readings_still_oldest_first(repo: Repository) -> None: + """A bounded read has to keep the END of the series, not the beginning. The chart's subject + is where the account is now; the first N rows ever written are the least interesting answer + to that, and on a long-running deployment they are also the ones furthest from the truth.""" + for offset in range(5): + repo.record_equity_point(_point(ts=NOW + offset * DAY, equity=str(1000 + offset))) + + got = repo.get_equity_points(limit=2) + + assert [p.ts for p in got] == [NOW + 3 * DAY, NOW + 4 * DAY] + assert [p.equity for p in got] == [Decimal("1003"), Decimal("1004")] + + +def test_a_limit_larger_than_the_table_returns_everything(repo: Repository) -> None: + repo.record_equity_point(_point(ts=NOW)) + assert len(repo.get_equity_points(limit=500)) == 1 + + +def test_a_limit_composes_with_the_mode_partition(repo: Repository) -> None: + """The limit must be applied WITHIN the mode, not to a blended read that is then filtered -- + otherwise asking for the last two live readings on a paper-heavy database returns nothing.""" + for offset in range(5): + repo.record_equity_point(_point(ts=NOW + offset * DAY, mode="paper")) + for offset in range(5, 8): + repo.record_equity_point(_point(ts=NOW + offset * DAY, mode="live", equity="250")) + + got = repo.get_equity_points(mode="live", limit=2) + + assert [p.ts for p in got] == [NOW + 6 * DAY, NOW + 7 * DAY] + + +def test_the_recorded_count_is_readable_without_reading_the_rows(repo: Repository) -> None: + """What lets a bounded chart say how much it is NOT showing. A count is the whole reason the + truncation can be stated honestly instead of the series quietly starting wherever the cap + happened to fall.""" + for offset in range(7): + repo.record_equity_point(_point(ts=NOW + offset * DAY)) + + assert repo.count_equity_points() == 7 + assert repo.count_equity_points(mode="live") == 0 + + +def test_an_existing_database_gains_the_table_on_migration() -> None: + conn = connect(":memory:") + migrate(conn) + # Literal 18, not `SCHEMA_VERSION - 1`: every version pin in tests/data/ is a literal + # precisely so a bump is acknowledged rather than silently absorbed. + conn.execute("UPDATE schema_version SET version = 18") + conn.execute("DROP TABLE equity_points") + conn.commit() + migrate(conn) + names = {r["name"] for r in conn.execute("SELECT name FROM sqlite_master WHERE type='table'")} + assert "equity_points" in names diff --git a/tests/data/test_migrations.py b/tests/data/test_migrations.py index b6ee842..ddb9b22 100644 --- a/tests/data/test_migrations.py +++ b/tests/data/test_migrations.py @@ -49,7 +49,7 @@ def test_fresh_database_is_stamped_at_the_current_version() -> None: conn = db.connect(":memory:") db.migrate(conn) version = conn.execute("SELECT version FROM schema_version").fetchone()["version"] - assert version == db.SCHEMA_VERSION == 18 + assert version == db.SCHEMA_VERSION == 19 def test_fresh_database_gets_no_subscription_row() -> None: @@ -612,7 +612,7 @@ def test_v14_migration_bumps_the_stored_version() -> None: conn = _v12_database() db.migrate(conn) stamped = conn.execute("SELECT version FROM schema_version").fetchone()["version"] - assert stamped == db.SCHEMA_VERSION == 18 + assert stamped == db.SCHEMA_VERSION == 19 def test_v14_migration_step_is_not_blocked_by_another_venues_existing_row() -> None: @@ -773,7 +773,7 @@ def test_v15_migration_bumps_the_stored_version() -> None: conn = _v12_database() db.migrate(conn) stamped = conn.execute("SELECT version FROM schema_version").fetchone()["version"] - assert stamped == db.SCHEMA_VERSION == 18 + assert stamped == db.SCHEMA_VERSION == 19 def test_v15_the_12_to_15_chain_creates_the_table_with_the_column_already_present() -> None: @@ -875,7 +875,7 @@ def test_an_existing_orders_table_gains_the_submit_book_by_ALTER() -> None: assert row["submit_best_bid"] is None assert row["submit_best_ask"] is None stamped = conn.execute("SELECT version FROM schema_version").fetchone()["version"] - assert stamped == db.SCHEMA_VERSION == 18 + assert stamped == db.SCHEMA_VERSION == 19 def test_v16_is_idempotent_per_column() -> None: diff --git a/tests/data/test_trade_outcomes.py b/tests/data/test_trade_outcomes.py index 3c1945d..6c2052f 100644 --- a/tests/data/test_trade_outcomes.py +++ b/tests/data/test_trade_outcomes.py @@ -35,11 +35,11 @@ def _outcome(**overrides: object) -> dict: return base -def test_schema_is_at_version_18() -> None: +def test_schema_is_at_version_19() -> None: conn = db.connect(":memory:") db.migrate(conn) version = conn.execute("SELECT version FROM schema_version").fetchone()["version"] - assert version == db.SCHEMA_VERSION == 18 + assert version == db.SCHEMA_VERSION == 19 def test_fresh_database_has_no_outcomes() -> None: diff --git a/tests/execution/test_equity.py b/tests/execution/test_equity.py index ce7e560..aab6b7f 100644 --- a/tests/execution/test_equity.py +++ b/tests/execution/test_equity.py @@ -238,3 +238,95 @@ def test_pending_purification_usd_is_zero_on_a_clean_ledger() -> None: repo.upsert_transaction(_reward_tx("cl1", "500", tx_type="Buy")) assert equity.pending_purification_usd(repo) == Decimal("0") + + +# -- the persisted series (#698) ------------------------------------------------------------ +# +# `equity_history` in `agent_state` is a 7-day window kept for the WEEKLY rail, and +# `record_external_flow` rewrites every point in it on a declared deposit. It is a rail's +# working set, so it cannot double as the record. These tests pin the durable one. + + +def test_a_cycle_appends_one_point_to_the_series() -> None: + repo = _repo() + repo.set_state("equity_state_mode", "paper") + equity.update_drawdown(repo, equity=Decimal("10000"), now_ts=NOW) + points = repo.get_equity_points() + assert len(points) == 1 + assert points[0].ts == NOW + assert points[0].equity == Decimal("10000") + + +def test_the_point_carries_the_high_water_mark_in_force_after_this_reading() -> None: + """The chart's rail-11 overlay reads `hwm` off the row rather than recomputing a running + maximum, because the two are not the same series: `record_external_flow` REBASES the HWM on + a declared deposit, and a recomputed maximum would draw a ceiling the rail never used.""" + repo = _repo() + repo.set_state("equity_state_mode", "paper") + equity.update_drawdown(repo, equity=Decimal("12000"), now_ts=NOW) + equity.update_drawdown(repo, equity=Decimal("9000"), now_ts=NOW + DAY) + assert [p.hwm for p in repo.get_equity_points()] == [Decimal("12000"), Decimal("12000")] + + +def test_the_point_is_stamped_with_the_mode_that_produced_it() -> None: + """`equity_state_mode` is the same stamp `_clear_live_mode_if_needed` reads before wiping + the shared HWM on a flip. Deriving the mode a second way at the call site would let the two + disagree, and a mislabelled row is worse than a missing one -- it lands in the wrong curve.""" + repo = _repo() + repo.set_state("equity_state_mode", "live") + equity.update_drawdown(repo, equity=Decimal("250"), now_ts=NOW) + assert [p.mode for p in repo.get_equity_points()] == ["live"] + + +def test_a_mode_flip_leaves_two_series_not_one_blended_curve() -> None: + """The whole reason for the `mode` column. Paper equity of $10k and live equity of $250 are + two unrelated accounts; joined into one line the flip reads as a 97.5% drawdown that never + happened.""" + repo = _repo() + repo.set_state("equity_state_mode", "paper") + equity.update_drawdown(repo, equity=Decimal("10000"), now_ts=NOW) + # What the agent does on a flip: the shared scalars are cleared, then the stamp changes. + repo.set_state("equity_high_water_mark", None) + repo.set_state("equity_history", []) + repo.set_state("equity_state_mode", "live") + equity.update_drawdown(repo, equity=Decimal("250"), now_ts=NOW + DAY) + + assert [p.equity for p in repo.get_equity_points(mode="paper")] == [Decimal("10000")] + assert [p.equity for p in repo.get_equity_points(mode="live")] == [Decimal("250")] + + +def test_the_split_is_recorded_when_the_caller_knows_it() -> None: + repo = _repo() + repo.set_state("equity_state_mode", "paper") + equity.update_drawdown( + repo, + equity=Decimal("10000"), + now_ts=NOW, + cash=Decimal("9000"), + unrealized=Decimal("-25.50"), + ) + point = repo.get_equity_points()[0] + assert point.cash == Decimal("9000") + assert point.unrealized == Decimal("-25.50") + + +def test_an_unknown_split_is_recorded_as_none_not_zero() -> None: + """A caller that knows only the total says so. Zero would assert a flat cash balance and a + flat unrealized P&L, and nothing observed either.""" + repo = _repo() + repo.set_state("equity_state_mode", "paper") + equity.update_drawdown(repo, equity=Decimal("10000"), now_ts=NOW) + point = repo.get_equity_points()[0] + assert point.cash is None + assert point.unrealized is None + + +def test_an_unstamped_mode_writes_no_point_and_still_updates_the_rail() -> None: + """Rail 11 must never be held hostage to the chart. The agent stamps the mode before every + `update_drawdown` (both branches do, unconditionally), so an unstamped call is not a real + cycle -- and a row labelled with a guessed mode would land in the wrong curve, which is the + one failure the partition exists to prevent. The scalars still advance.""" + repo = _repo() + equity.update_drawdown(repo, equity=Decimal("10000"), now_ts=NOW) + assert repo.get_equity_points() == [] + assert repo.get_state("equity_high_water_mark") == Decimal("10000") diff --git a/tests/execution/test_paper_equity.py b/tests/execution/test_paper_equity.py index 873a7d2..0c18c77 100644 --- a/tests/execution/test_paper_equity.py +++ b/tests/execution/test_paper_equity.py @@ -6,7 +6,7 @@ from decimal import Decimal -from keel.execution.equity import mark_positions +from keel.execution.equity import mark_positions, unrealized_on_marks def test_mark_positions_uses_fresh_price(): @@ -49,6 +49,91 @@ def test_mark_positions_skips_non_positive_qty(): assert eq == Decimal("1000") +# -- the unrealized leg of the same reading (#698) -------------------------------------------- +# +# `equity_points.unrealized` is written from here, so it MUST answer the marks `mark_positions` +# used for the SAME cycle: two helpers reading one set of positions under different fallback +# rules would file an equity and a P&L that cannot both be true. + + +def test_unrealized_is_the_gain_over_cost_at_the_fresh_price(): + pnl = unrealized_on_marks( + positions=[(Decimal("2"), Decimal("100"))], + price_by_product={"BTC-USD": Decimal("150")}, + product_ids=["BTC-USD"], + ) + assert pnl == Decimal("2") * (Decimal("150") - Decimal("100")) + + +def test_unrealized_is_zero_when_the_price_is_missing(): + """The cost-basis fallback, restated in P&L terms: `mark_positions` values that position AT + cost, so the only unrealized figure consistent with the equity it just reported is zero. + Anything else books a gain against a price nobody observed.""" + pnl = unrealized_on_marks( + positions=[(Decimal("2"), Decimal("100"))], + price_by_product={}, + product_ids=["BTC-USD"], + ) + assert pnl == Decimal("0") + + +def test_unrealized_is_zero_when_the_price_is_non_positive(): + pnl = unrealized_on_marks( + positions=[(Decimal("2"), Decimal("100"))], + price_by_product={"BTC-USD": Decimal("0")}, + product_ids=["BTC-USD"], + ) + assert pnl == Decimal("0") + + +def test_unrealized_skips_non_positive_qty(): + pnl = unrealized_on_marks( + positions=[(Decimal("0"), Decimal("100"))], + price_by_product={"BTC-USD": Decimal("150")}, + product_ids=["BTC-USD"], + ) + assert pnl == Decimal("0") + + +def test_unrealized_is_negative_on_a_losing_position(): + pnl = unrealized_on_marks( + positions=[(Decimal("2"), Decimal("100"))], + price_by_product={"BTC-USD": Decimal("90")}, + product_ids=["BTC-USD"], + ) + assert pnl == Decimal("-20") + + +def test_a_zero_cost_basis_position_is_all_unrealized_gain(): + """`mark_positions` values a fresh-priced position at `qty * mark` regardless of what it + cost, so a zero-basis holding (an airdrop, a migrated row with no recorded fill) is entirely + unrealized gain. Skipping it here would leave `cash + cost + unrealized` short of the equity + reported for the same cycle.""" + pnl = unrealized_on_marks( + positions=[(Decimal("2"), Decimal("0"))], + price_by_product={"BTC-USD": Decimal("150")}, + product_ids=["BTC-USD"], + ) + assert pnl == Decimal("300") + + +def test_cash_plus_cost_basis_plus_unrealized_reconstructs_the_equity(): + """The invariant that makes three columns one reading rather than three: a row whose parts + do not add back to `equity` cannot be reconciled, and the chart's whole claim is that these + are the numbers the engine acted on. The second position is deliberately unpriced, so the + identity is checked across BOTH the fresh-price and the fallback leg.""" + positions = [(Decimal("2"), Decimal("100")), (Decimal("5"), Decimal("20"))] + products = ["BTC-USD", "ETH-USD"] + prices = {"BTC-USD": Decimal("150")} + cash = Decimal("1000") + + equity = mark_positions(cash, positions, prices, products) + unrealized = unrealized_on_marks(positions, prices, products) + cost_basis_total = sum((qty * basis for qty, basis in positions), Decimal("0")) + + assert cash + cost_basis_total + unrealized == equity + + def test_mark_positions_with_no_positions_returns_cash(): eq = mark_positions(cash=Decimal("1000"), positions=[], price_by_product={}, product_ids=[]) assert eq == Decimal("1000") diff --git a/tests/strategy/test_paper.py b/tests/strategy/test_paper.py index 3ced098..70dea9c 100644 --- a/tests/strategy/test_paper.py +++ b/tests/strategy/test_paper.py @@ -506,6 +506,52 @@ def test_paper_equity_seed_and_mark(repo): assert eq > Decimal("29000") +def test_paper_unrealized_is_none_under_exactly_the_same_condition_as_equity(repo): + """Unseeded means there is no synthetic account to mark. The two must answer `None` + together, or `equity_points` would file a total with a split from a different account + state (#698).""" + trader = PaperTrader(repo) + assert trader.equity({"BTC-USD": Decimal("100")}) is None + assert trader.unrealized({"BTC-USD": Decimal("100")}) is None + + +def test_paper_unrealized_and_cash_reconcile_against_the_marked_equity(repo): + """The row-level invariant on the paper side: `cash + cost basis + unrealized == equity`, + all four read off one account state. The fill price is whatever slippage and fees made it, + which is exactly why the cost basis is read back from the trader rather than assumed.""" + trader = PaperTrader(repo) + trader.seed_cash(Decimal("30000"), now_ts=1_700_000_000) + trader.on_signal( + _enter_signal(setup=_setup(entry="100", stop="90", target="130")), qty=Decimal("5") + ) + + marks = {"BTC-USD": Decimal("120")} + equity = trader.equity(marks) + cash = trader.get_cash() + unrealized = trader.unrealized(marks) + + entry_fill = repo.get_orders(mode="paper")[0]["actual_fill"] + cost_basis = Decimal("5") * entry_fill + + assert unrealized == Decimal("5") * (Decimal("120") - entry_fill) + assert cash + cost_basis + unrealized == equity + + +def test_paper_unrealized_excludes_a_position_nothing_paid_for(repo): + """`equity()` drops an uncosted position (opened before cash was seeded) because marking it + would inflate equity with a position nothing paid for. Its gain has to be dropped for the + same reason -- otherwise the split reports a P&L the equity does not contain.""" + trader = PaperTrader(repo) + trader.on_signal( + _enter_signal(setup=_setup(entry="100", stop="90", target="130")), qty=Decimal("5") + ) + trader.seed_cash(Decimal("30000"), now_ts=1_700_000_000) + + marks = {"BTC-USD": Decimal("120")} + assert trader.equity(marks) == Decimal("30000") + assert trader.unrealized(marks) == Decimal("0") + + def test_paper_funding_check_rejects_when_cash_insufficient(repo): trader = PaperTrader(repo) trader.seed_cash(Decimal("50"), now_ts=1_700_000_000) diff --git a/tests/test_agent.py b/tests/test_agent.py index 4c5ef75..b75e231 100644 --- a/tests/test_agent.py +++ b/tests/test_agent.py @@ -1393,6 +1393,118 @@ def get_balances(self) -> list[Balance]: assert repo.get_state("drawdown_total_pct") == Decimal("0.3") +# -- the persisted equity series (#698) -------------------------------------------------------- + + +def test_a_live_cycle_records_the_equity_point_with_its_cash_and_unrealized( + repo: Repository, +) -> None: + """The agent already computed all three every cycle and persisted none of them. The point + has to carry the SPLIT, not just the total: a chart that can only draw one line cannot show + whether an equity move was a position marking up or cash arriving.""" + _seed_open_position(repo, PRODUCT, Decimal("2"), Decimal("100"), ts=1_000) + series = {(PRODUCT, Granularity.ONE_DAY): [_candle(1_000 + i * 86_400) for i in range(30)]} + broker = FakeBroker(series=series) + now = 1_000 + 29 * 86_400 + + run_once(broker, repo, _config(), now_ts=now) + + points = repo.get_equity_points() + assert len(points) == 1 + point = points[0] + assert point.mode == "live" + assert point.ts == now + # FakeBroker's cash, and 2 units bought at 100 now marked at the candle close of 100. + assert point.cash == Decimal("1000000") + assert point.unrealized == Decimal("0") + assert point.equity == Decimal("1000000") + Decimal("2") * Decimal("100") + + +def test_a_paper_cycle_records_the_split_off_the_synthetic_account(repo, monkeypatch) -> None: + """The paper branch's wiring, through the real loop rather than on `PaperTrader` alone. + + The two are different code: `tests/strategy/test_paper.py` proves `get_cash()` and + `unrealized()` agree with `equity()` on one account state, and this proves the AGENT hands + those two to `update_drawdown` for the same cycle it read the equity from -- a paper account + that seeds itself DURING this cycle is exactly where the total and the split could come from + two different states of the account. + """ + from keel.strategy.paper import PaperTrader + + _seed_rule(repo, monkeypatch, _AlwaysEnterRule(PRODUCT), status="paper") + cfg = _paper_config(paper=PaperConfig(starting_equity_usd=Decimal("10000"))) + + trader = PaperTrader(repo) + trader.seed_cash(Decimal("10000"), now_ts=0) + repo.set_state("equity_state_mode", "paper") + trader.on_signal( + _paper_enter_signal( + product_id=PRODUCT, entry=Decimal("100"), stop=Decimal("50"), target=Decimal("200"), + ts=0, + ), + qty=Decimal("10"), + ) + + broker = _MarketDataOnlyBroker( + series={(PRODUCT, Granularity.ONE_DAY): [_candle(0, "100"), _candle(86_400, "120")]} + ) + repo.set_state("kill_switch", False) + repo.set_state("last_feed_ts", 86_400) + + run_once(broker, repo, cfg, now_ts=86_400) + + point = repo.get_equity_points()[0] + assert point.mode == "paper" + assert point.cash == trader.get_cash() + assert point.cash is not None and point.unrealized is not None + # The reconciliation invariant, on the paper side and through the loop: the recorded parts + # add back to the recorded total. The entry fill is whatever slippage and fees made it, so + # the cost basis is read back rather than assumed. + entry_fill = repo.get_orders(mode="paper")[0]["actual_fill"] + assert point.cash + Decimal("10") * entry_fill + point.unrealized == point.equity + + +def test_the_live_split_reconciles_against_the_equity_it_was_read_with(repo: Repository) -> None: + """`cash + cost basis + unrealized == equity`, on ONE read of the account. + + Called directly with an explicit price map, like its neighbours below: through `run_once` + the map only covers products with a LIVE RULE, so a held-only position takes the cost-basis + fallback and the marked-up case -- the one where the parts can disagree -- never arises. + """ + _seed_open_position(repo, PRODUCT, Decimal("2"), Decimal("100"), ts=1_000) + broker = FakeBroker() + + parts = agent._mark_to_market_parts( + repo, broker, [PRODUCT], {PRODUCT: Decimal("150")}, "USD" + ) + + assert parts is not None + cost_basis = Decimal("2") * Decimal("100") + assert parts.unrealized == Decimal("2") * (Decimal("150") - Decimal("100")) + assert parts.cash + cost_basis + parts.unrealized == parts.equity + # And the total is exactly what the existing scalar path reports for the same read. + assert parts.equity == agent._mark_to_market_equity( + repo, broker, [PRODUCT], {PRODUCT: Decimal("150")}, "USD" + ) + + +def test_an_unreadable_equity_records_no_point(repo: Repository) -> None: + """`_mark_to_market_equity` returns `None` when NO balance could be read -- equity is + genuinely unknown, and the cycle already declines to touch rail 11's scalars for exactly + that reason. The series must decline too: a gap in the chart is honest, a point carrying + last cycle's number dressed as this cycle's reading is not.""" + + class _MuteBroker(FakeBroker): + def get_balances(self) -> list[Balance]: + return [] + + series = {(PRODUCT, Granularity.ONE_DAY): [_candle(1_000 + i * 86_400) for i in range(30)]} + + run_once(_MuteBroker(series=series), repo, _config(), now_ts=1_000 + 29 * 86_400) + + assert repo.get_equity_points() == [] + + # -- rail 11: equity must be valued from the ORDERS LOG, not from position_rule ---------------- diff --git a/tests/web/test_client_assets.py b/tests/web/test_client_assets.py index 7c5e42f..639ba10 100644 --- a/tests/web/test_client_assets.py +++ b/tests/web/test_client_assets.py @@ -1330,3 +1330,87 @@ def test_the_status_key_scan_found_the_keys_it_claims_to_check() -> None: # And the scan is bounded: `data.build` belongs to `/api/config` and is read by `buildLine`, # which lives BELOW `statusView` in the same file. assert "build" not in keys, keys + + +# -- the account-equity series, stacked above the curve (#698) --------------------------------- + + +def test_paper_and_live_are_told_apart_by_dash_not_only_by_colour() -> None: + """The same constraint the losing segment above answers, for the other pair of lines a reader + has to tell apart. Paper and live are unrelated accounts drawn on one canvas, so if the only + difference between their lines were a hue, a reader with red-green colour deficiency -- or + anyone on e-ink or a greyscale printout -- would see one continuous account.""" + css = staticfiles.STATIC_ROOT.joinpath("css", "keel.css").read_text(encoding="utf-8") + paper_rule = re.search(r"\.chart svg\.series \.line\.paper\s*\{([^}]*)\}", css) + assert paper_rule is not None, "the paper equity line has no rule of its own" + assert "stroke-dasharray" in paper_rule.group(1), ( + "the paper line must be dashed so the synthetic account survives greyscale" + ) + # And live must NOT be dashed, or dash stops being the thing that separates them. + live_rule = re.search(r"\.chart svg\.series \.line\.live\s*\{([^}]*)\}", css) + assert live_rule is None or "stroke-dasharray" not in live_rule.group(1), ( + "the live line must stay solid -- if both are dashed, dash is no longer the signal" + ) + + +def test_the_series_canvas_does_not_answer_to_the_curves_selector() -> None: + """`main.js`'s #602 wheel-zoom, drag-to-pan and cursor legend bind to `svg.curve` and reach + for it with `querySelector`, which takes the FIRST match in the DOM. The series is rendered + ABOVE the curve, so if it also called itself `curve` every one of those gestures would + silently retarget onto a chart they were not written for.""" + chart_code = _source("chart.js") + assert 'class: "series"' in chart_code, "the series canvas must not be class 'curve'" + + render_code = _source("render.js") + series_at = render_code.index("equitySeriesChart(") + curve_at = render_code.index("equityChart(journal.curve") + assert series_at < curve_at, "the equity series is stacked ABOVE the closed-trade curve" + + +def test_the_series_figure_does_not_intercept_the_curves_figure_lookups() -> None: + """The collision that `svg.series` alone does NOT fix, and that cost the journal-row + highlight once already. + + `main.js` reaches for the chart's WRAPPER with `contentNode.querySelector("figure.chart")` + in two places -- `highlightJournalRow` and the chart-action handler -- and that takes the + first match in document order. The series figure is appended ABOVE the curve, so if it were + a bare `figure.chart` those lookups would land on it; `highlightTrade` would then find no + `.highlight` group, return early, and hovering a journal row would silently do nothing. + Both halves are pinned here: the class the series wears, and the selector that excludes it. + """ + chart_code = _source("chart.js") + assert 'figure.className = "chart series"' in chart_code, ( + "the series figure must be distinguishable from the curve's figure by class" + ) + + main_code = _source("main.js") + assert 'querySelector("figure.chart")' not in main_code, ( + "a bare figure.chart lookup takes the series figure, which renders first" + ) + assert main_code.count('querySelector("figure.chart:not(.series)")') == 2, ( + "both figure lookups in main.js must exclude the series figure" + ) + + +def test_the_series_draws_each_mode_as_its_own_polyline() -> None: + """The one thing this chart must never do is join two accounts into one line. It is checked + on the source rather than a rendered DOM because a browser cannot run here: the loop over + `segments` IS the guarantee, and a `points` attribute built from a flattened list would be + the bug.""" + chart_code = _source("chart.js") + assert "for (const segment of series.segments)" in chart_code, ( + "the series must be drawn one segment at a time, never as one flat list of points" + ) + assert ".flat(" not in chart_code and "concat(" not in chart_code, ( + "flattening the segments would draw the paper/live flip as a single continuous line" + ) + + +def test_an_unknown_drawdown_ceiling_draws_no_floor_line() -> None: + """`dd_floor_y` is `null` when the rail setting is unknown, and a `null` coordinate must be + filtered out rather than drawn: SVG's zero is the TOP of the box, so a floor placed there + reads as a ceiling in force above every reading -- the opposite of "not known".""" + chart_code = _source("chart.js") + assert "point.dd_floor_y !== null" in chart_code, ( + "points with no recorded drawdown floor must be filtered before the floor is drawn" + ) diff --git a/tests/web/test_payload.py b/tests/web/test_payload.py index d999333..12cbd69 100644 --- a/tests/web/test_payload.py +++ b/tests/web/test_payload.py @@ -37,6 +37,7 @@ from typing import Any import pytest +from keel_core.types import EquityReading from keel.capabilities import CAPABILITIES, GATES from keel.commands.activity import ActivityCycle, ActivityEvent, ActivityFeed @@ -48,6 +49,7 @@ JournalReport, RuleTrackRecord, build_equity_curve, + build_equity_series, ) from keel.commands.jobs import JobStatus from keel.commands.setup import ACTIONS, STEPS, DeploymentState, StepState @@ -399,7 +401,12 @@ def _every_payload() -> dict[str, Any]: second copy to drift.""" return { "status": payload.status_payload(_status_report()), - "insights": payload.insights_payload(_insights_report()), + "insights": payload.insights_payload( + _insights_report(), + series=build_equity_series( + _equity_readings(), max_total_dd_pct=Decimal("0.20") + ), + ), "journal": _journal_json(), "activity": payload.activity_payload(_activity_feed()), "config": payload.config_payload( @@ -805,7 +812,9 @@ 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()) + built = payload.insights_payload( + _insights_report(), series=build_equity_series(_equity_readings()) + ) assert built["rules"][0]["win_rate"]["value"] == "41.5" assert built["rules"][0]["win_rate"]["display"] == "41.5%" @@ -1316,7 +1325,10 @@ def test_every_builder_survives_a_completely_empty_report() -> None: ) documents = { "status": payload.status_payload(empty_status), - "insights": payload.insights_payload(_insights_report(rules=[], closed_trade_count=0)), + "insights": payload.insights_payload( + _insights_report(rules=[], closed_trade_count=0), + series=build_equity_series([]), + ), "journal": _journal_json(entries=[], total_count=0, filters={}), "activity": payload.activity_payload( ActivityFeed(status="missing", source="/tmp/nope.log") @@ -1336,6 +1348,16 @@ def test_every_payload_is_json_serialisable_without_a_custom_encoder(builder: st 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.""" + if builder == "insights_payload": + # A second builder with a required keyword, for the same reason: a default + # `series` would serve an insights view with no chart. + json.dumps( + payload.insights_payload( + _insights_report(), series=build_equity_series(_equity_readings()) + ) + ) + return + if builder == "journal_payload": # The one builder with a second required argument. Spelled out rather than folded into # the table below, because folding it in would mean a default somewhere -- and the whole @@ -1346,8 +1368,178 @@ def test_every_payload_is_json_serialisable_without_a_custom_encoder(builder: st other = { "status_payload": _status_report(), - "insights_payload": _insights_report(), "activity_payload": _activity_feed(), }[builder] json.dumps(getattr(payload, builder)(other)) # no cls=, no default= + + +# -- the account-equity series (#698) --------------------------------------------------------- +# +# Serialised beside the closed-trade curve, never instead of it: the two answer different +# questions (what the ACCOUNT was worth, whichever cycles ran; what the closed TRADES did, in the +# order they closed). The rules below are the ones the contract already applies to the curve -- +# money as strings, coordinates bare, judgements written here -- plus the one this chart adds: +# a mode is a partition, and the payload must never let a client join across it. + + +def _equity_readings() -> list[EquityReading]: + """Two modes, a rebased high-water mark, and a cycle whose split was never recorded.""" + return [ + EquityReading( + ts=NOW_TS - 3 * 86_400, + mode="paper", + equity=Decimal("10000.55"), + cash=Decimal("9000.25"), + unrealized=Decimal("-12.30"), + hwm=Decimal("10500.00"), + ), + EquityReading( + ts=NOW_TS - 2 * 86_400, + mode="paper", + equity=Decimal("10600"), + cash=None, + unrealized=None, + hwm=Decimal("10600"), + ), + EquityReading( + ts=NOW_TS, + mode="live", + equity=Decimal("250.10"), + cash=Decimal("250.10"), + unrealized=Decimal("0"), + hwm=Decimal("250.10"), + ), + ] + + +def _series_json(max_total_dd_pct: Decimal | None = Decimal("0.20")) -> dict[str, Any]: + return payload.equity_series_payload( + build_equity_series(_equity_readings(), max_total_dd_pct=max_total_dd_pct) + ) + + +def test_the_series_crosses_as_one_run_of_points_per_mode() -> None: + """The partition, on the wire. Two paper cycles then a live one is two segments, and a client + handed one flat list would draw a line from $10,600 of paper money to $250 of real money and + call the drop a drawdown.""" + modes = [segment["mode"] for segment in _series_json()["segments"]] + counts = [len(segment["points"]) for segment in _series_json()["segments"]] + + assert modes == ["paper", "live"] + assert counts == [2, 1] + + +def test_a_segments_mode_is_a_bare_string_not_a_field() -> None: + """`mode` is an identifier, like `product_id` and `rule_name`: no precision hazard, no + rounding, no judgement. WHAT IS NOT A FIELD, in the module docstring.""" + assert _series_json()["segments"][0]["mode"] == "paper" + + +def test_coordinates_are_bare_strings_and_the_figures_beside_them_are_fields() -> None: + """The same split `_equity_point_payload` documents: `x`/`y` are positions inside a viewBox, + with nothing to format and no judgement to carry, while everything a reader is TOLD arrives + as a field.""" + point = _series_json()["segments"][0]["points"][0] + + assert point["x"] == "0.00" + assert isinstance(point["y"], str) + assert point["equity"]["display"] == "$10,000.55" + assert point["equity"]["value"] == "10000.55" + assert point["at"]["value"].endswith("Z") + + +def test_an_unrecorded_split_crosses_as_absent_not_as_zero() -> None: + """A cycle that knew its total but not its split. `$0.00` would state a flat cash balance + and a flat unrealized P&L, and neither was observed -- the same distinction `pnl_net` draws + for a trade with no recorded net.""" + point = _series_json()["segments"][0]["points"][1] + + assert point["cash"]["state"] == "unknown" + assert point["unrealized"]["state"] == "unknown" + assert point["equity"]["state"] == "neutral" + + +def test_the_unrealized_leg_carries_its_own_sign_as_a_state() -> None: + """Rule 3: a client must never decide "this is bad" from a minus sign. Unrealized P&L is a + gain-or-loss figure, so it is the one leg of the split that carries a verdict.""" + point = _series_json()["segments"][0]["points"][0] + + assert point["unrealized"]["state"] == "bad" + assert point["unrealized"]["display"].startswith("\u25bc") + + +def test_the_drawdown_floor_is_absent_rather_than_zero_when_the_rail_is_unknown() -> None: + """A caller that did not supply `max_total_dd_pct` has not said the ceiling is zero. The + coordinate goes `null` for the same reason: a `"0"` would place the line at the top of the + box, where it reads as a ceiling in force.""" + point = _series_json(max_total_dd_pct=None)["segments"][0]["points"][0] + + assert point["dd_floor"]["state"] == "unknown" + assert point["dd_floor_y"] is None + + +def test_the_drawdown_floor_crosses_with_a_coordinate_when_the_rail_is_known() -> None: + point = _series_json()["segments"][0]["points"][0] + + # 10500.00 * (1 - 0.20), exact: the payload never rounds a source Decimal. + assert point["dd_floor"]["value"] == "8400.0000" + assert point["dd_floor"]["display"] == "$8,400.00" + assert isinstance(point["dd_floor_y"], str) + + +def test_the_reading_says_what_the_chart_shows_including_the_flip() -> None: + """The chart's text equivalent, written HERE for the reason every other sentence on this wire + is. It has to name the mode split: a reader who cannot see the two separate lines is + otherwise told a single account went from ten thousand dollars to two hundred.""" + reading = _series_json()["reading"] + + assert "paper" in reading["display"] + assert "live" in reading["display"] + assert reading["state"] in ("good", "warn", "bad", "neutral", "unknown") + + +def test_a_single_mode_is_not_told_it_is_two_separate_accounts() -> None: + """The sentence explaining the split is what a reader who cannot see the chart is told about + the mode partition -- so it must not be said when there is no partition. A deployment that + has only ever run paper hears "they are separate accounts" and goes looking for a second line + that is not there, which is the same false continuity the segments prevent, inverted.""" + only_paper = payload.equity_series_payload(build_equity_series(_equity_readings()[:2])) + + assert "paper" in only_paper["reading"]["display"] + assert "separate accounts" not in only_paper["reading"]["display"] + # And the two-mode case still explains itself. + assert "separate accounts" in _series_json()["reading"]["display"] + + +def test_a_truncated_series_says_how_much_it_is_not_showing() -> None: + """A bounded read reaches the reader as a bounded read. The sentence is the only place a + screen-reader user is told the chart is a window rather than the whole record -- and a + sighted reader gets the same words, so neither is told a span that is not there.""" + truncated = payload.equity_series_payload( + build_equity_series(_equity_readings(), total_recorded=900) + ) + + assert "most recent" in truncated["reading"]["display"] + assert truncated["total_recorded"]["value"] == "900" + assert truncated["is_truncated"]["value"] == "true" + + +def test_a_complete_series_makes_no_truncation_claim() -> None: + complete = payload.equity_series_payload( + build_equity_series(_equity_readings(), total_recorded=3) + ) + + assert "most recent" not in complete["reading"]["display"] + assert complete["is_truncated"]["value"] == "false" + + +def test_an_empty_series_says_so_rather_than_describing_an_empty_chart() -> None: + """Not "the account was flat", and not an empty string. A deployment that has not completed a + cycle since the upgrade has no readings at all, and which of those two a reader is looking at + is the whole difference between an empty chart and a broken one.""" + empty = payload.equity_series_payload(build_equity_series([])) + + assert empty["segments"] == [] + assert empty["point_count"]["value"] == "0" + assert "no" in empty["reading"]["display"].lower()