diff --git a/docs/superpowers/specs/2026-08-23-web-ui-rewrite-design.md b/docs/superpowers/specs/2026-08-23-web-ui-rewrite-design.md index eb16f66..ceac12f 100644 --- a/docs/superpowers/specs/2026-08-23-web-ui-rewrite-design.md +++ b/docs/superpowers/specs/2026-08-23-web-ui-rewrite-design.md @@ -257,7 +257,12 @@ by outbound documentation links. - **live** — the `EventSource` subscription and its reconnect behaviour. - **format** — `Intl.DateTimeFormat` wrappers. Dates only; money is formatted server-side. - **docs** — constructs outbound keeltrading.com links, with the anchor and the `?v=` version. -- **sw** — the service worker. +- **sw** — the service worker. **Amended at #538: this one file is NOT under `js/`.** A service + worker's registration scope is its own directory, so `static/js/sw.js` would be scoped to + `/static/js/` and could not answer a navigation to `/static/insights` — the deep links §"Static + assets" requires. It ships at `static/sw.js` instead. The alternative, a `Service-Worker-Allowed` + header widening the scope from `js/`, was rejected for failing silently: remove the header and + the worker still installs, still activates, and simply stops controlling the app. In **js/external** there is nothing, and that is the intended end state rather than a stage. diff --git a/keel/web/render.py b/keel/web/render.py index fbd8419..36a6158 100644 --- a/keel/web/render.py +++ b/keel/web/render.py @@ -24,8 +24,20 @@ from collections.abc import Iterable, Sequence from decimal import Decimal from typing import Any +from urllib.parse import quote + +#: The published documentation root. Spelled here and in `static/js/docs.js`, and +#: `tests/web/test_doc_links.py` pins that the two agree. +DOCS_URL = "https://keeltrading.com/en/docs/" #: Nav order, and the labels. `/` first because the status page is the answer to "is it alive". +#: +#: **The eighth entry is an OUTBOUND link, and used to be a page (#539).** `/glossary` rendered +#: `docs/glossary.md` read from the working directory -- which no installed deployment has, since +#: `docs/` sits at the repository root, outside the `keel/` module `uv_build` packages. Every +#: install therefore rendered an empty glossary, and `help_console.load_glossary`'s docstring said +#: so. It is a link now, for the same reason the client's is: linking is the only form of this +#: that reaches an installed deployment at all. NAV: tuple[tuple[str, str], ...] = ( ("/", "Status"), ("/setup", "Setup"), @@ -34,7 +46,7 @@ ("/rules", "Rules"), ("/venues", "Venues"), ("/gates", "Gates"), - ("/glossary", "Glossary"), + (DOCS_URL, "Docs"), ) _STYLE = """ @@ -304,6 +316,7 @@ def page( path: str, body: str, build: str = "", + version: str = "", refresh_sec: int | None = None, ) -> str: """The document shell. `refresh_sec` emits a `` -- a zero-JS @@ -313,7 +326,29 @@ def page( nav_items = [] for href, label in NAV: on = ' class="on"' if href == path else "" - nav_items.append(f'{esc(label)}') + # An outbound entry is navigation, not a subresource, so `default-src 'none'` does not + # reach it. `noopener` still does matter: a new tab opened without it holds a + # `window.opener` handle back to a trading console on a token-bearing origin. + # + # `?v=` carries the running build, exactly as `static/js/docs.js` does for the client: + # the site pins `main` while an operator runs a tagged release, so a linked page can + # describe behaviour their build does not have. That skew is made VISIBLE rather than + # solved -- the build ends up in the URL bar of the page they are reading. `quote`, not + # an f-string, because a full version is `0.11.2+c1634a3fa17f` and a raw `+` in a query + # string decodes to a space. + # + # `version`, NOT `build`, and the two are different strings: `build` is the footer's + # human-readable LINE -- `keel 0.11.2+c1634a3fa17f (DIRTY) [checkout]` -- and the first + # spelling of this used it, putting that whole sentence percent-encoded into the query. + # Caught by looking at the rendered href, not by a test: both forms are non-empty + # strings and every assertion about "the link carries a version" passed. + away = "" + target = href + if href.startswith("https://"): + away = ' target="_blank" rel="noopener noreferrer"' + if version: + target = href + "?v=" + quote(version, safe="") + nav_items.append(f'{esc(label)}') nav = "".join(nav_items) meta_refresh = ( f'' if refresh_sec else "" @@ -948,34 +983,5 @@ def render_gates(gates: Sequence[Any], capabilities: Sequence[Any]) -> str: return "".join(parts) -def render_glossary(terms: Sequence[Any]) -> str: - parts = [ - '

Glossary

keel\'s vocabulary, and the fiqh terms it anchors to

', - '
', - ] - for term in terms: - marker = ' fiqh' if term.fiqh else "" - if term.fiqh and not term.stated: - marker = ' not stated in fiqh-basis' - parts.append(f"
{esc(term.term)}{marker}
") - parts.append(f"
{esc(term.definition)}
") - source = term.citation or term.source - if source: - parts.append(f'
{esc(source)}
') - parts.append("
") - if not terms: - # The normal state of an INSTALLED deployment, not a bug: `docs/glossary.md` is read from - # the working directory, and a deployment folder is a config, a database and an .env -- - # there is no docs checkout beside them. The TUI's help screen shows the same empty state - # for the same reason. Packaging the docs inside the artifact is D5's business (#438); - # until then this says which file is missing rather than implying the glossary is empty. - parts.append( - '

