Review: Tier 3 — Visual Authoring Tools (r1)¶
This review checks docs/designs/authoring/tier3-visual-tools.md against the current MAID codebase.
Issues¶
1) BLOCKER — The YAML round-trip story is not compatible with the current source-of-truth model¶
- Section reference: §2.1, lines 120-122; §4.8, lines 763-800; §18.1; Appendix A
- What's wrong: The document treats the visual editor, in-game building, and YAML as three surfaces over the “same underlying API,” then promises round-trip YAML import/export for rooms. That is not how the current codebase works. The admin world API mutates the live runtime world directly via
world.register_room()/world.unregister_room()(packages/maid-stdlib/src/maid_stdlib/api/admin/world.py:595-596, 656, 748-749) and exposes no file path, content-pack identity, definition ID, or provenance onRoomResponse/GraphNode(world.py:42-54, 192-208;packages/maid-engine/admin_frontend/src/types/api.ts:161-219). The document also says the export format is “the same format as@export,” but the actual builder export command writes loader YAML shaped like{_meta: {schema: ...}, <entity_type>s: [...]}with component payloads, not thearea:/rooms:/exits:structure shown in §4.8 (packages/maid-stdlib/src/maid_stdlib/commands/building/export.py:141-158). - Suggested fix: Pick a real source of truth. Either (a) make Tier 3 explicitly a live-world editor with a separate export pipeline, or (b) add provenance/file-targeting to the backend so edits can be mapped back to pack files. Do not claim “same API” or “same format as
@export” until the document specifies the translation layer and file ownership model.
2) BLOCKER — The collaboration/WebSocket design is incompatible with the existing /admin/ws protocol¶
- Section reference: §3.5, lines 473-515; Appendix B, lines 2671-2781
- What's wrong: The design assumes
/admin/wscan carry aneditorchannel with custom message types likecursor_move,lock_acquire,entity_update, andselection_change. Today it cannot. The only supported channels aremetrics,entities,logs, andevents(packages/maid-engine/src/maid_engine/api/admin/websocket.py:185-191, 321-323), and the message handler only understandsping,pong,subscribe, andunsubscribe(websocket.py:537-559). Unknown channels return an error (websocket.py:379-387). The router docstring for/admin/wsconfirms only those four channels (packages/maid-engine/src/maid_engine/api/admin/router.py:1209-1230). - Suggested fix: Rewrite the collaboration section around a concrete protocol change: new enum values, auth flow, channel registration, server-side handlers, rate limits, and broadcast behavior. As written, Appendix B is describing a protocol that the server will reject.
3) BLOCKER — The document is internally inconsistent about the editor WebSocket endpoint¶
- Section reference: §3.1, lines 201-203; §3.5, lines 473-475; Appendix B, line 2671
- What's wrong: The architecture diagram shows an
Editor WebSocket Hubat/admin/ws/editor, but the collaboration protocol later says “A new WebSocket channeleditorextends the existing/admin/wsendpoint,” and Appendix B repeats “via/admin/wswithchannel: "editor".” Those are different designs with different auth, routing, fan-out, and compatibility consequences. The current codebase has/admin/ws,/admin/dashboard/ws, and/admin/logs/stream— nothing at/admin/ws/editor(packages/maid-engine/src/maid_engine/api/admin/router.py:1204-1319). - Suggested fix: Choose one design. If it is a channel on
/admin/ws, document the channel/handler changes. If it is a separate endpoint, specify why it should not reuse the existing manager and what auth/origin logic it inherits.
4) BLOCKER — The review/changeset workflow conflicts with how the current admin API applies mutations¶
- Section reference: §16.4, lines 2242-2266; Appendix A, lines 2653-2666
- What's wrong: The design assumes a git-like staging area: create a changeset, review it, then apply it to the live world. The current admin APIs do not have any staging boundary. Room CRUD immediately mutates the world (
packages/maid-stdlib/src/maid_stdlib/api/admin/world.py:557-607, 619-667, 679-763), and entity CRUD/components/tags do the same in the entity router (packages/maid-stdlib/src/maid_stdlib/api/admin/entities.py). There is no pending state, no diff store, no transaction log, and no apply/reject mechanism. - Suggested fix: Treat changesets as a major backend subsystem, not an appendix bullet list. The design needs storage, diff format, authorization rules, live-world reconciliation semantics, and failure/rollback behavior.
5) HIGH — The document overstates the current undo/redo foundation; the existing implementation is not identity-preserving¶
- Section reference: §3.4, lines 420-471; §4.3, lines 682-684
- What's wrong: The document presents
useUndoRedoas a strong base for Tier 3 collaboration and broad editor operations. In reality, the current world-page undo/redo is a local UI convenience and already loses identity. Undoing a deleted room recreates a new room with a new UUID instead of restoring the old one (packages/maid-engine/admin_frontend/src/pages/World.tsx:431-439), and redoing a created room also creates a fresh room rather than the original entity (World.tsx:484-492). The recorded reverse data for room deletion does not even store the deleted room ID or exits (World.tsx:609-623). - Suggested fix: Do not build the broader design on top of the existing hook without first defining durable operation identities, reversible payloads, and server acknowledgement/versioning. Right now the doc is treating a page-local stack as if it were a real command log.
6) HIGH — The proposed map/editor UI assumes data that the current world API does not expose¶
- Section reference: §4.2, lines 620-665; §4.6-4.7, lines 724-759; §5; §16
- What's wrong: The design leans on
GraphNode/GraphEdgeand the existing world APIs, but the actual payloads are much thinner than the proposed UI requires.GraphNodeonly hasid,label,area_id,area_name, andplayer_count(packages/maid-stdlib/src/maid_stdlib/api/admin/world.py:192-208;packages/maid-engine/admin_frontend/src/types/api.ts:206-219).RoomResponsehas basic room text, exit list, counts, and metadata, but no tags, NPC list, item list, history, coordinates, lock state, or source file (world.py:42-54;types/api.ts:161-171). The doc’s room node design, layers, inspector tabs, and collaboration badges all need more backend data than the “existing foundation” actually provides. - Suggested fix: Add a gap analysis table: which proposed panels can use current endpoints, which require enriched room graph payloads, and which need entirely new APIs.
7) HIGH — The scalability section assumes viewport-aware graph loading that does not exist¶
- Section reference: §17.1-17.3, lines 2287-2364; Appendix A, lines 2641-2644
- What's wrong: The current
GET /admin/world/graphendpoint returns the entire graph, optionally filtered by onearea_id(packages/maid-stdlib/src/maid_stdlib/api/admin/world.py:924-1043). There is no viewport query, no pagination, no coordinate index, no boundary-edge summary, and no persisted room positions. Yet the performance section talks as if “progressive loading” is an incremental enhancement on top ofgetWorldGraph(). It is not. The backend would need a different data model and probably a spatial index or stored layout map. - Suggested fix: Reframe large-world support as a first-class backend milestone. The document should specify how rooms get coordinates, where layout state lives, and how viewport queries are answered without scanning every room on every pan.
8) HIGH — The document claims live synchronization across surfaces, but room/world edits are not broadcast today¶
- Section reference: §2.1, lines 120-122; §16; §18.2
- What's wrong: The design says edits from CLI/YAML/web become immediately visible everywhere via WebSocket push. That is not true for rooms/world topology in the current implementation. The admin broadcaster only publishes metrics, entity events, logs, and generic system events (
packages/maid-engine/src/maid_engine/api/admin/websocket.py:736-862). The world admin router does not use the broadcaster at all; it has no room-created/room-updated/room-deleted broadcasts (packages/maid-stdlib/src/maid_stdlib/api/admin/world.py). So even basic “someone else added a room; my map updates” behavior does not exist. - Suggested fix: Add explicit room/world event broadcasting to the design, including payload shapes and which clients subscribe. Do not imply the current WebSocket foundation already covers world topology changes.
9) HIGH — Content-pack extension loading is underspecified and does not match current routing/static serving¶
- Section reference: §15.1-15.3, lines 2143-2168; Appendix A, line 2635
- What's wrong: The design proposes
get_editor_extensions()on packs plus frontend dynamic imports from/admin/packs/${manifest.frontend_module}/index.js. None of that exists today. TheContentPackprotocol supportsregister_api_routes()but notget_editor_extensions()(packages/maid-engine/src/maid_engine/plugins/protocol.py:173-188), and the current admin packs router exposes metadata/load/reload APIs, not static JS bundles (packages/maid-engine/src/maid_engine/api/admin/packs.py). The admin router currently just callspack.register_api_routes(router)against the main admin router (packages/maid-engine/src/maid_engine/api/admin/router.py:1124-1129), so there is no documented mount point for extension assets. - Suggested fix: Specify whether extensions are API-only metadata, static assets served by the web server, or bundled into the admin frontend at build time. The current design handwaves the hardest part: safe distribution and loading of pack-provided UI code.
10) HIGH — The optimistic-locking protocol is not implementable with the current room models/endpoints¶
- Section reference: §3.5, lines 512-515; §16.2, lines 2211-2221
- What's wrong: The document promises field-level optimistic locking, conflict dialogs for same-field edits, and 5-minute lock expiry. Current room APIs have no version field, no ETag/precondition, no
updated_at, and no lock state in the payload (packages/maid-stdlib/src/maid_stdlib/api/admin/world.py:42-54, 111-118).PUT /world/rooms/{room_id}just overwrites fields immediately (world.py:619-667). Without a compare-and-swap token or revision number, “slower writer conflict dialog” is not a protocol detail — it is impossible. - Suggested fix: Add revision/version semantics to mutable resources before describing conflict resolution behavior. Otherwise the design should honestly say it is server-enforced exclusive locking, not optimistic concurrency.
11) MEDIUM — The editor role matrix does not match the actual admin API permissions¶
- Section reference: §16.5, lines 2271-2279
- What's wrong: The matrix says BUILDER can delete “own items,” MODERATOR can broadly delete, etc. The current admin APIs are stricter and simpler. Room deletion requires
AdminRole.ADMIN(packages/maid-stdlib/src/maid_stdlib/api/admin/world.py:679-693), while room create/update/create-exit/delete-exit use BUILDER for some operations (world.py:557-565, 619-628, 777-787, 864-878). The entity routes similarly have explicit route-level roles, not ownership-aware delete semantics (packages/maid-stdlib/src/maid_stdlib/api/admin/entities.py). The design is inventing permission behavior that is not in the codebase. - Suggested fix: Align the table with actual route permissions, or explicitly mark the role model as a proposed API change and enumerate every route that must change.
12) MEDIUM — Mobile/PWA/offline support is treated as a polish item, but it is effectively a separate product¶
- Section reference: §2.3, lines 133-137; §2.6, lines 163-167; §14; §19.6, lines 2592-2606; Appendix G, lines 3216-3238
- What's wrong: The current admin frontend has no service worker, no PWA manifest, and no IndexedDB layer (
packages/maid-engine/admin_frontend/src). Yet the design puts collaboration, mobile, pack extensions, and offline/PWA into a single 4-week phase. This is especially unrealistic because the same phase also assumes touch-friendly React Flow editing, slide-over inspectors, xterm-based play mode compatibility, and sync queues. - Suggested fix: Split offline/mobile into a separate track with explicit browser-storage model, sync conflict behavior, and reduced mobile scope (tablet-only may be realistic; phone-based graph editing probably is not).
13) MEDIUM — The performance claims are aspirational and partly contradict the document’s own technical caveats¶
- Section reference: §17.2-17.5, lines 2329-2414
- What's wrong: The doc targets 10,000 rooms loaded in <8s and 60fps pan/zoom, while also acknowledging SVG/React Flow degrades above ~3,000-5,000 visible nodes (
lines 2329-2338). It then treats Canvas/WebGL as optional future modes. That is contradictory: the stated benchmarks depend on capabilities the design itself has not committed to implement. Worse, the current backend still returns whole-world graphs with no viewport loading. - Suggested fix: Replace the benchmark table with staged targets tied to concrete milestones: current all-graph mode, viewport API mode, and optional Canvas mode. Otherwise the performance section reads like wishful thinking.
14) MEDIUM — Testing strategy is far too thin for a feature set this stateful and UI-heavy¶
- Section reference: document-wide; especially phases 1-5
- What's wrong: The design spends thousands of lines on features but almost nothing on how they will be verified. The current admin frontend test setup is unit-level Vitest (
packages/maid-engine/admin_frontend/package.json:6-12, 23-42;src/__tests__/...). There is no serious plan here for end-to-end flows, multi-user collaboration tests, visual regression on canvases, websocket protocol tests, or accessibility checks. - Suggested fix: Add a test matrix: frontend unit tests, protocol/integration tests for WebSockets, browser E2E for editor flows, and multi-client collaboration scenarios. This cannot be left implicit.
15) MEDIUM — Accessibility and browser-compatibility requirements are mostly absent¶
- Section reference: document-wide; minor mentions in §4.5 and §4.13 only
- What's wrong: There is no substantive a11y plan for keyboard-only editing, focus management in slide-over inspectors/modals, screen-reader fallbacks for graph operations, color-contrast rules for area/layer coding, or reduced-motion behavior for force layouts. Browser support is also unspecified despite depending on React Flow, dynamic imports, IndexedDB, service workers, and websocket-heavy auth flows.
- Suggested fix: Add explicit non-functional requirements for a11y and supported browsers. Without them, the design is effectively desktop-Chrome-only by accident.
16) LOW — The dependency summary understates frontend migration risk¶
- Section reference: Appendix G, lines 3216-3238
- What's wrong: The document says “No new major frameworks are introduced,” but it also proposes React Flow addon packages at
@reactflow/*v12 while the current admin frontend usesreactflowv11.11.4 (packages/maid-engine/admin_frontend/package.json:14-22). Even if the exact package names are placeholders, the doc is hiding a likely graph-stack migration in a footnote. - Suggested fix: Call out React Flow version strategy explicitly: stay on the current package, migrate the whole graph stack, or avoid addon packages that force version churn.
Overall assessment¶
The document’s product direction is strong, and its summary of the current admin frontend baseline is mostly accurate: the repo really does have a React/Vite/Tailwind admin UI, a World.tsx page using React Flow, and a basic undo/redo hook. But the design repeatedly turns that baseline into a much stronger claim than the code supports.
The biggest problem is that it blurs three very different systems — live runtime editing, YAML authoring, and collaborative review/staging — into one “same API” story. That is not true in the current MAID codebase. Before this moves forward, the design needs a sharper boundary between: 1. live-world admin editing, 2. pack/YAML file authoring, 3. collaboration/changeset storage, 4. extension asset loading, and 5. large-world performance architecture.
Until those are specified, this is an ambitious product sketch, not an implementation-ready technical design.