From 88fb17bcab15eec3f0661631fe28d02e463621c6 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Mon, 24 Aug 2026 09:55:11 -0400 Subject: [PATCH] feat(web): the remaining six views, an SVG curve nobody has to see, and a stream that says when it stops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bulk of the port: `/setup`, `/activity`, `/insights`, `/rules`, `/venues` and `/gates` as client views over #534's API, plus the equity curve and live updates. ── PARITY IS ON THE INFORMATION, AND TWO MORE #548s TURNED UP ON THE WAY ────── Every view is judged against the rendered pages AND `tests/commands/test_tui.py`, and where those disagree the divergence is written into the view's docstring rather than reproduced. Porting found two more instances of #548's pattern -- a lookup that misses, defaulted, with nothing raised: * `render.py::_STEP_KIND_NOTE` has three entries against `StepKind`'s four. The missing one is `operator_input`, the kind of the real `credentials` step, so the rendered `/setup` prints an EMPTY note on the step where "what may a wizard do for you" matters most. `payload._STEP_KIND_NOTES` carries all four. * `render.py::pct` appends `%` to four values that are FRACTIONS. The config ships `max_total_dd_pct: 0.20` and rail 11 compares the raw drawdown against it, so a 20% ceiling renders as "0.20%" on every rendered page and a 5% drawdown as "0.05%" -- a hundredfold understatement of a risk limit. `payload.ratio`'s docstring names this exact trap; the client shows its value. ── THE CHART: THE COORDINATES ARE COMPUTED IN PYTHON ────────────────────────── `build_equity_curve` returns plot coordinates, not just figures, and that is the decision rather than an accident of layering. Where a vertical axis starts is what makes a $3 wobble look like a collapse; keel does not let a front-end decide whether a number is bad, and the axis it is drawn against is the same delegation wearing different clothes. So the baseline is always zero, in Python, where a test can read it. The consequence is checkable: `chart.js` needs no arithmetic, so the lexer scan that guards `render.js` now runs over it too -- proven by mutation, not by reading. The curve is `role="img"` named by its own `
`, one string in one element, with the journal table beside it for the figures. ── LIVE UPDATES: A DROPPED CONNECTION IS THE POINT ──────────────────────────── `/api/events` streams an envelope whose `data` holds one revision marker and no figures at all -- `api.js` stays the only place data enters this client, which is the property a reader audits by opening one file. The agent runs daily, so this is not a feed: it is how a dead server stops looking like an unchanged one. Driving it in a real browser found four bugs no test here could have: * the marker watched `keel.db` only, and `keel serve` runs WAL (#470), so a committed write moves nothing. The feature was inert and its own comment said so approvingly. It watches `-wal` now, and not `-shm`, which readers touch. * a tick repainted the banner green over a view saying the report could not be built. The view's own read now wins; a loss of contact still shows fast. * a sort press deferred itself, because the click focused a button inside the view and the rebuild was waiting for focus to leave. Pressing a control and watching nothing happen is indistinguishable from a broken control. * `/activity` and `/gates` scrolled the document sideways at 360px -- on a log path and a dotted call site, in two paragraphs, with every table already scrolling correctly inside itself. ── `/setup` KEEPS ITS GATE, AND ITS ACTION SET IS UNCHANGED ─────────────────── `keel.commands.setup.ACTIONS` is a 0-line diff, `keel/web/security.py` is a 0-line diff, and nothing under `/api/*` answers a POST. The client lists every action keel offers, with its inputs; the button stays on the page that holds the write token, because `/api/setup` ships no CSRF token on purpose and inventing a way to hand the client one would be widening the write surface. A client that hides a button is not a gate, so nothing is hidden -- only the button is elsewhere. #540 deletes that page, and where a browser client gets a write token from is a decision that belongs to the milestone, not to this commit. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NyeggYtojNXCTHeD3JHxb6 --- keel/commands/insights.py | 207 +++++ keel/web/api.py | 15 +- keel/web/events.py | 218 ++++++ keel/web/payload.py | 146 +++- keel/web/server.py | 61 +- keel/web/static/css/keel.css | 204 ++++- keel/web/static/js/api.js | 92 ++- keel/web/static/js/chart.js | 150 ++++ keel/web/static/js/live.js | 191 +++++ keel/web/static/js/main.js | 349 ++++++++- keel/web/static/js/render.js | 1265 ++++++++++++++++++++++++++++++- tests/commands/test_insights.py | 138 ++++ tests/web/test_client_assets.py | 210 ++++- tests/web/test_events.py | 312 ++++++++ tests/web/test_payload.py | 67 +- 15 files changed, 3469 insertions(+), 156 deletions(-) create mode 100644 keel/web/events.py create mode 100644 keel/web/static/js/chart.js create mode 100644 keel/web/static/js/live.js create mode 100644 tests/web/test_events.py diff --git a/keel/commands/insights.py b/keel/commands/insights.py index 94ec8b39..41863338 100644 --- a/keel/commands/insights.py +++ b/keel/commands/insights.py @@ -40,6 +40,7 @@ import json import time +from collections.abc import Sequence from dataclasses import asdict, dataclass from datetime import UTC, datetime from decimal import Decimal @@ -108,6 +109,212 @@ def shown_count(self) -> int: return len(self.entries) +# -- the equity curve (#537) -------------------------------------------------------------------- +# +# The browser's one chart. It lives HERE, in the report layer, rather than in `keel/web/` -- and +# the coordinates live here too, which is the part that looks misplaced and is not. +# +# **Scaling a chart is a judgement about the data, not a drawing detail.** Where the vertical +# axis starts decides what the picture says: a curve plotted between its own min and max makes a +# $3 wobble look like a collapse, and one plotted from zero does not. keel already refuses to let +# a front-end decide whether a number is good (`keel/web/payload.py`'s closed `state` +# vocabulary); letting one decide the axis it is drawn against would be the same delegation +# wearing different clothes. So `_BASELINE` below is included in the range unconditionally, in +# Python, where a `Decimal` is in scope and a test can read it. +# +# The second reason is arithmetic. Normalising a series means subtracting, dividing and comparing +# it, and the client is the one place in this system where those operations happen in IEEE-754 +# doubles over values that were exact `Decimal`s a moment earlier. `keel/web/static/js/chart.js` +# consequently contains no arithmetic at all and `tests/web/test_client_assets.py` scans it for +# the absence, exactly as it scans `render.js` -- which is only possible because the numbers it +# draws with arrive finished. +# +# What it would take to move: a chart with a zoom or a pan control. That is interaction over a +# range the server did not choose, and at that point the range becomes a client concern and this +# builder becomes a service the client re-asks with new bounds. Nothing here is that today. + +#: The coordinate box the curve is expressed in, and the reason it is not pixels. +#: +#: An SVG `viewBox` is unitless: the browser scales it to whatever width the card ends up, so +#: these numbers are a fixed internal grid and never a size on screen. 1000x300 rather than +#: 100x100 because the coordinates are QUANTIZED to 2dp below, and a taller grid means the +#: rounding is a smaller fraction of a pixel at any realistic rendered size. +PLOT_WIDTH = Decimal("1000") +PLOT_HEIGHT = Decimal("300") + +#: The value the vertical axis always contains. Zero, because the figure plotted is CUMULATIVE +#: NET P&L -- the line between "this rule has made money" and "this rule has lost money" -- and a +#: curve drawn without it on the canvas cannot show which side of it you are on. +_BASELINE = Decimal("0") + +#: Coordinate precision. Two decimal places on a 1000-wide grid is a hundred-thousandth of the +#: width: far finer than any display, and short enough that a 50-point `points` attribute stays +#: readable in view-source, which is the whole argument for this interface. +_COORD = Decimal("0.01") + + +@dataclass(frozen=True) +class EquityPoint: + """One closed trade's contribution to the curve, and where it is drawn. + + `pnl` and `cumulative` are the exact figures; `x` and `y` are the plot coordinates in the + `PLOT_WIDTH` x `PLOT_HEIGHT` box. `y` grows DOWNWARD, because SVG's does: emitting a + mathematical y here and flipping it in the browser would be one subtraction, performed on the + one side of the wire that is not allowed to perform any. + """ + + index: int + closed_at: int | None + product_id: str + rule_name: str | None + pnl: Decimal + cumulative: Decimal + x: Decimal + y: Decimal + + +@dataclass(frozen=True) +class EquityCurve: + """The cumulative net-P&L curve over a journal's closed trades, ready to draw. + + `low`/`high` are the axis bounds actually used, `_BASELINE` included -- so they are the range + a reader should be told about, not merely the extremes of the data. `baseline_y` is where + zero sits in the box, which is what lets the chart draw the one gridline that carries meaning. + + Empty is a real answer and is not an error: a deployment with no closed trades has no curve, + and `points == []` is how that is said. There is no synthetic flat line at zero, because a + flat line at zero is what a run of break-even trades looks like and the two must not be + confused. + """ + + points: list[EquityPoint] + low: Decimal + high: Decimal + baseline_y: Decimal + width: Decimal + height: Decimal + + @property + def point_count(self) -> int: + """How many points the curve holds. + + A derived reading rather than a stored field, for the reason `JournalReport.shown_count` + is: a second field holding `len(self.points)` is state that can drift from the list it + describes. It exists at all because `keel/web/payload.py` may not call `len()` -- Rule 6e + of `tests/commands/test_console_thinness.py` bans it there, so every count on the wire is + one a report already holds. + """ + return len(self.points) + + +def _plot_y(value: Decimal, *, low: Decimal, span: Decimal) -> Decimal: + """`value` mapped into `0..PLOT_HEIGHT`, with the top of the box being `low + span`.""" + fraction = (value - low) / span + return (PLOT_HEIGHT - PLOT_HEIGHT * fraction).quantize(_COORD) + + +def build_equity_curve(entries: Sequence[JournalEntry]) -> EquityCurve: + """The cumulative net-P&L curve over `entries`, oldest first. + + **Rows with no recorded net are SKIPPED, never counted as zero.** `JournalEntry.pnl_net` is + `None` when the ledger has no net for that trade, and folding a `None` into a running total as + `0` would draw a flat segment that asserts "this trade broke even" -- the same collapse of + "not recorded" into "recorded as zero" that `keel/web/payload.py`'s `ABSENT` note traces back + to #198's always-passing fee rail. A skipped row still appears in the journal table beside the + chart with its own dash, so the omission is visible rather than silent. + + **The horizontal axis is trade ORDER, not time.** Spacing points by `closed_at` would give a + long quiet week the same visual weight as fifty trades, which is a statement about the + calendar and not about the track record; `keel insights journal` reads the same way, oldest + first. It also means a row whose `closed_at` is unusable still has a position on the axis. + + Callers pass the entries they are displaying, so the curve and the table beneath it can never + disagree about which trades they describe. + """ + running = _BASELINE + plotted: list[tuple[JournalEntry, Decimal, Decimal]] = [] + for entry in entries: + net = entry.pnl_net + if net is None: + continue + running = running + net + plotted.append((entry, net, running)) + + if not plotted: + return EquityCurve( + points=[], + low=_BASELINE, + high=_BASELINE, + baseline_y=PLOT_HEIGHT, + width=PLOT_WIDTH, + height=PLOT_HEIGHT, + ) + + totals = [total for _entry, _net, total in plotted] + low = min([*totals, _BASELINE]) + high = max([*totals, _BASELINE]) + span = high - low + if span == _BASELINE: + # Every trade broke even, so the curve is a horizontal line and there is no range to + # normalise against. Drawn on the baseline rather than at the top or the bottom of the + # box: the figure IS zero, and zero is where the baseline is. + middle = (PLOT_HEIGHT / 2).quantize(_COORD) + return EquityCurve( + points=[ + EquityPoint( + index=index, + closed_at=entry.closed_at, + product_id=entry.product_id, + rule_name=entry.rule_name, + pnl=net, + cumulative=total, + x=_plot_x(index, len(plotted)), + y=middle, + ) + for index, (entry, net, total) in enumerate(plotted) + ], + low=low, + high=high, + baseline_y=middle, + width=PLOT_WIDTH, + height=PLOT_HEIGHT, + ) + + points = [ + EquityPoint( + index=index, + closed_at=entry.closed_at, + product_id=entry.product_id, + rule_name=entry.rule_name, + pnl=net, + cumulative=total, + x=_plot_x(index, len(plotted)), + y=_plot_y(total, low=low, span=span), + ) + for index, (entry, net, total) in enumerate(plotted) + ] + return EquityCurve( + points=points, + low=low, + high=high, + baseline_y=_plot_y(_BASELINE, low=low, span=span), + width=PLOT_WIDTH, + height=PLOT_HEIGHT, + ) + + +def _plot_x(index: int, total: int) -> Decimal: + """The horizontal position of point `index` of `total`, in `0..PLOT_WIDTH`. + + A single point is CENTRED rather than placed at `x=0`. One trade drawn hard against the left + edge reads as the beginning of a line that has been cut off, which is a claim about missing + data; centred, it reads as the one observation it is. + """ + if total == 1: + return (PLOT_WIDTH / 2).quantize(_COORD) + return (PLOT_WIDTH * Decimal(index) / Decimal(total - 1)).quantize(_COORD) + + @dataclass(frozen=True) class GateDistance: rule_name: str diff --git a/keel/web/api.py b/keel/web/api.py index 2ffd909a..7f0da22b 100644 --- a/keel/web/api.py +++ b/keel/web/api.py @@ -216,7 +216,18 @@ def read_insights(cfg: ServeConfig, _query: Query, _state: Any, now_ts: int) -> def read_journal(cfg: ServeConfig, query: Query, _state: Any, now_ts: int) -> dict[str, Any]: - from keel.commands.insights import build_journal_report + """The trade journal, and the equity curve drawn from the SAME entries. + + `build_equity_curve` is called on `report.entries` -- not on the ledger, and not on a second + read -- so the chart and the table under it describe one list of trades. A curve built from + its own query could disagree with the rows beside it after a `?limit=`, and a chart that + disagrees with the table below it is worse than no chart. + + The curve is NOT part of `route.collection`, so `?sort=` reorders `entries` and leaves the + curve alone. That is deliberate: the curve's horizontal axis is trade order, and a curve + redrawn in `pnl` order would be a cumulative total of a sequence that never happened. + """ + from keel.commands.insights import build_equity_curve, build_journal_report limit = _journal_limit(query) repo = open_repo(cfg.db_path) @@ -226,7 +237,7 @@ def read_journal(cfg: ServeConfig, query: Query, _state: Any, now_ts: int) -> di ) finally: close_repo(repo) - return payload.journal_payload(report) + return payload.journal_payload(report, curve=build_equity_curve(report.entries)) def read_rules(cfg: ServeConfig, _query: Query, _state: Any, _now_ts: int) -> dict[str, Any]: diff --git a/keel/web/events.py b/keel/web/events.py new file mode 100644 index 00000000..0f302c07 --- /dev/null +++ b/keel/web/events.py @@ -0,0 +1,218 @@ +"""Server-sent events for the browser client (#537) -- liveness and freshness, and no figures. + +`keel serve` polls itself today: `main.js` re-reads its endpoint every fifteen seconds. That is +enough to keep a page current and it is not enough to make a DEAD server visible -- a browser +whose `fetch` is failing looks exactly like one whose report has not changed, and a dashboard +that silently goes stale in front of an operator is the one failure this whole interface exists +to prevent. `EventSource` closes that gap because it is a connection rather than a request: when +the process it is attached to goes away, the browser knows within a heartbeat instead of within +however long nobody happened to look. + +── WHAT THIS STREAM CARRIES, AND THE THING IT DELIBERATELY DOES NOT ───────────────────────────── + +A tick is an `envelope` -- the same four keys `keel/web/payload.py` puts on every `GET /api/*` +answer -- whose `data` holds exactly one string: a **revision marker**. No equity, no positions, +no counts, no report of any kind. + +Two reasons, and the second is the one that decided it: + + * **The agent runs daily.** There is no ticking price feed here to stream; what actually + changes between one second and the next is whether keel is alive and whether anything has + been written since the page last looked. A stream carrying a full status report every five + seconds would be building a report four hundred times an hour so that a number could stay the + same. + + * **`api.js` stays the only place data enters this client.** Its module docstring sells one + property -- "the interface is provably incapable of sending positions, equity or trade + history anywhere but the local process", audited by a reader who opens one file. A second + transport that also carried figures would make that two files, and the audit would become a + search. `live.js` opens the `EventSource`; when a tick says something changed, the client + re-reads through `api.js`'s single `fetch` like it does for everything else. + +`engine` rides on the tick anyway, because it is not a figure: it is the answer to "is keel +running", it is one short sentence, and it is the thing the page's one `aria-live` region holds. +A stream that could tell you the server is up but not tell you the deployment is gone would be +answering the easier half of the question. + +── THE REVISION MARKER: WHAT IT WATCHES, AND WHAT IT CANNOT SEE ───────────────────────────────── + +`revision()` is `os.stat` on the deployment's two files plus the background job's state. It is +CHEAP by construction -- two stats and a dict read, microseconds -- which is what lets it run on +every heartbeat where `deployment_state`'s 3.6 ms probe would not. + +It is deliberately a marker and not a timestamp: the client compares it to the last one it saw +and re-reads on inequality, so its only contract is that it CHANGES when something has been +written. A monotonic clock reading would invite arithmetic at the other end. + +**It watches the `-wal` file too, and the first draft did not.** `keel serve` runs SQLite in WAL +mode (#470), so a committed write lands in `keel.db-wal` and leaves `keel.db`'s size and mtime +exactly where they were until a checkpoint. Watching only the database file was therefore a marker +that never moved: driven in a real browser, a trade written while the page was open produced ticks +for twelve seconds and no refresh at all. The earlier revision of this note called that a +documented blind spot covered by the fifteen-second poll -- which was wrong twice over, because +the poll deliberately does not rebuild the view while a subscription is running. The feature was +inert and the note said so approvingly. + +`-shm` is deliberately NOT watched: it is the shared-memory index, and it is touched by READERS. +Including it would move the marker every time this endpoint itself opened the database, which is a +refresh loop rather than a change notification. + +What remains missed is narrow and worth stating: a write that is checkpointed and truncates the +`-wal` back to a size it has held before, within the same `st_mtime_ns` tick, would look +unchanged. Nanosecond timestamps make that essentially unreachable, and the fifteen-second poll's +banner refresh is what would surface a page that had somehow gone quiet. + +── WHY THIS IS STILL A READ, AND STILL BEHIND THE SAME DOOR ───────────────────────────────────── + +`/api/events` is a GET, handled in `server.do_GET` AFTER the same `Host` check and the same +session cookie every other path goes through -- `keel/web/__init__.py`'s guarantee ("the JSON API +is reads only") is unchanged, and `test_the_event_stream_is_behind_the_same_admission` asserts it +over a socket rather than by inspection. It answers no POST, for the same reason nothing else +under `API_PREFIX` does. +""" + +from __future__ import annotations + +import json +import os +import time +from collections.abc import Iterator +from typing import TYPE_CHECKING, Any + +from keel.web import api, payload + +if TYPE_CHECKING: # pragma: no cover - typing only + from keel.web.server import ServeConfig + +#: The stream's content type, and the only one `EventSource` accepts. +CONTENT_TYPE = "text/event-stream; charset=utf-8" + +#: Seconds between ticks. +#: +#: Five, against `server._REFRESH_SEC`'s fifteen, and the two are answering different questions. +#: Fifteen is how stale a FIGURE may get; five is how long a dead server may look alive, and that +#: is the number an operator is standing in front of. It is also the heartbeat that keeps the +#: connection from being reaped by anything in between -- there is nothing in between on loopback +#: today, but a stream whose liveness depends on there being no proxy is a stream that breaks the +#: first time someone forwards the port over SSH, which the design spec explicitly endorses. +HEARTBEAT_SEC = 5 + +#: How long one connection is allowed to live before the server closes it and lets the browser +#: reconnect. +#: +#: Ten minutes. `ThreadingHTTPServer` gives every open connection a thread, and a streamed +#: response holds one for as long as it runs -- so an unbounded stream turns "how many tabs has +#: this operator left open since Tuesday" into "how many threads is this process holding". A +#: bounded lifetime makes that a ceiling instead of a trend. `EventSource` reconnects on its own +#: when a stream ends, so the cost of the ceiling is one reconnection per tab per ten minutes and +#: the client cannot tell the difference -- which is exactly why `RETRY_MS` below is short. +MAX_STREAM_SEC = 600 + +#: What the browser is told to wait before reconnecting, in milliseconds (the `retry:` field). +#: +#: Two seconds. The default is browser-defined and around three, which is fine for a dropped +#: connection and wrong for the ordinary case here: every stream ends on purpose after +#: `MAX_STREAM_SEC`, so this delay is paid on a healthy connection too. Short enough that the +#: banner's "reconnecting" state is a blink rather than a fault, long enough that a server which +#: is genuinely down is not reconnected against in a tight loop. +RETRY_MS = 2000 + +#: The tick event's name. Named rather than left as the default `message` so that a later event +#: type -- a job finishing, say -- is an addition rather than a change of meaning for listeners +#: that already exist. +TICK_EVENT = "tick" + + +def revision(cfg: ServeConfig, *, job_state: str = "") -> str: + """A marker that changes when something the client is showing may have changed. + + Built from `os.stat` rather than from a database read on purpose: the client polls this at + `HEARTBEAT_SEC`, and a read that opened SQLite would be a read that can block behind the + agent's own write. A `stat` of a file that is not there is not an error either -- a machine + with no deployment is the first-run case, and it gets the stable marker `"-"` for that file + rather than an exception. + """ + # `-wal` alongside the database, because in WAL mode that is where a commit actually lands -- + # see the module note. `-shm` is left out on purpose: readers touch it, so watching it would + # make this endpoint's own reads look like writes. + parts = [job_state] + for path in (cfg.db_path, f"{cfg.db_path}-wal", cfg.config_path): + try: + info = os.stat(path) + except OSError: + parts.append("-") + continue + parts.append(f"{info.st_mtime_ns}.{info.st_size}") + return "|".join(parts) + + +def tick_document(cfg: ServeConfig, now_ts: int) -> dict[str, Any]: + """One tick, as the same envelope every `GET /api/*` answers with. + + Reusing `payload.envelope` rather than inventing a stream-shaped message is what lets + `live.js` hand a tick to the very same banner code a `fetch` reading goes through. The client + already has one function for "what does this answer say about the engine"; a second message + shape would have needed a second one, and the two would have drifted the first time a state + word was added. + """ + try: + from keel.commands import jobs + + state = api.deployment_state(cfg) + running = bool(state.has_usable_database) + job = jobs.status() + job_state = f"{job.key}:{job.state}" if job is not None else "" + except Exception: # pragma: no cover - `inspect` is total; this is the belt to its braces + # The same reading `api.respond` takes when the probe itself fails: we could not tell + # whether keel is set up, so do not claim it is. + running, job_state = False, "" + return payload.envelope( + now_ts, + running=running, + data={"revision": revision(cfg, job_state=job_state)}, + sort=None, + ) + + +def frame(event: str, document: dict[str, Any]) -> str: + """One SSE frame: an event name, a single `data:` line, and the blank line that ends it. + + The JSON is written with no newline in it -- `json.dumps` produces none by default and the + payload's leaves are all strings -- so one `data:` line is always enough. A multi-line body + would need every line prefixed, and getting that wrong produces a stream that parses as + silence rather than as an error, which is the worst failure mode available here. + """ + # A plain `json.dumps`, with no `default=` and no `cls=`, for the reason `server._send_json` + # gives at its own call: `payload.py` normalises every leaf to a string before it gets here, + # so there is nothing for an encoder to convert -- and the encoder a hurried author reaches + # for is `default=float`, which is the whole money contract dying in one keyword. Nothing + # monetary rides this stream today, and the habit is worth keeping anyway. + body = json.dumps(document, ensure_ascii=False) + return f"event: {event}\ndata: {body}\n\n" + + +def stream( + cfg: ServeConfig, + *, + now: Any = time.time, + sleep: Any = time.sleep, + max_sec: int = MAX_STREAM_SEC, + heartbeat_sec: int = HEARTBEAT_SEC, +) -> Iterator[str]: + """The frames of one connection, ending when `max_sec` is up. + + The FIRST tick is emitted before any wait. A stream that opened and then said nothing for five + seconds would leave the page showing whatever it had, with no way to tell a slow connection + from a dead one -- and the first thing a reconnecting client needs is the current revision, so + it can find out whether anything moved while it was away. + + `now` and `sleep` are injected so a test can run the whole lifetime of a connection without + spending it. They are not a configuration surface: nothing but a test passes them. + """ + yield f"retry: {RETRY_MS}\n\n" + started = now() + while True: + yield frame(TICK_EVENT, tick_document(cfg, int(now()))) + if now() - started >= max_sec: + return + sleep(heartbeat_sec) diff --git a/keel/web/payload.py b/keel/web/payload.py index 5c63fa33..ce190c59 100644 --- a/keel/web/payload.py +++ b/keel/web/payload.py @@ -116,6 +116,8 @@ from keel.commands.activity import ActivityCycle, ActivityEvent, ActivityFeed from keel.commands.insights import ( AccountSummary, + EquityCurve, + EquityPoint, GateDistance, InsightsReport, JournalEntry, @@ -915,9 +917,86 @@ def _journal_entry_payload(entry: JournalEntry) -> dict[str, Any]: _OUTCOME_STATES: Mapping[str, str] = {"win": GOOD, "loss": BAD, "dca": NEUTRAL, "open": NEUTRAL} -def journal_payload(report: JournalReport) -> dict[str, Any]: +def _equity_point_payload(point: EquityPoint) -> dict[str, Any]: + """One point of the curve: where to draw it, and what it says. + + `x` and `y` are BARE STRINGS, not `Field`s, and that is the one place in this module where a + figure crosses with no `display` beside it. They are not values a human reads -- there is + nothing to format, no judgement to carry and no unit -- they are positions inside an SVG + `viewBox`, decided by `keel.commands.insights.build_equity_curve` where a `Decimal` is in + scope and where the choice of axis can be tested. `_plain` still renders them, so a + coordinate cannot reach the wire in exponent notation any more than a price can. + + Everything a reader is actually TOLD -- the instant, the trade's net, the running total -- + arrives as a `Field`, and those are what the chart's text equivalent is built from. + """ + return { + "x": _plain(point.x), + "y": _plain(point.y), + "at": moment(point.closed_at), + "product_id": point.product_id, + "rule_name": point.rule_name or "", + "pnl": money(point.pnl, signed=True), + "cumulative": money(point.cumulative, signed=True), + } + + +def equity_curve_payload(curve: EquityCurve) -> dict[str, Any]: + """`build_equity_curve`'s `EquityCurve`, as JSON. + + **`reading` is the chart's text equivalent, and it is written HERE rather than in the browser + for the same reason every other sentence on this wire is.** A chart is data made visible, so a + reader who cannot see it has to be told the same thing in words. Assembling that sentence in + JavaScript would mean the client deciding what a curve says -- a judgement -- and doing it + from figures `render.js` is not allowed to read. It is a `label` rather than a bare string + because it carries the curve's verdict in its `state`, taken from the CLOSING figure, so the + spoken summary and the last point can never disagree about whether this is a profitable track + record. + + Both text equivalents ship, and neither is a fallback for the other: the sentence is what a + screen reader hears from the chart's `aria-label`, and the per-point rows are what someone + reads when they want the numbers rather than the shape. + + `net` is the LAST point's cumulative rather than a sum computed here -- `build_equity_curve` + already ran the running total, and re-adding it in this module would be a second arithmetic + of the same figure in the one place that is not allowed to hold one. + """ + net = money(curve.points[-1].cumulative, signed=True) if curve.points else absent() + low, high = money(curve.low), money(curve.high) + trades = count(curve.point_count) + if curve.points: + reading = ( + f"Cumulative net profit and loss over {trades['display']} closed trades, " + f"ending at {net['display']}, ranging from {low['display']} to {high['display']}." + ) + else: + # Not "the curve is flat", and not an empty string. A deployment with no closed trades has + # no track record at all, and which of those two a reader is looking at is the entire + # difference between an empty chart and a broken one. + reading = "No closed trades yet, so there is no curve to draw." + return { + "width": _plain(curve.width), + "height": _plain(curve.height), + "baseline_y": _plain(curve.baseline_y), + "point_count": trades, + "low": low, + "high": high, + "net": net, + "reading": label("curve", display=reading, state=net["state"]), + "points": [_equity_point_payload(point) for point in curve.points], + } + + +def journal_payload(report: JournalReport, *, curve: EquityCurve) -> dict[str, Any]: """`build_journal_report`'s `JournalReport`, as JSON. + `curve` is passed in rather than built here, and the direction is the point: `keel/web/api.py` + calls `build_equity_curve` on the entries THIS report carries, so the chart and the table + beneath it are two views of one list and cannot come to describe different trades. It is a + REQUIRED keyword, never a defaulted one -- a default would let every existing caller keep + working while quietly serving a journal with no chart, which is the shape of a suite that is + green because it stopped asking the question. + `total_count` is the full filtered count BEFORE `--limit` truncated `entries`, and `shown_count` is how many survived; both cross, so a client showing "50 of 812" needs no subtraction to know it is looking at a page. @@ -939,6 +1018,7 @@ def journal_payload(report: JournalReport) -> dict[str, Any]: # crosses as strings -- see `stringify`. "filters": {str(key): stringify(value) for key, value in sorted(report.filters.items())}, "entries": [_journal_entry_payload(e) for e in report.entries], + "curve": equity_curve_payload(curve), } @@ -988,6 +1068,28 @@ def _cycle_payload(cycle: ActivityCycle) -> dict[str, Any]: } +#: What each non-`ok` `ActivityFeed.status` means, in the words `render.py::render_activity` +#: already uses. Copied verbatim rather than paraphrased: two front-ends offering an operator two +#: different accounts of the same state is worse than either account alone, and `missing` in +#: particular has to keep the second sentence -- "it also happens when keel is run from a +#: directory that is not the deployment folder" -- because that is the actual cause most of the +#: time and it is not guessable from the word. +#: +#: `ok` is absent, and `.get(..., "")` is what that means: a healthy log needs no paragraph. +_FEED_STATUS_NOTES: Mapping[str, str] = { + "missing": ( + "No log file yet. This is normal before the first cycle -- it also happens when keel is " + "run from a directory that is not the deployment folder." + ), + "empty": "The log exists but the window held no records.", + "unparseable": "Lines were read, but none of them was a JSON record.", + "oversized": ( + "The bounded tail read landed inside a single record, so nothing whole survived it." + ), + "unreadable": "The log could not be read.", +} + + def activity_payload(feed: ActivityFeed) -> dict[str, Any]: """`build_activity_feed`'s `ActivityFeed`, as JSON. @@ -1014,6 +1116,16 @@ def activity_payload(feed: ActivityFeed) -> dict[str, Any]: on_state=NEUTRAL, off_state=WARN, ), + # The prose for a non-`ok` status, chosen HERE rather than in the client. + # + # `render.py::render_activity` keeps the same table and picks from it with + # `explain.get(feed.status, "")`, which is a lookup keyed on the raw status WORD -- and a + # browser client cannot do that: `tests/web/test_client_assets.py:: + # test_render_never_judges_a_value_itself` forbids `render.js` from reading `Field.value` + # at all, because a client that branches on a value is a client re-deriving a judgement. + # Prose selected by a state word is exactly that branch, so the selection crosses the wire + # already made. It is `""` for `ok`, which is not a state anyone needs a paragraph about. + "status_note": _FEED_STATUS_NOTES.get(feed.status, ""), "lines_read": count(feed.lines_read), # Lines read but unusable -- a crash's half-written JSON, a record with no timestamp. # Surfaced rather than swallowed: silently discarding input is how a feed comes to @@ -1300,6 +1412,35 @@ def config_payload(build: Any, *, describe: str = "") -> dict[str, Any]: "off_venue": WARN, } +#: What each kind of step means for the operator -- what keel may do, what it may only collect, +#: and what it cannot touch at all. `render.py::_STEP_KIND_NOTE`'s wording, carrying #437's whole +#: argument, moved onto the wire so a browser client places it rather than holding a fourth copy. +#: +#: **`operator_input` is here and is NOT in `render.py`'s table.** That table has three entries +#: against `StepKind`'s four, and the missing one is the kind of the real `credentials` step -- so +#: `_STEP_KIND_NOTE.get(kind, "")` renders that step with an EMPTY note today, silently dropping +#: the one line that says what a wizard may and may not do on the step where it matters most. It +#: is the same shape as #548's five: a lookup that misses, defaulted, with nothing raised. Found +#: while porting `/setup` to the client (#537), and fixed here rather than in `render.py` because +#: #540 deletes that function; the browser gets the fourth note, the rendered page keeps its gap +#: until it goes. +_STEP_KIND_NOTES: Mapping[str, str] = { + "mechanical": "keel can do this for you.", + "operator_input": ( + "keel needs something only you have -- a credential, a path, a value from the venue. It " + "records what you supply and asks for nothing it does not need." + ), + "judgement": ( + "Yours to decide. keel can record it; it must never choose it for you, and an " + "attestation without a cited source is refused exactly like a missing one." + ), + "off_venue": ( + "Happens in the venue's own dashboard, and keel cannot verify it -- the venue's API " + "does not expose it. Never shown as done here, because a green check that verifies " + "nothing turns an open risk into a false assurance." + ), +} + #: `JobStatus.state`, judged. _JOB_STATES: Mapping[str, str] = {"running": WARN, "done": GOOD, "failed": BAD} @@ -1318,6 +1459,9 @@ def _step_payload(item: Any) -> dict[str, Any]: "kind": label( item.step.kind.value, state=_STEP_KIND_STATES.get(item.step.kind.value, NEUTRAL) ), + # Selected here for the same reason `activity_payload`'s `status_note` is: prose chosen by + # a state word is a branch on `Field.value`, and `render.js` may not perform one. + "kind_note": _STEP_KIND_NOTES.get(item.step.kind.value, ""), "stage": item.step.stage.value, "why": item.step.why, "how": item.step.how, diff --git a/keel/web/server.py b/keel/web/server.py index fa31f7dc..65acdc81 100644 --- a/keel/web/server.py +++ b/keel/web/server.py @@ -52,7 +52,7 @@ from typing import Any from urllib.parse import parse_qs, quote, urlsplit -from keel.web import api, render, staticfiles +from keel.web import api, events, render, staticfiles from keel.web.security import ( SESSION_COOKIE, HostPolicy, @@ -314,6 +314,18 @@ def page_glossary(_cfg: ServeConfig, _query: dict[str, list[str]]) -> tuple[str, #: none. API_PREFIX = "/api/" +#: The one path under `API_PREFIX` that is a STREAM rather than a document (#537). +#: +#: It is not in `api.API_ROUTES` because it does not have that table's shape: every entry there +#: maps to `(status, document)` through `api.respond`, and an `EventSource` connection is a +#: response that never finishes. Bolting a "this one streams" flag onto `ApiRoute` would have put +#: a branch into the one function whose uniformity is the reason #536's `fetch` wrapper needs no +#: per-endpoint branch of its own. +#: +#: It is still a GET, still behind `_admitted()`, and still answers no POST -- `do_POST` refuses +#: everything under `API_PREFIX` before it ever looks at a path. +EVENTS_PATH = "/api/events" + def run_setup_action(cfg: ServeConfig, key: str, form: dict[str, str]) -> Any: """Perform one declared setup action. Returns its `ActionResult`, or `None` for a key that is @@ -641,6 +653,46 @@ def _api_client_header_ok(self) -> bool: return False return True + def _serve_events(self) -> None: + """The `EventSource` stream (#537), for as long as the browser holds the connection. + + **`Connection: close` and no `Content-Length`.** This handler speaks HTTP/1.1, where a + response with neither a length nor chunked framing has to be delimited by the connection + ending -- and keep-alive is what would otherwise be assumed. The alternative is chunked + transfer-encoding, hand-framed on top of `wfile`; it buys the ability to reuse a socket + that this endpoint holds open for ten minutes anyway, which is nothing, in exchange for a + second framing layer to get wrong. + + **Every write is flushed.** A buffered `wfile` is a stream that arrives in bursts when the + buffer happens to fill, which for frames this small means "never" -- the page would show + nothing at all and the failure would look exactly like a server that is not sending. + + **A HEAD gets the headers and no body.** `do_HEAD` delegates to `do_GET` here as it does + everywhere else, and a HEAD that opened a ten-minute stream would be a way to hold a + thread without ever reading from it. + + **A disconnect is not an error.** The browser closing the tab, navigating away, or being + killed all surface as a broken pipe on the next write; that is the normal end of a + subscription and it is caught and dropped rather than logged as a failure or allowed to + reach `BaseHTTPRequestHandler`'s 500 path, which would try to write a body to a socket + that is already gone. + """ + self.close_connection = True + self.send_response(200) + self.send_header("Content-Type", events.CONTENT_TYPE) + self.send_header("Connection", "close") + for name, value in _API_HEADERS: + self.send_header(name, value) + self.end_headers() + if self.command == "HEAD": + return + try: + for chunk in events.stream(self.cfg): + self.wfile.write(chunk.encode("utf-8")) + self.wfile.flush() + except (BrokenPipeError, ConnectionResetError): + return + def _serve_static(self, url_path: str) -> None: """One file under `staticfiles.STATIC_PREFIX` (#535), or the same 404 an unmapped `ROUTES` path gets -- containment and the Content-Type table are `staticfiles`'s job @@ -802,6 +854,13 @@ def do_GET(self) -> None: # noqa: N802 - stdlib's naming, not ours if not self._admitted(): return + if parsed.path == EVENTS_PATH: + # Live updates (#537). Checked BEFORE the `API_PREFIX` branch below, because that + # branch ends in `api.respond`, which would answer this path with the 404 it gives + # every name absent from its table -- correctly, since this endpoint is not in it. + self._serve_events() + return + if parsed.path.startswith(API_PREFIX): # The JSON API (#534). Reads only: `api.respond` maps a path to one bounded read and # returns `(status, document)` -- it never raises, so a broken report becomes a stated diff --git a/keel/web/static/css/keel.css b/keel/web/static/css/keel.css index 20e72b11..db9962e3 100644 --- a/keel/web/static/css/keel.css +++ b/keel/web/static/css/keel.css @@ -179,7 +179,7 @@ h2 { font-size: 1.05rem; margin: 2rem 0 0.6rem; } text-transform: uppercase; letter-spacing: 0.05em; } -.kv .v { font-size: 1.05rem; font-variant-numeric: tabular-nums; overflow-wrap: anywhere; } +.kv .v { font-size: 1.05rem; font-variant-numeric: tabular-nums; } .tablewrap { overflow-x: auto; @@ -220,6 +220,31 @@ td.num { text-align: right; font-variant-numeric: tabular-nums; padding-right: 1 .empty { color: var(--muted); padding: 1.5rem 0; } .note { color: var(--muted); font-size: 0.85rem; margin: 0.4rem 0 0; } +/* ── the prose that holds machine strings ───────────────────────────────────────────────────── + * + * A file path, a dotted call site, a product id: unbreakable runs with no space in them, and by + * default they set the MINIMUM width of the block that holds them. Outside a scroller that + * minimum becomes the document's, which is how a page whose tables all scroll correctly still + * ends up scrolling sideways. + * + * Found by driving a real browser at 360px, not by reading: `/activity` overflowed by 194px on + * the log path in its subheading and `/gates` by 123px on + * `keel.commands._common._require_interactive_confirmation` in a ``. Every table on both + * pages was already scrolling inside itself; the offending elements were two paragraphs. + * + * `anywhere` rather than `break-word`, and the difference is exactly this case: only `anywhere` + * makes the break count towards `min-content`, so only `anywhere` stops the long word forcing its + * ancestors wide in the first place. `break-word` wraps the text and leaves the page broken. + */ +.sub, +.note, +.kv .v, +code, +.paramlist dd, +.stopped .detail { + overflow-wrap: anywhere; +} + /* ── the engine banner ──────────────────────────────────────────────────────────────────────── * * The one region that is `aria-live` (see `index.html`), so it is also the one region styled to @@ -251,11 +276,7 @@ td.num { text-align: right; font-variant-numeric: tabular-nums; padding-right: 1 .stopped { max-width: 44rem; } .stopped h1 { margin-bottom: 0.5rem; } .stopped p { margin: 0 0 0.7rem; } -.stopped .detail { - color: var(--muted); - font-size: 0.88rem; - overflow-wrap: anywhere; -} +.stopped .detail { color: var(--muted); font-size: 0.88rem; } button { font: inherit; @@ -272,6 +293,177 @@ button { button:hover { filter: brightness(1.08); } button[disabled] { opacity: 0.6; cursor: default; } +/* ── sortable headers and the scope switch (#537) ───────────────────────────────────────────── + * + * Both are `