diff --git a/CLAUDE.md b/CLAUDE.md index 9ccbfe6..c5dfe29 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -109,7 +109,7 @@ Milestone M2 of `docs/design/fleet.md`; the operator flow is `docs/fleet/README. ## Fleet: the operator view + dispatch API (M3) -Milestone M3 of `docs/design/fleet.md`, and the end of v0. The **HTTP** wire is specified as its own versioned contract in **`docs/fleet/fleet-api.md`** (M1's MQTT one is `control-plane.md`); the operator flow is `docs/fleet/README.md` §6–9 and the measurements are `m3-verification.md`. **The two directions of the loop take different paths on purpose.** *Reads* ride MQTT: the browser subscribes to `mote/v1/+/{presence,health,pose,task/status}` over WebSockets, and because all of those are retained it has the whole fleet's state within a second of loading — no polling, no service in the middle. *Writes* ride HTTP: `POST /v1/robots//dispatch` authorizes an operator token (`fleetctl operator new --name `; the name is what the audit row records), writes the audit row, then publishes to the same `task/command` topic. **The topic tree did not change — only who publishes to it**, and `fleetctl dispatch` moved to the API too, so there is one write path rather than one per client. The command grammar is still parsed only by the robot's task layer: a parser in the server would be a second grammar to keep in step. **The browser cannot publish**: `server/ui/mqtt.mjs` is a hand-rolled subscribe-only MQTT 3.1.1 client that implements no PUBLISH packet, so the split is enforced by omission (M7 makes it structural with a subscribe-only broker credential). The UI is static ES modules — no bundler, no npm, no vendored library — served by the same stdlib `http.server`; `map.mjs` holds the Q5 world→pixel transform (`px = (wx-origin_x)/res`, `py = height - (wy-origin_y)/res`) and a pan/zoom/follow canvas, and only draws robots on the *same* site+floor as the selected one because a pose from another floor is a different map frame. **Basemaps come from site bundles on the fleet box** (`--maps-dir`, default `$MOTE_FLEET_HOME/sites`, the layout `sites.py` writes, seeded by rsync until **M4** makes the registry canonical behind the same two routes). **M1's websockets blocker is settled**: `pixi run fleet-broker` runs `eclipse-mosquitto` under docker with the repo's own `mosquitto.conf`, because conda-forge's build has none; `pixi run -e fleet fleet-broker-local` is the conda binary for a box without docker, and it strips the WS stanza and says so. Two things that run in the same file (`test_ui.py` → `ui_test.mjs`) are the MQTT codec and the transform, tested under node against the very files the browser loads; `browser_check.mjs` drives a real headless Chrome over CDP against a running stack and is an operator's tool, not a CI test — `pixi run fleet-ui-check` is that stack in one command (broker on ephemeral ports, server, a temp `MOTE_FLEET_HOME`, the sim's `office_world` bundle as the basemap, and `test/fake_robots.py`, which publishes `protocol.py` payloads and answers `task/command` and is *not* a second robot implementation), torn down afterwards; `-- --keep` leaves it up for UI work. It stays out of CI because it needs docker (conda's mosquitto still has no websockets) *and* a chrome, which the arm runner has not — the decision, and what wiring it in would take, are recorded in `m3-verification.md` §2 rather than left looking like coverage. **The phone is the realistic off-LAN client**, so below 760 px the three panes become one at a time behind a bottom tab bar (`server/ui/layout.mjs`), selecting a robot in the roster navigates to the map — what the desktop layout gets for free by showing both — and the canvas gained pinch-to-zoom (`pinchSpan`/`pinchUpdate` in `map.mjs`, pure and tested, because a division by a zero span puts NaN in the view scale and blanks the map for good) plus a fingertip-sized hit target. The breakpoint is a **silent** seam — CSS decides what is displayed, JS decides when a selection navigates, and disagreement yields a tab bar over stacked panes rather than an error — so it lives in `layout.mjs` and `ui_test.mjs` reads the stylesheet and holds it there, as it does for every pane having a tab and for `touch-action: none` on the canvas (without which the browser eats the drag and the pinch before a single pointer event arrives). Dispatch gained a **zone picker that writes `goto ` into the command box rather than sending it**: the grammar stays the robot's, and the keyboard leaves the common case. Three pre-existing bugs fell out, all of which a desk hides: `hidden` does not hide an element whose class sets `display` (the empty promote picker), the canvas backing store was resized on width alone so a height change left the previous frame's scale bar under the new one, and the scale bar was drawn in the dark theme's near-white on a white basemap — a canvas gets no cascade, so it now reads `--dim` off the element. Measurements, including `browser_check.mjs`'s phone pass, are `m3-verification.md` §9; **a real device is still the acceptance** — emulation gets the viewport and the touch points right and the thumb wrong. +Milestone M3 of `docs/design/fleet.md`, and the end of v0. The **HTTP** wire is specified as its own versioned contract in **`docs/fleet/fleet-api.md`** (M1's MQTT one is `control-plane.md`); the operator flow is `docs/fleet/README.md` §6–9 and the measurements are `m3-verification.md`. **The two directions of the loop take different paths on purpose.** *Reads* ride MQTT: the browser subscribes to `mote/v1/+/{presence,health,pose,task/status}` over WebSockets, and because all of those are retained it has the whole fleet's state within a second of loading — no polling, no service in the middle. *Writes* ride HTTP: `POST /v1/robots//dispatch` authorizes an operator token (`fleetctl operator new --name `; the name is what the audit row records), writes the audit row, then publishes to the same `task/command` topic. **The topic tree did not change — only who publishes to it**, and `fleetctl dispatch` moved to the API too, so there is one write path rather than one per client. The command grammar is still parsed only by the robot's task layer: a parser in the server would be a second grammar to keep in step. **The browser cannot publish**: `server/ui/mqtt.mjs` is a hand-rolled subscribe-only MQTT 3.1.1 client that implements no PUBLISH packet, so the split is enforced by omission (M7 makes it structural with a subscribe-only broker credential). The UI is static ES modules — no bundler, no npm, no vendored library — served by the same stdlib `http.server`; `map.mjs` holds the Q5 world→pixel transform (`px = (wx-origin_x)/res`, `py = height - (wy-origin_y)/res`) and a pan/zoom/follow canvas, and only draws robots on the *same* site+floor as the selected one because a pose from another floor is a different map frame. **Basemaps come from site bundles on the fleet box** (`--maps-dir`, default `$MOTE_FLEET_HOME/sites`, the layout `sites.py` writes, seeded by rsync until **M4** makes the registry canonical behind the same two routes). **M1's websockets blocker is settled**: `pixi run fleet-broker` runs `eclipse-mosquitto` under docker with the repo's own `mosquitto.conf`, because conda-forge's build has none; `pixi run -e fleet fleet-broker-local` is the conda binary for a box without docker, and it strips the WS stanza and says so. Two things that run in the same file (`test_ui.py` → `ui_test.mjs`) are the MQTT codec and the transform, tested under node against the very files the browser loads; `browser_check.mjs` drives a real headless Chrome over CDP against a running stack and is an operator's tool, not a CI test — `pixi run fleet-ui-check` is that stack in one command (broker on ephemeral ports, server, a temp `MOTE_FLEET_HOME`, the sim's `office_world` bundle as the basemap, and `test/fake_robots.py`, which publishes `protocol.py` payloads and answers `task/command` and is *not* a second robot implementation), torn down afterwards; `-- --keep` leaves it up for UI work. It stays out of CI because it needs docker (conda's mosquitto still has no websockets) *and* a chrome, which the arm runner has not — the decision, and what wiring it in would take, are recorded in `m3-verification.md` §2 rather than left looking like coverage. **A fourth pane, `review`, is where a candidate map is looked at and promoted** (`server/ui/review.mjs`; routes and rationale under the map registry below). It is a *mode*, not a column: opening it stands the operations panes down at every width, because two canvases — one canonical with robots on it, one a candidate without — is the confusion a dedicated view exists to remove. **The phone is the realistic off-LAN client**, so below 760 px the panes become one at a time behind a bottom tab bar (`server/ui/layout.mjs`), selecting a robot in the roster navigates to the map — what the desktop layout gets for free by showing both — and the canvas gained pinch-to-zoom (`pinchSpan`/`pinchUpdate` in `map.mjs`, pure and tested, because a division by a zero span puts NaN in the view scale and blanks the map for good) plus a fingertip-sized hit target. The breakpoint is a **silent** seam — CSS decides what is displayed, JS decides when a selection navigates, and disagreement yields a tab bar over stacked panes rather than an error — so it lives in `layout.mjs` and `ui_test.mjs` reads the stylesheet and holds it there, as it does for every pane having a tab and for `touch-action: none` on the canvas (without which the browser eats the drag and the pinch before a single pointer event arrives). Dispatch gained a **zone picker that writes `goto ` into the command box rather than sending it**: the grammar stays the robot's, and the keyboard leaves the common case. Three pre-existing bugs fell out, all of which a desk hides: `hidden` does not hide an element whose class sets `display` (the empty promote picker), the canvas backing store was resized on width alone so a height change left the previous frame's scale bar under the new one, and the scale bar was drawn in the dark theme's near-white on a white basemap — a canvas gets no cascade, so it now reads `--dim` off the element. Measurements, including `browser_check.mjs`'s phone pass, are `m3-verification.md` §9; **a real device is still the acceptance** — emulation gets the viewport and the touch points right and the thumb wrong. ## Fleet: the map registry (M4) @@ -163,6 +163,27 @@ its map at startup, so health now carries the revision each robot is actually running. M3's `/v1/maps` routes kept their shape and changed source; the dashboard additionally draws the floor's taught zones (circle, polygon or waypoint cross) from `/v1/maps///zones.json`. +**Promotion is a decision, so the operator has to be able to see what they are +deciding about** — and until the review pane existed the only thing on screen +was a timestamp, the canvas beside the picker being always the *canonical* +basemap. Three GETs answer it, the same three questions `/v1/maps` answers asked +of a revision that is not canonical: `…/revisions//{map.json,map.png, +zones.json}`. Three things are load-bearing. `read_map`'s **`image_url` is +revision-aware**, because a transform from the revision and pixels from +`/v1/maps` would draw the published map under the candidate's label — convincing +and wrong, which is the exact failure being removed. The zones read is +**revision-scoped and deliberately not gated on a published map** where +`read_zones` is (the review that matters most is the first candidate on a floor +with nothing published), which does not loosen the vocabulary/binding split: +naming a revision is naming a map frame, and these stay under `/v1/maps`-shaped +paths and never under `/v1/zones`. And it reports **`source: revision|floor`**, +because `_zones_file` falls back to the floor's `zones.yaml` and inherited zones +were taught in a *previous* session's frame — they draw perfectly over the new +map and are wrong by however far the two origins differ, which no coordinate can +say. One UI ordering bug fell out and is fixed in both panes: the floor's +revisions were fetched only *after* its basemap loaded, behind an early return, +so a floor whose only revisions were candidates listed none of them and **the +first promotion on any floor could never be made from a browser**. ## Fleet: the zone vocabulary diff --git a/docs/fleet/README.md b/docs/fleet/README.md index 69987ac..516ae34 100644 --- a/docs/fleet/README.md +++ b/docs/fleet/README.md @@ -623,8 +623,10 @@ heartbeats stopped. `fleet-server` serves the operator view at `http://:8080/`. It is the fleet-wide picture — who is out there, where they are, what they are doing, and -sending one of them somewhere — and nothing else: the deep single-robot view -(3D, sensors, teleop) is Foxglove's job (M2), which each robot row deep-links to. +sending one of them somewhere — plus the one decision the fleet cannot make for +you: which map a floor should be on (the **review** pane, §11). The deep +single-robot view (3D, sensors, teleop) is Foxglove's job (M2), which each robot +row deep-links to. ![The fleet dashboard](../images/fleet-ui.webp) @@ -662,14 +664,17 @@ outline for a `polygon`, a cross for a bare waypoint, each labelled — so the canonical revision, in that revision's map frame. Beside the map's floor label is the **canonical revision** it is showing, and, -when a robot has published one, a picker to promote a candidate onto the floor -(§11). Both need the operator token; without one the pane is read-only. +when the floor has candidates waiting, a button into the **review** pane — +which is where a candidate is looked at and promoted (§11). The map pane keeps +no promote control of its own: this canvas draws robots on the *published* +basemap, so promoting from beside it would mean promoting a map you have not +seen. ![The dashboard on a phone](../images/fleet-ui-phone.webp) **On a phone.** The realistic off-LAN client is a phone — it is what an operator has in a corridor, and "where is the robot and what is it doing" is exactly the -question you ask from one. Below 760 px the three panes become **one at a time** +question you ask from one. Below 760 px the four panes become **one at a time** behind a tab bar at the bottom of the screen, within thumb reach, so the map gets the whole display instead of a couple of hundred pixels between the roster and the detail pane. Two things follow from losing the side-by-side view: @@ -862,7 +867,48 @@ pixi run -e fleet fleetctl -- promote home ground 20260728T090412 # announced on mote/v1/registry/site/home/floor/ground/current (retained); agents will pull it ``` -The dashboard does the same thing with a picker beside the map (§9). +### Reviewing one before you promote it + +`fleetctl sites ` tells you a revision is *valid*. It cannot tell +you whether it is the map you want, and for a long time neither could the +dashboard: the promote picker listed candidates as timestamps and the canvas +beside it was always the published basemap, so a promotion was an act of faith +in a filename. The dashboard's **review** pane is where that decision is now +made. + +Open it from the tab bar, or from the map pane's `N candidates — review` button, +which appears whenever the floor on screen has something waiting. It shows: + +- **A site/floor picker of its own**, fed by the registry rather than by which + robot is selected. The floor worth reviewing is often one no robot is + reporting — mapped by a robot since switched off, or side-loaded. +- **Every revision of that floor**, newest first, the published one included so + you can see what you would be replacing. A revision the validator refused is + listed too, with its reason, because "why can I not promote the map my robot + just published" is a question this pane should answer. +- **The candidate's own map**, drawn from that revision's own image — not the + published one — with its own zones over it. Switching between two candidates + keeps the viewport, which is how you compare them. +- **Why it is promotable**: the validator's verdict and warnings, plus where the + revision came from, when it was mapped, its size and resolution, the free/ + occupied/unknown split, whether it carries a posegraph (i.e. whether mapping + can be continued in this frame), its bytes and digest. +- **The zones in it**, and — the part that is easy to miss — whether they are + the revision's own or **inherited from the floor**. Inherited zones were + taught in a previous session's frame: they draw perfectly over the new map and + are wrong by however far the two origins differ. The pane says so in words, + because the canvas cannot. +- **The promote button**, which is the same audited flip `fleetctl promote` + makes. + +Everything except that button is a read. Nothing you do here changes any floor +until you promote. + +**A floor with nothing published yet works the same way** — which was not always +true: the dashboard used to fetch a floor's revisions only after its basemap had +loaded, so a floor whose only revisions were candidates listed none of them and +its first promotion could not be made in a browser at all. Reviewing and +promoting the first map on a floor is now the ordinary path. ### What the robots then do diff --git a/docs/fleet/fleet-api.md b/docs/fleet/fleet-api.md index 4a24fef..fb42e4e 100644 --- a/docs/fleet/fleet-api.md +++ b/docs/fleet/fleet-api.md @@ -97,6 +97,9 @@ GET /v1/sites the registry: every floor + its canonic GET /v1/sites//floors/ every revision, validated, with provenance POST …/revisions/ upload a candidate revision (a robot) GET …/revisions//bundle.tar.gz pull a revision (a robot) +GET …/revisions//map.json that revision's own transform + size +GET …/revisions//map.png that revision's own image +GET …/revisions//zones.json that revision's own zone binding POST …/revisions//promote make it canonical (an operator) GET / the operator UI (static files) ``` @@ -451,6 +454,51 @@ a temporary directory, renames it into `maps//`, and flips the local `map` symlink — so a half-transferred revision is never visible and nothing has to be undone if the transfer dies. +### `GET …/revisions//map.json`, `…/map.png`, `…/zones.json` + +The same three questions `/v1/maps///…` answers, asked of a +revision that is **not** the floor's canonical one. Uploading is not publishing, +so an operator has a decision to make; these are what lets them see what they +are deciding about, and they are what the dashboard's review pane reads. + +`map.json` is `/v1/maps`' payload with `revision` naming this revision and +`image_url` pointing at *this* route's `map.png`: + +```json +{"schema":1,"site":"home","floor":"ground","revision":"20260802T145731", + "resolution":0.05,"origin":[-2.927,-2.934,0.0],"width":438,"height":238, + "image":"map.png", + "image_url":"/v1/sites/home/floors/ground/revisions/20260802T145731/map.png"} +``` + +That URL is load-bearing. `/v1/maps///map.png` serves whatever is +*published*, so a client that took the transform from here and the pixels from +there would draw the map the operator already has under the candidate's label — +which looks entirely convincing and is the exact failure this route removes. + +`zones.json` carries one extra field over the canonical route: + +```json +{"schema":1,"site":"home","floor":"ground","revision":"20260802T145731", + "source":"floor","frame_id":"map","zones":[…]} +``` + +`source` is `revision` when the revision carries its own `zones.yaml` and +`floor` when it inherits the floor's. The difference matters and the coordinates +cannot express it: inherited zones were taught in a *previous* SLAM session's +frame, so they draw perfectly over this map and are wrong by however far the two +origins differ. + +Unlike `read_zones` on the canonical route, this is **not gated on there being a +published map** — the review that matters most is the first candidate on a floor +with nothing published at all. That does not loosen the vocabulary/binding +split: these are still coordinates, still served under a path bound to a +basemap, and still never over `/v1/zones`. Naming a revision is naming a map +frame. + +All three are reads, so like every other read route they take no operator token; +M7 changes that for all of them at once. + --- ## What the browser is allowed to do @@ -461,3 +509,9 @@ listener with a client that implements no PUBLISH packet ([`ui/mqtt.mjs`](../../mote_fleet/server/ui/mqtt.mjs)) — the split is enforced by omission, not by intention. M7 makes that structural on the broker side too, with a subscribe-only credential. + +**Reviewing a candidate is all GETs.** The review pane reads a revision's +`map.json`, `map.png` and `zones.json` and writes nothing; its one write is the +`promote` M4 already had, which is authorized and audited like any other. Zone +*editing* is a separate write (`POST …/floors//zones`) that derives a new +candidate and still changes no published floor. diff --git a/mote_bringup/mote_bringup/bundle.py b/mote_bringup/mote_bringup/bundle.py index 5020246..afb4a25 100644 --- a/mote_bringup/mote_bringup/bundle.py +++ b/mote_bringup/mote_bringup/bundle.py @@ -651,11 +651,15 @@ def validate(revision_dir, *, require_posegraph: bool = True) -> Report: elif not report.files[name]: report.errors.append(f"{name} is empty") - for name in CONTINUABLE: - if report.files.get(name): - continue + # One message however many of them are absent. slam_toolbox writes the + # posegraph and its data as a pair, either half missing means exactly the + # same thing, and a line per file produced two entries with word-for-word + # identical text — which reads as two separate problems. + missing = [name for name in CONTINUABLE if not report.files.get(name)] + if missing: message = ( - f"{name} is missing — mapping cannot be continued in this frame " + f"{' and '.join(missing)} {'is' if len(missing) == 1 else 'are'} " + "missing — mapping cannot be continued in this frame " "(extend, don't remap)" ) (report.errors if require_posegraph else report.warnings).append(message) diff --git a/mote_bringup/test/test_bundle.py b/mote_bringup/test/test_bundle.py index 3c895e2..51d8afe 100644 --- a/mote_bringup/test/test_bundle.py +++ b/mote_bringup/test/test_bundle.py @@ -374,7 +374,23 @@ def test_a_missing_posegraph_is_an_error_for_a_publisher_and_a_warning_for_a_rea assert not bundle.validate(directory).ok lenient = bundle.validate(directory, require_posegraph=False) assert lenient.ok - assert any("map.posegraph" in warning for warning in lenient.warnings) + # One entry naming both halves, not one per file. slam_toolbox writes the + # posegraph and its data as a pair, so a line each produced two warnings + # with word-for-word identical text — two problems, to anyone reading them. + assert lenient.warnings == [ + "map.posegraph and map.data are missing — mapping cannot be continued " + "in this frame (extend, don't remap)" + ] + + +def test_half_a_posegraph_names_only_the_half_that_is_missing(tmp_path): + directory = revision(tmp_path / "rev") + (directory / "map.data").unlink() + report = bundle.validate(directory, require_posegraph=False) + assert report.warnings == [ + "map.data is missing — mapping cannot be continued in this frame " + "(extend, don't remap)" + ] def test_a_truncated_upload_is_caught(tmp_path): diff --git a/mote_fleet/server/bundle_store.py b/mote_fleet/server/bundle_store.py index f883145..eecd31b 100644 --- a/mote_fleet/server/bundle_store.py +++ b/mote_fleet/server/bundle_store.py @@ -232,8 +232,11 @@ def _report(self, directory: Path, upload: dict) -> dict: pass # a read-only store still answers, just without the cache return report.as_dict() + def revision_url(self, site: str, floor: str, revision: str, leaf: str) -> str: + return f"/v1/sites/{site}/floors/{floor}/revisions/{revision}/{leaf}" + def bundle_url(self, site: str, floor: str, revision: str) -> str: - return f"/v1/sites/{site}/floors/{floor}/revisions/{revision}/bundle.tar.gz" + return self.revision_url(site, floor, revision, "bundle.tar.gz") def read_map(self, site: str, floor: str, revision: str | None = None) -> dict: """A floor's map metadata for the world→pixel transform (Q5). @@ -243,6 +246,14 @@ def read_map(self, site: str, floor: str, revision: str | None = None) -> dict: revision on upload and the same the robot writes against — M3 shipped a hand-rolled reader here with a note that M4 would replace it, and this is that. + + ``image_url`` names the route the caller must fetch to get *these* + pixels, which is not the same route in both cases: the canonical map is + served under ``/v1/maps`` and a named revision under its own path. A + payload describing a candidate's transform while pointing at the + canonical image is the one failure the review view exists to remove — + the operator would be shown the map they already have, labelled as the + one they are about to promote. """ directory = self.revision_dir(site, floor, revision or self._live(site, floor)) meta = bundle.read_map(directory / bundle.MAP_YAML) @@ -259,7 +270,11 @@ def read_map(self, site: str, floor: str, revision: str | None = None) -> dict: site=site, floor=floor, revision=self.canonical(site, floor) if revision is None else revision, - image_url=f"/v1/maps/{site}/{floor}/map.png", + image_url=( + f"/v1/maps/{site}/{floor}/map.png" + if revision is None + else self.revision_url(site, floor, revision, "map.png") + ), ) meta["_image_path"] = str(image) return meta @@ -291,6 +306,38 @@ def read_zones(self, site: str, floor: str) -> dict: "zones": list(zones["zones"].values()), } + def read_revision_zones(self, site: str, floor: str, revision: str) -> dict: + """One **revision's** zones, for the operator reviewing it. + + Still a binding, and still served the way a binding must be: under the + revision's own path, beside the revision's own basemap, never over + ``/v1/zones``. What it drops is :meth:`read_zones`' gate on there being + a published map — which is the one thing that would make it useless + here, because the review that matters most is the *first* candidate on + a floor with nothing published at all. Naming a revision is naming a + map frame, so the coordinates still arrive with something to mean. + """ + # An unknown revision is a 404 about the revision rather than about the + # zones, and the floor-level fallback below must not answer for one. + directory = self.revision_dir(site, floor, revision) + path = self._zones_file(site, floor, revision=revision) + if path is None: + raise StoreError(f"no zones for {site}/{floor}/{revision}", 404) + zones = bundle.read_zones(path) + return { + "site": site, + "floor": floor, + "revision": revision, + # Which of ``_zones_file``'s two candidates answered. The reviewer + # cannot tell from the coordinates, and the difference is the trap + # M4 named: a revision carrying no zones inherits the floor's, which + # were taught in a *previous* SLAM session's frame and are therefore + # wrong for this map by exactly however far the two origins differ. + "source": "revision" if path.parent == directory else "floor", + "frame_id": zones["frame_id"], + "zones": list(zones["zones"].values()), + } + def read_vocabulary(self, site: str, floor: str) -> dict: """The floor's zone **vocabulary** — names, kinds and aliases, no coordinates and no frame. diff --git a/mote_fleet/server/fleet_server.py b/mote_fleet/server/fleet_server.py index 2705b19..005a01a 100644 --- a/mote_fleet/server/fleet_server.py +++ b/mote_fleet/server/fleet_server.py @@ -33,6 +33,9 @@ GET /v1/sites//floors/ revisions, validated, with provenance POST .../revisions/ upload a candidate revision (robot) GET .../revisions//bundle.tar.gz pull one (robot) + GET .../revisions//map.json that revision's own Q5 transform + GET .../revisions//map.png that revision's own image + GET .../revisions//zones.json that revision's own zone binding POST .../revisions//promote make it canonical (operator) POST /v1/sites//floors//zones edited zones -> new candidate (operator) GET / the operator UI (static files) @@ -132,10 +135,13 @@ # test without a package.json declaring the tree a module. mimetypes.add_type("text/javascript", ".mjs") -#: Route shape for the registry's per-revision paths. +#: Route shape for the registry's per-revision paths. The three review leaves +#: mirror ``/v1/maps///…`` exactly, because they answer the same +#: three questions about a revision that is *not* the floor's canonical one — +#: which is what an operator has to see before promoting it. REVISION_RE = re.compile( r"^(?P[^/]+)/floors/(?P[^/]+)/revisions/(?P[^/]+)" - r"(?P/promote|/bundle\.tar\.gz)?$" + r"(?P/promote|/bundle\.tar\.gz|/map\.json|/map\.png|/zones\.json)?$" ) @@ -532,8 +538,17 @@ def _map(self, rest: str): if leaf == "zones.json": self._store(lambda store: store.read_zones(site, floor)) return + self._send_map(leaf, lambda store: store.read_map(site, floor)) + + def _send_map(self, leaf: str, load): + """``map.json`` or ``map.png`` from one ``read_map`` call. + + Shared by the canonical basemap and by a revision under review, so the + transform and the pixels a client is handed can never come from + different reads of the store. + """ try: - meta = self.server.store.read_map(site, floor) + meta = load(self.server.store) except StoreError as exc: self._error(exc.code, str(exc)) return @@ -609,13 +624,22 @@ def _registry_get(self, rest: str): self._store(lambda store: store.detail(site, floor)) return match = REVISION_RE.match(rest) - if not match or match.group("leaf") != "/bundle.tar.gz": + # `/promote` is a POST, and a bare revision path has nothing to answer: + # both are 404 here rather than falling through to the bundle. + if not match or match.group("leaf") in (None, "/promote"): self._error(404, f"no route /v1/sites/{rest}") return site, floor = match.group("site"), match.group("floor") revision = match.group("revision") if not self._names(site, floor, revision): return + leaf = match.group("leaf").lstrip("/") + if leaf == "zones.json": + self._store(lambda store: store.read_revision_zones(site, floor, revision)) + return + if leaf in ("map.json", "map.png"): + self._send_map(leaf, lambda store: store.read_map(site, floor, revision)) + return try: blob = self.server.store.pack(site, floor, revision) except StoreError as exc: diff --git a/mote_fleet/server/ui/app.mjs b/mote_fleet/server/ui/app.mjs index 0167b26..6d10675 100644 --- a/mote_fleet/server/ui/app.mjs +++ b/mote_fleet/server/ui/app.mjs @@ -15,6 +15,7 @@ import { BrokerReader, parseTopic } from './mqtt.mjs'; import { MapView } from './map.mjs'; import { ZoneEditor } from './zone_editor.mjs'; +import { ReviewView } from './review.mjs'; import { setupPanes } from './layout.mjs'; const TOKEN_KEY = 'mote.operator.token'; @@ -42,6 +43,7 @@ const state = { const dom = {}; let mapView = null; let editor = null; +let review = null; let panes = null; let pending = false; @@ -192,6 +194,12 @@ async function ensureMap(record) { return; } dom.mapLabel.textContent = key; + // What revisions the floor has does not depend on it having a published map, + // and must not be fetched as though it did. This used to sit after the + // basemap fetch, behind its early return, so a floor whose only revisions + // were candidates reported no candidates — and the first promotion on any + // floor could never be made from the browser (observed live, 2026-08-02). + loadFloor(site, floor, key); try { const meta = await api(`/v1/maps/${site}/${floor}/map.json`); const image = new Image(); @@ -209,7 +217,6 @@ async function ensureMap(record) { api(`/v1/maps/${site}/${floor}/zones.json`) .then((body) => state.mapKey === key && setZones(body.zones)) .catch(() => state.mapKey === key && setZones([])); - loadFloor(site, floor, key); } // Taught places go two ways: onto the basemap, and into the dispatch picker. @@ -233,29 +240,25 @@ async function loadFloor(site, floor, key) { renderRevisions(); } -// Which revision the floor is on, and what else could be promoted onto it. A -// candidate is a map some robot published that changes nothing until an -// operator says so, so this is where that decision is made. +// Which revision the floor is on, and whether anything is waiting to replace +// it. The decision is deliberately *not* made here: this canvas draws live +// robots on the **canonical** basemap, so a promote button beside it would be +// a promotion made without ever seeing the map being promoted — which is what +// the review pane exists to end. This is the signpost to it. +// +// Candidates the validator refused are counted too: "why can I not promote the +// map my robot just published" is a question the review pane can answer and a +// filtered-out row cannot. function renderRevisions() { const floor = state.floor; - if (!floor) { - dom.revisions.hidden = true; - dom.mapRevision.textContent = ''; - return; - } - dom.mapRevision.textContent = floor.canonical || 'no published map'; - const candidates = floor.revisions.filter( - (revision) => !revision.canonical && revision.ok, - ); - dom.revisions.hidden = candidates.length === 0; - dom.revision.replaceChildren( - ...candidates.map((revision) => - el('option', { - value: revision.revision, - text: `${revision.revision}${revision.robot_id ? ` · ${revision.robot_id}` : ''}`, - }), - ), - ); + dom.mapRevision.textContent = floor ? floor.canonical || 'no published map' : ''; + const candidates = floor + ? floor.revisions.filter((revision) => !revision.canonical) + : []; + dom.reviewJump.hidden = candidates.length === 0; + dom.reviewJump.textContent = `${candidates.length} candidate${ + candidates.length === 1 ? '' : 's' + } — review`; } // The zones of the floor on screen, as a `goto` the operator does not have to @@ -278,29 +281,18 @@ function onZone() { dom.dispatchNote.className = 'note'; } -async function onPromote(event) { - event.preventDefault(); - const revision = dom.revision.value; - if (!state.floor || !revision) return; - const { site, floor } = state.floor; - dom.promoteNote.textContent = `promoting ${revision}…`; - dom.promoteNote.className = 'note'; - try { - const body = await api( - `/v1/sites/${site}/floors/${floor}/revisions/${revision}/promote`, - { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: '{"schema":1}' }, - ); - dom.promoteNote.textContent = body.announced - ? `${site}/${floor} is on ${body.revision}; robots will pull it` - : `promoted, but not announced: ${body.detail}`; - // Re-read the floor rather than assume: the basemap on screen is now a - // different map. - state.mapKey = null; - scheduleRender(); - } catch (error) { - dom.promoteNote.textContent = error.message; - dom.promoteNote.className = 'note error'; - } +// Into the review pane, on the floor already on screen. Promotion lives there +// because that is where the candidate's own map is drawn. +function onReviewJump() { + panes.show('review'); + if (state.mapKey) review.open(state.mapKey); +} + +// A promotion happened in the review pane: this pane's basemap is now a +// different map, so re-resolve it rather than keep drawing the old one. +function onPromoted() { + state.mapKey = null; + scheduleRender(); } // Editing starts from the zones on screen — the canonical floor's — and never @@ -330,7 +322,7 @@ async function onSaveZones() { }); editor.finish(); dom.zonesEdit.disabled = false; - dom.promoteNote.textContent = `zone candidate ${body.revision} saved \u2014 the zones shown are the candidate's; pick it in the picker and promote it`; + dom.promoteNote.textContent = `zone candidate ${body.revision} saved \u2014 the zones shown are the candidate's; open review to see it and promote it`; dom.promoteNote.className = 'note'; await loadFloor(site, floor, state.mapKey); } catch (error) { @@ -604,12 +596,26 @@ function bind() { token: 'token', mapLabel: 'map-label', mapRevision: 'map-revision', - revisions: 'revisions', - revision: 'revision', + reviewJump: 'review-jump', promoteNote: 'promote-note', canvas: 'map-canvas', follow: 'follow', fit: 'fit', + reviewFloors: 'review-floors', + reviewFloor: 'review-floor', + reviewCanonical: 'review-canonical', + reviewRevisions: 'review-revisions', + reviewVerdict: 'review-verdict', + reviewVerdictNotes: 'review-verdict-notes', + reviewNotesLabel: 'review-notes-label', + reviewProvenance: 'review-provenance', + reviewZones: 'review-zones', + reviewZoneSource: 'review-zone-source', + reviewCanvas: 'review-canvas', + reviewMapLabel: 'review-map-label', + reviewPromote: 'review-promote', + reviewFit: 'review-fit', + reviewNote: 'review-note', zonesEdit: 'zones-edit', zoneEditor: 'zone-editor', zoneRows: 'zone-rows', @@ -636,10 +642,39 @@ export async function boot() { scheduleRender(); }, }); - // The canvas has no size until its pane is on screen, so the map is told when - // it becomes visible rather than fitting into a hidden 0x0 box. - panes = setupPanes({ onShow: (name) => name === 'map' && mapView.shown() }); + review = new ReviewView({ + api, + onPromoted, + dom: { + canvas: dom.reviewCanvas, + floors: dom.reviewFloors, + floor: dom.reviewFloor, + canonical: dom.reviewCanonical, + revisions: dom.reviewRevisions, + verdict: dom.reviewVerdict, + verdictNotes: dom.reviewVerdictNotes, + notesLabel: dom.reviewNotesLabel, + provenance: dom.reviewProvenance, + zones: dom.reviewZones, + zoneSource: dom.reviewZoneSource, + mapLabel: dom.reviewMapLabel, + promote: dom.reviewPromote, + fit: dom.reviewFit, + note: dom.reviewNote, + }, + }); + // A canvas has no size until its pane is on screen, so each map is told when + // it becomes visible rather than fitting into a hidden 0x0 box. The review + // pane's canvas is hidden at *every* width until the pane is opened, so this + // is the only moment it can be fitted at all. + panes = setupPanes({ + onShow: (name) => { + if (name === 'map') mapView.shown(); + if (name === 'review') review.shown(); + }, + }); dom.zone.addEventListener('change', onZone); + dom.reviewJump.addEventListener('click', onReviewJump); dom.fit.addEventListener('click', () => { mapView.follow(null); dom.follow.checked = false; @@ -651,7 +686,6 @@ export async function boot() { mapView.draw(); }); dom.dispatch.addEventListener('submit', onDispatch); - dom.revisions.addEventListener('submit', onPromote); editor = new ZoneEditor(mapView, { panel: dom.zoneEditor, rows: dom.zoneRows, @@ -670,6 +704,9 @@ export async function boot() { document.getElementById('contract').textContent = state.config.contract; await checkOperator(); await loadRoster().catch((error) => console.warn('roster unavailable', error)); + // The registry's floors, not the fleet's: a floor worth reviewing may have no + // robot reporting it at all. + await review.loadFloors().catch((error) => console.warn('registry unavailable', error)); const { root, presence, health, pose, status } = state.config.topics; const reader = new BrokerReader({ diff --git a/mote_fleet/server/ui/index.html b/mote_fleet/server/ui/index.html index 897b91c..1a7bcdf 100644 --- a/mote_fleet/server/ui/index.html +++ b/mote_fleet/server/ui/index.html @@ -44,11 +44,10 @@

