feat(web): the JSON API over the existing reports, with server-side sort - #547
Merged
Conversation
…en keel isn't running Nine `GET /api/*` endpoints over the reports the console already builds, one per read the rendered routes perform. `/glossary` gets no counterpart on purpose: it becomes an outbound keeltrading.com link in #539 and its renderer is deleted in #540, so an `/api/glossary` would be a surface built in order to be removed, and any client written against it would break on the release that removes it. `test_the_glossary_has_no_api_counterpart` states that absence as an assertion rather than leaving it as an omission. Reads only. Not one route answers a POST; `X-Keel-Client: 1` still gates `POST /api/*` and still does not gate `/setup/*`; `keel/web/security.py` has a nought-line diff and so does `tests/commands/test_console_thinness.py`. THE ENVELOPE. Every success is `{as_of, engine, data, sort}` with a constant key set, so #536's single `fetch` wrapper needs no per-endpoint branch -- a client that must test whether a key is present is branching on payload SHAPE, which is the inference Rule 3 exists to remove. `engine` is a judged field with a closed two-word vocabulary; a stopped engine answers `data: null` at HTTP **200**, because #538's service worker and #536's wrapper both read a non-ok status as "the server is unreachable", and a first-run user who has set nothing up is not in an outage. `null` rather than `{}`: an empty object is a payload with every figure missing, and a view handed one renders zeros. What `engine` does NOT claim is that the agent ran recently. That needs a THRESHOLD, and the serialiser holds none by design (`_freshness_payload` refuses the same temptation). The evidence is already on the wire without arithmetic -- `data_freshness` carries candle ages in the CLI's own words, and the activity feed carries `last_cycle_before_scope`, which exists precisely so an empty view can say when keel last ran. A third `engine` word arrives when a report builder holds a heartbeat WITH its staleness verdict, and it is copied here, not computed here. REJECTED: a third `engine` word for "the report raised". The client behaviour required by a stopped engine and by an unbuildable report is identical -- show no figures, say why -- so it would be three branches to get two behaviours, and the HTTP status plus `error.detail` already carry the difference. REJECTED: one envelope with a nullable `error`, told apart by a field. Most refusals happen BEFORE the session cookie is checked, and filling in `engine` there would mean an unauthenticated request reading the deployment state off disk. The discriminator is the HTTP status, which `res.ok` already checks. SORT. `?sort=<column>&dir=asc|desc`, refused with the column list when the column is not declared -- silently ignoring one is how a client ships a sort header that does nothing and nobody notices for a release. Python orders with `Decimal`, in `payload.order_rows`, next to the `_plain` that wrote the strings being ordered. Measured reason, in the test: two ERC-20 quantities one wei apart (`0.100000000000000001` / `...002`, both legal at an 18-decimal base increment) map to the SAME double, because adjacent doubles near 0.1 are ~1.4e-17 apart. A float-keyed sort leaves the tied pair in arrival order; a mutation run confirms it answers `[0.099, ...002, ...001]` where `Decimal` answers `[0.099, ...001, ...002]`. Absent values sort last in BOTH directions -- `None` is "not recorded", not zero, and giving one a numeric key makes it the largest thing in a descending sort, which is the always-passing fee rail (#198) in a different hat. REJECTED: `?sort=-expectancy`. One signed token needs a grammar -- what a leading `-` means, what happens to a column starting with one, how a client strips it before comparing against `sort.columns`. Two orthogonal facts, two parameters, no parsing beyond a table lookup. REJECTED: sorting the source dataclasses before serialising. That needs a map from every payload key back to the report attribute it came from, in a second file, and the map rots the first time a key is renamed. `Field.value` is documented as exact and `Decimal`-parseable; re-parsing it is what it is for. The numeric `sort` companion field #533 rejected is still not here. REJECTED: numeric-or-text decided per row. That puts a `Decimal` and a `str` in one comparison, which raises. It is decided per COLUMN -- numeric only when every present value parses finite, so a `Decimal("NaN")` sentinel (which parses, and compares false to everything) drops the whole column to text ordering rather than making the result depend on the sort's comparison order. `GET /api/config` serves `version` (#539's `?v=`) and `build` (#538's cache key) as separate fields off a `BuildInfo` resolved ONCE by `serve_cmd`. `build_info()` shells out to git twice, and an endpoint a service worker polls must not fork a subprocess to answer; parsing the fields back out of the footer's `describe()` line would be a display string read as data. `GET /api/*` does not require `X-Keel-Client`, deliberately. The header buys a CORS preflight a hostile origin cannot satisfy, closing the plain-form-POST gap that `SameSite=Strict` and the HMAC token both assume shut -- a GET is not that gap: a cross-origin read cannot see the response without CORS headers this server never sends, and `SameSite=Strict` denies the cookie before any of it matters. Requiring it would cost `curl http://127.0.0.1:8765/api/status` and the address bar, which is how an operator checks the interface is telling the truth. Reverse it the day a GET can change something. Refusals under `/api/*` are JSON, path-scoped rather than `Accept`-negotiated: an HTML error page handed to `res.json()` is a parse error in the client, a worse diagnostic than the 403 it hides. The gate in front of `POST /api/*` is untouched -- same trigger, same status, same ordering; only the body's media type follows the path. Two stale claims corrected while in the area, both checked rather than inherited: `keel/web/__init__.py` still said "the request handler implements do_GET and do_HEAD and nothing else", which stopped being true at #437 -- and `server.do_POST` points readers at that file for the rule it must not break. `server.py`'s docstring said "six read-only pages" over eight routes. `open_repo`/`load_config`/`deployment_state`/`close_repo` moved from `server.py` to `api.py` unchanged, with their reasoning: both front-ends read keel through them, and a copy in each file would be two places deciding whether a view may migrate a live database. Measured: `setup.inspect` is 3.6 ms per call against a migrated database, vs 0.07 ms for `gather_status`. It runs once per response on every endpoint, `/api/config` included, because a uniform `engine` field is what lets one client wrapper handle every route. If something ever polls `/api/config` in a loop the fix is to cache the probe, not to make the field optional -- an optional field puts the branch back on the client. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NyeggYtojNXCTHeD3JHxb6
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
1 participant
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.
Closes #534. Unblocks #536–#539.
Reads only. No new write surface, and
keel/web/security.py,tests/commands/test_console_thinness.pyandtests/web/test_server.pyall have a 0-line diff — verified against the branch, not asserted.The envelope
Every route answers with the same key set:
{as_of, engine, data, sort}. Failures are{as_of, data: null, error: {status, title, detail}}, wherestatusis a string ("404", not404) — the no-JSON-numbers rule reaches the error document too, so a client never has to switch parsing mode./api/config/api/status/open_positions/api/setup/setup/api/activity/activitycycles/api/insights/insightsrules/api/journal/insightsentries/api/rules/rulesrules/api/venues/venuesvenues/api/gates/gates/glossarydeliberately has no counterpart — it becomes an outbound link in #539.test_the_api_routes_cover_every_html_route_that_readscompares both route tables mechanically, so #540 cannot delete an HTML route the API never replaced.Sorting
?sort=<column>&dir=asc|desc. An unknown column or direction is a 400 naming the valid ones — silently ignoring is how a dead sort control ships.sort.columnsis echoed even when nothing was sorted, so #537 reads the column list off the response rather than hardcoding it.Ordering lives in
payload.order_rows, beside the_plainthat wrote the strings, and isDecimalper column — never per row, which would mixDecimalandstrin one comparison. Absent values sort last in both directions.The float test asserts its own premise first. Two ERC-20 quantities one wei apart — legal at an 18-decimal base increment — are distinct as
Decimaland equal asfloat.test_float_and_decimal_orderings_of_the_same_column_genuinely_differstates that disagreement before the endpoint test runs, so the endpoint test cannot be green for the trivial reason that sorting sorts. Mutatingorder_rowstofloatproduces[0.099, ...002, ...001]whereDecimalgives[0.099, ...001, ...002].Engine stopped is a state, not an error
HTTP 200,
engine.state: "warn",data: null,as_ofpresent.Two deliberate calls recorded in the source: a non-2xx status is what #536/#538 will read as unreachable, and a first-run user with no deployment is not in an outage. And
data: nullrather than{}— a view handed an empty object renders zeros, which for a trading dashboard is worse than rendering nothing./api/config,/api/venuesand/api/gatesstill carry data when stopped: they describe the binary, not a deployment.The routing layer formats nothing, enforced by AST
test_the_routing_layer_formats_nothingwalksapi.pyand fails on anyformat()call or f-string carrying a format spec — "a second money renderer is never the one with_plain's no-exponent guarantee in it."test_rule_6_holds_in_the_api_layer_tooruns #533's own_rule6_findingsoverapi.py, rather than adding a stem toSERIALISER_STEMS— which would have meant editing the pinned file. That is whytest_console_thinness.pyis a 0-line diff.Four temptations to compute in routing are recorded with what was done instead, including a per-gate action count that is not emitted at all (Rule 6e bans
len(); the client readsactions.length).Shared test helper — declared
tests/web/test_payload.py::_every_payloadgained five entries. Purely additive, no signature change, and it brings the new payloads under the existing no-JSON-numbers walk and state-vocabulary check. Mutation-checked: a bare1and a"purple"state planted in the new builders fail both guards.It deliberately did not reuse
test_server.py::_requestintest_api.py— that helper defaultsclient_header="1", which would have answered the central GET question on the test's behalf. That is the #545 failure mode, avoided on purpose, with the reasoning in the module docstring.Two stale claims found and corrected
keel/web/__init__.pystill said "the request handler implementsdo_GETanddo_HEADand nothing else" — false since #437. Corrected rather than deleted, becauseserver.do_POSTpoints readers at that file for the rule it must not break, and the replacement guarantee is stronger: the write surface is a closed set, and no capability-increasing action is reachable.server.py's "six read-only pages" over eight routes is fixed too.One measurement worth knowing
setup.inspectis 3.6 ms/call against 0.07 ms forgather_status— 50×. It runs once per response on every endpoint including/api/config, deliberately, soengineis uniform and #536's client wrapper needs no table of which routes carry it. Imperceptible on loopback; recorded inapi.pywith the condition that would reverse it.Gates
ruff check keel tests packages— All checks passedmypy— no issues in 361 source filespytest -q— 4745 passed, 3 skipped (80 new intests/web/test_api.py)🤖 Generated with Claude Code
https://claude.ai/code/session_01NyeggYtojNXCTHeD3JHxb6