From a9b70446a065c36243a20ab68601c3691cb5c731 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Fri, 4 Sep 2026 12:34:43 -0400 Subject: [PATCH 1/2] feat(web): the Positions view -- marked at the rails' own price (#701, items 1-4) `keel status` showed a tranche's id, product, rule, qty, entry price and bracket, and `web/payload.py::_position_payload` explained in its own docstring why it showed no P&L: "`OpenPositionStatus` carries neither, so emitting one would mean this layer multiplied `qty` by `entry_price`... the fix, if it is wanted, is upstream." This is that fix, upstream, plus the endpoint and view that read it. THE MARK IS THE RAILS' MARK, AND THAT IS THE WHOLE POINT. `agent._mark_to_market_parts` values a holding at `repo.get_candles(product, finest)[-1].close`; `gather_positions` makes the same read through the same `_finest_granularity`. A positions page quoting a different current price would be a second answer to "what is this worth", and since the first answer moved rail 11's drawdown scalars, the page would be the wrong one. That is asserted across the module boundary rather than claimed. The two readers do NOT share a source for QUANTITY: the agent counts inventory from the filled-orders log -- never from `positions`, deliberately, after the phantom-drawdown bug that taught it -- and this report reads the tranche ledger. They share only the mark. So the reconciliation test is what says the two ledgers agree about what is held, and it fails the day they drift. WHAT IS ABSENT STAYS ABSENT. No cached candle means no mark, and no mark means no market value, no unrealized and no stop distance -- never zero. A zero market value renders a held position as a total loss, which is the most alarming thing this page could say and it would be saying it about missing data. `initial_stop` NULL is "not on this row" (a DCA leg, a pre-v12 tranche), so its distance is absent too: measured against a substituted zero it would read as a position comfortably clear of a stop it does not have. Signed where the sign means something, and nowhere else. `unrealized` and `stop_distance` carry verdicts; a market value does not, because an account is not good for being worth something and a glyph on every balance hides the one figure that matters. A negative stop distance means the tranche is trading THROUGH its protection -- keel is cash-spot and long-only, so a stop always sits below the mark and the sign has one meaning. `stop_distance_pct` crosses as a raw fraction with no `%`, the posture `ratio` already documents for the drawdown scalars. THE FRESHNESS CHIP IS THE ENTRY GATE'S VERDICT, not a data age. `missing`/`behind`/ `unconfirmed` are `entry_bar_ready`'s own words for why the agent would refuse to open here. `freshness.assess` tolerates the normal forming-bar lag; `entry_bar_ready` refuses a one-bar-late finer series because that lag is exactly what produces a duplicate real-money order. Showing the softer number would tell a reader the feed is fine while the engine's own gate is refusing it -- and this chip is the most common answer to "why has nothing happened", which is why it sits beside the money rather than under a disclosure. NO CLOSE ACTION, EVER. #701's own refusal, pinned on the source rather than trusted: an exit goes through the typed-phrase friction of the terminal path, because a panic tap on a table row must not be the last line of defence. The test fails the build the day the affordance arrives looking like an obvious convenience. A GUARD FOR A BUG THAT SHIPPED SILENTLY IN THE WRITING OF THIS. The view was first written calling `sorting(sort, onSort)` -- `sorting` is a TYPEDEF and a parameter name in `render.js`, never a function -- and every gate stayed green, because mypy does not read JavaScript, ruff does not either, and the view tests assert over source text rather than executing it. The page would have thrown `ReferenceError` on first render. `test_every_call_resolves_to_something_the_module_has` now scans both derivation-free modules for calls that are not in scope where they are made. Scoped PER FUNCTION, which is the part that took two attempts: the first version pooled every function's parameters into one module-wide set, so `sorting` -- a parameter of `table` and `headerCell` -- counted as defined inside `positionsView`, and the scan passed when the original bug was reinstated. It only came out because the guard was itself mutation-tested against the bug it was written for. Not in this commit, and split rather than faked: #701's attestation chip wants a quarterly expiry that neither `asset_attestations` nor `instrument_attestations` records (every other attestation table in the schema carries `attest_due_ts`; these two do not), and a "Purif: 0.38%" ratio whose denominator nothing holds -- `owed_by_asset` gives dollars, and what they are a percentage OF is a judgement. Co-Authored-By: Claude Opus 5 (1M context) --- keel/commands/positions.py | 265 +++++++++++++++++++++++++++ keel/web/api.py | 49 +++++ keel/web/payload.py | 104 +++++++++++ keel/web/static/index.html | 1 + keel/web/static/js/main.js | 3 + keel/web/static/js/render.js | 117 ++++++++++++ keel/web/staticfiles.py | 1 + tests/commands/test_positions.py | 302 +++++++++++++++++++++++++++++++ tests/web/test_client_assets.py | 94 ++++++++++ tests/web/test_positions_view.py | 222 +++++++++++++++++++++++ 10 files changed, 1158 insertions(+) create mode 100644 keel/commands/positions.py create mode 100644 tests/commands/test_positions.py create mode 100644 tests/web/test_positions_view.py diff --git a/keel/commands/positions.py b/keel/commands/positions.py new file mode 100644 index 0000000..f4566c5 --- /dev/null +++ b/keel/commands/positions.py @@ -0,0 +1,265 @@ +"""`keel`'s positions report -- what is held, what it is worth, and how close it is to its stop. + +**Strictly read-only.** No broker call, no write, no rail touched: a pure view over +`repo.get_open_positions()`, the candle cache, and `keel.data.freshness`. Same two layers as +`status.py`/`orders.py` -- a pure `gather_positions(repo, config, now_ts) -> PositionsReport` +that any renderer can call, and no click dependency in the builder. + +WHY THIS MODULE EXISTS RATHER THAN MORE FIELDS ON `OpenPositionStatus` (#701). `web/payload.py`'s +`_position_payload` carries no P&L and says why: "`OpenPositionStatus` carries neither, so +emitting one would mean this layer multiplied `qty` by `entry_price`... the fix, if it is wanted, +is upstream." This is that fix. It is a separate report rather than a wider status row because +the arithmetic here needs the candle cache and the config's granularities, which `gather_status` +does not read and should not start reading for the sake of one table inside it. + +THE MARK IS THE RAILS' MARK, AND THAT IS THE POINT. `agent._mark_to_market_parts` values a +holding at `repo.get_candles(product, finest)[-1].close`, and so does this, through the same +`_finest_granularity`. A positions page quoting a different current price would be a second +answer to "what is this worth" -- and since the first answer is the one that moved rail 11's +drawdown scalars, the page would be the wrong one. Everything derived below (market value, +unrealized P&L, stop distance) is derived from THAT number or from nothing at all. + +WHAT IS ABSENT STAYS ABSENT. A product with no cached candle has no mark, and a row with no mark +reports `None` for every figure that depends on one -- never zero. Zero market value renders a +held position as a total loss, which is the most alarming thing this page could say, and it +would be saying it about missing data. `initial_stop` is `None` for a DCA leg or a pre-v12 +tranche, so its distance is `None` too: a distance measured against a substituted zero would +read as a position comfortably clear of a stop it does not have. + +NO CLOSE ACTION, EVER (#701's own refusal). This module places nothing and cancels nothing. +Exits degrade to the typed-phrase friction path; a panic tap on a table row must never be the +last line of defence. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from decimal import Decimal, DivisionByZero, InvalidOperation +from typing import Any + +from keel_core.types import Granularity + +from keel import agent as agent_mod +from keel.config import Config +from keel.data import freshness as freshness_mod +from keel.data.repository import Repository + + +@dataclass(frozen=True) +class PositionRow: + """One open TRANCHE, projected onto what a reader needs. + + `qty` is what is STILL HELD -- `reduce_position` shrinks it on a scale-out and carries the + sold legs in the `realized_*` accumulators, so the two sides together are the tranche's whole + story and either alone understates it. + """ + + id: int + product_id: str + rule_name: str + opened_at: int + + #: The quantity still held, after any partial exits. + qty: Decimal + + #: What this tranche paid, and what it paid to get in. The fee is here because `keel status` + #: never showed it and it is the half of a round trip a reader most often forgets. + entry_fill: Decimal + entry_fee: Decimal + + #: The latest close of the FINEST configured series -- the agent's own mark. `None` when the + #: product has no cached candle at all: not observed, never "worth nothing". + mark: Decimal | None + + #: The instant that mark was recorded, so a reader can see how old the valuation is without + #: this module choosing a staleness threshold (`freshness` owns that judgement; see `ready`). + mark_ts: int | None + + #: `qty * mark`, and `qty * (mark - entry_fill)`. Both `None` without a mark. + market_value: Decimal | None + unrealized_pnl: Decimal | None + + #: The stop this tranche was SIZED against (`positions.initial_stop`). `None` means "not on + #: this row" -- a DCA leg, or a tranche predating v12 -- and NOT "no stop". + initial_stop: Decimal | None + + #: `mark - initial_stop`, SIGNED: a tranche trading THROUGH its stop is the state an operator + #: most needs to see, and an absolute distance would render it identically to one safely + #: above. `stop_distance_pct` is that distance as a fraction OF THE MARK. Both `None` when + #: either the stop or the mark is missing. + stop_distance: Decimal | None + stop_distance_pct: Decimal | None + + #: The legs already sold (#502). Zero rather than `None` when nothing has been: the + #: repository's own convention, because "never partially exited" and "has realized nothing" + #: are the same fact. + realized_qty: Decimal + realized_proceeds: Decimal + realized_fees: Decimal + + #: `entry_bar_ready`'s verdict for this product's entry-gate series -- the gate outcome the + #: agent itself would compute, not a data age. `ready_reason` is `"missing" | "behind" | + #: "unconfirmed"`, or `None` when ready. + ready: bool + ready_reason: str | None + + +@dataclass(frozen=True) +class PositionsReport: + now_ts: int + rows: tuple[PositionRow, ...] + + @property + def open_count(self) -> int: + """How many tranches this report holds. + + Derived rather than stored, for the reason `OrdersReport.shown_count` is: a stored count + can drift from the list it describes, and `keel/web/payload.py` may not call `len()` + (Rule 6e of `tests/commands/test_console_thinness.py`). + """ + return len(self.rows) + + @property + def products(self) -> tuple[str, ...]: + """Every product with an open tranche, in first-seen order -- what a grouped view keys + on. Held here so no renderer builds its own list and reaches a different one.""" + seen: list[str] = [] + for row in self.rows: + if row.product_id not in seen: + seen.append(row.product_id) + return tuple(seen) + + +def _safe_ratio(numerator: Decimal, denominator: Decimal) -> Decimal | None: + """`numerator / denominator`, or `None` when that is not a finite answer. + + A non-positive or non-finite mark reaches here from the candle cache, which is data this + module did not write. `Decimal` raises on a zero denominator rather than returning an + infinity, and an unguarded division would take a read-only page down over one bad row. + """ + if denominator == 0: + return None + try: + ratio = numerator / denominator + except (DivisionByZero, InvalidOperation): + return None + return ratio if ratio.is_finite() else None + + +def _mark_for( + repo: Repository, product_id: str, granularity: Granularity | None +) -> tuple[Decimal | None, int | None]: + """The agent's mark for `product_id`: the newest close of the finest configured series. + + Deliberately the same read `agent._mark_to_market_parts` makes. `None` when no series is + configured or nothing is cached -- and `None` also when the cached close is non-positive, + because a zero or negative price is not a valuation this page should publish as one. + """ + if granularity is None: + return None, None + candles = repo.get_candles(product_id, granularity) + if not candles: + return None, None + newest = candles[-1] + if newest.close <= 0: + return None, newest.ts + return newest.close, newest.ts + + +def _readiness_for( + repo: Repository, product_id: str, config: Config, now_ts: int +) -> tuple[bool, str | None]: + """`entry_bar_ready`'s verdict for this product, as `(ready, reason)`. + + The ENTRY-GATE question, not a staleness alert: `freshness.assess` tolerates the normal + forming-bar lag on purpose, and `entry_bar_ready` deliberately does not, because a one-bar- + late finer series is exactly the condition that produces a duplicate real-money order. A + positions page showing the softer verdict would tell a reader the feed is fine while the + agent's own gate is refusing to trade on it. + """ + granularities = config.market_data.granularities + coarsest = max(granularities, key=_granularity_rank) if granularities else None + if coarsest is None: + return False, "missing" + candles_by_tf = {g: repo.get_candles(product_id, g) for g in granularities} + verdict = freshness_mod.entry_bar_ready(candles_by_tf, coarsest, now_ts) + return verdict.ready, verdict.reason + + +def _granularity_rank(granularity: Granularity) -> int: + """Ordering over granularities, read from the agent's own table so the two cannot disagree + about which series is finer.""" + return agent_mod._GRANULARITY_ORDER.get(granularity, 0) + + +def gather_positions(repo: Repository, config: Config, *, now_ts: int) -> PositionsReport: + """Every OPEN tranche, marked and judged. + + Open only: a closed tranche is a `trade_outcomes` row and belongs to the journal, which + already reports it with its realised P&L. Showing both here would put one trade in two places + with two different figures for it. + + 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. + """ + granularity = agent_mod._finest_granularity(list(config.market_data.granularities)) + + marks: dict[str, tuple[Decimal | None, int | None]] = {} + readiness: dict[str, tuple[bool, str | None]] = {} + rows: list[PositionRow] = [] + for raw in repo.get_open_positions(): + product_id = str(raw.get("product_id") or "") + if product_id not in marks: + marks[product_id] = _mark_for(repo, product_id, granularity) + readiness[product_id] = _readiness_for(repo, product_id, config, now_ts) + mark, mark_ts = marks[product_id] + ready, ready_reason = readiness[product_id] + rows.append(_row_from_dict(raw, mark, mark_ts, ready, ready_reason)) + return PositionsReport(now_ts=now_ts, rows=tuple(rows)) + + +def _row_from_dict( + raw: dict[str, Any], + mark: Decimal | None, + mark_ts: int | None, + ready: bool, + ready_reason: str | None, +) -> PositionRow: + """One repository dict, projected. Every judgement this report makes is made here, once, so + no renderer has to make it twice.""" + qty = raw.get("qty") or Decimal("0") + entry_fill = raw.get("entry_fill") or Decimal("0") + initial_stop = raw.get("initial_stop") + + market_value = None if mark is None else qty * mark + unrealized = None if mark is None else qty * (mark - entry_fill) + + if mark is None or initial_stop is None: + stop_distance: Decimal | None = None + stop_distance_pct: Decimal | None = None + else: + stop_distance = mark - initial_stop + stop_distance_pct = _safe_ratio(stop_distance, mark) + + return PositionRow( + id=int(raw["id"]), + product_id=str(raw.get("product_id") or ""), + rule_name=str(raw.get("rule_name") or ""), + opened_at=int(raw.get("opened_at") or 0), + qty=qty, + entry_fill=entry_fill, + entry_fee=raw.get("entry_fee") or Decimal("0"), + mark=mark, + mark_ts=mark_ts, + market_value=market_value, + unrealized_pnl=unrealized, + initial_stop=initial_stop, + stop_distance=stop_distance, + stop_distance_pct=stop_distance_pct, + realized_qty=raw.get("realized_qty") or Decimal("0"), + realized_proceeds=raw.get("realized_proceeds") or Decimal("0"), + realized_fees=raw.get("realized_fees") or Decimal("0"), + ready=ready, + ready_reason=ready_reason, + ) diff --git a/keel/web/api.py b/keel/web/api.py index 668974b..845adfd 100644 --- a/keel/web/api.py +++ b/keel/web/api.py @@ -304,6 +304,33 @@ def read_orders(cfg: ServeConfig, query: Query, _state: Any, _now_ts: int) -> di return payload.orders_payload(report) +def read_positions(cfg: ServeConfig, _query: Query, _state: Any, now_ts: int) -> dict[str, Any]: + """What is held right now, marked at the price the rails used (#701). + + **READ ONLY, and there is no write route on this path.** #701's own refusal: a position is + closed through the typed-phrase friction of the exit path, never a table-row tap. A panic tap + must not be the last line of defence, so the affordance does not exist here to be tapped. + + No `?limit=` and no `?scope=`. Both exist on `/api/orders` because a book of orders grows + without bound; OPEN tranches do not -- the number is bounded by what the deployment holds + right now, and rail 4's concurrent-position cap bounds that. A cap here would hide a position + an operator is looking for, which is the one thing this page must never do. + + `config` is loaded because the mark comes from the FINEST configured series -- the same read + `agent._mark_to_market_parts` makes. Reading a different series would put a different current + price on the page from the one that moved rail 11's drawdown scalars. + """ + from keel.commands.positions import gather_positions + + repo = open_repo(cfg.db_path) + try: + config = load_config(cfg.config_path) + report = gather_positions(repo, config, now_ts=now_ts) + finally: + close_repo(repo) + return payload.positions_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. @@ -553,6 +580,28 @@ class ApiRoute: "created_at", ), ), + "/api/positions": ApiRoute( + html_route="/positions", + read=read_positions, + collection="rows", + # The figures an operator scans by. `unrealized` and `stop_distance` first in intent: + # "what is losing" and "what is closest to its stop" are the two questions this page + # exists to answer, and both are sorted server-side because both are Decimals that a + # browser would compare as doubles. + sortable=( + "product_id", + "rule_name", + "opened_at", + "qty", + "entry_fill", + "mark", + "market_value", + "unrealized", + "initial_stop", + "stop_distance", + "stop_distance_pct", + ), + ), "/api/rules": ApiRoute( html_route="/rules", read=read_rules, diff --git a/keel/web/payload.py b/keel/web/payload.py index e69d9fc..5ab412c 100644 --- a/keel/web/payload.py +++ b/keel/web/payload.py @@ -133,6 +133,7 @@ RuleTrackRecord, ) from keel.commands.orders import OrderRow, OrdersReport + from keel.commands.positions import PositionRow, PositionsReport from keel.commands.status import ( AutonomyStatus, MarketSessionStatus, @@ -1526,6 +1527,109 @@ def orders_payload(report: OrdersReport) -> dict[str, Any]: } +#: The entry-gate verdict, styled (#701). `entry_bar_ready`'s vocabulary, not a staleness one: +#: these words say why the AGENT would refuse to open a position on this product right now. +#: +#: All three are WARN rather than BAD. None of them is a loss or a broken deployment -- a feed +#: catches up, an unconfirmed bar confirms -- but each one means keel cannot act on this product +#: at this moment, and a reader scanning for "why did nothing happen" must be able to find them. +_READINESS_STATES: Mapping[str, str] = { + "missing": WARN, + "behind": WARN, + "unconfirmed": WARN, +} + +#: What each verdict means, spelled out. The word alone is a term of art; the sentence is what a +#: reader who has not read `freshness.py` can act on. +_READINESS_NOTES: Mapping[str, str] = { + "missing": "no cached bar for the entry-gate series -- keel would not open here", + "behind": "the entry-gate series is behind its expected bar -- keel would not open here", + "unconfirmed": "the newest bar is not confirmed closed by a finer series -- keel would wait", +} + + +def _readiness_field(ready: bool, reason: str | None) -> Field: + """The freshness chip: the ENTRY GATE's own verdict for one product. + + Deliberately not `_freshness_payload`'s age. That one answers "how old is this data", which + `freshness.assess` tolerates a forming bar for; this answers "would the agent trade on it", + which `entry_bar_ready` refuses a one-bar-late finer series for -- because that lag is + exactly the condition that produces a duplicate real-money order. A page showing the softer + number would tell a reader the feed is fine while the engine's own gate is refusing it. + + A `ready` row says so plainly rather than going blank: "nothing is wrong" is a finding on a + page whose other rows explain why keel is idle. + """ + if ready: + return label("ready", display="entry gate ready", state=GOOD) + word = reason or "unknown" + return label( + word, + display=_READINESS_NOTES.get(word, "the entry gate would not open here"), + state=_READINESS_STATES.get(word, UNKNOWN), + ) + + +def _position_row_payload(row: PositionRow) -> dict[str, Any]: + """One open tranche, placed. Nothing is decided here. + + Every judgement was made by `keel/commands/positions.py`: what the mark is, whether there is + one, what the stop distance is and whether the entry gate would open. This function chooses a + `state` word and a symbol for each figure, which is all -- and is what makes this page and + `keel status` incapable of disagreeing about the same tranche. + + **`unrealized` and `stop_distance` are the only two figures that carry a verdict**, because + they are the only two whose sign means something. A market value is a magnitude: an account + is not good for being worth something, and a glyph on every balance would hide the one figure + that matters. On `stop_distance`, negative means the tranche is trading THROUGH the + protection it was sized against -- keel is cash-spot and long-only (`CashAccountRequired`, + #372), so a stop always sits below the mark and the sign has one meaning. + + **No notional, no cost basis, no ratio this layer computed.** Everything here is on the + report. `stop_distance_pct` crosses as the raw FRACTION with no `%`, the same posture `ratio` + documents for the drawdown scalars -- rescaling it by 100 would be arithmetic Rule 2 forbids. + """ + return { + "id": str(row.id), + "product_id": row.product_id, + "rule_name": row.rule_name, + "opened_at": moment(row.opened_at), + "qty": quantity(row.qty), + "entry_fill": money(row.entry_fill), + "entry_fee": money(row.entry_fee), + "mark": money(row.mark), + "mark_at": moment(row.mark_ts), + "market_value": money(row.market_value), + "unrealized": money(row.unrealized_pnl, signed=True), + "initial_stop": money(row.initial_stop), + "stop_distance": money(row.stop_distance, signed=True), + "stop_distance_pct": ratio(row.stop_distance_pct), + "realized_qty": quantity(row.realized_qty), + "realized_proceeds": money(row.realized_proceeds), + "realized_fees": money(row.realized_fees), + "freshness": _readiness_field(row.ready, row.ready_reason), + } + + +def positions_payload(report: PositionsReport) -> dict[str, Any]: + """`gather_positions`'s `PositionsReport`, as JSON. + + `open_count` and `products` are READ from the report, never measured here -- Rule 6e bans + `len()` in this module, and both properties exist on the report so the ban costs nothing. + + `products` is the grouping key a view renders sections from. It is the report's own list, in + its own order, so a client cannot build a second one and reach a different answer about which + products this book holds. + """ + return { + "as_of": iso(report.now_ts), + "generated_at": moment(report.now_ts), + "open_count": count(report.open_count), + "products": [str(product) for product in report.products], + "rows": [_position_row_payload(row) for row in report.rows], + } + + # -- 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 fbe8c97..0aac406 100644 --- a/keel/web/static/index.html +++ b/keel/web/static/index.html @@ -118,6 +118,7 @@
  • Setup
  • Activity
  • Orders
    • +
  • Positions
  • Insights
  • Rules
  • Venues
    • diff --git a/keel/web/static/js/main.js b/keel/web/static/js/main.js index 108b2d8..123975e 100644 --- a/keel/web/static/js/main.js +++ b/keel/web/static/js/main.js @@ -45,6 +45,7 @@ import { insightsView, modeBadge, ordersView, + positionsView, refusedView, rulesView, setupView, @@ -97,6 +98,7 @@ const ROUTES = [ { name: "setup", label: "Setup", endpoints: ["setup"] }, { name: "activity", label: "Activity", endpoints: ["activity"] }, { name: "orders", label: "Orders", endpoints: ["orders"] }, + { name: "positions", label: "Positions", endpoints: ["positions"] }, { name: "insights", label: "Insights", endpoints: ["insights", "journal"] }, { name: "rules", label: "Rules", endpoints: ["rules"] }, { name: "venues", label: "Venues", endpoints: ["venues"] }, @@ -424,6 +426,7 @@ function mount(route, readings) { sorter(route, route.endpoints[1]), ); } + if (route.name === "positions") return positionsView(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 a9e089d..ad3d02f 100644 --- a/keel/web/static/js/render.js +++ b/keel/web/static/js/render.js @@ -1168,6 +1168,123 @@ function jobPanel(job) { * @param {(scope: string) => void} onScope * @returns {DocumentFragment} */ +/** + * The Positions view (#701): what is held, what it is worth, and how close it is to its stop. + * + * ── NO CLOSE ACTION, AND THAT IS THE DESIGN ────────────────────────────────────────────────── + * + * Alpaca's positions page has a per-row close. This one does not, ever. An exit goes through the + * typed-phrase friction of the terminal path, because a panic tap on a table row must not be the + * last line of defence between an operator and an unplanned market sell. The absence is pinned by + * `tests/web/test_positions_view.py::test_the_positions_view_has_no_close_action_anywhere`, so it + * survives the day it looks like an obvious convenience to add. + * + * ── GROUPED BY THE REPORT'S OWN PRODUCT LIST ───────────────────────────────────────────────── + * + * `data.products` rather than a set this file assembles from the rows: two answers to "which + * products does this book hold" is one too many, and the ordered one is already on the report. + * A product holds several TRANCHES -- that is what tranches are for -- so each section is a + * table of them rather than one row pretending to be the position. + * + * ── THE CHIP EXPLAINS THE IDLE DEPLOYMENT ──────────────────────────────────────────────────── + * + * `freshness` is the ENTRY GATE's verdict, not a data age: `missing`/`behind`/`unconfirmed` are + * the agent's own reasons for refusing to open here. It is the most common answer to "why has + * nothing happened", which is why it sits beside the money rather than under a disclosure. + * + * @param {any} data `/api/positions`'s `data`. + * @param {any} sort + * @param {(column: string) => void} onSort + * @returns {DocumentFragment} + */ +export function positionsView(data, sort, onSort) { + const fragment = document.createDocumentFragment(); + fragment.append(el("h1", undefined, "Positions")); + + const sub = el("p", "sub"); + sub.append(field(data.generated_at), " · "); + sub.append(field(data.open_count), " open tranche(s)"); + fragment.append(sub); + + const rows = data.rows || []; + if (rows.length === 0) { + // A real answer, not a blank panel: an account holding nothing is an ordinary state for a + // daily agent between entries, and it is not the same as a page that failed to load. + fragment.append(el("p", "empty", "No open positions. keel is holding nothing right now.")); + return fragment; + } + + for (const product of data.products || []) { + const held = rows.filter(/** @param {any} row */ (row) => row.product_id === product); + const id = ["h-pos", product].join("-"); + fragment.append(heading(id, product)); + + // The chip belongs to the PRODUCT, not the tranche: the entry gate asks about a series, so + // every tranche of one product shares one verdict and repeating it per row would suggest + // they could differ. + if (held.length === 0) continue; + const chip = el("p", "note"); + chip.append("entry gate: ", field(held[0].freshness)); + fragment.append(chip); + + fragment.append( + table( + id, + [ + { label: "opened (UTC)", numeric: false, key: "opened_at" }, + { label: "rule", numeric: false, key: "rule_name" }, + { label: "qty held", numeric: true, key: "qty" }, + { label: "entry", numeric: true, key: "entry_fill" }, + { label: "entry fee", numeric: true, key: "entry_fee" }, + { label: "mark", numeric: true, key: "mark" }, + { label: "value", numeric: true, key: "market_value" }, + { label: "unrealized", numeric: true, key: "unrealized" }, + { label: "stop", numeric: true, key: "initial_stop" }, + { label: "to stop", numeric: true, key: "stop_distance" }, + { label: "to stop %", numeric: true, key: "stop_distance_pct" }, + ], + held.map(/** @param {any} row */ (row) => [ + row.opened_at, + plain(row.rule_name) || "—", + row.qty, + row.entry_fill, + row.entry_fee, + row.mark, + row.market_value, + row.unrealized, + row.initial_stop, + row.stop_distance, + row.stop_distance_pct, + ]), + "No open tranches for this product.", + { sort: sort, onSort: onSort }, + ), + ); + + for (const row of held) { + // The realized side, under a disclosure: a scaled-out tranche has legs already booked, and + // they belong beside the running position rather than in the journal's separate account of + // the same trade. Collapsed because most tranches have never been scaled out. + const node = el("details", "row"); + const summary = el("summary"); + summary.append("tranche ", plain(row.id), " · realized legs"); + node.append(summary); + node.append( + gridCard([ + kv("realized qty", row.realized_qty), + kv("realized proceeds", row.realized_proceeds), + kv("realized fees", row.realized_fees), + kv("marked at", row.mark_at), + ]), + ); + fragment.append(node); + } + } + + return fragment; +} + + export function ordersView(data, sort, onSort, onScope, onStatus) { const fragment = document.createDocumentFragment(); fragment.append(el("h1", undefined, "Orders")); diff --git a/keel/web/staticfiles.py b/keel/web/staticfiles.py index a0e0ad5..abac86d 100644 --- a/keel/web/staticfiles.py +++ b/keel/web/staticfiles.py @@ -141,6 +141,7 @@ def resolve_static_asset(root: Path, url_path: str) -> Path | None: "setup", "activity", "orders", + "positions", "insights", "rules", "venues", diff --git a/tests/commands/test_positions.py b/tests/commands/test_positions.py new file mode 100644 index 0000000..2363f46 --- /dev/null +++ b/tests/commands/test_positions.py @@ -0,0 +1,302 @@ +"""The positions report -- issue #701. + +`positions` stores one row per TRANCHE, and until now the only projection of it was the small +table inside `keel status`: id, product, rule, qty, entry price, bracket. `web/payload.py`'s +`_position_payload` says in its own docstring why it carries no P&L -- "`OpenPositionStatus` +carries neither, so emitting one would mean this layer multiplied `qty` by `entry_price`... the +fix, if it is wanted, is upstream". This module is that fix, upstream. + +The property the whole thing rests on is that the MARK is the one the rails used. `agent` +values equity from `repo.get_candles(product, finest)[-1].close`; so does this. A positions page +showing a different current price from the one that moved the drawdown scalars would be two +answers to "what is this worth", and the page would be the wrong one. +""" + +from __future__ import annotations + +from decimal import Decimal +from pathlib import Path +from typing import Any + +import pytest +from keel_core.types import Candle, Granularity + +from keel import agent as agent_mod +from keel.commands.positions import gather_positions +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 +PRODUCT = "BTC-USD" + + +@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 _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 _open_tranche(repo: Repository, **overrides: Any) -> int: + row: dict[str, Any] = { + "product_id": PRODUCT, + "rule_name": "turtle_breakout", + "opened_at": NOW_TS - DAY, + "qty": Decimal("2"), + "entry_fill": Decimal("100"), + "entry_fee": Decimal("1.20"), + "initial_stop": Decimal("90"), + } + row.update(overrides) + return repo.open_position(**row) + + +#: The FINEST series `tests/conftest.py::VALID_CONFIG_YAML` configures, and therefore the one +#: the agent marks against. Not hardcoded out of habit: a helper that wrote any other series +#: would leave every mark `None` and every assertion below would fail for the wrong reason. +FINEST = Granularity.FIFTEEN_MINUTE + + +def _mark(repo: Repository, close: str, granularity: Granularity = FINEST) -> None: + repo.upsert_candles(PRODUCT, granularity, [_candle(NOW_TS - 900, close)]) + + +# -- the mark, and everything derived from it --------------------------------------------------- + + +def test_a_tranche_is_marked_at_the_latest_close(repo: Repository, tmp_path: Path) -> None: + _open_tranche(repo) + _mark(repo, "150") + + row = gather_positions(repo, _config(tmp_path), now_ts=NOW_TS).rows[0] + + assert row.mark == Decimal("150") + assert row.market_value == Decimal("2") * Decimal("150") + assert row.unrealized_pnl == Decimal("2") * (Decimal("150") - Decimal("100")) + + +def test_an_unmarked_product_reports_no_mark_rather_than_a_zero( + repo: Repository, tmp_path: Path +) -> None: + """No candle is "not observed", never "worth nothing". A zero here would render a held + position as a total loss, which is the single most alarming thing a positions page could say + and it would be saying it about missing data.""" + _open_tranche(repo) + + row = gather_positions(repo, _config(tmp_path), now_ts=NOW_TS).rows[0] + + assert row.mark is None + assert row.market_value is None + assert row.unrealized_pnl is None + + +def test_unrealized_is_negative_while_the_position_is_under_water( + repo: Repository, tmp_path: Path +) -> None: + _open_tranche(repo) + _mark(repo, "80") + + row = gather_positions(repo, _config(tmp_path), now_ts=NOW_TS).rows[0] + + assert row.unrealized_pnl == Decimal("-40") + + +def test_the_mark_is_the_one_the_rails_valued_the_account_at( + repo: Repository, tmp_path: Path +) -> None: + """THE acceptance criterion. `agent._mark_to_market_parts` values a holding at + `get_candles(product, finest)[-1].close`; a positions page quoting a different current price + would disagree with the equity that moved the drawdown scalars. + + Two granularities are stored with DIFFERENT closes, so a reader of the wrong series fails. + """ + _open_tranche(repo) + repo.upsert_candles(PRODUCT, Granularity.ONE_DAY, [_candle(NOW_TS - DAY, "111")]) + repo.upsert_candles(PRODUCT, Granularity.ONE_HOUR, [_candle(NOW_TS - 3600, "222")]) + repo.upsert_candles(PRODUCT, FINEST, [_candle(NOW_TS - 900, "333")]) + + row = gather_positions(repo, _config(tmp_path), now_ts=NOW_TS).rows[0] + + assert row.mark == Decimal("333"), "the FINEST configured series is the agent's mark" + + +def test_the_entry_fee_is_carried(repo: Repository, tmp_path: Path) -> None: + """What the tranche cost to open, beside what it is worth. `keel status` never showed it and + the fee is the half of a round trip a reader most often forgets.""" + _open_tranche(repo, entry_fee=Decimal("1.20")) + + assert gather_positions(repo, _config(tmp_path), now_ts=NOW_TS).rows[0].entry_fee == Decimal( + "1.20" + ) + + +# -- the stop ----------------------------------------------------------------------------------- + + +def test_the_stop_distance_is_reported_as_a_price_and_a_fraction( + repo: Repository, tmp_path: Path +) -> None: + """How far this tranche is from the protection it was sized against. Both forms, because + "$60 away" and "40% away" answer different questions and neither is derivable in the + browser.""" + _open_tranche(repo, initial_stop=Decimal("90")) + _mark(repo, "150") + + row = gather_positions(repo, _config(tmp_path), now_ts=NOW_TS).rows[0] + + assert row.stop_distance == Decimal("60") + assert row.stop_distance_pct == Decimal("60") / Decimal("150") + + +def test_a_tranche_with_no_recorded_stop_reports_no_distance( + repo: Repository, tmp_path: Path +) -> None: + """`initial_stop` is NULL for a DCA leg or a pre-v12 tranche -- "not on this row", not "no + stop". A distance computed against a substituted zero would read as a position 100% clear of + its stop, which is the most reassuring possible rendering of missing data.""" + _open_tranche(repo, initial_stop=None) + _mark(repo, "150") + + row = gather_positions(repo, _config(tmp_path), now_ts=NOW_TS).rows[0] + + assert row.initial_stop is None + assert row.stop_distance is None + assert row.stop_distance_pct is None + + +def test_no_mark_means_no_stop_distance_either(repo: Repository, tmp_path: Path) -> None: + """The distance is measured from the CURRENT price. With no mark there is nothing to measure + from, and measuring from the entry instead would quietly answer a different question.""" + _open_tranche(repo, initial_stop=Decimal("90")) + + row = gather_positions(repo, _config(tmp_path), now_ts=NOW_TS).rows[0] + + assert row.stop_distance is None + + +def test_a_position_below_its_stop_reports_a_negative_distance( + repo: Repository, tmp_path: Path +) -> None: + """Signed, not absolute. A tranche trading THROUGH its stop is the state an operator most + needs to see, and an absolute distance would render it identically to one safely above.""" + _open_tranche(repo, initial_stop=Decimal("90")) + _mark(repo, "85") + + assert gather_positions(repo, _config(tmp_path), now_ts=NOW_TS).rows[0].stop_distance == ( + Decimal("-5") + ) + + +# -- partial exits -------------------------------------------------------------------------------- + + +def test_realized_legs_are_reported_beside_the_running_position( + repo: Repository, tmp_path: Path +) -> None: + """A scaled-out tranche is one trade with legs already booked. `qty` is what is STILL held, + so without the realized side the row understates what the tranche has done.""" + position_id = _open_tranche(repo, qty=Decimal("2")) + repo.reduce_position( + position_id, + remaining_qty=Decimal("1"), + realized_qty=Decimal("1"), + realized_proceeds=Decimal("140"), + realized_fees=Decimal("0.50"), + ) + _mark(repo, "150") + + row = gather_positions(repo, _config(tmp_path), now_ts=NOW_TS).rows[0] + + assert row.qty == Decimal("1"), "qty is what is still held" + assert row.realized_qty == Decimal("1") + assert row.realized_proceeds == Decimal("140") + assert row.realized_fees == Decimal("0.50") + # And the unrealized side is measured on the REMAINING quantity only. + assert row.unrealized_pnl == Decimal("1") * (Decimal("150") - Decimal("100")) + + +def test_a_tranche_that_never_partially_exited_reports_zeros_not_absences( + repo: Repository, tmp_path: Path +) -> None: + """The repository's own convention (`_position_row_to_dict`): there is no difference between + "never partially exited" and "has realized nothing", so zero invents nothing here.""" + _open_tranche(repo) + + row = gather_positions(repo, _config(tmp_path), now_ts=NOW_TS).rows[0] + + assert row.realized_qty == Decimal("0") + assert row.realized_proceeds == Decimal("0") + + +# -- what the report covers ------------------------------------------------------------------------ + + +def test_only_open_tranches_are_reported(repo: Repository, tmp_path: Path) -> None: + open_id = _open_tranche(repo) + closed_id = _open_tranche(repo) + repo.close_position(closed_id, closed_at=NOW_TS) + + rows = gather_positions(repo, _config(tmp_path), now_ts=NOW_TS).rows + + assert [row.id for row in rows] == [open_id] + + +def test_an_empty_book_is_a_real_answer(repo: Repository, tmp_path: Path) -> None: + report = gather_positions(repo, _config(tmp_path), now_ts=NOW_TS) + + assert report.rows == () + assert report.open_count == 0 + + +# -- the acceptance criterion: one mark, two readers --------------------------------------------- + + +def test_the_unrealized_total_reconciles_with_the_agents_own_equity_read( + repo: Repository, tmp_path: Path +) -> None: + """#701's acceptance criterion, checked across the module boundary rather than asserted. + + The two readers do NOT share a source for quantity: `agent._mark_to_market_parts` counts + inventory from the FILLED ORDERS log (never from `positions`, deliberately -- see its own + note on the phantom-drawdown bug that caused), while this report reads the `positions` + tranche ledger. They share only the MARK. So this test is what says the two ledgers agree + about what is held, and it fails the day they drift -- which is the failure that would put a + different unrealized figure on the page from the one inside rail 11's equity. + """ + from tests.test_agent import FakeBroker, _seed_open_position + + _seed_open_position( + repo, + PRODUCT, + Decimal("2"), + Decimal("100"), + ts=NOW_TS - DAY, + rule_name="turtle_breakout", + ) + _mark(repo, "150") + + rows = gather_positions(repo, _config(tmp_path), now_ts=NOW_TS).rows + page_total = sum((row.unrealized_pnl or Decimal("0") for row in rows), Decimal("0")) + + parts = agent_mod._mark_to_market_parts( + repo, FakeBroker(), [PRODUCT], {PRODUCT: Decimal("150")}, "USD" + ) + + assert parts is not None + assert page_total == parts.unrealized == Decimal("100") diff --git a/tests/web/test_client_assets.py b/tests/web/test_client_assets.py index d5565a2..dc1dfd9 100644 --- a/tests/web/test_client_assets.py +++ b/tests/web/test_client_assets.py @@ -1495,3 +1495,97 @@ def test_every_table_emits_one_cell_per_declared_header() -> None: assert len(pairs) >= 6, f"the table scan found only {len(pairs)} tables; it has stopped working" mismatched = [(headers, cells) for headers, cells in pairs if headers != cells] assert mismatched == [], f"(headers, cells) mismatches: {mismatched}" + + +#: Globals the browser provides, which no module declares and every module may call. +#: +#: Deliberately short. A long list is a list that absorbs a typo -- the whole value of the scan +#: below is that an undefined name is loud, so anything added here should be a real browser API +#: someone can point at. +_BROWSER_GLOBALS = frozenset( + { + "Array", + "Boolean", + "Date", + "FormData", + "JSON", + "Map", + "Math", + "Number", + "Object", + "Set", + "String", + "console", + "document", + "window", + } +) + +#: Keywords that are followed by a parenthesis and are not calls. +_NOT_CALLS = frozenset( + {"await", "catch", "delete", "for", "function", "if", "instanceof", "new", "return", + "super", "switch", "typeof", "void", "while"} +) + + +def _undefined_calls(name: str) -> list[str]: + """Bare-identifier calls in `name` that are not in scope where they are made. + + Scoped PER FUNCTION, and that is the whole difficulty. A first version collected every + function's parameters into one module-wide set, which made `sorting` -- a parameter of + `table` and of `headerCell` -- count as defined inside `positionsView`, and that is exactly + the bug this scan exists to catch. Parameters are in scope in their own function only. + """ + code = _code_only(_source(name)) + + module_level = set(re.findall(r"\bfunction\s+([A-Za-z_$][\w$]*)", code)) + module_level |= set(re.findall(r"^(?:const|let|var)\s+([A-Za-z_$][\w$]*)\s*=", code, re.M)) + for block in re.findall(r"import\s*\{([^}]*)\}", code): + for imported in block.split(","): + imported = imported.strip().split(" as ")[-1].strip() + if imported: + module_level.add(imported) + + undefined: set[str] = set() + for match in re.finditer(r"\bfunction\s+([A-Za-z_$][\w$]*)\s*\(([^)]*)\)\s*\{", code): + body = _balanced_block(code, code.index("{", match.end() - 1))[0] + in_scope = set(module_level) + for param in match.group(2).split(","): + param = param.strip() + if param: + in_scope.add(param) + # Everything declared inside the body, at any depth, plus every arrow parameter -- a + # callback's own argument is in scope for the callback. + in_scope |= set(re.findall(r"\b(?:const|let|var)\s+([A-Za-z_$][\w$]*)", body)) + in_scope |= set(re.findall(r"\(([A-Za-z_$][\w$]*)\)\s*=>", body)) + in_scope |= set(re.findall(r"\b([A-Za-z_$][\w$]*)\s*=>", body)) + called = {m.group(1) for m in re.finditer(r"(? None: + """No JavaScript runs in this suite, so a call to a function that does not exist ships. + + It is not hypothetical. #701's positions table was written calling `sorting(sort, onSort)` -- + `sorting` is a TYPEDEF and a parameter name in this file, never a function -- and every gate + stayed green: mypy does not read JavaScript, ruff does not either, and the view tests assert + over source text rather than executing it. The page would have thrown `ReferenceError` on + first render. + + Scoped to the two derivation-free modules rather than the whole client: they are the ones + that touch almost no browser API, so the short `_BROWSER_GLOBALS` list stays honest. Widening + it to `main.js` would mean listing `fetch`, `setTimeout`, `URL` and friends, and a list long + enough to cover those is long enough to hide a typo. + """ + assert _undefined_calls(name) == [] + + +def test_the_call_scanner_can_fail() -> None: + """The premise. A scanner whose regex missed every call would pass any file.""" + code = "function a() { return b(1); }" + called = {m.group(1) for m in re.finditer(r"(? list[tuple[str, Any]]: + """Every leaf in a parsed JSON document, with the path that reaches it.""" + 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)] + + +# -- the positions payload (#701) --------------------------------------------------------------- + + +def _positions_report(tmp_path: Path, **overrides: Any) -> Any: + from tests.commands.test_positions import _config, _mark, _open_tranche + + conn = connect(str(tmp_path / "positions.db")) + migrate(conn) + repo = Repository(conn) + _open_tranche(repo, **overrides.pop("tranche", {})) + if "mark" in overrides: + mark = overrides.pop("mark") + if mark is not None: + _mark(repo, mark) + else: + _mark(repo, "150") + return gather_positions(repo, _config(tmp_path), now_ts=POS_NOW_TS) + + +def test_no_wire_value_in_the_positions_payload_is_ever_a_json_number(tmp_path: Path) -> None: + """#533's contract, over this payload. Positions carry more money per row than any other + view -- entry, fee, mark, market value, unrealized, stop, realized -- so it is the one most + likely to leak a float.""" + document = json.loads( + json.dumps(web_payload.positions_payload(_positions_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_the_unrealized_leg_carries_its_verdict_and_the_balances_do_not(tmp_path: Path) -> None: + """Rule 3, applied per figure. Unrealized P&L is a gain-or-loss and gets the glyph and the + state; a market value is a magnitude -- an account is not "good" for being worth something, + and a green number beside every held position would make the one that matters invisible.""" + row = web_payload.positions_payload(_positions_report(tmp_path))["rows"][0] + + assert row["unrealized"]["state"] == "good" + assert row["unrealized"]["display"].startswith("\u25b2") + assert row["market_value"]["state"] == "neutral" + assert row["entry_fill"]["state"] == "neutral" + + +def test_a_losing_tranche_reads_as_bad_without_the_client_seeing_a_minus( + tmp_path: Path, +) -> None: + row = web_payload.positions_payload(_positions_report(tmp_path, mark="80"))["rows"][0] + + assert row["unrealized"]["state"] == "bad" + + +def test_an_unmarked_tranche_crosses_as_absent_not_as_zero(tmp_path: Path) -> None: + """The distinction the service drew, surviving serialisation. `$0.00` market value would + render a held position as a total loss on the strength of a missing candle.""" + row = web_payload.positions_payload(_positions_report(tmp_path, mark=None))["rows"][0] + + assert row["mark"]["state"] == "unknown" + assert row["market_value"]["state"] == "unknown" + assert row["unrealized"]["state"] == "unknown" + # The entry side is still known -- it was recorded when the tranche opened. + assert row["entry_fill"]["value"] == "100" + + +def test_the_stop_distance_crosses_as_both_a_price_and_a_fraction(tmp_path: Path) -> None: + """Two questions, two fields. The fraction crosses UNRESCALED and with no `%`, the same + posture `ratio` documents for the drawdown scalars -- multiplying by 100 here would be the + serialiser inventing a figure the report never held.""" + row = web_payload.positions_payload(_positions_report(tmp_path))["rows"][0] + + assert row["stop_distance"]["value"] == "60" + assert row["stop_distance_pct"]["value"] == "0.4" + assert "%" not in json.dumps(row) + + +def test_a_tranche_through_its_stop_is_judged_bad(tmp_path: Path) -> None: + """The state an operator most needs to find by scanning. Below the stop the distance is + negative, and the sign is turned into a verdict HERE so no client reads a minus.""" + row = web_payload.positions_payload(_positions_report(tmp_path, mark="85"))["rows"][0] + + assert row["stop_distance"]["state"] == "bad" + + +def test_a_tranche_with_no_recorded_stop_says_so_rather_than_showing_zero( + tmp_path: Path, +) -> None: + report = _positions_report(tmp_path, tranche={"initial_stop": None}) + row = web_payload.positions_payload(report)["rows"][0] + + assert row["initial_stop"]["state"] == "unknown" + assert row["stop_distance"]["state"] == "unknown" + + +def test_the_freshness_chip_carries_the_entry_gate_verdict(tmp_path: Path) -> None: + """The gate outcome, not a data age. `missing`/`behind`/`unconfirmed` are the agent's own + words for why it would refuse to trade this product right now, and a chip that showed hours + since the last candle instead would answer a softer question than the engine asks.""" + row = web_payload.positions_payload(_positions_report(tmp_path))["rows"][0] + + assert row["freshness"]["value"] in ("ready", "missing", "behind", "unconfirmed") + assert row["freshness"]["state"] in ("good", "warn", "bad", "neutral", "unknown") + assert row["freshness"]["display"], "the chip must say something a human can read" + + +def test_an_unready_product_is_a_warning_not_a_neutral_note(tmp_path: Path) -> None: + """A product the entry gate would refuse is a fact about whether keel can act, so it is + styled as one. Neutral would leave it indistinguishable from an ordinary row.""" + report = _positions_report(tmp_path, mark=None) + row = web_payload.positions_payload(report)["rows"][0] + + assert row["freshness"]["value"] == "missing" + assert row["freshness"]["state"] == "warn" + + +def test_the_report_counts_and_products_cross_from_the_report(tmp_path: Path) -> None: + """Rule 6e: `payload.py` may not call `len()`, so both come off the report.""" + built = web_payload.positions_payload(_positions_report(tmp_path)) + + assert built["open_count"]["value"] == "1" + assert built["products"] == ["BTC-USD"] + + +# -- the view, in the client ---------------------------------------------------------------------- +# +# Source-text assertions: there is no JavaScript runtime in this suite, so what is pinned is that +# the declarations exist and read the right keys -- not that the DOM behaves. + + +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 test_the_positions_view_is_wired_into_the_client_router() -> None: + """A view in `main.js` alone routes on a click and 404s on a reload; a route in Python alone + is a page with nothing to render. Both tables and the renderer, or none of them.""" + from keel.web import staticfiles + + assert "positions" in staticfiles.CLIENT_ROUTES + main_code = _code("main.js") + assert "positionsView" in main_code + assert 'route.name === "positions"' in main_code + assert "export function positionsView" in _code("render.js") + + +def test_the_positions_view_has_no_close_action_anywhere() -> None: + """#701's central refusal, pinned rather than trusted. + + "No close button. Ever." A position is closed through the typed-phrase friction of the exit + path, because a panic tap on a table row must never be the last line of defence. This is the + kind of affordance that arrives later as an obvious convenience, so the absence is asserted + on the source and will fail the build the day someone adds one. + """ + view = _code("render.js").split("export function positionsView")[1][:4000] + for banned in ("close", "sell", "exit", "cancel", "liquidate"): + assert banned not in view.lower(), f"the positions view must offer no {banned} action" + + +def test_the_positions_view_groups_by_the_reports_own_product_list() -> None: + """`data.products`, not a list the client assembles. Two answers to "which products does this + book hold" is one too many, and the report already holds the ordered one.""" + assert "data.products" in _code("render.js") + + +def test_the_positions_view_shows_the_freshness_chip_per_row() -> None: + """The entry-gate verdict, on the page. It is the thing that explains an idle deployment, so + a view that carried every figure and not this one would leave the most common question + unanswered.""" + view = _code("render.js").split("export function positionsView")[1][:4000] + assert "freshness" in view + + +def test_the_positions_view_names_the_stop_distance_both_ways() -> None: + view = _code("render.js").split("export function positionsView")[1][:4000] + assert "stop_distance" in view + assert "stop_distance_pct" in view From 54817841c331fc29a2eb9f1fd7d59ca9197f7045 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Fri, 4 Sep 2026 13:17:28 -0400 Subject: [PATCH 2/2] fix(web): the entry-gate chip asked the wrong series, and the endpoint had no tests (#701) Review findings on this branch, and the two bugs in the fix for the first one. THE CHIP ASKED THE WRONG QUESTION. `_readiness_for` passed the COARSEST configured granularity to `entry_bar_ready` for every row. But `agent._entry_gate_granularity` gates a rule on the timeframe the RULE DECLARES, and falls back to the coarsest only for a rule that declares none -- and `granularity` is a constructor parameter on `TurtleBreakout`, `PullbackContinuation`, `CusumEvent` and `TripleBarrier`. So a tranche opened by a rule on ONE_HOUR was judged on the daily series: with an hourly feed three bars late and the daily series current, the page rendered a green "entry gate ready" over precisely the state the chip exists to expose. The reverse also held -- a product traded only by hourly rules, with no daily bars cached, read "keel would not open here" while the agent traded it fine. Now resolved through the agent's own function, per row. Two degradations, both deliberate: a `rule_name` nothing matches, and a rules row whose params no longer build, both fall back to the coarsest rather than raising. A chip is not worth a 500, and the fallback is what the agent itself uses for a rule that declares nothing. AND TWO BUGS IN THAT FIX, found by review before it landed. The lookup was keyed on `rules.kind` while `positions.rule_name` holds the rule's `name` -- a separate constructor argument that defaults to the kind. Two `turtle_breakout` rows on different timeframes, a configuration this codebase supports, collapsed to whichever row was read last, and one tranche silently inherited the other's granularity; the test written alongside used two different KINDS and so never covered it. Keyed on the name now, and a name that answers to two different granularities maps to the fallback rather than picking one -- which row opened a given tranche is genuinely unknowable from `rule_name` alone, and a chip that guessed would state it with the same confidence as a resolved one. The second: `_build_rule` ran once per TRANCHE rather than once per rule. It runs the rule's real constructor with its validation, and a DCA book is one rule with many tranches, so per-row building was the common case and not the edge one. Once per rule, pinned by a count. Both consequences of the per-row granularity are fixed too: the readiness cache was keyed on product alone, which handed the second tranche of a product the first one's verdict, and the view rendered one chip per product off `held[0]` -- the same error in the UI. The verdict is a per-tranche column now. THE ENDPOINT WAS EXECUTED BY NO TEST. `/api/positions` never reached `test_api.py`'s `API_ROUTES`, the hand-written tuple every generic endpoint pin is parametrised over -- the envelope, the no-JSON-number walk over the real bytes, the cache headers, the nosniff header, the POST refusal. The payload builder was well covered, and that module's own header says those are not the same statement. Added, with a test asserting the tuple equals `web_api.API_ROUTES` so the next route cannot slip out the same way. THE REFUSAL WAS PARTLY ASSERTING ABOUT ANOTHER VIEW. The view tests sliced a fixed `[:4000]` from `positionsView`, which ran 1362 characters past its end into `ordersView`. #701's central refusal -- no close action, ever -- therefore passed partly because `ordersView`'s first 1362 characters happen to contain none of those words, and would have started failing on an edit to code it does not describe. Sliced to the next `export function`. THE CALL SCANNER HAD BLIND SPOTS, including one that hid functions silently: a default parameter value closed the `([^)]*)` character class early, and the function stopped being scanned at all rather than reporting anything. Module-level arrow functions were never walked. Both fixed, with a floor assertion so a scan that walks nothing cannot pass, a positive control per shape, and the remaining limits -- object and class methods, arrow parameters admitted function-wide -- written down rather than implied. Smaller, all confirmed: `mark_at` returned the bar's timestamp beside a `None` mark, putting a valuation time on a valuation the row denies having; `_READINESS_STATES` sat one character from the pre-existing `_READINESS_STATE` for `VenueReadiness`, an unrelated vocabulary, and is now `_ENTRY_GATE_*`; `stop_distance_pct` rendered at `ratio`'s default two places, so a tranche 0.2% through its stop and one 0.2% above it both showed as "0.00" in exactly the range the column exists to make findable; the `or Decimal("0")` fallbacks on NOT NULL columns are direct reads, so the day one becomes nullable is a loud failure rather than a silent zero -- the substitution `_position_row_to_dict` refuses for `initial_stop`, for the same reason; and `ordersView`'s JSDoc, which the new function had been inserted underneath, is back above `ordersView`. That last one is the identical mistake made and fixed in #700. Co-Authored-By: Claude Opus 5 (1M context) --- keel/commands/positions.py | 118 +++++++++++++++--- keel/web/payload.py | 22 ++-- keel/web/static/js/render.js | 73 ++++++----- tests/commands/test_positions.py | 201 +++++++++++++++++++++++++++++++ tests/web/test_api.py | 17 +++ tests/web/test_client_assets.py | 79 ++++++++++-- tests/web/test_positions_view.py | 32 ++++- 7 files changed, 469 insertions(+), 73 deletions(-) diff --git a/keel/commands/positions.py b/keel/commands/positions.py index f4566c5..8c60717 100644 --- a/keel/commands/positions.py +++ b/keel/commands/positions.py @@ -133,9 +133,10 @@ def products(self) -> tuple[str, ...]: def _safe_ratio(numerator: Decimal, denominator: Decimal) -> Decimal | None: """`numerator / denominator`, or `None` when that is not a finite answer. - A non-positive or non-finite mark reaches here from the candle cache, which is data this - module did not write. `Decimal` raises on a zero denominator rather than returning an - infinity, and an unguarded division would take a read-only page down over one bad row. + `_mark_for` already refuses a non-positive close, so the only caller cannot pass a zero + denominator today -- this is the guard for the day a second caller arrives, and for the + non-finite results `Decimal` can produce from values this module did not write. Stated as + a guard rather than removed because a read-only page must not 500 over one bad row. """ if denominator == 0: return None @@ -162,30 +163,93 @@ def _mark_for( return None, None newest = candles[-1] if newest.close <= 0: - return None, newest.ts + # No mark AND no time for it. Returning the bar's ts beside a `None` close would put a + # valuation time on the row for a valuation the row says it does not have. + return None, None return newest.close, newest.ts def _readiness_for( - repo: Repository, product_id: str, config: Config, now_ts: int + repo: Repository, + product_id: str, + granularity: Granularity | None, + config: Config, + now_ts: int, ) -> tuple[bool, str | None]: - """`entry_bar_ready`'s verdict for this product, as `(ready, reason)`. + """`entry_bar_ready`'s verdict for one product on ONE gate granularity, as `(ready, reason)`. The ENTRY-GATE question, not a staleness alert: `freshness.assess` tolerates the normal forming-bar lag on purpose, and `entry_bar_ready` deliberately does not, because a one-bar- late finer series is exactly the condition that produces a duplicate real-money order. A positions page showing the softer verdict would tell a reader the feed is fine while the agent's own gate is refusing to trade on it. + + `granularity` is the caller's, and it must be the one `_entry_gate_granularity` would pick + for THIS position's rule -- see `_gate_granularity_for`. Asking about the coarsest series for + every row (the first version of this) reports that function's FALLBACK as though it were its + answer: correct for a daily deployment, wrong for any rule seeded on a finer timeframe, and + worded with the same confidence either way. """ granularities = config.market_data.granularities - coarsest = max(granularities, key=_granularity_rank) if granularities else None - if coarsest is None: + if granularity is None or not granularities: return False, "missing" candles_by_tf = {g: repo.get_candles(product_id, g) for g in granularities} - verdict = freshness_mod.entry_bar_ready(candles_by_tf, coarsest, now_ts) + verdict = freshness_mod.entry_bar_ready(candles_by_tf, granularity, now_ts) return verdict.ready, verdict.reason +def _gate_granularities(repo: Repository, config: Config) -> dict[str, Granularity | None]: + """`{rule name: the granularity the agent's entry gate would use}`, built ONCE. + + Keyed on the rule's `name` and NOT on `rules.kind`, because `positions.rule_name` holds the + name -- a constructor argument that defaults to the kind and is not the same field. Keyed by + kind, two `turtle_breakout` rows on different timeframes (a configuration this codebase + supports) collapse to whichever row was read last, and one tranche silently inherits the + other's granularity. + + Each rule is built once here rather than once per tranche: `_build_rule` runs the rule's real + constructor with its validation, and a DCA book is one rule with many tranches, so per-row + building is the common case rather than the edge one. + + A NAME THAT ANSWERS TO TWO DIFFERENT GRANULARITIES maps to `None`. `rules.name` is not unique + in the schema, so which row opened a given tranche is genuinely unknowable from `rule_name` + alone -- and a chip that picked one and stated it with the same confidence as a resolved one + would be asserting something nobody can check. `None` sends the caller to the fallback, which + is what the agent itself uses for a rule that declares nothing. + + A row whose params no longer build -- a renamed field, a kind since removed -- is skipped for + the same reason: unknowable, so fall back rather than raise. A chip is not worth a 500. + """ + granularities = list(config.market_data.granularities) + gates: dict[str, Granularity | None] = {} + seen: set[str] = set() + for row in repo.get_rules(): + try: + rule = agent_mod._build_rule(row) + except Exception: + continue + name = str(getattr(rule, "name", "") or row.get("kind") or "") + if not name: + continue + gate = agent_mod._entry_gate_granularity(rule, granularities) + if name in seen and gates.get(name) != gate: + gates[name] = None + else: + gates[name] = gate + seen.add(name) + return gates + + +def _fallback_granularity(config: Config) -> Granularity | None: + """What the agent gates a rule on when the rule declares nothing: the COARSEST configured + series. `_entry_gate_granularity`'s own fallback, and for its reason -- `Dca` reads the daily + bar directly, so gating it on the finest would miss a weeks-stale daily bar entirely.""" + granularities = list(config.market_data.granularities) + if not granularities: + return None + return max(granularities, key=_granularity_rank) + + def _granularity_rank(granularity: Granularity) -> int: """Ordering over granularities, read from the agent's own table so the two cannot disagree about which series is finer.""" @@ -203,18 +267,32 @@ def gather_positions(repo: Repository, config: Config, *, now_ts: int) -> Positi 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. """ - granularity = agent_mod._finest_granularity(list(config.market_data.granularities)) + 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) marks: dict[str, tuple[Decimal | None, int | None]] = {} - readiness: dict[str, tuple[bool, str | None]] = {} + # Keyed on (product, gate granularity), NOT on product alone: one product can hold tranches + # opened by rules on different timeframes, and a per-product cache would hand the second + # tranche the first one's verdict. + readiness: dict[tuple[str, Granularity | None], tuple[bool, str | None]] = {} rows: list[PositionRow] = [] for raw in repo.get_open_positions(): product_id = str(raw.get("product_id") or "") if product_id not in marks: - marks[product_id] = _mark_for(repo, product_id, granularity) - readiness[product_id] = _readiness_for(repo, product_id, config, now_ts) + 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) mark, mark_ts = marks[product_id] - ready, ready_reason = readiness[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)) @@ -228,8 +306,12 @@ def _row_from_dict( ) -> PositionRow: """One repository dict, projected. Every judgement this report makes is made here, once, so no renderer has to make it twice.""" - qty = raw.get("qty") or Decimal("0") - entry_fill = raw.get("entry_fill") or Decimal("0") + # Direct reads, not `raw.get(...) or Decimal("0")`. These three columns are NOT NULL in the + # `positions` DDL, so the fallback could only ever rewrite a zero as itself -- while quietly + # substituting one the day a column became nullable. That is the substitution + # `_position_row_to_dict` deliberately refuses for `initial_stop`, for the same reason. + qty = raw["qty"] + entry_fill = raw["entry_fill"] initial_stop = raw.get("initial_stop") market_value = None if mark is None else qty * mark @@ -246,10 +328,10 @@ def _row_from_dict( id=int(raw["id"]), product_id=str(raw.get("product_id") or ""), rule_name=str(raw.get("rule_name") or ""), - opened_at=int(raw.get("opened_at") or 0), + opened_at=int(raw["opened_at"]), qty=qty, entry_fill=entry_fill, - entry_fee=raw.get("entry_fee") or Decimal("0"), + entry_fee=raw["entry_fee"], mark=mark, mark_ts=mark_ts, market_value=market_value, diff --git a/keel/web/payload.py b/keel/web/payload.py index 5ab412c..1420fc3 100644 --- a/keel/web/payload.py +++ b/keel/web/payload.py @@ -1527,13 +1527,17 @@ def orders_payload(report: OrdersReport) -> dict[str, Any]: } -#: The entry-gate verdict, styled (#701). `entry_bar_ready`'s vocabulary, not a staleness one: -#: these words say why the AGENT would refuse to open a position on this product right now. +#: The entry-gate verdict, styled (#701). Named `_ENTRY_GATE_*` and not `_READINESS_*`: this +#: module already has a `_READINESS_STATE` for `VenueReadiness`, an unrelated vocabulary, and +#: two names one character apart in one file is a mis-edit waiting to happen. +#: +#: `entry_bar_ready`'s vocabulary, not a staleness one: these words say why the AGENT would +#: refuse to open a position on this product right now. #: #: All three are WARN rather than BAD. None of them is a loss or a broken deployment -- a feed #: catches up, an unconfirmed bar confirms -- but each one means keel cannot act on this product #: at this moment, and a reader scanning for "why did nothing happen" must be able to find them. -_READINESS_STATES: Mapping[str, str] = { +_ENTRY_GATE_STATES: Mapping[str, str] = { "missing": WARN, "behind": WARN, "unconfirmed": WARN, @@ -1541,7 +1545,7 @@ def orders_payload(report: OrdersReport) -> dict[str, Any]: #: What each verdict means, spelled out. The word alone is a term of art; the sentence is what a #: reader who has not read `freshness.py` can act on. -_READINESS_NOTES: Mapping[str, str] = { +_ENTRY_GATE_NOTES: Mapping[str, str] = { "missing": "no cached bar for the entry-gate series -- keel would not open here", "behind": "the entry-gate series is behind its expected bar -- keel would not open here", "unconfirmed": "the newest bar is not confirmed closed by a finer series -- keel would wait", @@ -1565,8 +1569,8 @@ def _readiness_field(ready: bool, reason: str | None) -> Field: word = reason or "unknown" return label( word, - display=_READINESS_NOTES.get(word, "the entry gate would not open here"), - state=_READINESS_STATES.get(word, UNKNOWN), + display=_ENTRY_GATE_NOTES.get(word, "the entry gate would not open here"), + state=_ENTRY_GATE_STATES.get(word, UNKNOWN), ) @@ -1603,7 +1607,11 @@ def _position_row_payload(row: PositionRow) -> dict[str, Any]: "unrealized": money(row.unrealized_pnl, signed=True), "initial_stop": money(row.initial_stop), "stop_distance": money(row.stop_distance, signed=True), - "stop_distance_pct": ratio(row.stop_distance_pct), + # FOUR places, not `ratio`'s default two. At two, a tranche 0.2% through its stop and one + # 0.2% above it render as "-0.00" and "0.00" side by side -- illegible in exactly the + # range this column exists to show, since a position near its stop is the one worth + # finding. The paired `stop_distance` still carries the verdict. + "stop_distance_pct": ratio(row.stop_distance_pct, places=4), "realized_qty": quantity(row.realized_qty), "realized_proceeds": money(row.realized_proceeds), "realized_fees": money(row.realized_fees), diff --git a/keel/web/static/js/render.js b/keel/web/static/js/render.js index ad3d02f..ff9cce7 100644 --- a/keel/web/static/js/render.js +++ b/keel/web/static/js/render.js @@ -1138,36 +1138,6 @@ function jobPanel(job) { * @param {(scope: string) => void} onScope * @returns {DocumentFragment} */ -/** - * The orders view (#659): what keel actually bought and sold, and whether anybody agreed to it. - * - * **`placement` is the first column, and that is the argument this view exists to make.** On a - * deployment running `autonomy: ON`, "did I approve this, or did keel place it alone" is the - * first question about an order, not the last -- so it leads the row, ahead of the product and - * ahead of the price. The word and its tone both arrive already decided - * (`payload._order_row_payload`), because `bypass` meaning the same thing as `autonomous` is a - * judgement, and judgements are made in Python. - * - * **`expected` and `actual` are adjacent, with the divergence between them.** That difference is - * realised slippage. Its tone is side-aware and comes from the server for the reason - * `_order_row_payload` states: paying more than expected is bad on a buy and good on a sell, so - * a client colouring the minus sign would be wrong on half the rows. - * - * **`fee` is the figure as recorded and never a rate.** No percentage is computed here, and none - * crosses the wire. A paper row's fee carries a `modelled` badge instead, because - * `PaperTrader` derives it from the configured rate and reading it back as a measurement of that - * rate is circular. - * - * **`raw_response` is not in the payload at all**, so this function could not render it if it - * tried. The venue's order id is, when there is one, and a sentence saying why there is not, - * when there is not. - * - * @param {any} data - * @param {any} sort - * @param {(column: string) => void} onSort - * @param {(scope: string) => void} onScope - * @returns {DocumentFragment} - */ /** * The Positions view (#701): what is held, what it is worth, and how close it is to its stop. * @@ -1192,6 +1162,10 @@ function jobPanel(job) { * the agent's own reasons for refusing to open here. It is the most common answer to "why has * nothing happened", which is why it sits beside the money rather than under a disclosure. * + * It is a COLUMN and not a per-product chip, because the gate granularity is the one the RULE + * that opened the tranche declares. Two tranches of one product, opened by rules on different + * timeframes, have two verdicts -- and a chip above the table would have to pick one. + * * @param {any} data `/api/positions`'s `data`. * @param {any} sort * @param {(column: string) => void} onSort @@ -1223,9 +1197,6 @@ export function positionsView(data, sort, onSort) { // every tranche of one product shares one verdict and repeating it per row would suggest // they could differ. if (held.length === 0) continue; - const chip = el("p", "note"); - chip.append("entry gate: ", field(held[0].freshness)); - fragment.append(chip); fragment.append( table( @@ -1242,6 +1213,11 @@ export function positionsView(data, sort, onSort) { { label: "stop", numeric: true, key: "initial_stop" }, { label: "to stop", numeric: true, key: "stop_distance" }, { label: "to stop %", numeric: true, key: "stop_distance_pct" }, + // PER TRANCHE, not per product. The gate granularity comes from the RULE that opened + // this tranche (`_gate_granularity_for`), so one product holding tranches from rules + // on different timeframes has two verdicts -- a single chip above the table would + // state one of them over the other. + { label: "entry gate", numeric: false, key: "freshness" }, ], held.map(/** @param {any} row */ (row) => [ row.opened_at, @@ -1255,6 +1231,7 @@ export function positionsView(data, sort, onSort) { row.initial_stop, row.stop_distance, row.stop_distance_pct, + row.freshness, ]), "No open tranches for this product.", { sort: sort, onSort: onSort }, @@ -1285,6 +1262,36 @@ export function positionsView(data, sort, onSort) { } +/** + * The orders view (#659): what keel actually bought and sold, and whether anybody agreed to it. + * + * **`placement` is the first column, and that is the argument this view exists to make.** On a + * deployment running `autonomy: ON`, "did I approve this, or did keel place it alone" is the + * first question about an order, not the last -- so it leads the row, ahead of the product and + * ahead of the price. The word and its tone both arrive already decided + * (`payload._order_row_payload`), because `bypass` meaning the same thing as `autonomous` is a + * judgement, and judgements are made in Python. + * + * **`expected` and `actual` are adjacent, with the divergence between them.** That difference is + * realised slippage. Its tone is side-aware and comes from the server for the reason + * `_order_row_payload` states: paying more than expected is bad on a buy and good on a sell, so + * a client colouring the minus sign would be wrong on half the rows. + * + * **`fee` is the figure as recorded and never a rate.** No percentage is computed here, and none + * crosses the wire. A paper row's fee carries a `modelled` badge instead, because + * `PaperTrader` derives it from the configured rate and reading it back as a measurement of that + * rate is circular. + * + * **`raw_response` is not in the payload at all**, so this function could not render it if it + * tried. The venue's order id is, when there is one, and a sentence saying why there is not, + * when there is not. + * + * @param {any} data + * @param {any} sort + * @param {(column: string) => void} onSort + * @param {(scope: string) => void} onScope + * @returns {DocumentFragment} + */ export function ordersView(data, sort, onSort, onScope, onStatus) { const fragment = document.createDocumentFragment(); fragment.append(el("h1", undefined, "Orders")); diff --git a/tests/commands/test_positions.py b/tests/commands/test_positions.py index 2363f46..8614eee 100644 --- a/tests/commands/test_positions.py +++ b/tests/commands/test_positions.py @@ -300,3 +300,204 @@ def test_the_unrealized_total_reconciles_with_the_agents_own_equity_read( assert parts is not None assert page_total == parts.unrealized == Decimal("100") + + +# -- the chip must ask the question the AGENT asks (#701 review finding) ------------------------ +# +# `_entry_gate_granularity` gates a rule on the timeframe the rule DECLARES, and falls back to +# the coarsest configured series only when it declares none. A chip that always asked about the +# coarsest would be reporting the fallback as though it were the answer -- right for a daily +# deployment, wrong for any rule seeded on a finer timeframe, and confidently worded either way. + + +def test_the_gate_verdict_follows_the_rules_own_declared_timeframe( + repo: Repository, tmp_path: Path +) -> None: + """A rule seeded on ONE_HOUR is gated on ONE_HOUR. Here the DAILY series is present and the + HOURLY one is absent, so the two possible readings disagree: the coarsest series says + something other than "missing", and the rule's own series says "missing".""" + repo.insert_rule("turtle_breakout", {"product_id": PRODUCT, "granularity": "ONE_HOUR"}) + _open_tranche(repo, rule_name="turtle_breakout") + repo.upsert_candles(PRODUCT, Granularity.ONE_DAY, [_candle(NOW_TS - DAY, "100")]) + + row = gather_positions(repo, _config(tmp_path), now_ts=NOW_TS).rows[0] + + assert row.ready is False + assert row.ready_reason == "missing", "the ONE_HOUR series this rule trades on has no bars" + + +def test_a_rule_that_declares_nothing_falls_back_to_the_coarsest_series( + repo: Repository, tmp_path: Path +) -> None: + """`_entry_gate_granularity`'s own fallback, and the reason for it: a rule silent about its + timeframe is most likely keying off the coarsest series (DCA reads the daily bar directly), + so gating on the finest would miss a weeks-stale daily bar entirely.""" + repo.insert_rule("dca", {"product_id": PRODUCT}) + _open_tranche(repo, rule_name="dca") + + row = gather_positions(repo, _config(tmp_path), now_ts=NOW_TS).rows[0] + + assert row.ready is False + assert row.ready_reason == "missing" + + +def test_an_unmatched_rule_name_degrades_to_the_coarsest_rather_than_crashing( + repo: Repository, tmp_path: Path +) -> None: + """`positions.rule_name` is the rule's `name`, which is a separate constructor argument from + the `kind` the rules table is keyed on -- they coincide by default and are not guaranteed to. + An unmatched name must leave the chip on the safe fallback, not take the page down.""" + _open_tranche(repo, rule_name="a_rule_no_longer_in_the_book") + + row = gather_positions(repo, _config(tmp_path), now_ts=NOW_TS).rows[0] + + assert row.ready_reason == "missing" + + +def test_two_tranches_of_one_product_on_different_timeframes_get_their_own_verdicts( + repo: Repository, tmp_path: Path +) -> None: + """The per-product cache the first version used cannot express this: one product, two rules, + two gate granularities, two answers. Caching by product alone would give the second tranche + the first one's verdict.""" + repo.insert_rule("turtle_breakout", {"product_id": PRODUCT, "granularity": "ONE_DAY"}) + repo.insert_rule("pullback_continuation", {"product_id": PRODUCT, "granularity": "ONE_HOUR"}) + _open_tranche(repo, rule_name="turtle_breakout") + _open_tranche(repo, rule_name="pullback_continuation") + # Only the DAILY series exists, and it is at its expected bar. + repo.upsert_candles(PRODUCT, Granularity.ONE_DAY, [_candle(NOW_TS - DAY, "100")]) + + rows = gather_positions(repo, _config(tmp_path), now_ts=NOW_TS).rows + by_rule = {row.rule_name: row for row in rows} + + assert by_rule["pullback_continuation"].ready_reason == "missing" + assert by_rule["turtle_breakout"].ready_reason != "missing" + + +def test_the_rules_table_is_read_once_however_many_tranches( + repo: Repository, tmp_path: Path +) -> None: + """Resolving a gate granularity per row would be one rules read per tranche. The map is built + once, like `gather_orders`' rule-name map.""" + repo.insert_rule("turtle_breakout", {"product_id": PRODUCT, "granularity": "ONE_DAY"}) + for _ in range(6): + _open_tranche(repo, rule_name="turtle_breakout") + + reads = {"n": 0} + original = repo.get_rules + + def counting(*args: Any, **kwargs: Any) -> Any: + reads["n"] += 1 + return original(*args, **kwargs) + + repo.get_rules = counting # type: ignore[method-assign] + gather_positions(repo, _config(tmp_path), now_ts=NOW_TS) + + assert reads["n"] == 1 + + +# -- the rule lookup is by NAME, and names can collide (#701 review) ---------------------------- + + +def test_two_rules_of_one_kind_on_different_timeframes_keep_their_own_verdicts( + repo: Repository, tmp_path: Path +) -> None: + """The map is keyed on what `positions.rule_name` actually holds -- the rule's `name`, a + constructor argument -- and NOT on `rules.kind`. Two `turtle_breakout` rows on different + timeframes is a configuration this codebase supports; keyed by kind they collapse to + whichever row was read last, and one tranche silently inherits the other's granularity.""" + repo.insert_rule( + "turtle_breakout", + {"product_id": PRODUCT, "granularity": "ONE_DAY", "name": "turtle_daily"}, + ) + repo.insert_rule( + "turtle_breakout", + {"product_id": PRODUCT, "granularity": "ONE_HOUR", "name": "turtle_hourly"}, + ) + _open_tranche(repo, rule_name="turtle_daily") + _open_tranche(repo, rule_name="turtle_hourly") + repo.upsert_candles(PRODUCT, Granularity.ONE_DAY, [_candle(NOW_TS - DAY, "100")]) + + rows = gather_positions(repo, _config(tmp_path), now_ts=NOW_TS).rows + by_rule = {row.rule_name: row for row in rows} + + assert by_rule["turtle_hourly"].ready_reason == "missing", "gated on the absent ONE_HOUR" + assert by_rule["turtle_daily"].ready_reason != "missing", "gated on the present ONE_DAY" + + +def test_two_rules_sharing_a_name_degrade_to_the_fallback( + repo: Repository, tmp_path: Path +) -> None: + """`name` is not unique in the schema. When two rows answer to one name with DIFFERENT gate + granularities, which one opened a tranche is genuinely unknowable from `rule_name` alone -- + so the chip takes the fallback rather than picking one and stating it with confidence.""" + repo.insert_rule( + "turtle_breakout", {"product_id": PRODUCT, "granularity": "ONE_DAY", "name": "same"} + ) + repo.insert_rule( + "turtle_breakout", {"product_id": PRODUCT, "granularity": "ONE_HOUR", "name": "same"} + ) + _open_tranche(repo, rule_name="same") + repo.upsert_candles(PRODUCT, Granularity.ONE_DAY, [_candle(NOW_TS - DAY, "100")]) + + row = gather_positions(repo, _config(tmp_path), now_ts=NOW_TS).rows[0] + + # The fallback is the COARSEST series, which is present here -- so "not missing" is the + # observable consequence of having declined to guess. + assert row.ready_reason != "missing" + + +def test_each_rule_is_built_once_however_many_tranches_it_opened( + repo: Repository, tmp_path: Path, monkeypatch: Any +) -> None: + """`_build_rule` runs a rule's real constructor, with its validation. Doing that per TRANCHE + is the same waste as a per-row query, and on a DCA book (one rule, many tranches) it is the + common case rather than the edge one.""" + repo.insert_rule( + "turtle_breakout", + {"product_id": PRODUCT, "granularity": "ONE_DAY", "name": "turtle_breakout"}, + ) + for _ in range(6): + _open_tranche(repo, rule_name="turtle_breakout") + + builds = {"n": 0} + original = agent_mod._build_rule + + def counting(row: Any) -> Any: + builds["n"] += 1 + return original(row) + + monkeypatch.setattr(agent_mod, "_build_rule", counting) + gather_positions(repo, _config(tmp_path), now_ts=NOW_TS) + + assert builds["n"] == 1, f"built the rule {builds['n']} times for 6 tranches" + + +def test_the_mark_cache_does_not_leak_between_products( + repo: Repository, tmp_path: Path +) -> None: + """A cache keyed carelessly would hand product B product A's price, and every figure derived + from it would be confidently wrong with nothing in the row to show it.""" + _open_tranche(repo, product_id="BTC-USD") + _open_tranche(repo, product_id="ETH-USD") + repo.upsert_candles("BTC-USD", FINEST, [_candle(NOW_TS - 900, "150")]) + repo.upsert_candles("ETH-USD", FINEST, [_candle(NOW_TS - 900, "20")]) + + rows = gather_positions(repo, _config(tmp_path), now_ts=NOW_TS).rows + by_product = {row.product_id: row for row in rows} + + assert by_product["BTC-USD"].mark == Decimal("150") + assert by_product["ETH-USD"].mark == Decimal("20") + + +def test_the_products_list_is_in_first_seen_order(repo: Repository, tmp_path: Path) -> None: + """What a grouped view renders sections from. Sorted or set-ordered, the page would reorder + itself between reads for no reason a reader could see.""" + _open_tranche(repo, product_id="SOL-USD") + _open_tranche(repo, product_id="BTC-USD") + _open_tranche(repo, product_id="SOL-USD") + + assert gather_positions(repo, _config(tmp_path), now_ts=NOW_TS).products == ( + "SOL-USD", + "BTC-USD", + ) diff --git a/tests/web/test_api.py b/tests/web/test_api.py index 578d759..c36e804 100644 --- a/tests/web/test_api.py +++ b/tests/web/test_api.py @@ -58,6 +58,7 @@ "/api/setup", "/api/activity", "/api/orders", + "/api/positions", "/api/insights", "/api/journal", "/api/rules", @@ -66,6 +67,22 @@ ) +def test_this_module_pins_every_route_the_server_serves() -> None: + """`API_ROUTES` above is hand-written, and everything in this file is parametrised over it -- + the envelope, the no-JSON-number walk, the cache headers, the nosniff header and the + POST refusal. A route left out of it is served with none of those checked. + + That happened: `/api/positions` (#701) shipped outside the tuple, so its reader was executed + by no test at all while its payload builder was well covered -- and this module's own header + says those are not the same statement. Cross-checked here so the next one cannot. + """ + from keel.web import api as web_api + + assert set(API_ROUTES) == set(web_api.API_ROUTES), ( + "API_ROUTES here must name every route the server actually serves" + ) + + # -- a real server ------------------------------------------------------------------------------ diff --git a/tests/web/test_client_assets.py b/tests/web/test_client_assets.py index dc1dfd9..6f4f81b 100644 --- a/tests/web/test_client_assets.py +++ b/tests/web/test_client_assets.py @@ -1528,13 +1528,49 @@ def test_every_table_emits_one_cell_per_declared_header() -> None: ) +def _scannable_functions(code: str) -> list[tuple[str, str]]: + """Every function body the call scan walks, as `(parameter list, body)`. + + Both shapes this codebase writes: `function name(params) { ... }` and the module-level + `const name = (params) => { ... }`. The parameter list is taken by BALANCED PARENTHESES + rather than `([^)]*)`, because a default value (`function f(a = el())`) closes the character + class early -- and the failure mode of that was not a false positive but a silent DROP: the + function stopped being scanned at all, quietly, which is the worst way for a guard to fail. + """ + out: list[tuple[str, str]] = [] + for match in re.finditer(r"\bfunction\s+[A-Za-z_$][\w$]*\s*\(", code): + params, after = _balanced_block(code, match.end() - 1) + brace = code.find("{", after) + if brace == -1: + continue + out.append((params, _balanced_block(code, brace)[0])) + for match in re.finditer(r"\b(?:const|let|var)\s+[A-Za-z_$][\w$]*\s*=\s*\(", code): + params, after = _balanced_block(code, match.end() - 1) + arrow = code[after : after + 4] + if "=>" not in arrow: + continue + brace = code.find("{", after) + if brace == -1: + continue + out.append((params, _balanced_block(code, brace)[0])) + return out + + def _undefined_calls(name: str) -> list[str]: """Bare-identifier calls in `name` that are not in scope where they are made. - Scoped PER FUNCTION, and that is the whole difficulty. A first version collected every - function's parameters into one module-wide set, which made `sorting` -- a parameter of - `table` and of `headerCell` -- count as defined inside `positionsView`, and that is exactly - the bug this scan exists to catch. Parameters are in scope in their own function only. + Scoped PER FUNCTION, which is the part that took two attempts. A first version collected + every function's parameters into one module-wide set, so `sorting` -- a parameter of `table` + and of `headerCell` -- counted as defined inside `positionsView`, and that is exactly the bug + this scan exists to catch. Parameters are in scope in their own function only. + + KNOWN BLIND SPOTS, stated rather than implied. Object-literal and class methods are not + walked (this codebase writes neither in these two modules). Arrow PARAMETERS are admitted + function-wide rather than per-arrow, so a callback argument sharing a name with a missing + function would mask it -- real JavaScript scoping needs a parser, and a regex that pretended + to do it would be worse than one whose limits are written down. What the scan does cover is + the mistake that actually shipped: calling a name that exists in the file as a typedef, a + parameter of some other function, or nothing at all. """ code = _code_only(_source(name)) @@ -1547,15 +1583,12 @@ def _undefined_calls(name: str) -> list[str]: module_level.add(imported) undefined: set[str] = set() - for match in re.finditer(r"\bfunction\s+([A-Za-z_$][\w$]*)\s*\(([^)]*)\)\s*\{", code): - body = _balanced_block(code, code.index("{", match.end() - 1))[0] + for params, body in _scannable_functions(code): in_scope = set(module_level) - for param in match.group(2).split(","): - param = param.strip() + for param in params[1:-1].split(","): + param = param.split("=")[0].strip() if param: in_scope.add(param) - # Everything declared inside the body, at any depth, plus every arrow parameter -- a - # callback's own argument is in scope for the callback. in_scope |= set(re.findall(r"\b(?:const|let|var)\s+([A-Za-z_$][\w$]*)", body)) in_scope |= set(re.findall(r"\(([A-Za-z_$][\w$]*)\)\s*=>", body)) in_scope |= set(re.findall(r"\b([A-Za-z_$][\w$]*)\s*=>", body)) @@ -1579,9 +1612,35 @@ def test_every_call_resolves_to_something_the_module_has(name: str) -> None: it to `main.js` would mean listing `fetch`, `setTimeout`, `URL` and friends, and a list long enough to cover those is long enough to hide a typo. """ + scanned = _scannable_functions(_code_only(_source(name))) + assert len(scanned) >= 5, ( + f"the call scan found only {len(scanned)} functions in {name}; it has stopped working" + ) assert _undefined_calls(name) == [] +@pytest.mark.parametrize( + "snippet", + [ + "function planted() { return nope(); }", + # The default value that used to close `([^)]*)` early and silently drop the function. + "function planted(a = el()) { return nope(); }", + # A module-level arrow, which the first version never walked at all. + "const planted = (a) => { return nope(); };", + ], +) +def test_the_call_scanner_sees_every_shape_this_codebase_writes(snippet: str) -> None: + """The premise, per shape. A scan that silently skipped one of these would report a clean + file while the missing call sat inside it -- which is how the guard fails without saying so. + """ + code = _code_only(snippet) + found: set[str] = set() + for params, body in _scannable_functions(code): + called = {m.group(1) for m in re.finditer(r"(? None: """The premise. A scanner whose regex missed every call would pass any file.""" code = "function a() { return b(1); }" diff --git a/tests/web/test_positions_view.py b/tests/web/test_positions_view.py index 14a33f7..b3ab75e 100644 --- a/tests/web/test_positions_view.py +++ b/tests/web/test_positions_view.py @@ -177,6 +177,18 @@ def _code(name: str) -> str: return _comments_only(_source(name)) +def _function_body(name: str, function: str) -> str: + """The source of ONE exported function, ending where the next one begins. + + A fixed `[:4000]` slice ran 1362 characters past `positionsView` into `ordersView`, which + made the no-close-action assertion below partly a statement about a different view: it would + have started failing, or passing, on edits to code it does not describe. + """ + after = _code(name).split("export function " + function)[1] + end = after.find("export function ") + return after if end == -1 else after[:end] + + def test_the_positions_view_is_wired_into_the_client_router() -> None: """A view in `main.js` alone routes on a click and 404s on a reload; a route in Python alone is a page with nothing to render. Both tables and the renderer, or none of them.""" @@ -197,7 +209,7 @@ def test_the_positions_view_has_no_close_action_anywhere() -> None: kind of affordance that arrives later as an obvious convenience, so the absence is asserted on the source and will fail the build the day someone adds one. """ - view = _code("render.js").split("export function positionsView")[1][:4000] + view = _function_body("render.js", "positionsView") for banned in ("close", "sell", "exit", "cancel", "liquidate"): assert banned not in view.lower(), f"the positions view must offer no {banned} action" @@ -211,12 +223,22 @@ def test_the_positions_view_groups_by_the_reports_own_product_list() -> None: def test_the_positions_view_shows_the_freshness_chip_per_row() -> None: """The entry-gate verdict, on the page. It is the thing that explains an idle deployment, so a view that carried every figure and not this one would leave the most common question - unanswered.""" - view = _code("render.js").split("export function positionsView")[1][:4000] - assert "freshness" in view + unanswered. + + PER ROW, not per product: the gate granularity comes from the RULE that opened the tranche, + so one product holding two tranches from rules on different timeframes has two verdicts. A + chip read off the first row and captioned for the whole product would state one tranche's + verdict over another's.""" + view = _function_body("render.js", "positionsView") + + assert '"freshness"' in view, "the entry-gate verdict must be a column of the table" + assert "held[0]" not in view, ( + "a per-product chip read off one row cannot represent tranches whose rules gate on " + "different granularities" + ) def test_the_positions_view_names_the_stop_distance_both_ways() -> None: - view = _code("render.js").split("export function positionsView")[1][:4000] + view = _function_body("render.js", "positionsView") assert "stop_distance" in view assert "stop_distance_pct" in view