diff --git a/keel/commands/orders.py b/keel/commands/orders.py index 93159b5..6847d36 100644 --- a/keel/commands/orders.py +++ b/keel/commands/orders.py @@ -50,6 +50,7 @@ import datetime import json import time +from collections.abc import Mapping from dataclasses import dataclass from decimal import Decimal, InvalidOperation from typing import Any @@ -294,6 +295,18 @@ class OrderRow: #: Why it is absent, when it is: `""` when there is an id to show. venue_order_id_detail: str + #: The NAME of the rule that placed this order -- `rules.kind`, the same string + #: `build_rule_track_record` calls `rule_name`, so the Orders view and the track-record + #: table name one rule one way. `""` when there is no name to show, for either of the two + #: reasons `rule_name_detail` tells apart. + rule_name: str + + #: Why the name is absent, when it is. Two states that both render as an empty cell and are + #: NOT the same fact: no rule was recorded against this order at all, or a rule WAS and that + #: rule is no longer in the book. The second is the one worth investigating, so it names the + #: id. `""` when there is a name. + rule_name_detail: str + rule_id: int | None created_at: int | None updated_at: int | None @@ -320,9 +333,17 @@ class OrdersReport: #: "nothing here" into a fact rather than a guess. total_count: int - #: Rows inside the scope, before `limit`. + #: Rows inside the scope, before `status` and before `limit`. Its meaning is unchanged by + #: the status filter on purpose: it is the denominator that says whether the SCOPE is what + #: emptied the table, and a count that moved with the open tab could not answer that. scoped_count: int + #: Rows inside the scope AND matching `status`, before `limit` -- what `rows` is a page of. + #: Equal to `scoped_count` when no status filter is applied. It exists because "1 of 3" on + #: a Canceled tab must count canceled orders; counting the scope there would be a true + #: number answering a question nobody asked. + filtered_count: int + #: Rows in `rows`. Held on the report rather than measured by a front-end: Rule 6e of #: `test_console_thinness.py` bans `len()` in the serialiser precisely so a count on the #: wire is one the report already carries. @@ -333,15 +354,45 @@ class OrdersReport: #: it from an empty live section. modes: tuple[str, ...] + #: The resolved status filter, lowercased, or `""` for "every status". Echoed back for the + #: reason `scope` is: a client rendering the active tab reads what was APPLIED rather than + #: trusting what it asked for. + status: str + + #: Every distinct `status` present in the whole book, sorted -- what a tab bar is built + #: from. From the WHOLE book and not the scope, deliberately: scoped, the Canceled tab + #: would vanish on a quiet day and return tomorrow, and a control that comes and goes is + #: worse than one that is sometimes empty. Same reasoning as `modes`. + statuses: tuple[str, ...] + #: `""` when there are rows; `"book"` when the book has never held an order; `"scope"` - #: when it holds orders and this window excluded all of them. Decided here, where the - #: counts are. + #: when it holds orders and this window excluded all of them; `"status"` when the window + #: holds orders and none of them wear the filtered status. Decided here, where the counts + #: are -- four different facts that all render as zero rows, and a reader who cannot tell + #: them apart learns the wrong one. empty_reason: str #: NEWEST FIRST. Reversed here, never in a renderer. rows: tuple[OrderRow, ...] +#: What an order with no `rule_id` says. A real state, not a defect: a manual order, or one +#: placed before the column was written. +NO_RULE_DETAIL = "no rule recorded against this order" + +#: What an order whose rule has left the book says. Deliberately different wording AND the id, +#: because "nothing placed this" and "rule 7 placed this and rule 7 is gone" are different facts +#: and only the second is worth chasing. +MISSING_RULE_DETAIL = "rule {rule_id} placed this order and is no longer in the book" + +#: What a row says when nobody LOOKED. A caller that passed no rule map has not searched the +#: book, so reporting the rule as gone would state the result of a search that never ran -- and +#: would send a reader hunting for a deleted rule that is sitting in the table. The same sentence +#: covers a rule that WAS found and has no `kind` to show: it is present, so "no longer in the +#: book" would be false, and the only honest report is that no name is available. +UNRESOLVED_RULE_DETAIL = "rule {rule_id} placed this order; its name is unresolved here" + + def _venue_order_id(raw_response: Any) -> str: """The venue's own order id, and nothing else, out of `raw_response`. @@ -422,12 +473,31 @@ def _adverse(side: str, difference: Decimal | None) -> bool | None: return None -def _row_from_dict(row: dict[str, Any]) -> OrderRow: +def _row_from_dict(row: dict[str, Any], rule_names: Mapping[int, str] | None = None) -> OrderRow: """One repository dict, projected. Every judgement this report makes about a row is made - here, once, so neither renderer has to make it twice.""" + here, once, so neither renderer has to make it twice. + + `rule_names` is `{rules.id: rules.kind}`, read ONCE by `gather_orders` and passed down. A + lookup per row would be one query per order against a book capped at `MAX_ORDERS_LIMIT`, on + a route with no proxy in front of it -- and would be invisible on any fixture small enough + to read. Omitted (the default), every row reports its rule as unresolved rather than + claiming there is none: a caller that did not supply the map has not established that. + """ mode = str(row.get("mode") or "") side = str(row.get("side") or "") confirmation = str(row.get("confirmation") or "") + rule_id = None if row.get("rule_id") is None else int(row["rule_id"]) + rule_name = "" if rule_id is None or rule_names is None else rule_names.get(rule_id, "") + if rule_name: + rule_detail = "" + elif rule_id is None: + rule_detail = NO_RULE_DETAIL + elif rule_names is None or rule_id in rule_names: + # Either nobody looked, or the rule was found and carries no name. Both are "present, or + # at least not shown to be absent" -- and neither supports the claim below. + rule_detail = UNRESOLVED_RULE_DETAIL.format(rule_id=rule_id) + else: + rule_detail = MISSING_RULE_DETAIL.format(rule_id=rule_id) expected = row.get("expected_fill") actual = row.get("actual_fill") difference, _ = _divergence(expected, actual) @@ -477,6 +547,8 @@ def _row_from_dict(row: dict[str, Any]) -> OrderRow: submit_book_detail=book_detail, venue_order_id=venue_id, venue_order_id_detail=venue_detail, + rule_name=rule_name, + rule_name_detail=rule_detail, rule_id=None if row.get("rule_id") is None else int(row["rule_id"]), created_at=None if row.get("created_at") is None else int(row["created_at"]), updated_at=None if row.get("updated_at") is None else int(row["updated_at"]), @@ -488,15 +560,31 @@ def gather_orders( *, now_ts: int, scope: str = DEFAULT_ORDERS_SCOPE, + status: str = "", limit: int | None = DEFAULT_ORDERS_LIMIT, ) -> OrdersReport: """Every order in the book, newest first, scoped and capped. - `repo.get_orders()` is called with NO arguments: no `mode`, no `product_id`, no `status`. - That is the load-bearing choice in this function. Each deployment book holds exactly one - mode, so any mode filter renders empty on the books that hold the other one, and a report - that showed nothing while the book held fifteen rows would be a surface asserting a fact it - had not established. The mode is carried per ROW instead. + `repo.get_orders()` is still called with NO arguments: no `mode`, no `product_id`, no + `status`. That is the load-bearing choice in this function, and the reason is about MODE -- + each deployment book holds exactly one, so any mode filter renders empty on the books that + hold the other one, and a report that showed nothing while the book held fifteen rows would + be a surface asserting a fact it had not established. The mode is carried per ROW instead. + + `status` (#700) narrows the report WITHOUT touching that call: the filter runs in the same + pass as the scope, over rows the repository handed back whole, so `total_count` and + `statuses` still describe the entire book and the empty states below can still tell "keel + has never traded" from "nothing wears this status". Pushing it into the query would trade + those for nothing -- the cap is applied after both narrowings either way. + + Filtering server-side rather than in the browser is the same argument `?sort=` makes: a + client filtering a CAPPED page would be filtering the fifty rows it happened to receive and + presenting the result as every rejected order. + + An unrecognised status is not refused. It is applied, matches nothing, and comes back with + `empty_reason == "status"` naming what was asked for -- which shows a typo plainly, and + cannot wrongly refuse a status some venue or some older row spells differently. `statuses` + is the list a client should build tabs from. A row with no `created_at` cannot be placed in time, so it is never excluded by a scope: excluding it would hide a row on the strength of a timestamp that does not exist. It counts @@ -504,29 +592,59 @@ def gather_orders( """ resolved_scope = normalise_scope(scope) resolved_limit = normalise_limit(limit) + resolved_status = (status or "").strip().lower() start_ts = scope_start_ts(resolved_scope, now_ts) raw = repo.get_orders() + # ONE read of a small table, before the row loop -- see `_row_from_dict`'s note on why this + # is not a per-row lookup. `kind` IS the rule's name here (`build_rule_track_record` reads + # the same column under the same meaning); there is no separate name column to prefer. + rule_names: dict[int, str] = { + int(rule["id"]): str(rule.get("kind") or "") + for rule in repo.get_rules() + if rule.get("id") is not None + } total = 0 modes: set[str] = set() + statuses: set[str] = set() scoped: list[dict[str, Any]] = [] + filtered: list[dict[str, Any]] = [] for row in raw: total += 1 modes.add(str(row.get("mode") or "")) + # From every row in the BOOK, before either narrowing -- see `statuses`' own note. + # LOWERCASED, like `resolved_status` below and for the same reason: the client marks the + # active tab with `name === current`, so a raw `Filled` tab would send `?status=Filled`, + # get `filled` echoed back, and highlight neither itself nor "all" -- while a book + # holding both spellings would show two tabs returning identical rows. `api.py` declines + # to refuse unknown statuses precisely because such spellings exist in real books, so + # they have to be folded here rather than assumed away. + statuses.add(str(row.get("status") or "").strip().lower()) created = row.get("created_at") if start_ts is not None and created is not None and int(created) < start_ts: continue scoped.append(row) + # Lowercased on BOTH sides. The stored word is lowercase today, but matching the + # caller's casing against a raw column would make a hand-typed `?status=FILLED` fail + # while the tab that sends `filled` works -- one control, two behaviours. + if resolved_status and str(row.get("status") or "").strip().lower() != resolved_status: + continue + filtered.append(row) # Newest first, HERE. `get_orders` orders by `id` ascending; reversing in each renderer # would be the same decision made twice and the second copy is the one that drifts. - scoped.reverse() - shown = tuple(_row_from_dict(row) for row in scoped[:resolved_limit]) + filtered.reverse() + shown = tuple(_row_from_dict(row, rule_names) for row in filtered[:resolved_limit]) + # Ordered widest cause first: a book with nothing in it is not a scope problem, and a scope + # that excluded everything is not the tab's doing. Reporting the narrowest true cause would + # send a reader to change the filter when the book is simply empty. if total == 0: empty_reason = "book" elif not scoped: empty_reason = "scope" + elif not filtered: + empty_reason = "status" else: empty_reason = "" @@ -535,10 +653,13 @@ def gather_orders( scope=resolved_scope, scope_start_ts=start_ts, limit=resolved_limit, + status=resolved_status, total_count=total, scoped_count=len(scoped), + filtered_count=len(filtered), shown_count=len(shown), modes=tuple(sorted(modes)), + statuses=tuple(sorted(s for s in statuses if s)), empty_reason=empty_reason, rows=shown, ) @@ -615,6 +736,18 @@ def render_orders(report: OrdersReport) -> list[str]: f"outside scope={report.scope}. Widen it (--scope all) to see them." ) return lines + if report.empty_reason == "status": + # The fourth empty state needs the fourth sentence. Without it this renderer prints its + # header, "showing 0 of N", and then nothing -- the silent blank the two branches above + # exist to prevent, and the one a reader is most likely to read as "keel never traded". + # Names the statuses the book DOES hold, because the next thing a reader wants is which + # filter would have worked. + held = ", ".join(report.statuses) if report.statuses else "none" + lines.append( + f"no orders with status={report.status} in this window -- the book holds " + f"{report.total_count}, with statuses: {held}." + ) + return lines for row in report.rows: placement = "AUTONOMOUS" if row.confirmation_is_autonomous else (row.confirmation or "--") @@ -654,6 +787,12 @@ def render_orders(report: OrdersReport) -> list[str]: lines.append(f" venue order id: {row.venue_order_id}") else: lines.append(f" {row.venue_order_id_detail}") + # The service resolves this for BOTH front-ends; rendering it only in the browser would + # leave `keel orders` unable to answer a question the report already holds. + if row.rule_name: + lines.append(f" rule: {row.rule_name}") + else: + lines.append(f" {row.rule_name_detail}") rule = "--" if row.rule_id is None else str(row.rule_id) lines.append( f" rule={rule} placed={_utc(row.created_at)} updated={_utc(row.updated_at)}" diff --git a/keel/web/api.py b/keel/web/api.py index 6a07fd6..668974b 100644 --- a/keel/web/api.py +++ b/keel/web/api.py @@ -285,11 +285,20 @@ def read_orders(cfg: ServeConfig, query: Query, _state: Any, _now_ts: int) -> di ) scope = normalise_scope(_first(query, "scope")) + # `?status=` is applied, never refused (#700). `?sort=`'s columns are a closed set this + # module declares, so an unknown one is a client bug worth a 400; a status is written by + # whatever placed the order -- a venue's own word, or an older spelling like `cancelled` + # already in `_ORDER_STATUS_STATES` -- so a refusal here would reject a status some book + # genuinely holds. An unmatched filter comes back empty with `empty_reason: "status"`, + # which shows a typo as plainly as a 400 would and cannot be wrong about a real one. + status = _first(query, "status") or "" raw_limit = _first(query, "limit") limit = DEFAULT_ORDERS_LIMIT if not raw_limit else _whole_number(raw_limit, MAX_ORDERS_LIMIT) repo = open_repo(cfg.db_path) try: - report = gather_orders(repo, now_ts=int(time.time()), scope=scope, limit=limit) + report = gather_orders( + repo, now_ts=int(time.time()), scope=scope, status=status, limit=limit + ) finally: close_repo(repo) return payload.orders_payload(report) diff --git a/keel/web/payload.py b/keel/web/payload.py index eacc623..e69d9fc 100644 --- a/keel/web/payload.py +++ b/keel/web/payload.py @@ -1342,6 +1342,13 @@ def activity_payload(feed: ActivityFeed) -> dict[str, Any]: "no orders in this window. The book holds orders, all of them outside this scope -- " "widen it to see them." ), + # A FIXED sentence that does not name the status asked for (#700). `?status=` is + # caller-controlled text; it crosses in `status`, which the client places as a value, and + # keeping it out of the prose means an unrecognised word cannot arrive as a sentence. + "status": ( + "no orders with this status in this window. The book holds orders with other statuses " + "-- switch tabs to see them." + ), } #: The two `mode` words, styled. `live` is `warn` not because live trading is wrong but because @@ -1374,6 +1381,15 @@ def _order_row_payload(row: OrderRow) -> dict[str, Any]: read out of it. A reader wanting the blob opens SQLite, which is the right amount of friction for unbounded venue JSON. + **#700 asked for a per-row disclosure exposing that blob, and it was REFUSED.** Not on + layout grounds: the column is whatever the venue chose to send, so it can carry session + metadata, internal identifiers, and error schemas nobody here has read, and a page that + renders it wholesale publishes all of that to anyone who can open the console. The + reduction to `venue_order_id` happens in the service precisely so that emitting the rest is + an impossible mistake here rather than a one-line one. What would reverse this: a curated, + named set of venue fields worth showing -- which is a different feature, with its own + review of what each field contains, and not "render the blob". + **`fee` is a figure, never a rate.** No percentage appears in this payload. Deriving one would be arithmetic Rule 3 forbids here, and would put a number on the page that the report does not hold. @@ -1460,6 +1476,12 @@ def _order_row_payload(row: OrderRow) -> dict[str, Any]: # a paper row shows a sentence rather than a blank that reads as missing data. "venue_order_id": row.venue_order_id, "venue_order_id_note": row.venue_order_id_detail, + # The NAME first, the id after it (#700). A foreign key is not something a reader can + # act on; `rules.kind` is the same string the track-record table names the rule by, so + # the two views cannot call one rule two things. `rule_note` tells the two absences + # apart -- no rule recorded, versus a rule that has left the book. + "rule_name": label(row.rule_name or "unattributed", state=NEUTRAL), + "rule_note": row.rule_name_detail, "rule_id": count(row.rule_id), "created_at": moment(row.created_at), "updated_at": moment(row.updated_at), @@ -1487,8 +1509,15 @@ def orders_payload(report: OrdersReport) -> dict[str, Any]: "scope": report.scope, "scope_start_at": moment(report.scope_start_ts), "limit": count(report.limit), + # The status FILTER as applied, and the tab bar to build (#700). Both bare strings, like + # `scope` and `modes` beside them: a status is an enum word with no precision hazard and + # no judgement -- the judgement is `_ORDER_STATUS_STATES`, and it is made per ROW, where + # a reader meets it. + "status": report.status, + "statuses": [str(status) for status in report.statuses], "total_count": count(report.total_count), "scoped_count": count(report.scoped_count), + "filtered_count": count(report.filtered_count), "shown_count": count(report.shown_count), "modes": [str(mode) for mode in report.modes], "empty_reason": report.empty_reason, diff --git a/keel/web/static/js/main.js b/keel/web/static/js/main.js index 9d89f5f..108b2d8 100644 --- a/keel/web/static/js/main.js +++ b/keel/web/static/js/main.js @@ -392,10 +392,22 @@ function mount(route, readings) { // The same server-side scope switch Activity uses, and deliberately the same control: two // views over one deployment whose "how far back" behaved differently would be a thing an // operator has to learn twice. - return ordersView(data, primary.sort, onSort, (scope) => { - paramsFor(route.endpoints[0]).scope = scope; - void paint(route, true, true); - }); + return ordersView( + data, + primary.sort, + onSort, + (scope) => { + paramsFor(route.endpoints[0]).scope = scope; + void paint(route, true, true); + }, + // #700: the status tab re-asks the SERVER. Filtering the received page here would filter + // the rows that happened to arrive under `?limit=` and present the result as every + // canceled order -- the same argument that puts `?sort=` on the server. + (status) => { + paramsFor(route.endpoints[0]).status = status; + void paint(route, true, true); + }, + ); } if (route.name === "insights") { const journal = readings[1]; diff --git a/keel/web/static/js/render.js b/keel/web/static/js/render.js index a4dd719..a9e089d 100644 --- a/keel/web/static/js/render.js +++ b/keel/web/static/js/render.js @@ -1168,7 +1168,7 @@ function jobPanel(job) { * @param {(scope: string) => void} onScope * @returns {DocumentFragment} */ -export function ordersView(data, sort, onSort, onScope) { +export function ordersView(data, sort, onSort, onScope, onStatus) { const fragment = document.createDocumentFragment(); fragment.append(el("h1", undefined, "Orders")); @@ -1177,10 +1177,21 @@ export function ordersView(data, sort, onSort, onScope) { fragment.append(sub); fragment.append(scopeSwitch(plain(data.scope), onScope, "Orders scope")); + // Guarded, so a caller that has not wired the tabs renders the view unchanged rather + // than a bar whose buttons do nothing. + if (onStatus) { + fragment.append(statusSwitch(plain(data.status), data.statuses || [], onStatus)); + } fragment.append( gridCard([ kv("shown", data.shown_count), + // Only when a status is actually on. Unfiltered, `filtered_count` equals `scoped_count` + // by construction, so the row would repeat the number below it under a label naming a + // filter that is not in force. With a tab open it is the denominator "shown" is a page + // of -- and the report is the only place allowed to work that out (Rule 2 keeps the + // subtraction out of the browser). + ...(plain(data.status) ? [kv("in this status", data.filtered_count)] : []), kv("in scope", data.scoped_count), kv("in this book", data.total_count), // Which modes this book actually holds. A deployment book holds one, and saying which @@ -1202,6 +1213,10 @@ export function ordersView(data, sort, onSort, onScope) { { label: "side", numeric: false, key: "side" }, { label: "product", numeric: false, key: "product_id" }, { label: "status", numeric: false, key: "status" }, + // The rule's NAME, beside the status rather than buried in the disclosure: on a + // book with several rules live, which one placed an order is a scanning + // question, and a foreign key in a detail panel does not answer it. + { label: "rule", numeric: false, key: "rule_name" }, { label: "qty", numeric: true, key: "qty" }, { label: "filled", numeric: true, key: "filled_quantity" }, { label: "expected", numeric: true, key: "expected_fill" }, @@ -1218,6 +1233,9 @@ export function ordersView(data, sort, onSort, onScope) { row.side, plain(row.product_id) || "—", row.status, + // Positional: `table()` pairs `columns[index]` with this array by index, so this cell + // sits where the "rule" header does and every later one shifts if it is missing. + row.rule_name, row.qty, row.filled_quantity, row.expected_fill, @@ -1301,7 +1319,11 @@ function orderDetail(row) { "venue order id", plain(row.venue_order_id) || plain(row.venue_order_id_note) || "—", ), - kv("rule", row.rule_id), + // The name is in the table; this is the id it resolved from, plus the sentence + // that tells the two absences apart -- no rule recorded at all, versus a rule + // that has left the book. A blank cell would collapse them. + kv("rule", plain(row.rule_note) || field(row.rule_name)), + kv("rule id", row.rule_id), kv("last updated", row.updated_at), ]), ); @@ -1384,6 +1406,52 @@ export function activityView(data, sort, onSort, onScope) { return fragment; } +/** + * The Orders status tabs (#700). + * + * **Built from `statuses`, never from a constant.** A tab bar listing every status keel CAN + * write would invite a reader to click into four empty tabs and conclude something about the + * engine from what is really a list of possibilities. `statuses` is what this book actually + * recorded, which is why the service carries it and why it comes from the whole book rather than + * the open scope -- a tab that vanishes on a quiet day is worse than one that is empty. + * + * **`current` is what the report APPLIED**, not what the client last asked for. If the two ever + * disagree the report is right, and a bar drawn from the request would show a filter that is not + * in force. + * + * The leading "all" tab is not decoration: without it, a reader who has clicked into a status has + * no way back short of knowing that the empty string means every status. + * + * @param {string} current `data.status` -- `""` when unfiltered. + * @param {string[]} statuses `data.statuses`. + * @param {(status: string) => void} onStatus + * @returns {HTMLElement} + */ +function statusSwitch(current, statuses, onStatus) { + const wrap = el("nav", "scopes"); + wrap.setAttribute("aria-label", "Order status"); + wrap.append(el("span", "k", "status")); + const all = el("button", "scopekey", "all"); + all.setAttribute("type", "button"); + // Pressing a tab replaces the whole view, which destroys the button that was pressed. + // `data-focus` is how `main.js` puts focus back on its replacement -- without it a keyboard + // user is returned to the top of the document on every tab press, which is exactly what the + // note above `sortTrigger` says this attribute exists to prevent. + all.setAttribute("data-focus", "status:"); + if (!current) all.setAttribute("aria-current", "true"); + all.addEventListener("click", () => onStatus("")); + wrap.append(all); + for (const name of statuses) { + const button = el("button", "scopekey", name); + button.setAttribute("type", "button"); + button.setAttribute("data-focus", "status:".concat(name)); + if (name === current) button.setAttribute("aria-current", "true"); + button.addEventListener("click", () => onStatus(name)); + wrap.append(button); + } + return wrap; +} + /** * The three scopes, and the one that is on. * diff --git a/tests/commands/test_orders.py b/tests/commands/test_orders.py index 2682c6f..1cf211a 100644 --- a/tests/commands/test_orders.py +++ b/tests/commands/test_orders.py @@ -99,6 +99,11 @@ def get_orders(self, *args: Any, **kwargs: Any) -> list[dict[str, Any]]: self.calls.append((args, kwargs)) return list(self.rows) + def get_rules(self, *args: Any, **kwargs: Any) -> list[dict[str, Any]]: + # `gather_orders` resolves rule NAMES off this (#700). Recorded nowhere: this stub + # exists to pin how `get_orders` is called, and that pin is unchanged. + return [] + def test_gather_passes_no_filter_of_any_kind_to_get_orders() -> None: """THE pin the operator's scope note asked for. @@ -664,12 +669,317 @@ def _report_with(**overrides: Any) -> OrdersReport: "scope": "all", "scope_start_ts": None, "limit": DEFAULT_ORDERS_LIMIT, + "status": "", "total_count": len(rows), "scoped_count": len(rows), + "filtered_count": len(rows), "shown_count": len(rows), "modes": ("live",), + "statuses": ("filled",), "empty_reason": "", "rows": rows, } base.update(overrides) return OrdersReport(**base) + + +# -- the status filter, server-side (#700) ---------------------------------------------------- +# +# The Orders view's tabs. Filtered HERE and not in the browser, for the reason `?sort=` is: a +# client that filtered a capped page would be filtering the 50 rows it happened to receive and +# calling the result "every rejected order", which is a different and false claim. +# +# It does NOT reach `get_orders()`. The unfiltered-read pin above is unchanged and still the +# load-bearing one -- the status narrowing happens in the same pass as the scope, over rows the +# repository handed back whole. + + +def test_a_status_filter_keeps_only_that_status(tmp_path: Path) -> None: + repo = _repo(tmp_path) + repo.insert_order(_order(status="filled")) + repo.insert_order(_order(status="canceled")) + repo.insert_order(_order(status="pending")) + + report = gather_orders(repo, now_ts=NOW_TS, status="canceled") + + assert [row.status for row in report.rows] == ["canceled"] + + +def test_no_status_filter_keeps_every_row(tmp_path: Path) -> None: + """The default is the whole book. A tabbed view opens on "All", and a report that quietly + defaulted to one status would answer a question nobody asked.""" + repo = _repo(tmp_path) + repo.insert_order(_order(status="filled")) + repo.insert_order(_order(status="canceled")) + + assert len(gather_orders(repo, now_ts=NOW_TS).rows) == 2 + assert len(gather_orders(repo, now_ts=NOW_TS, status="").rows) == 2 + + +def test_the_status_filter_is_matched_without_case(tmp_path: Path) -> None: + """`?status=FILLED` from a hand-typed URL is the same question as `?status=filled`. The + stored word is lowercase; matching on the caller's casing would make the tab work from the + UI and fail from the address bar.""" + repo = _repo(tmp_path) + repo.insert_order(_order(status="filled")) + + assert len(gather_orders(repo, now_ts=NOW_TS, status="FILLED").rows) == 1 + + +def test_the_status_filter_composes_with_the_scope(tmp_path: Path) -> None: + """Both narrowings apply, and the scope still comes first: a filled order from last month is + not in today's book, whichever tab is open.""" + repo = _repo(tmp_path) + repo.insert_order(_order(status="filled", created_at=TODAY_START + 60)) + repo.insert_order(_order(status="filled", created_at=TODAY_START - 86_400)) + repo.insert_order(_order(status="canceled", created_at=TODAY_START + 60)) + + report = gather_orders(repo, now_ts=NOW_TS, scope="today", status="filled") + + assert report.shown_count == 1 + + +def test_the_report_names_every_status_in_the_whole_book(tmp_path: Path) -> None: + """What the tabs are built from -- the statuses this deployment ACTUALLY recorded, not a + hardcoded list of the ones keel can write. `modes` is carried for the same reason: a reader + must never have to conclude which tabs exist from which ones came back empty. + + From the WHOLE book, deliberately: scoped to today, a tab bar would lose the Canceled tab on + a quiet day and reappear it tomorrow, and a control that comes and goes is worse than one + that is sometimes empty.""" + repo = _repo(tmp_path) + repo.insert_order(_order(status="filled", created_at=TODAY_START - 86_400)) + repo.insert_order(_order(status="canceled", created_at=TODAY_START + 60)) + repo.insert_order(_order(status="filled", created_at=TODAY_START + 60)) + + report = gather_orders(repo, now_ts=NOW_TS, scope="today") + + assert report.statuses == ("canceled", "filled") + + +def test_the_resolved_status_is_echoed_back(tmp_path: Path) -> None: + """Same contract as `scope`: what the report actually applied, so a client rendering the + active tab reads it back rather than trusting what it asked for.""" + repo = _repo(tmp_path) + repo.insert_order(_order(status="filled")) + + assert gather_orders(repo, now_ts=NOW_TS, status="FILLED").status == "filled" + assert gather_orders(repo, now_ts=NOW_TS).status == "" + + +def test_an_empty_status_tab_is_not_an_empty_book(tmp_path: Path) -> None: + """The three ways this table comes back empty are three different facts, and a reader who + cannot tell them apart learns the wrong one. "keel has never traded", "nothing in this + window", and "nothing with this status" all render as zero rows.""" + repo = _repo(tmp_path) + repo.insert_order(_order(status="filled", created_at=TODAY_START + 60)) + + assert gather_orders(repo, now_ts=NOW_TS, status="rejected").empty_reason == "status" + assert gather_orders(repo, now_ts=NOW_TS, scope="today", status="filled").empty_reason == "" + + +def test_an_out_of_scope_row_still_reports_the_scope_as_the_reason(tmp_path: Path) -> None: + """Scope is checked before status, so a book whose only row is outside the window says so + rather than blaming the tab -- the scope is the narrowing the reader chose first.""" + repo = _repo(tmp_path) + repo.insert_order(_order(status="filled", created_at=TODAY_START - 86_400)) + + report = gather_orders(repo, now_ts=NOW_TS, scope="today", status="filled") + assert report.empty_reason == "scope" + + +def test_the_filtered_count_is_what_the_shown_rows_are_a_page_of(tmp_path: Path) -> None: + """`scoped_count` keeps its meaning -- rows inside the SCOPE -- so a filtered view needs its + own denominator. Without it a tab showing 1 of 3 would be counting rows from other tabs, and + "1 of 3 canceled orders" would be false while every number in it was true.""" + repo = _repo(tmp_path) + repo.insert_order(_order(status="filled")) + repo.insert_order(_order(status="filled")) + repo.insert_order(_order(status="canceled")) + + report = gather_orders(repo, now_ts=NOW_TS, status="filled") + + assert report.total_count == 3 + assert report.scoped_count == 3 + assert report.filtered_count == 2 + assert report.shown_count == 2 + + +def test_the_filtered_count_reflects_the_cap_being_a_cap(tmp_path: Path) -> None: + repo = _repo(tmp_path) + for _ in range(4): + repo.insert_order(_order(status="filled")) + + report = gather_orders(repo, now_ts=NOW_TS, status="filled", limit=2) + + assert report.filtered_count == 4 + assert report.shown_count == 2 + +# -- the rule that placed it (#700) ----------------------------------------------------------- +# +# `orders.rule_id` is a foreign key, and a foreign key on a page is a number a reader cannot act +# on. The NAME is `rules.kind` -- the same string `build_rule_track_record` calls `rule_name`, so +# the Orders view and the track-record table name the same rule the same way. + + +class _RuleCountingRepo: + """Records how often each read is made, so the resolution cannot be N+1 unnoticed.""" + + def __init__(self, orders: list[dict[str, Any]], rules: list[dict[str, Any]]) -> None: + self._orders = orders + self._rules = rules + self.order_reads = 0 + self.rule_reads = 0 + + def get_orders(self, *args: Any, **kwargs: Any) -> list[dict[str, Any]]: + self.order_reads += 1 + return list(self._orders) + + def get_rules(self, *args: Any, **kwargs: Any) -> list[dict[str, Any]]: + self.rule_reads += 1 + return list(self._rules) + + +def test_the_rule_that_placed_an_order_is_named(tmp_path: Path) -> None: + repo = _repo(tmp_path) + rule_id = repo.insert_rule("turtle_breakout", {"product_id": "BTC-USD"}) + repo.insert_order(_order(rule_id=rule_id)) + + report = gather_orders(repo, now_ts=NOW_TS) + + assert report.rows[0].rule_name == "turtle_breakout" + + +def test_an_order_with_no_rule_says_so_rather_than_showing_a_blank(tmp_path: Path) -> None: + """A `NULL` rule_id is a real state -- a manual order, or one placed before the column was + written. It is not the same as a rule that has gone missing, and the sentence is what keeps + the two apart on a page where both render as no name.""" + repo = _repo(tmp_path) + repo.insert_order(_order(rule_id=None)) + + row = gather_orders(repo, now_ts=NOW_TS).rows[0] + + assert row.rule_name == "" + assert "no rule" in row.rule_name_detail.lower() + + +def test_a_rule_that_is_no_longer_in_the_book_is_named_as_missing() -> None: + """The distinction that matters for an audit trail: "nothing placed this" and "rule 7 placed + this and rule 7 is gone" are different facts, and only the second is worth chasing. Rendering + both as an empty cell would hide it. + + Driven through `_row_from_dict` rather than a repository, because `orders.rule_id` carries a + FOREIGN KEY to `rules(id)` and `db.connect` enables `PRAGMA foreign_keys = ON` -- so keel + cannot write this row, and a test that tried would be refused by SQLite rather than by the + code under test. The branch is defensive and stays: the map can also miss for a row written + by an older keel, by an external sqlite3 session with the pragma off, or by a caller passing + a partial map.""" + row = orders_service._row_from_dict(_order(id=1, rule_id=7), {}) + + assert row.rule_name == "" + assert "7" in row.rule_name_detail + assert row.rule_name_detail != orders_service._row_from_dict( + _order(id=2, rule_id=None), {} + ).rule_name_detail + + +def test_naming_the_rules_costs_one_read_however_many_orders(tmp_path: Path) -> None: + """The pin against an N+1. A per-row lookup would be invisible on a fixture of two and would + be one query per row on a real book -- `MAX_ORDERS_LIMIT` of them, on a route with no proxy + in front of it (`api.py`'s own note).""" + repo = _RuleCountingRepo( + [_order(id=i, rule_id=1) for i in range(1, 26)], + [{"id": 1, "kind": "turtle_breakout", "params": {}, "status": "live"}], + ) + + report = gather_orders(repo, now_ts=NOW_TS) # type: ignore[arg-type] + + assert repo.rule_reads == 1 + assert {row.rule_name for row in report.rows} == {"turtle_breakout"} + + +# -- review findings on the rule name and the status tabs (#700) ------------------------------ + + +def test_an_unresolved_map_does_not_claim_the_rule_is_gone() -> None: + """The docstring and the code disagreed, and the docstring was right. With no map supplied, + the row has not been LOOKED UP -- saying "rule 7 is no longer in the book" states the result + of a search nobody performed, and sends a reader hunting for a deleted rule that is sitting + in the table.""" + unlooked = orders_service._row_from_dict(_order(id=1, rule_id=7)) + searched = orders_service._row_from_dict(_order(id=1, rule_id=7), {}) + + assert unlooked.rule_name == "" + assert "no longer" not in unlooked.rule_name_detail + assert "unresolved" in unlooked.rule_name_detail.lower() + # And the genuine miss still says what it found. + assert "no longer" in searched.rule_name_detail + + +def test_a_rule_with_no_name_is_not_reported_as_a_missing_rule() -> None: + """A rule row whose `kind` is empty was found -- it just has nothing to show. Reporting it as + absent would blame the book for a blank column.""" + row = orders_service._row_from_dict(_order(id=1, rule_id=7), {7: ""}) + + assert row.rule_name == "" + assert "no longer" not in row.rule_name_detail + + +def test_the_tab_list_is_lowercased_like_the_filter_it_is_compared_against( + tmp_path: Path, +) -> None: + """The client highlights the active tab with `name === current`. `status` is lowercased on + the way in, so a tab carrying the raw column would match neither itself nor "all" -- and a + book holding both `Filled` and `filled` would show two tabs returning identical rows. + + Not hypothetical by this module's own reasoning: `api.py` declines to refuse unknown + statuses precisely because venue-written and older spellings exist in real books.""" + repo = _repo(tmp_path) + repo.insert_order(_order(status="Filled")) + repo.insert_order(_order(status="filled")) + + report = gather_orders(repo, now_ts=NOW_TS, status="Filled") + + assert report.statuses == ("filled",) + assert report.status == "filled" + assert report.filtered_count == 2, "both spellings are the same status" + + +def test_the_cli_renderer_says_which_rule_placed_an_order(tmp_path: Path) -> None: + """The service resolves the name for BOTH front-ends. Rendering it only in the browser would + leave `keel orders` unable to answer a question the report already holds -- the asymmetry + this module's header exists to prevent.""" + repo = _repo(tmp_path) + rule_id = repo.insert_rule("turtle_breakout", {"product_id": "BTC-USD"}) + repo.insert_order(_order(rule_id=rule_id)) + + text = "\n".join(render_orders(gather_orders(repo, now_ts=NOW_TS))) + + assert "turtle_breakout" in text + + +def test_the_cli_renderer_explains_a_status_tab_that_matched_nothing() -> None: + """The fourth empty reason needs the fourth sentence here too. Without it the CLI prints its + header, "showing 0 of 3", and then nothing at all -- the silent blank the other two branches + exist to prevent. Unreachable from `keel orders` today (it has no `--status`), which is why + it is pinned on the renderer rather than on the command.""" + report = OrdersReport( + now_ts=NOW_TS, + scope="all", + scope_start_ts=None, + limit=50, + status="rejected", + total_count=3, + scoped_count=3, + filtered_count=0, + shown_count=0, + modes=("live",), + statuses=("filled",), + empty_reason="status", + rows=(), + ) + + text = "\n".join(render_orders(report)) + + assert "rejected" in text + assert "filled" in text, "a reader must be told which statuses this book does hold" diff --git a/tests/web/test_client_assets.py b/tests/web/test_client_assets.py index 639ba10..d5565a2 100644 --- a/tests/web/test_client_assets.py +++ b/tests/web/test_client_assets.py @@ -1414,3 +1414,84 @@ def test_an_unknown_drawdown_ceiling_draws_no_floor_line() -> None: assert "point.dd_floor_y !== null" in chart_code, ( "points with no recorded drawdown floor must be filtered before the floor is drawn" ) + + +# -- every table declares as many cells as it declares headers --------------------------------- + + +def _balanced_block(text: str, start: int) -> tuple[str, int]: + """The bracketed block beginning at `start` (which must be a `[`), and the index after it.""" + depth = 0 + for index in range(start, len(text)): + char = text[index] + if char in "([{": + depth += 1 + elif char in ")]}": + depth -= 1 + if depth == 0: + return text[start : index + 1], index + 1 + raise AssertionError("unbalanced block") + + +def _top_level_items(block: str) -> int: + """How many elements a `[...]` literal declares, counting commas at depth 1 only.""" + inner = block[1:-1] + depth = 0 + items = 0 + seen_content = False + for char in inner: + if char in "([{": + depth += 1 + elif char in ")]}": + depth -= 1 + elif char == "," and depth == 0: + items += 1 + seen_content = False + continue + if not char.isspace(): + seen_content = True + return items + (1 if seen_content else 0) + + +def _table_calls(code: str) -> list[tuple[int, int]]: + """`(headers, cells)` for every `table(` call in `render.js` that inlines both.""" + pairs: list[tuple[int, int]] = [] + cursor = 0 + while True: + found = code.find("table(", cursor) + if found == -1: + return pairs + cursor = found + 6 + # The columns array is the first `[` after the id argument. + columns_at = code.find("[", cursor) + if columns_at == -1: + continue + columns_block, after = _balanced_block(code, columns_at) + if "label:" not in columns_block: + continue # not a table() call -- some other identifier ending in `table(` + headers = columns_block.count("label:") + # The cells come from a `.map(` whose arrow body is an array literal. + map_at = code.find(".map(", after) + if map_at == -1 or map_at > after + 400: + continue # rows passed as a variable; nothing inline to compare + cells_at = code.find("[", map_at) + if cells_at == -1: + continue + cells_block, _ = _balanced_block(code, cells_at) + pairs.append((headers, _top_level_items(cells_block))) + + +def test_every_table_emits_one_cell_per_declared_header() -> None: + """`table()` pairs `columns[index]` with `row.entries()` BY POSITION (render.js:400), so a + column added to the header list without a matching value silently shifts every later cell + one place left -- and the last column renders no `