Review: Tier 3 — Visual Authoring Tools (r2)¶

This review checks docs/designs/authoring/tier3-visual-tools.md against the current MAID codebase and focuses on whether the r1 blockers/highs were actually fixed.

Verification summary¶

Check	Result	Notes
1. Tier-1-compatible YAML export	FAIL	`_meta.schema` is present now, but the documented room shape is still not the canonical MAID room shape.
2. WebSocket references use `/admin/ws` + channel	PASS	I found no remaining `/admin/ws/editor` references.
3. Changesets removed from v1	PASS	§16.4 and Appendix A now defer changesets/review to post-v1.
4. Undo/redo honestly requires durable op log	PASS	§3.4 now accurately says Tier 3 needs a new durable operation log.
5. API gap analysis table exists	PASS	§3.2a exists, though it still misses some critical gaps noted below.
6. Scalability reframed as backend milestone	PASS	§17 now stages large-world support as backend work.
7. Room broadcasting added	PARTIAL	Added in the doc, but still insufficient for non-admin mutations.
8. Pack extensions duck-typed	PASS	§3.6/§15 now use optional duck-typed `get_editor_extensions()`.
9. Exclusive locking for v1	PASS	§16.2 is now framed around server-enforced exclusive locks.
10. Role matrix matches actual permissions	FAIL	MODERATOR/BUILDER behavior is still misstated.
11. Mobile/PWA split into separate track	PARTIAL	Later sections do this, but the executive summary still advertises PWA in Tier 3 deliverables.
12. Performance claims tied to staged milestones	PASS	§17.5 is now staged.
13. Testing strategy section exists	PASS	§18a added.
14. Accessibility section exists	PASS	§18b added.

Issues¶

1) BLOCKER — The “canonical” room YAML example is still not actually MAID’s canonical room format¶

Section reference: §4.8, especially lines 1053-1138
What’s wrong: The document fixed the _meta.schema part, but the room example is still not aligned with the real Tier 1 room shape. MAID’s room fixtures and loader tests use top-level exits, not components.ExitsComponent, e.g. packages/maid-engine/tests/fixtures/data/valid/rooms.yaml:1-19 and packages/maid-engine/tests/loader/test_prepare_phase.py:19-31. The loader explicitly treats exits as an allowed top-level room field (packages/maid-engine/src/maid_engine/loader/entity_types.py:12). Meanwhile the real @export command emits a document shaped as {_meta: {schema: ...}, <entity_type>s: [...]} where each exported item contains _id, components, and tags (packages/maid-stdlib/src/maid_stdlib/commands/building/export.py:141-152).

The doc also still says import uses the admin API via “the same code path as maid data load” (§4.8 line 1072). That is not true in the current codebase: the admin room API mutates the live world directly with world.register_room() / world.unregister_room() (packages/maid-stdlib/src/maid_stdlib/api/admin/world.py:595-596, 656, 749), while the loader pipeline is a separate path. - Suggested fix: Replace the example with the actual canonical room shape MAID already accepts: _meta.schema, rooms:, components:, and top-level exits. Remove ExitsComponent from the example unless the backend is explicitly being changed to support it. Also stop claiming import uses the same code path as maid data load unless a real translation/loader-backed import path is specified.

2) HIGH — The new room broadcasting design still does not support the document’s own “cross-surface sync” claim¶

Section reference: §3.5 lines 701-716; §18a.5 lines 2991-2995
What’s wrong: The design now adds room broadcasts to the admin world router, but the doc also promises: “In-game @dig north → map editor shows new room within 2s” (§18a.5). That does not follow from the proposed design. The proposed broadcasts are emitted from admin HTTP handlers, while in-game building commands mutate the world directly. For example, @dig calls world.register_room() directly in packages/maid-stdlib/src/maid_stdlib/commands/building/dig.py:249-280, bypassing the admin router entirely. So even if the router broadcasts are implemented exactly as written, the stated cross-surface sync behavior still will not happen for in-game edits.
Suggested fix: Move room/exit broadcasts to a shared world-mutation/event layer so admin routes, builder commands, and any other room mutation path all emit the same topology events. If that is not planned for v1, remove the “cross-surface sync” claim and the test case that depends on it.

