From 17a1ed9654eeedd0172cdbfa676c11a235943e07 Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Wed, 12 Aug 2026 22:16:13 +0100 Subject: [PATCH 01/13] Fleet UI: name a candidate's zones before promoting it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The zone editor shipped against the *canonical* revision, which left the case the mapping pipeline is actually built around unreachable. `segment-map` hands over a map whose rooms are called `zone_01`..`zone_07`; renaming them meant deriving from the published revision, so the placeholders had to be **promoted in order to be allowed to fix them** — publishing a map because it was wrong — and the coordinates were then rebound onto a frame they had not been drawn on. So the edit now names the revision it edits (`POST …/floors//zones` takes a `revision`; omitted, it is the canonical one as before), and the editor moves into the review pane that #100 built, which is the only place a candidate's own map is on screen. Editing stays a *derivation*: the named revision's map bytes are re-packed with the submitted zones and accepted as an ordinary candidate, inert until promoted, so nothing stored is ever written and promotion stays the only write that moves a floor. Five things are load-bearing. **A derivation is held to `promote`'s bar, not the upload's.** A revision with no posegraph cannot be extended — an error for a robot's upload, where the session can be re-run, and a *warning* on something already stored, which navigates perfectly and which `promote` accepts. `accept` therefore takes `require_posegraph`. Found in a browser, not by reading: every sim site bundle is such a revision, so `edit zones` sat beside a `promotable` verdict and could only ever fail. **The vocabulary half is editable, and `navigable` is not written when the kind already implies it.** Kind and aliases join name and display_name in each row. Every zone arrives from the server with `navigable` filled in (`zone_term` defaults it from the kind), so writing it back verbatim would carry a `keepout`'s `false` onto a zone just changed to `room` — a room nothing can be dispatched to, with nothing on screen to say why. Empty lists are dropped for the same reason: they say nothing the default does not. **The editor refuses exactly what the robot's loader refuses.** A name a dispatcher cannot type, and two zones answering one query — `ambiguities` mirrors `bundle.ambiguities` (names and aliases, not display names), because the loader *refuses* an ambiguous vocabulary rather than resolving it by dict order, and a stored candidate no robot will load is worse than a rejected save. **A pose can be placed, not only dragged.** A segmented room is a polygon with no `x`/`y`, so it drew no cross to drag and the robot derived a centroid — the middle of the outline rather than where you would send a robot. `⌖` arms the next map click. **After a save the pane selects the new candidate and re-reads it**, so the zones on screen are the saved ones from the server. That retires the MVP's frozen overlay, which existed only because the operations map had nothing to re-render but the stale set the edit was made from (read as data loss, 2026-08-02). Editing is a mode: floor picker, revision list and promote are locked while it is up, there is no autosave, `cancel` discards. One pre-existing defect fell out. The map pane's review button was hidden unless the floor on screen had candidates, and above 760 px the tab bar is hidden too — so the pane built for floors no robot is reporting was reachable only through a floor a robot *was* reporting. It is now always there, and says how many candidates the floor has when it knows. Verified: 13 API tests (`test_zone_edit.py`, incl. deriving from a candidate, a floor with nothing published, the vocabulary revision continuing from the edited revision, an unknown/traversing revision, and the posegraph bar), 50 node tests (the kind list read out of `bundle.py` so it cannot drift, alias round-trip, ambiguity, the `navigable` rule, the editor's home), and four new checks in `browser_check.mjs` — `pixi run fleet-ui-check` passes 30/30 against a real broker, server, fleet and chrome, at desk width and on an emulated phone. Still open on the task: carry-forward accept/reject (needs task 345), and grid snapping / shared vertices (task 350). Claude-Session: https://claude.ai/code/session_01LUGSpEmdmC68oBaWp6B5Qz Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 44 ++++++ docs/fleet/README.md | 65 +++++++- docs/fleet/fleet-api.md | 74 ++++++++- mote_fleet/README.md | 2 +- mote_fleet/server/bundle_store.py | 50 ++++-- mote_fleet/server/fleet_server.py | 25 ++- mote_fleet/server/ui/app.mjs | 78 +++------- mote_fleet/server/ui/index.html | 43 +++--- mote_fleet/server/ui/review.mjs | 117 +++++++++++++- mote_fleet/server/ui/style.css | 92 ++++++++--- mote_fleet/server/ui/zone_editor.mjs | 219 +++++++++++++++++++++++---- mote_fleet/test/browser_check.mjs | 101 ++++++++++++ mote_fleet/test/test_zone_edit.py | 163 ++++++++++++++++++-- mote_fleet/test/ui_test.mjs | 125 ++++++++++++++- 14 files changed, 1029 insertions(+), 169 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index fb1ef72..6eb7580 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -185,6 +185,50 @@ 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: editing a candidate's zones + +The write half of that pane (`ui/zone_editor.mjs`, route `POST /v1/sites// +floors//zones`, operator flow `docs/fleet/README.md` §11, contract +`fleet-api.md`): drag vertices and zones on the map, double-click an edge or +vertex to add or remove one, name the zone, its `display_name`, its `kind` and +its `aliases` in a row per zone, and arm `⌖` to place a pose with the next map +click. **The whole design is one rule: editing is a derivation, never a +mutation.** Saving re-packs *the revision under review* with the submitted zones +and accepts the result as an ordinary candidate — same `accept()` as a robot's +upload, inert until promoted — so a stored revision's bytes never change, which +is what the announced digests depend on, and promotion stays the only write that +moves a floor. Five things are load-bearing. **The edit names a source +revision**, which is what makes an *unpromoted* map editable: `segment-map` +hands over `zone_01`..`zone_07`, and deriving only from the canonical revision +meant promoting those placeholders in order to be allowed to fix them — +publishing a map because it was wrong — besides rebinding coordinates drawn on +one frame onto another's. **A derivation is held to `promote`'s bar, not the +upload's** (`accept(require_posegraph=…)`): a revision with no posegraph cannot +be extended, which is an error for a robot's upload where the session can be +re-run and a *warning* on something already stored, so the strict bar put an +`edit zones` button beside a `promotable` verdict that could only ever fail +(found in a browser; every sim bundle is such a revision). **It lives in the +review pane and nowhere else** — the operations canvas draws the *published* +basemap, so an editor there could only ever edit the published revision, and +"which map are these coordinates against" must have one answer. That also +retires the MVP's frozen-overlay workaround: after a save the pane selects the +new candidate and re-reads its zones, so what is on screen is the saved set from +the server rather than a held-over copy of what was typed. **`navigable` is +written only when it deviates from the kind**, because every zone arrives from +the server with the field filled in (`bundle.zone_term` defaults it) and writing +it back verbatim would carry a `keepout`'s `false` onto a zone just changed to +`room` — a room nothing can be dispatched to, with nothing on screen to say why. +And the editor **refuses client-side exactly what the robot's loader refuses**: +a non-dispatchable name, and two zones answering one query (`ambiguities` +mirrors `bundle.ambiguities` — names and aliases, not display names), since a +stored candidate no robot will load is worse than a rejected save. Editing is a +*mode*: the floor picker, the revision list and promote are disabled while it is +up, there is no autosave, and `cancel` discards. One pre-existing defect fell +out and is fixed: the map pane's review button was hidden unless the floor on +screen had candidates, and above 760 px the tab bar is hidden too — so the pane +built for floors no robot is reporting was reachable only through a floor a robot +was reporting. + ## Fleet: the zone vocabulary The API served the roster, the basemaps and dispatch, but not the one thing a diff --git a/docs/fleet/README.md b/docs/fleet/README.md index 516ae34..fee34c3 100644 --- a/docs/fleet/README.md +++ b/docs/fleet/README.md @@ -663,12 +663,15 @@ outline for a `polygon`, a cross for a bare waypoint, each labelled — so the `goto ` targets you can type are the ones you can see. They come from the canonical revision, in that revision's map frame. -Beside the map's floor label is the **canonical revision** it is showing, and, -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. +Beside the map's floor label is the **canonical revision** it is showing, and a +button into the **review** pane — which is where a candidate is looked at, its +zones named, and the map promoted (§11). It says how many candidates the floor +on screen has when it knows, and it is there either way: above 760 px the tab +bar is hidden, so a button that appeared only for a floor with candidates was +the sole door to a pane whose whole point is the floors *no robot is reporting*. +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) @@ -901,8 +904,54 @@ which appears whenever the floor on screen has something waiting. It shows: - **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. +Everything except that button and the zone editor below is a read. Nothing you +do here changes any floor until you promote. + +### Naming the rooms on it, before you promote it + +`pixi run segment-map` finds the rooms of a map but cannot know what they are +called, so a fresh revision arrives with `zone_01`..`zone_07`. **`edit zones`**, +beside the zone list, is where they get their names — on the candidate's own map, +where you can see which room is which. + +The controls are the map and the list together: + +- **On the map**: drag a vertex to follow a wall, drag a zone to move footprint + and pose together, double-click an edge to add a vertex or a vertex to remove + it. A polygon needs three, so the last removal is refused rather than quietly + making a line. +- **In the list**, per zone: the machine name (`goto `), the display name + an operator sees, the **kind** (zone/v0 — `room`, `corridor`, `keepout`, …), + and **aliases**, comma separated, for the other things people call it. `⌖` + arms the next map click as that zone's pose, which is the only way to give one + to a zone that has none — a segmented room is an outline whose pose the robot + would otherwise derive as a centroid, i.e. wherever the middle happens to be + rather than where you would send a robot. +- **`add zone`** drops a square at the view centre to be dragged into shape and + named; **`×`** deletes one. + +Two names the same is refused before it is saved — the robot's loader refuses a +vocabulary where one query answers to two zones rather than picking by luck, so +the editor must not produce one. A name a dispatcher cannot type (`Café`, `Drop +Off`) is refused the same way. + +**Saving derives a new candidate**: `save as candidate` sends the edited set, +and the server re-packs the revision you were editing with those zones in place +of its own. The revision you edited is untouched — including when it is the +published one — and the new candidate is selected in the pane, so the zones on +screen afterwards are the saved ones read back from the server. Promote it when +it looks right. Two consequences worth knowing: + +- **Iterating costs a candidate each save.** Editing a candidate derives from + *it*, so a floor's list grows while you work; the registry keeps the canonical + revision plus the five newest candidates, so the intermediates fall off on + their own. +- **A revision that inherited the floor's zones stops inheriting.** The saved + candidate carries them, which is what you want: inherited zones were taught in + another session's frame, and dragging them onto this map is the correction. + +`cancel` discards the edit. There is no autosave and nothing is written until +you save, so an edit you are unsure about costs nothing to abandon. **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 diff --git a/docs/fleet/fleet-api.md b/docs/fleet/fleet-api.md index fb42e4e..4cf23e2 100644 --- a/docs/fleet/fleet-api.md +++ b/docs/fleet/fleet-api.md @@ -101,6 +101,8 @@ 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) +POST /v1/sites//floors//zones edited zones of a revision -> + a new candidate (an operator) GET / the operator UI (static files) ``` @@ -499,6 +501,69 @@ 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. +### `POST /v1/sites//floors//zones` + +Operator token required. The edited zone set, and the revision it was edited +against: + +```json +{"schema": 1, "revision": "20260802T145731", + "zones": {"kitchen": {"x": 1.0, "y": -3.0, "yaw": 0.0, "kind": "room", + "display_name": "The Kitchen", "aliases": ["galley"], + "polygon": [[0.0,-4.0],[2.0,-4.0],[2.0,-2.0],[0.0,-2.0]]}}} +``` + +`zones` is the `zones.yaml` shape — keyed by name, both halves of zone/v0 in one +entry — and it **replaces** that revision's set rather than patching it. An entry +may echo its key as `name`, which is dropped: the file keys by name and carries +no second copy. + +```json +{"schema":1,"site":"home","floor":"ground","revision":"20260812T211029", + "derived_from":"20260802T145731","promoted":false,"warnings":[],"audit_id":31} +``` + +| Status | Meaning | +|---|---| +| `201` | stored as a candidate — `revision` is the new one, `derived_from` the edited one | +| `400` | `zones` is not a mapping, or a name is not a directory name | +| `401` | no usable operator token (recorded as an anonymous attempt) | +| `404` | no such floor, or no such `revision` on it | +| `409` | no `revision` given and the floor has nothing published to edit | +| `422` | the edited set is not readable as a `zones.yaml`; `errors` says why | + +**Editing is a derivation, not a mutation.** The named revision's map bytes are +re-packed with the submitted zones in place of its own and accepted as an +ordinary candidate — validated by the same code as a robot's upload, listed by +the floor route, promotable through the route above. Nothing already stored is +written: a promoted revision's bytes back a digest the fleet has been told, and a +candidate is immutable for the same reason an id is never reused. The response +carries `promoted: false` because that stays a separate, audited decision. + +**`revision` is what makes an unpromoted map editable**, and that is the point +rather than a convenience. A fresh build arrives carrying `zone_01`..`zone_07` +from `segment-map`; without it the only thing an edit could derive from was the +canonical revision, so renaming those placeholders meant promoting them first — +publishing a map *because* it was wrong. It also keeps the coordinates in the +frame they were drawn in: the operator is looking at that revision's own map. +Omitted, the canonical revision is edited, which is the same thing for a floor +whose published map is what is on screen. + +**The bar is the source's, not the upload's.** A revision with no posegraph is +one mapping cannot be continued from — an error for a robot's upload, where the +session can be re-run, and a *warning* on a stored revision, which navigates +perfectly and which `promote` will accept. A derivation is therefore validated +the way `promote` validates: holding an edit to a stricter bar than the revision +it derives from would put an `edit zones` button beside a `promotable` verdict +that could only ever fail. + +Two things this route deliberately does not do. It does not slugify or otherwise +repair a name — a zone a dispatcher cannot type is stored with a warning, as +everywhere else in the vocabulary (`problems` are reported, not enforced; the +*robot's* loader is what refuses one). And it never publishes: the audit row +names the actor, the floor and the revision edited, and the floor's `map` +symlink is untouched. + --- ## What the browser is allowed to do @@ -511,7 +576,8 @@ 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. +`map.json`, `map.png` and `zones.json`; the two writes beside them are the +`promote` M4 already had and the zone edit above, both operator-authorized and +both audited. The edit is the only write in the fleet that produces a revision, +and it produces an inert one: a candidate nobody is running, on a floor that has +not moved. diff --git a/mote_fleet/README.md b/mote_fleet/README.md index 0ae2542..fddb2ab 100644 --- a/mote_fleet/README.md +++ b/mote_fleet/README.md @@ -63,7 +63,7 @@ pixi run -e fleet fleetctl -- dispatch mote-01 goto kitchen | [`server/registry.py`](server/registry.py) | the SQLite row store: robots, enrollment tokens, operators, the audit log, transactional id allocation | | [`server/bundle_store.py`](server/bundle_store.py) | the map registry's byte store: candidate revisions, validation on the way in, the atomic flip that publishes one | | [`server/fleetctl.py`](server/fleetctl.py) | operator CLI: tokens, roster, dispatch, audit, watch | -| [`server/ui/`](server/ui/) | the dashboard: `index.html`, `app.mjs`, `map.mjs` (basemap + the Q5 transform, pan/pinch), `mqtt.mjs` (a subscribe-only MQTT client), `layout.mjs` (one pane at a time on a phone) | +| [`server/ui/`](server/ui/) | the dashboard: `index.html`, `app.mjs`, `map.mjs` (basemap + the Q5 transform, pan/pinch), `mqtt.mjs` (a subscribe-only MQTT client), `layout.mjs` (one pane at a time on a phone), `review.mjs` (see a candidate before promoting it), `zone_editor.mjs` (name its zones, saved as a new candidate) | | [`server/mosquitto.conf`](server/mosquitto.conf), [`broker.sh`](server/broker.sh) | the broker, its WebSocket listener, and where its state goes | | [`deploy/`](deploy/) | the deployed shape: an image for the API+UI, a compose file that runs it beside the broker, and `fleet-deploy.sh` (gated update, rollback, backup, restore) | diff --git a/mote_fleet/server/bundle_store.py b/mote_fleet/server/bundle_store.py index eecd31b..315a8d3 100644 --- a/mote_fleet/server/bundle_store.py +++ b/mote_fleet/server/bundle_store.py @@ -354,26 +354,44 @@ def read_vocabulary(self, site: str, floor: str) -> dict: raise StoreError(f"no zones for {site}/{floor}", 404) return bundle.vocabulary(bundle.read_zones(path), site, floor) - def derive_zones(self, site: str, floor: str, zones: dict, *, by: str): - """A new candidate revision: the canonical map's bytes, edited zones. + def derive_zones( + self, site: str, floor: str, zones: dict, *, by: str, source: str = "" + ): + """A new candidate revision: one revision's map bytes, edited zones. An operator's zone edit never touches a stored revision — a promoted revision's bytes back a digest the fleet has been told, and a candidate is immutable for the same reason one id is never reused. So an edit is - a *derivation*: pack the canonical revision with the submitted zones in + a *derivation*: pack the source revision with the submitted zones in place of its own, and accept the result as an ordinary candidate — validated by the same code as any upload, inert until promoted. + + ``source`` is the revision being edited, defaulting to the canonical + one. Naming a candidate is what makes an unpromoted map editable: the + first build of a floor arrives with `zone_01`..`zone_07` from + `segment-map`, and defaulting to the canonical would have meant + promoting placeholder names in order to be allowed to fix them — + publishing a map *because* it was wrong. It also puts the edit in the + frame it was drawn in: the operator renaming rooms is looking at the + candidate's own map in the review pane, and rebinding those names onto + the published map's frame is exactly the trap the pane exists to name. Returns ``(stored_revision, report, derived_from)``. """ - canonical = self.canonical(site, floor) - if not canonical: + source = source or self.canonical(site, floor) + if not source: raise StoreError( f"{site}/{floor} has no published map to edit zones on", 409 ) - rev_dir = self.revision_dir(site, floor, canonical) + rev_dir = self.revision_dir(site, floor, source) previous = {} - zones_file = rev_dir / bundle.ZONES_YAML - if zones_file.is_file(): + # The zones the operator was shown, which is what the edit is a delta + # of: `_zones_file` falls back to the floor's file for a revision + # carrying none, and so does the review pane that fed the editor. The + # `frame_id` and `vocabulary_revision` come from there for the same + # reason — an edit that silently reset the vocabulary revision would + # make a later carry-forward unable to tell which naming is newer. + zones_file = self._zones_file(site, floor, revision=source) + if zones_file is not None: try: previous = bundle.read_zones(zones_file) except bundle.BundleError: @@ -400,8 +418,9 @@ def derive_zones(self, site: str, floor: str, zones: dict, *, by: str): datetime.now(timezone.utc).strftime("%Y%m%dT%H%M%S"), blob, uploaded_by=by, + require_posegraph=False, ) - return stored, report, canonical + return stored, report, source def vocabularies(self) -> list: """Every floor's vocabulary, for a dispatcher bootstrapping a whole @@ -468,6 +487,7 @@ def accept( *, robot_id: str = "", uploaded_by: str = "", + require_posegraph: bool = True, ) -> tuple[str, bundle.Report]: """Store an uploaded revision as a **candidate**. Returns ``(id, report)``. @@ -475,6 +495,16 @@ def accept( robot is running. That separation is the milestone's conflict story: a second robot's map of the same floor is kept beside the first, and an operator promotes one. + + ``require_posegraph`` is what a *robot's upload* is held to and what a + derivation is not: a mapping session that produced no posegraph produced + a map nothing can extend, and that is worth refusing at the source. A + revision already in the registry has been judged by the looser rule + :meth:`promote` uses, so a derivation of one must not be held to a + stricter bar than the revision it came from — the review pane calls such + a revision promotable, and an `edit zones` button beside that verdict + that could only ever fail is worse than no button (observed: the sim's + own bundles carry no posegraph). """ if not self.root: raise StoreError("this server stores no site bundles", 404) @@ -491,7 +521,7 @@ def accept( except bundle.BundleError as exc: raise StoreError(str(exc), 400) from exc try: - report = bundle.validate(staging) + report = bundle.validate(staging, require_posegraph=require_posegraph) except bundle.BundleError as exc: # validate() documents that it reports rather than raises, and # it is tested that way. Belt and braces: a validator that diff --git a/mote_fleet/server/fleet_server.py b/mote_fleet/server/fleet_server.py index 005a01a..6985d87 100644 --- a/mote_fleet/server/fleet_server.py +++ b/mote_fleet/server/fleet_server.py @@ -37,7 +37,8 @@ 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) + POST /v1/sites//floors//zones edited zones of a revision -> + a new candidate (operator) GET / the operator UI (static files) pixi run fleet-server -- --db ~/fleet/registry.db --broker-host fleet-box @@ -816,13 +817,19 @@ def _promote(self, rest: str, body: dict): ) def _edit_zones(self, rest: str, body: dict): - """An operator's zone edit: derive a candidate from the canonical map. + """An operator's zone edit: derive a candidate from a revision. The edit writes nothing the fleet can see — the result is an ordinary candidate (same map bytes, new zones), validated like any upload and inert until the operator promotes it through the existing route. That keeps promoted revisions immutable, which the announced digests rely on, and keeps promotion the only write that changes a floor. + + The body's optional ``revision`` is the revision being edited; without + one the canonical map is edited, as before. It is a body field rather + than a path segment because the edited revision is an *input* to the + derivation and never the thing written — the route's own resource is + the floor's zones, and the result is a revision id neither end chose. """ parts = rest.split("/") if len(parts) != 3 or parts[1] != "floors": @@ -844,23 +851,27 @@ def _edit_zones(self, rest: str, body: dict): ) self._error(401, str(exc)) return - if not self._names(site, floor): + source = str(body.get("revision") or "") + if not self._names(site, floor, *([source] if source else [])): return zones = body.get("zones") if not isinstance(zones, dict): self._error(400, "a zones mapping is required: {zones: {name: {...}}}") return actor = operator["name"] + # The audit row names what was edited, not only which floor: two + # candidates of one floor are two different maps, and "who renamed the + # rooms on this map" is unanswerable from the floor alone. entry = registry.record( actor=actor, action="map.zones", - command=target, + command=f"{target}/{source}" if source else target, result="editing", remote=self.address_string(), ) try: stored, report, derived_from = self.server.store.derive_zones( - site, floor, zones, by=actor + site, floor, zones, by=actor, source=source ) except StoreError as exc: registry.finish(entry["id"], "rejected", str(exc)) @@ -876,7 +887,9 @@ def _edit_zones(self, rest: str, body: dict): traceback.print_exc() self._error(500, "the zone edit could not be stored") return - registry.finish(entry["id"], "stored", f"candidate {stored}") + registry.finish( + entry["id"], "stored", f"candidate {stored} from {derived_from}" + ) print( f"zone edit {target} by {actor}: candidate {stored} (from {derived_from})", file=sys.stderr, diff --git a/mote_fleet/server/ui/app.mjs b/mote_fleet/server/ui/app.mjs index 6d10675..45b2bba 100644 --- a/mote_fleet/server/ui/app.mjs +++ b/mote_fleet/server/ui/app.mjs @@ -14,7 +14,6 @@ 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'; @@ -42,7 +41,6 @@ const state = { const dom = {}; let mapView = null; -let editor = null; let review = null; let panes = null; let pending = false; @@ -182,10 +180,6 @@ async function ensureMap(record) { if (key === state.mapKey) return; state.mapKey = key; state.floor = null; - if (editor) { - editor.end(); - dom.zonesEdit.disabled = false; - } setZones([]); renderRevisions(); if (!key) { @@ -255,10 +249,16 @@ function renderRevisions() { 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`; + // Always reachable, and *not* conditional on this floor having candidates. + // The tab bar it shares the job with is hidden above 760 px, so while this + // was, a desk-width operator could reach the review pane only through a + // floor a robot was reporting — which is precisely the floor review is least + // needed for. The floor mapped last week by a robot since switched off, and + // the floor a build lands on with no robot near it, were both unreachable. + dom.reviewJump.hidden = false; + dom.reviewJump.textContent = candidates.length + ? `${candidates.length} candidate${candidates.length === 1 ? '' : 's'} — review` + : 'review maps'; } // The zones of the floor on screen, as a `goto` the operator does not have to @@ -295,42 +295,6 @@ function onPromoted() { scheduleRender(); } -// Editing starts from the zones on screen — the canonical floor's — and never -// writes back to them: saving derives a fresh candidate revision server-side, -// which the picker then promotes like any robot-published map. -function onEditZones() { - if (!state.mapKey || !mapView.map) return; - editor.begin(state.zones); - dom.zonesEdit.disabled = true; -} - -async function onSaveZones() { - const problem = editor.problems(); - if (problem) { - dom.zoneNote.textContent = problem; - dom.zoneNote.className = 'note error'; - return; - } - const [site, floor] = state.mapKey.split('/'); - dom.zoneNote.textContent = 'saving\u2026'; - dom.zoneNote.className = 'note'; - try { - const body = await api(`/v1/sites/${site}/floors/${floor}/zones`, { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ schema: 1, zones: editor.payload() }), - }); - editor.finish(); - dom.zonesEdit.disabled = false; - 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) { - dom.zoneNote.textContent = error.message; - dom.zoneNote.className = 'note error'; - } -} - // -- rendering ----------------------------------------------------------- function scheduleRender() { @@ -597,7 +561,6 @@ function bind() { mapLabel: 'map-label', mapRevision: 'map-revision', reviewJump: 'review-jump', - promoteNote: 'promote-note', canvas: 'map-canvas', follow: 'follow', fit: 'fit', @@ -661,6 +624,15 @@ export async function boot() { promote: dom.reviewPromote, fit: dom.reviewFit, note: dom.reviewNote, + // The zone editor's own controls. It lives in this pane because it edits + // the *selected revision's* zones over that revision's own map. + zonesEdit: dom.zonesEdit, + editor: dom.zoneEditor, + editorRows: dom.zoneRows, + editorNote: dom.zoneNote, + zoneAdd: dom.zoneAdd, + zoneSave: dom.zoneSave, + zoneCancel: dom.zoneCancel, }, }); // A canvas has no size until its pane is on screen, so each map is told when @@ -686,18 +658,6 @@ export async function boot() { mapView.draw(); }); dom.dispatch.addEventListener('submit', onDispatch); - editor = new ZoneEditor(mapView, { - panel: dom.zoneEditor, - rows: dom.zoneRows, - note: dom.zoneNote, - }); - dom.zonesEdit.addEventListener('click', onEditZones); - dom.zoneAdd.addEventListener('click', () => editor.addZone()); - dom.zoneSave.addEventListener('click', onSaveZones); - dom.zoneCancel.addEventListener('click', () => { - editor.end(); - dom.zonesEdit.disabled = false; - }); document.getElementById('token-form').addEventListener('submit', onToken); state.config = await api('/v1/config'); diff --git a/mote_fleet/server/ui/index.html b/mote_fleet/server/ui/index.html index 1a7bcdf..e0495a9 100644 --- a/mote_fleet/server/ui/index.html +++ b/mote_fleet/server/ui/index.html @@ -44,27 +44,17 @@

map

— - + an operator promotes it. Promotion, and editing its zones, happen + in the review pane where that candidate's own map is on screen — + this is the way in. It is always here, and says how many + candidates the floor on screen has when it knows: above 760 px + the tab bar is hidden, so a conditional button was the only door + to a pane whose whole point is floors no robot is reporting. --> + - -

- - + +

+ diff --git a/mote_fleet/server/ui/review.mjs b/mote_fleet/server/ui/review.mjs index 2673824..1052313 100644 --- a/mote_fleet/server/ui/review.mjs +++ b/mote_fleet/server/ui/review.mjs @@ -22,10 +22,13 @@ // here. The zone list below is deliberately a row per zone with its own // cells, because that is where those controls go. // -// The write surface does not grow: everything this view reads is a GET, and the -// one thing it writes is the promote M4 already had. +// Two writes leave this pane, and both are audited operator actions: the +// promote M4 already had, and the zone edit beside it (`zone_editor.mjs`), +// which derives a *new* candidate rather than touching the revision on screen. +// Nothing here changes a floor until the promote. import { MapView } from './map.mjs'; +import { ZoneEditor } from './zone_editor.mjs'; // -- routes --------------------------------------------------------------- @@ -240,13 +243,26 @@ export class ReviewView { // and an image decode; without this, clicking through two candidates draws // whichever finished last rather than the one selected. this.epoch = 0; + // The zones last loaded for the selected revision, which is what an edit + // starts from and what the map goes back to if the edit is cancelled. + this.zones = []; + this.editing = false; this.map = new MapView(dom.canvas); + this.editor = new ZoneEditor(this.map, { + panel: dom.editor, + rows: dom.editorRows, + note: dom.editorNote, + }); dom.floor.addEventListener('change', () => this.open(dom.floor.value)); dom.promote.addEventListener('click', () => this.promote()); dom.fit.addEventListener('click', () => { this.map.fit(); this.map.draw(); }); + dom.zonesEdit.addEventListener('click', () => this.beginEdit()); + dom.zoneAdd.addEventListener('click', () => this.editor.addZone()); + dom.zoneSave.addEventListener('click', () => this.saveZones()); + dom.zoneCancel.addEventListener('click', () => this.endEdit()); } // The pane's canvas has no size until the pane is on screen, so a fit done @@ -276,6 +292,11 @@ export class ReviewView { async open(key, revision = null) { const parsed = parseFloorKey(key); if (!parsed) return; + // An edit in progress owns the pane: re-opening a floor would reload the + // zones under it and drop the edit on the floor. The controls that lead + // here are disabled while editing; this covers the ones that arrive from + // elsewhere (the map pane's jump button). + if (this.editing) return; const epoch = (this.epoch += 1); this.key = key; this.dom.floor.value = key; @@ -344,6 +365,89 @@ export class ReviewView { } } + // -- editing ---------------------------------------------------------- + + // Editable when there is a revision selected and its map is on screen: the + // coordinates being dragged mean nothing except against that image, and a + // revision whose map failed to load has none. + editable() { + return Boolean(this.selected && this.map.map); + } + + // Editing is a mode on the selected revision, so while it is on, the things + // that would swap that revision out from under it are disabled rather than + // racing it. There is no autosave: an unsaved edit is lost to `cancel`, and + // nothing else can reach it. + renderEditControls() { + this.dom.zonesEdit.disabled = this.editing || !this.editable(); + this.dom.zonesEdit.hidden = !this.editable() && !this.editing; + this.dom.floor.disabled = this.editing; + this.dom.zones.hidden = this.editing; + this.dom.zoneSource.hidden = this.editing; + for (const row of this.dom.revisions.querySelectorAll('button')) { + row.disabled = this.editing; + } + if (this.editing) this.dom.promote.disabled = true; + } + + beginEdit() { + if (!this.editable() || this.editing) return; + this.editing = true; + // The editor draws its own zones, handles and pose crosses; leaving the + // read-only set under them would double every outline. + this.map.setZones([]); + this.editor.begin(this.zones); + this.renderEditControls(); + this.note( + this.selected.canonical + ? 'editing the published map’s zones — saving derives a new candidate' + : `editing candidate ${this.selected.revision} — saving derives a new one`, + ); + } + + endEdit() { + this.editing = false; + this.editor.note(''); + this.editor.end(); + this.map.setZones(this.zones); + this.renderEditControls(); + this.renderVerdict(); + } + + // Saving does not write the revision on screen: the server packs that + // revision's map bytes with the submitted zones and stores the result as an + // ordinary candidate. So the pane then *selects* the new candidate, and the + // zones on screen afterwards are the saved ones read back from the server — + // rather than the frozen overlay this needed when the editor lived on the + // operations map, where the only thing to re-render was the stale set the + // edit was made from (which read as data loss, 2026-08-02). + async saveZones() { + if (!this.editing || !this.selected) return; + const problem = this.editor.problems(); + if (problem) { + this.editor.note(problem, true); + return; + } + const { site, floor } = parseFloorKey(this.key); + const from = this.selected.revision; + this.editor.note('saving…'); + let body; + try { + body = await this.api(`/v1/sites/${site}/floors/${floor}/zones`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ schema: 1, revision: from, zones: this.editor.payload() }), + }); + } catch (error) { + this.editor.note(error.message, true); + return; + } + this.endEdit(); + await this.loadFloors(); + await this.open(this.key, body.revision); + this.note(`candidate ${body.revision} saved from ${from}; promote it when it looks right`); + } + // -- rendering -------------------------------------------------------- renderFloors() { @@ -399,6 +503,7 @@ export class ReviewView { el('p', { class: 'empty', text: 'no revisions on this floor' }), ); } + this.renderEditControls(); } renderVerdict() { @@ -423,10 +528,11 @@ export class ReviewView { ); } - // One row per zone, three cells. Read-only here on purpose — the write half - // (rename, alias, kind, click-to-teach a pose) is task 346, and it edits - // exactly these rows. + // One row per zone, three cells — what the revision says its places are. + // `edit zones` replaces this list with the editable one; it is the same set + // of rows with inputs in them. renderZones(zones, source = '') { + this.zones = zones; this.dom.zoneSource.textContent = zoneSource(source, zones.length); this.dom.zones.replaceChildren( ...zones.map((zone) => @@ -437,6 +543,7 @@ export class ReviewView { ]), ), ); + this.renderEditControls(); } note(text, bad = false) { diff --git a/mote_fleet/server/ui/style.css b/mote_fleet/server/ui/style.css index 78b28ed..9bb3e59 100644 --- a/mote_fleet/server/ui/style.css +++ b/mote_fleet/server/ui/style.css @@ -208,20 +208,6 @@ main { padding: 0 14px 10px; } -.revisions { - display: flex; - gap: 6px; - align-items: center; -} - -.revisions select { - max-width: 220px; -} - -#promote-note { - padding: 0 14px 8px; -} - #map-canvas { flex: 1; width: 100%; @@ -620,6 +606,14 @@ main { overflow-y: auto; } +/* The same lesson again, one element along: the read-only zone list is hidden + while the editable one is up, and a class that sets `display` outranks the + `hidden` attribute — leaving both lists on screen, each disagreeing with the + other about what the zones are. */ +.zone-rows[hidden] { + display: none; +} + .zone-row { display: grid; grid-template-columns: 1fr 1fr auto; @@ -638,11 +632,54 @@ main { min-width: 0; } +/* The editable row: name, label, kind, aliases, place-pose, delete. `minmax(0, + 1fr)` on every text cell rather than `1fr`, because a grid track's automatic + minimum is its content and an alias list would otherwise widen the row past + the pane instead of scrolling inside its own input. */ +.zone-editor .zone-row { + grid-template-columns: minmax(0, 1fr) minmax(0, 1fr) auto minmax(0, 1fr) auto auto; +} + +.zone-editor .zone-row select { + font: 12px ui-monospace, monospace; + min-width: 0; +} + +/* Armed: the next click on the map is this zone's pose. The state has to be + visible on the button that armed it — the pointer is about to be somewhere + else entirely, and a click that silently moved a pose would be worse than + no feature. */ +.zone-row button.armed { + border-color: var(--accent); + color: var(--accent); +} + .zone-actions { display: flex; gap: 6px; } +/* Editing holds the selected revision still, so the controls that would swap it + are disabled — and have to *look* disabled, or the pane reads as ignoring + clicks. */ +.zones-head button:disabled, +.revision-row:disabled { + opacity: 0.5; + cursor: not-allowed; +} + +/* The heading and its one control. `edit zones` sits with the zones rather than + in the pane head, because it acts on the list under it and not on the pane. */ +.zones-head { + display: flex; + align-items: baseline; + gap: 10px; +} + +.zones-head h3 { + margin-bottom: 0; +} + /* -- candidate review ----------------------------------------------------- */ /* Review is a *mode*, not a fourth column. The fleet-operations panes stand @@ -794,10 +831,9 @@ main:has(.review-pane.active) > .pane:not(.review-pane) { display: none; } -/* The editor's row grid ends in an `auto` column sized to a button; here the +/* The editable row's grid ends in `auto` columns sized to buttons; here the third cell is a phrase of arbitrary length, which would push the row wider - than the pane. Task 346 replaces these cells with controls and inherits the - row shape either way. */ + than the pane. */ #review-zones .zone-row { grid-template-columns: minmax(0, 1fr) 72px minmax(0, 1fr); } @@ -845,4 +881,26 @@ button.promote:disabled { flex: none; height: 46vh; } + + /* Six cells in a phone's width leaves each one a few characters wide, so the + editable row wraps instead of gridding: the fields take half the width + each and fall onto as many lines as they need, and the buttons keep their + own size. Nothing is hidden — a rename typed on a phone is exactly the + edit someone makes standing in the room being named. */ + .zone-editor .zone-row { + display: flex; + flex-wrap: wrap; + } + + .zone-editor .zone-row input, + .zone-editor .zone-row select { + flex: 1 1 45%; + } + + /* A wrapped row is three lines tall, so the desk's 180 px box would show not + quite two of them. The pane scrolls on a phone anyway (above), so the list + can take a screenful. */ + .zone-editor .zone-rows { + max-height: 46vh; + } } diff --git a/mote_fleet/server/ui/zone_editor.mjs b/mote_fleet/server/ui/zone_editor.mjs index 251e42b..0272c42 100644 --- a/mote_fleet/server/ui/zone_editor.mjs +++ b/mote_fleet/server/ui/zone_editor.mjs @@ -1,14 +1,23 @@ -// Zone editing on the fleet map: drag a vertex, drag a zone, drag a pose, -// rename, add, delete — then save the lot as a *candidate* revision. +// Zone editing on a map revision: drag a vertex, drag a zone, place a pose, +// rename, name the kind, add aliases, add, delete — then save the lot as a +// *candidate* revision. // -// The editor never writes to the floor it is looking at. Saving POSTs the -// edited set to the server, which derives a new candidate from the canonical -// revision (same map bytes, new zones); the operator then promotes it through -// the picker exactly like a robot-published map. Promoted revisions stay -// immutable, which the announced digests rely on. +// The editor never writes to the revision it is looking at. Saving POSTs the +// edited set to the server, which derives a new candidate from that revision +// (same map bytes, new zones); the operator then promotes it exactly like a +// robot-published map. Stored revisions stay immutable, which the announced +// digests rely on, and promotion stays the only write that changes a floor. // -// Geometry lives in pure functions over zone objects in *world* metres, so -// every edit operation is testable under node with no canvas and no DOM. +// It lives in the review pane (`review.mjs`) because a zone is a coordinate in +// one map frame: the map under the zones has to be the map they belong to, and +// only that pane draws a *candidate's* own map. Editing a candidate is the +// point — a fresh build arrives with `zone_01`..`zone_07` from `segment-map`, +// and an editor that could only edit the published map would have required +// promoting those placeholder names in order to be allowed to fix them. +// +// Geometry and the vocabulary rules live in pure functions over zone objects in +// *world* metres, so every edit operation is testable under node with no canvas +// and no DOM. import { pixelToWorld, worldToPixel } from './map.mjs'; @@ -119,13 +128,101 @@ export function freshZone(existing, cx, cy, half = 1.0) { // input rather than at save. export const NAME_RE = /^[a-z][a-z0-9_]*$/; +// zone/v0's kinds, in the spec's order. Mirrored from `bundle.ZONE_KINDS` +// rather than fetched, because this is a `