feat(web): status tabs and the rule that placed each order (#700) - #716
Merged
Conversation
Most of #700 was already shipped. The issue's "Context (verified)" says there is no `/api/orders` endpoint and no Orders view; both have existed since #659, along with expected-vs-actual fill, the divergence and whether it went against keel, fee with a charged/modelled flag, the placement record, `venue_order_id`, and #626's submit-time book. What was actually missing was two things, and this is those two. THE STATUS FILTER. `gather_orders(status=)`, applied in the same pass as the scope so `get_orders()` is still called with no arguments at all -- the load-bearing pin in that function is unchanged, and its reason was always about MODE rather than about filtering in general. Server-side because a client filtering the page it received under `?limit=` would be filtering fifty rows and presenting the result as every canceled order: the same argument that already puts `?sort=` here. An unknown status is applied, not refused. `?sort=`'s columns are a closed set this code declares, so an unknown one is a client bug worth a 400; a status is written by whatever placed the order, and `_ORDER_STATUS_STATES` already carries `cancelled` beside `canceled` because spellings differ. A refusal would reject a status some book genuinely holds. An unmatched filter comes back empty saying which emptiness it is, which shows a typo as plainly as a 400 and cannot be wrong about a real word. Three new report fields, each because a number already on the page would otherwise be quietly wrong: - `statuses` -- every status in the WHOLE book, which is what a tab bar is built from. Scoped, the Canceled tab would disappear 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`. - `filtered_count` -- `scoped_count` keeps its meaning (rows in the WINDOW), so a filtered view needs its own denominator. Without it "2 of 3 filled" counts rows from other tabs: every number true, the sentence false. - `empty_reason: "status"` -- a fourth way to render zero rows, and a fourth sentence. Reusing the empty-book wording would tell a reader keel has never traded on a deployment whose book is full. Checked widest-cause-first, so an empty book is never blamed on the open tab. The note is a FIXED string that does not echo the requested status. `?status=` is caller-controlled text; it crosses in `status`, which the client places as a value. THE RULE THAT PLACED IT. `rule_id` alone is a foreign key -- 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 table and the track-record table cannot name one rule two ways. Resolved from ONE read of the rules table, passed down as a map: a per-row lookup would be one query per order against a book capped at `MAX_ORDERS_LIMIT`, and would be invisible on any fixture small enough to read, so there is a pin on the read count. `rule_name_detail` separates two absences that both render as an empty cell: no rule was recorded against this order, or a rule was and that rule has left the book. Only the second is worth investigating. That second state cannot be written through keel today -- `orders.rule_id` carries a FOREIGN KEY and `db.connect` enables `PRAGMA foreign_keys = ON` -- so its test drives `_row_from_dict` directly rather than being refused by SQLite instead of by the code under test. The branch stays: the map can also miss for a row from an older keel, an external sqlite3 session with the pragma off, or a partial map. WHAT IS NOT HERE, AND WHY. #700 also asked for per-order quote provenance and `client_order_id`. Neither is recorded anywhere: the provenance strings exist only as confirm-gate banners (`confirm.py:33-36`) describing a preview shown to a human, and `client_order_id` is not a column. Both are split into #715 to be captured at placement first -- deriving either here would put a figure on the page that no record supports. The third omission is a refusal, not a gap: #700 asked for a disclosure exposing `raw_response`, and `payload.py` now records why it will not. The column holds whatever the venue chose to send -- session metadata, internal identifiers, error schemas nobody here has read -- and the service's reduction to `venue_order_id` exists so that emitting the rest is impossible here rather than one line away. What would reverse it is a curated, named set of venue fields, which is a different feature with its own review. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ble (#700) Review findings on this branch. The first one is the reason the rest are worth reading. THE COLUMN SHIFT. `table()` pairs `columns[index]` with `row.entries()` BY POSITION. The "rule" header was added at index 6 and no cell was added with it, so the Orders table declared fourteen headers over thirteen values: every column from index 6 rendered one place left, and the last rendered nothing. "rule" showed the quantity, "expected" showed the actual fill, "divergence" showed the fee, "fee" showed the PLACEMENT TIMESTAMP, and "placed (UTC)" had no cell at all. Every value on the page was individually true and every column named the wrong one -- on the view whose entire purpose is per-order honesty. The test that was supposed to cover this asserted `'"rule_name"' in code`, which the header declaration alone satisfies. It was written after the code and pinned the wrong thing. Replaced by a scan that compares HEADER COUNT to CELL COUNT for every `table()` call in `render.js` -- it found exactly one mismatch, `(14, 13)`, and it covers the other six tables for free. Nothing in the suite compared those two lists before. FOCUS. `statusSwitch`'s buttons carried no `data-focus`. Pressing a tab replaces the view, which destroys the button that was pressed, and `main.js` puts focus back by that attribute -- without it a keyboard user is returned to the top of the document on every tab press. `scopeSwitch`, twenty-five lines below and otherwise copied, has always set it. A CLAIM THE CODE COULD NOT SUPPORT. `MISSING_RULE_DETAIL` -- "rule 7 placed this order and is no longer in the book" -- also fired when the caller passed NO rule map, and when the rule was found with an empty `kind`. The first states the result of a search nobody ran, and would send a reader hunting for a deleted rule that is sitting in the table; the second calls a rule absent while it is present. `_row_from_dict`'s own docstring already said the right thing ("Omitted, every row reports its rule as unresolved rather than claiming there is none") -- the code disagreed with it. `UNRESOLVED_RULE_DETAIL` is the third sentence those two cases needed. TABS THAT MATCH NOTHING. `statuses` stored the column verbatim while `status` was lowercased, and the client marks the active tab with `name === current`. A book row spelled `Filled` produced a tab that sent `?status=Filled`, got `filled` echoed back, and highlighted neither itself nor "all" -- and a book holding both spellings showed two tabs returning identical rows. Folded at the source. This is not hypothetical by this code's own reasoning: `api.py` declines to refuse unknown statuses precisely because venue-written and older spellings exist in real books, so the case it anticipates was the case it mishandled. THE TERMINAL WAS LEFT BEHIND. The service resolves the rule name for BOTH front-ends and only the browser rendered it, so `keel orders` could not answer a question the report already held. And `render_orders` had no branch for `empty_reason == "status"`: it printed its header, "showing 0 of N", and then nothing -- the silent blank the other two branches exist to prevent, and the one most likely to read as "keel has never traded". Unreachable from `keel orders` today (no `--status` flag), so it is pinned on the renderer rather than on the command. It now names the statuses the book does hold, because the next thing a reader wants is which filter would have worked. Also: "in this status" is captioned only when a status is actually on -- unfiltered it repeated the count below it under a label naming a filter that was not in force -- and `scopeSwitch`'s doc block, which the new function had been inserted underneath, is back above `scopeSwitch`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Most of #700 was already shipped. The issue's "Context (verified)" says there is no
/api/ordersendpoint and no Orders view; both have existed since #659, along with expected-vs-actual fill, the divergence and whether it went against keel, fee with a charged/modelled flag, the placement record,venue_order_id, and #626's submit-time book. What was actually missing was two things, and this is those two.THE STATUS FILTER.
gather_orders(status=), applied in the same pass as the scope soget_orders()is still called with no arguments at all -- the load-bearing pin in that function is unchanged, and its reason was always about MODE rather than about filtering in general. Server-side because a client filtering the page it received under?limit=would be filtering fifty rows and presenting the result as every canceled order: the same argument that already puts?sort=here.An unknown status is applied, not refused.
?sort='s columns are a closed set this code declares, so an unknown one is a client bug worth a 400; a status is written by whatever placed the order, and_ORDER_STATUS_STATESalready carriescancelledbesidecanceledbecause spellings differ. A refusal would reject a status some book genuinely holds. An unmatched filter comes back empty saying which emptiness it is, which shows a typo as plainly as a 400 and cannot be wrong about a real word.Three new report fields, each because a number already on the page would otherwise be quietly wrong:
statuses-- every status in the WHOLE book, which is what a tab bar is built from. Scoped, the Canceled tab would disappear on a quiet day and return tomorrow, and a control that comes and goes is worse than one that is sometimes empty. Same reasoning asmodes.filtered_count--scoped_countkeeps its meaning (rows in the WINDOW), so a filtered view needs its own denominator. Without it "2 of 3 filled" counts rows from other tabs: every number true, the sentence false.empty_reason: "status"-- a fourth way to render zero rows, and a fourth sentence. Reusing the empty-book wording would tell a reader keel has never traded on a deployment whose book is full. Checked widest-cause-first, so an empty book is never blamed on the open tab.The note is a FIXED string that does not echo the requested status.
?status=is caller-controlled text; it crosses instatus, which the client places as a value.THE RULE THAT PLACED IT.
rule_idalone is a foreign key -- a number a reader cannot act on. The name isrules.kind, the same stringbuild_rule_track_recordcallsrule_name, so the Orders table and the track-record table cannot name one rule two ways. Resolved from ONE read of the rules table, passed down as a map: a per-row lookup would be one query per order against a book capped atMAX_ORDERS_LIMIT, and would be invisible on any fixture small enough to read, so there is a pin on the read count.rule_name_detailseparates two absences that both render as an empty cell: no rule was recorded against this order, or a rule was and that rule has left the book. Only the second is worth investigating. That second state cannot be written through keel today --orders.rule_idcarries a FOREIGN KEY anddb.connectenablesPRAGMA foreign_keys = ON-- so its test drives_row_from_dictdirectly rather than being refused by SQLite instead of by the code under test. The branch stays: the map can also miss for a row from an older keel, an external sqlite3 session with the pragma off, or a partial map.WHAT IS NOT HERE, AND WHY. #700 also asked for per-order quote provenance and
client_order_id. Neither is recorded anywhere: the provenance strings exist only as confirm-gate banners (confirm.py:33-36) describing a preview shown to a human, andclient_order_idis not a column. Both are split into #715 to be captured at placement first -- deriving either here would put a figure on the page that no record supports.The third omission is a refusal, not a gap: #700 asked for a disclosure exposing
raw_response, andpayload.pynow records why it will not. The column holds whatever the venue chose to send -- session metadata, internal identifiers, error schemas nobody here has read -- and the service's reduction tovenue_order_idexists so that emitting the rest is impossible here rather than one line away. What would reverse it is a curated, named set of venue fields, which is a different feature with its own review.What & why
Tests-first evidence
Gates (all must pass)
uv run ruff checkcleanuv run mypycleanuv run pytest -qgreenScope check
leave checked only if true, and if so: cite the source and open the discussion
BEFORE review (CONTRIBUTING.md, "Governance: rulings vs. machinery").