map

— - + an operator promotes it. Promotion happens in the review pane, + where the candidate's own map is on screen — this is only the + signpost that says there is something to look at. --> + @@ -68,6 +67,58 @@

map

+ +
+
+

review

+ — + +
+ +
+
+ +

+ published: — +

+
+ + +

validation

+

+ +
    + +

    provenance

    +
    + + +

    +
    + +
    + +

    zones

    + +

    +
    +
    +
    +
    +

    no robot selected

    @@ -126,6 +177,7 @@

    task status

    roster + diff --git a/mote_fleet/server/ui/map.mjs b/mote_fleet/server/ui/map.mjs index f070c39..96fd4ce 100644 --- a/mote_fleet/server/ui/map.mjs +++ b/mote_fleet/server/ui/map.mjs @@ -134,8 +134,16 @@ export class MapView { // -- data ------------------------------------------------------------- - setMap(map, image) { - const changed = !this.map || this.map.site !== map.site || this.map.floor !== map.floor; + // `refit` overrides the default test for a caller that knows better. The + // fleet map changes basemap only when it changes floor, so that is the test; + // the review view swaps between revisions *of* one floor, where holding the + // viewport is how two candidates get compared — but a revision of a different + // size has to be re-fitted or it lands off screen. + setMap(map, image, refit = null) { + const changed = + refit === null + ? !this.map || this.map.site !== map.site || this.map.floor !== map.floor + : refit; this.map = map; this.image = image; if (changed) { diff --git a/mote_fleet/server/ui/review.mjs b/mote_fleet/server/ui/review.mjs new file mode 100644 index 0000000..2673824 --- /dev/null +++ b/mote_fleet/server/ui/review.mjs @@ -0,0 +1,481 @@ +// Candidate review: see the map you are about to promote, then promote it. +// +// M4's rule is that uploading is not publishing, which puts a decision in front +// of an operator — and until this view existed, the only thing on screen to +// make it with was a timestamp in a `