No glossary here. keel reads docs/glossary.md from ' - "the folder it is run in, and an installed deployment has no docs checkout beside " - "its config and database.

" - ) - return "".join(parts) - - def render_message(heading: str, detail: str) -> str: return f'

{esc(heading)}

{esc(detail)}

' diff --git a/keel/web/server.py b/keel/web/server.py index 65acdc8..4031740 100644 --- a/keel/web/server.py +++ b/keel/web/server.py @@ -280,17 +280,11 @@ def page_gates(_cfg: ServeConfig, _query: dict[str, list[str]]) -> tuple[str, st return "Gates", render.render_gates(GATES, CAPABILITIES), None -def page_glossary(_cfg: ServeConfig, _query: dict[str, list[str]]) -> tuple[str, str, int | None]: - from keel.commands.help_console import load_glossary - - return "Glossary", render.render_glossary(load_glossary()), None - - ROUTES: dict[str, Callable[[ServeConfig, dict[str, list[str]]], tuple[str, str, int | None]]] = { # First-run detection (#437): every page that reads the database serves the checklist when # there is no database to read, rather than a 500 whose real cause is that the user has not - # set anything up yet. `/venues`, `/gates` and `/glossary` are not wrapped -- none of them - # touches the deployment, and all three are useful before one exists. + # set anything up yet. `/venues` and `/gates` are not wrapped -- neither touches the + # deployment, and both are useful before one exists. "/": needs_database(page_status), "/setup": page_setup, "/activity": needs_database(page_activity), @@ -298,7 +292,6 @@ def page_glossary(_cfg: ServeConfig, _query: dict[str, list[str]]) -> tuple[str, "/rules": needs_database(page_rules), "/venues": page_venues, "/gates": page_gates, - "/glossary": page_glossary, } @@ -466,6 +459,22 @@ def run_setup_action(cfg: ServeConfig, key: str, form: dict[str, str]) -> Any: _JSON_CONTENT_TYPE = "application/json; charset=utf-8" +def _docs_version(cfg: ServeConfig) -> str: + """The build the nav's documentation link should report, or `""`. + + Read off `build_info` rather than off `cfg.build`, because those are different strings and + only one of them is a version: `cfg.build` is the footer's human-readable LINE + (`keel 0.11.2+c1634a3fa17f (DIRTY) [checkout]`), and putting it in a query string produced + `?v=keel%200.11.2%2B...%20%28DIRTY%29%20%5Bcheckout%5D`. This is the same field `/api/config` + hands the client for the same purpose (`payload.config_document`'s `"build"`), so both + front-ends report the identical string while both exist. + + `""` when there is no build info at all -- an unversioned link is honest, and a link claiming + `?v=unknown` is not. + """ + return str(getattr(cfg.build_info, "full_version", "") or "") + + def _static_headers(content_type: str) -> tuple[tuple[str, str], ...]: """`_STATIC_BASE_HEADERS` plus CSP, but ONLY when `content_type` is one of `_CSP_CONTENT_TYPES` -- see the comments on `_STATIC_BASE_HEADERS` and `_CSP_CONTENT_TYPES` @@ -571,6 +580,7 @@ def _refuse(self, code: int, heading: str, detail: str) -> None: path="", body=render.render_message(heading, detail), build=self.cfg.build, + version=_docs_version(self.cfg), ), ) @@ -906,6 +916,7 @@ def do_GET(self) -> None: # noqa: N802 - stdlib's naming, not ours path=parsed.path, body=body, build=self.cfg.build, + version=_docs_version(self.cfg), refresh_sec=refresh, ), ) diff --git a/keel/web/static/css/keel.css b/keel/web/static/css/keel.css index db9962e..de68061 100644 --- a/keel/web/static/css/keel.css +++ b/keel/web/static/css/keel.css @@ -181,6 +181,22 @@ h2 { font-size: 1.05rem; margin: 2rem 0 0.6rem; } } .kv .v { font-size: 1.05rem; font-variant-numeric: tabular-nums; } +/* #539: a label that names a documented term links out to its definition. + `--muted`, NOT `--accent`: the label is the quietest text on the card and a link colour there + would pull the eye off the figure beside it, which is the thing the operator came to read. The + dotted underline is what marks it as a link -- and it is an underline rather than colour alone, + because #532's whole finding was that this palette had been separating meanings by hue. + `text-decoration-thickness` is set because a dotted border-bottom would sit below the descenders + and read as a divider instead. */ +.kv .k .doclink { + color: inherit; + text-decoration: underline dotted; + text-underline-offset: 0.2em; + text-decoration-thickness: 1px; +} +.kv .k .doclink:hover, +.kv .k .doclink:focus-visible { color: var(--fg); text-decoration-style: solid; } + .tablewrap { overflow-x: auto; /* A scrollable region must be reachable by keyboard. `tabindex="0"` on the wrapper (set in diff --git a/keel/web/static/icons/keel-192.png b/keel/web/static/icons/keel-192.png new file mode 100644 index 0000000..df63420 Binary files /dev/null and b/keel/web/static/icons/keel-192.png differ diff --git a/keel/web/static/icons/keel-512.png b/keel/web/static/icons/keel-512.png new file mode 100644 index 0000000..fae018d Binary files /dev/null and b/keel/web/static/icons/keel-512.png differ diff --git a/keel/web/static/icons/keel-maskable-512.png b/keel/web/static/icons/keel-maskable-512.png new file mode 100644 index 0000000..0c84478 Binary files /dev/null and b/keel/web/static/icons/keel-maskable-512.png differ diff --git a/keel/web/static/icons/keel.svg b/keel/web/static/icons/keel.svg new file mode 100644 index 0000000..1672d71 --- /dev/null +++ b/keel/web/static/icons/keel.svg @@ -0,0 +1 @@ + diff --git a/keel/web/static/index.html b/keel/web/static/index.html index 8df55e6..f6ec75c 100644 --- a/keel/web/static/index.html +++ b/keel/web/static/index.html @@ -38,6 +38,22 @@ to a declared base. --> + + + + + @@ -55,6 +71,23 @@
  • Rules
  • Venues
  • Gates
    • + +
  • Docs
    • diff --git a/keel/web/static/js/docs.js b/keel/web/static/js/docs.js new file mode 100644 index 0000000..adeb0e1 --- /dev/null +++ b/keel/web/static/js/docs.js @@ -0,0 +1,141 @@ +// @ts-check +/** + * Outbound documentation links (#539). **The app fetches, bundles and caches nothing.** + * + * ── WHY LINKING IS THE ONLY OPTION, NOT THE CHEAP ONE ─────────────────────────────────────── + * `docs/` lives at the REPOSITORY root, outside `keel/`, and `uv_build` packages the module root + * -- so no wheel has ever carried it. Every installed deployment, the signed desktop bundle + * included, renders an empty glossary today, and `keel/commands/help_console.py` says so in its + * own docstring: "an installed deployment has no docs/ checkout, and the help screen renders + * that notice as its empty state." + * + * That is not fixable by adding a packaging glob -- it is structural. Measured at #535: building + * with `artifacts` set and with `artifacts = []` produces byte-identical wheels, because + * `uv_build` ships the whole module root regardless of the key. Linking out is the only option + * that reaches an installed deployment at all. + * + * ── NO OFFLINE FALLBACK, DELIBERATELY ─────────────────────────────────────────────────────── + * No inline definitions, no cached snapshot, no entry in the service worker's `PRECACHE`. An + * operator running a trading engine has network by definition, and #538's whole argument is that + * a cached copy of something authoritative is worse than no copy: a definition that has since + * changed, presented as current, with nothing on screen to say which it is. + * + * ── THE ANCHOR CONTRACT, AND WHY A TEST HOLDS IT ──────────────────────────────────────────── + * `docs/glossary.md` states its own rule -- "Each entry is a `## term` heading, a definition, + * and a `Source:` line" -- and Astro emits kebab-cased IDs for those headings, so the anchor for + * a term is its heading kebab-cased. Nothing in either repository enforces that from the other + * side: a heading renamed upstream would break every deep link here **silently**, because a bad + * fragment is not an error, it is a page that opens at the top. + * + * `tests/web/test_doc_links.py` closes that by parsing this table and asserting every anchor + * exists as a heading in the named document, in this repository, where `docs/` is the source. + */ + +/** The published documentation root. One string, spelled once. */ +const SITE = "https://keeltrading.com/en/docs/"; + +/** + * The running build, for `?v=`. + * + * **Version skew is made VISIBLE here, not solved.** The site pins `main` while an operator runs + * a tagged release, so a linked page can describe behaviour their build does not have. Per- + * version documentation paths were rejected: `keeltrading.com/en/docs/v0.11.0/glossary` 404s + * today, and building versioned trees is work in the other repository plus a retention policy, + * across three languages and a sitemap. Carrying the version in the query string costs nothing + * and puts the operator's build in the URL bar of the page they are reading. + * + * Module state, written exactly once, at boot, by `main.js` -- the alternative is threading a + * version string through every render function to reach the four places that build a link. + */ +let version = ""; + +/** + * Record the build every documentation link should carry. Called once from `main.js`, from the + * same `/api/config` read that fills the footer. + * + * @param {string} build + */ +export function rememberVersion(build) { + version = typeof build === "string" ? build : ""; +} + +/** + * The URL for one document, optionally at one anchor. + * + * The trailing slash on the slug is not cosmetic: the site serves `…/docs/glossary/index.html`, + * and the un-slashed form is a redirect that some browsers resolve by dropping the fragment -- + * a deep link that lands at the top of the page, which is exactly the failure this module's + * anchor table exists to prevent. + * + * @param {string} slug a document slug, or `""` for the documentation index. + * @param {string} anchor a heading anchor, or `""` for the top of the page. + * @returns {string} + */ +export function documentUrl(slug, anchor) { + let url = SITE; + if (slug) url = url + slug + "/"; + if (version) url = url + "?v=" + encodeURIComponent(version); + if (anchor) url = url + "#" + anchor; + return url; +} + +/** The documentation index, for the header's outbound link. @returns {string} */ +export function indexUrl() { + return documentUrl("", ""); +} + +/** + * The labels this client puts on screen that name a term the documentation defines, mapped to + * where it is defined. + * + * **Keyed by the LABEL, not by the term.** The alternative -- tagging each call site with a term + * name -- spreads the decision across six views and makes "which words on this screen are + * defined somewhere" unanswerable without reading all of them. Keyed by label, the whole answer + * is this table, and `kv` consults it for every pair it builds, so a label that names a term is + * a link wherever it appears without a call site knowing. + * + * **Deliberately NOT exhaustive, and the omissions are the point.** `mode` reads `paper` or + * `live` and would need two different targets for one label; `evidence required` sits on the + * CAPABILITY gates (`keel.capabilities.GATES`), not the promotion gate, and linking it to + * `#promotion-gate` would be confidently wrong. A missing link costs a reader one search. A + * wrong one costs them their trust in every other link on the page. + * + * @type {Record} + */ +export const TERMS = { + autonomy: { slug: "glossary", anchor: "autonomy" }, + "autonomy configured": { slug: "glossary", anchor: "autonomy" }, + "autonomy lapses": { slug: "glossary", anchor: "autonomy" }, + "kill switch": { slug: "glossary", anchor: "kill-switch" }, + "rail 11": { slug: "glossary", anchor: "rail" }, + "withdrawal attestation (rail 17)": { slug: "glossary", anchor: "attestation" }, + "market session": { slug: "glossary", anchor: "market-clock" }, + session: { slug: "glossary", anchor: "session-bound-venue" }, + "paper stage": { slug: "glossary", anchor: "paper-mode" }, +}; + +/** + * A label as an outbound link to its definition, or `null` if the label names no term. + * + * `rel="noopener"` with `target="_blank"`: a new tab opened without it gets a `window.opener` + * handle back to this page, and this page is a trading console on a token-bearing origin. + * `noreferrer` too -- the server already sends `Referrer-Policy: no-referrer`, and a link that + * states it as well is one that keeps holding if this markup is ever read somewhere the header + * is not sent. + * + * @param {string} label + * @returns {HTMLAnchorElement | null} + */ +export function termLink(label) { + const target = Object.prototype.hasOwnProperty.call(TERMS, label) ? TERMS[label] : null; + if (!target) return null; + const anchor = document.createElement("a"); + anchor.className = "doclink"; + anchor.textContent = label; + anchor.href = documentUrl(target.slug, target.anchor); + anchor.target = "_blank"; + anchor.rel = "noopener noreferrer"; + // Named for a reader who arrives on the link out of context, and hears only the link text. + anchor.title = "Definition on keeltrading.com — opens in a new tab"; + return anchor; +} diff --git a/keel/web/static/js/main.js b/keel/web/static/js/main.js index 4d52cfa..bbd9eb2 100644 --- a/keel/web/static/js/main.js +++ b/keel/web/static/js/main.js @@ -34,6 +34,7 @@ */ import { read } from "./api.js"; +import { indexUrl, rememberVersion } from "./docs.js"; import { available, subscribe } from "./live.js"; import { activityView, @@ -127,6 +128,8 @@ const engineNode = must("engine"); const contentNode = must("content"); /** @type {HTMLElement} */ const buildNode = must("build"); +/** The header's outbound documentation link (#539); its href gains `?v=` once the build is known. */ +const docsNode = /** @type {HTMLAnchorElement} */ (must("docs-link")); /** * An element that `index.html` guarantees. Throwing beats rendering half a page: the two files @@ -585,13 +588,60 @@ if (window.location.pathname !== pathFor(booted)) { } /** - * The footer's build line, read once. + * Register the service worker (#538), keyed to the build that just answered. + * + * **After `/api/config`, never before, and that ordering is the whole design.** The worker's + * cache name comes from the build string, so registering before the build is known would install + * a worker under a name that has to be corrected on the next load -- two registrations, two + * caches, for one deployment. Waiting costs one round trip against a local socket. + * + * **A failed read registers nothing, deliberately.** With `keel serve` stopped this promise + * resolves with `data: null`, and the right response is to leave whatever worker is already + * installed exactly as it is: it is the one serving the shell that is letting the operator read + * this page at all. Re-registering it under `unknown` would swap a correct cache for an empty + * one at the precise moment the network cannot refill it. + * + * **`encodeURIComponent`, because the build string contains `+`.** `keel.version` produces + * `0.11.2+88fb17bcab15`, and a raw `+` in a query string decodes to a SPACE -- the worker would + * read a different build than the one that is running, and the cache key would silently stop + * tracking the binary it is supposed to track. + * + * @param {any} config `/api/config`'s `data`, or `null`. + */ +function registerWorker(config) { + if (!("serviceWorker" in navigator)) return; + const build = (config && (config.build || config.version)) || ""; + if (!build) return; + // Errors are swallowed on purpose and the app carries on: every failure mode here -- an + // unsupported browser, a user profile with workers disabled, a private window -- costs the + // offline shell and nothing else. A dashboard that refused to render because it could not + // install an optional cache would be trading a working page for a nicety. + void navigator.serviceWorker + .register(`${BASE}sw.js?v=${encodeURIComponent(build)}`, { scope: BASE }) + .catch(() => {}); +} + +/** + * The build, read once, and the three things that depend on it. * * Once, not per poll: `/api/config` describes the binary that is answering, and that cannot * change without the process restarting -- at which point the session token is new, every fetch * is a 403, and the banner says so. A version string re-read four times a minute would be four * times a minute spent confirming a constant. + * + * **The first view is painted from INSIDE this callback (#539), and that ordering is deliberate.** + * Every documentation link carries `?v=` (`docs.rememberVersion`), and a link built before + * the build is known would carry no version until the next poll -- fifteen seconds of links that + * quietly do not say which build the reader is running, on exactly the first screen they see. + * `/api/config` is the one endpoint that opens no database, so this costs a single round trip on + * a loopback socket, and it cannot hang the app: `api.read` resolves with a stopped reading + * rather than rejecting, so `show` runs even with nothing listening on the port. */ -void read("config").then((reading) => buildLine(buildNode, reading.data)); - -show(booted, false); +void read("config").then((reading) => { + const config = reading.data; + rememberVersion((config && (config.build || config.version)) || ""); + docsNode.href = indexUrl(); + buildLine(buildNode, config); + registerWorker(config); + show(booted, false); +}); diff --git a/keel/web/static/js/render.js b/keel/web/static/js/render.js index 64f7d11..dd7efe2 100644 --- a/keel/web/static/js/render.js +++ b/keel/web/static/js/render.js @@ -56,6 +56,7 @@ */ import { equityChart } from "./chart.js"; +import { termLink } from "./docs.js"; import { instant } from "./format.js"; /** @@ -158,7 +159,14 @@ function plain(value) { */ function kv(label, value) { const wrap = el("div", "kv"); - wrap.append(el("span", "k", label)); + // #539: a label that names a documented term becomes an outbound link to its definition, + // here rather than at each call site. `docs.TERMS` is the whole answer to "which words on + // this screen are defined somewhere", and a label it does not know stays plain text. + const linked = termLink(label); + const key = el("span", "k"); + if (linked) key.append(linked); + else key.textContent = label; + wrap.append(key); const holder = el("span", "v"); if (value instanceof Node) holder.append(value); else if (typeof value === "string") holder.textContent = value; diff --git a/keel/web/static/manifest.webmanifest b/keel/web/static/manifest.webmanifest new file mode 100644 index 0000000..5c2f385 --- /dev/null +++ b/keel/web/static/manifest.webmanifest @@ -0,0 +1,38 @@ +{ + "id": "/static/", + "name": "keel", + "short_name": "keel", + "description": "keel's read-only console, served from this machine only.", + "start_url": "/static/status", + "scope": "/static/", + "display": "standalone", + "orientation": "any", + "background_color": "#fbfaf8", + "theme_color": "#1a5578", + "icons": [ + { + "src": "/static/icons/keel.svg", + "sizes": "any", + "type": "image/svg+xml", + "purpose": "any" + }, + { + "src": "/static/icons/keel-192.png", + "sizes": "192x192", + "type": "image/png", + "purpose": "any" + }, + { + "src": "/static/icons/keel-512.png", + "sizes": "512x512", + "type": "image/png", + "purpose": "any" + }, + { + "src": "/static/icons/keel-maskable-512.png", + "sizes": "512x512", + "type": "image/png", + "purpose": "maskable" + } + ] +} diff --git a/keel/web/static/sw.js b/keel/web/static/sw.js new file mode 100644 index 0000000..a7c463c --- /dev/null +++ b/keel/web/static/sw.js @@ -0,0 +1,172 @@ +/** + * keel's service worker (#538). + * + * ── THE ONE RULE, AND WHY IT IS STRUCTURAL RATHER THAN CAREFUL ────────────────────────────── + * A PWA that caches financial data is actively dangerous: opening the app to last week's equity + * styled as current is worse than an error, because an error is visible. So `/api/*` is + * `NetworkOnly`, "no exceptions" (the design spec's Service worker table). + * + * That rule is enforced three times over, and the first one is the reason to trust it: + * + * 1. **Scope.** This file is served from `/static/`, so its registration scope is `/static/` + * and `/api/*` is OUTSIDE it. A service worker's `fetch` handler is never invoked for a + * request outside its own scope -- not "is skipped", not "returns early": the browser does + * not consult it at all. No edit to this file can cache an API response, because no edit to + * this file can see one. + * 2. **`PRECACHE`, a closed list.** The only writes to the cache are `addAll(PRECACHE)` at + * install. There is no runtime `cache.put`, anywhere, so there is no code path by which a + * response fetched later becomes a stored one. + * 3. **An explicit guard in `fetch`.** Belt and braces for the day #540 moves the shell to `/` + * and the scope widens to the whole origin -- at which point rules 1 and 2 stop being the + * same protection and this becomes the one that holds. `tests/web/test_service_worker.py` + * pins that the guard exists, so it cannot be tidied away as dead code before then. + * + * ── THE CACHE NAME IS THE BUILD, AND THAT IS THE SECOND HAZARD ────────────────────────────── + * `CacheFirst` on the shell means an upgraded engine could otherwise be met by a stale client + * holding an older contract -- subtler than a stale balance, because everything renders and only + * the fields are wrong. So the cache name carries the build: `main.js` registers this file as + * `sw.js?v=`, a different byte sequence for the browser to compare, which is what makes + * an upgrade trigger an update at all. `activate` then deletes every cache that is not this + * build's, so an old shell is gone rather than merely unused. + * + * ── WHAT THIS BUYS, IN ONE SENTENCE ───────────────────────────────────────────────────────── + * With `keel serve` stopped, opening the installed app shows the shell and its own banner saying + * keel is not running -- rather than the browser's dinosaur, or the server's 403 page, which is + * what the same click gets today. It never shows a figure. + */ + +/** + * This build's cache. Read from the registration URL's query string, which is the only channel a + * service worker has to its registrant that does not require the page to still be open. + * + * `"unknown"` is a real state, not a fallback nobody hits: a registration without `?v=` gets its + * own cache name and behaves correctly in every other respect. It is what a hand-typed + * registration in a console would produce, and it must not silently share a cache with a real + * build. + */ +const BUILD = new URL(self.location.href).searchParams.get("v") || "unknown"; +const CACHE = `keel-shell-${BUILD}`; + +/** The prefix this worker is allowed to touch, matching its own scope. */ +const BASE = "/static/"; + +/** The path prefix that is never cached, never stored, never served from a cache. */ +const API_PREFIX = "/api/"; + +/** The document every in-scope navigation resolves to -- `staticfiles.CLIENT_ENTRY`. */ +const SHELL = `${BASE}index.html`; + +/** + * Everything the app needs to paint with no network. + * + * A closed, hand-maintained list rather than a directory walk, because a service worker cannot + * walk a directory -- and `tests/web/test_service_worker.py` compares this list against the files + * actually present under `keel/web/static/`, so an asset added without a line here fails the + * build rather than producing an app that works until it is opened offline. + */ +const PRECACHE = [ + SHELL, + `${BASE}manifest.webmanifest`, + `${BASE}css/keel.css`, + `${BASE}js/api.js`, + `${BASE}js/chart.js`, + `${BASE}js/docs.js`, + `${BASE}js/format.js`, + `${BASE}js/live.js`, + `${BASE}js/main.js`, + `${BASE}js/render.js`, + `${BASE}icons/keel.svg`, + `${BASE}icons/keel-192.png`, + `${BASE}icons/keel-512.png`, + `${BASE}icons/keel-maskable-512.png`, +]; + +/** + * Install: fill this build's cache, then take over immediately. + * + * `skipWaiting` rather than waiting for every tab to close, and the version key is what makes + * that safe: the new worker serves the new build's cache, the old one is deleted in `activate`, + * and a client that reloads gets a consistent set. Waiting would leave an upgraded engine being + * read by the previous shell for as long as one tab stayed open -- exactly the failure the + * version key exists to prevent. + * + * `cache: "reload"` on every request: the server sends `Cache-Control: no-store` on static + * assets (`server._STATIC_BASE_HEADERS`), but the HTTP cache is not the only thing between here + * and the file, and an install that populated itself from a stale intermediate would bake the + * staleness in for the life of the build. + */ +self.addEventListener("install", (event) => { + event.waitUntil( + caches + .open(CACHE) + .then((cache) => cache.addAll(PRECACHE.map((path) => new Request(path, { cache: "reload" })))) + .then(() => self.skipWaiting()), + ); +}); + +/** + * Activate: delete every other keel cache, then claim open clients. + * + * Scoped to the `keel-shell-` prefix rather than deleting everything `caches.keys()` returns: + * this origin is keel's alone today, but a worker that deletes caches it did not create is a + * worker that will one day delete somebody else's. + */ +self.addEventListener("activate", (event) => { + event.waitUntil( + caches + .keys() + .then((names) => + Promise.all( + names + .filter((name) => name.startsWith("keel-shell-") && name !== CACHE) + .map((name) => caches.delete(name)), + ), + ) + .then(() => self.clients.claim()), + ); +}); + +/** + * Fetch: the shell and its assets from this build's cache; everything else untouched. + * + * "Untouched" means `respondWith` is never called -- the browser then performs the request + * exactly as it would with no worker installed, which is the correct behaviour for `/api/*` and + * for anything this worker has no opinion about. Calling `respondWith(fetch(request))` instead + * would be a same-behaviour-looking rewrite that quietly drops streaming and changes how a + * failed request reports itself. + */ +self.addEventListener("fetch", (event) => { + const request = event.request; + + // Only GET. A service worker sees no POST from this app -- `/setup/*` is a form on a rendered + // page outside this scope -- but responding from a cache to any non-GET is meaningless, and + // "meaningless" and "returns the wrong thing" are the same event here. + if (request.method !== "GET") return; + + const url = new URL(request.url); + if (url.origin !== self.location.origin) return; + + // Rule 3 (see the module comment): never anything under `/api/`. Unreachable while the scope is + // `/static/`, load-bearing the moment it is not. + if (url.pathname.startsWith(API_PREFIX)) return; + + if (!url.pathname.startsWith(BASE)) return; + + // A navigation is a request for a VIEW, not for a file: `/static/insights` names no asset, and + // the server answers it with the shell (`staticfiles.resolve_client_route`). Matching that here + // is what makes a deep link work with the engine stopped. + if (request.mode === "navigate") { + event.respondWith( + caches.match(SHELL, { cacheName: CACHE }).then((hit) => hit || fetch(request)), + ); + return; + } + + // `ignoreSearch`, because `main.js` registers `sw.js?v=...` and a cache-busting query on an + // asset would otherwise miss a file that is present and identical. + event.respondWith( + caches + .match(url.pathname, { cacheName: CACHE, ignoreSearch: true }) + .then((hit) => hit || fetch(request)), + ); +}); diff --git a/scripts/build_icons.py b/scripts/build_icons.py new file mode 100644 index 0000000..9cbeb72 --- /dev/null +++ b/scripts/build_icons.py @@ -0,0 +1,261 @@ +"""Generate keel's app icons (#538) from one geometry, with no dependencies. + +The PWA manifest needs raster icons, and a repository that commits PNGs without saying where +they came from has committed four files nobody can review or reproduce. So the mark is defined +ONCE below as plain geometry, and both the SVG and every PNG are emitted from it: +`scripts/build_icons.py --check` re-renders and compares bytes, which +`tests/web/test_icons.py` runs on every build. A hand-edited PNG fails; a changed shape has to +be changed here, where the change is readable in a diff. + +**Why hand-rolled rasterisation rather than Pillow.** keel's web surface ships zero JavaScript +dependencies on purpose (the design spec's §2), and the same argument applies with more force to +a build-time dependency that exists to draw three strokes: the whole rasteriser is a +point-in-polygon test and a `zlib.compress`, both stdlib, and the output is deterministic across +platforms in a way "whatever Pillow does with anti-aliasing this release" is not. Byte-identical +output is what makes `--check` a test rather than a suggestion. + +Release tooling: deliberately NOT shipped in the wheel. +""" + +from __future__ import annotations + +import argparse +import struct +import zlib +from math import hypot +from pathlib import Path + +#: Where the generated icons land, inside the served static tree. +ICON_DIR = Path(__file__).resolve().parent.parent / "keel" / "web" / "static" / "icons" + +#: The two brand colours, taken from `keel/web/static/css/keel.css`'s light palette so the +#: installed app's tile matches the page it opens. `--accent` (#1a5578) rather than `--fg`: the +#: mark has to survive being shrunk to a 16px favicon and sitting on an unknown desktop +#: background, and a near-black square is indistinguishable from every other near-black square. +BACKGROUND = (0x1A, 0x55, 0x78, 0xFF) +FOREGROUND = (0xFB, 0xFA, 0xF8, 0xFF) + + +def _stroke( + start: tuple[float, float], end: tuple[float, float], width: float +) -> tuple[tuple[float, float], ...]: + """A straight stroke of `width` as a four-point polygon, with butt caps. + + The mark below is three strokes, so it is written as three strokes rather than as twelve + hand-computed corners: a letterform whose geometry is spelled out corner by corner is one + nobody can adjust later without re-deriving the perpendiculars by hand, and every adjustment + to a monogram is a nudge. + """ + dx, dy = end[0] - start[0], end[1] - start[1] + length = hypot(dx, dy) + # A zero-length stroke has no perpendicular. It is a caller error rather than a shape, and + # returning an empty polygon here would silently drop a limb of the letter instead. + if length == 0: + raise ValueError("a stroke needs two distinct points") + nx, ny = -dy / length * width / 2, dx / length * width / 2 + return ( + (start[0] + nx, start[1] + ny), + (end[0] + nx, end[1] + ny), + (end[0] - nx, end[1] - ny), + (start[0] - nx, start[1] - ny), + ) + + +#: The mark: a lowercase `k`, in a 0..1 unit square with y increasing downward. +#: +#: **A monogram rather than a picture of a keel, and that was decided by looking.** Three +#: nautical marks were drawn and rendered first -- a hull in section over a keel, a hull in +#: profile with a fin, and a bulb keel -- and each one read as something else at icon size: a +#: funnel, a letter T, and an exclamation mark on a saucer. A launcher tile is 32-48 CSS pixels +#: on a background nobody chose, and at that size a silhouette gets one reading, which is not +#: necessarily the one it was drawn with. The name is unambiguous at every size, and this +#: application's whole argument is that a surface should say plainly what it is. +#: +#: Every coordinate is a fraction, so the SAME numbers render at 16px and at 512px. Sizes are +#: not special-cased and there is no hinting: a mark that needs different geometry at small +#: sizes is a mark with too much in it, and the fix is fewer shapes rather than more code. +_STEM = _stroke((0.305, 0.115), (0.305, 0.885), 0.150) +#: The arm and the leg meet at ONE point, and that point is INSIDE the stem rather than on its +#: right edge: butt caps that meet on the edge leave a small wedge of background at the +#: junction -- visible at 512px, and at 32px it reads as a broken letter rather than as a nick. +#: Ending both strokes inside the stem lets the stem cover the joint. +_JUNCTION = (0.335, 0.600) +_ARM = _stroke((0.755, 0.300), _JUNCTION, 0.140) +_LEG = _stroke(_JUNCTION, (0.775, 0.885), 0.140) + +SHAPES: tuple[tuple[tuple[float, float], ...], ...] = (_STEM, _ARM, _LEG) + +#: The safe-area inset a maskable icon is judged against. Android may crop a maskable icon to +#: any shape inside the middle 80% -- a circle, a squircle, a rounded square -- so the mark is +#: scaled to sit inside that circle rather than merely inside the square. Getting this wrong is +#: invisible on the developer's own launcher and clips the icon on somebody else's. +MASKABLE_SCALE = 0.72 + +#: Samples per axis inside each pixel. 3 means nine coverage tests per pixel, which is enough to +#: keep the letter's diagonals from stepping visibly at 192px and cheap enough that all four +#: icons render in well under a second. +_SUPERSAMPLE = 3 + +#: What gets written, and what `--check` compares against. `any` and `maskable` are separate +#: files rather than one file declared as both: a maskable icon has 20% padding by construction, +#: so declaring it `any` too puts a small mark in a big box everywhere the safe area is not +#: cropped -- the commonest way an install looks slightly wrong for no visible reason. +TARGETS: tuple[tuple[str, int, bool], ...] = ( + ("keel-192.png", 192, False), + ("keel-512.png", 512, False), + ("keel-maskable-512.png", 512, True), +) + +SVG_NAME = "keel.svg" + + +def _inside(polygon: tuple[tuple[float, float], ...], x: float, y: float) -> bool: + """Even-odd point-in-polygon. Ray-casts to the right and counts crossings. + + The `!=` on the two comparisons is what makes a vertex on the ray count once rather than + twice or zero times -- the classic crossing-number test, kept verbatim rather than + "simplified", because every simplification of it drops a boundary case. + """ + inside = False + count = len(polygon) + for index in range(count): + x0, y0 = polygon[index] + x1, y1 = polygon[(index - 1) % count] + if (y0 > y) != (y1 > y) and x < (x1 - x0) * (y - y0) / (y1 - y0) + x0: + inside = not inside + return inside + + +def _blend(coverage: float) -> tuple[int, int, int, int]: + """Foreground over background at `coverage`, rounded half-up. + + Composited here rather than left as a transparent foreground over a transparent background: + the icon is opaque by design (a manifest icon with alpha gets an arbitrary backdrop from + whatever is behind it), so every pixel is a straight mix of two known colours. + """ + return tuple( # type: ignore[return-value] + int(back + (fore - back) * coverage + 0.5) + for back, fore in zip(BACKGROUND, FOREGROUND, strict=True) + ) + + +def render(size: int, *, maskable: bool) -> bytes: + """One icon as PNG bytes.""" + scale = MASKABLE_SCALE if maskable else 1.0 + offset = (1.0 - scale) / 2.0 + shapes = tuple( + tuple((x * scale + offset, y * scale + offset) for x, y in shape) for shape in SHAPES + ) + + rows: list[bytes] = [] + step = 1.0 / (size * _SUPERSAMPLE) + for row in range(size): + pixels = bytearray() + for column in range(size): + hits = 0 + for sub_y in range(_SUPERSAMPLE): + y = (row * _SUPERSAMPLE + sub_y + 0.5) * step + for sub_x in range(_SUPERSAMPLE): + x = (column * _SUPERSAMPLE + sub_x + 0.5) * step + if any(_inside(shape, x, y) for shape in shapes): + hits += 1 + pixels.extend(_blend(hits / (_SUPERSAMPLE * _SUPERSAMPLE))) + rows.append(bytes(pixels)) + return _png(size, rows) + + +def _png(size: int, rows: list[bytes]) -> bytes: + """RGBA8 PNG, filter 0 on every scanline. + + No filtering (`0` = None) rather than the adaptive heuristic a full encoder uses: these are + flat-colour images where filtering buys a few hundred bytes, and a fixed filter is one fewer + thing for `--check` to have to reproduce identically. + """ + raw = b"".join(b"\x00" + row for row in rows) + header = struct.pack(">IIBBBBB", size, size, 8, 6, 0, 0, 0) + return b"".join( + ( + b"\x89PNG\r\n\x1a\n", + _chunk(b"IHDR", header), + _chunk(b"IDAT", zlib.compress(raw, 9)), + _chunk(b"IEND", b""), + ) + ) + + +def _chunk(kind: bytes, payload: bytes) -> bytes: + return b"".join( + ( + struct.pack(">I", len(payload)), + kind, + payload, + struct.pack(">I", zlib.crc32(kind + payload) & 0xFFFFFFFF), + ) + ) + + +def render_svg() -> bytes: + """The same mark as SVG, for the manifest's `any` entry and anywhere a vector is better. + + Emitted from `SHAPES` rather than hand-written beside it, so the vector and the rasters + cannot drift: a change to the hull that forgot the SVG would otherwise ship an icon that + disagrees with itself depending on which size the launcher picked. + + `viewBox="0 0 1 1"` lets the unit coordinates go in verbatim. No `' @@ -70,20 +69,13 @@ def test_a_failed_adapter_row_shows_the_error_and_not_the_placeholders() -> None assert "WIRED" not in html -def test_a_fiqh_term_that_fiqh_basis_does_not_state_says_so() -> None: - """`stated=False` means fiqh-basis does not define the term and the glossary entry says that - rather than substituting a help-authored summary. Losing that marker in a new front-end would - turn a disclaimed gap into an apparent citation.""" - term = GlossaryTerm( - term="something", - definition="fiqh-basis does not state this.", - source="", - citation=None, - fiqh=True, - stated=False, - ) - html = render.render_glossary([term]) - assert "not stated in fiqh-basis" in html +# `test_a_fiqh_term_that_fiqh_basis_does_not_state_says_so` lived here and went with +# `render_glossary` at #539. The property it protected did NOT go: the "not stated" disclaimer is +# written into the definition text in `docs/glossary.md` itself -- which is why +# `help_console.parse_glossary` can DERIVE `stated` from it (`stated = "not stated" not in +# source.lower()`), and why `tests/commands/test_help_console.py` asserts it on the gharar entry. +# A reader following the deep link lands on that prose. What was deleted is a renderer for a file +# no installed deployment has ever had. def test_utc_is_used_and_a_broken_timestamp_does_not_raise() -> None: diff --git a/tests/web/test_server.py b/tests/web/test_server.py index fa2051d..714f1a7 100644 --- a/tests/web/test_server.py +++ b/tests/web/test_server.py @@ -35,7 +35,6 @@ "/rules", "/venues", "/gates", - "/glossary", ) @@ -969,10 +968,19 @@ def test_the_printed_url_is_the_one_that_carries_the_token( def test_the_nav_and_the_routing_table_agree() -> None: """A page with no nav entry is unreachable; a nav entry with no page is a 404 the user is - invited to click. Neither is caught by testing either side alone.""" + invited to click. Neither is caught by testing either side alone. + + **The nav has one entry that is deliberately not a route (#539).** `Docs` links out to + keeltrading.com, because `docs/` has never shipped inside a wheel and the page that used to + render it was empty in every installed deployment. It is separated here by its scheme rather + than by its label, so a second outbound entry needs no edit and an internal entry that loses + its route still fails.""" from keel.web import render - assert {href for href, _label in render.NAV} == set(web_server.ROUTES) + internal = {href for href, _label in render.NAV if not href.startswith("https://")} + outbound = {href for href, _label in render.NAV if href.startswith("https://")} + assert internal == set(web_server.ROUTES) + assert outbound == {render.DOCS_URL}, "an unexpected outbound nav entry" assert set(ROUTES) == set(web_server.ROUTES), "this test module's list drifted from the server"