From 09982b4052e8cac93d905d3cc497e6ee389a8572 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Fri, 4 Sep 2026 14:20:08 -0400 Subject: [PATCH 1/2] feat(web): the Balances view, from what a cycle recorded (#702) #702 specified "a live adapter read". That was refused, deliberately and on the record. `keel serve` is a loopback reader over SQLite with no credentials, no broker handle and no outbound network -- `gather_status` says "no broker, no network", and even the Venues screen reads adapter declarations with "no broker handle, no network, no config, no credentials". A balances page is the obvious place to break that line, and breaking it would put venue credentials into the one process a browser can reach and hand an operator's rate limit to every tab left open on a view that re-polls every 15 seconds (`main.js`'s `POLL_MS`). Hundreds of REST calls an hour, during trading windows, for a number nobody asked to refresh. What replaced it is better rather than merely safer. Cash comes from `equity_points` (#698): the figure the agent READ AND SIZED AGAINST when it last evaluated the rails, stamped with when it read it. A fresher number the engine never saw explains nothing about why it did what it did. READ THROUGH THE MODE, ALWAYS. `equity_points` holds paper and live rows in one database, so the newest row FOR THE MODE IN FORCE is the answer -- the newest row of any mode would show a $10,000 synthetic balance on a live deployment holding $250. `paper_cash_usdc` is withheld entirely in live mode even though the key survives a paper->live flip: a synthetic balance beside real money is the most confusing thing this page could show. An unstamped mode, before the first cycle, yields no cash rather than a guess about which account to read. WHAT IS NOT RECORDED IS NAMED. #702 was written around settled-versus-total cash, and that distinction is not in the database: `equity_points.cash` comes from `_fetch_available_quote`, which reads `Balance.available` and stops, summed across every currency in play -- so the Alpaca adapter's T+1 clamp reaches storage with the distinction already flattened out of it. The tiles render as absent with `UNRECORDED IN CYCLE SNAPSHOT` beside them, rather than showing the available figure under a "settled" label that would answer the question this page exists to ask honestly. #719 records the pair; the report and payload were shaped so that lands as a value change, not a shape change. A PARTIAL SUM IS THE DANGEROUS SHAPE. An asset whose tranches are not all priced reports its QUANTITY and no value. Summing over the priced subset would look like a total and be a smaller one -- and a holding rendered as worth less than it is, on the page an operator checks to see what they have, is the failure worth engineering against. What is held is a fact; what it is worth was not observed. NO ACTION, ANYWHERE. #702's refusal, pinned on the payload AND on the view source: no buying power, no deposit, no withdraw, no transfer, not even an event listener. Cash is a fact, not an affordance, and keel is cash-spot by constitution -- a buying-power tile would advertise leverage the engine refuses to take. It is this page's version of the close button the Positions view also does not have. The per-asset rows fold `gather_positions` (#701) rather than re-reading the ledger, so Balances and Positions cannot come to disagree about what is held; every figure carries the instant it was recorded, including each asset's mark. One test-quality note worth keeping: the mode-partition test first passed against a report that ignored `mode` entirely, because its fixture made the live row the newest of the two. The fixture now makes the PAPER row newest, which is what makes the filter observable -- found by mutation, not by reading. Co-Authored-By: Claude Opus 5 (1M context) --- keel/commands/balances.py | 181 +++++++++++++++++++++ keel/web/api.py | 33 ++++ keel/web/payload.py | 78 +++++++++ keel/web/static/index.html | 1 + keel/web/static/js/main.js | 3 + keel/web/static/js/render.js | 91 +++++++++++ keel/web/staticfiles.py | 1 + tests/commands/test_balances.py | 271 ++++++++++++++++++++++++++++++++ tests/web/test_api.py | 1 + tests/web/test_balances_view.py | 204 ++++++++++++++++++++++++ 10 files changed, 864 insertions(+) create mode 100644 keel/commands/balances.py create mode 100644 tests/commands/test_balances.py create mode 100644 tests/web/test_balances_view.py diff --git a/keel/commands/balances.py b/keel/commands/balances.py new file mode 100644 index 0000000..22aa497 --- /dev/null +++ b/keel/commands/balances.py @@ -0,0 +1,181 @@ +"""What the account holds, as the last cycle recorded it -- issue #702. + +**NO BROKER CALL, AND THAT IS THE DESIGN.** A balances page is the obvious place to reach for a +live venue read, and this module deliberately does not. `keel serve`'s defining property is that +it is a loopback reader over SQLite with no credentials, no broker handle and no outbound +network: putting a venue read behind a page that re-polls every 15 seconds (`main.js`'s +`POLL_MS`) would hand an operator's rate limit to every browser tab left open, and would put +credentials into the one process a browser can reach. Every other read route already holds that +line (`gather_status`: "no broker, no network"; `list_installed_brokers`: "no broker handle, no +network, no config, no credentials"), and a balances view is not the place to break it. + +WHAT IS SHOWN INSTEAD IS BETTER, NOT MERELY SAFER. Cash comes from `equity_points` (#698) -- +the figure the agent read and SIZED AGAINST when it evaluated the rails that cycle -- stamped +with when it read it. A fresher number the engine never saw would explain nothing about why it +did what it did. + +WHAT IS NOT RECORDED IS SAID, NOT GUESSED. `equity_points.cash` comes from +`executor._fetch_available_quote`, which reads `Balance.available` and stops there; the venue's +settled-versus-total pair is never written down. This report carries that absence explicitly +rather than presenting the available figure under a label implying the distinction was checked +(see `settled_breakdown_recorded`). When a cycle records the pair, this becomes a read. +""" + +from __future__ import annotations + +from collections.abc import Sequence +from dataclasses import dataclass +from decimal import Decimal + +from keel.commands.positions import PositionRow, gather_positions +from keel.config import Config +from keel.data.repository import Repository + + +@dataclass(frozen=True) +class AssetBalanceRow: + """One PRODUCT's holding, summed across its tranches. + + Per asset, because that is the question a balances page answers. The tranche breakdown is + the Positions view's, and both read `gather_positions` so the two cannot disagree about what + is held. + """ + + product_id: str + + #: Quantity still held, summed over every open tranche of this product. + qty: Decimal + + #: The mark those tranches were valued at, and when it was read. `None` when the product has + #: no cached candle -- the same absence `PositionRow.mark` carries, for the same reason. + mark: Decimal | None + mark_as_of: int | None + + #: `qty * mark`, or `None` if ANY tranche of this product lacks a mark. + #: + #: A partial sum is the most dangerous shape available here: it looks like a total and is + #: not one, so a holding half of which could not be priced would render as a smaller holding + #: rather than an unknown one. Unknown is the only reading that cannot be misread. + market_value: Decimal | None + + +@dataclass(frozen=True) +class BalancesReport: + now_ts: int + + #: `paper`, `live`, or `""` before the first cycle stamps one. The partition every figure + #: below is read through: `equity_points` holds both modes in one database. + mode: str + + #: The newest recorded reading FOR THAT MODE, and the instant it was recorded. `cash` is + #: `None` when nothing has been recorded, and also when the recorded cycle knew its total + #: but not its split -- both are absences, never zero. + cash: Decimal | None + cash_as_of: int | None + equity: Decimal | None + unrealized: Decimal | None + hwm: Decimal | None + + #: Whether ANY reading exists for this mode. Distinct from `cash is None`: a deployment that + #: has never completed a cycle and one whose last cycle could not read a split are different + #: facts, and only the first is "this page has nothing to show yet". + has_recorded_cash: bool + + #: The synthetic account's CURRENT cash, in paper mode only (`agent_state`'s + #: `paper_cash_usdc`). It moves on every paper fill, so it answers "what does the paper + #: account hold now" beside `cash`'s "what did the cycle act on". `None` in live mode even + #: though the key survives a paper->live flip: a synthetic balance beside real money would be + #: the most confusing thing this page could show. + paper_cash: Decimal | None + + #: The venue's settled/total split. Both `None` and `settled_breakdown_recorded` False, + #: always, today -- see the module docstring. They are fields rather than an omission so the + #: page can SAY the distinction is unrecorded, and so that recording it later is a change to + #: the producer rather than to this shape. + settled_cash: Decimal | None + total_cash: Decimal | None + settled_breakdown_recorded: bool + + assets: tuple[AssetBalanceRow, ...] + + @property + def asset_count(self) -> int: + """How many products this report holds. Derived, and held here rather than measured by a + renderer: Rule 6e bans `len()` in `keel/web/payload.py`.""" + return len(self.assets) + + +def gather_balances(repo: Repository, config: Config, *, now_ts: int) -> BalancesReport: + """Everything the account holds, from what a cycle wrote down. No broker, no network. + + The mode is read FIRST and everything else is read through it. `equity_state_mode` is the + same stamp `agent._clear_live_mode_if_needed` maintains, and an unstamped one (before the + first cycle) yields no cash at all rather than a guess about which account to show. + """ + mode = str(repo.get_state("equity_state_mode") or "") + + reading = None + if mode: + recorded = repo.get_equity_points(mode=mode, limit=1) + # `limit=1` keeps the MOST RECENT reading (`get_equity_points`' own contract), so this is + # one row off an index rather than the whole series read to take its last element. + reading = recorded[-1] if recorded else None + + positions = gather_positions(repo, config, now_ts=now_ts) + return BalancesReport( + now_ts=now_ts, + mode=mode, + cash=None if reading is None else reading.cash, + cash_as_of=None if reading is None else reading.ts, + equity=None if reading is None else reading.equity, + unrealized=None if reading is None else reading.unrealized, + hwm=None if reading is None else reading.hwm, + has_recorded_cash=reading is not None, + paper_cash=repo.get_state("paper_cash_usdc") if mode == "paper" else None, + settled_cash=None, + total_cash=None, + settled_breakdown_recorded=False, + assets=_assets_from(positions.rows, positions.products), + ) + + +def _assets_from( + rows: Sequence[PositionRow], products: tuple[str, ...] +) -> tuple[AssetBalanceRow, ...]: + """Fold the per-tranche rows into one row per product, in the report's own product order. + + `products` comes from `PositionsReport`, not from a set built here: two answers to "which + products does this book hold" is one too many, and a set would reorder the page between + reads for no reason a reader could see. + """ + by_product: dict[str, list[PositionRow]] = {product: [] for product in products} + for row in rows: + by_product.setdefault(row.product_id, []).append(row) + + assets: list[AssetBalanceRow] = [] + for product in products: + held = by_product.get(product) or [] + if not held: + continue + qty = sum((row.qty for row in held), Decimal("0")) + marks = [row.mark for row in held] + # ANY missing mark makes the VALUE unknown -- never a sum over the priced subset. The + # quantity is still known and still shown: what is held is a fact, what it is worth is + # the part nobody observed. + if any(mark is None for mark in marks): + value: Decimal | None = None + else: + value = sum((row.market_value or Decimal("0") for row in held), Decimal("0")) + # Any tranche will do for the mark: `gather_positions` reads it ONCE PER PRODUCT and + # hands the same figure to every tranche of it, so "the first" and "the newest" are the + # same row here. Picking a maximum would imply they could differ. + assets.append( + AssetBalanceRow( + product_id=product, + qty=qty, + mark=held[0].mark, + mark_as_of=held[0].mark_ts, + market_value=value, + ) + ) + return tuple(assets) diff --git a/keel/web/api.py b/keel/web/api.py index 845adfd..c611268 100644 --- a/keel/web/api.py +++ b/keel/web/api.py @@ -331,6 +331,33 @@ def read_positions(cfg: ServeConfig, _query: Query, _state: Any, now_ts: int) -> return payload.positions_payload(report) +def read_balances(cfg: ServeConfig, _query: Query, _state: Any, now_ts: int) -> dict[str, Any]: + """What the account holds, as the last cycle recorded it (#702). + + **NO BROKER CALL, and this route is the one where that had to be decided on purpose.** A + balances page is the obvious place to read the venue live, and doing so would put credentials + into the process a browser talks to and hand an operator's rate limit to every tab left open + on a view that re-polls every 15 seconds. `keel serve` is a loopback reader over SQLite; every + other route here holds that line, and this one does too. `keel/commands/balances.py` carries + the full reasoning. + + READ ONLY, with no write route on this path and no action in the payload. #702's refusal: + cash is a fact, not an affordance -- no buying power, no deposit, no transfer. + + No `?limit=`: an account's asset list is bounded by what it holds, and a cap here would hide + a holding an operator is looking for. + """ + from keel.commands.balances import gather_balances + + repo = open_repo(cfg.db_path) + try: + config = load_config(cfg.config_path) + report = gather_balances(repo, config, now_ts=now_ts) + finally: + close_repo(repo) + return payload.balances_payload(report) + + def read_insights(cfg: ServeConfig, _query: Query, _state: Any, now_ts: int) -> dict[str, Any]: """The per-rule track records, the promotion-gate distances, and the account-equity series. @@ -602,6 +629,12 @@ class ApiRoute: "stop_distance_pct", ), ), + "/api/balances": ApiRoute( + html_route="/balances", + read=read_balances, + collection="assets", + sortable=("product_id", "qty", "mark", "market_value"), + ), "/api/rules": ApiRoute( html_route="/rules", read=read_rules, diff --git a/keel/web/payload.py b/keel/web/payload.py index 1420fc3..600ca23 100644 --- a/keel/web/payload.py +++ b/keel/web/payload.py @@ -120,6 +120,7 @@ if TYPE_CHECKING: # pragma: no cover - typing only from keel.commands.activity import ActivityCycle, ActivityEvent, ActivityFeed + from keel.commands.balances import AssetBalanceRow, BalancesReport from keel.commands.insights import ( AccountSummary, EquityCurve, @@ -1638,6 +1639,83 @@ def positions_payload(report: PositionsReport) -> dict[str, Any]: } +#: What the settled/unsettled split says while nothing records it (#702). A FIXED sentence, not +#: a figure: `equity_points.cash` is `Balance.available` and only that, so the distinction is not +#: a number this deployment has. Rendering the available figure under a "settled" label would +#: answer the question the page exists to ask honestly. +_SETTLED_UNRECORDED = "UNRECORDED IN CYCLE SNAPSHOT -- only the available figure is written down" + + +def _asset_balance_payload(row: AssetBalanceRow) -> dict[str, Any]: + """One product's holding. + + `qty` without a `market_value` is a real and common row: the holding is recorded, the price + was not observed. It is never a zero -- a worthless holding and an unpriced one look the same + on a page and are not the same fact. + """ + return { + "product_id": row.product_id, + "qty": quantity(row.qty), + "mark": money(row.mark), + "mark_as_of": moment(row.mark_as_of), + "market_value": money(row.market_value), + } + + +def balances_payload(report: BalancesReport) -> dict[str, Any]: + """`gather_balances`'s `BalancesReport`, as JSON (#702). + + **Every figure here was recorded by a cycle, and every one carries when.** `keel serve` makes + no network call -- see `keel/commands/balances.py` for why that is the design and not a + limitation -- so the as-of stamps are what keep a recorded page honest rather than merely + stale-looking. A tile with no time on it is a claim about now that was made at some other now. + + **No buying power, no deposit, no transfer, and no action of any kind.** #702's refusal: + cash is a fact, not an affordance, and this codebase is cash-spot by constitution + (`CashAccountRequired`, #372) -- a "buying power" figure would invite exactly the leverage the + engine refuses to take. + + `settled_cash` and `total_cash` cross as ABSENT with `settled_breakdown` saying why, rather + than being omitted: a client that had to notice a missing key would be inferring from payload + shape, and the day a cycle records the pair this becomes a value change rather than a shape + change. + """ + return { + "as_of": iso(report.now_ts), + "generated_at": moment(report.now_ts), + # A bare string, like `scope` and `mode` elsewhere: an enum word with no precision hazard + # and no judgement of its own. + "mode": report.mode, + "cash": money(report.cash), + "cash_as_of": moment(report.cash_as_of), + "equity": money(report.equity), + "unrealized": money(report.unrealized, signed=True), + "hwm": money(report.hwm), + "paper_cash": money(report.paper_cash), + "settled_cash": money(report.settled_cash), + "total_cash": money(report.total_cash), + "settled_breakdown": flag( + report.settled_breakdown_recorded, + on="settled and unsettled recorded", + off=_SETTLED_UNRECORDED, + on_state=NEUTRAL, + # UNKNOWN and not WARN: nothing is wrong, and nothing is late. The venue reports the + # split and no cycle writes it down, which is a gap in what keel records rather than + # a condition an operator can act on. + off_state=UNKNOWN, + ), + "recorded": flag( + report.has_recorded_cash, + on="as recorded by the last cycle", + off="no cycle has recorded a balance yet", + on_state=NEUTRAL, + off_state=UNKNOWN, + ), + "asset_count": count(report.asset_count), + "assets": [_asset_balance_payload(row) for row in report.assets], + } + + # -- the envelope (#534) ------------------------------------------------------------------------- # # Every `GET /api/*` success is wrapped in the same four keys, so #536's single `fetch` wrapper diff --git a/keel/web/static/index.html b/keel/web/static/index.html index 0aac406..b759e04 100644 --- a/keel/web/static/index.html +++ b/keel/web/static/index.html @@ -119,6 +119,7 @@
  • Activity
  • Orders
  • Positions
    • +
  • Balances
  • Insights
  • Rules
  • Venues
    • diff --git a/keel/web/static/js/main.js b/keel/web/static/js/main.js index 123975e..752271a 100644 --- a/keel/web/static/js/main.js +++ b/keel/web/static/js/main.js @@ -45,6 +45,7 @@ import { insightsView, modeBadge, ordersView, + balancesView, positionsView, refusedView, rulesView, @@ -99,6 +100,7 @@ const ROUTES = [ { name: "activity", label: "Activity", endpoints: ["activity"] }, { name: "orders", label: "Orders", endpoints: ["orders"] }, { name: "positions", label: "Positions", endpoints: ["positions"] }, + { name: "balances", label: "Balances", endpoints: ["balances"] }, { name: "insights", label: "Insights", endpoints: ["insights", "journal"] }, { name: "rules", label: "Rules", endpoints: ["rules"] }, { name: "venues", label: "Venues", endpoints: ["venues"] }, @@ -427,6 +429,7 @@ function mount(route, readings) { ); } if (route.name === "positions") return positionsView(data, primary.sort, onSort); + if (route.name === "balances") return balancesView(data, primary.sort, onSort); if (route.name === "rules") return rulesView(data, primary.sort, onSort); if (route.name === "venues") return venuesView(data, primary.sort, onSort); if (route.name === "gates") return gatesView(data); diff --git a/keel/web/static/js/render.js b/keel/web/static/js/render.js index ff9cce7..4661857 100644 --- a/keel/web/static/js/render.js +++ b/keel/web/static/js/render.js @@ -1138,6 +1138,97 @@ function jobPanel(job) { * @param {(scope: string) => void} onScope * @returns {DocumentFragment} */ +/** + * The Balances view (#702): what the account holds, as the last cycle recorded it. + * + * ── EVERY FIGURE IS STAMPED, BECAUSE EVERY FIGURE IS RECORDED ──────────────────────────────── + * + * Nothing here came from a network call -- `keel serve` reads SQLite and nothing else, and + * `keel/commands/balances.py` carries why that is the design rather than a limitation. What + * makes that honest instead of merely quiet is the as-of stamp beside each tile: this is the + * cash the engine sized against when it last evaluated the rails, and the page says when. + * + * ── NO BUYING POWER, NO DEPOSIT, NO TRANSFER, NO CTA ───────────────────────────────────────── + * + * #702's refusal. Cash is a fact, not an affordance. keel is cash-spot by constitution, so a + * "buying power" tile would advertise leverage the engine refuses to take, and a deposit button + * would be this page's version of the close button the Positions view also does not have. + * Pinned by `tests/web/test_balances_view.py`. + * + * ── THE SETTLED SPLIT IS NAMED AS MISSING, NOT OMITTED ─────────────────────────────────────── + * + * `settled_breakdown` is a field whose whole content is "unrecorded". Leaving the tiles out + * would let a reader assume the available figure IS the settled one; saying so is the only + * rendering that cannot be misread. + * + * @param {any} data `/api/balances`'s `data`. + * @param {any} sort + * @param {(column: string) => void} onSort + * @returns {DocumentFragment} + */ +export function balancesView(data, sort, onSort) { + const fragment = document.createDocumentFragment(); + fragment.append(el("h1", undefined, "Balances")); + + const sub = el("p", "sub"); + sub.append(field(data.generated_at), " · ", field(data.recorded)); + fragment.append(sub); + + fragment.append( + gridCard([ + kv("mode", plain(data.mode) || "unstamped"), + kv("available cash", data.cash), + kv("as of", data.cash_as_of), + kv("equity", data.equity), + kv("unrealized", data.unrealized), + kv("high water mark", data.hwm), + ]), + ); + + fragment.append(heading("h-settled", "Settled and unsettled")); + const settled = el("p", "note"); + settled.append(field(data.settled_breakdown)); + fragment.append(settled); + fragment.append( + gridCard([kv("settled", data.settled_cash), kv("total", data.total_cash)]), + ); + + if (plain(data.mode) === "paper") { + fragment.append(heading("h-paper", "Synthetic account")); + const note = el("p", "note"); + note.append("The paper account's cash right now, beside the cycle's recorded reading above."); + fragment.append(note); + fragment.append(gridCard([kv("paper cash", data.paper_cash)])); + } + + fragment.append(heading("h-assets", "Held assets")); + const assets = data.assets || []; + fragment.append( + table( + "h-assets", + [ + { label: "product", numeric: false, key: "product_id" }, + { label: "qty held", numeric: true, key: "qty" }, + { label: "mark", numeric: true, key: "mark" }, + { label: "marked at", numeric: false, key: "mark_as_of" }, + { label: "value", numeric: true, key: "market_value" }, + ], + assets.map(/** @param {any} row */ (row) => [ + plain(row.product_id) || "—", + row.qty, + row.mark, + row.mark_as_of, + row.market_value, + ]), + "No held assets. keel is holding nothing right now.", + { sort: sort, onSort: onSort }, + ), + ); + + return fragment; +} + + /** * The Positions view (#701): what is held, what it is worth, and how close it is to its stop. * diff --git a/keel/web/staticfiles.py b/keel/web/staticfiles.py index abac86d..7b68307 100644 --- a/keel/web/staticfiles.py +++ b/keel/web/staticfiles.py @@ -142,6 +142,7 @@ def resolve_static_asset(root: Path, url_path: str) -> Path | None: "activity", "orders", "positions", + "balances", "insights", "rules", "venues", diff --git a/tests/commands/test_balances.py b/tests/commands/test_balances.py new file mode 100644 index 0000000..f250e2d --- /dev/null +++ b/tests/commands/test_balances.py @@ -0,0 +1,271 @@ +"""The balances report -- issue #702. + +**`keel serve` makes no network call, and this report is why that stays true.** A balances page +is the obvious place to reach for a live venue read, and doing so would put credentials into the +web process, hand venue latency to a page that re-polls every 15 seconds, and spend an operator's +rate limit for every browser tab left open. Every figure here comes from what a CYCLE recorded: +`equity_points` (#698) for cash, the positions report (#701) for the per-asset rows. + +That is not a consolation prize. The cash this shows is the cash the engine actually sized +against when it evaluated the rails, stamped with when it read it -- which is a more useful +answer than a fresher number the engine never saw. +""" + +from __future__ import annotations + +from decimal import Decimal +from pathlib import Path +from typing import Any + +import pytest +from keel_core.types import Candle, EquityReading, Granularity + +from keel.commands.balances import gather_balances +from keel.config import Config +from keel.data.db import connect, migrate +from keel.data.repository import Repository +from tests.conftest import VALID_CONFIG_YAML + +NOW_TS = 1_800_000_000 +DAY = 86_400 +FINEST = Granularity.FIFTEEN_MINUTE + + +@pytest.fixture() +def repo(tmp_path: Path) -> Repository: + conn = connect(str(tmp_path / "keel.db")) + migrate(conn) + return Repository(conn) + + +def _config(tmp_path: Path) -> Config: + from keel.config import load_config + + path = tmp_path / "config.yaml" + path.write_text(VALID_CONFIG_YAML, encoding="utf-8") + return load_config(str(path)) + + +def _reading(ts: int, mode: str, cash: str | None, equity: str = "10000") -> EquityReading: + return EquityReading( + ts=ts, + mode=mode, + equity=Decimal(equity), + cash=None if cash is None else Decimal(cash), + unrealized=Decimal("0"), + hwm=Decimal(equity), + ) + + +def _candle(ts: int, close: str) -> Candle: + price = Decimal(close) + return Candle(ts=ts, open=price, high=price, low=price, close=price, volume=Decimal("1")) + + +def _tranche(repo: Repository, product_id: str, qty: str, **overrides: Any) -> int: + row: dict[str, Any] = { + "product_id": product_id, + "rule_name": "turtle_breakout", + "opened_at": NOW_TS - DAY, + "qty": Decimal(qty), + "entry_fill": Decimal("100"), + "entry_fee": Decimal("1"), + "initial_stop": Decimal("90"), + } + row.update(overrides) + return repo.open_position(**row) + + +# -- cash comes from the cycle that recorded it --------------------------------------------------- + + +def test_cash_is_the_newest_reading_for_the_mode_in_force(repo: Repository, tmp_path: Path) -> None: + """The mode partition, again. `equity_points` holds paper and live rows in one database, and + a page that took the newest row of ANY mode would show a $10,000 synthetic balance on a live + deployment that holds $250.""" + repo.set_state("equity_state_mode", "live") + # The PAPER row is the newer of the two, deliberately: an unfiltered "newest reading" read + # would return it, so this fixture is what makes the mode filter observable. With the live + # row newest, the test would pass against a report that ignored `mode` entirely. + repo.record_equity_point(_reading(NOW_TS - 3600, "live", "250.10", equity="250.10")) + repo.record_equity_point(_reading(NOW_TS - 60, "paper", "9000", equity="10000")) + + report = gather_balances(repo, _config(tmp_path), now_ts=NOW_TS) + + assert report.mode == "live" + assert report.cash == Decimal("250.10"), "the live reading, not the newer paper one" + assert report.cash_as_of == NOW_TS - 3600 + + +def test_the_newest_reading_wins_within_the_mode(repo: Repository, tmp_path: Path) -> None: + repo.set_state("equity_state_mode", "live") + repo.record_equity_point(_reading(NOW_TS - DAY, "live", "100")) + repo.record_equity_point(_reading(NOW_TS - 60, "live", "300")) + + assert gather_balances(repo, _config(tmp_path), now_ts=NOW_TS).cash == Decimal("300") + + +def test_a_deployment_that_has_recorded_nothing_says_so(repo: Repository, tmp_path: Path) -> None: + """A fresh deployment, or one that has not completed a cycle since the v19 upgrade. Absent, + never zero: a zero cash balance is a fact about an account, and this is the absence of any + fact at all.""" + repo.set_state("equity_state_mode", "live") + + report = gather_balances(repo, _config(tmp_path), now_ts=NOW_TS) + + assert report.cash is None + assert report.cash_as_of is None + assert report.has_recorded_cash is False + + +def test_a_reading_whose_split_was_never_recorded_reports_no_cash( + repo: Repository, tmp_path: Path +) -> None: + """`equity_points.cash` is nullable -- a cycle can know its total and not the split. The + equity is still shown; the cash is not invented from it.""" + repo.set_state("equity_state_mode", "live") + repo.record_equity_point(_reading(NOW_TS - 60, "live", None, equity="250")) + + report = gather_balances(repo, _config(tmp_path), now_ts=NOW_TS) + + assert report.cash is None + assert report.equity == Decimal("250") + assert report.cash_as_of == NOW_TS - 60, "the reading still has a time, it just has no split" + + +def test_the_recorded_equity_and_unrealized_ride_along(repo: Repository, tmp_path: Path) -> None: + """One reading, read once. Taking cash from one cycle and equity from another would show two + moments as though they were one.""" + repo.set_state("equity_state_mode", "live") + repo.record_equity_point(_reading(NOW_TS - 60, "live", "250.10", equity="1250.10")) + + report = gather_balances(repo, _config(tmp_path), now_ts=NOW_TS) + + assert report.equity == Decimal("1250.10") + assert report.hwm == Decimal("1250.10") + + +# -- what is NOT recorded ------------------------------------------------------------------------- + + +def test_the_settled_breakdown_is_reported_as_unrecorded( + repo: Repository, tmp_path: Path +) -> None: + """#702's centrepiece, and the honest answer to it today. `equity_points.cash` comes from + `_fetch_available_quote`, which reads `Balance.available` and nothing else -- the venue's + settled-versus-total pair is never written down. The report says so rather than presenting + the available figure under a label that implies the distinction was checked.""" + repo.set_state("equity_state_mode", "live") + repo.record_equity_point(_reading(NOW_TS - 60, "live", "250.10")) + + report = gather_balances(repo, _config(tmp_path), now_ts=NOW_TS) + + assert report.settled_cash is None + assert report.total_cash is None + assert report.settled_breakdown_recorded is False + + +# -- paper mode ------------------------------------------------------------------------------------ + + +def test_paper_mode_reads_the_synthetic_cash_beside_the_recorded_one( + repo: Repository, tmp_path: Path +) -> None: + """`paper_cash_usdc` is the live value of the synthetic account, which moves the moment a + paper fill happens -- while the recorded reading is from the last cycle. Both are shown: they + answer "what does the paper account hold now" and "what did the cycle act on".""" + repo.set_state("equity_state_mode", "paper") + repo.set_state("paper_cash_usdc", Decimal("9500.25")) + repo.record_equity_point(_reading(NOW_TS - 3600, "paper", "9000")) + + report = gather_balances(repo, _config(tmp_path), now_ts=NOW_TS) + + assert report.paper_cash == Decimal("9500.25") + assert report.cash == Decimal("9000") + + +def test_live_mode_reports_no_paper_cash_even_if_the_key_survives( + repo: Repository, tmp_path: Path +) -> None: + """`paper_cash_usdc` outlives a paper->live flip in `agent_state`. Showing it on a live page + would put a synthetic balance beside real money.""" + repo.set_state("equity_state_mode", "live") + repo.set_state("paper_cash_usdc", Decimal("9500.25")) + + assert gather_balances(repo, _config(tmp_path), now_ts=NOW_TS).paper_cash is None + + +# -- the per-asset rows ---------------------------------------------------------------------------- + + +def test_tranches_of_one_product_are_summed_into_one_asset_row( + repo: Repository, tmp_path: Path +) -> None: + """A balances page answers "what do I hold", which is per ASSET. The tranche breakdown is the + Positions view's job, and the two read the same report so they cannot disagree.""" + repo.set_state("equity_state_mode", "live") + _tranche(repo, "BTC-USD", "2") + _tranche(repo, "BTC-USD", "3") + repo.upsert_candles("BTC-USD", FINEST, [_candle(NOW_TS - 900, "150")]) + + rows = gather_balances(repo, _config(tmp_path), now_ts=NOW_TS).assets + + assert [row.product_id for row in rows] == ["BTC-USD"] + assert rows[0].qty == Decimal("5") + assert rows[0].market_value == Decimal("5") * Decimal("150") + + +def test_an_asset_with_an_unmarked_tranche_reports_no_value_rather_than_a_partial_sum( + repo: Repository, tmp_path: Path +) -> None: + """A partial sum is the most dangerous shape here: it looks like a total and is not one. If + any tranche of a product has no mark, the product's value is unknown -- and saying so is the + only reading that cannot be mistaken for a smaller holding.""" + repo.set_state("equity_state_mode", "live") + _tranche(repo, "BTC-USD", "2") + _tranche(repo, "ETH-USD", "3") + repo.upsert_candles("BTC-USD", FINEST, [_candle(NOW_TS - 900, "150")]) + + by_product = { + row.product_id: row + for row in gather_balances(repo, _config(tmp_path), now_ts=NOW_TS).assets + } + + assert by_product["BTC-USD"].market_value == Decimal("300") + assert by_product["ETH-USD"].qty == Decimal("3"), "the holding is known" + assert by_product["ETH-USD"].market_value is None, "its value is not" + + +def test_the_asset_mark_carries_the_time_it_was_read(repo: Repository, tmp_path: Path) -> None: + """Every figure on this page is stamped. A mark with no time is a claim about now that was + made at some other now.""" + repo.set_state("equity_state_mode", "live") + _tranche(repo, "BTC-USD", "2") + repo.upsert_candles("BTC-USD", FINEST, [_candle(NOW_TS - 900, "150")]) + + row = gather_balances(repo, _config(tmp_path), now_ts=NOW_TS).assets[0] + + assert row.mark == Decimal("150") + assert row.mark_as_of == NOW_TS - 900 + + +def test_no_open_positions_is_an_empty_asset_list_not_an_error( + repo: Repository, tmp_path: Path +) -> None: + repo.set_state("equity_state_mode", "live") + + assert gather_balances(repo, _config(tmp_path), now_ts=NOW_TS).assets == () + + +def test_an_unstamped_mode_reports_no_cash_rather_than_guessing( + repo: Repository, tmp_path: Path +) -> None: + """Before the first cycle, `equity_state_mode` is unset. Picking a mode to read would be + choosing which account's balance to show on a deployment that has not said.""" + repo.record_equity_point(_reading(NOW_TS - 60, "live", "250")) + + report = gather_balances(repo, _config(tmp_path), now_ts=NOW_TS) + + assert report.mode == "" + assert report.cash is None + assert report.has_recorded_cash is False diff --git a/tests/web/test_api.py b/tests/web/test_api.py index c36e804..3893333 100644 --- a/tests/web/test_api.py +++ b/tests/web/test_api.py @@ -59,6 +59,7 @@ "/api/activity", "/api/orders", "/api/positions", + "/api/balances", "/api/insights", "/api/journal", "/api/rules", diff --git a/tests/web/test_balances_view.py b/tests/web/test_balances_view.py new file mode 100644 index 0000000..e3cd165 --- /dev/null +++ b/tests/web/test_balances_view.py @@ -0,0 +1,204 @@ +"""The balances view, on the wire -- issue #702. + +The service decides; this pins that the browser gets the decision unaltered. The property that +matters most here is negative: **no figure on this page came from a network call**, and the +as-of stamps are what make that honest rather than hidden. +""" + +from __future__ import annotations + +import json +from decimal import Decimal +from pathlib import Path +from typing import Any + +from keel.commands.balances import gather_balances +from keel.data.db import connect, migrate +from keel.data.repository import Repository +from keel.web import payload as web_payload + +BAL_NOW_TS = 1_800_000_000 + + +def _walk(node: Any, path: str = "$") -> list[tuple[str, Any]]: + if isinstance(node, dict): + out: list[tuple[str, Any]] = [] + for key, value in node.items(): + out.extend(_walk(value, f"{path}.{key}")) + return out + if isinstance(node, list): + out = [] + for index, value in enumerate(node): + out.extend(_walk(value, f"{path}[{index}]")) + return out + return [(path, node)] + + +def _report(tmp_path: Path, *, mode: str = "live", cash: str | None = "250.10", **kw: Any) -> Any: + from tests.commands.test_balances import FINEST, _candle, _config, _reading, _tranche + + conn = connect(str(tmp_path / "balances.db")) + migrate(conn) + repo = Repository(conn) + repo.set_state("equity_state_mode", mode) + if cash is not None: + repo.record_equity_point(_reading(BAL_NOW_TS - 3600, mode, cash)) + if kw.get("paper_cash") is not None: + repo.set_state("paper_cash_usdc", Decimal(kw["paper_cash"])) + if kw.get("holding", True): + _tranche(repo, "BTC-USD", "2") + repo.upsert_candles("BTC-USD", FINEST, [_candle(BAL_NOW_TS - 900, "150")]) + return gather_balances(repo, _config(tmp_path), now_ts=BAL_NOW_TS) + + +def test_no_wire_value_in_the_balances_payload_is_ever_a_json_number(tmp_path: Path) -> None: + document = json.loads(json.dumps(web_payload.balances_payload(_report(tmp_path)))) + numbers = [ + path + for path, leaf in _walk(document) + if not isinstance(leaf, bool) and isinstance(leaf, (int, float)) + ] + assert numbers == [], numbers + + +def test_every_recorded_figure_carries_the_instant_it_was_recorded(tmp_path: Path) -> None: + """The as-of stamp is what makes a recorded page honest instead of stale. A cash figure with + no time on it is a claim about now that was made at some other now.""" + built = web_payload.balances_payload(_report(tmp_path)) + + assert built["cash_as_of"]["value"].endswith("Z") + assert built["assets"][0]["mark_as_of"]["value"].endswith("Z") + + +def test_the_settled_breakdown_says_it_is_unrecorded_rather_than_showing_available_as_settled( + tmp_path: Path, +) -> None: + """#702's centrepiece. `equity_points.cash` is the AVAILABLE figure and nothing else, so the + settled/unsettled split is not a number this deployment has. Labelling the available figure + "settled" would answer the question the page was built to ask honestly.""" + built = web_payload.balances_payload(_report(tmp_path)) + + assert built["settled_cash"]["state"] == "unknown" + assert built["total_cash"]["state"] == "unknown" + assert "UNRECORDED" in built["settled_breakdown"]["display"].upper() + assert built["settled_breakdown"]["state"] == "unknown" + + +def test_a_deployment_with_no_recorded_cycle_says_so(tmp_path: Path) -> None: + """Not "$0.00", and not a blank tile. A deployment that has not completed a cycle since the + series began recording has nothing to show, which is a different fact from an empty account.""" + built = web_payload.balances_payload(_report(tmp_path, cash=None)) + + assert built["cash"]["state"] == "unknown" + assert built["recorded"]["value"] == "false" + assert built["recorded"]["display"] + + +def test_paper_cash_crosses_only_in_paper_mode(tmp_path: Path) -> None: + live = web_payload.balances_payload(_report(tmp_path, mode="live", paper_cash="9500")) + nested = tmp_path / "p" + nested.mkdir() + paper = web_payload.balances_payload(_report(nested, mode="paper", paper_cash="9500")) + + assert live["paper_cash"]["state"] == "unknown" + assert paper["paper_cash"]["value"] == "9500" + + +def test_the_asset_rows_carry_quantity_and_value(tmp_path: Path) -> None: + row = web_payload.balances_payload(_report(tmp_path))["assets"][0] + + assert row["product_id"] == "BTC-USD" + assert row["qty"]["value"] == "2" + assert row["market_value"]["value"] == "300" + + +def test_an_unpriced_asset_shows_its_holding_and_no_value(tmp_path: Path) -> None: + """The quantity is a fact; its worth is not. A zero here would read as a worthless holding.""" + from tests.commands.test_balances import _config, _reading, _tranche + + conn = connect(str(tmp_path / "b.db")) + migrate(conn) + repo = Repository(conn) + repo.set_state("equity_state_mode", "live") + repo.record_equity_point(_reading(BAL_NOW_TS - 3600, "live", "250")) + _tranche(repo, "ETH-USD", "3") + + built = web_payload.balances_payload( + gather_balances(repo, _config(tmp_path), now_ts=BAL_NOW_TS) + ) + row = built["assets"][0] + + assert row["qty"]["value"] == "3" + assert row["market_value"]["state"] == "unknown" + + +def test_the_mode_and_counts_come_off_the_report(tmp_path: Path) -> None: + """Rule 6e: no `len()` in the serialiser.""" + built = web_payload.balances_payload(_report(tmp_path)) + + assert built["mode"] == "live" + assert built["asset_count"]["value"] == "1" + + +def test_the_payload_offers_no_buying_power_and_no_action(tmp_path: Path) -> None: + """#702's refusal, on the wire rather than only in the view. "Buying power" is a leverage + invitation, and a balances page that carried one would be inviting the operator to spend + money the cash-spot constitution refuses to lend them.""" + text = json.dumps(web_payload.balances_payload(_report(tmp_path))).lower() + + for banned in ("buying_power", "buying power", "deposit", "withdraw", "transfer"): + assert banned not in text, banned + + +# -- the view, in the client ----------------------------------------------------------------- + + +def _source(name: str) -> str: + from keel.web import staticfiles + + return (Path(staticfiles.__file__).parent / "static" / "js" / name).read_text(encoding="utf-8") + + +def _code(name: str) -> str: + from tests.web.test_client_assets import _comments_only + + return _comments_only(_source(name)) + + +def _view_body() -> str: + after = _code("render.js").split("export function balancesView")[1] + end = after.find("export function ") + return after if end == -1 else after[:end] + + +def test_the_balances_view_is_wired_into_the_client_router() -> None: + from keel.web import staticfiles + + assert "balances" in staticfiles.CLIENT_ROUTES + main_code = _code("main.js") + assert "balancesView" in main_code + assert 'route.name === "balances"' in main_code + assert "export function balancesView" in _code("render.js") + + +def test_the_balances_view_offers_no_action_of_any_kind() -> None: + """#702's refusal, pinned on the source. Cash is a fact, not an affordance -- and this is the + page where a deposit button or a "buying power" tile would look most like a courtesy.""" + body = _view_body().lower() + + for banned in ("buying", "deposit", "withdraw", "transfer", "button", "addeventlistener"): + assert banned not in body, f"the balances view must offer no {banned}" + + +def test_the_view_stamps_the_recorded_figures() -> None: + """A recorded page is honest only if it says when. Both stamps are read from the payload -- + the cash reading's, and each asset's mark.""" + body = _view_body() + + assert "cash_as_of" in body + assert "mark_as_of" in body + + +def test_the_view_names_the_settled_split_as_unrecorded() -> None: + """Omitting the tiles would let a reader take the available figure for the settled one.""" + assert "settled_breakdown" in _view_body() From acdf24ba570b476f7c6cd494276ed93aa2cba1e8 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Fri, 4 Sep 2026 14:53:22 -0400 Subject: [PATCH 2/2] fix(web): two tests that looked like coverage and were not (#702) Review findings on this branch. The first two are the ones worth reading; both are tests that passed for reasons other than the behaviour they name. THE MIXED-MARK GUARD COULD NOT FIRE, AND ITS TEST DID NOT REACH IT. `AssetBalanceRow` withholds a product's VALUE when any tranche of it lacks a mark, because a sum over the priced subset looks like a total and is a smaller one -- a holding rendered as worth less than it is, on the page an operator opens to see what they have. The test named for that seeded two different PRODUCTS rather than two tranches of one, so it never reached the branch: replacing `any(mark is None for mark in marks)` with `held[0].mark is None` left every test green. And the rule as written described a state that cannot occur. `gather_positions` reads the mark ONCE PER PRODUCT and hands every tranche of it the same figure, so `any(...)` is provably equivalent to the first row's mark -- through that caller. The guard is real but DEFENSIVE: what it protects is the fold, which a caller assembling rows from more than one read, or a mark cache that stopped being per-product, would reach immediately. Pinned now at `_assets_from` with hand-built rows, which is the only way to produce the state, and the prose says defensive instead of implying observed. THREE VIEWS HAD NO KEY-PARITY CHECK AT ALL. `balancesView` was missing from `_VIEW_ENDPOINTS` -- and so were `positionsView` and `ordersView`, from #701 and #700. Renaming `data.hwm` to `data.high_water_mark` in the balances view left the entire web suite green while the tile would have rendered blank forever, with nothing in the console naming the gap. That is precisely the failure `test_every_ported_view_reads_only_keys_its_ endpoint_sends` was written for. All three added; the scan finds 13, 4 and 11 keys respectively, so none of them is passing vacuously. THE PAGE PAID FOR A VERDICT IT NEVER SHOWS. Measured: three products cost 12 candle reads and a rules read per request -- four per product, three of them computing `entry_bar_ready` across every configured granularity -- on an endpoint the console re-polls every 15 seconds. Balances renders quantity, mark and value and no gate verdict. `gather_positions` now takes `with_readiness=False`, so the mark logic stays single-sourced rather than being copied here, and the skipped rows carry `ready=False, ready_reason=None` -- a pair `entry_bar_ready` never returns, so work that was skipped cannot be mistaken for a gate that refused. A DOCSTRING THAT OVER-CLAIMED. `mode` was described as "the partition every figure below is read through". True of cash, equity, unrealized, hwm and paper_cash; false of `assets`, because the `positions` table has no `mode` column. On a database that has flipped paper->live this page shows live cash beside holdings that may predate the flip. The docstring now says so and names the engine change that would fix it, rather than implying this report could infer it after the fact. Smaller: the "marked at" column declared `key: "mark_as_of"`, which `/api/balances` does not list as sortable -- a header that looks clickable and is not; product ordering and the per-product mark source are pinned rather than only argued (both mutations passed before); and the payload case where a cycle DID record a reading whose split was `None` -- `recorded` true and `cash` absent together, the distinction `has_recorded_cash` exists to draw -- is now covered. Co-Authored-By: Claude Opus 5 (1M context) --- keel/commands/balances.py | 32 +++++++-- keel/commands/positions.py | 32 ++++++--- keel/web/static/js/render.js | 4 +- tests/commands/test_balances.py | 120 ++++++++++++++++++++++++++++++++ tests/web/test_balances_view.py | 22 ++++++ tests/web/test_client_assets.py | 7 ++ 6 files changed, 201 insertions(+), 16 deletions(-) diff --git a/keel/commands/balances.py b/keel/commands/balances.py index 22aa497..7af680d 100644 --- a/keel/commands/balances.py +++ b/keel/commands/balances.py @@ -53,9 +53,16 @@ class AssetBalanceRow: #: `qty * mark`, or `None` if ANY tranche of this product lacks a mark. #: - #: A partial sum is the most dangerous shape available here: it looks like a total and is - #: not one, so a holding half of which could not be priced would render as a smaller holding + #: A partial sum is the most dangerous shape available here: it looks like a total and is not + #: one, so a holding half of which could not be priced would render as a SMALLER holding #: rather than an unknown one. Unknown is the only reading that cannot be misread. + #: + #: Through `gather_positions` that state cannot arise -- it reads the mark once per product + #: and hands every tranche of it the same figure -- so the guard in `_assets_from` is + #: DEFENSIVE, not a description of something observed. It is kept, and pinned at the fold + #: level rather than through `gather_balances`, because what it protects is the FOLD: a + #: caller assembling rows from more than one read, or a mark cache that stops being + #: per-product, reaches it immediately. market_value: Decimal | None @@ -63,8 +70,16 @@ class AssetBalanceRow: class BalancesReport: now_ts: int - #: `paper`, `live`, or `""` before the first cycle stamps one. The partition every figure - #: below is read through: `equity_points` holds both modes in one database. + #: `paper`, `live`, or `""` before the first cycle stamps one. + #: + #: The partition the CASH BLOCK is read through -- `equity_points` holds both modes in one + #: database, so cash, equity, unrealized, hwm and paper_cash are all selected by it. + #: + #: **It does NOT partition `assets`.** The `positions` table has no `mode` column: a tranche + #: is a tranche, whichever mode opened it. On a database that has flipped paper->live (which + #: `agent._clear_live_mode_if_needed` exists to handle) this page therefore shows live cash + #: beside holdings that may predate the flip. Recording a mode per tranche is the fix, and it + #: is an engine change, not something this report can infer after the fact. mode: str #: The newest recorded reading FOR THAT MODE, and the instant it was recorded. `cash` is @@ -121,7 +136,10 @@ def gather_balances(repo: Repository, config: Config, *, now_ts: int) -> Balance # one row off an index rather than the whole series read to take its last element. reading = recorded[-1] if recorded else None - positions = gather_positions(repo, config, now_ts=now_ts) + # `with_readiness=False`: this page shows quantity, mark and value and never the entry + # gate, so computing one would be three of every four candle reads plus a rules read and + # a rule construction, per request, on a view the console re-polls every 15 seconds. + positions = gather_positions(repo, config, now_ts=now_ts, with_readiness=False) return BalancesReport( now_ts=now_ts, mode=mode, @@ -161,7 +179,9 @@ def _assets_from( marks = [row.mark for row in held] # ANY missing mark makes the VALUE unknown -- never a sum over the priced subset. The # quantity is still known and still shown: what is held is a fact, what it is worth is - # the part nobody observed. + # the part nobody observed. Defensive against a caller whose rows do not share one mark + # per product; `gather_positions` does, so this cannot fire through it (see + # `AssetBalanceRow.market_value`). if any(mark is None for mark in marks): value: Decimal | None = None else: diff --git a/keel/commands/positions.py b/keel/commands/positions.py index 8c60717..bee59c3 100644 --- a/keel/commands/positions.py +++ b/keel/commands/positions.py @@ -256,7 +256,9 @@ def _granularity_rank(granularity: Granularity) -> int: return agent_mod._GRANULARITY_ORDER.get(granularity, 0) -def gather_positions(repo: Repository, config: Config, *, now_ts: int) -> PositionsReport: +def gather_positions( + repo: Repository, config: Config, *, now_ts: int, with_readiness: bool = True +) -> PositionsReport: """Every OPEN tranche, marked and judged. Open only: a closed tranche is a `trade_outcomes` row and belongs to the journal, which @@ -266,14 +268,23 @@ def gather_positions(repo: Repository, config: Config, *, now_ts: int) -> Positi The candle cache is read ONCE PER PRODUCT rather than once per tranche -- a book can hold several tranches of the same product (that is what tranches are for), and a per-row read would be one query per tranche for one answer they all share. + + `with_readiness=False` skips the entry-gate verdict entirely: no rules read, no rule built, + and no `entry_bar_ready` call -- which on the default three-granularity config is three of + the four candle reads this function makes per product. It exists for `gather_balances` + (#702), which renders quantity, mark and value and never shows a gate verdict, on an + endpoint the console re-polls every 15 seconds. Rows then carry `ready=False` with + `ready_reason=None`: NOT COMPUTED, and deliberately not the shape of any real verdict -- + `entry_bar_ready` never returns `(False, None)`, so a caller that skipped the work cannot + have its rows mistaken for a product the gate refused. """ mark_granularity = agent_mod._finest_granularity(list(config.market_data.granularities)) # ONE read of the rules table and ONE build per rule, before the row loop -- see # `_gate_granularities`. A lookup per tranche would be one query and one constructor call per # row for an answer the rows share, and would be invisible on any fixture small enough to - # read. - gates = _gate_granularities(repo, config) - fallback = _fallback_granularity(config) + # read. Skipped entirely when the caller does not render a verdict. + gates = _gate_granularities(repo, config) if with_readiness else {} + fallback = _fallback_granularity(config) if with_readiness else None marks: dict[str, tuple[Decimal | None, int | None]] = {} # Keyed on (product, gate granularity), NOT on product alone: one product can hold tranches @@ -287,12 +298,15 @@ def gather_positions(repo: Repository, config: Config, *, now_ts: int) -> Positi marks[product_id] = _mark_for(repo, product_id, mark_granularity) # `.get(...) or fallback` collapses the two unresolvable cases onto one answer: a name # nothing matches, and a name two rules answer to with different granularities. - gate = gates.get(str(raw.get("rule_name") or "")) or fallback - key = (product_id, gate) - if key not in readiness: - readiness[key] = _readiness_for(repo, product_id, gate, config, now_ts) + if with_readiness: + gate = gates.get(str(raw.get("rule_name") or "")) or fallback + key = (product_id, gate) + if key not in readiness: + readiness[key] = _readiness_for(repo, product_id, gate, config, now_ts) + ready, ready_reason = readiness[key] + else: + ready, ready_reason = False, None mark, mark_ts = marks[product_id] - ready, ready_reason = readiness[key] rows.append(_row_from_dict(raw, mark, mark_ts, ready, ready_reason)) return PositionsReport(now_ts=now_ts, rows=tuple(rows)) diff --git a/keel/web/static/js/render.js b/keel/web/static/js/render.js index 4661857..cda9ed6 100644 --- a/keel/web/static/js/render.js +++ b/keel/web/static/js/render.js @@ -1210,7 +1210,9 @@ export function balancesView(data, sort, onSort) { { label: "product", numeric: false, key: "product_id" }, { label: "qty held", numeric: true, key: "qty" }, { label: "mark", numeric: true, key: "mark" }, - { label: "marked at", numeric: false, key: "mark_as_of" }, + // No `key`: `/api/balances` does not declare `mark_as_of` sortable, and a key the + // server will not order by renders a header that looks clickable and is not. + { label: "marked at", numeric: false }, { label: "value", numeric: true, key: "market_value" }, ], assets.map(/** @param {any} row */ (row) => [ diff --git a/tests/commands/test_balances.py b/tests/commands/test_balances.py index f250e2d..5316c71 100644 --- a/tests/commands/test_balances.py +++ b/tests/commands/test_balances.py @@ -269,3 +269,123 @@ def test_an_unstamped_mode_reports_no_cash_rather_than_guessing( assert report.mode == "" assert report.cash is None assert report.has_recorded_cash is False + + +# -- the mixed-mark guard, at the level where it can exist (#702 review) ------------------------ + + +def _position_row(product_id: str, qty: str, mark: str | None) -> Any: + from keel.commands.positions import PositionRow + + price = None if mark is None else Decimal(mark) + return PositionRow( + id=1, + product_id=product_id, + rule_name="turtle_breakout", + opened_at=NOW_TS - DAY, + qty=Decimal(qty), + entry_fill=Decimal("100"), + entry_fee=Decimal("1"), + mark=price, + mark_ts=None if price is None else NOW_TS - 900, + market_value=None if price is None else Decimal(qty) * price, + unrealized_pnl=None, + initial_stop=None, + stop_distance=None, + stop_distance_pct=None, + realized_qty=Decimal("0"), + realized_proceeds=Decimal("0"), + realized_fees=Decimal("0"), + ready=True, + ready_reason=None, + ) + + +def test_one_unmarked_tranche_makes_the_whole_asset_value_unknown() -> None: + """The guard is DEFENSIVE and this is the only way to reach it. + + `gather_positions` reads the mark once per product and hands every tranche of it the same + figure, so a product whose tranches disagree cannot arise through that caller -- which means + a test going through `gather_balances` cannot exercise this branch, and one that seeds two + different PRODUCTS (as the first version of this test did) is not exercising it either. + + Driven through `_assets_from` directly, because the guard protects the FOLD, not that + caller: a future caller that assembles rows from more than one read, or a mark cache that + stops being per-product, would produce exactly this state -- and a sum over the priced subset + would render a holding as worth less than it is, which is the failure worth engineering + against on the page an operator checks to see what they have. + """ + from keel.commands.balances import _assets_from + + rows = [ + _position_row("BTC-USD", "2", "150"), + _position_row("BTC-USD", "3", None), + ] + + assets = _assets_from(rows, ("BTC-USD",)) + + assert assets[0].qty == Decimal("5"), "the holding is known" + assert assets[0].market_value is None, "its value is not -- never the priced subset's sum" + + +def test_a_fully_marked_asset_still_sums(tmp_path: Path) -> None: + """The other side of the guard: nothing is withheld when every tranche has a mark.""" + from keel.commands.balances import _assets_from + + rows = [ + _position_row("BTC-USD", "2", "150"), + _position_row("BTC-USD", "3", "150"), + ] + + assert _assets_from(rows, ("BTC-USD",))[0].market_value == Decimal("750") + + +def test_the_asset_order_follows_the_reports_product_order(tmp_path: Path) -> None: + """Pinned rather than argued. Sorted or set-ordered, the page would reorder itself between + reads for no reason a reader could see.""" + from keel.commands.balances import _assets_from + + rows = [ + _position_row("SOL-USD", "1", "10"), + _position_row("BTC-USD", "2", "150"), + ] + + assets = _assets_from(rows, ("SOL-USD", "BTC-USD")) + + assert [row.product_id for row in assets] == ["SOL-USD", "BTC-USD"] + + +# -- the read cost of one request (#702 review) ------------------------------------------------ + + +def test_balances_does_not_pay_for_the_entry_gate_it_never_renders( + repo: Repository, tmp_path: Path +) -> None: + """This page shows quantity, mark and value. It does not show the entry-gate verdict, and it + must not pay to compute one. + + Measured before the fix: three products cost 12 candle reads and a rules read per request -- + four reads per product, three of them for `entry_bar_ready` across every configured + granularity, on an endpoint the console re-polls every 15 seconds. The mark needs one read + per product and nothing else. + """ + repo.set_state("equity_state_mode", "live") + repo.record_equity_point(_reading(NOW_TS - 3600, "live", "250")) + repo.insert_rule("turtle_breakout", {"product_id": "BTC-USD", "granularity": "ONE_DAY"}) + for product in ("BTC-USD", "ETH-USD", "SOL-USD"): + _tranche(repo, product, "2") + repo.upsert_candles(product, FINEST, [_candle(NOW_TS - 900, "150")]) + + reads = {"candles": 0} + original = repo.get_candles + + def counting(*args: Any, **kwargs: Any) -> Any: + reads["candles"] += 1 + return original(*args, **kwargs) + + repo.get_candles = counting # type: ignore[method-assign] + gather_balances(repo, _config(tmp_path), now_ts=NOW_TS) + + assert reads["candles"] == 3, ( + f"one mark read per product and no more; got {reads['candles']} for 3 products" + ) diff --git a/tests/web/test_balances_view.py b/tests/web/test_balances_view.py index e3cd165..e5124b2 100644 --- a/tests/web/test_balances_view.py +++ b/tests/web/test_balances_view.py @@ -94,6 +94,28 @@ def test_a_deployment_with_no_recorded_cycle_says_so(tmp_path: Path) -> None: assert built["recorded"]["display"] +def test_a_recorded_cycle_with_no_split_reads_as_recorded_and_absent(tmp_path: Path) -> None: + """The combination `has_recorded_cash` exists to draw: a cycle DID run and record a reading, + and that reading had no cash split. "Nothing has been recorded" and "the last cycle could not + read a split" are different facts, and only the first means this page has nothing yet.""" + from tests.commands.test_balances import _config, _reading + + conn = connect(str(tmp_path / "split.db")) + migrate(conn) + repo = Repository(conn) + repo.set_state("equity_state_mode", "live") + repo.record_equity_point(_reading(BAL_NOW_TS - 3600, "live", None, equity="250")) + + built = web_payload.balances_payload( + gather_balances(repo, _config(tmp_path), now_ts=BAL_NOW_TS) + ) + + assert built["recorded"]["value"] == "true", "a cycle did record a reading" + assert built["cash"]["state"] == "unknown", "it just had no split to record" + assert built["equity"]["value"] == "250" + assert built["cash_as_of"]["value"].endswith("Z"), "the reading still has a time" + + def test_paper_cash_crosses_only_in_paper_mode(tmp_path: Path) -> None: live = web_payload.balances_payload(_report(tmp_path, mode="live", paper_cash="9500")) nested = tmp_path / "p" diff --git a/tests/web/test_client_assets.py b/tests/web/test_client_assets.py index 6f4f81b..48fe04c 100644 --- a/tests/web/test_client_assets.py +++ b/tests/web/test_client_assets.py @@ -1256,6 +1256,13 @@ def _status_view_keys() -> list[str]: _VIEW_ENDPOINTS: tuple[tuple[str, str, str], ...] = ( ("setupView", "data", "/api/setup"), ("activityView", "data", "/api/activity"), + # #659/#701/#702's views were added to the client without being added here, so nothing + # checked that they read keys their endpoint actually sends. Demonstrated on #702: renaming + # `data.hwm` to `data.high_water_mark` in `balancesView` left the whole web suite green, and + # the tile would have rendered blank forever with nothing in the console naming the gap. + ("ordersView", "data", "/api/orders"), + ("positionsView", "data", "/api/positions"), + ("balancesView", "data", "/api/balances"), ("insightsView", "insights", "/api/insights"), ("insightsView", "journal", "/api/journal"), ("rulesView", "data", "/api/rules"),