3) HIGH — The NPC “live preview” still claims an existing backend integration that does not exist¶

Section reference: §6.3 lines 1592-1594
What’s wrong: The document says the live preview “connects to the existing NPCDialogueSystem via the /admin/ws WebSocket, sending test messages and receiving AI responses without needing to be in-game.” The current /admin/ws implementation does not support anything like that. It only documents four channels (metrics, entities, logs, events) in packages/maid-engine/src/maid_engine/api/admin/router.py:1214-1230, and its message handler only processes ping, pong, subscribe, and unsubscribe in packages/maid-engine/src/maid_engine/api/admin/websocket.py:160-192, 541-562. There is no dialogue-preview channel, no message type, and no admin-side bridge to NPCDialogueSystem.
Suggested fix: Add a concrete gap-analysis entry and protocol/API design for dialogue preview (new endpoint or new /admin/ws message family), including auth, payloads, rate limits, and how preview context is built. Otherwise mark NPC live preview as post-v1.

4) HIGH — The role matrix still does not match the actual admin permission model¶

Section reference: §16.5 lines 2671-2703
What’s wrong: The matrix still says “For world editing, MODERATOR has the same permissions as BUILDER” (§16.5 lines 2701-2703). That is false in the actual code. AdminRole is an ordered hierarchy where MODERATOR = 20 and BUILDER = 30 (packages/maid-engine/src/maid_engine/api/admin/auth.py:81-92), so moderators do not inherit builder rights. The world routes require BUILDER for create/update/create-exit/delete-exit and ADMIN for delete-room (packages/maid-stdlib/src/maid_stdlib/api/admin/world.py:557-560, 619-623, 679-682, 777-780, 864-868).
Suggested fix: Update the matrix so MODERATOR remains player-management-only for current MAID behavior. If the intended product decision is to let moderators edit world content, call that out as a proposed auth change and list every route that must be relaxed.

5) MEDIUM — The executive summary still contradicts the later v1 scoping decisions¶

Section reference: §1 lines 98-100; §14.4 lines 2407-2423; §16.4 lines 2639-2667
What’s wrong: The later sections correctly split PWA/offline and review workflow out of v1, but the top-level summary still says Tier 3 delivers “Mobile Builder | Touch-optimized responsive layout, PWA support” and “Collaboration | Real-time cursors, lock/unlock, review workflow.” That is not the current scope the rest of the document describes.
Suggested fix: Make the executive summary match the real v1 scope. Change those rows to something like “tablet-responsive layout” and “presence + exclusive locks + change log,” with PWA/offline and review workflow explicitly labeled post-v1.

6) LOW — Layout persistence is still described inconsistently across sections¶

Section reference: §2.6 lines 212-218; §4.11 lines 1174-1175; §17.3 lines 2800-2802; Appendix G line 3877
What’s wrong: The document alternates between localStorage and IndexedDB for layout/browser persistence. §2.6 says localStorage for layout positions; §4.11 says positions are saved in the browser via IndexedDB; §17.3 again says localStorage for layout positions and only brings in IndexedDB in Stage 3; Appendix G says idb-keyval / IndexedDB is deferred. This is not fatal, but it is still inconsistent enough to confuse implementation.
Suggested fix: Pick one v1 browser-storage mechanism for editor layout state and use it consistently throughout the document. If IndexedDB is post-v1, keep it out of v1 sections.

Overall assessment¶

This is much better than r1. The major fixes are visible: /admin/ws/editor is gone, changesets are explicitly descoped, undo/redo is described honestly, the API gap table exists, scalability/performance are staged, pack extensions are duck-typed, and testing/accessibility sections were added.

However, I would not ship this yet. The biggest remaining problem is that the document still overclaims parity with MAID’s real room/YAML model, and it still promises behaviors (cross-surface sync, NPC live preview, moderator-as-builder permissions) that the documented backend design does not actually support.